fix(channels): expose inbound media download failures [AI-assisted] - #100119
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed July 4, 2026, 8:25 PM ET / 00:25 UTC. Summary 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.
Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. 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:
Next step before merge
Security Review findings
Review detailsBest 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:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 4ec7842be0af. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +400, Tests +704, Docs +1, Other 0. Total +1105 across 46 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
|
|
Land-ready at exact head What changed:
Proof:
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. |
…penclaw#100119) * fix(channels): expose inbound media download failures * fix(msteams): correlate attachment references
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
tsgo: passed.tbx_01kwqr0hayahgtekbfqq3yfev9, 10m15s, exit 0.AI-assisted implementation and review.