fix(dry-run): thread { dryRun } through the loaders pull, push, status and list use

src/__tests__/dry-run-load-path.test.ts

@@ -31,8 +31,11 @@ vi.mock('../utils/reports-branch.js', async (importOriginal) => ({
 
 import { contribute } from '../contribute.js';
 import { loadLocalConfigForScope } from '../config.js';
+import { pull } from '../pull.js';
+import { push } from '../push.js';
 import { recall } from '../recall.js';
 import { rolesSet } from '../roles-cmd.js';
+import { list, status } from '../status.js';
 import { tagsSubscribe, tagsUnsubscribe } from '../tags.js';
 import { updateReports } from '../utils/reports-branch.js';
 import { log } from '../utils/logger.js';
@@ -236,4 +239,141 @@ describe('--dry-run through the loaders the commands share (#850)', () => {
     expect(loaded?.primaryRole).toBe('hai');
     expect(fs.readFileSync(configPath, 'utf-8')).toContain('primaryRole: hai');
   });
+
+  // The command-level half of #850. Each of these reaches the legacy role
+  // migration through a loader it used to call bare, so the fixture's
+  // `config.yaml` gained `primaryRole` even though nothing had asked to write.
+  // `pull`/`push` carry `--dry-run`; `status`/`list` are read-only and pass it
+  // unconditionally (see the note at their `autoDetectInit` call site).
+  //
+  // The positive control is the test directly above: the SAME fixture does gain
+  // `primaryRole` when the flag is absent, so an unchanged tree here is a real
+  // result and not the harness failing to look.
+  const LOAD_ONLY_COMMANDS: Array<[string, () => Promise<void>]> = [
+    ['pull --dry-run', () => pull({ dryRun: true })],
+    ['push --dry-run', () => push({ dryRun: true })],
+    ['status', () => status({})],
+    ['list', () => list(undefined, {})],
+  ];
+
+  it.each(LOAD_ONLY_COMMANDS)('%s migrates nothing it loads (#850)', async (_command, run) => {
+    const { root, configPath } = legacyRoot();
+    const before = snapshotTree(root);
+    const error = await run().then(() => null, (e: unknown) => e);
+    expect(error).toBeNull();
+    expect(snapshotTree(root)).toEqual(before);
+    expect(fs.readFileSync(configPath, 'utf-8')).not.toContain('primaryRole');
+    // A dry run may parse the remote, but nothing else may reach a provider.
+    expect(providerCalls).toEqual([]);
+  });
+
+  /** A git project whose partition still carries its pre-#546 name, i.e. project scope. */
+  function projectRoot(): string {
+    const root = fs.mkdtempSync(path.join(os.tmpdir(), 'teamai-dry-run-project-'));
+    roots.push(root);
+    const home = path.join(root, 'home');
+    fs.mkdirSync(path.join(home, '.teamai'), { recursive: true });
+    // An installed agent, so a bootstrap that did run would seed and wire it.
+    fs.mkdirSync(path.join(home, '.claude'), { recursive: true });
+    vi.stubEnv('HOME', home);
+    process.chdir(setupLegacyNamedPartition(root));
+    return root;
+  }
+
+  // The project-scope half. These reach the same bare calls through
+  // `detectProjectConfig`, whose dry-run branch is what stops
+  // `adoptLegacyPartition` (a real `fs.rename`) and the self-heal bootstrap.
+  // The issue report located these by code path only; they are run here.
+  const PROJECT_SCOPE_COMMANDS: Array<[string, () => Promise<void>]> = [
+    ['pull --dry-run', () => pull({ dryRun: true })],
+    ['status', () => status({})],
+    ['list', () => list(undefined, {})],
+  ];
+
+  it.each(PROJECT_SCOPE_COMMANDS)('%s adopts no legacy partition on a git project (#850)', async (_command, run) => {
+    const root = projectRoot();
+    const before = snapshotTree(root);
+    const error = await run().then(() => null, (e: unknown) => e);
+    expect(error).toBeNull();
+    expect(snapshotTree(root)).toEqual(before);
+    expect(providerCalls).toEqual([]);
+  });
+});
+
+// The self-mode half of #866. `pull` and `push` are the two commands that take a
+// partition sync-lock, and every fixture above runs them at user scope, or on a
+// project partition that already exists. A fresh self-mode clone is the one
+// shape where the partition does NOT exist yet — so `acquireLock` creating the
+// lock's parent directory creates a directory that nothing removes afterwards.
+// `push` carries a second instance of the same mistake: its self-mode setup
+// (lock, `.teamai/.gitignore` self-heal, knowledge worktree) all runs before
+// `pushCore` reaches its own dry-run guard.
+describe('--dry-run on a fresh self-mode clone (#866)', () => {
+  const originalCwd = process.cwd();
+  let root: string;
+
+  beforeEach(() => {
+    root = fs.mkdtempSync(path.join(os.tmpdir(), 'teamai-dry-run-self-'));
+    const home = path.join(root, 'home');
+    fs.mkdirSync(path.join(home, '.teamai'), { recursive: true });
+    // An installed agent, so a bootstrap that did run would seed and wire it.
+    fs.mkdirSync(path.join(home, '.claude'), { recursive: true });
+    vi.stubEnv('HOME', home);
+    process.chdir(setupSelfModeClone(root));
+    vi.spyOn(log, 'info').mockImplementation(() => {});
+  });
+
+  afterEach(() => {
+    vi.restoreAllMocks();
+    vi.mocked(updateReports).mockClear();
+    providerCalls.length = 0;
+    vi.unstubAllEnvs();
+    process.chdir(originalCwd);
+    fs.rmSync(root, { recursive: true, force: true });
+  });
+
+  // Each command declares exactly which new entries it may leave behind. Both
+  // start empty: the point of #866 is that a preview writes nothing.
+  //
+  // `pull` is allowed one, and it is not this change's. `pull` counts the
+  // contribution queue so it can report how many learnings it would publish, and
+  // `publishQueuedLearnings` lists it through `listPendingForInstall`
+  // (`utils/pending-learnings.ts`), which holds the queue lock — `acquireLock`
+  // creates the lock's parent, so an install with no `<getTeamaiHome>/locks/`
+  // gets one and keeps it. That call is unchanged here, and identical on the
+  // base commit; it only became reachable on a fresh self-mode clone once
+  // detection stopped aborting first (#850). Declared and counted rather than
+  // filtered out, so any OTHER new entry still fails this test.
+  const PULL_LOCK_DIR = `${path.join('home', '.teamai', 'locks')}/`;
+  // `git fetch` — which the preview performs on purpose, so that its plan is
+  // based on the same `origin/<default>` the real push would branch from —
+  // leaves its own one-line record behind. It names no ref, changes no working
+  // tree, and git overwrites it on the next fetch. Same treatment as the entry
+  // above: declared and counted, so any other new entry still fails this test.
+  const FETCH_HEAD = path.join('app', '.git', 'FETCH_HEAD');
+  const SELF_COMMANDS: Array<[string, () => Promise<void>, string[]]> = [
+    ['pull --dry-run', () => pull({ dryRun: true }), [PULL_LOCK_DIR]],
+    ['push --dry-run', () => push({ dryRun: true }), [FETCH_HEAD]],
+  ];
+
+  it.each(SELF_COMMANDS)('%s creates no partition, worktree or lock file', async (_command, run, allowed) => {
+    // The reported symptom, named so a failure here reads as #866 rather than
+    // as an anonymous tree diff: taking the sync-lock used to create this
+    // directory, and releasing it removed the lock file but not the directory.
+    const partitionRoot = path.join(root, 'home', '.teamai', 'projects');
+    expect(fs.existsSync(partitionRoot)).toBe(false);
+    const before = snapshotTree(root);
+    const error = await run().then(() => null, (e: unknown) => e);
+    expect(error).toBeNull();
+    expect(fs.existsSync(partitionRoot)).toBe(false);
+
+    const after = snapshotTree(root);
+    const appeared = Object.keys(after).filter((key) => !(key in before));
+    const vanished = Object.keys(before).filter((key) => !(key in after));
+    expect({ appeared, vanished }).toEqual({ appeared: allowed, vanished: [] });
+    // Nothing that already existed may be rewritten, whatever it is.
+    for (const key of Object.keys(before)) expect(after[key]).toBe(before[key]);
+    // A dry run may parse the remote, but nothing else may reach a provider.
+    expect(providerCalls).toEqual([]);
+  });
 });

src/__tests__/pull-scope-isolation.test.ts

@@ -290,7 +290,11 @@ describe('pull scope isolation (issue #73)', () => {
 
     await pull({ silent: true });
 
-    expect(loadLocalConfigForScope).toHaveBeenCalledWith('user');
+    // The third argument is the LoadOptions the loader now receives so it can
+    // preview instead of migrate (#850). It rides along on every call, so the
+    // assertion has to mention it; `dryRun` is undefined here because this test
+    // drives `pull` without `--dry-run`.
+    expect(loadLocalConfigForScope).toHaveBeenCalledWith('user', undefined, { dryRun: undefined });
     expect(loadStateForScope).toHaveBeenCalledWith(expect.objectContaining({ scope: 'user' }));
     expect(loadStateForScope).toHaveBeenCalledWith(expect.objectContaining({ scope: 'project', projectRoot }));
     expect(log.info).toHaveBeenCalledWith(
@@ -346,7 +350,7 @@ describe('pull scope isolation (issue #73)', () => {
 
     await pull({ silent: true });
 
-    expect(loadLocalConfigForScope).toHaveBeenCalledWith('user');
+    expect(loadLocalConfigForScope).toHaveBeenCalledWith('user', undefined, { dryRun: undefined });
     expect(log.info).not.toHaveBeenCalledWith(SKIP_MSG);
     expect(pullSources).toHaveBeenCalledTimes(1);
     expect(vi.mocked(pullSources).mock.calls[0][0]).toMatchObject({ scope: 'user' });

src/pull.ts

@@ -1863,8 +1863,13 @@ export async function pull(
     // needs no lock.
     if (config.repo.kind === 'http') return true;
     const lock = path.join(getDataHome(config), SYNC_LOCK_FILENAME);
-    if (await acquireLock(lock)) {
-      heldLocks.set(config, lock);
+    // `acquireLock` under a dry run reads the lock's state instead of creating
+    // it — taking one is itself a write (#866) — so the preview answers with
+    // what the real run would have found: a live holder reports this scope as
+    // contended and skips it, exactly as a real pull does. Nothing is recorded
+    // for release, because nothing was taken.
+    if (await acquireLock(lock, { dryRun: options.dryRun })) {
+      if (!options.dryRun) heldLocks.set(config, lock);
       return true;
     }
     // User-visible: this scope is skipped wholesale (no fetch/deploy/reconcile),
@@ -1882,7 +1887,11 @@ export async function pull(
   let projectConfig: LocalConfig | null = null;
   const unreadable: string[] = [];
   try {
-    projectConfig = await detectProjectConfig(undefined, (configPath, error) => { unreadable.push(`${configPath}: ${error}`); });
+    projectConfig = await detectProjectConfig(
+      undefined,
+      (configPath, error) => { unreadable.push(`${configPath}: ${error}`); },
+      { dryRun: options.dryRun },
+    );
   } catch (e) {
     log.warn(`Project-scope detection error: ${(e as Error).message}`);
   }
@@ -1911,7 +1920,7 @@ export async function pull(
     log.info('project scope detected, skipped user scope');
   } else {
     try {
-      const loadedUserConfig = await loadLocalConfigForScope('user');
+      const loadedUserConfig = await loadLocalConfigForScope('user', undefined, { dryRun: options.dryRun });
       if (loadedUserConfig) {
         if (inheritUserScope) {
           inheritedUserConfig = loadedUserConfig;

src/push.ts

@@ -728,7 +728,7 @@ export async function push(
   result?: { completed: boolean },
 ): Promise<void> {
   // Auto-detect scope: project scope if cwd has project config, else user scope
-  const { localConfig, teamConfig } = await autoDetectInit();
+  const { localConfig, teamConfig } = await autoDetectInit(undefined, { dryRun: options.dryRun });
   assertNotReadOnly(localConfig, 'teamai push');
 
   // --project is a destination override expressed as a logical project. Each
@@ -777,8 +777,11 @@ export async function push(
     // migrateSelfA1 takes (for a pre-migration self install that is
     // <repo>/.teamai/.sync-lock). Like git-mode push, error on contention rather
     // than silently skipping (that would drop the user's changes).
+    // Under a dry run `acquireLock` reads the lock's state instead of creating
+    // it (#866), so the preview contends on exactly what a real push would, and
+    // leaves no partition directory behind.
     const selfSyncLock = path.join(getDataHome(localConfig), SYNC_LOCK_FILENAME);
-    if (!(await acquireLock(selfSyncLock))) {
+    if (!(await acquireLock(selfSyncLock, { dryRun: options.dryRun }))) {
       log.error('Another teamai pull/push/migration is in progress for this project. Re-run once it finishes.');
       process.exitCode = 1;
       return;
@@ -787,24 +790,26 @@ export async function push(
       // Self-heal an older .teamai/.gitignore that still ignores `env` (pre-beta.5).
       // Run against the ACTIVE tree (original localConfig, projectRoot intact) BEFORE
       // swapping into the worktree, so the fixed .gitignore lets env changes surface.
-      try {
-        const { migrateSelfModeGitignore } = await import('./init.js');
-        await migrateSelfModeGitignore(localConfig);
-      } catch { /* best-effort */ }
+      // A dry run skips it: it rewrites a tracked file in the user's active tree,
+      // which outlives the preview (#866), and it is idempotent, so the next real
+      // push performs it.
+      if (!options.dryRun) {
+        try {
+          const { migrateSelfModeGitignore } = await import('./init.js');
+          await migrateSelfModeGitignore(localConfig);
+        } catch { /* best-effort */ }
+      }
 
+      // A dry run runs this too, and must: the worktree is not a side effect of
+      // pushing, it is the only source of the CLEAN BASELINE self-mode scanners
+      // compare the active tree against. `env.ts` diffs `projectRoot/.teamai`
+      // against `repo.localPath`, and outside the worktree those are the same
+      // path in self mode — so skipping it makes the preview silently
+      // under-report every edit, rather than merely report it early (#866). It
+      // is disposable: `withKnowledgeWorktree` removes it in a `finally`.
       const { withKnowledgeWorktree, EmptyRepoError } = await import('./utils/reports-branch.js');
       try {
-        const activeConfigPath = path.join(localConfig.repo.localPath, 'teamai.yaml');
-        const activeConfig = await readFileSafe(activeConfigPath);
-        const businessRoot = localConfig.repo.businessRepoRoot ?? localConfig.projectRoot;
-        let pendingTeamConfig: string | null = null;
-        if (activeConfig !== null && businessRoot) {
-          const relativeConfigPath = path.relative(businessRoot, activeConfigPath).split(path.sep).join('/');
-          const committed = await getFileContentAtRev(businessRoot, 'HEAD', relativeConfigPath);
-          if (committed === null || committed.toString() !== activeConfig) {
-            pendingTeamConfig = activeConfig;
-          }
-        }
+        const pendingTeamConfig = await pendingSelfTeamConfig(localConfig);
         await withKnowledgeWorktree(localConfig, async (wtConfig) => {
           if (pendingTeamConfig !== null) {
             await writeFile(path.join(wtConfig.repo.localPath, 'teamai.yaml'), pendingTeamConfig);
@@ -830,7 +835,8 @@ export async function push(
   // A concurrent pull/push would corrupt it. Unlike pull, push must NOT silently
   // skip (that would drop the user's changes), so on contention we error out.
   const syncLock = path.join(getDataHome(localConfig), SYNC_LOCK_FILENAME);
-  const locked = await acquireLock(syncLock);
+  // Read-only acquisition under a dry run, exactly as the self-mode branch above.
+  const locked = await acquireLock(syncLock, { dryRun: options.dryRun });
   if (!locked) {
     log.error('Another teamai pull/push is in progress for this project. Re-run once it finishes.');
     process.exitCode = 1;
@@ -843,6 +849,24 @@ export async function push(
   }
 }
 
+/**
+ * The `teamai.yaml` a self-mode push has to carry, or null when HEAD already
+ * holds it. Read from the ACTIVE tree — the business repo is `businessRepoRoot`,
+ * and the worktree the push swaps into is a detached checkout of the same
+ * commits, so this is the one input `pushCore` cannot rediscover from the
+ * worktree alone. Read-only, which is why the `--dry-run` path calls it too
+ * (#866); writing it into the worktree stays with the real path.
+ */
+async function pendingSelfTeamConfig(localConfig: LocalConfig): Promise<string | null> {
+  const activeConfigPath = path.join(localConfig.repo.localPath, 'teamai.yaml');
+  const activeConfig = await readFileSafe(activeConfigPath);
+  const businessRoot = localConfig.repo.businessRepoRoot ?? localConfig.projectRoot;
+  if (activeConfig === null || !businessRoot) return null;
+  const relativeConfigPath = path.relative(businessRoot, activeConfigPath).split(path.sep).join('/');
+  const committed = await getFileContentAtRev(businessRoot, 'HEAD', relativeConfigPath);
+  return committed === null || committed.toString() !== activeConfig ? activeConfig : null;
+}
+
 async function pushCore(
   localConfig: LocalConfig,
   teamConfig: TeamaiConfig,

src/status.ts

@@ -42,8 +42,14 @@ export async function status(options: GlobalOptions): Promise<void> {
     await statusAll();
     return;
   }
-  // Auto-detect scope
-  const { localConfig, teamConfig } = await autoDetectInit();
+  // Auto-detect scope.
+  // This is a read-only command, so `dryRun` is passed unconditionally rather
+  // than forwarded from `options.dryRun`: the load must never migrate a legacy
+  // role config, adopt a pre-#546 partition, or run the self-heal bootstrap
+  // (#850). The preview path returns what a write would have produced, so the
+  // report below still tells the truth, and the migration then persists on the
+  // next command that writes.
+  const { localConfig, teamConfig } = await autoDetectInit(undefined, { dryRun: true });
   const scopeLabel = localConfig.scope;
 
   // Scope info
@@ -248,8 +254,9 @@ async function statusAll(): Promise<void> {
 }
 
 export async function list(type: string | undefined, options: ListOptions): Promise<void> {
-  // Auto-detect scope
-  const { localConfig, teamConfig } = await autoDetectInit();
+  // Auto-detect scope — read-only, so `dryRun: true` unconditionally, as in
+  // `status` above (#850).
+  const { localConfig, teamConfig } = await autoDetectInit(undefined, { dryRun: true });
   const repoPath = localConfig.repo.localPath;
 
   const source = options.source ?? 'all';

src/update.ts

@@ -379,8 +379,20 @@ async function acquireReclaimSentinel(sentinel: string, owner: string): Promise<
  * residual window exists only if the reclaiming process itself dies mid-reclaim:
  * stealing its dead-pid sentinel is not yet race-free, see #760.)
  */
-export async function acquireLock(lockPath?: string): Promise<boolean> {
+export async function acquireLock(
+  lockPath?: string,
+  options: { dryRun?: boolean } = {},
+): Promise<boolean> {
   const resolved = lockPath ?? expandHome(getUpdateLockPath());
+  // A preview does not take the lock, because taking one is itself a write:
+  // `ensureDir` below creates the lock's parent directory, which a fresh
+  // self-mode clone has no partition for yet and which `releaseLock` has no
+  // reason to remove — the directory would outlive the command (#866). What the
+  // preview still owes its caller is the ANSWER the real command would get, so
+  // it reads the lock's state instead of creating it: a live holder means the
+  // real run would have reported contention, anything else means it would have
+  // won. No owner is recorded, so `releaseLock` has nothing to undo.
+  if (options.dryRun) return (await lockState(resolved)) !== 'live';
   const owner = randomUUID();
   const payload = JSON.stringify({
     pid: process.pid,