mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-24 18:19:05 +02:00
Preserve workspace ownership during archive relocation
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
725614f9c4
commit
ea1a20ec45
2 files changed
+62
-28
No files matched your search
@@ -185,12 +185,18 @@ const support = await getEmbeddedPostgresTestSupport();
|
||||
expect(await db.select().from(executionWorkspaceRepositoryAccess).where(eq(executionWorkspaceRepositoryAccess.companyId, f.companyId))).toEqual(beforeRead);
|
||||
});
|
||||
|
||||
it("preserves legacy private publishers before moving their workspace away from shared files", async () => {
|
||||
it.each(["update", "archive"] as const)("preserves legacy private publishers before %s moves their workspace away from shared files", async operation => {
|
||||
const f = await fixture(), peer = await standardPeer(f);
|
||||
const repository = { kind: "url" as const, url: "https://github.com/public/legacy-shared" };
|
||||
await f.service.request({ companyId: f.companyId, issueId: peer.issueId, actor: peer.actor,
|
||||
request: { repository, requestKey: randomUUID() } });
|
||||
await privateTask(f);
|
||||
const [project] = await db.insert(projects).values({ companyId: f.companyId, name: "Private legacy source", visibility: "private" }).returning();
|
||||
await db.insert(projectAccessMembers).values([
|
||||
{ companyId: f.companyId, projectId: project.id, subjectType: "agent", subjectId: f.agentId },
|
||||
{ companyId: f.companyId, projectId: project.id, subjectType: "user", subjectId: f.userId },
|
||||
]);
|
||||
await db.update(executionWorkspaces).set({ projectId: project.id, sourceIssueId: f.issueId }).where(eq(executionWorkspaces.id, f.workspace.id));
|
||||
await f.request(repository);
|
||||
await localPrivateClone(f);
|
||||
const [checkout] = await f.service.prepareForAdmission(f.admission);
|
||||
@@ -200,12 +206,24 @@ const support = await getEmbeddedPostgresTestSupport();
|
||||
await db.delete(executionWorkspaceRepositoryAccess).where(eq(executionWorkspaceRepositoryAccess.companyId, f.companyId));
|
||||
const moved = path.join(root, `${f.issueId}-legacy-moved`);
|
||||
await mkdir(moved);
|
||||
await executionWorkspaceService(db).update(f.workspace.id, { cwd: moved });
|
||||
const patch = { cwd: moved, projectId: null, sourceIssueId: null, metadata: {} };
|
||||
const svc = executionWorkspaceService(db);
|
||||
if (operation === "archive") {
|
||||
const result = await svc.archiveWorkspaceUnderLifecycleLock({ id: f.workspace.id, patch, closedAt: new Date() });
|
||||
expect(result?.outcome).toBe("archived");
|
||||
if (result?.outcome === "archived") {
|
||||
expect(result.capturedGeneration).toBeGreaterThan(0);
|
||||
expect(result.workspace.status).toBe("archived");
|
||||
expect(result.workspace.metadata?._issuePrivacySources).toHaveProperty(f.issueId);
|
||||
}
|
||||
} else await svc.update(f.workspace.id, patch);
|
||||
await db.update(issues).set({ executionWorkspaceId: null }).where(eq(issues.id, f.issueId));
|
||||
await expect(assertExecutionWorkspaceRootReadAccess(db, { companyId: f.companyId, cwd: peer.admission.cwd,
|
||||
sourceWorkspaceId: peer.workspace.id, actor: peer.actor })).rejects.toThrow(/workspace access|authorization boundary/);
|
||||
await expect(f.service.prepareForAdmission(peer.admission)).rejects.toThrow(/workspace access|authorization boundary/);
|
||||
sourceWorkspaceId: peer.workspace.id, actor: peer.actor })).rejects.toThrow(/workspace access|authorization boundary|source project/);
|
||||
await expect(f.service.prepareForAdmission(peer.admission)).rejects.toThrow(/workspace access|authorization boundary|source project/);
|
||||
const retained = await db.select().from(executionWorkspaceRepositoryAccess).where(eq(executionWorkspaceRepositoryAccess.companyId, f.companyId));
|
||||
expect(retained.some(entry => entry.kind === "source" && entry.scopeId === f.workspace.id)).toBe(true);
|
||||
expect(retained.some(entry => entry.kind === "project_source" && entry.scopeId === project.id)).toBe(true);
|
||||
expect(retained.some(entry => entry.kind === "repository" || entry.kind === "reader")).toBe(false);
|
||||
expect(await readFile(path.join(checkout.cwd, "private.txt"), "utf8")).toBe("Private task edits\n");
|
||||
});
|
||||
|
||||
@@ -234,6 +234,34 @@ async function acquireExecutionWorkspaceLifecycleLock(
|
||||
);
|
||||
}
|
||||
|
||||
const workspaceSourceKeys = ["cwd", "projectId", "projectWorkspaceId", "sourceIssueId"] as const;
|
||||
|
||||
/** Caller holds the company root fence and the current workspace row lock. */
|
||||
async function preserveWorkspaceOwnershipForPatch(
|
||||
tx: DbTransaction, current: typeof executionWorkspaces.$inferSelect, patch: Partial<typeof executionWorkspaces.$inferInsert>,
|
||||
) {
|
||||
if (workspaceSourceKeys.some(key => patch[key] !== undefined && patch[key] !== current[key])) {
|
||||
await preserveExecutionWorkspaceRootProvenance(tx, {
|
||||
companyId: current.companyId, workspaceId: current.id, cwd: current.cwd,
|
||||
});
|
||||
}
|
||||
const nextPatch = { ...patch };
|
||||
const retainedSources = {
|
||||
...parseObject(parseObject(patch.metadata)._issuePrivacySources),
|
||||
...parseObject(current.metadata?._issuePrivacySources),
|
||||
};
|
||||
if (patch.sourceIssueId !== undefined && patch.sourceIssueId !== current.sourceIssueId && current.sourceIssueId) {
|
||||
retainedSources[current.sourceIssueId] = true;
|
||||
}
|
||||
if (Object.keys(retainedSources).length > 0) {
|
||||
nextPatch.metadata = {
|
||||
...(patch.metadata !== undefined ? patch.metadata ?? {} : current.metadata ?? {}),
|
||||
_issuePrivacySources: retainedSources,
|
||||
};
|
||||
}
|
||||
return nextPatch;
|
||||
}
|
||||
|
||||
export type ReopenClosedIsolatedExecutionWorkspaceResult =
|
||||
// `generation` is the lifecycle generation the reopen published the active row
|
||||
// at. It owns the reopen-pending flag. A later clear must present this same
|
||||
@@ -3189,8 +3217,7 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic
|
||||
},
|
||||
|
||||
update: async (id: string, patch: Partial<typeof executionWorkspaces.$inferInsert>) => {
|
||||
const sourceKeys = ["cwd", "projectId", "projectWorkspaceId", "sourceIssueId"] as const;
|
||||
if (patch.metadata === undefined && !sourceKeys.some(key => patch[key] !== undefined)) {
|
||||
if (patch.metadata === undefined && !workspaceSourceKeys.some(key => patch[key] !== undefined)) {
|
||||
const row = await db.update(executionWorkspaces).set({ ...patch, updatedAt: new Date() })
|
||||
.where(eq(executionWorkspaces.id, id)).returning().then(rows => rows[0] ?? null);
|
||||
return row ? toExecutionWorkspace(row) : null;
|
||||
@@ -3206,25 +3233,7 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic
|
||||
eq(executionWorkspaces.id, id), eq(executionWorkspaces.companyId, identity.companyId),
|
||||
)).for("no key update");
|
||||
if (!current) return null;
|
||||
if (sourceKeys.some(key => patch[key] !== undefined && patch[key] !== current[key])) {
|
||||
await preserveExecutionWorkspaceRootProvenance(tx, {
|
||||
companyId: current.companyId, workspaceId: current.id, cwd: current.cwd,
|
||||
});
|
||||
}
|
||||
const nextPatch = { ...patch };
|
||||
const retainedSources = {
|
||||
...parseObject(parseObject(patch.metadata)._issuePrivacySources),
|
||||
...parseObject(current.metadata?._issuePrivacySources),
|
||||
};
|
||||
if (patch.sourceIssueId !== undefined && patch.sourceIssueId !== current.sourceIssueId && current.sourceIssueId) {
|
||||
retainedSources[current.sourceIssueId] = true;
|
||||
}
|
||||
if (Object.keys(retainedSources).length > 0) {
|
||||
nextPatch.metadata = {
|
||||
...(patch.metadata !== undefined ? patch.metadata ?? {} : current.metadata ?? {}),
|
||||
_issuePrivacySources: retainedSources,
|
||||
};
|
||||
}
|
||||
const nextPatch = await preserveWorkspaceOwnershipForPatch(tx, current, patch);
|
||||
const [row] = await tx.update(executionWorkspaces).set({ ...nextPatch, updatedAt: new Date() })
|
||||
.where(and(eq(executionWorkspaces.id, id), eq(executionWorkspaces.companyId, current.companyId))).returning();
|
||||
return row ? toExecutionWorkspace(row) : null;
|
||||
@@ -3588,12 +3597,17 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic
|
||||
| { outcome: "reopen_pending" }
|
||||
| null
|
||||
> => {
|
||||
const [identity] = await db.select({ companyId: executionWorkspaces.companyId }).from(executionWorkspaces)
|
||||
.where(eq(executionWorkspaces.id, input.id));
|
||||
if (!identity) return null;
|
||||
return db.transaction(async (tx) => {
|
||||
await lockExecutionWorkspaceRootProvenance(tx, identity.companyId);
|
||||
await acquireExecutionWorkspaceLifecycleLock(tx, input.id);
|
||||
const fresh = await tx
|
||||
.select()
|
||||
.from(executionWorkspaces)
|
||||
.where(eq(executionWorkspaces.id, input.id))
|
||||
.where(and(eq(executionWorkspaces.id, input.id), eq(executionWorkspaces.companyId, identity.companyId)))
|
||||
.for("no key update")
|
||||
.then((rows) => rows[0] ?? null);
|
||||
if (!fresh || isClosedExecutionWorkspaceStatus(fresh.status)) {
|
||||
// The row is missing or already closed by a concurrent path. Do not
|
||||
@@ -3607,8 +3621,9 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic
|
||||
// rebuilt worktree. The route maps this to HTTP 409.
|
||||
return { outcome: "reopen_pending" };
|
||||
}
|
||||
const nextPatch = await preserveWorkspaceOwnershipForPatch(tx, fresh, input.patch);
|
||||
const baseMetadata =
|
||||
(input.patch.metadata as Record<string, unknown> | null | undefined)
|
||||
(nextPatch.metadata as Record<string, unknown> | null | undefined)
|
||||
?? (fresh.metadata as Record<string, unknown> | null);
|
||||
const archiveMetadata = clearMetadataReopenPendingConsumption(
|
||||
bumpExecutionWorkspaceLifecycleGeneration(baseMetadata),
|
||||
@@ -3616,7 +3631,7 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic
|
||||
const archived = await tx
|
||||
.update(executionWorkspaces)
|
||||
.set({
|
||||
...input.patch,
|
||||
...nextPatch,
|
||||
status: "archived",
|
||||
closedAt: input.closedAt,
|
||||
cleanupReason: null,
|
||||
@@ -3625,6 +3640,7 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic
|
||||
})
|
||||
.where(and(
|
||||
eq(executionWorkspaces.id, input.id),
|
||||
eq(executionWorkspaces.companyId, fresh.companyId),
|
||||
// Defense in depth: never archive a reopen-pending row even if the
|
||||
// flag appears between the read above and this write. The lifecycle
|
||||
// lock already serializes reopen and archive, so this predicate only
|
||||
|
||||
Reference in new issue
Block a user