diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index dc4c1d3f7d9..3f960499f76 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -2273,9 +2273,9 @@ The developer-server health poll also stops in hidden tabs. Workspace detail responses share concurrent Git inspections and reuse their results for up to thirty seconds after completion. Workspace identity and metadata changes invalidate the cached result. The cache holds at most 256 -entries. Close-readiness checks, the terminal-workspace reaper, and the final -cleanup validation still inspect Git afresh. A display result never authorizes -worktree removal. +entries. Close-readiness checks and final cleanup validation request separate +Git scans that cannot join an older in-flight scan. A display result never +authorizes worktree removal. The terminal-workspace reaper uses the database work coordinator. Startup scans recover existing workspace rows in bounded pages. Issue status and tree changes, @@ -2285,8 +2285,12 @@ transaction settles and holds idle admission while writes or cleanup are active. Empty and nonterminal-only lanes have no recurring timer or query. Terminal trees schedule their actual cooldown deadline; unresolved delivery or cleanup eligibility is checked again after five minutes. Nonterminal trees, cooldowns, and active runs -are checked before Git. Unchanged terminal candidates also back off for five -minutes during unrelated commit wakes. Scoped task/workspace hints invalidate +are checked before Git. The worker shares Git readiness across compatible rows +that point at one directory within a bounded page. Company, branch, base ref, +and runtime ownership must match. Each row keeps its own task and delivery +assessment, which reuses the task tree read by the early gate. Unchanged terminal +candidates also back off for five minutes during unrelated commit wakes. Scoped +task/workspace hints invalidate affected candidates; changed tree or workspace revisions invalidate them too. Deadline selection reuses task trees read by those pages and performs no separate full-inventory query. Commits during a paged rotation request another bounded diff --git a/maps/feature-map/workspaces.md b/maps/feature-map/workspaces.md index 35312f6fdfb..d3e8f2b70c4 100644 --- a/maps/feature-map/workspaces.md +++ b/maps/feature-map/workspaces.md @@ -109,7 +109,9 @@ Manual: Provision a disposable workspace, wait for readiness, start an available - Workspace IDs, runtime leases, and repository paths are not interchangeable. - Terminal workspace cleanup wakes after committed task/workspace/PR changes and restores cooldown deadlines at startup. Empty lanes stop scanning. Display Git - results can lag by thirty seconds; destructive cleanup still reads fresh Git. + results can lag by thirty seconds. Compatible records share one advisory Git + inspection per bounded page, while each record keeps its own delivery check. + Destructive cleanup starts a separate Git scan even if an older scan is running. The [worker tests](../../server/src/__tests__/terminal-workspace-work.test.ts) cover quiet lanes and deadlines. The [service tests](../../server/src/__tests__/execution-workspaces-service.test.ts) diff --git a/server/src/__tests__/execution-workspaces-service.test.ts b/server/src/__tests__/execution-workspaces-service.test.ts index 645dcf51077..6bd89ec434f 100644 --- a/server/src/__tests__/execution-workspaces-service.test.ts +++ b/server/src/__tests__/execution-workspaces-service.test.ts @@ -573,6 +573,36 @@ describeEmbeddedPostgres("executionWorkspaceService.getCloseReadiness", () => { expect(await fs.readFile(path.join(seed.worktreePath, "unsaved.txt"), "utf8")).toBe("keep this work\n"); }); + it("shares compatible Git inspections within a batch while keeping branch and company decisions separate", async () => { + const first = await seedTerminalWorkspace(); + const otherCompany = await seedTerminalWorkspace(); + const [original] = await db.select().from(executionWorkspaces) + .where(eq(executionWorkspaces.id, first.executionWorkspaceId)); + await db.insert(executionWorkspaces).values([ + { ...original!, id: randomUUID(), name: "Same directory", updatedAt: new Date("2020-01-01") }, + { ...original!, id: randomUUID(), name: "Different base", baseRef: "other-base", updatedAt: new Date("2020-01-01") }, + ]); + await db.update(executionWorkspaces).set({ + cwd: first.worktreePath, + providerRef: first.worktreePath, + branchName: original!.branchName, + baseRef: original!.baseRef, + }).where(eq(executionWorkspaces.id, otherCompany.executionWorkspaceId)); + const service = executionWorkspaceService(db, { workspaceReaperCooldownDays: 0 }); + const scan = vi.spyOn(workspaceGitOperationScheduler, "run"); + const select = vi.spyOn(db, "select"); + try { + expect(await service.sweepTerminalWorkspaces()).toMatchObject({ checked: 4, archived: 0 }); + expect(scan.mock.calls.filter(([input]) => input.operation === "execution_workspaces.close_readiness_status")) + .toHaveLength(3); + expect(select.mock.calls.filter(([fields]) => fields && "completedAt" in fields && "cancelledAt" in fields)) + .toHaveLength(4); + } finally { + scan.mockRestore(); + select.mockRestore(); + } + }); + it("backs off unchanged terminal candidates while unrelated commits wake the worker", async () => { await seedTerminalWorkspace(); let clock = Date.now(); diff --git a/server/src/services/execution-workspaces.ts b/server/src/services/execution-workspaces.ts index 70227a2faaf..43c990e1196 100644 --- a/server/src/services/execution-workspaces.ts +++ b/server/src/services/execution-workspaces.ts @@ -467,6 +467,7 @@ async function runExpensiveGitStatus(input: { operation: input.operation, fairnessKeys: input.fairnessKeys, cacheTtlMs: 0, + bypassSingleFlight: true, // Nested task worktrees can exceed the scheduler's 1 MiB default. // Keep exact file counts for readiness and reconciliation checks. maxStdoutBytes: 32 * 1024 * 1024, @@ -1697,8 +1698,9 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic async function assessDelivery( workspace: ExecutionWorkspaceRow, git: ExecutionWorkspaceCloseGitReadiness | null, + precomputedIssueTree?: Awaited>, ) { - const issueTree = await listWorkspaceIssueTree(workspace); + const issueTree = precomputedIssueTree ?? await listWorkspaceIssueTree(workspace); const sourceIssue = issueTree.find((issue) => issue.id === workspace.sourceIssueId) ?? null; const sourceIssueTerminal = Boolean(sourceIssue && TERMINAL_ISSUE_STATUSES.has(sourceIssue.status)); const subtreeTerminal = Boolean(sourceIssue && issueTree.every((issue) => TERMINAL_ISSUE_STATUSES.has(issue.status))); @@ -2953,6 +2955,7 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic skippedBackoff: 0, clearedStaleReopenPending: 0, }; + const readinessByWorkingTree = new Map>(); for (const workspace of candidates) { signal?.throwIfAborted(); @@ -3001,13 +3004,24 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic nextAt: now().getTime() + opts.terminalWorkspaceInspectionBackoffMs, }); } - const executionWorkspace = toExecutionWorkspace(workspace); - const { git, statusInspectionSucceeded } = await inspectGitCloseReadiness(executionWorkspace); + const workspacePath = readNullableString(workspace.providerRef) ?? readNullableString(workspace.cwd); + const createdByRuntime = workspace.providerType === "git_worktree" + ? isRuntimeOwnedGitBranch(workspace.metadata) + : workspace.metadata?.createdByRuntime === true; + const readinessKey = workspacePath + ? JSON.stringify([workspace.companyId, workspacePath, workspace.baseRef, workspace.branchName, createdByRuntime]) + : `workspace:${workspace.id}`; + let inspection = readinessByWorkingTree.get(readinessKey); + if (!inspection) { + inspection = inspectGitCloseReadiness(toExecutionWorkspace(workspace)); + readinessByWorkingTree.set(readinessKey, inspection); + } + const { git, statusInspectionSucceeded } = await inspection; if (!statusInspectionSucceeded) { result.skippedUndelivered += 1; continue; } - const assessment = await assessDelivery(workspace, git); + const assessment = await assessDelivery(workspace, git, issueTree); if (!assessment.sourceIssueTerminal || !assessment.subtreeTerminal) { if (reopenPending) { // The source issue left the terminal state, so the reopen transition diff --git a/server/src/services/workspace-git-operation-scheduler.test.ts b/server/src/services/workspace-git-operation-scheduler.test.ts index b81e0c6fab7..8a91958e54c 100644 --- a/server/src/services/workspace-git-operation-scheduler.test.ts +++ b/server/src/services/workspace-git-operation-scheduler.test.ts @@ -241,6 +241,37 @@ describe("WorkspaceGitOperationScheduler", () => { expect(scheduler.snapshot().inFlightCount).toBe(0); }); + it("starts a fresh scan after a file changes while an older scan is still running", async () => { + const workspace = await makeWorkspace(); + const firstCaptured = deferred(); + const releaseFirst = deferred(); + let calls = 0; + const runner: WorkspaceGitRunner = async () => { + calls += 1; + const files = await fs.readdir(workspace); + if (calls === 1) { + firstCaptured.resolve(); + await releaseFirst.promise; + } + return { stdout: files.join("\n"), stderr: "" }; + }; + const scheduler = createWorkspaceGitOperationScheduler({ runner, concurrency: 2 }); + const input = scanInput(workspace, "freshness"); + + const earlier = scheduler.run(input); + await firstCaptured.promise; + await fs.writeFile(path.join(workspace, "unsaved.txt"), "keep this work\n"); + const fresh = await scheduler.run({ ...input, bypassSingleFlight: true }); + releaseFirst.resolve(); + + expect((await earlier).stdout).toBe(""); + expect(fresh).toMatchObject({ stdout: "unsaved.txt", cacheHit: false, singleFlightJoined: false }); + expect(scheduler.snapshot()).toMatchObject({ + cacheEntryCount: 0, + totals: { started: 2, singleFlightJoins: 0 }, + }); + }); + it("serves cached results until TTL expiry and evicts least-recently-used entries", async () => { const firstWorkspace = await makeWorkspace("same-name"); const secondWorkspace = await makeWorkspace("same-name"); diff --git a/server/src/services/workspace-git-operation-scheduler.ts b/server/src/services/workspace-git-operation-scheduler.ts index 6b2875e6bfc..6ec6448401f 100644 --- a/server/src/services/workspace-git-operation-scheduler.ts +++ b/server/src/services/workspace-git-operation-scheduler.ts @@ -65,8 +65,8 @@ export interface WorkspaceGitScanInput { */ fairnessKeys?: readonly string[]; signal?: AbortSignal; - /** Successful-result cache duration. Use zero for correctness-sensitive guards. */ cacheTtlMs?: number; + bypassSingleFlight?: boolean; /** Per-operation wall-clock deadline. Defaults to the process-wide setting. */ timeoutMs?: number; env?: NodeJS.ProcessEnv; @@ -364,7 +364,10 @@ export class WorkspaceGitOperationScheduler { maxStdoutBytes, maxStderrBytes, }); - const cacheTtlMs = input.onStdout ? 0 : clampInteger(input.cacheTtlMs, this.defaultCacheTtlMs, 0, 60_000); + const inFlightKey = input.bypassSingleFlight ? `${key}#fresh:${randomUUID()}` : key; + const cacheTtlMs = input.onStdout || input.bypassSingleFlight + ? 0 + : clampInteger(input.cacheTtlMs, this.defaultCacheTtlMs, 0, 60_000); // A correctness-sensitive caller that explicitly disables caching must not // consume a result populated earlier by the file browser. const cached = cacheTtlMs > 0 ? this.readCache(key) : null; @@ -390,7 +393,7 @@ export class WorkspaceGitOperationScheduler { }; } - const existing = this.inFlight.get(key); + const existing = this.inFlight.get(inFlightKey); if (existing) { existing.joinCount += 1; this.totals.singleFlightJoins += 1; @@ -425,7 +428,7 @@ export class WorkspaceGitOperationScheduler { ...(input.fairnessKeys ?? []).filter(Boolean), ])).sort(); const scan: PendingScan = { - key, + key: inFlightKey, operation: input.operation, canonicalWorkspacePath, workspaceHash, @@ -443,7 +446,7 @@ export class WorkspaceGitOperationScheduler { waiters: new Map(), joinCount: 0, }; - this.inFlight.set(key, scan); + this.inFlight.set(inFlightKey, scan); this.queue.push(scan); const promise = this.addWaiter(scan, input.signal, false); this.drain();