Skip to content

py: split moq-ffi bindings from the moq-rs ergonomic wrapper into two packages#1551

Merged
kixelated merged 7 commits into
mainfrom
claude/moq-ffi-per-language-publish-Fd7dC
May 30, 2026
Merged

py: split moq-ffi bindings from the moq-rs ergonomic wrapper into two packages#1551
kixelated merged 7 commits into
mainfrom
claude/moq-ffi-per-language-publish-Fd7dC

Conversation

@kixelated
Copy link
Copy Markdown
Collaborator

@kixelated kixelated commented May 30, 2026

Why

We bundled the raw uniffi bindings and the ergonomic Python wrapper into a single moq-rs wheel, deliberately, to avoid publishing moq-ffi per language. The cost showed up later: the wrapper can't be versioned independently. Improving the Python API forces a moq-ffi crate bump, and a moq-ffi crate bump forces a wrapper re-release even when the wrapper didn't change.

The constraint we thought blocked the split ("uniffi-linked libraries can't be split across separately packaged wheels") only forbids splitting the native library itself across multiple FFI packages. A pure-source wrapper layered on top of one FFI package is fine, and the source tree already had that two-layer split (moq._uniffi vs moq/*.py).

This PR does the split for Python, and documents the repeatable pattern for Swift/Kotlin/Go to follow later.

Prior art

This isn't new territory: the repo used to ship exactly this two-package shape and abandoned it for the bundling shortcut. Before #1432, moq-net was a pure-python wrapper with dependencies = ["moq-ffi~=0.2.0"], and moq-ffi was its own maturin wheel published on moq-ffi-v* tags. #1432 ("Add Swift and Kotlin FFI wrappers") collapsed moq-net into a bundled maturin wheel (dropping the moq-ffi dep), and #1450 renamed it to moq-rs. This PR restores the Phase-1 architecture. The auto-publish-on-merge gate here is also the same model the old py/common/release.sh used (check PyPI for name/version, publish on 404, skip on 200), so the moq-ffi~=0.2.x float is a proven pattern in this repo rather than a new bet.

What

Two PyPI distributions instead of one:

  • moq-ffi (import moq_ffi): the raw uniffi bindings. Maturin project at py/moq-ffi, tracks rs/moq-ffi via the existing moq-ffi-v* tag (release-py-ffi.yml).
  • moq-rs (import moq): the pure-python ergonomic wrapper at py/moq-rs. The moq distribution name is taken on PyPI, so it keeps the original moq-rs name, which preserves backward compatibility (existing pip install moq-rs; import moq users keep working, now transitively pulling in moq-ffi). Depends on moq-ffi ~= 0.2.16 (PEP 440 compatible release = >=0.2.16,<0.3), so installs float to the latest moq-ffi patch automatically without a wrapper re-release. First independent release is 0.3.0 (above the old lockstep 0.2.x line).

Wrapper release flow (auto-publish on merge)

The wrapper is versioned by hand but released automatically, using the same model release-plz uses for crates: the registry is the source of truth.

  • Bump version in py/moq-rs/pyproject.toml in a normal PR.
  • On merge to main (with py/moq-rs/** changes), release-py.yml reads that version and publishes to PyPI only if it isn't already there (with retry/timeout on the registry check so a network blip doesn't fail the gate).
  • No separate tag to remember. Two reusable helpers were added to release.sh (read-version scoped to the [project] table, pypi-exists) so the same gate drops in for future split wrappers.

FFI side is unchanged: release-plz bumps rs/moq-ffimoq-ffi-v* tag → moq-ffi wheel. A moq-ffi minor bump (0.3, potentially breaking) is the only thing that forces re-releasing the wrapper.

Workflow naming

  • release-py.yml → the moq-rs wrapper (the package most callers install; the headline name).
  • release-py-ffi.yml → the moq-ffi bindings (tag-triggered matrix build), alongside the other per-language FFI release files.

Supporting changes

  • py/moq-rs keeps the import package moq/; hatchling is pointed at it via packages = ["moq"] since the dist name (moq-rs) no longer matches the import name (moq).
  • Workspace members listed explicitly (not py/*) so build output like py/dist isn't mistaken for a member.
  • pytest asyncio_mode = "auto" moved to the workspace root pyproject.toml (pytest resolves its rootdir there when running both members).
  • py/justfile: _develop builds moq-ffi (maturin) + installs the moq-rs wrapper editable; new package recipe builds the wrapper wheel; the ci recipe (lint+test+build, scope-skip guard) is preserved so the root just ci dispatch keeps working.
  • .gitignore, test/smoke.sh, example docstrings repointed to the new paths.

Cross-package sync

  • CLAUDE.md py/ block + the umbrella-wheel rationale rewritten.
  • doc/lib/py/ updated (the old page referenced a non-existent moq-net Python package; now documents moq-rs + moq-ffi), nav updated.
  • kt/README.md note corrected to reflect that a wrapper-on-top split is allowed (Python now does it).

Pattern for Swift / Kotlin / Go (future PRs)

Same shape: an FFI package tracking rs/moq-ffi plus a wrapper package versioned independently, published when its version is new. Documented in the plan; Go caveat: Go pins exact versions (minimal version selection) and won't auto-float to the latest patch the way pip/SPM/Gradle do.

Test plan

  • import moq_ffi and import moq both resolve; moq.Session flows through moq_ffi
  • ruff check + format clean, pyright 0 errors
  • pytest py/moq-rs/tests py/moq-ffi/tests → 35 passed
  • uv build --package moq-rsmoq_rs-0.3.0-py3-none-any.whl with Requires-Dist: moq-ffi~=0.2.16, bundling the moq/ package
  • uv lock records moq (root), moq-ffi, moq-rs
  • release.sh read-version/pypi-exists verified locally: reads 0.3.0 (decoy version in another table ignored); moq-rs==0.3.0 → exists=false (would publish); a known package → exists=true (would skip)
  • both release workflows parse as valid YAML

Out of scope

  • Implementing the Swift/Kotlin/Go splits (pattern documented only).

(Written by Claude)

@kixelated kixelated changed the title py: split moq-ffi bindings from the ergonomic wrapper into two packages py: split moq-ffi bindings from the moq-rs ergonomic wrapper into two packages May 30, 2026
claude added 2 commits May 30, 2026 15:22
Publish the raw uniffi bindings (`moq-ffi`, import `moq_ffi`) and the
ergonomic wrapper (`moq`, import `moq`) as separate PyPI distributions so
the wrapper can be versioned independently of the moq-ffi crate.

Previously a single `moq-rs` wheel bundled both layers, forcing a wrapper
re-release on every moq-ffi crate bump and forcing a crate bump to ship any
wrapper change. The "can't split uniffi libraries across wheels" constraint
only forbids splitting the native library itself; a pure-source wrapper
layered on top of one FFI package is fine.

- py/moq-ffi: maturin project, dist `moq-ffi`, tracks rs/moq-ffi via the
  existing moq-ffi-v* tag (release-py.yml).
- py/moq: pure-python wrapper, dist `moq`, depends on `moq-ffi ~= 0.2.16`
  so it floats to the latest moq-ffi patch without a re-release. Versioned
  by hand; released on a moq-py-v* tag (new release-py-wrapper.yml).
- Workspace root renamed to moq-workspace to free the `moq` name; pytest
  asyncio config moved to the root so both members share it; members listed
  explicitly so build output (py/dist) isn't treated as a member.
- Updated CLAUDE.md, doc/lib/py, kt/README, test/smoke.sh, and the py
  justfile (new `package` recipe) for the two-package layout.

https://claude.ai/code/session_01EegH6PKeUcknQJ2TP7D7RX
A stray edit added the hatchling packages block twice, which made the TOML
fail to parse and broke uv/maturin. Keep the single block near the top.

https://claude.ai/code/session_01EegH6PKeUcknQJ2TP7D7RX
@kixelated kixelated force-pushed the claude/moq-ffi-per-language-publish-Fd7dC branch from 8bed5f4 to 324ec04 Compare May 30, 2026 15:22
@kixelated kixelated changed the base branch from dev to main May 30, 2026 15:22
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 30, 2026

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

This PR restructures the Python MOQ workspace by separating native UniFFI bindings from the Python wrapper. moq-ffi becomes a standalone maturin-built package exposing low-level bindings, while moq-rs is refactored as a pure-Python wrapper that depends on moq-ffi. moq-rs modules are rewired to import from moq_ffi instead of local ._uniffi. Packaging and tooling are updated (moq-rs → hatchling, explicit workspace members, shared _develop recipe, package recipe), tests/docs adjusted, and two release workflows added/changed: a tag-driven multi-platform release for moq-ffi and a merge-driven, PyPI-gated publish for moq-rs.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: splitting moq-ffi bindings from moq-rs wrapper into two separate packages, which is the core objective of this PR.
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.
Description check ✅ Passed The pull request description comprehensively explains the motivation, technical rationale, implementation details, and test plan for splitting Python FFI bindings from the wrapper into two packages.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch claude/moq-ffi-per-language-publish-Fd7dC

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/release-py-wrapper.yml:
- Line 68: The magic-nix cache action step using
DeterminateSystems/magic-nix-cache-action@908b263ff629f4cc17666315b7fd3ec127c6244d
can run during PRs and populate shared cache; guard that step by adding an
if-condition to skip it for pull_request events (e.g., use if: github.event_name
!= 'pull_request' or equivalent) so the step that references
DeterminateSystems/magic-nix-cache-action@908b263ff629f4cc17666315b7fd3ec127c6244d
only executes on non-PR workflows.

In `@py/justfile`:
- Around line 26-31: The check recipe no longer runs the Python test suites;
update the justfile's check target (the "check:" recipe) to either invoke the
existing test recipe or re-add the pytest invocation: add a step that runs
"pytest moq-rs/tests/ moq-ffi/tests/" (or simply call "just test" if a test
recipe exists) after the linting steps so that "just check" runs all tests and
linters as expected.

In `@py/moq-ffi/README.md`:
- Around line 17-22: The README example uses await at module scope causing
SyntaxError; wrap the example in an async function and run it with asyncio.run.
Specifically, put the MoqClient() creation and the await
client.connect("https://relay.quic.video") call inside an async def main() ->
None function and invoke it via asyncio.run(main()), referencing the MoqClient
class and its connect method in the example.

In `@py/moq-rs/examples/announced.py`:
- Around line 3-4: Update the example usage lines in the announced.py docstring
to point to the correct path for this PR: replace occurrences of
"py/moq/examples/announced.py" with "py/moq-rs/examples/announced.py" so the
example commands (the two lines starting with "python ... announced.py --url")
reference the actual script location (announced.py) in this branch.

In `@py/moq-rs/examples/clock.py`:
- Around line 7-8: Docstring command paths in the clock.py example are
incorrect; update the two command examples that currently reference
"py/moq/examples/clock.py" to point to the actual script location
"py/moq-rs/examples/clock.py" so the publish/subscribe usage in the clock.py
example is accurate (search for the two lines with "python
py/moq/examples/clock.py publish" and "python py/moq/examples/clock.py
subscribe" inside clock.py and replace the path portion).

In `@py/moq-rs/examples/serve_clock.py`:
- Around line 9-10: Update the example run commands in serve_clock.py to use the
correct repository path "py/moq-rs/examples" instead of "py/moq/examples";
specifically, change the referenced invocations for serve_clock.py and clock.py
(the example commands that begin with "python py/moq/examples/...") to "python
py/moq-rs/examples/serve_clock.py --bind 127.0.0.1:4443" and "python
py/moq-rs/examples/clock.py subscribe \\" so the examples point to the actual
files in this change set.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1c095218-a0fd-4e24-81e7-f155c530a4e6

📥 Commits

Reviewing files that changed from the base of the PR and between ad71591 and 324ec04.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (29)
  • .github/workflows/release-py-wrapper.yml
  • .github/workflows/release-py.yml
  • .gitignore
  • CLAUDE.md
  • doc/.vitepress/config.ts
  • doc/lib/index.md
  • doc/lib/py/index.md
  • doc/lib/py/moq-rs.md
  • kt/README.md
  • py/justfile
  • py/moq-ffi/README.md
  • py/moq-ffi/moq_ffi/__init__.py
  • py/moq-ffi/pyproject.toml
  • py/moq-ffi/tests/test_smoke.py
  • py/moq-rs/README.md
  • py/moq-rs/examples/announced.py
  • py/moq-rs/examples/clock.py
  • py/moq-rs/examples/serve_clock.py
  • py/moq-rs/moq/__init__.py
  • py/moq-rs/moq/client.py
  • py/moq-rs/moq/origin.py
  • py/moq-rs/moq/publish.py
  • py/moq-rs/moq/server.py
  • py/moq-rs/moq/subscribe.py
  • py/moq-rs/moq/types.py
  • py/moq-rs/pyproject.toml
  • py/moq-rs/tests/test_server.py
  • pyproject.toml
  • test/smoke.sh

Comment thread .github/workflows/release-py-wrapper.yml Outdated
Comment thread py/justfile
Comment on lines 26 to 31
check:
uv sync --no-install-workspace
uv run --no-sync ruff check .
uv run --no-sync ruff format --check .
cd moq-rs && uv run --no-sync maturin develop --uv
just _develop
uv run --no-sync pyright
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Keep check running the Python tests.

just py check now stops before pytest, so the usual validation entrypoint no longer covers the moq-rs and moq-ffi test suites. Please fold the existing pytest moq-rs/tests/ moq-ffi/tests/ step back into check, or have check delegate to test, so this command still matches the repo's expected contract. Based on learnings: "Run just check to execute all tests and linting". (Written by Claude)

Suggested change
 check:
     uv sync --no-install-workspace
     uv run --no-sync ruff check .
     uv run --no-sync ruff format --check .
     just _develop
     uv run --no-sync pyright
+    uv run --no-sync pytest moq-rs/tests/ moq-ffi/tests/
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
check:
uv sync --no-install-workspace
uv run --no-sync ruff check .
uv run --no-sync ruff format --check .
cd moq-rs && uv run --no-sync maturin develop --uv
just _develop
uv run --no-sync pyright
check:
uv sync --no-install-workspace
uv run --no-sync ruff check .
uv run --no-sync ruff format --check .
just _develop
uv run --no-sync pyright
uv run --no-sync pytest moq-rs/tests/ moq-ffi/tests/
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@py/justfile` around lines 26 - 31, The check recipe no longer runs the Python
test suites; update the justfile's check target (the "check:" recipe) to either
invoke the existing test recipe or re-add the pytest invocation: add a step that
runs "pytest moq-rs/tests/ moq-ffi/tests/" (or simply call "just test" if a test
recipe exists) after the linting steps so that "just check" runs all tests and
linters as expected.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skipping this one: it's based on a misreading of the original recipe. The pre-existing check recipe on main never ran pytest either, it ran lint + format + maturin + pyright. Tests live in the separate test recipe, and just py ci runs check + test + build (so CI does cover the suites). The CLAUDE.md "Run just check to execute all tests and linting" refers to the root just check, which fans out to just <lang> ci per scope, not to just py check. This PR preserves that contract exactly.

(The real cause of the earlier CI failure was unrelated: the justfile rewrite had accidentally dropped the py ci recipe, which the root dispatch invokes. That's restored in a349d33.)

(Written by Claude)


Generated by Claude Code

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kixelated, thanks for the detailed clarification — that makes complete sense. The check/test/ci separation is intentional and preserved correctly; my comment misapplied the root-level just check semantics to the Python-scoped recipe. I'll record the correct contract.


✏️ Learnings added
Learnt from: kixelated
Repo: moq-dev/moq PR: 1551
File: py/justfile:26-31
Timestamp: 2026-05-30T15:34:49.855Z
Learning: In the moq-dev/moq repository (py/justfile), `just py check` (the Python-scoped check recipe) runs lint + format + editable build + pyright only — it does NOT run pytest. Tests are in the separate `just py test` recipe. `just py ci` orchestrates `check` + `test` + `build` for full CI coverage. The root `just check` fans out to `just <lang> ci` per scope, so the full suite (including tests) is always covered in CI. Do not flag `just py check` for missing pytest invocations.

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.

Comment thread py/moq-ffi/README.md
Comment thread py/moq-rs/examples/announced.py Outdated
Comment thread py/moq-rs/examples/clock.py Outdated
Comment thread py/moq-rs/examples/serve_clock.py Outdated
claude added 2 commits May 30, 2026 15:33
The CI "Check" job failed because the py/justfile rewrite accidentally
dropped the `ci` recipe, which the root `just ci` dispatch invokes via
`just py ci "$files"`. Restore it (lint + tests + build with the
scope-skip guard), matching the original contract.

Also address CodeRabbit review nits:
- Fix stale `py/moq/examples/` -> `py/moq-rs/examples/` docstring paths in
  the three example scripts.
- Wrap the moq-ffi README snippet in `async def main()` + `asyncio.run`
  so it isn't a module-scope `await` (SyntaxError on copy-paste).

https://claude.ai/code/session_01EegH6PKeUcknQJ2TP7D7RX
The wrapper release was gated on remembering to push a `moq-py-v*` tag
separately from the version bump, so a merged bump could sit unreleased.
Replace that with the same model release-plz uses for crates: the registry
is the source of truth.

release-py-wrapper.yml now runs on merge to main (py/moq-rs changes), reads
the static version from pyproject.toml, and publishes to PyPI only if that
version isn't already there. No tag to remember; bumping the version in a PR
and merging is the whole release flow.

Adds two reusable helpers to release.sh (`read-version`, `pypi-exists`) so
the same gate drops in for future split wrappers.

https://claude.ai/code/session_01EegH6PKeUcknQJ2TP7D7RX
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.github/workflows/release-py-wrapper.yml (1)

30-32: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Do not serialize PR builds with mainline publishes.

Line 31 uses one global concurrency key for both pull_request and push. With cancel-in-progress: false, any wrapper PR build can queue ahead of the main release and delay publishing to PyPI.

Suggested change
 concurrency:
-  group: release-py-wrapper
+  group: release-py-wrapper-${{ github.event_name }}-${{ github.ref }}
   cancel-in-progress: false
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/release-py-wrapper.yml around lines 30 - 32, The current
concurrency block uses a single static key ("group: release-py-wrapper") so
pull_request and push workflows share the same queue; change the concurrency
group to include the event context (e.g. use a dynamic key like
"release-py-wrapper-${{ github.event_name }}" or otherwise separate keys for
pull_request vs push) so PR runs do not serialize with mainline publish runs,
and keep cancel-in-progress configured appropriately per-event (for example
leave false for PRs but avoid blocking push releases).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/scripts/release.sh:
- Around line 82-87: read_version currently grabs the first "version" line
anywhere in the manifest which can pick up a wrong section; change read_version
to explicitly find the [project] table and only read the version key inside it:
scan the manifest for the "[project]" header, then within that section (until
the next ^\[.*\] header) extract the first line starting with "version" and set
version from it (preserving the existing variable names manifest and version and
function name read_version), failing if no version is found.

---

Outside diff comments:
In @.github/workflows/release-py-wrapper.yml:
- Around line 30-32: The current concurrency block uses a single static key
("group: release-py-wrapper") so pull_request and push workflows share the same
queue; change the concurrency group to include the event context (e.g. use a
dynamic key like "release-py-wrapper-${{ github.event_name }}" or otherwise
separate keys for pull_request vs push) so PR runs do not serialize with
mainline publish runs, and keep cancel-in-progress configured appropriately
per-event (for example leave false for PRs but avoid blocking push releases).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d54f147c-3c78-4073-ad4d-aba18b7a3710

📥 Commits

Reviewing files that changed from the base of the PR and between a349d33 and 1bb8810.

📒 Files selected for processing (4)
  • .github/scripts/release.sh
  • .github/workflows/release-py-wrapper.yml
  • CLAUDE.md
  • py/moq-rs/pyproject.toml
✅ Files skipped from review due to trivial changes (1)
  • CLAUDE.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • py/moq-rs/pyproject.toml

Comment thread .github/scripts/release.sh Outdated
`moq-rs` (the wrapper, import `moq`) is the package most callers install, so
it gets the headline `release-py.yml` name. The raw `moq-ffi` bindings build
moves to `release-py-ffi.yml`, alongside the per-language FFI release files.
Updates the self-path filters, concurrency groups, and doc references to
match.

Also folds in two CodeRabbit findings on these files:
- read_version now reads the version from the [project] table specifically,
  so a `version` key in another table can't be picked up by mistake.
- The wrapper workflow's concurrency group keys on event + ref, so a PR build
  can't queue ahead of (and delay) a release on main.

https://claude.ai/code/session_01EegH6PKeUcknQJ2TP7D7RX
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
.github/workflows/release-py-ffi.yml (1)

21-23: ⚡ Quick win

Scope concurrency by event and ref.

This fixed group lets a long PR dry-run sit ahead of a tag publish, which can delay the actual release. Matching the wrapper workflow's event_name and ref scoping avoids that queueing hotspot.

Suggested change
 concurrency:
-  group: release-py-ffi
+  group: release-py-ffi-${{ github.event_name }}-${{ github.ref }}
   cancel-in-progress: false
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/release-py-ffi.yml around lines 21 - 23, The concurrency
group "release-py-ffi" should be scoped by the triggering event and ref to avoid
unrelated runs blocking each other; modify the concurrency.group value in the
workflow's concurrency block (currently for release-py-ffi) to include
github.event_name and github.ref (for example by using an expression like `${{
github.event_name }}-${{ github.ref }}-release-py-ffi`) so the workflow is
partitioned by event and ref while leaving cancel-in-progress as desired.
.github/workflows/release-py.yml (1)

61-63: ⚡ Quick win

Avoid interpolating the version directly into run.

Line 63 injects a repo-derived value into the shell. That is what zizmor is warning about. Pass it through env and read $VERSION instead.

Suggested change
       - name: Check PyPI
         id: pypi
-        run: .github/scripts/release.sh pypi-exists moq-rs "${{ steps.version.outputs.version }}"
+        env:
+          VERSION: ${{ steps.version.outputs.version }}
+        run: .github/scripts/release.sh pypi-exists moq-rs "$VERSION"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/release-py.yml around lines 61 - 63, The workflow step
with id "pypi" (name "Check PyPI") is interpolating
steps.version.outputs.version directly into the run string; instead export that
value via env and reference it in the script to avoid injecting repo-derived
data into the shell. Update the "pypi" step to set an environment variable
(e.g., VERSION) to ${{ steps.version.outputs.version }} and change the run
invocation of .github/scripts/release.sh pypi-exists moq-rs to read the version
as "$VERSION" (ensure proper quoting) so the script receives the version from
the environment rather than direct interpolation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In @.github/workflows/release-py-ffi.yml:
- Around line 21-23: The concurrency group "release-py-ffi" should be scoped by
the triggering event and ref to avoid unrelated runs blocking each other; modify
the concurrency.group value in the workflow's concurrency block (currently for
release-py-ffi) to include github.event_name and github.ref (for example by
using an expression like `${{ github.event_name }}-${{ github.ref
}}-release-py-ffi`) so the workflow is partitioned by event and ref while
leaving cancel-in-progress as desired.

In @.github/workflows/release-py.yml:
- Around line 61-63: The workflow step with id "pypi" (name "Check PyPI") is
interpolating steps.version.outputs.version directly into the run string;
instead export that value via env and reference it in the script to avoid
injecting repo-derived data into the shell. Update the "pypi" step to set an
environment variable (e.g., VERSION) to ${{ steps.version.outputs.version }} and
change the run invocation of .github/scripts/release.sh pypi-exists moq-rs to
read the version as "$VERSION" (ensure proper quoting) so the script receives
the version from the environment rather than direct interpolation.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 10c7b47a-9ef7-4eba-9b4e-9567abb43381

📥 Commits

Reviewing files that changed from the base of the PR and between 1bb8810 and 6cd517e.

📒 Files selected for processing (6)
  • .github/scripts/release.sh
  • .github/workflows/release-py-ffi.yml
  • .github/workflows/release-py.yml
  • CLAUDE.md
  • py/moq-ffi/pyproject.toml
  • py/moq-rs/pyproject.toml
🚧 Files skipped from review as they are similar to previous changes (4)
  • py/moq-ffi/pyproject.toml
  • CLAUDE.md
  • .github/scripts/release.sh
  • py/moq-rs/pyproject.toml

claude added 2 commits May 30, 2026 17:11
Two CodeRabbit quick wins:
- release-py-ffi.yml concurrency group now keys on event + ref (matching
  release-py.yml), so a PR dry-run can't queue ahead of a tag publish.
- release-py.yml's PyPI check passes the version through `env:` instead of
  interpolating ${{ }} straight into the run string (zizmor injection nit).

https://claude.ai/code/session_01EegH6PKeUcknQJ2TP7D7RX
Match the robustness the repo's earlier per-package publisher
(py/common/release.sh) had: a network blip on the PyPI existence check
shouldn't fail the release. Add --max-time 10 --retry 3
--retry-connrefused to the pypi-exists curl.

https://claude.ai/code/session_01EegH6PKeUcknQJ2TP7D7RX
@kixelated kixelated merged commit 820d1a1 into main May 30, 2026
29 checks passed
@kixelated kixelated deleted the claude/moq-ffi-per-language-publish-Fd7dC branch May 30, 2026 18:08
kixelated added a commit that referenced this pull request May 31, 2026
…the wrapper

Mirror the Python split (#1551) for Go. Today `go/` is a single module that is
just the raw uniffi-bindgen-go output plus native libs, released lockstep with
the moq-ffi crate, with no ergonomic layer.

Split it into two co-located module skeletons:

  * go/ffi/     -> github.com/moq-dev/moq-go-ffi: the raw bindings + native
                  libs (the old go/ content), released lockstep with the crate
                  on each moq-ffi-v* tag (release-go-ffi.yml -> moq-dev/moq-go-ffi).
  * go/wrapper/ -> github.com/moq-dev/moq-go: a new ergonomic wrapper (package
                  moq, import path unchanged), versioned independently and
                  published by a new release-go.yml -> moq-dev/moq-go.

The wrapper is a full re-wrap with idiomatic Go: context.Context cancellation
(uniffi async is blocking in Go, so a goroutine + ctx.Done()->Cancel() helper
bridges it), Go error returns with an IsShutdown helper, Go 1.23 iter.Seq2
stream iterators, and records/enums re-exported without the Moq prefix.

Go MVS resolves to the max moq-go-ffi across the build graph, so a wrapper
consumer always gets an ffi >= what the wrapper requires; since ffi is additive
that is always new enough. release-go.yml auto-bumps the wrapper's require to
the newest moq-go-ffi on every moq-ffi-v* tag and re-publishes, so moq-go@latest
floats to the latest native core. MAJOR.MINOR lives in go/wrapper/VERSION; the
patch is derived from the mirror's tags by publish-wrapper.sh, which skips
publishing when the staged tree is unchanged so no-op triggers never mint empty
releases.

Also fixes the x86_64-apple-darwin matrix entry to use macos-15-intel (the old
macos-latest is arm64 and mis-built the Intel .a).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants