fix(tasks): deliver ACP completions to bound Discord threads#89279
Conversation
|
Codex review: passed. Reviewed June 19, 2026, 2:30 AM ET / 06:30 UTC. Summary PR surface: Source +24, Tests +142. Total +166 across 2 files. Reproducibility: yes. at source level. Current main queues successful ACP parent-review completions through the requester session for channel/group owner keys, and the linked canonical issue includes matching Discord thread-bound ACP evidence. Review metrics: 1 noteworthy metric.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. 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 exact head if deterministic merge gates pass, while preserving the broader #84022 room_event policy work through an open tracker if GitHub closes it. Do we have a high-confidence way to reproduce the issue? Yes at source level. Current main queues successful ACP parent-review completions through the requester session for channel/group owner keys, and the linked canonical issue includes matching Discord thread-bound ACP evidence. Is this the best way to solve the issue? Mostly yes. The task registry has the terminal outcome, owner, requesterOrigin, and idempotency context needed for this narrow fix; a generic thread-capable channel contract would be cleaner only if maintainers reject the Discord-specific exception. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 5d6ac2308657. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +24, Tests +142. Total +166 across 2 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
|
|
proof: override request I rechecked this proof blocker with a disposable checkout only. I did not run live Gateway/Discord delivery because the merge-relevant behavior is an external bound-thread delivery route and should not use a private/operator deployment as public proof. In the disposable setup, the focused task-registry regression tests passed and an additional runtime-seam check confirmed the narrow bound Discord thread exception, direct Deterministic non-production coverage observed:
Since the remaining evidence would require a real external messaging route, is maintainer |
|
Current-head non-production proof refresh for reviewers:
The remaining fully-real proof would require exercising an external Discord/Gateway delivery route. Could a maintainer please decide whether this current-head non-production boundary proof is acceptable, or whether you want a maintainer-controlled live delivery run before merge? @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review Behavior addressed:
Real setup tested:
Exact steps or command run after this patch: PROOF_TARGET='channel:<sandbox-thread>' \
PROOF_MARKER='89279-real-20260614T1057Z' \
node scripts/run-vitest.mjs run \
--config test/vitest/vitest.tasks.config.ts \
src/tasks/task-registry.test.ts \
-t 'proof: delivers delegated ACP completion to real sandbox Discord thread'Evidence after fix: Sanitized runtime evidence from the proof case: {
"marker": "89279-real-20260614T1057Z",
"taskStatusAfterLifecycleEnd": "succeeded",
"deliveryStatusAfterLifecycleEnd": "delivered",
"sendMessageCall": {
"channel": "discord",
"to": "channel:<parent-channel>",
"threadId": "<sandbox-thread>"
},
"visibleSandboxMessage": {
"contains": "Background task ready for review: ACP background task",
"containsMarker": true,
"messageId": "<redacted-message-id>"
}
}Observed result after fix:
What was not tested:
|
|
/clownfish automerge |
|
Clownfish is on the reef for this PR. 🐠 I tagged A maintainer can call |
|
🦞✅ Source: What merged:
Automerge notes:
The automerge loop is complete. Automerge progress:
|
…w#89279) Summary: - The branch adds a bounded task-registry predicate and tests so successful delegated ACP parent-review comple ... with a Discord channel target and threadId send the parent-review terminal message directly to that thread. - PR surface: Source +24, Tests +142. Total +166 across 2 files. - Reproducibility: yes. at source level. Current main queues successful ACP parent-review completions through ... annel/group owner keys, and the linked canonical issue includes matching Discord thread-bound ACP evidence. Automerge notes: - No ClawSweeper repair was needed after automerge opt-in. Validation: - ClawSweeper review passed for head 04ad66b. - Required merge gates passed before the squash merge. Prepared head SHA: 04ad66b Review: openclaw#89279 (comment) Co-authored-by: anyech <[email protected]>
…w#89279) Summary: - The branch adds a bounded task-registry predicate and tests so successful delegated ACP parent-review comple ... with a Discord channel target and threadId send the parent-review terminal message directly to that thread. - PR surface: Source +24, Tests +142. Total +166 across 2 files. - Reproducibility: yes. at source level. Current main queues successful ACP parent-review completions through ... annel/group owner keys, and the linked canonical issue includes matching Discord thread-bound ACP evidence. Automerge notes: - No ClawSweeper repair was needed after automerge opt-in. Validation: - ClawSweeper review passed for head 04ad66b. - Required merge gates passed before the squash merge. Prepared head SHA: 04ad66b Review: openclaw#89279 (comment) Co-authored-by: anyech <[email protected]>
Summary
Fixes #84022 by delivering successful parent-review ACP task terminal updates to the explicitly bound Discord thread that originated the request.
This is a narrow replacement shape after #86672 and #88251 were closed:
room_eventsuppression behavior;Promise.raceor leave the underlying ACP run alive in the background;Behavior change
Before:
session_queuedeven though the originating Discord thread was explicit and deliverable.After:
discord;channel:;threadId;The direct visible text uses the parent-review wording (
Background task ready for review...) so the thread receives a terminal update without incorrectly implying the child result was fully verified/finalized by the parent.Real behavior proof
Behavior addressed: A delegated ACP parent-review completion with an explicit Discord bound-thread requester origin sends the terminal update directly to that
threadId; non-bound/non-qualifying paths remain queued through the parent session.Real setup tested: Disposable checkout of PR branch
anyech/openclaw:fix/bound-thread-acp-terminal-delivery-20260602undertmp/github-open-work/real-proof-20260603T073207Z/pr-89279. Two non-production checks were run: (1) the focused task-registry runtime slice for the real ACP terminal delivery path with synthetic delivery runtime, and (2) a one-off harness over the real ACP spawn bootstrap delivery-plan code with synthetic Discord binding metadata. No production Gateway/config/runtime/state or private deployment proof was used.Exact steps or command run after this patch: Terminal commands run in the disposable checkout were
pnpm install --frozen-lockfile --ignore-scripts,node scripts/run-vitest.mjs run --config test/vitest/vitest.tasks.config.ts src/tasks/task-registry.test.ts -t 'bound Discord thread|without an explicit bound Discord thread|routes .* ACP completion through the parent session|delivers non-delegated ACP completion' --reporter=verbose --pool=forks --maxWorkers=1, andnode --import tsx proof-89279-direct.ts; full command transcript is inreview/real-proof-build-20260603T073207Z/messaging/exact-commands-used.txt.Evidence after fix: Terminal output / runtime log excerpts from
review/real-proof-build-20260603T073207Z/messaging/evidence/proof-89279-task-registry.logandreview/real-proof-build-20260603T073207Z/messaging/evidence/proof-89279-direct.log:{ "boundSession": {"channel":"discord","to":"channel:thread-bound-child","accountId":"acct-proof","threadId":"thread-bound-child","deliver":true}, "nonBoundRun": {"deliver":false}, "streamedRun": {"deliver":false} }Observed result after fix: The bound Discord ACP path delivers to the explicit synthetic
threadIdand marks/sends the direct update in the task registry; missing thread, non-channel target, non-Discord, and ordinary parent-session ACP routes staysession_queued/deliver:falseand do not send direct visible thread output.What was not tested: No live production Gateway, live Discord workspace, real bot token, or private deployment state was used. The outbound provider/transport was substituted at the boundary to keep proof non-production and sanitized. The one-off ACP spawn harness temporarily exported an internal helper only inside the disposable checkout and restored the file afterward; no code was pushed.