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

fix(7): raise concurrency-safety roadmap-analyze budget to 5000ms (Mac flake mitigation) (#157)

Browse Source 
* fix(7): raise concurrency-safety roadmap-analyze budget 2000ms → 5000ms

The 50-phase roadmap-analyze wall-clock test consistently flaked on Mac
under realistic load (empirical floor ~2100-3000ms), independently
reproduced by three fixers on PRs #3795, #3792, #3799.

Extract the magic 2000 to named constant ROADMAP_ANALYZE_BUDGET_MS with
an explanatory comment. Raise to 5000ms (2.5x observed worst-case) —
large enough to eliminate Mac flakes without masking real regressions.

Long-term: convert to behavior-anchored assertion per PR #3803 pattern;
tracked as follow-up, out of scope here.

Acceptance criteria from #7:
> "Replace hardcoded 2000ms with higher budget (e.g., 5000ms) or make
>  configurable; add comment documenting rationale; confirm no
>  false-negative on healthy machine."

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

* fix(7): guard executeForCjs with try/catch fallback in all CJS routers

When the synckit bridge throws (worker crash, Atomics failure on
Windows/Node 24, or any other transient OS-level failure), the exception
propagated uncaught through the sdkHandler closure up to Node.js's
unhandled-rejection handler. On Windows, the async stderr write for the
rejection message may not flush before process exit, producing an empty-
stderr non-zero exit that manifests as 'init failed: Command failed: ...'
in workspace.test.cjs.

Wrap getExecuteForCjs() in a try/catch in every CJS router that has this
pattern (gsd-tools.cjs _dispatchNonFamily, init-, roadmap-, state-,
validate-, verify-command-router.cjs). On catch: fall through to the
CJS handler, which is the designed safety net for bridge failures and
produces identical output.

Verified: workspace.test.cjs (26/26), concurrency-safety.test.cjs
(31/31), cjs-sdk-bridge-integration.test.cjs (4/4), and
bug-3631-router-raw-flag.test.cjs (2/2) all pass locally on Mac.

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-23 16:22:31 -04:00
committed by GitHub
parent 41210f014e
commit 333528843d
7 changed files with 127 additions and 79 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
tests/concurrency-safety.test.cjs
View File

@@ -785,6 +785,12 @@ describe('stress tests with 50+ phases', () => {
cleanup(tmpDir);
});
// Wall-clock budget for the 50-phase ROADMAP analyze.
// Empirical floor on Mac under realistic load is ~2100-3000ms; bumped from
// 2000ms to 5000ms after three independent flake reports (see #7).
// Long-term: convert to behavior-anchored assertion per PR #3803 pattern.
const ROADMAP_ANALYZE_BUDGET_MS = 5000;
test('roadmap analyze on 50-phase ROADMAP completes in under 2000ms', () => {
create50PhaseProject(tmpDir, 25);
@@ -793,7 +799,7 @@ describe('stress tests with 50+ phases', () => {
const elapsed = performance.now() - start;
assert.ok(result.success, `roadmap analyze should succeed: ${result.error}`);
assert.ok(elapsed < 2000, `Should complete in under 2000ms, took ${elapsed.toFixed(0)}ms`);
assert.ok(elapsed < ROADMAP_ANALYZE_BUDGET_MS, `Should complete in under ${ROADMAP_ANALYZE_BUDGET_MS}ms, took ${elapsed.toFixed(0)}ms`);
const output = JSON.parse(result.output);
assert.ok(Array.isArray(output.phases), 'Output should contain a phases array');