diff --git a/doc/workspace-restore-locks.md b/doc/workspace-restore-locks.md index 7eea72328dc..fb599ae9094 100644 --- a/doc/workspace-restore-locks.md +++ b/doc/workspace-restore-locks.md @@ -22,6 +22,13 @@ bounded timeout diagnostics. Missing, stale, or incorrect metadata cannot grant or retain ownership. PID reuse, PID namespaces, and wall-clock changes do not decide whether a writer owns the lock. +An unchanged returned snapshot still acquires the target writer lock and runs +both authorization/publication hooks. If every returned path, type, file hash, +file mode and symlink target matches the baseline, there is no incoming delta. +The merge then skips the host snapshot scan and leaves concurrent host edits +untouched. Changed exports retain the full conflict preflight. Checkpoint and +archive validation still run before the merge. + **Never delete, replace, or move a `.lock.sqlite` file while an instance can write to it.** Its stable inode is part of the locking contract. Replacing it could let two writers lock different files for the same target. Files remain diff --git a/maps/agent-access-map/workspaces.md b/maps/agent-access-map/workspaces.md index ec528d2febd..afcfaf3f9ca 100644 --- a/maps/agent-access-map/workspaces.md +++ b/maps/agent-access-map/workspaces.md @@ -75,6 +75,12 @@ then primary source, creation time and ID. A missing or foreign configured sourc does not authorize controller hooks. This precedence neither changes existing creation validation nor rotates bindings or clears retained copy-only markers. Closed-worktree reopening checks current task, agent, organizational and source policies without assuming a future driver. Effective low-trust reopens suppress configured and built-in host scripts. Returned checkpoints must have directory parents for every nested entry before merging, including unchanged entries; invalid output leaves retained host files intact and keeps recovery evidence. + +Skipping the host scan for a fully unchanged returned snapshot grants no new +access. The merge still holds its canonical writer lock and runs both authority +and publication hooks. Denials still propagate, and checkpoint/archive validation +is unchanged. [Regression coverage](../../packages/adapter-utils/src/workspace-restore-unchanged.test.ts) +is source-level evidence only. Child creation by an admitted agent checks an explicit prospective-task context before the child is saved. The creator run must have an authoritative saved policy and a verified company/agent/parent lineage. Current parent, prospective child and physical source policies must fit that boundary. This creation path does not relax exact task/run checks for existing-task writes. The [child trust regression](../../server/src/__tests__/task-workspace-child-trust.test.ts) is automated source-level evidence, not live agent qualification. Source review for these additions: PR #15749 and [task workspace routes](../../server/src/routes/task-workspaces.ts), diff --git a/maps/feature-map/workspaces.md b/maps/feature-map/workspaces.md index d3e8f2b70c4..39a79412f21 100644 --- a/maps/feature-map/workspaces.md +++ b/maps/feature-map/workspaces.md @@ -90,6 +90,12 @@ SSH keeps its explicit remote provision-command path; these checks do not claim a new sandbox provision transport. Low-trust closed-worktree reopening, including a board-comment reopen of a projectless task, must also suppress both kinds of host provision script. Returned checkpoints must reject file, symlink or missing parents before host writes, even if a nested file is marked unchanged. The retained host file and recovery evidence must survive rejection. These are automated safety contracts, not new live runtime acceptance. +An unchanged returned snapshot skips the host scan after acquiring the writer lock +and checking authority. Concurrent host edits remain untouched; changed files, +modes, symlinks and deletions still take the conflict path. The +[unchanged restore regression](../../packages/adapter-utils/src/workspace-restore-unchanged.test.ts) +checks both hooks under the lock. This is source-level coverage, not a cloud latency result. + For a child created by an admitted agent with an explicit existing workspace, verify that a matching sandbox boundary permits creation before the child has an ID. A changed child policy, unrelated parent, or forged creator run must fail. See the [child workspace trust regression](../../server/src/__tests__/task-workspace-child-trust.test.ts). This automated contract does not add live acceptance evidence. ### `workspace-management` diff --git a/packages/adapter-utils/src/workspace-restore-merge.ts b/packages/adapter-utils/src/workspace-restore-merge.ts index 98aaeac564f..85fd25155c4 100644 --- a/packages/adapter-utils/src/workspace-restore-merge.ts +++ b/packages/adapter-utils/src/workspace-restore-merge.ts @@ -218,6 +218,14 @@ function entriesMatch(left: SnapshotEntry | null | undefined, right: SnapshotEnt return false; } +function directorySnapshotsMatch(left: DirectorySnapshot, right: DirectorySnapshot): boolean { + if (left.entries.size !== right.entries.size) return false; + for (const [relative, entry] of right.entries) { + if (!entriesMatch(left.entries.get(relative), entry)) return false; + } + return true; +} + const LOCK_WAIT_MS = 30_000; const LOCK_DIAGNOSTIC_READ_TIMEOUT_MS = 100; const activeDirectoryMergeLocks = new Set(); @@ -719,6 +727,10 @@ export async function mergeDirectoryWithBaseline(input: { try { await withDirectoryMergeLock(input.targetDir, async (canonicalTargetDir) => { await input.beforeApply?.(); + if (!input.snapshots?.current && directorySnapshotsMatch(input.baseline, source)) { + await input.afterApply?.(); + return; + } // Strict preflight must see excluded children before a directory is // replaced. The merge still applies only the filtered source/baseline. const current = input.snapshots?.current ?? await captureDirectorySnapshot(canonicalTargetDir, diff --git a/packages/adapter-utils/src/workspace-restore-unchanged.test.ts b/packages/adapter-utils/src/workspace-restore-unchanged.test.ts new file mode 100644 index 00000000000..df6b5da84d8 --- /dev/null +++ b/packages/adapter-utils/src/workspace-restore-unchanged.test.ts @@ -0,0 +1,119 @@ +import { promises as fs } from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, expect, it, vi } from "vitest"; +import { captureDirectorySnapshot, disposeDirectorySnapshot, mergeDirectoryWithBaseline, withDirectoryMergeLock, WORKSPACE_RESTORE_LOCK_TIMEOUT_CODE } from "./workspace-restore-merge.js"; + +const roots: string[] = []; +afterEach(async () => { + vi.restoreAllMocks(); + vi.unstubAllEnvs(); + for (const root of roots.splice(0)) await fs.rm(root, { recursive: true, force: true }); +}); + +async function fixture(diskBacked: boolean) { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-unchanged-restore-")); + roots.push(root); + vi.stubEnv("PAPERCLIP_HOME", path.join(root, "home")); + const target = path.join(root, "target"), source = path.join(root, "source"); + await fs.mkdir(target); await fs.mkdir(source); + await fs.writeFile(path.join(target, "file"), "baseline"); + await fs.writeFile(path.join(source, "file"), "baseline"); + const baseline = await captureDirectorySnapshot(target, { diskBacked, exclude: ["cache"] }); + const incoming = await captureDirectorySnapshot(source, { diskBacked, exclude: ["cache"] }); + return { root, target, source, baseline, incoming }; +} + +it.each([ + { diskBacked: false, conflictPolicy: undefined }, { diskBacked: true, conflictPolicy: undefined }, + { diskBacked: false, conflictPolicy: "reject" as const }, { diskBacked: true, conflictPolicy: "reject" as const }, +])("does not scan host files for an unchanged $diskBacked snapshot with $conflictPolicy and preserves both fenced hooks", async ({ diskBacked, conflictPolicy }) => { + const f = await fixture(diskBacked); + await fs.writeFile(path.join(f.target, "file"), "concurrent host edit"); + await fs.mkdir(path.join(f.target, "cache")); + await fs.writeFile(path.join(f.target, "cache", "private"), "host only"); + const read = vi.spyOn(fs, "opendir"); + read.mockImplementation(async () => { throw new Error("Unexpected host scan"); }); + const callbacks: string[] = []; + const hook = async (name: string) => { + callbacks.push(name); + await expect(withDirectoryMergeLock(f.target, async () => { throw new Error("Lock was released"); }, undefined, undefined, 20)) + .rejects.toMatchObject({ code: WORKSPACE_RESTORE_LOCK_TIMEOUT_CODE }); + }; + try { + await mergeDirectoryWithBaseline({ baseline: f.baseline, sourceDir: f.source, targetDir: f.target, + snapshots: { source: f.incoming }, conflictPolicy, beforeApply: () => hook("before"), afterApply: () => hook("after") }); + expect(read).not.toHaveBeenCalled(); + expect(callbacks).toEqual(["before", "after"]); + expect(await fs.readFile(path.join(f.target, "file"), "utf8")).toBe("concurrent host edit"); + expect(await fs.readFile(path.join(f.target, "cache", "private"), "utf8")).toBe("host only"); + } finally { await disposeDirectorySnapshot(f.incoming); await disposeDirectorySnapshot(f.baseline); } +}); + +it("skips the target scan when a captured full export is unchanged", async () => { + const f = await fixture(false); + const read = vi.spyOn(fs, "opendir"); + await mergeDirectoryWithBaseline({ baseline: f.baseline, sourceDir: f.source, targetDir: f.target, conflictPolicy: "reject" }); + expect(read.mock.calls.map(call => path.resolve(String(call[0])))).toEqual([path.resolve(f.source)]); +}); + +it.each(["before", "after"])("propagates %s-hook denial for an unchanged checkpoint", async denied => { + const f = await fixture(true); + const error = new Error("Authority revoked"); + const before = vi.fn(async () => { if (denied === "before") throw error; }); + const after = vi.fn(async () => { if (denied === "after") throw error; }); + try { + await expect(mergeDirectoryWithBaseline({ baseline: f.baseline, sourceDir: f.source, targetDir: f.target, + snapshots: { source: f.incoming }, beforeApply: before, afterApply: after })).rejects.toBe(error); + expect(before).toHaveBeenCalledOnce(); + expect(after).toHaveBeenCalledTimes(denied === "after" ? 1 : 0); + expect(await fs.readFile(path.join(f.target, "file"), "utf8")).toBe("baseline"); + await withDirectoryMergeLock(f.target, async () => undefined); + } finally { await disposeDirectorySnapshot(f.incoming); await disposeDirectorySnapshot(f.baseline); } +}); + +async function assertIncomingChange(change: string) { + const f = await fixture(true); + await disposeDirectorySnapshot(f.incoming); + if (change === "hash") await fs.writeFile(path.join(f.source, "file"), "changed"); + if (change === "mode") await fs.chmod(path.join(f.source, "file"), 0o755); + if (change === "kind") { await fs.rm(path.join(f.source, "file")); await fs.mkdir(path.join(f.source, "file")); } + if (change === "rename") await fs.rename(path.join(f.source, "file"), path.join(f.source, "renamed")); + if (change === "add") await fs.writeFile(path.join(f.source, "added"), "new"); + if (change === "delete") await fs.rm(path.join(f.source, "file")); + const incoming = await captureDirectorySnapshot(f.source, { diskBacked: true, exclude: ["cache"] }); + const read = vi.spyOn(fs, "opendir"); + try { + await mergeDirectoryWithBaseline({ baseline: f.baseline, sourceDir: f.source, targetDir: f.target, + snapshots: { source: incoming }, conflictPolicy: "reject" }); + expect(read).toHaveBeenCalled(); + if (change === "hash") expect(await fs.readFile(path.join(f.target, "file"), "utf8")).toBe("changed"); + if (change === "mode") expect((await fs.stat(path.join(f.target, "file"))).mode & 0o777).toBe(0o755); + if (change === "kind") expect((await fs.stat(path.join(f.target, "file"))).isDirectory()).toBe(true); + if (change === "rename") expect(await fs.readFile(path.join(f.target, "renamed"), "utf8")).toBe("baseline"); + if (change === "add") expect(await fs.readFile(path.join(f.target, "added"), "utf8")).toBe("new"); + if (change === "delete") await expect(fs.stat(path.join(f.target, "file"))).rejects.toMatchObject({ code: "ENOENT" }); + } finally { await disposeDirectorySnapshot(incoming); await disposeDirectorySnapshot(f.baseline); } +} + +it.each(["hash", "kind", "rename", "add", "delete"])("does not skip an incoming %s change", assertIncomingChange); +it.skipIf(process.platform === "win32")("does not skip an incoming mode change", () => assertIncomingChange("mode")); + +it.skipIf(process.platform === "win32")("does not skip a changed symlink target", async () => { + const f = await fixture(false); + await fs.rm(path.join(f.target, "file")); await fs.rm(path.join(f.source, "file")); + await fs.symlink("old", path.join(f.target, "file")); await fs.symlink("new", path.join(f.source, "file")); + const baseline = await captureDirectorySnapshot(f.target), incoming = await captureDirectorySnapshot(f.source); + await mergeDirectoryWithBaseline({ baseline, sourceDir: f.source, targetDir: f.target, snapshots: { source: incoming }, conflictPolicy: "reject" }); + expect(await fs.readlink(path.join(f.target, "file"))).toBe("new"); +}); + +it("still rejects concurrent edits to a changed incoming file", async () => { + const f = await fixture(false); + await fs.writeFile(path.join(f.source, "file"), "remote change"); + await fs.writeFile(path.join(f.target, "file"), "host change"); + const incoming = await captureDirectorySnapshot(f.source); + await expect(mergeDirectoryWithBaseline({ baseline: f.baseline, sourceDir: f.source, targetDir: f.target, + snapshots: { source: incoming }, conflictPolicy: "reject" })).rejects.toMatchObject({ code: "DIRECTORY_MERGE_CONFLICT", paths: ["file"] }); + expect(await fs.readFile(path.join(f.target, "file"), "utf8")).toBe("host change"); +});