diff --git a/CHANGELOG.md b/CHANGELOG.md index 8538f8c6a..5f2ea65d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- **A session no longer offers to resume a conversation that is gone.** Reopening + a session lich had run Claude Code in asked whether to continue that + conversation — and answering yes could drop you into a terminal showing + Claude's "no conversation found" error instead of a session. lich stored the + conversation's id for good, but Claude Code prunes its own transcripts, so an + old enough session was offering something that could no longer happen. The + question is now only asked while the conversation is still there; when it is + not, the session starts fresh and says so. +- **A session whose worktree was deleted outside lich no longer opens an empty + terminal.** Removing a checkout by hand left its session behind, pointing at a + directory that is not there: opening it mounted a terminal that stayed blank, + with nothing on screen to say why. lich now recognises the missing checkout, + says so, and closes the session — closes, not deletes, so re-creating the + worktree brings the session and its conversation back. +- **A session that fails to start now says why.** Every other reason a terminal + could fail to open — a provider binary that is not installed, or a wrong path + in Settings › Providers — failed in silence behind a blank terminal. The + failure is now reported, and the session stays, ready to retry once the cause + is fixed. + ## [0.22.0] - 2026-07-28 ### Added diff --git a/frontend/src/components/TerminalHost.tsx b/frontend/src/components/TerminalHost.tsx index c34a6bf5e..4e54f0173 100644 --- a/frontend/src/components/TerminalHost.tsx +++ b/frontend/src/components/TerminalHost.tsx @@ -1,11 +1,21 @@ import { useEffect, useRef, useState } from "react" import { useMatch } from "react-router-dom" +import { toast } from "sonner" import { TerminalView } from "./TerminalView" import { ResumeSessionDialog } from "./ResumeSessionDialog" +import { Terminal as TerminalService } from "@/lib/rpc" import { useProjects } from "@/providers/projects" import { activeSessionId, resumableSession, sessionsOf } from "@/lib/session/sessions" +import { spawnDecision, type SpawnProbe } from "@/lib/session/spawn-gate" import type { Session } from "@/lib/session/sessions" +// The gate's two backend checks. Module-level so the effect below never takes a +// new object as a reason to run again. +const probe: SpawnProbe = { + workdirMissing: TerminalService.WorkdirMissing, + resumeAvailable: TerminalService.ResumeAvailable, +} + // TerminalHost keeps one persistent terminal per session, across every open // project, stacked in the same area. The router picks the active project and the // per-project active session decides which layer is visible — terminals are @@ -20,9 +30,10 @@ import type { Session } from "@/lib/session/sessions" // // A restored session that ran Claude Code before the last restart carries that // Claude session's id, so its spawn waits on the resume prompt: the terminal is -// mounted only once the user has said whether to continue that conversation. +// mounted only once the user has said whether to continue that conversation — +// and only when there is still a conversation to continue (ResumeAvailable). export function TerminalHost() { - const { projects, sessions } = useProjects() + const { projects, sessions, keepSession } = useProjects() const match = useMatch("/projects/:projectId") const activeProjectId = match?.params.projectId ?? null @@ -45,18 +56,45 @@ export function TerminalHost() { sessionsRef.current = sessions const spawnedRef = useRef(spawned) spawnedRef.current = spawned + const projectsRef = useRef(projects) + projectsRef.current = projects useEffect(() => { if (!activeProjectId || !visibleSessionId || spawnedRef.current.has(visibleSessionId)) { return } - const resumable = resumableSession(sessionsRef.current, activeProjectId, visibleSessionId) - if (resumable) { - setAsking(resumable) - return + const projectId = activeProjectId + const sessionId = visibleSessionId + const session = sessionsOf(sessionsRef.current, projectId).find((s) => s.id === sessionId) + const project = projectsRef.current.find((p) => p.id === projectId) + const resumable = resumableSession(sessionsRef.current, projectId, sessionId) + + let live = true + void spawnDecision(session?.path || project?.path || "", resumable, probe).then((decision) => { + if (!live) { + return + } + // Parking, not deleting: the checkout may come back (a re-created + // worktree, a mount that was not up yet), and the row still carries the + // provider conversation that a resume would need. + if (decision.verdict === "park") { + toast(decision.notice) + keepSession(projectId, sessionId) + return + } + if (decision.verdict === "ask" && resumable) { + setAsking(resumable) + return + } + if (decision.verdict === "fresh") { + toast(decision.notice) + } + setSpawned((prev) => new Set(prev).add(sessionId)) + }) + return () => { + live = false } - setSpawned((prev) => new Set(prev).add(visibleSessionId)) - }, [activeProjectId, visibleSessionId]) + }, [activeProjectId, visibleSessionId, keepSession]) // Answer the prompt and release the spawn: resume is the Claude session id to // continue, or "" to start fresh. diff --git a/frontend/src/components/TerminalView.tsx b/frontend/src/components/TerminalView.tsx index b2e0d65f0..0158dd5c9 100644 --- a/frontend/src/components/TerminalView.tsx +++ b/frontend/src/components/TerminalView.tsx @@ -9,6 +9,7 @@ import { toast } from "sonner" import { Button } from "@/components/ui/button" import { Input } from "@/components/ui/input" import { System, Terminal as Service } from "@/lib/rpc" +import { errorText } from "@/lib/utils" import { onAppEvent } from "@/lib/app-events" import { ensureTransport, onSessionData, sendInput } from "@/lib/terminal/term-transport" import { chordSequence, isSearchOpenChord } from "@/lib/terminal/term-keys" @@ -505,16 +506,26 @@ export function TerminalView({ resizeObserver.disconnect() }) - await Service.Start( - sessionId, - projectId, - cwd, - kind, - resume, - takeSetup(sessionId), - live.term.cols, - live.term.rows, - ) + try { + await Service.Start( + sessionId, + projectId, + cwd, + kind, + resume, + takeSetup(sessionId), + live.term.cols, + live.term.rows, + ) + } catch (error) { + // Every spawn failure arrives here as one opaque string — a binary that + // is not on $PATH, an exhausted fd table. The card stays, because all of + // them are worth another try once the cause is fixed; the one that is + // not (a checkout that is gone) never reaches the spawn, having been + // settled by the gate in TerminalHost. + toast.error(`Session failed to start: ${errorText(error)}`) + return + } if (disposed) { // Unmounted during the Start round-trip: the cleanup's Close raced // ahead of the spawn, so close again now that the PTY exists. The diff --git a/frontend/src/lib/rpc.ts b/frontend/src/lib/rpc.ts index f94c721f6..fc71d347b 100644 --- a/frontend/src/lib/rpc.ts +++ b/frontend/src/lib/rpc.ts @@ -85,6 +85,15 @@ async function post(path: string): Promise { } export const Terminal = { + /** Whether a session cannot start in cwd because the directory is gone — a + * worktree removed outside lich. False for every uncertainty, so only a + * provable absence closes a session. */ + WorkdirMissing: (cwd: string) => call("terminal.WorkdirMissing", [cwd]), + /** Whether the conversation providerSessionID names can still be reopened — + * false once the provider has pruned its transcript, which is the resume the + * prompt must not offer. */ + ResumeAvailable: (kind: string, providerSessionID: string) => + call("terminal.ResumeAvailable", [kind, providerSessionID]), /** resume: a provider session id to reopen (--resume); "" starts fresh. * setup: run the project's worktree setup script ahead of the provider — * passed once, by the first Start after the worktree is created. */ diff --git a/frontend/src/lib/session/spawn-gate.test.ts b/frontend/src/lib/session/spawn-gate.test.ts new file mode 100644 index 000000000..9ad5ecaf4 --- /dev/null +++ b/frontend/src/lib/session/spawn-gate.test.ts @@ -0,0 +1,74 @@ +import { describe, expect, it } from "vitest" +import { CHECKOUT_GONE, CONVERSATION_GONE, spawnDecision, type SpawnProbe } from "./spawn-gate" +import type { Session } from "./sessions" + +const resumable: Session = { + id: "s1", + label: "session", + kind: "claude", + providerSessionId: "conv-uuid", +} + +function probe(over: Partial = {}): SpawnProbe { + return { + workdirMissing: () => Promise.resolve(false), + resumeAvailable: () => Promise.resolve(true), + ...over, + } +} + +describe("spawnDecision", () => { + it("spawns when there is nothing to resume", async () => { + expect(await spawnDecision("/repo", null, probe())).toEqual({ verdict: "spawn" }) + }) + + it("asks when the conversation is still there", async () => { + expect(await spawnDecision("/repo", resumable, probe())).toEqual({ verdict: "ask" }) + }) + + it("starts fresh when the conversation is gone", async () => { + const decision = await spawnDecision( + "/repo", + resumable, + probe({ resumeAvailable: () => Promise.resolve(false) }), + ) + expect(decision).toEqual({ verdict: "fresh", notice: CONVERSATION_GONE }) + }) + + it("parks when the checkout is gone, without asking about the conversation", async () => { + let asked = false + const decision = await spawnDecision( + "/gone", + resumable, + probe({ + workdirMissing: () => Promise.resolve(true), + resumeAvailable: () => { + asked = true + return Promise.resolve(true) + }, + }), + ) + expect(decision).toEqual({ verdict: "park", notice: CHECKOUT_GONE }) + expect(asked).toBe(false) + }) + + // Both checks fail safe, in opposite directions: never close a session on an + // answer that never came, never drop a conversation that may still be live. + it("spawns when the workdir check fails", async () => { + const decision = await spawnDecision( + "/repo", + null, + probe({ workdirMissing: () => Promise.reject(new Error("rpc down")) }), + ) + expect(decision).toEqual({ verdict: "spawn" }) + }) + + it("still asks when the resume check fails", async () => { + const decision = await spawnDecision( + "/repo", + resumable, + probe({ resumeAvailable: () => Promise.reject(new Error("rpc down")) }), + ) + expect(decision).toEqual({ verdict: "ask" }) + }) +}) diff --git a/frontend/src/lib/session/spawn-gate.ts b/frontend/src/lib/session/spawn-gate.ts new file mode 100644 index 000000000..a30e79ce4 --- /dev/null +++ b/frontend/src/lib/session/spawn-gate.ts @@ -0,0 +1,56 @@ +import type { Session } from "./sessions" + +// What has to be settled before a session's terminal spawns. Both questions are +// about state that outlived the row naming it — the directory the PTY would +// start in, and the provider conversation the card would resume — so both are +// asked of the backend, and neither can be answered from the workspace alone. + +export type SpawnDecision = + /** Nothing in the way. */ + | { verdict: "spawn" } + /** A conversation is still there: raise the resume prompt. */ + | { verdict: "ask" } + /** The conversation is gone: spawn without it, and say so. */ + | { verdict: "fresh"; notice: string } + /** The checkout is gone: there is nothing to spawn into. */ + | { verdict: "park"; notice: string } + +/** The two backend checks, injected so the decision is testable without the RPC. */ +export interface SpawnProbe { + workdirMissing: (cwd: string) => Promise + resumeAvailable: (kind: string, providerSessionID: string) => Promise +} + +export const CHECKOUT_GONE = + "This session's worktree is gone, so the session was closed. Re-create the worktree to pick it up again." + +export const CONVERSATION_GONE = + "The previous conversation is no longer available — starting a new session." + +// spawnDecision resolves what a session's first view should do. Order matters: +// a session with no directory to run in has nothing to resume either, so the +// checkout is settled first. +// +// Both checks fail safe, in opposite directions, because their mistakes cost +// different things. A workdir check that errors spawns anyway — the PTY reports +// its own failure, and closing a session on a check that never answered would be +// the worse trade. A resume check that errors still asks — losing a live +// conversation to an answer nobody gave costs more than the error the check +// removes. +export async function spawnDecision( + cwd: string, + resumable: Session | null, + probe: SpawnProbe, +): Promise { + const missing = await probe.workdirMissing(cwd).catch(() => false) + if (missing) { + return { verdict: "park", notice: CHECKOUT_GONE } + } + if (!resumable) { + return { verdict: "spawn" } + } + const available = await probe + .resumeAvailable(resumable.kind, resumable.providerSessionId ?? "") + .catch(() => true) + return available ? { verdict: "ask" } : { verdict: "fresh", notice: CONVERSATION_GONE } +} diff --git a/internal/terminal/resume.go b/internal/terminal/resume.go new file mode 100644 index 000000000..01b0204a0 --- /dev/null +++ b/internal/terminal/resume.go @@ -0,0 +1,23 @@ +package terminal + +import "github.com/omartelo/lich/internal/providers" + +// ResumeAvailable reports whether the conversation providerSessionID names can +// still be reopened, so the frontend only offers a resume it can honour. +// +// The session row keeps its provider session id for good, but Claude Code prunes +// its own transcripts, so the id outlives the conversation it points at. Resuming +// one then dies inside the PTY with the provider's error in place of a session — +// the shortcut Start's doc names. The transcript on disk is what answers the +// question, and claudeContextUsage already reads it; this asks it for existence +// alone. +// +// False for every other provider: "--resume" is Claude Code's flag (resumeArgs), +// so there is nothing to offer anywhere else. +func (*Service) ResumeAvailable(kind, providerSessionID string) bool { + if kind != providers.Claude || providerSessionID == "" { + return false + } + _, ok := claudeTranscriptPath(providerSessionID) + return ok +} diff --git a/internal/terminal/resume_test.go b/internal/terminal/resume_test.go new file mode 100644 index 000000000..f3f83363a --- /dev/null +++ b/internal/terminal/resume_test.go @@ -0,0 +1,47 @@ +package terminal + +import ( + "os" + "path/filepath" + "testing" + + "github.com/omartelo/lich/internal/providers" +) + +// TestResumeAvailable proves the prompt's gate answers from the transcript on +// disk, not from the id alone: a pruned conversation is what used to reach the +// PTY as Claude's own error. +func TestResumeAvailable(t *testing.T) { + base := t.TempDir() + slug := filepath.Join(base, "projects", "-home-user-proj") + if err := os.MkdirAll(slug, 0o755); err != nil { + t.Fatal(err) + } + const live = "live-uuid" + if err := os.WriteFile(filepath.Join(slug, live+".jsonl"), []byte("{}\n"), 0o600); err != nil { + t.Fatal(err) + } + t.Setenv("CLAUDE_CONFIG_DIR", base) + + svc := &Service{} + tests := []struct { + name string + kind string + providerSessionID string + want bool + }{ + {"transcript on disk", providers.Claude, live, true}, + {"pruned transcript", providers.Claude, "gone-uuid", false}, + {"no id at all", providers.Claude, "", false}, + {"shell session", KindShell, live, false}, + {"another provider", providers.Codex, live, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := svc.ResumeAvailable(tt.kind, tt.providerSessionID); got != tt.want { + t.Errorf("ResumeAvailable(%q, %q) = %v, want %v", + tt.kind, tt.providerSessionID, got, tt.want) + } + }) + } +} diff --git a/internal/terminal/terminal.go b/internal/terminal/terminal.go index 73d0a064c..94a47e0d8 100644 --- a/internal/terminal/terminal.go +++ b/internal/terminal/terminal.go @@ -236,8 +236,11 @@ const readBufSize = 32 * 1024 // // A non-empty resume is a Claude session id to reopen (`--resume`), which the // frontend passes after the user accepted the prompt to continue the session -// this card ran before the last restart. An id Claude no longer knows fails in -// the PTY like any other bad invocation — the user sees Claude's own error. +// this card ran before the last restart. The prompt is only raised for a +// conversation ResumeAvailable still finds, so an id Claude no longer knows +// normally never reaches here; one that slips through (pruned between the check +// and the spawn) fails in the PTY like any other bad invocation — the user sees +// Claude's own error. // // setup is passed once, by the flow that just created this session's worktree: // it runs the project's worktree setup script (Settings › Project) in the PTY diff --git a/internal/terminal/workdir.go b/internal/terminal/workdir.go new file mode 100644 index 000000000..b55eefd69 --- /dev/null +++ b/internal/terminal/workdir.go @@ -0,0 +1,32 @@ +package terminal + +import ( + "errors" + "io/fs" + "os" +) + +// WorkdirMissing reports whether a session cannot spawn in cwd because the +// directory is not there — what a worktree removed outside lich leaves behind, +// where the checkout is gone but the session row pointing at it is not. The PTY +// would fail to chdir, and startPTY reports that the same way it reports a +// missing binary or an exhausted fd table, so the caller cannot tell an +// unrecoverable session from one a setting would fix. This separates the one +// case that is not worth retrying. +// +// Phrased as the missing case because false is the safe answer, and every +// uncertainty takes it: an empty cwd (Start resolves it to the home directory) +// and any stat failure that is not a plain "not there" — a permission error, a +// mount answering badly — fall through to the spawn, which reports its own +// failure. True is only a directory provably absent, or a path that exists and +// is not a directory. +func (*Service) WorkdirMissing(cwd string) bool { + if cwd == "" { + return false + } + info, err := os.Stat(cwd) + if err != nil { + return errors.Is(err, fs.ErrNotExist) + } + return !info.IsDir() +} diff --git a/internal/terminal/workdir_test.go b/internal/terminal/workdir_test.go new file mode 100644 index 000000000..0effc1af3 --- /dev/null +++ b/internal/terminal/workdir_test.go @@ -0,0 +1,37 @@ +package terminal + +import ( + "os" + "path/filepath" + "testing" +) + +// TestWorkdirMissing pins the asymmetry the doc promises: only a provable +// absence answers true, because a false answer costs a spawn that reports its +// own failure while a true one closes the user's session. +func TestWorkdirMissing(t *testing.T) { + dir := t.TempDir() + file := filepath.Join(dir, "not-a-dir") + if err := os.WriteFile(file, []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + + svc := &Service{} + tests := []struct { + name string + cwd string + want bool + }{ + {"existing directory", dir, false}, + {"empty cwd resolves to home", "", false}, + {"deleted checkout", filepath.Join(dir, "gone"), true}, + {"path is a file", file, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := svc.WorkdirMissing(tt.cwd); got != tt.want { + t.Errorf("WorkdirMissing(%q) = %v, want %v", tt.cwd, got, tt.want) + } + }) + } +}