Skip to content

feat(repo-fleet-hygiene): apply fleet action plans behind one gate - #2665

Merged
kyle-sexton merged 3 commits into
mainfrom
cursor/feat-fleet-apply-plan-f8b0
Aug 15, 2026
Merged

feat(repo-fleet-hygiene): apply fleet action plans behind one gate#2665
kyle-sexton merged 3 commits into
mainfrom
cursor/feat-fleet-apply-plan-f8b0

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Closes #2597

Summary

Adds repo-fleet-hygiene:apply, a sibling verb that consumes the existing fleet action-plan JSON behind one batch-wide confirmation gate. Audit stays read-only; execution is not a flag on audit-fleet.sh.

Fix

  • New skills/apply/ with apply-plan.sh: --plan-file required, dry-run default, --apply + interactive confirmation or --yes
  • Re-derives mutable OIDs / tips before destructive steps; fail-closed on drift
  • Preserves plan ordering (merged local branches before worktree cleanup)
  • Own allowed-tools surface so audit's Bash grant is not silently widened
  • README/SKILL contract updated; fleet version bumped; CATALOG regenerated
  • Tests for dry-run, confirmation stop, OID drift, ordering

Verification

bash plugins/repo-fleet-hygiene/skills/apply/scripts/apply-plan.test.sh
bash plugins/repo-fleet-hygiene/skills/audit/scripts/audit-fleet.test.sh

Related

Refs #2608 #2609 — action-plan prerequisite.
Refs #2605 — stranded-work delegation unblocked execution.

Open in Web Open in Cursor 

@kyle-sexton
kyle-sexton marked this pull request as ready for review August 15, 2026 03:42
@cursor

cursor Bot commented Aug 15, 2026

Copy link
Copy Markdown

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

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

@kyle-sexton
kyle-sexton enabled auto-merge (squash) August 15, 2026 03:43

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 715558fb9a

ℹ️ About Codex in GitHub

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

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

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/repo-fleet-hygiene/skills/apply/scripts/apply-plan.sh
Comment thread plugins/repo-fleet-hygiene/skills/apply/scripts/apply-plan.sh Outdated
Comment thread plugins/repo-fleet-hygiene/skills/apply/scripts/apply-plan.sh Outdated
kyle-sexton and others added 2 commits August 15, 2026 03:55
Add /repo-fleet-hygiene:apply with apply-plan.sh so a prior audit plan can
drive batched merged-local-branch deletion and ordered worktree cleanup
behind one confirmation, without widening audit-fleet.sh.

Closes #2597

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
Require audit provenance (generated_by/mode), audited canonicals, and
finding-backed action targets. Replace mapfile/${var,,}/tab-IFS decode
so empty prune fields and stock macOS Bash 3.2 work.

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
@cursor
cursor Bot force-pushed the cursor/feat-fleet-apply-plan-f8b0 branch from 715558f to b406289 Compare August 15, 2026 03:57
@claude

claude Bot commented Aug 15, 2026

Copy link
Copy Markdown

Claude finished @cursor[bot]'s task in 3m 37s —— View job


Claude Code Review

Todo

  • Gather context (PR diff, CLAUDE.md, REVIEW.md)
  • Run /review:code-review criteria against changed files
  • Check whether the automated Codex findings were addressed
  • Final summary

Summary

