Repository navigation
fix(ci): stop local and CI test runs from corrupting each other (#816) - #849
Merged
Merged
Conversation
The first cut had each side pass its own checkout and treat everything
else as foreign. Implementing the CI call site showed that would hurt CI
on every run: DerivedData lives in ~/Library/Developer/Xcode/DerivedData,
not under $GITHUB_WORKSPACE, so a stale OnlyCue.app left by an
interrupted previous job reads as foreign — and the job would burn its
whole 900s budget waiting for a process the very next lines of `Reset
stale test state` were about to kill.
Asking positively cannot make that mistake, and it lets the two sides ask
the different questions they actually have:
local is a CI job running? --runner
CI is anything running out of the --under <dir>
maintainer's working copy?
A new case pins the stale-own-app scenario so the negative form cannot
come back, and the existing cases move to the new flags. All five
mutants — inverted root match, ungated Runner.Worker, whole-line
classification, no-predicate guard removed, app/XCTRunner arms removed —
turn the suite red.
`Reset stale test state` asks first, before its pkills — those would reap a live local run's processes, and the point is to wait for it, not kill it. Bounded at 900s, then warn and continue: a red `dev` for a reason that has nothing to do with `dev` is exactly what this guard exists to stop, so it must not create one. timeout-minutes goes 90 -> 105 to cover the added wait on top of the existing ~82 min worst case. `UI tests (behavioral)` re-checks with no wait, purely to annotate, so a failing step's own log says whether the machine was contended at that moment instead of leaving it to be reconstructed from `ps` afterwards. scripts/dev/test.sh asks the mirror question with --runner, plus the two runner work roots as a second signal for a worker that died leaving its xcodebuild behind.
Also writes down the two things found by running the detector against the real machine rather than a canned table: the self-match on a shell whose command line merely mentioned xcodebuild, and the gap where a local run's app host is not attributable to ~/Projects.
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
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.
Summary
OnlyCueTestswas not flaky — the self-hosted runner shares the maintainer's Mac mini, and the two sides were destroying each other's test runs. This adds a pre-flight contention guard that both sides consult before starting.Closes #816
Root Cause
Not a test bug. The runner executes as user
chuanon the same machine as local development, so nothing separates a CI run from a local one except path. Two measurements on 2026-09-15:psthen showedOnlyCue.app --ui-test-seed=…at 220% CPU under a foreign DerivedData hash plus a liveRunner.Worker— the runner was executing dev run 34967315810 through both. With the machine verified idle, the same suite passed: 1822 tests, first try.devfailed three consecutive runs (34947557702, 34952854230, 34967315810), every one onUI tests (behavioral), every one with the same signature: 6–10 ×Lost connection to the application— the app process dying, not an assertion. Localxcodebuild/dotnet testwas running during all three. Run 34969558846, the first where the machine was deliberately left alone, passed first try with no retry.Each is n=1 on its own and neither is proof; together they say contention, not code. The damage is that the three red dev runs invite exactly the wrong conclusion ("something in #835 broke UI tests" — nothing did), and every future flake costs a debugging session to re-derive that.
Fix Description
scripts/ci/machine-busy.shanswers "is another OnlyCue test run using this machine?", and both sides call it. The two sides ask different questions, which is the part that took two attempts to get right:--runner(+ the runner work roots)--under "$HOME/Projects"The correction, recorded because it would have hurt CI on every run. The first design had each side pass its own checkout and treat everything else as foreign (
--self). DerivedData lives in~/Library/Developer/Xcode/DerivedData, not under$GITHUB_WORKSPACE, so a staleOnlyCue.appfrom an interrupted previous job reads as foreign — and CI would burn its entire 900 s budget waiting for a process the very next lines ofReset stale test statewere about to kill. Asking positively cannot make that mistake. Case 3c in the test suite pins it.CI warns, never fails. A red
devfor a reason that has nothing to do withdevis precisely what this guard exists to stop, so it must not create one. Bounded 900 s wait, then::warning::and continue.No load-average gate, reversing something I floated in the issue: process detection is the direct signal, load average is a lagging proxy for it, so gating on load would add a failure mode (an unrelated heavy process blocking tests indefinitely) to catch nothing the process check misses. Load is printed for diagnostics only.
Changes
scripts/ci/machine-busy.sh— the detector. Classifies on the executable (xcodebuild,OnlyCue.app/Contents/MacOS/OnlyCue,XCTRunner,Runner.Worker) and matches watched roots against the arguments.MACHINE_BUSY_PSseam so it can be tested.scripts/ci/machine-busy.test.sh— 17 cases, plain bash per the repo's existing*.test.shconvention.scripts/dev/test.sh— local wrapper: guard, then thekillall -9 testmanagerdreset (chore(ci): self-heal the testmanagerd control-session wedge on the runner #471/ci: daemon reset no-ops (sudo -n killall) so testmanagerd wedge never clears #595), thenxcodebuild testwith ad-hoc signing. Deliberately thin; it is not a build system..github/workflows/ci.yml— guard as the first action ofReset stale test state(before thepkills, which would otherwise reap a live local run's processes); a zero-wait annotating re-check atUI tests (behavioral);timeout-minutes90 → 105;machine-busy.test.shadded toCI script tests.docs/superpowers/specs/816-machine-contention-preflight.md— the spec, including the correction above.How to Test
scripts/ci/machine-busy.test.sh— 17 passed, 0 failedRunner.Worker(1), whole-line classification (7), no-predicate guard removed (1), app/XCTRunnerarms removed (2). My first mutation run reported all five surviving — theperlsubstitutions had silently failed to match; the rerun asserts the file actually changed before trusting the result.xcodebuildunder~/Projects→ CI-side guard exits 1 and names the pid; local-side guard stays 0; after it exits, 0.Runner.Worker,scripts/dev/test.sh --no-waitexits 1, names the worker, and noxcodebuildstarts.scripts/dev/test.sh→** TEST SUCCEEDED **, 1827 tests, 0 failures.scripts/ci/run-with-watchdog.test.shstill 9/9;shellcheckclean on all three scripts;ci.ymlparses.A real bug the canned fixtures could not catch. Running the detector for real flagged the very shell that invoked it, because the command I had typed mentioned
xcodebuild. Any shell, editor,grepor CI step naming the binary would have held the guard busy forever — a guard that manufactures the phantom failures it exists to prevent. Fixed TDD-style (failing case 3b first), by classifying on the executable rather than the whole command line.Regression Risk
Moderate and concentrated in
ci.yml, since a guard that misfires blocks every run — which is why the CI side can only ever warn.Two known gaps, stated so they are known rather than silent:
flockwould catch it, and a fixed shared path is a known trap in this repo (bug(ci): CIRuntime guards stop firing on the UI-test retry attempt #789) — a stale lock from akill -9'd local run would hold CI to the job timeout. The local wrapper is the mitigation, and it covers the direction that actually caused the three red runs.~/Projects. CI catches the localxcodebuildinstead, which is alive for the whole local run; because the guard runs before thepkills, a live local run is waited out rather than killed.Worst realistic cost of a misfire: 15 min of extra wall-clock on a job whose real work is already watchdog-capped.
OnlyCue verification (required)
Spec link:
docs/superpowers/specs/816-machine-contention-preflight.md#input--outputCloses: #816
8a694e8pins the contract,fd86a27implements it; the laterd3063f0corrects both together as one contract change, with the mutants proving the new cases can fail)docs/— the--self→--runner/--undercorrection is recorded in the specNote on the template: #816 carries both
bugandtype:test.bug.mdis used here because the change is a bug fix with a root cause and a regression surface;type:testdescribes the symptom's domain, not this change.Note on the check that will run: UI tests are gated on push-to-
dev, so this PR's green check does not cover XCUITests — and it does not exercise the CI-side guard either, since that lives inReset stale test stateon the same gated path. The guard's CI behavior is first observable on the post-mergedevrun.