fix(sessions): paused-turn transcript hygiene + pause-sentinel replay nudges#5451
Open
ardaerzin wants to merge 5 commits into
Open
fix(sessions): paused-turn transcript hygiene + pause-sentinel replay nudges#5451ardaerzin wants to merge 5 commits into
ardaerzin wants to merge 5 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughPaused 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 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 |
dosubot
Bot
added
size:M
Contributor
Railway Preview Environment
|
ardaerzin
changed the title
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
ardaerzin
force-pushed
the
fix/transcript-hygiene
branch
from
July 24, 2026 01:41
7787459 to
7fd5ac7
Compare
dosubot
Bot
added
size:L
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.
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.
…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.
… replay A cold reload rendered a paused-then-approved turn as two bubbles: a dangling "Write — awaiting approval" (the paused turn) plus a second "Write ✓" (the resume), because the runner re-emits the approved tool_call in the resume turn and every 'done' record closed the message. Verified against the durable records: both calls share one toolCallId; the resume turn carries no user row (the dedup fix), so it folds cleanly. transcriptToMessages now keeps the draft open on stopReason:"paused" so the resume folds into the same message, clears the paused flag once it completes, and dedupes a re-emitted tool_call by toolCallId so the call settles to a single ✓ — matching the live view.
ardaerzin
force-pushed
the
fix/transcript-hygiene
branch
from
July 24, 2026 14:30
7ff8d50 to
83df825
Compare
…tale trace comment Trace hover works on cold reload now that the runner stamps traceId on the done event, but two loose ends around it: extractTraceId's docstring still claimed records carry no trace link (a no-op until BE stamps), which is no longer true; and the paused-turn fold took the FIRST done's traceId, so a merged HITL turn's 'View full trace' pointed at the paused trace instead of the resume trace where the approved tool actually ran. Switch the done handler to last-wins so the merged turn takes the resume trace; a normal single-done turn is unaffected.
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.
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