From 30564601ed93f14431fc647c502c7e37621d8106 Mon Sep 17 00:00:00 2001 From: junes <673638712@qq.com> Date: Mon, 1 Jun 2026 18:50:16 +0800 Subject: [PATCH] fix(cli,web): hide Windows spawn windows and show queued attachments (#765) * fix(cli,web): hide Windows spawn windows and show queued attachments * fix(web): preserve attachment-only queued edit text --- .../agent/backends/acp/AcpSdkBackend.test.ts | 23 ++++++ .../agent/backends/acp/AcpStdioTransport.ts | 22 +++-- cli/src/claude/sdk/utils.ts | 3 +- cli/src/codex/utils/codexExecutable.ts | 3 +- cli/src/codex/utils/codexVersion.ts | 3 +- cli/src/commands/claude.ts | 7 +- cli/src/cursor/cursorRemoteLauncher.ts | 3 +- cli/src/modules/common/cursorModels.ts | 3 +- cli/src/modules/difftastic/index.ts | 3 +- cli/src/modules/ripgrep/index.ts | 3 +- cli/src/utils/spawnWithAbort.test.ts | 50 +++++++++++- cli/src/utils/spawnWithAbort.ts | 4 +- .../AssistantChat/QueuedMessagesBar.test.tsx | 80 ++++++++++++++++++- .../AssistantChat/QueuedMessagesBar.tsx | 55 ++++++++----- 14 files changed, 227 insertions(+), 35 deletions(-) diff --git a/cli/src/agent/backends/acp/AcpSdkBackend.test.ts b/cli/src/agent/backends/acp/AcpSdkBackend.test.ts index 8d70f0cc..0188e672 100644 --- a/cli/src/agent/backends/acp/AcpSdkBackend.test.ts +++ b/cli/src/agent/backends/acp/AcpSdkBackend.test.ts @@ -1,6 +1,7 @@ import { afterEach, describe, expect, it } from 'vitest'; import type { AgentMessage } from '@/agent/types'; import { AcpSdkBackend } from './AcpSdkBackend'; +import { buildAcpStdioSpawnOptions } from './AcpStdioTransport'; import { ACP_SESSION_UPDATE_TYPES } from './constants'; function sleep(ms: number): Promise { @@ -27,6 +28,14 @@ const originalStatics = { lateFlushQuietPeriodMs: backendStatics.LATE_FLUSH_QUIET_PERIOD_MS, lateFlushWindowMs: backendStatics.LATE_FLUSH_WINDOW_MS }; +const originalPlatformDescriptor = Object.getOwnPropertyDescriptor(process, 'platform'); + +function setPlatform(value: string) { + Object.defineProperty(process, 'platform', { + value, + configurable: true + }); +} afterEach(() => { backendStatics.UPDATE_QUIET_PERIOD_MS = originalStatics.updateQuietPeriodMs; @@ -36,9 +45,23 @@ afterEach(() => { backendStatics.LATE_FLUSH_INTERVAL_MS = originalStatics.lateFlushIntervalMs; backendStatics.LATE_FLUSH_QUIET_PERIOD_MS = originalStatics.lateFlushQuietPeriodMs; backendStatics.LATE_FLUSH_WINDOW_MS = originalStatics.lateFlushWindowMs; + if (originalPlatformDescriptor) { + Object.defineProperty(process, 'platform', originalPlatformDescriptor); + } }); describe('AcpSdkBackend', () => { + it('hides the ACP stdio shell on Windows', () => { + setPlatform('win32'); + + expect(buildAcpStdioSpawnOptions({ TEST_ENV: '1' })).toMatchObject({ + env: { TEST_ENV: '1' }, + stdio: ['pipe', 'pipe', 'pipe'], + shell: true, + windowsHide: true + }); + }); + it('allows the permission handler to resolve requests immediately', async () => { const backend = new AcpSdkBackend({ command: 'opencode' }); let capturedRequestId: string | null = null; diff --git a/cli/src/agent/backends/acp/AcpStdioTransport.ts b/cli/src/agent/backends/acp/AcpStdioTransport.ts index 7dea0524..2485388e 100644 --- a/cli/src/agent/backends/acp/AcpStdioTransport.ts +++ b/cli/src/agent/backends/acp/AcpStdioTransport.ts @@ -1,4 +1,4 @@ -import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process'; +import { spawn, type ChildProcessWithoutNullStreams, type SpawnOptions } from 'node:child_process'; import { logger } from '@/ui/logger'; import { killProcessByChildProcess } from '@/utils/process'; import { GEMINI_MODEL_PRESETS } from '@hapi/protocol'; @@ -37,6 +37,16 @@ export type AcpStderrError = { raw: string; }; +/** @internal Exported for regression tests. */ +export function buildAcpStdioSpawnOptions(env?: Record): SpawnOptions { + return { + env, + stdio: ['pipe', 'pipe', 'pipe'], + shell: process.platform === 'win32', + windowsHide: process.platform === 'win32' + }; +} + export class AcpStdioTransport { private readonly process: ChildProcessWithoutNullStreams; private readonly pending = new Map; }) { - this.process = spawn(options.command, options.args ?? [], { - env: options.env, - stdio: ['pipe', 'pipe', 'pipe'], - shell: process.platform === 'win32' - }); + this.process = spawn( + options.command, + options.args ?? [], + buildAcpStdioSpawnOptions(options.env) + ) as ChildProcessWithoutNullStreams; this.process.stdout.setEncoding('utf8'); this.process.stdout.on('data', (chunk) => this.handleStdout(chunk)); diff --git a/cli/src/claude/sdk/utils.ts b/cli/src/claude/sdk/utils.ts index 72214082..606daaaf 100644 --- a/cli/src/claude/sdk/utils.ts +++ b/cli/src/claude/sdk/utils.ts @@ -45,7 +45,8 @@ function findWhereResults(command: string): string[] { const result = execFileSync('where.exe', [command], { encoding: 'utf8', stdio: ['pipe', 'pipe', 'pipe'], - cwd: homedir() + cwd: homedir(), + windowsHide: process.platform === 'win32' }) return result diff --git a/cli/src/codex/utils/codexExecutable.ts b/cli/src/codex/utils/codexExecutable.ts index 1cba91c9..7a9e4986 100644 --- a/cli/src/codex/utils/codexExecutable.ts +++ b/cli/src/codex/utils/codexExecutable.ts @@ -15,7 +15,8 @@ function findWhereResults(command: string): string[] { const result = execFileSync('where.exe', [command], { encoding: 'utf8', stdio: ['pipe', 'pipe', 'pipe'], - cwd: homedir() + cwd: homedir(), + windowsHide: process.platform === 'win32' }); return result diff --git a/cli/src/codex/utils/codexVersion.ts b/cli/src/codex/utils/codexVersion.ts index 8e885408..aa976b78 100644 --- a/cli/src/codex/utils/codexVersion.ts +++ b/cli/src/codex/utils/codexVersion.ts @@ -58,7 +58,8 @@ export function assertCodexLocalSupported(): void { const result = spawn.sync(codexCommand.command, [...codexCommand.args, '--version'], { encoding: 'utf8', - env: withBunRuntimeEnv() + env: withBunRuntimeEnv(), + windowsHide: process.platform === 'win32' }) if (result.error) { diff --git a/cli/src/commands/claude.ts b/cli/src/commands/claude.ts index 62b44f5b..6b72a517 100644 --- a/cli/src/commands/claude.ts +++ b/cli/src/commands/claude.ts @@ -122,7 +122,12 @@ ${chalk.bold.cyan('Claude Code Options (from `claude --help`):')} const claudeHelp = execFileSync( 'claude', ['--help'], - { encoding: 'utf8', env: withBunRuntimeEnv(), shell: process.platform === 'win32' } + { + encoding: 'utf8', + env: withBunRuntimeEnv(), + shell: process.platform === 'win32', + windowsHide: process.platform === 'win32' + } ) console.log(claudeHelp) } catch { diff --git a/cli/src/cursor/cursorRemoteLauncher.ts b/cli/src/cursor/cursorRemoteLauncher.ts index 8debffe3..27b2fe72 100644 --- a/cli/src/cursor/cursorRemoteLauncher.ts +++ b/cli/src/cursor/cursorRemoteLauncher.ts @@ -197,7 +197,8 @@ class CursorRemoteLauncher extends RemoteLauncherBase { cwd, env: process.env, stdio: ['ignore', 'pipe', 'pipe'], - shell: process.platform === 'win32' + shell: process.platform === 'win32', + windowsHide: process.platform === 'win32' }); const abortHandler = () => { diff --git a/cli/src/modules/common/cursorModels.ts b/cli/src/modules/common/cursorModels.ts index 6a06e2d6..eb001104 100644 --- a/cli/src/modules/common/cursorModels.ts +++ b/cli/src/modules/common/cursorModels.ts @@ -61,7 +61,8 @@ async function runCursorModelProbe(): Promise { const child = spawn('agent', ['--list-models'], { env: process.env, stdio: ['ignore', 'pipe', 'pipe'], - shell: process.platform === 'win32' + shell: process.platform === 'win32', + windowsHide: process.platform === 'win32' }); let stdout = ''; let stderr = ''; diff --git a/cli/src/modules/difftastic/index.ts b/cli/src/modules/difftastic/index.ts index 2cadcc7b..6193f495 100644 --- a/cli/src/modules/difftastic/index.ts +++ b/cli/src/modules/difftastic/index.ts @@ -43,7 +43,8 @@ export function run(args: string[], options?: DifftasticOptions): Promise ({ import { spawnWithAbort } from './spawnWithAbort'; +const originalPlatformDescriptor = Object.getOwnPropertyDescriptor(process, 'platform'); + +function setPlatform(value: string) { + Object.defineProperty(process, 'platform', { + value, + configurable: true + }); +} + +function getSpawnOptions() { + const firstCall = spawnMock.mock.calls[0] as unknown[] | undefined; + const options = firstCall?.[2] as { windowsHide?: boolean; shell?: unknown } | undefined; + if (!options) { + throw new Error('Expected spawn options'); + } + return options; +} + function makeOptions(overrides: Partial[0]> = {}) { const controller = new AbortController(); return { @@ -55,7 +73,37 @@ describe('spawnWithAbort', () => { vi.clearAllMocks(); }); + afterEach(() => { + if (originalPlatformDescriptor) { + Object.defineProperty(process, 'platform', originalPlatformDescriptor); + } + }); + describe('normal exit (no abort)', () => { + it('hides spawned console windows on Windows by default', async () => { + setPlatform('win32'); + const { opts } = makeOptions(); + const p = spawnWithAbort(opts); + await waitForExitListener(); + + expect(getSpawnOptions().windowsHide).toBe(true); + + childEmitter.emit('exit', 0, null); + await expect(p).resolves.toBeUndefined(); + }); + + it('allows callers to explicitly keep Windows child windows visible', async () => { + setPlatform('win32'); + const { opts } = makeOptions({ windowsHide: false }); + const p = spawnWithAbort(opts); + await waitForExitListener(); + + expect(getSpawnOptions().windowsHide).toBe(false); + + childEmitter.emit('exit', 0, null); + await expect(p).resolves.toBeUndefined(); + }); + it('resolves when process exits with code 0', async () => { const { opts } = makeOptions(); const p = spawnWithAbort(opts); diff --git a/cli/src/utils/spawnWithAbort.ts b/cli/src/utils/spawnWithAbort.ts index ca2f5913..03d1d685 100644 --- a/cli/src/utils/spawnWithAbort.ts +++ b/cli/src/utils/spawnWithAbort.ts @@ -30,6 +30,7 @@ export type SpawnWithAbortOptions = { logExit?: boolean; shell?: SpawnOptions['shell']; stdio?: StdioOptions; + windowsHide?: SpawnOptions['windowsHide']; }; export async function spawnWithAbort(options: SpawnWithAbortOptions): Promise { @@ -52,7 +53,8 @@ export async function spawnWithAbort(options: SpawnWithAbortOptions): Promise { }) }) +describe('getQueuedMessagePreview', () => { + it('keeps attachment names with a text prompt', () => { + const message = { + id: 'queued-with-image', + localId: 'queued-with-image', + createdAt: 1000, + seq: null, + invokedAt: null, + status: 'queued', + content: { + role: 'user', + content: { + type: 'text', + text: 'Analyze this screenshot', + attachments: [{ + id: 'att-1', + filename: 'image.png', + mimeType: 'image/png', + size: 1234, + path: '/tmp/image.png', + }], + }, + }, + } as unknown as DecryptedMessage + + expect(getQueuedMessagePreview(message)).toEqual({ + text: 'Analyze this screenshot', + attachmentNames: ['image.png'], + }) + }) + + it('uses attachment names for attachment-only queued messages', () => { + const message = { + id: 'queued-image-only', + localId: 'queued-image-only', + createdAt: 1000, + seq: null, + invokedAt: null, + status: 'queued', + content: { + role: 'user', + content: { + type: 'text', + text: '', + attachments: [{ + id: 'att-1', + filename: 'image.png', + mimeType: 'image/png', + size: 1234, + path: '/tmp/image.png', + }], + }, + }, + } as unknown as DecryptedMessage + + expect(getQueuedMessagePreview(message)).toEqual({ + text: '', + attachmentNames: ['image.png'], + }) + }) +}) + +describe('getQueuedMessageEditText', () => { + it('keeps the prompt text when queued message has both text and attachments', () => { + expect(getQueuedMessageEditText({ + text: 'Analyze this screenshot', + attachmentNames: ['image.png'], + })).toBe('Analyze this screenshot') + }) + + it('falls back to attachment names for attachment-only queued messages', () => { + expect(getQueuedMessageEditText({ + text: '', + attachmentNames: ['image.png', 'trace.log'], + })).toBe('image.png, trace.log') + }) +}) + // --------------------------------------------------------------------------- // formatScheduledTime — cross-year support (#8) // --------------------------------------------------------------------------- diff --git a/web/src/components/AssistantChat/QueuedMessagesBar.tsx b/web/src/components/AssistantChat/QueuedMessagesBar.tsx index 5866a224..dbb6c8e1 100644 --- a/web/src/components/AssistantChat/QueuedMessagesBar.tsx +++ b/web/src/components/AssistantChat/QueuedMessagesBar.tsx @@ -75,23 +75,23 @@ function useQueuedMessages(sessionId: string): DecryptedMessage[] { }, [state]) } -function getTextFromMessage(msg: DecryptedMessage): string { +/** @internal Exported for unit testing. */ +export function getQueuedMessagePreview(msg: DecryptedMessage): { text: string; attachmentNames: string[] } { const normalized = normalizeDecryptedMessage(msg) if (!normalized || normalized.role !== 'user') { - return '' + return { text: '', attachmentNames: [] } } const text = (normalized.content.text ?? '').trim() - if (text) { - return text - } - // Attachment-only sends: the composer / POST /messages allow empty text - // when attachments are present. Fall back to the filenames so the chip - // is not blank. const attachments = normalized.content.attachments ?? [] - if (attachments.length === 0) { - return '' + return { + text, + attachmentNames: attachments.map((a) => a.filename ?? 'attachment'), } - return attachments.map((a) => a.filename ?? 'attachment').join(', ') +} + +/** @internal Exported for unit testing. */ +export function getQueuedMessageEditText(preview: { text: string; attachmentNames: string[] }): string { + return preview.text || preview.attachmentNames.join(', ') } /** @@ -203,7 +203,10 @@ export function QueuedMessagesBar({ aria-label="Queued messages" > {queued.map((msg) => { - const text = getTextFromMessage(msg) + const preview = getQueuedMessagePreview(msg) + const { text, attachmentNames } = preview + const editText = getQueuedMessageEditText(preview) + const hasAttachments = attachmentNames.length > 0 const localId = msg.localId ?? msg.id const isPending = cancelMutation.isPending && cancelMutation.variables?.localId === localId const canCancel = computeCanCancel({ id: msg.id, localId: msg.localId, isPending }) @@ -245,11 +248,11 @@ export function QueuedMessagesBar({ return } // Restore text into composer - if (text) { - assistantApi.composer().setText(text) + if (editText) { + assistantApi.composer().setText(editText) } // Restore schedule via parent callback (if provided) - onEdit?.({ text, pendingSchedule: restoredPendingSchedule }) + onEdit?.({ text: editText, pendingSchedule: restoredPendingSchedule }) }, } ) @@ -263,9 +266,25 @@ export function QueuedMessagesBar({ className="flex items-start gap-2 min-w-0 rounded-lg bg-[var(--app-secondary-bg)] px-3 py-2 shadow-sm" >
- - {text} - + {text ? ( + + {text} + + ) : null} + {hasAttachments ? ( +
+ {attachmentNames.map((name, index) => ( + + + {name} + + ))} +
+ ) : null} {msg.scheduledAt != null && msg.scheduledAt > Date.now() && (