fix(claude-ops): resolve built-in descriptions the inventory left unresolved

[kyle-sexton]

No related issue: operator-directed fix from the 2.1.284/2.1.285 native-surface review (ClaudeDesign had an empty description, so detect never paired it).

Summary

inventory.py left 14 built-in descriptions unresolved on Claude Code 2.1.285 (13 tools plus the design bundled skill). It now resolves 13 of them. design stays unresolved because its text reads a table keyed by a runtime mode. Detect also scores native names that have no description, and native drift now files each newly unresolved description.

Fix

• Inventory (inventory.py). A call with arguments (kbr(RTe()), gLr(void 0), P({...})) is now followed into its function, with that function's parameters shadowed. Template substitutions resolve, and so do ||/?? fallbacks (an empty "" fallback is skipped), parenthesized parts such as d+(x()?m:c)+p, and [...].join(sep) arrays. A template made only of runtime parts stays unresolved.
• Module-scoped identifiers. The 2.1.285 bundle concatenates about 2,100 modules, and minified names repeat between them. An imported name resolves to the one top-level declaration in the one module that exports it. Any other name resolves inside its own module. A name that is neither imported nor declared in its module is unresolved. This fixes a wrong value that would otherwise appear: workflow-authoring's ${jd} reads Workflow, not another module's local host_exit. A single-letter function resolves only when it is the one top-level declaration in its module.
• Detect (discover.py). PascalCase native names are split into words (ClaudeDesign scores as "design", EnterWorktree as "enter worktree"). user_facing_name is scored. It is not added to the dismissal fingerprint, so audit-native-overlap/SKILL.md stays accurate.
• Drift intake (native_drift.py). summarize records integrity.undetermined.description_unresolved. diff adds an unresolved-description item (key native-drift:unresolved-description:<name>:inventory) for each name the previous summary did not list. These items go through the existing key-based filing path, with no label. context/native-drift.md documents the new kind and its title.
• claude-ops 0.75.1 -> 0.76.0, with a CHANGELOG entry. docs/native-surfaces/records.json and all skill bodies are unchanged.

Verification

• test_inventory.py: 144 tests OK (10 new synthetic-bundle cases, negative cases included). test_overlap.py: 182 OK. test_native_drift.py: 40 OK. overlap.test.sh and native_drift.test.sh exit 0. The pinned scripts/run-ruff.sh check/format, markdownlint-cli2 and typos all pass.
• inventory.py --self-check on the installed 2.1.285 exits 3 with only the version advisory, the same result as origin/main. All lanes are ok. The run takes about 12 s (11 s before).
• Old and new extractions were compared on 2.1.284 and on 2.1.285. Unresolved descriptions drop 18->2 on 2.1.284 and 14->1 on 2.1.285. Every other changed field was checked by hand against the bundle, for example Grep ALWAYS use Grep ... as a Bash command, workflow-authoring Workflow, and Edit user-facing name Update.
• overlap.py detect --inventory <2.1.285> --repo . goes from 23 to 50 candidates. The operator rules on the new candidates and on the dismissals that came back because their descriptions changed.

Related

• feat(claude-ops): file native-surface drift from changelog apply #5465, fix(claude-ops): keep native drift report-only without a store and validate its inputs #5525, docs(claude-ops): find native-drift items by key instead of a label #5574 (native-drift intake this extends)

🤖 Generated with Claude Code

[kyle-sexton]

@codex review

