fix: address architecture review findings

[camcima]

Non-breaking remediation of the v4.1.0 architecture review (added as ARCHITECTURE_REVIEW.md). Every fix landed test-first — the failing test was observed before the change — and the suite grew from 369 to 388 tests across 48 files.

Confirmed bugs

1. A failed lock release was silently swallowed (High). MutexInterface.releaseLock() reports failure two ways: it throws, or it returns false. The engine handled only the throw. A false return — what the documented PostgreSQL advisory-lock adapter produces on a failed unlock — meant the operation resolved successfully, onReleaseError never fired, isLockAcquired() stayed true forever, and because the engine skips acquisition when the mutex reports the lock is already held, every later operation piggybacked on the stuck lock and never released it. That is the exact scenario the inline comment in runOperation says must not happen.

Both failure modes now flow through one releaseMutex() path and raise the new LockCanNotBeReleasedError. Semantics match the existing throw path: the hook always fires, a successful operation rejects (the caller must learn the lock may still be held), and a failed operation still rejects with its own error rather than the release error.

2. whenIdle() deadlocked permanently and silently (Medium-High). triggerEvent/checkTransitions got a re-entrancy guard in v4; whenIdle(), added in the same release, did not. Calling it from inside a callback can never resolve — the machine cannot reach idle while the runner is blocked on that very callback. Reproduced: the transition commits, the caller's promise never settles, and every future operation queues forever. It now throws ReentrancyError, consistent with its siblings.

API-consistency fixes

• Factory can now configure engine options. It previously forwarded only initialStateName, transitionSelector, and mutex, so factory-created machines silently ran on defaults — no back-pressure, no diagnostic sinks — in exactly the fleet-of-machines case those options exist for. Adds a FactoryStatemachineOptions constructor template, typed as StatemachineOptions minus the three fields the factory derives per subject, so a template value can never contradict the factory (enforced at compile time, pinned by a @ts-expect-error test).
• Statemachine.releaseLock() now reports failures through onReleaseError. The signature stays Promise<void>; changing it to Promise<boolean> is breaking and is deferred to v5.
• Observer accessors return snapshots. getBeforeObservers(), getAfterObservers(), and Event.getObservers() handed out the live collections, so a later detach mutated a list a caller already held, and a caller could cast and clear the engine's own registrations.
• AmbiguousTransitionError carries the competing transitions (candidates, also rendered into the message). A bare count doesn't identify the culprits, while its sibling errors are all context-rich.
• LockAdapterMutex shares one in-flight acquire. The acquired flag is only set after the adapter resolves, so two overlapping calls both passed the check and double-acquired a non-idempotent adapter. A failed acquire is still retryable.
• Removed a redundant cast; re-dated the overdue "removed in v4" deprecation notices to v5.

Docs

Release-failure semantics (mutex.md, errors.md), the factory options template (factory.md), whenIdle re-entrancy and the frame-context caveat (core.md), plus two things easy to misread from the API alone: Timeout schedules nothing — it only fires when something drives the machine (conditions.md) — and event observers are shared by every machine built from the process (observers.md).

Deliberately deferred

Breaking changes are batched for a planned v5 rather than fixed piecemeal: Event mutability (the one back door in the frozen graph), the OnEnterObserver magic-event-name convention, the machineName misnomer (it is the process name), the releaseLock return type, and removal of the deprecated Dispatcher interfaces.

Two non-breaking items were also left alone on purpose: the mermaid label quoting (s_x : "label") is pinned by tests and docs and needs verification in a real renderer before changing rendered output on a hypothesis, and OperationQueue's shift() is irrelevant at realistic queue depths. Both are recorded in the review's status table.

Verification

pnpm test 388/388 passing · pnpm lint clean · pnpm build succeeds with all new symbols in the emitted declarations.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JLxceiNFfXfpGoxdfQEZqw

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (9932a58) to head (c32c9f9).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff            @@
##              main       #52   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           61        62    +1     
  Lines         1801      1870   +69     
  Branches       268       277    +9     
=========================================
+ Hits          1801      1870   +69     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[camcima]

Filed the deferred items so nothing here relies on the review document being re-read later:

Breaking, batched under the v5 milestone: #53 (freeze Event, build-time command registration), #54 (first-class state entry hooks), #55 (releaseLock return type — the other half of what this PR could only partly fix), #56 (processName rename), #57 (deprecation removal).

Non-breaking: #58 (verify Mermaid quoting against a real renderer before touching pinned output), #59 (AsyncLocalStorage re-entrancy detection), #60 (queue introspection), #61 (Timeout scheduler helper), #62 (are-the-types-wrong in CI), #63 (queue dequeue cost — likely wontfix).

#64 indexes all of them with a suggested order. The last commit here links the review's deferred rows to the issue numbers.

Worth calling out: #59 is the one I'd take next. It is the same class of defect as the whenIdle() bug this PR fixes — a re-entrant call that silently wedges the machine forever — except it triggers after an await inside a callback, which the synchronous guard cannot see.