fix(claude-ops): let local and for-head bindings shadow imported and outer names in the inventory

[kyle-sexton]

Refs: #5640

Summary

The inventory extractor resolved a name through an import or an outer binding even when the reader's own scope declared or bound the same name, so a built-in description could read a foreign value.

Fix

• A name the module imports but an enclosing block of the reader declares now resolves as a runtime value, not through the import.
• A let/const in a for (...) head (including for await) shadows outer names in the loop body.
• Regression tests cover both; claude-ops bumps to 0.78.1 with a changelog entry, and extraction.md lists the scoping rules. The title's full parser replacement stays the owner's call, so this does not close the issue.

Verification

python3 -m unittest test_inventory in plugins/claude-ops/skills/inventory/scripts: 196 tests OK. plugin.json parses.

Related

Refs #5640; follows #5619.

🤖 Generated with Claude Code

[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-01T04:27:34.934176Z	8b9db26	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.

[claude]

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

---

Security review — /review:security-review

• Gather context (diff vs origin/main, read changed files)
• Run security criteria against the changed files
• Check for instruction-surface deletions (none found)
• Post results

Scope reviewed: plugins/claude-ops/skills/inventory/scripts/inventory.py, test_inventory.py, reference/extraction.md, CHANGELOG.md, plugin.json.

No security findings.

This PR is a scope-resolution bug fix in inventory.py's static JS/bytecode parser:

• _for_params (new, inventory.py#L653-L668) makes a for (...) head's let/const bindings shadow outer names in the loop body.
• The _declaration change (inventory.py#L1749-L1753) makes a name the reader's own enclosing block declares win over a same-named import.

Checked for the categories this lane cares about:

• Injection / unsafe eval: none — the tool only reads bytes with path.open(..., encoding="utf-8") and does regex/brace-map parsing; no eval, exec, subprocess, or shell invocation anywhere in the diff or the surrounding functions it touches.
• ReDoS: the two new regexes (_FOR_KEYWORD_RE, _FOR_DECL_RE) are linear — the lookahead-per-character pattern in _FOR_DECL_RE has no nested quantifiers that could backtrack exponentially.
• Trust boundary: this script is a local, read-only static-analysis tool a maintainer runs over the Claude Code binary and this repo's own source for documentation/inventory purposes. It doesn't process untrusted network input, handle credentials/secrets, or cross an authorization boundary — the fix only changes which binding a description string resolves against, affecting inventory output accuracy, not an exploitable security property.
• Instruction-surface deletions: extraction.md only adds scoping-rule documentation (for-head and local-shadowing rules); nothing is removed, narrowed, or softened, so the instruction-exception-register lens doesn't apply here.

[claude]

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

---

Code review complete

• Gather context (PR diff, changed files, CLAUDE.md)
• Review inventory.py scoping fix
• Review regression tests
• Review docs/changelog updates
• Post findings

Summary

Reviewed the scoping fix in plugins/claude-ops/skills/inventory/scripts/inventory.py (the new _declaration import-vs-local-declaration check, and the new _for_params/_FOR_KEYWORD_RE/_FOR_DECL_RE for-head shadowing), plus the new tests, extraction.md, CHANGELOG.md, and the plugin.json version bump.

One finding posted inline on inventory.py:654 (permalink): _FOR_DECL_RE's capture stops at the first of/in word it sees, so a for-head binding whose own name is of or in (e.g. for (of of list), for (const {of} of list)) is never added to the shadow set — the loop variable keeps resolving through the outer/imported binding, i.e. the exact bug class this PR otherwise fixes. Plausible in this tool's target (minified/bundled code with short mangled names), and not covered by the new tests, all of which use non-keyword-colliding names.

The two other mechanisms looked correct on manual trace:

• The _declaration check voiding an import when a visible local let/const shadows it (lines ~1748-1753) correctly scopes to the same chunk and only fires for declarations inside a block (not top-level re-declarations), matching the new test_a_local_declaration_shadows_an_imported_name case.
• The no-space destructured-head case (let{x}, const[x]) is handled correctly by the (?![\w$]) keyword-boundary check plus zero-width \s*.

Docs/changelog/version bump are consistent with the change.

Verification note: I could not execute python3 -m unittest test_inventory or scripts/run-ruff.sh in this environment (both were denied) to independently confirm the author's stated "196 tests OK" / plugin.json parses — that verification is author-claimed and unconfirmed by me. My _FOR_DECL_RE finding above was traced by hand against the regex and _param_names, not confirmed by running the suite.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8b9db260d1

ℹ️ 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 33s —— View job

---

Security review — /review:security-review

• Gather context (diff vs origin/main, read changed files)
• Run security criteria against the changed files
• Check for instruction-surface deletions (none found)
• Post results

Scope reviewed (HEAD b59020d): plugins/claude-ops/skills/inventory/scripts/inventory.py, test_inventory.py, reference/extraction.md, CHANGELOG.md, plugin.json.

No security findings.

This is the latest commit on the branch (b59020d), which merges origin/main and folds in fixes for the two issues raised in earlier review rounds on 8b9db26:

• _for_head_names now applies the for-head shadow set to unbraced loop bodies too (addresses the Codex P2 finding), and _FOR_DECL_RE now takes the whole head after let/const instead of stopping at the first of/in token, so a binding literally named of or in is still shadowed (addresses my earlier finding on 8b9db26). Both are covered by new regression tests and look correct on re-trace.
• The _declaration import-vs-local-shadowing check (L1769-L1774) is unchanged from the prior review and still correctly scopes to a block-visible declaration in the reader's own chunk.

Checked again against this lane's criteria on the full current diff:

• Injection / unsafe eval: none — still pure regex/brace-map parsing of local UTF-8 text; no eval, exec, subprocess, or shell invocation anywhere touched by this diff.
• ReDoS: the new/changed regexes (_FOR_KEYWORD_RE, _FOR_DECL_RE) are linear character-class/anchor patterns with no nested quantifiers; _FOR_DECL_RE's capture ([^;]*) is a single bounded scan, no backtracking blowup.
• Trust boundary: unchanged from the prior assessment — this is a local, read-only static-analysis tool a maintainer runs over the Claude Code binary and this repo's own source for documentation purposes. It doesn't cross an authorization boundary, handle credentials, or process attacker-supplied network input; the fix only changes which binding a description string resolves against (inventory output accuracy, not an exploitable property).
• Instruction-surface deletions: the only removed lines are in extraction.md, replaced by a strictly broader paragraph (adds catch-parameter, for-head, and enclosing-block shadowing rules) — no guardrail, CLAUDE.md/AGENTS.md, or hook rule is touched, so the instruction-exception-register lens doesn't apply.

[claude]

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

---

Code review complete

• Gather context (PR diff vs origin/main, changed files, CLAUDE.md/AGENTS.md)
• Invoke /review:code-review and follow its instructions
• Post findings as inline PR comments
• Post summary comment

Summary

Reviewed the full diff (inventory.py, test_inventory.py, extraction.md, CHANGELOG.md, plugin.json) at head b59020d, focused on the newest commit (3ea6bb6) that addressed the two prior Codex/Claude findings (unbraced loop body, binding named of/in).

One finding posted inline on inventory.py:1070-1077: the unbraced-for-body fix sets loop correctly when the for-head closes, but then overwrites it with NO_SCOPE on the very next depth-0 closing paren, even when that paren belongs to a nested if/while/call/second for still inside the same (single-statement) loop body. Traced by hand: function ff(x){for(const x of a)if(x)return x;return x} loses the for-head's shadow for x the moment if(x) closes, so return x resolves through the outer/imported binding again — the same bug class this PR otherwise fixes. Not covered by the new tests, which only exercise a direct return right after the for-head, or a statement that truly ends (;) before the return. Full repro and suggested direction (only clear loop at the statement-ending ,/;; union rather than overwrite on a nested paren) are in the inline comment.

The other two mechanisms in this diff (the _declaration import-vs-local-shadow check, and _FOR_DECL_RE now taking the whole head so a binding named of/in is caught) traced correctly and match their new tests.

Verification note: I could not execute python3 -m unittest test_inventory in this environment (denied) to confirm either my finding's repro or the author's stated "196 tests OK" — both are traced-by-hand/author-claimed, not independently run by me.

[claude]

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

---

Code review complete

• Gather context (PR diff vs origin/main, changed files, CLAUDE.md/AGENTS.md)
• Invoke /review:code-review and follow its instructions
• Post findings as inline PR comments
• Post summary comment

Summary

Reviewed the full diff at head 5812371, focused on the newest commit (5812371), which addressed the prior finding that a nested unbraced statement (e.g. if(x)return x) in a for-loop's body dropped the loop's shadow on the next depth-0 ). That fix changes loop from an overwrite to an accumulate-via-union, and threads loop into nested braced blocks (shadow | loop | ... on line 1055-1058).

One finding posted inline on inventory.py:1061-1077: fixing the overwrite also removed the only place loop got reset when the for-body's single statement is itself brace-delimited (if/else, try/catch, a nested braced loop) and ends at a } with no following ,/; — common in minified code that omits optional semicolons after blocks. loop is now only cleared at a top-level ,/;, so it leaks past the end of the for statement into later sibling statements in the same block, wrongly shadowing a same-named import/outer binding that is no longer inside the loop. Concrete repro and suggested direction are in the inline comment. Not covered by the new test (test_a_nested_statement_in_an_unbraced_for_body_keeps_the_head_binding), which only checks the shadow survives through a nested statement, not that it's dropped once the loop is actually over.

The rest of this commit's change — threading loop into the recursive _scan call for a braced branch body (line 1055-1058) — traced correctly for the cases the new test exercises (if(x)return x, for(const y of b)return x).

Verification note: I could not execute python3 -m unittest test_inventory in this environment (denied) to run either my finding's repro or the author's stated "196 tests OK" — both are traced-by-hand/author-claimed, not independently run by me.
(branch: fix/5640-inventory-scope-shadowing)

[claude]

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

---

Security review — /review:security-review

• Gather context (diff vs origin/main, read changed files)
• Run security criteria against the changed files
• Check for instruction-surface deletions (none found)
• Post results

Scope reviewed (HEAD 5812371): latest commit touches only plugins/claude-ops/skills/inventory/scripts/inventory.py and test_inventory.py (CHANGELOG.md, extraction.md, plugin.json are unchanged since the prior review at b59020d, already cleared with no findings).

No security findings.

This commit fixes the nested-statement scope-reset bug flagged in the prior Claude code-review round (loop was being overwritten to NO_SCOPE on any depth-0 closing paren, not just the for-head's own):

• inventory.py#L1055-L1058 now folds loop into the shadow set passed to a nested braced block.
• inventory.py#L1071-L1075 now unions (loop = loop | _for_head_names(...)) at each depth-0 ) instead of overwriting, so a nested unbraced if/while/for no longer clears an outer for-head's binding. loop is still reset only at the statement-ending depth-0 ,/; (L1077, unchanged).

Checked against this lane's criteria:

• Injection / unsafe eval: none — still pure regex/brace-map parsing of local UTF-8 text; no eval, exec, subprocess, or shell invocation touched by this diff.
• ReDoS: no regex changed in this commit — only control-flow (union vs. overwrite of a Scope, which is a frozenset-like union of names already bounded by the source text).
• Trust boundary: unchanged from prior rounds — a local, read-only static-analysis tool a maintainer runs over this repo's own source. The fix only changes which binding a description string resolves against (inventory output accuracy), not an exploitable property; it doesn't cross an authorization boundary, handle credentials, or process untrusted network input.
• Instruction-surface deletions: none — no CLAUDE.md/AGENTS.md/rules file/hook list touched by this commit.

[claude]

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

---

Code review complete

• Gather context (PR diff vs origin/main, changed files, CLAUDE.md/AGENTS.md)
• Invoke /review:code-review and follow its instructions
• Post findings as inline PR comments
• Post summary comment

Summary

Reviewed the full diff at head ca4f2ff, focused on the newest commit (ca4f2ff), which addressed the prior finding that loop leaked past the for-body's single statement when that statement was itself a brace-delimited compound (if/try/nested for/while) ending at } with no trailing ;. The fix adds a loop = NO_SCOPE reset after any {...} close, skipped only when the brace is immediately followed by else/catch/finally.

One finding posted inline on inventory.py:1061-1063: that reset isn't guarded by depth == 0, unlike the recursive-scan eligibility check a few lines above it in the same if c == "{": branch. So any object literal nested inside a call or condition's parens within the loop's own unbraced body statement (e.g. for(const x of a)if(t(x,{k:1}))return x) clears loop prematurely, before the statement that should still carry the shadow has finished — return x then resolves x through the outer/imported binding again, the same bug class this PR otherwise fixes. Full trace and suggested direction (nest the reset inside the existing block and depth == 0 and (prev == ")" or prev_word in (...)) guard) are in the inline comment. Not covered by the new test (test_an_unbraced_for_body_ending_in_a_block_ends_its_head_binding), which only has a direct if(c){g()}return x with no intervening object literal/nested parens.

The rest of this commit — resetting loop once the for-body's single braced-compound statement actually ends, and keeping it alive through else/catch/finally continuations — traced correctly for the case the new test exercises.

Verification note: I attempted to run the repro and python3 -m unittest test_inventory directly in this environment but command execution was denied (consistent with the verification notes on earlier rounds of this PR), so both my finding and the author's stated "196 tests OK" are traced/author-claimed, not independently executed by me.
(branch: fix/5640-inventory-scope-shadowing)

[claude]

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

---

Security review — /review:security-review

• Gather context (diff vs origin/main, read changed files)
• Run security criteria against the changed files
• Check for instruction-surface deletions (none found)
• Post results

Scope reviewed (HEAD ca4f2ff): the latest commit (ca4f2ff) touches only
inventory.py
and test_inventory.py. CHANGELOG.md, extraction.md, and plugin.json are unchanged since the
prior security-review round at 5812371, already cleared with no findings.

No security findings.

This commit is a 2-line addition that resets the unbraced-for-body loop shadow set to
NO_SCOPE once the loop's single statement closes a brace with no trailing else/catch/finally
(inventory.py#L1061-L1062),
fixing the scope-leak flagged in the prior code-review round. It reuses the existing
else|catch|finally keyword check already present a few lines up, adds no new regex, and changes
no I/O or control-flow outside this local bookkeeping variable.

Checked against this lane's criteria on the full current diff:

• Injection / unsafe eval: none — this remains pure regex/brace-map parsing of local UTF-8
  text (path.open(..., encoding="utf-8")); no eval, exec, subprocess, or shell invocation
  anywhere in the diff.
• ReDoS: no regex added or changed in this commit; the only new logic is a boolean
  reset of a local variable.
• Trust boundary: unchanged from prior rounds — a local, read-only static-analysis tool a
  maintainer runs over the Claude Code binary and this repo's own source for documentation
  purposes. It doesn't cross an authorization boundary, handle credentials, or process
  attacker-supplied network input; the fix only changes which binding a description string
  resolves against (inventory output accuracy), not an exploitable property.
• Instruction-surface deletions: none — no CLAUDE.md/AGENTS.md/rules file/hook list is
  touched by this PR; extraction.md's change (from the earlier commits, unchanged since) only
  adds scoping-rule documentation, nothing removed or narrowed.

(Note: outstanding correctness findings about this parsing logic's edge cases belong to the
code-review lane, not this security lane — no new ones identified here.)
(branch: fix/5640-inventory-scope-shadowing)

[claude]

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

---

Security review — /review:security-review

• Gather context (diff vs origin/main, read changed files)
• Run security criteria against the changed files
• Check for instruction-surface deletions (none found)
• Post results

Scope reviewed (HEAD 6244eb1): the latest commit (6244eb1) touches only
inventory.py.
CHANGELOG.md, extraction.md, plugin.json, and test_inventory.py are unchanged since the
prior security-review round at ca4f2ff, already cleared with no findings.

No security findings.

This commit adds a depth == 0 guard to the loop = NO_SCOPE reset that a prior code-review
round flagged as firing on every {...} the scan steps over, including one nested inside a
call's or condition's parens within the for-body's own statement
(inventory.py#L1062-L1065).
The fix narrows an existing conditional rather than adding new parsing surface.

Checked against this lane's criteria on the full current diff:

• Injection / unsafe eval: none — this remains pure regex/brace-map parsing of local UTF-8
  text (path.open(..., encoding="utf-8")); no eval, exec, subprocess, or shell invocation
  anywhere in the diff.
• ReDoS: no regex added or changed in this commit — the only new logic is an added
  depth == 0 boolean check guarding an existing reset of a local variable.
• Trust boundary: unchanged from every prior round — a local, read-only static-analysis tool
  a maintainer runs over the Claude Code binary and this repo's own source for documentation
  purposes. It doesn't cross an authorization boundary, handle credentials, or process
  attacker-supplied network input; the fix only changes which binding a description string
  resolves against (inventory output accuracy), not an exploitable property.
• Instruction-surface deletions: none — no CLAUDE.md/AGENTS.md/rules file/hook list is
  touched by this PR; extraction.md's change (from earlier commits, unchanged since) only adds
  scoping-rule documentation, nothing removed or narrowed.

(Note: outstanding correctness findings about this parsing logic's edge cases belong to the
code-review lane, not this security lane — no new ones identified here.)
(branch: fix/5640-inventory-scope-shadowing)

[claude]

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

---

Security review — /review:security-review

• Gather context (diff vs origin/main, read changed files)
• Run security criteria against the changed files
• Check for instruction-surface deletions (none found)
• Post results

Scope reviewed (HEAD 968b07c): plugins/claude-ops/skills/inventory/scripts/inventory.py, test_inventory.py, reference/extraction.md, CHANGELOG.md, plugin.json.

No security findings.

HEAD 968b07c is a merge of origin/main into this branch. Diffed against the last-reviewed commit (6244eb1), inventory.py, test_inventory.py, and extraction.md are byte-identical — no new logic since that round, which was already cleared with no findings. The merge only brings in:

• plugin.json / CHANGELOG.md: a version-string bump (0.78.3 → 0.79.1) and changelog merge, picking up an unrelated /claude-ops:check skill entry added on main. Prose/metadata only, no code.

Checked the full current diff against this lane's criteria (consistent with every prior round on this PR):

• Injection / unsafe eval: none — inventory.py is pure regex/brace-map parsing of local UTF-8 text (path.open(..., encoding="utf-8")); no eval, exec, subprocess, or shell invocation anywhere in the diff.
• ReDoS: no regex changed since the last reviewed commit; _FOR_KEYWORD_RE/_FOR_DECL_RE remain linear, no nested quantifiers.
• Trust boundary: unchanged from every prior round — a local, read-only static-analysis tool a maintainer runs over the Claude Code binary and this repo's own source for documentation purposes. It doesn't cross an authorization boundary, handle credentials, or process attacker-supplied network input; the fix only changes which binding a description string resolves against (inventory output accuracy), not an exploitable property.
• Instruction-surface deletions: none — extraction.md's change vs origin/main (permalink) only broadens an existing shadowing-rule paragraph (adds catch-parameter, for-head, and enclosing-block cases); nothing removed, narrowed, or softened. No CLAUDE.md/AGENTS.md/rules file/hook list is touched.

(Note: outstanding correctness findings about _scan's for-loop parsing edge cases belong to the code-review lane, not this security lane — no new ones identified here.)
(branch: fix/5640-inventory-scope-shadowing)

[claude]

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

---

Code review complete

• Gather context (PR diff vs origin/main, changed files, CLAUDE.md/AGENTS.md)
• Invoke /review:code-review and follow its instructions
• Post findings as inline PR comments
• Post summary comment

Summary

Reviewed the full diff at head 968b07c against origin/main. The latest commits since the last review round (5812371, ca4f2ff, 6244eb1) fixed the previously-flagged "any depth-0 brace resets loop" bug for nested statements inside the loop's own unbraced body (if(t(x,{k:1}))return x now correctly keeps the shadow — traced and confirmed). 968b07c itself is a clean merge of unrelated origin/main changes with no net diff to the reviewed files.

One finding posted inline on inventory.py:1061-1065: the depth == 0 guard added in 6244eb1 stops a nested brace from resetting loop, but the reset still fires unconditionally for any depth-0 {...}, including a bare object-literal value inside the for-body's own return statement — not only the statement-ending control-flow/function block the surrounding if is actually checking for. Concretely, function ff(x){for(const x of a)return {a:1}||x} clears the for-head's x shadow the moment {a:1} closes (well before the return statement is done), so the ||x fallback resolves x through the outer/imported binding instead of staying shadowed — the same bug class this PR fixes everywhere else. Same issue with a ternary: return cond?{y:1}:x. Full trace, and the suggested fix (move the reset inside the same prev == ")"/else/try/finally gate already used for the recursion eligibility check a few lines up), are in the inline comment.

The rest of this round's changes — the loop union-not-overwrite fix for nested unbraced statements, and the block-close reset now requiring depth == 0 — traced correctly for everything the existing tests exercise.

Verification note: python3 -m unittest test_inventory was denied in this environment, consistent with every earlier round on this PR. My finding above and the author's stated "196 tests OK" are both traced/author-claimed, not independently executed by me.
(branch: fix/5640-inventory-scope-shadowing)

[claude]

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

---

Code review complete

• Gather context (PR diff vs origin/main, changed files, CLAUDE.md/AGENTS.md)
• Invoke /review:code-review and follow its instructions
• Post findings as inline PR comments
• Post summary comment

Summary

Reviewed the full diff at head d86624e against origin/main. This round focuses on the newest commit, d86624e, which addresses the finding from the prior code-review round (posted on 968b07c): a value-position object literal (return {a:1}||x, return c?{k:1}:x) was prematurely clearing the unbraced for-body's loop shadow before the rest of the same statement finished evaluating.

No new findings. The fix moves the loop = NO_SCOPE reset (and its else/catch/finally continuation check) from an unconditional "any depth-0 {...} close" site to inside the existing if (block and depth == 0 and (prev == ")" or prev_word in ("else", "try", "finally")) and not _opens_function(...)) gate that already governs the recursive _scan call for a genuine statement-block body. I traced this against the three cases raised across this PR's review history:

• return {a:1}||x / return c?{k:1}:x — prev_word is "return" and prev isn't ")" when the object-literal brace is reached, so the gate is false, recursion is skipped, and (with this fix) the reset is skipped too — loop survives to shadow x in the fallback/branch. Matches the new "return c?{k:1}:x" subtest in test_a_nested_statement_in_an_unbraced_for_body_keeps_the_head_binding.
• for(const x,c){for(const x of a)if(c){g()}return x} — if(c) closes its condition paren (prev == ")") so the gate is true, the block recurses, and since nothing matching else|catch|finally follows {g()}, ends is True and loop correctly clears once the loop's own statement is over. Matches test_an_unbraced_for_body_ending_in_a_block_ends_its_head_binding.
• if(t(x,{k:1}))return x — the inner {k:1} sits at paren depth > 0 (inside t(...) inside if(...)), so the depth == 0 component of the gate is already false regardless of this change; still correctly doesn't reset. Matches the existing subtest for that shape.
• if(c){g()}else{h()}return x — traced the else-continuation path by hand: the first block's ends check sees else{...} following and keeps loop alive across the chain, and the final block (no continuation keyword following) clears it — consistent with intent and not regressed by this change.

The rest of the diff (CHANGELOG.md 0.79.0 → 0.79.1, plugin.json version bump, extraction.md's broadened shadowing-rule paragraph) is consistent with the code change and was already reviewed clean in earlier rounds; byte-identical since then except for this commit.

Verification note: python3 -m unittest test_inventory was denied in this environment, consistent with every earlier round on this PR. The trace above is hand-executed against _scan's updated control flow, not run through the actual test harness; the author's stated "196 tests OK" remains author-claimed and unconfirmed by me.
(branch: fix/5640-inventory-scope-shadowing)

[claude]

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

---

Security review — /review:security-review

• Gather context (diff vs origin/main, read changed files)
• Run security criteria against the changed files
• Check for instruction-surface deletions (none found)
• Post results

Scope reviewed (HEAD d86624e): the latest commit (d86624e) touches only
inventory.py
and test_inventory.py. CHANGELOG.md, extraction.md, and plugin.json are unchanged since
the prior security-review round at 968b07c, already cleared with no findings.

No security findings.

This commit relocates the loop = NO_SCOPE reset (previously unconditional at depth == 0 for
every {...} close) to inside the existing if (block and depth == 0 and (prev == ")" or prev_word in ("else", "try", "finally")) and not _opens_function(...)) guard that gates the
recursive _scan call
(inventory.py#L1061-L1070).
This fixes the scope-leak flagged in the prior code-review round, where a value-position object
literal in the for-body's own return statement (e.g. return {a:1}||x) was wrongly clearing the
loop binding before the statement finished. No new regex, no new I/O, no new control-flow surface
beyond moving an existing conditional's placement.

Checked against this lane's criteria on the full current diff:

• Injection / unsafe eval: none — still pure regex/brace-map parsing of local UTF-8 text
  (path.open(..., encoding="utf-8")); no eval, exec, subprocess, or shell invocation
  anywhere in the diff.
• ReDoS: no regex added or changed in this commit — the else|catch|finally pattern is
  reused verbatim, just relocated; still linear with no nested quantifiers.
• Trust boundary: unchanged from every prior round — a local, read-only static-analysis tool
  a maintainer runs over the Claude Code binary and this repo's own source for documentation
  purposes. It doesn't cross an authorization boundary, handle credentials, or process
  attacker-supplied network input; the fix only changes which binding a description string
  resolves against (inventory output accuracy), not an exploitable property.
• Instruction-surface deletions: none — no CLAUDE.md/AGENTS.md/rules file/hook list is
  touched by this PR; extraction.md is unchanged since the last-reviewed commit.

(Note: outstanding correctness findings about _scan's for-loop parsing edge cases belong to the
code-review lane, not this security lane — no new ones identified here.)
(branch: fix/5640-inventory-scope-shadowing)