Skip to content

fix(imessage): stage remote media before understanding#87113

Merged
steipete merged 1 commit into
openclaw:mainfrom
TurboTheTurtle:fix/imessage-remote-media-before-understanding-87089
May 27, 2026
Merged

fix(imessage): stage remote media before understanding#87113
steipete merged 1 commit into
openclaw:mainfrom
TurboTheTurtle:fix/imessage-remote-media-before-understanding-87089

Conversation

@TurboTheTurtle

@TurboTheTurtle TurboTheTurtle commented May 27, 2026

Copy link
Copy Markdown
Contributor

Summary

  • stage remote inbound iMessage media before media understanding runs when MediaRemoteHost is present
  • mark successfully pre-staged media as MediaStaged so the later pre-agent staging block does not retry the rewritten cache path
  • add regression coverage for the remote iMessage ordering and keep the existing SCP remote-path tests green

Root cause

The iMessage remote-host path intentionally passes raw remote Mac attachment paths plus MediaRemoteHost so stageSandboxMedia can SCP-fetch and rewrite them. getReplyFromConfig ran media understanding before that staging step, so image analysis saw /Users/.../Library/Messages/Attachments/... and rejected it before SCP had a chance to cache the file locally.

Real behavior proof

  • Behavior or issue addressed: Remote iMessage image attachments should be fetched into OpenClaw's local remote-cache before media understanding sees the attachment path. The failing behavior was media understanding receiving the raw macOS /Users/.../Library/Messages/Attachments/... path.
  • Real environment tested: Local macOS source checkout of this PR, running the actual OpenClaw stageSandboxMedia runtime with HOME isolated to a temp OpenClaw setup. The scenario used an iMessage remoteHost config, a raw macOS Messages attachment path, and a temporary local scp wrapper that copied the remote-path fixture into the destination requested by OpenClaw.
  • Exact steps or command run after this patch: Ran a one-off node --import tsx --input-type=module OpenClaw runtime probe from the PR checkout. The probe created a raw /Users/demo/Library/Messages/Attachments/ab/cd/photo.jpg attachment, set MediaRemoteHost=user@gateway-host, called the real stageSandboxMedia implementation with iMessage remote attachment roots, then printed the path that media understanding would receive after the pre-understanding staging step.
  • Evidence after fix: Copied terminal output from the after-fix runtime probe:
proof scenario: Linux gateway style iMessage remoteHost with raw macOS attachment path
before MediaPath=/Users/demo/Library/Messages/Attachments/ab/cd/photo.jpg
stagedCount=1
after MediaPath=/var/folders/sl/5dkd3zq12dv65j6jx57zq1hc0000gn/T/tmp.PzNk6ilXYX/.openclaw/media/remote-cache/agent-main-imessage-direct-user-629f8d24/photo.jpg
after MediaPaths=["/var/folders/sl/5dkd3zq12dv65j6jx57zq1hc0000gn/T/tmp.PzNk6ilXYX/.openclaw/media/remote-cache/agent-main-imessage-direct-user-629f8d24/photo.jpg"]
sessionCtx MediaPath=/var/folders/sl/5dkd3zq12dv65j6jx57zq1hc0000gn/T/tmp.PzNk6ilXYX/.openclaw/media/remote-cache/agent-main-imessage-direct-user-629f8d24/photo.jpg
rewrittenUsesRemoteCache=true
cachedFileBytes=34
mediaUnderstandingInputPath=/var/folders/sl/5dkd3zq12dv65j6jx57zq1hc0000gn/T/tmp.PzNk6ilXYX/.openclaw/media/remote-cache/agent-main-imessage-direct-user-629f8d24/photo.jpg
rawRemotePathStillVisibleToUnderstanding=false
  • Observed result after fix: The raw remote Messages path was rewritten to a local OpenClaw .openclaw/media/remote-cache/.../photo.jpg path before the handoff. The media-understanding input path was the local cache path, and rawRemotePathStillVisibleToUnderstanding=false confirms the failing raw /Users/... path no longer reaches the next step.
  • What was not tested: I did not use a live Apple Messages database or real remote Mac credentials in this public proof. The exercised behavior is the OpenClaw remote-host staging and path rewrite contract that sits between the iMessage monitor and media understanding.

Verification

  • node scripts/run-vitest.mjs src/auto-reply/reply/get-reply.message-hooks.test.ts src/auto-reply/reply.stage-sandbox-media.scp-remote-path.test.ts
  • pnpm exec oxfmt --check --threads=1 CHANGELOG.md src/auto-reply/reply/get-reply.ts src/auto-reply/reply/get-reply.message-hooks.test.ts
  • git diff --check
  • pnpm changed:lanes --json --base upstream/main
  • pnpm check:changed -- --base upstream/main

