fix(cleanup): keep the base-branch config revert out of the auto-commit (#1677)

restoreConfigFromBase replaces .claude/, CLAUDE.md and the other sensitive
paths with the PR base branch's versions, then deliberately unstages them so
the revert does not reach a commit. checkAndCommitOrDeleteBranch then ran a
bare `git add -A`, which staged them again and pushed a silent revert of the
PR author's own config onto their branch, under a commit message that says
only "Auto-commit: Save uncommitted changes from Claude".

restoreConfigFromBase now returns the paths it restored. run.ts threads them
through updateCommentLink into checkAndCommitOrDeleteBranch, which excludes
them via pathspec from both the staging and the git status check.

The exclusion is driven by what was actually restored rather than applied
unconditionally. This path also runs for issues, where no restore happens and
Claude may legitimately have been asked to edit CLAUDE.md or
.claude/settings.json; excluding those there would silently drop the work —
trading one silent-data-loss bug for another. Reverting the fix, dropping the
status scoping, and switching to an unconditional exclusion each fail the new
tests.

The status check is scoped the same way as the staging: when the reverted
config is the only dirty entry there is no real work, so the branch is now
correctly treated as empty and deleted instead of receiving a pure revert.

Reachable on a closed or merged PR where Claude left uncommitted changes with
use_commit_signing false — the only combination where a restore has run and
claudeBranch is set.

Fixes #1669

src/entrypoints/run.ts

@@ -159,6 +159,9 @@ async function run() {
   let context: GitHubContext | undefined;
   let octokit: Octokits | undefined;
   let workloadIdentity: WorkloadIdentityHandle | undefined;
+  // Paths reverted to the PR base branch, which cleanup must not commit back
+  // onto the PR author's branch. Empty unless restoreConfigFromBase ran.
+  let restoredConfigPaths: string[] = [];
   // Track whether we've completed prepare phase, so we can attribute errors correctly
   let prepareCompleted = false;
   try {
@@ -268,7 +271,7 @@ async function run() {
         validateBranchName(restoreBase);
       }
       if (restoreBase) {
-        restoreConfigFromBase(restoreBase);
+        restoredConfigPaths = restoreConfigFromBase(restoreBase);
       }
     }
 
@@ -348,6 +351,7 @@ async function run() {
           prepareSuccess,
           prepareError,
           useCommitSigning: context.inputs.useCommitSigning,
+          restoredConfigPaths,
         });
       } catch (error) {
         console.error("Error updating comment with job link:", error);

src/entrypoints/update-comment-link.ts

@@ -30,6 +30,12 @@ export type UpdateCommentLinkParams = {
   prepareSuccess: boolean;
   prepareError?: string;
   useCommitSigning: boolean;
+  /**
+   * Paths restored from the PR base branch by restoreConfigFromBase. The
+   * auto-commit in checkAndCommitOrDeleteBranch must leave these alone, or it
+   * commits the revert onto the PR author's branch.
+   */
+  restoredConfigPaths?: string[];
 };
 
 export async function updateCommentLink(
@@ -43,6 +49,7 @@ export async function updateCommentLink(
     context,
     octokit,
     useCommitSigning,
+    restoredConfigPaths = [],
   } = params;
 
   const { owner, repo } = context.repository;
@@ -116,6 +123,7 @@ export async function updateCommentLink(
     claudeBranch,
     baseBranch,
     useCommitSigning,
+    restoredConfigPaths,
   );
 
   // Check if we need to add PR URL when we have a new branch

src/github/operations/branch-cleanup.ts

@@ -9,10 +9,32 @@ export async function checkAndCommitOrDeleteBranch(
   claudeBranch: string | undefined,
   baseBranch: string,
   useCommitSigning: boolean,
+  restoredConfigPaths: string[] = [],
 ): Promise<{ shouldDeleteBranch: boolean; branchLink: string }> {
   let branchLink = "";
   let shouldDeleteBranch = false;
 
+  // On pull requests, restoreConfigFromBase replaces .claude/, CLAUDE.md and
+  // friends with the base branch's versions and leaves them unstaged so the
+  // revert does not reach a commit. Auto-committing with a bare `git add -A`
+  // would stage them anyway and push a silent revert of the PR author's own
+  // config onto their branch.
+  //
+  // The exclusion is driven by what was actually restored rather than applied
+  // unconditionally: this path also runs for issues, where no restore happens
+  // and Claude may legitimately have been asked to edit CLAUDE.md or
+  // .claude/settings.json. Excluding those there would silently drop the work.
+  const pathspecArgs =
+    restoredConfigPaths.length > 0
+      ? ["--", ".", ...restoredConfigPaths.map((p) => `:(exclude)${p}`)]
+      : [];
+
+  if (pathspecArgs.length > 0) {
+    console.log(
+      `Excluding base-restored config from auto-commit: ${restoredConfigPaths.join(", ")}`,
+    );
+  }
+
   if (claudeBranch) {
     // First check if the branch exists remotely
     let branchExistsRemotely = false;
@@ -57,15 +79,19 @@ export async function checkAndCommitOrDeleteBranch(
 
           // Check for uncommitted changes using git status
           try {
-            const gitStatus = await $`git status --porcelain`.quiet();
+            // Scoped the same way as the staging below: if the restored config
+            // is the only dirty entry there is no real work, and the branch
+            // should be treated as empty rather than receiving a pure revert.
+            const gitStatus =
+              await $`git status --porcelain ${pathspecArgs}`.quiet();
             const hasUncommittedChanges =
               gitStatus.stdout.toString().trim().length > 0;
 
             if (hasUncommittedChanges) {
               console.log("Found uncommitted changes, committing them...");
 
-              // Add all changes
-              await $`git add -A`;
+              // Add all changes, minus anything restored from the base branch
+              await $`git add -A ${pathspecArgs}`;
 
               // Commit with a descriptive message
               const runId = process.env.GITHUB_RUN_ID || "unknown";

src/github/operations/restore-config.ts

@@ -23,7 +23,7 @@ import { fetchDepthArgs } from "./fetch-depth";
 //   .gitconfig   — git reads ~/.gitconfig and .git/config, never cwd/.gitconfig.
 //   .bashrc etc. — shells source these from $HOME; checkout cannot reach $HOME.
 //   .vscode/.idea— IDE config; nothing in the CLI's startup path reads them.
-const SENSITIVE_PATHS = [
+export const SENSITIVE_PATHS = [
   ".claude",
   ".mcp.json",
   ".claude.json",
@@ -262,8 +262,11 @@ function ensureClaudePrExcludedFromGit(): void {
  *
  * @param baseBranch - PR base branch name. Must be pre-validated (branch.ts
  *   calls validateBranchName on it before returning).
+ * @returns The paths whose working-tree state now comes from the base branch
+ *   rather than the PR. Callers that stage files must exclude these, or they
+ *   will commit the revert back onto the PR author's branch.
  */
-export function restoreConfigFromBase(baseBranch: string): void {
+export function restoreConfigFromBase(baseBranch: string): string[] {
   console.log(
     `Restoring ${SENSITIVE_PATHS.join(", ")} from origin/${baseBranch} (PR head is untrusted)`,
   );
@@ -338,4 +341,9 @@ export function restoreConfigFromBase(baseBranch: string): void {
   } catch {
     // Nothing was staged, or paths don't exist on HEAD — either is fine.
   }
+
+  // Every sensitive path is reported, not just the ones that changed: the
+  // restore also deletes paths the PR added that are absent on base, and those
+  // deletions are stageable too.
+  return [...SENSITIVE_PATHS];
 }

test/branch-cleanup-restored-config.test.ts

@@ -0,0 +1,262 @@
+#!/usr/bin/env bun
+
+/**
+ * Tests the interaction between restoreConfigFromBase and the auto-commit in
+ * checkAndCommitOrDeleteBranch.
+ *
+ * On pull requests the restore replaces .claude/, CLAUDE.md and friends with
+ * the base branch's versions and leaves them unstaged, so the revert does not
+ * reach a commit. A bare `git add -A` re-staged them anyway and pushed a silent
+ * revert of the PR author's own config onto their branch.
+ *
+ * These run against real git — the fix is a pathspec, so a mock would only
+ * assert that the arguments were passed, not that git honours them.
+ */
+
+import { describe, test, expect, beforeEach, afterEach, spyOn } from "bun:test";
+import { execFileSync } from "node:child_process";
+import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { checkAndCommitOrDeleteBranch } from "../src/github/operations/branch-cleanup";
+import { SENSITIVE_PATHS } from "../src/github/operations/restore-config";
+import type { Octokits } from "../src/github/api/client";
+
+const BRANCH = "claude/issue-1-20260101-0000";
+
+let workDir: string;
+let remoteDir: string;
+let originalCwd: string;
+let logSpy: ReturnType<typeof spyOn>;
+let errorSpy: ReturnType<typeof spyOn>;
+
+function git(...args: string[]): string {
+  return execFileSync("git", args, { cwd: workDir, encoding: "utf-8" }).trim();
+}
+
+function write(relative: string, contents: string) {
+  const full = join(workDir, relative);
+  mkdirSync(join(full, ".."), { recursive: true });
+  writeFileSync(full, contents);
+}
+
+/** Branch exists, and has no commits ahead of base, so cleanup inspects git. */
+const mockOctokit = {
+  rest: {
+    repos: {
+      getBranch: async () => ({ data: {} }),
+      compareCommitsWithBasehead: async () => ({
+        data: { total_commits: 0 },
+      }),
+    },
+    git: { deleteRef: async () => ({ data: {} }) },
+  },
+} as unknown as Octokits;
+
+beforeEach(() => {
+  originalCwd = process.cwd();
+  const root = mkdtempSync(join(tmpdir(), "branch-cleanup-"));
+  remoteDir = join(root, "remote.git");
+  workDir = join(root, "work");
+
+  execFileSync("git", ["init", "-q", "--bare", remoteDir]);
+  execFileSync("git", ["init", "-q", "-b", "main", workDir]);
+  git("config", "user.email", "test@example.com");
+  git("config", "user.name", "Test");
+  git("remote", "add", "origin", remoteDir);
+
+  write(".claude/settings.json", '{"from":"base"}\n');
+  write("CLAUDE.md", "base docs\n");
+  write("src/app.ts", "base code\n");
+  git("add", "-A");
+  git("commit", "-qm", "base");
+  git("push", "-q", "origin", "main");
+  git("checkout", "-qb", BRANCH);
+  git("push", "-q", "origin", BRANCH);
+
+  process.chdir(workDir);
+  logSpy = spyOn(console, "log").mockImplementation(() => {});
+  errorSpy = spyOn(console, "error").mockImplementation(() => {});
+});
+
+afterEach(() => {
+  process.chdir(originalCwd);
+  logSpy.mockRestore();
+  errorSpy.mockRestore();
+  rmSync(join(workDir, ".."), { recursive: true, force: true });
+});
+
+/** Files touched by the most recent commit. */
+function filesInHeadCommit(): string[] {
+  return git("show", "--name-only", "--format=", "HEAD")
+    .split("\n")
+    .filter(Boolean)
+    .sort();
+}
+
+/**
+ * Reproduce the working-tree state restoreConfigFromBase leaves behind: the
+ * PR-authored config overwritten with the base branch's content, unstaged, so
+ * git reports it as a plain modification.
+ */
+function simulateRestoredConfig() {
+  write(".claude/settings.json", '{"from":"base"}\n');
+  write("CLAUDE.md", "base docs\n");
+}
+
+function authorPrConfigEdits() {
+  write(".claude/settings.json", '{"from":"pr-author"}\n');
+  write("CLAUDE.md", "pr author docs\n");
+  git("commit", "-qam", "PR author edits config");
+}
+
+describe("auto-commit with restored config paths", () => {
+  test("does not commit the base-branch revert onto the PR branch", async () => {
+    authorPrConfigEdits();
+    simulateRestoredConfig(); // config now reverted + unstaged
+    write("src/app.ts", "claude's real change\n");
+
+    const result = await checkAndCommitOrDeleteBranch(
+      mockOctokit,
+      "owner",
+      "repo",
+      BRANCH,
+      "main",
+      false,
+      [...SENSITIVE_PATHS],
+    );
+
+    expect(filesInHeadCommit()).toEqual(["src/app.ts"]);
+    expect(result.shouldDeleteBranch).toBe(false);
+  });
+
+  test("leaves the reverted config dirty in the working tree", async () => {
+    authorPrConfigEdits();
+    simulateRestoredConfig();
+    write("src/app.ts", "claude's real change\n");
+
+    await checkAndCommitOrDeleteBranch(
+      mockOctokit,
+      "owner",
+      "repo",
+      BRANCH,
+      "main",
+      false,
+      [...SENSITIVE_PATHS],
+    );
+
+    // --name-only gives bare paths, avoiding porcelain's status-column prefix.
+    const stillDirty = git("diff", "--name-only")
+      .split("\n")
+      .filter(Boolean)
+      .sort();
+    expect(stillDirty).toEqual([".claude/settings.json", "CLAUDE.md"]);
+  });
+
+  test("treats a branch whose only change is the revert as empty", async () => {
+    // No real work — just the reverted config. Committing here would push a
+    // pure revert and keep an otherwise-empty branch alive.
+    authorPrConfigEdits();
+    simulateRestoredConfig();
+
+    const before = git("rev-parse", "HEAD");
+    const result = await checkAndCommitOrDeleteBranch(
+      mockOctokit,
+      "owner",
+      "repo",
+      BRANCH,
+      "main",
+      false,
+      [...SENSITIVE_PATHS],
+    );
+
+    expect(git("rev-parse", "HEAD")).toBe(before);
+    expect(result.shouldDeleteBranch).toBe(true);
+    expect(result.branchLink).toBe("");
+  });
+
+  test("still commits Claude's own changes to non-config files", async () => {
+    authorPrConfigEdits();
+    simulateRestoredConfig();
+    write("src/app.ts", "changed\n");
+    write("src/new-file.ts", "added\n");
+
+    await checkAndCommitOrDeleteBranch(
+      mockOctokit,
+      "owner",
+      "repo",
+      BRANCH,
+      "main",
+      false,
+      [...SENSITIVE_PATHS],
+    );
+
+    expect(filesInHeadCommit()).toEqual(["src/app.ts", "src/new-file.ts"]);
+  });
+
+  test("pushes the commit to the branch", async () => {
+    authorPrConfigEdits();
+    simulateRestoredConfig();
+    write("src/app.ts", "claude's real change\n");
+
+    const result = await checkAndCommitOrDeleteBranch(
+      mockOctokit,
+      "owner",
+      "repo",
+      BRANCH,
+      "main",
+      false,
+      [...SENSITIVE_PATHS],
+    );
+
+    const remoteHead = execFileSync(
+      "git",
+      ["--git-dir", remoteDir, "rev-parse", BRANCH],
+      { encoding: "utf-8" },
+    ).trim();
+    expect(remoteHead).toBe(git("rev-parse", "HEAD"));
+    expect(result.branchLink).toContain(BRANCH);
+  });
+});
+
+describe("without restored config paths (the issue path)", () => {
+  test("commits config changes normally, since no revert happened", async () => {
+    // Reached for issues, where restoreConfigFromBase never runs and Claude may
+    // have been asked to edit CLAUDE.md. Excluding it here would drop the work.
+    write("CLAUDE.md", "claude wrote these docs\n");
+    write(".claude/settings.json", '{"written":"by claude"}\n');
+    write("src/app.ts", "and some code\n");
+
+    await checkAndCommitOrDeleteBranch(
+      mockOctokit,
+      "owner",
+      "repo",
+      BRANCH,
+      "main",
+      false,
+      [],
+    );
+
+    expect(filesInHeadCommit()).toEqual([
+      ".claude/settings.json",
+      "CLAUDE.md",
+      "src/app.ts",
+    ]);
+  });
+
+  test("defaults to committing everything when the argument is omitted", async () => {
+    // Backwards compatibility: the parameter is optional.
+    write("CLAUDE.md", "claude wrote these docs\n");
+
+    await checkAndCommitOrDeleteBranch(
+      mockOctokit,
+      "owner",
+      "repo",
+      BRANCH,
+      "main",
+      false,
+    );
+
+    expect(filesInHeadCommit()).toEqual(["CLAUDE.md"]);
+  });
+});