fix(check): derive scoped-executor scan set from filesystem (dead guard)

[chrisleekr]

What

Closes #203. The FR-009 destructive-action guard (scripts/check-no-destructive-actions.ts) hard-coded SCAN_FILES with src/daemon/scoped-explain-thread-executor.ts, which was removed when chat-thread subsumed explain-thread. readFileSync threw ENOENT and the guard crashed before validating anything — a dead CI safety guard. It surfaced only locally (it was wired into bun run check but not ci.yml).

Fix

• scripts/check-no-destructive-actions.ts — derive the scoped-executor set from the filesystem (scoped-*-executor.ts under src/daemon, via scopedExecutorFiles()) instead of a hard-coded list, so a renamed/removed executor cannot leave a stale entry. Hardenings from review:
  • withFileTypes + isFile() so a directory matching the regex can't cause EISDIR (parity with walk()).
  • A zero-match floor that throws if the naming convention ever drifts so that no executor matches — converts the worst-case fail-open (silently scan nothing) to fail-closed.
• .github/workflows/ci.yml — new "Destructive-action guard (FR-009)" step running bun run check:no-destructive, so the security guard is actually enforced in CI, not just local bun run check.
• test/scripts/check-no-destructive-actions.test.ts (new) — 8 cases via cwd-fixtures + a real-repo exit-0 regression: real-repo run, clean-with-only-some-executors, destructive-in-executor, destructive-under-ship, comment-not-flagged, non-executor-daemon-file-excluded (narrowing boundary), convention-drift fail-closed, missing-src/daemon fail-closed.
• docs/use/safety.md / CLAUDE.md — doc-sync: drop the stale "four scoped daemon executors" count (the derived set tracks the filesystem), and document the now-CI-enforced check:no-destructive + check:test-globs guards.

Red → green

All 8 tests failed against the broken script (ENOENT crash on any cwd); they pass after the derive-from-filesystem fix. The real guard now exits 0 ("3 scoped executors scanned").

Acceptance criteria (issue #203)

1. ✅ bun run check:no-destructive exits 0 on main.
2. ✅ No hard-coded non-existent files; the set is derived from the filesystem.
3. ✅ Regression test asserts no crash when only some executors exist + real-repo exit 0.
4. ✅ check:no-destructive wired into ci.yml.

Provenance

Filed by me during cycle 1 of this bulk-implement run, when the dead guard was discovered while fixing #201.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Tests

  • Added comprehensive test suite for destructive action detection validation.

• Chores

  • Enhanced CI workflow with stricter safety enforcement to prevent destructive git operations in specific workflow paths.
  • Updated documentation reflecting improved safety checks with dynamic executor discovery.

[coderabbitai]

Warning

Review limit reached

@chrisleekr, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 45 minutes and 33 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 35251682-437c-4612-ae70-b446b617abe8

📥 Commits

Reviewing files that changed from the base of the PR and between 8d0ddb4 and 31261ac.

📒 Files selected for processing (2)

• scripts/check-no-destructive-actions.ts
• test/scripts/check-no-destructive-actions.test.ts

📝 Walkthrough

Walkthrough

The PR fixes a crash in the check-no-destructive guard caused by a stale hardcoded file reference, replacing it with dynamic discovery. The check is tested comprehensively and wired into CI to prevent future stale entries from silently breaking the guard.

Changes

Dynamic Scoped-Executor Discovery and Enforcement

Layer / File(s)	Summary
Dynamic executor discovery and scanner integration scripts/check-no-destructive-actions.ts	scopedExecutorFiles() reads src/daemon/, filters files matching scoped-.*-executor.ts, sorts them, and throws if zero matches (fail-closed). The scan() function signature is updated to accept discovered executor paths and includes them in the scan iterable. A single call to scopedExecutorFiles() feeds the result into scan(), and the final "clean" log reports the actual executor count.
Comprehensive test coverage test/scripts/check-no-destructive-actions.test.ts	New Bun test suite with subprocess execution, temporary fixture creation/cleanup, and scenario tests covering successful discovery, fail-closed behavior when executors or daemon directory are missing, destructive command flagging in scoped executors, destructive call detection under ship workflows, comment filtering, non-executor daemon file exclusion, and regression coverage for the original crash.
CI integration and documentation .github/workflows/ci.yml, CLAUDE.md, docs/use/safety.md	Adds "Destructive-action guard (FR-009)" step to the CI lint-and-test job after the test-glob drift guard, documents the check in project notes, and updates safety documentation to reflect filesystem-based scoped executor discovery rather than a fixed hardcoded list.

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels: type: ci ⚙️, type: docs 📋

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: deriving scoped-executor scan set from filesystem rather than using a hardcoded list to fix a dead guard.
Linked Issues check	✅ Passed	The PR implements all acceptance criteria from #203: derives scoped executors from filesystem [#203], adds regression tests [#203], wires check:no-destructive into CI [#203], and replaces hardcoded SCAN_FILES with dynamic discovery [#203].
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing the FR-009 guard: script refactoring, CI integration, test coverage, and documentation updates—all within scope of issue #203.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull request overview

Restores the FR-009 destructive-action safety guard by removing a stale hard-coded executor list and deriving the scoped-executor scan set from the filesystem, then enforces the guard in CI to prevent future “dead guard” regressions.

Changes:

• Update check-no-destructive-actions to discover src/daemon/scoped-*-executor.ts files dynamically and fail closed on convention drift.
• Add an end-to-end test suite exercising fixture repos plus a real-repo regression run.
• Wire bun run check:no-destructive into the CI workflow and sync related documentation.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
scripts/check-no-destructive-actions.ts	Derives scoped executor scan targets from src/daemon and updates the guard’s reporting.
test/scripts/check-no-destructive-actions.test.ts	Adds regression + fixture-based coverage for the guard’s behavior and failure modes.
.github/workflows/ci.yml	Enforces the destructive-action guard in CI via a dedicated step.
docs/use/safety.md	Updates safety docs to reflect filesystem-derived executor scanning + CI enforcement.
CLAUDE.md	Documents the CI-enforced guards (test-glob drift + destructive-action).

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/check-no-destructive-actions.ts`:
- Around line 88-95: The scan function's pure-comment-line filter currently only
skips trimmed lines starting with '//' or '*' which misses single-line block
comments like '/* ... */', causing false positives; update the pure-comment
check in scan(...) to also skip lines that begin with '/*' or '*/' or that end
with '*/' (i.e., treat single-line block comment markers as pure comment lines),
keep the rest of the line-by-line scanning logic intact, and add a regression
test that feeds a file (e.g., a stub matching src/daemon/scoped-*-executor.ts)
containing a line like '/* NEVER call gh pr merge */' to assert scan() does not
report that line as a violation.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 127ad9bc-b6d1-47f8-8665-2180f9b3fad1

📥 Commits

Reviewing files that changed from the base of the PR and between 7a5cfb0 and 8d0ddb4.

📒 Files selected for processing (5)

• .github/workflows/ci.yml
• CLAUDE.md
• docs/use/safety.md
• scripts/check-no-destructive-actions.ts
• test/scripts/check-no-destructive-actions.test.ts