Spawned-chat lineage: fix subagents-pane routing, sidebar glyph, View parent thread, CLI spawn lineage#861
Conversation
…iew parent thread, CLI spawn lineage Fixes the Subagents-pane row for a spawned ADE chat opening the inline "Transcript not ready yet" drawer instead of routing to the child chat: the canonical dot-form subagent.started twin overwrote the chat:-prefixed taskId in snapshot derivation. Derivation now preserves the navigation alias via one mergeSubagentTaskId helper and exposes an explicit childSessionId + spawnKind on ChatSubagentSnapshot; the panel routes on childSessionId, shows the live child title (Work-level sessionTitleById), and adds an open › affordance. Lineage UX: session cards render a small clickable identicon (matching the parent pane's glyph) left of the status dot for spawned sessions, with parent-title tooltip; the child chat's breadcrumb is renamed to "View parent thread"; the Work select-session listener resolves the target lane for cross-lane jumps; the TUI drawer gains compact sub/peer markers. ade new chat --mode cli now records the same lineage for agent-provider CLI sessions via resumeMetadata (never chatSessionId, which is attached-terminal ownership and would have rescoped the parent's browser capability), projected onto TerminalSessionSummary by the session service. Shell terminals opt out explicitly. Browser-mock parity: lanes listDeleteProgress stub + spawnKind passthrough. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughChangesSpawn lineage
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 67f8d21d94
ℹ️ 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".
| // sessions keep chatSessionId free for true attached-terminal ownership. | ||
| const parentSessionId = readParentSessionId(args); | ||
| const orchestrationParentSessionId = mode === "chat" ? parentSessionId : undefined; | ||
| const orchestrationParentSessionId = parentSessionId; |
There was a problem hiding this comment.
When ade new chat --mode cli --auto-create-lane --parent <sessionId> is used, the resolveNewChatLaneArgs call above has already consumed --parent as the new lane's parentLaneId before readParentSessionId runs here. The CLI lineage path added in this commit therefore falls back to $ADE_CHAT_SESSION_ID/undefined and can also pass a chat session id to create_lane as a lane parent, so the documented explicit parent override for auto-lane CLI spawns either fails lane creation or creates an unlinked child. Read the lineage parent before lane parsing or keep --parent lane-only and require the parent-session aliases for lineage.
Useful? React with 👍 / 👎.
…eate-lane resolveNewChatLaneArgs consumed bare --parent as parentLaneId before readParentSessionId ran, silently eating the documented lineage flag whenever --auto-create-lane was used. Lane parentage now requires the explicit --parent-lane/--parent-lane-id aliases in ade new chat. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f133650c98
ℹ️ 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".
| ...(orchestrationParentSessionId ? { orchestrationParentSessionId } : {}), | ||
| ...(spawnKind ? { spawnKind } : {}), |
There was a problem hiding this comment.
When ade new chat --mode cli --type subagent|peer records a parent here, the metadata is persisted but never reaches the spawned CLI process environment: ptyService builds env only from chatSessionId/ownerSessionId, so the child gets ADE_CHAT_SESSION_ID set to its own terminal id and no ADE_PARENT_CHAT_SESSION_ID/ADE_SPAWN_KIND. In that spawned-CLI scenario, any follow-up ade new chat or self-report defaults to the terminal itself instead of the real parent chat, so the documented subagent/peer reporting lineage is lost even though the sidebar shows it.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/ade-cli/src/cli.test.ts`:
- Line 2508: Remove the duplicate block-scoped declaration of value in the
affected test scope, reusing the existing variable when validating staticPlan.
Ensure the launch and optional createLane type shape remains available without
redeclaring the identifier.
- Line 2491: Remove the duplicate block-scoped declaration of value in the
affected test scope, retaining a single declaration with the existing staticPlan
launch/createLane type. Update subsequent references to use that retained
variable without redeclaring it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cf45ff97-ce64-4384-b06f-20cfbbaef080
⛔ Files ignored due to path filters (6)
docs/features/agents/README.mdis excluded by!docs/**docs/features/chat/README.mdis excluded by!docs/**docs/features/chat/composer-and-ui.mdis excluded by!docs/**docs/features/terminals-and-sessions/README.mdis excluded by!docs/**docs/features/terminals-and-sessions/pty-and-processes.mdis excluded by!docs/**docs/features/terminals-and-sessions/ui-surfaces.mdis excluded by!docs/**
📒 Files selected for processing (26)
apps/ade-cli/README.mdapps/ade-cli/src/adeRpcServer.test.tsapps/ade-cli/src/adeRpcServer.tsapps/ade-cli/src/cli.test.tsapps/ade-cli/src/cli.tsapps/ade-cli/src/tuiClient/__tests__/Drawer.test.tsxapps/ade-cli/src/tuiClient/app.tsxapps/ade-cli/src/tuiClient/closedCliSessions.tsapps/ade-cli/src/tuiClient/components/Drawer.tsxapps/desktop/src/main/services/sessions/sessionService.test.tsapps/desktop/src/main/services/sessions/sessionService.tsapps/desktop/src/renderer/browserMock.tsapps/desktop/src/renderer/components/chat/AgentChatPane.tsxapps/desktop/src/renderer/components/chat/ChatSubagentsPanel.test.tsxapps/desktop/src/renderer/components/chat/ChatSubagentsPanel.tsxapps/desktop/src/renderer/components/chat/chatExecutionSummary.test.tsapps/desktop/src/renderer/components/chat/chatExecutionSummary.tsapps/desktop/src/renderer/components/chat/chatSubagentIdentity.tsxapps/desktop/src/renderer/components/chat/chatTranscriptRows.test.tsapps/desktop/src/renderer/components/terminals/SessionCard.test.tsxapps/desktop/src/renderer/components/terminals/SessionCard.tsxapps/desktop/src/renderer/components/terminals/SessionListPane.tsxapps/desktop/src/renderer/components/terminals/TerminalsPage.test.tsxapps/desktop/src/renderer/components/terminals/TerminalsPage.tsxapps/desktop/src/renderer/components/terminals/WorkViewArea.tsxapps/desktop/src/shared/types/sessions.ts
| "--print-config", | ||
| ]); | ||
| const staticPlan = expectStaticPlan(plan); | ||
| const value = staticPlan.value as { launch: Record<string, unknown>; createLane?: Record<string, unknown> }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Duplicate block-scoped variable declaration.
This duplicate declaration will cause a TypeScript compilation error. This finding shares a root cause with another site; see the consolidated comment at the end of the review for the resolution.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawn } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/ade-cli/src/cli.test.ts` at line 2491, Remove the duplicate block-scoped
declaration of value in the affected test scope, retaining a single declaration
with the existing staticPlan launch/createLane type. Update subsequent
references to use that retained variable without redeclaring it.
| "--print-config", | ||
| ]); | ||
| const staticPlan = expectStaticPlan(plan); | ||
| const value = staticPlan.value as { launch: Record<string, unknown>; createLane?: Record<string, unknown> }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Duplicate block-scoped variable declaration.
This duplicate declaration will cause a TypeScript compilation error. This finding shares a root cause with another site; see the consolidated comment at the end of the review for the resolution.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawn } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/ade-cli/src/cli.test.ts` at line 2508, Remove the duplicate block-scoped
declaration of value in the affected test scope, reusing the existing variable
when validating staticPlan. Ensure the launch and optional createLane type shape
remains available without redeclaring the identifier.
… agent CLIs Persisted spawn lineage never reached the spawned CLI process env, so a CLI child could not self-report to its parent chat the way chat-mode children can. PtyCreateArgs gains an optional spawnLineage passthrough; child identity (ADE_CHAT_SESSION_ID, browser capability) is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e167b4613
ℹ️ 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".
| if (args.spawnLineage) { | ||
| next.ADE_PARENT_CHAT_SESSION_ID = args.spawnLineage.parentChatSessionId; | ||
| next.ADE_SPAWN_KIND = args.spawnLineage.spawnKind ?? ""; |
There was a problem hiding this comment.
When this runs inside a headless/runtime process that already inherited ADE_PARENT_CHAT_SESSION_ID (for example, a nested spawned CLI), launching a new tracked CLI with --no-parent/no orchestrationParentSessionId keeps the old parent variables from baseLaunchEnv because this branch only overwrites them when spawnLineage is present. That makes an explicitly unlinked child inherit a stale parent id and can cause any self-reporting or env-based routing to target the wrong parent; clear ADE_PARENT_CHAT_SESSION_ID and ADE_SPAWN_KIND in the no-lineage path.
Useful? React with 👍 / 👎.
…ithout lineage Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 34d8ad3eeb
ℹ️ 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".
| laneId, | ||
| chatSessionId, | ||
| ownerSessionId: isTrackedAgentCliToolType(toolTypeHint) ? sessionId : null, | ||
| spawnLineage: args.spawnLineage, |
There was a problem hiding this comment.
When a spawned provider CLI exits and the user resumes it via pty.resumeSession/sendToSession, getOrCreateResumeFlight re-enters create() with existingSession.resumeMetadata but no args.spawnLineage. This line only forwards args.spawnLineage into withAdeTerminalContextEnv, so the resume launch deletes ADE_PARENT_CHAT_SESSION_ID/ADE_SPAWN_KIND even though the persisted metadata still has orchestrationParentSessionId/spawnKind; resumed subagents lose the parent env they need for self-reporting and nested lineage.
Useful? React with 👍 / 👎.
… resume Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const sessionTitleById = useMemo( | ||
| () => new Map(sessions.map((session) => [session.id, session.title] as const)), | ||
| [sessions], | ||
| ); |
There was a problem hiding this comment.
TerminalSessionSummary.title is typed string | null, so this produces a Map<string, string | null> — not assignable to the ReadonlyMap<string, string> declared on AgentChatPane's sessionTitleById prop. TypeScript will reject the assignment because Map.get() would return string | null | undefined where string | undefined is expected. SessionListPane.tsx avoids this by using primarySessionLabel(session), which always returns a non-null string.
| const sessionTitleById = useMemo( | |
| () => new Map(sessions.map((session) => [session.id, session.title] as const)), | |
| [sessions], | |
| ); | |
| const sessionTitleById = useMemo( | |
| () => new Map(sessions.map((session) => [session.id, session.title ?? ""] as const)), | |
| [sessions], | |
| ); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/components/terminals/WorkViewArea.tsx
Line: 1076-1079
Comment:
`TerminalSessionSummary.title` is typed `string | null`, so this produces a `Map<string, string | null>` — not assignable to the `ReadonlyMap<string, string>` declared on `AgentChatPane`'s `sessionTitleById` prop. TypeScript will reject the assignment because `Map.get()` would return `string | null | undefined` where `string | undefined` is expected. `SessionListPane.tsx` avoids this by using `primarySessionLabel(session)`, which always returns a non-null string.
```suggestion
const sessionTitleById = useMemo(
() => new Map(sessions.map((session) => [session.id, session.title ?? ""] as const)),
[sessions],
);
```
How can I resolve this? If you propose a fix, please make it concise.
There was a problem hiding this comment.
False positive: this map is built from sessions: TerminalSessionSummary[] (WorkViewArea props, line 1024), and TerminalSessionSummary.title is non-nullable string (shared/types/sessions.ts:125) — the string | null title belongs to the persisted session record type, not the summary. tsc --noEmit passes on this branch.
There was a problem hiding this comment.
You're right, this is a false positive. TerminalSessionSummary.title is typed as string (non-nullable) — confirmed in apps/desktop/src/shared/types/sessions.ts. The string | null variant belongs to ClaudeSessionPointer.title, which is a different type. The map construction is correct as written and TypeScript won't reject it. Sorry for the noise.
1 participant
Summary by CodeRabbit
New Features
subagentandpeersession types, including parent-session options.Bug Fixes
Documentation
Greptile Summary
This PR wires full spawn-lineage metadata (parent session ID + spawn kind) through the subagents pane, the Work sidebar session cards, and the ADE CLI — fixing routing, identity preservation, and UI navigation for spawned chats and tracked CLI sessions.
childSessionId(set during event derivation) instead of a fragilechat:-prefix check, preserving the link across the dot-twin event that previously stripped the prefix; live chat titles are resolved and displayed in spawned-chat rows.orchestrationParentSessionIdand navigates to the parent chat on click;TerminalsPagenow resolves the target lane automatically when the event omits it.orchestrationParentSessionIdandspawnKindare accepted by thecreatetool and written into initialresumeMetadata;ptyServiceexportsADE_PARENT_CHAT_SESSION_ID/ADE_SPAWN_KINDto spawned-agent environments and recovers them from persisted metadata on resume.Confidence Score: 4/5
Safe to merge after fixing the type error in WorkViewArea; all other changes are well-tested and the logic is sound.
WorkViewArea.tsxmapssession.title(typedstring | null) into aMap<string, string | null>and passes it whereReadonlyMap<string, string>is expected — a TypeScript compile error on the changed path. Everything else, including the dual-event preservation inchatExecutionSummary.ts, the lane-resolution fix inTerminalsPage.tsx, and the resume-metadata spread insessionService.ts, looks correct and is covered by new tests.apps/desktop/src/renderer/components/terminals/WorkViewArea.tsx — the
sessionTitleByIdmap storesstring | nullvalues.Important Files Changed
sessionTitleByIdmap and threads it intoSessionSurface/AgentChatPane; map built from rawsession.title(stringchildSessionId/spawnKindtoChatSubagentSnapshotand introducesmergeSubagentTaskId/mergeSubagentChildSessionIdhelpers that preserve thechat:-prefixed taskId and spawnKind across the dot-twin overwrite and later progress/result merges.childSessionId(replacing the fragilechat:-prefix check), addsresolveSpawnedChatTitleprop for live title resolution, and shows a navigation indicator on spawned-chat rows.laneIdfrom the loaded session list when theade:work:select-sessionevent omits it (as spawned-chat navigation always does).orchestrationParentSessionIdthat navigates to the parent chat on click; intentionally uses aspan(not nested button) to avoid invalid HTML while preserving mouse navigation.normalizeResumeMetadatato parseorchestrationParentSessionId/spawnKindfrom stored metadata; adds...currentMetadataspread so lineage fields survive subsequent resume-command updates.ADE_PARENT_CHAT_SESSION_ID/ADE_SPAWN_KINDto spawned-agent environments and clears them for unrelated terminals; recovers lineage from persisted resume metadata on session resume.createRPC tool withorchestrationParentSessionId/spawnKindparams; constructs initialresumeMetadatacarrying spawn lineage for agent-provider CLI sessions.--parentalias from lane parentage resolution to prevent ambiguity with the spawn-lineage--parentflag.Prompt To Fix All With AI
Reviews (2): Last reviewed commit: "fix(pty): restore spawn-lineage env from..." | Re-trigger Greptile