fix(agent-core-v2): carry unmaterialized steer requests over to the next turn - #3445
fix(agent-core-v2): carry unmaterialized steer requests over to the next turn#3445sailist wants to merge 1 commit into
Conversation
🦋 Changeset detectedLatest commit: 01e18ed The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01e18ed41a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const transferred = new Map<string, StepRequest>(); | ||
| for (const request of job.queue.drain()) { | ||
| if (request.state === 'pending' && !request.turnScoped) { | ||
| this.standaloneStepQueue.enqueue(request, 'tail'); |
There was a problem hiding this comment.
When another newTurn request is already in pendingTurns, this moves the unmaterialized steer only to standaloneStepQueue; pumpTurns() then starts the pre-existing turn, whose queue was populated earlier in createPendingTurn(), without moving these standalone requests into it. If that queued turn is the last one, the Ctrl+S message remains pending indefinitely instead of reaching the next turn, and may be injected into an unrelated later prompt. Transfer these requests into the next pending job when one exists, or merge standalone requests when starting every turn.
Useful? React with 👍 / 👎.
| for (const request of job.queue.drain()) { | ||
| if (request.state === 'pending' && !request.turnScoped) { | ||
| this.standaloneStepQueue.enqueue(request, 'tail'); | ||
| transferred.set(request.id, request); |
There was a problem hiding this comment.
If dispose() is called while an active turn has an unmaterialized non-turn-scoped request, it aborts the turn and drains standaloneStepQueue before the asynchronous turn cleanup reaches this code. This cleanup then transfers the request into the already-drained queue even though disposing prevents any future turn, leaving the request permanently pending and making the disposed loop continue to report pending work. On the disposal path, abort the request rather than transferring it.
Useful? React with 👍 / 👎.
1 participant
Related Issue
No linked issue — internal bug report (steer content silently lost when the turn is interrupted, with frame-by-frame video evidence).
Problem
When a prompt is steered (Ctrl+S) into the running turn,
steer()returns success once the request is assigned to the turn's job queue, but the content only enters the model context at the next step boundary. If the turn is cancelled (ESC) or fails in that window,releaseActiveTurncancels every queued step — including the not-yet-materialized steer request — and the content is silently lost, while the prompt record still settles as if delivered.The code already marks steer requests as
turnScoped: false, and the step loop'sabortTurnScoped()deliberately spares non-turn-scoped requests — butreleaseActiveTurnignores that distinction and cancels everything, and there is no hand-off path out of a dead turn's queue.What changed
releaseActiveTurnnow honorsturnScoped: before cancelling steps it drains the turn job queue and transfers pending non-turn-scoped requests to thestandaloneStepQueue. The next turn picks them up through the existingmoveStandaloneStepsTochannel and materializes them at its first step boundary with the usual logic.cancelStepgains anabortRequestflag so a transferred step can be settled (cancelled, result resolved) without aborting its request, which stayspendingfor the next turn.inject()path benefits from the same transfer for free.Checklist
/approve).gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.