Repository navigation
ci(iroh): make direct-only release gate deterministic - #8489
Merged
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds a direct-only Iroh release gate that runs focused iOS simulator transport tests, verifies bidirectional non-relay communication, reports results, and adjusts workflow setup and plan-selection tests. ChangesIroh direct transport release gate
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ReleaseGate
participant DirectGate
participant Xcodebuild
participant TransportTests
ReleaseGate->>DirectGate: Dispatch direct-only gate
DirectGate->>Xcodebuild: Run focused simulator tests
Xcodebuild->>TransportTests: Execute direct transport suite
TransportTests->>TransportTests: Exchange request and response payloads
TransportTests-->>DirectGate: Return direct-path test result
DirectGate-->>ReleaseGate: Emit JSON report
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 25✅ Passed checks (25 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Contributor
Greptile SummaryThis PR makes the
Confidence Score: 5/5Safe to merge — the change is entirely additive CI and test infrastructure with no modifications to production Swift sources. All changed files are CI workflow YAML, shell scripts, and a test-only Swift file under No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[run-iroh-release-gate.sh] --> B{--mode}
B -->|automatic| C[GATE_PLAN = app-rpc]
B -->|relay-only| C
B -->|direct-only| D[GATE_PLAN = simulator-direct-transport]
C -->|--print-plan| E[print app-rpc and exit]
D -->|--print-plan| F[print simulator-direct-transport and exit]
C --> G[Full app-RPC gate]
D --> H[exec run-iroh-direct-transport-gate.sh]
H --> I[Create fresh iOS Simulator]
I --> J[xcodebuild CmxIrohDirectTransportGateTests]
J --> K{xcresulttool validation}
K -->|fail| L[exit non-zero]
K -->|pass| M[Emit schema 3 verdict]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[run-iroh-release-gate.sh] --> B{--mode}
B -->|automatic| C[GATE_PLAN = app-rpc]
B -->|relay-only| C
B -->|direct-only| D[GATE_PLAN = simulator-direct-transport]
C -->|--print-plan| E[print app-rpc and exit]
D -->|--print-plan| F[print simulator-direct-transport and exit]
C --> G[Full app-RPC gate]
D --> H[exec run-iroh-direct-transport-gate.sh]
H --> I[Create fresh iOS Simulator]
I --> J[xcodebuild CmxIrohDirectTransportGateTests]
J --> K{xcresulttool validation}
K -->|fail| L[exit non-zero]
K -->|pass| M[Emit schema 3 verdict]
Reviews (4): Last reviewed commit: "test(iroh): reject empty direct gate run..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+70
to
+71
| if runtime.get("isAvailable", True) | ||
| and runtime.get("identifier", "").startswith("com.apple.CoreSimulator.SimRuntime.iOS") |
Contributor
There was a problem hiding this comment.
isAvailable key defaults to True when absent, so a runtime entry that omits the field is treated as available. simctl list runtimes -j includes this key for all known entries, but a partially-downloaded or damaged runtime that omits the key would be picked as a candidate and then fail during xcodebuild. Defaulting to False (exclude if unknown) is safer.
Suggested change
| if runtime.get("isAvailable", True) | |
| and runtime.get("identifier", "").startswith("com.apple.CoreSimulator.SimRuntime.iOS") | |
| if runtime.get("isAvailable", False) | |
| and runtime.get("identifier", "").startswith("com.apple.CoreSimulator.SimRuntime.iOS") |
Comment on lines
+73
to
+81
| preferred_names = ("iPhone 17", "iPhone 16") | ||
| device_types = [ | ||
| device for device in listing("devicetypes").get("devicetypes", []) | ||
| if device.get("name") in preferred_names | ||
| ] | ||
| if not runtimes or not device_types: | ||
| raise SystemExit("no available iOS runtime or supported iPhone device type") | ||
| runtime = max(runtimes, key=version_key) | ||
| device = min(device_types, key=lambda value: preferred_names.index(value["name"])) |
Contributor
There was a problem hiding this comment.
preferred_names = ("iPhone 17", "iPhone 16") means any CI runner image that ships only iPhone 15 (or earlier) simulators produces the opaque error "no available iOS runtime or supported iPhone device type" even when a perfectly valid runtime is present. A simple fallback — e.g. filtering all device types whose name starts with "iPhone" when neither preferred name is present — would make the script self-healing against Xcode version drift without changing the preference ordering for modern runners.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@scripts/run-iroh-direct-transport-gate.sh`:
- Around line 94-125: Update the test execution flow in
scripts/run-iroh-direct-transport-gate.sh to configure and retain an xcodebuild
result bundle, then inspect it after xcodebuild completes and fail if it reports
zero executed tests. Perform this validation before constructing or writing
REPORT_JSON, so passed:true is emitted only after at least one matching test has
run.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f86a3865-6e18-421a-8867-ce72ce985d7d
📒 Files selected for processing (5)
.github/workflows/iroh-release-gate.ymlPackages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxIrohDirectTransportGateTests.swiftscripts/lib/mobile-attach.test.mjsscripts/run-iroh-direct-transport-gate.shscripts/run-iroh-release-gate.sh
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
scripts/run-iroh-direct-transport-gate.sh (1)
114-124: 🎯 Functional Correctness | 🔴 Critical | 🏗️ Heavy liftFail closed when no tests execute.
xcodebuildexits0even when zero tests match the-only-testingfilter, meaning this script may silently emit apassed:truereport without actually verifying the direct transport.To guarantee that a test was executed, configure a result bundle and verify that the
testsCountmetric is greater than zero before writingREPORT_JSON.🛠️ Proposed fix to validate test execution
( cd "$REPO_ROOT/Packages/Shared/CmuxIrohTransport" python3 "$REPO_ROOT/scripts/ci/run_with_timeout.py" \ --timeout-seconds 600 \ -- xcodebuild \ -scheme CmuxIrohTransport \ -destination "platform=iOS Simulator,id=$SIMULATOR_ID" \ -derivedDataPath "$DERIVED_DATA" \ + -resultBundlePath "$DERIVED_DATA/ResultBundle.xcresult" \ "$XCODEBUILD_ACTION" \ -only-testing:CmuxIrohTransportTests/CmxIrohDirectTransportGateTests ) + +TESTS_RUN=$(python3 -c ' +import json, subprocess, sys +try: + out = subprocess.check_output(["xcrun", "xcresulttool", "get", "--path", sys.argv[1], "--format", "json"]) + data = json.loads(out) + tests = sum(int(a.get("actionResult", {}).get("metrics", {}).get("testsCount", {}).get("_value", "0")) for a in data.get("actions", {}).get("_values", [])) + print(tests) +except Exception: + print("0") +' "$DERIVED_DATA/ResultBundle.xcresult") + +if [[ "$TESTS_RUN" -eq 0 ]]; then + echo "error: xcodebuild executed 0 tests." >&2 + exit 1 +fi🤖 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 `@scripts/run-iroh-direct-transport-gate.sh` around lines 114 - 124, Update the xcodebuild invocation in the direct transport gate to emit a result bundle, then inspect its test-summary metrics and require testsCount to be greater than zero before writing REPORT_JSON or reporting success. Fail closed with the existing failure behavior when no tests execute, while preserving the current test filter and build action.
🤖 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 `@scripts/run-iroh-release-gate.sh`:
- Around line 117-142: Extract the duplicated Simulator selection logic into a
shared shell function in scripts/lib/mobile-attach.sh, preserving runtime
discovery, version selection, preferred iPhone ordering, fallback device-type
listing, and validation behavior. Replace the inline blocks in
scripts/run-iroh-release-gate.sh lines 117-142 and
scripts/run-iroh-direct-transport-gate.sh lines 70-96 with calls to the shared
helper, preserving each script’s existing use of the selected runtime and device
type.
---
Duplicate comments:
In `@scripts/run-iroh-direct-transport-gate.sh`:
- Around line 114-124: Update the xcodebuild invocation in the direct transport
gate to emit a result bundle, then inspect its test-summary metrics and require
testsCount to be greater than zero before writing REPORT_JSON or reporting
success. Fail closed with the existing failure behavior when no tests execute,
while preserving the current test filter and build action.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 07a8421c-bb93-44ae-948a-79c6857c779a
📒 Files selected for processing (2)
scripts/run-iroh-direct-transport-gate.shscripts/run-iroh-release-gate.sh
Comment on lines
+117
to
+142
| if runtime.get("isAvailable", False) | ||
| and runtime.get("identifier", "").startswith("com.apple.CoreSimulator.SimRuntime.iOS") | ||
| ] | ||
| device_types = [ | ||
| device for device in listing("devicetypes").get("devicetypes", []) | ||
| if device.get("name") in ("iPhone 17", "iPhone 16") | ||
| ] | ||
| if not runtimes or not device_types: | ||
| raise SystemExit("no available iOS runtime or supported iPhone device type") | ||
| if not runtimes: | ||
| raise SystemExit("no available iOS runtime") | ||
| runtime = max(runtimes, key=version_key) | ||
| preferred_names = ( | ||
| "iPhone 17", "iPhone 17 Pro", "iPhone 16", "iPhone 16 Pro", | ||
| "iPhone 15", "iPhone 15 Pro", "iPhone 14", "iPhone 14 Pro", | ||
| ) | ||
| preference = {name: index for index, name in enumerate(preferred_names)} | ||
| supported_device_types = runtime.get("supportedDeviceTypes") | ||
| if not isinstance(supported_device_types, list): | ||
| supported_device_types = listing("devicetypes").get("devicetypes", []) | ||
| device_types = sorted( | ||
| ( | ||
| device for device in supported_device_types | ||
| if str(device.get("name", "")).startswith("iPhone") | ||
| ), | ||
| key=lambda device: ( | ||
| preference.get(str(device.get("name", "")), len(preference)), | ||
| str(device.get("name", "")), | ||
| ), | ||
| ) | ||
| if not device_types: | ||
| raise SystemExit("available iOS runtime has no supported iPhone device type") |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Extract identical Simulator selection logic into a shared helper.
Both scripts contain an identical inline Python block used to discover an available iOS Simulator runtime and preferred iPhone device type. The shared root cause is duplicated environment-selection logic.
scripts/run-iroh-release-gate.sh#L117-L142: Extract this inline Python logic into a shared shell function (e.g., inscripts/lib/mobile-attach.sh, which is already sourced by both scripts).scripts/run-iroh-direct-transport-gate.sh#L70-L96: Replace this inline Python block by invoking the new shared helper function.
📍 Affects 2 files
scripts/run-iroh-release-gate.sh#L117-L142(this comment)scripts/run-iroh-direct-transport-gate.sh#L70-L96
🤖 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 `@scripts/run-iroh-release-gate.sh` around lines 117 - 142, Extract the
duplicated Simulator selection logic into a shared shell function in
scripts/lib/mobile-attach.sh, preserving runtime discovery, version selection,
preferred iPhone ordering, fallback device-type listing, and validation
behavior. Replace the inline blocks in scripts/run-iroh-release-gate.sh lines
117-142 and scripts/run-iroh-direct-transport-gate.sh lines 70-96 with calls to
the shared helper, preserving each script’s existing use of the selected runtime
and device type.
azooz2003-bit
force-pushed
the
feat-iroh-direct-gate
branch
from
July 19, 2026 16:48
717fcd3 to
ee96ad2
Compare
This was referenced Jul 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root cause
The direct-only matrix leg reused the physical-device identity-only app attach policy while disabling relays. That left a headless runner dependent on nondeterministic LAN discovery to obtain a dialable address, so the Mac never published a route even though direct Iroh transport itself was healthy.
Fix
The first commit adds the failing dispatch regression test. The second adds the implementation.
Verification
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Make the direct-only Iroh release gate deterministic by running a relay-disabled two-endpoint transport proof inside an isolated iOS Simulator. The gate now also fails if the test run is empty or has any failures.
CmxIrohDirectTransportGateTeststo bind two real Iroh endpoints, disable relays, verify EndpointIDs, round-trip a bidirectional stream, require a direct/private path, bound timeouts, and never construct cmux TCP transport.scripts/run-iroh-direct-transport-gate.shto boot a fresh Simulator, run the suite viaxcodebuild, emit a redacted schema 3 report, and reject empty or failing runs.scripts/run-iroh-release-gate.shto routedirect-onlyto the simulator transport proof, add--print-plan, and pass through--skip-build/--keep-simulator/--report-output; a Node test verifies mode-to-plan mapping.direct-only.Written for commit ee96ad2. Summary will update on new commits.
Summary by CodeRabbit
New Features
--print-plan, including adirect-onlyoption that runs the direct-transport path.Bug Fixes
direct-only.Tests
--print-planoutput across multiple modes.