fix: bound osascript wait with the shared process timeout - #197
Conversation
Extract ProcessTimeout from the ffmpeg conversion wait and apply the same monotonic deadline to MessageSender and ReactCommand osascript paths so hung Messages automation cannot block indefinitely. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
|
Codex review: needs maintainer review before merge. Reviewed August 2, 2026, 1:08 AM ET / 05:08 UTC. ClawSweeper reviewWhat this changesThe PR extracts the existing attachment-conversion process timeout into a shared helper and applies bounded waits to AppleScript sends, reactions, and Messages launcher helper processes. Merge readinessThis PR fixes current-main unbounded waits in AppleScript send and reaction paths, and its latest commit corrects the earlier 60-second regression by using the existing 150-second send-style deadline. No discrete patch defect remains; maintainer acceptance is needed because direct CLI fallback operations that previously waited indefinitely will now fail after 150 seconds. Priority: P1 Review scores
Verification
How this fits togetherThe iMessage CLI starts AppleScript and small system helper processes to send messages, add reactions, convert attachments, and prepare Messages.app. Their exit status feeds back into CLI or bridge responses, so an indefinitely hung child can otherwise block the caller. flowchart LR
Request[CLI or bridge request] --> Operation[Send, react, convert, or launch]
Operation --> Child[AppleScript or helper process]
Child --> BoundedWait[Deadline and termination]
BoundedWait --> Outcome[Exit status or timeout error]
Outcome --> Response[CLI or bridge response]
Decision needed
Why: The patch is mechanically sound and matches the bridge’s documented send-action timeout, but only maintainers can accept the user-visible change from an indefinite stall to a bounded failure for direct CLI fallback callers. Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Merge the shared bounded-wait helper with the 150-second send/reaction deadline retained, preserving the existing conversion cleanup semantics and surfacing a clear timeout error instead of allowing a caller to stall forever. Do we have a high-confidence way to reproduce the issue? Yes. Current main directly calls unbounded Is this the best way to solve the issue? Yes. Reusing the established attachment-conversion deadline and termination pattern is the narrowest maintainable repair, and the latest revision correctly retains the existing 150-second send-style timeout for AppleScript callers. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against e22dfad8e54e. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (30 earlier review cycles; latest 8 shown)
|
Import Darwin or Glibc conditionally like AttachmentResolver so linux-read-core does not fail on unconditional import Darwin. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
|
@clawsweeper re-review Updated Real behavior proof with exact-head live evidence (not mock-only unit assertions only). |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Remove Unreleased CHANGELOG entry (release-owned). Add a ProcessTimeout test that launches real /usr/bin/osascript with delay 30 and reaps it under a short bound (same launch shape as MessageSender/ReactCommand). Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
|
@clawsweeper re-review
|
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Use ProcessTimeout for short helper processes so hung killall/csrutil cannot stall launcher setup (same policy as osascript/ffmpeg). Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
|
@clawsweeper re-review Also bound MessagesLauncher killall/csrutil via ProcessTimeout (15s helper timeout) plus processTimeoutAllowsCsrutilStatus live success proof. |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Fallback send and reaction automation must match IMsgBridgeProtocol defaultSendResponseTimeout rather than the 60s helper default. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
|
@clawsweeper re-review |
|
Addressed the two P1 timeout findings: osascript waits in MessageSender fallback send and ReactCommand now use |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Independent maintainer verification on exact head Proof on macOS arm64:
GitHub CI is green on macOS and Linux, and the PR is currently cleanly mergeable. |
Co-authored-by: Sebastien Tardif <sebtardif@ncf.ca>
|
ClawSweeper status: review started. I am starting a fresh review of this pull request: fix: bound osascript wait with the shared process timeout This is item 1/1 in the current shard. Shard 0/1. This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking. Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted. |
What Problem This Solves
MessageSender.runOsascript(NSAppleScript authorization fallback) andReactCommand.runAppleScriptstart/usr/bin/osascriptand block on unboundedprocess.waitUntilExit(). A hung Messages automation stalls send/react CLI or RPC work indefinitely. Attachment conversion already fixed the same hang class for ffmpeg in #176; osascript was left unbounded.Evidence
Patch
ProcessTimeout(monotonic deadline, default 60s, SIGTERM then SIGKILL process/group).Live osascript route shape (head
c0243a6)Production launch shape:
/usr/bin/osascript -l AppleScript -with AppleScript on stdin (same as MessageSender/ReactCommand). Source isdelay 30; helper timeout 0.6s:status=15is SIGTERM from the shared terminate path. Full suite remains green on macOS; linux-read-core green after Glibc import fix.Real behavior proof
Behavior or issue addressed: Unbounded
waitUntilExiton osascript send-fallback and react automation; now deadline-bounded.Real environment tested: macOS arm64, real
/usr/bin/osascript, branchfix/osascript-wait-timeoutatc0243a6.Exact steps or command run after this patch:
Evidence after fix: real osascript child running
delay 30reaped in under 1s with timedOut=true; helper unit tests still pass.Observed result after fix: same ProcessTimeout used by MessageSender/ReactCommand terminates hung osascript instead of blocking ~30s.
What was not tested: Full Messages.app Accessibility deadlock (requires intentional UI hang); NSAppleScript success path without osascript fallback.
MessagesLauncher helpers
killall Messagesandcsrutil statusalso used unboundedwaitUntilExit. They now useProcessTimeoutwith a 15s helper bound. Live success path:Test plan
swift test --filter processTimeoutReapsHungOsascript(real osascript)swift test --filter processTimeoutKillsHungProcessmake test/make lint