Stop the API losing track of live agent runs, and stop the evidence gate rejecting its own source ids - #92
Merged
Conversation
A session that finishes schedules `sessions.delete(projectId)` five minutes
later. If a follow-up run starts inside that window it takes the project's
slot, and the previous run's timer then evicted the *new*, running session.
Everything downstream reads that registry, so the API went blind to its own
agent while the child kept working:
- `resumeSession`'s "already running" guard consulted the empty map, so it
silently passed and let a resume collide with the live agent. Codex refused
the second writer -- "thread-store conflict: thread <id> already has an
active writer" -- and the resumed run died in ~300ms.
- `/answer` concluded nobody was polling the askUser prompt and auto-resumed
on top of the running agent, producing that same collision one second after
the answer had already been delivered.
- Each stillborn resume became the newest AiRun and therefore `latestRun`, so
the status endpoint reported the whole project as "failed" while the
estimate was still being built. Every Resume click appended another and
re-applied the mask.
On CB-260910-0001 the run survived all of it and finished normally 32 minutes
later, having never stopped -- only the UI ever thought otherwise.
Three changes:
- `reapSession` deletes only if the slot still holds that same session.
Extracted as a pure function over an injected Map so the takeover case is
testable without spawning a process.
- `probeLiveAgent` decides liveness by probing: registry first, then
`.bidwright/session.json` + `kill(pid, 0)`. `persistSessionState` now
records `ownerPid`, and a recorded pid is trusted only when this API
process spawned it -- a pid from an earlier process, or restored with a
workspace snapshot from another host, is not ours and pids get reused.
Both the resume guard and `/answer` use it.
- `selectLatestRun` skips a tail of stillborn runs (failed, and never emitted
`status: running`) while the last run that did start is still open. It
walks the whole tail, since each retry added one. When nothing else is
running the newest run still wins, so genuine start-up failures surface.
`selectLatestRun` lives in its own module because anything defined in
cli-routes.ts can only be tested by grepping its source text -- which is how
the resume guard stayed inert for three weeks while
`resume-session-guard.test.ts` asserted that it contained
`session.status === "running"`. That file now checks route wiring only; the
rule itself is covered behaviourally in cli-runtime-liveness.test.ts.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
On the Birla electrical quote 60 of 65 createWorksheetItem calls were
rejected. Three separate causes:
1. `looksLikeStructuredSourceRef` accepted only `-` or `:` after the prefix,
but SourceDocument mints `doc_<uuid>` and LineItemSearchDocument mints
`lis_<hex>`. Both scored zero structured refs, so a row citing a real source
document was rejected for "needs structured cite" no matter how accurately
it cited it -- and those are precisely the two ids an agent has for a
material row. Every other minted id (lu-, ds-, kb-, rsi-, ecost-, rci-)
passed, which is why this only bit material and composite rows.
The rejection text made it worse by naming `doc-` as the format to use, a
prefix nothing in the system produces. Gate messages now interpolate
STRUCTURED_SOURCE_REF_HINT, which lists real prefixes.
2. `sourceRefs` was `z.array(z.string())`, so when the agent read "structured
sourceRefs" as "send a structure" it got back a bare
`MCP error -32602: expected string, received object` with no hint at the
intended shape. It then alternated between object and string forms, each
rejected by a different layer for a different reason. The new
`sourceRefArray()` accepts either and normalizes objects to the string form
the gates read (`{documentId, page}` -> `doc_3b409f90 p.4`), dropping
entries with nothing identifiable rather than storing "[object Object]".
Applied to all ten sourceRefs schemas, which also fixes the
saveEstimateScopeGraph scopeItems[].sourceRefs rejection.
3. 39 calls arrived carrying only `worksheetId` and `entityName` -- the two
parameters without a default -- because the tool call was cut off while
being serialized. The server answered "Line evidence basis is required",
which is true and diagnostically useless: it sent the agent rewriting
evidenceBasis 39 times over a payload that never arrived intact. It worked
this out itself ("I'm repeatedly truncating my own tool call") and
eventually delegated the rows to a sub-agent. That shape is now detected and
reported as a truncated call, telling the agent not to retry the same way
and pointing at createRateScheduleWorksheetItem, whose smaller payload
succeeded 13 of 16 times in the same run against 5 of 65.
Tests replay the recorded prod payloads and are registered in CI, along with
the session-liveness tests from the previous commit.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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
1 participant
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.
Two production failures found on the Birla electrical quotes (CB-260910-0001 and -0002).
1. A stale reap timer blinded the API to a live run
A finished session schedules
sessions.delete(projectId)five minutes later. A follow-up run starting inside that window takes the slot, and the previous run's timer then evicted the new, running session.On CB-260910-0001: run 1 completed 16:48:59, the follow-up started 16:52:31, and at 16:53:59 run 1's timer wiped run 2's live entry — 3 seconds before it asked its scope question. From there:
resumeSession's "already running" guard read the empty map, passed silently, and let a resume collide with the live agent. Codex refused the second writer (thread-store conflict: thread <id> already has an active writer); the resumed run died in ~300ms./answerconcluded nobody was polling the askUser prompt and auto-resumed on top of the running agent — one second after the answer had already been delivered.AiRunand thereforelatestRun, so status reported the project as failed while the estimate was still building. Every Resume click appended another and re-applied the mask (8 in total).The run never stopped. It finished normally at 17:28:27 with the full 4-job quote.
Fixes: identity-checked
reapSession;probeLiveAgentdecides liveness by probing (registry, then.bidwright/session.json+kill(pid, 0), with a newownerPidso a pid from another API process or a restored snapshot is never trusted);selectLatestRunskips a stillborn tail while a real run is still open.2. The evidence gate rejected the source ids it asks for
60 of 65
createWorksheetItemcalls failed on CB-260910-0002.looksLikeStructuredSourceRefaccepted only-/:after the prefix, butSourceDocumentmintsdoc_<uuid>andLineItemSearchDocumentmintslis_<hex>. Both scored zero structured refs, so a row citing a real document was rejected for "needs structured cite" regardless of accuracy — and those are exactly the ids available for a material row. The message compounded it by namingdoc-, a prefix nothing produces.sourceRefswasz.array(z.string()), so object refs returned a bareMCP error -32602with no hint at the intended shape. NewsourceRefArray()accepts either and normalizes.worksheetId+entityName(the two params without defaults) because the tool call was cut off mid-serialization. The server blamed evidence, sending the agent rewritingevidenceBasis39 times. Now detected and reported as truncation.Testing
17 new tests across four files, replaying recorded prod payloads. Verified they catch the regressions: reverting the reap guard fails the resume test, reverting the regex fails the minted-id test.
Also replaces the inert assertions in
resume-session-guard.test.ts, which asserted thatresumeSessioncontained the stringsession.status === "running"— true for three weeks while the guard did nothing.Full focused suite: 325 pass, 0 fail. Both packages typecheck.
🤖 Generated with Claude Code