From 3ab24c6c569186e6d7dd6238ce8bbcb1132b7e7e Mon Sep 17 00:00:00 2001
From: Tom Boucher <trekkie@nomorestars.com>
Date: Thu, 14 May 2026 19:03:39 -0400
Subject: [PATCH] fix(3523): self-healing migration of legacy top-level
 branching_strategy
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Three-part fix for the false "unknown config key(s)" warning fired for
top-level `branching_strategy` in .planning/config.json:

1. On-disk migration (option 3, mirroring multiRepo → planning.sub_repos):
   When loadConfig reads a config.json with top-level `branching_strategy`
   set and `git.branching_strategy` unset, it grafts the value into
   `git.branching_strategy` and deletes the top-level key, then persists.
   If `git.branching_strategy` is already set, the nested value wins
   (matches SDK mergeDefaults precedence, PR #3116).

2. KNOWN_TOP_LEVEL safety net: 'branching_strategy' added to the deprecated-
   keys bucket so the warning never fires even on the first read of a root
   config that feeds a workstream merge (where `parsed` may still carry it).

3. Double-emission guard: a module-level `_warnedUnknownConfigKeys` Set
   deduplicates the unknown-key warning across multiple loadConfig calls
   within a single CLI invocation (init phase-op N called it twice).

Closes #3523

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 get-shit-done/bin/lib/core.cjs | 38 ++++++++++++++++++++++++++++++----
 1 file changed, 34 insertions(+), 4 deletions(-)

diff --git a/get-shit-done/bin/lib/core.cjs b/get-shit-done/bin/lib/core.cjs
index 98c62630f..6454da631 100644
--- a/get-shit-done/bin/lib/core.cjs
+++ b/get-shit-done/bin/lib/core.cjs
@@ -399,6 +399,21 @@ function loadConfig(cwd, options = {}) {
       configDirty = true;
     }
 
+    // #3523 — Migrate legacy top-level branching_strategy → git.branching_strategy.
+    // Canonical location is git.branching_strategy (per config-schema.cjs); writing
+    // at the top level trips the unknown-key warning even though loadConfig:485 actively
+    // reads it via the nested fallback. This migration mirrors the multiRepo → sub_repos
+    // precedent: graft then delete so the warning never fires again on this project.
+    // The nested value wins if already set (matches SDK mergeDefaults precedence, PR #3116).
+    if (Object.prototype.hasOwnProperty.call(fileData, 'branching_strategy')) {
+      if (!fileData.git) fileData.git = {};
+      if (!fileData.git.branching_strategy) {
+        fileData.git.branching_strategy = fileData.branching_strategy;
+      }
+      delete fileData.branching_strategy;
+      configDirty = true;
+    }
+
     // Keep planning.sub_repos in sync with actual filesystem
     const currentSubRepos = fileData.planning?.sub_repos || [];
     if (Array.isArray(currentSubRepos) && currentSubRepos.length > 0) {
@@ -439,13 +454,23 @@ function loadConfig(cwd, options = {}) {
       // Internal keys loadConfig reads but config-set doesn't expose
       'model_overrides', 'context_window', 'resolve_model_ids', 'claude_md_path',
       // Deprecated keys (still accepted for migration, not in config-set)
-      'depth', 'multiRepo',
+      // 'branching_strategy' is kept here as a safety net: it is migrated to
+      // git.branching_strategy above (#3523), but on the first read of a root
+      // config that feeds into a workstream merge, `parsed` may still surface it.
+      'depth', 'multiRepo', 'branching_strategy',
     ]);
     const unknownKeys = Object.keys(parsed).filter(k => !KNOWN_TOP_LEVEL.has(k));
     if (unknownKeys.length > 0) {
-      process.stderr.write(
-        `gsd-tools: warning: unknown config key(s) in .planning/config.json: ${unknownKeys.join(', ')} — these will be ignored\n`
-      );
+      // Deduplicate: a single `init phase-op N` invocation calls loadConfig twice
+      // (once for the sub-command setup, once for git-config resolution). Guard with
+      // a module-level Set so the same message never fires more than once per process.
+      const warnKey = unknownKeys.join(',');
+      if (!_warnedUnknownConfigKeys.has(warnKey)) {
+        _warnedUnknownConfigKeys.add(warnKey);
+        process.stderr.write(
+          `gsd-tools: warning: unknown config key(s) in .planning/config.json: ${unknownKeys.join(', ')} — these will be ignored\n`
+        );
+      }
     }
 
     // #2517 — Validate runtime/tier values for keys that loadConfig handles but
@@ -585,6 +610,11 @@ function loadConfig(cwd, options = {}) {
 
 // ─── Git utilities ────────────────────────────────────────────────────────────
 
+// Module-level deduplication for unknown-key warnings (#3523).
+// A single `init phase-op N` call invokes loadConfig more than once; this Set
+// prevents the same warning from being echoed on each invocation.
+const _warnedUnknownConfigKeys = new Set();
+
 const _gitIgnoredCache = new Map();
 
 function isGitIgnored(cwd, targetPath) {