fix(agents): deliver native /compact replies through source suppression#90212
Conversation
|
Codex review: needs maintainer review before merge. Reviewed June 8, 2026, 11:59 PM ET / 03:59 UTC. Summary PR surface: Source 0, Tests +75. Total +75 across 18 files. Reproducibility: yes. source-reproducible: dispatch suppresses message_tool_only final replies unless an explicit command payload carries deliverDespiteSourceReplySuppression, and current main still has an unmarked native slash inline-action reply path. Review metrics: 1 noteworthy metric.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the narrow core delivery fix after maintainer acceptance of the command-reply visibility semantic, then close the linked user report after merge with the WebChat proof and focused tests. Do we have a high-confidence way to reproduce the issue? Yes, source-reproducible: dispatch suppresses message_tool_only final replies unless an explicit command payload carries deliverDespiteSourceReplySuppression, and current main still has an unmarked native slash inline-action reply path. Is this the best way to solve the issue? Yes, with maintainer acceptance of the semantic: the PR reuses the existing command-reply marker and central dispatch gates instead of adding a Feishu-only or direct-channel bypass. AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against 4c55dd85499b. Label changesLabel justifications:
Evidence reviewedPR surface: Source 0, Tests +75. Total +75 across 18 files. View PR surface stats
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
|
625b7ad to
df98fff
Compare
61eca5a to
e62637f
Compare
7774c0a to
79e4ebf
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review Updated the PR body for current head
|
|
🦞👀 Command router queued. I will update this comment with the next step. Re-review progress:
|
|
Refreshed this PR onto latest main and resolved the only merge conflict in src/commands/migrate/skill-selection-prompt.ts. The resolution keeps latest-main deprecation wording and does not alter the /compact delivery path. Updated the PR body with current head 83771dc and focused validation. @clawsweeper re-review |
|
🦞👀 Command router queued. I will update this comment with the next step. |
|
The actionlint check failure on the previous refreshed head was a GitHub release download 504 while installing actionlint, not a repository lint error. I amended the merge commit without content changes to rerun checks on current head 6897161 and updated the PR body head reference accordingly. |
|
The previous refreshed head hit another external install 504 in the OpenGrep changed-path scanner. I amended again without content changes to rerun checks on current head a7b5127; PR body head reference is updated. |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Updated this PR to current head 7809a85 after merging the latest main. The new commit stabilizes the SIGTERM local-agent wait in src/commands/agent-via-gateway.test.ts after the refreshed CI exposed a shard-order timing flake; it does not change the /compact delivery production path. Local focused validation passed, including the full agentic-commands-agent-channel shard (32 files / 313 tests), auto-reply focused tests, Feishu bot tests, unit-fast security test, tsgo, oxfmt, and diff check. @clawsweeper re-review |
|
🦞👀 Command router queued. I will update this comment with the next step. |
|
Merged the latest main again and resolved the new conflict in src/commands/agent-via-gateway.test.ts by preserving both test-stability fixes: the PR-side per-test initialCalls baseline and upstream's abortListenerAttached readiness wait before SIGTERM. Production /compact delivery logic is unchanged. Current head is 881ce6d. Local validation passed: agent-via-gateway.test.ts (55 tests), full agentic-commands-agent-channel shard (32 files / 313 tests), auto-reply focused tests (203 tests), Feishu bot tests (74 tests), unit-fast security test, tsgo, oxfmt, and diff check. @clawsweeper re-review |
|
🦞👀 Command router queued. I will update this comment with the next step. |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Updated this PR on top of the latest Conflict resolution notes:
Verification:
@clawsweeper re-review |
|
🦞👀 Command router queued. I will update this comment with the next step. Re-review progress:
|
|
Landed via rebase onto main.
Thanks @snowzlm! |
Summary
Addresses the native
/compactreply-loss path from #90185 and the related compaction notification lane where configured hook messages must not suppress the built-innotifyUsernotices.This PR covers these two confirmed paths:
/compactfast-path replies could return before the full inline-action command marker was applied. Channels usingmessage_tool_onlysource-reply delivery then treated the terminal command reply as ordinary source content and suppressed it, producingqueuedFinal=false, replies=0even though compaction ran.notifyUsercompaction notice because the notice lived behind anelse ifbranch.This PR intentionally does not claim to fix the separate CardKit streaming/rendering corruption mentioned around the issue; that remains separate scope if maintainers want it handled in another patch.
Root cause
Native slash commands can be handled before workspace bootstrap and before the normal inline command path. The full inline command path already marks terminal command replies with
deliverDespiteSourceReplySuppression; the native slash fast path did not. That meant an explicit/compactresult could be generated but later dropped by source-reply delivery policy in message-tool-only channels.The compaction notice path had a separate branch-ordering issue: hook messages and the built-in
notifyUsernotice were mutually exclusive even though they are independent user-visible lanes.Changes
/compact, directive replies, and inline replies returned from the fast path.notifyUsernotices independent./compactcommand-handler replies are marked for message-tool-only delivery./compactrouting coverage from the earlier patch.src/commands/migrate/skill-selection-prompt.tsby keeping the compatibility alias explanation and latest-main deprecation wording. That merge-only follow-up does not alter the/compactdelivery path.src/commands/agent-via-gateway.test.tsSIGTERM local-run waits after the latest main merge exposed a shard-order timing flake; production/compactdelivery logic is unchanged.mainagain and combine both SIGTERM test-stability fixes insrc/commands/agent-via-gateway.test.ts: the PR keeps the per-testinitialCallsbaseline and the upstreamabortListenerAttached.promisereadiness wait. Production/compactdelivery logic remains unchanged.Real behavior proof
Behavior or issue addressed: visible reply loss for native
/compact, plus the related compaction notice lane where hook messages and built-innotifyUsernotices must both remain deliverable.Real environment tested: Linux local OpenClaw checkout running the current PR head
881ce6d64c8f4a5ca4edd160a652f39688553abdas an isolated local gateway, connected by a WebChat WebSocket client. The model backend was a local mock OpenAI Responses provider so the proof did not depend on external model credentials. Gateway token, device identity material, loopback URL, and local filesystem paths are redacted/omitted.Exact steps or command run after this patch: Started the PR-head gateway in isolated local mode, connected a WebChat WebSocket client, approved the local WebChat device, sent
/compactthroughchat.send, waited for the finalchatevent, then read WebChat history. The captured command wasnode --import tsx .artifacts/proof-90185-live-webchat.tsagainst the current PR checkout.Evidence after fix: Redacted live WebChat terminal output from the current PR head:
{ "head": "881ce6d64c8f4a5ca4edd160a652f39688553abd", "mode": "isolated local WebChat websocket live run", "request": { "sessionKey": "main", "message": "/compact", "idempotencyKey": "proof-90185-1780629173685" }, "connect": { "ok": true, "payloadType": "hello-ok", "auth": "<redacted>" }, "send": { "ok": true, "payload": { "runId": "proof-90185-1780629173685", "status": "started" } }, "finalEvent": { "event": "chat", "payload": { "runId": "proof-90185-1780629173685", "sessionKey": "agent:main:main", "state": "final", "message": { "role": "assistant", "content": [ { "type": "text", "text": "⚙️ Compaction skipped: no real conversation messages yet • Context ?/128k" } ] } } }, "historySummary": { "ok": true, "messageCount": 2, "compactMessageCount": 2 } }Observed result after fix: The current head delivered a visible WebChat assistant reply for
/compactafter dispatch:⚙️ Compaction skipped: no real conversation messages yet • Context ?/128k. WebChat history contained bothuser: /compactand the assistant compaction reply, so the oldmessage_tool_onlysilent-drop path is no longer reproduced.What was not tested: A live Feishu tenant screenshot/video was not attached. WebChat was used as the live transport-visible proof path requested by ClawSweeper, with sensitive runtime details redacted.
Before-fix reproduction / root-cause proof
A source-level reproduction compared the current base (
upstream/main) with this PR head:{ "baseRef": "81eee470451d6039f899e52397111eea19926d80", "headRef": "881ce6d64c8f4a5ca4edd160a652f39688553abd", "reproduction": { "baseNativeSlashFastPathHasDeliveryMarker": false, "headNativeSlashFastPathHasDeliveryMarker": true, "dispatchRequiresMarkerForSuppressedDelivery": true } }Relevant base excerpt from
src/auto-reply/reply/get-reply-native-slash-fast-path.ts:That is the broken path: the native slash fast-path produced a terminal
/compactreply, but returned it without the explicit-command delivery marker. Dispatch already requiresdeliverDespiteSourceReplySuppressionto bypassmessage_tool_onlysource suppression, so an unmarked native slash reply can be generated and then dropped.Relevant PR-head excerpt:
Hook messages + built-in notifyUser notices
The second path is covered by the focused regression test added in this PR:
Expected visible sequence under
compaction.notifyUser: trueand hook-provided messages:This verifies hook messages are additive and no longer suppress the built-in
notifyUsercompaction notices.Validation after fix
Current-head local focused validation after merging latest upstream
main:Current GitHub PR status for
881ce6d64c8f4a5ca4edd160a652f39688553abdimmediately after the latest-main merge-resolution push:Additional validation for the shard exposed by CI after the latest main merge:
Risk / safety
sendPolicy: denybehavior because final dispatch still enforces send policy.Notes
The issue references separate CardKit streaming rendering problems via related issues. This PR fixes the
/compactreply-loss path and the compaction notice suppression path tracked here; it does not modify unrelated streaming card rendering code.