Skip to content
This repository was archived by the owner on Aug 10, 2026. It is now read-only.

fix(agent): pending-inbound queue + replay (#121) and thread-as-container working-ensure - #122

Merged
unforced merged 3 commits into
mainfrom
ag-thread-as-container
Jun 20, 2026
Merged

fix(agent): pending-inbound queue + replay (#121) and thread-as-container working-ensure#122
unforced merged 3 commits into
mainfrom
ag-thread-as-container

Conversation

@unforced

Copy link
Copy Markdown
Contributor

Two coherent parts plus a supporting finding. Central, subtle code — heavily commented, especially the turn_count and pending-replay logic. No version bump.

Part A — pending-inbound queue + replay-on-register (fixes #121; agent-side, NO vault change)

The bug. In daemon.ts contextFor.emit, an inbound for a channel with no live programmatic agent (programmatic.hasChannel(channel) === false — a just-defined agent not yet instantiated, or a brief channel/agent desync) fell through to a 0-subscriber SSE/MCP push and was silently dropped, while /api/vault/inbound still returned 200. The vault trigger acks 200 as done (stamps <trigger>_rendered_at) and never retries → permanent loss (0 turns, 0 threads, no reply).

The fix. The daemon now OWNS an inbound it can't yet process:

  • ProgrammaticAgentRegistry gains a per-channel pending-inbound buffer (FIFO, capped at PENDING_INBOUND_CAP = 50; past the cap the oldest is evicted with a loud log — bounded loss beats unbounded growth, and the durable inbound notes still exist in the vault), gated by an expectedChannels set.
  • New registry API: expectChannel / unexpectChannel / queuePending(channel,msg) → "queued"|"unknown" / pendingCount / isExpected.
  • register() drains the channel's pending buffer through the normal enqueue path (FIFO, arrival order) once the agent is live, then clears the expected mark (the live byChannel index is the truth).
  • emit (daemon.ts) wires it: no live agent → queuePending; "queued" returns (200, owned); "unknown" falls through to the existing push (a genuine telegram/bridge channel still works). The handler always 200s — a 4xx/5xx would strand the trigger in _pending_at.
  • buildInstantiateDeps.ensureChannel calls expectChannel before bringing the channel live (closing the desync window); deregister calls unexpectChannel.

In-memory is acceptable for v1: a daemon restart loses pending, but the durable inbound notes exist and loadAll + the 60s def-poll reconverge.

Part B — thread-as-container (definition → thread → message)

The #agent/thread note is now a CONTAINER ensured when the agent starts processing a message, then UPDATED by the turn — not a post-turn by-product.

API change — phase: "start" | "end" on ThreadRecord (transport.ts) / ThreadNote (registry.ts):

  • drain writes a working-ensure (status: "working", input shown, NO reply) with phase: "start" BEFORE deliver().
  • The existing post-turn recordThread writes the final record with phase: "end" (the default when absent — back-compat).

turn_count counted EXACTLY ONCE (on end), never double-counted across start+end — in VaultTransport.writeThread:

  • phase: "start"turn_count = prior (single) / 0 (multi); last_turn_at NOT advanced; started_at preserved/initialized.
  • phase: "end" → single increments prior + 1 (unless sameTurn, the outbound-failure re-record); multi = 1; last_turn_at advanced.
  • Trace: turn 1 start→0, end→1; turn 2 start→1 (unchanged), end→2; ok→error re-record stays 1.

The start-ensure and end-record share the per-turn threadId, so single-threaded UPSERTS its deterministic note and multi-threaded targets the SAME per-fire note (no duplicate). buildThreadSummaryBody handles the working state (no fake reply).

Latent-bug fix: buildWriteThread (daemon.ts) previously dropped threadId and sameTurn — so through the real daemon path a multi-threaded re-record would mint a duplicate note and single-threaded sameTurn would double-count. It now forwards threadId + sameTurn + phase.

Message ↔ thread linkage: the OUTBOUND #agent/message note is stamped with metadata.thread (the per-turn thread id), threaded WriteOutboundreply() via meta.thread. For multi-threaded this IS the per-fire note leaf. Deferred: INBOUND-note stamping (those notes are written externally before the turn knows its thread); single-threaded outbound→note-by-stable-path. Not blocking.

Part C — desync finding (supporting)

agent-defs.ts loadAll re-instantiates every enabled def on boot AND the 60s poll (instantiate always calls ensureChannel + setupAndRegister; register() is idempotent-replace). So any channel/agent desync self-heals within one poll cycle; the reload webhook is the fast path. Part A's expect+replay closes the residual window (inbound landing between channel-live and agent-register). No code change needed beyond Part A's expectChannel wiring.

Tests & gate

bun run test (typecheck + bun test ./src) = 995 pass, 0 fail.

  • Part A: inbound for an expected-but-unregistered channel is QUEUED (not dropped) and the handler 200s; register() drains FIFO + runs turns in order; cap enforced; unknown channel logs + 200s (no crash); channel-move clears old marks/buffer.
  • Part B: working-ensure write happens BEFORE deliver (gated-turn assertion); status working→ok/error; turn_count no-double-count at the transport (single 1→2, multi 0→1, sameTurn stays 1); start+end target the same note; working body has no fake reply; outbound stamped with thread id.
  • Regression: existing thread/drain/outbound-failure tests (incl. FIX-1 re-record + turn_count) updated to be phase-aware via an ends()/starts() filter — they still verify their substance with the added working-ensure.

Review

Independent reviewer pass: LGTM with nits, no critical issues. Both substantive nits fixed in the second commit (channel-move expected/pending leak + working-body wording); the turn_count invariant trace was independently verified.

unforced and others added 3 commits June 20, 2026 00:15
…ead-as-container working-ensure

Two coherent parts.

PART A — pending-inbound queue + replay-on-register (fix #121; agent-side, no vault change)
The daemon must OWN an inbound it can't yet process, never drop it. Before this, if
ctx.emit found no live programmatic agent for a channel, the inbound fell through to a
0-subscriber SSE/MCP push and was silently dropped — yet the handler still returned 200,
which the vault trigger acks as done (stamps `_rendered_at`) and NEVER retries. Permanent
loss: 0 turns, 0 threads, no reply.

ProgrammaticAgentRegistry now holds a per-channel PENDING-INBOUND buffer (FIFO, capped at
PENDING_INBOUND_CAP=50, oldest-evicted past the cap with a loud log) gated by an
expected-channels set:
- expectChannel(channel) — the def-instantiation path (buildInstantiateDeps.ensureChannel)
  marks a channel EXPECTED before bringing the channel transport live, closing the desync
  window where the channel is live but the agent isn't yet register()ed.
- queuePending(channel, msg) → "queued" (expected; buffered, owned, will replay) or
  "unknown" (nothing maps here; caller logs + falls through to the push path, still 200).
- register() drains the channel's pending buffer through the NORMAL enqueue path in arrival
  order, then clears the expected mark (the live byChannel index is the truth).
- unexpectChannel(channel) clears a stale mark + stranded buffer on teardown (deregister).

ctx.emit wires this: no live programmatic agent → queuePending; "queued" returns (200,
owned); "unknown" falls through to the existing push (a genuine telegram/bridge channel
still works). The handler keeps returning 200 throughout (a 4xx/5xx would strand the vault
trigger in `_pending_at`). In-memory is acceptable for v1: a daemon restart loses pending,
but the durable inbound notes still exist and loadAll + the 60s def-poll reconverge.

PART B — thread-as-container (definition → thread → message)
The #agent/thread note is now a CONTAINER ensured when the agent STARTS processing a
message, then UPDATED by the turn — not a post-turn by-product. drain() writes a working
thread note (status:working, input shown, NO reply) BEFORE deliver(); the existing
recordThread after the turn updates it to ok/error.

A new `phase: "start" | "end"` field on ThreadRecord/ThreadNote carries the lifecycle:
- phase:start (working-ensure) — status working; turn_count UNCHANGED (single: prior;
  multi: 0); last_turn_at NOT advanced.
- phase:end (default when absent — back-compat) — the final record; turn_count counted
  here (single increments unless sameTurn; multi=1); last_turn_at advanced.
So turn_count is counted EXACTLY ONCE per turn (on end) — never double-counted across the
start+end pair. The start-ensure and end-record share the per-turn threadId, so
single-threaded UPSERTS its deterministic note and multi-threaded targets the SAME per-fire
note (no duplicate). buildThreadSummaryBody handles the working state (no fake reply).

buildWriteThread now forwards threadId + sameTurn + phase (it previously dropped threadId
and sameTurn — a latent gap that would have duplicated the multi-threaded re-record note
and double-counted single-threaded sameTurn through the real daemon path).

Message↔thread link: the OUTBOUND #agent/message note is stamped with metadata.thread (the
per-turn thread id, threaded WriteOutbound → reply() via meta.thread). For multi-threaded
this IS the per-fire note leaf. INBOUND-note stamping is DEFERRED (externally written before
the turn knows its thread); single-threaded outbound→note-by-stable-path is a follow-up.

PART C — desync finding
agent-defs loadAll re-instantiates EVERY enabled def on boot AND the 60s poll (instantiate
always calls ensureChannel + setupAndRegister; register() is idempotent-replace). So any
channel/agent desync self-heals within one poll cycle; the reload webhook is the fast path.
Part A's expect+replay closes the residual window (inbound landing between channel-live and
agent-register). No code change needed beyond Part A's expectChannel wiring.

Tests: full gate green — `bun run test` (typecheck + bun test ./src) = 994 pass, 0 fail.
Existing thread/drain/outbound-failure tests updated to be phase-aware (assert the FINAL
records via an ends()/starts() phase filter) so they still verify their substance with the
added working-ensure. New tests cover: pending queue + FIFO replay + cap + unknown-channel
+ daemon 200-not-drop; working-ensure-before-deliver ordering; turn_count no-double-count
across start+end (single turns 1→2, multi 0→1); same-note targeting; working body has no
fake reply; outbound thread-id stamp.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…anup + working-body wording

- register() channel-move path now also drops the OLD channel's expectedChannels mark +
  pending buffer alongside byChannel/queues/draining (was a latent leak if a def's wake
  channel ever changes between polls). Adds a regression test.
- buildThreadSummaryBody working state no longer prints a confusing "0 turns, currently
  working" — it reads "working on the first turn" / "next turn" instead.

Gate: bun run test = 995 pass, 0 fail.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…g + full-lifecycle turn_count test

- deregister() now clears expectedChannels + pending for the channel (was only
  done by the daemon teardown wrapper) — closes the latent footgun for direct
  registry callers: a pending message after deregister had nothing to drain into.
- Add the one missing transport test: the full start→end(ok)→end(error,sameTurn)
  lifecycle, asserting turn_count goes 0→1→1 (start never counts; sameTurn never
  re-counts) — the phase+FIX-1 combination the prior tests didn't exercise.

Gates: bun run test 996 pass / 0 fail; typecheck clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@unforced
unforced merged commit 2af1c14 into main Jun 20, 2026
@unforced
unforced deleted the ag-thread-as-container branch June 20, 2026 06:26
unforced added a commit that referenced this pull request Jun 20, 2026
…r substrate (#123)

* feat(agent): agent-to-agent callback routing (reply_to) — orchestrator substrate

Request/response between agent threads. An inbound #agent/message/inbound note MAY
carry metadata.reply_to (the sender's channel), plus optional correlation_id and
delegation_depth. When the recipient's programmatic turn finishes (BOTH ok and error),
the daemon delivers a lightweight CALLBACK back to the reply_to channel — a brief
notification + LINK (source_thread / source_message) the orchestrator pulls the full
result from, NOT the duplicated reply.

Loop safety (3 layers): the callback note never carries reply_to (terminal, structural);
delegation_depth ceiling (MAX_DELEGATION_DEPTH=8) bounds runaway chains; an unknown
reply_to channel reuses the #122 own-it-don't-strand posture (log + no throw).

Concurrency: N callbacks returning to one orchestrator channel drain FIFO via the
existing per-channel serial drain (#122) — never concurrent, none lost; the orchestrator's
--resume session carries state across them.

- registry.ts: QueuedMessage gains replyTo/correlationId/delegationDepth; WriteCallback
  seam + CallbackMeta contract + MAX_DELEGATION_DEPTH; maybeDeliverCallback at all four
  drain terminal points; WriteOutbound return widened to surface the outbound note id
  for source_message (back-compat — void is in the union).
- daemon.ts: callbackFieldsFromMeta (extract + string->int coerce); contextFor.emit
  threads the fields onto the enqueue + pending paths; buildWriteCallback (resolve
  reply_to channel transport, own-it on unknown); buildWriteOutbound returns the note id.
- transport.ts: Transport.writeCallback? + CallbackMetadata.
- vault.ts: VaultTransport.writeCallback (writes a callback inbound note, strips any
  stray reply_to); writeInbound gains optional extraMeta.
- design/2026-06-20-agent-callbacks.md: the model, metadata contract, loop safety,
  concurrency story, summary+link rationale, deferred delegate MCP tool.

Gate: bun run test (typecheck + bun test ./src) green — 1017 pass, 0 fail.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(agent): clarify depth-0 omission + concurrency-test scope (reviewer nits)

Comment-only: callbackFieldsFromMeta notes that a literal "0" is omitted (the drain's
?? 0 fallback handles depth 0); the concurrency test comment clarifies it exercises the
drain-side FIFO property, not the real vault-IPC delivery path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(agent): accurate source_thread caveat (review nit) + follow-up #124

source_thread is resolvable for multi-threaded (per-fire note leaf) but is a
per-turn correlation id for single-threaded (NOT the deterministic note leaf), so
source_message is the reliable pull-link for single-threaded recipients. Fixed the
CallbackMeta doc, the metadata-contract table, and the deferred-notes; the proper
fix (widen the writeThread seam so source_thread is the written note id for both
modes) is tracked as #124. No logic change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

inbound message to a not-yet-live agent is silently dropped + never retried (define→message race)

1 participant