feat(pr): dashboard PR view, routes, node-state UI + user controls (U7)
Adds /api/pull-requests routes (list, detail, merge/approve/retry/close/ automerge — all re-fetch authoritative state before acting), a PullRequestView rendering every entity state distinctly (creating/failed/ unverified/responding/await-review/conflict) with the action bar, live auto-merge gate reason, and conflict CTA; TaskCard PR node-state badge + link; and the R16 column-move-backward guard. User actions route through the existing releaseHeldTaskByEvent primitives. Maps the new PR node kinds in the workflow editor's kind resolver. 13 route tests + lazy-view guard green; component test runs in CI. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
256
packages/dashboard/src/__tests__/routes-pull-requests.test.ts
Normal file
256
packages/dashboard/src/__tests__/routes-pull-requests.test.ts
Normal file
@@ -0,0 +1,256 @@
|
||||
// @vitest-environment node
|
||||
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import express from "express";
|
||||
import type { PrEntity, PrThreadState, Task, TaskStore } from "@fusion/core";
|
||||
import {
|
||||
createPullRequestsRouter,
|
||||
isBackwardMoveBlockedByOpenPr,
|
||||
PR_OPEN_BLOCKS_MOVE_BACK_MESSAGE,
|
||||
} from "../routes/register-pull-requests-routes.js";
|
||||
import { ApiError, sendErrorResponse } from "../api-error.js";
|
||||
import { request as REQUEST } from "../test-request.js";
|
||||
|
||||
function attachErrorHandler(app: express.Express) {
|
||||
app.use((err: unknown, _req: express.Request, res: express.Response, _next: express.NextFunction) => {
|
||||
if (err instanceof ApiError) {
|
||||
sendErrorResponse(res, err.statusCode, err.message, { details: err.details });
|
||||
return;
|
||||
}
|
||||
sendErrorResponse(res, 500, err instanceof Error ? err.message : "Internal server error");
|
||||
});
|
||||
}
|
||||
|
||||
function buildEntity(overrides: Partial<PrEntity> = {}): PrEntity {
|
||||
return {
|
||||
id: "PR-1",
|
||||
sourceType: "task",
|
||||
sourceId: "FN-1",
|
||||
repo: "owner/repo",
|
||||
headBranch: "feature/x",
|
||||
state: "open",
|
||||
prNumber: 42,
|
||||
prUrl: "https://example/pr/42",
|
||||
mergeable: "clean",
|
||||
checksRollup: "success",
|
||||
reviewDecision: "APPROVED",
|
||||
autoMerge: false,
|
||||
unverified: false,
|
||||
responseRounds: 0,
|
||||
createdAt: Date.now(),
|
||||
updatedAt: Date.now(),
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function createStore(entity: PrEntity, threads: PrThreadState[] = []) {
|
||||
let current = { ...entity };
|
||||
const store = {
|
||||
getPrEntity: vi.fn((id: string) => (id === current.id ? current : null)),
|
||||
listActivePrEntities: vi.fn(() => [current]),
|
||||
listPrThreadStates: vi.fn(() => threads),
|
||||
updatePrEntity: vi.fn((_id: string, patch: Partial<PrEntity>) => {
|
||||
current = { ...current, ...patch } as PrEntity;
|
||||
return current;
|
||||
}),
|
||||
getTask: vi.fn(async (id: string) => ({ id, column: "in-review" } as Task)),
|
||||
} as unknown as TaskStore;
|
||||
return { store, getCurrent: () => current, setCurrent: (e: PrEntity) => { current = e; } };
|
||||
}
|
||||
|
||||
function mount(store: TaskStore, opts?: Parameters<typeof createPullRequestsRouter>[1]) {
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use("/api/pull-requests", createPullRequestsRouter(store, opts));
|
||||
attachErrorHandler(app);
|
||||
return app;
|
||||
}
|
||||
|
||||
describe("pull request routes", () => {
|
||||
let entity: PrEntity;
|
||||
let threads: PrThreadState[];
|
||||
|
||||
beforeEach(() => {
|
||||
entity = buildEntity();
|
||||
threads = [
|
||||
{ prEntityId: "PR-1", threadId: "T1", headOid: "abc", outcome: "pending", updatedAt: Date.now() },
|
||||
{ prEntityId: "PR-1", threadId: "T2", headOid: "abc", outcome: "disagreed", updatedAt: Date.now() },
|
||||
];
|
||||
});
|
||||
|
||||
it("GET list returns entity with checks/threads/merge/conflict summary", async () => {
|
||||
const { store } = createStore(entity, threads);
|
||||
const app = mount(store);
|
||||
const res = await REQUEST(app, "GET", "/api/pull-requests");
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.pullRequests).toHaveLength(1);
|
||||
const pr = res.body.pullRequests[0];
|
||||
expect(pr.threads).toHaveLength(2);
|
||||
expect(pr.summary.checksRollup).toBe("success");
|
||||
expect(pr.summary.mergeable).toBe("clean");
|
||||
expect(pr.summary.conflicting).toBe(false);
|
||||
expect(pr.summary.pendingThreads).toBe(1);
|
||||
expect(pr.summary.disagreedThreads).toBe(1);
|
||||
});
|
||||
|
||||
it("GET list filters by repo and status", async () => {
|
||||
const { store } = createStore(entity, threads);
|
||||
const app = mount(store);
|
||||
let res = await REQUEST(app, "GET", "/api/pull-requests?repo=other/repo");
|
||||
expect(res.body.pullRequests).toHaveLength(0);
|
||||
res = await REQUEST(app, "GET", "/api/pull-requests?status=closed");
|
||||
expect(res.body.pullRequests).toHaveLength(0);
|
||||
res = await REQUEST(app, "GET", "/api/pull-requests?status=open");
|
||||
expect(res.body.pullRequests).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("GET :id reports conflicting summary and gate reason", async () => {
|
||||
const conflict = buildEntity({ mergeable: "conflicting", autoMerge: true });
|
||||
const { store } = createStore(conflict, threads);
|
||||
const app = mount(store);
|
||||
const res = await REQUEST(app, "GET", "/api/pull-requests/PR-1");
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.pullRequest.summary.conflicting).toBe(true);
|
||||
expect(res.body.pullRequest.summary.autoMergeReason).toBe("Blocked: conflict");
|
||||
});
|
||||
|
||||
it("GET :id returns 404 for unknown PR", async () => {
|
||||
const { store } = createStore(entity);
|
||||
const app = mount(store);
|
||||
const res = await REQUEST(app, "GET", "/api/pull-requests/PR-404");
|
||||
expect(res.status).toBe(404);
|
||||
});
|
||||
|
||||
it("merge re-fetches authoritative state before acting (not a stale client copy)", async () => {
|
||||
const { store } = createStore(entity, threads);
|
||||
const mergePr = vi.fn(async () => ({ released: true }));
|
||||
const app = mount(store, { mergePr });
|
||||
// Client sends a stale body claiming an old/wrong state — the route must ignore it.
|
||||
const res = await REQUEST(
|
||||
app,
|
||||
"POST",
|
||||
"/api/pull-requests/PR-1/merge",
|
||||
JSON.stringify({ entity: { id: "PR-1", state: "creating", mergeable: "conflicting" } }),
|
||||
{ "content-type": "application/json" },
|
||||
);
|
||||
expect(res.status).toBe(200);
|
||||
// getPrEntity is the authoritative re-read; it must have been consulted.
|
||||
expect((store.getPrEntity as unknown as ReturnType<typeof vi.fn>)).toHaveBeenCalledWith("PR-1");
|
||||
// The capability received the AUTHORITATIVE entity (clean/open), not the stale client copy.
|
||||
expect(mergePr).toHaveBeenCalledTimes(1);
|
||||
const arg = mergePr.mock.calls[0][0] as { entity: PrEntity };
|
||||
expect(arg.entity.state).toBe("open");
|
||||
expect(arg.entity.mergeable).toBe("clean");
|
||||
});
|
||||
|
||||
it("merge is rejected (409) when the authoritative entity is conflicting", async () => {
|
||||
const conflict = buildEntity({ mergeable: "conflicting" });
|
||||
const { store } = createStore(conflict, threads);
|
||||
const mergePr = vi.fn(async () => ({ released: true }));
|
||||
const app = mount(store, { mergePr });
|
||||
const res = await REQUEST(app, "POST", "/api/pull-requests/PR-1/merge", JSON.stringify({}), {
|
||||
"content-type": "application/json",
|
||||
});
|
||||
expect(res.status).toBe(409);
|
||||
expect(mergePr).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("approve/retry/close route to the injected engine capabilities", async () => {
|
||||
const { store } = createStore(entity, threads);
|
||||
const approvePr = vi.fn(async () => ({ released: true, action: "approve" }));
|
||||
const retryPr = vi.fn(async () => ({ released: true, action: "retry" }));
|
||||
const closePr = vi.fn(async () => ({ released: true, action: "close" }));
|
||||
const app = mount(store, { approvePr, retryPr, closePr });
|
||||
|
||||
for (const [path, spy] of [["approve", approvePr], ["retry", retryPr], ["close", closePr]] as const) {
|
||||
const res = await REQUEST(app, "POST", `/api/pull-requests/PR-1/${path}`, JSON.stringify({}), {
|
||||
"content-type": "application/json",
|
||||
});
|
||||
expect(res.status).toBe(200);
|
||||
expect(spy).toHaveBeenCalledTimes(1);
|
||||
expect(res.body.pullRequest.id).toBe("PR-1");
|
||||
}
|
||||
});
|
||||
|
||||
it("retry-create only acts on failed entities and routes to retryCreate", async () => {
|
||||
const failed = buildEntity({ state: "failed", failureReason: "auth" });
|
||||
const { store } = createStore(failed);
|
||||
const retryCreate = vi.fn(async () => ({ released: true }));
|
||||
const app = mount(store, { retryCreate });
|
||||
const res = await REQUEST(app, "POST", "/api/pull-requests/PR-1/retry-create", JSON.stringify({}), {
|
||||
"content-type": "application/json",
|
||||
});
|
||||
expect(res.status).toBe(200);
|
||||
expect(retryCreate).toHaveBeenCalledTimes(1);
|
||||
|
||||
// open entity → retry-create rejected (wrong state)
|
||||
const { store: openStore } = createStore(buildEntity({ state: "open" }));
|
||||
const retryCreate2 = vi.fn();
|
||||
const openApp = mount(openStore, { retryCreate: retryCreate2 as unknown as () => Promise<Record<string, unknown>> });
|
||||
const res2 = await REQUEST(openApp, "POST", "/api/pull-requests/PR-1/retry-create", JSON.stringify({}), {
|
||||
"content-type": "application/json",
|
||||
});
|
||||
expect(res2.status).toBe(409);
|
||||
expect(retryCreate2).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("action 400s when the capability is not wired", async () => {
|
||||
const { store } = createStore(entity);
|
||||
const app = mount(store, {}); // no approvePr
|
||||
const res = await REQUEST(app, "POST", "/api/pull-requests/PR-1/approve", JSON.stringify({}), {
|
||||
"content-type": "application/json",
|
||||
});
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
|
||||
it("automerge toggle persists the flip and returns the gate reason", async () => {
|
||||
const { store, getCurrent } = createStore(buildEntity({ autoMerge: false }));
|
||||
const app = mount(store);
|
||||
const res = await REQUEST(app, "POST", "/api/pull-requests/PR-1/automerge", JSON.stringify({ enabled: true }), {
|
||||
"content-type": "application/json",
|
||||
});
|
||||
expect(res.status).toBe(200);
|
||||
expect(getCurrent().autoMerge).toBe(true);
|
||||
expect(res.body.pullRequest.summary.autoMergeReason).toBe("Ready to merge");
|
||||
});
|
||||
});
|
||||
|
||||
describe("column move-backward guard (R16)", () => {
|
||||
// COLUMNS order: triage(0) todo(1) in-progress(2) in-review(3) done(4).
|
||||
it("blocks in-review (3) → in-progress (2) while an open PR exists, with guidance", () => {
|
||||
expect(
|
||||
isBackwardMoveBlockedByOpenPr({
|
||||
fromIndex: 3,
|
||||
toIndex: 2,
|
||||
activePrEntity: buildEntity({ state: "open" }),
|
||||
}),
|
||||
).toBe(true);
|
||||
expect(PR_OPEN_BLOCKS_MOVE_BACK_MESSAGE).toBe(
|
||||
"This task has an open PR. Merge or close the PR before moving it back.",
|
||||
);
|
||||
});
|
||||
|
||||
it("allows the backward move once the PR is terminal (no active entity)", () => {
|
||||
expect(
|
||||
isBackwardMoveBlockedByOpenPr({ fromIndex: 3, toIndex: 2, activePrEntity: null }),
|
||||
).toBe(false);
|
||||
// A terminal entity should also not block (defensive: store excludes these).
|
||||
expect(
|
||||
isBackwardMoveBlockedByOpenPr({
|
||||
fromIndex: 3,
|
||||
toIndex: 2,
|
||||
activePrEntity: buildEntity({ state: "closed" }),
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("never blocks a forward move even with an open PR", () => {
|
||||
expect(
|
||||
isBackwardMoveBlockedByOpenPr({
|
||||
fromIndex: 3,
|
||||
toIndex: 4,
|
||||
activePrEntity: buildEntity({ state: "open" }),
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -13,8 +13,10 @@ import { createDevServerRouter } from "../dev-server-routes.js";
|
||||
import type { AiSessionStore } from "../ai-session-store.js";
|
||||
import { createStashRecoveryRouter } from "./register-stash-recovery-routes.js";
|
||||
import { createBranchGroupsRouter } from "./register-branch-groups-routes.js";
|
||||
import { createPullRequestsRouter } from "./register-pull-requests-routes.js";
|
||||
import { GitHubClient, closeGroupPullRequest, reconcileGroupPullRequest } from "../github.js";
|
||||
import { reconcileBranchGroupPr } from "@fusion/engine";
|
||||
import { reconcileBranchGroupPr, releaseHeldTaskByEvent } from "@fusion/engine";
|
||||
import type { PrEntity } from "@fusion/core";
|
||||
|
||||
interface IntegratedRoutersOptions {
|
||||
router: Router;
|
||||
@@ -115,6 +117,29 @@ export function registerIntegratedRouters({
|
||||
return store.getBranchGroup(group.id) ?? group;
|
||||
},
|
||||
}));
|
||||
|
||||
// Unified PR entity view + user-controlled actions (U7, R11/R12/R13). Each
|
||||
// side-effecting action maps to a manual hold-release: the workflow's
|
||||
// user-controlled release edges own the GitHub side effects, so the route just
|
||||
// releases the entity's source task with an action-specific event tag. The
|
||||
// route layer already re-reads authoritative entity state before invoking these
|
||||
// callbacks (never a stale client copy). The engine primitive is imported
|
||||
// statically (FN-3049 — no runtime `await import`).
|
||||
const releaseForPr = async (entity: PrEntity, eventTag: string): Promise<Record<string, unknown>> => {
|
||||
// task-sourced entities release the task directly; branch-group-sourced
|
||||
// entities release the group's representative task (the sourceId is the task
|
||||
// id the workflow placed on the await hold in both cases).
|
||||
const result = await releaseHeldTaskByEvent(store, entity.sourceId, eventTag);
|
||||
return { released: result.released, toColumn: result.toColumn, rejection: result.rejection };
|
||||
};
|
||||
|
||||
router.use("/pull-requests", createPullRequestsRouter(store, {
|
||||
approvePr: ({ entity }) => releaseForPr(entity, "pr-approve"),
|
||||
mergePr: ({ entity }) => releaseForPr(entity, "pr-merge"),
|
||||
retryPr: ({ entity }) => releaseForPr(entity, "pr-retry"),
|
||||
closePr: ({ entity }) => releaseForPr(entity, "pr-close"),
|
||||
retryCreate: ({ entity }) => releaseForPr(entity, "pr-retry-create"),
|
||||
}));
|
||||
}
|
||||
|
||||
export function registerIntegratedDevServerRouter({ router, store }: DevServerRouterOptions): void {
|
||||
|
||||
239
packages/dashboard/src/routes/register-pull-requests-routes.ts
Normal file
239
packages/dashboard/src/routes/register-pull-requests-routes.ts
Normal file
@@ -0,0 +1,239 @@
|
||||
import { Router, type Request } from "express";
|
||||
import type { PrEntity, PrThreadState, TaskStore } from "@fusion/core";
|
||||
import {
|
||||
isPrEntityActive,
|
||||
isPrEntityActionable,
|
||||
isPrEntityAutoMergeReady,
|
||||
} from "@fusion/core";
|
||||
import { badRequest, notFound, ApiError } from "../api-error.js";
|
||||
|
||||
/**
|
||||
* Injected engine capabilities for the user-controlled PR actions (U7, R13).
|
||||
*
|
||||
* Each action maps to a manual hold-release (the workflow's user-controlled
|
||||
* release edges own the real GitHub side effects): approve/merge/retry/close all
|
||||
* fire the same release authority the scheduler's hold-release sweep uses. The
|
||||
* router never imports the engine directly (FN-3049) — capabilities arrive as
|
||||
* option callbacks wired in register-integrated-routers.ts. When a capability is
|
||||
* omitted the corresponding action 400s ("unavailable") rather than no-op'ing
|
||||
* silently.
|
||||
*
|
||||
* All side-effecting callbacks receive the AUTHORITATIVE entity the route just
|
||||
* re-read from the store — never a client-supplied copy. Acting on a stale
|
||||
* client copy is the bug class documented in
|
||||
* docs/solutions/logic-errors/queued-chat-message-flush-trusts-stale-isgenerating.md.
|
||||
*/
|
||||
export interface PullRequestsRouterOptions {
|
||||
/** Release the PR's source task to advance toward merge (approve / force-merge). */
|
||||
approvePr?: (input: { entity: PrEntity; projectId?: string }) => Promise<Record<string, unknown>>;
|
||||
/** Merge the PR via the workflow's merge release (force-merge). */
|
||||
mergePr?: (input: { entity: PrEntity; projectId?: string }) => Promise<Record<string, unknown>>;
|
||||
/** Request another review-response round (rework release). */
|
||||
retryPr?: (input: { entity: PrEntity; projectId?: string }) => Promise<Record<string, unknown>>;
|
||||
/** Close the PR terminally and reconcile the entity. */
|
||||
closePr?: (input: { entity: PrEntity; projectId?: string }) => Promise<Record<string, unknown>>;
|
||||
/** Retry a failed PR creation (state === "failed", R4). */
|
||||
retryCreate?: (input: { entity: PrEntity; projectId?: string }) => Promise<Record<string, unknown>>;
|
||||
}
|
||||
|
||||
function parseProjectId(req: Request): string | undefined {
|
||||
const value = req.query.projectId ?? req.body?.projectId;
|
||||
return typeof value === "string" && value.trim() ? value.trim() : undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* The live auto-merge gate reason shown next to the toggle (R11). Mirrors the
|
||||
* engine's auto-merge-ready predicate ordering so the UI never disagrees with
|
||||
* what the gate will actually do.
|
||||
*/
|
||||
export function autoMergeGateReason(entity: PrEntity): string {
|
||||
if (!entity.autoMerge) return "Auto-merge off";
|
||||
if (entity.mergeable === "conflicting") return "Blocked: conflict";
|
||||
if (entity.reviewDecision !== "APPROVED") return "Waiting for approval";
|
||||
if (entity.checksRollup !== "success") return "Waiting for checks";
|
||||
if (entity.mergeable !== "clean") return "Waiting for checks";
|
||||
if (isPrEntityAutoMergeReady(entity)) return "Ready to merge";
|
||||
return "Waiting for checks";
|
||||
}
|
||||
|
||||
/** Whether the entity is in a hard conflict (Merge must be disabled, R11). */
|
||||
export function isPrConflicting(entity: PrEntity): boolean {
|
||||
return entity.mergeable === "conflicting";
|
||||
}
|
||||
|
||||
/** Structured rejection message for the R16 column-move-backward block. */
|
||||
export const PR_OPEN_BLOCKS_MOVE_BACK_MESSAGE =
|
||||
"This task has an open PR. Merge or close the PR before moving it back.";
|
||||
|
||||
/**
|
||||
* R16: should a column move be blocked because the task has an open PR?
|
||||
*
|
||||
* A "backward" move (lower column index) of a task that still has an ACTIVE
|
||||
* (non-terminal) PR entity is rejected — the PR's lifecycle is workflow-owned and
|
||||
* dragging the card back would orphan the open GitHub PR. Forward moves and moves
|
||||
* of tasks whose PR is terminal (merged/closed/failed → no active entity) pass.
|
||||
*
|
||||
* Pure so the move route and tests consult one definition.
|
||||
*/
|
||||
export function isBackwardMoveBlockedByOpenPr(input: {
|
||||
fromIndex: number;
|
||||
toIndex: number;
|
||||
activePrEntity: Pick<PrEntity, "state"> | null | undefined;
|
||||
}): boolean {
|
||||
const { fromIndex, toIndex, activePrEntity } = input;
|
||||
if (fromIndex < 0 || toIndex < 0) return false;
|
||||
if (toIndex >= fromIndex) return false; // not backward
|
||||
return Boolean(activePrEntity && isPrEntityActive(activePrEntity));
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the merge-readiness summary the view renders above the checks list.
|
||||
* Pure derivation from authoritative entity state.
|
||||
*/
|
||||
export function buildPrSummary(entity: PrEntity, threads: PrThreadState[]) {
|
||||
const pendingThreads = threads.filter((t) => t.outcome === "pending").length;
|
||||
const disagreedThreads = threads.filter((t) => t.outcome === "disagreed").length;
|
||||
return {
|
||||
mergeable: entity.mergeable ?? "unknown",
|
||||
reviewDecision: entity.reviewDecision ?? null,
|
||||
checksRollup: entity.checksRollup ?? "none",
|
||||
conflicting: isPrConflicting(entity),
|
||||
autoMerge: entity.autoMerge,
|
||||
autoMergeReason: autoMergeGateReason(entity),
|
||||
autoMergeReady: isPrEntityAutoMergeReady(entity),
|
||||
actionable: isPrEntityActionable(entity),
|
||||
active: isPrEntityActive(entity),
|
||||
pendingThreads,
|
||||
disagreedThreads,
|
||||
};
|
||||
}
|
||||
|
||||
function serializePr(entity: PrEntity, threads: PrThreadState[]) {
|
||||
return {
|
||||
...entity,
|
||||
threads,
|
||||
summary: buildPrSummary(entity, threads),
|
||||
};
|
||||
}
|
||||
|
||||
export function createPullRequestsRouter(store: TaskStore, options?: PullRequestsRouterOptions): Router {
|
||||
const router = Router();
|
||||
|
||||
// GET /api/pull-requests — list active entities, optional repo/status filter.
|
||||
router.get("/", async (_req, res) => {
|
||||
const repoRaw = _req.query.repo;
|
||||
const statusRaw = _req.query.status;
|
||||
const repo = typeof repoRaw === "string" && repoRaw.trim() ? repoRaw.trim() : undefined;
|
||||
const status = typeof statusRaw === "string" && statusRaw.trim() ? statusRaw.trim() : undefined;
|
||||
if (
|
||||
status &&
|
||||
!["creating", "open", "responding", "merged", "closed", "failed"].includes(status)
|
||||
) {
|
||||
throw badRequest(
|
||||
"status must be one of: creating, open, responding, merged, closed, failed",
|
||||
);
|
||||
}
|
||||
|
||||
let entities = store.listActivePrEntities();
|
||||
if (repo) entities = entities.filter((e) => e.repo === repo);
|
||||
if (status) entities = entities.filter((e) => e.state === status);
|
||||
|
||||
const pullRequests = entities.map((entity) =>
|
||||
serializePr(entity, store.listPrThreadStates(entity.id)),
|
||||
);
|
||||
res.json({ pullRequests });
|
||||
});
|
||||
|
||||
// GET /api/pull-requests/:id — entity + thread states + checks/merge/conflict summary.
|
||||
router.get("/:id", async (req, res) => {
|
||||
const id = String(req.params.id ?? "").trim();
|
||||
if (!id) throw badRequest("id is required");
|
||||
const entity = store.getPrEntity(id);
|
||||
if (!entity) throw notFound("PR entity not found");
|
||||
res.json({ pullRequest: serializePr(entity, store.listPrThreadStates(id)) });
|
||||
});
|
||||
|
||||
/**
|
||||
* Shared action handler: re-read the AUTHORITATIVE entity (never trust a client
|
||||
* copy), gate it, then dispatch to the injected capability. Returns the freshly
|
||||
* re-read serialized entity so the client replaces its stale copy.
|
||||
*/
|
||||
function makeAction(
|
||||
name: string,
|
||||
capability: ((input: { entity: PrEntity; projectId?: string }) => Promise<Record<string, unknown>>) | undefined,
|
||||
opts: { requireActive?: boolean; requireState?: PrEntity["state"]; rejectConflict?: boolean } = {},
|
||||
) {
|
||||
return async (req: Request, res: import("express").Response) => {
|
||||
const id = String(req.params.id ?? "").trim();
|
||||
if (!id) throw badRequest("id is required");
|
||||
|
||||
// Re-fetch authoritative state — the side effect must never gate on a
|
||||
// stale client/SSE-delivered copy.
|
||||
const entity = store.getPrEntity(id);
|
||||
if (!entity) throw notFound("PR entity not found");
|
||||
|
||||
if (opts.requireState && entity.state !== opts.requireState) {
|
||||
throw new ApiError(409, `PR is not in '${opts.requireState}' state`, {
|
||||
code: "pr-wrong-state",
|
||||
retryable: false,
|
||||
});
|
||||
}
|
||||
if (opts.requireActive && !isPrEntityActive(entity)) {
|
||||
throw new ApiError(409, "PR is already terminal (merged/closed/failed)", {
|
||||
code: "pr-terminal",
|
||||
retryable: false,
|
||||
});
|
||||
}
|
||||
if (opts.rejectConflict && isPrConflicting(entity)) {
|
||||
throw new ApiError(409, "Resolve conflicts on GitHub before merging", {
|
||||
code: "pr-conflict",
|
||||
retryable: false,
|
||||
});
|
||||
}
|
||||
|
||||
if (!capability) {
|
||||
throw badRequest(`${name} is unavailable`);
|
||||
}
|
||||
|
||||
const result = await capability({ entity, projectId: parseProjectId(req) });
|
||||
// Re-read after the action so the response reflects authoritative state.
|
||||
const fresh = store.getPrEntity(id) ?? entity;
|
||||
res.json({
|
||||
...result,
|
||||
pullRequest: serializePr(fresh, store.listPrThreadStates(id)),
|
||||
});
|
||||
};
|
||||
}
|
||||
|
||||
router.post("/:id/approve", makeAction("Approve", options?.approvePr, { requireActive: true }));
|
||||
router.post(
|
||||
"/:id/merge",
|
||||
makeAction("Merge", options?.mergePr, { requireActive: true, rejectConflict: true }),
|
||||
);
|
||||
router.post("/:id/retry", makeAction("Retry", options?.retryPr, { requireActive: true }));
|
||||
router.post("/:id/close", makeAction("Close", options?.closePr, { requireActive: true }));
|
||||
router.post(
|
||||
"/:id/retry-create",
|
||||
makeAction("Retry PR creation", options?.retryCreate, { requireState: "failed" }),
|
||||
);
|
||||
|
||||
// Toggle auto-merge. Re-reads authoritative state then persists the flip.
|
||||
router.post("/:id/automerge", async (req, res) => {
|
||||
const id = String(req.params.id ?? "").trim();
|
||||
if (!id) throw badRequest("id is required");
|
||||
const entity = store.getPrEntity(id);
|
||||
if (!entity) throw notFound("PR entity not found");
|
||||
if (!isPrEntityActive(entity)) {
|
||||
throw new ApiError(409, "PR is already terminal (merged/closed/failed)", {
|
||||
code: "pr-terminal",
|
||||
retryable: false,
|
||||
});
|
||||
}
|
||||
const enabled =
|
||||
typeof req.body?.enabled === "boolean" ? req.body.enabled : !entity.autoMerge;
|
||||
const updated = store.updatePrEntity(id, { autoMerge: enabled });
|
||||
res.json({ pullRequest: serializePr(updated, store.listPrThreadStates(id)) });
|
||||
});
|
||||
|
||||
return router;
|
||||
}
|
||||
@@ -45,6 +45,7 @@ import { createTrackingIssueForTask } from "../github-tracking-hook.js";
|
||||
import { parseGitHubBadgeUrl } from "./register-git-github.js";
|
||||
import { planTaskWorktreePath, promoteHeldTask } from "@fusion/engine";
|
||||
import { buildBoardWorkflowsPayload } from "./board-workflows.js";
|
||||
import { isBackwardMoveBlockedByOpenPr, PR_OPEN_BLOCKS_MOVE_BACK_MESSAGE } from "./register-pull-requests-routes.js";
|
||||
import type { RunAuditEventInput } from "@fusion/core";
|
||||
import { ApiError, badRequest, conflict, notFound } from "../api-error.js";
|
||||
import type { ApiRoutesContext } from "./types.js";
|
||||
@@ -1364,6 +1365,35 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
||||
throw badRequest("preserveProgress must be a boolean");
|
||||
}
|
||||
|
||||
// R16: block moving a PR-await task "backward" (e.g. in-review → in-progress)
|
||||
// while it still has an open PR entity. The PR's lifecycle is workflow-owned;
|
||||
// dragging the card back would orphan the open GitHub PR. The user must
|
||||
// merge or close the PR first (a user-controlled release advances it
|
||||
// forward; this guard only rejects backward drags). Once the entity is
|
||||
// terminal (merged/closed/failed) the move is allowed.
|
||||
const moveTarget = column as Column;
|
||||
const guardTask = await scopedStore.getTask(req.params.id);
|
||||
if (guardTask) {
|
||||
const activePrEntity =
|
||||
scopedStore.getActivePrEntityBySource?.("task", guardTask.id) ??
|
||||
(guardTask.branchContext?.groupId
|
||||
? scopedStore.getActivePrEntityBySource?.("branch-group", guardTask.branchContext.groupId)
|
||||
: null);
|
||||
if (
|
||||
isBackwardMoveBlockedByOpenPr({
|
||||
fromIndex: COLUMNS.indexOf(guardTask.column as Column),
|
||||
toIndex: COLUMNS.indexOf(moveTarget),
|
||||
activePrEntity,
|
||||
})
|
||||
) {
|
||||
throw new ApiError(409, PR_OPEN_BLOCKS_MOVE_BACK_MESSAGE, {
|
||||
code: "pr-open-blocks-move-back",
|
||||
messageKey: "board.rejection.prOpenBlocksMoveBack",
|
||||
retryable: false,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// When manually promoting to in-progress, supply an allocator so
|
||||
// moveTask assigns a worktree path under its cross-task allocation
|
||||
// lock. This mirrors scheduler dispatch semantics — without it, a
|
||||
|
||||
Reference in New Issue
Block a user