-
Notifications
You must be signed in to change notification settings - Fork 7
Spawned-chat lineage: fix subagents-pane routing, sidebar glyph, View parent thread, CLI spawn lineage #861
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
67f8d21
f133650
2e167b4
34d8ad3
6f7dc9f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2442,6 +2442,90 @@ describe("ADE CLI", () => { | |
| expect((staticPlan.value as { launch: Record<string, unknown> }).launch.orchestrationParentSessionId) | ||
| .toBe("parent-session-1"); | ||
| }); | ||
|
|
||
| it("ade new chat --mode cli records the env parent as spawn lineage without terminal ownership", () => { | ||
| process.env.ADE_CHAT_SESSION_ID = "parent-session-1"; | ||
| const plan = buildCliPlan([ | ||
| "new", "chat", | ||
| "--mode", "cli", | ||
| "--lane", "lane-1", | ||
| "--provider", "codex", | ||
| "--type", "peer", | ||
| "--print-config", | ||
| ]); | ||
| const staticPlan = expectStaticPlan(plan); | ||
| const launch = (staticPlan.value as { launch: Record<string, unknown> }).launch; | ||
| expect(launch.orchestrationParentSessionId).toBe("parent-session-1"); | ||
| expect(launch.spawnKind).toBe("peer"); | ||
| expect(launch).not.toHaveProperty("chatSessionId"); | ||
| }); | ||
|
|
||
| it("ade new chat --mode cli --no-parent leaves the terminal unlinked", () => { | ||
| process.env.ADE_CHAT_SESSION_ID = "parent-session-1"; | ||
| const plan = buildCliPlan([ | ||
| "new", "chat", | ||
| "--mode", "cli", | ||
| "--lane", "lane-1", | ||
| "--provider", "codex", | ||
| "--no-parent", | ||
| "--print-config", | ||
| ]); | ||
| const staticPlan = expectStaticPlan(plan); | ||
| const launch = (staticPlan.value as { launch: Record<string, unknown> }).launch; | ||
| expect(launch).not.toHaveProperty("orchestrationParentSessionId"); | ||
| expect(launch).not.toHaveProperty("chatSessionId"); | ||
| }); | ||
|
|
||
| it("--auto-create-lane keeps --parent as the spawn-lineage session, not the lane parent", () => { | ||
| delete process.env.ADE_CHAT_SESSION_ID; | ||
| const plan = buildCliPlan([ | ||
| "new", "chat", | ||
| "--mode", "chat", | ||
| "--auto-create-lane", | ||
| "--lane-name", "spawn-target", | ||
| "--provider", "codex", | ||
| "--parent", "parent-session-1", | ||
| "--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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 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. (detect-child-process-typescript) 🤖 Prompt for AI Agents |
||
| expect(value.launch.orchestrationParentSessionId).toBe("parent-session-1"); | ||
| expect(value.createLane ?? {}).not.toHaveProperty("parentLaneId"); | ||
| }); | ||
|
|
||
| it("--auto-create-lane --parent-lane still sets the lane parent without touching lineage", () => { | ||
| delete process.env.ADE_CHAT_SESSION_ID; | ||
| const plan = buildCliPlan([ | ||
| "new", "chat", | ||
| "--mode", "chat", | ||
| "--auto-create-lane", | ||
| "--lane-name", "spawn-target", | ||
| "--provider", "codex", | ||
| "--parent-lane", "lane-parent-1", | ||
| "--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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 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. (detect-child-process-typescript) 🤖 Prompt for AI Agents |
||
| expect(value.createLane?.parentLaneId).toBe("lane-parent-1"); | ||
| expect(value.launch).not.toHaveProperty("orchestrationParentSessionId"); | ||
| }); | ||
|
|
||
| it("does not record spawn lineage for plain shell terminals", () => { | ||
| process.env.ADE_CHAT_SESSION_ID = "parent-session-1"; | ||
| const plan = buildCliPlan([ | ||
| "new", "chat", | ||
| "--mode", "cli", | ||
| "--lane", "lane-1", | ||
| "--provider", "shell", | ||
| "--type", "peer", | ||
| "--print-config", | ||
| ]); | ||
| const staticPlan = expectStaticPlan(plan); | ||
| const launch = (staticPlan.value as { launch: Record<string, unknown> }).launch; | ||
| expect(launch.provider).toBe("shell"); | ||
| expect(launch).not.toHaveProperty("orchestrationParentSessionId"); | ||
| expect(launch).not.toHaveProperty("spawnKind"); | ||
| }); | ||
| }); | ||
|
|
||
| it("prints chat create --prompt dry-run with the follow-up send", () => { | ||
|
|
@@ -5680,6 +5764,15 @@ describe("ADE CLI", () => { | |
| expect(newChatHelp.kind).toBe("help"); | ||
| if (newChatHelp.kind !== "help") return; | ||
| expect(newChatHelp.text).toContain("--type <subagent|peer|none>"); | ||
| expect(newChatHelp.text).toContain("Override with --parent <sessionId>"); | ||
| expect(newChatHelp.text).toContain("plain shell terminals don't record lineage"); | ||
|
|
||
| const newHelp = buildCliPlan(["new", "--help"]); | ||
| expect(newHelp.kind).toBe("help"); | ||
| if (newHelp.kind !== "help") return; | ||
| expect(newHelp.text).toContain("--type <subagent|peer|none>"); | ||
| expect(newHelp.text).toContain("--parent <sessionId>"); | ||
| expect(newHelp.text).toContain("Plain shell terminals do not record lineage"); | ||
|
|
||
| const laneCommandHelp = buildCliPlan(["help", "lanes"]); | ||
| expect(laneCommandHelp.kind).toBe("help"); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1378,6 +1378,12 @@ const HELP_BY_COMMAND: Record<string, string> = { | |
| --no-fast, --standard Disable fast service tier explicitly. | ||
| --prompt <text> First chat message or CLI initial input. | ||
|
|
||
| Spawn lineage: | ||
| In tracked agent shells, chat mode and agent-provider CLI mode default | ||
| their parent to $ADE_CHAT_SESSION_ID. Use --parent <sessionId> to override | ||
| it or --no-parent to opt out. CLI terminals record lineage without taking | ||
| attached-terminal ownership. Plain shell terminals do not record lineage. | ||
|
|
||
| Compatibility: | ||
| ade chat create still creates persistent Work chats. | ||
| ade shell start-cli still starts tracked provider CLI sessions. | ||
|
|
@@ -1397,6 +1403,12 @@ const HELP_BY_COMMAND: Record<string, string> = { | |
| The command defaults to the current ADE lane when ADE_LANE_ID is set. Use | ||
| --auto-create-lane or --lane auto to create a lane before launching. | ||
| --type <subagent|peer|none> sets only the cosmetic relationship and completion-report policy; a typed agent is a full agent. subagent wakes the parent, peer leaves a quiet note, and none (default) sends no report. | ||
|
|
||
| Spawn lineage: when run from a tracked agent shell (ADE_CHAT_SESSION_ID set), | ||
| the new session links back to the spawning chat. In CLI mode, the terminal | ||
| shows that spawn lineage on its own sidebar card and links back to the chat | ||
| (agent providers only — plain shell terminals don't record lineage). | ||
| Override with --parent <sessionId>; opt out with --no-parent. | ||
| `, | ||
| lanes: `${ADE_BANNER} | ||
| Lanes | ||
|
|
@@ -2535,12 +2547,12 @@ function readLaneId(args: string[]): string | null { | |
| } | ||
|
|
||
| /** | ||
| * Parent chat-session lineage for spawned chat sessions. Defaults to the | ||
| * spawning agent's own session — ADE injects ADE_CHAT_SESSION_ID into every | ||
| * tracked agent shell (chat runtimes and tracked CLI/PTY sessions) — so a | ||
| * child created via `ade chat create` links back to the chat that spawned it | ||
| * instead of becoming an orphan. `--parent <sessionId>` overrides the | ||
| * default; `--no-parent` opts out entirely. | ||
| * Parent chat-session lineage for spawned chat and agent-provider CLI | ||
| * sessions. Defaults to the spawning agent's own session — ADE injects | ||
| * ADE_CHAT_SESSION_ID into every tracked agent shell (chat runtimes and | ||
| * tracked CLI/PTY sessions). `--parent <sessionId>` overrides the default; | ||
| * `--no-parent` opts out entirely. CLI callers keep chatSessionId separate | ||
| * because that field represents attached-terminal ownership, not lineage. | ||
| */ | ||
| function readParentSessionId(args: string[]): string | undefined { | ||
| const override = readValue(args, ["--parent", "--parent-session", "--parent-session-id"]); | ||
|
|
@@ -4163,7 +4175,11 @@ function resolveNewChatLaneArgs(args: string[], prompt: string | null): { | |
| maybePut(createLaneArgs, "description", readValue(args, ["--description", "--desc"])); | ||
| maybePut(createLaneArgs, "baseBranch", readValue(args, ["--base", "--base-branch"])); | ||
| maybePut(createLaneArgs, "branchName", readValue(args, ["--branch-name"])); | ||
| maybePut(createLaneArgs, "parentLaneId", readValue(args, ["--parent", "--parent-lane", "--parent-lane-id"])); | ||
| // No bare `--parent` alias here: in `ade new chat` that flag is documented as | ||
| // the spawn-lineage parent SESSION (readParentSessionId), and this resolver | ||
| // runs first — consuming it would silently eat the lineage flag whenever | ||
| // --auto-create-lane is used. Lane parentage keeps the explicit aliases. | ||
| maybePut(createLaneArgs, "parentLaneId", readValue(args, ["--parent-lane", "--parent-lane-id"])); | ||
| return { laneId: null, autoCreateLane: true, createLaneArgs }; | ||
| } | ||
|
|
||
|
|
@@ -4184,7 +4200,7 @@ function buildNewChatPlan(args: string[], defaultMode: "chat" | "cli"): CliPlan | |
| const reasoningEffort = readValue(args, ["--reasoning-effort", "--effort", "--reasoning"]); | ||
| const permissionMode = readValue(args, ["--permission-mode", "--permissions"]); | ||
| const spawnTypeArg = readValue(args, ["--type", "--spawn-type"]); | ||
| const spawnKind = mode === "chat" ? spawnTypeArg?.trim().toLowerCase() : undefined; | ||
| const spawnKind = spawnTypeArg?.trim().toLowerCase(); | ||
| const fastMode = readFastModeFlag(args); | ||
| const title = readValue(args, ["--title"]); | ||
| const printConfig = readFlag(args, ["--print-config", "--dry-run"]); | ||
|
|
@@ -4218,10 +4234,10 @@ function buildNewChatPlan(args: string[], defaultMode: "chat" | "cli"): CliPlan | |
| }; | ||
|
|
||
| // Consume the flags in both modes so they never leak into the generic arg | ||
| // bag; lineage only applies to chat mode (a PTY session is not a chat | ||
| // record, so there is nothing to link). | ||
| // bag. Both chat and CLI modes record the same spawn lineage fields; CLI | ||
| // 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When Useful? React with 👍 / 👎. |
||
| const launchArgs = mode === "chat" | ||
| ? collectGenericObjectArgs(args, { | ||
| provider, | ||
|
|
@@ -4249,6 +4265,10 @@ function buildNewChatPlan(args: string[], defaultMode: "chat" | "cli"): CliPlan | |
| modelId: modelArg, | ||
| reasoningEffort, | ||
| ...(fastMode !== undefined ? { fastMode, codexFastMode: fastMode } : {}), | ||
| // Spawn lineage rides on resume metadata, which only agent providers | ||
| // have — plain shell terminals can't persist it, so don't pretend. | ||
| ...(provider !== "shell" && orchestrationParentSessionId ? { orchestrationParentSessionId } : {}), | ||
| ...(provider !== "shell" && spawnKind ? { spawnKind } : {}), | ||
| cols: readIntOption(args, ["--cols"], 120), | ||
| rows: readIntOption(args, ["--rows"], 36), | ||
| cwd: readValue(args, ["--cwd"]), | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When
ade new chat --mode cli --type subagent|peerrecords a parent here, the metadata is persisted but never reaches the spawned CLI process environment:ptyServicebuilds env only fromchatSessionId/ownerSessionId, so the child getsADE_CHAT_SESSION_IDset to its own terminal id and noADE_PARENT_CHAT_SESSION_ID/ADE_SPAWN_KIND. In that spawned-CLI scenario, any follow-upade new chator 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 👍 / 👎.