fix(discord): checkpoint acknowledged longwork turns by anyech · Pull Request #84864 · openclaw/openclaw

anyech · 2026-05-21T07:09:04Z

Summary

Track committed messaging-tool delivery with an explicit completed-tool ordinal, so later tool activity is detected without depending on tool metadata length or item lifecycle bookkeeping.
Surface a neutral Discord-only terminal checkpoint when a run sends an earlier ACK/progress update, completes later tool work, and would otherwise end with no final visible payload.
Keep the guard narrow: suppress heartbeat, room-event, silent, approval-prompt, and cron-follow-up lanes; require ACK/progress-like text so target-only, media-only, and final-message-plus-cleanup cases do not trigger duplicate noise.

Why this is not refactor-only

Concrete failure mode: a Discord/channel longwork turn can send an ACK/progress update with message.send, complete additional tool work, and then take the empty-payload/no-final-reply path, leaving the Discord requester with only the earlier ACK and no terminal status.
User-visible impact: the requester cannot tell from Discord whether the long-running work finished, silently stopped, or needs a status check/retry.
Behavior changed: this PR surfaces a narrow neutral terminal checkpoint for that acknowledged-longwork/no-final-payload case, while suppressing the checkpoint for terminal message.send, final-looking cleanup, target-only/media-only evidence, failed sends, heartbeat turns, room events, silent turns, approval prompts, and cron follow-up lanes.
Implementation support: the diff records the completed-tool ordinal when a messaging-tool delivery commits, propagates toolActivityAfterMessagingToolDelivery through the embedded runner result, and gates the synthesized checkpoint in runReplyAgent on that runtime delivery state plus Discord/ACK evidence.
Scope note: the state/type plumbing is in service of the user-visible delivery behavior above; this is not code movement or cleanup-only refactoring. The proof below remains intentionally non-production and does not claim live Discord transport coverage.

Tests

./node_modules/.bin/oxfmt --check --threads=1 <touched files>
node scripts/run-oxlint.mjs --tsconfig config/tsconfig/oxlint.core.json <touched files>
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-vitest.mjs run --config test/vitest/vitest.agents.config.ts src/agents/pi-embedded-subscribe.handlers.tools.test.ts
node scripts/run-tsgo.mjs -p test/tsconfig/tsconfig.core.test.json
node scripts/run-tsgo.mjs -p test/tsconfig/tsconfig.extensions.test.json

Real behavior proof

Behavior or issue addressed: Discord/channel turns that already sent an ACK/progress update through message.send, then completed additional tool work and ended without a final visible payload, now get a narrow neutral terminal checkpoint instead of silently ending. The guard intentionally stays Discord/channel-scoped and is suppressed for terminal message.send, final-looking messages followed by cleanup tools, target-only/media-only evidence, failed sends, heartbeat turns, room events, silent turns, approval prompts, and cron follow-up creation.
Real environment tested: Disposable OpenClaw source checkout rebased onto current upstream main; no production Gateway, production config, or live runtime state was used.
Exact steps or command run after this patch:
- Rebased fix/ack-longwork-terminal-checkpoint from 142c74983ab5d80886e9a2fd777f1684b1fcb821 onto upstream main 9bd97d2c60ebe68ea40c3a65d346ea56b0ed5a6c, resolving conflicts with the concurrent accepted-session-spawn state additions by preserving both state fields.
- Installed dependencies with corepack pnpm install --frozen-lockfile inside the disposable checkout.
- Ran git diff --check.
- Ran a conflict-marker scan over the touched source/test files.
- Ran ./node_modules/.bin/oxfmt --check --threads=1 <touched files>.
- Ran node scripts/run-vitest.mjs run --config test/vitest/vitest.e2e.config.ts src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.ts -t "acknowledged-longwork checkpoint".
- Ran node scripts/run-vitest.mjs run --config test/vitest/vitest.agents.config.ts src/agents/pi-embedded-subscribe.handlers.tools.test.ts.
- Ran node scripts/run-oxlint.mjs --tsconfig config/tsconfig/oxlint.core.json <touched files>.
- Ran the architecture subcommands directly: node --import tsx scripts/check-import-cycles.ts, node --import tsx scripts/check-madge-import-cycles.ts, node scripts/check-deprecated-api-usage.mjs, and node scripts/check-deprecated-jsdoc.mjs.
Evidence after fix (screenshot, recording, terminal capture, console output, redacted runtime log, linked artifact, or copied live output):

Rebase result:
- PR branch head: 7cfafb4fe66d188cd304db350525deea1696e81d
- Upstream main: 9bd97d2c60ebe68ea40c3a65d346ea56b0ed5a6c
- upstream/main is an ancestor of the PR branch

Formatting / static hygiene:
- git diff --check: pass
- conflict-marker scan over touched files: pass
- oxfmt: All matched files use the correct format. Finished in 144ms on 12 files using 1 threads.
- oxlint: Found 0 warnings and 0 errors. Finished in 10.1s on 12 files with 218 rules using 1 threads.

Focused behavior validation:
- agent-runner acknowledged-longwork e2e filter: 11 passed, 51 skipped, 1 file passed
- pi-embedded-subscribe.handlers.tools.test.ts: 40 passed, 1 file passed

Architecture subcommands after CI architecture annotation:
- Import cycle check: 0 runtime value cycle(s).
- Madge import cycle check: 0 cycle(s).
- deprecated API usage guard passed
- deprecated JSDoc guard passed

Observed result after fix: In the focused e2e proof, the positive Discord/channel case returned a non-error neutral checkpoint containing "sent an earlier update" and "without a final checkpoint", with source-suppression delivery metadata set. The suppressor cases did not return the checkpoint for terminal message.send, final-looking cleanup, target-only/media-only evidence, failed sends, heartbeat, room event, silentExpected, approval prompt, or cron-follow-up lanes.
What was not tested: No live Discord message was sent, no production Gateway/runtime/config/state was used, and no release/package publish path was exercised. The proof is an isolated source-checkout behavior proof plus targeted regression tests.
Before evidence (optional but encouraged): The earlier PR head failed the Real behavior proof check because the PR body lacked this structured after-fix proof section; GitHub reported the missing proof before this update.

clawsweeper · 2026-05-21T07:11:05Z

Codex review: needs real behavior proof before merge. Reviewed May 25, 2026, 2:23 AM ET / 06:23 UTC.

Summary
The PR records post-message-tool activity in the embedded Pi runner and synthesizes a Discord-only neutral terminal checkpoint when an ACK/progress message was delivered before later tool work and the run otherwise has no final visible payload.

PR surface: Source +211, Tests +498. Total +709 across 12 files.

Reproducibility: yes. from source and the PR's focused tests: a Discord/channel run can send an ACK/progress message through message.send, complete later tool work, and then return no final payload. I did not run a live Discord reproduction in this read-only review.

Review metrics: 3 noteworthy metrics.

Changed delivery surface: 1 Discord-visible fallback reply added. The patch can add a new message in Discord turns that previously ended without a final visible payload.
Regression coverage added: 13 acknowledged-longwork e2e cases plus 1 handler state test. The added tests cover the positive checkpoint path and the main suppressor lanes reviewers need to trust before merge.
Files affected: 12 files changed. The behavior crosses embedded runner state, subscribe handlers, reply synthesis, and tests, so reviewers should treat it as a multi-surface delivery change.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
Result: blocked until stronger real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

Add redacted live Discord/Gateway proof showing the ACK/progress message, later tool work, and exactly one neutral terminal checkpoint on the intended route; redact IPs, API keys, phone numbers, non-public endpoints, and other private details.
Update the PR body with that proof; ClawSweeper should re-review automatically, and if it does not, a maintainer can comment @clawsweeper re-review.

Proof guidance:
Needs stronger real behavior proof before merge: The PR body includes structured after-fix terminal output from targeted tests, but it explicitly did not test live Discord/Gateway delivery for the visible checkpoint behavior. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Mantis proof suggestion
A real Discord visual or transcript proof would materially verify the visible checkpoint behavior beyond mocked tests. A maintainer can ask Mantis to capture proof by posting a new PR comment that starts with the OpenClaw Mantis account mention, followed by:

visual task: verify a Discord longwork turn sends an ACK/progress update, completes later tool work, ends without a final payload, and shows exactly one neutral terminal checkpoint in the intended route.

Risk before merge

The PR intentionally adds a Discord-visible terminal checkpoint, so merging without live Discord/Gateway proof could still change real channel delivery in a way targeted mocks and source-checkout tests do not fully exercise.

Maintainer options:

Require live Discord checkpoint proof (recommended)
Ask for a redacted Discord/Gateway transcript, recording, screenshot, or runtime log that shows an ACK/progress message, later tool work, and exactly one neutral terminal checkpoint on the intended route.
Accept test-only proof intentionally
A maintainer can merge based on the focused e2e and handler coverage if they explicitly accept that no live transport path was exercised.

Next step before merge
The remaining blocker is maintainer/contributor live Discord proof for a user-visible delivery change, not a narrow code repair ClawSweeper can safely make for the external PR.

