Skip to content

[Fix] Fast turns killed mid-flight are closed out instead of vanishing - #2078

Closed
daniel-lxs wants to merge 1 commit into
developfrom
fix/fast-dead-turn-closeout
Closed

[Fix] Fast turns killed mid-flight are closed out instead of vanishing#2078
daniel-lxs wants to merge 1 commit into
developfrom
fix/fast-dead-turn-closeout

Conversation

@daniel-lxs

@daniel-lxs daniel-lxs commented Sep 2, 2026

Copy link
Copy Markdown
Member

Summary

Follow-up to #2016 and #2040, from a staging incident on 2026-09-02: a deploy killed the API while a Fast turn was mid-flight. The turn had already done something non-replayable, so it was correctly not resumed, but the process was killed before the shutdown drain could post the restart closeout. Result: no message in the thread, no interruption marker for the next message to resume from, and a session that read as active until the 15-minute lease lapsed.

Root cause on the platform side: Railway's default draining time is 0 seconds (SIGTERM immediately followed by SIGKILL), so the 20-second drain from #2006 never gets to run unless RAILWAY_DEPLOYMENT_DRAINING_SECONDS is set. Managed deployments get 60s for api and bullmq from the Cloud deployer; staging needs the same, see below. This PR makes the app robust either way.

