fix: serialize Messages bridge launches across processes

[goutamadwant]

Summary

• securely provision the lock parent and serialize the complete Messages bridge launch lifecycle with a per-container POSIX file lock
• re-check readiness after acquiring the lock so waiting launchers reuse the instance started by the owner
• reject symlinked, shared, non-regular, or multiply linked lock files and keep the descriptor close-on-exec
• cover independent launcher instances and lock release after launch failure

Closes #272.

Behavior proof

A compiled coordinator harness drove two independent launch owners against the same readiness state. With process-local coordination only it recorded two launch operations; with the shared lock it recorded one (unlocked=2, locked=1). The regression test uses the same independent-launcher composition from an absent container directory, and the failure-path test verifies that a throwing owner releases the lock for the next launcher.

The lock is held across readiness re-check, cleanup, spawn, and readiness wait. Because the descriptor uses O_CLOEXEC, the launched Messages process does not inherit ownership; process exit also releases the kernel lock.

Validation

• swiftc -frontend -parse Sources/IMsgCore/BridgeLaunchCoordinator.swift Sources/IMsgCore/MessagesLauncher.swift Tests/IMsgCoreTests/IMsgBridgeClientQueueTests.swift
• node --test scripts/build-docs-site.test.mjs
• make test-helper
• git diff --check

make test could not start in the local Command Line Tools-only environment because XCTest was unavailable. Hosted CI remains the full Swift-suite validation. No live Messages relaunch was performed, so the reporter's native duplicate-process evidence remains the live-system proof.

Disclosure: AI was used to understand the codebase and review the fix.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[clawsweeper]

Codex review: needs changes before merge. Reviewed September 6, 2026, 12:42 PM ET / 16:42 UTC.

ClawSweeper review

What this changes

The PR adds a secure per-container file lock so concurrent imsg processes share one Messages bridge launch, with tests for directory creation and lock release after failure.

Merge readiness

⛔ Needs changes before merge - 1 item remains

The fix remains necessary on current main. The reporter’s native before/after evidence clears the previous proof blocker, and no introduced correctness defect was found.

Priority: P1
Reviewed head: 78b3daebf7d2210b1d70350bf73beb1d01518707

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused repair with convincing native comparison evidence and no blocking code findings; the previously requested lint result remains outstanding.
Proof confidence	🦞 diamond lobster (5/6)	Sufficient (terminal): The reporter exercised MessagesLauncher through two independent imsg launch processes on macOS 26 with the real injected helper at the pinned head, reducing duplicate instances from 7/15 to 0/15 trials and resolving the previous native-proof gap.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The reporter exercised MessagesLauncher through two independent imsg launch processes on macOS 26 with the real injected helper at the pinned head, reducing duplicate instances from 7/15 to 0/15 trials and resolving the previous native-proof gap.
Evidence reviewed	6 items	Current main still lacks cross-process exclusion: The coordinator at the fetched main revision uses only a DispatchQueue. The local v0.15.1 tag points to that same revision, so the requested locking is absent from both reviewed main and the latest supplied release. Lock encloses the production lifecycle: Both synchronous and asynchronous launch entrypoints use the coordinator. The introduced lock encloses readiness checking and the existing cleanup, spawn, and readiness wait; defer releases the lock on errors. The lock descriptor is close-on-exec and checked for ownership, restrictive permissions, regular-file type, and a single link. Native proof resolves the earlier evidence gap: #274 (comment) reports two independent imsg launch processes on macOS 26 Apple Silicon with real Messages.app and its injected helper: released v0.15.1 produced duplicates in 7/15 trials, while pinned PR head 78b3dae produced duplicates in 0/15. The comment includes the invocation, surviving-process observations, and owner-only lock permissions. This exercises the changed launch boundary rather than only the coordinator harness.
Findings	None	None.
Security	None	None.

How this fits together

imsg’s launcher starts Messages.app with an injected helper that handles advanced messaging commands. CLI and bridge-client launch requests pass through the coordinator before cleanup, process creation, and readiness checks.

