Test suite deadlocks on real pgrep/process spawns; 249 suites bypass KeyPathTestCase safety base

[malpern]

Summary

The test suite hard-deadlocks on several suites that spawn real pgrep/process subprocesses, making swift test unable to complete locally. The deadlock signature is a process parked at ~0.2% CPU in state S with no KeyPath code on any thread — the main thread sits inside XCTest's observation block waiting on a continuation that never resolves. Normal suite runtime is ~60s; a hung run sits forever until killed.

This was hit repeatedly while validating PR #697 (Tier-2 cleanup). The deadlock is unrelated to those changes (pure SwiftUI view edits) — it reproduces on its own.

Confirmed deadlocking suites/tests

• ErrorHandlingTests (e.g. testResetToDefaultConfig / concurrent + reset config ops)
• KanataManagerResetTests
• KeyPathTests.testResetToDefaultConfig

There are likely more — these are just the ones reached before each run was killed. They share a pattern: they instantiate RuntimeCoordinator / call real saveConfiguration, resetToDefaultConfig, updateStatus(), cleanup().

Reproduces both in parallel and with --no-parallel, so it is intrinsic to these suites, not solely a parallelism artifact (though parallelism makes it more likely per the KeyPathTestCase docs).

Root cause

VHIDDeviceManager.detectConnectionHealth() (reached via RuntimeCoordinator) spawns pgrep subprocesses with 3s timeouts. Under repeated/parallel invocation these subprocesses deadlock. The project already has the fix — KeyPathTestCase base class — which installs the seam:

VHIDDeviceManager.testPIDProvider = { [] }   // no real pgrep ever spawns

…plus WizardDependencies injection and singleton reset. Its header comment documents this exact hazard.

The gap is adoption, not design:

Base class	Suite count
XCTestCase (direct)	249
KeyPathTestCase	12

The 249 direct-XCTestCase suites are mostly fine (pure logic), but any that construct RuntimeCoordinator / InstallerEngine / SystemValidator / VHIDDeviceManager are latent deadlocks. ErrorHandlingTests is a concrete example: it does lazy var manager: RuntimeCoordinator = .init() while extending XCTestCase directly.

Secondary symptom (the documented 18-failure baseline)

Separately, RuleCollectionsManagerTests (8), PackInstallIntegrationTests, and MapperSaveIntegrationTests fail (~18 total) with CustomRulesStore "dataCorrupted/Unexpected character 'o'" + temp-dir write failures — these look environmental. They run alphabetically after K, so locally a K-suite deadlock prevents ever reaching them. Worth confirming whether these are test-isolation/temp-dir issues vs. real bugs.

Proposed fixes

Reliability

1. Migrate offending suites to KeyPathTestCase. Start with the confirmed three, then grep for every test file referencing RuntimeCoordinator|InstallerEngine|SystemValidator|VHIDDeviceManager and convert any that extend XCTestCase directly.
2. Enforce mechanically — a SwiftLint custom rule or CI grep guard: a test file referencing those four types must subclass KeyPathTestCase. This is the single highest-leverage guard against regression.
3. Investigate the 18 baseline failures (temp-dir isolation in CustomRulesStore/PackInstall/MapperSave).

Speed / fail-fast
4. Add a per-test or per-suite timeout in CI (watchdog or --xctest-timeout) so a deadlock fails loudly in ~30s instead of hanging the CI slot silently. A deadlock that fails is far more useful than one that hangs.
5. Preserve the <5s / ~530-test target by keeping the seams (no disk/process/network in unit tests) — e.g. ErrorHandlingTests.testConcurrentConfigurationOperations (5 concurrent real saveConfiguration calls) should drive a fake store, not the filesystem.

Acceptance criteria

• swift test completes locally (parallel) without hanging
• No test suite spawns real pgrep/launchctl (all coordinator-touching suites on KeyPathTestCase)
• CI fails fast (bounded per-test timeout) instead of hanging on a deadlock
• Lint/CI guard prevents new direct-XCTestCase suites from touching the four hazard types
• The ~18 environmental failures are triaged (fixed or documented as known-flaky with isolation fix)

---

Filed from PR #697 validation; see CLAUDE.md test rules and Tests/KeyPathTests/KeyPathTestCase.swift.

[malpern]

Progress (autonomous round)

Done & merged:

• Lint guard (test(lint): guard the KeyPathTestCase seam against new deadlocking suites (#698) #704) — TestSeamLintTests ratchet blocks new XCTestCase-direct coordinator suites.
• 9 suites migrated to KeyPathTestCase (test: migrate 9 coordinator suites to KeyPathTestCase (#698) #706) — allowlist 17 → 8. Each verified in isolation (no deadlock, no behavior change): ContentViewDebounce, GrabRecoveryGate, InstallerEngineBrokerForwarding, KanataViewModel, KeyboardCapture, LayerSelector, OverlayHealthIndicatorObserver, RecordingCoordinator, WizardRecipeParity. CI build-and-test (full suite) green.
• CI fail-fast: already exists — Scripts/run-tests-safe.sh has a 240s watchdog that kills + exits 124, with a 10-min job cap. No change needed; the recommendation was already implemented.

Remaining (the hard 8 still allowlisted):

• Real-I/O suites — need deeper mocking, not just the base class: ErrorHandlingTests, KeyPathTests, KanataManagerResetTests (RuntimeCoordinatorResetTests.swift). Verified the seam fixes their pgrep hang (e.g. ErrorHandlingTests.testAsyncConfigurationErrors: hangs-forever → 8ms) but they then hang on real saveConfiguration/updateStatus/resetToDefaultConfig I/O. Migrating alone is insufficient; these need fake config/process stores.
• VHIDDeviceManagerTests — intentionally NOT migrated: it's the seam's own test (sets testPIDProvider per test, asserts the real-pgrep fallthrough at line ~171). Keep allowlisted.
• Reference-only (low priority): FacadeLintTests, PrivilegedOperationsCoordinatorTests, SystemRequirementsTests, WizardPureLogicTests — only name the types; migrate opportunistically.

Net: the deadlock surface is much smaller and regression is now blocked by the lint guard. The remaining work is the per-suite I/O mocking for the 3 real-I/O suites.

[malpern]

#1 (real-I/O mocking) — diagnosis from an attempt

Tried to fix the 3 real-I/O suites. Got a precise root cause but it needs deeper work than a single seam. Recording it so the next attempt starts ahead.

Exact hang (ErrorHandlingTests): testAsyncResetConfigurationErrors → manager.resetToDefaultConfig(). The method already guards its own TCP reload (!TestEnvironment.isRunningTests), so the hang is earlier, in the store/regenerate chain:

resetToDefaultConfig()
  → ruleCollectionsManager.bootstrap()              (RuleCollectionsManager+Bootstrap.swift)
     → regenerateConfigFromCollections()            (RuleCollectionsManager+Migrations.swift)
        → await onRulesChanged?()                    (callback into RuntimeCoordinator → triggerConfigReload)
        → configurationService.saveConfiguration(...) (also async I/O)

ConfigReloadCoordinator.triggerConfigReload() runs blocking pre-checks before its TCP guard (line 167) — notably KanataDaemonManager.refreshManagementStateInternal() (SMAppService IPC, the documented "blocks 10-30s" anti-pattern), plus a health check and JIT permission gate.

Attempted fix that was NOT sufficient: adding if TestEnvironment.isRunningTests { return no-op success } at the top of triggerConfigReload(). ErrorHandlingTests still hung at the same spot — so there's at least one more blocking point in the bootstrap → regenerateConfigFromCollections chain (likely configurationService.saveConfiguration or a store actor), not just the reload. Reverted.

What a real fix needs: seam the whole config-regeneration chain for tests — inject test config dir + test stores (CustomRulesStore.testStore(at:) / RuleCollectionStore already have DEBUG test-store hooks), and gate onRulesChanged/saveConfiguration/SMAppService in the regenerate path. This is per-suite test-design work across RuntimeCoordinator + RuleCollectionsManager + the stores — not a one-line guard.

Shipped instead: allowlist tightening 8→4 (#707).