refactor(codebase-health): stop audit at Phase 3, delegate remediation to implementation/verification

[kyle-sexton]

Summary

codebase-health's audit skill ran its own Phases 4-7 (fix / verify / self-review / retro), which duplicate lanes other plugins already own end-to-end — implementation:implement for the fix cadence and verification:confirm for verification. This trims the audit's scope so it stays read-only through its Phase 3 report and delegates all remediation to those two skills, referenced as soft dependencies ("when installed") per this repo's cross-plugin convention.

Fix

• Removed inline Phases 4-7 from skills/audit/SKILL.md; replaced with a ## Remediation (delegated to other plugins) section that routes fixing to /implementation:implement and verification to /verification:confirm as soft refs, matching the established (when the <plugin> plugin is installed) pattern used elsewhere in the fleet.
• --fix repurposed, not removed — it now hands the Phase 3 findings off to those lanes rather than fixing inline, preserving the naming-doctrine flag contract already codified in the skill. Bare audit still reports and STOPs at Phase 3.
• No content silently dropped. The two codebase-health-specific outputs that lived only in the old phases are preserved in the read-only surface:
  • The Config Drift → Missing Enforcement → Code Quality → Doc Drift fix order already travels with Phase 3's "Fix priority" section (cites reference/category-playbook.md), so the fix lane receives it with the findings.
  • Post-audit config-gap observations worth persisting via /codebase-health:setup (previously only in Phase 7's retrospective — an inference-time-distinct, post-audit observation) folded into Phase 3 as a new required section §5.
• Graceful-degrade guarded — the "Adapting to your environment" section now explicitly says remediation is delegated, NOT inlined: when the plugins are absent the findings table is the manual handoff, never a cue to re-inline a fix/verify/review loop (which would re-introduce the exact duplication this change removes).
• All parallel surfaces moved in lockstep so the skill does not drift against itself: frontmatter description, argument-hint arg-parsing block, ## Read-only default, ## Emit checklist, the zero-findings + fix-gate blocks, templates/checklist.md, README.md, plugin.json description, and evals.json (stale "Phases 4-7 skipped" reference fixed in eval 2; new eval 7 asserts --fix delegates rather than inlining).
• Version bump 0.6.2 → 0.7.0 (behavior/contract change) with a matching CHANGELOG.md entry.

Verification

Static skill-quality gate on the modified skill (skill-quality:check, v0.7.0), run against the plugin's own skills root:

INFO: description length 562/1536 chars
INFO: all 4 base-ref trigger phrase(s) preserved
INFO: SKILL.md 267/500 lines
INFO: markdownlint clean
WARN: SKILL.md is 267 lines (soft target 200 — consider pushing detail to progressive-disclosure spokes)
WARN: no Gotchas surface (inline '## Gotchas' or context/gotchas.md) — confirm the skill has no observed failure history
INFO: no metadata.category in frontmatter (optional — category not machine-readable)

CHECK-SKILL audit: PASS — 0 errors, 2 warning(s)

PASS. Both WARNs are pre-existing and advisory (line count was already over the soft target before this change; the skill never shipped a Gotchas surface) — neither is a regression from this refactor, and the trigger-phrase-preservation check confirms no description trigger was dropped by the rewrite. evals.json validates against the bundled schema (check-jsonschema: ok).

Independent review: a fresh reviewer (clean context, rationale withheld) audited the diff and flagged one broken internal anchor — the ## Remediation heading's em-dash produced an ambiguous slug. Fixed by removing the em-dash from the heading (Remediation (delegated to other plugins)) and updating both link references to the unambiguous #remediation-delegated-to-other-plugins anchor; re-ran the gate (still PASS).

Closes #262

Related

• refactor(codebase-health): delegate audit fix phases to implementation/verification lanes #262 — origin: skill-name audit + interview, contract at docs/topics/shadowed-skill-renames/PLAN.md (PR docs: codify skill-naming grammar and cross-plugin reference rules #256).
• No other open PR touches plugins/codebase-health/ (checked at creation), so no serialization / do-not-merge needed.

🤖 Generated with Claude Code

https://claude.ai/code/session_01K1V3gkrfSf75isB8MiDy3o

[claude]

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

---

Security Review — PR #789

• Gather PR diff and read changed files
• Analyze for injection surfaces, logic flaws, and dangerous patterns
• Review delegated-plugin soft-dependency design for authorization gaps
• Report findings

---

Findings

No security issues found.

This PR is a pure behavioral-contract and documentation refactor across six files (plugin.json, CHANGELOG.md, README.md, SKILL.md, evals.json, checklist.md). None of these files introduce executable code at distribution or install time. Full analysis below.

---

Pre-computed context shell commands (SKILL.md lines 9–13)

Current branch: !`git branch --show-current 2>/dev/null || echo "unknown"`
Working tree status: !`git status --porcelain 2>/dev/null | head -20 || echo "(unavailable)"`
Changed files (staged+unstaged): !`git diff --name-only HEAD 2>/dev/null || echo "none"`

These three !-prefixed commands execute in the consumer's shell at skill-load time. All three are read-only introspection commands with no user-supplied input interpolated into them — the outputs are injected into the skill's context as strings, not re-executed. These lines are unchanged from the pre-PR baseline and introduce no new risk.

--fix delegation pattern

The PR's central change is replacing inline fix/verify/self-review/retro phases with a pointer to /implementation:implement and /verification:confirm. This is a pointer-only delegation: SKILL.md explicitly states "a pointer into each lane, NOT an auto-run pipeline; the user drives them." No automatic invocation occurs; no credentials, tokens, or secrets are piped between plugins; and the soft-dependency guard ("when the plugin is installed") prevents the skill from calling a nonexistent target. The net security effect is a reduction in blast radius — --fix no longer applies edits inline.

Config file merge and path traversal

The skill reads from three hardcoded paths (~/.claude/codebase-health.md, .claude/codebase-health.md, .claude/codebase-health.local.md). None of these paths are user-controlled at runtime. Config contents govern primary-sources / verification-sources globs — no command execution surfaces. Additive merge semantics (later layer unions with earlier, never silently replaces) are unchanged from before this PR.

Scope argument handling

User-supplied scope ($ARGUMENTS path) is parsed in natural language by the model and forwarded to read-only tools (Glob, Read) that respect the existing permission system. No shell interpolation of the scope value occurs.

No GitHub Actions, dependency, or supply-chain changes

No .github/workflows/ files were modified. No package manifests or action pins were altered.

---

[claude]

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

---

PR Review

Tasks

• Load tools
• Read changed files and diff
• Review for correctness, security, and alignment with guidelines
• Post findings

---

Summary

The refactor is architecturally sound: the audit's scope is cleanly bounded to Phase 3 (report-only), remediation is delegated via the fleet's established soft-dependency convention, and all parallel surfaces (description, README, checklist, evals, plugin.json, CHANGELOG) move in lockstep. No security concerns. Two correctness issues worth addressing before merge, plus one cosmetic note.

---

Findings

1. Eval 4 output contract doesn't cover new §5 (Config-gap observations) — correctness gap

Eval 4 (scoped-run-output-contract) validates what the Phase 3 report must contain, but its expectations array doesn't include the new required §5 Config-gap observations:

// evals.json — eval id 4, expectations
"Presents findings in the required table..."
"Includes a 'Verified non-issues' section..."
"Includes an enforcement-escalation assessment..."
"Tags findings by confidence..."

SKILL.md L220–223 now mandates §5 as a required section on every audit run ("clean run is the best time to note coverage gaps"), making it part of the output contract. Eval 4 is the canonical "output contract" eval but doesn't assert it, so a model could omit §5 and still pass. Fix this →

2. --fix "hand off" semantics are ambiguous in the skill text

SKILL.md L263–266:

With --fix, present the Phase 3 summary count and hand the findings to /implementation:implement, then suggest /verification:confirm afterwards — a pointer into each lane, NOT an auto-run pipeline; the user drives them.

"Hand the findings to" and "a pointer into each lane" describe two different things: the first suggests the model invokes the skill with the findings as context; the second says it's just a suggestion for the user. If a model reads this literally it may try to auto-invoke the skill. The eval 7 expectation ("Routes fixing to /implementation:implement") doesn't clarify the mechanism either.

Consider tightening to: "present the Phase 3 findings, then output an explicit suggestion: Run /implementation:implement with the findings above, then /verification:confirm. Do NOT auto-invoke those skills." — making it unambiguous that --fix produces a user-directed prompt, not a model-driven invocation. Fix this →

---

Notes (non-blocking)

Eval 7 array position vs ID — Eval with id: 7 is inserted between id: 2 and id: 3 in the JSON array. The rest of the file is ordered by ID. This won't affect runtime behavior (IDs are opaque identifiers) but breaks the file's array-position convention and could trip up a future reviewer diffing by position. Sorting it to the end or re-assigning it id: 3 (with the others bumped) would be cleaner.

CHANGELOG date — ## [0.7.0] omits the date, consistent with all prior entries in this file (none have dates), so this is internally coherent. The Keep a Changelog spec recommends [X.Y.Z] - YYYY-MM-DD, but since the established style here consistently omits dates it's not a regression.

Soft-dependency pattern — Confirmed against the fleet: the (when the <plugin> plugin is installed) phrasing and route-to-dedicated-lane structure match exactly what implementation, verification, testing, and toolchain skills use. No deviation.

Version bump — 0.6.2 → 0.7.0 for a breaking behavior change is correct per SemVer §4 for 0.x.y pre-1.0 software (minor bump carries breaking). CHANGELOG entry accurately marks it as BREAKING.

---

[claude]

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

---

PR Review

Tasks

• Load tools
• Read changed files and diff
• Review for correctness, security, and alignment with guidelines
• Post findings

---

Summary

The refactor is architecturally clean and correctly-scoped: the audit's boundary is Phase 3 (report-only), remediation is delegated via the fleet's established soft-dependency convention, and all parallel surfaces — description, READMEs, checklist, evals, plugin.json, CHANGELOG — move in lockstep. The version bump to 0.7.0, the BREAKING label in the changelog, and the soft-dependency guard pattern all align with the fleet convention.

Two correctness issues from the prior review remain open; I've confirmed both, and found one additional gap. No new security concerns.

---

Findings

1. Eval 4 expected_output AND expectations both omit §5 — correctness gap

Eval 4 (scoped-run-output-contract) is the canonical output-contract eval. Its expected_output explicitly enumerates the required sections and stops at "enforcement escalation" — no mention of config-gap observations:

"expected_output": "...followed by the required sections — verified non-issues, drift patterns, fix priority, and enforcement escalation — with each finding confidence-tagged."

The expectations array also stops at four entries with no §5 assertion. SKILL.md L220–223 mandates §5 as a required section on every run. A model that omits §5 passes eval 4 as-written, making the eval contract drift from the skill contract. Fix this →

2. --fix delegation language is ambiguous about invocation model

SKILL.md L263–264:

With --fix, present the Phase 3 summary count and hand the findings to /implementation:implement,
then suggest /verification:confirm afterwards — a pointer into each lane, NOT an auto-run pipeline

"Hand the findings to" and "suggest" are inconsistent verbs for two actions that should have the same invocation model. More critically, "hand the findings to" reads as a model-driven invocation; the corrective "NOT an auto-run pipeline" that follows can't fully undo that first-read impression. A model encountering this could try to auto-invoke /implementation:implement while correctly treating /verification:confirm as a user prompt.

Suggested rewrite for the relevant sentence: "With --fix, present the Phase 3 findings and output an explicit user-directed suggestion to run /implementation:implement with those findings, then /verification:confirm. Do NOT auto-invoke either skill — the user drives both." Fix this →

---

Notes (non-blocking)

Eval 5 also missing §5 for the zero-findings case. SKILL.md L234–235 says §5 is required even on clean runs ("a clean run is the best time to note coverage gaps"). Eval 5 (clean-scope-no-invented-findings) doesn't assert this; a model can pass it while silently omitting the config-gap section on a zero-findings run. Less critical than eval 4 since the primary contract eval is eval 4, but worth a fifth expectation here too.

Eval 7 array position. Eval with id: 7 sits between id: 2 and id: 3 in the array. IDs are opaque so it's runtime-safe, but the file's established convention orders the array by ID. Moving it to the end (after id: 6) would restore that convention without affecting behavior.

Soft-dependency guard pattern matches the fleet. The (when the <plugin> plugin is installed) phrasing is consistent with implementation, verification, testing, and toolchain skills — no deviation found.

Graceful-degrade guarantee is tightened correctly. The Adapting section's new explicit prohibition ("NOT a cue to re-inline a fix/verify/review loop here") directly addresses the duplication risk this change removes. Wording is clear and unambiguous there.

Version and CHANGELOG. 0.6.2 → 0.7.0 with a BREAKING marker in CHANGELOG is correct for a behavior contract change in a 0.x plugin. CHANGELOG entry is complete — lists what was removed, what the new behavior is, and what was preserved.

---

[claude]

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

---

Security Review — PR #789

Tasks

• Load tools
• Fetch PR diff and read changed files
• Analyze for injection surfaces and logic flaws
• Analyze delegation pattern for authorization gaps
• Review shell command execution surfaces in SKILL.md
• Report findings

---

Findings

No security issues found.

This PR is a behavioral-contract and documentation refactor across seven files (SKILL.md, evals.json, checklist.md, plugin.json, CHANGELOG.md, two README.md). None of the changes introduce executable code at distribution or install time. Full analysis follows.

---

Pre-computed context shell commands (SKILL.md lines 11–13) — unchanged, no new risk

Current branch: !`git branch --show-current 2>/dev/null || echo "unknown"`
Working tree status: !`git status --porcelain 2>/dev/null | head -20 || echo "(unavailable)"`
Changed files (staged+unstaged): !`git diff --name-only HEAD 2>/dev/null || echo "none"`

These three !-prefixed commands execute in the consumer's shell at skill-load time and are identical to the pre-PR baseline — this PR does not touch them. All three are read-only introspection commands; no user-supplied input is interpolated into them, and their output is injected into the skill's model context as inert strings rather than re-executed. No new or changed risk.

$ARGUMENTS scope path handling — no injection surface

The user-supplied scope argument ($ARGUMENTS) is parsed by the model in natural language to extract a file/directory path and flags. The scope path is forwarded to read-only tools (Glob, Read) that respect the existing permission system. It is not interpolated into any shell command. No injection surface.

${CLAUDE_PLUGIN_ROOT} path references — system-controlled, no traversal

File references inside the skill use ${CLAUDE_PLUGIN_ROOT} — a system variable pointing to the plugin's isolated cache — not user-controlled input. The checklist copy instruction is a model directive, not a shell command. No path traversal surface.

--fix delegation pattern — pointer-only, no authorization escalation

The PR's core change replaces inline fix/verify/self-review/retro phases with pointers to /implementation:implement and /verification:confirm. The SKILL.md text is unambiguous:

"a pointer into each lane, NOT an auto-run pipeline; the user drives them"

and:

"Model auto-invocation never supplies --fix on its own."

The Phase 3 findings passed to the downstream lanes are structured documentation data, not shell commands. The soft-dependency guard (when the plugin is installed) prevents the skill from referencing nonexistent targets. The net security effect is a reduction in blast radius versus the old inline phases — --fix no longer applies edits autonomously.

Config file merge paths — hardcoded, no runtime user control

The three config layer paths (~/.claude/codebase-health.md, .claude/codebase-health.md, .claude/codebase-health.local.md) are hardcoded. Config contents (globs, example-claims) are consumed by the model as structured data, not executed. Merge semantics (additive union, explicit opt-out) are unchanged by this PR. No injection surface.

New §5 Config-gap observations — user-gated, no auto-write

The new required section asks the model to identify config coverage gaps and offer to persist them via /codebase-health:setup apply. This is a suggestion flow requiring explicit user action, not an automatic config write. No authorization concern.

GitHub Actions, dependencies, supply chain — no changes

No .github/workflows/ files were modified. No package manifests or action pins were altered.

---

[claude]

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

---

Security Review — PR #789

Tasks

• Load tools
• Fetch PR diff and read changed files
• Analyze for injection surfaces, logic flaws, and dangerous patterns
• Analyze delegation pattern for authorization gaps
• Review shell command execution surfaces in SKILL.md
• Report findings

---

Findings

No security issues found.

This PR is a behavioral-contract and documentation refactor across seven files (SKILL.md, evals.json, checklist.md, plugin.json, CHANGELOG.md, two README.md files). None of the changes introduce executable code, new permissions, or new attack surfaces. Full analysis follows.

---

Pre-computed context shell commands (SKILL.md L11–13) — unchanged, no new risk

Current branch: !`git branch --show-current 2>/dev/null || echo "unknown"`
Working tree status: !`git status --porcelain 2>/dev/null | head -20 || echo "(unavailable)"`
Changed files (staged+unstaged): !`git diff --name-only HEAD 2>/dev/null || echo "none"`

These three !-prefixed commands execute in the consumer's shell at skill-load time and are identical to the pre-PR baseline — a diff against HEAD~3 confirms they are byte-for-byte unchanged. All three are read-only introspection commands; no user-supplied input from $ARGUMENTS is interpolated into them, and their output is injected into the skill's model context as inert strings rather than re-executed. No new risk.

$ARGUMENTS scope path handling — no injection surface, unchanged

The user-supplied scope argument ($ARGUMENTS) is parsed by the model in natural language to extract a directory/file path and flags (--fix, --docs-only, etc.). The scope path is forwarded to read-only tools (Glob, Read) that respect the existing permission system. It is not interpolated into any shell command. This code path is unchanged by this PR.

--fix delegation pattern — pointer-only; net reduction in blast radius

The old --fix implementation applied edits inline during Phases 4–7, meaning the model was directly writing files. This PR replaces that with an explicit prohibition:

"With --fix, present the Phase 3 findings and output an explicit user-directed suggestion to run /implementation:implement with those findings, then /verification:confirm. Do NOT auto-invoke either skill — the user drives both."

The Phase 3 findings passed as context to those downstream lanes are structured documentation data (a drift table with file:line references), not shell commands or executable input. The soft-dependency guard (when the plugin is installed) prevents the skill from referencing nonexistent targets. The net security effect is a reduction in blast radius — --fix no longer applies edits autonomously, it produces a user prompt.

New §5 Config-gap observations — user-gated, no auto-write

The new required Phase 3 section (config-gap observations) asks the model to identify dimensions or globs worth adding to the tracked config and offer to persist them via /codebase-health:setup apply. This is an offer requiring explicit user action, not an automatic invocation or file write. No authorization concern.

${CLAUDE_PLUGIN_ROOT} path references — system-controlled, no traversal

File references inside the skill use ${CLAUDE_PLUGIN_ROOT} — a system variable pointing to the plugin's isolated cache, not a user-supplied value. The checklist copy instruction (${CLAUDE_PLUGIN_ROOT}/skills/audit/templates/checklist.md) is a model directive, not a shell command. No path traversal surface.

Config file merge paths — hardcoded, no runtime user control

The three config layer paths (~/.claude/codebase-health.md, .claude/codebase-health.md, .claude/codebase-health.local.md) are hardcoded string literals. Config contents (globs, example-claims) are consumed by the model as structured natural-language data and not executed. Merge semantics (additive union, explicit opt-out) are unchanged by this PR.

GitHub Actions, dependencies, supply chain — no changes

No .github/workflows/ files were modified. No package manifests, action pins, or dependency declarations were altered.

---

[claude]

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

---

PR Review

Tasks

• Load tools
• Read changed files and diff
• Verify prior review findings against HEAD
• Check for additional issues
• Post findings

---

Summary

All three correctness issues flagged in the earlier automated reviews are resolved in the HEAD commit (d13ab21): eval 4 now asserts §5 in both expected_output and expectations, eval 5 asserts §5 for the clean-run case, and the --fix delegation language is unambiguous. One new correctness issue found in a fresh read — a subtle conflict within SKILL.md between the fix-gate block and the Remediation section.

---

Issues

Fix gate says "present the summary count" but Remediation section says "present the Phase 3 findings" — model will get conflicting instructions

SKILL.md L241:

With --fix: present the summary count, then hand off to the remediation lanes below.

SKILL.md L263–264:

With --fix, present the Phase 3 findings and output an explicit user-directed suggestion...

"Present the summary count" (a bare number like "Found 3 findings") and "present the Phase 3 findings" (the full table, non-issues, drift patterns, fix priority, enforcement escalation, and config-gap observations) are two different outputs. Since /implementation:implement needs the full findings table to do anything useful, a model that reads L241 and produces only a count before handing off will silently break the delegation contract.

Eval 7's expected_output also says "presents the Phase 3 findings" — so L241 is the odd one out. The fix is to align L241 with the rest of the document: "present the Phase 3 findings" (or equivalently "present the full Phase 3 report"). Fix this →

---

Prior findings — now resolved in HEAD

Finding	Prior review	Status
Eval 4 expected_output omitted §5 config-gap observations	Reviews 1 & 2	✅ Fixed — expected_output now lists "config-gap observations"; expectations has a 5th entry
--fix "hand the findings to" ambiguous about auto-invocation	Reviews 1 & 2	✅ Fixed (d13ab21) — now "Do NOT auto-invoke either skill — the user drives both"
Eval 5 missing §5 assertion for clean run	Review 2 (advisory)	✅ Fixed — expectations now includes the config-gap-even-on-clean-run assertion

---

Notes (non-blocking)

Eval 7 array position. The entry with id: 7 is positioned between id: 2 and id: 3 in the JSON array, while the rest of the file is ordered by ID. IDs are opaque so this is runtime-safe, but moving it to the end of the array would restore the file's established ordering convention and avoid confusing future diffs by position.

Eval 5 expected_output prose still omits §5. The machine-checked expectations array now correctly asserts config-gap observations on a clean run, but the expected_output prose string stops at "proof of thoroughness" without mentioning §5. Since expectations is the eval contract that matters for automated checking, this is low-severity drift — but aligning the prose to match would make it fully self-consistent.

Soft-dependency guard pattern. Confirmed against the fleet: (when the <plugin> plugin is installed) phrasing in SKILL.md, checklist.md, and README matches implementation, verification, and other lanes exactly. No deviation.

Version, CHANGELOG, lockstep surfaces. 0.6.2 → 0.7.0 with a BREAKING marker is correct for a contract-level behavior change. All parallel surfaces (description in plugin.json, root README.md catalog entry, plugins/codebase-health/README.md, checklist.md, evals.json, SKILL.md frontmatter) moved in lockstep — no surface left on the old "fixes or presents for review" language.

---

[claude]

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

---

Security Review — PR #789

Tasks

• Load tools
• Fetch PR diff and read changed files
• Analyze for injection surfaces, logic flaws, and dangerous patterns
• Analyze delegation pattern for authorization gaps
• Review shell command execution surfaces in SKILL.md
• Report findings

---

Findings

No security issues found.

This PR is a behavioral-contract and documentation refactor across seven files (SKILL.md, evals.json, checklist.md, plugin.json, CHANGELOG.md, two README.md files). None of the changes introduce executable code, new permissions, or new attack surfaces. Full analysis follows.

---

Pre-computed context shell commands (SKILL.md L11–13) — unchanged, no new risk

Current branch: !`git branch --show-current 2>/dev/null || echo "unknown"`
Working tree status: !`git status --porcelain 2>/dev/null | head -20 || echo "(unavailable)"`
Changed files (staged+unstaged): !`git diff --name-only HEAD 2>/dev/null || echo "none"`

These three !-prefixed commands are byte-for-byte identical to the pre-PR baseline — the diff does not touch lines 11–13. All three are read-only introspection commands; no user-supplied input from $ARGUMENTS is interpolated into them, and their output is injected into the skill's model context as inert strings rather than re-executed. No new risk.

$ARGUMENTS scope path handling — no injection surface, unchanged

The user-supplied scope argument ($ARGUMENTS) is parsed by the model in natural language to extract a directory/file path and flags (--fix, --docs-only, etc.). The scope path is forwarded to read-only tools (Glob, Read) that respect the existing permission system; it is not interpolated into any shell command. Unchanged from the pre-PR baseline.

--fix delegation pattern — net reduction in blast radius

The old --fix implementation ran Phases 4–7 inline, where the model applied file edits directly. This PR replaces that with a text suggestion, enforced by explicit instruction at SKILL.md L263–267:

"Do NOT auto-invoke either skill — the user drives both. ... Never re-inline a fix/verify/review/retro loop here."

The Phase 3 findings passed as context to the downstream lanes are structured documentation data (a drift table with file:line references and human-readable descriptions) — not shell commands or executable input. The soft-dependency guard (when the plugin is installed) prevents the skill from referencing nonexistent targets. The net security effect is a reduction in blast radius: --fix no longer applies edits autonomously.

New §5 Config-gap observations — user-gated offer, no auto-write

The new required Phase 3 section (SKILL.md L220–223) asks the model to identify dimensions or globs worth adding to the tracked config and offer to persist them via /codebase-health:setup apply. This is a suggestion requiring explicit user action — the user sees the observations before any /codebase-health:setup apply is run, and the setup skill has its own user-confirmation flow. No authorization escalation or automatic write occurs.

disable-model-invocation: false and the read-only gate

The frontmatter allows model auto-invocation of this skill. With the old code, auto-invocation followed by a model supplying --fix could have triggered autonomous edits. The new code closes this in two layers: (1) SKILL.md L45 explicitly prohibits the model from supplying --fix autonomously; (2) even if that instruction were ignored, --fix now produces only a text suggestion rather than autonomous file writes. Defense-in-depth here is stronger post-PR than pre-PR.

Config file merge paths — hardcoded, no runtime user control

The three config layer paths (~/.claude/codebase-health.md, .claude/codebase-health.md, .claude/codebase-health.local.md) are hardcoded string literals — not user-supplied at runtime. Config contents (globs, example-claims) are consumed by the model as structured natural-language data and not executed. Merge semantics (additive union, explicit opt-out) are unchanged by this PR.

GitHub Actions, dependencies, supply chain — no changes

No .github/workflows/ files were modified. No package manifests, action pins, or dependency declarations were altered.

---

[claude]

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

---

PR Review

Tasks

• Load tools
• Read diff and changed files at HEAD
• Verify L241 fix and check for regressions
• Check for any new issues
• Post final review findings

---

Summary

The single correctness issue flagged in the previous automated review — the conflict at SKILL.md L241 between "present the summary count" (fix-gate block) and "present the Phase 3 findings" (Remediation section + eval 7) — is confirmed fixed in HEAD commit f0e9b98. All four correctness issues raised across the review cycle are now resolved. No new issues found in this commit.

---

Prior findings — all resolved

Finding	Raised in	Status
Eval 4 expected_output + expectations both omitted §5 config-gap observations	Reviews 1 & 2	✅ Fixed (d13ab21) — expected_output now lists "config-gap observations"; 5th expectation added
--fix "hand the findings to" ambiguous about auto-invocation model	Reviews 1 & 2	✅ Fixed (d13ab21) — "Do NOT auto-invoke either skill — the user drives both"
Eval 5 missing §5 assertion for the clean-run case	Review 2 (advisory)	✅ Fixed (d13ab21) — 4th expectation added: "…even on a clean run"
Fix-gate L241 said "summary count" while L263 + eval 7 said "Phase 3 findings"	Review 3	✅ Fixed (f0e9b98) — L241 now reads "present the full Phase 3 report"

---

No new issues

The HEAD change is a targeted, single-line fix at L241. No regressions found:

• SKILL.md L239–241 fix-gate block, L263–264 Remediation block, and evals.json eval 7 expected_output all now consistently describe --fix as presenting the full Phase 3 report before handing off — no "summary count" divergence remains.
• Graceful-degrade prohibition ("never re-inline a fix/verify/review/retro loop here"), soft-dependency guard pattern, and the user-drives-both contract are unaffected.
• All parallel surfaces (description, README, checklist, plugin.json, CHANGELOG) remain on the corrected 0.7.0 language from prior commits.

---

Residual advisory items (non-blocking, pre-existing)

These were raised in earlier reviews and remain open; neither blocks correctness:

1. Eval 7 array position — The entry with id: 7 (evals.json L31) sits between id: 2 and id: 3 in the array, while the rest of the file is ordered by ID. Runtime-safe (IDs are opaque), but moving it to the end of the array would restore the established ordering convention.

2. Eval 5 expected_output prose omits §5 — The machine-checked expectations array now correctly asserts config-gap observations on a clean run, but the expected_output prose stops at "proof of thoroughness" without mentioning §5. Low-severity — expectations is the binding eval contract — but aligning the prose would make it fully self-consistent.

---

This PR is ready to merge. All correctness issues from the review cycle are resolved. The change is architecturally sound, internally consistent, and moves all parallel surfaces in lockstep.

[kyle-sexton]

Post-green review classification (work lane) — final round:

Three review rounds ran on this PR as fix commits landed. All findings resolved:

Finding	Round	Classification	Disposition
Catalog drift (README.md marketplace block stale)	CI failure, pre-review	Branch-owned CI fix	Fixed — e97f4b6
Eval 4 output-contract missing §5 assertion	Rounds 1 & 2	VALID, non-blocking	Fixed — d13ab21
--fix "hand the findings to" ambiguous invocation model	Rounds 1 & 2	VALID, non-blocking	Fixed — d13ab21
Eval 5 missing §5 assertion (clean-run case)	Round 2 (advisory)	VALID, non-blocking	Fixed — d13ab21
SKILL.md L241 "summary count" contradicts L263-264 + eval 7 "Phase 3 findings"	Round 3 (fresh read)	VALID, branch-owned	Fixed — f0e9b98
Eval 7 array position; eval 5 expected_output prose omits §5	Rounds 1-3, repeated	Both explicitly low-severity/non-blocking (IDs are opaque; expectations is the binding eval contract, not expected_output prose)	Left alone, consistent with the lane's guidance the first two rounds

Round 3 (final) confirms no regressions and explicitly states "ready to merge." No DEFER-FORBIDDEN class applies anywhere in this PR's history (no data-loss, no fail-open gate — this touches an audit skill's own scope boundary, not a merge/security gate — and no security-posture finding was ever raised).

