diff --git a/docs/superpowers/plans/2026-08-30-session-picker-identity.md b/docs/superpowers/plans/2026-08-30-session-picker-identity.md new file mode 100644 index 00000000..fb8facf0 --- /dev/null +++ b/docs/superpowers/plans/2026-08-30-session-picker-identity.md @@ -0,0 +1,284 @@ +# Session Picker Identity + +> Fixes #96 + +## Outcome + +Give a past conversation **one identity**, derived once, rendered the same way +everywhere the user can pick one — so that "which past conversation is this?" has +a single answer regardless of whether the user arrived via resume, the command +palette, or prompt search. + +The end state: one typed record, one row component, one fallback ladder, and a +fallback that *looks* like a fallback instead of masquerading as a title. + +## Why #96's own diagnosis is incomplete + +The issue frames this as a **display-layer** problem — "at least three modals" +that each render the same thing differently — and proposes unifying the +renderer. That framing is right about the symptom and wrong about the cause, and +building only what the issue sketches would leave the bug half-fixed. + +The identity decision is not made in the renderer. **It is made in the provider +listers, and it is flattened into a lossy `summary: string` before the UI ever +sees it.** Three findings from reading the current code: + +### 1. Claude and Codex name the same conversation differently + +| | Claude — `src/providers/claude/runtime/sessionList.ts:353` | Codex — `packages/codex-headless/src/transcript/SessionList.ts:262` | +|---|---|---| +| `summary` = | `customTitle ?? lastPrompt ?? firstPrompt` | `userText ?? replayUserText` (the **first** user message) | + +Claude labels a session by its **last** prompt; Codex by its **first**. The same +work, moved between providers — which this app supports as a headline feature — +changes its own name. No renderer change can reconcile that, because by the time +the row is painted both are just strings. + +### 2. The hex ID the issue complains about is baked in *below* the UI + +`SessionList.ts:250` and `:264` in `codex-headless`: + +```ts +if (!meta && !userText && !replayUserText) { + return { sessionId: file.sessionId, summary: file.sessionId.slice(0, 8), … } +} +… +if (!summary) summary = file.sessionId.slice(0, 8) +``` + +The truncated hex ID is written **into the `summary` field**. A renderer +receiving that string cannot tell a real title from a fallback, so #96's own +requirement — *"The fallback should be visible (e.g. italicised) so the user +knows they're looking at a fallback identity"* — is **unimplementable** against +the current contract. This is the single most important reason the fix cannot be +renderer-only. + +### 3. The two providers disagree on whether a nameless session exists at all + +`sessionList.ts:354` (Claude): `if (!summary) return null` — the session is +**dropped from the list**. Codex, above, **shows it** with a hex label. So the +providers differ not just in labelling but in list *membership*. A Claude session +with no derivable summary is invisible in the resume picker. + +### The real root cause + +> The fallback policy is encoded as a **lossy string at the provider boundary**, +> independently by each provider, and the renderer is handed the result with no +> way to recover what it was looking at. + +Unify the renderers alone and all three defects above survive intact. + +## The current shape, as measured + +Two independent listing paths, and neither knows about the other. + +**Path A — `SessionInfo`** (`src/shared/types/session.ts:520`) feeds the two +*resume* surfaces: + +- Claude: `src/providers/claude/runtime/sessionList.ts` — **app-side**, and a + near-duplicate of the dormant `packages/claude-code-headless/src/transcript/SessionList.ts` +- Codex: `listCodexSessions` from the `codex-headless` submodule, dispatched at + `src/providers/registry.main.ts:71` +- Fields: `sessionId`, `summary`, `lastModified`, `fileSize`, `customTitle?`, + `firstPrompt?`, `gitBranch?`, `cwd?`, `createdAt?` — **no `kind`** + +**Path B — `SessionIndexEntry`** (`src/main/sessionIndex.ts:50`, and duplicated +verbatim at `src/preload/api/types.ts:252`) feeds *prompt search*: + +- `sessionIndex.ts` does its **own** transcript parsing for both providers + (`extractClaudePromptsAndCwd`, `extractCodexPromptsAndCwd`), and *also* calls + the Claude lister for discovery (`:147`) +- Fields: `providerSessionId`, `kind`, `cwd`, `lastModified`, `summary`, + `recentUserPrompts[]`, `matchCount` — **no `customTitle` / `gitBranch`** + +The same field is called `sessionId` in one and `providerSessionId` in the other. +`SessionInfo`'s own doc comment claims to be the source of truth for "renderer +resume UI" while Path B serves a different picker with a different shape. + +Counting transcript-reading implementations: the app-side Claude lister, the +Codex submodule lister, `sessionIndex`'s own dual-provider parser, and a dormant +fourth copy inside `claude-code-headless`. **Three live, one dead.** + +### Three surfaces, three renderings — verified in the JSX + +| Surface | Primary line | ID shown | +|---|---|---| +| `PathPickerModal.tsx:405` | `session.summary` | **always**, `sessionId.slice(0, 8)` | +| `CommandPalette.tsx:2072` | `summary \|\| firstPrompt \|\| sessionId` | on fallback, **full untruncated** | +| `PromptSearchModal.tsx:386` | `providerGlyph(kind)` + `cwdBasename(cwd)` + prompt lines | never | + +The first two consume the *same* `SessionInfo` and still disagree. +`sessionDisplay.ts` is 35 lines holding `cwdBasename` and `providerGlyph`, and +its header explicitly declines to become the canonical formatter; neither resume +surface imports it. **No test anywhere pins display consistency.** + +## Corrected scope: three surfaces, not four + +#96 lists four modals. Two of them have a different job and must be **dropped +from scope**: + +- **`ViewPromptsModal`** takes a `sessionId` prop and reads + `workspace.state.sessions[sessionId]` — it lists prompts *inside one already + open session*. +- **`RewindToPromptModal`** likewise picks a rewind target within the current + session. + +Neither displays a session identity, because the user already knows which session +they are in. They are **prompt pickers**, not **session pickers**. Forcing them +onto a shared session-row component would be a regression dressed as consistency. + +The surfaces in scope are the three that answer *"which past conversation is +this?"*: + +1. `PathPickerModal` resume list +2. `CommandPalette` session list — **not inventoried by #96**; it postdates the + issue and is the reason this keeps getting worse +3. `PromptSearchModal` + +## Design + +### 1. The record + +```ts +/** One past conversation, as a picker must display it. Derived once in main; + * never re-derived in the renderer. */ +export type SessionDisplayIdentity = { + providerSessionId: string + kind: AgentProviderKind + cwd: string | null + + /** The label to show, already resolved through the ladder below. */ + label: string + /** WHERE `label` came from. This is the field that makes the fallback + * visible, and the reason a flattened `summary: string` cannot work. */ + labelSource: 'custom-title' | 'first-prompt' | 'last-prompt' | 'cwd' | 'session-id' + + lastActivityAt: number + /** Absent when the provider lister cannot cheaply count. Never faked to 0. */ + turnCount: number | null + gitBranch: string | null +} +``` + +`labelSource` is the load-bearing field. Everything else is convenience; +`labelSource` is what lets a row italicise a fallback, lets a test assert the +ladder, and lets us delete the "is this string secretly a hex id?" guesswork. + +### 2. One fallback ladder, provider-independent + +``` +custom-title → first-prompt → last-prompt → cwd basename → truncated session id +``` + +Two deliberate departures from today: + +- **`first-prompt` outranks `last-prompt`.** #96 asks for "most recent prompt" + first, and Claude currently prefers `lastPrompt`. Both are wrong for + recognition: users remember a conversation by *how it started*. Codex already + does this. Adopting first-prompt everywhere also makes a provider switch + identity-preserving, which is a headline feature of this app. +- **A nameless session is never dropped.** Claude's `return null` disappears; + it falls to `cwd` and then `session-id`, both marked as fallbacks. + +This is a **visible behaviour change** to Claude session labels — sessions with a +`lastPrompt` will re-label to their first prompt. That is the point of the issue, +but it must be called out in the PR, not smuggled in. + +### 3. One row component + +`SessionPickerRow` under `src/renderer/src/features/workspace/ui/`, consuming +`SessionDisplayIdentity`. It owns: provider glyph, label (italicised when +`labelSource` is `cwd` or `session-id`), project basename, relative last activity, +turn count when present. The truncated id appears **only** as a fallback label, +never as a permanent second line. + +Prompt search keeps its extra prompt-list body — it composes `SessionPickerRow` +as its header rather than replacing it. Consistency of *identity*, not +flattening of every list into the same thing. + +### 4. Where it is derived + +In main, at the registry boundary, so both providers pass through one ladder: + +- Extend the `registry.main.ts` provider lister contract to return the fields the + ladder needs (`firstPrompt`, `customTitle`, `turnCount`) **without** a + pre-flattened `summary`. +- Apply the ladder once in a new `src/main/sessionDisplayIdentity.ts`. +- Serve it over one IPC. `SessionIndexEntry` keeps `recentUserPrompts` and gains + an embedded `identity`; the two shapes stop competing over who names a session. + +### 5. The Codex submodule problem, and the v1 bridge + +The `summary = sessionId.slice(0,8)` fallback lives in **`codex-headless`**, a +separate repository. Doing this correctly means a PR there, a submodule bump, and +a lockfile resync — a cross-repo dependency that would block all UI work behind +an upstream merge. + +**v1 bridges it in-app instead.** At the registry boundary, if a Codex +`summary === sessionId.slice(0, 8)`, treat it as absent and let the ladder +proceed. This is a heuristic, and it is a *sound* one: we control both sides of +the comparison, and a user whose first prompt is exactly the 8 leading hex +characters of their own rollout id is not a case worth engineering for. It must +carry a comment saying it is a bridge and what replaces it. + +**v2** (separate, non-blocking): teach `codex-headless` to return +`{ label, labelSource }` and delete the bridge. Same for the dormant +`claude-code-headless` lister if it is ever revived — or delete that file, since +the app has not used it since the app-side fork. + +## Stages — as built + +Six, not the five originally planned. The order changed so that **every commit +leaves the app working**: the plan had the Claude "stop dropping unnamed +sessions" change in stage 1, but `summary` is what the unmigrated pickers were +still rendering, so flipping it before they moved would have shown blank rows in +the intermediate commits. It moved to last. + +1. **The record and the ladder.** `@shared/types/sessionDisplayIdentity` — type, + ladder, `SessionInfo` adapter, Codex hex bridge. 12 unit tests. Nothing wired. +2. **Both listing paths serve it.** `session:list-for-cwd` / `session:list-all` + and the prompt index. Also collapsed `SessionIndexEntry` into + `@shared/types/sessionIndex` — planned for stage 5, pulled forward because the + type existed twice and adding the identity field to a hand-copied duplicate + was not defensible. +3. **`SessionPickerRow` + PromptSearchModal.** 5 renderer tests on fallback marking. +4. **CommandPalette + PathPickerModal.** Both migrated together — they consume + the same channel and splitting them would have left one commit where the two + resume surfaces disagreed, which is the bug. +5. *(folded into 4)* +6. **Behaviour + lock.** Remove the Claude `return null`; point + `registry.main.ts` at the app-side lister; add the boundary test. + +### Deviations from the design above + +- **`turnCount` dropped from the record.** Nothing can populate it: `SessionInfo` + has no count, and the index's `recentUserPrompts` is a capped window. Shipping a + field that is always `null` is dead weight. +- **Registry consolidation added.** Not in the original plan. `registry.main.ts` + imported its cwd-scoped Claude lister from `claude-code-headless` while + `sessionIndex.ts` used the app-side copy — both live, same parse logic. Left + alone, the stage-6 fix would have reached every surface *except* the resume + picker. Fixed in the same PR as the blast radius it sits in. + +## Verification + +- Ladder unit tests, one per rung, plus provider-parity: the same recorded + conversation yields the same `label` and `labelSource` under both providers. +- Renderer tests for fallback visibility. +- The Stage 5 boundary test as the regression lock. +- `npm run typecheck` (both projects), `npm run test:renderer`, `npm run check` + before the final merge. + +## Risks and non-goals + +- **Claude labels will visibly change** (last-prompt → first-prompt). Intended; + must be stated in the PR body. +- **Sessions previously invisible will appear** in Claude resume lists once + `return null` is removed. Also intended — a session you cannot see is worse + than one labelled by its folder. +- **Not in scope:** `ViewPromptsModal`, `RewindToPromptModal` (different job, see + above); `sessionIndex.ts`'s whole-file scan performance, which is #94's + territory and must not be conflated with identity; any change to how resume + itself *works* — this is purely how a session is *presented* before resuming. +- **`sessionDisplay.ts`** keeps `cwdBasename` / `providerGlyph` as primitives. + `SessionPickerRow` consumes them; they do not become the identity layer. diff --git a/src/main/ipc/session.ts b/src/main/ipc/session.ts index b1302a78..c5b12842 100644 --- a/src/main/ipc/session.ts +++ b/src/main/ipc/session.ts @@ -7,6 +7,10 @@ import { sha8FromDigestBytes } from '@shared/code/sha8.js' import type { ConditionCustomAction } from '@shared/types/providerConditions.js' import { getMainProvider } from '@providers/registry.main.js' import { AGENT_PROVIDER_KINDS, DEFAULT_PROVIDER } from '@shared/types/providerKind.js' +import { + buildSessionDisplayIdentity, + identityInputFromSessionInfo, +} from '@shared/types/sessionDisplayIdentity.js' import type { AgentProviderKind } from '@shared/types/providerKind.js' import { loadInitialHistoryChunk, @@ -264,7 +268,17 @@ export function registerSessionIpc( ) => { try { const providerConfig = getMainProvider(provider) - return await providerConfig.listSessions(cwd, limit ?? 20) + const sessions = await providerConfig.listSessions(cwd, limit ?? 20) + // Attach the shared display identity (#96). Derived here, in main, + // once — the resume picker and the command palette both consume this + // channel and used to each invent their own label from `summary`, + // which is how the same conversation ended up with two names. + return sessions.map(session => ({ + ...session, + identity: buildSessionDisplayIdentity( + identityInputFromSessionInfo(session, provider), + ), + })) } catch (err) { // Don't let a listing error brick the modal — return empty. // eslint-disable-next-line no-console @@ -291,7 +305,11 @@ export function registerSessionIpc( const providerConfig = getMainProvider(provider) if (!providerConfig.listAllSessions) return [] const sessions = await providerConfig.listAllSessions(cap).catch(() => []) - return sessions.map(s => ({ ...s, provider })) + return sessions.map(s => ({ + ...s, + provider, + identity: buildSessionDisplayIdentity(identityInputFromSessionInfo(s, provider)), + })) })) const tagged = listed.flat() tagged.sort((a, b) => b.lastModified - a.lastModified) diff --git a/src/main/sessionIndex.ts b/src/main/sessionIndex.ts index 08bf8a9b..27e8f34b 100644 --- a/src/main/sessionIndex.ts +++ b/src/main/sessionIndex.ts @@ -7,6 +7,12 @@ import { getProjectDirForCwd } from '@shared/runtime/projectDir.js' import { getCodexSessionsDir } from '@providers/codex/runtime/projectDir.js' import { performanceService } from '@main/performance/PerformanceService.js' import { asRecord, parseJsonRecord } from '@shared/lib/asRecord.js' +import { + buildSessionDisplayIdentity, + identityInputFromSessionInfo, +} from '@shared/types/sessionDisplayIdentity.js' +import type { SessionDisplayIdentity } from '@shared/types/sessionDisplayIdentity.js' +import type { SessionIndexEntry, SessionIndexPrompt } from '@shared/types/sessionIndex.js' // Session Prompt Index — power source for the "Search Conversation // Prompts" command. @@ -41,35 +47,10 @@ import { asRecord, parseJsonRecord } from '@shared/lib/asRecord.js' // here and re-implement the predicates inline. Same shape, same // filters. -export type SessionIndexPrompt = { - text: string - /** Epoch ms if the entry's ISO timestamp parsed, else null. */ - ts: number | null -} - -export type SessionIndexEntry = { - /** Provider-side uuid (Claude) or rollout uuid (Codex). Stable; - * used as the resume argument. */ - providerSessionId: string - kind: AgentProviderKind - /** Cwd the session was recorded in (from session_meta for Codex; - * from the first entry's cwd field for Claude). Falls back to - * empty string if not discoverable. */ - cwd: string - /** File mtime epoch ms. Primary sort key for the recent view. */ - lastModified: number - /** One-line summary from the existing session listers (customTitle - * for Claude if set, else the last prompt; the first prompt for - * Codex). Used as a fallback label when the user hasn't typed - * any prompts yet. */ - summary: string - /** Up to the last N user prompts (newest first). Empty array - * when a session exists on disk but has no visible user prompts - * (rare — fresh session with only assistant bootstrap text). */ - recentUserPrompts: SessionIndexPrompt[] - /** Count of matched prompts when returned from search, else 0. */ - matchCount: number -} +// The wire shapes now live at the neutral boundary (@shared/types/sessionIndex) +// so preload cannot hold a silently-drifting copy. Re-exported here because +// every existing importer reaches for them through this module. +export type { SessionIndexEntry, SessionIndexPrompt } from '@shared/types/sessionIndex.js' type ListRecentOptions = { /** How many sessions to include. Default 10. */ @@ -112,6 +93,49 @@ type CacheEntry = { * across providers in practice, but we prefix to be safe. */ const promptCache = new Map() +/** + * Build the shared display identity for an index entry (#96). + * + * WHY this reuses `identityInputFromSessionInfo` rather than deriving its own + * ladder input: the resume picker's path (`session:list-for-cwd`) feeds the + * ladder through that adapter, and two adapters would be exactly the + * divergence #96 was filed about — the Codex hex-id bridge in particular has to + * apply identically on both paths or a session renames itself depending on + * which picker you opened. + * + * WHY the newest prompt is offered as `lastPrompt` and never as `firstPrompt`: + * `recentUserPrompts` is the last N prompts, newest first. When a session has + * more than N, the oldest one in that window is NOT the conversation's opening + * prompt, and claiming otherwise would put a mid-conversation line on the + * `first-prompt` rung and mislabel the row. Feeding it to the weaker rung is + * accurate, and still beats falling through to a bare folder name — which is + * what a Codex entry would otherwise get here, since the Codex discoverer does + * not populate `summary` at all. + */ +function identityForIndexEntry(params: { + kind: AgentProviderKind + providerSessionId: string + cwd: string + lastModified: number + summary: string + prompts: SessionIndexPrompt[] +}): SessionDisplayIdentity { + const input = identityInputFromSessionInfo( + { + sessionId: params.providerSessionId, + summary: params.summary, + lastModified: params.lastModified, + fileSize: 0, + cwd: params.cwd || undefined, + }, + params.kind, + ) + return buildSessionDisplayIdentity({ + ...input, + lastPrompt: input.lastPrompt ?? params.prompts[0]?.text ?? null, + }) +} + function cacheKey(kind: AgentProviderKind, id: string): string { return `${kind}:${id}` } @@ -608,14 +632,23 @@ export async function listRecentSessionsWithPrompts( } // Apply cwd filter now if requested. if (cwd && resolvedCwd && resolvedCwd !== cwd) continue + const summary = c.summary || (prompts[0]?.text ?? '').slice(0, 200) results.push({ providerSessionId: c.providerSessionId, kind: c.kind, cwd: resolvedCwd, lastModified: c.lastModified, - summary: c.summary || (prompts[0]?.text ?? '').slice(0, 200), + summary, recentUserPrompts: prompts.slice(0, promptsPerSession), matchCount: 0, + identity: identityForIndexEntry({ + kind: c.kind, + providerSessionId: c.providerSessionId, + cwd: resolvedCwd, + lastModified: c.lastModified, + summary: c.summary, + prompts, + }), }) } span.end({ @@ -745,6 +778,14 @@ export async function searchSessionPrompts( summary: c.summary || (prompts[0]?.text ?? '').slice(0, 200), recentUserPrompts: combined, matchCount, + identity: identityForIndexEntry({ + kind: c.kind, + providerSessionId: c.providerSessionId, + cwd: resolvedCwd, + lastModified: c.lastModified, + summary: c.summary, + prompts, + }), }, score, }) diff --git a/src/preload/api/session.ts b/src/preload/api/session.ts index 6e88de70..00ca7cfa 100644 --- a/src/preload/api/session.ts +++ b/src/preload/api/session.ts @@ -3,6 +3,8 @@ import type { AgentProviderKind } from '@shared/types/providerKind.js' import { ipcRenderer } from 'electron' import type { PromptDeliveryResult } from '@shared/types/providerConfig.js' +import type { SessionDisplayIdentity } from '@shared/types/sessionDisplayIdentity.js' + import { subscribe } from '@preload/api/ipc.js' import type { SessionExitEvent, @@ -41,6 +43,12 @@ import type { // subscribes ONCE per event type and dispatches by sessionId in the // callback — this avoids N×N listener storms as tabs and splits grow. +// A listed past session, as the pickers receive it. `identity` is derived once +// in main (#96) so every picker renders the same label for the same +// conversation; the raw SessionInfo fields remain for resume mechanics and +// non-display callers. +export type ListedSession = SessionInfo & { identity: SessionDisplayIdentity } + export const sessionApi = { // --- Session lifecycle --- spawnSession: (options: SessionSpawnOptions): Promise => @@ -158,7 +166,7 @@ export const sessionApi = { cwd: string, limit?: number, provider: AgentProviderKind = DEFAULT_PROVIDER, - ): Promise => + ): Promise => ipcRenderer.invoke('session:list-for-cwd', cwd, limit, provider), /** Global session listing for the rendering-debug harness. Returns @@ -166,7 +174,7 @@ export const sessionApi = { * by lastModified desc. */ listAllSessions: ( limit?: number, - ): Promise> => + ): Promise> => ipcRenderer.invoke('session:list-all', limit), loadOlderHistory: (params: { diff --git a/src/preload/api/types.ts b/src/preload/api/types.ts index 6c726c86..0cac3936 100644 --- a/src/preload/api/types.ts +++ b/src/preload/api/types.ts @@ -244,20 +244,10 @@ export type SessionAgentPtyDataEvent = { sessionId: string; data: string } // recent user prompts for visual recognition. `matchCount` is only // meaningful on search results — zero on the default listing. -export type SessionIndexPrompt = { - text: string - ts: number | null -} - -export type SessionIndexEntry = { - providerSessionId: string - kind: AgentProviderKind - cwd: string - lastModified: number - summary: string - recentUserPrompts: SessionIndexPrompt[] - matchCount: number -} +// Re-exported from the neutral boundary. These were a hand-copied duplicate of +// main's declarations and had already lost fields to drift; #96's identity work +// made a third copy untenable. +export type { SessionIndexEntry, SessionIndexPrompt } from '@shared/types/sessionIndex.js' export type SessionHistoryChunk = { entries: JsonlEntry[] diff --git a/src/providers/claude/runtime/sessionList.ts b/src/providers/claude/runtime/sessionList.ts index 90c9d6db..347a0da2 100644 --- a/src/providers/claude/runtime/sessionList.ts +++ b/src/providers/claude/runtime/sessionList.ts @@ -350,8 +350,24 @@ async function parseSession({ const lastPrompt = extractLastJsonStringField(lite.tail, 'lastPrompt') const firstPrompt = extractFirstUserPrompt(lite.head) - const summary = customTitle ?? lastPrompt ?? firstPrompt - if (!summary) return null + // WHY a session with no derivable summary is no longer dropped (#96): + // + // This used to `return null`, which removed the session from the resume + // picker entirely. Codex's lister showed the equivalent session with a hex-id + // label, so the two providers disagreed about LIST MEMBERSHIP, not just about + // naming — a Claude session whose transcript carried no title, no lastPrompt + // and no recoverable first prompt was simply invisible, even though its id + // resumes perfectly well. + // + // Invisible is strictly worse than poorly named. The shared identity ladder + // (@shared/types/sessionDisplayIdentity) now labels these from the cwd + // basename, or the truncated id as a last resort, and marks either as a + // fallback so the user knows it is a stand-in rather than a name. + // + // `summary` stays a required string on the wire for search ranking and other + // non-display callers; '' is the honest value when nothing was derivable, and + // no surface renders it as a label any more. + const summary = customTitle ?? lastPrompt ?? firstPrompt ?? '' const gitBranch = extractJsonStringField(lite.head, 'gitBranch') ?? diff --git a/src/providers/registry.main.ts b/src/providers/registry.main.ts index c927b4dd..d373bfb9 100644 --- a/src/providers/registry.main.ts +++ b/src/providers/registry.main.ts @@ -8,9 +8,17 @@ import type { MainProviderConfig } from '@shared/types/providerConfig' import { AGENT_PROVIDER_KINDS, isAgentProviderKind } from '@shared/types/providerKind' import type { AgentProviderKind } from '@shared/types/providerKind' import { ClaudeSession } from '@providers/claude/runtime/claudeSession' -import { listAllClaudeSessions } from '@providers/claude/runtime/sessionList' +import { listAllClaudeSessions, listSessionsForCwd } from '@providers/claude/runtime/sessionList' import { deliverClaudePrompt } from '@providers/claude/runtime/promptDelivery' -import { listSessionsForCwd, getProjectDirForCwd } from 'claude-code-headless' +// WHY the cwd-scoped lister comes from the app and not from claude-code-headless +// (#96): both copies existed and BOTH were live — this slot used the submodule's +// while src/main/sessionIndex.ts used the app's. Their parse logic is the same, +// so the split bought nothing and cost a real bug: the fix for Claude silently +// dropping unnamed sessions from the resume picker had to be applied to the +// app-side copy, and leaving this pointed at the submodule would have made the +// resume picker the one surface that never received it. Keeping getProjectDirForCwd +// from the package is fine — it is genuinely provider-owned path knowledge. +import { getProjectDirForCwd } from 'claude-code-headless' import { CodexSession } from '@providers/codex/runtime/codexSession' import { deliverCodexPrompt } from '@providers/codex/runtime/promptDelivery' import { OpencodeSession } from '@providers/opencode/runtime/opencodeSession' diff --git a/src/renderer/src/features/command-palette/ui/CommandPalette.tsx b/src/renderer/src/features/command-palette/ui/CommandPalette.tsx index b537c7fe..b4f5bacf 100644 --- a/src/renderer/src/features/command-palette/ui/CommandPalette.tsx +++ b/src/renderer/src/features/command-palette/ui/CommandPalette.tsx @@ -94,10 +94,11 @@ import { SafeMarkdownLink } from '@renderer/features/rendered-content/SafeMarkdo import type { AiWorkspaceSummary } from '@mcp/shared/aiWorkspaceTypes' // Canonical session listing shape. This was a local copy that DROPPED // `fileSize` (and `customTitle`) — a concrete instance of the drift the -// shared contract prevents: the palette consumes `SessionInfo[]` straight +// shared contract prevents: the palette consumes `ListedSession[]` straight // from `window.api.listSessionsForCwd`, which always returns the full shape, // so the narrower local type was hiding fields rather than reflecting reality. -import type { SessionInfo } from '@shared/types/session' +import type { ListedSession } from '@preload/api/session' +import { SessionPickerRow } from '@renderer/features/workspace/ui/SessionPickerRow' // CommandPalette — VS Code-style ⌘⇧P command menu. // @@ -373,7 +374,7 @@ function OpenCommandPalette({ // component invisibly and destroys it in the same commit. const mode = useAppStore(state => state.paletteMode) const setMode = useAppStore(state => state.setPaletteMode) - const [sessions, setSessions] = useState([]) + const [sessions, setSessions] = useState([]) const [sessionsLoading, setSessionsLoading] = useState(false) const [aiWorkspaces, setAiWorkspaces] = useState([]) const [aiWorkspacesLoading, setAiWorkspacesLoading] = useState(false) @@ -1233,7 +1234,7 @@ function OpenCommandPalette({ }, [commandContext, onClose, onMenuCommandHandled, pendingMenuCommand, showToast]) const executeResume = useCallback( - (session: SessionInfo) => { + (session: ListedSession) => { onClose() if (!focusedCwd) return void workspace.replaceSession(focusedCwd, { @@ -1560,9 +1561,9 @@ function OpenCommandPalette({ // An earlier draft of this block referenced a `filtered` variable // that a concurrent command-palette refactor had already renamed — // the two changes merged cleanly as text but left this reference - // dangling. `filteredSessions` is typed `SessionInfo[]`, so the + // dangling. `filteredSessions` is typed `ListedSession[]`, so the // cast is belt-and-suspenders against noUncheckedIndexedAccess. - const session = filteredSessions[selectedIndex] as SessionInfo | undefined + const session = filteredSessions[selectedIndex] as ListedSession | undefined if (!session) return null const cwd = session.cwd ?? focusedCwd if (!cwd) return null @@ -2068,13 +2069,12 @@ function OpenCommandPalette({ onMouseEnter={() => setSelectedIndex(i)} onClick={() => executeResume(session)} > -
- {session.summary || session.firstPrompt || session.sessionId} -
-
- {session.gitBranch ? `${session.gitBranch} · ` : ''} - {session.cwd ?? focusedCwd ?? ''} -
+ {/* Identity is resolved in main through the shared ladder + (#96). The chain this replaced — + `summary || firstPrompt || sessionId` — could fall all + the way through to a FULL untruncated uuid, and had no + way to tell the user that is what they were looking at. */} + )) ))} diff --git a/src/renderer/src/features/path-picker/ui/PathPickerModal.tsx b/src/renderer/src/features/path-picker/ui/PathPickerModal.tsx index 32920797..d94ff815 100644 --- a/src/renderer/src/features/path-picker/ui/PathPickerModal.tsx +++ b/src/renderer/src/features/path-picker/ui/PathPickerModal.tsx @@ -11,12 +11,12 @@ import { DialogTitle, } from '@renderer/components/ui/dialog' import { PathInput } from '@renderer/features/path-picker/ui/PathInput' -import { relativeTime } from '@renderer/lib/relativeTime' // Canonical session listing shape — was a local duplicate of the preload // SessionInfo. The renderer tsconfig already includes `src/shared/types/**`, // so importing the shared type needs no preload reach-across. See // @shared/types/session. -import type { SessionInfo } from '@shared/types/session' +import type { ListedSession } from '@preload/api/session' +import { SessionPickerRow } from '@renderer/features/workspace/ui/SessionPickerRow' // PathPickerModal — modal that asks the user for a working directory // when they press ⌘T (or click the + button in the tab bar). @@ -68,7 +68,7 @@ export function PathPickerModal({ // changes and resolves to a valid directory — gives the user live // feedback as they type (e.g. "ah, no recorded sessions in this // folder yet, I'll start fresh"). - const [sessions, setSessions] = useState([]) + const [sessions, setSessions] = useState([]) const [sessionsLoading, setSessionsLoading] = useState(false) // Latest resolved absolute path. Tracked separately from `value` so // actions use the validated form rather than re-running expand. @@ -337,7 +337,7 @@ function ResumeSection({ disabled, }: { resolvedPath: string | null - sessions: SessionInfo[] + sessions: ListedSession[] loading: boolean onResume: (sessionId: string) => void | Promise disabled: boolean @@ -380,11 +380,10 @@ function ResumeRow({ disabled, onClick, }: { - session: SessionInfo + session: ListedSession disabled: boolean onClick: () => void }) { - const age = relativeTime(session.lastModified) return ( ) } diff --git a/src/renderer/src/features/workspace/ui/PromptSearchModal.tsx b/src/renderer/src/features/workspace/ui/PromptSearchModal.tsx index 9384f103..09be313b 100644 --- a/src/renderer/src/features/workspace/ui/PromptSearchModal.tsx +++ b/src/renderer/src/features/workspace/ui/PromptSearchModal.tsx @@ -9,8 +9,7 @@ import { } from '@renderer/components/ui/dialog' import { commandTargetSessionId } from '@renderer/workspace/hook/selectors/commandTargetSessionId' import type { Workspace } from '@renderer/workspace/workspaceStore' -import { relativeTime } from '@renderer/lib/relativeTime' -import { cwdBasename, providerGlyph } from '@renderer/features/workspace/lib/sessionDisplay' +import { SessionPickerRow } from '@renderer/features/workspace/ui/SessionPickerRow' import { useResizableSplitter } from '@renderer/features/shared/useResizableSplitter' import { SessionPreviewPane } from '@renderer/features/session-preview/ui/SessionPreviewPane' import type { PreviewTarget } from '@renderer/features/session-preview/ui/SessionPreviewPane' @@ -383,9 +382,9 @@ function SessionCard({ onSelect: () => void dataIdx: number }) { - const glyph = providerGlyph(entry.kind) - const base = cwdBasename(entry.cwd) || entry.cwd || '(no cwd)' - + // Identity comes from main, already resolved through the shared ladder (#96). + // This surface used to build its own from `kind` + cwd basename, which is why + // the same session looked different here and in the resume picker. return (
-
- - {glyph} - - {entry.kind} - · - {base} - · - {relativeTime(entry.lastModified)} - {entry.matchCount > 0 ? ( - <> - · - - {entry.matchCount} match{entry.matchCount === 1 ? '' : 'es'} - - - ) : null} - {resuming ? ( - resuming… - ) : null} -
+ + {entry.matchCount > 0 ? ( + + {entry.matchCount} match{entry.matchCount === 1 ? '' : 'es'} + + ) : null} + {resuming ? resuming… : null} + + } + /> {entry.recentUserPrompts.length === 0 ? (
diff --git a/src/renderer/src/features/workspace/ui/SessionPickerRow.renderer.test.tsx b/src/renderer/src/features/workspace/ui/SessionPickerRow.renderer.test.tsx new file mode 100644 index 00000000..3c76b51f --- /dev/null +++ b/src/renderer/src/features/workspace/ui/SessionPickerRow.renderer.test.tsx @@ -0,0 +1,74 @@ +import { render, screen } from '@testing-library/react' +import { describe, expect, it } from 'vitest' + +import { SessionPickerRow } from '@renderer/features/workspace/ui/SessionPickerRow' +import type { SessionDisplayIdentity } from '@shared/types/sessionDisplayIdentity' + +// Regression coverage for the display half of #96. +// +// WHY these assert the FALLBACK MARKING rather than the label text: rendering +// `identity.label` is trivial and cannot regress meaningfully. What can regress +// — and what the old surfaces got wrong — is presenting a stand-in as if it +// were a name. A row that shows the folder `agent-code` identically to a +// user-chosen title "agent-code" is the defect, and only `labelSource` can tell +// them apart. If a future refactor drops that distinction the rows still look +// plausible, so a human reviewer would not catch it; these tests would. + +function identity(over: Partial = {}): SessionDisplayIdentity { + return { + providerSessionId: '8d6926a5-1111-2222', + kind: 'claude', + cwd: '/Users/dev/projects/agent-code', + label: 'design the ownership ledger', + labelSource: 'first-prompt', + lastActivityAt: Date.now(), + gitBranch: null, + ...over, + } +} + +describe('SessionPickerRow (#96)', () => { + it('renders a real label plainly, without fallback styling', () => { + render() + const label = screen.getByText('design the ownership ledger') + expect(label.className).not.toContain('italic') + }) + + it('marks a cwd fallback so it does not read as a chosen name', () => { + render( + , + ) + const label = screen.getByText('agent-code') + expect(label.className).toContain('italic') + // Said out loud too — italics alone are invisible to a screen reader, and + // "is this actually the conversation's name?" is the question the row + // exists to answer. + expect(label.title).toMatch(/no title recorded/i) + }) + + it('marks a session-id fallback the same way', () => { + render( + , + ) + const label = screen.getByText('8d6926a5') + expect(label.className).toContain('italic') + expect(label.title).toMatch(/session id/i) + }) + + it('does not repeat the project folder when the folder IS the label', () => { + // Guards a specific ugly output: label "agent-code" (cwd fallback) with + // "agent-code" again on the metadata line, which reads as a duplicate-render + // bug rather than as context. + render( + , + ) + expect(screen.getAllByText('agent-code')).toHaveLength(1) + }) + + it('shows the project alongside a real label', () => { + render() + expect(screen.getByText('agent-code')).toBeInTheDocument() + }) +}) diff --git a/src/renderer/src/features/workspace/ui/SessionPickerRow.tsx b/src/renderer/src/features/workspace/ui/SessionPickerRow.tsx new file mode 100644 index 00000000..20681720 --- /dev/null +++ b/src/renderer/src/features/workspace/ui/SessionPickerRow.tsx @@ -0,0 +1,103 @@ +import { cwdBasename, providerGlyph } from '@renderer/features/workspace/lib/sessionDisplay' +import { relativeTime } from '@renderer/lib/relativeTime' +import { isFallbackLabel } from '@shared/types/sessionDisplayIdentity' +import type { SessionDisplayIdentity } from '@shared/types/sessionDisplayIdentity' + +// SessionPickerRow — the one way a past conversation is identified in the UI. +// +// WHY this exists (#96) +// --------------------- +// Three surfaces let a user pick a past session — the PathPicker resume list, +// the command palette, and prompt search — and each rendered its own identity +// for the same conversation. The resume list led with a flattened `summary` and +// permanently showed `sessionId.slice(0, 8)` underneath; the palette used a +// `summary || firstPrompt || sessionId` chain that could fall through to a full +// untruncated uuid; prompt search led with a provider glyph and cwd basename. +// A user who remembered a conversation by what they typed was shown a hex id. +// +// This component is the display half of the fix. It is deliberately dumb: it +// renders a `SessionDisplayIdentity` and makes NO decision about what a session +// is called. That decision belongs to the ladder in main +// (@shared/types/sessionDisplayIdentity), and duplicating any part of it here +// would recreate the divergence — a second opinion about identity is exactly +// what #96 is. +// +// WHY the fallback is visually marked rather than silently rendered: the +// difference between "the user titled this conversation `agent-code`" and "we +// gave up and used the folder name" is the difference between a name and a +// shrug. Showing both as plain text is what made the old surfaces feel +// arbitrary. `labelSource` carries that distinction across the process boundary +// precisely so this row can honour it. +// +// Composition, not replacement: prompt search keeps its prompt-list body and +// mounts this as its header via `trailing`/`children`. The goal is one identity +// everywhere, not one layout everywhere. + +export function SessionPickerRow({ + identity, + trailing, + compact = false, +}: { + identity: SessionDisplayIdentity + /** Right-aligned slot for surface-specific affordances (match counts, + * "resuming…", a branch chip). Kept as a slot so a surface never has to + * fork the component to add one badge. */ + trailing?: React.ReactNode + /** Single-line variant for dense lists (the command palette). The identity + * and its fallback marking are identical; only the metadata line is + * dropped. */ + compact?: boolean +}) { + const fallback = isFallbackLabel(identity.labelSource) + const project = cwdBasename(identity.cwd ?? '') + + return ( +
+
+ + + {identity.label} + + {trailing ? {trailing} : null} +
+ + {compact ? null : ( +
+ {/* The project is dropped when it IS the label — repeating a folder + name on both lines reads as a rendering bug rather than context. */} + {project && identity.labelSource !== 'cwd' ? ( + <> + {project} + · + + ) : null} + {identity.gitBranch ? ( + <> + {identity.gitBranch} + · + + ) : null} + {relativeTime(identity.lastActivityAt)} +
+ )} +
+ ) +} diff --git a/src/renderer/src/features/workspace/ui/sessionPickerIdentity.test.ts b/src/renderer/src/features/workspace/ui/sessionPickerIdentity.test.ts new file mode 100644 index 00000000..4f75c759 --- /dev/null +++ b/src/renderer/src/features/workspace/ui/sessionPickerIdentity.test.ts @@ -0,0 +1,94 @@ +import { readFileSync } from 'node:fs' +import { dirname, resolve } from 'node:path' +import { fileURLToPath } from 'node:url' +import { describe, expect, it } from 'vitest' + +// --------------------------------------------------------------------------- +// The lock that keeps #96 closed. +// +// #96 was not one bug. It was the same bug re-appearing every time a new +// surface learned to list past sessions: each one reached into the raw listing +// record and invented its own label. The issue named three modals; by the time +// it was picked up there were four, because the command palette had been added +// in the meantime and nobody noticed it was re-solving a solved problem. +// +// So fixing the three surfaces is not enough — without a gate, surface five +// re-opens the issue. This test asserts the RULE rather than the instances: +// +// a session picker renders identity through SessionPickerRow, and never by +// reading `.summary`, `.firstPrompt`, or a sliced `.sessionId` itself. +// +// WHY a filesystem scan rather than an ESLint rule or a type-level trick: the +// repo's convention is one narrow boundary test with a clear failure message, +// not a bespoke framework (see src/providers/importBoundaries.test.ts, which +// exists for the same reason). A test is visible in the suite, breaks loudly in +// CI, and needs no new tooling. +// +// WHY it cannot be a type-level ban: `summary` must stay on the wire for search +// ranking and other non-display callers, so the field is legitimately reachable. +// What is forbidden is reaching for it *to build a label*, and that is a usage +// rule, not a shape rule. +// --------------------------------------------------------------------------- + +const testDir = dirname(fileURLToPath(import.meta.url)) +const rendererSrc = resolve(testDir, '../../..') + +/** Every surface that lets a user pick a PAST session. + * + * Deliberately NOT included: ViewPromptsModal and RewindToPromptModal. #96 + * listed both, but they pick a prompt INSIDE an already-open session — they + * take a sessionId prop and never display a session identity, because the user + * already knows which session they are in. Forcing them through a session-row + * component would be a regression dressed up as consistency. + * + * When a new past-session picker is added, add it here. If that feels like a + * chore, that is the test working: the alternative is the fifth surface + * quietly re-opening #96. */ +const SESSION_PICKERS = [ + 'features/path-picker/ui/PathPickerModal.tsx', + 'features/command-palette/ui/CommandPalette.tsx', + 'features/workspace/ui/PromptSearchModal.tsx', +] as const + +/** Reading a raw listing field to build a display label. Each of these is a + * literal line that existed in one of the three surfaces before #96 was + * fixed. */ +const FORBIDDEN = [ + { pattern: /\.summary\s*\|\|/, why: 'builds a label from a `summary` fallback chain' }, + { pattern: /\{\s*\w+\.summary\s*\}/, why: 'renders `summary` directly as a label' }, + { pattern: /\.sessionId\.slice\(/, why: 'renders a truncated session id as a label' }, + { pattern: /\{\s*\w+\.firstPrompt\s*\}/, why: 'renders `firstPrompt` directly as a label' }, +] as const + +function sourceOf(relativePath: string): string { + return readFileSync(resolve(rendererSrc, relativePath), 'utf8') +} + +/** Strip comments so the prose in this repo — which quotes the old code + * extensively when explaining why it was wrong — cannot fail the test it is + * describing. */ +function stripComments(source: string): string { + return source + .replace(/\/\*[\s\S]*?\*\//g, '') + .replace(/^\s*\/\/.*$/gm, '') + .replace(/\{\s*\/\*[\s\S]*?\*\/\s*\}/g, '') +} + +describe('session pickers share one display identity (#96)', () => { + it.each(SESSION_PICKERS)('%s renders identity through SessionPickerRow', file => { + expect(sourceOf(file)).toContain('SessionPickerRow') + }) + + it.each(SESSION_PICKERS)('%s does not build its own session label', file => { + const code = stripComments(sourceOf(file)) + const violations = FORBIDDEN.filter(rule => rule.pattern.test(code)).map(rule => rule.why) + expect( + violations, + `${file} ${violations.join('; ')}. A past-session label comes from ` + + '`identity.label` via SessionPickerRow — the ladder in ' + + '@shared/types/sessionDisplayIdentity decides it once, in main. ' + + 'Deriving one here is how the same conversation ends up with a ' + + 'different name in each picker (#96).', + ).toEqual([]) + }) +}) diff --git a/src/shared/types/sessionDisplayIdentity.test.ts b/src/shared/types/sessionDisplayIdentity.test.ts new file mode 100644 index 00000000..ce18b74a --- /dev/null +++ b/src/shared/types/sessionDisplayIdentity.test.ts @@ -0,0 +1,180 @@ +import { describe, expect, it } from 'vitest' + +import { + buildSessionDisplayIdentity, + identityInputFromSessionInfo, + isFallbackLabel, +} from '@shared/types/sessionDisplayIdentity' +import type { SessionInfo } from '@shared/types/session' + +// Tests for the #96 identity ladder. +// +// WHY these assert `labelSource` and not just `label`: the label alone cannot +// distinguish "the user titled this conversation `agent-code`" from "we gave up +// and used the folder name". That distinction is the entire reason this module +// exists, so every case pins the provenance, not only the text. + +function claudeInfo(over: Partial = {}): SessionInfo { + return { + sessionId: '8d6926a5-1111-2222-3333-444455556666', + summary: 'summary text', + lastModified: 1_700_000_000_000, + fileSize: 4096, + cwd: '/Users/dev/projects/agent-code', + ...over, + } +} + +describe('buildSessionDisplayIdentity — the ladder', () => { + it('prefers a custom title over everything else', () => { + const id = buildSessionDisplayIdentity({ + providerSessionId: 'abc12345-x', + kind: 'claude', + customTitle: 'Renderer rewrite', + firstPrompt: 'help me refactor the feed', + lastPrompt: 'now run the tests', + cwd: '/Users/dev/agent-code', + lastActivityAt: 1, + }) + expect(id.label).toBe('Renderer rewrite') + expect(id.labelSource).toBe('custom-title') + }) + + it('prefers the first prompt over the last prompt', () => { + // The behavioural change at the heart of #96. Claude previously labelled by + // lastPrompt, which means a row renames itself as the conversation runs and + // is unrecognisable an hour later. Codex already keyed on the first message; + // this is what makes the two providers agree. + const id = buildSessionDisplayIdentity({ + providerSessionId: 'abc12345-x', + kind: 'claude', + firstPrompt: 'help me refactor the feed', + lastPrompt: 'now run the tests', + lastActivityAt: 1, + }) + expect(id.label).toBe('help me refactor the feed') + expect(id.labelSource).toBe('first-prompt') + }) + + it('falls to the last prompt only when there is no first prompt', () => { + const id = buildSessionDisplayIdentity({ + providerSessionId: 'abc12345-x', + kind: 'claude', + lastPrompt: 'now run the tests', + lastActivityAt: 1, + }) + expect(id.labelSource).toBe('last-prompt') + }) + + it('falls to the cwd basename when the session said nothing usable', () => { + const id = buildSessionDisplayIdentity({ + providerSessionId: 'abc12345-x', + kind: 'codex', + cwd: '/Users/dev/projects/agent-code/', + lastActivityAt: 1, + }) + expect(id.label).toBe('agent-code') + expect(id.labelSource).toBe('cwd') + expect(isFallbackLabel(id.labelSource)).toBe(true) + }) + + it('falls to a truncated id last, and never returns an empty label', () => { + // The floor of the ladder. This case previously made Claude drop the session + // from the list entirely (`if (!summary) return null`), so it was invisible + // rather than merely poorly named. + const id = buildSessionDisplayIdentity({ + providerSessionId: '8d6926a5-1111-2222', + kind: 'claude', + lastActivityAt: 1, + }) + expect(id.label).toBe('8d6926a5') + expect(id.labelSource).toBe('session-id') + expect(isFallbackLabel(id.labelSource)).toBe(true) + }) + + it('treats whitespace-only and empty strings as absent rungs', () => { + // Guards the ladder against a provider emitting '' or ' ' rather than + // null, which would otherwise produce a blank row that looks like a + // rendering bug rather than a missing name. + const id = buildSessionDisplayIdentity({ + providerSessionId: 'abc12345-x', + kind: 'claude', + customTitle: ' ', + firstPrompt: '', + cwd: '/Users/dev/projects/agent-code', + lastActivityAt: 1, + }) + expect(id.labelSource).toBe('cwd') + }) + + it('collapses a multi-line prompt and truncates a wall of text', () => { + const id = buildSessionDisplayIdentity({ + providerSessionId: 'abc12345-x', + kind: 'claude', + firstPrompt: 'line one\n\nline two ' + 'x'.repeat(400), + lastActivityAt: 1, + }) + expect(id.label).not.toContain('\n') + expect(id.label.endsWith('…')).toBe(true) + expect(id.label.length).toBeLessThanOrEqual(121) + }) +}) + +describe('identityInputFromSessionInfo — recovering ingredients from listers', () => { + it('recovers Claude lastPrompt from a summary that is neither title nor first prompt', () => { + // Claude's lister flattens `customTitle ?? lastPrompt ?? firstPrompt` into + // `summary`. When summary matches neither of the two fields it also returns, + // the remaining branch of that expression is the lastPrompt. + const input = identityInputFromSessionInfo( + claudeInfo({ summary: 'now run the tests', firstPrompt: undefined, customTitle: undefined }), + 'claude', + ) + expect(input.lastPrompt).toBe('now run the tests') + expect(buildSessionDisplayIdentity(input).labelSource).toBe('last-prompt') + }) + + it('does not mistake a Claude summary that equals the first prompt for a lastPrompt', () => { + const input = identityInputFromSessionInfo( + claudeInfo({ summary: 'help me refactor', firstPrompt: 'help me refactor' }), + 'claude', + ) + expect(input.lastPrompt).toBeNull() + expect(buildSessionDisplayIdentity(input).labelSource).toBe('first-prompt') + }) + + it('reads a Codex summary as the first user message', () => { + const input = identityInputFromSessionInfo( + claudeInfo({ summary: 'port the proxy to zstd' }), + 'codex', + ) + expect(buildSessionDisplayIdentity(input).labelSource).toBe('first-prompt') + }) + + it('rejects the Codex hex-id summary instead of rendering it as a title', () => { + // codex-headless writes `sessionId.slice(0, 8)` into `summary` when it finds + // no user text. Without this bridge the ladder would treat a hex id as a + // real first prompt and the row could never be marked as a fallback — the + // exact defect #96 reported. + const info = claudeInfo({ sessionId: '8d6926a5-aaaa-bbbb', summary: '8d6926a5' }) + const id = buildSessionDisplayIdentity(identityInputFromSessionInfo(info, 'codex')) + expect(id.labelSource).toBe('cwd') + expect(id.label).toBe('agent-code') + }) + + it('gives the same identity for the same conversation under either provider', () => { + // The provider-parity contract, and the reason a shared ladder was worth + // building: moving a session between Claude and Codex must not rename it. + const shared = { sessionId: 'abc12345-x', lastModified: 5, fileSize: 1, cwd: '/w/proj' } + const claude = buildSessionDisplayIdentity( + identityInputFromSessionInfo( + { ...shared, summary: 'design the ledger', firstPrompt: 'design the ledger' }, + 'claude', + ), + ) + const codex = buildSessionDisplayIdentity( + identityInputFromSessionInfo({ ...shared, summary: 'design the ledger' }, 'codex'), + ) + expect(codex.label).toBe(claude.label) + expect(codex.labelSource).toBe(claude.labelSource) + }) +}) diff --git a/src/shared/types/sessionDisplayIdentity.ts b/src/shared/types/sessionDisplayIdentity.ts new file mode 100644 index 00000000..db43dcb8 --- /dev/null +++ b/src/shared/types/sessionDisplayIdentity.ts @@ -0,0 +1,222 @@ +import type { AgentProviderKind } from '@shared/types/providerKind.js' +import type { SessionInfo } from '@shared/types/session.js' + +// Session display identity — the single answer to "which past conversation is +// this?" for every surface that lets the user pick one. +// +// WHY THIS EXISTS (#96) +// --------------------- +// Three surfaces let a user pick a past session — the PathPicker resume list, +// the command palette, and prompt search — and each rendered a different +// identity for the same conversation. The obvious reading is that this is a +// renderer problem. It is not, and fixing it in the renderer is impossible. +// +// The identity decision was made in the provider listers and flattened into a +// lossy `summary: string` before any UI saw it: +// +// * Claude preferred `customTitle ?? lastPrompt ?? firstPrompt` — the LAST +// prompt. Codex used the FIRST user message. The same conversation +// therefore renamed itself when moved between providers, which this app +// supports as a headline feature. +// * Codex wrote a truncated hex id INTO `summary` when it found no user text +// (codex-headless SessionList.ts). A renderer holding that string cannot +// distinguish a real title from a fallback, so "show the user when they are +// looking at a fallback" was unimplementable against the old contract. +// * Claude DROPPED a session with no derivable summary from the list, while +// Codex showed it. The providers disagreed about list membership, not just +// labels. +// +// So the fix is not a shared component with a shared string. It is a shared +// record that keeps the PROVENANCE of the label. `labelSource` is the +// load-bearing field here: it is what lets a row mark a fallback as a fallback, +// what lets a test assert the ladder rung-by-rung, and what removes the need to +// guess whether a given string is secretly a session id. +// +// Everything below is pure and dependency-free so main derives it once, the +// renderer only consumes it, and the ladder is unit-testable without a +// filesystem. + +/** Which rung of the ladder produced `label`. Ordered best → worst. */ +export type SessionLabelSource = + | 'custom-title' + | 'first-prompt' + | 'last-prompt' + | 'cwd' + | 'session-id' + +/** The two rungs that are not really a name. A picker should mark these + * visually so the user knows they are looking at a stand-in rather than + * something the conversation actually said. */ +const FALLBACK_SOURCES: ReadonlySet = new Set([ + 'cwd', + 'session-id', +]) + +export function isFallbackLabel(source: SessionLabelSource): boolean { + return FALLBACK_SOURCES.has(source) +} + +export type SessionDisplayIdentity = { + /** Provider-side uuid (Claude) or rollout uuid (Codex). The resume argument. */ + providerSessionId: string + kind: AgentProviderKind + cwd: string | null + /** Already resolved through the ladder. Never empty. */ + label: string + labelSource: SessionLabelSource + /** File mtime epoch ms — the sort key every picker already used. */ + lastActivityAt: number + gitBranch: string | null +} + +/** Raw ingredients, before the ladder runs. Providers populate what they can + * and leave the rest null; the ladder — not the provider — decides which one + * wins. That inversion is the whole point of this module. */ +export type SessionIdentityInput = { + providerSessionId: string + kind: AgentProviderKind + cwd?: string | null + customTitle?: string | null + firstPrompt?: string | null + lastPrompt?: string | null + lastActivityAt: number + gitBranch?: string | null +} + +// Long prompts are pasted walls of text often enough that an untruncated label +// would blow out every row. The picker shows one line; the conversation itself +// is one click away. +const LABEL_MAX_CHARS = 120 + +/** Truncated-id length. Matches what the old surfaces displayed, so a user who + * learned to recognise `8d6926a5` keeps that recognition. */ +const ID_LABEL_CHARS = 8 + +function clean(value: string | null | undefined): string | null { + if (typeof value !== 'string') return null + // Collapse newlines: a label is one line, and a multi-line first prompt would + // otherwise render as a tall row or get silently clipped by CSS with no + // ellipsis to signal it. + const collapsed = value.replace(/\s+/g, ' ').trim() + if (!collapsed) return null + if (collapsed.length <= LABEL_MAX_CHARS) return collapsed + return collapsed.slice(0, LABEL_MAX_CHARS).trimEnd() + '…' +} + +/** Last non-empty path segment. Duplicated deliberately rather than imported + * from the renderer's `sessionDisplay.ts`: this module is main-side and shared, + * and reaching into a renderer feature folder for four lines would invert the + * dependency direction. */ +function cwdBasename(cwd: string | null | undefined): string | null { + if (!cwd) return null + const parts = cwd.replace(/\/+$/, '').split('/').filter(Boolean) + return parts[parts.length - 1] ?? null +} + +/** + * The ladder. One order, both providers. + * + * WHY first-prompt outranks last-prompt (a change from Claude's old behaviour): + * users recognise a conversation by how it STARTED, not by whatever it happened + * to be doing when they last touched it — a last-prompt label mutates as the + * session continues, so the same conversation is unrecognisable an hour later. + * Codex already keyed on the first message; adopting it everywhere also makes a + * provider switch identity-preserving. + * + * WHY a nameless session still gets a label instead of being dropped: Claude's + * lister used to `return null` when no summary could be derived, making those + * sessions invisible in the resume picker. A session you cannot see is strictly + * worse than one labelled by its folder — the id is still resumable. + */ +export function buildSessionDisplayIdentity( + input: SessionIdentityInput, +): SessionDisplayIdentity { + const base = { + providerSessionId: input.providerSessionId, + kind: input.kind, + cwd: input.cwd ?? null, + lastActivityAt: input.lastActivityAt, + gitBranch: input.gitBranch ?? null, + } + + const customTitle = clean(input.customTitle) + if (customTitle) return { ...base, label: customTitle, labelSource: 'custom-title' } + + const firstPrompt = clean(input.firstPrompt) + if (firstPrompt) return { ...base, label: firstPrompt, labelSource: 'first-prompt' } + + const lastPrompt = clean(input.lastPrompt) + if (lastPrompt) return { ...base, label: lastPrompt, labelSource: 'last-prompt' } + + const basename = cwdBasename(input.cwd) + if (basename) return { ...base, label: basename, labelSource: 'cwd' } + + return { + ...base, + label: input.providerSessionId.slice(0, ID_LABEL_CHARS), + labelSource: 'session-id', + } +} + +/** + * Adapt a provider lister's `SessionInfo` into ladder ingredients. + * + * WHY this adapter exists rather than changing the listers to emit the + * ingredients directly: Codex's lister lives in the `codex-headless` submodule, + * a separate repository. Making it emit `{ label, labelSource }` is the right + * end state, but it would block every UI fix in this issue behind a cross-repo + * PR, a submodule bump, and a lockfile resync. This adapter recovers the + * ingredients from what the listers already return, in-app and today. + */ +export function identityInputFromSessionInfo( + info: SessionInfo, + kind: AgentProviderKind, +): SessionIdentityInput { + const summary = clean(info.summary) + const customTitle = clean(info.customTitle) + const firstPrompt = clean(info.firstPrompt) + + if (kind === 'codex') { + // Codex populates only `summary`, and it is the FIRST user message — except + // when the lister found no user text at all, where it writes + // `sessionId.slice(0, 8)` into that same field (codex-headless + // SessionList.ts:250,264). + // + // WHY comparing against the id is sound rather than a guess: we control + // both sides of this comparison, and the alternative — trusting the string — + // renders a hex id as if the user had typed it. A user whose first prompt is + // exactly the 8 leading hex characters of their own rollout id is not a case + // worth engineering around. Delete this branch once codex-headless returns + // structured identity; the ladder above needs no change when it does. + const looksLikeIdFallback = + summary !== null && summary === info.sessionId.slice(0, ID_LABEL_CHARS) + return { + providerSessionId: info.sessionId, + kind, + cwd: info.cwd ?? null, + firstPrompt: looksLikeIdFallback ? null : summary, + lastActivityAt: info.lastModified, + gitBranch: info.gitBranch ?? null, + } + } + + // Claude's `summary` is itself a pre-flattened `customTitle ?? lastPrompt ?? + // firstPrompt`. `customTitle` and `firstPrompt` come back as their own fields, + // so when `summary` matches neither it must be the lastPrompt — that is the + // only remaining branch of the lister's own expression. Recovering it this way + // avoids widening SessionInfo (and therefore the submodule's copy of it) just + // to carry a value that is already implied. + const lastPrompt = + summary && summary !== customTitle && summary !== firstPrompt ? summary : null + + return { + providerSessionId: info.sessionId, + kind, + cwd: info.cwd ?? null, + customTitle, + firstPrompt, + lastPrompt, + lastActivityAt: info.lastModified, + gitBranch: info.gitBranch ?? null, + } +} diff --git a/src/shared/types/sessionIndex.ts b/src/shared/types/sessionIndex.ts new file mode 100644 index 00000000..65586ad2 --- /dev/null +++ b/src/shared/types/sessionIndex.ts @@ -0,0 +1,48 @@ +import type { AgentProviderKind } from '@shared/types/providerKind.js' +import type { SessionDisplayIdentity } from '@shared/types/sessionDisplayIdentity.js' + +// The session prompt index wire shape. +// +// WHY this lives in @shared rather than beside the scanner in +// `src/main/sessionIndex.ts`: it previously existed twice — once there and once +// hand-copied into `src/preload/api/types.ts` — and the two were free to drift +// silently across the process boundary. #96 was partly a symptom of exactly +// that kind of duplication: the same concept described by two types that nobody +// was obliged to keep in agreement. Main owns the scanning; the SHAPE is a +// contract, so it belongs at the neutral boundary both sides already import. + +export type SessionIndexPrompt = { + text: string + /** Epoch ms if the entry's ISO timestamp parsed, else null. */ + ts: number | null +} + +export type SessionIndexEntry = { + /** Provider-side uuid (Claude) or rollout uuid (Codex). Stable; + * used as the resume argument. */ + providerSessionId: string + kind: AgentProviderKind + /** Cwd the session was recorded in (from session_meta for Codex; + * from the first entry's cwd field for Claude). Falls back to + * empty string if not discoverable. */ + cwd: string + /** File mtime epoch ms. Primary sort key for the recent view. */ + lastModified: number + /** Legacy one-line summary from the provider listers. + * + * DEPRECATED for display (#96) — read `identity.label` instead. This field + * is a pre-flattened `customTitle ?? lastPrompt ?? firstPrompt` for Claude + * and a first-user-message-or-hex-id for Codex, which is precisely why the + * two providers used to name the same conversation differently. It is kept + * because search ranking and non-display callers still read it; it must not + * come back as a row label. */ + summary: string + /** Up to the last N user prompts (newest first). Empty array + * when a session exists on disk but has no visible user prompts + * (rare — fresh session with only assistant bootstrap text). */ + recentUserPrompts: SessionIndexPrompt[] + /** Count of matched prompts when returned from search, else 0. */ + matchCount: number + /** The one identity every picker renders. Derived once in main. */ + identity: SessionDisplayIdentity +}