feat(harness-ops): follow whole-module loads and computed keys in the parser reader's flow check by kyle-sexton · Pull Request #5970 · melodic-software/claude-code-plugins

kyle-sexton · 2026-10-03T01:14:19Z

This PR leaves the issue open: the default reader is not flipped.

Summary

#5901 asked for the parser reader to read the Explore and Plan disallowed_tools literal on 2.1.284-2.1.288 and then become the inventory default. Two causes kept both lists partial: whole-module (namespace) loads of the re-exporting chunk, and the sink rule. This PR removes the first and narrows the second. Under the brief's stop rule it does not flip the default: the sink rule still fires on every build, and some of what it fires on cannot be cleared without assuming something the analysis cannot prove (details under Fix).

Namespace loads are followed (cause 1 removed). The helper's new namespace op follows each import(), require(), import.meta.require() and import*as load of the exporting file to its reads. It accepts only reads of other exports by name: a member read that is not a call, an object pattern without rest, a {names:ns} record read only by name or tested, and await Promise.all([...]) destructured by an array pattern. The built-ins those shapes rely on join the trusted names the sink rule watches. A global write to a trusted name is now a sink. A namespace settled through a promise needs its module to export no then. On every installed build, all 13-14 loads of the re-exporting chunk pass.
node:vm counts as code built from a string. Any vm/node:vm load except an import naming only isContext is a sink like eval and Function, and so is a vm runner's name (runInThisContext, runInNewContext, runInContext, compileFunction, SourceTextModule, SyntheticModule) read from any object. Code in a new context still reaches this realm's prototypes through this.constructor.constructor.
A load the parser cannot name fails closed (verifier gap 1, also on main). Some loads could pull in the exporting file whole without the parser seeing which file: an aliased require or import.meta.require, .call, a comma callee, or import(x) with no literal specifier. The loads op now reports any module that holds one, and that module fails every export hop.
The sink rule reads computed keys (cause 2 narrowed). A computed-key write or define counts only for the trusted names its key can spell when every value of the key is known: literals, numbers, boolean or typeof results, or variables written only with those.
harness-ops 3.0.1 -> 3.1.0 (minor). --reader stays regex; README, SKILL.md and native-drift.md are unchanged because the default did not change.

Fix

What still blocks a literal read, measured on 2.1.288 with the flow's own trusted names (some, includes, has). Module counts are lower bounds, because each module reports at most 20 hits:

Sink kind	Modules	Why it cannot be cleared here
computed-key write on a target not shown fresh	178 (201 before key provenance)	Receivers are parameters, `this` in methods, call results; clearing them needs interprocedural points-to over the 40 MB bundle
computed-key define (`Object.defineProperty(o,k,...)`)	92	Same
prototype swap (`__proto__=`, `setPrototypeOf`)	22	Same
definer read other than as a callee (`Hn=Object.defineProperty.bind(Object)`)	11	Aliased definers can be called with any key
node:vm load or runner	5	The workflow runtime, plugin loader and a test kit run code from strings through vm
(namespace stage) a load the parser cannot name	3	Each could load the re-exporting chunk whole unseen
code built from a string	8	ajv runs validator code it generates at runtime (`Function(self,scope,code)(...)`), protobufjs's `inquire` makes a direct `eval` call, and 3 modules call a member `.eval(...)` whose receiver cannot be shown to be something other than the global object. Clearing ajv's case would mean assuming generated code never patches a built-in prototype. That assumption is unsound

The namespace stage needed no unsound assumption. The sink stage does, for ajv at least.

Verification

INVENTORY_REQUIRE_ACORN=1 full harness-ops suite: all 13 test_*.py modules (python3 -m unittest per directory) and 39 of the 40 *.test.sh pass. audit-install-state/scripts/install_state.test.sh fails here and on origin/main the same way: under Python 3.14, python -m unittest <absolute path>.py fails to import the module. This PR does not touch that skill, and test_install_state.py passes when run as a module.
New tests in test_reader_findings.py:
- test_a_namespace_read_only_by_name_keeps_the_literal (7 shapes).
- test_a_namespace_read_other_than_by_name_stays_partial (adversarial, one or more per acceptance rule).
- test_what_a_namespace_load_trusts_stays_checked (a replaced Promise, Promise.resolve, then, Promise.prototype.constructor, an Object.prototype getter, a then export, an export*).
- test_a_computed_key_known_to_name_no_trusted_name_clears and test_a_computed_key_that_may_name_a_trusted_name_stays_a_sink.
- test_a_bundle_calling_vm_run_in_this_context_stays_partial covers 11 spellings: runInThisContext, runInNewContext (the verifier's probe), runInContext, Script#runInNewContext, SourceTextModule, a destructured compileFunction, a computed read and an escaping alias. A plain named import of isContext stays literal.
- test_a_load_the_parser_cannot_name_stays_partial covers the verifier's gap-1 probes, which read a wrong literal before this change: an aliased import.meta.require, require.call, (0,require), import.meta.require.call, import(s) and require(s). As controls, typeof require, require.resolve, a require parameter and a {require:1} key stay literal.
At f2b395c, after both gap fixes:
- INVENTORY_REQUIRE_ACORN=1 unittest passes for all 13 harness-ops Python modules.
- scripts/run-ruff.sh check and format --check are clean.
- --reader compare --self-check on 2.1.288 prints OK, with reader compare: ok, 2167 of 2167 modules parse.
- --reader compare --self-check on 2.1.284 prints reader compare: ok, 2151 of 2151 modules parse. The overall verdict is DEGRADED, the same as on main: there is a version advisory, and the builtin_plugins canaries are absent on that build.
- The {names:...} case in test_the_module_table_names_the_exporters_own_file now reads literal when the record is never read, and partial with a computed read.
python3 inventory.py --binary <build> --reader compare --binary-only on this branch (rerun at f2b395c) and on origin/main 57ca27a:

Build	Explore literal	Plan literal	wrong->unresolved	Parser output vs main
2.1.284	no (partial)	no (partial)	4	identical
2.1.285	no (partial)	no (partial)	4	identical
2.1.286	no (partial)	no (partial)	4	identical
2.1.287	no (partial)	no (partial)	4	identical
2.1.288	no (partial)	no (partial)	4	identical

The 4 wrong->unresolved entries are the Explore and Plan disallowed_tools and disallowed_tools_source on every build, the same as on main.

scripts/run-ruff.sh check and format --check are clean; markdownlint-cli2 is clean on the changed docs; scripts/check-changelog-parity.sh --check-bump origin/main passes.

claude-ops inventory: replace the regex bundle reader with a JavaScript parser #5640 (parser reader plan and the decisions behind it)
feat(harness-ops): follow bindings across modules and guard builtin patches in the parser reader #5891 (P4: cross-module flow and the sink rule)
chore(harness-ops): validate the native-surface registry against Claude Code 2.1.288 #5957 (2.1.288 validation that recorded the partial Explore/Plan lists)

Known gaps, unchanged by this PR:

A spread's Symbol.iterator and the array iterator's next are not trusted names, so the sink rule does not watch them for a spread.
require and import() are recognized by name only. A shadowing local binding, or a loader reached through another global such as module.require or a createRequire result, is caught only when it is aliased or called with a non-literal.

🤖 Generated with Claude Code

…their reads The parser reader failed an export hop whenever any code loaded the exporting file whole (import(), require(), import*as, export*). It now follows each such load in the new `namespace` helper op and accepts only reads of other exports by name: a member read that is not a call, an object pattern without rest, a record property (`{names:ns}`) bound to a variable read only by name, and Promise.all destructuring. Promise.all, await and record lookups add the built-ins they rely on to the trusted names the sink rule checks, a global write to a trusted name is now a sink, and a namespace settled through a promise requires the module to export no `then`. Load sites now come from the AST (`loads` op) instead of a regex over raw text. On 2.1.284-2.1.288 every load of the Explore/Plan array's re-exporting chunk passes; the lists still read partial because of the sink rule. Refs #5901 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…trusted name The sink rule counted every computed-key write or define on an object that is not provably fresh as able to write any trusted built-in name. It now works out what the key can evaluate to: literals, numbers (arithmetic, `++`, unary minus), boolean and typeof results, and variables every write of which is one of those. Such a key counts only for the trusted names it spells; any other key still counts for all. On 2.1.288 this clears about a fifth of the computed-key writes (1136 hits in 201 modules down to 877 in 178); the rest have receivers and keys only an interprocedural analysis could pin down. Refs #5901 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…mp to 3.1.0 - A member read through an optional chain, `(ns?.f)()`, still passes `ns` as `this`, so the namespace and record walks refuse it as a method call. - `exports()` answers None for a module with `export*`, whose names it cannot list, so a `then` re-exported through it is not missed. - extraction.md describes the namespace walk, key provenance and the sinks that remain on 2.1.284-2.1.288, which keep the Explore and Plan lists partial under the parser and the default reader at regex. - harness-ops 3.0.1 -> 3.1.0 with a CHANGELOG entry. Refs #5901 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

github-actions · 2026-10-03T01:17:18Z

PR body contract — issue linkage

This PR body conforms to the issue-linkage contract. Nothing to do.

…rom a string `runInThisContext` (on vm or a vm.Script) and `compileFunction` run a string as code in this realm, where it could patch a built-in the flow trusts, but the sink rule did not see them. A read or destructured key of either from any object, a named import of either, and a `vm` or `node:vm` load used other than by named reads are now sinks like eval and Function. On 2.1.288 five modules hit it; `--reader compare` output is unchanged on 2.1.284-2.1.288. Refs #5901 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…r cannot name Two wrong-value gaps the #5970 verifier found, both also on main: - Code run in a new vm context still reaches this realm (`this.constructor.constructor("...")()` inside `runInNewContext` patches the outer Array.prototype). Any `vm`/`node:vm` load except an import naming only `isContext`, and any vm runner's name read from any object, is now a sink. - A load the AST scan did not recognize (an aliased `require` or `import.meta.require`, `require.call`, `(0,require)(...)`, `import(x)` with a non-literal specifier) could load the exporting file whole and enumerate it unseen. The `loads` op now reports such a module, and it fails every export hop. `--reader compare` output is unchanged on 2.1.284-2.1.288. Refs #5901 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

claude · 2026-10-03T01:44:39Z

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

Security review (`/review:security-review`)

Read trigger comment and repo instructions
Invoke /review:security-review and follow its instructions
Post findings via this comment

Scope: Reviewed only the files changed in this PR (git diff origin/main...HEAD), 8 files, against
f2b395c.

Skip-gate check: PR is open, has a security-relevant-looking surface (a parser/static-analysis
engine), and has no prior passing review on this head, so a full pass was done rather than skipping.

What this PR does, security-wise: It extends parser_helper.cjs (a long-lived Node subprocess
driven by parser_reader.py over a JSON-lines stdin/stdout protocol) with new static-analysis ops
(loads, namespace) and narrows the existing sinks op, all used to read disallowed_tools
literals out of compiled Claude Code builds for an advisory inventory report. Per the PR
description, the default reader is intentionally not flipped by this change — the analysis stays
opt-in (--reader parser), so nothing here changes an enforced security boundary.

Findings: none.

Specifically checked for, and did not find:

Code execution of analyzed content. The helper only ever parses the bundle source with
acorn.parse (parser_helper.cjs#L1722);
there is no eval, new Function, vm.*, child_process, or shell invocation anywhere in the
diff that would run the (potentially adversarial) bundle code being inspected — including in the
new vm/node:vm sink detector itself, which only pattern-matches AST nodes, it never loads
node:vm.
Injection surfaces. No new subprocess/shell/os.system calls were added in
inventory.py/parser_reader.py; the existing subprocess.Popen plumbing for the long-lived
helper process is untouched by this diff. Communication with the helper stays JSON-over-stdio, so
there's no command-string construction to inject into.
ReDoS / ad-hoc regex risk. The diff actually removes a hand-rolled regex
(_NAMESPACE_SITE_RE in inventory.py) in favor of walking the AST via the new loads/namespace
ops, which reduces rather than adds this class of risk.
Fail-open behavior. Every new gap-closing path is conservative/fail-closed by design: an
unnamed load (loads reporting "*"), an unrecognized namespace-read shape, a computed key whose
value can't be fully enumerated, or a vm/node:vm import other than a bare isContext import
all cause the read to stay "partial"/count as a sink rather than silently being treated as safe.
Secrets/credentials handling. No tokens, credentials, or secrets are read, logged, or
persisted anywhere in this diff; it only moves JS source/offsets between the Python driver and the
Node helper.
Instruction-surface deletions. extraction.md's changes narrow/update factual claims about
what the parser can currently prove (e.g. replacing "nothing rules out the namespace loads and
sinks below" with an updated claim reflecting the new loads/namespace ops) rather than deleting
or softening a standing guardrail, hook allow/deny list, or enforcement rule, so the
instruction exception register
lens doesn't surface anything here. (The register file itself wasn't fetchable with this skill's
tools, so this is recognition-by-consequence as the skill instructs, not a consulted read of that
file.)
GitHub Actions / workflow changes. None — no .github/workflows files are touched by this PR,
so zizmor's lane is not applicable here.

No CRITICAL, IMPORTANT, or SUGGESTION findings to report.

claude · 2026-10-03T01:44:43Z

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

Code review (`/review:code-review`)

Gather context (PR body, diff, custom instructions)
Run /review:code-review against this PR per custom instructions
Post inline comments for anchorable findings
Post summary comment

Scope: full diff origin/main...HEAD (8 files, +958/-113), focused on the correctness-critical core: parser_helper.cjs's new Namespace walk and computed-key provenance in sinks(), and the Python wiring in _namespace_holds/_flow_holds (inventory.py) and the new loads/namespace/modules_named reader methods (parser_reader.py).

Findings: none that meet the bar to block or flag.

What I traced through in detail, with concrete inputs, looking for a case where the walk would unsoundly call something "safe" when it isn't:

Namespace.value/byName/readOnly/pattern/record/promiseAll (parser_helper.cjs:132-313): every branch not explicitly matched falls through to throw new Unresolved, so an unhandled shape (loose ==, a spread, an enumeration, a second write to the record's binding, a method call passing the namespace/record as this) fails closed rather than silently passing. Checked this against the new adversarial tests in test_reader_findings.py and traced the control flow by hand for several shapes not in the test list (e.g. ns == x, globalThis.Promise = f); all fail closed or are correctly caught as sinks.
keyValues/keyNames (parser_helper.cjs:432-498): cycles through a variable (a=b;b=a) terminate via seenKeys, union-with-null correctly propagates "unknown", and the for-in/for-of exclusion in the Identifier case (isLoop, which only matches ForInStatement/ForOfStatement) still accepts a C-style for(let i=0;...;i++) loop counter as called out in the extraction.md prose.
_namespace_holds (inventory.py:2344-2389): matches loaded files to the exporting module by basename, same heuristic the pre-existing _namespace_targets used; confirmed this can only make the result more conservative (never unsoundly "safe") when two files share a basename, since the namespace op's rejection rule (no read of the followed name) is checked on the loader's own code regardless of which actual file a shared basename resolves to.
exports()'s new star handling and the thenable/then-export check wiring between parser_helper.cjs and inventory.py line up correctly (verified the star field is always present on the ok path in exportsOf, and the at + lo offset math in parser_reader.py's new namespace() method matches the existing pattern in neighboring methods).
The vm/node:vm sink additions and the for (const arg of node.arguments) ... hit("argument", ...) loop that also happens to catch trusted-name string arguments generically: confirmed that loop predates this PR (unchanged on origin/main), so it's out of scope here.
Version bump (3.0.1 → 3.1.0), CHANGELOG entry, and extraction.md prose all line up with what the code actually does, including the explicit claim that --reader default stays regex (confirmed in inventory.py:5912, unchanged).

One cosmetic nit, below the bar to flag on its own: the new docstring for _namespace_holds in inventory.py has a stray repeated word — "A promise settled with the namespace reads its then export, so then each file's module must be known..." (

claude-code-plugins/plugins/harness-ops/skills/inventory/scripts/inventory.py

Lines 2357 to 2358 in f2b395c

    
               A promise settled 
        
               with the namespace reads its `then` export, so then each file's module

). Not worth a commit on its own.

Not independently verified (author-claimed only): the test suite run, ruff/markdownlint output, and the --reader compare numbers in the PR body — the sandbox denied shell execution for this review, so I read the diff and the surrounding source statically rather than re-running the suite.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f2b395cf89

ℹ️ 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 · 2026-10-03T01:51:31Z

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-03T03:32:36.655952Z	`67f5fd1`	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.

…eta as an unknown load Codex on #5970: `const{require:r}=import.meta;r("/a.js")` hid the load, so a namespace enumerated through `r` read a wrong literal. `import.meta` used other than by a named read, and any destructured `require` key, now count as a load the parser cannot name, failing every export hop. `--reader compare` output is unchanged on 2.1.284-2.1.288. Refs #5901 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

claude · 2026-10-03T01:58:59Z

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

Code review (`/review:code-review`)

Gather context (PR body, diff, custom instructions)
Run /review:code-review against this PR per custom instructions
Post inline comments for anchorable findings
Post summary comment

Scope: full diff origin/main...HEAD (8 files, +967/-113) at
9898961,
with focused attention on the newest commit (9898961, +10/-1 over the already-reviewed
f2b395c) that closes the Codex P1 finding about destructured/aliased import.meta.require.

1 finding, posted inline on
parser_helper.cjs:1642:

The new unknownLoad branch that flags a destructured require:-named key
(node.type === "Property" && parent?.type === "ObjectPattern") resolves the key with the plain
keyName helper, which only handles a literal or an expression-free template literal and returns
null — i.e. "not require" — for any other computed-key expression. A key computed from a
variable or conditional (const {[k]: r} = ns where k can evaluate to "require") therefore
fails open: no entry is added to loads, so _namespace_holds can approve a namespace read
through that module even though it may resolve to require at runtime. This is the same bug
class this commit fixes for import.meta and literal require: keys, just reached through a
dynamic key. The PR already built exactly the provenance tracking needed to close this
(keyValues/keyNames, parser_helper.cjs:1363-1433, used by the sinks rule to fail closed on
an unresolvable computed key) — reusing it here instead of keyName would make the destructuring
check consistent with the rest of the PR's fail-closed design. No test in
test_reader_findings.py covers a non-literal computed destructured key, so the new adversarial
tests (lines 692-695) didn't catch it.

Checked and found sound:

The 9898961 fix itself for the two cases it targets: a direct const m = import.meta /
{require:r} = import.meta destructure (caught via the isImportMeta branch regardless of key
spelling, since it fires whenever import.meta isn't read by a named member access) and a
literal {require: r} = <anything> destructure (caught via the new ObjectPattern branch).
Traced both through scanOf's walk by hand against the new test cases at
test_reader_findings.py:692-695.
The docstring typo flagged in the prior round's code-review comment
(inventory.py — repeated "then") is fixed in this commit.
No other logic changed since the prior full review at f2b395c (confirmed via
git diff f2b395c..9898961, which touches only inventory.py (comment), parser_helper.cjs
(the unknownLoad branches above), and the new test cases).

Not independently re-verified (author-claimed only, per the PR body): the test suite run,
ruff/markdownlint output, and the --reader compare numbers — this review was a static read of
the diff, not a re-run of the suite.

claude · 2026-10-03T01:59:00Z

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

Security review (`/review:security-review`)

Read trigger comment and repo instructions
Invoke /review:security-review and follow its instructions
Post findings via this comment

Scope: PR is open; head is 9898961 (full diff origin/main...HEAD, 8 files, +967/-113).
The prior security review on this PR covered f2b395c and found nothing; the only change since
is the latest commit, 9898961 ("fix(harness-ops): count a destructured require or an aliased
import.meta as an unknown load"), which fixes exactly the P1 Codex finding from the f2b395c
review round (destructured/aliased import.meta.require escaping the loads analysis). This
review re-verified the full diff's security-relevant surface and focused verification effort on
that delta.

Skip-gate check: not skipped — the head changed since the last passing review, so a fresh
pass was done rather than reusing it.

What changed in the delta (f2b395c → 9898961):
parser_helper.cjs#L1636-L1657's
unknownLoad() gains two branches, plus a matching test addition and a docstring typo fix
(inventory.py, cosmetic only, no security effect).

Findings: none.

I traced the new logic by hand against the Codex P1 scenario and adjacent shapes to check it
fails closed rather than silently approving an export it shouldn't:

isImportMeta(node) branch:
import.meta read any way other than a named member (import.meta.foo) now counts as an
unknown load. const m = import.meta, {require:r} = import.meta, foo(import.meta), and a
computed read with an unspellable key (import.meta[x]) are all caught here, because the
import.meta node's own parent isn't a MemberExpression reading a named key. Benign reads
like import.meta.url still fall through unflagged (memberName(parent) is non-null).
Property/ObjectPattern branch:
a generic catch-all — any destructuring with a literal key named require, regardless of the
source expression — covers the sibling case the new tests also added,
const {require: r} = globalThis, which isn't import.meta at all and so isn't reached by the
first branch.
Both branches only ever add to the unknownLoad/sink side (fail closed); neither can turn a
previously-flagged load into one that's silently trusted, so this is a strict tightening with no
new soundness gap. The object-literal control case from the PR body ({require:1} as a value,
not a pattern) is unaffected since that's an ObjectExpression, not an ObjectPattern.
Confirmed against the existing direct-call path (import.meta.require("/a.js") with a literal
arg) that recognition of a legitimately-named, literal-specifier load is unchanged — the new
branches only touch the escape/aliasing shapes, not the direct-call shape already handled lower
in the function.
Same prior-review conclusions still hold for the unchanged 7 files: no code execution of
analyzed bundle content (still only acorn.parse, no eval/vm/child_process added), no new
injection surface, no secrets handling, and the fail-closed design pattern ("*"/sink on any
unrecognized shape) is preserved.

Not independently verified: I could not execute the test suite or reproduce the
--reader compare run in this sandbox — Bash tool calls here require interactive approval that
wasn't available — so the test_a_load_the_parser_cannot_name_stays_partial additions and the
author's rerun numbers are author-claimed only, verified by static trace rather than execution.

No CRITICAL, IMPORTANT, or SUGGESTION findings to report.

kyle-sexton · 2026-10-03T02:33:37Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 989896145d

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

# Conflicts: # plugins/harness-ops/CHANGELOG.md

kyle-sexton · 2026-10-03T02:45:45Z

@codex review

claude · 2026-10-03T02:46:04Z

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

Security review (`/review:security-review`)

Read trigger comment and repo instructions
Invoke /review:security-review and follow its instructions
Post findings via this comment

Skip gate: review skipped, prior review still applies.

Current head is
5bada6b,
a merge of origin/main into this branch. I diffed 9898961 (the commit the last successful
security review on this PR covered, comment)
against 5bada6b and confirmed every file under
plugins/harness-ops/skills/inventory/ — parser_helper.cjs, inventory.py, parser_reader.py,
test_parser_reader.py, test_reader_findings.py, extraction.md — is byte-identical between
the two commits (git diff --stat 9898961 5bada6b -- plugins/harness-ops/skills/inventory/
produces no output). The only changes are a version bump (3.1.0 → 3.2.0) and a CHANGELOG.md
reshuffle, both artifacts of merging in unrelated origin/main history (d2724ac, the
prerequisites feature), not new changes to this PR's own diff.

Per the skip gate ("this head already has a successful security review that still applies"), no
new review is needed: the 9898961 review's findings (none; see
comment)
still hold for 5bada6b.

claude · 2026-10-03T02:46:08Z

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

Code review (`/review:code-review`)

Gather context (PR body, diff, custom instructions)
Run /review:code-review against this PR per custom instructions
Post inline comments for anchorable findings
Post summary comment

Skip gate: applies (point 4 — this head already has a successful review from this lane that still applies).

Checked git diff 9898961..5bada6befb6 scoped to plugins/harness-ops/skills/inventory/scripts/ (and the rest of the changed-files list): it is empty. The merge commit at this PR's current head,
5bada6b,
only folds in unrelated origin/main history (d2724ac, the prerequisites checker) — which lands
identically on both sides of origin/main...HEAD, so it doesn't appear in the PR's diff — and a
mechanical version-bump renumber (3.1.0 → 3.2.0) to avoid colliding with main's own 3.1.0
bump. None of that touches inventory.py, parser_helper.cjs, parser_reader.py, or either test
file. The diff this lane reviews (origin/main...HEAD, 8 files, +967/-113) is byte-for-byte the
same diff already reviewed on this head's immediate predecessor commits.

This lane's code-review already ran twice on that unchanged content:

At f2b395c (full pass, no findings).
At 9898961 (delta pass over the gap-1 fix commit), which raised one finding: a computed/non-literal destructured key (const {[k]: r} = ns) reaching unknownLoad() at
parser_helper.cjs:1642
without being routed through the keyValues/keyNames provenance tracking, so it fails open instead of closed.

That finding was independently raised by Codex at the same line and already triaged by the PR author as VALID (defer) — confirmed synthetic/no observed --reader compare effect on 2.1.284-2.1.288, with the fix plan tracked on
#5640
rather than fixed in this PR, per this PR's two-round review cap. No new finding to add on top of it, and nothing in this unchanged diff warrants revisiting that triage.

No new inline comments posted (nothing new to anchor — the one open finding is already inline on this diff from the prior round and already triaged).
· branch feat/5901-parser-default

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5bada6befb

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

# Conflicts: # plugins/harness-ops/.claude-plugin/plugin.json # plugins/harness-ops/CHANGELOG.md

kyle-sexton · 2026-10-03T03:27:59Z

@codex review

claude · 2026-10-03T03:28:19Z

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

Security review (`/review:security-review`)

Read trigger comment and repo instructions
Invoke /review:security-review and follow its instructions
Post findings via this comment

Skip gate: review skipped, prior review still applies.

Current head is
67f5fd1,
a merge of origin/main into this branch. I compared the PR's own diff (origin/main...HEAD,
scoped to all 8 changed files) at this head against the diff at
989896145d
— the last head with a full, successful security-review pass that found no findings — and at
5bada6b,
which this lane already confirmed was byte-identical to 989896145d on every reviewed source
file.

git diff --stat origin/main...67f5fd1 reports the same 8 files, +967/-113, exactly matching
the PR description's own verification numbers. A direct git diff 989896145d 67f5fd1 (scoped to
plugins/harness-ops/) shows only:

plugins/harness-ops/skills/inventory/scripts/parser_reader.py and
test_parser_reader.py: the _ps_q curly-single-quote PowerShell-escaping fix and its tests.
Confirmed this is commit
c8fa858
(PR fix(harness-ops,miro): escape every PowerShell single-quote character in the repair command #5972, already merged to origin/main — git merge-base --is-ancestor c8fa858 origin/main
succeeds), pulled in by the merge. It doesn't appear in the origin/main...HEAD three-dot diff
because it sits on both sides equally.
plugins/harness-ops/.claude-plugin/plugin.json and CHANGELOG.md: a version renumber
(3.1.0 → 3.2.0) and a CHANGELOG reshuffle to avoid colliding with the version numbers
origin/main had already taken for unrelated releases (3.0.2, 3.1.0, 3.1.1) during the
same merge. No new prose or behavior claim.

None of inventory.py, parser_helper.cjs, parser_reader.py's own PR-authored logic,
test_reader_findings.py, or extraction.md changed at all between 989896145d and 67f5fd1.

Per the skip gate ("this head already has a successful security review that still applies"), no
new review was needed: the 989896145d review's findings — none, see
comment
— and the 5bada6b skip-gate confirmation still hold for 67f5fd1.
· branch feat/5901-parser-default

claude · 2026-10-03T03:28:20Z

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

Code review (`/review:code-review`)

Gather context (PR body, diff, custom instructions)
Run /review:code-review against this PR per custom instructions
Post inline comments for anchorable findings
Post summary comment

Skip gate: applies (point 4 — this head already has a successful review from this lane that still applies).

Current head is 67f5fd1, a merge of origin/main into this branch. I checked git diff origin/main...HEAD --stat against the PR's own scoped files and confirmed:

merge-base(origin/main, HEAD) is c8fa858 — i.e. c8fa858 (fix(harness-ops,miro): escape every PowerShell single-quote character in the repair command (#5972)) is already on origin/main, not new PR content.
git diff 5bada6b HEAD is empty for inventory.py, parser_helper.cjs, test_reader_findings.py, and extraction.md — the core analysis files this PR's logic lives in are byte-identical to the commit the last successful review on this head (5bada6b) already covered.
The only two-dot diff since 5bada6b is in parser_reader.py/test_parser_reader.py, and that content is entirely the PowerShell-quoting fix from c8fa858 (already merged to main via fix(harness-ops,miro): escape every PowerShell single-quote character in the repair command #5972) riding along in the merge — not new work on this PR's branch. The three-dot diff origin/main...HEAD for those two files (62 insertions / 2 deletions total) matches the PR's own changed_files table exactly (parser_reader.py +60/-2, test_parser_reader.py +2/-0), so this PR's actual diff against origin/main is unchanged.

This lane's code review already ran on this unchanged diff twice — at
f2b395c
and 9898961,
most recently re-confirmed unchanged at 5bada6b — with no new findings beyond the one already-triaged Codex finding (a computed/destructured-key require read escaping unknownLoad's literal-only keyName check, triaged VALID (defer), tracked on
#5640).
No new inline comments to post — nothing in this head's diff differs from what was already reviewed.
· branch feat/5901-parser-default

chatgpt-codex-connector · 2026-10-03T03:32:34Z

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

Reviewed commit: 67f5fd1286

ℹ️ 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 and others added 4 commits October 2, 2026 20:42

Merge remote-tracking branch 'origin/main' into feat/5901-parser-default

b8fe2e0

github-actions Bot added the needs-issue-linkage label Oct 3, 2026

kyle-sexton mentioned this pull request Oct 3, 2026

feat(harness-ops): read Explore/Plan disallowed_tools literal under the parser reader, then flip the inventory default #5901

Open

kyle-sexton marked this pull request as ready for review October 3, 2026 01:44

chatgpt-codex-connector Bot reviewed Oct 3, 2026

View reviewed changes

Comment thread plugins/harness-ops/skills/inventory/scripts/js/parser_helper.cjs

claude Bot reviewed Oct 3, 2026

View reviewed changes

Comment thread plugins/harness-ops/skills/inventory/scripts/js/parser_helper.cjs

chatgpt-codex-connector Bot reviewed Oct 3, 2026

View reviewed changes

Comment thread plugins/harness-ops/skills/inventory/scripts/js/parser_helper.cjs

kyle-sexton mentioned this pull request Oct 3, 2026

claude-ops inventory: replace the regex bundle reader with a JavaScript parser #5640

Open

Merge remote-tracking branch 'origin/main' into feat/5901-parser-default

5bada6b

# Conflicts: # plugins/harness-ops/CHANGELOG.md

chatgpt-codex-connector Bot reviewed Oct 3, 2026

View reviewed changes

Comment thread plugins/harness-ops/skills/inventory/scripts/js/parser_helper.cjs

github-actions Bot removed the needs-issue-linkage label Oct 3, 2026

Merge remote-tracking branch 'origin/main' into feat/5901-parser-default

67f5fd1

# Conflicts: # plugins/harness-ops/.claude-plugin/plugin.json # plugins/harness-ops/CHANGELOG.md

kyle-sexton merged commit c653ab5 into main Oct 3, 2026
18 checks passed

kyle-sexton deleted the feat/5901-parser-default branch October 3, 2026 03:33

This was referenced Oct 3, 2026

feat: adopt GitHub async merge, merge queue, stacked PRs and the new App token format #5935

Merged

fix: clear claude.ai marketplace sync warnings and gate them in the validator #5902

Merged

Conversation

kyle-sexton commented Oct 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Fix

Verification

Related

Uh oh!

github-actions Bot commented Oct 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude Bot commented Oct 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Security review (/review:security-review)

Uh oh!

claude Bot commented Oct 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code review (/review:code-review)

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

chatgpt-codex-connector Bot commented Oct 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

claude Bot commented Oct 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code review (/review:code-review)

Uh oh!

claude Bot commented Oct 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Security review (/review:security-review)

Uh oh!

Uh oh!

kyle-sexton commented Oct 3, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

kyle-sexton commented Oct 3, 2026

Uh oh!

claude Bot commented Oct 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Security review (/review:security-review)

Uh oh!

claude Bot commented Oct 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code review (/review:code-review)

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

kyle-sexton commented Oct 3, 2026

Uh oh!

claude Bot commented Oct 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Security review (/review:security-review)

Uh oh!

claude Bot commented Oct 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code review (/review:code-review)

Uh oh!

chatgpt-codex-connector Bot commented Oct 3, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

kyle-sexton commented Oct 3, 2026 •

edited

Loading

github-actions Bot commented Oct 3, 2026 •

edited

Loading

claude Bot commented Oct 3, 2026 •

edited

Loading

Security review (`/review:security-review`)

claude Bot commented Oct 3, 2026 •

edited

Loading

Code review (`/review:code-review`)

chatgpt-codex-connector Bot commented Oct 3, 2026 •

edited

Loading

claude Bot commented Oct 3, 2026 •

edited

Loading

Code review (`/review:code-review`)

claude Bot commented Oct 3, 2026 •

edited

Loading

Security review (`/review:security-review`)

claude Bot commented Oct 3, 2026 •

edited

Loading

Security review (`/review:security-review`)

claude Bot commented Oct 3, 2026 •

edited

Loading

Code review (`/review:code-review`)

claude Bot commented Oct 3, 2026 •

edited

Loading

Security review (`/review:security-review`)

claude Bot commented Oct 3, 2026 •

edited

Loading

Code review (`/review:code-review`)