fix(plugin-sdk): failed raw channel sends report success by mushuiyu886 · Pull Request #109906 · openclaw/openclaw

mushuiyu886 · 2026-07-17T10:07:05Z

What Problem This Solves

Fixes an issue where plugins using the legacy raw channel send adapter could report a failed provider delivery as successfully sent when the provider returned ok: false.

Why This Change Was Made

The outbound adapter contract treats a resolved result as a successful delivery, while the legacy raw adapter forwarded an object that carried ok: false. The adapter boundary now throws the provider error before delegating to the exported normalization helper. This matches the documented Plugin SDK migration contract while preserving the helper's existing direct-call behavior and public API.

User Impact

Failed legacy plugin sends now reach the normal outbound failure path instead of being counted and surfaced as successful deliveries.

Evidence

Maintainer-focused Blacksmith Testbox run: src/plugin-sdk/channel-send-result.test.ts, 6/6 passed, including the provider-error fallback case added during review: https://github.com/openclaw/openclaw/actions/runs/29638389495
Fresh maintainer autoreview of the final tree found no actionable findings (confidence 0.96).
Third-party live control (ntfy.sh): a publish to a random temporary topic returned HTTP 200 with message id F52yv796UlWT, and polling that topic observed the same random marker.
Third-party live rejection (ntfy.sh): the provider rejected an invalid priority with HTTP 400 and invalid priority parameter. On current origin/main (b2e34b03b5), the legacy adapter incorrectly resolved { ok: false }; with this patch, the same provider rejection rejects with ntfy HTTP 400: invalid priority parameter.
pnpm plugin-sdk:surface:check passes. scripts/check-changed.mjs passed conflict-marker, max-lines, attribution, wildcard export, duplicate coverage, dependency pin, and formatting checks. Its Plugin SDK API baseline lane fails identically on a clean current origin/main control, so no baseline file is changed here.

clawsweeper · 2026-07-17T10:16:40Z

Codex review: needs maintainer review before merge. Reviewed July 18, 2026, 6:24 AM ET / 10:24 UTC.

Summary
The branch makes legacy raw channel-send adapters reject { ok: false } provider results, preserves direct buildChannelSendResult normalization behavior, adds regression coverage, and carries a bounded safe-reader update for plugin/skill audit files.

PR surface: Source +33, Tests +13. Total +46 across 4 files.

Reproducibility: yes. from source and the supplied live provider transcript: the pre-fix raw adapter forwarded ok: false into normalization, whereas the proposed adapter boundary rejects it. A fresh local replay was not needed to establish the failing control flow.

Review metrics: none identified.

Stored data model
Persistent data-model change detected: serialized state: src/security/audit-extra.async.test.ts, serialized state: src/security/audit-extra.async.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

Obtain the assigned Plugin SDK owner’s explicit confirmation of the legacy raw-adapter rejection contract.
[P2] Refresh review on the exact current head after required checks complete.

Risk before merge

[P1] Existing third-party code that relied on a legacy raw adapter resolving { ok: false } will now receive a rejected send promise; that is consistent with the stated outbound delivery contract, but it is still a public Plugin SDK behavior correction that needs owner sign-off before merge.

Maintainer options:

Confirm the corrected legacy contract (recommended)
Merge after the Plugin SDK owner confirms that rejecting ok: false is the supported adapter behavior and the refreshed exact-head checks are green.
Retain the previous adapter behavior
Pause this change if external consumers must continue receiving resolved failed-result objects from legacy raw adapter methods.

Next step before merge

[P2] No mechanical code repair remains; the assigned owner must decide the intended legacy Plugin SDK failure contract before merge.

Maintainer decision needed

Question: Should the legacy raw channel-send adapter treat provider { ok: false } results as rejected delivery failures while preserving direct buildChannelSendResult normalization for callers that use it directly?
Rationale: This corrects a public legacy Plugin SDK runtime behavior; source and live proof support the change, but only the owner can confirm that exception propagation is the intended compatibility contract for external adapters.
Likely owner: steipete — The PR is assigned to steipete, who also authored the latest focused contract coverage on this exact path.
Options:
- Confirm adapter-level rejection (recommended): Accept failed raw send results as outbound failures and merge after exact-head checks complete.
- Preserve resolved failure results: Keep the legacy adapter non-throwing and define a separate documented failure boundary if external compatibility requires it.

Security
Cleared: The diff adds no third-party execution, dependency, workflow, permission, secret, publishing, or package-resolution risk; the included audit change tightens file-read bounds through existing safe-reader helpers.

Review details

Best possible solution:

Keep failed raw provider results as adapter-level exceptions, retain buildChannelSendResult as the compatible direct normalization helper, and merge only after the Plugin SDK owner confirms that this is the intended legacy migration contract on the refreshed head.

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

Yes, from source and the supplied live provider transcript: the pre-fix raw adapter forwarded ok: false into normalization, whereas the proposed adapter boundary rejects it. A fresh local replay was not needed to establish the failing control flow.

Is this the best way to solve the issue?

Yes, conditionally: moving the throw into the non-exported raw-adapter boundary fixes the incorrect delivery classification without changing the exported direct normalization helper’s contract.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 2e21d53c84ec.

Label changes

Label changes:

add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
remove rating: 🦞 diamond lobster: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.

Label justifications:

P2: The change fixes incorrect delivery reporting for legacy plugin sends with a bounded but user-visible failure-path impact.
merge-risk: 🚨 compatibility: The PR changes the runtime failure contract exposed to legacy raw Plugin SDK adapter consumers from a resolved failed result to a rejected promise.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body supplies an after-fix live ntfy.sh rejection/control transcript tied to the changed error path, plus a focused six-test Testbox result; no additional contributor proof is needed.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body supplies an after-fix live ntfy.sh rejection/control transcript tied to the changed error path, plus a focused six-test Testbox result; no additional contributor proof is needed.

Evidence reviewed

PR surface:

Source +33, Tests +13. Total +46 across 4 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	2	38	5	+33
Tests	2	39	26	+13
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	4	77	31	+46

What I checked:

Raw adapter boundary: The proposed adapter-only helper throws on a failed raw provider result before delegating successful results to the existing exported normalization helper, preserving the direct helper’s established failure-result behavior. (src/plugin-sdk/channel-send-result.ts:91, ce078f3ebe6d)
Regression coverage: The PR changes the raw-adapter test to assert success normalization, provider failure rejection, and the channel-specific fallback error when no provider message exists. (src/plugin-sdk/channel-send-result.test.ts:116, ce078f3ebe6d)
Prior review continuity: The earlier P1 finding to keep throwing inside the raw adapter was addressed; the supplied latest completed review at 6b90591 found no remaining line-level findings and requested owner confirmation of the legacy SDK contract. (src/plugin-sdk/channel-send-result.ts:91, ce078f3ebe6d)
Real-provider proof: The PR body records an ntfy.sh control and a rejected invalid-priority request: pre-fix legacy behavior resolved an ok: false result, while the patched adapter rejected with the provider error. The focused six-test Testbox run is supplemental regression evidence. (src/plugin-sdk/channel-send-result.test.ts:116, ce078f3ebe6d)
Recent area history: The recent test commit adding fallback-error coverage was authored by the assigned reviewer, tying the current contract discussion to the same Plugin SDK test surface. (src/plugin-sdk/channel-send-result.test.ts:137, 6b90591a4d84)
Security-audit scope: The additional audit change replaces an unbounded manifest read with the existing safe-file helpers and adds a size-limit regression test; it adds no dependency, workflow, permission, secret, or package-resolution change. (src/security/audit-extra.async.ts:97, ce078f3ebe6d)

Likely related people:

steipete: Assigned on this PR and authored the recent raw-send fallback-error coverage plus the security-audit commit carried at the current head. (role: recent area contributor and assigned reviewer; confidence: high; commits: 6b90591a4d84, ce078f3ebe6d; files: src/plugin-sdk/channel-send-result.test.ts, src/security/audit-extra.async.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

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 keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Review history (6 earlier review cycles)

reviewed 2026-07-17T10:13:15.063Z sha 182d215 :: needs maintainer review before merge. :: none
reviewed 2026-07-17T10:46:12.305Z sha 96cab3f :: needs changes before merge. :: [P1] Keep failure throwing inside the raw adapter
reviewed 2026-07-17T11:14:20.264Z sha fe98cbe :: needs maintainer review before merge. :: none
reviewed 2026-07-17T11:55:13.505Z sha fe98cbe :: needs maintainer review before merge. :: none
reviewed 2026-07-18T09:23:50.320Z sha 6b90591 :: needs maintainer review before merge. :: none
reviewed 2026-07-18T10:06:17.926Z sha 6b90591 :: needs maintainer review before merge. :: none

…nd-failures

mushuiyu886 · 2026-07-17T11:08:52Z

Addressed the compatibility finding:

Restored buildChannelSendResult direct-call failure normalization.
Moved failed-result rejection to the createRawChannelSendResultAdapter boundary through a non-exported helper.
Re-ran the focused test (5/5), Plugin SDK surface check, and ntfy live success/rejection proof.

@clawsweeper re-review

clawsweeper · 2026-07-17T11:08:55Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

State: Complete
Detail: The durable review comment was published and its verdict was queued for target-serialized routing.
Run: https://github.com/openclaw/clawsweeper/actions/runs/29578463231
Updated: 2026-07-17T11:58:14.746Z

Co-authored-by: 杨浩宇0668001029 <[email protected]>

…emoved fs dependency) (openclaw#110589)

steipete · 2026-07-18T10:30:37Z

Merged via squash.

Prepared head SHA: ce078f3ebe6da2d24b3b5ca1abe3ad0b34d13e17
Landed commit: adff355caf2cbd791579f1339c3370adc9d6ef77

…9906) * fix(plugin-sdk): reject failed raw channel sends * fix(plugin-sdk): preserve raw result helper compatibility * test(plugin-sdk): cover raw send fallback errors Co-authored-by: 杨浩宇0668001029 <[email protected]> * fix(security): bound skill-file audit read via safe reader (restore removed fs dependency) (openclaw#110589) --------- Co-authored-by: Peter Steinberger <[email protected]>

fix(plugin-sdk): reject failed raw channel sends

182d215

openclaw-barnacle Bot added the size: S label Jul 17, 2026

Merge branch 'main' into fix/plugin-sdk-raw-send-failures

96cab3f

mushuiyu886 added 2 commits July 17, 2026 19:01

fix(plugin-sdk): preserve raw result helper compatibility

4996725

Merge remote-tracking branch 'origin/main' into fix/plugin-sdk-raw-se…

fe98cbe

…nd-failures

openclaw-barnacle Bot added size: XS and removed size: S labels Jul 17, 2026

steipete self-assigned this Jul 18, 2026

test(plugin-sdk): cover raw send fallback errors

6b90591

Co-authored-by: 杨浩宇0668001029 <[email protected]>

openclaw-barnacle Bot added size: S and removed size: XS labels Jul 18, 2026

clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jul 18, 2026

fix(security): bound skill-file audit read via safe reader (restore r…

ce078f3

…emoved fs dependency) (openclaw#110589)

steipete requested a review from a team as a code owner July 18, 2026 10:20

clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. and removed rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. labels Jul 18, 2026

steipete merged commit adff355 into openclaw:main Jul 18, 2026
123 of 125 checks passed

github-actions Bot mentioned this pull request Jul 18, 2026

📡 Upstream Digest — 2026-07-18 10:56 UTC curtismercier/openclaw-mods#1333

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(plugin-sdk): failed raw channel sends report success#109906

fix(plugin-sdk): failed raw channel sends report success#109906
steipete merged 6 commits into
openclaw:mainfrom
mushuiyu886:fix/plugin-sdk-raw-send-failures

mushuiyu886 commented Jul 17, 2026 •

edited by steipete

Loading

Uh oh!

clawsweeper Bot commented Jul 17, 2026 •

edited

Loading

Uh oh!

mushuiyu886 commented Jul 17, 2026

Uh oh!

clawsweeper Bot commented Jul 17, 2026 •

edited

Loading

Uh oh!

Uh oh!

steipete commented Jul 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

mushuiyu886 commented Jul 17, 2026 • edited by steipete Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Uh oh!

clawsweeper Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mushuiyu886 commented Jul 17, 2026

Uh oh!

clawsweeper Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

steipete commented Jul 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mushuiyu886 commented Jul 17, 2026 •

edited by steipete

Loading

clawsweeper Bot commented Jul 17, 2026 •

edited

Loading

clawsweeper Bot commented Jul 17, 2026 •

edited

Loading