Repository navigation
ci: preserve Swift Testing method selectors in app-host shards - #13831
Merged
Merged
Conversation
|
Warning Review limit reachedNext included review available in 3 minutes. View limit detailsLimit details: You’ve used all 10 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository: manaflow-ai/cmux/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (2)
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
|
All contributors have signed the CLA ✍️ ✅ |
teamleaderleo
marked this pull request as ready for review
September 23, 2026 01:30
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
This was referenced Sep 23, 2026
teamleaderleo
added a commit
that referenced
this pull request
Sep 23, 2026
* fix(ci): unquote replayed argv so post-#13831 selectors resolve The replay tool reads each batch's -only-testing selectors from the argv that run-app-host-xcodebuild.sh records with printf '%q', then unquoted them with .strip("'"). %q escapes shell metacharacters, and since #13831 gave Swift Testing selectors their trailing parens there is now something to escape: the recorded line reads arg=-only-testing:cmuxTests/Suite/testFoo\(\) 145 of 235 selector lines in one real batch carry those escapes. Read literally they match nothing, so check_run returns at its first gate with "selector matched zero built tests" and never reaches the analysis the tool exists to produce. Replaying run 35828371879 shard 2 against main emits 278 such lines and no verdict. This was invisible when the tool was written because it was validated against a pre-#13831 run, where selectors had no parens and %q had nothing to escape. It is broken on every run from here on. Unquote with shlex, and fail loudly on an unbalanced quote rather than skipping the line, since a silently shorter selector set is the exact failure mode this tool exists to expose. Also derive each batch's xcode status from its own log. The .meta does not record it and the default was 65 for every batch, but check_run treats the status as evidence: a batch that really exited 0 replayed as 65 reports "xcodebuild exited 65 without a typed failed Test Case", a contradiction that never occurred. xcodebuild's closing banner carries it. --xcode-status stays as an override. Before: 278 "selector matched zero built tests", no verdict. After: RATCHET_NEW_FAILURE SidebarHiddenPresentationTests/ visibilityToggleKeepsAppKitTableContainerMounted() missing typed test result: TabManagerSessionSnapshotTests/ testGhosttyFocusSurfaceIdRecordsMappedPanelInFocusHistory() RATCHET_NEW_FAILURE WorkspaceContentViewVisibilityTests/ testMinimalModeToggleDoesNotReevaluateChromeHeavyBodies() Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(ci): refuse argv forms the replay parser cannot decode Two hardening findings from review, both verified rather than reasoned. printf %q falls back to ANSI-C $'...' when a value holds a non-printable character, and shlex neither decodes that form nor raises on it. Checked against a real bash subprocess rather than an assumed encoding: printf %q -> "$'-only-testing:cmuxTests/S/testA\tB()'" shlex -> ["$-only-testing:cmuxTests/S/testA\\tB()"] The leading $ and the literal backslash make it fail the -only-testing prefix test, so the selector is dropped with no error. That is the silent selector-set shrink the unbalanced-quote guard was added to prevent, arriving through a path that guard does not cover. Unreachable for today's identifiers, which is why it is refused rather than decoded. %q output is also one token by construction, so more than one token means the recorded argv is not what this parser assumes. Refuse that too instead of silently taking the first. Re-verified end to end against run 35828371879 shard 2: same two batches, same recovered verdicts as before the hardening. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: pin the replay parser against real printf %q output The decoding here is easy to get wrong by reasoning about it -- both of us did, in opposite directions, before anyone ran bash. So two of these build their fixture by shelling out to `printf %q` rather than asserting an assumed encoding, which is what would have caught the escaped parens at review time instead of after the tool shipped non-functional. The other four pin the refusals: ANSI-C `$'...'`, an unbalanced quote, and multi-token argv. None is reachable for today's identifiers, but each would otherwise drop a selector without a word, and a selector that silently vanishes reads exactly like a test the batch never ran -- the finding this tool exists to report. Five of the six fail against the parser as merged in #13972; the bare-identifier case passes both and is the control. Registered in tests/test-execution.toml and run from the app-host-process guard group, so the execution registry stays complete. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
rustybret
pushed a commit
to rustybret/bmux
that referenced
this pull request
Sep 23, 2026
9963f3f ci: stop admitting macOS after a run has already failed (manaflow-ai#13969) c7fb3b1 ci: put the switch for paid macOS capacity in the repository (manaflow-ai#13973) 0044d5a ci: mirror app-host batch output without racing a tail (manaflow-ai#13990) a0b1dea fix(ci): unquote replayed argv so post-manaflow-ai#13831 selectors resolve (manaflow-ai#13974) c14b142 test: target local Undo at the editable responder (manaflow-ai#13989) 5985cf5 Mirror terminal focus before the runtime surface exists (manaflow-ai#13968) afd5c47 ci: let E2E dispatches adopt a compiled product instead of rebuilding (manaflow-ai#13958) # Conflicts: # .github/workflows/ci-guards.yml # .github/workflows/ci-macos-compat.yml # .github/workflows/ci-macos.yml # .github/workflows/ci.yml # .github/workflows/cli-pipe-regressions.yml # .github/workflows/cloud-command-deadlines.yml # .github/workflows/cloud-machine-tests.yml # .github/workflows/cmux-tui-build-package.yml # .github/workflows/iroh-release-gate.yml # .github/workflows/nightly.yml # .github/workflows/perf-activation.yml # .github/workflows/plain-paste-worker.yml # .github/workflows/relay-tls.yml # .github/workflows/release.yml # .github/workflows/terminal-hang-diagnostics.yml # .github/workflows/test-e2e.yml # .github/workflows/tmux-corpus.yml
This was referenced Sep 23, 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.
Large suites migrated to Swift Testing were split into bare XCTest-style method selectors. Xcode exits successfully while executing zero Swift Testing methods for those selectors; the result-completeness guard then rejects the missing results. This caused missing-method failures across the app-host shards in run 35800631230.
Emit explicit
()signatures for no-argument methods, preserving the existing timing lookup and shard weights. Keep a suite whole if parameterized, multiline, inline, or non-test-prefixed Swift Testing declarations cannot be represented by the method parser. The required result-completeness checks remain unchanged.Validation:
ModernTests/testSentinelruns zero tests and exits 0, whileModernTests/testSentinel()executes the deliberately failing assertion and exits 65. XCTest runs its sentinel with either spelling.No app code, runner settings, or artifact-policy changes. Final-head hosted CI is pending; no full cmux app-host rerun is claimed.