From 12c01348be31817dde45c3bfff78411f1435386d Mon Sep 17 00:00:00 2001 From: wenshao Date: Sun, 9 Aug 2026 15:10:44 +0800 Subject: [PATCH 01/10] test(core): give shell-registry fixtures their own output paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The retention-cap tests time out at 15s on shared CI runners while passing everywhere else, and it is not load. Every settle writes a status sidecar next to the entry's outputPath, and the fixture's default pointed all of them at one fixed path: `/tmp/s1.output`, so `/tmp/s1.status` on every machine that ever ran this suite. On a sticky-bit /tmp the file can already belong to another user, and then the atomic write's rename answers EPERM and retries with a blocking exponential backoff — 50, 100, 200ms — before giving up. That is ~350ms per settle, measured at 362ms; the cap tests settle 34 entries each, which is 12.3s of a 15s budget before the assertions even run. A/B on one machine with that exact shape (an unrenameable file planted at the shared path): the old fixture reports `Test timed out in 15000ms` three times over, the new one passes 57/57 in 778ms. The fix is the fixture, not the retry policy: the backoff protects real writes against transient EPERM and should stay. Entries now get output paths under a per-test temp directory keyed by shellId, which the existing afterEach already cleans, so no two entries — and no two jobs — share a sidecar. The two tests that hand-wrote `/tmp` paths to exercise `&` and control characters in a basename keep exactly that subject and move their directory too. Seen on feat/daemon-git-worktree-guard and feat/review-capture-tui, four tests apiece, same describe block, with the file taking 120s. --- .../services/backgroundShellRegistry.test.ts | 41 ++++++++++++++++--- 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/packages/core/src/services/backgroundShellRegistry.test.ts b/packages/core/src/services/backgroundShellRegistry.test.ts index ba07b80f640..3c6f169ac18 100644 --- a/packages/core/src/services/backgroundShellRegistry.test.ts +++ b/packages/core/src/services/backgroundShellRegistry.test.ts @@ -35,6 +35,8 @@ afterEach(() => { rmSync(dir, { recursive: true, force: true }); } tmpDirs = []; + // The per-test entry directory goes with them. + entryDir = undefined; }); function makeOutputFile(content: string): string { @@ -51,16 +53,34 @@ function makeTempDir(): string { return dir; } +// Every settle writes a status sidecar next to the entry's outputPath, so +// the fixture must not point them all at one SHARED path. With the old +// fixed `/tmp/s1.output`, every entry in a test wrote `/tmp/s1.status` — a +// path a sticky-bit /tmp shares with every other job on the machine. When +// that file already belongs to another user, the atomic write's rename +// answers EPERM and retries with a blocking exponential backoff (50, 100, +// 200ms), so ONE settle costs ~350ms: measured, 34 of them take 12.3s of a +// 15s test budget, which is what turned the retention-cap tests into +// timeouts on shared CI runners while passing everywhere else. A per-run +// directory keyed by shellId cannot collide with anything. +let entryDir: string | undefined; + +function entryOutputPath(shellId: string): string { + entryDir ??= makeTempDir(); + return join(entryDir, `${shellId}.output`); +} + function makeEntry( overrides: Partial = {}, ): ShellTaskRegistration { + const shellId = overrides.shellId ?? 's1'; return { - shellId: 's1', + shellId, command: 'sleep 60', cwd: '/tmp', status: 'running', startTime: 1000, - outputPath: '/tmp/s1.output', + outputPath: entryOutputPath(shellId), abortController: new AbortController(), ...overrides, }; @@ -318,12 +338,16 @@ describe('BackgroundShellRegistry', () => { const reg = new BackgroundShellRegistry(); const callback = vi.fn(); reg.setNotificationCallback(callback); + // The `&` in the BASENAME is what this test is about; the directory + // is the run's own so the settle below writes its status sidecar + // somewhere no other job shares. + const outputPath = join(makeTempDir(), 'out&err.log'); reg.register( makeEntry({ shellId: 'a&b', command: 'echo "