Skip to content

fix(claude-config): discharge nine stranded review findings in audit-instructions and setup - #2003

Merged
kyle-sexton merged 11 commits into
mainfrom
stranded/claude-config-batch
Aug 9, 2026
Merged

fix(claude-config): discharge nine stranded review findings in audit-instructions and setup#2003
kyle-sexton merged 11 commits into
mainfrom
stranded/claude-config-batch

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Summary

Discharges nine verifier-confirmed review findings against claude-config's audit-instructions
and setup skills, and bumps the plugin to 0.21.10 with a changelog entry.

Seven of the nine are instruction-surface defects in audit-instructions — a skill that audits other
people's instruction surfaces and was violating its own rules on three of them. One is a scanner
false negative reproduced and fixed with tests. One is a prerequisite the docs understated by two
skills.

What changed, per finding

# Complaint Discharge
1 The skill forbids hardcoding ~/.claude, then hardcodes it SKILL.md:45, :75, :197, :219-222 now resolve against the user root Phase A establishes
2 I3 rejects @path imports as non-deferring, then names a skills: preload as a valid destination criteria.md:196-201 strikes the preload; only conditional runtime invocation qualifies
3 A subagent's own memory is graded real but never inventoried Inventory bullet at SKILL.md:216-229; co-residency row at conflict-criteria.md:98
4 The liveness gate resolved a closed five-input list with no hook enablement SKILL.md:175-185 and conflict-criteria.md:270-292 resolve disableAllHooks per scope plus allowManagedHooksOnly
5 A nested project memory pair routed to a check that cannot discover the file SKILL.md:413-417 narrows the boundary to root-level project; .claude/rules/** deliberately unchanged
6 BOUNDARY_ERE omits and, dropping a real conflict COORD_ERE / COORD_HEAD_ERE at conflict-scan.sh:131-143, three new test cases
7 The requirements list names only jq/curl README.md:163-177 and setup/SKILL.md:28-33,46-52,106-108 name awk/sort across all three skills that use them
8 I14's startup set omits ./.claude/CLAUDE.md criteria.md:612-621 covers both supported root locations
9 I14's supporting-document exemption ignores startup @path imports criteria.md:636-642 resolves imports first, to four hops

Finding 6, reproduced

conflict-scan.sh --count on the finding's exact strings:

Case origin/main this branch
"Always use Read and never use Bash" vs "Never use Read" 0 1
"Always use Read but never use Bash" vs "Never use Read" 1 1

A bare and boundary would also return 1 for the first row while creating a new false negative:
"Never use Bash and Grep" is one directive over two objects, and cutting at the coordinator strips
the never governing Grep. The boundary therefore requires a polarity token after and, and is
consumed asymmetrically — a leading window resumes after the coordinator alone so that token still
classifies its entity. Case 35 is the must-not-flag test and fails under a bare-and boundary.

Citations re-verified against the live docs

Every citation this batch introduced was diffed character-for-character against the raw markdown of
hooks, memory, and sub-agents (fetched 2026-08-08). Three did not survive and were corrected:

  • disableAllHooks has no documented "own settings level and below" cascade. The docs say
    "Disable all hooks", with exactly one carve-out: set in user, project, or local settings it cannot
    reach managed hooks. The earlier wording invented a directional cascade. Rewritten.
  • The skills: citation was a splice of the frontmatter-table sentence and the body sentence,
    quotable as neither. Replaced with the real one: "The full content of each listed skill is injected
    into the subagent's context at startup."
  • Imports recurse "with a maximum depth of four hops" — the text pointed at a "hop limit" the page
    never names. Now states the figure and quotes the wording.

Also corrected: the AGENTS.md import is recommended (a symlink is a co-equal alternative, and the
import is mandated only on Windows), not prescribed; subagent memory paths take upstream's
<name-of-agent> placeholder; and the auto-memory gate now names what the subagent actually loses —
"the memory instructions or the memory tool access".

Also fixed in passing

conflict-scan.test.sh:378 had unescaped backticks inside a double-quoted assertion message, so the
suite ran and as a command (and: command not found on stderr) and printed the label with a hole in
it. Exit status stayed 0, so no gate saw it. Now single-quoted, matching the file's existing idiom.

Deliberately not changed

  • ${CLAUDE_CONFIG_DIR:-~/.claude} — that form is the correct resolver, not a hardcode.
  • ~/.claude inside quoted upstream text (criteria.md:651, conflict-criteria.md:111) — altering a
    quotation is a worse defect than the one it would resolve.
  • .claude/rules/** routing to claude-memory's C6 — C6's rules discovery is recursive, so nested
    rules do not leak. Finding 5 is narrower than filed.

Testing

  • conflict-scan.test.sh — 46/46, no stderr noise
  • All 7 claude-config plugin tests — PASS
  • check-changelog-parity.sh --check / --check-bump origin/main / --check-order — PASS
  • check-changed-skills.sh origin/main — 4 skills, 0 failed (audit-instructions 281/500 lines)
  • check-shell-portability.sh / check-skill-portability.sh — PASS
  • shellcheck, markdownlint-cli2, typos — clean
  • validate-plugins.sh, check-contract-slice-prune.sh, check-contract-clause-coverage.py,
    check-silent-skips.sh, check-cross-plugin-source-drift.sh, check-skill-leaf-names.sh,
    check-orphaned-fixtures.sh — PASS

Related

Discharges review findings filed on #1316, #1343, and #1349.

No linked issue

kyle-sexton and others added 5 commits August 7, 2026 08:28
…w threads

Stop hardcoding ~/.claude in the audit-instructions guidance that forbids it;
drop the "or preloads" clause from the I3 deferral bar since skills: injects
full skill content into every dispatch; inventory subagent-owned memory in
Phase A and add the co-residency row it feeds; make the hook liveness
procedure per-scope; narrow the C6 route to root-level project pairs; add the
missing polarity-bearing coordination boundary to BOUNDARY_ERE; and name the
awk/sort prerequisites the README omitted.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… all prerequisites

Continues the audit-instructions batch. Adds the agent-memory inventory bullet
and its co-residency row with a config-root-aware user path, makes the hook
liveness procedure resolve per scope so managed hooks survive a user, project,
or local disable, and names the awk and sort prerequisites the README omitted
alongside jq and curl.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fix the test message whose backticks ran `and` as a command and blanked the
assertion label, resolve the two residual `~/.claude` operative paths the skill's
own rule forbids, and record the batch in the changelog against a bumped version.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… docs

Fetched hooks, memory, and sub-agents and diffed every citation the batch
introduced. Three did not survive: `disableAllHooks` has no documented
"own level and below" cascade — its reach is all hooks with a managed carve-out;
the `skills:` citation was a splice of two real sentences; and imports recurse
"with a maximum depth of four hops", a figure the text pointed at without naming.
The `AGENTS.md` import is recommended, not prescribed, and mandated only on
Windows. Subagent memory paths take upstream's `<name-of-agent>` placeholder, and
the auto-memory gate names what the subagent actually loses.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@cursor

cursor Bot commented Aug 8, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@claude

claude Bot commented Aug 8, 2026

Copy link
Copy Markdown

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


Security Review — PR #2003

  • Gather context (PR diff, changed files)
  • Review shell scripts (conflict-scan.sh, conflict-scan.test.sh) for injection/portability security issues
  • Review skill markdown files (SKILL.md, criteria.md, conflict-criteria.md) for instruction-surface issues
  • Review README.md / CHANGELOG.md / plugin.json changes
  • Synthesize findings and post review

No security issues found.

Scope of this PR: a version bump (plugin.json), a changelog/README prose update, five instruction-surface
markdown files (audit-instructions and setup skills), and an awk-based text-classification script
(conflict-scan.sh) plus its test suite. No GitHub Actions workflow, permissions block, hook wiring, or
dependency pin is touched, so the zizmor-covered lanes (unpinned actions, dangerous triggers, excessive
permissions, template injection) don't apply here — there's nothing in that category in this diff.

What I specifically checked and ruled out:

  • conflict-scan.sh (blob): the new COORD_ERE/COORD_HEAD_ERE regexes and cut_lead() awk function are static, hardcoded patterns passed via -v — no user- or file-content-controlled string reaches the awk program text, so there's no awk/shell injection surface here (same trust model as the pre-existing BOUNDARY_ERE et al.). File arguments are only ever read as data (getline line < file), never executed or eval'd.
  • conflict-scan.test.sh: the PR's own "fixed in passing" item — unescaped backticks inside a double-quoted assertion string that caused `and` to be shell-executed as a command — is a real fix and I confirmed no other double-quoted string in the file still contains unescaped backticks (the remaining backtick occurrences are all inside # comments).
  • SKILL.md / criteria.md / conflict-criteria.md changes: these are prose instructions consumed by the model, not executable config. They resolve paths against ${CLAUDE_CONFIG_DIR:-~/.claude} for reads only (auditing existing local memory/hook/agent files) and don't introduce any new network egress, credential handling, or write/exec instruction.
  • plugin.json: version bump only, no manifest field changes (no new mcpServers, permissions, or trust-relevant fields).
  • README.md / CHANGELOG.md: prose only.
  • Scanned the full diff for embedded secrets/tokens/credentials — no matches beyond the prose word "token" (polarity-classification terminology).

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e59a42a7d5

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

Comment thread plugins/claude-config/skills/audit-instructions/scripts/conflict-scan.sh Outdated
@claude

claude Bot commented Aug 8, 2026

Copy link
Copy Markdown

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


Review: nine stranded findings, claude-config 0.21.10

No blocking issues found. This is a careful, well-scoped fix batch, and I independently re-verified the parts most worth checking rather than taking the PR body's claims at face value.

What I verified myself (not just author-claimed):

  • Fetched sub-agents, hooks, and memory from code.claude.com directly and diffed them against every corrected quote in this PR:
    • disableAllHooks per-scope cascade (conflict-criteria.md:253-262, SKILL.md:175-185) — matches the live hooks page verbatim, including the managed-hook carve-out.
    • The skills: field quote ("The full content of each listed skill is injected into the subagent's context at startup") — criteria.md:212-213 and conflict-criteria.md:101 now quote this exactly; the doc's frontmatter-table sentence the PR says was wrongly spliced before is a different, similar-but-not-identical sentence, confirming the fix was real.
    • "maximum depth of four hops" (criteria.md:640,655) — exact match to the memory page.
    • Subagent memory paths (agent-memory/<name-of-agent>/, .claude/agent-memory/<name-of-agent>/, .claude/agent-memory-local/<name-of-agent>/) at SKILL.md:222-225 — exact match to the sub-agents page's persistent-memory table.
  • Manually traced the conflict-scan.sh cut_lead/COORD_ERE/COORD_HEAD_ERE logic (lines 131-143, 165-184, 247-250) against all three new test cases (33/34/35). The asymmetric leading/trailing cut is correct: the trailing window truncates before the coordinator (so the first entity doesn't inherit the second directive's polarity), while the leading window truncates after the coordinator alone (so the second directive's own polarity token survives to classify it). The bare-and case correctly does not match COORD_ERE (no polarity token follows), so it isn't treated as a boundary — confirmed this doesn't regress the "never X and Y" one-directive-two-objects reading.
  • Confirmed via Grep that no double-quoted assert_* message in conflict-scan.test.sh still contains a raw backtick (the passing-fix for the and: command not found bug is complete).
  • Cross-checked the awk/sort usage claims against the actual scripts: check-plugin-drift.sh uses both, fix-plugin-drift.sh uses sort only (no awk), permission-rule-check.sh uses both, check-structure.sh uses neither — matches exactly what setup/SKILL.md and README.md now say.
  • Read the full conflict-criteria.md routing table and co-residency table, and SKILL.md's Phase A inventory section, in context (not just the diff hunks) — the "root-level project" narrowing for finding feat(hook-telemetry): marketplace-wide telemetry contract + markdown-formatter producer #5 is applied consistently everywhere it's referenced, and the ~/.claude → resolved-root substitutions in finding chore: initialize marketplace scaffold #1 are applied at every site Phase A itself lists, without touching the two deliberately-preserved quoted/resolver forms.

Not independently verified (sandbox in this session blocks bash <script> and shellcheck execution, only read-only tools and gh/grep succeeded): the PR's conflict-scan.test.sh 46/46 run, the claude-config plugin test suite, and the check-changelog-parity.sh/check-shell-portability.sh/etc. results are author-claimed and unverified by me. Nothing in the code changes gives reason to doubt them, but I could not run them myself in this lane.

Minor observations (not blocking):

  • plugin.json version (0.21.10) and the new CHANGELOG.md entry stay in the format the file already uses (no per-entry dates elsewhere either — consistent).
  • The two "deliberately not changed" ~/.claude occurrences called out in the PR body (criteria.md:651, conflict-criteria.md:111) are indeed inside quoted upstream doc text, so leaving them hardcoded is correct — changing them would misquote the source.

Security-scoped concerns are out of scope for this lane (repo has a dedicated claude-security-review workflow) — nothing here would have raised any regardless (no secrets, no injection surface changed; conflict-scan.sh's new patterns operate only on static ERE construction, no new external input paths).

No inline comments — I didn't find anything anchored to a specific line worth flagging separately from the above.

@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown

Claude has reviewed this PR 5 times. The lane skips further automatic reviews after 5; deleting this comment resets the count.

Main shipped 0.22.0 while this work sat, so the branch's entry is renumbered to
0.22.1 and stacked above it; the manifest follows.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Aug 8, 2026

Copy link
Copy Markdown

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


Code Review — PR #2003

One inline finding posted (conflict-scan.sh:139) — confirms and independently verifies the outstanding Codex comment.

What I checked

I reviewed all 9 changed files against origin/main (git diff origin/main...HEAD). The Bash sandbox in this lane blocks script execution (bash <script> requires approval that isn't granted here), so I could not run conflict-scan.test.sh or the other author-claimed test commands myself — those results in the PR body's Testing section are author-claimed and unverified by me. What I could do, and did:

  • Statically traced the cut_lead/COORD_ERE awk logic (conflict-scan.sh:131-297) by hand against the Codex-flagged input, character-by-character through the pad/post/pre substring math. This confirmed the finding is real — see the inline comment. It's a genuine gap in the new machinery this PR adds for finding perf(markdown-formatter): unwired producer builds telemetry payload before the opt-in guard (~90ms/edit on Windows) #6: COORD_ERE (line 139) carries a shorter token alternation than MANDATE_ERE (line 113), missing use|present|ask, so a coordinated second directive phrased as a bare "...and use X" (very plausible instruction phrasing, and the exact form MANDATE_ERE's own comment names as the primary mandate verb) is not recognized as a window boundary. That produces both a false negative (the case Codex's example targets) and a possible false positive against a third file using Always use X. None of the three new tests (cases 33–35) exercise a bare-use second directive, so this shipped untested.
  • Verified the cross-plugin routing claim for finding feat(hook-telemetry): marketplace-wide telemetry contract + markdown-formatter producer #5 by reading claude-memory's actual discovery command (plugins/claude-memory/skills/audit/context/audit.md:12): find . -maxdepth 1 -name "CLAUDE.md" -o -name "CLAUDE.local.md". This confirms C6 truly cannot discover a nested CLAUDE.md, and confirms .claude/rules discovery there (find .claude/rules -name "*.md" -type f, no -maxdepth) is recursive — so the "root-level project only" narrowing in SKILL.md:413-417 is correctly scoped and the "deliberately not changed" .claude/rules/** claim holds.
  • Verified the awk/sort prerequisite claims in README.md and setup/SKILL.md against the actual scripts via grep: check-plugin-drift.sh uses both, fix-plugin-drift.sh uses only sort, permission-rule-check.sh uses both, check-structure.sh uses neither — exactly matching what the new text says.
  • Confirmed the backtick fix is complete: no remaining double-quoted assert_* message in conflict-scan.test.sh contains an unescaped backtick.
  • Read the full SKILL.md/criteria.md/conflict-criteria.md diffs in context; the "resolved user root" term used throughout (e.g. SKILL.md:188,200,223) is defined before its first use (SKILL.md:124), so there's no dangling forward reference.

I did not re-fetch the live hooks/memory/sub-agents docs myself in this session — the prior automated review already did that verification character-for-character, and I have no reason to doubt it, but I'm not re-claiming it as mine.

Minor, non-blocking observation

The PR title/body says it "bumps the plugin to 0.21.10", but plugin.json actually goes 0.22.00.22.1, matching the new CHANGELOG.md entry. Just a stale number in the PR description text, not a code issue.

@claude

claude Bot commented Aug 8, 2026

Copy link
Copy Markdown

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


Security Review — PR #2003 (re-run at f0884f8)

  • Gather context (gh pr diff / local diff against origin/main, changed files)
  • Review conflict-scan.sh / conflict-scan.test.sh for injection/portability issues
  • Review skill markdown files (SKILL.md, criteria.md, conflict-criteria.md) for instruction-surface security issues
  • Review README.md / CHANGELOG.md / plugin.json changes
  • Synthesize findings and post review

No security issues found.

This run re-diffed the branch at HEAD f0884f8 against origin/main (same 9 files as the prior review at e59a42a, now including the merge commit). Scope is unchanged: a version bump (plugin.json), prose-only README.md/CHANGELOG.md updates, five instruction-surface markdown files (audit-instructions and setup skills), and an awk-based text-classification script (conflict-scan.sh) plus its test suite. No GitHub Actions workflow, permissions block, hook wiring, or dependency pin is touched, so the zizmor-covered lanes (unpinned actions, dangerous triggers, excessive permissions, template injection) don't apply.

What I checked:

  • conflict-scan.sh (COORD_ERE/COORD_HEAD_ERE/cut_lead, lines 131-184): the new regexes are static, hardcoded EREs passed to awk via -v — no user- or file-content-controlled string reaches the awk program text itself, so there's no injection surface (same trust model as the pre-existing patterns). File arguments are only ever read as data (getline), never executed or eval'd.
  • conflict-scan.test.sh: all new/changed assertion strings are static heredocs and literal fixtures; confirmed no double-quoted assert_* message anywhere in the file still contains an unescaped backtick (the "fixed in passing" backtick-in-double-quotes bug the PR describes is fully resolved).
  • SKILL.md / criteria.md / conflict-criteria.md / setup/SKILL.md: prose instructions consumed by the model, not executable config. They only add path-resolution and liveness-gate guidance (${CLAUDE_CONFIG_DIR:-~/.claude}, disableAllHooks/allowManagedHooksOnly per scope, @path import depth) for read-only auditing of local files — no new network egress, credential handling, or write/exec instruction introduced.
  • plugin.json: version bump only (0.22.00.22.1), no manifest field changes (no new mcpServers, permissions, or trust-relevant fields).
  • README.md / CHANGELOG.md: prose only, documenting awk/sort as additional prerequisites — no behavior change.
  • Scanned the full diff for embedded secrets/tokens/credentials — no matches beyond the prose word "token" used as polarity-classification terminology.

Out of scope for this lane: Codex's inline comment flags a real completeness gap in COORD_ERE (a second directive using a bare mandate form like and use Read isn't recognized as a coordination boundary) — that's a text-classification false-negative in an audit tool, not an injection/auth/secrets/GitHub-Actions issue, so it falls outside this security-only review.

Comment thread plugins/claude-config/skills/audit-instructions/scripts/conflict-scan.sh Outdated
Main shipped 0.23.0 through 0.25.0 and its own 0.22.1 while this work sat, so
the branch's entry is renumbered to 0.25.1. The memory-layer sentence composes
both sides: this branch's config-root-aware user path with main's widened
I6-I27 check range.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Aug 8, 2026

Copy link
Copy Markdown

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


Security Review — PR #2003 (re-run at f07eef3)

  • Gather context (gh pr diff against origin/main, changed files)
  • Review conflict-scan.sh / conflict-scan.test.sh for injection/portability issues
  • Review skill markdown files (SKILL.md, criteria.md, conflict-criteria.md) for instruction-surface security issues
  • Review README.md / CHANGELOG.md / plugin.json changes
  • Synthesize findings and post review

No security issues found.

This run re-diffed the branch at HEAD f07eef3 (merge commit) against origin/main via gh pr diff — same 9 files as the prior reviews on this PR, content unchanged (only the plugin.json version numbers shifted, 0.25.00.25.1, from origin/main moving forward under the rebase/merge). Scope: a version bump (plugin.json), prose-only README.md/CHANGELOG.md updates, five instruction-surface markdown files (audit-instructions and setup skills), and an awk-based text-classification script (conflict-scan.sh) plus its test suite. No GitHub Actions workflow, permissions block, hook wiring, or dependency pin is touched, so the zizmor-covered lanes (unpinned actions, dangerous triggers, excessive permissions, template injection) don't apply here.

What I checked directly:

  • conflict-scan.sh (COORD_ERE/COORD_HEAD_ERE/cut_lead, lines 131–184): the new regexes (COORD_ERE, COORD_HEAD_ERE) are static, hardcoded EREs passed to awk via -v — no user- or file-content-controlled string reaches the awk program text itself, so there is no awk/shell injection surface (same trust model as the pre-existing BOUNDARY_ERE et al.). File arguments are only ever consumed as data via getline line < file, never eval'd or executed. command -v awk/command -v sort gate at the top (lines 77–82) are unmodified.
  • conflict-scan.test.sh: all new/changed assertion strings (cases 33–35, lines ~338–376) are static heredocs and literal fixtures written to $TEST_TMPDIR; confirmed no double-quoted assert_* message anywhere in the file still contains an unescaped backtick (the PR's own "fixed in passing" bug — backticks in a double-quoted string causing shell command substitution — is a real, complete fix).
  • SKILL.md / criteria.md / conflict-criteria.md / setup/SKILL.md: prose instructions consumed by the model, not executable config. They only add path-resolution and liveness-gate guidance (${CLAUDE_CONFIG_DIR:-~/.claude}, disableAllHooks/allowManagedHooksOnly per scope, @path import depth) for read-only auditing of local files — no new network egress, credential handling, or write/exec instruction is introduced anywhere in the diff.
  • plugin.json: version bump only (0.25.00.25.1), no manifest field changes (no new mcpServers, permissions, or trust-relevant fields).
  • README.md / CHANGELOG.md: prose only, documenting awk/sort as additional prerequisites across three skills — no behavior change, no secrets.
  • Scanned the full diff for embedded secrets/tokens/credentials — no matches beyond the prose word "token" used as polarity-classification terminology.

Out of scope for this lane: the outstanding Codex/Claude inline comment on conflict-scan.sh:139 (COORD_ERE's token set omitting use|present|ask from MANDATE_ERE, causing a text-classification false-negative/false-positive in the conflict scanner) is a correctness gap in an audit tool, not an injection/auth/secrets/GitHub-Actions issue — it falls outside this security-only review and I'm not re-litigating it here.
· branch stranded/claude-config-batch

@claude

claude Bot commented Aug 8, 2026

Copy link
Copy Markdown

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


Code Review — PR #2003 (re-run at f07eef3)

  • Gather context (diff against origin/main, prior review comments)
  • Review shell script changes (conflict-scan.sh, conflict-scan.test.sh)
  • Review skill markdown changes (SKILL.md, criteria.md, conflict-criteria.md, setup/SKILL.md)
  • Review README.md/CHANGELOG.md/plugin.json
  • Check outstanding Codex/Claude findings (COORD_ERE gap) for current status
  • Post findings

One inline finding posted (conflict-scan.sh:139) — the COORD_ERE token-set gap that Codex and a prior automated review flagged on earlier commits of this PR is still present, unchanged, and unaddressed on this HEAD. I re-traced it by hand against the current code (this sandbox blocks script execution, so it's a static trace, not a run) and confirmed the false-negative/false-positive pair it produces still reproduces. Details and a fix suggestion are in the inline comment.

What I independently checked (not just re-stating author or prior-review claims)

  • The COORD_ERE gap — traced by hand through cut_lead/classify (lines 139, 165–184, 198–200) against Never use `Bash` and use `Read` on this exact HEAD. Confirmed Read is misclassified prohibit instead of mandate because and use doesn't match COORD_ERE's alternation (missing use|present|ask, which MANDATE_ERE at line 113 carries). This is the same defect Codex and a prior review reported — it has not been fixed across the commits since.
  • Test coverage — read all of conflict-scan.test.sh's new cases (33–35); confirmed none exercise a bare use (or present/ask) second directive, so the gap above ships untested.
  • awk/sort prerequisite claims in README.md/setup/SKILL.md — grepped the four scripts myself: check-plugin-drift.sh uses both (lines 156–210), fix-plugin-drift.sh uses only sort (no awk match), permission-rule-check.sh uses both, check-structure.sh uses neither. Matches exactly what the new text says.
  • The C6 routing claim for finding feat(hook-telemetry): marketplace-wide telemetry contract + markdown-formatter producer #5 — read plugins/claude-memory/skills/audit/context/audit.md:12,15 myself: find . -maxdepth 1 -name "CLAUDE.md" -o -name "CLAUDE.local.md" (non-recursive) vs find .claude/rules -name "*.md" -type f (recursive, no -maxdepth). Confirms C6 genuinely can't discover a nested CLAUDE.md/CLAUDE.local.md, and confirms .claude/rules discovery there is recursive — so the "root-level project only" narrowing in SKILL.md is correctly scoped, and leaving .claude/rules/** routing unchanged is correct.
  • Forward reference to "the user root Phase A resolves" (SKILL.md:48-49) — appears before the actual resolution rule at SKILL.md:135. Checked this isn't a dangling/undefined term: the phrase explicitly names "Phase A" as where it's resolved, so it's an intentional forward pointer to a named section rather than a broken reference. Not a defect.

I did not re-fetch the live hooks/memory/sub-agents docs myself this run — two prior automated reviews on earlier commits of this PR already diffed the corrected quotes (disableAllHooks cascade, the skills: field quote, "four hops") character-for-character against those pages, and the relevant text is unchanged since. I have no basis to doubt that prior verification, but I'm not re-claiming it as mine.

Not independently verified — script execution is blocked in this sandbox (only read-only tools succeeded), so conflict-scan.test.sh's 46/46 claim, the claude-config plugin test suite, and the check-changelog-parity.sh/check-shell-portability.sh/etc. results in the PR body's Testing section are author-claimed and unverified by me.

Everything else

Read the full diffs of SKILL.md, criteria.md, conflict-criteria.md, and setup/SKILL.md in context (not just hunks) — no other correctness, architecture, or maintainability issues found. The cut_lead extraction (dedup of the pre-window boundary logic into a shared function) is a reasonable refactor and preserves the original bnd-cut semantics.
· branch stranded/claude-config-batch

Comment thread plugins/claude-config/skills/audit-instructions/scripts/conflict-scan.sh Outdated
Main shipped 0.26.0 and 0.27.0 while this work sat, so the branch's entry is
renumbered to 0.27.1 and its description follows main's, which names the new
audit-prompting-postures skill. The memory-layer sentence composes both sides:
this branch's config-root-aware user path with main's widened I6-I28 range.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Aug 9, 2026

Copy link
Copy Markdown

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


Security Review — PR #2003 (re-run at cbf78a0)

  • Gather context (git diff origin/main...HEAD, changed files)
  • Review conflict-scan.sh / conflict-scan.test.sh for injection/portability issues
  • Review skill markdown files (SKILL.md, criteria.md, conflict-criteria.md, setup/SKILL.md) for instruction-surface security issues
  • Review README.md / CHANGELOG.md / plugin.json changes
  • Synthesize findings and post review

No security issues found.

This run re-diffed the branch at HEAD cbf78a0 (merge of origin/main) against origin/main. Same 9 files as every prior review on this PR, content unchanged since the last security pass at f07eef3 (only the merge commit itself landed since then — no new hunks in the changed files). Scope is unchanged: a version bump (plugin.json, 0.27.00.27.1), prose-only README.md/CHANGELOG.md updates, five instruction-surface markdown files (audit-instructions and setup skills), and an awk-based text-classification script (conflict-scan.sh) plus its test suite. No GitHub Actions workflow, permissions block, hook wiring, or dependency pin is touched, so the zizmor-covered lanes (unpinned actions, dangerous triggers, excessive permissions, template injection) don't apply here.

What I checked directly:

  • conflict-scan.sh (COORD_ERE/COORD_HEAD_ERE/cut_lead, lines 131–184): the new regexes are static, hardcoded EREs passed to awk via -v — no user- or file-content-controlled string reaches the awk program text itself, so there is no awk/shell injection surface (same trust model as the pre-existing BOUNDARY_ERE et al.). File arguments are only ever consumed as data via getline line < file, never eval'd or executed.
  • conflict-scan.test.sh: all new/changed assertion strings (cases 33–35) are static heredocs written with 'EOF' (no shell expansion inside the fixtures) and literal fixtures; confirmed no double-quoted assert_* message anywhere in the file still contains an unescaped backtick — the PR's "fixed in passing" backtick-in-double-quotes bug (which made `and` execute as a command) is fully and correctly resolved.
  • SKILL.md / criteria.md / conflict-criteria.md / setup/SKILL.md: prose instructions consumed by the model, not executable config. They only add path-resolution and liveness-gate guidance (${CLAUDE_CONFIG_DIR:-~/.claude}, disableAllHooks/allowManagedHooksOnly per scope, @path import depth) for read-only auditing of local files — no new network egress, credential handling, or write/exec instruction anywhere in the diff.
  • plugin.json: version bump only, no manifest field changes (no new mcpServers, permissions, or trust-relevant fields).
  • README.md / CHANGELOG.md: prose only, documenting awk/sort as additional prerequisites — no behavior change.
  • Scanned the full diff for embedded secrets/tokens/credentials — no matches beyond the prose word "token" used as polarity-classification terminology.

Out of scope for this lane: the outstanding Codex/Claude inline finding on conflict-scan.sh:139 (COORD_ERE's token set omitting use|present|ask from MANDATE_ERE, causing a text-classification false-negative/false-positive in the conflict scanner) is still present on this HEAD and unaddressed across all commits since it was first reported. It's a correctness gap in an audit tool's classifier, not an injection/auth/secrets/GitHub-Actions issue, so it stays outside this security-only review — as flagged consistently by the code-review lane, not this one.
· branch stranded/claude-config-batch

@claude

claude Bot commented Aug 9, 2026

Copy link
Copy Markdown

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


Code Review — PR #2003 (re-run at cbf78a0)

  • Gather context (diff against origin/main, prior review threads)
  • Check whether the outstanding COORD_ERE finding (conflict-scan.sh:139) is fixed at current HEAD
  • Review remaining changed files for correctness/maintainability
  • Post findings

One inline finding re-posted (conflict-scan.sh:139) — the COORD_ERE token-set gap Codex and two prior automated reviews flagged on earlier commits is still open, unchanged, byte-for-byte, at this HEAD. Re-traced by hand (script execution is blocked in this sandbox, so this is a static trace, not a run): COORD_ERE omits use|present|ask from its alternation even though MANDATE_ERE (line 113) carries them, so Never use `Bash` and use `Read` still misclassifies Read as prohibit. Test cases 33–35 don't exercise a bare use/present/ask second directive, so it ships untested — see the inline comment for the full trace.

One new finding, no changed line to anchor it to:

  • plugins/claude-config/skills/audit-instructions/reference/criteria.md:90permalink. This PR's finding chore: initialize marketplace scaffold #1 fixed SKILL.md:45-49 from `.claude/rules/`, `~/.claude/rules/` to `.claude/rules/`, and `rules/` under the user root Phase A resolves, on the grounds that the skill "forbids hardcoding ~/.claude, then hardcodes it." criteria.md:90 carries the identical surface-partition sentence — (CLAUDE.md, CLAUDE.local.md, `.claude/rules/`, `~/.claude/rules/`) — word-for-word what SKILL.md:45 said before the fix, and it wasn't touched by this PR. It's not upstream-quoted text (unlike the deliberately-preserved criteria.md:651/conflict-criteria.md:111, which quote the sub-agents docs verbatim) — it's this skill's own descriptive prose defining "memory-layer surfaces," the same category the SKILL.md fix addressed. Left as-is, criteria.md and SKILL.md now describe the same concept inconsistently, and the same hardcode the PR set out to eliminate survives in the reference file. Suggest applying the same "user root Phase A resolves" substitution here.

I re-verified this is a real gap: criteria.md:90's surface-partition sentence is unchanged in git diff origin/main...HEAD, and I checked it isn't nested inside a blockquote/citation the way the two deliberately-preserved instances are (confirmed by reading their surrounding context).

What I independently checked (not just restating prior review or author claims):

  • Manually traced cut_lead/COORD_ERE/COORD_HEAD_ERE (conflict-scan.sh:131-184) against the Codex-flagged input on this exact HEAD — confirmed the misclassification still reproduces (see inline comment).
  • Read conflict-scan.test.sh:343-379 (cases 33-35) — confirmed none exercise a bare use/present/ask second directive.
  • Grepped every ~/.claude occurrence across the five touched skill/reference files, classified each as: the ${CLAUDE_CONFIG_DIR:-~/.claude} resolver form (correct), explanatory prose about not hardcoding it (correct), quoted upstream doc text at criteria.md:710 and conflict-criteria.md:111 (deliberately preserved, correctly so — confirmed by reading context), factual statements about another plugin's (claude-memory's) own fixed implementation paths in conflict-criteria.md:44-62 (out of scope — describing a third party's hardcode, not this skill's own resolvable path), and the one unaddressed instance at criteria.md:90 flagged above.
  • Diffed README.md, plugin.json, and CHANGELOG.md — version bump (0.27.00.27.1) matches the new changelog entry; the awk/sort prerequisite text is internally consistent with setup/SKILL.md's changes.
  • Read the full SKILL.md, criteria.md, and conflict-criteria.md diffs in context (not just hunks) — the finding feat: markdown-formatter plugin (pilot migration) #3 (subagent memory inventory), ci: onboard to the CI platform (ci-workflows + standards) #4 (hook-liveness gate), feat(hook-telemetry): marketplace-wide telemetry contract + markdown-formatter producer #5 (nested-project routing), build(dependabot): fast-track first-party melodic-software/* actions #8/chore: adopt widened comment-hygiene policy and re-pin ci-workflows @9567b8b #9 (I14 startup set and import resolution) changes read correctly and match their changelog descriptions; no other correctness issues found there.

Not independently verified — test/lint execution is blocked in this sandbox (only read-only tools succeeded), so the PR body's conflict-scan.test.sh 46/46 claim, the claude-config plugin test suite, and the check-changelog-parity.sh/check-shell-portability.sh/etc. results are author-claimed and unverified by me.

Security-scoped concerns are out of scope for this lane (this repo has a dedicated claude-security-review workflow) and are not addressed here.
· branch stranded/claude-config-batch

Comment thread plugins/claude-config/skills/audit-instructions/scripts/conflict-scan.sh Outdated
…sifiers

COORD_ERE carried a hand-copied list that had fallen behind MANDATE_ERE — `use`,
`present`, and `ask` were in the classifier and missing from the coordinator, so
`Never use X and use Y` found no boundary, Y inherited the leading `never`, and
its pair with `Never use Y` went unreported while `Always use Y` produced a
false one. `always` sat in both lists and masked the gap.

The coordinator now composes the two classifier alternations instead of
restating them, making the divergence unrepresentable. Adds regression cases for
the bare-`use` and `present` forms; the existing `always` case is the control
that passed throughout.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Aug 9, 2026

Copy link
Copy Markdown

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


Security Review — PR #2003 (re-run at 66daa51)

  • Gather context (git log/git show, changed files)
  • Review conflict-scan.sh / conflict-scan.test.sh for injection/portability issues
  • Review skill markdown files (SKILL.md, criteria.md, conflict-criteria.md, setup/SKILL.md) for instruction-surface security issues
  • Review README.md / CHANGELOG.md / plugin.json changes
  • Synthesize findings and post review

No security issues found.

This run's HEAD is 66daa51, one new commit on top of the cbf78a0 state reviewed previously. This session's sandbox couldn't reach the network (git fetch was blocked), so I diffed the new commit directly with git show 66daa51 rather than a full origin/main...HEAD diff; the other 8 files are unchanged since the last full pass at cbf78a0; and that pass's results (no security issues, only the then-outstanding COORD_ERE correctness gap) still stand for that code.

What's new in 66daa51 — touches only conflict-scan.sh, conflict-scan.test.sh, and CHANGELOG.md:

  • conflict-scan.sh (PROHIBIT_ALT/MANDATE_ALT/COORD_ERE, lines 109–143): this is the fix for the COORD_ERE token-set gap that Codex and three prior automated reviews flagged on earlier commits of this PR. COORD_ERE is now built by string-composing the pre-existing PROHIBIT_ALT/MANDATE_ALT shell variables ("[^a-z],? *and +(${PROHIBIT_ALT}|${MANDATE_ALT})[^a-z]") instead of retyping a hand-copied alternation. From a security standpoint this changes nothing about the trust model: PROHIBIT_ALT, MANDATE_ALT, and the composed COORD_ERE are all still static, hardcoded shell string literals — no user- or file-content-controlled data flows into any of these variables or into the regex text. They're passed to awk via -v coord="$COORD_ERE" (line ~165), same mechanism as before — -v assigns a value, it doesn't interpolate into the awk program source, so there's no awk/shell injection surface here, unchanged from every prior review of this file. File arguments remain read-only via getline.
  • Also in this commit: a whitespace/indentation-only reflow of the -h|--help case block (lines 66–71) and no functional change there.
  • conflict-scan.test.sh: two new regression cases (bare-use and present coordinated directives) are static heredocs with literal fixture content, consistent with the rest of the file — no injection surface.
  • CHANGELOG.md: prose-only addition describing the fix; no secrets or executable content.

Confirms the previously out-of-scope correctness finding is now resolved (noted for completeness, not as a security finding — it was never security-relevant): the COORD_ERE/MANDATE_ERE token-set divergence that Codex and prior reviews flagged (use/present/ask missing from the coordinator) is fixed by construction, since the coordinator now composes from the same alternation the classifier uses rather than a separately maintained copy.

No GitHub Actions workflow, permissions block, hook wiring, or dependency pin is touched by this commit or by the PR as a whole, so the zizmor-covered lanes (unpinned actions, dangerous triggers, excessive permissions, template injection) remain not applicable. Scanned this commit's diff for embedded secrets/tokens/credentials — none found.
· branch stranded/claude-config-batch

@claude

claude Bot commented Aug 9, 2026

Copy link
Copy Markdown

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


Code Review — PR #2003 (re-run at 66daa51)

  • Gather context (diff against origin/main, prior review threads)
  • Check whether the latest commit (66daa51, "compose the coordinator's token set from the classifiers") resolves the outstanding COORD_ERE finding
  • Check whether the criteria.md:90 hardcode-consistency finding from the prior review is addressed
  • Review remaining changed files for correctness/maintainability
  • Post findings

One inline finding posted (CHANGELOG.md:13-15, new) — malformed inline-code spans in the new changelog entry.

One cross-file finding, no changed line to anchor it to — see below.

The long-standing COORD_ERE finding is now fixed

Commit 66daa51 resolves the gap Codex and three prior automated reviews flagged on earlier commits of this PR. COORD_ERE no longer hand-copies a token list; it's composed from PROHIBIT_ALT and MANDATE_ALT directly (conflict-scan.sh:139):

COORD_ERE="[^a-z],? *and +(${PROHIBIT_ALT}|${MANDATE_ALT})[^a-z]"

I re-traced cut_lead/COORD_ERE by hand (script execution is blocked in this sandbox, so this is a static trace, not a run) against Never use `Bash` and use `Read` for file inspection. for entity Read:

  • Leading text before Read: never use `bash` and use.
  • COORD_ERE's alternation now includes use (via the composed MANDATE_ALT), so coord matches and use.
  • coordhead ('[^a-z],? *and +') matches only the coordinator itself, so cut_lead truncates before use, leaving use in the returned leading text.
  • The window becomes ... use ... Read ...window ~ mandate matches on use, and neither prewindow nor postwindow contains a prohibition token, so Read now correctly classifies as mandate instead of inheriting the leading never.

This also makes the divergence structurally unrepresentable going forward, since both regexes are built from the same two alternation variables rather than independently retyped lists — a stronger fix than just adding the missing tokens. The new test cases 34b (conflict-scan.test.sh:360-379) exercise exactly the bare-use and present forms the prior gap missed, and case 35 confirms the bare-and-joins-two-objects reading still doesn't regress. I did not execute the suite myself (sandboxed), so the 46/46 pass claim remains author-claimed and unverified by me, but the logic traces correctly.

Still open: criteria.md:90 carries the same hardcode this PR's finding #1 set out to eliminate

Finding #1 fixed SKILL.md:45-49 and SKILL.md:79 from a hardcoded ~/.claude/rules/ to ".claude/rules/, and rules/ under the user root Phase A resolves," on the grounds that the skill "forbids hardcoding ~/.claude, then hardcodes it." reference/criteria.md:90 carries the identical surface-partition sentence — (CLAUDE.md, CLAUDE.local.md, `.claude/rules/`, `~/.claude/rules/`) — word-for-word what SKILL.md said before the fix, and this PR (across all commits, including the two landed since a prior review first flagged this) hasn't touched it.

I confirmed this is a live, unaddressed instance and not a quoted citation: it's this skill's own descriptive prose defining "memory-layer surfaces" for the check-tier partition, not upstream doc text (unlike the deliberately-preserved instances at criteria.md:710 and conflict-criteria.md:111, which quote the sub-agents docs verbatim). Left as-is, criteria.md and SKILL.md now describe the same partition inconsistently, and the exact hardcode category this PR's finding #1 targets survives in the reference file it's paired with. Suggest applying the same "rules/ under the user root Phase A resolves" substitution here.

Fix this →

Everything else

Read the full diffs of SKILL.md, conflict-criteria.md, README.md, and setup/SKILL.md in context. Independently verified (not just restating author claims):

  • Grepped check-plugin-drift.sh, fix-plugin-drift.sh, and permission-rule-check.sh myself for awk/sort usage — confirms exactly what README.md:187-197 and setup/SKILL.md:28-52 now say: check-plugin-drift.sh uses both, fix-plugin-drift.sh uses only sort, permission-rule-check.sh uses both.
  • Read conflict-criteria.md's new co-residency row and liveness-gate section (lines 98, 251-292) in context — the disableAllHooks/allowManagedHooksOnly per-scope resolution and the subagent-memory row match SKILL.md's Phase A additions consistently.
  • plugin.json version bump (0.27.00.27.1) matches the new CHANGELOG.md entry heading.

A minor, non-blocking style note: conflict-scan.sh's case "${1:-}" in block (lines 64-70) was reformatted from 2-space-indented case items to flush-left as an incidental part of this commit — cosmetic only, and it now matches inventory.sh/check-structure.sh's convention (though instruction-scan.sh/permission-rule-check.sh still indent theirs, so the repo is inconsistent either way). Not worth a fix on its own.

Not independently verified — test/lint execution is blocked in this sandbox (only read-only tools succeeded), so the PR body's conflict-scan.test.sh 46/46 claim, the claude-config plugin test suite, and the check-changelog-parity.sh/check-shell-portability.sh/etc. results are author-claimed and unverified by me.

Security-scoped concerns are out of scope for this lane (this repo has a dedicated claude-security-review workflow) and are not addressed here.
· branch stranded/claude-config-batch

Comment thread plugins/claude-config/CHANGELOG.md Outdated
kyle-sexton and others added 2 commits August 9, 2026 00:21
…ntry

The example strings contain backticked terms, so a single-backtick span parses
as alternating code and plain runs rather than one literal example. Wraps them
in double backticks and restores the dropped spaces.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Main shipped its own 0.27.1 (the audit-instructions description trim) while this
work sat, so the branch's entry is renumbered to 0.27.2 and stacked above it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Aug 9, 2026

Copy link
Copy Markdown

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


Security Review — PR #2003 (re-run at e109ed5)

  • Gather context (diff against origin/main, changed files)
  • Review conflict-scan.sh / conflict-scan.test.sh for injection/portability issues
  • Review skill markdown files (SKILL.md, criteria.md, conflict-criteria.md, setup/SKILL.md) for instruction-surface security issues
  • Review README.md / CHANGELOG.md / plugin.json changes
  • Synthesize findings and post review

No security issues found.

This run's HEAD (e109ed5) is a merge of origin/main plus one new commit since the last full security pass at 66daa51/cbf78a0: e369210, which touches only CHANGELOG.md. git diff origin/main...HEAD --stat confirms the same 9 files as every prior review on this PR, with no new hunks in conflict-scan.sh, conflict-scan.test.sh, or any of the skill markdown files since that pass.

What's new in e369210: a prose-only fix to the changelog entry's example text — wraps `Never use`Bash` and use `Read `` etc. in double backticks so nested single-backtick spans render literally in Markdown, and restores two dropped spaces. This is documentation formatting only (it was flagged as a non-security Markdown-rendering defect in the prior code-review pass, not a security finding); it introduces no code, no new egress, no credential/secret handling, and no executable content. No security implication.

Everything else (the conflict-scan.sh COORD_ERE/cut_lead awk logic, the conflict-scan.test.sh fixtures, the SKILL.md/criteria.md/conflict-criteria.md/setup/SKILL.md instruction-surface prose, plugin.json's version bump, and README.md) is byte-for-byte unchanged since the last full security pass at cbf78a0, which found no issues:

  • conflict-scan.sh: COORD_ERE/COORD_HEAD_ERE/cut_lead (lines 131–184) remain static, hardcoded EREs composed from the PROHIBIT_ALT/MANDATE_ALT shell variables and passed to awk via -v — no user- or file-content-controlled string reaches the awk program text, so there's no injection surface. File arguments are only ever read as data via getline, never executed or eval'd.
  • conflict-scan.test.sh: all assertion strings are static heredocs and literal fixtures; no double-quoted assert_* message contains an unescaped backtick.
  • Skill markdown files: prose instructions consumed by the model, not executable config — only add path-resolution and liveness-gate guidance for read-only auditing of local files, no new network egress, credential handling, or write/exec instruction.
  • plugin.json: version bump only, no manifest field changes (no new mcpServers, permissions, or trust-relevant fields).

No GitHub Actions workflow, permissions block, hook wiring, or dependency pin is touched anywhere in this PR, so the zizmor-covered lanes (unpinned actions, dangerous triggers, excessive permissions, template injection) don't apply. Scanned the full diff again for embedded secrets/tokens/credentials — no matches beyond the prose word "token" used as polarity-classification terminology.

Out of scope for this lane: the COORD_ERE correctness gap that Codex and prior automated reviews flagged was resolved in commit 66daa51 (confirmed by a prior code-review pass); it was a text-classification defect, not a security issue, and was never in scope for this security lane regardless.
· branch stranded/claude-config-batch

@kyle-sexton
kyle-sexton merged commit 74f063d into main Aug 9, 2026
33 checks passed
@kyle-sexton
kyle-sexton deleted the stranded/claude-config-batch branch August 9, 2026 04:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant