Skip to content

actor: Add a supervision kernel for the durable actor runtime - #1123

Open
Roasbeef wants to merge 22 commits into
mainfrom
actor-supervision-kernel
Open

actor: Add a supervision kernel for the durable actor runtime#1123
Roasbeef wants to merge 22 commits into
mainfrom
actor-supervision-kernel

Conversation

@Roasbeef

@Roasbeef Roasbeef commented Aug 8, 2026

Copy link
Copy Markdown
Member

In this PR, we add a supervision kernel to the durable actor runtime, closing
the BEAM gap in its failure management. It is the fourth and last part of the
durable-actor hardening series, after #1119 and #1121.

The limping actor

Today a panic inside a durable behavior's Receive is recovered by the worker
loop and converted into a nack (see the existing recovery in
baselib/actor/durable_actor.go and TestDurableActorPanicRecovery). The
message is redelivered, and it is redelivered into the very same behavior
instance whose in-memory state the panic may have left half-mutated. Every
message after the panic then runs against state nobody can vouch for. The actor
does not die, which sounds like a virtue, but what it actually does is limp:
the runtime has an excellent story for a process that crashes (checkpoint,
restart message, dedup, lease expiry) and no story at all for a behavior that
corrupts itself in place.

Panic means restart

The worker loops now belong to a supervision goroutine that owns them a
generation at a time. Each generation runs under its own context derived from
the actor's lifetime context, so cancelling it drains every worker without
touching the mailbox. A recovered panic becomes a behaviorPanic, which is
what lets the runtime tell "the behavior returned an error" (an ordinary,
retryable message failure) apart from "the behavior panicked" (its state is
suspect). The panicking worker hands that error to supervision, which cancels
the generation, runs the behavior's OnStop hook if it has one (bounded by
CleanupTimeout), reloads the persisted FSM checkpoint, prepends a
RestartMessage at RestartPriority, and starts a fresh generation of the
configured worker count.

Two orderings inside that sequence are load-bearing.

The first is that the delivery's ack, nack, and dead-letter bookkeeping runs to
completion before the panic reaches supervision, so the message that
triggered the panic burns its attempt exactly as it does today. A deterministic
poison message therefore climbs to max_attempts and dead-letters rather than
restarting the actor forever, which is precisely the crash loop a naive
restart-on-panic would introduce.

The second is that on the classic path, where the whole Receive runs inside
one framework transaction, that bookkeeping is deliberately redone outside
the transaction. Supervision returns the panic from the transaction to force a
rollback of the behavior's own partial writes, because committing them
alongside the nack would persist exactly the torn state the restart exists to
escape, and the checkpoint reload would then hand it straight back.

The restart deliberately bypasses the Once-guarded Start and Stop. Those
guard the actor's public lifecycle, which a restart does not touch: the ID, the
DurableMailbox, and the cached Ref are the same objects afterwards, so
callers holding an ActorRef observe nothing beyond a pause in processing, and
senders keep enqueueing across the restart gap. In-flight Ask promises are
completed rather than dropped: the panicking turn's promise takes the panic
error, a sibling worker's turn sees its generation context cancelled and takes
that context error, and a message that had not yet reached the behavior is
redelivered afterwards with its promise still registered on the surviving
mailbox. DurableAsk responses travel through the outbox, so a restart only
delays them.

Ordering the hand-off under a worker pool

RestartPriority makes the claim query hand the restart message out first, and
that is where the guarantee used to stop: it orders the claims, not the
turns. With NumWorkers > 1 the supervisor launched every worker at once, so
one worker took the restart while a sibling immediately took the row behind it,
and a normal turn ran against a behavior that was still rebuilding itself from
the checkpoint. Latent today (the only pool adopter is the stateless
serverconn egress) and pre-existing on the boot path, but supervision makes
restart-under-pool routine, so a priority alone is no longer enough.

A generation now warms up one worker at a time. The supervisor launches a
single worker and the rest of the pool waits until that worker has resolved the
generation's restart hand-off. For a row supervision enqueued itself (every
supervised restart) the barrier waits for it unconditionally, so the guarantee
is exact rather than timing-dependent on the path this PR creates. The boot
hand-off is different in kind, since an owner prepends it before Start and
the actor never sees it; there the barrier orders the first claim and releases
on an idle tick, which orders the common case without claiming more than it can
know.

The release rule is shaped so the barrier cannot wedge a pool. A first claim of
anything other than a restart releases it before that message is processed,
so a generation with nothing to order pays nothing. A restore that fails is
dead-lettered and one that panics tears the generation down; both resolve the
hand-off. Whatever ends the warm-up worker releases it, including a closed
mailbox. And a Stop mid-barrier releases it through the generation context,
so shutdown never parks behind a restore. A single-worker actor is already
strictly sequential and gets no barrier at all.

What "restart" does not mean, and the adopter wiring

This is the part worth reading closely, because the name oversells it. The
framework does not rebuild the behavior. It stops the workers, optionally
calls OnStop, and redelivers a RestartMessage; the same Go value keeps
serving afterwards with whatever fields the panic left behind. The clean slate
exists exactly where the RestartMessage handler rebuilds in-memory state from
the durable row, and nowhere else.

That made the first cut of this PR vacuous in practice. All six production
construction sites are Read/Commit, and three of them (credit op, OOR session,
OOR registry) explicitly no-op'd the restart message on the reasoning that
"restore already ran at construction", which was true of the boot-time message
and false of a supervised one. The other two never registered RestartTLVType
at all. Worse, the panic path bypasses the commitFailed reload guards those
behaviors already carry, since those are armed only on commit-error paths, so a
panic after an in-memory FSM advance left the behavior ahead of the durable row
and the restart delivered a no-op.

So this PR wires the adopters:

  • credit op and OOR session arm their existing commitFailed guard, so
    the next turn reloads from the durable row before dispatching. Arming rather
    than reloading inline keeps the restart message free of IO, which matters
    because it is delivered with max_attempts 1.
  • OOR registry drops the staged handoff and re-runs the same non-terminal
    restore the boot path uses; that restore already skips resident sessions, so
    it is idempotent by construction.
  • unroll gains a sealed-surface adapter for RestartTLVType (its Msg
    interface is sealed, so the bare framework type would fail the mailbox cast
    and dead-letter) plus explicit handling that re-runs restoreCheckpoint. Its
    dispatch previously rejected unknown messages with a typed error, so a
    restart would have nack-looped.
  • serverconn egress gains the same adapter and a documented no-op. It keeps
    no per-turn state a checkpoint could restore: every turn reads its whole
    input from the durable message and consumes it in one Commit, and the
    in-memory state the connector holds belongs to the connection rather than to
    a turn. Arriving at "no restore needed" by accident is not the same as
    choosing it, so it says so.

Two obligations follow for handler authors, both documented. A restart message
is delivered once and dead-lettered rather than nacked when its turn fails
(a nacked row at attempts == max_attempts is neither leasable nor reapable,
so it would strand forever), which makes restore handlers responsible for their
own idempotency. And OnStop now runs mid-life, once per restart, so it must
be idempotent and must leave the behavior able to serve a new generation. A
panic escaping OnStop is recovered rather than allowed to take the process
down, since a restart calls it precisely when the behavior's invariants are
broken.

An actor whose codec still cannot carry a restart message degrades to cycling
its worker generation with no OnStop and no restore. That check happens
before the teardown, not after: a teardown the behavior cannot be rebuilt from
is strictly worse than leaving it running.

Restart intensity: default off

Restarts can be bounded by MaxRestarts inside RestartWindow, tracked in a
sliding window. It defaults off (DefaultMaxRestarts is
UnlimitedRestarts, and a zero value normalizes to it).

The first cut of this PR defaulted it on at five per sixty seconds, and the
arithmetic against real config is unkind: the default Tell retry policy gives a
message five attempts, each of which panics and restarts, so one poison Tell
burns the whole budget. Two poison messages in a minute would permanently kill
an actor that main would have dead-lettered twice and kept serving; the
serverconn egress sender would die that way while its heartbeat kept the
connection looking healthy.

The asymmetry decides it. Restarting forever is strictly no worse than the
nack-and-continue loop supervision replaces (both feed the same message to the
same behavior, both rate-limited by the nack backoff), while silent terminal
death is genuinely new and genuinely invisible: a terminated actor still holds
its ID and its mailbox rows. Nothing outside this package's tests calls Watch
yet, so a finite budget today is unobserved by construction. It is therefore
opt-in, documented as such on the config field, with RecommendedMaxRestarts
(the BEAM intensity) available for owners that do the wiring.

Supervision also keeps the mailbox tidy: each restart deletes the restart row
the previous one enqueued before writing its own, so at most one is pending at
a time rather than one per restart.

The Watch contract

(*DurableActor).Watch(ctx) gives another component a way to observe the
terminal event. It returns a channel that receives exactly one
TerminationInfo and is then closed, carrying the reason (stopped, context
cancelled, restart intensity exceeded, restart failed), the failure behind it,
the lifetime restart count, and whether the restart budget was exhausted.
Delivery is non-blocking by construction: each watcher gets a buffer-of-one
channel written exactly once, so a watcher that is slow or gone can never park
the actor's shutdown path, which is this repo's #1093 invariant. Registering
after termination returns a channel already loaded with the notification, and
cancelling the watching context releases the registration. Stop publishes for
an actor that was never started, so a watcher on one does not wait forever.
Nothing outside baselib/actor consumes it yet; adopters come later.

Testing

Tests live in baselib/actor/supervision_test.go, driven through the existing
mockDeliveryStore / mockTxAwareStore harness and the clock injection
pattern. The one that carries the most weight drives a Read/Commit behavior
shaped like the real adopters (in-memory mirror plus reload guard), has it
advance the mirror past the durable row and panic, and asserts the next turn
sees durable truth; it fails with the stale value if the restart handler
no-ops, which is what every Read/Commit adopter did before this PR.

The rest: panic restores from checkpoint, identity stays stable, a returned
error does not restart, a poison message dead-letters across restarts, a
restart racing Stop reports a graceful stop, a failed checkpoint load
terminates with TerminationRestartFailed, a panicking OnStop is recovered
and not run twice, two workers panicking in the same generation cost one unit
of budget, the intensity limit terminates with the right Watch reason,
Watch reports a graceful stop, eight unread watchers do not park shutdown, a
cancelled watcher deregisters, a watcher on a never-started actor still fires, a
four-worker pool has every worker drained by a restart, a four-worker pool runs zero normal turns while a restore is gated and drains its backlog once the gate opens, that barrier releases on a failed restore and on Stop, a pool with no hand-off reaches full width, a panicking turn's error reaches
ExecTx (which is what makes a real store roll back), restart rows neither
pile up nor strand, a Stoppable behavior with an unsupported codec is left
running, the sliding window and unlimited opt-out, the budget defaults, and the
termination reason strings.

go test -count=1 and -race pass for baselib/actor and for every adopter
touched (credit, oor, unroll, serverconn, ledger,
internal/actortest, also under -tags test_sqlite). Every commit on the
branch compiles independently. make fmt-changed-check,
make lint-changed-local (0 issues), sg scan on the changed non-test files
(no new warnings), and make commitmsg-lint are all green.

In this commit, we add a small read-only probe to MessageCodec that
reports whether a TLV type has a constructor registered. Encoding is
happy to serialize any TLVMessage, but decoding needs a registered
constructor, so today the only way to learn that a consumer cannot read
a message type is to enqueue the message and watch it dead-letter on the
far side.

The supervision kernel in the following commits needs exactly this
answer before it prepends a RestartMessage to an actor's mailbox: an
actor whose codec never registered the restart type would dead-letter
that message on every single restart, so we would rather skip the
enqueue and say so in a warning.
In this commit, we add the building blocks the durable actor runtime
needs before it can supervise a panicking behavior, with no wiring yet
so the mechanism can be reviewed on its own.

The first piece is behaviorPanic, the error a recovered panic is
converted into. This is what lets the runtime tell two failures apart
that look identical today: a behavior that returns an error is an
ordinary message failure, while a behavior that panicked left its
in-memory state half-mutated and cannot be trusted with the next
message. We keep the recovered value and the stack captured at the
recover site, and render the error exactly as the runtime rendered it
before ("panic: <value>") so the nack reasons, dead-letter rows, and log
strings a panicking behavior produces do not change.

The second piece is restartTracker, a BEAM-style intensity budget: at
most N restarts inside a sliding window, counted off an injected clock
so tests can drive the window without sleeping. A negative budget is the
explicit "restart forever" opt-out. The tracker keeps a lifetime total
separately from the windowed timestamps, and that total is atomic
because it is the one field anything outside the supervision goroutine
reads.

The last piece is watcherRegistry, which holds the termination watchers
registered against an actor. Each watcher gets a channel with a buffer
of one that is written exactly once and then closed, which is what makes
publishing a termination unconditionally non-blocking: no watcher,
however slow or absent, can park the actor's shutdown path. A watcher
that registers after the actor has already terminated is served straight
from the recorded notification rather than waiting forever, and a
watcher whose interest lapses can deregister and have its channel
closed.
In this commit, we close the BEAM gap in the durable actor runtime's
failure management. Until now a panic inside a behavior's Receive was
recovered by the worker loop and turned into a nack, which redelivered
the message into the very same behavior instance whose in-memory state
the panic may have left half-mutated. The actor kept limping instead of
restarting clean, and every message after the panic ran against state
nobody could vouch for.

The worker loops now belong to a supervision goroutine that owns them a
generation at a time. Each generation runs under its own context derived
from the actor's lifetime context, so cancelling it drains every worker
without touching the mailbox. When a behavior panics, the recovered
value reaches the worker as a behaviorPanic, the worker hands it to
supervision, and supervision cancels the generation, runs the behavior's
OnStop hook if it has one (bounded by CleanupTimeout), reloads the
persisted FSM checkpoint, prepends a RestartMessage at RestartPriority,
and starts a fresh generation of the configured worker count. That is
the same pair of startup steps an owner performs when it boots the actor
for the first time, so the restart needs nothing from the behavior
beyond its existing RestartMessage handling.

The ordering inside that sequence is load-bearing. The delivery's normal
ack, nack, and dead-letter bookkeeping runs to completion before the
panic is handed to supervision, so the message that triggered the panic
burns its attempt exactly as it does today. A deterministic poison
message therefore climbs to max_attempts and dead-letters instead of
restarting the actor forever, which is precisely the crash loop a naive
restart-on-panic would introduce.

The restart deliberately bypasses the Once-guarded Start and Stop. Those
guard the actor's public lifecycle, which a restart does not touch: the
ID, the DurableMailbox, and the cached Ref are the same objects
afterwards, so callers holding an ActorRef observe nothing beyond a
pause in processing, and senders keep enqueueing across the restart gap.
In-flight Ask promises are completed rather than dropped: the panicking
turn's promise takes the panic error, a sibling worker's turn sees its
generation context cancelled and takes that context error, and a message
that had not yet reached the behavior is simply redelivered afterwards
with its promise still registered on the surviving mailbox. DurableAsk
responses travel through the outbox, so a restart only delays them.

Restarts are bounded by MaxRestarts inside RestartWindow, tracked in a
sliding window off the config's injected clock. We default the budget on
at five restarts per sixty seconds rather than leaving it unlimited, and
we normalize a zero MaxRestarts to that default rather than to
"unlimited", so a hand-built config cannot end up with an unbounded
crash loop by forgetting a field. The reasoning is that restart-on-panic
is itself new behavior, so there is no prior semantics to preserve by
defaulting the bound off, and an unbounded default would trade today's
quiet corruption for a loud livelock. UnlimitedRestarts stays available
as a deliberate opt-out. Breaking the budget is terminal: we log at
error level (a panic is an internal bug, which the log-level rule
allows), cancel the lifetime context so further sends fail fast instead
of piling into a mailbox nothing will drain, tear down, and publish the
termination.

