Repair stale bundled services after KeyPath updates

[malpern]

Summary

• classify stale bundled helper and Kanata identities as repairable installer state
• force-refresh an active stale Kanata SMAppService registration during runtime repair
• wait for normal asynchronous unregister completion before invoking the privileged bootout fallback
• add structured freshness postconditions and regression coverage

Installer state-matrix row

Running and TCP responding, but active bundled runtime identity is stale after an app update. The repair must replace the registration with the current app bundle and independently verify freshness.

Validation

• focused installer/service suites: 34 tests passed
• post-rebase full suite: 3,998 XCTest cases, 29 skipped, 0 failures; all executed Swift Testing suites passed
• Apple notarization accepted private build 8 submission b2f5e259-e792-4b12-b4d8-a0efbf2957af
• managed macOS 15.7.7 clone: build 8 independently detected active build 6 as stale, executed install-required-runtime-services once, passed the recipe postcondition, and converged to a fresh running build 8 runtime
• macOS logged successful SMAppService unregister, uninstalled state, successful register, and active state; no osascript or administrator-authentication event appeared in the repair window
• final clone verification: strict signature valid, Gatekeeper notarization accepted, helper 1.3.0 fresh, VirtualHID healthy, bundled Kanata 1.12.1-prerelease-1

Release safety

The accepted build was private and was not published to the public Sparkle feed or installed over the laptop app. The branch was subsequently rebased over two unrelated master UI commits and the full suite was rerun. Rebuild and repeat signed/notarized validation from merged master before publishing a public release.

[claude]

Review finding

ServiceBootstrapper.registerKanataWithSMAppService (Sources/KeyPathInstallationWizard/Core/ServiceBootstrapper.swift, around the new refreshActiveRegistration branch): when state == .smappserviceActive and refreshActiveRegistration is true, the code now unconditionally calls fixBrokenSMAppServiceState() and returns its result — it no longer checks daemonManager.isRegisteredButNotLoaded() first. Previously, an already-active and healthy registration would just return true as a fast no-op.

PrivilegedOperationsRouter.swift always passes refreshActiveKanataRegistration: true for this operation, with a comment saying it covers "missing, stopped, or stale" runtime — i.e. the caller doesn't actually know whether the active registration is stale vs. already correct for the current bundle. So any time this install-runtime-services action runs while Kanata happens to already be actively and correctly registered (e.g. triggered by an unrelated repair reason), it will now force a full unregister/re-register cycle instead of a no-op:

• Briefly disrupts a working Kanata service (input remapping stops during the boot-out/reinstall).
• If fixBrokenSMAppServiceState() fails for any transient reason on an otherwise-healthy install, the whole operation now surfaces as PrivilegedOperationError.installationFailed, whereas before a genuinely healthy active registration would have succeeded.

Test coverage confirms parameter forwarding (testInstallAllServicesForwardsActiveKanataRefreshRequest) but doesn't cover the "already fresh + active + refreshActiveRegistration=true" case, so this behavior change isn't exercised.

Suggest gating the forced refresh on actual staleness (e.g. only take the refresh path when kanataServiceFreshness == .stale, matching how InstallerDecisionPipeline already decides to add .installRequiredRuntimeServices for staleness) rather than forcing it for every caller of this operation regardless of whether a refresh is actually needed.

[malpern]

Addressed in b00346f. The composite runtime repair now rechecks the active runtime identity before refreshing: fresh or unknown active registrations remain untouched, while stale registrations take the unregister/register path. Added paired regression tests proving fresh active performs 0 unregister/register calls and stale active performs exactly 1 of each. Focused set: 79/79; full suite: 4,000 XCTest cases, 0 failures; pinned SwiftFormat: 0/8 files.

[claude]

Review finding — helper postcondition narrowing may cause false install failures

Sources/KeyPathInstallationWizard/Core/InstallerEngine+Recipes.swift: the installPrivilegedHelper / reinstallPrivilegedHelper recipes had their expectedPostconditions changed from [.helperReadyOrApprovalPending] to [.helperFreshOrApprovalPending] — a replacement, not an addition. The new postcondition (InstallerEngineTypes.swift) is:

