feat(cli): add --message-file flag to openclaw message send#79200
feat(cli): add --message-file flag to openclaw message send#79200Joseff531 wants to merge 2 commits into
Conversation
openclaw-barnacle
Bot
added
cli
|
Codex review: found issues before merge. Reviewed July 3, 2026, 5:05 PM ET / 21:05 UTC. Summary PR surface: Source +26, Tests +115, Docs +1. Total +142 across 5 files. Reproducibility: yes. at source level: current main and v2026.6.11 expose inline Review metrics: 1 noteworthy metric.
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:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Resolve file input at the CLI preprocessing boundary or through an explicit sandbox-aware read capability, forward canonical message text into current Do we have a high-confidence way to reproduce the issue? Yes, at source level: current main and v2026.6.11 expose inline Is this the best way to solve the issue? No, not as implemented: the flag direction is reasonable, but file reading belongs at a CLI-only or sandbox-aware boundary rather than inside the shared outbound runner. The branch also needs a rebase/refit to the current payload builder before it can be judged mergeable. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 010b61746379. Label changesLabel justifications:
Evidence reviewedPR surface: Source +26, Tests +115, Docs +1. Total +142 across 5 files. View PR surface stats
Security concerns:
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
|
openclaw-barnacle
Bot
added
proof: supplied
|
Thanks for the careful review — both findings are valid and are fixed in the follow-up commit ( P2 — File content bypasses inline-message normalization Root cause: the original code assigned file content into Fix: file content is now kept in a separate P3 — Empty Root cause: Fix: changed to Docs
Test changes
All 13 tests pass. @clawsweeper re-review Re-review progress:
|
2b1d989 to
6b199b9
Compare
|
Live validation on PR head Checks run:
Results:
Live smoke: Exercised the real node --import tsx/esm --input-type=module << 'SCRIPT'
# file containing backticks, $, real newlines, and literal \n (would corrupt via -m "$(cat ...)")
await fs.writeFile(filePath, " hello\\nworld\n```json\n{\"status\":\"done\"}\n```\n$VAR", "utf8");
await runDrySend({ cfg: workspaceConfig, actionParams: { channel: "workspace", target: "#C12345678", messageFile: filePath } });
# mutual exclusion — non-empty --message
await runDrySend({ ..., actionParams: { ..., message: "inline", messageFile: filePath } });
# mutual exclusion — empty --message ""
await runDrySend({ ..., actionParams: { ..., message: "", messageFile: filePath } });
# missing file
await runDrySend({ ..., actionParams: { ..., messageFile: "/tmp/.../nope.txt" } });
SCRIPTOutput: This verifies: file content (including backticks, |
3cfa3e3 to
fc54165
Compare
0efffa4 to
cf9bc0c
Compare
clawsweeper
Bot
added
the
proof: sufficient
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Hi, @steipete |
|
This pull request has been automatically marked as stale due to inactivity. |
clawsweeper
Bot
removed
the
proof: sufficient
clawsweeper
Bot
added
the
rating: 🌊 off-meta tidepool
clawsweeper
Bot
added
proof: sufficient
clawsweeper
Bot
added
status: ⏳ waiting on author
clawsweeper
Bot
added
rating: 🦐 gold shrimp
clawsweeper
Bot
added
P2
1 participant
Summary
openclaw message send -m "$(cat report.txt)"fails or corrupts messages when the content contains backticks,${},!, or newlines — all common in agent completion reports containing code blocks or JSON.--message-file <path>toopenclaw message send. The flag reads the file as UTF-8 and uses its content as the message body, bypassing the shell entirely. Mutually exclusive with--message; errors clearly on missing/unreadable files or when both flags are supplied.--message/-mbehaviour, delivery path, gateway RPC, channel adapters, or any other flag. This is a purely additive CLI input path.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
message sendsurface, different bug)Real behavior proof (required for external PRs)
openclaw message sendcorrupts or fails to deliver agent reports containing code blocks, JSON, or shell-special characters when passed via-m "$(cat ...)".node --import tsx/esmrunning the realhandleSendActioncode path with real temp files — no mocking, real fs I/O.node --import tsx/esmto load the realrunDrySendhelper against a temp file containing backticks,$, and newlines. Three scenarios: successful send, mutual-exclusion error, and missing-file error.node --import tsx/esmexercising the real module:$, newlines, and JSON) reaches the send action successfully —result.kindis"send". Mutual exclusion and missing-file errors fire with clear messages and the correct text.--message, so no new risk there.-m "$(cat file)") fails silently on special chars; no reliable before-state to capture.Root Cause (if applicable)
N/A — this is a new feature, not a regression. The shell-expansion problem is inherent to passing arbitrary text as a CLI argument.
Regression Test Plan (if applicable)
src/infra/outbound/message-action-runner.send-validation.test.ts--message-filereads file content and sends successfully; mutual exclusivity with--messageis enforced; ENOENT produces a clear error; multiline/special-char content passes through without mangling.handleSendAction; dry-run tests exercise the full code path without a real gateway.send-validationsuite.User-visible / Behavior Changes
New flag
--message-file <path>onopenclaw message send. Existing--message/-mis unchanged. If both are supplied together, the command exits with a clear error. If the file is missing or unreadable, a clear error is printed and the command exits non-zero.Diagram (if applicable)
Security Impact (required)
--media <path>which already exists.Repro + Verification
Environment
Steps
printf '```json\n{"status":"done"}\n```\n$VAR' > /tmp/msg.txtopenclaw message send -c telegram -t <id> --message-file /tmp/msg.txtExpected
$, and newlinesActual (before fix)
Evidence
Live terminal output captured above (3 scenarios: send, mutual-exclusion error, missing-file error — all via real
nodeexecution). 12/12 tests pass in send-validation suite;pnpm check:changedall lanes green.Human Verification (required)
result.kind: sendwith no corruption; mutual exclusion error fires when both--messageand--message-fileare given; ENOENT producesmessage file not found: <path>; multiline/special-char content (backticks,$, newlines, JSON) passes through intact;--mediaalone still works without either message flag.causeattached to all re-thrown fs errors (lint rule); file with only whitespace is allowed (consistent with--message ""); EACCES path is guarded separately from generic read errors.Review Conversations
Compatibility / Migration
Risks and Mitigations
--message "$(cat largefile)". No new surface — channel adapters already enforce their own size limits downstream.--media <local-path>, which already accepts arbitrary local paths.