fix(actor): derive liveness from last activity, not last completed step — a child blocked mid-step was indistinguishable from a dead one - #1965
Open
wqymi wants to merge 4 commits into
Open
Conversation
wqymi
changed the title
`deriveLiveness` placed unbounded trust in a `running`/`pending` registry row. Two consequences, both of which make the orchestrator's child roster lie: - a child that died BEFORE its first turn returned `progressing` forever, because `turnCount === 0` returned early and skipped the staleness window entirely; - a child that died AFTER some turns kept reading `stalled`, and `stalled` is presented as "in progress" — routable, and implying work is already in flight. ActorRegistry's orphan sweep is the only repair for such a row, and it runs once at process init and only for rows carrying a different instance_id. Until it runs (and for any row it cannot reach) the derivation must not assert progress. Adds DEFAULT_LIVENESS_ABANDON_MS (30m), measured from the last turn for a started child and from spawn time for one that never started. The turnCount-0 leniency is kept — a queued or cold-starting first turn is still `progressing` — but bounded instead of unbounded. Past the bound the row reads `idle`: the honest reading and the only non-routable bucket.
deriveLiveness read last_turn_time, whose only writer is the per-step heartbeat ActorRegistry.updateTurn. The finest thing it could see was a COMPLETED step, so a child blocked mid-step — inside a long tool call, a slow model call, a retry/backoff — was indistinguishable from a dead one. To avoid killing those, the abandonment bound had to clear the longest legitimate step (20+ minutes here), which is why it was 30 minutes. Read the signal that already exists instead. part.time_updated advances on every part write, so MAX over an actor's slice is "the last time anything succeeded" at per-API-call granularity, emitted today with no new schema. Denormalised onto the registry row as last_activity_time rather than joined at read time: the roster is rebuilt per request and iterates every peer child, and a read-time MAX over a measured 172-child / 45k-part roster cost 25-38ms warm and 339-487ms cold, versus zero extra queries for a column that rides along in the row listPeerChildren already selects. The writer is the PartUpdated projector, the single writer of part rows. With activity granularity the turnCount === 0 special case and the 30-minute window both collapse into one uniform bound of 10 minutes: 2x the repo's existing STUCK_THRESHOLD_MS, ~16x the measured p99 inter-activity gap (38.0s) and ~2x p99.9 (296.8s), and ~345x the worst measured first-activity latency after spawn (1735ms, n=172) — which is what licensed deleting the not-yet-started leniency, since the "slow first turn queued behind the concurrency gate" it protected is empirically under two seconds. turnCount is a counter again and is no longer read as evidence. Tests pinning the old step-grained semantics were rewritten deliberately, not relaxed; each rewritten case is marked and makes a strictly more specific claim than the one it replaced.
wqymi
force-pushed
the
fix/roster-liveness-dead-child
branch
from
July 29, 2026 17:05
f3dd1a3 to
2375280
Compare
wqymi
changed the title
…sumers have The last_activity_time heartbeat added in the previous commit writes on every part write, and that path has no throttle. bash.ts's per-chunk handler ends in a bare ctx.metadata(), which reaches Session.updatePart through SessionProcessor.updateToolCall (session/processor.ts) with no interval check anywhere on the way, so a chatty shell command performed one registry UPDATE — carrying a correlated subquery over `message` — per decoded stdout chunk. Measured end-to-end on this branch (real bash tool, real stream chunks, real projector, counting bun:sqlite statement executions and rows changed) with a 200k-line command: 453-575 registry rows rewritten per second, strictly 1:1 with part upserts, over three runs. The earlier affordability argument cited updatePartDelta (session.ts:771), which does only bus.publish and never touches the DB — but that is a different function from the updatePart on the metadata path, so it was made about the wrong code path and the rate was never measured. Nothing reads the column at that resolution. Its only consumers are DEFAULT_LIVENESS_STALL_MS (90s, the progressing/stalled display) and DEFAULT_LIVENESS_ABANDON_MS (10m, the routable/idle bound), so any write beyond one per tens of seconds is discarded information. ACTIVITY_COALESCE_MS = 5s is added next to those two constants and applied as a staleness predicate in the same WHERE: touch the row only when it records no activity yet, or when what it records is already older than the interval. 18x of margin under the stall window and 120x under the abandonment bound, above the measured p50 inter-activity gap (994ms) so it actually coalesces, below p90 (6.7s) so an ordinarily-paced child still records nearly every activity it has. Semantics are unchanged. The predicate lives in the WHERE rather than a process-local cache so it stays correct across instances and restarts, and it additionally makes the column monotonic: an out-of-order event carrying an older data.time can no longer drag it backwards. IS NULL is the first disjunct because the column is nullable and a fresh row records NULL, which must still take its first write. Two tests. One pins that a burst of writes inside the interval leaves the column at its first value while the parts themselves still land and the row still reads progressing; removing the predicate fails it (Expected 1785347884813, Received 1785347884821), and inverting the comparison — which degrades to no coalescing — fails it the same way. One pins that an actor emitting parts continuously across a span longer than the interval reads progressing at every sample and never lags by more than the interval; keeping only the IS NULL disjunct, so the column freezes after its first write, fails it (Expected <= 6000, Received 6069), and raising the constant above the stall window fails its safety precondition (Expected < 90000, Received 240000). After the change the same chatty command rewrites the registry row 0 times across the whole burst, bounded at one write per actor per 5s. The UPDATE statement still executes per part write — the guard suppresses the row mutation, not the statement — which is the intended and sufficient reduction: no dirtied page, no WAL append, no index or time_updated churn.
…to that
deriveLiveness already classified `stalled` off last_activity_time, but every
surface that reports the verdict still described the step clock, and the window
was still the number picked for the step clock. Both were wrong in the same
direction, and together they produced a measured false-positive channel.
Wording. The one user-visible surface, renderActorNotification, read
`appears stalled (no turn advance for Ns)` while N was silence since the last
part write — the noun and the number described different clocks. Two background
children inside single long steps (`bun ci`, a full test suite, `git worktree
add`) emitted a dozen-plus such notifications at ~90-130s while writing parts
continuously: a tool's streaming output calls ctx.metadata per chunk, which
reaches updatePart and advances last_activity_time. It now reads `no activity
for Ns`, and no longer claims the child "has made no progress" — a long step IS
progress, and we cannot see inside one, only whether anything is coming out of
it. Same correction to the TUI toast (which now also carries the duration), the
`session list` and fleet bucket headings ("no recent activity"), the
ActorStalled payload (lastTurnTime -> lastActivityTime, the reference the
predicate actually used), and six stale comments.
Window. 90_000 meant "no completed step for 90s"; read against activity it means
"no part written for 90s", a far stronger claim of silence. Against this branch's
own 43,120 measured inter-activity gaps (p50 994ms / p90 6.7s / p99 38.0s /
p99.9 296.8s), 90s lands strictly between p99 and p99.9 — between 0.1% and 1% of
gaps from HEALTHY children exceed it. Switching the signal turned the old
threshold into a new false-positive class rather than leaving it harmless.
6 minutes. Bounded below by the deepest measured natural silence (p99.9 =
296,811ms) PLUS ACTIVITY_COALESCE_MS, because recorded activity lags reality by
up to one coalesce interval and the threshold must clear 301,811ms or that lag
alone can flip a tail-but-healthy row. That is what rules out the otherwise tidy
300_000 (== STUCK_THRESHOLD_MS): it is below 296,811 + 5,000. 360_000 is 1.21x
p99.9, clears p99.9+coalesce by 58.2s, is 72x the coalesce interval, and leaves
a 240s `stalled` band (~5 scans at WATCHDOG_SCAN_INTERVAL_MS). Pinned as a
value, like the abandonment bound, so re-narrowing fails a test. Cost accepted
deliberately: a genuine stall surfaces in ~6min instead of ~90s, still
backstopped by the 10m abandonment bound — a signal believed late beats one that
is not believed at all.
registry.ts's stuck scan (STUCK_THRESHOLD_MS, SQL filtering on last_turn_time)
is deliberately untouched: "is a step taking too long" is a different question,
and the step clock is the right input for it.
Two fixtures encoding the old window as bare literals (5 minutes in
liveness.test.ts and session-tool.test.ts, 4 minutes in the custom-abandonMs
case) are restated against the constants with in-file notes — they had silently
become `progressing` rows asserting `stalled`. No assertion was weakened.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR ships — two steps, kept as two commits
This PR does two things to the same function, in order, and the commits are kept separate
because the second one largely retires the first:
773edc5db— bound the unreliable signal.deriveLivenessplaced unbounded trustin a
running/pendingrow, so a dead child stayed routable forever. This addsDEFAULT_LIVENESS_ABANDON_MSso the claim expires. Correct on its own, and its revertprobes stay attributable to it.
2375280de— replace the signal so the bound barely matters. The reason the bound hadto be so lenient (30 minutes) is that the signal was step-grained: it could only see a
completed step, so a child blocked mid-step looked exactly like a dead one. This reads
last activity instead, which already exists in the database and is written today. With
that, the
turnCount === 0special case and the 30-minute window both collapse into asingle 10-minute bound.
Read step 2 as the real fix and step 1 as the stopgap it grew out of. Sections below are
labelled by step.
Why this matters more than a display nit
The orchestrator's fleet roster exists for exactly one purpose: make the
orchestrator route work to an existing child instead of creating a new one — that is
#1741's whole point. So a dead child displayed as
progressingis worse than no rosterentry at all: the orchestrator routes work into a session that can never answer, and
progressingadditionally suppresses re-dispatch because something appears to already bein flight. Every consumer of
deriveLivenessinherits this: the roster,session list(which groups
progressing/stalledunder the heading "In progress"),session status,and the fleet table. #1741's route-to-existing mechanism is only as good as the liveness
signal it reads, so an over-optimistic signal weakens it directly.
stalledis not a safe fallback either. It also lives under "In progress" and it alsomarks the child as routable; it only says "no recent turn", not "nobody is home".
Step 1 — the defect: an unbounded claim
deriveLiveness(src/actor/schema.ts) placed unbounded trust in arunning/pendingrow. Two shapes:
progressingforever.if (actor.turnCount === 0) return "progressing"returned early and bypassed the staleness window entirely. The comment'sintent is right — a queued or cold-starting first turn must not be misread as a stall —
but it implemented that leniency as unbounded.
stalledforever, i.e. still "in progress".The only repair for a row whose owner died is
ActorRegistry's orphan sweep(
registry.ts:UPDATE actor_registry SET status='idle', last_outcome='failure', last_error='orphaned: process restarted' WHERE status IN ('pending','running') AND instance_id != $instanceID). That sweep runs once, at process init, and only for rowscarrying a different
instance_id. Until it runs — and for any row it cannot reach — thederivation is the only thing standing between a corpse and the router, and it asserted
progress. Same defect class as #1960's orphaned
runningtool parts: a persisted transientwhose repair lives on exactly one path.
Step 1 — the fix: bound the claim
One new constant and one guard, both in
deriveLiveness:Reference clock per row shape: a started child's evidence of life is its last turn; a
never-started one has none, so its spawn time (
time.created) is the only honestreference — which is what B needed: a longer bound measured from spawn, not removal of the
leniency.
Why 30 minutes, picked the way
DEFAULT_LIVENESS_STALL_MS's comment picks 90s:updateTurnis a per-step heartbeat, so a child that has genuinely begun cannot go 30mwithout bumping
last_turn_time— that is an order of magnitude past the 5-minutestuck-detection cutoff (
STUCK_THRESHOLD_MS) and unreachable by a live turn. A child thathas not begun is waiting on the concurrency gate, which admits work continuously, so 30m
of zero admissions means its process is gone.
Past the bound the row reads
idle— already documented as "finished with no recordedoutcome (or an unknown state)" — and
idleis the one bucket no consumer treats asroutable or in-flight. Deliberate direction of error: past the bound we would rather
report a still-queued child as
idle(worst case: the orchestrator creates a duplicate,which is wasteful) than as
progressing(worst case: work routed into a corpse andre-dispatch suppressed, which is unrecoverable).
Nothing is written to the database and no new repair mechanism is introduced; this is
purely the read side becoming honest.
Step 1 — what I could NOT confirm, and the named follow-up
The report I worked from assumed the registry "never reconciles rows whose process is gone
— they never even became
idle". Measured on the two real fixture rows, that is nottrue, and I am reporting it rather than fixing a defect that is not there. Both rows in
the isolated dev home read:
ses_05c0af252ffeigpmiJbqaa8vUV("Fix calc.py add() bug")idlefailureorphaned: process restartedses_05c08a1e9ffeFg2DEKBuhhBxuS("Modernize docs/README.md install steps")idlefailureorphaned: process restartedAll six orphaned rows share
time_completed = 1785163573647— one sweep, at process init.So the sweep works and reached exactly these rows; the roster that showed
stalled/progressingwas rendered while they were stillrunningunder the then-liveinstance, and reconciliation only came at the next process start. That reframes the
registry-side defect from "no reconciliation exists" to "reconciliation is bound to
process init, so it cannot see a row whose owner is the CURRENT instance".
That residual half is genuinely larger than this PR:
Completing that follow-up — the other half is signal granularity, and step 2 supplies it.
As written above, the follow-up covers only ownership: peers share the parent's
PROCESS_INSTANCE_IDandActor.fromRowdropsinstance_id, so no consumer can ask who ownsa row. That is still open and still needs a lifecycle change. But it is not the whole residual,
and the part it omits turned out to need no new schema at all: the reason the bound above had to
be 30 minutes is that
last_turn_timeis written only by the per-step heartbeat(
ActorRegistry.updateTurn,registry.ts:223, called once fromprompt.ts:3322afterstep++), so the finest event liveness could observe was a completed step — and a singlelegitimate step here can run 20+ minutes.
MAX(part.time_updated)already records "the lasttime anything succeeded" at per-API-call granularity and is written today
(
session.sql.ts:68→storage/schema.sql.ts:3; measured: 750,545 of 840,186 part rows havetime_updated > time_created). Reading that is the real fix, and it is what commit2375280dein this PR does — which is why the bound is now 10 minutes and theturnCount === 0special case is gone. Ownership remains the open follow-up; granularity isclosed here.
Step 1 — current scope on
main: this caveat was stale, and it invertedAn earlier revision of this description flagged that
<active-sessions>andlistPeerChildrenwere not onmain(#1741 open, head8847d324e, zero grep hits) andthat the roster therefore could not be reached from here. #1741 has since merged —
origin/mainis now its merge commit57ff02ed5— so that caveat is false, and what itimplied is inverted: this is not a scope downgrade, it means the routing hazard this PR
fixes is live on
maintoday. Verified onorigin/mainblobs at57ff02ed5:src/session/llm.ts:36importsderiveLiveness;:348callsactorReg.listPeerChildren;:355maps every child throughderiveLiveness;:366keeps exactlyprogressingandstalledas the routable "working" set;:377renders that status into the injectedroster line. The roster is on
mainand it reads this signal.src/actor/schema.ts:81onmainstill returns"progressing"unconditionally whenturnCount === 0, and:82bounds a started row bystallMsalone —deriveLivenessthere takes no
abandonMsat all. So a child that died before its first turn readsprogressingforever, directly into the roster the orchestrator routes from.maintool consumers this PR asserts through are unchanged:tool/fleet.ts:97,tool/session.ts:697and:967, which groupprogressing/stalledunder "In progress"(
tool/session.ts:983-984,tool/fleet.ts:146-147).Two things follow from the merge that I am correcting rather than restating. The literal
<active-sessions>tag no longer exists anywhere onmain— #1741's final commit deletedit — so the block is
ROSTER_HEADER(session/llm.ts:73-76) and the prompt calls it your"FLEET ROSTER" (
orchestrator.txt:30); naming it by the old tag would now be wrong. Andwhile the roster is reachable, this PR adds no test at that injection point:
test/session/llm.test.tsandtest/session/system.test.tsonmainhave zero hits forderiveLiveness,ROSTER_HEADERorlistPeerChildren, and I did not add one. The fix isin the shared derivation, so the roster inherits it; what is directly asserted below is the
tool consumers.
Step 1 tests — one per defect, each revert-probed
test/actor/liveness.test.ts(+4) andtest/tool/fleet.test.ts(+1). The fleet test usesthe two measured fixture rows' real field values (turnCount 26 / 20) replayed a day after
their last turn, and asserts through
assembleFleet— the consumer — that neither lands ina routable bucket.
Every pre-existing expectation is unchanged; the existing literals only gained the
timefield the widened
Picknow requires, and the existing "10-minute-old cold start is stillprogressing" case still passes (10m < 30m), which is the point of keeping the leniency.
Probe 1 — restore the unbounded
turnCount === 0early return (defect B):Probe 2 — keep B's spawn bound, drop the bound for rows that HAVE run a turn (defect A):
Step 1 suite numbers
bun typecheckexit 0.Scoped run only (
test/actor+ everything that references the signal, found withrg -l 'deriveLiveness|active-sessions|listPeerChildren' test), because concurrentheavyweight suites on this host starve each other. Same command both sides
(
bun test test/actor test/tool/fleet.test.ts test/tool/session-tool.test.ts --timeout 120000),same worktree and
node_modules, the three changed files reverted toorigin/mainfor thebaseline:
origin/main(60af8f1)Note on the baseline commit:
60af8f1predates the #1741 merge that is nowmain's tip(
57ff02ed5). These numbers are the record of that comparison run, not a claim abouttoday's
main. The three files this PR changes are unchanged by that merge, which is whythe branch still applies cleanly.
+5 testsis exactly the five new cases. My failure set is a strict subset of thebaseline's: both runs fail
spawn no-deadlock (F56) > checkpoint-writer settles (no hang) when session permission is '*':'ask' …, and the baseline additionally fails foursession tool joincases on their 30s timeouts. The runs were sequential rather thanside-by-side, so the four extra baseline failures are load-dependent flakes, not a
regression — none of them is touched by this change, and none appears on this branch.
Step 2 — the signal itself was too coarse
Mechanism
deriveLiveness's inputs all came from the actor registry row, and the only writer ofturn_count/last_turn_timeisActorRegistry.updateTurn(
packages/opencode/src/actor/registry.ts:223), called from exactly one place —packages/opencode/src/session/prompt.ts:3322, immediately afterstep++, commented"Per-step turn heartbeat: only writer of turn_count".
So the finest thing liveness could see was a completed step. A child blocked mid-step —
inside a long tool call, a slow model call, a retry/backoff — was indistinguishable from a
dead one. That is why the bounds had to be stretched: a single legitimate step in this repo
can run 20+ minutes (a live test run ≈1225 s;
git worktree add>120 s), so any bound had toclear the longest possible step, and the 30 minutes in step 1 existed only because the
signal was coarse.
The finer signal already exists and was simply not read.
PartTable(
packages/opencode/src/session/session.sql.ts:68) spreads...Timestamps, andTimestamps(
packages/opencode/src/storage/schema.sql.ts:3) declarestime_updated: integer().notNull().$onUpdate(() => Date.now()). Part rows are written when atool call starts and updated as it progresses, so
MAX(part.time_updated)over an actor'sslice is already "the last time anything succeeded", at per-API-call granularity.
Verified that it is really written, rather than assumed — the projector writes parts with
onConflictDoUpdate, and it was not obvious that Drizzle's$onUpdatefires on that path.Measured on a 5.4 GB production database:
Deltas on individual rows (23644 ms, 8988 ms, 118 ms, 1 ms) show a single part row being
updated repeatedly while one tool call runs. Streaming token deltas do not hit the table
—
Session.updatePartDelta(packages/opencode/src/session/session.ts:771) only publishes abus event — so the write rate is bounded by discrete part lifecycle events plus
ctx.metadata()progress calls, not by tokens.Shape: denormalised column, chosen by measurement
The seam to respect:
partrows carrysession_idbut no agent id — the agent slicelives on the
messagerow — so a per-slice read needs a join toMessageTable, not asingle-table read.
The roster is rebuilt per request and iterates every peer child
(
packages/opencode/src/session/llm.ts:348), so read-time cost is paid on a hot path. Measuredagainst the worst real roster on disk — a parent with 172 peer children and 45,377
parts across them:
MAXper child)last_activity_timecolumnlistPeerChildrenalready selects)Batching does not rescue the read-time shape: (a′) is no better than (a) because the cost
is scanning the 45 k parts, not per-query overhead, and it grows without bound as children
accumulate history. So (b).
The writer is
packages/opencode/src/session/projectors.ts:118— theMessageV2.Event.PartUpdatedprojector, which is the single writer of
partrows and therefore already fires on every partwrite. No new hook in the session loop. It is sequenced after the part insert inside the same
try, so activity is recorded only if the part actually landed, and it resolves the owningactor through the message's primary key (
session_id+message.agent_id), which hitsactor_registry's primary key directly and stays correct for peers, subagents andmainalike. For peers this is exact because
runAgentLooppasses the actor id as the message'sagentID(packages/opencode/src/actor/spawn.ts:263).The bound: 10 minutes, and why that number
One reference for every row and no per-row special case. Measured over 43,120 real
inter-activity gaps across that 172-child roster, and over the first-activity latency of all
172:
10 minutes is therefore:
STUCK_THRESHOLD_MS(packages/opencode/src/actor/registry.ts:16), the repo's ownexisting "stuck" cutoff — an anchor that already survived review, not a fresh magic number;
turnCount === 0leniency rather than keeping it: the "slow first turn queued behind theconcurrency gate" it protected is empirically under two seconds, because the user message
that starts the turn is itself persisted as a part. Step 1's comment reasoned that a
not-yet-begun child "is waiting on a continuously-admitting concurrency gate" and so needed
30 minutes of grace; measurement says it needs about two.
Direction of error is unchanged and still deliberate: prefer a duplicate child (wasteful) over
routing into a corpse with re-dispatch suppressed (unrecoverable).
DEFAULT_LIVENESS_STALL_MSstays at 90 s. It is a display distinction only —stalledisstill routable — and against the activity signal it became more meaningful, not less: 90 s of
true silence is already past the 99th percentile.
turnCountis a counter againIt is no longer in
deriveLiveness'sPick, so the compiler now prevents it being read asevidence of life.
updateTurnstill writes it andturn-heartbeat.test.tsstill pins that itadvances per step — that test needed no change, because the per-step heartbeat is still a real
thing, just not the liveness signal.
Two consequences fixed in the same commit, both cases of a number disagreeing with its own
predicate:
packages/opencode/src/actor/spawn.ts:940reported the stall duration asnow - actor.lastTurnTimewhile classifying on activity; it now reports the quantity theclassification actually used.
packages/opencode/src/tool/fleet.ts:112computed a column literally namedlastActivityMsfrom
lastTurnTime, i.e. from the last completed step; it now uses the same referencederiveLivenessclassifies on, so the displayed age cannot contradict the displayed liveness.Deliberately not touched: the separate stuck-detection scan at
packages/opencode/src/actor/registry.ts:477still filters onlast_turn_timein SQL. Itanswers a different question — "is a step taking too long to complete" — which remains
meaningful and is not liveness.
The nullable column
last_activity_timeis nullable on purpose: rows predating the migration, and a row readbetween
register()and its first part write, have genuinely recorded no activity, andNULLsays so instead of inventing a timestamp.
registerwritesnullexplicitly(
packages/opencode/src/actor/registry.ts:152) andfromRowflattens with?? undefined(
:44).The comparison uses
??, never!== undefined. PerAGENTS.md§ "Reading a nullable column",a
!== undefinedguard on a nullable column is a silent no-op that typechecks and readscorrectly —
null !== undefinedistrue. Probe 2 below is that exact mutation, and it turnsa live child
idle.Step 2 files changed
packages/opencode/src/actor/schema.ts:112DEFAULT_LIVENESS_ABANDON_MS30 min → 10 minpackages/opencode/src/actor/schema.ts:115PickdropslastTurnTime/turnCount, addslastActivityTimepackages/opencode/src/actor/schema.ts:126lastActivityTime ?? time.created;turnCount === 0case deletedpackages/opencode/src/actor/schema.ts:37-42Actor.lastActivityTime(optional)packages/opencode/src/actor/actor.sql.ts:34last_activity_time: integer()(nullable)packages/opencode/migration/20260729000000_actor_registry_last_activity_time/migration.sqlALTER TABLE … ADD COLUMNpackages/opencode/src/actor/registry.ts:44fromRowreads it with?? undefinedpackages/opencode/src/actor/registry.ts:152registerwritesnullpackages/opencode/src/session/projectors.ts:6,140-152packages/opencode/src/actor/spawn.ts:940packages/opencode/src/tool/fleet.ts:112lastActivityMsreads activityStep 2 tests, and the ones deliberately rewritten
New in
test/actor/liveness.test.ts: the mid-step child (recent activity, no completed turnfor 25 m) reads
progressing; the converse (many turns, an hour of silence) is not routable;the bound is pinned to 10 minutes as a value; a
nullactivity column falls back to spawntime; and an end-to-end case where a real part write advances
last_activity_timewhileturn_countstays at 0 — progress visible with zero completed steps, which is exactly whatthe old signal could not express.
Rewritten, not weakened. Several tests encoded the old step-grained semantics. Each is
marked in-file and makes a strictly more specific claim than the one it replaced:
liveness.test.ts"not-yet-started child (turnCount 0) is never stalled" assertedprogressingfor a row silent 10 minutes. That assertion was the fabrication beingremoved. Replaced by the spawn-fallback case, which keeps the real intent (a freshly spawned
child is not mistaken for wedged) and states it against the mechanism that now carries it.
liveness.test.ts"the turnCount-0 leniency still holds inside the abandonment bound"asserted
progressingat 30 min − 1 ms of silence. Under one uniform bound the honestreading is
stalled— still routable, so nothing is lost operationally — and the case nowalso pins both sides of the boundary plus one millisecond past it.
liveness.test.tsintegration "not-yet-started row reads progressing even far past thewindow" — same reason; now asserts the row has no recorded activity, is
progressingunderthe real window, and
stalledunder a 1 ms one.fleet.test.tsandsession-tool.test.tsexpressed "wedged" by backdatinglast_turn_time. They now backdate activity; the assertions themselves are untouched. Thesession-toolfixture moved from 10 min to 5 min of silence so it still meansstalledunder the tighter bound rather than sliding into
idle.stall-watchdog.test.tsexpressed "child resumed" asupdateTurn, which is no longeractivity, so the re-arm step silently stopped re-arming. It now also freshens activity —
which is what a resuming child really does. The notification-count assertions are unchanged.
fleet.test.ts's "a row still claiming running a day after its last turn is not routable" —the one built from the measured fixture rows — passed unchanged, because a day-old row with
no activity falls back to spawn time and is still past the bound.
Step 2 revert probes, verbatim
Probe 1 — remove the per-part writer in
projectors.ts(the mechanism):Probe 2 — change the nullable read
??into!== undefined(theAGENTS.mdhazard):A guard whose field is right but whose predicate is wrong: the mutation typechecks, reads
correctly in review, and turns a one-second-old live child into
idle.Probe 3 — widen the bound back to 30 minutes:
Probe 4 — restore the step-grained rule (
turnCount === 0 ? time.created : lastTurnTime),i.e. revert step 2's core claim while keeping step 1:
Step 2 suite numbers
bun typecheckexit 0.Same command both sides, same worktree and
node_modules, baseline detached at the sameorigin/mainthis branch is rebased onto (b81670870):origin/main(b81670870)2375280de)+10 tests, zero failures on either side — so unlike step 1's run there is no sharedflake set to reason about. (Step 1's table above, with its 5 baseline / 1 branch failures, is
the record of an older run at
60af8f1; the flakes there were load-dependentsession tool jointimeouts.)What I did NOT verify
RUN_ORCHESTRATOR_LIVEsuite was executed; a gated suitethat has not been run is not verification.
test/session/llm.test.tsand
test/session/system.test.tsstill have zero hits forderiveLiveness/ROSTER_HEADER/listPeerChildren, and I did not add one. The fix is in the shared derivation and is assertedthrough the tool consumers (
assembleFleet,session list).sqlite3warm/cold on a 5.4 GB file and will differ elsewhere. The ordering they establish(0 extra queries beats 25–38 ms warm on a per-request path) is what the shape decision rests
on, not the absolute numbers.
sub-hour gaps in each slice's activity stream, which includes a child sitting idle between
turns. A row that is genuinely
idlenever reaches the window, so those gaps inflate thetail rather than the reverse — but I cannot prove from history that no healthy running
actor is ever silent longer than 10 minutes, which is why the bound keeps ~2× margin over
p99.9 and errs toward keeping the child routable.
covered by a unit case pinning the
nullshape, not by an on-disk upgrade.bashcallsctx.metadata()per output chunk(
packages/opencode/src/tool/bash.ts:679), so a chatty command advances activitycontinuously; a command that emits nothing for minutes (e.g.
git worktree add) advances itonly at start and end. That case is still bounded by the 10 minutes rather than by the step
window, which is the margin the number is carrying.
Follow-up commit
40f6c807f— the heartbeat's write rate, measured then coalescedA re-review raised one substantive concern about
2375280de: the activity heartbeat writes farmore often than its consumer's resolution requires. It was right, and the reason it slipped
through is worth stating plainly — the affordability argument in
2375280de's message citedupdatePartDelta(packages/opencode/src/session/session.ts:771), which only callsbus.publishand never touches the DB. That is true, but it is a different function from theupdateParton the metadata path, so the argument was made about the wrong code path and theactual rate was never measured.
The path, re-verified on this branch
packages/opencode/src/tool/bash.ts:679ends its per-chunk handler with a barereturn ctx.metadata({ ... })— no interval check, no coalescing. For the bash tool thatctx.metadatais the one constructed atpackages/opencode/src/session/prompt.ts:1161, whichcalls
SessionProcessor.updateToolCall; that lands onsession.updatePartatpackages/opencode/src/session/processor.ts:332with no throttle anywhere in between;updatePart(session.ts:583) runsSyncEvent.run(PartUpdated, …), and the projector(
packages/opencode/src/session/projectors.ts:119) then does the part upsert plus theregistry UPDATE with its correlated subquery over
message.Measured, before
End-to-end on this branch: real
BashTool, realStream.decodeTextchunks from a real spawnedshell, real projector against a real SQLite DB, counting statement executions and rows
changed at the
bun:sqlitestatement level (a 200,000-lineecholoop, three runs):Strictly one registry UPDATE per part upsert per decoded stdout chunk, each one running the
correlated subquery. So the concern was real, not hypothetical.
Two instrumentation artifacts were found and removed before trusting any of these numbers, both
of which had inflated an earlier reading: patching both
Database.prototype.prepareandDatabase.prototype.querydouble-wraps the same statement (queryis built onprepare), andbuilding the service layer inside the metadata callback constructs a fresh
ActorRegistrylayerper call, whose init re-runs the orphan sweep.
Measured, after
0registry rows rewritten across the entire burst, bounded at one write per actor per 5 s.Stated precisely, because it is the honest limit of a
WHERE-clause guard: the UPDATEstatement still executes per part write — the guard suppresses the row mutation, not the
statement. That is the intended and sufficient reduction (no dirtied page, no WAL append, no
index or
time_updatedchurn), and the predicate was deliberately kept in theWHEREratherthan a process-local cache so it stays correct across instances and restarts. It also makes the
column monotonic as a side effect: an out-of-order event carrying an older
data.timecan nolonger drag it backwards.
The constant
ACTIVITY_COALESCE_MS = 5_000, named next to the other liveness constants inpackages/opencode/src/actor/schema.ts(not inlined in the query), and justified against bothconsumers of the column — which are the only things that read it, and are three orders of
magnitude coarser than the write rate:
DEFAULT_LIVENESS_STALL_MS(90 s) — theprogressing/stalleddisplay. Anactively-writing actor's recorded activity now lags reality by at most the interval, so
worst-case apparent age is 5 s against a 90 s threshold: 18× of margin, and the flip is
unreachable by coalescing alone.
DEFAULT_LIVENESS_ABANDON_MS(10 min) — the routable/idlebound: 120× of margin.it targets, and below p90 (6.7 s) so an ordinarily-paced child still records very nearly every
activity it has.
IS NULLis the first disjunct because the column is nullable and a fresh row recordsNULL,which must still take its first write.
Tests, with revert probes
Two tests were added and neither existing assertion was relaxed — in particular the cases pinning
the 10-minute bound as a value and pinning
lastActivityTime ?? time.createdare untouched.leaves the column at its first value, while the parts themselves still land and the row still
reads
progressing. Removing the staleness predicate fails it:expect(received).toBe(expected) / Expected: 1785347884813 / Received: 1785347884821.Inverting the comparison (
<→>), which silently degrades to no coalescing at all, failsit the same way.
ACTIVITY_COALESCE_MSreadsprogressingat every sample and never lags by more than theinterval. Keeping only the
IS NULLdisjunct — so the column freezes after its first write —fails it:
expect(received).toBeLessThanOrEqual(expected) / Expected: <= 6000 / Received: 6069.Raising the constant above the stall window fails its safety precondition:
expect(received).toBeLessThan(expected) / Expected: < 90000 / Received: 240000.Scoped run (
liveness,stall-watchdog,turn-heartbeat,registry,registry-status,fleet,session-tool,message-v2): 153 pass, 1 skip, 0 fail, against 141 pass, 1 skip,0 fail for the identical command on this branch's base
b81670870— +12 is the 2 new casesplus the 10 the branch's earlier commit added.
bun typecheckexit 0;oxlint0 errors.The "saturated concurrency gate" scenario has no mechanism here
The same re-review flagged that a pending child emitting no part for over 10 minutes now reads
idle, hypothesising a saturated concurrency gate holding it un-started. That premise doesnot exist in this codebase. There is no semaphore, permit or concurrency limit anywhere in
packages/opencode/src/actor/— a search forSemaphore/makeSemaphore/withPermits/maxConcurrentacross that directory returns nothing. The spawn admission path is an immediateunbounded fork:
Actor.spawn(packages/opencode/src/actor/spawn.ts:782) →forkWork→Effect.forkIn(scope)atpackages/opencode/src/actor/spawn.ts:657, with nothing gating it. Thesingle
concurrency:occurrence in the whole directory ispackages/opencode/src/actor/spawn.ts:841, and it readsconcurrency: "unbounded"— inActor.cancel's cascade, not on the spawn path at all.Combined with this branch's own measurement of first activity after spawn (p50 194 ms, max
1,735 ms, and 0 of 172 children with no activity at all), a child sitting un-started for more
than 10 minutes has no reachable mechanism. Recorded here so the concern is answered rather than
left hanging.
The genuine residual is unchanged and already noted above: a silent tool call such as
git worktree addthat writes parts only at start and end.Follow-up commit
2e5f0886d— the verdict's wording, and its window, re-derived for the activity signalThe two earlier commits switched the input of
deriveLivenesstolast_activity_time. This onefinishes that change on the two things it left behind: every surface that reports the verdict
still described the step clock, and the threshold was still the number chosen for the step clock.
Both were wrong in the same direction, and together they produced a measured false-positive
channel.
The evidence: a dozen-plus false notifications in one evening
stalledis not display-only. It drives a user-visible notification —packages/opencode/src/actor/spawn.ts:919(TUI toast) plus the inbox card rendered byrenderActorNotification— and the card read:Two long-running background children spent the evening emitting that at ~90-130 s while
healthy and demonstrably writing parts continuously. They were inside single long steps
(
bun ci, a whole test suite,git worktree add), and a tool's streaming output callsctx.metadataper chunk, which reachesupdatePartand therefore advanceslast_activity_time. So the number in the message was silence since the last part write, whilethe noun said "turn advance" — two different clocks in one sentence, and the message asserted
has made no progressabout children that were visibly progressing.Over a dozen such notifications were emitted, and the operator began treating every one of them
as noise. That is the actual damage: a false-positive channel trains its reader to ignore it,
so a true stall would have been missed. The fix needed no new mechanism — the column already
exists, is already written, and was already validated on this branch.
The seam,
file:linebefore → after40f6c807f)2e5f0886d)src/inbox/render.ts:72—` (no turn advance for ${…}s)`src/inbox/render.ts:80—` (no activity for ${…}s)`src/inbox/render.ts:73—has made no progresssrc/inbox/render.ts:81—nothing has landed for it in that timestalledForMsdocsrc/inbox/render.ts:39— "since the child's last turn advanced"src/inbox/render.ts:39-42— silence; explicitly not time since the last completed stepsrc/actor/spawn.ts:919—appears stalled(no duration)src/actor/spawn.ts:927—appears stalled (no activity for Ns)ActorStalledpayloadsrc/actor/events.ts:54—lastTurnTimesrc/actor/events.ts:58—lastActivityTime(the reference the predicate used)ActorStalleddocsrc/actor/events.ts:44-47— "no turn advance … re-arms after turnCount advances"src/actor/events.ts:43-51— nothing landed … re-arms once activity landssrc/actor/spawn.ts:867-868— "now-lastTurnTime > STALL_MS AND turnCount not advancing"src/actor/spawn.ts:867-869— nothing landed pastDEFAULT_LIVENESS_STALL_MSsrc/actor/spawn.ts:878— "turnCount advanced"src/actor/spawn.ts:879— "activity landed again"src/actor/spawn.ts:42-45— "between the per-step turn heartbeat and the … (90s) window"src/actor/spawn.ts:42-45— "well inside the … (6m) window"session listheadingsrc/tool/session.ts:984—stalled (running/pending, no recent turn)src/tool/session.ts:989—stalled (running/pending, no recent activity)session listdocsrc/tool/session.ts:958-961— "(…, lastTurnTime) … updateTurn bumps last_turn_time per step"src/tool/session.ts:958-961— "(…, lastActivityTime) … the PartUpdated projector bumps last_activity_time per part"session statusdocsrc/tool/session.ts:1059— "(status/lastOutcome/lastTurnTime)"src/tool/session.ts:1059-1060— "(status/lastOutcome/lastActivityTime)"session statusoutputsrc/tool/session.ts:1069-1078— printed onlylastTurnTimenext to the verdictsrc/tool/session.ts:1078-1085— printslastActivityTime … — liveness derives from thisand keepslastTurnTime … — last COMPLETED step, not the liveness inputsrc/tool/fleet.ts:152—stalled (no recent turn)src/tool/fleet.ts:155—stalled (no recent activity)lastActivityMsdocsrc/tool/fleet.ts:47— "ms since the last turn advanced"src/tool/fleet.ts:47-50— ms since anything last landed; "not the step clock"src/session/projectors.ts:147— "deriveLiveness's 90s stall display"src/session/projectors.ts:147— "deriveLiveness's 6m stall display"Fourteen sites. The three flagged as at-minimum (
schema.ts:58,schema.ts:66,events.ts:44asnumbered on
main) were already correct inschema.tsafter2375280de;events.tswas not, andeleven more were not. Every one of them described "turn advance" for a predicate that had stopped
reading the turn clock — the same class of stale comment that caused real misreadings in this
codebase.
The threshold: 90 s → 6 min, and why 90 s did not survive the change
Under the old signal 90 s meant "no completed step in 90 s". Under the new one it means
"no part written in 90 s" — a much stronger statement about silence. Re-checked against this
branch's own measurements rather than assumed to carry over:
90 s lands strictly between p99 and p99.9, so between 0.1 % and 1 % of inter-activity gaps
produced by perfectly healthy children exceed it. Across the 43,120 measured gaps that is up to
~430 natural trips. So switching the signal did not leave 90 s harmless — it converted it into a
new class of false positive, which is exactly what was observed at ~90-130 s.
The number is bounded from below by two measurements:
fires on healthy children by construction.
ACTIVITY_COALESCE_MS = 5_000. Coalescing means an actor's apparent age lags realityby up to one interval, so apparent age = real gap + up to 5 s. The threshold must clear
296,811 + 5,000 = 301,811 ms, or the coalescing lag alone can flip a tail-but-healthy row.
This is what disqualifies the otherwise tidy
300_000(which would have equalledSTUCK_THRESHOLD_MSand read very cleanly): 300,000 < 301,811. It is asserted as a test.And from above by
DEFAULT_LIVENESS_ABANDON_MS = 600_000, which must remain the larger of the twoor
stalledbecomes unreachable.DEFAULT_LIVENESS_STALL_MS = 6 * 60_000(src/actor/schema.ts:110):come near flipping a verdict;
ACTIVITY_COALESCE_MS;stalledband ≈ 5 scans atWATCHDOG_SCAN_INTERVAL_MS = 45_000;git worktree addthat writes parts only at start and end).Cost, accepted deliberately: a genuine stall now surfaces within ~6 min instead of ~90 s. The
10-minute abandonment bound still catches a row whose owner is actually gone. A signal that is
believed late beats one that is not believed at all — which is precisely what the old number
produced. Pinned as a value, the same treatment
DEFAULT_LIVENESS_ABANDON_MSalready gets, sore-narrowing it fails a test rather than passing review.
stalledremains a distinct verdict with its own threshold — it is not merged intoidle.registry.ts's stuck scan is intentionally unchangedSTUCK_THRESHOLD_MS = 5 * 60 * 1000and its SQL filter onlast_turn_time(
src/actor/registry.ts) are deliberately left on the step signal and are not touched by thiscommit — verified:
registry.tsdoes not appear in2e5f0886d's file list. That scan answers adifferent question, "is a step taking too long", for which the last-completed-step clock is the
correct input. Nothing here argues it should change.
The two required tests, with revert probes verbatim
1. A child inside a long step with recent activity is NOT stalled —
test/actor/liveness.test.ts, at the ages actually observed (90,001 / 100,000 / 130,000 ms andp99.9 = 296,811 ms), with
time.created25 minutes old so no completed step exists to rescue it.Reverting only
src/actor/schema.tsto40f6c807f(90 s window) and keeping the tests:The same probe also fails the value pin and the measurement invariant:
→
25 pass, 3 failon that file with the window reverted;28 pass, 0 failwith it restored.2. A child with genuinely no activity past the threshold IS stalled — same file: exactly at the
window is still
progressing(the<=convention), one ms past it flips tostalled, and it staysstalled— routable, notidle— right up to the abandonment bound, flipping toidleone mspast it. This is the guard that widening the window cannot quietly disable the verdict.
3. The notification says what it measures —
test/inbox/parse-actor-notification.test.ts.Reverting only
src/inbox/render.tsto40f6c807f:→
15 pass, 2 failon that file with the text reverted.Before each probe the working diff was dumped to a patch outside the worktree and verified with
git apply --reverse --check; after each probe the tree was restored from that patch and confirmedbyte-identical with
git diff | diff -q -.Fixtures deliberately rewritten (no assertion weakened)
Three fixtures encoded the old window as a bare literal and had silently become
progressingrows asserting
stalled. Each is restated against the constants, with an in-fileREWRITTENnote:liveness.test.ts"pending is treated as live and split by the same window"lastActivityTime: now - 5 * 60_000now - (DEFAULT_LIVENESS_STALL_MS + 1)liveness.test.ts"custom abandonMs overrides the default bound"now - 4 * 60_000, custom bound2 * 60_000silentFor = STALL_MS + 60_000, custom boundsilentFor - 1session-tool.test.tsstalled-peer row (in-line comment said "5 minutes is past the 90s stall window")Date.now() - 5 * 60_000Date.now() - (DEFAULT_LIVENESS_STALL_MS + 60_000)Each now pins "past the window" — the property the case was always about — at any window value,
so none of them can silently invert again. The claims are identical or stricter; nothing was
relaxed to go green. The
session listheading assertion was updated to the reworded heading, andthe
session statustest gained three assertions (lastActivityTime:,liveness derives from this,not the liveness input) while keeping its existinglastTurnTime:assertion.Suite numbers, against a pristine baseline of the identical command
Command (from
packages/opencode):bun test test/actor/liveness.test.ts test/actor/stall-watchdog.test.ts test/tool/fleet.test.ts test/tool/session-tool.test.ts test/inboxb8167087088ad361740d6e28fb35faae49afdf7840f6c807f2e5f0886dThe baseline sha was printed and compared: it equals
git merge-base 40f6c807f origin/main(
b81670870) —origin/mainmoved repeatedly today, so this is the branch's own base, notwhatever
mainhappens to be now. +7 cases and +26 assertions over the previous head.bun typecheckexit 0 (forced,Cached: 0 cached, 12 total— not a cache replay).oxlinton the ten changed files: 0 errors, 42 warnings — identical count before and after, sothis commit introduces none.
prettier --writewas not run (this repo is not prettier-clean underits own config); surrounding style was hand-matched.
Remote content verification
Verified by reading back the pushed blobs, not local files:
A scan of every
.tsfile in the pushed tree forno turn advance/no recent turn/last turn advanced/turnCount advancreturns exactly one hit — the deliberate contrast commentat
src/inbox/render.ts:74that explains the correction.git merge-base --is-ancestor 40f6c807f origin/fix/roster-liveness-dead-childpasses, so the three pre-existing commits are untouched andthis is a pure append.