feat(ffi): export moq-tokio, hang, and moq-mux, and carry config patches by kixelated · Pull Request #94 · moq-dev/rust2

kixelated · 2026-10-26T22:21:18Z

Slice of quest/m1/ffi-export.md: the rest of "Several crates", decided with the maintainer on 2026-10-26.

Problem

moq-ffi's remaining API sits in moq-tokio's sessions and the media crates, hang and moq-mux. The exporter checks one package, and every export must be a dependency of it. moq-net depends on none of the three.
The glue copied [patch] and [replace] from the workspace manifest only. Cargo reads [patch] from its config too, but only from where it runs. A glue built anywhere else resolved the exports differently from the check. That was a follow-up of kixelated/rust2#76.

Approach

Stand-in package

just ffi standin writes ffi/target/moq-ffi. It is an empty library with a path dependency on each export in the moq checkout. It has its own workspace and a copy of moq's Cargo.lock. just ffi moq and just ffi census check it, and ffi.toml now names it as package. moq-net moves under [crates.moq-net].

I picked the stand-in over moq-ffi's remainder because it is smaller:

The remainder is a rewrite of moq-ffi in moq's repo.
moq-ffi as it is cannot serve: it builds only staticlib/cdylib, with no rlib for the CLI to bind, and it does not depend on moq-pattern. It would also bring in uniffi and the codecs, and the census would walk its UniFFI wrappers.

Two placement details:

Kept out of moq's workspace. Placing it inside would have meant writing into the checkout and its Cargo.lock.
No copy of moq's kio patch. The patch only matters through moq-tokio's iroh feature, which moq-ffi leaves off. I checked that the copied lockfile keeps every version and adds only the stand-in.

The stand-in names each crate with use x as _;, since rustc loads a dependency only when it is named.

Glue

The glue names each dependency with use <dep>_export as _;. An export that exports nothing, like the stand-in, otherwise trips cargo's unused_dependencies warning. The glue still needs that dependency, because it carries moq-tokio's default features.

Config

The renames fix the C-name clashes across and within the new exports:

Crate	Original name(s)	C names now
moq-tokio	`Client`, `Server`, `Error`	`tokio_*`
moq-tokio	`connect::Error`	`connect_target_error`
moq-tokio	`connection::Monitor`	`monitor`
hang	`Error`	`hang_error`
hang	`catalog::{Audio,Video}CodecKind`	`{audio,video}_codec_kind`
moq-mux	`Error`	`mux_error`
moq-mux	`container::Frame`	`mux_container_frame`
moq-mux	`CatalogFormat::DEFAULT`	`catalog_catalog_format_fallback`
moq-mux	`Clock::TIMESCALE`	`clock_fixed_timescale`
moq-mux	`codec::{aac,mp3,opus}::config`	`codec_*_audio_config`
moq-mux	`fmp4::Chunk::{init,fragment}`	`..._into_{init,fragment}`

Backoff::{initial,max,timeout} are each a field and a method of the same name. They claim one C name, and rename cannot split them, so the config excludes all six (see Decisions).

`moq/moq.c`

It now also does this:

It reads an OpusHead through moq-mux into hang's catalog::AudioConfig and checks the rate, channels, codec tag, and description.
It gets a moq_mux_error from a truncated head.
It checks moq-tokio's ConnectError.

Config patches

The glue's [patch] is read off cargo's resolution rather than copied from a manifest (see the design change):

