feat(channels): batch 1 producers drop media placeholder bodies#111447
Conversation
openclaw-barnacle
Bot
added
docs
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: abbcd6d81f
ℹ️ 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".
| const mediaInputs: ChannelInboundMediaInput[] = attachments.map((attachment) => ({ | ||
| contentType: attachment.contentType, | ||
| })); |
There was a problem hiding this comment.
When a Google Chat message has multiple attachments, every item after the first is deliberately left type-only here. buildChannelInboundEventContext projects those facts into index-aligned MediaPaths entries containing "" (src/channels/inbound-event/context.ts:352 and src/channels/inbound-event/media.ts:161), but buildInboundMediaNote counts and renders every MediaPaths entry (src/auto-reply/media-note.ts:134), so a normal two-attachment turn tells the model that an undownloaded second file is attached and emits a blank attachment path; Mattermost mixed download failures now hit the same path. Preserve index alignment for structured processing, but exclude facts without a path or URL from prompt attachment lines.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 231e43f1ab
ℹ️ 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".
| : nativeMediaKind | ||
| ? [{ kind: nativeMediaKind }] | ||
| : []; | ||
| const rawBody = textContent; |
There was a problem hiding this comment.
When a captionless LINE video or document is successfully downloaded, this leaves BodyForAgent empty while retaining only structured media. Bound ACP dispatch only converts image attachments (src/auto-reply/reply/agent-turn-attachments.ts:101-107), and if media understanding produces no text or extracted image, tryDispatchAcpReply classifies the empty prompt plus zero supported attachments as acp_empty_prompt and completes without running the model (src/auto-reply/reply/dispatch-acp.ts:685-736). The changed Mattermost path has the same regression for captionless non-image files. Preserve a downstream text fallback derived from the structured media facts when ACP cannot pass the attachment itself.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
rating: 🦪 silver shellfish
|
Codex review: needs real behavior proof before merge. Reviewed July 19, 2026, 1:12 PM ET / 17:12 UTC. Summary PR surface: Source +104, Tests +294, Docs +8, Generated 0, Other +2. Total +408 across 18 files. Reproducibility: no. live reproduction was supplied in the reviewed materials, but the failure paths are high-confidence from source: captionless non-image facts yield no ACP attachments and no prompt text, while type-only Review metrics: 1 noteworthy metric.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Copy recommended automerge instructionNext step before merge
Security Review findings
Review detailsBest possible solution: Keep structured media as the canonical channel contract, but add a downstream ACP-safe text fallback for unsupported captionless media and have prompt-note rendering omit facts with neither a usable path nor URL; then show redacted after-fix channel evidence for successful and failed media-only cases. Do we have a high-confidence way to reproduce the issue? No live reproduction was supplied in the reviewed materials, but the failure paths are high-confidence from source: captionless non-image facts yield no ACP attachments and no prompt text, while type-only Is this the best way to solve the issue? No. Replacing producer placeholders is directionally sound, but doing so without repairing ACP’s unsupported-media fallback and the prompt-note consumer creates two downstream regressions; the shared consumer fixes are the narrower durable solution. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against d4c7549d1ca7. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +104, Tests +294, Docs +8, Generated 0, Other +2. Total +408 across 18 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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
Review history (1 earlier review cycle)
|
Media-placeholder program batch 1: Google Chat, Zalo, LINE, and Mattermost stop minting <media:kind> placeholder bodies. Media-only messages carry an empty caption plus one structured fact per native attachment (type-only when a download fails or is rejected, so payload positions and kind signals stay aligned). The shared formatMediaPlaceholderText SDK formatter renders text-only carriers (Mattermost pending-room lines) from structured facts; per-channel placeholder builders and the expected-count side channel are deleted.
clawsweeper
Bot
added
rating: 🧂 unranked krab
231e43f to
00432a7
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 00432a7f65
ℹ️ 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".
| const mediaInputs: ChannelInboundMediaInput[] = attachments.map((attachment) => ({ | ||
| contentType: attachment.contentType, | ||
| })); |
There was a problem hiding this comment.
When a captionless attachment lacks attachmentDataRef.resourceName—for example, the no-ref attachment already constructed in extensions/googlechat/src/monitor.test.ts:288—downloadAttachment returns null, leaving this fact with only contentType while rawBody remains empty. buildChannelInboundMediaPayload then emits no MediaPath(s) or MediaUrl(s), so buildInboundMediaNote returns undefined (src/auto-reply/media-note.ts:136-144), and bound ACP completes the turn as acp_empty_prompt without invoking the model (src/auto-reply/reply/dispatch-acp.ts:731-736). Unlike the existing multi-attachment blank-path finding, this fresh single-attachment case loses the prior <media:attachment> signal entirely; retain an unavailable-media text fallback when no attachment can be materialized.
Useful? React with 👍 / 👎.
|
Merged via squash.
|
…claw#111447) * feat(channels): batch 1 producers drop media placeholder bodies Media-placeholder program batch 1: Google Chat, Zalo, LINE, and Mattermost stop minting <media:kind> placeholder bodies. Media-only messages carry an empty caption plus one structured fact per native attachment (type-only when a download fails or is rejected, so payload positions and kind signals stay aligned). The shared formatMediaPlaceholderText SDK formatter renders text-only carriers (Mattermost pending-room lines) from structured facts; per-channel placeholder builders and the expected-count side channel are deleted. * fix(mattermost): satisfy type, deadcode, and SDK manifest gates
1 participant
What Problem This Solves
Batch 1 of the media-placeholder elimination program (hard-delete end state, producers migrate before core consumer deletion; PR #111315 was the program's PR1). Four channel plugins — Google Chat, Zalo, LINE, Mattermost — minted
<media:kind>placeholder bodies for media-only inbound messages, mixing model/transport scaffolding into user-authored text that core then had to regex-un-parse.Why This Change Was Made
formatMediaPlaceholderText(facts)insrc/channels/inbound-event/media.ts, exported via the existingopenclaw/plugin-sdk/channel-inboundsubpath. Kind inference reuses media-core (kind→ MIME → URL/path extension →attachment); existing count vocabulary; mixed kinds collapse; no filename annotations. Mattermost's pending-room text (a text-only carrier) is its first consumer.docs/plugins/sdk-channel-inbound.md.User Impact
Media-only messages from these four channels no longer inject
<media:image>-style text into transcripts, previews, or model prompts; captions are exactly what the user typed. Failed downloads still surface their existing unavailable notices, and (new) their attachment kind/count still reaches the model context instead of vanishing.Evidence
node scripts/run-vitest.mjs(each file green): formatter 8/8, googlechat monitor 17/17, zalo polling lifecycle 15/15, LINE bot-message-context 19/19, mattermost monitor-resources 18/18 + monitor 54/54, plus core consumers unchanged: inbound-media 6/6, user-turn-transcript 31/31, session-memory transcript 3/3, media-understanding apply 58/58.check:changedexit 0 — API manifest/exports/surface, four tsgo lanes, core/extensions/scripts lint, plugin boundaries, formatting, import-cycle guards (run 29688888129).--checkgreen, formatter suite rerun green; surface budget re-derived against main's harvested base (8182 + 2 = 8184).<media:*,mediaPlaceholder, orexpectedCountmatches.