diff --git a/doc/execution-semantics.md b/doc/execution-semantics.md index 01c64e634f1..94a47a39638 100644 --- a/doc/execution-semantics.md +++ b/doc/execution-semantics.md @@ -657,6 +657,13 @@ qualify as capacity failures. ### Workspace scan failures before provider startup +Workspace source preparation keeps its dedicated advisory-lock connection active +while authorization and filesystem work use other connections. Keepalive queries +follow the configured idle-transaction timeout without changing that setting. +Preparation retains source exclusion until its work and any in-flight keepalive +query finish. Ordinary pool reads remain available, including with a one-slot pool. +This does not authorize replay after a connection failure or bypass admission gates. + Repository discovery distinguishes an ordinary folder from a failed Git read. A timeout, full scan queue, cancellation, output limit, or Git failure must keep its typed cause through workspace preparation and run persistence. It must not diff --git a/maps/agent-access-map/workspaces.md b/maps/agent-access-map/workspaces.md index 7eeb62a1e3a..d3eb13d3ef4 100644 --- a/maps/agent-access-map/workspaces.md +++ b/maps/agent-access-map/workspaces.md @@ -135,6 +135,13 @@ a running service. ## `provisioning` +Source preparation keeps its dedicated lock connection active during work on +other connections. It preserves company/source exclusion and existing admission +authority; it adds no agent API or permission. The +[source-lock regression](../../server/src/services/isolated-remote-workspace-source-lock.test.ts) +covers PostgreSQL idle timeout, competing preparation, ordinary pool availability, +and failure cleanup. Deployed agent recovery requires separate live evidence. + Review: `baseline`. API: `present` — representative operation IDs: diff --git a/maps/feature-map/workspaces.md b/maps/feature-map/workspaces.md index 5380d9af9a4..95234e045e5 100644 --- a/maps/feature-map/workspaces.md +++ b/maps/feature-map/workspaces.md @@ -55,6 +55,12 @@ Automated: [workspace card](../../ui/src/components/IssueWorkspaceCard.test.tsx) Manual: Run a task that writes a small file, follow the bound workspace, and inspect the file and revision. Reload and verify links still refer to the same workspace, not another checkout or a stale run. +For slow source preparation, configure a short PostgreSQL idle-transaction +timeout in a disposable instance. Hold preparation beyond that interval and +verify it retains its source lock, rejects competing preparation, and leaves +ordinary reads available. The [source-lock regression](../../server/src/services/isolated-remote-workspace-source-lock.test.ts) +uses real PostgreSQL; it does not prove provider startup or deployed recovery. + For fresh standard-trust tasks using an implicit repository default, create a disposable project checkout whose shared Git storage overlaps a private task workspace. Verify that source admission creates a separate checkout from the diff --git a/server/src/services/isolated-remote-workspace-source-lock.test.ts b/server/src/services/isolated-remote-workspace-source-lock.test.ts new file mode 100644 index 00000000000..36f8d460cb1 --- /dev/null +++ b/server/src/services/isolated-remote-workspace-source-lock.test.ts @@ -0,0 +1,72 @@ +import { randomUUID } from "node:crypto"; +import { setTimeout as delay } from "node:timers/promises"; +import { sql } from "drizzle-orm"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { createDb } from "@paperclipai/db"; +import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "../__tests__/helpers/embedded-postgres.js"; +import { IsolatedRepositorySourceBusy, withIsolatedRepositorySourceLock } from "./isolated-remote-workspace-source.js"; + +const support = await getEmbeddedPostgresTestSupport(); +const describePostgres = support.supported ? describe : describe.skip; +if (!support.supported) console.warn(`Workspace source lock tests require Postgres: ${support.reason}`); + +describePostgres("workspace source ownership during preparation", () => { + let database: Awaited>; + let db: ReturnType; + + beforeAll(async () => { + database = await startEmbeddedPostgresTestDatabase("workspace-source-lock-"); + db = createDb(database.connectionString, { maxConnections: 1 }); + const rows = await db.execute(sql`select current_user as name`); + const role = String(rows[0].name); + await db.execute(sql`alter role ${sql.identifier(role)} set idle_in_transaction_session_timeout = '1s'`); + }); + + afterAll(async () => { + try { + await db?.$client.end({ timeout: 1 }); + } finally { + await database?.cleanup(); + } + }); + + it("keeps preparation exclusive beyond the idle timeout while the ordinary pool stays usable", async () => { + const companyId = randomUUID(), issueId = randomUUID(); + let entered!: () => void, release!: () => void; + const started = new Promise(resolve => { entered = resolve; }); + const pending = new Promise(resolve => { release = resolve; }); + const preparation = withIsolatedRepositorySourceLock(db, companyId, issueId, async () => { + await db.execute(sql`select 1`); + entered(); + await pending; + return withIsolatedRepositorySourceLock(db, companyId, issueId, async () => "prepared"); + }).then(value => ({ value }), error => ({ error })); + + try { + await started; + await delay(1400); + await expect(db.execute(sql`select 1 as usable`)).resolves.toMatchObject([{ usable: 1 }]); + await expect(withIsolatedRepositorySourceLock(db, companyId, issueId, async () => "overlap", true)) + .rejects.toBeInstanceOf(IsolatedRepositorySourceBusy); + await expect(withIsolatedRepositorySourceLock(db, randomUUID(), issueId, async () => "independent", true)) + .resolves.toBe("independent"); + } finally { + release(); + await preparation; + } + expect(await preparation).toEqual({ value: "prepared" }); + await expect(withIsolatedRepositorySourceLock(db, companyId, issueId, async () => "next", true)) + .resolves.toBe("next"); + }); + + it("releases a failed preparation and preserves the original failure", async () => { + const companyId = randomUUID(), issueId = randomUUID(); + const failure = new Error("source preparation failed"); + await expect(withIsolatedRepositorySourceLock(db, companyId, issueId, async () => { + await delay(1400); + throw failure; + })).rejects.toBe(failure); + await expect(withIsolatedRepositorySourceLock(db, companyId, issueId, async () => "recovered", true)) + .resolves.toBe("recovered"); + }); +}); diff --git a/server/src/services/isolated-remote-workspace-source.ts b/server/src/services/isolated-remote-workspace-source.ts index db50622f63f..44e7c6ae7db 100644 --- a/server/src/services/isolated-remote-workspace-source.ts +++ b/server/src/services/isolated-remote-workspace-source.ts @@ -11,6 +11,7 @@ import { resolvePaperclipInstanceRoot } from "../home-paths.js"; import { materializeManagedProjectWorkspace } from "./managed-repository-checkout.js"; import { measureRunPhase } from "./execution-preparation-timing.js"; import { isRestrictedWorkspaceSourceDenial } from "./workspace-source-restriction.js"; +import { withWorkspaceLockKeepalive } from "./native-runtime/native-workspace-lock-keepalive.js"; const execFile = promisify(execFileCallback); @@ -24,11 +25,14 @@ export async function withIsolatedRepositorySourceLock(db: Db, companyId: str const held = sourceLocks.getStore() ?? []; if (held.some(lock => lock.db === db && lock.key === key)) return action(); return withDedicatedDbConnection(db, connection => connection.transaction(async tx => { - if (tryOnly) { - const rows = await tx.execute<{ acquired: boolean }>(sql`select pg_try_advisory_xact_lock(hashtextextended(${key}, 0)) as acquired`); - if (!rows[0]?.acquired) throw new IsolatedRepositorySourceBusy(); - } else await tx.execute(sql`select pg_advisory_xact_lock(hashtextextended(${key}, 0))`); - return sourceLocks.run([...held, { db, key }], action); + const acquire = tryOnly + ? sql`pg_try_advisory_xact_lock(hashtextextended(${key}, 0))` + : sql`pg_advisory_xact_lock(hashtextextended(${key}, 0))`; + const rows = await tx.execute(sql`select ${acquire} as acquired, + (select setting::bigint from pg_settings where name = 'idle_in_transaction_session_timeout') as idle_timeout_ms`); + if (tryOnly && !rows[0]?.acquired) throw new IsolatedRepositorySourceBusy(); + return withWorkspaceLockKeepalive(tx, Number(rows[0]?.idle_timeout_ms), + () => sourceLocks.run([...held, { db, key }], action)); })); }