diff --git a/.changeset/eager-wolves-tumble.md b/.changeset/eager-wolves-tumble.md
new file mode 100644
index 000000000..ab236d541
--- /dev/null
+++ b/.changeset/eager-wolves-tumble.md
@@ -0,0 +1,5 @@
+---
+type: Fixed
+pr: 3764
+---
+**`gap-analysis` now respects padded-prefix CONTEXT.md** — `gap-checker.cjs:136` silently missed `NN-CONTEXT.md` decisions on phases using the padded-prefix convention; now matches the dual-form pattern used elsewhere.
diff --git a/get-shit-done/bin/lib/core.cjs b/get-shit-done/bin/lib/core.cjs
index 85227309d..305ac8bd6 100644
--- a/get-shit-done/bin/lib/core.cjs
+++ b/get-shit-done/bin/lib/core.cjs
@@ -23,6 +23,7 @@ const {
withPlanningLock,
getActiveWorkstream,
setActiveWorkstream,
+ findContextMdIn,
} = require('./planning-workspace.cjs');
const { findProjectRoot } = require('./project-root.generated.cjs');
@@ -1810,7 +1811,7 @@ function getPhaseFileStats(phaseDir) {
plans: filterPlanFiles(files),
summaries: filterSummaryFiles(files),
hasResearch: files.some(f => f.endsWith('-RESEARCH.md') || f === 'RESEARCH.md'),
- hasContext: files.some(f => f.endsWith('-CONTEXT.md') || f === 'CONTEXT.md'),
+ hasContext: findContextMdIn(files) !== null,
hasVerification: files.some(f => f.endsWith('-VERIFICATION.md') || f === 'VERIFICATION.md'),
hasReviews: files.some(f => f.endsWith('-REVIEWS.md') || f === 'REVIEWS.md'),
};
diff --git a/get-shit-done/bin/lib/gap-checker.cjs b/get-shit-done/bin/lib/gap-checker.cjs
index 0869581af..cd8e35054 100644
--- a/get-shit-done/bin/lib/gap-checker.cjs
+++ b/get-shit-done/bin/lib/gap-checker.cjs
@@ -17,7 +17,7 @@
const fs = require('fs');
const path = require('path');
const { escapeRegex, output, error } = require('./core.cjs');
-const { planningPaths, planningDir } = require('./planning-workspace.cjs');
+const { planningPaths, planningDir, findContextMdIn } = require('./planning-workspace.cjs');
const { parseDecisions } = require('./decisions.cjs');
/**
@@ -133,16 +133,24 @@ function runGapAnalysis(cwd, phaseDir) {
const reqMd = fs.existsSync(reqPath) ? fs.readFileSync(reqPath, 'utf-8') : '';
const reqItems = parseRequirements(reqMd).map(r => ({ ...r, source: 'REQUIREMENTS.md' }));
- const ctxPath = path.join(absPhaseDir, 'CONTEXT.md');
- const ctxMd = fs.existsSync(ctxPath) ? fs.readFileSync(ctxPath, 'utf-8') : '';
+ // Read the phase directory once; reuse the listing for both context detection
+ // and plan-file enumeration (avoids redundant readdirSync calls).
+ let phaseDirFiles = [];
+ try {
+ if (fs.existsSync(absPhaseDir)) phaseDirFiles = fs.readdirSync(absPhaseDir);
+ } catch { /* unreadable */ }
+
+ const ctxFile = findContextMdIn(phaseDirFiles);
+ const ctxPath = ctxFile ? path.join(absPhaseDir, ctxFile) : null;
+ const ctxMd = ctxPath ? fs.readFileSync(ctxPath, 'utf-8') : '';
const dItems = parseDecisions(ctxMd).map(d => ({ ...d, source: 'CONTEXT.md' }));
const items = [...reqItems, ...dItems];
let planText = '';
try {
- if (fs.existsSync(absPhaseDir)) {
- const files = fs.readdirSync(absPhaseDir).filter(f => /-PLAN\.md$/.test(f));
+ if (phaseDirFiles.length > 0) {
+ const files = phaseDirFiles.filter(f => /-PLAN\.md$/.test(f));
planText = files.map(f => {
try { return fs.readFileSync(path.join(absPhaseDir, f), 'utf-8'); }
catch { return ''; }
diff --git a/get-shit-done/bin/lib/init.cjs b/get-shit-done/bin/lib/init.cjs
index a7d155430..04fc77cbd 100644
--- a/get-shit-done/bin/lib/init.cjs
+++ b/get-shit-done/bin/lib/init.cjs
@@ -6,7 +6,7 @@ const fs = require('fs');
const path = require('path');
const { execGit, platformWriteSync, platformReadSync } = require('./shell-command-projection.cjs');
const { loadConfig, resolveModelInternal, findPhaseInternal, getRoadmapPhaseInternal, pathExistsInternal, gitWorktreeInfoInternal, generateSlugInternal, getMilestoneInfo, getMilestonePhaseFilter, stripShippedMilestones, extractCurrentMilestone, normalizePhaseName, toPosixPath, output, error, checkAgentsInstalled, phaseTokenMatches } = require('./core.cjs');
-const { planningPaths, planningDir, planningRoot } = require('./planning-workspace.cjs');
+const { planningPaths, planningDir, planningRoot, findContextMdIn } = require('./planning-workspace.cjs');
const { maskIfSecret } = require('./secrets.cjs');
const scanPhasePlans = require('./plan-scan.cjs');
const { stateExtractField } = require('./state-document.cjs');
@@ -407,7 +407,7 @@ function cmdInitPlanPhase(cwd, phase, raw, options = {}) {
const phaseDirFull = path.join(cwd, phaseInfo.directory);
try {
const files = fs.readdirSync(phaseDirFull);
- const contextFile = files.find(f => f.endsWith('-CONTEXT.md') || f === 'CONTEXT.md');
+ const contextFile = findContextMdIn(phaseDirFull);
if (contextFile) {
result.context_path = toPosixPath(path.join(phaseInfo.directory, contextFile));
}
@@ -885,7 +885,7 @@ function cmdInitPhaseOp(cwd, phase, raw) {
const phaseDirFull = path.join(cwd, phaseInfo.directory);
try {
const files = fs.readdirSync(phaseDirFull);
- const contextFile = files.find(f => f.endsWith('-CONTEXT.md') || f === 'CONTEXT.md');
+ const contextFile = findContextMdIn(phaseDirFull);
if (contextFile) {
result.context_path = toPosixPath(path.join(phaseInfo.directory, contextFile));
}
@@ -1198,7 +1198,7 @@ function cmdInitManager(cwd, raw) {
const phaseFiles = fs.readdirSync(fullDir);
planCount = listPhasePlanFiles(fullDir).length;
summaryCount = listPhaseSummaryFiles(fullDir).length;
- hasContext = phaseFiles.some(f => f.endsWith('-CONTEXT.md') || f === 'CONTEXT.md');
+ hasContext = findContextMdIn(fullDir) !== null;
hasResearch = phaseFiles.some(f => f.endsWith('-RESEARCH.md') || f === 'RESEARCH.md');
if (summaryCount >= planCount && planCount > 0) diskStatus = 'complete';
diff --git a/get-shit-done/bin/lib/planning-workspace.cjs b/get-shit-done/bin/lib/planning-workspace.cjs
index 79106ab45..bb195bb39 100644
--- a/get-shit-done/bin/lib/planning-workspace.cjs
+++ b/get-shit-done/bin/lib/planning-workspace.cjs
@@ -371,6 +371,35 @@ function setActiveWorkstream(cwd, name) {
createPlanningWorkspace(cwd).activeWorkstream.set(name);
}
+/**
+ * Locate the CONTEXT.md file in a phase directory, handling both the bare
+ * form (`CONTEXT.md`) and the padded-prefix convention (`NN-CONTEXT.md`,
+ * `NN.N-CONTEXT.md`, etc.) used by gsd-discuss-phase output.
+ *
+ * Returns the filename (not the full path) of the first match, or null if
+ * no CONTEXT.md exists in the directory.
+ *
+ * Canonical dual-form predicate extracted here to eliminate the 5-site
+ * duplication that previously existed across init.cjs, roadmap.cjs,
+ * core.cjs, gap-checker.cjs (#3739).
+ *
+ * @param {string|string[]} absDirOrFiles - Absolute path to the phase directory,
+ * OR an already-read files array (avoids a redundant readdirSync at call sites
+ * that already hold a directory listing).
+ * @returns {string|null}
+ */
+function findContextMdIn(absDirOrFiles) {
+ try {
+ const files = Array.isArray(absDirOrFiles)
+ ? absDirOrFiles
+ : fs.readdirSync(absDirOrFiles);
+ if (files.includes('CONTEXT.md')) return 'CONTEXT.md';
+ return files.find(f => f.endsWith('-CONTEXT.md')) ?? null;
+ } catch {
+ return null;
+ }
+}
+
module.exports = {
createPlanningWorkspace,
createSharedPointerAdapter,
@@ -382,4 +411,5 @@ module.exports = {
withPlanningLock,
getActiveWorkstream,
setActiveWorkstream,
+ findContextMdIn,
};
diff --git a/get-shit-done/bin/lib/roadmap.cjs b/get-shit-done/bin/lib/roadmap.cjs
index 7ce7d6e8e..901b0d3a9 100644
--- a/get-shit-done/bin/lib/roadmap.cjs
+++ b/get-shit-done/bin/lib/roadmap.cjs
@@ -6,7 +6,7 @@ const fs = require('fs');
const path = require('path');
const { escapeRegex, normalizePhaseName, phaseMarkdownRegexSource, phaseMarkdownRegexSourceExact, output, error, findPhaseInternal, stripShippedMilestones, extractCurrentMilestone, replaceInCurrentMilestone, phaseTokenMatches } = require('./core.cjs');
const { platformWriteSync } = require('./shell-command-projection.cjs');
-const { planningPaths, withPlanningLock } = require('./planning-workspace.cjs');
+const { planningPaths, withPlanningLock, findContextMdIn } = require('./planning-workspace.cjs');
const scanPhasePlans = require('./plan-scan.cjs');
/**
@@ -41,13 +41,13 @@ function coerceTruthToString(t) {
function countPhasePlansAndSummaries(phaseDir) {
const { planCount, summaryCount } = scanPhasePlans(phaseDir);
// hasContext and hasResearch are not plan-scan concerns — read the directory
- // once for the non-plan metadata that cmdRoadmapAnalyze needs.
+ // once and share the listing for all non-plan metadata that cmdRoadmapAnalyze needs.
let phaseFiles = [];
try { phaseFiles = fs.readdirSync(phaseDir); } catch { /* empty */ }
return {
planCount,
summaryCount,
- hasContext: phaseFiles.some(f => f.endsWith('-CONTEXT.md') || f === 'CONTEXT.md'),
+ hasContext: findContextMdIn(phaseFiles) !== null,
hasResearch: phaseFiles.some(f => f.endsWith('-RESEARCH.md') || f === 'RESEARCH.md'),
};
}
diff --git a/tests/bug-2798-context-window-config-key.test.cjs b/tests/bug-2798-context-window-config-key.test.cjs
index cb1d11b23..26c8d34d4 100644
--- a/tests/bug-2798-context-window-config-key.test.cjs
+++ b/tests/bug-2798-context-window-config-key.test.cjs
@@ -55,7 +55,11 @@ describe('bug-2798: context_window is a valid config key', () => {
cleanup(tmpDir);
});
- test('config-set context_window succeeds (not rejected as unknown key)', () => {
+ test('config-set context_window succeeds (not rejected as unknown key)', (t) => {
+ if (!fs.existsSync(SDK_CLI)) {
+ t.skip('sdk/dist/cli.js not built — run `cd sdk && npm run build` to enable this integration test');
+ return;
+ }
const result = runConfigSet('context_window', 1000000, tmpDir);
assert.strictEqual(result.exitCode, 0, 'should exit 0 (key is valid)');
@@ -64,7 +68,11 @@ describe('bug-2798: context_window is a valid config key', () => {
assert.strictEqual(result.json?.key, 'context_window');
});
- test('context_window value is written to config.json', () => {
+ test('context_window value is written to config.json', (t) => {
+ if (!fs.existsSync(SDK_CLI)) {
+ t.skip('sdk/dist/cli.js not built — run `cd sdk && npm run build` to enable this integration test');
+ return;
+ }
runConfigSet('context_window', 500000, tmpDir);
const config = JSON.parse(
@@ -73,7 +81,11 @@ describe('bug-2798: context_window is a valid config key', () => {
assert.strictEqual(config.context_window, 500000, 'context_window should be persisted');
});
- test('config-schema CJS and SDK allowlists both include context_window', () => {
+ test('config-schema CJS and SDK allowlists both include context_window', (t) => {
+ if (!fs.existsSync(path.join(REPO_ROOT, 'sdk', 'dist', 'query', 'config-schema.js'))) {
+ t.skip('sdk/dist/query/config-schema.js not built — run `cd sdk && npm run build` to enable this integration test');
+ return;
+ }
const cjsSchema = require(path.join(REPO_ROOT, 'get-shit-done', 'bin', 'lib', 'config-schema.cjs'));
const sdkSchema = require(path.join(REPO_ROOT, 'sdk', 'dist', 'query', 'config-schema.js'));
diff --git a/tests/bug-3739-gap-checker-padded-prefix-context.test.cjs b/tests/bug-3739-gap-checker-padded-prefix-context.test.cjs
new file mode 100644
index 000000000..36fb84238
--- /dev/null
+++ b/tests/bug-3739-gap-checker-padded-prefix-context.test.cjs
@@ -0,0 +1,222 @@
+/**
+ * Bug #3739: gap-analysis silently skips CONTEXT.md decisions when the file
+ * uses the padded-prefix convention (e.g. 01-CONTEXT.md, 02.1-CONTEXT.md).
+ *
+ * Verifies:
+ * 1. Padded-prefix CONTEXT.md (NN-CONTEXT.md) decisions ARE included in the
+ * gap report — was silently skipped before the fix.
+ * 2. Decisions from padded-prefix CONTEXT.md ARE checked for coverage.
+ * 3. Bare CONTEXT.md still works — no regression on the existing path.
+ * 4. A padded-prefix decision that is NOT covered in the plan is surfaced
+ * as "Not covered" (not silently dropped from the report).
+ * 5. planning-workspace.cjs findContextMdIn() helper returns the right
+ * filename for both bare and padded forms (unit test for the extractor).
+ */
+
+'use strict';
+
+const { describe, test, beforeEach, afterEach } = require('node:test');
+const assert = require('node:assert/strict');
+const fs = require('fs');
+const path = require('path');
+const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs');
+
+const REPO_ROOT = path.join(__dirname, '..');
+
+describe('bug #3739 — gap-analysis padded-prefix CONTEXT.md', () => {
+ let tmpDir;
+ let phaseDir;
+
+ function writeRequirements(ids) {
+ const lines = ids.map((id, i) => `- [ ] **${id}** Requirement ${i + 1}`);
+ fs.writeFileSync(
+ path.join(tmpDir, '.planning', 'REQUIREMENTS.md'),
+ `# Requirements\n\n${lines.join('\n')}\n`
+ );
+ }
+
+ function writeContextAs(filename, decisions) {
+ const dLines = decisions.map(d => `- **${d.id}:** ${d.text}`).join('\n');
+ fs.writeFileSync(
+ path.join(phaseDir, filename),
+ `# Phase Context\n\n\n## Implementation Decisions\n\n${dLines}\n\n`
+ );
+ }
+
+ function writePlan(name, body) {
+ fs.writeFileSync(path.join(phaseDir, `${name}-PLAN.md`), body);
+ }
+
+ function ensureConfig() {
+ const r = runGsdTools('config-ensure-section', tmpDir);
+ assert.ok(r.success, `config-ensure-section failed: ${r.error}`);
+ }
+
+ beforeEach(() => {
+ tmpDir = createTempProject();
+ phaseDir = path.join(tmpDir, '.planning', 'phases', '01-test');
+ fs.mkdirSync(phaseDir, { recursive: true });
+ ensureConfig();
+ });
+
+ afterEach(() => {
+ cleanup(tmpDir);
+ });
+
+ // ── Test 1: padded-prefix decisions appear in the gap report ─────────────
+
+ test('decisions from padded-prefix CONTEXT.md (01-CONTEXT.md) appear in gap report', () => {
+ writeContextAs('01-CONTEXT.md', [
+ { id: 'D-01', text: 'Use library X' },
+ { id: 'D-02', text: 'Fail loud on unknown input' },
+ ]);
+ writePlan('01', '# Plan\n\nImplements D-01 and D-02.\n');
+
+ const r = runGsdTools(['gap-analysis', '--phase-dir', phaseDir], tmpDir);
+ assert.ok(r.success, `gap-analysis failed: ${r.error}`);
+ const out = JSON.parse(r.output);
+
+ const d01 = out.rows.find(x => x.item === 'D-01');
+ const d02 = out.rows.find(x => x.item === 'D-02');
+
+ assert.ok(d01, 'D-01 row must appear in gap report when CONTEXT.md uses padded-prefix 01-CONTEXT.md');
+ assert.ok(d02, 'D-02 row must appear in gap report when CONTEXT.md uses padded-prefix 01-CONTEXT.md');
+ assert.strictEqual(d01.source, 'CONTEXT.md', 'source label must be CONTEXT.md');
+ assert.strictEqual(d01.status, 'Covered', 'D-01 is mentioned in plan — must be Covered');
+ assert.strictEqual(d02.status, 'Covered', 'D-02 is mentioned in plan — must be Covered');
+ });
+
+ // ── Test 2: uncovered padded-prefix decision surfaces as Not covered ──────
+
+ test('uncovered decision from padded-prefix CONTEXT.md surfaces as Not covered', () => {
+ writeContextAs('01-CONTEXT.md', [
+ { id: 'D-01', text: 'Use library X' },
+ ]);
+ writePlan('01', '# Plan\n\nUnrelated work, no mention of any D-NN.\n');
+
+ const r = runGsdTools(['gap-analysis', '--phase-dir', phaseDir], tmpDir);
+ assert.ok(r.success, `gap-analysis failed: ${r.error}`);
+ const out = JSON.parse(r.output);
+
+ const d01 = out.rows.find(x => x.item === 'D-01');
+ assert.ok(d01, 'D-01 row must appear even when not covered');
+ assert.strictEqual(d01.status, 'Not covered',
+ 'D-01 must be Not covered (not silently absent) when plan omits it');
+ });
+
+ // ── Test 3 (counter-test): bare CONTEXT.md still works — no regression ───
+
+ test('bare CONTEXT.md still works (regression guard)', () => {
+ writeContextAs('CONTEXT.md', [
+ { id: 'D-05', text: 'Bare form decision' },
+ ]);
+ writePlan('01', '# Plan\n\nImplements D-05.\n');
+
+ const r = runGsdTools(['gap-analysis', '--phase-dir', phaseDir], tmpDir);
+ assert.ok(r.success, `gap-analysis failed: ${r.error}`);
+ const out = JSON.parse(r.output);
+
+ const d05 = out.rows.find(x => x.item === 'D-05');
+ assert.ok(d05, 'D-05 must appear when CONTEXT.md uses bare filename');
+ assert.strictEqual(d05.status, 'Covered', 'D-05 must be Covered');
+ });
+
+ // ── Test 4: deeper padded prefix (02.1-CONTEXT.md) ───────────────────────
+
+ test('multi-segment padded prefix (02.1-CONTEXT.md) decisions appear in gap report', () => {
+ writeContextAs('02.1-CONTEXT.md', [
+ { id: 'D-03', text: 'Use postgres' },
+ ]);
+ writePlan('01', '# Plan\n\nImplements D-03.\n');
+
+ const r = runGsdTools(['gap-analysis', '--phase-dir', phaseDir], tmpDir);
+ assert.ok(r.success, `gap-analysis failed: ${r.error}`);
+ const out = JSON.parse(r.output);
+
+ const d03 = out.rows.find(x => x.item === 'D-03');
+ assert.ok(d03, 'D-03 must appear from 02.1-CONTEXT.md');
+ assert.strictEqual(d03.status, 'Covered');
+ });
+
+ // ── Test 5: findContextMdIn helper unit test ─────────────────────────────
+
+ test('findContextMdIn helper returns padded filename when present', () => {
+ const { findContextMdIn } = require('../get-shit-done/bin/lib/planning-workspace.cjs');
+ // Write 01-CONTEXT.md into the phase dir (already created in beforeEach)
+ fs.writeFileSync(path.join(phaseDir, '01-CONTEXT.md'), '# context\n');
+
+ const found = findContextMdIn(phaseDir);
+ assert.strictEqual(found, '01-CONTEXT.md',
+ 'findContextMdIn must return the padded-prefix filename');
+ });
+
+ test('findContextMdIn helper returns bare filename when only bare form exists', () => {
+ const { findContextMdIn } = require('../get-shit-done/bin/lib/planning-workspace.cjs');
+ fs.writeFileSync(path.join(phaseDir, 'CONTEXT.md'), '# context\n');
+
+ const found = findContextMdIn(phaseDir);
+ assert.strictEqual(found, 'CONTEXT.md',
+ 'findContextMdIn must return CONTEXT.md for bare form');
+ });
+
+ test('findContextMdIn helper returns null when no CONTEXT.md exists', () => {
+ const { findContextMdIn } = require('../get-shit-done/bin/lib/planning-workspace.cjs');
+ // phaseDir exists but is empty (no CONTEXT.md)
+ const found = findContextMdIn(phaseDir);
+ assert.strictEqual(found, null,
+ 'findContextMdIn must return null when no CONTEXT.md exists');
+ });
+
+ // ── Test 5b: findContextMdIn accepts pre-read files array (avoids double readdirSync) ──
+
+ test('findContextMdIn accepts an already-read files array (avoids double readdirSync)', () => {
+ const { findContextMdIn } = require('../get-shit-done/bin/lib/planning-workspace.cjs');
+ // Passing an array directly should behave identically to passing a directory path.
+ assert.strictEqual(findContextMdIn(['CONTEXT.md', 'other.md']), 'CONTEXT.md',
+ 'bare form found in array');
+ assert.strictEqual(findContextMdIn(['01-CONTEXT.md', 'other.md']), '01-CONTEXT.md',
+ 'padded form found in array');
+ assert.strictEqual(findContextMdIn(['unrelated.md']), null,
+ 'returns null when no CONTEXT.md in array');
+ // Bare wins over padded when both are present
+ assert.strictEqual(findContextMdIn(['01-CONTEXT.md', 'CONTEXT.md']), 'CONTEXT.md',
+ 'bare form preferred over padded form when both in array');
+ });
+
+ // ── Test 6: dual-file precedence — bare CONTEXT.md wins over padded form ──
+
+ test('findContextMdIn prefers bare CONTEXT.md over padded form (helper level)', () => {
+ const { findContextMdIn } = require('../get-shit-done/bin/lib/planning-workspace.cjs');
+ // Write BOTH forms into the phase directory
+ fs.writeFileSync(path.join(phaseDir, 'CONTEXT.md'), '# bare context\n');
+ fs.writeFileSync(path.join(phaseDir, '01-CONTEXT.md'), '# padded context\n');
+
+ const found = findContextMdIn(phaseDir);
+ assert.strictEqual(found, 'CONTEXT.md',
+ 'findContextMdIn must return bare CONTEXT.md when both forms exist — matches pre-refactor gap-checker behavior');
+ });
+
+ test('gap-analysis uses bare CONTEXT.md decisions when both forms exist (integration level)', () => {
+ // Bare form has D-BARE; padded form has D-PADDED.
+ // If the integration path resolves bare correctly, only D-BARE appears in the report.
+ const bareContent =
+ '# Phase Context\n\n\n## Implementation Decisions\n\n- **D-BARE:** From bare form\n\n';
+ const paddedContent =
+ '# Phase Context\n\n\n## Implementation Decisions\n\n- **D-PADDED:** From padded form\n\n';
+ fs.writeFileSync(path.join(phaseDir, 'CONTEXT.md'), bareContent);
+ fs.writeFileSync(path.join(phaseDir, '01-CONTEXT.md'), paddedContent);
+
+ writePlan('01', '# Plan\n\nImplements D-BARE.\n');
+
+ const r = runGsdTools(['gap-analysis', '--phase-dir', phaseDir], tmpDir);
+ assert.ok(r.success, `gap-analysis failed: ${r.error}`);
+ const out = JSON.parse(r.output);
+
+ const dBare = out.rows.find(x => x.item === 'D-BARE');
+ const dPadded = out.rows.find(x => x.item === 'D-PADDED');
+
+ assert.ok(dBare, 'D-BARE (from bare CONTEXT.md) must appear in gap report');
+ assert.ok(!dPadded, 'D-PADDED (from 01-CONTEXT.md) must NOT appear — bare form takes precedence');
+ assert.strictEqual(dBare.status, 'Covered', 'D-BARE must be Covered');
+ });
+});
diff --git a/tests/locking-bugs-1909-1916-1925-1927.test.cjs b/tests/locking-bugs-1909-1916-1925-1927.test.cjs
index 191c50774..6105dc016 100644
--- a/tests/locking-bugs-1909-1916-1925-1927.test.cjs
+++ b/tests/locking-bugs-1909-1916-1925-1927.test.cjs
@@ -20,7 +20,7 @@ const assert = require('node:assert/strict');
const fs = require('fs');
const path = require('path');
const os = require('os');
-const { execFileSync, execSync } = require('child_process');
+const { execFileSync, execSync, spawn } = require('child_process');
const { promisify } = require('util');
const { exec } = require('child_process');
@@ -233,6 +233,26 @@ describe('#1925 TOCTOU: state commands use readModifyWriteStateMd', () => {
});
test('state add-blocker: both concurrent calls append different blockers', async () => {
+ // Deterministic concurrency via file-barrier synchronization (Option A).
+ //
+ // Problem with the prior design: Promise.all([execAsync(A), execAsync(B)])
+ // offers no guarantee that both subprocesses are alive simultaneously. On a
+ // loaded CI runner one subprocess can fully complete (acquire lock → transform
+ // → release lock → exit) before the other's Node runtime has even started.
+ // When that happens the second subprocess never contends on the lock — the
+ // test trivially passes — but the test also fails to exercise what it claims
+ // to test. On Docker overlay-fs under load the opposite pathology occurs:
+ // both subprocesses race O_EXCL creation, and depending on scheduler timing
+ // one can observe stale fs state, causing a lost update that fails the
+ // assertion. Either way, the outcome is non-deterministic.
+ //
+ // Redesign: a barrier file forces both subprocesses to reach their "ready"
+ // gate before either is allowed to proceed. The barrier is removed only
+ // after BOTH have signalled readiness, guaranteeing true overlap in the
+ // critical section. No sleep-based synchronization; the barrier loop uses
+ // Atomics.wait (same primitive as acquireStateLock) so it yields the CPU
+ // instead of spinning.
+
writeStateMd(tmpDir, [
'# Project State',
'',
@@ -242,14 +262,96 @@ describe('#1925 TOCTOU: state commands use readModifyWriteStateMd', () => {
'None.',
].join('\n') + '\n');
- const nodeBin = process.execPath;
- const cmdA = `"${nodeBin}" "${TOOLS_PATH}" state add-blocker --text "Need API credentials" --cwd "${tmpDir}"`;
- const cmdB = `"${nodeBin}" "${TOOLS_PATH}" state add-blocker --text "Waiting for design review" --cwd "${tmpDir}"`;
+ // ── Barrier infrastructure ────────────────────────────────────────────────
+ // barrierPath: exists while subprocesses must hold. Removed by the test
+ // orchestrator once both subprocesses have signalled readiness.
+ // ready-{id}: each subprocess creates this file to signal it is at the gate.
+ const barrierPath = path.join(tmpDir, '.barrier');
+ const readyA = path.join(tmpDir, '.ready-a');
+ const readyB = path.join(tmpDir, '.ready-b');
+ fs.writeFileSync(barrierPath, '1'); // erect the barrier
+ if (fs.existsSync(readyA)) fs.unlinkSync(readyA);
+ if (fs.existsSync(readyB)) fs.unlinkSync(readyB);
- await Promise.all([
- execAsync(cmdA, { encoding: 'utf-8' }).catch(() => {}),
- execAsync(cmdB, { encoding: 'utf-8' }).catch(() => {}),
- ]);
+ // ── Wrapper script written to tmpDir ─────────────────────────────────────
+ // Each subprocess runs this wrapper, which:
+ // 1. Writes its ready-signal so the orchestrator knows it is alive.
+ // 2. Spins (Atomics.wait, 10 ms steps) until the barrier is removed.
+ // 3. Immediately calls gsd-tools to exercise the real lock contention.
+ //
+ // TOOLS_PATH and the caller-supplied args are injected via env vars to avoid
+ // shell-quoting complexity when the tmpDir path contains spaces.
+ const wrapperPath = path.join(tmpDir, '.barrier-wrapper.cjs');
+ fs.writeFileSync(wrapperPath, [
+ "'use strict';",
+ 'const fs = require("fs");',
+ 'const path = require("path");',
+ 'const { execFileSync } = require("child_process");',
+ 'const { TOOLS_PATH, BARRIER_FILE, READY_FILE, BLOCKER_TEXT, CWD_PATH } = process.env;',
+ '',
+ '// Signal readiness to the orchestrator.',
+ 'fs.writeFileSync(READY_FILE, String(process.pid));',
+ '',
+ '// Wait at the barrier (yield via Atomics.wait so we do not spin the CPU).',
+ '// Budget: 10 s — if the orchestrator never releases us, something is broken.',
+ 'const sab = new SharedArrayBuffer(4);',
+ 'const sai = new Int32Array(sab);',
+ 'const deadline = Date.now() + 10000;',
+ 'while (fs.existsSync(BARRIER_FILE)) {',
+ ' if (Date.now() > deadline) { process.stderr.write("barrier timeout\\n"); process.exit(1); }',
+ ' Atomics.wait(sai, 0, 0, 10); // sleep 10 ms, then re-check',
+ '}',
+ '',
+ '// Barrier is down — execute the actual gsd-tools command.',
+ 'execFileSync(process.execPath, [TOOLS_PATH, "state", "add-blocker", "--text", BLOCKER_TEXT, "--cwd", CWD_PATH], {',
+ ' stdio: "pipe",',
+ '});',
+ ].join('\n'));
+
+ const nodeBin = process.execPath;
+
+ // ── Spawn both subprocesses ───────────────────────────────────────────────
+ // Both start immediately; both block at the barrier until the orchestrator
+ // confirms both are ready, then both proceed to contend on the STATE.md lock.
+ function spawnWrapper(blockerId, readyFile) {
+ return new Promise((resolve, reject) => {
+ const child = spawn(nodeBin, [wrapperPath], {
+ env: {
+ ...process.env,
+ TOOLS_PATH,
+ BARRIER_FILE: barrierPath,
+ READY_FILE: readyFile,
+ BLOCKER_TEXT: blockerId,
+ CWD_PATH: tmpDir,
+ },
+ stdio: 'pipe',
+ });
+ let stderr = '';
+ child.stderr.on('data', (d) => { stderr += d.toString(); });
+ child.on('close', (code) => {
+ if (code !== 0) reject(new Error(`wrapper exited ${code}: ${stderr}`));
+ else resolve();
+ });
+ });
+ }
+
+ const promiseA = spawnWrapper('Need API credentials', readyA);
+ const promiseB = spawnWrapper('Waiting for design review', readyB);
+
+ // ── Orchestrate: wait for both ready-signals, then drop the barrier ───────
+ // Poll with Atomics.wait (10 ms steps). Budget: 10 s.
+ const sab2 = new SharedArrayBuffer(4);
+ const sai2 = new Int32Array(sab2);
+ const deadline2 = Date.now() + 10000;
+ while (!fs.existsSync(readyA) || !fs.existsSync(readyB)) {
+ if (Date.now() > deadline2) throw new Error('Timed out waiting for both subprocesses to reach barrier');
+ Atomics.wait(sai2, 0, 0, 10);
+ }
+ // Both subprocesses are at the gate — drop the barrier simultaneously.
+ fs.unlinkSync(barrierPath);
+
+ // ── Collect results ───────────────────────────────────────────────────────
+ await Promise.all([promiseA, promiseB]);
const content = readStateMd(tmpDir);
assert.ok(