[failproofai-487] Skip require-*-before-stop gates in Claude Code plan mode

[NiveditJain]

Problem

At the end of a plan-mode turn in Claude Code, the require-push-before-stop gate fired:

Stop hook error: MANDATORY ACTION REQUIRED from failproofai (policy: failproofai/require-push-before-stop): Branch "luv-341" has not been pushed to remote "origin". Run now: git push -u origin luv-341

This is a contradiction by construction: plan mode is research-and-plan-only — the agent makes no commits, pushes, or PRs by design — yet the gate demanded a git push before it could stop, blocking the agent from finishing its turn.

Root cause

The five require-*-before-stop builtins fire on every Stop event and never consult the session's permission mode, even though it is fully available:

• Claude Code sends permission_mode: "plan" on the Stop payload during plan mode.
• failproofai already resolves it onto ctx.session.permissionMode (resolve-permission-mode.ts:49 → handler.ts:164) — but only uses it for activity logging and the dashboard badge, never for policy gating.

So a plan-mode Stop runs the real git check, finds no pushed branch (correctly — plan mode changed nothing), and returns exitCode: 2 with the MANDATORY ACTION message.

Fix

Add a one-line isPlanMode(ctx) guard as the first statement of each of the five policies (commit / push / pr / no-conflicts / ci-green), matching each policy's existing early-skip idiom. They skip in plan mode and stay fully active in default sessions; custom Stop policies are untouched. permission_mode: "plan" is Claude-specific in practice (every other CLI resolves to "default"), which matches where the bug occurs.

Tests

A plan-mode regression test in each policy's suite asserts the policy allows and that no git/gh subprocess runs (the guard short-circuits before any exec). Full local run: 1912 unit + 298 e2e passing; lint (0 errors) + tsc --noEmit clean.

🤖 Generated with Claude Code

https://claude.ai/code/session_012Gn9p2tji5JjqAWAVg23ds

Summary by CodeRabbit

• Bug Fixes
  • Stop workflow checks now correctly skip “before stop” gate requirements in plan mode.
  • Plan-mode sessions no longer block on commit, push, pull request, merge-conflict, or CI-green conditions.
  • Prevents unnecessary git/CI side effects when no changes are meant to be committed or pushed.
• Chores
  • Updated the 0.0.13-beta.2 changelog entry (dated 2026-07-10) to reflect the plan-mode Stop-gate fix.
• Tests
  • Added regression tests verifying Stop-gate short-circuit behavior in plan mode for multiple policy scenarios.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ce2ba21a-cd8d-4e9e-8814-638ff816a763

📥 Commits

Reviewing files that changed from the base of the PR and between 94c3fbd and cf22614.

📒 Files selected for processing (1)

• CHANGELOG.md

✅ Files skipped from review due to trivial changes (1)

• CHANGELOG.md

---

📝 Walkthrough

Walkthrough

The stop policies detect Claude Code plan mode and allow completion without checking commits, pushes, pull requests, conflicts, or CI status. Regression tests verify no subprocesses run, and the changelog documents the 0.0.13-beta.2 fix.

Changes

Plan-mode workflow gating

Layer / File(s)	Summary
Plan-mode policy bypass src/hooks/builtin-policies.ts	Five stop policies return allow in permissionMode: "plan" before running repository, pull request, conflict, or CI checks.
Regression coverage and release note __tests__/hooks/builtin-policies.test.ts, CHANGELOG.md	Tests cover plan-mode allowances and confirm subprocesses are not invoked; the changelog records the release fix.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• FailproofAI/failproofai#410: Modifies the same CI stop-policy logic with different handling for neutral conclusions.

Poem

A rabbit planned beneath the moon,
No commits needed, not a single push tune.
Five gates now softly step aside,
While quiet checks stay uninvoked inside.
Hop, hop—plan mode wins the ride!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description is detailed, but it does not follow the required template and omits the Type of Change and Checklist sections.	Add the template's ## Description, ## Type of Change, and ## Checklist sections, and fill the checklist items or mark them appropriately.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly matches the main change: skipping require-*-before-stop gates in Claude Code plan mode.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

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.}