Finally, Watch gives other components a way to observe that terminal
event. It returns a channel that receives exactly one TerminationInfo
and is then closed, carrying the reason, the failure behind it, the
lifetime restart count, and whether the restart budget was exhausted.
Registering after the actor has terminated returns a channel already
loaded with the notification, so a watcher cannot lose the race against
a stopping actor, and cancelling the watching context releases the
registration. Nothing outside this package consumes it yet.

One rough edge is worth naming: an actor whose codec never registered
the RestartMessage cannot decode one, so we skip the checkpoint hand-off
with a warning rather than enqueue a message that would dead-letter on
every restart.
In this commit, we cover the supervision kernel against the existing
mockDeliveryStore and mockTxAwareStore harness. The behaviors under test
are typed over the generic TLVMessage rather than a concrete message
struct, because a supervised restart delivers the framework's own
RestartMessage and a behavior narrowed to one concrete type could never
receive it.

The restart path is covered from both ends: a panicking behavior is
handed back its persisted checkpoint through a RestartMessage, its
OnStop hook runs before the rebuild, and its ID, mailbox, and Ref are
the same objects on the far side. We also pin the negative case, since
supervision must not fire on every failure: a behavior that returns an
error retries as before and never restarts the actor.

The poison-message test is the one that pins the nack-before-restart
ordering. A message that panics on every delivery burns an attempt on
each pass, so it reaches max_attempts and dead-letters after three
restarts while the actor stays alive and keeps serving traffic, which is
the crash loop we would otherwise have introduced.

For the intensity budget, an actor with a budget of two restarts and a
behavior that always panics terminates on the third, and its watcher
observes the right reason, the exhausted flag, the restart count, and
the panic itself. The terminated actor also refuses further sends rather
than accumulating a backlog nothing will drain.

The Watch contract gets three tests: a graceful Stop reports itself as
such with no restarts, eight watchers that never read do not park the
shutdown path and each still receives exactly one notification followed
by a closed channel, and a watcher whose context is cancelled has its
registration released.

The multi-worker test drives a four-worker pool on the Read/Commit path.
Three workers park inside their turns, the fourth panics, and every
parked turn observes cancellation, which is the evidence that a restart
drains the whole pool rather than only the worker that failed.

The remaining tests are narrower: the restart tracker's sliding window
and unlimited opt-out against a test clock, the budget defaults landing
on the bounded value from both the default config and a bare hand-built
one, the codec-without-RestartMessage path restarting without polluting
the dead letter table, and the termination reason strings.
In this commit, we fold the supervision kernel into the package
knowledge graph. The actor package's CLAUDE.md and AGENTS.md gain the
new config knobs, the Watch surface, and the termination types under key
types, plus four invariants: that a panic means restart rather than
redeliver, that a restart preserves the actor's public identity, what
happens to in-flight Ask promises across one, and that exceeding the
restart budget is terminal.