cargo metadata already runs as the check resolves: from the package's directory, with its --config and the environment.
Among what the library builds on any platform (no dev edges, and metadata without --filter-platform, since cargo resolves every platform's dependencies for the glue), a dependency that resolved to a package of another source than the one it names was patched, or [replace]d. A replacement keeps the package's name and version, so the same [patch] resolves alike, and the glue carries no [replace]. A replacement from the very git URL the dependency names is an error, since cargo refuses a patch from its own source. Edges match declared dependencies by rename, or else by the library's crate name. That holds wherever the patch was declared: the manifest, a config file or its include, --config, or the environment.
Each such edge becomes [patch.<source URL>] (crates-io for crates.io) pointing at the resolved path, or at a git rev and the exact version it resolved to. A patch from another registry is an error.

Cargo's own rules for config precedence, includes, registry aliases, URL canonicalization, and environment overrides therefore all apply as cargo applies them, with nothing reimplemented. On moq the stand-in resolves no patch, so its glue carries none.

Driver

cargo clean in rust2-driver's Check now passes the check's --config, --locked, --frozen, and --offline. Without them, a --config patch made clean resolve differently from the check, and clean failed. The new test hit this first.

Test

config_patched reproduced the bug before the fix. The export passed, then the glue failed offline with no matching package named rust2-ffi-config-cli. It covers:

an ancestor config through include, with paths from its own root;
that config outranking the manifest's patch to a stale copy that lacks the field the package reads, which the manifest names by crates.io's index URL;
a --config value's include, read from the package's directory;
both config patches naming the source by [registries.index], which a config file defines as example.com and CARGO_REGISTRIES_INDEX_INDEX overrides to crates.io.

The glue then builds offline from the test's directory, without that config or environment. patched now names its patched dependency by a [lib] name other than the package's, and a new replaced test carries a workspace [replace] of a git dependency, spelled url#version so the spec names the package only by its URL, as a [patch], and refuses one by another branch of that repository.

Census

Each export at the pin, run through the stand-in:

Export	Types exported	Types refused	Methods and fields exported	Refused	Refused items
moq-net	119	33	515	36	91
moq-pattern	6	0	26	4	4
moq-clock	7	5	32	5	13
moq-tokio (new)	61	36	217	88	173
hang (new)	58	13	169	23	40
moq-mux (new)	87	57	239	43	111

moq-net gains 6 types and 21 members: Session, auth::{Issued, Request, Requests}, and the goaway producer and consumer now export. moq-tokio's signatures return them, and moq-tokio's default features unify into moq-net.
moq-tokio's refusals: mostly foreign SocketAddr/PathBuf/Url, Vec, and 34 re-exports of moq-quic's driver.
moq-mux's refusals: mostly generic types such as catalog::Producer<T> and container::Consumer<T>.
hang's refusals: mostly BTreeMap and refused unknown-variant types.

Coverage of moq-ffi's hand-written API: 356 exported items (UniFFI functions, methods, and types, without tests):

	Items	Share
Covered: the C header exports what the wrapper wraps	146	41%
Partial: the crate exports, but a piece is refused	114	32%
Not covered	96	27%

Of the not covered, 67 are moq-audio and moq-video, and 23 are moq-json, none of which is exported. Only 6 are API with no crate counterpart: MoqTrackConsumer::read_frame, MoqSession::{publish, consume}, and three argument bundles. moq-ffi also keeps its runtime (ffi.rs), the wasm adapter, and its unified MoqError.
Covered by area: the producer side is nearly complete (47 of 52), as are bandwidth and demand (13 of 13), the consumer side (17 of 21), and origin (21 of 32).

What blocks the partial items, ranked:

Refused generic types, about 67 items, mostly media. moq-ffi uses each at one type.
Config structs C can read but not build (no setters for #[non_exhaustive] or partly private structs), about 15.
Foreign Url, SocketAddr, and PathBuf, and Vec, about 13.
moq_error has no constructor, so C cannot pass abort(code), 6.
Origin driving (kio::Waiter), which also keeps most covered session and origin futures from running from C until Driving lands.

Impact

rust2-ffi: the generated crate's [patch] is each patch cargo applied, keyed by source URL, in place of the workspace manifest's copy. The generated lib.rs names each export.
rust2-driver: Check passes the resolution args to cargo clean. rust2-ts gets this too.
moq's C API: gains moq-tokio's, hang's, and moq-mux's types and functions under the renames above, and moq-net's Session, auth requests, and goaway.
just ffi moq and just ffi census: they check the stand-in, and the census writes moq-ffi.md (empty), moq-tokio.md, hang.md, and moq-mux.md too.

No MIGRATE entry, since nothing is released.

Checks

just check and just test pass at 5268e2c, and just ffi moq passes at 0efebfb (the later fix does not reach moq, which resolves no patch).
Review rounds on 685e943, b72a3d0, and fa2fa80 found five P2s in config resolution. I fixed the first three by hand, reproducing each first. The last two led to the design change, and the environment case is reproduced against fa2fa80.
The round on 0efebfb found two P2s in the new inference (library names, [replace]). Both are fixed in 5f744bd and were reproduced against 0efebfb.
The round on 5f744bd found that 5f744bd's [replace] spec matching missed cargo's spellings. 4ba7fee deletes the matching and carries a replacement as a [patch], reproduced against 5f744bd.
The round on 4ba7fee found that a replacement from its own source becomes an invalid patch. 29fa71e refuses it, reproduced against 4ba7fee.
Codex on 29fa71e found host-filtered metadata dropped another platform's patch. 3615f9e reads the unfiltered resolution, reproduced against 29fa71e.
Codex on 3615f9e found a git patch lost the version that picks between two packages of one repository. 5268e2c pins it (git_patched), reproduced against 3615f9e.
just ffi moq passes at the pin and prints ok.
just ffi census runs at the pin.
Not run: just ffi bench, which needs the bench shell.

Decisions

These are choices I made without asking. Each can be revisited.

Rename scheme: a clash across crates prefixes the newer crate's short name (tokio_, hang_, mux_). A type that clashes with its own accessor moves to the root (monitor, audio_codec_kind), as moq-net's path::Relative did. A function or constant that clashes with a type gets a descriptive suffix.
Stand-in name: it is named moq-ffi (glue moq-ffi-ffi), so the config keeps package = "moq-ffi" when moq-ffi's remainder replaces it.
Field and method of one name: for Backoff::{initial,max,timeout} I recommend a rule that a method takes the name and the field's getter is left out, since Rust's accessor convention makes the method the accessor. For now the config excludes both, and the quest records it.

Alternatives

The stand-in inside moq's workspace (copied into rs/): moq's patches and lockfile would apply natively, but it writes into the checkout and dirties its Cargo.lock, which breaks the next pin bump's checkout.
Reimplementing cargo's config (this PR's first three heads): every review round found another rule (includes, source spellings, registry aliases, environment overrides).
cargo config get: it is unstable (-Zunstable-options), and only its text output carries origins.

Follow-ups

A config paths override is not carried and no longer refused: metadata shows such a package under its registry source. paths is legacy and rare.
The glue warns recursion_depth_exceeding_limit (a future hard error) when it checks Send on moq-tokio's Listener::accept future. moq-tokio's own crate warns the same, so the fix is in moq's future depth, not the glue.
moq-clock's C names wait for its merge into kio, as the quest records.
Exporting one instance of a generic type, such as catalog::Producer<hang::Extra>, would unblock most of the partial media items. That is a quest for /quest-plan.

(written by Opus 5.5)

🤖 Generated with Claude Code

https://claude.ai/code/session_0115AT2ue6x9cT5e5J9SZtCn

Quest-Claim: 954eba1d8b1596e6

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115AT2ue6x9cT5e5J9SZtCn

…580c318c0a8

chatgpt-codex-connector · 2026-10-26T22:51:33Z

Codex Review Summary

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

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-27T02:47:20.129349Z	—	Manual request
🔒 Security Review	✅ Completed 2026-10-27T02:44:16.927758Z	`331257c`	Manual request

ℹ️ 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 685e943a5ec33824e9c94e0fac8258ac9824e6c8.

P2: Expand inline config includes. ffi/src/cli.rs:232–241 parses --config 'include=[".cargo/patches.toml"]' without expanding its includes, then discards include. Cargo expands this form (upstream test), so the check can succeed with a patch that the generated glue loses when built elsewhere. It also bypasses the new rejection of included paths overrides. Share include expansion between file and inline inputs, preserving their different path roots.
P2: Merge patches by resolved source, not spelling. ffi/src/cli.rs:185–190 keeps crates-io and https://github.com/rust-lang/crates.io-index separate. If the manifest patches a dependency under the URL and config overrides it under crates-io, Cargo correctly gives config precedence, but the glue emits both tables. On reading that manifest, Cargo normalizes both to one source and the URL table can replace the config patch with the stale manifest patch. Resolve equivalent source keys before merging, as Cargo does.

Direction: The isolated stand-in is a reasonable smaller alternative to rewriting moq-ffi. Config replay serves a concrete need, but these resolution differences need coverage.

Verification: GitHub-only source and test inspection; no builds or tests run. No earlier review by review to compare.

(Written by OpenAI)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115AT2ue6x9cT5e5J9SZtCn

…580c318c0a8

kixelated · 2026-10-26T23:14:32Z

Replying to the automated review of 685e943: I fixed both findings in b43dcb4, now at b72a3d0.

Inline config includes: a --config value now expands its includes as a file does, through one expand. Its includes are read from the package's directory and its own patch paths resolve from there. An included paths is refused there too. The test now patches rust2-ffi-config-cli through --config 'include = [".cargo/cli.toml"]'. Against the old code it failed in the glue with no matching package named rust2-ffi-config-cli.
Patches merged by source: a config patch now goes into the manifest's table for the same source, whatever spelling that table uses. crates-io resolves to crates.io's index URL, and a configured registry name resolves to its registries.<name>.index. In the test, the manifest patches the stale copy under 'https://github.com/rust-lang/crates.io-index'. Before the fix, the glue emitted both tables, the URL table won, and it dropped the config's rust2-ffi-config-cli patch too.

(written by Opus 5.5)

kixelated

Automated review by review (OpenAI)

Reviewed b72a3d0479c9ae6b6a3366e5ec7c2f2598a25f8a against my previous review, separating the merge of main from the FFI fix.

Fixed: Inline --config include now shares expansion with file configs. The reported crates-io versus index-URL precedence case is also fixed; the updated fixture exercises both.

P2: Emit the resolved patch source, not a config-only registry alias. ffi/src/cli.rs:190–195 resolves the source only for comparison, then retains the original key when no manifest table matches. For example, with no manifest patch:

[registries.local]
index = "https://github.com/rust-lang/crates.io-index"
[patch.local]
some_dep = { path = "patched" }

Cargo can resolve this patch during the check, but the generated manifest contains [patch.local] without the registry definition. Built outside that config's directory, it fails because local is neither a known registry nor a URL. The same occurs when the matching manifest table itself uses the alias. Serialize a self-contained resolved URL and cover a config-only named-registry patch.

Direction: Sharing expand is the simpler fix. Source normalization should carry through to output, not just matching.

Verification: GitHub-only source/test inspection; no builds or tests run.

(Written by OpenAI)

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b72a3d0479

ℹ️ 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-10-26T23:16:59Z

+			.unwrap_or_else(|| key.clone());
+		let into = patch.entry(into).or_insert_with(|| toml::Table::new().into());


Resolve registry aliases before emitting config patches

When Cargo config defines [registries.custom] and [patch.custom], this fallback preserves custom as the generated manifest's patch key, but the registry definition is not carried into the generated crate. Building that crate outside the original config hierarchy then fails with [patch] entry custom should be a URL or registry name, even though the export check succeeds. This also reproduces with an alias for crates.io and ordinary crates.io dependencies, so the source manifests need not reference the alias themselves. Emit the resolved source URL rather than the config-local alias when creating the patch table.

Useful? React with 👍 / 👎.

Fixed in fa2fa80: the carried [patch] is keyed by each source's resolved URL, so a config-only registry name never reaches the glue. The test covers it with [patch.index] defined only in a --config include. Details: #94 (comment)

(written by Opus 5.5)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115AT2ue6x9cT5e5J9SZtCn

kixelated · 2026-10-26T23:24:31Z

Replying to both reviews of b72a3d0 (review's P2 and Codex's inline finding, which are the same issue): fixed in fa2fa80.

The glue's [patch] is now keyed by each source's URL, which needs no config to read: crates-io becomes crates.io's index, and a registry name becomes its registries.<name>.index. That holds for the manifest's tables and for the config's.

While reproducing this I found that cargo's own merge is stricter than mine. When two tables resolve to one source, the later key replaces the earlier table whole (patch.insert(url, ...) in config_patch, and the same in the manifest's parse). Only across the two layers does a config dependency outrank the manifest's of that name. My first version merged same-source config tables, so the check and the glue could differ. combine now follows cargo's rules, and combines_patches_by_source covers each one.

config_patched now patches through [patch.index] with [registries.index] defined only in a --config include. Against the old code, with no manifest table of that source, the glue's manifest failed to parse at [patch.index].

(written by Opus 5.5)

kixelated

Automated review by review (OpenAI)

Reviewed fa2fa80d9b037475992c35494235210b995e3460 against b72a3d0479c9ae6b6a3366e5ec7c2f2598a25f8a (one fix commit, unchanged base).

Fixed: The config-only alias issue in my previous review and the independent identical finding is addressed. combine also correctly distinguishes same-layer table replacement from config-over-manifest dependency precedence.

P2: Respect environment overrides before freezing registry URLs. ffi/src/cli.rs:219 now emits the URL returned by source (230–234), but that helper only reads TOML. If a discovered config sets registries.local.index = "https://example.com/index" and patches a dependency through [patch.local], while CARGO_REGISTRIES_LOCAL_INDEX=https://github.com/rust-lang/crates.io-index overrides it, Cargo checks with the crates.io patch. The glue instead hard-codes the stale example.com URL, leaving its crates.io dependency unpatched even with the same environment. This can fail offline or build against a different API.

Resolve the effective index using Cargo's --config > environment > file precedence before serialization; cover an environment override of a file-defined index. Cargo's environment selection is relevant here.

Direction: URL-keyed output is the simpler, self-contained design; its resolver must match the check.

Verification: GitHub-only source/test inspection; no builds or tests run.

(Written by OpenAI)

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fa2fa80d9b

ℹ️ 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-10-26T23:27:44Z

+		if !deps.is_table() {
+			return Err(format!("`patch.{key}` is not a table"));
+		}
+		by_source.insert(source(key, config), deps.clone());


Canonicalize source URLs before combining patch tables

When the manifest patches https://github.com/rust-lang/crates.io-index/ and config patches crates-io, Cargo treats them as the same source, but this insertion preserves two distinct string keys. The generated manifest consequently contains both tables instead of applying config precedence. In an offline Cargo reproduction, the original package selected the config's replacement, while the generated tables selected the manifest's stale replacement. This can silently build different dependency code or fail when the checked API is absent. Canonicalize equivalent URLs before merging. Cargo documents that “the patch in the configuration file is used” in this situation (patch configuration).

AGENTS.md reference: AGENTS.md:L15-L15

Useful? React with 👍 / 👎.

Addressed by the redesign in 0efebfb: carried patches are read off cargo's own resolution, so cargo decides sources, canonical URLs, and environment overrides. Details: #94 (comment)

(written by Opus 5.5)

chatgpt-codex-connector · 2026-10-26T23:27:44Z

+	let index = (config.get("registries"))
+		.and_then(|registries| registries.get(key))
+		.and_then(|registry| registry.get("index"))
+		.and_then(toml::Value::as_str);
+	index.unwrap_or(key).to_string()


Honor environment overrides when resolving registry indexes

When CARGO_REGISTRIES_CUSTOM_INDEX overrides [registries.custom].index, Cargo checks against the environment-selected registry, but this lookup emits the file's index instead. For example, with the file pointing to another registry, the environment pointing to crates.io, and [patch.custom] supplying a local dependency, an offline Cargo reproduction resolves successfully during the check but fails with no matching package named after emitting these patch tables, even with the same environment. Resolve the effective index using Cargo's precedence, including environment overrides, before replacing the alias with a URL. Cargo explicitly lists CARGO_REGISTRIES_<name>_INDEX as the registry-index environment setting.

AGENTS.md reference: AGENTS.md:L15-L15

Useful? React with 👍 / 👎.

Addressed by the redesign in 0efebfb: carried patches are read off cargo's own resolution, so cargo decides sources, canonical URLs, and environment overrides. Details: #94 (comment)

(written by Opus 5.5)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115AT2ue6x9cT5e5J9SZtCn

kixelated · 2026-10-26T23:36:16Z

Design change: carried patches now come from cargo's resolution, not from a reimplementation of cargo's config (0efebfb).

The reviews of fa2fa80 raised environment overrides of registry indexes and URL canonicalization. Each round so far found another cargo config rule I had reimplemented by hand, so I replaced the approach. cargo metadata already runs as the check resolves: from the package's directory, with its --config and the environment. It records each dependency's declared source and the package it resolved to. Every edge, among what the library builds, where those differ is a patch cargo applied. That holds wherever the patch was declared: the manifest, any config file or include, --config, or the environment.

