[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]

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 c8ede301c0207e53432f98876fa70e5d9f34579d. The lanes clear this reason on their own; the next scheduled run judges it again.

a pr-unblock lane stopped its head (pr-unblock/address-feedback is failure), which clears when the
  lane runs again

[melodic-automation-lanes]

pr-unblock / address-feedback finished round 1 of max 4 on 70b50d1.

[melodic-automation-lanes]

The pr-unblock lane stopped fix-ci on this pull request: the lane could fix a failing check only by weakening a test, which is refused, so it runs one fresh fixer round on this head that must keep the test's assertions. The lane sets no label: the run ends in a failing pr-unblock/fix-ci status on the head the lane gated.

• Reason: ci-fix-weaken-retry
• Head: 7014fa5e682ffca7dca0b3392a3836c26cca9989
• Run: https://github.com/melodic-software/claude-code-plugins/actions/runs/41609256148

When a retry on this head can change the outcome, the lane runs one on its own, up to its per-head cap, and edits this comment if it stops again; a run that does not stop clears the status. When no retry can (a review thread the lane does not take or settle, or a failing check after the retries are spent), a person resolves the threads or gets the checks green, then dispatches the workflow below: that run finds the cause gone and clears the status. On a pull request the lanes App opened, the dispatchers start that run once per stop; on any other, a person does. A new commit to the branch starts clean.

To run it again on this head now, dispatch the pr-unblock workflow:

gh workflow run pr-unblock.yml -R melodic-software/claude-code-plugins -f pr-number=12665 -f activity=fix-ci

[github-actions]

claude-security-review needs no new review of c8ede301c0207e53432f98876fa70e5d9f34579d: no file of this pull request changed since the review of d1cfff6.

Security review: PR #12665 (head d1cfff6)

No security issues found. 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 strictCode and keySpellsOf to the inventory skill's offline static analyser. They bound which names a computed property key can spell. Three helpers (exportNames, argumentsAliased, inBlock) move to module level and behave the same, and one Unresolved reason string is reworded.
• Since this lane's last review (at 4f6e66a2), the head adds the fix for the settled thread at L2179: maySpellAnywhereSink and a new computed-read check that reports indirect-eval, function-constructor, vm-string-code or definer-escape when a bounded key can spell eval, Function, a vm runner or a getter/setter definer. It also adds test cases for those reads. The settled thread is marked lines changed since resolution: no, and the code it raised is now covered.
• The two Python test files, extraction.md and a changelog fragment contain only tests, docs and the release note.

Checks

• Execution and injection: the helper only parses and walks an AST. 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 have no nested quantifiers. 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 and produces an inventory report. It holds no token, gates no merge and feeds no credentialed lane. If the new bounds were unsound, the inventory would under-report a sink. That is a correctness question for the code-review lane, not a leak, a compromise or a charge.
• Trust controls: none touched. Nothing changes in the trusted-trigger check, the lane token, the kill switch, the token broker, the 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.
• Prior findings: this lane's earlier summary at 4f6e66a2 reported 0 findings, so nothing is repeated. The round cap was not reached.

critical-findings: 0

[github-actions]

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

claude-lane: claude-review

[github-actions]

Blocking: the fix for computed eval/Function reads covers member reads only, not destructured keys.

This check handles node.type === "MemberExpression" only. A computed key in an object pattern is judged only by maySpellConstructor (L2338) and by the vm destructure check, which takes literal keys only (L2358). So in strict code a destructured key that keySpellsOf bounds to eval or Function gets no hit:

export{};const J=(o,k)=>{const{[k]:f}=o;return f};J(globalThis,"eval")(s);
// k bounded to {"eval"} by its only caller: spells(k,"constructor") false,
// spells(k,"require") false -> no hit. Before this PR: function-constructor.

export{};const J=(o,k)=>{const{[k]:F}=o;return F};J(globalThis,"Function")(s)();

The same applies to a vm runner taken from a vm namespace by a bounded destructured key.

Literal destructuring ({eval:f}=globalThis) already had this gap before the PR. Computed keys did not: before, every unresolved computed key counted as possibly spelling constructor. This PR removed that catch-all for every key keySpellsOf bounds.

Fix: extend the condition to node.type === "Property" && holder?.type === "ObjectPattern" && node.computed, using node.key in place of node.property. Better still, check every destructured key, literal ones included, against the same five names. Then add the two modules above to test_a_computed_key_counts_only_for_the_names_it_may_spell as expected hits.

claude-lane: claude-review

[github-actions]

Advisory: the doc and the changelog fragment don't describe the behavior change from the round-1 fix.

maySpellAnywhereSink (parser_helper.cjs:2337) means a constructor read is exempt as inert only when its key also can't spell eval, Function, a vm runner or __defineGetter__/__defineSetter__. An unknown key, such as a for in key or any key in sloppy code, can spell all of them.

So a discarded or tested unknown-key read is now a function-constructor hit, in sloppy code too. Common bundle code like for(k in o)if(o[k]) used to be exempt and now counts.

This paragraph still says a constructor read that is "discarded, tested, compared…" does not count, and that "any member named eval or Function" counts, which reads as literal names only. The fragment says sloppy code "is bounded only by known values", which suggests its results are unchanged.

It is the safe direction, but the next host run's count of function-constructor hits will rise for a reason the doc doesn't give. Please say in this paragraph and in the fragment that a computed read whose key may spell one of those members counts wherever it stands, inert or not.

claude-lane: claude-review

[github-actions]

claude-review needs no new review of c8ede301c0207e53432f98876fa70e5d9f34579d: no file of this pull request changed since the review of d1cfff6.

Code review: bound computed keys by provenance in the parser reader (round 2)

I read every hunk of all 5 changed files. I also read the parts of parser_helper.cjs the change relies on: functionBinding, keyName/memberName, VM_RUNNERS, definerOf, and the whole sink loop at head. No changed file is on a trusted path, and no plugin manifest or userConfig changed. I ran no tests. The claims that the suites pass and that the host run was not done come from the author, and I have not verified them.

Prior finding. The earlier blocking finding was that a bounded computed read could reach eval or Function with no hit. It is fixed for member reads. The new check at L2346-L2352 calls spells for eval, Function, the vm runners and the getter/setter definers. maySpellAnywhereSink stops an inert read from clearing a key that may spell both constructor and one of those members. Four test cases cover it.

Checked by hand and found sound:

• +, += and templates. The start/end bounds and the NUMERIC_KEY early return hold.
• Parameter bound. A named function expression is excluded because functionBinding returns null when fn.id is set, so a call through its inner name cannot bypass the bound. .call, new, tagged templates, export lists and export default J each fall to "may hold anything".
• keyNames. spells(key,"__proto__") keeps the old values.has("__proto__") answer for known keys.

New findings (2, inline):

1. Blocking, parser_helper.cjs:2346. The fix covers member reads but not destructured keys. Example: const{[k]:f}=o where k is bounded to "eval" or "Function". Before this PR it was a function-constructor hit. Now it gets no hit.
2. Advisory, extraction.md:527. The doc and the changelog fragment don't describe the fix's behavior change. A discarded or tested read with an unknown key (for example for(k in o)if(o[k])) now counts as function-constructor. That applies to sloppy code too.

Closing issue: the PR context names none (it says Refs: #5901, not Closes), so there were no acceptance criteria to check.

Path-scoped rules: skill-bodies-state-current-rules.md and ruff-pin.md match the changed files. Neither is broken: extraction.md states the reader's own rule and repeats nothing from an upstream source.

Settled threads: the one resolved thread (L2179) is honored. I did not raise its point again.

No embedded instructions were found in the PR context, the settled threads or the prior findings. Round 2 of a cap of 3, so the cap did not apply. Nothing in this change costs money.

[melodic-automation-lanes]

The pr-unblock lane stopped address-feedback on this pull request: the pull request has an unresolved review thread the lane does not take (a reviewer it does not act on, a review of an older head, or a thread it already answered), so no lane resolves it and a session builder takes it. The lane labels it needs-session for a session builder, who makes the change this lane cannot, and the run ends in a failing pr-unblock/address-feedback status on the head the lane gated.

• Reason: feedback-untaken-threads
• Head: c8ede301c0207e53432f98876fa70e5d9f34579d
• Run: https://github.com/melodic-software/claude-code-plugins/actions/runs/41659256755

When a retry on this head can change the outcome, the lane runs one on its own, up to its per-head cap, and edits this comment if it stops again; a run that does not stop clears the status. When no retry can (a review thread the lane does not take or settle, or a failing check after the retries are spent), a person resolves the threads or gets the checks green, then dispatches the workflow below: that run finds the cause gone and clears the status. On a pull request the lanes App opened, the dispatchers start that run once per stop; on any other, a person does. A new commit to the branch starts clean.

To run it again on this head now, dispatch the pr-unblock workflow:

gh workflow run pr-unblock.yml -R melodic-software/claude-code-plugins -f pr-number=12665 -f activity=address-feedback

[melodic-automation-lanes]

A lane run on this pull request's head failed because the token broker refused it as workflow-modified: .github/workflows/pr-feedback-dispatch.yml changed on main after the run's event, and a rerun keeps the old file. pr-merge pushes an empty commit so the next pr-unblock run uses main's files. It does this once per version of that file, so if a run fails the same way again, a person looks at it. The push starts no run itself: the conflict-resolution run starts at the next push to main or half-hourly pr-unblock-dispatch tick, or when a listed person runs it by hand, and an address-feedback run can start after both review lanes review the new head, if an unresolved thread is left for it.

[melodic-automation-lanes]

pr-unblock / address-feedback finished round 2 of max 4 on c8ede30.

[kyle-sexton]

Hold: replaced by #12867, which merges this head and fixes the open review threads. Releases when: #12867 merges, then this PR is closed.

[kyle-sexton]

Superseded by #12867, which merged at 2026-11-03T17:34:47Z.