Implement the Dynamic Stream: pattern advertisements in moq-net - #3011
Implement the Dynamic Stream: pattern advertisements in moq-net#3011kixelated wants to merge 2 commits into
Conversation
The advertise half of the dynamic questline, per the lite-06 draft on this branch. Path grows segment-aware has_suffix/strip_suffix, and a new Pattern type carries the (prefix, suffix) pair with matching, overlap, and specificity. The wire is a new module beside announce: DYNAMIC_REQUEST/DYNAMIC_OK then DYNAMIC_START/END/UPDATE on stream type 0x7, carrying the pattern whole (the request prefix filters, never rebases), a hop list, and one route cost. The pattern table lives inside the origin's existing shared dynamic state, off to the side of the announce tree, since a pattern is not a tree node. Producer::advertise checks the prefix against the publish scope (the catch-all and every suffix-only pattern demand an unscoped handle) and returns a handle that reprices in place and retracts on drop; Consumer::patterns() observes the table rebased by the cursor's root and filtered by its excluded origin, deduplicated to the best route per pattern. The subscriber opens one Dynamic Stream per allowed prefix on lite-06+ and mirrors received advertisements into the table through the same hop/reflection/link-cost pipeline announcements use; every failure retires the machine quietly, since a peer without support resets the unknown stream type and that must never cost the session. The publisher serves the table as an epoch-driven snapshot diff, with no demand watch or cost discount, because a pattern is never warm. CAPACITY lands as the draft's assigned 0x30, round-tripping unlike the placeholder codes around it. Resolution (routing a subscribe through the table) is the next change; nothing consumes the table for dispatch yet. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The non-Linux fallback referenced Criterion without a path or import, so `cargo test --all-targets` fails on macOS; Linux CI never compiles that cfg arm, which is how it landed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 93ebd5244f
ℹ️ 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".
| let Some(source) = entry else { | ||
| // Locally dropped (reflected or unauthorized); the id stays | ||
| // tracked but there is nothing to update. | ||
| return Ok(()); |
There was a problem hiding this comment.
When a DYNAMIC_START was dropped because its hop chain was reflected, by_id stores None, and every later DYNAMIC_UPDATE for that ID returns here without examining the replacement hops. A route update is explicitly allowed to change the hop chain, so a non-reflected replacement must become visible again; as written, the pattern remains unavailable for the rest of the stream. Preserve the pattern alongside the optional source and retry insertion when an update becomes routable.
Useful? React with 👍 / 👎.
| .ok(), | ||
| None => None, | ||
| }; | ||
| run.by_id.insert(id, source); |
There was a problem hiding this comment.
When a peer sends a second DYNAMIC_START for a pattern that is already live on this stream, this unconditional insertion assigns another ID and creates another source instead of resetting the stream for a protocol violation. The Dynamic Stream mirrors the Announce Stream rule that each pattern has at most one current advertisement per stream; accepting duplicates also means ending one ID can incorrectly leave the same peer's pattern active through the other ID.
Useful? React with 👍 / 👎.
| cost: cost.min(broadcast::MAX_COST), | ||
| advertiser: self.info, | ||
| }; | ||
| let id = self.dynamic.lock().patterns.insert(route); |
There was a problem hiding this comment.
If the origin Driver has already been dropped, this insertion never checks OriginDynamicState::closed, so Producer::advertise still returns Ok and exposes a pattern even though dynamic requests now fail with Error::Closed. A surviving session can consequently forward a capability that the origin can no longer serve, contrary to the driver's documented teardown contract that later producer mutations fail with Closed.
Useful? React with 👍 / 👎.
|
|
||
| fn absolute_pattern(&self, pattern: &crate::Pattern) -> crate::PatternOwned { | ||
| crate::Pattern { | ||
| prefix: self.root.join(&pattern.prefix), |
There was a problem hiding this comment.
With a rooted producer, both the root and the supplied prefix can individually fit the 32-part limit while their joined prefix exceeds it. This helper stores that oversized path without returning BoundsExceeded; when a publisher later forwards the table, DynamicAdvert::encode fails and aborts the entire Dynamic Stream, suppressing otherwise valid advertisements too. Validate the joined prefix here as create_broadcast does for rooted paths.
Useful? React with 👍 / 👎.
1 participant
The advertise half of moq.pro's dynamic questline, implementing the Dynamic Stream that #3005 (this PR's base branch) specifies. Stacked on that branch; rebase onto
devonce it merges.What's here
Path::has_suffix/strip_suffix— the segment-aware suffix twins of the existing prefix pair, plus thePatterntype:(prefix, suffix)withmatches,overlaps, andspecificity.lite/dynamic.rs, stream type 0x7):DYNAMIC_REQUEST/DYNAMIC_OK, thenDYNAMIC_START/END/UPDATEwith implicit Dynamic IDs. The pattern prefix travels whole — the request prefix filters which patterns a stream carries, never rebases the encoding (the lossy-rebasing fix from the draft review). One route cost varint, no Epoch.Producer::advertise(pattern, cost)checks the prefix against the publish scope — the catch-all and every suffix-only pattern demand an unscoped handle — and returns a handle that reprices in place (set_cost) and retracts on drop.Consumer::patterns()observes the table rebased by the cursor's root, filtered by its excluded origin, and deduplicated to the best route per pattern (cost, then chain length, then advertiser id), so duplicates present combined.StreamError::Capacity= 0x30, the draft's one assigned code in the reserved band; it round-trips, unlike the placeholders around it.Deliberately not here
Resolution — routing a subscribe/FETCH for an unadvertised path through the table (specificity tiers, the rendezvous hash, route install, capacity re-resolution) — is the questline's next quest and a follow-up PR; nothing consumes the table for dispatch yet. The JS client is a later quest too.
The last commit cherry-picks #3010 (the macOS bench-stub fix) so
just testcompiles workspace-wide here; it drops out on rebase.just checkandjust testpass.🤖 Generated with Claude Code
(written by Fable 5)