Skip to content

Commit adde4fc

Browse files
mchwangclaude
andauthored
D: real-Docker export through a recovery handle after a restart (#51) (#109)
The last "Done when" item of #51: removal by recovery handle had a real-Docker case, but export by handle only ran against the fake Docker CLI. An earlier Node process now allocates real task storage and exits without releasing it; this process recovers it, exports through the handle (refused without F's baseline, failed with a wrong one, correct with the right one, storage unchanged, no export container left), then removes the storage by that handle. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent cce650b commit adde4fc

1 file changed

Lines changed: 58 additions & 0 deletions

File tree

‎test/agent-container.test.ts‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { createHash, randomBytes, randomUUID } from 'node:crypto';
44
import { chmodSync, existsSync, mkdtempSync, mkdirSync, readFileSync, renameSync, rmSync, statSync, symlinkSync, writeFileSync } from 'node:fs';
55
import { tmpdir } from 'node:os';
66
import { join } from 'node:path';
7+
import { pathToFileURL } from 'node:url';
78
import { afterAll, afterEach, beforeAll, describe, expect, it } from 'vitest';
89
import { captureInvocation, type InvocationInput, type Phase } from '../agents/contract.ts';
910
import { AGENT_IMAGE, assertBuiltAgentImage, buildAgentImage } from '../agents/container/image.ts';
@@ -2157,6 +2158,63 @@ describe('real Docker agent isolation', () => {
21572158
}
21582159
}, 180_000);
21592160

2161+
it('exports through a recovery handle after a restart, then removes the storage by that handle', async () => {
2162+
const data = fixture({ hostile: agentBase });
2163+
const owner = { runnerOwner: randomBytes(16).toString('hex'), attemptId: `restart-${randomUUID()}`, allocationId: randomUUID() };
2164+
const staging = join(data.root, 'restart-staging'); mkdirSync(staging);
2165+
// An earlier process clones and allocates task storage, then exits without releasing it, as a crash does: this
2166+
// process never held the allocator value.
2167+
const specifier = (path: string) => JSON.stringify(pathToFileURL(join(import.meta.dirname, path)).href);
2168+
const earlier = spawnSync(process.execPath, ['--input-type=module', '-e', [
2169+
`import { createTaskClone } from ${specifier('../git/clone.ts')};`,
2170+
`import { buildAgentImage } from ${specifier('../agents/container/image.ts')};`,
2171+
`import { prepareTaskFilesystems } from ${specifier('../agents/container/storage.ts')};`,
2172+
'const { source, staging, head, owner } = JSON.parse(process.env.EARLIER);',
2173+
// Image trust is per process: a restarted process builds (from cache) before it allocates, as the server does.
2174+
'const imageId = buildAgentImage();',
2175+
"const clone = createTaskClone({ source, parent: staging, taskId: 'task-1', head });",
2176+
'const limits = { workBytes: 16 * 1024 * 1024, workInodes: 512, metadataBytes: 16 * 1024 * 1024, metadataInodes: 512 };',
2177+
// The last line, after anything the build wrote to standard output.
2178+
"process.stdout.write('\\n' + JSON.stringify(prepareTaskFilesystems(clone, limits, imageId, owner)));",
2179+
].join('\n')], { encoding: 'utf8', timeout: 120_000, env: { ...process.env,
2180+
EARLIER: JSON.stringify({ source: data.source, staging, head: data.clone.head, owner }) } });
2181+
try {
2182+
if (earlier.status !== 0) throw new Error(`The earlier process failed (${earlier.status}): ${earlier.stderr}`);
2183+
const left = JSON.parse(earlier.stdout.trim().split('\n').pop()!) as ReturnType<typeof prepareTaskFilesystems>;
2184+
agentChanges(left);
2185+
const report = await recoverLeftovers(owner.runnerOwner);
2186+
expect(report.removed).toEqual([]);
2187+
expect(report.storage).toEqual([{ ...owner, workVolume: left.workVolume, metadataVolume: left.metadataVolume,
2188+
keeper: left.keeper }]);
2189+
const handle = report.storage[0]!;
2190+
// D keeps no baseline across a restart: the export needs the one F recorded, and checks the metadata against it.
2191+
await expect(exportTaskDiff(handle, { base: data.clone.head, imageId })).rejects.toThrow('needs the metadataBaseline');
2192+
await expect(exportTaskDiff(handle, { base: data.clone.head, imageId, metadataBaseline: 'c'.repeat(64) }))
2193+
.rejects.toThrow('metadata changed');
2194+
const before = storageSnapshot(left);
2195+
const exported = await exportTaskDiff(handle, { base: data.clone.head, imageId, metadataBaseline: left.metadataBaseline });
2196+
expect(storageSnapshot(left)).toBe(before);
2197+
const text = exported.diff.toString('utf8');
2198+
expect(exported.truncated).toBe(false);
2199+
for (const expected of ['+committed', '+changed', '+staged only', '+brand new', 'GIT binary patch'])
2200+
expect(text).toContain(expected);
2201+
// Every export, refused or not, removed its container: only the keeper is left before release.
2202+
expect(docker('ps', '--all', '--quiet', '--no-trunc', '--filter', `label=io.codeboost.allocation=${owner.allocationId}`))
2203+
.toBe(docker('container', 'inspect', '--format', '{{.Id}}', left.keeper));
2204+
// The handle still releases the storage after the export, once.
2205+
removeTaskFilesystems(handle);
2206+
expect(isRecoveredTaskStorage(handle)).toBe(false);
2207+
for (const [kind, ref] of [['container', left.keeper], ['volume', left.workVolume], ['volume', left.metadataVolume]] as const)
2208+
expect(spawnSync('docker', [kind, 'inspect', ref], { stdio: 'ignore' }).status).not.toBe(0);
2209+
} finally {
2210+
// Never throws, so a failure above is the one reported.
2211+
const labelled = (...args: string[]) => spawnSync('docker', [...args, '--quiet', '--filter',
2212+
`label=io.codeboost.allocation=${owner.allocationId}`], { encoding: 'utf8' }).stdout?.split('\n').filter(Boolean) ?? [];
2213+
for (const name of labelled('ps', '--all')) spawnSync('docker', ['rm', '--force', name], { stdio: 'ignore' });
2214+
for (const name of labelled('volume', 'ls')) spawnSync('docker', ['volume', 'rm', '--force', name], { stdio: 'ignore' });
2215+
}
2216+
}, 180_000);
2217+
21602218
it('never removes a task keeper replaced by another runner, but still removes the owned volumes', () => {
21612219
const data = fixture(), filesystems = data.filesystems, owner = taskFilesystemOwner(filesystems);
21622220
taskFilesystems.splice(taskFilesystems.indexOf(filesystems), 1);

0 commit comments

Comments
 (0)