Security
Cleared: The diff changes TypeScript runtime state, reply synthesis, and tests without adding dependencies, workflows, lockfiles, package scripts, secrets handling, or privileged execution paths.

Review details

Best possible solution:

Land the narrow checkpoint after maintainers have redacted live Discord/Gateway evidence that the ACK, later tool work, and single neutral terminal checkpoint are delivered only to the intended reply route.

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

Yes from source and the PR's focused tests: a Discord/channel run can send an ACK/progress message through message.send, complete later tool work, and then return no final payload. I did not run a live Discord reproduction in this read-only review.

Is this the best way to solve the issue?

Mostly yes: carrying explicit post-delivery tool activity into runReplyAgent and gating the checkpoint by Discord route-aware ACK evidence is a narrow maintainable fix. The remaining blocker is real transport proof, not an obvious code-shape defect.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
remove rating: 🧂 unranked krab: Current PR rating is rating: 🦪 silver shellfish, so this older rating label is no longer current.

Label justifications:

P2: This is a normal-priority Discord delivery fix with limited blast radius and strong targeted tests, but no live outage evidence.
merge-risk: 🚨 message-delivery: The PR changes when OpenClaw sends an extra user-visible Discord checkpoint, which can affect message delivery even with green CI.
rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body includes structured after-fix terminal output from targeted tests, but it explicitly did not test live Discord/Gateway delivery for the visible checkpoint behavior. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence reviewed

PR surface:

Source +211, Tests +498. Total +709 across 12 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	10	213	2	+211
Tests	2	498	0	+498
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	12	711	2	+709

What I checked:

Root policy read: Read the full root AGENTS.md and applied its guidance on whole delivery-surface review, user-visible proof, and message-delivery merge risk. (AGENTS.md:1, b96211063755)
Scoped policy read: Read the scoped agent guidance; it favors focused helper tests for expensive embedded-runner surfaces but does not remove the root requirement for real proof of visible Discord behavior. (src/agents/pi-embedded-runner/run/AGENTS.md:1, b96211063755)
PR live state: The pull request is open, authored by an external contributor, mergeable clean, and currently changes 12 files with 711 additions and 2 deletions at head 009db0b. (009db0ba61bb)
Current main behavior: Current main still has the empty-payload path that drains queued followups and returns no visible payload when there is no reply payload or fallback notice, so the PR is not obsolete on main. (src/auto-reply/reply/agent-runner.ts:1736, b96211063755)
Main lacks PR mechanism: Current main does not contain the acknowledged-longwork checkpoint text or the toolActivityAfterMessagingToolDelivery state, confirming the central change is unique to the PR branch. (src/auto-reply/reply/agent-runner.ts, b96211063755)
ACK predicate tightened: The latest diff excludes final/result-like first-person text before accepting ACK/progress wording, addressing the prior false-positive concern. (src/auto-reply/reply/agent-runner.ts:168, 009db0ba61bb)

Likely related people:

steipete: Recent GitHub file history shows multiple current-main commits on agent-runner fallback notices, messaging tool dedupe, and embedded runner delivery-adjacent behavior. (role: recent area contributor; confidence: high; commits: 4b0f16d496e5, 72f6016ce596, eb66def65604; files: src/auto-reply/reply/agent-runner.ts, src/auto-reply/reply/reply-payloads-dedupe.ts, src/agents/pi-embedded-subscribe.handlers.tools.ts)
vincentkoc: Local blame on the current shallow checkout attributes the surrounding empty-payload and messaging-tool evidence lines to Vincent Koc's current-main commit, and GitHub history shows recent dedupe/thread-id work in the same helper surface. (role: latest surrounding-line contributor; confidence: high; commits: 2a73725b5d6a, 16b0a6202c69, 642e1dfcdfb1; files: src/auto-reply/reply/agent-runner.ts, src/auto-reply/reply/reply-payloads-dedupe.ts, src/agents/pi-embedded-subscribe.handlers.tools.ts)
giodl73-repo: Recent merged history includes internal UI message-tool reply mirroring and preview/final reply dedupe work adjacent to this checkpoint delivery path. (role: recent message-tool and reply contributor; confidence: medium; commits: d7a078f1962b, bd51d8f2dd97, 0eca3a92e3e3; files: src/agents/pi-embedded-subscribe.handlers.tools.ts, src/auto-reply/reply/agent-runner.ts)
jalehman: Recent merged history modified the same runReplyAgent decision surface while centralizing turn admission and queue behavior. (role: adjacent reply-lane contributor; confidence: medium; commits: 62b51a6295ee; files: src/auto-reply/reply/agent-runner.ts)
anyech: Although also the PR author, anyech has prior merged current-main work on Discord verbose tool-progress delivery in the affected reply-delivery path. (role: prior Discord delivery contributor; confidence: medium; commits: 57da466ecbb5; 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.

clawsweeper · 2026-05-21T07:16:22Z

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.

anyech · 2026-05-22T09:12:38Z

Follow-up update after the proof/rebase pass:

Rebasing is done: branch head is now 7cfafb4fe66d188cd304db350525deea1696e81d, with current upstream main as an ancestor.
The PR body now has the structured ## Real behavior proof fields, including Observed result after fix and What was not tested.
The latest Real behavior proof check is now green.
Local disposable-checkout validation passed for the touched behavior path:
- git diff --check
- conflict-marker scan over touched files
- ./node_modules/.bin/oxfmt --check --threads=1 <touched files>
- node scripts/run-vitest.mjs run --config test/vitest/vitest.e2e.config.ts src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.ts -t "acknowledged-longwork checkpoint"
- node scripts/run-vitest.mjs run --config test/vitest/vitest.agents.config.ts src/agents/pi-embedded-subscribe.handlers.tools.test.ts
- node scripts/run-oxlint.mjs --tsconfig config/tsconfig/oxlint.core.json <touched files>

One CI check still reports red: check-additional-runtime-topology-architecture. I checked its log and it failed on a Madge cycle in the existing plugin metadata snapshot files. A local rerun of the same architecture subcommands in the disposable checkout passed with Madge import cycle check: 0 cycle(s), plus the import-cycle/deprecated API/JSDoc guards passed. I could not rerun the GitHub job from this token because the Actions job rerun endpoint requires repository admin rights.

Track messaging-tool delivery with a completed-tool ordinal so post-ACK activity is detected without depending on tool metadata or item lifecycle timing. Surface a neutral Discord checkpoint when a turn sends an earlier ACK/progress update, completes later tool work, and otherwise has no final visible payload. Keep the guard narrow: Discord only, suppressed for heartbeat/room/silent/ approval/cron lanes, and only triggered by ACK/progress-like text rather than target-only or media-only delivery evidence.

anyech · 2026-05-30T07:12:45Z

Closing this PR as superseded/deferred to reduce the open external PR queue.

The underlying acknowledged-longwork terminal-checkpoint issue is still valid, but this branch has become stale and conflict-heavy against current main, and the remaining behavior is better revisited as a smaller current-main implementation if maintainers still want this exact path.

Related active delivery work should continue in the more focused Discord/final-delivery PRs rather than keeping this broader stale branch open.

openclaw-barnacle Bot added agents Agent runtime and tooling size: L triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. triage: refactor-only Candidate: refactor/cleanup-only PR without maintainer context. labels May 21, 2026

anyech force-pushed the fix/ack-longwork-terminal-checkpoint branch from 142c749 to 7cfafb4 Compare May 22, 2026 09:04

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 22, 2026

anyech force-pushed the fix/ack-longwork-terminal-checkpoint branch from 39fe32a to 1e55092 Compare May 23, 2026 07:35

clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels May 23, 2026

anyech force-pushed the fix/ack-longwork-terminal-checkpoint branch from 1e55092 to b232f89 Compare May 23, 2026 20:01

OpenClaw Assistant added 2 commits May 25, 2026 06:10

fix(discord): tighten longwork checkpoint evidence

009db0b

anyech force-pushed the fix/ack-longwork-terminal-checkpoint branch from 9338640 to 009db0b Compare May 25, 2026 06:15

clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels May 25, 2026

anyech mentioned this pull request May 25, 2026

fix(discord): allow marked room_event final fallback delivery #86672

Closed

anyech closed this May 30, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(discord): checkpoint acknowledged longwork turns#84864

fix(discord): checkpoint acknowledged longwork turns#84864
anyech wants to merge 2 commits into
openclaw:mainfrom
anyech:fix/ack-longwork-terminal-checkpoint

anyech commented May 21, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented May 21, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented May 21, 2026

Uh oh!

anyech commented May 22, 2026

Uh oh!

anyech commented May 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

anyech commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why this is not refactor-only

Tests

Real behavior proof

Uh oh!

clawsweeper Bot commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

clawsweeper Bot commented May 21, 2026

Uh oh!

anyech commented May 22, 2026

Uh oh!

anyech commented May 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

anyech commented May 21, 2026 •

edited

Loading

clawsweeper Bot commented May 21, 2026 •

edited

Loading