Skip to content

Test optional Iroh limiter configuration - #8761

Merged
lawrencecchen merged 2 commits into
mainfrom
feat-iroh-optional-limiter-test
Jul 24, 2026
Merged

lawrencecchen merged 2 commits into
mainfrom
feat-iroh-optional-limiter-test

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Jul 23, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • treat CMUX_IROH_RATE_LIMIT_ID as optional in the production environment fixture
  • verify Vercel production validation succeeds without the optional limiter ID

Testing

  • cd web && bun test tests/client-config-env.test.ts
  • cd web && bun run typecheck
  • cd web && bun test
  • bunx biome check web/tests/client-config-env.test.ts
  • cd web && bunx eslint tests/client-config-env.test.ts

Context


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Treat CMUX_IROH_RATE_LIMIT_ID as optional in the production test fixture and ensure explicit Vercel production validation passes without it to fix the CI regression.

Written for commit 2984045. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes
    • Vercel production deployments now succeed when the optional Iroh rate-limit identifier is omitted.
    • Removed an incorrect validation error requiring the identifier in this deployment scenario.

@coderabbitai

coderabbitai Bot commented Jul 23, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The production environment test fixture no longer supplies CMUX_IROH_RATE_LIMIT_ID. A new test verifies that explicit Vercel production configuration succeeds without the optional limiter id.

Changes

Iroh limiter validation

Layer / File(s) Summary
Vercel production environment coverage
web/tests/client-config-env.test.ts
Removes the limiter id from the production fixture and verifies successful environment import without the CMUX_IROH_RATE_LIMIT_ID is required error.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • manaflow-ai/cmux#8714: Makes CMUX_IROH_RATE_LIMIT_ID optional and adds fail-open handling and tests for missing firewall rules.

Suggested reviewers: azooz2003-bit

🚥 Pre-merge checks | ✅ 25
✅ Passed checks (25 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Cmux Swift Actor Isolation ✅ Passed Only web/tests/client-config-env.test.ts changed; no Swift code or actor-isolation risks were introduced.
Cmux Swift Blocking Runtime ✅ Passed PR only changes a TypeScript test file; no Swift files or blocking-runtime sync code were modified.
Cmux Browser Automation Off-Main ✅ Passed Diff only touches web/tests/client-config-env.test.ts; it doesn't modify browser socket automation codepaths or policy routing covered by the rule.
Cmux Expensive Synchronous Load ✅ Passed Diff only changes a web test file; no Swift files or main-actor load paths were touched.
Cmux Cache Substitution Correctness ✅ Passed Only a test file changed; the patch adds/adjusts env-validation cases and touches no production persistence/history/snapshot cache path.
Cmux No Hacky Sleeps ✅ Passed Diff only updates a test fixture/assertions; no sleep, timer, polling, or wall-clock wait was introduced, and the rule allows deterministic test scaffolding.
Cmux Algorithmic Complexity ✅ Passed Only web/tests/client-config-env.test.ts changed, and the rule explicitly exempts test-only scaffolding.
Cmux Swift Concurrency ✅ Passed The diff only changes web/tests/client-config-env.test.ts; no Swift files or concurrency patterns were introduced.
Cmux Swift @Concurrent ✅ Passed PR changes only web/tests/client-config-env.test.ts; no Swift files or Swift concurrency annotations are touched.
Cmux Swift Package Boundaries ✅ Passed Only a TypeScript test file changed; no Swift, Package.swift, or Xcode project files were touched, so the Swift package boundary rule is not implicated.
Cmux Swiftpm Lockfiles ✅ Passed PR only changes web/tests/client-config-env.test.ts; no SwiftPM/Xcode/.gitignore/workflow/dependency files are touched, so the rule is not implicated.
Cmux Swift Logging ✅ Passed PR only changes a TypeScript test file; no Swift runtime/app code or logging statements were added or modified, so the Swift logging rule doesn't apply.
Cmux User-Facing Error Privacy ✅ Passed Diff only changes tests; the rule explicitly allows tests and no end-user-facing copy was added.
Cmux Full Internationalization ✅ Passed Only a test fixture/assertion changed; no user-facing Swift/web text, catalogs, or locale files were touched.
Cmux Swiftui State Layout ✅ Passed Diff only changes web/tests/client-config-env.test.ts; no SwiftUI files or state/layout code are touched, so the SwiftUI-state-layout rule is not applicable.
Cmux Architecture Rethink ✅ Passed PR only changes a TypeScript test fixture; no Swift code or architectural pattern change is present, so the Swift rethink rule does not apply.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed HEAD only changes web/tests/client-config-env.test.ts; no Swift window code or cmuxAuxiliaryWindowIdentifiers changes are present.
Cmux Source Artifacts ✅ Passed Only web/tests/client-config-env.test.ts changed, and it’s an intentional test/fixture update, not source-control artifact output.
Cmux No Test Or Debug Seam In Production Source ✅ Passed The PR only changes a web TypeScript test; no Swift files under any production Sources/ path were touched, so the seam rule is not applicable.
Cmux No Ambient Global State ✅ Passed Diff only touches a TypeScript test; no new production Swift globals, singletons, or static-only namespaces were added.
Title check ✅ Passed The title clearly matches the main change: making the Iroh limiter config optional in tests.
Description check ✅ Passed The summary and testing sections are complete and relevant, though the template's Review Trigger and Checklist sections are missing.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat-iroh-optional-limiter-test

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.

@greptile-apps

greptile-apps Bot commented Jul 23, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a CI regression by updating the test fixture and associated test case for CMUX_IROH_RATE_LIMIT_ID, which was made optional in an earlier commit. The changes are confined to the test file and correctly align expectations with the updated production validation logic.

  • Removes CMUX_IROH_RATE_LIMIT_ID from the requiredIrohProductionEnv fixture, reflecting that the field is now optional.
  • Renames and rewrites the former "requires the Iroh limiter id" test to "allows explicit Vercel production without the optional Iroh limiter id," adding the required production spreads and flipping the exit-code/stderr assertions to match the new optional behavior.
  • Removes the manual CMUX_IROH_RATE_LIMIT_ID reference from the "self-hosted relay without legacy minter" test, which was also sourcing the value from the now-updated fixture.

Confidence Score: 5/5

Safe to merge — test-only change with no production code affected.

The change removes one field from a test fixture and updates a single test assertion to match a behavioural change already landed in production. All other tests remain intact, and a separate test (line 143) still exercises CMUX_IROH_RATE_LIMIT_ID when explicitly supplied, so coverage for the optional path is preserved.

No files require special attention.

Important Files Changed

Filename Overview
web/tests/client-config-env.test.ts Test-only change: removes CMUX_IROH_RATE_LIMIT_ID from the required Iroh production fixture and updates one test to verify Vercel production validation succeeds without this now-optional field. Logic is consistent and coverage is maintained.

Reviews (2): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile

@lawrencecchen
lawrencecchen merged commit d3dda7b into main Jul 24, 2026
6 checks passed
@lawrencecchen
lawrencecchen deleted the feat-iroh-optional-limiter-test branch July 24, 2026 05:12
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.

1 participant