diff --git a/packages/core/src/services/backgroundShellRegistry.test.ts b/packages/core/src/services/backgroundShellRegistry.test.ts index a462c0fa918..e0ac88d316b 100644 --- a/packages/core/src/services/backgroundShellRegistry.test.ts +++ b/packages/core/src/services/backgroundShellRegistry.test.ts @@ -27,14 +27,32 @@ import { type ShellTaskRegistration, } from './backgroundShellRegistry.js'; import { todoWorkChainContext } from '../utils/promptIdContext.js'; +import { escapeXml } from '../utils/xml.js'; +import { stripDisplayControlChars } from '../utils/terminalSafe.js'; + +/** + * Builds the expected `` element with the same + * `stripDisplayControlChars` + `escapeXml` pipeline the registry applies. + * Expected paths below come from `tmpdir()`, which can legally contain XML + * metacharacters (`&` on Windows, `<` on POSIX) or bidi overrides, so + * hand-rolling the escaping would make these cases depend on the host's TMPDIR. + */ +function expectedOutputFileElement(path: string): string { + return `${escapeXml(stripDisplayControlChars(path))}`; +} let tmpDirs: string[] = []; +let tmpFiles: string[] = []; afterEach(() => { for (const dir of tmpDirs) { rmSync(dir, { recursive: true, force: true }); } + for (const file of tmpFiles) { + rmSync(file, { force: true }); + } tmpDirs = []; + tmpFiles = []; }); function makeOutputFile(content: string): string { @@ -54,19 +72,34 @@ function makeTempDir(): string { 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', abortController: new AbortController(), ...overrides, + // Every register/complete/fail/cancel mirrors the entry into a + // `.status` sidecar, so the default outputPath decides where + // that write lands. A fixed `/tmp/s1.output` pointed every entry in this + // file — across tests, across workers, across CI jobs — at the single + // path `/tmp/s1.status`. `/tmp` is sticky, so once that file belongs to + // another uid the atomic rename fails EPERM, and `renameWithRetrySync` + // burns its full 50+100+200ms backoff before the registry swallows the + // error. Give each entry its own directory instead: no shared state, and + // the sidecar write actually succeeds. + outputPath: + overrides.outputPath ?? join(makeTempDir(), `shell-${shellId}.output`), }; } describe('BackgroundShellRegistry', () => { + it('gives each entry a unique default outputPath', () => { + expect(makeEntry().outputPath).not.toBe(makeEntry().outputPath); + }); + describe('register / get / getAll', () => { it('captures the Todo work-chain owner at registration', () => { const reg = new BackgroundShellRegistry(); @@ -282,7 +315,7 @@ describe('BackgroundShellRegistry', () => { expect(modelText).toContain( 'first line\nfinal result', ); - expect(modelText).toContain(`${outputPath}`); + expect(modelText).toContain(expectedOutputFileElement(outputPath)); expect(meta).toEqual({ shellId: 'a', status: 'completed', @@ -318,12 +351,13 @@ describe('BackgroundShellRegistry', () => { const reg = new BackgroundShellRegistry(); const callback = vi.fn(); reg.setNotificationCallback(callback); + const outputPath = join(makeTempDir(), 'out&err.log'); reg.register( makeEntry({ shellId: 'a&b', command: 'echo "