Skip to content

Commit 582f786

Browse files
mchwangclaude
andcommitted
Workflow limits for the longer teardown; full test of the loaded daemon; keeper warning (#189)
Review round 52: the Live probes job limit rises to 90 minutes, as the teardown hooks it covers now allow 24 (vitest-config pins it); the Agent isolation comment says its 45 minutes cover ordinary runs; the loaded-daemon test answers one network with a holder and one named container, so every kind of call is checked; a failed foreign-keeper removal is warned of when the body failed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 607f7ed commit 582f786

5 files changed

Lines changed: 31 additions & 15 deletions

File tree

‎.github/workflows/agent-isolation.yml‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@ permissions:
99
jobs:
1010
real-docker:
1111
runs-on: ubuntu-latest
12-
# Covers the 10 min image build, 2 min teardown and the per-test Docker budgets.
12+
# Covers the 10 min image build and ordinary run times of the Docker-backed files. It does not cover every test
13+
# and hook at its own Vitest limit: the five afterAll hooks that sweep allow 46 min between them (#189), which only
14+
# a stalled daemon reaches.
1315
timeout-minutes: 45
1416
steps:
1517
- uses: actions/checkout@v4

‎.github/workflows/live-probes.yml‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,11 @@ jobs:
1515
runs-on: ubuntu-latest
1616
# The probes' own time limits total 33 min (6, 8, 8 and 11). agent-container and agent-supervisor may each spend
1717
# up to 10 min building the image before their probes; agent-question builds inside its probe's own 11 min. The
18-
# teardown hooks have Vitest limits that total 9 min: agent-container 2 after each test and 2 at the end,
19-
# agent-supervisor 3 and agent-question 2. That is 62 min, plus setup. These are only Vitest's limits: the afterAll
20-
# hooks in agent-container and agent-question call docker through spawnSync with no timeout of their own, and
21-
# Vitest's hook timer cannot fire while such a call blocks. The 75 min job limit caps the run.
22-
timeout-minutes: 75
18+
# teardown hooks have Vitest limits that total 24 min: agent-container 2 after each test and 13 at the end (two
19+
# sweeps of up to 5.5 min each, #189), agent-supervisor 7 and agent-question 2. That is 77 min, plus setup. These
20+
# are only Vitest's limits: Vitest's hook timer cannot fire while a synchronous docker call blocks, and some calls in
21+
# these hooks have no timeout of their own (the sweeps' calls have 30 s each). The 90 min job limit caps the run.
22+
timeout-minutes: 90
2323
steps:
2424
# Fail at once when the secret is missing, so a missing secret never looks like a passing run.
2525
- name: Require the Claude token

‎test/agent-container.test.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2591,8 +2591,10 @@ describe('real Docker agent isolation', () => {
25912591
let released: unknown;
25922592
try { removeTaskFilesystems(filesystems); } catch (error) { released = error; console.warn('cleanup: removing the storage failed:', error); }
25932593
removeLabelledObjects('io.codeboost.allocation', owner.allocationId, 'test');
2594-
if (passed && keeperRemoved.status !== 0) throw new Error(`docker rm --force ${filesystems.keeper} failed: `
2595-
+ (keeperRemoved.error?.message ?? (keeperRemoved.signal ? `killed by ${keeperRemoved.signal}` : keeperRemoved.stderr?.trim())));
2594+
const keeperFailure = keeperRemoved.status === 0 ? undefined : `docker rm --force ${filesystems.keeper} failed: `
2595+
+ (keeperRemoved.error?.message ?? (keeperRemoved.signal ? `killed by ${keeperRemoved.signal}` : keeperRemoved.stderr?.trim()));
2596+
if (keeperFailure && !passed) console.warn('cleanup:', keeperFailure);
2597+
if (keeperFailure && passed) throw new Error(keeperFailure);
25962598
if (passed && released) throw released;
25972599
}
25982600
}, 7 * 60_000);

‎test/docker-sweep.test.ts‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -128,12 +128,19 @@ it('talks to the daemon set when it loaded, though a test changed DOCKER_HOST or
128128
expect(new Set(lines)).toEqual(new Set([loaded]));
129129
});
130130

131-
it('takes the daemon set at load, not the default, when one was set', async () => {
131+
it('takes the daemon set at load, not the default, when one was set, for every call', async () => {
132132
const root = mkdtempSync(join(tmpdir(), 'codeboost-sweep-')); roots.push(root);
133-
const seen = join(root, 'seen');
134-
// Records the address each call is given, and answers as Docker 27.4 does for a missing container and an empty `ls`.
135-
writeFileSync(join(root, 'docker'), ['#!/bin/sh', `printf '%s|%s\\n' "\${DOCKER_HOST-unset}" "\${DOCKER_CONTEXT-unset}" >> '${seen}'`,
136-
'if [ "$2" = inspect ]; then echo; echo "Error response from daemon: No such container: x" >&2; exit 1; fi', 'exit 0'].join('\n'),
133+
const seen = join(root, 'seen'), net = 'n'.repeat(64), holder = 'a'.repeat(64), named = 'b'.repeat(64);
134+
// Records the address and command of each call. Answers as Docker 27.4 does, in the parts the code reads, for one
135+
// swept network with one holder and for a container named x; every other call gets no output and exit 0, as for
136+
// `ls` with no match, and the removals' stdout is not read. The network line holds only the fields the sweep reads.
137+
writeFileSync(join(root, 'docker'), ['#!/bin/sh', 'for arg; do last="$arg"; done',
138+
`printf '%s|%s|%s %s\\n' "\${DOCKER_HOST-unset}" "\${DOCKER_CONTEXT-unset}" "$1" "$2" >> '${seen}'`,
139+
`if [ "$1 $2" = "network ls" ]; then printf '%s\\n' '{"ID":"${net}","Name":"swept"}'; exit 0; fi`,
140+
`if [ "$1" = ps ]; then printf '%s\\n' ${holder}; exit 0; fi`,
141+
`if [ "$1 $2" = "container inspect" ] && [ "$last" = x ]; then printf '%s\\n' '"${named}" "/x"'; exit 0; fi`,
142+
`if [ "$1 $2" = "container inspect" ]; then printf '%s\\n' '"${holder}" {"swept":{}}'; exit 0; fi`,
143+
'exit 0'].join('\n'),
137144
{ mode: 0o755 });
138145
// A fresh copy of the module, loaded while a daemon is set, as on a machine with Colima or rootless Docker.
139146
vi.stubEnv('DOCKER_HOST', 'unix:///codeboost-loaded.sock');
@@ -146,7 +153,12 @@ it('takes the daemon set at load, not the default, when one was set', async () =
146153
fresh.removeByName('container', 'x', 'test', join(root, 'docker'));
147154
fresh.removeLabelledObjects(TEST_RUN_LABEL, 'x', 'test', join(root, 'docker'));
148155
} finally { vi.unstubAllEnvs(); }
149-
expect(new Set(readFileSync(seen, 'utf8').split('\n').filter(Boolean))).toEqual(new Set(['unix:///codeboost-loaded.sock|codeboost-loaded']));
156+
const lines = readFileSync(seen, 'utf8').split('\n').filter(Boolean).map(line => line.split('|'));
157+
// Every kind of call the two make: inspect and removal by name, the listings, the holder lookup and read, the
158+
// holders' removal and the network's.
159+
expect(new Set(lines.map(line => line[2]))).toEqual(new Set(['container inspect', 'rm --force', 'container ls', 'network ls',
160+
'volume ls', 'ps --all', 'network rm']));
161+
expect(new Set(lines.map(line => `${line[0]}|${line[1]}`))).toEqual(new Set(['unix:///codeboost-loaded.sock|codeboost-loaded']));
150162
});
151163

152164
it.each([

‎test/vitest-config.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,7 @@ describe('CI workflows', () => {
317317
const job = live.jobs['live-probes']!;
318318
// No `defaults`, `env`, `strategy` or `container`: any of them could change what the steps run.
319319
expect(Object.keys(job).toSorted()).toEqual(['runs-on', 'steps', 'timeout-minutes']);
320-
expect(job).toMatchObject({ 'runs-on': 'ubuntu-latest', 'timeout-minutes': 75 });
320+
expect(job).toMatchObject({ 'runs-on': 'ubuntu-latest', 'timeout-minutes': 90 });
321321
const token = { CLAUDE_CODE_OAUTH_TOKEN: '${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}' };
322322
expect(job.steps.map(step => Object.keys(step).toSorted())).toEqual([['env', 'name', 'run'], ['uses'], ['uses', 'with'],
323323
['run'], ['env', 'name', 'run'], ['name', 'run']]);

0 commit comments

Comments
 (0)