fix(workspaces): read committed files before live allocations (#18677)

## Thinking Path

> - Paperclip keeps task files in managed workspaces.
> - A final remote save commits a durable snapshot and clears writer
authority.
> - The old allocation can remain recorded while its transport becomes
unavailable.
> - Files still tried that live allocation before the accepted saved
version.
> - This PR reads the validated save when authority is clean and no
writer owns it.
> - Dirty work and active writers still require the live allocation.

## Linked Issues or Issue Description

Related: #18403 introduced remote Files access. #18665 fixes a separate
completion-settlement issue and is included in the base.

**What happened?**

A file preview returned `422 workspace_unavailable` after a remote task
had saved its files. Its clean authority still had a live location. The
same original bytes were readable from the committed save after that
location cleared. The exact original live transport failure is unknown.

**Expected behavior**

Once the workspace is clean and has no writer, Files reads its exact
current committed save. A retained location must not make accepted files
depend on remote command execution.

**Steps to reproduce**

Create a real committed workspace archive. Keep its allocation active in
the recorded authority. Clear dirty state and writer authority. Make
live execution unavailable, then request the ordinary file preview
route. Before this fix it returns 422. After this fix it returns the
saved bytes without a provider command.

## What Changed

- Select the saved-file loader before consulting a live location when
authority is clean and has no writer.
- Keep exact company, workspace, snapshot, archive, and authority-change
checks.
- Keep the live path for dirty work and active writers. Never substitute
their older saved bytes.
- Cover browse, resolve, preview, download, rejection controls, and
saved-copy cleanup through real routes and storage.
- Document the selection rule and update its feature and access-map
recipes.

## Verification

- Red: the new ordinary-route regression expected HTTP 200 but received
422 against the old service.
- A separate compatibility control reproduced the optional-loader
regression. Valid live reads still work when the loader is absent;
no-location reads still fail, and a configured invalid save never falls
back to live bytes.
- Green: 98 ordinary file-route and authorization tests passed with real
PostgreSQL and archive storage.
- All 46 opt-in remote workspace lifecycle tests passed against one
owned local disposable PostgreSQL database. Its database, server, and
files were cleaned after the run. No live S3 or provider flags were
enabled.
- Full workspace typecheck passed after the optional-loader
compatibility repair. Full build passed locally before that narrow
repair. All normal CI checks pass on the current head, including build
and server suites. The separate Paperclip Review job and the full local
test suite remain in progress.
- Feature/access-map validation and `git diff --check` passed.
- A read-only staging check preserved and verified the original saved
bytes. It did not rerun the agent or qualify the failed journey. The
fixed active-location path has local regression evidence; deployed
acceptance is pending.

## Risks

A clean workspace now uses the existing bounded saved-archive loader
while a location remains recorded. Each Files request can retrieve and
extract that archive. Existing disk, size, concurrency, cancellation,
and cleanup bounds remain. A dirty workspace or current writer still
uses live files and fails closed when they are unavailable. This change
neither releases compute nor changes access permissions. It does not
identify the original provider RPC subreason.

## Model Used

OpenAI GPT-6 through Codex with reasoning, code execution, and browser
tools. The runtime does not expose a more specific model identifier or
context-window size.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
DottaandPaperclip authored and GitHub committed 2026-11-02 16:00:04 -06:00
1 parent 5d40e75253
commit 919ddbf46d
5 files changed
+135 -7

No files matched your search

+9
View File
@@ -1728,6 +1728,15 @@ host, sandbox, or Git remote. Files changed since the last save still depend on
the live sandbox: losing that allocation can lose those edits. There is no
periodic workspace backup in this release.
Files reads use the current committed saved version when remote authority is
clean and has no active writer, even while its old allocation remains recorded.
The saved loader verifies the company, workspace, snapshot, and archive before
reading. Dirty or writer-owned authority uses the live allocation and cannot
substitute an older save if that allocation is unavailable. Reads retain their
authority-change fence and remove private saved-file copies after use.
Embedders that omit the optional saved-file loader keep valid live reads. They
still report unavailability when neither a live location nor a loader exists.
Existing local task workspaces require an explicit import. Run from a source
checkout with the deployment's database, S3, and key configuration loaded:
+3
View File
@@ -296,6 +296,9 @@ workspace metadata, and operation logs retain their app-data privacy.
For enrolled remote workspaces the same board file-resource endpoints read the
authorized live allocation or a verified committed saved version. Returned
`source.kind` is `live` or `saved`, with `savedAt` for a saved copy. An unavailable
clean allocation does not prevent reads of its current committed save once
writer authority is clear; saved reads validate the exact company, workspace,
and snapshot even while a remote location remains recorded. An unavailable
dirty allocation cannot silently fall back to stale saved bytes. Remote paths
never become controller filesystem authority. Confined path/symlink checks,
bounded search/preview, download streaming, and cancellation remain enforced by
+4
View File
@@ -102,6 +102,10 @@ Their filesystem transport is local test execution, not live sandbox/provider pr
The Files panel must label **Live workspace** or **Saved copy** with its save
time. Confirm browse, preview, and download agree. A missing allocation with
unsaved work must report unavailability rather than show an old save as current.
After a final save clears dirty state and writer authority, Files must read the
current committed saved version even while the old allocation is still recorded
or its read transport is unavailable. Confirm the original bytes and Saved copy
label; a new writer or dirty state must retain the live-read requirement.
Close must retain dirty/unfinished work and reject custom cleanup/teardown
commands; a clean saved workspace can release compute while retaining files.
Manual runtime commands, jobs, repairs, and service starts must reject remote
@@ -152,15 +152,20 @@ describeDatabase("remote workspace ordinary file routes", () => {
});
const runtime = { execute, syncOut } as unknown as Pick<EnvironmentRuntimeService, "execute" | "syncOut">;
const remoteFiles = createRemoteWorkspaceFiles(db, { environmentRuntime: runtime });
const app = filesApp(ids.company, remoteFiles);
return { root, ids, authority, execute, syncOut, remoteFiles, app, beforeCopy: (callback: () => Promise<void>) => { beforeCopy = callback; } };
}
function filesApp(companyId: string, remoteFiles: ReturnType<typeof createRemoteWorkspaceFiles>) {
const app = express();
app.use(express.json());
app.use((req, _res, next) => {
req.actor = { type: "board", userId: "local-board", companyIds: [ids.company], source: "local_implicit", isInstanceAdmin: true } as Request["actor"];
req.actor = { type: "board", userId: "local-board", companyIds: [companyId], source: "local_implicit", isInstanceAdmin: true } as Request["actor"];
next();
});
app.use("/api", fileResourceRoutes(db, { remoteFiles }));
app.use(errorHandler);
return { root, ids, authority, execute, syncOut, remoteFiles, app, beforeCopy: (callback: () => Promise<void>) => { beforeCopy = callback; } };
return app;
}
it("lists, resolves, previews, and downloads live bytes without a controller checkout", async () => {
@@ -323,7 +328,7 @@ describeDatabase("remote workspace ordinary file routes", () => {
{ workspace: "execution", path: "docs/result.md" })).rejects.toMatchObject({ status: 422, details: { code: "file_changed" } });
});
async function savedFileFixture() {
async function savedFileFixture(keepLocation = false) {
const f = await fixture(), storageRoot = await fs.mkdtemp(path.join(os.tmpdir(), "saved-file-budget-"));
directories.push(storageRoot);
const storage = createStorageService(createLocalDiskStorageProvider(path.join(storageRoot, "objects")));
@@ -335,11 +340,113 @@ describeDatabase("remote workspace ordinary file routes", () => {
const saved = await uploadWorkspaceSnapshot({ db, storage, companyId: f.ids.company, workspaceId: f.ids.workspace,
snapshotId: snapshot.id, archivePath, manifestDigest: archive.manifestDigest });
await db.update(executionWorkspaceSnapshots).set({ status: "committed" }).where(eq(executionWorkspaceSnapshots.id, snapshot.id));
await db.update(executionWorkspaces).set({ metadata: { remoteWorkspace: { ...f.authority, location: null, snapshotId: snapshot.id } } })
await db.update(executionWorkspaces).set({ metadata: { remoteWorkspace: { ...f.authority, location: keepLocation ? f.authority.location : null, snapshotId: snapshot.id } } })
.where(eq(executionWorkspaces.id, f.ids.workspace));
return { ...f, storage, saved, input: { companyId: f.ids.company, workspaceId: f.ids.workspace, snapshotId: snapshot.id } };
}
it("serves committed saved bytes through ordinary routes while the live allocation is unavailable", async () => {
const f = await savedFileFixture(true);
f.execute.mockRejectedValue(new Error("synthetic live allocation unavailable"));
const load = createSavedWorkspaceLoader(db, f.storage), closed: Promise<void>[] = [];
const loadSavedWorkspace = vi.fn(async (input: Parameters<typeof load>[0]) => {
const saved = await load(input);
let finish!: () => void, fail!: (error: unknown) => void;
closed.push(new Promise<void>((resolve, reject) => { finish = resolve; fail = reject; }));
return { ...saved, cleanup: async () => {
try { await saved.cleanup(); finish(); } catch (error) { fail(error); throw error; }
} };
});
const remoteFiles = createRemoteWorkspaceFiles(db, { loadSavedWorkspace, environmentRuntime: { execute: f.execute, syncOut: f.syncOut } });
const app = filesApp(f.ids.company, remoteFiles), base = `/api/issues/${f.ids.issue}/file-resources`;
const preview = await request(app).get(`${base}/content`).query({ workspace: "execution", path: "docs/result.md" }).expect(200);
expect(preview.body).toMatchObject({ content: { data: "Remote useful result\n" }, resource: { source: { kind: "saved", snapshotId: f.saved.id } } });
const listed = await request(app).get(`${base}/list`).query({ workspace: "execution", path: "docs" }).expect(200);
expect(listed.body).toMatchObject({ workspace: { source: { kind: "saved", snapshotId: f.saved.id } } });
const resolved = await request(app).get(`${base}/resolve`).query({ workspace: "execution", path: "docs/result.md" }).expect(200);
expect(resolved.body).toMatchObject({ source: { kind: "saved", snapshotId: f.saved.id }, capabilities: { preview: true, download: true } });
const download = await request(app).get(`${base}/content`).query({ workspace: "execution", path: "docs/result.md", download: true }).expect(200);
expect(download.text).toBe("Remote useful result\n");
expect(f.execute).not.toHaveBeenCalled(); expect(f.syncOut).not.toHaveBeenCalled();
expect(loadSavedWorkspace).toHaveBeenCalledTimes(4);
// A streamed response can finish at the client before its server finally block.
await Promise.all(closed);
for (const loaded of loadSavedWorkspace.mock.results) await expect(fs.stat((await loaded.value).root)).rejects.toMatchObject({ code: "ENOENT" });
const [workspace] = await db.select().from(executionWorkspaces).where(eq(executionWorkspaces.id, f.ids.workspace));
expect(workspace!.metadata?.remoteWorkspace).toMatchObject({ location: f.authority.location, dirty: false, writerRunId: null });
});
it.each(["dirty", "writer"] as const)("keeps %s work on the live path and never substitutes its older save", async state => {
const f = await savedFileFixture(true);
let writerRunId: string | null = null;
if (state === "writer") {
const [agent] = await db.insert(agents).values({ companyId: f.ids.company, name: "Current workspace writer" }).returning();
const [run] = await db.insert(heartbeatRuns).values({ companyId: f.ids.company, agentId: agent!.id, status: "running" }).returning();
writerRunId = run!.id;
}
await db.update(executionWorkspaces).set({ metadata: { remoteWorkspace: { ...f.authority, snapshotId: f.saved.id, dirty: state === "dirty", writerRunId } } })
.where(eq(executionWorkspaces.id, f.ids.workspace));
await fs.writeFile(path.join(f.root, "docs", "result.md"), "Unsaved live result\n");
const loadSavedWorkspace = vi.fn(createSavedWorkspaceLoader(db, f.storage));
const remoteFiles = createRemoteWorkspaceFiles(db, { loadSavedWorkspace, environmentRuntime: { execute: f.execute, syncOut: f.syncOut } });
const app = filesApp(f.ids.company, remoteFiles), route = `/api/issues/${f.ids.issue}/file-resources/content`;
const current = await request(app).get(route).query({ workspace: "execution", path: "docs/result.md" }).expect(200);
expect(current.body).toMatchObject({ resource: { source: { kind: "live" } }, content: { data: "Unsaved live result\n" } });
f.execute.mockRejectedValueOnce(new Error("synthetic live allocation unavailable"));
const unavailable = await request(app).get(route).query({ workspace: "execution", path: "docs/result.md" }).expect(422);
expect(unavailable.body).not.toHaveProperty("content"); expect(loadSavedWorkspace).not.toHaveBeenCalled();
});
it.each([true, false])("preserves the optional saved-loader boundary with a recorded location=%s", async keepLocation => {
const f = await savedFileFixture(keepLocation);
const remoteFiles = createRemoteWorkspaceFiles(db, { environmentRuntime: { execute: f.execute, syncOut: f.syncOut } });
const response = await request(filesApp(f.ids.company, remoteFiles)).get(`/api/issues/${f.ids.issue}/file-resources/content`)
.query({ workspace: "execution", path: "docs/result.md" }).expect(keepLocation ? 200 : 422);
if (keepLocation) {
expect(response.body).toMatchObject({ resource: { source: { kind: "live" } }, content: { data: "Remote useful result\n" } });
expect(f.execute).toHaveBeenCalled();
} else {
expect(response.body).not.toHaveProperty("content"); expect(f.execute).not.toHaveBeenCalled();
}
});
it.each(["company", "workspace", "uncommitted"] as const)("rejects a saved snapshot with an invalid %s binding before object retrieval", async invalid => {
const f = await savedFileFixture(true);
if (invalid === "company") {
const [other] = await db.insert(companies).values({ name: "Other saved-file company" }).returning();
await db.update(executionWorkspaceSnapshots).set({ companyId: other!.id }).where(eq(executionWorkspaceSnapshots.id, f.saved.id));
} else if (invalid === "workspace") {
const [other] = await db.insert(executionWorkspaces).values({ companyId: f.ids.company, mode: "isolated_workspace",
strategyType: "project_primary", providerType: "cloud_sandbox", name: "Other saved-file workspace" }).returning();
await db.update(executionWorkspaceSnapshots).set({ executionWorkspaceId: other!.id }).where(eq(executionWorkspaceSnapshots.id, f.saved.id));
} else await db.update(executionWorkspaceSnapshots).set({ status: "uploaded" }).where(eq(executionWorkspaceSnapshots.id, f.saved.id));
const getObject = vi.spyOn(f.storage, "getObject");
const remoteFiles = createRemoteWorkspaceFiles(db, { loadSavedWorkspace: createSavedWorkspaceLoader(db, f.storage),
environmentRuntime: { execute: f.execute, syncOut: f.syncOut } });
const response = await request(filesApp(f.ids.company, remoteFiles)).get(`/api/issues/${f.ids.issue}/file-resources/content`)
.query({ workspace: "execution", path: "docs/result.md" });
expect(response.status).toBeGreaterThanOrEqual(400); expect(response.body).not.toHaveProperty("content");
expect(getObject).not.toHaveBeenCalled(); expect(f.execute).not.toHaveBeenCalled(); expect(f.syncOut).not.toHaveBeenCalled();
getObject.mockRestore();
});
it("fences a saved read if a new writer claims the workspace during extraction and removes its scratch files", async () => {
const f = await savedFileFixture(true), load = createSavedWorkspaceLoader(db, f.storage);
let savedRoot: string | undefined;
const loadSavedWorkspace = async (input: Parameters<typeof load>[0]) => {
const saved = await load(input); savedRoot = saved.root;
await db.update(executionWorkspaces).set({ metadata: { remoteWorkspace: { ...f.authority, snapshotId: f.saved.id, dirty: true, epoch: 2 } } })
.where(eq(executionWorkspaces.id, f.ids.workspace));
return saved;
};
const remoteFiles = createRemoteWorkspaceFiles(db, { loadSavedWorkspace, environmentRuntime: { execute: f.execute, syncOut: f.syncOut } });
const response = await request(filesApp(f.ids.company, remoteFiles)).get(`/api/issues/${f.ids.issue}/file-resources/content`)
.query({ workspace: "execution", path: "docs/result.md" }).expect(422);
expect(response.body.details.code).toBe("workspace_changed"); expect(response.body).not.toHaveProperty("content");
expect(savedRoot).toBeDefined(); await expect(fs.stat(savedRoot!)).rejects.toMatchObject({ code: "ENOENT" });
expect(f.execute).not.toHaveBeenCalled(); expect(f.syncOut).not.toHaveBeenCalled();
});
it("shares fail-fast saved Files admission across loader instances until owned cleanup completes", async () => {
const f = await savedFileFixture(), availableBytes = async () => 8n * 1024n ** 3n;
const first = createSavedWorkspaceLoader(db, f.storage, { availableBytes });
@@ -625,7 +732,7 @@ describeDatabase("remote workspace ordinary file routes", () => {
});
it.each(["confirmed_stop", "unknown_stop", "dirty", "foreign_receipt"])(
"uses saved bytes only for a clean workspace with an exact stopped receipt (%s)", async state => {
"uses clean saved authority independently of an allocation's stop receipt (%s)", async state => {
const f = await fixture();
const snapshotId = randomUUID();
const agentId = randomUUID();
@@ -650,7 +757,7 @@ describeDatabase("remote workspace ordinary file routes", () => {
const remoteFiles = createRemoteWorkspaceFiles(db, { loadSavedWorkspace });
const content = workspaceFileResourceService(db, undefined, { remoteFiles }).readContent(f.ids.issue,
{ workspace: "execution", path: "docs/result.md" });
if (state === "confirmed_stop") {
if (state !== "dirty") {
await expect(content).resolves.toMatchObject({ content: { data: "Saved stopped result\n" },
resource: { source: { kind: "saved", snapshotId, savedAt: "2026-10-30T12:00:00.000Z" } } });
expect(loadSavedWorkspace).toHaveBeenCalledWith({ companyId: f.ids.company, workspaceId: f.ids.workspace, snapshotId });
@@ -64,6 +64,7 @@ export interface SavedWorkspaceFiles {
export interface RemoteWorkspaceFileOptions {
environmentRuntime?: Pick<EnvironmentRuntimeService, "execute" | "syncOut">;
/** Verify the exact company-owned, committed snapshot before exposing files. */
loadSavedWorkspace?(input: {
companyId: string;
workspaceId: string;
@@ -290,7 +291,11 @@ export function createRemoteWorkspaceFiles(db: Db, options: RemoteWorkspaceFileO
...(input.signal ? { signal: input.signal } : {}) });
return { root: saved.root, source: { kind: "saved", savedAt: saved.savedAt, snapshotId: authority.snapshotId } as WorkspaceFileSource };
};
if (authority.location) {
// A final save clears dirty state and writer authority. Its accepted
// bytes do not depend on an allocation that can already be closing.
if (authority.snapshotId && !authority.dirty && !authority.writerRunId && options.loadSavedWorkspace) {
({ root, source } = await loadSaved());
} else if (authority.location) {
const [leaseRow] = await db.select().from(environmentLeases).where(and(
eq(environmentLeases.companyId, input.companyId), eq(environmentLeases.id, authority.location.leaseId),
)).limit(1);