Skip to content

fix(discord): guard message-tool-only final delivery#84485

Closed
anyech wants to merge 16 commits into
openclaw:mainfrom
anyech:fix/discord-message-tool-only-runtime-guard
Closed

fix(discord): guard message-tool-only final delivery#84485
anyech wants to merge 16 commits into
openclaw:mainfrom
anyech:fix/discord-message-tool-only-runtime-guard

Conversation

@anyech

@anyech anyech commented May 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Discord message_tool_only turns can currently end without any visible source-channel delivery when the model does not call the message tool:

  • terminal NO_REPLY despite an owed visible status/result;
  • empty assistant final after tool work;
  • substantive normal final text that is then suppressed by message-tool-only source reply policy.

Prompt/tool-description guidance helps, but it is advisory. This adds a narrow runtime guard at the runReplyAgent result/suppression boundary.

What changed

  • Detect Discord sourceReplyDeliveryMode: "message_tool_only" turns with no committed successful message-tool/media/target delivery evidence.
  • For substantive final payloads, mark them with existing deliverDespiteSourceReplySuppression metadata so dispatch can surface them instead of silently suppressing them.
  • For NO_REPLY / empty-final paths, synthesize a visible guard/error payload, also marked for source delivery.
  • For non-mentioned Discord threads/channels where silent replies are otherwise allowed, keep pure ambient/no-tool NO_REPLY quiet but treat tool activity, block progress, cron side effects, or substantive final text as response-owed evidence.
  • Also cover Discord tool/progress activity followed by empty/NO_REPLY finals when sourceReplyDeliveryMode is not message_tool_only; real thread traces showed that path can occur while visible progress/tool chatter is emitted but no final/status answer is delivered.
  • Keep privacy/silence boundaries:
    • skip heartbeats;
    • skip room_event strict/background turns;
    • skip turns with committed message-tool delivery evidence;
    • keep unaddressed group/channel silence quiet when empty replies are allowed.

Related context

This complements, rather than replaces, nearby work:

Real behavior proof

  • Behavior addressed: Discord message_tool_only turns should no longer end with no visible source-channel reply when a directly addressed turn produces NO_REPLY, an empty final, or normal final text without a successful message.send.
  • Real setup tested: Real OpenClaw deployment on Linux/arm64, installed package OpenClaw 2026.5.12 (f066dd2), with the same runtime-guard logic from this PR ported to the installed compiled runtime. The deployment was restarted after applying the patch so the running gateway loaded the updated runtime.
  • Exact steps or command run after this patch:
    1. Applied the PR-shaped runtime guard to the installed runtime bundle.
    2. Restarted the real gateway process.
    3. Ran the updated hotfix verifier and syntax check against the installed runtime bundle after the panel-reviewed visible-reply-evidence revision.
    4. Captured a follow-up real deployment regression for non-mentioned Discord thread/channel tool work followed by NO_REPLY / empty final, then ported the response-owed guard to source and to the installed runtime bundle on disk.
    5. Ran the response-owed hotfix verifier and node --check against the installed runtime bundle.
    6. Confirmed the gateway process start time was after the earlier visible-evidence patch was applied; the response-owed disk hotfix still requires a later gateway restart to become active in the running gateway.
  • Evidence after fix: Redacted terminal output from the real deployment:
$ openclaw --version
OpenClaw 2026.5.12 (f066dd2)

$ ps -eo pid,lstart,cmd | grep '[n]ode .*openclaw.*gateway'
<PID> Wed May 20 07:36:08 2026 /usr/bin/node <openclaw-install>/dist/index.js gateway --port <gateway-port>

