Skip to content

docs(disk-hygiene): name the hook note as the one primary source for guard values - #5508

Merged
kyle-sexton merged 8 commits into
mainfrom
docs/4215-clean-guard-values-one-source
Sep 30, 2026
Merged

kyle-sexton merged 8 commits into
mainfrom
docs/4215-clean-guard-values-one-source

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Closes #4215

Summary

The owner decision on #4215 (2026-09-29) chose option A: keep the UserPromptExpansion hook that injects hook_python and data_root, and accept the one denied probe in the no-hook path. This PR applies that decision to the skill wording.

Fix

  • plugins/disk-hygiene/skills/clean/SKILL.md: one primary source (the disk-hygiene guard values note) and one fallback (bare-python probe denied once, then the probe's data_root field).
  • plugins/disk-hygiene/skills/clean/reference/safety-model.md: records that the hook is the chosen delivery path and the denied probe in the no-hook path is an accepted residual.
  • disk-hygiene 0.28.19 -> 0.28.20 with a CHANGELOG entry.

Verification

  • scripts/check-changelog-parity.sh --check --check-order: passes.
  • scripts/validate-plugins.sh: all manifests and the catalog validated.
  • python3 -m unittest discover -s skills/clean/scripts -p 'test_*.py' in plugins/disk-hygiene: 546 tests OK.

Related

#4632 (hook), #4726 (probe fields), #5336 (untouched by this change).

🤖 Generated with Claude Code

kyle-sexton and others added 3 commits September 29, 2026 22:13
@kyle-sexton
kyle-sexton marked this pull request as ready for review September 30, 2026 02:31
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-30T02:32:25.646616Z 0a74e0c Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

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


Code review complete

  • Gather context (PR diff, changed files)
  • Read changed files in full
  • Review for correctness / CLAUDE.md alignment
  • Adversarially validate candidate findings
  • Post inline comments / summary

No findings. This is a scoped documentation change that applies the linked owner decision (#4215, option A) consistently across the three touched docs:

  • SKILL.md's guard-values guidance now names exactly one primary source (the disk-hygiene guard values hook note) and one fallback (the bare-python probe denial → rerun → data_root field), matching the PR description.
  • safety-model.md's added note (lines 272–273) is a direct corollary of the existing verified record just above it (Verified 2026-09-27 against https://code.claude.com/docs/en/hooks, lines 258–270), so it doesn't need its own separate four-part verification record under .claude/rules/skill-bodies-state-current-rules.md.
  • The plugin.json version bump (0.28.19 → 0.28.20) and CHANGELOG.md entry are consistent with each other and with the repo's Keep a Changelog format; no other file references the old version string.

One purely cosmetic item, not flagged as a finding since it's a formatting nit a prose-wrap linter would catch, not a correctness issue: SKILL.md line 126 is noticeably longer than the surrounding ~96-char wrapped lines in that bullet.

Verification claims in the PR body (changelog-parity script, validate-plugins.sh, unittest run) are author-reported and were not independently re-run by this review.

@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

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


Security review: no findings

Scope reviewed: plugins/disk-hygiene/.claude-plugin/plugin.json, plugins/disk-hygiene/CHANGELOG.md, plugins/disk-hygiene/skills/clean/SKILL.md, plugins/disk-hygiene/skills/clean/reference/safety-model.md (via gh pr diff 5508).

This PR only edits skill/reference prose and bumps the plugin version + changelog. No code in the guard/engine (e.g. hygiene.py) changes, so the actual enforcement — rejecting bare python/python3, requiring the guard-resolved absolute interpreter, denying every engine call when data_root is none/null, and "never submit a scan to learn either value" — is untouched.

I read the SKILL.md and safety-model.md edits under this skill's instruction-surface lens (diffs that delete/narrow/soften a standing instruction), since the PR does restructure the wording around how hook_python/data_root are sourced, and cross-checked against the instruction exception register (irreversible-action, secret-handling, data-integrity, external-publication, legal-compliance, agent-authority):

  • The old wording allowed a looser path ("when the note is absent, take both from the probe") that didn't strictly require the denied-bare-python step in every no-note case. The new wording (SKILL.md L118-128) collapses this to a single fallback: note present → use it; note absent → probe once with bare python, let the guard deny it and name the real interpreter, then rerun. That's a tightening of the no-note path, not a weakening.
  • "Never submit a scan to learn either value" is preserved verbatim.
  • The safety-model.md addition (L269-274) is additive — it documents the one-denied-probe residual as an accepted, owner-approved outcome rather than removing or loosening any check.

No Gate 0 class is deleted, narrowed, or softened here, and there's no injection, credential, or authorization surface in a docs/version-bump change of this shape. No security findings to report.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

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


Security review: no findings

Skip gate hit (criterion 3 — this head already has a successful security review that still applies): commit 824c680 is Merge origin/main into docs/4215-clean-guard-values-one-source (merge: 0a74e0c + 0f2a19d). gh pr diff 5508 shows the PR's actual diff against its merge-base is unchanged from the two commits already reviewed at 0a74e0c — the merge only pulls in unrelated origin/main history (machine-profile docs, architecture plugin, work-items, wizard, repo-hygiene changes) and touches none of this PR's four files. A prior CI security-review pass already covered this exact content (see the claude comment above timestamped 2026-09-30T02:41:25Z): no findings.

Independent confirmation of that prior conclusion, scoped to plugins/disk-hygiene/.claude-plugin/plugin.json, plugins/disk-hygiene/CHANGELOG.md, plugins/disk-hygiene/skills/clean/SKILL.md, plugins/disk-hygiene/skills/clean/reference/safety-model.md:

  • No code in the guard/engine (hygiene.py, engine_grammar.py) changes — the actual enforcement (rejecting bare python/python3, requiring the guard-resolved absolute interpreter, denying every engine call when data_root is none/null, never submitting a scan to learn either value) is untouched.
  • This is an instruction-surface edit (guard-value sourcing wording in SKILL.md/safety-model.md), reviewed under that lens per this skill's criteria: the old wording let the no-note fallback pull both hook_python and data_root straight from the probe; the new wording (SKILL.md L118-129) requires the denied bare-python probe step first in every no-note case, then a rerun to get data_root. That's a tightening, not a weakening, of the no-note path.
  • "Never submit a scan to learn either value" is preserved verbatim.
  • The safety-model.md addition (L272-274) is additive documentation of an accepted, owner-approved residual — it doesn't remove or loosen any check.
  • plugin.json/CHANGELOG.md are a version bump and changelog entry only.

No Gate 0 class (per the instruction exception register) is deleted, narrowed, or softened. No injection, credential, or authorization surface in a docs/version-bump change of this shape.

No security findings to report.
· Branch: docs/4215-clean-guard-values-one-source

@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

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


Code review complete

  • Gather context (PR diff via gh pr diff 5508, changed files)
  • Invoke /review:code-review and follow its instructions
  • Post findings as inline comments / summary

1 finding posted inline on SKILL.md lines 111-113:

Dropping "normally" from "The expansion of this command carries a disk-hygiene guard values note..." turns it into an unconditional claim, but the immediately following sentence ("Only when the note is absent, submit the probe...") only makes sense if the note can be missing — which safety-model.md's own text confirms it can ("a hook that fails prints nothing and leaves the skill on the kill-switch probe"). The paragraph now contradicts itself; the old "normally" hedge avoided this. An agent following the first sentence literally could treat the fallback branch as unreachable.

Everything else in this diff (plugin.json version bump 0.29.0→0.29.1, the matching CHANGELOG.md entry, and the safety-model.md addition recording the hook as the chosen primary path) is internally consistent and correctly applies the linked owner decision from #4215.

Note: this PR's HEAD is a merge commit (824c680) with origin/main as its second parent; I diffed via gh pr diff 5508 (the actual PR-vs-base diff) rather than comparing branch tips directly, since the shallow local clone has no common ancestor with the current origin/main ref and a raw tip-to-tip diff would have pulled in unrelated upstream changes to these same files.

Verification claims in the PR body (changelog-parity script, validate-plugins.sh, unittest run) are author-reported and were not independently re-run by this review.

Comment thread plugins/disk-hygiene/skills/clean/SKILL.md Outdated
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

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


Security review: no findings

Skip gate hit (criterion 3 — this head already has a successful security review that still applies): HEAD 121d44d is Merge origin/main into docs/4215-clean-guard-values-one-source. gh pr diff 5508 shows the PR's actual diff against its merge-base is unchanged from the two commits already reviewed at 824c680 — the merge only pulls in unrelated origin/main history (8e525fe PowerShell mutation-word check, dab0b0a Windows-drive-tmp guard) and touches none of this PR's four files. A prior CI security-review pass already covered this exact content (see the claude comment above timestamped 2026-09-30T04:51:50Z): no findings.

Independent confirmation, scoped to plugins/disk-hygiene/.claude-plugin/plugin.json, plugins/disk-hygiene/CHANGELOG.md, plugins/disk-hygiene/skills/clean/SKILL.md, plugins/disk-hygiene/skills/clean/reference/safety-model.md:

  • No code in the guard/engine (hygiene.py, engine_grammar.py) changes — the actual enforcement (rejecting bare python/python3, requiring the guard-resolved absolute interpreter, denying every engine call when data_root is none/null, never submitting a scan to learn either value) is untouched.
  • Read under the instruction-surface lens (docs/conventions/instruction-exception-register/README.md is present in this checkout and was consulted): the old no-note fallback could pull both hook_python and data_root straight from the probe without necessarily hitting the denied-bare-python step first. The new wording (SKILL.md L118-129) requires the denied bare-python probe in every no-note case before a rerun supplies data_root. That's a tightening of the no-note path, not a weakening — no protected class from the register is deleted, narrowed, or softened.
  • "Never submit a scan to learn either value" is preserved verbatim.
  • The safety-model.md addition (L272-274) is additive documentation of an accepted, owner-approved residual — it removes no check.
  • plugin.json/CHANGELOG.md are a version bump (0.28.19 → 0.28.20) and matching changelog entry only.

No injection, credential, or authorization surface in a docs/version-bump change of this shape.

No security findings to report.

(Note: a separate code-review pass on this same content flagged a prose-consistency issue — the dropped "normally" hedge on the first sentence of the SKILL.md L111-113 paragraph — as a maintainability/clarity finding, not a security one; it's out of scope for this lane.)
· Branch: docs/4215-clean-guard-values-one-source

@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

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


Code review complete

Skip gate hit (criterion 4 — this head already has a successful review from this lane that still applies): 121d44d is Merge origin/main into docs/4215-clean-guard-values-one-source, merging in 8e525fe (PowerShell quote-relief fix) and dab0b0a (guardrails open ( fix) — neither touches this PR's four files. git diff 824c680 121d44d -- plugins/disk-hygiene/skills/clean/SKILL.md plugins/disk-hygiene/skills/clean/reference/safety-model.md is empty: the content of both docs is byte-identical to the commit this lane already reviewed (see the claude code-review comment above, timestamped 2026-09-30T04:55:13Z).

That prior pass posted one inline finding, and it still stands, unresolved, at this HEAD:

  • SKILL.md:113: dropping "normally" makes "The expansion of this command carries a disk-hygiene guard values note..." an unconditional claim, but the next sentence ("Only when the note is absent, submit the probe...") only makes sense if the note can be missing — which safety-model.md itself confirms ("a hook that fails prints nothing and leaves the skill on the kill-switch probe"). The paragraph still contradicts itself as of this HEAD. Not re-posted as a new inline comment since it's the same finding on unchanged lines; see the existing inline thread for the suggested fix (restore a hedge, e.g. "normally carries").

No new findings from the merge itself — it's a routine origin/main pull with no conflicts touched in this PR's scope. plugin.json/CHANGELOG.md landed at version 0.29.2 (renumbered past the PR description's stated 0.28.19 → 0.28.20 by intervening merges from main), and the changelog entry text is consistent with that version — not a defect, just noting the description is now stale relative to the merged version number.

Verification claims in the PR body (changelog-parity script, validate-plugins.sh, unittest run) are author-reported and were not independently re-run by this review.
· Branch: docs/4215-clean-guard-values-one-source

kyle-sexton and others added 2 commits September 30, 2026 01:51
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

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


Security review: no findings

Scope reviewed: plugins/disk-hygiene/.claude-plugin/plugin.json, plugins/disk-hygiene/CHANGELOG.md, plugins/disk-hygiene/skills/clean/SKILL.md, plugins/disk-hygiene/skills/clean/reference/safety-model.md. git diff origin/main...HEAD --stat confirms this is still exactly the four files in the PR description (16 insertions / 9 deletions), matching the changed-files list.

What's new since the last security-review pass (121d44d, 2026-09-30T05:32:15Z): commit 7599823 ("hedge the guard values note as normally present") restored the word "normally" to the first sentence of the guard-values paragraph, addressing the code-review finding posted against this PR. Current text at SKILL.md L117-118:

The expansion of this command normally carries a disk-hygiene guard values note naming hook_python and data_root, resolved by the guard's own code; use both from the first call. Only when the note is absent, submit the probe once with bare python...

The HEAD 0410541 commit itself is Merge origin/main into docs/4215-clean-guard-values-one-source; git merge-base 0410541 origin/main resolves to origin/main's own tip (bfd57a0), confirming the merge pulled in nothing beyond what's already on main (the elevation/policy-rules-v2 work visible in git log/git show --stat on that merge commit is pre-existing main content reachable through the merge, not new content this PR introduces — git diff origin/main...HEAD isolates the PR's actual four-file, 16/9-line diff from that noise).

Independent read, under the instruction-surface lens (docs/conventions/instruction-exception-register/README.md present in this checkout and consulted):

  • Restoring "normally" is a pure prose hedge — it doesn't touch enforcement. The paragraph's actual rules are unchanged: bare python/python3 is still rejected, the note-absent fallback still requires the denied bare-python probe before a rerun supplies data_root, and "Never submit a scan to learn either value" is preserved verbatim.
  • No code in the guard/engine (hygiene.py, engine_grammar.py) changes. The actual enforcement — rejecting bare interpreters, requiring the guard-resolved absolute interpreter, denying every engine call when data_root is none/null — is untouched.
  • The safety-model.md addition (L279-281) is additive documentation of the hook as the chosen primary path and the one denied probe as an accepted residual — it removes no check.
  • plugin.json/CHANGELOG.md are a version bump (0.30.0 → 0.30.1) and matching changelog entry only.

No Gate 0 class from the instruction exception register is deleted, narrowed, or softened by this diff. No injection, credential, or authorization surface in a docs/version-bump change of this shape.

No security findings to report.

(The code-review lane's earlier finding on the dropped "normally" hedge is now addressed by 7599823; that's a code-review/maintainability matter, out of scope for this lane.)
· Branch: docs/4215-clean-guard-values-one-source

@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

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


Code review complete

  • Gather context (PR diff via git diff origin/main...HEAD, HEAD 0410541)
  • Invoke /review:code-review and follow its instructions
  • Post findings as inline comments / summary

Skip gate not applicable: the last successful pass from this lane (on 121d44d) left one finding still open — the dropped "normally" hedge on SKILL.md lines 111-113 — so this head needed a fresh review, not a skip.

No findings. Commit 7599823 ("hedge the guard values note as normally present") fixes that open finding: SKILL.md:118 now reads "The expansion of this command normally carries a disk-hygiene guard values note...", which makes the following "Only when the note is absent, submit the probe..." branch reachable again and consistent with safety-model.md's own statement that "a hook that fails prints nothing and leaves the skill on the kill-switch probe" (safety-model.md:271). The paragraph is now internally consistent.

Rechecked the rest of the diff (git diff origin/main...HEAD, 4 files, +16/-9, matching the PR description):

  • SKILL.md's fallback sequence (bare-python probe denied → names interpreter → rerun → data_root from the probe) is coherent and matches safety-model.md's description of the guard-computed values.
  • safety-model.md's added note (L279-280) is a direct corollary of the existing verified record just above it and doesn't need its own four-part verification record under .claude/rules/skill-bodies-state-current-rules.md — it states an internal design decision, not a restated external/volatile fact.
  • plugin.json (0.30.0 → 0.30.1) and the new CHANGELOG.md entry are consistent with each other and correctly ordered above [0.30.0]. (The PR description's stated 0.28.19 → 0.28.20 is stale relative to intervening merges from main; not a defect in the diff itself.)
  • MD013 (line length) is disabled repo-wide in .markdownlint-cli2.jsonc, so the one visually longer wrapped line at SKILL.md:118 is cosmetic only, not a lint or correctness issue.

Verification claims in the PR body (changelog-parity script, validate-plugins.sh, unittest run) are author-reported and were not independently re-run by this review.
· Branch: docs/4215-clean-guard-values-one-source

…-values-one-source

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

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


Code review complete

  • Gather context (PR diff via gh pr diff 5508, current HEAD bf56277)
  • Invoke /review:code-review and follow its instructions
  • Post findings as inline comments / summary

Skip gate hit (criterion 4 — this head already has a successful review from this lane that still applies): bf56277 is Merge remote-tracking branch 'origin/main' into docs/4215-clean-guard-values-one-source. gh pr diff 5508 shows the PR's actual diff is still exactly the same three commits (4b7ed60, 3ba198a, 7599823) across the same four files (16 insertions / 9 deletions) that the prior code-review pass on 0410541 already reviewed and cleared with no findings (see the claude comment above timestamped 2026-09-30T13:13:37Z). Confirmed on disk that SKILL.md:118 still reads "...normally carries a disk-hygiene guard values note...", the fix for the one finding this lane raised earlier in the PR's history (the dropped "normally" hedge, resolved by commit 7599823).

No new content for this lane to review at this HEAD.
· Branch: docs/4215-clean-guard-values-one-source

@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

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


Security review: no findings

Skip gate hit (criterion 3 — this head already has a successful security review that still applies). gh pr diff 5508 (GitHub's own base...head computation) shows the PR's actual diff is still exactly the four files from the description, +16/−9, and its content is byte-identical to what the prior security-review pass covered at HEAD 0410541 (2026-09-30T13:12:57Z, "No security findings to report"):

  • SKILL.md: same guard-values wording, including the "normally carries" hedge restored by 7599823.
  • safety-model.md: same additive note on the hook as chosen primary path and the denied probe as accepted residual.
  • plugin.json / CHANGELOG.md: only the version numbers differ (now 0.32.0 → 0.32.1, previously 0.30.0 → 0.30.1) because two intervening merges from main (580090f disk-image protection, 45bb22f playwright check skill) bumped the baseline version past this PR's own bump. Neither merge touches this PR's four files or their diff content — confirmed via git diff 0410541 bf56277 -- <the 4 files>, which shows only unrelated CHANGELOG.md/safety-model.md/SKILL.md sections from those other features, none overlapping this PR's lines.

Independent confirmation, scoped to plugins/disk-hygiene/.claude-plugin/plugin.json, plugins/disk-hygiene/CHANGELOG.md, plugins/disk-hygiene/skills/clean/SKILL.md, plugins/disk-hygiene/skills/clean/reference/safety-model.md:

  • No code in the guard/engine (hygiene.py, engine_grammar.py) changes — the actual enforcement (rejecting bare python/python3, requiring the guard-resolved absolute interpreter, denying every engine call when data_root is none/null, never submitting a scan to learn either value) is untouched.
  • Read under the instruction-surface lens (docs/conventions/instruction-exception-register/README.md present in this checkout and consulted): this diff's only wording change from the last-reviewed content is the already-restored "normally" hedge, which doesn't reopen the earlier finding — the note-absent fallback branch stays reachable and consistent with safety-model.md's own statement that a failed hook leaves the skill on the kill-switch probe. No protected class from the register is deleted, narrowed, or softened.
  • "Never submit a scan to learn either value" is preserved verbatim.
  • plugin.json/CHANGELOG.md are a version bump and matching changelog entry only.

No injection, credential, or authorization surface in a docs/version-bump change of this shape.

No security findings to report.
· Branch: docs/4215-clean-guard-values-one-source

@kyle-sexton
kyle-sexton merged commit adff838 into main Sep 30, 2026
18 checks passed
@kyle-sexton
kyle-sexton deleted the docs/4215-clean-guard-values-one-source branch September 30, 2026 15:32
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.

disk-hygiene:clean: hook interpreter and --data-root are only discoverable by provoking a guard denial

1 participant