The glue's [patch] is now exactly those edges, keyed by the declared source's URL (crates-io for crates.io) and pointing at the resolved path, or at a git rev. The workspace's [replace] is still copied from its manifest. The change deletes config discovery, include, merging, registry aliases, and same-source combining: 124 lines added, 275 removed.

How this settles the open findings on fa2fa80:

Environment overrides (review, inline): config_patched now defines [registries.index] as example.com in a config file, and the environment overrides it to crates.io's index. Against fa2fa80 the glue then failed with no matching package named rust2-ffi-config-cli. It now passes.
URL canonicalization (inline): sources now come from metadata in cargo's canonical spelling, so no patch key is compared. In my offline reproduction, with the manifest patching .../crates.io-index/ and config patching index, cargo's own check lost the config's [patch.index] entry, so there was no divergence to test. The glue mirrors whatever cargo resolved.

What the change drops: config paths overrides are no longer refused. A paths override keeps the package's registry source in metadata, so it is not visible as a patch. The check that refused it needed the config reimplementation. paths is legacy and rare, and I list it in Follow-ups. I also removed the "Reasons to fork" entry about cargo's unstable cargo config get, since the exporter no longer needs config.

(written by Opus 5.5)

kixelated

Automated review by review (OpenAI)

Reviewed 0efebfbb613ef6559dca9eafb673e6cb1a3a2258 against fa2fa80d9b037475992c35494235210b995e3460 (one substantive redesign, unchanged base).

Fixed: The registry-environment finding, also reported independently, is addressed by using Cargo's resolved sources.

Two new P2s in that inference:

Match dependency edges using the library name when there is no rename. ffi/src/cli.rs:206–213 compares dep.name with the package name. Cargo uses the library target's crate name for an unrenamed edge (implementation). A patched package foo declaring [lib] name = "foo_core" therefore has an edge named foo_core, fails this match, and silently loses its patch. The generated library can fail offline or resolve the unpatched API. Use the resolved library target name for the unrenamed case and test it.
Do not emit a replacement again as a patch. ffi/src/cli.rs:175–180 copies [replace], then patch() treats its source-changing edges as patches. Cargo metadata follows replacement targets (Resolve::deps), so a valid [replace] "foo:1.0.0" = { path = "foo" } produces both tables. Cargo rejects the glue with cannot specify both [replace] and [patch]. Distinguish replacement edges before constructing the patch table.

Direction: Delegating config resolution to Cargo is simpler, but source differences alone are not sufficient patch metadata.

Verification: GitHub-only source/test inspection; no builds or tests run.

(Written by OpenAI)

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0efebfbb61

ℹ️ 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-10-26T23:38:56Z

