mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-08 12:33:05 +01:00
fix(runtime): preserve legacy identities and bound checkpoint imports (#18957)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Ephemeral web servers must preserve native Runner sessions before removing local storage. > - Legacy session imports validate the workspace and saved provider authority. > - A protocol lease can retain a logical workspace ID while copy-back uses a separate physical sandbox lease ID. > - Comparing these different IDs blocks valid legacy sessions during enrollment. > - This pull request checks the workspace binding, retains the full saved protocol identity check, and bounds checkpoint inserts inside the owner transaction. > - Existing sessions can migrate without changing their identity or discarding files. ## Linked Issues or Issue Description Refs #18943 and #18841. **What happened?** The first full staging enrollment stopped with `native_controller_migration_artifact_binding`. A successful native session had valid controller and provider-pack identities, but its physical sandbox lease had a different ID. **Expected behavior** Import the original session when its workspace and saved provider authority match. Reject foreign workspaces and protocol identities. **Steps to reproduce** Create a retained session whose sync record points to the execution workspace and a separate physical lease. Run the offline controller migration. The prior comparison rejects this valid record. **Paperclip version or commit** Reproduced on `84aa989bf9d08fc005ac273c37f9e1d860a4db25`. **Deployment mode** Docker with external PostgreSQL and shared object storage. ## What Changed - Compare the sync record's workspace ID with the frozen execution workspace. - Keep the full saved provider-pack check for company, agent, workspace, session, Runner, protocol lease, remote path, and provider digest. - Use distinct physical and protocol IDs in the database-backed migration fixture. Reject a foreign workspace and a changed protocol lease before any checkpoint import. - Limit migration inserts to 64 records and about 512 KiB of serialized rows per statement. Send a larger individual record alone. Keep all statements in the same transaction. - Verify that a conflict in a later batch rolls back earlier batches, then import and decrypt the complete checkpoint. - Record both live failures and the comparison evidence in the preservation plan. ## Verification - The updated fixture failed on the original comparison and passes with this fix. - Targeted migration, controller recovery, and provider authority tests: 39 passed. - Full repository typecheck and the latest server typecheck passed locally. The complete PR workflow passed on `a1f3637c96` (run 41827558125), including build and test gates. The Docker Compose setup download failed once before the test step; its infrastructure retry passed. A separate full local test/build run is still in progress. - A read-only staging preflight inspected 807 selected legacy sessions. It found zero mismatches under the corrected workspace and full saved-provider checks. - A single 538-record, 2.2 MB checkpoint insert repeatedly closed the live database connection on pooled and direct endpoints. A forced-rollback comparison imported the identical checkpoint with batches of 64. An unbatched control failed again after cache warm-up. The exact server-side disconnect cause remains unknown; all failed receipts are retained. - The original failed enrollment and volume are retained. The corrected importer committed 32 additional sessions and 2,982 checkpoint records. Full import and deploy handoff remain pending while memory use is investigated. ## Risks The import checks durable session authority. A weaker identity check could import the wrong session. This change retains the saved provider authority validation and adds an explicit foreign-protocol rejection test. Batching adds statements for large imports, but a late failure still rolls back the complete owner transaction. It does not rewrite session identity, change provider lifecycle, or detach any volume. ## Model Used OpenAI Codex, GPT-6 family, with reasoning, tool use, and code execution. The exact deployment model ID and context window are not exposed in this session. ## 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
84aa989bf9
commit
d12de135a7
4 files changed
+93
-8
No files matched your search
@@ -117,3 +117,28 @@ with matching, mismatched, and absent migration receipts. An executor-level
|
||||
test restores real backup files into a created warm lease only for the exact
|
||||
migrated session, verifies the retained provider identity, and checks the new
|
||||
lease's backup stamp. Neither test replaces the migration decision helper.
|
||||
|
||||
### Live enrollment: protocol and physical lease identities
|
||||
|
||||
The first full legacy import exposed a false binding failure: the controller's
|
||||
`environmentLeaseId` retained its logical workspace identity, while
|
||||
`nativeWorkspaceSync.leaseId` identified a distinct physical sandbox lease row.
|
||||
The migration now validates the sync record's `workspaceId` against the frozen
|
||||
execution workspace. The saved provider-pack authority still independently
|
||||
verifies the exact protocol lease, runner, session, company, agent, provider and
|
||||
remote path. A database-backed fixture uses distinct IDs, rejects foreign
|
||||
workspaces and protocol identities, and restores the original checkpoint after
|
||||
the predecessor home is deleted. The original failed live attempt and its
|
||||
attached volume remain preserved until the corrected owner passes verification.
|
||||
|
||||
The next live import exposed a second boundary: one insert of 538 encrypted
|
||||
checkpoint records (about 2.2 MB) repeatedly closed the database connection on
|
||||
both pooled and direct endpoints, including after warming the database cache.
|
||||
The identical import completed with smaller inserts in a forced-rollback
|
||||
transaction. The importer now limits each statement to 64 records and about
|
||||
512 KiB of serialized rows; an individual larger record is sent alone. All
|
||||
statements remain in the original owner transaction. A real PostgreSQL test
|
||||
forces a conflict in a later batch, verifies that earlier batches roll back,
|
||||
then imports and decrypts the complete original checkpoint. The precise
|
||||
server-side reason for the connection closure is not yet established; the
|
||||
unbatched failures and batched comparison remain in the verification record.
|
||||
@@ -4,11 +4,12 @@ import { join } from "node:path";
|
||||
import { eq } from "drizzle-orm";
|
||||
import { afterEach, expect, it, vi } from "vitest";
|
||||
import { agentTaskSessions, heartbeatRuns, issues, nativeHarnessBackups, nativeRunFinalizations, nativeRunnerCheckpoints } from "@paperclipai/db";
|
||||
import { DurablePrpControlPlane, parseNativeExecutionInput } from "../../vendor/paperclip-runner/index.js";
|
||||
import { captureLegacyDurablePrpCheckpoint, DurablePrpControlPlane, parseNativeExecutionInput } from "../../vendor/paperclip-runner/index.js";
|
||||
import { describeEmbeddedPostgres, nativeRestartFixture } from "../../__tests__/helpers/native-restart.js";
|
||||
import { nativeToolContractFingerprintForTarget } from "./native-session-resume.js";
|
||||
import { nativeControllerMigrationDescriptor } from "./native-session-executor.js";
|
||||
import { hasLocalNativeControllerObligations, migrateNativeControllers } from "./native-controller-migration.js";
|
||||
import { insertMigratedNativeRunnerCheckpoint, loadNativeRunnerCheckpoint } from "./native-runner-checkpoint.js";
|
||||
import { resolveNativeControllerRecovery } from "./native-controller-recovery.js";
|
||||
import { resolveRemoteProviderPackAuthority } from "./remote-provider-pack-authority.js";
|
||||
import { canonicalProviderPackJson } from "./remote-provider-pack-verification.js";
|
||||
@@ -18,10 +19,10 @@ import { createStorageService } from "../../storage/service.js";
|
||||
import { legacyNativeControllerArchive } from "./native-controller-archive.js";
|
||||
import { restoreDurableNativeHarnessBackup } from "./native-harness-backup-storage.js";
|
||||
|
||||
describeEmbeddedPostgres.each(["import", "archive", "success"])("offline native controller migration (%s)", scenario => {
|
||||
describeEmbeddedPostgres.each(["import", "archive", "success", "batch"])("offline native controller migration (%s)", scenario => {
|
||||
const environment = nativeRestartFixture();
|
||||
afterEach(() => vi.unstubAllEnvs());
|
||||
async function fixture() {
|
||||
async function fixture(commandCount = 1) {
|
||||
vi.stubEnv("PAPERCLIP_SECRETS_MASTER_KEY", "73".repeat(32));
|
||||
const f = await environment.seedRun(randomUUID());
|
||||
const { companyId, agentId, paperclipHome, runtimeRoot } = environment.current;
|
||||
@@ -42,7 +43,10 @@ describeEmbeddedPostgres.each(["import", "archive", "success"])("offline native
|
||||
environmentLeaseId: f.native.environmentLeaseId, turnId: f.native.turnId, itemId: f.native.itemId };
|
||||
const core = new DurablePrpControlPlane({ stateDirectory: join(paths.root, "control-plane"), identity,
|
||||
expectedRunnerVersion: "migration-fixture", expectedRunnerDigest: `sha256:${"a".repeat(64)}` });
|
||||
core.issueBootstrapTicket(); core.queueCommand("session.snapshot", {}, "retained-command"); await core.stop();
|
||||
core.issueBootstrapTicket();
|
||||
for (let index = 0; index < commandCount; index++) core.queueCommand("session.snapshot",
|
||||
{ diagnostic: "x".repeat(commandCount > 1 ? 6000 : 0) }, `retained-command-${index}`);
|
||||
await core.stop();
|
||||
const file = join(paths.root, "control-plane/control-plane-state.json");
|
||||
const state = JSON.parse(await readFile(file, "utf8"));
|
||||
state.tickets = {};
|
||||
@@ -66,7 +70,7 @@ describeEmbeddedPostgres.each(["import", "archive", "success"])("offline native
|
||||
identity: { companyId, agentId, issueId: f.issueId, runId: f.runId, sessionId: f.native.normalizedSessionId } };
|
||||
const profile = { ...f.preparedProfile, nativeExecutionInput: execution, nativeToolContractFingerprint: fingerprint, sessionCheckpoint: snapshot,
|
||||
nativeWorkspaceSync: { schema: "paperclip.native-workspace-sync/v1", state: "prepared", descriptorSha256: "a".repeat(64),
|
||||
baselineSha256: "b".repeat(64), finalHostSha256: null, workspaceId: randomUUID(), leaseId: f.native.environmentLeaseId,
|
||||
baselineSha256: "b".repeat(64), finalHostSha256: null, workspaceId: binding.executionWorkspaceId, leaseId: randomUUID(),
|
||||
providerLeaseId: randomUUID(), remoteCwd: "/remote/workspace", resourceDisposition: "destroy" } };
|
||||
await f.db.update(heartbeatRuns).set({ status: "failed", finishedAt: new Date(), runnerProfileJson: profile }).where(eq(heartbeatRuns.id, f.runId));
|
||||
await f.db.update(nativeRunFinalizations).set({ phase: "terminal_failure", leaseOwner: null }).where(eq(nativeRunFinalizations.runId, f.runId));
|
||||
@@ -82,10 +86,19 @@ describeEmbeddedPostgres.each(["import", "archive", "success"])("offline native
|
||||
const { companyId, agentId, paperclipHome, runtimeRoot, profile, file, source, session, queuedId, identity, snapshot } = f;
|
||||
expect(await hasLocalNativeControllerObligations(f.db)).toBe(true);
|
||||
await f.db.update(heartbeatRuns).set({ runnerProfileJson: { ...profile,
|
||||
nativeWorkspaceSync: { ...profile.nativeWorkspaceSync, leaseId: randomUUID() } } }).where(eq(heartbeatRuns.id, f.runId));
|
||||
nativeWorkspaceSync: { ...profile.nativeWorkspaceSync, workspaceId: randomUUID() } } }).where(eq(heartbeatRuns.id, f.runId));
|
||||
await expect(migrateNativeControllers(f.db, { writersStopped: true, signal: AbortSignal.timeout(30_000) }))
|
||||
.rejects.toThrow("native_controller_migration_artifact_binding");
|
||||
await f.db.update(heartbeatRuns).set({ runnerProfileJson: profile }).where(eq(heartbeatRuns.id, f.runId));
|
||||
expect(profile.nativeWorkspaceSync.leaseId).not.toBe(identity.environmentLeaseId);
|
||||
expect(profile.nativeWorkspaceSync.workspaceId).toBe(identity.environmentLeaseId);
|
||||
const foreignProtocol = JSON.parse(source);
|
||||
foreignProtocol.identity.environmentLeaseId = randomUUID();
|
||||
await writeFile(file, JSON.stringify(foreignProtocol), { mode: 0o600 });
|
||||
await expect(migrateNativeControllers(f.db, { writersStopped: true, signal: AbortSignal.timeout(30_000) }))
|
||||
.rejects.toThrow("saved provider pack authority is invalid or does not match this execution");
|
||||
expect(await f.db.select().from(nativeRunnerCheckpoints).where(eq(nativeRunnerCheckpoints.runId, f.runId))).toHaveLength(0);
|
||||
await writeFile(file, source, { mode: 0o600 });
|
||||
await f.db.update(nativeRunFinalizations).set({ leaseOwner: "unsettled-owner" }).where(eq(nativeRunFinalizations.runId, f.runId));
|
||||
await expect(migrateNativeControllers(f.db, { writersStopped: true, signal: AbortSignal.timeout(30_000) })).rejects.toThrow("normal_ownership_recovery_required");
|
||||
expect(await f.db.select().from(nativeRunnerCheckpoints).where(eq(nativeRunnerCheckpoints.runId, f.runId))).toHaveLength(0);
|
||||
@@ -112,6 +125,41 @@ describeEmbeddedPostgres.each(["import", "archive", "success"])("offline native
|
||||
expect(JSON.parse(await readFile(join(replacement, "control-plane/control-plane-state.json"), "utf8")).identity).toEqual(identity);
|
||||
expect(recovered.snapshot).toEqual(snapshot);
|
||||
});
|
||||
if (scenario === "batch") it("imports a large checkpoint with bounded writes and rolls back every batch on a late conflict", async () => {
|
||||
const f = await fixture(160);
|
||||
const binding = { companyId: f.companyId, issueId: f.issueId, runId: f.runId };
|
||||
const checkpoint = captureLegacyDurablePrpCheckpoint(f.source, { runId: f.runId,
|
||||
runnerInstanceId: f.native.runnerInstanceId, normalizedSessionId: f.native.normalizedSessionId },
|
||||
{ version: "migration-fixture", digest: `sha256:${"a".repeat(64)}` });
|
||||
const last = [...checkpoint.records].at(-1)!;
|
||||
await f.db.transaction(tx => insertMigratedNativeRunnerCheckpoint(tx, binding, {
|
||||
...checkpoint, records: new Map([last]),
|
||||
}));
|
||||
const before = await f.db.select().from(nativeRunnerCheckpoints).where(eq(nativeRunnerCheckpoints.runId, f.runId));
|
||||
const parameters: number[] = [];
|
||||
const payloadBytes: number[] = [];
|
||||
const previousDebug = f.db.$client.options.debug;
|
||||
f.db.$client.options.debug = (_connection, query, values) => {
|
||||
if (query.startsWith('insert into "native_runner_checkpoints"')) {
|
||||
parameters.push(values.length);
|
||||
payloadBytes.push(values.reduce((sum, value) => sum + Buffer.byteLength(JSON.stringify(value)), 0));
|
||||
}
|
||||
};
|
||||
try {
|
||||
await expect(f.db.transaction(tx => insertMigratedNativeRunnerCheckpoint(tx, binding, checkpoint)))
|
||||
.rejects.toThrow();
|
||||
expect(await f.db.select().from(nativeRunnerCheckpoints).where(eq(nativeRunnerCheckpoints.runId, f.runId))).toEqual(before);
|
||||
expect(parameters.length).toBeGreaterThan(1);
|
||||
expect(Math.max(...parameters)).toBeLessThanOrEqual(320);
|
||||
expect(Math.max(...payloadBytes)).toBeLessThanOrEqual(512 * 1024);
|
||||
await f.db.delete(nativeRunnerCheckpoints).where(eq(nativeRunnerCheckpoints.runId, f.runId));
|
||||
expect(await migrateNativeControllers(f.db, { writersStopped: true, signal: AbortSignal.timeout(30_000) }))
|
||||
.toMatchObject({ imported: 1 });
|
||||
const restored = await loadNativeRunnerCheckpoint(f.db, binding);
|
||||
expect(restored?.records).toEqual(checkpoint.records);
|
||||
expect(await readFile(f.file, "utf8")).toBe(f.source);
|
||||
} finally { f.db.$client.options.debug = previousDebug; }
|
||||
}, 60_000);
|
||||
if (scenario === "archive") it("preserves an already-failed legacy controller without inventing executable recovery authority", async () => {
|
||||
const f = await fixture();
|
||||
await rm(join(f.paths.root, "provider-pack-authority.json"));
|
||||
|
||||
@@ -124,7 +124,7 @@ export async function migrateNativeControllers(db: Db, input: { writersStopped:
|
||||
const workspace = readNativeWorkspaceSyncReference(admission?.workspace);
|
||||
const remoteCwd = workspace?.remoteCwd ?? execution.workspace.cwd;
|
||||
if (typeof identity?.environmentLeaseId !== "string" || !remoteCwd
|
||||
|| (workspace && workspace.leaseId !== identity.environmentLeaseId)) {
|
||||
|| (workspace && workspace.workspaceId !== execution.binding.executionWorkspaceId)) {
|
||||
throw new Error("native_controller_migration_artifact_binding");
|
||||
}
|
||||
const unavailable = (): never => { throw new Error("native_controller_migration_artifact_unavailable"); };
|
||||
|
||||
@@ -20,7 +20,19 @@ function encryptParts(binding: Binding, records: ReadonlyMap<string, string>) {
|
||||
|
||||
export async function insertMigratedNativeRunnerCheckpoint(tx: CheckpointTransaction, binding: Binding, checkpoint: DurablePrpCheckpoint) {
|
||||
if (checkpoint.runId !== binding.runId) throw new Error("native_runner_checkpoint_binding_mismatch");
|
||||
await tx.insert(nativeRunnerCheckpoints).values(encryptParts(binding, checkpoint.records));
|
||||
let batch: ReturnType<typeof encryptParts> = [];
|
||||
let batchBytes = 0;
|
||||
for (const part of encryptParts(binding, checkpoint.records)) {
|
||||
const bytes = Buffer.byteLength(JSON.stringify(part));
|
||||
if (batch.length && (batch.length >= 64 || batchBytes + bytes > 512 * 1024)) {
|
||||
await tx.insert(nativeRunnerCheckpoints).values(batch);
|
||||
batch = [];
|
||||
batchBytes = 0;
|
||||
}
|
||||
batch.push(part);
|
||||
batchBytes += bytes;
|
||||
}
|
||||
if (batch.length) await tx.insert(nativeRunnerCheckpoints).values(batch);
|
||||
}
|
||||
|
||||
function decryptPart(cipher: ReturnType<typeof createLocalEncryptedCipher>, binding: Binding,
|
||||
|
||||
Reference in new issue
Block a user