refactor: centralize inbound supplemental context#86479
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed May 27, 2026, 4:13 AM ET / 08:13 UTC. Summary Reproducibility: yes. for the review finding by source inspection: the mock exposes the production Review metrics: 2 noteworthy metrics.
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: Mantis proof suggestion Risk before merge
Maintainer options:
Copy recommended automerge instructionNext step before merge Security Review findings
Review detailsBest possible solution: Fix the mock compatibility regression, add redacted live proof for at least one affected real channel path, and have maintainers explicitly review the public SDK compatibility and CI-helper scope before merge. Do we have a high-confidence way to reproduce the issue? Yes for the review finding by source inspection: the mock exposes the production Is this the best way to solve the issue? No, not yet; the SDK refactor preserves the deprecated runtime alias in production, but the test helper should mirror that alias instead of casting the wrong mock shape. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against 7e702bb43d0a. Label changesLabel changes:
Label justifications:
Evidence reviewedAcceptance 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
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 813f2f174d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| SupplementalContext: entry.replyToBody | ||
| ? { | ||
| quote: { |
There was a problem hiding this comment.
Preserve Signal quote metadata when quoted text is empty
Guarding SupplementalContext on entry.replyToBody drops quote context whenever the quoted message has no text (for example, media-only quotes), even if replyToSender is available and visibility allowed. Before this change, sender/quote metadata was still forwarded in that case via the legacy fields; now those turns lose quote context entirely, which regresses prompt context for Signal replies to non-text messages.
Useful? React with 👍 / 👎.
|
ClawSweeper PR egg 🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat. Where did the egg go?
|
62ecf5d to
18417ae
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 18417ae00f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| quote: entry.replyToBody | ||
| ? { | ||
| body: entry.replyToBody, | ||
| sender: entry.replyToSender, | ||
| isQuote: entry.replyToIsQuote, | ||
| } | ||
| : undefined, |
There was a problem hiding this comment.
Preserve quote sender metadata when quote text is empty
Fresh evidence in this commit: supplemental.quote is now only populated when entry.replyToBody is truthy, so replies to media-only/empty-text Signal messages lose ReplyToSender/quote context entirely even when sender visibility is allowed. This is a behavior regression from the previous path, which still forwarded quote metadata without requiring quote text, and it weakens prompt context for common non-text reply targets.
Useful? React with 👍 / 👎.
6e8d105 to
5ac45fc
Compare
|
Verification before landing:
|
Summary
Refactors channel SDK ownership toward the two primary channel families:
openclaw/plugin-sdk/channel-inboundnow owns inbound helper splinters: envelope formatting, location context, inbound roots, inbound logging, and direct-DM dispatch/access helpers.openclaw/plugin-sdk/channel-outboundnow owns outbound helper splinters: reply options, typing/ack logging, lifecycle helpers, and channel streaming/progress helpers.channel-envelope,channel-location,channel-inbound-roots,channel-pairing-paths,channel-reply-options-runtime,channel-logging,channel-streaming,direct-dm,direct-dm-access, andtest-utils.src/channels/*; old public files re-export only for compatibility.dispatchChannelInboundReply, ClickClack usesruntime.channel.inbound.dispatchReply, and Matrix/MS Teams helpers no longer mock deprecated prepared replies.runtime.channel.inbound.runPreparedReplyis now marked deprecated.direct-dm-guard-policyas a narrow live seam after review showed Nostr startup should not import broadchannel-inboundjust to construct pre-crypto guard policy.provider-selection-runtimeorprovider-auth-result; no cleaner replacement without worse ownership or build topology.origin/main(42ba297b0a) and migrated the remaining bundled extension test/support imports off deprecated SDK subpaths (command-auth,config-schema,config-types,reply-dedupe,telegram-command-config).actions/setup-node.transcriptstool-display metadata from the newer base and regenerated the OpenClawKit display resource for the PR merge ref.channel-streaming.Net LOC vs
origin/main:+5442/-5489, net-47.Verification
Behavior addressed: fewer public channel SDK seams, more channel helper ownership in
channel-inbound/channel-outbound, no bundled imports from newly deprecated channel helper subpaths.Real environment tested: local macOS source checkout.
Exact steps or command run after this patch:
CI=1 OPENCLAW_VITEST_MAX_WORKERS=1 OPENCLAW_TEST_PROJECTS_SERIAL=1 OPENCLAW_VITEST_FS_MODULE_CACHE_PATH=.artifacts/vitest-cache/channel-cleanup fnm exec --using v24.15.0 node scripts/run-vitest.mjs run extensions/feishu/src/bot.broadcast.test.ts extensions/feishu/src/bot.test.ts extensions/feishu/src/comment-handler.test.ts extensions/mattermost/src/mattermost/monitor.inbound-system-event.test.ts extensions/qa-channel/src/channel.test.ts extensions/zalouser/src/monitor.group-gating.test.ts src/gateway/server-restart-sentinel.test.ts src/plugins/contracts/extension-package-project-boundaries.test.tsnode --import tsx scripts/tool-display.ts --checkpnpm tool-display:checkbash -n .github/actions/setup-pnpm-store-cache/ensure-node.shopenclaw_node_download_platformandopenclaw_node_version_matchesCI=1 OPENCLAW_VITEST_MAX_WORKERS=1 OPENCLAW_TEST_PROJECTS_SERIAL=1 OPENCLAW_VITEST_FS_MODULE_CACHE_PATH=.artifacts/vitest-cache/image-tool-timeout fnm exec --using v24.15.0 node scripts/run-vitest.mjs run src/agents/tools/image-tool.test.ts --testNamePattern "iMessage account attachment roots|iMessage wildcard attachment roots"git diff --checknode scripts/check-deprecated-api-usage.mjspnpm check:deprecated-jsdocpnpm plugin-sdk:api:checkpnpm check:test-typesCI=1 OPENCLAW_VITEST_MAX_WORKERS=1 OPENCLAW_TEST_PROJECTS_SERIAL=1 OPENCLAW_VITEST_FS_MODULE_CACHE_PATH=.artifacts/vitest-cache/channel-refactor-touched fnm exec --using v24.15.0 node scripts/run-vitest.mjs run ...(22 touched extension files, 353 tests)node scripts/run-vitest.mjs run extensions/whatsapp/src/auto-reply.web-auto-reply.last-route.test.ts extensions/imessage/src/channel-inbound-roots.contract.test.tsnode scripts/run-vitest.mjs run src/plugin-sdk/channel-streaming.test.ts extensions/slack/src/monitor/message-handler/dispatch.preview-fallback.test.ts src/plugins/contracts/plugin-sdk-runtime-api-guardrails.test.ts src/plugins/contracts/plugin-sdk-subpaths.test.tsnode scripts/run-vitest.mjs run src/plugin-sdk/direct-dm.test.ts extensions/nostr/src/nostr-bus.inbound.test.ts extensions/nostr/src/channel.inbound.test.ts src/plugins/contracts/plugin-sdk-subpaths.test.tsnode scripts/run-vitest.mjs run src/plugin-sdk/provider-auth-result.test.ts src/plugin-sdk/provider-auth-runtime.test.tsnode scripts/run-vitest.mjs extensions/clickclack/src/inbound.test.ts src/plugin-sdk/inbound-reply-dispatch.test.ts extensions/discord/src/monitor/message-handler.process.test.ts extensions/slack/src/monitor/message-handler/dispatch.preview-fallback.test.ts extensions/matrix/src/matrix/monitor/handler.test.ts extensions/msteams/src/monitor-handler/message-handler.authz.test.tsnode scripts/check-deprecated-api-usage.mjs --rule=extension-plugin-sdk-compat-subpathsNODE_OPTIONS=--max-old-space-size=12288 pnpm buildenv -u OPENCLAW_TESTBOX pnpm check:changed.agents/skills/autoreview/scripts/autoreview --mode localEvidence after fix: all listed commands passed after the final accepted review fix; final autoreview reported no accepted/actionable findings.
Observed result after fix: deprecated usage guard rejects the newly deprecated public subpaths, bundled extension code no longer calls deprecated SDK/channel compatibility seams, and current branch LOC remains net negative.
What was not tested: live external channel roundtrips; this change is SDK topology/import ownership plus unit/build/changed-gate proof.