diff --git a/server/src/__tests__/issue-thread-interactions-service.test.ts b/server/src/__tests__/issue-thread-interactions-service.test.ts index eae6dbc28d..30dc0b1be7 100644 --- a/server/src/__tests__/issue-thread-interactions-service.test.ts +++ b/server/src/__tests__/issue-thread-interactions-service.test.ts @@ -2069,6 +2069,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { async function seedAcceptGateFixture(options?: { kind?: AcceptGateInteractionKind; sourceRunId?: string | null; + sourceRunStatus?: string; }) { const companyId = randomUUID(); const projectId = randomUUID(); @@ -2115,6 +2116,8 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { runtimeConfig: {}, permissions: {}, }); + const sourceRunStatus = options?.sourceRunStatus ?? "succeeded"; + const sourceRunTerminal = sourceRunStatus !== "running"; await db.insert(heartbeatRuns).values([ ...(sourceRunId ? [ @@ -2123,9 +2126,9 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { companyId, agentId, invocationSource: "manual", - status: "succeeded", + status: sourceRunStatus, startedAt: new Date("2026-05-23T21:55:00.000Z"), - finishedAt: new Date("2026-05-23T22:05:00.000Z"), + finishedAt: sourceRunTerminal ? new Date("2026-05-23T22:05:00.000Z") : null, }, ] : []), @@ -2300,6 +2303,105 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { }); }); + it("allows request_confirmation accept when the source run's workspace_finalize failed", async () => { + // A sync-back that ran and FAILED is terminal. The run will not retry it, so + // the confirmation must not stay wedged behind a misleading "still syncing" + // error — the user can merge/act manually. + const { companyId, executionWorkspaceId, issueId, goalId, interactionId, sourceRunId } = + await seedAcceptGateFixture({ sourceRunStatus: "failed" }); + + await db.insert(workspaceOperations).values({ + companyId, + executionWorkspaceId, + heartbeatRunId: sourceRunId, + phase: "workspace_config_freshness", + status: "succeeded", + startedAt: new Date("2026-05-23T22:00:00.000Z"), + }); + await db.insert(workspaceOperations).values({ + companyId, + executionWorkspaceId, + heartbeatRunId: sourceRunId, + phase: "workspace_finalize", + status: "failed", + startedAt: new Date("2026-05-23T22:05:00.000Z"), + }); + + const accepted = await interactionsSvc.acceptInteraction( + { id: issueId, companyId, goalId, projectId: null }, + interactionId, + {}, + { userId: "local-board" }, + ); + + expect(accepted.interaction).toMatchObject({ + id: interactionId, + kind: "request_confirmation", + status: "accepted", + }); + }); + + it("allows request_confirmation accept when a running workspace_finalize is stale (source run ended)", async () => { + // The source run died mid-finalize, leaving a `running` op that will never + // advance. A terminal/missing owner run means the record is stale, so the + // gate must not wait on it forever. + const { companyId, executionWorkspaceId, issueId, goalId, interactionId, sourceRunId } = + await seedAcceptGateFixture({ sourceRunStatus: "failed" }); + + await db.insert(workspaceOperations).values({ + companyId, + executionWorkspaceId, + heartbeatRunId: sourceRunId, + phase: "workspace_finalize", + status: "running", + startedAt: new Date("2026-05-23T22:05:00.000Z"), + }); + + const accepted = await interactionsSvc.acceptInteraction( + { id: issueId, companyId, goalId, projectId: null }, + interactionId, + {}, + { userId: "local-board" }, + ); + + expect(accepted.interaction).toMatchObject({ + id: interactionId, + kind: "request_confirmation", + status: "accepted", + }); + }); + + it("refuses request_confirmation accept while a workspace_finalize is running on a live source run", async () => { + // A genuinely in-flight sync-back on a still-active run must still block, so + // the confirmation cannot race commits that are actively being synced back. + const { companyId, executionWorkspaceId, issueId, goalId, interactionId, sourceRunId } = + await seedAcceptGateFixture({ sourceRunStatus: "running" }); + + await db.insert(workspaceOperations).values({ + companyId, + executionWorkspaceId, + heartbeatRunId: sourceRunId, + phase: "workspace_finalize", + status: "running", + startedAt: new Date("2026-05-23T22:05:00.000Z"), + }); + + await expect( + interactionsSvc.acceptInteraction( + { id: issueId, companyId, goalId, projectId: null }, + interactionId, + {}, + { userId: "local-board" }, + ), + ).rejects.toMatchObject({ + status: 409, + message: expect.stringContaining( + "the run that created this interaction has not finished syncing its workspace", + ), + details: { executionWorkspaceId, sourceRunId }, + }); + }); + it("allows request_confirmation accept when sourceRunId is null", async () => { const { companyId, executionWorkspaceId, issueId, goalId, interactionId, foreignRunId } = await seedAcceptGateFixture({ sourceRunId: null }); diff --git a/server/src/services/issue-thread-interactions.ts b/server/src/services/issue-thread-interactions.ts index a5c74a5eea..153d548d3f 100644 --- a/server/src/services/issue-thread-interactions.ts +++ b/server/src/services/issue-thread-interactions.ts @@ -888,6 +888,11 @@ export function issueThreadInteractionService(db: Db) { if (!executionWorkspaceId) return; + // Block only while the source run's worktree sync-back is genuinely still + // pending or in flight. A finalize that reached a terminal outcome — including + // a `failed` sync-back or a stale `running` record left by an ended run — is + // treated as settled by `runWorkspaceIsFinalized`, so a dead run can no longer + // wedge this confirmation forever. const isFinalized = await runWorkspaceIsFinalized( args.db, args.issue.companyId, diff --git a/server/src/services/issues.ts b/server/src/services/issues.ts index e1281028a7..9b45ca648a 100644 --- a/server/src/services/issues.ts +++ b/server/src/services/issues.ts @@ -1059,11 +1059,42 @@ async function listPendingFinalizeBlockerIssueIds( } /** - * Returns whether a specific run's operations on a specific execution workspace - * reached the workspace_finalize barrier. + * Whether a heartbeat run has reached a terminal state or no longer exists. + * A terminal/missing run can make no further progress on its execution + * workspace, so callers must not wait on it to advance an in-flight operation. + */ +export async function heartbeatRunIsTerminalOrMissing( + dbOrTx: Pick, + runId: string, +): Promise { + const run = await dbOrTx + .select({ status: heartbeatRuns.status }) + .from(heartbeatRuns) + .where(eq(heartbeatRuns.id, runId)) + .then((rows: Array<{ status: string }>) => rows[0] ?? null); + if (!run) return true; + return TERMINAL_HEARTBEAT_RUN_STATUSES.has(run.status); +} + +/** + * Returns whether a specific run's sync-back on a specific execution workspace + * has settled — i.e. the accept/review gates that guard against a still-in-flight + * worktree sync no longer need to block on this run. * - * Runs with no operations on the workspace are considered finalized because - * they never touched the workspace state that accept/review gates protect. + * Semantics: + * - No operations recorded → settled. The run never touched the workspace state + * the gates protect. + * - Earlier phases recorded but no `workspace_finalize` yet → NOT settled. The + * sync-back hasn't been attempted; the gate should wait for it. + * - Latest `workspace_finalize` reached a terminal status (`succeeded`, `failed`, + * or `skipped`) → settled. A finalize that ran and finished is done even if it + * failed: it will not retry within this run, so continuing to block would wedge + * the gate forever — a failed sync-back must not permanently block a + * confirmation accept behind a misleading "still syncing" error. + * - Latest `workspace_finalize` is still `running` → in flight, so NOT settled — + * unless the owning run has itself ended, in which case the `running` record is + * stale (the process died mid-finalize) and we treat it as settled rather than + * wait on a run that can never make progress. */ export async function runWorkspaceIsFinalized( dbOrTx: Pick, @@ -1086,13 +1117,24 @@ export async function runWorkspaceIsFinalized( ), ); - let latest: { phase: string; status: string; startedAt: Date } | null = null; + if (rows.length === 0) return true; + + let latestFinalize: { status: string; startedAt: Date } | null = null; for (const row of rows) { - if (!latest || row.startedAt > latest.startedAt) latest = row; + if (row.phase !== "workspace_finalize") continue; + if (!latestFinalize || row.startedAt > latestFinalize.startedAt) latestFinalize = row; } - if (!latest) return true; - return latest.phase === "workspace_finalize" && latest.status === "succeeded"; + // The run touched the workspace but hasn't reached the sync-back phase yet. + if (!latestFinalize) return false; + + // A finalize that reached any terminal status is settled — including `failed` + // and `skipped`. It will not retry within this run, so gates must stop waiting. + if (latestFinalize.status !== "running") return true; + + // Finalize is still marked `running`. It is only genuinely in flight while the + // owning run is alive; a `running` record left behind by an ended run is stale. + return heartbeatRunIsTerminalOrMissing(dbOrTx, runId); } async function listIssueDependencyReadinessMap( @@ -4456,13 +4498,7 @@ export function issueService(db: Db) { } async function isTerminalOrMissingHeartbeatRun(runId: string, dbOrTx: DbReader = db) { - const run = await dbOrTx - .select({ status: heartbeatRuns.status }) - .from(heartbeatRuns) - .where(eq(heartbeatRuns.id, runId)) - .then((rows) => rows[0] ?? null); - if (!run) return true; - return TERMINAL_HEARTBEAT_RUN_STATUSES.has(run.status); + return heartbeatRunIsTerminalOrMissing(dbOrTx, runId); } async function adoptStaleCheckoutRun(input: {