Compare commits

...

2 commits

Author SHA1 Message Date
a2ea21b330 fix(shutdown): hold the signal listener until the exit decision is made
Some checks failed
CI / Node 24 (push) Successful in 12m16s
CI / Node 22 (push) Successful in 12m31s
CI / Bun (latest) (push) Successful in 12m32s
CI / Integration + conformance (Node 22) (push) Failing after 16m4s
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.
2026-09-03 10:57:10 -07:00
aac853d3e8 fix(release): wall-entry commits under an explicit git identity
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.
2026-09-03 10:57:03 -07:00
3 changed files with 135 additions and 44 deletions

View file

@ -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 * Ensure a clean, up-to-date local clone of the releases repo at
* `cacheDir`, checked out on `main` cloning fresh if `cacheDir` has no * `cacheDir`, checked out on `main` cloning fresh if `cacheDir` has no
@ -423,9 +453,14 @@ function publishEntry(entry, product, remote, cacheDir) {
return return
} }
const identity = resolveWallCommitIdentity()
try { try {
git(['add', `${product}.json`], cacheDir) 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) { } 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`) 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`)
} }

View file

@ -544,6 +544,23 @@ export class Brainy<T = any> implements BrainyInterface<T> {
* and narrate forever. Reset by {@link deregisterShutdownHooksIfIdle}. */ * and narrate forever. Reset by {@link deregisterShutdownHooksIfIdle}. */
private static beforeExitNarrated = false 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 /** Poll cadence (ms) for the migration LOCK when a provider exposes no
* event-driven `whenMigrationComplete()` signal. See {@link awaitMigrationLock}. */ * event-driven `whenMigrationComplete()` signal. See {@link awaitMigrationLock}. */
private static readonly MIGRATION_POLL_INTERVAL_MS = 250 private static readonly MIGRATION_POLL_INTERVAL_MS = 250
@ -2196,13 +2213,29 @@ export class Brainy<T = any> implements BrainyInterface<T> {
*/ */
const closeOnShutdown = async () => { const closeOnShutdown = async () => {
console.log('Shutdown signal received - flushing pending data...') console.log('Shutdown signal received - flushing pending data...')
// DEFER ONE MACROTASK. A host application registers its own listener on // HOLD THE LISTENER FOR THE WHOLE RUN. Closing the LAST live instance
// the same signal, and Node runs listeners in registration order — ours // below calls close() → deregisterShutdownHooksIfIdle(), which removes
// is usually first, because the brain was opened before the host wired // Brainy's own SIGTERM/SIGINT listeners from `process` — synchronously,
// its shutdown. Yielding once lets every other listener for this signal // before THIS invocation has reached exitIfSoleShutdownOwner()'s
// run its synchronous prologue, so a host that calls close() gets to be // process.exit(0). Left alone, that opens a window with no registered
// the owner. It is only a courtesy, never the safety: close()'s own // listener for the signal at all, so a second/concurrent delivery of
// single-flight gate is what makes a lost race harmless. // 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<void>((resolve) => setImmediate(resolve)) await new Promise<void>((resolve) => setImmediate(resolve))
let closedCount = 0 let closedCount = 0
@ -2242,6 +2275,13 @@ export class Brainy<T = any> implements BrainyInterface<T> {
`their writer locks were released, but their next open will run crash recovery.` `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<T = any> implements BrainyInterface<T> {
* script that closed every brain exits on its own a library must never * script that closed every brain exits on its own a library must never
* keep its host process alive. Re-initializing later re-registers them * keep its host process alive. Re-initializing later re-registers them
* (the `shutdownHooksRegisteredGlobally` flag resets here). * (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 { private static deregisterShutdownHooksIfIdle(): void {
if (Brainy.instances.length > 0 || !Brainy.shutdownHooksRegisteredGlobally) { if (Brainy.instances.length > 0 || !Brainy.shutdownHooksRegisteredGlobally || Brainy.shutdownSignalHandlerActive) {
return return
} }
if (Brainy.sigtermListener) process.off('SIGTERM', Brainy.sigtermListener) if (Brainy.sigtermListener) process.off('SIGTERM', Brainy.sigtermListener)

View file

@ -104,6 +104,14 @@ let cacheDir: string
beforeEach(() => { beforeEach(() => {
dir = mkdtempSync(join(tmpdir(), 'wall-entry-test-')) 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() remoteDir = initBareRemote()
cacheDir = join(mkdtempSync(join(tmpdir(), 'wall-cache-')), 'soulcraft-releases') cacheDir = join(mkdtempSync(join(tmpdir(), 'wall-cache-')), 'soulcraft-releases')
}) })