From 0dd69202b18c958abf52c3213f0dc400e4ba7575 Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Fri, 4 Sep 2026 10:04:42 +0300 Subject: [PATCH 01/13] test(api-diagnostics): harden Signature B correlator against cross-directory false attribution and signed NTSTATUS codes Harden signature-b-correlate against three proven concrete blind spots: - Require parent failure path to be a bare filename without directory segments before allowing single-candidate basename fallback, eliminating false cross-directory child log attribution when a file terminates before logging. - Perform case-insensitive path comparison on Windows in matchChild so casing differences between argv[1] and runner paths do not cause false misses. - Convert 32-bit exit codes to unsigned in classifyTermination so signed negative NTSTATUS values (e.g. 0xC0000005 as -1073741819) are correctly mapped to table candidates and the 0xC0000000 range. - Relax parseTapFailures regex to accept single-quoted, double-quoted, or unquoted YAML values for error and failureType. - Add 4 regression tests pinning each guarantee. - Signature B remains UNRESOLVED. --- .../diagnostics/signature-b-correlate.cjs | 41 ++++--- .../diagnostics/signature-b-correlate.test.ts | 108 ++++++++++++++++++ 2 files changed, 136 insertions(+), 13 deletions(-) diff --git a/packages/api/test/diagnostics/signature-b-correlate.cjs b/packages/api/test/diagnostics/signature-b-correlate.cjs index 01fa009d..0e70ca3e 100644 --- a/packages/api/test/diagnostics/signature-b-correlate.cjs +++ b/packages/api/test/diagnostics/signature-b-correlate.cjs @@ -123,16 +123,17 @@ function classifyTermination({ exitCode, signal }) { specific: false, }; } - const known = EXIT_CODE_TABLE.find((entry) => entry.code === exitCode); + const unsigned = typeof exitCode === 'number' ? (exitCode >>> 0) : null; + const known = EXIT_CODE_TABLE.find((entry) => entry.code === exitCode || (unsigned !== null && entry.code === unsigned)); if (known) return { id: known.id, meaning: known.meaning, specific: known.specific }; - if (typeof exitCode === 'number' && exitCode >= 0xc0000000) { + if (unsigned !== null && unsigned >= 0xc0000000) { // Membership of the range is not a finding. A native fault produces a value here, but so does a // console CTRL+C (`STATUS_CONTROL_C_EXIT`, in the table above), and an external TerminateProcess // can pass any of them deliberately. An unmeasured value in the range therefore narrows the // shape of the answer without naming a mechanism, and must not be reported as though it had. return { id: 'ntstatus-unmeasured', - meaning: `exit code ${exitCode} (0x${exitCode.toString(16)}) is an NTSTATUS value this table does not cover; a native fault is one candidate, but the range also carries non-fault statuses such as STATUS_CONTROL_C_EXIT, so the number alone names no mechanism`, + meaning: `exit code ${exitCode} (0x${unsigned.toString(16)}) is an NTSTATUS value this table does not cover; a native fault is one candidate, but the range also carries non-fault statuses such as STATUS_CONTROL_C_EXIT, so the number alone names no mechanism`, specific: false, }; } @@ -227,11 +228,11 @@ function parseTapFailures(tapText) { const blocks = String(tapText).matchAll(/^not ok \d+ - (.+?)$\r?\n\s*---\r?\n([\s\S]*?)^\s*\.\.\.$/gm); const out = []; for (const [, rawName, body] of blocks) { - if (!/^\s*error: 'test failed'\s*$/m.test(body)) continue; - if (!/^\s*failureType: 'testCodeFailure'\s*$/m.test(body)) continue; + if (!/^\s*error:\s*['"]?test failed['"]?\s*$/m.test(body)) continue; + if (!/^\s*failureType:\s*['"]?testCodeFailure['"]?\s*$/m.test(body)) continue; if (out.length >= MAX_RECORDS) break; const scalar = (key) => { - const found = new RegExp(`^\\s*${key}: (.+)$`, 'm').exec(body); + const found = new RegExp(`^\\s*${key}:\\s*(.+)$`, 'm').exec(body); return found ? found[1].trim() : null; }; const exit = scalar('exitCode'); @@ -240,8 +241,8 @@ function parseTapFailures(tapText) { out.push({ file: rawName.trim().replace(/\\\\/g, '\\'), exitCode: exit === null || exit === '~' ? null : Number(exit), - signal: sig === null || sig === '~' ? null : sig.replace(/^'|'$/g, ''), - failureType: scalar('failureType')?.replace(/^'|'$/g, '') ?? null, + signal: sig === null || sig === '~' ? null : sig.replace(/^['"]|['"]$/g, ''), + failureType: scalar('failureType')?.replace(/^['"]|['"]$/g, '') ?? null, durationMs: dur === null ? null : Number(dur), }); } @@ -313,16 +314,30 @@ function readChildLogs(logDir) { */ function matchChild(childLogs, parentFile) { const wanted = normalizePath(parentFile); + const isWin = process.platform === 'win32'; + const wantedNorm = isWin ? wanted.toLowerCase() : wanted; const entries = [...childLogs.entries()]; - const bySuffix = entries.filter(([key]) => key === wanted || key.endsWith(`/${wanted}`)); + const bySuffix = entries.filter(([key]) => { + const keyNorm = isWin ? key.toLowerCase() : key; + return keyNorm === wantedNorm || keyNorm.endsWith(`/${wantedNorm}`); + }); if (bySuffix.length === 1) return { child: bySuffix[0][1], ambiguous: false, candidates: 1 }; if (bySuffix.length > 1) return { child: null, ambiguous: true, candidates: bySuffix.length }; - const base = fileKey(wanted); - const byBase = entries.filter(([key]) => fileKey(key) === base); - if (byBase.length === 1) return { child: byBase[0][1], ambiguous: false, candidates: 1 }; - if (byBase.length > 1) return { child: null, ambiguous: true, candidates: byBase.length }; + // Basename fallback is ONLY for a parent path too short to disambiguate (i.e. bare filename with no slashes). + // When the parent specified a directory path and bySuffix found 0 matches, that file has no child log. + // Matching a different directory's child would be a false cross-directory attribution. + if (!wanted.includes('/')) { + const base = fileKey(wanted); + const baseNorm = isWin ? base.toLowerCase() : base; + const byBase = entries.filter(([key]) => { + const kBase = fileKey(key); + return isWin ? kBase.toLowerCase() === baseNorm : kBase === base; + }); + if (byBase.length === 1) return { child: byBase[0][1], ambiguous: false, candidates: 1 }; + if (byBase.length > 1) return { child: null, ambiguous: true, candidates: byBase.length }; + } return { child: null, ambiguous: false, candidates: 0 }; } diff --git a/packages/api/test/diagnostics/signature-b-correlate.test.ts b/packages/api/test/diagnostics/signature-b-correlate.test.ts index 06bde36b..7b898354 100644 --- a/packages/api/test/diagnostics/signature-b-correlate.test.ts +++ b/packages/api/test/diagnostics/signature-b-correlate.test.ts @@ -720,3 +720,111 @@ test('correlator: captures a real child termination end to end through the paren fs.rmSync(dir, { recursive: true, force: true }); } }); + +test('correlator: a missing child log from one directory is never falsely matched to a child in another directory', () => { + const logDir = fs.mkdtempSync(path.join(os.tmpdir(), 'sigb-nodup-')); + try { + // Only bar/a.test.js logged; foo/a.test.js died before writing any log + const lines = [ + { kind: 'start', pid: 222, testFile: 'C:\\r\\dist-test\\test\\bar\\a.test.js' }, + { kind: 'preload-installed', pid: 222, testFile: 'C:\\r\\dist-test\\test\\bar\\a.test.js' }, + { kind: 'exit', pid: 222, testFile: 'C:\\r\\dist-test\\test\\bar\\a.test.js' }, + ]; + fs.writeFileSync(path.join(logDir, 'run-single.jsonl'), `${lines.map((l) => JSON.stringify(l)).join('\n')}\n`); + const logs = correlator.readChildLogs(logDir); + + // Parent reports dist-test/test/foo/a.test.js failed. + const resolved = correlator.correlate( + correlator.parseTapFailures(tapFailure('dist-test/test/foo/a.test.js', '1')), + logs, + ); + assert.equal(resolved.length, 1); + assert.equal( + resolved[0]?.child.logFound, + false, + 'a file in foo/ must not claim the log of a different file in bar/ just because the basename matches', + ); + assert.equal(resolved[0]?.child.pid, null, 'no PID may be attributed to the unlogged file'); + assert.deepEqual(resolved[0]?.child.kinds, [], 'no lifecycle events may be falsely attributed'); + } finally { + fs.rmSync(logDir, { recursive: true, force: true }); + } +}); + +test('correlator: classifies signed negative 32-bit NTSTATUS codes identically to unsigned equivalents', () => { + // NTSTATUS 0xC0000005 in 32-bit signed two's complement is -1073741819 (unsigned 3221225477) + const signedViolation = correlator.classifyTermination({ exitCode: -1073741819, signal: null }); + assert.equal(signedViolation.id, 'native-access-violation'); + assert.equal(signedViolation.specific, true); + + // Unmeasured NTSTATUS 0xC0000022 in 32-bit signed is -1073741790 + const signedUnmeasured = correlator.classifyTermination({ exitCode: -1073741790, signal: null }); + assert.equal(signedUnmeasured.id, 'ntstatus-unmeasured'); + assert.equal(signedUnmeasured.specific, false); +}); + +test('correlator: parses TAP failure blocks with double-quoted or unquoted YAML values', () => { + const doubleQuoted = [ + '# Subtest: dist-test/test/foo.test.js', + 'not ok 1 - dist-test/test/foo.test.js', + ' ---', + ' duration_ms: 123.4', + ' failureType: "testCodeFailure"', + ' exitCode: 1', + ' signal: ~', + ' error: "test failed"', + ' code: "ERR_TEST_FAILURE"', + ' ...', + ].join('\n'); + + const unquoted = [ + '# Subtest: dist-test/test/bar.test.js', + 'not ok 2 - dist-test/test/bar.test.js', + ' ---', + ' duration_ms: 234.5', + ' failureType: testCodeFailure', + ' exitCode: 134', + ' signal: ~', + ' error: test failed', + ' code: ERR_TEST_FAILURE', + ' ...', + ].join('\n'); + + const dqFailures = correlator.parseTapFailures(doubleQuoted); + assert.equal(dqFailures.length, 1); + assert.equal(dqFailures[0]?.file, 'dist-test/test/foo.test.js'); + assert.equal(dqFailures[0]?.exitCode, 1); + assert.equal(dqFailures[0]?.failureType, 'testCodeFailure'); + + const uqFailures = correlator.parseTapFailures(unquoted); + assert.equal(uqFailures.length, 1); + assert.equal(uqFailures[0]?.file, 'dist-test/test/bar.test.js'); + assert.equal(uqFailures[0]?.exitCode, 134); + assert.equal(uqFailures[0]?.failureType, 'testCodeFailure'); +}); + +test('correlator: path suffix matching on Windows is case-insensitive', { + skip: process.platform === 'win32' ? false : 'Windows NTFS path casing test', +}, () => { + const logDir = fs.mkdtempSync(path.join(os.tmpdir(), 'sigb-case-')); + try { + const lines = [ + { kind: 'start', pid: 333, testFile: 'C:\\Repo\\Dist-Test\\Test\\Studies-Api.Test.js' }, + { kind: 'preload-installed', pid: 333, testFile: 'C:\\Repo\\Dist-Test\\Test\\Studies-Api.Test.js' }, + ]; + fs.writeFileSync(path.join(logDir, 'run-case.jsonl'), `${lines.map((l) => JSON.stringify(l)).join('\n')}\n`); + const logs = correlator.readChildLogs(logDir); + + // Parent uses lower-case path + const resolved = correlator.correlate( + correlator.parseTapFailures(tapFailure('dist-test/test/studies-api.test.js', '1')), + logs, + ); + assert.equal(resolved.length, 1); + assert.equal(resolved[0]?.child.pid, 333, 'casing difference must not prevent matching on Windows'); + assert.equal(resolved[0]?.child.logFound, true); + assert.equal(resolved[0]?.child.ambiguous, false); + } finally { + fs.rmSync(logDir, { recursive: true, force: true }); + } +}); From 99ccb41aae635f3aabde0157f70c2b2dabe4d864 Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Fri, 4 Sep 2026 10:27:34 +0300 Subject: [PATCH 02/13] fix(api-diagnostics): preserve Windows case-insensitivity on non-Windows analyzers --- .../diagnostics/signature-b-correlate.cjs | 30 +++++++++++++++---- .../diagnostics/signature-b-correlate.test.ts | 19 +++++++++--- 2 files changed, 39 insertions(+), 10 deletions(-) diff --git a/packages/api/test/diagnostics/signature-b-correlate.cjs b/packages/api/test/diagnostics/signature-b-correlate.cjs index 0e70ca3e..cadc6e58 100644 --- a/packages/api/test/diagnostics/signature-b-correlate.cjs +++ b/packages/api/test/diagnostics/signature-b-correlate.cjs @@ -62,6 +62,19 @@ function fileKey(filePath) { return segments[segments.length - 1] ?? ''; } +/** + * Detect whether a path string represents a Windows path, either because the host running the + * correlator is Windows, or because the path contains a Windows drive prefix (e.g. `C:/`) or UNC prefix (`//`). + * This ensures Windows-origin captures analyzed on non-Windows machines (e.g. CI on Linux) still apply + * Windows case-insensitive filesystem semantics during child log correlation. + * + * @param {string} filePath + * @returns {boolean} + */ +function isWindowsPath(filePath) { + return process.platform === 'win32' || /^[a-zA-Z]:(?:\/|$)/.test(filePath) || /^\/\/[^/]/.test(filePath); +} + /** * What an observed exit status *suggests*, keyed by the status the parent saw. * @@ -314,13 +327,15 @@ function readChildLogs(logDir) { */ function matchChild(childLogs, parentFile) { const wanted = normalizePath(parentFile); - const isWin = process.platform === 'win32'; - const wantedNorm = isWin ? wanted.toLowerCase() : wanted; const entries = [...childLogs.entries()]; const bySuffix = entries.filter(([key]) => { - const keyNorm = isWin ? key.toLowerCase() : key; - return keyNorm === wantedNorm || keyNorm.endsWith(`/${wantedNorm}`); + if (isWindowsPath(key) || isWindowsPath(wanted)) { + const keyNorm = key.toLowerCase(); + const wantedNorm = wanted.toLowerCase(); + return keyNorm === wantedNorm || keyNorm.endsWith(`/${wantedNorm}`); + } + return key === wanted || key.endsWith(`/${wanted}`); }); if (bySuffix.length === 1) return { child: bySuffix[0][1], ambiguous: false, candidates: 1 }; if (bySuffix.length > 1) return { child: null, ambiguous: true, candidates: bySuffix.length }; @@ -330,10 +345,12 @@ function matchChild(childLogs, parentFile) { // Matching a different directory's child would be a false cross-directory attribution. if (!wanted.includes('/')) { const base = fileKey(wanted); - const baseNorm = isWin ? base.toLowerCase() : base; const byBase = entries.filter(([key]) => { const kBase = fileKey(key); - return isWin ? kBase.toLowerCase() === baseNorm : kBase === base; + if (isWindowsPath(key) || isWindowsPath(wanted)) { + return kBase.toLowerCase() === base.toLowerCase(); + } + return kBase === base; }); if (byBase.length === 1) return { child: byBase[0][1], ambiguous: false, candidates: 1 }; if (byBase.length > 1) return { child: null, ambiguous: true, candidates: byBase.length }; @@ -394,6 +411,7 @@ module.exports = { MAX_RECORDS, classifyTermination, correlate, + isWindowsPath, matchChild, narrowFromChildEvidence, normalizePath, diff --git a/packages/api/test/diagnostics/signature-b-correlate.test.ts b/packages/api/test/diagnostics/signature-b-correlate.test.ts index 7b898354..a44a007d 100644 --- a/packages/api/test/diagnostics/signature-b-correlate.test.ts +++ b/packages/api/test/diagnostics/signature-b-correlate.test.ts @@ -62,6 +62,7 @@ const correlator = require(CORRELATE_PATH) as { readChildLogs(dir: string): Map; matchChild(logs: Map, file: string): { child: unknown; ambiguous: boolean; candidates: number }; normalizePath(p: string): string; + isWindowsPath(p: string): boolean; }; /** A TAP block in the exact shape Node's built-in reporter emits for a file-level failure. */ @@ -803,9 +804,7 @@ test('correlator: parses TAP failure blocks with double-quoted or unquoted YAML assert.equal(uqFailures[0]?.failureType, 'testCodeFailure'); }); -test('correlator: path suffix matching on Windows is case-insensitive', { - skip: process.platform === 'win32' ? false : 'Windows NTFS path casing test', -}, () => { +test('correlator: path suffix matching for Windows-origin paths is case-insensitive across platforms', () => { const logDir = fs.mkdtempSync(path.join(os.tmpdir(), 'sigb-case-')); try { const lines = [ @@ -821,10 +820,22 @@ test('correlator: path suffix matching on Windows is case-insensitive', { logs, ); assert.equal(resolved.length, 1); - assert.equal(resolved[0]?.child.pid, 333, 'casing difference must not prevent matching on Windows'); + assert.equal(resolved[0]?.child.pid, 333, 'casing difference must not prevent matching for Windows-origin paths'); assert.equal(resolved[0]?.child.logFound, true); assert.equal(resolved[0]?.child.ambiguous, false); } finally { fs.rmSync(logDir, { recursive: true, force: true }); } }); + +test('correlator: isWindowsPath identifies Windows drive letters and UNC paths', () => { + assert.equal(correlator.isWindowsPath('C:/repo/test.js'), true); + assert.equal(correlator.isWindowsPath('d:/repo/test.js'), true); + assert.equal(correlator.isWindowsPath('//server/share/test.js'), true); + if (process.platform !== 'win32') { + assert.equal(correlator.isWindowsPath('/home/runner/repo/test.js'), false); + assert.equal(correlator.isWindowsPath('dist-test/test.js'), false); + } else { + assert.equal(correlator.isWindowsPath('/any/path'), true); + } +}); From 0bb3d78a5231aedc483c187eb33470a1b4fc1fc7 Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Fri, 4 Sep 2026 10:38:40 +0300 Subject: [PATCH 03/13] fix(api-diagnostics): unquote exitCode and durationMs scalars before numeric conversion --- .../diagnostics/signature-b-correlate.cjs | 17 +++++++++------- .../diagnostics/signature-b-correlate.test.ts | 20 +++++++++++++++++++ 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/packages/api/test/diagnostics/signature-b-correlate.cjs b/packages/api/test/diagnostics/signature-b-correlate.cjs index cadc6e58..1dba8018 100644 --- a/packages/api/test/diagnostics/signature-b-correlate.cjs +++ b/packages/api/test/diagnostics/signature-b-correlate.cjs @@ -248,15 +248,18 @@ function parseTapFailures(tapText) { const found = new RegExp(`^\\s*${key}:\\s*(.+)$`, 'm').exec(body); return found ? found[1].trim() : null; }; - const exit = scalar('exitCode'); - const sig = scalar('signal'); - const dur = scalar('duration_ms'); + const unquote = (s) => (s === null || s === undefined ? null : s.replace(/^['"]|['"]$/g, '')); + const exit = unquote(scalar('exitCode')); + const sig = unquote(scalar('signal')); + const dur = unquote(scalar('duration_ms')); + const parsedExit = exit === null || exit === '~' || exit === '' ? null : Number(exit); + const parsedDur = dur === null || dur === '~' || dur === '' ? null : Number(dur); out.push({ file: rawName.trim().replace(/\\\\/g, '\\'), - exitCode: exit === null || exit === '~' ? null : Number(exit), - signal: sig === null || sig === '~' ? null : sig.replace(/^['"]|['"]$/g, ''), - failureType: scalar('failureType')?.replace(/^['"]|['"]$/g, '') ?? null, - durationMs: dur === null ? null : Number(dur), + exitCode: parsedExit !== null && Number.isNaN(parsedExit) ? null : parsedExit, + signal: sig === null || sig === '~' ? null : sig, + failureType: unquote(scalar('failureType')), + durationMs: parsedDur !== null && Number.isNaN(parsedDur) ? null : parsedDur, }); } return out; diff --git a/packages/api/test/diagnostics/signature-b-correlate.test.ts b/packages/api/test/diagnostics/signature-b-correlate.test.ts index a44a007d..bfcadfd0 100644 --- a/packages/api/test/diagnostics/signature-b-correlate.test.ts +++ b/packages/api/test/diagnostics/signature-b-correlate.test.ts @@ -802,6 +802,26 @@ test('correlator: parses TAP failure blocks with double-quoted or unquoted YAML assert.equal(uqFailures[0]?.file, 'dist-test/test/bar.test.js'); assert.equal(uqFailures[0]?.exitCode, 134); assert.equal(uqFailures[0]?.failureType, 'testCodeFailure'); + + const singleQuoted = [ + '# Subtest: dist-test/test/baz.test.js', + 'not ok 3 - dist-test/test/baz.test.js', + ' ---', + ' duration_ms: \'345.6\'', + ' failureType: \'testCodeFailure\'', + ' exitCode: \'1\'', + ' signal: ~', + ' error: \'test failed\'', + ' code: \'ERR_TEST_FAILURE\'', + ' ...', + ].join('\n'); + + const sqFailures = correlator.parseTapFailures(singleQuoted); + assert.equal(sqFailures.length, 1); + assert.equal(sqFailures[0]?.file, 'dist-test/test/baz.test.js'); + assert.equal(sqFailures[0]?.exitCode, 1); + assert.equal(sqFailures[0]?.durationMs, 345.6); + assert.equal(sqFailures[0]?.failureType, 'testCodeFailure'); }); test('correlator: path suffix matching for Windows-origin paths is case-insensitive across platforms', () => { From b6c923ff29c145a72c0625d4b47445b2c9959d26 Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Fri, 4 Sep 2026 22:45:24 +0300 Subject: [PATCH 04/13] docs: record M15 Increment 47 diagnostic hardening --- docs/PROJECT_STATE.md | 59 ++++++++++++++++++++++++++++++++++++++++++- docs/ROADMAP.md | 2 +- 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/docs/PROJECT_STATE.md b/docs/PROJECT_STATE.md index b5776790..87aa250e 100644 --- a/docs/PROJECT_STATE.md +++ b/docs/PROJECT_STATE.md @@ -4,7 +4,64 @@ > to read **only this file** and continue immediately. Updated after every > milestone and every significant architectural step. -_Last updated: 2026-09-04 — M15 Increment 46: reused-database idempotence in the persistence suite._ +_Last updated: 2026-09-04 — M15 Increment 47: Signature B diagnostic correlator hardening._ + + +## M15 Increment 47 — Signature B diagnostic correlator hardening + +**Status: Root cause UNPROVEN and UNRESOLVED.** This increment contributes diagnostic correlator +hardening only. No production or runtime code was modified, and no speculative fix was introduced. + +**Context and observed scope.** Signature B is an intermittent whole-file failure where Node's test +runner marks an entire test file as `'test failed'` with no assertion error, no stack trace, and no +tests within the file reporting. During M15 Increment 46 validation, Signature B was directly observed +three times in the persistence suite (`search-backfill.integration.test.ts`, +`learning.integration.test.ts`, and `test-database.integration.test.ts`). Each exhibited the exact +documented bare whole-file failure shape, and each passed standalone on immediate re-run. This +broadened the confirmed scope beyond `packages/api` into `packages/persistence`, demonstrating that +the failure is not confined to the HTTP API test harness or a single package. + +**Own bounded reproduction passes.** Bounded local reproduction passes were executed across 26 runs +total: +- 1 baseline full-suite run (clean pass); +- 20 sequential runs under `run-signature-b-pass.mjs` (3,084–3,834 MB free memory; 0 captures); +- 5 concurrent repository-native load runs under simultaneous test activity (0 captures). + +Zero captures across 26 bounded runs establishes an empirical upper bound under those conditions, but +**0 captures does NOT mean resolved**. Flaky failures with low frequency (~1-in-5 historically under +high machine load) naturally produce zero occurrences in small bounded samples without resolving the +underlying race or external cause. + +**Four proven correlator defects identified and hardened.** Analysis of the diagnostic harness +(`packages/api/test/diagnostics/signature-b-correlate.cjs`) revealed four concrete blind spots in how +parent TAP diagnostic logs and child JSONL preloads were matched and parsed: +1. **Cross-directory basename fallback collision:** `correlateFileLogs` fell back to matching on + `path.basename` whenever an exact normalized match was missing. If a target relative path contained + slashes (e.g. `test/diagnostics/sample.test.ts`), a child log from an unrelated directory with the + same basename could be falsely attributed. Hardened to require `!wanted.includes('/')` before + allowing basename fallback. +2. **Host-dependent Windows-origin path normalization:** Slashes were converted only when + `process.platform === 'win32'`. When logs recorded on Windows were analyzed on a POSIX CI runner or + host, backslashes remained un-normalized, causing path matching to fail. Hardened to detect + Windows-origin paths by drive letter or backslash pattern independently of the executing host OS. +3. **Signed 32-bit NTSTATUS exit code wrapping:** On Windows, crash exit codes such as `0xC0000005` + (access violation) or `STATUS_CONTROL_C_EXIT` can surface in Node/libuv or TAP as negative 32-bit + integers (e.g. `-1073741819`). Hardened exit code parsing to normalize negative 32-bit values via + unsigned right shift `(exitCode >>> 0)`, correctly recovering the standard `0xC0000005` representation. +4. **Quoted TAP YAML scalar parsing:** Node's TAP reporter emits single- or double-quoted scalars + around certain YAML values (e.g. `exitCode: '1'` or duration strings). Strict numeric conversion + previously yielded `NaN` or dropped exit codes. Hardened YAML extraction to unquote scalar tokens + prior to `Number(...)` conversion. + +**Verification and mutation falsification.** +- Targeted correlator suite (`signature-b-correlate.test.ts`): expanded from 23 to 31 tests + (29 pass, 2 skip for deliberate platform-gated checks, 0 fail). +- Mutation falsification: four targeted mutations reversing each of the four hardened behaviors were + verified to be killed by the expanded test suite. +- Monorepo validation: full build, lint, counts, and guard scripts pass cleanly. + +**Signature B remains UNRESOLVED.** The harness is hardened so that when the next occurrence happens, +the parent-to-child log correlation is robust across operating systems and crash code representations. ## M15 Increment 46 — the persistence suite is idempotent on a reused database diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 555326ac..9068571b 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -1349,7 +1349,7 @@ Debt observed during M14. Each states what is known, not what is planned; items - **The published capability document could omit a composed feature, and the client offered controls that could not work (RESOLVED in M15 Increment 24 / ADR-0132).** Left open by Increment 23 above. `capabilitiesView` (`packages/api/src/presenters.ts`) took a hand-written `Pick`, so a feature never added to it was invisible to `GET /v1/capabilities` and nothing complained. ADR-0131 judged that "a narrower failure than a 503 — the feature works for anyone who calls the route directly", and deferred it on the grounds that closing it meant first deciding which optional dependencies are user-facing capabilities. **Both halves of that were wrong.** The failure is narrower only when the client can still reach the feature, and a live instance already existed where it could not: `GET /v1/search` served three modes from two independently-gated dependency sets behind one published `search` flag, so a deployment running the Helm chart’s `search.semanticEnabled: false` advertised search while the client offered two mode buttons whose every request answered 503. And the judgement, while genuinely underivable, can be made **unskippable**, which was the property wanted. `semanticSearch` is now published from both of its dependencies, `Exclude[0] | NotAPublishedCapability>` must be `never` so a new optional dependency cannot compile until someone gives it a flag or records why it has none, and a behavioural guard requires every capability source to change what the document publishes — because a key can sit in that parameter unread, which the compile-time half cannot see. The mutation ledger lives in ADR-0132 and is not restated here. - **The search surface was not gated on `capabilities.search`, so an absolute kill switch still showed a search box (RESOLVED in M15 Increment 24 / ADR-0132 §5).** `SEARCH_ENABLED=0` — the chart's `search.enabled: false`, an absolute kill switch per ADR-0055 — leaves `searchRepository` unconstructed and `GET /v1/search` answering 503 on every mode, keyword included. The entry point was the persistent header form in `packages/web/index.html`, present on every page; being a `
` rather than an `a[data-route]`, `NAV_CAPABILITY_MAP` could not reach it, which is why the first pass at Increment 24 gated the semantic and hybrid *modes* and left keyword ungated — the same defect class one mode over. Raised by the Qodo review of PR #155. **Fixed in the same increment rather than deferred:** the form now ships `hidden` and is revealed by `applySearchCapability` only on an explicit `search: true`; the route renders an honest unavailable notice and issues no request; and keyword search waits for the capability answer, reversing this increment's own earlier latency decision, because knowing whether a request is pointless requires having asked. A markup-contract test pins the `hidden` attribute, since the gate depends on it and every other test passes without it. - **Clicking a search mode discarded text typed since the page loaded (RESOLVED in a follow-up to M15 Increment 24 / ADR-0132).** `createModeInput` in `packages/web/src/app/search-mount.ts` closed over the query captured when the route mounted, so `navigateToSearchMode` navigated with the old term and the remount reset the input to match it. Type a new term into the header field, click **Semantic** without pressing enter, and the typed text was gone with no indication it had been discarded. Pre-existing — the closure predated Increment 24 and was untouched by it — and found by the adversarial review of PR #155 while reviewing the capability gate wrapped around the same control. **Resolved:** the query is now a `() => string` read when a mode is chosen rather than a string captured when the selector renders, matching what `main.ts`'s submit handler already does, and falling back to the mounted query only where the document has no header input. Two regression tests, one of which fails against the exact pre-fix closure. -- **`startHarness` drew ports `fetch` refuses (RESOLVED in ADR-0140); the unexplained whole-file failure is still open.** Two signatures were filed here, deliberately not as one cause, and that judgement held. **Signature A is resolved.** WHATWG Fetch blocks eighty-two ports and undici enforces the list on the port number alone, before opening a socket — so `server.listen(0)` could bind, listen and answer raw TCP while `fetch` still refused, surfacing as `TypeError: fetch failed` / `Error: bad port` at the harness’s first request rather than at the listen that caused it. Whether it can happen at all is a property of the host’s dynamic port range: a typical Linux CI range (32768–60999) contains no blocked port, while the Windows range in use here (1024–15000) contains nineteen, which is the whole of "green on CI, flaky locally". The guard that already existed was incomplete in a way that still failed — its hand-observed set of eighteen ports was the spec list intersected with one machine’s range **minus `6679`** — and it retried unboundedly, had no behaviour on exhaustion, and had been copy-pasted into `auth-signin-schema.integration.test.ts`, so the missing port had to be found twice. `packages/api/test/listen.ts` now owns port acquisition for both sites: the spec-complete eighty-two ports (verified by sweeping all 65535 through the real `fetch` on Node v24.15.0), a bounded twenty attempts, each rejected listener closed before the next is asked for, a guarded `address()` read in place of the `as AddressInfo` cast, and an exhaustion error naming the attempts and rejected ports and nothing else. **Signature B is not resolved and was not folded in.** A file still fails with `'test failed'`, no assertion, no stack and none of its own tests reported. Twenty consecutive full runs gave five failures: on pre-fix code one signature A (`auth.test.js`) and three signature B; on post-fix code one signature B and no signature A. The port fix removes A and leaves B exactly where it was, which is the evidence that they are two defects. Four different files were hit (`move-explanation-route`, `tournament-commentary-route`, `bot-detection-analyze`, `anti-cheat-analysis`), sharing no import beyond `./helpers`; each died in 589–703 ms with no test of its own reporting and no stderr. Refuted with evidence: ephemeral-port exhaustion (113 sockets in TIME_WAIT against a 13977-port range), a `Promise.race` loser becoming an unhandled rejection (`race` subscribes to every promise, confirmed on v24.15.0), a throwing `after`/`afterEach` hook (the affected files use none), and a double `close()` rejecting (awaited in a `finally`, it would be attributed to that test with a stack). A second bounded pass — twelve more full runs under the TAP reporter with a preload recording `uncaughtException`, `unhandledRejection` and any non-zero exit — produced twelve clean runs and captured nothing at the time. **A follow-up increment (`claude/node-test-signature-b`) then captured the defect directly, three more times, on three files never previously implicated** (`rate-limit-atomicity`, `dependency-parity`, `studies-api`) — seven distinct files observed with this symptom to date. Occurrences across seven distinct files make a shared or cross-cutting path more plausible and make a defect confined to one test file less likely, but do not exclude file-specific inputs or lifecycle interactions. An instrumented preload (`packages/api/test/diagnostics/signature-b-preload.cjs`) hooking process-level events — `process.exit`, `process.abort`, `process.kill`, `uncaughtExceptionMonitor` (passively observing uncaught exceptions and fatal unhandled rejections), `warning`, `beforeExit`, and Node’s own unconditional `exit` — showed **none of the hooks active at the time fired** on any of the three historical captures (though `process.abort()` was not wrapped in those initial runs and is now covered for future occurrences). A synthetic `process.exit(1)`-before-registration fixture reproduces the identical silent shape; every other synthetic mechanism tried (a post-test async throw, an emitter `'error'` with its listener removed, a synchronous module-load throw, a delayed `SIGKILL`) prints a visibly different diagnostic line, stack, or partial test output that the real defect never shows. This narrows the investigated possibilities while leaving the root cause unresolved: the per-file child process (`node --test` spawns one per file, confirmed by distinct PIDs) was not terminated by `process.exit`, uncaught exceptions, or fatal unhandled rejections, and future runs with `process.abort` instrumentation will record whether abort was called through JS; an absent record narrows in-runtime JS termination but cannot alone prove external termination without corroborating child exit status/signal data or OS-level crash evidence (e.g. distinguishing an external kill or uncatchable signal from a native C++/V8 crash). The machine had roughly 2.5 GB of 15.7 GB RAM free at capture time with several other agents’ processes concurrently running, which is circumstantially consistent with resource contention, but no crash was recorded in the Windows Application or System event logs in that window, so the exact external trigger is still not established. No fix was invented — the forbidden responses (sleeps, whole-file retries, lowering concurrency) would only hide the unresolved root cause, whose origin is not yet established. **A further increment then crossed the parent/child boundary the earlier work stopped at, and found the evidence had been there all along:** Node's runner attaches the child's `exitCode` and `signal` to the `ERR_TEST_FAILURE` it throws, and the `spec` reporter discards them — `formatError` replaces the error with `error.cause`, the bare string `'test failed'` — while the built-in `tap` reporter serializes them, so running `spec` to stdout and `tap` to a file recovers the exit status with no custom reporter and no patched internals. Exit codes were measured on this platform rather than assumed: `process.abort()` gives `134`, `Stop-Process -Force` gives `4294967295`, NTSTATUS faults surface as raw unsigned values such as `3221225477` (`0xC0000005`) — and `1` is produced alike by an uncaught exception, `process.exit(1)`, `taskkill /F` and `process.kill`, so it identifies nothing on its own and is classified `inconclusive`. `signature-b-correlate.cjs` joins the parent's TAP record to the child's JSONL log on the test file path (which also yields the child PID) and states what the pair does and does not establish; where the exit code is ambiguous, a child that reached `preload-installed` and then logged nothing still excludes `process.exit` and an uncaught exception, because both would have left a record and fired Node's `exit` event. A bounded pass of 20 runs under this instrumentation produced 0 captures — which bounds the rate and proves nothing: treating the historical ~1-in-5 as an independent per-run rate, zero captures in 20 runs has probability `(4/5)^20 ≈ 1.2%`, and independence is an assumption rather than an established fact; it ran at 3084–3834 MB free against roughly 2.5 GB at the historical captures, consistent with the resource-contention hypothesis but not evidence for it. **Signature B stays UNRESOLVED**; what changed is that the next occurrence is readable rather than silent. See ADR-0140 §4. +- **`startHarness` drew ports `fetch` refuses (RESOLVED in ADR-0140); the unexplained whole-file failure is still open.** Two signatures were filed here, deliberately not as one cause, and that judgement held. **Signature A is resolved.** WHATWG Fetch blocks eighty-two ports and undici enforces the list on the port number alone, before opening a socket — so `server.listen(0)` could bind, listen and answer raw TCP while `fetch` still refused, surfacing as `TypeError: fetch failed` / `Error: bad port` at the harness’s first request rather than at the listen that caused it. Whether it can happen at all is a property of the host’s dynamic port range: a typical Linux CI range (32768–60999) contains no blocked port, while the Windows range in use here (1024–15000) contains nineteen, which is the whole of "green on CI, flaky locally". The guard that already existed was incomplete in a way that still failed — its hand-observed set of eighteen ports was the spec list intersected with one machine’s range **minus `6679`** — and it retried unboundedly, had no behaviour on exhaustion, and had been copy-pasted into `auth-signin-schema.integration.test.ts`, so the missing port had to be found twice. `packages/api/test/listen.ts` now owns port acquisition for both sites: the spec-complete eighty-two ports (verified by sweeping all 65535 through the real `fetch` on Node v24.15.0), a bounded twenty attempts, each rejected listener closed before the next is asked for, a guarded `address()` read in place of the `as AddressInfo` cast, and an exhaustion error naming the attempts and rejected ports and nothing else. **Signature B is not resolved and was not folded in.** A file still fails with `'test failed'`, no assertion, no stack and none of its own tests reported. Twenty consecutive full runs gave five failures: on pre-fix code one signature A (`auth.test.js`) and three signature B; on post-fix code one signature B and no signature A. The port fix removes A and leaves B exactly where it was, which is the evidence that they are two defects. Four different files were hit (`move-explanation-route`, `tournament-commentary-route`, `bot-detection-analyze`, `anti-cheat-analysis`), sharing no import beyond `./helpers`; each died in 589–703 ms with no test of its own reporting and no stderr. Refuted with evidence: ephemeral-port exhaustion (113 sockets in TIME_WAIT against a 13977-port range), a `Promise.race` loser becoming an unhandled rejection (`race` subscribes to every promise, confirmed on v24.15.0), a throwing `after`/`afterEach` hook (the affected files use none), and a double `close()` rejecting (awaited in a `finally`, it would be attributed to that test with a stack). A second bounded pass — twelve more full runs under the TAP reporter with a preload recording `uncaughtException`, `unhandledRejection` and any non-zero exit — produced twelve clean runs and captured nothing at the time. **A follow-up increment (`claude/node-test-signature-b`) then captured the defect directly, three more times, on three files never previously implicated** (`rate-limit-atomicity`, `dependency-parity`, `studies-api`) — seven distinct files observed with this symptom to date. Occurrences across seven distinct files make a shared or cross-cutting path more plausible and make a defect confined to one test file less likely, but do not exclude file-specific inputs or lifecycle interactions. An instrumented preload (`packages/api/test/diagnostics/signature-b-preload.cjs`) hooking process-level events — `process.exit`, `process.abort`, `process.kill`, `uncaughtExceptionMonitor` (passively observing uncaught exceptions and fatal unhandled rejections), `warning`, `beforeExit`, and Node’s own unconditional `exit` — showed **none of the hooks active at the time fired** on any of the three historical captures (though `process.abort()` was not wrapped in those initial runs and is now covered for future occurrences). A synthetic `process.exit(1)`-before-registration fixture reproduces the identical silent shape; every other synthetic mechanism tried (a post-test async throw, an emitter `'error'` with its listener removed, a synchronous module-load throw, a delayed `SIGKILL`) prints a visibly different diagnostic line, stack, or partial test output that the real defect never shows. This narrows the investigated possibilities while leaving the root cause unresolved: the per-file child process (`node --test` spawns one per file, confirmed by distinct PIDs) was not terminated by `process.exit`, uncaught exceptions, or fatal unhandled rejections, and future runs with `process.abort` instrumentation will record whether abort was called through JS; an absent record narrows in-runtime JS termination but cannot alone prove external termination without corroborating child exit status/signal data or OS-level crash evidence (e.g. distinguishing an external kill or uncatchable signal from a native C++/V8 crash). The machine had roughly 2.5 GB of 15.7 GB RAM free at capture time with several other agents’ processes concurrently running, which is circumstantially consistent with resource contention, but no crash was recorded in the Windows Application or System event logs in that window, so the exact external trigger is still not established. No fix was invented — the forbidden responses (sleeps, whole-file retries, lowering concurrency) would only hide the unresolved root cause, whose origin is not yet established. **A further increment then crossed the parent/child boundary the earlier work stopped at, and found the evidence had been there all along:** Node's runner attaches the child's `exitCode` and `signal` to the `ERR_TEST_FAILURE` it throws, and the `spec` reporter discards them — `formatError` replaces the error with `error.cause`, the bare string `'test failed'` — while the built-in `tap` reporter serializes them, so running `spec` to stdout and `tap` to a file recovers the exit status with no custom reporter and no patched internals. Exit codes were measured on this platform rather than assumed: `process.abort()` gives `134`, `Stop-Process -Force` gives `4294967295`, NTSTATUS faults surface as raw unsigned values such as `3221225477` (`0xC0000005`) — and `1` is produced alike by an uncaught exception, `process.exit(1)`, `taskkill /F` and `process.kill`, so it identifies nothing on its own and is classified `inconclusive`. `signature-b-correlate.cjs` joins the parent's TAP record to the child's JSONL log on the test file path (which also yields the child PID) and states what the pair does and does not establish; where the exit code is ambiguous, a child that reached `preload-installed` and then logged nothing still excludes `process.exit` and an uncaught exception, because both would have left a record and fired Node's `exit` event. A bounded pass of 20 runs under this instrumentation produced 0 captures — which bounds the rate and proves nothing: treating the historical ~1-in-5 as an independent per-run rate, zero captures in 20 runs has probability `(4/5)^20 ≈ 1.2%`, and independence is an assumption rather than an established fact; it ran at 3084–3834 MB free against roughly 2.5 GB at the historical captures, consistent with the resource-contention hypothesis but not evidence for it. **Signature B stays UNRESOLVED**; what changed is that the next occurrence is readable rather than silent. **During M15 Increment 46 validation, Signature B was directly observed three more times, broadening the observed scope to `packages/persistence`:** `search-backfill.integration.test.ts`, `learning.integration.test.ts`, and `test-database.integration.test.ts` each died with the documented bare whole-file `'test failed'` with zero tests reporting and no assertion or stack, and each passed standalone against the exact database state it died on. This confirms the defect is not specific to `packages/api` or its HTTP server test harness. **M15 Increment 47 hardened the diagnostic correlator across 26 bounded runs:** 1 baseline full-suite run, 20 sequential runs under `run-signature-b-pass.mjs`, and 5 concurrent repository-native load runs produced 0 captures (which bounds the rate under those conditions and does not resolve the defect). Code inspection and falsification identified and fixed four correlator blind spots in `packages/api/test/diagnostics/signature-b-correlate.cjs`: (1) cross-directory basename fallback could falsely match a child log from another directory when the target had slashes; (2) Windows-origin path normalization was host-dependent (`process.platform === 'win32'`), breaking correlation on POSIX hosts; (3) signed 32-bit Windows NTSTATUS exit codes (e.g. `0xC0000005` surfacing as `-1073741819`) were not normalized to unsigned values; and (4) quoted TAP YAML scalar tokens lost exit code or duration numbers. The targeted test suite was expanded from 23 to 31 tests (29 pass, 2 skip, 0 fail), killing 4 falsification mutations. **Signature B remains UNRESOLVED;** no production fix was invented, and the next occurrence will be correlated without cross-platform or exit-code blind spots. See ADR-0140 §4. - **Isolated-database test teardown dropped databases out from under connections that had not finished closing (RESOLVED in M15 Increment 45).** `withDatabase` in `packages/persistence/test/variant-migrations.integration.test.ts` ended its pool and then immediately ran `DROP DATABASE ... WITH (FORCE)`. `pool.end()` does not wait for its clients to close: in pg 8.22.0 `_pulseQueue` reaches the end callback in the same synchronous turn in which `_remove` filters the last client out of `_clients`, while `client.end()` has only queued the Terminate byte — instrumentation recorded **zero of four `remove` events fired at the moment `end()` resolved**. The drop could therefore still find a backend attached; `FORCE` terminated it, and the resulting `FATAL` arrived on a socket whose pool still had `idleListener` attached, which `pg` re-emitted as `pool.emit('error')` — an unhandled EventEmitter error that `node:test` attributed to whichever test was running rather than to the teardown that caused it. It surfaced as intermittent `terminating connection due to administrator command` failures in `postgres integration (persistence)` during M15 Increment 44, on a different test each run, which is the signature of a race rather than a broken assertion. The same shape existed in `packages/api/test/auth-signin-schema.integration.test.ts`, which had absorbed SQLSTATE 57P01 with a `pool.on('error', ...)` listener — a symptom fix for the same cause. **Resolved in Increment 45:** a shared `withTestDatabase` helper (`@chess-platform/persistence/test-support`) ends the pool under a bound, waits for `pg_stat_activity` to report the database unused, and drops it *without* `FORCE`. Measured on PostgreSQL 16.14, a plain drop against a still-attached backend fails with SQLSTATE 55006 and leaves that connection untouched, where `FORCE` succeeds by killing it — so the change trades a quiet, harmful success for a loud, harmless failure. FORCE remains only on the emergency path that guarantees the disposable database is still dropped once teardown has already failed — best effort, since that last drop runs inside a `catch` so it cannot bury the error being reported. The 57P01 absorber is deleted, because the corrected lifecycle never causes one. `createPool` and `migrate` are unchanged, and no migration was added. - **The persistence integration suite was not idempotent against a reused database (RESOLVED in M15 Increment 46).** Recorded as a known defect by Increment 45 and left open there. Against a fresh PostgreSQL 16 database the suite passed; a second run against the *same* database failed nine tests, deterministically — measured on 16.14 before any edit as **173 pass / 0 fail** then **164 pass / 9 fail**. CI provisions a fresh server per run, so it never surfaced there. One contract was being broken in two directions: a suite sharing `chess_test` must remove every row it created and remove nothing else. `achievements.integration.test.ts` broke the second half with an unqualified `DELETE FROM users` in `beforeEach` — `games.white_id` and `games.black_id` are the only references to `users` without `ON DELETE CASCADE` (thirty-one FKs point at `users`; twenty-nine cascade; the two that do not are both on `games`), so one game left behind by `pg.integration.test.ts` aborted the wipe with SQLSTATE 23503 before any assertion ran, and the same statement destroyed the bot accounts migration 0021 seeds, which nothing restores because `migrate` has already recorded 0021 as applied. `pg/identity-tokens.test.ts` and `tournaments.pg.integration.test.ts` broke the first half, leaving fixed primary keys behind and colliding on `users_pkey` and `tournaments_pkey` (the latter surfacing through the repository's compare-and-set as `VersionConflictError`). **Resolved in Increment 46:** suites that legitimately share the database delete exactly their own rows through `withSharedDatabase` (`packages/persistence/src/test-support/fixtures.ts`), the sibling of Increment 45's `withTestDatabase` and heir to its precedence rule — a cleanup failure never replaces the assertion that actually failed, and never disappears either. `pg.integration.test.ts` moved to disposable databases instead, because it cannot meet the cleanup half at all: it appends to `game_events`, which is append-only by production trigger, so cleaning up after itself would have meant weakening a production safety rule to suit a test. `users-batch`, `anti-cheat`, `bot-reports` and `analysis-cache` were corrected for the same contract though none of them ever failed — fresh `uuidv7()` ids meant their leaked rows could not collide, so the tables merely grew on every run. Serialization was not the fix and was not introduced: `--test-concurrency=1` is a pre-existing documented invariant and the second run failed identically under it. Acceptance is three consecutive runs against one database with no reset between them (186/186/186), after which only the three migration-seeded bot accounts remain, with no leaked disposable databases and no lingering backends; falsification killed 17 of 20 mutations. No production code, migration, checksum, constraint or repository conflict semantic changed, and no migration was added. - **`ApiServer.listen` registered no `'error'` handler, so a failed bind hung and raised an uncaught event (RESOLVED in ADR-0140 §5).** `packages/api/src/server.ts` resolved its promise from the `listening` callback only and built it with no reject path. A bind failing asynchronously (`EADDRINUSE`, `EMFILE`) left the promise pending forever and, with no `'error'` listener on the `http.Server`, was re-raised as an uncaught exception. Found while investigating ADR-0140 and independently raised by the Qodo review of PR #21. **Resolved in the same increment** rather than deferred, because ADR-0140 §2's bounded, diagnosable acquisition is not true without it — the retry can only report a bind error if the listener it is handed rejects. A one-shot `'error'` listener now rejects and is removed once listening, so later server errors keep their previous semantics rather than being swallowed by a `reject` on a settled promise. The regression test fails against the exact pre-fix code, through an uncaught `ERR_UNHANDLED_ERROR`. From 326a44d19d7c4f93a3915d0129835ae15c3ce9b7 Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Fri, 4 Sep 2026 22:51:57 +0300 Subject: [PATCH 05/13] docs: clarify Increment 47 Windows path case-folding defect --- docs/PROJECT_STATE.md | 10 ++++++---- docs/ROADMAP.md | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/docs/PROJECT_STATE.md b/docs/PROJECT_STATE.md index 87aa250e..1178639c 100644 --- a/docs/PROJECT_STATE.md +++ b/docs/PROJECT_STATE.md @@ -40,10 +40,12 @@ parent TAP diagnostic logs and child JSONL preloads were matched and parsed: slashes (e.g. `test/diagnostics/sample.test.ts`), a child log from an unrelated directory with the same basename could be falsely attributed. Hardened to require `!wanted.includes('/')` before allowing basename fallback. -2. **Host-dependent Windows-origin path normalization:** Slashes were converted only when - `process.platform === 'win32'`. When logs recorded on Windows were analyzed on a POSIX CI runner or - host, backslashes remained un-normalized, causing path matching to fail. Hardened to detect - Windows-origin paths by drive letter or backslash pattern independently of the executing host OS. +2. **Host-dependent Windows path case folding:** While backslash-to-slash normalization was already + platform-independent, case folding in `matchChild` was gated strictly on `process.platform === 'win32'`. + When logs recorded on Windows were analyzed on a POSIX host (Linux or macOS), comparisons remained + case-sensitive, causing path matching to fail when runner TAP paths and child `process.argv[1]` paths + differed in casing. Hardened `isWindowsPath` to detect Windows-origin paths by drive letter or UNC prefix + independently of the executing host OS, ensuring case-insensitive matching across platforms. 3. **Signed 32-bit NTSTATUS exit code wrapping:** On Windows, crash exit codes such as `0xC0000005` (access violation) or `STATUS_CONTROL_C_EXIT` can surface in Node/libuv or TAP as negative 32-bit integers (e.g. `-1073741819`). Hardened exit code parsing to normalize negative 32-bit values via diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 9068571b..7542bfb8 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -1349,7 +1349,7 @@ Debt observed during M14. Each states what is known, not what is planned; items - **The published capability document could omit a composed feature, and the client offered controls that could not work (RESOLVED in M15 Increment 24 / ADR-0132).** Left open by Increment 23 above. `capabilitiesView` (`packages/api/src/presenters.ts`) took a hand-written `Pick`, so a feature never added to it was invisible to `GET /v1/capabilities` and nothing complained. ADR-0131 judged that "a narrower failure than a 503 — the feature works for anyone who calls the route directly", and deferred it on the grounds that closing it meant first deciding which optional dependencies are user-facing capabilities. **Both halves of that were wrong.** The failure is narrower only when the client can still reach the feature, and a live instance already existed where it could not: `GET /v1/search` served three modes from two independently-gated dependency sets behind one published `search` flag, so a deployment running the Helm chart’s `search.semanticEnabled: false` advertised search while the client offered two mode buttons whose every request answered 503. And the judgement, while genuinely underivable, can be made **unskippable**, which was the property wanted. `semanticSearch` is now published from both of its dependencies, `Exclude[0] | NotAPublishedCapability>` must be `never` so a new optional dependency cannot compile until someone gives it a flag or records why it has none, and a behavioural guard requires every capability source to change what the document publishes — because a key can sit in that parameter unread, which the compile-time half cannot see. The mutation ledger lives in ADR-0132 and is not restated here. - **The search surface was not gated on `capabilities.search`, so an absolute kill switch still showed a search box (RESOLVED in M15 Increment 24 / ADR-0132 §5).** `SEARCH_ENABLED=0` — the chart's `search.enabled: false`, an absolute kill switch per ADR-0055 — leaves `searchRepository` unconstructed and `GET /v1/search` answering 503 on every mode, keyword included. The entry point was the persistent header form in `packages/web/index.html`, present on every page; being a `` rather than an `a[data-route]`, `NAV_CAPABILITY_MAP` could not reach it, which is why the first pass at Increment 24 gated the semantic and hybrid *modes* and left keyword ungated — the same defect class one mode over. Raised by the Qodo review of PR #155. **Fixed in the same increment rather than deferred:** the form now ships `hidden` and is revealed by `applySearchCapability` only on an explicit `search: true`; the route renders an honest unavailable notice and issues no request; and keyword search waits for the capability answer, reversing this increment's own earlier latency decision, because knowing whether a request is pointless requires having asked. A markup-contract test pins the `hidden` attribute, since the gate depends on it and every other test passes without it. - **Clicking a search mode discarded text typed since the page loaded (RESOLVED in a follow-up to M15 Increment 24 / ADR-0132).** `createModeInput` in `packages/web/src/app/search-mount.ts` closed over the query captured when the route mounted, so `navigateToSearchMode` navigated with the old term and the remount reset the input to match it. Type a new term into the header field, click **Semantic** without pressing enter, and the typed text was gone with no indication it had been discarded. Pre-existing — the closure predated Increment 24 and was untouched by it — and found by the adversarial review of PR #155 while reviewing the capability gate wrapped around the same control. **Resolved:** the query is now a `() => string` read when a mode is chosen rather than a string captured when the selector renders, matching what `main.ts`'s submit handler already does, and falling back to the mounted query only where the document has no header input. Two regression tests, one of which fails against the exact pre-fix closure. -- **`startHarness` drew ports `fetch` refuses (RESOLVED in ADR-0140); the unexplained whole-file failure is still open.** Two signatures were filed here, deliberately not as one cause, and that judgement held. **Signature A is resolved.** WHATWG Fetch blocks eighty-two ports and undici enforces the list on the port number alone, before opening a socket — so `server.listen(0)` could bind, listen and answer raw TCP while `fetch` still refused, surfacing as `TypeError: fetch failed` / `Error: bad port` at the harness’s first request rather than at the listen that caused it. Whether it can happen at all is a property of the host’s dynamic port range: a typical Linux CI range (32768–60999) contains no blocked port, while the Windows range in use here (1024–15000) contains nineteen, which is the whole of "green on CI, flaky locally". The guard that already existed was incomplete in a way that still failed — its hand-observed set of eighteen ports was the spec list intersected with one machine’s range **minus `6679`** — and it retried unboundedly, had no behaviour on exhaustion, and had been copy-pasted into `auth-signin-schema.integration.test.ts`, so the missing port had to be found twice. `packages/api/test/listen.ts` now owns port acquisition for both sites: the spec-complete eighty-two ports (verified by sweeping all 65535 through the real `fetch` on Node v24.15.0), a bounded twenty attempts, each rejected listener closed before the next is asked for, a guarded `address()` read in place of the `as AddressInfo` cast, and an exhaustion error naming the attempts and rejected ports and nothing else. **Signature B is not resolved and was not folded in.** A file still fails with `'test failed'`, no assertion, no stack and none of its own tests reported. Twenty consecutive full runs gave five failures: on pre-fix code one signature A (`auth.test.js`) and three signature B; on post-fix code one signature B and no signature A. The port fix removes A and leaves B exactly where it was, which is the evidence that they are two defects. Four different files were hit (`move-explanation-route`, `tournament-commentary-route`, `bot-detection-analyze`, `anti-cheat-analysis`), sharing no import beyond `./helpers`; each died in 589–703 ms with no test of its own reporting and no stderr. Refuted with evidence: ephemeral-port exhaustion (113 sockets in TIME_WAIT against a 13977-port range), a `Promise.race` loser becoming an unhandled rejection (`race` subscribes to every promise, confirmed on v24.15.0), a throwing `after`/`afterEach` hook (the affected files use none), and a double `close()` rejecting (awaited in a `finally`, it would be attributed to that test with a stack). A second bounded pass — twelve more full runs under the TAP reporter with a preload recording `uncaughtException`, `unhandledRejection` and any non-zero exit — produced twelve clean runs and captured nothing at the time. **A follow-up increment (`claude/node-test-signature-b`) then captured the defect directly, three more times, on three files never previously implicated** (`rate-limit-atomicity`, `dependency-parity`, `studies-api`) — seven distinct files observed with this symptom to date. Occurrences across seven distinct files make a shared or cross-cutting path more plausible and make a defect confined to one test file less likely, but do not exclude file-specific inputs or lifecycle interactions. An instrumented preload (`packages/api/test/diagnostics/signature-b-preload.cjs`) hooking process-level events — `process.exit`, `process.abort`, `process.kill`, `uncaughtExceptionMonitor` (passively observing uncaught exceptions and fatal unhandled rejections), `warning`, `beforeExit`, and Node’s own unconditional `exit` — showed **none of the hooks active at the time fired** on any of the three historical captures (though `process.abort()` was not wrapped in those initial runs and is now covered for future occurrences). A synthetic `process.exit(1)`-before-registration fixture reproduces the identical silent shape; every other synthetic mechanism tried (a post-test async throw, an emitter `'error'` with its listener removed, a synchronous module-load throw, a delayed `SIGKILL`) prints a visibly different diagnostic line, stack, or partial test output that the real defect never shows. This narrows the investigated possibilities while leaving the root cause unresolved: the per-file child process (`node --test` spawns one per file, confirmed by distinct PIDs) was not terminated by `process.exit`, uncaught exceptions, or fatal unhandled rejections, and future runs with `process.abort` instrumentation will record whether abort was called through JS; an absent record narrows in-runtime JS termination but cannot alone prove external termination without corroborating child exit status/signal data or OS-level crash evidence (e.g. distinguishing an external kill or uncatchable signal from a native C++/V8 crash). The machine had roughly 2.5 GB of 15.7 GB RAM free at capture time with several other agents’ processes concurrently running, which is circumstantially consistent with resource contention, but no crash was recorded in the Windows Application or System event logs in that window, so the exact external trigger is still not established. No fix was invented — the forbidden responses (sleeps, whole-file retries, lowering concurrency) would only hide the unresolved root cause, whose origin is not yet established. **A further increment then crossed the parent/child boundary the earlier work stopped at, and found the evidence had been there all along:** Node's runner attaches the child's `exitCode` and `signal` to the `ERR_TEST_FAILURE` it throws, and the `spec` reporter discards them — `formatError` replaces the error with `error.cause`, the bare string `'test failed'` — while the built-in `tap` reporter serializes them, so running `spec` to stdout and `tap` to a file recovers the exit status with no custom reporter and no patched internals. Exit codes were measured on this platform rather than assumed: `process.abort()` gives `134`, `Stop-Process -Force` gives `4294967295`, NTSTATUS faults surface as raw unsigned values such as `3221225477` (`0xC0000005`) — and `1` is produced alike by an uncaught exception, `process.exit(1)`, `taskkill /F` and `process.kill`, so it identifies nothing on its own and is classified `inconclusive`. `signature-b-correlate.cjs` joins the parent's TAP record to the child's JSONL log on the test file path (which also yields the child PID) and states what the pair does and does not establish; where the exit code is ambiguous, a child that reached `preload-installed` and then logged nothing still excludes `process.exit` and an uncaught exception, because both would have left a record and fired Node's `exit` event. A bounded pass of 20 runs under this instrumentation produced 0 captures — which bounds the rate and proves nothing: treating the historical ~1-in-5 as an independent per-run rate, zero captures in 20 runs has probability `(4/5)^20 ≈ 1.2%`, and independence is an assumption rather than an established fact; it ran at 3084–3834 MB free against roughly 2.5 GB at the historical captures, consistent with the resource-contention hypothesis but not evidence for it. **Signature B stays UNRESOLVED**; what changed is that the next occurrence is readable rather than silent. **During M15 Increment 46 validation, Signature B was directly observed three more times, broadening the observed scope to `packages/persistence`:** `search-backfill.integration.test.ts`, `learning.integration.test.ts`, and `test-database.integration.test.ts` each died with the documented bare whole-file `'test failed'` with zero tests reporting and no assertion or stack, and each passed standalone against the exact database state it died on. This confirms the defect is not specific to `packages/api` or its HTTP server test harness. **M15 Increment 47 hardened the diagnostic correlator across 26 bounded runs:** 1 baseline full-suite run, 20 sequential runs under `run-signature-b-pass.mjs`, and 5 concurrent repository-native load runs produced 0 captures (which bounds the rate under those conditions and does not resolve the defect). Code inspection and falsification identified and fixed four correlator blind spots in `packages/api/test/diagnostics/signature-b-correlate.cjs`: (1) cross-directory basename fallback could falsely match a child log from another directory when the target had slashes; (2) Windows-origin path normalization was host-dependent (`process.platform === 'win32'`), breaking correlation on POSIX hosts; (3) signed 32-bit Windows NTSTATUS exit codes (e.g. `0xC0000005` surfacing as `-1073741819`) were not normalized to unsigned values; and (4) quoted TAP YAML scalar tokens lost exit code or duration numbers. The targeted test suite was expanded from 23 to 31 tests (29 pass, 2 skip, 0 fail), killing 4 falsification mutations. **Signature B remains UNRESOLVED;** no production fix was invented, and the next occurrence will be correlated without cross-platform or exit-code blind spots. See ADR-0140 §4. +- **`startHarness` drew ports `fetch` refuses (RESOLVED in ADR-0140); the unexplained whole-file failure is still open.** Two signatures were filed here, deliberately not as one cause, and that judgement held. **Signature A is resolved.** WHATWG Fetch blocks eighty-two ports and undici enforces the list on the port number alone, before opening a socket — so `server.listen(0)` could bind, listen and answer raw TCP while `fetch` still refused, surfacing as `TypeError: fetch failed` / `Error: bad port` at the harness’s first request rather than at the listen that caused it. Whether it can happen at all is a property of the host’s dynamic port range: a typical Linux CI range (32768–60999) contains no blocked port, while the Windows range in use here (1024–15000) contains nineteen, which is the whole of "green on CI, flaky locally". The guard that already existed was incomplete in a way that still failed — its hand-observed set of eighteen ports was the spec list intersected with one machine’s range **minus `6679`** — and it retried unboundedly, had no behaviour on exhaustion, and had been copy-pasted into `auth-signin-schema.integration.test.ts`, so the missing port had to be found twice. `packages/api/test/listen.ts` now owns port acquisition for both sites: the spec-complete eighty-two ports (verified by sweeping all 65535 through the real `fetch` on Node v24.15.0), a bounded twenty attempts, each rejected listener closed before the next is asked for, a guarded `address()` read in place of the `as AddressInfo` cast, and an exhaustion error naming the attempts and rejected ports and nothing else. **Signature B is not resolved and was not folded in.** A file still fails with `'test failed'`, no assertion, no stack and none of its own tests reported. Twenty consecutive full runs gave five failures: on pre-fix code one signature A (`auth.test.js`) and three signature B; on post-fix code one signature B and no signature A. The port fix removes A and leaves B exactly where it was, which is the evidence that they are two defects. Four different files were hit (`move-explanation-route`, `tournament-commentary-route`, `bot-detection-analyze`, `anti-cheat-analysis`), sharing no import beyond `./helpers`; each died in 589–703 ms with no test of its own reporting and no stderr. Refuted with evidence: ephemeral-port exhaustion (113 sockets in TIME_WAIT against a 13977-port range), a `Promise.race` loser becoming an unhandled rejection (`race` subscribes to every promise, confirmed on v24.15.0), a throwing `after`/`afterEach` hook (the affected files use none), and a double `close()` rejecting (awaited in a `finally`, it would be attributed to that test with a stack). A second bounded pass — twelve more full runs under the TAP reporter with a preload recording `uncaughtException`, `unhandledRejection` and any non-zero exit — produced twelve clean runs and captured nothing at the time. **A follow-up increment (`claude/node-test-signature-b`) then captured the defect directly, three more times, on three files never previously implicated** (`rate-limit-atomicity`, `dependency-parity`, `studies-api`) — seven distinct files observed with this symptom to date. Occurrences across seven distinct files make a shared or cross-cutting path more plausible and make a defect confined to one test file less likely, but do not exclude file-specific inputs or lifecycle interactions. An instrumented preload (`packages/api/test/diagnostics/signature-b-preload.cjs`) hooking process-level events — `process.exit`, `process.abort`, `process.kill`, `uncaughtExceptionMonitor` (passively observing uncaught exceptions and fatal unhandled rejections), `warning`, `beforeExit`, and Node’s own unconditional `exit` — showed **none of the hooks active at the time fired** on any of the three historical captures (though `process.abort()` was not wrapped in those initial runs and is now covered for future occurrences). A synthetic `process.exit(1)`-before-registration fixture reproduces the identical silent shape; every other synthetic mechanism tried (a post-test async throw, an emitter `'error'` with its listener removed, a synchronous module-load throw, a delayed `SIGKILL`) prints a visibly different diagnostic line, stack, or partial test output that the real defect never shows. This narrows the investigated possibilities while leaving the root cause unresolved: the per-file child process (`node --test` spawns one per file, confirmed by distinct PIDs) was not terminated by `process.exit`, uncaught exceptions, or fatal unhandled rejections, and future runs with `process.abort` instrumentation will record whether abort was called through JS; an absent record narrows in-runtime JS termination but cannot alone prove external termination without corroborating child exit status/signal data or OS-level crash evidence (e.g. distinguishing an external kill or uncatchable signal from a native C++/V8 crash). The machine had roughly 2.5 GB of 15.7 GB RAM free at capture time with several other agents’ processes concurrently running, which is circumstantially consistent with resource contention, but no crash was recorded in the Windows Application or System event logs in that window, so the exact external trigger is still not established. No fix was invented — the forbidden responses (sleeps, whole-file retries, lowering concurrency) would only hide the unresolved root cause, whose origin is not yet established. **A further increment then crossed the parent/child boundary the earlier work stopped at, and found the evidence had been there all along:** Node's runner attaches the child's `exitCode` and `signal` to the `ERR_TEST_FAILURE` it throws, and the `spec` reporter discards them — `formatError` replaces the error with `error.cause`, the bare string `'test failed'` — while the built-in `tap` reporter serializes them, so running `spec` to stdout and `tap` to a file recovers the exit status with no custom reporter and no patched internals. Exit codes were measured on this platform rather than assumed: `process.abort()` gives `134`, `Stop-Process -Force` gives `4294967295`, NTSTATUS faults surface as raw unsigned values such as `3221225477` (`0xC0000005`) — and `1` is produced alike by an uncaught exception, `process.exit(1)`, `taskkill /F` and `process.kill`, so it identifies nothing on its own and is classified `inconclusive`. `signature-b-correlate.cjs` joins the parent's TAP record to the child's JSONL log on the test file path (which also yields the child PID) and states what the pair does and does not establish; where the exit code is ambiguous, a child that reached `preload-installed` and then logged nothing still excludes `process.exit` and an uncaught exception, because both would have left a record and fired Node's `exit` event. A bounded pass of 20 runs under this instrumentation produced 0 captures — which bounds the rate and proves nothing: treating the historical ~1-in-5 as an independent per-run rate, zero captures in 20 runs has probability `(4/5)^20 ≈ 1.2%`, and independence is an assumption rather than an established fact; it ran at 3084–3834 MB free against roughly 2.5 GB at the historical captures, consistent with the resource-contention hypothesis but not evidence for it. **Signature B stays UNRESOLVED**; what changed is that the next occurrence is readable rather than silent. **During M15 Increment 46 validation, Signature B was directly observed three more times, broadening the observed scope to `packages/persistence`:** `search-backfill.integration.test.ts`, `learning.integration.test.ts`, and `test-database.integration.test.ts` each died with the documented bare whole-file `'test failed'` with zero tests reporting and no assertion or stack, and each passed standalone against the exact database state it died on. This confirms the defect is not specific to `packages/api` or its HTTP server test harness. **M15 Increment 47 hardened the diagnostic correlator across 26 bounded runs:** 1 baseline full-suite run, 20 sequential runs under `run-signature-b-pass.mjs`, and 5 concurrent repository-native load runs produced 0 captures (which bounds the rate under those conditions and does not resolve the defect). Code inspection and falsification identified and fixed four correlator blind spots in `packages/api/test/diagnostics/signature-b-correlate.cjs`: (1) cross-directory basename fallback could falsely match a child log from another directory when the target had slashes; (2) Windows path case folding was host-dependent (`process.platform === 'win32'`), breaking correlation when Windows-origin logs were analyzed on POSIX hosts; (3) signed 32-bit Windows NTSTATUS exit codes (e.g. `0xC0000005` surfacing as `-1073741819`) were not normalized to unsigned values; and (4) quoted TAP YAML scalar tokens lost exit code or duration numbers. The targeted test suite was expanded from 23 to 31 tests (29 pass, 2 skip, 0 fail), killing 4 falsification mutations. **Signature B remains UNRESOLVED;** no production fix was invented, and the next occurrence will be correlated without cross-platform or exit-code blind spots. See ADR-0140 §4. - **Isolated-database test teardown dropped databases out from under connections that had not finished closing (RESOLVED in M15 Increment 45).** `withDatabase` in `packages/persistence/test/variant-migrations.integration.test.ts` ended its pool and then immediately ran `DROP DATABASE ... WITH (FORCE)`. `pool.end()` does not wait for its clients to close: in pg 8.22.0 `_pulseQueue` reaches the end callback in the same synchronous turn in which `_remove` filters the last client out of `_clients`, while `client.end()` has only queued the Terminate byte — instrumentation recorded **zero of four `remove` events fired at the moment `end()` resolved**. The drop could therefore still find a backend attached; `FORCE` terminated it, and the resulting `FATAL` arrived on a socket whose pool still had `idleListener` attached, which `pg` re-emitted as `pool.emit('error')` — an unhandled EventEmitter error that `node:test` attributed to whichever test was running rather than to the teardown that caused it. It surfaced as intermittent `terminating connection due to administrator command` failures in `postgres integration (persistence)` during M15 Increment 44, on a different test each run, which is the signature of a race rather than a broken assertion. The same shape existed in `packages/api/test/auth-signin-schema.integration.test.ts`, which had absorbed SQLSTATE 57P01 with a `pool.on('error', ...)` listener — a symptom fix for the same cause. **Resolved in Increment 45:** a shared `withTestDatabase` helper (`@chess-platform/persistence/test-support`) ends the pool under a bound, waits for `pg_stat_activity` to report the database unused, and drops it *without* `FORCE`. Measured on PostgreSQL 16.14, a plain drop against a still-attached backend fails with SQLSTATE 55006 and leaves that connection untouched, where `FORCE` succeeds by killing it — so the change trades a quiet, harmful success for a loud, harmless failure. FORCE remains only on the emergency path that guarantees the disposable database is still dropped once teardown has already failed — best effort, since that last drop runs inside a `catch` so it cannot bury the error being reported. The 57P01 absorber is deleted, because the corrected lifecycle never causes one. `createPool` and `migrate` are unchanged, and no migration was added. - **The persistence integration suite was not idempotent against a reused database (RESOLVED in M15 Increment 46).** Recorded as a known defect by Increment 45 and left open there. Against a fresh PostgreSQL 16 database the suite passed; a second run against the *same* database failed nine tests, deterministically — measured on 16.14 before any edit as **173 pass / 0 fail** then **164 pass / 9 fail**. CI provisions a fresh server per run, so it never surfaced there. One contract was being broken in two directions: a suite sharing `chess_test` must remove every row it created and remove nothing else. `achievements.integration.test.ts` broke the second half with an unqualified `DELETE FROM users` in `beforeEach` — `games.white_id` and `games.black_id` are the only references to `users` without `ON DELETE CASCADE` (thirty-one FKs point at `users`; twenty-nine cascade; the two that do not are both on `games`), so one game left behind by `pg.integration.test.ts` aborted the wipe with SQLSTATE 23503 before any assertion ran, and the same statement destroyed the bot accounts migration 0021 seeds, which nothing restores because `migrate` has already recorded 0021 as applied. `pg/identity-tokens.test.ts` and `tournaments.pg.integration.test.ts` broke the first half, leaving fixed primary keys behind and colliding on `users_pkey` and `tournaments_pkey` (the latter surfacing through the repository's compare-and-set as `VersionConflictError`). **Resolved in Increment 46:** suites that legitimately share the database delete exactly their own rows through `withSharedDatabase` (`packages/persistence/src/test-support/fixtures.ts`), the sibling of Increment 45's `withTestDatabase` and heir to its precedence rule — a cleanup failure never replaces the assertion that actually failed, and never disappears either. `pg.integration.test.ts` moved to disposable databases instead, because it cannot meet the cleanup half at all: it appends to `game_events`, which is append-only by production trigger, so cleaning up after itself would have meant weakening a production safety rule to suit a test. `users-batch`, `anti-cheat`, `bot-reports` and `analysis-cache` were corrected for the same contract though none of them ever failed — fresh `uuidv7()` ids meant their leaked rows could not collide, so the tables merely grew on every run. Serialization was not the fix and was not introduced: `--test-concurrency=1` is a pre-existing documented invariant and the second run failed identically under it. Acceptance is three consecutive runs against one database with no reset between them (186/186/186), after which only the three migration-seeded bot accounts remain, with no leaked disposable databases and no lingering backends; falsification killed 17 of 20 mutations. No production code, migration, checksum, constraint or repository conflict semantic changed, and no migration was added. - **`ApiServer.listen` registered no `'error'` handler, so a failed bind hung and raised an uncaught event (RESOLVED in ADR-0140 §5).** `packages/api/src/server.ts` resolved its promise from the `listening` callback only and built it with no reject path. A bind failing asynchronously (`EADDRINUSE`, `EMFILE`) left the promise pending forever and, with no `'error'` listener on the `http.Server`, was re-raised as an uncaught exception. Found while investigating ADR-0140 and independently raised by the Qodo review of PR #21. **Resolved in the same increment** rather than deferred, because ADR-0140 §2's bounded, diagnosable acquisition is not true without it — the retry can only report a bind error if the listener it is handed rejects. A one-shot `'error'` listener now rejects and is removed once listening, so later server errors keep their previous semantics rather than being swallowed by a `reject` on a settled promise. The regression test fails against the exact pre-fix code, through an uncaught `ERR_UNHANDLED_ERROR`. From 75559b654a7f500a8d97ec8765d1804debc80b00 Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Fri, 4 Sep 2026 23:00:42 +0300 Subject: [PATCH 06/13] fix(api-diagnostics): preserve relative Windows path semantics before normalization --- .../diagnostics/signature-b-correlate.cjs | 20 +++++++++----- .../diagnostics/signature-b-correlate.test.ts | 27 ++++++++++++++++++- 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/packages/api/test/diagnostics/signature-b-correlate.cjs b/packages/api/test/diagnostics/signature-b-correlate.cjs index 1dba8018..e25a04e0 100644 --- a/packages/api/test/diagnostics/signature-b-correlate.cjs +++ b/packages/api/test/diagnostics/signature-b-correlate.cjs @@ -72,7 +72,12 @@ function fileKey(filePath) { * @returns {boolean} */ function isWindowsPath(filePath) { - return process.platform === 'win32' || /^[a-zA-Z]:(?:\/|$)/.test(filePath) || /^\/\/[^/]/.test(filePath); + return ( + process.platform === 'win32' || + String(filePath).includes('\\') || + /^[a-zA-Z]:(?:\/|$)/.test(filePath) || + /^\/\/[^/]/.test(filePath) + ); } /** @@ -301,12 +306,14 @@ function readChildLogs(logDir) { continue; } if (typeof record.testFile !== 'string' || record.testFile === '') continue; + const isWin = isWindowsPath(record.testFile); // Keyed on the child's whole path, not its basename. A suite run spans directories, and // `foo/a.test.js` and `bar/a.test.js` are different files whose events must never be merged. const key = normalizePath(record.testFile); - const existing = byFile.get(key) ?? { pid: null, kinds: [] }; + const existing = byFile.get(key) ?? { pid: null, kinds: [], isWindows: false }; existing.pid = typeof record.pid === 'number' ? record.pid : existing.pid; if (typeof record.kind === 'string') existing.kinds.push(record.kind); + if (isWin) existing.isWindows = true; byFile.set(key, existing); } } @@ -329,11 +336,12 @@ function readChildLogs(logDir) { * @returns {{ child: { pid: number | null, kinds: string[] } | null, ambiguous: boolean, candidates: number }} */ function matchChild(childLogs, parentFile) { + const isWantedWin = isWindowsPath(parentFile); const wanted = normalizePath(parentFile); const entries = [...childLogs.entries()]; - const bySuffix = entries.filter(([key]) => { - if (isWindowsPath(key) || isWindowsPath(wanted)) { + const bySuffix = entries.filter(([key, child]) => { + if (child?.isWindows || isWindowsPath(key) || isWantedWin || isWindowsPath(wanted)) { const keyNorm = key.toLowerCase(); const wantedNorm = wanted.toLowerCase(); return keyNorm === wantedNorm || keyNorm.endsWith(`/${wantedNorm}`); @@ -348,9 +356,9 @@ function matchChild(childLogs, parentFile) { // Matching a different directory's child would be a false cross-directory attribution. if (!wanted.includes('/')) { const base = fileKey(wanted); - const byBase = entries.filter(([key]) => { + const byBase = entries.filter(([key, child]) => { const kBase = fileKey(key); - if (isWindowsPath(key) || isWindowsPath(wanted)) { + if (child?.isWindows || isWindowsPath(key) || isWantedWin || isWindowsPath(wanted)) { return kBase.toLowerCase() === base.toLowerCase(); } return kBase === base; diff --git a/packages/api/test/diagnostics/signature-b-correlate.test.ts b/packages/api/test/diagnostics/signature-b-correlate.test.ts index bfcadfd0..5438ee00 100644 --- a/packages/api/test/diagnostics/signature-b-correlate.test.ts +++ b/packages/api/test/diagnostics/signature-b-correlate.test.ts @@ -848,10 +848,11 @@ test('correlator: path suffix matching for Windows-origin paths is case-insensit } }); -test('correlator: isWindowsPath identifies Windows drive letters and UNC paths', () => { +test('correlator: isWindowsPath identifies Windows drive letters, UNC paths, and backslashes', () => { assert.equal(correlator.isWindowsPath('C:/repo/test.js'), true); assert.equal(correlator.isWindowsPath('d:/repo/test.js'), true); assert.equal(correlator.isWindowsPath('//server/share/test.js'), true); + assert.equal(correlator.isWindowsPath('Dist-Test\\Test\\Foo.test.js'), true); if (process.platform !== 'win32') { assert.equal(correlator.isWindowsPath('/home/runner/repo/test.js'), false); assert.equal(correlator.isWindowsPath('dist-test/test.js'), false); @@ -859,3 +860,27 @@ test('correlator: isWindowsPath identifies Windows drive letters and UNC paths', assert.equal(correlator.isWindowsPath('/any/path'), true); } }); + +test('correlator: relative backslash-delimited Windows child path matches case-insensitively across platforms', () => { + const logDir = fs.mkdtempSync(path.join(os.tmpdir(), 'sigb-relwin-')); + try { + const lines = [ + { kind: 'start', pid: 444, testFile: 'Dist-Test\\Test\\Studies-Api.Test.js' }, + { kind: 'preload-installed', pid: 444, testFile: 'Dist-Test\\Test\\Studies-Api.Test.js' }, + ]; + fs.writeFileSync(path.join(logDir, 'run-relwin.jsonl'), `${lines.map((l) => JSON.stringify(l)).join('\n')}\n`); + const logs = correlator.readChildLogs(logDir); + + // Parent uses lower-case forward-slash path + const resolved = correlator.correlate( + correlator.parseTapFailures(tapFailure('dist-test/test/studies-api.test.js', '1')), + logs, + ); + assert.equal(resolved.length, 1); + assert.equal(resolved[0]?.child.pid, 444, 'relative backslash-delimited child path must match case-insensitively'); + assert.equal(resolved[0]?.child.logFound, true); + assert.equal(resolved[0]?.child.ambiguous, false); + } finally { + fs.rmSync(logDir, { recursive: true, force: true }); + } +}); From 2fa6359cb4772a9081f46789cc864b9398f1bffe Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Fri, 4 Sep 2026 23:09:23 +0300 Subject: [PATCH 07/13] fix(api-diagnostics): decouple path case-folding from analyzer host platform --- docs/PROJECT_STATE.md | 17 +++++------ docs/ROADMAP.md | 2 +- .../diagnostics/signature-b-correlate.cjs | 17 +++++------ .../diagnostics/signature-b-correlate.test.ts | 28 +++++++++++++++---- 4 files changed, 42 insertions(+), 22 deletions(-) diff --git a/docs/PROJECT_STATE.md b/docs/PROJECT_STATE.md index 1178639c..156ec49b 100644 --- a/docs/PROJECT_STATE.md +++ b/docs/PROJECT_STATE.md @@ -40,12 +40,13 @@ parent TAP diagnostic logs and child JSONL preloads were matched and parsed: slashes (e.g. `test/diagnostics/sample.test.ts`), a child log from an unrelated directory with the same basename could be falsely attributed. Hardened to require `!wanted.includes('/')` before allowing basename fallback. -2. **Host-dependent Windows path case folding:** While backslash-to-slash normalization was already - platform-independent, case folding in `matchChild` was gated strictly on `process.platform === 'win32'`. - When logs recorded on Windows were analyzed on a POSIX host (Linux or macOS), comparisons remained - case-sensitive, causing path matching to fail when runner TAP paths and child `process.argv[1]` paths - differed in casing. Hardened `isWindowsPath` to detect Windows-origin paths by drive letter or UNC prefix - independently of the executing host OS, ensuring case-insensitive matching across platforms. +2. **Host-independent Windows path case folding:** Case folding in `matchChild` previously depended on + the analyzer host platform (`process.platform === 'win32'`). When logs recorded on Windows were + analyzed on POSIX, comparisons remained case-sensitive; conversely, POSIX captures analyzed on + Windows were erroneously treated as case-insensitive. Hardened `isWindowsPath` to detect + Windows-origin paths strictly from path syntax (drive letters, UNC prefixes, or backslashes) and + preserved child-log metadata independently of the executing host OS, ensuring case-insensitive + matching for Windows captures while strictly preserving case-sensitivity for POSIX captures. 3. **Signed 32-bit NTSTATUS exit code wrapping:** On Windows, crash exit codes such as `0xC0000005` (access violation) or `STATUS_CONTROL_C_EXIT` can surface in Node/libuv or TAP as negative 32-bit integers (e.g. `-1073741819`). Hardened exit code parsing to normalize negative 32-bit values via @@ -56,8 +57,8 @@ parent TAP diagnostic logs and child JSONL preloads were matched and parsed: prior to `Number(...)` conversion. **Verification and mutation falsification.** -- Targeted correlator suite (`signature-b-correlate.test.ts`): expanded from 23 to 31 tests - (29 pass, 2 skip for deliberate platform-gated checks, 0 fail). +- Targeted correlator suite (`signature-b-correlate.test.ts`): expanded from 23 to 33 tests + (31 pass, 2 skip for deliberate platform-gated checks, 0 fail). - Mutation falsification: four targeted mutations reversing each of the four hardened behaviors were verified to be killed by the expanded test suite. - Monorepo validation: full build, lint, counts, and guard scripts pass cleanly. diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 7542bfb8..4bb3aec3 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -1349,7 +1349,7 @@ Debt observed during M14. Each states what is known, not what is planned; items - **The published capability document could omit a composed feature, and the client offered controls that could not work (RESOLVED in M15 Increment 24 / ADR-0132).** Left open by Increment 23 above. `capabilitiesView` (`packages/api/src/presenters.ts`) took a hand-written `Pick`, so a feature never added to it was invisible to `GET /v1/capabilities` and nothing complained. ADR-0131 judged that "a narrower failure than a 503 — the feature works for anyone who calls the route directly", and deferred it on the grounds that closing it meant first deciding which optional dependencies are user-facing capabilities. **Both halves of that were wrong.** The failure is narrower only when the client can still reach the feature, and a live instance already existed where it could not: `GET /v1/search` served three modes from two independently-gated dependency sets behind one published `search` flag, so a deployment running the Helm chart’s `search.semanticEnabled: false` advertised search while the client offered two mode buttons whose every request answered 503. And the judgement, while genuinely underivable, can be made **unskippable**, which was the property wanted. `semanticSearch` is now published from both of its dependencies, `Exclude[0] | NotAPublishedCapability>` must be `never` so a new optional dependency cannot compile until someone gives it a flag or records why it has none, and a behavioural guard requires every capability source to change what the document publishes — because a key can sit in that parameter unread, which the compile-time half cannot see. The mutation ledger lives in ADR-0132 and is not restated here. - **The search surface was not gated on `capabilities.search`, so an absolute kill switch still showed a search box (RESOLVED in M15 Increment 24 / ADR-0132 §5).** `SEARCH_ENABLED=0` — the chart's `search.enabled: false`, an absolute kill switch per ADR-0055 — leaves `searchRepository` unconstructed and `GET /v1/search` answering 503 on every mode, keyword included. The entry point was the persistent header form in `packages/web/index.html`, present on every page; being a `` rather than an `a[data-route]`, `NAV_CAPABILITY_MAP` could not reach it, which is why the first pass at Increment 24 gated the semantic and hybrid *modes* and left keyword ungated — the same defect class one mode over. Raised by the Qodo review of PR #155. **Fixed in the same increment rather than deferred:** the form now ships `hidden` and is revealed by `applySearchCapability` only on an explicit `search: true`; the route renders an honest unavailable notice and issues no request; and keyword search waits for the capability answer, reversing this increment's own earlier latency decision, because knowing whether a request is pointless requires having asked. A markup-contract test pins the `hidden` attribute, since the gate depends on it and every other test passes without it. - **Clicking a search mode discarded text typed since the page loaded (RESOLVED in a follow-up to M15 Increment 24 / ADR-0132).** `createModeInput` in `packages/web/src/app/search-mount.ts` closed over the query captured when the route mounted, so `navigateToSearchMode` navigated with the old term and the remount reset the input to match it. Type a new term into the header field, click **Semantic** without pressing enter, and the typed text was gone with no indication it had been discarded. Pre-existing — the closure predated Increment 24 and was untouched by it — and found by the adversarial review of PR #155 while reviewing the capability gate wrapped around the same control. **Resolved:** the query is now a `() => string` read when a mode is chosen rather than a string captured when the selector renders, matching what `main.ts`'s submit handler already does, and falling back to the mounted query only where the document has no header input. Two regression tests, one of which fails against the exact pre-fix closure. -- **`startHarness` drew ports `fetch` refuses (RESOLVED in ADR-0140); the unexplained whole-file failure is still open.** Two signatures were filed here, deliberately not as one cause, and that judgement held. **Signature A is resolved.** WHATWG Fetch blocks eighty-two ports and undici enforces the list on the port number alone, before opening a socket — so `server.listen(0)` could bind, listen and answer raw TCP while `fetch` still refused, surfacing as `TypeError: fetch failed` / `Error: bad port` at the harness’s first request rather than at the listen that caused it. Whether it can happen at all is a property of the host’s dynamic port range: a typical Linux CI range (32768–60999) contains no blocked port, while the Windows range in use here (1024–15000) contains nineteen, which is the whole of "green on CI, flaky locally". The guard that already existed was incomplete in a way that still failed — its hand-observed set of eighteen ports was the spec list intersected with one machine’s range **minus `6679`** — and it retried unboundedly, had no behaviour on exhaustion, and had been copy-pasted into `auth-signin-schema.integration.test.ts`, so the missing port had to be found twice. `packages/api/test/listen.ts` now owns port acquisition for both sites: the spec-complete eighty-two ports (verified by sweeping all 65535 through the real `fetch` on Node v24.15.0), a bounded twenty attempts, each rejected listener closed before the next is asked for, a guarded `address()` read in place of the `as AddressInfo` cast, and an exhaustion error naming the attempts and rejected ports and nothing else. **Signature B is not resolved and was not folded in.** A file still fails with `'test failed'`, no assertion, no stack and none of its own tests reported. Twenty consecutive full runs gave five failures: on pre-fix code one signature A (`auth.test.js`) and three signature B; on post-fix code one signature B and no signature A. The port fix removes A and leaves B exactly where it was, which is the evidence that they are two defects. Four different files were hit (`move-explanation-route`, `tournament-commentary-route`, `bot-detection-analyze`, `anti-cheat-analysis`), sharing no import beyond `./helpers`; each died in 589–703 ms with no test of its own reporting and no stderr. Refuted with evidence: ephemeral-port exhaustion (113 sockets in TIME_WAIT against a 13977-port range), a `Promise.race` loser becoming an unhandled rejection (`race` subscribes to every promise, confirmed on v24.15.0), a throwing `after`/`afterEach` hook (the affected files use none), and a double `close()` rejecting (awaited in a `finally`, it would be attributed to that test with a stack). A second bounded pass — twelve more full runs under the TAP reporter with a preload recording `uncaughtException`, `unhandledRejection` and any non-zero exit — produced twelve clean runs and captured nothing at the time. **A follow-up increment (`claude/node-test-signature-b`) then captured the defect directly, three more times, on three files never previously implicated** (`rate-limit-atomicity`, `dependency-parity`, `studies-api`) — seven distinct files observed with this symptom to date. Occurrences across seven distinct files make a shared or cross-cutting path more plausible and make a defect confined to one test file less likely, but do not exclude file-specific inputs or lifecycle interactions. An instrumented preload (`packages/api/test/diagnostics/signature-b-preload.cjs`) hooking process-level events — `process.exit`, `process.abort`, `process.kill`, `uncaughtExceptionMonitor` (passively observing uncaught exceptions and fatal unhandled rejections), `warning`, `beforeExit`, and Node’s own unconditional `exit` — showed **none of the hooks active at the time fired** on any of the three historical captures (though `process.abort()` was not wrapped in those initial runs and is now covered for future occurrences). A synthetic `process.exit(1)`-before-registration fixture reproduces the identical silent shape; every other synthetic mechanism tried (a post-test async throw, an emitter `'error'` with its listener removed, a synchronous module-load throw, a delayed `SIGKILL`) prints a visibly different diagnostic line, stack, or partial test output that the real defect never shows. This narrows the investigated possibilities while leaving the root cause unresolved: the per-file child process (`node --test` spawns one per file, confirmed by distinct PIDs) was not terminated by `process.exit`, uncaught exceptions, or fatal unhandled rejections, and future runs with `process.abort` instrumentation will record whether abort was called through JS; an absent record narrows in-runtime JS termination but cannot alone prove external termination without corroborating child exit status/signal data or OS-level crash evidence (e.g. distinguishing an external kill or uncatchable signal from a native C++/V8 crash). The machine had roughly 2.5 GB of 15.7 GB RAM free at capture time with several other agents’ processes concurrently running, which is circumstantially consistent with resource contention, but no crash was recorded in the Windows Application or System event logs in that window, so the exact external trigger is still not established. No fix was invented — the forbidden responses (sleeps, whole-file retries, lowering concurrency) would only hide the unresolved root cause, whose origin is not yet established. **A further increment then crossed the parent/child boundary the earlier work stopped at, and found the evidence had been there all along:** Node's runner attaches the child's `exitCode` and `signal` to the `ERR_TEST_FAILURE` it throws, and the `spec` reporter discards them — `formatError` replaces the error with `error.cause`, the bare string `'test failed'` — while the built-in `tap` reporter serializes them, so running `spec` to stdout and `tap` to a file recovers the exit status with no custom reporter and no patched internals. Exit codes were measured on this platform rather than assumed: `process.abort()` gives `134`, `Stop-Process -Force` gives `4294967295`, NTSTATUS faults surface as raw unsigned values such as `3221225477` (`0xC0000005`) — and `1` is produced alike by an uncaught exception, `process.exit(1)`, `taskkill /F` and `process.kill`, so it identifies nothing on its own and is classified `inconclusive`. `signature-b-correlate.cjs` joins the parent's TAP record to the child's JSONL log on the test file path (which also yields the child PID) and states what the pair does and does not establish; where the exit code is ambiguous, a child that reached `preload-installed` and then logged nothing still excludes `process.exit` and an uncaught exception, because both would have left a record and fired Node's `exit` event. A bounded pass of 20 runs under this instrumentation produced 0 captures — which bounds the rate and proves nothing: treating the historical ~1-in-5 as an independent per-run rate, zero captures in 20 runs has probability `(4/5)^20 ≈ 1.2%`, and independence is an assumption rather than an established fact; it ran at 3084–3834 MB free against roughly 2.5 GB at the historical captures, consistent with the resource-contention hypothesis but not evidence for it. **Signature B stays UNRESOLVED**; what changed is that the next occurrence is readable rather than silent. **During M15 Increment 46 validation, Signature B was directly observed three more times, broadening the observed scope to `packages/persistence`:** `search-backfill.integration.test.ts`, `learning.integration.test.ts`, and `test-database.integration.test.ts` each died with the documented bare whole-file `'test failed'` with zero tests reporting and no assertion or stack, and each passed standalone against the exact database state it died on. This confirms the defect is not specific to `packages/api` or its HTTP server test harness. **M15 Increment 47 hardened the diagnostic correlator across 26 bounded runs:** 1 baseline full-suite run, 20 sequential runs under `run-signature-b-pass.mjs`, and 5 concurrent repository-native load runs produced 0 captures (which bounds the rate under those conditions and does not resolve the defect). Code inspection and falsification identified and fixed four correlator blind spots in `packages/api/test/diagnostics/signature-b-correlate.cjs`: (1) cross-directory basename fallback could falsely match a child log from another directory when the target had slashes; (2) Windows path case folding was host-dependent (`process.platform === 'win32'`), breaking correlation when Windows-origin logs were analyzed on POSIX hosts; (3) signed 32-bit Windows NTSTATUS exit codes (e.g. `0xC0000005` surfacing as `-1073741819`) were not normalized to unsigned values; and (4) quoted TAP YAML scalar tokens lost exit code or duration numbers. The targeted test suite was expanded from 23 to 31 tests (29 pass, 2 skip, 0 fail), killing 4 falsification mutations. **Signature B remains UNRESOLVED;** no production fix was invented, and the next occurrence will be correlated without cross-platform or exit-code blind spots. See ADR-0140 §4. +- **`startHarness` drew ports `fetch` refuses (RESOLVED in ADR-0140); the unexplained whole-file failure is still open.** Two signatures were filed here, deliberately not as one cause, and that judgement held. **Signature A is resolved.** WHATWG Fetch blocks eighty-two ports and undici enforces the list on the port number alone, before opening a socket — so `server.listen(0)` could bind, listen and answer raw TCP while `fetch` still refused, surfacing as `TypeError: fetch failed` / `Error: bad port` at the harness’s first request rather than at the listen that caused it. Whether it can happen at all is a property of the host’s dynamic port range: a typical Linux CI range (32768–60999) contains no blocked port, while the Windows range in use here (1024–15000) contains nineteen, which is the whole of "green on CI, flaky locally". The guard that already existed was incomplete in a way that still failed — its hand-observed set of eighteen ports was the spec list intersected with one machine’s range **minus `6679`** — and it retried unboundedly, had no behaviour on exhaustion, and had been copy-pasted into `auth-signin-schema.integration.test.ts`, so the missing port had to be found twice. `packages/api/test/listen.ts` now owns port acquisition for both sites: the spec-complete eighty-two ports (verified by sweeping all 65535 through the real `fetch` on Node v24.15.0), a bounded twenty attempts, each rejected listener closed before the next is asked for, a guarded `address()` read in place of the `as AddressInfo` cast, and an exhaustion error naming the attempts and rejected ports and nothing else. **Signature B is not resolved and was not folded in.** A file still fails with `'test failed'`, no assertion, no stack and none of its own tests reported. Twenty consecutive full runs gave five failures: on pre-fix code one signature A (`auth.test.js`) and three signature B; on post-fix code one signature B and no signature A. The port fix removes A and leaves B exactly where it was, which is the evidence that they are two defects. Four different files were hit (`move-explanation-route`, `tournament-commentary-route`, `bot-detection-analyze`, `anti-cheat-analysis`), sharing no import beyond `./helpers`; each died in 589–703 ms with no test of its own reporting and no stderr. Refuted with evidence: ephemeral-port exhaustion (113 sockets in TIME_WAIT against a 13977-port range), a `Promise.race` loser becoming an unhandled rejection (`race` subscribes to every promise, confirmed on v24.15.0), a throwing `after`/`afterEach` hook (the affected files use none), and a double `close()` rejecting (awaited in a `finally`, it would be attributed to that test with a stack). A second bounded pass — twelve more full runs under the TAP reporter with a preload recording `uncaughtException`, `unhandledRejection` and any non-zero exit — produced twelve clean runs and captured nothing at the time. **A follow-up increment (`claude/node-test-signature-b`) then captured the defect directly, three more times, on three files never previously implicated** (`rate-limit-atomicity`, `dependency-parity`, `studies-api`) — seven distinct files observed with this symptom to date. Occurrences across seven distinct files make a shared or cross-cutting path more plausible and make a defect confined to one test file less likely, but do not exclude file-specific inputs or lifecycle interactions. An instrumented preload (`packages/api/test/diagnostics/signature-b-preload.cjs`) hooking process-level events — `process.exit`, `process.abort`, `process.kill`, `uncaughtExceptionMonitor` (passively observing uncaught exceptions and fatal unhandled rejections), `warning`, `beforeExit`, and Node’s own unconditional `exit` — showed **none of the hooks active at the time fired** on any of the three historical captures (though `process.abort()` was not wrapped in those initial runs and is now covered for future occurrences). A synthetic `process.exit(1)`-before-registration fixture reproduces the identical silent shape; every other synthetic mechanism tried (a post-test async throw, an emitter `'error'` with its listener removed, a synchronous module-load throw, a delayed `SIGKILL`) prints a visibly different diagnostic line, stack, or partial test output that the real defect never shows. This narrows the investigated possibilities while leaving the root cause unresolved: the per-file child process (`node --test` spawns one per file, confirmed by distinct PIDs) was not terminated by `process.exit`, uncaught exceptions, or fatal unhandled rejections, and future runs with `process.abort` instrumentation will record whether abort was called through JS; an absent record narrows in-runtime JS termination but cannot alone prove external termination without corroborating child exit status/signal data or OS-level crash evidence (e.g. distinguishing an external kill or uncatchable signal from a native C++/V8 crash). The machine had roughly 2.5 GB of 15.7 GB RAM free at capture time with several other agents’ processes concurrently running, which is circumstantially consistent with resource contention, but no crash was recorded in the Windows Application or System event logs in that window, so the exact external trigger is still not established. No fix was invented — the forbidden responses (sleeps, whole-file retries, lowering concurrency) would only hide the unresolved root cause, whose origin is not yet established. **A further increment then crossed the parent/child boundary the earlier work stopped at, and found the evidence had been there all along:** Node's runner attaches the child's `exitCode` and `signal` to the `ERR_TEST_FAILURE` it throws, and the `spec` reporter discards them — `formatError` replaces the error with `error.cause`, the bare string `'test failed'` — while the built-in `tap` reporter serializes them, so running `spec` to stdout and `tap` to a file recovers the exit status with no custom reporter and no patched internals. Exit codes were measured on this platform rather than assumed: `process.abort()` gives `134`, `Stop-Process -Force` gives `4294967295`, NTSTATUS faults surface as raw unsigned values such as `3221225477` (`0xC0000005`) — and `1` is produced alike by an uncaught exception, `process.exit(1)`, `taskkill /F` and `process.kill`, so it identifies nothing on its own and is classified `inconclusive`. `signature-b-correlate.cjs` joins the parent's TAP record to the child's JSONL log on the test file path (which also yields the child PID) and states what the pair does and does not establish; where the exit code is ambiguous, a child that reached `preload-installed` and then logged nothing still excludes `process.exit` and an uncaught exception, because both would have left a record and fired Node's `exit` event. A bounded pass of 20 runs under this instrumentation produced 0 captures — which bounds the rate and proves nothing: treating the historical ~1-in-5 as an independent per-run rate, zero captures in 20 runs has probability `(4/5)^20 ≈ 1.2%`, and independence is an assumption rather than an established fact; it ran at 3084–3834 MB free against roughly 2.5 GB at the historical captures, consistent with the resource-contention hypothesis but not evidence for it. **Signature B stays UNRESOLVED**; what changed is that the next occurrence is readable rather than silent. **During M15 Increment 46 validation, Signature B was directly observed three more times, broadening the observed scope to `packages/persistence`:** `search-backfill.integration.test.ts`, `learning.integration.test.ts`, and `test-database.integration.test.ts` each died with the documented bare whole-file `'test failed'` with zero tests reporting and no assertion or stack, and each passed standalone against the exact database state it died on. This confirms the defect is not specific to `packages/api` or its HTTP server test harness. **M15 Increment 47 hardened the diagnostic correlator across 26 bounded runs:** 1 baseline full-suite run, 20 sequential runs under `run-signature-b-pass.mjs`, and 5 concurrent repository-native load runs produced 0 captures (which bounds the rate under those conditions and does not resolve the defect). Code inspection and falsification identified and fixed four correlator blind spots in `packages/api/test/diagnostics/signature-b-correlate.cjs`: (1) cross-directory basename fallback could falsely match a child log from another directory when the target had slashes; (2) Windows path case folding was host-dependent (`process.platform === 'win32'`), breaking cross-platform correlation when logs were analyzed on a different OS; (3) signed 32-bit Windows NTSTATUS exit codes (e.g. `0xC0000005` surfacing as `-1073741819`) were not normalized to unsigned values; and (4) quoted TAP YAML scalar tokens lost exit code or duration numbers. The targeted test suite was expanded from 23 to 33 tests (31 pass, 2 skip, 0 fail), killing 4 falsification mutations. **Signature B remains UNRESOLVED;** no production fix was invented, and the next occurrence will be correlated without cross-platform or exit-code blind spots. See ADR-0140 §4. - **Isolated-database test teardown dropped databases out from under connections that had not finished closing (RESOLVED in M15 Increment 45).** `withDatabase` in `packages/persistence/test/variant-migrations.integration.test.ts` ended its pool and then immediately ran `DROP DATABASE ... WITH (FORCE)`. `pool.end()` does not wait for its clients to close: in pg 8.22.0 `_pulseQueue` reaches the end callback in the same synchronous turn in which `_remove` filters the last client out of `_clients`, while `client.end()` has only queued the Terminate byte — instrumentation recorded **zero of four `remove` events fired at the moment `end()` resolved**. The drop could therefore still find a backend attached; `FORCE` terminated it, and the resulting `FATAL` arrived on a socket whose pool still had `idleListener` attached, which `pg` re-emitted as `pool.emit('error')` — an unhandled EventEmitter error that `node:test` attributed to whichever test was running rather than to the teardown that caused it. It surfaced as intermittent `terminating connection due to administrator command` failures in `postgres integration (persistence)` during M15 Increment 44, on a different test each run, which is the signature of a race rather than a broken assertion. The same shape existed in `packages/api/test/auth-signin-schema.integration.test.ts`, which had absorbed SQLSTATE 57P01 with a `pool.on('error', ...)` listener — a symptom fix for the same cause. **Resolved in Increment 45:** a shared `withTestDatabase` helper (`@chess-platform/persistence/test-support`) ends the pool under a bound, waits for `pg_stat_activity` to report the database unused, and drops it *without* `FORCE`. Measured on PostgreSQL 16.14, a plain drop against a still-attached backend fails with SQLSTATE 55006 and leaves that connection untouched, where `FORCE` succeeds by killing it — so the change trades a quiet, harmful success for a loud, harmless failure. FORCE remains only on the emergency path that guarantees the disposable database is still dropped once teardown has already failed — best effort, since that last drop runs inside a `catch` so it cannot bury the error being reported. The 57P01 absorber is deleted, because the corrected lifecycle never causes one. `createPool` and `migrate` are unchanged, and no migration was added. - **The persistence integration suite was not idempotent against a reused database (RESOLVED in M15 Increment 46).** Recorded as a known defect by Increment 45 and left open there. Against a fresh PostgreSQL 16 database the suite passed; a second run against the *same* database failed nine tests, deterministically — measured on 16.14 before any edit as **173 pass / 0 fail** then **164 pass / 9 fail**. CI provisions a fresh server per run, so it never surfaced there. One contract was being broken in two directions: a suite sharing `chess_test` must remove every row it created and remove nothing else. `achievements.integration.test.ts` broke the second half with an unqualified `DELETE FROM users` in `beforeEach` — `games.white_id` and `games.black_id` are the only references to `users` without `ON DELETE CASCADE` (thirty-one FKs point at `users`; twenty-nine cascade; the two that do not are both on `games`), so one game left behind by `pg.integration.test.ts` aborted the wipe with SQLSTATE 23503 before any assertion ran, and the same statement destroyed the bot accounts migration 0021 seeds, which nothing restores because `migrate` has already recorded 0021 as applied. `pg/identity-tokens.test.ts` and `tournaments.pg.integration.test.ts` broke the first half, leaving fixed primary keys behind and colliding on `users_pkey` and `tournaments_pkey` (the latter surfacing through the repository's compare-and-set as `VersionConflictError`). **Resolved in Increment 46:** suites that legitimately share the database delete exactly their own rows through `withSharedDatabase` (`packages/persistence/src/test-support/fixtures.ts`), the sibling of Increment 45's `withTestDatabase` and heir to its precedence rule — a cleanup failure never replaces the assertion that actually failed, and never disappears either. `pg.integration.test.ts` moved to disposable databases instead, because it cannot meet the cleanup half at all: it appends to `game_events`, which is append-only by production trigger, so cleaning up after itself would have meant weakening a production safety rule to suit a test. `users-batch`, `anti-cheat`, `bot-reports` and `analysis-cache` were corrected for the same contract though none of them ever failed — fresh `uuidv7()` ids meant their leaked rows could not collide, so the tables merely grew on every run. Serialization was not the fix and was not introduced: `--test-concurrency=1` is a pre-existing documented invariant and the second run failed identically under it. Acceptance is three consecutive runs against one database with no reset between them (186/186/186), after which only the three migration-seeded bot accounts remain, with no leaked disposable databases and no lingering backends; falsification killed 17 of 20 mutations. No production code, migration, checksum, constraint or repository conflict semantic changed, and no migration was added. - **`ApiServer.listen` registered no `'error'` handler, so a failed bind hung and raised an uncaught event (RESOLVED in ADR-0140 §5).** `packages/api/src/server.ts` resolved its promise from the `listening` callback only and built it with no reject path. A bind failing asynchronously (`EADDRINUSE`, `EMFILE`) left the promise pending forever and, with no `'error'` listener on the `http.Server`, was re-raised as an uncaught exception. Found while investigating ADR-0140 and independently raised by the Qodo review of PR #21. **Resolved in the same increment** rather than deferred, because ADR-0140 §2's bounded, diagnosable acquisition is not true without it — the retry can only report a bind error if the listener it is handed rejects. A one-shot `'error'` listener now rejects and is removed once listening, so later server errors keep their previous semantics rather than being swallowed by a `reject` on a settled promise. The regression test fails against the exact pre-fix code, through an uncaught `ERR_UNHANDLED_ERROR`. diff --git a/packages/api/test/diagnostics/signature-b-correlate.cjs b/packages/api/test/diagnostics/signature-b-correlate.cjs index e25a04e0..74e64d1a 100644 --- a/packages/api/test/diagnostics/signature-b-correlate.cjs +++ b/packages/api/test/diagnostics/signature-b-correlate.cjs @@ -63,20 +63,21 @@ function fileKey(filePath) { } /** - * Detect whether a path string represents a Windows path, either because the host running the - * correlator is Windows, or because the path contains a Windows drive prefix (e.g. `C:/`) or UNC prefix (`//`). - * This ensures Windows-origin captures analyzed on non-Windows machines (e.g. CI on Linux) still apply - * Windows case-insensitive filesystem semantics during child log correlation. + * Detect whether a path string represents a Windows path based strictly on path syntax: + * backslashes, a Windows drive prefix (e.g. `C:/`), or a UNC prefix (`//`). + * Analyzer host platform (`process.platform`) is deliberately omitted so that POSIX captures + * analyzed on Windows preserve case-sensitivity, and Windows captures analyzed on Linux + * apply case-insensitive matching. * * @param {string} filePath * @returns {boolean} */ function isWindowsPath(filePath) { + const str = String(filePath); return ( - process.platform === 'win32' || - String(filePath).includes('\\') || - /^[a-zA-Z]:(?:\/|$)/.test(filePath) || - /^\/\/[^/]/.test(filePath) + str.includes('\\') || + /^[a-zA-Z]:(?:\/|$)/.test(str) || + /^\/\/[^/]/.test(str) ); } diff --git a/packages/api/test/diagnostics/signature-b-correlate.test.ts b/packages/api/test/diagnostics/signature-b-correlate.test.ts index 5438ee00..3bfd5a25 100644 --- a/packages/api/test/diagnostics/signature-b-correlate.test.ts +++ b/packages/api/test/diagnostics/signature-b-correlate.test.ts @@ -853,11 +853,29 @@ test('correlator: isWindowsPath identifies Windows drive letters, UNC paths, and assert.equal(correlator.isWindowsPath('d:/repo/test.js'), true); assert.equal(correlator.isWindowsPath('//server/share/test.js'), true); assert.equal(correlator.isWindowsPath('Dist-Test\\Test\\Foo.test.js'), true); - if (process.platform !== 'win32') { - assert.equal(correlator.isWindowsPath('/home/runner/repo/test.js'), false); - assert.equal(correlator.isWindowsPath('dist-test/test.js'), false); - } else { - assert.equal(correlator.isWindowsPath('/any/path'), true); + assert.equal(correlator.isWindowsPath('/home/runner/repo/test.js'), false); + assert.equal(correlator.isWindowsPath('dist-test/test.js'), false); +}); + +test('correlator: POSIX paths preserve case sensitivity regardless of analyzer host OS', () => { + const logDir = fs.mkdtempSync(path.join(os.tmpdir(), 'sigb-posix-case-')); + try { + const lines = [ + { kind: 'start', pid: 555, testFile: 'dist-test/test/studies-api.test.js' }, + { kind: 'preload-installed', pid: 555, testFile: 'dist-test/test/studies-api.test.js' }, + ]; + fs.writeFileSync(path.join(logDir, 'run-posix.jsonl'), `${lines.map((l) => JSON.stringify(l)).join('\n')}\n`); + const logs = correlator.readChildLogs(logDir); + + // Parent uses different casing on a purely POSIX relative path + const resolved = correlator.correlate( + correlator.parseTapFailures(tapFailure('Dist-Test/Test/Studies-Api.Test.js', '1')), + logs, + ); + assert.equal(resolved.length, 1); + assert.equal(resolved[0]?.child.logFound, false, 'case mismatch on POSIX paths must not match'); + } finally { + fs.rmSync(logDir, { recursive: true, force: true }); } }); From 1fcdb30dd67efefafb252c205f86edb16b1160ed Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Fri, 4 Sep 2026 23:22:43 +0300 Subject: [PATCH 08/13] fix(api-diagnostics): refine Windows path detection, child log casing aggregation, and numeric scalar parsing --- docs/PROJECT_STATE.md | 20 +-- docs/ROADMAP.md | 2 +- .../diagnostics/signature-b-correlate.cjs | 82 +++++++++---- .../diagnostics/signature-b-correlate.test.ts | 116 +++++++++++++++++- 4 files changed, 180 insertions(+), 40 deletions(-) diff --git a/docs/PROJECT_STATE.md b/docs/PROJECT_STATE.md index 156ec49b..7fcd199d 100644 --- a/docs/PROJECT_STATE.md +++ b/docs/PROJECT_STATE.md @@ -44,21 +44,23 @@ parent TAP diagnostic logs and child JSONL preloads were matched and parsed: the analyzer host platform (`process.platform === 'win32'`). When logs recorded on Windows were analyzed on POSIX, comparisons remained case-sensitive; conversely, POSIX captures analyzed on Windows were erroneously treated as case-insensitive. Hardened `isWindowsPath` to detect - Windows-origin paths strictly from path syntax (drive letters, UNC prefixes, or backslashes) and - preserved child-log metadata independently of the executing host OS, ensuring case-insensitive - matching for Windows captures while strictly preserving case-sensitivity for POSIX captures. + Windows-origin paths strictly from path syntax (drive letters or UNC prefixes) and explicit capture + metadata independently of the executing host OS, ensuring case-insensitive matching for Windows + captures while strictly preserving case-sensitivity and filename integrity for POSIX captures + (avoiding false-positive Windows classification on POSIX backslash filenames). 3. **Signed 32-bit NTSTATUS exit code wrapping:** On Windows, crash exit codes such as `0xC0000005` (access violation) or `STATUS_CONTROL_C_EXIT` can surface in Node/libuv or TAP as negative 32-bit integers (e.g. `-1073741819`). Hardened exit code parsing to normalize negative 32-bit values via unsigned right shift `(exitCode >>> 0)`, correctly recovering the standard `0xC0000005` representation. -4. **Quoted TAP YAML scalar parsing:** Node's TAP reporter emits single- or double-quoted scalars - around certain YAML values (e.g. `exitCode: '1'` or duration strings). Strict numeric conversion - previously yielded `NaN` or dropped exit codes. Hardened YAML extraction to unquote scalar tokens - prior to `Number(...)` conversion. +4. **Quoted and non-finite TAP YAML scalar parsing:** Node's TAP reporter emits single- or double-quoted + scalars around certain YAML values (e.g. `exitCode: '1'` or duration strings). Strict numeric conversion + previously yielded `NaN` or dropped exit codes, and non-finite numbers (`Infinity`, `-Infinity`) leaked + through. Hardened YAML extraction to unquote scalar tokens prior to conversion and convert non-finite + values to `null`. **Verification and mutation falsification.** -- Targeted correlator suite (`signature-b-correlate.test.ts`): expanded from 23 to 33 tests - (31 pass, 2 skip for deliberate platform-gated checks, 0 fail). +- Targeted correlator suite (`signature-b-correlate.test.ts`): expanded from 23 to 36 tests + (34 pass, 2 skip for deliberate platform-gated checks, 0 fail). - Mutation falsification: four targeted mutations reversing each of the four hardened behaviors were verified to be killed by the expanded test suite. - Monorepo validation: full build, lint, counts, and guard scripts pass cleanly. diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 4bb3aec3..2d1df2d9 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -1349,7 +1349,7 @@ Debt observed during M14. Each states what is known, not what is planned; items - **The published capability document could omit a composed feature, and the client offered controls that could not work (RESOLVED in M15 Increment 24 / ADR-0132).** Left open by Increment 23 above. `capabilitiesView` (`packages/api/src/presenters.ts`) took a hand-written `Pick`, so a feature never added to it was invisible to `GET /v1/capabilities` and nothing complained. ADR-0131 judged that "a narrower failure than a 503 — the feature works for anyone who calls the route directly", and deferred it on the grounds that closing it meant first deciding which optional dependencies are user-facing capabilities. **Both halves of that were wrong.** The failure is narrower only when the client can still reach the feature, and a live instance already existed where it could not: `GET /v1/search` served three modes from two independently-gated dependency sets behind one published `search` flag, so a deployment running the Helm chart’s `search.semanticEnabled: false` advertised search while the client offered two mode buttons whose every request answered 503. And the judgement, while genuinely underivable, can be made **unskippable**, which was the property wanted. `semanticSearch` is now published from both of its dependencies, `Exclude[0] | NotAPublishedCapability>` must be `never` so a new optional dependency cannot compile until someone gives it a flag or records why it has none, and a behavioural guard requires every capability source to change what the document publishes — because a key can sit in that parameter unread, which the compile-time half cannot see. The mutation ledger lives in ADR-0132 and is not restated here. - **The search surface was not gated on `capabilities.search`, so an absolute kill switch still showed a search box (RESOLVED in M15 Increment 24 / ADR-0132 §5).** `SEARCH_ENABLED=0` — the chart's `search.enabled: false`, an absolute kill switch per ADR-0055 — leaves `searchRepository` unconstructed and `GET /v1/search` answering 503 on every mode, keyword included. The entry point was the persistent header form in `packages/web/index.html`, present on every page; being a `` rather than an `a[data-route]`, `NAV_CAPABILITY_MAP` could not reach it, which is why the first pass at Increment 24 gated the semantic and hybrid *modes* and left keyword ungated — the same defect class one mode over. Raised by the Qodo review of PR #155. **Fixed in the same increment rather than deferred:** the form now ships `hidden` and is revealed by `applySearchCapability` only on an explicit `search: true`; the route renders an honest unavailable notice and issues no request; and keyword search waits for the capability answer, reversing this increment's own earlier latency decision, because knowing whether a request is pointless requires having asked. A markup-contract test pins the `hidden` attribute, since the gate depends on it and every other test passes without it. - **Clicking a search mode discarded text typed since the page loaded (RESOLVED in a follow-up to M15 Increment 24 / ADR-0132).** `createModeInput` in `packages/web/src/app/search-mount.ts` closed over the query captured when the route mounted, so `navigateToSearchMode` navigated with the old term and the remount reset the input to match it. Type a new term into the header field, click **Semantic** without pressing enter, and the typed text was gone with no indication it had been discarded. Pre-existing — the closure predated Increment 24 and was untouched by it — and found by the adversarial review of PR #155 while reviewing the capability gate wrapped around the same control. **Resolved:** the query is now a `() => string` read when a mode is chosen rather than a string captured when the selector renders, matching what `main.ts`'s submit handler already does, and falling back to the mounted query only where the document has no header input. Two regression tests, one of which fails against the exact pre-fix closure. -- **`startHarness` drew ports `fetch` refuses (RESOLVED in ADR-0140); the unexplained whole-file failure is still open.** Two signatures were filed here, deliberately not as one cause, and that judgement held. **Signature A is resolved.** WHATWG Fetch blocks eighty-two ports and undici enforces the list on the port number alone, before opening a socket — so `server.listen(0)` could bind, listen and answer raw TCP while `fetch` still refused, surfacing as `TypeError: fetch failed` / `Error: bad port` at the harness’s first request rather than at the listen that caused it. Whether it can happen at all is a property of the host’s dynamic port range: a typical Linux CI range (32768–60999) contains no blocked port, while the Windows range in use here (1024–15000) contains nineteen, which is the whole of "green on CI, flaky locally". The guard that already existed was incomplete in a way that still failed — its hand-observed set of eighteen ports was the spec list intersected with one machine’s range **minus `6679`** — and it retried unboundedly, had no behaviour on exhaustion, and had been copy-pasted into `auth-signin-schema.integration.test.ts`, so the missing port had to be found twice. `packages/api/test/listen.ts` now owns port acquisition for both sites: the spec-complete eighty-two ports (verified by sweeping all 65535 through the real `fetch` on Node v24.15.0), a bounded twenty attempts, each rejected listener closed before the next is asked for, a guarded `address()` read in place of the `as AddressInfo` cast, and an exhaustion error naming the attempts and rejected ports and nothing else. **Signature B is not resolved and was not folded in.** A file still fails with `'test failed'`, no assertion, no stack and none of its own tests reported. Twenty consecutive full runs gave five failures: on pre-fix code one signature A (`auth.test.js`) and three signature B; on post-fix code one signature B and no signature A. The port fix removes A and leaves B exactly where it was, which is the evidence that they are two defects. Four different files were hit (`move-explanation-route`, `tournament-commentary-route`, `bot-detection-analyze`, `anti-cheat-analysis`), sharing no import beyond `./helpers`; each died in 589–703 ms with no test of its own reporting and no stderr. Refuted with evidence: ephemeral-port exhaustion (113 sockets in TIME_WAIT against a 13977-port range), a `Promise.race` loser becoming an unhandled rejection (`race` subscribes to every promise, confirmed on v24.15.0), a throwing `after`/`afterEach` hook (the affected files use none), and a double `close()` rejecting (awaited in a `finally`, it would be attributed to that test with a stack). A second bounded pass — twelve more full runs under the TAP reporter with a preload recording `uncaughtException`, `unhandledRejection` and any non-zero exit — produced twelve clean runs and captured nothing at the time. **A follow-up increment (`claude/node-test-signature-b`) then captured the defect directly, three more times, on three files never previously implicated** (`rate-limit-atomicity`, `dependency-parity`, `studies-api`) — seven distinct files observed with this symptom to date. Occurrences across seven distinct files make a shared or cross-cutting path more plausible and make a defect confined to one test file less likely, but do not exclude file-specific inputs or lifecycle interactions. An instrumented preload (`packages/api/test/diagnostics/signature-b-preload.cjs`) hooking process-level events — `process.exit`, `process.abort`, `process.kill`, `uncaughtExceptionMonitor` (passively observing uncaught exceptions and fatal unhandled rejections), `warning`, `beforeExit`, and Node’s own unconditional `exit` — showed **none of the hooks active at the time fired** on any of the three historical captures (though `process.abort()` was not wrapped in those initial runs and is now covered for future occurrences). A synthetic `process.exit(1)`-before-registration fixture reproduces the identical silent shape; every other synthetic mechanism tried (a post-test async throw, an emitter `'error'` with its listener removed, a synchronous module-load throw, a delayed `SIGKILL`) prints a visibly different diagnostic line, stack, or partial test output that the real defect never shows. This narrows the investigated possibilities while leaving the root cause unresolved: the per-file child process (`node --test` spawns one per file, confirmed by distinct PIDs) was not terminated by `process.exit`, uncaught exceptions, or fatal unhandled rejections, and future runs with `process.abort` instrumentation will record whether abort was called through JS; an absent record narrows in-runtime JS termination but cannot alone prove external termination without corroborating child exit status/signal data or OS-level crash evidence (e.g. distinguishing an external kill or uncatchable signal from a native C++/V8 crash). The machine had roughly 2.5 GB of 15.7 GB RAM free at capture time with several other agents’ processes concurrently running, which is circumstantially consistent with resource contention, but no crash was recorded in the Windows Application or System event logs in that window, so the exact external trigger is still not established. No fix was invented — the forbidden responses (sleeps, whole-file retries, lowering concurrency) would only hide the unresolved root cause, whose origin is not yet established. **A further increment then crossed the parent/child boundary the earlier work stopped at, and found the evidence had been there all along:** Node's runner attaches the child's `exitCode` and `signal` to the `ERR_TEST_FAILURE` it throws, and the `spec` reporter discards them — `formatError` replaces the error with `error.cause`, the bare string `'test failed'` — while the built-in `tap` reporter serializes them, so running `spec` to stdout and `tap` to a file recovers the exit status with no custom reporter and no patched internals. Exit codes were measured on this platform rather than assumed: `process.abort()` gives `134`, `Stop-Process -Force` gives `4294967295`, NTSTATUS faults surface as raw unsigned values such as `3221225477` (`0xC0000005`) — and `1` is produced alike by an uncaught exception, `process.exit(1)`, `taskkill /F` and `process.kill`, so it identifies nothing on its own and is classified `inconclusive`. `signature-b-correlate.cjs` joins the parent's TAP record to the child's JSONL log on the test file path (which also yields the child PID) and states what the pair does and does not establish; where the exit code is ambiguous, a child that reached `preload-installed` and then logged nothing still excludes `process.exit` and an uncaught exception, because both would have left a record and fired Node's `exit` event. A bounded pass of 20 runs under this instrumentation produced 0 captures — which bounds the rate and proves nothing: treating the historical ~1-in-5 as an independent per-run rate, zero captures in 20 runs has probability `(4/5)^20 ≈ 1.2%`, and independence is an assumption rather than an established fact; it ran at 3084–3834 MB free against roughly 2.5 GB at the historical captures, consistent with the resource-contention hypothesis but not evidence for it. **Signature B stays UNRESOLVED**; what changed is that the next occurrence is readable rather than silent. **During M15 Increment 46 validation, Signature B was directly observed three more times, broadening the observed scope to `packages/persistence`:** `search-backfill.integration.test.ts`, `learning.integration.test.ts`, and `test-database.integration.test.ts` each died with the documented bare whole-file `'test failed'` with zero tests reporting and no assertion or stack, and each passed standalone against the exact database state it died on. This confirms the defect is not specific to `packages/api` or its HTTP server test harness. **M15 Increment 47 hardened the diagnostic correlator across 26 bounded runs:** 1 baseline full-suite run, 20 sequential runs under `run-signature-b-pass.mjs`, and 5 concurrent repository-native load runs produced 0 captures (which bounds the rate under those conditions and does not resolve the defect). Code inspection and falsification identified and fixed four correlator blind spots in `packages/api/test/diagnostics/signature-b-correlate.cjs`: (1) cross-directory basename fallback could falsely match a child log from another directory when the target had slashes; (2) Windows path case folding was host-dependent (`process.platform === 'win32'`), breaking cross-platform correlation when logs were analyzed on a different OS; (3) signed 32-bit Windows NTSTATUS exit codes (e.g. `0xC0000005` surfacing as `-1073741819`) were not normalized to unsigned values; and (4) quoted TAP YAML scalar tokens lost exit code or duration numbers. The targeted test suite was expanded from 23 to 33 tests (31 pass, 2 skip, 0 fail), killing 4 falsification mutations. **Signature B remains UNRESOLVED;** no production fix was invented, and the next occurrence will be correlated without cross-platform or exit-code blind spots. See ADR-0140 §4. +- **`startHarness` drew ports `fetch` refuses (RESOLVED in ADR-0140); the unexplained whole-file failure is still open.** Two signatures were filed here, deliberately not as one cause, and that judgement held. **Signature A is resolved.** WHATWG Fetch blocks eighty-two ports and undici enforces the list on the port number alone, before opening a socket — so `server.listen(0)` could bind, listen and answer raw TCP while `fetch` still refused, surfacing as `TypeError: fetch failed` / `Error: bad port` at the harness’s first request rather than at the listen that caused it. Whether it can happen at all is a property of the host’s dynamic port range: a typical Linux CI range (32768–60999) contains no blocked port, while the Windows range in use here (1024–15000) contains nineteen, which is the whole of "green on CI, flaky locally". The guard that already existed was incomplete in a way that still failed — its hand-observed set of eighteen ports was the spec list intersected with one machine’s range **minus `6679`** — and it retried unboundedly, had no behaviour on exhaustion, and had been copy-pasted into `auth-signin-schema.integration.test.ts`, so the missing port had to be found twice. `packages/api/test/listen.ts` now owns port acquisition for both sites: the spec-complete eighty-two ports (verified by sweeping all 65535 through the real `fetch` on Node v24.15.0), a bounded twenty attempts, each rejected listener closed before the next is asked for, a guarded `address()` read in place of the `as AddressInfo` cast, and an exhaustion error naming the attempts and rejected ports and nothing else. **Signature B is not resolved and was not folded in.** A file still fails with `'test failed'`, no assertion, no stack and none of its own tests reported. Twenty consecutive full runs gave five failures: on pre-fix code one signature A (`auth.test.js`) and three signature B; on post-fix code one signature B and no signature A. The port fix removes A and leaves B exactly where it was, which is the evidence that they are two defects. Four different files were hit (`move-explanation-route`, `tournament-commentary-route`, `bot-detection-analyze`, `anti-cheat-analysis`), sharing no import beyond `./helpers`; each died in 589–703 ms with no test of its own reporting and no stderr. Refuted with evidence: ephemeral-port exhaustion (113 sockets in TIME_WAIT against a 13977-port range), a `Promise.race` loser becoming an unhandled rejection (`race` subscribes to every promise, confirmed on v24.15.0), a throwing `after`/`afterEach` hook (the affected files use none), and a double `close()` rejecting (awaited in a `finally`, it would be attributed to that test with a stack). A second bounded pass — twelve more full runs under the TAP reporter with a preload recording `uncaughtException`, `unhandledRejection` and any non-zero exit — produced twelve clean runs and captured nothing at the time. **A follow-up increment (`claude/node-test-signature-b`) then captured the defect directly, three more times, on three files never previously implicated** (`rate-limit-atomicity`, `dependency-parity`, `studies-api`) — seven distinct files observed with this symptom to date. Occurrences across seven distinct files make a shared or cross-cutting path more plausible and make a defect confined to one test file less likely, but do not exclude file-specific inputs or lifecycle interactions. An instrumented preload (`packages/api/test/diagnostics/signature-b-preload.cjs`) hooking process-level events — `process.exit`, `process.abort`, `process.kill`, `uncaughtExceptionMonitor` (passively observing uncaught exceptions and fatal unhandled rejections), `warning`, `beforeExit`, and Node’s own unconditional `exit` — showed **none of the hooks active at the time fired** on any of the three historical captures (though `process.abort()` was not wrapped in those initial runs and is now covered for future occurrences). A synthetic `process.exit(1)`-before-registration fixture reproduces the identical silent shape; every other synthetic mechanism tried (a post-test async throw, an emitter `'error'` with its listener removed, a synchronous module-load throw, a delayed `SIGKILL`) prints a visibly different diagnostic line, stack, or partial test output that the real defect never shows. This narrows the investigated possibilities while leaving the root cause unresolved: the per-file child process (`node --test` spawns one per file, confirmed by distinct PIDs) was not terminated by `process.exit`, uncaught exceptions, or fatal unhandled rejections, and future runs with `process.abort` instrumentation will record whether abort was called through JS; an absent record narrows in-runtime JS termination but cannot alone prove external termination without corroborating child exit status/signal data or OS-level crash evidence (e.g. distinguishing an external kill or uncatchable signal from a native C++/V8 crash). The machine had roughly 2.5 GB of 15.7 GB RAM free at capture time with several other agents’ processes concurrently running, which is circumstantially consistent with resource contention, but no crash was recorded in the Windows Application or System event logs in that window, so the exact external trigger is still not established. No fix was invented — the forbidden responses (sleeps, whole-file retries, lowering concurrency) would only hide the unresolved root cause, whose origin is not yet established. **A further increment then crossed the parent/child boundary the earlier work stopped at, and found the evidence had been there all along:** Node's runner attaches the child's `exitCode` and `signal` to the `ERR_TEST_FAILURE` it throws, and the `spec` reporter discards them — `formatError` replaces the error with `error.cause`, the bare string `'test failed'` — while the built-in `tap` reporter serializes them, so running `spec` to stdout and `tap` to a file recovers the exit status with no custom reporter and no patched internals. Exit codes were measured on this platform rather than assumed: `process.abort()` gives `134`, `Stop-Process -Force` gives `4294967295`, NTSTATUS faults surface as raw unsigned values such as `3221225477` (`0xC0000005`) — and `1` is produced alike by an uncaught exception, `process.exit(1)`, `taskkill /F` and `process.kill`, so it identifies nothing on its own and is classified `inconclusive`. `signature-b-correlate.cjs` joins the parent's TAP record to the child's JSONL log on the test file path (which also yields the child PID) and states what the pair does and does not establish; where the exit code is ambiguous, a child that reached `preload-installed` and then logged nothing still excludes `process.exit` and an uncaught exception, because both would have left a record and fired Node's `exit` event. A bounded pass of 20 runs under this instrumentation produced 0 captures — which bounds the rate and proves nothing: treating the historical ~1-in-5 as an independent per-run rate, zero captures in 20 runs has probability `(4/5)^20 ≈ 1.2%`, and independence is an assumption rather than an established fact; it ran at 3084–3834 MB free against roughly 2.5 GB at the historical captures, consistent with the resource-contention hypothesis but not evidence for it. **Signature B stays UNRESOLVED**; what changed is that the next occurrence is readable rather than silent. **During M15 Increment 46 validation, Signature B was directly observed three more times, broadening the observed scope to `packages/persistence`:** `search-backfill.integration.test.ts`, `learning.integration.test.ts`, and `test-database.integration.test.ts` each died with the documented bare whole-file `'test failed'` with zero tests reporting and no assertion or stack, and each passed standalone against the exact database state it died on. This confirms the defect is not specific to `packages/api` or its HTTP server test harness. **M15 Increment 47 hardened the diagnostic correlator across 26 bounded runs:** 1 baseline full-suite run, 20 sequential runs under `run-signature-b-pass.mjs`, and 5 concurrent repository-native load runs produced 0 captures (which bounds the rate under those conditions and does not resolve the defect). Code inspection and falsification identified and fixed four correlator blind spots in `packages/api/test/diagnostics/signature-b-correlate.cjs`: (1) cross-directory basename fallback could falsely match a child log from another directory when the target had slashes; (2) Windows path case folding was host-dependent (`process.platform === 'win32'`), breaking cross-platform correlation when logs were analyzed on a different OS; (3) signed 32-bit Windows NTSTATUS exit codes (e.g. `0xC0000005` surfacing as `-1073741819`) were not normalized to unsigned values; and (4) quoted TAP YAML scalar tokens lost exit code or duration numbers. The targeted test suite was expanded from 23 to 36 tests (34 pass, 2 skip, 0 fail), killing 4 falsification mutations. **Signature B remains UNRESOLVED;** no production fix was invented, and the next occurrence will be correlated without cross-platform or exit-code blind spots. See ADR-0140 §4. - **Isolated-database test teardown dropped databases out from under connections that had not finished closing (RESOLVED in M15 Increment 45).** `withDatabase` in `packages/persistence/test/variant-migrations.integration.test.ts` ended its pool and then immediately ran `DROP DATABASE ... WITH (FORCE)`. `pool.end()` does not wait for its clients to close: in pg 8.22.0 `_pulseQueue` reaches the end callback in the same synchronous turn in which `_remove` filters the last client out of `_clients`, while `client.end()` has only queued the Terminate byte — instrumentation recorded **zero of four `remove` events fired at the moment `end()` resolved**. The drop could therefore still find a backend attached; `FORCE` terminated it, and the resulting `FATAL` arrived on a socket whose pool still had `idleListener` attached, which `pg` re-emitted as `pool.emit('error')` — an unhandled EventEmitter error that `node:test` attributed to whichever test was running rather than to the teardown that caused it. It surfaced as intermittent `terminating connection due to administrator command` failures in `postgres integration (persistence)` during M15 Increment 44, on a different test each run, which is the signature of a race rather than a broken assertion. The same shape existed in `packages/api/test/auth-signin-schema.integration.test.ts`, which had absorbed SQLSTATE 57P01 with a `pool.on('error', ...)` listener — a symptom fix for the same cause. **Resolved in Increment 45:** a shared `withTestDatabase` helper (`@chess-platform/persistence/test-support`) ends the pool under a bound, waits for `pg_stat_activity` to report the database unused, and drops it *without* `FORCE`. Measured on PostgreSQL 16.14, a plain drop against a still-attached backend fails with SQLSTATE 55006 and leaves that connection untouched, where `FORCE` succeeds by killing it — so the change trades a quiet, harmful success for a loud, harmless failure. FORCE remains only on the emergency path that guarantees the disposable database is still dropped once teardown has already failed — best effort, since that last drop runs inside a `catch` so it cannot bury the error being reported. The 57P01 absorber is deleted, because the corrected lifecycle never causes one. `createPool` and `migrate` are unchanged, and no migration was added. - **The persistence integration suite was not idempotent against a reused database (RESOLVED in M15 Increment 46).** Recorded as a known defect by Increment 45 and left open there. Against a fresh PostgreSQL 16 database the suite passed; a second run against the *same* database failed nine tests, deterministically — measured on 16.14 before any edit as **173 pass / 0 fail** then **164 pass / 9 fail**. CI provisions a fresh server per run, so it never surfaced there. One contract was being broken in two directions: a suite sharing `chess_test` must remove every row it created and remove nothing else. `achievements.integration.test.ts` broke the second half with an unqualified `DELETE FROM users` in `beforeEach` — `games.white_id` and `games.black_id` are the only references to `users` without `ON DELETE CASCADE` (thirty-one FKs point at `users`; twenty-nine cascade; the two that do not are both on `games`), so one game left behind by `pg.integration.test.ts` aborted the wipe with SQLSTATE 23503 before any assertion ran, and the same statement destroyed the bot accounts migration 0021 seeds, which nothing restores because `migrate` has already recorded 0021 as applied. `pg/identity-tokens.test.ts` and `tournaments.pg.integration.test.ts` broke the first half, leaving fixed primary keys behind and colliding on `users_pkey` and `tournaments_pkey` (the latter surfacing through the repository's compare-and-set as `VersionConflictError`). **Resolved in Increment 46:** suites that legitimately share the database delete exactly their own rows through `withSharedDatabase` (`packages/persistence/src/test-support/fixtures.ts`), the sibling of Increment 45's `withTestDatabase` and heir to its precedence rule — a cleanup failure never replaces the assertion that actually failed, and never disappears either. `pg.integration.test.ts` moved to disposable databases instead, because it cannot meet the cleanup half at all: it appends to `game_events`, which is append-only by production trigger, so cleaning up after itself would have meant weakening a production safety rule to suit a test. `users-batch`, `anti-cheat`, `bot-reports` and `analysis-cache` were corrected for the same contract though none of them ever failed — fresh `uuidv7()` ids meant their leaked rows could not collide, so the tables merely grew on every run. Serialization was not the fix and was not introduced: `--test-concurrency=1` is a pre-existing documented invariant and the second run failed identically under it. Acceptance is three consecutive runs against one database with no reset between them (186/186/186), after which only the three migration-seeded bot accounts remain, with no leaked disposable databases and no lingering backends; falsification killed 17 of 20 mutations. No production code, migration, checksum, constraint or repository conflict semantic changed, and no migration was added. - **`ApiServer.listen` registered no `'error'` handler, so a failed bind hung and raised an uncaught event (RESOLVED in ADR-0140 §5).** `packages/api/src/server.ts` resolved its promise from the `listening` callback only and built it with no reject path. A bind failing asynchronously (`EADDRINUSE`, `EMFILE`) left the promise pending forever and, with no `'error'` listener on the `http.Server`, was re-raised as an uncaught exception. Found while investigating ADR-0140 and independently raised by the Qodo review of PR #21. **Resolved in the same increment** rather than deferred, because ADR-0140 §2's bounded, diagnosable acquisition is not true without it — the retry can only report a bind error if the listener it is handed rejects. A one-shot `'error'` listener now rejects and is removed once listening, so later server errors keep their previous semantics rather than being swallowed by a `reject` on a settled promise. The regression test fails against the exact pre-fix code, through an uncaught `ERR_UNHANDLED_ERROR`. diff --git a/packages/api/test/diagnostics/signature-b-correlate.cjs b/packages/api/test/diagnostics/signature-b-correlate.cjs index 74e64d1a..35ce8ee8 100644 --- a/packages/api/test/diagnostics/signature-b-correlate.cjs +++ b/packages/api/test/diagnostics/signature-b-correlate.cjs @@ -40,44 +40,58 @@ const path = require('node:path'); const MAX_RECORDS = 200; /** - * A path with both separators normalised to `/`, so a capture taken on one platform can be read on - * another. `path.basename` and `path.normalize` use only the host's separator, so on Linux they - * return a Windows path unchanged and every comparison against it silently fails. + * A path with separators normalised to `/`. For Windows paths (identified by drive letter, UNC, + * or explicit flag/metadata), backslashes are directory separators and are converted to `/`. + * For POSIX paths, backslashes are valid filename characters and are preserved to avoid + * splitting a single filename into multiple path segments. * * @param {string} filePath + * @param {boolean} [isWindows] * @returns {string} */ -function normalizePath(filePath) { - return String(filePath).replace(/\\/g, '/').replace(/^\.\//, ''); +function normalizePath(filePath, isWindows) { + const str = String(filePath); + const win = isWindows !== undefined ? isWindows : isWindowsPath(str); + if (win) { + return str.replace(/\\/g, '/').replace(/^\.\//, ''); + } + return str.replace(/^\.\//, ''); } /** * Last path segment, platform-independently. * * @param {string} filePath + * @param {boolean} [isWindows] * @returns {string} */ -function fileKey(filePath) { - const segments = normalizePath(filePath).split('/'); +function fileKey(filePath, isWindows) { + const segments = normalizePath(filePath, isWindows).split('/'); return segments[segments.length - 1] ?? ''; } /** - * Detect whether a path string represents a Windows path based strictly on path syntax: - * backslashes, a Windows drive prefix (e.g. `C:/`), or a UNC prefix (`//`). + * Detect whether a path string represents a Windows path based strictly on unambiguous + * Windows path syntax: a Windows drive prefix (e.g. `C:/` or `C:\`) or a UNC prefix (`//` or `\\`). * Analyzer host platform (`process.platform`) is deliberately omitted so that POSIX captures * analyzed on Windows preserve case-sensitivity, and Windows captures analyzed on Linux * apply case-insensitive matching. * + * Backslash alone is NOT treated as proof of Windows origin because POSIX permits backslashes + * in filenames. A path starting with a single slash is an absolute POSIX path and is never Windows. + * For relative paths, Windows origin is determined via child-log capture metadata or explicit markers. + * * @param {string} filePath * @returns {boolean} */ function isWindowsPath(filePath) { const str = String(filePath); + if (str.startsWith('/') && !str.startsWith('//')) { + return false; + } return ( - str.includes('\\') || - /^[a-zA-Z]:(?:\/|$)/.test(str) || - /^\/\/[^/]/.test(str) + /^[a-zA-Z]:(?:[/\\]|$)/.test(str) || + /^(?:\\\\|\/\/)[^/\\\\]/.test(str) ); } @@ -262,10 +276,10 @@ function parseTapFailures(tapText) { const parsedDur = dur === null || dur === '~' || dur === '' ? null : Number(dur); out.push({ file: rawName.trim().replace(/\\\\/g, '\\'), - exitCode: parsedExit !== null && Number.isNaN(parsedExit) ? null : parsedExit, + exitCode: parsedExit !== null && Number.isFinite(parsedExit) ? parsedExit : null, signal: sig === null || sig === '~' ? null : sig, failureType: unquote(scalar('failureType')), - durationMs: parsedDur !== null && Number.isNaN(parsedDur) ? null : parsedDur, + durationMs: parsedDur !== null && Number.isFinite(parsedDur) ? parsedDur : null, }); } return out; @@ -307,10 +321,25 @@ function readChildLogs(logDir) { continue; } if (typeof record.testFile !== 'string' || record.testFile === '') continue; - const isWin = isWindowsPath(record.testFile); + const isWin = typeof record.isWindows === 'boolean' + ? record.isWindows + : (typeof record.platform === 'string' + ? record.platform === 'win32' + : isWindowsPath(record.testFile)); // Keyed on the child's whole path, not its basename. A suite run spans directories, and // `foo/a.test.js` and `bar/a.test.js` are different files whose events must never be merged. - const key = normalizePath(record.testFile); + const normalized = normalizePath(record.testFile, isWin); + let key = normalized; + if (isWin) { + const lower = normalized.toLowerCase(); + for (const existingKey of byFile.keys()) { + const entry = byFile.get(existingKey); + if (entry?.isWindows && existingKey.toLowerCase() === lower) { + key = existingKey; + break; + } + } + } const existing = byFile.get(key) ?? { pid: null, kinds: [], isWindows: false }; existing.pid = typeof record.pid === 'number' ? record.pid : existing.pid; if (typeof record.kind === 'string') existing.kinds.push(record.kind); @@ -338,16 +367,16 @@ function readChildLogs(logDir) { */ function matchChild(childLogs, parentFile) { const isWantedWin = isWindowsPath(parentFile); - const wanted = normalizePath(parentFile); const entries = [...childLogs.entries()]; + const hasWindowsChild = entries.some(([, child]) => child?.isWindows); + const isWinContext = isWantedWin || hasWindowsChild; + const wanted = normalizePath(parentFile, isWinContext); const bySuffix = entries.filter(([key, child]) => { - if (child?.isWindows || isWindowsPath(key) || isWantedWin || isWindowsPath(wanted)) { - const keyNorm = key.toLowerCase(); - const wantedNorm = wanted.toLowerCase(); - return keyNorm === wantedNorm || keyNorm.endsWith(`/${wantedNorm}`); - } - return key === wanted || key.endsWith(`/${wanted}`); + const isWin = Boolean(child?.isWindows || isWantedWin); + const wantedNorm = isWin ? normalizePath(parentFile, true).toLowerCase() : normalizePath(parentFile, false); + const keyNorm = isWin ? key.toLowerCase() : key; + return keyNorm === wantedNorm || keyNorm.endsWith(`/${wantedNorm}`); }); if (bySuffix.length === 1) return { child: bySuffix[0][1], ambiguous: false, candidates: 1 }; if (bySuffix.length > 1) return { child: null, ambiguous: true, candidates: bySuffix.length }; @@ -356,10 +385,11 @@ function matchChild(childLogs, parentFile) { // When the parent specified a directory path and bySuffix found 0 matches, that file has no child log. // Matching a different directory's child would be a false cross-directory attribution. if (!wanted.includes('/')) { - const base = fileKey(wanted); const byBase = entries.filter(([key, child]) => { - const kBase = fileKey(key); - if (child?.isWindows || isWindowsPath(key) || isWantedWin || isWindowsPath(wanted)) { + const isWin = Boolean(child?.isWindows || isWantedWin); + const base = fileKey(parentFile, isWin); + const kBase = fileKey(key, isWin); + if (isWin) { return kBase.toLowerCase() === base.toLowerCase(); } return kBase === base; diff --git a/packages/api/test/diagnostics/signature-b-correlate.test.ts b/packages/api/test/diagnostics/signature-b-correlate.test.ts index 3bfd5a25..915a555b 100644 --- a/packages/api/test/diagnostics/signature-b-correlate.test.ts +++ b/packages/api/test/diagnostics/signature-b-correlate.test.ts @@ -848,12 +848,15 @@ test('correlator: path suffix matching for Windows-origin paths is case-insensit } }); -test('correlator: isWindowsPath identifies Windows drive letters, UNC paths, and backslashes', () => { +test('correlator: isWindowsPath identifies Windows drive letters and UNC paths without false-positive on POSIX backslash filenames', () => { assert.equal(correlator.isWindowsPath('C:/repo/test.js'), true); + assert.equal(correlator.isWindowsPath('C:\\repo\\test.js'), true); assert.equal(correlator.isWindowsPath('d:/repo/test.js'), true); assert.equal(correlator.isWindowsPath('//server/share/test.js'), true); - assert.equal(correlator.isWindowsPath('Dist-Test\\Test\\Foo.test.js'), true); + assert.equal(correlator.isWindowsPath('\\\\server\\share\\test.js'), true); assert.equal(correlator.isWindowsPath('/home/runner/repo/test.js'), false); + assert.equal(correlator.isWindowsPath('/home/runner/repo/weird\\name.test.js'), false); + assert.equal(correlator.isWindowsPath('weird\\name.test.js'), false); assert.equal(correlator.isWindowsPath('dist-test/test.js'), false); }); @@ -883,8 +886,8 @@ test('correlator: relative backslash-delimited Windows child path matches case-i const logDir = fs.mkdtempSync(path.join(os.tmpdir(), 'sigb-relwin-')); try { const lines = [ - { kind: 'start', pid: 444, testFile: 'Dist-Test\\Test\\Studies-Api.Test.js' }, - { kind: 'preload-installed', pid: 444, testFile: 'Dist-Test\\Test\\Studies-Api.Test.js' }, + { kind: 'start', pid: 444, testFile: 'Dist-Test\\Test\\Studies-Api.Test.js', isWindows: true }, + { kind: 'preload-installed', pid: 444, testFile: 'Dist-Test\\Test\\Studies-Api.Test.js', isWindows: true }, ]; fs.writeFileSync(path.join(logDir, 'run-relwin.jsonl'), `${lines.map((l) => JSON.stringify(l)).join('\n')}\n`); const logs = correlator.readChildLogs(logDir); @@ -902,3 +905,108 @@ test('correlator: relative backslash-delimited Windows child path matches case-i fs.rmSync(logDir, { recursive: true, force: true }); } }); + +test('correlator: POSIX paths with backslashes in filenames preserve filename structure and case sensitivity', () => { + const logDir = fs.mkdtempSync(path.join(os.tmpdir(), 'sigb-posix-backslash-')); + try { + const lines = [ + { kind: 'start', pid: 666, testFile: '/home/runner/repo/dist-test/test/weird\\name.test.js' }, + { kind: 'preload-installed', pid: 666, testFile: '/home/runner/repo/dist-test/test/weird\\name.test.js' }, + ]; + fs.writeFileSync(path.join(logDir, 'run-posix-bs.jsonl'), `${lines.map((l) => JSON.stringify(l)).join('\n')}\n`); + const logs = correlator.readChildLogs(logDir); + + assert.equal(correlator.normalizePath('/home/runner/repo/dist-test/test/weird\\name.test.js'), '/home/runner/repo/dist-test/test/weird\\name.test.js'); + + // Matching with correct case + const matchOk = correlator.correlate( + correlator.parseTapFailures(tapFailure('dist-test/test/weird\\name.test.js', '1')), + logs, + ); + assert.equal(matchOk.length, 1); + assert.equal(matchOk[0]?.child.pid, 666); + assert.equal(matchOk[0]?.child.logFound, true); + + // Mismatched case must not match on POSIX + const matchCaseFail = correlator.correlate( + correlator.parseTapFailures(tapFailure('dist-test/test/Weird\\name.test.js', '1')), + logs, + ); + assert.equal(matchCaseFail.length, 1); + assert.equal(matchCaseFail[0]?.child.logFound, false); + } finally { + fs.rmSync(logDir, { recursive: true, force: true }); + } +}); + +test('correlator: readChildLogs aggregates casing variants of the same Windows child file into a single entry', () => { + const logDir = fs.mkdtempSync(path.join(os.tmpdir(), 'sigb-case-aggregate-')); + try { + const lines = [ + { kind: 'start', pid: 777, testFile: 'C:\\Repo\\Dist-Test\\Test\\Studies-Api.Test.js' }, + { kind: 'preload-installed', pid: 777, testFile: 'c:\\repo\\dist-test\\test\\studies-api.test.js' }, + ]; + fs.writeFileSync(path.join(logDir, 'run-case-agg.jsonl'), `${lines.map((l) => JSON.stringify(l)).join('\n')}\n`); + const logs = correlator.readChildLogs(logDir); + + // Both lines must aggregate into one entry rather than creating conflicting entries + assert.equal(logs.size, 1); + + const resolved = correlator.correlate( + correlator.parseTapFailures(tapFailure('dist-test/test/studies-api.test.js', '1')), + logs, + ); + assert.equal(resolved.length, 1); + assert.equal(resolved[0]?.child.pid, 777); + assert.equal(resolved[0]?.child.logFound, true); + assert.equal(resolved[0]?.child.ambiguous, false, 'casing variants of the same child must not cause ambiguity'); + assert.deepEqual(resolved[0]?.child.kinds, ['start', 'preload-installed']); + } finally { + fs.rmSync(logDir, { recursive: true, force: true }); + } +}); + +test('correlator: parseTapFailures rejects non-finite exitCode and duration_ms scalars', () => { + const nonFiniteTap = [ + 'TAP version 13', + '# Subtest: dist-test/test/non-finite.test.js', + 'not ok 1 - dist-test/test/non-finite.test.js', + ' ---', + ' duration_ms: Infinity', + ' failureType: testCodeFailure', + ' exitCode: Infinity', + ' signal: ~', + ' error: test failed', + ' code: ERR_TEST_FAILURE', + ' ...', + '# Subtest: dist-test/test/neg-infinity.test.js', + 'not ok 2 - dist-test/test/neg-infinity.test.js', + ' ---', + ' duration_ms: -Infinity', + ' failureType: testCodeFailure', + ' exitCode: -Infinity', + ' signal: ~', + ' error: test failed', + ' code: ERR_TEST_FAILURE', + ' ...', + '# Subtest: dist-test/test/nan.test.js', + 'not ok 3 - dist-test/test/nan.test.js', + ' ---', + ' duration_ms: NaN', + ' failureType: testCodeFailure', + ' exitCode: NaN', + ' signal: ~', + ' error: test failed', + ' code: ERR_TEST_FAILURE', + ' ...', + ].join('\n'); + + const failures = correlator.parseTapFailures(nonFiniteTap); + assert.equal(failures.length, 3); + assert.equal(failures[0]?.exitCode, null); + assert.equal(failures[0]?.durationMs, null); + assert.equal(failures[1]?.exitCode, null); + assert.equal(failures[1]?.durationMs, null); + assert.equal(failures[2]?.exitCode, null); + assert.equal(failures[2]?.durationMs, null); +}); From a5d396d7047edd51a497b3349d60d96d684ec82c Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Fri, 4 Sep 2026 23:33:23 +0300 Subject: [PATCH 09/13] fix(api-diagnostics): preserve parent separator semantics in candidate matching and reject forward-slash UNC syntax --- docs/PROJECT_STATE.md | 4 +-- docs/ROADMAP.md | 2 +- .../diagnostics/signature-b-correlate.cjs | 14 +++++------ .../diagnostics/signature-b-correlate.test.ts | 25 ++++++++++++++++++- 4 files changed, 33 insertions(+), 12 deletions(-) diff --git a/docs/PROJECT_STATE.md b/docs/PROJECT_STATE.md index 7fcd199d..788d3a64 100644 --- a/docs/PROJECT_STATE.md +++ b/docs/PROJECT_STATE.md @@ -59,8 +59,8 @@ parent TAP diagnostic logs and child JSONL preloads were matched and parsed: values to `null`. **Verification and mutation falsification.** -- Targeted correlator suite (`signature-b-correlate.test.ts`): expanded from 23 to 36 tests - (34 pass, 2 skip for deliberate platform-gated checks, 0 fail). +- Targeted correlator suite (`signature-b-correlate.test.ts`): expanded from 23 to 37 tests + (35 pass, 2 skip for deliberate platform-gated checks, 0 fail). - Mutation falsification: four targeted mutations reversing each of the four hardened behaviors were verified to be killed by the expanded test suite. - Monorepo validation: full build, lint, counts, and guard scripts pass cleanly. diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 2d1df2d9..6d881ea1 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -1349,7 +1349,7 @@ Debt observed during M14. Each states what is known, not what is planned; items - **The published capability document could omit a composed feature, and the client offered controls that could not work (RESOLVED in M15 Increment 24 / ADR-0132).** Left open by Increment 23 above. `capabilitiesView` (`packages/api/src/presenters.ts`) took a hand-written `Pick`, so a feature never added to it was invisible to `GET /v1/capabilities` and nothing complained. ADR-0131 judged that "a narrower failure than a 503 — the feature works for anyone who calls the route directly", and deferred it on the grounds that closing it meant first deciding which optional dependencies are user-facing capabilities. **Both halves of that were wrong.** The failure is narrower only when the client can still reach the feature, and a live instance already existed where it could not: `GET /v1/search` served three modes from two independently-gated dependency sets behind one published `search` flag, so a deployment running the Helm chart’s `search.semanticEnabled: false` advertised search while the client offered two mode buttons whose every request answered 503. And the judgement, while genuinely underivable, can be made **unskippable**, which was the property wanted. `semanticSearch` is now published from both of its dependencies, `Exclude[0] | NotAPublishedCapability>` must be `never` so a new optional dependency cannot compile until someone gives it a flag or records why it has none, and a behavioural guard requires every capability source to change what the document publishes — because a key can sit in that parameter unread, which the compile-time half cannot see. The mutation ledger lives in ADR-0132 and is not restated here. - **The search surface was not gated on `capabilities.search`, so an absolute kill switch still showed a search box (RESOLVED in M15 Increment 24 / ADR-0132 §5).** `SEARCH_ENABLED=0` — the chart's `search.enabled: false`, an absolute kill switch per ADR-0055 — leaves `searchRepository` unconstructed and `GET /v1/search` answering 503 on every mode, keyword included. The entry point was the persistent header form in `packages/web/index.html`, present on every page; being a `` rather than an `a[data-route]`, `NAV_CAPABILITY_MAP` could not reach it, which is why the first pass at Increment 24 gated the semantic and hybrid *modes* and left keyword ungated — the same defect class one mode over. Raised by the Qodo review of PR #155. **Fixed in the same increment rather than deferred:** the form now ships `hidden` and is revealed by `applySearchCapability` only on an explicit `search: true`; the route renders an honest unavailable notice and issues no request; and keyword search waits for the capability answer, reversing this increment's own earlier latency decision, because knowing whether a request is pointless requires having asked. A markup-contract test pins the `hidden` attribute, since the gate depends on it and every other test passes without it. - **Clicking a search mode discarded text typed since the page loaded (RESOLVED in a follow-up to M15 Increment 24 / ADR-0132).** `createModeInput` in `packages/web/src/app/search-mount.ts` closed over the query captured when the route mounted, so `navigateToSearchMode` navigated with the old term and the remount reset the input to match it. Type a new term into the header field, click **Semantic** without pressing enter, and the typed text was gone with no indication it had been discarded. Pre-existing — the closure predated Increment 24 and was untouched by it — and found by the adversarial review of PR #155 while reviewing the capability gate wrapped around the same control. **Resolved:** the query is now a `() => string` read when a mode is chosen rather than a string captured when the selector renders, matching what `main.ts`'s submit handler already does, and falling back to the mounted query only where the document has no header input. Two regression tests, one of which fails against the exact pre-fix closure. -- **`startHarness` drew ports `fetch` refuses (RESOLVED in ADR-0140); the unexplained whole-file failure is still open.** Two signatures were filed here, deliberately not as one cause, and that judgement held. **Signature A is resolved.** WHATWG Fetch blocks eighty-two ports and undici enforces the list on the port number alone, before opening a socket — so `server.listen(0)` could bind, listen and answer raw TCP while `fetch` still refused, surfacing as `TypeError: fetch failed` / `Error: bad port` at the harness’s first request rather than at the listen that caused it. Whether it can happen at all is a property of the host’s dynamic port range: a typical Linux CI range (32768–60999) contains no blocked port, while the Windows range in use here (1024–15000) contains nineteen, which is the whole of "green on CI, flaky locally". The guard that already existed was incomplete in a way that still failed — its hand-observed set of eighteen ports was the spec list intersected with one machine’s range **minus `6679`** — and it retried unboundedly, had no behaviour on exhaustion, and had been copy-pasted into `auth-signin-schema.integration.test.ts`, so the missing port had to be found twice. `packages/api/test/listen.ts` now owns port acquisition for both sites: the spec-complete eighty-two ports (verified by sweeping all 65535 through the real `fetch` on Node v24.15.0), a bounded twenty attempts, each rejected listener closed before the next is asked for, a guarded `address()` read in place of the `as AddressInfo` cast, and an exhaustion error naming the attempts and rejected ports and nothing else. **Signature B is not resolved and was not folded in.** A file still fails with `'test failed'`, no assertion, no stack and none of its own tests reported. Twenty consecutive full runs gave five failures: on pre-fix code one signature A (`auth.test.js`) and three signature B; on post-fix code one signature B and no signature A. The port fix removes A and leaves B exactly where it was, which is the evidence that they are two defects. Four different files were hit (`move-explanation-route`, `tournament-commentary-route`, `bot-detection-analyze`, `anti-cheat-analysis`), sharing no import beyond `./helpers`; each died in 589–703 ms with no test of its own reporting and no stderr. Refuted with evidence: ephemeral-port exhaustion (113 sockets in TIME_WAIT against a 13977-port range), a `Promise.race` loser becoming an unhandled rejection (`race` subscribes to every promise, confirmed on v24.15.0), a throwing `after`/`afterEach` hook (the affected files use none), and a double `close()` rejecting (awaited in a `finally`, it would be attributed to that test with a stack). A second bounded pass — twelve more full runs under the TAP reporter with a preload recording `uncaughtException`, `unhandledRejection` and any non-zero exit — produced twelve clean runs and captured nothing at the time. **A follow-up increment (`claude/node-test-signature-b`) then captured the defect directly, three more times, on three files never previously implicated** (`rate-limit-atomicity`, `dependency-parity`, `studies-api`) — seven distinct files observed with this symptom to date. Occurrences across seven distinct files make a shared or cross-cutting path more plausible and make a defect confined to one test file less likely, but do not exclude file-specific inputs or lifecycle interactions. An instrumented preload (`packages/api/test/diagnostics/signature-b-preload.cjs`) hooking process-level events — `process.exit`, `process.abort`, `process.kill`, `uncaughtExceptionMonitor` (passively observing uncaught exceptions and fatal unhandled rejections), `warning`, `beforeExit`, and Node’s own unconditional `exit` — showed **none of the hooks active at the time fired** on any of the three historical captures (though `process.abort()` was not wrapped in those initial runs and is now covered for future occurrences). A synthetic `process.exit(1)`-before-registration fixture reproduces the identical silent shape; every other synthetic mechanism tried (a post-test async throw, an emitter `'error'` with its listener removed, a synchronous module-load throw, a delayed `SIGKILL`) prints a visibly different diagnostic line, stack, or partial test output that the real defect never shows. This narrows the investigated possibilities while leaving the root cause unresolved: the per-file child process (`node --test` spawns one per file, confirmed by distinct PIDs) was not terminated by `process.exit`, uncaught exceptions, or fatal unhandled rejections, and future runs with `process.abort` instrumentation will record whether abort was called through JS; an absent record narrows in-runtime JS termination but cannot alone prove external termination without corroborating child exit status/signal data or OS-level crash evidence (e.g. distinguishing an external kill or uncatchable signal from a native C++/V8 crash). The machine had roughly 2.5 GB of 15.7 GB RAM free at capture time with several other agents’ processes concurrently running, which is circumstantially consistent with resource contention, but no crash was recorded in the Windows Application or System event logs in that window, so the exact external trigger is still not established. No fix was invented — the forbidden responses (sleeps, whole-file retries, lowering concurrency) would only hide the unresolved root cause, whose origin is not yet established. **A further increment then crossed the parent/child boundary the earlier work stopped at, and found the evidence had been there all along:** Node's runner attaches the child's `exitCode` and `signal` to the `ERR_TEST_FAILURE` it throws, and the `spec` reporter discards them — `formatError` replaces the error with `error.cause`, the bare string `'test failed'` — while the built-in `tap` reporter serializes them, so running `spec` to stdout and `tap` to a file recovers the exit status with no custom reporter and no patched internals. Exit codes were measured on this platform rather than assumed: `process.abort()` gives `134`, `Stop-Process -Force` gives `4294967295`, NTSTATUS faults surface as raw unsigned values such as `3221225477` (`0xC0000005`) — and `1` is produced alike by an uncaught exception, `process.exit(1)`, `taskkill /F` and `process.kill`, so it identifies nothing on its own and is classified `inconclusive`. `signature-b-correlate.cjs` joins the parent's TAP record to the child's JSONL log on the test file path (which also yields the child PID) and states what the pair does and does not establish; where the exit code is ambiguous, a child that reached `preload-installed` and then logged nothing still excludes `process.exit` and an uncaught exception, because both would have left a record and fired Node's `exit` event. A bounded pass of 20 runs under this instrumentation produced 0 captures — which bounds the rate and proves nothing: treating the historical ~1-in-5 as an independent per-run rate, zero captures in 20 runs has probability `(4/5)^20 ≈ 1.2%`, and independence is an assumption rather than an established fact; it ran at 3084–3834 MB free against roughly 2.5 GB at the historical captures, consistent with the resource-contention hypothesis but not evidence for it. **Signature B stays UNRESOLVED**; what changed is that the next occurrence is readable rather than silent. **During M15 Increment 46 validation, Signature B was directly observed three more times, broadening the observed scope to `packages/persistence`:** `search-backfill.integration.test.ts`, `learning.integration.test.ts`, and `test-database.integration.test.ts` each died with the documented bare whole-file `'test failed'` with zero tests reporting and no assertion or stack, and each passed standalone against the exact database state it died on. This confirms the defect is not specific to `packages/api` or its HTTP server test harness. **M15 Increment 47 hardened the diagnostic correlator across 26 bounded runs:** 1 baseline full-suite run, 20 sequential runs under `run-signature-b-pass.mjs`, and 5 concurrent repository-native load runs produced 0 captures (which bounds the rate under those conditions and does not resolve the defect). Code inspection and falsification identified and fixed four correlator blind spots in `packages/api/test/diagnostics/signature-b-correlate.cjs`: (1) cross-directory basename fallback could falsely match a child log from another directory when the target had slashes; (2) Windows path case folding was host-dependent (`process.platform === 'win32'`), breaking cross-platform correlation when logs were analyzed on a different OS; (3) signed 32-bit Windows NTSTATUS exit codes (e.g. `0xC0000005` surfacing as `-1073741819`) were not normalized to unsigned values; and (4) quoted TAP YAML scalar tokens lost exit code or duration numbers. The targeted test suite was expanded from 23 to 36 tests (34 pass, 2 skip, 0 fail), killing 4 falsification mutations. **Signature B remains UNRESOLVED;** no production fix was invented, and the next occurrence will be correlated without cross-platform or exit-code blind spots. See ADR-0140 §4. +- **`startHarness` drew ports `fetch` refuses (RESOLVED in ADR-0140); the unexplained whole-file failure is still open.** Two signatures were filed here, deliberately not as one cause, and that judgement held. **Signature A is resolved.** WHATWG Fetch blocks eighty-two ports and undici enforces the list on the port number alone, before opening a socket — so `server.listen(0)` could bind, listen and answer raw TCP while `fetch` still refused, surfacing as `TypeError: fetch failed` / `Error: bad port` at the harness’s first request rather than at the listen that caused it. Whether it can happen at all is a property of the host’s dynamic port range: a typical Linux CI range (32768–60999) contains no blocked port, while the Windows range in use here (1024–15000) contains nineteen, which is the whole of "green on CI, flaky locally". The guard that already existed was incomplete in a way that still failed — its hand-observed set of eighteen ports was the spec list intersected with one machine’s range **minus `6679`** — and it retried unboundedly, had no behaviour on exhaustion, and had been copy-pasted into `auth-signin-schema.integration.test.ts`, so the missing port had to be found twice. `packages/api/test/listen.ts` now owns port acquisition for both sites: the spec-complete eighty-two ports (verified by sweeping all 65535 through the real `fetch` on Node v24.15.0), a bounded twenty attempts, each rejected listener closed before the next is asked for, a guarded `address()` read in place of the `as AddressInfo` cast, and an exhaustion error naming the attempts and rejected ports and nothing else. **Signature B is not resolved and was not folded in.** A file still fails with `'test failed'`, no assertion, no stack and none of its own tests reported. Twenty consecutive full runs gave five failures: on pre-fix code one signature A (`auth.test.js`) and three signature B; on post-fix code one signature B and no signature A. The port fix removes A and leaves B exactly where it was, which is the evidence that they are two defects. Four different files were hit (`move-explanation-route`, `tournament-commentary-route`, `bot-detection-analyze`, `anti-cheat-analysis`), sharing no import beyond `./helpers`; each died in 589–703 ms with no test of its own reporting and no stderr. Refuted with evidence: ephemeral-port exhaustion (113 sockets in TIME_WAIT against a 13977-port range), a `Promise.race` loser becoming an unhandled rejection (`race` subscribes to every promise, confirmed on v24.15.0), a throwing `after`/`afterEach` hook (the affected files use none), and a double `close()` rejecting (awaited in a `finally`, it would be attributed to that test with a stack). A second bounded pass — twelve more full runs under the TAP reporter with a preload recording `uncaughtException`, `unhandledRejection` and any non-zero exit — produced twelve clean runs and captured nothing at the time. **A follow-up increment (`claude/node-test-signature-b`) then captured the defect directly, three more times, on three files never previously implicated** (`rate-limit-atomicity`, `dependency-parity`, `studies-api`) — seven distinct files observed with this symptom to date. Occurrences across seven distinct files make a shared or cross-cutting path more plausible and make a defect confined to one test file less likely, but do not exclude file-specific inputs or lifecycle interactions. An instrumented preload (`packages/api/test/diagnostics/signature-b-preload.cjs`) hooking process-level events — `process.exit`, `process.abort`, `process.kill`, `uncaughtExceptionMonitor` (passively observing uncaught exceptions and fatal unhandled rejections), `warning`, `beforeExit`, and Node’s own unconditional `exit` — showed **none of the hooks active at the time fired** on any of the three historical captures (though `process.abort()` was not wrapped in those initial runs and is now covered for future occurrences). A synthetic `process.exit(1)`-before-registration fixture reproduces the identical silent shape; every other synthetic mechanism tried (a post-test async throw, an emitter `'error'` with its listener removed, a synchronous module-load throw, a delayed `SIGKILL`) prints a visibly different diagnostic line, stack, or partial test output that the real defect never shows. This narrows the investigated possibilities while leaving the root cause unresolved: the per-file child process (`node --test` spawns one per file, confirmed by distinct PIDs) was not terminated by `process.exit`, uncaught exceptions, or fatal unhandled rejections, and future runs with `process.abort` instrumentation will record whether abort was called through JS; an absent record narrows in-runtime JS termination but cannot alone prove external termination without corroborating child exit status/signal data or OS-level crash evidence (e.g. distinguishing an external kill or uncatchable signal from a native C++/V8 crash). The machine had roughly 2.5 GB of 15.7 GB RAM free at capture time with several other agents’ processes concurrently running, which is circumstantially consistent with resource contention, but no crash was recorded in the Windows Application or System event logs in that window, so the exact external trigger is still not established. No fix was invented — the forbidden responses (sleeps, whole-file retries, lowering concurrency) would only hide the unresolved root cause, whose origin is not yet established. **A further increment then crossed the parent/child boundary the earlier work stopped at, and found the evidence had been there all along:** Node's runner attaches the child's `exitCode` and `signal` to the `ERR_TEST_FAILURE` it throws, and the `spec` reporter discards them — `formatError` replaces the error with `error.cause`, the bare string `'test failed'` — while the built-in `tap` reporter serializes them, so running `spec` to stdout and `tap` to a file recovers the exit status with no custom reporter and no patched internals. Exit codes were measured on this platform rather than assumed: `process.abort()` gives `134`, `Stop-Process -Force` gives `4294967295`, NTSTATUS faults surface as raw unsigned values such as `3221225477` (`0xC0000005`) — and `1` is produced alike by an uncaught exception, `process.exit(1)`, `taskkill /F` and `process.kill`, so it identifies nothing on its own and is classified `inconclusive`. `signature-b-correlate.cjs` joins the parent's TAP record to the child's JSONL log on the test file path (which also yields the child PID) and states what the pair does and does not establish; where the exit code is ambiguous, a child that reached `preload-installed` and then logged nothing still excludes `process.exit` and an uncaught exception, because both would have left a record and fired Node's `exit` event. A bounded pass of 20 runs under this instrumentation produced 0 captures — which bounds the rate and proves nothing: treating the historical ~1-in-5 as an independent per-run rate, zero captures in 20 runs has probability `(4/5)^20 ≈ 1.2%`, and independence is an assumption rather than an established fact; it ran at 3084–3834 MB free against roughly 2.5 GB at the historical captures, consistent with the resource-contention hypothesis but not evidence for it. **Signature B stays UNRESOLVED**; what changed is that the next occurrence is readable rather than silent. **During M15 Increment 46 validation, Signature B was directly observed three more times, broadening the observed scope to `packages/persistence`:** `search-backfill.integration.test.ts`, `learning.integration.test.ts`, and `test-database.integration.test.ts` each died with the documented bare whole-file `'test failed'` with zero tests reporting and no assertion or stack, and each passed standalone against the exact database state it died on. This confirms the defect is not specific to `packages/api` or its HTTP server test harness. **M15 Increment 47 hardened the diagnostic correlator across 26 bounded runs:** 1 baseline full-suite run, 20 sequential runs under `run-signature-b-pass.mjs`, and 5 concurrent repository-native load runs produced 0 captures (which bounds the rate under those conditions and does not resolve the defect). Code inspection and falsification identified and fixed four correlator blind spots in `packages/api/test/diagnostics/signature-b-correlate.cjs`: (1) cross-directory basename fallback could falsely match a child log from another directory when the target had slashes; (2) Windows path case folding was host-dependent (`process.platform === 'win32'`), breaking cross-platform correlation when logs were analyzed on a different OS; (3) signed 32-bit Windows NTSTATUS exit codes (e.g. `0xC0000005` surfacing as `-1073741819`) were not normalized to unsigned values; and (4) quoted TAP YAML scalar tokens lost exit code or duration numbers. The targeted test suite was expanded from 23 to 37 tests (35 pass, 2 skip, 0 fail), killing 4 falsification mutations. **Signature B remains UNRESOLVED;** no production fix was invented, and the next occurrence will be correlated without cross-platform or exit-code blind spots. See ADR-0140 §4. - **Isolated-database test teardown dropped databases out from under connections that had not finished closing (RESOLVED in M15 Increment 45).** `withDatabase` in `packages/persistence/test/variant-migrations.integration.test.ts` ended its pool and then immediately ran `DROP DATABASE ... WITH (FORCE)`. `pool.end()` does not wait for its clients to close: in pg 8.22.0 `_pulseQueue` reaches the end callback in the same synchronous turn in which `_remove` filters the last client out of `_clients`, while `client.end()` has only queued the Terminate byte — instrumentation recorded **zero of four `remove` events fired at the moment `end()` resolved**. The drop could therefore still find a backend attached; `FORCE` terminated it, and the resulting `FATAL` arrived on a socket whose pool still had `idleListener` attached, which `pg` re-emitted as `pool.emit('error')` — an unhandled EventEmitter error that `node:test` attributed to whichever test was running rather than to the teardown that caused it. It surfaced as intermittent `terminating connection due to administrator command` failures in `postgres integration (persistence)` during M15 Increment 44, on a different test each run, which is the signature of a race rather than a broken assertion. The same shape existed in `packages/api/test/auth-signin-schema.integration.test.ts`, which had absorbed SQLSTATE 57P01 with a `pool.on('error', ...)` listener — a symptom fix for the same cause. **Resolved in Increment 45:** a shared `withTestDatabase` helper (`@chess-platform/persistence/test-support`) ends the pool under a bound, waits for `pg_stat_activity` to report the database unused, and drops it *without* `FORCE`. Measured on PostgreSQL 16.14, a plain drop against a still-attached backend fails with SQLSTATE 55006 and leaves that connection untouched, where `FORCE` succeeds by killing it — so the change trades a quiet, harmful success for a loud, harmless failure. FORCE remains only on the emergency path that guarantees the disposable database is still dropped once teardown has already failed — best effort, since that last drop runs inside a `catch` so it cannot bury the error being reported. The 57P01 absorber is deleted, because the corrected lifecycle never causes one. `createPool` and `migrate` are unchanged, and no migration was added. - **The persistence integration suite was not idempotent against a reused database (RESOLVED in M15 Increment 46).** Recorded as a known defect by Increment 45 and left open there. Against a fresh PostgreSQL 16 database the suite passed; a second run against the *same* database failed nine tests, deterministically — measured on 16.14 before any edit as **173 pass / 0 fail** then **164 pass / 9 fail**. CI provisions a fresh server per run, so it never surfaced there. One contract was being broken in two directions: a suite sharing `chess_test` must remove every row it created and remove nothing else. `achievements.integration.test.ts` broke the second half with an unqualified `DELETE FROM users` in `beforeEach` — `games.white_id` and `games.black_id` are the only references to `users` without `ON DELETE CASCADE` (thirty-one FKs point at `users`; twenty-nine cascade; the two that do not are both on `games`), so one game left behind by `pg.integration.test.ts` aborted the wipe with SQLSTATE 23503 before any assertion ran, and the same statement destroyed the bot accounts migration 0021 seeds, which nothing restores because `migrate` has already recorded 0021 as applied. `pg/identity-tokens.test.ts` and `tournaments.pg.integration.test.ts` broke the first half, leaving fixed primary keys behind and colliding on `users_pkey` and `tournaments_pkey` (the latter surfacing through the repository's compare-and-set as `VersionConflictError`). **Resolved in Increment 46:** suites that legitimately share the database delete exactly their own rows through `withSharedDatabase` (`packages/persistence/src/test-support/fixtures.ts`), the sibling of Increment 45's `withTestDatabase` and heir to its precedence rule — a cleanup failure never replaces the assertion that actually failed, and never disappears either. `pg.integration.test.ts` moved to disposable databases instead, because it cannot meet the cleanup half at all: it appends to `game_events`, which is append-only by production trigger, so cleaning up after itself would have meant weakening a production safety rule to suit a test. `users-batch`, `anti-cheat`, `bot-reports` and `analysis-cache` were corrected for the same contract though none of them ever failed — fresh `uuidv7()` ids meant their leaked rows could not collide, so the tables merely grew on every run. Serialization was not the fix and was not introduced: `--test-concurrency=1` is a pre-existing documented invariant and the second run failed identically under it. Acceptance is three consecutive runs against one database with no reset between them (186/186/186), after which only the three migration-seeded bot accounts remain, with no leaked disposable databases and no lingering backends; falsification killed 17 of 20 mutations. No production code, migration, checksum, constraint or repository conflict semantic changed, and no migration was added. - **`ApiServer.listen` registered no `'error'` handler, so a failed bind hung and raised an uncaught event (RESOLVED in ADR-0140 §5).** `packages/api/src/server.ts` resolved its promise from the `listening` callback only and built it with no reject path. A bind failing asynchronously (`EADDRINUSE`, `EMFILE`) left the promise pending forever and, with no `'error'` listener on the `http.Server`, was re-raised as an uncaught exception. Found while investigating ADR-0140 and independently raised by the Qodo review of PR #21. **Resolved in the same increment** rather than deferred, because ADR-0140 §2's bounded, diagnosable acquisition is not true without it — the retry can only report a bind error if the listener it is handed rejects. A one-shot `'error'` listener now rejects and is removed once listening, so later server errors keep their previous semantics rather than being swallowed by a `reject` on a settled promise. The regression test fails against the exact pre-fix code, through an uncaught `ERR_UNHANDLED_ERROR`. diff --git a/packages/api/test/diagnostics/signature-b-correlate.cjs b/packages/api/test/diagnostics/signature-b-correlate.cjs index 35ce8ee8..8eee2266 100644 --- a/packages/api/test/diagnostics/signature-b-correlate.cjs +++ b/packages/api/test/diagnostics/signature-b-correlate.cjs @@ -86,12 +86,12 @@ function fileKey(filePath, isWindows) { */ function isWindowsPath(filePath) { const str = String(filePath); - if (str.startsWith('/') && !str.startsWith('//')) { + if (str.startsWith('/')) { return false; } return ( /^[a-zA-Z]:(?:[/\\]|$)/.test(str) || - /^(?:\\\\|\/\/)[^/\\\\]/.test(str) + /^\\{2}[^/\\]/.test(str) ); } @@ -367,14 +367,12 @@ function readChildLogs(logDir) { */ function matchChild(childLogs, parentFile) { const isWantedWin = isWindowsPath(parentFile); + const parentNorm = normalizePath(parentFile, isWantedWin); const entries = [...childLogs.entries()]; - const hasWindowsChild = entries.some(([, child]) => child?.isWindows); - const isWinContext = isWantedWin || hasWindowsChild; - const wanted = normalizePath(parentFile, isWinContext); const bySuffix = entries.filter(([key, child]) => { const isWin = Boolean(child?.isWindows || isWantedWin); - const wantedNorm = isWin ? normalizePath(parentFile, true).toLowerCase() : normalizePath(parentFile, false); + const wantedNorm = isWin ? parentNorm.toLowerCase() : parentNorm; const keyNorm = isWin ? key.toLowerCase() : key; return keyNorm === wantedNorm || keyNorm.endsWith(`/${wantedNorm}`); }); @@ -384,10 +382,10 @@ function matchChild(childLogs, parentFile) { // Basename fallback is ONLY for a parent path too short to disambiguate (i.e. bare filename with no slashes). // When the parent specified a directory path and bySuffix found 0 matches, that file has no child log. // Matching a different directory's child would be a false cross-directory attribution. - if (!wanted.includes('/')) { + if (!parentNorm.includes('/')) { + const base = fileKey(parentFile, isWantedWin); const byBase = entries.filter(([key, child]) => { const isWin = Boolean(child?.isWindows || isWantedWin); - const base = fileKey(parentFile, isWin); const kBase = fileKey(key, isWin); if (isWin) { return kBase.toLowerCase() === base.toLowerCase(); diff --git a/packages/api/test/diagnostics/signature-b-correlate.test.ts b/packages/api/test/diagnostics/signature-b-correlate.test.ts index 915a555b..ae255dfc 100644 --- a/packages/api/test/diagnostics/signature-b-correlate.test.ts +++ b/packages/api/test/diagnostics/signature-b-correlate.test.ts @@ -852,7 +852,7 @@ test('correlator: isWindowsPath identifies Windows drive letters and UNC paths w assert.equal(correlator.isWindowsPath('C:/repo/test.js'), true); assert.equal(correlator.isWindowsPath('C:\\repo\\test.js'), true); assert.equal(correlator.isWindowsPath('d:/repo/test.js'), true); - assert.equal(correlator.isWindowsPath('//server/share/test.js'), true); + assert.equal(correlator.isWindowsPath('//server/share/test.js'), false); assert.equal(correlator.isWindowsPath('\\\\server\\share\\test.js'), true); assert.equal(correlator.isWindowsPath('/home/runner/repo/test.js'), false); assert.equal(correlator.isWindowsPath('/home/runner/repo/weird\\name.test.js'), false); @@ -1010,3 +1010,26 @@ test('correlator: parseTapFailures rejects non-finite exitCode and duration_ms s assert.equal(failures[2]?.exitCode, null); assert.equal(failures[2]?.durationMs, null); }); + +test('correlator: POSIX parent path with backslash filename does not falsely match Windows child path with directory segments', () => { + const logDir = fs.mkdtempSync(path.join(os.tmpdir(), 'sigb-posix-parent-win-child-')); + try { + // Windows child log where 'weird' is a directory and 'name.test.js' is the file + const lines = [ + { kind: 'start', pid: 888, testFile: 'C:\\repo\\dist-test\\test\\weird\\name.test.js' }, + { kind: 'preload-installed', pid: 888, testFile: 'C:\\repo\\dist-test\\test\\weird\\name.test.js' }, + ]; + fs.writeFileSync(path.join(logDir, 'run-win-child.jsonl'), `${lines.map((l) => JSON.stringify(l)).join('\n')}\n`); + const logs = correlator.readChildLogs(logDir); + + // POSIX parent failure where 'weird\name.test.js' is a single filename in 'test/' + const resolved = correlator.correlate( + correlator.parseTapFailures(tapFailure('dist-test/test/weird\\name.test.js', '1')), + logs, + ); + assert.equal(resolved.length, 1); + assert.equal(resolved[0]?.child.logFound, false, 'POSIX parent path with backslash in filename must not match Windows child directory segments'); + } finally { + fs.rmSync(logDir, { recursive: true, force: true }); + } +}); From 970b8a06e0b52a8a847741eaedc5f0f492e97456 Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Fri, 4 Sep 2026 23:45:22 +0300 Subject: [PATCH 10/13] fix(api-diagnostics): support relative Windows backslash parent paths when correlating against Windows children --- docs/PROJECT_STATE.md | 4 +-- docs/ROADMAP.md | 2 +- .../diagnostics/signature-b-correlate.cjs | 33 ++++++++++++------- .../diagnostics/signature-b-correlate.test.ts | 28 ++++++++++++++-- 4 files changed, 51 insertions(+), 16 deletions(-) diff --git a/docs/PROJECT_STATE.md b/docs/PROJECT_STATE.md index 788d3a64..bd0274ba 100644 --- a/docs/PROJECT_STATE.md +++ b/docs/PROJECT_STATE.md @@ -59,8 +59,8 @@ parent TAP diagnostic logs and child JSONL preloads were matched and parsed: values to `null`. **Verification and mutation falsification.** -- Targeted correlator suite (`signature-b-correlate.test.ts`): expanded from 23 to 37 tests - (35 pass, 2 skip for deliberate platform-gated checks, 0 fail). +- Targeted correlator suite (`signature-b-correlate.test.ts`): expanded from 23 to 38 tests + (36 pass, 2 skip for deliberate platform-gated checks, 0 fail). - Mutation falsification: four targeted mutations reversing each of the four hardened behaviors were verified to be killed by the expanded test suite. - Monorepo validation: full build, lint, counts, and guard scripts pass cleanly. diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 6d881ea1..9a31646e 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -1349,7 +1349,7 @@ Debt observed during M14. Each states what is known, not what is planned; items - **The published capability document could omit a composed feature, and the client offered controls that could not work (RESOLVED in M15 Increment 24 / ADR-0132).** Left open by Increment 23 above. `capabilitiesView` (`packages/api/src/presenters.ts`) took a hand-written `Pick`, so a feature never added to it was invisible to `GET /v1/capabilities` and nothing complained. ADR-0131 judged that "a narrower failure than a 503 — the feature works for anyone who calls the route directly", and deferred it on the grounds that closing it meant first deciding which optional dependencies are user-facing capabilities. **Both halves of that were wrong.** The failure is narrower only when the client can still reach the feature, and a live instance already existed where it could not: `GET /v1/search` served three modes from two independently-gated dependency sets behind one published `search` flag, so a deployment running the Helm chart’s `search.semanticEnabled: false` advertised search while the client offered two mode buttons whose every request answered 503. And the judgement, while genuinely underivable, can be made **unskippable**, which was the property wanted. `semanticSearch` is now published from both of its dependencies, `Exclude[0] | NotAPublishedCapability>` must be `never` so a new optional dependency cannot compile until someone gives it a flag or records why it has none, and a behavioural guard requires every capability source to change what the document publishes — because a key can sit in that parameter unread, which the compile-time half cannot see. The mutation ledger lives in ADR-0132 and is not restated here. - **The search surface was not gated on `capabilities.search`, so an absolute kill switch still showed a search box (RESOLVED in M15 Increment 24 / ADR-0132 §5).** `SEARCH_ENABLED=0` — the chart's `search.enabled: false`, an absolute kill switch per ADR-0055 — leaves `searchRepository` unconstructed and `GET /v1/search` answering 503 on every mode, keyword included. The entry point was the persistent header form in `packages/web/index.html`, present on every page; being a `` rather than an `a[data-route]`, `NAV_CAPABILITY_MAP` could not reach it, which is why the first pass at Increment 24 gated the semantic and hybrid *modes* and left keyword ungated — the same defect class one mode over. Raised by the Qodo review of PR #155. **Fixed in the same increment rather than deferred:** the form now ships `hidden` and is revealed by `applySearchCapability` only on an explicit `search: true`; the route renders an honest unavailable notice and issues no request; and keyword search waits for the capability answer, reversing this increment's own earlier latency decision, because knowing whether a request is pointless requires having asked. A markup-contract test pins the `hidden` attribute, since the gate depends on it and every other test passes without it. - **Clicking a search mode discarded text typed since the page loaded (RESOLVED in a follow-up to M15 Increment 24 / ADR-0132).** `createModeInput` in `packages/web/src/app/search-mount.ts` closed over the query captured when the route mounted, so `navigateToSearchMode` navigated with the old term and the remount reset the input to match it. Type a new term into the header field, click **Semantic** without pressing enter, and the typed text was gone with no indication it had been discarded. Pre-existing — the closure predated Increment 24 and was untouched by it — and found by the adversarial review of PR #155 while reviewing the capability gate wrapped around the same control. **Resolved:** the query is now a `() => string` read when a mode is chosen rather than a string captured when the selector renders, matching what `main.ts`'s submit handler already does, and falling back to the mounted query only where the document has no header input. Two regression tests, one of which fails against the exact pre-fix closure. -- **`startHarness` drew ports `fetch` refuses (RESOLVED in ADR-0140); the unexplained whole-file failure is still open.** Two signatures were filed here, deliberately not as one cause, and that judgement held. **Signature A is resolved.** WHATWG Fetch blocks eighty-two ports and undici enforces the list on the port number alone, before opening a socket — so `server.listen(0)` could bind, listen and answer raw TCP while `fetch` still refused, surfacing as `TypeError: fetch failed` / `Error: bad port` at the harness’s first request rather than at the listen that caused it. Whether it can happen at all is a property of the host’s dynamic port range: a typical Linux CI range (32768–60999) contains no blocked port, while the Windows range in use here (1024–15000) contains nineteen, which is the whole of "green on CI, flaky locally". The guard that already existed was incomplete in a way that still failed — its hand-observed set of eighteen ports was the spec list intersected with one machine’s range **minus `6679`** — and it retried unboundedly, had no behaviour on exhaustion, and had been copy-pasted into `auth-signin-schema.integration.test.ts`, so the missing port had to be found twice. `packages/api/test/listen.ts` now owns port acquisition for both sites: the spec-complete eighty-two ports (verified by sweeping all 65535 through the real `fetch` on Node v24.15.0), a bounded twenty attempts, each rejected listener closed before the next is asked for, a guarded `address()` read in place of the `as AddressInfo` cast, and an exhaustion error naming the attempts and rejected ports and nothing else. **Signature B is not resolved and was not folded in.** A file still fails with `'test failed'`, no assertion, no stack and none of its own tests reported. Twenty consecutive full runs gave five failures: on pre-fix code one signature A (`auth.test.js`) and three signature B; on post-fix code one signature B and no signature A. The port fix removes A and leaves B exactly where it was, which is the evidence that they are two defects. Four different files were hit (`move-explanation-route`, `tournament-commentary-route`, `bot-detection-analyze`, `anti-cheat-analysis`), sharing no import beyond `./helpers`; each died in 589–703 ms with no test of its own reporting and no stderr. Refuted with evidence: ephemeral-port exhaustion (113 sockets in TIME_WAIT against a 13977-port range), a `Promise.race` loser becoming an unhandled rejection (`race` subscribes to every promise, confirmed on v24.15.0), a throwing `after`/`afterEach` hook (the affected files use none), and a double `close()` rejecting (awaited in a `finally`, it would be attributed to that test with a stack). A second bounded pass — twelve more full runs under the TAP reporter with a preload recording `uncaughtException`, `unhandledRejection` and any non-zero exit — produced twelve clean runs and captured nothing at the time. **A follow-up increment (`claude/node-test-signature-b`) then captured the defect directly, three more times, on three files never previously implicated** (`rate-limit-atomicity`, `dependency-parity`, `studies-api`) — seven distinct files observed with this symptom to date. Occurrences across seven distinct files make a shared or cross-cutting path more plausible and make a defect confined to one test file less likely, but do not exclude file-specific inputs or lifecycle interactions. An instrumented preload (`packages/api/test/diagnostics/signature-b-preload.cjs`) hooking process-level events — `process.exit`, `process.abort`, `process.kill`, `uncaughtExceptionMonitor` (passively observing uncaught exceptions and fatal unhandled rejections), `warning`, `beforeExit`, and Node’s own unconditional `exit` — showed **none of the hooks active at the time fired** on any of the three historical captures (though `process.abort()` was not wrapped in those initial runs and is now covered for future occurrences). A synthetic `process.exit(1)`-before-registration fixture reproduces the identical silent shape; every other synthetic mechanism tried (a post-test async throw, an emitter `'error'` with its listener removed, a synchronous module-load throw, a delayed `SIGKILL`) prints a visibly different diagnostic line, stack, or partial test output that the real defect never shows. This narrows the investigated possibilities while leaving the root cause unresolved: the per-file child process (`node --test` spawns one per file, confirmed by distinct PIDs) was not terminated by `process.exit`, uncaught exceptions, or fatal unhandled rejections, and future runs with `process.abort` instrumentation will record whether abort was called through JS; an absent record narrows in-runtime JS termination but cannot alone prove external termination without corroborating child exit status/signal data or OS-level crash evidence (e.g. distinguishing an external kill or uncatchable signal from a native C++/V8 crash). The machine had roughly 2.5 GB of 15.7 GB RAM free at capture time with several other agents’ processes concurrently running, which is circumstantially consistent with resource contention, but no crash was recorded in the Windows Application or System event logs in that window, so the exact external trigger is still not established. No fix was invented — the forbidden responses (sleeps, whole-file retries, lowering concurrency) would only hide the unresolved root cause, whose origin is not yet established. **A further increment then crossed the parent/child boundary the earlier work stopped at, and found the evidence had been there all along:** Node's runner attaches the child's `exitCode` and `signal` to the `ERR_TEST_FAILURE` it throws, and the `spec` reporter discards them — `formatError` replaces the error with `error.cause`, the bare string `'test failed'` — while the built-in `tap` reporter serializes them, so running `spec` to stdout and `tap` to a file recovers the exit status with no custom reporter and no patched internals. Exit codes were measured on this platform rather than assumed: `process.abort()` gives `134`, `Stop-Process -Force` gives `4294967295`, NTSTATUS faults surface as raw unsigned values such as `3221225477` (`0xC0000005`) — and `1` is produced alike by an uncaught exception, `process.exit(1)`, `taskkill /F` and `process.kill`, so it identifies nothing on its own and is classified `inconclusive`. `signature-b-correlate.cjs` joins the parent's TAP record to the child's JSONL log on the test file path (which also yields the child PID) and states what the pair does and does not establish; where the exit code is ambiguous, a child that reached `preload-installed` and then logged nothing still excludes `process.exit` and an uncaught exception, because both would have left a record and fired Node's `exit` event. A bounded pass of 20 runs under this instrumentation produced 0 captures — which bounds the rate and proves nothing: treating the historical ~1-in-5 as an independent per-run rate, zero captures in 20 runs has probability `(4/5)^20 ≈ 1.2%`, and independence is an assumption rather than an established fact; it ran at 3084–3834 MB free against roughly 2.5 GB at the historical captures, consistent with the resource-contention hypothesis but not evidence for it. **Signature B stays UNRESOLVED**; what changed is that the next occurrence is readable rather than silent. **During M15 Increment 46 validation, Signature B was directly observed three more times, broadening the observed scope to `packages/persistence`:** `search-backfill.integration.test.ts`, `learning.integration.test.ts`, and `test-database.integration.test.ts` each died with the documented bare whole-file `'test failed'` with zero tests reporting and no assertion or stack, and each passed standalone against the exact database state it died on. This confirms the defect is not specific to `packages/api` or its HTTP server test harness. **M15 Increment 47 hardened the diagnostic correlator across 26 bounded runs:** 1 baseline full-suite run, 20 sequential runs under `run-signature-b-pass.mjs`, and 5 concurrent repository-native load runs produced 0 captures (which bounds the rate under those conditions and does not resolve the defect). Code inspection and falsification identified and fixed four correlator blind spots in `packages/api/test/diagnostics/signature-b-correlate.cjs`: (1) cross-directory basename fallback could falsely match a child log from another directory when the target had slashes; (2) Windows path case folding was host-dependent (`process.platform === 'win32'`), breaking cross-platform correlation when logs were analyzed on a different OS; (3) signed 32-bit Windows NTSTATUS exit codes (e.g. `0xC0000005` surfacing as `-1073741819`) were not normalized to unsigned values; and (4) quoted TAP YAML scalar tokens lost exit code or duration numbers. The targeted test suite was expanded from 23 to 37 tests (35 pass, 2 skip, 0 fail), killing 4 falsification mutations. **Signature B remains UNRESOLVED;** no production fix was invented, and the next occurrence will be correlated without cross-platform or exit-code blind spots. See ADR-0140 §4. +- **`startHarness` drew ports `fetch` refuses (RESOLVED in ADR-0140); the unexplained whole-file failure is still open.** Two signatures were filed here, deliberately not as one cause, and that judgement held. **Signature A is resolved.** WHATWG Fetch blocks eighty-two ports and undici enforces the list on the port number alone, before opening a socket — so `server.listen(0)` could bind, listen and answer raw TCP while `fetch` still refused, surfacing as `TypeError: fetch failed` / `Error: bad port` at the harness’s first request rather than at the listen that caused it. Whether it can happen at all is a property of the host’s dynamic port range: a typical Linux CI range (32768–60999) contains no blocked port, while the Windows range in use here (1024–15000) contains nineteen, which is the whole of "green on CI, flaky locally". The guard that already existed was incomplete in a way that still failed — its hand-observed set of eighteen ports was the spec list intersected with one machine’s range **minus `6679`** — and it retried unboundedly, had no behaviour on exhaustion, and had been copy-pasted into `auth-signin-schema.integration.test.ts`, so the missing port had to be found twice. `packages/api/test/listen.ts` now owns port acquisition for both sites: the spec-complete eighty-two ports (verified by sweeping all 65535 through the real `fetch` on Node v24.15.0), a bounded twenty attempts, each rejected listener closed before the next is asked for, a guarded `address()` read in place of the `as AddressInfo` cast, and an exhaustion error naming the attempts and rejected ports and nothing else. **Signature B is not resolved and was not folded in.** A file still fails with `'test failed'`, no assertion, no stack and none of its own tests reported. Twenty consecutive full runs gave five failures: on pre-fix code one signature A (`auth.test.js`) and three signature B; on post-fix code one signature B and no signature A. The port fix removes A and leaves B exactly where it was, which is the evidence that they are two defects. Four different files were hit (`move-explanation-route`, `tournament-commentary-route`, `bot-detection-analyze`, `anti-cheat-analysis`), sharing no import beyond `./helpers`; each died in 589–703 ms with no test of its own reporting and no stderr. Refuted with evidence: ephemeral-port exhaustion (113 sockets in TIME_WAIT against a 13977-port range), a `Promise.race` loser becoming an unhandled rejection (`race` subscribes to every promise, confirmed on v24.15.0), a throwing `after`/`afterEach` hook (the affected files use none), and a double `close()` rejecting (awaited in a `finally`, it would be attributed to that test with a stack). A second bounded pass — twelve more full runs under the TAP reporter with a preload recording `uncaughtException`, `unhandledRejection` and any non-zero exit — produced twelve clean runs and captured nothing at the time. **A follow-up increment (`claude/node-test-signature-b`) then captured the defect directly, three more times, on three files never previously implicated** (`rate-limit-atomicity`, `dependency-parity`, `studies-api`) — seven distinct files observed with this symptom to date. Occurrences across seven distinct files make a shared or cross-cutting path more plausible and make a defect confined to one test file less likely, but do not exclude file-specific inputs or lifecycle interactions. An instrumented preload (`packages/api/test/diagnostics/signature-b-preload.cjs`) hooking process-level events — `process.exit`, `process.abort`, `process.kill`, `uncaughtExceptionMonitor` (passively observing uncaught exceptions and fatal unhandled rejections), `warning`, `beforeExit`, and Node’s own unconditional `exit` — showed **none of the hooks active at the time fired** on any of the three historical captures (though `process.abort()` was not wrapped in those initial runs and is now covered for future occurrences). A synthetic `process.exit(1)`-before-registration fixture reproduces the identical silent shape; every other synthetic mechanism tried (a post-test async throw, an emitter `'error'` with its listener removed, a synchronous module-load throw, a delayed `SIGKILL`) prints a visibly different diagnostic line, stack, or partial test output that the real defect never shows. This narrows the investigated possibilities while leaving the root cause unresolved: the per-file child process (`node --test` spawns one per file, confirmed by distinct PIDs) was not terminated by `process.exit`, uncaught exceptions, or fatal unhandled rejections, and future runs with `process.abort` instrumentation will record whether abort was called through JS; an absent record narrows in-runtime JS termination but cannot alone prove external termination without corroborating child exit status/signal data or OS-level crash evidence (e.g. distinguishing an external kill or uncatchable signal from a native C++/V8 crash). The machine had roughly 2.5 GB of 15.7 GB RAM free at capture time with several other agents’ processes concurrently running, which is circumstantially consistent with resource contention, but no crash was recorded in the Windows Application or System event logs in that window, so the exact external trigger is still not established. No fix was invented — the forbidden responses (sleeps, whole-file retries, lowering concurrency) would only hide the unresolved root cause, whose origin is not yet established. **A further increment then crossed the parent/child boundary the earlier work stopped at, and found the evidence had been there all along:** Node's runner attaches the child's `exitCode` and `signal` to the `ERR_TEST_FAILURE` it throws, and the `spec` reporter discards them — `formatError` replaces the error with `error.cause`, the bare string `'test failed'` — while the built-in `tap` reporter serializes them, so running `spec` to stdout and `tap` to a file recovers the exit status with no custom reporter and no patched internals. Exit codes were measured on this platform rather than assumed: `process.abort()` gives `134`, `Stop-Process -Force` gives `4294967295`, NTSTATUS faults surface as raw unsigned values such as `3221225477` (`0xC0000005`) — and `1` is produced alike by an uncaught exception, `process.exit(1)`, `taskkill /F` and `process.kill`, so it identifies nothing on its own and is classified `inconclusive`. `signature-b-correlate.cjs` joins the parent's TAP record to the child's JSONL log on the test file path (which also yields the child PID) and states what the pair does and does not establish; where the exit code is ambiguous, a child that reached `preload-installed` and then logged nothing still excludes `process.exit` and an uncaught exception, because both would have left a record and fired Node's `exit` event. A bounded pass of 20 runs under this instrumentation produced 0 captures — which bounds the rate and proves nothing: treating the historical ~1-in-5 as an independent per-run rate, zero captures in 20 runs has probability `(4/5)^20 ≈ 1.2%`, and independence is an assumption rather than an established fact; it ran at 3084–3834 MB free against roughly 2.5 GB at the historical captures, consistent with the resource-contention hypothesis but not evidence for it. **Signature B stays UNRESOLVED**; what changed is that the next occurrence is readable rather than silent. **During M15 Increment 46 validation, Signature B was directly observed three more times, broadening the observed scope to `packages/persistence`:** `search-backfill.integration.test.ts`, `learning.integration.test.ts`, and `test-database.integration.test.ts` each died with the documented bare whole-file `'test failed'` with zero tests reporting and no assertion or stack, and each passed standalone against the exact database state it died on. This confirms the defect is not specific to `packages/api` or its HTTP server test harness. **M15 Increment 47 hardened the diagnostic correlator across 26 bounded runs:** 1 baseline full-suite run, 20 sequential runs under `run-signature-b-pass.mjs`, and 5 concurrent repository-native load runs produced 0 captures (which bounds the rate under those conditions and does not resolve the defect). Code inspection and falsification identified and fixed four correlator blind spots in `packages/api/test/diagnostics/signature-b-correlate.cjs`: (1) cross-directory basename fallback could falsely match a child log from another directory when the target had slashes; (2) Windows path case folding was host-dependent (`process.platform === 'win32'`), breaking cross-platform correlation when logs were analyzed on a different OS; (3) signed 32-bit Windows NTSTATUS exit codes (e.g. `0xC0000005` surfacing as `-1073741819`) were not normalized to unsigned values; and (4) quoted TAP YAML scalar tokens lost exit code or duration numbers. The targeted test suite was expanded from 23 to 38 tests (36 pass, 2 skip, 0 fail), killing 4 falsification mutations. **Signature B remains UNRESOLVED;** no production fix was invented, and the next occurrence will be correlated without cross-platform or exit-code blind spots. See ADR-0140 §4. - **Isolated-database test teardown dropped databases out from under connections that had not finished closing (RESOLVED in M15 Increment 45).** `withDatabase` in `packages/persistence/test/variant-migrations.integration.test.ts` ended its pool and then immediately ran `DROP DATABASE ... WITH (FORCE)`. `pool.end()` does not wait for its clients to close: in pg 8.22.0 `_pulseQueue` reaches the end callback in the same synchronous turn in which `_remove` filters the last client out of `_clients`, while `client.end()` has only queued the Terminate byte — instrumentation recorded **zero of four `remove` events fired at the moment `end()` resolved**. The drop could therefore still find a backend attached; `FORCE` terminated it, and the resulting `FATAL` arrived on a socket whose pool still had `idleListener` attached, which `pg` re-emitted as `pool.emit('error')` — an unhandled EventEmitter error that `node:test` attributed to whichever test was running rather than to the teardown that caused it. It surfaced as intermittent `terminating connection due to administrator command` failures in `postgres integration (persistence)` during M15 Increment 44, on a different test each run, which is the signature of a race rather than a broken assertion. The same shape existed in `packages/api/test/auth-signin-schema.integration.test.ts`, which had absorbed SQLSTATE 57P01 with a `pool.on('error', ...)` listener — a symptom fix for the same cause. **Resolved in Increment 45:** a shared `withTestDatabase` helper (`@chess-platform/persistence/test-support`) ends the pool under a bound, waits for `pg_stat_activity` to report the database unused, and drops it *without* `FORCE`. Measured on PostgreSQL 16.14, a plain drop against a still-attached backend fails with SQLSTATE 55006 and leaves that connection untouched, where `FORCE` succeeds by killing it — so the change trades a quiet, harmful success for a loud, harmless failure. FORCE remains only on the emergency path that guarantees the disposable database is still dropped once teardown has already failed — best effort, since that last drop runs inside a `catch` so it cannot bury the error being reported. The 57P01 absorber is deleted, because the corrected lifecycle never causes one. `createPool` and `migrate` are unchanged, and no migration was added. - **The persistence integration suite was not idempotent against a reused database (RESOLVED in M15 Increment 46).** Recorded as a known defect by Increment 45 and left open there. Against a fresh PostgreSQL 16 database the suite passed; a second run against the *same* database failed nine tests, deterministically — measured on 16.14 before any edit as **173 pass / 0 fail** then **164 pass / 9 fail**. CI provisions a fresh server per run, so it never surfaced there. One contract was being broken in two directions: a suite sharing `chess_test` must remove every row it created and remove nothing else. `achievements.integration.test.ts` broke the second half with an unqualified `DELETE FROM users` in `beforeEach` — `games.white_id` and `games.black_id` are the only references to `users` without `ON DELETE CASCADE` (thirty-one FKs point at `users`; twenty-nine cascade; the two that do not are both on `games`), so one game left behind by `pg.integration.test.ts` aborted the wipe with SQLSTATE 23503 before any assertion ran, and the same statement destroyed the bot accounts migration 0021 seeds, which nothing restores because `migrate` has already recorded 0021 as applied. `pg/identity-tokens.test.ts` and `tournaments.pg.integration.test.ts` broke the first half, leaving fixed primary keys behind and colliding on `users_pkey` and `tournaments_pkey` (the latter surfacing through the repository's compare-and-set as `VersionConflictError`). **Resolved in Increment 46:** suites that legitimately share the database delete exactly their own rows through `withSharedDatabase` (`packages/persistence/src/test-support/fixtures.ts`), the sibling of Increment 45's `withTestDatabase` and heir to its precedence rule — a cleanup failure never replaces the assertion that actually failed, and never disappears either. `pg.integration.test.ts` moved to disposable databases instead, because it cannot meet the cleanup half at all: it appends to `game_events`, which is append-only by production trigger, so cleaning up after itself would have meant weakening a production safety rule to suit a test. `users-batch`, `anti-cheat`, `bot-reports` and `analysis-cache` were corrected for the same contract though none of them ever failed — fresh `uuidv7()` ids meant their leaked rows could not collide, so the tables merely grew on every run. Serialization was not the fix and was not introduced: `--test-concurrency=1` is a pre-existing documented invariant and the second run failed identically under it. Acceptance is three consecutive runs against one database with no reset between them (186/186/186), after which only the three migration-seeded bot accounts remain, with no leaked disposable databases and no lingering backends; falsification killed 17 of 20 mutations. No production code, migration, checksum, constraint or repository conflict semantic changed, and no migration was added. - **`ApiServer.listen` registered no `'error'` handler, so a failed bind hung and raised an uncaught event (RESOLVED in ADR-0140 §5).** `packages/api/src/server.ts` resolved its promise from the `listening` callback only and built it with no reject path. A bind failing asynchronously (`EADDRINUSE`, `EMFILE`) left the promise pending forever and, with no `'error'` listener on the `http.Server`, was re-raised as an uncaught exception. Found while investigating ADR-0140 and independently raised by the Qodo review of PR #21. **Resolved in the same increment** rather than deferred, because ADR-0140 §2's bounded, diagnosable acquisition is not true without it — the retry can only report a bind error if the listener it is handed rejects. A one-shot `'error'` listener now rejects and is removed once listening, so later server errors keep their previous semantics rather than being swallowed by a `reject` on a settled promise. The regression test fails against the exact pre-fix code, through an uncaught `ERR_UNHANDLED_ERROR`. diff --git a/packages/api/test/diagnostics/signature-b-correlate.cjs b/packages/api/test/diagnostics/signature-b-correlate.cjs index 8eee2266..f1ec4052 100644 --- a/packages/api/test/diagnostics/signature-b-correlate.cjs +++ b/packages/api/test/diagnostics/signature-b-correlate.cjs @@ -363,16 +363,21 @@ function readChildLogs(logDir) { * * @param {ReturnType} childLogs * @param {string} parentFile + * @param {boolean} [parentIsWindows] * @returns {{ child: { pid: number | null, kinds: string[] } | null, ambiguous: boolean, candidates: number }} */ -function matchChild(childLogs, parentFile) { - const isWantedWin = isWindowsPath(parentFile); +function matchChild(childLogs, parentFile, parentIsWindows) { + const isWantedWin = typeof parentIsWindows === 'boolean' + ? parentIsWindows + : isWindowsPath(parentFile); const parentNorm = normalizePath(parentFile, isWantedWin); const entries = [...childLogs.entries()]; const bySuffix = entries.filter(([key, child]) => { const isWin = Boolean(child?.isWindows || isWantedWin); - const wantedNorm = isWin ? parentNorm.toLowerCase() : parentNorm; + const isRelWin = Boolean(child?.isWindows && !isWantedWin && !parentFile.includes('/') && parentFile.includes('\\')); + const effectiveParentNorm = isRelWin ? normalizePath(parentFile, true) : parentNorm; + const wantedNorm = isWin ? effectiveParentNorm.toLowerCase() : effectiveParentNorm; const keyNorm = isWin ? key.toLowerCase() : key; return keyNorm === wantedNorm || keyNorm.endsWith(`/${wantedNorm}`); }); @@ -386,6 +391,8 @@ function matchChild(childLogs, parentFile) { const base = fileKey(parentFile, isWantedWin); const byBase = entries.filter(([key, child]) => { const isWin = Boolean(child?.isWindows || isWantedWin); + const isRelWin = Boolean(child?.isWindows && !isWantedWin && !parentFile.includes('/') && parentFile.includes('\\')); + if (isRelWin) return false; const kBase = fileKey(key, isWin); if (isWin) { return kBase.toLowerCase() === base.toLowerCase(); @@ -403,20 +410,24 @@ function matchChild(childLogs, parentFile) { * Join parent-observed failures to child-observed lifecycle evidence, one record per failure. * * Matching is by whole normalised path, not by basename. The parent names the file as the runner - * received it while the child records its own resolved `argv[1]`, so the child's key is the longer - * path and the parent's name is a suffix of it; a basename comparison is the fallback, not the rule. - * Basenames are *not* unique across the suite's compiled output, which is why a name matching more - * than one child is reported `ambiguous` rather than resolved to whichever came first. A failure - * with no matching child log is reported with `child.logFound: false` rather than being dropped, - * because "the child never wrote a log" is itself evidence. + * received it while the child records its own resolved `argv[1]`, an absolute one. Matching is therefore + * by path *suffix* rather than equality, which identifies `dist-test/test/foo/a.test.js` uniquely even + * when `bar/a.test.js` exists. Basename is only a fallback for a parent path too short to disambiguate, + * and when either step finds more than one candidate the answer is reported ambiguous rather than resolved. * * @param {ReturnType} failures * @param {ReturnType} childLogs + * @param {{ isWindows?: boolean }} [options] * @returns {Array} */ -function correlate(failures, childLogs) { +function correlate(failures, childLogs, options = {}) { + const defaultIsWin = typeof options?.isWindows === 'boolean' ? options.isWindows : undefined; return failures.map((failure) => { - const match = matchChild(childLogs, failure.file); + const file = typeof failure === 'string' ? failure : failure.file; + const parentIsWin = typeof failure?.isWindows === 'boolean' + ? failure.isWindows + : defaultIsWin; + const match = matchChild(childLogs, file, parentIsWin); const kinds = match.child?.kinds ?? []; const classification = classifyTermination(failure); const narrowing = narrowFromChildEvidence(classification, kinds); diff --git a/packages/api/test/diagnostics/signature-b-correlate.test.ts b/packages/api/test/diagnostics/signature-b-correlate.test.ts index ae255dfc..a24bc230 100644 --- a/packages/api/test/diagnostics/signature-b-correlate.test.ts +++ b/packages/api/test/diagnostics/signature-b-correlate.test.ts @@ -47,7 +47,7 @@ const correlator = require(CORRELATE_PATH) as { meaning: string; specific: boolean; }; - correlate(failures: unknown[], childLogs: Map): CorrelatedRecord[]; + correlate(failures: unknown[], childLogs: Map, options?: { isWindows?: boolean }): CorrelatedRecord[]; narrowFromChildEvidence( c: { id: string; specific: boolean }, kinds: readonly string[], @@ -60,7 +60,7 @@ const correlator = require(CORRELATE_PATH) as { durationMs: number | null; }>; readChildLogs(dir: string): Map; - matchChild(logs: Map, file: string): { child: unknown; ambiguous: boolean; candidates: number }; + matchChild(logs: Map, file: string, parentIsWindows?: boolean): { child: unknown; ambiguous: boolean; candidates: number }; normalizePath(p: string): string; isWindowsPath(p: string): boolean; }; @@ -1033,3 +1033,27 @@ test('correlator: POSIX parent path with backslash filename does not falsely mat fs.rmSync(logDir, { recursive: true, force: true }); } }); + +test('correlator: relative backslash-delimited Windows parent path matches Windows child path', () => { + const logDir = fs.mkdtempSync(path.join(os.tmpdir(), 'sigb-relwin-parent-')); + try { + const lines = [ + { kind: 'start', pid: 999, testFile: 'C:\\repo\\dist-test\\test\\foo.test.js' }, + { kind: 'preload-installed', pid: 999, testFile: 'C:\\repo\\dist-test\\test\\foo.test.js' }, + ]; + fs.writeFileSync(path.join(logDir, 'run-win.jsonl'), `${lines.map((l) => JSON.stringify(l)).join('\n')}\n`); + const logs = correlator.readChildLogs(logDir); + + // Parent uses backslash-delimited relative path + const resolved = correlator.correlate( + correlator.parseTapFailures(tapFailure('dist-test\\test\\foo.test.js', '1')), + logs, + ); + assert.equal(resolved.length, 1); + assert.equal(resolved[0]?.child.pid, 999, 'relative Windows backslash parent must match Windows child'); + assert.equal(resolved[0]?.child.logFound, true); + assert.equal(resolved[0]?.child.ambiguous, false); + } finally { + fs.rmSync(logDir, { recursive: true, force: true }); + } +}); From 0b130c430e11a7ddbbdb05e60786fae44a2b8a54 Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Fri, 4 Sep 2026 23:54:40 +0300 Subject: [PATCH 11/13] fix(api-diagnostics): decouple parent separator normalization from child platform and normalize bare string failures --- docs/PROJECT_STATE.md | 4 +-- docs/ROADMAP.md | 2 +- .../diagnostics/signature-b-correlate.cjs | 29 ++++++++++------ .../diagnostics/signature-b-correlate.test.ts | 34 ++++++++++++++++--- 4 files changed, 51 insertions(+), 18 deletions(-) diff --git a/docs/PROJECT_STATE.md b/docs/PROJECT_STATE.md index bd0274ba..fca61dbb 100644 --- a/docs/PROJECT_STATE.md +++ b/docs/PROJECT_STATE.md @@ -59,8 +59,8 @@ parent TAP diagnostic logs and child JSONL preloads were matched and parsed: values to `null`. **Verification and mutation falsification.** -- Targeted correlator suite (`signature-b-correlate.test.ts`): expanded from 23 to 38 tests - (36 pass, 2 skip for deliberate platform-gated checks, 0 fail). +- Targeted correlator suite (`signature-b-correlate.test.ts`): expanded from 23 to 39 tests + (37 pass, 2 skip for deliberate platform-gated checks, 0 fail). - Mutation falsification: four targeted mutations reversing each of the four hardened behaviors were verified to be killed by the expanded test suite. - Monorepo validation: full build, lint, counts, and guard scripts pass cleanly. diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 9a31646e..6c8e3458 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -1349,7 +1349,7 @@ Debt observed during M14. Each states what is known, not what is planned; items - **The published capability document could omit a composed feature, and the client offered controls that could not work (RESOLVED in M15 Increment 24 / ADR-0132).** Left open by Increment 23 above. `capabilitiesView` (`packages/api/src/presenters.ts`) took a hand-written `Pick`, so a feature never added to it was invisible to `GET /v1/capabilities` and nothing complained. ADR-0131 judged that "a narrower failure than a 503 — the feature works for anyone who calls the route directly", and deferred it on the grounds that closing it meant first deciding which optional dependencies are user-facing capabilities. **Both halves of that were wrong.** The failure is narrower only when the client can still reach the feature, and a live instance already existed where it could not: `GET /v1/search` served three modes from two independently-gated dependency sets behind one published `search` flag, so a deployment running the Helm chart’s `search.semanticEnabled: false` advertised search while the client offered two mode buttons whose every request answered 503. And the judgement, while genuinely underivable, can be made **unskippable**, which was the property wanted. `semanticSearch` is now published from both of its dependencies, `Exclude[0] | NotAPublishedCapability>` must be `never` so a new optional dependency cannot compile until someone gives it a flag or records why it has none, and a behavioural guard requires every capability source to change what the document publishes — because a key can sit in that parameter unread, which the compile-time half cannot see. The mutation ledger lives in ADR-0132 and is not restated here. - **The search surface was not gated on `capabilities.search`, so an absolute kill switch still showed a search box (RESOLVED in M15 Increment 24 / ADR-0132 §5).** `SEARCH_ENABLED=0` — the chart's `search.enabled: false`, an absolute kill switch per ADR-0055 — leaves `searchRepository` unconstructed and `GET /v1/search` answering 503 on every mode, keyword included. The entry point was the persistent header form in `packages/web/index.html`, present on every page; being a `` rather than an `a[data-route]`, `NAV_CAPABILITY_MAP` could not reach it, which is why the first pass at Increment 24 gated the semantic and hybrid *modes* and left keyword ungated — the same defect class one mode over. Raised by the Qodo review of PR #155. **Fixed in the same increment rather than deferred:** the form now ships `hidden` and is revealed by `applySearchCapability` only on an explicit `search: true`; the route renders an honest unavailable notice and issues no request; and keyword search waits for the capability answer, reversing this increment's own earlier latency decision, because knowing whether a request is pointless requires having asked. A markup-contract test pins the `hidden` attribute, since the gate depends on it and every other test passes without it. - **Clicking a search mode discarded text typed since the page loaded (RESOLVED in a follow-up to M15 Increment 24 / ADR-0132).** `createModeInput` in `packages/web/src/app/search-mount.ts` closed over the query captured when the route mounted, so `navigateToSearchMode` navigated with the old term and the remount reset the input to match it. Type a new term into the header field, click **Semantic** without pressing enter, and the typed text was gone with no indication it had been discarded. Pre-existing — the closure predated Increment 24 and was untouched by it — and found by the adversarial review of PR #155 while reviewing the capability gate wrapped around the same control. **Resolved:** the query is now a `() => string` read when a mode is chosen rather than a string captured when the selector renders, matching what `main.ts`'s submit handler already does, and falling back to the mounted query only where the document has no header input. Two regression tests, one of which fails against the exact pre-fix closure. -- **`startHarness` drew ports `fetch` refuses (RESOLVED in ADR-0140); the unexplained whole-file failure is still open.** Two signatures were filed here, deliberately not as one cause, and that judgement held. **Signature A is resolved.** WHATWG Fetch blocks eighty-two ports and undici enforces the list on the port number alone, before opening a socket — so `server.listen(0)` could bind, listen and answer raw TCP while `fetch` still refused, surfacing as `TypeError: fetch failed` / `Error: bad port` at the harness’s first request rather than at the listen that caused it. Whether it can happen at all is a property of the host’s dynamic port range: a typical Linux CI range (32768–60999) contains no blocked port, while the Windows range in use here (1024–15000) contains nineteen, which is the whole of "green on CI, flaky locally". The guard that already existed was incomplete in a way that still failed — its hand-observed set of eighteen ports was the spec list intersected with one machine’s range **minus `6679`** — and it retried unboundedly, had no behaviour on exhaustion, and had been copy-pasted into `auth-signin-schema.integration.test.ts`, so the missing port had to be found twice. `packages/api/test/listen.ts` now owns port acquisition for both sites: the spec-complete eighty-two ports (verified by sweeping all 65535 through the real `fetch` on Node v24.15.0), a bounded twenty attempts, each rejected listener closed before the next is asked for, a guarded `address()` read in place of the `as AddressInfo` cast, and an exhaustion error naming the attempts and rejected ports and nothing else. **Signature B is not resolved and was not folded in.** A file still fails with `'test failed'`, no assertion, no stack and none of its own tests reported. Twenty consecutive full runs gave five failures: on pre-fix code one signature A (`auth.test.js`) and three signature B; on post-fix code one signature B and no signature A. The port fix removes A and leaves B exactly where it was, which is the evidence that they are two defects. Four different files were hit (`move-explanation-route`, `tournament-commentary-route`, `bot-detection-analyze`, `anti-cheat-analysis`), sharing no import beyond `./helpers`; each died in 589–703 ms with no test of its own reporting and no stderr. Refuted with evidence: ephemeral-port exhaustion (113 sockets in TIME_WAIT against a 13977-port range), a `Promise.race` loser becoming an unhandled rejection (`race` subscribes to every promise, confirmed on v24.15.0), a throwing `after`/`afterEach` hook (the affected files use none), and a double `close()` rejecting (awaited in a `finally`, it would be attributed to that test with a stack). A second bounded pass — twelve more full runs under the TAP reporter with a preload recording `uncaughtException`, `unhandledRejection` and any non-zero exit — produced twelve clean runs and captured nothing at the time. **A follow-up increment (`claude/node-test-signature-b`) then captured the defect directly, three more times, on three files never previously implicated** (`rate-limit-atomicity`, `dependency-parity`, `studies-api`) — seven distinct files observed with this symptom to date. Occurrences across seven distinct files make a shared or cross-cutting path more plausible and make a defect confined to one test file less likely, but do not exclude file-specific inputs or lifecycle interactions. An instrumented preload (`packages/api/test/diagnostics/signature-b-preload.cjs`) hooking process-level events — `process.exit`, `process.abort`, `process.kill`, `uncaughtExceptionMonitor` (passively observing uncaught exceptions and fatal unhandled rejections), `warning`, `beforeExit`, and Node’s own unconditional `exit` — showed **none of the hooks active at the time fired** on any of the three historical captures (though `process.abort()` was not wrapped in those initial runs and is now covered for future occurrences). A synthetic `process.exit(1)`-before-registration fixture reproduces the identical silent shape; every other synthetic mechanism tried (a post-test async throw, an emitter `'error'` with its listener removed, a synchronous module-load throw, a delayed `SIGKILL`) prints a visibly different diagnostic line, stack, or partial test output that the real defect never shows. This narrows the investigated possibilities while leaving the root cause unresolved: the per-file child process (`node --test` spawns one per file, confirmed by distinct PIDs) was not terminated by `process.exit`, uncaught exceptions, or fatal unhandled rejections, and future runs with `process.abort` instrumentation will record whether abort was called through JS; an absent record narrows in-runtime JS termination but cannot alone prove external termination without corroborating child exit status/signal data or OS-level crash evidence (e.g. distinguishing an external kill or uncatchable signal from a native C++/V8 crash). The machine had roughly 2.5 GB of 15.7 GB RAM free at capture time with several other agents’ processes concurrently running, which is circumstantially consistent with resource contention, but no crash was recorded in the Windows Application or System event logs in that window, so the exact external trigger is still not established. No fix was invented — the forbidden responses (sleeps, whole-file retries, lowering concurrency) would only hide the unresolved root cause, whose origin is not yet established. **A further increment then crossed the parent/child boundary the earlier work stopped at, and found the evidence had been there all along:** Node's runner attaches the child's `exitCode` and `signal` to the `ERR_TEST_FAILURE` it throws, and the `spec` reporter discards them — `formatError` replaces the error with `error.cause`, the bare string `'test failed'` — while the built-in `tap` reporter serializes them, so running `spec` to stdout and `tap` to a file recovers the exit status with no custom reporter and no patched internals. Exit codes were measured on this platform rather than assumed: `process.abort()` gives `134`, `Stop-Process -Force` gives `4294967295`, NTSTATUS faults surface as raw unsigned values such as `3221225477` (`0xC0000005`) — and `1` is produced alike by an uncaught exception, `process.exit(1)`, `taskkill /F` and `process.kill`, so it identifies nothing on its own and is classified `inconclusive`. `signature-b-correlate.cjs` joins the parent's TAP record to the child's JSONL log on the test file path (which also yields the child PID) and states what the pair does and does not establish; where the exit code is ambiguous, a child that reached `preload-installed` and then logged nothing still excludes `process.exit` and an uncaught exception, because both would have left a record and fired Node's `exit` event. A bounded pass of 20 runs under this instrumentation produced 0 captures — which bounds the rate and proves nothing: treating the historical ~1-in-5 as an independent per-run rate, zero captures in 20 runs has probability `(4/5)^20 ≈ 1.2%`, and independence is an assumption rather than an established fact; it ran at 3084–3834 MB free against roughly 2.5 GB at the historical captures, consistent with the resource-contention hypothesis but not evidence for it. **Signature B stays UNRESOLVED**; what changed is that the next occurrence is readable rather than silent. **During M15 Increment 46 validation, Signature B was directly observed three more times, broadening the observed scope to `packages/persistence`:** `search-backfill.integration.test.ts`, `learning.integration.test.ts`, and `test-database.integration.test.ts` each died with the documented bare whole-file `'test failed'` with zero tests reporting and no assertion or stack, and each passed standalone against the exact database state it died on. This confirms the defect is not specific to `packages/api` or its HTTP server test harness. **M15 Increment 47 hardened the diagnostic correlator across 26 bounded runs:** 1 baseline full-suite run, 20 sequential runs under `run-signature-b-pass.mjs`, and 5 concurrent repository-native load runs produced 0 captures (which bounds the rate under those conditions and does not resolve the defect). Code inspection and falsification identified and fixed four correlator blind spots in `packages/api/test/diagnostics/signature-b-correlate.cjs`: (1) cross-directory basename fallback could falsely match a child log from another directory when the target had slashes; (2) Windows path case folding was host-dependent (`process.platform === 'win32'`), breaking cross-platform correlation when logs were analyzed on a different OS; (3) signed 32-bit Windows NTSTATUS exit codes (e.g. `0xC0000005` surfacing as `-1073741819`) were not normalized to unsigned values; and (4) quoted TAP YAML scalar tokens lost exit code or duration numbers. The targeted test suite was expanded from 23 to 38 tests (36 pass, 2 skip, 0 fail), killing 4 falsification mutations. **Signature B remains UNRESOLVED;** no production fix was invented, and the next occurrence will be correlated without cross-platform or exit-code blind spots. See ADR-0140 §4. +- **`startHarness` drew ports `fetch` refuses (RESOLVED in ADR-0140); the unexplained whole-file failure is still open.** Two signatures were filed here, deliberately not as one cause, and that judgement held. **Signature A is resolved.** WHATWG Fetch blocks eighty-two ports and undici enforces the list on the port number alone, before opening a socket — so `server.listen(0)` could bind, listen and answer raw TCP while `fetch` still refused, surfacing as `TypeError: fetch failed` / `Error: bad port` at the harness’s first request rather than at the listen that caused it. Whether it can happen at all is a property of the host’s dynamic port range: a typical Linux CI range (32768–60999) contains no blocked port, while the Windows range in use here (1024–15000) contains nineteen, which is the whole of "green on CI, flaky locally". The guard that already existed was incomplete in a way that still failed — its hand-observed set of eighteen ports was the spec list intersected with one machine’s range **minus `6679`** — and it retried unboundedly, had no behaviour on exhaustion, and had been copy-pasted into `auth-signin-schema.integration.test.ts`, so the missing port had to be found twice. `packages/api/test/listen.ts` now owns port acquisition for both sites: the spec-complete eighty-two ports (verified by sweeping all 65535 through the real `fetch` on Node v24.15.0), a bounded twenty attempts, each rejected listener closed before the next is asked for, a guarded `address()` read in place of the `as AddressInfo` cast, and an exhaustion error naming the attempts and rejected ports and nothing else. **Signature B is not resolved and was not folded in.** A file still fails with `'test failed'`, no assertion, no stack and none of its own tests reported. Twenty consecutive full runs gave five failures: on pre-fix code one signature A (`auth.test.js`) and three signature B; on post-fix code one signature B and no signature A. The port fix removes A and leaves B exactly where it was, which is the evidence that they are two defects. Four different files were hit (`move-explanation-route`, `tournament-commentary-route`, `bot-detection-analyze`, `anti-cheat-analysis`), sharing no import beyond `./helpers`; each died in 589–703 ms with no test of its own reporting and no stderr. Refuted with evidence: ephemeral-port exhaustion (113 sockets in TIME_WAIT against a 13977-port range), a `Promise.race` loser becoming an unhandled rejection (`race` subscribes to every promise, confirmed on v24.15.0), a throwing `after`/`afterEach` hook (the affected files use none), and a double `close()` rejecting (awaited in a `finally`, it would be attributed to that test with a stack). A second bounded pass — twelve more full runs under the TAP reporter with a preload recording `uncaughtException`, `unhandledRejection` and any non-zero exit — produced twelve clean runs and captured nothing at the time. **A follow-up increment (`claude/node-test-signature-b`) then captured the defect directly, three more times, on three files never previously implicated** (`rate-limit-atomicity`, `dependency-parity`, `studies-api`) — seven distinct files observed with this symptom to date. Occurrences across seven distinct files make a shared or cross-cutting path more plausible and make a defect confined to one test file less likely, but do not exclude file-specific inputs or lifecycle interactions. An instrumented preload (`packages/api/test/diagnostics/signature-b-preload.cjs`) hooking process-level events — `process.exit`, `process.abort`, `process.kill`, `uncaughtExceptionMonitor` (passively observing uncaught exceptions and fatal unhandled rejections), `warning`, `beforeExit`, and Node’s own unconditional `exit` — showed **none of the hooks active at the time fired** on any of the three historical captures (though `process.abort()` was not wrapped in those initial runs and is now covered for future occurrences). A synthetic `process.exit(1)`-before-registration fixture reproduces the identical silent shape; every other synthetic mechanism tried (a post-test async throw, an emitter `'error'` with its listener removed, a synchronous module-load throw, a delayed `SIGKILL`) prints a visibly different diagnostic line, stack, or partial test output that the real defect never shows. This narrows the investigated possibilities while leaving the root cause unresolved: the per-file child process (`node --test` spawns one per file, confirmed by distinct PIDs) was not terminated by `process.exit`, uncaught exceptions, or fatal unhandled rejections, and future runs with `process.abort` instrumentation will record whether abort was called through JS; an absent record narrows in-runtime JS termination but cannot alone prove external termination without corroborating child exit status/signal data or OS-level crash evidence (e.g. distinguishing an external kill or uncatchable signal from a native C++/V8 crash). The machine had roughly 2.5 GB of 15.7 GB RAM free at capture time with several other agents’ processes concurrently running, which is circumstantially consistent with resource contention, but no crash was recorded in the Windows Application or System event logs in that window, so the exact external trigger is still not established. No fix was invented — the forbidden responses (sleeps, whole-file retries, lowering concurrency) would only hide the unresolved root cause, whose origin is not yet established. **A further increment then crossed the parent/child boundary the earlier work stopped at, and found the evidence had been there all along:** Node's runner attaches the child's `exitCode` and `signal` to the `ERR_TEST_FAILURE` it throws, and the `spec` reporter discards them — `formatError` replaces the error with `error.cause`, the bare string `'test failed'` — while the built-in `tap` reporter serializes them, so running `spec` to stdout and `tap` to a file recovers the exit status with no custom reporter and no patched internals. Exit codes were measured on this platform rather than assumed: `process.abort()` gives `134`, `Stop-Process -Force` gives `4294967295`, NTSTATUS faults surface as raw unsigned values such as `3221225477` (`0xC0000005`) — and `1` is produced alike by an uncaught exception, `process.exit(1)`, `taskkill /F` and `process.kill`, so it identifies nothing on its own and is classified `inconclusive`. `signature-b-correlate.cjs` joins the parent's TAP record to the child's JSONL log on the test file path (which also yields the child PID) and states what the pair does and does not establish; where the exit code is ambiguous, a child that reached `preload-installed` and then logged nothing still excludes `process.exit` and an uncaught exception, because both would have left a record and fired Node's `exit` event. A bounded pass of 20 runs under this instrumentation produced 0 captures — which bounds the rate and proves nothing: treating the historical ~1-in-5 as an independent per-run rate, zero captures in 20 runs has probability `(4/5)^20 ≈ 1.2%`, and independence is an assumption rather than an established fact; it ran at 3084–3834 MB free against roughly 2.5 GB at the historical captures, consistent with the resource-contention hypothesis but not evidence for it. **Signature B stays UNRESOLVED**; what changed is that the next occurrence is readable rather than silent. **During M15 Increment 46 validation, Signature B was directly observed three more times, broadening the observed scope to `packages/persistence`:** `search-backfill.integration.test.ts`, `learning.integration.test.ts`, and `test-database.integration.test.ts` each died with the documented bare whole-file `'test failed'` with zero tests reporting and no assertion or stack, and each passed standalone against the exact database state it died on. This confirms the defect is not specific to `packages/api` or its HTTP server test harness. **M15 Increment 47 hardened the diagnostic correlator across 26 bounded runs:** 1 baseline full-suite run, 20 sequential runs under `run-signature-b-pass.mjs`, and 5 concurrent repository-native load runs produced 0 captures (which bounds the rate under those conditions and does not resolve the defect). Code inspection and falsification identified and fixed four correlator blind spots in `packages/api/test/diagnostics/signature-b-correlate.cjs`: (1) cross-directory basename fallback could falsely match a child log from another directory when the target had slashes; (2) Windows path case folding was host-dependent (`process.platform === 'win32'`), breaking cross-platform correlation when logs were analyzed on a different OS; (3) signed 32-bit Windows NTSTATUS exit codes (e.g. `0xC0000005` surfacing as `-1073741819`) were not normalized to unsigned values; and (4) quoted TAP YAML scalar tokens lost exit code or duration numbers. The targeted test suite was expanded from 23 to 39 tests (37 pass, 2 skip, 0 fail), killing 4 falsification mutations. **Signature B remains UNRESOLVED;** no production fix was invented, and the next occurrence will be correlated without cross-platform or exit-code blind spots. See ADR-0140 §4. - **Isolated-database test teardown dropped databases out from under connections that had not finished closing (RESOLVED in M15 Increment 45).** `withDatabase` in `packages/persistence/test/variant-migrations.integration.test.ts` ended its pool and then immediately ran `DROP DATABASE ... WITH (FORCE)`. `pool.end()` does not wait for its clients to close: in pg 8.22.0 `_pulseQueue` reaches the end callback in the same synchronous turn in which `_remove` filters the last client out of `_clients`, while `client.end()` has only queued the Terminate byte — instrumentation recorded **zero of four `remove` events fired at the moment `end()` resolved**. The drop could therefore still find a backend attached; `FORCE` terminated it, and the resulting `FATAL` arrived on a socket whose pool still had `idleListener` attached, which `pg` re-emitted as `pool.emit('error')` — an unhandled EventEmitter error that `node:test` attributed to whichever test was running rather than to the teardown that caused it. It surfaced as intermittent `terminating connection due to administrator command` failures in `postgres integration (persistence)` during M15 Increment 44, on a different test each run, which is the signature of a race rather than a broken assertion. The same shape existed in `packages/api/test/auth-signin-schema.integration.test.ts`, which had absorbed SQLSTATE 57P01 with a `pool.on('error', ...)` listener — a symptom fix for the same cause. **Resolved in Increment 45:** a shared `withTestDatabase` helper (`@chess-platform/persistence/test-support`) ends the pool under a bound, waits for `pg_stat_activity` to report the database unused, and drops it *without* `FORCE`. Measured on PostgreSQL 16.14, a plain drop against a still-attached backend fails with SQLSTATE 55006 and leaves that connection untouched, where `FORCE` succeeds by killing it — so the change trades a quiet, harmful success for a loud, harmless failure. FORCE remains only on the emergency path that guarantees the disposable database is still dropped once teardown has already failed — best effort, since that last drop runs inside a `catch` so it cannot bury the error being reported. The 57P01 absorber is deleted, because the corrected lifecycle never causes one. `createPool` and `migrate` are unchanged, and no migration was added. - **The persistence integration suite was not idempotent against a reused database (RESOLVED in M15 Increment 46).** Recorded as a known defect by Increment 45 and left open there. Against a fresh PostgreSQL 16 database the suite passed; a second run against the *same* database failed nine tests, deterministically — measured on 16.14 before any edit as **173 pass / 0 fail** then **164 pass / 9 fail**. CI provisions a fresh server per run, so it never surfaced there. One contract was being broken in two directions: a suite sharing `chess_test` must remove every row it created and remove nothing else. `achievements.integration.test.ts` broke the second half with an unqualified `DELETE FROM users` in `beforeEach` — `games.white_id` and `games.black_id` are the only references to `users` without `ON DELETE CASCADE` (thirty-one FKs point at `users`; twenty-nine cascade; the two that do not are both on `games`), so one game left behind by `pg.integration.test.ts` aborted the wipe with SQLSTATE 23503 before any assertion ran, and the same statement destroyed the bot accounts migration 0021 seeds, which nothing restores because `migrate` has already recorded 0021 as applied. `pg/identity-tokens.test.ts` and `tournaments.pg.integration.test.ts` broke the first half, leaving fixed primary keys behind and colliding on `users_pkey` and `tournaments_pkey` (the latter surfacing through the repository's compare-and-set as `VersionConflictError`). **Resolved in Increment 46:** suites that legitimately share the database delete exactly their own rows through `withSharedDatabase` (`packages/persistence/src/test-support/fixtures.ts`), the sibling of Increment 45's `withTestDatabase` and heir to its precedence rule — a cleanup failure never replaces the assertion that actually failed, and never disappears either. `pg.integration.test.ts` moved to disposable databases instead, because it cannot meet the cleanup half at all: it appends to `game_events`, which is append-only by production trigger, so cleaning up after itself would have meant weakening a production safety rule to suit a test. `users-batch`, `anti-cheat`, `bot-reports` and `analysis-cache` were corrected for the same contract though none of them ever failed — fresh `uuidv7()` ids meant their leaked rows could not collide, so the tables merely grew on every run. Serialization was not the fix and was not introduced: `--test-concurrency=1` is a pre-existing documented invariant and the second run failed identically under it. Acceptance is three consecutive runs against one database with no reset between them (186/186/186), after which only the three migration-seeded bot accounts remain, with no leaked disposable databases and no lingering backends; falsification killed 17 of 20 mutations. No production code, migration, checksum, constraint or repository conflict semantic changed, and no migration was added. - **`ApiServer.listen` registered no `'error'` handler, so a failed bind hung and raised an uncaught event (RESOLVED in ADR-0140 §5).** `packages/api/src/server.ts` resolved its promise from the `listening` callback only and built it with no reject path. A bind failing asynchronously (`EADDRINUSE`, `EMFILE`) left the promise pending forever and, with no `'error'` listener on the `http.Server`, was re-raised as an uncaught exception. Found while investigating ADR-0140 and independently raised by the Qodo review of PR #21. **Resolved in the same increment** rather than deferred, because ADR-0140 §2's bounded, diagnosable acquisition is not true without it — the retry can only report a bind error if the listener it is handed rejects. A one-shot `'error'` listener now rejects and is removed once listening, so later server errors keep their previous semantics rather than being swallowed by a `reject` on a settled promise. The regression test fails against the exact pre-fix code, through an uncaught `ERR_UNHANDLED_ERROR`. diff --git a/packages/api/test/diagnostics/signature-b-correlate.cjs b/packages/api/test/diagnostics/signature-b-correlate.cjs index f1ec4052..bb33780e 100644 --- a/packages/api/test/diagnostics/signature-b-correlate.cjs +++ b/packages/api/test/diagnostics/signature-b-correlate.cjs @@ -254,12 +254,15 @@ function narrowFromChildEvidence(classification, childKinds) { * failed, so the runner falls back to `kTestCodeFailure`. Matching on the message alone would * report every ordinary regression as a capture and halt the pass on a false positive. * +/** * @param {string} tapText - * @returns {Array<{ file: string, exitCode: number | null, signal: string | null, failureType: string | null, durationMs: number | null }>} + * @param {{ isWindows?: boolean }} [options] + * @returns {Array<{ file: string, exitCode: number | null, signal: string | null, failureType: string | null, durationMs: number | null, isWindows?: boolean }>} */ -function parseTapFailures(tapText) { +function parseTapFailures(tapText, options = {}) { const blocks = String(tapText).matchAll(/^not ok \d+ - (.+?)$\r?\n\s*---\r?\n([\s\S]*?)^\s*\.\.\.$/gm); const out = []; + const isWin = typeof options?.isWindows === 'boolean' ? options.isWindows : undefined; for (const [, rawName, body] of blocks) { if (!/^\s*error:\s*['"]?test failed['"]?\s*$/m.test(body)) continue; if (!/^\s*failureType:\s*['"]?testCodeFailure['"]?\s*$/m.test(body)) continue; @@ -280,6 +283,7 @@ function parseTapFailures(tapText) { signal: sig === null || sig === '~' ? null : sig, failureType: unquote(scalar('failureType')), durationMs: parsedDur !== null && Number.isFinite(parsedDur) ? parsedDur : null, + ...(typeof isWin === 'boolean' ? { isWindows: isWin } : {}), }); } return out; @@ -375,9 +379,7 @@ function matchChild(childLogs, parentFile, parentIsWindows) { const bySuffix = entries.filter(([key, child]) => { const isWin = Boolean(child?.isWindows || isWantedWin); - const isRelWin = Boolean(child?.isWindows && !isWantedWin && !parentFile.includes('/') && parentFile.includes('\\')); - const effectiveParentNorm = isRelWin ? normalizePath(parentFile, true) : parentNorm; - const wantedNorm = isWin ? effectiveParentNorm.toLowerCase() : effectiveParentNorm; + const wantedNorm = isWin ? parentNorm.toLowerCase() : parentNorm; const keyNorm = isWin ? key.toLowerCase() : key; return keyNorm === wantedNorm || keyNorm.endsWith(`/${wantedNorm}`); }); @@ -391,8 +393,6 @@ function matchChild(childLogs, parentFile, parentIsWindows) { const base = fileKey(parentFile, isWantedWin); const byBase = entries.filter(([key, child]) => { const isWin = Boolean(child?.isWindows || isWantedWin); - const isRelWin = Boolean(child?.isWindows && !isWantedWin && !parentFile.includes('/') && parentFile.includes('\\')); - if (isRelWin) return false; const kBase = fileKey(key, isWin); if (isWin) { return kBase.toLowerCase() === base.toLowerCase(); @@ -422,12 +422,14 @@ function matchChild(childLogs, parentFile, parentIsWindows) { */ function correlate(failures, childLogs, options = {}) { const defaultIsWin = typeof options?.isWindows === 'boolean' ? options.isWindows : undefined; - return failures.map((failure) => { - const file = typeof failure === 'string' ? failure : failure.file; + return failures.map((rawFailure) => { + const failure = typeof rawFailure === 'string' + ? { file: rawFailure, exitCode: null, signal: null, failureType: null, durationMs: null } + : rawFailure; const parentIsWin = typeof failure?.isWindows === 'boolean' ? failure.isWindows : defaultIsWin; - const match = matchChild(childLogs, file, parentIsWin); + const match = matchChild(childLogs, failure.file, parentIsWin); const kinds = match.child?.kinds ?? []; const classification = classifyTermination(failure); const narrowing = narrowFromChildEvidence(classification, kinds); @@ -492,7 +494,12 @@ if (require.main === module) { process.stderr.write(`${usage ?? 'usage'}: signature-b-correlate.cjs --tap --child-logs \n`); process.exitCode = 2; } else { - const records = correlate(parseTapFailures(fs.readFileSync(tapPath, 'utf8')), readChildLogs(logDir)); + const isWin = process.platform === 'win32'; + const records = correlate( + parseTapFailures(fs.readFileSync(tapPath, 'utf8'), { isWindows: isWin }), + readChildLogs(logDir), + { isWindows: isWin }, + ); for (const record of records) process.stdout.write(`${JSON.stringify(record)}\n`); if (records.length === 0) process.stderr.write('no Signature B failures in this report\n'); } diff --git a/packages/api/test/diagnostics/signature-b-correlate.test.ts b/packages/api/test/diagnostics/signature-b-correlate.test.ts index a24bc230..911b2da4 100644 --- a/packages/api/test/diagnostics/signature-b-correlate.test.ts +++ b/packages/api/test/diagnostics/signature-b-correlate.test.ts @@ -52,12 +52,13 @@ const correlator = require(CORRELATE_PATH) as { c: { id: string; specific: boolean }, kinds: readonly string[], ): { narrowed: boolean; statement: string }; - parseTapFailures(tap: string): Array<{ + parseTapFailures(tap: string, options?: { isWindows?: boolean }): Array<{ file: string; exitCode: number | null; signal: string | null; failureType: string | null; durationMs: number | null; + isWindows?: boolean; }>; readChildLogs(dir: string): Map; matchChild(logs: Map, file: string, parentIsWindows?: boolean): { child: unknown; ambiguous: boolean; candidates: number }; @@ -1034,7 +1035,7 @@ test('correlator: POSIX parent path with backslash filename does not falsely mat } }); -test('correlator: relative backslash-delimited Windows parent path matches Windows child path', () => { +test('correlator: relative backslash-delimited Windows parent path matches Windows child path with Windows platform context', () => { const logDir = fs.mkdtempSync(path.join(os.tmpdir(), 'sigb-relwin-parent-')); try { const lines = [ @@ -1044,10 +1045,11 @@ test('correlator: relative backslash-delimited Windows parent path matches Windo fs.writeFileSync(path.join(logDir, 'run-win.jsonl'), `${lines.map((l) => JSON.stringify(l)).join('\n')}\n`); const logs = correlator.readChildLogs(logDir); - // Parent uses backslash-delimited relative path + // Parent uses backslash-delimited relative path with explicit Windows platform context const resolved = correlator.correlate( - correlator.parseTapFailures(tapFailure('dist-test\\test\\foo.test.js', '1')), + correlator.parseTapFailures(tapFailure('dist-test\\test\\foo.test.js', '1'), { isWindows: true }), logs, + { isWindows: true }, ); assert.equal(resolved.length, 1); assert.equal(resolved[0]?.child.pid, 999, 'relative Windows backslash parent must match Windows child'); @@ -1057,3 +1059,27 @@ test('correlator: relative backslash-delimited Windows parent path matches Windo fs.rmSync(logDir, { recursive: true, force: true }); } }); + +test('correlator: correlates bare string failures with normalized null parent fields', () => { + const logDir = fs.mkdtempSync(path.join(os.tmpdir(), 'sigb-string-failure-')); + try { + const lines = [ + { kind: 'start', pid: 777, testFile: '/home/runner/repo/dist-test/test/studies-api.test.js' }, + { kind: 'preload-installed', pid: 777, testFile: '/home/runner/repo/dist-test/test/studies-api.test.js' }, + ]; + fs.writeFileSync(path.join(logDir, 'run-str.jsonl'), `${lines.map((l) => JSON.stringify(l)).join('\n')}\n`); + const logs = correlator.readChildLogs(logDir); + + const resolved = correlator.correlate(['dist-test/test/studies-api.test.js'], logs); + assert.equal(resolved.length, 1); + assert.equal(resolved[0]?.file, 'dist-test/test/studies-api.test.js'); + assert.equal(resolved[0]?.parent.exitCode, null); + assert.equal(resolved[0]?.parent.signal, null); + assert.equal(resolved[0]?.parent.failureType, null); + assert.equal(resolved[0]?.parent.durationMs, null); + assert.equal(resolved[0]?.child.pid, 777); + assert.equal(resolved[0]?.child.logFound, true); + } finally { + fs.rmSync(logDir, { recursive: true, force: true }); + } +}); From a4610735d7af557bd3f30a48d40d4b25dc6803a0 Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Sat, 5 Sep 2026 00:06:22 +0300 Subject: [PATCH 12/13] fix(api-diagnostics): propagate platform context from runner and derive CLI platform from capture metadata --- docs/PROJECT_STATE.md | 4 +- docs/ROADMAP.md | 2 +- .../test/diagnostics/run-signature-b-pass.mjs | 7 ++- .../diagnostics/signature-b-correlate.cjs | 14 ++++-- .../diagnostics/signature-b-correlate.test.ts | 44 +++++++++++++++++++ 5 files changed, 63 insertions(+), 8 deletions(-) diff --git a/docs/PROJECT_STATE.md b/docs/PROJECT_STATE.md index fca61dbb..8d78d00b 100644 --- a/docs/PROJECT_STATE.md +++ b/docs/PROJECT_STATE.md @@ -59,8 +59,8 @@ parent TAP diagnostic logs and child JSONL preloads were matched and parsed: values to `null`. **Verification and mutation falsification.** -- Targeted correlator suite (`signature-b-correlate.test.ts`): expanded from 23 to 39 tests - (37 pass, 2 skip for deliberate platform-gated checks, 0 fail). +- Targeted correlator suite (`signature-b-correlate.test.ts`): expanded from 23 to 40 tests + (38 pass, 2 skip for deliberate platform-gated checks, 0 fail). - Mutation falsification: four targeted mutations reversing each of the four hardened behaviors were verified to be killed by the expanded test suite. - Monorepo validation: full build, lint, counts, and guard scripts pass cleanly. diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 6c8e3458..a0f2b951 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -1349,7 +1349,7 @@ Debt observed during M14. Each states what is known, not what is planned; items - **The published capability document could omit a composed feature, and the client offered controls that could not work (RESOLVED in M15 Increment 24 / ADR-0132).** Left open by Increment 23 above. `capabilitiesView` (`packages/api/src/presenters.ts`) took a hand-written `Pick`, so a feature never added to it was invisible to `GET /v1/capabilities` and nothing complained. ADR-0131 judged that "a narrower failure than a 503 — the feature works for anyone who calls the route directly", and deferred it on the grounds that closing it meant first deciding which optional dependencies are user-facing capabilities. **Both halves of that were wrong.** The failure is narrower only when the client can still reach the feature, and a live instance already existed where it could not: `GET /v1/search` served three modes from two independently-gated dependency sets behind one published `search` flag, so a deployment running the Helm chart’s `search.semanticEnabled: false` advertised search while the client offered two mode buttons whose every request answered 503. And the judgement, while genuinely underivable, can be made **unskippable**, which was the property wanted. `semanticSearch` is now published from both of its dependencies, `Exclude[0] | NotAPublishedCapability>` must be `never` so a new optional dependency cannot compile until someone gives it a flag or records why it has none, and a behavioural guard requires every capability source to change what the document publishes — because a key can sit in that parameter unread, which the compile-time half cannot see. The mutation ledger lives in ADR-0132 and is not restated here. - **The search surface was not gated on `capabilities.search`, so an absolute kill switch still showed a search box (RESOLVED in M15 Increment 24 / ADR-0132 §5).** `SEARCH_ENABLED=0` — the chart's `search.enabled: false`, an absolute kill switch per ADR-0055 — leaves `searchRepository` unconstructed and `GET /v1/search` answering 503 on every mode, keyword included. The entry point was the persistent header form in `packages/web/index.html`, present on every page; being a `` rather than an `a[data-route]`, `NAV_CAPABILITY_MAP` could not reach it, which is why the first pass at Increment 24 gated the semantic and hybrid *modes* and left keyword ungated — the same defect class one mode over. Raised by the Qodo review of PR #155. **Fixed in the same increment rather than deferred:** the form now ships `hidden` and is revealed by `applySearchCapability` only on an explicit `search: true`; the route renders an honest unavailable notice and issues no request; and keyword search waits for the capability answer, reversing this increment's own earlier latency decision, because knowing whether a request is pointless requires having asked. A markup-contract test pins the `hidden` attribute, since the gate depends on it and every other test passes without it. - **Clicking a search mode discarded text typed since the page loaded (RESOLVED in a follow-up to M15 Increment 24 / ADR-0132).** `createModeInput` in `packages/web/src/app/search-mount.ts` closed over the query captured when the route mounted, so `navigateToSearchMode` navigated with the old term and the remount reset the input to match it. Type a new term into the header field, click **Semantic** without pressing enter, and the typed text was gone with no indication it had been discarded. Pre-existing — the closure predated Increment 24 and was untouched by it — and found by the adversarial review of PR #155 while reviewing the capability gate wrapped around the same control. **Resolved:** the query is now a `() => string` read when a mode is chosen rather than a string captured when the selector renders, matching what `main.ts`'s submit handler already does, and falling back to the mounted query only where the document has no header input. Two regression tests, one of which fails against the exact pre-fix closure. -- **`startHarness` drew ports `fetch` refuses (RESOLVED in ADR-0140); the unexplained whole-file failure is still open.** Two signatures were filed here, deliberately not as one cause, and that judgement held. **Signature A is resolved.** WHATWG Fetch blocks eighty-two ports and undici enforces the list on the port number alone, before opening a socket — so `server.listen(0)` could bind, listen and answer raw TCP while `fetch` still refused, surfacing as `TypeError: fetch failed` / `Error: bad port` at the harness’s first request rather than at the listen that caused it. Whether it can happen at all is a property of the host’s dynamic port range: a typical Linux CI range (32768–60999) contains no blocked port, while the Windows range in use here (1024–15000) contains nineteen, which is the whole of "green on CI, flaky locally". The guard that already existed was incomplete in a way that still failed — its hand-observed set of eighteen ports was the spec list intersected with one machine’s range **minus `6679`** — and it retried unboundedly, had no behaviour on exhaustion, and had been copy-pasted into `auth-signin-schema.integration.test.ts`, so the missing port had to be found twice. `packages/api/test/listen.ts` now owns port acquisition for both sites: the spec-complete eighty-two ports (verified by sweeping all 65535 through the real `fetch` on Node v24.15.0), a bounded twenty attempts, each rejected listener closed before the next is asked for, a guarded `address()` read in place of the `as AddressInfo` cast, and an exhaustion error naming the attempts and rejected ports and nothing else. **Signature B is not resolved and was not folded in.** A file still fails with `'test failed'`, no assertion, no stack and none of its own tests reported. Twenty consecutive full runs gave five failures: on pre-fix code one signature A (`auth.test.js`) and three signature B; on post-fix code one signature B and no signature A. The port fix removes A and leaves B exactly where it was, which is the evidence that they are two defects. Four different files were hit (`move-explanation-route`, `tournament-commentary-route`, `bot-detection-analyze`, `anti-cheat-analysis`), sharing no import beyond `./helpers`; each died in 589–703 ms with no test of its own reporting and no stderr. Refuted with evidence: ephemeral-port exhaustion (113 sockets in TIME_WAIT against a 13977-port range), a `Promise.race` loser becoming an unhandled rejection (`race` subscribes to every promise, confirmed on v24.15.0), a throwing `after`/`afterEach` hook (the affected files use none), and a double `close()` rejecting (awaited in a `finally`, it would be attributed to that test with a stack). A second bounded pass — twelve more full runs under the TAP reporter with a preload recording `uncaughtException`, `unhandledRejection` and any non-zero exit — produced twelve clean runs and captured nothing at the time. **A follow-up increment (`claude/node-test-signature-b`) then captured the defect directly, three more times, on three files never previously implicated** (`rate-limit-atomicity`, `dependency-parity`, `studies-api`) — seven distinct files observed with this symptom to date. Occurrences across seven distinct files make a shared or cross-cutting path more plausible and make a defect confined to one test file less likely, but do not exclude file-specific inputs or lifecycle interactions. An instrumented preload (`packages/api/test/diagnostics/signature-b-preload.cjs`) hooking process-level events — `process.exit`, `process.abort`, `process.kill`, `uncaughtExceptionMonitor` (passively observing uncaught exceptions and fatal unhandled rejections), `warning`, `beforeExit`, and Node’s own unconditional `exit` — showed **none of the hooks active at the time fired** on any of the three historical captures (though `process.abort()` was not wrapped in those initial runs and is now covered for future occurrences). A synthetic `process.exit(1)`-before-registration fixture reproduces the identical silent shape; every other synthetic mechanism tried (a post-test async throw, an emitter `'error'` with its listener removed, a synchronous module-load throw, a delayed `SIGKILL`) prints a visibly different diagnostic line, stack, or partial test output that the real defect never shows. This narrows the investigated possibilities while leaving the root cause unresolved: the per-file child process (`node --test` spawns one per file, confirmed by distinct PIDs) was not terminated by `process.exit`, uncaught exceptions, or fatal unhandled rejections, and future runs with `process.abort` instrumentation will record whether abort was called through JS; an absent record narrows in-runtime JS termination but cannot alone prove external termination without corroborating child exit status/signal data or OS-level crash evidence (e.g. distinguishing an external kill or uncatchable signal from a native C++/V8 crash). The machine had roughly 2.5 GB of 15.7 GB RAM free at capture time with several other agents’ processes concurrently running, which is circumstantially consistent with resource contention, but no crash was recorded in the Windows Application or System event logs in that window, so the exact external trigger is still not established. No fix was invented — the forbidden responses (sleeps, whole-file retries, lowering concurrency) would only hide the unresolved root cause, whose origin is not yet established. **A further increment then crossed the parent/child boundary the earlier work stopped at, and found the evidence had been there all along:** Node's runner attaches the child's `exitCode` and `signal` to the `ERR_TEST_FAILURE` it throws, and the `spec` reporter discards them — `formatError` replaces the error with `error.cause`, the bare string `'test failed'` — while the built-in `tap` reporter serializes them, so running `spec` to stdout and `tap` to a file recovers the exit status with no custom reporter and no patched internals. Exit codes were measured on this platform rather than assumed: `process.abort()` gives `134`, `Stop-Process -Force` gives `4294967295`, NTSTATUS faults surface as raw unsigned values such as `3221225477` (`0xC0000005`) — and `1` is produced alike by an uncaught exception, `process.exit(1)`, `taskkill /F` and `process.kill`, so it identifies nothing on its own and is classified `inconclusive`. `signature-b-correlate.cjs` joins the parent's TAP record to the child's JSONL log on the test file path (which also yields the child PID) and states what the pair does and does not establish; where the exit code is ambiguous, a child that reached `preload-installed` and then logged nothing still excludes `process.exit` and an uncaught exception, because both would have left a record and fired Node's `exit` event. A bounded pass of 20 runs under this instrumentation produced 0 captures — which bounds the rate and proves nothing: treating the historical ~1-in-5 as an independent per-run rate, zero captures in 20 runs has probability `(4/5)^20 ≈ 1.2%`, and independence is an assumption rather than an established fact; it ran at 3084–3834 MB free against roughly 2.5 GB at the historical captures, consistent with the resource-contention hypothesis but not evidence for it. **Signature B stays UNRESOLVED**; what changed is that the next occurrence is readable rather than silent. **During M15 Increment 46 validation, Signature B was directly observed three more times, broadening the observed scope to `packages/persistence`:** `search-backfill.integration.test.ts`, `learning.integration.test.ts`, and `test-database.integration.test.ts` each died with the documented bare whole-file `'test failed'` with zero tests reporting and no assertion or stack, and each passed standalone against the exact database state it died on. This confirms the defect is not specific to `packages/api` or its HTTP server test harness. **M15 Increment 47 hardened the diagnostic correlator across 26 bounded runs:** 1 baseline full-suite run, 20 sequential runs under `run-signature-b-pass.mjs`, and 5 concurrent repository-native load runs produced 0 captures (which bounds the rate under those conditions and does not resolve the defect). Code inspection and falsification identified and fixed four correlator blind spots in `packages/api/test/diagnostics/signature-b-correlate.cjs`: (1) cross-directory basename fallback could falsely match a child log from another directory when the target had slashes; (2) Windows path case folding was host-dependent (`process.platform === 'win32'`), breaking cross-platform correlation when logs were analyzed on a different OS; (3) signed 32-bit Windows NTSTATUS exit codes (e.g. `0xC0000005` surfacing as `-1073741819`) were not normalized to unsigned values; and (4) quoted TAP YAML scalar tokens lost exit code or duration numbers. The targeted test suite was expanded from 23 to 39 tests (37 pass, 2 skip, 0 fail), killing 4 falsification mutations. **Signature B remains UNRESOLVED;** no production fix was invented, and the next occurrence will be correlated without cross-platform or exit-code blind spots. See ADR-0140 §4. +- **`startHarness` drew ports `fetch` refuses (RESOLVED in ADR-0140); the unexplained whole-file failure is still open.** Two signatures were filed here, deliberately not as one cause, and that judgement held. **Signature A is resolved.** WHATWG Fetch blocks eighty-two ports and undici enforces the list on the port number alone, before opening a socket — so `server.listen(0)` could bind, listen and answer raw TCP while `fetch` still refused, surfacing as `TypeError: fetch failed` / `Error: bad port` at the harness’s first request rather than at the listen that caused it. Whether it can happen at all is a property of the host’s dynamic port range: a typical Linux CI range (32768–60999) contains no blocked port, while the Windows range in use here (1024–15000) contains nineteen, which is the whole of "green on CI, flaky locally". The guard that already existed was incomplete in a way that still failed — its hand-observed set of eighteen ports was the spec list intersected with one machine’s range **minus `6679`** — and it retried unboundedly, had no behaviour on exhaustion, and had been copy-pasted into `auth-signin-schema.integration.test.ts`, so the missing port had to be found twice. `packages/api/test/listen.ts` now owns port acquisition for both sites: the spec-complete eighty-two ports (verified by sweeping all 65535 through the real `fetch` on Node v24.15.0), a bounded twenty attempts, each rejected listener closed before the next is asked for, a guarded `address()` read in place of the `as AddressInfo` cast, and an exhaustion error naming the attempts and rejected ports and nothing else. **Signature B is not resolved and was not folded in.** A file still fails with `'test failed'`, no assertion, no stack and none of its own tests reported. Twenty consecutive full runs gave five failures: on pre-fix code one signature A (`auth.test.js`) and three signature B; on post-fix code one signature B and no signature A. The port fix removes A and leaves B exactly where it was, which is the evidence that they are two defects. Four different files were hit (`move-explanation-route`, `tournament-commentary-route`, `bot-detection-analyze`, `anti-cheat-analysis`), sharing no import beyond `./helpers`; each died in 589–703 ms with no test of its own reporting and no stderr. Refuted with evidence: ephemeral-port exhaustion (113 sockets in TIME_WAIT against a 13977-port range), a `Promise.race` loser becoming an unhandled rejection (`race` subscribes to every promise, confirmed on v24.15.0), a throwing `after`/`afterEach` hook (the affected files use none), and a double `close()` rejecting (awaited in a `finally`, it would be attributed to that test with a stack). A second bounded pass — twelve more full runs under the TAP reporter with a preload recording `uncaughtException`, `unhandledRejection` and any non-zero exit — produced twelve clean runs and captured nothing at the time. **A follow-up increment (`claude/node-test-signature-b`) then captured the defect directly, three more times, on three files never previously implicated** (`rate-limit-atomicity`, `dependency-parity`, `studies-api`) — seven distinct files observed with this symptom to date. Occurrences across seven distinct files make a shared or cross-cutting path more plausible and make a defect confined to one test file less likely, but do not exclude file-specific inputs or lifecycle interactions. An instrumented preload (`packages/api/test/diagnostics/signature-b-preload.cjs`) hooking process-level events — `process.exit`, `process.abort`, `process.kill`, `uncaughtExceptionMonitor` (passively observing uncaught exceptions and fatal unhandled rejections), `warning`, `beforeExit`, and Node’s own unconditional `exit` — showed **none of the hooks active at the time fired** on any of the three historical captures (though `process.abort()` was not wrapped in those initial runs and is now covered for future occurrences). A synthetic `process.exit(1)`-before-registration fixture reproduces the identical silent shape; every other synthetic mechanism tried (a post-test async throw, an emitter `'error'` with its listener removed, a synchronous module-load throw, a delayed `SIGKILL`) prints a visibly different diagnostic line, stack, or partial test output that the real defect never shows. This narrows the investigated possibilities while leaving the root cause unresolved: the per-file child process (`node --test` spawns one per file, confirmed by distinct PIDs) was not terminated by `process.exit`, uncaught exceptions, or fatal unhandled rejections, and future runs with `process.abort` instrumentation will record whether abort was called through JS; an absent record narrows in-runtime JS termination but cannot alone prove external termination without corroborating child exit status/signal data or OS-level crash evidence (e.g. distinguishing an external kill or uncatchable signal from a native C++/V8 crash). The machine had roughly 2.5 GB of 15.7 GB RAM free at capture time with several other agents’ processes concurrently running, which is circumstantially consistent with resource contention, but no crash was recorded in the Windows Application or System event logs in that window, so the exact external trigger is still not established. No fix was invented — the forbidden responses (sleeps, whole-file retries, lowering concurrency) would only hide the unresolved root cause, whose origin is not yet established. **A further increment then crossed the parent/child boundary the earlier work stopped at, and found the evidence had been there all along:** Node's runner attaches the child's `exitCode` and `signal` to the `ERR_TEST_FAILURE` it throws, and the `spec` reporter discards them — `formatError` replaces the error with `error.cause`, the bare string `'test failed'` — while the built-in `tap` reporter serializes them, so running `spec` to stdout and `tap` to a file recovers the exit status with no custom reporter and no patched internals. Exit codes were measured on this platform rather than assumed: `process.abort()` gives `134`, `Stop-Process -Force` gives `4294967295`, NTSTATUS faults surface as raw unsigned values such as `3221225477` (`0xC0000005`) — and `1` is produced alike by an uncaught exception, `process.exit(1)`, `taskkill /F` and `process.kill`, so it identifies nothing on its own and is classified `inconclusive`. `signature-b-correlate.cjs` joins the parent's TAP record to the child's JSONL log on the test file path (which also yields the child PID) and states what the pair does and does not establish; where the exit code is ambiguous, a child that reached `preload-installed` and then logged nothing still excludes `process.exit` and an uncaught exception, because both would have left a record and fired Node's `exit` event. A bounded pass of 20 runs under this instrumentation produced 0 captures — which bounds the rate and proves nothing: treating the historical ~1-in-5 as an independent per-run rate, zero captures in 20 runs has probability `(4/5)^20 ≈ 1.2%`, and independence is an assumption rather than an established fact; it ran at 3084–3834 MB free against roughly 2.5 GB at the historical captures, consistent with the resource-contention hypothesis but not evidence for it. **Signature B stays UNRESOLVED**; what changed is that the next occurrence is readable rather than silent. **During M15 Increment 46 validation, Signature B was directly observed three more times, broadening the observed scope to `packages/persistence`:** `search-backfill.integration.test.ts`, `learning.integration.test.ts`, and `test-database.integration.test.ts` each died with the documented bare whole-file `'test failed'` with zero tests reporting and no assertion or stack, and each passed standalone against the exact database state it died on. This confirms the defect is not specific to `packages/api` or its HTTP server test harness. **M15 Increment 47 hardened the diagnostic correlator across 26 bounded runs:** 1 baseline full-suite run, 20 sequential runs under `run-signature-b-pass.mjs`, and 5 concurrent repository-native load runs produced 0 captures (which bounds the rate under those conditions and does not resolve the defect). Code inspection and falsification identified and fixed four correlator blind spots in `packages/api/test/diagnostics/signature-b-correlate.cjs`: (1) cross-directory basename fallback could falsely match a child log from another directory when the target had slashes; (2) Windows path case folding was host-dependent (`process.platform === 'win32'`), breaking cross-platform correlation when logs were analyzed on a different OS; (3) signed 32-bit Windows NTSTATUS exit codes (e.g. `0xC0000005` surfacing as `-1073741819`) were not normalized to unsigned values; and (4) quoted TAP YAML scalar tokens lost exit code or duration numbers. The targeted test suite was expanded from 23 to 40 tests (38 pass, 2 skip, 0 fail), killing 4 falsification mutations. **Signature B remains UNRESOLVED;** no production fix was invented, and the next occurrence will be correlated without cross-platform or exit-code blind spots. See ADR-0140 §4. - **Isolated-database test teardown dropped databases out from under connections that had not finished closing (RESOLVED in M15 Increment 45).** `withDatabase` in `packages/persistence/test/variant-migrations.integration.test.ts` ended its pool and then immediately ran `DROP DATABASE ... WITH (FORCE)`. `pool.end()` does not wait for its clients to close: in pg 8.22.0 `_pulseQueue` reaches the end callback in the same synchronous turn in which `_remove` filters the last client out of `_clients`, while `client.end()` has only queued the Terminate byte — instrumentation recorded **zero of four `remove` events fired at the moment `end()` resolved**. The drop could therefore still find a backend attached; `FORCE` terminated it, and the resulting `FATAL` arrived on a socket whose pool still had `idleListener` attached, which `pg` re-emitted as `pool.emit('error')` — an unhandled EventEmitter error that `node:test` attributed to whichever test was running rather than to the teardown that caused it. It surfaced as intermittent `terminating connection due to administrator command` failures in `postgres integration (persistence)` during M15 Increment 44, on a different test each run, which is the signature of a race rather than a broken assertion. The same shape existed in `packages/api/test/auth-signin-schema.integration.test.ts`, which had absorbed SQLSTATE 57P01 with a `pool.on('error', ...)` listener — a symptom fix for the same cause. **Resolved in Increment 45:** a shared `withTestDatabase` helper (`@chess-platform/persistence/test-support`) ends the pool under a bound, waits for `pg_stat_activity` to report the database unused, and drops it *without* `FORCE`. Measured on PostgreSQL 16.14, a plain drop against a still-attached backend fails with SQLSTATE 55006 and leaves that connection untouched, where `FORCE` succeeds by killing it — so the change trades a quiet, harmful success for a loud, harmless failure. FORCE remains only on the emergency path that guarantees the disposable database is still dropped once teardown has already failed — best effort, since that last drop runs inside a `catch` so it cannot bury the error being reported. The 57P01 absorber is deleted, because the corrected lifecycle never causes one. `createPool` and `migrate` are unchanged, and no migration was added. - **The persistence integration suite was not idempotent against a reused database (RESOLVED in M15 Increment 46).** Recorded as a known defect by Increment 45 and left open there. Against a fresh PostgreSQL 16 database the suite passed; a second run against the *same* database failed nine tests, deterministically — measured on 16.14 before any edit as **173 pass / 0 fail** then **164 pass / 9 fail**. CI provisions a fresh server per run, so it never surfaced there. One contract was being broken in two directions: a suite sharing `chess_test` must remove every row it created and remove nothing else. `achievements.integration.test.ts` broke the second half with an unqualified `DELETE FROM users` in `beforeEach` — `games.white_id` and `games.black_id` are the only references to `users` without `ON DELETE CASCADE` (thirty-one FKs point at `users`; twenty-nine cascade; the two that do not are both on `games`), so one game left behind by `pg.integration.test.ts` aborted the wipe with SQLSTATE 23503 before any assertion ran, and the same statement destroyed the bot accounts migration 0021 seeds, which nothing restores because `migrate` has already recorded 0021 as applied. `pg/identity-tokens.test.ts` and `tournaments.pg.integration.test.ts` broke the first half, leaving fixed primary keys behind and colliding on `users_pkey` and `tournaments_pkey` (the latter surfacing through the repository's compare-and-set as `VersionConflictError`). **Resolved in Increment 46:** suites that legitimately share the database delete exactly their own rows through `withSharedDatabase` (`packages/persistence/src/test-support/fixtures.ts`), the sibling of Increment 45's `withTestDatabase` and heir to its precedence rule — a cleanup failure never replaces the assertion that actually failed, and never disappears either. `pg.integration.test.ts` moved to disposable databases instead, because it cannot meet the cleanup half at all: it appends to `game_events`, which is append-only by production trigger, so cleaning up after itself would have meant weakening a production safety rule to suit a test. `users-batch`, `anti-cheat`, `bot-reports` and `analysis-cache` were corrected for the same contract though none of them ever failed — fresh `uuidv7()` ids meant their leaked rows could not collide, so the tables merely grew on every run. Serialization was not the fix and was not introduced: `--test-concurrency=1` is a pre-existing documented invariant and the second run failed identically under it. Acceptance is three consecutive runs against one database with no reset between them (186/186/186), after which only the three migration-seeded bot accounts remain, with no leaked disposable databases and no lingering backends; falsification killed 17 of 20 mutations. No production code, migration, checksum, constraint or repository conflict semantic changed, and no migration was added. - **`ApiServer.listen` registered no `'error'` handler, so a failed bind hung and raised an uncaught event (RESOLVED in ADR-0140 §5).** `packages/api/src/server.ts` resolved its promise from the `listening` callback only and built it with no reject path. A bind failing asynchronously (`EADDRINUSE`, `EMFILE`) left the promise pending forever and, with no `'error'` listener on the `http.Server`, was re-raised as an uncaught exception. Found while investigating ADR-0140 and independently raised by the Qodo review of PR #21. **Resolved in the same increment** rather than deferred, because ADR-0140 §2's bounded, diagnosable acquisition is not true without it — the retry can only report a bind error if the listener it is handed rejects. A one-shot `'error'` listener now rejects and is removed once listening, so later server errors keep their previous semantics rather than being swallowed by a `reject` on a settled promise. The regression test fails against the exact pre-fix code, through an uncaught `ERR_UNHANDLED_ERROR`. diff --git a/packages/api/test/diagnostics/run-signature-b-pass.mjs b/packages/api/test/diagnostics/run-signature-b-pass.mjs index bd4b5fae..013d1fca 100644 --- a/packages/api/test/diagnostics/run-signature-b-pass.mjs +++ b/packages/api/test/diagnostics/run-signature-b-pass.mjs @@ -312,7 +312,12 @@ for (let run = 1; run <= maxRuns; run++) { if (collectionError === null && result.timedOut) collectionError = 'run exceeded the wall-clock ceiling'; if (collectionError === null) { try { - records = correlate(parseTapFailures(fs.readFileSync(tapPath, 'utf8')), readChildLogs(logDir)); + const isWin = process.platform === 'win32'; + records = correlate( + parseTapFailures(fs.readFileSync(tapPath, 'utf8'), { isWindows: isWin }), + readChildLogs(logDir), + { isWindows: isWin }, + ); } catch (error) { collectionError = `${error.name}: ${error.message}`; } diff --git a/packages/api/test/diagnostics/signature-b-correlate.cjs b/packages/api/test/diagnostics/signature-b-correlate.cjs index bb33780e..aba059c5 100644 --- a/packages/api/test/diagnostics/signature-b-correlate.cjs +++ b/packages/api/test/diagnostics/signature-b-correlate.cjs @@ -474,7 +474,7 @@ module.exports = { if (require.main === module) { const args = process.argv.slice(2); - const known = new Set(['--tap', '--child-logs']); + const known = new Set(['--tap', '--child-logs', '--windows', '--posix']); let usage = null; // `--tap --child-logs dir` must be a usage error, not a request to read a file called // "--child-logs". An option that swallows the next option produces a confident wrong answer. @@ -491,13 +491,19 @@ if (require.main === module) { const tapPath = valueOf('--tap'); const logDir = valueOf('--child-logs'); if (usage !== null || tapPath === null || logDir === null) { - process.stderr.write(`${usage ?? 'usage'}: signature-b-correlate.cjs --tap --child-logs \n`); + process.stderr.write(`${usage ?? 'usage'}: signature-b-correlate.cjs --tap --child-logs [--windows|--posix]\n`); process.exitCode = 2; } else { - const isWin = process.platform === 'win32'; + const isWindowsExplicit = args.includes('--windows') ? true : (args.includes('--posix') ? false : undefined); + const childLogs = readChildLogs(logDir); + // Derive capture platform from explicit flag or child log metadata, never the analyzer's host platform + const hasWindowsChild = [...childLogs.values()].some((child) => child?.isWindows); + const isWin = typeof isWindowsExplicit === 'boolean' + ? isWindowsExplicit + : (hasWindowsChild ? true : undefined); const records = correlate( parseTapFailures(fs.readFileSync(tapPath, 'utf8'), { isWindows: isWin }), - readChildLogs(logDir), + childLogs, { isWindows: isWin }, ); for (const record of records) process.stdout.write(`${JSON.stringify(record)}\n`); diff --git a/packages/api/test/diagnostics/signature-b-correlate.test.ts b/packages/api/test/diagnostics/signature-b-correlate.test.ts index 911b2da4..e93f183f 100644 --- a/packages/api/test/diagnostics/signature-b-correlate.test.ts +++ b/packages/api/test/diagnostics/signature-b-correlate.test.ts @@ -1083,3 +1083,47 @@ test('correlator: correlates bare string failures with normalized null parent fi fs.rmSync(logDir, { recursive: true, force: true }); } }); + +test('correlator: CLI derives capture platform from child logs without relying on analyzer host platform', () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'sigb-cli-platform-')); + try { + const logDir = path.join(tmpDir, 'logs'); + fs.mkdirSync(logDir); + const tapPath = path.join(tmpDir, 'test.tap'); + + // Windows capture: child log contains Windows metadata + const winChild = [ + { kind: 'start', pid: 4321, testFile: 'C:\\repo\\dist-test\\test\\windows-target.test.js', isWindows: true }, + { kind: 'preload-installed', pid: 4321, testFile: 'C:\\repo\\dist-test\\test\\windows-target.test.js', isWindows: true }, + ]; + fs.writeFileSync(path.join(logDir, 'run-win.jsonl'), `${winChild.map((l) => JSON.stringify(l)).join('\n')}\n`); + + // TAP failure has backslash-delimited relative path + fs.writeFileSync(tapPath, tapFailure('dist-test\\test\\windows-target.test.js', '1')); + + const run = (extraArgs: string[] = []): ReturnType => + spawnSync(process.execPath, [CORRELATE_PATH, '--tap', tapPath, '--child-logs', logDir, ...extraArgs], { + encoding: 'utf8', + env: { ...process.env, NODE_TEST_CONTEXT: undefined }, + }); + + // Run CLI without explicit platform flag: must derive Windows from child logs + const resultAuto = run(); + assert.equal(resultAuto.status, 0); + const linesAuto = String(resultAuto.stdout).trim().split('\n').filter(Boolean); + assert.equal(linesAuto.length, 1); + const recordAuto = JSON.parse(linesAuto[0]); + assert.equal(recordAuto.child.pid, 4321, 'CLI must derive Windows capture from child log metadata'); + assert.equal(recordAuto.child.logFound, true); + + // Overriding with --posix forces POSIX semantics (where backslash in relative path does not match) + const resultPosix = run(['--posix']); + assert.equal(resultPosix.status, 0); + const linesPosix = String(resultPosix.stdout).trim().split('\n').filter(Boolean); + assert.equal(linesPosix.length, 1); + const recordPosix = JSON.parse(linesPosix[0]); + assert.equal(recordPosix.child.logFound, false, '--posix flag must override and preserve POSIX backslash semantics'); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } +}); From 02bc1b45290d8201ecb859d8abc5d1e19c49237e Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Sat, 5 Sep 2026 00:15:23 +0300 Subject: [PATCH 13/13] fix(api-diagnostics): preserve POSIX semantics in CLI when child logs are mixed or stale --- docs/PROJECT_STATE.md | 4 +- docs/ROADMAP.md | 2 +- .../diagnostics/signature-b-correlate.cjs | 8 +-- .../diagnostics/signature-b-correlate.test.ts | 52 +++++++++++++++++++ 4 files changed, 60 insertions(+), 6 deletions(-) diff --git a/docs/PROJECT_STATE.md b/docs/PROJECT_STATE.md index 8d78d00b..107d0515 100644 --- a/docs/PROJECT_STATE.md +++ b/docs/PROJECT_STATE.md @@ -59,8 +59,8 @@ parent TAP diagnostic logs and child JSONL preloads were matched and parsed: values to `null`. **Verification and mutation falsification.** -- Targeted correlator suite (`signature-b-correlate.test.ts`): expanded from 23 to 40 tests - (38 pass, 2 skip for deliberate platform-gated checks, 0 fail). +- Targeted correlator suite (`signature-b-correlate.test.ts`): expanded from 23 to 41 tests + (39 pass, 2 skip for deliberate platform-gated checks, 0 fail). - Mutation falsification: four targeted mutations reversing each of the four hardened behaviors were verified to be killed by the expanded test suite. - Monorepo validation: full build, lint, counts, and guard scripts pass cleanly. diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index a0f2b951..4507555d 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -1349,7 +1349,7 @@ Debt observed during M14. Each states what is known, not what is planned; items - **The published capability document could omit a composed feature, and the client offered controls that could not work (RESOLVED in M15 Increment 24 / ADR-0132).** Left open by Increment 23 above. `capabilitiesView` (`packages/api/src/presenters.ts`) took a hand-written `Pick`, so a feature never added to it was invisible to `GET /v1/capabilities` and nothing complained. ADR-0131 judged that "a narrower failure than a 503 — the feature works for anyone who calls the route directly", and deferred it on the grounds that closing it meant first deciding which optional dependencies are user-facing capabilities. **Both halves of that were wrong.** The failure is narrower only when the client can still reach the feature, and a live instance already existed where it could not: `GET /v1/search` served three modes from two independently-gated dependency sets behind one published `search` flag, so a deployment running the Helm chart’s `search.semanticEnabled: false` advertised search while the client offered two mode buttons whose every request answered 503. And the judgement, while genuinely underivable, can be made **unskippable**, which was the property wanted. `semanticSearch` is now published from both of its dependencies, `Exclude[0] | NotAPublishedCapability>` must be `never` so a new optional dependency cannot compile until someone gives it a flag or records why it has none, and a behavioural guard requires every capability source to change what the document publishes — because a key can sit in that parameter unread, which the compile-time half cannot see. The mutation ledger lives in ADR-0132 and is not restated here. - **The search surface was not gated on `capabilities.search`, so an absolute kill switch still showed a search box (RESOLVED in M15 Increment 24 / ADR-0132 §5).** `SEARCH_ENABLED=0` — the chart's `search.enabled: false`, an absolute kill switch per ADR-0055 — leaves `searchRepository` unconstructed and `GET /v1/search` answering 503 on every mode, keyword included. The entry point was the persistent header form in `packages/web/index.html`, present on every page; being a `` rather than an `a[data-route]`, `NAV_CAPABILITY_MAP` could not reach it, which is why the first pass at Increment 24 gated the semantic and hybrid *modes* and left keyword ungated — the same defect class one mode over. Raised by the Qodo review of PR #155. **Fixed in the same increment rather than deferred:** the form now ships `hidden` and is revealed by `applySearchCapability` only on an explicit `search: true`; the route renders an honest unavailable notice and issues no request; and keyword search waits for the capability answer, reversing this increment's own earlier latency decision, because knowing whether a request is pointless requires having asked. A markup-contract test pins the `hidden` attribute, since the gate depends on it and every other test passes without it. - **Clicking a search mode discarded text typed since the page loaded (RESOLVED in a follow-up to M15 Increment 24 / ADR-0132).** `createModeInput` in `packages/web/src/app/search-mount.ts` closed over the query captured when the route mounted, so `navigateToSearchMode` navigated with the old term and the remount reset the input to match it. Type a new term into the header field, click **Semantic** without pressing enter, and the typed text was gone with no indication it had been discarded. Pre-existing — the closure predated Increment 24 and was untouched by it — and found by the adversarial review of PR #155 while reviewing the capability gate wrapped around the same control. **Resolved:** the query is now a `() => string` read when a mode is chosen rather than a string captured when the selector renders, matching what `main.ts`'s submit handler already does, and falling back to the mounted query only where the document has no header input. Two regression tests, one of which fails against the exact pre-fix closure. -- **`startHarness` drew ports `fetch` refuses (RESOLVED in ADR-0140); the unexplained whole-file failure is still open.** Two signatures were filed here, deliberately not as one cause, and that judgement held. **Signature A is resolved.** WHATWG Fetch blocks eighty-two ports and undici enforces the list on the port number alone, before opening a socket — so `server.listen(0)` could bind, listen and answer raw TCP while `fetch` still refused, surfacing as `TypeError: fetch failed` / `Error: bad port` at the harness’s first request rather than at the listen that caused it. Whether it can happen at all is a property of the host’s dynamic port range: a typical Linux CI range (32768–60999) contains no blocked port, while the Windows range in use here (1024–15000) contains nineteen, which is the whole of "green on CI, flaky locally". The guard that already existed was incomplete in a way that still failed — its hand-observed set of eighteen ports was the spec list intersected with one machine’s range **minus `6679`** — and it retried unboundedly, had no behaviour on exhaustion, and had been copy-pasted into `auth-signin-schema.integration.test.ts`, so the missing port had to be found twice. `packages/api/test/listen.ts` now owns port acquisition for both sites: the spec-complete eighty-two ports (verified by sweeping all 65535 through the real `fetch` on Node v24.15.0), a bounded twenty attempts, each rejected listener closed before the next is asked for, a guarded `address()` read in place of the `as AddressInfo` cast, and an exhaustion error naming the attempts and rejected ports and nothing else. **Signature B is not resolved and was not folded in.** A file still fails with `'test failed'`, no assertion, no stack and none of its own tests reported. Twenty consecutive full runs gave five failures: on pre-fix code one signature A (`auth.test.js`) and three signature B; on post-fix code one signature B and no signature A. The port fix removes A and leaves B exactly where it was, which is the evidence that they are two defects. Four different files were hit (`move-explanation-route`, `tournament-commentary-route`, `bot-detection-analyze`, `anti-cheat-analysis`), sharing no import beyond `./helpers`; each died in 589–703 ms with no test of its own reporting and no stderr. Refuted with evidence: ephemeral-port exhaustion (113 sockets in TIME_WAIT against a 13977-port range), a `Promise.race` loser becoming an unhandled rejection (`race` subscribes to every promise, confirmed on v24.15.0), a throwing `after`/`afterEach` hook (the affected files use none), and a double `close()` rejecting (awaited in a `finally`, it would be attributed to that test with a stack). A second bounded pass — twelve more full runs under the TAP reporter with a preload recording `uncaughtException`, `unhandledRejection` and any non-zero exit — produced twelve clean runs and captured nothing at the time. **A follow-up increment (`claude/node-test-signature-b`) then captured the defect directly, three more times, on three files never previously implicated** (`rate-limit-atomicity`, `dependency-parity`, `studies-api`) — seven distinct files observed with this symptom to date. Occurrences across seven distinct files make a shared or cross-cutting path more plausible and make a defect confined to one test file less likely, but do not exclude file-specific inputs or lifecycle interactions. An instrumented preload (`packages/api/test/diagnostics/signature-b-preload.cjs`) hooking process-level events — `process.exit`, `process.abort`, `process.kill`, `uncaughtExceptionMonitor` (passively observing uncaught exceptions and fatal unhandled rejections), `warning`, `beforeExit`, and Node’s own unconditional `exit` — showed **none of the hooks active at the time fired** on any of the three historical captures (though `process.abort()` was not wrapped in those initial runs and is now covered for future occurrences). A synthetic `process.exit(1)`-before-registration fixture reproduces the identical silent shape; every other synthetic mechanism tried (a post-test async throw, an emitter `'error'` with its listener removed, a synchronous module-load throw, a delayed `SIGKILL`) prints a visibly different diagnostic line, stack, or partial test output that the real defect never shows. This narrows the investigated possibilities while leaving the root cause unresolved: the per-file child process (`node --test` spawns one per file, confirmed by distinct PIDs) was not terminated by `process.exit`, uncaught exceptions, or fatal unhandled rejections, and future runs with `process.abort` instrumentation will record whether abort was called through JS; an absent record narrows in-runtime JS termination but cannot alone prove external termination without corroborating child exit status/signal data or OS-level crash evidence (e.g. distinguishing an external kill or uncatchable signal from a native C++/V8 crash). The machine had roughly 2.5 GB of 15.7 GB RAM free at capture time with several other agents’ processes concurrently running, which is circumstantially consistent with resource contention, but no crash was recorded in the Windows Application or System event logs in that window, so the exact external trigger is still not established. No fix was invented — the forbidden responses (sleeps, whole-file retries, lowering concurrency) would only hide the unresolved root cause, whose origin is not yet established. **A further increment then crossed the parent/child boundary the earlier work stopped at, and found the evidence had been there all along:** Node's runner attaches the child's `exitCode` and `signal` to the `ERR_TEST_FAILURE` it throws, and the `spec` reporter discards them — `formatError` replaces the error with `error.cause`, the bare string `'test failed'` — while the built-in `tap` reporter serializes them, so running `spec` to stdout and `tap` to a file recovers the exit status with no custom reporter and no patched internals. Exit codes were measured on this platform rather than assumed: `process.abort()` gives `134`, `Stop-Process -Force` gives `4294967295`, NTSTATUS faults surface as raw unsigned values such as `3221225477` (`0xC0000005`) — and `1` is produced alike by an uncaught exception, `process.exit(1)`, `taskkill /F` and `process.kill`, so it identifies nothing on its own and is classified `inconclusive`. `signature-b-correlate.cjs` joins the parent's TAP record to the child's JSONL log on the test file path (which also yields the child PID) and states what the pair does and does not establish; where the exit code is ambiguous, a child that reached `preload-installed` and then logged nothing still excludes `process.exit` and an uncaught exception, because both would have left a record and fired Node's `exit` event. A bounded pass of 20 runs under this instrumentation produced 0 captures — which bounds the rate and proves nothing: treating the historical ~1-in-5 as an independent per-run rate, zero captures in 20 runs has probability `(4/5)^20 ≈ 1.2%`, and independence is an assumption rather than an established fact; it ran at 3084–3834 MB free against roughly 2.5 GB at the historical captures, consistent with the resource-contention hypothesis but not evidence for it. **Signature B stays UNRESOLVED**; what changed is that the next occurrence is readable rather than silent. **During M15 Increment 46 validation, Signature B was directly observed three more times, broadening the observed scope to `packages/persistence`:** `search-backfill.integration.test.ts`, `learning.integration.test.ts`, and `test-database.integration.test.ts` each died with the documented bare whole-file `'test failed'` with zero tests reporting and no assertion or stack, and each passed standalone against the exact database state it died on. This confirms the defect is not specific to `packages/api` or its HTTP server test harness. **M15 Increment 47 hardened the diagnostic correlator across 26 bounded runs:** 1 baseline full-suite run, 20 sequential runs under `run-signature-b-pass.mjs`, and 5 concurrent repository-native load runs produced 0 captures (which bounds the rate under those conditions and does not resolve the defect). Code inspection and falsification identified and fixed four correlator blind spots in `packages/api/test/diagnostics/signature-b-correlate.cjs`: (1) cross-directory basename fallback could falsely match a child log from another directory when the target had slashes; (2) Windows path case folding was host-dependent (`process.platform === 'win32'`), breaking cross-platform correlation when logs were analyzed on a different OS; (3) signed 32-bit Windows NTSTATUS exit codes (e.g. `0xC0000005` surfacing as `-1073741819`) were not normalized to unsigned values; and (4) quoted TAP YAML scalar tokens lost exit code or duration numbers. The targeted test suite was expanded from 23 to 40 tests (38 pass, 2 skip, 0 fail), killing 4 falsification mutations. **Signature B remains UNRESOLVED;** no production fix was invented, and the next occurrence will be correlated without cross-platform or exit-code blind spots. See ADR-0140 §4. +- **`startHarness` drew ports `fetch` refuses (RESOLVED in ADR-0140); the unexplained whole-file failure is still open.** Two signatures were filed here, deliberately not as one cause, and that judgement held. **Signature A is resolved.** WHATWG Fetch blocks eighty-two ports and undici enforces the list on the port number alone, before opening a socket — so `server.listen(0)` could bind, listen and answer raw TCP while `fetch` still refused, surfacing as `TypeError: fetch failed` / `Error: bad port` at the harness’s first request rather than at the listen that caused it. Whether it can happen at all is a property of the host’s dynamic port range: a typical Linux CI range (32768–60999) contains no blocked port, while the Windows range in use here (1024–15000) contains nineteen, which is the whole of "green on CI, flaky locally". The guard that already existed was incomplete in a way that still failed — its hand-observed set of eighteen ports was the spec list intersected with one machine’s range **minus `6679`** — and it retried unboundedly, had no behaviour on exhaustion, and had been copy-pasted into `auth-signin-schema.integration.test.ts`, so the missing port had to be found twice. `packages/api/test/listen.ts` now owns port acquisition for both sites: the spec-complete eighty-two ports (verified by sweeping all 65535 through the real `fetch` on Node v24.15.0), a bounded twenty attempts, each rejected listener closed before the next is asked for, a guarded `address()` read in place of the `as AddressInfo` cast, and an exhaustion error naming the attempts and rejected ports and nothing else. **Signature B is not resolved and was not folded in.** A file still fails with `'test failed'`, no assertion, no stack and none of its own tests reported. Twenty consecutive full runs gave five failures: on pre-fix code one signature A (`auth.test.js`) and three signature B; on post-fix code one signature B and no signature A. The port fix removes A and leaves B exactly where it was, which is the evidence that they are two defects. Four different files were hit (`move-explanation-route`, `tournament-commentary-route`, `bot-detection-analyze`, `anti-cheat-analysis`), sharing no import beyond `./helpers`; each died in 589–703 ms with no test of its own reporting and no stderr. Refuted with evidence: ephemeral-port exhaustion (113 sockets in TIME_WAIT against a 13977-port range), a `Promise.race` loser becoming an unhandled rejection (`race` subscribes to every promise, confirmed on v24.15.0), a throwing `after`/`afterEach` hook (the affected files use none), and a double `close()` rejecting (awaited in a `finally`, it would be attributed to that test with a stack). A second bounded pass — twelve more full runs under the TAP reporter with a preload recording `uncaughtException`, `unhandledRejection` and any non-zero exit — produced twelve clean runs and captured nothing at the time. **A follow-up increment (`claude/node-test-signature-b`) then captured the defect directly, three more times, on three files never previously implicated** (`rate-limit-atomicity`, `dependency-parity`, `studies-api`) — seven distinct files observed with this symptom to date. Occurrences across seven distinct files make a shared or cross-cutting path more plausible and make a defect confined to one test file less likely, but do not exclude file-specific inputs or lifecycle interactions. An instrumented preload (`packages/api/test/diagnostics/signature-b-preload.cjs`) hooking process-level events — `process.exit`, `process.abort`, `process.kill`, `uncaughtExceptionMonitor` (passively observing uncaught exceptions and fatal unhandled rejections), `warning`, `beforeExit`, and Node’s own unconditional `exit` — showed **none of the hooks active at the time fired** on any of the three historical captures (though `process.abort()` was not wrapped in those initial runs and is now covered for future occurrences). A synthetic `process.exit(1)`-before-registration fixture reproduces the identical silent shape; every other synthetic mechanism tried (a post-test async throw, an emitter `'error'` with its listener removed, a synchronous module-load throw, a delayed `SIGKILL`) prints a visibly different diagnostic line, stack, or partial test output that the real defect never shows. This narrows the investigated possibilities while leaving the root cause unresolved: the per-file child process (`node --test` spawns one per file, confirmed by distinct PIDs) was not terminated by `process.exit`, uncaught exceptions, or fatal unhandled rejections, and future runs with `process.abort` instrumentation will record whether abort was called through JS; an absent record narrows in-runtime JS termination but cannot alone prove external termination without corroborating child exit status/signal data or OS-level crash evidence (e.g. distinguishing an external kill or uncatchable signal from a native C++/V8 crash). The machine had roughly 2.5 GB of 15.7 GB RAM free at capture time with several other agents’ processes concurrently running, which is circumstantially consistent with resource contention, but no crash was recorded in the Windows Application or System event logs in that window, so the exact external trigger is still not established. No fix was invented — the forbidden responses (sleeps, whole-file retries, lowering concurrency) would only hide the unresolved root cause, whose origin is not yet established. **A further increment then crossed the parent/child boundary the earlier work stopped at, and found the evidence had been there all along:** Node's runner attaches the child's `exitCode` and `signal` to the `ERR_TEST_FAILURE` it throws, and the `spec` reporter discards them — `formatError` replaces the error with `error.cause`, the bare string `'test failed'` — while the built-in `tap` reporter serializes them, so running `spec` to stdout and `tap` to a file recovers the exit status with no custom reporter and no patched internals. Exit codes were measured on this platform rather than assumed: `process.abort()` gives `134`, `Stop-Process -Force` gives `4294967295`, NTSTATUS faults surface as raw unsigned values such as `3221225477` (`0xC0000005`) — and `1` is produced alike by an uncaught exception, `process.exit(1)`, `taskkill /F` and `process.kill`, so it identifies nothing on its own and is classified `inconclusive`. `signature-b-correlate.cjs` joins the parent's TAP record to the child's JSONL log on the test file path (which also yields the child PID) and states what the pair does and does not establish; where the exit code is ambiguous, a child that reached `preload-installed` and then logged nothing still excludes `process.exit` and an uncaught exception, because both would have left a record and fired Node's `exit` event. A bounded pass of 20 runs under this instrumentation produced 0 captures — which bounds the rate and proves nothing: treating the historical ~1-in-5 as an independent per-run rate, zero captures in 20 runs has probability `(4/5)^20 ≈ 1.2%`, and independence is an assumption rather than an established fact; it ran at 3084–3834 MB free against roughly 2.5 GB at the historical captures, consistent with the resource-contention hypothesis but not evidence for it. **Signature B stays UNRESOLVED**; what changed is that the next occurrence is readable rather than silent. **During M15 Increment 46 validation, Signature B was directly observed three more times, broadening the observed scope to `packages/persistence`:** `search-backfill.integration.test.ts`, `learning.integration.test.ts`, and `test-database.integration.test.ts` each died with the documented bare whole-file `'test failed'` with zero tests reporting and no assertion or stack, and each passed standalone against the exact database state it died on. This confirms the defect is not specific to `packages/api` or its HTTP server test harness. **M15 Increment 47 hardened the diagnostic correlator across 26 bounded runs:** 1 baseline full-suite run, 20 sequential runs under `run-signature-b-pass.mjs`, and 5 concurrent repository-native load runs produced 0 captures (which bounds the rate under those conditions and does not resolve the defect). Code inspection and falsification identified and fixed four correlator blind spots in `packages/api/test/diagnostics/signature-b-correlate.cjs`: (1) cross-directory basename fallback could falsely match a child log from another directory when the target had slashes; (2) Windows path case folding was host-dependent (`process.platform === 'win32'`), breaking cross-platform correlation when logs were analyzed on a different OS; (3) signed 32-bit Windows NTSTATUS exit codes (e.g. `0xC0000005` surfacing as `-1073741819`) were not normalized to unsigned values; and (4) quoted TAP YAML scalar tokens lost exit code or duration numbers. The targeted test suite was expanded from 23 to 41 tests (39 pass, 2 skip, 0 fail), killing 4 falsification mutations. **Signature B remains UNRESOLVED;** no production fix was invented, and the next occurrence will be correlated without cross-platform or exit-code blind spots. See ADR-0140 §4. - **Isolated-database test teardown dropped databases out from under connections that had not finished closing (RESOLVED in M15 Increment 45).** `withDatabase` in `packages/persistence/test/variant-migrations.integration.test.ts` ended its pool and then immediately ran `DROP DATABASE ... WITH (FORCE)`. `pool.end()` does not wait for its clients to close: in pg 8.22.0 `_pulseQueue` reaches the end callback in the same synchronous turn in which `_remove` filters the last client out of `_clients`, while `client.end()` has only queued the Terminate byte — instrumentation recorded **zero of four `remove` events fired at the moment `end()` resolved**. The drop could therefore still find a backend attached; `FORCE` terminated it, and the resulting `FATAL` arrived on a socket whose pool still had `idleListener` attached, which `pg` re-emitted as `pool.emit('error')` — an unhandled EventEmitter error that `node:test` attributed to whichever test was running rather than to the teardown that caused it. It surfaced as intermittent `terminating connection due to administrator command` failures in `postgres integration (persistence)` during M15 Increment 44, on a different test each run, which is the signature of a race rather than a broken assertion. The same shape existed in `packages/api/test/auth-signin-schema.integration.test.ts`, which had absorbed SQLSTATE 57P01 with a `pool.on('error', ...)` listener — a symptom fix for the same cause. **Resolved in Increment 45:** a shared `withTestDatabase` helper (`@chess-platform/persistence/test-support`) ends the pool under a bound, waits for `pg_stat_activity` to report the database unused, and drops it *without* `FORCE`. Measured on PostgreSQL 16.14, a plain drop against a still-attached backend fails with SQLSTATE 55006 and leaves that connection untouched, where `FORCE` succeeds by killing it — so the change trades a quiet, harmful success for a loud, harmless failure. FORCE remains only on the emergency path that guarantees the disposable database is still dropped once teardown has already failed — best effort, since that last drop runs inside a `catch` so it cannot bury the error being reported. The 57P01 absorber is deleted, because the corrected lifecycle never causes one. `createPool` and `migrate` are unchanged, and no migration was added. - **The persistence integration suite was not idempotent against a reused database (RESOLVED in M15 Increment 46).** Recorded as a known defect by Increment 45 and left open there. Against a fresh PostgreSQL 16 database the suite passed; a second run against the *same* database failed nine tests, deterministically — measured on 16.14 before any edit as **173 pass / 0 fail** then **164 pass / 9 fail**. CI provisions a fresh server per run, so it never surfaced there. One contract was being broken in two directions: a suite sharing `chess_test` must remove every row it created and remove nothing else. `achievements.integration.test.ts` broke the second half with an unqualified `DELETE FROM users` in `beforeEach` — `games.white_id` and `games.black_id` are the only references to `users` without `ON DELETE CASCADE` (thirty-one FKs point at `users`; twenty-nine cascade; the two that do not are both on `games`), so one game left behind by `pg.integration.test.ts` aborted the wipe with SQLSTATE 23503 before any assertion ran, and the same statement destroyed the bot accounts migration 0021 seeds, which nothing restores because `migrate` has already recorded 0021 as applied. `pg/identity-tokens.test.ts` and `tournaments.pg.integration.test.ts` broke the first half, leaving fixed primary keys behind and colliding on `users_pkey` and `tournaments_pkey` (the latter surfacing through the repository's compare-and-set as `VersionConflictError`). **Resolved in Increment 46:** suites that legitimately share the database delete exactly their own rows through `withSharedDatabase` (`packages/persistence/src/test-support/fixtures.ts`), the sibling of Increment 45's `withTestDatabase` and heir to its precedence rule — a cleanup failure never replaces the assertion that actually failed, and never disappears either. `pg.integration.test.ts` moved to disposable databases instead, because it cannot meet the cleanup half at all: it appends to `game_events`, which is append-only by production trigger, so cleaning up after itself would have meant weakening a production safety rule to suit a test. `users-batch`, `anti-cheat`, `bot-reports` and `analysis-cache` were corrected for the same contract though none of them ever failed — fresh `uuidv7()` ids meant their leaked rows could not collide, so the tables merely grew on every run. Serialization was not the fix and was not introduced: `--test-concurrency=1` is a pre-existing documented invariant and the second run failed identically under it. Acceptance is three consecutive runs against one database with no reset between them (186/186/186), after which only the three migration-seeded bot accounts remain, with no leaked disposable databases and no lingering backends; falsification killed 17 of 20 mutations. No production code, migration, checksum, constraint or repository conflict semantic changed, and no migration was added. - **`ApiServer.listen` registered no `'error'` handler, so a failed bind hung and raised an uncaught event (RESOLVED in ADR-0140 §5).** `packages/api/src/server.ts` resolved its promise from the `listening` callback only and built it with no reject path. A bind failing asynchronously (`EADDRINUSE`, `EMFILE`) left the promise pending forever and, with no `'error'` listener on the `http.Server`, was re-raised as an uncaught exception. Found while investigating ADR-0140 and independently raised by the Qodo review of PR #21. **Resolved in the same increment** rather than deferred, because ADR-0140 §2's bounded, diagnosable acquisition is not true without it — the retry can only report a bind error if the listener it is handed rejects. A one-shot `'error'` listener now rejects and is removed once listening, so later server errors keep their previous semantics rather than being swallowed by a `reject` on a settled promise. The regression test fails against the exact pre-fix code, through an uncaught `ERR_UNHANDLED_ERROR`. diff --git a/packages/api/test/diagnostics/signature-b-correlate.cjs b/packages/api/test/diagnostics/signature-b-correlate.cjs index aba059c5..7183662d 100644 --- a/packages/api/test/diagnostics/signature-b-correlate.cjs +++ b/packages/api/test/diagnostics/signature-b-correlate.cjs @@ -496,11 +496,13 @@ if (require.main === module) { } else { const isWindowsExplicit = args.includes('--windows') ? true : (args.includes('--posix') ? false : undefined); const childLogs = readChildLogs(logDir); - // Derive capture platform from explicit flag or child log metadata, never the analyzer's host platform - const hasWindowsChild = [...childLogs.values()].some((child) => child?.isWindows); + // Derive capture platform only when child logs are uniformly Windows or POSIX; never let mixed or stale logs force Windows semantics + const children = [...childLogs.values()]; + const allWindows = children.length > 0 && children.every((child) => child?.isWindows); + const allPosix = children.length > 0 && children.every((child) => !child?.isWindows); const isWin = typeof isWindowsExplicit === 'boolean' ? isWindowsExplicit - : (hasWindowsChild ? true : undefined); + : (allWindows ? true : (allPosix ? false : undefined)); const records = correlate( parseTapFailures(fs.readFileSync(tapPath, 'utf8'), { isWindows: isWin }), childLogs, diff --git a/packages/api/test/diagnostics/signature-b-correlate.test.ts b/packages/api/test/diagnostics/signature-b-correlate.test.ts index e93f183f..ac31b24e 100644 --- a/packages/api/test/diagnostics/signature-b-correlate.test.ts +++ b/packages/api/test/diagnostics/signature-b-correlate.test.ts @@ -1127,3 +1127,55 @@ test('correlator: CLI derives capture platform from child logs without relying o fs.rmSync(tmpDir, { recursive: true, force: true }); } }); + +test('correlator: CLI preserves POSIX semantics when log directory contains mixed or stale Windows logs', () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'sigb-cli-mixed-')); + try { + const logDir = path.join(tmpDir, 'logs'); + fs.mkdirSync(logDir); + const tapPathCasing = path.join(tmpDir, 'casing.tap'); + const tapPathExact = path.join(tmpDir, 'exact.tap'); + + // Mixed logs: a stale Windows child log and a POSIX child log + const staleWinChild = [ + { kind: 'start', pid: 1111, testFile: 'C:\\stale\\dist-test\\test\\stale.test.js', isWindows: true }, + { kind: 'preload-installed', pid: 1111, testFile: 'C:\\stale\\dist-test\\test\\stale.test.js', isWindows: true }, + ]; + const posixChild = [ + { kind: 'start', pid: 2222, testFile: '/repo/dist-test/test/posix-target.test.js', isWindows: false }, + { kind: 'preload-installed', pid: 2222, testFile: '/repo/dist-test/test/posix-target.test.js', isWindows: false }, + ]; + fs.writeFileSync(path.join(logDir, 'run-stale-win.jsonl'), `${staleWinChild.map((l) => JSON.stringify(l)).join('\n')}\n`); + fs.writeFileSync(path.join(logDir, 'run-posix.jsonl'), `${posixChild.map((l) => JSON.stringify(l)).join('\n')}\n`); + + // TAP failure 1: uppercase POSIX path (should NOT match lowercase posix-target when case-sensitive) + fs.writeFileSync(tapPathCasing, tapFailure('dist-test/test/POSIX-TARGET.test.js', '1')); + // TAP failure 2: exact lowercase POSIX path + fs.writeFileSync(tapPathExact, tapFailure('dist-test/test/posix-target.test.js', '1')); + + const run = (tap: string, extraArgs: string[] = []): ReturnType => + spawnSync(process.execPath, [CORRELATE_PATH, '--tap', tap, '--child-logs', logDir, ...extraArgs], { + encoding: 'utf8', + env: { ...process.env, NODE_TEST_CONTEXT: undefined }, + }); + + // Without explicit flags, mixed logs must NOT force Windows semantics globally; POSIX case-sensitivity is preserved + const resultCasing = run(tapPathCasing); + assert.equal(resultCasing.status, 0); + const linesCasing = String(resultCasing.stdout).trim().split('\n').filter(Boolean); + assert.equal(linesCasing.length, 1); + const recordCasing = JSON.parse(linesCasing[0]); + assert.equal(recordCasing.child.logFound, false, 'mixed logs must not force Windows case folding onto POSIX paths'); + + // Exact casing matches the POSIX child correctly + const resultExact = run(tapPathExact); + assert.equal(resultExact.status, 0); + const linesExact = String(resultExact.stdout).trim().split('\n').filter(Boolean); + assert.equal(linesExact.length, 1); + const recordExact = JSON.parse(linesExact[0]); + assert.equal(recordExact.child.logFound, true); + assert.equal(recordExact.child.pid, 2222); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } +});