From 5ad708aa4ebd728dd5e40b01907a17f605bd1a8d Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 28 Aug 2026 01:53:23 -0400 Subject: [PATCH] fix(#3972): one worktreesOptedOut ladder in planning-workspace, shared by resolver and guard (#3979) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(#3972): the guard fallback must share the opt-out ladder * fix(#3972): one worktreesOptedOut ladder in planning-workspace, shared by resolver and guard * test(#3972): without a workstream the root config is the effective config * fix(#3972): guard the whole ladder body — malformed env values degrade, never throw * chore(#3972): changeset fragment (pr number backfilled after PR creation) * chore(#3972): backfill changeset PR number (3979) --------- Co-authored-by: sim --- .changeset/eager-elks-snooze.md | 5 ++ gsd-core/bin/gsd-tools.cjs | 40 +++--------- hooks/gsd-agent-isolation-guard.js | 42 ++++++++++--- src/planning-workspace.cts | 53 ++++++++++++++++ tests/gsd-agent-isolation-guard.test.cjs | 78 ++++++++++++++++++++++++ 5 files changed, 175 insertions(+), 43 deletions(-) create mode 100644 .changeset/eager-elks-snooze.md diff --git a/.changeset/eager-elks-snooze.md b/.changeset/eager-elks-snooze.md new file mode 100644 index 000000000..61696cd76 --- /dev/null +++ b/.changeset/eager-elks-snooze.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3979 +--- +**The isolation guard no longer denies sequential dispatches a workstream explicitly opted out of** — the sentinel-absent fallback now reads `workflow.use_worktrees` through the same project/workstream-aware ladder as the resolver and `config-get`, instead of the flat root config where a workstream-local opt-out was invisible. (#3972) diff --git a/gsd-core/bin/gsd-tools.cjs b/gsd-core/bin/gsd-tools.cjs index 2621f1705..ab6327769 100755 --- a/gsd-core/bin/gsd-tools.cjs +++ b/gsd-core/bin/gsd-tools.cjs @@ -1751,40 +1751,14 @@ function dispatchOverlayCapabilityCommand({ command, args, cwd, raw, error, load * comment above). Never throws. */ function projectWorktreesOptedOut(cwd) { - // #3963: the read must see the value `config-get` sees. Under - // GSD_WORKSTREAM, config-get inherits the ROOT config for keys the - // workstream config omits (config.cts resolveFromRootConfig, the #2714 - // inheritance; loadConfig's deep-merge implements the same ladder for - // its consumers), so a root-level `use_worktrees: false` is part of the - // effective config even when the workstream file omits the key. Mirror - // that ladder for this one key with direct reads — never loadConfig, - // which normalizes and rewrites config on a path that backs sentinel - // writes (same discipline as the resolveRuntime comment in - // resolveDispatchIsolationDecision). - // Semantics: the workstream's OWN key wins; otherwise the root's (only - // under the ws-env gate — loadConfigResolved does NOT inherit root under - // GSD_PROJECT alone, and this read must not diverge from that); strict - // `=== false`; any read failure degrades to "not opted out". + // #3972: single owner — planning-workspace's worktreesOptedOut ladder + // (scoped own-key, root inheritance under the ws gate, strict === false). + // Kept as a local name so routeDispatchIsolation's call sites read the + // same as they did in #3938/#3963; the logic itself now lives beside + // planningDir/planningRoot where every isolation surface can share it. try { - const { planningDir, planningRoot } = require('./lib/planning-workspace.cjs'); - const readCfg = (p) => { - try { - return JSON.parse(require('fs').readFileSync(p, 'utf8')); - } catch { - return null; - } - }; - const ownKey = (cfg) => - cfg != null && typeof cfg === 'object' - && cfg.workflow != null && typeof cfg.workflow === 'object' - && Object.prototype.hasOwnProperty.call(cfg.workflow, 'use_worktrees'); - const wsCfg = readCfg(require('path').join(planningDir(cwd), 'config.json')); - if (ownKey(wsCfg)) return wsCfg.workflow.use_worktrees === false; - if (process.env['GSD_WORKSTREAM']) { - const rootCfg = readCfg(require('path').join(planningRoot(cwd), 'config.json')); - if (ownKey(rootCfg)) return rootCfg.workflow.use_worktrees === false; - } - return false; + const { worktreesOptedOut } = require('./lib/planning-workspace.cjs'); + return worktreesOptedOut(cwd); } catch { return false; } diff --git a/hooks/gsd-agent-isolation-guard.js b/hooks/gsd-agent-isolation-guard.js index 05c3d3766..9a9ef31dd 100644 --- a/hooks/gsd-agent-isolation-guard.js +++ b/hooks/gsd-agent-isolation-guard.js @@ -299,18 +299,40 @@ function resolveRegistryIsolation(cwd, configPath) { if (isolation === 'harness-worktree') { let useWorktrees = true; + // #3972: the opt-out read shares the ONE owner every other + // isolation-deciding surface uses — planning-workspace's + // worktreesOptedOut ladder (scoped own-key wins; root inherited under + // the GSD_WORKSTREAM gate; strict === false). A flat single-file read + // here made a workstream-LOCAL opt-out invisible, so the sentinel-absent + // fallback denied a sequential dispatch the config explicitly allowed. + // Reached through ensureRuntimeBuild() like every other compiled-lib + // require in this file (scripts/lint-hooks-runtime-build-seam.cjs); if + // the library is unreachable the flat-root read below remains as the + // degraded fallback — today's behavior, never worse. + let ladderAnswered = false; try { - const raw = fs.readFileSync(configPath, 'utf-8'); - const parsedCfg = JSON.parse(raw); - if (parsedCfg && typeof parsedCfg === 'object' && parsedCfg.workflow && - typeof parsedCfg.workflow === 'object' && parsedCfg.workflow.use_worktrees === false) { - useWorktrees = false; - } + ensureRuntimeBuild(); + const { worktreesOptedOut } = require('../gsd-core/bin/lib/planning-workspace.cjs'); + useWorktrees = !worktreesOptedOut(cwd); + ladderAnswered = true; } catch { - // Unreadable config already propagated to the outer caller's catch - // before this point in practice (resolveRuntimeIdentity reads it - // first); tolerate defensively and keep the conservative (enforce) - // default rather than silently disabling the guard. + // Unbuilt runtime library or a ladder failure — fall through to the + // legacy flat-root read (conservative: keep enforcing). + } + if (!ladderAnswered) { + try { + const raw = fs.readFileSync(configPath, 'utf-8'); + const parsedCfg = JSON.parse(raw); + if (parsedCfg && typeof parsedCfg === 'object' && parsedCfg.workflow && + typeof parsedCfg.workflow === 'object' && parsedCfg.workflow.use_worktrees === false) { + useWorktrees = false; + } + } catch { + // Unreadable config already propagated to the outer caller's catch + // before this point in practice (resolveRuntimeIdentity reads it + // first); tolerate defensively and keep the conservative (enforce) + // default rather than silently disabling the guard. + } } if (!useWorktrees) isolation = 'none'; } diff --git a/src/planning-workspace.cts b/src/planning-workspace.cts index 528463862..1f559ee00 100644 --- a/src/planning-workspace.cts +++ b/src/planning-workspace.cts @@ -144,6 +144,58 @@ function planningRoot(cwd: string): string { return path.join(cwd, '.planning'); } +/** + * #3972: the ONE owner of "is this planning scope opted out of worktrees?" — + * the effective `workflow.use_worktrees === false` read every + * isolation-deciding surface must share (config-get's merged view is the + * contract). Ladder: the scoped config's OWN key wins (planningDir is + * project- and workstream-aware); otherwise the flat root's key, but only + * under the GSD_WORKSTREAM env gate — config-get deliberately does NOT + * inherit root under GSD_PROJECT alone, and this read must not diverge + * (#3963). Strict `=== false` (never coerced); any read failure degrades to + * "not opted out" (worktrees on — the fail-safe direction: the guard keeps + * enforcing). Direct file reads only — never loadConfig, which normalizes + * and rewrites config on paths that back sentinel writes. + */ +function worktreesOptedOut(cwd: string): boolean { + // #3972 review: the WHOLE body is guarded — planningDir/planningRoot + // themselves throw on a GSD_PROJECT/GSD_WORKSTREAM value containing path + // separators or `..`, and this contract ("any failure degrades to not + // opted out — worktrees on, keep enforcing") must hold for that shape too. + try { + return worktreesOptedOutUnguarded(cwd); + } catch { + return false; + } +} + +function worktreesOptedOutUnguarded(cwd: string): boolean { + type MaybeConfig = { workflow?: unknown } | null; + const readCfg = (p: string): MaybeConfig => { + try { + return JSON.parse(String(fs.readFileSync(p, 'utf8'))) as MaybeConfig; + } catch { + return null; + } + }; + const ownKey = (cfg: MaybeConfig): { present: boolean; value: unknown } => { + if (cfg === null || typeof cfg !== 'object') return { present: false, value: undefined }; + const wf = cfg.workflow; + if (wf === null || typeof wf !== 'object' || Array.isArray(wf)) return { present: false, value: undefined }; + const wfRec = wf as Record; + return Object.prototype.hasOwnProperty.call(wfRec, 'use_worktrees') + ? { present: true, value: wfRec['use_worktrees'] } + : { present: false, value: undefined }; + }; + const scoped = ownKey(readCfg(path.join(planningDir(cwd), 'config.json'))); + if (scoped.present) return scoped.value === false; + if (process.env['GSD_WORKSTREAM']) { + const root = ownKey(readCfg(path.join(planningRoot(cwd), 'config.json'))); + if (root.present) return root.value === false; + } + return false; +} + // Sorted list of workstream directory names under `/.planning/workstreams`, // or `[]` when the project is flat (no workstreams dir). Single source of truth // for the "workstream mode" detection shared by the #1912/#2028 fail-safe guards @@ -469,6 +521,7 @@ function findContextMdIn(absDirOrFiles: string | string[]): string | null { } export = { + worktreesOptedOut, createPlanningWorkspace, createSharedPointerAdapter, createSessionScopedPointerAdapter, diff --git a/tests/gsd-agent-isolation-guard.test.cjs b/tests/gsd-agent-isolation-guard.test.cjs index 3ddc27942..a85b44007 100644 --- a/tests/gsd-agent-isolation-guard.test.cjs +++ b/tests/gsd-agent-isolation-guard.test.cjs @@ -1552,3 +1552,81 @@ describe('gsd-agent-isolation-guard.js: #3582 cold tree — RuntimeBuildError su assert.ok(out.reason.length > 0); }); }); + +// ─── #3972: the guard's sentinel-absent fallback shares the opt-out ladder ─── +// The guard's own config read was flat-root, so a workstream-LOCAL +// use_worktrees=false was invisible to it: with no sentinel present (fresh +// checkout) the fallback denied the sequential dispatch the config +// explicitly allowed. The ladder now lives in planning-workspace and both +// the resolver and the guard consume it. +describe('guard fallback — worktreesOptedOut ladder (#3972)', () => { + function wsFixture(rootCfg, wsCfg) { + const dir = createTempDir('gsd-3972-guard-'); + fs.mkdirSync(path.join(dir, '.planning', 'workstreams', 'alpha'), { recursive: true }); + fs.writeFileSync(path.join(dir, '.planning', 'config.json'), JSON.stringify(rootCfg)); + fs.writeFileSync(path.join(dir, '.planning', 'workstreams', 'alpha', 'config.json'), JSON.stringify(wsCfg)); + return dir; + } + + test('#3972: a workstream-local use_worktrees=false opts the fallback out', (t) => { + const dir = wsFixture({ runtime: 'claude' }, { runtime: 'claude', workflow: { use_worktrees: false } }); + t.after(() => cleanup(dir)); + assert.equal(fs.existsSync(sentinelFile(dir)), false, 'fixture: sentinel absent — the fallback is under test'); + const r = runHook(agentPayload(), dir, { GSD_WORKSTREAM: 'alpha' }); + assert.equal(r.status, 0, + `#3972: the workstream opted out — the sentinel-absent fallback must allow; got stdout=${r.stdout}`); + }); + + test('#3972: a root-only opt-out under a workstream also allows (the #3963 shape, guard side)', (t) => { + const dir = wsFixture({ runtime: 'claude', workflow: { use_worktrees: false } }, { runtime: 'claude' }); + t.after(() => cleanup(dir)); + const r = runHook(agentPayload(), dir, { GSD_WORKSTREAM: 'alpha' }); + assert.equal(r.status, 0, 'the inherited root opt-out must reach the fallback too'); + }); + + test('#3972: no opt-out anywhere still denies (unchanged)', (t) => { + const dir = wsFixture({ runtime: 'claude' }, { runtime: 'claude' }); + t.after(() => cleanup(dir)); + const r = runHook(agentPayload(), dir, { GSD_WORKSTREAM: 'alpha' }); + assert.equal(r.status, 2, 'the guard must keep demanding the harness flag when nothing opted out'); + }); +}); + +describe('worktreesOptedOut — ladder unit semantics (#3972)', () => { + const { worktreesOptedOut } = require('../gsd-core/bin/lib/planning-workspace.cjs'); + + test('scoped own-key wins; root inherited only under the ws gate; strict === false', (t) => { + const dir = createTempDir('gsd-3972-unit-'); + t.after(() => cleanup(dir)); + fs.mkdirSync(path.join(dir, '.planning', 'workstreams', 'alpha'), { recursive: true }); + const root = path.join(dir, '.planning', 'config.json'); + const ws = path.join(dir, '.planning', 'workstreams', 'alpha', 'config.json'); + const prev = process.env['GSD_WORKSTREAM']; + t.after(() => { if (prev === undefined) delete process.env['GSD_WORKSTREAM']; else process.env['GSD_WORKSTREAM'] = prev; }); + + fs.writeFileSync(ws, JSON.stringify({ workflow: { use_worktrees: false } })); + process.env['GSD_WORKSTREAM'] = 'alpha'; + assert.equal(worktreesOptedOut(dir), true, 'scoped own false'); + + fs.writeFileSync(ws, JSON.stringify({ workflow: { use_worktrees: true } })); + assert.equal(worktreesOptedOut(dir), false, 'scoped own true wins over any root'); + + fs.writeFileSync(ws, JSON.stringify({ runtime: 'claude' })); + fs.writeFileSync(root, JSON.stringify({ workflow: { use_worktrees: false } })); + assert.equal(worktreesOptedOut(dir), true, 'root false inherited under the ws gate'); + + delete process.env['GSD_WORKSTREAM']; + // Without a workstream, planningDir IS the flat root — the root's own key + // is the scoped read (the plain-project opt-out), so this answers true. + // The no-cross-inheritance contract (a GSD_PROJECT-scoped dir ignoring the + // flat root) is pinned by the resolver-level #3963 boundary test. + assert.equal(worktreesOptedOut(dir), true, 'no ws: the root config IS the effective config'); + + fs.writeFileSync(ws, JSON.stringify({ workflow: { use_worktrees: 'false' } })); + process.env['GSD_WORKSTREAM'] = 'alpha'; + assert.equal(worktreesOptedOut(dir), false, 'string "false" never coerces'); + + fs.writeFileSync(ws, '{ malformed'); + assert.equal(worktreesOptedOut(dir), true, 'unreadable scoped config falls to the root view under the gate'); + }); +});