feat: open peer-advert's extension tail and let sessions re-advertise it (P4) - #91
Merged
Conversation
Mearman
marked this pull request as ready for review
September 12, 2026 19:24
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Mearman
force-pushed
the
feat/peer-advert-extension
branch
from
September 12, 2026 19:32
9ac87e4 to
2868953
Compare
Member
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2868953ff9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Mearman
enabled auto-merge (rebase)
September 12, 2026 20:09
Mearman
force-pushed
the
feat/peer-advert-extension
branch
2 times, most recently
from
September 12, 2026 21:05
824aa2a to
92398f7
Compare
auto-merge was automatically disabled
September 13, 2026 07:15
Rebase failed
peer-advert gains the same * tstr => any extension tail token-claims already carries, the generalised gossip-extension point named in the agent-comms migration design: a node's presence status, its own accept/refuse policy for a claim class, and any future domain's own gossiped fact all ride this one open tail, added once rather than as a series of separately-named fields each time a new fact needs gossiping. An unrecognised key here is exactly as trustworthy, and exactly as ignorable, as any other unrecognised value in an open discriminator elsewhere in this spec.
sendGossipUpdate re-sends this side's own self-advert with a fresh snapshot-seconds and any given extensions merged onto peer-advert's own open tail, since wireUpConnection's own initial self-advert is otherwise never repeated over a connection's lifetime -- there was no way to keep a gossiped fact (presence status, an accept/refuse policy) live once the connection settled. MeshSession owns no timer of its own for this: a caller decides its own re-advertisement cadence, matching the session's existing DOM-free, fully unit-testable design; a caller that never calls this again after connecting keeps today's exact gossip-once-on-connect behaviour. Factors the self-advert construction wireUpConnection already built inline into a shared buildSelfAdvert helper, so both call sites stay in sync rather than duplicating the frame shape.
…pace and guard them in TS
The Rust codec's peer_advert_from rejected any key beyond device/addresses/snapshot-seconds as DecodeError::UnknownKey, so a TS peer's first sendGossipUpdate call (carrying its new extension tail) would disconnect every Rust peer instead of being ignored as transport.cddl's own comment requires. PeerAdvert gains an extra: CanonicalMap<String, CborValue> field, decoded and re-encoded the same way room-notice-claims/token-claims already handle their own open tails.
sendGossipUpdate's extension bag could previously shadow peer-advert's own mandatory fields (a caller passing { device: ... } silently overwrote the session's real device-id on the wire) and had no namespacing convention, so two independent applications advertising over one session could collide on an identical bare key like "status". validateGossipExtensions now rejects both cases immediately: a key colliding with device/addresses/snapshot-seconds, or a key not shaped "<domain>/<field>". buildSelfAdvert also spreads extensions before the mandatory fields, not after, so the field order itself can't shadow them even if the guard were ever bypassed.
Documents the domain-qualified key convention in spec/CONVENTIONS.md and spec/transport.cddl, marks the peer-advert extension bag as shipped rather than anticipated, and adds a conformance vector exercising an extension-bearing peer-advert end to end.
…nto generate.ts protocol.cddl is a mechanically concatenated file (spec/generate.sh); editing transport.cddl without regenerating it left protocol.cddl stale, which CI's own CDDL Validate job catches. The peer-advert extension conformance vector was hand-added directly to frames.v1.json in the previous commit, but that file is itself generated from conformance/generate.ts's own vector definitions -- moved there so a future `pnpm run generate` doesn't overwrite or drift from it. Regenerating reproduced byte-identical output to the hand-computed vector, confirming it was correct.
Mearman
force-pushed
the
feat/peer-advert-extension
branch
from
September 13, 2026 07:17
81a4901 to
4084b73
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
peer-advertgains the same* tstr => anyextension tailtoken-claimsalready carries -- the generalised gossip-extension point the agent-comms migration design names for P4: presence status, a node's own accept/refuse policy, and any future domain's own gossiped fact all ride this one open tail rather than a series of separately-named fields.MeshSession.sendGossipUpdate(extensions?)re-sends this side's own self-advert with a freshsnapshot-secondsand the given extensions merged in, since the initial self-advertwireUpConnectionsends at connect time is otherwise never repeated over a connection's lifetime. No timer lives insideMeshSessionitself -- a caller (agent-comms, in this case) owns its own re-advertisement cadence, matching the session's existing DOM-free, fully unit-testable design.buildSelfAdverthelper so both call sites stay in sync.This is the foundational piece of P4 (presence). Consuming it for real periodic re-advertisement and presence tracking is agent-comms' own follow-up work, tracked in
agent-comms#48's own notes on what P3.8's remaining agent-presence retirement is blocked on.Test plan
just spec-- regeneratedprotocol.cddl, validates cleanlyjust build,just lint,just test,just conformance-- all green across every subtreesendGossipUpdate is not a function) and GREEN with it