diff --git a/.changeset/fix-claude-response-viewer.md b/.changeset/fix-claude-response-viewer.md new file mode 100644 index 00000000..53cf018b --- /dev/null +++ b/.changeset/fix-claude-response-viewer.md @@ -0,0 +1,9 @@ +--- +'aicodeman': patch +--- + +Normalize Claude conversations in the response viewer. A Claude transcript is an append-only event log, so one logical exchange spans many JSONL rows: tool-result rows, meta/image/skill rows, compact summaries, task and team notifications, sidechains, replayed assistant snapshots, and multi-block assistant output. The viewer rendered a card per row, which produced duplicate and truncated cards that read as lost responses. Cards are now built at real human-turn boundaries, replayed assistant snapshots are deduplicated, and sidechain rows (which belong to subagents, not the main conversation) no longer leak in. An identical prompt that legitimately recurs after an assistant reply is still kept as its own turn. + +Measured over 40 real transcripts: 3108 cards became 621, duplicate cards dropped from 74 to 8 (all of them genuinely repeated turns), no assistant text was lost, and the non-`context=full` last-response text was byte-identical on every file. + +Also rebinds recovered sessions to their transcript. `reconcileSessions()` can recover a lost mux session as a `restored-` placeholder with a stale working directory, which made transcript lookup by cwd find nothing. The placeholder still carries the first eight characters of the conversation UUID, so the viewer now rebinds to the matching top-level transcript when exactly one candidate matches. diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index 1535187c..6f954c77 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -145,7 +145,9 @@ The general rule: **any new endpoint that turns a caller-supplied `sessionId` in ### Header button visibility (multi-monitor, response viewer, file viewer, cron) **Multi-monitor button** (header, top-right; the notification bell it sits beside stays hidden — notifications live in Settings → Notifications). `app.launchMultiMonitor()` (in `panels-ui.js`) POSTs `/api/system/span-displays`, which spawns `scripts/span-codeman.sh` — a fresh, maximized browser `--app` window sized to the union of all displays (macOS; needs "Displays have separate Spaces" OFF). Supports the gesture layer's in-page floating session panels dragging across the physical monitor seam. **Opt-in:** hidden by default; enable under App Settings → Display → **Header Displays** ("Multi-monitor Button", `showMultiMonitorButton`). The button carries a `btn-multimonitor--hidden` class in the template; `renderIndexHtml` strips that class at render when the setting is on (a unique class token, not a brittle match on the aria-label/style copy), and `applyHeaderVisibilitySettings()` toggles the same class live on save. Solo (detached) windows hide it via `body.solo-mode`. -**Response-viewer (eye) button** (header) is likewise **hidden by default** — enable under App Settings → Display → **Response Viewer** (`showResponseViewer`). Works for Claude AND Codex sessions (#152): Codex last-responses are located via a 4-layer rollout resolution under `CODEX_HOME` (history pin → originator match → resume-UUID → cwd fallback with other-pane exclusion), with injected-context filtering and event/legacy dedup — tests in `test/routes/session-routes-codex-last-response.test.ts`. Purely client-side (no `renderIndexHtml` step): the template ships with `btn-response-viewer-header--hidden` and `applyHeaderVisibilitySettings()` (settings-ui.js) toggles it after settings load. Hiding must go through that marker class — the base rule is `display:inline-flex !important`, so an inline style can't override it. `showResponseViewer` is in the `displayKeys` per-device set (settings-ui.js), so it does NOT sync across devices. +**Response-viewer (eye) button** (header) is likewise **hidden by default** — enable under App Settings → Display → **Response Viewer** (`showResponseViewer`). Works for Claude AND Codex sessions (#152): Codex last-responses are located via a 4-layer rollout resolution under `CODEX_HOME` (history pin → originator match → resume-UUID → cwd fallback with other-pane exclusion), with injected-context filtering and event/legacy dedup — tests in `test/routes/session-routes-codex-last-response.test.ts`. + +⚠️ **Claude transcripts are grouped at real human-turn boundaries, not per JSONL row.** A Claude transcript is an append-only event log, so one logical exchange spans many rows: tool-result rows, meta/image/skill rows, compact summaries, task/team notifications, sidechains, replayed assistant snapshots, and multi-block assistant output. Rendering a card per row was the bug: it produced duplicate and truncated cards that looked like the viewer had lost the response. The grouping walks to the next genuine user turn and dedups replayed assistant snapshots while preserving the tool/task/skill/compact/team metadata filtering. Related: a recovered `restored-` tmux placeholder carries a **stale cwd**, so transcript lookup by working directory finds nothing; it rebinds to the matching top-level Claude transcript UUID instead when that match is unambiguous. Tests: `test/routes/session-routes-claude-last-response.test.ts`. Purely client-side (no `renderIndexHtml` step): the template ships with `btn-response-viewer-header--hidden` and `applyHeaderVisibilitySettings()` (settings-ui.js) toggles it after settings load. Hiding must go through that marker class — the base rule is `display:inline-flex !important`, so an inline style can't override it. `showResponseViewer` is in the `displayKeys` per-device set (settings-ui.js), so it does NOT sync across devices. **File Viewer button** (header, 1.4.1) is **shown by default on desktop** since `211f3c0` (post-1.8.0): toggle under App Settings → Display → **Header Displays** → File Viewer (`showFileViewerButton`, in the per-device `displayKeys` set, fallback default `true`). Purely client-side like the response viewer: the template now ships the button VISIBLE (no `--hidden` class) and `applyHeaderVisibilitySettings()` toggles the `btn-file-viewer--hidden` marker class after settings load; phones still hide it via mobile.css. The button toggles the file-browser panel open/closed without opening the settings modal (`panels-ui.js`). The same commit set the **default desktop header** to WS/CPU/MEM + File Viewer + gear: the token-count chip (`showTokenCount`, no settings-UI toggle) and the lifecycle-log button (`showLifecycleLog`) both default **OFF** now (templates ship them hidden; stored prefs still honored). The plan-usage chip default is unchanged (opt-in, see Plan-usage chip). The **Cron toolbar button** joined the same opt-in pattern in 1.6.0: template ships `btn-cron--hidden`, `applyHeaderVisibilitySettings()` toggles it via the per-device `showCronButton` setting (default OFF, App Settings → Display → Header Displays); cron jobs themselves are unaffected. ### Gesture control: the setting diff --git a/src/web/routes/session-routes.ts b/src/web/routes/session-routes.ts index 632cb092..20636f8a 100644 --- a/src/web/routes/session-routes.ts +++ b/src/web/routes/session-routes.ts @@ -1016,6 +1016,180 @@ export function registerSessionRoutes( return candidateSid; } + interface ClaudeResponseMessage { + role: 'user' | 'assistant'; + text: string; + timestamp?: string; + } + + interface ClaudeTranscriptEntry { + type?: string; + timestamp?: string; + isMeta?: boolean; + isSidechain?: boolean; + isCompactSummary?: boolean; + message?: { content?: unknown }; + } + + function extractClaudeText(content: unknown, separator: string): string { + if (typeof content === 'string') return content; + if (!Array.isArray(content)) return ''; + return content + .filter( + (block): block is { type: string; text: string } => + !!block && + typeof block === 'object' && + (block as { type?: string }).type === 'text' && + typeof (block as { text?: string }).text === 'string' + ) + .map((block) => block.text) + .join(separator); + } + + function isClaudeSyntheticUserMessage(entry: ClaudeTranscriptEntry, text: string): boolean { + if (entry.isMeta || entry.isCompactSummary) return true; + return /^(?:|||(); + let currentAssistantFragments = new Set(); + + for (const line of content.split('\n')) { + if (!line) continue; + let entry: ClaudeTranscriptEntry; + try { + entry = JSON.parse(line) as ClaudeTranscriptEntry; + } catch { + continue; + } + // Sidechains belong to agents/forks, not the main conversation. Meta user + // rows include repeated image dimensions and other UI-generated context. + if (entry.isSidechain) continue; + + if (entry.type === 'user') { + const text = extractClaudeText(entry.message?.content, '\n').trim(); + // A tool_result block has no text block and naturally drops out here. + if (!text || isClaudeSyntheticUserMessage(entry, text)) continue; + if (!full) continue; + + const previous = messages.at(-1); + if (previous?.role === 'user') { + // Claude can replay the initial user row while restoring a transcript. + // Only collapse duplicates within the same unanswered user turn; the + // same prompt after an assistant response remains a legitimate turn. + if (currentUserFragments.has(text)) continue; + previous.text += `\n\n${text}`; + currentUserFragments.add(text); + } else { + messages.push({ role: 'user', text, timestamp: entry.timestamp }); + currentUserFragments = new Set([text]); + } + currentAssistantFragments.clear(); + continue; + } + + if (entry.type !== 'assistant') continue; + const text = extractClaudeText(entry.message?.content, '\n\n').trim(); + if (!text) continue; + lastText = text; + lastTimestamp = entry.timestamp || ''; + if (!full) continue; + + const previous = messages.at(-1); + if (previous?.role === 'assistant') { + // Replayed snapshots sometimes repeat an identical text block. Distinct + // progress/final blocks are kept, but remain inside one Claude card. + if (currentAssistantFragments.has(text)) continue; + previous.text += `\n\n${text}`; + previous.timestamp = entry.timestamp || previous.timestamp; + currentAssistantFragments.add(text); + } else { + messages.push({ role: 'assistant', text, timestamp: entry.timestamp }); + currentAssistantFragments = new Set([text]); + } + currentUserFragments.clear(); + } + + return full ? { text: lastText, timestamp: lastTimestamp, messages } : { text: lastText, timestamp: lastTimestamp }; + } + + /** Locate a top-level Claude transcript, including recovered tmux sessions. */ + async function findClaudeTranscript( + projectsDir: string, + conversationId: string, + codemanSessionId: string + ): Promise<{ sessionId: string; path: string } | null> { + let projectDirs: import('node:fs').Dirent[]; + try { + projectDirs = await fs.readdir(projectsDir, { withFileTypes: true }); + } catch { + return null; + } + + const safeIds = [...new Set([conversationId, codemanSessionId])].filter((value) => /^[a-zA-Z0-9._-]+$/.test(value)); + for (const candidateId of safeIds) { + for (const projectDir of projectDirs) { + if (!projectDir.isDirectory()) continue; + const jsonlPath = join(projectsDir, projectDir.name, `${candidateId}.jsonl`); + try { + const stat = await fs.stat(jsonlPath); + if (stat.isFile()) return { sessionId: candidateId, path: jsonlPath }; + } catch { + /* continue */ + } + } + } + + // If mux-sessions.json was lost or stale, reconcileSessions() historically + // recovered `codeman-40568a29` as `restored-40568a29` and used the server cwd. + // The tmux name still carries the first eight UUID characters, which safely + // reconnects the viewer when exactly one matching top-level transcript exists. + const restoredMatch = /^restored-([a-f0-9]{8,})$/i.exec(codemanSessionId); + if (!restoredMatch) return null; + const fragment = restoredMatch[1].toLowerCase(); + const candidates: Array<{ sessionId: string; path: string; mtimeMs: number }> = []; + const uuidPattern = /^[a-f0-9]{8}-[a-f0-9]{4}-[1-5][a-f0-9]{3}-[89ab][a-f0-9]{3}-[a-f0-9]{12}$/i; + + for (const projectDir of projectDirs) { + if (!projectDir.isDirectory()) continue; + const dirPath = join(projectsDir, projectDir.name); + let files: import('node:fs').Dirent[]; + try { + files = await fs.readdir(dirPath, { withFileTypes: true }); + } catch { + continue; + } + for (const file of files) { + if (!file.isFile() || !file.name.endsWith('.jsonl')) continue; + const candidateId = file.name.slice(0, -'.jsonl'.length); + if (!candidateId.toLowerCase().startsWith(fragment) || !uuidPattern.test(candidateId)) continue; + const path = join(dirPath, file.name); + const stat = await fs.stat(path).catch(() => null); + if (stat) candidates.push({ sessionId: candidateId, path, mtimeMs: stat.mtimeMs }); + } + } + + const candidateIds = new Set(candidates.map((candidate) => candidate.sessionId)); + if (candidateIds.size !== 1) return null; + candidates.sort((a, b) => b.mtimeMs - a.mtimeMs); + return candidates[0] ?? null; + } + app.get('/api/sessions/:id/last-response', async (req) => { const { id } = req.params as { id: string }; const session = findSessionOrFail(ctx, id, req); @@ -1045,106 +1219,30 @@ export function registerSessionRoutes( } } - // The Claude conversation ID (used as JSONL filename) + const query = req.query as { context?: string }; const claudeSessionId = session.claudeSessionId || session.id; - let transcriptText = ''; - let transcriptTimestamp = ''; - - try { - const projectDirs = await fs.readdir(projectsDir); - for (const projDir of projectDirs) { - const jsonlPath = join(projectsDir, projDir, `${claudeSessionId}.jsonl`); - try { - const content = await fs.readFile(jsonlPath, 'utf8'); - const lines = content.trim().split('\n'); - - // Search from end for last assistant message with text - for (let i = lines.length - 1; i >= 0; i--) { - try { - const entry = JSON.parse(lines[i]); - if (entry.type === 'assistant' && entry.message?.content) { - const blocks = Array.isArray(entry.message.content) - ? entry.message.content - : [{ type: 'text', text: String(entry.message.content) }]; - const textBlocks = blocks - .filter((b: { type: string; text?: string }) => b.type === 'text' && b.text) - .map((b: { type: string; text?: string }) => b.text); - if (textBlocks.length > 0) { - transcriptText = textBlocks.join('\n\n'); - transcriptTimestamp = entry.timestamp || ''; - break; - } - } - } catch { - // Skip unparseable lines - } - } - if (transcriptText) break; // Found it, stop scanning directories - } catch { - // File doesn't exist in this project dir, continue - } - } - } catch { - // projects dir doesn't exist + const transcript = await findClaudeTranscript(projectsDir, claudeSessionId, session.id); + if (!transcript) { + return query.context === 'full' ? { text: '', timestamp: '', messages: [] } : { text: '', timestamp: '' }; } - // If ?context=full, return all user+assistant messages for conversation view - const query = req.query as { context?: string }; - if (query.context === 'full' && transcriptText) { - const allMessages: Array<{ role: string; text: string; timestamp?: string }> = []; - try { - const projectDirs = await fs.readdir(projectsDir); - for (const projDir of projectDirs) { - const jsonlPath = join(projectsDir, projDir, `${claudeSessionId}.jsonl`); - try { - const content = await fs.readFile(jsonlPath, 'utf8'); - const lines = content.trim().split('\n'); - for (const line of lines) { - try { - const entry = JSON.parse(line); - if (entry.type === 'user' && entry.message?.content) { - const text = - typeof entry.message.content === 'string' - ? entry.message.content - : (entry.message.content as Array<{ type: string; text?: string }>) - .filter((b) => b.type === 'text' && b.text) - .map((b) => b.text) - .join('\n'); - // Skip system/command messages - if (text && !text.startsWith('')) { - allMessages.push({ role: 'user', text, timestamp: entry.timestamp }); - } - } else if (entry.type === 'assistant' && entry.message?.content) { - const blocks = Array.isArray(entry.message.content) - ? entry.message.content - : [{ type: 'text', text: String(entry.message.content) }]; - const text = blocks - .filter((b: { type: string; text?: string }) => b.type === 'text' && b.text) - .map((b: { type: string; text?: string }) => b.text) - .join('\n\n'); - if (text) { - allMessages.push({ role: 'assistant', text, timestamp: entry.timestamp }); - } - } - } catch { - /* skip */ - } - } - if (allMessages.length > 0) break; - } catch { - /* continue */ - } - } - } catch { - /* ignore */ + if (transcript.sessionId !== session.claudeSessionId && transcript.sessionId !== session.id) { + session.adoptClaudeSessionId(transcript.sessionId); + if (session.docker) { + void persistDockerCaseClaudeSessionId( + CODEMAN_CONFIG_DIR, + session.docker.containerName, + transcript.sessionId + ).catch(() => {}); } - return { text: transcriptText, timestamp: transcriptTimestamp, messages: allMessages }; } - return { - text: transcriptText, - timestamp: transcriptTimestamp, - }; + try { + const content = await fs.readFile(transcript.path, 'utf8'); + return parseClaudeResponseTranscript(content, query.context === 'full'); + } catch { + return query.context === 'full' ? { text: '', timestamp: '', messages: [] } : { text: '', timestamp: '' }; + } }); function isCodexInjectedContext(text: string): boolean { diff --git a/test/routes/session-routes-claude-last-response.test.ts b/test/routes/session-routes-claude-last-response.test.ts new file mode 100644 index 00000000..fd15c195 --- /dev/null +++ b/test/routes/session-routes-claude-last-response.test.ts @@ -0,0 +1,178 @@ +/** + * @fileoverview Claude transcript normalization tests for the response viewer. + * + * Uses app.inject() with a temporary HOME; no real ports or user transcripts. + * Port: N/A + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import Fastify, { type FastifyInstance } from 'fastify'; +import fastifyCookie from '@fastify/cookie'; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { installRouteErrorHandler } from '../../src/web/route-error-handler.js'; +import { registerSessionRoutes } from '../../src/web/routes/session-routes.js'; +import { ApiErrorCode, httpStatusForErrorCode } from '../../src/types.js'; +import { createMockRouteContext, type MockRouteContext } from '../mocks/index.js'; + +interface LocalHarness { + app: FastifyInstance; + ctx: MockRouteContext; +} + +async function createEnvelopeHarness(): Promise { + const app = Fastify({ logger: false }); + await app.register(fastifyCookie); + const ctx = createMockRouteContext(); + registerSessionRoutes(app, ctx); + + app.addHook('preSerialization', (req, reply, payload: unknown, done) => { + if (!req.url.startsWith('/api') || payload === null || typeof payload !== 'object') { + return done(null, payload); + } + const response = payload as { success?: unknown; errorCode?: unknown }; + if (response.success === false) { + if (reply.statusCode === 200 && typeof response.errorCode === 'string') { + reply.code(httpStatusForErrorCode(response.errorCode as ApiErrorCode)); + } + return done(null, payload); + } + if (response.success === true) return done(null, payload); + return done(null, { success: true, data: payload }); + }); + + installRouteErrorHandler(app); + await app.ready(); + return { app, ctx }; +} + +const userEntry = (text: string, extras: Record = {}) => ({ + type: 'user', + timestamp: '2026-07-21T00:00:00Z', + message: { content: [{ type: 'text', text }] }, + ...extras, +}); + +const assistantEntry = (text: string, timestamp: string) => ({ + type: 'assistant', + timestamp, + message: { content: [{ type: 'text', text }] }, +}); + +describe('GET /api/sessions/:id/last-response (claude)', () => { + let harness: LocalHarness; + let testHome: string; + let previousHome: string | undefined; + + beforeEach(async () => { + testHome = mkdtempSync(join(tmpdir(), 'codeman-claude-rv-')); + previousHome = process.env.HOME; + process.env.HOME = testHome; + harness = await createEnvelopeHarness(); + }); + + afterEach(async () => { + if (previousHome === undefined) delete process.env.HOME; + else process.env.HOME = previousHome; + rmSync(testHome, { recursive: true, force: true }); + await harness.app.close(); + }); + + function writeTranscript(sessionId: string, entries: unknown[]): void { + const projectDir = join(testHome, '.claude', 'projects', '-workspace'); + mkdirSync(projectDir, { recursive: true }); + writeFileSync(join(projectDir, `${sessionId}.jsonl`), entries.map((entry) => JSON.stringify(entry)).join('\n')); + } + + async function getLastResponse(sessionId: string, full = false) { + const response = await harness.app.inject({ + method: 'GET', + url: `/api/sessions/${sessionId}/last-response${full ? '?context=full' : ''}`, + }); + return { response, body: JSON.parse(response.body) }; + } + + it('recovers a placeholder tmux session by UUID prefix and groups JSONL fragments into turns', async () => { + const restoredId = 'restored-40568a29'; + const conversationId = '40568a29-d4eb-4eb6-b671-8401428e4f39'; + const session = harness.ctx._session as typeof harness.ctx._session & { + claudeSessionId: string; + adoptClaudeSessionId: ReturnType; + }; + harness.ctx.sessions.delete(session.id); + session.id = restoredId; + session.mode = 'claude'; + session.workingDir = '/wrong/recovered/cwd'; + session.claudeSessionId = restoredId; + session.adoptClaudeSessionId = vi.fn((newId: string) => { + session.claudeSessionId = newId; + }); + harness.ctx.sessions.set(restoredId, session); + + writeTranscript(conversationId, [ + userEntry('first prompt'), + userEntry('first prompt'), // restore replay before any assistant output + { type: 'assistant', message: { content: [{ type: 'thinking', thinking: 'hidden' }] } }, + assistantEntry('Checking the files.', '2026-07-21T00:00:01Z'), + { type: 'assistant', message: { content: [{ type: 'tool_use', id: 'tool-1' }] } }, + { type: 'user', message: { content: [{ type: 'tool_result', tool_use_id: 'tool-1' }] } }, + assistantEntry('Checking the files.', '2026-07-21T00:00:02Z'), // replayed snapshot + assistantEntry('The first result is ready.', '2026-07-21T00:00:03Z'), + userEntry('[Image dimensions generated by the CLI]', { isMeta: true }), + userEntry('/status'), + userEntry('Another Claude session sent a message: done'), + userEntry('background agent completed'), + userEntry('This session is being continued from a previous conversation', { isCompactSummary: true }), + userEntry('second prompt'), + assistantEntry('First half.', '2026-07-21T00:00:04Z'), + { ...assistantEntry('sidechain text', '2026-07-21T00:00:05Z'), isSidechain: true }, + assistantEntry('Second half.', '2026-07-21T00:00:06Z'), + ]); + + const full = await getLastResponse(restoredId, true); + expect(full.response.statusCode).toBe(200); + expect(full.body.data).toEqual({ + text: 'Second half.', + timestamp: '2026-07-21T00:00:06Z', + messages: [ + { role: 'user', text: 'first prompt', timestamp: '2026-07-21T00:00:00Z' }, + { + role: 'assistant', + text: 'Checking the files.\n\nThe first result is ready.', + timestamp: '2026-07-21T00:00:03Z', + }, + { role: 'user', text: 'second prompt', timestamp: '2026-07-21T00:00:00Z' }, + { role: 'assistant', text: 'First half.\n\nSecond half.', timestamp: '2026-07-21T00:00:06Z' }, + ], + }); + expect(session.adoptClaudeSessionId).toHaveBeenCalledWith(conversationId); + + const brief = await getLastResponse(restoredId); + expect(brief.body.data).toEqual({ text: 'Second half.', timestamp: '2026-07-21T00:00:06Z' }); + }); + + it('keeps an identical user prompt when it occurs again after an assistant response', async () => { + const sessionId = harness.ctx._session.id; + const session = harness.ctx._session as typeof harness.ctx._session & { + claudeSessionId: string; + adoptClaudeSessionId: ReturnType; + }; + session.claudeSessionId = sessionId; + session.adoptClaudeSessionId = vi.fn(); + writeTranscript(sessionId, [ + userEntry('continue'), + assistantEntry('First answer.', '2026-07-21T00:00:01Z'), + userEntry('continue'), + assistantEntry('Second answer.', '2026-07-21T00:00:02Z'), + ]); + + const { body } = await getLastResponse(sessionId, true); + expect(body.data.messages.map((message: { role: string; text: string }) => [message.role, message.text])).toEqual([ + ['user', 'continue'], + ['assistant', 'First answer.'], + ['user', 'continue'], + ['assistant', 'Second answer.'], + ]); + }); +});