Reset the messaging delivery claim on each turn

Thomas Schmidtrequested to merge
628435-messaging-turn-reset into master

Why are we doing this?

Duo messaging sessions (Slack today) deliver a turn's answer through ResultDelivery#deliver_success, which returns early when workflow.messaging_callback_context['delivered_at'] is present. We use delivered_at to ensure that we don't do duplicate deliveries.

The claim is reset by convention: whatever starts a new turn must clear it, or the next turn's answer is silently skipped. The service that did this, Ai::Messaging::ResumeServerSideFlowService, is removed in !256183 (merged), and its replacement (Ai::DuoWorkflows::ExecuteRunService, added in !255976 (merged)) does not reset it. So a continuation — a reply to an input_required session, or a tool-approval resume — would enqueue turn 2, the answer would be produced, and deliver_success would skip it because turn 1's delivered_at was still set. Nothing user-facing ships that path yet, so the bug is latent; this fixes it before a reply feature lands on it. It came out of review on !255976 (merged).

What does this MR do?

Makes the reset adapter-owned and applies it on every Workhorse continuation:

  • Ai::Messaging::Adapters::Base#per_turn_reset returns the universal reset, { 'delivered_at' => nil }. The Slack adapter overrides it to add its own per-turn key, status_ts — the progress message deliver_result edits in place, cleared so a new turn posts its own message instead of overwriting the answer the user is still reading. No adapter-specific key appears in the service or workflow layers.
  • Workflow#reset_messaging_turn! resolves the adapter from the persisted context (messaging_callback_context['adapter'] via AdapterRegistry) and merges its per_turn_reset. It no-ops when there is no callback context or no registered adapter — a non-messaging session has nothing to reset.
  • ExecuteRunService#start_workhorse_run calls it on a continuation (!workflow.created?), immediately before ServerSideTurnWorker.perform_async. That placement matters: the reset re-opens the previous turn's delivery gate, so it must run only once the new turn is certain to be enqueued — otherwise a continuation that fails validation would re-open the gate with no new turn to close it. A first turn does not reset — a fresh context has nothing to clear.

The CI path deliberately does not reset. start_ci_run's provisioning and token guards can still fail after a reset would run, and no messaging surface resumes on CI today. Applying the reset uniformly on both runtimes — once the consolidated dispatch runs after provisioning has succeeded — is tracked in #629005.

The approval path in !255538 (merged) enters through ExecuteRunService, so it inherits the reset with no change of its own.

References

How to set up and validate locally

Specs cover the behavior. To poke it by hand in a GDK console (bundle exec rails console):

wf = Ai::DuoWorkflows::Workflow.last # a session that already delivered a turn
wf.claim_messaging_callback_delivery # simulate turn 1's delivery
wf.messaging_callback_context['delivered_at'] # => present

# A Workhorse continuation reopens the gate before enqueueing the next turn:
Ai::DuoWorkflows::ExecuteRunService.new(wf, event: { type: :input, text: 'ok, fix it' }).execute
wf.reload.messaging_callback_context['delivered_at'] # => nil, so turn 2 can be delivered

The regression spec drives this end to end on the Workhorse path: deliver turn 1, run a continuation through ExecuteRunService, assert the gate reopens so turn 2's deliver_success would not skip. A second spec pins the failure case: a continuation that fails validation after the claim was set leaves the gate closed.

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