No inline review threads exist on this PR (GraphQL query returned empty) — nothing to resolve.

Handing off to the babysit lane for ongoing watch.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[claude]

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

---

Security Review — PR #789

Tasks

• Load tools
• Fetch PR diff and read changed files
• Analyze shell execution surfaces and injection vectors
• Analyze --fix delegation pattern for authorization gaps
• Analyze new §5 config-gap observations surface
• Check GitHub Actions / supply-chain changes
• Report findings

---

Findings

No security issues found.

This PR is a behavioral-contract and documentation refactor across seven files (SKILL.md, evals.json, checklist.md, plugin.json, CHANGELOG.md, two README.md files). No executable code, new permissions, or new attack surfaces are introduced. Full analysis follows.

---

Pre-computed context shell commands (SKILL.md L11–13) — unchanged, no new risk

Current branch: !`git branch --show-current 2>/dev/null || echo "unknown"`
Working tree status: !`git status --porcelain 2>/dev/null | head -20 || echo "(unavailable)"`
Changed files (staged+unstaged): !`git diff --name-only HEAD 2>/dev/null || echo "none"`

These three !-prefixed commands execute in the consumer's shell at skill-load time. The diff confirms they are byte-for-byte unchanged from the pre-PR baseline — the PR touches neither these lines nor any surrounding context that affects their evaluation. All three are read-only introspection commands; no user-supplied input from $ARGUMENTS is interpolated into them, and their output is injected into the skill's model context as inert strings rather than re-executed. No new risk.

