refactor(agent-core-v2): single producer for turn and step ids - #3662
Conversation
- the human agent state machine now assigns turn ids and echoes the drained queue item in turn.started; the facade adopts machine ids instead of reserving its own - the turn machine produces the step ordinal and surfaces it via a first-class step.started fact; the llm requester layer stays step-agnostic - turn.prompt wire records carry an explicit turnId; turnKey and transcript foldFacts read it directly with a counting fallback for legacy records - queued prompts are addressed by promptId: cancel(turnId) is active-turn only, cancelQueued targets queue entries, Turn.id resolves once the turn starts - drop PromptPayload.disabledTools and its klient/node-sdk pass-through (no client consumer)
|
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: b7c771a1b6
ℹ️ 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".
| await agent.prompt({ | ||
| input: input.input, | ||
| disabledTools: input.disabledTools, | ||
| promptId: input.promptId, |
There was a problem hiding this comment.
When an embedded SDKRpcClientV2 caller supplies disabledTools (for example, ['Bash']), this forwarding path now drops the property, while the corresponding AgentPromptService.submit call to setSessionDisabledTools was also deleted. The SDK uses Klient's in-memory transport, whose previous contract accepted this field, so existing compiled JavaScript callers now silently leave the requested tools enabled rather than merely encountering the kap-server validation mentioned in the commit. Keep forwarding and enforcing the denylist, or treat its removal as a public breaking change with the required major changeset.
AGENTS.md reference: AGENTS.md:L64-L64
Useful? React with 👍 / 👎.
1 participant
Related Issue
Internal refactor (no linked issue).
Problem
Turn and step ids currently have multiple producers. The facade (
AgentLoopService) reserves turn ids at prompt submission (burning ids on queue cancel), while the human agent state machine keeps its own turn clock whose value is ignored except for one fragileresetHistoryalignment point. The facade then guesses which queue entry the machine drained via queue-id diffs (reconcileDrainedQueueEntry,bindMachineTurn,unboundDrained,machineTurnUnbound). Step ordinals are produced in two places (the loop gate and the engine'scurrentStep). On top of that, the transcript package'sfoldFactsre-derives turn ids a third time by countingturn.promptrecords, and the kap-server projection layers have to predict future turn ids for queued prompts.What changed
turn.startedcarries the machine-assigned id plus the drained queue item id, and the facade adopts it deterministically at gate time. The facade's id reservation, the four-piece binding heuristic, and theresetHistoryclock alignment are deleted.step.startedfact event that flows through the xstate actor tree to the engine. The llm requester layer stays step-agnostic.turn.promptwire records now carry an explicitturnId;turnKeyand the transcript package'sfoldFactsread it directly, keeping the legacy counting derivation only as a fallback for old records (ahasExplicitTurnId-gated filter tolerates burned-id gaps in new wires).cancel(turnId)is active-turn only, a newcancelQueued(queueId)removes machine queue entries,status().pendingPromptIdsreplacespendingTurnIds, andTurn.idresolves once the turn actually starts (RPCturn_idresults are read afterturn.ready).PromptPayload.disabledToolsand its klient/node-sdk pass-through: the field had no working client consumer (the RPC boundary rejected it unconditionally) — the RESTdisabled_toolsroute is untouched.Checklist
/approve).gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.