From 570014c938bd94cd15276cc3c705292467135fee Mon Sep 17 00:00:00 2001 From: mintaka Date: Sun, 13 Sep 2026 22:13:51 -0400 Subject: [PATCH] =?UTF-8?q?docs(designs):=20correct=20T5=20=E2=80=94=20the?= =?UTF-8?q?=20owed=20surface=20and=20the=20offline=20premise=20(RIG-3532)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The frozen coverage record (RIG-3473) prescribes a T5 mechanism the code falsifies on two independent axes. Both were verified against the tree before T5 was implemented (#1225); this amends the record so the next reader is not misled, and records WHY the shipped test differs rather than rewriting the task as if it had always said this. 1. `CountOwedMentions`/`ClearOwedMention` are the wrong durable surface: a home-channel post never writes an `owed_mentions` row, because `RecordOwedMention` requires an @-mention AND an out-of-sweep-set agent, and `InSweepSet` is satisfied by the home-channel disjunct. A test asserting on that count is green while proving nothing. The surface the sweep actually consumes is `UndeliveredMessages`. 2. Posting to a provisioned-but-not-started agent cannot leave a message owed: the post wakes the agent and fresh-starts it against the placement `Provision` recorded, and the waker is wired in the assembly the e2e fixture runs. Offline has to be a despawn boundary. The durable-not-live-tail requirement the record states is unaffected and is why the shipped test reads the store. Co-authored-by: Matt Wilkinson --- .../compass-comms-multi-actor-test-coverage/design.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/designs/server/compass-comms-multi-actor-test-coverage/design.md b/docs/designs/server/compass-comms-multi-actor-test-coverage/design.md index 9d4377c7d..6befe556c 100644 --- a/docs/designs/server/compass-comms-multi-actor-test-coverage/design.md +++ b/docs/designs/server/compass-comms-multi-actor-test-coverage/design.md @@ -147,6 +147,13 @@ The offline-redelivery records (`docs/designs/server/compass-mention-offline-red func TestCommsOfflineRedeliveryOnSessionStart(t *testing.T) // //go:build podman ``` +**AMENDED post-freeze (RIG-3532, implemented in `legcomms_redeliver_test.go`).** Two of this task's prescriptions above are falsified by the code; both were verified against the tree before implementation, and the shipped test takes a different shape. The reasoning in the paragraph above — assert durably, never on the live fan — stands and is why the test reads the store. + +1. **The durable surface is `UndeliveredMessages`, not `owed_mentions`.** `CountOwedMentions`/`ClearOwedMention` are the wrong surface here: `owed_mentions` rows are written only by `RecordOwedMention`, whose callsites (`delivery/dispatch.go`, `routeMentionsFor` and `routeAskAnswerFor`) require the message to `@`-mention the agent **and** the agent to be outside the channel's sweep set. `InSweepSet` is satisfied by `cm.channel_id = aa.home_channel_id`, so a post to the agent's own home channel never records a row — `CountOwedMentions` reads 0 both before and after the sweep, and a test asserting on it would be green while proving nothing. `(*store.Store).UndeliveredMessages` is what the start-sweep itself consumes (`drainStarts` → `sweepSession`), so it is the surface that actually moves. +2. **"Offline" must be a despawn, not a never-started agent.** "Post to a provisioned-but-not-started agent's home channel, then `StartSession`" is unreachable: the post itself starts the agent. `fanOut` resolves the agent as a recipient (same home-channel disjunct), finds no live session, calls `wake`, and `freshStart` resolves the placement `Provision` already recorded (`service.go` `RecordAgentPlacement`) and calls `hub.Start` — the behavior `TestWakeAgentNeverStartedFreshStarts` pins. The waker is wired in the real assembly the e2e fixture runs (`server/sinks.go` `SetAgentWaker`). A subsequent `StartSession` would be a second, racing start. The shipped test instead posts **after** `RemoveWorkspace` deletes the placement, which makes the wake a benign `no_placement` no-op (`wakeOnce`'s `errWakeNoPlacement` arm), so the message stays owed across a real lifetime boundary. It therefore costs two containers rather than the one estimated above, and drives one warm turn in lifetime 1 so the resume has a transcript to reconstruct. + +The test also asserts the message **is** owed before the resume, so the "no longer owed" post-condition cannot pass vacuously. + ### T6 — DB tier: supervisor orchestration depth (`supervisor_orchestration_pgtest_test.go`) The sharpest thin spot: one test func today (`supervisor_orchestration_pgtest_test.go:38` `TestSupervisorAssignsToTwoWorkersAuditable` is the file's only `func Test`). Add: (a) **many concurrent conversations** — one supervisor, 4 workers, per-worker topics in one coordination channel; interleaved posts; assert per-topic `ListMessages` returns exactly that worker's thread in order (per-agent conversation isolation at the DB tier, complementing `agent_conversation_pgtest_test.go`); (b) worker→supervisor upward reports land on the worker's topic with author intact; (c) `SearchMessages` audit across 4 workers scoped by membership (extending the existing outsider-zero-hits case); (d) coordination-channel membership reflects a mid-scenario `ReparentAgent` (consumes `go/internal/comms/coordination.go:197-203` `ReconcileCoordinationMembership`).