Skip to content
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 (reopened from #106) - #108

Draft
schickling-assistant wants to merge 1 commit into
mainfrom
schickling-assistant/2026-07-21-reopen-106
Draft

fix: make an always-on agent reachable, and stop reporting liveness it hasn't earned (reopened from #106)#108
schickling-assistant wants to merge 1 commit into
mainfrom
schickling-assistant/2026-07-21-reopen-106

Conversation

@schickling-assistant

@schickling-assistant schickling-assistant commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Restores the content of PR #106, which was merged by an agent account without the maintainer's review or agreement, and was therefore reverted in PR #107. This reopens it as a draft so it can be reviewed properly before landing.

The diff is byte-identical to what #106 contained — no content was changed while re-landing it.

Refs #101, #102.

The two problems

1. A ding sidecar could advertise available while sitting on an undelivered message. st ding refreshed the watched identity's status mtime on a timer entirely decoupled from whether it was actually delivering. Root cause is in guardedDeliver: on the no-input branch a changing frame returns held unconditionally, and forceCap is computed but only 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. With ST_DING_DEBUG off — the production posture — stderr was completely silent. A sender read a healthy, available recipient and got nothing.

The hold decision itself is correct and is kept: 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 it now warns loudly on stderr and suspends the status heartbeat, freezing the mtime so readers derive staleness through the existing path. Invariant: a sidecar must not write liveness it has not earned.

2. st agents reported available for an identity that convoy ls --tree called DEAD (status stale 3m ago) at the same instant. Worth noting for review: the original issue's premise — that st agents applies no freshness window — turned out to be wrong. It reads through readIdentityStatusreadState, which already applies STATUS_STALE_MS. This is a threshold mismatch, not a missing check.

The two windows answer different questions: the existing one asks "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 asks "is this agent live right now?" and is sized for the ding's 30s heartbeat, hence ~2 minutes. Tightening STATUS_STALE_MS to ~120s would flap every MCP-refreshed agent into unknown between refreshes. So this names both windows instead: STATUS_LIVENESS_MS (2 min) alongside STATUS_STALE_MS (15 min), and adds 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.

Please look closely at these

The roster states age rather than rendering a verdict. The third commit reverses a judgement call from the second, and is kept separate for that reason. The second commit rendered a non-live status as available (stale 3m) — but st agents enumerates a mixed population whose writers do not share a cadence, so a perfectly healthy MCP-backed agent (5-min refresh) judged against a 2-minute window reads stale for three of every five minutes. That is the same trap one layer up: swapping "reports dead agents as available" for "reports live agents as stale". 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

A verdict still exists where it can be justified: live stays in --json, and readIdentityLiveness takes a livenessMs override so a consumer that knows its agents run a ding keeps a definite answer.

Semantics change worth reviewing deliberately. 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.

The fourth commit fixes a defect the first one introduced — please check the reasoning holds. Suspending the heartbeat on a held message was cleared only on successful delivery. But a held message's most likely fate is never being delivered at all: the agent reads and archives it itself (the documented boot ritual is "drain your inbox"), the buffered poke is dropped as stale, and the stall becomes permanent. Net effect: a transient "unreachable while busy" became a permanent "reads dead" for a healthy agent — worse than the defect it set out to fix. The stall is now cleared on the drained checkpoints, which is precisely "no message remains undeliverable", rather than on any single message ceasing to need delivery (which would resume the heartbeat while a second message is still genuinely stuck).

Also in that commit: readIdentityLiveness documented "single stat + single read" while performing 4 stats + 2 reads, and 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.

recorded deliberately survives both windows: an identity that was busy and went quiet is not the same fact as one that cleanly went offline. 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).

Verification

The ding stall was reproduced against an isolated root with a stub pty, never against a live bus.

On this branch: nix flake check (the CI gate) passes — completions, typecheck, help. Unit suite green at 49 files / 1323 tests. The repo's integration suite has 16 failing files / 26 failing tests, but those are pre-existing and environmental — main fails the identical set with or without this change.

Note on deployment

This never reached production. The consuming flake.lock pins smalltalk at 850266f8 (the merge of #105), which is below #106, so no deployed system ever ran this code.

…s it hasn't earned

Restores the content of PR #106, which was merged without review and
reverted in PR #107. Opened as a draft for proper review.

No content changes from the original: the resulting tree is
byte-identical to e646250.

Refs #101, #102
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant