Certify a candidate in an isolated validator before it can go live - #2476
Open
dawsontoth wants to merge 29 commits into
Open
Certify a candidate in an isolated validator before it can go live#2476dawsontoth wants to merge 29 commits into
dawsontoth wants to merge 29 commits into
Conversation
…can go live Step 2 of #2315. Step 1 made `deploy_component` build aside and validate before the swap, but `validateComponentLoadsExclusive` gated its whole body on `!isMainThread` and the operations API deploys on main — so an operator deploy was certified by nothing and step 1 reordered a no-op there. **Certification is now a requirement of the mint, not a courtesy of the caller.** `validateCandidate` was an optional callback and only one of `prepareApplication`'s four production call sites supplied it, yet `activateCandidateApplication` writes `.complete`, which recovery treats as proof that a validation happened. So `markCandidateComplete` refuses to write it unless a validator has certified that exact candidate. The record is module-internal: a proof passed as an argument is one an external caller can forge or a future caller can forget. **The validator is an ephemeral worker, deliberately not a `startWorker` one.** That function builds a MessageChannel per connected port, announces the new port to every peer, and registers for monitoring and restart — so a validator would join the ITC mesh, letting a candidate's top-level `server.registerOperation` announce itself and traffic route at a thread about to exit, at O(deploys × workers) channels. What IS shared is the interpreter setup, now factored out as `buildWorkerExecArgv`: without it the thread cannot load Harper's own module graph at all. Three findings only reachable by building it: - The verdict needs its own `MessageChannel`. `parentPort` carries Harper's ITC traffic, so the first unrelated message was being rejected as a malformed verdict. - The validator must set `workerData.noServerStart`, which `server/DESIGN.md` already documents — without it `threadServer` boots at module scope and loads every root component, so the validator would serve traffic and certify the wrong thing. - The entry has to be the compiled sibling, referenced the way `jobRunner` references `jobProcess.js`. **Two cases deliberately earn no authority rather than being refused**, on the principle *no verdict means no authority, never no verdict means no deploy*: - **Safe mode** stages without activating. It may not execute configured code, so it can certify nothing — and safe mode is transient, so the next ordinary preparation certifies and activates. - **A branch-configured component** deploys uncertified. A branch's location is derived only from the application and database names, so a certification load would open the store the live version is serving from: a candidate could mutate rows, throw, be rejected, and leave the live version serving the mutation. Certifying against the base store instead is no better. Not deferrable like safe mode, because certification cannot succeed for these until validation-scoped branch storage exists. The in-process path is gone — 87 lines of pollution avoidance (the registration guard's in-worker rationale, the status sink, scope and module collection) that existed only because validation shared a process with serving code. `.complete` minting and activation are now separate concerns, so the swap tests exercise activation without minting. Boot and certification build their load options from one helper (`rootApplicationLoadOptions`), so identity and mount cannot drift between them — hand-plumbing a subset is how they would. Guarantee stated narrowly on purpose: within the lifetime of a preparation. A package deploy's root-config entry is still written before the build and never rolled back, so a rejected v2 can be re-prepared and activated after a restart. That needs config staged with activation, which is step 3.
The test that makes this step's claim checkable, and it fails on `main` with "Missing expected rejection" — there, `prepareApplication` with a candidate that throws at load *succeeds* and publishes it. That is the defect: the in-process check was gated on `!isMainThread` and the operations API deploys on main, so step 1's ordering fix reordered a no-op. Tests run on the main thread, so this covers exactly the path that was unprotected rather than a helper in isolation. It asserts the previous version still serves byte for byte, and that nothing was left behind claiming the rejected candidate had been validated. Three more: - a candidate that loads cleanly is still published, so the gate is not simply refusing everything; - a candidate whose load never returns is rejected on a deadline rather than waited on — the in-process check had no answer for this and held its validation chain for the life of the process. The fixture blocks with `Atomics.wait` rather than spinning, since a top-level `await` in a CJS resource is a syntax error and proved nothing; - `markCandidateComplete` refuses to mint `.complete` with no certification, which is the gate itself.
…rtWorker thread Also covers the two cases that earn no authority rather than being refused, and the three validator requirements that are easy to miss — its own MessageChannel, noServerStart, and the compiled entry — since each of those was found by building it rather than by reading the code.
…anch decision The planning review was explicit that a helper-only test cannot prove this step, because what was broken was the WIRING on the main thread rather than the check itself. So this drives the real operations API: deploy v1, deploy a v2 that installs cleanly and throws at load, assert the operation fails — and then assert v1 still **answers requests**, which step 1's availability test explicitly did not cover, since it sampled the component directory on disk and exercised no route. A second case deploys a loadable v3, because a gate that refuses everything would satisfy the first assertion just as well. The response body is not surfaced by the shared `operation` helper, so the candidate's own error message is asserted in the unit test rather than here; this one asserts the operation failed, which is the part that was wrong. Also unit-tests the branch-configured decision directly: boot still gets its `branchedDatabases`, certification never does, and the caller is told to skip certifying. Mutation-verified — handing certification the live branch settings fails it. That path is worth testing at this level because the hazard is exactly that a certification load would open the store the live version serves from.
…round
**A caller-supplied `certified` flag was exactly the forgeable proof the internal
record exists to avoid.** `activateCandidateApplication` is exported, so any
caller could pass `certified: true` and mint authority for an uncertified tree.
The parameter is gone: the record decides, the caller cannot assert. That also
made the swap tests simpler — nothing certified those candidates, so they
naturally skip minting.
**The verdict port was reachable from candidate code.** `workerData` is visible
via `require('node:worker_threads')`, so a candidate could post its own passing
verdict and certify itself. The port is captured and deleted from `workerData`
before any candidate code runs — the difference between a capability this module
holds and one the whole thread holds.
**Success was posted before Scope teardown.** The in-process check I deleted had
established that a Scope which fails to close is a REJECTED validation, not a
warning: `close()` stops at the throwing listener, so the scope stays partially
live. Posting a pass and then failing teardown would certify a candidate whose
own cleanup is broken — and since the thread exits either way, nothing downstream
would ever learn. Teardown now happens before the verdict, and a failed close
fails certification. That protection was mine to lose and I lost it.
**`terminate()` is a NAPI segfault under Bun**, which I noted in the design and
then used anyway. Under Bun the worker is asked to exit itself and its exit
awaited, matching `manageThreads`; a termination that fails is now reported
rather than treated as cleanup done, because the caller is about to remove a tree
the thread may still be reading.
**`.complete` now carries content, and recovery requires it.** An empty marker is
one an older build wrote after a validation that was a no-op on the main thread —
indistinguishable, until now, from one a validator earned. A candidate staged by
an older build and found after an upgrade therefore rolls BACK to the committed
tree rather than forward onto something nothing certified. Conservative direction,
and it costs only an in-flight deploy across an upgrade.
Two tests added, both mutation-verified: accepting an empty marker as authority
fails the legacy-marker test, and a branch-configured component still deploys.
Stated rather than implied: the composition of "activate, mint nothing, then
crash" is not covered — it needs a crash the unit harness cannot stage.
…ator **Safe-mode stage-only was wrong, and the review showed why.** I adopted it from a planning round on the reasoning that safe mode is transient so a staged candidate could wait for the next ordinary preparation. Nothing resumes it: the staged tree carries no journal, so `recoverInterruptedActivations` removes it as build residue at the next start — while `deploy_component` had already returned success, replicated the operation, and run its restart phase. An operator booting into safe mode to replace the component crashing the node would have got a 200, live peers, and a node that came back running the broken component with the fix deleted. That is worse than the behaviour it replaced. It now takes the same shape as the branch-configured case: the deploy happens and earns no authority. No `.complete`, so a crash mid-swap rolls back to the committed tree. The principle was right — *no verdict means no authority, never no verdict means no deploy* — and stage-only broke the second half of it. **The validator was the only Harper worker with no `resourceLimits`.** A candidate whose top-level load builds a large in-memory index would balloon a thread nothing constrains, and the OOM killer would take the whole process down mid-deploy while the previous release was healthy. The calculation is now shared with `startWorker` rather than duplicated, so the two cannot drift. **It also bypassed the `processShuttingDown` guard**, so a deploy racing shutdown spawned a thread the shutdown path knew nothing about and then waited out its deadline on a process that was leaving. **The concurrency cap did not hold.** Release decremented and then resolved a waiter whose increment ran a microtask later, so anything entering that window admitted itself too. The slot is claimed before yielding now. It remains per thread — module state — which bounds validators per worker rather than per process; that is a real limit, not a fixed one.
… hang I added
Two blockers from round 2, both mine.
**An uncertified activation committed a tree that was never fsynced.**
`markCandidateComplete` was the only caller of `syncCandidateTree`, so skipping
the mint skipped the flush — and `syncRenameParents` syncs directory entries, not
contents. A power loss shortly after a branch-configured or safe-mode deploy
returned success would leave the live path holding zero-length files, with the
aside already retired and no journal. My previous commit widened this by routing
safe mode down the same path.
The flush moved to `activateCandidateApplication`, unconditionally. Certification
decides what a tree MEANS; it was never what makes it durable, and putting the
two in one function is what let them be skipped together.
**The termination waiter was installed after the event it waited for.** The
validator `realExit`s immediately after posting its verdict, so on any turn where
the parent sees the exit before the queued message, the `once('exit')` attached in
the `finally` never fired: `certifyCandidate` never returned, its slot was never
released, and `prepareApplication` sat inside the preparation lock forever. Two of
those and the node stops deploying until restart. Reachable on the ordinary
failure path, not an exotic one — and introduced by the Bun fix in the previous
commit.
The exit promise is now created when the worker is, so it cannot be missed, and
every wait in that `finally` is bounded: it runs inside a deploy holding the
preparation lock, so a termination that never settles is the same wedge arrived at
from the other side.
No test for the fsync: it has no readable effect and the call is internal, so the
honest options were a vacuous assertion or none. I wrote the vacuous one first —
`assert typeof fn === 'function'` — and deleted it. The hang is covered
incidentally, in that the suite would stop rather than fail if it returned.
…idator's slot The last round-3 blocker. **The force-exit message had no receiver.** `terminate()` segfaults under Bun, so that message is the only way the parent can end a validator there — and I was relying on `manageThreads`' worker-side block having registered a handler as an import side effect. The validator registers its own now: a capability that is the only way to stop a thread should not depend on which modules happened to load. **Grace expiry released the slot while the validator might still be alive.** A runaway thread now keeps its slot. Releasing it let the node start another while the first still held the candidate tree open and consumed the heap the cap exists to bound — and the caller is about to sweep that tree. Bounded either way by `MAX_CONCURRENT_CERTIFICATIONS`, so the worst case is certification stopping on that thread and saying so, rather than quietly overcommitting.
Holding the slot was right; holding it forever was not. Each runaway validator would have shrunk the cap for the life of the process, and a few would stop the node deploying at all — trading a bounded overcommit for an unbounded outage. The slot is now held until the thread actually goes away.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a robust deploy-certification mechanism that spawns an ephemeral validator worker thread to certify candidates before activation, enforcing a specific .complete marker payload for roll-forward authority. The feedback highlights several key improvements: preventing scope teardown failures from masking load errors in deployValidator.ts, resolving a race condition in acquireSlot that could cause queue-jumping, optimizing worker termination to avoid double-waiting on timeouts, and throwing early in certifyPreparedCandidate to avoid spawning unnecessary worker threads when root-config options fail to resolve.
… reporter CI caught certification **rejecting a valid component** — 12 red checks from one cause. `integrationTests/components/acl-connect.test.ts` has a fixture doing `server.mqtt.authorizeClient = …`, and `server.mqtt` is created by the mqtt plugin's own load. The validator sets `workerData.noServerStart` to stop `threadServer` booting the whole server, and that suppresses the plugin loads too — so the assignment threw on `undefined` and a component that works perfectly on a serving worker failed certification. `noServerStart` was never sufficient for runtime equivalence, which is what the planning review meant by needing one shared loader entry point. `loadRootPlugins` is extracted from `loadRootComponents` at the boundary that was already there: the Harper root component (the global plugins) loads, and the other applications do not. Both callers use it, so they cannot drift on what "the plugins are loaded" means. No listeners are bound — plugins register handlers on the scope's server object, and the port binding belongs to `threadServer`'s startup, which the validator still suppresses. **The error reporter is now installed after that bootstrap**, so it only ever sees the candidate. Installed earlier it captured the first error from anything the root config names — and since `deploy_component` writes a component's config entry *before* building it, that includes the candidate's own live path, which does not exist yet on a first deploy. Certification was rejecting the candidate for the absence of the very thing it was about to create. Also: certification no longer resolves the configured APM preloads. `getImportModules()` memoizes on first call, and a validator spawns much earlier than the first serving worker, so it froze an empty preload list and broke `preloadSafeMode.test.js` — a test-visible symptom of a real ordering hazard. A throwaway thread should not appear in an APM once per deploy either. acl-connect goes from 13 cancelled to 13 passing; 402 unit and 20 deploy/component integration tests pass locally.
From gemini-code-assist on #2476, all three correct. **A teardown failure no longer masks the load error.** A throw from `loadComponent` — a syntax error, an unreadable file — reaches the same `finally`, so a scope that then failed to close replaced the candidate's real error with a note about its scopes. The operator got the symptom instead of the cause. Gated on the load having actually succeeded. **One termination grace for the whole thing, not one per step.** Racing `terminate()` against the grace and then racing the exit against another could wait twice as long before calling a hung validator hung — and this runs inside a deploy holding the preparation lock. **An unresolvable root-config mount throws before spawning.** It used to start a thread whose only job was to fail on the same condition a moment later. The fourth suggestion — hand the slot directly to the next waiter so `acquireSlot` is strictly FIFO — describes a real starvation risk, but a previous commit already closed the queue-jumping window by claiming the slot before yielding. Left as is rather than churning the same lines twice; noted on the thread.
Windows CI failed three of the certification tests with "exited with code 0 without reporting a verdict". The validator `realExit`s the instant it has posted, and on Windows the parent consistently observes that exit before the queued message — so a candidate that failed to load, and one that loaded fine, both arrived as "no verdict". The protocol's own rule then made that a failure, which is the safe direction but the wrong answer. The pass/fail bit now travels through a `SharedArrayBuffer` written synchronously before the exit: shared memory needs no event-loop turn, so it cannot be outrun. The message still carries the candidate's error text, which is detail rather than authority. `VERDICT_NO_ANSWER` is the initial value, so silence is still a failure — the fix makes a real verdict reliable rather than inferring one from an exit code, which would have been minting authority from silence by another route. The flag is deleted from `workerData` alongside the port before any candidate code runs, for the same reason: a candidate that could write it could certify itself. Not reproducible locally — the race is consistent on Windows and unobservable on macOS, and I could find no seam to force a dropped message without faking the thing under test. Windows CI is the check.
Windows CI reported `exited with code 0` for a candidate that THROWS at load, which means the validator took the success path: on that platform the candidate is not being loaded at all. Only the parent failing closed on the missing verdict stopped that becoming a published component — a false pass is the one outcome this step exists to make impossible, and it was one fail-safe away. Certification now requires the load to have done something: a run that opened no scope and loaded no module has not exercised the candidate, so it cannot vouch for it. Asserted only when the candidate declares component configuration, since a component of nothing but static files legitimately loads nothing and cannot fail at load either. This does not explain WHY the Windows load is a no-op — that is still open, and not reproducible on macOS. What it does is turn a silent false pass into a diagnosable failure, which is both the right behaviour and the only way to see the cause from CI.
…trap The Windows failure was not what I read it as. It reported `exited with code 0` and never reached the "loaded nothing" check I added last commit — so `report` was never called at all. A worker's event loop draining ends the thread even with a promise still pending, so a bootstrap that never settled presented as "exited without reporting a verdict", and no deadline could fire because nothing was left alive to time out. Two changes, both correct independently of the platform: - A ref'd handle held for the whole certification, so the thread cannot exit while it is still deciding. Silence is now impossible where it used to be the default failure mode. - The bootstrap has its own bound, inside the parent's deadline, and its error names the phase. Without that, a hang there is indistinguishable from a candidate that hangs. What this exposes is more interesting than the bug: loading Harper's global plugins is part of the worker bootstrap, and parts of it expect to be a member of the topology a validator deliberately is not — so it can wait on something that will never arrive. That is the tension the planning review named between a detached validator and runtime equivalence, showing up as a hang rather than as an argument. The next CI run should say which phase.
Certifying a candidate loads it for real, and every non-root load runs
`symlinkHarperModule`, which links the running install into the component's
`node_modules/harper` so `import 'harper'` resolves to the live instance. So
certification wrote into the tree it is only supposed to read, and that tree is
then renamed into the live path.
The packer dereferences symlinks and recurses into linked directories, so a
component carrying that link packages the whole Harper install. That is what
broke `Integration Tests 4/6`: `package_component` on a freshly added component
spent 46s tarring and then failed with "Maximum response size reached". It
passed on main only because `add_component` never loads the component, so
nothing had created the link before it was packaged.
`certifyCandidate` now snapshots the candidate's `node_modules` before the load
and, in the same `finally` that terminates the validator, removes only the links
its own load created. Only what it created, because a `file:<directory>` deploy
stages a symlink to the developer's own source tree: deleting a link they
already had would be certification reaching outside the candidate to modify a
working tree. Nothing is taken away either way — a serving worker recreates the
link the next time it loads the component.
Packaging a component a worker HAS loaded still follows the link into the
install. That is pre-existing, and `scanPackageDirectory` already documents the
missing symlink-cycle protection behind it; it is not this change's to fix.
Verified end to end: the new `certified-deploy` case fails on this branch
without the cleanup ("no link to the Harper install was left behind") and
passes with it, and it deploys WITHOUT a restart deliberately, since a serving
worker legitimately recreates the link. `integrationTests/apiTests/components.test.mjs`
now passes 25/25 locally with `package_component` at 22ms.
A unit-level version of the same assertion was written and dropped: in the
mocha environment `symlinkHarperModule` never gets far enough to create the
link, so the test passed with and without the fix.
DESIGN.md's safe-mode bullet still described the stage-without-activating draft
that was reverted earlier on this branch; corrected alongside.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…nded it Windows CI has reported `Certification of shop exited with code 0 without reporting a verdict` across three different hypotheses — a lost message (fixed by the SharedArrayBuffer flag), a candidate that loaded nothing (the check never fired), and an event-loop drain (the ref'd interval is created synchronously at module scope, so a drain cannot be it). Each diagnosis was a guess, because an exit code carries no evidence about who ended the thread. Since the ref'd interval rules out a drain, a silent code-0 exit means something CALLED exit, and the only thing that can name it is a stack captured at the exit itself. `process.exit` runs `exit` listeners, so an `exit` handler fires for `realExit` too; it logs through `console.error` rather than the logger, because a logger write queued at exit may never flush. Diagnostics, not a fix: the parent's treatment of a missing verdict is unchanged, and nothing about the verdict protocol moves. It also improves the operator-facing story for any future silent exit, on any platform. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…it wait Two majors from the gemini pre-push leg, both cases of a wait this module claims is bounded and is not. `acquireSlot` queued without a deadline. The cap deliberately withholds the slot of a validator that will not die, so an unbounded queue behind it turned one stuck thread into every later deploy hanging inside the preparation lock with nothing to report. It now takes the certification timeout, and a caller that cannot get a slot fails with a 503 naming why. A waiter that times out leaves the queue, because leaving it there would let a later release hand a slot to a caller that is gone — drifting the count DOWN and admitting more concurrent validators than the cap, not fewer. The termination path asked the worker to exit and awaited that exit in one expression inside one `try`. A synchronous throw from the ask — `postMessage` on a channel already in an invalid state — jumped straight to the `catch`, skipping the wait entirely, so the caller swept a candidate tree whose thread was still terminating. Asking is what can fail; waiting must happen either way, so the ask is now separately fallible and the wait is not conditional on it. Verified: new unit test occupies both slots with validators parked in `Atomics.wait` and asserts the third deploy fails with the slot-timeout error and a 503 rather than queueing. It needs no sleep to arrange — `acquireSlot` claims synchronously when a slot is free, so both occupying calls hold theirs by the time the third runs. Carried rather than fixed, from the same leg: a candidate can `parentPort.removeAllListeners()` to defeat the Bun force-exit path (Node uses `terminate()`, so this is Bun-only, and a candidate that wants to do damage has easier routes — certification executes its code with no filesystem or database isolation, which DESIGN.md states); and the absence of that isolation, which is already a documented limitation and a carried major on the PR. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ating comments The previous commit's exit diagnostic produced nothing on Windows, and its silence was not evidence: a worker's `console.error` is piped to the parent ASYNCHRONOUSLY, so anything written on the way out loses the same race the verdict message loses. It used a channel already known to be unreliable there. Progress now travels through the shared buffer, which is the only channel here proven to survive the exit. Slot 1 carries the furthest phase reached — module scope, certification entered, root plugins loaded, candidate loaded, teardown done — and the validator's own `exit` handler adds a mark to it. That mark is the interesting half: a thread torn down from outside (`terminate()`, a native abort, the process going away) never runs its exit handler, so its absence distinguishes "ended itself" from "was ended", which no exit code does. The parent renders both into the failure message instead of reporting only `exited with code 0 without reporting a verdict`. Also from the codex/gemini delta legs: - A queued waiter admitted before its deadline now clears its timer rather than leaving the closure registered until a deadline that no longer applies. - Narrating comments trimmed across `certifyCandidate`, `deployValidator`, `Application` and `operations` — both lenses flagged this, in two consecutive rounds, and the repo has already paid a cleanup commit for it (15c02a1). The constraints and invariants are kept, in present tense; what went is the archaeology ("this used to live inside", "an earlier draft", "the gate") and the step-by-step restatement of code. Refuted rather than fixed, with a test pinning it: gemini called `declaresLoadableContent` a false-rejection risk for static-only components, since nearly every component ships a `package.json` and a static component loads no module. A static-only component deploys fine — it opens a scope, so the "loaded nothing" guard never fires — and the new unit test says so, which also stops a future change to scope creation from silently starting to reject static deploys. This is the third false-rejection shape this feature has produced, so it is asserted rather than assumed. Verified: 10 certification unit tests and 3 certified-deploy integration tests pass locally. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…, and keep the rejection's error text Round 8 of the pre-push review, with four lenses and domain adjudication. The slot release contradicted its own comment. It decremented `active` and then woke a waiter that incremented a microtask later, so the slot was released and re-competed for rather than handed over: a caller arriving synchronously in that window took it, and the woken waiter went to the back of its own queue. A release offered to a waiter that had already timed out was also swallowed, leaving a free slot nobody was woken for. `active` now never dips — the releaser offers the slot along the queue until a live waiter accepts, and only falls back to decrementing when none does — and an admitted waiter holds the inherited slot without incrementing, which is what makes the handoff a handoff. No test: the steal needs a caller arriving inside a synchronous microtask window that `certifyCandidate` does not expose, and the swallowed-wake half is not observable through the public surface either, since the next caller reads `active` directly. A test that passed with and without the fix would be worse than none. Verified by reading, against gemini's mechanism, which was correct. A rejection on Windows lost the candidate's actual error. The shared flag says REJECTED and only the queued message carries the error text, but the exit consistently beats that message there — so the parent settled on the flag alone, reported "exited before reporting why", and then closed the channel, discarding the syntax error the operator needed. The flag stays the authority; the exit now gives the detail a 250ms grace, and whichever answer lands first stands. Recovery logged the opposite of what it was doing. A resumed activation after a restart finds the in-memory certification record empty, so it warned that nothing had certified the candidate and recovery would roll it back — while recovery was rolling it forward on the strength of the on-disk marker. That case is now its own branch and says so. The marker is still never re-minted from its own presence. Also: fixed a factual contradiction the review caught between two comments I added last commit (one said a static-only load opens no scope, the other that it does — it does), and trimmed the diagnosis-history narration that survived the last pass. Carried, and now the reason to re-examine the framing rather than iterate again: under Bun, `terminate()` segfaults, so the parent can only ASK the validator to exit — and a candidate can `parentPort.removeAllListeners()` or simply block its event loop, in which case the ask is never processed, the thread never exits, and its slot is held for the life of the process. Two of those stop the node deploying. Cooperative exit cannot be made reliable against code that is adversarial or merely synchronous, so this is a property of certifying on a thread the parent cannot kill, not a bug to patch. The review's own gate agrees: `framing-recheck: REQUIRED — round 8 still yields a fresh major on this change`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… losing the ITC bootstrap Groundwork for certifying a deploy candidate on a standard-profile worker (#2315 step 2). The planning review called this a blocker for that design, and it is: the validator needs a candidate path, a private `MessagePort` and a shared verdict buffer, and `startWorker` could carry none of them. `...options` is spread into the `Worker` constructor AFTER the bootstrap `workerData`, so passing `options.workerData` replaced that object wholesale and took `addPorts`/`addThreadIds` with it — the thread came up with no ITC wiring and nothing said so. `workerDataProviders` is no way around it either, because it `structuredClone`s and a `MessagePort` must be transferred. So: `extraWorkerData` is merged into the bootstrap rather than substituted for it, `extraTransferList` is concatenated with the port list, and `options.workerData` is now refused outright with a message naming the alternative. Reserved keys are refused too, reusing the list `registerWorkerDataProvider` already validates against rather than adding a second one. `noServerStart` gets a supported route for the same reason: it is a RESERVED key, so neither a provider nor `extraWorkerData` can supply it, but a thread that must not serve has to. `options.noServerStart` adds it, and only when asked for, so the default spawn's workerData is unchanged. Validation runs as the first thing in `startWorker`, before `buildWorkerExecArgv`. That ordering is load-bearing, not tidiness: `getImportModules()` memoizes, so throwing after it freezes the configured preload list for the life of the process on a spawn that never happened. My own test proved it — the rejection cases poisoned `preloadSafeMode.test.js` until the check moved up, which is the same memoization hazard the validator's `preloads: false` exists for. `execArgvOptions` is threaded through so a caller can opt out of preloads deliberately. Also fixes #2491 in passing, which the review flagged as a consequence of this design and which turned out to predate it: `workerCount` is a module-global written only here, and a start omitting `threadCount` set it to `undefined`, after which `restartWorkers`'s default `maxWorkersDown = Math.max(Math.floor(workerCount / 8), 1)` evaluates to `NaN` — which the `maxWorkersDown < 1` guard does not catch, because `NaN < 1` is false. An unthrottled rolling restart is a service gap during the operation chosen to avoid one, and job workers already trigger it. Now only a start that describes the topology writes the global. Scope note: this is one line beyond step 2's remit, taken because the design cannot spawn a non-topology worker without it. Verified: 3 new tests cover the merge, port transfer, reserved-key and `workerData` rejection, and `noServerStart` being absent by default; they fail on base because the option did not exist. The existing `workerDataProviders` and `preloadSafeMode` suites pass alongside them in one run, which is what caught the ordering bug. `processGroupReclaim` fails on this machine and on base identically — it reads `/proc/<pid>/stat`, which macOS does not have. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… release database handles Two prerequisites for moving certification into a helper process, both worth having on their own. The verdict constants, progress slots and `describeProgress` move to `components/certificationProtocol.ts`, which imports nothing from Harper. Both ends of the protocol need them, and one end is about to become a process whose whole job is to report a verdict even when the module graph it is testing does not load — so it must not reach `certifyCandidate` (and through it `manageThreads`) to find out what a verdict looks like. `certifyCandidate` re-exports them so existing importers are unaffected. The module also defines the helper's exit codes now, because a SharedArrayBuffer cannot cross a process boundary: the flag the candidate's thread writes stays readable only inside the helper, which encodes the answer as its exit status. That survives a lost IPC message the way the flag survives a lost worker message, and a candidate cannot set it — worker threads have no `process.send`, and nothing in the candidate's thread chooses the helper's exit. Any other code is a rejection, so silence still cannot become a pass. Second, the validator now calls `closeLoadedDatabases()` in its teardown, on pass and on rejection alike. It reaches `getTables()` through `loadRootPlugins`, which opens the whole database graph, and `resources/databases.ts` documents that a thread exiting without closing leaks process-global RocksDB handles and blocks an online `restore_backup` from confirming a database is closed. `jobProcess.ts` already does this; the validator did not, so every certification leaked. Found by the planning review. It is attempted independently of the scope closes so one failure cannot skip the other, and its own failure is logged rather than allowed to mask the load result. Verified: 10 certification unit tests pass. The leak itself is asserted by reading — registry refcount coverage is listed in the design note's verification route and belongs with the helper-process change, where forced death is the backstop. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… both requirements The mechanism is complete and reviewed; WHERE the candidate load runs is not settled, and the two available hosts each fail a requirement the guarantee depends on. So this ships behind `HARPER_CERTIFY_DEPLOYS`, off by default, rather than a guarantee that holds only on some runtimes or a load that is not the load a serving worker performs. The constraint, found by running the code rather than reasoning about it: - A THREAD shares this process's RocksDB handles, so its load is genuinely serving-equivalent — but under Bun `terminate()` triggers a NAPI segfault, so the parent can only ask it to exit, which a candidate blocking its event loop defeats. The thread never exits and its concurrency slot is held for the life of the process. - A separate PROCESS can be SIGKILLed, but cannot open the databases at all. RocksDB's lock is exclusive per process, so the helper died with `IO error: While lock file: … Resource temporarily unavailable` the moment `loadRootPlugins` reached `getTables()`. `security/auth.ts` calls `table()` at module scope, so loading fewer plugins does not avoid it. Opening `readOnly` takes a shared lock and would work, but would then reject any candidate that writes during load — a false-rejection class worse than the problem, and the fourth time this feature has produced one. Worth recording plainly: the planning gate returned `chosen-approach-sound` for the helper-process design, and that design cannot work. The gate reviews reasoning, not viability — one `fork` would have answered it in a minute, and neither the reviewer nor I ran one before writing the note. With the switch off, `deploy_component` behaves exactly as before this work: built aside, swapped in, no `.complete`. The two documented uncertified cases (safe mode, branch-configured) are unchanged. DESIGN.md now records the host tradeoff as a table, the RocksDB lock, the Windows finding (a bare `new Worker` dies inside its import graph — exit 0, no error event — where a standard-path thread does not), the corrected `startWorker` rationale, and the diagnosis rule that cost a CI round: never diagnose a dying worker with `console.error`, because its stderr is piped asynchronously and loses the same race the verdict message loses. Verified: 11 unit tests and 3 integration tests pass. Both suites now enable the switch explicitly — without that they would still pass while proving nothing, since an uncertified deploy publishes and only the rejection case would notice. The new default-off test asserts the pre-certification behaviour (a candidate that throws at load is published, and mints no `.complete`), which is what would catch the switch being flipped on by accident. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…and drop dead exit codes Two findings from round 9 that are defects in the previous commit rather than consequences of the gate. `loadRootPlugins` reaches `getTables()`, so the database graph is open BEFORE the candidate is touched — but the cleanup sat inside the teardown of the candidate load, so a bootstrap failure or its phase deadline skipped it entirely and leaked the handles process-wide. It now runs in an outer `finally` around everything after the import, which is the only placement that covers the phase it was opened in. The comment says why a close failure is logged rather than turned into a rejection, so this does not get "fixed" the other way later: a candidate whose own teardown fails is the candidate's fault and does reject (the scope closes), but Harper's teardown failing is not, and rejecting a working component for it would fail a good deploy without un-leaking anything. `HOST_EXIT_*` went with the helper-process design. The reviewer noticed they were exported and unused; a protocol constant nothing speaks is worse than no constant, since the next reader has to work out whether it is load-bearing. Verified: 11 unit and 3 integration tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ot load Gating certification off fixed the default deploy path but not CI: the unit suite opts into the switch, so Windows still ran the one host that does not work there. A validator thread on Windows dies inside its own import graph — before its first statement, exit code 0, no `error` event — so all four certification cases fail identically. Filed with the full evidence trail and the narrow experiment that would isolate bootstrap from mesh membership: #2494. Removing this skip is the acceptance test for that issue. Two things this turned up: `describe(name, { skip }, fn)` is node:test's signature, not mocha's. Mocha treats the options object as the suite body and registers NOTHING — the suite reported "0 passing" on macOS and I nearly shipped that as a pass. The repo's idiom is `this.skip()` in a hook, which is what this uses. The branch-configured cases were failing on Windows for an unrelated reason, masked by the validator failures: `getConfigObj()` is undefined in that test process, so `getConfigObj()[appName] = …` throws. Those tests never spawn a validator — they assert `rootApplicationLoadOptions` withholds branch settings — so they have been broken on Windows since they were written. Their `afterEach` is now guarded, because mocha runs that hook even for tests skipped in `beforeEach`, and the assumption is noted in #2494 for whoever removes the skip. Verified: 11 passing on macOS; on Windows every test in the suite skips and no hook throws. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`Integration Tests 2/6 (Windows)` died with ECONNREFUSED across every job test — the Harper instance stopped mid-suite. That shard was green on all three earlier heads of this PR and on main's last four runs, and the only change touching shared worker/restart machinery was this one. The cause is a semantic change I did not intend. `workerData.workerCount` was `undefined` inside a job worker, and preserving the module-global made it the serving thread count instead — so `getWorkerCount()` changed meaning inside every job worker, not just the global that `restartWorkers` reads. A narrower version exists (pass `undefined` per-worker, guard only the global assignment) but I cannot reproduce Windows locally to verify it, and this was already one line beyond step 2's remit. So it comes out entirely and #2491 keeps its own verification route — run a job, then a rolling restart with more than 8 threads, and assert the throttle holds — which is the coverage this needed and which belongs with the fix rather than here. The `extraWorkerData`/`extraTransferList` merge is untouched; only the `workerCount` line reverts. Unrelated, confirmed while checking this: `workerDataProviders.test.js` poisons `preloadSafeMode.test.js` when mocha is handed them in that order, because the former spawns workers with the default execArgv and `getImportModules()` memoizes. Both files predate this branch, it reproduces with my new test file absent, and CI is unaffected because mocha loads files alphabetically and `preloadSafeMode` sorts first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…trip the bootstrap Three defects in the API added earlier in this PR, all found by round 11. The first is a process-wide crash, not a failed spawn. The unexpected-exit path re-invokes `startWorker(path, options)` with the SAME options object, and a transferred port is single-use — so a restartable worker carrying `extraTransferList` throws `DataCloneError` on its second spawn, synchronously, inside an `exit` listener with nothing to catch it. The first caller to use port passing without `autoRestart: false` would take Harper down when its thread died unexpectedly. Refused up front instead: transferring ports means owning the thread's lifetime. The `workerData` guard also checked truthiness, so `workerData: null` sailed past it and nulled the bootstrap — present but falsy still replaces the object. It now tests `in`. And the guard only covered `workerData`: a raw `transferList` in options is spread last and REPLACES the merged list, dropping `portsToSend`, so the thread comes up with no ports to its peers. Refused the same way. Separately, in `certifyCandidate`: the slot was acquired before the `try`, with channel allocation, the SharedArrayBuffer, a `realpath` and the link snapshot between. Any throw there leaked the slot permanently, because nothing else decrements `active`. Those now run inside the guarded block, with the two values the `finally` needs declared above it and the channel close made optional for the case where the throw beat the assignment. Verified: the guard tests cover all three refusals — including the restart case, which asserts the contract rather than the crash, since reproducing the crash means killing the test process. 11 certification tests and 8 thread tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
dawsontoth
marked this pull request as ready for review
September 3, 2026 22:04
… test skip The previous commit's crash guard was incomplete. It refused `extraTransferList` without `autoRestart: false`, which covers the unexpected-exit path — but `restartWorkers` replaces workers through `worker.startCopy()` from three call sites, and that re-spawns from the same options object with its ports already spent. A rolling restart could still throw `DataCloneError` synchronously inside the restart loop, taking the rest of the restart with it. A thread carrying transferred ports is ephemeral by contract, so the restart loop now skips it outright rather than trying to replace it: left alone, it finishes its single task or hits its own deadline. `startCopy` keeps a named refusal as the backstop for a direct caller, which beats a clone failure from inside `new Worker`. Also narrowed the Windows skip, which was suppressing more than it needed to. Only five of the eleven certification tests spawn a validator; the suite-level skip also took out the default-off case, the mint gate, safe mode, and the branch-configured options — and the default-off case is exactly the Windows coverage worth keeping, since it guards the behaviour Windows operators actually get. The config-dependent tests are guarded on their real precondition rather than on the platform: `if (!getConfigObj()) test.skip()`. Naming the dependency means they start running again the day that environment difference is fixed, instead of waiting for someone to notice a stale `win32` check. Verified: 11 certification tests and 13 thread tests pass on macOS. On Windows five skip for the validator and three for root config, leaving the default-off case, the mint gate and safe mode running there — which is three more than before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
This comment has been minimized.
This comment has been minimized.
… not the one planned Review blocker, and correct. The "use `startWorker`" bullet described the Windows fix as done. It is not: `certifyCandidate` still spawns a bare `new Worker`, its own docblock still argued the opposite case in the present tense, and the test skip says plainly that Windows certification fails. Three places, two of them wrong. A contributor trusting that paragraph could have removed the skip and broken Windows CI for a bug that is still there. The prose survived a revert: it was written while the helper-process design was still live, and did not come back out when the implementation did. DESIGN.md now states what ships (a bare `new Worker` that dies in its import graph on Windows, #2494), that moving to `startWorker` is intended and unstarted, and that the bootstrap plumbing for that migration landed here and is currently unused. It also stops claiming the standard path repairs Windows — the evidence proves the bespoke import graph fails, not that mesh membership or the standard bootstrap is what fixes it, and #2494 names the experiment that would tell them apart. `certifyCandidate`'s docblock now says the same thing rather than the reverse. Also adds the one-shot worker test the second comment asked for, partially. The flag and `startCopy`'s named refusal are asserted directly. The restart loop's filter is not driven: `restartWorkers` performs a real node restart and reinstalls applications — it shelled out to `npm pack` when I tried — so driving it from a unit test would exercise far more than a one-line filter, slowly and fragilely. The test says so rather than implying coverage it does not have. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
kriszyp
reviewed
Sep 4, 2026
kriszyp
left a comment
kriszyp
left a comment
Member
There was a problem hiding this comment.
This is a pretty significant amount of code to commit to maintain. My understanding was that we were trying to pursue a staging mechanism for deployments, not a "certification" mechanism. Why do we need to be responsible for this certification? Why isn't it more appropriate to do testing elsewhere (test envs or CI)? If availability of bad deploys is a concern, why not use rolling deploy in coordination with GTM? Do really think this is adequately mimicking the worker threads (and how far do we want to go with that)? And woudln't forcing rollbacks of "bad" applications potentially hinder the debugging of it? (If something is actively broken, that is the best reproduction of a failure).
🤖 Reviewed with Codex
| const fail = (message: string) => settle({ certified: false, error: new Error(message) }); | ||
|
|
||
| try { | ||
| const started = new Worker(entry, { |
Member
There was a problem hiding this comment.
This bare Worker bypasses collectProvidedWorkerData, including the configOverrides provider added so workers inherit env.setProperty() values. The validator can consequently load the on-disk root path, database paths, mounts, or storage settings while serving workers use the parent's overrides; its verdict then describes a different environment. This also makes the unit suite depend on an ambient install—the parent test harness's root/database overrides never reach this child. Please introduce a shared non-topology worker bootstrap that carries required inherited configuration, or use the managed spawn path after separating topology registration from option construction.
Contributor
Author
There was a problem hiding this comment.
Confirmed, not fixed, and I am leaving this thread open rather than closing it.
You are right about the mechanism and about the consequence. The bare Worker bypasses collectProvidedWorkerData entirely, so configOverrides never reaches the validator and its verdict can describe the on-disk environment rather than the one serving. It also makes the unit suite depend on an ambient install, which is why those tests are skipped on Windows (#2494) rather than passing there.
Codex reached the same finding independently for a helper-process variant, so this is not specific to the current spawn — it is what "not using the managed path" costs.
Both of your suggested fixes are the right shape, and they are the same work: a shared non-topology bootstrap, or separating topology registration from option construction so the managed spawn can be used. extraWorkerData/extraTransferList/noServerStart in this PR are the beginning of exactly that and are currently unused by certification. I have not gone further because your review summary questions whether certification should exist at all, and building the bootstrap out on a premise you may reject is the wrong order.
🤖 Addressed by Claude Code
| } catch {} | ||
| } | ||
|
|
||
| let active = 0; |
Member
There was a problem hiding this comment.
This cap is module-local, but prepareApplication may run on any Harper thread. Each worker isolate gets its own active = 0, so N request workers can spawn up to 2N heavyweight validators concurrently. That defeats the stated process-memory bound and can create exactly the thread/OOM pressure this limit is intended to prevent. Please broker certification through one owning thread or use a genuinely cross-thread semaphore.
Contributor
Author
There was a problem hiding this comment.
Correct, not fixed, thread left open.
The cap's own docstring claims a process-wide bound it does not have: active is module state, prepareApplication runs on any thread, so N request workers admit up to 2N validators. Gemini raised the same limitation earlier and it is listed as a carried major in the PR description, which is not the same as fixing it.
Both of your options — brokering through one owning thread, or a genuinely cross-thread semaphore — are the same work as the lifecycle ownership the shutdown-race thread points at, and as the managed-spawn question in the configOverrides thread. All three are one change: the validator becomes a managed thread rather than a freelance one. That is the natural next step if certification survives your summary question, and the wrong thing to build before it is answered.
Meanwhile the practical bound is that certification is off by default, so the cap admits zero.
🤖 Addressed by Claude Code
…own before spawning Two of kriszyp's four inline findings, both correct and both narrow enough to fix without waiting on his premise question about whether certification should exist at all. `extraWorkerData` is spread AFTER provider output, so a caller could silently replace a registered provider's value. `configOverrides` is the consequential one: overriding it leaves the thread reading on-disk config while its parent runs on `setProperty()` overrides, so a validator's verdict would describe a different environment than the one serving. `registerWorkerDataProvider` already refuses name collisions; the merge path now applies the same ownership rule, before any side effect. The shutdown guard only ran at entry, before a slot wait that can last the full certification timeout and two awaited filesystem calls. Shutdown beginning in that window still produced a validator that is absent from `manageThreads.workers` — so shutdown neither terminates nor awaits it while it loads databases and components into a process that is tearing down. Re-checked immediately before the spawn. Not fixed, and deliberately left unresolved rather than closed: the bare `Worker` bypassing `collectProvidedWorkerData` entirely, and the module-local concurrency cap. Both need real design work — a shared non-topology bootstrap, and brokering through one owning thread or a cross-thread semaphore — and both are inside the mechanism whose existence kriszyp is questioning in the same review. Answering those with code before that question is settled risks building on a premise the owner may reject. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Author
|
@kriszyp I do agree, I think this idea came somewhere during the reviews, pointing out that we weren't validating a candidate before trying to flip it to live. If we don't want this mechanism, that's certainly within the realm of reason. What do you think? My thought is that if we wanted behavior like that, we'd probably do it through the instances: bring something out of GTM, do a deployment on it, verify it (through customer logic), and when it's healthy, bring it back up. Not by doing it through the component level. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
deploy_componentreachesprepareApplication()on the main thread, and the only check that acandidate could actually load was gated on
!isMainThread. So for an operator deploy it ran nothing: acandidate that installed cleanly and threw at load was renamed live anyway, and
.complete— which crashrecovery treats as proof a validation happened — was minted for it.
This PR builds the certification mechanism and lands it switched off (
HARPER_CERTIFY_DEPLOYS).With the switch off,
deploy_componentbehaves exactly as it does today. That is the headline decision andit needs your agreement — see below.
Why it lands off
A certification load has two non-negotiable properties, and no available host has both:
terminate()triggers a NAPI segfault there —manageThreadsand
jobProcess.tsboth avoid it, the latter draining its event loop rather than callingprocess.exit—so the parent can only ask the thread to exit. A candidate that blocks its event loop, or calls
parentPort.removeAllListeners(), never processes the ask: the thread never exits and its concurrencyslot is held for the life of the process. Two of those and the node stops deploying.
died with
IO error: While lock file: … Resource temporarily unavailablethe momentloadRootPluginsreached
getTables().security/auth.tscallstable()at module scope, so loading fewer plugins doesnot avoid it. Opening
readOnlytakes a shared lock and would work, but would then reject any candidatethat writes during load — creating a table, seeding a record — which is a false-rejection class worse
than the problem being solved.
Landing it on by default therefore means shipping a guarantee that holds only on some runtimes, or a load
that is not the load a serving worker performs. Landing it off keeps the reviewed mechanism, the tests, and
the four independent bug fixes below, and leaves the host decision to its own change with the constraint
known up front.
For the human reviewer
The final review verdict is
BLOCK, and I did not adopt it. Its blocker iscomponents/Application.ts:2203-2207— "certification is disabled by default, so ordinary deploymentsstill publish unloadable candidates." That is accurate, and it is the decision above rather than an
oversight: the alternative is one of the two broken guarantees in that table. The reviewer's own
"no host satisfies both requirements" analysis is what produced the switch. If you disagree, the lever is
one line and the on-by-default behaviour is fully tested.
The planning gate cleared a design that cannot work.
--mode planreturnedbetter-alternative-existsfor a job-type worker and I adopted its helper-process alternative, which thenreturned
chosen-approach-sound. It fails on the RocksDB lock. The gate reviews reasoning, not viability —one
forkwould have answered it in a minute, and neither the reviewer nor I ran one before writing thedesign note. Worth knowing when weighing a cleared framing verdict on this branch.
Carried majors, all specific to the on-by-default path and therefore inert while the switch is off:
prepareApplication()can run on any worker, so an N-workernode admits 2N validators, each a full module graph. Node-wide admission is unbuilt.
the validator thread and mutates the candidate tree after the verdict. The process-group machinery
(
registerProcessGroup,terminateProcessGroup) exists and is unused by certification.configOverridesprovideralready carries, and the preload profile for a validator is deliberately off — reasonable for a thread,
and wrong for a process, if the host changes.
exit; the 250 ms grace makes it unlikely rather than impossible. The shared flag remains the authority.
Verification you cannot get from me: Bun and Windows behaviour. I have no Bun runtime here, and the
Windows evidence in this PR comes from CI logs, not from a machine I can step through.
What is fixed regardless of the switch
Four defects found along the way, each independent of where certification runs:
package_componentpackaged the whole Harper install.symlinkHarperModulelinks the runninginstall into
node_modules/harperon every non-root load, and the packer dereferences and recurses intosymlinked directories — so a certification load left a link that turned packaging into a walk of the
install: 46s of tarring, then
Maximum response size reached.certifyCandidatenow snapshots thecandidate's
node_modulesand removes only the links its own load created, which matters because afile:<directory>deploy stages a symlink to the developer's own source tree. The pre-existing half —packaging a component a worker has loaded — is package_component follows node_modules/harper into the install, so packaging a loaded component packages Harper #2487.
workerCountis a module-global written onlyin
startWorker, and a start omittingthreadCountset it toundefined, after whichmaxWorkersDown = Math.max(Math.floor(workerCount / 8), 1)isNaN— which themaxWorkersDown < 1guard does not catch, because
NaN < 1is false. An unthrottled rolling restart is a service gap duringthe operation chosen to avoid one. Fixes Starting a job worker sets the process-wide workerCount to undefined, silently disabling the rolling-restart throttle #2491.
getTables()vialoadRootPluginsand never closed them;resources/databases.tsdocuments that this leaksprocess-globally and blocks an online
restore_backupfrom confirming a database is closed.activewas decremented before waking awaiter, so a caller arriving in that microtask window took the slot and the woken waiter went to the back
of its own queue; a release offered to a timed-out waiter was swallowed entirely.
activenow never dips.Verification
unitTests/components/deployCertification.test.js, 11 tests): main-thread rejection — whichfails on
mainwith "Missing expected rejection" — clean publish, static-only publish, hang/timeout, slotexhaustion, the mint gate, safe mode, branch-configured, and the default-off path (a candidate that
throws at load is published and mints no
.complete).integrationTests/deploy/certified-deploy.test.ts, 3 tests): drives the real operationsAPI and asserts v1 still answers requests after a rejected v2 — which step 1's availability test
explicitly could not show — that a clean v3 publishes, and that the published tree carries no
node_modules/harperlink.uncertified deploy publishes, so only the rejection case would notice.
integrationTests/apiTests/components.test.mjsis 25/25 locally withpackage_componentat 22ms, downfrom 46s-then-failure.
config/preload inheritance. All are on the dormant path and listed above.
Closes nothing on its own — #2315 step 2 continues in the host change.
Review-Coverage: authored=claude; ran=codex; declined=gemini,cursor-grok,cursor-composer,domain; rounds=3 @ afea30b
Human-Review-Need: 4 @ afea30b