$ARGUMENTS scope path handling — no injection surface, unchanged

The user-supplied scope ($ARGUMENTS) is parsed by the model in natural language and forwarded to read-only tools (Glob, Read) that respect the existing permission system. It is not interpolated into any shell command. This code path is unchanged.

--fix delegation — net reduction in blast radius

The old --fix ran Phases 4–7 inline, where the model applied file edits directly. This PR replaces that with an explicit text suggestion, enforced at multiple layers:

• SKILL.md L45: "Model auto-invocation never supplies --fix on its own."
• SKILL.md L264–267: "Do NOT auto-invoke either skill — the user drives both. … Never re-inline a fix/verify/review/retro loop here."
• SKILL.md L59–60: Graceful-degrade section explicitly prohibits the model from re-implementing autonomous fix behavior when delegation targets are absent.

The Phase 3 findings passed as context to the downstream lanes are structured documentation data (a drift table with file:line references and human-readable descriptions) — not shell commands or executable input. The soft-dependency guard (when the plugin is installed) prevents the skill from referencing nonexistent targets. The net security effect is a reduction in blast radius: --fix no longer applies edits autonomously; it produces a user-directed suggestion.

disable-model-invocation: false — no new risk

The frontmatter setting is unchanged from the pre-PR baseline. Under the old code, model auto-invocation followed by a model supplying --fix could have triggered autonomous file edits. The new code closes this in two layers: (1) SKILL.md L45 explicitly prohibits the model from supplying --fix autonomously; (2) even if that instruction were ignored, --fix now produces only a text suggestion rather than file writes. Defense-in-depth here is stronger post-PR.

New §5 Config-gap observations — user-gated, no auto-write

The new required Phase 3 section (SKILL.md L220–223) asks the model to identify dimensions or globs worth adding to the tracked config and offer to persist them via /codebase-health:setup apply. This is an offer requiring explicit user action — no automatic invocation or file write occurs. The observations themselves are config-coverage metadata (source globs, dimension names), not credentials or secrets.

${CLAUDE_PLUGIN_ROOT} path references — system-controlled, no traversal

File references inside the skill use ${CLAUDE_PLUGIN_ROOT} — a system variable pointing to the plugin's isolated cache, not a user-supplied value. The checklist copy instruction is a model directive, not a shell command. No path traversal surface.

GitHub Actions, dependencies, supply chain — no changes

No .github/workflows/ files were modified. No package manifests, action pins, or dependency declarations were altered.

---

[claude]

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

---

PR Review

Tasks

• Load tools
• Read changed files at HEAD
• Verify prior findings and advisory items
• Check for any new issues
• Post final review

---

Summary

All four correctness issues from the automated review cycle are confirmed fixed at HEAD. One new correctness issue found — the intro paragraph of plugins/codebase-health/README.md was not updated by this PR, leaving a stale description that directly contradicts the PR's central behavioral change.

---

New Finding

Intro paragraph of plugins/codebase-health/README.md still describes inline fixing — stale since the PR's core change

