[plonkus]

Goal: Install Update in the Mac app finishes or fails; it never sits on "Updating…" forever.

Outside users today: Mac app users on builds that have Install Update, a handful of hosts.

Minimal solution for that user count: Spawn latch update with an empty signal mask; that alone ends the hang. Beyond it, only what stops the next unknown from looking the same: a deadline on the app's wait, Retry stopping the run it gave up on, a failure the app didn't see shown once at the next launch, and timeouts on the two system tools the update waits for.

Complexity added: One extra file the app keeps in its own app-update/ folder (the run's pid and start time), a 20-minute deadline, a protocol method for a leftover failure, and a signal-mask reset at the top of latch's main. No new process, service or state outside app-update/.

Size: Expected net LOC: about +110 in code and +160 in tests. Actual: +269 net (+295/−26), 157 of the added lines tests.

Why

An in-app update could hang forever, with the toolbar pill on "Updating…" until the window was quit. The Mac app started latch update with posix_spawn from a Swift concurrency thread, which blocks most signals, and latch inherited that mask. With SIGCHLD blocked, tokio never reaps the first program the update runs: it stays <defunct>, latch update waits for it forever, and with SIGTERM blocked too only kill -9 stops it. Other latch commands the app runs are unaffected, since Foundation's Process resets the mask.

What changed

• The fix. The app now spawns the update with an empty signal mask (POSIX_SPAWN_SETSIGMASK).
• latch clears its own mask at startup, in main before any thread starts, so a blocked mask from any starter can't cause this again.
• A stuck update fails instead of spinning. The app waits up to 20 minutes, then shows the update as failed ("Update failed · Retry"; Details: "The update was still running after 20 minutes. Retry stops it and starts again."). The update's pid and start time are recorded in app-update/pid, and the next attempt kills that run's process group first. The start time is checked, so an unrelated process that later got the same pid is never killed.
• A failure the app didn't see is shown at the next launch. If the update failed after its window was quit, latch's error is read from the app-update/err.log it left, shown once, and the file is removed.
• codesign and spctl time out after 120 seconds each, with timed_out ("codesign did not finish in 120 seconds"). Before, a timeout would have been reported as a bad signature.

What it doesn't do

This can't fix builds that are already installed: their app still spawns the update with signals blocked, so an in-app update from those builds can still hang. They update by running install.sh again. Once on this build, later in-app updates work.

Tests

• Swift: the spawned update blocks no signals (fails without the fix); a hung update fails at the deadline and the next run kills it and its child; a leftover failure is shown once.
• CLI: latch started with SIGCHLD and SIGTERM blocked still stops on SIGTERM (fails without the fix); tool() times out with timed_out.
• The full macOS gate (just test) passes.

🤖 Generated with Claude Code

https://claude.ai/code/session_01RFixJTbcqzMj4yVHDwaQCw

[srosro]

📋 First review of this PR — reviewed cbb1059. 🧪 Tests not run. ✅ Prior-art (KID) checked. 🔍 8 sibling repos not searched (8 allowlist; names withheld on a public repo). 📐 Complexity budget unchecked — the PR body is missing Goal, Outside users today, Minimal solution for that user count, Complexity added, Size: Expected net LOC; fill in those PR-template fields so the next review can check the budget.

It appears @plonkus is working towards letting Mac users complete Install Update without an endless “Updating…” state, see failures even after reopening the app, and retry a stuck attempt by clearing inherited signal masks in LatchProcess.swift and main.rs, bounding update and signature-check waits, stopping the previous update on retry, and surfacing leftover errors at launch — reviewing against that goal.

Overview — The PR fixes inherited signal blocking, bounds the app’s update wait, and recovers structured failures after relaunch. It extends the existing detached-update path without introducing a separate lifecycle manager, which fits this repo’s pre-PMF operating point: one engineer and a handful of internal hosts.

Strengths — Retry cleanup checks both PID and process start time before signaling the prior update’s process group. The shared continuation is consumed once, so a late process exit cannot complete an already timed-out call again. Signature-check deadlines preserve timed_out while retaining the requirement that verification succeeds before installation. Live and recovered failures share the existing error decoder and display state.

Probes

None. All seven specialist assessments and the cross-angle check of spawning, retry cleanup, installer handoff, and relaunch handling produced no actionable findings.

Security — None.

Test coverage — Focused tests cover inherited signals, timeout and retry with child-process cleanup, mismatched process records, one-time failure recovery, and signature-tool deadlines; the staged just test attempt stopped before checks ran because cargo-nextest was missing (exit 127), while the author reports a full macOS gate pass that this read-only review did not independently verify.

For AI authors — (Codex, Claude Code, Cursor, etc. reading this PR): The Probes above are load-bearing. Treat each [open] probe as a hard requirement — answer it directly in your reply or revisit the structural decision. Do NOT silently absorb a probe by adding more code; that path inverts the cost stance the probe is encoding. This repo’s operating point (see its REVIEW.md) prefers cutting LOC over adding it; an [open] [simplification] probe whose answer is “no, this complexity isn’t needed” should land as a deletion in your next push, not a new defensive guard.

---

How to use: auto-reviews every new PR and re-reviews after a period of inactivity. Trigger an incremental re-review with /srosro-update-review, or a whole-PR re-review with /srosro-review.

For humans only: push-access collaborators can post:

• /srosro-approve — APPROVE the PR.
• /srosro-props [from: <specialist>] — give props to a specialist's contribution.
• /srosro-critique [from: <specialist>] — flag a specialist's contribution as a misread.
• /srosro-memorize <feedback> — teach a calibration lesson (learn-from-replies updates COMMENT_REVIEW_MISTAKES.md from your body, sentiment-aware via LLM).

Props: /srosro-props [from: shape] caught a real layering bug we'd have shipped.
Critique: /srosro-critique [from: architecture-refined] DRY suggestion misread distinct seams.
Calibration: /srosro-memorize the architecture-refined DRY finding was a misread; those helpers serve different contracts.

AI agents must not use /srosro-memorize, /srosro-props, or /srosro-critique — those signals tune shared global state.

Generated by sam's ai review bot.

[srosro]

Approving per automated review above.

[plonkus]

/srosro-update-review

[srosro]

📋 Re-review of changes from cbb1059 to 04ea25d (git diff cbb1059..04ea25d). 🧪 Tests not run. ✅ Prior-art (KID) checked. 🔍 8 sibling repos not searched (8 allowlist; names withheld on a public repo). ⚡ Small-diff lane (4 lines): skipped security, data-integrity, architecture-refined, tests, consumers — screened by aggregator. 📐 Complexity budget unchecked — the PR body is missing Goal, Outside users today, Minimal solution for that user count, Complexity added, Size: Expected net LOC; fill in those PR-template fields so the next review can check the budget.

It appears @plonkus is working towards ensuring Mac app users get a completed update or a retryable failure instead of remaining on “Updating…” forever. The staged diff supports that broader PR goal narrowly by moving the wait deadline in LatchProcess.swift from a Swift Task to DispatchQueue.global().asyncAfter, with the latest commit citing Xcode 26’s rejection of the Task implementation — reviewing against that goal.

Overview — This increment replaces only the deadline scheduler in the existing detached-update path. It preserves the production caller’s 20-minute wait and retry behavior without adding another lifecycle abstraction, fitting the repo’s pre-PMF operating point: one engineer and a handful of internal hosts. The prior review had no unresolved findings.

Strengths — The exit callback and deadline callback atomically take the same continuation before resuming it, preserving exactly-once completion when they race. The duration conversion retains fractional seconds, and LiveLatchClient.update() continues through the existing API.

Probes

None. The specialist conclusions, current caller, continuation handling, and additional screening of the changed scheduling path produced no actionable findings.

Security — None.

Test coverage — Existing DetachedUpdateTests covers normal completion, timeout, and retry stopping the previous process and its child; the staged just test attempt stopped before checks ran because cargo-nextest was missing (exit 127), a reviewer-environment limitation, while the author’s reported full macOS gate pass was not independently verified in this read-only review.

For AI authors — (Codex, Claude Code, Cursor, etc. reading this PR): The Probes above are load-bearing. Treat each [open] probe as a hard requirement — answer it directly in your reply or revisit the structural decision. Do NOT silently absorb a probe by adding more code; that path inverts the cost stance the probe is encoding. This repo's operating point (see its REVIEW.md) prefers cutting LOC over adding it; an [open] [simplification] probe whose answer is "no, this complexity isn't needed" should land as a deletion in your next push, not a new defensive guard.

---

How to use: auto-reviews every new PR and re-reviews after a period of inactivity. Trigger an incremental re-review with /srosro-update-review, or a whole-PR re-review with /srosro-review.

For humans only: push-access collaborators can post:

• /srosro-approve — APPROVE the PR.
• /srosro-props [from: <specialist>] — give props to a specialist's contribution.
• /srosro-critique [from: <specialist>] — flag a specialist's contribution as a misread.
• /srosro-memorize <feedback> — teach a calibration lesson (learn-from-replies updates COMMENT_REVIEW_MISTAKES.md from your body, sentiment-aware via LLM).

Props: /srosro-props [from: shape] caught a real layering bug we'd have shipped.
Critique: /srosro-critique [from: architecture-refined] DRY suggestion misread distinct seams.
Calibration: /srosro-memorize the architecture-refined DRY finding was a misread; those helpers serve different contracts.

AI agents must not use /srosro-memorize, /srosro-props, or /srosro-critique — those signals tune shared global state.

Generated by sam's ai review bot.

[srosro]

Approving per automated review above.