diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 11098023c..757a8c425 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,11 @@ ## [Unreleased] +### Fixed + +- Delete pre-created shell snapshot file when snapshot creation fails ([#4236](https://github.com/can1357/oh-my-pi/issues/4236)) + + ## [16.3.0] - 2026-07-02 ### Added diff --git a/packages/coding-agent/src/utils/shell-snapshot.ts b/packages/coding-agent/src/utils/shell-snapshot.ts index 191694663..5980fb932 100644 --- a/packages/coding-agent/src/utils/shell-snapshot.ts +++ b/packages/coding-agent/src/utils/shell-snapshot.ts @@ -260,6 +260,7 @@ export async function getOrCreateSnapshot( // Generate and execute snapshot script const script = generateSnapshotScript(shell, snapshotPath, rcFile); + let succeeded = false; try { const snapshotEnv = sanitizeSnapshotEnv(env); const spawnEnv: Record = {}; @@ -289,10 +290,19 @@ export async function getOrCreateSnapshot( } scrubSnapshotInPlace(snapshotPath); cachedSnapshotPaths.set(cacheKey, snapshotPath); + succeeded = true; return snapshotPath; } } catch { // Snapshot creation failed, proceed without it + } finally { + if (!succeeded) { + try { + fs.rmSync(snapshotPath, { force: true }); + } catch { + // best-effort cleanup; force: true ignores ENOENT + } + } } return null; diff --git a/packages/coding-agent/test/shell-snapshot.test.ts b/packages/coding-agent/test/shell-snapshot.test.ts index 7585e64a9..f34c031b9 100644 --- a/packages/coding-agent/test/shell-snapshot.test.ts +++ b/packages/coding-agent/test/shell-snapshot.test.ts @@ -315,4 +315,81 @@ describe("getOrCreateSnapshot", () => { const content = await fs.readFile(snapshotPath!, "utf8"); expect(content).toContain(`export __MISE_EXE='${REAL_ECHO}'`); }); + it("cleans up the empty snapshot file when the shell exits with a non-zero code", async () => { + const testRoot = await fs.mkdtemp(path.join(os.tmpdir(), "omp-snap-fail-")); + const originalTmpDir = process.env.TMPDIR; + process.env.TMPDIR = testRoot; + try { + const fakeShell = path.join(testRoot, "fail-shell.sh"); + await fs.writeFile(fakeShell, `#!/bin/sh\nexit 1\n`); + await fs.chmod(fakeShell, 0o755); + + const env = { ...process.env, HOME: testRoot }; + const snapshotDir = path.join(testRoot, "omp-shell-snapshots"); + + const snapshotPath = await getOrCreateSnapshot(fakeShell, env); + expect(snapshotPath).toBeNull(); + + // Verify that no snapshot files were left behind in the isolated tmpdir + if (existsSync(snapshotDir)) { + const files = await fs.readdir(snapshotDir); + expect(files).toEqual([]); + } + } finally { + if (originalTmpDir === undefined) delete process.env.TMPDIR; + else process.env.TMPDIR = originalTmpDir; + await fs.rm(testRoot, { recursive: true, force: true }); + } + }); + + it("cleans up the empty snapshot file when the shell fails to spawn", async () => { + const testRoot = await fs.mkdtemp(path.join(os.tmpdir(), "omp-snap-spawn-fail-")); + const originalTmpDir = process.env.TMPDIR; + process.env.TMPDIR = testRoot; + try { + const fakeShell = path.join(testRoot, "does-not-exist-shell"); + + const env = { ...process.env, HOME: testRoot }; + const snapshotDir = path.join(testRoot, "omp-shell-snapshots"); + + const snapshotPath = await getOrCreateSnapshot(fakeShell, env); + expect(snapshotPath).toBeNull(); + + if (existsSync(snapshotDir)) { + const files = await fs.readdir(snapshotDir); + expect(files).toEqual([]); + } + } finally { + if (originalTmpDir === undefined) delete process.env.TMPDIR; + else process.env.TMPDIR = originalTmpDir; + await fs.rm(testRoot, { recursive: true, force: true }); + } + }); + + it("cleans up the empty snapshot file when the shell execution times out", async () => { + const testRoot = await fs.mkdtemp(path.join(os.tmpdir(), "omp-snap-timeout-")); + const originalTmpDir = process.env.TMPDIR; + process.env.TMPDIR = testRoot; + try { + const fakeShell = path.join(testRoot, "timeout-shell.sh"); + // Sleep longer than SNAPSHOT_TIMEOUT_MS (2000) + await fs.writeFile(fakeShell, `#!/bin/sh\nsleep 3\n`); + await fs.chmod(fakeShell, 0o755); + + const env = { ...process.env, HOME: testRoot }; + const snapshotDir = path.join(testRoot, "omp-shell-snapshots"); + + const snapshotPath = await getOrCreateSnapshot(fakeShell, env); + expect(snapshotPath).toBeNull(); + + if (existsSync(snapshotDir)) { + const files = await fs.readdir(snapshotDir); + expect(files).toEqual([]); + } + } finally { + if (originalTmpDir === undefined) delete process.env.TMPDIR; + else process.env.TMPDIR = originalTmpDir; + await fs.rm(testRoot, { recursive: true, force: true }); + } + }, 5000); // increase test timeout to 5s to accommodate the 2s snapshot timeout });