Separate plugin workspace completion from maintenance inspection (#17329)

## Thinking Path

> - Paperclip manages AI agents and their work.
> - Storage plugins own remote task workspaces.
> - Run completion currently requests the same inspection used for
storage cleanup.
> - That inspection can fetch remote refs and scan files after the agent
has finished.
> - Completion needs retained workspace identity and durable writes, not
permission to delete files.
> - This change adds an optional completion operation and keeps
maintenance inspection separate.

## Linked Issues or Issue Description

**What happened?**

A successful plugin-backed run can fail during completion because the
storage provider performs a slow remote-recoverability inspection. Dirty
or unpublished output does not need a deletion-safety proof to remain
valid task output.

**Expected behavior**

Completion verifies the assigned workspace and confirms durable writes
through the provider. Cleanup still requires the existing full
inspection.

**Steps to reproduce**

Run a task with plugin-owned storage whose inspection fetches remote
refs. Let the agent finish, then observe completion waiting for the
maintenance inspection.

Related context: #16428 added finalization observability. The duplicate
search found no existing PR for this provider contract.

## What Changed

- Add optional `workspaceProvider.finalizationVersion: 1` and strict
`finalize` / `finalized` schemas.
- Pass original run and agent context through live completion and
recovered completion.
- Give the new operation a 30-second deadline and keep ownership checks
and lifecycle serialization.
- Retain the inspection fallback for providers without the capability.
Keep maintenance inspection and removal unchanged.

## Verification

- SDK build passed.
- Targeted SDK and completion unit tests: 7 passed.
- Added integration coverage for capability selection, legacy fallback,
maintenance separation, and failure propagation.
- Local integration setup timed out starting PostgreSQL; the integration
suite passed in GitHub CI.
- Full build/typecheck and broader tests passed in GitHub CI (55
successful checks, 2 skipped). Local checks remained targeted as
requested.
- Diff reviewed for secrets and operational identifiers.

## Risks

The capability is opt-in. Providers must not advertise it until their
completion implementation is available. An unavailable provider or
failed durability barrier remains retryable. No schema or stored-binding
migration is required.

Test-file size exception: the provider integration suite grows from 973
to 1,002 lines. The 29-line case follows the same prepared binding
through legacy completion, opted-in completion, and maintenance
inspection using the existing single-connection database fixture.
Keeping it there avoids a second embedded-Postgres lifecycle for one
transition test. The completion helper unit tests are in a separate
focused file.

## Model Used

OpenAI GPT-6 Codex, with reasoning and code/test tools.

## 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
- [x] 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:
Nicky LeachandPaperclip authored and GitHub committed 2026-10-23 09:07:35 -07:00
1 parent 0fb152ee94
commit a708a13336
13 files changed
+137 -28

No files matched your search

@@ -14,6 +14,7 @@ import {
import {
preparePluginWorkspace,
inspectPluginWorkspace,
finalizePluginWorkspace,
removePluginWorkspace,
type WorkspaceWorkers,
} from "../services/plugin-workspaces.js";
@@ -42,6 +43,34 @@ const support = await getEmbeddedPostgresTestSupport();
});
const fixture = (companyId?: string) =>
createPluginWorkspaceFixture(db, companyId);
it("uses bounded finalization only for opted-in providers, preserving maintenance inspection", async () => {
const f = await fixture();
const workspace = (await preparePluginWorkspace(db, f.workers, f.input))!;
const context = { runId: f.runId, agentId: f.input.agentId };
expect((await finalizePluginWorkspace(db, f.workers, workspace, context)).kind).toBe("inspection");
const [plugin] = await db.select().from(plugins).where(eq(plugins.id, f.pluginId));
await db.update(plugins).set({ manifestJson: { ...plugin.manifestJson,
workspaceProvider: { ...plugin.manifestJson.workspaceProvider!, finalizationVersion: 1 } } }).where(eq(plugins.id, f.pluginId));
const completionIds: string[] = [];
const call = f.workers.call.bind(f.workers);
f.workers.call = async (...args) => {
const p = args[2] as any;
if (p.operation.kind !== "finalize") return call(...args);
expect(p.agentId).toBe(context.agentId);
completionIds.push(p.operationId);
expect(Date.parse(p.deadline) - Date.now()).toBeLessThanOrEqual(30_000);
expect(args[3]).toBe(40_000);
return { kind: "finalized", branchName: workspace.branchName, headSha: "a".repeat(40) };
};
expect((await finalizePluginWorkspace(db, f.workers, workspace, context)).kind).toBe("finalized");
await finalizePluginWorkspace(db, f.workers, workspace, context);
await finalizePluginWorkspace(db, f.workers, workspace, { ...context, runId: randomUUID() });
expect(completionIds[1]).toBe(completionIds[0]);
expect(completionIds[2]).not.toBe(completionIds[0]);
expect((await inspectPluginWorkspace(db, f.workers, workspace)).kind).toBe("inspection");
f.workers.call = async () => { throw new Error("sync failed"); };
await expect(finalizePluginWorkspace(db, f.workers, workspace, context)).rejects.toThrow("incomplete");
});
it("keeps the single-connection pool responsive while provider preparation holds its lifecycle lock", async () => {
const f = await fixture();
let entered!: () => void, release!: () => void;
@@ -209,7 +209,7 @@ describe.skipIf(!support.supported)("heartbeat runtime execution boundary", () =
name: "Plugin worktree", providerType: "plugin", providerRef: "provider:opaque-worktree", cwd: "/workspaces/plugin-only",
mode: "isolated_workspace", strategyType: "git_worktree", branchName: "task-branch" }).returning();
input.workspace.persistedExecutionWorkspace = workspace;
const inspect = vi.spyOn(pluginWorkspaces, "inspectPluginWorkspace").mockResolvedValue({ kind: "inspection",
const inspect = vi.spyOn(pluginWorkspaces, "finalizePluginWorkspace").mockResolvedValue({ kind: "inspection",
branchName: branch === "expected" ? "task-branch" : "other-branch", headSha: "a".repeat(40),
hasDirtyTrackedFiles: true, hasUntrackedFiles: false, hasLocalOnlyData: true, isMergedIntoBase: false, remotelyRecoverable: false });
try {
@@ -221,7 +221,7 @@ describe.skipIf(!support.supported)("heartbeat runtime execution boundary", () =
await expect(result).rejects.toThrow("plugin_workspace_branch_mismatch");
expect((await finalizations()).some(row => row.status === "succeeded")).toBe(false);
}
expect(inspect).toHaveBeenCalledWith(db, undefined, expect.objectContaining({ id: workspace.id, providerRef: workspace.providerRef }));
expect(inspect).toHaveBeenCalledWith(db, undefined, expect.objectContaining({ id: workspace.id, providerRef: workspace.providerRef }), { agentId: input.agent.id, runId: input.run.id });
} finally { inspect.mockRestore(); }
});
@@ -284,7 +284,7 @@ export async function executeHeartbeatRuntime(db: Db, input: HeartbeatRuntimeExe
)
: persistedExecutionWorkspace;
pluginWorkspaceFinalization = workspaceRecord
? await inspectPluginWorkspaceForFinalization(db, options.pluginWorkerManager, workspaceRecord, agent.id)
? await inspectPluginWorkspaceForFinalization(db, options.pluginWorkerManager, workspaceRecord, agent.id, run.id)
: null;
if (pluginWorkspaceFinalization || workspaceRecord?.strategyType !== "git_worktree") return null;
@@ -0,0 +1,20 @@
import { describe, it, expect, vi } from "vitest";
import { inspectPluginWorkspaceForFinalization } from "./native-plugin-workspace-finalization.js";
import { finalizePluginWorkspace } from "../plugin-workspaces.js";
vi.mock("../plugin-workspaces.js", () => ({ finalizePluginWorkspace: vi.fn() }));
const db = {} as Parameters<typeof inspectPluginWorkspaceForFinalization>[0];
const workspace = { id: "workspace", providerType: "plugin", strategyType: "git_worktree", branchName: "task" } as Parameters<typeof inspectPluginWorkspaceForFinalization>[2];
describe("provider run completion", () => {
it("passes original run ownership and accepts retained unpublished work", async () => {
vi.mocked(finalizePluginWorkspace).mockResolvedValue({kind:"finalized",headSha:"a".repeat(40),branchName:"task"});
expect(await inspectPluginWorkspaceForFinalization(db,undefined,workspace,"agent","run"))
.toMatchObject({providerType:"plugin",inspection:{kind:"finalized"}});
expect(finalizePluginWorkspace).toHaveBeenLastCalledWith(db,undefined,workspace,{agentId:"agent",runId:"run"});
});
it("fails closed on changed branch and durability failure", async () => {
vi.mocked(finalizePluginWorkspace).mockResolvedValue({kind:"finalized",headSha:"a".repeat(40),branchName:"wrong"});
await expect(inspectPluginWorkspaceForFinalization(db,undefined,workspace,"agent","run")).rejects.toThrow("branch_mismatch");
vi.mocked(finalizePluginWorkspace).mockRejectedValue(new Error("sync incomplete"));
await expect(inspectPluginWorkspaceForFinalization(db,undefined,workspace,"agent","run")).rejects.toThrow("sync incomplete");
});
});
@@ -1,5 +1,5 @@
import type { Db, executionWorkspaces } from "@paperclipai/db";
import { inspectPluginWorkspace, type WorkspaceWorkers } from "../plugin-workspaces.js";
import { finalizePluginWorkspace, type WorkspaceWorkers } from "../plugin-workspaces.js";
/** Validate retained plugin files without interpreting a provider reference as a Core path. */
export async function inspectPluginWorkspaceForFinalization(
@@ -7,9 +7,10 @@ export async function inspectPluginWorkspaceForFinalization(
workers: WorkspaceWorkers | undefined,
workspace: typeof executionWorkspaces.$inferSelect,
agentId: string,
runId: string,
): Promise<Record<string, unknown> | null> {
if (workspace.providerType === "plugin") {
const inspection = await inspectPluginWorkspace(db, workers, workspace);
const inspection = await finalizePluginWorkspace(db, workers, workspace, { agentId, runId });
if (workspace.strategyType === "git_worktree"
&& (!workspace.branchName || inspection.branchName !== workspace.branchName)) {
throw new Error("plugin_workspace_branch_mismatch");
@@ -730,12 +730,12 @@ describe("native workspace finalization recovery", () => {
mode: "isolated_workspace", strategyType: providerType === "plugin" ? "git_worktree" : "project_primary",
branchName: "task-branch", metadata: providerType === "plugin" ? {} : { runnerTaskWorkspaceOwner: { agentId } } }).returning();
const seeded = await seedRun({ executionWorkspaceId: workspace.id, title: "Plugin result before controller restart" });
const inspect = vi.spyOn(pluginWorkspaces, "inspectPluginWorkspace").mockResolvedValue({ kind: "inspection",
const inspect = vi.spyOn(pluginWorkspaces, "finalizePluginWorkspace").mockResolvedValue({ kind: "inspection",
branchName: "task-branch", headSha: "a".repeat(40), hasDirtyTrackedFiles: true, hasUntrackedFiles: false,
hasLocalOnlyData: true, isMergedIntoBase: false, remotelyRecoverable: false });
try {
expect(await resumeNativeWorkspaceFinalization({ db, runId: seeded.runId })).toMatchObject({ status: "succeeded" });
if (providerType === "plugin") expect(inspect).toHaveBeenCalledWith(db, undefined, expect.objectContaining({ id: workspace.id }));
if (providerType === "plugin") expect(inspect).toHaveBeenCalledWith(db, undefined, expect.objectContaining({ id: workspace.id }), { agentId, runId: seeded.runId });
else expect(inspect).not.toHaveBeenCalled();
} finally { inspect.mockRestore(); }
});
@@ -749,7 +749,7 @@ describe("native workspace finalization recovery", () => {
]).returning();
const blocked = await seedRun({ executionWorkspaceId: unavailable.id, title: "Plugin is unavailable after restart" });
const ready = await seedRun({ executionWorkspaceId: healthy.id, title: "Other result can recover" });
const inspect = vi.spyOn(pluginWorkspaces, "inspectPluginWorkspace").mockRejectedValue(new Error(message));
const inspect = vi.spyOn(pluginWorkspaces, "finalizePluginWorkspace").mockRejectedValue(new Error(message));
const code = message.startsWith("plugin_workspace_") ? message : "plugin_workspace_inspection_failed";
try {
await expect(reconcileNativeFinalizations(db, [blocked.runId, ready.runId])).resolves.toBeDefined();
@@ -244,7 +244,7 @@ export async function resumeNativeWorkspaceFinalization(input: {
let pluginWorkspace: Record<string, unknown> | null;
try {
pluginWorkspace = workspace
? await inspectPluginWorkspaceForFinalization(input.db, input.pluginWorkerManager, workspace, bound.agentId)
? await inspectPluginWorkspaceForFinalization(input.db, input.pluginWorkerManager, workspace, bound.agentId, input.runId)
: null;
} catch (error) {
await ownership.assertHeld();
+38 -17
View File
@@ -90,6 +90,9 @@ export async function callWorkspaceProvider(
const selectedProvider = await provider(db, workers, workspace.companyId, pluginId);
if (input.operation.kind === "prepare" && input.operation.fresh && selectedProvider.manifestJson.workspaceProvider?.freshWorkspaceVersion !== 1)
throw conflict("Fresh workspace preparation is unavailable");
if (input.operation.kind === "finalize" &&
(selectedProvider.manifestJson.workspaceProvider?.finalizationVersion !== 1 || !input.runId || !input.agentId))
throw conflict("Workspace finalization is unavailable");
const generation =
typeof workspace.metadata?.lifecycleGeneration === "number"
? workspace.metadata.lifecycleGeneration
@@ -111,11 +114,12 @@ export async function callWorkspaceProvider(
? randomUUID()
: createHash("sha256")
.update(
JSON.stringify([workspace.id, operationGeneration, operation]),
JSON.stringify([workspace.id, operationGeneration, operation,
...(operation.kind === "finalize" ? [input.runId, input.agentId] : [])]),
)
.digest("hex"),
generation: operationGeneration,
deadline: new Date(Date.now() + 300_000).toISOString(),
deadline: new Date(Date.now() + (operation.kind === "finalize" ? 30_000 : 300_000)).toISOString(),
runId: input.runId ?? null,
agentId: input.agentId ?? null,
operation,
@@ -126,7 +130,7 @@ export async function callWorkspaceProvider(
pluginId,
"workspaceOperation",
params,
310_000,
operation.kind === "finalize" ? 40_000 : 310_000,
);
} catch {
throw conflict(
@@ -604,25 +608,42 @@ async function inspectBinding(
return result;
}
export async function inspectPluginWorkspace(
db: Db,
workers: WorkspaceWorkers | undefined,
workspace: Workspace,
) {
/** Reconcile durable lifecycle intent under the same lock for either read path. */
async function withRetainedWorkspace<T>(db: Db, workers: WorkspaceWorkers | undefined,
workspace: Workspace, operation: (workspace: Workspace) => Promise<T>) {
return withExecutionWorkspaceLifecycleLock(db, workspace.id, async () => {
// Keep recovery intent/results durable across an ambiguous provider response.
workspace = await reconcilePluginWorkspaceReservation(
db,
workers,
workspace,
);
workspace = await reconcilePluginWorkspaceReservation(db, workers, workspace);
if (workspace.metadata?.workspaceProviderRestoration)
workspace = await finishPendingRestoration(db, workers, workspace);
if (workspace.metadata?.workspaceProviderRemoval)
workspace = await finishPendingRemoval(db, workers, workspace);
if (workspace.metadata?.workspaceProviderRemoved === true)
return removedInspection();
return inspectBinding(db, workers, workspace);
return operation(workspace);
});
}
export async function inspectPluginWorkspace(db: Db, workers: WorkspaceWorkers | undefined, workspace: Workspace) {
return withRetainedWorkspace(db, workers, workspace, (current) =>
current.metadata?.workspaceProviderRemoved === true
? Promise.resolve(removedInspection()) : inspectBinding(db, workers, current));
}
/** Completion does not need the deletion-safety proof supplied by inspect. */
export async function finalizePluginWorkspace(db: Db, workers: WorkspaceWorkers | undefined,
workspace: Workspace, context: { runId: string; agentId: string }) {
return withRetainedWorkspace(db, workers, workspace, async (current) => {
if (current.metadata?.workspaceProviderRemoved === true)
throw conflict("Workspace was removed before finalization");
const binding = readWorkspaceProviderBinding(current.metadata);
if (!binding) throw conflict("Workspace storage ownership is missing");
const selected = await provider(db, workers, current.companyId, binding.pluginId);
// Existing providers continue to use their established inspection contract.
if (selected.manifestJson.workspaceProvider?.finalizationVersion !== 1)
return inspectBinding(db, workers, current);
const result = await callWorkspaceProvider(db, workers, {
workspace: current, ...context, operation: { kind: "finalize", binding },
});
if (result.kind !== "finalized") throw conflict("Workspace finalization is incomplete");
return result;
});
}