fix(#3972): one worktreesOptedOut ladder in planning-workspace, shared by resolver and guard (#3979)
* 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 <sim@local>
This commit is contained in:
5
.changeset/eager-elks-snooze.md
Normal file
5
.changeset/eager-elks-snooze.md
Normal file
@@ -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)
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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';
|
||||
}
|
||||
|
||||
@@ -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<string, unknown>;
|
||||
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 `<root>/.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,
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user