feat(compass): SEA-1364 T3 — commit relayed agent conversation frames to comms

[seal-agent]

Replaces the observedConversationSink stub with the real comms write-through. This is #900 T3 (SEA-1364).

Read this first: what this PR does and does not deliver

The Server half is complete and adversarially verified. The end-to-end path is still dead, and the missing piece is not in this lane.

The first-party agent's only conversation-frame emitter is EventMapper.#appendBlock (packages/compass-agent/src/mapping.ts:386-393). It emits conversationUpdated with no id — the comment there states the agent has no server id to mint and never stamps one. conversationPosted appears only in the type union and in tests; no production path emits it. Nothing stamps an id in between: relay.go forwards verbatim (pinned by relay_test.go:187-189, "relay forwards, does not reclassify") and the Runner's PostConversationFrame is still Unimplemented.

An update addresses a row by id, so an id-less one addresses nothing. Until the Runner reconciles server-minted ids into update frames, zero conversation rows commit. That reconciliation is compass-runner's lane and is deliberately not built here; #900's D-7 already echoes committed_message_id on the ack, which is the id source it will use.

So this PR is not "the conversation write-through works." It is: the Server-side write path is correct, proven, and waiting on its input. The review caught this — my first revision would have replaced a stub that silently discarded 100% of agent turns with a write-through that silently discarded 100% of agent turns, which is the same defect one layer down.

Rather than paper over it, the gap is now loud and pinned:

• Frames with an empty message.id get their own contract-defect classification and counter, distinct from ordinary per-frame refusals — so total frame loss can never be mistaken for a healthy relay rejecting the odd bad frame. Logged at ERROR stating the consequence outright: "agent conversation relay is misconfigured: NOTHING is being committed to comms."
• TestDeliverIDLessConversationUpdateIsAContractDefectAndCommitsNothing drives the exact shape #appendBlock produces and asserts today's real behavior: zero sink calls, ContractDefects == 1, RefusedFrames == 0, stream survives. It cites the emitter and states it flips to asserting a committed row once reconciliation lands. I mutation-tested it — neutering the shape check turns it red, so it is not vacuous.
• Counters exported via FrameDiagnostics() (one snapshot under one lock) and logged on drain, so a 100%-refusal steady state is visible without log archaeology.

The stale comment that hid this work

sinks.go claimed the write-through "needs the session→channel mapping T5's store adds." That mapping landed in 0003_agent_ownership.sql. Three lanes read that comment and concluded the work was gated — it was gated by prose, not reality. Every trace is deleted; grepping for session→channel mapping, T5 write-through, T4-minimal, and observedConversationSink returns zero matches.

Three forks, ruled before implementation

1. The hub resolves session→account; ConversationSink gains the account. Resolution happens once at the Deliver site against the same accountForSession binding RelayCommsCall uses. One binding site is a security property — two resolvers can disagree, and attribution is the last place you want ambiguity.

2. MessageUpdated uses a forked authorizing update. store.UpdateMessageBlocks takes a bare MessageID with no membership and no authorship check — fine where it is used (AnswerAsk already resolved through a membership JOIN and deliberately permits a non-author member to answer), unusable for a relayed id. New UpdateMessageBlocksAsAuthor is a single UPDATE … RETURNING gated on membership AND authorship. Both halves matter: authorship alone lets an account edit its message in a channel it was removed from; membership alone lets any member rewrite another's words. Unknown id, wrong author, and revoked member collapse to one identical ErrNotFound.

3. Unbound session fails closed. No fallback, no bootstrap-admin attribution, no silent drop. enroll() clears sessionAccounts on re-enroll deliberately, so a frame crossing a reconnect lands here — ruled behavior, documented with an explicit "do not fix this with a fallback," pinned by a test.

Also fixed from review

• Nil-deref panic: homeChannel dereferenced acc.Agent, which is nil for a user account (store/accounts.go:318-322) — a real SIGSEGV inside the PublishEvents goroutine, confirmed RED before the fix. Now a fail-closed CodeFailedPrecondition.
• parent_message_id was silently dropped, though plumbed end-to-end on both messages. Threaded, with the docstring now explaining why this field is forwarded while Container/ClientRequestId are not.
• errNoConversationVariant remapped InvalidArgument → Internal: it marks an impossible state, so it should tear down loudly rather than hide among expected refusals.
• Sink error-vocabulary contract documented on the interface, with a test asserting a bare errors.New tears the stream down and increments neither counter — pinning the fail-safe that was only implicit in connect.CodeOf.

Verified clean under adversarial review

The authz predicate: EXISTS correctly correlated to m.channel_id, one parameter bound to both halves so they cannot be satisfied by different principals, RETURNING column order matching scanMessages exactly, D9 collapse total (identical sentinel, identical text, single round-trip so no timing channel), and genuinely race-free under READ COMMITTED. Verified by diff that updateMessageBlocksExec, UpdateMessageBlocks, AnswerAsk, and RequireAgentSessionSubscriber are byte-identical to main — the enumeration-oracle hazard is honored in fact, not just prose.

Verification

go build, go vet, gofmt -l (empty), go test -race ./internal/... ./server/... all clean. Real-Postgres tests genuinely executed — including all 13 AnswerAsk regressions and the non-author-member case a shared-helper change would break.

Two known non-issues: the relayed-path idempotency retry is not exercisable (no key reaches PublishEventsRequest; #894/T2 adds idempotency_key = 4, and the hook is commented at the exact line), and TestServeShutdownWithLiveCommsSubscriberReturnsClean fails on this base — pre-existing, fixed by #973, verified as a chain (fails on a clean origin/main worktree, passes on #973's branch).

Worth recording: my first pgtest run printed ok while every test skipped. pgtest.RequireDSN skips when it cannot start a container — podman here is at num_locks (2048) with 2039 leaked volumes from concurrent agents. Re-ran against a live Postgres to get real execution. Same failure shape as SEA-1510 (pgtest never runs in CI).

Targets main for review; lands in sealedsecurity/compass after the port.

Spec-impact: docs/specs/product/compass.md

The relayed-events requirement (:405-417) mandates that an unrecognized frame be "logged and counted rather than silently discarded". This PR extends that posture to a case the spec does not yet name: a frame that is recognized but structurally unusable because the relay's two ends disagree. Deliberately not written into the spec in this PR, because the spec states what the system SHALL do and the honest claim today is that relayed conversation does not commit at all. Writing a requirement describing the working path would put the same fiction in the spec that this PR just removed from sinks.go. The requirement lands with the Runner-side id reconciliation that makes it true.

---

Ported from sealedsecurity/sealed#996, unchanged in content — Compass code now lives here, so this is where it ships. The sealed PR is closed in favour of this one.

Verification (this repo has no CI yet, so a local run is the only evidence there is): go build ./..., go vet -tags "pgtest unix" ./..., gofmt -l clean, go test -race ./internal/... ./server/..., and the pgtest suites for ./internal/store/, ./internal/comms/, ./server/ against a live postgres:17 — all green on this branch.

[linear-code]

SEA-1364

[seal-agent]

Cross-branch integration verified locally. This repo has no CI yet, so this is the only integration evidence there is — and it is the check that caught a real defect in the sealed copies of these same diffs (a deleted symbol still cited by a sibling branch, invisible to any single-diff review).

Merged all five of this lane's branches together onto main — #29, #30, #31, #32, #33 — in one tree:

merge boundary rc=0   merge drop rc=0   merge ask rc=0   merge t3 rc=0   merge schema rc=0

All five merge clean, no conflicts. On the combined tree, against a live postgres:17:

gofmt -l           clean
go build ./...     BUILD_OK
go vet -tags…      VET_OK
go test -race ./internal/... ./server/...        all pass
go test -tags pgtest ./internal/store/...        ok  39.358s
go test -tags pgtest ./internal/comms/...        ok  23.879s
go test -tags pgtest ./server/...                ok  20.286s

Worth stating plainly: ./server/ only goes green here because #29 is in the tree. On main today TestServeShutdownWithLiveCommsSubscriberReturnsClean fails against a live Postgres — pre-existing, confirmed on a clean main checkout — and #29 is the fix. The other four each show that same failure alone, and it is not theirs.

A per-package throwaway Postgres container also starves under this suite's parallelism (the store package hit its 600s timeout four times over); one shared database runs the same suite in ~30s. Relevant when CI is set up here.

[greptile-apps]

Greptile Summary

The PR replaces the observed conversation sink with a durable, account-attributed comms write-through.

• Resolves relayed sessions to accounts and classifies refused or structurally unusable frames.
• Adds authored message updates, ask-ID reconciliation, parent-message forwarding, and comms fan-out.
• Exposes frame-loss diagnostics and logs their final snapshot during server shutdown.

Confidence Score: 2/5

The PR is not safe to merge until authored updates serialize with membership revocation and surplus ask IDs are reconciled safely.

The update authorization reads membership from a statement snapshot without coordinating with concurrent removal, so an edit can persist after revocation; separately, surplus asks bypass server-owned ID reconciliation and can introduce an identifier collision that makes ask resolution ambiguous.

Files Needing Attention: go/internal/store/messages.go, go/internal/comms/agent_caller.go

Important Files Changed

Filename	Overview
go/internal/comms/agent_caller.go	Adds durable post/update handling and ask-ID reconciliation; the previously reported surplus-ask reconciliation gap remains.
go/internal/store/messages.go	Adds membership-and-authorship-gated block updates; the previously reported concurrent revocation race remains.
go/internal/runnerhub/hub.go	Resolves frame identity, dispatches conversation writes, and records refusal and contract-defect diagnostics.
go/server/sinks.go	Replaces the observation-only sink with the comms adapter and adds final diagnostic logging.
go/server/serve.go	Refactors shutdown draining to close streams, drain configured servers, and report frame diagnostics.

_{Reviews (3): Last reviewed commit: "fix(compass): reconcile ask_id on relaye..." | Re-trigger Greptile}

[sealedsecurity-bot]

Review of record — BLOCKING on 1 finding (medium), parked as an Open Question for Matt

Ran the mandatory review (skill://review, Opus). One gating finding, plus low notes and an adjudication of the two Greptile threads.

OQ (medium, latent) — the UPDATE path strips the immutable ask_id

CommitAgentUpdate (go/internal/comms/agent_caller.go:217) maps the relayed frame through the shared blocksFromWire → askFromWire, and askFromWire unconditionally returns AskID: "" (go/internal/comms/mapping.go:276). UpdateMessageBlocksAsAuthor then rejects any ask block with AskID=="" (go/internal/store/messages.go:234, "an update must carry the existing id"). Net: the write-through can never persist an update to a message containing an ask block — the id is stripped microseconds before the validator demands it.

• Stripping is correct for the POST path (ask_id is server-minted at append via mintAskIDs; honoring a wire value would collide ids). The UPDATE path needs the opposite: preserve the existing immutable ask_id.
• Masked today: every relayed update is id-less and caught by deliverConversation's shape-check as a contract defect before reaching CommitAgentUpdate, so it activates silently exactly when Runner-side id reconciliation lands (the C3 future this PR builds toward). This is the conversation write-through reconciliation contract compass flagged.
• Misclassified: an ask-bearing update refused solely by this strip lands in the ordinary refusedFrames bucket (quiet Warn), not contractDefects.

The fix is partly a design decision (how to source the ask_id on update), so it is Matt's call, parked here per rule://decision-authority. Options + my recommendation:

1. (recommended) Fix in-port now — add an UPDATE-path wire→store mapping that preserves the existing immutable ask_id by reading it from the stored message row (the message exists on update), instead of stripping. No wire-trust, no mint-collision. Re-label the two ask with no ask id tests as pinning a known limitation + add a positive test that an ask-bearing update with its id succeeds.
2. Defer — merge as-is (defect is masked until id-reconciliation lands), file a SEA follow-up I own to fix before C3.
3. Minimum-safety — don't reconcile now, but classify an ask-bearing update refusal as a distinct loud contract defect (not a quiet routine refusedFrame); file the full fix as a follow-up.

Adjudicated, non-gating

• Greptile P1 (membership-revocation race, messages.go:243-256) — real in the strict-serializability sense but not a meaningful defect: single-statement READ COMMITTED in-flight-before-revoke success is standard accepted authz TOCTOU; authorship is also required (worst case: an agent edits its own message one instant post-removal, no cross-account escalation); identical property already exists in AnswerAsk. No code change; optionally tighten the comment.
• Greptile P2 is the OQ above — confirmed real.
• Low: the two ask with no ask_id tests certify the defect as correct contract; idempotency gap in CommitAgentPost correctly deferred to #894/T2.

Build/vet/unit(unix) clean at c05b74d. Holding merge-ready pending Matt's ruling on the OQ.

[mattwilkinsonn]

• feat(compass): SEA-1364 T3 — commit relayed agent conversation frames to comms #32 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[greptile-apps]

Greptile has paused reviews on this repository — it used its 100 free open-source review credits for this billing period. Reviews resume automatically on August 1. To continue before then, an organization admin can keep reviews running past the free credits — those bill as normal usage.

[sealedsecurity-bot]

Review-of-record: CLEAN (2 rounds)

The ask_id reconciliation fix went through two adversarial review rounds:

• Round 1 surfaced the core defect (ask-bearing UPDATE refused because askFromWire strips ask_id on the update path) — resolved by the reconciliation fix: updateBlocksFromWire preserves the wire id, reconcileUpdateAskIDs reconciles it against the stored row (immutable ask_id → race-free store.MessageAskIDs read, store's single-statement authz UPDATE untouched).
• Round 2 flagged a medium security finding — a surplus ask block (beyond the stored ask count) carrying a caller-chosen non-empty ask_id would fall through reconciliation and persist, reopening the ask_id forgery/collision vector askFromWire strips to prevent on POST. Fixed: reconcileUpdateAskIDs now rejects any surplus ask (CodeInvalidArgument, distinct surplusAskError) before the store write, since an UPDATE cannot legitimately introduce a new ask.
• Round 3 confirmed the surplus fix resolves the finding with no new ≥floor issue: CLEAN.

Red-green regression tests: TestCommitAgentUpdatePersistsAskBlock (positive), TestCommitAgentUpdateRejectsAskIDMismatch, TestCommitAgentUpdateRejectsSurplusForgedAsk (RED = forged surplus accepted pre-fix; GREEN post-fix, row untouched).

Deferred (low, defense-in-depth): SEA-1552

The review noted a low: MessageAskIDs is a pre-authz message-id-scoped read, so reconcileUpdateAskIDs can branch a distinct response class (InvalidArgument vs NotFound) on a message the actor cannot see — a latent existence oracle. Not live-exploitable today (the hub swallows the refusal class via countRefused; neither code nor message reaches the agent), and not elevated by the surplus fix. Tracked as a follow-up hardening in SEA-1552 rather than expanding this PR's scope.