test: migrate 9 coordinator suites to KeyPathTestCase (#698)

[malpern]

Part of #698 — shrinks the test-seam allowlist from 17 → 8 by migrating suites that construct coordinators onto KeyPathTestCase (installs the testPIDProvider seam, preventing the real-pgrep deadlock).

Migrated (9) — each verified in isolation locally

ContentViewDebounceTests, GrabRecoveryGateTests, InstallerEngineBrokerForwardingTests, KanataViewModelTests, KeyboardCaptureTests, LayerSelectorTests, OverlayHealthIndicatorObserverTests, RecordingCoordinatorTests, WizardRecipeParityTests.

• Batch of 8: 47 tests / 0 failures / 4.3s; KeyboardCaptureTests: 22 / 0 / 1.6s — no deadlock, no behavior change.

Deliberately still allowlisted

• Real-I/O suites (ErrorHandlingTests, KeyPathTests, KanataManagerResetTests): the seam fixes their pgrep hang (verified: ErrorHandlingTests.testAsyncConfigurationErrors went from hangs forever → 8ms) but they also do real saveConfiguration/updateStatus I/O that still hangs. Need deeper per-suite mocking — tracked in Test suite deadlocks on real pgrep/process spawns; 249 suites bypass KeyPathTestCase safety base #698.
• VHIDDeviceManagerTests: the seam's own test suite (sets testPIDProvider per test, asserts the real-pgrep fallthrough). Must not be force-seamed.
• FacadeLintTests, PrivilegedOperationsCoordinatorTests, SystemRequirementsTests, WizardPureLogicTests: only reference the type names.

The TestSeamLintTests ratchet passes with the reduced allowlist (verified in isolation). CI build-and-test exercises the full suite as the backstop.

[claude]

Code Review: PR #706 — Migrate 9 coordinator suites to KeyPathTestCase

Overall: LGTM with one minor doc nit. This is a clean, well-scoped mechanical refactor. The changes do exactly what they say, the ratchet enforces the invariant going forward, and the PR description provides solid evidence of no regressions.

---

What this PR does

Migrates 9 test suites from XCTestCase → KeyPathTestCase to install the testPIDProvider seam, eliminating real-pgrep calls that could deadlock. The TestSeamLintTests allowlist shrinks from 17 → 8 as the ratchet mechanism.

---

Code quality ✅

• Every change is a single-line swap — minimal blast radius, easy to verify
• Allowlist delta exactly matches the 9 listed suites (17 − 9 = 8 ✓)
• The "deliberately still allowlisted" commentary in the PR body is exactly the right level of explanation for a ratchet PR
• Confirmed test results (47 tests / 0 failures / 4.3s; 22 / 0 / 1.6s) give good confidence no behavior changed

---

One doc nit 📝

The PR description refers to KanataManagerResetTests as a real-I/O suite remaining on the allowlist:

ErrorHandlingTests, KeyPathTests, KanataManagerResetTests

But the file is actually RuntimeCoordinatorResetTests.swift (the allowlist and the file on disk both use RuntimeCoordinator). Worth a quick fix before merge so the description stays accurate for future readers tracing issue #698.

---

No concerns on

• Correctness: KeyPathTestCase inherits XCTestCase, so all existing test machinery is preserved
• GrabRecoveryGateTests lacking @MainActor: pre-existing condition, not introduced here; KeyPathTestCase doesn't require it
• Performance: strictly an improvement — seam blocks real pgrep shell-outs in these suites
• Test coverage: no tests removed or weakened; the lint ratchet is the guard rail
• Security: N/A

---

Verdict: Fix the KanataManagerResetTests → RuntimeCoordinatorResetTests typo in the body and this is ready to merge.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 41c3bde3ea

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Migrate the remaining logging suite too

This file still contains Phase1LoggingTests: XCTestCase at line 127, and that suite constructs RuntimeCoordinator() in testLoggingCapturesActualOperations. Because TestSeamLintTests skips an entire file once it sees : KeyPathTestCase, changing only ContentViewDebounceTests here removes the file from the allowlist while leaving the logging suite without the VHIDDeviceManager.testPIDProvider seam, so this test can still take the real pgrep path that this migration is meant to avoid.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Migrate the second observer suite

This file also defines HealthObserverInitializationOrderTests: XCTestCase at line 218, and that suite creates SystemValidator(vhidDeviceManager: VHIDDeviceManager(), ...). Since the lint guard checks for : KeyPathTestCase at file scope, this partial migration makes the file pass the ratchet even though that remaining suite still does not install the pgrep-avoidance seam, leaving the deadlock-prone path possible under parallel test runs.

Useful? React with 👍 / 👎.