plugins/codebase-health/README.md L3–6 (unchanged from pre-PR baseline):

A Claude Code plugin for repo-wide drift auditing: it verifies that a codebase's factual claims — in docs, config, code, and architecture notes — still match reality. Every claim is checked against ground truth via a parallel per-file subagent fan-out, findings are severity-rated, and the audit either fixes them or presents them for review.

The PR correctly updated the skill table (L12–13) and the "The audit" section (L17+) to say "report read-only; remediation is delegated." But it left the intro blurb untouched. "Either fixes them or presents them for review" was accurate pre-PR when --fix applied edits inline. Post-PR the audit never fixes inline — it always reports read-only, and --fix only hands the findings to the dedicated lane. A reader landing on the plugin page gets a contradictory first sentence versus every other paragraph on the same page.

Suggested fix: replace "and the audit either fixes them or presents them for review" with "and the audit reports them read-only, delegating remediation to the implementation/verification lanes."

Fix this →

---

Prior findings — all resolved at HEAD

Finding	Raised in	Status
Eval 4 expected_output + expectations both omitted §5 config-gap observations	Reviews 1 & 2	✅ Fixed (d13ab21)
--fix "hand the findings to" ambiguous about auto-invocation model	Reviews 1 & 2	✅ Fixed (d13ab21)
Eval 5 missing §5 assertion for the clean-run case	Review 2 (advisory)	✅ Fixed (d13ab21)
Fix-gate L241 said "summary count" while L263 + eval 7 said "Phase 3 findings"	Review 3	✅ Fixed (f0e9b98)

The d13ab21 evals diff was reviewed in full. Eval 4's expected_output string now ends "…enforcement escalation, and config-gap observations…" and has a 5th expectation. Eval 5 has a 4th expectation covering the clean-run case. Eval 7 is correctly scoped to assert user-driven delegation only. SKILL.md L241 now reads "present the full Phase 3 report."

---

Residual advisory items (non-blocking, pre-existing from prior rounds)

1. Eval 7 array position — The entry with id: 7 is inserted between id: 2 and id: 3 in the JSON array (evals.json L31), while the rest of the file is ordered by ID. IDs are opaque — runtime-safe — but moving it to the end of the array restores the established ordering convention.

2. Eval 5 expected_output prose omits §5 — The expectations array now correctly asserts config-gap observations on a clean run (the binding eval contract), but the expected_output prose string stops at "proof of thoroughness" without mentioning §5. Low-severity — expectations is authoritative — but aligning the prose would make it fully self-consistent.

---

Security

No concerns. The --fix delegation is pointer-only and user-gated (confirmed via SKILL.md L263-267); the !-prefixed shell commands at L11-13 are unchanged read-only introspection; $ARGUMENTS is parsed in natural language and forwarded only to read-only tools. The PR's net security effect is a reduction in blast radius — --fix no longer applies file edits autonomously.