$ node <proof-script> --verify
{
  "ok": true,
  "file": "<openclaw-install>/dist/agent-runner.runtime-<hash>.js",
  "verification": {
    "version": "2026.5.12",
    "prShapeSentinel": true,
    "visibleEvidenceSentinel": true,
    "responseOwedSentinel": true,
    "marker": true,
    "importedGetReplyPayloadMetadata": true,
    "softerGuardText": true,
    "selectiveHelper": true,
    "skipsBlockedPayloads": true,
    "preservesAlreadyMarkedReentry": true,
    "noBroadMap": true,
    "guardUsesVisibleEvidence": true,
    "ignoresToolProgressEvidence": true,
    "excludesCronAddsFromGuardEvidence": true,
    "traceSynthesized": true,
    "traceMarking": true,
    "drainComment": true,
    "finalMark": true,
    "substantiveFinalHelper": true,
    "predicateUsesResponseActivity": true,
    "responseActivityFromTools": true,
    "responseActivityFromBlockProgress": true,
    "responseActivityFromCron": true,
    "ok": true
  }
}

$ node --check <openclaw-install>/dist/agent-runner.runtime-<hash>.js
exit status: 0
  • Observed result after fix: After the earlier restart, the real deployment was running with the PR-shaped runtime guard loaded. The verifier confirmed the guard text, selective first-substantive-payload marking, beforeAgentRunBlocked skip, re-entry preservation for already-marked payloads, guard trace logging, pending-tool-drain ordering comment, and the narrower visible-reply evidence split. A follow-up response-owed disk hotfix verifier confirmed the non-mentioned Discord thread/channel coverage. A later real trace showed an additional gap where sourceReplyDeliveryMode was not message_tool_only and toolSummary was absent even though visible tool/progress chatter existed; the latest source update adds a regression test for that path and treats successful progress/side-effect activity as response-owed activity while still keeping pure ambient/no-tool silence quiet.
  • What was not tested: I did not run the upstream PR branch itself as the production gateway. The source PR head is covered by the targeted tests above; the real-environment proof covers the equivalent compiled runtime seam after applying the same guard logic to an installed deployment. The deployment proof is used as real-behavior evidence for the guard seam; source tests cover the latest refined predicate. Local installed-runtime hotfixes are temporary validation artifacts and may require a separate restart to match the latest source head exactly.

Latest v3 refinement

After applying the response-owed variant in a real deployment, follow-up Discord traces exposed an additional path: the source reply mode was not message_tool_only, but the run still emitted tool/progress chatter and then ended with an empty final. The latest source head handles that path while avoiding a false positive where block-streamed content already delivered a substantive automatic reply:

  • Discord + no committed visible final evidence + tool/progress activity + empty/NO_REPLY final ⇒ guard payload.
  • Non-message_tool_only substantive final payloads stay on the normal automatic-delivery path.
  • message_tool_only substantive finals are still marked for source delivery.
  • Room events, explicit silent-expected turns, committed message.send evidence, and approval prompts still skip the guard.

Validation

Validated in an isolated disposable checkout; no live Gateway/runtime/config/state was used.

corepack pnpm exec oxfmt --check --threads=1 \
  src/auto-reply/reply/agent-runner.ts \
  src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.ts

node scripts/run-vitest.mjs run --config test/vitest/vitest.e2e.config.ts \
  src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.ts

node scripts/run-oxlint.mjs --tsconfig config/tsconfig/oxlint.core.json \
  src/auto-reply/reply/agent-runner.ts \
  src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.ts

Result:

All matched files use the correct format.
1 test file passed; 67 tests passed.
Found 0 warnings and 0 errors.
Typecheck for the core test tsconfig passed.

@openclaw-barnacle openclaw-barnacle Bot added size: M triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 20, 2026
@clawsweeper

clawsweeper Bot commented May 20, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 8, 2026, 5:46 PM ET / 21:46 UTC.

Summary
The branch adds a Discord-specific runReplyAgent delivery guard plus block-reply evidence tracking and E2E coverage so message-tool-only or response-owed Discord turns without confirmed visible source delivery return a generic marked guard instead of ending silently.

PR surface: Source +245, Tests +959. Total +1204 across 6 files.

