Fix iOS CI false-green: align manual-pairing tests with encrypted-route restriction, catch Swift Testing failures in success override - #5906
Merged
Conversation
…tion d7ee593 restricted routeAllowsStackAuth to encrypted/loopback routes (Tailscale, iroh, loopback) as a security fix but only updated the policy unit tests. Four cmuxFeatureTests auth-contract tests still asserted the old contract (Stack token sent over plain-TCP LAN/.local manual routes) and have failed on every ios-simulator run since, masked by the workflow's success-override grep not recognizing Swift Testing failure output. Rewrite the three LAN/.local tests as rejection-contract tests: pairing fails before any RPC (and the Stack bearer token) leaves the device, and the actionable route-not-allowed error is surfaced. Repoint the probe-then-fallback test at a Tailscale host so the method_not_found to synthetic-ticket fallback path keeps its coverage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
selected_tests_passed_despite_xcodebuild_status only knew XCTest failure formats. Swift Testing prints failures as '✘ Test ... failed' and the final 'Failing tests:' section is not always flushed into the tee'd log, so a run with genuinely failing Swift Testing tests (exit 65) could be misclassified as a runner cleanup failure and turn the job green. That false green let the stale manual-pairing tests merge red on #5876 and earlier. Add the Swift Testing failure markers to the negative grep. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR updates iOS pairing tests to enforce encrypted-routes-only behavior, rejecting untrusted plain-TCP targets with a "pairing route is not allowed" error. It also improves CI test failure detection by teaching the workflow to recognize Swift Testing failure markers (✘ Test/✘ Suite) alongside existing XCTest patterns. ChangesCI Workflow Test Failure Detection
Manual Host Pairing Security Enforcement Tests
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 20 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (20 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 fixes two CI correctness issues: (1) four
Confidence Score: 4/5Safe to merge — both files are test/CI-only with no production Swift changes. The Swift test rewrites correctly track the new encrypted-routes-only policy and the zero-requests assertion is a solid contract check. The CI grep fix closes the confirmed false-green loophole. The remaining gap is whether
Important Files Changed
Sequence DiagramsequenceDiagram
participant App as iOS App
participant Store as CMUXMobileShellStore
participant Policy as routeAllowsStackAuth
participant Transport as ScriptedTransport
Note over App,Transport: LAN/Bonjour route (rejected by policy)
App->>Store: connectManualHost("192.168.1.77" / "devbox.local")
Store->>Policy: routeAllowsStackAuth(route)
Policy-->>Store: false (unencrypted TCP)
Store-->>App: "connectionError = "This pairing route is not allowed...""
Note right of Store: phase=.pairing, state=.disconnected
Note over App,Transport: Tailscale route (accepted by policy)
App->>Store: connectManualHost("100.71.210.41")
Store->>Policy: routeAllowsStackAuth(route)
Policy-->>Store: true (Tailscale encrypted)
Store->>Transport: mobile.attach_ticket.create (+ Stack token)
Transport-->>Store: method_not_found
Store->>Transport: workspace.list (synthetic ticket fallback)
Transport-->>Store: workspace list
Store-->>App: "phase=.workspaces, state=.connected"
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| grep -Eq "Test Suite 'Selected tests' passed|Test Suite 'cmuxUITests' passed" "$log_path" && | ||
| grep -Eq "Executed [1-9][0-9]* tests, with 0 failures \\(0 unexpected\\)" "$log_path" && | ||
| ! grep -Eq "Test Suite '.*' failed|Test Case '.*' failed|Assertion Failure|Failing tests:|with [1-9][0-9]* failures|with [0-9]+ failures \\([1-9][0-9]* unexpected\\)" "$log_path" | ||
| ! grep -Eq "Test Suite '.*' failed|Test Case '.*' failed|Assertion Failure|Failing tests:|with [1-9][0-9]* failures|with [0-9]+ failures \\([1-9][0-9]* unexpected\\)|✘ Test|✘ Suite" "$log_path" |
Contributor
There was a problem hiding this comment.
✘ Test and ✘ Suite cover assertion failures and normal test failures, but Swift Testing can also terminate a test runner process via an uncaught thrown error or a fatal error, which in xcodebuild's log may surface as Fatal error:, error: ..., or just a non-zero exit without any ✘ marker. The existing Assertion Failure pattern catches one XCTest form; there is no equivalent for Swift Testing's Issue.record path or for a fatal thrown error outside a #expect. If any test in the suite crashes rather than fails gracefully, the negative grep could still pass and false-green the job. Worth adding a broader safety net (e.g. Issue recorded) once the exact log tokens are confirmed against a real crash log.
lawrencecchen
added a commit
that referenced
this pull request
Jun 12, 2026
Re-applies origin/feat-ios-dog-unified (771f532, dog 11b) onto current origin/main (f2627b9). Main's reviewed forms win for landed features (composer #5876, QR #5872, presence gate #5912, CI fix #5906): seven pairing/RPC/transport files take main's private-extension structure, persistPairedMacFromTicket keeps main's serialized-write-chain lookup, QR pairing tests keep main's durable-write polls. Dog-carried unlanded work is preserved: MobileHostService+Capabilities.swift superset (notification.dismiss.v1, terminal.paste.v1, workspace.groups.v1, DEBUG dogfood verbs) replaces main's inline subset var, dismiss-sync observer and capability flag resets kept, dogfood pane model kept.
hhsw2015
pushed a commit
to hhsw2015/cmux
that referenced
this pull request
Jun 12, 2026
…te restriction, catch Swift Testing failures in success override (manaflow-ai#5906) * Align manual-pairing auth-contract tests with encrypted-route restriction d7ee593 restricted routeAllowsStackAuth to encrypted/loopback routes (Tailscale, iroh, loopback) as a security fix but only updated the policy unit tests. Four cmuxFeatureTests auth-contract tests still asserted the old contract (Stack token sent over plain-TCP LAN/.local manual routes) and have failed on every ios-simulator run since, masked by the workflow's success-override grep not recognizing Swift Testing failure output. Rewrite the three LAN/.local tests as rejection-contract tests: pairing fails before any RPC (and the Stack bearer token) leaves the device, and the actionable route-not-allowed error is surfaced. Repoint the probe-then-fallback test at a Tailscale host so the method_not_found to synthetic-ticket fallback path keeps its coverage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Catch Swift Testing failures in the ios-simulator success override selected_tests_passed_despite_xcodebuild_status only knew XCTest failure formats. Swift Testing prints failures as '✘ Test ... failed' and the final 'Failing tests:' section is not always flushed into the tee'd log, so a run with genuinely failing Swift Testing tests (exit 65) could be misclassified as a runner cleanup failure and turn the job green. That false green let the stale manual-pairing tests merge red on manaflow-ai#5876 and earlier. Add the Swift Testing failure markers to the negative grep. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
austinywang
added a commit
that referenced
this pull request
Jun 17, 2026
…eforeReplay (#5911) The terminal-output collector tests waited for stream chunks with a fixed 200ms polling budget (200×1ms). On the slower iPad simulator leg the live "current" replay chunk lands after that window, so the assertion saw only the "old" snapshot and the test failed — a flake previously masked by the test-ios false-green override (#5906). Replace the time-budget poll with a deterministic continuation-based synchronization point on TerminalOutputCollector: waitForLines(_:) parks a CheckedContinuation and resumes the instant the stream delivers the target chunk count, with no per-platform timing budget to overrun. unmount() releases any parked waiter so a pending wait never hangs. Applied to all three sibling tests that shared the fragile poll pattern. Closes #5911 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This branch was successfully deployed
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.
Two test/CI-only fixes extracted from #5726 so every open iOS branch can rebase onto them instead of re-fixing divergently.
The bug: d7ee59384 (June 4, restrict Stack token to encrypted routes — correct security fix) updated the policy unit tests but left four cmuxFeatureTests auth-contract tests asserting the OLD contract (Stack token sent over plain-TCP LAN/.local manual routes). They have failed deterministically since, throwing insecureManualRoute before any request is sent. Nobody noticed because the ios-simulator workflow's success-override grep only recognizes XCTest failure formats, not Swift Testing's ✘ output — so the jobs false-greened (verified: the green iphone job on the merged #5876 run shows the identical 4 failures, 12 issues, exit 65, TEST FAILED in its log).
Fixes:
Residual: the loophole existed since June 4; other iOS test failures may have slipped onto main in that window. Audit queued separately.
🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Low Risk
Test and CI workflow changes only; no production pairing or auth logic is modified in this PR.
Overview
Fixes iOS CI false greens and brings manual-host pairing feature tests in line with the existing encrypted-route Stack-auth policy (no app behavior changes in this diff).
The
test-ios.ymlsuccess-override helper now treats Swift Testing failures (✘ Test/✘ Suite) like XCTest failures, so exit 65 with real test failures cannot be misread as simulator cleanup and pass the job.In
cmuxFeatureTests, three LAN /.localmanual-pairing cases are rewritten to assert rejection before any RPC (pairing phase, actionable error, zero recorded requests). The attach-ticket probe fallback test now uses a Tailscale IP host instead of a private LAN address somethod_not_found→ Stack-auth fallback coverage remains on a trusted route.Reviewed by Cursor Bugbot for commit c5c7860. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fix iOS CI false greens and align manual pairing tests with the encrypted-route-only policy. Failing simulator runs now fail, and tests no longer expect Stack auth over plain LAN/.local routes.
test-ios.ymlso failures aren’t masked by the success override.Written for commit c5c7860. Summary will update on new commits.
Summary by CodeRabbit
Tests
Chores