v3.0.0 — execution engine rewrite, typed errors, hardened builder - #17

…erver

- States, transitions, and Process are now constructed only via the builder
  (internal symbol-based construction key)
- ProcessBuilder validates: missing/multiple initial state, unknown source,
  unknown target, empty event name, conflicting duplicate transitions,
  optional strict orphan-state detection
- Process is frozen; State and Transition have no public mutators
- Closes IMPLEMENTATION_REVIEW.md #3 (graph immutability), #5 (empty event
  names at build time), #6 (duplicate-with-conflict at build time)

The two-pass simplification of build() created Transitions pointing at
Pass-1 stub States while Process exposed Pass-2 final States, so
t.getTargetState() !== process.getState(t.getTargetState().getName())
even though the names matched. Statemachine traversal would land on
orphan State instances at runtime.

Replaced the multi-pass approach with a depth-first leaf-first
construction that builds each target State before the Transition
referencing it is created, guaranteeing referential identity for acyclic
graphs. Cyclic graphs are handled by a second-sweep rebuild scoped only
to the cycle members. Added a referential-identity test to lock the
contract.

The DFS+sweep approach left transitions in cycle members pointing at
orphan stub State instances rather than the final States exposed by
Process. Cycle detection missed it because both source and target had
non-empty transitions — they just pointed at the wrong objects.

Replaced with a two-phase construction: build all final States empty,
then create Transitions targeting those States and attach them via a
new internal-only State._initTransitions method. Identity holds by
construction for any graph topology (acyclic, cyclic, self-loop).

Added regression tests for cycles and self-loops.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

- Per-instance FIFO operation queue serializes concurrent triggerEvent and
  checkTransitions calls on the same Statemachine instance
- Two-phase observer lifecycle: BeforeTransitionObserver may veto, then
  Event-bound dispatcher commands run, then commit, then
  AfterTransitionObservers run with frozen TransitionFrame
- Errors from after-observers no longer roll back state; one error rethrown
  directly, multiple aggregated into AggregateError
- Constructor takes options object; getSelectedTransition / getCurrentContext
  removed from public surface
- Mutex acquireLock no longer short-circuits on isAcquired()
- Closes IMPLEMENTATION_REVIEW.md #1 (concurrent ops), #4 (transaction
  semantics)

…backObserver to v3

- TransitionLogger: reads from TransitionFrame; subject identity
  injection deferred to subclasses
- StatefulStatusChanger: takes the subject at construction
- CallbackObserver: documented as Event-only in v3

- Factory: uses v3 options-object Statemachine constructor; exposes
  attachBeforeObserver and attachAfterObserver
- Removed SetupHelper and StateCollectionMerger; both depended on
  user-mutable State/Transition which no longer exist in v3
- Migration paths documented in docs/migration/v2-to-v3.md (Phase 7)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Note: timing workaround (setTimeout(0)) used in OnEnterObserver tests
because triggerEvent's Promise currently resolves before chained ops
drain — this is a known src/Statemachine.ts bug to fix separately.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Also fix CallbackCondition constructor arg order in process-builder.test.ts
(signature is new CallbackCondition(name, callback), not callback-first).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…and #4

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

- Document chained-event timing explicitly in v2-to-v3 migration guide
  (await triggerEvent resolves before OnEnterObserver-enqueued chains drain;
  shows the setTimeout(0) yield pattern used in tests)
- Remove dead if-block in TransitionLogger that contained only a comment

Co-Authored-By: Claude Code <noreply@anthropic.com>

Closes the gap between prepublishOnly (lint+test) and the CI pack job
(build+pack): now an actual npm publish runs the full sequence —
build, lint, test, and tarball verification — so a stale or missing
dist cannot reach the registry through any path.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…ation

The previous example claimed an event-based transition outranks an
automatic transition during triggerEvent. In v3, automatic transitions
are not in the active set when an event is supplied, so the comparison
could not occur. Rewrite to compare event+condition vs event-only —
two transitions that genuinely appear together — and add a subsection
documenting the event vs automatic phase separation.

Closes IMPLEMENTATION_REVIEW.md #9.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…r example

The previous example used pg_advisory_lock (blocking, doesn't fit the
boolean acquireLock contract) and interpolated the lock name into the
SQL string (an injection footgun if copied). Rewrite to use
pg_try_advisory_lock with parameterized queries via db.query(sql, params).