Changes

  • fast_agent_parent_events gains shutdown_at and settled_at (migration 0074, additive, ignored by the previous release).
  • Stop signal stamp. On SIGTERM the API stamps every active durable turn with shutdown_at alongside the drain rather than after it, so a kill that follows immediately still leaves evidence. The stamp also shortens each stamped turn's Redis lock to the drain window plus a minute, through the lock's own ownership-checked renew script, so a killed process cannot pin its conversation (and its dead turn's closeout) behind the lock's full 600s TTL; a turn that finishes releases the lock itself and a live turn's renewal extends it again.
  • Worker parity. Resumed turns run in the worker, whose shutdown previously did none of this: no drain, no abort, no lock release. It now runs the same bounded sequence as the API (stop admitting, stamp and shorten, let in-flight turns finish, abort the stragglers), which also means an interrupted resumed turn gets its durable claim released and the queue woken instead of waiting out the claim lease. To make the stamp possible at all, the queue drain now binds the executing row to the turn lock the way the webhook handlers do, and every accepting path binds through one helper: a row bound after the stop signal's one-time stamp pass (a worker between claiming its row and binding it) is stamped and its lock shortened at bind time, so it cannot die unmarked.
  • Settled mark. A turn marks settled_at when the user has an outcome: answer delivered, or withdrawn from replay and closed out (including the restart closeout after an abort). A parked turn does not settle; its outcome belongs to the resumed run.
  • Dead-turn reconciler. The sessions reconcile job (every 60s) closes out inline turns that are withdrawn or delivered but never settled when either the stop stamp is older than a minute (drain window plus margin) or the row is stale and the responding lease has lapsed (crash without SIGTERM). It leaves any conversation whose turn lock is still held alone (a process is executing it), and does its re-checks and writes in one transaction under the conversation advisory lock, so a turn finishing concurrently can never end up with an interruption after its real reply. Because a killed process never releases its lock, the stamp step also shortens each bound turn's lock to the drain window plus a minute; a turn that finishes releases it anyway, and a live turn's renewal extends it again. It writes the honest restart closeout as a visible transcript row with an interruptionReason (so [Fix] A nudge after an interrupted Fast turn starts a context-free conversation #2010's unresolved-request envelope fires on the next message), releases the responding lease so the session stops reading as active, and settles the row. A turn that already has a terminal reply is settled without a second closeout. Pending rows are never touched; recovery re-runs those.

Not in scope: posting the closeout to the chat surface from the reconciler. The transcript row and the unresolved-request carry-over are what matter for the next message; a Slack post from the reconciler would need the surface adapter in the worker and can follow if the silence in the thread turns out to matter.

Staging config

Set RAILWAY_DEPLOYMENT_DRAINING_SECONDS=60 on the staging api and bullmq services (matching what the Cloud deployer applies to managed deployments). Without it, every deploy still cuts in-flight turns short; with this PR they at least get closed out honestly within about a minute instead of silently.

Tests

  • Real-DB repository test: fresh stamp not acted on, stamp older than the window closes out and releases the lease, second pass is a no-op, settled and pending and already-spoken-for rows are left alone, lapsed-lease crash path, stamp refused after settle.
  • Turn lock: stamps only bound rows, keeps going when a stamp fails.
  • API shutdown: stamp runs alongside the drain and before the abort.
  • Service: settled after delivery and after the restart closeout, not settled when parked.
  • Queue: resumed inline rows are bound to the lock during the run.

Smoke (local, per the PR smoke-testing skill)

PM2 stack run from this branch's worktree at 4f3d7ca7 (PM2 cwd and runtime PID verified), mock Slack harness, migration 0074 applied. Signals were sent to the validated API runtime process directly. Evidence: fast_agent_parent_events, fast_agent_messages, sessions.responding_until, mock Slack thread, worker log.

  • Railway-style kill (SIGTERM, SIGKILL 300ms later) during a non-replayable turn. The turn had saved a memory (withdrawn from replay) and was generating a long answer. shutdown_at landed in the same second as SIGTERM and the turn's Redis lock TTL dropped from 600s to 79s on the spot. The dying process posted nothing. On the final code (16b15b7d) the lock expired 80s after the kill and the next reconcile tick, 88s after the kill, wrote the restart closeout with interruptionReason=api_shutdown by converting the hidden retry notice in place (one closeout row), released the lease (session no longer active), and settled the row. An earlier run of the same scenario before the liveness fence closed out at 97s. A bare "hey" in the thread then produced "Picking this back up: ..." with resumesTurnId pointing at the dead turn, so the [Fix] A nudge after an interrupted Fast turn starts a context-free conversation #2010 carry-over fires from the reconciler's row. (The nudge waited for the dead process's Redis turn lock to expire first; that 10-minute lock TTL after a crash is pre-existing and separate.)
  • Found and fixed during the run: the dead turn had a hidden retry notice active, and after the lease was released the retry-notice reconciler flipped it into a second visible interruption line. The dead-turn closeout now converts an active notice in place; the DB test covers it.
  • SIGTERM only (drain completes). Stamped at SIGTERM, the turn finished inside the drain and settled, the process exited after the drain, and after a full reconcile tick the transcript still has exactly one row (the answer). No false closeout.
  • SIGKILL only (crash, no stamp). The lease was backdated to simulate the 15-minute lapse; the next tick closed the turn out with expired_lease_reconcile, one closeout row, lease cleared, settled. The same path also closed out two genuinely dead turns left over from earlier local runs on its first tick, with no side effects on live rows.

Not exercised live: the worker-side shutdown path (a resumed turn running in bullmq when the worker is killed) and non-Slack surfaces; unit tests cover the row binding, the stamp, the ownership-checked shortening, and the settle rules.

@roomote-community

roomote-community Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

No code issues found. See task

  • fast-agent-service.ts:4194 Treats a best-effort persistence failure as a recorded shutdown closeout, suppressing the repair marker.
  • fast-agent-conversation-repository.ts:578 Reconciles under the conversation advisory lock and re-checks terminal state before closeout.
  • fast-agent-turn-lock.ts:104 Shortens a successor's lock without verifying ownership, risking a duplicate or dead-turn reconciliation.
  • apps/bullmq/src/index.ts:362 Worker shutdown did not shorten resumed-turn locks, delaying dead-turn closeout after an immediate kill.
  • apps/bullmq/src/index.ts:385 Waits for durable-row stamping after the drain deadline, so a slow stamp still extends shutdown and delays claim release.
  • fast-agent-conversation-repository.ts:610 A delivered progress-only response can be converted into a false restart closeout if the process dies between its separate delivery and settlement writes.
  • schema.ts:3191 The minute-by-minute dead-turn scan has no usable index for unsettled inline terminal rows and will scan historical parent events as the table grows.
  • packages/db/drizzle/0075_freezing_tigra.sql:1 Replaces an already journaled migration identity, causing databases that applied the prior version to re-run the duplicate ADD COLUMN statements.
  • fast-agent-turn-lock.ts:94 Misses durable rows bound after the one-time shutdown snapshot, leaving immediately killed turns unstamped and their locks unshortened.
  • fast-agent-service.ts:4316 Marks a retry-notice shutdown closeout settled when the surface replacement succeeds but its best-effort canonical persistence fails.

Reviewed fdf2e4b

Comment thread packages/cloud-agents/src/server/fast-agent/fast-agent-service.ts Outdated
Comment thread packages/cloud-agents/src/server/fast-agent/fast-agent-conversation-repository.ts Outdated
Comment thread packages/cloud-agents/src/server/fast-agent/fast-agent-turn-lock.ts Outdated
Comment thread apps/bullmq/src/index.ts Outdated
Comment thread apps/bullmq/src/index.ts Outdated
@daniel-lxs
daniel-lxs force-pushed the fix/fast-dead-turn-closeout branch from 8cc0a83 to 2712352 Compare September 3, 2026 15:46
Comment thread packages/db/src/schema.ts
@daniel-lxs
daniel-lxs force-pushed the fix/fast-dead-turn-closeout branch from 2712352 to 362cb95 Compare September 3, 2026 15:55
Comment thread packages/db/drizzle/0075_freezing_tigra.sql Outdated
@daniel-lxs
daniel-lxs force-pushed the fix/fast-dead-turn-closeout branch from 362cb95 to 17b874c Compare September 3, 2026 16:01
Comment thread packages/cloud-agents/src/server/fast-agent/fast-agent-turn-lock.ts
@daniel-lxs
daniel-lxs force-pushed the fix/fast-dead-turn-closeout branch from 17b874c to b26efaa Compare September 3, 2026 16:18
Comment thread packages/cloud-agents/src/server/fast-agent/fast-agent-service.ts Outdated
A process killed between SIGTERM and the end of its drain (Railway's
default grace is zero seconds) left an active Fast turn with no closeout,
no interruption marker for the next message to pick the request up from,
and a session that read as active until the lease lapsed.

Every inline turn now records a stop signal the moment its process
receives one, before the drain and alongside it, and records when it
reached a user-visible outcome. The stamp also shortens the turn's Redis
lock (ownership-checked) so a killed process cannot pin its conversation
behind the lock's full TTL, and it can never delay the straggler abort or
the exit. The worker runs the same bounded sequence as the API, so
resumed turns get the drain, abort, claim release and queue wake they
previously lacked.

The sessions reconciler closes out the rest: an unsettled turn whose
process was told to stop over a minute ago, or whose lease has lapsed,
gets the honest restart closeout, its session stops reading as active,
and the next human message carries the unresolved request. It skips
conversations whose turn lock is still held and does its re-checks and
writes under the conversation advisory lock, so a turn finishing
concurrently can never end up with an interruption after its real reply.
Pending rows stay with recovery. Additive columns, N-1 safe.
@daniel-lxs
daniel-lxs force-pushed the fix/fast-dead-turn-closeout branch from b26efaa to fdf2e4b Compare September 3, 2026 16:25
@daniel-lxs

Copy link
Copy Markdown
Member Author

Superseded by #2145, which makes every interrupted Fast turn resume automatically with a record of what its earlier attempt already did. Closing; can be reopened if that approach does not land.

@daniel-lxs daniel-lxs closed this Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant