mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-31 06:04:11 +01:00
perf: avoid scanning unused provider pack trees during archive upload (#18340)
## Thinking Path > - Paperclip manages AI agents and their work. > - Native agents need verified provider files in remote sandboxes. > - A fresh sandbox can need the bundled provider-pack archive. > - The upload path also hashes the expanded controller tree, which it does not upload. > - That synchronous scan blocks the server event loop and delays other requests. > - This change verifies the archive source and the installed pack, and keeps source-tree checks for directory uploads. ## Linked Issues or Issue Description **What happened?** Provider-pack archive fallback scans and hashes the full expanded controller tree before reading the build archive. The archive is the source sent to the sandbox. The extra scan blocks the Node event loop for several seconds. An overlapping asynchronous operation can then appear to take as long as that scan. **Expected behavior** Verify the bytes used by the selected upload path. Keep full verification of the installed pack against the selected manifest before a provider can use it. **Steps to reproduce** 1. Use a packaged provider pack with a build archive. 2. Start a remote native run whose sandbox needs that pack. 3. Inspect `provider_pack.archive.verify` and event-loop delay on the controller. Related: #18308 adds timing detail for workspace preparation. A search found no duplicate archive-verification fix. ## What Changed - Use the verified archive without scanning the unused expanded directory. - Keep full directory verification when no archive exists or the remote lacks archive support. - Keep archive digest checks and full verification of installed bytes. - Add `provider_pack.directory.verify` for directory fallback. - Add regression cases for both archive formats, fallback, and final verification failure. ## Verification - Both archive regressions failed before the change and passed after it. - 80 focused archive, selection, and verification cases pass on the rebased branch. - 20 native manifest and admission cases pass. - Server typecheck, server build, and the runtime bundle pass. - A disposable sandbox from the recorded image exercised the real uploader and full installed-pack verifier. It accepted the unchanged build bytes and rejected an altered proxy file. No model was called. The sandbox was deleted and its absence was verified. - A local baseline/archive/archive/baseline probe used the same unchanged public build pack. Preparation took 30.673 s, 84 ms, 106 ms, and 5.371 s. The slow baseline outlier is retained. This measures one controller phase; it does not establish a whole-turn or hosted speedup. ## Risks The archive path no longer checks expanded files that it does not send. Archive checks still fail closed, and the installed files must pass the same full manifest verification. Directory fallback keeps its source-tree check. Installs without archives retain that synchronous scan. The local sandbox probe tests upload and installed-content checks; it does not qualify admission of an older pack by a newer controller. ## Model Used OpenAI Codex, GPT-6 family, with tool use and local code execution. The exact runtime model ID and context-window size 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) --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
6d61990692
commit
ebd05e9f4c
4 files changed
+115
-34
No files matched your search
@@ -611,6 +611,16 @@ They are children of the existing pack selection/upload path and also appear in
|
||||
the local run timing log. Their fixed names add no paths, credentials, or content
|
||||
attributes. The operator endpoint gate remains unchanged. Source installs without
|
||||
prebuilt archives retain the existing directory upload span.
|
||||
`provider_pack.directory.verify` measures expanded source verification when
|
||||
directory staging is required.
|
||||
|
||||
Archive installation verifies the archive bytes and then the installed pack
|
||||
against the selected manifest. It does not hash the unused expanded controller
|
||||
tree. Directory installation still verifies that tree before upload, including
|
||||
when the remote cannot extract an available archive. The final installed-pack
|
||||
verification applies to both paths. A slow controller tree scan can block the
|
||||
server event loop and inflate overlapping asynchronous span durations; a copy
|
||||
span alone does not establish that the disk spent all that time copying.
|
||||
|
||||
The native provider lifecycle emits one foldable OpenTelemetry trace, linked to
|
||||
the server attempt described above. This is
|
||||
|
||||
@@ -65,7 +65,7 @@ import { nativeCompletionFeedback } from "./native-completion-feedback.js";
|
||||
import { hasAcknowledgedNativeReassignmentStopIntent, hasAcknowledgedNativeStopIntent } from "../acknowledged-native-stop.js";
|
||||
import { stoppedCodexTurnIsTextOnly } from "./stopped-codex-turn.js";
|
||||
import { prepareVerifiedRemoteProviderPack, useVerifiedPreinstalledRemoteProviderPack, shouldUseBundledAcpxImageAssets } from "./remote-provider-pack.js";
|
||||
import { readProviderPackArchive, stageProviderPackArchive } from "./provider-pack-archive.js";
|
||||
import { uploadVerifiedProviderPack } from "./provider-pack-upload.js";
|
||||
import { selectRemotePiCompanion } from "./remote-pi-companion.js";
|
||||
import { readNativeLocalProcessStop, PROCESS_START_REQUESTED, withReleasedNativeRunProcess } from "../native-local-process-stop.js";
|
||||
import { remoteLeaseCleanupScope } from "../remote-execution-termination.js";
|
||||
@@ -12740,46 +12740,24 @@ async function createRunnerdBackendWithinSessionClaim(
|
||||
"runner_remote_provider_artifact_incompatible: this remote transport cannot stage a provider pack; preinstall the exact manifest-matched pack",
|
||||
);
|
||||
}
|
||||
const archive = await measureNativeRunnerSpan(input.trace, "provider_pack.archive.verify", () => {
|
||||
readRemoteProviderPackManifest(configuredProviderPackRoot);
|
||||
return readProviderPackArchive(configuredProviderPackRoot, expectedProviderPackManifest!.digest);
|
||||
},
|
||||
);
|
||||
const escapedPackRoot = stagedRemoteProviderPackRoot.replaceAll(
|
||||
"'",
|
||||
"'\\''",
|
||||
);
|
||||
const cleared = await measureNativeRunnerSpan(input.trace, "provider_pack.clear_stale", () => remoteCommandRunner.execute({
|
||||
command: "sh",
|
||||
args: ["-c", `rm -rf '${escapedPackRoot}'`],
|
||||
await uploadVerifiedProviderPack({
|
||||
root: configuredProviderPackRoot,
|
||||
expectedDigest: expectedProviderPackManifest!.digest,
|
||||
runner: remoteCommandRunner,
|
||||
cwd: remoteTarget.remoteCwd,
|
||||
bypassSession: true,
|
||||
timeoutMs: 10_000,
|
||||
}));
|
||||
if (cleared.exitCode !== 0 || cleared.timedOut) {
|
||||
throw new Error(
|
||||
"runner_remote_provider_artifact_incompatible: stale provider pack could not be replaced",
|
||||
);
|
||||
}
|
||||
await measureNativeRunnerSpan(input.trace, "provider_pack.upload", async () => {
|
||||
if (archive && await stageProviderPackArchive({
|
||||
archive,
|
||||
runner: remoteCommandRunner,
|
||||
targetPath: stagedRemoteProviderPackRoot,
|
||||
reuseRoots: preinstalledProviderPackRoots,
|
||||
trace: input.trace,
|
||||
})) return;
|
||||
await stageRemoteRunnerDirectory({
|
||||
targetPath: stagedRemoteProviderPackRoot,
|
||||
reuseRoots: preinstalledProviderPackRoots,
|
||||
trace: input.trace,
|
||||
verifyDirectory: () => { readRemoteProviderPackManifest(configuredProviderPackRoot); },
|
||||
stageDirectory: () => stageRemoteRunnerDirectory({
|
||||
target: remoteTarget,
|
||||
runner: remoteCommandRunner,
|
||||
sourcePath: configuredProviderPackRoot,
|
||||
targetPath: stagedRemoteProviderPackRoot,
|
||||
mode: 0o700,
|
||||
});
|
||||
}),
|
||||
verifyUploaded: () => verifyRemoteProviderPack(stagedRemoteProviderPackRoot),
|
||||
});
|
||||
await measureNativeRunnerSpan(input.trace, "provider_pack.verify_uploaded", () =>
|
||||
verifyRemoteProviderPack(stagedRemoteProviderPackRoot),
|
||||
);
|
||||
activeRemoteProviderPackRoot = stagedRemoteProviderPackRoot;
|
||||
},
|
||||
});
|
||||
|
||||
@@ -7,6 +7,7 @@ import { promisify } from "node:util";
|
||||
import { afterEach, expect, it, vi } from "vitest";
|
||||
import type { CommandManagedRuntimeRunner } from "@paperclipai/adapter-utils/command-managed-runtime";
|
||||
import { readProviderPackArchive, stageProviderPackArchive } from "./provider-pack-archive.js";
|
||||
import { uploadVerifiedProviderPack } from "./provider-pack-upload.js";
|
||||
// @ts-expect-error Build-time script has no TypeScript declaration.
|
||||
import { createProviderPackArchive, createProviderPackLayers } from "../../../../scripts/create-provider-pack-archive.mjs";
|
||||
|
||||
@@ -211,6 +212,54 @@ it("keeps the directory fallback when the remote has no zstd", async () => {
|
||||
expect(remote.syncIn).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.skipIf(!tar || !hasZstd).each(["v1", "v2"])("installs verified %s archive bytes without reading the unused expanded pack", async version => {
|
||||
const { root, pack } = version === "v1" ? await fixture() : await layeredFixture();
|
||||
const remote = await runner(root);
|
||||
const targetPath = path.join(root, "uploaded");
|
||||
const expected = await fs.readFile(path.join(pack, "bin/tool"));
|
||||
// Only the archive is uploaded. The expanded tree is a different source,
|
||||
// and deliberately no longer matches the bytes sealed in that archive.
|
||||
await fs.writeFile(path.join(pack, "bin/tool"), "unused expanded bytes");
|
||||
const verifyDirectory = vi.fn(() => { throw Error("expanded_directory_was_read"); });
|
||||
const stageDirectory = vi.fn(async () => { throw Error("directory_was_uploaded"); });
|
||||
const verifyUploaded = vi.fn(async () => {
|
||||
expect(await fs.readFile(path.join(targetPath, "bin/tool"))).toEqual(expected);
|
||||
expect(JSON.parse(await fs.readFile(path.join(targetPath, "provider-pack.json"), "utf8")).digest).toBe(digest);
|
||||
});
|
||||
await uploadVerifiedProviderPack({ root: pack, expectedDigest: digest, runner: remote, cwd: root,
|
||||
targetPath, reuseRoots: [], verifyDirectory, stageDirectory, verifyUploaded });
|
||||
expect(verifyDirectory).not.toHaveBeenCalled();
|
||||
expect(stageDirectory).not.toHaveBeenCalled();
|
||||
expect(verifyUploaded).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it.each(["absent", "unsupported"])("verifies directory bytes before fallback when the archive is %s", async mode => {
|
||||
const { root, pack } = await fixture();
|
||||
if (mode === "absent") await fs.unlink(`${pack}.archive.json`);
|
||||
const remote = { syncIn: vi.fn(), execute: vi.fn().mockResolvedValue({ exitCode: 0, timedOut: false, stdout: "", stderr: "" }) };
|
||||
// The capability probe fails; clearing the old pack succeeds.
|
||||
remote.execute.mockResolvedValueOnce({ exitCode: 0, timedOut: false });
|
||||
remote.execute.mockResolvedValue({ exitCode: 127, timedOut: false });
|
||||
const stageDirectory = vi.fn(), verifyUploaded = vi.fn();
|
||||
await expect(uploadVerifiedProviderPack({ root: pack, expectedDigest: digest, runner: remote, cwd: root,
|
||||
targetPath: path.join(root, "uploaded"), reuseRoots: [], verifyDirectory: () => { throw Error("directory_digest_mismatch"); },
|
||||
stageDirectory, verifyUploaded })).rejects.toThrow("directory_digest_mismatch");
|
||||
expect(remote.syncIn).not.toHaveBeenCalled();
|
||||
expect(stageDirectory).not.toHaveBeenCalled();
|
||||
expect(verifyUploaded).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.skipIf(!tar || !hasZstd)("rejects an installed pack that fails final verification", async () => {
|
||||
const { root, pack } = await fixture();
|
||||
const remote = await runner(root), targetPath = path.join(root, "uploaded");
|
||||
const verifyUploaded = vi.fn(async () => { throw Error("installed_digest_mismatch"); });
|
||||
await expect(uploadVerifiedProviderPack({ root: pack, expectedDigest: digest, runner: remote, cwd: root,
|
||||
targetPath, reuseRoots: [], verifyDirectory: () => {}, stageDirectory: async () => {}, verifyUploaded }))
|
||||
.rejects.toThrow("installed_digest_mismatch");
|
||||
expect(remote.syncIn).toHaveBeenCalledOnce();
|
||||
expect(verifyUploaded).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it.skipIf(!tar || !hasZstd).each([
|
||||
["v1", "bsd-tar"], ["v2", "bsd-tar"],
|
||||
["v1", "older-gnu-tar"], ["v2", "older-gnu-tar"],
|
||||
|
||||
@@ -0,0 +1,44 @@
|
||||
import type { CommandManagedRuntimeRunner } from "@paperclipai/adapter-utils/command-managed-runtime";
|
||||
import type { NativeRunTrace } from "./native-run-trace.js";
|
||||
import { readProviderPackArchive, stageProviderPackArchive } from "./provider-pack-archive.js";
|
||||
|
||||
const quote = (value: string) => `'${value.replaceAll("'", "'\\''")}'`;
|
||||
|
||||
/** Upload a pack and verify the installed bytes before exposing it to a provider. */
|
||||
export async function uploadVerifiedProviderPack(input: {
|
||||
root: string;
|
||||
expectedDigest: string;
|
||||
runner: CommandManagedRuntimeRunner;
|
||||
cwd: string;
|
||||
targetPath: string;
|
||||
reuseRoots: readonly string[];
|
||||
trace?: NativeRunTrace;
|
||||
verifyDirectory: () => void;
|
||||
stageDirectory: () => Promise<void>;
|
||||
verifyUploaded: () => Promise<void>;
|
||||
}): Promise<void> {
|
||||
const measure = <T>(name: string, fn: () => Promise<T>) => input.trace ? input.trace.measure(name, fn) : fn();
|
||||
const verifyDirectory = () => measure("provider_pack.directory.verify", async () => input.verifyDirectory());
|
||||
// Archive installation reads the sealed archives, not the expanded tree.
|
||||
// Hash the directory only when it is actually the upload source. The
|
||||
// installed pack still passes full verification against the selected identity.
|
||||
const archive = await measure("provider_pack.archive.verify", () =>
|
||||
readProviderPackArchive(input.root, input.expectedDigest));
|
||||
if (!archive) await verifyDirectory();
|
||||
const cleared = await measure("provider_pack.clear_stale", () => input.runner.execute({
|
||||
command: "sh", args: ["-c", `rm -rf ${quote(input.targetPath)}`],
|
||||
cwd: input.cwd, bypassSession: true, timeoutMs: 10_000,
|
||||
}));
|
||||
if (cleared.exitCode !== 0 || cleared.timedOut) {
|
||||
throw new Error("runner_remote_provider_artifact_incompatible: stale provider pack could not be replaced");
|
||||
}
|
||||
await measure("provider_pack.upload", async () => {
|
||||
if (archive && await stageProviderPackArchive({
|
||||
archive, runner: input.runner, targetPath: input.targetPath,
|
||||
reuseRoots: input.reuseRoots, trace: input.trace,
|
||||
})) return;
|
||||
if (archive) await verifyDirectory();
|
||||
await input.stageDirectory();
|
||||
});
|
||||
await measure("provider_pack.verify_uploaded", input.verifyUploaded);
|
||||
}
|
||||
Reference in new issue
Block a user