Skip to content

fix(tasks): route group acp completions through parent#77365

Open
funmerlin wants to merge 1 commit intoopenclaw:mainfrom
funmerlin:fix/group-acp-task-completions
Open

fix(tasks): route group acp completions through parent#77365
funmerlin wants to merge 1 commit intoopenclaw:mainfrom
funmerlin:fix/group-acp-task-completions

Conversation

@funmerlin
Copy link
Copy Markdown

Summary

  • route group-channel ACP terminal task updates through the parent session instead of direct channel delivery
  • add a regression test covering group-channel requester sessions
  • document the group-channel completion routing behavior

Fixes #77251

Tests

  • pnpm test src/tasks/task-registry.test.ts
  • pnpm exec oxfmt --check --threads=1 src/tasks/task-registry.ts src/tasks/task-registry.test.ts

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation channel: discord Channel integration: discord channel: matrix Channel integration: matrix cli CLI command changes scripts Repository scripts docker Docker and sandbox tooling agents Agent runtime and tooling extensions: qa-lab size: XL labels May 4, 2026
@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented May 4, 2026

Codex review: needs changes before merge.

Summary
The branch changes task completion delivery to skip direct sends for channel-shaped owner sessions, adds task-registry coverage, updates task delivery docs, and appends changelog entries.

Reproducibility: yes. Source inspection shows current main direct-sends ACP terminal updates when requesterOrigin is deliverable, and the PR's channel-only helper still leaves documented :group: session keys on that path.

Next step before merge
A narrow automated repair can update the helper, add the missing group-key test, and clean the changelog without a product decision.

Security
Cleared: The current four-file diff touches task code, tests, docs, and changelog only; I found no dependency, workflow, install-script, permission, or secret-handling change.

Review findings

  • [P2] Handle documented group session keys — src/tasks/task-registry.ts:1070
  • [P3] Remove unrelated changelog entries — CHANGELOG.md:548-551
Review details

Best possible solution:

Update the routing helper to classify both documented group and channel owner sessions, cover both forms in focused tests, and keep docs/changelog scoped to ACP task completion routing.

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

Yes. Source inspection shows current main direct-sends ACP terminal updates when requesterOrigin is deliverable, and the PR's channel-only helper still leaves documented :group: session keys on that path.

Is this the best way to solve the issue?

No. Parent-session routing is the right direction, but the implementation should use a group-or-channel session classification and add a true :group: regression case before merge.

Full review comments:

  • [P2] Handle documented group session keys — src/tasks/task-registry.ts:1070
    The new helper only disables direct delivery when the owner session contains channel. Documented group sessions use agent:<agentId>:<channel>:group:<id>, so Telegram/WhatsApp-style group ACP completions will still post the generic terminal task text directly and bypass the parent-session wake this PR is meant to restore. Check for both group and channel owner sessions, or reuse the existing chat-type parsing helper, and add a :group: regression case.
    Confidence: 0.9
  • [P3] Remove unrelated changelog entries — CHANGELOG.md:548-551
    The changelog hunk adds several bullets about OpenAI transport, session lifecycle, gateway repair, and plugin/update behavior that this branch does not implement. Landing those entries here would publish release notes for unrelated changes and make changelog audit/dedupe misleading; keep only the task-routing bullet for this PR.
    Confidence: 0.82

Overall correctness: patch is incorrect
Overall confidence: 0.88

Acceptance criteria:

  • pnpm test src/tasks/task-registry.test.ts
  • pnpm exec oxfmt --check --threads=1 src/tasks/task-registry.ts src/tasks/task-registry.test.ts docs/automation/tasks.md CHANGELOG.md
  • pnpm check:changed

What I checked:

  • Current main direct-delivery gate: Current main sends terminal ACP task updates directly whenever the resolved requester origin has a deliverable channel and target. (src/tasks/task-registry.ts:1057, 1df2ac442a75)
  • Direct-send call path: When canDeliverTaskToRequesterOrigin is true, current main calls sendMessage with the generic terminal task text instead of queueing a system event for the parent session. (src/tasks/task-registry.ts:1149, 1df2ac442a75)
  • Group session key contract: The group-channel docs define group sessions as agent:<agentId>:<channel>:group:<id> and room/channel sessions as agent:<agentId>:<channel>:channel:<id>. Public docs: docs/channels/groups.md. (docs/channels/groups.md:133, 1df2ac442a75)
  • PR helper misses group keys: The PR helper returns true only when the parsed owner-session rest contains channel, so documented :group: sessions still take the direct-delivery path. (src/tasks/task-registry.ts:1070, c1fcc91a7a58)
  • Regression test covers channel-shaped key only: The added regression test uses agent:main:guildchat:channel:123, leaving agent:main:<channel>:group:<id> untested. (src/tasks/task-registry.test.ts:802, c1fcc91a7a58)
  • Changelog scope drift: The PR changelog hunk adds the intended task-routing bullet plus unrelated OpenAI transport, session lifecycle, gateway repair, and plugin/update bullets. (CHANGELOG.md:548, c1fcc91a7a58)

Likely related people:

  • vincentkoc: Blame and symbol history for the task delivery gate, nearby regression tests, and task delivery docs all point to Vincent Koc in the available current-main history. (role: current-main task delivery maintainer; confidence: medium; commits: 6b7f9eafed4b; files: src/tasks/task-registry.ts, src/tasks/task-registry.test.ts, docs/automation/tasks.md)

Remaining risk / open question:

  • I did not execute the PR's tests because this sweep is read-only; the verdict is based on source, docs, provided PR diff, and local history inspection.

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

Re-review progress:

@funmerlin funmerlin force-pushed the fix/group-acp-task-completions branch from b44d02d to c1fcc91 Compare May 4, 2026 15:35
@openclaw-barnacle openclaw-barnacle Bot added size: S and removed channel: discord Channel integration: discord channel: matrix Channel integration: matrix cli CLI command changes scripts Repository scripts docker Docker and sandbox tooling agents Agent runtime and tooling extensions: qa-lab size: XL labels May 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Improvements or additions to documentation size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: background ACP/subagent completions in group channels can stop at generic task-status messages instead of parent-agent replies

1 participant