Repository navigation
fix(server): Codex skills sent during a running turn run as their own turn - #34138
juliusmarminge wants to merge 9 commits into
Conversation
…never invoked Codex only loads skills on turn/start, so a `$skill` steered into a running turn reaches the model as plain text. The new test fails on main because the adapter sends turn/steer for `$zebra-probe` while skills/list reports it enabled. The `$PATH` control still steers and passes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…turn Codex loads skill instructions only on turn/start, so a `$skill` steered into a running turn reached the model as plain text and explicit-only skills never ran. The Codex adapter now refuses such a steer when the text names an enabled Codex skill, and the steer follow-up path queues the same message as the next turn. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…er error Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…steers stay steers A steer redispatched because it could not join its turn was already sent, so it no longer releases a queue Stop held while the adapter decided. Only user messages are checked for skill mentions; delegated results and agent messages keep steering as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e queue Send now on a queued skill message, or a skill steered while other messages wait, used to land behind everything already queued. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e queued The requeued message waits held behind the run Stop reached, whether or not other messages were queued and whether or not that run has already ended. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts # apps/server/src/orchestration-v2/ProviderTurnControlService.ts # apps/server/src/orchestration-v2/SteeringCompletion.integration.test.ts
…okup still steers A second steer that had to wait for a new turn jumped ahead of the first one, since every re-dispatched steer took queue position 1. It now goes behind the re-dispatched steers already waiting and still ahead of ordinary queued messages. The skills/list lookup a steer waits on is bounded at 5 seconds and falls back to a plain steer. AI-assisted; verified with the tests and tools listed above. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — Explicit Codex skill mentions sent during a running turn now wait for their own turn, where skills can load. The change preserves queue order and Stop holds, with a bounded lookup that falls back to ordinary steering. You can add or adjust custom eligibility rules. Learn more. |
Thread transfer impact✅ Thread transfer remains within every enforced ceiling.
Baseline: Scenario and decoded snapshot size10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed. |
|
No actionable comments were generated in the recent review. Walkthrough
Merge Risk:
|
Codex loads skills only on turn/start, so a $skill mention sent as turn/steer reaches the model as plain text. The Codex adapter checks a user's steer for enabled skill mentions (skills/list for the turn's cwd). If one matches, it refuses the steer with requiresNewTurn. ProviderTurnControlService passes the flag on, and the effect worker's existing missed-steer fallback re-dispatches the message as its own next turn. I kept the author's approach and merged origin/main with a merge commit. Conflicts were in ProviderTurnControlService (kept both requiresNewTurn and main's unresponsive) and in both test harnesses (the new parameters go after main's new positional ones). After the merge the adapter tests also needed NodeCrypto.layer. I fixed both open findings. (1) Reversed order: every re-dispatched steer took queuePosition 1 and stopRunForQueuedCommand pushed the earlier one behind it. A re-dispatched steer keeps the user_message turn item it got when steered, and ordinary queued messages only get one when they start. The orchestrator counts queued runs that have such an item and places the new steer after them, still ahead of ordinary queued messages. stopRunForQueuedCommand takes an optional queuePosition that defaults to 1, so the /goal and promote callers are unchanged. (2) The skills/list lookup is bounded with Effect.timeout('5 seconds') and falls back to a plain steer through the existing orElseSucceed. I extended the existing 'queues a steer the provider can only run as its own turn' test to queue a message and then send two skill steers, expecting [steer1, steer2, queued]. It failed with the steers swapped before the fix. I merged the two 'still steers' adapter tests into one table and added a stalled skills/list row, which hangs without the timeout. The push to fix/issue-9317 was refused because another run had already pushed its own merge and the same two fixes there (65de896). I did not overwrite it, and pushed to the fallback branch.
Fixes #9317
Supersedes https://git.cafe/pingdotgg/t3code/pulls/26357. The change is @juliusmarminge's; their commits are kept.
Reproduction on main
Not executed during screening. Install a skill with a recognizable response instruction and set policy.allow_implicit_invocation: false in its agents/openai.yaml. Open a Codex thread in T3 Code and ask it to run sleep 45 before replying DONE. While it is running, select that skill from the composer skill menu and submit it using Steer. On main, the mention enters the existing turn as plain text without skill injection; with this change, it waits and starts a separate turn that invokes the skill. Repeat from an idle thread as a control.
Evidence
Real web app and Codex provider; fresh isolated data on each commit; Chromium, 1280×800, light theme. Install an explicit-only skill whose instruction is to reply
EXPLICIT_SKILL_CONFIRMED. In a fresh thread, requestsleep 45followed byDONE; while running, select the skill from the composer menu and submit with Steer. Repeat in another fresh thread and press Stop. As an idle control, invoke the same skill afterREADY.Synthetic skill seed script · Reproduction instructions
Private provider and environment labels were hidden or redacted. AI-assisted; verified with the tests and tools listed above.
Not shown:
Tests
vp test run src/orchestration-v2/SteeringCompletion.integration.test.ts: 49 passed (the extended ordering case failed before the Orchestrator fix with the two steers swapped). vp test run src/orchestration-v2/Adapters/CodexAdapterV2.test.ts -t 'steering a skill mention': 4 passed (the stalled skills/list row timed out without the timeout). vp run check:changed: format, lint, knip, typecheck (27 packages) and tests (8 files, 236 tests) all passed. Not run: a real Codex session.
Not in this change
AI-assisted; verified with the tests and tools listed above.