Skip to content

fix(subagents): keep thread-bound spawns isolated by default#79050

Closed
jalehman wants to merge 2 commits into
mainfrom
fix/reopen-pr-78060-thread-spawn-isolated
Closed

fix(subagents): keep thread-bound spawns isolated by default#79050
jalehman wants to merge 2 commits into
mainfrom
fix/reopen-pr-78060-thread-spawn-isolated

Conversation

@jalehman

@jalehman jalehman commented May 7, 2026

Copy link
Copy Markdown
Contributor

What

Reapplies the intended #78060 repair on a fresh OpenClaw-owned branch after the original contributor PR was accidentally closed with an unrelated empty head. Thread-bound native sessions_spawn now defaults omitted context to isolated, while explicit context: "fork" and explicit threadBindings.defaultSpawnContext: "fork" continue to fork.

Why

#78055 reports subagent sessions inheriting unrelated requester transcript history. The original #78060 carried the right fix, but its contributor branch now points at f2458d8828de71997ffa290aa2dc6e1177d06b57, which is already on main and leaves the closed PR with no files. This PR supersedes #78060 and restores the intended behavior change.

Attribution

This PR preserves the original contributor commit as the first commit on the branch:

  • 41facf822677757f29fe5f3b103a143a341d17cc cherry-picks original commit 901404acf60043fa3b95fcfdf57c5244c2143495, authored by Eva (agent) <eva+agent-78055@100yen.org>.
  • 90619451037ffd39022a00267a6911200832f2b3 is the Josh-authored sync commit for generated config docs, type comments, and changelog.

Changes

  • Default thread-bound spawns to isolated
  • Preserve explicit fork overrides
  • Update docs and config hints
  • Regenerate config metadata checksums
  • Add changelog entry

Testing

  • node scripts/test-projects.mjs src/channels/thread-bindings-policy.test.ts src/agents/subagent-spawn.context.test.ts - passed, 2 Vitest shards
  • pnpm config:docs:check - passed
  • pnpm config:channels:check - passed
  • pnpm config:schema:check - passed
  • pnpm exec oxfmt --check --threads=1 CHANGELOG.md docs/.generated/config-baseline.sha256 docs/channels/discord.md docs/concepts/session-tool.md docs/gateway/config-agents.md docs/gateway/config-channels.md docs/tools/subagents.md src/agents/subagent-spawn.context.test.ts src/channels/thread-bindings-policy.ts src/channels/thread-bindings-policy.test.ts src/config/bundled-channel-config-metadata.generated.ts src/config/schema.help.ts src/config/types.base.ts src/config/types.discord.ts extensions/discord/src/config-ui-hints.ts extensions/telegram/src/config-ui-hints.ts - passed
  • git diff --check - passed
  • pnpm tsgo:test:src - failed on unrelated current-main OpenAI fixture typing in src/agents/openai-transport-stream.test.ts and src/agents/pi-embedded-runner/openai-stream-wrappers.test.ts; this branch does not touch those files

Refs #78055. Supersedes #78060.

Eva (agent) and others added 2 commits May 7, 2026 11:20
Avoid implicit requester transcript forks for thread-bound native subagent spawns unless context="fork" or threadBindings.defaultSpawnContext="fork" is explicitly configured.

Refs #78055
@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation channel: discord Channel integration: discord channel: telegram Channel integration: telegram gateway Gateway runtime agents Agent runtime and tooling size: S maintainer Maintainer-authored PR labels May 7, 2026
@clawsweeper

clawsweeper Bot commented May 7, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs changes before merge.

Summary
The PR changes omitted thread-bound native sessions_spawn context from fork to isolated, preserves explicit fork overrides, and updates tests, docs, config metadata, and changelog.

Reproducibility: yes. The current-main source path is clear: omitted context plus thread: true reaches resolveThreadBindingSpawnPolicy, whose fallback is currently "fork", and the existing test asserts that fork behavior.

Real behavior proof
Sufficient (terminal): The linked superseded PR body provides copied live terminal output showing after-fix Discord and Telegram policies resolve omitted thread-bound context to isolated, and this PR reapplies that behavioral change with docs/generated sync.

Next step before merge
One narrow docs repair remains: Matrix docs should match the new shared isolated default.

Security
Cleared: The diff changes a runtime default, docs, tests, generated config metadata, and changelog without adding dependency, workflow, secret, download, package, or new code-execution surfaces.

Review findings

  • [P3] Update Matrix thread-binding docs for isolated default — src/channels/thread-bindings-policy.ts:195
