Skip to content

ci(ios): bound the xcodebuild test invocation so a teardown wedge fails fast - #13927

Merged
teamleaderleo merged 1 commit into
manaflow-ai:mainfrom
teamleaderleo:fix/ios-xctest-teardown-bound
Sep 23, 2026
Merged

teamleaderleo merged 1 commit into
manaflow-ai:mainfrom
teamleaderleo:fix/ios-xctest-teardown-bound

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 23, 2026 •

Copy link
Copy Markdown
Collaborator

In run 35823476668 both ios-simulator legs finished their tests in about two minutes, then emitted nothing for 32m45s and were killed by timeout-minutes: 35.

xctest wedges after the run rather than during it: its last line is a suite result, it never emits the Test run with N tests summary, and at 05:50:16 it logs

Failed to terminate process: Error Domain=com.apple.extensionKit.errorDomain Code=18
  ... RBSRequestErrorDomain Code=3 "No such process found"

— waiting on a termination for a process that is already gone. xcodebuild is still alive at kill time; the runner's orphan sweep has to terminate it (pid 1740 (xcodebuild), pid 1791 (DTServiceHub)).

Resulting behavior

The invocation had no bound of its own, so the 35-minute job ceiling was the only one. That is 71 macOS runner-minutes per occurrence for no added signal, and it is invisible: a timeout-minutes expiry reports as conclusion: cancelled, not failure, so the job never appears in a census of failing jobs and reads like an ordinary supersession.

This bounds the invocation with scripts/blacksmith-bounded-command.sh — already what testbox-broker-guard.yml uses, and written for macOS hosts that have neither timeout nor gtimeout. A wedge now becomes a non-zero exit inside the retry loop that is already there, where selected_tests_passed_despite_xcodebuild_status knows both cases:

  • selected tests passed, bad exit → tolerated as a runner cleanup failure, job green
  • tests failed → the real failures surface

So the run above would have reported its five TerminalViewportSpacingTests assertion failures in about three minutes, as a failure, instead of being cancelled at thirty-five.

900s is generous against the ~2 minutes the non-UI suite actually takes and low enough against the 35-minute ceiling to leave the later steps room. A wedge does not match the loop's retry greps, so it consumes one attempt rather than two.

Why now

The lane was gated shut until an hour ago: #13886 made ios-simulator require package-conventions-lint, which was red on main (#10409), so runs stopped before the simulator legs. #13904 fixed the lint. With that gate open, every run that schedules the simulator legs is a candidate to burn 71 runner-minutes this way.

Validation

actionlint clean on the changed file — its one SC2129 finding reproduces on unmodified main. shellcheck clean on the bounded-command helper, which this change puts on a hot path. bash -n on the step body. All 128 test commands in ci-guards.yml pass except tests/test_ghostty_zig_version_sync.sh, which needs the ghostty submodule this worktree does not check out and fails identically on an unmodified tree.

Not executed on a macOS runner. The behavior this changes only appears when xcodebuild wedges, which I cannot reproduce on demand; the success path is unchanged apart from the wrapper. CI here is the first execution.

Closes #13918

— SlateHarrier g1 🗝️
run_cmux_test_failures_20260923_d · section D (test failures) of the 09-23 handoff. Specimen surfaced by the session working #13904.

🤖 Generated with Claude Code


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

Bounds the iOS xcodebuild test invocation in CI so a post-run teardown wedge fails fast instead of hanging until the job's 35-minute ceiling (closes #13918).

  • A wedge now exits non-zero inside the existing retry loop, where a bad exit after passing tests is tolerated as a cleanup failure and real test failures surface as a failure rather than a cancelled timeout.
  • Sets the bound at 900 seconds, generous against the ~2-minute suite and low enough to leave room for the later steps.

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

Review in cubic

Summary by CodeRabbit

  • Bug Fixes
    • Improved reliability of iOS test runs by ensuring stalled tests are recognized as failures instead of blocking the run indefinitely. This helps keep validation results timely while preserving the existing handling of cleanup failures when selected tests pass.

…ls fast

In run 35823476668 both `ios-simulator` legs finished their tests in about
two minutes, then produced no output for 32m45s and were killed by
`timeout-minutes: 35`.

xctest wedges after the run: its last line is a suite result, it never
emits the `Test run with N tests` summary, and at 05:50:16 it logs
`Failed to terminate process: ... extensionKit Code=18` with underlying
`RBSRequestErrorDomain Code=3 "No such process found"` -- waiting on a
termination for a process that is already gone. `xcodebuild` is still
alive at kill time; the runner's orphan sweep has to terminate it.

The invocation had no bound of its own, so the job ceiling was the only
one. That costs 71 macOS runner-minutes per occurrence for no added
signal, and it is invisible: a `timeout-minutes` expiry reports as
`cancelled`, not `failure`, so the job does not appear in any census of
failing jobs and reads like an ordinary supersession.

Bound the invocation with `scripts/blacksmith-bounded-command.sh`, which
already backs `testbox-broker-guard.yml` and works on macOS hosts with no
GNU coreutils. A wedge now becomes a non-zero exit inside the existing
retry loop, where `selected_tests_passed_despite_xcodebuild_status`
already knows the case: tests passed plus a bad exit is tolerated as a
runner cleanup failure, and tests failed surfaces the real failures. So
the specimen above would have reported its five assertion failures in
about three minutes instead of being cancelled at thirty-five.

900s is generous against the ~2 minutes the non-UI suite takes, and low
enough against the 35-minute ceiling to leave the later steps room. A
wedge does not match the loop's retry greps, so it consumes one attempt,
not two.

Verified: `actionlint` clean on the changed file (its one SC2129 finding
reproduces on unmodified `main`); `shellcheck` clean on the bounded-command
helper; `bash -n` on the step body; all 128 `ci-guards.yml` test commands
pass except `test_ghostty_zig_version_sync.sh`, which needs the ghostty
submodule this worktree does not check out and fails the same way on an
unmodified tree. Not executed on a macOS runner.

Refs manaflow-ai#13918

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@coderabbitai

coderabbitai Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: e4f81c10-0a5d-4b72-af5e-1850994b4c00

📥 Commits

Reviewing files that changed from the base of the PR and between ce1c55c and e7142ac.

📒 Files selected for processing (1)
  • .github/workflows/test-ios.yml

Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The iOS simulator test step sets a 900-second limit for each xcodebuild invocation. It wraps the command with scripts/blacksmith-bounded-command.sh. A timeout exits non-zero, allowing the existing retry loop to process the result.

Changes

iOS simulator test timeout

Layer / File(s) Summary
Bound the xcodebuild invocation
.github/workflows/test-ios.yml
The test step sets XCODEBUILD_TIMEOUT_SECONDS to 900 and applies it to each xcodebuild invocation through the bounded-command script.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to e7142

The invocation is bounded, and the reported teardown wedge should no longer consume the full job limit. No actionable merge-blocking risk remains after normal checks.

🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The change satisfies issue [#13918] ask 1. It sets XCODEBUILD_TIMEOUT_SECONDS to 900 and wraps each simulator xcodebuild invocation with scripts/blacksmith-bounded-command.sh, so a teardown we… Implement and validate a workflow-level signal that identifies timeout expiry separately from superseded cancellation, or split that requirement into a separately tracked change and link its implementation to [#13918].
✅ Passed checks (24 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely identifies the primary change: bounding the iOS xcodebuild invocation to fail fast on teardown wedges.
Description check ✅ Passed The description clearly explains the problem, implementation, resulting behavior, validation, and macOS testing limitation. The Demo Video is not applicable to this CI-only change. The Review Trigger …
Out of Scope Changes check ✅ Passed The diff changes only .github/workflows/test-ios.yml. The timeout variable, command wrapper, and explanatory comments directly support the teardown bound requested by [#13918]. No unrelated source, …
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Cmux Cloud Persistent Session And Early Input ✅ Passed PASS: The pull request changes only .github/workflows/test-ios.yml. It adds a timeout environment variable and wraps an xcodebuild invocation with scripts/blacksmith-bounded-command.sh. It does …
Cmux Swift Actor Isolation ✅ Passed PASS: The authoritative PR diff changes only .github/workflows/test-ios.yml. It adds a 900-second shell-command timeout around xcodebuild; it introduces no Swift production declarations or actor-i…
Cmux Swift Blocking Runtime ✅ Passed PASS: The authoritative diff changes only .github/workflows/test-ios.yml. It adds a shell timeout wrapper and an environment variable; it does not add or expand any production Swift synchronization …
Cmux Browser Automation Off-Main ✅ Passed The pull request changes only .github/workflows/test-ios.yml. It adds a timeout wrapper around xcodebuild in the iOS simulator test step. It does not change browser socket commands, WebKit/AppKit …
Cmux Expensive Synchronous Load ✅ Passed PASS: The authoritative diff changes only .github/workflows/test-ios.yml. It adds a timeout wrapper around an iOS xcodebuild invocation. It adds no production Swift code and no agent-history load …
Cmux Cache Substitution Correctness ✅ Passed PASS: The pull request changes only .github/workflows/test-ios.yml. It adds a timeout environment variable and wraps xcodebuild in a shell helper. It does not change production Swift, TypeScript, …
Cmux No Hacky Sleeps ✅ Passed PASS: The pull request changes only .github/workflows/test-ios.yml. The rule explicitly excludes GitHub Actions workflow YAML from this check. The referenced scripts/blacksmith-bounded-command.sh …
Cmux Algorithmic Complexity ✅ Passed PASS: The PR changes only .github/workflows/test-ios.yml. It adds a fixed 900-second bound and wraps one xcodebuild invocation. It does not add collection scans, per-target rescans, sorting/filter…
Cmux Swift Concurrency ✅ Passed PASS: The pull request changes only .github/workflows/test-ios.yml. It adds a shell-script wrapper and timeout environment variable around xcodebuild; it introduces no Swift code or legacy Swift c…
Cmux Swift @Concurrent ✅ Passed PASS: The pull request changes only .github/workflows/test-ios.yml; it contains no Swift changes. The cmux Swift @concurrent`` check is not applicable.
Cmux Swift Package Boundaries ✅ Passed PASS. The pull request changes only .github/workflows/test-ios.yml. It adds a timeout wrapper around xcodebuild; it contains no production Swift changes and cannot violate the Swift package bounda…
Cmux Swiftpm Lockfiles ✅ Passed PASS. The PR changes only .github/workflows/test-ios.yml and wraps an existing xcodebuild test-without-building call with a timeout. It does not change Package.swift, package references, `.gitig…
Cmux Swift Logging ✅ Passed The authoritative PR diff changes only .github/workflows/test-ios.yml. It adds a timeout environment variable and wraps xcodebuild with scripts/blacksmith-bounded-command.sh; it adds no Swift or…
Cmux User-Facing Error Privacy ✅ Passed PASS. The authoritative diff changes only .github/workflows/test-ios.yml, in the internal ios-simulator CI job. The added timeout variable, wrapper invocation, and comments produce CI/operator dia…
Cmux Full Internationalization ✅ Passed PASS: The PR changes only .github/workflows/test-ios.yml. It adds an operational timeout environment variable, a bounded xcodebuild invocation, and developer-facing workflow comments. It adds no u…
Cmux Swiftui State Layout ✅ Passed PASS: The pull request changes only .github/workflows/test-ios.yml. The diff adds a shell timeout wrapper and an environment variable; it does not change SwiftUI or Swift source. Therefore, the Swif…
Cmux Architecture Rethink ✅ Passed PASS. The pull request changes only .github/workflows/test-ios.yml; it introduces no Swift architecture code. The change adds a 900-second bound around an existing xcodebuild invocation and uses t…
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS: The pull request changes only .github/workflows/test-ios.yml. The authoritative diff contains no Swift or window declarations, so it does not introduce or change a cmux-owned auxiliary window …
Cmux Source Artifacts ✅ Passed PASS. The PR changes only .github/workflows/test-ios.yml, a hand-written CI configuration. The diff adds an environment setting, comments, and a wrapper call; it adds no logs, caches, build output, …
Cmux No Test Or Debug Seam In Production Source ✅ Passed The pull request changes only .github/workflows/test-ios.yml. It adds a timeout environment variable and wraps xcodebuild with scripts/blacksmith-bounded-command.sh; it adds no Swift file or pro…
Full details: Linked Issues check

Explanation

The change satisfies issue [#13918] ask 1. It sets XCODEBUILD_TIMEOUT_SECONDS to 900 and wraps each simulator xcodebuild invocation with scripts/blacksmith-bounded-command.sh, so a teardown wedge returns a non-zero status within the retry loop. The change does not satisfy ask 2. It does not make timeout-minutes expiry distinguishable from superseded cancellation.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review — holds up, merging

One workflow file, +12/-1, green, CLEAN.

I checked the two things that would make this not work rather than assuming them:

The wrapper propagates the child's status. scripts/blacksmith-bounded-command.sh prefers gtimeout/timeout and otherwise falls back to a Python implementation ending in raise SystemExit(process.wait(timeout=seconds)), so a real xcodebuild failure still exits with xcodebuild's code rather than being flattened into a generic timeout code. That matters because the loop below classifies on it.

The pipeline still reports the right status. The invocation is piped to tee, but the step runs under set -euo pipefail and the branch below reads status="${PIPESTATUS[0]}", so the wrapper's exit is what gets classified — not tee's. A timeout therefore lands in selected_tests_passed_despite_xcodebuild_status, which is exactly right for the failure mode in #13918: a teardown wedge happens after the run, so the log already contains the passing summary and that helper converts it to exit 0 instead of a spurious red.

That is the part worth stating plainly, because it is the whole value: this does not just fail faster, it turns an unclassifiable 35-minute job timeout into an outcome the existing cleanup-failure path already knows how to accept. Before this, a wedge after a green run burned the job ceiling and reported failure.

Non-blocking, but the stated margin is per-invocation, not per-job. The comment says 900s is "well under the job's 35-minute ceiling so a wedge still leaves room for the steps after this one." True for one attempt; the enclosing loop is for attempt in 1 2, so two wedged attempts is 1800s of xcodebuild alone, plus two simctl erase/boot/bootstatus cycles, against a 35-minute ceiling. The second wedge would likely still hit the job timeout and lose the post-steps. In practice the retry only fires on a launch-failure grep, so a teardown wedge should not reach attempt 2 — but if you wanted the bound to hold unconditionally, ~600s would survive two attempts with room to spare. Not worth blocking on.

Worth knowing that this lane's reliability is load-bearing right now: PR CI runs no iOS job at all, so dispatch runs of this workflow are the only iOS signal that exists, and I have been relying on them today to verify test changes that CI reports green without executing.

Enabling auto-merge; required checks remain the gate.

— Zarathustra g1 🌱
Run: run_cmux_mainred_triage_20260923_c6

@teamleaderleo
teamleaderleo enabled auto-merge (squash) September 23, 2026 06:58
@teamleaderleo
teamleaderleo merged commit ca867b7 into manaflow-ai:main Sep 23, 2026
52 checks passed
rustybret pushed a commit to rustybret/bmux that referenced this pull request Sep 23, 2026
ca867b7 ci(ios): bound the xcodebuild test invocation so a teardown wedge fails fast (manaflow-ai#13927)
9ffbb6a ci: compile the E2E test product once, in its own job (manaflow-ai#13908)
827f614 ci: let the macOS 15 and 26 pools share one Swift package cache (manaflow-ai#13925)
b79a83b Price GPT-6 models in coderouter API-equivalent estimates (manaflow-ai#13892)
ff20a22 Expose in-flight drag intent to custom JavaScript sidebars (manaflow-ai#13841)
3344583 Capture Cloud Desktop click destinations before queued opens (manaflow-ai#13897)
ac041c1 test(ios): assert the letterbox a daemon-push shrink actually produces (manaflow-ai#13920)
ce1c55c Catch guard-group drift between ci.yml and GROUPS (manaflow-ai#13924)
3466781 ci: keep leading whitespace in workload profile git output (manaflow-ai#13883)
78e0d83 Make the shortcut reference list every action the schema accepts (manaflow-ai#13911)
94fc7e8 ci: stop buying a universal Release build for CI janitors and reporters (manaflow-ai#13912)
b91fff1 fix(ios): restore the package conventions lint to green on main (manaflow-ai#13904)
6defb93 ci: skip the nightly publish when no changed path reaches the app (manaflow-ai#13899)
c57b001 ci: let E2E runs seed the compilation cache from any revision on main (manaflow-ai#13900)

# Conflicts:
#	.github/workflows/nightly.yml
#	.github/workflows/perf-activation.yml
#	.github/workflows/test-depot.yml
#	.github/workflows/test-e2e.yml
#	.github/workflows/test-ios.yml
@teamleaderleo
teamleaderleo deleted the fix/ios-xctest-teardown-bound branch September 23, 2026 11:35
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.

ios-simulator wedges 33 min in xctest teardown after the suite finishes, and reports as cancelled

1 participant