From 9bc907c687d5aff7214b208edab1eddec72d413f Mon Sep 17 00:00:00 2001 From: Kevin Lago Date: Mon, 6 Jul 2026 12:58:41 -0400 Subject: [PATCH] fix(planner): resume the prior conversation on non-destructive restarts (#2396) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reopening a project (or a sandbox-toggle relaunch) went through handleRestart, which hardcoded a plain `claude` launch — so the planner always started a fresh chat instead of resuming. Make handleRestart resume-capable by default, mirroring the mount path's launch exactly (`claude --continue 2>/dev/null || claude` + startupPromptFreshOnly + continueSession); only the destructive ops (clear-plan / switch-blueprint) pass an explicit `fresh: true` to launch a genuinely new session with the intro. - usePlanningSession: handleRestart takes `{ fresh?: boolean }`; doClearPlan/doSwitchBlueprint pass fresh: true; default carries the resume trio. - plannerLaunch: the Claude branch now requests continueSession: true (defensive — the backend ANDs it with real history), which also covers the mount launch in usePlannerTerminal via launch.continueSession. - pty_create: one resume-decision log line (has_history + resumed) so an "always fresh" regression is visible in the logs. - tests: lock the resume trio on handleRestart's default launch, the plain-fresh destructive path (direct + via doClearPlan), and the Claude-branch continueSession in plannerLaunchConfig. Closes #2396 Co-Authored-By: Claude Fable 5 --- src-tauri/src/console/pty/mod.rs | 15 ++++- .../planner/session/plannerLaunch.test.ts | 2 + src/features/planner/session/plannerLaunch.ts | 9 ++- .../session/usePlanningSession.test.ts | 59 ++++++++++++++++++- .../planner/session/usePlanningSession.ts | 32 ++++++---- 5 files changed, 102 insertions(+), 15 deletions(-) diff --git a/src-tauri/src/console/pty/mod.rs b/src-tauri/src/console/pty/mod.rs index 61535ec3..826407dd 100644 --- a/src-tauri/src/console/pty/mod.rs +++ b/src-tauri/src/console/pty/mod.rs @@ -390,13 +390,24 @@ pub(crate) fn pty_create( // The session harness (#1078 P0): ClaudeCodeAdapter is the only impl today; it reproduces the // exact launch behavior this block had inline. bsc-agent becomes a second adapter (P2). let has_history = harness.detect_history(&cwd); - let launch = match plan_launch( + let plan = plan_launch( startup_prompt.as_deref(), init_cmd.as_deref(), has_history, continue_session.unwrap_or(false), startup_prompt_fresh_only.unwrap_or(false), - ) { + ); + // #2396: make the resume decision visible — an "always fresh" regression (a caller dropping the + // resume init/flag) shows up in the logs as `resumed=false` right next to `has_history=true`. + // An Init launch resumes when its command carries `--continue` AND there's history to continue + // (the `claude --continue || claude` chain falls back to fresh on its own otherwise). + let resumed = match &plan { + LaunchPlan::Prompt { resume } => *resume, + LaunchPlan::Init(s) => s.contains("--continue") && has_history, + LaunchPlan::None => false, + }; + log::info!("pty[{pane_id}] launch decision · has_history={has_history} · resumed={resumed}"); + let launch = match plan { LaunchPlan::Prompt { resume } => Some(harness.launch_command(startup_prompt.as_deref().unwrap_or(""), resume)), LaunchPlan::Init(s) => Some(s), LaunchPlan::None => None, diff --git a/src/features/planner/session/plannerLaunch.test.ts b/src/features/planner/session/plannerLaunch.test.ts index d1bfc462..03d23f5a 100644 --- a/src/features/planner/session/plannerLaunch.test.ts +++ b/src/features/planner/session/plannerLaunch.test.ts @@ -25,6 +25,8 @@ describe("plannerLaunchConfig", () => { expect(l.providerId).toBeUndefined(); expect(l.initCmd).toContain("claude"); expect(l.startupPromptFreshOnly).toBe(true); + // Defensive (#2396): resume is requested explicitly; the backend ANDs it with real history. + expect(l.continueSession).toBe(true); expect(l.env).toEqual(GH); // no BSC_AGENT_* env for a Claude planner }); diff --git a/src/features/planner/session/plannerLaunch.ts b/src/features/planner/session/plannerLaunch.ts index 2cc2a630..eb46e3b3 100644 --- a/src/features/planner/session/plannerLaunch.ts +++ b/src/features/planner/session/plannerLaunch.ts @@ -31,8 +31,9 @@ export interface PlannerLaunch { /** Whether the intro is suppressed on a resumed session (#1240). Claude keeps the fresh-only * greeting; bsc-agent (a one-shot agent loop, not a REPL) always bakes the intro as its task. */ startupPromptFreshOnly: boolean; - /** Request conversation resume — only meaningful for bsc-agent (its $BSC_AGENT_SESSION continuity); - * the backend ANDs it with "history actually exists". */ + /** Request conversation resume — the backend ANDs it with "history actually exists", so a fresh + * project is unaffected (Claude resumes via `--continue`; bsc-agent via its $BSC_AGENT_SESSION + * continuity). */ continueSession?: boolean; } @@ -50,6 +51,10 @@ export function plannerLaunchConfig(s: AppStore, ghEnv: Record): initCmd: "claude --continue 2>/dev/null || claude", env: ghEnv, startupPromptFreshOnly: true, + // Defensive (#2396): request resume explicitly. With the fresh-only intro guard a baked prompt + // only ever fires when there's no history (where resume is a no-op), but if a caller ever bakes + // a prompt onto a cwd WITH history this resumes it instead of silently forking a fresh session. + continueSession: true, }; } // The planner sees every installed MCP server (#1054); pass them to the runtime as $BSC_AGENT_MCP. diff --git a/src/features/planner/session/usePlanningSession.test.ts b/src/features/planner/session/usePlanningSession.test.ts index db7f0070..0102420f 100644 --- a/src/features/planner/session/usePlanningSession.test.ts +++ b/src/features/planner/session/usePlanningSession.test.ts @@ -4,7 +4,7 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import type { MutableRefObject } from "react"; -import { renderHook } from "@testing-library/react"; +import { renderHook, act, waitFor } from "@testing-library/react"; import { invoke } from "@tauri-apps/api/core"; import type { Terminal } from "@xterm/xterm"; import { useAppStore } from "@/store"; @@ -79,4 +79,61 @@ describe("usePlanningSession", () => { expect(deps.setSwitchOpen).toHaveBeenCalledWith(false); expect(invokeMock).not.toHaveBeenCalledWith("clear_project_plan_files", expect.anything()); }); + + // #2396: reopening a project used to start a FRESH planner chat — the restart path hardcoded a + // plain `claude` launch. These pin the resume-by-default launch config (the mount path's trio) + // and the explicit plain-fresh escape hatch the destructive ops use. + describe("handleRestart resume config (#2396)", () => { + // A live-ish terminal so handleRestart runs through to pty_create. + const term = () => + ({ current: { clear: vi.fn(), cols: 80, rows: 24 } as unknown as Terminal }) as MutableRefObject; + + const ptyCreateArgs = () => { + const call = invokeMock.mock.calls.find(([cmd]) => cmd === "pty_create"); + expect(call).toBeDefined(); + return call![1] as Record; + }; + + beforeEach(() => { + // The default Claude harness; per-command returns so the intro composes and the launch fires. + useAppStore.setState({ llmProvider: "anthropic", fleetHarness: "claude" }); + invokeMock.mockImplementation(async (cmd: string) => { + if (cmd === "planner_intro_prompt") return "hello, I am the planner"; + if (cmd === "setup_workspaces") return { planning_dir: "/hub/proj" }; + return null; + }); + }); + + it("resumes by default: --continue init chain + fresh-only intro + resume request", async () => { + const { result } = renderHook(() => usePlanningSession(makeDeps({ termRef: term() }))); + + await act(() => result.current.handleRestart()); + + const args = ptyCreateArgs(); + expect(args.initCmd).toBe("claude --continue 2>/dev/null || claude"); + expect(args.startupPromptFreshOnly).toBe(true); + expect(args.continueSession).toBe(true); + }); + + it("launches plain-fresh when passed fresh: true (the destructive ops)", async () => { + const { result } = renderHook(() => usePlanningSession(makeDeps({ termRef: term() }))); + + await act(() => result.current.handleRestart({ fresh: true })); + + const args = ptyCreateArgs(); + expect(args.initCmd).toBe("claude"); + expect(args.startupPromptFreshOnly).toBe(false); + expect(args.continueSession).toBe(false); + }); + + it("doClearPlan restarts through the plain-fresh path", async () => { + const { result } = renderHook(() => usePlanningSession(makeDeps({ termRef: term() }))); + + await act(() => result.current.doClearPlan()); + + // handleRestart is fired void from doClearPlan — wait for the relaunch to land. + await waitFor(() => expect(ptyCreateArgs().initCmd).toBe("claude")); + expect(ptyCreateArgs().continueSession).toBe(false); + }); + }); }); diff --git a/src/features/planner/session/usePlanningSession.ts b/src/features/planner/session/usePlanningSession.ts index 877849bc..61d3ee88 100644 --- a/src/features/planner/session/usePlanningSession.ts +++ b/src/features/planner/session/usePlanningSession.ts @@ -2,7 +2,8 @@ // from Planning.tsx. Owns the `restarting` flag and the four lifecycle operations: // • regenerateWorkspace — rewrite CLAUDE.md + the context baseline for the CURRENT blueprint // version WITHOUT touching plan section files (shared by restart + "keep files"). -// • handleRestart — kill the PTY, regenerate, and re-spawn `claude` with a fresh intro (#1240). +// • handleRestart — kill the PTY, regenerate, and re-spawn `claude`. Resume-capable by +// default (#2396); the destructive ops pass `fresh: true` for a brand-new session (#1240). // • keepPlanFiles — adopt the new blueprint/template version on disk, clearing the staleness // WITHOUT wiping plan files or restarting into a destructive reconciliation (#827). // • doClearPlan — delete on-disk plan files FIRST (awaited), wipe the store, unlink repos, @@ -47,7 +48,7 @@ export interface PlanningSessionDeps { export interface PlanningSession { restarting: boolean; - handleRestart: () => Promise; + handleRestart: (opts?: { fresh?: boolean }) => Promise; keepPlanFiles: () => Promise; doClearPlan: () => Promise; doSwitchBlueprint: (targetId: string) => Promise; @@ -95,7 +96,14 @@ export function usePlanningSession(deps: PlanningSessionDeps): PlanningSession { return paths; } - async function handleRestart() { + // Relaunch the planner session (#2396). RESUME-CAPABLE by default — the launch mirrors the mount + // path exactly (`claude --continue || claude` + the fresh-only intro guard + a resume request), so + // a non-destructive relaunch (sandbox-toggle flip, reopening a project) continues the prior + // conversation instead of starting over. The destructive ops (clear-plan / switch-blueprint) pass + // `fresh: true` to launch a genuinely NEW session — plain `claude`, re-greeted with the intro even + // though history exists (#1240), since the plan that conversation referred to was just wiped. + async function handleRestart(opts?: { fresh?: boolean }) { + const fresh = opts?.fresh === true; const term = termRef.current; if (!term || restarting) return; setRestarting(true); @@ -105,16 +113,15 @@ export function usePlanningSession(deps: PlanningSessionDeps): PlanningSession { const paths = await regenerateWorkspace(); const token = useAppStore.getState().githubToken; const ghEnv: Record = token ? { GH_TOKEN: token, GITHUB_TOKEN: token } : {}; - // A deliberate restart launches a brand-new session — re-greet with the intro (#1240). No - // fresh-only guard here: the user explicitly restarted, so fire it even though history exists. + // The intro rides along on every restart: a `fresh` restart always fires it (#1240 — the user + // wiped the plan, so re-greet even though history exists); the default resume restart keeps the + // fresh-only guard, so a returning user with history resumes quietly instead of being re-greeted. const introMode = plannerIntroMode({ isAuthoring, isExisting: treatAsExisting }); const introText = await safeInvoke("planner_intro_prompt", { mode: introMode }, "", (e: unknown) => console.error("planner intro prompt failed:", e)); // Relaunch on the selected harness (Claude Code or bsc-agent for any LLM, incl. a forced // local/ollama provider), carrying the plan-only role gate + provider/model/MCP via BSC_AGENT_* // env so a restarted local-model planner keeps the same context, prompt, state, and permissions. - // A restart is a deliberate brand-new session (#1240) — re-greet with the intro and DON'T request - // resume (no `continueSession`), so clear-plan / switch-blueprint start genuinely fresh. const launch = plannerLaunchConfig(useAppStore.getState(), ghEnv); // bsc-agent (one-shot) never gets stage 1's directive via runtime injection — bake it into the // intro so a weak local model begins the stage instead of waiting to be advanced (#qwen). @@ -138,8 +145,13 @@ export function usePlanningSession(deps: PlanningSessionDeps): PlanningSession { cols: term.cols, rows: term.rows, cwd: sandbox.cwd, - initCmd: launch.providerId === "bsc-agent" ? launch.initCmd : "claude", + // Default (resume): the harness's own launch — for Claude the `--continue || claude` chain — + // with the fresh-only intro guard + resume request, exactly like the mount path (#2396). + // Destructive (`fresh`): plain `claude` + an always-firing intro, so the session starts over. + initCmd: fresh && launch.providerId !== "bsc-agent" ? "claude" : launch.initCmd, startupPrompt, + startupPromptFreshOnly: fresh ? false : launch.startupPromptFreshOnly, + continueSession: fresh ? false : launch.continueSession, env: launch.env, providerId: launch.providerId, wslDistro: sandbox.wslDistro, @@ -166,7 +178,7 @@ export function usePlanningSession(deps: PlanningSessionDeps): PlanningSession { store.clearPlan(effectiveProjectId); store.setActiveProjectRepos([]); store.setPlanningContext(planningPitch, ""); - void handleRestart(); + void handleRestart({ fresh: true }); // destructive — the plan is gone, start a NEW session (#2396) } // Switch the project to another blueprint (#1281 — any → any other project blueprint, confirmed via @@ -181,7 +193,7 @@ export function usePlanningSession(deps: PlanningSessionDeps): PlanningSession { await safeInvoke("clear_project_plan_files", { projectKey: effectiveProjectId }, undefined, console.error); store.setActiveProjectRepos([]); store.setPlanningContext(planningPitch, ""); - void handleRestart(); + void handleRestart({ fresh: true }); // destructive — new blueprint, start a NEW session (#2396) } return { restarting, handleRestart, keepPlanFiles, doClearPlan, doSwitchBlueprint };