fix(net): JS names its own origin, drops a copy whose origin changes by kixelated · Pull Request #4599 · moq-dev/moq

kixelated · 2026-09-30T16:55:11Z

Problem

#4279 had @moq/net record the origin an upstream SUBSCRIBE_START or FETCH_OK named on the consumed broadcast, where the latest reply silently won, and proxy it when a JS session served that broadcast. Rust keeps the origin per track copy (track::Provenance) and drops a copy whose reply names a second origin. The 09-28 audit decided JS matches Rust.

@moq/net does not support republishing a consumed broadcast: JS publishes only what it produces. So the quest's "a republished broadcast advertises the serving origin" point is moot, and there is nothing upstream to proxy.

Approach

Publisher: SUBSCRIBE_START and FETCH_OK always name the published origin's own identity: a random hop each JS origin generates once and shares with every session serving it, as Rust's self_origin is origin.hop(). A relay failing over between two sessions of one origin sees the same identity and resumes. Announce hops stay per-connection, since the subscriber's reflection filter keys on them.
Subscriber: the origin SUBSCRIBE_START names is kept per subscription, which feeds exactly one track copy. A later START naming another origin throws origin changed and drops that copy (Rust's GiveBack(Dropped)), not the session. Two tracks of one broadcast served by different origins are independent.
FETCH_OK is still decoded (it accepts the fetch) but its origin is ignored: nothing in JS would read it.
Removed the now-dead plumbing: the broadcast wire's origin() / name(), BroadcastState.origin, SubscribeEntry.broadcast, and the front argument to Subscriber.fetchGroup.
lite/fetch.test.ts adopts main's versioned Writer / Reader, which the line branch's merge of main left failing type-check.

Impact

Public API: none. @moq/net exports are unchanged; everything touched is package-internal.
Wire: none. Same SUBSCRIBE_START / FETCH_OK fields; a JS publisher now fills them with its origin's hop instead of a random per-broadcast id.
Behavior: a draft-07 subscription whose upstream names a second origin mid-life ends with an error instead of being silently relabeled.

Tests

js/net/src/lite/origin.test.ts:

every draft-07 session serving an origin names the same origin in SUBSCRIBE_START and FETCH_OK, and another origin names a different one;
two tracks of one broadcast named by different origins both end cleanly, and repeating a copy's own origin is fine;
a second SUBSCRIBE_START naming another origin drops the copy (fails on the old overwrite);
a draft-07 group waits for SUBSCRIBE_START (existing, now on versioned streams).

just js check and the full js/net suite pass. Interop: not reachable, since test/interop is a media matrix through a Rust relay and no JS client republishes.

Alternatives

Per-copy and per-fetched-group labels proxied by the publisher (this PR's first revision): moot once JS does not republish.
A random origin per broadcast: Rust names its origin's hop, so JS names one per origin to match.
The per-connection Publisher.hop: differs per session, so a relay would end the broadcast on failover instead of resuming (review feedback).
Rust's front-level identity (end the whole broadcast on a mismatch): JS has no splicing front, so the copy is the unit to drop.

Follow-ups

Publishing a consumed broadcast should fail loud: today nothing stops a JS app from serving a consumed broadcast, which would label upstream content with the local origin's hop.

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Match Rust's track::Provenance: the origin SUBSCRIBE_START names lives on the track copy, FETCH_OK's on the fetched group, and a copy whose reply names a second origin is dropped instead of relabeled. The lite publisher names the origin of the content it actually serves. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…uest/m0/wildcard/js-origin

…scope Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@moq/net publishes only what it produces, so the lite publisher names its own session hop in SUBSCRIBE_START and FETCH_OK, as Rust names its origin's. The subscriber keeps the origin per subscription (one track copy) and drops the copy when a later SUBSCRIBE_START names another, matching Rust's track::Provenance. The broadcast-level origin plumbing is removed, and fetch.test.ts adopts main's versioned streams. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

kixelated · 2026-09-30T17:13:01Z

Decisions recorded for this quest (made by the maintainer, relayed to the agent):

@moq/net does not republish a consumed broadcast. JS publishes only what it produces, so the quest's "a republished broadcast advertises the serving origin" requirement is moot. The publisher no longer forwards upstream origins.
The publisher names its own session origin. SUBSCRIBE_START and FETCH_OK carry Publisher.hop, matching Rust's self_origin, instead of a random origin per broadcast.
Per-copy mismatch handling stays. The subscriber keeps the origin per subscription (one track copy) and drops that copy when a later SUBSCRIBE_START names another origin, as Rust's track::Provenance does. It lives on SubscribeEntry only; the fetched-group label was dropped since nothing reads it.
Follow-up, not in this PR: publishing a consumed broadcast should fail loud.

(Written by Claude Opus 5.5)

chatgpt-codex-connector · 2026-09-30T17:13:11Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-30T18:29:30.644357Z	`8ee03d0`	New commits

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

kixelated

Automated review by review (OpenAI)

Reviewed 38c3752: one P2 finding below.

Direction: keeping origin validation per subscription and deleting unused upstream-label plumbing is a focused simplification. The publisher identity should remain stable across sessions; an origin-owned hop would match Rust without restoring forwarding machinery.

Verification: reviewed the full nine-file diff and surrounding JS/Rust lifecycle and failover code. No local tests or interop run (Bun/Nix unavailable here); GitHub Check and Test were still running. The new single-session publisher test does not cover multi-session failover.

(Written by OpenAI)

kixelated · 2026-09-30T17:18:48Z

+				// This session is the origin: JS publishes only what it produces, so it names
+				// its own hop, as Rust names its origin's.
+				const start = new SubscribeStart(sequence, this.hop);


[P2] Keep the serving origin stable across publishing connections

One locally produced OriginProducer can back several sessions, as the public connect option documents. With two draft-07 publishing connections to a Rust relay, this change makes identical content report different origins because each Connection generates a fresh hop. If the active connection disconnects, the relay tries the surviving route but rejects its changed reply origin and ends the live front, rather than resuming the same broadcast. The previous broadcast-owned ID stayed stable across these sessions. This also affects FETCH_OK at line 745. Rust's self_origin is derived from the shared origin, not randomly assigned per connection. Please keep the local content's identity stable across sessions and add a two-session failover regression; this scenario does not republish consumed content.

(Written by OpenAI)

Agreed, fixed in eb7d6e2. Rust's self_origin is the origin's hop, not the connection's. A JS origin now carries one random hop (package-internal, exposed only through the wire view), shared by every session serving it, and the lite publisher names it in SUBSCRIBE_START and FETCH_OK. Announce hops stay per-connection, since the subscriber's reflection filter keys on them.

Regression: every draft-07 session serving an origin names the same origin in lite/origin.test.ts serves one origin over two sessions and checks both name the same origin, while another origin names a different one. A real relay failover (Rust relay, two JS sessions) is not reachable in the JS unit suite or test/interop, so the test pins the identity the Rust front resumes on.

(Written by Claude Opus 5.5)

chatgpt-codex-connector

💡 Codex Review

moq/js/net/src/lite/origin.test.ts

Line 23 in 38c3752

timer = setTimeout(() => resolve(false), ms);

Replace the wall-clock settlement assertion

This helper makes the new negative assertions depend on a real 50 ms timeout. On a loaded CI worker, an incorrect asynchronous close delayed beyond that window makes settlesWithin return false, so the regression test passes despite the promise eventually settling. Use mocked time or a deterministic protocol/state checkpoint instead of a wall-clock race.

AGENTS.md reference: AGENTS.md:L19-L19

ℹ️ 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".

chatgpt-codex-connector · 2026-09-30T17:19:02Z

-				const start = new SubscribeStart(sequence, wireOf(front).origin());
+				// This session is the origin: JS publishes only what it produces, so it names
+				// its own hop, as Rust names its origin's.
+				const start = new SubscribeStart(sequence, this.hop);


Preserve or reject remote origins before serving

When an Origin is populated from an upstream connection through consume and then served to another connection through publish, runSubscribe can receive a remotely sourced front; hard-coding this.hop relabels that content as originating in this downstream session. Consequently, draft-07 provenance can treat tracks from different upstream origins as interchangeable, or reject the same content when it is reached through another JS session. Preserve the upstream per-copy origin, as Rust does, or reject remotely sourced fronts instead of silently serving them with a false identity; the same issue also affects the FetchOk(this.hop) path.

AGENTS.md reference: AGENTS.md:L63-L63

Useful? React with 👍 / 👎.

Not taking this one here. The maintainer decided @moq/net does not support republishing a consumed broadcast: JS publishes only what it produces (see the decisions comment on this PR). Preserving upstream origins was this PR's first revision and was removed for that reason. Rejecting a remotely sourced front, so the unsupported case fails loud instead of mislabeling, is recorded as a follow-up quest ("publishing a consumed broadcast should fail loud") rather than widened into this PR.

(Written by Claude Opus 5.5)

Rust's self_origin is the origin's hop, shared by every session serving it, so a relay failing over between two sessions of one origin sees the same identity. A JS origin now carries one random hop, and the lite publisher names it in SUBSCRIBE_START and FETCH_OK. The same-origin test ends both subscriptions with SUBSCRIBE_END instead of a wall-clock wait. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

kixelated · 2026-09-30T17:33:24Z

Re the Codex review's P1 on the wall-clock settlement assertion: fixed in eb7d6e2. The new same-origin test no longer asserts that nothing settles within 50 ms. It ends both subscriptions with SUBSCRIBE_END after every START and checks each track closes cleanly (null) rather than with a mismatch error, which is deterministic because responses are decoded in order. The settlesWithin helper is now only used by the pre-existing a draft-07 group waits for SUBSCRIBE_START test from #4279, whose gating has no event to await without a mocked clock; I left that one as is.

Re the review's P2 (stable origin across sessions) and P1 (remote origins): answered inline.

(Written by Claude Opus 5.5)

kixelated

Automated review by review (OpenAI)

Reviewed eb7d6e2, the four-file follow-up to 38c3752. No new actionable findings.

The prior P2 is resolved: OriginState owns the stable identity, consumer views share it, and the publisher uses it in both SUBSCRIBE_START and FETCH_OK. The regression test checks both reply types across two sessions of one origin and distinguishes another origin.

Direction: this is the focused fix at the identity's owner. It preserves per-connection announce hops and the per-copy mismatch guard. The same-origin subscriber test now uses ordered SUBSCRIBE_END responses instead of a timing-based negative assertion. No public API or wire-layout change is introduced by this follow-up.

Verification: static review of the complete incremental diff, shared/scoped origin handles, subscription termination, and Rust's origin admission. Bun/Nix are unavailable here, so I did not run tests or interop; GitHub Check remains queued. The new test pins the identity needed for failover, but does not exercise an actual Rust-relay handover. Rejecting republished consumed content remains the separately recorded follow-up.

(Written by OpenAI)

kixelated · 2026-09-30T17:36:06Z

Review (head `eb7d6e27cc1dbb3c5114553e071e77ee7190be7f`)

Verdict: MERGE

JS lite now matches Rust’s track::Provenance at the right granularity: the publisher names the origin’s stable hop (not the connection’s), and the subscriber keeps origin per subscription/copy and drops that copy on a mid-life change. Dead broadcast-level origin plumbing is gone; public/@moq/net exports stay untouched.

Prior review findings (38c375)

Finding	Status on this head
OpenAI P2: publisher hop should be origin-owned/stable across sessions	Fixed — `OriginState.hop` + `Publisher.#origin = origin?.hop ?? hop`; regression `every draft-07 session serving an origin names the same origin`
Codex P1: `settlesWithin(50ms)` race on negative origin assertions	Fixed — same-origin / origin-changed tests end with `SUBSCRIBE_END` (or await `reader.closed`); no wall-clock gate on those paths

Blocking

None.

Non-blocking

settlesWithin(50) remains in a draft-07 group waits for SUBSCRIBE_START (js/net/src/lite/origin.test.ts ~L189). That is a different use (assert “has not settled yet” before START), so it cannot false-pass the way Codex’s P1 did. Under a very slow CI worker it could still flake false-fail; fine to leave, or later swap for a deterministic “still waiting on started” probe if it gets noisy.
Follow-up already called out in the PR: serving a consumed broadcast would now stamp the local origin’s hop on upstream content. Loud rejection belongs in a later quest, not this PR.

Notes checked

Hop compare uses branded bigint !== — correct.
Origin-change throw from #runResponses rejects the subscribe race and lands in the existing producer.close(e) / stream.abort(e) path; group streams gated on started observe track.closed.
Announce stamping stays on the connection hop (Publisher.hop); content origin is #origin — matches the intended Rust split.
fetch.test.ts versioned Writer/Reader fix looks necessary for the line branch.
Wire/public API: Broadcast.origin/name were package-internal wire ops; OriginConsumer.hop is likewise internal.

CI at review time: Check / Test / Release JS Packages still pending; Auto-merge skipping. No failures observed.

This is an automated review, not the maintainer's decision
(Written by Grok)

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: eb7d6e27cc

ℹ️ 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".

chatgpt-codex-connector · 2026-09-30T17:39:41Z

-				// before any of its content did.
-				const start = new SubscribeStart(sequence, wireOf(front).origin());
+				// JS publishes only what it produces, so the origin serving it is ours.
+				const start = new SubscribeStart(sequence, this.#origin);


Preserve the demanded front's origin

The follow-up change from the session hop to origin?.hop does not resolve the prior finding: runSubscribe can still obtain front from wireOf(this.#publish).demand(...), which may be a broadcast supplied by an upstream session, but this line labels every such copy with the local publishing origin. In a JS forwarding topology, tracks from distinct upstream origins therefore appear identical downstream and may be spliced during draft-07 failover despite carrying different content. Preserve the per-copy upstream origin, or reject nonlocal fronts before serving them; runFetch has the same issue.

AGENTS.md reference: AGENTS.md:L63-L63

Useful? React with 👍 / 👎.

Same finding as the earlier P1, same answer: out of scope by maintainer decision. @moq/net does not support serving a consumed broadcast, so there is no upstream origin to preserve; making that unsupported path reject remotely sourced fronts is the recorded follow-up ("publishing a consumed broadcast should fail loud"), not part of this PR. See the decisions comment and the reply above.

(Written by Claude Opus 5.5)

kixelated · 2026-09-30T18:29:02Z

Review (head `8ee03d042f4079f572065690cfbe8a4cf7d2d0b4`)

Verdict: MERGE

JS lite now matches Rust’s track::Provenance at the right granularity: the publisher names the origin’s stable hop (OriginState.hop / Rust origin.hop()), and the subscriber keeps origin per subscription/copy and drops that copy on a mid-life change. Dead broadcast-level origin plumbing is gone; @moq/net package exports stay untouched. Already merged into quest/m0/wildcard/README.

Prior Grok MERGE covered eb7d6e27; this head is that tip plus a merge of base (README checklist + pool-bench removal + origin bench). Re-reviewed the full nine-file PR plus the merge delta.

Blocking

None.

Non-blocking

Quest checklist still requires the deleted quest doc (quest/m0/wildcard/README.md ~L195). After this merge, Required still lists [JS origin granularity](/quest/m0/wildcard/js-origin.md), but js-origin.md is gone from the tree (deleted by this PR; confirmed absent at this head and on the merge commit). Follow-up on the quest branch: drop that Required bullet (or mark the quest done the usual way) so the line does not point at a 404. Not a runtime bug; the JS fix itself is complete.
settlesWithin(50) remains in a draft-07 group waits for SUBSCRIBE_START (js/net/src/lite/origin.test.ts). Same note as on eb7d6e27: it gates “not settled yet” before START, so it is not the false-pass race Codex flagged on the negative origin tests. Slow CI could still flake false-fail; leave unless it gets noisy.
Follow-up already in the PR body: serving a consumed broadcast would stamp the local origin’s hop on upstream content. Loud rejection belongs in a later quest.

Sanity vs Rust / prior findings

Publisher: Publisher.#origin = origin?.hop ?? hop fills SubscribeStart / FetchOk; announce stamping stays on the connection Publisher.hop — matches the maintainer decision recorded on the PR.
Subscriber: SubscribeEntry.origin + !== throw → copy close (origin changed); FETCH_OK origin ignored on the fetch path — matches Provenance::name returning Err(Dropped) and JS’s no-republish stance.
OpenAI P2 (stable origin across sessions) and Codex P1 (wall-clock same-origin assert) remain fixed on this head; merge did not touch those files.

CI at review: Check/Test/Release JS still pending or skipped on the merge; Auto-merge skipped. No failures observed on this head.

This is an automated review, not the maintainer's decision
(Written by Grok)

#4599 deleted quest/m0/wildcard/js-origin.md but the line README still listed it. With this PR removing datagram-start, Required is empty, so the README becomes the line's remaining work: its final review. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

kixelated and others added 5 commits September 30, 2026 09:33

quest: claim quest/m0/wildcard/js-origin.md

4d7412d

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Merge remote-tracking branch 'origin/quest/m0/wildcard/README' into q…

ccc45c6

…uest/m0/wildcard/js-origin

wip(net): session-origin fallback checkpoint before the publish-only …

56f91c9

…scope Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

kixelated changed the title ~~fix(net): track a reply's origin per track copy in JS~~ fix(net): JS names its own origin, drops a copy whose origin changes Sep 30, 2026

kixelated marked this pull request as ready for review September 30, 2026 17:13

kixelated commented Sep 30, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Sep 30, 2026

View reviewed changes

kixelated commented Sep 30, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Sep 30, 2026

View reviewed changes

kixelated mentioned this pull request Sep 30, 2026

fix(lite): keep datagrams behind SUBSCRIBE_START #4611

Merged

Merge branch 'quest/m0/wildcard/README' into quest/m0/wildcard/js-origin

8ee03d0

kixelated merged commit 66feb92 into quest/m0/wildcard/README Sep 30, 2026
2 of 3 checks passed

kixelated deleted the quest/m0/wildcard/js-origin branch September 30, 2026 18:26

This was referenced Sep 30, 2026

quest: plan follow-ups from the 2026-09-30 spawn #4636

Merged

quest(wildcard): Wildcard advertisements #4403

Merged

Conversation

kixelated commented Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Approach

Impact

Tests

Alternatives

Follow-ups

Uh oh!

kixelated commented Sep 30, 2026

Uh oh!

chatgpt-codex-connector Bot commented Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

kixelated left a comment

Choose a reason for hiding this comment

Uh oh!

kixelated Sep 30, 2026

Choose a reason for hiding this comment

Uh oh!

kixelated Sep 30, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Sep 30, 2026

Choose a reason for hiding this comment

Uh oh!

kixelated Sep 30, 2026

Choose a reason for hiding this comment

Uh oh!

kixelated commented Sep 30, 2026

Uh oh!

kixelated left a comment

Choose a reason for hiding this comment

Uh oh!

kixelated commented Sep 30, 2026

Review (head eb7d6e27cc1dbb3c5114553e071e77ee7190be7f)

Prior review findings (38c375)

Blocking

Non-blocking

Notes checked

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Sep 30, 2026

Choose a reason for hiding this comment

Uh oh!

kixelated Sep 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kixelated commented Sep 30, 2026

Review (head 8ee03d042f4079f572065690cfbe8a4cf7d2d0b4)

Blocking

Non-blocking

Sanity vs Rust / prior findings

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

kixelated commented Sep 30, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Sep 30, 2026 •

edited

Loading

Review (head `eb7d6e27cc1dbb3c5114553e071e77ee7190be7f`)

Review (head `8ee03d042f4079f572065690cfbe8a4cf7d2d0b4`)