fix(messages): keep Codex source replies tool-gated#83571
Conversation
openclaw-barnacle
Bot
added
docs
|
Codex review: needs real behavior proof before merge. Workflow note: Future ClawSweeper reviews update this same comment in place. How this review workflow works
Summary Reproducibility: yes. at source level. Current main sets the Codex harness default to PR rating Rank-up moves:
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. Real behavior proof Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge Security Review detailsBest possible solution: Refresh the branch, confirm maintainer acceptance of the Codex direct-chat default change, attach redacted real direct-channel proof, and then land the bounded harness-default removal with the existing docs/changelog coverage. Do we have a high-confidence way to reproduce the issue? Yes, at source level. Current main sets the Codex harness default to Is this the best way to solve the issue? Yes, mostly. Removing the Codex plugin-local default is the narrow owner-boundary fix and preserves generic harness default compatibility, but the default-policy and upgrade behavior need explicit maintainer acceptance plus real transport proof. Label justifications:
Acceptance criteria:
What I checked:
Likely related people:
Codex review notes: model gpt-5.5, reasoning high; reviewed against 57c952f67985. |
clawsweeper
Bot
added
rating: 🧂 unranked krab
Mantis Telegram Desktop ProofSummary: Mantis did not generate before/after GIFs because this PR does not have a clean Telegram-visible before/after proof in the standard Mantis run.
|
06272c7 to
f24741b
Compare
steipete
changed the title
…ce across all channel ingresses Sweeps the same root-cause fix as PR #86825 (Mattermost) across the remaining channels that exhibited the latent gap: Feishu, IRC, Line, MS Teams, Nextcloud Talk, Signal, ClickClack, QA Channel, and the public 'direct-dm' plugin SDK helper (with Nostr passing through it as the first internal consumer). Each channel inbound handler already computed CommandAuthorized for typed slash commands but never set CommandSource. Without CommandSource: 'text', resolveCommandTurnContext defaults the turn kind to 'normal', so the explicit- command exception in source-reply-delivery-mode.ts:54-56 never fires and acknowledgements ('Session reset.', 'New session started.', 'Stopped N subagents', etc.) are silently dropped under message_tool_only delivery modes — most commonly the Codex harness default for DMs that #83571 introduced. For each channel, this PR sets: CommandSource: commandAuthorized && hasControlCommand ? 'text' : undefined where 'hasControlCommand' uses the channel's existing isControlCommandMessage helper (not shouldComputeCommandAuthorized, which would be too permissive and incorrectly tag inline command tokens within ordinary text). For ClickClack and QA Channel (CommandAuthorized hardcoded true), the tag only depends on the body being a control command. For Signal and Line, a new optional 'hasControlCommand' field on the entry/params struct carries the signal from the access-policy layer where it is already computed. The public direct-dm SDK helper grows an optional commandSource parameter so third-party plugins built on it can pass through their own control-command detection; Nostr is updated as the first consumer. Adds focused regression tests (Signal, MS Teams, direct-dm SDK, plus a negative-control assertion on the existing MS Teams inline-command test) that fail without the fix and pass with it. Fixes #86664.
…ce across all channel ingresses Sweeps the same root-cause fix as PR #86825 (Mattermost) across the remaining channels that exhibited the latent gap: Feishu, IRC, Line, MS Teams, Nextcloud Talk, Signal, ClickClack, QA Channel, and the public 'direct-dm' plugin SDK helper (with Nostr passing through it as the first internal consumer). Each channel inbound handler already computed CommandAuthorized for typed slash commands but never set CommandSource. Without CommandSource: 'text', resolveCommandTurnContext defaults the turn kind to 'normal', so the explicit- command exception in source-reply-delivery-mode.ts:54-56 never fires and acknowledgements ('Session reset.', 'New session started.', 'Stopped N subagents', etc.) are silently dropped under message_tool_only delivery modes — most commonly the Codex harness default for DMs that #83571 introduced. For each channel, this PR sets: CommandSource: commandAuthorized && hasControlCommand ? 'text' : undefined where 'hasControlCommand' uses the channel's existing isControlCommandMessage helper (not shouldComputeCommandAuthorized, which would be too permissive and incorrectly tag inline command tokens within ordinary text). For ClickClack and QA Channel (CommandAuthorized hardcoded true), the tag only depends on the body being a control command. For Signal and Line, a new optional 'hasControlCommand' field on the entry/params struct carries the signal from the access-policy layer where it is already computed. The public direct-dm SDK helper grows an optional commandSource parameter so third-party plugins built on it can pass through their own control-command detection; Nostr is updated as the first consumer. Adds focused regression tests (Signal, MS Teams, direct-dm SDK, plus a negative-control assertion on the existing MS Teams inline-command test) that fail without the fix and pass with it. Fixes #86664.
1 participant
Summary
messages.visibleReplies: "automatic"and document it.channels.modelByChannel.<provider>["*"]apply to direct chats before harness delivery defaults.Verification
Behavior addressed: Codex-backed direct/source chats stay message-tool-only by default, including first turns before a session entry;
messages.visibleReplies: "automatic"restores automatic final delivery; channel/default/session/parent/one-turn/heartbeat model overrides can select a non-Codex runtime before Codex delivery defaults suppress final text.Real environment tested: local macOS focused tests; Blacksmith Testbox through Crabbox
tbx_01krxhbnxfdtbfdezc04g8j3na.Exact steps or command run after this patch:
pnpm test src/auto-reply/reply/dispatch-from-config.test.ts src/channels/model-overrides.test.ts extensions/codex/index.test.ts -- --reporter=verbose;OPENCLAW_TEST_TRUST_BUNDLED_PLUGINS_DIR=1 OPENCLAW_BUNDLED_PLUGINS_DIR=extensions pnpm prompt:snapshots:check;git diff --check;AUTOREVIEW_OPENCLAW_MAINTAINER_VALIDATION=1 codex --dangerously-bypass-approvals-and-sandbox --sandbox danger-full-access review --uncommitted;node scripts/crabbox-wrapper.mjs run --provider blacksmith-testbox --blacksmith-org openclaw --blacksmith-workflow .github/workflows/ci-check-testbox.yml --blacksmith-job check --blacksmith-ref main --idle-timeout 90m --ttl 240m --timing-json -- CI=1 NODE_OPTIONS=--max-old-space-size=4096 OPENCLAW_TEST_PROJECTS_PARALLEL=6 OPENCLAW_VITEST_MAX_WORKERS=1 OPENCLAW_VITEST_NO_OUTPUT_TIMEOUT_MS=900000 OPENCLAW_TESTBOX=1 OPENCLAW_TESTBOX_REMOTE_RUN=1 pnpm check:changedEvidence after fix: focused tests passed (126 auto-reply, 7 channels, 6 extensions); prompt snapshots current; final autoreview exited with no accepted/actionable findings; Testbox changed gate passed at https://github.com/openclaw/openclaw/actions/runs/26033838659.
Observed result after fix: Codex direct/source defaults are message-tool-only unless config explicitly sets automatic, while selected non-Codex model/channel/default/session/parent/one-turn/heartbeat overrides keep automatic final delivery.
What was not tested: live Telegram/Signal/Codex gateway roundtrip.