mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-25 02:26:12 +02:00
fix: preserve qualified snapshots during native setup validation (#17445)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native agent setup can validate a selected sandbox before the agent runs. > - A qualified snapshot binds the saved environment and resolved runtime to an immutable provider snapshot. > - Setup validation changes lease retention so each test starts fresh and leaves an archived sandbox for inspection. > - The test passed that changed config as the saved environment, which made valid snapshot proof appear stale. > - This pull request keeps saved qualification separate from the two test retention settings. > - Native setup can use the qualified snapshot and still clean up its test lease. ## Linked Issues or Issue Description Related: #15997, #17262. **What happened?** Native setup validation can return `environment_lease_acquire_failed` for a sandbox whose saved snapshot qualification is valid. The test changes `reuseLease` and `archiveOnRelease` before the runtime checks the saved configuration fingerprint. **Expected behavior** The test must qualify the original saved environment, acquire a fresh lease from its immutable snapshot, and preserve archive-on-release in the lease metadata. A changed image, provider, runtime config, or stale proof must still fail. **Steps to reproduce** 1. Save a Daytona snapshot environment with `reuseLease: true` and `archiveOnRelease: false`. 2. Store valid company-scoped snapshot qualification for the saved and resolved runtime configurations. 3. Run native agent setup validation against that environment. 4. Observe that the validation retention overrides invalidate the saved fingerprint before a provider lease is acquired. **Paperclip version or commit** Reproduced from master at `785d54636e1c8513ee5503d602daf6a5ee1970d6`. The fix is based on `62f60bf158746c5807763778326b1eba4f2ed18a`. **Deployment mode** Any server with a qualified native Daytona sandbox environment. ## What Changed - Pass the original saved environment to native setup lease acquisition. - Allow only `reuseLease` and `archiveOnRelease` to differ in the run configuration. Apply those fields after saved snapshot qualification. - Keep both retention fields in the acquired lease metadata so provider release still archives the test sandbox. - Add service-to-runtime cases for valid and stale proof, immutable snapshot selection, fresh lease retention, release behavior, and rejected non-retention overrides. - Extend the route regression to require the original saved configuration. ## Verification - The focused route regression failed before the fix because acquisition had no `configuredEnvironment`. - All 101 focused route, snapshot, and orchestration tests passed on the exact rebased head. - Full repository typecheck and build passed on exact head `15e13e119941090e85638005de54b2d14ea52c00`. - The local database regression could not start PostgreSQL because macOS could not allocate system semaphores. The full `pnpm test:run` attempt also reported database startup failures and was stopped as incomplete. Neither attempt is counted as passing. - Exact-head Linux CI passed all 179 environment runtime integration tests, including the seven new regression cases. Its server shard passed 3,635 tests with 16 existing skips. All exact-head CI gates passed. The final repository review must also complete before merge. - No live provider or model call is part of this verification. This PR does not deploy the fix. ## Risks The acquisition boundary now accepts two explicit retention-only differences. All other config fields, environment identity, company-scoped proof, resolved runtime fingerprint, and template binding remain subject to the existing checks. Regression cases reject changed snapshot, image, provider, and environment variables before provider dispatch. No schema or API contract changes. No user-facing command or documentation contract changes. ## Model Used OpenAI Codex, GPT-6 family, with reasoning, repository inspection, code execution, and test tools. The session does not expose an exact backend 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 - [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:
1 parent
62f60bf158
commit
b7506671b9
4 files changed
+68
-3
No files matched your search
@@ -804,6 +804,10 @@ describe("agent test-environment route", () => {
|
||||
expect(mockEnvironmentRuntime.acquireRunLease).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
applyCustomImageTemplate: true,
|
||||
configuredEnvironment: expect.objectContaining({
|
||||
id: "11111111-1111-4111-8111-111111111111",
|
||||
config: { provider: "fake-plugin" },
|
||||
}),
|
||||
// The Test lease re-checks the company binding, so a binding change
|
||||
// between the route guard and the lease cannot open a foreign sandbox.
|
||||
assertCompanyBinding: true,
|
||||
|
||||
@@ -45,6 +45,7 @@ import {
|
||||
import * as sandboxProviderRuntime from "../services/sandbox-provider-runtime.ts";
|
||||
import * as environmentsModule from "../services/environments.ts";
|
||||
import { logger } from "../middleware/logger.ts";
|
||||
import { agentEnvironmentTestService } from "../services/agent-environment-test.js";
|
||||
import { resolveRunnerEnvironmentForRun } from "../services/runner-environment-lifecycle.js";
|
||||
import { environmentService } from "../services/environments.ts";
|
||||
import { remoteExecutionHasStopped } from "../services/remote-execution-termination.ts";
|
||||
@@ -358,6 +359,65 @@ describeEmbeddedPostgres("environmentRuntimeService", () => {
|
||||
expect(acquired.lease.metadata?.snapshot).toBe(expectedSnapshot);
|
||||
});
|
||||
|
||||
it.each(["qualified", "changed-base", "changed-runtime"] as const)("validates native setup against saved snapshot qualification (%s)", async state => {
|
||||
const pluginId = randomUUID();
|
||||
const { companyId, environment: baseEnvironment } = await seedEnvironment();
|
||||
const config = { provider: "daytona", snapshot: "managed-snapshot", reuseLease: true, archiveOnRelease: false, timeoutMs: 1234 };
|
||||
await insertSandboxProviderPlugin(pluginId, { environmentDrivers: [{ driverKey: "daytona", kind: "sandbox_provider", displayName: "Daytona", configSchema: { type: "object" } }] });
|
||||
const svc = environmentService(db);
|
||||
await svc.update(baseEnvironment.id, { driver: "sandbox", config });
|
||||
const environment = (await svc.getById(baseEnvironment.id))!;
|
||||
const parsed = await resolveEnvironmentDriverConfigForRuntime(db, companyId, environment);
|
||||
const snapshotId = "a6b9e7d0-f886-471a-b461-640dd03e8d0f";
|
||||
const proof = { v: 2, companyId, snapshot: config.snapshot, template: null,
|
||||
baseConfigFingerprint: createHash("sha256").update(stableStringify(config)).digest("hex"),
|
||||
runtimeConfigFingerprint: state === "changed-runtime" ? "0".repeat(64) : createHash("sha256").update(stableStringify(parsed.config)).digest("hex"),
|
||||
snapshotIdentity: { provider: "daytona", requestedRef: config.snapshot, id: snapshotId, name: config.snapshot, organizationId: "provider-account", createdAt: "2000-01-01T00:00:00.000Z" },
|
||||
};
|
||||
await db.update(environments).set({ metadata: { paperclipSandboxPlatformProbes: { [companyId]: proof } },
|
||||
...(state === "changed-base" ? { config: { ...config, snapshot: "replacement-snapshot" } } : {}) }).where(eq(environments.id, environment.id));
|
||||
const worker = createFreshPluginLeaseWorker(pluginId);
|
||||
vi.mocked(worker.call).mockImplementation(async (_id, method, params: any) => {
|
||||
if (method === "environmentAcquireLease") return { providerLeaseId: "setup-snapshot-lease", metadata: { provider: "daytona", snapshot: params.config.snapshot, remoteCwd: "/workspace" } };
|
||||
if (method === "environmentRealizeWorkspace") return { cwd: "/workspace" };
|
||||
if (method === "environmentReleaseLease") return { providerLeaseId: params.providerLeaseId, state: "archived" };
|
||||
throw new Error(`Unexpected ${method}`);
|
||||
});
|
||||
const result = await agentEnvironmentTestService(db, worker).resolveAdapterTestExecutionContext({ companyId, environmentId: environment.id, adapterType: "paperclip_runner" });
|
||||
if (state !== "qualified") {
|
||||
expect(result.executionTarget).toBeNull();
|
||||
expect(result.fallbackChecks).toMatchObject([{ code: "environment_lease_acquire_failed", detail: expect.stringContaining("sandbox_snapshot_qualification_required") }]);
|
||||
expect(worker.call).not.toHaveBeenCalled();
|
||||
return;
|
||||
}
|
||||
expect(result.fallbackChecks).toEqual([]);
|
||||
expect(result.executionTarget).toMatchObject({ kind: "remote", transport: "sandbox" });
|
||||
expect(worker.call).toHaveBeenCalledWith(pluginId, "environmentAcquireLease", expect.objectContaining({ config: expect.objectContaining({ snapshot: snapshotId, reuseLease: false, archiveOnRelease: true }) }), expect.any(Number));
|
||||
const leases = await svc.listLeases(environment.id);
|
||||
expect(leases).toHaveLength(1);
|
||||
expect(leases[0]).toMatchObject({ leasePolicy: "ephemeral", heartbeatRunId: null, providerLeaseId: "setup-snapshot-lease" });
|
||||
expect(leases[0]!.metadata).toMatchObject({ snapshot: snapshotId, reuseLease: false, archiveOnRelease: true });
|
||||
await result.release("released");
|
||||
expect(worker.call).toHaveBeenCalledWith(pluginId, "environmentReleaseLease", expect.objectContaining({ config: expect.objectContaining({ archiveOnRelease: true }), providerLeaseId: "setup-snapshot-lease" }), expect.any(Number));
|
||||
expect((await svc.getById(environment.id))!.config).toEqual(config);
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ snapshot: "another-snapshot" },
|
||||
{ image: "another-image" },
|
||||
{ provider: "another-provider" },
|
||||
{ env: { DIFFERENT_RUNTIME: "changed" } },
|
||||
])("rejects non-retention run overrides before provider dispatch (%j)", async override => {
|
||||
const { companyId, environment: baseEnvironment } = await seedEnvironment();
|
||||
const environment = { ...baseEnvironment, driver: "sandbox", config: { provider: "daytona", snapshot: "managed-snapshot" } };
|
||||
const worker = createFreshPluginLeaseWorker(randomUUID());
|
||||
const service = environmentRuntimeService(db, { pluginWorkerManager: worker });
|
||||
await expect(service.acquireRunLease({ companyId, environment: { ...environment, config: { ...environment.config, ...override, reuseLease: false, archiveOnRelease: true } },
|
||||
configuredEnvironment: environment, issueId: null, heartbeatRunId: null, persistedExecutionWorkspace: null, adapterType: "paperclip_runner" }))
|
||||
.rejects.toThrow("Run environment does not match its saved configuration.");
|
||||
expect(worker.call).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.each(["failed-probe", "config-change", "config-and-metadata-change"] as const)("refuses a mutable snapshot after %s invalidates its proof", async invalidation => {
|
||||
const { companyId, environment: baseEnvironment, runId } = await seedEnvironment();
|
||||
const config = { provider: "daytona", snapshot: "managed-snapshot", reuseLease: false };
|
||||
|
||||
@@ -231,6 +231,7 @@ export function agentEnvironmentTestService(db: Db, pluginWorkerManager?: Plugin
|
||||
...(environment.driver === "plugin" && input.adapterType === "paperclip_runner"
|
||||
? { requestedExpiresAt: new Date(Date.now() + 180_000) } : {}),
|
||||
environment: testEnvironment,
|
||||
configuredEnvironment: environment,
|
||||
issueId: null,
|
||||
heartbeatRunId: null,
|
||||
persistedExecutionWorkspace: null,
|
||||
|
||||
@@ -1856,8 +1856,8 @@ function createSandboxEnvironmentDriver(
|
||||
// or account used for qualification and custom-template resolution.
|
||||
if (configuredEnvironment.id !== input.environment.id
|
||||
|| configuredEnvironment.driver !== input.environment.driver
|
||||
|| stableStringify({ ...configuredEnvironment.config, reuseLease: undefined })
|
||||
!== stableStringify({ ...input.environment.config, reuseLease: undefined })) {
|
||||
|| stableStringify({ ...configuredEnvironment.config, reuseLease: undefined, archiveOnRelease: undefined })
|
||||
!== stableStringify({ ...input.environment.config, reuseLease: undefined, archiveOnRelease: undefined })) {
|
||||
throw new Error("Run environment does not match its saved configuration.");
|
||||
}
|
||||
const storedParsed = parseEnvironmentDriverConfig(input.environment);
|
||||
@@ -1877,7 +1877,7 @@ function createSandboxEnvironmentDriver(
|
||||
&& hasSandboxSnapshotQualification(configuredEnvironment, input.companyId)) {
|
||||
throw new Error("sandbox_snapshot_qualification_required: The snapshot qualification is missing or stale. Test this environment again before running Paperclip Runner.");
|
||||
}
|
||||
if (input.configuredEnvironment) parsed.config = { ...parsed.config, reuseLease: storedParsed.config.reuseLease };
|
||||
if (input.configuredEnvironment) parsed.config = { ...parsed.config, reuseLease: storedParsed.config.reuseLease, archiveOnRelease: storedParsed.config.archiveOnRelease };
|
||||
if (pinnedSnapshotId) parsed.config = { ...parsed.config, snapshot: pinnedSnapshotId };
|
||||
|
||||
// Check if this provider should be handled by a plugin.
|
||||
|
||||
Reference in new issue
Block a user