Skip to content

repo-hygiene: clean's destructive guard stays armed after the run and matches quoted argument text #7333

Closed

Description

@kyle-sexton

Refs: #7332

Problem

/repo-hygiene:clean registers a session-scoped PreToolUse hook (skills/clean/scripts/destructive-guard.sh). In one session, a read-only scan-batch run at the start left the guard armed for over an hour. It then blocked two gh commands (an issue comment, and the gh issue create for this issue) whose --body merely contained the text git branch -D. It would equally have blocked a user-confirmed worktree branch deletion done through /source-control:worktree cleanup.

Two defects

  1. Lifetime: the guard outlives the clean run that armed it, so it gates unrelated work later in the session. It should disarm when the run ends, or arm only for tiers that mutate (not scan / scan-batch).
  2. Precision: it matches destructive patterns inside quoted argument text, not only in commands that execute. Per the hook-precision convention, the fix needs a stay-quiet test that fails before it (a gh issue comment --body '... git branch -D ...' case).

🤖 Generated with Claude Code

https://claude.ai/code/session_011WAGt7U42mcXW8yEoWdNZ2

Activity

  1. added
    needs-triageNot yet classified. Floor until a type and one priority tier are set.
    on Oct 12, 2026
  2. github-actions commented on Oct 12, 2026

    @github-actions
    Contributor

    Understood as two related defects in /repo-hygiene:clean's session-scoped destructive-guard.sh PreToolUse hook: (1) the guard stays armed for the rest of the session after a run finishes, including read-only scan/scan-batch runs, so it can gate unrelated later commands; (2) it matches destructive-looking text inside quoted argument strings (e.g. a gh issue comment --body containing git branch -D) rather than only text that would actually execute as a command. Reproduction, affected script paths, and the two fix directions (disarm on run end or scope to mutating tiers; add a stay-quiet regression test for the quoted-argument case) are all included, so this looks actionable as written. Referenced issue #7332 is a separate source-control:worktree scoping report from the same session and does not appear to be a duplicate of this one. No related open issue duplicates this report.

    Automated triage. A maintainer reviews it.

  3. removed
    needs-triageNot yet classified. Floor until a type and one priority tier are set.
    on Oct 12, 2026
  4. added theissue type on Oct 12, 2026
  5. kyle-sexton commented on Oct 13, 2026

    @kyle-sexton
    MemberAuthor

    Owner decision (2026-10-13): approved the recommendation.

    • How does the guard stop gating after the clean run ends? → A per-session run-open marker. The mutating tiers write it under CLAUDE_PLUGIN_DATA, keyed by the hook input's session_id, before their first --apply call; scan and scan-batch never write it. It is cleared when the run reaches its final outcome, with an idle timeout as a fallback. With no marker, the guard lets the command through. Keep the frontmatter hook registered; do not use once: true. Fix the false header claim at destructive-guard.sh:2 (skill hooks last the rest of the session). Apply the same design to fix(disk-hygiene): release the clean session belt when no run is open #6377 in its own PR.
      Basis: https://code.claude.com/docs/en/hooks (skill frontmatter hooks, once, session_id); origin/main plugins/repo-hygiene/skills/clean/scripts/destructive-guard.sh:2
    • How does it ignore destructive text inside quoted arguments? → Run is_destructive() over each segment's argv from hook::bash_parse_segments, not over the raw string; no second regex quote-stripper. The PowerShell branch needs separate handling or keeps raw matching. Lead with a stay-quiet case in destructive-guard.test.sh (gh issue comment --body '... git branch -D ...') that fails before the fix.
      Basis: origin/main lib/hook-utils.sh:4553, 4593; docs/conventions/hook-precision/README.md
    • Can repo-hygiene take the parser without new dependencies? → Yes, with one new scripts/shared-copies.txt line copying lib/hook-utils.sh to where the guard sources it (skills/clean/scripts/). Source the copy in the test and run it once to confirm its helpers.
      Basis: origin/main scripts/shared-copies.txt

    Next: an agent lane, via admission, opens one PR; it narrows a guard, so a human merges it.

    This was generated by AI.

  6. kyle-sexton commented on Oct 13, 2026

    @kyle-sexton
    MemberAuthor

    Owner ratified this admission in an attended session on 2026-10-13: work-class: scoped. It touches a guard, CI script, settings or trust surface, so an agent may build it but the owner merges the PR: put 'Hold: a human merges this' in the PR body.

  7. added
    work-class: scopedA briefed fix or small feature; blast radius bounded by the brief, tests exist.
    agent-readyFully specified and briefed; eligible for autonomous pickup from the frontier.
    on Oct 13, 2026
  8. removed
    agent-readyFully specified and briefed; eligible for autonomous pickup from the frontier.
    on Oct 14, 2026
  9. kyle-sexton commented on Oct 18, 2026

    @kyle-sexton
    MemberAuthor

    Decision recorded.

    Question: Admit this issue to the issue lane with the brief below, which implements the decision already discussed in this thread? (Premise holds: destructive-guard.sh:1-3 still claims it is active only while the skill runs, matches raw text, and repo-hygiene carries no hook-utils.sh copy; the owner's decision fixes the design (run-open marker, per-segment argv via hook::bash_parse_segments, one new shared-copies line); about 5 hand-edited files, 1 generated copy, 1 fragment; no open PR touches repo-hygiene.)

    Owner's answer: Yes. The owner was asked "Admit the 7 shared-copy issues and #6888 to the paced queue, recording each one's decision as a 'Decision recorded.' comment, and close #6227? #7373 and #6109 wait for their overlapping PRs." and answered "Yes (Recommended)" in an attended backlog-shaping session on 2026-10-18.

    Posted by AI from the owner's answer in that session.

  10. kyle-sexton commented on Oct 18, 2026

    @kyle-sexton
    MemberAuthor

    Agent Brief

    Type: Bug
    Summary: Arm repo-hygiene clean's destructive guard only while a mutating clean run is open, and match parsed command words instead of raw text, using a generated copy of the canonical lib/hook-utils.sh.

    Current behavior:
    The clean skill's frontmatter registers its destructive guard as a PreToolUse hook. Skill hooks stay active for the rest of the session, so after any clean run, read-only scan and scan-batch included, the guard keeps blocking unrelated commands. Its header claims it is active only while the skill is invoked. is_destructive() regex-matches the raw command string, so gh issue comment --body '... git branch -D ...' is blocked.

    Desired behavior:

    • A per-session run-open marker lives under the plugin data directory (CLAUDE_PLUGIN_DATA), keyed by the hook input's session_id. The mutating tiers write it before their first apply call; scan and scan-batch never write it. It is cleared when the run reaches its final outcome, and an idle timeout clears a stale one.
    • With no live marker for the session, the guard allows the command.
    • The frontmatter hook stays registered; no once: true.
    • The guard's header states that skill hooks last the rest of the session and that the marker scopes the guard.
    • For the Bash tool, is_destructive() runs over each segment's argv from hook::bash_parse_segments, not over the raw string, with no second quote-stripping regex. The PowerShell branch keeps raw matching or gets its own handling.
    • repo-hygiene receives lib/hook-utils.sh as a generated copy where the guard sources it, through one new line in the shared-copy registry.

    Key interfaces:

    • Run-open marker: location under the plugin data directory, keyed by session id, with an idle timeout.
    • Guard decision: allow when no live marker; Bash matching per parsed segment argv.
    • Shared-copy registry: one new line copying lib/hook-utils.sh into the clean skill's scripts directory.

    Acceptance criteria:

    • A stay-quiet case in the guard's test file (gh issue comment --body '... git branch -D ...' with a live marker) fails before the fix and passes after.
    • Tests show: with no marker the guard allows git branch -D x; with a live marker it blocks it; a marker past the idle timeout allows it; the scan tiers write no marker.
    • The existing block cases in the guard's test file still block with a live marker.
    • The test sources the generated copy and calls its parser once.
    • scripts/run-test-file.sh plugins/repo-hygiene/skills/clean/scripts/destructive-guard.test.sh passes.
    • The registry gains exactly one line; the lane then ran scripts/sync-shared-copies.sh to regenerate the copies, and scripts/sync-shared-copies.sh --check exits 0.
    • repo-hygiene has one new changelog fragment under .changes/repo-hygiene/ whose bump is not none.
    • If the clean skill's SKILL.md changes, plugins/skill-quality/scripts/check-skill.sh passes on it.

    Out of scope:


    This was generated by AI during backlog shaping. The owner approved its admission in an attended backlog-shaping session on 2026-10-18.

  11. added
    agent-readyFully specified and briefed; eligible for autonomous pickup from the frontier.
    on Oct 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    agent-readyFully specified and briefed; eligible for autonomous pickup from the frontier.priority: mediumReal value, no hard deadline; normal backlog flow.work-class: scopedA briefed fix or small feature; blast radius bounded by the brief, tests exist.

    Type

    Fields

    No fields configured for Bug.

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions