Skip to content

CI: GitHub-hosted fallback while the mini is offline - #1344

Draft
malpern wants to merge 7 commits into
masterfrom
ci/hosted-runner-fallback
Draft

malpern wants to merge 7 commits into
masterfrom
ci/hosted-runner-fallback

Conversation

@malpern

@malpern malpern commented Oct 26, 2026

Copy link
Copy Markdown
Owner

Summary

The mini, which hosts both CI runners, is offline for servicing, so no PR's required checks can run. This adds a switch that moves CI to GitHub-hosted runners without changing the mini setup.

  • Switch: set the repo variable KEYPATH_CI_RUNNER=hosted, and build-and-test and code-quality run on GitHub's xcode-27 image (macOS 27, public preview). Delete the variable and both jobs run on the mini exactly as before. The job names, and so the required checks, are the same either way.
  • Hosted runs only:
    • Pin Xcode 27.0, build 27A266a (the build the Macs use), and fail if the image changes it. Set SDKROOT so cargo and Swift use the same SDK.
    • Disk-reserve floor of 12 GiB instead of 40. This is a guess until a run shows real usage.
    • 60-minute timeout for build-and-test and 30 for code-quality, since every run is cold.
    • A separate concurrency group for each job, so the two run in parallel.
    • The test-runner crash allowance is off.
    • Install mise, SwiftLint and ripgrep from Homebrew, and trust mise.toml so the SwiftFormat pin applies.
  • Lint scripts: both now fail if rg is missing. They used to pass without checking anything.
  • Contract test: DeploymentScriptContractTests now reads the runner expression. It checks that only build-and-test uses the safe runner and that both jobs switch together.

The variable is already set, so this PR's own CI is the first hosted run. Coverage, cache-warm and runner-health stay on the mini; they're scheduled jobs and don't block PRs.

Status: draft until a hosted run passes

The local review gate still reports open findings that only a real run can settle:

  • whether xcode-27 resolves to a runner;
  • whether 12 GiB of disk and 60 minutes are enough;
  • whether the image's Xcode_27.app stays at 27.0;
  • whether the macOS 27 SwiftPM AppSSO trap affects hosted runners.

I'll fix what actually breaks, then mark this ready.

Not changed: SwiftLint on hosted runs is Homebrew's current version, not pinned. It lints only changed files.

Rollback

gh variable delete KEYPATH_CI_RUNNER --repo malpern/KeyPath

🤖 Generated with Claude Code

malpern and others added 4 commits October 25, 2026 17:03
Setting the repo variable KEYPATH_CI_RUNNER=hosted runs build-and-test
and code-quality on GitHub's xcode-27 image (macOS 27, Xcode 27.0
27A266a). Unset, both jobs run on the mini exactly as before. Job names,
and so the required checks, don't change.

On hosted runs:
- the disk-reserve check is skipped
- the timeout is 60 minutes, since every run is cold
- each job gets its own concurrency group
- mise and SwiftLint are installed from Homebrew

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… test

- Hosted runs now pin Xcode 27.0 through the stable-Xcode path, which
  keeps the scripts' version check, instead of an override that skips it.
- Each hosted job gets its own concurrency group, so the two jobs run in
  parallel.
- The disk check runs on hosted too, with a smaller floor (12 GiB).
- code-quality gets 30 minutes on hosted.
- mise is told to trust mise.toml.
- The test-runner crash allowance is off on hosted.
- The contract test now reads the runner expression and checks both
  jobs switch together.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…r guard

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Both lint scripts read a missing rg as "no hits" and passed without
checking anything.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Oct 26, 2026

Copy link
Copy Markdown

Review of the supplied diff: no blocking issues. The runner-selection expressions are valid. The hosted path is gated on one variable, and the jobs fall back to the self-hosted labels when the variable is unset. The DeploymentScriptContractTests changes also guard against a literal safe-runner job slipping into ci.yml.

Minor notes, not blockers:

  • lint-no-subprocess-in-autofixer.sh: the command -v rg guard fixes the missing-binary case. rg exit code 2 (an error, such as a missing file) still falls into the 'no hits' branch of the if. Consider checking for exit code 1 explicitly.
  • Install ripgrep (hosted runner) runs before the autofixer lint. If lint-no-sleep.sh or any other rg user runs earlier in build-and-test, move the install step ahead of it. I couldn't tell the order from the diff.
  • The hosted KEYPATH_ALLOW_TEST_RUNNER_CRASH_SUCCESS=0 setting and the 12 GiB disk floor are untested guesses, as the comments already say. The first hosted run may need those tuned.
  • The Xcode-select step is duplicated across both jobs. A composite action or a shared script would keep them from drifting.

rg exits 2 on an error such as a missing path, and both lints read that
as "no hits". The autofixer lint had been passing that way since
WizardAutoFixer.swift was deleted; it now says so explicitly.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Oct 26, 2026

Copy link
Copy Markdown

Review of PR 1344: no blocking findings. The runner-selection expressions, job-level concurrency and timeout expressions, and the rg exit-code handling all look correct.

Minor, non-blocking:

  1. Scripts/lint-no-subprocess-in-autofixer.sh now exits 0 when WizardAutoFixer.swift is missing. That is right for the deleted file. But a rename or move would also pass silently. Consider scanning the InstallationWizard directory instead, or deleting the lint.
  2. On hosted runners swiftlint is installed unpinned through brew, while swiftformat is pinned by mise. A SwiftLint version drift could produce hosted-only lint failures. Consider pinning it if the fallback is used for long.
  3. Install ripgrep (hosted runner) in build-and-test runs after the Kanata steps. If any earlier step or script uses rg, it will fail on hosted. It is fine if none do.
  4. The lint scripts write to fixed /tmp paths. This was already the case, but it is a hazard when jobs overlap on the shared mini runner. mktemp would be safer.

Test coverage: the DeploymentScriptContractTests update correctly pins the safe-runner choice to once, the hosted selector to twice, and the absence of a literal runs-on safe runner.

… lint

WizardAutoFixer.swift was deleted on 2026-05-02 (9d62b11), so the lint
has had nothing to check since. The sleep lint wrote to a fixed /tmp
path, which overlapping jobs could share.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Oct 26, 2026

Copy link
Copy Markdown

Review of the diff only. I found nothing blocking, and I haven't run the workflow.

  • lint-no-sleep.sh: The rg exit-code handling is correct. A missing rg or a bad path now fails instead of passing silently. exit 1 inside search still aborts when it's called from an if.
  • Deleted autofixer lint: WizardAutoFixer is already deleted, so the old script was dead. It passed only because rg erroring on the missing file counted as no hits. I can't see the whole tree from the diff, so grep for leftover references to lint-no-subprocess-in-autofixer in docs, hooks and other workflows.
  • Hosted build-and-test: Please confirm on the first hosted run that it has what the mini has.
    • KEYPATH_BUNDLED_SIMULATOR_OVERRIDE points at /opt/homebrew/bin/kanata-simulator, and the kanata --version step assumes Kanata and the simulator are installed. Neither is provisioned in the hosted path.
    • Only code-quality installs ripgrep. If build-and-test runs any rg-based lint, it needs rg too, and the new rg guard will now fail hard there.
  • Xcode pin: The hosted step sets KEYPATH_STABLE_XCODE_DEVELOPER_DIR to Xcode 27.0, but Scripts/lib/xcode.sh and DeploymentScriptContractTests pin 26.6.0 through KEYPATH_STABLE_XCODE_VERSION. If the scripts check that version separately, the hosted run will fail on a mismatch. Override the version too, or confirm the check is skipped.
  • Duplication: The "Select Xcode 27.0" step is copied verbatim into both jobs. A composite action or a shared script would keep them from drifting.
  • Tests: The contract test is updated for the new runner expression and guards against a literal safe-runner job. Nothing exercises the hosted path, so the first hosted run is the real test.

The suite shared SubprocessRunnerFake.shared with nine other test
files. Swift Testing runs suites in parallel, so another suite's reset()
could clear the command log between the service's pgrep and the
assertion. On the first hosted run, isRunning was true but the pgrep
command was missing from the log. Each test now creates its own fake.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Oct 26, 2026

Copy link
Copy Markdown

Review of PR 1344: no blocking issues found. A few non-blocking notes:

  1. Hosted build-and-test tooling. code-quality installs ripgrep and mise/SwiftLint on the hosted path, but build-and-test has no equivalent step. /opt/homebrew/bin/kanata-simulator is also hard-coded in its env. The diff doesn't show whether the kanata / kanata-simulator install step and any rg-dependent scripts work on a stock xcode-27 image. The first hosted run should confirm this. lint-no-sleep.sh now fails fast when rg is missing, which is good.
  2. Removed autofixer lint. Deleting lint-no-subprocess-in-autofixer.sh is fine because WizardAutoFixer.swift no longer exists (AGENTS.md says it was deleted). The old script would have passed silently on the missing file, since rg exited 2 inside an if. The lint-no-sleep.sh exit-code handling (only rc 1 counts as a pass) fixes the same kind of silent pass.
  3. Contract test coupling. DeploymentScriptContractTests now matches exact expression strings in ci.yml. Any reformatting of the runs-on expression will fail it. That is acceptable, since it is the intended guard, but the failure message could point at the expected literal.
  4. Test isolation. Giving each Karabiner test its own SubprocessRunnerFake() is the right fix for the parallel shared.reset() race. Other tests that still use SubprocessRunnerFake.shared with reset() can have the same flake. This is not blocking.

This branch has not been deployed

No deployments
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.

1 participant