fix: make bridge launch readiness timeout configurable - #276
Conversation
…ow starts as failures `imsg launch` waited a hardcoded 15s for Messages.app to publish the bridge-ready lock file, then threw. On a cold start after a crash, or a loaded machine, that window is too short: the instance comes up healthy moments later but the command has already exited non-zero with an error naming SIP and permissions, neither of which was the cause. That misreport has a real consequence. A supervisor that relaunches on a non-zero exit starts a second Messages.app while the first is still coming up, and two injected instances then compete for the same bridge queue. - resolve the timeout from IMSG_LAUNCH_READY_TIMEOUT, defaulting to 15s and clamped to 600s so a typo cannot hang a launch - re-check readiness once after the deadline, so a launch that actually succeeded in the final polling gap is not reported as a failure - carry the timeout in the error and point at `imsg status` and the override before blaming SIP or permissions Measured on macOS 26 (Apple Silicon, SIP disabled): a cold start after a hard kill took 18.05s to report ready, past the previous hardcoded limit.
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
clawsweeper
Bot
added
P2
|
Codex review: needs maintainer review before merge. Reviewed September 7, 2026, 4:25 AM ET / 08:25 UTC. ClawSweeper reviewWhat this changesAdds a bounded environment setting for Messages bridge startup waits, a final readiness check, clearer timeout guidance, regression tests, and documentation. Merge readiness✅ Ready for maintainer review This remains useful: current main and v0.15.1 retain the fixed timeout. The prior public-error compatibility defect is resolved, and no blocking defect remains in the reviewed patch. Priority: P2 Review scores
Verification
How this fits togetherThe Messages launcher starts the injected helper used by advanced messaging commands. CLI and library callers wait for its readiness file before proceeding or receiving a timeout error. flowchart TD
A[CLI and library callers] --> B[Messages launcher]
C[Environment timeout setting] --> D[Validate and bound wait]
B --> E[Start Messages helper]
D --> F[Poll readiness file]
E --> F
F --> G[Continue when ready]
F --> H[Timeout with recovery guidance]
Before mergeNone. Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Retain the bounded startup override and readiness-based success contract while keeping cross-process launch serialization in its separate fix. Do we have a high-confidence way to reproduce the issue? Yes: current main deterministically times out if the readiness file appears after its fixed 15-second wait; the linked report supplies a native example. This read-only review did not execute a reproduction. Is this the best way to solve the issue? Yes: resolving the budget in the shared launcher covers CLI and library callers while preserving defaults and the public error API; the final documentation commit supports the chosen configuration contract. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 646ea7af9616. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (3 earlier review cycles)
|
IMsgCore is an exported library, so external callers construct and match `MessagesLauncherError.socketTimeout` directly. Adding a required associated value made those previously valid expressions fail to compile on upgrade; updating this repository's own caller only migrated the in-repo use. Swift cannot rescue this with a default on the payload — `case socketTimeout(seconds: TimeInterval = 15)` turns `.socketTimeout` into a function reference rather than a value, so the old spelling still breaks. Restore the payload-free case and resolve the timeout inside `description` instead. `waitForReady` is only ever called with LaunchReadinessTimeout.resolve(), so the reported number matches the wait that actually elapsed, and the public construction contract is unchanged. Adds a source-compatibility regression fixture covering the shapes an external consumer relies on: member-syntax and fully-qualified construction, use in a collection literal, switch matching, and throw/catch. Reintroducing the associated value fails to compile against that fixture, which is the point.
|
Good catch — that was a real source-compatibility break and I had only considered in-repo callers. Fixed in What changed
I first checked whether a default could preserve the old spelling. It cannot — Swift turns the case into a function reference: public enum E: Error { case socketTimeout(seconds: TimeInterval = 15) }
let a: E = .socketTimeout
// error: member 'socketTimeout(seconds:)' is a function that produces expected
// type 'E'; did you mean to call it?So the associated value had to go. The diagnostic is kept by resolving the timeout inside case .socketTimeout:
// Resolved rather than stored: the case stays payload-free so external
// `.socketTimeout` construction keeps compiling, and `waitForReady` is
// only ever called with this same resolved value.
let seconds = LaunchReadinessTimeout.resolve()
Source-compatibility regression fixtureAdded I verified it fails closed rather than assuming it does. Reintroducing the associated value (with the source tree otherwise made self-consistent, so only the fixture is left failing) breaks the build: Gates at this head
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
clawsweeper
Bot
added
rating: 🐚 platinum hermit
steipete
changed the title
|
Verification for
The new guide section documents Exact-head macOS/Linux CI: https://github.com/openclaw/imsg/actions/runs/34100099441. Fixes #273. Changelog notes are consolidated in the final notes PR. |
clawsweeper
Bot
added
rating: 🐚 platinum hermit
2 participants
Messages can take longer than 15 seconds to become bridge-ready after a cold start. The existing timeout then reports a launch error with misleading SIP/permission guidance, even when the process becomes ready shortly afterward.
Allow CLI, library, and supervisor callers to set
IMSG_LAUNCH_READY_TIMEOUTin seconds. Keep the 15-second default, bound positive overrides at 600 seconds, and fall back to the default for invalid values. Recheck readiness at the deadline and explain that Messages may still be starting, with animsg statuscheck before another launch. The public payload-freeMessagesLauncherError.socketTimeoutcase and readiness-based success contract are preserved.The advanced IMCore guide documents the setting, bounds, defaults, and supervisor use. Tests cover timeout resolution, diagnostics, and external-style construction/matching of the public error. Native evidence from the contributor's Mac shows a successful cold start in 18.054 seconds with a 60-second budget and the new diagnostic with a one-second budget. The maintainer proof comment records the final head's validation and remaining host limitations.
Fixes #273. Cross-process launch serialization remains independently handled by #274. Changelog notes are consolidated in the final release-notes PR.