fix(runner): answered approvals close before a carried sibling re-parks the turn - #5910
Merged
Merged
Conversation
…arried sibling re-parks In the resume block, the carried-sibling re-park fired synchronously after the answer batch — before the Promise.race — so a freshly-approved execution's real completion was never observed and the paused-settle replaced it with APPROVED_EXECUTION_RESULT_UNKNOWN (issue #5907). The re-park now waits for each answered/allowed execution's closure first, on the same per-call bound the paused-settle uses. Pi stays exempt on purpose: it prepares the whole batch before executing any call, so while a carried sibling gate is pending closure is impossible and the park-and-carry branch keeps owning those spans. The carried gate's re-park semantics are unchanged.
…ed re-park The carry test reproduced the exact two-gates-one-answered topology of issue #5907 but never asserted the answered gate's result — the failure mode had zero assertion coverage. It now delivers frames in the fixed order (answer, real completion, then the re-park), asserts the closure window is open when the answer lands, that the real completion reaches the tracer, and that no sentinel replaces it. A sibling test pins the window end to end: the completion is delivered only while the pause has not re-armed, so it goes red on the pre-fix ordering (the settle replaced the result with the UNKNOWN sentinel) and green after.
…; the harness answers every raised gate name-06 now mounts both rename ops with permission: allow, matching the product build-kit overlay — these calls never gate in the product, so the scenario measures naming behavior, not approval mechanics; the approval-race regression (issue #5907) is guarded at the runner unit layer instead, noted in the scenario's about block. qa_matrix_lib's Turn.approval was a single dict overwritten by each tool-approval-request frame, so a multi-gate turn only ever answered the LAST gate and left the rest pending until their TTL. Turn.approvals now collects every raised gate in raise order (a re-raise refreshes its id in place), approval_reply answers each, assistant_message marks each gated part, and Turn.approval stays as a back-compat property returning the last gate — every existing caller reads truthiness plus ids and none assigns, so single-gate flows are byte-identical. Unit tests pin the single-gate message, the answer-all behavior, and the re-raise refresh.
dosubot
Bot
added
the
size:M
|
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: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR adds ordered multi-gate approval tracking, answers all gated tool calls, refreshes repeated gates, and delays approval re-parking until approved tools close. It also updates approval tests and self-naming scenario permissions. ChangesApproval flow
Self-naming scenario permissions
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant ApprovalReply
participant runTurn
participant ApprovedTool
participant SiblingGate
ApprovalReply->>runTurn: resume approved tool call
runTurn->>ApprovedTool: wait for closure
ApprovedTool->>runTurn: deliver real completion
runTurn->>SiblingGate: re-arm carried approval pause
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 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 |
![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: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: f0eab286-c566-4bb9-a803-33ca448d502f
📒 Files selected for processing (5)
.agents/skills/agent-release-gate/resources/qa_matrix_lib.py.agents/skills/agent-release-gate/resources/test_qa_matrix_lib_approvals.pybenchmarks/agent-config-editing/scenarios/09-self-naming.jsonservices/runner/src/engines/sandbox_agent/run-turn.tsservices/runner/tests/unit/session-keepalive-approval.test.ts
Contributor
Railway Preview Environment
Updated at 2026-08-10T16:51:32.796Z |
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.
Context
When one turn raises two approval gates and the resume answers only one, the answered call executed but its result was replaced with the synthetic
APPROVED_EXECUTION_RESULT_UNKNOWN: the carried (unanswered) sibling triggered the pause synchronously, before the race that should observe the answered call's real completion. Full characterization with runner-log and DB evidence: #5907. Live evidence includes a production-path shape (an auto-allowed rename racing a client-tool park).Changes
waitForToolCallClosurewindow, on the same per-call bound the paused-settle path uses. Pi is exempt by design: its batch semantics make a pending sibling during closure impossible, so the park-and-carry branch keeps owning those spans. Carried-gate re-park semantics are unchanged.permission: allow, mirroring the shipped overlay (the product never gates these; the approval-race regression lives at the runner unit layer). The QA harness now collects and answers EVERY raised gate in a turn instead of only the last (back-compatible for single-gate flows; all consumers audited).Tests
Red/green proven by stashing the fix: before, 2 failed with
APPROVED_EXECUTION_RESULT_UNKNOWNwritten into the settle; after, 48/48 in the file, 2145/2145 across the runner suite, tsc clean. Scenario JSON validated, persona drift locks green, CI-pinned ruff clean.Known follow-up (not this PR)
A carried gate is re-parked server-side but every later frame for it is suppressed, so the client is never re-told it owes an answer and the gate expires at the 10-minute TTL. Needs its own issue: re-emit carried gates on resume or re-render from the durable interaction rows.