---

[kyle-sexton]

Babysit worker classification (SAFE tier) — re-derived from live GitHub state at HEAD f0e9b98:

#	Finding	Classification	Evidence	Reacted
1	plugins/codebase-health/README.md L5–6 intro still says the audit "either fixes them or presents them for review" — stale, contradicts the PR's report-only + delegated-remediation change	VALID — fixing	Confirmed at HEAD: L6 reads "either fixes them or presents them for review" while L14 skill-table says "report read-only; remediation is delegated to the implementation/verification lanes" and L17–29 "The audit" section says the same. The intro is the one surface the PR left on the old language. Branch-owned doc drift.	👍

Residual advisory items (eval 7 array position; eval 5 expected_output prose omitting §5) remain intentionally deferred — both explicitly labeled low-severity/non-blocking across every review round (IDs are opaque; expectations is the binding eval contract, not expected_output prose). No DEFER-FORBIDDEN class applies (docs-only, no data-loss/fail-open/security-posture finding).

[kyle-sexton]

Fixed in e543ed6: README intro L6 now reads "the audit reports them read-only — remediation is delegated to the implementation/verification lanes", in lockstep with the skill table (L14) and "The audit" section. Finding #1 addressed.

[claude]

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

---

Security Review — PR #789

Tasks

• Load tools
• Fetch PR diff and read changed files
• Analyze shell execution surfaces and injection vectors
• Analyze --fix delegation pattern for authorization gaps
• Analyze new §5 config-gap observations surface
• Check GitHub Actions / supply-chain changes
• Report findings

---

Findings

No security issues found.

This PR is a behavioral-contract and documentation refactor across seven files (SKILL.md, evals.json, checklist.md, plugin.json, CHANGELOG.md, two README.md files). No executable code, new permissions, or new attack surfaces are introduced. Full analysis follows.

---

Pre-computed context shell commands (SKILL.md L11–13) — unchanged, no new risk

