FN-7554: add PR-based revert path for autoMerge:false projects
Adds a PR-based revert path for done/archived tasks in autoMerge:false projects instead of refusing outright.
- New engine export `prepareRevertPrBranch` (packages/engine/src/task-revert.ts) prepares a dedicated `fusion/revert-<id>` branch off the base branch's HEAD and applies the revert commit(s) there, never mutating the base branch itself.
- `POST /api/tasks/:id/revert` route gains an additive `{ mode: "pr", clean: true, prUrl, prNumber, revertBranch, existingPr? }` result for clean single-repo reverts under autoMerge:false, reusing GitHubClient.createPr, findPrForBranch idempotency, and the manual:true PR handoff.
- Existing `{ mode: "git" | "ai", ... }` result shapes and the autoMerge:true path are unchanged.
- Workspace (multi-repo) tasks are explicitly refused for PR-based revert (out of scope; single PR cannot represent a multi-repo revert).
- Adds real-git integration tests for the new branch-prep/apply/commit flow and expands the dashboard route test coverage.
- Adds a changeset for @runfusion/fusion (minor) and a small task-management doc update.
Files changed:
.changeset/fn-7554-pr-based-revert.md | 7 +
docs/task-management.md | 2 +-
.../src/__tests__/task-revert-route.test.ts | 196 +++++++++++++++++++-
.../src/routes/register-task-workflow-routes.ts | 182 ++++++++++++++++++
.../src/__tests__/task-revert-pr.real-git.test.ts | 206 +++++++++++++++++++++
packages/engine/src/index.ts | 3 +
packages/engine/src/task-revert.ts | 154 +++++++++++++++
7 files changed, 747 insertions(+), 3 deletions(-)
Fusion-Task-Id: FN-7554
Fusion-Task-Lineage: 9b1bfd82-2428-4cf7-9b36-77afe3517a14
Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
This commit is contained in:
gsxdsm
7
.changeset/fn-7554-pr-based-revert.md
Normal file
7
.changeset/fn-7554-pr-based-revert.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": minor
|
||||
---
|
||||
|
||||
summary: Open a revert PR for done/archived tasks when autoMerge is disabled instead of refusing.
|
||||
category: feature
|
||||
dev: `POST /api/tasks/:id/revert` gains an additive `{ mode: "pr", clean: true, prUrl, prNumber, revertBranch, existingPr? }` result for clean single-repo reverts under `autoMerge:false`, reusing `GitHubClient.createPr`, `findPrForBranch` idempotency, and the `manual:true` PR handoff. New engine export `prepareRevertPrBranch` (packages/engine/src/task-revert.ts) prepares the dedicated `fusion/revert-<id>` branch without ever mutating the base branch. Existing `{ mode: "git" | "ai", ... }` shapes and the `autoMerge:true` path are unchanged.
|
||||
@@ -685,7 +685,7 @@ Recovery/backfill guidance:
|
||||
- Git-path response contract (additive only): `{ mode: "git", clean, revertCommitSha?, revertCommitShas?, conflicts?, alreadyReverted?, unsupported?, needsHuman?, reason? }`. A clean revert lands a `revert(FN-xxxx): ...` commit carrying a `Fusion-Task-Id` trailer on the resolved base branch; `revertCommitShas` reports every commit created (all of them for `per-sha`, the single one for `squash`) alongside the existing `revertCommitSha`.
|
||||
- AI-undo response contract: `{ mode: "ai", createdTaskId: "FN-YYYY", alreadyOpen?: true }`. The created task is an ordinary `triage`-column board task (via the normal `store.createTask` path) that references the source task's id, mission, and landed files, and instructs undoing the source task's behavior while preserving unrelated later changes to the same files, using a `revert(FN-xxxx): ...` commit convention. It carries NO dependency on the (already done/archived) source task. A `sourceMetadata.revertOf` marker makes repeated fallback calls idempotent — while an AI-undo task for that source is still open, a further call returns the same `createdTaskId` with `alreadyOpen: true` instead of creating a duplicate; a prior undo task that itself reached `done`/`archived` does not suppress a fresh one.
|
||||
- **Workspace (multi-repo) tasks (FN-7547):** tasks with `workspaceWorktrees` populated (`isWorkspaceTask`) are revertable too — the route dispatches to a dedicated workspace path that reasons about every sub-repo's integration branch as ONE all-or-nothing unit. It resolves each sub-repo's attributable commit(s), dry-run classifies every sub-repo first, and only commits a `revert(FN-xxxx): ...` commit on EACH sub-repo when every sub-repo classifies clean/already-reverted; if any sub-repo conflicts, no sub-repo is committed and every touched sub-repo worktree is rolled back to its pre-call state. Response contract for workspace tasks: `{ mode: "git", clean, workspace: { repos: [{ repo, classification, revertCommitSha?, conflicts?, alreadyReverted? }] }, conflicts?: {repo, file, ...}[] }`. A conflicting workspace result still falls back to the AI-undo task under `"auto"` mode, same as a single-repo conflicting result.
|
||||
- `autoMerge:false` projects return `needsHuman` and never trigger the AI-undo fallback (a human/future UI decides).
|
||||
- **`autoMerge:false` PR-based revert (FN-7554):** for a single-repo task whose git revert classifies **clean**, `autoMerge:false` no longer dead-ends at `needsHuman`. The route prepares a dedicated `fusion/revert-<id>` branch off the resolved base branch (via the engine's `prepareRevertPrBranch`, which NEVER writes to the base branch itself), pushes it, and opens a GitHub PR through the same owner/repo resolution, `githubRateLimiter` gate, `findPrForBranch` idempotency, and `manual: true` handoff as `POST /tasks/:id/pr/create`. Response: `{ mode: "pr", clean: true, prUrl, prNumber, revertBranch, existingPr? }` — a second call while the PR is still open links the existing PR (`existingPr: true`) instead of re-pushing. GitHub unconfigured or rate-limited still degrades gracefully to `{ mode: "git", needsHuman: true, reason }`, and a conflicting/unsupported/already-reverted classification is unaffected (no PR is opened; `"auto"` mode still falls back to the AI-undo task on conflict/unsupported). Workspace (multi-repo) tasks are not yet covered by this PR path — they keep the existing `needsHuman` result under `autoMerge:false`.
|
||||
- **Dashboard auto-linking (FN-7555):** the AI-undo task's card shows an "Undo of FN-xxxx" chip and its detail view shows a clickable "Created to undo FN-xxxx" link back to the source task. The source task's detail view shows an "Undo task: FN-YYYY" link whenever an OPEN undo task referencing it exists in the loaded tasks (matching `TaskStore.findOpenRevertTaskForSource`'s open-only semantics — a `done`/`archived`/soft-deleted undo task is never surfaced as active). Both directions are derived client-side from `sourceMetadata.revertOf`; no new API. A dedicated Done/Archived card revert-trigger action is still a separate follow-up (see FN-7525).
|
||||
|
||||
|
||||
|
||||
@@ -18,6 +18,7 @@ import { execFileSync } from "node:child_process";
|
||||
import type { Task, TaskStore } from "@fusion/core";
|
||||
import { createApiRoutes } from "../routes.js";
|
||||
import { request as performRequest } from "../test-request.js";
|
||||
import { githubRateLimiter } from "../github-poll.js";
|
||||
|
||||
// FNXC:TaskRevert 2026-07-04-00:00: the route now guards against `rootDir`
|
||||
// (the shared user checkout) sitting on a branch other than the resolved
|
||||
@@ -31,11 +32,18 @@ function makeGitRepoOnMain(): string {
|
||||
execFileSync("git", ["config", "user.email", "test@example.com"], { cwd: dir });
|
||||
execFileSync("git", ["config", "user.name", "Test"], { cwd: dir });
|
||||
execFileSync("git", ["commit", "--allow-empty", "-m", "init"], { cwd: dir });
|
||||
// FN-7554: a local bare "origin" remote lets `mode:"pr"` tests exercise a
|
||||
// REAL `git push -u origin <revertBranch>` without any network dependency.
|
||||
const originDir = mkdtempSync(join(tmpdir(), "kb-task-revert-route-origin-"));
|
||||
execFileSync("git", ["init", "--bare", "-b", "main"], { cwd: originDir });
|
||||
execFileSync("git", ["remote", "add", "origin", originDir], { cwd: dir });
|
||||
execFileSync("git", ["push", "-u", "origin", "main"], { cwd: dir });
|
||||
return dir;
|
||||
}
|
||||
|
||||
const performTaskRevertMock = vi.fn();
|
||||
const revertWorkspaceTaskMock = vi.fn();
|
||||
const prepareRevertPrBranchMock = vi.fn();
|
||||
|
||||
vi.mock("@fusion/engine", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("@fusion/engine")>();
|
||||
@@ -43,9 +51,25 @@ vi.mock("@fusion/engine", async (importOriginal) => {
|
||||
...actual,
|
||||
performTaskRevert: (...args: unknown[]) => performTaskRevertMock(...args),
|
||||
revertWorkspaceTask: (...args: unknown[]) => revertWorkspaceTaskMock(...args),
|
||||
prepareRevertPrBranch: (...args: unknown[]) => prepareRevertPrBranchMock(...args),
|
||||
};
|
||||
});
|
||||
|
||||
// FN-7554: stub GitHubClient at the route boundary — `findPrForBranch`/`createPr`
|
||||
// idempotency + push/create behavior is exercised here; real GitHubClient HTTP/gh-CLI
|
||||
// behavior is covered by github.test.ts.
|
||||
const findPrForBranchMock = vi.fn();
|
||||
const createPrMock = vi.fn();
|
||||
|
||||
vi.mock("../github.js", () => ({
|
||||
GitHubClient: vi.fn().mockImplementation(function (this: unknown) {
|
||||
return {
|
||||
findPrForBranch: (...args: unknown[]) => findPrForBranchMock(...args),
|
||||
createPr: (...args: unknown[]) => createPrMock(...args),
|
||||
};
|
||||
}),
|
||||
}));
|
||||
|
||||
// FNXC:TaskRevert 2026-07-04-00:00 (FN-7524): `createAiUndoTask` is NOT mocked —
|
||||
// these route tests exercise the real engine helper against a fake store
|
||||
// (`createTask`/`findOpenRevertTaskForSource`), proving the route wires the
|
||||
@@ -84,7 +108,7 @@ function makeWorkspaceTask(overrides: Partial<Task>): Task {
|
||||
|
||||
function createMockStore(
|
||||
task: Task,
|
||||
opts?: { openUndoTask?: Task | null; createdUndoTask?: Task },
|
||||
opts?: { openUndoTask?: Task | null; createdUndoTask?: Task; autoMerge?: boolean },
|
||||
): TaskStore {
|
||||
let nextId = 800;
|
||||
const createTask = vi.fn().mockImplementation(async (input: { description: string; source?: { sourceParentTaskId?: string; sourceMetadata?: Record<string, unknown> } }) => {
|
||||
@@ -104,12 +128,15 @@ function createMockStore(
|
||||
const findOpenRevertTaskForSource = vi.fn().mockResolvedValue(opts?.openUndoTask ?? null);
|
||||
return {
|
||||
getSettings: vi.fn().mockResolvedValue({}),
|
||||
getSettingsFast: vi.fn().mockResolvedValue({ autoMerge: true }),
|
||||
getSettingsFast: vi.fn().mockResolvedValue({ autoMerge: opts?.autoMerge ?? true }),
|
||||
getRootDir: vi.fn().mockReturnValue(makeGitRepoOnMain()),
|
||||
getTask: vi.fn().mockResolvedValue(task),
|
||||
getTaskCommitAssociationsByLineageId: vi.fn().mockResolvedValue([]),
|
||||
createTask,
|
||||
findOpenRevertTaskForSource,
|
||||
updatePrInfo: vi.fn().mockResolvedValue(task),
|
||||
addPrInfo: vi.fn().mockResolvedValue(task),
|
||||
logEntry: vi.fn().mockResolvedValue(undefined),
|
||||
on: vi.fn(),
|
||||
off: vi.fn(),
|
||||
} as unknown as TaskStore;
|
||||
@@ -459,3 +486,168 @@ describe("POST /tasks/:id/revert — FN-7524 mode + AI-undo fallback", () => {
|
||||
expect(performTaskRevertMock.mock.calls[0]?.[0]).toMatchObject({ granularity: "squash" });
|
||||
});
|
||||
});
|
||||
|
||||
// FN-7554: mode:"pr" — PR-based revert for autoMerge:false projects.
|
||||
describe("POST /tasks/:id/revert — FN-7554 mode:'pr' (autoMerge:false)", () => {
|
||||
const originalGithubRepository = process.env.GITHUB_REPOSITORY;
|
||||
|
||||
afterEach(() => {
|
||||
vi.clearAllMocks();
|
||||
vi.restoreAllMocks();
|
||||
if (originalGithubRepository === undefined) {
|
||||
delete process.env.GITHUB_REPOSITORY;
|
||||
} else {
|
||||
process.env.GITHUB_REPOSITORY = originalGithubRepository;
|
||||
}
|
||||
});
|
||||
|
||||
it("clean + autoMerge:false → mode:'pr', pushes and creates the PR with manual:true persistence", async () => {
|
||||
process.env.GITHUB_REPOSITORY = "o/r";
|
||||
const task = makeTask({ id: "FN-100", column: "done" });
|
||||
const store = createMockStore(task, { autoMerge: false });
|
||||
const rootDir = (store.getRootDir as () => string)();
|
||||
// `prepareRevertPrBranch` is mocked (real branch-prep behavior is proven by
|
||||
// task-revert-pr.real-git.test.ts), so create the branch it would have
|
||||
// created locally, so the route's REAL `git push -u origin <branch>` has
|
||||
// something to push.
|
||||
execFileSync("git", ["branch", "fusion/revert-fn-100"], { cwd: rootDir });
|
||||
vi.spyOn(githubRateLimiter, "canMakeRequest").mockReturnValue(true);
|
||||
findPrForBranchMock.mockResolvedValue(null);
|
||||
prepareRevertPrBranchMock.mockResolvedValue({
|
||||
eligible: true,
|
||||
revertBranch: "fusion/revert-fn-100",
|
||||
revertCommitShas: ["abc"],
|
||||
});
|
||||
createPrMock.mockResolvedValue({ number: 7, url: "https://github.com/o/r/pull/7" });
|
||||
|
||||
const res = await REQUEST(createApp(store), "POST", `/api/tasks/${task.id}/revert`);
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body).toMatchObject({
|
||||
mode: "pr",
|
||||
clean: true,
|
||||
prUrl: "https://github.com/o/r/pull/7",
|
||||
prNumber: 7,
|
||||
revertBranch: "fusion/revert-fn-100",
|
||||
});
|
||||
expect(createPrMock).toHaveBeenCalledTimes(1);
|
||||
expect(createPrMock.mock.calls[0]?.[0]).toMatchObject({ head: "fusion/revert-fn-100" });
|
||||
expect(typeof createPrMock.mock.calls[0]?.[0]?.body).toBe("string");
|
||||
expect((createPrMock.mock.calls[0]?.[0]?.body as string).length).toBeGreaterThan(0);
|
||||
expect(store.updatePrInfo as ReturnType<typeof vi.fn>).toHaveBeenCalledWith(
|
||||
task.id,
|
||||
expect.objectContaining({ manual: true, number: 7 }),
|
||||
);
|
||||
expect(performTaskRevertMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("existing PR idempotency: links the existing PR without re-preparing/re-pushing", async () => {
|
||||
process.env.GITHUB_REPOSITORY = "o/r";
|
||||
const task = makeTask({ id: "FN-100", column: "done" });
|
||||
const store = createMockStore(task, { autoMerge: false });
|
||||
vi.spyOn(githubRateLimiter, "canMakeRequest").mockReturnValue(true);
|
||||
findPrForBranchMock.mockResolvedValue({ number: 9, url: "https://github.com/o/r/pull/9" });
|
||||
|
||||
const res = await REQUEST(createApp(store), "POST", `/api/tasks/${task.id}/revert`);
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body).toMatchObject({
|
||||
mode: "pr",
|
||||
clean: true,
|
||||
prUrl: "https://github.com/o/r/pull/9",
|
||||
prNumber: 9,
|
||||
existingPr: true,
|
||||
});
|
||||
expect(prepareRevertPrBranchMock).not.toHaveBeenCalled();
|
||||
expect(createPrMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("GitHub unconfigured degrade: no GITHUB_REPOSITORY and no git remote → needsHuman", async () => {
|
||||
delete process.env.GITHUB_REPOSITORY;
|
||||
const task = makeTask({ id: "FN-100", column: "done" });
|
||||
const store = createMockStore(task, { autoMerge: false });
|
||||
|
||||
const res = await REQUEST(createApp(store), "POST", `/api/tasks/${task.id}/revert`);
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body).toMatchObject({ mode: "git", needsHuman: true });
|
||||
expect(String((res.body as { reason?: string }).reason ?? "")).toMatch(/no GitHub repository/i);
|
||||
expect(prepareRevertPrBranchMock).not.toHaveBeenCalled();
|
||||
expect(createPrMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rate-limited degrade: needsHuman without touching prepareRevertPrBranch/createPr", async () => {
|
||||
process.env.GITHUB_REPOSITORY = "o/r";
|
||||
const task = makeTask({ id: "FN-100", column: "done" });
|
||||
const store = createMockStore(task, { autoMerge: false });
|
||||
vi.spyOn(githubRateLimiter, "canMakeRequest").mockReturnValue(false);
|
||||
|
||||
const res = await REQUEST(createApp(store), "POST", `/api/tasks/${task.id}/revert`);
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body).toMatchObject({ mode: "git", needsHuman: true });
|
||||
expect(String((res.body as { reason?: string }).reason ?? "")).toMatch(/rate limit/i);
|
||||
expect(prepareRevertPrBranchMock).not.toHaveBeenCalled();
|
||||
expect(createPrMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("conflicting under autoMerge:false, mode:'git' → { mode: 'git', clean: false, conflicts } without a PR", async () => {
|
||||
process.env.GITHUB_REPOSITORY = "o/r";
|
||||
const task = makeTask({ id: "FN-100", column: "done" });
|
||||
const store = createMockStore(task, { autoMerge: false });
|
||||
vi.spyOn(githubRateLimiter, "canMakeRequest").mockReturnValue(true);
|
||||
findPrForBranchMock.mockResolvedValue(null);
|
||||
prepareRevertPrBranchMock.mockResolvedValue({
|
||||
eligible: false,
|
||||
classification: "conflicting",
|
||||
conflicts: [{ file: "foo.ts", status: "UU" }],
|
||||
});
|
||||
|
||||
const res = await POST_JSON(createApp(store), `/api/tasks/${task.id}/revert`, { mode: "git" });
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body).toMatchObject({ mode: "git", clean: false, conflicts: [{ file: "foo.ts", status: "UU" }] });
|
||||
expect(createPrMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("conflicting under autoMerge:false, mode:'auto' → falls back to the AI-undo task", async () => {
|
||||
process.env.GITHUB_REPOSITORY = "o/r";
|
||||
const task = makeTask({ id: "FN-960", column: "done" });
|
||||
const store = createMockStore(task, { autoMerge: false });
|
||||
vi.spyOn(githubRateLimiter, "canMakeRequest").mockReturnValue(true);
|
||||
findPrForBranchMock.mockResolvedValue(null);
|
||||
prepareRevertPrBranchMock.mockResolvedValue({
|
||||
eligible: false,
|
||||
classification: "conflicting",
|
||||
conflicts: [{ file: "foo.ts", status: "UU" }],
|
||||
});
|
||||
|
||||
const res = await POST_JSON(createApp(store), `/api/tasks/${task.id}/revert`, { mode: "auto" });
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body).toMatchObject({ mode: "ai" });
|
||||
expect((res.body as { createdTaskId?: string }).createdTaskId).toBeTruthy();
|
||||
expect(createPrMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("regression — autoMerge:true unchanged: still calls performTaskRevert and returns the existing shape", async () => {
|
||||
process.env.GITHUB_REPOSITORY = "o/r";
|
||||
const task = makeTask({ id: "FN-970", column: "done" });
|
||||
const store = createMockStore(task, { autoMerge: true });
|
||||
performTaskRevertMock.mockResolvedValue({ mode: "git", clean: true, revertCommitSha: "abc123", revertCommitShas: ["abc123"] });
|
||||
|
||||
const res = await REQUEST(createApp(store), "POST", `/api/tasks/${task.id}/revert`);
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body).toMatchObject({ mode: "git", clean: true, revertCommitSha: "abc123" });
|
||||
expect(prepareRevertPrBranchMock).not.toHaveBeenCalled();
|
||||
expect(createPrMock).not.toHaveBeenCalled();
|
||||
expect(performTaskRevertMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("regression — non-done/archived guard unchanged: still 4xx before any engine/GitHub call", async () => {
|
||||
process.env.GITHUB_REPOSITORY = "o/r";
|
||||
const task = makeTask({ id: "FN-971", column: "in-progress" });
|
||||
const store = createMockStore(task, { autoMerge: false });
|
||||
|
||||
const res = await REQUEST(createApp(store), "POST", `/api/tasks/${task.id}/revert`);
|
||||
expect(res.status).toBeGreaterThanOrEqual(400);
|
||||
expect(res.status).toBeLessThan(500);
|
||||
expect(prepareRevertPrBranchMock).not.toHaveBeenCalled();
|
||||
expect(performTaskRevertMock).not.toHaveBeenCalled();
|
||||
expect(createPrMock).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -14,6 +14,7 @@ import type {
|
||||
DuplicateMatch,
|
||||
RunAuditEvent,
|
||||
ArtifactType,
|
||||
PrInfo,
|
||||
} from "@fusion/core";
|
||||
import {
|
||||
COLUMNS,
|
||||
@@ -47,6 +48,7 @@ import {
|
||||
type NearDuplicateCandidate,
|
||||
} from "@fusion/core";
|
||||
import { GitHubClient } from "../github.js";
|
||||
import { githubRateLimiter } from "../github-poll.js";
|
||||
import { createTrackingIssueForTask } from "../github-tracking-hook.js";
|
||||
import { parseGitHubBadgeUrl } from "./register-git-github.js";
|
||||
import {
|
||||
@@ -56,7 +58,9 @@ import {
|
||||
revertWorkspaceTask,
|
||||
TaskRevertError,
|
||||
createAiUndoTask,
|
||||
prepareRevertPrBranch,
|
||||
type AiUndoTaskResult,
|
||||
type PrepareRevertPrBranchResult,
|
||||
} from "@fusion/engine";
|
||||
import { buildBoardWorkflowsPayload } from "./board-workflows.js";
|
||||
import { isBackwardMoveBlockedByOpenPr, PR_OPEN_BLOCKS_MOVE_BACK_MESSAGE } from "./register-pull-requests-routes.js";
|
||||
@@ -1819,6 +1823,184 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
||||
});
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:TaskRevert 2026-07-05-00:00 (FN-7554 — mode:"pr" contract, additive over FN-7523/24/47/48):
|
||||
`performTaskRevert` refuses (`needsHuman`) whenever autoMerge is
|
||||
effectively off, because it would otherwise force-write a revert commit
|
||||
directly onto `baseBranch` — a branch this project has opted out of
|
||||
automated writes to. Instead of stopping at that dead end, open a real
|
||||
revert PR: `prepareRevertPrBranch` (engine) applies the revert commit(s)
|
||||
onto a DEDICATED `fusion/revert-<id>` branch off `baseBranch` HEAD
|
||||
(never mutating `baseBranch` itself), then this route pushes that branch
|
||||
and opens a PR via `GitHubClient.createPr` — reusing the exact
|
||||
owner/repo resolution, `githubRateLimiter` gate, `findPrForBranch`
|
||||
idempotency, and `manual: true` handoff that `/pr/create` already uses.
|
||||
This ONLY applies to the single-repo git path (workspace tasks already
|
||||
returned above) and ONLY for `mode !== "ai"` (the `mode === "ai"` case
|
||||
already returned earlier in this handler). Every existing
|
||||
`{ mode: "git" | "ai", ... }` result shape is unchanged — this adds a
|
||||
new `{ mode: "pr", ... }` variant. Graceful `needsHuman` degrade (NOT a
|
||||
thrown error) covers: GitHub unconfigured, and GitHub rate-limited —
|
||||
both leave the caller with the same actionable `needsHuman` contract
|
||||
the `autoMerge:true` code path never has to think about.
|
||||
PR-based revert of WORKSPACE (multi-repo) tasks under autoMerge:false is
|
||||
explicitly deferred (would require per-sub-repo branches + multiple
|
||||
PRs) — see the FN-7554 follow-up task; workspace tasks above still get
|
||||
the existing `needsHuman` result from `revertWorkspaceTask`.
|
||||
*/
|
||||
const effectiveAutoMerge = task.autoMerge ?? settings.autoMerge ?? true;
|
||||
if (effectiveAutoMerge === false) {
|
||||
let owner: string;
|
||||
let repo: string;
|
||||
const envRepo = process.env.GITHUB_REPOSITORY;
|
||||
if (envRepo) {
|
||||
const [o, r] = envRepo.split("/");
|
||||
owner = o;
|
||||
repo = r;
|
||||
} else {
|
||||
const gitRepo = getCurrentRepo(rootDir);
|
||||
if (!gitRepo) {
|
||||
res.json({
|
||||
mode: "git",
|
||||
needsHuman: true,
|
||||
reason: "autoMerge is disabled and no GitHub repository is configured; cannot open a revert PR",
|
||||
});
|
||||
return;
|
||||
}
|
||||
owner = gitRepo.owner;
|
||||
repo = gitRepo.repo;
|
||||
}
|
||||
|
||||
const repoKey = `${owner}/${repo}`;
|
||||
if (!githubRateLimiter.canMakeRequest(repoKey)) {
|
||||
res.json({
|
||||
mode: "git",
|
||||
needsHuman: true,
|
||||
reason: "GitHub API rate limit exceeded; try again later",
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
const revertBranch = `fusion/revert-${task.id.toLowerCase()}`;
|
||||
const client = new GitHubClient();
|
||||
let existingPr: Awaited<ReturnType<typeof client.findPrForBranch>>;
|
||||
try {
|
||||
existingPr = await client.findPrForBranch({ head: revertBranch, state: "all", owner, repo });
|
||||
} catch (error) {
|
||||
// FNXC:TaskRevert 2026-07-05-00:00 (FN-7554): GitHub reachability failure
|
||||
// (network down, auth rejected, 5xx, etc.) degrades to needsHuman with an
|
||||
// explicit reason rather than bubbling up as a 500 — mirrors the
|
||||
// no-remote-configured / rate-limited degrade paths above.
|
||||
res.json({
|
||||
mode: "git",
|
||||
needsHuman: true,
|
||||
reason: `GitHub is unavailable; could not check for an existing revert PR (${error instanceof Error ? error.message : String(error)})`,
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
const persistPrInfo = async (prInfo: PrInfo): Promise<void> => {
|
||||
const existingPrs = task.prInfos ?? (task.prInfo ? [task.prInfo] : []);
|
||||
if (existingPrs.length > 0) {
|
||||
await scopedStore.addPrInfo(task.id, prInfo);
|
||||
} else {
|
||||
await scopedStore.updatePrInfo(task.id, prInfo);
|
||||
}
|
||||
};
|
||||
|
||||
if (existingPr) {
|
||||
// Idempotency — mirrors `/pr/create`: never re-prepare/re-push when an
|
||||
// open (or all-state) PR already exists for this branch, just link it.
|
||||
const prInfo: PrInfo = { ...existingPr, manual: true };
|
||||
await persistPrInfo(prInfo);
|
||||
await scopedStore.logEntry(task.id, "Linked existing revert PR", `PR #${prInfo.number}: ${prInfo.url}`);
|
||||
res.json({
|
||||
mode: "pr",
|
||||
clean: true,
|
||||
prUrl: prInfo.url,
|
||||
prNumber: prInfo.number,
|
||||
revertBranch,
|
||||
existingPr: true,
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
const prepared: PrepareRevertPrBranchResult = await prepareRevertPrBranch({
|
||||
task,
|
||||
worktreePath: rootDir,
|
||||
baseBranch,
|
||||
revertBranch,
|
||||
commitAssociationSource: {
|
||||
getTaskCommitAssociationsByLineageId: (lineageId: string) =>
|
||||
scopedStore.getTaskCommitAssociationsByLineageId(lineageId),
|
||||
},
|
||||
});
|
||||
|
||||
if (!prepared.eligible) {
|
||||
if ("alreadyReverted" in prepared && prepared.alreadyReverted) {
|
||||
res.json({ mode: "git", clean: true, alreadyReverted: true });
|
||||
return;
|
||||
}
|
||||
if ("classification" in prepared && prepared.classification === "conflicting") {
|
||||
if (mode === "auto") {
|
||||
res.json(await createAiUndoResult());
|
||||
return;
|
||||
}
|
||||
res.json({ mode: "git", clean: false, conflicts: prepared.conflicts });
|
||||
return;
|
||||
}
|
||||
if ("unsupported" in prepared && prepared.unsupported) {
|
||||
if (mode === "auto") {
|
||||
res.json(await createAiUndoResult());
|
||||
return;
|
||||
}
|
||||
res.json({ mode: "git", unsupported: true, reason: prepared.reason });
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
if (prepared.eligible) {
|
||||
// FNXC:TaskRevert 2026-07-05-00:00 (FN-7554): push/create-PR failures
|
||||
// (network down, auth rejected, remote rejects push, GitHub 5xx, etc.)
|
||||
// degrade to needsHuman with an explicit reason instead of a thrown 500 —
|
||||
// the revert branch/commit(s) already prepared locally are left in place
|
||||
// (never force-written to `baseBranch`) so a retry can reuse them.
|
||||
try {
|
||||
await runGitCommand(["push", "-u", "origin", prepared.revertBranch], rootDir, 60_000);
|
||||
const prTitle = `revert(${task.id}): undo landed work`;
|
||||
const prBody =
|
||||
`This PR reverts the work landed by task ${task.id}.\n\n` +
|
||||
`See \`GET /api/tasks/${task.id}/diff\` for the landed diff being reverted.\n`;
|
||||
const created = await client.createPr({
|
||||
owner,
|
||||
repo,
|
||||
title: prTitle,
|
||||
body: prBody,
|
||||
head: prepared.revertBranch,
|
||||
base: baseBranch,
|
||||
});
|
||||
const prInfo: PrInfo = { ...created, manual: true };
|
||||
await persistPrInfo(prInfo);
|
||||
await scopedStore.logEntry(task.id, "Created revert PR", `PR #${prInfo.number}: ${prInfo.url}`);
|
||||
res.json({
|
||||
mode: "pr",
|
||||
clean: true,
|
||||
prUrl: prInfo.url,
|
||||
prNumber: prInfo.number,
|
||||
revertBranch: prepared.revertBranch,
|
||||
});
|
||||
return;
|
||||
} catch (error) {
|
||||
res.json({
|
||||
mode: "git",
|
||||
needsHuman: true,
|
||||
reason: `GitHub is unavailable; could not push the revert branch or open the PR (${error instanceof Error ? error.message : String(error)})`,
|
||||
});
|
||||
return;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const result = await performTaskRevert({
|
||||
task,
|
||||
worktreePath: rootDir,
|
||||
|
||||
206
packages/engine/src/__tests__/task-revert-pr.real-git.test.ts
Normal file
206
packages/engine/src/__tests__/task-revert-pr.real-git.test.ts
Normal file
@@ -0,0 +1,206 @@
|
||||
import { execSync, spawnSync } from "node:child_process";
|
||||
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { prepareRevertPrBranch } from "../task-revert.js";
|
||||
import type { Task } from "@fusion/core";
|
||||
|
||||
const hasGit = spawnSync("git", ["--version"], { stdio: "pipe" }).status === 0;
|
||||
const describeIfGit = hasGit ? describe : describe.skip;
|
||||
|
||||
function git(repo: string, command: string): string {
|
||||
return execSync(command, { cwd: repo, encoding: "utf-8", stdio: ["pipe", "pipe", "pipe"] }).trim();
|
||||
}
|
||||
|
||||
function makeTask(overrides: Partial<Task>): Task {
|
||||
return {
|
||||
id: "FN-A",
|
||||
lineageId: "FN-A",
|
||||
description: "",
|
||||
column: "done",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
...overrides,
|
||||
} as Task;
|
||||
}
|
||||
|
||||
// FN-7554: real-git regression coverage for the PR-based revert branch-prep
|
||||
// helper — clean → dedicated branch (base never mutated), conflicting/
|
||||
// already-reverted/unsupported pass-through, idempotent local branch reset,
|
||||
// and dirty-tree refusal.
|
||||
describeIfGit("prepareRevertPrBranch real-git scenarios", { timeout: 30_000 }, () => {
|
||||
const dirs: string[] = [];
|
||||
afterEach(() => {
|
||||
for (const dir of dirs.splice(0)) rmSync(dir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
function repoFixture() {
|
||||
const repo = mkdtempSync(join(tmpdir(), "kb-revert-pr-"));
|
||||
dirs.push(repo);
|
||||
git(repo, "git init -b main");
|
||||
git(repo, 'git config user.email "test@example.com"');
|
||||
git(repo, 'git config user.name "Test User"');
|
||||
git(repo, "git config commit.gpgsign false");
|
||||
writeFileSync(join(repo, "foo.ts"), "line1\n");
|
||||
git(repo, "git add foo.ts && git commit -m 'init'");
|
||||
return repo;
|
||||
}
|
||||
|
||||
it("clean → eligible: creates fusion/revert-<id> branch with revert commit, base untouched", async () => {
|
||||
const repo = repoFixture();
|
||||
writeFileSync(join(repo, "foo.ts"), "line1\nfeature-a\n");
|
||||
git(repo, "git add foo.ts && git commit -m 'feat(FN-A): add feature a'");
|
||||
const sha = git(repo, "git rev-parse HEAD");
|
||||
const mainHeadBefore = git(repo, "git rev-parse main");
|
||||
|
||||
const task = makeTask({ mergeDetails: { commitSha: sha, mergeTargetBranch: "main" } });
|
||||
const result = await prepareRevertPrBranch({
|
||||
task,
|
||||
worktreePath: repo,
|
||||
baseBranch: "main",
|
||||
revertBranch: "fusion/revert-fn-a",
|
||||
});
|
||||
|
||||
expect(result).toMatchObject({ eligible: true, revertBranch: "fusion/revert-fn-a" });
|
||||
if (result.eligible) {
|
||||
expect(result.revertCommitShas.length).toBe(1);
|
||||
}
|
||||
|
||||
// (a) revert branch exists, tip is a revert(FN-A): commit carrying the trailer.
|
||||
const branchTipSubject = git(repo, "git log -1 --format=%s fusion/revert-fn-a");
|
||||
expect(branchTipSubject).toMatch(/^revert\(FN-A\):/);
|
||||
const branchTipBody = git(repo, "git log -1 --format=%B fusion/revert-fn-a");
|
||||
expect(branchTipBody).toContain("Fusion-Task-Id: FN-A");
|
||||
expect(git(repo, "git show fusion/revert-fn-a:foo.ts")).toBe("line1");
|
||||
|
||||
// (b) main HEAD is byte-identical to before the call — base never written.
|
||||
expect(git(repo, "git rev-parse main")).toBe(mainHeadBefore);
|
||||
|
||||
// (c) checkout restored to main and clean.
|
||||
expect(git(repo, "git rev-parse --abbrev-ref HEAD")).toBe("main");
|
||||
expect(git(repo, "git status --porcelain")).toBe("");
|
||||
});
|
||||
|
||||
it("conflicting → pass-through: no revert branch left behind, base + checkout unchanged", async () => {
|
||||
const repo = repoFixture();
|
||||
writeFileSync(join(repo, "foo.ts"), "line1\nfeature-a\n");
|
||||
git(repo, "git add foo.ts && git commit -m 'feat(FN-A): add feature a'");
|
||||
const shaA = git(repo, "git rev-parse HEAD");
|
||||
|
||||
// Task B later modifies the exact same region touched by task A.
|
||||
writeFileSync(join(repo, "foo.ts"), "line1\nfeature-a-modified-by-b\n");
|
||||
git(repo, "git commit -am 'feat(FN-B): modify same region'");
|
||||
|
||||
const mainHeadBefore = git(repo, "git rev-parse main");
|
||||
const statusBefore = git(repo, "git status --porcelain");
|
||||
|
||||
const task = makeTask({ mergeDetails: { commitSha: shaA, mergeTargetBranch: "main" } });
|
||||
const result = await prepareRevertPrBranch({
|
||||
task,
|
||||
worktreePath: repo,
|
||||
baseBranch: "main",
|
||||
revertBranch: "fusion/revert-fn-a",
|
||||
});
|
||||
|
||||
expect(result).toMatchObject({ eligible: false, classification: "conflicting" });
|
||||
if (!result.eligible && result.classification === "conflicting") {
|
||||
expect(result.conflicts.length).toBeGreaterThan(0);
|
||||
expect(result.conflicts.some((c) => c.file === "foo.ts")).toBe(true);
|
||||
}
|
||||
|
||||
const branchList = git(repo, "git branch --list fusion/revert-fn-a");
|
||||
expect(branchList).toBe("");
|
||||
expect(git(repo, "git rev-parse main")).toBe(mainHeadBefore);
|
||||
expect(git(repo, "git rev-parse --abbrev-ref HEAD")).toBe("main");
|
||||
expect(git(repo, "git status --porcelain")).toBe(statusBefore);
|
||||
});
|
||||
|
||||
it("already-reverted → pass-through: no branch, base unchanged", async () => {
|
||||
const repo = repoFixture();
|
||||
writeFileSync(join(repo, "foo.ts"), "line1\nfeature-a\n");
|
||||
git(repo, "git add foo.ts && git commit -m 'feat(FN-A): add feature a'");
|
||||
const sha = git(repo, "git rev-parse HEAD");
|
||||
|
||||
// Manually revert the change on main before calling prepareRevertPrBranch.
|
||||
git(repo, `git revert --no-edit ${sha}`);
|
||||
const mainHeadBefore = git(repo, "git rev-parse main");
|
||||
|
||||
const task = makeTask({ mergeDetails: { commitSha: sha, mergeTargetBranch: "main" } });
|
||||
const result = await prepareRevertPrBranch({
|
||||
task,
|
||||
worktreePath: repo,
|
||||
baseBranch: "main",
|
||||
revertBranch: "fusion/revert-fn-a",
|
||||
});
|
||||
|
||||
expect(result).toMatchObject({ eligible: false, classification: "already-reverted", alreadyReverted: true });
|
||||
const branchList = git(repo, "git branch --list fusion/revert-fn-a");
|
||||
expect(branchList).toBe("");
|
||||
expect(git(repo, "git rev-parse main")).toBe(mainHeadBefore);
|
||||
expect(git(repo, "git rev-parse --abbrev-ref HEAD")).toBe("main");
|
||||
});
|
||||
|
||||
it("workspace unsupported: a task with workspaceWorktrees populated is refused", async () => {
|
||||
const repo = repoFixture();
|
||||
const task = makeTask({
|
||||
workspaceWorktrees: { "repo-a": { worktreePath: "/tmp/whatever", branch: "main" } },
|
||||
});
|
||||
const result = await prepareRevertPrBranch({
|
||||
task,
|
||||
worktreePath: repo,
|
||||
baseBranch: "main",
|
||||
revertBranch: "fusion/revert-fn-a",
|
||||
});
|
||||
expect(result).toMatchObject({ eligible: false, unsupported: true, reason: "workspace-task-pr-revert-unsupported" });
|
||||
});
|
||||
|
||||
it("idempotent local branch reset: a stale local branch pointing elsewhere is reset off base with the fresh revert commit", async () => {
|
||||
const repo = repoFixture();
|
||||
writeFileSync(join(repo, "foo.ts"), "line1\nfeature-a\n");
|
||||
git(repo, "git add foo.ts && git commit -m 'feat(FN-A): add feature a'");
|
||||
const sha = git(repo, "git rev-parse HEAD");
|
||||
|
||||
// Pre-create a stale local branch pointing at an unrelated commit.
|
||||
git(repo, "git branch fusion/revert-fn-a main~1");
|
||||
const staleTip = git(repo, "git rev-parse fusion/revert-fn-a");
|
||||
expect(staleTip).not.toBe(git(repo, "git rev-parse main"));
|
||||
|
||||
const task = makeTask({ mergeDetails: { commitSha: sha, mergeTargetBranch: "main" } });
|
||||
const result = await prepareRevertPrBranch({
|
||||
task,
|
||||
worktreePath: repo,
|
||||
baseBranch: "main",
|
||||
revertBranch: "fusion/revert-fn-a",
|
||||
});
|
||||
|
||||
expect(result).toMatchObject({ eligible: true });
|
||||
const branchTipSubject = git(repo, "git log -1 --format=%s fusion/revert-fn-a");
|
||||
expect(branchTipSubject).toMatch(/^revert\(FN-A\):/);
|
||||
expect(git(repo, "git rev-parse --abbrev-ref HEAD")).toBe("main");
|
||||
});
|
||||
|
||||
it("dirty-tree refusal: a stray staged change is refused without any branch/base mutation", async () => {
|
||||
const repo = repoFixture();
|
||||
writeFileSync(join(repo, "foo.ts"), "line1\nfeature-a\n");
|
||||
git(repo, "git add foo.ts && git commit -m 'feat(FN-A): add feature a'");
|
||||
const sha = git(repo, "git rev-parse HEAD");
|
||||
|
||||
writeFileSync(join(repo, "stray.txt"), "stray change\n");
|
||||
git(repo, "git add stray.txt");
|
||||
|
||||
const mainHeadBefore = git(repo, "git rev-parse main");
|
||||
const preStatus = git(repo, "git status --porcelain");
|
||||
|
||||
const task = makeTask({ mergeDetails: { commitSha: sha, mergeTargetBranch: "main" } });
|
||||
await expect(
|
||||
prepareRevertPrBranch({ task, worktreePath: repo, baseBranch: "main", revertBranch: "fusion/revert-fn-a" }),
|
||||
).rejects.toThrow();
|
||||
|
||||
const branchList = git(repo, "git branch --list fusion/revert-fn-a");
|
||||
expect(branchList).toBe("");
|
||||
expect(git(repo, "git rev-parse main")).toBe(mainHeadBefore);
|
||||
expect(git(repo, "git status --porcelain")).toBe(preStatus);
|
||||
});
|
||||
});
|
||||
@@ -292,6 +292,9 @@ export {
|
||||
type WorkspaceRepoRevertResult,
|
||||
type WorkspaceTaskRevertResult,
|
||||
type RevertWorkspaceTaskOptions,
|
||||
prepareRevertPrBranch,
|
||||
type PrepareRevertPrBranchResult,
|
||||
type PrepareRevertPrBranchOptions,
|
||||
} from "./task-revert.js";
|
||||
export {
|
||||
resolveBranchGroupMergeRouting,
|
||||
|
||||
@@ -756,6 +756,160 @@ async function applyAndCommitRevert(opts: {
|
||||
}
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// FN-7554: PR-based revert for autoMerge:false projects.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
export type PrepareRevertPrBranchResult =
|
||||
| { eligible: true; revertBranch: string; revertCommitShas: string[] }
|
||||
| { eligible: false; classification: "conflicting"; conflicts: TaskRevertConflict[] }
|
||||
| { eligible: false; classification: "already-reverted"; alreadyReverted: true }
|
||||
| { eligible: false; unsupported: true; reason: string };
|
||||
|
||||
export interface PrepareRevertPrBranchOptions {
|
||||
task: Pick<Task, "id" | "lineageId" | "column" | "mergeDetails" | "workspaceWorktrees">;
|
||||
/** The shared checkout, verified on `baseBranch` by the caller before this is invoked. */
|
||||
worktreePath: string;
|
||||
/** Resolved mergeTargetBranch / integration branch. NEVER written to — see the doc comment below. */
|
||||
baseBranch: string;
|
||||
/** e.g. `fusion/revert-<task-id-lowercase>`. */
|
||||
revertBranch: string;
|
||||
execAsyncImpl?: ExecAsyncImpl;
|
||||
commitAssociationSource?: TaskCommitAssociationSource;
|
||||
}
|
||||
|
||||
/**
|
||||
* FNXC:TaskRevert 2026-07-05-00:00 (FN-7554 — PR-based revert for
|
||||
* autoMerge:false projects):
|
||||
*
|
||||
* `performTaskRevert` refuses (`needsHuman`) whenever autoMerge is
|
||||
* effectively off, because it commits directly onto `worktreePath`'s current
|
||||
* HEAD branch (the base branch) and this project has opted that branch out
|
||||
* of automated writes. This function gives that dead end an actionable path:
|
||||
* it prepares a DEDICATED `revertBranch` off `baseBranch`'s HEAD, applies the
|
||||
* revert commit(s) onto THAT branch only, and leaves `baseBranch` itself
|
||||
* completely untouched — the caller (the API route) then pushes the branch
|
||||
* and opens a real GitHub PR against `baseBranch`, so the change still lands
|
||||
* through the project's normal human-review flow instead of a forced write.
|
||||
*
|
||||
* NEVER-WRITE-TO-BASE INVARIANT: this function only ever mutates
|
||||
* `revertBranch`. `baseBranch`'s ref is never advanced, reset, or committed
|
||||
* to. The shared checkout (`worktreePath`) is ALWAYS restored to the branch
|
||||
* it was on when this function was called (`originalBranch`), in a `finally`
|
||||
* — regardless of success, classification pass-through, or thrown failure —
|
||||
* so a caller that shares this checkout across requests never observes it
|
||||
* left mid-revert on `revertBranch`.
|
||||
*
|
||||
* REUSE, NOT REIMPLEMENTATION: commit application/message/trailer generation
|
||||
* is delegated entirely to the shared `applyAndCommitRevert` helper (the same
|
||||
* one `performTaskRevert`'s squash path and `revertWorkspaceTask` use) — this
|
||||
* function only adds the branch-prep/checkout-restore choreography around it.
|
||||
*
|
||||
* WORKSPACE DEFERRAL: workspace (multi-repo) tasks are refused with
|
||||
* `{ eligible: false, unsupported: true, reason: "workspace-task-pr-revert-unsupported" }`
|
||||
* — a single PR against a single base branch cannot coherently represent a
|
||||
* multi-repo, multi-branch revert. PR-based workspace revert is explicitly
|
||||
* out of scope here (see the FN-7554 PROMPT's Step 6 follow-up task).
|
||||
*/
|
||||
export async function prepareRevertPrBranch(opts: PrepareRevertPrBranchOptions): Promise<PrepareRevertPrBranchResult> {
|
||||
const { task, worktreePath, baseBranch, revertBranch } = opts;
|
||||
const execImpl = opts.execAsyncImpl ?? defaultExecAsync;
|
||||
|
||||
if (isWorkspaceTask(task)) {
|
||||
return { eligible: false, unsupported: true, reason: "workspace-task-pr-revert-unsupported" };
|
||||
}
|
||||
|
||||
const resolved = await resolveTaskRevertCommits(task, {
|
||||
worktreePath,
|
||||
execAsyncImpl: execImpl,
|
||||
commitAssociationSource: opts.commitAssociationSource,
|
||||
});
|
||||
if (!resolved.supported) {
|
||||
return { eligible: false, unsupported: true, reason: resolved.reason };
|
||||
}
|
||||
|
||||
const classification = await classifyTaskRevert({
|
||||
worktreePath,
|
||||
commits: resolved.shas,
|
||||
execAsyncImpl: execImpl,
|
||||
});
|
||||
|
||||
if (classification.classification === "already-reverted") {
|
||||
return { eligible: false, classification: "already-reverted", alreadyReverted: true };
|
||||
}
|
||||
if (classification.classification === "conflicting") {
|
||||
return { eligible: false, classification: "conflicting", conflicts: classification.conflicts ?? [] };
|
||||
}
|
||||
|
||||
// classification === "clean" — prepare the dedicated revert branch.
|
||||
let originalBranch: string;
|
||||
try {
|
||||
const { stdout } = await runGit(execImpl, "git rev-parse --abbrev-ref HEAD", worktreePath);
|
||||
originalBranch = stdout.trim();
|
||||
} catch (error) {
|
||||
throw new TaskRevertError("failed to resolve current branch before preparing revert PR branch", "head-resolve-failed", error);
|
||||
}
|
||||
|
||||
const { stdout: statusOut } = await runGit(execImpl, "git status --porcelain", worktreePath);
|
||||
if (statusOut.trim().length > 0) {
|
||||
throw new TaskRevertError(
|
||||
"working tree is dirty; refusing to prepare a revert PR branch",
|
||||
"dirty-working-tree",
|
||||
);
|
||||
}
|
||||
|
||||
let branchCreated = false;
|
||||
try {
|
||||
// FNXC:TaskRevert 2026-07-05-00:00: `-B` (create-or-reset) makes re-running
|
||||
// this idempotent when a stale local `revertBranch` already exists from a
|
||||
// prior failed/aborted attempt — it is reset off `baseBranch` HEAD rather
|
||||
// than accumulating on top of whatever it previously pointed at. `baseBranch`
|
||||
// itself is only ever read here (`git checkout -B <revertBranch> <baseBranch>`
|
||||
// does not move `baseBranch`'s ref).
|
||||
await runGit(
|
||||
execImpl,
|
||||
`git checkout -B ${quoteShellArg(revertBranch)} ${quoteShellArg(baseBranch)}`,
|
||||
worktreePath,
|
||||
);
|
||||
branchCreated = true;
|
||||
|
||||
const applied = await applyAndCommitRevert({
|
||||
worktreePath,
|
||||
commits: resolved.shas,
|
||||
taskId: task.id,
|
||||
execAsyncImpl: execImpl,
|
||||
});
|
||||
|
||||
if ("alreadyReverted" in applied) {
|
||||
// Defensive: the branch moved between classify and apply. Nothing to
|
||||
// commit on the fresh revertBranch — treat as already-reverted.
|
||||
return { eligible: false, classification: "already-reverted", alreadyReverted: true };
|
||||
}
|
||||
if ("conflicts" in applied) {
|
||||
// Late conflict — applyAndCommitRevert already rolled worktreePath back
|
||||
// to the pre-apply HEAD (the tip of revertBranch, i.e. baseBranch's HEAD).
|
||||
return { eligible: false, classification: "conflicting", conflicts: applied.conflicts };
|
||||
}
|
||||
|
||||
return { eligible: true, revertBranch, revertCommitShas: [applied.revertCommitSha] };
|
||||
} catch (error) {
|
||||
// On any thrown failure after branch creation, never leave a dangling
|
||||
// partial revert branch behind — best-effort restore + delete.
|
||||
if (branchCreated) {
|
||||
await runGit(execImpl, `git checkout ${quoteShellArg(originalBranch)}`, worktreePath).catch(() => undefined);
|
||||
await runGit(execImpl, `git branch -D ${quoteShellArg(revertBranch)}`, worktreePath).catch(() => undefined);
|
||||
}
|
||||
throw error instanceof TaskRevertError ? error : new TaskRevertError("failed to prepare revert PR branch", "revert-pr-branch-prepare-failed", error);
|
||||
} finally {
|
||||
// FNXC:TaskRevert 2026-07-05-00:00: ALWAYS restore the shared checkout to
|
||||
// the branch it was on when this function was called, so the checkout is
|
||||
// left exactly where it started — on `baseBranch` (unmutated) in the
|
||||
// documented caller contract — regardless of success/pass-through/failure
|
||||
// above. The revert commit(s) live ONLY on `revertBranch`.
|
||||
await runGit(execImpl, `git checkout ${quoteShellArg(originalBranch)}`, worktreePath).catch(() => undefined);
|
||||
}
|
||||
}
|
||||
|
||||
export interface WorkspaceRepoRevertCommits {
|
||||
commits: string[];
|
||||
source: TaskRevertCommitSource;
|
||||
|
||||
Reference in New Issue
Block a user