Skip to content

fix: make two processes sharing one mailbox visible (#161) - #169

Closed
yonidavidson wants to merge 3 commits into
perf/167-bounded-mirrorfrom
fix/161-mailbox-leases
Closed

yonidavidson wants to merge 3 commits into
perf/167-bounded-mirrorfrom
fix/161-mailbox-leases

Conversation

@yonidavidson

Copy link
Copy Markdown
Owner

Closes #161. Stacked on #168#166#165 (rebases down to main as those merge).

A subagent inherits its parent's git identity and process tree, so it derives the parent's alias — and because reads consume, one agentcomm inbox from a subagent drains the mailbox its parent is waiting on. Silently, because the alias looks right.

The one suggestion not taken, and why

The issue proposes deriving the alias from something per-process (pid, spawn nonce). That would break the model deliberately: a session is a mailbox, and the sticky per-session fingerprint is exactly what fixed alias drift in #157 — making the name per-process would re-split every mailbox the moment a wrapper process appears. What was actually missing is visibility, which is the issue's own second and third suggestions.

What changed

  • src/session.ts — mailbox leases. While a process acts as an alias it leaves lease-<alias>-<pid>.json in the state dir; a lease whose pid is dead (or older than 30 min) is ignored and removed, never inherited. Local files, because two processes of one agent session on one machine is exactly the local case.
  • src/cli.tsguardMailbox classifies each command: a consuming read (inbox, ack) warns that it is taking mail addressed to a live holder, names what that process is doing, and gives the remedy (--as <alias>-<role>); a status write warns whose roster line it replaces; heartbeats, peek, wait and claim lease quietlywait because holding the mailbox is the whole point of a listener, claim because a shared queue is meant to have many claimers.
  • The guidance file written into repos now says a subagent derives the same alias as its parent.

Tests

test/mailbox-lease.e2e.test.ts: a real second process holds the alias while a consuming read runs (warns, with the remedy), a solo read stays silent and cleans up its lease, a status write reports whose line it takes while a heartbeat does not, and a dead process's lease is ignored and removed.

`peek` shows messages without consuming; `inbox` consumes what it shows.
With only those two there was no way to mark mail read that arrived any
other way, so an agent that had read all five messages via `peek` and
replied to every one still showed five unread — and the stop guard blocked
every turn for fifteen turns demanding it read them.

`agentcomm ack <id…>` / `ack --all` archives pending messages by id. It
works off the KEYS (the id is in the key), so nothing is re-fetched, and
ids that are not pending for you are reported with a non-zero exit instead
of passing silently — acking someone else's mail is not a quiet no-op.

`peek` now closes with the one-liner that clears what you handled, the
stop guard says the same thing, and the guidance file teaches peek+ack as
the non-destructive read path.
The daemon mirrored the entire bus — every key AND every body — and
refreshed it on every poll. On a snapshot-capable backend that is one round
trip, but it read every blob in the store each time: pending mail, the
roster, the whole 30-day archive, and every telemetry batch. The cost of a
poll therefore grew with all history, forever, on a bus where nothing but
the last few messages is hot.

The mirror now holds what is worth holding. `agents/` is re-read every poll
(records mutate); `inbox/` is always warm; `read/` and `events/` are warm
only inside a window (AGENTCOMM_MIRROR_HISTORY_MS, default 7 days). Older
history keeps its KEY, so list, log, purge and channel discovery see the
whole store exactly as before, and a cold body loads on demand through the
`get` passthrough. Message blobs are immutable, so a body already held is
never re-read either: steady state is "the roster plus whatever is new".

`Snapshottable.snapshot` grew a body filter and now reports the keys it saw
alongside the bodies it read — listing names is cheap, reading blobs is not.
A subagent inherits its parent's git identity and process tree, so it
derives the parent's alias. Because reads consume, one `agentcomm inbox`
from a subagent drains the mailbox its parent is waiting on — silently,
since the alias looks exactly right. One did; it noticed only because the
CLI echoed the parent's name and the agent recognised it. The smaller
version of the same thing: a subagent's `register --status` overwrote the
parent's line on the shared roster.

Identity stays per-session on purpose — a session IS a mailbox, and the
sticky fingerprint that fixed alias drift depends on it. What was missing
is visibility, so while a process acts as an alias it now leaves a local
lease behind, and anything destructive another live process does to that
mailbox says so: a consuming read warns that it is taking mail addressed
to a live holder and names the remedy (--as <alias>-<role>), and a status
write warns whose line it is replacing. Heartbeats, peeks and queue claims
lease quietly — a shared queue is meant to have many claimers.

Leases are local files, because the case they cover — two processes of one
agent session on one machine — is exactly the local case. A lease from a
dead process is ignored and cleaned up, never inherited.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant