fix(cron): a session killed by the wall-clock cap was logged "success" - #21
Open
nyem69 wants to merge 1 commit into
Open
fix(cron): a session killed by the wall-clock cap was logged "success"#21nyem69 wants to merge 1 commit into
nyem69 wants to merge 1 commit into
Conversation
sessions.maxDurationMinutes kills the engine with an "Interrupted: ..."
reason. SessionManager treated every "Interrupted" prefix as a benign
user interrupt and flattened the row to status:"idle", lastError:null —
so the cron runner's `finalSession.status === "error"` test never fired
and appendRunLog wrote {"status":"success","error":null}. No reply was
delivered, no ops-alert, nothing in the row said a timeout happened.
75 runs across ~20 jobs went dark this way (jinn-group-watcher x24,
collect-telegram x10, collect-x x5, plan-url-queue-processor x4,
melaka-breaking-watch x4). The system-steward monthly audit failed 2/2
months — Jun 1 and Jul 1 — while its own run history read "success"
both times, which is how the blind spot stayed invisible: the steward's
reliability check reads that same field.
- shared/timeout.ts: SESSION_TIMEOUT_PREFIX sentinel + sessionTimeoutReason(),
mirroring SESSION_BUDGET_STOP_PREFIX. Still starts with "Interrupted" so
the engines' retry-skip and the existing idiom keep working, but is
distinguishable from "Interrupted by user" / "new message received".
- sessions/manager.ts: a timeout persists as status:"interrupted" with the
reason intact; benign interrupts still resolve to a clean idle row.
onForceInterrupt (engine never started) carries the same sentinel.
- cron/runner.ts: new terminal status "session_timeout" carrying the reason
and turn count, plus an ops-alert that names the duration and warns about
partial external writes when sideEffects:true. Latency alert suppressed on
a timeout — a wall-clock kill always trips it, so the turtle is noise.
- gateway/api.ts: same carve-out on the web-dispatch path, so /api/sessions/<id>
stops reporting a timed-out session as clean idle.
6 runner tests; 5 fail against the old runner. Full suite 804 passing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014ruD7fhpAnexqgKd1LxJTT
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.
The bug
sessions.maxDurationMinuteskills the engine with an"Interrupted: ..."reason.SessionManagertreated every"Interrupted"prefix as a benign user interrupt and flattened the row tostatus:"idle",lastError:null— so the cron runner'sfinalSession.status === "error"test never fired andappendRunLogwrote{"status":"success","error":null}.No reply delivered, no ops-alert, and nothing in the row saying a timeout happened. The only surviving trace was a large
durationMs.Blast radius
75 runs across ~20 jobs logged
"success"after running past the cap:jinn-group-watchercollect-telegramcollect-xplan-url-queue-processormelaka-breaking-watchThe
system-stewardmonthly audit failed 2/2 months (Jun 1, Jul 1) while its own run history read"success"both times — which is how the blind spot stayed invisible. The steward's reliability check reads that same field, so it reported "0 failures" for a job that failed every time it ran.The fix
shared/timeout.ts—SESSION_TIMEOUT_PREFIXsentinel +sessionTimeoutReason(), mirroringSESSION_BUDGET_STOP_PREFIX. Still starts with"Interrupted"so the engines' retry-skip and the existingstartsWith("Interrupted")idiom keep working, but is distinguishable from"Interrupted by user"/"Interrupted: new message received".sessions/manager.ts— a timeout persists asstatus:"interrupted"with the reason intact. Benign interrupts still resolve to a clean idle row (unchanged).onForceInterrupt(engine never started) carries the same sentinel so both timeout shapes classify identically.cron/runner.ts— new terminal statussession_timeoutcarrying the reason and turn count, plus an ops-alert naming the duration that warns about partial external writes whensideEffects:true.gateway/api.ts— same carve-out on the web-dispatch path, so/api/sessions/<id>stops reporting a timed-out session as clean idle.One deliberate call: the latency alert is suppressed on a timeout. A wall-clock kill always trips the threshold, so the 🐢 would double-fire on top of the failure alert.
Testing
6 new cases in
cron/__tests__/runner.test.ts. Verified meaningful — revertingrunner.tsalone fails 5 of 6; the sixth is the no-false-positive case that must pass either way. Full suite 804 passing,tsc --noEmitclean.Known gap, not addressed here
Per-job
maxDurationMinutesstill does not exist —CronJobhas no duration field.system-stewardwill now alert when it times out, but will still time out. The only lever today is settingemployee:on the job and puttingmaxDurationMinutesin that employee's org file. Worth a follow-up, following themaxTurns/sessionBudget.hardCappattern.Separately noted while tracing:
resultPreviewhas been hardcodednullat all three call sites since the Connectors rewrite (74d9afe) — 82,444/82,444 nulls. Dead field, left alone here.🤖 Generated with Claude Code
https://claude.ai/code/session_014ruD7fhpAnexqgKd1LxJTT