Skip to content

test(auto-reply): guard message-tool-only reply privacy#76661

Merged
steipete merged 1 commit into
mainfrom
codex/message-tool-reply-policy-tests
May 3, 2026
Merged

test(auto-reply): guard message-tool-only reply privacy#76661
steipete merged 1 commit into
mainfrom
codex/message-tool-reply-policy-tests

Conversation

@steipete

@steipete steipete commented May 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Problem: [codex] Fix group message-tool reply delivery #76647 attempted to fix group/channel message_tool replies by auto-routing normal final output, but that changes the documented delivery contract.
  • Why it matters: in message_tool_only, final/block/tool output is transcript-private; visible group/channel replies must come from the explicit message tool so send policy, tool auditability, and duplicate suppression stay intact.
  • What changed: added regression coverage for same-provider group/channel final replies and same-provider ACP final replies when an origin route exists.
  • What did NOT change (scope boundary): no runtime dispatch changes; this PR intentionally preserves the current message-tool-only privacy model.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor required for the fix
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

Root Cause (if applicable)

  • Root cause: N/A; this is a guardrail PR for the existing delivery contract.
  • Missing detection / guardrail: existing tests covered suppressed final delivery, but not the same-provider OriginatingChannel/OriginatingTo route shape from [codex] Fix group message-tool reply delivery #76647.
  • Contributing context: explicit message(action=send) already has same-origin target inference through the message tool context; normal final replies must remain private in message_tool_only mode.

Regression Test Plan (if applicable)

  • Coverage level that should have caught this:
    • Unit test
    • Seam / integration test
    • End-to-end test
    • Existing coverage already sufficient
  • Target test or file: src/auto-reply/reply/dispatch-from-config.test.ts, src/auto-reply/reply/dispatch-acp.test.ts
  • Scenario the test should lock in: same-provider group/channel origin routes do not make message_tool_only final ACP or normal dispatch replies visible.
  • Why this is the smallest reliable guardrail: it exercises the central dispatch seams that decide whether to call the dispatcher or routeReply.
  • Existing test that already covers this (if any): existing tests covered private message_tool_only turns without the same-provider origin route.
  • If no new test is added, why not: N/A

User-visible / Behavior Changes

None.

Diagram (if applicable)

message_tool_only final reply -> transcript/private result -> no dispatcher/routeReply delivery
message(action=send)          -> message tool target inference -> visible source chat delivery

Security Impact (required)

  • New permissions/capabilities? No
  • Secrets/tokens handling changed? No
  • New/changed network calls? No
  • Command/tool execution surface changed? No
  • Data access scope changed? No
  • If any Yes, explain risk + mitigation: N/A

Repro + Verification

Environment

  • OS: macOS local; Linux Testbox
  • Runtime/container: repo pnpm test wrapper
  • Model/provider: N/A
  • Integration/channel (if any): auto-reply group/channel dispatch, ACP dispatch
  • Relevant config (redacted): default message_tool_only group/channel delivery

Steps

  1. Run focused auto-reply dispatch tests.
  2. Run changed tests in Blacksmith Testbox.

Expected

  • Same-provider group/channel message_tool_only final replies are not queued through dispatcher or routeReply.
  • ACP message_tool_only final replies stay private even when an origin route exists.

Actual

  • Matches expected.

Evidence

  • Failing test/log before + passing after
  • Trace/log snippets
  • Screenshot/recording
  • Perf numbers (if relevant)

Human Verification (required)

  • Verified scenarios: local focused tests and Testbox changed test gate.
  • Edge cases checked: same-provider origin route for normal dispatch and ACP dispatch.
  • What you did not verify: live Discord/WhatsApp delivery.

Review Conversations

  • I replied to or resolved every bot review conversation I addressed in this PR.
  • I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

  • Backward compatible? Yes
  • Config/env changes? No
  • Migration needed? No
  • If yes, exact upgrade steps: N/A

Risks and Mitigations

None.

@steipete steipete mentioned this pull request May 3, 2026
Closed
@openclaw-barnacle openclaw-barnacle Bot added size: S maintainer Maintainer-authored PR labels May 3, 2026
@clawsweeper

clawsweeper Bot commented May 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

Summary
This PR adds regression tests that assert same-provider group/channel and ACP final replies stay private in message_tool_only mode when an origin route exists.

Reproducibility: not applicable. as a test-only guardrail PR rather than a bug report. Source and docs inspection gives high confidence that the intended behavior is current-main behavior, and the proposed tests target the missing same-provider origin-route coverage.

Next step before merge
No repair lane: this is a protected-label, test-only PR with no concrete blocker for automation to fix.

Security
Cleared: The diff only adds regression tests and does not touch runtime code, dependencies, workflows, secrets, permissions, or package metadata.

Review details

Best possible solution:

Land the focused regression tests after normal maintainer review and CI/Testbox validation, preserving the existing message-tool-only privacy contract.

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

Not applicable as a test-only guardrail PR rather than a bug report. Source and docs inspection gives high confidence that the intended behavior is current-main behavior, and the proposed tests target the missing same-provider origin-route coverage.

Is this the best way to solve the issue?

Yes. Adding focused coverage at the normal dispatch and ACP dispatch seams is the narrowest maintainable solution because it preserves the documented delivery contract without changing runtime behavior.

What I checked:

Likely related people:

  • steipete: Recent main-branch commits cover source reply delivery mode, tool-only ACP typing, visible-reply warnings, and dispatch refactors in the affected files. (role: recent maintainer; confidence: high; commits: 67b16a4a6d25, 249cb5437364, 829153771067; files: src/auto-reply/reply/source-reply-delivery-mode.ts, src/auto-reply/reply/dispatch-from-config.ts, src/auto-reply/reply/dispatch-acp.ts)
  • scoootscooob: The current group/channel tool-only visible-reply default appears in history at the commit that rewrote group reply handling for messages.groupChat.visibleReplies. (role: introduced behavior; confidence: medium; commits: 3c636208b0a0; files: src/auto-reply/reply/source-reply-delivery-mode.ts, src/auto-reply/reply/dispatch-from-config.ts, docs/channels/groups.md)
  • osolmaz: The ACP runtime refactor history includes delivered-text visibility and duplicate direct fallback reply work near the ACP delivery path this PR guards. (role: adjacent owner; confidence: medium; commits: 154a7edb7cad; files: src/auto-reply/reply/dispatch-acp.ts, src/auto-reply/reply/dispatch-acp-delivery.ts)

Remaining risk / open question:

  • Focused tests were not run in this read-only review; CI or the stated Testbox gate should still validate the PR branch before merge.

Codex review notes: model gpt-5.5, reasoning high; reviewed against e0a83c00461c.

@steipete
steipete merged commit 6e2f394 into main May 3, 2026
103 of 106 checks passed
@steipete
steipete deleted the codex/message-tool-reply-policy-tests branch May 3, 2026 12:12
@steipete

steipete commented May 3, 2026

Copy link
Copy Markdown
Contributor Author

Landed via rebase onto main.

  • Focused local gate: pnpm test src/auto-reply/reply/dispatch-from-config.test.ts src/auto-reply/reply/dispatch-acp.test.ts src/auto-reply/reply/dispatch-acp-delivery.test.ts passed.
  • Testbox changed gate: OPENCLAW_TESTBOX=1 pnpm test:changed passed on tbx_01kqpvecpm4ph7bts31rat5eh8.
  • Source PR SHA: 742a22f
  • Landed SHA: 6e2f394

@clawsweeper clawsweeper Bot mentioned this pull request May 5, 2026
Closed
3 tasks
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

maintainer Maintainer-authored PR size: S

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@steipete