Reproducibility: yes. The PR branch contains targeted runReplyAgent E2E cases for Discord NO_REPLY, empty final, message-send evidence, approval prompts, unmentioned silence, tool/progress activity, and block-streamed replies; the PR body also includes redacted real-runtime and Discord sandbox evidence.

Review metrics: 1 noteworthy metric.

  • Delivery policy surface: 1 Discord runtime guard added, 0 config surfaces added. The PR changes when suppressed Discord source turns become visible without adding an operator opt-in knob, so maintainer acceptance matters 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:

  • [P2] Maintainers should explicitly accept or reject the Discord guard policy before merge.

Risk before merge

  • [P1] Merging intentionally changes some Discord message_tool_only and response-owed empty/NO_REPLY turns from quiet or suppressed completion into a visible generic error guard, so maintainers should explicitly accept that policy shift.
  • [P1] Because the predicate sits in shared runReplyAgent delivery logic, a false positive could create an unexpected or duplicate Discord source reply even though the current test coverage covers the reviewed edge cases.

Maintainer options:

  1. Accept the Discord guard policy (recommended)
    Proceed if maintainers want this generic visible guard as the compatibility break for Discord turns that otherwise look silently completed.
  2. Tighten the response-owed predicate
    Require a narrower source-evidence or addressed-turn rule before merge if unexpected Discord replies are not acceptable.
  3. Pause behind unified recovery direction
    Close or pause this branch if the project wants the broader stranded-final retry design to own this class instead of a Discord-specific runtime guard.

Next step before merge

  • [P2] The remaining action is maintainer acceptance of the compatibility-sensitive Discord delivery policy plus normal exact-head merge checks; there is no narrow automated code repair to queue.

Maintainer decision needed

  • Question: Should OpenClaw accept a Discord-specific visible guard when a message-tool-only or response-owed Discord turn has work/progress but no confirmed current-source delivery?
  • Rationale: The implementation appears bounded and privacy-conscious, but it changes the user-visible semantics of message-tool-only suppression and empty Discord turns, which is an intentional product/compatibility choice rather than a mechanical code defect.
  • Likely owner: steipete — Recent merged history shows direct ownership of the message-tool-only privacy baseline and empty-reply delivery policy in this area.
  • Options:
    • Accept bounded Discord guard (recommended): Merge after normal exact-head merge checks, accepting that these Discord no-delivery cases should show a generic visible guard instead of staying silent.
    • Narrow before merge: Ask for a stricter predicate or default-off policy if maintainers want fewer behavior changes for ambient or non-mentioned Discord turns.
    • Defer to broader recovery: Pause this PR if maintainers want one unified stranded-final recovery policy owned by the broader message-tool-only recovery work instead of a Discord-specific guard.

Security
Cleared: No concrete security or supply-chain concern was found; the diff changes runtime reply predicates/tests and keeps private final text out of the visible guard payload.

Review details

Best possible solution:

Land this only if maintainers want the bounded Discord guard policy; otherwise narrow it behind an approved delivery rule or leave the broader stranded-final recovery work to the separate canonical recovery discussion.

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

Yes. The PR branch contains targeted runReplyAgent E2E cases for Discord NO_REPLY, empty final, message-send evidence, approval prompts, unmentioned silence, tool/progress activity, and block-streamed replies; the PR body also includes redacted real-runtime and Discord sandbox evidence.

Is this the best way to solve the issue?

Yes if maintainers accept the policy: the guard lives at the delivery decision boundary, uses existing suppression-bypass metadata, and avoids publishing private final text. The remaining uncertainty is product/compatibility acceptance, not a concrete patch defect found in this review.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 7d82e5b65895.

Label changes

