Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
NiveditJain merged 3 commits into from
Jul 11, 2026
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d9b36b3
[failproofai-487] Skip require-*-before-stop gates in Claude Code pla…
NiveditJain Jul 11, 2026
94c3fbd
[failproofai-487] Correct changelog PR reference to #488
NiveditJain Jul 11, 2026
cf22614
Merge branch 'main' into luv-487
NiveditJain Jul 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
# Changelog

## 0.0.13-beta.2 — 2026-07-11
## 0.0.13-beta.2 — 2026-07-10

### Docs
### Fixes
- Skip the `require-*-before-stop` workflow gates during Claude Code plan mode (`permission_mode: "plan"`) — plan mode makes no commits/pushes/PRs by design, so the gates were wrongly demanding actions plan mode forbids (e.g. `git push` with nothing to push, blocking the agent from finishing). (#488)
- Fix translation regressions from the #486 docs sync: strip hallucinated `{#…}` heading-ID syntax that broke MDX parsing (`ja/built-in-policies`), restore dropped YAML frontmatter (`tr/cli/hook`) and the `--cli …|hermes` inline code span (`he/configuration`), and un-translate an inline-code placeholder (`de/cli/auth`). (#487)

## 0.0.13-beta.1 — 2026-07-10
Expand Down
59 changes: 59 additions & 0 deletions __tests__/hooks/builtin-policies.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2303,6 +2303,18 @@ describe("hooks/builtin-policies", () => {
expect(result.reason).toContain("uncommitted changes");
});

it("allows in plan mode without running git (research-only, nothing to commit)", async () => {
// Regression: Stop workflow gates must not fire in Claude Code plan mode,
// where the agent makes no changes by design. The guard short-circuits
// before any git subprocess runs, even when the tree looks dirty.
vi.mocked(execSync).mockReturnValue("M src/index.ts\n");
const ctx = makeCtx({ eventType: "Stop", session: { cwd: "/repo", permissionMode: "plan" } });
const result = await policy.fn(ctx);
expect(result.decision).toBe("allow");
expect(result.reason).toContain("Plan mode");
expect(execSync).not.toHaveBeenCalled();
});

it("denies when there are untracked files", async () => {
vi.mocked(execSync).mockReturnValue("?? newfile.ts\n");
const ctx = makeCtx({ eventType: "Stop", session: { cwd: "/repo" } });
Expand Down Expand Up @@ -2471,6 +2483,18 @@ describe("hooks/builtin-policies", () => {
expect(result.reason).toContain("new-feature");
});

it("allows in plan mode even when the branch was never pushed (the reported bug)", async () => {
// Regression for the plan-mode false positive: a plan-only turn makes no
// commits/pushes, so requiring `git push -u` before stop is a contradiction.
mockPushScenario({ branch: "new-feature", hasTracking: false });
const ctx = makeCtx({ eventType: "Stop", session: { cwd: "/repo", permissionMode: "plan" } });
const result = await policy.fn(ctx);
expect(result.decision).toBe("allow");
expect(result.reason).toContain("Plan mode");
expect(execSync).not.toHaveBeenCalled();
expect(execFileSync).not.toHaveBeenCalled();
});

it("deny message includes branch name and remote", async () => {
mockPushScenario({ branch: "my-feature", hasTracking: false });
const ctx = makeCtx({ eventType: "Stop", session: { cwd: "/repo" } });
Expand Down Expand Up @@ -2687,6 +2711,16 @@ describe("hooks/builtin-policies", () => {
expect(result.reason).toContain("gh pr create");
});

it("allows in plan mode without checking for a PR (research-only)", async () => {
mockPrScenario({ prResult: null });
const ctx = makeCtx({ eventType: "Stop", session: { cwd: "/repo", permissionMode: "plan" } });
const result = await policy.fn(ctx);
expect(result.decision).toBe("allow");
expect(result.reason).toContain("Plan mode");
expect(execSync).not.toHaveBeenCalled();
expect(execFileSync).not.toHaveBeenCalled();
});

it("deny message includes the branch name", async () => {
mockPrScenario({ branch: "my-feature", prResult: null });
const ctx = makeCtx({ eventType: "Stop", session: { cwd: "/repo" } });
Expand Down Expand Up @@ -3013,6 +3047,19 @@ describe("hooks/builtin-policies", () => {
expect(result.reason).toContain("Rebase or merge origin/main");
});

it("allows in plan mode without probing for conflicts (research-only)", async () => {
mockConflictsScenario({
mergeTreeStatus: 1,
mergeTreeStdout: "treeoid123\nsrc/app.ts\n\nCONFLICT (content): ...\n",
});
const ctx = makeCtx({ eventType: "Stop", session: { cwd: "/repo", permissionMode: "plan" } });
const result = await policy.fn(ctx);
expect(result.decision).toBe("allow");
expect(result.reason).toContain("Plan mode");
expect(execSync).not.toHaveBeenCalled();
expect(execFileSync).not.toHaveBeenCalled();
});

it("deny reason falls back to \"one or more files\" when stdout parse yields no files", async () => {
mockConflictsScenario({
mergeTreeStatus: 1,
Expand Down Expand Up @@ -3259,6 +3306,18 @@ describe("hooks/builtin-policies", () => {
expect(result.reason).toContain('"test"');
});

it("allows in plan mode without querying CI (research-only)", async () => {
mockCiScenario("feat/branch", JSON.stringify([
{ status: "completed", conclusion: "failure", name: "test" },
]));
const ctx = makeCtx({ eventType: "Stop", session: { cwd: "/repo", permissionMode: "plan" } });
const result = await policy.fn(ctx);
expect(result.decision).toBe("allow");
expect(result.reason).toContain("Plan mode");
expect(execSync).not.toHaveBeenCalled();
expect(execFileSync).not.toHaveBeenCalled();
});

it("denies listing multiple failed checks by name", async () => {
mockCiScenario("feat/branch", JSON.stringify([
{ status: "completed", conclusion: "failure", name: "test" },
Expand Down
16 changes: 16 additions & 0 deletions src/hooks/builtin-policies.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1088,7 +1088,19 @@ function warnBackgroundProcess(ctx: PolicyContext): PolicyResult {

// -- Workflow (Stop event) policies --

/**
* Claude Code plan mode (permission_mode: "plan") is research-and-plan-only — the
* agent makes no commits, pushes, or PRs by design. The Stop-workflow gates below
* all assume the agent produced code changes, so in plan mode they demand actions
* plan mode forbids (e.g. a push with nothing to push). Skip them there. Only Claude
* reports "plan" today; every other CLI resolves to "default".
*/
function isPlanMode(ctx: PolicyContext): boolean {
return ctx.session?.permissionMode === "plan";

@hermes-exosphere hermes-exosphere Jul 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

}

function requireCommitBeforeStop(ctx: PolicyContext): PolicyResult {
if (isPlanMode(ctx)) return allow("Plan mode — no changes made, skipping commit check.");
const cwd = ctx.session?.cwd;
if (!cwd) return allow("No working directory available, skipping commit check.");

Expand All @@ -1111,6 +1123,7 @@ function requireCommitBeforeStop(ctx: PolicyContext): PolicyResult {
}

function requirePushBeforeStop(ctx: PolicyContext): PolicyResult {
if (isPlanMode(ctx)) return allow("Plan mode — no changes made, skipping push check.");
const cwd = ctx.session?.cwd;
if (!cwd) return allow("No working directory available, skipping push check.");

Expand Down Expand Up @@ -1205,6 +1218,7 @@ function requirePushBeforeStop(ctx: PolicyContext): PolicyResult {
}

function requirePrBeforeStop(ctx: PolicyContext): PolicyResult {
if (isPlanMode(ctx)) return allow("Plan mode — no changes made, skipping PR check.");
const cwd = ctx.session?.cwd;
if (!cwd) return allow("No working directory available, skipping PR check.");

Expand Down Expand Up @@ -1298,6 +1312,7 @@ function requirePrBeforeStop(ctx: PolicyContext): PolicyResult {
}

function requireNoConflictsBeforeStop(ctx: PolicyContext): PolicyResult {
if (isPlanMode(ctx)) return allow("Plan mode — no changes made, skipping conflict check.");
const cwd = ctx.session?.cwd;
if (!cwd) return allow("No working directory available, skipping conflict check.");

Expand Down Expand Up @@ -1396,6 +1411,7 @@ function requireNoConflictsBeforeStop(ctx: PolicyContext): PolicyResult {
}

function requireCiGreenBeforeStop(ctx: PolicyContext): PolicyResult {
if (isPlanMode(ctx)) return allow("Plan mode — no changes made, skipping CI check.");
const cwd = ctx.session?.cwd;
if (!cwd) return allow("No working directory available, skipping CI check.");

Expand Down