From df9a6314da12b12d35de7d324a246e65d44a95fc Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Wed, 18 Feb 2026 17:30:12 -0800 Subject: [PATCH] refactor: Enhance session management and error handling in AgentService and related components - Improved session handling by implementing ensureSession to load sessions from disk if not in memory, reducing "session not found" errors. - Enhanced error messages for non-existent sessions, providing clearer diagnostics. - Updated CodexProvider and OpencodeProvider to improve error handling and messaging. - Refactored various routes to use async/await for better readability and error handling. - Added event emission for merge and stash operations in the MergeService and StashService. - Cleaned up error messages in AgentExecutor to remove redundant prefixes and ANSI codes for better clarity. --- apps/server/src/index.ts | 11 +- apps/server/src/providers/codex-provider.ts | 3 +- apps/server/src/providers/codex-sdk-client.ts | 6 +- .../server/src/providers/opencode-provider.ts | 283 +++++++++++++++++- .../server/src/routes/agent/routes/history.ts | 2 +- .../src/routes/agent/routes/queue-list.ts | 2 +- apps/server/src/routes/agent/routes/send.ts | 10 +- apps/server/src/routes/worktree/index.ts | 11 +- .../src/routes/worktree/routes/merge.ts | 12 +- .../src/routes/worktree/routes/stash-apply.ts | 5 +- apps/server/src/services/agent-executor.ts | 43 ++- apps/server/src/services/agent-service.ts | 251 ++++++++++++++-- apps/server/src/services/auto-mode/facade.ts | 19 +- apps/server/src/services/merge-service.ts | 15 +- apps/server/src/services/stash-service.ts | 24 +- .../src/services/worktree-branch-service.ts | 40 ++- .../tests/unit/services/agent-service.test.ts | 121 +++++++- .../src/components/ui/truncated-file-path.tsx | 4 +- .../dialogs/commit-worktree-dialog.tsx | 4 + .../board-view/dialogs/git-pull-dialog.tsx | 30 +- .../dialogs/stash-changes-dialog.tsx | 72 +++-- .../board-view/hooks/use-board-actions.ts | 7 +- 22 files changed, 827 insertions(+), 148 deletions(-) diff --git a/apps/server/src/index.ts b/apps/server/src/index.ts index 6cf55bb7..0acea6c9 100644 --- a/apps/server/src/index.ts +++ b/apps/server/src/index.ts @@ -665,8 +665,15 @@ terminalWss.on('connection', (ws: WebSocket, req: import('http').IncomingMessage // Check if session exists const session = terminalService.getSession(sessionId); if (!session) { - logger.info(`Session ${sessionId} not found`); - ws.close(4004, 'Session not found'); + logger.warn( + `Terminal session ${sessionId} not found. ` + + `The session may have exited, been deleted, or was never created. ` + + `Active terminal sessions: ${terminalService.getSessionCount()}` + ); + ws.close( + 4004, + 'Session not found. The terminal session may have expired or been closed. Please create a new terminal.' + ); return; } diff --git a/apps/server/src/providers/codex-provider.ts b/apps/server/src/providers/codex-provider.ts index 8c819d9d..178760c6 100644 --- a/apps/server/src/providers/codex-provider.ts +++ b/apps/server/src/providers/codex-provider.ts @@ -876,7 +876,7 @@ export class CodexProvider extends BaseProvider { if (errorLower.includes('rate limit')) { enhancedError = `${errorText}\n\nTip: You're being rate limited. Try reducing concurrent tasks or waiting a few minutes before retrying.`; } else if (errorLower.includes('authentication') || errorLower.includes('unauthorized')) { - enhancedError = `${errorText}\n\nTip: Check that your OPENAI_API_KEY is set correctly or run 'codex auth login' to authenticate.`; + enhancedError = `${errorText}\n\nTip: Check that your OPENAI_API_KEY is set correctly or run 'codex login' to authenticate.`; } else if ( errorLower.includes('does not exist') || errorLower.includes('do not have access') || @@ -1058,7 +1058,6 @@ export class CodexProvider extends BaseProvider { async detectInstallation(): Promise { const cliPath = await findCodexCliPath(); const hasApiKey = Boolean(await resolveOpenAiApiKey()); - await getCodexAuthIndicators(); const installed = !!cliPath; let version = ''; diff --git a/apps/server/src/providers/codex-sdk-client.ts b/apps/server/src/providers/codex-sdk-client.ts index e477b3bb..ced24510 100644 --- a/apps/server/src/providers/codex-sdk-client.ts +++ b/apps/server/src/providers/codex-sdk-client.ts @@ -179,7 +179,9 @@ export async function* executeCodexSdkQuery( let combinedMessage = buildSdkErrorMessage(errorInfo.message, userMessage); // Enhance error messages with actionable tips for common Codex issues - const errorLower = errorInfo.message.toLowerCase(); + // Normalize inputs to avoid crashes from nullish values + const errorLower = (errorInfo?.message ?? '').toLowerCase(); + const modelLabel = options?.model ?? ''; if ( errorLower.includes('does not exist') || @@ -188,7 +190,7 @@ export async function* executeCodexSdkQuery( ) { // Model not found - provide helpful guidance combinedMessage += - `\n\nTip: The model '${options.model}' may not be available on your OpenAI plan. ` + + `\n\nTip: The model '${modelLabel}' may not be available on your OpenAI plan. ` + `Some models (like gpt-5.3-codex) require a ChatGPT Pro/Plus subscription and OAuth login via 'codex login'. ` + `Try using a different model (e.g., gpt-5.1 or gpt-5.2), or authenticate with 'codex login' instead of an API key.`; } else if ( diff --git a/apps/server/src/providers/opencode-provider.ts b/apps/server/src/providers/opencode-provider.ts index d2fa13d9..07068b08 100644 --- a/apps/server/src/providers/opencode-provider.ts +++ b/apps/server/src/providers/opencode-provider.ts @@ -192,6 +192,28 @@ export interface OpenCodeToolErrorEvent extends OpenCodeBaseEvent { part?: OpenCodePart & { error: string }; } +/** + * Tool use event - The actual format emitted by OpenCode CLI when a tool is invoked. + * Contains the tool name, call ID, and the complete state (input, output, status). + * Note: OpenCode CLI emits 'tool_use' (not 'tool_call') as the event type. + */ +export interface OpenCodeToolUseEvent extends OpenCodeBaseEvent { + type: 'tool_use'; + part: OpenCodePart & { + type: 'tool'; + callID?: string; + tool?: string; + state?: { + status?: string; + input?: unknown; + output?: string; + title?: string; + metadata?: unknown; + time?: { start: number; end: number }; + }; + }; +} + /** * Union type of all OpenCode stream events */ @@ -200,6 +222,7 @@ export type OpenCodeStreamEvent = | OpenCodeStepStartEvent | OpenCodeStepFinishEvent | OpenCodeToolCallEvent + | OpenCodeToolUseEvent | OpenCodeToolResultEvent | OpenCodeErrorEvent | OpenCodeToolErrorEvent; @@ -311,8 +334,8 @@ export class OpencodeProvider extends CliProvider { * Arguments built: * - 'run' subcommand for executing queries * - '--format', 'json' for JSONL streaming output - * - '-c', '' for working directory (using opencode's -c flag) * - '--model', '' for model selection (if specified) + * - '--session', '' for continuing an existing session (if sdkSessionId is set) * * The prompt is passed via stdin (piped) to avoid shell escaping issues. * OpenCode CLI automatically reads from stdin when input is piped. @@ -326,6 +349,14 @@ export class OpencodeProvider extends CliProvider { // Add JSON output format for JSONL parsing (not 'stream-json') args.push('--format', 'json'); + // Handle session resumption for conversation continuity. + // The opencode CLI supports `--session ` to continue an existing session. + // The sdkSessionId is captured from the sessionID field in previous stream events + // and persisted by AgentService for use in follow-up messages. + if (options.sdkSessionId) { + args.push('--session', options.sdkSessionId); + } + // Handle model selection // Convert canonical prefix format (opencode-xxx) to CLI slash format (opencode/xxx) // OpenCode CLI expects provider/model format (e.g., 'opencode/big-model') @@ -398,15 +429,198 @@ export class OpencodeProvider extends CliProvider { return subprocessOptions; } + /** + * Check if an error message indicates a session-not-found condition. + * + * Centralizes the pattern matching for session errors to avoid duplication. + * Strips ANSI escape codes first since opencode CLI uses colored stderr output + * (e.g. "\x1b[91m\x1b[1mError: \x1b[0mSession not found"). + * + * IMPORTANT: Patterns must be specific enough to avoid false positives. + * Generic patterns like "notfounderror" or "resource not found" match + * non-session errors (e.g. "ProviderModelNotFoundError") which would + * trigger unnecessary retries that fail identically, producing confusing + * error messages like "OpenCode session could not be created". + * + * @param errorText - Raw error text (may contain ANSI codes) + * @returns true if the error indicates the session was not found + */ + private static isSessionNotFoundError(errorText: string): boolean { + const cleaned = OpencodeProvider.stripAnsiCodes(errorText).toLowerCase(); + + // Explicit session-related phrases — high confidence + if ( + cleaned.includes('session not found') || + cleaned.includes('session does not exist') || + cleaned.includes('invalid session') || + cleaned.includes('session expired') || + cleaned.includes('no such session') + ) { + return true; + } + + // Generic "NotFoundError" / "resource not found" are only session errors + // when the message also references a session path or session ID. + // Without this guard, errors like "ProviderModelNotFoundError" or + // "Resource not found: /path/to/config.json" would false-positive. + if (cleaned.includes('notfounderror') || cleaned.includes('resource not found')) { + return cleaned.includes('/session/') || cleaned.includes('session'); + } + + return false; + } + + /** + * Strip ANSI escape codes from a string. + * + * The OpenCode CLI uses colored stderr output (e.g. "\x1b[91m\x1b[1mError: \x1b[0m"). + * These escape codes render as garbled text like "[91m[1mError: [0m" in the UI + * when passed through as-is. This utility removes them so error messages are + * clean and human-readable. + */ + private static stripAnsiCodes(text: string): string { + return text.replace(/\x1b\[[0-9;]*m/g, ''); + } + + /** + * Clean a CLI error message for display. + * + * Strips ANSI escape codes AND removes the redundant "Error: " prefix that + * the OpenCode CLI prepends to error messages in its colored stderr output + * (e.g. "\x1b[91m\x1b[1mError: \x1b[0mSession not found" → "Session not found"). + * + * Without this, consumers that wrap the message in their own "Error: " prefix + * (like AgentService or AgentExecutor) produce garbled double-prefixed output: + * "Error: Error: Session not found". + */ + private static cleanErrorMessage(text: string): string { + let cleaned = OpencodeProvider.stripAnsiCodes(text).trim(); + // Remove leading "Error: " prefix (case-insensitive) if present. + // The CLI formats errors as: \x1b[91m\x1b[1mError: \x1b[0m + // After ANSI stripping this becomes: "Error: " + cleaned = cleaned.replace(/^Error:\s*/i, '').trim(); + return cleaned || text; + } + + /** + * Execute a query with automatic session resumption fallback. + * + * When a sdkSessionId is provided, the CLI receives `--session `. + * If the session no longer exists on disk the CLI will fail with a + * "NotFoundError" / "Resource not found" / "Session not found" error. + * + * The opencode CLI writes this to **stderr** and exits non-zero. + * `spawnJSONLProcess` collects stderr and **yields** it as + * `{ type: 'error', error: }` — it is NOT thrown. + * After `normalizeEvent`, the error becomes a yielded `ProviderMessage` + * with `type: 'error'`. A simple try/catch therefore cannot intercept it. + * + * This override iterates the parent stream, intercepts yielded error + * messages that match the session-not-found pattern, and retries the + * entire query WITHOUT the `--session` flag so a fresh session is started. + * + * Session-not-found retry is ONLY attempted when `sdkSessionId` is set. + * Without the `--session` flag the CLI always creates a fresh session, so + * retrying without it would be identical to the first attempt and would + * fail the same way — producing a confusing "session could not be created" + * message for what is actually a different error (model not found, auth + * failure, etc.). + * + * All error messages (session or not) are cleaned of ANSI codes and the + * CLI's redundant "Error: " prefix before being yielded to consumers. + * + * After a successful retry, the consumer (AgentService) will receive a new + * session_id from the fresh stream events, which it persists to metadata — + * replacing the stale sdkSessionId and preventing repeated failures. + */ + async *executeQuery(options: ExecuteOptions): AsyncGenerator { + // When no sdkSessionId is set, there is nothing to "retry without" — just + // stream normally and clean error messages as they pass through. + if (!options.sdkSessionId) { + for await (const msg of super.executeQuery(options)) { + // Clean error messages so consumers don't get ANSI or double "Error:" prefix + if (msg.type === 'error' && msg.error && typeof msg.error === 'string') { + msg.error = OpencodeProvider.cleanErrorMessage(msg.error); + } + yield msg; + } + return; + } + + // sdkSessionId IS set — the CLI will receive `--session `. + // If that session no longer exists, intercept the error and retry fresh. + const buffered: ProviderMessage[] = []; + let sessionError = false; + + try { + for await (const msg of super.executeQuery(options)) { + if (msg.type === 'error') { + const errorText = msg.error || ''; + if (OpencodeProvider.isSessionNotFoundError(errorText)) { + sessionError = true; + opencodeLogger.info( + `OpenCode session error detected (session "${options.sdkSessionId}") ` + + `— retrying without --session to start fresh` + ); + break; // stop consuming the failed stream + } + + // Non-session error — clean and buffer + if (msg.error && typeof msg.error === 'string') { + msg.error = OpencodeProvider.cleanErrorMessage(msg.error); + } + } + + buffered.push(msg); + } + } catch (error) { + // Also handle thrown exceptions (e.g. from mapError in cli-provider) + const errMsg = error instanceof Error ? error.message : String(error); + if (OpencodeProvider.isSessionNotFoundError(errMsg)) { + sessionError = true; + opencodeLogger.info( + `OpenCode session error detected (thrown, session "${options.sdkSessionId}") ` + + `— retrying without --session to start fresh` + ); + } else { + throw error; + } + } + + if (sessionError) { + // Retry the entire query without the stale session ID. + const retryOptions = { ...options, sdkSessionId: undefined }; + opencodeLogger.info('Retrying OpenCode query without --session flag...'); + + // Stream the retry directly to the consumer. + // If the retry also fails, it's a genuine error (not session-related) + // and should be surfaced as-is rather than masked with a misleading + // "session could not be created" message. + for await (const retryMsg of super.executeQuery(retryOptions)) { + if (retryMsg.type === 'error' && retryMsg.error && typeof retryMsg.error === 'string') { + retryMsg.error = OpencodeProvider.cleanErrorMessage(retryMsg.error); + } + yield retryMsg; + } + } else { + // No session error — flush buffered messages to the consumer + for (const msg of buffered) { + yield msg; + } + } + } + /** * Normalize a raw CLI event to ProviderMessage format * * Maps OpenCode event types to the standard ProviderMessage structure: * - text -> type: 'assistant', content with type: 'text' * - step_start -> null (informational, no message needed) - * - step_finish with reason 'stop' -> type: 'result', subtype: 'success' + * - step_finish with reason 'stop'/'end_turn' -> type: 'result', subtype: 'success' + * - step_finish with reason 'tool-calls' -> null (intermediate step, not final) * - step_finish with error -> type: 'error' - * - tool_call -> type: 'assistant', content with type: 'tool_use' + * - tool_use -> type: 'assistant', content with type: 'tool_use' (OpenCode CLI format) + * - tool_call -> type: 'assistant', content with type: 'tool_use' (legacy format) * - tool_result -> type: 'assistant', content with type: 'tool_result' * - error -> type: 'error' * @@ -472,7 +686,14 @@ export class OpencodeProvider extends CliProvider { }; } - // Successful completion (reason: 'stop' or 'end_turn') + // Intermediate step completion (reason: 'tool-calls') — the agent loop + // is continuing because the model requested tool calls. Skip these so + // consumers don't mistake them for final results. + if (finishEvent.part?.reason === 'tool-calls') { + return null; + } + + // Final completion (reason: 'stop' or 'end_turn') return { type: 'result', subtype: 'success', @@ -494,6 +715,53 @@ export class OpencodeProvider extends CliProvider { }; } + // OpenCode CLI emits 'tool_use' events (not 'tool_call') when the model invokes a tool. + // The event format includes the tool name, call ID, and state with input/output. + // Handle both 'tool_use' (actual CLI format) and 'tool_call' (legacy/alternative) for robustness. + case 'tool_use': { + const toolUseEvent = openCodeEvent as OpenCodeBaseEvent; + const part = toolUseEvent.part as OpenCodePart & { + callID?: string; + tool?: string; + state?: { + status?: string; + input?: unknown; + output?: string; + }; + }; + + // Generate a tool use ID if not provided + const toolUseId = part?.callID || part?.call_id || generateToolUseId(); + const toolName = part?.tool || part?.name || 'unknown'; + + const content: ContentBlock[] = [ + { + type: 'tool_use', + name: toolName, + tool_use_id: toolUseId, + input: part?.state?.input || part?.args, + }, + ]; + + // If the tool has already completed (state.status === 'completed'), also emit the result + if (part?.state?.status === 'completed' && part?.state?.output) { + content.push({ + type: 'tool_result', + tool_use_id: toolUseId, + content: part.state.output, + }); + } + + return { + type: 'assistant', + session_id: toolUseEvent.sessionID, + message: { + role: 'assistant', + content, + }, + }; + } + case 'tool_call': { const toolEvent = openCodeEvent as OpenCodeToolCallEvent; @@ -560,6 +828,13 @@ export class OpencodeProvider extends CliProvider { errorMessage = errorEvent.part.error; } + // Clean error messages: strip ANSI escape codes AND the redundant "Error: " + // prefix the CLI adds. The OpenCode CLI outputs colored stderr like: + // \x1b[91m\x1b[1mError: \x1b[0mSession not found + // Without cleaning, consumers that wrap in their own "Error: " prefix + // produce "Error: Error: Session not found". + errorMessage = OpencodeProvider.cleanErrorMessage(errorMessage); + return { type: 'error', session_id: errorEvent.sessionID, diff --git a/apps/server/src/routes/agent/routes/history.ts b/apps/server/src/routes/agent/routes/history.ts index 0859a142..e11578d7 100644 --- a/apps/server/src/routes/agent/routes/history.ts +++ b/apps/server/src/routes/agent/routes/history.ts @@ -16,7 +16,7 @@ export function createHistoryHandler(agentService: AgentService) { return; } - const result = agentService.getHistory(sessionId); + const result = await agentService.getHistory(sessionId); res.json(result); } catch (error) { logError(error, 'Get history failed'); diff --git a/apps/server/src/routes/agent/routes/queue-list.ts b/apps/server/src/routes/agent/routes/queue-list.ts index 1096c701..7299e871 100644 --- a/apps/server/src/routes/agent/routes/queue-list.ts +++ b/apps/server/src/routes/agent/routes/queue-list.ts @@ -19,7 +19,7 @@ export function createQueueListHandler(agentService: AgentService) { return; } - const result = agentService.getQueue(sessionId); + const result = await agentService.getQueue(sessionId); res.json(result); } catch (error) { logError(error, 'List queue failed'); diff --git a/apps/server/src/routes/agent/routes/send.ts b/apps/server/src/routes/agent/routes/send.ts index 15e97f63..4f6e527c 100644 --- a/apps/server/src/routes/agent/routes/send.ts +++ b/apps/server/src/routes/agent/routes/send.ts @@ -53,7 +53,15 @@ export function createSendHandler(agentService: AgentService) { thinkingLevel, }) .catch((error) => { - logger.error('Background error in sendMessage():', error); + const errorMsg = (error as Error).message || 'Unknown error'; + logger.error(`Background error in sendMessage() for session ${sessionId}:`, errorMsg); + + // Emit error via WebSocket so the UI is notified even though + // the HTTP response already returned 200. This is critical for + // session-not-found errors where sendMessage() throws before it + // can emit its own error event (no in-memory session to emit from). + agentService.emitSessionError(sessionId, errorMsg); + logError(error, 'Send message failed (background)'); }); diff --git a/apps/server/src/routes/worktree/index.ts b/apps/server/src/routes/worktree/index.ts index b2af8ae0..09b291de 100644 --- a/apps/server/src/routes/worktree/index.ts +++ b/apps/server/src/routes/worktree/index.ts @@ -80,7 +80,7 @@ export function createWorktreeRoutes( '/merge', validatePathParams('projectPath'), requireValidProject, - createMergeHandler() + createMergeHandler(events) ); router.post( '/create', @@ -126,7 +126,12 @@ export function createWorktreeRoutes( requireValidWorktree, createListBranchesHandler() ); - router.post('/switch-branch', requireValidWorktree, createSwitchBranchHandler(events)); + router.post( + '/switch-branch', + validatePathParams('worktreePath'), + requireValidWorktree, + createSwitchBranchHandler(events) + ); router.post('/open-in-editor', validatePathParams('worktreePath'), createOpenInEditorHandler()); router.post( '/open-in-terminal', @@ -230,7 +235,7 @@ export function createWorktreeRoutes( '/stash-apply', validatePathParams('worktreePath'), requireGitRepoOnly, - createStashApplyHandler() + createStashApplyHandler(events) ); router.post( '/stash-drop', diff --git a/apps/server/src/routes/worktree/routes/merge.ts b/apps/server/src/routes/worktree/routes/merge.ts index 3405d413..9f8b3bb4 100644 --- a/apps/server/src/routes/worktree/routes/merge.ts +++ b/apps/server/src/routes/worktree/routes/merge.ts @@ -9,9 +9,10 @@ import type { Request, Response } from 'express'; import { getErrorMessage, logError } from '../common.js'; +import type { EventEmitter } from '../../../lib/events.js'; import { performMerge } from '../../../services/merge-service.js'; -export function createMergeHandler() { +export function createMergeHandler(events: EventEmitter) { return async (req: Request, res: Response): Promise => { try { const { projectPath, branchName, worktreePath, targetBranch, options } = req.body as { @@ -34,7 +35,14 @@ export function createMergeHandler() { const mergeTo = targetBranch || 'main'; // Delegate all merge logic to the service - const result = await performMerge(projectPath, branchName, worktreePath, mergeTo, options); + const result = await performMerge( + projectPath, + branchName, + worktreePath, + mergeTo, + options, + events + ); if (!result.success) { if (result.hasConflicts) { diff --git a/apps/server/src/routes/worktree/routes/stash-apply.ts b/apps/server/src/routes/worktree/routes/stash-apply.ts index 15137f5e..615905cb 100644 --- a/apps/server/src/routes/worktree/routes/stash-apply.ts +++ b/apps/server/src/routes/worktree/routes/stash-apply.ts @@ -11,10 +11,11 @@ */ import type { Request, Response } from 'express'; +import type { EventEmitter } from '../../../lib/events.js'; import { getErrorMessage, logError } from '../common.js'; import { applyOrPop } from '../../../services/stash-service.js'; -export function createStashApplyHandler() { +export function createStashApplyHandler(events: EventEmitter) { return async (req: Request, res: Response): Promise => { try { const { worktreePath, stashIndex, pop } = req.body as { @@ -50,7 +51,7 @@ export function createStashApplyHandler() { } // Delegate all stash apply/pop logic to the service - const result = await applyOrPop(worktreePath, idx, { pop }); + const result = await applyOrPop(worktreePath, idx, { pop }, events); if (!result.success) { logError(new Error(result.error ?? 'Stash apply failed'), 'Stash apply failed'); diff --git a/apps/server/src/services/agent-executor.ts b/apps/server/src/services/agent-executor.ts index e005b339..7a0967eb 100644 --- a/apps/server/src/services/agent-executor.ts +++ b/apps/server/src/services/agent-executor.ts @@ -255,7 +255,15 @@ export class AgentExecutor { } } } else if (msg.type === 'error') { - throw new Error(msg.error || 'Unknown error'); + // Clean the error: strip ANSI codes and the redundant "Error: " prefix + // that CLI providers add. Without this, wrapping in new Error() produces + // "Error: Error: Session not found" (double-prefixed). + const cleanedError = + (msg.error || 'Unknown error') + .replace(/\x1b\[[0-9;]*m/g, '') + .replace(/^Error:\s*/i, '') + .trim() || 'Unknown error'; + throw new Error(cleanedError); } else if (msg.type === 'result' && msg.subtype === 'success') scheduleWrite(); } await writeToFile(); @@ -390,9 +398,15 @@ export class AgentExecutor { input: b.input, }); } - } else if (msg.type === 'error') - throw new Error(msg.error || `Error during task ${task.id}`); - else if (msg.type === 'result' && msg.subtype === 'success') { + } else if (msg.type === 'error') { + // Clean the error: strip ANSI codes and redundant "Error: " prefix + const cleanedError = + (msg.error || `Error during task ${task.id}`) + .replace(/\x1b\[[0-9;]*m/g, '') + .replace(/^Error:\s*/i, '') + .trim() || `Error during task ${task.id}`; + throw new Error(cleanedError); + } else if (msg.type === 'result' && msg.subtype === 'success') { taskOutput += msg.result || ''; responseText += msg.result || ''; } @@ -556,7 +570,14 @@ export class AgentExecutor { content: b.text, }); } - if (msg.type === 'error') throw new Error(msg.error || 'Error during plan revision'); + if (msg.type === 'error') { + const cleanedError = + (msg.error || 'Error during plan revision') + .replace(/\x1b\[[0-9;]*m/g, '') + .replace(/^Error:\s*/i, '') + .trim() || 'Error during plan revision'; + throw new Error(cleanedError); + } if (msg.type === 'result' && msg.subtype === 'success') revText += msg.result || ''; } const mi = revText.indexOf('[SPEC_GENERATED]'); @@ -674,9 +695,15 @@ export class AgentExecutor { input: b.input, }); } - else if (msg.type === 'error') - throw new Error(msg.error || 'Unknown error during implementation'); - else if (msg.type === 'result' && msg.subtype === 'success') responseText += msg.result || ''; + else if (msg.type === 'error') { + const cleanedError = + (msg.error || 'Unknown error during implementation') + .replace(/\x1b\[[0-9;]*m/g, '') + .replace(/^Error:\s*/i, '') + .trim() || 'Unknown error during implementation'; + throw new Error(cleanedError); + } else if (msg.type === 'result' && msg.subtype === 'success') + responseText += msg.result || ''; } return { responseText }; } diff --git a/apps/server/src/services/agent-service.ts b/apps/server/src/services/agent-service.ts index 663a1383..bae39db2 100644 --- a/apps/server/src/services/agent-service.ts +++ b/apps/server/src/services/agent-service.ts @@ -106,32 +106,26 @@ export class AgentService { sessionId: string; workingDirectory?: string; }) { - if (!this.sessions.has(sessionId)) { - const messages = await this.loadSession(sessionId); - const metadata = await this.loadMetadata(); - const sessionMetadata = metadata[sessionId]; - - // Determine the effective working directory + // ensureSession handles loading from disk if not in memory. + // For startConversation, we always want to create a session even if + // metadata doesn't exist yet (new session), so we fall back to creating one. + let session = await this.ensureSession(sessionId, workingDirectory); + if (!session) { + // Session doesn't exist on disk either — create a fresh in-memory session. const effectiveWorkingDirectory = workingDirectory || process.cwd(); const resolvedWorkingDirectory = path.resolve(effectiveWorkingDirectory); - - // Validate that the working directory is allowed using centralized validation validateWorkingDirectory(resolvedWorkingDirectory); - // Load persisted queue - const promptQueue = await this.loadQueueState(sessionId); - - this.sessions.set(sessionId, { - messages, + session = { + messages: [], isRunning: false, abortController: null, workingDirectory: resolvedWorkingDirectory, - sdkSessionId: sessionMetadata?.sdkSessionId, // Load persisted SDK session ID - promptQueue, - }); + promptQueue: [], + }; + this.sessions.set(sessionId, session); } - const session = this.sessions.get(sessionId)!; return { success: true, messages: session.messages, @@ -139,6 +133,90 @@ export class AgentService { }; } + /** + * Ensure a session is loaded into memory. + * + * Sessions may exist on disk (in metadata and session files) but not be + * present in the in-memory Map — for example after a server restart, or + * when a client calls sendMessage before explicitly calling startConversation. + * + * This helper transparently loads the session from disk when it is missing + * from memory, eliminating "session not found" errors for sessions that + * were previously created but not yet initialized in memory. + * + * If both metadata and session files are missing, the session truly doesn't + * exist. A detailed diagnostic log is emitted so developers can track down + * how the invalid session ID was generated. + * + * @returns The in-memory Session object, or null if the session doesn't exist at all + */ + private async ensureSession( + sessionId: string, + workingDirectory?: string + ): Promise { + const existing = this.sessions.get(sessionId); + if (existing) { + return existing; + } + + // Try to load from disk — the session may have been created earlier + // (e.g. via createSession) but never initialized in memory. + let metadata: Record; + let messages: Message[]; + try { + [metadata, messages] = await Promise.all([this.loadMetadata(), this.loadSession(sessionId)]); + } catch (error) { + // Disk read failure should not be treated as "session not found" — + // it's a transient I/O problem. Log and return null so callers can + // surface an appropriate error message. + this.logger.error( + `Failed to load session ${sessionId} from disk (I/O error — NOT a missing session):`, + error + ); + return null; + } + + const sessionMetadata = metadata[sessionId]; + + // If there's no metadata AND no persisted messages, the session truly doesn't exist. + // Log diagnostic info to help track down how we ended up with an invalid session ID. + if (!sessionMetadata && messages.length === 0) { + this.logger.warn( + `Session "${sessionId}" not found: no metadata and no persisted messages. ` + + `This can happen when a session ID references a deleted/expired session, ` + + `or when the server restarted and the session was never persisted to disk. ` + + `Available session IDs in metadata: [${Object.keys(metadata).slice(0, 10).join(', ')}${Object.keys(metadata).length > 10 ? '...' : ''}]` + ); + return null; + } + + const effectiveWorkingDirectory = + workingDirectory || sessionMetadata?.workingDirectory || process.cwd(); + const resolvedWorkingDirectory = path.resolve(effectiveWorkingDirectory); + + // Validate that the working directory is allowed using centralized validation + validateWorkingDirectory(resolvedWorkingDirectory); + + // Load persisted queue + const promptQueue = await this.loadQueueState(sessionId); + + const session: Session = { + messages, + isRunning: false, + abortController: null, + workingDirectory: resolvedWorkingDirectory, + sdkSessionId: sessionMetadata?.sdkSessionId, + promptQueue, + }; + + this.sessions.set(sessionId, session); + this.logger.info( + `Auto-initialized session ${sessionId} from disk ` + + `(${messages.length} messages, sdkSessionId: ${sessionMetadata?.sdkSessionId ? 'present' : 'none'})` + ); + return session; + } + /** * Send a message to the agent and stream responses */ @@ -159,10 +237,18 @@ export class AgentService { thinkingLevel?: ThinkingLevel; reasoningEffort?: ReasoningEffort; }) { - const session = this.sessions.get(sessionId); + const session = await this.ensureSession(sessionId, workingDirectory); if (!session) { - this.logger.error('ERROR: Session not found:', sessionId); - throw new Error(`Session ${sessionId} not found`); + this.logger.error( + `Session not found: ${sessionId}. ` + + `The session may have been deleted, never created, or lost after a server restart. ` + + `In-memory sessions: ${this.sessions.size}, requested ID: ${sessionId}` + ); + throw new Error( + `Session ${sessionId} not found. ` + + `The session may have been deleted or expired. ` + + `Please create a new session and try again.` + ); } if (session.isRunning) { @@ -439,8 +525,13 @@ export class AgentService { const toolUses: Array<{ name: string; input: unknown }> = []; for await (const msg of stream) { - // Capture SDK session ID from any message and persist it - if (msg.session_id && !session.sdkSessionId) { + // Capture SDK session ID from any message and persist it. + // Update when: + // - No session ID set yet (first message in a new session) + // - The provider returned a *different* session ID (e.g., after a + // "Session not found" recovery where the provider started a fresh + // session — the stale ID must be replaced with the new one) + if (msg.session_id && msg.session_id !== session.sdkSessionId) { session.sdkSessionId = msg.session_id; // Persist the SDK session ID to ensure conversation continuity across server restarts await this.updateSession(sessionId, { sdkSessionId: msg.session_id }); @@ -503,12 +594,43 @@ export class AgentService { // streamed error messages instead of throwing. Handle these here so the // Agent Runner UX matches the Claude/Cursor behavior without changing // their provider implementations. - const rawErrorText = + + // Clean error text: strip ANSI escape codes and the redundant "Error: " + // prefix that CLI providers (especially OpenCode) add to stderr output. + // The OpenCode provider strips these in normalizeEvent/executeQuery, but + // we also strip here as a defense-in-depth measure. + // + // Without stripping the "Error: " prefix, the wrapping at line ~647 + // (`content: \`Error: ${enhancedText}\``) produces double-prefixed text: + // "Error: Error: Session not found" — confusing for the user. + const rawMsgError = (typeof msg.error === 'string' && msg.error.trim()) || 'Unexpected error from provider during agent execution.'; + let rawErrorText = rawMsgError.replace(/\x1b\[[0-9;]*m/g, '').trim() || rawMsgError; + // Remove the CLI's "Error: " prefix to prevent double-wrapping + rawErrorText = rawErrorText.replace(/^Error:\s*/i, '').trim() || rawErrorText; const errorInfo = classifyError(new Error(rawErrorText)); + // Detect provider-side session errors and proactively clear the stale + // sdkSessionId so the next attempt starts a fresh provider session. + // This handles providers that don't have built-in session recovery + // (unlike OpenCode which auto-retries without the session flag). + const errorLower = rawErrorText.toLowerCase(); + if ( + session.sdkSessionId && + (errorLower.includes('session not found') || + errorLower.includes('session expired') || + errorLower.includes('invalid session') || + errorLower.includes('no such session')) + ) { + this.logger.info( + `Clearing stale sdkSessionId for session ${sessionId} after provider session error` + ); + session.sdkSessionId = undefined; + await this.clearSdkSessionId(sessionId); + } + // Keep the provider-supplied text intact (Codex already includes helpful tips), // only add a small rate-limit hint when we can detect it. const enhancedText = errorInfo.isRateLimit @@ -569,13 +691,36 @@ export class AgentService { this.logger.error('Error:', error); + // Strip ANSI escape codes and the "Error: " prefix from thrown error + // messages so the UI receives clean text without double-prefixing. + let rawThrownMsg = ((error as Error).message || '').replace(/\x1b\[[0-9;]*m/g, '').trim(); + rawThrownMsg = rawThrownMsg.replace(/^Error:\s*/i, '').trim() || rawThrownMsg; + const thrownErrorMsg = rawThrownMsg.toLowerCase(); + + // Check if the thrown error is a provider-side session error. + // Clear the stale sdkSessionId so the next retry starts fresh. + if ( + session.sdkSessionId && + (thrownErrorMsg.includes('session not found') || + thrownErrorMsg.includes('session expired') || + thrownErrorMsg.includes('invalid session') || + thrownErrorMsg.includes('no such session')) + ) { + this.logger.info( + `Clearing stale sdkSessionId for session ${sessionId} after thrown session error` + ); + session.sdkSessionId = undefined; + await this.clearSdkSessionId(sessionId); + } + session.isRunning = false; session.abortController = null; + const cleanErrorMsg = rawThrownMsg || (error as Error).message; const errorMessage: Message = { id: this.generateId(), role: 'assistant', - content: `Error: ${(error as Error).message}`, + content: `Error: ${cleanErrorMsg}`, timestamp: new Date().toISOString(), isError: true, }; @@ -585,7 +730,7 @@ export class AgentService { this.emitAgentEvent(sessionId, { type: 'error', - error: (error as Error).message, + error: cleanErrorMsg, message: errorMessage, }); @@ -596,8 +741,8 @@ export class AgentService { /** * Get conversation history */ - getHistory(sessionId: string) { - const session = this.sessions.get(sessionId); + async getHistory(sessionId: string) { + const session = await this.ensureSession(sessionId); if (!session) { return { success: false, error: 'Session not found' }; } @@ -613,7 +758,7 @@ export class AgentService { * Stop current agent execution */ async stopExecution(sessionId: string) { - const session = this.sessions.get(sessionId); + const session = await this.ensureSession(sessionId); if (!session) { return { success: false, error: 'Session not found' }; } @@ -635,9 +780,16 @@ export class AgentService { if (session) { session.messages = []; session.isRunning = false; + session.sdkSessionId = undefined; // Clear stale provider session ID to prevent "Session not found" errors await this.saveSession(sessionId, []); } + // Clear the sdkSessionId from persisted metadata so it doesn't get + // reloaded by ensureSession() after a server restart. + // This prevents "Session not found" errors when the provider-side session + // no longer exists (e.g., OpenCode CLI sessions expire on disk). + await this.clearSdkSessionId(sessionId); + return { success: true }; } @@ -794,6 +946,23 @@ export class AgentService { return true; } + /** + * Clear the sdkSessionId from persisted metadata. + * + * This removes the provider-side session ID so that the next message + * starts a fresh provider session instead of trying to resume a stale one. + * Prevents "Session not found" errors from CLI providers like OpenCode + * when the provider-side session has been deleted or expired. + */ + async clearSdkSessionId(sessionId: string): Promise { + const metadata = await this.loadMetadata(); + if (metadata[sessionId] && metadata[sessionId].sdkSessionId) { + delete metadata[sessionId].sdkSessionId; + metadata[sessionId].updatedAt = new Date().toISOString(); + await this.saveMetadata(metadata); + } + } + // Queue management methods /** @@ -808,7 +977,7 @@ export class AgentService { thinkingLevel?: ThinkingLevel; } ): Promise<{ success: boolean; queuedPrompt?: QueuedPrompt; error?: string }> { - const session = this.sessions.get(sessionId); + const session = await this.ensureSession(sessionId); if (!session) { return { success: false, error: 'Session not found' }; } @@ -837,8 +1006,10 @@ export class AgentService { /** * Get the current queue for a session */ - getQueue(sessionId: string): { success: boolean; queue?: QueuedPrompt[]; error?: string } { - const session = this.sessions.get(sessionId); + async getQueue( + sessionId: string + ): Promise<{ success: boolean; queue?: QueuedPrompt[]; error?: string }> { + const session = await this.ensureSession(sessionId); if (!session) { return { success: false, error: 'Session not found' }; } @@ -852,7 +1023,7 @@ export class AgentService { sessionId: string, promptId: string ): Promise<{ success: boolean; error?: string }> { - const session = this.sessions.get(sessionId); + const session = await this.ensureSession(sessionId); if (!session) { return { success: false, error: 'Session not found' }; } @@ -877,7 +1048,7 @@ export class AgentService { * Clear all prompts from the queue */ async clearQueue(sessionId: string): Promise<{ success: boolean; error?: string }> { - const session = this.sessions.get(sessionId); + const session = await this.ensureSession(sessionId); if (!session) { return { success: false, error: 'Session not found' }; } @@ -960,10 +1131,24 @@ export class AgentService { } } + /** + * Emit an event to the agent stream (private, used internally). + */ private emitAgentEvent(sessionId: string, data: Record): void { this.events.emit('agent:stream', { sessionId, ...data }); } + /** + * Emit an error event for a session. + * + * Public method so that route handlers can surface errors to the UI + * even when sendMessage() throws before it can emit its own error event + * (e.g., when the session is not found and no in-memory session exists). + */ + emitSessionError(sessionId: string, error: string): void { + this.events.emit('agent:stream', { sessionId, type: 'error', error }); + } + private async getSystemPrompt(): Promise { // Load from settings (no caching - allows hot reload of custom prompts) const prompts = await getPromptCustomization(this.settingsService, '[AgentService]'); diff --git a/apps/server/src/services/auto-mode/facade.ts b/apps/server/src/services/auto-mode/facade.ts index cb948d62..8e4b1d73 100644 --- a/apps/server/src/services/auto-mode/facade.ts +++ b/apps/server/src/services/auto-mode/facade.ts @@ -388,7 +388,7 @@ export class AutoModeServiceFacade { .replace(/\{\{taskName\}\}/g, task.description) .replace(/\{\{taskIndex\}\}/g, String(taskIndex + 1)) .replace(/\{\{totalTasks\}\}/g, String(allTasks.length)) - .replace(/\{\{taskDescription\}\}/g, task.description || task.description); + .replace(/\{\{taskDescription\}\}/g, task.description || `Task ${task.id}`); if (feedback) { taskPrompt = taskPrompt.replace(/\{\{userFeedback\}\}/g, feedback); } @@ -575,12 +575,17 @@ export class AutoModeServiceFacade { useWorktrees = false, _calledInternally = false ): Promise { - return this.recoveryService.resumeFeature( - this.projectPath, - featureId, - useWorktrees, - _calledInternally - ); + try { + return await this.recoveryService.resumeFeature( + this.projectPath, + featureId, + useWorktrees, + _calledInternally + ); + } catch (error) { + this.handleFacadeError(error, 'resumeFeature', featureId); + throw error; + } } /** diff --git a/apps/server/src/services/merge-service.ts b/apps/server/src/services/merge-service.ts index 32ed07ae..9bc60da3 100644 --- a/apps/server/src/services/merge-service.ts +++ b/apps/server/src/services/merge-service.ts @@ -5,7 +5,7 @@ */ import { createLogger } from '@automaker/utils'; -import { createEventEmitter } from '../lib/events.js'; +import { type EventEmitter } from '../lib/events.js'; import { execGitCommand } from '../lib/git.js'; const logger = createLogger('MergeService'); @@ -52,10 +52,9 @@ export async function performMerge( branchName: string, worktreePath: string, targetBranch: string = 'main', - options?: MergeOptions + options?: MergeOptions, + emitter?: EventEmitter ): Promise { - const emitter = createEventEmitter(); - if (!projectPath || !branchName || !worktreePath) { return { success: false, @@ -100,7 +99,7 @@ export async function performMerge( } // Emit merge:start after validating inputs - emitter.emit('merge:start', { branchName, targetBranch: mergeTo, worktreePath }); + emitter?.emit('merge:start', { branchName, targetBranch: mergeTo, worktreePath }); // Merge the feature branch into the target branch (using safe array-based commands) const mergeMessage = options?.message || `Merge ${branchName} into ${mergeTo}`; @@ -134,7 +133,7 @@ export async function performMerge( } // Emit merge:conflict event with conflict details - emitter.emit('merge:conflict', { branchName, targetBranch: mergeTo, conflictFiles }); + emitter?.emit('merge:conflict', { branchName, targetBranch: mergeTo, conflictFiles }); return { success: false, @@ -145,7 +144,7 @@ export async function performMerge( } // Emit merge:error for non-conflict errors before re-throwing - emitter.emit('merge:error', { + emitter?.emit('merge:error', { branchName, targetBranch: mergeTo, error: err.message || String(mergeError), @@ -196,7 +195,7 @@ export async function performMerge( } // Emit merge:success with merged branch, target branch, and deletion info - emitter.emit('merge:success', { + emitter?.emit('merge:success', { mergedBranch: branchName, targetBranch: mergeTo, deleted: options?.deleteWorktreeAndBranch ? { worktreeDeleted, branchDeleted } : undefined, diff --git a/apps/server/src/services/stash-service.ts b/apps/server/src/services/stash-service.ts index 501ab4a3..8a201333 100644 --- a/apps/server/src/services/stash-service.ts +++ b/apps/server/src/services/stash-service.ts @@ -15,7 +15,7 @@ */ import { createLogger } from '@automaker/utils'; -import { createEventEmitter } from '../lib/events.js'; +import type { EventEmitter } from '../lib/events.js'; import { execGitCommand } from '../lib/git.js'; import { getErrorMessage, logError } from '../routes/worktree/common.js'; @@ -130,16 +130,16 @@ function isConflictOutput(output: string): boolean { export async function applyOrPop( worktreePath: string, stashIndex: number, - options?: StashApplyOptions + options?: StashApplyOptions, + events?: EventEmitter ): Promise { - const emitter = createEventEmitter(); const operation: 'apply' | 'pop' = options?.pop ? 'pop' : 'apply'; const stashRef = `stash@{${stashIndex}}`; logger.info(`[StashService] ${operation} ${stashRef} in ${worktreePath}`); // 1. Emit start event - emitter.emit('stash:start', { worktreePath, stashIndex, stashRef, operation }); + events?.emit('stash:start', { worktreePath, stashIndex, stashRef, operation }); try { // 2. Run git stash apply / pop @@ -155,7 +155,7 @@ export async function applyOrPop( const combinedOutput = `${errStdout}\n${errStderr}`; // 3. Emit progress with raw output - emitter.emit('stash:progress', { + events?.emit('stash:progress', { worktreePath, stashIndex, operation, @@ -166,7 +166,7 @@ export async function applyOrPop( if (isConflictOutput(combinedOutput)) { const conflictFiles = await getConflictedFiles(worktreePath); - emitter.emit('stash:conflicts', { + events?.emit('stash:conflicts', { worktreePath, stashIndex, operation, @@ -183,7 +183,7 @@ export async function applyOrPop( message: `Stash ${operation === 'pop' ? 'popped' : 'applied'} with conflicts. Please resolve the conflicts.`, }; - emitter.emit('stash:success', { + events?.emit('stash:success', { worktreePath, stashIndex, operation, @@ -202,12 +202,12 @@ export async function applyOrPop( // exit 0 even when conflicts occur during apply) const combinedOutput = stdout; - emitter.emit('stash:progress', { worktreePath, stashIndex, operation, output: combinedOutput }); + events?.emit('stash:progress', { worktreePath, stashIndex, operation, output: combinedOutput }); if (isConflictOutput(combinedOutput)) { const conflictFiles = await getConflictedFiles(worktreePath); - emitter.emit('stash:conflicts', { + events?.emit('stash:conflicts', { worktreePath, stashIndex, operation, @@ -224,7 +224,7 @@ export async function applyOrPop( message: `Stash ${operation === 'pop' ? 'popped' : 'applied'} with conflicts. Please resolve the conflicts.`, }; - emitter.emit('stash:success', { + events?.emit('stash:success', { worktreePath, stashIndex, operation, @@ -245,7 +245,7 @@ export async function applyOrPop( message: `Stash ${operation === 'pop' ? 'popped' : 'applied'} successfully`, }; - emitter.emit('stash:success', { + events?.emit('stash:success', { worktreePath, stashIndex, operation, @@ -258,7 +258,7 @@ export async function applyOrPop( logError(error, `Stash ${operation} failed`); - emitter.emit('stash:failure', { + events?.emit('stash:failure', { worktreePath, stashIndex, operation, diff --git a/apps/server/src/services/worktree-branch-service.ts b/apps/server/src/services/worktree-branch-service.ts index 99e224e6..9047def7 100644 --- a/apps/server/src/services/worktree-branch-service.ts +++ b/apps/server/src/services/worktree-branch-service.ts @@ -73,7 +73,8 @@ async function hasAnyChanges(cwd: string): Promise { /** * Stash all local changes (including untracked files) - * Returns true if a stash was created, false if there was nothing to stash + * Returns true if a stash was created, false if there was nothing to stash. + * Throws on unexpected errors so callers abort rather than proceeding silently. */ async function stashChanges(cwd: string, message: string): Promise { try { @@ -95,8 +96,26 @@ async function stashChanges(cwd: string, message: string): Promise { .filter((l) => l.trim()).length; return countAfter > countBefore; - } catch { - return false; + } catch (error) { + const errorMsg = getErrorMessage(error); + + // "Nothing to stash" is benign – no work was lost, just return false + if ( + errorMsg.toLowerCase().includes('no local changes to save') || + errorMsg.toLowerCase().includes('nothing to stash') + ) { + logger.debug('stashChanges: nothing to stash', { cwd, message, error: errorMsg }); + return false; + } + + // Unexpected error – log full details and re-throw so the caller aborts + // rather than proceeding with an un-stashed working tree + logger.error('stashChanges: unexpected error during stash', { + cwd, + message, + error: errorMsg, + }); + throw new Error(`Failed to stash changes in ${cwd}: ${errorMsg}`); } } @@ -284,7 +303,20 @@ export async function performSwitchBranch( action: 'push', }); const stashMessage = `automaker-branch-switch: ${previousBranch} → ${targetBranch}`; - didStash = await stashChanges(worktreePath, stashMessage); + try { + didStash = await stashChanges(worktreePath, stashMessage); + } catch (stashError) { + const stashErrorMsg = getErrorMessage(stashError); + events?.emit('switch:error', { + worktreePath, + branchName, + error: `Failed to stash local changes: ${stashErrorMsg}`, + }); + return { + success: false, + error: `Failed to stash local changes before switching branches: ${stashErrorMsg}`, + }; + } } try { diff --git a/apps/server/tests/unit/services/agent-service.test.ts b/apps/server/tests/unit/services/agent-service.test.ts index fed1eae3..11e92097 100644 --- a/apps/server/tests/unit/services/agent-service.test.ts +++ b/apps/server/tests/unit/services/agent-service.test.ts @@ -123,9 +123,10 @@ describe('agent-service.ts', () => { }); expect(result.success).toBe(true); - // First call reads session file, metadata file, and queue state file (3 calls) + // First call reads metadata file and session file via ensureSession (2 calls) + // Since no metadata or messages exist, a fresh session is created without loading queue state. // Second call should reuse in-memory session (no additional calls) - expect(fs.readFile).toHaveBeenCalledTimes(3); + expect(fs.readFile).toHaveBeenCalledTimes(2); }); }); @@ -330,14 +331,14 @@ describe('agent-service.ts', () => { sessionId: 'session-1', }); - const history = service.getHistory('session-1'); + const history = await service.getHistory('session-1'); expect(history).toBeDefined(); expect(history?.messages).toEqual([]); }); - it('should handle non-existent session', () => { - const history = service.getHistory('nonexistent'); + it('should handle non-existent session', async () => { + const history = await service.getHistory('nonexistent'); expect(history).toBeDefined(); // Returns error object }); }); @@ -356,10 +357,108 @@ describe('agent-service.ts', () => { await service.clearSession('session-1'); - const history = service.getHistory('session-1'); + const history = await service.getHistory('session-1'); expect(history?.messages).toEqual([]); expect(fs.writeFile).toHaveBeenCalled(); }); + + it('should clear sdkSessionId from persisted metadata to prevent stale session errors', async () => { + // Setup: Session exists in metadata with an sdkSessionId (simulating + // a session that previously communicated with a CLI provider like OpenCode) + const metadata = { + 'session-1': { + id: 'session-1', + name: 'Test Session', + workingDirectory: '/test/dir', + createdAt: '2024-01-01T00:00:00Z', + updatedAt: '2024-01-01T00:00:00Z', + sdkSessionId: 'stale-opencode-session-id', + }, + }; + + vi.mocked(fs.readFile).mockResolvedValue(JSON.stringify(metadata)); + vi.mocked(fs.writeFile).mockResolvedValue(undefined); + vi.mocked(fs.mkdir).mockResolvedValue(undefined); + + // Start the session (loads from disk metadata) + await service.startConversation({ + sessionId: 'session-1', + workingDirectory: '/test/dir', + }); + + // Clear the session + await service.clearSession('session-1'); + + // Verify that the LAST writeFile call to sessions-metadata.json + // (from clearSdkSessionId) has sdkSessionId removed. + // Earlier writes may still include it (e.g., from updateSessionTimestamp). + const writeFileCalls = vi.mocked(fs.writeFile).mock.calls; + const metadataWriteCalls = writeFileCalls.filter( + (call) => + typeof call[0] === 'string' && (call[0] as string).includes('sessions-metadata.json') + ); + + expect(metadataWriteCalls.length).toBeGreaterThan(0); + const lastMetadataWriteCall = metadataWriteCalls[metadataWriteCalls.length - 1]; + const savedMetadata = JSON.parse(lastMetadataWriteCall[1] as string); + expect(savedMetadata['session-1'].sdkSessionId).toBeUndefined(); + }); + }); + + describe('clearSdkSessionId', () => { + it('should remove sdkSessionId from persisted metadata', async () => { + const metadata = { + 'session-1': { + id: 'session-1', + name: 'Test Session', + workingDirectory: '/test/dir', + createdAt: '2024-01-01T00:00:00Z', + updatedAt: '2024-01-01T00:00:00Z', + sdkSessionId: 'old-provider-session-id', + }, + }; + + vi.mocked(fs.readFile).mockResolvedValue(JSON.stringify(metadata)); + vi.mocked(fs.writeFile).mockResolvedValue(undefined); + + await service.clearSdkSessionId('session-1'); + + const writeFileCalls = vi.mocked(fs.writeFile).mock.calls; + expect(writeFileCalls.length).toBeGreaterThan(0); + + const savedMetadata = JSON.parse(writeFileCalls[0][1] as string); + expect(savedMetadata['session-1'].sdkSessionId).toBeUndefined(); + expect(savedMetadata['session-1'].updatedAt).not.toBe('2024-01-01T00:00:00Z'); + }); + + it('should do nothing if session has no sdkSessionId', async () => { + const metadata = { + 'session-1': { + id: 'session-1', + name: 'Test Session', + workingDirectory: '/test/dir', + createdAt: '2024-01-01T00:00:00Z', + updatedAt: '2024-01-01T00:00:00Z', + }, + }; + + vi.mocked(fs.readFile).mockResolvedValue(JSON.stringify(metadata)); + vi.mocked(fs.writeFile).mockResolvedValue(undefined); + + await service.clearSdkSessionId('session-1'); + + // writeFile should not have been called since there's no sdkSessionId to clear + expect(fs.writeFile).not.toHaveBeenCalled(); + }); + + it('should do nothing if session does not exist in metadata', async () => { + vi.mocked(fs.readFile).mockResolvedValue('{}'); + vi.mocked(fs.writeFile).mockResolvedValue(undefined); + + await service.clearSdkSessionId('nonexistent'); + + expect(fs.writeFile).not.toHaveBeenCalled(); + }); }); describe('createSession', () => { @@ -654,15 +753,15 @@ describe('agent-service.ts', () => { it('should return queue for session', async () => { await service.addToQueue('session-1', { message: 'Test prompt' }); - const result = service.getQueue('session-1'); + const result = await service.getQueue('session-1'); expect(result.success).toBe(true); expect(result.queue).toBeDefined(); expect(result.queue?.length).toBe(1); }); - it('should return error for non-existent session', () => { - const result = service.getQueue('nonexistent'); + it('should return error for non-existent session', async () => { + const result = await service.getQueue('nonexistent'); expect(result.success).toBe(false); expect(result.error).toBe('Session not found'); @@ -686,7 +785,7 @@ describe('agent-service.ts', () => { }); it('should remove prompt from queue', async () => { - const queueResult = service.getQueue('session-1'); + const queueResult = await service.getQueue('session-1'); const promptId = queueResult.queue![0].id; const result = await service.removeFromQueue('session-1', promptId); @@ -731,7 +830,7 @@ describe('agent-service.ts', () => { const result = await service.clearQueue('session-1'); expect(result.success).toBe(true); - const queueResult = service.getQueue('session-1'); + const queueResult = await service.getQueue('session-1'); expect(queueResult.queue?.length).toBe(0); expect(mockEvents.emit).toHaveBeenCalled(); }); diff --git a/apps/ui/src/components/ui/truncated-file-path.tsx b/apps/ui/src/components/ui/truncated-file-path.tsx index cf9dbb56..6be81df9 100644 --- a/apps/ui/src/components/ui/truncated-file-path.tsx +++ b/apps/ui/src/components/ui/truncated-file-path.tsx @@ -33,8 +33,8 @@ export function TruncatedFilePath({ path, className }: TruncatedFilePathProps) { return ( - {dirPart} - {filePart} + {dirPart} + {filePart} ); } diff --git a/apps/ui/src/components/views/board-view/dialogs/commit-worktree-dialog.tsx b/apps/ui/src/components/views/board-view/dialogs/commit-worktree-dialog.tsx index 7c09a447..6f56cc0a 100644 --- a/apps/ui/src/components/views/board-view/dialogs/commit-worktree-dialog.tsx +++ b/apps/ui/src/components/views/board-view/dialogs/commit-worktree-dialog.tsx @@ -178,6 +178,10 @@ function parseDiff(diffText: string): ParsedFileDiff[] { } if (currentHunk) { + // Skip trailing empty line produced by split('\n') to avoid phantom context line + if (line === '' && i === lines.length - 1) { + continue; + } if (line.startsWith('+')) { currentHunk.lines.push({ type: 'addition', diff --git a/apps/ui/src/components/views/board-view/dialogs/git-pull-dialog.tsx b/apps/ui/src/components/views/board-view/dialogs/git-pull-dialog.tsx index 89ed3ecc..14a4d387 100644 --- a/apps/ui/src/components/views/board-view/dialogs/git-pull-dialog.tsx +++ b/apps/ui/src/components/views/board-view/dialogs/git-pull-dialog.tsx @@ -41,10 +41,10 @@ type PullPhase = | 'error'; // Something went wrong interface PullResult { - branch: string; + branch?: string; remote?: string; - pulled: boolean; - message: string; + pulled?: boolean; + message?: string; hasLocalChanges?: boolean; localChangedFiles?: string[]; hasConflicts?: boolean; @@ -52,6 +52,7 @@ interface PullResult { conflictFiles?: string[]; stashed?: boolean; stashRestored?: boolean; + stashRecoveryFailed?: boolean; } interface GitPullDialogProps { @@ -167,9 +168,10 @@ export function GitPullDialog({ if (!worktree || !pullResult || !onCreateConflictResolutionFeature) return; const effectiveRemote = pullResult.remote || remote; + const branch = pullResult.branch ?? worktree.branch; const conflictInfo: MergeConflictInfo = { - sourceBranch: effectiveRemote ? `${effectiveRemote}/${pullResult.branch}` : pullResult.branch, - targetBranch: pullResult.branch, + sourceBranch: `${effectiveRemote || 'origin'}/${branch}`, + targetBranch: branch, targetWorktreePath: worktree.path, conflictFiles: pullResult.conflictFiles || [], operationType: 'merge', @@ -307,14 +309,16 @@ export function GitPullDialog({ )} - {pullResult?.stashed && !pullResult?.stashRestored && ( -
- - - {pullResult.message} - -
- )} + {pullResult?.stashed && + (!pullResult?.stashRestored || pullResult?.stashRecoveryFailed) && ( +
+ + + {pullResult?.message ?? + 'Stash could not be restored. Your changes remain in the stash.'} + +
+ )} diff --git a/apps/ui/src/components/views/board-view/dialogs/stash-changes-dialog.tsx b/apps/ui/src/components/views/board-view/dialogs/stash-changes-dialog.tsx index c07aaa36..18f4e9fa 100644 --- a/apps/ui/src/components/views/board-view/dialogs/stash-changes-dialog.tsx +++ b/apps/ui/src/components/views/board-view/dialogs/stash-changes-dialog.tsx @@ -284,6 +284,7 @@ export function StashChangesDialog({ const [selectedFiles, setSelectedFiles] = useState>(new Set()); const [expandedFile, setExpandedFile] = useState(null); const [isLoadingDiffs, setIsLoadingDiffs] = useState(false); + const [loadDiffsError, setLoadDiffsError] = useState(null); // Parse diffs const parsedDiffs = useMemo(() => parseDiff(diffContent), [diffContent]); @@ -297,42 +298,47 @@ export function StashChangesDialog({ return map; }, [parsedDiffs]); - // Load diffs when dialog opens - useEffect(() => { - if (open && worktree) { + const loadDiffs = useCallback( + async (cancelled: { current: boolean }) => { setIsLoadingDiffs(true); + setLoadDiffsError(null); setFiles([]); setDiffContent(''); setSelectedFiles(new Set()); setExpandedFile(null); - - let cancelled = false; - - const loadDiffs = async () => { - try { - const api = getHttpApiClient(); - const result = await api.git.getDiffs(worktree.path); - if (result.success) { - const fileList = result.files ?? []; - if (!cancelled) setFiles(fileList); - if (!cancelled) setDiffContent(result.diff ?? ''); - // Select all files by default - if (!cancelled) setSelectedFiles(new Set(fileList.map((f: FileStatus) => f.path))); - } - } catch (err) { - console.warn('Failed to load diffs for stash dialog:', err); - } finally { - if (!cancelled) setIsLoadingDiffs(false); + try { + const api = getHttpApiClient(); + const result = await api.git.getDiffs(worktree!.path); + if (result.success) { + const fileList = result.files ?? []; + if (!cancelled.current) setFiles(fileList); + if (!cancelled.current) setDiffContent(result.diff ?? ''); + // Select all files by default + if (!cancelled.current) + setSelectedFiles(new Set(fileList.map((f: FileStatus) => f.path))); } - }; - - loadDiffs(); + } catch (err) { + console.warn('Failed to load diffs for stash dialog:', err); + if (!cancelled.current) { + setLoadDiffsError(err instanceof Error ? err.message : 'Failed to load changes'); + } + } finally { + if (!cancelled.current) setIsLoadingDiffs(false); + } + }, + [worktree] + ); + // Load diffs when dialog opens + useEffect(() => { + if (open && worktree) { + const cancelled = { current: false }; + loadDiffs(cancelled); return () => { - cancelled = true; + cancelled.current = true; }; } - }, [open, worktree]); + }, [open, worktree, loadDiffs]); const handleToggleFile = useCallback((filePath: string) => { setSelectedFiles((prev) => { @@ -466,6 +472,20 @@ export function StashChangesDialog({ Loading changes... + ) : loadDiffsError ? ( +
+ Failed to load changes + +
) : files.length === 0 ? (
No changes detected diff --git a/apps/ui/src/components/views/board-view/hooks/use-board-actions.ts b/apps/ui/src/components/views/board-view/hooks/use-board-actions.ts index 00c97fd7..134dee78 100644 --- a/apps/ui/src/components/views/board-view/hooks/use-board-actions.ts +++ b/apps/ui/src/components/views/board-view/hooks/use-board-actions.ts @@ -1151,6 +1151,9 @@ export function useBoardActions({ if (result.success) { // Refresh features from server to sync React Query cache loadFeatures(); + toast.success('All verified features archived', { + description: `Archived ${verifiedFeatures.length} feature(s).`, + }); } else { logger.error('Bulk archive failed:', result); // Reload features to sync state with server @@ -1162,10 +1165,6 @@ export function useBoardActions({ // Reload features to sync state with server on error loadFeatures(); } - - toast.success('All verified features archived', { - description: `Archived ${verifiedFeatures.length} feature(s).`, - }); }, [features, runningAutoTasks, autoMode, updateFeature, currentProject, loadFeatures]); const handleDuplicateFeature = useCallback(