Repository navigation
Conversation
…ops (#813 review) - hop_counter_candidates is {next+gap, next, next+gap+1 ..= next+gap+SLACK}: at most HOP_COUNTER_MAX_OPENS (SLACK+2) AEAD opens per frame whatever the reported gap. The dropped frames' own counters are never tried. Both open loops pick the counter through open_at_first_counter. - A peer-opened link whose frames find the arrivals queue full is AvDelivery::ArrivalsFull, counted as av_inbound_dropped_arrivals_full, remembered per link (bounded at 1024 links), and reported as dropped_before on the link's first delivered frame. - Witnesses: a junk frame after 1000 drops costs exactly SLACK+2 opens and the post-gap frame opens first try; an arrivals-full link with more drops than the slack reaches resyncs a real subscriber (fails with the gap report disabled). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ksniaWCvWoUwFQHRG3HQL
2b0d270 to
c600068
Compare
…ops (#813 review) - hop_counter_candidates is {next+gap, next, next+gap+1 ..= next+gap+SLACK}: at most HOP_COUNTER_MAX_OPENS (SLACK+2) AEAD opens per frame whatever the reported gap. The dropped frames' own counters are never tried. Both open loops pick the counter through open_at_first_counter. - A peer-opened link whose frames find the arrivals queue full is AvDelivery::ArrivalsFull, counted as av_inbound_dropped_arrivals_full, remembered per link (bounded at 1024 links), and reported as dropped_before on the link's first delivered frame. - Witnesses: a junk frame after 1000 drops costs exactly SLACK+2 opens and the post-gap frame opens first try; an arrivals-full link with more drops than the slack reaches resyncs a real subscriber (fails with the gap report disabled). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ksniaWCvWoUwFQHRG3HQL
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@codex review |
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. |
![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: e8d2a1db35
ℹ️ 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".
| crate::av_addressing::AV_STREAM_GROUP_PREFIX, | ||
| ))); | ||
| } | ||
| self.check_blackhole(&dest_hash.into_bytes()).await?; |
There was a problem hiding this comment.
When an operator blackholes a peer using one of the federation destination hashes exposed by the path table, open_av_link checks only the unrelated, session-derived address. Unlike the regular send path, which checks every resolved candidate to prevent alternate-address bypass, this allows the blackholed peer to remain reachable through any A/V address. Resolve the peer first and apply the deny-list to all of its federation candidates before dialing the derived address.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 43ed890. open_av_link now checks the deny-list on the derived address and on every federation candidate the peer resolves to, as the regular send path does (av_dial_target, src/transport/reticulum.rs).
Witness: tests/av_spine_live_node_805.rs blackholes B by its federation destination, then asserts open_av_link to B's call address fails with PeerBlackholed naming that hash. With the candidate loop removed, the dial succeeds: Ok(AvLink { .. }).
🤖 Generated with Claude Code
| // Malformed wire — skip this frame, keep pulling. | ||
| let Ok(sealed) = SealedAvChunk::from_bytes(&bytes) else { | ||
| let Ok(sealed) = SealedAvChunk::from_bytes(&frame.bytes) else { | ||
| continue; |
There was a problem hiding this comment.
If queue overflow drops more than the eight-counter resync slack and the first subsequently queued frame is malformed, that frame carries the sole nonzero dropped_before report, but this branch discards it. AvLinkQueue::offer has already reset its drop accumulator, so the next valid frame reports zero while its sender counter remains far ahead; it and every later frame then fail authentication permanently. The subscriber and equivalent relay path need to retain the reported gap across parse/open failures until a frame authenticates.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 43ed890. A new HopGap (src/transport/realtime_av_dispatcher.rs) adds up each frame's dropped_before and clears it only when a frame authenticates. Both the subscriber loop and the relay pump use it, so an unparseable or unopenable frame can no longer discard the report.
Witnesses:
a_junk_frame_does_not_swallow_the_drop_report: both junk shapes, each carrying a gap larger than the slack.the_relay_pump_keeps_the_drop_report_across_junk.
With the gap carry reverted, they fail with "chunk 1: the hop lost its drop report" and "the relay lost the drop report: nothing forwarded".
🤖 Generated with Claude Code
| QueueOffer::ConsumerGone => { | ||
| links.queues.remove(&link_id); | ||
| AvDelivery::ConsumerGone |
There was a problem hiding this comment.
When a link's receiver is dropped, this removes its only ownership record, so the next frame takes the unclaimed-link branch and emits a fresh AvArrival. This makes a rejected peer-opened link repeatedly reappear, and for an open_av_link caller that intentionally drops the unused inbound half (as edge_node does), two reverse frames cause an initiator-opened link to be surfaced through the API that promises only peer-opened links, with the peer's destination reported as though it were an inbound address. Retain whether the link was claimed/previously arrived, or close/tombstone it when its consumer disappears.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 43ed890. On ConsumerGone the link's queue entry now stays in the sink, as a tombstone, until LinkClosed calls forget(). Later frames on it stay ConsumerGone and are never routed down the unclaimed-link branch. Live links bound the map.
Witness: av_sink::tests::a_link_whose_consumer_is_gone_never_arrives_again covers a link this node dialled whose inbound half was dropped, and a peer-opened link that arrived and was then rejected. Neither is ever surfaced again. With the old remove, it fails: left: Arrived.
🤖 Generated with Claude Code
|
Conflict to carry, found while porting #853 (the responder-side inbound reap): this PR routes A/V frames to |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3471dcddb6
ℹ️ 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".
| if let Err(e) = opened { | ||
| tracing::warn!(peer = %m, error = %e, "A/V link did not open (CIRISEdge#805)"); | ||
| av_open_failures.push(e); |
There was a problem hiding this comment.
When any member's A/V dial fails, this records the error but deliberately continues; every later delivery to that member fails, yet perf.publish_fanout still sets ok solely from frames_sent == cfg.frames, and frames_sent increments once per source frame regardless of successful delivery. Consequently even a run with zero opened A/V links can report this required benchmark leg as successful and calculate fanout_chunks_per_s as though every subscriber received every chunk. Include av_open_failures and delivery_failures in the verdict and base delivered throughput on successful member sends.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 43ed890. perf.publish_fanout now uses fanout_verdict(). It requires every source frame sealed, zero A/V link-open failures, and zero member delivery failures.
fanout_chunks_per_snow counts member sends the link accepted (member_sends_delivered), not frames × roster.- A late joiner's frames before its
Readyopens its link are counted separately (frames_awaiting_link). They are neither a failure nor a delivery.
Witness: edge_node tests::the_fanout_verdict_fails_when_members_did_not_get_the_stream. With the old frames_sent == frames_requested, it fails: "no A/V link opened to two members: not a fan-out".
🤖 Generated with Claude Code
| let plane = classify_link_plane(self.scope_addresses.get().map(Arc::as_ref), &dest_hash); | ||
| if plane != LinkPlane::Av { |
There was a problem hiding this comment.
When two federation key IDs resolve to the same transport identity, passing peer B's A/V address together with peer C's destination_key_id passes this plane-only check and the link proof can still succeed. The initiator then records C in av_dialed_peer, so reverse frames on a link actually opened to B's member address are attributed through C's #393 binding. Resolve the InboundAddress here and reject it unless member_key_id() equals destination_key_id.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 43ed890. Before any dial, open_av_link resolves the address's table entry and refuses with Config("… belongs to member <B> …") unless member_key_id() equals destination_key_id (av_dial_target).
Witness: av_spine_live_node_805 dials B's call address under another directory key id and gets the refusal by name. Without the check, that call reached dial resolution and failed differently (Unreachable).
The fixture cannot construct two key ids that share one transport identity. The check runs before identity resolution, so the shared-identity case is refused the same way.
🤖 Generated with Claude Code
| Ok(id) => id, | ||
| Err(e) => { | ||
| // The dial registered nothing that outlives it: a link that | ||
| // never established closes, and `LinkClosed` forgets both. | ||
| return Err(e); |
There was a problem hiding this comment.
If the derived link establishes but identification subsequently fails, before_establish has already claimed an AvSink queue and inserted av_dialed_peer, but this error arm returns without closing the link or removing either record. Because no AvLink is returned, the caller has no link ID with which to invoke link_teardown; repeated identification failures can therefore leave live links and sink queues consuming the node's bounded link capacity until an independent close event occurs. Explicitly clean up the registered state and close the partially opened link before returning the error.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 43ed890. open_av_link records the link id the dial dispatched. On any dial error after registration, undo_partial_av_dial removes the sink queue and the av_dialed_peer record and closes the link.
Witness: av_spine_live_node_805 forces two identify failures on established links through a new seam, fail_next_derived_identifies_for_test. Then the sink count and the dialed-peer count are back to baseline, and no established A/V-plane link remains. With the cleanup disabled, it fails: left: 2 against a baseline of 0.
🤖 Generated with Claude Code
| drop_inbound(Some(link_id), DROP_AV_FRAME_UNATTRIBUTED, &detail); | ||
| return; |
There was a problem hiding this comment.
When the #393 gate is temporarily false, every successfully sent A/V frame is dropped here before reaching AvSink, so none contributes to InboundWireFrame::dropped_before. This is readily longer than the eight-counter recovery slack because the binding cache deliberately retains a stale false verdict for up to three seconds; at video cadence the first frame admitted after recovery can therefore be dozens of counters ahead, will not authenticate, and all subsequent frames remain permanently ahead as well. Track these per-link pre-sink drops and report the gap on the next admitted frame, or close the link so a redial resets its counter.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 43ed890, the preferred way. In deliver_av_frame, the #393 refusal arm now calls AvSink::note_dropped_before_sink. It counts the frame into the link's next dropped_before, or into the pre-arrival count for a link with no consumer yet. HopGap then skips the counter past the window once the binding recovers.
Witness: av_sink::tests::frames_refused_by_the_gate_still_resync_the_hop. It sends frame 0, then 45 gated frames (about 3 s at 15 fps, five times the slack), then recovery, and a real subscriber opens the first admitted frame. With the count disabled, it fails: "chunk 46: the hop never re-authenticated".
This is witnessed at the sink and subscriber seam, not over real links. Holding a real binding stale-false for 3 s is not practical in the loopback fixture. The production refusal arm calls the same method.
🤖 Generated with Claude Code
…ery departure (#853, main port of v40.0.10) (#854) * fix(transport): the responder closes idle inbound links, and every link departure is counted (#853) Main port of v40.0.10 (971a605 + dc5b296), with two design changes. - Link direction from LinkEstablished { is_initiator }; inbound_links / outbound_links gauges; EdgeLinkInfo.direction in the UDL and the regenerated bindings. - Responder-side idle reap on the pool tick (inbound_link_idle_bound, default 120 s, 0 disables), node-authoritative teardown; busy: link_in_flight, inbound transfers, outbound ships, sender-side resource progress, LXMF serve activity, the attribution handshake. - {inbound,outbound}_link_closed_by_reason with one token per way a link leaves, and reconcile_vanished_links pruning links the node no longer holds as `vanished`. Changed from the release line: - The LXMF serve exemption is BOUNDED: a link is busy for one inbound idle bound after its last LXMF activity (a request, its response handed to the link), then the normal rule applies. A sender-side resource still making progress (e.g. a large LXMF response) is busy too. Witness (f): fails with the exemption made for-life and with it removed. - hold_link_open / release_link_hold removed: no A/V caller on main, and #813 tracks link ownership through LinkPlane::Av + AvSink. - Main has no P0 telemetry (#820) yet, so the new metrics are in EdgeMetrics / EdgeMetricsBundle only; their PyO3 / UniFFI flattening arrives with #820's port. Gates: fmt; clippy --all-targets -D warnings on CI's three sets; lib filtered 819/853/link/reap/metrics/observability 84 passed (84 under ffi-uniffi); 6 integration files 57 passed; links_ffi + peer_mutation_ffi + witness under ffi-uniffi 30 passed; bindings regen matches; 0 failed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C5FDihqVKTSXyNYs2CLnrK * fix(transport): mark the swallow seam's early return for the #425 scanner (#853) inbound_exits_are_instrumented_or_marked failed the v40.0.10 tag run on the bare `return;` after the swallow_link_closed test seam in the LinkClosed arm. The seam is empty in production and the swallowed close is still counted as `vanished` by reconciliation. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C5FDihqVKTSXyNYs2CLnrK * fix(transport): the four reaper review fixes (#853, Codex on #854) 1. Outbound use restarts the idle clock. A successful outbound send on a link (a Channel-first frame fully sent on the scoped lease and both reverse-path arms; a Resource whose completion future resolved) stamps link_last_outbound_at, kept apart from link_last_inbound_at so the #353 reverse-path selector still reads only inbound liveness. The idle clock is max(last inbound, last outbound, establishment). An initiator-only peer's link this node keeps answering on is no longer reaped under it. 2. Idle selection skips links the node no longer holds, so a queued LinkClosed is never pre-empted and misfiled as idle_expired; those wait for their event or reconciliation's `vanished`. 3. A local teardown counts the close under its cause, closes at the node, then runs the full LinkBook::forget eagerly (plus LXMF, pool stamp, scoped pool): a lost LinkClosed leaks no attribution, stamps or transfer marks. 4. Two consecutive idle observations. leviculum 0.27's std handle does not surface a link's last-inbound (link_stats has none; core's last_inbound_secs counts keepalives), so a link is closed only by the second pass that finds it idle with the same (last inbound, last outbound) snapshot; activity queued behind the timer lands between. Witnesses, each failing before its fix (mutation runs, file restored): - (g) a_link_this_node_keeps_sending_on_is_not_reaped_854: without the outbound stamp, A's reply path is gone under its own sends. - the_idle_reap_skips_links_the_node_no_longer_holds_854: without the node filter, idle_expired is counted for a link the node had closed. - a_local_teardown_forgets_the_link_everywhere_854: without the eager forget, attribution/stamps/transfer marks remain. - (h) a_link_is_reaped_only_on_the_second_idle_observation_854: with one observation, the first pass closes the link. Gates: fmt; clippy --all-targets -D warnings on CI's three sets; lib filtered 819/853/854/link/reap/metrics/observability/inbound_exits 87 passed (87 under ffi-uniffi); 6 integration files 59 passed; links_ffi + peer_mutation_ffi + witness under ffi-uniffi 32 passed; 0 failed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C5FDihqVKTSXyNYs2CLnrK * fix(transport,replication): end the pool churn, and let the responder speak (#853) From the Datum link-storm trace (storm-v4006): ~890 `link identified` and ~825 PeerClosed per 10 min from one v40.0.6 initiator, and no responder line at the canonical's RUST_LOG=info. - No PoolFull close on release: release_link_after_transfer no longer trims the pool to the idle cap, and the reaper applies no cap either; pooled lanes die only by the idle bound. pool_full is retired (always 0, kept for its series); idle_cap is accepted but not enforced. - LinkClaim: a pool lane's transfer claim is an RAII guard released on Drop, so a send cancelled mid-ship (the responder's 60 s reply timeout) no longer retires its lane. - A pooled lane closing ChannelExhausted or Stale logs a throttled WARN with running counts (the starved-responder signature). - Responder round lifecycle at INFO (started; both completion paths with duration_ms; reply failures stay WARN), with peer, kind and the arrival link; counter responder_rounds_total{kind, outcome}. - responder_link_up_seconds{stage: bundle_gate, announce_intake, owner_binding} from LinkEstablished on a peer-dialled link, and responder_link_up_total{ok, bundle_refused, timeout, link_gone}. - EdgeMetrics only on main (accessors responder_rounds_total() / responder_link_up_seconds() / responder_link_up_total()); the bundle and both bindings carry them when #848 lands. FixedHistogram / HistogramSnapshot come over verbatim from the release line. Witnesses: a_burst_closes_nothing_on_release_and_the_bound_retires_it_853 (14 concurrent sends, cap 1), a_cancelled_send_gives_its_lane_back_853, each_responder_round_outcome_increments_its_token_once_853, a_responder_link_up_lands_in_a_bucket_and_counts_its_outcome_853. Fail-before: with the release trim + reaper cap restored the burst witness fails (12 lanes closed pool_full); with LinkClaim's Drop a no-op the cancel witness fails (the lane stays claimed). Gates: fmt; clippy --all-targets -D warnings on CI's three sets; lib filtered 819/853/854/link/reap/metrics/observability/inbound_exits 89 passed (89 under ffi-uniffi); 6 integration files 60 passed (link_pool_819 3 runs green); links_ffi + peer_mutation_ffi + witness under ffi-uniffi 32 passed; 0 failed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C5FDihqVKTSXyNYs2CLnrK --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
3471dcd to
43ed890
Compare
…ops (#813 review) - hop_counter_candidates is {next+gap, next, next+gap+1 ..= next+gap+SLACK}: at most HOP_COUNTER_MAX_OPENS (SLACK+2) AEAD opens per frame whatever the reported gap. The dropped frames' own counters are never tried. Both open loops pick the counter through open_at_first_counter. - A peer-opened link whose frames find the arrivals queue full is AvDelivery::ArrivalsFull, counted as av_inbound_dropped_arrivals_full, remembered per link (bounded at 1024 links), and reported as dropped_before on the link's first delivered frame. - Witnesses: a junk frame after 1000 drops costs exactly SLACK+2 opens and the post-gap frame opens first try; an arrivals-full link with more drops than the slack reaches resyncs a real subscriber (fails with the gap report disabled). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ksniaWCvWoUwFQHRG3HQL
- open_av_link applies the operator deny-list to every federation candidate of the peer, not only the derived address. - open_av_link refuses an A/V address whose table entry names a different member than destination_key_id, before dialling, so a link to one member is never attributed to another. - A derived dial that fails after registering its consumer (identify failure on an established link) undoes the sink queue and the dial record and closes the link. - HopGap: a reported drop gap survives junk frames until a frame authenticates, in the subscriber loop and the relay pump. - A link whose consumer dropped its receiver stays recorded in the sink until LinkClosed and is never re-surfaced as a fresh arrival. - Frames refused at the #393 gate are counted into the link's next dropped_before, so the hop resyncs after a stale-binding window. - edge_node: perf.publish_fanout fails on link-open or member-send failures; throughput counts accepted member sends; a late joiner's pre-link frames are reported separately. Each fix has a witness that fails with the fix reverted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C5FDihqVKTSXyNYs2CLnrK
…undo clippy::too_many_lines (122/100) after the #813 review fixes. The checks (plane, member ownership, deny-list, transport key) move to av_dial_target and the failed-dial cleanup to undo_partial_av_dial. No behaviour change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C5FDihqVKTSXyNYs2CLnrK
A link dialled to a derived address in an A/V session's group (the av-stream: namespace) is LinkPlane::Av for life, classified at establishment by the same reverse-index probe as Scoped. The event loop matches on the plane once per frame: Identity/Scoped take the envelope path (attribute_and_deliver now takes an EnvelopePlane with no A/V variant), Av goes to the transport-owned AvSink. Misrouting is unrepresentable rather than checked. - Attribution: A/V frames pass the same #393 gate (gate_attribution, extracted unchanged from attribute_and_deliver), responder side by LinkIdentified, initiator side by the peer open_av_link dialled. - Backpressure: per-link bounded queue (AvLinkQueue, shared with LinkDataPump), drop-newest, never awaits; drops counted in the new EdgeMetrics::av_plane ledger and reported to the consumer so its dense hop counter skips the gap (InboundWireFrame::dropped_before). - #720: LeviculumAvSender sizes the frame against the link Channel limit (link MDU minus CHANNEL_ENVELOPE_HEADER_SIZE, capped at u16) and refuses with the typed ChunkTooLarge; the spine reports LegOutcome::Oversized. A refused or congested send no longer burns the hop counter. - API: ReticulumTransport::open_av_link / take_av_arrivals. - FSD §3.6 + invariant 10. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ksniaWCvWoUwFQHRG3HQL
Two loopback transports with field-shaped identities; A publishes through AvSpine over an A/V link while a replication round and an identity-plane Deliver run between the same transports. B receives every chunk in order and byte-identical, replication completes, no A/V-link frame reaches B's replication listener, a replication frame forced onto the A/V link stays on the A/V side, and an oversized chunk is refused as ChunkTooLarge without burning the hop counter. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ksniaWCvWoUwFQHRG3HQL
TransportMediaLink (Transport::send, Resource path) is replaced by AvPlaneMediaLink: one open_av_link per member to its address in the call's av-stream: group, each chunk one link-Channel message. Members install the call's addresses before Ready and read media from take_av_arrivals into the same mailbox. Observers keep the federation send as a refusal probe. perf.publish_fanout reports av links opened, open failures, chunks refused too large and congested. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ksniaWCvWoUwFQHRG3HQL
…ops (#813 review) - hop_counter_candidates is {next+gap, next, next+gap+1 ..= next+gap+SLACK}: at most HOP_COUNTER_MAX_OPENS (SLACK+2) AEAD opens per frame whatever the reported gap. The dropped frames' own counters are never tried. Both open loops pick the counter through open_at_first_counter. - A peer-opened link whose frames find the arrivals queue full is AvDelivery::ArrivalsFull, counted as av_inbound_dropped_arrivals_full, remembered per link (bounded at 1024 links), and reported as dropped_before on the link's first delivered frame. - Witnesses: a junk frame after 1000 drops costs exactly SLACK+2 opens and the post-gap frame opens first try; an arrivals-full link with more drops than the slack reaches resyncs a real subscriber (fails with the gap report disabled). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ksniaWCvWoUwFQHRG3HQL
…Edge::metrics() open_av_link reads the transport's attached bag (OnceLock, #810) instead of the pre-#810 Option. The live-node witness now builds its transports with no with_metrics and an Edge over each through EdgeBuilder, and asserts the av_plane counters land in Edge::metrics().snapshot() on both sides. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ksniaWCvWoUwFQHRG3HQL
metrics_snapshot carries every EdgeMetrics::av_plane entry as av_plane.<label>, beside #810's transport.* counters, so the A/V plane is visible to mobile operators as it is in the Rust snapshot and the pyo3 dict. Contract test the_uniffi_snapshot_carries_the_av_plane_ledger_805 (fails with the projection removed). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ksniaWCvWoUwFQHRG3HQL
A/V-plane frames route to deliver_av_frame and never entered attribute_and_deliver, the only place link_last_inbound_at was stamped. CIRISEdge#853's responder-side idle reap reads that stamp, so a peer-opened A/V link carrying media would read as idle and be reaped mid-call. The stamp is set after the #393 gate passes, so an unattributed flood cannot keep a link alive, and a quiet A/V link still ages out. The reply selector also reads the map but never selects an Av link. Witness: av_spine_live_node_805 asserts B holds a last-inbound stamp for the A/V link; it fails with the stamp removed. The quiet-link reap test waits on #853's seams reaching main. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C5FDihqVKTSXyNYs2CLnrK
- open_av_link applies the operator deny-list to every federation candidate of the peer, not only the derived address. - open_av_link refuses an A/V address whose table entry names a different member than destination_key_id, before dialling, so a link to one member is never attributed to another. - A derived dial that fails after registering its consumer (identify failure on an established link) undoes the sink queue and the dial record and closes the link. - HopGap: a reported drop gap survives junk frames until a frame authenticates, in the subscriber loop and the relay pump. - A link whose consumer dropped its receiver stays recorded in the sink until LinkClosed and is never re-surfaced as a fresh arrival. - Frames refused at the #393 gate are counted into the link's next dropped_before, so the hop resyncs after a stale-binding window. - edge_node: perf.publish_fanout fails on link-open or member-send failures; throughput counts accepted member sends; a late joiner's pre-link frames are reported separately. Each fix has a witness that fails with the fix reverted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C5FDihqVKTSXyNYs2CLnrK
…undo clippy::too_many_lines (122/100) after the #813 review fixes. The checks (plane, member ownership, deny-list, transport key) move to av_dial_target and the failed-dial cleanup to undo_partial_av_dial. No behaviour change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C5FDihqVKTSXyNYs2CLnrK
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 43ed890136
ℹ️ 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 bytes = encode_frame(KIND_MEDIA, header, chunk); | ||
| self.transport | ||
| .send(to, &bytes) | ||
| .await | ||
| .map(|_| ()) | ||
| .map_err(|e| e.to_string()) | ||
| match sender.send(&bytes).await { |
There was a problem hiding this comment.
With the mesh harness's default 500-byte MTU, the new sender accepts only a 425-byte Channel message, but this wraps an entire H.264 NAL from annexb_access_units—plus the sealed-chunk and JSON framing overhead—into one message. Normal 320×240 keyframe/slice NALs are substantially larger, so LeviculumAvSender returns ChunkTooLarge and the benchmark drops those video frames instead of exercising live A/V delivery. Split/reassemble media at the Channel limit, or otherwise size the generated units to the actual per-link budget.
Useful? React with 👍 / 👎.
| fanout_verdict( | ||
| frames_sent, | ||
| cfg.frames, | ||
| av_open_failures.len(), | ||
| delivery_failures, | ||
| ), |
There was a problem hiding this comment.
When a mid-stream joiner's Ready is delayed, its sends take the frames_awaiting_link branch and are skipped without incrementing either av_open_failures or delivery_failures. Although the new verdict checks those two failure counts, it can therefore report perf.publish_fanout as successful after omitting one or more frames for the late member—or even if that member never becomes ready. Include frames_awaiting_link in the verdict or validate the expected number of accepted member sends.
Useful? React with 👍 / 👎.
| for m in &admitted { | ||
| let opened = match occ.call_address(m) { | ||
| Some(addr) => link.open(m, &addr).await, |
There was a problem hiding this comment.
In the checked bench-mesh/compose.yaml topology, the publisher is directly attached only to relay-1, while subscribers bootstrap through that relay; nevertheless this dials each subscriber's announce-suppressed derived address directly. The transport explicitly treats such addresses as broadcast-only and only a directly attached neighbor can answer, while the relay is not admitted to the cohort and has no call address of its own. Consequently the actual relay-backed mesh reports NoRouteToPeer for every subscriber A/V link instead of carrying media; construct publisher→relay→subscriber A/V hops or make the benchmark topology directly attach the endpoints.
Useful? React with 👍 / 👎.
| match table.and_then(|t| t.accepts_inbound(dest.as_bytes())) { | ||
| Some(a) if crate::av_addressing::is_av_stream_group(a.group().group_id()) => LinkPlane::Av, | ||
| Some(_) => LinkPlane::Scoped, | ||
| None => LinkPlane::Identity, |
There was a problem hiding this comment.
If an inbound link establishes against a superseded A/V address just before seal_rotation retracts that address, but its queued LinkEstablished event is handled just after the seal, this live-table lookup returns None and permanently records the link as Identity. Retirement intentionally leaves already-established links running, so subsequent media on this link is routed into the envelope/replication path rather than the A/V sink. Preserve the destination's plane at acceptance or retain enough retired-address metadata to classify links whose establishment races the seal.
Useful? React with 👍 / 👎.
| Err(AvDispatcherError::Congested) => { | ||
| self.congested.fetch_add(1, Relaxed); | ||
| Err(AvDispatcherError::Congested.to_string()) |
There was a problem hiding this comment.
Under normal Channel pacing, LeviculumAvSender::send returns Congested when try_send sees Busy or PacingDelay, but this new media-link implementation converts that signal directly into a dropped delivery. The publisher's frame loop has no frame-rate sleep or backoff and immediately attempts the next frame, so after the first accepted packet it can run through the 15-fps source in milliseconds while repeatedly hitting the same pacing window and dropping most of the stream. Defer/retry according to PeerBackoff, or pace the source before continuing to the next encoded frame.
Useful? React with 👍 / 👎.
…ction) Two A/V links A dials to B's call address: one carries a frame every 250 ms, the other goes quiet after its first. With B's inbound idle bound at 2 s, B's #853 reap passes close the quiet link (its consumer's receiver ends) and leave the live one, whose frames deliver_av_frame stamps as last-inbound liveness, delivering. LinkBook::forget now clears the A/V sink and dial records, so every way a link leaves (event, teardown, reap, vanished) ends its consumer. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C5FDihqVKTSXyNYs2CLnrK
43ed890 to
337c5cd
Compare
|
Rebased onto main b4d4cd4 (#854, the #853 inbound reap). The fixes referenced as New in this round:
🤖 Generated with Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 337c5cd2de
ℹ️ 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".
| sender: crate::transport::realtime_av_runtime::LeviculumAvSender::new(node, link_id) | ||
| .with_metrics(metrics), |
There was a problem hiding this comment.
When a peer-opened link is used in the documented reverse direction (a subscriber dials a relay and the relay streams through AvArrival.sender), this plain LeviculumAvSender never updates link_last_outbound_at; successful sends only increment AV_SENT. The responder-side idle reaper therefore continues measuring from the initial inbound frame or establishment and closes the actively streaming link after the default 120-second bound. Wire successful sends from this sender into note_outbound_use (or otherwise exempt/mark the link busy) so outbound-only A/V traffic keeps an inbound link alive.
Useful? React with 👍 / 👎.
| self.check_blackhole(&candidate.dest_hash.into_bytes()) | ||
| .await?; | ||
| } | ||
| let Some(transport_ed25519) = candidates.first().map(|c| c.transport_ed25519) else { |
There was a problem hiding this comment.
When the rooted announce cache still contains a peer's old transport identity while the resolver has its rotated identity, resolve_dial_candidates deliberately returns candidates paired with different signing keys, but this unconditionally selects the first cached key. The derived A/V address itself is independent of those federation destination hashes, so the link proof is attempted with the stale key and open_av_link fails even though a valid current key is present later in the candidate list. Try the distinct candidate keys, or select the authoritative current binding, rather than always borrowing the first candidate's key.
Useful? React with 👍 / 👎.
This branch has not been deployed
1 participant
CIRISEdge#805 item 4. The A/V spine now rides the same
ReticulumTransport, node and event loop as replication. #720 lands here too.Design
LinkPlane::Av(FSD §3.6). A link dialled to a derived address whose group is in theav-stream:namespace is an A/V link for life. That is the namespaceav_addressing::session_group_idalready used, now one constant (AV_STREAM_GROUP_PREFIX). The plane is set at establishment by the same reverse-index probe that separates scoped links from identity links (Identity-plane replication rides the self-room's derived-address link — D2 drops every row UNATTRIBUTED (dest = D1's scoped address matches no rooted peer); one link pool serving two planes (#722's actual cause) #728). No frame content picks the route.LinkPlane::route).attribute_and_deliveris the only road to the replication sink, and it now takes anEnvelopePlane, which has no A/V variant. A/V frames go todeliver_av_frameand then the transport-ownedAvSink. Resources are refused on A/V links (AcceptNone). If one completes anyway, it is dropped by name.gate_attributionwas extracted unchanged fromattribute_and_deliver, and both paths call it. On the responder side the candidate peer comes fromLinkIdentified. On the initiator side it is the peeropen_av_linkdialled, using that peer's verified-route transport key and the link proof. There is no unattributed A/V path.AvLinkQueue, whichLinkDataPumpnow uses too). A full queue drops the newest frame. Nothing on the event loop waits. The receiver is told how many frames were dropped (InboundWireFrame::dropped_before), so the subscriber's dense hop counter skips the gap instead of desyncing. Drops are counted in a newEdgeMetrics::av_planeledger, which also appears in the snapshot and the pyo3 dict. Drops log at DEBUG with a throttle.HOP_COUNTER_MAX_OPENS(10) AEAD opens per frame, whatever the gap. The dropped frames' own counters are never tried.ArrivalsFulland counted underav_inbound_dropped_arrivals_full. The drop count is remembered per link (bounded at 1024 links) and reported on the link's first delivered frame, so the hop resyncs.LeviculumAvSenderchecks frame size againstlink_channel_message_limitbefore calling leviculum. That limit is the link MDU minusCHANNEL_ENVELOPE_HEADER_SIZE, capped at u16, and it shares one helper with the fragmenter. An oversized frame is refused as the typedChunkTooLarge, and the spine reports it asLegOutcome::Oversized. A refused or congested send no longer burns the hop counter, because the bytes never left the process.ReticulumTransport::open_av_link(peer, &MemberAddress)andtake_av_arrivals().edge_node.TransportMediaLink, which used the Resource path, is replaced byAvPlaneMediaLink, which uses the Channel path over A/V links. Each node installs the call's addresses beforeReady, and members read media from the A/V arrivals. Observers keep the federation send because it is a refusal probe.Witness
tests/av_spine_live_node_805.rsruns on real loopback Reticulum. A publishes throughAvSpinewhile a replication round and an identity-plane Deliver run on the same transports. B receives all chunks in order and byte-identical. The rows land and A's round completes. B's replication listener holds no A/V-link frame. Theav_planecounters match exactly. A replication frame forced onto the A/V link stays on the A/V side. An oversized chunk is refused asChunkTooLarge, and the next chunk still opens.With only the A/V route reverted to the envelope path (what main does), the test fails:
Review follow-up: with the gap report disabled, the arrivals-full witness fails (the hop never resyncs). The old candidate set made 1009 opens for the junk-frame case, against the new bound of 10.
Against
origin/mainthe test does not compile, becauseopen_av_link,take_av_arrivalsandav_planedo not exist there.Not witnessed over real links: a 500-byte-MTU link. Loopback TCP negotiates a 16 KiB link MTU. The check reads the live link's MDU, and the 425-byte arithmetic is pinned by unit tests.
Dependencies and scope
av_planecounters use the transport's attached metrics slot. The witness builds each Edge throughEdgeBuilderwith nowith_metrics, and asserts the counters land inEdge::metrics().snapshot(). Theav_planeledger is in the Rust snapshot, the pyo3 dict, and the UniFFI counters asav_plane.<label>.#[non_exhaustive]:transport::reticulum::LinkPlanegainsAv.transport::av_spine::LegOutcomegainsOversized.transport::realtime_av_dispatcher::AvDispatcherErrorgainsChunkTooLarge.observability::EdgeMetricsSnapshotgains theav_planefield (the struct is not#[non_exhaustive]).AvLinkReceivergains a defaultedrecv_framemethod.AvDeliveryispub(crate). The version bump is the lead's call.🤖 Generated with Claude Code
https://claude.ai/code/session_011ksniaWCvWoUwFQHRG3HQL