Fixes #87089

Attribution note: if maintainers squash or rework this PR, please preserve author attribution or include Co-authored-by: Andy Ye <[email protected]>.

@openclaw-barnacle openclaw-barnacle Bot added size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 27, 2026
@clawsweeper

clawsweeper Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed May 26, 2026, 10:26 PM ET / 02:26 UTC.

Summary
The PR stages MediaRemoteHost inbound media before media understanding in get-reply and adds a regression test for remote iMessage media ordering.

PR surface: Source +44, Tests +68. Total +112 across 2 files.

Reproducibility: yes. Current main is source-reproducible: iMessage remoteHost passes raw attachment paths plus MediaRemoteHost, while get-reply.ts currently runs media understanding before stageSandboxMedia.

Review metrics: none identified.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

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

Next step before merge
No automated repair remains; the branch is cleanly mergeable and the remaining action is normal maintainer review/landing.

Security
Cleared: The diff reorders use of the existing validated remote staging helper and adds a regression test; it does not add new dependencies, workflow permissions, secrets handling, or command construction.

Review details

Best possible solution:

Land this focused ordering fix with the regression test once required checks remain green, keeping remote staging in the existing stageSandboxMedia contract without adding config or new channel policy.

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

Yes. Current main is source-reproducible: iMessage remoteHost passes raw attachment paths plus MediaRemoteHost, while get-reply.ts currently runs media understanding before stageSandboxMedia.

Is this the best way to solve the issue?

Yes. Moving only MediaRemoteHost staging before media understanding reuses the existing SCP/cache rewrite contract and preserves the existing local-media staging path.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 90653775a96f.

Label changes

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal output from a runtime probe showing a raw remote Messages path rewritten into .openclaw/media/remote-cache before media understanding input.
  • add rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • remove rating: 🐚 platinum hermit: Current PR rating is rating: 🦞 diamond lobster, so this older rating label is no longer current.

Label justifications:

  • P1: The PR fixes a broken iMessage remote attachment workflow where image understanding can receive an unreachable raw Mac path before SCP staging.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes after-fix terminal output from a runtime probe showing a raw remote Messages path rewritten into .openclaw/media/remote-cache before media understanding input.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal output from a runtime probe showing a raw remote Messages path rewritten into .openclaw/media/remote-cache before media understanding input.
Evidence reviewed

PR surface:

Source +44, Tests +68. Total +112 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 44 0 +44
Tests 1 68 0 +68
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 112 0 +112

What I checked:

  • Repository policy read: Root AGENTS.md was read fully; no scoped AGENTS.md owns src/auto-reply, and the only maintainer note found was Telegram-specific. (AGENTS.md:1, 90653775a96f)
  • Current main ordering bug: Current main applies media understanding at get-reply.ts:390 before the later sandbox media staging block at get-reply.ts:939, so raw remote media can reach understanding first. (src/auto-reply/reply/get-reply.ts:390, 90653775a96f)
  • iMessage remote path contract: The iMessage monitor skips local attachment staging when remoteHost is set, carries raw attachment paths forward, and sets MediaRemoteHost on the inbound context. (extensions/imessage/src/monitor/monitor-provider.ts:618, 90653775a96f)
  • Existing staging contract: stageSandboxMedia already handles MediaRemoteHost by validating remote roots, SCP-staging into the remote cache, and rewriting MediaPath/MediaUrls on the context. (src/auto-reply/reply/stage-sandbox-media.ts:36, 90653775a96f)
  • PR diff review: The PR head adds a pre-understanding remote-media staging helper/call and a focused message-hooks regression test; the prior CHANGELOG.md edit is no longer in the diff. (src/auto-reply/reply/get-reply.ts:187, 3e10811de359)
  • Related item search: Search results show this PR and the linked open bug as the active same-root-cause items; older iMessage attachment/SCP items are closed or different root causes.

