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
base: main
Choose a base branch
from
Draft

fix: make an always-on agent reachable, and stop reporting liveness it hasn't earned (reopened from #106) #108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 96 additions & 7 deletions src/commands/agents.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,29 @@ import {
} from '../common.ts';
import { type State } from '../types.ts';

import { readIdentityStatus } from './status.ts';
import { readIdentityLiveness } from './status.ts';

export interface AgentSummary {
/** The agent's name. Field kept as `identity` for back-compat with
* embedder destructures; will rename to `agent` in a follow-up. */
identity: string;
/** Derived, trust-windowed state — UNCHANGED semantics. Still what
* `--status` filters on. See `live`/`statusMtimeMs` for freshness. */
status: State;
name: string | null;
/** #102: freshness, so a roster reader can't mistake a stale value
* for a current one. `live` uses the shared STATUS_LIVENESS_MS
* window; `recorded` preserves the on-disk value across it so a
* stale-but-was-`busy` agent stays distinguishable from one that
* cleanly went `offline`.
*
* Note `statusMtimeMs` is the ABSOLUTE mtime, not an age. An age
* would make two back-to-back reads of an unchanged bus return
* different data; callers render the relative form themselves (see
* `formatAge`). Mirrors how `lastActivity` already reports mtimes. */
live: boolean;
statusMtimeMs: number | null;
recorded: State | null;
}

