diff --git a/.changeset/kind-jaguars-romp.md b/.changeset/kind-jaguars-romp.md new file mode 100644 index 000000000..ba88a2043 --- /dev/null +++ b/.changeset/kind-jaguars-romp.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3616 +--- +**A terminal session now follows the workstream your repo says is active** — with `.planning/active-workstream` naming a workstream, any invocation that had never run `workstream use` silently resolved the flat `.planning/` tree instead: it misreported milestone, phase and progress on reads, and wrote to the superseded flat `STATE.md`. Because the stale tree is well-formed, nothing warned, and the documented workaround was to prepend `GSD_WORKSTREAM=` or `--ws` to every command. A session that has never set its own pointer now inherits the repo marker. Session isolation is unchanged — a session that owns a pointer is never repointed — and the two workstream-mode fail-safe guards now say whether a marker exists but failed to resolve, instead of claiming none is set. Note that clearing a session's pointer returns it to inheriting the marker rather than forcing flat mode. (#3579) diff --git a/gsd-core/bin/gsd-tools.cjs b/gsd-core/bin/gsd-tools.cjs index a939c072d..311cfa444 100755 --- a/gsd-core/bin/gsd-tools.cjs +++ b/gsd-core/bin/gsd-tools.cjs @@ -271,8 +271,7 @@ try { } } catch { /* advisory — never block */ } -const { getActiveWorkstream } = require('./lib/planning-workspace.cjs'); -const { resolveActiveWorkstream, applyResolvedWorkstreamEnv } = require('./lib/active-workstream-store.cjs'); +const { resolveActiveWorkstream, applyResolvedWorkstreamEnv, peekActiveWorkstream } = require('./lib/active-workstream-store.cjs'); const state = require('./lib/state.cjs'); const phase = require('./lib/phase.cjs'); const roadmap = require('./lib/roadmap.cjs'); @@ -1838,11 +1837,14 @@ function dispatchOverlayCapabilityCommand({ command, args, cwd, raw, error, load // 1. SENTINEL-FREE, not write-free. This route writes nothing itself, and // in particular never writes .gsd/dispatch-isolation-sentinel.json — // the only write that can hard-block a later executor dispatch. It is - // NOT a claim of total filesystem purity: like every gsd-tools - // invocation, it runs the shared bootstrap and active-workstream - // resolution first, and getActiveWorkstream self-heals (unlinks) a - // stale or invalid pointer. That is pre-existing, verb-independent, - // and harmless to dispatch. + // NOT an unconditional claim of total filesystem purity: like every + // gsd-tools invocation, it runs the shared bootstrap and + // active-workstream resolution first. As of #3579's root-cause fix + // that bootstrap resolves via the non-mutating peekActiveWorkstream + // (never unlinks); an actual stale/invalid pointer is still + // self-healed, but only by whichever verb's own getActiveWorkstream + // call later consumes it for real — this inspection route makes no + // such call, so it is now also side-effect-free on the pointer file. // // 2. SHARED NEGOTIATION, for the arguments this verb accepts. Both verbs // call resolveDispatchIsolationDecision, so the natural resolution @@ -4125,8 +4127,21 @@ async function main() { // Priority: --ws flag > GSD_WORKSTREAM env var > session/shared pointer > null. let workstreamContext = null; try { + // #3579 root-cause fix: this bootstrap resolution only decides whether to + // populate GSD_WORKSTREAM env for downstream routing — it is a check, not + // the consuming read. Using the mutating getActiveWorkstream here + // self-healed (cleared) a present-but-unresolvable pointer BEFORE the + // dispatched command's own resolution/diagnostic ran, so a second read in + // the same process (e.g. a subcommand's own getActiveWorkstream call, or + // a fail-safe guard's diagnoseUnresolvedActiveWorkstream) observed + // already-cleared state — silently falling through to a fallback marker + // it should never have inherited (isolation violation), or losing the + // evidence a diagnostic needed to explain why nothing resolved. peek + // shares the identical resolution logic and only differs by never + // calling adapter.clear(); self-heal still happens, exactly once, at + // whichever call site actually consumes the workstream for real. workstreamContext = resolveActiveWorkstream(cwd, args, process.env, { - getStored: getActiveWorkstream, + getStored: peekActiveWorkstream, }); args = workstreamContext.args; // Set env var so all modules (planningDir, planningPaths) auto-resolve workstream paths. diff --git a/gsd-core/references/workstream-flag.md b/gsd-core/references/workstream-flag.md index 8f3aaff42..624945440 100644 --- a/gsd-core/references/workstream-flag.md +++ b/gsd-core/references/workstream-flag.md @@ -9,8 +9,12 @@ parallel milestone work by multiple Claude Code instances on the same codebase. 1. `--ws ` flag (explicit, highest priority) 2. `GSD_WORKSTREAM` environment variable (per-instance) -3. Session-scoped active workstream pointer in temp storage (per runtime session / terminal) -4. `.planning/active-workstream` file (legacy shared fallback when no session key exists) +3. Session-scoped active workstream pointer in temp storage (per runtime session / terminal), + when that pointer exists and is non-blank +4. `.planning/active-workstream` file — consulted whenever step 3 has nothing to say: either + there is no session identity at all, or there is one but it has never pointed at a + workstream. A session that already has its own pointer (step 3) is never overridden by + this step, even if that pointer is stale. 5. `null` — flat mode (no workstreams) ## Why session-scoped pointers exist @@ -25,6 +29,11 @@ GSD now prefers a session-scoped pointer keyed by runtime/session identity or the controlling TTY). This keeps concurrent sessions isolated while preserving legacy compatibility for runtimes that do not expose a stable session key. +A session that has never set its own pointer inherits `.planning/active-workstream` +(step 4) rather than silently falling back to flat mode — this does not weaken the +isolation guarantee above: inheritance only fires when a session's own pointer is +absent, and a session that has ever set one is never repointed by the shared file. + ## Session Identity Resolution When GSD resolves the session-scoped pointer in step 3 above, it uses this order: @@ -48,7 +57,13 @@ routing hot path. Session-scoped pointers are intentionally lightweight and best-effort: -- Clearing a workstream for one session removes only that session's pointer file +- Clearing a workstream for one session removes only that session's pointer file. + This returns that session to step 4 of Resolution Priority above — it goes back + to **inheriting** `.planning/active-workstream` (if a marker exists there), not + to flat mode. A cleared session with no marker present resolves to `null`; a + cleared session with a marker present resolves to whatever that marker names. + To force flat mode for a cleared session, remove the shared marker file, or use + an explicit override such as `--ws` / `GSD_WORKSTREAM` on the command in question. - If that was the last pointer for the repo, GSD also removes the now-empty per-project temp directory - If sibling session pointers still exist, the temp directory is left in place @@ -77,7 +92,7 @@ This ensures workstream scope chains automatically through the workflow: ├── config.json # Shared ├── milestones/ # Shared ├── codebase/ # Shared -├── active-workstream # Legacy shared fallback only +├── active-workstream # Shared marker; inherited when a session has no pointer of its own └── workstreams/ ├── feature-a/ # Workstream A │ ├── STATE.md diff --git a/src/active-workstream-store.cts b/src/active-workstream-store.cts index 6062ac483..d3ec5c42a 100644 --- a/src/active-workstream-store.cts +++ b/src/active-workstream-store.cts @@ -222,23 +222,142 @@ function pickActiveWorkstreamAdapter(cwd: string, opts: ActiveWorkstreamOpts = { return createSharedPointerAdapter(cwd); } +/** + * Read-resolution chain for getActiveWorkstream/peekActiveWorkstream (#3579). + * + * pickActiveWorkstreamAdapter (above) picks exactly one adapter and remains + * the seam for WRITE paths (set/clear), where "which pointer do I mutate" has + * only one right answer: the session pointer when a session key exists, + * otherwise the shared marker. Reads are different — a session that has + * never called `workstream use` has no opinion of its own, so it should + * inherit the repo-wide `.planning/active-workstream` marker rather than + * resolve to nothing. This returns an ORDERED chain: [owned, ...fallbacks]. + * `chain[0]` ("owned") is exactly what pickActiveWorkstreamAdapter would have + * returned — resolveFromChain() self-heals only chain[0], never a fallback, + * so one session's read can never delete another scope's marker. Fallbacks + * are consulted ONLY when chain[0].read() comes back absent/empty; a session + * with its own (even stale/invalid) pointer never falls through — that is + * the isolation guarantee and it must not be weakened by inheritance. + */ +function pickActiveWorkstreamAdapterChain(cwd: string, opts: ActiveWorkstreamOpts = {}): WorkstreamPointerAdapter[] { + if (opts.activeWorkstreamAdapter) { + return [opts.activeWorkstreamAdapter]; + } + + // #3579 item 3: when a caller supplies `opts.activeWorkstreamAdapters` at + // all, honor ONLY what it provides. The prior `|| createXPointerAdapter(...)` + // fallback synthesized a REAL filesystem adapter for whichever half a test + // double omitted — so a test injecting only `{ session }` silently touched + // the real shared marker file, and one injecting only `{ shared }` silently + // touched the real session-scoped tmp file. A missing half now gets a + // no-op in-memory adapter (always reads null) instead — this preserves the + // chain[0]-is-owned / rest-are-fallback shape resolveFromChain relies on + // without ever reaching disk. A caller that wants a real adapter for one + // half can still construct and pass it explicitly. + const injected = opts.activeWorkstreamAdapters; + const sessionKey = getWorkstreamSessionKey(); + + if (!sessionKey) { + const shared = injected + ? (injected.shared ?? createMemoryPointerAdapter(null)) + : createSharedPointerAdapter(cwd); + return [shared]; + } + + const session = injected + ? (injected.session ?? createMemoryPointerAdapter(null)) + : createSessionScopedPointerAdapter(cwd, sessionKey); + const shared = injected + ? (injected.shared ?? createMemoryPointerAdapter(null)) + : createSharedPointerAdapter(cwd); + + return session ? [session, shared] : [shared]; +} + +/** + * Shared "does this stored name resolve" predicate — format-valid AND its + * workstream directory exists. Factored out so resolveFromChain's owned/ + * fallback arms (and diagnoseUnresolvedActiveWorkstream, #3579 item 1) share + * one definition of "resolvable" instead of re-deriving the same two checks. + */ +function resolvesToExistingWorkstream(cwd: string, name: string | null): name is string { + if (!name || !validateWorkstreamName(name)) return false; + return fs.existsSync(path.join(planningRoot(cwd), 'workstreams', name)); +} + +/** + * Resolves a stored workstream name by walking an adapter chain. + * + * chain[0] is "owned" by this resolution: an absent/empty read falls through + * to the next adapter, but a present-and-bad read (invalid name, or a name + * whose workstream dir no longer exists) is resolved right there — self- + * healed via adapter.clear() when `selfHeal` is true, and never consulted + * further. Anything after chain[0] is a read-only fallback (the inherited + * marker): a bad value there resolves to null WITHOUT ever calling clear(), + * so a pointer-less session's read can never delete the shared marker that + * other sessions/scopes still depend on. + */ +function resolveFromChain(cwd: string, chain: WorkstreamPointerAdapter[], selfHeal: boolean): string | null { + if (chain.length === 0) return null; + const [owned, ...fallbacks] = chain; + + const ownedName = owned.read(); + if (ownedName) { + if (!resolvesToExistingWorkstream(cwd, ownedName)) { + if (selfHeal) owned.clear(); + return null; + } + return ownedName; + } + + for (const adapter of fallbacks) { + const name = adapter.read(); + if (resolvesToExistingWorkstream(cwd, name)) return name; + } + + return null; +} + +/** + * Diagnostic sibling of resolveFromChain (#3579 item 1). getActiveWorkstream/ + * peekActiveWorkstream collapse EVERY unresolvable case to `null`, which is + * exactly right for routing — but a fail-safe guard reporting "no active + * workstream is set" to an operator needs to distinguish two very different + * situations that both produce that same `null`: + * + * (a) no marker/pointer exists anywhere in the chain at all, vs. + * (b) a marker/pointer EXISTS (names a value) but that value didn't + * resolve — either the name fails validateWorkstreamName, or it's a + * well-formed name whose `workstreams/` directory is missing. + * + * Walks the same chain resolveFromChain uses and, for the first adapter that + * held a non-empty raw value, reports why it didn't resolve. Read-only: never + * calls adapter.clear() (mirrors peekActiveWorkstream, not getActiveWorkstream + * — a diagnostic read must not have side effects). Reuses + * resolvesToExistingWorkstream so this can never disagree with the actual + * resolution predicate above. + */ +function diagnoseUnresolvedActiveWorkstream( + cwd: string, + opts: ActiveWorkstreamOpts = {}, +): { present: boolean; value: string | null; reason: 'invalid_name' | 'missing_workstream_dir' | null } { + const chain = pickActiveWorkstreamAdapterChain(cwd, opts); + for (const adapter of chain) { + const raw = adapter.read(); + if (!raw) continue; + if (resolvesToExistingWorkstream(cwd, raw)) continue; + return { + present: true, + value: raw, + reason: validateWorkstreamName(raw) ? 'missing_workstream_dir' : 'invalid_name', + }; + } + return { present: false, value: null, reason: null }; +} + function getActiveWorkstream(cwd: string, opts: ActiveWorkstreamOpts = {}): string | null { - const adapter = pickActiveWorkstreamAdapter(cwd, opts); - if (!adapter) return null; - - const name = adapter.read(); - if (!name || !validateWorkstreamName(name)) { - adapter.clear(); - return null; - } - - const wsDir = path.join(planningRoot(cwd), 'workstreams', name); - if (!fs.existsSync(wsDir)) { - adapter.clear(); - return null; - } - - return name; + const chain = pickActiveWorkstreamAdapterChain(cwd, opts); + return resolveFromChain(cwd, chain, true); } /** @@ -254,16 +373,8 @@ function getActiveWorkstream(cwd: string, opts: ActiveWorkstreamOpts = {}): stri * pointer file is left exactly as it was for whatever created it to fix. */ function peekActiveWorkstream(cwd: string, opts: ActiveWorkstreamOpts = {}): string | null { - const adapter = pickActiveWorkstreamAdapter(cwd, opts); - if (!adapter) return null; - - const name = adapter.read(); - if (!name || !validateWorkstreamName(name)) return null; - - const wsDir = path.join(planningRoot(cwd), 'workstreams', name); - if (!fs.existsSync(wsDir)) return null; - - return name; + const chain = pickActiveWorkstreamAdapterChain(cwd, opts); + return resolveFromChain(cwd, chain, false); } function setActiveWorkstream(cwd: string, name: string | null | undefined, opts: ActiveWorkstreamOpts = {}): void { @@ -381,8 +492,10 @@ export = { createSessionScopedPointerAdapter, createMemoryPointerAdapter, pickActiveWorkstreamAdapter, + pickActiveWorkstreamAdapterChain, getActiveWorkstream, peekActiveWorkstream, + diagnoseUnresolvedActiveWorkstream, setActiveWorkstream, clearActiveWorkstream, parseCliWorkstream, diff --git a/src/init.cts b/src/init.cts index b4cdf6c8c..fbd16f7aa 100644 --- a/src/init.cts +++ b/src/init.cts @@ -78,7 +78,7 @@ const { listCodebaseMapFiles, } = onboardProjection; -const { output, error } = io; +const { output, error, ERROR_REASON } = io; const { loadConfig, loadConfigResolved } = configLoader; const { resolveModelInternal, resolveGranularityInternal, assertValidGranularityOverride } = modelResolver; const { findPhaseInternal, listMilestonePhaseDirs } = phaseLocator; @@ -97,7 +97,9 @@ const { planningDir, planningRoot, listAvailableWorkstreams, - getActiveWorkstream, + peekActiveWorkstream, + diagnoseUnresolvedActiveWorkstream, + describeUnresolvedWorkstreamReason, findContextMdIn, } = planningWorkspace; @@ -1372,7 +1374,13 @@ function cmdInitNewMilestone(cwd: string, raw: boolean, options: Record = {}): void { - const resolvedWorkstream = process.env['GSD_WORKSTREAM'] || getActiveWorkstream(cwd); + // #3579 root-cause fix: read-only informational field — peek, don't + // self-heal (see cmdInitNewMilestone's identical rationale above). + const resolvedWorkstream = process.env['GSD_WORKSTREAM'] || peekActiveWorkstream(cwd); const workstreamActive = !!resolvedWorkstream; const result: Record = { @@ -2914,12 +2924,33 @@ function cmdInitProgress(cwd: string, raw: boolean, options: Record stored active pointer) so // an explicit --ws (which sets GSD_WORKSTREAM) satisfies the check. const _availableWorkstreams = listAvailableWorkstreams(cwd); - const _resolvedWorkstream = process.env['GSD_WORKSTREAM'] || getActiveWorkstream(cwd); + // #3579 root-cause fix: this is a check, not a consuming read — use the + // non-mutating peek so an unresolvable pointer isn't self-healed (cleared) + // here and then found "absent" by diagnoseUnresolvedActiveWorkstream below, + // which would misreport a present-but-bad marker as no marker at all. + const _resolvedWorkstream = process.env['GSD_WORKSTREAM'] || peekActiveWorkstream(cwd); if (_availableWorkstreams.length > 0 && !_resolvedWorkstream) { + // #3579: getActiveWorkstream now inherits a pointer-less session's read + // from the shared .planning/active-workstream marker, so reaching this + // branch with a marker actually present means the marker EXISTED but + // didn't resolve (invalid name, or its workstream dir is gone) — a + // materially different situation from "nothing was ever set" and one + // that deserves its own diagnostic instead of the generic message below. + const _diagnosis = diagnoseUnresolvedActiveWorkstream(cwd); + if (_diagnosis.present) { + error( + `init.progress requires a workstream in workstream mode — the active-workstream marker names '${_diagnosis.value}', but it did not resolve: ${describeUnresolvedWorkstreamReason(_diagnosis.reason)}. Root STATE.md (likely stale) would be reported otherwise. ` + + `Pass --ws or run ${formatGsdSlash('workstream set', _slashRuntime) as string} to point it at an existing workstream. ` + + `Available workstreams: ${_availableWorkstreams.join(', ')}`, + ERROR_REASON.WORKSTREAM_MODE_MARKER_UNRESOLVED, + { marker_value: _diagnosis.value, marker_reason: _diagnosis.reason }, + ); + } error( `init.progress requires a workstream in workstream mode — no active workstream is set, so root STATE.md (likely stale) would be reported. ` + `Pass --ws or run ${formatGsdSlash('workstream set', _slashRuntime) as string} first. ` + `Available workstreams: ${_availableWorkstreams.join(', ')}`, + ERROR_REASON.WORKSTREAM_MODE_NONE_ACTIVE, ); } diff --git a/src/io.cts b/src/io.cts index 7bbe0223b..f575fe883 100644 --- a/src/io.cts +++ b/src/io.cts @@ -192,6 +192,12 @@ const ERROR_REASON = Object.freeze({ PHASE_VERIFICATION_INCOMPLETE: 'phase_verification_incomplete', PHASE_PLAN_COVERAGE_INCOMPLETE: 'phase_plan_coverage_incomplete', SUMMARY_NO_PLANNING: 'summary_no_planning', + // #3579: workstream-mode fail-safe guards (init.progress, phase.complete) — + // distinguishes "no marker/pointer anywhere" from "a marker exists but + // didn't resolve" so a JSON-error-mode caller can branch on `reason` + // instead of regexing the human message. + WORKSTREAM_MODE_NONE_ACTIVE: 'workstream_mode_none_active', + WORKSTREAM_MODE_MARKER_UNRESOLVED: 'workstream_mode_marker_unresolved', // graphify GRAPHIFY_NO_GRAPH: 'graphify_no_graph', GRAPHIFY_INVALID_QUERY: 'graphify_invalid_query', diff --git a/src/phase.cts b/src/phase.cts index 60ec38a02..f7b349946 100644 --- a/src/phase.cts +++ b/src/phase.cts @@ -82,8 +82,10 @@ const { readVerificationStatus } = verificationMod; import planDependencyGraphMod = require('./plan-dependency-graph.cjs'); const { computeHaltPropagation, buildSummaryFileIndex, isSummaryFileHalted, isSummaryFileBlocked } = planDependencyGraphMod; -const { planningDir, withPlanningLock, listAvailableWorkstreams, getActiveWorkstream } = - planningWorkspace; +const { + planningDir, withPlanningLock, listAvailableWorkstreams, + peekActiveWorkstream, diagnoseUnresolvedActiveWorkstream, describeUnresolvedWorkstreamReason, +} = planningWorkspace; // eslint-disable-next-line @typescript-eslint/no-require-imports -- milestone-lock.cjs is an export= CommonJS module import milestoneLockMod = require('./milestone-lock.cjs'); const { extractFrontmatter } = frontmatterMod; @@ -2208,12 +2210,33 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void { // init.progress got (resolution: GSD_WORKSTREAM env > stored active pointer; an // explicit --ws sets GSD_WORKSTREAM upstream and satisfies the check). const availableWorkstreams = listAvailableWorkstreams(cwd); - const resolvedWorkstream = process.env['GSD_WORKSTREAM'] || getActiveWorkstream(cwd); + // #3579 root-cause fix: this is a check, not a consuming read — use the + // non-mutating peek so an unresolvable pointer isn't self-healed (cleared) + // here and then found "absent" by diagnoseUnresolvedActiveWorkstream below, + // which would misreport a present-but-bad marker as no marker at all. + const resolvedWorkstream = process.env['GSD_WORKSTREAM'] || peekActiveWorkstream(cwd); if (availableWorkstreams.length > 0 && !resolvedWorkstream) { + // #3579: getActiveWorkstream now inherits a pointer-less session's read + // from the shared .planning/active-workstream marker, so reaching this + // branch with a marker actually present means the marker EXISTED but + // didn't resolve (invalid name, or its workstream dir is gone) — a + // materially different situation from "nothing was ever set" and one + // that deserves its own diagnostic instead of the generic message below. + const diagnosis = diagnoseUnresolvedActiveWorkstream(cwd); + if (diagnosis.present) { + error( + `phase.complete requires a workstream in workstream mode — the active-workstream marker names '${diagnosis.value}', but it did not resolve: ${describeUnresolvedWorkstreamReason(diagnosis.reason)}. Root STATE.md/ROADMAP.md (likely stale) would be written otherwise. ` + + `Pass --ws or run ${formatGsdSlash('workstream set', resolveRuntime(cwd)) as string} to point it at an existing workstream. ` + + `Available workstreams: ${availableWorkstreams.join(', ')}`, + ERROR_REASON.WORKSTREAM_MODE_MARKER_UNRESOLVED, + { marker_value: diagnosis.value, marker_reason: diagnosis.reason }, + ); + } error( `phase.complete requires a workstream in workstream mode — no active workstream is set, so root STATE.md/ROADMAP.md (likely stale) would be written. ` + `Pass --ws or run ${formatGsdSlash('workstream set', resolveRuntime(cwd)) as string} first. ` + `Available workstreams: ${availableWorkstreams.join(', ')}`, + ERROR_REASON.WORKSTREAM_MODE_NONE_ACTIVE, ); } diff --git a/src/planning-workspace.cts b/src/planning-workspace.cts index 1957cfc76..528463862 100644 --- a/src/planning-workspace.cts +++ b/src/planning-workspace.cts @@ -25,8 +25,10 @@ const { createSessionScopedPointerAdapter, createMemoryPointerAdapter, getActiveWorkstream: getStoredActiveWorkstream, + peekActiveWorkstream: peekStoredActiveWorkstream, setActiveWorkstream: setStoredActiveWorkstream, clearActiveWorkstream: clearStoredActiveWorkstream, + diagnoseUnresolvedActiveWorkstream: diagnoseUnresolvedStoredActiveWorkstream, } = activeWorkstreamStore; // Track .planning/.lock files held by this process so they can be removed on exit. @@ -391,10 +393,47 @@ function getActiveWorkstream(cwd: string): string | null { return getStoredActiveWorkstream(cwd); } +// #3579 root-cause fix: read-only sibling of getActiveWorkstream, thin +// pass-through to active-workstream-store's non-mutating peek. Callers that +// only need to KNOW whether a workstream resolves (a guard's initial check, +// a bootstrap that will re-derive the answer anyway) must use this instead +// of getActiveWorkstream — the mutating variant self-heals (clears) a +// present-but-unresolvable chain[0] value, and a later read in the SAME +// process (another getActiveWorkstream call, or diagnoseUnresolvedActiveWorkstream) +// would then observe already-cleared state instead of the original evidence, +// silently changing the answer or losing the diagnostic reason. Self-heal +// still happens — exactly once, wherever the real consuming call site invokes +// getActiveWorkstream — this sibling just avoids triggering it prematurely. +function peekActiveWorkstream(cwd: string): string | null { + return peekStoredActiveWorkstream(cwd); +} + function setActiveWorkstream(cwd: string, name: string): void { setStoredActiveWorkstream(cwd, name); } +// #3579 item 1: read-only diagnostic sibling of getActiveWorkstream, thin +// pass-through to active-workstream-store's chain walk. Lets the #1912/#2028 +// fail-safe guards (init.progress, phase.complete) distinguish "no marker at +// all" from "a marker exists but didn't resolve" without duplicating the +// resolution predicate. +function diagnoseUnresolvedActiveWorkstream(cwd: string): { + present: boolean; + value: string | null; + reason: 'invalid_name' | 'missing_workstream_dir' | null; +} { + return diagnoseUnresolvedStoredActiveWorkstream(cwd); +} + +// #3579 item 1: human-readable clause for diagnoseUnresolvedActiveWorkstream's +// `reason`, shared by the init.progress and phase.complete fail-safe guards so +// the two error messages describe the same failure the same way instead of +// drifting (CLAUDE.md's Generative Fix Divergence anti-pattern). +function describeUnresolvedWorkstreamReason(reason: 'invalid_name' | 'missing_workstream_dir' | null): string { + if (reason === 'invalid_name') return 'the name is not a valid workstream name'; + return "its workstream directory doesn't exist (it may have been renamed or removed)"; +} + /** * Locate the CONTEXT.md file in a phase directory, handling both the bare * form (`CONTEXT.md`) and the padded-prefix convention (`NN-CONTEXT.md`, @@ -441,7 +480,10 @@ export = { quickDirFrom, withPlanningLock, getActiveWorkstream, + peekActiveWorkstream, setActiveWorkstream, + diagnoseUnresolvedActiveWorkstream, + describeUnresolvedWorkstreamReason, findContextMdIn, // Test seam (audit M1): inject a deterministic isPidAlive so the liveness-gated // steal decision is exercised without real pids. Mirrors capability-lock.cts. diff --git a/tests/active-workstream-store.unit.test.cjs b/tests/active-workstream-store.unit.test.cjs index b9ca7a7d3..1ea70f27e 100644 --- a/tests/active-workstream-store.unit.test.cjs +++ b/tests/active-workstream-store.unit.test.cjs @@ -19,7 +19,10 @@ const { createSessionScopedPointerAdapter, createMemoryPointerAdapter, pickActiveWorkstreamAdapter, + pickActiveWorkstreamAdapterChain, getActiveWorkstream, + peekActiveWorkstream, + diagnoseUnresolvedActiveWorkstream, setActiveWorkstream, clearActiveWorkstream, parseCliWorkstream, @@ -328,6 +331,95 @@ describe('pickActiveWorkstreamAdapter', () => { }); }); +// ── pickActiveWorkstreamAdapterChain (#3579) ───────────────────────────────── +// +// Each test below is written to KILL a specific surviving mutant reported by +// Stryker on this function: the `if (!sessionKey)` guard (and its block-body +// removal), the two `?? createMemoryPointerAdapter(null)` fallback sites, and +// the `session ? [session, shared] : []` shape mutant. + +describe('pickActiveWorkstreamAdapterChain', () => { + let saved; + beforeEach(() => { + saved = saveSessionEnv(); + clearSessionEnv(); + }); + afterEach(() => restoreSessionEnv(saved)); + + test('returns single-element [shared] chain when no session key present', () => { + // Kills: `if (!sessionKey)` → `if (false)`, and the BlockStatement removal + // on that guard's body. Both mutants would instead fall into the + // sessionKey-truthy branch and, since `injected.session` is supplied, + // return a 2-element [session, shared] chain instead of [shared]. + const shared = createMemoryPointerAdapter('shared-ws'); + const session = createMemoryPointerAdapter('session-ws'); + const chain = pickActiveWorkstreamAdapterChain('/fake', { + activeWorkstreamAdapters: { session, shared }, + }); + assert.equal(chain.length, 1); + assert.strictEqual(chain[0], shared); + }); + + test('returns [session, shared] chain (length 2, in order) when session key present', () => { + // Kills: `return session ? [session, shared] : []` → `: []`. A mutant + // there collapses the chain to empty even though session key is set. + process.env.GSD_SESSION_KEY = 'chain-session'; + const shared = createMemoryPointerAdapter('shared-ws'); + const session = createMemoryPointerAdapter('session-ws'); + const chain = pickActiveWorkstreamAdapterChain('/fake', { + activeWorkstreamAdapters: { session, shared }, + }); + assert.equal(chain.length, 2); + assert.strictEqual(chain[0], session); + assert.strictEqual(chain[1], shared); + }); + + test('no session key + adapters given without shared: fallback is inert memory adapter', () => { + // Kills: `injected.shared ?? createMemoryPointerAdapter(null)` → `&&` on + // the no-sessionKey branch. Under the mutant, `undefined && ...` is + // `undefined`, so chain[0] would be undefined instead of a usable adapter. + const chain = pickActiveWorkstreamAdapterChain('/fake', { + activeWorkstreamAdapters: {}, + }); + assert.equal(chain.length, 1); + assert.notEqual(chain[0], undefined); + assert.equal(typeof chain[0].read, 'function'); + assert.equal(chain[0].read(), null); + }); + + test('session key present, only session injected: shared half is inert memory adapter', () => { + // Kills: `injected.shared ?? createMemoryPointerAdapter(null)` → `&&` on + // the sessionKey-truthy branch (the shared fallback site). + process.env.GSD_SESSION_KEY = 'chain-partial-shared'; + const session = createMemoryPointerAdapter('session-ws'); + const chain = pickActiveWorkstreamAdapterChain('/fake', { + activeWorkstreamAdapters: { session }, + }); + assert.equal(chain.length, 2); + assert.strictEqual(chain[0], session); + assert.notEqual(chain[1], undefined); + assert.equal(typeof chain[1].read, 'function'); + assert.equal(chain[1].read(), null); + // in-memory only — proves it never touches the real shared marker file + chain[1].write('should-not-persist'); + assert.equal(chain[1].read(), 'should-not-persist'); + }); + + test('session key present, only shared injected: session half is inert memory adapter', () => { + // Kills: `injected.session ?? createMemoryPointerAdapter(null)` → `&&`. + process.env.GSD_SESSION_KEY = 'chain-partial-session'; + const shared = createMemoryPointerAdapter('shared-ws'); + const chain = pickActiveWorkstreamAdapterChain('/fake', { + activeWorkstreamAdapters: { shared }, + }); + assert.equal(chain.length, 2); + assert.notEqual(chain[0], undefined); + assert.equal(typeof chain[0].read, 'function'); + assert.equal(chain[0].read(), null); + assert.strictEqual(chain[1], shared); + }); +}); + // ── getWorkstreamSessionKey ─────────────────────────────────────────────────── describe('getWorkstreamSessionKey', () => { @@ -550,6 +642,173 @@ describe('getActiveWorkstream', () => { }); }); +// ── resolvesToExistingWorkstream (#3579, exercised via getActiveWorkstream) ── +// +// resolvesToExistingWorkstream is internal; these two tests reach it through +// its public callers and are designed to disagree with two survived mutants: +// `if (!name || !validateWorkstreamName(name))` → `if (false)` (name check +// skipped entirely) and → `if (false)`/`&&` (the `||` weakened to `&&`). Both +// tests plant a REAL directory at the exact stored name so that if the name +// check is bypassed, `fs.existsSync` would wrongly say "resolved". + +describe('resolvesToExistingWorkstream — name-check short-circuit (#3579)', () => { + let tmpDir; + let saved; + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-unit-resolves-')); + saved = saveSessionEnv(); + clearSessionEnv(); + }); + afterEach(() => { + restoreSessionEnv(saved); + cleanup(tmpDir); + }); + + test('empty name from a fallback adapter never resolves, even though the workstreams dir itself exists', () => { + // Kills `if (!name || ...)` → `if (false)`: without the early return, the + // mutant checks fs.existsSync(join(root, 'workstreams', '')), which + // collapses to the 'workstreams' dir itself — which DOES exist here. + makePlanningDir(tmpDir); // creates .planning/workstreams with no children + process.env.GSD_SESSION_KEY = 'resolves-empty-name'; + const session = createMemoryPointerAdapter(''); // owned: falls through + const shared = createMemoryPointerAdapter(''); // fallback: also empty + const result = getActiveWorkstream(tmpDir, { + activeWorkstreamAdapters: { session, shared }, + }); + assert.equal(result, null); + }); + + test('non-empty malformed name never resolves, even when a matching directory exists on disk', () => { + // Kills both `if (false)` (name check skipped) and `||` → `&&` (since + // !name is false here, only the format-check operand is true — OR yields + // true/unresolvable, AND yields false/would-check-disk). We create a + // literal 'bad name!' directory so a mutant that reaches fs.existsSync + // reports true instead of the correct "invalid format" false. + makePlanningDir(tmpDir, 'bad name!'); + const adapter = createMemoryPointerAdapter('bad name!'); + const result = getActiveWorkstream(tmpDir, { activeWorkstreamAdapter: adapter }); + assert.equal(result, null); + assert.equal(adapter.read(), null); // self-healed: cleared as unresolvable + }); +}); + +// ── resolveFromChain (#3579, exercised via getActiveWorkstream/peekActiveWorkstream) ── + +describe('resolveFromChain — owned/fallback/selfHeal discrimination (#3579)', () => { + let tmpDir; + let saved; + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-unit-fromchain-')); + saved = saveSessionEnv(); + clearSessionEnv(); + }); + afterEach(() => { + restoreSessionEnv(saved); + cleanup(tmpDir); + }); + + test('falls through to a resolvable fallback when the owned pointer is empty', () => { + // Kills `if (ownedName) {` → `if (true)`: the mutant would treat a null + // ownedName as needing resolution, resolve it to null immediately, and + // return before ever consulting the fallback — losing the real + // 'fallback-ws' result. Also kills the `for (const adapter of fallbacks)` + // BlockStatement removal (an emptied loop body would likewise never find + // the match and fall through to `return null`). + makePlanningDir(tmpDir, 'fallback-ws'); + process.env.GSD_SESSION_KEY = 'owned-empty-1'; + const session = createMemoryPointerAdapter(null); + const shared = createMemoryPointerAdapter('fallback-ws'); + const result = getActiveWorkstream(tmpDir, { + activeWorkstreamAdapters: { session, shared }, + }); + assert.equal(result, 'fallback-ws'); + }); + + test('an unresolvable fallback name still resolves to null (never returned as-is)', () => { + // Kills the fallback `if (resolvesToExistingWorkstream(cwd, name))` → + // `if (true)`: the mutant would return the bogus fallback name + // unconditionally instead of continuing to null. + makePlanningDir(tmpDir); // workstreams root exists, 'ghost-fallback' does not + process.env.GSD_SESSION_KEY = 'owned-empty-2'; + const session = createMemoryPointerAdapter(null); + const shared = createMemoryPointerAdapter('ghost-fallback'); + const result = getActiveWorkstream(tmpDir, { + activeWorkstreamAdapters: { session, shared }, + }); + assert.equal(result, null); + }); + + test('peekActiveWorkstream (selfHeal=false) does NOT clear a stale owned pointer', () => { + // Kills `if (selfHeal)` → `if (true)`: the mutant would clear the + // adapter unconditionally, even for the read-only peek path. + makePlanningDir(tmpDir); // 'stale-ws' has no matching dir + const adapter = createMemoryPointerAdapter('stale-ws'); + const result = peekActiveWorkstream(tmpDir, { activeWorkstreamAdapter: adapter }); + assert.equal(result, null); + assert.equal(adapter.read(), 'stale-ws'); // untouched — proves no self-heal + }); +}); + +// ── diagnoseUnresolvedActiveWorkstream (#3579) ──────────────────────────────── +// +// Each test asserts the FULL returned object (present/value/reason) for one +// of the three distinguishable outcomes, which is what kills the +// `present: true` → `present: false` and the StringLiteral ('' for the +// reason strings) mutants — a partial assertion on just `present` or `value` +// would leave those survivable. + +describe('diagnoseUnresolvedActiveWorkstream — full-object assertions (#3579)', () => { + let tmpDir; + let saved; + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-unit-diagnose-')); + saved = saveSessionEnv(); + clearSessionEnv(); + }); + afterEach(() => { + restoreSessionEnv(saved); + cleanup(tmpDir); + }); + + test('nothing set anywhere: present:false, value:null, reason:null', () => { + // Kills `if (!raw)` → `raw` (inverted) and → `if (false)`: either mutant + // would fall through the `continue` and wrongly report presence for a + // null/empty raw value. + const adapter = createMemoryPointerAdapter(null); + const result = diagnoseUnresolvedActiveWorkstream(tmpDir, { activeWorkstreamAdapter: adapter }); + assert.deepEqual(result, { present: false, value: null, reason: null }); + }); + + test('well-formed name with no matching workstream dir: reason is exactly "missing_workstream_dir"', () => { + // Kills `if (!raw)` → `if (true)` (would always `continue`, hiding this + // case as present:false), the fallback `if (resolvesToExistingWorkstream(...))` + // → `if (true)`, the `present: true` → `present: false` mutant, and the + // StringLiteral mutant on 'missing_workstream_dir'. + makePlanningDir(tmpDir); // workstreams root exists, 'ghost-ws' does not + const adapter = createMemoryPointerAdapter('ghost-ws'); + const result = diagnoseUnresolvedActiveWorkstream(tmpDir, { activeWorkstreamAdapter: adapter }); + assert.deepEqual(result, { present: true, value: 'ghost-ws', reason: 'missing_workstream_dir' }); + }); + + test('malformed name: reason is exactly "invalid_name"', () => { + // Kills the StringLiteral mutant on 'invalid_name' and the ternary's + // false-branch selection (validateWorkstreamName(raw) ? ... : 'invalid_name'). + const adapter = createMemoryPointerAdapter('bad name!'); + const result = diagnoseUnresolvedActiveWorkstream(tmpDir, { activeWorkstreamAdapter: adapter }); + assert.deepEqual(result, { present: true, value: 'bad name!', reason: 'invalid_name' }); + }); + + test('a stored pointer that actually resolves reports present:false (nothing unresolved)', () => { + // Kills the fallback `if (resolvesToExistingWorkstream(cwd, raw)) continue;` + // → `if (false)`: the mutant would never skip a resolving value and + // wrongly report it as an unresolved marker. + makePlanningDir(tmpDir, 'good-ws'); + const adapter = createMemoryPointerAdapter('good-ws'); + const result = diagnoseUnresolvedActiveWorkstream(tmpDir, { activeWorkstreamAdapter: adapter }); + assert.deepEqual(result, { present: false, value: null, reason: null }); + }); +}); + // ── setActiveWorkstream ─────────────────────────────────────────────────────── describe('setActiveWorkstream', () => { diff --git a/tests/workstream.test.cjs b/tests/workstream.test.cjs index 0455e3720..7d9b396f0 100644 --- a/tests/workstream.test.cjs +++ b/tests/workstream.test.cjs @@ -167,6 +167,17 @@ describe('session-scoped active workstream routing', () => { }); test('clearing one session does not clear another session pointer', () => { + // A prior test in this shared-tmpDir block ('session-scoped pointer ignores + // legacy shared active-workstream file') writes 'beta' into the shared + // .planning/active-workstream marker. Under #3579 semantics, a cleared + // session INHERITS that marker instead of resolving flat — so leaving the + // marker in place here would make this test assert marker-inheritance + // behavior it never intended to cover (that is pinned separately below). + // This test's real intent is sibling-pointer isolation, not inheritance, + // so remove the marker to make alpha's post-clear resolution independent + // of whatever an earlier test in this block left on disk. + try { fs.unlinkSync(path.join(tmpDir, '.planning', 'active-workstream')); } catch {} + const clearAlpha = runGsdTools(['workstream', 'set', '--clear', '--raw'], tmpDir, { GSD_SESSION_KEY: 'session-alpha' }); const alpha = runGsdTools(['workstream', 'get'], tmpDir, { GSD_SESSION_KEY: 'session-alpha' }); const beta = runGsdTools(['workstream', 'get', '--raw'], tmpDir, { GSD_SESSION_KEY: 'session-beta' }); @@ -242,6 +253,220 @@ describe('session resolution hardening', () => { }); }); +// #3579: a session that has an identity (session key) but has never run +// `workstream use` must inherit the repo-wide `.planning/active-workstream` +// marker instead of resolving to nothing. peekActiveWorkstream has no CLI +// surface (only the statusline hook calls it), so these tests require the +// built module directly — same pattern as the planning-workspace.cjs require +// used elsewhere in this file. +describe('session inherits shared marker when pointer-less (#3579)', () => { + let tmpDir; + const { + getActiveWorkstream, + peekActiveWorkstream, + } = require('../gsd-core/bin/lib/active-workstream-store.cjs'); + + beforeEach(() => { + tmpDir = createFixture(); + + for (const [ws, status] of [['alpha', 'Alpha active'], ['beta', 'Beta active']]) { + const wsDir = path.join(tmpDir, '.planning', 'workstreams', ws); + fs.mkdirSync(path.join(wsDir, 'phases'), { recursive: true }); + fs.writeFileSync(path.join(wsDir, 'STATE.md'), `# State\n**Status:** ${status}\n`); + } + }); + + afterEach(() => cleanup(tmpDir)); + + test('session key present, no session pointer, marker names an existing workstream -> inherits the marker', () => { + fs.writeFileSync(path.join(tmpDir, '.planning', 'active-workstream'), 'alpha\n'); + + const result = runGsdTools(['workstream', 'get', '--raw'], tmpDir, { GSD_SESSION_KEY: 'no-pointer-session' }); + + assert.ok(result.success, `get failed: ${result.error}`); + assert.strictEqual(result.output, 'alpha'); + }); + + test('peekActiveWorkstream inherits the marker and mutates nothing', (t) => { + const markerPath = path.join(tmpDir, '.planning', 'active-workstream'); + fs.writeFileSync(markerPath, 'alpha\n'); + const sessionDir = getSessionPointerDir(tmpDir); + const savedSessionKey = process.env.GSD_SESSION_KEY; + process.env.GSD_SESSION_KEY = 'peek-no-pointer'; + t.after(() => { + if (savedSessionKey !== undefined) process.env.GSD_SESSION_KEY = savedSessionKey; + else delete process.env.GSD_SESSION_KEY; + }); + + const resolved = peekActiveWorkstream(tmpDir); + assert.strictEqual(resolved, 'alpha'); + assert.strictEqual(fs.readFileSync(markerPath, 'utf-8'), 'alpha\n', 'peek must not mutate the shared marker'); + assert.ok(!fs.existsSync(sessionDir), 'peek must not create a session pointer file'); + }); + + test('session pointer names beta while marker names alpha -> resolves beta, marker untouched (isolation)', () => { + const markerPath = path.join(tmpDir, '.planning', 'active-workstream'); + fs.writeFileSync(markerPath, 'alpha\n'); + runGsdTools(['workstream', 'set', 'beta', '--raw'], tmpDir, { GSD_SESSION_KEY: 'has-own-pointer' }); + + const result = runGsdTools(['workstream', 'get', '--raw'], tmpDir, { GSD_SESSION_KEY: 'has-own-pointer' }); + + assert.ok(result.success, `get failed: ${result.error}`); + assert.strictEqual(result.output, 'beta'); + assert.strictEqual(fs.readFileSync(markerPath, 'utf-8'), 'alpha\n', 'session with its own pointer must never repoint the shared marker'); + }); + + test('workstream set --clear with a valid repo marker present -> session returns to inheriting the marker, not flat', () => { + const markerPath = path.join(tmpDir, '.planning', 'active-workstream'); + fs.writeFileSync(markerPath, 'alpha\n'); + runGsdTools(['workstream', 'set', 'beta', '--raw'], tmpDir, { GSD_SESSION_KEY: 'clear-then-inherit-session' }); + + const clear = runGsdTools(['workstream', 'set', '--clear', '--raw'], tmpDir, { GSD_SESSION_KEY: 'clear-then-inherit-session' }); + const result = runGsdTools(['workstream', 'get', '--raw'], tmpDir, { GSD_SESSION_KEY: 'clear-then-inherit-session' }); + + assert.ok(clear.success, `clear failed: ${clear.error}`); + assert.ok(result.success, `get after clear failed: ${result.error}`); + assert.strictEqual( + result.output, + 'alpha', + 'clearing a session pointer must fall back to inheriting the shared marker (step 4), not resolve to null/flat', + ); + }); + + test('no session key, marker present -> resolves the marker (pre-existing path stays green)', () => { + fs.writeFileSync(path.join(tmpDir, '.planning', 'active-workstream'), 'alpha\n'); + + const result = runGsdTools(['workstream', 'get', '--raw'], tmpDir, {}); + + assert.ok(result.success, `get failed: ${result.error}`); + assert.strictEqual(result.output, 'alpha'); + }); + + test('no workstreams/ dir -> flat/null, unchanged', (t) => { + // createFixture() only creates .planning/phases, not .planning/workstreams. + const flatDir = createFixture(); + t.after(() => cleanup(flatDir)); + + const savedSessionKey = process.env.GSD_SESSION_KEY; + process.env.GSD_SESSION_KEY = 'flat-no-workstreams-dir'; + t.after(() => { + if (savedSessionKey !== undefined) process.env.GSD_SESSION_KEY = savedSessionKey; + else delete process.env.GSD_SESSION_KEY; + }); + + assert.strictEqual(getActiveWorkstream(flatDir), null); + assert.strictEqual(peekActiveWorkstream(flatDir), null); + }); + + test('marker names a non-existent workstream, session has no pointer -> resolves null and the marker file is NOT deleted', () => { + const markerPath = path.join(tmpDir, '.planning', 'active-workstream'); + fs.writeFileSync(markerPath, 'ghost-workstream\n'); + + const result = runGsdTools(['workstream', 'get'], tmpDir, { GSD_SESSION_KEY: 'stale-marker-session' }); + + assert.ok(result.success, `get failed: ${result.error}`); + assert.strictEqual(JSON.parse(result.output).active, null); + assert.ok(fs.existsSync(markerPath), 'a pointer-less session read must never delete the shared marker'); + assert.strictEqual(fs.readFileSync(markerPath, 'utf-8'), 'ghost-workstream\n'); + }); + + test('session pointer file exists but is whitespace-only -> treated as absent, inherits the marker', () => { + const markerPath = path.join(tmpDir, '.planning', 'active-workstream'); + fs.writeFileSync(markerPath, 'beta\n'); + + runGsdTools(['workstream', 'set', 'alpha', '--raw'], tmpDir, { GSD_SESSION_KEY: 'whitespace-pointer-session' }); + const sessionDir = getSessionPointerDir(tmpDir); + const sessionFile = getSessionPointerFileName('GSD_SESSION_KEY', 'whitespace-pointer-session'); + fs.writeFileSync(path.join(sessionDir, sessionFile), ' \n'); + + const result = runGsdTools(['workstream', 'get', '--raw'], tmpDir, { GSD_SESSION_KEY: 'whitespace-pointer-session' }); + + assert.ok(result.success, `get failed: ${result.error}`); + assert.strictEqual(result.output, 'beta'); + }); + + test('shared marker is whitespace-only, session pointer-less -> resolves null', () => { + const markerPath = path.join(tmpDir, '.planning', 'active-workstream'); + fs.writeFileSync(markerPath, ' \n'); + + const result = runGsdTools(['workstream', 'get'], tmpDir, { GSD_SESSION_KEY: 'whitespace-marker-session' }); + + assert.ok(result.success, `get failed: ${result.error}`); + assert.strictEqual(JSON.parse(result.output).active, null); + }); + + test('shared marker file is empty, session pointer-less -> resolves null', () => { + const markerPath = path.join(tmpDir, '.planning', 'active-workstream'); + fs.writeFileSync(markerPath, ''); + + const result = runGsdTools(['workstream', 'get'], tmpDir, { GSD_SESSION_KEY: 'empty-marker-session' }); + + assert.ok(result.success, `get failed: ${result.error}`); + assert.strictEqual(JSON.parse(result.output).active, null); + }); + + test('session pointer is stale (its workstream is deleted) while marker names a different, still-valid workstream -> resolves null, never falls through to inherit the marker', () => { + const markerPath = path.join(tmpDir, '.planning', 'active-workstream'); + fs.writeFileSync(markerPath, 'beta\n'); + runGsdTools(['workstream', 'set', 'alpha', '--raw'], tmpDir, { GSD_SESSION_KEY: 'stale-own-pointer-session' }); + // eslint-disable-next-line local/no-raw-rmsync-in-tests -- mid-test fault injection: simulates a deleted workstream to exercise stale-pointer self-cleanup + fs.rmSync(path.join(tmpDir, '.planning', 'workstreams', 'alpha'), { recursive: true, force: true }); + + const result = runGsdTools(['workstream', 'get'], tmpDir, { GSD_SESSION_KEY: 'stale-own-pointer-session' }); + + assert.ok(result.success, `get failed: ${result.error}`); + assert.strictEqual( + JSON.parse(result.output).active, + null, + 'a stale OWNED pointer must self-heal to null — it must not fall through to the shared marker just because it failed to resolve', + ); + assert.strictEqual(fs.readFileSync(markerPath, 'utf-8'), 'beta\n', 'the shared marker must remain untouched'); + }); + + // #3579 item 1: the #1912/#2028 workstream-mode fail-safe guards must + // distinguish "no marker/pointer anywhere" from "a marker exists but did + // not resolve" — asserted structurally (via --json-errors) per CONTRIBUTING, + // not by regexing the human-readable message. + test('init.progress fail-safe guard: no marker at all -> reason workstream_mode_none_active', () => { + const result = runGsdTools(['--json-errors', 'init', 'progress'], tmpDir); + + assert.equal(result.success, false, 'must still refuse'); + const payload = JSON.parse(result.error); + assert.equal(payload.reason, 'workstream_mode_none_active'); + }); + + test('init.progress fail-safe guard: marker present but names a missing workstream dir -> reason workstream_mode_marker_unresolved, distinct message', () => { + const noneResult = runGsdTools(['--json-errors', 'init', 'progress'], tmpDir); + const nonePayload = JSON.parse(noneResult.error); + + fs.writeFileSync(path.join(tmpDir, '.planning', 'active-workstream'), 'ghost-workstream\n'); + const unresolvedResult = runGsdTools(['--json-errors', 'init', 'progress'], tmpDir); + + assert.equal(unresolvedResult.success, false, 'must still refuse'); + const unresolvedPayload = JSON.parse(unresolvedResult.error); + assert.equal(unresolvedPayload.reason, 'workstream_mode_marker_unresolved'); + assert.equal(unresolvedPayload.marker_value, 'ghost-workstream'); + assert.equal(unresolvedPayload.marker_reason, 'missing_workstream_dir'); + assert.notEqual( + unresolvedPayload.message, + nonePayload.message, + 'the marker-present-but-unresolved diagnostic must differ from the no-marker-at-all diagnostic', + ); + }); + + test('init.progress fail-safe guard: marker present but names an invalid workstream name -> reason workstream_mode_marker_unresolved, invalid_name', () => { + fs.writeFileSync(path.join(tmpDir, '.planning', 'active-workstream'), 'bad/name\n'); + + const result = runGsdTools(['--json-errors', 'init', 'progress'], tmpDir); + + assert.equal(result.success, false, 'must still refuse'); + const payload = JSON.parse(result.error); + assert.equal(payload.reason, 'workstream_mode_marker_unresolved'); + assert.equal(payload.marker_value, 'bad/name'); + assert.equal(payload.marker_reason, 'invalid_name'); + }); +}); + describe('pointer lifecycle hardening', () => { let tmpDir;