This PR ships in two commits. The second (b406289, "harden apply-plan validation and Bash 3.2") is a direct, targeted fix for all three issues the automated Codex review flagged on the first commit:

  • P1 — unvalidated plan artifacts (apply-plan.sh): the parser now requires generated_by: repo-fleet-hygiene/audit, mode: read-only, that each action's canonical matches an audited: true repository entry, and that every action target resolves to a real findings[] entry of a matching kind (plugins/repo-fleet-hygiene/skills/apply/scripts/apply-plan.sh#L101-L229). Verified this is exercised by new tests apply-plan.test.sh §6 ("rejects plan without generated_by", "rejects action target without audit finding") — a hand-authored action list with no backing finding is now correctly rejected before any git command runs.
  • P1 — Bash-4-only syntax: mapfile was replaced with a portable while read loop, and ${a,,}/${b,,} were replaced with tr '[:upper:]' '[:lower:]' (apply-plan.sh#L247-L252, #L370-L373). No remaining mapfile/,,}/readarray/declare -A usage in the script.
  • P2 — empty TSV fields collapsing: the Python→Bash handoff now delimits fields with ASCII Unit Separator (\x1f) instead of tabs, so an intentionally-empty ref_name/expected_oid for prunable-worktree/missing-worktree no longer shifts columns (apply-plan.sh#L84-90, #L560-577). Covered by the new §7 test asserting prunable-worktree decodes to [prune-worktrees] rather than being misread as a skipped merged-worktree action.

I independently read the full apply-plan.sh (763 lines) and its test suite (511 lines) rather than just trusting the fix commit's message, and confirmed the fixes match what they claim. I was not able to execute the test suite myself in this environment (Bash execution outside gh pr * is not permitted for this review lane), so test-passing is based on static reading of the assertions, not an observed run — the PR description's own verification commands are author-claimed and unverified by me.

Docs (README.md, skills/audit/SKILL.md, skills/apply/SKILL.md, security-review.md, CHANGELOG.md) were updated consistently with the new :apply verb, and the allowed-tools-pairing.test.sh guard now covers the apply skill's own script grant so it can't silently widen.

No new correctness/maintainability findings beyond what's already fixed. One residual note, intentionally not raised as a finding here: apply-plan.sh's provenance checks are self-referential (they validate internal consistency of the JSON, not that it was produced by a real prior audit run against real repository state) — the actual backstop against a maliciously-crafted plan remains the printed ordered plan + explicit one-time confirmation gate. Since this repo carries a claude-security-review workflow, that class of finding belongs to the security lane per this org's REVIEW.md split, not this code-review lane.

@github-actions

github-actions Bot commented Aug 15, 2026

Copy link
Copy Markdown

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

Rewrite the embedded Python OID regex without \\s, silence intentional
SC2310 predicate patterns, regenerate the skill cheat sheet, and drop the
extra CHANGELOG blank line that failed markdownlint.

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
@claude

claude Bot commented Aug 15, 2026

Copy link
Copy Markdown

Claude finished @cursor[bot]'s task in 1m 28s —— View job


Claude Code Review

Todo

  • Gather context (PR diff, prior review comments, CLAUDE.md)
  • Invoke /review:code-review and follow its instructions exactly
  • Check whether the third commit (since the last lane review) introduces new findings
  • Final summary

Summary

The lane already reviewed this PR at commit b406289 (see the comment above) and confirmed the three Codex-flagged P1/P2 issues (unvalidated plan artifacts, Bash-4-only syntax, TSV field collapsing) were fixed. HEAD has since moved to 3b51b43, "clear apply-plan CI hygiene and portability," which is pure lint/portability cleanup:

  • oid_re in the embedded Python parser: \s+[ \t]+ (apply-plan.sh#L127). The heredoc it lives in is quoted (<<'PY'), so this was never a shell-quoting hazard; functionally a no-op since every evidence string producer in audit-fleet.sh emits headRefOid <oid> with a single literal space, never a newline.
  • Three *) ;; no-op default cases added to case statements that previously had no default (worktree_path_for_branch, worktree_locked, refresh_worktree_cleanup) — shellcheck SC2249 silencing, no behavior change.
  • # shellcheck disable=SC2310 comments on apply-plan.sh and apply-plan.test.sh for intentional status-in-conditional patterns — accurately scoped, not a blanket disable.
  • Test file: dropped an unused OID_UNBOUND capture (add_merged_branch ... >/dev/null now discards output it never read) — correct dead-code removal, the call is still made for its git side effect.
  • docs/SKILL-CHEAT-SHEET.md gains the /repo-fleet-hygiene:apply row; CHANGELOG.md drops a stray blank line for markdownlint.

I read this commit's diff in full against the surrounding code (not just the commit message) and found nothing that changes runtime behavior incorrectly — all four code hunks are lint/portability-only, and the regex narrowing is provably a no-op given how the only producer of evidence strings formats them.

No new correctness or maintainability findings on top of what the lane already reported at b406289. Nothing to post inline.

I was not able to execute apply-plan.test.sh or audit-fleet.test.sh myself in this environment (no test-runner tool available to this lane), so the PR description's verification commands remain author-claimed and unverified by me — same limitation noted in the prior review.

@kyle-sexton
kyle-sexton merged commit c3dc92b into main Aug 15, 2026
40 checks passed
@kyle-sexton
kyle-sexton deleted the cursor/feat-fleet-apply-plan-f8b0 branch August 15, 2026 04:17
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.

repo-fleet-hygiene: the plugin reports per-repository facts a sibling already owns, and cannot perform the fleet-wide cleanup it exists for

1 participant