Skip to content

fix(channels): expose inbound media download failures [AI-assisted] - #100119

Merged
steipete merged 2 commits into
mainfrom
codex/issue-100092-inbound-media
Jul 5, 2026
Merged

steipete merged 2 commits into
mainfrom
codex/issue-100092-inbound-media

Conversation

@steipete

@steipete steipete commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Inbound attachment downloads can fail after a channel has already rendered an optimistic media placeholder. WhatsApp, LINE, Signal, iMessage, Microsoft Teams, Feishu, Mattermost, and Zalo could therefore dispatch phantom placeholders, lose captions, or drop media-only turns even though no bytes reached the agent.

Closes #100092.

Why This Change Was Made

Add one shared agent-facing unavailable-media formatter, while keeping attachment counting, download ownership, and native placeholder selection inside each channel. Raw and command bodies remain the original user text; only the agent-facing body receives the failure notice. Partial successes retain placeholders derived from materialized media rather than advertised media.

User Impact

Users now see a concise channel-specific unavailable-attachment notice when inbound media cannot be materialized. Captions and successfully downloaded attachments are preserved, media-only failures still reach the agent, and command/debounce classification is unchanged by the notice text.

Evidence

  • Fresh autoreview: clean after eight fix cycles.
  • Focused monitor/integration matrix: 345 tests, 11 Vitest shards.
  • Channel SDK contracts: 306 tests, 4 Vitest shards.
  • Core/extensions production and test tsgo: passed.
  • SDK surface, oxfmt, oxlint, diff checks: passed.
  • Fresh Testbox changed gate: tbx_01kwqr0hayahgtekbfqq3yfev9, 10m15s, exit 0.
  • Testbox hydration run: https://github.com/openclaw/openclaw/actions/runs/28723304449
  • External credentialed channel sends were not performed; failure behavior is exercised at the real channel monitor/ingestion boundary with injected transport failures.

AI-assisted implementation and review.

@openclaw-barnacle openclaw-barnacle Bot added channel: imessage Channel integration: imessage channel: line Channel integration: line channel: mattermost Channel integration: mattermost channel: msteams Channel integration: msteams channel: signal Channel integration: signal channel: whatsapp-web Channel integration: whatsapp-web channel: zalo Channel integration: zalo scripts Repository scripts channel: feishu Channel integration: feishu size: XL maintainer Maintainer-authored PR labels Jul 5, 2026
@clawsweeper

clawsweeper Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 4, 2026, 8:25 PM ET / 00:25 UTC.

Summary
The PR adds a shared inbound-media-unavailable formatter and channel-specific handling so failed media materialization reaches the agent as an unavailable-attachment notice while raw and command text stay original.

PR surface: Source +400, Tests +704, Docs +1, Other 0. Total +1105 across 46 files.

Reproducibility: yes. at source level: current main has representative channel paths such as WhatsApp where media download failure is swallowed before downstream body construction. I did not run live credentialed channel sends in this read-only review.

Review metrics: 2 noteworthy metrics.

  • Public SDK exports: 1 function added. A new channel-inbound export becomes third-party plugin API, so maintainers should notice the contract expansion before merge.
  • Release-owned files: 1 changelog edit. CHANGELOG.md is release-owned in this repo; normal PRs should keep release-note context in the PR body or commit message instead.

Stored data model
Persistent data-model change detected: serialized state: extensions/feishu/src/bot.helpers.test.ts, serialized state: extensions/feishu/src/bot.test.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #100092
Summary: This PR is the candidate fix for the linked cross-channel inbound media materialization bug.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🦐 gold shrimp
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:

  • Document or remove the public SDK helper export.
  • Remove the CHANGELOG.md edit.
  • [P1] Add redacted live channel proof or maintainer proof override for representative affected channels.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body lists tests, Testbox, and injected transport failures, and explicitly says external credentialed channel sends were not performed; that is not after-fix real behavior proof for this user-visible channel flow. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] The PR adds one public channel-inbound SDK helper; green CI does not decide whether maintainers want that helper as supported third-party plugin API without docs/contract alignment.
  • [P1] The patch changes raw, command, and agent-facing body separation across eight bundled channel runtimes, but the PR body says external credentialed channel sends were not performed.
  • [P1] The PR edits release-owned CHANGELOG.md; release generation should own that file even when the PR body has useful release-note context.

Maintainer options:

  1. Settle the SDK contract first (recommended)
    Either document and test formatInboundMediaUnavailableText as a supported channel-inbound SDK helper or keep the formatter out of the public plugin SDK before merge.
  2. Accept the additive SDK surface
    Maintainers may intentionally accept the new public helper as-is, but that owns the undocumented third-party plugin API surface added by this PR.
  3. Add live channel proof
    Provide a redacted live run, logs, or reproducible transcript for at least representative affected channels so the raw/command/agent body split is proven outside mocked failure injection.

Next step before merge

  • [P1] The protected maintainer label, public SDK contract choice, and missing real behavior proof need maintainer handling rather than an automatic repair lane.

Security
Cleared: No concrete security or supply-chain regression was found; the PR changes channel runtime/test code and SDK exports without new dependencies, lockfiles, workflows, or secret handling.

Review findings

  • [P2] Document or keep the new SDK helper private — src/plugin-sdk/channel-inbound.ts:186
  • [P3] Remove the release-owned changelog edit — CHANGELOG.md:36
Review details

Best possible solution:

Land the channel-owned unavailable-media invariant only after the SDK contract is either documented and accepted as public API or kept internal, the release-owned changelog edit is removed, and maintainers accept the current proof level or add live channel proof.

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

Yes, at source level: current main has representative channel paths such as WhatsApp where media download failure is swallowed before downstream body construction. I did not run live credentialed channel sends in this read-only review.

Is this the best way to solve the issue?

No, not yet as submitted: the channel invariant is the right direction, but the public SDK helper needs an explicit contract/docs decision and the release-owned changelog edit should be removed.

Full review comments:

  • [P2] Document or keep the new SDK helper private — src/plugin-sdk/channel-inbound.ts:186
    This line makes formatInboundMediaUnavailableText part of openclaw/plugin-sdk/channel-inbound, which is third-party plugin API. The scoped SDK policy requires public subpath expansions to keep docs and contract checks aligned; right now only the surface budget changes, so maintainers inherit an undocumented public helper. Please either document and lock this as supported API, or keep the formatter out of the public SDK surface.
    Confidence: 0.86
  • [P3] Remove the release-owned changelog edit — CHANGELOG.md:36
    CHANGELOG.md is release-owned in this repo, and this PR already carries the needed release-note context in its body. Please remove this entry and leave changelog generation to the release flow.
    Confidence: 0.92

Overall correctness: patch is incorrect
Overall confidence: 0.82

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 4ec7842be0af.

Label changes

Label changes:

  • add P1: The PR targets a high-impact channel workflow where failed inbound media can mislead the agent or drop media-only user turns.
  • add merge-risk: 🚨 compatibility: The diff adds a public plugin SDK function export without corresponding docs/contract alignment.
  • add merge-risk: 🚨 message-delivery: The diff changes how several channel runtimes separate raw, command, and agent-facing message bodies when inbound media fails.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
  • add 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 lists tests, Testbox, and injected transport failures, and explicitly says external credentialed channel sends were not performed; that is not after-fix real behavior proof for this user-visible channel flow. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

  • P1: The PR targets a high-impact channel workflow where failed inbound media can mislead the agent or drop media-only user turns.
  • merge-risk: 🚨 compatibility: The diff adds a public plugin SDK function export without corresponding docs/contract alignment.
  • merge-risk: 🚨 message-delivery: The diff changes how several channel runtimes separate raw, command, and agent-facing message bodies when inbound media fails.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
  • 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 lists tests, Testbox, and injected transport failures, and explicitly says external credentialed channel sends were not performed; that is not after-fix real behavior proof for this user-visible channel flow. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +400, Tests +704, Docs +1, Other 0. Total +1105 across 46 files.

View PR surface stats
Area Files Added Removed Net
Source 26 551 151 +400
Tests 18 715 11 +704
Docs 1 1 0 +1
Config 0 0 0 0
Generated 0 0 0 0
Other 1 2 2 0
Total 46 1269 164 +1105

What I checked:

  • Protected PR state: Live PR metadata shows this PR is open, non-draft, mergeable but unstable, and labeled maintainer, so this review must not close it as cleanup. (1cf513c47254)
  • Linked source-repro issue: The linked issue remains open and describes source-level reproduction for failed media materialization across WhatsApp, LINE, Signal, iMessage, Microsoft Teams, Feishu, Mattermost, and Zalo. (4ec7842be0af)
  • Current-main representative bug path: On current main, WhatsApp catches non-limit downloadMediaMessage failures and returns undefined, allowing downstream body construction to proceed without materialized media bytes. (extensions/whatsapp/src/inbound/media.ts:65, 4ec7842be0af)
  • PR head representative fix path: The PR head propagates WhatsApp media download failures to the monitor, where the agent body gets an unavailable notice and commandBody remains the original body. (extensions/whatsapp/src/inbound/monitor.ts:1160, 1cf513c47254)
  • Public SDK surface expansion: The PR exports formatInboundMediaUnavailableText from openclaw/plugin-sdk/channel-inbound, making it a public plugin SDK helper while the channel-inbound docs still only describe the existing core helper set. (src/plugin-sdk/channel-inbound.ts:186, 1cf513c47254)
  • Scoped policy applied: Root, extensions, channels, plugin-sdk, and scripts AGENTS.md files were read; the plugin SDK guide requires public subpath expansions to keep docs and contract checks aligned, and root policy says CHANGELOG.md is release-owned. (src/plugin-sdk/AGENTS.md:55, 4ec7842be0af)

Likely related people:

  • steipete: Authored the current Telegram unavailable-media failure reference fix and also opened the linked issue that this PR targets, so he is a strong routing candidate for the intended cross-channel contract. (role: recent related fix author; confidence: high; commits: 77f0f40bb1f5; files: extensions/telegram/src/bot-handlers.runtime.ts, extensions/telegram/src/bot/delivery.resolve-media.ts)
  • vincentkoc: Recent commits touched shared channel text/preview and LINE route behavior near the affected inbound channel surfaces. (role: recent channel contributor; confidence: medium; commits: c16bb8725a8a, d1cd74b2431d; files: extensions/msteams/src/monitor-handler/message-handler.ts, extensions/line/src/bot-message-context.ts)
  • RomneyDa: Recent channel lazy-loader and async timing refactors touched adjacent channel/runtime paths, and local shallow blame points to the current snapshot for several affected files. (role: recent adjacent refactor contributor; confidence: medium; commits: 59b08b46930b, c7aca4f02938, c2fc7aa28a49; files: extensions/zalo/src/monitor.ts, extensions/signal/src/monitor/event-handler.ts, src/channels/inbound-event/media.ts)
  • lanzhi-lee: Recently changed the public plugin SDK channel identity hook context, which is adjacent to this PR's new channel-inbound SDK export. (role: recent plugin SDK contributor; confidence: medium; commits: bd479958c04a; files: src/plugin-sdk/channel-inbound.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.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels Jul 5, 2026
@steipete

steipete commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

Land-ready at exact head 1cf513c472548a1a0af4cb6c6ede382f5eccc4e9.

What changed:

  • Failed inbound attachment materialization now preserves captions and dispatches an agent-facing unavailable notice across WhatsApp, LINE, Signal, iMessage, Microsoft Teams, Feishu, Mattermost, and Zalo.
  • Raw/command bodies remain original user input; partial successes retain placeholders derived from materialized media.
  • Teams HTML references are correlated with matching file/card entries, avoiding false failure notices.

Proof:

  • Focused monitor/integration matrix: 345 tests across 11 Vitest shards.
  • Channel SDK contracts: 306 tests across 4 shards.
  • Latest-main Feishu/Teams rerun: 143 tests; final Teams correlation proof: 32 tests.
  • Core/extensions production and test tsgo: passed.
  • SDK surface report, oxfmt, oxlint, and diff checks: passed.
  • Fresh Testbox changed gate: tbx_01kwqr0hayahgtekbfqq3yfev9, 10m15s, exit 0; hydration run https://github.com/openclaw/openclaw/actions/runs/28723304449.
  • Exact-head CI: https://github.com/openclaw/openclaw/actions/runs/28724134626, all jobs successful/skipped.
  • Fresh autoreview after the final correction: no actionable findings.
  • scripts/pr review-validate-artifacts 100119: READY, zero findings, behavioral sweep passed.
  • scripts/pr prepare-run 100119: complete; prepared and PR heads both 1cf513c472548a1a0af4cb6c6ede382f5eccc4e9.

Known proof gap: no credentialed external channel sends were performed. The changed failure behavior is exercised through the real channel monitor/ingestion paths with injected transport failures.

@steipete
steipete merged commit 73fc0f5 into main Jul 5, 2026
209 of 223 checks passed
@steipete
steipete deleted the codex/issue-100092-inbound-media branch July 5, 2026 00:28
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 5, 2026
…penclaw#100119)

* fix(channels): expose inbound media download failures

* fix(msteams): correlate attachment references
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

channel: feishu Channel integration: feishu channel: imessage Channel integration: imessage channel: line Channel integration: line channel: mattermost Channel integration: mattermost channel: msteams Channel integration: msteams channel: signal Channel integration: signal channel: whatsapp-web Channel integration: whatsapp-web channel: zalo Channel integration: zalo maintainer Maintainer-authored PR merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. P1 High-priority user-facing bug, regression, or broken workflow. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. scripts Repository scripts size: XL status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Inbound media failures can dispatch placeholders without bytes

1 participant