diff --git a/docs/design/session-title/session-title-design.md b/docs/design/session-title/session-title-design.md new file mode 100644 index 000000000..7f439a393 --- /dev/null +++ b/docs/design/session-title/session-title-design.md @@ -0,0 +1,376 @@ +# Session Title Design + +> A 3-7 word sentence-case session title generated by the fast model after +> the first assistant turn. Persisted in the session JSONL with a +> `titleSource: 'auto' | 'manual'` tag, surfaced in the session picker, +> and regeneratable on demand via `/rename --auto`. + +## Overview + +`/rename` (#3093) lets a user label a session so they can find it again in +the picker later, but until they run it the picker shows the first user +prompt — often truncated mid-sentence, or describing a framing question +rather than what the session actually became about. Manual renaming is +optional friction most users never do. + +The goal is to make session names _useful by default_: + +- **Descriptive** of what the session actually accomplished, not just the + opening line. 3-7 words, sentence case, git-commit-subject style. +- **Best-effort**: fires in the background after the first reply; if it + fails the user never sees an error. +- **Deferential to the user**: never clobber a `/rename` title the user + chose deliberately, even across CLI tabs on the same session. +- **Explicitly regeneratable** via `/rename --auto` for the "auto title + became stale / I want a fresh one" case. + +## Triggers + +| Trigger | Conditions | Implementation | +| ---------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------- | +| **Auto** | After `recordAssistantTurn` fires. Skipped if an existing title is set, another attempt is in-flight, cap reached, non-interactive, env disabled, or no fast model. | `ChatRecordingService.maybeTriggerAutoTitle` — fire-and-forget | +| **Manual** | User runs `/rename --auto` | `renameCommand.ts` via `tryGenerateSessionTitle` | + +Both paths funnel into a single function — `tryGenerateSessionTitle(config, +signal)` — to guarantee identical prompt, schema, model selection, and +sanitization. The auto trigger is a best-effort background call; the +manual `/rename --auto` is a blocking user action that surfaces a +reason-specific error on failure. + +## Architecture + +``` +┌─────────────────────────────────────────────────────────────────────────┐ +│ packages/core/src/services/ │ +│ │ +│ ┌──────────────────────────┐ │ +│ │ chatRecordingService.ts │ │ +│ │ │ │ +│ │ recordAssistantTurn() │ │ +│ │ │ │ │ +│ │ ↓ │ │ +│ │ maybeTriggerAutoTitle() │── 6 guards ──→ IIFE(autoTitleController) │ +│ │ │ │ │ │ +│ │ └── resume hydrate │ ↓ │ +│ │ via │ tryGenerateSessionTitle │ +│ │ getSessionTitle- │ (sessionTitle.ts) │ +│ │ Info │ │ │ +│ │ │ ↓ │ +│ └──────────────────────────┘ BaseLlmClient.generateJson │ +│ (fastModel + JSON schema) │ +│ │ │ +│ ┌──────────────────────────┐ ↓ │ +│ │ sessionService.ts │ sanitizeTitle + sanity checks │ +│ │ │ │ │ +│ │ getSessionTitleInfo() │◀── cross-process ↓ │ +│ │ uses │ re-read recordCustomTitle │ +│ │ readLastJsonString- │ before write (…, 'auto') │ +│ │ FieldsSync │ │ +│ │ (sessionStorageUtils) │ │ +│ └──────────────────────────┘ │ +│ │ +│ ┌─────────────────────┐ │ +│ │ utils/terminalSafe │ │ +│ │ stripTerminalCtrl- │ │ +│ │ Sequences │ │ +│ └─────────────────────┘ │ +└─────────────────────────────────────────────────────────────────────────┘ + +┌─────────────────────────────────────────────────────────────────────────┐ +│ packages/cli/src/ui/ │ +│ │ +│ commands/renameCommand.ts ─── /rename → manual │ +│ ─── /rename → kebab │ +│ ─── /rename --auto → auto │ +│ ─── /rename -- --literal → manual │ +│ ─── /rename --unknown-flag → error │ +│ │ +│ components/SessionPicker.tsx ── dims rows where │ +│ session.titleSource === 'auto' │ +└─────────────────────────────────────────────────────────────────────────┘ +``` + +### Files + +| File | Responsibility | +| ---------------------------------------------------- | ---------------------------------------------------------------------------------- | +| `packages/core/src/services/sessionTitle.ts` | One-shot LLM call + history filter + sanitize. Exports `tryGenerateSessionTitle`. | +| `packages/core/src/services/chatRecordingService.ts` | `maybeTriggerAutoTitle` trigger, guards, cross-process re-read, abort-on-finalize. | +| `packages/core/src/services/sessionService.ts` | `getSessionTitleInfo` public accessor; `renameSession` accepts `titleSource`. | +| `packages/core/src/utils/sessionStorageUtils.ts` | `extractLastJsonStringFields` + `readLastJsonStringFieldsSync` atomic pair reader. | +| `packages/core/src/utils/terminalSafe.ts` | `stripTerminalControlSequences` shared by sentence-case and kebab paths. | +| `packages/cli/src/ui/commands/renameCommand.ts` | `/rename --auto`, sentinel parser, failure-reason message map. | +| `packages/cli/src/ui/components/SessionPicker.tsx` | Dim styling for `titleSource === 'auto'`. | + +## Prompt Design + +### System Prompt + +Replaces the main agent's system prompt for this single call so the model +only tries to label the session, not behave as a coding assistant. + +Bullets below correspond 1:1 with `TITLE_SYSTEM_PROMPT`: + +- 3-7 words, sentence case (only first word and proper nouns capitalized). +- No trailing punctuation, no markdown, no quotes. +- Match the dominant language of the conversation; for Chinese, budget + roughly 12-20 characters. +- Be specific about the user's actual goal — name the feature, bug, or + subject area. Avoid vague catch-alls like "Code changes" or "Help + request". +- Four good examples (three English + one Chinese) and four bad examples + (too vague / too long / wrong case / trailing punctuation). +- Return only a JSON object with a single `title` key. + +### Structured Output (JSON schema) + +Instead of wrapping output in tags (as session-recap does), we use +`BaseLlmClient.generateJson` with a function-calling schema: + +```ts +const TITLE_SCHEMA = { + type: 'object', + properties: { + title: { + type: 'string', + description: + 'A concise sentence-case session title, 3-7 words, no trailing punctuation.', + }, + }, + required: ['title'], +}; +``` + +Why function calling rather than free text + tag extraction: + +1. Cross-provider reliability — OpenAI-compatible endpoints, Gemini, and + Qwen's native tool-calling all implement function calling; tag parsing + would rely on every model respecting a text convention. +2. No reasoning-preamble leakage — the function call arguments come back + structured, so a "thinking" paragraph before the answer can't bleed + into the title. +3. Simpler post-processing — a single `typeof result.title === 'string'` + check plus `sanitizeTitle` covers every realistic model drift. + +The model may still return something the schema allows but the UX +rejects (empty string, whitespace-only, 500 chars, markdown fencing, +control chars). `sanitizeTitle` handles all of these and returns `''` → +service returns `{ok: false, reason: 'empty_result'}`. + +### Call Parameters + +| Parameter | Value | Reason | +| ----------------- | ------------------------------ | ----------------------------------------------------------------------------------------------- | +| `model` | `getFastModel()` — no fallback | Auto-titling on main-model tokens is too expensive to be silent. | +| `schema` | `TITLE_SCHEMA` | Forces `{title: string}`; filters shape drift at the transport layer. | +| `maxOutputTokens` | `100` | More than enough for 7 words plus schema overhead. | +| `temperature` | `0.2` | Mostly deterministic — session titles benefit from stability across regeneration. | +| `maxAttempts` | `1` | Titles are best-effort cosmetic metadata; retries would queue behind user-visible main traffic. | + +Contrast with session-recap, which falls back to the main model. Title +generation is triggered automatically and often; silently spending +main-model tokens without a user opt-in is a real bill surprise. Manual +`/rename --auto` explicitly fails with `no_fast_model` rather than +fallback — forcing the user to make the fast-model choice consciously. + +## History Filtering + +`geminiClient.getChat().getHistory()` returns `Content[]` that includes +tool calls, tool responses (often 10K+ tokens of file content), and model +thought parts. Feeding that raw into the title LLM would bias the label +toward implementation noise like "Called grep on auth module". + +`filterToDialog` keeps only `user` / `model` entries with non-empty text +and no `thought` / `thoughtSignature` parts. `takeRecentDialog` slices to +the last 20 messages and refuses to start on a dangling model/tool +response. `flattenToTail` converts to "Role: text" lines and slices the +last 1000 characters. + +### The 1000-character tail slice + +A session that starts with `help me debug X` but pivots to refactoring Y +should be titled about Y. Titling by the head locks in the opening +framing; titling by the tail captures what the session became. + +### UTF-16 surrogate handling + +`.slice(-1000)` on a UTF-16 code-unit boundary can orphan a high or low +surrogate if a CJK supplementary char or emoji gets cut. Some providers +respond to the resulting invalid UTF-16 with a 400 — which, without +handling, would burn an attempt for no reason. `flattenToTail` drops a +leading orphaned low surrogate; `sanitizeTitle` scrubs any orphaned +surrogate after the max-length trim on the output path too. + +## Persistence + +### Record shape + +`CustomTitleRecordPayload` grows an optional `titleSource: 'auto' | +'manual'` field: + +```jsonc +{ + "type": "system", + "subtype": "custom_title", + "systemPayload": { + "customTitle": "Debug login button on mobile", + "titleSource": "auto", + }, +} +``` + +The field is optional, and absent-in-legacy records are treated as +`undefined`. `SessionPicker` dims rows only on a strict `=== 'auto'` +match — a pre-change user `/rename` title is never silently reclassified +as a model guess. + +### Resume hydration + +On resume, `ChatRecordingService` constructor calls +`sessionService.getSessionTitleInfo(sessionId)` to read **both** the +title and its source. Without hydrating the source, `finalize()`'s +re-append (which runs on every session lifecycle event) would rewrite +auto as manual on every resume cycle — silently stripping the dim +affordance. + +### Atomic pair read + +`extractLastJsonStringFields` returns `customTitle` and `titleSource` +from the **same matching line** in a single scan. Two separate +`readLastJsonStringFieldSync` calls could land on different records if +an older line has only the primary field, yielding a mismatched pair. +The extractor also requires a proper closing quote on the primary value, +so a crash-truncated trailing record can't win the latest-match race. + +### Full-file scan cap + +Phase-2 (when the tail-window fast path misses) streams the whole file +in 64KB chunks. Capped at `MAX_FULL_SCAN_BYTES = 64 MB` so a corrupt +multi-GB JSONL can't freeze the session picker on the main event loop. +The picker's latency envelope survives corruption. + +### Symlink defense + +Session reads open with `O_NOFOLLOW` (falls back to plain read-only on +Windows, where the constant is not exposed). Defense in depth so a +symlink planted in `~/.qwen/projects//chats/` can't redirect a +metadata read to an unrelated file. + +## Concurrency and Edge Cases + +### Trigger guard order + +`maybeTriggerAutoTitle` checks six conditions in this exact order — each +short-circuits the rest so the cheap ones run first: + +1. `currentCustomTitle` set → skip. Never overwrite manual / prior auto. +2. `autoTitleController !== undefined` → skip. One attempt at a time. +3. `autoTitleAttempts >= 3` → skip. Cap bounds total waste. +4. `!config.isInteractive()` → skip. Headless `qwen -p` / CI never spends + fast-model tokens on a one-shot session. +5. `autoTitleDisabledByEnv()` → skip. `QWEN_DISABLE_AUTO_TITLE=1` + explicit opt-out. +6. `!config.getFastModel()` → skip. No fast-model → no-op. + +### Why the cap is 3, not 1 + +The first assistant turn can be a pure tool-call with no user-visible +text (e.g. the model opens with a `grep`). `tryGenerateSessionTitle` +returns `{ok: false, reason: 'empty_history'}` in that case. Without a +retry window, an entire session's chance at a title would be burned on +turn 1 before the user said anything interesting. Cap of 3 covers the +common "first turn is noise" case while still bounding runaway retry on +a persistently failing fast model. + +### Cross-process manual-rename race + +Two CLI tabs on the same session file can diverge in memory. Tab A runs +`/rename foo` and writes `titleSource: manual`. Tab B's +`ChatRecordingService` has its own `currentCustomTitle = undefined` and +would naively overwrite with an auto title. + +After the LLM call resolves, the IIFE re-reads the JSONL via +`sessionService.getSessionTitleInfo`. If the file shows +`source: 'manual'`, the IIFE bails AND syncs its in-memory state so +subsequent turns respect the rename too. Cost: one 64KB tail read per +successful generation; negligible. + +### Abort propagation on `finalize()` + +`autoTitleController` doubles as the in-flight flag. `finalize()` (run +on session switch and process shutdown) calls +`autoTitleController.abort()` before re-appending the title record. The +LLM socket is cancelled promptly; session switch doesn't wait on a slow +fast-model call. The IIFE's `finally` block clears +`autoTitleController` only if it's still the active one, so a finalize +mid-flight doesn't race a concurrent `recordAssistantTurn`. + +### Manual `/rename` lands mid-flight + +Between the IIFE's `await` completing and the `recordCustomTitle('auto')` +call, the user could `/rename foo`. The IIFE re-checks +`this.currentTitleSource === 'manual'` and bails. The in-process check +AND the cross-process re-read both run; manual wins at both layers. + +## Configuration + +### User-facing knobs + +| Setting / env var | Default | Effect | +| --------------------------- | ------- | --------------------------------------------------------------------------------------------------- | +| `fastModel` | unset | Required for auto-titling. Unset → no-op (no main-model fallback). | +| `QWEN_DISABLE_AUTO_TITLE=1` | unset | Opt out of the auto trigger without unsetting `fastModel`. `/rename --auto` still works on request. | + +No `settings.json` toggle — the env var is the only user-visible +off-switch. Rationale: the feature is cosmetic and cheap; a settings +toggle would add a UI surface for something that can live as a one-time +env export for the few users who want to disable it. + +### Why auto doesn't fall back to the main model + +Auto-titling is triggered unconditionally after every assistant turn. +If a user without a fast model were silently charged main-model tokens +for every new session's title, the cost delta is invisible until the +monthly bill arrives. Failing quietly (no-op, no title, no cost) is the +safer default. `/rename --auto` surfaces `no_fast_model` as an +actionable error so the user can set one if they want to. + +## Observability + +`createDebugLogger('SESSION_TITLE')` emits `debugLogger.warn` from the +generator's catch block. Failures are fully transparent to the user — +auto-title is an auxiliary feature and never throws into the UI. + +Developers can grep for the `[SESSION_TITLE]` tag in the debug log +(`~/.qwen/debug/.txt`; `latest.txt` symlinks to the current +session). A working end-to-end call produces no log output; a failing +one gets one WARN line with the underlying error message. + +## Security Hardening + +The title value is rendered verbatim in the terminal (session picker) +AND persisted in a user-readable JSONL file. Both surfaces are attack +reachable if a compromised or prompt-injected fast model returns +hostile text. + +| Concern | Guard | +| ------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------- | +| ANSI / OSC-8 / CSI injection | `stripTerminalControlSequences` before both JSONL write and picker render. | +| Clickable-link smuggle via OSC-8 | Same — OSC sequences stripped as whole units, not just the ESC byte. | +| Invalid UTF-16 surrogates | Scrubbed in `flattenToTail` (LLM input) and `sanitizeTitle` (LLM output after max-length trim). | +| Subtype-line spoof via user message content | `lineContains: '"subtype":"custom_title"'` — user text that happens to contain the literal phrase can't shadow a real record. | +| Symlink redirect on session reads | `O_NOFOLLOW` (no-op on Windows where the constant is missing). | +| Truncated trailing JSONL record | `extractLastJsonStringFields` requires a closing quote before a record wins the latest-match race. | +| Pathological file size freezing the picker | `MAX_FULL_SCAN_BYTES = 64 MB` cap on Phase-2 full-file scan. | +| Paired CJK bracket decorators (`【Draft】`) | Stripped as a unit so a lone closing bracket doesn't dangle. | + +## Out of Scope + +| Item | Why not | +| ------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------- | +| Auto-regenerate when the title goes stale | `/rename --auto` is the explicit user-triggered path. Silent mid-session title swaps would confuse users scrolling back through the picker. | +| WebUI / VSCode dim-styling parity | Those surfaces read `customTitle` already and will show auto titles as if manual. A follow-up can wire the `titleSource` through. | +| Settings-dialog toggle for auto generation | Env var is the single knob. Full settings UI is easy to add later if user demand surfaces. | +| i18n locale catalog entries for new strings | Consistent with existing `/rename` strings, which fall through to English. A repo-wide i18n pass is out of scope. | +| Migration to re-classify legacy records | Back-compat by design: absent `titleSource` is treated as manual. Rewriting old records would risk losing user intent. | +| Non-interactive auto-titling | `qwen -p` / CI scripts throw the session away; fast-model tokens for a title no one will ever resume is pure waste. | diff --git a/packages/cli/src/ui/commands/renameCommand.test.ts b/packages/cli/src/ui/commands/renameCommand.test.ts index bc334c8b3..854b50d52 100644 --- a/packages/cli/src/ui/commands/renameCommand.test.ts +++ b/packages/cli/src/ui/commands/renameCommand.test.ts @@ -9,16 +9,31 @@ import { renameCommand } from './renameCommand.js'; import { type CommandContext } from './types.js'; import { createMockCommandContext } from '../../test-utils/mockCommandContext.js'; +const tryGenerateSessionTitleMock = vi.fn(); + +vi.mock('@qwen-code/qwen-code-core', async (importOriginal) => { + const original = + (await importOriginal()) as typeof import('@qwen-code/qwen-code-core'); + return { + ...original, + tryGenerateSessionTitle: (...args: unknown[]) => + tryGenerateSessionTitleMock(...args), + }; +}); + describe('renameCommand', () => { let mockContext: CommandContext; beforeEach(() => { mockContext = createMockCommandContext(); + tryGenerateSessionTitleMock.mockReset(); }); it('should have the correct name and description', () => { expect(renameCommand.name).toBe('rename'); - expect(renameCommand.description).toBe('Rename the current conversation'); + expect(renameCommand.description).toBe( + 'Rename the current conversation. --auto lets the fast model pick a title.', + ); }); it('should return error when config is not available', async () => { @@ -103,7 +118,7 @@ describe('renameCommand', () => { const result = await renameCommand.action!(mockContext, 'my-feature'); - expect(mockRecordCustomTitle).toHaveBeenCalledWith('my-feature'); + expect(mockRecordCustomTitle).toHaveBeenCalledWith('my-feature', 'manual'); expect(result).toEqual({ type: 'message', messageType: 'info', @@ -130,6 +145,7 @@ describe('renameCommand', () => { expect(mockRenameSession).toHaveBeenCalledWith( 'test-session-id', 'my-feature', + 'manual', ); expect(result).toEqual({ type: 'message', @@ -159,4 +175,270 @@ describe('renameCommand', () => { content: 'Failed to rename session.', }); }); + + describe('bare /rename model selection', () => { + // Pins the kebab-case path's model choice: bare `/rename` (no args) + // prefers fastModel when one is configured, falls back to the main + // model otherwise. Previous tests mocked `getHistory: []` which bailed + // before the model selection ran, leaving this regression-prone. + function mockConfigForKebab(opts: { fastModel?: string; model?: string }): { + config: unknown; + generateContent: ReturnType; + } { + const generateContent = vi.fn().mockResolvedValue({ + candidates: [{ content: { parts: [{ text: 'fix-login-bug' }] } }], + }); + const config = { + getChatRecordingService: vi.fn().mockReturnValue({ + recordCustomTitle: vi.fn().mockReturnValue(true), + }), + getFastModel: vi.fn().mockReturnValue(opts.fastModel), + getModel: vi.fn().mockReturnValue(opts.model ?? 'main-model'), + getGeminiClient: vi.fn().mockReturnValue({ + getHistory: vi.fn().mockReturnValue([ + { role: 'user', parts: [{ text: 'fix the login bug' }] }, + { + role: 'model', + parts: [{ text: 'Looking at the handler now.' }], + }, + ]), + }), + getContentGenerator: vi.fn().mockReturnValue({ generateContent }), + }; + return { config, generateContent }; + } + + it('uses fastModel when configured', async () => { + const { config, generateContent } = mockConfigForKebab({ + fastModel: 'qwen-turbo', + model: 'main-model', + }); + mockContext = createMockCommandContext({ + services: { config: config as never }, + }); + + await renameCommand.action!(mockContext, ''); + + expect(generateContent).toHaveBeenCalledOnce(); + expect(generateContent.mock.calls[0][0].model).toBe('qwen-turbo'); + }); + + it('falls back to main model when fastModel is unset', async () => { + const { config, generateContent } = mockConfigForKebab({ + fastModel: undefined, + model: 'main-model', + }); + mockContext = createMockCommandContext({ + services: { config: config as never }, + }); + + await renameCommand.action!(mockContext, ''); + + expect(generateContent).toHaveBeenCalledOnce(); + expect(generateContent.mock.calls[0][0].model).toBe('main-model'); + }); + }); + + describe('--auto flag', () => { + it('refuses --auto when no fast model is configured', async () => { + const mockConfig = { + getChatRecordingService: vi.fn().mockReturnValue({ + recordCustomTitle: vi.fn(), + }), + getFastModel: vi.fn().mockReturnValue(undefined), + }; + mockContext = createMockCommandContext({ + services: { config: mockConfig as never }, + }); + + const result = await renameCommand.action!(mockContext, '--auto'); + + expect(result).toEqual({ + type: 'message', + messageType: 'error', + content: + '/rename --auto requires a fast model. Configure one with `/model --fast `.', + }); + expect(tryGenerateSessionTitleMock).not.toHaveBeenCalled(); + }); + + it('refuses --auto combined with a positional name', async () => { + const mockConfig = { + getChatRecordingService: vi.fn().mockReturnValue({ + recordCustomTitle: vi.fn(), + }), + getFastModel: vi.fn().mockReturnValue('qwen-turbo'), + }; + mockContext = createMockCommandContext({ + services: { config: mockConfig as never }, + }); + + const result = await renameCommand.action!(mockContext, '--auto my-name'); + + expect(result).toEqual({ + type: 'message', + messageType: 'error', + content: + '/rename --auto does not take a name. Use `/rename ` to set a name yourself.', + }); + expect(tryGenerateSessionTitleMock).not.toHaveBeenCalled(); + }); + + it('writes an auto-sourced title on --auto success', async () => { + tryGenerateSessionTitleMock.mockResolvedValue({ + ok: true, + title: 'Fix login button on mobile', + modelUsed: 'qwen-turbo', + }); + const mockRecordCustomTitle = vi.fn().mockReturnValue(true); + const mockConfig = { + getChatRecordingService: vi.fn().mockReturnValue({ + recordCustomTitle: mockRecordCustomTitle, + }), + getFastModel: vi.fn().mockReturnValue('qwen-turbo'), + }; + mockContext = createMockCommandContext({ + services: { config: mockConfig as never }, + }); + + const result = await renameCommand.action!(mockContext, '--auto'); + + expect(tryGenerateSessionTitleMock).toHaveBeenCalledOnce(); + expect(mockRecordCustomTitle).toHaveBeenCalledWith( + 'Fix login button on mobile', + 'auto', + ); + expect(result).toEqual({ + type: 'message', + messageType: 'info', + content: 'Session renamed to "Fix login button on mobile"', + }); + }); + + it('surfaces empty_history reason with actionable hint', async () => { + tryGenerateSessionTitleMock.mockResolvedValue({ + ok: false, + reason: 'empty_history', + }); + const mockConfig = { + getChatRecordingService: vi.fn().mockReturnValue({ + recordCustomTitle: vi.fn(), + }), + getFastModel: vi.fn().mockReturnValue('qwen-turbo'), + }; + mockContext = createMockCommandContext({ + services: { config: mockConfig as never }, + }); + + const result = await renameCommand.action!(mockContext, '--auto'); + + expect(result).toEqual({ + type: 'message', + messageType: 'error', + content: + 'No conversation to title yet — send at least one message first.', + }); + }); + + it('surfaces model_error reason distinctly', async () => { + tryGenerateSessionTitleMock.mockResolvedValue({ + ok: false, + reason: 'model_error', + }); + const mockConfig = { + getChatRecordingService: vi.fn().mockReturnValue({ + recordCustomTitle: vi.fn(), + }), + getFastModel: vi.fn().mockReturnValue('qwen-turbo'), + }; + mockContext = createMockCommandContext({ + services: { config: mockConfig as never }, + }); + + const result = await renameCommand.action!(mockContext, '--auto'); + + expect(result).toMatchObject({ + messageType: 'error', + }); + expect((result as { content: string }).content).toMatch( + /rate limit, auth, or network error/, + ); + }); + + it('rejects unknown flag with sentinel hint', async () => { + const mockConfig = { + getChatRecordingService: vi.fn().mockReturnValue({ + recordCustomTitle: vi.fn(), + }), + getFastModel: vi.fn().mockReturnValue('qwen-turbo'), + }; + mockContext = createMockCommandContext({ + services: { config: mockConfig as never }, + }); + + const result = await renameCommand.action!( + mockContext, + '--my-label-with-dashes', + ); + + expect(result).toMatchObject({ messageType: 'error' }); + const content = (result as { content: string }).content; + expect(content).toMatch(/Unknown flag "--my-label-with-dashes"/); + expect(content).toMatch(/\/rename -- --my-label-with-dashes/); + expect(tryGenerateSessionTitleMock).not.toHaveBeenCalled(); + }); + + it('surfaces aborted reason when user cancels', async () => { + tryGenerateSessionTitleMock.mockResolvedValue({ + ok: false, + reason: 'aborted', + }); + const mockConfig = { + getChatRecordingService: vi.fn().mockReturnValue({ + recordCustomTitle: vi.fn(), + }), + getFastModel: vi.fn().mockReturnValue('qwen-turbo'), + }; + mockContext = createMockCommandContext({ + services: { config: mockConfig as never }, + }); + + const result = await renameCommand.action!(mockContext, '--auto'); + + expect(result).toEqual({ + type: 'message', + messageType: 'error', + content: 'Title generation was cancelled.', + }); + }); + + it('falls back to SessionService.renameSession with auto source', async () => { + tryGenerateSessionTitleMock.mockResolvedValue({ + ok: true, + title: 'Audit auth middleware', + modelUsed: 'qwen-turbo', + }); + const mockRenameSession = vi.fn().mockResolvedValue(true); + const mockConfig = { + getChatRecordingService: vi.fn().mockReturnValue(undefined), + getSessionId: vi.fn().mockReturnValue('test-session-id'), + getSessionService: vi.fn().mockReturnValue({ + renameSession: mockRenameSession, + }), + getFastModel: vi.fn().mockReturnValue('qwen-turbo'), + }; + mockContext = createMockCommandContext({ + services: { config: mockConfig as never }, + }); + + const result = await renameCommand.action!(mockContext, '--auto'); + + expect(mockRenameSession).toHaveBeenCalledWith( + 'test-session-id', + 'Audit auth middleware', + 'auto', + ); + expect(result).toMatchObject({ messageType: 'info' }); + }); + }); }); diff --git a/packages/cli/src/ui/commands/renameCommand.ts b/packages/cli/src/ui/commands/renameCommand.ts index e1a594331..5a4137bf6 100644 --- a/packages/cli/src/ui/commands/renameCommand.ts +++ b/packages/cli/src/ui/commands/renameCommand.ts @@ -5,10 +5,13 @@ */ import type { Content } from '@google/genai'; -import type { Config } from '@qwen-code/qwen-code-core'; import { getResponseText, SESSION_TITLE_MAX_LENGTH, + stripTerminalControlSequences, + tryGenerateSessionTitle, + type Config, + type SessionTitleFailureReason, } from '@qwen-code/qwen-code-core'; import type { SlashCommand, SlashCommandActionReturn } from './types.js'; import { CommandKind } from './types.js'; @@ -38,9 +41,11 @@ function extractConversationText(history: Content[]): string { } /** - * Calls the LLM to generate a short session title from conversation history. + * Calls the LLM to generate a short kebab-case session title from conversation + * history. Used when `/rename` is invoked with no arguments — produces a + * filesystem-style name for sessions the user wants to keep long-term. */ -async function generateSessionTitle( +async function generateKebabTitle( config: Config, signal?: AbortSignal, ): Promise { @@ -51,9 +56,15 @@ async function generateSessionTitle( return null; } + // Prefer the fast model for title generation — it's much cheaper and + // faster than the main model, and title generation is a small bounded + // task that doesn't need main-model reasoning. Falls back to the main + // model when no fast model is configured so this path never fails to + // start. + const model = config.getFastModel() ?? config.getModel(); const response = await config.getContentGenerator().generateContent( { - model: config.getModel(), + model, contents: [ { role: 'user', @@ -79,8 +90,13 @@ async function generateSessionTitle( if (!text) { return null; } - // Clean up: take first line, remove quotes/backticks - const cleaned = text.split('\n')[0].replace(/["`']/g, '').trim(); + // Clean up: strip ANSI / control sequences via the shared helper + // (same security concern as the sentence-case path — the title renders + // directly in the picker), then take the first line and drop quotes. + const cleaned = stripTerminalControlSequences(text) + .split('\n')[0] + .replace(/["`']/g, '') + .trim(); return cleaned.length > 0 && cleaned.length <= MAX_TITLE_LENGTH ? cleaned : null; @@ -89,12 +105,88 @@ async function generateSessionTitle( } } +/** + * Translate a title-generation failure reason into a human-actionable + * message. Exists so `/rename --auto` doesn't collapse to a generic "could + * not generate" that leaves the user guessing about the cause. + */ +function autoFailureMessage(reason: SessionTitleFailureReason): string { + switch (reason) { + case 'no_fast_model': + return t( + '/rename --auto requires a fast model. Configure one with `/model --fast `.', + ); + case 'empty_history': + return t( + 'No conversation to title yet — send at least one message first.', + ); + case 'empty_result': + return t( + 'The fast model returned no usable title. Try `/rename ` to set one yourself.', + ); + case 'aborted': + return t('Title generation was cancelled.'); + case 'model_error': + return t( + 'The fast model could not generate a title (rate limit, auth, or network error). Check debug log or try again.', + ); + case 'no_client': + return t('Session is still initializing — try again in a moment.'); + default: + return t('Could not generate a title.'); + } +} + +/** + * Parse `--auto` out of the args. Kept simple rather than bringing in an + * argv parser — we only have one flag. + * + * Rules: + * - `--auto` (case-insensitive) sets auto=true. + * - `--` terminates flag parsing; everything after is positional, so users + * can legitimately name sessions starting with `--` via `/rename -- --foo`. + * - Any other `--xxx` before `--` bubbles up as `unknownFlag` for a clean + * error, rather than silently becoming part of the title (`--Auto` typo, + * `--help` expectation, etc.). + */ +function parseArgs(raw: string): { + auto: boolean; + positional: string; + unknownFlag?: string; +} { + const trimmed = raw.trim().replace(/[\r\n]+/g, ' '); + if (!trimmed) return { auto: false, positional: '' }; + const parts = trimmed.split(/\s+/); + let auto = false; + let unknownFlag: string | undefined; + let flagsDone = false; + const rest: string[] = []; + for (const p of parts) { + if (!flagsDone && p === '--') { + flagsDone = true; + continue; + } + if (!flagsDone && p.startsWith('--')) { + if (p.toLowerCase() === '--auto') { + auto = true; + continue; + } + if (!unknownFlag) unknownFlag = p; + continue; + } + rest.push(p); + } + return { auto, positional: rest.join(' '), unknownFlag }; +} + export const renameCommand: SlashCommand = { name: 'rename', altNames: ['tag'], kind: CommandKind.BUILT_IN, get description() { - return t('Rename the current conversation'); + return t( + 'Rename the current conversation. --auto lets the fast model pick a title.', + ); }, action: async (context, args): Promise => { const { config } = context.services; @@ -107,10 +199,87 @@ export const renameCommand: SlashCommand = { }; } - let name = args.trim().replace(/[\r\n]+/g, ' '); + const { auto, positional, unknownFlag } = parseArgs(args); + if (unknownFlag) { + return { + type: 'message', + messageType: 'error', + content: t( + 'Unknown flag "{{flag}}". Supported: --auto. To use this as a literal name, run `/rename -- {{flag}}`.', + { flag: unknownFlag }, + ), + }; + } + let name = positional; + // Track where the title came from so the session picker can dim + // auto-generated titles; explicit user text stays 'manual'. + let titleSource: 'auto' | 'manual' = 'manual'; - // If no name provided, auto-generate one from conversation history - if (!name) { + if (auto) { + // Explicit user-triggered auto-title. This overwrites whatever title + // is currently set (manual or auto) because the user asked for it. + // Requires a configured fast model — we don't silently fall back to + // the main model here because `--auto` is a deliberate opt-in to the + // sentence-case fast-model flow, and surprising a user with a main- + // model call would defeat the purpose. + if (!config.getFastModel()) { + return { + type: 'message', + messageType: 'error', + content: t( + '/rename --auto requires a fast model. Configure one with `/model --fast `.', + ), + }; + } + if (positional) { + return { + type: 'message', + messageType: 'error', + content: t( + '/rename --auto does not take a name. Use `/rename ` to set a name yourself.', + ), + }; + } + const dots = ['.', '..', '...']; + let dotIndex = 0; + const baseText = t('Regenerating session title'); + context.ui.setPendingItem({ + type: 'info', + text: baseText + dots[dotIndex], + }); + const timer = setInterval(() => { + dotIndex = (dotIndex + 1) % dots.length; + context.ui.setPendingItem({ + type: 'info', + text: baseText + dots[dotIndex], + }); + }, 500); + // try/finally ensures the spinner stops even if tryGenerateSessionTitle + // ever throws (it currently swallows internally, but defensively so + // future regressions don't leak an interval timer). + let outcome: Awaited>; + try { + outcome = await tryGenerateSessionTitle( + config, + context.abortSignal ?? new AbortController().signal, + ); + } finally { + clearInterval(timer); + context.ui.setPendingItem(null); + } + if (!outcome.ok) { + return { + type: 'message', + messageType: 'error', + content: autoFailureMessage(outcome.reason), + }; + } + name = outcome.title; + titleSource = 'auto'; + } else if (!name) { + // Legacy no-arg behavior: kebab-case, generated via the main content + // generator with fallback to fastModel. Preserved as-is for users who + // prefer filesystem-style names. const dots = ['.', '..', '...']; let dotIndex = 0; const baseText = t('Generating session name'); @@ -125,9 +294,13 @@ export const renameCommand: SlashCommand = { text: baseText + dots[dotIndex], }); }, 500); - const generated = await generateSessionTitle(config, context.abortSignal); - clearInterval(timer); - context.ui.setPendingItem(null); + let generated: string | null; + try { + generated = await generateKebabTitle(config, context.abortSignal); + } finally { + clearInterval(timer); + context.ui.setPendingItem(null); + } if (!generated) { return { type: 'message', @@ -151,7 +324,7 @@ export const renameCommand: SlashCommand = { // Record the custom title in the current session's JSONL file const chatRecordingService = config.getChatRecordingService(); if (chatRecordingService) { - const ok = chatRecordingService.recordCustomTitle(name); + const ok = chatRecordingService.recordCustomTitle(name, titleSource); if (!ok) { return { type: 'message', @@ -163,7 +336,11 @@ export const renameCommand: SlashCommand = { // Fallback: write via SessionService for non-recording sessions const sessionId = config.getSessionId(); const sessionService = config.getSessionService(); - const success = await sessionService.renameSession(sessionId, name); + const success = await sessionService.renameSession( + sessionId, + name, + titleSource, + ); if (!success) { return { type: 'message', diff --git a/packages/cli/src/ui/components/SessionPicker.tsx b/packages/cli/src/ui/components/SessionPicker.tsx index 33b62c06b..c80c35be1 100644 --- a/packages/cli/src/ui/components/SessionPicker.tsx +++ b/packages/cli/src/ui/components/SessionPicker.tsx @@ -94,6 +94,11 @@ function SessionListItemView({ const promptText = session.customTitle || session.prompt || '(empty prompt)'; const truncatedPrompt = truncateText(promptText, maxPromptWidth); + // Dim auto-generated titles so users can distinguish a model guess from + // a title they chose themselves with `/rename`. Selected row keeps the + // accent color — legibility of the focused row wins over source hinting. + const isAutoTitle = + session.titleSource === 'auto' && Boolean(session.customTitle); return ( @@ -111,7 +116,13 @@ function SessionListItemView({ {prefix} {truncatedPrompt} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index a318e4730..55b920508 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -151,6 +151,8 @@ export * from './services/gitService.js'; export * from './services/gitWorktreeService.js'; export * from './services/sessionRecap.js'; export * from './services/sessionService.js'; +export * from './services/sessionTitle.js'; +export { stripTerminalControlSequences } from './utils/terminalSafe.js'; export * from './services/shellExecutionService.js'; export * from './utils/bareMode.js'; diff --git a/packages/core/src/services/chatRecordingService.autoTitle.test.ts b/packages/core/src/services/chatRecordingService.autoTitle.test.ts new file mode 100644 index 000000000..152c1d6f3 --- /dev/null +++ b/packages/core/src/services/chatRecordingService.autoTitle.test.ts @@ -0,0 +1,508 @@ +/** + * @license + * Copyright 2025 Qwen Code + * SPDX-License-Identifier: Apache-2.0 + */ + +import { randomUUID } from 'node:crypto'; +import path from 'node:path'; +import { execSync } from 'node:child_process'; +import fs from 'node:fs'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { Config } from '../config/config.js'; +import { + ChatRecordingService, + type ChatRecord, +} from './chatRecordingService.js'; +import * as jsonl from '../utils/jsonl-utils.js'; + +const tryGenerateSessionTitleMock = vi.fn(); + +vi.mock('./sessionTitle.js', () => ({ + tryGenerateSessionTitle: (...args: unknown[]) => + tryGenerateSessionTitleMock(...args), +})); + +/** + * Most tests assert on the success-outcome path: this helper wraps + * `{title, modelUsed}` in the new `{ok: true, ...}` shape so we don't have + * to repeat it everywhere. Failure outcomes are spelled out where they + * exercise distinct reasons. + */ +function mockOk(title: string, modelUsed = 'qwen-turbo'): void { + tryGenerateSessionTitleMock.mockResolvedValue({ + ok: true, + title, + modelUsed, + }); +} + +vi.mock('node:path'); +vi.mock('node:child_process'); +vi.mock('node:crypto', () => ({ + randomUUID: vi.fn(), + createHash: vi.fn(() => ({ + update: vi.fn(() => ({ + digest: vi.fn(() => 'mocked-hash'), + })), + })), +})); +vi.mock('../utils/jsonl-utils.js'); + +/** + * Let the fire-and-forget auto-title promise kicked off by + * `recordAssistantTurn` settle. The IIFE awaits the generation mock, which + * adds at least one microtask hop; a single `Promise.resolve()` flush isn't + * always enough. A setImmediate boundary after several microtask ticks + * covers pathological cases where a mock resolves via a deeper await chain. + */ +async function flushMicrotasks(): Promise { + for (let i = 0; i < 8; i++) { + await Promise.resolve(); + } + await new Promise((resolve) => setImmediate(resolve)); + for (let i = 0; i < 4; i++) { + await Promise.resolve(); + } +} + +function findCustomTitleRecord(): ChatRecord | undefined { + return vi + .mocked(jsonl.writeLineSync) + .mock.calls.map((c) => c[1] as ChatRecord) + .find((r) => r.type === 'system' && r.subtype === 'custom_title'); +} + +describe('ChatRecordingService - auto-title trigger', () => { + let chatRecordingService: ChatRecordingService; + let mockConfig: Config; + let fastModelValue: string | undefined; + let uuidCounter = 0; + + beforeEach(() => { + uuidCounter = 0; + fastModelValue = 'qwen-turbo'; + tryGenerateSessionTitleMock.mockReset(); + + mockConfig = { + getSessionId: vi.fn().mockReturnValue('test-session-id'), + getProjectRoot: vi.fn().mockReturnValue('/test/project/root'), + getCliVersion: vi.fn().mockReturnValue('1.0.0'), + storage: { + getProjectTempDir: vi + .fn() + .mockReturnValue('/test/project/root/.qwen/tmp/hash'), + getProjectDir: vi + .fn() + .mockReturnValue('/test/project/root/.qwen/projects/test-project'), + }, + getModel: vi.fn().mockReturnValue('qwen-plus'), + getFastModel: vi.fn(() => fastModelValue), + isInteractive: vi.fn().mockReturnValue(true), + getDebugMode: vi.fn().mockReturnValue(false), + getToolRegistry: vi.fn().mockReturnValue({ + getTool: vi.fn().mockReturnValue({ + displayName: 'Test Tool', + description: 'A test tool', + isOutputMarkdown: false, + }), + }), + getResumedSessionData: vi.fn().mockReturnValue(undefined), + // Default SessionService for the cross-process re-read: returns no + // title, i.e. "nothing else has landed on disk" — tests that need + // a specific on-disk state override this mock. + getSessionService: vi.fn().mockReturnValue({ + getSessionTitleInfo: vi.fn().mockReturnValue({}), + getSessionTitle: vi.fn().mockReturnValue(undefined), + }), + } as unknown as Config; + + vi.mocked(randomUUID).mockImplementation( + () => + `00000000-0000-0000-0000-00000000000${++uuidCounter}` as `${string}-${string}-${string}-${string}-${string}`, + ); + vi.mocked(path.join).mockImplementation((...args) => args.join('/')); + vi.mocked(path.dirname).mockImplementation((p) => { + const parts = p.split('/'); + parts.pop(); + return parts.join('/'); + }); + vi.mocked(execSync).mockReturnValue('main\n'); + vi.spyOn(fs, 'mkdirSync').mockImplementation(() => undefined); + vi.spyOn(fs, 'writeFileSync').mockImplementation(() => undefined); + vi.spyOn(fs, 'existsSync').mockReturnValue(false); + + chatRecordingService = new ChatRecordingService(mockConfig); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('writes an auto-sourced title after the first assistant turn', async () => { + mockOk('Fix login button'); + + chatRecordingService.recordAssistantTurn({ + model: 'qwen-plus', + message: [{ text: 'Looking at the button handler now.' }], + }); + + await flushMicrotasks(); + + const titleRecord = findCustomTitleRecord(); + expect(titleRecord).toBeDefined(); + expect(titleRecord?.systemPayload).toEqual({ + customTitle: 'Fix login button', + titleSource: 'auto', + }); + expect(chatRecordingService.getCurrentTitleSource()).toBe('auto'); + expect(tryGenerateSessionTitleMock).toHaveBeenCalledOnce(); + }); + + it('does not trigger when no fast model is configured', async () => { + fastModelValue = undefined; + + chatRecordingService.recordAssistantTurn({ + model: 'qwen-plus', + message: [{ text: 'hi' }], + }); + await flushMicrotasks(); + + expect(tryGenerateSessionTitleMock).not.toHaveBeenCalled(); + expect(findCustomTitleRecord()).toBeUndefined(); + }); + + it('does not overwrite a manual title', async () => { + chatRecordingService.recordCustomTitle('chose-this-myself', 'manual'); + vi.mocked(jsonl.writeLineSync).mockClear(); + + chatRecordingService.recordAssistantTurn({ + model: 'qwen-plus', + message: [{ text: 'reply' }], + }); + await flushMicrotasks(); + + expect(tryGenerateSessionTitleMock).not.toHaveBeenCalled(); + expect(findCustomTitleRecord()).toBeUndefined(); + expect(chatRecordingService.getCurrentCustomTitle()).toBe( + 'chose-this-myself', + ); + expect(chatRecordingService.getCurrentTitleSource()).toBe('manual'); + }); + + it('retries on empty_result up to the cap, then stops', async () => { + tryGenerateSessionTitleMock.mockResolvedValue({ + ok: false, + reason: 'empty_result', + }); + + for (let i = 0; i < 5; i++) { + chatRecordingService.recordAssistantTurn({ + model: 'qwen-plus', + message: [{ text: `turn ${i}` }], + }); + await flushMicrotasks(); + } + + // Cap is 3. + expect(tryGenerateSessionTitleMock).toHaveBeenCalledTimes(3); + expect(findCustomTitleRecord()).toBeUndefined(); + }); + + it('retries across turns after a transient thrown error (up to cap)', async () => { + // A transient error (network blip, 429, bad UTF-16 in one turn's history) + // must NOT permanently disable auto-titling — the next turn should retry. + // The attempt cap bounds total waste. + tryGenerateSessionTitleMock + .mockRejectedValueOnce(new Error('transient')) + .mockResolvedValueOnce({ + ok: true, + title: 'Recovered title', + modelUsed: 'qwen-turbo', + }); + + chatRecordingService.recordAssistantTurn({ + model: 'qwen-plus', + message: [{ text: 'turn 1' }], + }); + await flushMicrotasks(); + chatRecordingService.recordAssistantTurn({ + model: 'qwen-plus', + message: [{ text: 'turn 2' }], + }); + await flushMicrotasks(); + + expect(tryGenerateSessionTitleMock).toHaveBeenCalledTimes(2); + const titleRecord = findCustomTitleRecord(); + expect(titleRecord?.systemPayload).toEqual({ + customTitle: 'Recovered title', + titleSource: 'auto', + }); + }); + + it('does not trigger when QWEN_DISABLE_AUTO_TITLE is set', async () => { + const prev = process.env['QWEN_DISABLE_AUTO_TITLE']; + process.env['QWEN_DISABLE_AUTO_TITLE'] = '1'; + try { + chatRecordingService.recordAssistantTurn({ + model: 'qwen-plus', + message: [{ text: 'reply' }], + }); + await flushMicrotasks(); + expect(tryGenerateSessionTitleMock).not.toHaveBeenCalled(); + expect(findCustomTitleRecord()).toBeUndefined(); + } finally { + if (prev === undefined) delete process.env['QWEN_DISABLE_AUTO_TITLE']; + else process.env['QWEN_DISABLE_AUTO_TITLE'] = prev; + } + }); + + it('still triggers when QWEN_DISABLE_AUTO_TITLE is falsy ("0")', async () => { + mockOk('Fix login button'); + const prev = process.env['QWEN_DISABLE_AUTO_TITLE']; + process.env['QWEN_DISABLE_AUTO_TITLE'] = '0'; + try { + chatRecordingService.recordAssistantTurn({ + model: 'qwen-plus', + message: [{ text: 'reply' }], + }); + await flushMicrotasks(); + expect(tryGenerateSessionTitleMock).toHaveBeenCalledOnce(); + } finally { + if (prev === undefined) delete process.env['QWEN_DISABLE_AUTO_TITLE']; + else process.env['QWEN_DISABLE_AUTO_TITLE'] = prev; + } + }); + + it('does not trigger in non-interactive mode', async () => { + vi.mocked(mockConfig.isInteractive).mockReturnValue(false); + + chatRecordingService.recordAssistantTurn({ + model: 'qwen-plus', + message: [{ text: 'reply' }], + }); + await flushMicrotasks(); + + expect(tryGenerateSessionTitleMock).not.toHaveBeenCalled(); + expect(findCustomTitleRecord()).toBeUndefined(); + }); + + it('prevents concurrent in-flight generations across rapid turns', async () => { + // Generation never resolves (simulates slow LLM); successive turns + // within the same process must NOT start additional generations. + tryGenerateSessionTitleMock.mockImplementation(() => new Promise(() => {})); + + for (let i = 0; i < 5; i++) { + chatRecordingService.recordAssistantTurn({ + model: 'qwen-plus', + message: [{ text: `turn ${i}` }], + }); + await flushMicrotasks(); + } + + // Only the first turn should have launched a generation; subsequent + // turns are blocked because autoTitleController is still set. + expect(tryGenerateSessionTitleMock).toHaveBeenCalledTimes(1); + }); + + it('preserves titleSource across resume (auto stays auto)', async () => { + const mockSessionService = { + getSessionTitleInfo: vi.fn().mockReturnValue({ + title: 'Auto-generated title', + source: 'auto', + }), + getSessionTitle: vi.fn().mockReturnValue('Auto-generated title'), + }; + const resumedConfig = { + ...mockConfig, + getResumedSessionData: vi.fn().mockReturnValue({ + lastCompletedUuid: 'parent-uuid', + }), + getSessionService: vi.fn().mockReturnValue(mockSessionService), + } as unknown as Config; + + const svc = new ChatRecordingService(resumedConfig); + + expect(svc.getCurrentCustomTitle()).toBe('Auto-generated title'); + expect(svc.getCurrentTitleSource()).toBe('auto'); + + // finalize() was called by the constructor — the re-appended record + // must carry titleSource: 'auto', not 'manual'. + const finalizeRecord = vi + .mocked(jsonl.writeLineSync) + .mock.calls.map((c) => c[1] as ChatRecord) + .find((r) => r.type === 'system' && r.subtype === 'custom_title'); + expect(finalizeRecord?.systemPayload).toEqual({ + customTitle: 'Auto-generated title', + titleSource: 'auto', + }); + }); + + it('preserves titleSource across resume (manual stays manual)', async () => { + // Symmetric to the auto-stays-auto case: if a user deliberately ran + // /rename on a session, resuming must NOT rewrite that to auto or to + // anything else. The worst regression path here would silently + // reclassify a user-chosen name as a model guess. + const mockSessionService = { + getSessionTitleInfo: vi.fn().mockReturnValue({ + title: 'User chose this', + source: 'manual', + }), + getSessionTitle: vi.fn().mockReturnValue('User chose this'), + }; + const resumedConfig = { + ...mockConfig, + getResumedSessionData: vi.fn().mockReturnValue({ + lastCompletedUuid: 'parent-uuid', + }), + getSessionService: vi.fn().mockReturnValue(mockSessionService), + } as unknown as Config; + + const svc = new ChatRecordingService(resumedConfig); + + expect(svc.getCurrentCustomTitle()).toBe('User chose this'); + expect(svc.getCurrentTitleSource()).toBe('manual'); + + const finalizeRecord = vi + .mocked(jsonl.writeLineSync) + .mock.calls.map((c) => c[1] as ChatRecord) + .find((r) => r.type === 'system' && r.subtype === 'custom_title'); + expect(finalizeRecord?.systemPayload).toEqual({ + customTitle: 'User chose this', + titleSource: 'manual', + }); + }); + + it('preserves undefined titleSource on legacy resume (no rewrite)', async () => { + const mockSessionService = { + // Legacy record: only title surfaces, no source field. + getSessionTitleInfo: vi.fn().mockReturnValue({ + title: 'Legacy title', + }), + getSessionTitle: vi.fn().mockReturnValue('Legacy title'), + }; + const resumedConfig = { + ...mockConfig, + getResumedSessionData: vi.fn().mockReturnValue({ + lastCompletedUuid: 'parent-uuid', + }), + getSessionService: vi.fn().mockReturnValue(mockSessionService), + } as unknown as Config; + + const svc = new ChatRecordingService(resumedConfig); + + expect(svc.getCurrentCustomTitle()).toBe('Legacy title'); + // Must stay undefined so the JSONL isn't upgraded to a misleading + // `titleSource: 'manual'` we can't actually verify. + expect(svc.getCurrentTitleSource()).toBeUndefined(); + + const finalizeRecord = vi + .mocked(jsonl.writeLineSync) + .mock.calls.map((c) => c[1] as ChatRecord) + .find((r) => r.type === 'system' && r.subtype === 'custom_title'); + // Payload must NOT contain a titleSource field when source is unknown. + expect(finalizeRecord?.systemPayload).toEqual({ + customTitle: 'Legacy title', + }); + }); + + it('does not overwrite a manual title written by another process', async () => { + // Cross-process race: this CRS instance doesn't know about a /rename + // issued from another CLI tab, but the persisted JSONL does. Before + // writing an auto title we must re-read and bail if the file already + // has source='manual'. + mockOk('Auto guess'); + const otherProcessManual = vi.fn().mockReturnValue({ + title: 'User chose this', + source: 'manual', + }); + vi.mocked(mockConfig.getSessionService).mockReturnValue({ + getSessionTitleInfo: otherProcessManual, + getSessionTitle: vi.fn(), + } as never); + + chatRecordingService.recordAssistantTurn({ + model: 'qwen-plus', + message: [{ text: 'reply' }], + }); + await flushMicrotasks(); + + expect(tryGenerateSessionTitleMock).toHaveBeenCalledOnce(); + expect(otherProcessManual).toHaveBeenCalled(); + // No auto record was appended. + expect(findCustomTitleRecord()).toBeUndefined(); + // In-memory state synced to the on-disk manual title so later turns + // also skip the trigger. + expect(chatRecordingService.getCurrentCustomTitle()).toBe( + 'User chose this', + ); + expect(chatRecordingService.getCurrentTitleSource()).toBe('manual'); + }); + + it('aborts the in-flight generation on finalize and suppresses the title write', async () => { + // Model rejects when the signal fires — mirrors what a real provider's + // fetch layer does when the AbortController aborts. Previously this + // test only checked that `signal.aborted` flipped; but what we actually + // care about is that NO custom_title record gets written after abort. + let capturedSignal: AbortSignal | undefined; + tryGenerateSessionTitleMock.mockImplementation( + (_config: unknown, signal: AbortSignal) => + new Promise((_resolve, reject) => { + capturedSignal = signal; + signal.addEventListener('abort', () => { + reject(new Error('aborted')); + }); + }), + ); + + chatRecordingService.recordAssistantTurn({ + model: 'qwen-plus', + message: [{ text: 'turn' }], + }); + await flushMicrotasks(); + + expect(capturedSignal).toBeDefined(); + expect(capturedSignal?.aborted).toBe(false); + // No title yet — generation is still pending. + expect(findCustomTitleRecord()).toBeUndefined(); + + chatRecordingService.finalize(); + expect(capturedSignal?.aborted).toBe(true); + + await flushMicrotasks(); + // The aborted generation must NOT result in a custom_title record — + // even though the mock technically "completed" (via rejection). + expect(findCustomTitleRecord()).toBeUndefined(); + expect(chatRecordingService.getCurrentCustomTitle()).toBeUndefined(); + }); + + it('respects a late /rename that lands while the LLM call is in flight', async () => { + // Simulate slow LLM: resolves after a manual rename lands. + let resolveLlm: (v: unknown) => void = () => {}; + tryGenerateSessionTitleMock.mockImplementationOnce( + () => + new Promise((resolve) => { + resolveLlm = resolve; + }), + ); + + chatRecordingService.recordAssistantTurn({ + model: 'qwen-plus', + message: [{ text: 'turn' }], + }); + await flushMicrotasks(); + + // User renames while the title LLM call is still pending. + chatRecordingService.recordCustomTitle('user-chosen', 'manual'); + vi.mocked(jsonl.writeLineSync).mockClear(); + + // Now the LLM call returns a title. + resolveLlm({ ok: true, title: 'Auto Title', modelUsed: 'qwen-turbo' }); + await flushMicrotasks(); + + // No auto-title record should have been written. + expect(findCustomTitleRecord()).toBeUndefined(); + expect(chatRecordingService.getCurrentCustomTitle()).toBe('user-chosen'); + expect(chatRecordingService.getCurrentTitleSource()).toBe('manual'); + }); +}); diff --git a/packages/core/src/services/chatRecordingService.customTitle.test.ts b/packages/core/src/services/chatRecordingService.customTitle.test.ts index 479aa58c7..0842ca635 100644 --- a/packages/core/src/services/chatRecordingService.customTitle.test.ts +++ b/packages/core/src/services/chatRecordingService.customTitle.test.ts @@ -50,6 +50,8 @@ describe('ChatRecordingService - recordCustomTitle', () => { .mockReturnValue('/test/project/root/.qwen/projects/test-project'), }, getModel: vi.fn().mockReturnValue('qwen-plus'), + getFastModel: vi.fn().mockReturnValue(undefined), + isInteractive: vi.fn().mockReturnValue(false), getDebugMode: vi.fn().mockReturnValue(false), getToolRegistry: vi.fn().mockReturnValue({ getTool: vi.fn().mockReturnValue({ @@ -94,6 +96,7 @@ describe('ChatRecordingService - recordCustomTitle', () => { expect(writtenRecord.subtype).toBe('custom_title'); expect(writtenRecord.systemPayload).toEqual({ customTitle: 'my-feature', + titleSource: 'manual', }); expect(writtenRecord.sessionId).toBe('test-session-id'); }); @@ -137,7 +140,10 @@ describe('ChatRecordingService - recordCustomTitle', () => { .calls[0][1] as ChatRecord; expect(record.type).toBe('system'); expect(record.subtype).toBe('custom_title'); - expect(record.systemPayload).toEqual({ customTitle: 'my-feature' }); + expect(record.systemPayload).toEqual({ + customTitle: 'my-feature', + titleSource: 'manual', + }); }); it('should not write anything when no custom title was set', () => { @@ -156,7 +162,10 @@ describe('ChatRecordingService - recordCustomTitle', () => { expect(jsonl.writeLineSync).toHaveBeenCalledOnce(); const record = vi.mocked(jsonl.writeLineSync).mock .calls[0][1] as ChatRecord; - expect(record.systemPayload).toEqual({ customTitle: 'second-name' }); + expect(record.systemPayload).toEqual({ + customTitle: 'second-name', + titleSource: 'manual', + }); }); }); }); diff --git a/packages/core/src/services/chatRecordingService.test.ts b/packages/core/src/services/chatRecordingService.test.ts index fff565198..6307c6d67 100644 --- a/packages/core/src/services/chatRecordingService.test.ts +++ b/packages/core/src/services/chatRecordingService.test.ts @@ -52,6 +52,8 @@ describe('ChatRecordingService', () => { .mockReturnValue('/test/project/root/.gemini/projects/test-project'), }, getModel: vi.fn().mockReturnValue('gemini-pro'), + getFastModel: vi.fn().mockReturnValue(undefined), + isInteractive: vi.fn().mockReturnValue(false), getDebugMode: vi.fn().mockReturnValue(false), getToolRegistry: vi.fn().mockReturnValue({ getTool: vi.fn().mockReturnValue({ diff --git a/packages/core/src/services/chatRecordingService.ts b/packages/core/src/services/chatRecordingService.ts index 0565c7ad2..c833ae367 100644 --- a/packages/core/src/services/chatRecordingService.ts +++ b/packages/core/src/services/chatRecordingService.ts @@ -18,8 +18,7 @@ import { import * as jsonl from '../utils/jsonl-utils.js'; import { getGitBranch } from '../utils/gitUtils.js'; import { createDebugLogger } from '../utils/debugLogger.js'; - -const debugLogger = createDebugLogger('CHAT_RECORDING'); +import { tryGenerateSessionTitle } from './sessionTitle.js'; import type { ChatCompressionInfo, ToolCallResponseInfo, @@ -28,6 +27,38 @@ import type { Status } from '../core/coreToolScheduler.js'; import type { AgentResultDisplay } from '../tools/tools.js'; import type { UiEvent } from '../telemetry/uiTelemetry.js'; +const debugLogger = createDebugLogger('CHAT_RECORDING'); + +/** + * Maximum number of auto-title generation attempts per session. See + * {@link ChatRecordingService.autoTitleAttempts} for the rationale behind + * retrying across turns. + */ +const AUTO_TITLE_ATTEMPT_CAP = 3; + +/** + * Users who don't want the fast model silently generating titles can opt + * out at runtime: `QWEN_DISABLE_AUTO_TITLE=1` (or any truthy-ish value) + * makes {@link ChatRecordingService.maybeTriggerAutoTitle} a no-op without + * touching the rest of the feature (so `/rename --auto` still works on + * explicit user request). Read per-call rather than cached so tests can + * flip the var between cases without reloading the module; the cost of + * one env lookup per assistant turn is irrelevant next to an LLM call. + */ +function autoTitleDisabledByEnv(): boolean { + const v = process.env['QWEN_DISABLE_AUTO_TITLE']; + if (!v) return false; + // Accept "0", "false", "no", "off" (case-insensitive) as "not disabled". + const lowered = v.trim().toLowerCase(); + return ( + lowered !== '' && + lowered !== '0' && + lowered !== 'false' && + lowered !== 'no' && + lowered !== 'off' + ); +} + /** * A single record stored in the JSONL file. * Forms a tree structure via uuid/parentUuid for future checkpointing support. @@ -151,11 +182,27 @@ export interface AtCommandRecordPayload { } /** - * Stored payload for custom title set via /rename. + * Source of a custom session title. + * - `manual`: set by the user via `/rename` (or pre-2026 records without + * a source field — treated as manual for safety so auto can't overwrite + * a title a user deliberately chose). + * - `auto`: generated by the session-title service from conversation text; + * safe to re-generate or be replaced by a manual rename. + */ +export type TitleSource = 'manual' | 'auto'; + +/** + * Stored payload for custom title set via /rename or auto-generation. */ export interface CustomTitleRecordPayload { /** The custom title for the session */ customTitle: string; + /** + * How this title was produced. Absent on legacy records — readers should + * treat `undefined` as `'manual'` so existing user-set titles are never + * replaced by auto-generation after an upgrade. + */ + titleSource?: TitleSource; } /** @@ -195,23 +242,55 @@ export class ChatRecordingService { private readonly config: Config; /** In-memory cache of the current session's custom title (for re-append on exit) */ private currentCustomTitle: string | undefined; + /** + * Source of {@link currentCustomTitle}. `undefined` on legacy records that + * pre-date the `titleSource` field — that's treated as manual everywhere + * (safe default) without rewriting the persisted record. + */ + private currentTitleSource: TitleSource | undefined; + /** + * How many auto-title attempts have been made this process. + * + * We don't commit to "one attempt per session" because the first assistant + * turn may be a pure tool-call with no user-visible text (e.g., the model + * opens with a search) — the title service returns null, and we'd waste + * the whole session's chance on a turn that never had a shot. Instead we + * retry for a handful of turns until either the title lands or we hit the + * cap, which protects against a persistently failing fast-model looping + * on every turn. {@link AUTO_TITLE_ATTEMPT_CAP} sets the ceiling. + */ + private autoTitleAttempts = 0; + /** + * AbortController for the in-flight auto-title LLM call, or `undefined` + * when no generation is pending. Doubles as the in-flight guard — a + * defined controller means "one is running; don't launch another". + * Stored on the instance so {@link finalize} (called on session switch + * and shutdown) can cancel a pending call cleanly rather than letting + * it burn tokens after the session has already moved on. + */ + private autoTitleController: AbortController | undefined; constructor(config: Config) { this.config = config; this.lastRecordUuid = config.getResumedSessionData()?.lastCompletedUuid ?? null; - // On resume, load the cached custom title from the session file and - // immediately re-append it to EOF. This keeps the title within the - // 64KB tail window even as new messages push it deeper into the file. - // Without this, a crash mid-session could lose the title if the exit - // re-append never runs. + // On resume, load the cached custom title AND its source from the + // session file. Preserving the persisted source is load-bearing: the + // SessionPicker dim-styling depends on it, and hardcoding `'manual'` + // would silently downgrade auto-titled sessions every time they get + // resumed. Legacy records (no `titleSource` field) stay `undefined` — + // treated as manual for safety without rewriting the JSONL. + // + // We then re-append a custom_title record to EOF so the title stays + // within the tail window that readers scan (guarding against a crash + // before the next finalize). if (config.getResumedSessionData()) { try { const sessionService = config.getSessionService(); - this.currentCustomTitle = sessionService.getSessionTitle( - config.getSessionId(), - ); + const info = sessionService.getSessionTitleInfo(config.getSessionId()); + this.currentCustomTitle = info.title; + this.currentTitleSource = info.source; this.finalize(); } catch { // Best-effort — don't block construction @@ -219,6 +298,23 @@ export class ChatRecordingService { } } + /** + * Returns the current custom title, if any. Read-only accessor for + * callers (e.g. auto-title trigger) that need to know whether a title is + * already set before attempting generation. + */ + getCurrentCustomTitle(): string | undefined { + return this.currentCustomTitle; + } + + /** + * Returns the source of the current custom title, or `undefined` when no + * title is set. + */ + getCurrentTitleSource(): TitleSource | undefined { + return this.currentTitleSource; + } + /** * Returns the session ID. * @returns The session ID. @@ -403,11 +499,97 @@ export class ChatRecordingService { } this.appendRecord(record); + this.maybeTriggerAutoTitle(); } catch (error) { debugLogger.error('Error saving assistant turn:', error); } } + /** + * Fire-and-forget: after an assistant turn is recorded, attempt to generate + * a short session title from the conversation so far. Runs at most once per + * process lifetime per session and only when: + * + * - No title is already set (auto must never overwrite a manual rename, + * and we don't need to regenerate an existing auto title mid-session). + * - A fast model is configured — the service itself also guards this, + * but checking here avoids paying for the import/history load when + * there's no point. + * + * Errors are swallowed. The title is best-effort and must never surface + * as a user-visible error or interrupt recording. + */ + private maybeTriggerAutoTitle(): void { + if (this.currentCustomTitle) return; + if (this.autoTitleController) return; + if (this.autoTitleAttempts >= AUTO_TITLE_ATTEMPT_CAP) return; + // Opt-out env var — lets users silence auto-titling without having to + // unset their fast model (which would break `/rename --auto`, recap, + // compression, and other fast-model features). + if (autoTitleDisabledByEnv()) return; + // Headless/one-shot CLI flows (`qwen -p "…"`, cron, CI scripts) run a + // single prompt and throw the session away. Spending fast-model tokens + // on a title no one will ever resume is pure waste; skip entirely. + // Checked before `getFastModel()` because it's strictly cheaper (a bool + // field read vs. a method that looks up available models for the auth + // type). + if (!this.config.isInteractive()) return; + if (!this.config.getFastModel()) return; + + this.autoTitleAttempts++; + const controller = new AbortController(); + this.autoTitleController = controller; + + void (async () => { + try { + const outcome = await tryGenerateSessionTitle( + this.config, + controller.signal, + ); + if (!outcome.ok) return; + if (controller.signal.aborted) return; + // Re-check in case a /rename landed while the LLM call was in flight — + // manual wins. In-process is the common path. + if (this.currentTitleSource === 'manual') return; + // Cross-process guard: another CLI tab writing to the same JSONL + // could have renamed (manually) since we started. Re-read the file's + // latest title record before we append so we don't clobber it. + // Cost is one 64KB tail read; happens once per successful generation. + try { + const sessionService = this.config.getSessionService(); + const onDisk = sessionService.getSessionTitleInfo( + this.config.getSessionId(), + ); + if (onDisk.source === 'manual') { + // Sync in-memory state with what landed on disk so subsequent + // turns don't retry against a stale cache. + this.currentCustomTitle = onDisk.title; + this.currentTitleSource = 'manual'; + return; + } + } catch { + // Best-effort — if the re-read fails for any reason, fall through + // to the in-process check (which already passed) and proceed. + } + this.recordCustomTitle(outcome.title, 'auto'); + } catch (err) { + // Don't permanently disable: transient failures (network blips, rate + // limits, bad UTF-16 in one turn's history) should still allow a + // later turn to retry. The attempt cap bounds total waste. + debugLogger.warn( + `Auto-title generation failed: ${err instanceof Error ? err.message : String(err)}`, + ); + } finally { + // Clear only if we're still the active controller — `finalize()` + // may have swapped to a new one during a subsequent session, and + // we shouldn't overwrite that. + if (this.autoTitleController === controller) { + this.autoTitleController = undefined; + } + } + })(); + } + /** * Records tool results (function responses) sent back to the model. * Writes immediately to disk. @@ -511,23 +693,30 @@ export class ChatRecordingService { } /** - * Records a custom title for the session (set via /rename). + * Records a custom title for the session. * Appended as a system record so it persists with the session data. * Also caches the title in memory for re-append on shutdown. * + * @param customTitle The title text. + * @param titleSource Where the title came from — defaults to `'manual'` + * so existing `/rename` call sites keep their behavior unchanged. * @returns true if the record was written successfully, false on I/O error. */ - recordCustomTitle(customTitle: string): boolean { + recordCustomTitle( + customTitle: string, + titleSource: TitleSource = 'manual', + ): boolean { try { const record: ChatRecord = { ...this.createBaseRecord('system'), type: 'system', subtype: 'custom_title', - systemPayload: { customTitle }, + systemPayload: { customTitle, titleSource }, }; this.appendRecord(record); this.currentCustomTitle = customTitle; + this.currentTitleSource = titleSource; return true; } catch (error) { debugLogger.error('Error saving custom title record:', error); @@ -547,6 +736,17 @@ export class ChatRecordingService { * Best-effort: errors are logged but never thrown. */ finalize(): void { + // Cancel any pending auto-title LLM call — the session is transitioning + // (switch / shutdown) and the result is no longer useful. Without this, + // a slow fast-model call could keep a socket open past the logical end + // of the session. + if (this.autoTitleController) { + try { + this.autoTitleController.abort(); + } catch { + // best-effort + } + } if (!this.currentCustomTitle) { return; } @@ -555,7 +755,12 @@ export class ChatRecordingService { ...this.createBaseRecord('system'), type: 'system', subtype: 'custom_title', - systemPayload: { customTitle: this.currentCustomTitle }, + systemPayload: { + customTitle: this.currentCustomTitle, + ...(this.currentTitleSource + ? { titleSource: this.currentTitleSource } + : {}), + }, }; this.appendRecord(record); } catch (error) { diff --git a/packages/core/src/services/sessionService.rename.test.ts b/packages/core/src/services/sessionService.rename.test.ts index 5ba227cee..87d2e5ce7 100644 --- a/packages/core/src/services/sessionService.rename.test.ts +++ b/packages/core/src/services/sessionService.rename.test.ts @@ -109,6 +109,7 @@ describe('SessionService - rename and custom title', () => { expect(writtenRecord.subtype).toBe('custom_title'); expect(writtenRecord.systemPayload).toEqual({ customTitle: 'my-feature', + titleSource: 'manual', }); expect(writtenRecord.sessionId).toBe(sessionIdA); }); @@ -501,6 +502,85 @@ describe('SessionService - rename and custom title', () => { expect(result.items).toHaveLength(1); expect(result.items[0].customTitle).toBeUndefined(); + expect(result.items[0].titleSource).toBeUndefined(); + }); + + it('should surface titleSource on session list items', async () => { + const now = Date.now(); + const titleContent = + JSON.stringify({ + type: 'system', + subtype: 'custom_title', + systemPayload: { + customTitle: 'Fix login bug', + titleSource: 'auto', + }, + }) + '\n'; + + readdirSyncSpy.mockReturnValue([ + `${sessionIdA}.jsonl`, + ] as unknown as Array>); + + statSyncSpy.mockReturnValue({ + mtimeMs: now, + size: titleContent.length, + isFile: () => true, + } as unknown as fs.Stats); + + vi.mocked(jsonl.readLines).mockResolvedValue([recordA1]); + + readSyncSpy.mockImplementation( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (_fd: number, buffer: any) => { + const data = Buffer.from(titleContent); + data.copy(buffer); + return data.length; + }, + ); + + const result = await sessionService.listSessions(); + + expect(result.items[0].customTitle).toBe('Fix login bug'); + expect(result.items[0].titleSource).toBe('auto'); + }); + + it('leaves titleSource undefined for legacy records without the field', async () => { + // Back-compat: old sessions written before titleSource existed should + // be treated as manual (via `undefined` — consumers check `=== 'auto'`), + // so auto-generation never dims a title the user chose pre-upgrade. + const now = Date.now(); + const titleContent = + JSON.stringify({ + type: 'system', + subtype: 'custom_title', + systemPayload: { customTitle: 'legacy-title' }, + }) + '\n'; + + readdirSyncSpy.mockReturnValue([ + `${sessionIdA}.jsonl`, + ] as unknown as Array>); + + statSyncSpy.mockReturnValue({ + mtimeMs: now, + size: titleContent.length, + isFile: () => true, + } as unknown as fs.Stats); + + vi.mocked(jsonl.readLines).mockResolvedValue([recordA1]); + + readSyncSpy.mockImplementation( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (_fd: number, buffer: any) => { + const data = Buffer.from(titleContent); + data.copy(buffer); + return data.length; + }, + ); + + const result = await sessionService.listSessions(); + + expect(result.items[0].customTitle).toBe('legacy-title'); + expect(result.items[0].titleSource).toBeUndefined(); }); }); }); diff --git a/packages/core/src/services/sessionService.ts b/packages/core/src/services/sessionService.ts index 19b7af1f5..a067862b0 100644 --- a/packages/core/src/services/sessionService.ts +++ b/packages/core/src/services/sessionService.ts @@ -15,11 +15,15 @@ import * as jsonl from '../utils/jsonl-utils.js'; import type { ChatCompressionRecordPayload, ChatRecord, + TitleSource, UiTelemetryRecordPayload, } from './chatRecordingService.js'; import { uiTelemetryService } from '../telemetry/uiTelemetry.js'; import { createDebugLogger } from '../utils/debugLogger.js'; -import { readLastJsonStringFieldSync } from '../utils/sessionStorageUtils.js'; +import { + readLastJsonStringFieldSync, + readLastJsonStringFieldsSync, +} from '../utils/sessionStorageUtils.js'; const debugLogger = createDebugLogger('SESSION'); @@ -44,8 +48,16 @@ export interface SessionListItem { filePath: string; /** Number of messages in the session (unique message UUIDs) */ messageCount: number; - /** Custom title set via /rename, if any */ + /** Custom title set via /rename or auto-generated by the title service, if any */ customTitle?: string; + /** + * Source of {@link customTitle}. `undefined` on legacy records without a + * source field — consumers should treat `undefined` and `'manual'` the + * same way for display (full-contrast). UI affordances like dimming are + * reserved for `'auto'` so users can tell a model guess from a name they + * chose. + */ + titleSource?: TitleSource; } /** @@ -164,7 +176,62 @@ export class SessionService { * field. */ private readSessionTitleFromFile(filePath: string): string | undefined { - return readLastJsonStringFieldSync(filePath, 'customTitle', 'custom_title'); + // Match only on actual custom_title system records. `'custom_title'` as + // a loose substring can land on a user message that happens to contain + // the literal "custom_title" (code review of this very file, etc.); + // requiring the full `"subtype":"custom_title"` pattern guarantees the + // match is on a system record written by {@link writeLineSync}, which + // JSON.stringifies records in a predictable compact form. + return readLastJsonStringFieldSync( + filePath, + 'customTitle', + '"subtype":"custom_title"', + ); + } + + /** + * Reads both the custom title and its source from a session file in a + * single pass — the helper extracts both fields from the same matching + * `custom_title` line, so the pair is always consistent (never one field + * from an old record and another from a new one). + * + * `titleSource` is absent on legacy records written before the field was + * introduced — callers treat `undefined` as equivalent to `'manual'` so a + * user's pre-upgrade rename is never displayed as if it were auto-generated. + */ + private readSessionTitleInfoFromFile(filePath: string): { + title?: string; + source?: TitleSource; + } { + const hit = readLastJsonStringFieldsSync( + filePath, + 'customTitle', + ['titleSource'], + '"subtype":"custom_title"', + ); + const title = hit['customTitle']; + if (!title) return {}; + const rawSource = hit['titleSource']; + const source = + rawSource === 'auto' || rawSource === 'manual' ? rawSource : undefined; + return { title, source }; + } + + /** + * Public accessor: returns both the current custom title and its source + * for a given session. Used by `ChatRecordingService` on resume to + * preserve the persisted `titleSource` rather than defaulting to manual. + */ + getSessionTitleInfo(sessionId: string): { + title?: string; + source?: TitleSource; + } { + if (!SESSION_FILE_PATTERN.test(`${sessionId}.jsonl`)) { + return {}; + } + const chatsDir = this.getChatsDir(); + const filePath = path.join(chatsDir, `${sessionId}.jsonl`); + return this.readSessionTitleInfoFromFile(filePath); } /** @@ -365,6 +432,7 @@ export class SessionService { const prompt = this.extractFirstPromptFromRecords(records); + const titleInfo = this.readSessionTitleInfoFromFile(filePath); items.push({ sessionId: firstRecord.sessionId, cwd: firstRecord.cwd, @@ -374,7 +442,8 @@ export class SessionService { gitBranch: firstRecord.gitBranch, filePath, messageCount, - customTitle: this.readSessionTitleFromFile(filePath), + customTitle: titleInfo.title, + titleSource: titleInfo.source, }); } @@ -589,9 +658,16 @@ export class SessionService { * * @param sessionId The session ID to rename * @param title The new custom title + * @param titleSource Where the title came from. Defaults to `'manual'` so + * existing callers are unchanged — pass `'auto'` only for titles produced + * by the auto-title generator. * @returns true if renamed successfully, false if session not found */ - async renameSession(sessionId: string, title: string): Promise { + async renameSession( + sessionId: string, + title: string, + titleSource: TitleSource = 'manual', + ): Promise { if (!SESSION_FILE_PATTERN.test(`${sessionId}.jsonl`)) { return false; } @@ -616,7 +692,11 @@ export class SessionService { // chain and cause the session to appear empty on next load. const lastUuid = this.readLastRecordUuid(filePath); - // Append a custom_title system record + // Append a custom_title system record. `renameSession` is the + // fallback path when no live recording service is attached (e.g., from + // the WebUI or VSCode extension). Callers pass `titleSource='auto'` + // only when the title came from the auto-generator; defaults to + // 'manual' for explicit user renames. const titleRecord: ChatRecord = { uuid: randomUUID(), parentUuid: lastUuid, @@ -626,7 +706,7 @@ export class SessionService { subtype: 'custom_title', cwd: records[0].cwd, version: records[0].version, - systemPayload: { customTitle: title }, + systemPayload: { customTitle: title, titleSource }, }; jsonl.writeLineSync(filePath, titleRecord); return true; @@ -705,8 +785,8 @@ export class SessionService { // Cheap check first: tail-read the title and skip non-matches before // doing the full hydration work (first-record read, project filter, // message count, prompt extraction). - const customTitle = this.readSessionTitleFromFile(filePath); - if (customTitle?.toLowerCase().trim() !== normalizedTitle) continue; + const titleInfo = this.readSessionTitleInfoFromFile(filePath); + if (titleInfo.title?.toLowerCase().trim() !== normalizedTitle) continue; const records = await jsonl.readLines( filePath, @@ -730,7 +810,8 @@ export class SessionService { gitBranch: firstRecord.gitBranch, filePath, messageCount, - customTitle, + customTitle: titleInfo.title, + titleSource: titleInfo.source, }); } diff --git a/packages/core/src/services/sessionTitle.test.ts b/packages/core/src/services/sessionTitle.test.ts new file mode 100644 index 000000000..05bb83b9d --- /dev/null +++ b/packages/core/src/services/sessionTitle.test.ts @@ -0,0 +1,303 @@ +/** + * @license + * Copyright 2025 Qwen Code + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, expect, it, vi } from 'vitest'; +import type { Content } from '@google/genai'; +import type { Config } from '../config/config.js'; +import { sanitizeTitle, tryGenerateSessionTitle } from './sessionTitle.js'; + +interface MockOptions { + fastModel?: string | undefined; + history?: Content[]; + generateJsonResult?: + | Record + | ((...args: unknown[]) => Promise>); +} + +function makeConfig(opts: MockOptions): { + config: Config; + generateJson: ReturnType; +} { + const generateJson = vi.fn(async (...args: unknown[]) => { + const r = opts.generateJsonResult; + if (!r) throw new Error('no generateJsonResult configured'); + return typeof r === 'function' ? r(...args) : r; + }); + + const config = { + getFastModel: vi.fn(() => opts.fastModel ?? undefined), + getModel: vi.fn(() => 'qwen-plus'), + getGeminiClient: vi.fn(() => ({ + getChat: () => ({ + getHistory: () => opts.history ?? [], + }), + })), + getBaseLlmClient: vi.fn(() => ({ generateJson })), + } as unknown as Config; + + return { config, generateJson }; +} + +const DIALOG_HISTORY: Content[] = [ + { role: 'user', parts: [{ text: 'my login button is broken on mobile' }] }, + { + role: 'model', + parts: [{ text: "Let's look at the button handler and the viewport CSS." }], + }, +]; + +describe('tryGenerateSessionTitle', () => { + it('returns {ok:false, reason:"no_fast_model"} when fast model is absent', async () => { + const { config } = makeConfig({ + fastModel: undefined, + history: DIALOG_HISTORY, + }); + const outcome = await tryGenerateSessionTitle( + config, + new AbortController().signal, + ); + expect(outcome).toEqual({ ok: false, reason: 'no_fast_model' }); + }); + + it('returns {ok:false, reason:"empty_history"} for a fresh session', async () => { + const { config } = makeConfig({ + fastModel: 'qwen-turbo', + history: [], + }); + const outcome = await tryGenerateSessionTitle( + config, + new AbortController().signal, + ); + expect(outcome).toEqual({ ok: false, reason: 'empty_history' }); + }); + + it('returns {ok:false, reason:"model_error"} when the LLM throws', async () => { + const { config } = makeConfig({ + fastModel: 'qwen-turbo', + history: DIALOG_HISTORY, + generateJsonResult: () => Promise.reject(new Error('API down')), + }); + const outcome = await tryGenerateSessionTitle( + config, + new AbortController().signal, + ); + expect(outcome).toEqual({ ok: false, reason: 'model_error' }); + }); + + it('returns {ok:false, reason:"aborted"} when the user cancels', async () => { + const controller = new AbortController(); + const { config } = makeConfig({ + fastModel: 'qwen-turbo', + history: DIALOG_HISTORY, + generateJsonResult: async () => { + controller.abort(); + throw new Error('aborted'); + }, + }); + const outcome = await tryGenerateSessionTitle(config, controller.signal); + expect(outcome).toEqual({ ok: false, reason: 'aborted' }); + }); + + it('returns {ok:false, reason:"empty_result"} when the model returns junk', async () => { + const { config } = makeConfig({ + fastModel: 'qwen-turbo', + history: DIALOG_HISTORY, + generateJsonResult: { title: ' ... ' }, + }); + const outcome = await tryGenerateSessionTitle( + config, + new AbortController().signal, + ); + expect(outcome).toEqual({ ok: false, reason: 'empty_result' }); + }); + + it('returns {ok:true, title, modelUsed} on success', async () => { + const { config, generateJson } = makeConfig({ + fastModel: 'qwen-turbo', + history: DIALOG_HISTORY, + generateJsonResult: { title: 'Fix login button on mobile' }, + }); + const outcome = await tryGenerateSessionTitle( + config, + new AbortController().signal, + ); + expect(outcome).toEqual({ + ok: true, + title: 'Fix login button on mobile', + modelUsed: 'qwen-turbo', + }); + // Schema call must use the fast model (not the main model) and the + // canonical title schema with required:['title'] and maxAttempts:1. + expect(generateJson).toHaveBeenCalledOnce(); + const callOpts = generateJson.mock.calls[0][0] as { + model: string; + schema: { + type: string; + required: string[]; + properties: { title: { type: string } }; + }; + maxAttempts: number; + }; + expect(callOpts.model).toBe('qwen-turbo'); + expect(callOpts.schema.type).toBe('object'); + expect(callOpts.schema.required).toEqual(['title']); + expect(callOpts.schema.properties.title.type).toBe('string'); + expect(callOpts.maxAttempts).toBe(1); + }); + + it('sanitizes residual markdown and trailing punctuation from the model result', async () => { + const { config } = makeConfig({ + fastModel: 'qwen-turbo', + history: DIALOG_HISTORY, + generateJsonResult: { title: '**Fix login button.**' }, + }); + const outcome = await tryGenerateSessionTitle( + config, + new AbortController().signal, + ); + expect(outcome).toMatchObject({ ok: true, title: 'Fix login button' }); + }); + + it('filters tool-call and tool-result turns from the prompt', async () => { + // Users' tool invocations can carry 10K-token payloads (file dumps, grep + // output). Those must never reach the title LLM — both for cost and + // because they dilute the "what is this session about" signal. + const history: Content[] = [ + { role: 'user', parts: [{ text: 'scan the auth module' }] }, + { + role: 'model', + parts: [ + { text: 'Scanning…' }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + { functionCall: { name: 'grep', args: { q: 'auth' } } } as any, + ], + }, + { + role: 'user', + parts: [ + { + functionResponse: { + name: 'grep', + response: { output: 'TEN_THOUSAND_TOKENS_OF_FILE_DUMP' }, + }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any, + ], + }, + { + role: 'model', + parts: [{ text: 'The middleware stores tokens unsafely.' }], + }, + ]; + + let capturedContents: Content[] | null = null; + const generateJson = vi.fn(async (opts: { contents: Content[] }) => { + capturedContents = opts.contents; + return { title: 'Audit auth middleware' }; + }); + const config = { + getFastModel: vi.fn(() => 'qwen-turbo'), + getModel: vi.fn(() => 'qwen-plus'), + getGeminiClient: vi.fn(() => ({ + getChat: () => ({ getHistory: () => history }), + })), + getBaseLlmClient: vi.fn(() => ({ generateJson })), + } as unknown as Config; + + const outcome = await tryGenerateSessionTitle( + config, + new AbortController().signal, + ); + expect(outcome).toMatchObject({ ok: true, title: 'Audit auth middleware' }); + + // The tool-response payload must NOT have leaked into the prompt. + expect(capturedContents).not.toBeNull(); + const serialized = JSON.stringify(capturedContents); + expect(serialized).not.toContain('TEN_THOUSAND_TOKENS_OF_FILE_DUMP'); + expect(serialized).toContain('scan the auth module'); + expect(serialized).toContain('middleware stores tokens unsafely'); + }); + + it('tail-slices conversations longer than 1000 characters', async () => { + // A session that pivots mid-conversation — the final topic is what the + // title should reflect. Feeding the head risks titling the session by + // what the user opened with rather than what they ended up doing. + const longUserText = 'BEGIN_HEAD ' + 'x'.repeat(3000) + ' END_TAIL_MARKER'; + const history: Content[] = [ + { role: 'user', parts: [{ text: longUserText }] }, + { role: 'model', parts: [{ text: 'END_MODEL_MARKER' }] }, + ]; + + let captured: string | null = null; + const generateJson = vi.fn(async (opts: { contents: Content[] }) => { + captured = opts.contents[0]?.parts?.[0]?.text ?? ''; + return { title: 'Long session' }; + }); + const config = { + getFastModel: vi.fn(() => 'qwen-turbo'), + getModel: vi.fn(() => 'qwen-plus'), + getGeminiClient: vi.fn(() => ({ + getChat: () => ({ getHistory: () => history }), + })), + getBaseLlmClient: vi.fn(() => ({ generateJson })), + } as unknown as Config; + + await tryGenerateSessionTitle(config, new AbortController().signal); + + expect(captured).not.toBeNull(); + expect(captured).toContain('END_MODEL_MARKER'); + expect(captured).not.toContain('BEGIN_HEAD'); + }); +}); + +describe('sanitizeTitle', () => { + it('strips leading and trailing markdown markers', () => { + expect(sanitizeTitle('> **Fix login button**')).toBe('Fix login button'); + expect(sanitizeTitle('- Fix login button')).toBe('Fix login button'); + expect(sanitizeTitle('`Fix login button`')).toBe('Fix login button'); + }); + + it('strips trailing punctuation in ASCII and CJK', () => { + expect(sanitizeTitle('Fix login button.')).toBe('Fix login button'); + expect(sanitizeTitle('修复登录按钮。')).toBe('修复登录按钮'); + expect(sanitizeTitle('修复登录按钮，')).toBe('修复登录按钮'); + }); + + it('normalizes internal whitespace', () => { + expect(sanitizeTitle('Fix login button')).toBe('Fix login button'); + }); + + it('returns empty for noise-only input', () => { + expect(sanitizeTitle('')).toBe(''); + expect(sanitizeTitle(' \n ')).toBe(''); + expect(sanitizeTitle('...')).toBe(''); + expect(sanitizeTitle('**')).toBe(''); + }); + + it('strips terminal escape sequences (ANSI, OSC-8, BEL)', () => { + // SECURITY: title renders directly to terminal; escapes must not survive. + expect(sanitizeTitle('\x1b[2J\x1b[HHello world')).toBe('Hello world'); + expect(sanitizeTitle('before\x07after')).toBe('before after'); + // OSC-8 hyperlink injection — opens a clickable link in supporting terminals. + expect(sanitizeTitle('\x1b]8;;http://evil\x1b\\click\x1b]8;;\x1b\\')).toBe( + 'click', + ); + // Null byte in value would otherwise corrupt the JSONL writer on some runtimes. + expect(sanitizeTitle('a\x00b')).toBe('a b'); + }); + + it('drops orphaned surrogates after max-length truncation', () => { + // Build a title that lands a surrogate pair exactly at the truncation boundary. + const base = 'x'.repeat(199); + // `"😀"` is a single emoji (two UTF-16 code units). After + // slice(0, 200) we'd keep only the high surrogate. + const title = base + '😀!'; + const sanitized = sanitizeTitle(title); + // High surrogate must not linger on its own. + expect(sanitized).not.toMatch(/[\uD800-\uDBFF](?![\uDC00-\uDFFF])/); + expect(sanitized.length).toBeLessThanOrEqual(200); + }); +}); diff --git a/packages/core/src/services/sessionTitle.ts b/packages/core/src/services/sessionTitle.ts new file mode 100644 index 000000000..b0fc6ab2c --- /dev/null +++ b/packages/core/src/services/sessionTitle.ts @@ -0,0 +1,274 @@ +/** + * @license + * Copyright 2025 Qwen Code + * SPDX-License-Identifier: Apache-2.0 + */ + +import type { Content } from '@google/genai'; +import type { Config } from '../config/config.js'; +import { createDebugLogger } from '../utils/debugLogger.js'; +import { stripTerminalControlSequences } from '../utils/terminalSafe.js'; +import { SESSION_TITLE_MAX_LENGTH } from './sessionService.js'; + +const debugLogger = createDebugLogger('SESSION_TITLE'); + +const MAX_CONVERSATION_CHARS = 1000; +const RECENT_MESSAGE_WINDOW = 20; + +const TITLE_SYSTEM_PROMPT = `Generate a concise, sentence-case title (3-7 words) that captures what this programming-assistant session is about. Think of it as a git commit subject for the session. + +Rules: +- 3-7 words. +- Sentence case: capitalize only the first word and proper nouns. NOT Title Case. +- No trailing punctuation. +- No quotes, backticks, or markdown. +- Match the dominant language of the conversation (English or Chinese). For Chinese, treat as roughly 12-20 characters total; still no trailing punctuation. +- Be specific about the user's actual goal — name the feature, bug, or subject area. Avoid vague "Code changes", "Help request", "Conversation". + +Good examples: +{"title": "Fix login button on mobile"} +{"title": "Add OAuth authentication flow"} +{"title": "Debug failing CI pipeline tests"} +{"title": "重构用户鉴权中间件"} + +Bad (too vague): {"title": "Code changes"} +Bad (too long): {"title": "Investigate and fix the session title generation issue in the chat recording service"} +Bad (wrong case): {"title": "Fix Login Button On Mobile"} +Bad (trailing punctuation): {"title": "Fix login button."} + +Return ONLY a JSON object with a single "title" key. No preamble, no reasoning, no closing remarks.`; + +const TITLE_USER_PROMPT = + 'Generate the session title now. Populate the schema with a single short title string.'; + +const TITLE_SCHEMA = { + type: 'object', + properties: { + title: { + type: 'string', + description: + 'A concise sentence-case session title, 3-7 words, no trailing punctuation.', + }, + }, + required: ['title'], +} as const; + +const LEADING_MARKERS_RE = /^[\s>*\-#`"'_]+/; +const TRAILING_MARKERS_RE = /[\s*`"'_]+$/; +const TRAILING_PUNCT_RE = /[.!?。！？,，;；:：]+$/; +// Paired CJK brackets (e.g. `【Draft】Fix login`): strip as a whole so a +// lone closing bracket doesn't dangle after a leading-char-class strip. +const LEADING_PAIRED_BRACKETS_RE = + /^\s*[「『【〈《][^」』】〉》]*[」』】〉》]\s*/; +const TRAILING_PAIRED_BRACKETS_RE = + /\s*[「『【〈《][^」』】〉》]*[」』】〉》]\s*$/; + +/** + * Reason a title generation didn't produce a usable title. Separated from + * the success payload so callers (esp. the interactive `/rename --auto` + * command) can surface actionable messages instead of a generic "could not + * generate". + * + * - `no_fast_model`: config.getFastModel() returned undefined. User needs to + * configure one via `/model --fast `. + * - `no_client`: BaseLlmClient or GeminiClient not yet initialized. Rare, + * usually means the session hasn't authenticated yet. + * - `empty_history`: the conversation has fewer than 2 turns of usable text. + * User should send at least one message before asking for a title. + * - `empty_result`: the model returned nothing parseable into a title. Often + * means the model is too small or the conversation text is meaningless + * (e.g., only tool calls). + * - `aborted`: AbortSignal fired (user pressed Ctrl-C / new session / switch). + * - `model_error`: the LLM call threw — rate limit, auth, network, etc. + */ +export type SessionTitleFailureReason = + | 'no_fast_model' + | 'no_client' + | 'empty_history' + | 'empty_result' + | 'aborted' + | 'model_error'; + +export type SessionTitleOutcome = + | { ok: true; title: string; modelUsed: string } + | { ok: false; reason: SessionTitleFailureReason }; + +/** + * Generate a short (3-7 word, sentence-case) title for the current session + * using the configured fast model. Best-effort — never throws. + * + * Returns a discriminated result so callers can either handle failures + * generically (`if (!outcome.ok) return null`) or map failure reasons to + * actionable messages (as `/rename --auto` does). + */ +export async function tryGenerateSessionTitle( + config: Config, + abortSignal: AbortSignal, +): Promise { + try { + const model = config.getFastModel(); + if (!model) return { ok: false, reason: 'no_fast_model' }; + + const geminiClient = config.getGeminiClient(); + if (!geminiClient) return { ok: false, reason: 'no_client' }; + + const fullHistory = geminiClient.getChat().getHistory(); + if (fullHistory.length < 2) return { ok: false, reason: 'empty_history' }; + + const dialog = filterToDialog(fullHistory); + const recentHistory = takeRecentDialog(dialog, RECENT_MESSAGE_WINDOW); + if (recentHistory.length === 0) { + return { ok: false, reason: 'empty_history' }; + } + + const conversationText = flattenToTail( + recentHistory, + MAX_CONVERSATION_CHARS, + ); + if (!conversationText.trim()) return { ok: false, reason: 'empty_history' }; + + const baseLlmClient = config.getBaseLlmClient(); + if (!baseLlmClient) return { ok: false, reason: 'no_client' }; + + const result = await baseLlmClient.generateJson({ + model, + systemInstruction: TITLE_SYSTEM_PROMPT, + schema: TITLE_SCHEMA as unknown as Record, + contents: [ + { + role: 'user', + parts: [ + { + text: `Conversation so far:\n${conversationText}\n\n${TITLE_USER_PROMPT}`, + }, + ], + }, + ], + config: { + temperature: 0.2, + maxOutputTokens: 100, + }, + abortSignal, + promptId: 'session_title', + // Titles are best-effort cosmetic metadata — one shot only, no long retry loop. + maxAttempts: 1, + }); + + if (abortSignal.aborted) return { ok: false, reason: 'aborted' }; + + const rawTitle = + typeof result?.['title'] === 'string' ? (result['title'] as string) : ''; + const title = sanitizeTitle(rawTitle); + if (!title) return { ok: false, reason: 'empty_result' }; + + return { ok: true, title, modelUsed: model }; + } catch (err) { + debugLogger.warn( + `Session title generation failed: ${err instanceof Error ? err.message : String(err)}`, + ); + if (abortSignal.aborted) return { ok: false, reason: 'aborted' }; + return { ok: false, reason: 'model_error' }; + } +} + +/** + * Normalize a raw title string coming back from the schema-enforced JSON + * call. The schema guarantees a string, but models routinely ignore the + * "no markdown / no trailing punctuation" guidance, so we strip those + * post-hoc. Exported for unit tests. Returns '' if nothing recoverable. + */ +export function sanitizeTitle(s: string): string { + // SECURITY: strip terminal control sequences first. The title renders + // directly in the picker — a model-returned ANSI/OSC-8 escape would + // otherwise execute on every render. See `stripTerminalControlSequences` + // for the coverage list. + let t = stripTerminalControlSequences(s).trim(); + // Strip paired CJK bracket prefix/suffix first (as units) so we don't end + // up with a lone closing bracket after the single-character strips below. + t = t.replace(LEADING_PAIRED_BRACKETS_RE, ''); + t = t.replace(TRAILING_PAIRED_BRACKETS_RE, ''); + t = t.replace(LEADING_MARKERS_RE, ''); + t = t.replace(TRAILING_MARKERS_RE, ''); + t = t.replace(TRAILING_PUNCT_RE, ''); + t = t.replace(/\s+/g, ' ').trim(); + if (!t) return ''; + if (t.length > SESSION_TITLE_MAX_LENGTH) { + t = t.slice(0, SESSION_TITLE_MAX_LENGTH).trim(); + // slice() can split a surrogate pair at the boundary — drop any + // orphaned surrogate so the resulting string stays well-formed UTF-16. + t = t.replace(/[\uD800-\uDBFF](?![\uDC00-\uDFFF])/g, ''); + t = t.replace(/(? + typeof part?.text === 'string' && + part.text.trim() !== '' && + !part.thought && + !part.thoughtSignature, + ); + if (textParts.length === 0) continue; + out.push({ role: msg.role, parts: textParts }); + } + return out; +} + +/** + * Take the most recent N messages while preserving turn structure: never + * start the slice on a model response that would dangle without its preceding + * user message. + */ +function takeRecentDialog(history: Content[], windowSize: number): Content[] { + if (history.length <= windowSize) return history; + let start = history.length - windowSize; + while (start < history.length && history[start]?.role !== 'user') { + start++; + } + return history.slice(start); +} + +/** + * Flatten filtered dialog to labeled plain text, then tail-slice to the last + * N characters. Tail (rather than head) captures what the session has become, + * not just how it opened — e.g. a session that starts with "help me debug X" + * but ends up refactoring Y should get a title about Y. + */ +function flattenToTail(history: Content[], maxChars: number): string { + const lines: string[] = []; + for (const msg of history) { + const role = msg.role === 'user' ? 'User' : 'Assistant'; + const text = (msg.parts ?? []) + .map((p) => p.text ?? '') + .join('') + .trim(); + if (!text) continue; + lines.push(`${role}: ${text}`); + } + const joined = lines.join('\n'); + if (joined.length <= maxChars) return joined; + let tail = joined.slice(-maxChars); + // `.slice` on a UTF-16 code-unit boundary can strand a lone low-surrogate + // at the start (if the slice cut through a CJK supplementary char or emoji). + // JSON-serializing that to the LLM produces an invalid surrogate that some + // providers reject with 400s, burning an attempt against the 3-cap for no + // real reason. Drop the dangling surrogate so the payload is always + // well-formed UTF-16. + if (tail.length > 0) { + const firstCode = tail.charCodeAt(0); + if (firstCode >= 0xdc00 && firstCode <= 0xdfff) { + tail = tail.slice(1); + } + } + return tail; +} diff --git a/packages/core/src/utils/sessionStorageUtils.test.ts b/packages/core/src/utils/sessionStorageUtils.test.ts index 1e8cf9eb7..0e5263677 100644 --- a/packages/core/src/utils/sessionStorageUtils.test.ts +++ b/packages/core/src/utils/sessionStorageUtils.test.ts @@ -11,8 +11,10 @@ import path from 'node:path'; import { extractJsonStringField, extractLastJsonStringField, + extractLastJsonStringFields, LITE_READ_BUF_SIZE, readLastJsonStringFieldSync, + readLastJsonStringFieldsSync, unescapeJsonString, } from './sessionStorageUtils.js'; @@ -280,4 +282,243 @@ describe('sessionStorageUtils', () => { ).toBe('last'); }); }); + + describe('extractLastJsonStringFields', () => { + it('returns undefined for every key when primary is absent', () => { + const hit = extractLastJsonStringFields( + '{"type":"user","message":"hi"}', + 'customTitle', + ['titleSource'], + 'custom_title', + ); + expect(hit).toEqual({ customTitle: undefined, titleSource: undefined }); + }); + + it('extracts secondary field from the same line as the primary', () => { + const text = + '{"subtype":"custom_title","customTitle":"A","titleSource":"auto"}\n'; + const hit = extractLastJsonStringFields( + text, + 'customTitle', + ['titleSource'], + 'custom_title', + ); + expect(hit).toEqual({ customTitle: 'A', titleSource: 'auto' }); + }); + + it('when primary appears on multiple lines, picks the latest and its own secondary', () => { + const text = [ + '{"subtype":"custom_title","customTitle":"A","titleSource":"manual"}', + '{"subtype":"custom_title","customTitle":"B","titleSource":"auto"}', + '', + ].join('\n'); + const hit = extractLastJsonStringFields( + text, + 'customTitle', + ['titleSource'], + 'custom_title', + ); + expect(hit).toEqual({ customTitle: 'B', titleSource: 'auto' }); + }); + + it('returns secondary=undefined when the winning line lacks it (legacy record)', () => { + const text = '{"subtype":"custom_title","customTitle":"legacy"}\n'; + const hit = extractLastJsonStringFields( + text, + 'customTitle', + ['titleSource'], + 'custom_title', + ); + expect(hit).toEqual({ customTitle: 'legacy', titleSource: undefined }); + }); + + it('never lets titleSource from an OLDER line leak into a NEWER primary match', () => { + // Older record has both fields; newer record (wins) has only customTitle. + // If the implementation did two separate scans, titleSource would leak + // from the older line — the single-pass contract forbids this. + const text = [ + '{"subtype":"custom_title","customTitle":"old","titleSource":"auto"}', + '{"subtype":"custom_title","customTitle":"new"}', + '', + ].join('\n'); + const hit = extractLastJsonStringFields( + text, + 'customTitle', + ['titleSource'], + 'custom_title', + ); + expect(hit).toEqual({ customTitle: 'new', titleSource: undefined }); + }); + + it('respects lineContains — matches on non-tagged lines are ignored', () => { + // A user message happens to contain a customTitle substring; the line + // doesn't include "custom_title" so it's filtered out. + const text = [ + '{"type":"user","message":"I want customTitle: \\"fake\\""}', + '{"subtype":"custom_title","customTitle":"real","titleSource":"manual"}', + '', + ].join('\n'); + const hit = extractLastJsonStringFields( + text, + 'customTitle', + ['titleSource'], + 'custom_title', + ); + expect(hit).toEqual({ customTitle: 'real', titleSource: 'manual' }); + }); + + it('rejects a crash-truncated trailing record with no closing quote', () => { + // A clean record is followed by a truncated partial write. A naive + // implementation would pick the truncated line as "latest" and return + // titleSource=undefined (since the line never got its source written). + // We require both fields from the last VALID record. + const text = + '{"subtype":"custom_title","customTitle":"A","titleSource":"auto"}\n' + + '{"subtype":"custom_title","customTitle":"B'; + const hit = extractLastJsonStringFields( + text, + 'customTitle', + ['titleSource'], + 'custom_title', + ); + expect(hit).toEqual({ customTitle: 'A', titleSource: 'auto' }); + }); + + it('handles escaped quotes inside the primary value', () => { + const text = + '{"subtype":"custom_title","customTitle":"He said \\"hi\\"","titleSource":"manual"}\n'; + const hit = extractLastJsonStringFields( + text, + 'customTitle', + ['titleSource'], + 'custom_title', + ); + expect(hit).toEqual({ + customTitle: 'He said "hi"', + titleSource: 'manual', + }); + }); + }); + + describe('readLastJsonStringFieldsSync', () => { + let tmpDir: string; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'sst-readfields-')); + }); + + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + function writeFile(name: string, content: string): string { + const p = path.join(tmpDir, name); + fs.writeFileSync(p, content); + return p; + } + + it('returns all-undefined for a missing file', () => { + const p = path.join(tmpDir, 'nope.jsonl'); + expect( + readLastJsonStringFieldsSync( + p, + 'customTitle', + ['titleSource'], + 'custom_title', + ), + ).toEqual({ customTitle: undefined, titleSource: undefined }); + }); + + it('returns the atomic pair when tail contains the match', () => { + const p = writeFile( + 'tail.jsonl', + '{"subtype":"custom_title","customTitle":"A","titleSource":"auto"}\n', + ); + expect( + readLastJsonStringFieldsSync( + p, + 'customTitle', + ['titleSource'], + 'custom_title', + ), + ).toEqual({ customTitle: 'A', titleSource: 'auto' }); + }); + + it('falls through to full-file scan when tail has no match and finds the pair', () => { + // Primary+secondary near start, filler > LITE_READ_BUF_SIZE, nothing in tail. + const header = + '{"subtype":"custom_title","customTitle":"X","titleSource":"auto"}\n'; + const filler = + '{"type":"user","message":"' + 'x'.repeat(LITE_READ_BUF_SIZE) + '"}\n'; + const p = writeFile('phase2.jsonl', header + filler); + expect( + readLastJsonStringFieldsSync( + p, + 'customTitle', + ['titleSource'], + 'custom_title', + ), + ).toEqual({ customTitle: 'X', titleSource: 'auto' }); + }); + + it('handles records straddling a Phase-2 chunk boundary', () => { + // Goal: place the winning custom_title record so it begins in Phase-2 + // chunk N and ends in chunk N+1. That exercises the `carry` logic in + // readLastJsonStringFieldsSync — without it, the partial first chunk + // wouldn't contain the closing quote and the match would be missed. + // + // Layout: + // [padA bytes..............][custom_title line][padB bytes................] + // with padA + fixed header bytes + half of the title line < + // LITE_READ_BUF_SIZE, and the rest of the title line spilling into + // the next chunk. padB is >> tail window so Phase-2 fires (tail miss). + const titleLine = + '{"subtype":"custom_title","customTitle":"spanner","titleSource":"manual"}'; + const userHeader = '{"type":"user","message":"'; + const userFooter = '"}\n'; + // Position the title line so its middle lands on the chunk boundary. + const padALen = + LITE_READ_BUF_SIZE - + userHeader.length - + userFooter.length - + Math.floor(titleLine.length / 2); + const padA = 'a'.repeat(padALen); + const padB = 'b'.repeat(LITE_READ_BUF_SIZE * 2); + const p = writeFile( + 'straddle.jsonl', + userHeader + + padA + + userFooter + + titleLine + + '\n' + + userHeader + + padB + + userFooter, + ); + expect( + readLastJsonStringFieldsSync( + p, + 'customTitle', + ['titleSource'], + 'custom_title', + ), + ).toEqual({ customTitle: 'spanner', titleSource: 'manual' }); + }); + + it('does not let a truncated trailing partial record win', () => { + const p = writeFile( + 'truncated.jsonl', + '{"subtype":"custom_title","customTitle":"A","titleSource":"auto"}\n' + + '{"subtype":"custom_title","customTitle":"B', + ); + expect( + readLastJsonStringFieldsSync( + p, + 'customTitle', + ['titleSource'], + 'custom_title', + ), + ).toEqual({ customTitle: 'A', titleSource: 'auto' }); + }); + }); }); diff --git a/packages/core/src/utils/sessionStorageUtils.ts b/packages/core/src/utils/sessionStorageUtils.ts index c91c8615d..3c76e528e 100644 --- a/packages/core/src/utils/sessionStorageUtils.ts +++ b/packages/core/src/utils/sessionStorageUtils.ts @@ -16,6 +16,33 @@ import fs from 'node:fs'; /** Size of the head/tail buffer for lite metadata reads (64KB). */ export const LITE_READ_BUF_SIZE = 64 * 1024; +/** + * Maximum size (bytes) we'll scan in the Phase-2 full-file fallback. Tail- + * read fast path covers the realistic case (metadata is re-appended on every + * session lifecycle event). A pathological / corrupt session file that's + * tens of GB should NOT block the picker for minutes while we scan it all. + * The session picker renders on the main event loop, so blocking I/O here + * freezes the UI. + */ +export const MAX_FULL_SCAN_BYTES = 64 * 1024 * 1024; + +/** + * Flags used when opening session files for metadata reads. `O_NOFOLLOW` + * refuses to follow symlinks — defense in depth so a symlink planted in + * `~/.qwen/tmp//chats/` (by another local user or an extension with + * filesystem access) can't redirect a metadata read to an unrelated file. + * Falls back to plain read-only when the flag isn't available (e.g. Windows + * doesn't expose O_NOFOLLOW; the constant is `undefined` there). + * + * Computed lazily so tests that stub out `fs` don't blow up at module-init + * time trying to read `fs.constants.O_RDONLY`. + */ +function getReadOpenFlags(): number { + const constants = fs.constants; + if (!constants) return 0; + return (constants.O_RDONLY ?? 0) | (constants.O_NOFOLLOW ?? 0); +} + // --------------------------------------------------------------------------- // JSON string field extraction — no full parse, works on truncated lines // --------------------------------------------------------------------------- @@ -63,6 +90,92 @@ export function extractJsonStringField( return undefined; } +/** + * Like extractJsonStringField but finds the LAST well-formed occurrence of + * `primaryKey` and returns every `otherKeys` value extracted from THAT SAME + * line. Two separate `extractLastJsonStringField` calls can land on different + * records when an older line contains only one of the fields — this function + * guarantees the returned fields all come from the same record. + * + * Validation: a primary-key match counts only when its string value has a + * proper closing quote. A crash-truncated trailing record (`"customTitle":"x` + * with no closing `"`) is ignored — otherwise it could "win" the latest-match + * race and cause the function to extract secondaries from a partial line + * where they don't appear. + * + * When `lineContains` is provided, only lines containing that substring are + * considered matches (same semantics as the single-field version). + */ +export function extractLastJsonStringFields( + text: string, + primaryKey: string, + otherKeys: string[], + lineContains?: string, +): Record { + const out: Record = { [primaryKey]: undefined }; + for (const k of otherKeys) out[k] = undefined; + + const patterns = [`"${primaryKey}":"`, `"${primaryKey}": "`]; + + let bestPrimaryValue: string | undefined; + let bestLineStart = -1; + let bestLineEnd = -1; + let bestOffset = -1; + + for (const pattern of patterns) { + let searchFrom = 0; + while (true) { + const idx = text.indexOf(pattern, searchFrom); + if (idx < 0) break; + searchFrom = idx + pattern.length; + + // Line-contains filter first (cheap) + const lineStart = text.lastIndexOf('\n', idx) + 1; + const eol = text.indexOf('\n', idx); + const lineEnd = eol < 0 ? text.length : eol; + if (lineContains) { + const line = text.slice(lineStart, lineEnd); + if (!line.includes(lineContains)) continue; + } + + // Validate the value: walk to a non-escaped closing quote. A truncated + // trailing write (no closing quote before EOF) is rejected — this is + // the guard that keeps crash-recovery safe. + const valueStart = idx + pattern.length; + let i = valueStart; + let terminated = false; + while (i < text.length) { + if (text[i] === '\\') { + i += 2; + continue; + } + if (text[i] === '"') { + terminated = true; + break; + } + i++; + } + if (!terminated) continue; + + // We accept this match; keep it if it's the latest so far. + if (idx > bestOffset) { + bestOffset = idx; + bestLineStart = lineStart; + bestLineEnd = lineEnd; + bestPrimaryValue = unescapeJsonString(text.slice(valueStart, i)); + } + } + } + + if (bestOffset < 0) return out; + out[primaryKey] = bestPrimaryValue; + const line = text.slice(bestLineStart, bestLineEnd); + for (const k of otherKeys) { + out[k] = extractJsonStringField(line, k); + } + return out; +} + /** * Like extractJsonStringField but finds the LAST occurrence. * Useful for fields that are appended (customTitle, aiTitle, etc.) @@ -157,7 +270,7 @@ export function readLastJsonStringFieldSync( const fileSize = stats.size; if (fileSize === 0) return undefined; - fd = fs.openSync(filePath, 'r'); + fd = fs.openSync(filePath, getReadOpenFlags()); // Phase 1: tail window — fast path. const tailLength = Math.min(fileSize, LITE_READ_BUF_SIZE); @@ -176,16 +289,18 @@ export function readLastJsonStringFieldSync( // to scan. if (tailOffset === 0) return undefined; - // Phase 2: stream the whole file and return the last hit. Scanning from - // offset 0 (rather than [0, tailOffset)) avoids the edge case where a - // single record straddles the Phase 1/Phase 2 boundary — duplicate work - // on the tail bytes is harmless because we only care about the final - // match. + // Phase 2: stream the file up to MAX_FULL_SCAN_BYTES and return the last + // hit. Scanning from offset 0 (rather than [0, tailOffset)) avoids the + // edge case where a single record straddles the Phase 1/Phase 2 boundary + // — duplicate work on the tail bytes is harmless because we only care + // about the final match. The hard cap bounds worst-case latency for + // pathologically large session files (which would freeze the picker). let lastHit: string | undefined; let readOffset = 0; let carry = ''; - while (readOffset < fileSize) { - const toRead = Math.min(LITE_READ_BUF_SIZE, fileSize - readOffset); + const scanLimit = Math.min(fileSize, MAX_FULL_SCAN_BYTES); + while (readOffset < scanLimit) { + const toRead = Math.min(LITE_READ_BUF_SIZE, scanLimit - readOffset); const buf = Buffer.alloc(toRead); const bytesRead = fs.readSync(fd, buf, 0, toRead, readOffset); if (bytesRead === 0) break; @@ -225,3 +340,105 @@ export function readLastJsonStringFieldSync( } } } + +/** + * Like {@link readLastJsonStringFieldSync} but extracts multiple fields from + * the same matching line atomically (single file scan, consistent pair). + * + * The primary key determines the "winning" line (latest occurrence on a line + * that also contains `lineContains`). Every other requested field is pulled + * from that same line — never from an earlier or later record — so callers + * get a consistent record snapshot. Useful when a record pairs a payload + * field with its metadata (e.g. `customTitle` + `titleSource`). + * + * Missing fields (primary or secondary) appear in the returned object with + * value `undefined`. I/O errors yield `undefined` for every key. + */ +export function readLastJsonStringFieldsSync( + filePath: string, + primaryKey: string, + otherKeys: string[], + lineContains?: string, +): Record { + const emptyResult: Record = {}; + emptyResult[primaryKey] = undefined; + for (const k of otherKeys) emptyResult[k] = undefined; + + let fd: number | undefined; + try { + const stats = fs.statSync(filePath); + const fileSize = stats.size; + if (fileSize === 0) return emptyResult; + + fd = fs.openSync(filePath, getReadOpenFlags()); + + // Phase 1: tail window fast path. + const tailLength = Math.min(fileSize, LITE_READ_BUF_SIZE); + const tailOffset = fileSize - tailLength; + const tailBuffer = Buffer.alloc(tailLength); + const tailBytes = fs.readSync(fd, tailBuffer, 0, tailLength, tailOffset); + if (tailBytes > 0) { + const tailText = tailBuffer.toString('utf-8', 0, tailBytes); + const hit = extractLastJsonStringFields( + tailText, + primaryKey, + otherKeys, + lineContains, + ); + if (hit[primaryKey] !== undefined) return hit; + } + + if (tailOffset === 0) return emptyResult; + + // Phase 2: stream the file up to MAX_FULL_SCAN_BYTES, track the latest + // match. Hard cap bounds worst-case latency on pathological files. + let latest: Record | undefined; + let readOffset = 0; + let carry = ''; + const scanLimit = Math.min(fileSize, MAX_FULL_SCAN_BYTES); + while (readOffset < scanLimit) { + const toRead = Math.min(LITE_READ_BUF_SIZE, scanLimit - readOffset); + const buf = Buffer.alloc(toRead); + const bytesRead = fs.readSync(fd, buf, 0, toRead, readOffset); + if (bytesRead === 0) break; + readOffset += bytesRead; + const chunk = carry + buf.toString('utf-8', 0, bytesRead); + const lastNewline = chunk.lastIndexOf('\n'); + if (lastNewline < 0) { + carry = chunk; + continue; + } + const complete = chunk.slice(0, lastNewline + 1); + carry = chunk.slice(lastNewline + 1); + + const hit = extractLastJsonStringFields( + complete, + primaryKey, + otherKeys, + lineContains, + ); + if (hit[primaryKey] !== undefined) latest = hit; + } + if (carry) { + const hit = extractLastJsonStringFields( + carry, + primaryKey, + otherKeys, + lineContains, + ); + if (hit[primaryKey] !== undefined) latest = hit; + } + + return latest ?? emptyResult; + } catch { + return emptyResult; + } finally { + if (fd !== undefined) { + try { + fs.closeSync(fd); + } catch { + // best-effort + } + } + } +} diff --git a/packages/core/src/utils/terminalSafe.ts b/packages/core/src/utils/terminalSafe.ts new file mode 100644 index 000000000..17951ba51 --- /dev/null +++ b/packages/core/src/utils/terminalSafe.ts @@ -0,0 +1,43 @@ +/** + * @license + * Copyright 2025 Qwen Code + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Strip the terminal control sequences from arbitrary text so the result can + * safely render in a TTY without painting cursor moves, clearing the screen, + * or injecting OSC-8 hyperlinks. + * + * Covers: + * - OSC sequences (`\x1b]...\x07` or `\x1b]...\x1b\\`) — handled as whole + * units so the ST/BEL terminator is also stripped. + * - CSI sequences (`\x1b[...`) — the common "cursor/color/erase" + * family. + * - SS2/SS3 / DCS leaders (`\x1b[NOP]`). + * - Any remaining C0/C1 control bytes plus DEL, flattened to a space. This + * backstop means a bare `\x1b` that wasn't part of a recognized sequence + * still can't execute — the terminal only interprets ESC followed by + * specific bytes. + * + * Used for LLM-returned text that ends up in the session picker (titles); + * without this, a compromised or prompt-injected fast model could paint on + * the user's terminal on every render. + */ +export function stripTerminalControlSequences(s: string): string { + // These regexes deliberately match control characters; the whole point of + // this module is to neutralize them. The no-control-regex rule is + // suppressed per-line rather than file-wide so any future additions still + // opt in explicitly. + return ( + s + // eslint-disable-next-line no-control-regex + .replace(/\x1b\][^\x07\x1b]*(?:\x07|\x1b\\)/g, ' ') + // eslint-disable-next-line no-control-regex + .replace(/\x1b\[[\d;?]*[a-zA-Z]/g, ' ') + // eslint-disable-next-line no-control-regex + .replace(/\x1b[NOP]/g, ' ') + // eslint-disable-next-line no-control-regex + .replace(/[\x00-\x1f\x7f]/g, ' ') + ); +}