flowchart TD
  A[Concurrent imsg processes] --> B[Launch coordinator]
  B --> C[Acquire container file lock]
  C --> D{Bridge ready?}
  D -->|Yes| E[Reuse running Messages]
  D -->|No| F[Clean up and launch Messages]
  F --> G[Wait for helper readiness]
  G --> H[Release lock]
  E --> H

Loading

Before merge

• Complete next step (P2) - Attach the previously requested make lint output for this head; the native proof and base-compared make test results are sufficient.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Native concurrent-launch trials	Duplicates: 7/15 on release; 0/15 on PR head	The reporter exercised independent CLI processes through the real Messages launch lifecycle.
Production and test delta	Production +86/-6; tests +66/-2	Production growth implements secure cross-process coordination, with focused first-use and failure-release coverage.

Technical review

Best possible solution:

Keep launch serialization in the shared coordinator so all launch entrypoints reuse one ready bridge, while tracking readiness-timeout behavior separately.

Do we have a high-confidence way to reproduce the issue?

Yes. The reporter reproduced duplicate native Messages instances in 7/15 concurrent-launch trials on v0.15.1, which matches the fetched main revision; this read-only review did not rerun them.

Is this the best way to solve the issue?

Yes. Locking the existing shared launch boundary and rechecking readiness is a focused repair that avoids competing launch implementations.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 646ea7af9616.

Labels

Label changes:

• add proof: sufficient: Contributor real behavior proof is sufficient. The reporter exercised MessagesLauncher through two independent imsg launch processes on macOS 26 with the real injected helper at the pinned head, reducing duplicate instances from 7/15 to 0/15 trials and resolving the previous native-proof gap.
• add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The reporter exercised MessagesLauncher through two independent imsg launch processes on macOS 26 with the real injected helper at the pinned head, reducing duplicate instances from 7/15 to 0/15 trials and resolving the previous native-proof gap.
• remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.
• remove rating: 🦪 silver shellfish: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.

Label justifications:

• P1: The reported launch race creates competing bridge instances and causes real outbound messaging failures.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The reporter exercised MessagesLauncher through two independent imsg launch processes on macOS 26 with the real injected helper at the pinned head, reducing duplicate instances from 7/15 to 0/15 trials and resolving the previous native-proof gap.
• proof: sufficient: Contributor real behavior proof is sufficient. The reporter exercised MessagesLauncher through two independent imsg launch processes on macOS 26 with the real injected helper at the pinned head, reducing duplicate instances from 7/15 to 0/15 trials and resolving the previous native-proof gap.

Evidence

What I checked:

• Current main still lacks cross-process exclusion: The coordinator at the fetched main revision uses only a DispatchQueue. The local v0.15.1 tag points to that same revision, so the requested locking is absent from both reviewed main and the latest supplied release. (Sources/IMsgCore/BridgeLaunchCoordinator.swift:3, 646ea7af9616)
• Lock encloses the production lifecycle: Both synchronous and asynchronous launch entrypoints use the coordinator. The introduced lock encloses readiness checking and the existing cleanup, spawn, and readiness wait; defer releases the lock on errors. The lock descriptor is close-on-exec and checked for ownership, restrictive permissions, regular-file type, and a single link. (Sources/IMsgCore/BridgeLaunchCoordinator.swift:54, 78b3daebf7d2)
• Native proof resolves the earlier evidence gap: fix: serialize Messages bridge launches across processes #274 (comment) reports two independent imsg launch processes on macOS 26 Apple Silicon with real Messages.app and its injected helper: released v0.15.1 produced duplicates in 7/15 trials, while pinned PR head 78b3dae produced duplicates in 0/15. The comment includes the invocation, surviving-process observations, and owner-only lock permissions. This exercises the changed launch boundary rather than only the coordinator harness. (78b3daebf7d2)
• Prior finding and validation follow-up: The former missing-parent finding is resolved by creating the lock directory before opening the file. Added fixtures cover an absent container and release after a throwing launch. Native proof covers an existing container; these complementary checks adequately address the prior first-use/upgrade request without requiring another native setup matrix. The reporter supplied make test results: 746 tests with three failures reproduced identically on the base, now tracked at tests: three RPCServerTests fail on any host with a live injected bridge (green only in CI) #275. No make lint result appears in the supplied body or discussion. (Tests/IMsgCoreTests/IMsgBridgeClientQueueTests.swift:359, 78b3daebf7d2)
• Repository review guidance: Read the complete root AGENTS.md and checked Sources, Tests, and .agents for scoped policies and maintainer notes; none additional applied. The root guidance calls for focused changes, verification examples, and make lint/make test output. (AGENTS.md:23, 78b3daebf7d2)
• Area routing history: Available main-branch history associates Peter Steinberger with coordinator and delivery work, and Omar Shahine with the bridge port and IPC path hardening. Raw commit records were inspected, but missing historical blobs prevented complete blame and parent-line comparisons; these are routing candidates, not verified introduction attributions. (Sources/IMsgCore/MessagesLauncher.swift, 646ea7af9616)

Likely related people:

• Peter Steinberger: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• omarshahine: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Attach make lint output for the reviewed head to complete the repository’s requested validation record.

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (2 earlier review cycles)

• reviewed 2026-09-06T09:16:47.526Z sha 601ff58 :: needs real behavior proof before merge. :: [P2] Provision the lock parent before opening the launch lock
• reviewed 2026-09-06T12:06:13.541Z sha 78b3dae :: needs real behavior proof before merge. :: none

[omarshahine]

Reporter of #272 here. I have the machine that produced the original duplicate-instance evidence, so I ran the native after-fix proof this review is asking for, on PR head 78b3dae.

Environment: macOS 26 (Apple Silicon), SIP disabled, full Xcode 26 with XCTest, real Messages.app and a real injected helper. Paths redacted below.

Native concurrent-launch proof

Harness: two genuinely independent imsg launch processes (not in-process coordinators), started 1.2 s apart so the second killall lands while the first instance is still spawning. Every trial hard-kills all Messages instances first, then counts survivors and their pids.

"$BIN" launch & A=$!
sleep 1.2
"$BIN" launch & B=$!
wait $A; wait $B
pgrep -f "Messages.app/Contents/MacOS/Messages"

Control — released 0.15.1 (no cross-process lock), 15 trials:

trial  1: 2 instance(s)  pids=[25623 25633]   <-- DUPLICATE
trial  3: 2 instance(s)  pids=[25966 25967]   <-- DUPLICATE
trial  5: 2 instance(s)  pids=[26211 26212]   <-- DUPLICATE
trial  4: 2 instance(s)  pids=[28007 28008]   <-- DUPLICATE
trial  6: 2 instance(s)  pids=[28294 28295]   <-- DUPLICATE
trial  7: 2 instance(s)  pids=[28496 28506]   <-- DUPLICATE
trial  9: 2 instance(s)  pids=[28785 28786]   <-- DUPLICATE
RESULT: 7/15 trials produced duplicate injected Messages.app instances

Treatment — this PR at 78b3dae, 15 trials, identical harness:

trial  1: 1 instance(s)
trial  2: 1 instance(s)
...
trial 15: 1 instance(s)
RESULT: 0/15 trials produced duplicate instances

7/15 → 0/15. These are real killall → cleanup → spawn → readiness cycles against the live helper, which is the gap the review flagged.

The lock is created as specified, owner-only regular file:

-rw-------  1 <user>  staff  0  .../Data/.imsg-launch.lock

macOS test suite

make test on the PR branch: 746 tests, 3 failing — all in RPCServerTests:

• rpcSendResolvesChatID()
• rpcSendReportsMisroutedChatGhost()
• rpcSendReturnsSentMessageIdentifiersWhenResolved()

These are pre-existing and not caused by this PR. I ran the same three against the base commit 646ea7a (chore: prepare 0.15.1 release) in a separate worktree and got byte-identical failures. They look environment-dependent rather than flaky — on a host with a live injected bridge the send path resolves to bridge_v2, while the tests assert applescript:

Expectation failed: (data?["transport"] as? String → "bridge_v2") == "applescript"
Expectation failed: (data?["operation"] as? String → "send-message") == "send"

So they appear to pass in CI (no live bridge) and fail on any developer machine that actually has the helper injected. Worth a separate issue; it is unrelated to this change.

make build succeeds and produces signed universal artifacts (arm64 x86_64, dylib arm64e arm64 x86_64). Only pre-existing deprecation warnings in IMsgInjected.m.

Why this matters

The duplicate instances are not cosmetic. Both copies claim off the same v2 queue, so a request claimed by the instance the client is not reading from is consumed and never answered — surfacing as The published bridge request vanished before a response arrived. On my host that silently dropped 49 outbound replies over a week before I traced it. This is the right fix.

Also worth noting for #273: several control trials show imsg launch reporting a readiness timeout while the instance came up healthy, which is one of the ways a supervisor is tempted into the second launch that triggers this race.

[omarshahine]

@clawsweeper re-review

[clawsweeper]

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

• State: Complete
• Detail: The durable review result and its route handoff completed.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/34046368953
• Updated: 2026-09-06T16:43:56.287Z

[omarshahine]

make lint for head 78b3dae, same host as the native proof above (macOS 26, Apple Silicon, Xcode 26, SwiftLint 0.65.1).

It passes — exit code 0:

$ make lint
swift format lint --recursive Sources Tests TestsLinux
swiftlint
...
Done linting! Found 17 violations, 0 serious in 249 files.

swift format lint produced no output. SwiftLint reports 0 serious violations, so the target does not fail.

Base comparison

Since I had both worktrees, I ran the same target on base 646ea7a and diffed the violation sets rather than just reporting the total.

Base: 15 violations. This PR: 17. The two additional ones are both type_body_length, and both cross the threshold for the first time here:

Sources/IMsgCore/MessagesLauncher.swift
  Type Body Length: Class body should span 300 lines or less; currently spans 301  (NEW)

Tests/IMsgCoreTests/IMsgBridgeClientQueueTests.swift
  Type Body Length: Struct body should span 300 lines or less; currently spans 339 (NEW)

For completeness, file_length on those same two files is not new — both already violated it on base and this PR grows them:

File	Base	This PR
MessagesLauncher.swift	501	505
IMsgBridgeClientQueueTests.swift	501	565

So the honest summary is: lint passes, and the change adds two type_body_length warnings to files that were already over the file-length limit. MessagesLauncher.swift is one line past the class-body threshold (301/300).

Flagging it only because make lint exits 0 either way, so CI will not surface it. Entirely a maintainer judgement call — the launcher file was already at the limit before this change, and the fix itself is small and well-scoped.

[steipete]

Verified head 78b3daebf7d2210b1d70350bf73beb1d01518707.

The lock covers readiness recheck, queue cleanup, spawn, and readiness wait in both launch entrypoints. It is owner-only, rejects unsafe lock files, closes on exec, and releases on exceptions and owner exit.

Local proof compiled the complete production coordinator and SecurePath into a separate-process harness: main performed two launch operations in 10/10 trials; this head performed one in 10/10. Killing the owner released the lock and allowed the waiter to proceed. All 18 queue/readiness tests pass, including independent launchers and recovery after a throwing launch. make lint passes with 17 size warnings and zero serious violations; the two additional size warnings were already documented by the reporter.

Independent branch autoreview against origin/main is clean at P0–P2. Built and signed the actual CLI with its matching Developer ID; version, launch help, and read-only status run successfully. This Mac has SIP enabled, so the native overlapping-Messages proof remains the reporter's exact-head result: 7/15 duplicate instances on v0.15.1 versus 0/15 here, documented at #274 (comment).

Approved the reviewed fork workflow; full macOS/Linux CI is green on this exact head: https://github.com/openclaw/imsg/actions/runs/34031928047.

Recommended landing order includes #279 first (independent CLI test output-capture repair) and #277 (host-independent RPC fixtures). No changelog edit is included here; release notes are consolidated in the final notes PR. Fixes #272.