[hermes-exosphere]

🔍 Automated code review started — analyzing [{"additions":5,"deletions":0,"path":"CHANGELOG.md"},{"additions":59,"deletions":0,"path":"tests/hooks/builtin-policies.test.ts"},{"additions":16,"deletions":0,"path":"src/hooks/builtin-policies.ts"}] files, +80/-0...

⏱️ This may take a few minutes. Results will be posted here when complete.

[hermes-exosphere]

🔵 INFO (non-blocking): isPlanMode gates on the session's current permission mode at Stop time, but the five checks it skips (commit / push / PR / conflicts / CI) inspect the git working-tree and remote state, which can carry work from an earlier default-mode turn.

So if the tree has uncommitted/unpushed changes from a prior turn and the current turn ends in plan mode (e.g. the user toggled to plan mode before stopping), all five gates are skipped and enforcement is deferred to the next non-plan Stop. Nothing is lost — the work stays in the tree and the next default-mode Stop re-checks it — and this matches the PR's intent (plan mode = "don't bug me"), so no change is required.

Flagging only so it's a conscious call: the "…no changes made…" reason string is an approximation — it means this turn made none, not that the tree is clean.

[hermes-exosphere]

🔍 Automated Code Review

📋 Executive Summary

This PR fixes a real contradiction-by-construction bug: the five require-*-before-stop workflow gates fired on Claude Code plan-mode Stop events, demanding a git push/commit/PR that plan mode forbids by design — blocking the agent from finishing its turn. The fix adds a one-line isPlanMode(ctx) guard (ctx.session?.permissionMode === "plan") as the first statement of each gate, short-circuiting to allow() before any subprocess runs. The change is additive (+80/-0), tightly scoped, and well-tested. No breaking changes, no logical errors found.

---

📊 Change Architecture

graph TD
    A[Stop event · Claude Code] --> B["handler.ts<br/>resolvePermissionMode()"]
    B -->|"session.permissionMode"| C["isPlanMode(ctx)<br/>NEW helper"]
    C -->|"=== 'plan'"| D["allow() — skip gate<br/>NEW guard ×5"]
    C -->|"!== 'plan'"| E["Run git/gh checks<br/>(unchanged)"]
    D --> F[Agent stops cleanly]
    E --> G{clean?}
    G -->|yes| F
    G -->|no| H["deny — MANDATORY ACTION"]
    style C fill:#90EE90
    style D fill:#90EE90

Loading

Legend: 🟢 New · 🔵 Modified · 🟡 Breaking-change risk

---

🔴 Breaking Changes

✅ No breaking changes detected. The change is purely additive — a new early-return guard that only triggers when permissionMode === "plan". Every non-plan session (default / acceptEdits / bypassPermissions, and every non-Claude CLI, which all resolve to "default") takes the identical pre-existing code path. Custom Stop policies are untouched. No schema, API, config, dependency, or serialization changes.

---

⚠️ Issues Found

No blocking issues. One non-blocking observation:

1. 🔵 Info — src/hooks/builtin-policies.ts:1099 — The guard keys off the current session mode, while the skipped checks inspect working-tree/remote state that can carry work from an earlier default-mode turn → enforcement is deferred (not lost) to the next non-plan Stop. Acceptable and intentional; see inline note.

---

🔬 Logical / Bug Analysis

• Guard placement ✅ — isPlanMode(ctx) is the first statement in all five functions (commit/push/pr/no-conflicts/ci-green), short-circuiting before the cwd check and any execSync/execFileSync. Verified at builtin-policies.ts:1103, 1126, 1221, 1315, 1414.
• Type safety ✅ — SessionMetadata.permissionMode is string | undefined (types.ts:686); the optional-chained ctx.session?.permissionMode === "plan" is null-safe. Undefined session → falsy → gates run normally (safe default).
• Blast radius ✅ — resolvePermissionMode (resolve-permission-mode.ts) only ever returns "plan" for Claude when the payload carries permission_mode: "plan". Codex maps approval_policy (never/on-request) → full-auto/default; Copilot/Cursor/OpenCode/Pi/Gemini/Hermes all fall back to "default". So the guard is inert everywhere except genuine Claude plan mode — exactly where the bug occurs.
• Helper reuse ✅ — Each gate keeps its own tailored reason string ("skipping commit check", etc.), matching the file's existing if (!cwd) return allow(...) idiom. Good consistency; no over-abstraction.
• No race conditions / leaks / off-by-ones — the change adds no loops, I/O, or shared state; it removes a code path rather than adding one.

