From 6b34557ba3c913d064444dfc31761fa667d964e5 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 14 Aug 2026 03:04:09 -0400 Subject: [PATCH] fix(#3311): milestone lock makes parallel-phase state conflicts visible (#3455) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#3311): milestone lock makes parallel-phase state conflicts visible Two sessions running different phases in one working tree silently clobbered STATE.md's single un-scoped ## Current Position slot: byte-level serialization already existed (STATE.md.lock since #464), but nothing ever surfaced that two sessions claimed two different phases, and state.advance-plan — which takes no phase argument — kept advancing whatever plan the just-clobbered position named. Adds the maintainer-chosen milestone lock (issue #3311 comment): an advisory .planning/milestone.lock claim keyed by phase + session id (session identity via getWorkstreamSessionKey: env-first, then controlling TTY). begin-phase claims (inside the STATE.md lock), advance-plan detects a claim/position mismatch and heartbeats a matching claim, phase.complete warns via warnings[] and releases the claim when the claimed phase completes. Conflicts warn (stderr + typed milestone_conflict JSON field) instead of blocking, per the decision's blocking/warning latitude; TTL 4h with heartbeat liveness expires abandoned claims. milestone.lock is registered in the canonical artifact registry so validate.health W019 recognizes it. * chore(#3311): point changeset fragment at pr 3455 --------- Co-authored-by: sim --- .changeset/gentle-moles-sprint.md | 5 + .gitignore | 2 + docs/INVENTORY-MANIFEST.json | 1 + docs/INVENTORY.md | 1 + eslint.config.mjs | 2 + src/artifacts.cts | 1 + src/milestone-lock.cts | 264 ++++++++++++ src/phase.cts | 29 ++ src/state.cts | 43 +- tests/milestone-lock.test.cjs | 655 ++++++++++++++++++++++++++++++ 10 files changed, 1000 insertions(+), 3 deletions(-) create mode 100644 .changeset/gentle-moles-sprint.md create mode 100644 src/milestone-lock.cts create mode 100644 tests/milestone-lock.test.cjs diff --git a/.changeset/gentle-moles-sprint.md b/.changeset/gentle-moles-sprint.md new file mode 100644 index 000000000..c0d5bc917 --- /dev/null +++ b/.changeset/gentle-moles-sprint.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3455 +--- +Parallel phases running in the same working tree no longer corrupt STATE.md silently: state.begin-phase, state.advance-plan and phase.complete now consult a milestone claim (.planning/milestone.lock) keyed by phase + session id, and surface a visible milestone_conflict warning (stderr plus a typed JSON field, and phase.complete's warnings[]) when another live session holds a different phase — instead of silently overwriting the single Current Position slot. diff --git a/.gitignore b/.gitignore index ec1ee11a0..092f554bf 100644 --- a/.gitignore +++ b/.gitignore @@ -100,6 +100,8 @@ build/ /gsd-core/bin/lib/capability-consent.cjs /gsd-core/bin/lib/capability-lock.cjs /gsd-core/bin/lib/markdown-sectionizer.cjs +# #3311: new module (milestone lock) — emitted artifact, never edited. +/gsd-core/bin/lib/milestone-lock.cjs # #2657: these two (markdown-table.cjs, write-set.cjs) already had the # pattern below since #2248, but stayed tracked because `git rm --cached` # was never run — same untracking fix as the seven lines that follow. diff --git a/docs/INVENTORY-MANIFEST.json b/docs/INVENTORY-MANIFEST.json index 9dba0851f..6cc973dc8 100644 --- a/docs/INVENTORY-MANIFEST.json +++ b/docs/INVENTORY-MANIFEST.json @@ -426,6 +426,7 @@ "markdown-table.cjs", "mcp-catalog.cjs", "mcp-server.cjs", + "milestone-lock.cjs", "milestone.cjs", "model-adapter.cjs", "model-catalog.cjs", diff --git a/docs/INVENTORY.md b/docs/INVENTORY.md index a2a8bf0e2..81bc3e2bc 100644 --- a/docs/INVENTORY.md +++ b/docs/INVENTORY.md @@ -595,6 +595,7 @@ Full listing: `gsd-core/bin/lib/*.cjs`. | `health-diagnostic-rules/state-consistency.cjs` | Health-diagnostic rules: STATE.md consistency checks (W002, W011, W021, W026) against config/ROADMAP/disk, ported behavior-preserving from `cmdValidateHealth`; W024 (state_head freshness) is a documented gap, deliberately not migrated (ADR-3180 §8.2/§8.3/§8.5, Phase 11, #3309) | | `state.cjs` | STATE.md parsing, updating, progression, metrics | | `state-document.cjs` | Pure STATE.md field extraction, replacement, status normalization, and progress calculation transforms | +| `milestone-lock.cjs` | Milestone lock (compiled from `src/milestone-lock.cts`, gitignored) — advisory (phase, session id) claim over STATE.md's single Current Position slot: `.planning/milestone.lock` claim IO, liveness (TTL + heartbeat), conflict detection, and the shared stderr warning; consumed by `state.begin-phase` / `state.advance-plan` / `phase.complete` so parallel phases in one working tree get a visible conflict instead of silently overwriting each other (#3311) | | `surface.cjs` | Runtime surface module — manages the runtime enable/disable surface state independently of the install-time profile marker (ADR-0011 Phase 2) | | `task-command-router.cjs` | Thin CJS subcommand router adapter for `gsd-tools task` | | `template.cjs` | Template selection and filling with variable substitution | diff --git a/eslint.config.mjs b/eslint.config.mjs index a6deeb823..bbf3fdc62 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -140,6 +140,8 @@ export default tseslint.config( 'gsd-core/bin/lib/pattern.cjs', 'gsd-core/bin/lib/text-lines.cjs', 'gsd-core/bin/lib/token-scanner.cjs', + // #3311: tsc-generated runtime artifact — lint src/milestone-lock.cts, not this. + 'gsd-core/bin/lib/milestone-lock.cjs', 'gsd-core/bin/lib/health-diagnostic-types.cjs', 'gsd-core/bin/lib/health-diagnostic.cjs', 'gsd-core/bin/lib/health-diagnostic-rules/root-existence.cjs', diff --git a/src/artifacts.cts b/src/artifacts.cts index 39b762faf..d594edc91 100644 --- a/src/artifacts.cts +++ b/src/artifacts.cts @@ -26,6 +26,7 @@ export const CANONICAL_EXACT: ReadonlySet = new Set([ 'RETROSPECTIVE.md', 'WINDOWS.md', // #3224: broken-windows ledger (src/broken-windows.cts, LEDGER_FILE_NAME) 'STATE-ARCHIVE.md', // state.cts's cmdStatePrune writes this at the .planning/ root + 'milestone.lock', // #3311: milestone (phase + session) claim (src/milestone-lock.cts); persistent, unlike the transient STATE.md.lock/WAITING.json ]); // Pattern-match canonical file names (regex tests on the basename) diff --git a/src/milestone-lock.cts b/src/milestone-lock.cts new file mode 100644 index 000000000..4b223a74e --- /dev/null +++ b/src/milestone-lock.cts @@ -0,0 +1,264 @@ +/** + * Milestone Lock — advisory (phase, session) claim over the single Current + * Position slot in STATE.md (#3311). + * + * Problem it solves: two sessions running DIFFERENT phases in the same working + * tree both read-modify-write `## Current Position`. The byte-level STATE.md + * lock (#464) already serializes those writes, so no write is lost — but the + * SEMANTIC clobber is silent: `state.advance-plan` takes no phase argument and + * advances whatever plan the (possibly just clobbered) Current Position names, + * and nothing ever surfaces that two sessions claimed two different phases + * against the one single-slot field. + * + * This module is the maintainer-chosen fix (issue #3311 comment): a milestone + * lock keyed by phase + session id, with a second session WARNED instead of + * silently overwriting. It is an advisory claim file, not a mutex: + * + * - `.planning/milestone.lock` (workstream-scoped via planningDir) holds + * `{ phase, session, pid, updated_at }`. + * - Session identity reuses `getWorkstreamSessionKey()` + * (active-workstream-store.cjs): env-first (GSD_SESSION_KEY, + * CLAUDE_SESSION_ID, CODEX_THREAD_ID, …), then the controlling TTY. Two + * agent sessions in different terminals/runtimes resolve different keys; + * headless runs resolve to null. + * - Liveness is age-based only: a claim is live while + * `now - updated_at < MILESTONE_LOCK_TTL_MS`. There is deliberately NO + * pid-liveness gate — CLI invocations are short-lived, so the recorded pid + * is dead by the next invocation regardless of whether the agent SESSION is + * still active; pid-death would make every claim instantly stale. Active + * sessions heartbeat their claim (advance-plan on a matching position + * refreshes updated_at); an abandoned claim self-expires after the TTL. + * - Conflict rule: a live claim for a DIFFERENT phase held by a DIFFERENT + * session. Same phase never conflicts (the lock is keyed by phase — two + * sessions on one phase are doing the same work). A non-null session key + * equal to the caller's never conflicts (one orchestrating session may + * re-target its own claim). A null session key never counts as "same" as + * anything, so headless parallel phases are still detected. The one + * exception is advance-plan (checkMilestonePosition), which reports ANY + * live claim/position phase mismatch regardless of session — a session's + * own legitimate re-targeting goes through begin-phase, which keeps the + * claim in sync, so a same-session mismatch is just as anomalous. + * - On conflict the existing claim is LEFT INTACT (not stolen): as long as two + * phases are concurrently active, every Current Position mutation keeps + * reporting the conflict. The conflicting command still proceeds (warn, not + * block — a stale-but-live claim must not brick later sessions). + */ + +import fs from 'node:fs'; +import path from 'node:path'; +import { realClock } from './clock.cjs'; +// eslint-disable-next-line @typescript-eslint/no-require-imports -- planning-workspace.cjs is an export= CommonJS module +import planningWorkspace = require('./planning-workspace.cjs'); +// eslint-disable-next-line @typescript-eslint/no-require-imports -- active-workstream-store.cjs is an export= CommonJS module +import activeWorkstreamStore = require('./active-workstream-store.cjs'); + +export const MILESTONE_LOCK_FILENAME = 'milestone.lock'; + +/** + * How long a claim stays live without a heartbeat. Active sessions heartbeat + * on every advance-plan against their claimed phase; a phase's planning stage + * (begin-phase → first completed plan) can legitimately run for hours, so the + * floor must comfortably exceed the longest expected gap between heartbeats + * while still expiring an abandoned claim within a working day. + */ +export const MILESTONE_LOCK_TTL_MS = 4 * 60 * 60 * 1000; + +interface MilestoneClaim { + phase: string; + session: string | null; + pid: number; + updated_at: number; +} + +export interface MilestoneConflict { + /** Phase the live claim holds. */ + locked_phase: string; + /** Session key of the live claim's holder (null = headless/unknown). */ + locked_session: string | null; + /** Phase the calling command targets / the Current Position names. */ + phase: string; + /** Calling session's key (null = headless/unknown). */ + session: string | null; +} + +function milestoneLockPath(cwd: string): string { + return path.join(planningWorkspace.planningDir(cwd), MILESTONE_LOCK_FILENAME); +} + +/** + * Normalize a phase token for claim comparison: trim, and collapse an integer + * spelling to its canonical form so "01", "1" and " 1 " compare equal while + * decimals ("2.5") compare by their trimmed text. + */ +function normalizePhaseToken(value: string): string { + const trimmed = String(value).trim(); + if (/^\d+$/.test(trimmed)) return String(parseInt(trimmed, 10)); + return trimmed; +} + +function sameSession(a: string | null, b: string | null): boolean { + // A null key is "unknown", not "equal" — two headless sessions must still + // conflict across phases (the CI/cron shape of #3311). + if (a === null || b === null) return false; + return a === b; +} + +function parseClaim(raw: string | null | undefined): MilestoneClaim | null { + if (!raw) return null; + try { + const parsed = JSON.parse(raw) as Partial; + if (typeof parsed.phase !== 'string' || !parsed.phase.trim()) return null; + return { + phase: parsed.phase, + session: typeof parsed.session === 'string' ? parsed.session : null, + pid: typeof parsed.pid === 'number' ? parsed.pid : 0, + updated_at: typeof parsed.updated_at === 'number' ? parsed.updated_at : 0, + }; + } catch { + // Corrupt body (concurrent partial write, hand edit) — treat as no claim. + // The next writer replaces the file; never crash a state command over an + // advisory sidecar. + return null; + } +} + +function readClaim(cwd: string): MilestoneClaim | null { + try { + return parseClaim(fs.readFileSync(milestoneLockPath(cwd), 'utf-8')); + } catch { + return null; // absent or unreadable — same posture as parseClaim + } +} + +function writeClaim(cwd: string, claim: MilestoneClaim): void { + ensureClaimDir(cwd); + fs.writeFileSync(milestoneLockPath(cwd), JSON.stringify(claim, null, 2) + '\n'); +} + +// planningWorkspace.planningDir targets .planning (or the workstream-scoped +// variant); the parent is created by every GSD command that gets this far, but +// the claim write must not throw if it is somehow missing. +function ensureClaimDir(cwd: string): void { + try { + fs.mkdirSync(path.dirname(milestoneLockPath(cwd)), { recursive: true }); + } catch { + /* best-effort — see writeClaim callers' advisory posture */ + } +} + +function isClaimLive(claim: MilestoneClaim, nowMs: number): boolean { + return nowMs - claim.updated_at < MILESTONE_LOCK_TTL_MS; +} + +function currentSessionKey(): string | null { + return activeWorkstreamStore.getWorkstreamSessionKey(); +} + +function toConflict(claim: MilestoneClaim, phase: string, session: string | null): MilestoneConflict { + return { + locked_phase: claim.phase, + locked_session: claim.session, + phase, + session, + }; +} + +/** + * Human-visible warning for a conflict. Emitted on stderr unconditionally — + * visibility is the entire point of this lock (#3311's "silent last-write-wins"). + */ +export function warnMilestoneConflict(conflict: MilestoneConflict, action: string): void { + const holder = conflict.locked_session ?? 'an unknown (headless) session'; + const actor = conflict.session ?? 'an unknown (headless) session'; + process.stderr.write( + `[gsd-tools] WARNING: milestone lock conflict (#3311): ${holder} holds the milestone claim for phase ` + + `${conflict.locked_phase}, but ${actor} is running ${action} for phase ${conflict.phase}. ` + + `STATE.md's ## Current Position is a single slot — concurrent phases overwrite each other's ` + + `position. Verify Current Position before trusting it.\n`, + ); +} + +/** + * begin-phase entry point: claim `phase` for this session. + * + * Returns a conflict descriptor (and leaves the existing claim intact) when a + * live claim for a DIFFERENT phase is held by a DIFFERENT session; otherwise + * takes/refreshes the claim and returns null. + */ +export function claimMilestonePhase(cwd: string, phase: string, nowMs?: number): MilestoneConflict | null { + const now = nowMs ?? realClock.now(); + const session = currentSessionKey(); + const existing = readClaim(cwd); + if ( + existing !== null && + isClaimLive(existing, now) && + normalizePhaseToken(existing.phase) !== normalizePhaseToken(phase) && + !sameSession(existing.session, session) + ) { + return toConflict(existing, phase, session); + } + writeClaim(cwd, { phase: String(phase).trim(), session, pid: process.pid, updated_at: now }); + return null; +} + +/** + * advance-plan entry point: `positionPhase` is the phase STATE.md's Current + * Position currently names. + * + * A live claim naming a DIFFERENT phase means the position was moved away from + * the claimed phase without a matching begin-phase — the #3311 flip — and is + * returned as a conflict REGARDLESS of session: this session's own legitimate + * re-targeting goes through begin-phase, which keeps the claim in sync, so a + * same-session mismatch is exactly as anomalous as a cross-session one. A live + * claim matching the position phase is heartbeat-refreshed (this session is + * actively working that phase, so the claim stays live). No claim is ever + * created here: advance-plan has no phase argument of its own, so it can only + * corroborate or contradict an existing claim, not originate one. + */ +export function checkMilestonePosition(cwd: string, positionPhase: string, nowMs?: number): MilestoneConflict | null { + const now = nowMs ?? realClock.now(); + const existing = readClaim(cwd); + if (existing === null || !isClaimLive(existing, now)) return null; + const session = currentSessionKey(); + if (normalizePhaseToken(existing.phase) === normalizePhaseToken(positionPhase)) { + // Heartbeat — keep the claim's liveness anchored to actual activity. + writeClaim(cwd, { phase: existing.phase, session: existing.session, pid: process.pid, updated_at: now }); + return null; + } + return toConflict(existing, positionPhase, session); +} + +/** + * phase.complete entry point (read-only check): a live claim for a DIFFERENT + * phase held by a DIFFERENT session is a conflict. phase.complete never claims + * — it ends work on a phase rather than starting it. + */ +export function checkMilestoneConflictForPhase(cwd: string, phase: string, nowMs?: number): MilestoneConflict | null { + const now = nowMs ?? realClock.now(); + const existing = readClaim(cwd); + if (existing === null || !isClaimLive(existing, now)) return null; + if (normalizePhaseToken(existing.phase) === normalizePhaseToken(phase)) return null; + const session = currentSessionKey(); + if (!sameSession(existing.session, session)) { + return toConflict(existing, phase, session); + } + return null; +} + +/** + * Release the claim when the phase it names completes — regardless of which + * session completes it (an orchestrator cleaning up after a dead session must + * not be blocked by the dead session's own claim). Returns whether a matching + * claim was removed. + */ +export function releaseMilestonePhase(cwd: string, phase: string): boolean { + const existing = readClaim(cwd); + if (existing === null) return false; + if (normalizePhaseToken(existing.phase) !== normalizePhaseToken(phase)) return false; + try { + fs.unlinkSync(milestoneLockPath(cwd)); + return true; + } catch { + return false; // already gone — same outcome for the caller + } +} \ No newline at end of file diff --git a/src/phase.cts b/src/phase.cts index 0d9d82886..f83e617b4 100644 --- a/src/phase.cts +++ b/src/phase.cts @@ -83,6 +83,8 @@ const { computeHaltPropagation, buildSummaryFileIndex, isSummaryFileHalted } = p const { planningDir, withPlanningLock, listAvailableWorkstreams, getActiveWorkstream } = 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; const { readModifyWriteStateMd, @@ -2248,7 +2250,28 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void { let nextPhaseName: string | null = null; let isLastPhase = true; + // #3311: typed conflict descriptor surfaced on the result JSON alongside the + // warnings[] entry below (same parity pattern as + // verification_stale_check_indeterminate). + let milestoneConflict: milestoneLockMod.MilestoneConflict | null = null; + const verificationBlocked = withPlanningLock(cwd, () => { + // #3311: completing a phase while a live milestone claim (phase + session) + // holds a DIFFERENT phase means two sessions are working two phases against + // the single Current Position slot. Warn via the established warnings[] + // channel (rendered by execute-phase.md's "If has_warnings is true" step) + // rather than blocking — the claim may simply be stale-but-live. + milestoneConflict = milestoneLockMod.checkMilestoneConflictForPhase(cwd, phaseNum); + if (milestoneConflict) { + const holder = milestoneConflict.locked_session ?? 'an unknown (headless) session'; + const actor = milestoneConflict.session ?? 'an unknown (headless) session'; + warnings.push( + `milestone lock conflict (#3311): ${holder} holds the milestone claim for phase ` + + `${milestoneConflict.locked_phase}, but ${actor} is completing phase ${phaseNum} — ` + + `STATE.md's Current Position is a single slot; verify it before trusting it`, + ); + milestoneLockMod.warnMilestoneConflict(milestoneConflict, `phase.complete ${phaseNum}`); + } // #2617: pass the project's runtime so the blocked-completion error below // suggests the command surface this runtime actually installs // ($gsd-… on Codex) rather than a hard-coded Claude-style string. @@ -2969,6 +2992,11 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void { } else { runPhaseCompleteTransaction(); } + // #3311: a successful completion of the CLAIMED phase releases the + // milestone claim — regardless of which session completes it (an + // orchestrator cleaning up after a dead session must not be blocked by the + // dead session's own claim). No-ops when the claim names another phase. + milestoneLockMod.releaseMilestonePhase(cwd, phaseNum); return null; }); @@ -3027,6 +3055,7 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void { warnings, has_warnings: warnings.length > 0, verification_stale_check_indeterminate: staleCheckIndeterminate, + milestone_conflict: milestoneConflict, }; output(result, raw); diff --git a/src/state.cts b/src/state.cts index 6bab9a712..8eac7f00c 100644 --- a/src/state.cts +++ b/src/state.cts @@ -47,6 +47,11 @@ import stateTransitionMod = require('./state-transition.cjs'); // #2573 D5: used to pin `git rev-parse` to the project's own repo. Imports only // node builtins, so it introduces no cycle on this path. import { findProjectRoot } from './project-root.cjs'; +// #3311: advisory (phase, session) claim over the single Current Position slot. +// Imports only node builtins + planning-workspace + active-workstream-store, so +// it introduces no cycle on this path. +// eslint-disable-next-line @typescript-eslint/no-require-imports +import milestoneLockMod = require('./milestone-lock.cjs'); const { transitionCore, applyStatePreservation, sliceCurrentPositionSection } = stateTransitionMod; type StateTransitionIntent = stateTransitionMod.StateTransitionIntent; type StateTransitionDeps = stateTransitionMod.StateTransitionDeps; @@ -592,7 +597,25 @@ function cmdStateAdvancePlan(cwd: string, raw: boolean): void { }; let resultData: Record | undefined; + // #3311: the milestone (phase + session) claim is consulted INSIDE the + // STATE.md lock, so the position read and the claim read cannot interleave + // with another session's Current Position write. + let milestoneConflict: milestoneLockMod.MilestoneConflict | null = null; readModifyWriteStateMd(statePath, (content) => { + // advance-plan has no phase argument of its own — the phase it advances is + // whatever ## Current Position names. Compare that against the milestone + // claim: a mismatch means another session moved the single-slot position + // away from the claimed phase (the #3311 flip) and must be surfaced, not + // silently absorbed. + const body = stripFrontmatter(content); + const positionScope = matchCurrentPositionSection(body) ?? body; + const positionPhase = parseProsePhaseField(stateExtractField(positionScope, 'Phase')).phase; + if (positionPhase !== null) { + milestoneConflict = milestoneLockMod.checkMilestonePosition(cwd, positionPhase); + if (milestoneConflict) { + milestoneLockMod.warnMilestoneConflict(milestoneConflict, 'state.advance-plan'); + } + } const result = transitionCore(content, intent, deps); resultData = result.data; return result.content; @@ -604,9 +627,9 @@ function cmdStateAdvancePlan(cwd: string, raw: boolean): void { } if (resultData['advanced'] === false) { - output(resultData, raw, 'false'); + output({ ...resultData, milestone_conflict: milestoneConflict }, raw, 'false'); } else { - output(resultData, raw, 'true'); + output({ ...resultData, milestone_conflict: milestoneConflict }, raw, 'true'); } } @@ -2891,7 +2914,17 @@ function cmdStateBeginPhase(cwd: string, phaseNumber: string | number, phaseName authoritativeFm: intent.phaseName ? { current_phase_name: intent.phaseName } : undefined, }; let updated: string[] = []; + // #3311: begin-phase is the claim point — it is the one Current Position + // transition that explicitly names its phase, so it both records this + // session's claim and detects a conflicting live claim for a different + // phase. The check runs INSIDE the STATE.md lock so concurrent begin-phase + // calls cannot both read "no claim" and both write. + let milestoneConflict: milestoneLockMod.MilestoneConflict | null = null; readModifyWriteStateMd(statePath, (content) => { + milestoneConflict = milestoneLockMod.claimMilestonePhase(cwd, String(phaseNumber)); + if (milestoneConflict) { + milestoneLockMod.warnMilestoneConflict(milestoneConflict, `state.begin-phase ${phaseNumber}`); + } const result = transitionCore(content, intent, deps); updated = result.updated; // #3127 resume: the core preserved the mid-flight Current Phase Name, so @@ -2904,7 +2937,11 @@ function cmdStateBeginPhase(cwd: string, phaseNumber: string | number, phaseName return result.content; }, cwd, rmwOptions); - output({ updated, phase: phaseNumber, phase_name: phaseName || null, plan_count: planCount || null }, raw, updated.length > 0 ? 'true' : 'false'); + output( + { updated, phase: phaseNumber, phase_name: phaseName || null, plan_count: planCount || null, milestone_conflict: milestoneConflict }, + raw, + updated.length > 0 ? 'true' : 'false', + ); } /** diff --git a/tests/milestone-lock.test.cjs b/tests/milestone-lock.test.cjs new file mode 100644 index 000000000..1ec3ccbe4 --- /dev/null +++ b/tests/milestone-lock.test.cjs @@ -0,0 +1,655 @@ +// allow-test-rule: source-text-is-the-product (see #3311) +// Reads .planning/STATE.md (and the milestone.lock claim JSON) because the +// deployed text of those files IS the contract under test: #3311 is exactly +// about two sessions silently overwriting each other's Current Position text. + +'use strict'; + +// #3311 — milestone lock keyed by phase + session id. +// +// The defect: two sessions running different phases in ONE working tree both +// read-modify-write the single un-scoped `## Current Position` slot in +// STATE.md. Byte-level serialization already exists (STATE.md.lock, #464), so +// no write is lost — but the SEMANTIC clobber is silent: `state.advance-plan` +// takes no phase argument and advances whatever plan the (possibly just +// clobbered) Current Position names, and nothing ever surfaces that two +// sessions claimed two different phases. +// +// The fix (maintainer decision on #3311): an advisory claim file +// `.planning/milestone.lock` holding { phase, session, pid, updated_at }. +// A second session working a DIFFERENT phase gets a visible warning +// (stderr + typed `milestone_conflict` JSON field / phase.complete's +// warnings[]) instead of a silent overwrite. +// +// Session identity comes from getWorkstreamSessionKey() (env-first). Tests +// drive it with GSD_SESSION_KEY via runGsdTools's per-call env override; +// helpers blank ambient session identity, so default runs are headless +// (null session) deterministically. + +const { test, describe, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); +const helpers = require('./helpers.cjs'); +const { runGsdTools, createTempProject, cleanup, TOOLS_PATH } = helpers; +const processSeam = require('./helpers/process-seam.cjs'); + +// runGsdTools's legacy shape drops stderr on success, but the #3311 contract +// is exactly that a conflict is VISIBLE — these tests must see stderr. Drive +// the process seam directly for the conflicting invocations (same env base +// runGsdTools applies, exposed as the TEST_ENV_BASE getter). +function runToolsWithStderr(args, cwd, env = {}) { + return processSeam.runNode([TOOLS_PATH, ...args], { + cwd, + env: { ...process.env, ...helpers.TEST_ENV_BASE, ...env }, + timeoutMs: 60000, + }); +} + +const SESSION_A = { GSD_SESSION_KEY: 'session-a' }; +const SESSION_B = { GSD_SESSION_KEY: 'session-b' }; + +// Pinned clock base for deterministic claim freshness (realClock.now() honors +// GSD_TEST_MODE + GSD_NOW_MS in the subprocess under test). +const T0 = Date.parse('2026-08-10T09:00:00.000Z'); +// A delta comfortably past any plausible claim TTL (4h) — staleness driver. +const WEEK_LATER = T0 + 7 * 24 * 60 * 60 * 1000; + +function milestoneLockPath(tmpDir) { + return path.join(tmpDir, '.planning', 'milestone.lock'); +} + +function readClaim(tmpDir) { + return JSON.parse(fs.readFileSync(milestoneLockPath(tmpDir), 'utf-8')); +} + +const BEGIN_PHASE_STATE_MD = [ + '# Project State', + '', + '**Current Phase:** 1', + '**Current Phase Name:** setup', + '**Total Phases:** 5', + '**Current Plan:** 0', + '**Total Plans in Phase:** 0', + '**Status:** Ready to plan', + '**Last Activity:** 2026-03-20', + '', + '## Current Position', + 'Phase: 1 of 5 (setup)', + 'Plan: 0 of ? in current phase', + 'Status: Ready to plan', + 'Last activity: 2026-03-20 — roadmap created', + 'Progress: [..........] 0%', + '', +].join('\n'); + +// STATE.md whose Current Position names phase 2 — the post-clobber state a +// session-A advance-plan must detect against its phase-1 claim. +const ADVANCE_POSITION_PHASE_2 = [ + '# Project State', + '', + '**Current Plan:** 1', + '**Total Plans in Phase:** 3', + '**Status:** Executing', + '**Last Activity:** 2026-08-10', + '', + '## Current Position', + 'Phase: 2 of 5 (api)', + 'Plan: 1 of 3 in current phase', + 'Status: Executing', + 'Last activity: 2026-08-10 — began api work', + '', +].join('\n'); + +/** + * Minimal project where `phase complete ` reaches its transaction: + * ROADMAP with two phases, phase dir with one plan + summary + passed + * verification for phase 1, and an empty (already-verified) phase 2. + */ +function writePhaseCompleteFixture(tmpDir) { + const planningDir = path.join(tmpDir, '.planning'); + const phase1Dir = path.join(planningDir, 'phases', '01-foundation'); + const phase2Dir = path.join(planningDir, 'phases', '02-api'); + fs.mkdirSync(phase1Dir, { recursive: true }); + fs.mkdirSync(phase2Dir, { recursive: true }); + + fs.writeFileSync( + path.join(planningDir, 'ROADMAP.md'), + [ + '# Roadmap', + '', + '- [ ] Phase 1: Foundation', + '- [ ] Phase 2: API', + '', + '### Phase 1: Foundation', + '**Goal:** Setup', + '**Plans:** 1 plans', + '', + '### Phase 2: API', + '**Goal:** Build API', + '**Plans:** 0 plans', + '', + '## Progress', + '', + '| Phase | Plans Complete | Status | Completed |', + '|-------|----------------|--------|-----------|', + '| 01. Foundation | 0/1 | Not started | - |', + '| 02. API | 0/1 | Not started | - |', + '', + ].join('\n'), + ); + + fs.writeFileSync( + path.join(planningDir, 'STATE.md'), + [ + '# State', + '', + '**Current Phase:** 01', + '**Current Phase Name:** Foundation', + '**Status:** In progress', + '**Current Plan:** 01-01', + '**Last Activity:** 2026-08-10', + '', + '## Current Position', + 'Phase: 1 of 2 (Foundation)', + 'Plan: 1 of 1 in current phase', + 'Status: Executing', + '', + ].join('\n'), + ); + + fs.writeFileSync(path.join(phase1Dir, '01-01-PLAN.md'), '# Plan\n'); + fs.writeFileSync(path.join(phase1Dir, '01-01-SUMMARY.md'), '# Summary\n'); + fs.writeFileSync( + path.join(phase1Dir, '01-VERIFICATION.md'), + ['---', 'status: passed', '---', '', '# Verification', ''].join('\n'), + ); + fs.writeFileSync( + path.join(phase2Dir, '02-VERIFICATION.md'), + ['---', 'status: passed', '---', '', '# Verification', ''].join('\n'), + ); +} + +// ───────────────────────────────────────────────────────────────────────────── +// M1–M4, M7, M8, M11 — begin-phase claim behavior +// ───────────────────────────────────────────────────────────────────────────── + +describe('#3311 milestone lock: begin-phase claims', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject(); + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), BEGIN_PHASE_STATE_MD); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + test('M1: begin-phase records a milestone.lock claim with phase + session key', () => { + const result = runGsdTools( + ['state', 'begin-phase', '--phase', '1', '--name', 'setup', '--plans', '4'], + tmpDir, + { ...SESSION_A, GSD_TEST_MODE: '1', GSD_NOW_MS: String(T0) }, + ); + assert.ok(result.success, `begin-phase failed: ${result.error}`); + + const claim = readClaim(tmpDir); + assert.strictEqual(claim.phase, '1', `claim phase must be 1; got ${JSON.stringify(claim)}`); + assert.ok( + typeof claim.session === 'string' && claim.session.includes('session-a'), + `claim session must carry the session key; got ${JSON.stringify(claim)}`, + ); + assert.strictEqual(claim.updated_at, T0, 'claim updated_at must be the pinned now'); + }); + + test('M2: second session beginning a DIFFERENT phase gets a visible conflict, claim left intact, STATE.md still updated', () => { + const first = runGsdTools( + ['state', 'begin-phase', '--phase', '1', '--name', 'setup', '--plans', '4'], + tmpDir, + { ...SESSION_A, GSD_TEST_MODE: '1', GSD_NOW_MS: String(T0) }, + ); + assert.ok(first.success, `first begin-phase failed: ${first.error}`); + + const second = runToolsWithStderr( + ['state', 'begin-phase', '--phase', '2', '--name', 'api', '--plans', '3'], + tmpDir, + { ...SESSION_B, GSD_TEST_MODE: '1', GSD_NOW_MS: String(T0 + 1000) }, + ); + assert.strictEqual(second.exitCode, 0, `second begin-phase failed: ${second.stderr}`); + + const out = JSON.parse(second.stdout); + assert.ok( + out.milestone_conflict && out.milestone_conflict.locked_phase === '1', + `output must carry milestone_conflict.locked_phase 1; got ${second.stdout}`, + ); + assert.match( + second.stderr || '', + /milestone/i, + 'a visible stderr warning must accompany the conflict', + ); + + // Warn, not block: STATE.md still moves to phase 2 … + const stateContent = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); + assert.match(stateContent, /^Phase:.*2.*EXECUTING/m, 'STATE.md must still be updated (warn, not block)'); + + // … and the first session's claim is NOT overwritten, so the conflict + // stays visible on every subsequent mutation. + const claim = readClaim(tmpDir); + assert.strictEqual(claim.phase, '1', `session A's claim must be left intact; got ${JSON.stringify(claim)}`); + assert.ok(claim.session.includes('session-a'), 'claim session must still be session A'); + }); + + test('M3: same session re-targeting a different phase — no conflict, claim follows', () => { + const first = runGsdTools( + ['state', 'begin-phase', '--phase', '1', '--name', 'setup', '--plans', '4'], + tmpDir, + { ...SESSION_A, GSD_TEST_MODE: '1', GSD_NOW_MS: String(T0) }, + ); + assert.ok(first.success, `first begin-phase failed: ${first.error}`); + assert.ok(!JSON.parse(first.output).milestone_conflict, 'no conflict on fresh claim'); + + const second = runGsdTools( + ['state', 'begin-phase', '--phase', '2', '--name', 'api', '--plans', '3'], + tmpDir, + { ...SESSION_A, GSD_TEST_MODE: '1', GSD_NOW_MS: String(T0 + 1000) }, + ); + assert.ok(second.success, `second begin-phase failed: ${second.error}`); + + const out = JSON.parse(second.output); + assert.ok(!out.milestone_conflict, `same session re-targeting must not conflict; got ${second.output}`); + + const claim = readClaim(tmpDir); + assert.strictEqual(claim.phase, '2', `claim must follow the session's new phase; got ${JSON.stringify(claim)}`); + }); + + test('M4: different session, SAME phase — no conflict (keyed by phase)', () => { + const first = runGsdTools( + ['state', 'begin-phase', '--phase', '1', '--name', 'setup', '--plans', '4'], + tmpDir, + { ...SESSION_A, GSD_TEST_MODE: '1', GSD_NOW_MS: String(T0) }, + ); + assert.ok(first.success, `first begin-phase failed: ${first.error}`); + + const second = runGsdTools( + ['state', 'begin-phase', '--phase', '1', '--name', 'setup', '--plans', '4'], + tmpDir, + { ...SESSION_B, GSD_TEST_MODE: '1', GSD_NOW_MS: String(T0 + 1000) }, + ); + assert.ok(second.success, `second begin-phase failed: ${second.error}`); + + const out = JSON.parse(second.output); + assert.ok(!out.milestone_conflict, `same phase from another session must not conflict; got ${second.output}`); + }); + + test('M7: headless sessions (no session identity) still conflict across phases', () => { + // No session env at all — both invocations resolve a null session key. + // A null key must never count as "same session", or the headless parallel + // case (CI, cron) would be undetectable. + const first = runGsdTools( + ['state', 'begin-phase', '--phase', '1', '--name', 'setup', '--plans', '4'], + tmpDir, + { GSD_TEST_MODE: '1', GSD_NOW_MS: String(T0) }, + ); + assert.ok(first.success, `first begin-phase failed: ${first.error}`); + + const second = runGsdTools( + ['state', 'begin-phase', '--phase', '2', '--name', 'api', '--plans', '3'], + tmpDir, + { GSD_TEST_MODE: '1', GSD_NOW_MS: String(T0 + 1000) }, + ); + assert.ok(second.success, `second begin-phase failed: ${second.error}`); + + const out = JSON.parse(second.output); + assert.ok( + out.milestone_conflict && out.milestone_conflict.locked_phase === '1', + `headless cross-phase conflict must be detected; got ${second.output}`, + ); + }); + + test('M8: a claim older than the TTL is stale — takeover without conflict', () => { + const first = runGsdTools( + ['state', 'begin-phase', '--phase', '1', '--name', 'setup', '--plans', '4'], + tmpDir, + { ...SESSION_A, GSD_TEST_MODE: '1', GSD_NOW_MS: String(T0) }, + ); + assert.ok(first.success, `first begin-phase failed: ${first.error}`); + + // A week later, session B begins phase 2: the abandoned claim must not + // fire a false conflict. + const second = runGsdTools( + ['state', 'begin-phase', '--phase', '2', '--name', 'api', '--plans', '3'], + tmpDir, + { ...SESSION_B, GSD_TEST_MODE: '1', GSD_NOW_MS: String(WEEK_LATER) }, + ); + assert.ok(second.success, `second begin-phase failed: ${second.error}`); + + const out = JSON.parse(second.output); + assert.ok(!out.milestone_conflict, `stale claim must not conflict; got ${second.output}`); + + const claim = readClaim(tmpDir); + assert.strictEqual(claim.phase, '2', `stale claim must be taken over; got ${JSON.stringify(claim)}`); + assert.strictEqual(claim.updated_at, WEEK_LATER, 'takeover must re-stamp updated_at'); + }); + + test('M11: a corrupt milestone.lock body is treated as no claim — no crash, no conflict', () => { + fs.writeFileSync(milestoneLockPath(tmpDir), 'not-json{garbage'); + + const result = runGsdTools( + ['state', 'begin-phase', '--phase', '2', '--name', 'api', '--plans', '3'], + tmpDir, + { ...SESSION_B, GSD_TEST_MODE: '1', GSD_NOW_MS: String(T0) }, + ); + assert.ok(result.success, `begin-phase failed on corrupt claim: ${result.error}`); + + const out = JSON.parse(result.output); + assert.ok(!out.milestone_conflict, `corrupt claim must not conflict; got ${result.output}`); + + const claim = readClaim(tmpDir); + assert.strictEqual(claim.phase, '2', `corrupt claim must be replaced; got ${JSON.stringify(claim)}`); + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// M5, M6 — advance-plan flip detection + heartbeat +// ───────────────────────────────────────────────────────────────────────────── + +describe('#3311 milestone lock: advance-plan position checks', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject(); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + test('M5: position phase matching the claim advances silently and heartbeats the claim', () => { + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), BEGIN_PHASE_STATE_MD); + const begin = runGsdTools( + ['state', 'begin-phase', '--phase', '1', '--name', 'setup', '--plans', '3'], + tmpDir, + { ...SESSION_A, GSD_TEST_MODE: '1', GSD_NOW_MS: String(T0) }, + ); + assert.ok(begin.success, `begin-phase failed: ${begin.error}`); + + const before = readClaim(tmpDir); + assert.strictEqual(before.updated_at, T0); + + const adv = runGsdTools( + ['state', 'advance-plan'], + tmpDir, + { ...SESSION_A, GSD_TEST_MODE: '1', GSD_NOW_MS: String(T0 + 60_000) }, + ); + assert.ok(adv.success, `advance-plan failed: ${adv.error}`); + + const out = JSON.parse(adv.output); + assert.ok(out.advanced === true, `advance-plan must advance; got ${adv.output}`); + assert.ok(!out.milestone_conflict, `matching claim must not conflict; got ${adv.output}`); + + const after = readClaim(tmpDir); + assert.strictEqual(after.phase, '1', 'claim phase unchanged'); + assert.strictEqual( + after.updated_at, + T0 + 60_000, + `claim must be heartbeat-refreshed; got ${JSON.stringify(after)}`, + ); + }); + + test('M6: Current Position naming ANOTHER session\'s claimed phase is reported as a conflict', () => { + // Session A legitimately began phase 1 … + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), BEGIN_PHASE_STATE_MD); + const begin = runGsdTools( + ['state', 'begin-phase', '--phase', '1', '--name', 'setup', '--plans', '3'], + tmpDir, + { ...SESSION_A, GSD_TEST_MODE: '1', GSD_NOW_MS: String(T0) }, + ); + assert.ok(begin.success, `begin-phase failed: ${begin.error}`); + + // … then session B's begin-phase clobbered the single Current Position + // slot to phase 2 (the #3311 flip). Session A's next advance-plan must + // SAY SO instead of silently advancing phase 2's plan counter. + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), ADVANCE_POSITION_PHASE_2); + + const adv = runToolsWithStderr( + ['state', 'advance-plan'], + tmpDir, + { ...SESSION_A, GSD_TEST_MODE: '1', GSD_NOW_MS: String(T0 + 60_000) }, + ); + assert.strictEqual(adv.exitCode, 0, `advance-plan failed: ${adv.stderr}`); + + const out = JSON.parse(adv.stdout); + assert.ok( + out.milestone_conflict && out.milestone_conflict.locked_phase === '1', + `position/claim mismatch must surface milestone_conflict; got ${adv.stdout}`, + ); + assert.match(adv.stderr || '', /milestone/i, 'a visible stderr warning must accompany the conflict'); + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// M9, M10 — phase.complete conflict warning + claim release +// ───────────────────────────────────────────────────────────────────────────── + +describe('#3311 milestone lock: phase.complete', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject(); + writePhaseCompleteFixture(tmpDir); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + test('M9: completing a phase while a live claim holds a DIFFERENT phase warns via warnings[]', () => { + // Session A is mid-phase-1 (live claim) … + fs.writeFileSync(path.join(tmpDir, '.planning', 'milestone.lock'), JSON.stringify({ + phase: '1', + session: 'gsd-session-key-session-a', + pid: 4242, + updated_at: Date.now(), + })); + + // … and session B completes phase 2 in the same tree. + const result = runGsdTools( + ['phase', 'complete', '2'], + tmpDir, + SESSION_B, + ); + assert.ok(result.success, `phase complete failed: ${result.error}`); + + const out = JSON.parse(result.output); + assert.ok( + Array.isArray(out.warnings) && out.warnings.some((w) => /milestone/i.test(w)), + `warnings[] must carry the milestone-lock conflict; got ${result.output}`, + ); + assert.strictEqual(out.has_warnings, true, 'has_warnings must be true'); + assert.ok( + out.milestone_conflict && out.milestone_conflict.locked_phase === '1', + `typed milestone_conflict must be present; got ${result.output}`, + ); + + // The live claim is left intact for the same reason as M2. + const claim = readClaim(tmpDir); + assert.strictEqual(claim.phase, '1', 'session A\'s claim must be left intact'); + }); + + test('M10: completing the CLAIMED phase releases the claim', () => { + fs.writeFileSync(path.join(tmpDir, '.planning', 'milestone.lock'), JSON.stringify({ + phase: '1', + session: 'gsd-session-key-session-a', + pid: 4242, + updated_at: Date.now(), + })); + + const result = runGsdTools(['phase', 'complete', '1'], tmpDir, SESSION_A); + assert.ok(result.success, `phase complete failed: ${result.error}`); + + const out = JSON.parse(result.output); + assert.ok(!out.milestone_conflict, `completing the claimed phase must not conflict; got ${result.output}`); + + assert.ok( + !fs.existsSync(milestoneLockPath(tmpDir)), + 'milestone.lock must be released when the claimed phase completes', + ); + }); + + test('M10b: no claim file — phase.complete carries no conflict and creates no claim', () => { + const result = runGsdTools(['phase', 'complete', '1'], tmpDir, SESSION_A); + assert.ok(result.success, `phase complete failed: ${result.error}`); + + const out = JSON.parse(result.output); + assert.ok(!out.milestone_conflict, `no claim must mean no conflict; got ${result.output}`); + assert.ok( + !Array.isArray(out.warnings) || !out.warnings.some((w) => /milestone/i.test(w)), + 'no milestone warning without a claim', + ); + assert.ok(!fs.existsSync(milestoneLockPath(tmpDir)), 'phase.complete must not create a claim'); + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// G1 — lock-domain serialization guard (triage brief regression test 1) +// ───────────────────────────────────────────────────────────────────────────── + +describe('#3311 guard: phase.complete does not lose a concurrent STATE.md write', () => { + test('G1: a STATE.md edit landing while phase.complete waits on STATE.md.lock survives', () => { + const tmpDir = createTempProject(); + try { + writePhaseCompleteFixture(tmpDir); + + const stateMod = require('../gsd-core/bin/lib/state.cjs'); + const phaseMod = require('../gsd-core/bin/lib/phase.cjs'); + const statePath = path.join(tmpDir, '.planning', 'STATE.md'); + const lockPath = statePath + '.lock'; + + // Pre-hold STATE.md.lock the way a concurrent `state.advance-plan` + // mid-transform would: a body with a pid the liveness probe reports as + // VERIFIED-LIVE, so the phase.complete path must WAIT, not steal. + fs.writeFileSync(lockPath, '4242'); + stateMod._setLockProbes({ isPidAlive: (pid) => pid === 4242 }); + + let injected = false; + stateMod._setStateLockTestHooks({ + onLoopIteration(ctx) { + // Gate on iteration >= 1: the hook fires at the top of EVERY loop + // iteration, so iteration 0 alone proves nothing. Injecting only + // once the first open() has hit EEXIST proves the writer is + // actually HONORING the held lock rather than bypassing it. + if (ctx.iteration < 1 || injected) return; + injected = true; + // The "advance-plan write" lands on disk while phase.complete waits … + fs.writeFileSync( + statePath, + fs.readFileSync(statePath, 'utf-8') + '\n## Concurrent Notes\nsurvived-concurrent-writer\n', + ); + // … then the concurrent holder releases. + fs.unlinkSync(lockPath); + }, + }); + + // Swallow fd-1 writes (io.output writes JSON straight to fd 1). + const origWriteSync = fs.writeSync; + const captured = []; + fs.writeSync = function patchedWriteSync(fd, data, ...rest) { + if (fd === 1) { + captured.push(String(data)); + return String(data).length; + } + return origWriteSync(fd, data, ...rest); + }; + + let threw = null; + try { + phaseMod.cmdPhaseComplete(tmpDir, '1', true); + } catch (e) { + threw = e; + } finally { + fs.writeSync = origWriteSync; + stateMod._resetStateLockTestHooks(); + stateMod._resetLockProbes(); + } + assert.ok(!threw, `phase.complete must succeed under contention: ${threw && threw.message}`); + assert.ok(injected, 'the EEXIST contention path must have been reached (lock honored)'); + + const finalContent = fs.readFileSync(statePath, 'utf-8'); + assert.ok( + finalContent.includes('survived-concurrent-writer'), + `the concurrent writer's STATE.md edit must NOT be lost; got:\n${finalContent}`, + ); + // And the completion itself also landed (both writes present). + const payload = JSON.parse(captured.join('') || '{}'); + assert.strictEqual(payload.completed_phase, '1', `completion result must be phase 1; got ${captured.join('')}`); + } finally { + cleanup(tmpDir); + } + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// G2 — targeted-field-replace guard (triage brief regression test 3) +// ───────────────────────────────────────────────────────────────────────────── + +describe('#3311 guard: advancePlan stays a targeted field replace', () => { + test('G2: only Status / Last Activity / Plan lines change inside ## Current Position', () => { + const { transitionCore } = require('../gsd-core/bin/lib/state-transition.cjs'); + + const fixedClock = Object.freeze({ + today: () => '2026-08-10', + localToday: () => '2026-08-10', + nowIso: () => '2026-08-10T12:00:00.000Z', + }); + + const before = [ + '# Project State', + '', + '**Current Plan:** 1', + '**Total Plans in Phase:** 3', + '**Status:** Executing', + '**Last Activity:** 2026-08-09', + '', + '## Current Position', + 'Phase: 2 of 5 (api)', + 'Plan: 1 of 3 in current phase', + 'Status: Executing', + // Bare date = handler-generated shape, so advancePlan's template-aware + // replace is permitted to update it (an executor-authored value like + // "2026-08-09 — began api work" is deliberately preserved instead). + 'Last activity: 2026-08-09', + 'Owner: alice', + '', + '## Next Steps', + '', + 'Do the thing.', + '', + ].join('\n'); + + const result = transitionCore(before, { kind: 'advancePlan' }, { clock: fixedClock, sourcePath: 'STATE.md' }); + const after = result.content; + + const section = (text) => { + const m = text.match(/## Current Position\s*\r?\n([\s\S]*?)(?=\r?\n##|$)/i); + assert.ok(m, 'Current Position section must exist'); + return m[1]; + }; + const beforeSection = section(before); + assert.ok(beforeSection.length > 0, 'before-section must be non-empty'); + const afterSection = section(after); + + // Untouched lines — the triage's "must not turn into a whole-block rewrite". + assert.match(afterSection, /^Phase: 2 of 5 \(api\)$/m, 'Phase line must be byte-identical'); + assert.match(afterSection, /^Owner: alice$/m, 'unrelated custom line must be byte-identical'); + assert.ok( + after.includes('## Next Steps\n\nDo the thing.'), + 'the sibling ## Next Steps section must be untouched', + ); + + // Mutated lines. + assert.match(afterSection, /^Plan: 2 of 3/m, 'Plan line must advance'); + assert.match(afterSection, /^Status: Ready to execute$/m, 'Status line must update'); + assert.match(afterSection, /^Last activity: 2026-08-10/m, 'Last activity line must update'); + }); +});