case .helperFreshOrApprovalPending:
    return context.helper.freshness(expectedVersion: ...) == .fresh || context.helper.requiresApproval

This drops the previous context.helper.isReady fallback entirely. If freshness(expectedVersion:) returns .unknown (e.g. version metadata not yet refreshed immediately after the helper install completes, or any transient version-read hiccup) rather than .stale, the postcondition now fails even though the helper is installed and functionally working — per AGENTS.md's "mutating installer actions must be postcondition-verified" rule, this would surface as a reported install failure for what is actually a successful, working install.

The added tests (InstallerEnginePlanTests.swift) only cover the planning side (that a stale-version helper produces a reinstallPrivilegedHelper action) — there's no test exercising postcondition verification for a freshly-installed helper whose version read comes back .unknown right after install, which is the scenario most likely to regress.

Suggest either OR'ing context.helper.isReady back into .helperFreshOrApprovalPending, or confirming freshness() synchronously invalidates any version cache before postcondition check and adding a regression test for the .unknown-post-install case.

Other changes (SMAppService unregister-settle polling in ServiceBootstrapper.swift, runtime freshness plumbing) look sound and are reasonably covered by the new tests.

[malpern]

Addressed the second finding in 7b3ac9a with explicit postcondition coverage and confirmed the production capture path. InstallerEngine.captureFreshContext invalidates validator/service caches before the final observation. HelperManager.getHelperHealth returns .healthy only when getHelperVersion returns a non-nil version; a nil read becomes registeredButUnresponsive or temporarilyUnavailable, so production does not report isWorking=true with an unknown version. I intentionally did not OR helper.isReady back in because the existing W6 contract requires unknown version to remain not fresh. New tests prove: working+unknown still satisfies readiness but fails freshness; exact version and pending approval satisfy freshness. Focused set: 94/94; full suite: 4,002 XCTest cases, 0 failures; pinned SwiftFormat: 0/11 files.

[claude]

Correctness: installPrivilegedHelper/reinstallPrivilegedHelper postconditions drop the readiness check

In InstallerEngine+Recipes.swift, both helper-install recipes had their expectedPostconditions replaced:

// before
expectedPostconditions: [.helperReadyOrApprovalPending]
// after
expectedPostconditions: [.helperFreshOrApprovalPending]

.helperReadyOrApprovalPending verifies context.helper.isReady || context.helper.requiresApproval, while .helperFreshOrApprovalPending verifies context.helper.freshness(expectedVersion:) == .fresh || context.helper.requiresApproval. Per testHelperFreshPostconditionRejectsWorkingHelperWithUnknownVersion, freshness is derived purely from version-string comparison and is independent of isWorking/isReady. That means a helper whose on-disk version matches but that isn't actually working (XPC not responding, launchd not loaded, etc.) would now satisfy the postcondition even though it previously would not — the install/repair recipe could report success for a non-functional helper.

Contrast this with the installRequiredRuntimeServices recipe change in the same file, where the analogous freshness postcondition was added alongside the existing readiness postcondition ([.runtimeReadyOrApprovalPending, .runtimeFreshOrApprovalPending, .vhidServicesHealthy]) rather than replacing it. The helper recipes look like they should follow the same additive pattern:

expectedPostconditions: [.helperReadyOrApprovalPending, .helperFreshOrApprovalPending]

No test currently covers helperReady: false with a matching helperVersion to catch this gap (the existing tests only vary version while holding helperReady: true, or vary approval while holding version nil). Worth adding a case for "installed at expected version but not working" to lock in the intended behavior either way.

[malpern]

Addressed in 34bf95c. Both helper install recipes now declare the additive pair [.helperReadyOrApprovalPending, .helperFreshOrApprovalPending], matching the runtime recipe pattern. The underlying freshness check already guards isInstalled && isWorking, but the recipe contract now explicitly preserves both guarantees. Added a regression case proving an exact-version but nonworking helper fails the combined postconditions. Focused set: 95/95; full suite: 4,003 XCTest cases, 0 failures; pinned SwiftFormat: 0/11 files.

[claude]

Reviewed the diff (no repo inspection tools used). One design question worth confirming before merge, plus a minor nit:

1. New stale-unregister race guard may be bypassed on the refreshActiveRegistration path (Sources/KeyPathInstallationWizard/Core/ServiceBootstrapper.swift).
The PR adds waitForKanataSMAppServiceRemoval() specifically to avoid forcing a privileged bootout before SMAppService's async unregister() has actually settled — a real race this PR is fixing. That guard is wired into the generic repair retry loop, but the new refreshActiveRegistration branch in registerKanataWithSMAppService(refreshActiveRegistration:) instead calls the pre-existing fixBrokenSMAppServiceState() when activeRuntimeFreshness() reports .stale. If fixBrokenSMAppServiceState() doesn't reuse waitForKanataSMAppServiceRemoval/bootOutStaleKanataSMAppServiceJob, refreshing an already-active-but-stale registration (the Sparkle-update case this PR targets) could hit the exact same launchd staleness race that was just fixed for the other path. Worth confirming fixBrokenSMAppServiceState() shares that safeguard, or wiring it in explicitly.

2. Minor style nit: in Sources/KeyPathInstallationWizard/Core/InstallerEngine+Recipes.swift, the new expectedPostconditions arrays for installPrivilegedHelper and reinstallPrivilegedHelper are missing a trailing comma after .helperFreshOrApprovalPending, unlike the installRequiredRuntimeServices array in the same file (which has a trailing comma after .vhidServicesHealthy,). Since SwiftFormat 0.61.1 is pinned per AGENTS.md, worth running it before merge to keep this consistent.

Everything else (freshness postconditions, installAllServices(refreshActiveKanataRegistration:) plumbing, and the new tests) looks correct and well covered.

[malpern]

Confirmed both points before merge:

1. The stale-active refresh branch calls fixBrokenSMAppServiceState() at ServiceBootstrapper.swift:1171. That shared function calls daemonManager.unregister(), then waitForKanataSMAppServiceRemoval() at line 726, and only invokes bootOutStaleKanataSMAppServiceJob if the async unregister did not settle. The branch therefore already uses the new race guard. testStaleActiveRegistrationIsRefreshedForRuntimeUpdate exercises this exact path with states [.smappserviceActive, .uninstalled] and verifies one unregister/one register; the separate wait tests cover both settled and persistent-registration outcomes.

2. Pinned SwiftFormat 0.61.1 reports 0/1 files requiring formatting for InstallerEngine+Recipes.swift; the multiline array style is formatter-compliant.

No code change needed for this comment.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8bb027496f

ℹ️ 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]

Verify the helper's registered bundle identity

When upgrading from build 4 to this build, the existing BundleProgram helper can still point at the previous app bundle while responding with the unchanged KeyPathHelperContract.version (1.3.0), so this condition classifies it as fresh and skips re-registration. This is concrete for this update because the shipped helper behavior changed to require VirtualHID 8.0 while the helper version remained 1.3.0; compare the registered parent bundle version/path, or force unregister/register after the bundle update, rather than relying only on the XPC version response.

AGENTS.md reference: AGENTS.md:L317-L320

Useful? React with 👍 / 👎.

[malpern]

Post-merge acceptance is complete on exact merge commit e75ff72e5. KeyPath 1.0.1 build 9 passed Developer ID signing, Apple notarization (d24f7a6e-abec-464e-af3c-5f33931c7e12), stapling, Gatekeeper, and a signed private-feed Sparkle install on managed macOS 15.7.7 lease cbx_940aaa3ecba3. Before repair, independent status showed app build 9 with active runtime build 8 and freshness stale. One install-required-runtime-services run passed its structured postcondition without an admin prompt; final status shows active runtime build 9 fresh, Kanata 1.12.1-prerelease-1 running, helper 1.3.0 fresh/working, and VirtualHID healthy. Whole-system status remains false only because this disposable clone intentionally lacks Kanata Input Monitoring. Evidence: /Volumes/KeyPath Lab/CrabBox/KeyPathInstallerLab/artifacts/cbx_940aaa3ecba3/20260730T164730Z. No public feed/release or laptop app was modified.