Label justifications:

  • P2: The PR targets a real Discord message-delivery failure with a bounded but compatibility-sensitive runtime blast radius.
  • merge-risk: 🚨 compatibility: Existing Discord message-tool-only or silent-allowed turns may now emit a visible guard where current behavior could stay quiet.
  • merge-risk: 🚨 message-delivery: The changed predicate decides whether Discord replies are emitted, suppressed, or treated as already delivered, which can affect missing or duplicate messages.
  • 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 (terminal): The PR body supplies redacted real-runtime terminal proof plus a Discord sandbox send-evidence case, and the current head has the Real behavior proof check passing.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body supplies redacted real-runtime terminal proof plus a Discord sandbox send-evidence case, and the current head has the Real behavior proof check passing.
Evidence reviewed

PR surface:

Source +245, Tests +959. Total +1204 across 6 files.

View PR surface stats
Area Files Added Removed Net
Source 2 247 2 +245
Tests 4 969 10 +959
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 6 1216 12 +1204

What I checked:

  • AGENTS policy read and applied: The root policy was read fully and treats fallback/message-delivery behavior as compatibility-sensitive, which is why this review keeps the PR in maintainer-decision territory rather than auto-closing or auto-repairing it. (AGENTS.md:33, 7d82e5b65895)
  • Guard predicate added on PR head: The PR head adds isDiscordMessageToolOnlyReplyGuardCandidate, which skips heartbeats, room_event, explicit silence, and already-visible evidence, then limits the guard to Discord hints and message-tool-only or response-activity empty-final cases. (src/auto-reply/reply/agent-runner.ts:270, 1c14c66889f8)
  • Current-source delivery evidence checked before guard: The PR computes current-source message-tool evidence, substantive block delivery, and direct block delivery before deciding to synthesize the Discord guard, so it is not only checking the raw final payload list. (src/auto-reply/reply/agent-runner.ts:2161, 1c14c66889f8)
  • Guard payload stays on the existing suppression-bypass contract: Current main dispatch only bypasses source-reply suppression for payloads carrying deliverDespiteSourceReplySuppression, while sendPolicy: deny still wins; the PR uses that existing metadata path for the generic guard payload. (src/auto-reply/reply/dispatch-from-config.ts:3904, 7d82e5b65895)
  • Current main does not already implement this Discord guard: Current main still resolves non-internal room events to message_tool_only and makes sourceReplyDeliveryMode: message_tool_only suppress automatic source delivery; no Discord delivery-guard text or predicate exists on main. (src/auto-reply/reply/source-reply-delivery-mode.ts:70, 7d82e5b65895)
  • Block-streaming evidence added on PR head: The PR extends BlockReplyPipeline with didObserveBlockReply and didStreamSubstantiveReply, allowing the guard to distinguish status/progress chatter from substantive visible block delivery. (src/auto-reply/reply/block-reply-pipeline.ts:20, 1c14c66889f8)

Likely related people:

  • steipete: Authored and merged the strict message_tool_only baseline and recently merged broad empty-reply hardening in the same auto-reply delivery surface. (role: recent area contributor and merger; confidence: high; commits: 575936473d13, 2c31af136429; files: src/auto-reply/reply/agent-runner.ts, src/auto-reply/reply/source-reply-delivery-mode.ts, src/auto-reply/reply/dispatch-from-config.ts)
  • vincentkoc: Merged and reviewed recent block-streaming suppression work, and authored adjacent block reply pipeline hardening used by this PR's delivery-evidence changes. (role: adjacent owner; confidence: high; commits: 9549545dd0a0, 6c85b90469f9; files: src/auto-reply/reply/block-reply-pipeline.ts, src/auto-reply/reply/agent-runner-payloads.ts)
  • 100yenadmin: Authored the open broader recovery PR for message-tool-only stranded finals and recently landed adjacent Codex-output preservation work in the agent reply area. (role: adjacent recovery contributor; confidence: medium; commits: 414ecd2b9689; files: src/auto-reply/reply/agent-runner.ts, src/auto-reply/reply/followup-runner.ts)
  • martingarramon: Reviewed this PR's room_event boundary question and agreed that room_event is the right synthetic/non-conversational exclusion for this guard. (role: reviewer; confidence: medium; files: src/auto-reply/reply/agent-runner.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 (3 earlier review cycles)
  • reviewed 2026-07-03T15:48:16.570Z sha def627d :: needs maintainer review before merge. :: none
  • reviewed 2026-07-05T13:14:27.419Z sha a8b7e2d :: needs maintainer review before merge. :: none
  • reviewed 2026-07-08T21:38:12.242Z sha 1c14c66 :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels May 20, 2026