The durable actor architecture doc gains a section under recovery and
restart explaining the same mechanism in prose, since the recovery flow
documented there previously covered only process crashes and said
nothing about the in-process equivalent.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c16ebef19f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread baselib/actor/durable_actor.go Outdated
Comment on lines +631 to +634
for i := 0; i < a.numWorkers; i++ {
workers.Add(1)

go a.worker(&workers)
go a.worker(runCtx, &workers)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Serialize restoration before launching the worker pool

When NumWorkers > 1 and ordinary messages remain queued, launching every worker together allows one worker to lease the RestartMessage while another immediately leases the next row; LeaseNextMailboxMessage excludes the already-leased restart row and orders only the remaining eligible rows. The second behavior call can therefore run concurrently with checkpoint restoration and observe stale or partially restored state, despite the recovery contract requiring the restart message to run first. Process the restart message behind a barrier before starting the full pool.

AGENTS.md reference: baselib/actor/AGENTS.md:L112-L112

Useful? React with 👍 / 👎.

Comment thread baselib/actor/durable_actor.go Outdated
Comment on lines +815 to +822
if !a.codec.Supports(RestartTLVType) {
logger(a.ctx).WarnS(a.ctx, "Restarting durable actor without "+
"checkpoint restore: codec has no RestartMessage",
nil,
"actor_id", a.id,
)

return nil

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Terminate when a stopped behavior cannot be restored

For a classic Stoppable actor whose codec omits RestartTLVType, supervision has already invoked OnStop before reaching this branch, but returning nil starts a new generation with the same behavior object and no restoration message. If OnStop released a database handle, listener, subscription, or other behavior-owned resource, subsequent messages now run against a torn-down instance with no hook capable of rebuilding it. Treat the unsupported restore as terminal, or avoid tearing down the behavior when no restart handoff is possible.

AGENTS.md reference: baselib/actor/AGENTS.md:L100-L100

Useful? React with 👍 / 👎.

@litbot-9000

litbot-9000 commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

This PR's supervision-kernel changes landed new restart-handling behavior in credit, oor, serverconn, and unroll whose per-package CLAUDE.md/AGENTS.md invariants were not updated alongside the code (baselib/actor's docs were already updated in this PR and need no further change).

diff --git a/credit/AGENTS.md b/credit/AGENTS.md
index 13ce4f8a..449455fd 100644
--- a/credit/AGENTS.md
+++ b/credit/AGENTS.md
@@ -52,6 +52,19 @@ credit redemptions against the swap-server credit ledger, as a crash-safe
 - Every external call the behavior makes (`CreateCredit`, `SendOOR`,
   `StartPay`, `RedeemCredit`) must stay idempotent by op key or payment hash,
   since a redelivered message or a reload-after-`commitFailed` re-runs it.
+- A supervised restart is not a no-op for `opBehavior`. The framework reuses
+  the same behavior instance across a restart, so a panic that left `rec`
+  advanced past the last durable checkpoint would otherwise survive it: the
+  `actor.RestartMessage` case arms the same `commitFailed` reload guard the
+  failed-commit path uses, and the next driving turn rebuilds `rec` from the
+  durable row before it dispatches. The reload is deferred to that turn rather
+  than run on the restart turn itself because the framework delivers the
+  restart message with `MaxAttempts` 1 and dead-letters (rather than retries) a
+  restart turn that fails.
+- `CreditDurableMsg` is unsealed (`actor.TLVMessage` alone), so the codec
+  registers the framework's `actor.RestartMessage` directly under
+  `actor.RestartTLVType`; no package-local adapter type is needed for the
+  decoded message to satisfy the mailbox's cast.
 - Auto-redeem is receive-triggered, not a periodic sweep (except a single
   boot-time reconcile); `triggerRedeem` fires only after the settled receive's
   terminal snapshot commits, so a crash before that leaves no half-applied
diff --git a/credit/CLAUDE.md b/credit/CLAUDE.md
index 13ce4f8a..449455fd 100644
--- a/credit/CLAUDE.md
+++ b/credit/CLAUDE.md
@@ -52,6 +52,19 @@ credit redemptions against the swap-server credit ledger, as a crash-safe
 - Every external call the behavior makes (`CreateCredit`, `SendOOR`,
   `StartPay`, `RedeemCredit`) must stay idempotent by op key or payment hash,
   since a redelivered message or a reload-after-`commitFailed` re-runs it.
+- A supervised restart is not a no-op for `opBehavior`. The framework reuses
+  the same behavior instance across a restart, so a panic that left `rec`
+  advanced past the last durable checkpoint would otherwise survive it: the
+  `actor.RestartMessage` case arms the same `commitFailed` reload guard the
+  failed-commit path uses, and the next driving turn rebuilds `rec` from the
+  durable row before it dispatches. The reload is deferred to that turn rather
+  than run on the restart turn itself because the framework delivers the
+  restart message with `MaxAttempts` 1 and dead-letters (rather than retries) a
+  restart turn that fails.
+- `CreditDurableMsg` is unsealed (`actor.TLVMessage` alone), so the codec
+  registers the framework's `actor.RestartMessage` directly under
+  `actor.RestartTLVType`; no package-local adapter type is needed for the
+  decoded message to satisfy the mailbox's cast.
 - Auto-redeem is receive-triggered, not a periodic sweep (except a single
   boot-time reconcile); `triggerRedeem` fires only after the settled receive's
   terminal snapshot commits, so a crash before that leaves no half-applied
diff --git a/oor/AGENTS.md b/oor/AGENTS.md
index bb983c14..158ce181 100644
--- a/oor/AGENTS.md
+++ b/oor/AGENTS.md
@@ -122,6 +122,23 @@ For field-level detail, use `go doc github.com/lightninglabs/wavelength/oor.<Sym
 - Server-side lineage-cap rejection surfaces as a typed `*ErrLineageTooLarge`
   via `ClassifySubmitError`, so wallet callers can switch on the cause
   without depending on the `oorpb` proto type.
+- A supervised restart is not a no-op for either behavior. The framework reuses
+  the same behavior instance across a restart, so whatever the panic left in
+  goroutine-owned memory outlives it unless the `actor.RestartMessage` handler
+  rebuilds from durable truth. `sessionBehavior` arms the same `commitFailed`
+  reload guard its failed-commit path uses, so the next turn stops the stale
+  FSM and rebuilds it from the registry row before dispatching; the rebuild is
+  deferred to that turn because `restore()` starts an FSM goroutine that must
+  outlive the turn, and because the framework delivers the restart message with
+  `MaxAttempts` 1 and dead-letters (rather than retries) a restart turn that
+  fails. `oorRegistryBehavior` instead restores inline: it drops the staged
+  `pendingHandoff` (it belonged to the turn that died) and re-runs
+  `handleRestoreNonTerminal`, which is cheap and idempotent because it skips
+  sessions that are already resident.
+- `OORDurableMsg` is unsealed (`actor.TLVMessage` alone), so the shared codec
+  registers the framework's `actor.RestartMessage` directly under
+  `actor.RestartTLVType`; no package-local adapter type is needed for the
+  decoded message to satisfy the mailbox's cast.
 
 ## Deep Docs
 
diff --git a/oor/CLAUDE.md b/oor/CLAUDE.md
index bb983c14..158ce181 100644
--- a/oor/CLAUDE.md
+++ b/oor/CLAUDE.md
@@ -122,6 +122,23 @@ For field-level detail, use `go doc github.com/lightninglabs/wavelength/oor.<Sym
 - Server-side lineage-cap rejection surfaces as a typed `*ErrLineageTooLarge`
   via `ClassifySubmitError`, so wallet callers can switch on the cause
   without depending on the `oorpb` proto type.
+- A supervised restart is not a no-op for either behavior. The framework reuses
+  the same behavior instance across a restart, so whatever the panic left in
+  goroutine-owned memory outlives it unless the `actor.RestartMessage` handler
+  rebuilds from durable truth. `sessionBehavior` arms the same `commitFailed`
+  reload guard its failed-commit path uses, so the next turn stops the stale
+  FSM and rebuilds it from the registry row before dispatching; the rebuild is
+  deferred to that turn because `restore()` starts an FSM goroutine that must
+  outlive the turn, and because the framework delivers the restart message with
+  `MaxAttempts` 1 and dead-letters (rather than retries) a restart turn that
+  fails. `oorRegistryBehavior` instead restores inline: it drops the staged
+  `pendingHandoff` (it belonged to the turn that died) and re-runs
+  `handleRestoreNonTerminal`, which is cheap and idempotent because it skips
+  sessions that are already resident.
+- `OORDurableMsg` is unsealed (`actor.TLVMessage` alone), so the shared codec
+  registers the framework's `actor.RestartMessage` directly under
+  `actor.RestartTLVType`; no package-local adapter type is needed for the
+  decoded message to satisfy the mailbox's cast.
 
 ## Deep Docs
 
diff --git a/serverconn/AGENTS.md b/serverconn/AGENTS.md
index a88da6d0..22e40275 100644
--- a/serverconn/AGENTS.md
+++ b/serverconn/AGENTS.md
@@ -100,6 +100,21 @@ background ingress polling with event routing.
   (`validateInboundEnvelope`); a mismatch is always a permanent
   `*mailboxconn.StatusError` — there is no legacy zero-version fallback,
   since client and operator are always deployed with a negotiated version.
+- A supervised restart of the egress actor has nothing to rebuild, and
+  `ServerConnectionActor.Receive` consumes the restart message as an explicit
+  no-op to say so. Every egress turn reads its whole input from the durable
+  message, converts it, sends it over the edge, and consumes the message in one
+  Commit; nothing carries over between turns. The in-memory state the connector
+  does hold belongs to the CONNECTION, not to a turn (the unary response
+  registry, the last-send timestamp, the cached incompatibility, the ingress
+  cancel), and it is owned by the ingress loop and its callers, so a panicking
+  egress turn cannot leave any of it half-written.
+- `ServerConnMsg` is sealed, so `NewServerConnCodec` registers a package-local
+  `restartMsg` adapter (embedding `actor.RestartMessage`) under
+  `actor.RestartTLVType` rather than the bare framework type. The durable
+  mailbox casts every decoded message to `ServerConnMsg`; an unadapted restart
+  would fail that cast and dead-letter instead of being handled. Any future
+  sealed message surface that adopts supervision needs the same adapter.
 
 ## Deep Docs
 
diff --git a/serverconn/CLAUDE.md b/serverconn/CLAUDE.md
index a88da6d0..22e40275 100644
--- a/serverconn/CLAUDE.md
+++ b/serverconn/CLAUDE.md
@@ -100,6 +100,21 @@ background ingress polling with event routing.
   (`validateInboundEnvelope`); a mismatch is always a permanent
   `*mailboxconn.StatusError` — there is no legacy zero-version fallback,
   since client and operator are always deployed with a negotiated version.
+- A supervised restart of the egress actor has nothing to rebuild, and
+  `ServerConnectionActor.Receive` consumes the restart message as an explicit
+  no-op to say so. Every egress turn reads its whole input from the durable
+  message, converts it, sends it over the edge, and consumes the message in one
+  Commit; nothing carries over between turns. The in-memory state the connector
+  does hold belongs to the CONNECTION, not to a turn (the unary response
+  registry, the last-send timestamp, the cached incompatibility, the ingress
+  cancel), and it is owned by the ingress loop and its callers, so a panicking
+  egress turn cannot leave any of it half-written.
+- `ServerConnMsg` is sealed, so `NewServerConnCodec` registers a package-local
+  `restartMsg` adapter (embedding `actor.RestartMessage`) under
+  `actor.RestartTLVType` rather than the bare framework type. The durable
+  mailbox casts every decoded message to `ServerConnMsg`; an unadapted restart
+  would fail that cast and dead-letter instead of being handled. Any future
+  sealed message surface that adopts supervision needs the same adapter.
 
 ## Deep Docs
 
diff --git a/unroll/AGENTS.md b/unroll/AGENTS.md
index f7ed9727..e192e2ac 100644
--- a/unroll/AGENTS.md
+++ b/unroll/AGENTS.md
@@ -305,6 +305,22 @@ For field-level detail, use `go doc github.com/lightninglabs/wavelength/unroll.<
   operator-sourced OOR artifacts flow into proof assembly, so a
   zero- or short-output node maps to a retryable error rather than
   a goroutine panic.
+- **A supervised restart restores the checkpoint inline.** The
+  framework reuses the same `behavior` instance across a restart, so
+  a panic that advanced `b.pending` or `b.sweepTx` past the last
+  `Staged` checkpoint would otherwise survive it. The `restartMsg`
+  case re-runs the constructor's `restoreCheckpoint`, overwriting
+  both from the durable row. It drives no FSM transition and writes
+  nothing, so it returns before the Commit like the status probe.
+  The restore gets exactly one shot: the framework delivers the
+  restart message with `MaxAttempts` 1 and dead-letters (rather than
+  retries) a restart turn that fails.
+- **The restart message needs a sealed adapter.** `Msg` is sealed, so
+  `newCodec` registers a package-local `restartMsg` (embedding
+  `actor.RestartMessage`) under `actor.RestartTLVType` rather than
+  the bare framework type. The durable mailbox casts every decoded
+  message to `Msg`; an unadapted restart would fail that cast and
+  dead-letter instead of restoring.
 
 ## Deep Docs
 
diff --git a/unroll/CLAUDE.md b/unroll/CLAUDE.md
index f7ed9727..e192e2ac 100644
--- a/unroll/CLAUDE.md
+++ b/unroll/CLAUDE.md
@@ -305,6 +305,22 @@ For field-level detail, use `go doc github.com/lightninglabs/wavelength/unroll.<
   operator-sourced OOR artifacts flow into proof assembly, so a
   zero- or short-output node maps to a retryable error rather than
   a goroutine panic.
+- **A supervised restart restores the checkpoint inline.** The
+  framework reuses the same `behavior` instance across a restart, so
+  a panic that advanced `b.pending` or `b.sweepTx` past the last
+  `Staged` checkpoint would otherwise survive it. The `restartMsg`
+  case re-runs the constructor's `restoreCheckpoint`, overwriting
+  both from the durable row. It drives no FSM transition and writes
+  nothing, so it returns before the Commit like the status probe.
+  The restore gets exactly one shot: the framework delivers the
+  restart message with `MaxAttempts` 1 and dead-letters (rather than
+  retries) a restart turn that fails.
+- **The restart message needs a sealed adapter.** `Msg` is sealed, so
+  `newCodec` registers a package-local `restartMsg` (embedding
+  `actor.RestartMessage`) under `actor.RestartTLVType` rather than
+  the bare framework type. The durable mailbox casts every decoded
+  message to `Msg`; an unadapted restart would fail that cast and
+  dead-letter instead of restoring.
 
 ## Deep Docs

How to apply: save the diff above and git apply it, or run the doc-gardening skill locally (/doc-gardening scoped to credit oor serverconn unroll) to regenerate the same updates. Each CLAUDE.md must stay byte-identical to its sibling AGENTS.md (make doc-check enforces the pair).


Posted by the per-PR doc-gardening advisory. Advisory only — this does not block merge. Run log: https://github.com/lightninglabs/wavelength/pull/1123/checks

Roasbeef added 13 commits August 7, 2026 18:15
In this commit, we split PrependRestartMessage so the enqueued row ID
comes back to the caller, and we document the two constraints the
supervision kernel needs from that row.

The ID matters because the kernel prepends one restart message per
restart, forever, if an actor keeps panicking. Handing the ID back lets
it delete the row it wrote last time before writing the next, so a run
of restarts leaves at most one restart row in the mailbox rather than
one per restart. The next commit wires that up.

The documentation matters because the row carries max_attempts 1, and
that number has a sharp edge nobody had cause to notice while restart
messages were only ever enqueued once at boot. Nacking such a row leaves
it at attempts == max_attempts, which the claim query will not lease
again and which nothing will ever dead-letter either: it simply strands
in the mailbox forever. The runtime therefore has to treat a failed
restart turn as terminal rather than retryable, which in turn makes
restore handlers responsible for their own idempotency. Saying so on the
constructor is the only place a handler author will look.
In this commit, we correct the Stoppable documentation, which promised
more than the durable runtime is about to deliver. It said OnStop runs
"during actor shutdown, after the message processing loop exits but
before the actor's goroutine terminates", which reads as a once-per-
lifetime, the-behavior-is-being-thrown-away hook. Supervised restarts
break both halves of that reading: the hook now runs mid-life, once per
restart, against a behavior instance that keeps serving afterwards.

Two obligations follow for implementations, and neither is obvious from
the old wording. The hook must be idempotent, because a restart that
runs it and then fails to carry itself out falls through to the terminal
teardown. And it must leave the behavior able to serve a new generation
rather than assuming it is done, since the restart hands the same
instance a RestartMessage and carries on. We also record that a panic
escaping the hook is recovered rather than allowed to take the process
down, which matters because a restart calls OnStop precisely when the
behavior's invariants are known to be broken.
In this commit, we stop the classic transactional path from committing a
panicking turn's own writes.

That path wraps the WHOLE of Receive in a single framework transaction,
so a behavior that panics half way through leaves its partial writes
sitting in that transaction, and the framework then adds the nack and
commits the lot. The result is the exact failure the supervision kernel
exists to escape: torn state persisted durably, ready for the checkpoint
reload to hand straight back to the restarted behavior. Rolling the
behavior forward from a checkpoint is only worth anything if the
checkpoint is not itself torn.

We now return the panic from inside the transaction closure, which
forces the rollback, and redo the message's ack, nack, and dead-letter
bookkeeping afterwards through finishNonTx. That keeps the property the
kernel depends on, namely that the poison message burns its attempt and
eventually dead-letters, while discarding everything the panicking
behavior wrote. The promise is no longer deferred on this path because
there is no commit left to wait for: the result is an error either way,
so the caller can have it immediately.

While we are here we give the pre-existing transaction-failure nack the
same treatment finishNonTx already gives its writes, running it on a
detached and bounded context. A nack is a durable write, and running it
on the turn's own cancellable context means a Stop landing mid-failure
loses it, which leaves the message leased until its lease expires
instead of retryable now.
In this commit, we reverse the default we shipped for the restart
intensity budget. DefaultMaxRestarts becomes UnlimitedRestarts, a zero
MaxRestarts normalizes to that, and a finite budget becomes something an
owner opts into.

The budget as we first defaulted it (five restarts per sixty seconds)
was a silent permanent kill switch, and the arithmetic against real
config is worse than it looks. The default Tell retry policy gives a
message five attempts, and under supervision each of those attempts
panics and restarts the actor, so ONE poison Tell burns the entire
budget on its own. Two poison messages inside a minute would kill an
actor permanently where the runtime we are replacing would have
dead-lettered both and carried on serving. The serverconn egress sender
would die that way while its heartbeat kept the connection looking
healthy.

The asymmetry is what decides it. Restarting forever is strictly no
worse than the nack-and-continue loop supervision replaces: both feed
the same message back to the same behavior, and both are rate-limited by
the nack backoff, so the failure mode is one we already ship. Silent
terminal death is genuinely new, and it is invisible: a terminated actor
still holds its ID and its mailbox rows, so nothing notices unless
someone is watching. Nothing outside this package's tests calls Watch
yet, which means a finite budget today is unobserved by construction.

A finite budget is therefore only a safe trade where the owner wires
Watch and reacts to TerminationRestartIntensityExceeded, and that is a
property of the owner, not of the framework, so it has to be chosen
rather than inherited. We say exactly that on the config field, and we
keep the BEAM intensity available as RecommendedMaxRestarts for owners
that do the wiring.
In this commit, we stop the runtime from nacking a restart message whose
turn failed, and send it to the dead letter queue instead.

A restart message is enqueued with max_attempts 1 because it must be
delivered exactly once. Nacking such a row leaves it at attempts ==
max_attempts, which is a state nothing recovers from: the claim query
will not lease it again, so it is never redelivered, and nothing ever
walks it to the dead letter table either, so it simply sits in the
mailbox forever. That was harmless while restart messages were only
enqueued once at boot by an owner that would notice, and it stops being
harmless now that the supervision kernel enqueues one per restart. A
behavior whose restore keeps failing under an unlimited restart budget
would accumulate one stranded row per restart, without bound.

Dead-lettering is both terminal and visible, which is what we want from
a control message that cannot be retried. The cost is that a restore
handler gets exactly one attempt per restart, so it has to be idempotent
rather than leaning on redelivery, which is the obligation the previous
commit wrote down on the constructor.
In this commit, we fix three problems with the restart sequence itself,
all of which live in the same handful of functions.

The first is ordering. We were checking whether the codec can carry a
RestartMessage only after the generation had been torn down and the
behavior's OnStop had run. An actor that cannot be handed its checkpoint
therefore paid a mid-life teardown, kept the same behavior instance, and
got no state rebuild in return, which is strictly worse than the
nack-and-continue it replaced. We now ask the question first, and when
the answer is no we degrade to cycling the worker generation and leave
the behavior alone. The restart budget is still spent either way, so a
finite intensity still bounds the degraded path.

The second is that OnStop ran bare in the supervision goroutine. A
restart calls it precisely when the behavior's invariants are known to
be broken, so a cleanup that trips over the same corrupt state is a
realistic outcome rather than a theoretical one, and it would have taken
the whole process down. It now runs under recover, and a panicking
cleanup terminates the actor as a failed restart. The hook is also
idempotent per behavior generation now, because a restart that ran it
and then failed used to fall through to the terminal teardown and stop
the behavior a second time for one teardown.

The third is that restart rows accumulated. Each restart enqueued a
fresh row with a fresh UUID and nothing removed the previous one, so an
actor that panicked faster than it drained its mailbox grew the set,
without bound under an unlimited budget. Supervision now deletes the row
it wrote last time before writing the next, which holds the mailbox to
at most one pending restart row. A row that was already consumed makes
the delete a no-op, and a delete that fails is not worth failing the
restart over: the worst case is the extra row we were trying to avoid.
In this commit, we close two gaps in the Watch contract that only show
up on an actor that is stopped without ever having been started.

Such an actor has no supervision loop, so nothing publishes its
termination and a watcher registered against it waits forever for a
notification nobody will send. Worse, the watcher's own cleanup
goroutine parked on the actor's done channel, which that actor never
closes, so a watcher holding a background context leaked the goroutine
outright. Stop now publishes the termination itself when the actor was
never started, and the cleanup goroutine parks on the watcher registry's
own publish signal rather than on the actor's done channel, so it
retires either way. A Start racing that publish loses harmlessly:
publishing is first-wins and the loop it launches exits immediately
against the already-cancelled lifetime context.

We also write down two things about terminationInfo that are easier to
find in a comment than to rediscover from the code.
TerminationContextCancelled is currently unreachable, because the
lifetime context is rooted at context.Background and Stop is the only
thing that cancels it; it exists for the construction path that takes an
externally owned context and would otherwise have no way to report
itself. And the stopRequested read there is not ordered against a
concurrent Stop, so a Stop landing in the same instant as an unrelated
exit can be reported either way. Both readings describe the same event,
namely that the actor was shut down rather than that it failed, so the
ambiguity costs a watcher nothing and is not worth a lock.
In this commit, we make the credit operation behavior actually restore
itself when the actor framework restarts it.

The handler treated a RestartMessage as a no-op on the reasoning that
"restore already ran at construction". That reasoning held while the
only restart message an operation ever saw was the one its owner
prepended at boot, immediately after construction had already restored.
It stops holding now that the supervision kernel restarts a panicking
actor, because a restart message then reaches a LIVE behavior: the
framework stops the workers and redelivers the message, but the same Go
value keeps serving afterwards, carrying whatever the panic left in it.

A panic mid-dispatch can leave rec advanced in memory past the last
durable checkpoint, which is exactly the divergence the failed-commit
path already guards against. So we arm the same guard rather than invent
a second one: the next turn reloads rec from the durable row before it
dispatches, so the redelivered event re-applies against last-committed
state instead of a stale in-memory advance.

We arm the guard rather than reload inline so the restart message stays
a pure control message with no IO of its own. That matters because the
framework delivers it with max_attempts 1 and dead-letters, rather than
retries, a restart turn that fails.
In this commit, we make both OOR behaviors restore themselves when the
actor framework restarts them, rather than consuming the restart message
as a no-op on the grounds that restore already ran at construction. That
reasoning covered the boot-time restart message; it does not cover the
one the supervision kernel sends after a panic, because that message
reaches a live behavior and the same instance keeps serving afterwards
with whatever the panic left in it.

For the session behavior, a panic mid-dispatch can leave b.fsm advanced
past the last durably-committed snapshot, which is the same divergence
the failed-commit path already handles. We arm that guard, so the next
turn stops the stale FSM and rebuilds it from the registry row before
dispatching. Deferring the rebuild to that turn rather than doing it
inline matters twice over: restore starts an FSM goroutine that has to
outlive the turn it was created in, and the framework dead-letters
rather than retries a restart turn that fails.

For the registry, the goroutine-owned routing state is the active child
set and any staged handoff. We drop the handoff, which belonged to the
turn that died, and re-run the same non-terminal restore the boot path
uses, so the active set is reconciled against the durable rows again.
That restore skips sessions that are already resident, so running it
against a mostly-intact active set is cheap and idempotent, which is
what makes it safe to call here as well as at boot.
In this commit, we teach the VTXO unroll actor to handle the restart
message the supervision kernel sends after a panic. It could not before,
in two separate ways.

The codec never registered the framework's RestartTLVType, so a restart
message would have arrived undecodable. Registering the framework type
directly is not enough either: this package's Msg surface is sealed with
an unexported marker, and the durable mailbox casts every decoded
message to Msg, so the bare framework type would fail that cast and be
dead-lettered as a type mismatch. We register a small adapter instead,
which embeds the framework message and adds nothing but the seal, so the
encoding, decoding, TLV type, and restart priority all stay the
framework's. That keeps the sealed surface intact rather than opening it
up for one message.

Getting the message through is only half of it. The dispatch switch
rejects anything it does not recognise with a typed error, which for a
restart message would mean a failing turn on every restart. And the
handler has real work to do: a panic can leave b.pending or b.sweepTx
advanced in memory past the last Staged checkpoint, and the framework
reuses the same behavior instance across the restart, so nothing else
would undo that. We re-run the same restoreCheckpoint the constructor
runs, which overwrites both from the durable row. It drives no FSM
transition and writes nothing, so it returns before the Commit exactly
as the read-only status probe above it does.
In this commit, we let the egress sender receive the restart message the
supervision kernel sends after a panic, and handle it as a documented
no-op rather than leaving it undecodable.

The mechanics match the unroll actor: the codec never registered the
framework's RestartTLVType, and registering the framework type directly
would not work either, because ServerConnMsg is sealed with an
unexported marker and the durable mailbox casts every decoded message to
it. We register an adapter that embeds the framework message and adds
nothing but the seal.

The handler is where this actor differs from the others, and the reason
is worth stating rather than leaving as an apparent oversight. The
egress sender keeps no per-turn state a checkpoint could restore: each
turn reads its whole input from the durable message, converts it, sends
it over the edge, and consumes the message in one Commit, with nothing
carried between turns. The in-memory state the connector does hold
belongs to the CONNECTION rather than to a turn (the unary response
registry, the last-send timestamp, the cached incompatibility, the
ingress cancel), and it is owned by the ingress loop and its callers, so
a panicking egress turn cannot leave any of it half-written. Consuming
the restart as a no-op is therefore the whole restore.

Without the registration the actor would have taken the framework's
degraded path, which cycles the worker generation with no restore at
all. That happens to be the same outcome here, but arriving at it by
accident is not the same as choosing it, and the next reader of this
handler deserves to know which one it is.
In this commit, we add the tests for the rework, each one pinned to a
failure the review found rather than to a line of code.

The one that carries the most weight is the divergence test. It drives a
Read/Commit behavior shaped like the real adopters, with an in-memory
mirror of a durable row and the reload guard they all carry, and has it
advance the mirror past the row and then panic. The next turn must see
durable truth. That test fails with the value the panic left behind if
the restart handler treats the message as a no-op, which is what every
Read/Commit adopter did before this rework, so it is the one that would
have caught the finding in the first place.

The rest cover the sharp edges around it. A restart racing Stop reports
a graceful stop rather than a supervision failure. A checkpoint load
that fails terminates with TerminationRestartFailed. An OnStop that
panics is recovered, terminates the actor, and is not then run a second
time by the terminal teardown. Two workers panicking inside the SAME
generation cost one unit of restart budget rather than one each, which
is the regression that would let a pool burn a finite budget N times
faster than a single-worker actor for the same fault. A classic
Stoppable behavior whose codec cannot carry a restart message is left
running rather than given a mid-life teardown it cannot be rebuilt from.
A behavior whose restore keeps failing leaves at most one pending
restart row and dead-letters the rest instead of stranding them. A panic
on the transactional path reaches ExecTx as the transaction's error,
which is what makes a real store discard the behavior's partial writes.
And a watcher on an actor that is stopped without ever being started
still gets its notification.

The harness grows what those need: a checkpoint-error injection point, a
first-wins record of the error each transaction returned (last-wins
would let a later successful restart turn erase the evidence), an
OnStop that panics, and a barrier that holds several turns until they
can panic together.
In this commit, we rewrite the supervision documentation, which
overpromised in the way the name "restart" invites.

The docs read as though the framework rebuilds a panicking actor. It
does not. It stops the workers, optionally calls OnStop, and redelivers
a RestartMessage; the same behavior INSTANCE keeps serving afterwards
with whatever fields the panic left in it. The clean slate exists
exactly where the RestartMessage handler rebuilds in-memory state from
the durable row, and nowhere else. We say that plainly, name the
adopters and the reload seam each of them uses, and note that a behavior
with no in-memory turn state may consume the message as a no-op provided
it says so and says why.

We also correct the intensity claim, which still described a default-on
budget of five restarts per sixty seconds, and record what the budget
actually costs: breaking it is terminal, and a terminated actor still
holds its ID and its mailbox rows, so it keeps looking alive to anything
that is not watching. That is why a finite budget belongs to owners that
wire Watch.

The remaining additions are the obligations the rework created for
handler authors, which had nowhere to live: a restart message is
delivered once and dead-lettered rather than retried, so restore
handlers must be idempotent; OnStop now runs mid-life and more than
once, so it must be idempotent and must leave the behavior able to serve
another generation; a panicking turn's own writes are rolled back rather
than committed alongside the nack; and an actor whose codec cannot carry
a restart message degrades to cycling its workers rather than taking a
teardown it cannot be rebuilt from.
@Roasbeef

Roasbeef commented Aug 8, 2026

Copy link
Copy Markdown
Member Author

An adversarial audit pass ran against the first 5 commits; the 13 commits on top address what it found. The audit's headline was blunt and correct: the kernel mechanics held up, but the two central product claims did not. Dispositions:

Fixed:

  1. Restart-from-checkpoint was vacuous for every production adopter (major). The behavior instance is reused across generations, and all six adopters either no-op'd the RestartMessage or could not decode it, so the panic path bypassed their existing commitFailed reload guards and "restart" restored nothing. Now every adopter is wired for real: credit and the OOR session arm their commitFailed reload seam (deferred, because restore starts goroutines that must outlive the turn), the OOR registry clears the dead turn's pending hand-off and re-runs its idempotent restore, unroll registers a sealed RestartMessage adapter and re-runs restoreCheckpoint (its old default arm would have nack-looped the restart), and serverconn's egress gets the adapter plus a documented no-op with the statelessness argument spelled out. A regression test drives a Right-path behavior into in-memory divergence, panics it, and proves the reload (verified non-vacuous: reverting the guard fails it). The docs now state the honest contract: the instance is reused, and the clean slate exists exactly where the handler rebuilds from the checkpoint.

  2. Default-on intensity was a silent kill switch (major). One poison Tell burned the whole 5/60s budget through the retry schedule; two in a minute permanently killed an actor nothing observes (no production Watch callers). Reversed: the default is now UnlimitedRestarts (zero normalizes to unlimited), with RecommendedMaxRestarts = 5 for owners that opt in AND wire Watch; the config GoDoc states the trade explicitly.

  3. Bare OnStop could crash the process (major). runStopHook now recovers panics into a termination (never a process crash), is idempotent per generation (deduping the double call on the restart-failed path), and the Stoppable contract is rewritten for mid-life, more-than-once invocation.

  4. The classic tx path committed a panicking turn's torn writes together with the nack (major), so the checkpoint the restart restored could contain the corruption. The panic now forces rollback of the behavior's writes inside the tx closure, with the nack redone detached and bounded so the poison message still burns its attempt.

  5. Ordering and hygiene: the codec Supports probe runs before teardown (an unsupported codec degrades to generation-cycling without a mid-life OnStop), restart rows are deduped to at most one pending per mailbox and can no longer strand past max_attempts (a failed restart turn dead-letters), Watch on a never-started actor is well-defined and leak-free, and the two dead-code/race notes carry comments.

Deliberately unchanged from the first cut: no Watch adopters outside baselib/actor, no supervision for the non-durable path, one-for-one only, no inter-restart backoff.

@Roasbeef

Roasbeef commented Aug 8, 2026

Copy link
Copy Markdown
Member Author

@claude review this

@claude

claude Bot commented Aug 8, 2026

Copy link
Copy Markdown

Claude finished @Roasbeef's task in 3m 3s —— View job


Review: supervision kernel for the durable actor runtime

I read the supervision core (supervision.go, the new supervise/superviseGeneration/restartFromCheckpoint/runStopHook in durable_actor.go), the classic-path rollback, restart.go, the Watch contract, and all five adopters. This is careful, well-argued work — the commit series and the disposition note match what's in the tree. My comments center on the two Codex findings plus a few smaller notes.


Codex P1 — "serialize restoration before launching the worker pool"

Valid in principle, latent in practice, and it contradicts a documented invariant. With NumWorkers > 1, supervise() launches all workers together (durable_actor.go:659-663). LeaseNextMailboxMessage hands the RestartMessage (at RestartPriority) to one worker and the next eligible row to a sibling immediately, so a normal turn can run against the shared behavior instance while the restart handler is still rebuilding it. That directly violates the invariant the package documents — "RestartMessage has RestartPriority … ensuring it is processed before all other messages on recovery" (CLAUDE.md) — which only holds under strictly-sequential (single-worker) delivery.

Why it's currently latent, and why I'd still act on it:

  • The only NumWorkers > 1 adopter is the serverconn egress sender (serverconn/runtime.go:80), and its restart is a documented no-op (no per-turn state to restore). credit/oor/unroll all restore, but all run single-worker. So no adopter is exposed today.
  • The same non-barrier already exists on the boot path (PrependRestartMessage + pool start), so this isn't new to supervision — but supervision is the first thing to make "restart a restoring behavior under a pool" a routine event rather than a once-at-boot one.

Recommendation: either narrow the invariant's wording to say the priority guarantee is single-worker-only and that a multi-worker restoring behavior must be self-serializing, or add a real barrier (process the restart turn to completion before starting the remaining workers). Given the pool guard already restricts pools to the Read/Commit path and the sole pool adopter is stateless, documenting the constraint is a defensible minimum; a barrier is the safer long-term answer if any future pool behavior restores state. Fix this →

Codex P2 — "terminate when a stopped behavior cannot be restored"

Already addressed — the finding is stale. It was raised against c16ebef, where the codec Supports probe ran after OnStop. In the current HEAD the ordering is reversed: superviseGeneration checks a.codec.Supports(RestartTLVType) at durable_actor.go:734 and returns the degrade-to-cycling path at :743, before runStopHook() at :750. So a Stoppable behavior whose codec omits RestartTLVType is not torn down — its workers cycle and the instance keeps serving, exactly the "a teardown it can't be rebuilt from is strictly worse than leaving it running" argument in the comment at :731-733 and in commit 9df6e04. No change needed; the concern the P2 raised (OnStop releases a resource, then messages run against a torn-down instance) cannot occur on this path anymore.


Other observations

Solid:

  • The classic-path rollback (processInTransaction, :1274-1307) is the right call: returning the panic from the ExecTx closure to roll back partial writes, then redoing ack/nack on a detached, bounded context so the poison message still burns its attempt. The nack-before-restart ordering is what makes a deterministic poison message dead-letter instead of crash-looping, and the restart-message dead-letter-not-nack rule (isRestartDelivery, :1226; MaxAttempts 1) closes the "stranded row at attempts == max_attempts" gap.
  • watcherRegistry is genuinely non-blocking: buffer-of-one, written once, select+default guard, published channel so a watcher on a never-started actor doesn't park on a done that never closes. The serverconn+round: never park the ingress path, make backpressure visible #1093 invariant holds.
  • Defaulting MaxRestarts to UnlimitedRestarts (zero normalizes to it) is the right reversal — a finite budget without a Watch observer converts a visible crash loop into invisible permanent death, and there are no production Watch callers yet.
  • runStopHook recovering an OnStop panic into a TerminationRestartFailed rather than crashing the process, plus per-generation idempotency, matches the rewritten Stoppable contract.

Minor:

  • Adopter wiring for credit/oor uses the deferred commitFailed = true arm rather than reloading inline; the reasoning (keep the restart message IO-free because it's max_attempts 1, and restore() starts a goroutine that must outlive the turn) is sound and documented. Worth confirming the guard can't be cleared by an intervening commit before the next dispatch — on the single-worker path it can't, but the coupling is implicit.
  • terminationInfo (:808) reads stopRequested unordered against a concurrent Stop; the comment already owns this and argues both readings describe "the actor was shut down." Agreed it's harmless, and the note is the right way to handle it.
  • Doc gardening advisory: the baselib/actor/CLAUDE.md/AGENTS.md in the tree already reflect the supervision kernel (the advisory diff appears to predate the doc commits on the branch) — no action needed beyond confirming the two files stay byte-identical.

Verdict: No blocking correctness issues in the kernel. The one thing I'd resolve before merge is the P1 gap — as a minimum, tighten the RestartPriority ordering invariant to state it holds only under single-worker delivery, so a future pool adopter that restores state doesn't inherit a silent race.

· branch actor-supervision-kernel

In this commit, we make the test store hand out messages in the order
the real SQL does: highest priority first, then oldest, using the UUIDv7
message ID as the age tiebreak the query's id ordering provides.

The mock ranged over its message map and took whichever entry Go's
randomised iteration reached first. That is invisible while a test has
one message in flight, and quietly wrong the moment ordering is the
thing under test: RestartPriority means nothing if the claim does not
honour it, so a test that asserts a restart message is handled before
the backlog behind it would pass or fail on map iteration order rather
than on the behavior it is meant to pin.

Nothing about the change is specific to the barrier tests that follow.
The mock simply models the claim query more faithfully than it did.
In this commit, we make the RestartMessage ordering guarantee hold for a
competing-consumer pool, which it did not.

RestartPriority makes the claim query hand the restart message out
before every other row, and that is where the guarantee stopped. It
orders the CLAIMS, not the TURNS. With NumWorkers greater than one the
supervisor launched every worker at once, so one worker took the restart
message while a sibling immediately took the row behind it, and a normal
turn ran against the same behavior instance while the restart handler
was still rebuilding it from the checkpoint. The invariant says a
restart message is processed before all other messages on recovery; a
priority alone cannot deliver that.

A generation now warms up one worker at a time. The supervisor launches
a single worker and the rest of the pool waits until that worker has
resolved the generation's restart hand-off. For a row supervision
enqueued itself, which is every supervised restart, the barrier waits
for that row unconditionally, so the guarantee is exact rather than
timing-dependent on the path this kernel creates. The boot hand-off is
different in kind: an owner prepends it before Start, so the actor never
sees it and cannot prove it is there. For that case the barrier orders
the first claim and releases on an idle tick, which orders the common
case honestly without claiming more than it can know.

The release rule is shaped so the barrier cannot wedge a pool that has
no hand-off waiting for it. A first claim of anything other than a
restart message releases the pool before that message is even
processed, so a generation with nothing to order pays nothing and never
serializes behind its own first turn. A restore that fails is
dead-lettered and a restore that panics tears the generation down, and
both resolve the hand-off. Whatever ends the warm-up worker releases the
pool, including a closed mailbox. And a Stop landing mid-barrier
releases it through the generation context, so shutdown never parks
behind a restore that will not finish.

A single-worker actor is already strictly sequential and needs none of
this, so it gets a nil barrier, which is a working no-op through every
method: the same code path with nothing to pay for.
In this commit, we pin the pool ordering guarantee and the three ways
the barrier has to be unable to wedge a pool.

The central test provokes a real supervised restart on a four-worker
pool, waits until the restore turn is parked inside a gate, and only
then queues a backlog behind it. That sequencing is what makes the test
deterministic rather than a race against the poll interval: by the time
the backlog is enqueued the barrier is provably shut and the warm-up
worker is provably parked, so any normal turn that runs is a real
ordering violation and not a timing artifact. Removing the barrier turns
the assertion from "never" into "satisfied", so the test fails for the
reason it exists.

The rest cover the release paths. A restore that fails is dead-lettered
rather than retried, and the pool fans out to drain the backlog behind
it. A Stop landing while the barrier is shut terminates cleanly instead
of parking shutdown behind a restore that will never finish, which is
the deadlock the context arm exists to prevent. And a pool with no
hand-off to order reaches full width, which is what proves the barrier
costs an ordinary generation nothing.
In this commit, we correct the RestartMessage ordering invariant, which
claimed more than the priority could deliver.

It read as though RestartPriority alone ensured a restart message is
processed before all other messages on recovery. That holds for a single
worker and does not hold for a pool, where the priority orders the
claims and leaves the turns to race. The invariant now says what
actually enforces the guarantee, namely the single-worker warm-up
barrier, and is explicit about the difference between the row
supervision enqueues itself (waited for unconditionally) and the boot
hand-off an owner prepends before Start (ordered on the first claim,
since the actor cannot see it).

We also record why the barrier cannot wedge a pool, because that is the
first question a reader will have: a first claim of anything else
releases it, a failed or panicking restore releases it, whatever ends
the warm-up worker releases it, and a Stop releases it through the
generation context.
@Roasbeef

Roasbeef commented Aug 8, 2026

Copy link
Copy Markdown
Member Author

P1 resolved in 26b6248 with the barrier rather than the doc-narrowing minimum, plus 60e2221 (tests) and c0fc477 (docs).

A generation now launches one warm-up worker and holds the remaining NumWorkers-1 behind a barrier until the restart hand-off resolves. The barrier is in-process and edge-triggered rather than store-polled, for a load-bearing reason: the restart row deliberately does NOT use a deterministic per-actor ID (a fixed ID would collide with processed_messages and dedup-skip the next restart's restore), so there is no stable key to poll. Instead the warm-up worker classifies its first claim: a RestartMessage holds the barrier across the whole restore turn; anything else opens it immediately (RestartPriority would have won the claim, so no hand-off was pending). On the supervised-restart path the guarantee is exact (a required flag makes the barrier ignore the idle tick until the worker's first claim, closing a fan-out race found during implementation); the boot hand-off remains first-claim-ordered plus idle-tick best-effort, and the docs state that distinction rather than paper over it. Deferred open on every worker exit covers panic, closed mailbox, and cancelled generation; a dead-lettered restore, the codec-unsupported degrade, and Stop mid-barrier all release cleanly. Single-worker actors get a nil barrier and an unchanged path.

The barrier test was verified non-vacuous (disabling the barrier flips the central never-assertion), which required first making the mock store's claim order priority-aware like the real query. The invariant wording in the package guides now names the barrier instead of overclaiming RestartPriority alone.

CI re-running on the new head.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants