[Fix] Fast turns killed mid-flight are closed out instead of vanishing - #2078
Closed
daniel-lxs wants to merge 1 commit into
Closed
[Fix] Fast turns killed mid-flight are closed out instead of vanishing#2078daniel-lxs wants to merge 1 commit into
daniel-lxs wants to merge 1 commit into
Conversation
Contributor
|
No code issues found. See task
Reviewed fdf2e4b |
daniel-lxs
force-pushed
the
fix/fast-dead-turn-closeout
branch
from
September 3, 2026 15:46
8cc0a83 to
2712352
Compare
daniel-lxs
force-pushed
the
fix/fast-dead-turn-closeout
branch
from
September 3, 2026 15:55
2712352 to
362cb95
Compare
daniel-lxs
force-pushed
the
fix/fast-dead-turn-closeout
branch
from
September 3, 2026 16:01
362cb95 to
17b874c
Compare
daniel-lxs
force-pushed
the
fix/fast-dead-turn-closeout
branch
from
September 3, 2026 16:18
17b874c to
b26efaa
Compare
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
force-pushed
the
fix/fast-dead-turn-closeout
branch
from
September 3, 2026 16:25
b26efaa to
fdf2e4b
Compare
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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_SECONDSis 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_eventsgainsshutdown_atandsettled_at(migration 0074, additive, ignored by the previous release).shutdown_atalongside 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.settled_atwhen 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.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=60on 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
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.shutdown_atlanded 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 withinterruptionReason=api_shutdownby 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: ..." withresumesTurnIdpointing 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.)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.