Fix duplicate and stale Slack progress messages for Duo

What does this MR do and why?

The problem, from the user's side

When you mention GitLab Duo in Slack (behind the default-off slack_duo_api_flow flag), Duo replies in the thread with one message. That message shows progress (plan, what the agent is doing) and is edited into the final answer. Since tool approvals were added (the agent pauses and asks before, for example, creating an issue), that reply falls apart in three ways:

  1. The first progress text is posted twice. This happens when the agent pauses for an approval before it has posted anything, which is the normal approval path. On GDK both copies were posted 26 ms apart.

image.png

2. After you approve, the agent's earlier text is posted again as a new message, instead of the reply continuing. With two approvals in a row, the thread fills with repeats of the same sentence.

image.png

3. A late progress update can overwrite the final answer, or leave a stray progress message below it.

Why this happens

Several independent jobs write the same Slack message: ProgressDeliveryWorker (after each checkpoint), CallbackWorker (the approval request, which first flushes pending progress; failures; CI answers), ServerSideTurnWorker (Workhorse answers) and the approval decision. They share state through messaging_callback_context, and each job decides based on the copy it read when it started.

  1. Double first post: the approval flush and the progress worker both saw no status_ts (the message pointer) and both posted.
  2. Re-post after an approval: Slack's per_turn_reset cleared status_ts on every continuation. That rule was written for reply turns of the postponed thread-continuity work (!255094 (merged), !255095 (closed)). But in Slack today the only continuation is an approval resume, and !256391 (merged) applied the reset to it.
  3. Late overwrite: the progress worker checks delivered_at when it starts, possibly on a lagging replica. The answer can land while the tick is still rendering.

A latent fourth issue: writers merged back the status_ts they read at job start, which could overwrite a newer value stored by another job.

What this MR changes

  1. Keep the Slack message when a session continues. Delete Slack's per_turn_reset override. A continuation now only reopens the delivery claim, so the resumed run keeps editing the message the user is watching.
  2. Let only one job open the progress message.
    • Opening the first message takes a short Gitlab::ExclusiveLease, the same try_obtain / cancel-in-ensure pattern as the other Duo workflow services. Then it checks status_ts on the primary, so a job holding a stale copy of the context can't post a second message, even after the lease expires. The losing job skips, and the next progress tick edits the winner's message.
    • Writers stop writing back status_ts: the adapter persists it, and ProgressDeliveryWorker persists only its cursor.
  3. Recheck delivery before writing progress. Right before a progress write, the adapter reads delivered_at from the primary.

The model gains one method, messaging_callback_context_on_primary (a single-row pick on the primary, the same shape as MergeRequest#state_changed_since_load?). No raw SQL is added, and claim_messaging_callback_delivery is unchanged from master.

Commit 2 first used a jsonb claim for the opener; commit 4 replaces it with the lease after review. Reviewing the full diff is simpler than reviewing commit by commit.

Result: each mention gets one reply message (progress, then the answer), plus one message per approval request (request, then "Approved" or "Denied").

Options we considered and rejected
  • Post the reply message up front, at the mention. This would remove the opener race entirely. !246174 (merged) replaced exactly that with Slack's rotating loading status, because a static message made the run look stuck. We tested on GDK whether an up-front message could keep the indicator:

    • assistant.threads.setStatus survives an edit, but any new message in the thread, including the user's, clears it, and it times out after 2 minutes.
    • agents.sessions.setStatus, where the indicator persists, returns not_authorized without the (irreversible) agent_view migration.

    So an up-front message would look stuck again. Worth revisiting with the Agents API migration; the lease can be deleted then.

  • A jsonb claim, like delivered_at. Unlike delivered_at, status_ts only exists after the post, so the claim can't be the fact itself. It would need a separate marker that only matters while one job is posting, plus its own reset rule, and it would stay set if the job died mid-post. The delivery claim stays a jsonb claim: it's a lasting fact that later events and the progress worker read.

  • A waiting lease (in_lock). The loser would wait, then edit. But that means sleeping Sidekiq threads, and skipping is already correct for progress, because the next tick edits.

  • A sentinel value in status_ts. Other writers would call chat.update with a fake ts.

  • Dropping the approval flush. That brings back the approval request appearing above the agent's text.

Known limitations
  • After an approval, the final answer lands in the reply message, which sits above the approval message. This is intended: the approval stays a separate message, so the user gets notified.
  • The delivery re-check narrows the late-overwrite window to one Slack call; it does not close it. Closing it would need serialized writes.
  • deliver_result and on_flow_failed still open a message without the lease, as on master. A stray message below an answer is still possible in a narrow window. Flows without progress (for example "say hello") are unaffected: the answer is the only post.
  • When the approval flush loses the lease, the approval request can appear just above the agent's text.

Relation to the approval-decision MR

This MR is independent of !255538 (merged) and targets master. That MR still clears status_ts and pending_approval itself. It will get a matching change (record the answered fingerprint instead of clearing anything) proposed separately. Until then, behaviour after an approval is only fully fixed once both land.

References

Screenshots or screen recordings

Slack only; no GitLab UI changes. Two approvals in a row (create an issue, then comment on it), on GDK:

Before After
image.png image.png

How to set up and validate locally

Prerequisites

  • GitLab for Slack on GDK with the bot token and your Slack user linked, and Slack events and interactions reaching GDK.

  • Feature.enable(:slack_duo_api_flow).

  • To make sessions pause for an approval: add require_tool_approval: true to the slack_agent component in your GDK AI gateway (duo_workflow_service/agent_platform/v1/flows/configs/slack_assistant/1.0.0.yml, tracked in #629377), then run gdk restart duo-workflow-service.

  • Run gdk restart rails-web rails-background-jobs.

  • If no approval message appears, that's the known DWS race fixed in gitlab-org/modelops/applied-ml/code-suggestions/ai-assist!7008 (merged). Re-fire the hook from the Rails console:

    w = Ai::DuoWorkflows::Workflow.order(:id).last
    ctx = w.messaging_callback_context
    Ai::Messaging::Adapters::Slack.from_callback_context(ctx).on_approval_required(callback_context: ctx, workflow: w)

Helpers

State of the latest Slack session (Rails console):

w = Ai::DuoWorkflows::Workflow.where("messaging_callback_context->>'adapter' = 'slack'").order(:id).last.reload
[w.id, w.status_name, w.messaging_callback_context.slice('status_ts', 'pending_approval', 'delivered_at')]

Slack writes (shell):

tail -c 3000000 log/integrations_json.log | jq -c 'select(.message | test("posting message|updating message")) | {time, caller: .["meta.caller_id"], msg: .message}' | tail -15

Scenarios (replace <bot> and <project>)

  1. A pause before the first message posts no duplicate.

    @<bot> create an issue in <project> titled "Pause test" with the description "Session message test."
    • Expect: one progress message and one approval message with a Review button.
    • Log: exactly two posting message entries. When the approval flush loses the lease, only the approval request is posted by CallbackWorker.
    • State: status_ts set.
    • Answering the request needs !255538 (merged).
  2. Quick answers stay one message. Send @<bot> what's 2+2? three times in a row, then @<bot> summarize the last 3 messages in this channel in one sentence.

    • Expect: exactly one bot message per mention.
    • Log: no ProgressDeliveryWorker write after a session's final write.
  3. Progress keeps editing one message.

    @<bot> in <project>, list the 5 most recent issues and summarize each in one line

    While it runs, post a message of your own in the thread.

    • Expect: Duo's reply is edited in place into the answer, and no extra bot message appears. The loading indicator disappears when you post; that's Slack's behaviour and unchanged.
  4. Two sessions in one thread. Send two mentions a few seconds apart.

    • Expect: two replies, each answering its own question, and neither edits the other.
  5. Optional, with !255538 (merged) on top, including its follow-up that stops clearing context:

    @<bot> in <project>, create an issue titled "Double", then add a comment to that issue saying "Follow-up"

    Approve both requests.

    • Expect: three bot messages. The reply is edited through both approvals into the final answer, and the agent's earlier text is never re-posted.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Thomas Schmidt

Merge request reports

Loading
Loading