Repository navigation
fix(agent): make turn lifecycles explicit state machines - #2311
Conversation
Relay final render on timeout with checkpoint hand-off, one ChatSession slot transition writing each turn-end once, per-phase autonomous outcomes, socket open deadline and stale heartbeat clearing, executor unspawned-claim grace, and a terminal Closed progress feed.
…socket close Review follow-ups: interrupted renders keep their ending on the last posted message, autonomous rows settle in one uninterruptible step with bounded writes, and a socket that throws on close is still dropped and redialled.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 16 seconds. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
This comment has been minimized.
This comment has been minimized.
…rt settle path ConnectorSocket takes its connector registry as an optional constructor argument, so its test drives a fake heartbeat protocol instead of naming a vendor. The autonomous turn's abort and settle wiring moves to autonomous-settle.ts, tested for user abort, supersede, stale expiry and an abort that lands mid-settle.
Maple review🟡 Confidence 6/10 · needs attention. Held at 6 because a correctness warning is open. Reworks the autonomous turn into explicit settle/abort paths with coherent failure status, and makes the socket reconnect on a stalled handshake. The phase/settle logic reads correctly, but the new settle deadlines cannot cut a hung write.
Findings🟠 Warning · F1 ·
|
![maple-review-bot[bot]](https://avatars.githubusercontent.com/in/4108169?s=60&v=4){kind=small}
Makisuo
added
the
preview:web
🍁 Maple PR previewNote Preview resources were removed when the preview label came off. Final commit |
Preview test: turn lifecycles (
|
| # | Check | Result |
|---|---|---|
| 1a | Send a message: the turn streams and ends | PASS. Turns ran tool calls and text and ended with one turn-end reason=stop each |
| 1b | Send while a turn runs | PASS. 409 A turn is already running for this conversation. There were never two turns at once |
| 1c | Stop mid-stream | PASS. POST /abort 204 → one turn-end reason=aborted ~3s later. The next message got 202 right away and completed (ready) |
| 1d | Reload mid-turn | PASS. I dropped the stream after 5s and reloaded the page. A send probe got 409 (the turn was still held). The tail resumed from the cursor and got exactly one turn-end, and the next message worked |
| 2a | Investigation runs to a diagnosis | PASS. A went investigating → diagnosed in ~38s. progress.step_count rose live (13 → 25 → 37 on B) |
| 2b | Stop an investigation | PASS. Abort during B's autonomous pass → failed within 3s, error interrupted: the run was stopped before it submitted a diagnosis; retry. It did not hang in investigating. A follow-up afterwards worked |
| 2c | Second start while one runs | PASS for the turn slot: only one run. A restart on running B was refused. The status code is wrong, see below |
| 3 | Stuck states, console, requests | No turn or investigation was left running. Every turn-start on both sessions has exactly one turn-end: A 7/7, B 2/2, no duplicates, no orphans |
Evidence:
- Completed and stopped chat turns:
- Reload mid-turn, resumed to one
turn-end:
- Finished investigation:
- Stopped investigation:
- Video, stop then a new message: https://github.com/MapleTechLabs/maple/blob/pr-assets/pr-2311/06-stop-then-new-message.mp4?raw=true
Problems found (none caused by this PR)
GET /api/chat/sessions/<id>/historyreturns 503 "The chat session is unavailable" on every investigation session that has events. It returns 200 on an empty session. PR fix(backend): guarded status transitions for error issues and investigations #2310's preview, which does not touchapps/ai, does the same, andhistory()and the route are unchanged in this diff. So it is pre-existing, but the chat panel'sloadHistoryhits it on every open. Repro: start any investigation, thenGET /api/chat/sessions/<org>%3Ainv-<id>/history. The trace shows no failing child span (for examplec5af5f9e78865a9a4fb8f60af6441db0), so it is probably the DO RPC returninghistory()'s messages.POST /v2/investigations/:id/restartwhile a run is in flight answers 503investigation_start_failed"Retry in a few seconds", with an Error span "This investigation already has a turn in flight" and a "Route answered with a server error" log. It also runsUPDATE investigationsbefore it finds the turn in flight. That should be a 409, with no write before the check (InvestigationService.restartInvestigation, trace316b558de5f148223b24776c93eb84ea).
Console: apart from the 503s above, only replay/ingest noise (session-replay POSTs aborted on navigation, an rrweb insertRule warning, Canvas2D warnings).
Traces (deployment.environment = pr-2311, via the maple CLI)
- 9
chat.turnspans, one per turn I ran (2 autonomous + 7 follow-ups), allOk. The aborted chat turn (299e78007404f9cfda807d38407fe4a7) is 12.2s and ends at the abort, with no Error spans. - Investigation A (
d324b06b…):chat.turn38.4s >invoke_agent investigate37.8s, 4×recordProgress, exactly oneInvestigationService.submitDiagnosis+ApplyDiagnosis.applyDiagnosisWrites. - Investigation B, stopped (
e5c2e39f…):invoke_agent investigate30.4s, 4×recordProgress, exactly oneInvestigationService.failInvestigation(35ms), nosubmitDiagnosis. One settle per turn. - Error spans in pr-2311: only the
/history503s and the restart 503 above.
Test data
- In the
Previeworg on pr-2311: investigations A (inv_21LjELPTT9tFCU3Pbf7NtEd, diagnosed) and B (inv_h7b6NAAEyo2vPi5rjKwPGs, failed), with ~9 chat turns tagged[pr-2311 …]. - On pr-2310's preview: one baseline investigation tagged
[baseline for pr-2311 history check]. - All of it is in preview Neon branches and goes away on teardown.
🤖 Generated with Claude Code
Makisuo
removed
the
preview:web
Seven implicit state machines in the agent runtime, each made explicit.
1. Relay timeout lost the final answer. The driver's final render ran as a sequential step, so the relay's
Effect.timeoutskipped it and the platform message stayed mid-stream; the relay then forgot the checkpoint, so nothing settled it later. Now the driver renders on interrupt too (editing only messages already posted, with a "still working" notice), and on timeout the relay hands the checkpoint to the scheduled settle (handOffTurn) instead of dropping it.RELAY_TIMEOUTis 27 min, past the session's 25 minTURN_STALE_MS, so they no longer race; the checkpoint TTL moves to 35 min to match.2/3. ChatSession turn slot. Reads were side-effecting (expiring a turn and appending
turn-endfromholdsTurnon every streamed event), legacy rows with a nullrunning_sincenever went stale, andturn-endwas written from five places in the DO and four in the runner, some unguarded. Nowslot()is a pure read (Idle | Held | Stale, null start time or id counts as Stale) and onetransition()(Begin, End, Abort, Expire, Resume) is a compare-and-set and the only writer of a turn'sturn-end. The runner ends turns withsession.endTurn(messageId, ending); the DO ends a runner that returned without one as failed. The alarm expires a stale claim instead of re-arming forever.4. turn-runner phases.
failure,heldandsubmittedwere overwritten per phase, so an earlier max-steps vanished from the final reason.autonomous-outcome.tsrecords each phase's outcome; the finalturn-endand the PR's failure reason are derived from all phases. An abort (not a supersede, which owns its row) or an interrupt now still runs the fail writes; settling runs once, uninterruptible, with bounded writes.5. ConnectorSocket. A heartbeat stored by an evicted activation fired before the new socket's
onOpen, and Discord's staleawaitingAckkilled the fresh connection.connectclears it, no heartbeat runs before the socket opens, and a dialled socket that does not open inOPEN_TIMEOUT_MSis given up and backed off. A socket that throws oncloseis still dropped.6. Executor prepare. A claim whose process was never spawned (DO evicted in between) read
runningforever. Like the checkout script, it islostafter a two minute grace and reclaimed;failedis now reset as it is reported, matching checkout.7. Progress feed.
resumeafterclosereopened a feed with no writer fiber. The feed is now Open | Paused | Closed and Closed is terminal.Tests
apps/ai:ChatSession.test.ts,autonomous-outcome.test.ts,progress.test.ts,ChatSessionObject.test.ts,budgets.test.ts: 86 passed. New: stale read expires nothing, legacy null-start row is stale and admits a turn, alarm stops on a legacy claim, one turn-end under end+abort either order, earlier max-steps kept, resume after close is a no-op.apps/chat-bot:src/relay+src/socket: 103 passed. New: relay timeout posts the final render and hands off the checkpoint, handed-off checkpoint settles on its next job, stale heartbeat not fired after eviction, pre-open timeout reconnects, refused close still redials.packages/chat-platformdriver.test.ts: 16 passed (new: interrupt edits only posted messages, ending on the last).apps/sandboxexecutor scripts/session: 35 passed (new: unspawned claim lost after grace, run against bash; failed prepare reset).oxlint -c .oxlintrc.effect.jsonclean on touched files.verify:imagenot run: the executor change is a status read in the existing script shape, exercised against bash in the unit test.🤖 Generated with Claude Code
Open in [code]smith · Need help on this PR? Tag
@codesmith-botwith what you need.