[kyle-sexton]

Closes #6903

Summary

The docs-naming configuration can now express ADR 0059's scope, so /docs-naming:audit-file-names inventories the same set scripts/check-docs-naming.sh enforces, and scripts/check-docs-naming.test.sh compares the whole gate with the emitted one instead of only its docs/ half.

Fix

• Config schema (backward compatible): a roots entry may be an object, {"path", "extensions", "exempt_paths"}. extensions limits the root to those extensions, matched in any case (letters and digits only). exempt_paths takes paths out of the basename rule only when no other root claims them without exempting them, and the case-collision check still covers them. A plain string root behaves as before. Documented in plugins/docs-naming/reference/config.md ("Root objects").
• plugins/docs-naming/scripts/root-scopes.jq turns roots into git pathspecs. inventory.sh and emit-gate.sh both read it, so the audit and the gate share one definition.
• The emitted gate template walks the scope once per root to mark which paths get judged. This works in Bash 3.2, which has no associative arrays. It also folds paths for the collision check with one tr instead of one per path: with a repository-wide root, --check on this repo took 9s per-path and takes 0.2s now.
• setup check validates the root-object shape. emit-gate.sh refuses an extension outside [A-Za-z0-9] and refuses a newline in a path or exempt entry.
• .claude/docs-naming.json declares the ADR 0059 scope: docs, plus . limited to md, exempting **/fixtures/**, **/evals/** and **/vendor/**.
• check-docs-naming.test.sh seeds the whole scope: markdown outside docs/, the excluded trees, collisions, and role names outside docs/. It also seeds every name in the script's EXEMPT_NAMES as well as the config's. New case 11d: a name exempt in the script but missing from the config fails the drift check. New config mutants (root object removed, an exempt path dropped, extension changed) must also be caught.
• ADR 0059 gets a dated amendment recording the new schema and noting that its two "docs/ only" statements no longer hold.
• docs-naming changelog fragment (minor).

Verification

• All docs-naming suites pass. resolve-config 56, inventory 37, emit-findings 41, sweep 41, emit-gate 84, apply-rename 89, setup-apply 25, setup-check 31 (run from the repo root; it depends on the working directory, and that was already true on main).
• scripts/check-docs-naming.test.sh passes 38/38, scripts/check-docs-naming.sh --check is clean, and the emitted gate from the repo config is clean on this tree.
• inventory.sh on this repo: 2817 scanned, 0 offenders, which matches the gate.
• shellcheck (repo rcfile) passes on the changed scripts and on the emitted pair. shfmt reports no drift beyond what main already has. markdownlint passes and check-changelog-fragments.sh --check passes.
• check-script-contract.test.sh fails 2 check-html-assets cases locally because npm ci has not been run. That is unrelated, and main shows the same result.

Related

• ADR 0059 (#6746)

🤖 Generated with Claude Code

[claude]

Claude finished @kyle-sexton's task in 6m 33s —— View job

---

Security review — PR #121

• Gather context (PR diff, changed files)
• Invoke /review:security-review skill
• Follow its instructions exactly
• Post findings

Reviewed the files changed in this PR (not the unrelated repository), focusing on the apply action and scripts/apply_plan.py, which is the only new write-capable surface. The design already does a lot right: apply_plan.py never shells out itself (branch/PR/issue creation stays with the orchestrating agent under the user's own Codex approvals), rows require sentence + record-on-final-state, item ids are regex- and range-validated, surface is checked against path traversal and absolute paths, the ledger symlink is refused, and |/backtick/control-character injection into the line-oriented stdout and the markdown table is explicitly tested (including a dedicated "newline in a surface" forged-output test).

1 finding (adversarially verified by a separate verifier subagent before reporting — confirmed):

IMPORTANT — UNSAFE_RE in apply_plan.py only strips ASCII control characters ([\x00-\x1f\x7f]), not Unicode bidirectional-control or zero-width characters. surface/record pass this check and then land verbatim in the committed ledger ## Decisions table (marker()'s ledger-row: line) and in GitHub issue bodies (Hand-off section) — both artifacts the whole apply safety model relies on a human reviewing/confirming before any push, PR, or issue is created. Since sentence (and by extension the classification that shapes surface/record) traces back to upstream Codex release-note prose, a crafted release note could, via diff's classification step, smuggle a bidi-override (U+202E) or zero-width character into one of these fields, making the reviewed PR/issue/ledger text visually diverge from the real bytes being acted on (Trojan-Source-style). Posted inline at apply_plan.py:69 with a suggested regex extension.

No CRITICAL issues found. GitHub Actions hardening (triggers, permissions, pinning) is zizmor's lane and out of scope here; none of the changed files touch workflow YAML anyway.

[claude]

Claude finished @kyle-sexton's task in 5m 56s —— View job

---

Claude's review of #121

• Read context and repo documentation preflight
• Review diff (git diff origin/main...HEAD)
• Review apply_plan.py for correctness/error handling
• Review SKILL.md / references/apply.md for alignment
• Review codex_releases.py changes
• Review test coverage
• Post findings (inline + summary)

No blocking findings.

I read the full diff (plugin.json, SKILL.md, references/apply.md, apply_plan.py, the codex_releases.py after.txt/saved_after addition, and both test files) line by line, including the follow-up hardening commit (20b43d1), and traced the trickier paths by hand:

• owner()'s dot-segment skip (apply_plan.py L138-148) correctly attributes a .codex-plugin/plugin.json surface to the plugin directory above it rather than to the manifest directory itself — matches the new "both manifest locations" test.
• The gap:/after.txt logic (apply_plan.py L203-213, codex_releases.py L277-287) is deliberately conservative: any explicit --range that starts after the marker is treated as a possible skip unless after.txt (written only for a default-range fetch) matches the current marker — even when the explicit range happens to be contiguous. That's intentional per the tests, not a false-positive bug.
• Field validation in check_row (apply_plan.py L88-135) blocks control characters everywhere and |/backtick specifically in surface/record, which is exactly what's needed since sentence is never placed in a table cell (the ledger-row: line only has 5 columns: items, surface, lens, state, record) while surface/record are.
• plan/marker row bucketing (out-of-scope → done → file → group) matches references/apply.md's described priority, and marker's "held-by" calculation correctly ignores scope (any non-final row anywhere in rows.json holds the marker, per spec).
• The symlink refusal, CRLF-preserving write_marker, and malformed-releases.json handling all match their new tests.

I did not independently execute npm test / the Python scripts — this sandbox requires approval for node/python3 invocations that wasn't grantable here, so the PR body's own test/validate output is author-claimed and unverified by me. Everything above is from static reading of the diff and cross-checking it against the new assertions in tests/codex-changelog.test.mjs.

Security-shaped aspects (path traversal, injection via row fields, symlink handling) are intentionally left to the dedicated security-review lane on this repo rather than duplicated here.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 20b43d1a8b

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-11T04:28:43.361183Z	20b43d1	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[github-actions]

claude-review has reviewed this pull request through 0349331; a later push is reviewed from there.

[github-actions]

claude-security-review has reviewed this pull request through 0349331; a later push is reviewed from there.

[claude]

Claude finished @kyle-sexton's task in 1m 59s —— View job

---

Security review — PR #121 (incremental, since 20b43d1)

• Gather context (incremental diff, changed files)
• Invoke /review:security-review skill
• Follow its instructions exactly
• Post findings

Scope: only the delta since 20b43d1 (.claude-lane/incremental.diff), i.e. agents/openai.yaml, scripts/apply_plan.py, and tests/codex-changelog.test.mjs. This commit directly fixes the two findings raised on the prior head:

• Unicode bidi/zero-width spoofing (this lane's own 20b43d1 finding): UNSAFE_RE (ASCII-only [\x00-\x1f\x7f]) is replaced by unsafe(), which rejects any character in Unicode categories Cc (control), Cf (format — covers bidi overrides U+202E and isolates U+2066-U+2069, and zero-width characters like U+200B/U+200D), Zl, Zp. Verified: Cf does classify the bidi-override and zero-width codepoints named in the original finding, so surface/sentence/record can no longer carry them into the ledger table or issue bodies. New tests cover a bidi override in record and a zero-width space in sentence (tests/codex-changelog.test.mjs).
• Ledger writes escaping the repository root (Codex bot's P1 finding): new check_ledger() resolves the ledger against --root and walks each path component checking for a literal .. segment or an intermediate symlink, in addition to the existing final-component check, before any write. I traced the relative_to/parts logic by hand for a root/../outside style path and for a symlinked intermediate directory — both are caught (.. is kept as a literal path component by PurePath rather than collapsed, so the component-wise loop catches it; a symlinked ancestor is caught by current.is_symlink() at the point it's traversed). New tests cover a ledger under a symlinked directory and a ledger outside --root (tests/codex-changelog.test.mjs).

Also moved: the ledger-row: emission now happens before the range-gap early return, so final (applied/declined/deferred/filed) rows still get recorded in the ledger even when the marker itself can't move — a correctness fix (Codex bot's P2 finding), not a security issue, but I checked it doesn't reintroduce the earlier problem (rows are still gated by FINAL_STATES/record validation same as before).

I checked for regressions the fix itself could introduce: no leftover references to the removed UNSAFE_RE symbol, the new category set still excludes plain whitespace (Zs) so normal text isn't rejected, and check_ledger is only invoked for the marker subcommand's write path — plan's read-only --root use (owner lookup) is unaffected and doesn't need the same hardening.

No security issues found in this incremental diff.

[claude]

Claude finished @kyle-sexton's task in 3m 2s —— View job

---

Claude's incremental code review of #121

• Read context and repo documentation preflight
• Read .claude-lane/incremental.diff
• Invoke /review:code-review and follow its instructions
• Review changed files: agents/openai.yaml, scripts/apply_plan.py, tests/codex-changelog.test.mjs
• Post summary (no changed line needed an inline anchor)

No blocking findings. This commit (0349331) addresses all three open review comments from the prior pass (20b43d1) and the fixes line up correctly with their callers and tests:

• Unicode bidi/zero-width spoofing (flagged by claude-security-review on 20b43d1): apply_plan.py replaces the ASCII-only UNSAFE_RE with unsafe(), checking unicodedata.category() against {"Cc", "Cf", "Zl", "Zp"}. This is a cleaner fix than the bot's suggested regex (whose literal bidi characters embedded in the pattern would have been unreadable/fragile) and correctly covers U+202E, zero-width joiners/spaces, and line/paragraph separators. New tests (tests/codex-changelog.test.mjs ~L162-163) exercise a bidi override in record and a zero-width space in sentence.
• Ledger path containment / symlinked-ancestor traversal (Codex bot, P1): the new check_ledger() resolves the ledger relative to --root, rejects any literal .. segment and any symlinked ancestor directory (not just a symlinked final path), and rejects a ledger outside root outright. I traced the lexical relative_to/.parts walk by hand for a foo/../../etc/passwd-style path and for a root-external path; both are caught before the loop even needs the per-segment symlink check. --root is now accepted for marker too (previously plan-only), which is what the new containment check needs, and the arg-conflict check was correctly narrowed to just --select. New tests cover a symlinked ledger file, a symlinked ancestor directory, and a ledger outside the root.
• Lost ledger-row: lines on a range gap (Codex bot, P2): the ledger-row: print loop in marker() now runs before the gap check's early return, so final rows are still reported to the orchestrator when the marker can't advance. The new assertion at test L177 (lines["ledger-row"].length === 1) confirms this.

I also checked the test helper change (tests/codex-changelog.test.mjs L148-154): apply() now auto-adds --root <dirname of --ledger> for marker calls that don't pass --root explicitly, which is needed now that check_ledger enforces containment against the (previously-unused-by-marker) root. Every existing marker call in the file passes --ledger, so the fallback (args[0] when --ledger is absent) is never actually exercised — not a bug given current call sites, just not independently load-bearing.

I was not able to execute node --test, npm test, or the Python script directly in this sandbox (command approval unavailable for node/python3 invocations), so this review is from static reading of the diff cross-checked against the updated test assertions, not independent execution. Security-shaped aspects were left to the dedicated security-review lane, consistent with this repo's lane split.