From aac853d3e8ef7f4668559c744cbbc71dd2bbcf6a Mon Sep 17 00:00:00 2001 From: David Snelling Date: Thu, 3 Sep 2026 10:57:03 -0700 Subject: [PATCH 1/2] fix(release): wall-entry commits under an explicit git identity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit git commit in the cache clone relied on ambient user.name/user.email, which the box has neither globally nor per-repo — every push-side test failed there with "unable to auto-detect email address" while passing on a laptop with a global identity configured. Resolve the identity from the repository the rail is actually running in (process.cwd(), the developer's own checkout release.sh invokes this from) and pass it explicitly via -c user.name/-c user.email on the commit; refuse by name if neither is set. Give the test fixtures a repo-local identity the same way seedRemote already does for the seed clone, so the suite is deterministic on any host. --- scripts/wall-entry.mjs | 37 ++++++++++++++++++++++++++- tests/unit/release/wall-entry.test.ts | 8 ++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/scripts/wall-entry.mjs b/scripts/wall-entry.mjs index d4ec7ba5..5079cf86 100644 --- a/scripts/wall-entry.mjs +++ b/scripts/wall-entry.mjs @@ -337,6 +337,36 @@ function git(args, cwd) { } } +/** + * Resolve the git identity for the wall commit from the repository the rail + * is actually running in — the developer's own checkout (`process.cwd()`; + * `release.sh` invokes this script from the repo root with no `cd`), via + * git's normal config precedence (repo-local, then global, then system). + * Never guessed and never left to git's own "who are you?" prompt: a host + * with no configured identity anywhere (a bare CI box, say) must refuse + * loudly rather than have git manufacture a placeholder identity or hang. + * @returns {{name: string, email: string}} + */ +function resolveWallCommitIdentity() { + const repo = process.cwd() + let name = '' + let email = '' + try { + name = git(['config', 'user.name'], repo) + } catch { + name = '' + } + try { + email = git(['config', 'user.email'], repo) + } catch { + email = '' + } + if (!name || !email) { + fail('no git identity for the wall commit — set user.name/user.email') + } + return { name, email } +} + /** * Ensure a clean, up-to-date local clone of the releases repo at * `cacheDir`, checked out on `main` — cloning fresh if `cacheDir` has no @@ -423,9 +453,14 @@ function publishEntry(entry, product, remote, cacheDir) { return } + const identity = resolveWallCommitIdentity() + try { git(['add', `${product}.json`], cacheDir) - git(['commit', '-m', `chore(wall): ${product} ${entry.version}`], cacheDir) + git( + ['-c', `user.name=${identity.name}`, '-c', `user.email=${identity.email}`, 'commit', '-m', `chore(wall): ${product} ${entry.version}`], + cacheDir, + ) } catch (err) { fail(`cannot commit the wall entry in "${cacheDir}" — ${/** @type {Error} */ (err).message}\n cure: inspect "${cacheDir}" by hand and re-run once its git state is clean`) } diff --git a/tests/unit/release/wall-entry.test.ts b/tests/unit/release/wall-entry.test.ts index 8bf9d357..b29ae326 100644 --- a/tests/unit/release/wall-entry.test.ts +++ b/tests/unit/release/wall-entry.test.ts @@ -104,6 +104,14 @@ let cacheDir: string beforeEach(() => { dir = mkdtempSync(join(tmpdir(), 'wall-entry-test-')) + // wall-entry.mjs is run with this dir as its cwd, standing in for the real + // developer checkout it reads its commit identity from (process.cwd()) — + // give it a repo-local identity the same way seedRemote gives one to the + // seed clone, so the suite is deterministic on a host with no global git + // config (a bare CI box) as much as one with a developer's own. + execFileSync('git', ['init', '-q', dir]) + git(['config', 'user.name', 'Wall Entry Test'], dir) + git(['config', 'user.email', 'wall-entry-test@example.com'], dir) remoteDir = initBareRemote() cacheDir = join(mkdtempSync(join(tmpdir(), 'wall-cache-')), 'soulcraft-releases') }) From a2ea21b330d49933b70cd9fed1c7fb46afb89cb8 Mon Sep 17 00:00:00 2001 From: David Snelling Date: Thu, 3 Sep 2026 10:57:10 -0700 Subject: [PATCH 2/2] fix(shutdown): hold the signal listener until the exit decision is made MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closing the last live instance inside the SIGTERM/SIGINT handler calls close() -> deregisterShutdownHooksIfIdle(), which removes Brainy's own signal listener from process synchronously, before that same handler invocation has reached the point where it decides whether to exit. That opens a window with no registered listener for the signal at all: a second/concurrent delivery of the same signal during that window falls through to Node's default disposition and kills the process outright, after the clean shutdown already finished, so the process reports a signal kill instead of the 0 clause (a) and (b) of shutdown-single-owner.test.ts pin — intermittent under load, which is why it only ever showed up on the box. Add a static flag that stays true for the whole closeOnShutdown() run and makes deregisterShutdownHooksIfIdle() defer rather than remove the listener while that run is still deciding; closeOnShutdown()'s own finally re-runs the deregistration check once it is actually done, so the listener never leaks past its use. --- src/brainy.ts | 134 ++++++++++++++++++++++++++++++++++---------------- 1 file changed, 91 insertions(+), 43 deletions(-) diff --git a/src/brainy.ts b/src/brainy.ts index b8eb7f56..fc08f291 100644 --- a/src/brainy.ts +++ b/src/brainy.ts @@ -544,6 +544,23 @@ export class Brainy implements BrainyInterface { * and narrate forever. Reset by {@link deregisterShutdownHooksIfIdle}. */ private static beforeExitNarrated = false + /** True for the entire duration of ONE `closeOnShutdown()` run (the + * signal-path handler in {@link registerShutdownHooks}) — from before it + * starts closing instances until after it has decided whether to exit. + * THE RACE THIS CLOSES: closing the LAST live instance calls + * `close()` → `deregisterShutdownHooksIfIdle()` synchronously, which + * removes `Brainy.sigtermListener` from `process` — while `closeOnShutdown` + * (that very listener's OWN still-running invocation) hasn't yet reached + * `exitIfSoleShutdownOwner()`'s `process.exit(0)`. In that window Node has + * NO registered SIGTERM listener, so a second/concurrent delivery of the + * same signal (a raced re-send, common on a loaded host) falls through to + * Node's default disposition and kills the process outright — the + * clean-shutdown work already finished, but the process never reports the + * 0 it earned. `deregisterShutdownHooksIfIdle()` checks this flag and + * defers; `closeOnShutdown()`'s `finally` re-runs the deregistration check + * once it is done, so the listener never actually leaks past its use. */ + private static shutdownSignalHandlerActive = false + /** Poll cadence (ms) for the migration LOCK when a provider exposes no * event-driven `whenMigrationComplete()` signal. See {@link awaitMigrationLock}. */ private static readonly MIGRATION_POLL_INTERVAL_MS = 250 @@ -2196,51 +2213,74 @@ export class Brainy implements BrainyInterface { */ const closeOnShutdown = async () => { console.log('Shutdown signal received - flushing pending data...') - // DEFER ONE MACROTASK. A host application registers its own listener on - // the same signal, and Node runs listeners in registration order — ours - // is usually first, because the brain was opened before the host wired - // its shutdown. Yielding once lets every other listener for this signal - // run its synchronous prologue, so a host that calls close() gets to be - // the owner. It is only a courtesy, never the safety: close()'s own - // single-flight gate is what makes a lost race harmless. - await new Promise((resolve) => setImmediate(resolve)) + // HOLD THE LISTENER FOR THE WHOLE RUN. Closing the LAST live instance + // below calls close() → deregisterShutdownHooksIfIdle(), which removes + // Brainy's own SIGTERM/SIGINT listeners from `process` — synchronously, + // before THIS invocation has reached exitIfSoleShutdownOwner()'s + // process.exit(0). Left alone, that opens a window with no registered + // listener for the signal at all, so a second/concurrent delivery of + // the same signal (a raced re-send — not rare on a loaded host) falls + // through to Node's default disposition and kills the process outright + // AFTER the clean-shutdown work already finished, reporting a signal + // kill instead of the 0 the shutdown earned. Setting this flag makes + // deregisterShutdownHooksIfIdle() defer; the `finally` below re-checks + // it once this run is fully done — closeOnShutdown, not a nested + // close(), owns exactly when the listener actually comes off. + Brainy.shutdownSignalHandlerActive = true + try { + // DEFER ONE MACROTASK. A host application registers its own listener + // on the same signal, and Node runs listeners in registration order — + // ours is usually first, because the brain was opened before the + // host wired its shutdown. Yielding once lets every other listener + // for this signal run its synchronous prologue, so a host that calls + // close() gets to be the owner. It is only a courtesy, never the + // safety: close()'s own single-flight gate is what makes a lost race + // harmless. + await new Promise((resolve) => setImmediate(resolve)) - let closedCount = 0 - let deferredCount = 0 - let failedCount = 0 - // Snapshot: close() splices Brainy.instances while we iterate. - for (const instance of [...Brainy.instances]) { - if (!instance.initialized) continue - // SOMEONE ELSE OWNS THIS ONE. Not a flush, not a lock release, not a - // component close — nothing. Touching a brain whose close is running - // is the whole defect this handler was rewritten for. - if (instance.closed || instance._closeInFlight !== null) { - deferredCount++ - continue + let closedCount = 0 + let deferredCount = 0 + let failedCount = 0 + // Snapshot: close() splices Brainy.instances while we iterate. + for (const instance of [...Brainy.instances]) { + if (!instance.initialized) continue + // SOMEONE ELSE OWNS THIS ONE. Not a flush, not a lock release, not a + // component close — nothing. Touching a brain whose close is running + // is the whole defect this handler was rewritten for. + if (instance.closed || instance._closeInFlight !== null) { + deferredCount++ + continue + } + try { + // Law 1: this try/catch is the isolation — the loop continues. + await instance.close() + closedCount++ + } catch (error) { + failedCount++ + console.error('Failed to close one Brainy instance on shutdown:', error) + } } - try { - // Law 1: this try/catch is the isolation — the loop continues. - await instance.close() - closedCount++ - } catch (error) { - failedCount++ - console.error('Failed to close one Brainy instance on shutdown:', error) + if (closedCount > 0) { + console.log(`Flushed successfully (${closedCount} instance${closedCount > 1 ? 's' : ''})`) } - } - if (closedCount > 0) { - console.log(`Flushed successfully (${closedCount} instance${closedCount > 1 ? 's' : ''})`) - } - if (deferredCount > 0) { - console.log( - `${deferredCount} Brainy instance${deferredCount > 1 ? 's are' : ' is'} already ` + - `closing — left to the caller that owns that close.` - ) - } - if (failedCount > 0) { - console.error( - `${failedCount} Brainy instance${failedCount > 1 ? 's' : ''} did not complete shutdown — ` + - `their writer locks were released, but their next open will run crash recovery.` - ) + if (deferredCount > 0) { + console.log( + `${deferredCount} Brainy instance${deferredCount > 1 ? 's are' : ' is'} already ` + + `closing — left to the caller that owns that close.` + ) + } + if (failedCount > 0) { + console.error( + `${failedCount} Brainy instance${failedCount > 1 ? 's' : ''} did not complete shutdown — ` + + `their writer locks were released, but their next open will run crash recovery.` + ) + } + } finally { + // Release the hold and run the deferred check ourselves — the last + // close() above may have found the flag set and skipped its own + // deregistration, so nobody else will do this if we don't. + Brainy.shutdownSignalHandlerActive = false + Brainy.deregisterShutdownHooksIfIdle() } } @@ -2404,9 +2444,17 @@ export class Brainy implements BrainyInterface { * script that closed every brain exits on its own — a library must never * keep its host process alive. Re-initializing later re-registers them * (the `shutdownHooksRegisteredGlobally` flag resets here). + * + * Deferred (not skipped — {@link closeOnShutdown}'s `finally` always + * re-checks) while a signal-path shutdown is actively running: that + * handler's OWN still-in-flight invocation is `Brainy.sigtermListener`, and + * removing it out from under itself — which closing the LAST instance here + * would otherwise do, synchronously, mid-run — would leave `process` with + * no listener for the signal for the remainder of that run. See + * {@link shutdownSignalHandlerActive}'s doc for the exact race this closes. */ private static deregisterShutdownHooksIfIdle(): void { - if (Brainy.instances.length > 0 || !Brainy.shutdownHooksRegisteredGlobally) { + if (Brainy.instances.length > 0 || !Brainy.shutdownHooksRegisteredGlobally || Brainy.shutdownSignalHandlerActive) { return } if (Brainy.sigtermListener) process.off('SIGTERM', Brainy.sigtermListener)