[melodic-automation-lanes]

Refs: #5901

Summary

Opened by the work-resolve-issue lane. The run's own summary:

This is fix (a) from the 2026-11-03 host run: under --reader=parser, a computed read no longer counts as an unknown load or a Function reach when its key provably cannot spell require or constructor. A new keySpellsOf in parser_helper.cjs bounds keys that keyValuesOf leaves unknown: + (a number, or a string that starts with its left side and ends with its right, so e\[n+1\] spells neither), +=, templates, branches, variables, and parameters of a non-exported function that is only ever called directly. unknownLoad, the constructor checks and the computed-write keyNames now all use it, so one place decides what a key may spell. The bound applies only in strict code (a module with an import or export, or a first-statement "use strict"). Sloppy code, including folded Function bodies, keeps the old answers, because a fresh-context review showed f.caller can call a function with any argument there. Fix (b) is not built: Flow.calls already follows a returned array into importers through its export-call exit. Flow.parameterValues threw the same reason string, "the function is used other than by a call", and it does not skip an export-list reference. So the refusal reported near chunk-bwpc82tt.js's export list most likely came from a callback parameter of a list-exported function (inference, unverified). That reason now reads "the parameter's function is used other than by a call", so the next host run can tell the two apart. The --reader default is unchanged, held for its own owner decision as recorded 2026-10-26.

Remaining work

• Host run on the current build: inventory.py --reader compare --self-check --binary-only to count how many of 2.1.316's "*" modules (751, 422 with no load text) still load an unnamed file, and whether Explore and Plan disallowed_tools resolve fully with no value->value change
• Fix (b): find which non-call reference refuses the flow at chunk-bwpc82tt.js's export list. The new parameterValues reason string tells it apart from Flow.calls on the next host run. Then follow that callback across modules or show it only reads the array
• Bound whatever computed reads and loads the host run still counts as unknown (for-in keys, parameters of exported or escaping functions, modules that may run sloppy, real dynamic loads)

Not verified

