diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index f5b7edaa274..7ad912abaf8 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -1065,6 +1065,20 @@ Workspace preparation resolves an existing root symlink before reading the snaps Execution-workspace close-readiness and branch-reconciliation status checks allow up to 32 MiB of buffered output. This accommodates nested task worktrees while preserving exact untracked-file counts. Larger output fails the scan and blocks destructive cleanup. Other buffered scan bounds stay unchanged. +Heartbeat configuration, environment selection, and finalization read persisted +workspace records through the company-scoped `getStoredById` service method. +Those reads do not inspect Git, refresh runtime service health, or resolve PR +delivery. Workspace detail responses still load live display state, and the +execution and cleanup boundaries retain their live branch and cleanliness +guards. Close readiness derives ancestry from its existing ahead/behind +comparison, avoiding a second history walk with `merge-base`. + +Measure the avoided reads on synthetic workspaces with +`pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-workspace-record-reads.ts`. +The benchmark checks identical persisted fields and reports wall time and actual +Git command counts. Use `--resident-mib=1536` to exercise a large Node process on +Linux. It measures workspace reads, not end-to-end agent latency. + The cache intentionally trades up to a few seconds of changed-file freshness for stable server latency. The file browser retains an explicit refresh action, does not start its query while the panel or browser tab is hidden, and presents overloads as retryable failures rather than an empty workspace. A full queue returns `503` with code `workspace_git_scan_saturated`; a scan exceeding its wall-clock limit returns `504` with code `workspace_git_scan_timeout`. Both responses include `Retry-After: 1`. Sandbox Git sync treats only the selected repository root as a clone source. A selected subfolder uses directory sync within that folder, applies the enclosing repository's ignore rules, and does not transfer parent files or Git history. diff --git a/scripts/benchmark-workspace-record-reads.ts b/scripts/benchmark-workspace-record-reads.ts new file mode 100644 index 00000000000..d92711b1333 --- /dev/null +++ b/scripts/benchmark-workspace-record-reads.ts @@ -0,0 +1,81 @@ +import assert from "node:assert/strict"; +import { execFile } from "node:child_process"; +import { randomUUID } from "node:crypto"; +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { promisify } from "node:util"; +import { companies, createDb, executionWorkspaces, startEmbeddedPostgresTestDatabase } from "../packages/db/src/index.js"; +import { executionWorkspaceService } from "../server/src/services/execution-workspaces.js"; + +const run = promisify(execFile); +const root = await fs.mkdtemp(path.join(os.tmpdir(), "workspace-record-benchmark-")); +const repo = path.join(root, "repo"); +const worktree = path.join(root, "worktree"); +const residentMiB = Number(process.argv.find(arg => arg.startsWith("--resident-mib="))?.split("=")[1] ?? 0); +assert.ok(Number.isInteger(residentMiB) && residentMiB >= 0 && residentMiB <= 2048); +const previousTrace = process.env.GIT_TRACE; +const keepAlive = setInterval(() => {}, 1000); +let database: Awaited> | undefined; +let db: ReturnType | undefined; +try { + await fs.mkdir(repo); + const git = (...args: string[]) => run("git", ["-C", repo, ...args]); + await git("init", "-b", "main"); + await git("config", "user.name", "Workspace benchmark"); + await git("config", "user.email", "benchmark@paperclip.local"); + await fs.writeFile(path.join(repo, "README.md"), "Synthetic workspace\n"); + await git("add", "README.md"); + await git("commit", "-m", "Fixture"); + await git("worktree", "add", "-b", "fixture", worktree); + database = await startEmbeddedPostgresTestDatabase("workspace-record-benchmark-"); + db = createDb(database.connectionString); + const companyId = randomUUID(); + await db.insert(companies).values({ id: companyId, name: "Synthetic workspace reads", issuePrefix: "BENCH" }); + const rows = await db.insert(executionWorkspaces).values(Array.from({ length: 20 }, (_, index) => ({ + companyId, name: `Fixture ${index}`, mode: "isolated_workspace", strategyType: "git_worktree", + cwd: worktree, providerRef: worktree, providerType: "git_worktree", baseRef: "main", branchName: "fixture", + }))).returning(); + const service = executionWorkspaceService(db); + const resident = Array.from({ length: residentMiB }, () => Buffer.alloc(1024 * 1024, 1)); + const persisted = (workspace: NonNullable>>) => { + const { runtimeServices: _services, deliveryState: _delivery, ...record } = workspace; + return record; + }; + const results = []; + let expected: ReturnType[] | undefined; + await service.getStoredById(companyId, rows[0].id); + for (const mode of ["detail", "stored"] as const) { + const trace = path.join(root, `${mode}.trace`); + process.env.GIT_TRACE = trace; + const start = performance.now(); + const records = []; + for (const row of rows) { + const workspace = mode === "detail" + ? await service.getById(row.id) + : await service.getStoredById(companyId, row.id); + assert.ok(workspace); + records.push(persisted(workspace)); + } + const wallMs = performance.now() - start; + if (expected) assert.deepEqual(records, expected); + else expected = records; + const traceText = await fs.readFile(trace, "utf8").catch(error => { + if (error.code === "ENOENT") return ""; + throw error; + }); + results.push({ mode, reads: rows.length, wallMs, gitCommands: traceText.split("\n").filter(line => line.includes("built-in: git ")).length }); + } + assert.equal(results[1].gitCommands, 0); + console.log(JSON.stringify({ node: process.version, platform: process.platform, arch: process.arch, + residentMiB: resident.length, rssMiB: Math.round(process.memoryUsage().rss / 1024 / 1024), + equalPersistedRecords: true, results }, null, 2)); +} finally { + if (previousTrace === undefined) delete process.env.GIT_TRACE; + else process.env.GIT_TRACE = previousTrace; + try { + await db?.$client.end({ timeout: 0 }); + await database?.cleanup(); + await fs.rm(root, { recursive: true, force: true }); + } finally { clearInterval(keepAlive); } +} diff --git a/server/src/__tests__/execution-workspaces-service.test.ts b/server/src/__tests__/execution-workspaces-service.test.ts index 6bd89ec434f..e87418af0bb 100644 --- a/server/src/__tests__/execution-workspaces-service.test.ts +++ b/server/src/__tests__/execution-workspaces-service.test.ts @@ -855,6 +855,58 @@ describeEmbeddedPostgres("executionWorkspaceService.getCloseReadiness", () => { expect(workspace).toMatchObject({ status: "archived", cleanupReason: "issue_terminal" }); }, 20_000); + it("reads company-scoped execution configuration without live Git inspection", async () => { + const seed = await seedAncestryTerminalWorkspace(); + const service = executionWorkspaceService(db, { + inspectGitCloseReadiness: async () => { throw new Error("Git inspection unavailable"); }, + }); + const config = { provisionCommand: "echo prepare", environmentId: randomUUID() }; + await db.update(executionWorkspaces).set({ metadata: { config } }) + .where(eq(executionWorkspaces.id, seed.executionWorkspaceId)); + + expect(await service.getStoredById(seed.companyId, seed.executionWorkspaceId)).toMatchObject({ + id: seed.executionWorkspaceId, companyId: seed.companyId, + cwd: seed.worktreePath, baseRef: "main", config, + deliveryState: "unknown", runtimeServices: [], + }); + await expect(service.getStoredById(randomUUID(), seed.executionWorkspaceId)) + .rejects.toThrow("Task workspace access is no longer available"); + expect(await service.getStoredById(seed.companyId, randomUUID())).toBeNull(); + await expect(service.getById(seed.executionWorkspaceId)).rejects.toThrow("Git inspection unavailable"); + }); + + it("derives ancestry from the existing comparison and stays unknown for an invalid base", async () => { + const seed = await seedAncestryTerminalWorkspace(); + const traceDir = await fs.mkdtemp(path.join(os.tmpdir(), "workspace-git-trace-")); + tempDirs.add(traceDir); + const tracePath = path.join(traceDir, "git.trace"); + const previousTrace = process.env.GIT_TRACE; + process.env.GIT_TRACE = tracePath; + try { + const merged = await svc.getCloseReadiness(seed.executionWorkspaceId); + expect(merged?.git).toMatchObject({ aheadCount: 0, behindCount: 0, isMergedIntoBase: true }); + expect(await fs.readFile(tracePath, "utf8")).not.toContain("merge-base"); + + await fs.writeFile(path.join(seed.worktreePath, "new.txt"), "Undelivered work\n"); + await runGit(seed.worktreePath, ["add", "new.txt"]); + await runGit(seed.worktreePath, ["commit", "-m", "Undelivered work"]); + const ahead = await svc.getCloseReadiness(seed.executionWorkspaceId); + expect(ahead?.git).toMatchObject({ aheadCount: 1, behindCount: 0, isMergedIntoBase: false }); + expect(ahead?.deliveryState).toBe("unmerged"); + + await db.update(executionWorkspaces).set({ baseRef: "missing-base" }) + .where(eq(executionWorkspaces.id, seed.executionWorkspaceId)); + const unknown = await svc.getCloseReadiness(seed.executionWorkspaceId); + expect(unknown?.git).toMatchObject({ aheadCount: null, behindCount: null, isMergedIntoBase: null }); + expect(unknown?.deliveryState).toBe("unknown"); + expect(await svc.sweepTerminalWorkspaces()).toMatchObject({ archived: 0, skippedUndelivered: 1 }); + await expect(fs.access(seed.worktreePath)).resolves.toBeUndefined(); + } finally { + if (previousTrace === undefined) delete process.env.GIT_TRACE; + else process.env.GIT_TRACE = previousTrace; + } + }); + it("fails closed before archive when git status inspection is unavailable", async () => { const seeded = await seedAncestryTerminalWorkspace(); const statusSpy = vi.spyOn(workspaceGitOperationScheduler, "run") diff --git a/server/src/services/execution-workspaces.ts b/server/src/services/execution-workspaces.ts index 05e0bc77a63..6bd5d2762b0 100644 --- a/server/src/services/execution-workspaces.ts +++ b/server/src/services/execution-workspaces.ts @@ -963,27 +963,16 @@ async function inspectGitCloseReadiness(workspace: ExecutionWorkspace): Promise< if (repoRoot && baseRef) { try { const counts = (await runGit(["rev-list", "--left-right", "--count", `${baseRef}...HEAD`], workspacePath)).stdout.trim(); - const [behindRaw, aheadRaw] = counts.split(/\s+/); - behindCount = behindRaw ? Number.parseInt(behindRaw, 10) : 0; - aheadCount = aheadRaw ? Number.parseInt(aheadRaw, 10) : 0; + const match = /^(\d+)\s+(\d+)$/.exec(counts); + if (!match) throw new Error("Git returned invalid ahead/behind counts"); + behindCount = Number(match[1]); + aheadCount = Number(match[2]); + isMergedIntoBase = aheadCount === 0; } catch (error) { warnings.push( `Could not compare this workspace against ${baseRef}: ${error instanceof Error ? error.message : String(error)}`, ); } - - try { - await runGit(["merge-base", "--is-ancestor", "HEAD", baseRef], workspacePath); - isMergedIntoBase = true; - } catch (error) { - const code = typeof error === "object" && error && "code" in error ? (error as { code?: unknown }).code : null; - if (code === 1) isMergedIntoBase = false; - else { - warnings.push( - `Could not determine whether this workspace is merged into ${baseRef}: ${error instanceof Error ? error.message : String(error)}`, - ); - } - } } return { @@ -1748,7 +1737,7 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic } let mergedPullRequest = false; let pullRequestStateUnknown = false; - const workspaceHeadSha = git?.repoRoot && git.workspacePath + const workspaceHeadSha = sourceIssueTerminal && git?.repoRoot && git.workspacePath ? await runGit(["rev-parse", "HEAD"], git.workspacePath) .then((result) => result.stdout.trim() || null) .catch(() => null) @@ -2793,6 +2782,12 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic return null; }, + getStoredById: async (companyId: string, id: string) => { + const [row] = await db.select().from(executionWorkspaces).where(eq(executionWorkspaces.id, id)); + if (row && row.companyId !== companyId) throw forbidden("Task workspace access is no longer available"); + return row ? toExecutionWorkspace(row) : null; + }, + getById: async (id: string) => { const row = await db .select() diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index b4c464fe871..1d17cbc8cd8 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -2251,7 +2251,7 @@ export function heartbeatService( ); const contextProjectId = readNonEmptyString(context.projectId); const boundSourceWorkspace = issueContext?.executionWorkspaceId - ? await executionWorkspacesSvc.getById(issueContext.executionWorkspaceId) : null; + ? await executionWorkspacesSvc.getStoredById(agent.companyId, issueContext.executionWorkspaceId) : null; const [selectedWorkspaceSource] = !boundSourceWorkspace && issueContext?.projectWorkspaceId ? await db.select({ projectId: projectWorkspaces.projectId }).from(projectWorkspaces) .where(and(eq(projectWorkspaces.id, issueContext.projectWorkspaceId), eq(projectWorkspaces.companyId, agent.companyId))) : []; diff --git a/server/src/services/heartbeat/environment-selection.test.ts b/server/src/services/heartbeat/environment-selection.test.ts index 5403f8164ef..884ddf1d1dc 100644 --- a/server/src/services/heartbeat/environment-selection.test.ts +++ b/server/src/services/heartbeat/environment-selection.test.ts @@ -89,6 +89,27 @@ describe.skipIf(!support.supported)("heartbeat environment selection boundary", expect(result.persistedNativeExecutionInput?.binding.executionWorkspaceId).toBe(original.id); }); + it("selects the saved execution workspace without inspecting live Git delivery state", async () => { + const { input, workspace, recover } = await fixture(); + const saved = await workspace(); + recover(saved.id); + input.services.executionWorkspacesSvc = executionWorkspaceService(db, { + inspectGitCloseReadiness: async () => { + throw new Error("Live Git inspection is unavailable"); + }, + }); + + const result = await selectHeartbeatEnvironment(db, input); + + expect(result.reusableExistingExecutionWorkspace).toMatchObject({ + id: saved.id, + companyId: input.agent.companyId, + cwd: home, + strategyType: "git_worktree", + }); + expect(result.nativeRecoveryExecutionWorkspaceId).toBe(saved.id); + }); + it("does not treat a synthetic run-id recovery binding as a persisted workspace", async () => { const { input, recover } = await fixture(); recover(input.run.id); diff --git a/server/src/services/heartbeat/environment-selection.ts b/server/src/services/heartbeat/environment-selection.ts index f0436a60727..8a732a90dba 100644 --- a/server/src/services/heartbeat/environment-selection.ts +++ b/server/src/services/heartbeat/environment-selection.ts @@ -64,7 +64,7 @@ export interface HeartbeatEnvironmentSelectionInput { issueAssigneeOverrides: { adapterConfig: Record | null } | null; }; services: { - executionWorkspacesSvc: Pick, "getById" | "validateSelection">; + executionWorkspacesSvc: Pick, "getStoredById" | "validateSelection">; environmentsSvc: Pick, "ensureLocalEnvironment" | "findManagedSandboxEnvironment" | "findKubernetesEnvironment" | "ensureKubernetesEnvironment" | "getById">; instanceSettings: Pick, "get" | "getExperimental">; @@ -105,7 +105,7 @@ export async function selectHeartbeatEnvironment(db: Db, input: HeartbeatEnviron persistedNativeExecutionWorkspaceId ?? readNonEmptyString(issueRef?.executionWorkspaceId); const existingExecutionWorkspace = requestedExecutionWorkspaceId - ? await executionWorkspacesSvc.getById(requestedExecutionWorkspaceId) + ? await executionWorkspacesSvc.getStoredById(agent.companyId, requestedExecutionWorkspaceId) : null; const nativeRecoveryExecutionWorkspaceId = resolveNativeRecoveryExecutionWorkspaceBinding({ diff --git a/server/src/services/heartbeat/runtime-execution.ts b/server/src/services/heartbeat/runtime-execution.ts index bb5f5b99fe2..80e4bf183c5 100644 --- a/server/src/services/heartbeat/runtime-execution.ts +++ b/server/src/services/heartbeat/runtime-execution.ts @@ -175,7 +175,7 @@ export interface HeartbeatRuntimeExecutionInput { Pick & Pick, "cancelRunInternal"> & Pick, "enqueueWakeup" | "dispatchPendingNativeStatusWakeups"> & { - executionWorkspacesSvc: Pick, "getById">; + executionWorkspacesSvc: Pick, "getStoredById">; instanceSettings: Pick, "getExperimental">; envOrchestrator: Pick, "releaseForRun">; }; @@ -266,7 +266,8 @@ export async function executeHeartbeatRuntime(db: Db, input: HeartbeatRuntimeExe let adapterFinalizeOutcome: "succeeded" | "failed" | null = null; const inspectFinalizeWorkspaceBranch = async () => { const workspaceRecord = persistedExecutionWorkspace?.id - ? await executionWorkspacesSvc.getById( + ? await executionWorkspacesSvc.getStoredById( + agent.companyId, persistedExecutionWorkspace.id, ) : persistedExecutionWorkspace;