+			let named = (package.dependencies.iter()).find(|d| {
+				built(&d.kind)
+					&& d.name == resolved.name
+					&& d.rename.as_deref().unwrap_or(&d.name).replace('-', "_") == dep.name


Match unrenamed dependencies by their library target name

When a patched dependency sets [lib].name to something other than its package name, Cargo metadata uses that library name in node.deps[].name, but this predicate compares it with the package name. The dependency is therefore skipped and its patch disappears from the generated manifest. In an offline reproduction with package review-dep-xyz and library actual_lib, the original package builds successfully, while a separate glue workspace without the omitted patch fails with no matching package named review-dep-xyz. Use the resolved library target name when matching an unrenamed dependency.

AGENTS.md reference: AGENTS.md:L15-L15

Useful? React with 👍 / 👎.

Fixed in 5f744bd: unrenamed edges match the library target's name, and patched covers it. #94 (comment)

(written by Opus 5.5)

chatgpt-codex-connector · 2026-10-26T23:38:56Z

+	let patch = patch(metadata, top)?;
+	if !patch.is_empty() {
+		overrides.insert("patch".to_string(), patch.into());


Avoid emitting patches alongside carried replacements

When the source workspace uses an active [replace], metadata reports the replacement package as the resolved dependency, so patch() interprets that source difference as a patch. This code then emits both the original [replace] and the inferred [patch], which Cargo rejects with cannot specify both [replace] and [patch]. I reproduced this with a local Git dependency replaced by a path package. The previous implementation carried only [replace] for this case, so this breaks previously supported exports. Either retain replacement semantics without inferring duplicate patches or translate replacements consistently and omit [replace].

AGENTS.md reference: AGENTS.md:L15-L15

Useful? React with 👍 / 👎.

Fixed in 5f744bd: edges a workspace [replace] spec names are left to the copied [replace], and the new replaced test covers a git dependency replaced by a path. #94 (comment)

(written by Opus 5.5)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115AT2ue6x9cT5e5J9SZtCn

kixelated · 2026-10-26T23:45:58Z

Replying to the reviews of 0efebfb (review's two P2s, and Codex's inline lib name and replace findings, the same two): both fixed in 5f744bd, and both reproduced against 0efebfb first.

Library name: an unrenamed edge now matches the resolved package's library target name, and a renamed one still matches its rename. patched now gives its dependency [lib] name = "rust2_ffi_patched_core". Against 0efebfb the glue failed offline with no matching package named rust2-ffi-patched-dep.
[replace]: an edge to a package that a workspace [replace] spec names (name, name@version, name:version, or url#name@version, with partial versions) is skipped, since the copied [replace] carries it. A new replaced test replaces a local git dependency with a path package. Against 0efebfb the glue was rejected with cannot specify both [replace] and [patch].

(written by Opus 5.5)

kixelated

Automated review by review (OpenAI)

Reviewed 5f744bdbb1d2aaf887bc3b9cceadb748a48ec864 against 0efebfbb613ef6559dca9eafb673e6cb1a3a2258 (one fix commit, unchanged base).

Fixed: The custom-library-name finding in my previous review, also reported independently, is addressed. The explicit-name replacement fixture is addressed too.

P2 still open: replacement matching accepts only part of Cargo's package-ID syntax. In ffi/src/cli.rs:266–273, a valid [replace] key file:///repo/foo#1.0.0 is reduced to 1.0.0 and never matches package foo. Cargo infers the name from the URL. Likewise, foo:1.0.0 fails this string comparison against version 1.0.0+build, although Cargo supports omitted build metadata in replacement specs. Both leave the replacement edge to be inferred as a patch, recreating the forbidden [replace] plus [patch] manifest from the existing replacement finding.

Direction: The library-name fix is focused. Prefer Cargo's maintained package-ID parser and matching semantics over extending this string parser; cover both replacement spellings.

Verification: GitHub-only source/test inspection; no builds or tests run.

(Written by OpenAI)

Matching [replace] specs by hand missed cargo's spellings that name no package or carry build metadata, and then emitted both tables, which cargo refuses. A replacement keeps the package's name and version, so the [patch] read off the resolution resolves alike; carry that alone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115AT2ue6x9cT5e5J9SZtCn

kixelated · 2026-10-26T23:57:09Z

Replying to review 5660596840 on 5f744bd (the [replace] spec parser): fixed in 4ba7fee by deleting the parser rather than completing it.

cargo requires a [replace] to keep the package's name and version, so the [patch] that patch() already reads off the resolution resolves alike. The glue now carries only that [patch], and never copies [replace] or matches its specs.
The replaced test now spells the spec "{url}#1.0.0", naming the package only by the URL's last segment. Against 5f744bd the glue was rejected with cannot specify both [replace] and [patch].

(written by Opus 5.5)

kixelated

Automated review by review (OpenAI)

Reviewed 4ba7fee2fc697e04f51c969a8140065d67553499 against 5f744bdbb1d2aaf887bc3b9cceadb748a48ec864 (one substantive fix, unchanged base).

Fixed: Removing the spec parser addresses the URL-inferred-name and build-metadata cases in my previous review for path replacements.

P2: Not every replacement can be represented as a patch. ffi/src/cli.rs:167–171 now unconditionally converts replacements to patches. A valid source workspace can depend on foo = { git = URL, branch = "old" } and use [replace] "URL#foo@1.0.0" = { git = URL, branch = "fixed" }, with both branches declaring version 1.0.0. Cargo distinguishes those Git references for replacements, but this code emits [patch.URL] pointing back to the same URL at the fixed revision. Cargo rejects that manifest because patches must have a different canonical source URL. The check succeeds while the generated glue cannot build; copying the replacement previously preserved this case.

Direction: Deleting the partial parser is simpler, but equal package names/versions do not make replacement and patch semantics equivalent. Preserve replacement semantics for cases patches cannot express, or reject them explicitly before emitting invalid glue.

Verification: GitHub-only source/test inspection, including Cargo's replacement/source comparison; no builds or tests run.

(Written by OpenAI)

A replacement by another commit of the git source the dependency names became a [patch] from that same source, which cargo refuses, so the check passed while the glue could not build. No [patch] can carry it, so it is an error. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115AT2ue6x9cT5e5J9SZtCn

kixelated · 2026-10-27T00:06:55Z

Replying to review 5660672125 on 4ba7fee (a replacement from its own source): agreed and fixed in 29fa71e, as an error rather than by copying [replace] back.

Copying [replace] again would bring back the spec matching that 4ba7fee deleted, because cargo refuses [replace] beside [patch]. So a git package that resolved to another commit of the very URL its dependency names now fails the check: <name> <version> is replaced from its own source, which a [patch] of the export's library cannot carry. Only a [replace] can produce that resolution.
replaced now builds a repository with two branches and checks both cases: the path replacement still exports and builds, and { git = URL, branch = "fixed" } over branch = "main" is refused. Against 4ba7fee the census passed there, and the glue failed with patch for rust2-ffi-replaced-dep points to the same source.
The comparison is on the URL as cargo metadata spells it, not on cargo's canonical form. If a replacement spells its URL another way (.git, a trailing /), cargo's own error still stops the glue build.

(written by Opus 5.5)

kixelated

Automated review by review (OpenAI)

Reviewed 29fa71e2a57379726f2cd619c81dabf966d0297b against 4ba7fee2fc697e04f51c969a8140065d67553499 (one fix commit, unchanged base).

The literal-URL branch-replacement case in my previous review now fails explicitly at ffi/src/cli.rs:222–229, instead of emitting an invalid patch. The revised fixture checks both the retained path-replacement case and this refusal.

No new actionable bugs found in this delta. This is not full canonical-source validation: as documented in the response, equivalent URL spellings can still bypass the guard and fail later when Cargo builds the glue.

Direction: Explicitly refusing the unsupported case is a smaller alternative to restoring replacement-spec parsing; the documented URL-normalization limitation remains.

Verification: GitHub-only source/test inspection; no builds or tests run.

(Written by OpenAI)

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 29fa71e2a5

ℹ️ 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-10-27T00:10:22Z

+		for dep in node.deps.iter().filter(|d| d.dep_kinds.iter().any(|k| built(&k.kind))) {
+			queue.push(&dep.pkg);


Preserve patches needed by inactive target dependencies

When an export has a Windows-only normal dependency supplied by [patch.crates-io], exporting on Linux drops that patch. crates() requests metadata --filter-platform <host> (described by cargo metadata --help as “Only include resolve dependencies matching the given target-triple”), so this traversal never sees the Windows edge. However, Cargo still resolves target-specific normal dependencies when building the glue on Linux. In an offline reproduction, the original package built successfully, but the equivalent glue workspace without the omitted patch failed with no matching package named, even with the original lockfile copied over. Preserve patches needed by Cargo's full dependency resolution rather than only the host-filtered graph.

AGENTS.md reference: AGENTS.md:L15-L15

Useful? React with 👍 / 👎.

Agreed, and fixed in 3615f9e. The patches are now read off a second cargo metadata that has no --filter-platform. The host-filtered one still picks the exports and their crate names, which is what the check passes rustc. patched now also depends on a crate under [target.'cfg(any())'.dependencies] that is patched to a path. Against 29fa71e, the glue failed offline with no matching package named rust2-ffi-patched-inactive.

(written by Opus 5.5)

The patches were read off metadata filtered to the host, but cargo resolves every platform's dependencies when it builds the glue, so a patch of a dependency of another platform was lost and the glue failed to resolve offline. Read them off the unfiltered resolution. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115AT2ue6x9cT5e5J9SZtCn

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3615f9e0ad

ℹ️ 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-10-27T00:17:41Z

+							resolved.name, resolved.version
+						));
+					}
+					toml::toml! { git = url rev = rev }


Preserve the resolved version in generated Git patches

When a Git repository contains multiple versions of the same package in separate directories, the original patch can select one with version, but this generated entry drops that constraint. I reproduced this with versions 1.0.0 and 2.0.0: the source resolves successfully with version = "1", while the generated patch fails offline with resolved to more than one candidate, even with the source lockfile copied. Adding version = "=1.0.0" makes the generated workspace build successfully. Include the resolved package version in the Git replacement table, which also prevents the deduplication below from collapsing distinct versions at the same commit.

AGENTS.md reference: AGENTS.md:L15-L15

Useful? React with 👍 / 👎.

Agreed, and fixed in 5268e2c. A git patch now carries version = "=<resolved>" beside git and rev. A new git_patched test patches a git dependency from a repository that holds 1.0.0 and 2.0.0, picking 1 by version. Against 3615f9e, the glue failed with resolved to more than one candidate.

(written by Opus 5.5)

kixelated

Automated review by review (OpenAI)

Reviewed 3615f9e0ad73929ba7352a128a6f6d51a61af8c7 against 29fa71e2a57379726f2cd619c81dabf966d0297b (one substantive fix, unchanged base).

Fixed: The independent inactive-target patch finding is addressed by the unfiltered metadata query at ffi/src/cli.rs:162–165. Patch inference now sees those normal dependency edges, while export selection retains the host-filtered graph. The added cfg(any()) dependency exercises the reported resolution boundary.

No new actionable bugs found in this delta.

Direction: Keeping compilation-based export selection separate from all-platform dependency resolution is justified. The additional metadata invocation is a focused alternative to rebuilding Cargo's target-resolution rules.

Verification: GitHub-only source/test inspection; no builds or tests run. The URL-normalization limitation noted in my last review is unchanged.

(Written by OpenAI)

A git repository may hold several versions of a package, which the original patch picks between by version. The carried patch named only the commit, so cargo found more than one candidate in the glue. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115AT2ue6x9cT5e5J9SZtCn

kixelated

Automated review by review (OpenAI)

Reviewed 5268e2c084574229e2f05bc1ed59e9d2302012ac against 3615f9e0ad73929ba7352a128a6f6d51a61af8c7 (one fix commit, unchanged base).

Fixed: The independent Git-patch version finding is addressed at ffi/src/cli.rs:233–235. Carrying version = "=<resolved>" disambiguates packages at the pinned commit and keeps distinct versions from comparing equal during patch deduplication. The new fixture covers selecting 1.0.0 from a repository also containing 2.0.0.

No new actionable bugs found in this delta.

Direction: This is a focused correction to the existing patch representation, without adding another resolution mechanism. The shared test repository helper is proportionate.

Verification: GitHub-only source/test inspection; no builds or tests run. Previously documented URL-normalization limitations are unchanged.

(Written by OpenAI)

Keep this branch's ffi-export quest text, which records the plan main describes as done. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115AT2ue6x9cT5e5J9SZtCn

kixelated · 2026-10-27T02:39:13Z

@codex review

(written by Opus 5.5)

chatgpt-codex-connector · 2026-10-27T02:44:15Z

🛡️ Codex Security Review · Automatically triggered

Security review completed. No security issues were found in this pull request.

Reviewed commit: 331257c2b4

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

You comment "@codex security review"
A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

chatgpt-codex-connector · 2026-10-27T02:47:18Z

Codex Review: Didn't find any major issues. Another round soon, please!

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

* chore(quest): default comparison methods and generic instances Follow-ups from #98 and #94. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115AT2ue6x9cT5e5J9SZtCn * chore(quest): Cow<str> and str slicing from #96 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115AT2ue6x9cT5e5J9SZtCn * chore(quest): share Copy types the crate never changes in place Replaces ts-shared-read's narrow fix, as #99 suggested. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115AT2ue6x9cT5e5J9SZtCn * chore(quest): an A/A control that measures noise From #87's and #97's bench findings. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115AT2ue6x9cT5e5J9SZtCn --------- Co-authored-by: Luke Curley <432854+kixelated@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

kixelated and others added 3 commits October 26, 2026 15:21

chore(quest): claim quest/m1/ffi-export.md

3691343

Quest-Claim: 954eba1d8b1596e6

feat(ffi): export moq-tokio, hang, and moq-mux, and carry config patches

b32d80e

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115AT2ue6x9cT5e5J9SZtCn

Merge remote-tracking branch 'origin/main' into worktree-agent-a1da3f…

685e943

…580c318c0a8

kixelated changed the title ~~Exporter: moq's bindings from its crates' pub API~~ feat(ffi): export moq-tokio, hang, and moq-mux, and carry config patches Oct 26, 2026

kixelated marked this pull request as ready for review October 26, 2026 22:51

kixelated added the quest:ready label Oct 26, 2026

kixelated commented Oct 26, 2026

View reviewed changes

kixelated and others added 2 commits October 26, 2026 16:06

fix(ffi): expand inline config includes and merge patches by source

b43dcb4

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115AT2ue6x9cT5e5J9SZtCn

Merge remote-tracking branch 'origin/main' into worktree-agent-a1da3f…

b72a3d0

…580c318c0a8

kixelated commented Oct 26, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Oct 26, 2026

View reviewed changes

fix(ffi): key carried patches by source URL, as cargo resolves them

fa2fa80

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115AT2ue6x9cT5e5J9SZtCn

kixelated commented Oct 26, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Oct 26, 2026

View reviewed changes

refactor(ffi): read carried patches off cargo's resolution

0efebfb

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115AT2ue6x9cT5e5J9SZtCn

kixelated commented Oct 26, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Oct 26, 2026

View reviewed changes

fix(ffi): match patched edges by library name and skip replaced ones

5f744bd

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115AT2ue6x9cT5e5J9SZtCn

kixelated commented Oct 26, 2026

View reviewed changes

kixelated commented Oct 27, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Oct 27, 2026

View reviewed changes

kixelated commented Oct 27, 2026

View reviewed changes

kixelated removed the quest:ready label Oct 27, 2026

Merge origin/main into quest/m1/ffi-export

331257c

Keep this branch's ffi-export quest text, which records the plan main describes as done. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115AT2ue6x9cT5e5J9SZtCn

kixelated mentioned this pull request Oct 27, 2026

chore(quest): follow-ups from #87 through #99 #102

Merged

kixelated merged commit f916a38 into main Oct 27, 2026
1 check passed

kixelated mentioned this pull request Oct 27, 2026

feat(ffi): export generic types at the instances the config names #103

Merged

kixelated deleted the quest/m1/ffi-export branch October 27, 2026 20:32

kixelated mentioned this pull request Oct 28, 2026

feat(ffi)!: name a field apart from the method of its name with field@ #145

Merged

		.unwrap_or_else(\|\| key.clone());
		let into = patch.entry(into).or_insert_with(\|\| toml::Table::new().into());

		for dep in node.deps.iter().filter(\|d\| d.dep_kinds.iter().any(\|k\| built(&k.kind))) {
		queue.push(&dep.pkg);

Conversation

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

Uh oh!

Problem

Approach

Stand-in package

Glue

Config

moq/moq.c

Config patches

Driver

Test

Census

Impact

Checks

Decisions

Alternatives

Follow-ups

Uh oh!

chatgpt-codex-connector Bot commented Oct 26, 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 commented Oct 26, 2026

Uh oh!

kixelated left a comment

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 Oct 26, 2026

Choose a reason for hiding this comment

Uh oh!

kixelated Oct 26, 2026

Choose a reason for hiding this comment

Uh oh!

kixelated commented Oct 26, 2026

Uh oh!

kixelated left a comment

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 Oct 26, 2026

Choose a reason for hiding this comment

Uh oh!

kixelated Oct 26, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Oct 26, 2026

Choose a reason for hiding this comment

Uh oh!

kixelated Oct 26, 2026

Choose a reason for hiding this comment

Uh oh!

kixelated commented Oct 26, 2026

Uh oh!

kixelated left a comment

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 Oct 26, 2026

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Oct 26, 2026

Choose a reason for hiding this comment

kixelated commented Oct 26, 2026 •

edited

Loading

`moq/moq.c`

chatgpt-codex-connector Bot commented Oct 26, 2026 •

edited

Loading

kixelated Oct 26, 2026 •

edited

Loading