---

🧪 Evidence — Build & Test Results

Unit tests — vitest run __tests__/hooks/builtin-policies.test.ts

 Test Files  1 passed (1)
      Tests  424 passed (424)
   Duration  1.15s

New plan-mode regression tests (one per policy) — all pass

 ✓ require-commit-before-stop       > allows in plan mode without running git (research-only, nothing to commit)
 ✓ require-push-before-stop         > allows in plan mode even when the branch was never pushed (the reported bug)
 ✓ require-pr-before-stop           > allows in plan mode without checking for a PR (research-only)
 ✓ require-no-conflicts-before-stop > allows in plan mode without probing for conflicts (research-only)
 ✓ require-ci-green-before-stop     > allows in plan mode without querying CI (research-only)
 Tests  5 passed | 419 skipped (424)

Each test primes a failing mock scenario (dirty tree / never-pushed branch / failing CI) and asserts execSync/execFileSync were not called — proving the guard short-circuits before any subprocess, not merely that it returns allow.

Typecheck & lint (CI "quality" job)

$ bunx tsc --noEmit          -> exit 0 (clean)
$ bunx eslint src/hooks/builtin-policies.ts __tests__/hooks/builtin-policies.test.ts
                             -> exit 0 (0 errors, 0 warnings)

Version consistency & CI rollup

package.json version : 0.0.13-beta.2
CHANGELOG top heading: ## 0.0.13-beta.2 — 2026-07-10   (match)

GitHub checks: quality ✅ · build ✅ · docs ✅ · test (default) ✅ ·
               test (log-debug) ✅ · test (hook-log-file) ✅ · test-e2e ✅ ·
               OSV-Scanner ✅ · Socket Security ✅ · CodeRabbit ✅

---

🔗 Issue Linkage

No GitHub issue is formally linked (closingIssuesReferences empty), but the PR body documents the concrete failure it addresses (the require-push-before-stop MANDATORY-ACTION false positive on branch luv-341 during a plan-mode turn) and the [failproofai-487] commit prefix ties it to internal tracking #487. Traceability is adequate; a Closes #… line would be a nice-to-have.

---

💡 Suggestions

• (optional) Consider whether isPlanMode should live next to resolvePermissionMode / in a shared helper module if other policies later need mode-awareness — fine as a local helper for now.
• (optional) The "…no changes made…" reason strings could read "…skipping — plan mode makes no changes this turn…" to reflect the per-turn semantics noted inline. Cosmetic only.
• CodeRabbit's pre-merge check flagged the PR description as missing the repo's Type-of-Change / Checklist template sections — worth filling in for template compliance (non-blocking).

---

🏆 Verdict

✅ APPROVED — Correct, minimal, and well-tested fix for a genuine plan-mode false positive. Additive-only with strong regression coverage (subprocess-not-called assertions), clean tsc/eslint, and green CI. The single inline note is informational, not blocking.

---

Automated code review · 2026-07-11 · verified locally against 94c3fbd

[hermes-exosphere]

✅ Automated review: APPROVED.

Correct, minimal, additive (+80/−0) fix for a genuine plan-mode false positive in the five require-*-before-stop gates. Verified locally against 94c3fbd:

• 424 unit tests pass (incl. 5 new plan-mode regression tests — one per policy, each asserting no subprocess runs)
• tsc --noEmit clean · eslint clean · version-consistency OK · all CI checks green

No breaking changes, no logical errors. One non-blocking INFO note left inline re: deferred enforcement of pre-existing uncommitted work — acceptable and intentional. 🟢