Current branch: !`git branch --show-current 2>/dev/null || echo "unknown"`
Working tree status: !`git status --porcelain 2>/dev/null | head -20 || echo "(unavailable)"`
Changed files (staged+unstaged): !`git diff --name-only HEAD 2>/dev/null || echo "none"`

These three !-prefixed commands execute in the consumer's shell at skill-load time. The diff confirms they are byte-for-byte unchanged from the pre-PR baseline — lines 11–13 appear in no hunk. All three are read-only introspection commands with no $ARGUMENTS interpolation; their output is injected into the skill's model context as inert strings, not re-executed. No new risk.

$ARGUMENTS scope path handling — no injection surface, unchanged

The user-supplied scope ($ARGUMENTS) is parsed by the model in natural language and forwarded to read-only tools (Glob, Read) that respect the existing permission system. It is not interpolated into any shell command. This code path is unchanged by this PR.

--fix delegation — net reduction in blast radius

The old --fix ran Phases 4–7 inline, where the model applied file edits directly. This PR replaces that with an explicit text suggestion guarded at multiple layers:

• SKILL.md L45: "Model auto-invocation never supplies --fix on its own."
• SKILL.md L264–267: "Do NOT auto-invoke either skill — the user drives both. … Never re-inline a fix/verify/review/retro loop here."
• SKILL.md L59–60: Graceful-degrade section explicitly prohibits re-implementing autonomous fix behavior when delegation targets are absent.

The Phase 3 findings passed as context to downstream lanes are structured documentation data (file:line references and human-readable descriptions), not shell commands or executable input. The soft-dependency guard (when the plugin is installed) prevents the skill from referencing nonexistent targets. Net security effect: reduction in blast radius — --fix no longer applies file edits autonomously.

disable-model-invocation: false — no new risk

The frontmatter setting is unchanged from the pre-PR baseline. Under the old code, model auto-invocation followed by a model supplying --fix could have triggered autonomous file edits. The new code closes this in two layers: (1) L45 explicitly prohibits the model from supplying --fix autonomously; (2) even if that instruction were ignored, --fix now produces only a text suggestion rather than file writes. Defense-in-depth is stronger post-PR.

New §5 Config-gap observations — user-gated, no auto-write

The new required Phase 3 section (SKILL.md L220–223) identifies dimensions or globs worth adding to the tracked config and offers to persist them via /codebase-health:setup apply. This is an offer requiring explicit user action — no automatic invocation or file write occurs. The observations themselves are config-coverage metadata (source globs, dimension names), not credentials or secrets.

GitHub Actions, dependencies, supply chain — no changes

No .github/workflows/ files were modified. No package manifests, action pins, or dependency declarations were altered.

---

[claude]

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

---

PR Review — e543ed6

Tasks

• Load tools
• Read changed files at HEAD
• Verify e543ed6 README fix and check for regressions
• Confirm all prior review-cycle findings are resolved
• Post final review findings

---

Summary

The e543ed6 fix is correct and complete. All five correctness issues raised across the automated review cycle are resolved at HEAD. No new issues found. This PR is ready to merge.

---

e543ed6 Fix — Verified

plugins/codebase-health/README.md L3–6 now reads:

…findings are severity-rated, and the audit reports them read-only — remediation is delegated to the implementation/verification lanes.

This is internally consistent with:

• The skill table at L14: "report read-only; remediation is delegated to the implementation/verification lanes"
• The "The audit" section at L17–29
• The root README.md catalog entry at L97
• The plugin.json description
• SKILL.md frontmatter and all inline behavior sections

No regressions. The rest of the PR surfaces are unchanged from the prior passing commit (f0e9b98).

---

All Prior Findings — Resolved

Finding	Raised in	Status
Eval 4 expected_output + expectations both omitted §5 config-gap observations	Reviews 1 & 2	✅ Fixed (d13ab21)
--fix "hand the findings to" ambiguous about auto-invocation model	Reviews 1 & 2	✅ Fixed (d13ab21)
Eval 5 missing §5 assertion for the clean-run case	Review 2 (advisory)	✅ Fixed (d13ab21)
Fix-gate L241 "summary count" contradicted L263 + eval 7 "Phase 3 findings"	Review 3	✅ Fixed (f0e9b98)
README.md intro still described inline fixing post-refactor	Review 4	✅ Fixed (e543ed6)

---

No New Issues

The e543ed6 change is a targeted single-sentence fix. No new issues introduced:

• SKILL.md fix-gate (L239–241), Remediation section (L263–267), and eval 7 expected_output all consistently describe --fix as presenting the full Phase 3 report before a user-directed handoff — no divergence remains
• Soft-dependency guard pattern ((when the \implementation` plugin is installed)`) confirmed in SKILL.md, checklist.md, and README — matches the fleet convention exactly
• Graceful-degrade prohibition ("NOT a cue to re-inline a fix/verify/review loop here") is intact
• No GitHub Actions, dependency, or supply-chain changes

---

Residual Advisory Items (non-blocking, pre-existing across all review rounds)

1. Eval 7 array position — evals.json L31: entry with id: 7 sits between id: 2 and id: 3; the rest of the file is ordered by ID. Runtime-safe (IDs are opaque) — reordering to the end of the array would restore the file's established convention.

2. Eval 5 expected_output prose omits §5 — The expectations array correctly asserts config-gap observations on a clean run (the binding eval contract), but the expected_output prose string stops at "proof of thoroughness" without mentioning §5. Low-severity — expectations is authoritative.

Both items have been explicitly labeled low-severity/non-blocking across every review round and are intentionally deferred.

---

This PR is ready to merge. All correctness issues from the five-round review cycle are resolved. The refactor is architecturally sound, internally consistent, and all parallel surfaces move in lockstep with the behavioral change.