Skip to content

Clear stale subagent lineage on top-level sessions#67946

Open
pfrederiksen wants to merge 1 commit intoopenclaw:mainfrom
pfrederiksen:fix/stale-subagent-lineage-depth
Open

Clear stale subagent lineage on top-level sessions#67946
pfrederiksen wants to merge 1 commit intoopenclaw:mainfrom
pfrederiksen:fix/stale-subagent-lineage-depth

Conversation

@pfrederiksen
Copy link
Copy Markdown

Fixes #67943.

Summary

Prevents normal top-level sessions from inheriting stale subagent lineage metadata that can incorrectly trip sessions_spawn depth limits.

What changed

  • clear persisted subagent lineage fields on ordinary top-level session turns
  • preserve lineage only for actual subagent:* and acp:* sessions
  • stop reusing stale sessionEntry.spawnedBy for normal top-level agent runs
  • add regression coverage proving top-level sessions no longer pass stale lineage through

Validation

  • npm test -- --run src/agents/agent-command.live-model-switch.test.ts

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: S labels Apr 17, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 17, 2026

Greptile Summary

This PR fixes issue #67943 by preventing regular top-level sessions from inheriting stale subagent lineage metadata (spawnedBy, spawnDepth, subagentRole, subagentControlScope, forkedFromParent, spawnedWorkspaceDir) that could incorrectly trip sessions_spawn depth limits. The fix gates these fields behind a preserveSpawnLineage check (keyed on isSubagentSessionKey || isAcpSessionKey) in both session.ts (at session state initialization) and agent-command.ts (at the point spawnedBy is passed to runAgentAttempt), and adds a regression test confirming spawnedBy is undefined for top-level session runs.

Confidence Score: 5/5

Safe to merge — targeted fix with a clear regression test and no collateral risk.

All three changed files are narrowly scoped: the production changes gate stale lineage fields behind a well-tested predicate, the test converts a static mock to a spy and adds a regression case that directly covers the bug scenario. No P0/P1 findings.

No files require special attention.

Reviews (1): Last reviewed commit: "clear stale subagent lineage on top-leve..." | Re-trigger Greptile

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7f772863cb

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/auto-reply/reply/session.ts Outdated
@pfrederiksen
Copy link
Copy Markdown
Author

Good catch. I narrowed the cleanup so it only clears stale subagent lineage fields, while preserving for normal thread sessions that have already forked from their parent. Re-pushed.

@openclaw-barnacle openclaw-barnacle Bot added gateway Gateway runtime size: M and removed size: S labels Apr 17, 2026
@pfrederiksen
Copy link
Copy Markdown
Author

CI is green now, this is ready to merge. ✅

@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented Apr 27, 2026

Codex review: needs maintainer review before merge.

What this changes:

The PR gates persisted subagent lineage fields to subagent/ACP session keys, stops top-level agent runs from reusing stale sessionEntry.spawnedBy, and adds regression coverage for top-level sessions with stale stored lineage.

Maintainer follow-up before merge:

This is an existing contributor PR that appears correct but needs maintainer handling for the dirty merge state and normal landing judgment, not a separate automated replacement PR.

Security review:

Security review cleared: The diff only changes session lineage handling and a focused test; it does not touch dependencies, CI workflows, secrets, package metadata, downloads, or other supply-chain surfaces.

Review details

Best possible solution:

Rebase or otherwise resolve the dirty merge state, rerun the changed checks, then land this narrow lineage-gating fix if the diff remains limited to the session-state and agent-attempt paths plus focused regression coverage.

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

Yes. A high-confidence code-level reproduction is to seed a normal top-level session entry with stale spawnedBy or spawnDepth; current main carries that metadata into initSessionState and runAgentAttempt, and the PR's added test covers the spawnedBy path.

Is this the best way to solve the issue?

Yes, after resolving merge conflicts. The PR uses the existing session-key classifiers to preserve lineage for real subagent/ACP sessions while clearing it for ordinary top-level sessions, and it avoids the earlier fork-marker regression by preserving parentSessionKey and forkedFromParent.

Acceptance criteria:

  • After conflict resolution, rerun the changed PR checks or equivalent CI; the touched-surface local target would be pnpm test src/agents/agent-command.live-model-switch.test.ts plus the changed gate.