export interface AgentSummaryEnriched extends AgentSummary {
Expand Down Expand Up @@ -82,11 +97,17 @@ export function getAgents(
opts: GetAgentsOpts = {}
): AgentSummary[] | AgentSummaryEnriched[] {
const ids = listAgents(root);
const base: AgentSummary[] = ids.map((id) => ({
identity: id,
status: readIdentityStatus(id, root),
name: readNameFile(id, root),
}));
const base: AgentSummary[] = ids.map((id) => {
const l = readIdentityLiveness(id, root);
return {
identity: id,
status: l.status,
name: readNameFile(id, root),
live: l.live,
statusMtimeMs: l.mtimeMs,
recorded: l.recorded,
};
});
const filtered =
opts.status !== undefined && opts.status !== ''
? base.filter((m) => m.status === opts.status)
Expand Down Expand Up @@ -252,6 +273,18 @@ function agentsHelp(name: string): string {
' --status STATE only agents in STATE (available|busy|away|dnd|offline).\n' +
' --json machine-readable array.\n' +
' --enrich (with --json) add inbox counts + last-activity.\n\n' +
' Freshness: a status value is only as good as its mtime, so one\n' +
' that has not been touched recently is shown with its age:\n' +
' available touched recently\n' +
' available (3m ago) last touched 3m ago\n' +
' unknown (was busy, 22m ago) too old to trust; last claimed `busy`\n' +
' The age is a fact, not a verdict: agents have different status\n' +
' writers (a ding heartbeat touches every 30s, an MCP server every\n' +
' 5min), so how old is too old depends on the agent. --json carries\n' +
' `live` (per the shared STATUS_LIVENESS_MS window) plus\n' +
' `statusMtimeMs` / `recorded` for consumers that know their own\n' +
' cadence. The `status` field itself is unchanged, as is what\n' +
' --status filters on.\n\n' +
' Examples:\n' +
` ${name} agents # id / status / name, tab-separated\n` +
` ${name} agents --status available # only agents marked available\n` +
Expand Down Expand Up @@ -300,11 +333,67 @@ export function cmdAgentsCli(
return 0;
}
for (const m of r.items) {
ctx.stdout(`${m.identity}\t${m.status}\t${m.name ?? ''}\n`);
ctx.stdout(
`${m.identity}\t${renderStatusCell(m)}\t${m.name ?? ''}\n`
);
}
return 0;
}

/**
* #102: the status cell, annotated with the AGE of the value.
*
* `st agents` used to print the derived state bare, so a status file
* last touched minutes ago rendered identically to one touched a second
* ago. The value was never wrong exactly — it was unqualified, and a
* roster command is precisely where "as of when?" matters.
*
* Deliberately a FACT ("3m ago"), not a VERDICT ("stale"). `st agents`
* enumerates every agent under the root, and different agents have
* different status writers with different cadences: the ding heartbeat
* touches every 30s, but an MCP-server-backed agent only refreshes
* every STATUS_REFRESH_MS (5 min). A single global "this one is stale"
* verdict cannot be right for both — calling a healthy MCP agent stale
* for 3 of every 5 minutes would just be a new way of being wrong.
*
* So the roster reports the age and lets the reader judge. A consumer
* that KNOWS its agents run a ding (convoy) applies STATUS_LIVENESS_MS
* via the exported `live` field / `readIdentityLiveness`, which is
* where a verdict can actually be justified.
*
* Shapes:
* available touched inside the liveness window
* available (3m ago) older than that — stated, not judged
* unknown (was busy, 22m ago) past the trust window; derived state is
* `unknown`, but we still say what it last
* claimed, because "was busy and went
* quiet" and "cleanly offline" are
* different facts
* offline no status file at all; nothing to age
*/
function renderStatusCell(m: AgentSummary, now = Date.now()): string {
if (m.statusMtimeMs === null) return m.status; // no file → nothing to qualify
if (m.live) return m.status;
const age = `${formatAge(now - m.statusMtimeMs)} ago`;
// Trust window blew past: `status` is `unknown`, so surface what the
// file still records rather than dropping the signal entirely.
if (m.status === 'unknown' && m.recorded !== null) {
return `unknown (was ${m.recorded}, ${age})`;
}
return `${m.status} (${age})`;
}

/** Compact age: `45s`, `3m`, `2h`, `4d`. */
export function formatAge(ms: number): string {
const s = Math.max(0, Math.round(ms / 1000));
if (s < 60) return `${s}s`;
const m = Math.round(s / 60);
if (m < 60) return `${m}m`;
const h = Math.round(m / 60);
if (h < 24) return `${h}h`;
return `${Math.round(h / 24)}d`;
}

// ─── Deprecated aliases (brief-009 item 3) ─────────────────────────────
//
// `members` was renamed to `agents`. The old names remain as
Expand Down
112 changes: 112 additions & 0 deletions src/commands/ding.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -478,8 +478,64 @@ export async function runDing(deps: DingDeps): Promise<void> {
| { kind: 'failed' }
| { kind: 'held'; inputText: string; staleCount: number };

// ─── Delivery-stall tracking (#101) ───────────────────────────────
//
// The pane guard deliberately NEVER force-submits into a frame that
// is still changing: a submit landing mid-turn seeds Claude Code's
// queued-input replay bug (see the comment in `guardedDeliver` and
// the "keeps HOLDING, never force-submits" regression test). That
// decision stands — but it means a target pane that keeps changing
// holds every poke for as long as it keeps changing, with no upper
// bound.
//
// Before this fix that state was INVISIBLE and, worse, DISHONEST:
// the hold logged only under ST_DING_DEBUG, while the status-refresh
// tick (which is entirely decoupled from delivery) kept bumping the
// identity's status mtime. Senders therefore read `available` from an
// agent whose mail the sidecar was demonstrably not delivering.
//
// `deliveryStalled` closes that gap. Once a message has been held
// PAST the hold cap, the sidecar has proven it cannot currently
// deliver, so it (a) says so loudly, once, on stderr and (b) stops
// refreshing the status file. The mtime then freezes and readers
// derive staleness through the normal path — the same death-coupling
// the session-gone watch already relies on. A successful delivery
// clears the stall and the heartbeat resumes.
//
// Invariant: a sidecar must not write liveness it has not earned.
let deliveryStalled = false;
/** Loud-log the stall exactly once per stall episode, not per retry. */
let loggedStall = false;

function markDeliveryStalled(ev: BufferedEvent, holds: number): void {
deliveryStalled = true;
if (loggedStall) return;
loggedStall = true;
log(
`st ding: DELIVERY STALLED — "${ev.filename}" has been held ` +
`${holds} times (cap ${maxHolds}) because pty session ` +
`"${deps.ptySession}" is never static long enough to submit ` +
`into safely. The message is NOT lost — it stays in the inbox ` +
`and is retried every ${holdRetryMs}ms — but it will not be ` +
`delivered until the pane goes idle. Suspending the status ` +
`heartbeat for "${deps.identity}" so peers stop reading this ` +
`agent as available while its mail is undeliverable.\n`
);
}

function clearDeliveryStall(): void {
if (!deliveryStalled) return;
deliveryStalled = false;
loggedStall = false;
log(
`st ding: delivery recovered for "${deps.identity}" — resuming ` +
`the status heartbeat.\n`
);
}

async function normalDeliver(ev: BufferedEvent): Promise<GuardOutcome> {
const ok = await deliver(send, deps.ptySession, ev, log);
if (ok) clearDeliveryStall();
return ok ? { kind: 'delivered' } : { kind: 'failed' };
}

Expand Down Expand Up @@ -525,6 +581,7 @@ export async function runDing(deps: DingDeps): Promise<void> {
return { kind: 'failed' };
}
dbg(`preserve-delivered ${ev.filename} (kept un-submitted input)`);
clearDeliveryStall();
return { kind: 'delivered' };
}

Expand Down Expand Up @@ -577,6 +634,10 @@ export async function runDing(deps: DingDeps): Promise<void> {
if (!hasInput) {
if (a.frameChanged) {
dbg(`frame changing (mid-turn) → holding ${ev.filename} (hold ${holds + 1}; cap won't force into an active turn)`);
// #101: the hold itself is correct (never submit mid-turn), but
// past the cap the sidecar has proven it is not delivering —
// say so, and stop asserting liveness we have not earned.
if (forceCap) markDeliveryStalled(ev, holds + 1);
return { kind: 'held', inputText: '', staleCount: 0 };
}
return normalDeliver(ev); // frame static (idle / walked away) → safe to submit
Expand All @@ -594,6 +655,9 @@ export async function runDing(deps: DingDeps): Promise<void> {
ev.lastInputText !== inputText;
if (changed) {
dbg(`pane active (frame/input changing) → holding ${ev.filename} (hold ${holds + 1})`);
// #101: same invariant as the no-input branch above — a pane that
// stays active past the cap means we are not delivering.
if (forceCap) markDeliveryStalled(ev, holds + 1);
return { kind: 'held', inputText, staleCount: 1 };
}
const staleCount = (ev.inputStaleCount ?? 1) + 1;
Expand Down Expand Up @@ -846,6 +910,19 @@ export async function runDing(deps: DingDeps): Promise<void> {
deps.statusRefreshIntervalMs ?? LIVENESS_HEARTBEAT_MS;
let statusRefreshTimer: ReturnType<typeof setInterval> | undefined;
function runStatusRefreshTick(): void {
// #101 invariant: a sidecar must not write liveness it has not
// earned. While delivery is stalled (a message held past the cap on
// a pane that never goes static) this sidecar is NOT surfacing the
// agent's mail — refreshing the mtime here would advertise an
// availability it cannot honor. Skip the touch instead: the mtime
// freezes, and every reader derives staleness through the existing
// path. `markDeliveryStalled` already logged the reason once.
if (deliveryStalled) {
dbg(
`status refresh suppressed for "${deps.identity}" — delivery stalled`
);
return;
}
const outcome = refreshIdentityStatus(deps.identity, deps.st.root);
if (outcome === 'error') {
log(
Expand Down Expand Up @@ -895,8 +972,37 @@ export async function runDing(deps: DingDeps): Promise<void> {
}
}
}
// #106 follow-up: prune messages the agent archived while they
// were buffered, BEFORE the status gate below. Two reasons this
// cannot wait for the drain loop's own `stillInInbox` check:
//
// 1. The drain never runs while the identity is busy/dnd (the
// SUPPRESS_STATES return below). A busy agent that reads and
// archives its own mail — literally the documented boot
// ritual, "drain your inbox" — would otherwise leave the
// archived event parked in the buffer indefinitely.
// 2. `deliveryStalled` is cleared on the drained checkpoints
// below. If archived events are never pruned, the buffer
// never drains, and a stall outlives the message that caused
// it — permanently suspending the heartbeat of a healthy
// agent whose inbox is empty.
//
// Archived means "no longer needs delivery" regardless of status,
// so this prune is status-independent by construction.
for (let i = buffer.length - 1; i >= 0; i -= 1) {
if (!stillInInbox(buffer[i]!.filename)) {
dbg(
`buffered message ${buffer[i]!.filename} archived while held → dropping stale poke`
);
buffer.splice(i, 1);
}
}
if (buffer.length === 0 && readPending.length === 0) {
disarmTimer();
// Nothing is awaiting delivery, so nothing is undeliverable:
// the stall's cause is gone and the stall must not outlive it.
// (No-op unless a stall is actually up.)
clearDeliveryStall();
return;
}
let state: State;
Expand Down Expand Up @@ -969,6 +1075,12 @@ export async function runDing(deps: DingDeps): Promise<void> {
if (deferred.length > 0) buffer.push(...deferred);
if (buffer.length === 0 && readPending.length === 0) {
disarmTimer();
// Same invariant as the pre-gate checkpoint above: the buffer
// drained, so no held message remains undeliverable. Note this
// deliberately does NOT fire while a second message is still
// deferred — a stall must survive as long as ANY message is
// still undeliverable, which is the whole point of #106.
clearDeliveryStall();
}
} finally {
flushing = false;
Expand Down
Loading
Loading