mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-11 05:11:49 +01:00
feat: parallelize sandbox file-sync behind a provider opt-in capability (#11736)
## Thinking Path > - Paperclip runs AI agents through local and remote execution adapters. > - Sandbox providers move workspace and asset files before and after agent runs. > - Serial file transfers delay startup and teardown when several operations do not depend on each other. > - Providers need an opt-in contract so existing providers keep their serial behavior. > - This pull request adds a bounded scheduler and routes inbound and outbound sync operations through it. > - The benefit is shorter sandbox setup and teardown with stable errors, clear telemetry, and a safe opt-in path. ## Linked Issues or Issue Description **Subsystem affected** Cross-cutting (multiple of the above): packages/shared, packages/adapter-utils, packages/plugins, and server. **Problem or motivation** Sandbox sync processes the workspace, assets, and referenced projects in series. This adds avoidable wait time to agent startup and teardown. **Proposed solution** Add a fail-closed provider capability named concurrentSyncOperations. Use a bounded scheduler with a limit of four operations. Preserve operation order for error reporting. Keep non-opted-in providers on the serial path. **Alternatives considered** Increase the serial transfer speed or add provider-specific schedulers. Those options do not provide one shared contract or stable behavior across providers. **Roadmap alignment** ROADMAP.md lists cloud and sandbox agents as a product area. This change improves sandbox execution without changing the control-plane contract. **Additional context** The Daytona provider opts in. Board trials on this commit showed overlap for inbound sync and outbound restore, with no referenced-project staging failures. ## What Changed - Add the concurrentSyncOperations sandbox capability and fail-closed parsing. - Add a bounded settle-all scheduler with stable input-order errors. - Parallelize inbound workspace, asset, and referenced-project sync operations when the provider opts in. - Parallelize outbound workspace and asset restore operations when the provider opts in. - Surface referenced-project failure text in run logs and server telemetry. - Add Daytona sync spans and the capability declaration. - Preserve in-flight upload scratch tarballs during workspace wipe. - Add unit and regression tests for the scheduler, coordinators, provider behavior, telemetry, and wipe race. ## Verification - Run the adapter-utils and server type checks. - Run the targeted adapter-utils, server, and Daytona test suites. - Run the full automated sweep. - Review six cold Daytona trials, with three serial and three parallel runs. - Confirm that parallel trials show inbound overlap and outbound restore overlap. - Confirm that providers without the capability keep serial behavior. ## Risks - Providers must opt in only when their file operations can run safely at the same time. - A provider that declares the capability incorrectly can expose transfer races. - The scheduler keeps a limit of four to bound resource use. - Providers without the capability keep the prior serial behavior. ## Model Used OpenAI GPT-5 in the Codex runtime. The model used tool calls, code inspection, and GitHub workflow support. The model did not author the implementation commits. ## 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 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
a5f3ac6b48
commit
233be4b36c
30 files changed
+3032
-478
No files matched your search
@@ -19,6 +19,9 @@ const SNAPSHOT: EffectiveSandboxCapabilities = {
|
||||
persistentProcessSessions: true,
|
||||
independentControlCommands: false,
|
||||
incrementalSessionOutput: false,
|
||||
// Concurrent sync operations need BOTH sync verbs; this snapshot verified only
|
||||
// inbound sync, so the opt-in stays off.
|
||||
concurrentSyncOperations: false,
|
||||
};
|
||||
|
||||
// A snapshot that grants every capability. A test overrides one flag to prove
|
||||
@@ -30,6 +33,7 @@ const FULL_GRANT: EffectiveSandboxCapabilities = {
|
||||
persistentProcessSessions: true,
|
||||
independentControlCommands: true,
|
||||
incrementalSessionOutput: true,
|
||||
concurrentSyncOperations: true,
|
||||
};
|
||||
|
||||
// Build a sandbox execution target with a fixed snapshot and a fixed
|
||||
@@ -185,6 +189,42 @@ describe("effective snapshot gates the sync decision", () => {
|
||||
});
|
||||
});
|
||||
|
||||
// The execution target carries the concurrent-sync opt-in on both the effective
|
||||
// snapshot and the runner. The runner Boolean feeds the sync client, which then
|
||||
// tells the orchestrator whether it may run sync operations concurrently.
|
||||
describe("effective snapshot carries the concurrent-sync capability", () => {
|
||||
beforeEach(() => {
|
||||
mockResolveEnvironmentDriverConfigForRuntime.mockReset();
|
||||
});
|
||||
|
||||
it("carries the concurrent-sync opt-in on the snapshot and the runner", async () => {
|
||||
const { target } = await buildSandboxTarget({ snapshot: FULL_GRANT, supportsSync: true });
|
||||
expect(target.effectiveCapabilities?.concurrentSyncOperations).toBe(true);
|
||||
expect(target.runner?.allowConcurrentSyncOperations).toBe(true);
|
||||
});
|
||||
|
||||
it("keeps the concurrent-sync opt-in off when the snapshot removes it", async () => {
|
||||
const { target } = await buildSandboxTarget({
|
||||
snapshot: { ...FULL_GRANT, concurrentSyncOperations: false },
|
||||
supportsSync: true,
|
||||
});
|
||||
expect(target.effectiveCapabilities?.concurrentSyncOperations).toBe(false);
|
||||
expect(target.runner?.allowConcurrentSyncOperations).toBe(false);
|
||||
});
|
||||
|
||||
it("keeps the concurrent-sync opt-in off when the resolution rejects", async () => {
|
||||
// A rejected resolution carries no snapshot; it must not read as an open
|
||||
// grant. The runner keeps the opt-in off.
|
||||
const { target } = await buildSandboxTarget({
|
||||
snapshot: null,
|
||||
supportsSync: true,
|
||||
rejectResolution: true,
|
||||
});
|
||||
expect(target.effectiveCapabilities).toBeUndefined();
|
||||
expect(target.runner?.allowConcurrentSyncOperations).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
// Session-output streaming is decided downstream from the carried snapshot
|
||||
// alone (see `streamAgentSessionOutput` in `acpx-engine/execute.ts`): the bridge
|
||||
// streams only when the snapshot grants `incrementalSessionOutput`. That key is
|
||||
|
||||
@@ -1019,17 +1019,18 @@ describe("buildReferencedProjectRunObservability", () => {
|
||||
failures: [
|
||||
{ projectId: "project-b", reason: "authorization" },
|
||||
{ projectId: "project-c", reason: "resolution" },
|
||||
{ projectId: "project-d", reason: "staging" },
|
||||
{ projectId: "project-d", reason: "staging", error: "extract failed: boom" },
|
||||
],
|
||||
});
|
||||
|
||||
// Requested is the synced count plus every dropped project, so the counts reconcile.
|
||||
expect(observability.referenced_projects_requested).toBe(4);
|
||||
expect(observability.referenced_projects_synced).toBe(1);
|
||||
// A staging failure carries its error message; a failure without one omits the field.
|
||||
expect(observability.referenced_project_failures).toEqual([
|
||||
{ project_id: "project-b", reason: "authorization" },
|
||||
{ project_id: "project-c", reason: "resolution" },
|
||||
{ project_id: "project-d", reason: "staging" },
|
||||
{ project_id: "project-d", reason: "staging", error: "extract failed: boom" },
|
||||
]);
|
||||
});
|
||||
|
||||
|
||||
@@ -311,4 +311,21 @@ describe("clampProviderSpanAttributes", () => {
|
||||
// A non-finite number yields no attribute; `exec.command` is not allowed.
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps the transfer direction for each closed-set value", () => {
|
||||
expect(clampProviderSpanAttributes({ [A.transferDirection]: "inbound" })).toEqual({
|
||||
[A.transferDirection]: "inbound",
|
||||
});
|
||||
expect(clampProviderSpanAttributes({ [A.transferDirection]: "outbound" })).toEqual({
|
||||
[A.transferDirection]: "outbound",
|
||||
});
|
||||
});
|
||||
|
||||
it("drops a transfer direction outside the closed set or of the wrong type", () => {
|
||||
// A free-form string, an empty string, and a non-string all yield no
|
||||
// attribute, so the direction stays bounded and low-cardinality.
|
||||
expect(clampProviderSpanAttributes({ [A.transferDirection]: "sideways" })).toEqual({});
|
||||
expect(clampProviderSpanAttributes({ [A.transferDirection]: "" })).toEqual({});
|
||||
expect(clampProviderSpanAttributes({ [A.transferDirection]: 1 })).toEqual({});
|
||||
});
|
||||
});
|
||||
@@ -290,6 +290,40 @@ describe("sandbox capability contract normalizer", () => {
|
||||
expect(effective.incrementalSessionOutput).toBe(false);
|
||||
});
|
||||
|
||||
it("test_concurrent_sync_operations_is_opt_in_and_needs_both_sync_verbs", () => {
|
||||
// Parallel bidirectional file sync is opt-in and direction-neutral. It needs
|
||||
// both sync verbs, so a provider that verifies only one direction cannot get
|
||||
// the capability. An absent declaration denies it even with both verbs.
|
||||
const undeclared = resolveEffectiveSandboxCapabilities({
|
||||
verifiedMethods: ["environmentSyncIn", "environmentSyncOut"],
|
||||
declared: null,
|
||||
});
|
||||
expect(undeclared.concurrentSyncOperations).toBe(false);
|
||||
|
||||
// A positive declaration with both verified verbs resolves true.
|
||||
const bothVerbs = resolveEffectiveSandboxCapabilities({
|
||||
verifiedMethods: ["environmentSyncIn", "environmentSyncOut"],
|
||||
declared: { concurrentSyncOperations: true },
|
||||
});
|
||||
expect(bothVerbs.concurrentSyncOperations).toBe(true);
|
||||
|
||||
// Only the inbound verb: the outbound prerequisite is missing, so it resolves
|
||||
// false.
|
||||
const inOnly = resolveEffectiveSandboxCapabilities({
|
||||
verifiedMethods: ["environmentSyncIn"],
|
||||
declared: { concurrentSyncOperations: true },
|
||||
});
|
||||
expect(inOnly.concurrentSyncOperations).toBe(false);
|
||||
|
||||
// Only the outbound verb: the inbound prerequisite is missing, so it resolves
|
||||
// false.
|
||||
const outOnly = resolveEffectiveSandboxCapabilities({
|
||||
verifiedMethods: ["environmentSyncOut"],
|
||||
declared: { concurrentSyncOperations: true },
|
||||
});
|
||||
expect(outOnly.concurrentSyncOperations).toBe(false);
|
||||
});
|
||||
|
||||
it("test_unknown_or_unavailable_verification_resolves_false", () => {
|
||||
const declaredAll = {
|
||||
reusableLeases: true,
|
||||
@@ -298,6 +332,7 @@ describe("sandbox capability contract normalizer", () => {
|
||||
persistentProcessSessions: true,
|
||||
independentControlCommands: true,
|
||||
incrementalSessionOutput: true,
|
||||
concurrentSyncOperations: true,
|
||||
};
|
||||
|
||||
for (const verifiedMethods of [null, undefined, [] as string[]]) {
|
||||
|
||||
@@ -317,6 +317,12 @@ export async function resolveEnvironmentExecutionTarget(input: {
|
||||
// here. The client falls back to the chunked upload path when this is
|
||||
// false.
|
||||
supportsSingleStreamStdinProgress: false,
|
||||
// Carry the verified concurrent-sync opt-in to the sync client. The
|
||||
// client copies it onto the native path and ignores it on the base64
|
||||
// fallback, which always permits concurrency. A null snapshot or a
|
||||
// provider that never opted in keeps it false, so an unverified
|
||||
// provider never permits concurrent sync operations.
|
||||
allowConcurrentSyncOperations: effectiveCapabilities?.concurrentSyncOperations === true,
|
||||
execute: async (commandInput) => {
|
||||
// Record true start and stop timestamps around the provider await,
|
||||
// so the exec span and the result carry a real wall time.
|
||||
|
||||
@@ -89,6 +89,7 @@ export const SANDBOX_CAPABILITY_KEYS = [
|
||||
"persistentProcessSessions",
|
||||
"independentControlCommands",
|
||||
"incrementalSessionOutput",
|
||||
"concurrentSyncOperations",
|
||||
] as const;
|
||||
|
||||
export type SandboxCapabilityKey = (typeof SANDBOX_CAPABILITY_KEYS)[number];
|
||||
@@ -104,6 +105,7 @@ export type SandboxCapabilityKey = (typeof SANDBOX_CAPABILITY_KEYS)[number];
|
||||
*/
|
||||
const SANDBOX_CAPABILITY_OPT_IN_KEYS: ReadonlySet<SandboxCapabilityKey> = new Set([
|
||||
"incrementalSessionOutput",
|
||||
"concurrentSyncOperations",
|
||||
]);
|
||||
|
||||
/**
|
||||
@@ -134,6 +136,12 @@ const SANDBOX_CAPABILITY_OPT_IN_KEYS: ReadonlySet<SandboxCapabilityKey> = new Se
|
||||
* provider tails the session log through it. The verified verb is necessary
|
||||
* but not sufficient: this key is opt-in, so the declaration is the real gate
|
||||
* (see {@link SANDBOX_CAPABILITY_OPT_IN_KEYS}).
|
||||
* - `concurrentSyncOperations` requires BOTH sync verbs, because parallel
|
||||
* bidirectional transfer runs an inbound and an outbound transfer at the same
|
||||
* time. The two verbs are separate required groups, so a provider that
|
||||
* verifies only one direction cannot get the capability. The verbs are
|
||||
* necessary but not sufficient: this key is opt-in, so the declaration is the
|
||||
* real gate (see {@link SANDBOX_CAPABILITY_OPT_IN_KEYS}).
|
||||
*/
|
||||
const SANDBOX_CAPABILITY_PREREQUISITE_METHODS: Record<SandboxCapabilityKey, readonly (readonly string[])[]> = {
|
||||
// Reusable leases require ALL reuse verbs. Each verb is its own required
|
||||
@@ -147,6 +155,7 @@ const SANDBOX_CAPABILITY_PREREQUISITE_METHODS: Record<SandboxCapabilityKey, read
|
||||
persistentProcessSessions: [["environmentExecute"]],
|
||||
independentControlCommands: [["environmentExecute"]],
|
||||
incrementalSessionOutput: [["environmentExecute"]],
|
||||
concurrentSyncOperations: [["environmentSyncIn"], ["environmentSyncOut"]],
|
||||
};
|
||||
|
||||
function capabilityIsVerified(
|
||||
@@ -230,6 +239,7 @@ export function resolveEffectiveSandboxCapabilities(input: {
|
||||
persistentProcessSessions: resolve("persistentProcessSessions"),
|
||||
independentControlCommands: resolve("independentControlCommands"),
|
||||
incrementalSessionOutput: resolve("incrementalSessionOutput"),
|
||||
concurrentSyncOperations: resolve("concurrentSyncOperations"),
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -2865,6 +2865,12 @@ export type ReferencedProjectFailureReason = "authorization" | "resolution" | "s
|
||||
export interface ReferencedProjectFailure {
|
||||
projectId: string;
|
||||
reason: ReferencedProjectFailureReason;
|
||||
/**
|
||||
* The failure message, when the layer that dropped the project produced one. A `staging` failure
|
||||
* carries the remote extract or sync error here, so a reader of the run log learns why the project
|
||||
* dropped. An `authorization` or `resolution` drop omits this field.
|
||||
*/
|
||||
error?: string;
|
||||
}
|
||||
|
||||
export interface ResolvedRunReferencedProjects {
|
||||
@@ -3209,6 +3215,8 @@ export interface ReferencedProjectRunObservability {
|
||||
referenced_project_failures: Array<{
|
||||
project_id: string;
|
||||
reason: ReferencedProjectFailureReason;
|
||||
/** The failure message for a `staging` drop; absent for an `authorization` or `resolution` drop. */
|
||||
error?: string;
|
||||
}>;
|
||||
}
|
||||
|
||||
@@ -3231,6 +3239,9 @@ export function buildReferencedProjectRunObservability(input: {
|
||||
referenced_project_failures: input.failures.map((failure) => ({
|
||||
project_id: failure.projectId,
|
||||
reason: failure.reason,
|
||||
// Carry the error only when the layer produced one, so an authorization or resolution drop
|
||||
// stays a two-field entry and a staging drop names its reason.
|
||||
...(failure.error !== undefined ? { error: failure.error } : {}),
|
||||
})),
|
||||
};
|
||||
}
|
||||
@@ -16067,6 +16078,7 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {})
|
||||
failures: referencedProjectStagingFailures.map((failure) => ({
|
||||
projectId: failure.projectId,
|
||||
reason: "staging" as const,
|
||||
error: failure.error,
|
||||
})),
|
||||
});
|
||||
logger.info(
|
||||
|
||||
@@ -545,6 +545,7 @@ const PROVIDER_SPAN_ATTR_ALLOWLIST: ReadonlySet<string> = new Set<string>([
|
||||
SPAN_ATTRS.packWallMs,
|
||||
SPAN_ATTRS.transferWallMs,
|
||||
SPAN_ATTRS.transferGuardCount,
|
||||
SPAN_ATTRS.transferDirection,
|
||||
]);
|
||||
|
||||
/** The subset of allowed keys that carry a finite number. */
|
||||
@@ -557,11 +558,15 @@ const PROVIDER_SPAN_NUMERIC_ATTRS: ReadonlySet<string> = new Set<string>([
|
||||
/** The closed value set for the `outcome` attribute. */
|
||||
const KNOWN_SPAN_OUTCOMES: ReadonlySet<string> = new Set(["ok", "skipped", "failed"]);
|
||||
|
||||
/** The closed value set for the `transfer.direction` attribute. */
|
||||
const KNOWN_TRANSFER_DIRECTIONS: ReadonlySet<string> = new Set(["inbound", "outbound"]);
|
||||
|
||||
/**
|
||||
* Re-clamp the worker-sent attributes at the trust boundary. Drop every key that
|
||||
* is not on the allowlist. Re-map `provider` through `normalizeProviderFamily`,
|
||||
* bound `outcome` to its closed set, and keep a numeric attribute only when it
|
||||
* is a finite number. The result holds only bounded, low-cardinality values.
|
||||
* bound `outcome` and `transfer.direction` each to its closed set, and keep a
|
||||
* numeric attribute only when it is a finite number. The result holds only
|
||||
* bounded, low-cardinality values.
|
||||
*/
|
||||
export function clampProviderSpanAttributes(
|
||||
raw: Record<string, unknown> | undefined,
|
||||
@@ -578,6 +583,10 @@ export function clampProviderSpanAttributes(
|
||||
if (typeof value === "string" && KNOWN_SPAN_OUTCOMES.has(value)) clamped[key] = value;
|
||||
continue;
|
||||
}
|
||||
if (key === SPAN_ATTRS.transferDirection) {
|
||||
if (typeof value === "string" && KNOWN_TRANSFER_DIRECTIONS.has(value)) clamped[key] = value;
|
||||
continue;
|
||||
}
|
||||
if (PROVIDER_SPAN_NUMERIC_ATTRS.has(key)) {
|
||||
if (typeof value === "number" && Number.isFinite(value)) clamped[key] = value;
|
||||
continue;
|
||||
|
||||
Reference in new issue
Block a user