Fix CLI stop for KeepAlive Kanata service

[malpern]

Summary

• disable the KeepAlive Kanata launchd job before signaling it so keypath service stop reaches its stopped postcondition
• re-enable the job before start/restart and restore enabled state after an unexpected stop failure
• advance the privileged helper contract to 1.3.1 and app build to 10
• make installed-app release smoke operate only on the canonical config path and use a formatting-stable simple-remap assertion

Validation

• targeted helper lifecycle, helper version, and CLI service tests: 14 passed
• accessibility audit: passed (379 files)
• plist and shell syntax validation: passed
• full safe suite: 4 unrelated existing visual snapshot mismatches (three Home Row timing snapshots and Repair settings); 4,278 XCTest cases ran, plus 820 Swift Testing cases passed
• review gate: remote review selected; merge is gated on GitHub claude-review

Release context

Local acceptance of the notarized 1.0.1 build 9 found that the helper successfully signaled Kanata, but launchd immediately respawned the KeepAlive job. This PR is the build-10 release gate before publishing 1.0.1 to the public Sparkle feed.

[claude]

Reviewed the diff. The disable-before-SIGTERM / enable-before-kickstart ordering looks correct and matches the KeepAlive-respawn race described in the bug doc.

One coverage gap worth closing before merge:

• HelperServiceKeepAliveLifecycleLintTests only verifies two orderings (disable-before-kill in stopKanataService, enable-before-kickstart in start/restart). It does not cover the third, subtlest behavior introduced here: stopKanataService re-enabling the service when the SIGTERM launchctl kill itself fails (Sources/KeyPathHelper/HelperService.swift, the guard result.status == 0 else { ...enable...; throw } block). That's the one path that restores prior state on partial failure, and it's exactly the kind of logic a regression could silently break without any test catching it.

Minor/non-blocking: the lint tests locate function bodies via a textual "\n func " search from #filePath. That's a reasonable low-cost check for ordering within a single file, but it only verifies source text shape, not runtime behavior (e.g. it wouldn't catch the enable/disable calls being reordered behind a conditional or moved outside the executed path). Given Self.run is easily injectable, a small behavioral test double for HelperService that asserts actual call order/state would be more robust long-term, but the lint test is a fine stopgap for now.

Everything else (version bumps, Info.plist version sync, smoke-script CONFIG_DIR hardening, assert_matches addition) looks correct and consistent with the stated fix.

[claude]

Reviewed the diff. One correctness concern in HelperService.swift, plus a test-fragility note.

1. Silently swallowed rollback failure in stopKanataService (Sources/KeyPathHelper/HelperService.swift)

When the SIGTERM kickstart/kill call fails after disable succeeded, the code does a best-effort re-enable:

guard result.status == 0 else {
    _ = Self.run("/bin/launchctl", ["enable", Self.kanataServiceTarget], timeout: 15)
    throw HelperError.operationFailed(...)
}

The re-enable result is discarded (_ = ...). If that call also fails, the function still only reports the original "failed to stop" error — the caller has no way to know the service was left in a disabled state. Since com.keypath.kanata is the keyboard-remapping daemon, this failure mode means input capture stays down with no diagnostic pointing at the real cause (job disabled, not just "stop failed"). Worth at least logging the rollback failure distinctly, or folding it into the thrown error message, so this doesn't manifest as a silent, hard-to-diagnose "keyboard stopped working" report.

2. Lint test checks source text, not behavior (Tests/KeyPathTests/Lint/HelperServiceKeepAliveLifecycleLintTests.swift)

The new test extracts function bodies via string search and asserts that literal argument-array strings (e.g. ["disable", Self.kanataServiceTarget]) appear in the expected order. This only guards against a textual reordering of these exact literals — any equivalent refactor (e.g., extracting a runLaunchctl(action:) helper, or building the argument array dynamically) would silently defeat the check while preserving or breaking the real invariant either way. It's a reasonable stopgap, but it doesn't actually exercise HelperService behavior, so consider it a lint/reminder rather than real regression coverage for the KeepAlive fix.

Everything else (version bumps, qa-script assert_matches addition, hardcoding CONFIG_DIR) looks consistent and low-risk.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 12ae70fbb0

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

Clear the persistent disable in every start path

launchctl disable records a persistent override for this label, but only the helper's CLI start/restart methods issue the corresponding enable. After keypath service stop, ordinary app paths—including Settings, wizard, and auto-start—use ServiceLifecycleCoordinator.startKanata() (ServiceLifecycleCoordinator.swift:193-231), whose SMAppService register/kickstart flow never enables the label, so those paths cannot restart Kanata until the user runs a CLI start/restart. Route this transition through the canonical lifecycle coordinator or ensure every start path clears the override.

AGENTS.md reference: AGENTS.md:L331-L332

Useful? React with 👍 / 👎.