Closes IMPLEMENTATION_REVIEW.md #11.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Working-tree-only spec and plan markdowns under docs/superpowers/
are gitignored-equivalent (untracked, never committed) but were
tripping pnpm format:check locally. Adding the directory to
.prettierignore keeps pnpm format:check clean without forcing the
superpowers tooling to format-pass before every commit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Previously runOperation called mutex.acquireLock() unconditionally
on every dequeued operation. Manual lock management with
autoreleaseLock: false (per docs/mutex.md:298-308) failed for
non-idempotent mutex implementations like pg_try_advisory_lock,
because the second acquire would throw or return false.

Now runOperation checks isAcquired() first, and tracks whether it
acquired in this call (acquiredHere) so it only releases what it
acquired. Manual acquire+triggerEvent+release composes correctly
with non-idempotent mutexes.

Replaces the prior "does not short-circuit when isAcquired returns
true" regression test, whose contradictory fake mutex (acquireLock
returning false while isAcquired returns true) encoded an invariant
that this fix intentionally inverts. The new test uses a realistic
non-idempotent mutex that throws on a redundant acquire.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…less of name

The previous validateNoConflictingDuplicates required BOTH different
identity AND different name to flag a conflict. Two CallbackCondition
objects sharing the name "shared" but encoding different logic were
therefore silently deduplicated — the original review #6 bug. The
regression test at tests/process-builder.test.ts:181 declared this
case but tested a different one (different names), so it never
exercised the bug.

Now: same condition reference dedups; different references conflict.
The buildAllStates dedup-key likewise uses object identity, not name.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Previously the event-bound dispatcher (which runs CallbackObserver
and similar commands attached to Event instances via event.attach())
was gated on currentState !== target, meaning self-transitions and
false-condition matched events both skipped the dispatcher entirely.
This contradicted docs/observers.md (which builds the OnEnterObserver
example around a self-transition firing a CallbackObserver) and broke
the simple "trigger event => command runs" mental model.

Move the dispatcher to the top of processOperation, where it fires
once per user-supplied event, before transition selection. Automatic
transitions in the iteration loop don't carry an event, so no
re-firing. State-machine before/after observers remain gated on real
state change.

The previously-passing test "should not execute observers on
self-transition" was inverted: event observers must fire on
self-transition. After-observers still don't fire (no state change).
Adds a regression test for the false-condition case.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The DuplicateTransitionError description previously said duplicates
were detected by "different condition objects with different names"
and that "same condition name" silently deduplicated. v3 dedups by
object reference identity, not by name: same reference dedups, any
two different references conflict (regardless of whether they share
a getName() value). Cross-link to the conditions guide for rationale.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Section 7 of the v2-to-v3 migration guide previously stated that v3
"always delegates to mutex.acquireLock()" with "no isAcquired()
short-circuit". That was the v3 design intent at one point but the
shipped v3 contract (per the review-fixes wave) is the opposite:
runOperation consults isAcquired() first and skips the redundant
acquireLock() when the mutex is already held.

Rewrite the section to describe what v3 actually does, including a
worked example of manual acquireLock/triggerEvent/releaseLock
composing correctly with non-idempotent mutex implementations like
pg_try_advisory_lock and redis SET NX. The "no short-circuit" claim
is gone.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…etBeforeObservers

The pre-existing veto-by-throw test exercised only the path where a
BeforeTransitionObserver throws and aborts the transition. Adds:

- Happy-path test: a single before-observer notify completes
  successfully, sees a frozen ProposedTransitionFrame, and the
  transition commits.
- Multi-observer ordering test: three before-observers attached in
  order, the second throws, the third never runs — confirms the
  loop short-circuits on the first throw and respects attach order.
- detachBefore / detachAfter: attach an observer, transition once,
  detach, transition again, assert the observer was not re-invoked.
  Also exercises detaching an observer that was never attached
  (must be a no-op).
- getBeforeObservers: empty initially, returns attached observers in
  attach order, reflects detachments.

Closes the Statemachine.ts coverage gap on lines 88-90, 93-94,
101-103, 280.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

… factory

Factory.ts had 55% function coverage before this change because five
public methods were never tested:

- attachBeforeObserver / detachBeforeObserver — analogous to the
  already-tested after-observer pair, asserts the observer is
  propagated to (or absent from) the statemachine returned by
  createStatemachine.
