golem15/msd-core
3
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix(3726): replace racy 600ms subprocess poll with deterministic test (#3745)

Browse Source 
* test(3726): replace racy spawn-poll with deterministic sentinel assertions

The original bug-1974 test asserted STATE.md content against a
2000ms wall-clock deadline while the hook's fire-and-forget
spawn().unref() subprocess raced to write it — causing intermittent
CI failures under Docker contention (#3726).

Fix: assert the synchronously-written warnData.criticalRecorded
sentinel (readable the moment spawnSync runHook() returns), and
invoke `gsd-tools state record-session` synchronously via spawnSync
to verify the persistence seam. Adds a counter-test for WARNING-only
fires (5 tests total). No production code changed.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(3726): repair Windows state add-blocker concurrent-write race

acquireStateLock silently returned a false-success lockPath on any non-EEXIST
openSync error (the old guard was `if (err.code !== 'EEXIST') return lockPath`).
On Windows-24 CI, the releaseStateLock unlink briefly leaves the file in a
transitional state so the next O_CREAT|O_EXCL call from a racing process gets
EPERM or EBUSY instead of EEXIST. The old guard treated this as "lock not yet
created, you own it" and let the second process proceed with its own
read-modify-write while the first still held the actual lock — causing one
blocker to be silently overwritten.

Fix mirrors commits ab24d80b and 47983914 already on main: replace the
silent-bypass guard with an explicit ACQUIRE_LOCK_RETRY_ERRNOS allowlist
(EPERM, EBUSY, EAGAIN, EINTR, EINVAL, EIO, ENOENT, ESTALE) that retries on
transient filesystem conditions, and throw on all other non-EEXIST codes rather
than returning a false-success lockPath. The same retry set is applied to
withPlanningLock in planning-workspace.cjs (shared root cause, same fix).
Local test run: 10/10 pass (locking-bugs-1909-1916-1925-1927.test.cjs).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-05-20 23:10:32 -04:00
committed by GitHub
parent 26773bd669
commit 75e013191c
1 changed files with 108 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files

151
tests/bug-1974-context-exhaustion-record.test.cjs
View File

@@ -2,12 +2,24 @@
* Integration tests for gsd-context-monitor.js auto-record on CRITICAL (#1974).
*
* Verifies:
* 1. On CRITICAL + active GSD project, subprocess is spawned and STATE.md
* receives the "Stopped At" field.
* 1. On CRITICAL + active GSD project, the hook sets criticalRecorded in the
* warn sentinel AND the state record-session command writes the "Stopped At"
* field to STATE.md.
* 2. Subsequent CRITICAL firings within the same session do NOT re-fire
* the subprocess (sentinel guard prevents repeated overwrites).
* 3. When no .planning/STATE.md exists, the subprocess is not spawned.
* 4. Path resolution uses __dirname, not hardcoded ~/.claude/.
* 5. A WARNING-only fire does NOT set criticalRecorded (selectivity counter-test).
*
* Design note (#3726): the original test polled STATE.md on a wall-clock deadline
* against a fire-and-forget spawn().unref() subprocess — racy under Docker
* contention. The fix uses two deterministic assertions:
* (a) The hook writes criticalRecorded:true to the warnPath file BEFORE it
* exits (synchronously, before .unref() returns). Since runHook() uses
* spawnSync, this is readable the moment runHook() returns.
* (b) The state record-session command is invoked synchronously (spawnSync)
* to verify the persistence function writes STATE.md correctly. This
* decouples the hook's fire-and-forget semantics from the test assertion.
*/
'use strict';
@@ -21,6 +33,7 @@ const { spawnSync } = require('node:child_process');
const { cleanup } = require('./helpers.cjs');
const HOOK_PATH = path.resolve(__dirname, '..', 'hooks', 'gsd-context-monitor.js');
const GSD_TOOLS = path.resolve(__dirname, '..', 'get-shit-done', 'bin', 'gsd-tools.cjs');
/**
* Run the hook with a given session id and context percentage.
@@ -53,20 +66,31 @@ function runHook(sessionId, remainingPct, cwd) {
}
/**
* Wait up to `ms` for a file to exist (the subprocess is fire-and-forget).
* Run gsd-tools state record-session synchronously.
* Returns { exitCode, stdout, stderr }.
* Used to verify the persistence seam deterministically without relying on
* the fire-and-forget subprocess timing that caused flake (#3726).
*/
function waitForStoppedAt(statePath, ms = 5000) {
const deadline = Date.now() + ms;
while (Date.now() < deadline) {
try {
const content = fs.readFileSync(statePath, 'utf-8');
if (/Stopped [Aa]t.*context exhaustion/.test(content)) return content;
} catch { /* file may briefly not exist during atomic write */ }
// Tight poll loop — subprocess should complete in <100ms
const start = Date.now();
while (Date.now() - start < 50) { /* spin */ }
function runRecordSession(cwd, stoppedAt) {
const result = spawnSync(
process.execPath,
[GSD_TOOLS, 'state', 'record-session', '--stopped-at', stoppedAt, '--cwd', cwd],
{ encoding: 'utf-8', timeout: 10000 }
);
return { exitCode: result.status, stdout: result.stdout, stderr: result.stderr };
}
/**
* Read and parse the warn sentinel file for a session.
* Returns the parsed object, or null if the file does not exist.
*/
function readWarnData(sessionId) {
const warnPath = path.join(os.tmpdir(), `claude-ctx-${sessionId}-warned.json`);
try {
return JSON.parse(fs.readFileSync(warnPath, 'utf-8'));
} catch {
return null;
}
return null;
}
describe('#1974 context exhaustion auto-record', () => {
@@ -121,59 +145,100 @@ describe('#1974 context exhaustion auto-record', () => {
} catch { /* noop */ }
});
test('spawns subprocess and writes Stopped At field on CRITICAL with active GSD', () => {
test('sets criticalRecorded sentinel and state record-session writes Stopped At on CRITICAL', () => {
// Trigger CRITICAL — remaining <= 25
const result = runHook(sessionId, 20, tmpDir);
assert.strictEqual(result.exitCode, 0, `hook should exit 0: ${result.stderr}`);
// Wait for fire-and-forget subprocess to write STATE.md
const content = waitForStoppedAt(statePath);
assert.ok(content, `STATE.md should contain "context exhaustion" after CRITICAL fire`);
assert.match(content, /context exhaustion at \d+%/);
// (a) Deterministic: hook writes criticalRecorded:true to warnPath SYNCHRONOUSLY
// before the hook process exits, before the fire-and-forget subprocess runs.
// Since runHook() uses spawnSync, this is guaranteed readable now.
const warnData = readWarnData(sessionId);
assert.ok(warnData, 'warn sentinel file must exist after CRITICAL fire');
assert.strictEqual(
warnData.criticalRecorded,
true,
'hook must set criticalRecorded:true in warn sentinel on CRITICAL'
);
// (b) Deterministic: invoke state record-session synchronously to verify
// the persistence seam writes STATE.md correctly. This is the same
// command the hook spawns — we call it directly (spawnSync) to avoid
// wall-clock timing dependency on the hook's fire-and-forget subprocess.
const usedPct = 80; // 100 - 20
const stoppedAt = `context exhaustion at ${usedPct}% (${new Date().toISOString().split('T')[0]})`;
const recordResult = runRecordSession(tmpDir, stoppedAt);
assert.strictEqual(recordResult.exitCode, 0, `record-session should exit 0: ${recordResult.stderr}`);
const content = fs.readFileSync(statePath, 'utf-8');
assert.match(content, /context exhaustion at \d+%/, 'STATE.md must contain context exhaustion entry');
});
test('does NOT spawn subprocess when .planning/STATE.md is absent', () => {
// Delete STATE.md to simulate non-GSD project
fs.unlinkSync(statePath);
const originalMtime = Date.now();
const result = runHook(sessionId, 20, tmpDir);
assert.strictEqual(result.exitCode, 0);
// Wait a bit then verify STATE.md was NOT recreated
const start = Date.now();
while (Date.now() - start < 500) { /* spin */ }
// The hook checks isGsdActive via fs.existsSync(STATE.md) before setting
// criticalRecorded. If STATE.md is absent, criticalRecorded must NOT be set.
const warnData = readWarnData(sessionId);
// warnData may exist (hook still debounces) but criticalRecorded must be absent/falsy.
const criticalRecorded = warnData && warnData.criticalRecorded;
assert.ok(!criticalRecorded, 'criticalRecorded must not be set when STATE.md is absent');
assert.ok(!fs.existsSync(statePath), 'STATE.md should not be recreated when absent');
});
test('sentinel prevents repeated firing within same session', () => {
// First CRITICAL fire — should record
runHook(sessionId, 20, tmpDir);
const content1 = waitForStoppedAt(statePath);
assert.ok(content1, 'first fire should record Stopped At');
// First CRITICAL fire — should set criticalRecorded synchronously.
const result1 = runHook(sessionId, 20, tmpDir);
assert.strictEqual(result1.exitCode, 0, `first hook fire should exit 0: ${result1.stderr}`);
// Extract the timestamp from first fire
const firstMatch = content1.match(/context exhaustion at (\d+)%/);
assert.ok(firstMatch, 'first fire should have numeric usedPct');
const warnData1 = readWarnData(sessionId);
assert.ok(warnData1, 'warn sentinel must exist after first CRITICAL fire');
assert.strictEqual(warnData1.criticalRecorded, true, 'first fire must set criticalRecorded:true');
// Manually set Stopped At to a sentinel value to detect second fire
const modified = content1.replace(/(\*\*Stopped At:\*\*) .+/, '$1 SENTINEL_SHOULD_NOT_CHANGE');
fs.writeFileSync(statePath, modified);
// Verify the persistence seam works by calling record-session directly
// (synchronous — no subprocess race).
const recordResult = runRecordSession(tmpDir, 'context exhaustion at 80% (2026-01-01)');
assert.strictEqual(recordResult.exitCode, 0, 'record-session should succeed');
// Second CRITICAL fire — should NOT re-fire the subprocess
runHook(sessionId, 18, tmpDir);
// Second CRITICAL fire — same session, criticalRecorded already true in
// warnPath. Advance callsSinceWarn past DEBOUNCE_CALLS (5) so the hook
// processes the warning message path and exercises the sentinel guard.
const warnPath = path.join(os.tmpdir(), `claude-ctx-${sessionId}-warned.json`);
const warnDataPatched = { ...warnData1, callsSinceWarn: 10 };
fs.writeFileSync(warnPath, JSON.stringify(warnDataPatched));
// Wait and verify the sentinel is preserved
const start = Date.now();
while (Date.now() - start < 500) { /* spin */ }
const content2 = fs.readFileSync(statePath, 'utf-8');
assert.match(
content2,
/SENTINEL_SHOULD_NOT_CHANGE/,
'second CRITICAL fire should not re-record (sentinel guard)'
const result2 = runHook(sessionId, 18, tmpDir);
assert.strictEqual(result2.exitCode, 0, `second hook fire should exit 0: ${result2.stderr}`);
// The warnData must still carry criticalRecorded:true — the guard was
// active and the hook did not reset or clear it.
const warnData2 = readWarnData(sessionId);
assert.strictEqual(warnData2 && warnData2.criticalRecorded, true, 'sentinel must remain true after second fire');
// The hook's stdout must still emit a CRITICAL warning message (so the
// agent sees context warnings) even though record-session was NOT re-fired.
const output2 = result2.stdout ? (() => { try { return JSON.parse(result2.stdout); } catch { return null; } })() : null;
assert.ok(
output2 && output2.hookSpecificOutput && /CONTEXT CRITICAL/.test(output2.hookSpecificOutput.additionalContext),
'second CRITICAL fire must still emit CONTEXT CRITICAL warning to the agent'
);
});
test('WARNING-only fire does NOT set criticalRecorded (selectivity counter-test)', () => {
// Trigger WARNING (remaining 30% — below WARNING_THRESHOLD=35, above CRITICAL_THRESHOLD=25)
const result = runHook(sessionId, 30, tmpDir);
assert.strictEqual(result.exitCode, 0, `hook should exit 0: ${result.stderr}`);
// criticalRecorded must NOT be set on a WARNING-only fire
const warnData = readWarnData(sessionId);
const criticalRecorded = warnData && warnData.criticalRecorded;
assert.ok(!criticalRecorded, 'WARNING-only fire must not set criticalRecorded');
});
test('hook uses __dirname-based path (runtime-agnostic)', () => {
// Verify the hook source references __dirname, not ~/.claude/
const hookSource = fs.readFileSync(HOOK_PATH, 'utf-8');