Skip to content

feat(comms): coordination-channel auto-provision from tree edges (SEA-1722) - #166

Merged
mattwilkinsonn merged 2 commits into
mainfrom
compass-comms-1722-t5-coordination
Aug 6, 2026
Merged

feat(comms): coordination-channel auto-provision from tree edges (SEA-1722)#166
mattwilkinsonn merged 2 commits into
mainfrom
compass-comms-1722-t5-coordination

Conversation

@seal-agent

Copy link
Copy Markdown
Contributor

What

Auto-provision and reconcile a manager's coordination channel from the agent tree's parent edges (SEA-1722 T5, design.md:530-592). When an agent gains or moves a parent, the manager's <handle>-coordination channel (OWNER_ONLY, mandatory-subscription, owner = manager) is get-or-created and its membership resynced to the manager plus its direct reports — atomically with the tree edge.

How

The store's two parent-edge writers (CreateAgent, ReparentAgent) invoke a store-registered CoordinationHook on their own tx, right after writing agent_accounts.parent_agent_id. The comms-owned closure (reconcileCoordinationTx) runs the in-tx reconcile — channel upsert + membership rows + D2 cursor seeds — on that same tx, so coordination state commits atomically with the tree edge. The store never imports comms types; the hook is an injected callback.

ChannelChanged is emitted post-commit, best-effort: the in-tx reconcile records touched channels into a ctx-scoped buffer that the parent-edge RPC drains and emits after the store confirms the commit. A lost emit self-heals on the next reconcile / D1 sweep, so a dropped event never leaves the tree edge and channel state divergent.

Design realizations (both confirmed against the frozen record)

  • Per-owner group get-or-create (design.md:570-585): one __coordination__ VisibilityOwner group per owner hosts every coordination channel under that owner, resolved by a deterministic SELECT-then-INSERT under a per-owner advisory lock (LockOwnerCoordinationTx, keyed coordination:+owner, namespaced off ReparentAgent's per-owner tree lock so the two never spuriously serialize). All of an owner's coordination channels sharing one group is the invariant the same-owner collision analysis depends on.
  • No savepoint (design.md:554 in-tx vs 591-592 never-wedges-report-creation): the reconcile runs directly on the parent-edge writer's tx. Its only expected failure — a same-owner name collision — is resolved WITHOUT erroring by the advisory-lock + SELECT-guided suffix search in UpsertCoordinationChannelTx (a user's manually-named channel is suffixed around -2, -3, …, never adopted, never a unique-violation). Normal operation cannot poison the tx; only a genuine store fault returns an error, which correctly rolls back the whole parent-edge write (fail-loud beats a half-provisioned tree).

Reparent reconciles both managers

ReparentAgent captures the OLD parent before the UPDATE overwrites the edge, then reconciles BOTH affected managers: the new parent gains the report (reparent-in adds it) and the old parent loses it (reparent-out removes it, carried in ChannelChanged.removed_account_ids). Empty sides (promote-to-root / former-root move) skip; old == new (no-op move) skips the redundant second resync.

Notes

  • Membership seeds each agent member's D2 delivery cursor in-tx, so a mandatory channel never mints an un-seeded delivery target (the fail-dangerous D2 hazard). seedDeliveryCursor is agent-only + idempotent, so a human member yields no cursor and a re-run touches nothing.
  • No P3 relay-arm change — coordination is provisioned from the tree, not an agent comms-call variant.
  • The migrations/0007_agent_tree.sql edit is a comment-only INVARIANT note on the parent_agent_id column (documenting the hook contract). The migration runner is version-tracked, not checksummed (store.go migrate), so editing an applied migration's comment is runtime-safe.

Tests (red-first proven)

9 pgtest suites: 6 store (coordination_pgtest_test.go) + 3 comms (coordination_pgtest_test.go), covering hook-fires-for-parent, first-report-provisions-once, reparent-moves-membership (both managers), despawned-report-keeps-membership, collision-manager-owned-resumes, collision-user-owned-suffixes (never-adopt + parent-edge-never-wedged), and the manual EnsureCoordinationChannel backfill entrypoint.

Red-first: neutering invokeCoordinationHook turns the 4 hook-dependent store tests red with sensible messages (empty fired-list, 0 channels provisioned, member not moved, no suffix); restoring turns them green.

Verification

  • go build ./... clean; go vet -tags pgtest (store + comms) clean.
  • CI-exact lint golangci-lint run --config .golangci.yml ./... (no pgtest tag — the authoritative gate) → 0 issues.
  • store + comms pgtest suites green under -race (store 83s / comms 23s), including T4's born-mandatory tests — no regression.

Stacking / collect notes

Stacked on the T4 channel-policy branch (#161, base tip 8d2cfd6c born-mandatory). GitHub auto-repoints this base → main when #161 merges.

Collect overlap with T6 (#165): both add an identical store.GetChannel(ctx, id) (Channel, error) exported wrapper over getChannel (T5 for the post-commit emit read, T6 for the pinned-board authz read). Byte-identical body, different doc comment — a clean union fold whichever merges second (keep one method), not a conflict.

Refs SEA-1722

@linear-code

linear-code Bot commented Aug 5, 2026

Copy link
Copy Markdown

SEA-1722

@seal-agent
seal-agent force-pushed the compass-comms-1722-t5-coordination branch 2 times, most recently from cfba1aa to ffea6c2 Compare August 5, 2026 21:14
@seal-agent
seal-agent force-pushed the compass-comms-1722-t4-channel-policy branch from 4074868 to bfccd5f Compare August 5, 2026 21:14
Base automatically changed from compass-comms-1722-t4-channel-policy to main August 6, 2026 00:28
seal-agent and others added 2 commits August 5, 2026 20:33
…-1722)

Auto-provision and reconcile a manager's coordination channel from the
agent tree's parent edges. The store's two parent-edge writers (CreateAgent,
ReparentAgent) invoke a store-registered hook on their own tx right after
writing agent_accounts.parent_agent_id; the comms-owned closure runs the
in-tx reconcile — channel upsert + membership rows + D2 cursor seeds — on
that same tx, so coordination state commits atomically with the tree edge.
ChannelChanged is emitted post-commit best-effort (a lost emit self-heals on
the next reconcile / D1 sweep).

Design realizations (both confirmed against the frozen record, design.md
530-592):

- Per-owner group get-or-create: one `__coordination__` VisibilityOwner group
  per owner hosts every coordination channel under that owner, resolved by a
  deterministic SELECT-then-INSERT under a per-owner advisory lock
  (LockOwnerCoordinationTx, keyed `coordination:`+owner, namespaced off
  ReparentAgent's tree lock). All of an owner's coordination channels share
  one group, the invariant the same-owner collision analysis depends on
  (design.md:570-585).
- No savepoint: the reconcile runs directly on the parent-edge writer's tx.
  Its only expected failure — a same-owner name collision — is resolved
  WITHOUT erroring by the advisory-lock + SELECT-guided suffix search in
  UpsertCoordinationChannelTx (a user's manually-named channel is suffixed
  around, never adopted, never a unique-violation). Normal operation cannot
  poison the tx; only a genuine store fault returns an error, which correctly
  rolls back the whole parent-edge write (design.md:554 in-tx vs 591-592
  never-wedges-report-creation).

ReparentAgent reconciles BOTH affected managers: the new parent gains the
report (reparent-in adds it) and the old parent loses it (reparent-out
removes it, carried in ChannelChanged.removed_account_ids), captured before
the UPDATE overwrites the edge. Membership seeds each agent member's D2
cursor in-tx so a mandatory channel never mints an un-seeded delivery target.

No P3 relay-arm change (coordination is provisioned from the tree, not an
agent comms-call variant). The migrations/0007 edit is a comment-only
INVARIANT note on the parent_agent_id column; the migration runner is
version-tracked, not checksummed, so editing an applied migration's comment
is runtime-safe.

Red-first proven: neutering invokeCoordinationHook turns the 4 hook-dependent
store tests + 3 comms tests red with sensible messages (empty fired-list, 0
channels provisioned, member not moved, no suffix), restoring turns them
green.

Stacked on the T4 channel-policy branch (SEA-1722); base tip 8d2cfd6
(born-mandatory cursor seed). Full gate green on the rebased tree: build/vet
clean, CI-exact lint (--config .golangci.yml ./..., no pgtest tag) 0 issues,
store + comms pgtest suites green under -race.

Refs SEA-1722

Co-authored-by: Matt Wilkinson <matt@sealedsecurity.com>
Review-fix for the T5 coordination PR (mandatory skill://review, 2 medium + 3
low). Both mediums are the same root cause — the `__coordination__` namespace
was claimed-reserved in docstrings but not enforced — fixed at the store/reconcile
layer without touching the user RPC surface, honoring all four frozen invariants
(owner-private, never-wedge, never-adopt, no-savepoint).

- EnsureOwnerCoordinationGroupTx: the get-half SELECT matched a group by
  (owner_user_id, name, top-level) with no visibility filter, so a user-planted
  top-level VisibilityShared group named `__coordination__` was adopted and the
  OWNER_ONLY coordination channel inserted into it — making an owner-private
  channel visible to every account (cross-tenant metadata leak). Discriminate the
  SELECT on `visibility = VisibilityOwner` so a mis-visibility planted group is
  never adopted; the create-half then mints the correct owner-visible group.
- UpsertCoordinationChannelTx: the reconcile runs on the parent-edge writer's tx
  with no savepoint, and its plain INSERT could raise a unique-violation on
  channels_group_name_key when a user's concurrent CreateChannel committed the
  same (group, name) between our SELECT and INSERT — poisoning the tx and wedging
  report creation (the exact outcome the design forbids). Make the INSERT
  poison-free: `ON CONFLICT (group_id, name) WHERE group_id IS NOT NULL DO NOTHING
  RETURNING id`. A concurrent commit now yields zero rows (not a raised
  violation); the loop re-resolves the name and suffixes it (never-adopt), and
  the parent-edge write never wedges. Both docstrings rewritten from their prior
  false claims to the true-by-construction invariant.
- comms EnsureCoordinationChannel: move the len==0 internal-invariant guard ahead
  of emitCoordChanges (surface the fault before the no-op emit).
- Test-doc honesty: correct the coordChannels helper comment.

Red-first: TestReconcileIgnoresMisVisibilityUserGroup (drop the visibility
discriminator -> the shared group is adopted, channel leaks to a third account);
TestUpsertConcurrentUserInsertSuffixesWithoutWedge (deterministic two-tx race, a
committed concurrent user insert -> without ON CONFLICT the reconcile raises
23505 and would wedge the parent-edge write); TestCreateAgentSuffixesAroundUser
ChannelWithoutWedge (drives the real registered closure: manager's channel
suffixes to -2, CreateAgent still succeeds). Each proven by reverting the fix.

Co-authored-by: Matt Wilkinson <matt@sealedsecurity.com>
@seal-agent
seal-agent force-pushed the compass-comms-1722-t5-coordination branch from ffea6c2 to 07e05ff Compare August 6, 2026 00:42
@mattwilkinsonn
mattwilkinsonn merged commit 3d92b8b into main Aug 6, 2026
1 check passed
@mattwilkinsonn
mattwilkinsonn deleted the compass-comms-1722-t5-coordination branch August 6, 2026 01:03
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.

2 participants