Skip to content

fix(server): Codex V2 runs skills typed with any currency sigil - #13461

Merged
juliusmarminge merged 2 commits into
t3code/codex-turn-mappingfrom
v2/codex-skill-alias
Sep 24, 2026
Merged

juliusmarminge merged 2 commits into
t3code/codex-turn-mappingfrom
v2/codex-skill-alias

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Sep 24, 2026 •

Copy link
Copy Markdown
Member

The web and mobile composers let users trigger a skill with any currency symbol, not just $ (#12098), so €review shows up as a skill chip. V1 Codex rewrote the sigil to $ before sending the turn. The V2 Codex adapter sent the text unchanged. Codex only parses $name as a skill mention (TOOL_MENTION_SIGIL in codex-rs/skills/src/mentions.rs at rust-v0.156.1), so on V2 €review reached Codex as plain text and the skill was never invoked.

What changed

  • packages/shared/src/composerInlineTokens.ts now exports SKILL_MENTION_PATTERN, the prompt-side skill mention regex. The composer's chip regex is built from the same source. The two differ only at the end: the chip regex needs whitespace after the name, so a half-typed $re at the cursor doesn't become a chip. The prompt pattern also matches at end of text, because a sent prompt can end with a mention. This is the same (?=\s|$) variant that V1 Codex, Claude and Cursor already copy locally.
  • CodexAdapterV2.ts adds codexSkillMentionText, which does the same $1$$$2 rewrite as V1. toCodexInput applies it to the user's message text before attachment context is appended. That covers both turn/start and turn/steer. Currency amounts (€20, €5k, €100M, €1e6) and glued tokens (5€review) don't match and stay prose.

The Claude and Cursor drivers and the V1 runtime still have their own local copies of the pattern. I left them alone to keep this PR focused, and because V1 is being removed separately.

Verification

  • vp test run src/orchestration-v2/Adapters/CodexAdapterV2.test.ts src/orchestration-v2/testkit/CodexReplayFixtures.integration.test.ts (apps/server): 118 passed. The new table test covers €review do it → $review do it, £ship, a mid-sentence ¥review, a mention after a newline, the astral 𑿝, digit-leading €2spec, unchanged $review, and prose left alone: costs €20, €5k, €100M/€1e6, 5€review.
  • A scripted-replay test runs startTurn with €review do it and steerTurn with then £ship it, and expects the outbound turn/start and turn/steer frames to carry $review do it and then $ship it. Revert check: with toCodexInput reverted to text: turnInput.message.text, this test fails with CodexAppServerReplayFrameMismatchError on turn/start (received €review do it), and it passes with the fix.
  • None of the Codex replay fixtures has a turn/start or turn/steer input with a currency-prefixed token, so no transcript changes. The fixture suite passes unchanged.
  • vp test run src/composerInlineTokens.test.ts src/composerTrigger.test.ts (packages/shared): 45 passed. vp test run src/composer-editor-mentions.test.ts (apps/web): 34 passed. These cover the chip regex that is now derived from the shared source.
  • tsc --noEmit for apps/server and packages/shared: no errors. vp lint on the touched files: no new warnings.
  • Live check with codex exec --ephemeral 0.156.1 in a temp repo with a .agents/skills/t3probe skill. For $t3probe, Codex injected the skill and the model answered with the skill's marker directly, with no tool calls. For €t3probe, the skill was not injected. The model guessed the intent from the skill list and ran sed to read the SKILL.md itself (it also picked up an unrelated user skill). So € only works when the model happens to guess, which confirms $ is the only sigil Codex parses.
  • Not run: a full orchestrator run of the V2 Codex adapter against a live provider.

Model: Claude Opus 5.5 (Claude Code)

🤖 Generated with Claude Code


Devin Review

The composers accept any currency symbol as the skill sigil (`€review`),
and V1 Codex rewrote it to `$` before sending. The V2 Codex adapter sent
the text unchanged, so Codex, which only parses `$name`, never invoked
the skill.

Export the prompt-side skill mention pattern from composerInlineTokens,
derive the composer chip regex from the same source, and canonicalize
the sigil in the V2 Codex input builder for both turn/start and
turn/steer.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Sep 24, 2026
@github-actions

github-actions Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ No successful main baseline artifact is available yet. This run establishes the initial measurement.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire — 4.9 KiB — 6.8 KiB ✅
Codex Thread snapshot wire — 3.7 KiB — 4.9 KiB ✅
Codex Live turn WebSocket wire — 1.1 KiB — 2.0 KiB ✅
Codex Live turn WebSocket decoded — 20.4 KiB — 29.3 KiB ✅
Codex Live turn messages — 1 — 8 ✅
Claude Total thread wire — 4.9 KiB — 6.8 KiB ✅
Claude Thread snapshot wire — 3.7 KiB — 4.9 KiB ✅
Claude Live turn WebSocket wire — 1.2 KiB — 2.0 KiB ✅
Claude Live turn WebSocket decoded — 20.8 KiB — 29.3 KiB ✅
Claude Live turn messages — 2 — 8 ✅

Baseline: unavailable · PR result: 008162e · Source CI: success

Scenario and decoded snapshot size

10 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.

  • Codex decoded thread snapshot: 106.1 KiB
  • Claude decoded thread snapshot: 106.4 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Sep 24, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 008162e

Macroscope's review found this PR approvable — This is a focused Codex V2 bug fix that restores execution of explicitly typed skill mentions while preserving ordinary currency amounts and existing dollar syntax. The production change is localized and covered by direct and replay tests for both turn start and steering.

Notes:

  • No code objects were reviewed. Approvability was decided on eligibility alone.

You can add or adjust custom eligibility rules. Learn more.

…tTurn and steerTurn

The table test only called codexSkillMentionText, so dropping the call
from toCodexInput left every test green. Replay a scripted turn that
sends `€review do it` and steers with `then £ship it`, and expect the
outbound turn/start and turn/steer frames to carry `$` mentions.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@macroscopeapp
macroscopeapp Bot dismissed their stale review September 24, 2026 18:19

Dismissing prior approval to re-evaluate 008162e

@juliusmarminge
juliusmarminge merged commit 7a7339a into t3code/codex-turn-mapping Sep 24, 2026
24 of 25 checks passed
@juliusmarminge
juliusmarminge deleted the v2/codex-skill-alias branch September 24, 2026 19:30
juliusmarminge added a commit that referenced this pull request Sep 24, 2026
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
juliusmarminge added a commit that referenced this pull request Sep 24, 2026
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
juliusmarminge added a commit that referenced this pull request Sep 25, 2026
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant