docs: document shell test-helper duplication and exit-code divergence as deliberate#853
docs: document shell test-helper duplication and exit-code divergence as deliberate#853kyle-sexton wants to merge 11 commits into
Conversation
… as deliberate Resolves the fork in #820: the per-plugin shell assert-helper copies (guardrails, claude-ops, source-control, repo-hygiene, work-items) and their divergent per-script exit-code taxonomies stay as-is rather than being consolidated into a shared mechanism. The existing lib/ + sync-*.sh vendoring precedent (hook-utils.sh) only covers clusters meant to stay byte-identical; these five copies are three genuinely different shapes already, so forcing them onto one shared library would mean a bigger, riskier redesign than the coupling it removes — and would cross the plugin-independence boundary the plugin philosophy already draws. Adds docs/conventions/shell-test-helpers/README.md as the owner doc, registers it in PLUGIN-PHILOSOPHY.md's convention registry, and adds a one-line pointer at each copy site (plus scripts/check-skill-portability.test.sh, which opted out for an unrelated reason: it is repo tooling, not a plugin). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K1V3gkrfSf75isB8MiDy3o
|
Claude finished @kyle-sexton's task in 2m 11s —— View job PR #853 Review
SummaryThis is a clean docs-only PR. No behavior changes anywhere — all 141 additions are comments and documentation. The architecture decision is well-reasoned and the execution is careful. A few minor observations below, none blocking. Documentation (
|
| Plugin | Before | After |
|---|---|---|
repo-hygiene |
0.4.4 | 0.4.5 |
source-control |
0.15.7 | 0.15.8 |
guardrails |
0.9.5 | 0.9.6 |
claude-ops |
0.17.1 | 0.17.2 |
work-items |
0.18.1 | 0.18.2 |
The merge-order race condition flagged in the PR description (competing bumps in #826, #839, #840, #844) is real, but it's correctly flagged and requires a rebase after those land — not a defect in this PR.
CHANGELOG entries
All five entries follow the established format: ## [x.y.z] → ### Documentation → one-paragraph description ending with "No behavior change." Consistent across all five plugins. No issues.
Relative path correctness
- README links (
../../../plugins/...) are correct fromdocs/conventions/shell-test-helpers/(three levels up to repo root). docs/PLUGIN-PHILOSOPHY.mdlink (conventions/shell-test-helpers/README.md) is correct relative todocs/.- Shell file pointer comments use repo-root-anchored prose ("at the repo root"), appropriate for shell comments where relative paths would be meaningless.
Verdict
Approve. No correctness issues, no security concerns, no behavior changes. The one minor observation (Conformance section doesn't explicitly name check-skill-portability.test.sh) is a documentation clarity nit, not a defect. The architecture reasoning in the README is sound and gives future contributors a clear trigger for when to revisit the guardrails/claude-ops pair consolidation.
|
Claude finished @kyle-sexton's task in 1m 7s —— View job Security Review
No security issues found. This PR is verifiably documentation and comments only. Here is what each category of change does and why none carries security risk: Shell scripts (6 files modified): Each change adds exactly 3–4 lines of shell comments ( New markdown doc (
The attack surface of this diff is zero: no executable code paths are added or modified, no external resources are introduced, no trust boundaries are crossed, and no secrets or credentials appear anywhere in the changes. |
…de-ops, work-items Re-checked open PRs touching these plugins' plugin.json right before the version-bump collision protocol requires it: source-control carries #839 (0.15.8) and #840 (0.15.9), claude-ops carries #844 (0.17.2), work-items carries #826 (0.18.2) — all still open. Bumps this PR's claims one past each plugin's current highest open-PR claim (source-control 0.15.10, claude-ops 0.17.3, work-items 0.18.3) so this PR does not collide at merge time regardless of which sibling lands first. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K1V3gkrfSf75isB8MiDy3o
|
Claude finished @kyle-sexton's task in 1m 47s —— View job PR #853 Review
What changed since the last reviewThe branch gained one new commit —
The math is correct in each case: Documentation (
|
|
Claude finished @kyle-sexton's task in 1m 19s —— View job Security Review — PR #853
No security issues found. This PR is verifiably comments and documentation only. The full diff was audited category by category: Shell scripts (6 files modified): Each change adds 3–4 lines of New markdown doc (
The effective attack surface of this diff is zero: no executable code paths added or modified, no external resources introduced, no trust boundaries crossed, no secrets or credentials present anywhere in the changes. |
Resolves conflicts on source-control and work-items plugin.json/CHANGELOG.md: main advanced source-control to 0.15.8 (#839, merged) and work-items to 0.18.2 (#826, merged) since this branch was last rebased. Also re-checked live open PRs at merge time and found work-items now carries a new open claim, #857 at 0.19.0 (Jira adapter) — re-bumped this branch's work-items claim from 0.18.3 to 0.19.1 to stay ahead of it. source-control's 0.15.10 and claude-ops's 0.17.3 remain valid (still one past #840/#860's 0.15.9 and #844/#860's 0.17.2 open claims, respectively). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K1V3gkrfSf75isB8MiDy3o
|
Claude finished @kyle-sexton's task in 2m 16s —— View job PR #853 Review (post-sync)
What's new since the last reviewThe branch gained one commit — The merge resolved conflicts on 1. 2. New open PR Version arithmetic at HEAD
All correct. One thing worth noting: work-items lands as a minor versionThe work-items claim is now 0.19.1 — a minor version release — for what is semantically a patch-level docs-only change. This is the unavoidable consequence of the "bump past highest open claim" strategy when the blocking PR (#857) happens to occupy a minor slot. The CHANGELOG entry calls it out explicitly:
This is not a defect — the strategy is correctly applied and self-documented — but it means work-items consumers will see PR description do-not-merge conditions are now partially staleThe PR body lists the holds as:
The conditions in the body don't match the current state. This won't block the merge — the Documentation and shell filesUnchanged since VerdictApprove. The merge is clean. Version math is correct. The work-items minor-version jump is documented and expected. The only action item before removing |
|
Claude finished @kyle-sexton's task in 1m 33s —— View job Security Review — PR #853
No security issues found. This PR is verifiably documentation and comments only. Full audit by category: Shell scripts (6 files modified): Each change adds 3–4 lines of New markdown doc (
GitHub Actions: No The effective attack surface of this diff is zero. |
repo-hygiene had no open-PR collision at this PR's original open, but #860 (chore/skill-shell-declaration) opened since and claims 0.4.5 exactly, matching this branch's prior claim. Bump to 0.4.6 to stay ahead of it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K1V3gkrfSf75isB8MiDy3o
|
Claude finished @kyle-sexton's task in 48s —— View job Security Review — PR #853
No security issues found. This PR is verifiably documentation and comments only. Full audit by category: Shell scripts (6 files modified): Each change adds 3–4 lines of New markdown doc (
GitHub Actions: No The effective attack surface of this diff is zero. |
|
Claude finished @kyle-sexton's task in 1m 25s —— View job PR #853 Review (post-sync)
What's new since the last reviewOne new commit: PR #860 ( Only two files changed: Version arithmetic at HEAD
All correct. CHANGELOG updateThe prior
Consistent with how the same note was added for Documentation and shell filesUnchanged since Do-not-merge holds (updated)#860 is now a blocker for both
VerdictApprove. The fix commit is correct and minimal — two files, right version numbers, CHANGELOG updated in sync with the stated reason. All prior review findings unchanged. No correctness issues, no security concerns, no behavior changes anywhere in the diff. |
Re-syncs onto main after #860 (fleet-wide `shell: bash` declaration sweep) merged, which bumped source-control to 0.15.9, claude-ops to 0.17.2, and repo-hygiene to 0.4.5. Resolved conflicts on those three plugins' plugin.json and CHANGELOG.md; re-derived each held version against the new main plus a fresh live-open-PR check rather than assuming the prior numbers still held: - source-control 0.15.10: still one past main's 0.15.9 and #840's live 0.15.9 claim. - claude-ops 0.17.3: still one past main's 0.17.2 and #844's live 0.17.2 claim. - repo-hygiene 0.4.6: still one past main's new 0.4.5 (#860, now merged); no open-PR collision remains for this plugin. - work-items 0.19.1 and guardrails 0.9.6 were untouched by this merge (no conflict) and remain correct against #857/#861 (both still 0.19.0, open) and no open collision, respectively. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K1V3gkrfSf75isB8MiDy3o
|
Claude finished @kyle-sexton's task in 2m 20s —— View job PR #853 Review (post-sync)
What's new since the last reviewOne new commit: The merge pulled in Version arithmetic at HEAD
All correct. The merge commit message documents the re-derivation reasoning against the new main baseline — the logic holds up. One minor observation: claude-ops CHANGELOG attributionThe Do-not-merge hold picture (updated after this sync)With
Before removing Documentation and shell filesUnchanged since VerdictApprove. The merge is clean. Version arithmetic is correct against the new main baseline established by |
|
Claude finished @kyle-sexton's task in 1m 7s —— View job Security Review — PR #853
No security issues found. All 18 changed files were read and audited. This PR adds comments and documentation only — no new executable code paths anywhere in the diff. Category-by-category breakdown: Shell scripts (6 files modified): Each change adds 3–4 lines of New markdown doc (
GitHub Actions: No The effective attack surface of this diff is zero. |
#840 rebased since this branch's last check and re-derived its own claim from 0.15.9 to 0.15.10, so this PR's prior 0.15.10 now collides exactly. Bump to 0.15.11 (one past #840's current live claim). claude-ops (0.17.3), work-items (0.19.1), repo-hygiene (0.4.6), and guardrails (0.9.6) were independently re-verified against current main and all live open PRs and remain correct — no change needed for those four. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K1V3gkrfSf75isB8MiDy3o
#857 (Jira Cloud adapter) merged into main since this branch's last rebase, bumping work-items to 0.19.0. Resolved the resulting conflict, keeping this branch's 0.19.1 (already one past both the new main and #861's still-open 0.19.0 claim, so no further bump needed). Re-verified all 5 plugins fresh against current main and every live open PR (#840 0.15.10, #844 0.17.2, #861 0.19.0): source-control 0.15.11, claude-ops 0.17.3, repo-hygiene 0.4.6, and guardrails 0.9.6 all remain correct with no change required. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K1V3gkrfSf75isB8MiDy3o
An unrelated claude-ops docs PR (#877, closing #864) merged since this branch's last rebase and landed at exactly 0.17.3 — the same version this branch was claiming. Re-bumped claude-ops to 0.17.4 to clear the exact collision (plugin.json auto-merged to an identical-looking "0.17.3" string without flagging a conflict, since both sides happened to write the same literal text for unrelated reasons; caught by diffing main's version history directly, not by trusting a clean auto-merge). CHANGELOG.md split into two sections (0.17.3 for #877's actual change, 0.17.4 for this PR's). Also swept for new open PRs against all 5 plugins post-merge: #870 (feat/800-audit-instructions) newly touches repo-hygiene (claims 0.4.5) and claude-ops (claims 0.17.2) — both below this branch's current claims (0.4.6 / 0.17.4), no collision. source-control (0.15.11), work-items (0.19.1), and guardrails (0.9.6) re-confirmed against current main and every live open PR (#840 0.15.10, #844 0.17.2, #861 0.19.0) — unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K1V3gkrfSf75isB8MiDy3o
|
Claude finished @kyle-sexton's task in 2m 12s —— View job PR #853 Review (post-sync)
What's new since the last reviewTwo new commits since
Notable: silent auto-merge collision on claude-ops
The fix is correct: claude-ops bumped to This catch was non-trivial: without the manual diff, a clean merge would have silently shipped two unrelated changes under a single version number. Worth flagging as a process observation, not a defect: the "bump past highest claim" strategy is necessary but not sufficient when a sibling lands at exactly your claimed version — the merge itself won't warn you. Version arithmetic at HEAD
All correct. CHANGELOG structure
Do-not-merge holds (updated)
Before removing Documentation and shell filesUnchanged since VerdictApprove. The two new commits are correct and handle a genuinely tricky edge case (silent same-string auto-merge) well. Version arithmetic is clean across all five plugins. The CHANGELOG split for claude-ops is the best outcome possible given the collision. No correctness issues, no security concerns, no behavior changes anywhere in the diff. |
|
Claude finished @kyle-sexton's task in 1m 10s —— View job Security Review — PR #853 (post-sync)
No security issues found. All 18 changed files were read and audited against the HEAD commit ( Shell scripts (6 files modified): Each change adds 3–4 lines of New markdown doc (
GitHub Actions: No The effective attack surface of this diff is zero. |
claude-ops: #844 rebased and re-derived its claim from 0.17.2 to 0.17.4, colliding exactly with this branch's prior 0.17.4. Bump to 0.17.5. source-control: a new PR, #882 (fix/511-babysit-self-identity-decouple), claims 0.16.0 — a minor bump above this branch's prior 0.15.11 and above #840's 0.15.10. Bump to 0.16.1 to stay ahead of both. repo-hygiene (0.4.6), guardrails (0.9.6), and work-items (0.19.1) were re-verified fresh against current main and every live open PR (main: repo-hygiene 0.4.5, guardrails 0.9.5, work-items 0.19.0; #861 0.19.0) and remain correct, no change needed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K1V3gkrfSf75isB8MiDy3o
|
Claude finished @kyle-sexton's task in 1m 39s —— View job PR #853 Review (post-sync)
What's new since the last reviewOne new commit: Two plugins were affected — claude-ops: source-control: A newly opened Version arithmetic at HEAD
All correct. Notable: second minor-version jump in the series
The source-control CHANGELOG now skips from CHANGELOG attribution accuracy
Do-not-merge holds (updated)
Before removing Documentation and shell filesUnchanged since VerdictApprove. The fix commit is correct and minimal — four files, version math checks out, CHANGELOG attribution updated to match the live collision state. The source-control minor-version jump is documented and expected. No correctness issues, no security concerns, no behavior changes anywhere in the diff. |
|
Claude finished @kyle-sexton's task in 1m 23s —— View job Security Review — PR #853
No security issues found. All 18 changed files were read and audited against HEAD ( Shell scripts (6 files modified): Each change adds 3–4 lines of New markdown doc (
GitHub Actions: No The effective attack surface of this diff is zero. |
#844 (claude-ops) merged into main at exactly 0.17.4, which this branch's prior 0.17.5 was already one past — kept as-is, no re-bump needed for the number itself, just resolved the resulting plugin.json/CHANGELOG.md conflict. Full fresh collision sweep after the merge found claude-ops now has NO open-PR collision at all. It also found #861 (work-items) re-derived its own claim from 0.19.0 to 0.20.0 since the last check (following #857's Jira-adapter minor bump into main) — colliding with this branch's prior 0.19.1. Re-bumped work-items to 0.20.1. source-control (0.16.1, held behind #882's 0.16.0 and #840's 0.15.10), repo-hygiene (0.4.6, no collision), and guardrails (0.9.6, no collision) re-verified against current main and all live open PRs — unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K1V3gkrfSf75isB8MiDy3o
|
Claude finished @kyle-sexton's task in 2m 43s —— View job PR #853 Review (post-sync)
What's new since the last reviewOne new merge commit: The merge pulled in 1. claude-ops conflict resolved without re-bump. 2. work-items re-bumped 0.19.1 → 0.20.1. Version arithmetic at HEAD
All correct. Notable: work-items minor-version jump is now the largest in the series
Do-not-merge holds (updated)With
Before removing Documentation and shell filesUnchanged since VerdictApprove. The merge commit is correct and self-documenting. Version arithmetic checks out at HEAD. The work-items minor-version jump is documented and expected. claude-ops has cleared its last open-PR hold. The only remaining gate before unlabeling is confirming #840, #861, and #882 are settled — verify fresh against live open PRs, not this snapshot. |
|
Claude finished @kyle-sexton's task in 1m 14s —— View job Security Review — PR #853
No security issues found. All 18 changed files were read and audited against HEAD ( Shell scripts (6 files modified): Each change adds 3–4 lines of The two files with the most security-sensitive pre-existing bodies were read in full to confirm the change boundary:
The New markdown doc (
GitHub Actions: No The effective attack surface of this diff is zero. |
#882 (source-control) merged into main at 0.16.0, which this branch's prior 0.16.1 was already one past — resolved the resulting plugin.json/CHANGELOG.md conflict, kept as-is. Full fresh collision sweep after the merge found source-control now carries TWO additional open PRs beyond #840: #895 (fix/548-babysit-worktree-head-safety, claims 0.15.10) and #898 (feat/399-shared-worktree-helper, claims 0.17.0 — a minor bump above this branch's prior 0.16.1). Re-bumped source-control to 0.17.1 to stay ahead of all three (#898's 0.17.0, #895's and #840's 0.15.10). claude-ops (0.17.5, no collision), work-items (0.20.1, held behind #861's 0.20.0), repo-hygiene (0.4.6, no collision), and guardrails (0.9.6, no collision) re-verified against current main and all live open PRs — unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K1V3gkrfSf75isB8MiDy3o
|
Claude finished @kyle-sexton's task in 3m 9s —— View job PR #853 Review (post-sync)
What's new since the last reviewOne new commit: The merge pulled in main changes including Version arithmetic at HEAD
All correct. CHANGELOG entries for all five plugins match their plugin.json versions. Notable: source-control now has the largest version gap in this PR's history
Documentation and shell filesUnchanged since Do-not-merge holds (updated)
Before removing VerdictApprove. The merge commit is correct. Version arithmetic checks out at HEAD. The source-control version gap (0.16.0 → 0.17.1) is the largest yet but is documented and correctly attributed. The remaining gate before unlabeling is confirming the four open holds are settled — verify fresh, not from this snapshot. |
|
Claude finished @kyle-sexton's task in 1m 21s —— View job Security Review — PR #853
No security issues found. All 18 changed files were read and audited against HEAD ( Shell scripts (6 files modified): Each change adds 3–4 lines of The two files with the most security-sensitive pre-existing bodies were read in full to confirm the change boundary:
The New markdown doc (
GitHub Actions: No The effective attack surface of this diff is zero. |
…th live labels (#922) ## Summary `plugins/work-items/skills/triage/SKILL.md` had two docs-match-reality gaps against shipped state: - The "Attention view (no number)" bucket list named only the status-axis raw marker (`status:needs-triage`), leaving a repo that files raw intake on the priority axis (`priority:needs-triage`, per PR #802's already-merged dual-axis Scope wording) invisible to the no-arg attention view. - `status:needs-decision` is a live, actively-used label (e.g. issue #505, per CHANGELOG `[0.16.1]`/`#562`), and the closing invariant already referenced it as a routing outcome that clears the raw marker — but the state machine itself never introduced it as a side exit, unlike `needs-info`, human-gated, and close. ## Fix - Attention-view bucket 2 now reads "Raw marker — `status:needs-triage` / `priority:needs-triage`, whichever axis the repo files it under", mirroring the Scope section's exact dual-axis wording (SKILL.md line 35). - The "Side exits from any state" sentence and the state-machine ASCII diagram now name `status:needs-decision` ("awaiting a human or maintainer judgment call" — the label's live GitHub description) alongside the existing side exits. - Version-bumped `work-items` 0.21.0 → 0.21.1 (patch, matching the `#562` precedent for the same doc-only shape) with a matching CHANGELOG entry. Scope note: the raw-state table row (line 48, `unlabeled or status:needs-triage`) is intentionally left single-axis — broader raw-marker axis canonicalization across that table, evals, and reference docs is issue #818's job, not this one. Also intentionally out of scope: adding `needs-decision` to step 2's "Target state" list or step 5's outcome table — doing so would require defining when triage applies it vs. the human-gated role, which the label's live usage does not settle (it appears both with and without `needs-human` across open issues) and which this issue scoped as pure docs-match-reality, not a design decision. ## Verification - Confirmed PR #802's merged Scope-section wording verbatim (`gh pr view 802`): "carrying the raw marker (`status:needs-triage` / `priority:needs-triage`, whichever axis the repo files it under)" — the attention-view bucket now mirrors this exactly. - Confirmed `status:needs-decision` is a live label via `gh issue list --label "status: needs-decision"` (21+ open/closed issues, e.g. #881, #718, #664, #551) and confirmed its GitHub description ("Awaiting a human or maintainer judgment call") used verbatim in the new SKILL.md text. - Confirmed issue #505's prior routing to `status:needs-decision` via the plugin's own CHANGELOG `[0.16.1]` entry (`#562`), which already treated `needs-decision` as a routing outcome in the closing invariant — this PR closes the gap between that reference and the state machine's own introduction of the label. - Traced the ASCII diagram's pipe/branch alignment by hand to confirm the new `status:needs-decision` line reads correctly as a side exit at the same indentation level as `close`. Closes #817 ## Related - #817 (this fix) - #802 (source of the dual-axis raw-marker wording mirrored here) - #505 (live example of `status:needs-decision` usage; see also CHANGELOG `#562`) - #818 — separate, unopened-PR item covering raw-marker axis canonicalization across the state table, evals, and reference docs; deliberately not touched here to avoid overlap. Re-checked immediately before opening this PR: no PR exists yet for #818. - #853 — open draft PR (labeled `do-not-merge`, stale at plugin version 0.19.0) also touches `plugins/work-items/.claude-plugin/plugin.json` and `CHANGELOG.md` for an unrelated shell-test-helper doc change. Re-checked immediately before opening this PR: no version-bump collision — #853 is stale relative to current `main` (0.21.0) and will need to rebase regardless of this PR. Claude-Session: https://claude.ai/code/session_01K1V3gkrfSf75isB8MiDy3o Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…triage docs + evals (#923) ## Summary The live raw-intake marker is applied on whichever axis a consuming repo files it under — `priority: needs-triage` (the current live label, applied by `.github/workflows/issue-triage-label.yml`) or `status: needs-triage`. `SKILL.md`'s "Scope: raw intake only" section already documents this dual-axis treatment (landed in #802), but several other spots in the same skill, its evals, and reference docs still described or asserted the marker as status-axis-only — a priority-axis label documented as status-only. ## Fix Brought every remaining occurrence in line with #802's already-merged dual-axis wording (`status:needs-triage` / `priority:needs-triage`, whichever axis the repo files it under): - `plugins/work-items/skills/triage/SKILL.md` — Triage-states table (`raw` row) and Attention-view bucket 2, both previously `status:needs-triage` only. - `plugins/work-items/reference/dogfood-filing.md` — step 4 ("Label `needs-triage`, then hand off") previously instructed filers to always `Apply status:needs-triage`; now applies the raw marker on whichever axis the repo files it under, resolved from the live label set. - `plugins/work-items/reference/label-taxonomy.md` and `plugins/work-items/reference/tracker-seam.md` — the axis-grammar tables listed `needs-triage` as a Status-axis example only; added a note that it is dual-axis with a pointer to SKILL.md's "Scope: raw intake only". - `plugins/work-items/skills/triage/evals/evals.json` — evals 4 and 5 asserted "clears `status:needs-triage`" without acknowledging the marker's dual-axis nature; reworded to "clears the raw-intake marker (here `status:needs-triage`)" so the assertion doesn't imply that's the only valid axis. - Version bump `work-items` `0.21.0` → `0.21.1` + matching `CHANGELOG.md` entry. No routing logic changed — this is a doc/eval consistency fix matching an already-ratified decision. ## Verification - `skill-quality:check` static contract gate (18 checks) over the `triage` skill: **PASS — 0 errors, 1 warning** (pre-existing "no Gotchas surface" warning, unrelated to this change). All 9 base-ref trigger phrases preserved; markdownlint clean. - `check-jsonschema` against the bundled `evals.schema.json`: `ok -- validation done`. - Manually cross-checked every edit against PR #802's diff to confirm the dual-axis phrasing (`status:needs-triage` / `priority:needs-triage`, whichever axis the repo files it under) is reused verbatim, not reinvented. - Confirmed the live label is priority-axis via `.github/workflows/issue-triage-label.yml` ("Applies the `priority: needs-triage` floor label..."). Closes #818 ## Related - #818 — this fix. The issue body already carries a ratified/defaulted decision (dual-axis treatment, maintainer-vetoable alternative was single-canonical-axis migration — not implemented here per the issue's explicit instruction), satisfying rule 4d's citation requirement. - #802 — the ratifying source for the dual-axis wording; this PR's phrasing is copied verbatim from its already-merged "Scope: raw intake only" section. - **Version-bump collision with PR #922 (issue #817):** #922 also bumps `work-items` `0.21.0` → `0.21.1` and touches `SKILL.md`'s state-machine/attention-view sections — the same lines this PR's dual-axis fix touches, since the marker lives in those sections. Both PRs are currently open against the same `0.21.0` base. Opening this PR as **draft + `do-not-merge`** until the version numbers and any overlapping `SKILL.md` lines are reconciled by whoever merges second (normal git-merge conflict resolution, per the collision-serialization protocol). - PR #853 (issue #820, unrelated) also touches `work-items/plugin.json` and `CHANGELOG.md` with a stale bump (`0.19.0` → `0.20.1`, already behind current `origin/main`) — noted for visibility, not a live collision with this change. Claude-Session: https://claude.ai/code/session_01K1V3gkrfSf75isB8MiDy3o
… FLEET_STATE_HOOK_UTILS source footgun (#907) ## Summary `fleet-state.sh` sourced `hook-utils.sh` from a caller-supplied `FLEET_STATE_HOOK_UTILS` path, guarded only by an existence check (`[[ -f "$HOOK_UTILS" ]]`). Any process able to set that env var — a project `.claude/settings.json` env block, an inherited shell environment, another hook that exports it — could redirect `source` at an arbitrary file, executed with the script's ambient permissions. The guard checked only that the path existed, never that it was the trusted default. This removes the override entirely and loads `hook-utils.sh` unconditionally from the script-relative plugin root. ## Fix `hook-utils.sh` is a fixed sibling shipped inside the plugin, always at `<plugin-root>/hooks/hook-utils.sh` relative to the script. The fix resolves it from the script's own location (`PLUGIN_ROOT_DEFAULT`, derived from `BASH_SOURCE`), which no environment variable can influence: ```bash HOOK_UTILS="$PLUGIN_ROOT_DEFAULT/hooks/hook-utils.sh" ``` `CLAUDE_PLUGIN_ROOT` is no longer consulted for this sibling file (it equals the script-relative root in production, and tests set it to a *fake* value to exercise marketplace self-resolution). It remains in use for marketplace self-resolution further down the script — that concern legitimately needs the install-metadata env var; locating a shipped sibling file does not. ### Options considered The issue proposed two remediations; I document both and why Option A wins: - **Option 1 — gate the override behind a test-context flag.** No such flag exists in this repo's convention; the test harness sets the individual `FLEET_STATE_*` override vars directly. Adding one would be a new abstraction invented solely to keep an override alive. - **Option 2 — validate the sourced path against an expected location before sourcing.** Once the trusted baseline is the script-relative default, an override is honored *only* when it canonically equals that default. Both existing test call sites already resolved to exactly that file, and an attacker path is rejected — so both branches source the identical file and the override cannot even inject a mock. The seam is provably dead code. - **Option A — remove the override (chosen).** Since a validated override could only ever equal the trusted default, the override carries no capability worth keeping. Removing it is the root-cause fix: it fully closes the arbitrary-file `source` with no new abstraction and no dead seam. The three now-redundant `FLEET_STATE_HOOK_UTILS=` lines in the test harness (which already pointed at the real file) and the doc-comment entry were removed with it. No behavior change in production: `PLUGIN_ROOT_DEFAULT` and `CLAUDE_PLUGIN_ROOT` resolve to the same directory there, so the same real `hook-utils.sh` loads. ## Verification **Existing test suite (`fleet-state.test.sh`) — full pass on the committed code:** ``` 23 cases, 0 failed ``` (Later re-runs on this Windows host intermittently exceed the command timeout on the `git init` and binary-copy cases from AV/disk contention; that is environmental, not a test failure — the suite completes green on the exact committed tree.) **Exploit path closed — malicious override in a non-test context:** ``` === BEFORE (origin/main) — malicious override in a NON-test context === PWNED: attacker code executed via FLEET_STATE_HOOK_UTILS === AFTER (fixed) — same malicious override === (no PWNED — override ignored, exploit closed) ``` The repro sets `FLEET_STATE_HOOK_UTILS` to a script that echoes a marker, then runs `fleet-state.sh --marketplace nope` with no test env. Before: the attacker file is sourced. After: the override is ignored and the trusted sibling loads. Closes #797 ## Related - #797 — this issue. - #795 (`fix/254-fleet-state-review-gaps`) — the security-review pass that surfaced this as a pre-existing, out-of-scope finding and deferred it here. - #853 (`docs/820-shell-helper-dedup-or-document`) — open draft (do-not-merge) that also bumps `plugins/claude-ops/.claude-plugin/plugin.json` + `CHANGELOG.md` (no code overlap with this PR). This PR is provisionally parked behind #853's unresolved cross-plugin version/CHANGELOG bump: a merge-drive must re-diff the version claim before either PR flips ready. Carrying `do-not-merge` until then. Claude-Session: https://claude.ai/code/session_01K1V3gkrfSf75isB8MiDy3o Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Summary
Closes the design fork raised in #820: whether the shell assert-helper duplicated across
5 plugins (and the divergent per-script exit-code taxonomies alongside it) should be
consolidated into a shared mechanism, or documented as deliberate. This PR documents.
Fix
docs/conventions/shell-test-helpers/README.mdas the owner doc explaining why theduplication and divergence stay as-is, and registers it in
docs/PLUGIN-PHILOSOPHY.md'sconvention registry table.
guardrails/hooks/guardrails-test-helpers.sh,claude-ops/hooks/claude-ops-test-helpers.sh,source-control/scripts/test-helpers.sh,repo-hygiene/skills/clean/scripts/lib/test-helpers.sh,work-items/tools/work-item-tracker/tests/lib.sh.scripts/check-skill-portability.test.sh, which opted outfor an unrelated reason (it's repo tooling, not a plugin, so no plugin assertion library
applies) — noted so the fork's second observation isn't left unexplained.
plugin.json+ adds aCHANGELOG.mdentry for every plugin whose helper file gainedthe pointer comment. Each has been re-derived from current
mainmany times as sibling PRsmerged or rebased mid-flight (see Related for the current picture):
repo-hygiene: 0.4.4→0.4.6source-control: 0.15.7→0.16.1claude-ops: 0.17.1→0.17.5work-items: 0.18.1→0.20.1guardrails: 0.9.5→0.9.6 (no open-PR collision at any point so far)No behavior change anywhere — comments and docs only.
Decision
Chose Option B: document the per-plugin duplication and exit-code divergence as deliberate.
No shared helper introduced.
Investigation before deciding:
under
lib/(e.g.lib/hook-utils.sh), copied — not imported — into each carrying plugin bya dedicated
scripts/sync-*.sh, tracked inscripts/cross-plugin-source-registry.txt, anddrift-checked by
scripts/check-cross-plugin-source-drift.sh --check.check-cross-plugin-source-drift.sh discoverconfirms it never even flags the fiveassert-helper files as a cluster candidate — they live at different paths per plugin and
are not byte-identical, so they fall outside that mechanism's scope entirely.
that drifted — a hook-contract shape (
guardrails/claude-ops:ok/bad,PASS/FAIL,make_sink/wait_for_sink), a skill-script shape (source-control/repo-hygiene:pass/fail,FAILED/CASE_NUM, file-existence assertions), and a vendored-seam shape (work-items: sameprimitives, but owned by the seam itself so it stays correct wherever the seam is resolved
from, independent of this repo's tooling).
existing
*.test.shonto it — a bigger, riskier change than the coupling it would remove,and it would cross the plugin-independence boundary
docs/PLUGIN-PHILOSOPHY.md's designboundary section already draws (no plugin imports files from a sibling plugin).
remove-path.sh0/1/2/3/4,git-tree-reset-batch.sh0/1/2 forwardinga child's 5/7,
check-skill-portability.sh0/1/2) encode genuinely different per-scriptcontracts, not arbitrary numbering — each script already documents its own
Exit:line, anda shared usage/exit helper would either flatten those contracts or grow per-caller branching.
guardrails-test-helpers.shandclaude-ops-test-helpers.share theone pair that already share a shape closely. If they converge to byte-identical, vendoring
just that pair through the existing
lib/+sync-*.sh+ registry mechanism is the smaller,precedented move — recorded as the trigger in the owner doc rather than acted on now.
Verification
shellcheckclean on all 6 edited shell files.check-skill-portability.test.shsuite: 16/16 pass.check-cross-plugin-source-drift.sh --check: no unregistered or drifted clusters.check-changelog-parity.sh --check: passes with every version bump.*.test.shthat sources an edited helper (repo-hygiene/clean, guardrails hooks,claude-ops hooks, work-items adapters/lib) — all green, confirming the comment-only edits
changed no behavior.
markdownlint-cli2andlycheeclean on the new and modified docs.Related
disk-hygieneis Python-only(
hygiene.py) with no shell assert-helper — the actual duplication lives in the 5 pluginsthe issue body names (repo-hygiene, source-control, guardrails, claude-ops, work-items) plus
root
scripts/. Treating the title as a triage typo (disk-hygiene vs. repo-hygiene, both"-hygiene" plugins with a
cleanskill) rather than touching disk-hygiene.do-not-mergeheld. This session has had exceptionally heavy concurrent-lane trafficagainst these same 5 plugins — this PR has been rebased/re-derived nine times as siblings
merged (fix(source-control): actionable owner/repo diagnostic on readiness-gate fetch failure #839, fix(work-items): resolve consumer-local gh-bot.sh wrapper independent of adapter location #826, feat(work-items): Jira Cloud adapter for the work-item-tracker seam #857, docs(claude-ops): record why lanes staleness probe stays a body instruction (#864) #877, feat(claude-config): add audit-instructions skill, superseding audit-model-fit (0.9.0) #870, test(claude-ops): cover morning-brief telemetry any=0 and rec-maxlen=0; harden usage() #844) or rebased in place (fix(source-control): invoke babysit mutation wrappers by bin/ path, not bare name (#484) #840, feat(source-control): dedicated --self/--extra-self self-identity flags for babysit snapshot #882, feat(work-items): add mini-SDLC pipeline-shape SSOT + wire work Step 5 to it #861, each more
than once). Current picture, last verified fresh at commit
e75e45ed(mergeable: MERGEABLE;all 5 plugins re-checked against current
mainAND every live open PR):repo-hygiene(claims 0.4.6): no open-PR collision. Main is at 0.4.5.source-control(claims 0.16.1): held behind feat(source-control): dedicated --self/--extra-self self-identity flags for babysit snapshot #882 (fix/511-babysit-self-identity-decouple,claims 0.16.0, open) and fix(source-control): invoke babysit mutation wrappers by bin/ path, not bare name (#484) #840 (claims 0.15.10, open). Main is at 0.15.9.
claude-ops(claims 0.17.5): no open-PR collision anymore — test(claude-ops): cover morning-brief telemetry any=0 and rec-maxlen=0; harden usage() #844 (the PR this leg waspreviously held behind) has merged, landing at exactly 0.17.4; this claim stays one past it.
Main is at 0.17.4.
work-items(claims 0.20.1): held behind feat(work-items): add mini-SDLC pipeline-shape SSOT + wire work Step 5 to it #861 (feat/613-mini-sdlc-pipeline-ssot). feat(work-items): add mini-SDLC pipeline-shape SSOT + wire work Step 5 to it #861itself has re-derived its claim twice as
mainmoved — from 0.19.0 up to 0.20.0 (followingfeat(work-items): Jira Cloud adapter for the work-item-tracker seam #857's Jira-adapter minor bump into main) — so this PR's claim moved from 0.19.1 to 0.20.1 to
stay ahead. Main is at 0.19.0.
guardrails(claims 0.9.6): no open-PR collision. Main is at 0.9.5.including siblings re-deriving their own claims upward more than once, an unrelated PR
landing at the exact same version by coincidence, and legs clearing and new ones opening.
Re-run the full collision protocol (
gh pr list --repo melodic-software/claude-code-plugins --state open --json number,headRefName,filesfiltered per plugin, AND a fresh diff of eachplugin's version on
mainsince this PR's last rebase) immediately before removingdo-not-merge— do not trust this snapshot.Closes #820
Work-class: C2 (mechanical) — attended triage 2026-07-23, operator-ratified. 🤖