mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-24 01:41:42 +02:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Tasks can use isolated repository workspaces. > - Core currently prepares those repositories on its own filesystem. > - A storage provider may only expose repositories inside remote agent computers. > - Project storage and task workspace preparation must have one provider owner. > - This pull request adds that contract and keeps execution as a separate plugin interface. > - Core keeps authorization, durable task bindings, and lifecycle fences. ## Linked Issues or Issue Description **Subsystem affected** Plugin SDK, project workspace policy, heartbeat preparation, execution leases, and workspace lifecycle. **Problem or motivation** Core cannot prepare a task workspace when its repository storage is unavailable on the Core host. An execution lease must not be required just to prepare persistent storage. **Proposed solution** Let one plugin provide project resource lifecycle and workspace prepare, inspect, restore, and remove behavior. Core reserves a task workspace before it calls the plugin. The plugin returns an opaque durable reference. The matching execution plugin receives that reference on its task RPC. **Alternatives considered** Preparing repositories on Core would require a provider mount on Core. Preparing them only during task launch would tie persistent workspace lifecycle to execution leases and agent availability. **Roadmap alignment** This extends the remote provider model in ROADMAP.md. Related execution work is in #16978. This PR does not replace that task bridge. Native execution of managed storage requires the bridge; this change fails closed when it is absent. ## What Changed - Add one storage provider declaration with supported modes/strategies, a capability, a validated RPC contract, and a test harness. - Persist provider ownership, repository revision, and execution-workspace identity before external effects. Reconcile retries with a stable operation ID. - Prepare provider storage before environment acquisition. Keep Core filesystem preparation out of this path. - Pin the durable reference to the matching execution plugin and validate the lease binding before task dispatch. - Route inspection, reopen, and removal through the existing lifecycle process. Keep local data when remote recovery is uncertain. - Keep the application database pool free during lifecycle locks. Preserve pending workspace selections, validate source edits before retirement, and retry unfinished closed storage cleanup. - Protect provider metadata and live repository sources from caller edits. Storage edits retire only closed records with confirmed removal; retired records cannot reopen. ## Verification - Current head `8214b3ca5`: all 25 provider lifecycle tests pass with `maxConnections=1`. They cover preparation, inspection, reopening, removal, a paused provider with responsive ordinary SQL and a competing source edit, pending selection preservation, rejected source edits, cleanup retry scheduling, and providers that cache operation results, and committed cleanup notifications for storage intent and recovered state. - Final frozen-tree lifecycle run on `8214b3ca5`: all 318 tests passed in 10 files, covering workspace services, reopen, compaction, task binding, project sources, heartbeat preparation, display caching, and source locks. - Direct server `tsc --noEmit` passes. The SDK build and all 128 SDK tests passed during initial verification; later SDK changes are documentation only. - Full CI on the previous head `ec5c7209a` passed all 55 applicable checks; 2 conditional checks skipped. Standard Greptile reviewed current head `342e6d9c3` at 5/5 with no unresolved threads. All 55 applicable CI checks passed on `342e6d9c3`; 2 conditional checks skipped. The Slack browser and CLI startup jobs passed on their single unchanged-head rerun. Its Apex review returned 4/5 with one pending-restoration issue. Commit `1b58943b1` reconciles that durable intent before cleanup, advances the generation without reopening the Core row, and blocks direct removal or retirement while intent remains unresolved. Standard Greptile reviewed `1b58943b1` at 5/5 with no new findings. All 54 applicable CI checks passed on `1b58943b1`; 3 conditional checks skipped. The chat job passed on its single unchanged-head rerun. Apex then returned 4/5 for a missing cleanup wake after recovery. Commit `8214b3ca5` atomically registers cleanup intent before storage intent and recovery-result writes. Its provider test observes real committed notification hints and the visible advanced generation. Standard Greptile reviewed `8214b3ca5` at 5/5 with no new findings. All 55 applicable current-head CI checks passed on the first run; 2 conditional checks skipped. Final Apex verification completed at 5/5 on this exact head: 38 files reviewed, no new comments, and no unresolved threads. The PR is ready for review and mergeable. - The first Apex review found five issues after a standard 5/5: application-pool starvation, stale pending selections, retirement during rejected source edits, lost cleanup retries, and reused inspection identities. Commits `6d72e7fd4` and `342e6d9c3` fix each with regression coverage. Archive lock waits use dedicated connections, and source edits fail promptly if the lifecycle is busy. Retirement compares normalized values. The normalized-source regression initially failed with the raw-value comparison; after moving the check after normalization, a clean provider-suite rerun passed all 24 tests at that stage. Those threads are resolved; the earlier Apex 0/5 is not a passing acceptance result. - Earlier local integration attempts could not bind localhost inside the sandbox. The same suites passed with permission to start local PostgreSQL. - The earlier broad local `pnpm test:run` overlapped edits and reported stale-revision and checkout-path/Git-fixture failures. It was not a final-tree result. Full CI subsequently passed; the original local failure remains part of verification history. - Local repository-wide typecheck/build cannot complete because `cargo` is unavailable. Direct server TypeScript checks and affected tests pass; full Rust and build validation comes from CI. - An earlier browser run on `ff918c5a8` failed in the unchanged case-filter journey: expected “Done,” received “All statuses,” with an announcement handler recorded during the assertion. The cause is not confirmed. [The failed job](https://github.com/paperclipai/paperclip/actions/runs/39500611857/job/118896954508) remains verification history. The unchanged shard passed in the next full CI run on `ec5c7209a`; no UI or fixture change was made. - Current-head browser shard 2 failed waiting five seconds for the mocked Slack setup Success heading. This PR changes neither that UI nor its fixture. [The failed job](https://github.com/paperclipai/paperclip/actions/runs/39506070513/job/118916021232) remains verification evidence. The cause is not confirmed; the single unchanged-shard rerun passed; no UI or fixture change was made, and the original cause remains unconfirmed. - The workspace test shard passed its 3,978 general tests, then its CLI company-import/export fixture timed out after 30 seconds waiting for startup health before creating any company or workspace. [The failed job](https://github.com/paperclipai/paperclip/actions/runs/39506070513/job/118916021855) remains verification history. The child reached external-PostgreSQL startup without exiting or reporting an exception. Its cause is not confirmed; the single unchanged-job rerun passed, including the CLI fixture. No startup or fixture change was made, and the original cause remains unconfirmed. - The new cached-removal regression initially failed because its fake worker advertised availability for unrelated fixture plugins, so another workspace changed its fake checkout state. Worker availability now matches the fixture plugin; the complete 25-test provider suite passes. - On `1b58943b1`, the chat shard timed out polling for one conversation after an authorized GitHub description edit. [The failed job](https://github.com/paperclipai/paperclip/actions/runs/39510521657/job/118930193418) remains evidence. The exact case passes locally against PostgreSQL (1 passed, 1,380 unrelated cases skipped), and this PR changes neither that test nor its handler. The original cause is unconfirmed; the single unchanged-job CI rerun passed without a code or fixture change. The original cause remains unconfirmed. - The diff was checked for credentials, private references, and generated artifacts. ## Risks This is an additive provider contract with no database migration. Existing projects keep their local behavior unless their policy selects a storage plugin. A selected provider must be ready and enabled. Unknown or unsafe cleanup fails closed. Managed native execution depends on #16978. Core refuses a host Runner fallback. Providers must fence lifecycle generations, preserve ambiguous retries, and report remote recovery honestly. Provider-specific live storage and guest mount qualification remain separate from these Core contract tests. ## Model Used OpenAI Codex (GPT-6), with repository inspection, code execution, and tests. The exact model variant was not exposed. ## 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 linked existing issues or described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links - [x] My branch name describes the change and contains no internal ticket id - [x] I have run tests locally and they pass (affected suites; broad-run limits are documented above) - [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>
120 lines
3.4 KiB
TypeScript
120 lines
3.4 KiB
TypeScript
import { describe, expect, it } from "vitest";
|
|
import {
|
|
workspaceProviderOperationSchema,
|
|
parseWorkspaceProviderResult,
|
|
type PluginWorkspaceParams,
|
|
} from "../src/workspace-provider.js";
|
|
import { createWorkspaceProviderHarness } from "../src/testing.js";
|
|
import { pluginManifestV1Schema } from "@paperclipai/shared";
|
|
const id = (n: number) =>
|
|
`10000000-0000-4000-8000-${String(n).padStart(12, "0")}`;
|
|
const params: PluginWorkspaceParams = {
|
|
pluginId: id(1),
|
|
companyId: id(2),
|
|
projectId: id(3),
|
|
executionWorkspaceId: id(4),
|
|
operationId: "stable",
|
|
generation: 0,
|
|
deadline: "2099-01-01T00:00:00Z",
|
|
runId: null,
|
|
agentId: null,
|
|
operation: {
|
|
kind: "prepare",
|
|
source: {
|
|
id: id(5),
|
|
revision: "source-v1",
|
|
repoUrl: "https://github.com/example/repo.git",
|
|
repoRef: null,
|
|
defaultRef: "main",
|
|
},
|
|
mode: "isolated_workspace",
|
|
strategy: "git_worktree",
|
|
branchName: "task",
|
|
baseRef: "main",
|
|
existing: null,
|
|
},
|
|
};
|
|
const result = {
|
|
kind: "prepared" as const,
|
|
binding: {
|
|
pluginId: id(1),
|
|
projectId: id(3),
|
|
executionWorkspaceId: id(4),
|
|
projectWorkspaceId: id(5),
|
|
sourceRevision: "source-v1",
|
|
workspaceRef: "opaque",
|
|
},
|
|
branchName: "task",
|
|
baseSha: "a".repeat(40),
|
|
created: true,
|
|
branchCreatedByRuntime: true,
|
|
};
|
|
describe("workspace provider SDK", () => {
|
|
it("prepares without a VM, lease, or host path", async () => {
|
|
const harness = createWorkspaceProviderHarness(async (input) => {
|
|
expect(input.agentId).toBeNull();
|
|
expect(input).not.toHaveProperty("lease");
|
|
expect(input.operation).not.toHaveProperty("localPath");
|
|
return result;
|
|
});
|
|
expect(await harness.operation(params)).toEqual(result);
|
|
});
|
|
it("rejects unrelated operation results and changed source revisions", () => {
|
|
expect(() =>
|
|
parseWorkspaceProviderResult(params, { kind: "removed" }),
|
|
).toThrow();
|
|
expect(() =>
|
|
parseWorkspaceProviderResult(params, {
|
|
...result,
|
|
binding: { ...result.binding, sourceRevision: "other" },
|
|
}),
|
|
).toThrow();
|
|
expect(() =>
|
|
parseWorkspaceProviderResult(params, {
|
|
...result,
|
|
binding: { ...result.binding, executionWorkspaceId: id(99) },
|
|
}),
|
|
).toThrow();
|
|
expect(() =>
|
|
parseWorkspaceProviderResult(params, { ...result, branchName: "wrong" }),
|
|
).toThrow();
|
|
expect(() =>
|
|
workspaceProviderOperationSchema.parse({
|
|
...params.operation,
|
|
shell: "arbitrary",
|
|
}),
|
|
).toThrow();
|
|
});
|
|
it("requires project lifecycle and storage management as one contract", () => {
|
|
const manifest = {
|
|
id: "test.storage",
|
|
apiVersion: 1,
|
|
version: "1.0.0",
|
|
displayName: "Storage",
|
|
description: "Storage",
|
|
author: "Test",
|
|
categories: ["workspace"],
|
|
entrypoints: { worker: "./worker.js" },
|
|
workspaceProvider: {
|
|
modes: ["isolated_workspace", "operator_branch"],
|
|
strategies: ["git_worktree"],
|
|
},
|
|
capabilities: ["workspaces.manage", "events.subscribe"],
|
|
resourceLifecycle: ["project"],
|
|
};
|
|
expect(pluginManifestV1Schema.safeParse(manifest).success).toBe(true);
|
|
expect(
|
|
pluginManifestV1Schema.safeParse({
|
|
...manifest,
|
|
resourceLifecycle: ["agent"],
|
|
}).success,
|
|
).toBe(false);
|
|
expect(
|
|
pluginManifestV1Schema.safeParse({
|
|
...manifest,
|
|
capabilities: ["events.subscribe"],
|
|
}).success,
|
|
).toBe(false);
|
|
});
|
|
});
|