Repository navigation
fix(fleet): resume interrupted CLI-owned sandboxes #1878
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1245,7 +1245,7 @@ describe('fleet command support', () => { | |
| }); | ||
| }); | ||
|
|
||
| it('plain --sandbox materializes the inferred repository through Relayfile and starts in its live relative cwd', async () => { | ||
| it('plain --sandbox resumes an interrupted owned provision and starts in its live relative cwd', async () => { | ||
| vi.stubEnv('RELAY_AGENT_TOKEN', undefined); | ||
| const revision = '0123456789abcdef0123456789abcdef01234567'; | ||
| const repositorySelection = { | ||
|
|
@@ -1269,8 +1269,17 @@ describe('fleet command support', () => { | |
| sentinelPath: '/github/repos/AgentWorkforce/cloud/.relayfile/clone.json', | ||
| }; | ||
| }); | ||
| const ensureCloudFleetSandbox = vi.fn(async () => { | ||
| const ensureCloudFleetSandbox = vi.fn(async (input: { sandboxId?: string; name?: string }) => { | ||
| events.push('ensure'); | ||
| if (ensureCloudFleetSandbox.mock.calls.length === 1) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: This test only covers the single-resume-success path. The new retry logic's other branches — no retry for caller-supplied Prompt for AI agents |
||
| throw new CloudFleetSandboxProvisionError('gateway timed out', { | ||
| cloudWorkspaceId: 'cloud-workspace', | ||
| sandboxId: input.sandboxId, | ||
| nodeName: input.name, | ||
| providerId: 'agent37', | ||
| outcomeUnknown: true, | ||
| }); | ||
| } | ||
| return { | ||
| outcome: 'provisioned' as const, | ||
| providerId: 'agent37' as const, | ||
|
|
@@ -1346,7 +1355,7 @@ describe('fleet command support', () => { | |
| { from: 'user' } | ||
| ); | ||
|
|
||
| expect(events).toEqual(['materialize', 'ensure', 'spawn']); | ||
| expect(events).toEqual(['materialize', 'ensure', 'ensure', 'spawn']); | ||
| expect(materializeCloudRelayfileRepository).toHaveBeenCalledWith({ | ||
| workspaceId: 'rw_abc', | ||
| repository: 'AgentWorkforce/cloud', | ||
|
|
@@ -1363,6 +1372,11 @@ describe('fleet command support', () => { | |
| ], | ||
| }) | ||
| ); | ||
| expect(ensureCloudFleetSandbox).toHaveBeenCalledTimes(2); | ||
| expect(ensureCloudFleetSandbox.mock.calls[1]?.[0]).toMatchObject({ | ||
| sandboxId: ensureCloudFleetSandbox.mock.calls[0]?.[0].sandboxId, | ||
| name: ensureCloudFleetSandbox.mock.calls[0]?.[0].name, | ||
| }); | ||
|
Comment on lines
+1376
to
+1379
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Assert that the replay carries a sandbox identity. This assertion compares two optional fields. If both requests omit 🤖 Prompt for AI AgentsSource: Learnings |
||
| const ensureInput = ensureCloudFleetSandbox.mock.calls[0]?.[0] as Record<string, unknown>; | ||
| expect(ensureInput.repos).toBeUndefined(); | ||
| expect(ensureInput.repoRevisions).toBeUndefined(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -94,6 +94,7 @@ const FLEET_CLIS = new Set([ | |||||||||||||||
| const MIN_VERIFIED_CONFIRM_TIMEOUT_MS = 95_000; | ||||||||||||||||
| const CLOUD_SANDBOX_ID_PATTERN = | ||||||||||||||||
| /^sbx_[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/; | ||||||||||||||||
| const MAX_OWNED_SANDBOX_RESUME_ATTEMPTS = 3; | ||||||||||||||||
|
|
||||||||||||||||
| function spawnInvocationWithPlacement(invocation: Record<string, unknown>): Record<string, unknown> { | ||||||||||||||||
| return { ...invocation, placement: spawnPlacementReceipt(invocation) }; | ||||||||||||||||
|
|
@@ -238,6 +239,30 @@ export interface FleetCommandDependencies { | |||||||||||||||
| retireOwnedBindings: typeof retireOwnedIntegrationBindings; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| async function ensureOwnedCloudFleetSandbox( | ||||||||||||||||
| deps: Pick<FleetCommandDependencies, 'ensureCloudFleetSandbox' | 'warn'>, | ||||||||||||||||
| input: Parameters<typeof ensureCloudFleetSandbox>[0], | ||||||||||||||||
| ownedSandboxId: string | undefined | ||||||||||||||||
| ): Promise<EnsureCloudFleetSandboxResult> { | ||||||||||||||||
| for (let resumeAttempt = 0; ; resumeAttempt += 1) { | ||||||||||||||||
| try { | ||||||||||||||||
| return await deps.ensureCloudFleetSandbox(input); | ||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift 🔎 Supported by static analysis🏁 Script executed: rg -n "reused|ensureOwnedCloudFleetSandbox|cleanup|ownedSandboxId" packages/cli/src/cli/commands/fleet.ts | head -80
sed -n '239,270p;900,960p' packages/cli/src/cli/commands/fleet.ts
rg -n "reused" packages/cloud/src/fleet-sandbox.ts | head -30Repository: AgentWorkforce/relay Length of output: 5865 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- fleet.ts ownership, retry, spawn, and cleanup ---'
sed -n '220,275p;700,840p;850,915p;915,940p;1025,1090p' packages/cli/src/cli/commands/fleet.ts
printf '%s\n' '--- fleet-sandbox.ts result types and response mapping ---'
sed -n '150,205p;640,735p' packages/cloud/src/fleet-sandbox.ts
printf '%s\n' '--- relevant Cloud ensure callers and error construction ---'
rg -n -C 5 "CloudFleetSandboxProvisionError|ensureCloudFleetSandbox\\(|outcomeUnknown|outcome === 'reused'|shouldCleanupSandbox|ownedSandboxId" packages/cli/src/cli/commands/fleet.ts packages/cloud/src/fleet-sandbox.ts packages/cli/src/cli -g '*.ts'Repository: AgentWorkforce/relay Length of output: 42211 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- exact Cloud normalize branches and error classification ---'
sed -n '680,790p' packages/cloud/src/fleet-sandbox.ts
sed -n '790,940p' packages/cloud/src/fleet-sandbox.ts
printf '%s\n' '--- reused and resume tests ---'
rg -n -C 12 "outcome: 'reused'|reused|resum|same sandbox|outcomeUnknown" packages/cloud packages/cli/src/cli/commands/fleet.test.ts -g '*.ts'Repository: AgentWorkforce/relay Length of output: 42478 🏁 Script executed: #!/bin/bash
set -e
rg -n -C 5 "fleet/nodes/sandbox/ensure|outcome.*reused|reused.*outcome|sandboxId.*reused|forceProvision" . \
-g '*.ts' -g '*.tsx' -g '*.js' -g '*.md' -g '*.json' \
-g '!node_modules' -g '!dist' -g '!build'Repository: AgentWorkforce/relay Length of output: 44172 Handle If the first owned request is accepted but its response is interrupted, the CLI marks the outcome unknown and retries the same Cleanup also skips 🤖 Prompt for AI Agents |
||||||||||||||||
| } catch (error) { | ||||||||||||||||
| if ( | ||||||||||||||||
| ownedSandboxId === undefined || | ||||||||||||||||
| !(error instanceof CloudFleetSandboxProvisionError) || | ||||||||||||||||
| !error.outcomeUnknown || | ||||||||||||||||
|
Comment on lines
+252
to
+254
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P1: Only replay errors tied to the same requested identity. A non-OK response with a different sandbox ID is marked outcome-unknown but has no trusted Prompt for AI agents
Suggested change
|
||||||||||||||||
| resumeAttempt >= MAX_OWNED_SANDBOX_RESUME_ATTEMPTS | ||||||||||||||||
| ) { | ||||||||||||||||
| throw error; | ||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P1: Preserve the first outcome-unknown error when a resume attempt fails with an ordinary error. Otherwise the CLI can exit without replay instructions for a sandbox that may already be running. Prompt for AI agents |
||||||||||||||||
| } | ||||||||||||||||
| deps.warn( | ||||||||||||||||
| `Cloud interrupted preparation of caller-owned sandbox '${ownedSandboxId}'; resuming the same sandbox (${resumeAttempt + 1}/${MAX_OWNED_SANDBOX_RESUME_ATTEMPTS}).` | ||||||||||||||||
| ); | ||||||||||||||||
| } | ||||||||||||||||
|
Comment on lines
+250
to
+262
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Failed resume loses sandbox identity After an interrupted ensure, Learn moreA sandbox spawned without Example: Cloud accepts Recommended fix: Retain the first outcome-unknown error or its owned ID in Was this helpful? React with 👍 or 👎 to provide feedback. |
||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| function withFleetDefaults(overrides: Partial<FleetCommandDependencies> = {}): FleetCommandDependencies { | ||||||||||||||||
| const core = overrides.core ?? withDefaults(); | ||||||||||||||||
| const sdk = overrides.sdk ?? withSdkDefaults(); | ||||||||||||||||
|
|
@@ -714,27 +739,31 @@ export function registerFleetCommands( | |||||||||||||||
| ? 'long-running-agent' | ||||||||||||||||
| : 'standard-long-running-agent'; | ||||||||||||||||
| try { | ||||||||||||||||
| sandbox = await deps.ensureCloudFleetSandbox({ | ||||||||||||||||
| workspaceId: relayWorkspaceId, | ||||||||||||||||
| requiredCapability: `spawn:${cli}`, | ||||||||||||||||
| maxAgents: 1, | ||||||||||||||||
| mountRelayfile: mountSandboxRelayfile, | ||||||||||||||||
| ...(liveRepository | ||||||||||||||||
| ? { relayfilePaths: liveRelayfileMountPaths(liveRepository, sandboxRelayfilePaths) } | ||||||||||||||||
| : sandboxRelayfilePaths === undefined | ||||||||||||||||
| ? {} | ||||||||||||||||
| : { relayfilePaths: sandboxRelayfilePaths }), | ||||||||||||||||
| ...(sandboxId === undefined ? {} : { sandboxId }), | ||||||||||||||||
| forceProvision: true, | ||||||||||||||||
| ...(sandboxProvider === undefined ? {} : { providerId: sandboxProvider }), | ||||||||||||||||
| workloadProfile, | ||||||||||||||||
| waitTimeoutMs: 90_000, | ||||||||||||||||
| ...(effectiveSandboxName === undefined ? {} : { name: effectiveSandboxName }), | ||||||||||||||||
| ...(checkoutRepository && sandboxRepository ? { repos: [sandboxRepository.repository] } : {}), | ||||||||||||||||
| ...(checkoutRepository && sandboxRepository | ||||||||||||||||
| ? { repoRevisions: { [sandboxRepository.repository]: sandboxRepository.revision } } | ||||||||||||||||
| : {}), | ||||||||||||||||
| }); | ||||||||||||||||
| sandbox = await ensureOwnedCloudFleetSandbox( | ||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P1: The retry can turn a first unknown provision into Prompt for AI agents |
||||||||||||||||
| deps, | ||||||||||||||||
| { | ||||||||||||||||
| workspaceId: relayWorkspaceId, | ||||||||||||||||
| requiredCapability: `spawn:${cli}`, | ||||||||||||||||
| maxAgents: 1, | ||||||||||||||||
| mountRelayfile: mountSandboxRelayfile, | ||||||||||||||||
| ...(liveRepository | ||||||||||||||||
| ? { relayfilePaths: liveRelayfileMountPaths(liveRepository, sandboxRelayfilePaths) } | ||||||||||||||||
| : sandboxRelayfilePaths === undefined | ||||||||||||||||
| ? {} | ||||||||||||||||
| : { relayfilePaths: sandboxRelayfilePaths }), | ||||||||||||||||
| ...(sandboxId === undefined ? {} : { sandboxId }), | ||||||||||||||||
| forceProvision: true, | ||||||||||||||||
| ...(sandboxProvider === undefined ? {} : { providerId: sandboxProvider }), | ||||||||||||||||
| workloadProfile, | ||||||||||||||||
| waitTimeoutMs: 90_000, | ||||||||||||||||
| ...(effectiveSandboxName === undefined ? {} : { name: effectiveSandboxName }), | ||||||||||||||||
| ...(checkoutRepository && sandboxRepository ? { repos: [sandboxRepository.repository] } : {}), | ||||||||||||||||
| ...(checkoutRepository && sandboxRepository | ||||||||||||||||
| ? { repoRevisions: { [sandboxRepository.repository]: sandboxRepository.revision } } | ||||||||||||||||
| : {}), | ||||||||||||||||
| }, | ||||||||||||||||
| shouldCleanupSandbox ? sandboxId : undefined | ||||||||||||||||
| ); | ||||||||||||||||
| assertSandboxRepositoryRevision(sandbox, checkoutRepository ? sandboxRepository : undefined); | ||||||||||||||||
| } catch (error) { | ||||||||||||||||
| if ( | ||||||||||||||||
|
|
||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P3: This bullet describes an unbounded automatic resume, but recovery is limited to three attempts (
MAX_OWNED_SANDBOX_RESUME_ATTEMPTS = 3in packages/cli/src/cli/commands/fleet.ts): after the third interrupted response the CLI still warns and requires the manual--sandbox-idreplay. State the concrete bound in the entry so users know resume is not indefinite.Prompt for AI agents