This repository was archived by the owner on Jul 24, 2026. It is now read-only.
fix: make an always-on agent reachable, and stop reporting liveness it hasn't earned - #106
Merged
schickling-assistant merged 4 commits intoJul 21, 2026
Conversation
`st ding` refreshed the watched identity's status mtime on a timer that
was entirely decoupled from whether it was actually delivering. Combined
with the pane guard's mid-turn hold, that let a sidecar sit on a message
indefinitely while continuing to advertise the agent as `available`.
Reproduced (isolated root + stub `pty`, never against a live bus): boot
`st ding` on an empty inbox, send one message, hold the target pane's
frame changing. Result: zero `pty send` calls, message stays in the
inbox, process alive, status mtime bumped every tick, and — with
ST_DING_DEBUG off, i.e. the production posture — *completely silent*
stderr. A sender reads a healthy, available recipient and gets nothing.
Root cause is not a failure to arm. The watcher arms and fires; the
startup scan runs; the session-watch's "not yet registered" path does
not gate delivery (verified separately). The block is in
`guardedDeliver`: on the no-input branch a changing frame returns `held`
unconditionally. `forceCap` is computed but only ever consulted on the
un-submitted-input branch, so the hold has no upper bound. A pane that
is never byte-static across the 300ms diff — the normal state of a
working agent — parks every poke for as long as it keeps working.
The hold decision itself is correct and stays: submitting into an active
Claude Code turn seeds CC's queued-input replay bug, and there is an
explicit regression test forbidding a force-submit. What was wrong is
that the sidecar lied about it. So:
- past the hold cap, warn once, loudly, on stderr (production had no
signal at all for this state)
- past the hold cap, suspend the status heartbeat, so the mtime
freezes and readers derive staleness through the existing path
- on a successful delivery, clear the stall and resume
This is the "gate the liveness write" option from the issue, moved from
*armed* state to *delivery* state, which is what actually predicts
whether mail gets surfaced. Invariant: a sidecar must not write liveness
it has not earned.
Refs #101
agent-session-id: 0abcedc7-6b71-4046-9e7c-f645268c0b15
agent-tool: Claude Code
agent-tool-version: 2.1.215
agent-model: claude-opus-4-8
agent-runtime-profile: /nix/store/acr8a3l2v366jgmwiq8xdrhgz1py0db5-coding-agent-runtime-profile/share/coding-agents/profile.json
agent-skills-manifest: /nix/store/sj1v5j91h8v8d1w9lca4040302lwrd6v-agent-skills-corpus/share/agent-skills/manifest.json
tooling-profile: dotfiles@unknown-dirty
`st agents` reported `available` for an identity that `convoy ls --tree`
correctly called `DEAD (status stale 3m ago)` at the same instant.
The issue's premise — that `st agents` applies NO freshness window —
turned out to be wrong, and it is worth stating plainly because it
changes the fix. `st agents` reads through `readIdentityStatus` ->
`readState`, which already applies STATUS_STALE_MS and collapses to
`unknown`. Verified: backdate a status file 3 minutes and `st agents`
says `available`; backdate it 20 minutes and it says `unknown`.
So this is a threshold mismatch, not a missing check. The existing
window answers "do we still trust this value?" and is sized for the
SLOWEST writer — the MCP server's 5-minute refresh — hence 15 minutes.
Convoy's window answers a different question, "is this agent live right
now?", and is sized for the ding's 30s heartbeat, hence ~2 minutes.
Both are defensible; they were never the same question.
That makes the tempting fix a trap: tightening STATUS_STALE_MS to
convoy's ~120s would flap every MCP-refreshed agent into `unknown`
between refreshes. So instead of one window with a contested value,
this names both:
- STATUS_LIVENESS_MS (2 min), the reader half of the R=30s/T~=120s
liveness contract the ding already documents, alongside the
existing STATUS_STALE_MS (15 min) trust window
- readIdentityLiveness(), one reader returning both verdicts plus the
recorded value and mtime, exported from the package index so
consumers inherit a definition instead of each inventing one
- `st agents` annotates rather than lies: `available (stale 3m)`,
and `unknown (was busy, 22m)` past the trust window
`recorded` deliberately survives both windows. An identity that was
`busy` and went quiet is not the same fact as one that cleanly went
`offline` — collapsing them loses exactly the signal you want when
something died mid-work.
SEMANTICS NOTE for reviewers: the derived `status` field is unchanged,
and so is what `--status` filters on. What changes is the rendered text
cell of `st agents` (now qualified when not live) and the JSON shape
(additive `live` / `statusMtimeMs` / `recorded`). Scripts parsing column
2 of the text output will see a qualifier they did not see before; that
is the deliberate, reviewable part of this change.
`statusMtimeMs` is an absolute mtime rather than an age so two
back-to-back reads of an unchanged bus still compare equal — an `ageMs`
field broke that invariant (caught by bus-reader's repeatability test).
Refs #102
agent-session-id: 0abcedc7-6b71-4046-9e7c-f645268c0b15
agent-tool: Claude Code
agent-tool-version: 2.1.215
agent-model: claude-opus-4-8
agent-runtime-profile: /nix/store/acr8a3l2v366jgmwiq8xdrhgz1py0db5-coding-agent-runtime-profile/share/coding-agents/profile.json
agent-skills-manifest: /nix/store/sj1v5j91h8v8d1w9lca4040302lwrd6v-agent-skills-corpus/share/agent-skills/manifest.json
tooling-profile: dotfiles@unknown-dirty
Refinement of the previous commit, kept separate because it reverses a
judgement call rather than extending one.
That commit rendered a non-live status as `available (stale 3m)`. But
`st agents` enumerates a MIXED population and the writers do not share
a cadence: a ding-backed agent's status is touched every 30s
(LIVENESS_HEARTBEAT_MS), while an MCP-server-backed agent only refreshes
every 5 minutes (STATUS_REFRESH_MS). Judged against a single 2-minute
window, a perfectly healthy MCP-backed agent reads `stale` for three of
every five minutes.
That is the same trap as tightening STATUS_STALE_MS to ~120s, one layer
up: it swaps "reports dead agents as available" for "reports live agents
as stale". Avoiding the first while walking into the second is not a
fix, and the previous commit message claimed the design avoided exactly
this.
So the roster now states the age and lets the reader judge:
available touched inside the liveness window
available (3m ago) older than that — stated, not judged
unknown (was busy, 22m ago) past the trust window
The age is true regardless of which writer an identity has, and it is
what the original report was actually missing: at the moment `st agents`
said `available` and convoy said `DEAD (status stale 3m ago)`, the age
was the fact that would have reconciled them.
A verdict still exists where it can be justified: `live` (per
STATUS_LIVENESS_MS) stays in --json, and `readIdentityLiveness` takes a
`livenessMs` override, so a consumer that KNOWS its agents run a ding
keeps a definite answer while inheriting one shared definition.
Refs #102
agent-session-id: 0abcedc7-6b71-4046-9e7c-f645268c0b15
agent-tool: Claude Code
agent-tool-version: 2.1.215
agent-model: claude-opus-4-8
agent-runtime-profile: /nix/store/acr8a3l2v366jgmwiq8xdrhgz1py0db5-coding-agent-runtime-profile/share/coding-agents/profile.json
agent-skills-manifest: /nix/store/sj1v5j91h8v8d1w9lca4040302lwrd6v-agent-skills-corpus/share/agent-skills/manifest.json
tooling-profile: dotfiles@unknown-dirty
#106 suspended the status heartbeat once a message was held past the hold cap, so peers stop reading `available` from an agent whose mail the sidecar demonstrably is not delivering. But the stall was cleared ONLY on a successful delivery (`normalDeliver` / `preserveDeliver`). The held message's most likely fate is never being delivered at all: the agent reads and archives it itself — the documented boot ritual is literally "drain your inbox". The buffered poke was then dropped as a stale poke via `continue`, no delivery ever happened, and the stall became permanent. The heartbeat never resumed even once the pane went idle with an empty inbox, because nothing was left to deliver and only a delivery could clear it. Net effect: #106 converted a transient "unreachable while busy" into a permanent "reads dead" for a perfectly healthy agent — worse than the defect it set out to fix. Fix: clear the stall on the drained checkpoints, which is precisely the condition "no message remains undeliverable" — not on any single message ceasing to need delivery, which would resume the heartbeat while a second message is still genuinely stuck. Archived-while-buffered events are now also pruned BEFORE the busy/dnd suppress-return rather than only inside the drain loop. The drain never runs while the identity is busy/dnd, so a busy agent archiving its own mail would otherwise leave the event parked in the buffer forever and the buffer would never drain — the stall would outlive its cause by that path even with the checkpoint clear. Also: `readIdentityLiveness` documented "single stat + single read" while performing 4 stats + 2 reads (it delegated to `readState`, which repeats the whole sequence). Worse, `status` and `recorded` came from two different reads, so a concurrent status write could return a pair that never existed on disk. Now derived from one stat + one read, with the caller's `now` governing the whole verdict instead of only `ageMs`/`live`. Tests: three new cases, each failing before this change — archived-while-held clears the stall; one-of-two archived does NOT (over-clearing guard); archived on a busy identity still clears (the suppress-gate path). agent-session-id: 0abcedc7-6b71-4046-9e7c-f645268c0b15 agent-tool: Claude Code agent-tool-version: 2.1.215 agent-model: claude-opus-4-8 agent-runtime-profile: /nix/store/acr8a3l2v366jgmwiq8xdrhgz1py0db5-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/sj1v5j91h8v8d1w9lca4040302lwrd6v-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@unknown-dirty
schickling-assistant
marked this pull request as ready for review
July 20, 2026 23:47
schickling-assistant
deleted the
schickling-assistant/2026-07-20-rapid-ada-17
branch
July 21, 2026 00:23
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Fixes the two defects blocking a reachable always-on agent. Two independent commits, reviewable and revertable separately.
Both issues turned out to be misdiagnosed in their write-ups. Neither original framing survived reproduction. That is the most important thing in this PR, so it is up front.
Defect 1 — #101: the sidecar was armed the whole time
What the issue said
What actually happens
The sidecar arms fine. The watcher arms, the startup scan runs, and delivery works. What is broken is that a message can be held indefinitely, silently, while the sidecar keeps advertising the agent as
available.Three things I checked before touching code:
session_startis not written by smalltalk at all. Nothing in this repo emits it — that log is convoy/pty sidecar telemetry. "Only onesession_startrecord" is not evidence that ding did nothing; ding simply does not write there. It was a red herring.target session "<id>" not yet registeredline does not gate delivery. I ran a ding against a target with no pidfile at all: it delivered normally. The session-watch only controls exit, and its startup grace suppresses that. Also a red herring.guardedDeliver. On the no-input branch, a changing pane frame returnsheldunconditionally.forceCapis computed at the top of the function but only ever consulted on the un-submitted-input branch — so this hold has no upper bound.assessPanecompares two fullpty peek --plainframes taken 300 ms apart with byte equality. Any difference means "mid-turn". A working agent's pane changes constantly, so every poke is held for as long as the agent keeps working — and the hold logs only underST_DING_DEBUG, which production does not set. MeanwhilestartStatusRefreshruns on a timer completely decoupled from delivery, bumping the status mtime the entire time.Net effect matches the report exactly: process alive, message sits in the inbox, status reads
available, stderr completely silent.Reproduced
Isolated temp root, stub
ptyonPATH, never against the live bus:Flip the stub pane to static and the message is delivered within one retry — so there is no deeper state bug; the hold does recover.
I also sampled real panes on this host read-only. Two of three were byte-changing at that instant; the Claude Code status line carries an elapsed clock (
⏱14h31m) and a cost counter. But an idle pane sampled 8× in a row was static every time.Correction to the issue's claim
The issue says an agent that "boots, drains, and goes idle is unreachable for the rest of its life." That over-states it. An idle pane is byte-static and a held message lands within ~20 s of the pane going idle. The accurate claim is narrower and still a blocker: an agent is unreachable for exactly as long as its pane keeps changing — i.e. the whole time it is working. For an always-on agent on long tasks that is effectively unbounded, and it is unbounded by construction since the cap is never consulted on this branch.
The fix, and what it deliberately does not do
The hold decision itself is correct and unchanged. Submitting into an active Claude Code turn seeds CC's queued-input replay bug, and there is an explicit regression test in this repo (
hold cap with a still-changing (mid-turn) frame → keeps HOLDING, never force-submits) forbidding a force-submit. That test still passes. Adding a hard time ceiling that force-submits would reintroduce precisely the bug the hold exists to prevent, so I did not.What was wrong is that the sidecar lied about it. So past the hold cap it now:
440b1f4.On the issue's (a)/(b)/(c) options
All three are premised on the sidecar failing to arm, which I disproved. Reframed: the violated invariant is not "unarmed writes
available" but "armed-but-not-delivering writesavailable".Defect 2 — #102: there is already a window; it is the wrong question
What the issue said
What actually happens
st agentsdoes apply a window. It reads throughreadIdentityStatus→readState, which appliesSTATUS_STALE_MSand collapses tounknown. Verified:So this is a threshold mismatch, not a missing check — which matters, because it changes what the correct fix is.
The two windows answer different questions:
STATUS_STALE_MSBoth are defensible. They were never the same question, and
st agentsat 3 minutes was answering the first one correctly.This makes the obvious fix a trap. Tightening
STATUS_STALE_MSto ~120 s to match convoy would flap every MCP-refreshed agent intounknownbetween refreshes.The fix — (c), plus (a)'s annotation
STATUS_LIVENESS_MS(2 min) named alongside the existingSTATUS_STALE_MS— the reader half of the R=30 s/T≈120 s contract the ding already documents incommon.tsreadIdentityLiveness(), one reader returning both verdicts plus the recorded value and mtime, exported from the package index so consumers inherit a definition instead of each inventing one. That is the part that actually stops the divergencest agentsstates the age of the value:available (3m ago), andunknown (was busy, 22m ago)past the trust windowThe roster reports a fact, not a verdict — this is deliberate and I got it wrong first. My initial commit rendered
available (stale 3m). Butst agentsenumerates a mixed population and the writers don't share a cadence: a ding-backed agent is touched every 30 s, an MCP-backed one only every 5 min. Judged against a single 2-minute window, a perfectly healthy MCP-backed agent readsstalefor three of every five minutes — the same trap as tighteningSTATUS_STALE_MS, one layer up, just swapping "dead agents read available" for "live agents read stale". The third commit reverses that: the age is true regardless of writer, and it is precisely what was missing whenst agentssaidavailableand convoy saidDEAD (status stale 3m ago). A verdict still exists where it can be justified —livestays in--json, andreadIdentityLivenesstakes alivenessMsoverride — so a consumer that knows its agents run a ding keeps a definite answer while inheriting one shared definition.On the issue's question 3 — yes, the distinction matters and is preserved.
recordedsurvives both windows, so an identity that wasbusyand went quiet stays distinguishable from one that cleanly wentoffline. That is exactly the signal you want when something died mid-work.statusfield, and what--statusfilters on. A stale-but-trustedavailablestill matches--status available. There is a test pinning this.st agentsnow carries an age once past the liveness window. Anything parsing column 2 of the text output will see something it did not see before.--jsongainslive/statusMtimeMs/recorded.statusMtimeMsis an absolute mtime rather than an age so two back-to-back reads of an unchanged bus still compare equal. AnageMsfield broke that — caught by bus-reader's existing repeatability test, which is a good test.Follow-up: a stall must not outlive its cause
An adversarial review found — and reproduced with a test — that the #101 fix as first written was worse than the defect it fixed. Fixed in
440b1f4.The blocker
clearDeliveryStall()was called from exactly two places:normalDeliveron success, andpreserveDeliver. Both are delivery paths. But the held message's most likely fate is never being delivered at all:continue) — but the stall is never cleared.The heartbeat then never resumes, not even after the pane goes idle with an empty inbox: there is nothing left to deliver, so nothing can ever clear it. A transient "unreachable while busy" became a permanent "reads dead" for a perfectly healthy agent.
Precondition, stated so it is not waved off: the cap must trip before the agent archives. Both are ordinary.
The fix
The stall is cleared on the drained checkpoints — the two
buffer.length === 0 && readPending.length === 0 → disarmTimer()points — which express exactly the condition "no message remains undeliverable".The tempting one-line fix (clear on the archived-while-held
continue) is wrong, and there is a test pinning that:deliveryStalledis a single per-daemon flag, so clearing it the moment any message is archived resumes the heartbeat while a second message is still genuinely stuck on the busy pane — re-introducing precisely the unearned liveness #101 exists to prevent. A single-message test passes straight over that regression.Archived-while-buffered events are additionally pruned before the busy/dnd suppress-return, not only inside the drain loop. The drain never runs while the identity is
busy/dnd, so a busy agent archiving its own mail would otherwise leave the event parked in the buffer forever — the buffer would never drain and the stall would outlive its cause by that path even with the checkpoint clear.Evidence
Three new tests, each failing before the fix and passing after (verified by reverting the fix, re-running, and restoring):
All three failed identically without the fix —
expected <mtime> to be greater than <mtime>, i.e. the heartbeat never resumed. The three pre-existing #101 tests pass in both states, so the fix does not weaken the original guarantee.Can the stall still outlive its cause?
One narrow residual, pre-existing and not introduced here: an entry stuck in
readPendingkeeps the drained checkpoint from firing. It requires a message that failsbuildEventpermanently. In practicecmdReadfalls back from inbox to archive, so the common archived-before-read case still reads fine, lands in the buffer, and is pruned normally — the residual needs a file present in neither folder, or genuinely unparseable. Deliberately not "fixed" by pruningreadPendingon!stillInInbox: an arrival is renamed into the inbox, so that predicate is briefly false mid-arrival and pruning on it would drop real messages. Flagging rather than silently trading a rare stuck heartbeat for a message-loss bug.readIdentityLiveness: the comment was lyingSame review, lower severity. The doc comment claimed "single stat + single read". It performed 4 stats + 2 reads — it delegated to
readState, which does its ownexistsSync+statSync+readFileSync, and then repeated the whole sequence itself.Worse than the cost:
statusandrecordedcame from two different reads, so a status write landing between them could return a pair that never existed on disk (e.g.status: busyalongsiderecorded: 'available').Now derived from one
stat+ oneread, with every field consistent by construction. The collapsedexistsSync+statSyncpair was itself a stat-then-stat TOCTOU. The caller's injectednownow governs the whole verdict rather than onlyageMs/live— previouslystatussilently used a secondDate.now().The two defects are coupled
Worth flagging: #101's fix depends on #102's window to bite. Suspending the heartbeat freezes the mtime, but with only the 15-minute trust window a sender still reads
availablefor 15 more minutes. WithSTATUS_LIVENESS_MS, the roster tells the truth in ~2. Verified end to end:That is the argument for shipping them together.
Verification
nix flake checkgreen as of the first two commits. Not re-run after the follow-up commit — that commit was gated ontsc -p tsconfig.build.json --noEmit(clean oversrc/), which is the same typecheckflake checkenforces, plus the full unit suite@compoundingtech/ptyfile:devDependency is absent from the dependency closure available here. ThereadIdentityLivenesschange is still directly covered by thestatusandstatus-stalenessunit files; what is unexercised is only the end-to-endst agentsrender surfacevitest run tests/unit); 16 new tests — 3 for Ding refreshes status for a dead provider — an unarmed sidecar forges liveness #101, 10 forst agentshas no freshness window — reports stale status as live #102, and 3 for the stall-outliving-its-cause follow-up.tsc -p tsconfig.build.json --noEmitclean oversrc/AgentSummarychange (three added keys, and those tests assert output shape), so I ran them explicitly in the nix sandbox where devDeps exist —tests/unit/mcp/, plusbus-reader/exports/overview/status: all greennix flake checkgateshelp,completions, andtypecheckonly — vitest is not a CI gate (see the comment inflake.nixabout Adopt the shared build-identity contract: st --version has no build identity, MCP serverInfo reports a different version #103). So the test evidence here is local, not enforced. Worth deciding separately whether that should change.tests/unit/common.test.tsfails to parse onmain—stConfig/stConfigFromare each imported twice. Worth a separate one-line fix. The other collection failures in my tree are just a missingnode_modules, not a code problem.Reproduced vs inferred
Reproduced: the unbounded silent hold and its full signature; that an unregistered target still delivers; that a held message recovers when the pane goes static; that
st agentsalready windows at 15 min and reportsavailableat 3 min; the stall → frozen heartbeat → aged roster → recovery lifecycle.Inferred: that this specific mechanism caused the production incident. The evidence fits tightly (
available+ silent + undelivered + alive is a signature nothing else in this code produces), and the production launch is exactlyst ding <host>.cos --identity <host>.cos --root <net>/smalltalkwith pane-guard defaults on — but the incident itself predates the diagnosis and I could not replay it.Corrected: "never arms" (it arms), "unreachable for the rest of its life" (only while the pane keeps changing), and "no freshness window" (there is one, sized for a different question).
Corrected in this PR's own earlier state: that the stall "resumes on the next successful delivery" was an inadequate clear condition, and that
readIdentityLivenessdid a "single stat + single read" when it did four and two. Both found by adversarial review, both reproduced with failing tests before being fixed.Testing safety
All testing ran against isolated temp roots with a stub
ptybinary, and the unit suite pinsPTY_SESSION_DIRto a fresh temp dir viatests/setup/pty-isolation.ts. The live hosted agent, its bus, and all 45+ production pty sessions were untouched —pty lscount 91 before and 91 after, across both the original work and the follow-up. No process was pattern-killed; only processes I created were killed. The one read-only touch of production waspty peek --plainto characterize whether real panes are byte-static.Closes #101
Refs #102 — deliberately not
Closes. The issue was raised as a consensus question because it changes what a command means, so the semantics note above is a proposal, not a settled call. Close it if the shape is agreed.