Skip to content

Commit eccc116

Browse files
mchwangclaude
andcommitted
Reconcile killed container creates before disposing credentials
A `docker create` client killed by its deadline may still land in the daemon. Cleanup now keeps checking for the container through a bounded settle window and retains staged credentials if absence cannot be proven. Also give the immutable-image test the same timeout as its neighbours. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 463ed9f commit eccc116

2 files changed

Lines changed: 47 additions & 12 deletions

File tree

‎agents/container/run.ts‎

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -42,18 +42,30 @@ const canonicalDockerBindSource = (source: string) => {
4242
const desktopHostPath = source.startsWith('/host_mnt/') ? source.slice('/host_mnt'.length) : source;
4343
try { return realpathSync(desktopHostPath); } catch { return source; }
4444
};
45-
const removeContainerOrThrow = (profile: ContainerProfile) => {
46-
const remaining = createDeadline(30_000);
47-
const before = spawnSync('docker', ['container', 'inspect', profile.name], {
48-
encoding: 'utf8', timeout: remaining(), env: dockerEnvironment(), stdio: ['ignore', 'pipe', 'pipe'],
49-
});
50-
if (before.status !== 0) {
45+
/** How long a killed `docker create` may still materialize its container in the daemon. */
46+
const CREATE_SETTLE_MS = 10_000;
47+
const sleep = (ms: number) => Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms);
48+
const removeContainerOrThrow = (profile: ContainerProfile, createUnsettled = false) => {
49+
const remaining = createDeadline(30_000 + (createUnsettled ? CREATE_SETTLE_MS : 0));
50+
const settleBy = performance.now() + (createUnsettled ? CREATE_SETTLE_MS : 0);
51+
let before: ReturnType<typeof spawnSync>;
52+
for (;;) {
53+
before = spawnSync('docker', ['container', 'inspect', profile.name], {
54+
encoding: 'utf8', timeout: remaining(), env: dockerEnvironment(), stdio: ['ignore', 'pipe', 'pipe'],
55+
});
56+
if (before.status === 0) break;
5157
const missing = !before.error && /No such (?:object|container)/i.test(`${before.stdout ?? ''}\n${before.stderr ?? ''}`);
5258
if (!missing) throw new Error('Failed to establish ownership of the agent container; staged credentials were retained.');
53-
disposeContainerProfile(profile);
54-
return;
59+
if (!createUnsettled) {
60+
disposeContainerProfile(profile);
61+
return;
62+
}
63+
// A killed create may still land in the daemon; absence is not proof until the settle window passes.
64+
if (performance.now() >= settleBy)
65+
throw new Error('Agent container creation did not settle; staged credentials were retained.');
66+
sleep(250);
5567
}
56-
const inspected = JSON.parse(before.stdout || '[]')[0] as { Config?: { Labels?: Record<string, string> } } | undefined;
68+
const inspected = JSON.parse(String(before.stdout || '[]'))[0] as { Config?: { Labels?: Record<string, string> } } | undefined;
5769
if (inspected?.Config?.Labels?.['io.codeboost.invocation'] !== profile.ownershipId)
5870
throw new Error('Agent container name is held by another invocation; staged credentials were retained.');
5971
const result = spawnSync('docker', ['rm', '--force', profile.name], {
@@ -230,16 +242,25 @@ export function validateContainer(container: string, profile: ContainerProfile,
230242
export function createValidatedContainer(profile: ContainerProfile, timeoutMs = 30_000,
231243
secrets: Readonly<Record<string, string>> = {}): string {
232244
const remaining = createDeadline(timeoutMs);
245+
let createUnsettled = false;
233246
try {
234247
validateSecrets(profile, secrets);
235248
assertContainerProfile(profile);
236-
docker(profile.args, { timeoutMs: remaining(), secrets });
249+
const createTimeout = remaining();
250+
createUnsettled = true;
251+
try { docker(profile.args, { timeoutMs: createTimeout, secrets }); }
252+
catch (error) {
253+
// A nonzero exit means the daemon answered; a killed client leaves the request in flight.
254+
createUnsettled = typeof (error as { status?: unknown }).status !== 'number';
255+
throw error;
256+
}
257+
createUnsettled = false;
237258
validateContainer(profile.name, profile, remaining());
238259
assertContainerProfile(profile);
239260
remaining();
240261
return profile.name;
241262
} catch (error) {
242-
try { removeContainerOrThrow(profile); }
263+
try { removeContainerOrThrow(profile, createUnsettled); }
243264
catch (cleanupError) { throw new AggregateError([error, cleanupError], 'Container creation failed and cleanup did not settle.'); }
244265
throw error;
245266
}

‎test/agent-container.test.ts‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,20 @@ describe('real Docker agent isolation', () => {
271271
docker('rm', '--force', first.name); containers.delete(first.name);
272272
}, 60_000);
273273

274+
it('retains credentials when a killed create cannot be proven absent', () => {
275+
const data = fixture(), unsettled = profile(data, 'planning', ['true']);
276+
const shim = join(data.root, 'docker-shim'); mkdirSync(shim);
277+
const realDocker = execFileSync('sh', ['-c', 'command -v docker'], { encoding: 'utf8' }).trim();
278+
// The create client hangs until its deadline kills it, so the daemon outcome stays unknown.
279+
writeFileSync(join(shim, 'docker'), ['#!/bin/sh', 'if [ "$1" = create ]; then exec sleep 30; fi',
280+
`exec '${realDocker}' "$@"`].join('\n'), { mode: 0o755 });
281+
const path = process.env.PATH;
282+
process.env.PATH = `${shim}:${path}`;
283+
try { expect(() => createValidatedContainer(unsettled, 1_000)).toThrow('cleanup did not settle'); }
284+
finally { process.env.PATH = path; }
285+
expect(existsSync(unsettled.codexAuthFile!)).toBe(true);
286+
}, 60_000);
287+
274288
it('refuses a Codex auth path that is a link without resolving it', () => {
275289
const data = fixture(), link = join(data.root, 'auth-link.json');
276290
symlinkSync(data.fakeAuth, link);
@@ -340,7 +354,7 @@ describe('real Docker agent isolation', () => {
340354
expect(() => prepareTaskFilesystems({ ...data.clone }, {
341355
workBytes: 1024, workInodes: 16, metadataBytes: 1024, metadataInodes: 16,
342356
}, imageId)).toThrow('trusted clone builder');
343-
});
357+
}, 60_000);
344358

345359
if (process.env.CODEBOOST_RUN_AUTH_PROBES === '1') {
346360
it('runs the authenticated Codex startup path with isolated writable state', () => {

0 commit comments

Comments
 (0)