Fix: Restore views properly, model selection for commit and pr and speed up some cli models with session resume (#801)

* Changes from fix/restoring-view

* feat: Add resume query safety checks and optimize store selectors

* feat: Improve session management and model normalization

* refactor: Extract prompt building logic and handle file path parsing for renames
This commit is contained in:
gsxdsm
2026-02-22 10:45:45 -08:00
committed by GitHub
parent 2f071a1ba3
commit 9305ecc242
26 changed files with 761 additions and 203 deletions

View File

@@ -51,6 +51,7 @@ import { CODEX_MODELS } from './codex-models.js';
const CODEX_COMMAND = 'codex';
const CODEX_EXEC_SUBCOMMAND = 'exec';
const CODEX_RESUME_SUBCOMMAND = 'resume';
const CODEX_JSON_FLAG = '--json';
const CODEX_MODEL_FLAG = '--model';
const CODEX_VERSION_FLAG = '--version';
@@ -355,9 +356,14 @@ function resolveSystemPrompt(systemPrompt?: unknown): string | null {
return null;
}
function buildPromptText(options: ExecuteOptions): string {
return typeof options.prompt === 'string'
? options.prompt
: extractTextFromContent(options.prompt);
}
function buildCombinedPrompt(options: ExecuteOptions, systemPromptText?: string | null): string {
const promptText =
typeof options.prompt === 'string' ? options.prompt : extractTextFromContent(options.prompt);
const promptText = buildPromptText(options);
const historyText = options.conversationHistory
? formatHistoryAsText(options.conversationHistory)
: '';
@@ -370,6 +376,11 @@ function buildCombinedPrompt(options: ExecuteOptions, systemPromptText?: string
return `${historyText}${systemSection}${HISTORY_HEADER}${promptText}`;
}
function buildResumePrompt(options: ExecuteOptions): string {
const promptText = buildPromptText(options);
return `${HISTORY_HEADER}${promptText}`;
}
function formatConfigValue(value: string | number | boolean): string {
return String(value);
}
@@ -793,16 +804,22 @@ export class CodexProvider extends BaseProvider {
}
const searchEnabled =
codexSettings.enableWebSearch || resolveSearchEnabled(resolvedAllowedTools, restrictTools);
const schemaPath = await writeOutputSchemaFile(options.cwd, options.outputFormat);
const imageBlocks = codexSettings.enableImages ? extractImageBlocks(options.prompt) : [];
const imagePaths = await writeImageFiles(options.cwd, imageBlocks);
const isResumeQuery = Boolean(options.sdkSessionId);
const schemaPath = isResumeQuery
? null
: await writeOutputSchemaFile(options.cwd, options.outputFormat);
const imageBlocks =
!isResumeQuery && codexSettings.enableImages ? extractImageBlocks(options.prompt) : [];
const imagePaths = isResumeQuery ? [] : await writeImageFiles(options.cwd, imageBlocks);
const approvalPolicy =
hasMcpServers && options.mcpAutoApproveTools !== undefined
? options.mcpAutoApproveTools
? 'never'
: 'on-request'
: codexSettings.approvalPolicy;
const promptText = buildCombinedPrompt(options, combinedSystemPrompt);
const promptText = isResumeQuery
? buildResumePrompt(options)
: buildCombinedPrompt(options, combinedSystemPrompt);
const commandPath = executionPlan.cliPath || CODEX_COMMAND;
// Build config overrides for max turns and reasoning effort
@@ -832,21 +849,30 @@ export class CodexProvider extends BaseProvider {
const preExecArgs: string[] = [];
// Add additional directories with write access
if (codexSettings.additionalDirs && codexSettings.additionalDirs.length > 0) {
if (
!isResumeQuery &&
codexSettings.additionalDirs &&
codexSettings.additionalDirs.length > 0
) {
for (const dir of codexSettings.additionalDirs) {
preExecArgs.push(CODEX_ADD_DIR_FLAG, dir);
}
}
// If images were written to disk, add the image directory so the CLI can access them
// If images were written to disk, add the image directory so the CLI can access them.
// Note: imagePaths is set to [] when isResumeQuery is true, so this check is sufficient.
if (imagePaths.length > 0) {
const imageDir = path.join(options.cwd, CODEX_INSTRUCTIONS_DIR, IMAGE_TEMP_DIR);
preExecArgs.push(CODEX_ADD_DIR_FLAG, imageDir);
}
// Model is already bare (no prefix) - validated by executeQuery
const codexCommand = isResumeQuery
? [CODEX_EXEC_SUBCOMMAND, CODEX_RESUME_SUBCOMMAND]
: [CODEX_EXEC_SUBCOMMAND];
const args = [
CODEX_EXEC_SUBCOMMAND,
...codexCommand,
CODEX_YOLO_FLAG,
CODEX_SKIP_GIT_REPO_CHECK_FLAG,
...preExecArgs,
@@ -855,6 +881,7 @@ export class CodexProvider extends BaseProvider {
CODEX_JSON_FLAG,
...configOverrideArgs,
...(schemaPath ? [CODEX_OUTPUT_SCHEMA_FLAG, schemaPath] : []),
...(options.sdkSessionId ? [options.sdkSessionId] : []),
'-', // Read prompt from stdin to avoid shell escaping issues
];

View File

@@ -30,6 +30,7 @@ import {
type CopilotRuntimeModel,
} from '@automaker/types';
import { createLogger, isAbortError } from '@automaker/utils';
import { resolveModelString } from '@automaker/model-resolver';
import { CopilotClient, type PermissionRequest } from '@github/copilot-sdk';
import {
normalizeTodos,
@@ -116,6 +117,12 @@ export interface CopilotError extends Error {
suggestion?: string;
}
type CopilotSession = Awaited<ReturnType<CopilotClient['createSession']>>;
type CopilotSessionOptions = Parameters<CopilotClient['createSession']>[0];
type ResumableCopilotClient = CopilotClient & {
resumeSession?: (sessionId: string, options: CopilotSessionOptions) => Promise<CopilotSession>;
};
// =============================================================================
// Tool Name Normalization
// =============================================================================
@@ -516,7 +523,11 @@ export class CopilotProvider extends CliProvider {
}
const promptText = this.extractPromptText(options);
const bareModel = options.model || DEFAULT_BARE_MODEL;
// resolveModelString may return dash-separated canonical names (e.g. "claude-sonnet-4-6"),
// but the Copilot SDK expects dot-separated version suffixes (e.g. "claude-sonnet-4.6").
// Normalize by converting the last dash-separated numeric pair to dot notation.
const resolvedModel = resolveModelString(options.model || DEFAULT_BARE_MODEL);
const bareModel = resolvedModel.replace(/-(\d+)-(\d+)$/, '-$1.$2');
const workingDirectory = options.cwd || process.cwd();
logger.debug(
@@ -554,12 +565,14 @@ export class CopilotProvider extends CliProvider {
});
};
// Declare session outside try so it's accessible in the catch block for cleanup.
let session: CopilotSession | undefined;
try {
await client.start();
logger.debug(`CopilotClient started with cwd: ${workingDirectory}`);
// Create session with streaming enabled for real-time events
const session = await client.createSession({
const sessionOptions: CopilotSessionOptions = {
model: bareModel,
streaming: true,
// AUTONOMOUS MODE: Auto-approve all permission requests.
@@ -572,13 +585,33 @@ export class CopilotProvider extends CliProvider {
logger.debug(`Permission request: ${request.kind}`);
return { kind: 'approved' };
},
});
};
const sessionId = session.sessionId;
logger.debug(`Session created: ${sessionId}`);
// Resume the previous Copilot session when possible; otherwise create a fresh one.
const resumableClient = client as ResumableCopilotClient;
let sessionResumed = false;
if (options.sdkSessionId && typeof resumableClient.resumeSession === 'function') {
try {
session = await resumableClient.resumeSession(options.sdkSessionId, sessionOptions);
sessionResumed = true;
logger.debug(`Resumed Copilot session: ${session.sessionId}`);
} catch (resumeError) {
logger.warn(
`Failed to resume Copilot session "${options.sdkSessionId}", creating a new session: ${resumeError}`
);
session = await client.createSession(sessionOptions);
}
} else {
session = await client.createSession(sessionOptions);
}
// session is always assigned by this point (both branches above assign it)
const activeSession = session!;
const sessionId = activeSession.sessionId;
logger.debug(`Session ${sessionResumed ? 'resumed' : 'created'}: ${sessionId}`);
// Set up event handler to push events to queue
session.on((event: SdkEvent) => {
activeSession.on((event: SdkEvent) => {
logger.debug(`SDK event: ${event.type}`);
if (event.type === 'session.idle') {
@@ -596,7 +629,7 @@ export class CopilotProvider extends CliProvider {
});
// Send the prompt (non-blocking)
await session.send({ prompt: promptText });
await activeSession.send({ prompt: promptText });
// Process events as they arrive
while (!sessionComplete || eventQueue.length > 0) {
@@ -604,7 +637,7 @@ export class CopilotProvider extends CliProvider {
// Check for errors first (before processing events to avoid race condition)
if (sessionError) {
await session.destroy();
await activeSession.destroy();
await client.stop();
throw sessionError;
}
@@ -624,11 +657,19 @@ export class CopilotProvider extends CliProvider {
}
// Cleanup
await session.destroy();
await activeSession.destroy();
await client.stop();
logger.debug('CopilotClient stopped successfully');
} catch (error) {
// Ensure client is stopped on error
// Ensure session is destroyed and client is stopped on error to prevent leaks.
// The session may have been created/resumed before the error occurred.
if (session) {
try {
await session.destroy();
} catch (sessionCleanupError) {
logger.debug(`Failed to destroy session during cleanup: ${sessionCleanupError}`);
}
}
try {
await client.stop();
} catch (cleanupError) {

View File

@@ -450,6 +450,11 @@ export class CursorProvider extends CliProvider {
cliArgs.push('--model', model);
}
// Resume an existing chat when a provider session ID is available
if (options.sdkSessionId) {
cliArgs.push('--resume', options.sdkSessionId);
}
// Use '-' to indicate reading prompt from stdin
cliArgs.push('-');

View File

@@ -270,6 +270,11 @@ export class GeminiProvider extends CliProvider {
cliArgs.push('--include-directories', options.cwd);
}
// Resume an existing Gemini session when one is available
if (options.sdkSessionId) {
cliArgs.push('--resume', options.sdkSessionId);
}
// Note: Gemini CLI doesn't have a --thinking-level flag.
// Thinking capabilities are determined by the model selection (e.g., gemini-2.5-pro).
// The model handles thinking internally based on the task complexity.

View File

@@ -5,12 +5,12 @@
* 1. Discard ALL changes (when no files array is provided)
* - Resets staged changes (git reset HEAD)
* - Discards modified tracked files (git checkout .)
* - Removes untracked files and directories (git clean -fd)
* - Removes untracked files and directories (git clean -ffd)
*
* 2. Discard SELECTED files (when files array is provided)
* - Unstages selected staged files (git reset HEAD -- <files>)
* - Reverts selected tracked file changes (git checkout -- <files>)
* - Removes selected untracked files (git clean -fd -- <files>)
* - Removes selected untracked files (git clean -ffd -- <files>)
*
* Note: Git repository validation (isGitRepo) is handled by
* the requireGitRepoOnly middleware in index.ts
@@ -52,6 +52,22 @@ function validateFilePath(filePath: string, worktreePath: string): boolean {
}
}
/**
* Parse a file path from git status --porcelain output, handling renames.
* For renamed files (R status), git reports "old_path -> new_path" and
* we need the new path to match what parseGitStatus() returns in git-utils.
*/
function parseFilePath(rawPath: string, indexStatus: string, workTreeStatus: string): string {
const trimmedPath = rawPath.trim();
if (indexStatus === 'R' || workTreeStatus === 'R') {
const arrowIndex = trimmedPath.indexOf(' -> ');
if (arrowIndex !== -1) {
return trimmedPath.slice(arrowIndex + 4);
}
}
return trimmedPath;
}
export function createDiscardChangesHandler() {
return async (req: Request, res: Response): Promise<void> => {
try {
@@ -91,11 +107,16 @@ export function createDiscardChangesHandler() {
// Parse the status output to categorize files
// Git --porcelain format: XY PATH where X=index status, Y=worktree status
// Preserve the exact two-character XY status (no trim) to keep index vs worktree info
// For renamed files: XY OLD_PATH -> NEW_PATH
const statusLines = status.trim().split('\n').filter(Boolean);
const allFiles = statusLines.map((line) => {
const fileStatus = line.substring(0, 2);
const filePath = line.slice(3).trim();
const rawPath = line.slice(3);
const indexStatus = fileStatus.charAt(0);
const workTreeStatus = fileStatus.charAt(1);
// Parse path consistently with parseGitStatus() in git-utils,
// which extracts the new path for renames
const filePath = parseFilePath(rawPath, indexStatus, workTreeStatus);
return { status: fileStatus, path: filePath };
});
@@ -122,8 +143,12 @@ export function createDiscardChangesHandler() {
const untrackedFiles: string[] = []; // Untracked files (?)
const warnings: string[] = [];
// Track which requested files were matched so we can handle unmatched ones
const matchedFiles = new Set<string>();
for (const file of allFiles) {
if (!filesToDiscard.has(file.path)) continue;
matchedFiles.add(file.path);
// file.status is the raw two-character XY git porcelain status (no trim)
// X = index/staging status, Y = worktree status
@@ -151,6 +176,16 @@ export function createDiscardChangesHandler() {
}
}
// Handle files from the UI that didn't match any entry in allFiles.
// This can happen due to timing differences between the UI loading diffs
// and the discard request, or path format differences.
// Attempt to clean unmatched files directly as untracked files.
for (const requestedFile of files) {
if (!matchedFiles.has(requestedFile)) {
untrackedFiles.push(requestedFile);
}
}
// 1. Unstage selected staged files (using execFile to bypass shell)
if (stagedFiles.length > 0) {
try {
@@ -174,9 +209,10 @@ export function createDiscardChangesHandler() {
}
// 3. Remove selected untracked files
// Use -ffd (double force) to also handle nested git repositories
if (untrackedFiles.length > 0) {
try {
await execGitCommand(['clean', '-fd', '--', ...untrackedFiles], worktreePath);
await execGitCommand(['clean', '-ffd', '--', ...untrackedFiles], worktreePath);
} catch (error) {
const msg = getErrorMessage(error);
logError(error, `Failed to clean untracked files: ${msg}`);
@@ -234,11 +270,12 @@ export function createDiscardChangesHandler() {
}
// 3. Remove untracked files and directories
// Use -ffd (double force) to also handle nested git repositories
try {
await execGitCommand(['clean', '-fd'], worktreePath);
await execGitCommand(['clean', '-ffd', '--'], worktreePath);
} catch (error) {
const msg = getErrorMessage(error);
logError(error, `git clean -fd failed: ${msg}`);
logError(error, `git clean -ffd failed: ${msg}`);
warnings.push(`Failed to remove untracked files: ${msg}`);
}

View File

@@ -29,6 +29,7 @@ export interface AgentExecutionOptions {
credentials?: Credentials;
claudeCompatibleProvider?: ClaudeCompatibleProvider;
mcpServers?: Record<string, unknown>;
sdkSessionId?: string;
sdkOptions?: {
maxTurns?: number;
allowedTools?: string[];

View File

@@ -93,6 +93,7 @@ export class AgentExecutor {
credentials,
claudeCompatibleProvider,
mcpServers,
sdkSessionId,
sdkOptions,
} = options;
const { content: promptContent } = await buildPromptWithImages(
@@ -129,6 +130,7 @@ export class AgentExecutor {
thinkingLevel: options.thinkingLevel,
credentials,
claudeCompatibleProvider,
sdkSessionId,
};
const featureDirForOutput = getFeatureDir(projectPath, featureId);
const outputPath = path.join(featureDirForOutput, 'agent-output.md');
@@ -217,6 +219,9 @@ export class AgentExecutor {
try {
const stream = provider.executeQuery(executeOptions);
streamLoop: for await (const msg of stream) {
if (msg.session_id && msg.session_id !== options.sdkSessionId) {
options.sdkSessionId = msg.session_id;
}
receivedAnyStreamMessage = true;
appendRawEvent(msg);
if (abortController.signal.aborted) {
@@ -385,6 +390,9 @@ export class AgentExecutor {
taskCompleteDetected = false;
for await (const msg of taskStream) {
if (msg.session_id && msg.session_id !== options.sdkSessionId) {
options.sdkSessionId = msg.session_id;
}
if (msg.type === 'assistant' && msg.message?.content) {
for (const b of msg.message.content) {
if (b.type === 'text') {
@@ -599,6 +607,9 @@ export class AgentExecutor {
for await (const msg of provider.executeQuery(
this.buildExecOpts(options, revPrompt, sdkOptions?.maxTurns ?? DEFAULT_MAX_TURNS)
)) {
if (msg.session_id && msg.session_id !== options.sdkSessionId) {
options.sdkSessionId = msg.session_id;
}
if (msg.type === 'assistant' && msg.message?.content)
for (const b of msg.message.content)
if (b.type === 'text') {
@@ -698,6 +709,7 @@ export class AgentExecutor {
: undefined,
credentials: o.credentials,
claudeCompatibleProvider: o.claudeCompatibleProvider,
sdkSessionId: o.sdkSessionId,
};
}
@@ -717,6 +729,9 @@ export class AgentExecutor {
for await (const msg of provider.executeQuery(
this.buildExecOpts(options, contPrompt, options.sdkOptions?.maxTurns ?? DEFAULT_MAX_TURNS)
)) {
if (msg.session_id && msg.session_id !== options.sdkSessionId) {
options.sdkSessionId = msg.session_id;
}
if (msg.type === 'assistant' && msg.message?.content)
for (const b of msg.message.content) {
if (b.type === 'text') {

View File

@@ -329,12 +329,6 @@ export class AgentService {
timestamp: new Date().toISOString(),
};
// Build conversation history from existing messages BEFORE adding current message
const conversationHistory = session.messages.map((msg) => ({
role: msg.role,
content: msg.content,
}));
session.messages.push(userMessage);
session.isRunning = true;
session.abortController = new AbortController();
@@ -406,6 +400,7 @@ export class AgentService {
}
}
let combinedSystemPrompt: string | undefined;
// Load project context files (CLAUDE.md, CODE_QUALITY.md, etc.) and memory files
// Use the user's message as task context for smart memory selection
const contextResult = await loadContextFiles({
@@ -423,7 +418,7 @@ export class AgentService {
// Build combined system prompt with base prompt and context files
const baseSystemPrompt = await this.getSystemPrompt();
const combinedSystemPrompt = contextFilesPrompt
combinedSystemPrompt = contextFilesPrompt
? `${contextFilesPrompt}\n\n${baseSystemPrompt}`
: baseSystemPrompt;
@@ -513,6 +508,14 @@ export class AgentService {
: stripProviderPrefix(effectiveModel);
// Build options for provider
const conversationHistory = session.messages
.slice(0, -1)
.map((msg) => ({
role: msg.role,
content: msg.content,
}))
.filter((msg) => msg.content.trim().length > 0);
const options: ExecuteOptions = {
prompt: '', // Will be set below based on images
model: bareModel, // Bare model ID (e.g., "gpt-5.1-codex-max", "composer-1")
@@ -522,7 +525,8 @@ export class AgentService {
maxTurns: maxTurns,
allowedTools: allowedTools,
abortController: session.abortController!,
conversationHistory: conversationHistory.length > 0 ? conversationHistory : undefined,
conversationHistory:
conversationHistory && conversationHistory.length > 0 ? conversationHistory : undefined,
settingSources: settingSources.length > 0 ? settingSources : undefined,
sdkSessionId: session.sdkSessionId, // Pass SDK session ID for resuming
mcpServers: Object.keys(mcpServers).length > 0 ? mcpServers : undefined, // Pass MCP servers configuration