From f75c5aec0e73bc3b04672e227c79fd0f0e70c9a1 Mon Sep 17 00:00:00 2001
From: Tom Boucher <trekkie@nomorestars.com>
Date: Fri, 24 Apr 2026 18:05:20 -0400
Subject: [PATCH] test(#2638): cover rescue, conflict-resolution, and
 idempotency paths

Adds 3 cases to broaden coverage of the self-heal logic:
- rescue: top-level sub_repos seeds planning.sub_repos when canonical absent
- conflict: canonical planning.sub_repos wins over stale top-level
- idempotent: second loadConfig on canonical config is a no-op
---
 ...2638-sub-repos-canonical-location.test.cjs | 76 +++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/tests/bug-2638-sub-repos-canonical-location.test.cjs b/tests/bug-2638-sub-repos-canonical-location.test.cjs
index 75d2ca07..dbad02c7 100644
--- a/tests/bug-2638-sub-repos-canonical-location.test.cjs
+++ b/tests/bug-2638-sub-repos-canonical-location.test.cjs
@@ -140,4 +140,80 @@ describe('bug #2638 — sub_repos canonical location', () => {
     );
     assert.deepStrictEqual(after.planning?.sub_repos, ['backend']);
   });
+
+  test('rescue: top-level sub_repos seeds planning.sub_repos when canonical slot is empty', () => {
+    makeSubRepo(tmpDir, 'backend');
+    writeConfig(tmpDir, {
+      sub_repos: ['backend'],
+    });
+
+    loadConfig(tmpDir);
+
+    const after = readConfig(tmpDir);
+    assert.strictEqual(
+      Object.prototype.hasOwnProperty.call(after, 'sub_repos'),
+      false,
+      'top-level sub_repos must be removed after rescue'
+    );
+    assert.deepStrictEqual(
+      after.planning?.sub_repos,
+      ['backend'],
+      'top-level value must be promoted to planning.sub_repos'
+    );
+    assert.ok(
+      !stderrCapture.includes('unknown config key'),
+      `rescue path should not warn, got: ${stderrCapture}`
+    );
+  });
+
+  test('conflict resolution: canonical planning.sub_repos wins when both set with different values', () => {
+    makeSubRepo(tmpDir, 'backend');
+    makeSubRepo(tmpDir, 'frontend');
+    // Stale top-level disagrees with canonical; canonical (planning.sub_repos)
+    // is the source of truth per #2561 and must be preserved.
+    writeConfig(tmpDir, {
+      sub_repos: ['stale-old-name'],
+      planning: { sub_repos: ['backend', 'frontend'] },
+    });
+
+    loadConfig(tmpDir);
+
+    const after = readConfig(tmpDir);
+    assert.strictEqual(
+      Object.prototype.hasOwnProperty.call(after, 'sub_repos'),
+      false,
+      'stale top-level sub_repos must be discarded'
+    );
+    assert.deepStrictEqual(
+      after.planning?.sub_repos,
+      ['backend', 'frontend'],
+      'canonical planning.sub_repos must win over stale top-level on conflict'
+    );
+  });
+
+  test('idempotent: a second loadConfig on an already-canonical config is a no-op', () => {
+    makeSubRepo(tmpDir, 'backend');
+    makeSubRepo(tmpDir, 'frontend');
+    writeConfig(tmpDir, {
+      planning: { sub_repos: ['backend', 'frontend'] },
+    });
+
+    loadConfig(tmpDir);
+    const firstPass = fs.readFileSync(
+      path.join(tmpDir, '.planning', 'config.json'),
+      'utf-8'
+    );
+
+    loadConfig(tmpDir);
+    const secondPass = fs.readFileSync(
+      path.join(tmpDir, '.planning', 'config.json'),
+      'utf-8'
+    );
+
+    assert.strictEqual(
+      firstPass,
+      secondPass,
+      'second loadConfig must not modify an already-canonical config'
+    );
+  });
 });