@clawsweeper

clawsweeper Bot commented May 20, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper PR egg

🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat.

Where did the egg go?
  • The egg game starts only after the PR passes the real-behavior proof check.
  • Before that, no creature or rarity is rolled. The treat waits for real proof.
  • This is still just collectible flavor: proof affects review readiness, not creature quality.

@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 20, 2026
@clawsweeper clawsweeper Bot added P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. labels May 20, 2026
@anyech
anyech force-pushed the fix/discord-message-tool-only-runtime-guard branch from b93852b to 788ec23 Compare May 20, 2026 08:23

@martingarramon martingarramon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This catches a real Discord message_tool_only gap: if the model finishes without message.send, the turn should not silently collapse into NO_REPLY.

From the candidacy check, the guard is skipped for heartbeats, visible reply evidence, non-message_tool_only delivery, room_event, and silentExpected. It also respects allowEmptyAssistantReplyAsSilent, except for directly addressed turns. The tests cover the positive and suppression paths well, especially committed sends and approval prompts.

One question: does currentInboundEventKind === "room_event" cover all non-conversational inbound event kinds that should bypass this, or are there other synthetic/internal kinds worth excluding?

LGTM.

@anyech
anyech force-pushed the fix/discord-message-tool-only-runtime-guard branch 3 times, most recently from 77fd52d to ecac27b Compare May 20, 2026 22:33
@anyech

anyech commented May 22, 2026

Copy link
Copy Markdown
Contributor Author

Thanks, good question.

For the current inbound-turn taxonomy, yes: room_event is the synthetic/non-conversational bucket that should bypass this guard. The gateway protocol schema currently allows only user_request and room_event for inboundTurnKind, and the MCP request normalization likewise drops anything outside those two values. So this PR intentionally special-cases room_event rather than trying to infer broader non-conversational behavior from looser strings.

If we add another synthetic/internal inbound kind later, it should either be normalized into room_event for this delivery path or added explicitly to the bypass predicate/tests at the same time. I kept the guard narrow here so ordinary directly-addressed message_tool_only Discord turns still fail visibly instead of disappearing behind NO_REPLY.

@martingarramon

Copy link
Copy Markdown
Contributor

room_event as the synthetic/non-conversational bucket is the right boundary — the guard is for conversational inbound turns and room_event is explicitly excluded. LGTM.

@RomneyDa

Copy link
Copy Markdown
Member

Heads up: this PR needs to be updated against current main before the new required Dependency Guard check can pass.

@anyech
anyech force-pushed the fix/discord-message-tool-only-runtime-guard branch from 48173e5 to 40fc3e8 Compare May 30, 2026 06:59
@clawsweeper clawsweeper Bot added rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels May 30, 2026
@anyech
anyech force-pushed the fix/discord-message-tool-only-runtime-guard branch from 40fc3e8 to 9ec4ab5 Compare June 2, 2026 08:02
OpenClaw Assistant added 2 commits June 27, 2026 09:46
Resolve current-main drift for the message-tool-only guard branch while preserving the source-visible delivery guard behavior.

Validation:
- node scripts/run-vitest.mjs run src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.ts src/auto-reply/tokens.test.ts --reporter=dot
- node scripts/run-tsgo.mjs -p tsconfig.core.json --incremental --tsBuildInfoFile .artifacts/tsgo-cache/core.tsbuildinfo
- git diff --check
@anyech

anyech commented Jun 28, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Behavior addressed: Current-head refresh after merging current main into the Discord message-tool-only guard branch and fixing the post-merge import/typecheck drift.

Findings addressed:

  • Preserved the source-visible delivery guard behavior from the existing PR.
  • Resolved the current-main merge conflict in src/auto-reply/reply/agent-runner.ts.
  • Removed the stale post-merge unused import that caused the first refreshed CI red.

Exact steps or command run after this patch:

  • node scripts/run-vitest.mjs run src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.ts src/auto-reply/tokens.test.ts --reporter=dot
  • node scripts/run-tsgo.mjs -p tsconfig.core.json --incremental --tsBuildInfoFile .artifacts/tsgo-cache/core.tsbuildinfo
  • git diff --check

Evidence after fix:

  • src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.ts: 86 passed.
  • src/auto-reply/tokens.test.ts: 39 passed.
  • tsgo:core: passed.
  • git diff --check: passed.
  • Current GitHub checks on head 67446c402737d30ca0caa3372fc0c494322de138 are green in my latest scan.

Observed result after fix: The branch is clean against current main, focused guard tests pass, and current-head CI is green.

What was not tested: I did not repeat the older Discord sandbox visible-message proof on this refreshed head; the existing PR already has proof: sufficient, and this update only resolves current-main merge/typecheck drift.

Requested outcome if accepted: restore/update to status: 👀 ready for maintainer look.

@clawsweeper

clawsweeper Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
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.

@anyech

anyech commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Current head status reconciliation request:

  • Current head: 67446c402737d30ca0caa3372fc0c494322de138.
  • Current observed GitHub state: clean/mergeable with no failing or pending checks in the latest refresh.
  • Prior Discord sandbox proof was supplied and accepted earlier for this PR, but the public proof predates the latest source-refresh head. A later current-head re-review request was acknowledged by ClawSweeper, but no durable review/status transition followed.

Author-side status:

  • The current head has no failing checks.
  • The source branch is mergeable.
  • I attempted to rerun the narrow current-head guard proof locally, but the current Vitest invocation path started broad test discovery instead of the intended three guard cases; I am not citing that incomplete run as proof.

Please re-evaluate the current head and either transition it toward maintainer review if the existing proof plus green current head is sufficient, or return a concrete current-head proof gap that is still author-actionable.

Treat successfully delivered substantive direct-block payloads as visible Discord reply evidence so the message-tool-only guard does not synthesize a duplicate warning. Status/progress-only direct blocks remain activity and still require a final visible reply.

Validation:
- focused E2E cases: 2 passed, 86 skipped
- type-aware oxlint: 0 warnings/errors
- git diff --check

Note: full tsgo reached unrelated existing config/secrets errors in this branch.
@anyech

anyech commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Current head addresses the unresolved substantive direct-block delivery finding.

Behavior fixed:

  • A successfully delivered substantive direct-block payload now counts as visible Discord reply evidence.
  • The message-tool-only guard no longer synthesizes a duplicate/misleading warning after that visible delivery.
  • Status/progress-only direct blocks remain activity only and still trigger the guard when no final visible reply exists.

Current head: 77390ad7a370de884e0982589c931ebd3ef1fe98.

Validation:

  • Focused E2E regressions passed: 2 tests, 86 skipped.
  • Type-aware targeted oxlint: 0 warnings and 0 errors.
  • git diff --check passed.
  • Full tsgo --noEmit reached unrelated existing config/secrets errors in this branch; no new error was attributed to the changed files.

Please re-evaluate the current head for transition toward maintainer review.

@clawsweeper

clawsweeper Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
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.

@anyech

anyech commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Current head: 35149e4434b48ed5676f6b5a29831ce0fc53b151

Behavior addressed:

  • Merged latest main into the Discord visible-reply guard branch to clear the previous dirty mergeability state.
  • Resolved the current-head test import drift by preserving both the branch's reply-payload metadata assertions and current-main heartbeat scope coverage.
  • Preserved the substantive direct-block fix from the prior head: a successfully delivered substantive direct-block payload counts as visible reply evidence, while status/progress-only direct blocks remain activity only.

Real setup tested:

  • Non-production isolated source checkout with local dependencies installed for this PR worktree.
  • No live Discord channel, production Gateway, or real outbound transport was used in this merge-refresh validation.

Exact steps or command run after this patch:

  • node scripts/run-vitest.mjs run src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.ts --reporter=dot
  • node scripts/run-oxlint.mjs src/auto-reply/reply/agent-runner.ts src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.ts
  • git diff --check

Evidence after fix:

  • Current-head guard E2E passed: 89 tests.
  • Targeted type-aware lint completed successfully.
  • Whitespace check completed successfully.

Observed result after fix:

  • The branch now includes latest main and the current-head test suite still covers the visible-reply guard behavior.

What was not tested:

  • No new live Discord sandbox send was performed in this merge-refresh step.
  • Existing public live/sandbox proof remains earlier in the thread; this comment is the current-head merge-refresh validation.

@clawsweeper

clawsweeper Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
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.

@anyech

anyech commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Current head update: pushed 88bce6acd8 to address the latest Discord guard finding.

Quick reviewer map:

  • Hook-blocked before-agent responses are now exempt from the Discord message-tool-only delivery guard.
  • The existing hook-block metadata path still marks the payload as beforeAgentRunBlocked.
  • The generic Discord delivery guard still applies to ordinary message-tool-only final replies that did not actually send.

Behavior addressed:
Before-agent hook policy-block payloads should remain visible as the policy-block response, not be replaced by the generic Discord delivery guard.

Real setup tested:
Isolated source checkout on this PR head using the Discord reply-agent E2E harness.

Exact steps or command run after this patch:

pnpm exec vitest run --config test/vitest/vitest.e2e.config.ts src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.ts --reporter=json --outputFile=<local-report>
pnpm exec oxlint src/auto-reply/reply/agent-runner.ts src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.ts
git diff --check

Evidence after fix:

  • E2E report: 90 total, 90 passed, 0 failed.
  • Targeted lint: 0 errors.
  • Whitespace check: clean.

Observed result after fix:
The new regression preserves the hook-block text, does not include the generic Discord delivery guard text, and keeps beforeAgentRunBlocked: true metadata. The existing guard coverage remains in the same E2E file for non-hook message-tool-only final replies.

What was not tested:
No production Discord delivery or Mantis run was performed in this validation.

@clawsweeper

clawsweeper Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
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.

@anyech

anyech commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Current head update: pushed b913499c8e as a no-source-change CI retrigger on top of 88bce6acd8.

Why this head changed:

  • The prior head had one red QA Smoke CI check.
  • The failed scenario was native-command-session-target, and the CI artifact showed the recovery reply arrived just after the scenario timeout.
  • GitHub job rerun is permission-gated for this account, so I used a no-code head refresh rather than repeating a comment-only loop.

Behavior addressed:
No runtime behavior changed in this commit; it only retriggers CI after validating the failed QA scenario locally.

Real setup tested:
Non-production QA harness using the same smoke profile scenario with the Telegram crabline local provider.

Exact steps or command run after this patch:

OPENCLAW_BUILD_PRIVATE_QA=1 OPENCLAW_ENABLE_PRIVATE_QA_CLI=1 OPENCLAW_DISABLE_BUNDLED_PLUGINS=0 OPENCLAW_QA_REDACT_PUBLIC_METADATA=1 OPENCLAW_QA_TRANSPORT_READY_TIMEOUT_MS=180000 pnpm openclaw qa run --repo-root . --qa-profile smoke-ci --scenario native-command-session-target --concurrency 1 --output-dir <local-output-dir>

Evidence after fix:

  • Focused QA scenario result: 1 total, 1 passed, 0 failed.
  • Scenario detail: native command reply was the expected abort message; recovery reply was the expected recovery marker.

Observed result after fix:
The focused scenario completed successfully outside CI, so the red CI run is treated as a timing flake rather than a source regression.

What was not tested:
No production Discord or Telegram delivery was performed in this validation.

@clawsweeper

clawsweeper Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
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.

@anyech

anyech commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Current-head CI follow-up for maintainers:

  • Current head: b913499c8e.
  • Current label state is already status: 👀 ready for maintainer look, but the latest QA Smoke CI job is red.
  • The failed scenario in the CI artifact is runtime-inventory-drift-check.
  • I reran that exact scenario against the current PR head in a non-production isolated source checkout with redacted QA metadata.

Validation:

  • pnpm openclaw qa run --repo-root . --qa-profile smoke-ci --scenario runtime-inventory-drift-check --concurrency 1 --output-dir .artifacts/qa-e2e/pr84485-runtime-inventory-local
  • Result: 1 total, 1 passed, 0 failed.
  • The scenario completed in about 52s locally.

Observed result:

  • This looks like a CI timeout/runner-side red for the current head rather than a source regression in this PR.
  • I cannot rerun the GitHub job directly from this account because job rerun is permission-gated.

Request:

  • Could a maintainer rerun QA Smoke CI for this head?
  • I am not pushing another no-code commit here because that already retriggered CI once and produced a different timeout-style QA red; a maintainer rerun is the cleaner next transition.

@anyech

anyech commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Behavior addressed:

  • Created a no-code current-head update to retrigger CI after the QA Smoke CI job failed on the previous head.
  • No runtime source behavior changed in this update.

Evidence after fix:

  • Head: def627d7a6d3b89f8b93b82371f83cd115e77eab
  • Previous blocker: the failed QA Smoke CI job could not be rerun through the available GitHub rerun API because that action was permission-gated.
  • The prior current-head local focused QA validation for the failed scenario passed; this new head is only to obtain a fresh CI run.

Exact steps or command run after this patch:

  • git commit --allow-empty -m "chore: retrigger QA smoke after runner failure"

Observed result after fix:

  • Branch updated successfully and CI should run again for the new head.

What was not tested:

  • No source files changed in this head; CI should verify the current matrix.

@clawsweeper

clawsweeper Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
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.

@anyech

anyech commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Current-head update for this PR.

What changed:

  • merged latest main into the PR branch
  • pushed current head a8b7e2d9365e

Validation run on the merged current head:

  • focused reply-agent Vitest set: 2 shards passed, 431 tests total
  • targeted oxlint on the touched reply-agent files: passed
  • pnpm tsgo:core: passed
  • targeted git diff --check: clean

Please re-evaluate the current head for ready status once CI lands.

@clawsweeper

clawsweeper Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
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.

Resolve current main drift for PR openclaw#84485 and keep the PR-specific behavior intact.
@anyech

anyech commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Current head update: merged latest main and resolved the current auto-reply Discord delivery guard conflicts.

Head: 1c14c66

Validation:

  • Focused auto-reply shards passed: 386 tests.
  • Typecheck passed: tsgo:core.
  • Final sanity passed: no unmerged index entries, no anchored conflict markers in touched files, and diff check clean.

@clawsweeper

clawsweeper Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
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.

@anyech

anyech commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

Closing this voluntarily.

Since this PR opened, #99536 merged the canonical recovery for the core failure reported in #85714: a substantive private final that would otherwise be stranded in message_tool_only mode now gets one guarded source-delivery retry, with a sanitized visible failure if recovery cannot deliver. That broader recovery also has dedicated QA coverage and applies beyond a Discord-specific final guard.

The remaining unique behavior in this branch is narrower: whether an empty or NO_REPLY final after tool/progress activity should override message-tool-only silence and emit a generic visible Discord reply. That is a product-policy choice rather than an unresolved implementation defect, and the current 16-commit conflicted branch is no longer the right review surface for it.

I am therefore retiring this implementation rather than rebasing another overlapping delivery path. I am not opening a replacement PR now. If maintainers explicitly want the residual visible-guard policy, I would revisit it as a minimal current-main change with the silence and duplication semantics decided up front.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: XL status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants