diff --git a/.changeset/fn-7577-workspace-pr-revert.md b/.changeset/fn-7577-workspace-pr-revert.md new file mode 100644 index 0000000000..a231551ed5 --- /dev/null +++ b/.changeset/fn-7577-workspace-pr-revert.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": minor +--- + +summary: Open one revert PR per sub-repo for workspace tasks when autoMerge is disabled. +category: feature +dev: `POST /api/tasks/:id/revert` gains an additive workspace `{ mode: "pr", clean: true, workspace: { repos: [{ repo, revertBranch, prUrl, prNumber, existingPr? }] } }` result for clean multi-repo reverts under `autoMerge:false`, extending FN-7554's single-repo `mode:"pr"` path. New engine export `prepareWorkspaceRevertPrBranches` (packages/engine/src/task-revert.ts) classifies every sub-repo first and only prepares a dedicated `fusion/revert-` branch per sub-repo when all are clean/already-reverted (all-or-nothing at the branch-prep phase), never force-writing any sub-repo integration branch. The route resolves owner/repo and checks the rate limiter for every sub-repo before pushing/creating any PR, so GitHub-unconfigured/rate-limited cases degrade the whole task to `needsHuman` rather than opening a partial subset of PRs. Existing `{ mode: "git" | "ai" | "pr", ... }` shapes, the `autoMerge:true` workspace path, and FN-7554's single-repo path are unchanged. diff --git a/docs/task-management.md b/docs/task-management.md index b4f3bc8f47..f592e41fe2 100644 --- a/docs/task-management.md +++ b/docs/task-management.md @@ -695,7 +695,8 @@ 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` 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-` 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`. +- **`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-` 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). +- **`autoMerge:false` PR-based revert extended to workspace tasks (FN-7577):** a workspace task whose git revert classifies **clean across every sub-repo** also opens PRs instead of dead-ending at `needsHuman` under `autoMerge:false`. The engine's `prepareWorkspaceRevertPrBranches` mirrors the workspace all-or-nothing classify-all contract: it dry-run classifies EVERY sub-repo first, and only prepares one `fusion/revert-` branch per sub-repo (never writing any sub-repo's integration branch) when every sub-repo classifies clean/already-reverted — a single conflicting sub-repo aborts the WHOLE preparation with no branch created anywhere. The route then resolves owner/repo and checks the rate limiter for EVERY sub-repo before pushing/creating any PR (so a GitHub-unconfigured or rate-limited sub-repo degrades the whole task to `needsHuman` rather than opening a partial subset), then opens one PR per sub-repo reusing FN-7554's per-sub-repo `findPrForBranch` idempotency and `manual: true` handoff. Response: `{ mode: "pr", clean: true, workspace: { repos: [{ repo, revertBranch, prUrl, prNumber, existingPr? }] } }`. Existing `{ mode: "git" | "ai" | "pr" }` shapes, the `autoMerge:true` workspace path, and FN-7554's single-repo path are unchanged. - **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). - **Configurable AI-undo workflow default (FN-7556, UI: FN-7578):** the project setting `aiUndoTaskWorkflowId` (default `builtin:review-heavy`) selects the workflow applied to every AI-undo task created above (`mode:"ai"` and the `auto`/workspace conflict fallbacks all share one creation seam, so all three inherit this default) — a stricter review posture is warranted because these tasks reverse already-shipped code. A blank/unset value means the created task inherits the project default workflow (pre-FN-7556 behavior); the route falls back to inherit (with a logged warning) if the configured id is blank or does not resolve to a real workflow, so a misconfigured id never breaks AI-undo task creation. Editable from **Settings → General → AI-undo task workflow** (choose "Inherit project default workflow" to store the blank/inherit sentinel). See [Settings Reference → Project Settings](./settings-reference.md#project-settings). diff --git a/packages/dashboard/src/__tests__/task-revert-route.test.ts b/packages/dashboard/src/__tests__/task-revert-route.test.ts index 2566b1210d..6d7c38390b 100644 --- a/packages/dashboard/src/__tests__/task-revert-route.test.ts +++ b/packages/dashboard/src/__tests__/task-revert-route.test.ts @@ -11,7 +11,7 @@ this suite stubs `performTaskRevert` at the route boundary and asserts: */ import { afterEach, describe, expect, it, vi } from "vitest"; import express from "express"; -import { mkdtempSync } from "node:fs"; +import { mkdirSync, mkdtempSync } from "node:fs"; import { join } from "node:path"; import { tmpdir } from "node:os"; import { execFileSync } from "node:child_process"; @@ -20,6 +20,25 @@ import { createApiRoutes } from "../routes.js"; import { request as performRequest } from "../test-request.js"; import { githubRateLimiter } from "../github-poll.js"; +// FN-7577: `getCurrentRepo` is mocked at the `@fusion/core` boundary (partial +// mock, everything else passes through to the real module) so workspace +// mode:"pr" tests can resolve distinct owner/repo per sub-repo without a real +// GitHub remote. The returned wrapper defers reading `getCurrentRepoMock` (and +// falls back to the REAL `getCurrentRepo`) until CALL time — never inside the +// synchronous factory body — so existing single-repo FN-7554 tests (which +// rely on real local-remote resolution / the `GITHUB_REPOSITORY` env +// override) are unaffected; workspace tests below override via +// `mockImplementation`. +const getCurrentRepoMock = vi.fn(); +vi.mock("@fusion/core", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + getCurrentRepo: (...args: [string?]) => + getCurrentRepoMock.getMockImplementation() ? getCurrentRepoMock(...args) : actual.getCurrentRepo(...args), + }; +}); + // 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 // base branch (see the branch-mismatch check in register-task-workflow-routes.ts). @@ -44,6 +63,7 @@ function makeGitRepoOnMain(): string { const performTaskRevertMock = vi.fn(); const revertWorkspaceTaskMock = vi.fn(); const prepareRevertPrBranchMock = vi.fn(); +const prepareWorkspaceRevertPrBranchesMock = vi.fn(); vi.mock("@fusion/engine", async (importOriginal) => { const actual = await importOriginal(); @@ -52,6 +72,7 @@ vi.mock("@fusion/engine", async (importOriginal) => { performTaskRevert: (...args: unknown[]) => performTaskRevertMock(...args), revertWorkspaceTask: (...args: unknown[]) => revertWorkspaceTaskMock(...args), prepareRevertPrBranch: (...args: unknown[]) => prepareRevertPrBranchMock(...args), + prepareWorkspaceRevertPrBranches: (...args: unknown[]) => prepareWorkspaceRevertPrBranchesMock(...args), }; }); @@ -106,6 +127,29 @@ function makeWorkspaceTask(overrides: Partial): Task { }); } +// FN-7577: real multi-sub-repo git fixture for workspace mode:"pr" tests — +// each sub-repo is its own real git repo with a real bare "origin" remote, so +// the route's REAL `git push -u origin ` has something to push +// (mirrors `makeGitRepoOnMain`'s single-repo pattern, once per sub-repo). +function makeWorkspaceGitRoot(repoRels: string[]): { rootDir: string; repoDirs: Record } { + const rootDir = mkdtempSync(join(tmpdir(), "kb-task-revert-ws-route-")); + const repoDirs: Record = {}; + for (const rel of repoRels) { + const dir = join(rootDir, rel); + mkdirSync(dir, { recursive: true }); + execFileSync("git", ["init", "-b", "main"], { cwd: dir }); + 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 }); + const originDir = mkdtempSync(join(tmpdir(), "kb-task-revert-ws-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 }); + repoDirs[rel] = dir; + } + return { rootDir, repoDirs }; +} + function createMockStore( task: Task, opts?: { @@ -114,6 +158,7 @@ function createMockStore( autoMerge?: boolean; aiUndoTaskWorkflowId?: string; knownWorkflowIds?: string[]; + rootDir?: string; }, ): TaskStore { let nextId = 800; @@ -145,7 +190,7 @@ function createMockStore( aiUndoTaskWorkflowId: opts?.aiUndoTaskWorkflowId, }), getWorkflowDefinition, - getRootDir: vi.fn().mockReturnValue(makeGitRepoOnMain()), + getRootDir: vi.fn().mockReturnValue(opts?.rootDir ?? makeGitRepoOnMain()), getTask: vi.fn().mockResolvedValue(task), getTaskCommitAssociationsByLineageId: vi.fn().mockResolvedValue([]), createTask, @@ -699,3 +744,267 @@ describe("POST /tasks/:id/revert — FN-7554 mode:'pr' (autoMerge:false)", () => expect(createPrMock).not.toHaveBeenCalled(); }); }); + +// FN-7577: mode:"pr" — PR-based revert extended to WORKSPACE (multi-repo) +// tasks under autoMerge:false. Real per-sub-repo branch-prep behavior is +// proven by packages/engine/src/__tests__/task-revert-workspace-pr.real-git.test.ts; +// this suite stubs `prepareWorkspaceRevertPrBranches` at the engine boundary +// and `getCurrentRepo` at the core boundary, and asserts the route's +// per-sub-repo PR orchestration, atomic pre-check degrade ordering, and +// idempotency. +describe("POST /tasks/:id/revert — FN-7577 workspace 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; + } + }); + + function mockRepoResolution(resolvable: Record): void { + getCurrentRepoMock.mockImplementation((cwd?: string) => { + for (const [rel, value] of Object.entries(resolvable)) { + if (typeof cwd === "string" && cwd.endsWith(rel)) return value; + } + return null; + }); + } + + it("all clean + autoMerge:false → mode:'pr' (multi-PR), one PR per sub-repo, manual:true persistence", async () => { + delete process.env.GITHUB_REPOSITORY; + const task = makeWorkspaceTask({ id: "FN-100", column: "done" }); + const { rootDir, repoDirs } = makeWorkspaceGitRoot(["repo-a", "repo-b"]); + const store = createMockStore(task, { autoMerge: false, rootDir }); + // `prepareWorkspaceRevertPrBranches` is mocked (real branch-prep behavior is + // proven by the engine real-git suite) — create the branches it would have + // created locally, so the route's REAL `git push -u origin ` per + // sub-repo has something to push. + execFileSync("git", ["branch", "fusion/revert-fn-100"], { cwd: repoDirs["repo-a"] }); + execFileSync("git", ["branch", "fusion/revert-fn-100"], { cwd: repoDirs["repo-b"] }); + mockRepoResolution({ "repo-a": { owner: "o", repo: "repo-a" }, "repo-b": { owner: "o", repo: "repo-b" } }); + vi.spyOn(githubRateLimiter, "canMakeRequest").mockReturnValue(true); + findPrForBranchMock.mockResolvedValue(null); + prepareWorkspaceRevertPrBranchesMock.mockResolvedValue({ + eligible: true, + repos: [ + { repo: "repo-a", revertBranch: "fusion/revert-fn-100", integrationBranch: "main", revertCommitShas: ["a"] }, + { repo: "repo-b", revertBranch: "fusion/revert-fn-100", integrationBranch: "main", revertCommitShas: ["b"] }, + ], + }); + let callCount = 0; + createPrMock.mockImplementation(async () => { + callCount += 1; + return { number: 100 + callCount, url: `https://github.com/o/repo/pull/${100 + callCount}` }; + }); + + 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, + workspace: { + repos: [ + { repo: "repo-a", revertBranch: "fusion/revert-fn-100" }, + { repo: "repo-b", revertBranch: "fusion/revert-fn-100" }, + ], + }, + }); + expect(createPrMock).toHaveBeenCalledTimes(2); + expect(createPrMock.mock.calls[0]?.[0]).toMatchObject({ owner: "o", repo: "repo-a", head: "fusion/revert-fn-100", base: "main" }); + expect(createPrMock.mock.calls[1]?.[0]).toMatchObject({ owner: "o", repo: "repo-b", head: "fusion/revert-fn-100", base: "main" }); + for (const call of createPrMock.mock.calls) { + expect(typeof call[0]?.body).toBe("string"); + expect((call[0]?.body as string).length).toBeGreaterThan(0); + } + expect(store.updatePrInfo as ReturnType).toHaveBeenCalledWith(task.id, expect.objectContaining({ manual: true })); + expect(revertWorkspaceTaskMock).not.toHaveBeenCalled(); + expect(performTaskRevertMock).not.toHaveBeenCalled(); + }); + + it("existing PR idempotency: links repo-a's existing PR and only creates a PR for repo-b", async () => { + delete process.env.GITHUB_REPOSITORY; + const task = makeWorkspaceTask({ id: "FN-101", column: "done" }); + const { rootDir, repoDirs } = makeWorkspaceGitRoot(["repo-a", "repo-b"]); + const store = createMockStore(task, { autoMerge: false, rootDir }); + execFileSync("git", ["branch", "fusion/revert-fn-101"], { cwd: repoDirs["repo-b"] }); + mockRepoResolution({ "repo-a": { owner: "o", repo: "repo-a" }, "repo-b": { owner: "o", repo: "repo-b" } }); + vi.spyOn(githubRateLimiter, "canMakeRequest").mockReturnValue(true); + findPrForBranchMock.mockImplementation(async ({ repo }: { repo: string }) => + repo === "repo-a" ? { number: 55, url: "https://github.com/o/repo-a/pull/55" } : null, + ); + prepareWorkspaceRevertPrBranchesMock.mockResolvedValue({ + eligible: true, + repos: [ + { repo: "repo-a", revertBranch: "fusion/revert-fn-101", integrationBranch: "main", revertCommitShas: ["a"] }, + { repo: "repo-b", revertBranch: "fusion/revert-fn-101", integrationBranch: "main", revertCommitShas: ["b"] }, + ], + }); + createPrMock.mockResolvedValue({ number: 56, url: "https://github.com/o/repo-b/pull/56" }); + + 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, + workspace: { + repos: [ + { repo: "repo-a", prNumber: 55, existingPr: true }, + { repo: "repo-b", prNumber: 56 }, + ], + }, + }); + expect(createPrMock).toHaveBeenCalledTimes(1); + expect(createPrMock.mock.calls[0]?.[0]).toMatchObject({ repo: "repo-b" }); + }); + + it("GitHub unconfigured degrade (whole-task): needsHuman, no createPr for ANY sub-repo", async () => { + delete process.env.GITHUB_REPOSITORY; + const task = makeWorkspaceTask({ id: "FN-102", column: "done" }); + const { rootDir, repoDirs } = makeWorkspaceGitRoot(["repo-a", "repo-b"]); + const store = createMockStore(task, { autoMerge: false, rootDir }); + execFileSync("git", ["branch", "fusion/revert-fn-102"], { cwd: repoDirs["repo-a"] }); + execFileSync("git", ["branch", "fusion/revert-fn-102"], { cwd: repoDirs["repo-b"] }); + // repo-b has NO configured GitHub repository. + mockRepoResolution({ "repo-a": { owner: "o", repo: "repo-a" }, "repo-b": null }); + prepareWorkspaceRevertPrBranchesMock.mockResolvedValue({ + eligible: true, + repos: [ + { repo: "repo-a", revertBranch: "fusion/revert-fn-102", integrationBranch: "main", revertCommitShas: ["a"] }, + { repo: "repo-b", revertBranch: "fusion/revert-fn-102", integrationBranch: "main", revertCommitShas: ["b"] }, + ], + }); + + 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(createPrMock).not.toHaveBeenCalled(); + expect(findPrForBranchMock).not.toHaveBeenCalled(); + }); + + it("rate-limited degrade (whole-task): needsHuman without touching createPr for any sub-repo", async () => { + delete process.env.GITHUB_REPOSITORY; + const task = makeWorkspaceTask({ id: "FN-103", column: "done" }); + const { rootDir, repoDirs } = makeWorkspaceGitRoot(["repo-a", "repo-b"]); + const store = createMockStore(task, { autoMerge: false, rootDir }); + execFileSync("git", ["branch", "fusion/revert-fn-103"], { cwd: repoDirs["repo-a"] }); + execFileSync("git", ["branch", "fusion/revert-fn-103"], { cwd: repoDirs["repo-b"] }); + mockRepoResolution({ "repo-a": { owner: "o", repo: "repo-a" }, "repo-b": { owner: "o", repo: "repo-b" } }); + vi.spyOn(githubRateLimiter, "canMakeRequest").mockImplementation((repoKey: string) => repoKey !== "o/repo-b"); + prepareWorkspaceRevertPrBranchesMock.mockResolvedValue({ + eligible: true, + repos: [ + { repo: "repo-a", revertBranch: "fusion/revert-fn-103", integrationBranch: "main", revertCommitShas: ["a"] }, + { repo: "repo-b", revertBranch: "fusion/revert-fn-103", integrationBranch: "main", revertCommitShas: ["b"] }, + ], + }); + + 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(createPrMock).not.toHaveBeenCalled(); + expect(findPrForBranchMock).not.toHaveBeenCalled(); + }); + + it("conflicting under autoMerge:false, mode:'git' → { mode: 'git', clean: false, workspace, conflicts }, no PR", async () => { + delete process.env.GITHUB_REPOSITORY; + const task = makeWorkspaceTask({ id: "FN-104", column: "done" }); + const { rootDir } = makeWorkspaceGitRoot(["repo-a", "repo-b"]); + const store = createMockStore(task, { autoMerge: false, rootDir }); + prepareWorkspaceRevertPrBranchesMock.mockResolvedValue({ + eligible: false, + classification: "conflicting", + conflicts: [{ repo: "repo-b", file: "b.ts", status: "UU" }], + repos: [ + { repo: "repo-a", classification: "clean" }, + { repo: "repo-b", classification: "conflicting", conflicts: [{ file: "b.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: [{ repo: "repo-b", file: "b.ts" }], + }); + expect(createPrMock).not.toHaveBeenCalled(); + }); + + it("conflicting under autoMerge:false, mode:'auto' → falls back to the AI-undo task", async () => { + delete process.env.GITHUB_REPOSITORY; + const task = makeWorkspaceTask({ id: "FN-105", column: "done" }); + const { rootDir } = makeWorkspaceGitRoot(["repo-a", "repo-b"]); + const store = createMockStore(task, { autoMerge: false, rootDir }); + prepareWorkspaceRevertPrBranchesMock.mockResolvedValue({ + eligible: false, + classification: "conflicting", + conflicts: [{ repo: "repo-b", file: "b.ts", status: "UU" }], + repos: [ + { repo: "repo-a", classification: "clean" }, + { repo: "repo-b", classification: "conflicting", conflicts: [{ file: "b.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("empty prep (all already-reverted) → { mode: 'git', clean: true, workspace: { repos: [] } }, no createPr", async () => { + delete process.env.GITHUB_REPOSITORY; + const task = makeWorkspaceTask({ id: "FN-106", column: "done" }); + const { rootDir } = makeWorkspaceGitRoot(["repo-a", "repo-b"]); + const store = createMockStore(task, { autoMerge: false, rootDir }); + prepareWorkspaceRevertPrBranchesMock.mockResolvedValue({ eligible: true, repos: [] }); + + 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, workspace: { repos: [] } }); + expect(createPrMock).not.toHaveBeenCalled(); + expect(findPrForBranchMock).not.toHaveBeenCalled(); + }); + + it("regression — workspace autoMerge:true unchanged: still calls revertWorkspaceTask, prepareWorkspaceRevertPrBranches/createPr not called", async () => { + const task = makeWorkspaceTask({ id: "FN-107", column: "done" }); + const { rootDir } = makeWorkspaceGitRoot(["repo-a", "repo-b"]); + const store = createMockStore(task, { autoMerge: true, rootDir }); + revertWorkspaceTaskMock.mockResolvedValue({ + mode: "git", + clean: true, + workspace: { + repos: [ + { repo: "repo-a", classification: "clean", revertCommitSha: "rev-a" }, + { repo: "repo-b", classification: "clean", revertCommitSha: "rev-b" }, + ], + }, + }); + + 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 }); + expect(revertWorkspaceTaskMock).toHaveBeenCalledTimes(1); + expect(prepareWorkspaceRevertPrBranchesMock).not.toHaveBeenCalled(); + expect(createPrMock).not.toHaveBeenCalled(); + }); + + it("regression — single-repo autoMerge:false unchanged: still takes prepareRevertPrBranch, prepareWorkspaceRevertPrBranches not called", async () => { + process.env.GITHUB_REPOSITORY = "o/r"; + const task = makeTask({ id: "FN-108", column: "done" }); + const store = createMockStore(task, { autoMerge: false }); + vi.spyOn(githubRateLimiter, "canMakeRequest").mockReturnValue(true); + findPrForBranchMock.mockResolvedValue({ number: 21, url: "https://github.com/o/r/pull/21" }); + + 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, prNumber: 21, existingPr: true }); + expect(prepareWorkspaceRevertPrBranchesMock).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/dashboard/src/routes/register-task-workflow-routes.ts b/packages/dashboard/src/routes/register-task-workflow-routes.ts index 5fe91460c9..6884cd5130 100644 --- a/packages/dashboard/src/routes/register-task-workflow-routes.ts +++ b/packages/dashboard/src/routes/register-task-workflow-routes.ts @@ -1,5 +1,5 @@ import { createReadStream } from "node:fs"; -import { resolve, sep } from "node:path"; +import { join, resolve, sep } from "node:path"; import type { TaskStore, Task, @@ -60,8 +60,11 @@ import { TaskRevertError, createAiUndoTask, prepareRevertPrBranch, + prepareWorkspaceRevertPrBranches, type AiUndoTaskResult, type PrepareRevertPrBranchResult, + type PrepareWorkspaceRevertPrBranchesResult, + type WorkspaceRepoRevertPrBranch, } from "@fusion/engine"; import { buildBoardWorkflowsPayload } from "./board-workflows.js"; import { isBackwardMoveBlockedByOpenPr, PR_OPEN_BLOCKS_MOVE_BACK_MESSAGE } from "./register-pull-requests-routes.js"; @@ -1704,10 +1707,15 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork conflicts?, alreadyReverted?, unsupported?, needsHuman?, reason? }` OR, for workspace tasks (FN-7547), `{ mode: "git", clean, workspace: { repos: [{ repo, classification, revertCommitSha?, conflicts?, alreadyReverted? }] }, conflicts?: {repo, file, ...}[] }` OR - `{ mode: "ai", createdTaskId: "FN-YYYY", alreadyOpen?: true }`. The AI-undo task is created via - `createAiUndoTask` (engine) + `TaskStore.findOpenRevertTaskForSource` (core) for the idempotency - guard — a second call while an undo task is still open returns the SAME `createdTaskId` with - `alreadyOpen: true` rather than creating a duplicate. + `{ mode: "ai", createdTaskId: "FN-YYYY", alreadyOpen?: true }` OR, for a single-repo task under + `autoMerge:false` (FN-7554), `{ mode: "pr", clean: true, prUrl, prNumber, revertBranch, existingPr? }` + OR, for a WORKSPACE task under `autoMerge:false` (FN-7577 — additive over FN-7554/FN-7547), + `{ mode: "pr", clean: true, workspace: { repos: [{ repo, revertBranch, prUrl, prNumber, existingPr? }] } }` + — one revert PR opened per sub-repo, all-or-nothing at the branch-prep phase + (`prepareWorkspaceRevertPrBranches`), never force-writing any sub-repo integration branch. The + AI-undo task is created via `createAiUndoTask` (engine) + `TaskStore.findOpenRevertTaskForSource` + (core) for the idempotency guard — a second call while an undo task is still open returns the + SAME `createdTaskId` with `alreadyOpen: true` rather than creating a duplicate. */ router.post("/tasks/:id/revert", async (req, res) => { try { @@ -1800,6 +1808,169 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork UNCHANGED. `granularity` does not apply to the workspace path. */ if (isWorkspaceTask(task)) { + /* + FNXC:TaskRevert 2026-07-05-00:00 (FN-7577 — workspace mode:"pr" dispatch, + additive over FN-7554/FN-7547): `revertWorkspaceTask` refuses + (`needsHuman`) whenever autoMerge is effectively off, the same dead end + `performTaskRevert` hits for single-repo tasks. Instead of stopping + there, take the multi-PR path: `prepareWorkspaceRevertPrBranches` + classifies EVERY sub-repo first and only prepares one dedicated + `fusion/revert-` branch per sub-repo (never writing any sub-repo + integration branch) when every sub-repo is clean/already-reverted; this + route then opens ONE revert PR per prepared sub-repo branch, reusing + FN-7554's per-repo owner/repo resolution, rate-limiter gate, + `findPrForBranch` idempotency, and `manual:true` handoff. The + `autoMerge:true` workspace path below (the existing `revertWorkspaceTask` + call) is UNCHANGED. + */ + const effectiveAutoMerge = task.autoMerge ?? settings.autoMerge ?? true; + + if (effectiveAutoMerge === false) { + const revertBranch = `fusion/revert-${task.id.toLowerCase()}`; + + const prepared: PrepareWorkspaceRevertPrBranchesResult = await prepareWorkspaceRevertPrBranches({ + task, + workspaceRootDir: rootDir, + settings, + revertBranch, + commitAssociationSource: { + getTaskCommitAssociationsByLineageId: (lineageId: string) => + scopedStore.getTaskCommitAssociationsByLineageId(lineageId), + }, + }); + + if (!prepared.eligible) { + if ("classification" in prepared && prepared.classification === "conflicting") { + if (mode === "auto") { + res.json(await createAiUndoResult()); + return; + } + res.json({ mode: "git", clean: false, workspace: { repos: prepared.repos }, 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) { + // prepared.eligible === true + if (prepared.repos.length === 0) { + // Every sub-repo was already-reverted — nothing to PR. + res.json({ mode: "git", clean: true, workspace: { repos: [] } }); + return; + } + + /* + FNXC:TaskRevert 2026-07-05-00:00 (FN-7577 — atomic pre-check ordering): + Resolve owner/repo AND check the rate limiter for EVERY prepared + sub-repo BEFORE pushing/creating any PR, so the two common degrade + cases (GitHub unconfigured / rate-limited) never leave a partial + subset of PRs open across sub-repos. Nothing has been pushed to any + remote yet at this point, so degrading here only needs to delete the + purely-local prepared branches. + */ + const cleanupPreparedBranches = async (): Promise => { + for (const repoBranch of prepared.repos) { + const repoRootDir = join(rootDir, repoBranch.repo); + await runGitCommand(["checkout", repoBranch.integrationBranch], repoRootDir, 10_000).catch(() => undefined); + await runGitCommand(["branch", "-D", revertBranch], repoRootDir, 10_000).catch(() => undefined); + } + }; + + const targets: { repoBranch: WorkspaceRepoRevertPrBranch; owner: string; repo: string }[] = []; + for (const repoBranch of prepared.repos) { + const gitRepo = getCurrentRepo(join(rootDir, repoBranch.repo)); + if (!gitRepo) { + await cleanupPreparedBranches(); + res.json({ + mode: "git", + needsHuman: true, + reason: "autoMerge is disabled and one or more sub-repos have no GitHub repository configured; cannot open revert PRs", + }); + return; + } + targets.push({ repoBranch, owner: gitRepo.owner, repo: gitRepo.repo }); + } + + for (const target of targets) { + const repoKey = `${target.owner}/${target.repo}`; + if (!githubRateLimiter.canMakeRequest(repoKey)) { + await cleanupPreparedBranches(); + res.json({ + mode: "git", + needsHuman: true, + reason: "GitHub API rate limit exceeded; try again later", + }); + return; + } + } + + /* + FNXC:TaskRevert 2026-07-05-00:00 (FN-7577 — idempotent multi-PR + recovery contract): from this point on, a thrown error (network down, + push rejected, GitHub 5xx, etc.) is surfaced via the shared `catch` + below rather than a graceful needsHuman degrade, because an earlier + sub-repo in this loop may already have an open remote PR by the time a + later sub-repo fails — this route NEVER attempts to close/delete an + already-created remote PR. A re-run of this endpoint is safe: + `findPrForBranch` links any already-created sub-repo PR instead of + re-creating it, and `prepareWorkspaceRevertPrBranches`'s `checkout -B` + re-preps local branches for any sub-repo not yet pushed. + */ + const resultRepos: { repo: string; revertBranch: string; prUrl: string; prNumber: number; existingPr?: boolean }[] = []; + const persistPrInfo = async (prInfo: PrInfo): Promise => { + 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); + } + }; + + for (const target of targets) { + const client = new GitHubClient(); + const existingPr = await client.findPrForBranch({ head: revertBranch, state: "all", owner: target.owner, repo: target.repo }); + + if (existingPr) { + // Idempotency — never re-push/re-create when an open (or all-state) + // PR already exists for this sub-repo's branch, just link it. + const prInfo: PrInfo = { ...existingPr, manual: true }; + await persistPrInfo(prInfo); + await scopedStore.logEntry(task.id, "Linked existing revert PR", `${target.repoBranch.repo}: PR #${prInfo.number}: ${prInfo.url}`); + resultRepos.push({ repo: target.repoBranch.repo, revertBranch, prUrl: prInfo.url, prNumber: prInfo.number, existingPr: true }); + continue; + } + + await runGitCommand(["push", "-u", "origin", revertBranch], join(rootDir, target.repoBranch.repo), 60_000); + const prTitle = `revert(${task.id}): undo landed work (${target.repoBranch.repo})`; + const prBody = + `This PR reverts the work landed by task ${task.id} in sub-repo \`${target.repoBranch.repo}\`.\n\n` + + `See \`GET /api/tasks/${task.id}/diff\` for the full landed diff being reverted.\n`; + const created = await client.createPr({ + owner: target.owner, + repo: target.repo, + title: prTitle, + body: prBody, + head: revertBranch, + base: target.repoBranch.integrationBranch, + }); + const prInfo: PrInfo = { ...created, manual: true }; + await persistPrInfo(prInfo); + await scopedStore.logEntry(task.id, "Created revert PR", `${target.repoBranch.repo}: PR #${prInfo.number}: ${prInfo.url}`); + resultRepos.push({ repo: target.repoBranch.repo, revertBranch, prUrl: prInfo.url, prNumber: prInfo.number }); + } + + res.json({ mode: "pr", clean: true, workspace: { repos: resultRepos } }); + return; + } + } + const workspaceResult = await revertWorkspaceTask({ task, workspaceRootDir: rootDir, diff --git a/packages/engine/src/__tests__/task-revert-workspace-pr.real-git.test.ts b/packages/engine/src/__tests__/task-revert-workspace-pr.real-git.test.ts new file mode 100644 index 0000000000..e2fb93571e --- /dev/null +++ b/packages/engine/src/__tests__/task-revert-workspace-pr.real-git.test.ts @@ -0,0 +1,371 @@ +import { exec, execSync, spawnSync } from "node:child_process"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { promisify } from "node:util"; +import { afterEach, describe, expect, it } from "vitest"; +import { prepareWorkspaceRevertPrBranches } from "../task-revert.js"; +import type { Task } from "@fusion/core"; + +const realExecAsync = promisify(exec); + +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 { + return { + id: "FN-A", + lineageId: "FN-A", + description: "", + column: "done", + dependencies: [], + steps: [], + currentStep: 0, + ...overrides, + } as Task; +} + +/* +FNXC:TaskRevert 2026-07-05-00:00 (FN-7577): +Real multi-sub-repo git fixture coverage for `prepareWorkspaceRevertPrBranches` +— the Symptom Verification regression suite for the workspace `mode:"pr"` +branch-prep primitive. Mirrors the two-sub-repo fixture pattern from +`task-revert.workspace.real-git.test.ts` (FN-7547) combined with the +single-repo branch-prep assertions from `task-revert-pr.real-git.test.ts` +(FN-7554): clean → per-sub-repo `fusion/revert-` branches with +integration branches left byte-identical; one conflicting sub-repo aborts the +WHOLE preparation with no branch created anywhere; already-reverted → +eligible with empty repos; mixed clean/already-reverted → only the +still-clean sub-repo gets a branch; non-workspace task → unsupported; +idempotent local branch reset; dirty-tree/branch-mismatch refusal; and a +late-conflict multi-branch cleanup. +*/ +describeIfGit("prepareWorkspaceRevertPrBranches real-git scenarios", { timeout: 30_000 }, () => { + const dirs: string[] = []; + afterEach(() => { + for (const dir of dirs.splice(0)) rmSync(dir, { recursive: true, force: true }); + }); + + function subRepoFixture(workspaceRoot: string, repoRel: string, initialFile: string, initialContent: string): string { + const repoRootDir = join(workspaceRoot, repoRel); + git(workspaceRoot, `mkdir -p ${repoRel}`); + git(repoRootDir, "git init -b main"); + git(repoRootDir, 'git config user.email "test@example.com"'); + git(repoRootDir, 'git config user.name "Test User"'); + git(repoRootDir, "git config commit.gpgsign false"); + writeFileSync(join(repoRootDir, initialFile), initialContent); + git(repoRootDir, `git add ${initialFile} && git commit -m 'init'`); + return repoRootDir; + } + + function workspaceFixture() { + const workspaceRoot = mkdtempSync(join(tmpdir(), "kb-revert-ws-pr-")); + dirs.push(workspaceRoot); + const repoA = subRepoFixture(workspaceRoot, "repo-a", "a.ts", "line1\n"); + const repoB = subRepoFixture(workspaceRoot, "repo-b", "b.ts", "line1\n"); + return { workspaceRoot, repoA, repoB }; + } + + function landTaskCommit(repoRootDir: string, file: string, content: string, commitSubject: string): string { + writeFileSync(join(repoRootDir, file), content); + git(repoRootDir, `git commit -am ${JSON.stringify(commitSubject)}`); + return git(repoRootDir, "git rev-parse HEAD"); + } + + function makeWorkspaceTask(shaA: string, shaB: string, overrides: Partial = {}): Task { + return makeTask({ + column: "done", + workspaceWorktrees: { + "repo-a": { worktreePath: "repo-a", branch: "fusion/FN-A", landedSha: shaA }, + "repo-b": { worktreePath: "repo-b", branch: "fusion/FN-A", landedSha: shaB }, + }, + mergeDetails: { commitSha: shaA, workspaceLandedShas: { "repo-a": shaA, "repo-b": shaB } }, + ...overrides, + }); + } + + it("all clean → eligible, per-sub-repo branches, integration branches unwritten", async () => { + const { workspaceRoot, repoA, repoB } = workspaceFixture(); + const shaA = landTaskCommit(repoA, "a.ts", "line1\nfeature-a\n", "feat(FN-A): add feature in repo-a"); + const shaB = landTaskCommit(repoB, "b.ts", "line1\nfeature-a\n", "feat(FN-A): add feature in repo-b"); + + const mainHeadBeforeA = git(repoA, "git rev-parse main"); + const mainHeadBeforeB = git(repoB, "git rev-parse main"); + + const task = makeWorkspaceTask(shaA, shaB); + const result = await prepareWorkspaceRevertPrBranches({ + task, + workspaceRootDir: workspaceRoot, + settings: {}, + revertBranch: "fusion/revert-fn-a", + }); + + expect(result).toMatchObject({ eligible: true }); + if (result.eligible) { + expect(result.repos).toHaveLength(2); + const byRepo = Object.fromEntries(result.repos.map((r) => [r.repo, r])); + expect(byRepo["repo-a"]).toMatchObject({ revertBranch: "fusion/revert-fn-a", integrationBranch: "main" }); + expect(byRepo["repo-b"]).toMatchObject({ revertBranch: "fusion/revert-fn-a", integrationBranch: "main" }); + expect(byRepo["repo-a"].revertCommitShas).toHaveLength(1); + expect(byRepo["repo-b"].revertCommitShas).toHaveLength(1); + } + + for (const repoRootDir of [repoA, repoB]) { + const branchTipSubject = git(repoRootDir, "git log -1 --format=%s fusion/revert-fn-a"); + expect(branchTipSubject).toMatch(/^revert\(FN-A\):/); + const branchTipBody = git(repoRootDir, "git log -1 --format=%B fusion/revert-fn-a"); + expect(branchTipBody).toContain("Fusion-Task-Id: FN-A"); + // checkout restored to main, clean. + expect(git(repoRootDir, "git rev-parse --abbrev-ref HEAD")).toBe("main"); + expect(git(repoRootDir, "git status --porcelain")).toBe(""); + } + + // (b) each sub-repo's main HEAD is byte-identical to before — integration + // branch never written. + expect(git(repoA, "git rev-parse main")).toBe(mainHeadBeforeA); + expect(git(repoB, "git rev-parse main")).toBe(mainHeadBeforeB); + expect(git(repoA, "git show fusion/revert-fn-a:a.ts")).toBe("line1"); + expect(git(repoB, "git show fusion/revert-fn-a:b.ts")).toBe("line1"); + }); + + it("one sub-repo conflicting → whole-task aborted, NO branches anywhere (Symptom Verification)", async () => { + const { workspaceRoot, repoA, repoB } = workspaceFixture(); + const shaA = landTaskCommit(repoA, "a.ts", "line1\nfeature-a\n", "feat(FN-A): add feature in repo-a"); + const shaB = landTaskCommit(repoB, "b.ts", "line1\nfeature-a\n", "feat(FN-A): add feature in repo-b"); + + // Task B later modifies the exact same region touched by task A in repo-b only. + landTaskCommit(repoB, "b.ts", "line1\nfeature-a-modified-by-b\n", "feat(FN-B): modify same region in repo-b"); + + const mainHeadBeforeA = git(repoA, "git rev-parse main"); + const mainHeadBeforeB = git(repoB, "git rev-parse main"); + + const task = makeWorkspaceTask(shaA, shaB); + const result = await prepareWorkspaceRevertPrBranches({ + task, + workspaceRootDir: workspaceRoot, + settings: {}, + revertBranch: "fusion/revert-fn-a", + }); + + expect(result).toMatchObject({ eligible: false, classification: "conflicting" }); + if (!result.eligible && result.classification === "conflicting") { + expect(result.conflicts.some((c) => c.repo === "repo-b")).toBe(true); + } + + for (const repoRootDir of [repoA, repoB]) { + expect(git(repoRootDir, "git branch --list fusion/revert-fn-a")).toBe(""); + } + expect(git(repoA, "git rev-parse main")).toBe(mainHeadBeforeA); + expect(git(repoB, "git rev-parse main")).toBe(mainHeadBeforeB); + expect(git(repoA, "git rev-parse --abbrev-ref HEAD")).toBe("main"); + expect(git(repoB, "git rev-parse --abbrev-ref HEAD")).toBe("main"); + }); + + it("all already-reverted → eligible with empty repos, no branches, integration branches unchanged", async () => { + const { workspaceRoot, repoA, repoB } = workspaceFixture(); + const shaA = landTaskCommit(repoA, "a.ts", "line1\nfeature-a\n", "feat(FN-A): add feature in repo-a"); + const shaB = landTaskCommit(repoB, "b.ts", "line1\nfeature-a\n", "feat(FN-A): add feature in repo-b"); + + // Manually revert both sub-repos on main before calling the branch-prep primitive. + git(repoA, `git revert --no-edit ${shaA}`); + git(repoB, `git revert --no-edit ${shaB}`); + const mainHeadBeforeA = git(repoA, "git rev-parse main"); + const mainHeadBeforeB = git(repoB, "git rev-parse main"); + + const task = makeWorkspaceTask(shaA, shaB); + const result = await prepareWorkspaceRevertPrBranches({ + task, + workspaceRootDir: workspaceRoot, + settings: {}, + revertBranch: "fusion/revert-fn-a", + }); + + expect(result).toMatchObject({ eligible: true, repos: [] }); + for (const repoRootDir of [repoA, repoB]) { + expect(git(repoRootDir, "git branch --list fusion/revert-fn-a")).toBe(""); + } + expect(git(repoA, "git rev-parse main")).toBe(mainHeadBeforeA); + expect(git(repoB, "git rev-parse main")).toBe(mainHeadBeforeB); + }); + + it("mixed clean + already-reverted → only the still-clean sub-repo gets a branch", async () => { + const { workspaceRoot, repoA, repoB } = workspaceFixture(); + const shaA = landTaskCommit(repoA, "a.ts", "line1\nfeature-a\n", "feat(FN-A): add feature in repo-a"); + const shaB = landTaskCommit(repoB, "b.ts", "line1\nfeature-a\n", "feat(FN-A): add feature in repo-b"); + + // Manually revert repo-b only. + git(repoB, `git revert --no-edit ${shaB}`); + const mainHeadBeforeA = git(repoA, "git rev-parse main"); + const mainHeadBeforeB = git(repoB, "git rev-parse main"); + + const task = makeWorkspaceTask(shaA, shaB); + const result = await prepareWorkspaceRevertPrBranches({ + task, + workspaceRootDir: workspaceRoot, + settings: {}, + revertBranch: "fusion/revert-fn-a", + }); + + expect(result).toMatchObject({ eligible: true }); + if (result.eligible) { + expect(result.repos).toHaveLength(1); + expect(result.repos[0].repo).toBe("repo-a"); + } + + const branchTipSubject = git(repoA, "git log -1 --format=%s fusion/revert-fn-a"); + expect(branchTipSubject).toMatch(/^revert\(FN-A\):/); + expect(git(repoB, "git branch --list fusion/revert-fn-a")).toBe(""); + expect(git(repoA, "git rev-parse main")).toBe(mainHeadBeforeA); + expect(git(repoB, "git rev-parse main")).toBe(mainHeadBeforeB); + }); + + it("non-workspace task → unsupported", async () => { + const { workspaceRoot } = workspaceFixture(); + const task = makeTask({ column: "done" }); + + const result = await prepareWorkspaceRevertPrBranches({ + task, + workspaceRootDir: workspaceRoot, + settings: {}, + revertBranch: "fusion/revert-fn-a", + }); + + expect(result).toMatchObject({ eligible: false, unsupported: true, reason: "not-a-workspace-task" }); + }); + + it("idempotent local branch reset: a stale local branch in one sub-repo is reset off integration with the fresh revert commit", async () => { + const { workspaceRoot, repoA, repoB } = workspaceFixture(); + const shaA = landTaskCommit(repoA, "a.ts", "line1\nfeature-a\n", "feat(FN-A): add feature in repo-a"); + const shaB = landTaskCommit(repoB, "b.ts", "line1\nfeature-a\n", "feat(FN-A): add feature in repo-b"); + + // Pre-create a stale local revert branch in repo-a pointing at an unrelated commit. + git(repoA, "git branch fusion/revert-fn-a main~1"); + const staleTip = git(repoA, "git rev-parse fusion/revert-fn-a"); + expect(staleTip).not.toBe(git(repoA, "git rev-parse main")); + + const task = makeWorkspaceTask(shaA, shaB); + const result = await prepareWorkspaceRevertPrBranches({ + task, + workspaceRootDir: workspaceRoot, + settings: {}, + revertBranch: "fusion/revert-fn-a", + }); + + expect(result).toMatchObject({ eligible: true }); + const branchTipSubject = git(repoA, "git log -1 --format=%s fusion/revert-fn-a"); + expect(branchTipSubject).toMatch(/^revert\(FN-A\):/); + expect(git(repoA, "git rev-parse --abbrev-ref HEAD")).toBe("main"); + }); + + it("dirty-tree refusal: refuses without mutating any sub-repo when one has a stray uncommitted change", async () => { + const { workspaceRoot, repoA, repoB } = workspaceFixture(); + const shaA = landTaskCommit(repoA, "a.ts", "line1\nfeature-a\n", "feat(FN-A): add feature in repo-a"); + const shaB = landTaskCommit(repoB, "b.ts", "line1\nfeature-a\n", "feat(FN-A): add feature in repo-b"); + + writeFileSync(join(repoB, "b.ts"), "line1\nfeature-a\nSTRAY UNCOMMITTED CHANGE\n"); + + const mainHeadBeforeA = git(repoA, "git rev-parse main"); + const mainHeadBeforeB = git(repoB, "git rev-parse main"); + + const task = makeWorkspaceTask(shaA, shaB); + await expect( + prepareWorkspaceRevertPrBranches({ + task, + workspaceRootDir: workspaceRoot, + settings: {}, + revertBranch: "fusion/revert-fn-a", + }), + ).rejects.toMatchObject({ code: "dirty-working-tree" }); + + for (const repoRootDir of [repoA, repoB]) { + expect(git(repoRootDir, "git branch --list fusion/revert-fn-a")).toBe(""); + } + expect(git(repoA, "git rev-parse main")).toBe(mainHeadBeforeA); + expect(git(repoB, "git rev-parse main")).toBe(mainHeadBeforeB); + }); + + it("branch-mismatch refusal: refuses without mutating any sub-repo when one is checked out on a different branch", async () => { + const { workspaceRoot, repoA, repoB } = workspaceFixture(); + const shaA = landTaskCommit(repoA, "a.ts", "line1\nfeature-a\n", "feat(FN-A): add feature in repo-a"); + const shaB = landTaskCommit(repoB, "b.ts", "line1\nfeature-a\n", "feat(FN-A): add feature in repo-b"); + + git(repoB, "git checkout -b some-other-branch"); + + const mainHeadBeforeA = git(repoA, "git rev-parse main"); + const mainHeadBeforeB = git(repoB, "git rev-parse main"); + + const task = makeWorkspaceTask(shaA, shaB); + await expect( + prepareWorkspaceRevertPrBranches({ + task, + workspaceRootDir: workspaceRoot, + settings: {}, + revertBranch: "fusion/revert-fn-a", + }), + ).rejects.toMatchObject({ code: "branch-mismatch" }); + + for (const repoRootDir of [repoA, repoB]) { + expect(git(repoRootDir, "git branch --list fusion/revert-fn-a")).toBe(""); + } + expect(git(repoA, "git rev-parse main")).toBe(mainHeadBeforeA); + expect(git(repoB, "git rev-parse main")).toBe(mainHeadBeforeB); + }); + + it("late-conflict multi-branch cleanup: repo-a's prepped branch is deleted when repo-b conflicts during apply (branch moved between classify and apply)", async () => { + const { workspaceRoot, repoA, repoB } = workspaceFixture(); + const shaA = landTaskCommit(repoA, "a.ts", "line1\nfeature-a\n", "feat(FN-A): add feature in repo-a"); + const shaB = landTaskCommit(repoB, "b.ts", "line1\nfeature-a\n", "feat(FN-A): add feature in repo-b"); + + const mainHeadBeforeA = git(repoA, "git rev-parse main"); + const mainHeadBeforeB = git(repoB, "git rev-parse main"); + + const task = makeWorkspaceTask(shaA, shaB); + + /* + FNXC:TaskRevert 2026-07-05-00:00 (FN-7577 test): both sub-repos classify + CLEAN in Phase 1 (repo-b's main is still untouched at that point). Once + Phase 2 starts checking out repo-a's branch (repo-a sorts first), inject a + conflicting commit directly onto repo-b's `main` — simulating repo-b's + branch moving between classify and apply. When Phase 2 reaches repo-b, + `checkout -B fusion/revert-fn-a main` branches off the NEW (conflicting) + tip, so applying repo-b's revert commit now conflicts — a genuine late + conflict. Assert repo-a's already-prepped branch is rolled back too. + */ + let injected = false; + const execAsyncImpl: typeof realExecAsync = (async (command: string, options: Record) => { + if (!injected && options?.cwd === repoA && /git checkout -B/.test(command)) { + injected = true; + writeFileSync(join(repoB, "b.ts"), "line1\nfeature-a-modified-by-b\n"); + execSync("git commit -am 'feat(FN-B): modify same region in repo-b'", { cwd: repoB, stdio: "pipe" }); + } + return realExecAsync(command, options as never); + }) as typeof realExecAsync; + + const result = await prepareWorkspaceRevertPrBranches({ + task, + workspaceRootDir: workspaceRoot, + settings: {}, + revertBranch: "fusion/revert-fn-a", + execAsyncImpl, + }); + + expect(result).toMatchObject({ eligible: false, classification: "conflicting" }); + if (!result.eligible && result.classification === "conflicting") { + expect(result.conflicts.some((c) => c.repo === "repo-b")).toBe(true); + } + // repo-a's already-prepped branch from this pass is rolled back too — all-or-nothing. + expect(git(repoA, "git branch --list fusion/revert-fn-a")).toBe(""); + expect(git(repoB, "git branch --list fusion/revert-fn-a")).toBe(""); + expect(git(repoA, "git rev-parse main")).toBe(mainHeadBeforeA); + // repo-b's main legitimately advanced due to the injected commit (this test + // simulates an external actor landing work mid-preparation) — the + // invariant is that NO revert branch/commit was created anywhere, not that + // repo-b's HEAD is frozen (that HEAD moved before this function ever ran + // Phase 2 for repo-b). + expect(git(repoB, "git rev-parse main")).not.toBe(mainHeadBeforeB); + }); +}); diff --git a/packages/engine/src/index.ts b/packages/engine/src/index.ts index 90721e2a4e..cdd22c1c47 100644 --- a/packages/engine/src/index.ts +++ b/packages/engine/src/index.ts @@ -295,6 +295,10 @@ export { prepareRevertPrBranch, type PrepareRevertPrBranchResult, type PrepareRevertPrBranchOptions, + prepareWorkspaceRevertPrBranches, + type PrepareWorkspaceRevertPrBranchesResult, + type PrepareWorkspaceRevertPrBranchesOptions, + type WorkspaceRepoRevertPrBranch, } from "./task-revert.js"; export { resolveBranchGroupMergeRouting, diff --git a/packages/engine/src/task-revert.ts b/packages/engine/src/task-revert.ts index 47299fdb88..7d4bea5745 100644 --- a/packages/engine/src/task-revert.ts +++ b/packages/engine/src/task-revert.ts @@ -35,6 +35,17 @@ * captures `preRevertHead` before touching the tree and guarantees a full * `git revert --abort` + `git reset --hard ` rollback in a * `finally` block, regardless of how the dry-run terminates. + * + * FNXC:TaskRevert 2026-07-05-00:00 (FN-7577 — PR-based revert extended to + * workspace tasks): `prepareRevertPrBranch` (FN-7554) explicitly refuses + * workspace tasks (`workspace-task-pr-revert-unsupported`) because a single + * PR against a single base branch cannot represent a multi-repo revert. + * `prepareWorkspaceRevertPrBranches` fills that gap: it mirrors + * `revertWorkspaceTask`'s classify-all-then-commit-all skeleton but, instead + * of force-committing onto each sub-repo's integration branch, prepares one + * dedicated `` per sub-repo (never writing any integration + * branch) so the caller can open one PR per sub-repo. See its own doc + * comment below for the full contract. */ import { exec } from "node:child_process"; import { join } from "node:path"; @@ -1225,6 +1236,290 @@ export async function revertWorkspaceTask(opts: RevertWorkspaceTaskOptions): Pro return { mode: "git", clean: true, workspace: { repos } }; } +// --------------------------------------------------------------------------- +// FN-7577: PR-based revert for WORKSPACE (multi-repo) tasks under autoMerge:false. +// --------------------------------------------------------------------------- + +export interface WorkspaceRepoRevertPrBranch { + repo: string; + /** Same branch NAME across every sub-repo (`fusion/revert-`). */ + revertBranch: string; + /** This sub-repo's resolved integration branch — the PR base. NEVER written to. */ + integrationBranch: string; + revertCommitShas: string[]; +} + +export type PrepareWorkspaceRevertPrBranchesResult = + | { eligible: true; repos: WorkspaceRepoRevertPrBranch[] } + | { + eligible: false; + classification: "conflicting"; + conflicts: (TaskRevertConflict & { repo: string })[]; + repos: WorkspaceRepoRevertResult[]; + } + | { eligible: false; unsupported: true; reason: string }; + +export interface PrepareWorkspaceRevertPrBranchesOptions { + task: Pick; + /** Project root dir; each sub-repo lives at `join(workspaceRootDir, repoRel)` (mirrors `revertWorkspaceTask`). */ + workspaceRootDir: string; + /** Project settings, passed through to `resolveIntegrationBranch` per sub-repo with `integrationBranch`/`baseBranch` stripped (KTD1). */ + settings: IntegrationBranchSettings; + /** e.g. `fusion/revert-` — the SAME branch name prepared in every sub-repo. */ + revertBranch: string; + execAsyncImpl?: ExecAsyncImpl; + commitAssociationSource?: TaskCommitAssociationSource; +} + +interface WorkspaceRepoRevertPrContext { + repo: string; + repoRootDir: string; + integrationBranch: string; + commits: string[]; + classification: ClassifyTaskRevertResult; +} + +type PrepareOneWorkspaceRepoBranchOutcome = + | { kind: "applied"; revertCommitSha: string } + | { kind: "already-reverted" } + | { kind: "conflicts"; conflicts: TaskRevertConflict[] }; + +/** + * FNXC:TaskRevert 2026-07-05-00:00 (FN-7577 — per-sub-repo branch prep + * primitive, mirrors FN-7554's single-repo `prepareRevertPrBranch` body): + * Prepares ONE sub-repo's dedicated `revertBranch` off `integrationBranch` + * HEAD (`-B`, idempotent re-run), applies+commits via the shared + * `applyAndCommitRevert` (REUSE, not reimplementation — same helper + * `performTaskRevert`/`revertWorkspaceTask`/`prepareRevertPrBranch` all use), + * and ALWAYS restores the sub-repo checkout back to `integrationBranch` + * before returning — `integrationBranch`'s ref itself is never advanced, + * reset, or committed to. A redundant/failed local `revertBranch` (nothing + * to commit, or a late apply-time conflict) is deleted so this sub-repo is + * left byte-identical to its pre-call state whenever no PR branch results. + */ +async function prepareOneWorkspaceRepoRevertBranch(opts: { + execImpl: ExecAsyncImpl; + repoRootDir: string; + integrationBranch: string; + revertBranch: string; + commits: string[]; + taskId: string; +}): Promise { + const { execImpl, repoRootDir, integrationBranch, revertBranch, commits, taskId } = opts; + 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 — reset off + // `integrationBranch` HEAD rather than accumulating on top of whatever it + // previously pointed at. `integrationBranch` is only ever READ here. + await runGit(execImpl, `git checkout -B ${quoteShellArg(revertBranch)} ${quoteShellArg(integrationBranch)}`, repoRootDir); + branchCreated = true; + + const applied = await applyAndCommitRevert({ worktreePath: repoRootDir, commits, taskId, execAsyncImpl: execImpl }); + + if ("alreadyReverted" in applied) { + // Defensive: the branch moved between classify and apply — nothing to + // commit on the fresh revertBranch. Delete the now-redundant branch so + // this sub-repo contributes no branch to the caller. + await runGit(execImpl, `git checkout ${quoteShellArg(integrationBranch)}`, repoRootDir).catch(() => undefined); + await runGit(execImpl, `git branch -D ${quoteShellArg(revertBranch)}`, repoRootDir).catch(() => undefined); + return { kind: "already-reverted" }; + } + if ("conflicts" in applied) { + // Late conflict — applyAndCommitRevert already rolled repoRootDir back to + // the tip of revertBranch (== integrationBranch HEAD). Delete the + // now-redundant branch; the caller handles multi-repo rollback. + await runGit(execImpl, `git checkout ${quoteShellArg(integrationBranch)}`, repoRootDir).catch(() => undefined); + await runGit(execImpl, `git branch -D ${quoteShellArg(revertBranch)}`, repoRootDir).catch(() => undefined); + return { kind: "conflicts", conflicts: applied.conflicts }; + } + + // applied.applied === true — leave `revertBranch` in place (it IS the PR + // branch) but restore the sub-repo checkout back to `integrationBranch`, + // never leaving it mid-revert on `revertBranch`. + await runGit(execImpl, `git checkout ${quoteShellArg(integrationBranch)}`, repoRootDir).catch(() => undefined); + return { kind: "applied", revertCommitSha: 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(integrationBranch)}`, repoRootDir).catch(() => undefined); + await runGit(execImpl, `git branch -D ${quoteShellArg(revertBranch)}`, repoRootDir).catch(() => undefined); + } + throw error instanceof TaskRevertError + ? error + : new TaskRevertError("failed to prepare revert branch for sub-repo", "revert-pr-branch-prepare-failed", error); + } +} + +/** + * FNXC:TaskRevert 2026-07-05-00:00 (FN-7577 — workspace PR-revert branch prep, + * the all-or-nothing gate this task adds on top of FN-7554/FN-7547): + * + * NEVER-WRITE-TO-INTEGRATION-BRANCH INVARIANT: mirrors `prepareRevertPrBranch` + * (single-repo) extended to N sub-repos — no sub-repo's integration branch + * ref is EVER advanced, reset, or committed to. Every revert commit lives + * ONLY on that sub-repo's `` (same branch NAME across every + * sub-repo, distinct branch OBJECT per sub-repo git history). + * + * CLASSIFY-ALL-THEN-PREP-ALL (Phase 1 / Phase 2), mirroring + * `revertWorkspaceTask`'s whole-task all-or-nothing contract: Phase 1 + * dry-run classifies EVERY sub-repo first (reusing the shared + * `classifyTaskRevert`, itself always rolling each tree back + * byte-identical). If ANY sub-repo classifies `conflicting`, this function + * returns immediately with NO branch created anywhere — Phase 2 (branch + * prep) never runs for ANY sub-repo. Only when every sub-repo classifies + * clean/already-reverted does Phase 2 run, preparing one dedicated + * `` per sub-repo that actually has commits to revert + * (`prepareOneWorkspaceRepoRevertBranch`, reusing `applyAndCommitRevert` — no + * commit-message/trailer duplication). A LATE conflict during Phase 2 (a + * sub-repo's branch moved between classify and apply) rolls back every + * PREVIOUSLY prepped sub-repo's branch in this pass (checkout back + + * `git branch -D`) before returning conflicting, so the whole preparation + * stays all-or-nothing even when the failure surfaces mid-pass rather than + * during classification. + * + * THIS FUNCTION DOES NOT GATE ON `autoMerge` — the caller (the API route) + * decides when to invoke this primitive; it stays a pure branch-prep + * building block usable independent of that policy decision. + */ +export async function prepareWorkspaceRevertPrBranches( + opts: PrepareWorkspaceRevertPrBranchesOptions, +): Promise { + const { task, workspaceRootDir, revertBranch } = opts; + const execImpl = opts.execAsyncImpl ?? defaultExecAsync; + + if (!isWorkspaceTask(task) || Object.keys(task.workspaceWorktrees ?? {}).length === 0) { + return { eligible: false, unsupported: true, reason: "not-a-workspace-task" }; + } + + const workspaceWorktrees = task.workspaceWorktrees ?? {}; + const repoKeys = Object.keys(workspaceWorktrees).sort(); + + const attribution = await resolveWorkspaceTaskRevertCommits(task, { + workspaceRootDir, + execAsyncImpl: execImpl, + commitAssociationSource: opts.commitAssociationSource, + }); + + // Phase 1: resolve each sub-repo's integration branch, refuse (without + // mutating) on branch-mismatch/dirty-tree, then dry-run classify EVERY + // sub-repo — mirrors `revertWorkspaceTask`'s Phase 1 verbatim. + const contexts: WorkspaceRepoRevertPrContext[] = []; + for (const repoRel of repoKeys) { + const repoRootDir = join(workspaceRootDir, repoRel); + + let integrationBranch: string; + try { + integrationBranch = await resolveIntegrationBranch(repoRootDir, { ...opts.settings, integrationBranch: undefined, baseBranch: undefined }); + } catch (error) { + throw new TaskRevertError(`failed to resolve integration branch for sub-repo ${repoRel}`, "integration-branch-resolve-failed", error); + } + + const currentBranch = (await runGit(execImpl, "git rev-parse --abbrev-ref HEAD", repoRootDir)).stdout.trim(); + if (currentBranch !== integrationBranch) { + throw new TaskRevertError( + `sub-repo ${repoRel} checkout is on "${currentBranch}", not its integration branch "${integrationBranch}"; switch to "${integrationBranch}" before preparing a revert PR branch`, + "branch-mismatch", + ); + } + + const { stdout: statusOut } = await runGit(execImpl, "git status --porcelain", repoRootDir); + if (statusOut.trim().length > 0) { + throw new TaskRevertError( + `working tree for sub-repo ${repoRel} is dirty; refusing to prepare a revert PR branch`, + "dirty-working-tree", + ); + } + + const commits = attribution[repoRel]?.commits ?? []; + const classification = await classifyTaskRevert({ worktreePath: repoRootDir, commits, execAsyncImpl: execImpl }); + + contexts.push({ repo: repoRel, repoRootDir, integrationBranch, commits, classification }); + } + + const anyConflicting = contexts.some((ctx) => ctx.classification.classification === "conflicting"); + if (anyConflicting) { + const repos: WorkspaceRepoRevertResult[] = contexts.map((ctx) => ({ + repo: ctx.repo, + classification: ctx.classification.classification, + conflicts: ctx.classification.conflicts, + alreadyReverted: ctx.classification.alreadyReverted, + })); + const conflicts = contexts.flatMap((ctx) => + (ctx.classification.conflicts ?? []).map((conflict) => ({ ...conflict, repo: ctx.repo })), + ); + return { eligible: false, classification: "conflicting", conflicts, repos }; + } + + // Phase 2: every sub-repo classified clean/already-reverted — prepare a + // dedicated revert branch per sub-repo that actually has commits to revert. + const preppedRepos: WorkspaceRepoRevertPrBranch[] = []; + const preppedForRollback: { repo: string; repoRootDir: string; integrationBranch: string }[] = []; + + try { + for (const ctx of contexts) { + if (ctx.classification.classification === "already-reverted" || ctx.commits.length === 0) { + // Nothing to revert in this sub-repo — contributes no branch. + continue; + } + + const outcome = await prepareOneWorkspaceRepoRevertBranch({ + execImpl, + repoRootDir: ctx.repoRootDir, + integrationBranch: ctx.integrationBranch, + revertBranch, + commits: ctx.commits, + taskId: task.id, + }); + + if (outcome.kind === "already-reverted") { + continue; + } + + if (outcome.kind === "conflicts") { + // Late conflict — roll back every PREVIOUSLY prepped sub-repo's branch + // in this pass so the whole preparation stays all-or-nothing. + for (const prepped of preppedForRollback) { + await runGit(execImpl, `git checkout ${quoteShellArg(prepped.integrationBranch)}`, prepped.repoRootDir).catch(() => undefined); + await runGit(execImpl, `git branch -D ${quoteShellArg(revertBranch)}`, prepped.repoRootDir).catch(() => undefined); + } + const conflicts = outcome.conflicts.map((conflict) => ({ ...conflict, repo: ctx.repo })); + const repos: WorkspaceRepoRevertResult[] = contexts.map((c) => ({ + repo: c.repo, + classification: c.repo === ctx.repo ? "conflicting" : c.classification.classification, + conflicts: c.repo === ctx.repo ? outcome.conflicts : c.classification.conflicts, + alreadyReverted: c.classification.alreadyReverted, + })); + return { eligible: false, classification: "conflicting", conflicts, repos }; + } + + // outcome.kind === "applied" + preppedRepos.push({ + repo: ctx.repo, + revertBranch, + integrationBranch: ctx.integrationBranch, + revertCommitShas: [outcome.revertCommitSha], + }); + preppedForRollback.push({ repo: ctx.repo, repoRootDir: ctx.repoRootDir, integrationBranch: ctx.integrationBranch }); + } + } catch (error) { + // Unexpected thrown failure mid-pass — best-effort restore + delete every + // already-prepped sub-repo branch so a failed prep never leaves dangling + // half-built revert branches. + for (const prepped of preppedForRollback) { + await runGit(execImpl, `git checkout ${quoteShellArg(prepped.integrationBranch)}`, prepped.repoRootDir).catch(() => undefined); + await runGit(execImpl, `git branch -D ${quoteShellArg(revertBranch)}`, prepped.repoRootDir).catch(() => undefined); + } + throw error instanceof TaskRevertError + ? error + : new TaskRevertError("failed to prepare workspace revert PR branches", "workspace-revert-pr-branch-prepare-failed", error); + } + + return { eligible: true, repos: preppedRepos }; +} + // ──────────────────────────────────────────────────────────────────────── // FN-7524: AI-undo fallback // ────────────────────────────────────────────────────────────────────────