Likely related people:

  • shakkernerd: Git blame ties the current get-reply media-understanding/staging ordering and stageSandboxMedia implementation to the recent refactor: mark user turn persistence inside runtimes work. (role: recent area contributor; confidence: high; commits: e9a2f1090090, 20d7bf75259f; files: src/auto-reply/reply/get-reply.ts, src/auto-reply/reply/stage-sandbox-media.ts)
  • homer-byte: Merged iMessage attachment staging work in fix(imessage): make inbound image attachments readable by agents #78580 established the remote-host raw-path contract that this PR now routes through core staging before understanding. (role: adjacent iMessage media contributor; confidence: medium; commits: 689ed879cb52, dc81190022fd, 0fac83f34f11; files: extensions/imessage/src/monitor/monitor-provider.ts, extensions/imessage/src/monitor/media-staging.ts, extensions/imessage/src/media-contract.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 proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 27, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P1 High-priority user-facing bug, regression, or broken workflow. labels May 27, 2026
@clawsweeper

clawsweeper Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper PR egg

✨ Hatched: 🥚 common Frosted Merge Sprite

Hatch command

Comment @clawsweeper hatch when this PR is hatchable.

Hatchability rules:

  • Merged PRs are hatchable.
  • Open PRs are hatchable when they are status: 👀 ready for maintainer look, status: 🚀 automerge armed, or labeled clawsweeper:automerge.
  • Closed unmerged PRs are hatchable only when one of those hatchable labels is still present in the durable record.

Rarity: 🥚 common.
Trait: guards the happy path.
Image traits: location flaky test forest; accessory proof snapshot camera; palette plum, gold, and soft gray; mood sleepy but ready; pose nestled inside a glowing shell; shell woven fiber shell; lighting soft studio lighting; background tiny shells and proof notes.
Share on X: post this hatch
Copy: My PR egg hatched a 🥚 common Frosted Merge Sprite in ClawSweeper.

What is this egg doing here?
  • Eggs appear after the PR passes real-behavior proof. It is here for vibes, not verdicts: it does not change labels, ratings, merge decisions, or automation.
  • The shell reacts to review momentum: open follow-up work warms it up, re-review makes it wobble, and a clean final review lets it hatch.
  • Hatchability usually comes from sufficient real-behavior proof, no blocking P0/P1/P2 findings, no security attention needed, and clean correctness. A merged PR is already final, so merge makes the egg hatchable independently.
  • The hatch is seeded from this repository and PR number, so the same PR keeps the same creature; the reviewed head SHA can only change safe visual details.
  • Rarity is just collectible sparkle: 🥚 common, 🌱 uncommon, 💎 rare, ✨ glimmer, and 🌈 legendary.

@TurboTheTurtle
TurboTheTurtle force-pushed the fix/imessage-remote-media-before-understanding-87089 branch from 0ccd1b7 to 3e10811 Compare May 27, 2026 02:18
@TurboTheTurtle

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Resolved the conflict by rebasing onto current main and removed the direct CHANGELOG.md edit as requested. The PR now only changes the runtime and regression test files.

@clawsweeper

clawsweeper Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 27, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels May 27, 2026
@steipete steipete self-assigned this May 27, 2026
@steipete

Copy link
Copy Markdown
Contributor

Verification before merge:

Behavior addressed: remote iMessage image attachments with MediaRemoteHost are staged into the local remote-cache before media understanding sees the path.
Real environment tested: local source checkout on PR head merged with current origin/main.
Exact steps or command run after this patch:

  • git checkout -B land-pr-87113 refs/remotes/pr/87113
  • git merge --no-ff --no-edit origin/main
  • node scripts/run-vitest.mjs src/auto-reply/reply/get-reply.message-hooks.test.ts src/auto-reply/reply.stage-sandbox-media.scp-remote-path.test.ts
  • pnpm exec oxfmt --check --threads=1 src/auto-reply/reply/get-reply.ts src/auto-reply/reply/get-reply.message-hooks.test.ts
  • git diff --check origin/main...HEAD
    Evidence after fix: Vitest passed 2 files / 11 tests; oxfmt reported both touched files correctly formatted; git diff whitespace check passed.
    Observed result after fix: regression coverage confirms stageSandboxMedia runs before media understanding and rewrites MediaPath/MediaPaths/MediaUrl/MediaUrls to the staged cache path, with MediaStaged set before the later staging guard.
    What was not tested: no live Apple Messages database or real remote Mac credentials in this maintainer proof; contributor supplied a runtime SCP-wrapper proof in the PR body.

@steipete
steipete merged commit ae52be9 into openclaw:main May 27, 2026
118 of 121 checks passed
@TurboTheTurtle
TurboTheTurtle deleted the fix/imessage-remote-media-before-understanding-87089 branch May 27, 2026 03:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P1 High-priority user-facing bug, regression, or broken workflow. proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: iMessage remote attachment staged after media understanding — image tool sees raw Mac path before SCP fetch

2 participants