fix(imessage): retry watch.subscribe startup failures#65482
Conversation
🔒 Aisle Security AnalysisWe found 1 potential security issue(s) in this PR:
1. 🟡 Terminal/log injection and potential sensitive data exposure via unsanitized startup error logging in iMessage monitor
DescriptionIn
This is inconsistent with the newly introduced Vulnerable code: runtime.error?.(danger(`imessage: monitor failed: ${String(err)}`));
runtime.log?.(
warn(
`imessage: watch.subscribe startup failed (attempt ${attempt}/${WATCH_SUBSCRIBE_MAX_ATTEMPTS}): ${String(err)}; retrying`,
),
);RecommendationSanitize and bound error text before logging.
Example fix: import { sanitizeTerminalText, truncateUtf16Safe } from "openclaw/plugin-sdk/text-runtime";
const MAX_ERR_CHARS = 300;
const safeErr = (() => {
const s = sanitizeTerminalText(String(err));
return s.length > MAX_ERR_CHARS ? `${truncateUtf16Safe(s, MAX_ERR_CHARS - 1)}…` : s;
})();
runtime.error?.(danger(`imessage: monitor failed: ${safeErr}`));
runtime.log?.(warn(`imessage: watch.subscribe startup failed ...: ${safeErr}; retrying`));If RPC error Analyzed PR: #65482 at commit Last updated on: 2026-04-12T18:43:58Z |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: defb51239b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Greptile SummaryThis PR adds a bounded 3-attempt retry loop to the
Confidence Score: 5/5Safe to merge; implementation is correct and tests are solid. One P2 CHANGELOG placement issue to fix. All code logic is sound — retry loop, abort handling, and client cleanup are correctly implemented. The only finding is a P2 doc placement issue (CHANGELOG entry in the wrong released section), which does not affect runtime behavior. CHANGELOG.md — entry needs to move from Prompt To Fix All With AIThis is a comment left during a code review.
Path: CHANGELOG.md
Line: 213
Comment:
**CHANGELOG entry placed in released version block**
This fix entry landed at line 213, which is inside the `## 2026.4.10` block (lines 76–214), not in `## Unreleased`. Per `CLAUDE.md`: *"Changelog placement: in the active version block, append new entries to the end of the target section"* — the active block for in-flight work is `## Unreleased > ### Fixes`. The entry should be moved there to avoid misattributing this fix to an already-shipped release.
**Context Used:** CLAUDE.md ([source](https://app.greptile.com/review/custom-context?memory=fd949e91-5c3a-4ab5-90a1-cbe184fd6ce8))
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix(imessage): retry watch.subscribe sta..." | Re-trigger Greptile |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 10d15644a0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
1b660f7 to
2408d10
Compare
* fix(imessage): retry watch.subscribe startup failures * fix(imessage): sanitize watch error logging
* fix(imessage): retry watch.subscribe startup failures * fix(imessage): sanitize watch error logging
* fix(imessage): retry watch.subscribe startup failures * fix(imessage): sanitize watch error logging
Summary
watch.subscribestartup timeout or early RPC close immediately fails the monitor and bounces the channel.watch.subscribestartup failures locally before surfacing a fatal monitor error, and a focused regression test locks that behavior in.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Root Cause (if applicable)
monitorIMessageProvidertreated subscribe-time RPC startup failures as fatal on the first attempt, so a transientwatch.subscribetimeout or early RPC close tore down the monitor immediately.watch.subscribe.Regression Test Plan (if applicable)
extensions/imessage/src/monitor.watch-subscribe-retry.test.tswatch.subscribestartup timeout retries locally and succeeds; repeated startup timeouts still fail after the bounded retry budget.User-visible / Behavior Changes
watch.subscribestartup stalls instead of immediately tearing down on the first timeout.Diagram (if applicable)
Security Impact (required)
Yes, explain risk + mitigation:Repro + Verification
Environment
Steps
watch.subscriberequest fail withimsg rpc timeout (watch.subscribe).Expected
Actual
Evidence
Human Verification (required)
OPENCLAW_LOCAL_CHECK=0 pnpm test:serial extensions/imessage/src/monitor.watch-subscribe-retry.test.ts;OPENCLAW_LOCAL_CHECK=0 pnpm buildReview Conversations
Compatibility / Migration
Risks and Mitigations