fix(sessions): paused-turn transcript hygiene + pause-sentinel replay nudges#5451
fix(sessions): paused-turn transcript hygiene + pause-sentinel replay nudges#5451ardaerzin wants to merge 3 commits into
Conversation
Two coupled defects made a parked-then-resumed approval turn rehydrate wrong on a cold reload: - The runner re-persisted the user prompt on every resume. An approval reply's tail is the tool_result envelope (no text), so resolvePromptText fell back to the ORIGINAL prompt and wrote a duplicate user row. Guard the write with tailIsFreshUserMessage so the prompt persists only on the turn that introduced it. - Nothing marked a turn that ended mid-approval. finish() now stamps stopReason:"paused" on the terminal `done` record (existing wire field, no contract change); transcriptToMessages carries it to message.metadata.paused. With the duplicate row gone, AgentConversation's server-transcript adoption can no longer lean on a bumped message count. Make it state-aware: adopt when the server is ahead by count OR when the local tail is stuck paused while the server turn is terminal (a resume that completed elsewhere). Adds paused-end-marker coverage to transcriptToMessages.test.ts.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughPaused stop reasons now flow through runner finalization and transcript replay. Approval sentinel outputs receive explicit transcript guidance, resume tails avoid duplicate user persistence, and the web client marks paused messages and reconciles them with completed server transcripts. ChangesPaused turn flow
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant SandboxAgent
participant SandboxAgentOtel
participant RunnerTranscript
participant WebReplay
participant AgentConversation
SandboxAgent->>SandboxAgentOtel: finish(stopReason)
SandboxAgentOtel->>WebReplay: emit done with stopReason paused
SandboxAgent->>RunnerTranscript: render approval tool results
RunnerTranscript-->>WebReplay: emit retry or do-not-retry guidance
WebReplay->>AgentConversation: set paused metadata
AgentConversation->>AgentConversation: reconcile server and local tails
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Railway Preview Environment
|
On cold replay `messageTranscript` fed both pause-terminalization sentinels to the model through the generic `[<tool> error: …]` branch. The "error" framing makes the model read a parked call as a refusal and abandon it (the parallel-approval bug), and for an approved-but-unobserved call it invites a retry of something that may already have run. Render each sentinel as an explicit, non-error nudge with opposite guidance: - DEFERRED_NOT_EXECUTED — skipped for another approval, not denied; re-issue the call. - APPROVED_EXECUTION_RESULT_UNKNOWN — may already have run; do not assume failure, do not retry a side-effecting call. The user-facing render already distinguishes both (ToolActivity); this restores the model-facing half dropped when the fix train folded the deferred check into the combined isPauseSyntheticResult.
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6b3871f6-d134-4462-a223-3be98c4b6f10
📒 Files selected for processing (8)
services/runner/src/engines/sandbox_agent/run-turn.tsservices/runner/src/engines/sandbox_agent/transcript.tsservices/runner/src/server.tsservices/runner/src/tracing/otel.tsservices/runner/tests/unit/transcript.test.tsweb/oss/src/components/AgentChatSlice/AgentConversation.tsxweb/oss/src/components/AgentChatSlice/assets/transcriptToMessages.test.tsweb/oss/src/components/AgentChatSlice/assets/transcriptToMessages.ts
…local tail The paused-tail adoption exception could fire when the server transcript was shorter than the local one: a lagging snapshot ending in a settled assistant message would discard newer locally persisted approval state. The intended case (a resume that completed elsewhere) always has the server at least as long, so require serverMsgs.length >= prev.length and an assistant tail for the exception.
|
@coderabbitai review |
✅ Action performedReview finished.
|
Stacks on
feat/sessions-storage-rework(PR #5436). Two of the approval fixes from the handoff, rebuilt against the merged code.1. Paused-turn transcript hygiene (handoff task 2)
A parked-then-resumed approval turn rehydrated wrong on a cold reload:
tool_resultenvelope (no text), soresolvePromptTextfell back to the original prompt and wrote a second user row. Guarded withtailIsFreshUserMessage.finish()now stampsstopReason:"paused"on the terminaldonerecord (existing wire field — no contract change);transcriptToMessagescarries it tomessage.metadata.paused.AgentConversation's adoption heuristic can no longer lean on a bumped message count — it's now state-aware (adopt when the server is ahead by count OR the local tail is stuck paused while the server turn is terminal).2. Pause-sentinel replay nudges (handoff task 3)
On cold replay
messageTranscriptfed both pause sentinels to the model through the generic[<tool> error: …]branch. "error" makes the model read a parked call as a refusal and abandon it, and for an approved-but-unobserved call it invites a retry of something that may already have run. Each now renders as an explicit, non-error nudge with opposite guidance:DEFERRED_NOT_EXECUTED→ skipped for another approval, not denied; re-issue the call.APPROVED_EXECUTION_RESULT_UNKNOWN→ may already have run; do not assume failure, do not retry a side-effecting call.The user-facing render already distinguished both (
ToolActivity); this restores the model-facing half dropped when the fix train folded the deferred check intoisPauseSyntheticResult.Verification
tsc --noEmitclean; 1246/1246 unit tests pass (incl. the wire-contract golden)pnpm --filter @agenta/oss exec tsc --noEmit— zero errors in the changed filestranscriptToMessages.test.ts8/8,transcript.test.ts22/22