fix(bundle): only sweep bundle-owned files in commit, preserving user files in --out dir (#162)

commitBundle's stale-file sweep removed EVERY directory entry not part
of the fresh bundle, so 'test failure get --out <dir>' / 'test artifact
get --out <dir>' pointed at a pre-existing, populated directory silently
deleted the user's unrelated files (exit 0, no warning) — on the very
first write, not just re-commits.

Scope the sweep to entries the bundle format owns (result.json,
failure.json, video.mp4, meta.json, steps, .tmp, .partial, code.<ext>).
Stale bundle files are still cleaned — an old video.mp4 when the new
bundle has no video, a code.py when the new bundle writes code.ts — but
foreign files and directories are never touched.

Fixes #159

Co-authored-by: Kshitij Bhardwaj <tothemoon202154@outlook.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

Author: 3 people

src/lib/bundle.test.ts

@@ -8,7 +8,7 @@
  * the full http+fetch path is wired against MSW).
  */
 
-import { existsSync, mkdtempSync } from 'node:fs';
+import { existsSync, mkdirSync, mkdtempSync, readFileSync, writeFileSync } from 'node:fs';
 import { tmpdir } from 'node:os';
 import { join } from 'node:path';
 import { describe, expect, it } from 'vitest';
@@ -18,6 +18,7 @@ import {
   assertNoEscape,
   BUNDLE_SCHEMA_VERSION,
   buildMeta,
+  isBundleOwnedEntry,
   pickCodeExtension,
   resolveBundleDir,
   STREAM_URL_MAX_RETRIES,
@@ -694,6 +695,37 @@ describe('streamUrlToFile retry', () => {
   });
 });
 
+describe('isBundleOwnedEntry', () => {
+  it('owns the fixed bundle file set', () => {
+    for (const entry of [
+      'result.json',
+      'failure.json',
+      'video.mp4',
+      'meta.json',
+      'steps',
+      '.tmp',
+      '.partial',
+    ]) {
+      expect(isBundleOwnedEntry(entry)).toBe(true);
+    }
+  });
+
+  it('owns code.<ext> for any single-token extension', () => {
+    expect(isBundleOwnedEntry('code.ts')).toBe(true);
+    expect(isBundleOwnedEntry('code.js')).toBe(true);
+    expect(isBundleOwnedEntry('code.py')).toBe(true);
+  });
+
+  it('does not own foreign entries', () => {
+    expect(isBundleOwnedEntry('notes.txt')).toBe(false);
+    expect(isBundleOwnedEntry('src')).toBe(false);
+    expect(isBundleOwnedEntry('.git')).toBe(false);
+    expect(isBundleOwnedEntry('code.tar.gz')).toBe(false);
+    expect(isBundleOwnedEntry('mycode.ts')).toBe(false);
+    expect(isBundleOwnedEntry('code.')).toBe(false);
+  });
+});
+
 describe('step artifact path validation', () => {
   // A fetchImpl that fails the test if called — proves validation rejects
   // before any write happens.
@@ -807,6 +839,49 @@ describe('step artifact path validation', () => {
     expect(existsSync(join(res.dir, 'meta.json'))).toBe(true);
   });
 
+  describe('commit sweep ownership (data-loss guard)', () => {
+    it('preserves pre-existing foreign files and directories in the --out dir', async () => {
+      const dir = mkdtempSync(join(tmpdir(), 'bundle-test-'));
+      writeFileSync(join(dir, 'notes.txt'), 'important notes\n', 'utf8');
+      mkdirSync(join(dir, 'src'));
+      writeFileSync(join(dir, 'src', 'app.js'), "console.log('app')\n", 'utf8');
+
+      const res = await writeBundle(stepCtx(3), {
+        dir,
+        failedOnly: false,
+        fetchImpl: throwIfFetched,
+      });
+
+      // The bundle landed…
+      expect(existsSync(join(res.dir, 'meta.json'))).toBe(true);
+      expect(existsSync(join(res.dir, 'result.json'))).toBe(true);
+      // …and the user's unrelated files survived the commit sweep.
+      expect(readFileSync(join(dir, 'notes.txt'), 'utf8')).toBe('important notes\n');
+      expect(readFileSync(join(dir, 'src', 'app.js'), 'utf8')).toBe("console.log('app')\n");
+    });
+
+    it('still sweeps a stale bundle-owned video.mp4 the new bundle does not write', async () => {
+      const dir = mkdtempSync(join(tmpdir(), 'bundle-test-'));
+      writeFileSync(join(dir, 'video.mp4'), 'stale-bytes', 'utf8');
+
+      // stepCtx has videoUrl: null → the fresh bundle ships no video.
+      await writeBundle(stepCtx(3), { dir, failedOnly: false, fetchImpl: throwIfFetched });
+
+      expect(existsSync(join(dir, 'video.mp4'))).toBe(false);
+    });
+
+    it('sweeps a stale code file with a different extension than the new bundle writes', async () => {
+      const dir = mkdtempSync(join(tmpdir(), 'bundle-test-'));
+      writeFileSync(join(dir, 'code.py'), '# stale python code\n', 'utf8');
+
+      // baseCtx.code.language is 'typescript' → the fresh bundle writes code.ts.
+      await writeBundle(stepCtx(3), { dir, failedOnly: false, fetchImpl: throwIfFetched });
+
+      expect(existsSync(join(dir, 'code.ts'))).toBe(true);
+      expect(existsSync(join(dir, 'code.py'))).toBe(false);
+    });
+  });
+
   describe('assertNoEscape', () => {
     it('returns the resolved path for an in-bounds segment', () => {
       const base = mkdtempSync(join(tmpdir(), 'bundle-test-'));

src/lib/bundle.ts

@@ -543,6 +543,30 @@ async function freshTmpDir(dir: string): Promise<string> {
  * caught reading the dir during it sees no meta and refuses to consume
  * (per §7.3). That's what we want.
  */
+/**
+ * Whether a top-level directory entry belongs to the bundle format —
+ * i.e. something a prior `writeBundle` could have produced and this
+ * commit is therefore allowed to clean up. `code.<ext>` is matched by
+ * pattern (not the current run's extension) so a stale `code.py` is
+ * still swept when the new bundle writes `code.ts`. Everything else in
+ * the directory is the user's and must never be deleted (`--out` can
+ * point at a pre-existing, populated directory).
+ */
+export function isBundleOwnedEntry(entry: string): boolean {
+  if (
+    entry === 'result.json' ||
+    entry === 'failure.json' ||
+    entry === 'video.mp4' ||
+    entry === 'meta.json' ||
+    entry === 'steps' ||
+    entry === '.tmp' ||
+    entry === '.partial'
+  ) {
+    return true;
+  }
+  return /^code\.[A-Za-z0-9]+$/.test(entry);
+}
+
 async function commitBundle(
   tmpDir: string,
   dir: string,
@@ -553,20 +577,22 @@ async function commitBundle(
 
   // (2) Sweep stale top-level files that the new bundle won't write.
   // If the prior run wrote `video.mp4` and the new run has no video,
-  // an in-place rename leaves the old video lingering. Enumerate
-  // current top-level entries and remove anything that isn't being
-  // freshly renamed in.
+  // an in-place rename leaves the old video lingering. Only entries the
+  // bundle format OWNS are candidates: `--out` may point at a directory
+  // that also holds the user's unrelated files, and those must survive
+  // the commit (deleting them would be silent data loss).
   const topLevel = files.filter(f => !f.startsWith('steps/'));
   const newTopLevelSet = new Set(topLevel);
   newTopLevelSet.add('meta.json'); // about to land last, do not delete
   const existing = await readdir(dir).catch(() => [] as string[]);
   for (const entry of existing) {
     // Preserve the writer's own scratch dir + the .partial marker
-    // (we'll re-evaluate .partial at the end of commit). Anything else
-    // not-listed in the new bundle is stale.
+    // (we'll re-evaluate .partial at the end of commit). Any other
+    // bundle-owned entry not-listed in the new bundle is stale.
     if (entry === '.tmp' || entry === '.partial') continue;
     if (newTopLevelSet.has(entry)) continue;
     if (entry === 'steps') continue; // handled below
+    if (!isBundleOwnedEntry(entry)) continue; // foreign file — never touch
     await rm(join(dir, entry), { recursive: true, force: true });
   }