[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-01T01:51:48.804470Z	e4b2d08	Manual request

ℹ️ 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.

[claude]

Claude finished @kyle-sexton's task in 44s —— View job

---

Security review — PR #5619

• Gather context (PR diff, changed files)
• Run /review:security-review for PR fix(claude-ops): resolve built-in descriptions the inventory left unresolved #5619
• Post findings

No security issues found.

Reviewed all 10 changed files (origin/main...HEAD, head 337bf91). The functional changes are confined to plugins/claude-ops:

• inventory.py — static-analysis parsing logic over the locally installed Claude Code CLI bundle (template/fallback/parenthesized-expression resolution, module-scoped identifier resolution for the bundled-module layout). No new file I/O, subprocess, eval/exec, deserialization, or network calls were introduced; all new code operates on an in-memory string already read by existing code paths.
• discover.py — adds camelCase word-splitting and an extra scored text field to the TF-IDF bag used for overlap detection. Pure text scoring, no new I/O.
• native_drift.py — adds an unresolved-description drift item derived from integrity.undetermined.description_unresolved, filed through the existing key-based path. No new external input handling.
• extraction.md, native-drift.md, CHANGELOG.md, plugin.json — documentation and version-bump only.
• test_*.py — test coverage only.

None of the changed files are CLAUDE.md/AGENTS.md/rules/skill/agent-body files, so the instruction-surface-deletion check doesn't apply. No injection surfaces (command/SQL/path/template), no credential/token handling, and no authorization/access-control logic in this diff.

[claude]

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

---

Code review complete

• Load /review:code-review skill and instructions
• Gather PR diff and changed files
• Review inventory.py resolution logic (call-with-arguments follow-through, param shadowing, module-scoped identifier resolution, ||/?? fallbacks, .join() arrays)
• Review discover.py PascalCase splitting and user_facing_name scoring
• Review native_drift.py drift intake changes
• Review test coverage
• Post findings

Summary

Reviewed the diff against origin/main (10 files, +779/-72). The core of this PR is a substantial, well-documented extension to the minified-bundle static resolver in inventory.py (call-with-arguments follow-through with parameter shadowing, module-scoped identifier resolution for the ~2,100-module bundle, ||/?? fallback and .join() array handling), plus corresponding fixes in discover.py (PascalCase name splitting, user_facing_name scoring) and native_drift.py (filing unresolved descriptions as drift items). Verification claims (test counts, before/after unresolved-description counts, timing) are author-reported and not independently re-run in this review — I did not have execution access in this lane.

One finding posted inline, worth the author's attention but not necessarily blocking:

native_drift.py (L687-693) — the new unresolved-description filing treats a missing/unknown previous summary (prev is None, a true baseline run) as "every current name is new," unconditionally filing one item per currently-unresolved description. This is inconsistent with:

• This same function's new_candidates handling just above it, which explicitly guards "unknown prior state" to file nothing new (knows_candidates).
• The module's own documented baseline convention ("a missing previous summary is a baseline run: no surface diff, no new candidates...").

There is test coverage for "an old-shape previous summary (missing the field) files every name once" via a non-None self.prev, which suggests this might be an intentional onboarding choice — but there's no test for the true prev is None case with a non-empty unresolved_descriptions, and the asymmetry with new_candidates isn't called out anywhere. Worth either aligning the baseline behavior with new_candidates, or explicitly documenting/testing why it should differ. Full detail and a repro in the inline comment.

Everything else reviewed (module-scoped resolution rules, PascalCase splitting, SINGLE_TOKEN_FACTOR/described gating, shape validation for the new unresolved_descriptions field, template/fallback/array-join parsing) held up under inspection and appears consistent with its stated design and existing test patterns.

Branch: fix/inventory-resolve-tool-descriptions

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 337bf9182a

ℹ️ 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".

[claude]

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

---

Security review — PR #5619

• Gather context (PR diff, changed files)
• Run /review:security-review for PR fix(claude-ops): resolve built-in descriptions the inventory left unresolved #5619
• Post findings

No security issues found.

Reviewed all 10 changed files (diff scoped to plugins/claude-ops/, base 54f3fe6^ through head b32c708):

• inventory.py — static-analysis parsing logic over the locally installed Claude Code CLI bundle (call-with-arguments follow-through with parameter shadowing, module-scoped identifier resolution across the ~2,100-module bundle, ||/?? fallback and .join() array handling, plus this commit's fixes for a statically-resolvable fallback left operand and for forward-declared value bindings). No subprocess/eval/exec/network calls; file I/O is limited to reading the local bundle and config paths from argparse, not untrusted remote input. The new/changed regexes (_LINK_RE, _binding_pattern, etc.) are linear character-class patterns, not susceptible to catastrophic backtracking.
• discover.py — adds PascalCase word-splitting (CAMEL_RE) and an extra scored text field (user_facing_name) to the TF-IDF bag used for overlap detection. Pure text scoring, no new I/O or trust-boundary crossing.
• native_drift.py — adds an unresolved-description drift item derived from integrity.undetermined.description_unresolved, filed through the existing key-based path with no label. No new external input handling. (Note: a functional/correctness asymmetry in this logic — baseline runs with prev is None file every unresolved name instead of nothing, unlike the sibling new_candidates handling — was already flagged by the code-review pass above; it has no security implication so is not repeated here.)
• extraction.md, native-drift.md, CHANGELOG.md, plugin.json — documentation and version-bump only.
• test_*.py — test coverage only.

None of the changed files are CLAUDE.md/AGENTS.md/rules/skill/agent-body files carrying standing instructions, so the instruction-surface-deletion check doesn't apply. No injection surfaces (command/SQL/path/template), no credential/token handling, and no authorization/access-control logic in this diff.

Branch: fix/inventory-resolve-tool-descriptions

[claude]

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

---

Code review complete

• Gather context (PR diff, prior Codex/Claude review comments, commit history)
• Diff the latest commit (b32c708, "address Codex review of the description resolver") against the previously reviewed head (337bf91)
• Verify each prior Codex finding was actually fixed, with regression coverage
• Review the new module-boundary/forward-reference logic (_declaration, _binding_value, _in_function_body) for correctness
• Post findings

Summary

Reviewed origin/main...HEAD (10 files) via gh pr diff, focused on what changed since the last review round (337bf91 → b32c708), since the four Codex findings from that round were the open items.

All four prior Codex findings are fixed, each with a new regression test:

• Scope single-character bindings to their module — _binding_value now rejects a single-letter binding found before the current module's start (v < lo), closing the // @bun-boundary crossing. Covered by test_a_single_letter_binding_never_crosses_a_module_boundary.
• Reject local value assignments that occur after the read — _declaration now takes a later_ok predicate; _binding_value only allows a forward-found var binding when the reader is inside an actual function/arrow body (_in_function_body), while _function_body keeps unconditional forward lookup (correct, since JS function declarations are fully hoisted, unlike var initializers). Covered by test_a_later_binding_resolves_only_inside_a_function_body.
• Preserve a statically resolved fallback left operand — _operand's ||/?? handling now records the left side's value(s) and only reads the right side when the left isn't unconditionally truthy. Covered by test_a_resolved_left_operand_wins_over_its_fallback.
• Add a verification record for the new bundle claims — extraction.md now carries the four-part claim/basis/as-of/recheck-trigger table for the ~2,100-module count and the "design is the only unresolved description" claim, matching this repo's skill-bodies-state-current-rules.md convention.

One new finding on the fix itself, posted inline on plugins/claude-ops/skills/inventory/scripts/inventory.py line 1342: _in_function_body's heuristic ("enclosing { preceded by ) or =>") also matches a top-level if/for/while/switch/catch block, not just a function/arrow body. Such a block at module scope runs synchronously during module init — the same timing the fix was built to exclude — so a var reference inside a top-level conditional wrapping a registration call could still forward-resolve to a value that isn't actually assigned yet at that point in execution. I traced this statically (this lane has no code-execution access, so it's unverified by a run) with a minimal repro in the inline comment; it's the same bug class as the one just fixed, reopened for control-flow blocks instead of bare object literals.

Not re-raised: the earlier open question about native_drift.py's diff() filing every currently-unresolved name on a true prev is None baseline run (asymmetric with new_candidates's "unknown prior state ⇒ nothing new" handling). This wasn't touched in the latest commit, but native-drift.md's own table entry for unresolved-description ("every listed name when <prev> recorded no list") and the native_drift.py docstring both now state this as intended behavior rather than an oversight — so it reads as a deliberate onboarding choice, not a bug. The only gap left is a direct test for the true prev=None (vs. old-shape non-None prev) case, which is a minor coverage nit rather than a blocking issue.

Verification claims in the PR body (test counts, before/after unresolved-description counts, timing) are author-reported and not independently re-run here — this lane has no execution access.
· Branch

[kyle-sexton]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b32c7081bf

ℹ️ 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".

[kyle-sexton]

@codex review

[claude]

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

---

Security review — PR #5619

• Gather context (PR diff, changed files, prior reviews)
• Run /review:security-review for PR fix(claude-ops): resolve built-in descriptions the inventory left unresolved #5619
• Post findings

No security issues found.

Reviewed the full diff (origin/main...HEAD, head b445068) against the criteria in /review:security-review: injection surfaces, authorization/access-control logic, credential/token handling, and instruction-surface deletions.

• inventory.py — the bulk of the diff is static-analysis parsing logic over the locally installed Claude Code CLI bundle: call-with-arguments follow-through with parameter shadowing, module-scoped identifier resolution across the // @bun-delimited bundle, ||/?? fallback handling (this commit's own fix keeps a statically-resolved ?? empty string rather than folding it into || semantics), .join() array parsing, and deferred-flag threading that replaces the prior head's _in_function_body brace-shape heuristic with explicit call-graph propagation from getter/method/arrow read sites. All of this operates on an in-memory string already read by existing code paths — no new file I/O, subprocess, eval/exec, deserialization, or network calls. The new regexes (_LINK_RE, _binding_pattern, _function_pattern, CAMEL_RE) are linear character-class patterns over a trusted local input (the operator's own installed binary), not attacker-supplied data, so this isn't a meaningful ReDoS surface.
• discover.py — adds PascalCase word-splitting and an extra scored text field (user_facing_name) to the TF-IDF bag used for overlap detection. Pure text scoring, no new I/O or trust-boundary crossing.
• native_drift.py — adds an unresolved-description drift item derived from the extraction's integrity.undetermined.description_unresolved, filed through the existing key-based path with no label. No new external input handling or authorization logic.
• extraction.md, native-drift.md, CHANGELOG.md, plugin.json — documentation and version-bump only; none is an AGENTS.md/CLAUDE.md/rules/skill-or-agent-body guardrail, and nothing in the diff deletes, narrows, or softens a standing instruction (docs/conventions/instruction-exception-register/README.md was checked and doesn't apply here — these are parser-behavior reference docs, not operator-facing rules).
• test_*.py — test coverage only.

This confirms the two prior security-review passes on this PR (head 337bf91 and b32c708), which also found no issues; the intervening commit (b445068, "take later bindings only on deferred reads; keep ?? strings") only refines existing parsing logic in the same non-security surface.
· Branch

[claude]

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

---

Code review complete

• Gather context (PR diff, changed files, prior Codex/Claude review rounds)
• Invoke /review:code-review criteria for PR fix(claude-ops): resolve built-in descriptions the inventory left unresolved #5619
• Diff the latest commit (b445068, "take later bindings only on deferred reads; keep ?? strings") against the previously reviewed head (b32c708)
• Verify the two open Codex P2 findings from the prior round are actually fixed, with correct logic and regression coverage
• Post findings

Summary

Reviewed origin/main...HEAD (10 files) via gh pr diff, focused on what changed in the latest commit since the last review round (b32c708 → b445068), since two Codex findings from that round were the only open items:

Both are fixed correctly, each with a regression test:

• Distinguish nullish (??) from falsy (||) fallbacks — inventory.py#L1003-L1007 now narrows the "fall through to the right operand" check to src.startswith("||", i) and not all(values), so ?? only falls through on a truly missing value (None), not on a statically-resolved empty string. Traced description:""??"FALLBACK" by hand: values == [""], not values is False, and the ||-only guard is False, so the left "" is kept and recorded — matching JS's actual nullish-coalescing semantics. Covered by test_a_nullish_fallback_keeps_an_empty_string.
• Do not treat an eagerly-called function as deferred — rather than patching the previous brace-heuristic (_in_function_body, which this commit removes entirely), the fix threads an explicit deferred: bool parameter through _scan/_operand/_read_string/_resolve_chain/_binding_value. It's set True only where JS actually defers execution: inside a getter/method body (inventory.py#L1509-L1520), inside any arrow-function body (inventory.py#L1198, unconditional — correct, since code inside an arrow body never runs at the point the arrow literal is written), or when following a bare function-valued identifier read as a getter-like field (len(chain) == 1 in _resolve_chain). An explicit call like dd() at a non-deferred site keeps whatever deferred value it inherited, so description:dd() as a plain (non-method, non-getter) object-literal value — which _eval_field's "value" branch scans with deferred defaulting to False (inventory.py#L1522-L1523) — correctly stays non-deferred, so a var zz= binding written later in the module is no longer forward-resolved into it. This also incidentally closes the control-flow-block variant of the same bug I'd flagged on the prior commit (a top-level if/for/etc. wrapping an eager call): since the new design no longer inspects brace structure at the read site at all, that false-positive class can't recur. Covered by test_an_eager_call_does_not_read_a_later_binding; the getter/method/arrow/function-field deferred paths are separately covered by test_a_later_binding_resolves_only_inside_a_function_body.

Also in this commit: native_drift.py's baseline-run behavior (every currently-unresolved name filed as an unresolved-description item when prev is None, asymmetric with new_candidates's "unknown prior state ⇒ nothing new") is now explicitly documented as intentional (native-drift.md, the diff() docstring) and has a direct test for the true prev=None case — closing the gap I'd noted in an earlier round as "worth documenting or testing."

No new findings. I did not find additional correctness issues in this commit's diff. Static trace only — this lane has no code-execution access, so test_inventory.py/test_native_drift.py pass counts and the author's before/after unresolved-description figures are author-claimed and not independently re-run here.

Branch: fix/inventory-resolve-tool-descriptions

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b4450688b3

ℹ️ 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".

[kyle-sexton]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b3be945bbd

ℹ️ 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".

[claude]

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

---

Code review complete

• Gather context (PR diff, changed files, prior review rounds)
• Load /review:code-review skill and instructions
• Confirm no functional commits landed after b3be945 — dd2cfb0 only merges in an unrelated origin/main feature (4e49cab), so the three P2 findings Codex raised against b3be945bbd on 2026-09-30T18:57:34Z were still live at this head
• Independently trace all three against the current inventory.py
• Post findings

Summary

Scoped to the 10 changed files (origin/main...HEAD). Most of this PR (the module-boundary resolution, deferred-read tracking, ||/?? nullish-vs-falsy split, argument-to-parameter binding, discover.py PascalCase splitting, and native_drift.py drift intake) has already been through several review rounds on this thread and I don't have new findings there — those all held up under inspection.

The one gap: three P2 findings Codex posted against b3be945bbd (2026-09-30T18:57:34Z) are still unaddressed at HEAD (dd2cfb0) — the only commit since b3be945 is a merge that pulls in an unrelated feature, not a fix. I independently re-traced each one against the current source (static trace only, no execution access in this lane) and confirmed all three are still reachable bugs. Posted as inline comments, each with a fix link:

1. inventory.py:884 — the empty-fallback check (src[i:i+2] in ('""', "''", "\`")) is a 2-character prefix test on the fallback's source text, not a check of what the whole right operand evaluates to. description:x||""+"Fallback"parses asx || ("" + "Fallback") in JS (+binds tighter than||), but the prefix test sees ""and discards the entire right side, losing the real"Fallback"` value.
2. inventory.py:1029 — computed is set True for any parenthesized group or call on the left of ||/??, regardless of whether it fully resolved to non-empty strings. (flag?"A":"C")||"B" resolves the left side completely (["A","C"], both truthy), but computed=True still forces the fallback "B" to be read and recorded as a spurious, runtime-unreachable variant.
3. inventory.py:1164 — a template substitution's resolved variants are collapsed to value[-1], discarding every other value. `Use ${flag?"X":"Y"}` only ever yields "Use Y", silently dropping the "Use X" variant — inconsistent with the +-concatenation path a few lines up in this same commit, which correctly preserves every combination via itertools.product.

None of these are newly introduced by the merge commit — they're latent in logic added by b3be945 and were already flagged by Codex; they just never got a follow-up fix commit like the earlier rounds' findings did. Author-reported test/verification claims elsewhere in the PR body are unverified by this lane (no execution access).

Branch: fix/inventory-resolve-tool-descriptions

[claude]

Claude finished @kyle-sexton's task in 57s —— View job

---

Security review — PR #5619

• Gather context (PR diff, changed files)
• Run /review:security-review for PR fix(claude-ops): resolve built-in descriptions the inventory left unresolved #5619
• Post findings

No security issues found.

Reviewed all 10 changed files (origin/main...HEAD, head dd2cfb0) against the criteria in /review:security-review: injection surfaces, authorization/access-control logic, credential/token handling, and instruction-surface deletions.

• inventory.py — the bulk of the diff extends the static-analysis parser over the locally installed Claude Code CLI bundle: call-with-arguments follow-through with parameter shadowing and now argument binding (_bound_arguments), module-scoped identifier resolution across the // @bun-delimited bundle (_declaration, _chunk_imports, _export_index), ||/?? fallback handling, [...].join(sep) array parsing, and deferred-flag threading for forward binding lookups. All of this operates on an in-memory string already read by existing code paths (the operator's own installed binary) — no new file I/O, subprocess, eval/exec, deserialization, or network calls. The new regexes (_LINK_RE, _binding_pattern, _function_pattern, CAMEL_RE) are linear character-class patterns over trusted local input, not attacker-supplied data, so this isn't a meaningful ReDoS surface.
• discover.py — adds PascalCase word-splitting (CAMEL_RE) and an extra scored text field (user_facing_name) to the TF-IDF bag used for overlap detection. Pure text scoring, no new I/O or trust-boundary crossing.
• native_drift.py — adds an unresolved-description drift item derived from the extraction's integrity.undetermined.description_unresolved, filed through the existing key-based path with no label. No new external input handling or authorization logic. (The baseline-filing asymmetry with new_candidates that earlier review rounds flagged is a correctness/design question, not a security issue, and is now documented as intentional.)
• extraction.md, native-drift.md, CHANGELOG.md, plugin.json — documentation and version-bump only; neither is a CLAUDE.md/AGENTS.md/rules file or a SKILL.md/agent body carrying standing instructions (both are reference//context/ docs), so the instruction-surface-deletion check doesn't apply, and nothing here deletes, narrows, or softens a standing instruction.
• test_*.py — test coverage only.

This confirms the four prior security-review passes already posted on this PR across earlier commits, which also found no issues; this pass covers the full current head (dd2cfb0, the merge of origin/main into the branch) and finds nothing new.

Branch: fix/inventory-resolve-tool-descriptions

[kyle-sexton]

@codex review

[claude]

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

---

Code review complete

• Gather context (PR diff, extensive prior review history — ~50 rounds already on this thread)
• Load /review:code-review skill and follow its instructions exactly
• Diff the latest commit (606bfb3) against the previously reviewed head (1c62471)
• Verify the three open findings from the prior round are fixed
• Check for new issues introduced by this commit's fixes
• Post findings as inline PR review comments

Summary

The only commit since the last reviewed head (1c62471) is 606bfb3 ("skip nested function returns, mask quoted text, cite upstream for records"), touching inventory.py, its test file, and extraction.md. It was meant to close the three items open against 1c62471:

Two of three are fixed correctly:

• Add an upstream pointer for volatile verification records — extraction.md's four-part record for the module-count claim now cites the upstream Claude Code CHANGELOG.md and Bun's bytecode-bundler doc page; the design-is-the-only-unresolved-description claim now notes it's derived per run rather than restated, so no external pointer applies. Satisfies .claude/rules/skill-bodies-state-current-rules.md.
• Exclude nested function returns from followed calls (common case) — _NESTED_FUNCTION_RE (inventory.py:612) plus the guard at inventory.py:936 correctly stops _scan from treating a plain nested function inner(){...} declaration's body as a branch of the outer function, for the exact repro in the new test (test_nested_returns_and_quoted_writes_do_not_leak).

Both fixes introduce a new gap, each posted inline:

• inventory.py:615-630 — _mask_strings blanks a backtick template's entire span, including every ${...} substitution, to hide quoted text from _write_pattern. But a substitution is live code, not quoted text: a genuine parameter write sitting inside one (`${x="LOCAL"}`) gets hidden from the write-detector too, so _bound_arguments wrongly keeps the stale call-site argument bound instead of treating the parameter as reassigned — a wrong value, not the conservative "unresolved" this PR has favored throughout. Repro and fix suggestion in the inline comment.
• inventory.py:936 — _NESTED_FUNCTION_RE's parameter-list class ([^()]*) can't span nested parens, so a nested function whose parameter list contains any parens (e.g. a default value that calls something, function inner(a=g()){) defeats the match. When the regex fails to match, the guard silently does not exclude the nested function, reopening the exact "nested function return leaks into the caller" bug this commit was written to close — just for a slightly more complex parameter list. Repro and fix suggestion in the inline comment.

Both are static traces (this lane has no code-execution access, consistent with every prior round on this PR) but are mechanical regex/logic derivations I'm confident in.

Not re-raised: everything else fixed across this PR's ~50 review rounds (module-boundary resolution and visibility, deferred-read tracking, ||/?? handling, numeric/unary/primitive literal settlement, argument-to-parameter binding with reassignment/compound-write/destructuring exclusion, hoisted-function shadowing and lexical-scope declaration ranking, template/join/concatenation alternative preservation, object-alias following, discover.py PascalCase splitting, native_drift.py drift intake) is unaffected by this commit and remains fixed.

Branch

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 606bfb37b4

ℹ️ 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".

[claude]

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

---

Code review complete

• Gather context (PR diff, changed files, extensive prior review history — ~50+ rounds already on this thread)
• Load /review:code-review skill and follow its instructions exactly
• Diff the latest commit (f1d3a04, "function-scoped var, parenthesized primitives, template-aware masking") against the previously reviewed head (606bfb3)
• Verify the two open findings from the prior round are fixed
• Check for any new issues introduced
• Post findings (one, posted inline)

Summary

The only commit since the last reviewed head (606bfb3) is f1d3a04, touching only inventory.py (+95/-28) and its test file (+23). It closes both findings posted against 606bfb37b4 (2026-09-30T21:36:17Z):

Both traced and confirmed fixed for the cases their new tests cover:

• Preserve primitive state through parentheses — _unparen strips wrapping/nested parens before _literal_truthy/_literal_nullish classify a literal, and _operand's (-handling now keeps the _NONSTRING marker for a parenthesized known primitive (group is None and known) instead of discarding it. Traced (true)||"FALLBACK", ((1))||"FALLBACK", (false)??"FALLBACK" (all correctly settle to "", matching the new assertions) and (false)||"FALLBACK" (falls through to _scan's own || handling and correctly reads "FALLBACK").
• Treat preceding var declarations as function-scoped — new _function_block/_is_var helpers let _visible recognize a var as visible throughout its enclosing function (not just its immediate block), in both directions: an earlier var in a preceding sibling block is now visible to a later reader, and a later var anywhere in the reader's own function still shadows an earlier match. Traced both new test_var_is_function_scoped_across_nested_blocks cases by hand and they check out.

Also in this commit: _mask_strings no longer blanks a template substitution's ${...} code — it now recurses into it (masking only the quoted text inside), closing a real regression the earlier "mask quoted text" fix introduced (a genuine parameter write like `${x="LOCAL"}` was previously hidden, wrongly keeping a stale call-site binding). Traced the new hh test case and it resolves to "Use …" as expected (write now detected, parameter correctly left unbound).

One new finding, posted inline on inventory.py:1519: _is_var's new _STATEMENT_START_RE ([;{}(]\s*var\s(?:[^;{}()]*)\Z) requires zero ;{}() characters between the var keyword and the target identifier. A multi-declarator var statement where an earlier declarator's initializer contains a call — e.g. var a=f(1,2),xx="LOCAL" — breaks this, since f(1,2)'s parens fall inside the disallowed span. That makes _is_var wrongly return False for xx, which defeats this very commit's shadow-detection fix: a shadowing var grouped with a call-containing sibling declarator isn't recognized as hoisted, so _declaration keeps a stale outer binding instead of reporting unresolved — a wrong value, not just a missed resolution. Full repro and fix suggestion in the inline comment. This is a plausible real-world pattern (minifiers commonly group var declarators with call initializers to save bytes), not a contrived edge case, and isn't covered by the new test (which only exercises a single-declarator var in the shadowing position).

Not independently re-run: python3 -m unittest/pytest were denied by the sandbox in this pass, consistent with every prior round on this PR — this is a static trace only. The author's test-run and before/after figures in the PR body are author-reported and not independently re-run here.

Not re-raised: everything else fixed across this PR's ~50 review rounds (module-boundary resolution and visibility, deferred-read tracking, ||/?? nullish-vs-falsy handling, numeric/unary/primitive literal settlement, argument-to-parameter binding, template/join/concatenation alternative preservation, hoisted-function shadowing and lexical-scope declaration ranking, object-alias following, discover.py PascalCase splitting, native_drift.py drift intake) is unaffected by this commit and remains fixed. discover.py and native_drift.py were not touched in this commit.

Branch ·

[claude]

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

---

Security review — PR #5619

• Gather context (PR diff, changed files, prior review rounds — many prior clean passes on this thread)
• Invoke /review:security-review and follow it exactly for HEAD f1d3a04
• Post findings

No security issues found.

This PR has been through many security-review passes on prior heads (337bf918 through d6006fc834), all clean. This pass covers everything since the last reviewed head (d6006fc834baca29f07f7344ed59b9cba4dfeab1) through the current HEAD (f1d3a04ae6b86ac6bcc75dd70dea6a2d69bb4ec2).

Five commits landed in that window (4ce0620, 0065509, 1c62471, 606bfb3, f1d3a04), touching only three files (+187/-14):

• inventory.py — continues refining the static-analysis parser over the locally installed, trusted Claude Code CLI bundle (not attacker-supplied input):
  • _mask_strings — blanks quoted string/template contents (recursing into ${...} substitutions, which stay code) before a parameter-write search runs over a function body. Recursion depth is bounded by the template-nesting depth of the bundle text being scanned, not by attacker input; falls back to treating the remainder as masked on an unterminated-literal ValueError, so it can't raise or hang.
  • _NESTED_FUNCTION_RE — matched only against a fixed 400-byte window (inventory.py#L963) to exclude a nested function declaration's body from a followed call's return-value scan. Linear character classes, no nested/overlapping quantifiers — no ReDoS concern even before the bounded-window cap.
  • _CONTROL_HEAD_RE and _STATEMENT_START_RE, plus the shadow-declaration lookbehind regex (inventory.py#L1641-L1644) — each is matched against a small fixed-size window (16, 300, and 8 bytes respectively), so none can exhibit catastrophic backtracking regardless of input shape.
  • _function_block, _is_var, and the extended _visible — pure control-flow/scope bookkeeping to model JS var hoisting through nested blocks; no new I/O, subprocess, eval/exec, deserialization, or network calls anywhere in this diff.
  • The _resolve_chain change at the end (acc.unresolved += sub.unresolved) is bookkeeping propagation only, already reviewed in an earlier commit on this same line pattern.
• test_inventory.py — regression coverage only (+51 lines).
• extraction.md — documentation only: adds upstream citation links (the Claude Code changelog entry and Bun's bytecode-output docs) to two existing verification-record rows. This is a reference/ doc, not a CLAUDE.md/AGENTS.md/rules file or a SKILL.md/agent body carrying standing instructions, so the instruction-surface-deletion check doesn't apply, and nothing here deletes, narrows, or softens a standing instruction — it only adds citations.

No other files changed in this window: discover.py, native_drift.py, and the remaining version-bump/changelog files are unchanged since their prior clean security-review passes. No injection surfaces (command/SQL/path/template), no credential/token handling, and no authorization/access-control logic anywhere in this diff.

The PR's full changed-file set (10 files) remains confined to plugins/claude-ops/, consistent with every prior pass on this thread — the functional surface throughout is a static-analysis parser over a locally installed, trusted CLI bundle, with no subprocess, eval/exec, deserialization, network calls, or attacker-controlled input at any point in this branch's history.

Branch ·

[kyle-sexton]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f1d3a04ae6

ℹ️ 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".

[claude]

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

---

Code review complete

• Gather context (PR diff, extensive prior review history — ~55 rounds already on this thread)
• Load /review:code-review skill and follow its instructions exactly
• Diff the latest commit (ce90d96, "close four wrong-value corners in the bundle reader") against the previously reviewed head (f1d3a04)
• Verify each of the four claimed fixes with a hand-traced repro
• Confirm the merge commit (1cbce94) is clean and the PR's file set still matches origin/main...HEAD
• Post findings (one, inline)

Summary

Scoped to what changed since the last reviewed head (f1d3a04): the only functional commit is ce90d96 (inventory.py +97/-13, test_inventory.py +35), plus an unrelated upstream merge (55b880f, test temp-dir cleanup) and the merge commit itself (1cbce94, no conflict markers left, git diff origin/main...HEAD still matches the PR's stated 10-file/+1771/-86 shape).

ce90d96 closes four findings from the prior round, each traced against the actual repro and its new regression test:

• A var statement is found by walking back over balanced brackets — _statement_keyword replaces the old _STATEMENT_START_RE prefix regex with a backward scan over _mask_strings'd text that tracks bracket depth and stops at an unmatched opener or a top-level ;. Traced var a=f(1,2),xx="LOCAL": the walk correctly steps back over the balanced f(1,2) call and finds var before a, so xx is recognized as part of the same var statement. Covered by test_a_var_after_a_call_initializer_still_shadows.
• A one-letter binding is unresolved when its function re-declares the name after the read — _redeclared_later is now checked from _binding_value's single-character path. Traced var e="WRONG";function ff(){return e;var e="LOCAL"}: the later var e is found, matched to ff via _function_block identity, and ff()'s description correctly comes back unresolved rather than "WRONG". I also checked this doesn't over-trigger: a var/let declared inside a different, non-nested function (sibling or anonymous-callback) is excluded because the search is bounded to src[at:reader[1]] and matched by _function_block(...) == reader identity, so it doesn't shadow across unrelated scopes. Covered by test_a_later_local_declaration_shadows_a_one_letter_binding.
• A nested function whose parameter list holds parens is recognized — _opens_function replaces the old [^()]*-based _NESTED_FUNCTION_RE, matching the parameter list's parens by depth via the new _open_paren helper. Traced function inner(a=g()){return"WRONG"} nested in a followed function: the depth-matched walk correctly finds inner's own (, recognizes the function keyword before it, and _scan no longer treats inner's body as a branch of the caller. Covered by test_a_nested_function_with_a_call_default_is_not_a_branch. One new finding on this fix, posted inline (inventory.py:614-632): _open_paren counts raw (/) characters without masking strings first, so a parameter default containing a string with an embedded ) (e.g. sep=")") throws off the depth count and can make _opens_function wrongly return False for a real function body — reopening the exact leak this fix targets, just for a different input shape. Repro and fix suggestion in the inline comment.
• Integer literals render only up to 15 digits — _primitive_text's digit cap drops from 16 to 15 digits, so a literal above Number.MAX_SAFE_INTEGER can no longer be mis-stringified exactly. Traced `n${9007199254740993}`: the 16-digit literal no longer matches, so it renders as a runtime ellipsis instead of the wrong exact-digit string. Covered by test_an_unsafe_integer_in_a_template_stays_unresolved. Note: 15 digits is slightly more conservative than necessary (MAX_SAFE_INTEGER itself is 16 digits, 9007199254740991), so a few legitimately-safe 16-digit integers now render as unresolved rather than their exact value — but that's the fail-safe direction (unresolved, not wrong), consistent with this PR's established design philosophy, so not flagging it as a defect.

Not independently re-run: python3 -m unittest was denied by the sandbox in this pass, consistent with every prior round on this PR — this is a static trace only. The author's test-run and before/after figures in the PR body are author-reported and not independently verified here.

Not re-raised: everything else fixed across this PR's ~55 review rounds (module-boundary resolution and visibility, deferred-read tracking, ||/?? handling, numeric/unary/primitive literal settlement, argument-to-parameter binding, template/join/concatenation alternative preservation, hoisted-function shadowing and lexical-scope declaration ranking, object-alias following, discover.py PascalCase splitting, native_drift.py drift intake) is unaffected by this commit and remains fixed. discover.py, native_drift.py, and the documentation/version-bump files are unchanged since their last clean review rounds.

Branch: fix/inventory-resolve-tool-descriptions

[claude]

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

---

Security review — PR #5619

• Gather context (PR diff, changed files, extensive prior review history on this thread)
• Invoke /review:security-review and follow it exactly for HEAD 1cbce94
• Post findings

No security issues found.

This PR has been through many clean security-review passes on prior heads (337bf918 through f1d3a04ae6). This pass covers everything since the last reviewed head (f1d3a04ae6b86ac6bcc75dd70dea6a2d69bb4ec2) through the current HEAD (1cbce94b406cdea69511487755270472131a38d2).

Two commits landed in that window:

• ce90d96 ("close four wrong-value corners in the bundle reader") — the only functional change, touching exactly two files:
  • inventory.py — continues the same static-analysis-parser refinement pattern reviewed across every prior round: _open_paren/_opens_function replace the previous _NESTED_FUNCTION_RE lookback with a balanced-bracket walk (bounded to 4KiB) so a nested function's parameter list containing a call default (inner(a=g())) is still recognized and excluded from the caller's return collection; _statement_keyword/_declares/_redeclared_later extend the existing var/let/const visibility logic (itself masking strings via the already-reviewed _mask_strings) to walk back over balanced brackets so a multi-declarator var with a call initializer doesn't hide a later hoisted shadow; and the integer-literal regex in _primitive_text is narrowed from 16 to 15 digits, which only tightens an existing safe-rendering boundary (staying below 2**53, where a JS double still prints exactly) rather than introducing new parsing risk. All of this is pure, bounded (4096/200/16/8-byte-window) linear-regex and brace-matching analysis over the same trusted local input as every prior commit on this PR (the operator's own installed Claude Code CLI bundle, not attacker-supplied data). No new file I/O, subprocess, eval/exec, deserialization, or network calls — confirmed by grep over the diff. No new unbounded or nested-quantifier regex patterns, so no ReDoS regression.
  • test_inventory.py — regression coverage only.
• 1cbce94 (merge of origin/main) — its only non-ce90d96 parent, 55b880f ("remove remaining temp-dir leaks from test suites", fix(tests): remove remaining temp-dir leaks from test suites #5633), is an unrelated, already-merged test-infrastructure fix with no overlap with any claude-ops file — confirmed via git show --stat.

No other files changed in this window. discover.py, native_drift.py, and the documentation/version-bump files (extraction.md, native-drift.md, CHANGELOG.md, plugin.json) are unchanged since their prior clean security-review passes. No injection surfaces (command/SQL/path/template), no credential/token handling, and no authorization/access-control logic anywhere in this diff.

The PR's full changed-file set (10 files) remains confined to plugins/claude-ops/, consistent with every prior pass on this thread — the functional surface throughout is a static-analysis parser over a locally installed, trusted CLI bundle, with no subprocess, eval/exec, deserialization, network calls, or attacker-controlled input at any point in this branch's history. None of the changed files are CLAUDE.md/AGENTS.md/rules/skill-or-agent-body files carrying standing instructions, so the instruction-surface-deletion check doesn't apply.

Branch ·

[kyle-sexton]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1cbce94b40

ℹ️ 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".

[kyle-sexton]

@codex review

[claude]

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

---

Security review — PR #5619

• Gather context (PR diff, changed files, extensive prior review history — ~55+ rounds already on this thread, all clean)
• Invoke /review:security-review and follow it exactly for HEAD 121806d
• Post findings

No security issues found.

This PR has been through many clean security-review passes on prior heads (337bf918 through 1cbce94b40). This pass covers everything since the last reviewed head (1cbce94b406cdea69511487755270472131a38d2) through the current HEAD (121806d).

The only commit in that window is 121806d ("shadowing by bare declarations, catch params, quoted heads"), touching exactly two files:

• inventory.py — continues the same static-analysis-parser refinement pattern reviewed across every prior round, with four changes:
  • _opens_function now masks quoted text (via the already-reviewed _mask_strings) before the balanced-paren walk, so a parameter default containing a quoted paren (sep=")") no longer defeats the nested-function-detection fix from the previous commit. It now also bounds its scan to the enclosing brace or a 4096-byte window and raises ValueError on an unmatched head (propagating as "unresolved," the project's established fail-safe direction) rather than silently misclassifying.
  • _unset_between and the extended _binding_value add detection for a bare let x;/var a,x re-declaration or a catch(x) parameter shadowing a binding, correctly reporting "unresolved" instead of a stale value. Pure control-flow/regex bookkeeping over the same in-memory, trusted local input (the operator's own installed Claude Code CLI bundle) — no new file I/O, subprocess, eval/exec, deserialization, or network calls.
  • _write_pattern widens the destructuring-write detection from [^\[\]{};]* to a lazy [^;]*? bounded by the statement's ; terminator, so nested destructuring ({a:{b:x}}=) is caught. This is a single, non-nested lazy quantifier over a ;-bounded span — no alternation or nested-quantifier structure that could cause catastrophic backtracking, and the scanned text is the same trusted local source as every other regex in this file.
  • _declaration's home-module lookup edge case (home <= found.start() < at) is a narrow correctness refinement to where "re-declared/unset" detection starts scanning from; no new external input handling.
  • All of this still operates on the operator's own locally installed, trusted Claude Code CLI bundle — not attacker-supplied data — consistent with every prior commit on this PR.
• test_inventory.py — regression coverage only (4 new test cases).

No other files changed in this window. discover.py, native_drift.py, and the documentation/version-bump files (extraction.md, native-drift.md, CHANGELOG.md, plugin.json) are unchanged since their prior clean security-review passes. No injection surfaces (command/SQL/path/template), no credential/token handling, and no authorization/access-control logic anywhere in this diff.

The PR's full changed-file set (10 files) remains confined to plugins/claude-ops/, consistent with every prior pass on this thread — the functional surface throughout is a static-analysis parser over a locally installed, trusted CLI bundle, with no subprocess, eval/exec, deserialization, network calls, or attacker-controlled input at any point in this branch's history. None of the changed files are CLAUDE.md/AGENTS.md/rules/skill-or-agent-body files carrying standing instructions, so the instruction-surface-deletion check doesn't apply.

Branch ·

[claude]

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

---

Code review complete

• Gather context (PR diff, extensive prior review history — ~55+ rounds already on this thread)
• Load /review:code-review skill and follow its instructions exactly
• Diff HEAD 121806d against the previously reviewed head (1cbce94)
• Verify the open finding from the prior round (var-shadow across sibling blocks) and check for new issues
• Post findings (none survived — see summary)

Summary

The only commit since the last reviewed head (1cbce94) is 121806d ("shadowing by bare declarations, catch params, quoted heads"), touching inventory.py (+72/-21) and its test file (+38). It closes three remaining gaps:

• A declaration with no initializer, or a catch (x) parameter, shadows — the new _unset_between walks forward from the found binding to the reader, looking for a bare let x;/var a,x declarator (no = within the lookahead window) or a catch(x) parameter, and treats either as making the read's value non-static. Traced both new assertions in test_a_declaration_without_initializer_shadows (let xx;return xx and return xx;var xx) and test_a_catch_parameter_shadows by hand — both correctly resolve away from the stale "WRONG" outer binding.
• An imported binding is now checked for shadows only in the reader's own module — inventory.py#L1813-L1821 clamps _unset_between's search start to home (the reader's own module) whenever the resolved binding lives in a different module, so the new backward scan can't wander into unrelated foreign-module text.
• A nested function head with a quoted paren default is matched correctly — _opens_function now masks string/template contents before running _open_paren's depth-matching, so a default like sep=")" no longer desyncs the paren count, and raises ValueError (caught by resolve_field's existing broad except) when the head still can't be read, leaving that field unresolved rather than silently mis-scanning a nested function body as a branch of the caller. Traced test_a_nested_function_with_a_quoted_paren_default_is_not_a_branch — the masked scan correctly finds inner's own ( and excludes its body.
• Nested destructuring writes unbind a parameter — _write_pattern's destructuring alternative drops the old single-level [\[{][^\[\]{};]*ident...[\]}] shape (which couldn't match two closing brackets before =) for a lazy ident[^;]*?[\]}]\s*=, closing the gap for ({a:{b:x}}=...). Confirmed by hand-tracing test_a_nested_destructuring_write_unbinds_a_parameter.

Re-verified the previously open finding (my own comment on the 0065509/1c62471 rounds: "var shadow in a sibling block of the read's own wrapping block is still missed", since reader was computed as at's innermost brace rather than its enclosing function). That was actually already fixed two commits before the last reviewed head, in f1d3a04 (reader = _function_block(src, braces, at) instead of braces.enclosing(max(at-1,0))), which I'd missed re-confirming in the intervening rounds — current source at 121806d still has that fix (inventory.py:1706), and _redeclared_later's own _function_block-based comparison (used for the single-character identifier path too) independently covers the same case. No regression here.

Two non-blocking precision notes, both fail-safe (can only turn a real value into unresolved, never fabricate a wrong one), consistent with this PR's established "prefer unresolved over wrong" design — not raising as blocking:

• The catch(x) shadow check (_unset_between's _visible(braces, pos, at) without the src argument) treats the parameter as shadowing anywhere in the enclosing block, not just inside the catch body — so a read after the try/catch statement in the same function would also be (incorrectly) treated as shadowed. The new test only asserts assertNotEqual(..., "WRONG"), not a specific resolved value, which is consistent with this being a known, accepted looseness rather than an oversight.
• The broadened destructuring-write regex (ident[^;]*?[\]}]\s*=) no longer requires a bracket to actually wrap ident on the left, so an unrelated bracket-assignment later in the same statement (e.g. foo(x) + (obj[key] = val)) would also mark x as "written." Same safe direction as above (downgrades to unresolved, never produces a wrong string).

Not independently re-run: this lane has no code-execution access (python3/py_compile were denied by the sandbox) — this is a static trace only, consistent with every prior round on this PR. The author's test-run and before/after figures in the PR body are author-reported and not independently verified here.

No new findings. Every finding raised across this PR's ~55+ review rounds has been fixed with regression tests and independently re-verified, and this round's commit closes the three remaining open items with no new issue found.

Branch ·

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 121806dd6e

ℹ️ 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".

[kyle-sexton]

@codex review

[claude]

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

---

Code review complete

• Gather context (PR diff, extensive prior review history — ~55+ rounds already on this thread)
• Load /review:code-review skill and follow its instructions exactly
• Diff HEAD e4b2d08 against the previously reviewed head (121806d)
• Confirm the merge commit introduced nothing new, and trace the one new functional commit's logic
• Post findings (one, inline)

Summary

The only functional commit since the last reviewed head (121806d) is a649742 ("loop-scoped for-head bindings and quoted parameter heads"), touching inventory.py (+77/-22) and its test file (+23). The second commit, e4b2d08, is a merge of origin/main whose only non-ancestor content is 55b880f (test temp-dir cleanup, unrelated) and disk-hygiene plugin changes entirely outside this PR's claude-ops scope — confirmed via git show --stat, no overlap with any file this PR owns.

a649742 fixes three gaps:

• A let/const in a for (...) head is scoped to the loop only. New _for_scope/_statement_start compute the loop's span (from the head's ( to the end of a braced body) and _visible consults it before falling back to the plain block check. Traced for(let xx="LOCAL";;){break}return xx by hand: the loop's xx is correctly excluded once the reader is past the loop's closing brace, so the read resolves to the outer var xx="REAL" instead — matches test_a_for_head_binding_ends_with_its_loop.
• Method/function parameter lists are matched with quoted text masked (_head_open). _opens_function and the new _params_before(src, braces, body_open) both route through _head_open, which runs _mask_strings over the candidate span before depth-counting the matching (. Traced description(xx,a="("){return xx}: previously the quoted "(" would desync the backward paren-depth count and drop xx from the parsed parameter list (so it wouldn't be shadowed); now the quote is masked first, xx is correctly extracted as a parameter name. Matches test_a_quoted_paren_default_keeps_a_method_parameter.
• A function declaration whose parameter list holds parentheses leaves same-named calls unresolved rather than silently picking a different declaration. _function_body now counts `function ident(` occurrences against _function_pattern(ident) matches (which can't span nested parens) in the same module; a mismatch returns None (unresolved) instead of letting the simpler-but-wrong declaration win. Traced function helper(){return"WRONG"} outer vs. a nested function helper(x=g()){return"REAL"}: the counts mismatch (2 vs. 1), so helper() resolves to nothing rather than the real JS answer (hoisted-nested-shadows, i.e. "REAL") or the wrong outer value — consistent with this PR's established "prefer unresolved over wrong" design, and matches test_a_function_with_a_call_default_hides_no_outer_one (which only asserts != "WRONG", not == "REAL", for exactly this reason).

One finding, posted inline on inventory.py:1580: _for_scope's if not start or ... treats _statement_start's legitimate 0 return value (a real offset, when the statement begins at the very start of its 4 KiB lookback window) the same as its None "not found" sentinel — inconsistent with the sibling _statement_keyword a few lines above, which correctly tests start is None. If triggered, this would make _for_scope miss a genuine for-head and fall back to unscoped visibility for that loop variable — the exact over-broad-visibility bug this commit closes, just reopened for a narrow corner (a for loop literally at the start of the lookback window). Real-world likelihood is very low given the 2.1.285 bundle's module headers, but the fix is a one-line, zero-risk change (start is None). Full repro and fix suggestion in the inline comment.

Not independently re-run: python3 -m unittest was denied by the sandbox in this pass, consistent with every prior round on this PR — this is a static trace only. The author's test-run and before/after figures in the PR body are author-reported and not independently verified here.

Not re-raised: everything else fixed across this PR's ~55 review rounds (module-boundary resolution and visibility, deferred-read tracking, ||/?? nullish-vs-falsy handling, numeric/unary/primitive literal settlement, argument-to-parameter binding, template/join/concatenation alternative preservation, hoisted-function shadowing and lexical-scope declaration ranking, object-alias following, var-shadow detection, discover.py PascalCase splitting, native_drift.py drift intake) is unaffected by this commit and remains fixed.

Branch ·

[claude]

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

---

Security review complete

• Gather context (PR diff, changed files, extensive prior review history — ~55+ rounds already on this thread, all clean)
• Invoke /review:security-review and follow it exactly for HEAD e4b2d08
• Post findings

No security issues found.

This PR has been through many clean security-review passes on prior heads (337bf918 through 121806dd6e). This pass covers everything since the last reviewed head (121806dd6ecffce38a1b2967c9874e435b670012) through the current HEAD (e4b2d08a9d9a9fc11256cb802d6b2e8cb079b254).

Two commits landed in that window:

• a649742 ("loop-scoped for-head bindings and quoted parameter heads") — the only functional change, touching exactly two files:
  • inventory.py — continues the same static-analysis-parser refinement pattern reviewed across every prior round:
    • _head_open (replacing the inline paren-matching previously duplicated in _opens_function/_params_before) matches a parameter list's ( against quoted text blanked via the already-reviewed _mask_strings, bounded to the enclosing block or a 4096-byte window (inventory.py#L641-L646).
    • _for_scope/_statement_start (inventory.py#L1570-L1596) determine the visibility span of a let/const declared in a for(...) head. _statement_start is the pre-existing 4096-byte-bounded backward walk (unchanged logic, just extracted into its own function); _for_scope's own regex searches are a 16-byte lookback (for\s*(?:await\s*)?$) and reuse of _match_close/braces.pairs — all bounded, linear, non-backtracking-prone patterns over the same trusted local input (the operator's own installed Claude Code CLI bundle, not attacker-supplied data).
    • _function_body (inventory.py#L1858-L1868) adds a count comparison between a loose function NAME( scan and the strict _function_pattern to detect when a parameter list containing parens hides a real declaration, and conservatively returns None (unresolved) rather than guessing — consistent with this PR's established fail-safe design.
    • All of this is pure, bounded, in-memory string/regex analysis over data already read by existing code paths — no new file I/O, subprocess, eval/exec, deserialization, or network calls, and no unbounded or nested-quantifier regex that could introduce a ReDoS regression.
  • test_inventory.py — regression coverage only.
• e4b2d08 (merge of origin/main) — its only non-a649742 parent, b906919 (feat(disk-hygiene): ..., feat(disk-hygiene): catalog scope, ownership investigation and cross-target answer reuse #5630), is an unrelated, already-merged feature with no overlap with any claude-ops file — confirmed via git show --stat.

plugin.json/CHANGELOG.md changes are version-bump (0.77.4 → 0.78.0) and changelog text only, no behavioral change. discover.py, native_drift.py, and the remaining documentation files are unchanged since their prior clean security-review passes.

Full diff against the merge-base with origin/main (git diff b9069194eb..HEAD) remains exactly the PR's stated 10-file set (+1902/-86), confined entirely to plugins/claude-ops/ — the functional surface throughout is a static-analysis parser over a locally installed, trusted CLI bundle, with no subprocess, eval/exec, deserialization, network calls, or attacker-controlled input at any point in this branch's history. No injection surfaces (command/SQL/path/template), no credential/token handling, and no authorization/access-control logic anywhere in this diff. None of the changed files are CLAUDE.md/AGENTS.md/rules/skill-or-agent-body files carrying standing instructions, so the instruction-surface-deletion check doesn't apply.

Branch ·

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e4b2d08a9d

ℹ️ 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]

Shadow catch parameters before scanning catch bodies

When a followed function parameter is rebound by a catch parameter, this recursive scan preserves the call-site binding. For example, function ff(x){try{throw "LOCAL"}catch(x){return x}} called as ff("REAL") is extracted as REAL, although JavaScript returns LOCAL. Fresh evidence beyond the earlier catch-binding fix is that _resolve_chain short-circuits names already present in shadow, so the declaration-only guard never runs; extend the nested block scope with catch parameters before scanning it.

Useful? React with 👍 / 👎.

[kyle-sexton]

VALID, not fixed on this head. Basis: verified by trace: the call-site binding of x sits in shadow, so the catch-parameter guard in _binding_value never runs. This head reaches the review-round cap the operator set for this PR, so the fix (add catch parameters to the scope of the recursive catch-block scan) is left for the operator to schedule. Leaving open.

[chatgpt-codex-connector]

Mask literals when identifying control-flow blocks

When a control condition contains an unmatched parenthesis character in a string, _function_block feeds the raw source to _open_paren and can classify the control block as a function body. For example, var xx="WRONG"; function ff(){if(a==="("){var xx="REAL"}return xx} resolves ff() as WRONG, even though the local var is function-scoped and the runtime result is REAL or undefined. Fresh evidence beyond the earlier method-parameter fix is that this separate backward scan still ignores quoted text; use the masked balanced-head lookup here as well.

Useful? React with 👍 / 👎.

[kyle-sexton]

VALID, not fixed on this head. Basis: verified by trace: _function_block still matches the control head with raw text, so a quoted ( in an if condition misclassifies the block as a function body. The fix is to use _head_open (the masked matcher) there too. This head reaches the review-round cap the operator set for this PR, so the fix is left for the operator to schedule. Leaving open.