Skip to content

fix(imessage): honor actions.reply=false when resolving replyToId#92920

Closed
1052326311 wants to merge 1 commit into
openclaw:mainfrom
1052326311:fix/imessage-actions-reply-92142
Closed

fix(imessage): honor actions.reply=false when resolving replyToId#92920
1052326311 wants to merge 1 commit into
openclaw:mainfrom
1052326311:fix/imessage-actions-reply-92142

Conversation

@1052326311

@1052326311 1052326311 commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Summary

When channels.imessage.accounts.<id>.actions.reply is explicitly false, the iMessage send path still forwarded replyToId to the imsg bridge, causing outbound delivery to fail with:

System Integrity Protection (SIP) is enabled. Refusing to inject
into Messages.app. Disable SIP before using imsg launch.

The bridge attempts a threaded/native reply via Messages.app private API, which requires SIP to be disabled. When actions.reply is false, the user has explicitly opted out of reply actions (making this a no-SIP-safe configuration), but the send layer ignored this setting.

Fix

Gate resolvedReplyToId on account.config.actions?.reply !== false. When actions.reply is false, replyToId is stripped before any delivery path, so the bridge sends a normal top-level message instead of a threaded reply.

Default behavior (actions.reply undefined or true) is preserved unchanged.

Files changed

  • extensions/imessage/src/send.ts — gate resolvedReplyToId on actions.reply
  • extensions/imessage/src/send.test.ts — 5 new regression tests

Real behavior proof

Behavior addressed: iMessage final reply delivery fails under SIP because replyToId is forwarded to the imsg bridge even when actions.reply is false.

Real environment tested: Local OpenClaw checkout at PR head, macOS/Darwin arm64, Node v22.22.0.

Before-fix (source-level):

const resolvedReplyToId = sanitizeReplyToId(opts.replyToId);
// → replyToId is always set, imsg sees --reply-to, triggers SIP gated reply injection

After-fix (source-level):

const replyActionsEnabled = account.config.actions?.reply !== false;
const resolvedReplyToId = replyActionsEnabled ? sanitizeReplyToId(opts.replyToId) : undefined;
// → when actions.reply=false, replyToId is stripped, imsg sends a normal top-level message

Test results:

node scripts/run-vitest.mjs extensions/imessage/src/send.test.ts extensions/imessage/src/monitor/deliver.test.ts

Test Files  2 passed (2)
Tests      52 passed (52)

New regression tests in actions.reply gating block:

  1. includes reply_to when actions.reply is true — explicit enable still works
  2. includes reply_to when actions.reply is not set — default (not configured) preserves existing behavior
  3. strips reply_to when actions.reply is false — core fix
  4. strips reply_to from media sends when actions.reply is false — media path also gated
  5. strips reply_to from attachment send-attachment when actions.reply is false and audioAsVoice — voice attachment path gated

Closes #92142

1052326311
fix(imessage): honor actions.reply=false when resolving replyToId
d264775 
When `channels.imessage.accounts.<id>.actions.reply` is explicitly
`false` (e.g. because SIP prevents private-API injection), the
iMessage send path still forwarded `replyToId` to the imsg bridge,
causing outbound delivery to fail with:

  System Integrity Protection (SIP) is enabled. Refusing to inject
  into Messages.app. Disable SIP before using imsg launch.

Gate `resolvedReplyToId` on `account.config.actions?.reply !== false`.
The default (actions.reply undefined or true) preserves existing behavior.

Tests: 46 → 51 (5 new regression tests)
Closes openclaw#92142
@openclaw-barnacle openclaw-barnacle Bot added channel: imessage Channel integration: imessage size: S labels Jun 14, 2026
@clawsweeper

clawsweeper Bot commented Jun 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 14, 2026, 4:49 AM ET / 08:49 UTC.

Summary
The PR gates iMessage replyToId/reply_to serialization on existing actions.reply config and adds regression tests for text, media, and voice attachment sends.

PR surface: Source +5, Tests +112. Total +117 across 2 files.

Reproducibility: yes. for source-level reproduction: current main forwards final-reply replyToId into iMessage send and serializes reply_to without checking actions.reply. I did not run a live SIP-enabled macOS iMessage repro in this read-only review.

Review metrics: none identified.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦞 diamond lobster
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P1] Add redacted real macOS/iMessage proof showing actions.reply=false sends a normal top-level message without the SIP private-reply error.
  • Refresh the PR body/checks after proof so the exact head can be re-reviewed.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body provides source-level reasoning and unit-test output, but no after-fix real iMessage/macOS behavior proof; add redacted terminal output, logs, screenshot/video, copied live output, or a linked artifact, then update the PR body to trigger a fresh review or ask a maintainer for @clawsweeper re-review.

Risk before merge

  • [P1] External real behavior proof is still missing: source-level tests do not show a SIP-enabled macOS/iMessage send degrading to a normal top-level message when actions.reply is false.
  • [P1] The live PR status rollup is not green for the exact head, so merge still needs refreshed checks after proof is added.

Maintainer options:

  1. Decide the mitigation before merge
    Merge the narrow send-boundary gate after the contributor adds redacted real macOS/iMessage proof and the exact PR head has green validation.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] No automated repair is appropriate because the remaining blocker is contributor-supplied real behavior proof, not a clear code defect.

Security
Cleared: The diff only changes iMessage send logic and tests; it does not add dependency, workflow, credential, package, or code-execution surface.

Review details

Best possible solution:

Merge the narrow send-boundary gate after the contributor adds redacted real macOS/iMessage proof and the exact PR head has green validation.

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

Yes for source-level reproduction: current main forwards final-reply replyToId into iMessage send and serializes reply_to without checking actions.reply. I did not run a live SIP-enabled macOS iMessage repro in this read-only review.

Is this the best way to solve the issue?

Yes, the patch shape appears to be the narrow maintainable fix because it gates at the resolved iMessage send boundary shared by text, media, and final delivery callers. Merge readiness still depends on real behavior proof and exact-head checks.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 439a9e97fd61.

Label changes

Label changes:

  • add P2: This fixes a concrete iMessage delivery failure with limited channel-specific blast radius.

Label justifications:

  • P2: This fixes a concrete iMessage delivery failure with limited channel-specific blast radius.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦞 diamond lobster.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body provides source-level reasoning and unit-test output, but no after-fix real iMessage/macOS behavior proof; add redacted terminal output, logs, screenshot/video, copied live output, or a linked artifact, then update the PR body to trigger a fresh review or ask a maintainer for @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +5, Tests +112. Total +117 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 6 1 +5
Tests 1 112 0 +112
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 118 1 +117

What I checked:

Likely related people:

  • steipete: History shows iMessage RPC adapter, send/media, and private-API documentation work near this boundary. (role: feature introducer and adjacent owner; confidence: medium; commits: cbac34347b62, 3bb02d333865, d85b2a0e8148; files: extensions/imessage/src/send.ts, docs/channels/imessage.md)
  • mvanhorn: git log -S reply_to points to prior outbound reply-tag work in the same send module. (role: reply metadata contributor; confidence: medium; commits: e0972db7a292; files: extensions/imessage/src/send.ts)
  • scoootscooob: History shows the iMessage channel move into extensions/imessage, carrying the plugin-owned send and delivery paths. (role: delivery path refactor contributor; confidence: medium; commits: 0ce23dc62d37; files: extensions/imessage/src/send.ts, extensions/imessage/src/monitor/deliver.ts)
  • vincentkoc: Current blame in this grafted checkout and the latest release snapshot carry the active iMessage send and delivery files before this PR. (role: recent area contributor; confidence: low; commits: efca4b7e64c2, 8c802aa68351; files: extensions/imessage/src/send.ts, extensions/imessage/src/monitor/deliver.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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.

How this review workflow works
  • 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.

@openclaw-barnacle openclaw-barnacle Bot added the triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. label Jun 14, 2026
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. labels Jun 14, 2026
@omarshahine omarshahine mentioned this pull request Jun 15, 2026
Merged
@omarshahine ChatGPT Codex Connector

omarshahine commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Thanks for sending this fix. I’m closing this as superseded by maintainer PR #93137.

That PR carries the same #92142 fix at the iMessage sender boundary while keeping the maintainer-owned branch authoritative. Your PR’s broader sender-boundary coverage helped confirm the desired test shape, and #93137 includes focused text and media regression checks. The issue remains tracked through #93137.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

channel: imessage Channel integration: imessage P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

iMessage final replies fail under SIP because outbound delivery passes reply_to even when reply actions are disabled

2 participants

@1052326311 @omarshahine