- setTransitionSelector — a custom ScoreTransition selector reaches
  the configured transition and the resulting statemachine
  transitions correctly under it.
- setMutexFactory — given a stateful subject, the factory invokes
  the mutex factory with the per-subject resource string, the
  resulting statemachine acquires and releases that resource through
  the configured adapter exactly once per operation. Includes the
  setMutexFactory(null) clearing path.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The two pre-existing strictOrphans tests both built graphs where the
initial state had no transitions, so the BFS reachability loop body
in validateOrphans (ProcessBuilder.ts:333-335) was never executed.

Adds two cases that exercise the traversal:

- "strictOrphans walks transitions and rejects states unreachable
  through them" — three reachable states (a -> b -> c) plus a fourth
  lonely state with no incoming transition. The BFS must walk from
  a to b to c, and the orphan check must correctly identify "lonely"
  as the only unreachable state.
- "strictOrphans accepts a fully reachable graph" — same a -> b -> c
  topology without the orphan, builds successfully.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

GraphBuilder.getTransitionLabel collects the names of observers
attached to a transition's event and renders them as "C: <names>"
inside the transition label. The pre-existing graph tests built
transitions whose events had no attached observers, so this whole
block (the for-loop over event.getObservers and the C: prefix
construction) was uncovered. The Named-detection helpers (isNamed,
convertToString) were also unreached because they're only used for
this rendering.

Adds three cases:

- Named observer (has getName()) → label contains "C: <name>".
- Multiple observers in attach order, comma-joined.
- Non-Named observer (only update()) → label uses the String(obj)
  fallback via toString().

Closes the GraphBuilder.ts coverage gap on lines 24-31, 51-56,
89-96.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The multi-observer test passed inline object literals with getName +
update properties to event.attach(), which type-checks the literal
strictly against Observer (which has only update). Pulling the
observers into named consts widens the literal type before the call
and lets TypeScript accept them. Test behavior unchanged.

A typecheck slipped through on the previous commit because the test
suite runs without TypeScript strict-mode for tests; pnpm lint catches
it via tsconfig.test.json.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Coverage analysis surfaced dead code on Dispatcher:

- isReady() — defined on the class and on DispatcherInterface, but
  has zero callers in src/ or tests/. Removed from both.
- onReadyCallback (3rd param of dispatch) plus the onReadyCallbacks
  field and the trailing invoke loop — the only call site
  (Statemachine.ts:233) passes two arguments, so the optional
  callback path was unreachable. Removed from both the class and
  the interface.

CallbackInterface stays exported because DispatcherInterface still
extends it (the parent only declares invoke(), which is what a
"callback" is). Dispatcher itself is internal (lives in src/internal/
and is not exported from the package entry point), so the runtime
class shape change is invisible to consumers.

Folds into v3.0.0 since the unshipped version is the only place this
shape was ever public — no users to migrate.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…ore/after API

Three documentation surfaces still referenced the v2 unified-observer
methods that were removed in the v3 rewrite:

- docs/factory.md "Methods" table listed attachStatemachineObserver,
  detachStatemachineObserver, getStatemachineObservers — none of
  which exist on the v3 Factory class. Replaced with the four v3
  methods: attach/detachBeforeObserver, attach/detachAfterObserver.
- docs/factory.md two example snippets called
  factory.attachStatemachineObserver(...) for StatefulStatusChanger,
  TransitionLogger, and OnEnterObserver — all three are
  AfterTransitionObserver implementations, so the calls become
  attachAfterObserver.
- docs/interfaces.md FactoryInterface snippet showed the same
  removed methods. Replaced with the actual interface shape from
  src/interfaces/FactoryInterface.ts (no get* method — Factory
  doesn't expose its observer collections).

Surfaced by GitHub Copilot's review of PR #17.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Codecov was running with default behavior (no config file present),
which enabled both project and patch status checks with auto targets
— meaning every PR diff was independently graded for coverage and
could fail CI on small dips even when overall coverage stayed healthy.

Add a codecov.yml that:

- Sets the project status target to a fixed 80% floor. As long as
  total coverage stays at or above 80%, the project check passes,
  regardless of whether a particular commit slightly reduces it.
  Current coverage sits at 98.62% statements, leaving plenty of
  headroom.
- Disables the patch (per-diff) status entirely. PRs that don't
  add tests for a small change won't fail Codecov on patch
  coverage; the project floor remains the single coverage gate.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>