What I checked:

  • current main preserves stale lineage: Current main copies spawnedBy, spawnedWorkspaceDir, spawnDepth, subagentRole, and subagentControlScope from the existing session entry without checking whether the session key is actually a subagent or ACP session. (src/auto-reply/reply/session.ts:646, 7969f1f07ccc)
  • current main reuses stale spawnedBy in agent attempts: Current main passes normalizedSpawned.spawnedBy ?? sessionEntry?.spawnedBy into runAgentAttempt, so a normal top-level run can inherit stale stored parentage. (src/agents/agent-command.ts:937, 7969f1f07ccc)
  • depth calculation depends on stored lineage: getSubagentDepthFromSessionStore prefers stored spawnDepth and otherwise follows entry.spawnedBy, which explains why stale stored lineage can trip sessions_spawn depth checks. (src/agents/subagent-depth.ts:150, 7969f1f07ccc)
  • PR gates lineage preservation: The fetched PR diff adds preserveSpawnLineage = isSubagentSessionKey(sessionKey) || isAcpSessionKey(sessionKey) and only carries spawned lineage fields when that predicate is true, while leaving parentSessionKey and forkedFromParent available for normal thread fork behavior. (src/auto-reply/reply/session.ts:618, 9b21e1ad94f0)
  • PR regression coverage: The PR adds a regression test that seeds a normal top-level session entry with stale spawnedBy and asserts runAgentAttempt receives spawnedBy: undefined. (src/agents/agent-command.live-model-switch.test.ts:509, 9b21e1ad94f0)
  • subagent session contract: The docs describe subagents as running in their own agent:<agentId>:subagent:<uuid> session, matching the PR's isSubagentSessionKey preservation boundary. Public docs: docs/tools/subagents.md. (docs/tools/subagents.md:11, 7969f1f07ccc)

Likely related people:

  • steipete: Recent public commit history shows repeated maintenance in agent-command.ts, subagent-spawn.ts, and session reset/state paths, including subagent spawn policy and model/runtime fixes adjacent to this PR. (role: recent maintainer; confidence: high; commits: 93d5cd10151f, 6c0cdf43e48e, 2e99c1d22726; files: src/agents/agent-command.ts, src/agents/subagent-spawn.ts, src/auto-reply/reply/session.ts)
  • tyler6204: The nested subagent orchestration and depth-control behavior appears in the merged Agents: add nested subagent orchestration controls and reduce subagent token waste history, which is central to the depth-limit symptom this PR fixes. (role: introduced adjacent behavior; confidence: medium; commits: b8f66c260db8; files: src/agents/subagent-depth.ts, src/agents/subagent-spawn.ts, docs/tools/subagents.md)
  • vincentkoc: Current local blame and public path history show recent maintenance in session rollover/state and subagent depth helper surfaces touched by this review. (role: recent maintainer; confidence: medium; commits: bf0d2d70be51, 74e7b8d47b18, 93fbe26adbbc; files: src/auto-reply/reply/session.ts, src/agents/subagent-depth.ts)

Remaining risk / open question:

  • GitHub reports the PR branch as dirty/unmergeable against the current base, so the reviewed diff may need conflict resolution before merge.
  • I did not run local tests because this review was required to keep the checkout read-only; validation is from source inspection, the fetched PR diff, and public PR check-run results.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 7969f1f07ccc.

@pfrederiksen pfrederiksen force-pushed the fix/stale-subagent-lineage-depth branch from f3d2367 to 9b21e1a Compare April 28, 2026 22:11
@openclaw-barnacle openclaw-barnacle Bot added size: S and removed gateway Gateway runtime size: M labels Apr 28, 2026
@pfrederiksen
Copy link
Copy Markdown
Author

pfrederiksen commented Apr 28, 2026

Resolved the review concerns.

I rebuilt the branch on current main and force-pushed a clean single-commit version. GitHub now reports the branch as mergeable.

I also dropped the unrelated changes called out in review:

  • skills test changes
  • gateway hook test changes
  • plugin-SDK manifest-like type change
  • extension-boundary assertion change

The PR diff is now limited to the stale top-level spawn-lineage fix plus focused regression coverage in:

  • src/auto-reply/reply/session.ts
  • src/agents/agent-command.ts
  • src/agents/agent-command.live-model-switch.test.ts

Validation run locally:

  • focused oxlint on touched files: passed
  • agent-command live model switch regression test: 26 passed across the relevant projects
  • pnpm tsgo:core:test: passed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Normal sessions can inherit stale subagent lineage and incorrectly hit sessions_spawn depth limits

1 participant