Skip to content

fix(message): show dry-run output for send/poll#80604

Closed
alchip wants to merge 2 commits into
openclaw:mainfrom
alchip:fix/message-dry-run-output-80507
Closed

fix(message): show dry-run output for send/poll#80604
alchip wants to merge 2 commits into
openclaw:mainfrom
alchip:fix/message-dry-run-output-80507

Conversation

@alchip

@alchip alchip commented May 11, 2026 •
edited
Loading

Copy link
Copy Markdown

Summary

  • make CLI formatter use result.dryRun instead of handledBy === "dry-run"
  • ensure message send --dry-run and message poll --dry-run render [dry-run] text instead of success checkmark
  • add focused tests for dry-run send/poll formatting

Why

send/poll results return handledBy: "core" | "plugin" with a separate dryRun flag, so the old dry-run branch was unreachable.

Fixes #80507

Validation

  • CI=true pnpm vitest run src/commands/message-format.test.ts

Real behavior proof

  • Behavior or issue addressed: message send --dry-run / message poll --dry-run could print success-style output instead of dry-run output.
  • Real environment tested: Local OpenClaw source checkout on macOS (Darwin arm64), running command-line execution in a real shell.
  • Exact steps or command run after this patch: 
    CI=true pnpm tsx -e "import { formatMessageCliText } from './src/commands/message-format.ts'; const send={kind:'send',action:'send',channel:'slack',to:'channel:C123',handledBy:'core',payload:{messageId:'unknown'},dryRun:true}; const poll={kind:'poll',action:'poll',channel:'slack',to:'channel:C123',handledBy:'core',payload:{messageId:'unknown'},dryRun:true}; console.log('SEND:', formatMessageCliText(send as any).join('\\n')); console.log('POLL:', formatMessageCliText(poll as any).join('\\n'));"
  • Evidence after fix: 
    SEND: [dry-run] would run send via slack
POLL: [dry-run] would run poll via slack
  • Observed result after fix: Both dry-run flows now emit explicit [dry-run] output and no longer appear as successful real sends.
  • What was not tested: End-to-end delivery plugin behavior (out of scope for this formatter-only fix).

@openclaw-barnacle openclaw-barnacle Bot added commands Command implementations size: XS triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 11, 2026
@clawsweeper

clawsweeper Bot commented May 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Thanks for the context here. I did a careful shell check against current main, and this is already implemented.

Close: current main already contains the same result.dryRun formatter fix via #94684, and the linked dry-run CLI bug is closed. This PR's remaining unique test file did not land, but maintainers already accepted the proof-positive one-line implementation, so keeping a second proof-blocked branch open is not useful.

Root-cause cluster
Relationship: superseded
Canonical: #94684
Summary: This PR and the merged replacement target the same dry-run message formatter bug; the replacement landed on main and closed the canonical bug report.

Members:

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

So I’m closing this as already implemented rather than keeping a duplicate issue open.

Review details

Best possible solution:

Keep the merged result.dryRun implementation from #94684 and close this duplicate branch; add extra formatter regression tests later only if maintainers want more coverage.

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

Yes for the original bug: the linked report and v2026.6.8 source show send/poll dry-runs carrying dryRun separately while the old formatter checked handledBy. Current main no longer reproduces that path because it checks result.dryRun.

Is this the best way to solve the issue?

Yes. Checking result.dryRun in the formatter is the narrow maintainable fix; changing outbound handledBy semantics would broaden the runner contract unnecessarily, and this specific PR is now superseded by the merged implementation.

Security review:

Security review cleared: The diff is limited to CLI formatting and focused tests, with no dependency, workflow, secret, permission, package, or code-execution surface change.

AGENTS.md: found and applied where relevant.

What I checked:

Likely related people:

  • lzyyzznl: Authored the merged replacement branch that made the same formatter predicate change now present on main. (role: merged fix author; confidence: high; commits: dce6d6a0d383, 508e3bf41353; files: src/commands/message-format.ts)
  • Peter Steinberger: Introduced the message CLI/tool unification and dominates history across the formatter, command entry point, and outbound result boundary. (role: feature-history owner; confidence: high; commits: 3636a2bf51d9, 2e3ef1b9e180; files: src/commands/message-format.ts, src/commands/message.ts, src/infra/outbound/message-action-runner.ts)
  • vincentkoc: Recent history and live PR discussion connect him to the affected command/outbound paths and the automerge of the proof-positive replacement. (role: recent area contributor and automerge trigger; confidence: medium; commits: 844f405ac1be, 9fced927101c; files: src/commands/message-format.ts, src/infra/outbound/message-action-runner.ts)

Codex review notes: model internal, reasoning high; reviewed against 9594300f8c92; fix evidence: commit 508e3bf41353, main fix timestamp 2026-06-19T03:51:50Z.

@openclaw-barnacle openclaw-barnacle Bot added triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 11, 2026
This was referenced May 11, 2026
Closed
Closed
Closed
Closed
@openclaw-barnacle

openclaw-barnacle Bot commented May 26, 2026

Copy link
Copy Markdown

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

@openclaw-barnacle openclaw-barnacle Bot added the stale Marked as stale due to inactivity label May 26, 2026
@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. P2 Normal backlog priority with limited blast radius. labels May 26, 2026
@clawsweeper

clawsweeper Bot commented May 26, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper PR egg

🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat.

Where did the egg go?
  • The egg game starts only after the PR passes the real-behavior proof check.
  • Before that, no creature or rarity is rolled. The treat waits for real proof.
  • This is still just collectible flavor: proof affects review readiness, not creature quality.

@openclaw-barnacle openclaw-barnacle Bot removed the stale Marked as stale due to inactivity label May 27, 2026
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels May 27, 2026
This was referenced Jun 13, 2026
Closed
Merged
@clawsweeper

clawsweeper Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper applied the proposed close for this PR.

@clawsweeper clawsweeper Bot closed this Jun 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

commands Command implementations P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI.

Projects

None yet

Milestone

No milestone

1 participant

@alchip