fix(attribution): runGit null-on-failure, versionless v3→v4 migration

z54M (Copilot): runGit returned '' on both successful-empty-output
and silent failure, so a `--name-only` that errored mid-way through
the diff fan-out aliased to a real `--allow-empty` commit. The
empty-commit branch then preserved pending attributions, leaving
the just-committed file's tracked AI edit alive to re-attribute on
the next commit. Switch runGit to `Promise<string | null>`,
distinguishing exit code 0 (any output, including '') from non-zero
(null). The diff-stage fan-out and ancillary probes now treat null
as analysis failure and bail with `return null` instead of falling
into the empty-commit path.

z539 (Copilot): the v3→v4 `shouldMigrate` only fired on
`$version === 3`. A versionless settings file carrying the legacy
`general.gitCoAuthor: false` boolean would skip every migration
(gitCoAuthor isn't in V1_INDICATOR_KEYS — it post-dates V2), get
its `$version` normalized to 4 by the loader, and leave the
boolean in place. The settings dialog then reads the V4
`{commit, pr}` shape, sees missing keys, defaults both to true, and
silently overwrites the user's opt-out on the next save. Also fire
when `$version` is absent AND the value at `general.gitCoAuthor`
is a boolean. Tests cover the new path and confirm the existing
versioned/object-shape paths are untouched.

packages/cli/src/config/migration/versions/v3-to-v4.test.ts

@@ -39,6 +39,35 @@ describe('V3ToV4Migration', () => {
       expect(migration.shouldMigrate('x')).toBe(false);
       expect(migration.shouldMigrate(42)).toBe(false);
     });
+
+    // `gitCoAuthor` post-dates the V1 indicator-key list, so a settings
+    // file that has ONLY this legacy boolean shape (no `$version`,
+    // no other migration-triggering keys) wouldn't fire any earlier
+    // migration. The v3→v4 step must catch it directly so the dialog
+    // doesn't silently overwrite the user's stored opt-out with the
+    // schema defaults on next save.
+    it('returns true for versionless settings with legacy boolean gitCoAuthor', () => {
+      expect(
+        migration.shouldMigrate({
+          general: { gitCoAuthor: false },
+        }),
+      ).toBe(true);
+    });
+
+    it('returns false for versionless settings without gitCoAuthor', () => {
+      expect(migration.shouldMigrate({ general: {} })).toBe(false);
+      expect(migration.shouldMigrate({})).toBe(false);
+    });
+
+    it('returns false for versionless settings with already-object gitCoAuthor', () => {
+      // User who hand-edited to the v4 shape — let the loader's
+      // version normalization handle it without rewriting.
+      expect(
+        migration.shouldMigrate({
+          general: { gitCoAuthor: { commit: false, pr: true } },
+        }),
+      ).toBe(false);
+    });
   });
 
   describe('migrate', () => {

packages/cli/src/config/migration/versions/v3-to-v4.ts

@@ -39,7 +39,25 @@ export class V3ToV4Migration implements SettingsMigration {
       return false;
     }
     const s = settings as Record<string, unknown>;
-    return s['$version'] === 3;
+    if (s['$version'] === 3) {
+      return true;
+    }
+    // Versionless settings file (no $version key) with the legacy
+    // boolean `gitCoAuthor` shape: the V1/V2 migrations don't list
+    // `gitCoAuthor` as an indicator key (it post-dates them), so a
+    // settings file that has ONLY this shape wouldn't trigger any
+    // earlier migration and would land here at the v3→v4 boundary
+    // without being rewritten. Handle the boolean directly so the
+    // settings dialog (which reads the v4 `{commit, pr}` shape) can
+    // surface the user's prior choice instead of silently overwriting
+    // their opt-out with the schema defaults on first save.
+    if (s['$version'] === undefined) {
+      const value = getNestedProperty(s, GIT_CO_AUTHOR_PATH);
+      if (typeof value === 'boolean') {
+        return true;
+      }
+    }
+    return false;
   }
 
   migrate(

packages/core/src/tools/shell.ts

@@ -2122,7 +2122,15 @@ export class ShellToolInvocation extends BaseToolInvocation<
       deletedFiles: new Set(),
     };
 
-    const runGit = async (args: string): Promise<string> => {
+    // Distinguish a successful git command with no output (e.g.
+    // `--allow-empty` -> empty `--name-only` listing) from a failed
+    // git command (silenced by ShellExecutionService) so the caller
+    // can choose between the empty-commit sentinel and the analysis-
+    // failure sentinel. Returning the same `''` for both used to
+    // alias `--allow-empty` to a `--name-only` failure, which left
+    // pending attributions tracked across the just-committed file
+    // and re-attributed it on the next commit.
+    const runGit = async (args: string): Promise<string | null> => {
       const handle = await ShellExecutionService.execute(
         `git ${args}`,
         cwd,
@@ -2132,7 +2140,7 @@ export class ShellToolInvocation extends BaseToolInvocation<
         {},
       );
       const r = await handle.result;
-      return r.exitCode === 0 ? r.output : '';
+      return r.exitCode === 0 ? r.output : null;
     };
 
     try {
@@ -2161,7 +2169,19 @@ export class ShellToolInvocation extends BaseToolInvocation<
           runGit('log -1 --pretty=%P HEAD'),
           runGit('rev-parse --show-toplevel'),
         ]);
-      const hasParent = hasParentOutput.length > 0;
+      // `rev-parse --verify HEAD~1` is allowed to fail (shallow
+      // clone, true root commit) — treat null and '' uniformly.
+      const hasParent = hasParentOutput !== null && hasParentOutput.length > 0;
+      // `log -1 --pretty=%P HEAD` MUST succeed; if git can't read the
+      // current HEAD's metadata we have no way to tell shallow apart
+      // from a real root commit. Bail.
+      if (parentShaOutput === null) {
+        debugLogger.warn(
+          'getCommittedFileInfo: log -1 --pretty=%P HEAD failed; ' +
+            'cannot distinguish shallow clone from true root commit.',
+        );
+        return null;
+      }
       const isTrueRootCommit = parentShaOutput.trim().length === 0;
       // Shallow clone: HEAD has a parent recorded but the object
       // isn't local. Bail rather than over-attribute via --root.
@@ -2177,8 +2197,9 @@ export class ShellToolInvocation extends BaseToolInvocation<
       // reconcile paths from `git diff` (relative to the toplevel)
       // against absolute paths recorded by the edit/write tools.
       // Using the configured target directory as base would zero out
-      // attribution for any file outside it.
-      const repoRoot = repoRootOutput.trim();
+      // attribution for any file outside it. Tolerate failure (null
+      // -> empty string -> caller falls back to targetDir).
+      const repoRoot = (repoRootOutput ?? '').trim();
 
       // Choose the diff range:
       // - amend: `HEAD@{1}..HEAD` — the actual amend delta. The
@@ -2191,8 +2212,8 @@ export class ShellToolInvocation extends BaseToolInvocation<
       let diffArgs: { name: string; status: string; numstat: string };
       if (isAmend) {
         // Verify HEAD@{1} actually exists; reflogs can be GC'd.
-        const hasReflog =
-          (await runGit('rev-parse --verify HEAD@{1}')).length > 0;
+        const reflogProbe = await runGit('rev-parse --verify HEAD@{1}');
+        const hasReflog = reflogProbe !== null && reflogProbe.length > 0;
         if (!hasReflog) {
           // Without a pre-amend snapshot we can't compute the amend
           // delta; emitting `HEAD~1..HEAD` would over-attribute.
@@ -2226,6 +2247,23 @@ export class ShellToolInvocation extends BaseToolInvocation<
         runGit(diffArgs.numstat),
       ]);
 
+      // ANY of the three diffs failing (null) is an analysis failure,
+      // NOT an empty commit. Without this check, a `--name-only` that
+      // failed silently used to alias to `--allow-empty`, leaving the
+      // just-committed file's tracked AI edit in the singleton and
+      // re-attributing it to the next commit.
+      if (
+        nameOutput === null ||
+        statusOutput === null ||
+        numstatOutput === null
+      ) {
+        debugLogger.warn(
+          'getCommittedFileInfo: one or more diff calls failed; ' +
+            'cannot distinguish empty commit from analysis failure.',
+        );
+        return null;
+      }
+
       const files = nameOutput
         .split('\n')
         .map((f) => f.trim())