relay: resolve a pipelined joining FETCH against its own subscription

[afrind]

In local-forwarder mode, a joining FETCH pipelined behind its SUBSCRIBE could arrive before the relay finished setting up that subscription. The relay sent such a fetch upstream unresolved, where the publisher resolved it against the relay's own upstream subscription and its old or missing Largest. The fetch now waits for the setup and resolves against the joiner's Largest. A joining fetch that does not resolve fails at the relay. The wait adds an upstream RTT on a cold track and lets the cache serve the fetch.

---

This change is 

Summary by CodeRabbit

• Bug Fixes
  • Joining fetch requests now wait for subscription setup to complete before resolving their requested range, preventing premature fetch attempts.
  • Fetches now return an error if setup fails, no matching subscription is available, or the request cannot be resolved.
  • Unresolved joining fetch requests are no longer forwarded upstream, avoiding requests that cannot be matched to an active subscription.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used

📚 Code guidelines (4)

CONTRIBUTING.md — configured
docs/dev/local-forwarder-flow.md — configured
docs/dev/track-filter-ranking.md — configured
docs/relay-hops.md — configured

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: openmoq/moqx/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: de96efb5-14f7-40bb-8dae-2033ab9e962c

📥 Commits

Reviewing files that changed from the base of the PR and between 01ba7bb and 0231329.

📒 Files selected for processing (2)

• src/MoqxRelay.cpp
• src/MoqxRelay.h

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

---

📝 Walkthrough

Walkthrough

Local-forwarder joining FETCH requests now wait for subscription setup, resolve against the resulting subscription, and dispatch as standalone fetches. Resolution errors are returned without forwarding unresolved requests upstream.

Changes

Joining FETCH handling

Layer / File(s)	Summary
Resolve joining FETCH requests src/MoqxRelay.h, src/MoqxRelay.cpp, test/MoqxRelayFetchTests.cpp	The resolver checks for a matching session subscription, waits for pending setup, and converts a valid joining FETCH to a standalone fetch. Tests cover missing subscriptions and mismatched request IDs.
Wait for setup and dispatch src/MoqxRelay.cpp, test/MoqxRelayFetchTests.cpp	The subscriber filter retries resolution after setup completes, then dispatches the resolved FETCH. Tests check successful setup, failed setup, and that unresolved FETCH requests do not reach upstream.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: akash-a-n

Merge Risk: ⚪ Minimal · up to 02313

Joining FETCH requests now wait for their pipelined SUBSCRIBE setup and resolve against it, and failures return errors instead of going upstream unresolved. Tests cover delayed setup, setup failure, a missing subscription and a mismatched request ID. No concrete merge-blocking risk was identified.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 02313

The change narrows upstream forwarding by resolving joining requests against the caller’s subscription and rejecting unresolved requests. No introduced security vulnerability was established. Remaining uncertainty concerns production authorization and how waiting requests are bounded and cancelled.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The demonstrated exposure is joining FETCH traffic admitted to the local-forwarder publisher interface, its requested track, and the relay’s existing cache or selected upstream publisher. A stalled setup can delay dependent FETCH requests. Tenant isolation and the maximum independently attackable session or process scope cannot be determined from the available authorization and admission evidence.

Trust Boundaries and Controls

• observed — The changed path preserves the captured request session through waiting and delegates to the existing dispatch boundary only after local resolution. Namespace validation remains in fetchImpl. These visible request controls do not establish production authentication or tenant authorization; those controls and the forwarder dependency’s internal ownership checks remain outside the reviewed source.

Resilience and Maintainability Implications

• observed — Each pending joining FETCH retains its request, session, consumer, and readiness future until setup advances or waiting is interrupted. The changed helper contains no explicit deadline or admission bound. Registry failure wakes waiters, but session-level cancellation and outstanding-request limits were not available, so an exploitable resource-exhaustion regression is not established.

Hardening Proposals

• proposed — Establish the effective outstanding-request bound and cancellation guarantee for FETCH requests waiting on setup. Add targeted interruption and stalled-setup coverage; introduce a local bound or deadline only if the session layer does not already provide one.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: resolving a pipelined joining FETCH against its own subscription.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

_{Comment @coderabbitai help to get the list of available commands.}

[afrind]

@coderabbitai Thanks for the review.

Addressed in 0231329: the setup could finish before the waiting task started, and the readiness-guarded loop then skipped re-resolution. The helper now waits and re-resolves at least once.

Not addressing:

• Docstring coverage. This repo comments invariants and non-obvious behavior only, so we don't add docstrings to meet a coverage threshold.
• Cross-session ownership. The resolver looks the subscriber up by the fetch's own session ID, so a joining fetch can only match a subscription from its own session.
• Cancellation and teardown during the wait. The wait lasts only until the subscribe's setup succeeds or fails. A failed or torn-down setup removes the registry entry, and the retry then fails the fetch. This matches the existing wait in the subscribe path.

🤖 Generated with Claude Code

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
ast-grep outline src/MoqxRelay.cpp --items all --match 'resolveJoiningFetchOnSubscriberExec|fetch|subscribe|removeSubscriber' --view expanded
rg -n -C4 'resolveJoiningFetchOnSubscriberExec|setupComplete|setupPromise|joiningRequestID' src/MoqxRelay.cpp src/MoqxRelay.h
git show --stat 02313297

Length of output: 5580

---

🏁 Script executed:

#!/bin/bash
sed -n '210,270p' src/MoqxRelay.cpp
sed -n '2290,2415p' src/MoqxRelay.cpp
sed -n '2575,2604p' src/MoqxRelay.cpp
rg -n -C5 'setupPromise|setupBaton|pendingSetup|\.erase\(|removeSubscriber\(|getSubscriber\(' src/MoqxRelay.cpp

Length of output: 11860

---

@afrind Thanks for the clarification and guidance.

I checked the updated code:

• Setup race: fetchAfterSetup now uses a do/while loop. It waits and re-resolves at least once, even if setup finishes before the task starts.
• Cross-session ownership: The resolver passes the FETCH’s own session.sessionId() to resolveJoiningFetch. I withdraw this concern.
• Cancellation and teardown: The setup path clears the pending entry and wakes waiters on failure. The FETCH then retries resolution. This matches the existing SUBSCRIBE wait. I withdraw this concern.
• Docstring coverage: A coverage threshold does not justify comments that conflict with the repository’s documentation policy. I withdraw this request.

---

✏️ Learnings added

Learnt from: afrind
URL: https://github.com/openmoq/moqx/pull/783

Timestamp: 2026-10-02T01:36:45.318Z
Learning: In openmoq/moqx, comments document invariants and non-obvious behavior. Do not request docstrings solely to meet a documentation coverage threshold.

---

Learnt from: afrind
URL: https://github.com/openmoq/moqx/pull/783

Timestamp: 2026-10-02T01:36:45.318Z
Learning: In openmoq/moqx src/MoqxRelay.cpp, MoqxRelay::resolveJoiningFetchOnSubscriberExec resolves a joining FETCH using the FETCH's own MoQSession::sessionId(), which restricts subscription matching to that session. Pending local-forwarder setup waits follow the existing subscribeFromSubscriberExec pattern: setup completion wakes waiters, and failed or torn-down setup removes the registry entry so retrying resolution fails when no replacement exists.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

You are interacting with an AI system.

[gmarzot]

@gmarzot reviewed 3 files and all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on akash-a-n).