fix(data-layout): finish interrupted migrations + smoke-check the clone + clean failure (#374 P1-3 review)

Adversarial self-review of the migration found three issues; all fixed with
regression tests.

[H1] A crash between the partition rename (step 3) and the source retire (step 5)
left the partition authoritative but the legacy dir — including its plaintext env
— lingering in the workspace forever: the next run's planMigration hit the
"partition exists" check and returned null, so the legacy dir was never retired,
breaking the zero-residue guarantee. Fix: planMigration now returns a
'retire-only' plan when the partition exists but a legacy dir still lingers;
runMigration finishes the job by retiring the leftover to .teamai.bak WITHOUT
re-copying onto the authoritative partition. The same path handles the
under-lock TOCTOU case (a sibling built the partition while we waited).

[L5] verifyStaging now smoke-checks the staged team-repo clone with
`git rev-parse HEAD` (on staging, before the rename), so a partial/corrupt copy
aborts with the source untouched instead of promoting a broken clone the next
pull would choke on. Existence of .git alone is no longer taken as proof.

[M3] The async preAction hook under program.parse() would surface a migration
failure as a raw unhandled-rejection stack. maybeMigrate is now wrapped: on
failure teamai prints a clean error ("your original .teamai is unchanged, re-run
to retry") and exits non-zero, rather than crashing into the command on partial
state.

Tests: retire-only finishes an interrupted run without overwriting the
partition; a corrupt staged clone aborts leaving source + no partition/.bak/
.staging; http-mode install (no team-repo) migrates. Verified end-to-end with
the built CLI: the interrupted-state pull retires the lingering legacy dir and
moves the plaintext env out of the workspace while keeping the partition intact.

docs/designs/data-directory-layout.md

@@ -131,15 +131,20 @@ partition on the next write command, so the workspace ends up with zero residue.
 
 **Gate** (`planMigration`, deliberately NOT `detectProjectConfig` — that
 short-circuits on an existing partition and runs the self-heal bootstrap as a side
-effect, both of which would mask the raw legacy state). Migrate iff:
+effect, both of which would mask the raw legacy state). Act iff:
 - in a git repo (the partition only exists for git repos), AND
 - `<workspaceRoot>/.teamai/config.yaml` exists, AND
-- `<partition>/config.yaml` does NOT (the partition, once built, is authoritative), AND
 - the legacy config is `scope: project` (user data never lives under `.teamai/`), AND
 - the legacy config is NOT `kind: self` — **self mode is a hard no-op**: its `.teamai/`
   is team knowledge committed to main, and `init --self` already retires any partition,
   so moving it would break "knowledge on main".
 
+The plan's **mode** then depends on the partition: a full copy when
+`<partition>/config.yaml` does not exist yet, or **retire-only** when it does (a prior
+run built the partition but was interrupted before retiring the source — see Interrupt
+recovery). retire-only never re-copies onto the authoritative partition; it only cleans
+up the leftover legacy dir.
+
 **Steps** (`runMigration`) — copy → verify → atomic rename, so an interruption never
 leaves data half-in-both-places:
 
@@ -162,8 +167,18 @@ leaves data half-in-both-places:
 
 Interrupt recovery: staging is a separate sibling dir, so a crash before step 3 leaves
 the partition absent and the source intact — a rerun discards `.staging/` and starts
-clean. A crash between steps 3 and 5 leaves the partition built (so the next run stands
-down) with the legacy dir still present (double-read still works).
+clean. A crash between steps 3 and 5 leaves the partition built with the legacy dir
+still present; the next write command's `planMigration` sees "partition exists AND
+legacy lingers" and returns a **retire-only** plan that finishes the job — it retires
+the leftover legacy dir to `.teamai.bak/` WITHOUT re-copying onto the now-authoritative
+partition. This closes the gap where the legacy dir (including its plaintext `env`)
+would otherwise linger in the workspace forever, breaking the zero-residue guarantee.
+
+The staged team-repo clone is smoke-checked (`git rev-parse HEAD`) before the rename,
+so a partial/corrupt copy aborts with the source untouched rather than promoting a
+broken clone. If a write command's migration fails, teamai prints a clean error and
+exits non-zero (the source is intact, so a rerun retries safely) instead of surfacing
+a raw async-hook rejection.
 
 **Downgrade is not supported** — an older teamai treats a partitioned install as
 uninitialized; `.teamai.bak/` is the manual rollback. Flag prominently in release notes.

src/__tests__/migrate.test.ts

@@ -118,12 +118,17 @@ describe('planMigration', () => {
     expect(await planMigration(repoRoot)).toBeNull();
   });
 
-  it('skips when a partition config already exists (partition is authoritative)', async () => {
+  it('plans a retire-only cleanup when a partition exists but legacy lingers', async () => {
+    // Interrupted prior run: partition built, source never retired. Instead of
+    // skipping (which would leave the legacy dir — incl. plaintext env — forever),
+    // planMigration must return a retire-only plan to finish the cleanup.
     await seedLegacyLayout();
     const partition = projectDataHome(repoRoot);
     await fse.ensureDir(partition);
     await fse.writeFile(path.join(partition, 'config.yaml'), 'repo: {}\n');
-    expect(await planMigration(repoRoot)).toBeNull();
+    const plan = await planMigration(repoRoot);
+    expect(plan).not.toBeNull();
+    expect(plan!.mode).toBe('retire-only');
   });
 
   it('skips a user-scope legacy config', async () => {
@@ -183,6 +188,34 @@ describe('runMigration', () => {
     expect(await fse.pathExists(path.join(`${legacyDir}.bak`, 'config.yaml'))).toBe(true);
   });
 
+  it('rebases repo.localPath from the legacy dir onto the partition', async () => {
+    await seedLegacyLayout();
+    const plan = await planMigration(repoRoot);
+    await runMigration(plan!);
+    // The migrated config must name the team-repo INSIDE the partition, not the
+    // now-retired legacy path — otherwise the next pull reads the wrong clone.
+    const partition = projectDataHome(repoRoot);
+    const migrated = YAML.parse(
+      await fse.readFile(path.join(partition, 'config.yaml'), 'utf-8'),
+    );
+    expect(migrated.repo.localPath).toBe(path.join(partition, 'team-repo'));
+    expect(migrated.repo.localPath).not.toContain('.teamai/team-repo');
+  });
+
+  it('leaves a localPath that is not inside the legacy dir untouched', async () => {
+    // e.g. an install whose team-repo clone lives elsewhere entirely.
+    const external = path.join(base, 'external-clone');
+    await writeLegacyConfig({
+      repo: { localPath: external, remote: 'git@example.com:t/r.git', kind: 'git' },
+    });
+    const plan = await planMigration(repoRoot);
+    await runMigration(plan!);
+    const migrated = YAML.parse(
+      await fse.readFile(path.join(projectDataHome(repoRoot), 'config.yaml'), 'utf-8'),
+    );
+    expect(migrated.repo.localPath).toBe(external);
+  });
+
   it('does not carry a live sync-lock into the backup', async () => {
     await seedLegacyLayout();
     const plan = await planMigration(repoRoot);
@@ -212,6 +245,63 @@ describe('runMigration', () => {
     expect(await planMigration(repoRoot)).toBeNull();
   });
 
+  it('retire-only mode retires a leftover legacy dir without re-copying (finishes an interrupted run)', async () => {
+    // Simulate a crash between the partition rename and the source retire: the
+    // partition is already built AND the legacy dir still lingers.
+    await seedLegacyLayout();
+    const partition = projectDataHome(repoRoot);
+    await fse.ensureDir(partition);
+    await fse.writeFile(path.join(partition, 'config.yaml'), 'repo:\n  kind: git\n');
+    await fse.writeFile(path.join(partition, 'sentinel'), 'authoritative\n');
+
+    const plan = await planMigration(repoRoot);
+    expect(plan!.mode).toBe('retire-only');
+    const result = await runMigration(plan!);
+    expect(result).toBe('migrated');
+    // Legacy retired → workspace zero-residue (the plaintext env no longer lingers).
+    expect(await fse.pathExists(legacyDir)).toBe(false);
+    expect(await fse.pathExists(`${legacyDir}.bak`)).toBe(true);
+    // The authoritative partition was NOT overwritten by a re-copy.
+    expect(await fse.pathExists(path.join(partition, 'sentinel'))).toBe(true);
+    // A follow-up plan is now null — the workspace is clean.
+    expect(await planMigration(repoRoot)).toBeNull();
+  });
+
+  it('aborts without touching the source when the staged clone is corrupt', async () => {
+    // A team-repo whose .git is present but not a real repo → verify's rev-parse
+    // smoke-check must fail, leaving the source intact and no partition/.bak.
+    await writeLegacyConfig();
+    await fse.writeJson(path.join(legacyDir, 'state.json'), {});
+    const tr = path.join(legacyDir, 'team-repo');
+    await fse.ensureDir(path.join(tr, '.git')); // a .git dir that is NOT a valid repo
+    await fse.writeFile(path.join(tr, 'README'), 'x\n');
+
+    const plan = await planMigration(repoRoot);
+    await expect(runMigration(plan!)).rejects.toThrow(/not a usable git repository/);
+    // Source untouched; nothing half-migrated.
+    expect(await fse.pathExists(path.join(legacyDir, 'config.yaml'))).toBe(true);
+    expect(await fse.pathExists(`${legacyDir}.bak`)).toBe(false);
+    expect(await fse.pathExists(projectDataHome(repoRoot))).toBe(false);
+    expect(await fse.pathExists(`${projectDataHome(repoRoot)}.staging`)).toBe(false);
+  });
+
+  it('migrates an http-mode install (no team-repo clone)', async () => {
+    await writeLegacyConfig({
+      repo: { localPath: legacyDir, remote: 'https://team.example/api', kind: 'http', url: 'https://team.example/api' },
+    });
+    await fse.writeFile(path.join(legacyDir, 'token'), 'api-key-xyz\n');
+    await fse.writeJson(path.join(legacyDir, 'state.json'), {});
+
+    const plan = await planMigration(repoRoot);
+    expect(plan!.mode).toBe('full');
+    const result = await runMigration(plan!);
+    expect(result).toBe('migrated');
+    const partition = projectDataHome(repoRoot);
+    expect(await fse.pathExists(path.join(partition, 'config.yaml'))).toBe(true);
+    expect(await fse.pathExists(path.join(partition, 'token'))).toBe(true);
+    expect(await fse.pathExists(legacyDir)).toBe(false);
+  });
+
   it('recovers from a leftover staging dir (interrupted prior run)', async () => {
     await seedLegacyLayout();
     const partition = projectDataHome(repoRoot);

src/index.ts

@@ -34,7 +34,17 @@ program
     if (TEAMAI_HOOK_SUBCOMMANDS.includes(name as (typeof TEAMAI_HOOK_SUBCOMMANDS)[number])) return;
     if (!MIGRATION_TRIGGER_COMMANDS.has(name)) return;
     const { maybeMigrate } = await import('./migrate.js');
-    await maybeMigrate({ dryRun: !!opts.dryRun });
+    try {
+      await maybeMigrate({ dryRun: !!opts.dryRun });
+    } catch (e) {
+      // A failed migration must not proceed into the command on stale/partial
+      // state. Surface a clean message and exit — the copy→verify→rename design
+      // leaves the source intact, so a rerun retries safely. (Without this the
+      // async-hook rejection would surface as a raw unhandled-rejection stack.)
+      log.error(`Auto-migration failed: ${(e as Error).message}`);
+      log.error('Your original .teamai data is unchanged. Re-run the command to retry.');
+      process.exit(1);
+    }
   });
 
 program

src/migrate.ts

@@ -4,7 +4,8 @@ import YAML from 'yaml';
 import { LocalConfigSchema, SYNC_LOCK_FILENAME } from './types.js';
 import { resolveAnchors } from './utils/git.js';
 import { projectDataHome } from './utils/partition.js';
-import { pathExists, readFileSafe, remove, writeFile } from './utils/fs.js';
+import { realpath } from 'node:fs/promises';
+import { expandHome, pathExists, readFileSafe, remove, writeFile } from './utils/fs.js';
 import { acquireLock, releaseLock } from './update.js';
 import { log } from './utils/logger.js';
 
@@ -45,6 +46,15 @@ export interface MigrationPlan {
   legacyDir: string;
   partitionDir: string;
   anchor: string;
+  /**
+   * 'full': copy legacy → partition, then retire the source.
+   * 'retire-only': the partition is already built (e.g. a prior run crashed
+   * between the partition rename and the source retire), so just clean up the
+   * leftover legacy dir. Without this, planMigration would return null on the
+   * "partition exists" check and the legacy dir — including its plaintext `env`
+   * — would linger in the workspace forever, breaking the zero-residue promise.
+   */
+  mode: 'full' | 'retire-only';
 }
 
 /**
@@ -57,10 +67,11 @@ export interface MigrationPlan {
  *  - not a git repo (the partition only exists for git repos; a non-git
  *    `.teamai/` is already at its final location),
  *  - no legacy config.yaml (nothing to migrate),
- *  - a partition config already exists (the partition is authoritative once
- *    built; detection never looks back at legacy),
  *  - the legacy config is user scope (user data never lives under `.teamai/`),
  *  - the legacy config is self mode (its `.teamai/` is committed team knowledge).
+ *
+ * When a partition config already exists AND a legacy dir still lingers, returns
+ * a 'retire-only' plan to finish an interrupted migration instead of skipping.
  */
 export async function planMigration(cwd?: string): Promise<MigrationPlan | null> {
   const anchors = await resolveAnchors(cwd ?? process.cwd());
@@ -70,10 +81,6 @@ export async function planMigration(cwd?: string): Promise<MigrationPlan | null>
   const legacyConfig = path.join(legacyDir, 'config.yaml');
   if (!(await pathExists(legacyConfig))) return null;
 
-  const partitionDir = projectDataHome(anchors.projectAnchor);
-  // The partition, once built, is authoritative — never migrate on top of it.
-  if (await pathExists(path.join(partitionDir, 'config.yaml'))) return null;
-
   // Read the legacy config directly to gate on scope/kind. A malformed config is
   // treated as "nothing to migrate" rather than crashing a write command.
   const content = await readFileSafe(legacyConfig);
@@ -90,7 +97,15 @@ export async function planMigration(cwd?: string): Promise<MigrationPlan | null>
   if (scope !== 'project') return null;
   if (kind === 'self') return null;
 
-  return { legacyDir, partitionDir, anchor: anchors.projectAnchor };
+  const partitionDir = projectDataHome(anchors.projectAnchor);
+  // If the partition is already built, the copy is done (or was done by a prior
+  // run that crashed before retiring the source). Don't re-copy onto the
+  // authoritative partition — just finish the job by retiring the leftover
+  // legacy dir, so the workspace really does end up residue-free.
+  const mode: MigrationPlan['mode'] =
+    (await pathExists(path.join(partitionDir, 'config.yaml'))) ? 'retire-only' : 'full';
+
+  return { legacyDir, partitionDir, anchor: anchors.projectAnchor, mode };
 }
 
 /**
@@ -114,14 +129,21 @@ export async function runMigration(
   plan: MigrationPlan,
   opts: { dryRun?: boolean } = {},
 ): Promise<'migrated' | 'skipped' | 'dry-run'> {
-  const { legacyDir, partitionDir, anchor } = plan;
+  const { legacyDir, partitionDir, anchor, mode } = plan;
 
   if (opts.dryRun) {
-    const entries = await listMigratableEntries(legacyDir);
-    log.info(
-      `[dry-run] would migrate ${entries.length} item(s) from ${legacyDir} ` +
-        `to ${partitionDir}, then rename the old directory to ${legacyDir}.bak`,
-    );
+    if (mode === 'retire-only') {
+      log.info(
+        `[dry-run] partition already built at ${partitionDir}; would retire the ` +
+          `leftover ${legacyDir} to ${legacyDir}.bak`,
+      );
+    } else {
+      const entries = await listMigratableEntries(legacyDir);
+      log.info(
+        `[dry-run] would migrate ${entries.length} item(s) from ${legacyDir} ` +
+          `to ${partitionDir}, then rename the old directory to ${legacyDir}.bak`,
+      );
+    }
     return 'dry-run';
   }
 
@@ -134,11 +156,26 @@ export async function runMigration(
   const staging = `${partitionDir}.staging`;
   let lockReleased = false;
   try {
+    // 'retire-only': a prior run already built the partition but crashed before
+    // retiring the source. The partition is authoritative — do NOT re-copy onto
+    // it — just finish by retiring the leftover legacy dir.
+    if (mode === 'retire-only') {
+      await releaseLock(lockPath);
+      lockReleased = true;
+      const backup = await retireLegacy(legacyDir);
+      log.success(`Finished an interrupted migration: retired ${legacyDir} to ${backup}`);
+      return 'migrated';
+    }
+
     // Re-check under the lock: a sibling worktree may have migrated while we
-    // waited (TOCTOU). If the partition config now exists, stand down.
+    // waited (TOCTOU). If the partition config now exists, retire our leftover
+    // legacy dir rather than copying onto the authoritative partition.
     if (await pathExists(path.join(partitionDir, 'config.yaml'))) {
-      log.debug('migration skipped: partition built by a concurrent process');
-      return 'skipped';
+      await releaseLock(lockPath);
+      lockReleased = true;
+      const backup = await retireLegacy(legacyDir);
+      log.debug(`partition built by a concurrent process; retired ${legacyDir} to ${backup}`);
+      return 'migrated';
     }
 
     // 1. Copy into a sibling staging dir (NOT the partition itself) so an
@@ -160,6 +197,13 @@ export async function runMigration(
     // 2. Verify the staged copy before making it authoritative.
     await verifyStaging(legacyDir, staging);
 
+    // 2b. Rebase absolute paths persisted in config.yaml that pointed INTO the
+    //     legacy dir (chiefly repo.localPath → <legacyDir>/team-repo) onto the
+    //     partition. Without this, the migrated config would still name the old
+    //     team-repo location, so the next pull would read/clone the wrong path.
+    //     Done in staging (pre-rename) so it stays inside the atomic window.
+    await rebaseConfigPaths(path.join(staging, 'config.yaml'), legacyDir, partitionDir);
+
     // 3. Atomic switch: same-filesystem rename of the staged dir onto the final
     //    partition path. partitionDir does not exist yet (planMigration + the
     //    under-lock re-check both gate on its config.yaml, and nothing else
@@ -177,13 +221,7 @@ export async function runMigration(
     await releaseLock(lockPath);
     lockReleased = true;
 
-    // Retire the source to `.teamai.bak/` (same-fs sibling → atomic rename).
-    // Never auto-delete: it is the manual rollback path (downgrading to an
-    // older teamai is not supported — see release notes / design doc R6).
-    const backup = `${legacyDir}.bak`;
-    await remove(backup);
-    await fse.rename(legacyDir, backup);
-
+    const backup = await retireLegacy(legacyDir);
     log.success(`Migrated teamai data to ${partitionDir}`);
     log.info(
       `Old data preserved at ${backup} — remove it once you've confirmed ` +
@@ -211,6 +249,72 @@ export async function maybeMigrate(opts: { dryRun?: boolean } = {}): Promise<voi
   await runMigration(plan, opts);
 }
 
+/**
+ * Rewrite absolute paths in the staged config.yaml that pointed into the legacy
+ * dir so they name the partition instead. Only `repo.localPath` is persisted as
+ * an absolute path today (the team-repo clone at `<legacyDir>/team-repo`); a path
+ * NOT inside legacyDir (e.g. an http install whose localPath sits elsewhere) is
+ * left untouched. Preserves every other field verbatim via YAML round-trip.
+ */
+async function rebaseConfigPaths(
+  stagedConfig: string,
+  legacyDir: string,
+  partitionDir: string,
+): Promise<void> {
+  const content = await readFileSafe(stagedConfig);
+  if (!content) return;
+  let doc: Record<string, unknown>;
+  try {
+    doc = YAML.parse(content);
+  } catch {
+    return; // verifyStaging already validated parseability; be defensive anyway
+  }
+  const repo = doc?.repo as { localPath?: string } | undefined;
+  const rebased = await rebasePath(repo?.localPath, legacyDir, partitionDir);
+  if (repo && rebased !== undefined && rebased !== repo.localPath) {
+    repo.localPath = rebased;
+    await writeFile(stagedConfig, YAML.stringify(doc));
+  }
+}
+
+/**
+ * If `p` is inside `fromDir`, return the equivalent path inside `toDir`;
+ * otherwise return `p` unchanged (undefined stays undefined).
+ *
+ * `fromDir` is realpath-normalized (it comes from resolveAnchors), but the
+ * persisted `p` may use a symlinked spelling (e.g. macOS `/tmp` → `/private/tmp`)
+ * or a `~` prefix, so a raw string compare would miss the match. We expand `~`
+ * and realpath `p` first — the old location still exists at this point in the
+ * migration (the source is renamed to `.bak` only afterwards) — so both sides are
+ * canonical before path.relative decides containment. A `..` result means `p`
+ * escapes fromDir and is left alone (e.g. an external clone).
+ */
+async function rebasePath(
+  p: string | undefined,
+  fromDir: string,
+  toDir: string,
+): Promise<string | undefined> {
+  if (!p) return p;
+  const expanded = expandHome(p);
+  const canonical = await realpath(expanded).catch(() => expanded);
+  const rel = path.relative(fromDir, canonical);
+  if (rel === '') return toDir;
+  if (rel.startsWith('..') || path.isAbsolute(rel)) return p;
+  return path.join(toDir, rel);
+}
+
+/**
+ * Retire the source dir to a `.bak` sibling (same-fs → atomic rename). Never
+ * auto-deleted: it is the manual rollback path (downgrading to an older teamai
+ * is not supported — see release notes / design doc R6). Returns the backup path.
+ */
+async function retireLegacy(legacyDir: string): Promise<string> {
+  const backup = `${legacyDir}.bak`;
+  await remove(backup);
+  await fse.rename(legacyDir, backup);
+  return backup;
+}
+
 /** Top-level entries under a legacy `.teamai/` that migration will copy. */
 async function listMigratableEntries(legacyDir: string): Promise<string[]> {
   const names = await fse.readdir(legacyDir);
@@ -221,9 +325,12 @@ async function listMigratableEntries(legacyDir: string): Promise<string[]> {
  * Verify a staged copy is complete enough to become authoritative:
  *  - config.yaml parses as a LocalConfig,
  *  - if the source has a team-repo git clone, the staged copy has its `.git`
- *    too (proves the copy did NOT drop `.git` — the copyDir-vs-fse.copy trap),
+ *    AND `git rev-parse HEAD` works on it (proves the copy did NOT drop `.git`
+ *    — the copyDir-vs-fse.copy trap — and the clone is actually usable, not just
+ *    present-but-corrupt),
  *  - every migratable top-level entry made it across.
- * Throws on any shortfall so the caller aborts without touching the source.
+ * Runs on the STAGING copy, before the atomic rename, so any shortfall aborts
+ * with the source untouched and the partial staging discarded.
  */
 async function verifyStaging(legacyDir: string, staging: string): Promise<void> {
   const stagedConfig = path.join(staging, 'config.yaml');
@@ -237,10 +344,23 @@ async function verifyStaging(legacyDir: string, staging: string): Promise<void>
 
   const legacyGit = path.join(legacyDir, 'team-repo', '.git');
   if (await pathExists(legacyGit)) {
-    const stagedGit = path.join(staging, 'team-repo', '.git');
-    if (!(await pathExists(stagedGit))) {
+    const stagedRepo = path.join(staging, 'team-repo');
+    if (!(await pathExists(path.join(stagedRepo, '.git')))) {
       throw new Error('migration verify: team-repo/.git missing after copy (clone would be broken)');
     }
+    // Smoke-check the clone: a working rev-parse proves the .git is intact, not
+    // just present. Catches a partial/corrupt copy that a mere existence check
+    // would wave through.
+    try {
+      const { execFile } = await import('node:child_process');
+      const { promisify } = await import('node:util');
+      await promisify(execFile)('git', ['rev-parse', 'HEAD'], { cwd: stagedRepo });
+    } catch (e) {
+      throw new Error(
+        `migration verify: the staged team-repo clone is not a usable git ` +
+          `repository (${(e as Error).message})`,
+      );
+    }
   }
 
   const expected = await listMigratableEntries(legacyDir);