• The Python suites that drive the helper (test_parser_reader, test_reader_findings, test_inventory_under_parser) were not run: this lane can run no node or python interpreter. Left to the PR's CI. Every new and edited case was traced by hand twice, once by the fresh-context reviewer.
• Neither acceptance criterion was checked: both need a host run of inventory.py --reader compare --self-check --binary <build> --binary-only on the current Claude Code build, and this lane has no Claude Code binary. How many of the 751 "*" modules on 2.1.316 this clears is unmeasured.
• Tidy skipped: the change is 336 changed lines (parser helper 225, tests 81, reference doc 23, fragment 7), over the 300-line tidy cutoff.
• review-venue.txt was absent from the scratchpad, so the review ran locally in one fresh-context subagent, as the skill directs.
• Pre-existing and left unfixed (the reviewer's finding 2): keyValuesOf analyzes every program as a module, so in sloppy code (folded Function bodies) a block function sharing a var's name is not counted as a write to it. Example: `var k="x";{function k(){}}k.toString=()=>"constructor";(()=>0)

Fix

• plugins/harness-ops/skills/inventory/scripts/js/parser_helper.cjs: add strictCode and keySpellsOf (key-provenance predicate with a per-node keyValues cache and a step limit); route unknownLoad, keyNames, prototypeOnly and maySpellConstructor through it; hoist exportNames, argumentsAliased and inBloc
• plugins/harness-ops/skills/inventory/scripts/test_parser_reader.py: sinks cases for bounded and unbounded keys in strict code (n+1, a variable, +=, templates, a parameter whose callers pass known keys, recursion, plus escaping, exported, block-declared, arguments-reading and written parameters); slo
• plugins/harness-ops/skills/inventory/scripts/test_reader_findings.py: end-to-end cases in which a strict module holding such reads keeps the spread literal, and ones where a key that may spell require, an escaping function, or sloppy code still reads partial; one more parameter case that stays a sin
• plugins/harness-ops/skills/inventory/reference/extraction.md: describe the strict-code key bounds in the sink rule and the load scan, and why sloppy code keeps known-values-only bounds
• .changes/harness-ops/work-resolve-issue-issue-5901-41567742390-1-68615e17.md: minor fragment under Added

Verification

The runner ran each check from the base copy against 4f6e66a:

• scripts/check-changelog-fragments.sh --check: passed
• scripts/check-changed-skills.sh --merge-base: passed

The run reports:

• check-changelog-fragments (run 3 times, last after the final edit): All 18 changelog fragment(s) under .changes/ are valid
• check-changed-skills (run 3 times): CHECK-SKILL inventory PASS, 0 errors, 2 warnings (SKILL.md body size, pre-existing and untouched)
• check-affected (run 3 times): 12 passed, 6 skipped (typos not installed; validate-plugins sub-steps and changed-skills that run the checked tree's code); markdown, ruff, comment-hygiene, machine-specific-paths, changelog-parity and stale-base-overlap passed
• fresh-context review subagent over the whole diff: 1 new HIGH (in sloppy code, including folded Function bodies, h.caller reaches a function without its name, so the parameter bound was unsound there), fixed by limiting every new bound to strict code; LOW repeated keyValues work, fixed with a per-
• read https: //developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Addition: both operands are coerced to primitives; if one side is a string both are concatenated as strings; BigInt mixed with a non-BigInt throws TypeError; otherwise numeric addition
• read https: //developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/caller: a non-strict function's caller is the function that called it, null when that caller is strict; reading caller on a strict function throws
• read https: //developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/function: in non-strict code a block function declaration's name becomes visible outside the block (results vary by engine); in strict code it is scoped to the block
• read https: //developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Strict_mode: modules are strict automatically; a whole-script "use strict" must come before any other statement; sloppy arguments track named parameters, strict ones do not; Function code invokes strict mode inside its own body
• The lane run: https://github.com/melodic-software/claude-code-plugins/actions/runs/41567742390

Left out: the items under Not verified.

Related

#5901

[github-actions]

claude-security-review reviewed 4f6e66a2a3843d515ecef4d95e47c4e7267e9f5d: read 5 of 5 changed file(s); 0 finding(s), 0 as inline comments.

Security review: PR #12665 (head 4f6e66a)

I found no security issues.

I read every hunk of all 5 changed files. None of them matches a trusted path.

What changed

• plugins/harness-ops/skills/inventory/scripts/js/parser_helper.cjs adds two functions to the inventory skill's offline static analyser: strictCode and keySpellsOf. keySpellsOf works out which names a computed property key could spell. The PR also moves three existing helpers (exportNames, argumentsAliased, inBlock) out of freshnessOf to module level without changing how they behave, and rewords one Unresolved reason string.
• The two Python test files, extraction.md and a changelog fragment hold only tests, docs and the release note.

Checks

• Execution and injection: the helper only parses and walks an AST (espree/eslint-scope). The diff adds no eval, vm, Function, child process, file write, network call or shell use. The new regexes (NUMERIC_KEY, NUMERIC_HEAD, NUMERIC_TAIL) are anchored and contain no nested quantifiers, so they can't cause catastrophic backtracking. A step limit (SPELL_STEPS = 512) and a depth limit (16) bound the recursion in walk.
• Trust boundary: the analyser reads a Claude Code build to produce an inventory report. It doesn't decide who or what the automation trusts. It holds no token, gates no merge and doesn't feed a credentialed lane. If the new key bounds turn out to be unsound, the inventory under-reports a sink or an unknown load. That is a correctness question for the code-review lane, not a leak, a compromise or a charge.
• Trust controls: none touched. No change to trusted-trigger, lane token, kill switch, token broker, merge holds or any .github/actions/** action.
• Plugin manifests: none changed.
• Instruction surface: no standing instruction is deleted or softened. The extraction.md edits describe the new analysis rules and remove no guardrail.
• Embedded text: the PR context contains no directive aimed at this lane.

critical-findings: 0

[github-actions]

claude-review: 1 inline finding(s) on 4f6e66a2a3843d515ecef4d95e47c4e7267e9f5d; the summary is in this lane's pull request comment.

claude-lane: claude-review

[github-actions]

Blocking: in strict code, a bounded computed read can now reach eval or Function with no sink hit.

Before this change, any computed key keyValues could not resolve was treated as possibly spelling constructor. That catch-all also covered reads of the other dangerous members: the eval/Function checks at L2314-L2316 call memberName, and memberName returns null for any non-literal key. So those checks never see a computed read.

Now spells only asks whether a key can spell constructor (and require in unknownLoad). A key it bounds can still spell eval or Function:

export{};globalThis[`ev${"al"}`]("Array.prototype.includes=()=>!0")
// texts ["ev",""]: can't spell "constructor" or "require" -> no hit (before: function-constructor + unknown-load)

export{};const J=(o,k)=>o[k];J(globalThis,"Function")(s)()
// k is bounded to {"Function"} by the only caller -> no hit

In both modules a string now runs as code with nothing reported, so a trusted list can stay literal when it should read partial. The same gap already existed for known-valued keys (var k="Function";globalThis[k]), but this PR extends it to every template, + and parameter key that keySpellsOf bounds.

Fix: for a computed read (node.computed && memberName(node) === null), also report indirect-eval / function-constructor when spells(node.property, "eval") / spells(node.property, "Function") is true. Probably do the same for the VM_RUNNERS names and __defineGetter__/__defineSetter__ in definerOf. Then add both modules above to test_a_computed_key_counts_only_for_the_names_it_may_spell as expected sinks.

claude-lane: claude-review

[github-actions]

claude-review reviewed 4f6e66a2a3843d515ecef4d95e47c4e7267e9f5d: read 5 of 5 changed file(s); 1 finding(s), 1 as inline comments.

Code review: bound computed keys by provenance in the parser reader

I read every hunk of all 5 changed files. I also read the parts of parser_helper.cjs the change relies on: keyValuesOf, functionBinding, keyName/memberName, unknownLoad, the sink loop and scanOf's deferred. None of the changed files is on a trusted path, and no userConfig or plugin manifest changed.

What holds up when traced by hand (I did not run any tests):

• + and +=. The start/end checks in affix agree with ToPrimitive/ToString. Results that come out as numbers (true+1, undefined+1 → NaN) are covered because the code returns early with NUMERIC_KEY.test(name).
• Templates. Overlapping start and end texts only over-approximate, which is the safe direction.
• Cycles. A cycle through seen returning false is sound. Every recursive next() passes the value through unchanged (?:, logical, sequence, =, initializers, call arguments). + asks only keyValues, never walk.
• Strict-code detection. strictCode reads acorn's directive, which is the raw text, so an escaped "use\x20strict" correctly does not count. Import and export statements mark a module.
• scanOf's deferred. Changing its return from new Set() to false keeps the same meaning under the new spells contract.
• Renamed refusal reason. Nothing else in plugins/harness-ops matches the old string at parser_helper.cjs:958. The other throw site at L817 keeps the old string on purpose.

Finding (inline at parser_helper.cjs:2179): in strict code, a computed read whose key is bounded but unresolved can still reach eval or Function, and nothing now reports it. Before this change, any key keyValues could not resolve fell into the "may spell constructor" catch-all. That catch-all also covered member reads of eval and Function, because the name checks at L2314-L2316 see only literal keys. With the new bound, both of these get no hit:

• globalThis[`ev${"al"}`](s)
• const J=(o,k)=>o[k];J(globalThis,"Function")(s)()

The same gap already existed for a key with known values (var k="Function";globalThis[k]). This PR widens it to every key keySpellsOf bounds, and no test covers it.

Closing issue: the PR context names no closing issue (it says Refs: #5901, not Closes), so there were no acceptance criteria to check. The author says the two host-run criteria were not run and the Python suites were left to CI. Both claims are the author's and I have not verified them.

Path-scoped rules: .claude/rules/skill-bodies-state-current-rules.md and ruff-pin.md match changed files. extraction.md describes the reader's own rule and restates nothing from an upstream source, so neither rule is broken.

No embedded instructions were found in the PR context. There are no prior findings, and the round cap does not apply. Nothing in this change costs money.

[melodic-automation-lanes]

Unattended merge: native auto-merge leaves this pull request unarmed at head 4f6e66a2a3843d515ecef4d95e47c4e7267e9f5d. The lanes clear this reason on their own; the next scheduled run judges it again.

pr-merge's gate refuses its verifier verdict (verify:missing)