Skip to content

Fix subagent thread spawn safety#83274

Closed
Svetznaniy33 wants to merge 3 commits into
openclaw:mainfrom
Svetznaniy33:fix/telegram-thread-spawn-safety
Closed

Fix subagent thread spawn safety#83274
Svetznaniy33 wants to merge 3 commits into
openclaw:mainfrom
Svetznaniy33:fix/telegram-thread-spawn-safety

Conversation

@Svetznaniy33

@Svetznaniy33 Svetznaniy33 commented May 17, 2026

Copy link
Copy Markdown

Summary

  • prevent unsafe raw subagent/tool output from being surfaced in completion/timeout announces
  • fail closed for persistent thread-bound subagent sessions when hook readiness lacks a routable delivery origin
  • allow Telegram to opt into automatic thread-bound session spawn policy

Tests

  • OPENCLAW_VITEST_MAX_WORKERS=1 node scripts/run-vitest.mjs run --config test/vitest/vitest.agents.config.ts src/agents/sessions-spawn-hooks.test.ts
  • OPENCLAW_VITEST_MAX_WORKERS=1 node scripts/run-vitest.mjs run --config test/vitest/vitest.agents.config.ts src/agents/subagent-announce.timeout.test.ts
  • OPENCLAW_VITEST_MAX_WORKERS=1 node scripts/run-vitest.mjs run --config test/vitest/vitest.channels.config.ts src/channels/thread-bindings-policy.test.ts
  • corepack pnpm format:check src/agents/subagent-announce-output.ts src/agents/subagent-announce.timeout.test.ts src/agents/subagent-spawn.ts src/agents/sessions-spawn-hooks.test.ts src/channels/thread-bindings-policy.ts src/channels/thread-bindings-policy.test.ts

Real behavior proof

  • Automated unit/integration coverage was run against the local safe checkout for the thread-bound Telegram/session spawn paths listed above.
  • Live Telegram Desktop/native proof could not be generated from this environment because the Mantis Telegram Desktop capability is unavailable (openclaw-mantis[bot], comment 4472640801).
  • Remaining blocker: the repository's Real behavior proof CI requires after-fix evidence from a real OpenClaw setup (screenshot, recording, terminal screenshot, console output, redacted runtime logs, linked artifact, or copied live output). This cannot be automatically reproduced in the current safe checkout without access to a routable live OpenClaw Telegram setup; maintainer-provided live proof or a proof: override label is required.

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 17, 2026
@clawsweeper

clawsweeper Bot commented May 17, 2026

Copy link
Copy Markdown
Contributor

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Close as superseded: merged current-main work already replaced the raw-output and core thread-binding pieces, and the remaining Telegram forum-topic spawn behavior is tracked by the canonical open issue rather than this stale, broader branch.

Root-cause cluster
Relationship: superseded
Canonical: #42986
Summary: This PR overlaps merged current-main raw-output and core thread-binding fixes; the remaining Telegram forum-topic subagent spawn capability is tracked by the canonical issue and an active candidate PR.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Canonical path: The remaining Telegram sessions_spawn(thread:true, mode:session) forum-topic capability is tracked by #42986; #82023 is an active but conflicting candidate for that issue.

So I’m closing this here and keeping the remaining discussion on #42986 and #82023.

Review details

Best possible solution:

Close this branch and keep the remaining Telegram child-topic subagent spawn work on the canonical issue using an explicit capability that preserves Telegram's normal current-topic placement.

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

Yes, source-level. Current main shows the raw-output and core thread-binding pieces already replaced, while Telegram still stays on current placement and the remaining child-topic spawn capability is tracked separately.

Is this the best way to solve the issue?

No. This branch is no longer the best solution because it mixes superseded fixes with a broad Telegram placement change; the safer path is the canonical explicit capability work on #42986.

Security review:

Security review cleared: No dependency, workflow, permission, secret-handling, package execution, or supply-chain changes were found; the blocker is functional supersession and routing compatibility.

AGENTS.md: found and applied where relevant.

What I checked:

Likely related people:

  • steipete: Authored the merged core SessionBindingService refactor that supersedes this PR's older subagent_spawning and delivery-origin implementation path. (role: core thread-binding refactor author; confidence: high; commits: 3fc0df953cf1; files: src/agents/subagent-spawn.ts, src/agents/subagent-spawn.thread-binding.test.ts, docs/plugins/hooks.md)
  • brokemac79: Authored the merged completion-handoff change that covers the raw tool/toolResult output suppression portion of this branch. (role: subagent completion-output contributor; confidence: high; commits: f4b92f5e6c70; files: src/agents/subagent-announce-output.ts, src/agents/subagent-announce-output.test.ts, src/agents/subagent-announce-delivery.ts)
  • vincentkoc: Recent commits touched Telegram channel/thread-binding and channel policy surfaces adjacent to the remaining capability decision. (role: recent Telegram/channel contributor; confidence: medium; commits: e012f2cd3cc8, 30955d366025, 5370e73ee984; files: extensions/telegram/src/channel.ts, extensions/telegram/src/thread-bindings.ts, src/channels/thread-bindings-policy.ts)
  • efpiva: Authored the active PR that targets the remaining Telegram forum-topic subagent binding capability with live proof, though it is still open and conflicting. (role: open implementation candidate author; confidence: medium; commits: 987388309d68; files: extensions/telegram/src/subagent-hooks.ts, extensions/telegram/thread-binding-api.ts, src/agents/runtime-capabilities.ts)

Codex review notes: model internal, reasoning high; reviewed against 0c7bac34ae68.

@clawsweeper clawsweeper Bot added mantis: telegram-visible-proof Mantis should capture Telegram visible proof. P1 High-priority user-facing bug, regression, or broken workflow. impact:security Security boundary, credential, authz, sandbox, or sensitive-data risk. impact:message-loss Channel message delivery can be lost, duplicated, or misrouted. impact:session-state Session, memory, transcript, context, or agent state can drift or corrupt. labels May 17, 2026
@clawsweeper
clawsweeper Bot temporarily deployed to qa-live-shared May 17, 2026 21:46 Inactive
@openclaw-mantis

Copy link
Copy Markdown
Contributor

Mantis Telegram Desktop Proof

Summary: Mantis did not generate before/after GIFs because PR 83274 changes Telegram-origin subagent thread-binding/tool availability and unsafe subagent announce filtering, but the native Telegram proof harness mock model cannot emit the required sessions_spawn(thread=true) tool call; a Telegram Desktop recording would only show a generic text reply or /status, not the PR behavior.

Raw QA files: https://artifacts.openclaw.ai/mantis/telegram-desktop/pr-83274/run-26003579850-1/index.json

@openclaw-barnacle openclaw-barnacle Bot added channel: telegram Channel integration: telegram size: M and removed size: S labels May 18, 2026
@Svetznaniy33

Copy link
Copy Markdown
Author

@clawsweeper re-review

Addressed the review findings in 801308c:

  • Removed the hardcoded Telegram special-case from core thread-binding spawn policy; Telegram now advertises child placement through its bundled thread-binding-api.ts public surface.
  • Preserved indirect thread-bound session routing by resolving a bound child conversation when the hook reports ready without a direct deliveryOrigin, and only using direct delivery when that route is actually routable.
  • Applied unsafe-output filtering to the fallback latest-assistant reply path, with regression coverage.

Verification run locally:

  • OPENCLAW_VITEST_MAX_WORKERS=1 node scripts/run-vitest.mjs run --config test/vitest/vitest.agents.config.ts src/agents/subagent-announce-output.test.ts src/agents/subagent-announce.timeout.test.ts src/agents/subagent-spawn.thread-binding.test.ts src/agents/sessions-spawn-hooks.test.ts
  • OPENCLAW_VITEST_MAX_WORKERS=1 node scripts/run-vitest.mjs run --config test/vitest/vitest.channels.config.ts src/channels/thread-bindings-policy.test.ts src/channels/plugins/thread-binding-api.test.ts
  • corepack pnpm format:check ...
  • git diff --check

@clawsweeper

clawsweeper Bot commented May 18, 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.

Re-review progress:

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. 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: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels May 18, 2026
@Svetznaniy33

Copy link
Copy Markdown
Author

@clawsweeper re-review

Pushed follow-up commit ff9a55cc8a for the latest blockers:

  • Kept Telegram placement consistent for loaded plugins and the bundled public artifact: extensions/telegram/src/channel.ts now advertises defaultTopLevelPlacement: "child", with a regression test comparing it to extensions/telegram/thread-binding-api.ts.
  • Replaced the invalid binding status comparison with the real active-status check (binding.status === "active").
  • Fixed the hook mock type regression by explicitly typing the test hook result so deliveryOrigin remains optional.

Local verification after the fix:

OPENCLAW_TSGO_MAX_WORKERS=1 corepack pnpm tsgo:core:test
OPENCLAW_TSGO_MAX_WORKERS=1 corepack pnpm tsgo:extensions:test
OPENCLAW_VITEST_MAX_WORKERS=1 node scripts/run-vitest.mjs run --config test/vitest/vitest.agents.config.ts src/agents/subagent-spawn.thread-binding.test.ts src/agents/sessions-spawn-hooks.test.ts
OPENCLAW_VITEST_MAX_WORKERS=1 node scripts/run-vitest.mjs run --config test/vitest/vitest.extension-telegram.config.ts extensions/telegram/src/thread-bindings.test.ts
OPENCLAW_VITEST_MAX_WORKERS=1 node scripts/run-vitest.mjs run --config test/vitest/vitest.channels.config.ts src/channels/thread-bindings-policy.test.ts src/channels/plugins/thread-binding-api.test.ts
OPENCLAW_TSGO_MAX_WORKERS=1 corepack pnpm tsgo:core
OPENCLAW_TSGO_MAX_WORKERS=1 corepack pnpm tsgo:extensions
corepack pnpm format:check src/agents/subagent-spawn.ts src/agents/sessions-spawn-hooks.test.ts src/agents/subagent-spawn.thread-binding.test.ts extensions/telegram/src/channel.ts extensions/telegram/thread-binding-api.ts extensions/telegram/src/thread-bindings.test.ts
git diff --check

All passed locally.

Real behavior proof is still pending: I do not have a safe redacted live Telegram runtime transcript from this checkout to attach yet. The latest commit addresses the concrete source/CI blockers from the review comment.

@clawsweeper

clawsweeper Bot commented May 18, 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.

Re-review progress:

@clawsweeper clawsweeper Bot added status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed impact:session-state Session, memory, transcript, context, or agent state can drift or corrupt. impact:security Security boundary, credential, authz, sandbox, or sensitive-data risk. impact:message-loss Channel message delivery can be lost, duplicated, or misrouted. labels May 18, 2026
@clawsweeper clawsweeper Bot removed the mantis: telegram-visible-proof Mantis should capture Telegram visible proof. label May 26, 2026
@clawsweeper

clawsweeper Bot commented May 26, 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.

@clawsweeper

clawsweeper Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

@Svetznaniy33 thanks for the PR. ClawSweeper is still waiting on real behavior proof before this can move forward.

Useful proof can be a screenshot, short video, terminal output, copied live output, linked artifact, or redacted logs that show the changed behavior after the fix. Please redact private tokens, phone numbers, private endpoints, customer data, and anything else sensitive.

Once proof is added to the PR body or a comment, ClawSweeper or a maintainer can re-check it.

@clawsweeper

clawsweeper Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

@Svetznaniy33 thanks for the PR. ClawSweeper is still waiting on real behavior proof before this can move forward.

Useful proof can be a screenshot, short video, terminal output, copied live output, linked artifact, or redacted logs that show the changed behavior after the fix. Please redact private tokens, phone numbers, private endpoints, customer data, and anything else sensitive.

Once proof is added to the PR body or a comment, ClawSweeper or a maintainer can re-check it.

@clawsweeper clawsweeper Bot added P2 Normal backlog priority with limited blast radius. and removed P1 High-priority user-facing bug, regression, or broken workflow. labels Jun 28, 2026
@openclaw-barnacle openclaw-barnacle Bot added the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jun 28, 2026
@steipete steipete closed this Jul 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling channel: telegram Channel integration: telegram 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: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: M status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants