fix(trust-local): stop by unloading the job, not by signalling it - #26
Merged
Merged
Conversation
`aikey service stop trust-local` printed "stop succeeded" and did not stop
anything. Measured on macOS: launchd spawned a fresh pid ~10s after the
stop and the service was serving on :8801 again at t+20s, with launchd's
own `runs` counter going 40 -> 41.
Two individually-reasonable decisions combined badly: the LaunchAgent sets
unconditional KeepAlive (crash self-healing, which private deployments
need), and macOS stop was `launchctl kill TERM` — a signal that leaves the
job loaded. launchd dutifully brought it back, and stop() returned without
checking anything. Linux is unaffected (`systemctl --user stop` is
synchronous and systemd never restarts a deliberately stopped unit) and
Windows already verifies its process tree, so macOS was the only platform
where "stop" was a lie.
It survived this long because trust-local is a PyInstaller onefile that
needs ~20s to unpack before it binds. During that window the service is
genuinely unreachable, so any healthz-based check shorter than the boot
time sees a clean stop. Only launchd's `runs` counter gave it away.
Fix — stop now unloads the job:
- stop: `launchctl bootout gui/<uid>/<label>`. Nothing remains for launchd
to respawn. "Could not find specified service" counts as success:
stop is idempotent, the caller's post-condition already holds.
- start: bootstraps the job back in before `kickstart -k`, since a prior
stop may have removed it from the domain. An "already loaded"
error is the normal path and is ignored.
- The plist is NOT changed. Unconditional KeepAlive is exactly right for
every case that isn't a deliberate stop, and leaving it alone means
already-installed machines need no plist migration.
Rejected alternative, recorded so nobody retries it: setting
`KeepAlive={SuccessfulExit:false}` (matching the Linux unit's
Restart=on-failure) does NOT work. With it in force, SIGTERM still
respawned the job (`runs` 1 -> 2 -> 3). launchd tracks the PyInstaller
bootloader parent, and a signalled parent terminates BY SIGNAL rather than
exiting 0 — an unsuccessful exit either way. The policy cannot separate
"user stopped it" from "it crashed"; adopting it would only have cost
self-healing on clean-exit crashes. The reasoning is now a comment in
ai-degrade-detector's install_service.sh so it isn't "fixed" back.
Also adds post-stop verification on macOS, mirroring what windows_stop()
already does. It polls launchd's job state, NOT /healthz: healthz cannot
distinguish "stopped" from "restarting" — that is the exact artifact that
made a 15s observation window report a clean stop during diagnosis. The
12s budget clears launchd's ~10s respawn floor and deliberately does not
need to clear the 20s boot time, because job state flips at spawn.
Verified live against the unmodified plist:
A. stop stays stopped — 30s: job unloaded, no pid, no listener
B. start after bootout — bootstrap+kickstart works, healthz 200 in 27s
C. crash self-healing — kill -9 the child, launchd respawns, healthz 200
The new binary run against the old behaviour reports the failure and
exits 1 instead of claiming success.
Fence: parse_launchd_pid unit test built from real captured `launchctl
print` output. The dangerous direction is failing to parse a RUNNING job's
pid, which would put stop straight back to reporting false success.
Bugfix: workflow/CI/bugfix/20260814-trust-local-stop-does-not-stop-macos.md
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
配套 PR:aikeylabs/ai-degrade-detector(plist 注释,说明为什么 KeepAlive 必须保持
<true/>)· aikeylabs/workflow(bugfix 文档)问题
实测:launchd 约 10s 后拉起新 pid,t+20s 又在 8801 上服务了,launchd 自己的
runs计数 40 → 41。只影响 macOS。 Linux 不受影响(
systemctl --user stop同步,systemd 不重启被显式停掉的 unit);Windows 的windows_stop()本来就有 survivor 校验。macOS 是唯一「stop 是谎话」的平台。根因
两个各自都合理的决定凑在一起:
KeepAlive=<true/>—— 崩溃自愈,私有化部署需要。launchctl kill TERM—— 只发信号,不卸载 job。launchd 尽职地把它拉回来,而
stop()直接返回,不做任何校验。为什么活这么久没被发现
trust-local 是 PyInstaller onefile,冷启动 ~20s 解包才 bind 8801。重启后那 20s 里服务确实不可达,所以任何短于启动耗时的 healthz 检查都会看到一次"干净的 stop"。诊断时我第一次用 15s 窗口测,6 行
listener=none,差点结案 —— 拆穿它的是runs计数器。修法
停止语义由卸载 job 表达,而不是给进程发信号:
stop→launchctl bootout gui/<uid>/<label>,launchd 没东西可重启。「服务未加载」视为成功(stop 幂等,调用方要的后置条件已成立)。start→ 先bootstrap("已加载"错误忽略)再kickstart -k,因为 stop 现在会把 job 移出 domain。被否掉的方案(记录下来免得有人再试)
把 plist 改成
KeepAlive={SuccessfulExit:false}(对齐 Linux 的Restart=on-failure)不成立:改完之后 SIGTERM 照样触发重启(runs1 → 2 → 3)。原因:launchd 跟踪的是 PyInstaller bootloader 父进程,父进程收到信号是被信号终止而不是 exit 0 —— 在 launchd 眼里同样是 unsuccessful exit。这个策略在信号式 stop 面前根本区分不了「用户停的」和「它崩了」,采纳它只会白白削弱干净退出场景的自愈。理由已写进 ai-degrade-detector 的 plist 注释,防止后人"顺手改回去"。
(d) stop 后校验
对齐
windows_stop()已有的做法。关键点:轮询 launchd 的 job 状态,不探 /healthz —— healthz 分不清「已停止」和「正在重启」,这正是 15s 窗口报假绿的原因。12s 预算盖过实测 ~10s 的 launchd 重启节流下限,不需要盖过 20s 启动耗时,因为 job 状态在 spawn 瞬间就翻转。验收(plist 保持原样未改)
kill -9子进程 → launchd 重新拉起(68809→69013),t+25s 恢复,healthz=200 ✅新二进制打在旧行为上会如实报错并 exit 1,不再谎报成功。
围栏:
parse_launchd_pid单测,样本取自真机launchctl print输出(cargo test --lib trust_local_service,5 passed)。危险方向是运行中的 job 解析不出 pid —— 那会让 stop 直接退回谎报成功,所以单测重点锁这一侧,外加original pid = 999这类邻近键不得被误判。说明
origin/develop-v1.0.5切出(本地 checkout 停在fix/dgcheck-canonical-host-in-selfdesc,故走独立 worktree,未触碰)。🤖 Generated with Claude Code