Review details

Best possible solution:

Update the remaining Matrix docs, then land the shared fallback and regression coverage once exact-head checks are green while leaving broader stale announce freshness on #78055.

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

Yes. The current-main source path is clear: omitted context plus thread: true reaches resolveThreadBindingSpawnPolicy, whose fallback is currently "fork", and the existing test asserts that fork behavior.

Is this the best way to solve the issue?

Yes, with one docs cleanup. Changing the shared thread-binding policy fallback is the narrowest runtime fix because explicit context: "fork" and configured defaultSpawnContext: "fork" still override it.

Full review comments:

  • [P3] Update Matrix thread-binding docs for isolated default — src/channels/thread-bindings-policy.ts:195
    Changing the shared fallback here affects Matrix too: Matrix's subagent hook calls resolveThreadBindingSpawnPolicy, but docs/channels/matrix.md still tells operators to set defaultSpawnContext: "isolated" when they do not want forks. After this merge that instruction is stale; please update the Matrix docs to say isolated is the default and fork is the opt-in.
    Confidence: 0.86

Overall correctness: patch is correct
Overall confidence: 0.88

Acceptance criteria:

  • pnpm exec oxfmt --check --threads=1 docs/channels/matrix.md
  • pnpm check:docs
  • git diff --check

What I checked:

  • Current main forks omitted thread-bound context: On current main, resolveThreadBindingSpawnPolicy falls back to "fork" when no account/channel/session defaultSpawnContext is configured, and resolveSubagentContextMode uses that policy for thread-requested native subagent spawns. (src/channels/thread-bindings-policy.ts:191, 95a1c915312a)
  • PR changes the shared fallback: The PR diff changes the final fallback from "fork" to "isolated" and updates the thread-bound spawn regression expectations so omitted thread-bound spawns no longer call forkSessionFromParent. (src/channels/thread-bindings-policy.ts:195, 90619451037f)
  • Explicit fork remains available: The precedence order still honors account, channel, and session defaultSpawnContext before the fallback, and the existing explicit context: "fork" test remains in the touched context-mode test file. (src/agents/subagent-spawn.context.test.ts:52, 90619451037f)
  • Related live proof exists on superseded source PR: The superseded source PR body includes copied terminal output from a live OpenClaw 2026.5.6 config showing after-fix Discord and Telegram thread-bound spawn policies resolve defaultSpawnContext to "isolated"; the earlier ClawSweeper review accepted that terminal proof.
  • Remaining Matrix docs drift: Matrix uses resolveThreadBindingSpawnPolicy for native thread-bound subagent spawns, but docs/channels/matrix.md still tells operators to set defaultSpawnContext: "isolated" when they do not want forks, which becomes stale after the shared fallback changes. (docs/channels/matrix.md:562, 95a1c915312a)

Likely related people:

  • @steipete: Recent GitHub path history shows several central changes to thread-bound session spawning and subagent spawn hardening in src/channels/thread-bindings-policy.ts, src/agents/subagent-spawn.ts, and docs/tools/subagents.md. (role: recent maintainer and introducer of adjacent behavior; confidence: high; commits: 8612af754b4d, 4f31cbbf553c, 10b89a3b5552; files: src/channels/thread-bindings-policy.ts, src/agents/subagent-spawn.ts, docs/tools/subagents.md)
  • @vincentkoc: Recent history includes subagent announcement documentation/output work, and the local checkout blame points to this maintainer across the currently materialized affected files. (role: adjacent maintainer; confidence: medium; commits: e80de466e5e1, a6159bb60d0f; files: docs/tools/subagents.md, src/channels/thread-bindings-policy.ts, src/agents/subagent-spawn.ts)
  • @shakkernerd: Recent path history on src/channels/thread-bindings-policy.ts includes thread placement metadata maintenance adjacent to the policy helper changed here. (role: adjacent channel/thread placement owner; confidence: medium; commits: 8b32c3125296; files: src/channels/thread-bindings-policy.ts)

Remaining risk / open question:

  • Matrix thread-binding docs remain stale until the low-priority review finding is addressed.
  • The PR body reports pnpm tsgo:test:src failing on unrelated current-main OpenAI fixture typing, so exact-head typecheck proof still needs maintainer/CI validation before merge.

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

Re-review progress:

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: discord Channel integration: discord channel: telegram Channel integration: telegram docs Improvements or additions to documentation gateway Gateway runtime maintainer Maintainer-authored PR proof: sufficient ClawSweeper judged the real behavior proof convincing. size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant