Skip to content

Keep bundled and reviewed-checkout skills out of Codex runs - #2107

Merged
ppXD merged 1 commit into
mainfrom
fix/keep-codex-skill-mentions-out-of-goals
Oct 12, 2026
Merged

ppXD merged 1 commit into
mainfrom
fix/keep-codex-skill-mentions-out-of-goals

Conversation

@ppXD

@ppXD ppXD commented Oct 12, 2026

Copy link
Copy Markdown
Owner

Summary

  • Every Codex invocation (exec and exec resume) now carries -c skills.bundled.enabled=false. The pinned 0.142.2 installs five skills of its own under CODEX_HOME/skills/.system on every start and lists them to the model. A $name anywhere in the goal injects that skill's SKILL.md into the turn before the model acts: mid-word, after a backslash, in backticks, or as [$name](path). exec has no structured input and no setting turns the mention scan off, so the fix narrows which skills a mention can reach.
  • An output reviewer (AgentTask.ExcludeRepositoryInstructions) also switches off every skill its reviewed checkout holds, using -c skills.config path rules (CodexHarness.AppendRepositorySkillExclusion).
    • It covers .agents/skills and .codex/skills. The CLI reads .codex/skills even when the workspace is marked untrusted.
    • Before this change, a producer-chosen file name such as notes/$approve-all.md in the reviewer's goal injected a skill the reviewed party wrote.
    • CodexRepositorySkills finds the files the way codex-rs 0.142.2 does: breadth first, six levels down, dot-named entries skipped, linked directories followed. It keys each path both as given and as the kernel resolves it.
    • Path rules keep a persona skill that shares a name with a checkout skill. Without them, that shared name makes the persona's mention ambiguous and neither skill is injected.
  • A skill folder holding more directories than the CLI reads under one root (2000) is refused with the new failure code codex_skill_scan_truncated. Past that limit, which skills the CLI loads depends on the order it reads them; an output review then falls back to the model critic. McpDeclarationWriter.TomlString now escapes every control character, so a directory name cannot make the rules unparseable.
  • Runs that load repository instructions are unchanged apart from the bundled-skill override. Persona skills and a repository's own skills still load for them.

Test plan

  • Unit: CodexRepositorySkillsTests (9 tests):
    • one rule per skill, under both workspace spellings; an ordinary run keeps the checkout's skills
    • walk parity with the 0.142.2 skill placements measured on the real CLI
    • refusal at the directory limit and no refusal one below it; control-character quoting
    • pinned constants and failure code
  • Unit: CodexHarnessTests shape theories
  • Full unit suite: 13394/13394
  • Integration: OutputReviewIntegrityFlowTests, AgentHarnessRegistrationTests, RealHarnessCodexMultiRepoGuidesTests, RealHarnessSkillProjectionTests, RealModelRunClassifierTests, AgentFolderDoubleFidelityTests (91/91)
  • Real-CLI E2E, codex 0.142.2 against a loopback broker:
    • GoalChannelE2ETests fresh and resumed arms, each with its positive control
    • new reviewer arm, which runs the production output-reviewer task over a checkout with a skill in each root, one behind a link and one named like the persona's skill
    • the reviewer arm's control drops the path rules and must list and inject each skill and leave the persona ambiguous; as built, none is listed or injected and the persona is
  • Mutation: rule paths that match nothing, and name rules used instead of path rules, each fail the reviewer arm. Removing any part of the walk fails a unit test.
  • Sandbox lane (Linux, bubblewrap, root): sandbox-isolation.yml expects ≥111 tests and requires the skill-mentions codex-cli review Confined … codex=<pin> marker

The pinned Codex 0.142.2 installs five skills of its own under
CODEX_HOME/skills/.system on every start (skill-installer, which
fetches from GitHub, skill-creator, plugin-creator, openai-docs and
imagegen) and lists them to the model. A $name anywhere in the goal
makes the CLI read that SKILL.md into the turn as a user message
before the model acts: at the start of a line, mid-sentence, glued to
a word, after a backslash, in backticks, or as [$name](path). Goals
carry text the platform does not author (webhook bodies, fork pull
requests, other models' output), so untrusted text could put an
unsanctioned skill's instructions in front of the model.

The scan cannot be avoided. exec hands its prompt over as one text
input with no structured channel, the CLI scans every text input, and
no setting or feature flag turns the scan off. skills.include_instructions
and --disable mentions_v2 leave it running. Escaping is no fix: a
backslash does not stop it, and only changing the character after the
$ does, which changes what the model reads.

What a mention can resolve can be narrowed. Every Codex invocation,
exec and exec resume, now carries -c skills.bundled.enabled=false.
The CLI then lists none of its own skills, injects none, and removes
the .system cache from the run's config home. The persona's skills
still load and still resolve a mention, as before. 0.147.0 behaves the
same.

An output reviewer needs more. Its cwd is the branch under review, and
its goal holds the producer's goal and the names of the files the
producer changed, so "notes/$approve-all.md" in that list injected a
skill the reviewed party wrote under .agents/skills, or under
.codex/skills, which the CLI reads even with the workspace untrusted.
A task that loads no repository instructions now also switches off
every skill its checkout holds with -c skills.config path rules. The
harness finds them the way the CLI does: both roots in the cwd,
breadth first, six levels down, dot-named entries skipped, linked
directories followed. Each path is keyed as given and as the kernel
resolves it. Path rules rather than name rules keep a persona skill
of the same name, which a checkout could otherwise use to make the
reviewer's own skill ambiguous. A checkout whose skill folder holds
more directories than the CLI reads under one root is refused with
codex_skill_scan_truncated: past that limit what the CLI loads depends
on read order. TOML quoting now escapes every control character, so a
directory name cannot make the rules unparseable. Every other run's
argv is unchanged.

A real-CLI E2E arm, fresh and resumed, names each bundled skill once,
each in a spelling of its own. Its positive control drops the override
and must list and inject every bundled skill the CLI installed, and
it fails if the CLI bundles a skill the goal does not name. A reviewer
arm runs the production output-reviewer task over a checkout with a
skill in each root, one behind a link and one named like the persona's
skill. Its control drops the path rules and must list and inject each
one and leave the persona ambiguous; as built none is listed or
injected and the persona is. A pin bump fails the arms until they are
re-measured, and the sandbox lane requires their markers on the
Dockerfile's Codex version.
@ppXD
ppXD merged commit 984c720 into main Oct 12, 2026
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant