fix(agents): inherit subagent thinking defaults#84007
Conversation
|
Codex review: needs maintainer review before merge. Reviewed May 25, 2026, 10:45 PM ET / 02:45 UTC. Summary PR surface: Source +84, Tests +388. Total +472 across 7 files. Reproducibility: yes. Current Review metrics: 1 noteworthy metric.
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 focused agents fix after required checks finish, preserving explicit/configured subagent thinking precedence ahead of requester inheritance. Do we have a high-confidence way to reproduce the issue? Yes. Current Is this the best way to solve the issue? Yes. The PR fixes the missing resolver input at the native spawn seam, keeps explicit and configured subagent thinking ahead of caller inheritance, and adds focused regression coverage for the precedence. AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against 609d70d35e4f. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +84, Tests +388. Total +472 across 7 files. View PR surface stats
Acceptance criteria:
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
|
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
martingarramon
left a comment
There was a problem hiding this comment.
The fallback chain in resolveSubagentThinkingOverride is correctly ordered and the session-state read is safe (readRequesterThinkingLevel catches errors and returns undefined, so a missing or unreadable session store doesn't break spawning). One documentation mismatch to fix before merge:
PR body description doesn't match code
The summary says the fallback order is "explicit spawn thinking, target agent subagent default, global subagent default, then requester session thinking." But the code places requesterAgentConfig.subagents.thinking before targetAgentConfig.subagents.thinking in resolvedThinkingDefaultRaw, and the test "prefers requester-agent subagent thinking over target-agent subagent thinking" confirms this is intentional. The correct order is: explicit spawn → requester agent config → target agent config → global config → requester session thinking.
The body should document this precedence clearly so maintainers can consciously approve it.
Core implementation: confirmed correct
readRequesterThinkingLevel reads entry.thinkingLevel from the requester's session store — the right source. The thinkingCandidateRaw fallback order is correctly guarded: params.thinkingOverrideRaw || resolvedThinkingDefaultRaw || params.callerThinkingRaw. Config-derived values take precedence over session state.
|
@martingarramon thanks for the catch. I updated the PR body to match the implemented precedence exactly: explicit spawn -> requester agent config -> target agent config -> global config -> requester session thinking. No code change was needed because the resolver/tests already enforce this order. |
|
Pi proof for PR #84007 is now complete. Environment:
Commands run on the Pi via the paired OpenClaw node host: Result: This covers the requested
Main also restored the Pi node exec approval file after the proof run. |
|
The description now matches the implemented precedence. Pi proof looks clean — aarch64 kernel, branch checked out directly, 5 thinking-inheritance tests pass. LGTM. |
|
ClawSweeper PR egg ✨ Hatched: 🥚 common Frosted Merge Sprite Hatch commandComment Hatchability rules:
Rarity: 🥚 common. What is this egg doing here?
|
|
@clawsweeper re-review I moved the real Pi proof into the PR body using the exact proof field labels. Proof summary:
martingarramon also reviewed the updated precedence wording and Pi proof and left LGTM: #84007 (comment) |
|
@clawsweeper re-review Added the requested rank-up proof to the PR body: redacted live native sessions_spawn output showing inherited child thinking level. Key evidence:
I redacted auth/account details and did not paste full prompts. The earlier Pi proof remains in the PR body for branch-checkout aarch64 test coverage. |
|
PR-Machine stale-base refresh for Pushed merge commit Validation run after the merge:
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
LGTM holds on |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Pushed Validation:
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Body updated. The documented precedence now puts requester agent subagent default before target agent subagent default, matching resolvedThinkingDefaultRaw and the test expectation. LGTM. |
|
Updated Validation passed:
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Updated Removed the unrelated generated viewer/template/script/UI/extension churn from the PR diff by rebuilding the branch on current Validation passed:
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
PR body documents the precedence as explicit spawn → requester agent config → target agent config → global config → requester session thinking, matching |
Summary
Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Motivation
A user-selected session thinking level should carry into subagents by default. Before this change, a parent session could run with one thinking setting while a child session silently lost that setting unless the spawn request or config repeated it.
Real behavior proof (required for external PRs)
sessions_spawn(runtime="subagent")now preserves requester session thinking as the final fallback when no explicit or configured subagent thinking is set.fix-subagent-thinking-defaults../node_modules/.bin/vitest run src/agents/subagent-spawn.test.ts --config test/vitest/vitest.agents.config.ts./node_modules/.bin/vitest run src/agents/openclaw-tools.subagents.sessions-spawn-applies-thinking-default.test.ts --config test/vitest/vitest.unit-fast.config.tsthinkingLevelonto the child session, while explicit/configured thinking continues to take precedence.Root Cause (if applicable)
The subagent thinking resolver only knew about explicit spawn input and configured subagent defaults. It did not receive requester session thinking, so it had no way to preserve the caller's persisted thinking level.
Regression Test Plan (if applicable)
src/agents/openclaw-tools.subagents.sessions-spawn-applies-thinking-default.test.tssrc/agents/subagent-spawn.test.tsUser-visible / Behavior Changes
Subagents spawned without explicit or configured subagent thinking now inherit the requester session's thinking level.
Diagram (if applicable)
Security Impact (required)
Yes/No) NoYes/No) NoYes/No) NoYes/No) NoYes/No) NoRepro + Verification
Environment
agents.defaults.subagents.thinkingand mocked requester session store entriesSteps
Expected
Subagent thinking precedence is explicit spawn value, requester agent default, target agent default, global default, then requester session thinking.
Actual
The new tests match that precedence and verify child-session persistence.
Evidence
Included evidence:
Human Verification (required)
Review Conversations
Compatibility / Migration
Yes/No) YesYes/No) NoYes/No) NoRisks and Mitigations
Pi real behavior proof — 2026-05-20
Behavior or issue addressed:
Real environment tested:
Exact steps or command run after this patch:
Evidence after fix:
Observed result after fix:
What was not tested:
Live native sessions_spawn proof — 2026-05-20
Behavior or issue addressed:
Real environment tested:
Exact steps or command run after this patch:
Evidence after fix:
Observed result after fix:
What was not tested: