fix(outbound): materialize buffer-only message.send attachments#90794
fix(outbound): materialize buffer-only message.send attachments#90794LiuwqGit wants to merge 4 commits into
Conversation
|
Codex review: needs maintainer review before merge. Reviewed June 7, 2026, 4:26 AM ET / 08:26 UTC. Summary PR surface: Source +139, Tests +433. Total +572 across 7 files. Reproducibility: yes. from source: current main advertises top-level Review metrics: none identified. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the core outbound materialization path after maintainer review and required checks, and track structured Do we have a high-confidence way to reproduce the issue? Yes from source: current main advertises top-level Is this the best way to solve the issue? Yes: materializing the buffer into the existing outbound media store before channel dispatch preserves the current channel media path/URL contract instead of adding per-channel buffer handling. AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against eae4d284e755. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +139, Tests +433. Total +572 across 7 files. View PR surface stats
Acceptance criteria:
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
|
|
@clawsweeper re-review — addressed prior P1/P2 feedback in 127de6f:
Validation: |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review — addressed dry-run + L3 proof blockers in 8e88fe7:
Validation: L3 proof 1 passed + filtered vitest 9 passed + |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
8e88fe7 to
898319b
Compare
CI status note (898319b)PR-scoped checks are green
Remaining failures are merge-with-main upstream blockers, not regressions in the #90768 outbound buffer-send diff:
This branch was rebased onto latest Happy to keep this PR focused on #90768; if maintainers prefer, I can split upstream lint/architecture fixes into a separate PR or wait for main to clear those gates. |
Stage buffer + filename/contentType into the outbound media store and set mediaUrl before core send delivery. Closes openclaw#90768. Co-authored-by: Cursor <[email protected]>
ClawSweeper re-review: replace unbounded maxBytes fallback with resolveSendBufferMaxBytes (resolveAttachmentMaxBytes ?? MEDIA_MAX_BYTES) and add oversized buffer-only send rejection tests at materialization and runner layers. Co-authored-by: Cursor <[email protected]>
Gate send buffer materialization on non-dry-run paths, keep byte-cap validation during previews, and add L3 runtime proof with real outbound media store I/O plus dry-run vs real-send runMessageAction coverage. Co-authored-by: Cursor <[email protected]>
Unblocks check:test-types after rebasing onto latest main. Co-authored-by: Cursor <[email protected]>
898319b to
138a477
Compare
|
Superseded by the maintainer fix now on This kept the core shape from this PR: buffer-only Proof on the landed commit:
Thanks @LiuwqGit. Closing this PR as superseded by the credited main commit. |
Fixes openclaw#90768 Incorporates the send-buffer materialization shape proposed in openclaw#90794 by @LiuwqGit, with maintainer fixes for dry-run, gateway delivery, byte-cap, target-validation, and downstream plugin dispatch paths.
Fixes openclaw#90768 Incorporates the send-buffer materialization shape proposed in openclaw#90794 by @LiuwqGit, with maintainer fixes for dry-run, gateway delivery, byte-cap, target-validation, and downstream plugin dispatch paths.
Fixes openclaw#90768 Incorporates the send-buffer materialization shape proposed in openclaw#90794 by @LiuwqGit, with maintainer fixes for dry-run, gateway delivery, byte-cap, target-validation, and downstream plugin dispatch paths.
Summary
What problem does this PR solve?
message.sendcould fail when callers supplied attachment bytes only viabuffer+filename+contentType, because the durable send path expectedmedia/mediaUrlpaths and never materialized buffer-only payloads.Why does this matter now?
impact:message-loss).What is the intended outcome?
sendparams are staged into the OpenClaw outbound media store and exposed asmedia/mediaUrl/mediaUrlsbefore normal channel delivery, preserving the existing URL/path contract, default outbound media byte cap, and dry-run no-write semantics.What is intentionally out of scope?
sendAttachmentbehavior changes; explicitmedia/mediaUrl/pathsources are left untouched.What does success look like?
message.sendwith onlybuffer+ metadata delivers through the core outbound path with a staged outbound media path; oversized buffer-only payloads are rejected before staging; dry-run previews do not write outbound media files.What should reviewers focus on?
resolveSendBufferMaxBytes+materializeSendBufferMediaParamsinmessage-action-params.ts(dryRungate +MEDIA_MAX_BYTES)resolveOutboundAttachmentFromBufferinmedia/outbound-attachment.tsmessage-action-params.test.ts,message-action-runner.media.test.ts, andmessage-action-buffer-send.runtime-proof.test.tsLinked context
Which issue does this close?
Closes #90768
Which issues, PRs, or discussions are related?
clawsweeper:queueable-fix,clawsweeper:fix-shape-clear,clawsweeper:source-repro)#20258/#50312per reporterWas this requested by a maintainer or owner?
Real behavior proof (required for external PRs)
message.sendattachments materialize to outbound media paths before channel dispatch; dry-run previews skip outbound file writes; payloads above the default media byte cap are rejected before staging/dispatch.fix/issue-90768-send-buffer-materialize, Vitest runtime proof on Windows host (E:/Projects/skills/work/openclaw) using real outbound media store I/O (noexecuteSendActionmocks in L3 proof test).node scripts/run-vitest.mjs src/infra/outbound/message-action-buffer-send.runtime-proof.test.ts --reporter=verbosenode scripts/run-vitest.mjs src/infra/outbound/message-action-params.test.ts src/infra/outbound/message-action-runner.media.test.ts -t "buffer-only|dry run|oversized|materializ"pnpm check:test-types{ "l3_proof": { "environment": "local vitest runtime (no outbound-send mocks)", "dry_run": { "outbound_file_count_before": 0, "outbound_file_count_after_hydrate": 0, "outbound_file_count_after_runMessageAction": 0, "args_media": null, "sendResult_mediaUrl": null }, "real_send": { "staged_path": "<redacted>/.openclaw/media/outbound/artifact---<uuid>.txt", "staged_contents": "artifact bytes", "sendResult_mediaUrl": "<redacted>/.openclaw/media/outbound/artifact---<uuid>.txt", "sendResult_delivery": { "messageId": "msg-runtime-proof" } } }, "vitest": { "filtered": "9 passed (params + runner media + dry-run regressions)", "runtime_proof": "1 passed (message-action-buffer-send.runtime-proof.test.ts)" }, "check:test-types": "exit 0" }hydrateAttachmentParamsForAction/runMessageActionleave outbound media file count unchanged and do not setargs.media/sendResult.mediaUrl. Real send stagesartifact bytesto a readable outbound path and passes the same bytes throughrunMessageAction→ core delivery.hydrateAttachmentParamsForActionskippedsend; buffer-only sends lackedmediaUrl. Initial draft used unboundedmaxBytesand wrote outbound files during dry-run previews.Tests and validation
Which commands did you run?
node scripts/run-vitest.mjs src/infra/outbound/message-action-buffer-send.runtime-proof.test.ts→ 1 passed (L3 proof JSON logged)node scripts/run-vitest.mjs src/infra/outbound/message-action-params.test.ts src/infra/outbound/message-action-runner.media.test.ts -t "buffer-only|dry run|oversized|materializ"→ 9 passedpnpm check:test-types→ exit 0What regression coverage was added or updated?
materializeSendBufferMediaParamsskips outbound writes whendryRunis true; still validates byte cap and normalizes metadata.What failed before this fix, if known?
message.sendnever convertedbufferinto a deliverablemediaUrl; dry-run previews could write outbound media files.If no test was added, why not?
Risk checklist
Did user-visible behavior change? (
Yes/No)Yes.
Did config, environment, or migration behavior change? (
Yes/No)No.
Did security, auth, secrets, network, or tool execution behavior change? (
Yes/No)No.
What is the highest-risk area?
How is that risk mitigated?
MEDIA_MAX_BYTES; preserves dry-run no-write behavior consistent with existing attachment hydration.Current review state
What is the next action?
What is still waiting on author, maintainer, CI, or external proof?
Which bot or reviewer comments were addressed?
MEDIA_MAX_BYTEScap viaresolveSendBufferMaxBytes.dryRungate on send buffer materialization).runMessageActiondry-run vs real-send comparison.