From cfb1febfe35076ea9f3b12ab46b147809c0d2d10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=98=93=E8=89=AF?= <1204183885@qq.com> Date: Tue, 7 Jul 2026 18:51:15 +0800 Subject: [PATCH 01/11] Avoid refreshing session activity on load (#6439) * fix(core): avoid refreshing session activity on load * test(core): align resumed title source expectations * docs(core): clarify resumed title reanchor --- .../chatRecordingService.autoTitle.test.ts | 36 ++++++++-------- .../chatRecordingService.customTitle.test.ts | 42 +++++++++++++++++-- .../core/src/services/chatRecordingService.ts | 28 ++++++++----- 3 files changed, 73 insertions(+), 33 deletions(-) diff --git a/packages/core/src/services/chatRecordingService.autoTitle.test.ts b/packages/core/src/services/chatRecordingService.autoTitle.test.ts index 5061afe454..43de494d06 100644 --- a/packages/core/src/services/chatRecordingService.autoTitle.test.ts +++ b/packages/core/src/services/chatRecordingService.autoTitle.test.ts @@ -364,16 +364,14 @@ describe('ChatRecordingService - auto-title trigger', () => { expect(svc.getCurrentCustomTitle()).toBe('Auto-generated title'); expect(svc.getCurrentTitleSource()).toBe('auto'); - // finalize() was called by the constructor — drain the queued async - // write before inspecting the mock. + await svc.flush(); + expect(findCustomTitleRecord()).toBeUndefined(); + + svc.recordUserMessage([{ text: 'resume work' }]); await svc.flush(); - // The re-appended record must carry titleSource: 'auto', not 'manual'. - const finalizeRecord = vi - .mocked(jsonl.writeLine) - .mock.calls.map((c) => c[1] as ChatRecord) - .find((r) => r.type === 'system' && r.subtype === 'custom_title'); - expect(finalizeRecord?.systemPayload).toEqual({ + const reanchoredRecord = findCustomTitleRecord(); + expect(reanchoredRecord?.systemPayload).toEqual({ customTitle: 'Auto-generated title', titleSource: 'auto', }); @@ -404,12 +402,13 @@ describe('ChatRecordingService - auto-title trigger', () => { expect(svc.getCurrentCustomTitle()).toBe('User chose this'); expect(svc.getCurrentTitleSource()).toBe('manual'); await svc.flush(); + expect(findCustomTitleRecord()).toBeUndefined(); - const finalizeRecord = vi - .mocked(jsonl.writeLine) - .mock.calls.map((c) => c[1] as ChatRecord) - .find((r) => r.type === 'system' && r.subtype === 'custom_title'); - expect(finalizeRecord?.systemPayload).toEqual({ + svc.recordUserMessage([{ text: 'resume work' }]); + await svc.flush(); + + const reanchoredRecord = findCustomTitleRecord(); + expect(reanchoredRecord?.systemPayload).toEqual({ customTitle: 'User chose this', titleSource: 'manual', }); @@ -438,13 +437,14 @@ describe('ChatRecordingService - auto-title trigger', () => { // `titleSource: 'manual'` we can't actually verify. expect(svc.getCurrentTitleSource()).toBeUndefined(); await svc.flush(); + expect(findCustomTitleRecord()).toBeUndefined(); - const finalizeRecord = vi - .mocked(jsonl.writeLine) - .mock.calls.map((c) => c[1] as ChatRecord) - .find((r) => r.type === 'system' && r.subtype === 'custom_title'); + svc.recordUserMessage([{ text: 'resume work' }]); + await svc.flush(); + + const reanchoredRecord = findCustomTitleRecord(); // Payload must NOT contain a titleSource field when source is unknown. - expect(finalizeRecord?.systemPayload).toEqual({ + expect(reanchoredRecord?.systemPayload).toEqual({ customTitle: 'Legacy title', }); }); diff --git a/packages/core/src/services/chatRecordingService.customTitle.test.ts b/packages/core/src/services/chatRecordingService.customTitle.test.ts index f6a6357cd2..224d409818 100644 --- a/packages/core/src/services/chatRecordingService.customTitle.test.ts +++ b/packages/core/src/services/chatRecordingService.customTitle.test.ts @@ -135,8 +135,9 @@ describe('ChatRecordingService - recordCustomTitle', () => { }); describe('finalize', () => { - it('should re-append cached custom title to EOF', async () => { + it('should re-append cached custom title to EOF after new content', async () => { chatRecordingService.recordCustomTitle('my-feature'); + chatRecordingService.recordUserMessage([{ text: 'new work' }]); await chatRecordingService.flush(); vi.mocked(jsonl.writeLine).mockClear(); @@ -153,6 +154,17 @@ describe('ChatRecordingService - recordCustomTitle', () => { }); }); + it('should not write anything when the title is already the latest record', async () => { + chatRecordingService.recordCustomTitle('my-feature'); + await chatRecordingService.flush(); + vi.mocked(jsonl.writeLine).mockClear(); + + chatRecordingService.finalize(); + await chatRecordingService.flush(); + + expect(jsonl.writeLine).not.toHaveBeenCalled(); + }); + it('should not write anything when no custom title was set', async () => { chatRecordingService.finalize(); await chatRecordingService.flush(); @@ -160,9 +172,33 @@ describe('ChatRecordingService - recordCustomTitle', () => { expect(jsonl.writeLine).not.toHaveBeenCalled(); }); + it('should not re-append a resumed title without new content', async () => { + vi.mocked(mockConfig.getResumedSessionData).mockReturnValue({ + lastCompletedUuid: null, + } as unknown as ReturnType); + const getSessionTitleInfo = vi.fn().mockReturnValue({ + title: 'resumed-title', + source: 'manual', + }); + ( + mockConfig as unknown as { + getSessionService: () => { + getSessionTitleInfo: typeof getSessionTitleInfo; + }; + } + ).getSessionService = () => ({ getSessionTitleInfo }); + + const svc = new ChatRecordingService(mockConfig); + svc.finalize(); + await svc.flush(); + + expect(jsonl.writeLine).not.toHaveBeenCalled(); + }); + it('should re-append the latest title after multiple renames', async () => { chatRecordingService.recordCustomTitle('first-name'); chatRecordingService.recordCustomTitle('second-name'); + chatRecordingService.recordUserMessage([{ text: 'new work' }]); await chatRecordingService.flush(); vi.mocked(jsonl.writeLine).mockClear(); @@ -288,10 +324,8 @@ describe('ChatRecordingService - recordCustomTitle', () => { ).getSessionService = () => ({ getSessionTitleInfo }); const svc = new ChatRecordingService(mockConfig); - // Constructor's finalize re-appends a custom_title record on resume - // — clear it out so we can isolate the threshold-triggered re-anchor. await svc.flush(); - vi.mocked(jsonl.writeLine).mockClear(); + expect(jsonl.writeLine).not.toHaveBeenCalled(); const bulkText = 'x'.repeat(2000); for (let i = 0; i < 20; i++) { diff --git a/packages/core/src/services/chatRecordingService.ts b/packages/core/src/services/chatRecordingService.ts index 39c1c8a791..29b97dcfcd 100644 --- a/packages/core/src/services/chatRecordingService.ts +++ b/packages/core/src/services/chatRecordingService.ts @@ -564,6 +564,7 @@ export class ChatRecordingService { * returning undefined if the title is beyond both windows). */ private bytesSinceTitleAnchor = 0; + private hasNonTitleContentSinceTitleAnchor = false; constructor(config: Config) { this.config = config; @@ -577,16 +578,19 @@ export class ChatRecordingService { // resumed. Legacy records (no `titleSource` field) stay `undefined` — // treated as manual for safety without rewriting the JSONL. // - // We then re-append a custom_title record to EOF so the title stays - // within the tail window that readers scan (guarding against a crash - // before the next finalize). + // Do not re-append during construction: loading/resuming a session is a + // read operation from the user's perspective, and touching the JSONL mtime + // would make session lists treat it as fresh activity. if (config.getResumedSessionData()) { try { const sessionService = config.getSessionService(); const info = sessionService.getSessionTitleInfo(config.getSessionId()); this.currentCustomTitle = info.title; this.currentTitleSource = info.source; - this.finalize(); + if (info.title) { + // Prime the threshold so the first real content write re-anchors. + this.bytesSinceTitleAnchor = TITLE_REANCHOR_BYTES; + } } catch { // Best-effort — don't block construction } @@ -793,9 +797,11 @@ export class ChatRecordingService { private updateTitleAnchorTracking(record: ChatRecord): void { if (record.type === 'system' && record.subtype === 'custom_title') { this.bytesSinceTitleAnchor = 0; + this.hasNonTitleContentSinceTitleAnchor = false; return; } if (!this.currentCustomTitle) return; + this.hasNonTitleContentSinceTitleAnchor = true; // +1 for the trailing newline jsonl.writeLine appends. this.bytesSinceTitleAnchor += Buffer.byteLength(JSON.stringify(record), 'utf8') + 1; @@ -1319,13 +1325,10 @@ export class ChatRecordingService { } /** - * Finalizes the current session by re-appending cached metadata to EOF. - * - * Call this whenever leaving the current session — whether switching to - * another session, shutting down the process, or any other transition. - * This single entry point replaces scattered re-append calls and ensures - * the custom_title record stays within the last 64KB tail window that - * readSessionTitleFromFile() scans. + * Finalizes the current session by re-appending cached metadata to EOF, but + * only after this recorder has appended non-title content since the last + * title anchor. Pure load/resume must remain read-only so session lists do + * not treat restored sessions as newly active. * * Best-effort: errors are logged but never thrown. */ @@ -1344,6 +1347,9 @@ export class ChatRecordingService { if (!this.currentCustomTitle) { return; } + if (!this.hasNonTitleContentSinceTitleAnchor) { + return; + } try { const record: ChatRecord = { ...this.createBaseRecord('system'), From 17138b525f822dd33481c9a60f91060acf7d7423 Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Tue, 7 Jul 2026 19:32:59 +0800 Subject: [PATCH 02/11] fix(web-shell): hide rotating loading phrase in split-view pane status (#6447) Split-view panes now render a compact streaming status: the spinner, elapsed time, token count, and cancel hint stay, but the rotating "witty" loading phrase is suppressed. StreamingStatus gains a showPhrase prop (default true, so the main chat is unchanged); when false it also skips the phrase-rotation timer so each pane avoids a needless interval. --- .../client/components/ChatPane.test.tsx | 8 +++++++ .../web-shell/client/components/ChatPane.tsx | 4 +++- .../components/StreamingStatus.test.tsx | 24 +++++++++++++++++-- .../client/components/StreamingStatus.tsx | 22 ++++++++++++++--- 4 files changed, 52 insertions(+), 6 deletions(-) diff --git a/packages/web-shell/client/components/ChatPane.test.tsx b/packages/web-shell/client/components/ChatPane.test.tsx index 5a3e495e54..32923397e2 100644 --- a/packages/web-shell/client/components/ChatPane.test.tsx +++ b/packages/web-shell/client/components/ChatPane.test.tsx @@ -56,6 +56,7 @@ vi.mock('./StreamingStatus', () => ({
), })); @@ -163,6 +164,13 @@ describe('ChatPane', () => { expect(container!.textContent).toContain('Refactor core'); }); + it('suppresses the rotating loading phrase in its compact status', () => { + render(); + expect(testid('pane-streaming')?.getAttribute('data-show-phrase')).toBe( + 'false', + ); + }); + it('sends a prompt to its own session on submit', () => { render(); act(() => diff --git a/packages/web-shell/client/components/ChatPane.tsx b/packages/web-shell/client/components/ChatPane.tsx index 4ae6c265a8..f7f7e1c139 100644 --- a/packages/web-shell/client/components/ChatPane.tsx +++ b/packages/web-shell/client/components/ChatPane.tsx @@ -206,7 +206,9 @@ export function ChatPane({ title, isCurrent, onClose, onError }: ChatPaneProps) />
)} - + {/* Panes keep the composer status compact: spinner + elapsed time + + token count + cancel hint, but no rotating "witty" loading phrase. */} + { vi.restoreAllMocks(); }); -function render(customization: WebShellCustomization = {}): HTMLElement { +function render( + customization: WebShellCustomization = {}, + props: { showPhrase?: boolean } = {}, +): HTMLElement { const container = document.createElement('div'); document.body.appendChild(container); const root = createRoot(container); @@ -43,7 +46,7 @@ function render(customization: WebShellCustomization = {}): HTMLElement { root.render( - + , ); @@ -90,6 +93,23 @@ describe('StreamingStatus loading phrases', () => { ); }); + it('hides the phrase but keeps the dynamic status when showPhrase is false', () => { + pinPhraseSelection(); + // A resolver that would otherwise supply a phrase must still be suppressed. + const container = render({ loadingPhrases: () => ['should not appear'] }, { + showPhrase: false, + }); + // The witty phrase (the "废话文学") is gone: no label span. + expect(labelText(container)).toBeUndefined(); + expect(container.textContent).not.toContain('should not appear'); + // But the dynamic status stays: spinner + meta (elapsed time + cancel hint). + const spans = container.firstElementChild?.querySelectorAll('span') ?? []; + expect(spans.length).toBe(2); + expect(spans[0]?.textContent).not.toBe(''); // spinner frame + expect(container.textContent).toContain('esc to cancel'); // meta/cancel hint + expect(container.textContent).toMatch(/\ds/); // elapsed time, e.g. "0s" + }); + it('falls back to the built-in defaults when the resolver returns undefined', () => { pinPhraseSelection(); const container = render({ loadingPhrases: () => undefined }); diff --git a/packages/web-shell/client/components/StreamingStatus.tsx b/packages/web-shell/client/components/StreamingStatus.tsx index 77dc6cd551..36b48b3ce7 100644 --- a/packages/web-shell/client/components/StreamingStatus.tsx +++ b/packages/web-shell/client/components/StreamingStatus.tsx @@ -12,11 +12,21 @@ import styles from './StreamingStatus.module.css'; interface StreamingStatusProps { startedAt?: number; + /** + * When false, hide the rotating "witty" loading phrase and skip its rotation + * timer entirely — the spinner, elapsed time, token count, and cancel hint + * still render. Split-view panes pass false to keep each pane's composer + * status compact. Defaults to true (the main chat shows the phrase). + */ + showPhrase?: boolean; } const SPINNER_FRAMES = ['⠋', '⠙', '⠹', '⠸', '⠼', '⠴', '⠦', '⠧', '⠇', '⠏']; -export function StreamingStatus({ startedAt }: StreamingStatusProps) { +export function StreamingStatus({ + startedAt, + showPhrase = true, +}: StreamingStatusProps) { const streamingState = useStreamingState(); const { estimatedOutputTokens, isReceivingContent } = useStreamingLoadingMetrics(); @@ -71,6 +81,10 @@ export function StreamingStatus({ startedAt }: StreamingStatusProps) { }, [isActive, startedAt]); useEffect(() => { + // Callers that hide the phrase (e.g. split-view panes) don't need the + // rotation timer at all — bail before arming it so panes don't each run a + // needless interval and re-render on every tick. + if (!showPhrase) return; if (streamingState === 'idle') { setLoadingPhrase(resolvePhrases(language)[0] ?? ''); return; @@ -90,7 +104,7 @@ export function StreamingStatus({ startedAt }: StreamingStatusProps) { pickPhrase(); const interval = setInterval(pickPhrase, PHRASE_CHANGE_INTERVAL_MS); return () => clearInterval(interval); - }, [language, streamingState, resolvePhrases]); + }, [language, streamingState, resolvePhrases, showPhrase]); useEffect(() => { if (streamingState === 'idle') return; @@ -113,7 +127,9 @@ export function StreamingStatus({ startedAt }: StreamingStatusProps) { return (
{spinnerChar} - {loadingPhrase && {loadingPhrase}} + {showPhrase && loadingPhrase && ( + {loadingPhrase} + )} ({timeStr} {tokenStr} · {t('stream.cancel')}) From 6fdd0fc710fe34706139947d3b31894b92f1c69a Mon Sep 17 00:00:00 2001 From: jinye Date: Tue, 7 Jul 2026 19:40:15 +0800 Subject: [PATCH 03/11] fix(core): Support large text range reads (#6404) * fix(core): support large text range reads Allow text reads to stream bounded line ranges for files larger than the previous 10MB guard, while preserving media size limits and forwarding cancellation through read_file/read_many_files/ACP paths. Refs #6403 Co-authored-by: Qwen-Coder * fix(core): address large text review feedback Co-authored-by: Qwen-Coder * fix(core): propagate abort signals in text reads Co-authored-by: Qwen-Coder * fix(core): validate streamed utf8 reads Co-authored-by: Qwen-Coder * fix(core): handle disabled line truncation for large reads Co-authored-by: Qwen-Coder * fix(core): use kebab-case for text range reader Co-authored-by: Qwen-Coder * fix(core): preserve artifact size errors for large sources Co-authored-by: Qwen-Coder * fix(core): address large text review follow-up Co-authored-by: Qwen-Coder * fix(core): allow default large text reads Co-authored-by: Qwen-Coder * fix(core): honor text read byte caps Co-authored-by: Qwen-Coder * fix(core): clarify invalid utf8 range read errors Co-authored-by: Qwen-Coder * fix(core): prevent truncated full large reads Co-authored-by: Qwen-Coder * fix(core): preserve unbounded line-zero reads Co-authored-by: Qwen-Coder * fix(core): forward artifact read cancellation Pass artifact execution abort signals into source file reads and preserve cancellation semantics when the read is aborted. Add regression coverage for unbounded large UTF-8 range reads and offsets beyond EOF. Co-authored-by: Qwen-Coder * fix(core): address file read review feedback Co-authored-by: Qwen-Coder * fix(core): preserve large text mutation reads Allow default unbounded readTextFile calls to keep reading full large text files so mutation tools can prepare complete snapshots after a prior ranged read. Co-authored-by: Qwen-Coder --------- Co-authored-by: Qwen-Coder Co-authored-by: qwen-code-dev-bot --- .../service/filesystem.test.ts | 44 ++- .../src/acp-integration/service/filesystem.ts | 33 +- packages/cli/src/serve/fs/policy.ts | 12 +- .../serve/fs/workspace-file-system.test.ts | 14 + .../src/ui/hooks/atCommandProcessor.test.ts | 34 ++ .../src/services/fileSystemService.test.ts | 69 +++- .../core/src/services/fileSystemService.ts | 57 +++- .../src/tools/artifact/artifact-tool.test.ts | 38 ++- .../core/src/tools/artifact/artifact-tool.ts | 33 +- packages/core/src/tools/edit.test.ts | 33 ++ packages/core/src/tools/read-file.test.ts | 29 +- packages/core/src/tools/read-file.ts | 7 +- packages/core/src/utils/fileUtils.test.ts | 311 ++++++++++++++++-- packages/core/src/utils/fileUtils.ts | 121 ++++++- packages/core/src/utils/pathReader.test.ts | 7 +- .../core/src/utils/read-text-range.test.ts | 294 +++++++++++++++++ packages/core/src/utils/read-text-range.ts | 281 ++++++++++++++++ packages/core/src/utils/readManyFiles.test.ts | 44 +++ packages/core/src/utils/readManyFiles.ts | 30 +- .../core/src/utils/text-range-constants.ts | 8 + 20 files changed, 1417 insertions(+), 82 deletions(-) create mode 100644 packages/core/src/utils/read-text-range.test.ts create mode 100644 packages/core/src/utils/read-text-range.ts create mode 100644 packages/core/src/utils/text-range-constants.ts diff --git a/packages/cli/src/acp-integration/service/filesystem.test.ts b/packages/cli/src/acp-integration/service/filesystem.test.ts index d53c512a45..ba31dc6de5 100644 --- a/packages/cli/src/acp-integration/service/filesystem.test.ts +++ b/packages/cli/src/acp-integration/service/filesystem.test.ts @@ -107,6 +107,39 @@ describe('AcpFileSystemService', () => { }); }); + it('converts core-only read params at the ACP boundary', async () => { + const mockResponse = { + content: 'slice', + _meta: { bom: false, encoding: 'utf-8' }, + }; + const client = { + readTextFile: vi.fn().mockResolvedValue(mockResponse), + } as unknown as AgentSideConnection; + const signal = new AbortController().signal; + + const svc = new AcpFileSystemService( + client, + 'session-1', + { readTextFile: true, writeTextFile: true }, + createFallback(), + ); + + await svc.readTextFile({ + path: '/some/file.txt', + line: 0, + limit: 5, + maxOutputBytes: 1024, + signal, + }); + + expect(client.readTextFile).toHaveBeenCalledWith({ + path: '/some/file.txt', + line: 1, + limit: 5, + sessionId: 'session-1', + }); + }); + it('converts RESOURCE_NOT_FOUND error to ENOENT', async () => { const resourceNotFoundError = { code: RESOURCE_NOT_FOUND_CODE, @@ -953,11 +986,20 @@ describe('AcpFileSystemService', () => { fallback, ); - const result = await svc.readTextFile({ path: '/some/file.txt' }); + const signal = new AbortController().signal; + const result = await svc.readTextFile({ + path: '/some/file.txt', + line: 0, + maxOutputBytes: 2048, + signal, + }); expect(result).toEqual(fallbackResponse); expect(fallback.readTextFile).toHaveBeenCalledWith({ path: '/some/file.txt', + line: 0, + maxOutputBytes: 2048, + signal, }); expect(client.readTextFile).not.toHaveBeenCalled(); }); diff --git a/packages/cli/src/acp-integration/service/filesystem.ts b/packages/cli/src/acp-integration/service/filesystem.ts index 95fb1d14b6..bcc010f716 100644 --- a/packages/cli/src/acp-integration/service/filesystem.ts +++ b/packages/cli/src/acp-integration/service/filesystem.ts @@ -13,6 +13,7 @@ import type { } from '@agentclientprotocol/sdk'; import { RequestError } from '@agentclientprotocol/sdk'; import type { + CoreReadTextFileRequest, FileSystemService, ReadTextFileResponse, } from '@qwen-code/qwen-code-core'; @@ -105,6 +106,29 @@ async function resolveRealPath(value: string): Promise { } } +function toAcpReadTextFileRequest( + params: CoreReadTextFileRequest, + sessionId: string, +): ReadTextFileRequest { + // `maxOutputBytes`, `signal`, and `stats` are core-local concerns that the + // current ACP schema cannot represent. Keep this boundary explicit if the + // schema grows. + const request: ReadTextFileRequest = { + path: params.path, + sessionId, + }; + if (params._meta !== undefined) { + request._meta = params._meta; + } + if (params.limit !== undefined) { + request.limit = params.limit; + } + if (params.line != null) { + request.line = params.line + 1; + } + return request; +} + export class AcpFileSystemService implements FileSystemService { constructor( private readonly connection: AgentSideConnection, @@ -115,7 +139,7 @@ export class AcpFileSystemService implements FileSystemService { ) {} async readTextFile( - params: Omit, + params: CoreReadTextFileRequest, ): Promise { if (!this.capabilities.readTextFile) { return this.fallback.readTextFile(params); @@ -123,10 +147,9 @@ export class AcpFileSystemService implements FileSystemService { let response: ReadTextFileResponse; try { - response = await this.connection.readTextFile({ - ...params, - sessionId: this.sessionId, - }); + response = await this.connection.readTextFile( + toAcpReadTextFileRequest(params, this.sessionId), + ); } catch (error) { const errorCode = getErrorCode(error); diff --git a/packages/cli/src/serve/fs/policy.ts b/packages/cli/src/serve/fs/policy.ts index 1cd7cfe5c3..0061b6cd98 100644 --- a/packages/cli/src/serve/fs/policy.ts +++ b/packages/cli/src/serve/fs/policy.ts @@ -17,12 +17,12 @@ import type { Intent, ResolvedPath } from './paths.js'; * typical source files, small enough that an SSE replay buffer * doesn't fill on a single read. * - * Files **above** this cap are refused with `file_too_large` rather - * than truncated — the underlying `readFileWithLineAndLimit` - * reads the whole file into memory before slicing lines, so soft - * truncation past the cap would still OOM the daemon. Files - * **at or below** the cap honor a tighter `opts.maxBytes` via - * post-decode truncation (`enforceReadSize`); that's where the + * Full-snapshot reads above this cap are refused with `file_too_large` + * rather than truncated. `readText` can serve explicit line windows + * from larger files, but the default read/edit contract still needs a + * bounded snapshot for hash stability, SSE buffering, and oldText + * matching. Files at or below the cap honor a tighter `opts.maxBytes` + * via post-decode truncation (`enforceReadSize`); that's where the * `meta.truncated = true` flag fires. * * `enforceReadBytesSize` (the `readBytes` gate) and `edit()` use the diff --git a/packages/cli/src/serve/fs/workspace-file-system.test.ts b/packages/cli/src/serve/fs/workspace-file-system.test.ts index 51222c23ed..a6f9fd73d0 100644 --- a/packages/cli/src/serve/fs/workspace-file-system.test.ts +++ b/packages/cli/src/serve/fs/workspace-file-system.test.ts @@ -735,6 +735,20 @@ describe('WorkspaceFileSystem - write/edit', () => { expect(after).toBe('foo=42\nbar=2\n'); }); + it('edit() preserves the tail of files larger than the default range cap', async () => { + const target = path.join(h.workspace, 'large-edit.txt'); + const tail = 'tail-marker\n'; + const content = `foo=1\n${'body\n'.repeat(6_000)}${tail}`; + await fsp.writeFile(target, content); + const r = await h.fs.resolve('large-edit.txt', 'edit'); + + const out = await h.fs.edit(r, 'foo=1', 'foo=42'); + + expect(out.writtenBytes).toBeGreaterThan(25_000); + const after = await fsp.readFile(target, 'utf-8'); + expect(after).toBe(`foo=42\n${'body\n'.repeat(6_000)}${tail}`); + }); + it('throws parse_error when oldText is not present', async () => { const target = path.join(h.workspace, 'c.txt'); await fsp.writeFile(target, 'abc'); diff --git a/packages/cli/src/ui/hooks/atCommandProcessor.test.ts b/packages/cli/src/ui/hooks/atCommandProcessor.test.ts index 4ead67f5a8..52437f6400 100644 --- a/packages/cli/src/ui/hooks/atCommandProcessor.test.ts +++ b/packages/cli/src/ui/hooks/atCommandProcessor.test.ts @@ -156,6 +156,40 @@ describe('handleAtCommand', () => { expect(result.toolDisplays![0].status).toBe(ToolCallStatus.Success); }); + it('should attach a truncated text file larger than 10MB', async () => { + const filePath = await createTestFile( + path.join(testRootDir, 'large.log'), + 'x'.repeat(11 * 1024 * 1024), + ); + + const result = await handleAtCommand({ + query: `@${filePath}`, + config: mockConfig, + onDebugMessage: mockOnDebugMessage, + messageId: 626, + signal: abortController.signal, + }); + + const processedText = Array.isArray(result.processedQuery) + ? result.processedQuery + .map((part) => + typeof part === 'string' + ? part + : 'text' in part + ? part.text + : JSON.stringify(part), + ) + .join('') + : ''; + + expect(processedText).toContain( + 'Showing lines 1-1 of at least 1 total lines', + ); + expect(processedText).toContain('... [truncated]'); + expect(result.shouldProceed).toBe(true); + expect(result.toolDisplays![0].status).toBe(ToolCallStatus.Success); + }); + it('should only allow actual temp directory paths outside the workspace', async () => { const tempParentDir = await fsPromises.mkdtemp( path.join(os.tmpdir(), 'at-command-temp-'), diff --git a/packages/core/src/services/fileSystemService.test.ts b/packages/core/src/services/fileSystemService.test.ts index c89b40dff0..495f669662 100644 --- a/packages/core/src/services/fileSystemService.test.ts +++ b/packages/core/src/services/fileSystemService.test.ts @@ -87,7 +87,6 @@ describe('StandardFileSystemService', () => { expect(readFileWithLineAndLimit).toHaveBeenCalledWith({ path: '/test/file.txt', limit: Infinity, - line: 0, }); expect(result.content).toBe('Hello, World!'); expect(result._meta?.bom).toBe(false); @@ -116,6 +115,74 @@ describe('StandardFileSystemService', () => { expect(result._meta?.originalLineCount).toBe(100); }); + it('should preserve explicit line zero for offset reads', async () => { + vi.mocked(readFileWithLineAndLimit).mockResolvedValue({ + content: 'line 1', + bom: false, + encoding: 'utf-8', + originalLineCount: 100, + }); + + await fileSystem.readTextFile({ + path: '/test/file.txt', + line: 0, + }); + + expect(readFileWithLineAndLimit).toHaveBeenCalledWith({ + path: '/test/file.txt', + limit: Infinity, + line: 0, + }); + }); + + it('should pass maxOutputBytes and return byte-truncation metadata', async () => { + vi.mocked(readFileWithLineAndLimit).mockResolvedValue({ + content: 'partial', + bom: false, + encoding: 'utf-8', + originalLineCount: 100, + truncatedByBytes: true, + }); + + const result = await fileSystem.readTextFile({ + path: '/test/file.txt', + limit: 10, + line: 5, + maxOutputBytes: 128, + }); + + expect(readFileWithLineAndLimit).toHaveBeenCalledWith({ + path: '/test/file.txt', + limit: 10, + line: 5, + maxOutputBytes: 128, + }); + expect(result._meta?.truncatedByBytes).toBe(true); + }); + + it('should pass cached stats to readFileWithLineAndLimit', async () => { + const stats = { size: 123 } as import('node:fs').Stats; + vi.mocked(readFileWithLineAndLimit).mockResolvedValue({ + content: 'line 1', + bom: false, + encoding: 'utf-8', + originalLineCount: 1, + }); + + await fileSystem.readTextFile({ + path: '/test/file.txt', + maxOutputBytes: 128, + stats, + }); + + expect(readFileWithLineAndLimit).toHaveBeenCalledWith({ + path: '/test/file.txt', + limit: Infinity, + maxOutputBytes: 128, + stats, + }); + }); + it('should return encoding info for GBK file', async () => { vi.mocked(readFileWithLineAndLimit).mockResolvedValue({ content: '你好世界', diff --git a/packages/core/src/services/fileSystemService.ts b/packages/core/src/services/fileSystemService.ts index 4ecfd0ed7d..62189c7d36 100644 --- a/packages/core/src/services/fileSystemService.ts +++ b/packages/core/src/services/fileSystemService.ts @@ -5,6 +5,7 @@ */ import os from 'node:os'; +import type { Stats } from 'node:fs'; import * as path from 'node:path'; import { globSync } from 'glob'; import { atomicWriteFile } from '../utils/atomicFileWrite.js'; @@ -29,10 +30,26 @@ export type ReadTextFileResponse = { bom?: boolean; encoding?: string; originalLineCount?: number; + originalLineCountExact?: boolean; lineEnding?: LineEnding; + truncatedByBytes?: boolean; }; }; +export type CoreReadTextFileRequest = Omit< + ReadTextFileRequest, + 'sessionId' | 'line' +> & { + /** + * Core-local callers use 0-based line offsets. ACP protocol boundaries remain + * 1-based and convert explicitly before remote calls. + */ + line?: number | null; + maxOutputBytes?: number; + signal?: AbortSignal; + stats?: Stats; +}; + /** * Supported file encodings for new files. */ @@ -50,9 +67,7 @@ export type FileEncodingType = (typeof FileEncoding)[keyof typeof FileEncoding]; * Interface for file system operations that may be delegated to different implementations */ export interface FileSystemService { - readTextFile( - params: Omit, - ): Promise; + readTextFile(params: CoreReadTextFileRequest): Promise; writeTextFile( params: Omit, @@ -261,18 +276,32 @@ export function encodeTextFileContent( */ export class StandardFileSystemService implements FileSystemService { async readTextFile( - params: Omit, + params: CoreReadTextFileRequest, ): Promise { - const { path, limit, line } = params; - // Use encoding-aware reader that handles BOM and non-UTF-8 encodings (e.g. GBK) - const { content, bom, encoding, originalLineCount } = - await readFileWithLineAndLimit({ - path, - limit: limit ?? Number.POSITIVE_INFINITY, - line: line || 0, - }); - const lineEnding = detectLineEnding(content); - return { content, _meta: { bom, encoding, originalLineCount, lineEnding } }; + const { path, limit, line, maxOutputBytes, signal, stats } = params; + const readResult = await readFileWithLineAndLimit({ + path, + limit: limit ?? Number.POSITIVE_INFINITY, + ...(line !== undefined && line !== null ? { line } : {}), + ...(maxOutputBytes !== undefined ? { maxOutputBytes } : {}), + ...(signal !== undefined ? { signal } : {}), + ...(stats !== undefined ? { stats } : {}), + }); + const detectedLineEnding = + readResult.lineEnding ?? detectLineEnding(readResult.content); + return { + content: readResult.content, + _meta: { + bom: readResult.bom, + encoding: readResult.encoding, + originalLineCount: readResult.originalLineCount, + originalLineCountExact: readResult.originalLineCountExact, + lineEnding: detectedLineEnding, + ...(readResult.truncatedByBytes !== undefined + ? { truncatedByBytes: readResult.truncatedByBytes } + : {}), + }, + }; } async writeTextFile( diff --git a/packages/core/src/tools/artifact/artifact-tool.test.ts b/packages/core/src/tools/artifact/artifact-tool.test.ts index 84b5ec8fad..9f68d4f151 100644 --- a/packages/core/src/tools/artifact/artifact-tool.test.ts +++ b/packages/core/src/tools/artifact/artifact-tool.test.ts @@ -134,6 +134,31 @@ describe('ArtifactTool', () => { expect(res.error?.type).toBe(ToolErrorType.FILE_NOT_FOUND); }); + it('forwards cancellation signals to source file reads', async () => { + const file = path.join(workdir, 'page.html'); + const controller = new AbortController(); + const readTextFile = vi.fn(async () => ({ content: '

x

' })); + const signalAwareTool = new ArtifactTool( + { + ...makeConfig(), + getFileSystemService: () => ({ readTextFile }), + } as unknown as Config, + { + kind: 'oss', + publish: async () => ({ id: 'x', url: 'https://h/x' }), + }, + openSpy as unknown as UrlOpener, + ); + + await signalAwareTool.build({ file_path: file }).execute(controller.signal); + + expect(readTextFile).toHaveBeenCalledWith({ + path: file, + maxOutputBytes: MAX_ARTIFACT_BYTES, + signal: controller.signal, + }); + }); + it('returns EXECUTION_FAILED when the publisher throws', async () => { const file = await writeFragment('page.html', '

x

'); const failingTool = new ArtifactTool( @@ -154,11 +179,22 @@ describe('ArtifactTool', () => { expect(openSpy).not.toHaveBeenCalled(); }); - it('enforces the size cap', async () => { + it('rejects source fragments that exceed the artifact byte budget', async () => { const big = '

' + 'a'.repeat(MAX_ARTIFACT_BYTES) + '

'; const file = await writeFragment('big.html', big); const res = await tool.build({ file_path: file }).execute(signal); expect(res.error?.type).toBe(ToolErrorType.FILE_TOO_LARGE); + expect(res.error?.message).toContain('source exceeds'); + expect(res.error?.message).toContain(`${MAX_ARTIFACT_BYTES} byte limit`); + expect(openSpy).not.toHaveBeenCalled(); + }); + + it('enforces the published artifact size cap', async () => { + const almostTooBig = 'a'.repeat(MAX_ARTIFACT_BYTES - 1); + const file = await writeFragment('wrapped-big.html', almostTooBig); + const res = await tool.build({ file_path: file }).execute(signal); + expect(res.error?.type).toBe(ToolErrorType.FILE_TOO_LARGE); + expect(res.error?.message).toContain('Artifact is too large'); expect(openSpy).not.toHaveBeenCalled(); }); diff --git a/packages/core/src/tools/artifact/artifact-tool.ts b/packages/core/src/tools/artifact/artifact-tool.ts index 5dce543c61..691a88295a 100644 --- a/packages/core/src/tools/artifact/artifact-tool.ts +++ b/packages/core/src/tools/artifact/artifact-tool.ts @@ -62,6 +62,14 @@ Set artifact.autoOpen=false in settings.json, or QWEN_ARTIFACT_NO_AUTO_OPEN=1, t const debugLogger = createDebugLogger('artifact'); +function cancelledArtifactResult(): ToolResult { + const message = 'Artifact publishing was cancelled.'; + return { + llmContent: message, + returnDisplay: message, + }; +} + class ArtifactToolInvocation extends BaseToolInvocation< ArtifactToolParams, ToolResult @@ -130,11 +138,26 @@ class ArtifactToolInvocation extends BaseToolInvocation< // Read the fragment the model wrote. let fragment: string; try { - const { content } = await this.config + const { content, _meta } = await this.config .getFileSystemService() - .readTextFile({ path: file_path }); + .readTextFile({ + path: file_path, + maxOutputBytes: MAX_ARTIFACT_BYTES, + signal, + }); + if (_meta?.truncatedByBytes === true) { + const message = `Artifact is too large (source exceeds the ${MAX_ARTIFACT_BYTES} byte limit). Trim the content or split it across multiple artifacts.`; + return { + llmContent: message, + returnDisplay: message, + error: { message, type: ToolErrorType.FILE_TOO_LARGE }, + }; + } fragment = content; } catch (err) { + if (signal.aborted || isAbortError(err)) { + return cancelledArtifactResult(); + } const notFound = isNodeError(err) && err.code === 'ENOENT'; const message = notFound ? `Artifact source file not found: ${file_path}. Write the page content to this file first.` @@ -193,11 +216,7 @@ class ArtifactToolInvocation extends BaseToolInvocation< // A user-initiated cancel (Esc / aborted signal) is not a failure — // surface it as a cancellation rather than a publish error. if (signal.aborted || isAbortError(err)) { - const message = 'Artifact publishing was cancelled.'; - return { - llmContent: message, - returnDisplay: message, - }; + return cancelledArtifactResult(); } const message = `Failed to publish artifact: ${getErrorMessage(err)}`; return { diff --git a/packages/core/src/tools/edit.test.ts b/packages/core/src/tools/edit.test.ts index 15fb31ddcb..2aed6e775a 100644 --- a/packages/core/src/tools/edit.test.ts +++ b/packages/core/src/tools/edit.test.ts @@ -1313,6 +1313,39 @@ describe('EditTool', () => { expect(fs.readFileSync(filePath, 'utf8')).toBe('X\nline b\nline c\n'); }); + it('allows editing a large text file after a ranged read', async () => { + const initialContent = [ + 'target', + 'context 1', + 'context 2', + 'context 3', + 'context 4', + 'x'.repeat(11 * 1024 * 1024), + ].join('\n'); + fs.writeFileSync(filePath, initialContent, 'utf8'); + const stats = fs.statSync(filePath); + fileReadCache.recordRead(filePath, stats, { + full: false, + cacheable: true, + }); + (mockConfig.getApprovalMode as Mock).mockReturnValueOnce( + ApprovalMode.AUTO_EDIT, + ); + + const result = await tool + .build({ + file_path: filePath, + old_string: 'target', + new_string: 'updated', + }) + .execute(abortSignal); + + expect(result.error).toBeUndefined(); + expect(fs.readFileSync(filePath, 'utf8').startsWith('updated\n')).toBe( + true, + ); + }); + it('rejects an edit when the previous read was non-cacheable (binary / pdf / image)', async () => { // ReadFile records every successful read into the cache, // including binary / PDF / image reads that produce a diff --git a/packages/core/src/tools/read-file.test.ts b/packages/core/src/tools/read-file.test.ts index 276e1744bf..9b126433a0 100644 --- a/packages/core/src/tools/read-file.test.ts +++ b/packages/core/src/tools/read-file.test.ts @@ -423,9 +423,8 @@ describe('ReadFileTool', () => { }); }); - it('should return error for a file that is too large', async () => { + it('should read and truncate a text file larger than 10MB', async () => { const filePath = path.join(tempRootDir, 'largefile.txt'); - // 11MB of content exceeds 10MB limit const largeContent = 'x'.repeat(11 * 1024 * 1024); await fsp.writeFile(filePath, largeContent, 'utf-8'); const params: ReadFileToolParams = { file_path: filePath }; @@ -435,10 +434,28 @@ describe('ReadFileTool', () => { >; const result = await invocation.execute(abortSignal); - expect(result).toHaveProperty('error'); - expect(result.error?.type).toBe(ToolErrorType.FILE_TOO_LARGE); - expect(result.error?.message).toContain( - 'File size exceeds the 10MB limit', + expect(result.error).toBeUndefined(); + expect(result.returnDisplay).toBe( + 'Read lines 1-1 of at least 1 from largefile.txt (truncated)', + ); + expect(result.llmContent).toContain( + 'Showing lines 1-1 of at least 1 total lines', + ); + expect(result.llmContent).toContain('... [truncated]'); + }); + + it('should propagate an aborted signal before reading', async () => { + const filePath = path.join(tempRootDir, 'abort.txt'); + await fsp.writeFile(filePath, 'content', 'utf-8'); + const invocation = tool.build({ file_path: filePath }) as ToolInvocation< + ReadFileToolParams, + ToolResult + >; + const controller = new AbortController(); + controller.abort(); + + await expect(invocation.execute(controller.signal)).rejects.toThrow( + /abort/i, ); }); diff --git a/packages/core/src/tools/read-file.ts b/packages/core/src/tools/read-file.ts index eca595532a..c4ccc77b69 100644 --- a/packages/core/src/tools/read-file.ts +++ b/packages/core/src/tools/read-file.ts @@ -132,7 +132,7 @@ class ReadFileToolInvocation extends BaseToolInvocation< return 'ask'; } - async execute(): Promise { + async execute(signal: AbortSignal): Promise { const absPath = path.resolve(this.params.file_path); const projectRoot = this.config.getTargetDir(); // Auto-memory files (AGENTS.md and friends under the auto-memory @@ -206,6 +206,7 @@ class ReadFileToolInvocation extends BaseToolInvocation< offset: this.params.offset, limit: this.params.limit, pages: this.params.pages, + signal, }, ); @@ -284,7 +285,9 @@ class ReadFileToolInvocation extends BaseToolInvocation< ) { const [start, end] = result.linesShown!; const total = result.originalLineCount!; - llmContent = `Showing lines ${start}-${end} of ${total} total lines.\n\n---\n\n${result.llmContent}`; + const totalLabel = + result.originalLineCountExact === false ? `at least ${total}` : total; + llmContent = `Showing lines ${start}-${end} of ${totalLabel} total lines.\n\n---\n\n${result.llmContent}`; } else { llmContent = result.llmContent || ''; } diff --git a/packages/core/src/utils/fileUtils.test.ts b/packages/core/src/utils/fileUtils.test.ts index 0ff64f1b6f..c0cef12435 100644 --- a/packages/core/src/utils/fileUtils.test.ts +++ b/packages/core/src/utils/fileUtils.test.ts @@ -29,11 +29,14 @@ import { processSingleFileContent, detectBOM, decodeBufferWithEncodingInfo, + readFileWithLineAndLimit, readFileWithEncoding, readFileWithEncodingInfo, detectFileEncoding, fileExists, } from './fileUtils.js'; +import { iconvEncode } from './iconvHelper.js'; +import { LargeNonUtf8TextError } from './read-text-range.js'; import type { Config } from '../config/config.js'; import { StandardFileSystemService } from '../services/fileSystemService.js'; import { ToolErrorType } from '../tools/tool-error.js'; @@ -1993,6 +1996,149 @@ describe('fileUtils', () => { expect(result.linesShown).toEqual([1, 2]); }); + it('should preserve default full file-system reads for large text files', async () => { + const content = `head\n${'x'.repeat(11 * 1024 * 1024)}`; + actualNodeFs.writeFileSync(testTextFilePath, content); + + const result = await fsService.readTextFile({ path: testTextFilePath }); + + expect(result.content).toBe(content); + expect(result._meta?.originalLineCount).toBe(2); + expect(result._meta?.originalLineCountExact).toBe(true); + expect(result._meta?.truncatedByBytes).not.toBe(true); + }); + + it('should stream explicit offset reads for large text files', async () => { + actualNodeFs.writeFileSync( + testTextFilePath, + `skip\n${'line\n'.repeat(3 * 1024 * 1024)}`, + ); + + const result = await fsService.readTextFile({ + path: testTextFilePath, + line: 1, + }); + + expect(result.content.startsWith('line\n')).toBe(true); + expect(result.content.startsWith('skip\n')).toBe(false); + expect(result._meta?.originalLineCountExact).toBe(false); + expect(result._meta?.truncatedByBytes).toBe(true); + }); + + it('should preserve unbounded explicit line-zero reads below the large-file threshold', async () => { + const content = `head\n${'body\n'.repeat(6_000)}tail\n`; + actualNodeFs.writeFileSync(testTextFilePath, content); + + const result = await fsService.readTextFile({ + path: testTextFilePath, + line: 0, + }); + + expect(result.content).toBe(content); + expect(result._meta?.truncatedByBytes).not.toBe(true); + }); + + it('should enforce maxOutputBytes for default file-system reads below the large-file threshold', async () => { + actualNodeFs.writeFileSync(testTextFilePath, 'x'.repeat(100)); + + const result = await fsService.readTextFile({ + path: testTextFilePath, + maxOutputBytes: 10, + }); + + expect(result.content).toBe('x'.repeat(10)); + expect(result._meta?.originalLineCount).toBe(1); + expect(result._meta?.originalLineCountExact).toBe(true); + expect(result._meta?.truncatedByBytes).toBe(true); + }); + + it('should propagate large non-UTF-8 errors through bounded reads', async () => { + const gbkLine = iconvEncode('中文日志行\n', 'gbk'); + const gbkChunk = Buffer.concat( + Array.from({ length: 1024 }, () => gbkLine), + ); + const repeatCount = Math.ceil((11 * 1024 * 1024) / gbkChunk.length); + actualNodeFs.writeFileSync( + testTextFilePath, + Buffer.concat(Array.from({ length: repeatCount }, () => gbkChunk)), + ); + + await expect( + readFileWithLineAndLimit({ + path: testTextFilePath, + limit: 10, + maxOutputBytes: 10_000, + }), + ).rejects.toThrow(LargeNonUtf8TextError); + }); + + it('should propagate aborts from unbounded full reads', async () => { + actualNodeFs.writeFileSync(testTextFilePath, 'hello\nworld'); + const controller = new AbortController(); + controller.abort(); + + await expect( + readFileWithLineAndLimit({ + path: testTextFilePath, + limit: Number.POSITIVE_INFINITY, + signal: controller.signal, + }), + ).rejects.toThrow(/abort/i); + }); + + it('should propagate aborts before large unbounded full reads', async () => { + actualNodeFs.writeFileSync( + testTextFilePath, + 'x'.repeat(11 * 1024 * 1024), + ); + const controller = new AbortController(); + controller.abort(); + + await expect( + readFileWithLineAndLimit({ + path: testTextFilePath, + limit: Number.POSITIVE_INFINITY, + signal: controller.signal, + }), + ).rejects.toThrow(/abort/i); + }); + + it('should use provided stats when reading with line and byte limits', async () => { + actualNodeFs.writeFileSync(testTextFilePath, 'hello\nworld'); + const stats = actualNodeFs.statSync(testTextFilePath); + const statSpy = vi + .spyOn(fs.promises, 'stat') + .mockRejectedValueOnce(new Error('unexpected stat')); + + try { + const result = await readFileWithLineAndLimit({ + path: testTextFilePath, + limit: 1, + maxOutputBytes: 100, + stats, + }); + + expect(result.content).toBe('hello'); + expect(statSpy).not.toHaveBeenCalled(); + } finally { + statSpy.mockRestore(); + } + }); + + it('should not byte-truncate multibyte text before the character limit', async () => { + const content = '你'.repeat(1000); + actualNodeFs.writeFileSync(testTextFilePath, content, 'utf-8'); + + const result = await processSingleFileContent( + testTextFilePath, + mockConfig, + ); + + expect(result.llmContent).toBe(content); + expect(result.returnDisplay).toBe(''); + expect(result.isTruncated).toBe(false); + }); + it('should truncate long lines in text files', async () => { const longLine = 'a'.repeat(2500); actualNodeFs.writeFileSync( @@ -2073,31 +2219,152 @@ describe('fileUtils', () => { ); }); - it('should return an error if the file size exceeds 10MB', async () => { - // Create a small test file - actualNodeFs.writeFileSync(testTextFilePath, 'test content'); + it('should read large text files through bounded truncation instead of the 10MB gate', async () => { + const lines = Array.from( + { length: 65_000 }, + (_, index) => `Line ${index + 1} ${'x'.repeat(180)}`, + ); + actualNodeFs.writeFileSync(testTextFilePath, lines.join('\n')); - // Spy on fs.promises.stat to return a large file size - const statSpy = vi.spyOn(fs.promises, 'stat').mockResolvedValueOnce({ - size: 11 * 1024 * 1024, - isDirectory: () => false, - isFile: () => true, - } as fs.Stats); + const result = await processSingleFileContent( + testTextFilePath, + mockConfig, + ); - try { - const result = await processSingleFileContent( - testTextFilePath, - mockConfig, - ); + expect(result.error).toBeUndefined(); + expect(result.llmContent).toContain('Line 1'); + expect(result.returnDisplay).toContain('Read lines 1-'); + expect(result.isTruncated).toBe(true); + expect(result.originalLineCount).toBeGreaterThanOrEqual( + result.linesShown?.[1] ?? 1, + ); + expect(result.originalLineCount).toBeLessThan(65_000); + expect(result.originalLineCountExact).toBe(false); + expect(result.linesShown?.[0]).toBe(1); + }); - expect(result.error).toContain('File size exceeds the 10MB limit'); - expect(result.returnDisplay).toContain( - 'File size exceeds the 10MB limit', - ); - expect(result.llmContent).toContain('File size exceeds the 10MB limit'); - } finally { - statSpy.mockRestore(); - } + it('should stream large text files when line truncation is disabled', async () => { + actualNodeFs.writeFileSync( + testTextFilePath, + 'x'.repeat(11 * 1024 * 1024), + ); + const noLineLimitConfig = { + ...mockConfig, + getTruncateToolOutputLines: () => Number.POSITIVE_INFINITY, + } as unknown as Config; + + const result = await processSingleFileContent( + testTextFilePath, + noLineLimitConfig, + ); + + expect(result.error).toBeUndefined(); + expect(typeof result.llmContent).toBe('string'); + expect(result.llmContent).toContain('... [truncated]'); + expect(result.returnDisplay).toBe( + 'Read lines 1-1 of at least 1 from test.txt (truncated)', + ); + expect(result.isTruncated).toBe(true); + expect(result.originalLineCountExact).toBe(false); + }); + + it('should mark byte truncation metadata without character truncation', async () => { + actualNodeFs.writeFileSync(testTextFilePath, 'visible'); + const byteTruncatedConfig = { + ...mockConfig, + getTruncateToolOutputThreshold: () => Number.POSITIVE_INFINITY, + getFileSystemService: () => ({ + readTextFile: vi.fn().mockResolvedValue({ + content: 'visible', + _meta: { + originalLineCount: 1, + originalLineCountExact: false, + truncatedByBytes: true, + }, + }), + }), + } as unknown as Config; + + const result = await processSingleFileContent( + testTextFilePath, + byteTruncatedConfig, + ); + + expect(typeof result.llmContent).toBe('string'); + const llmContent = result.llmContent as string; + expect(llmContent).toBe('visible\n... [truncated]'); + expect(llmContent.match(/\.\.\. \[truncated\]/g)).toHaveLength(1); + expect(result.returnDisplay).toBe( + 'Read lines 1-1 of at least 1 from test.txt (truncated)', + ); + expect(result.isTruncated).toBe(true); + }); + + it('should use selected range as a lower bound when large file metadata is missing', async () => { + actualNodeFs.writeFileSync( + testTextFilePath, + 'x'.repeat(11 * 1024 * 1024), + ); + const missingMetadataConfig = { + ...mockConfig, + getFileSystemService: () => ({ + readTextFile: vi.fn().mockResolvedValue({ + content: 'visible\nnext', + }), + }), + } as unknown as Config; + + const result = await processSingleFileContent( + testTextFilePath, + missingMetadataConfig, + { offset: 9, limit: 2 }, + ); + + expect(result.originalLineCount).toBe(11); + expect(result.originalLineCountExact).toBe(false); + expect(result.returnDisplay).toBe( + 'Read lines 10-11 of at least 11 from test.txt', + ); + }); + + it('should preserve disabled output truncation for large text files', async () => { + const byteLength = 11 * 1024 * 1024; + actualNodeFs.writeFileSync(testTextFilePath, 'x'.repeat(byteLength)); + const noCharacterLimitConfig = { + ...mockConfig, + getTruncateToolOutputThreshold: () => Number.POSITIVE_INFINITY, + } as unknown as Config; + + const result = await processSingleFileContent( + testTextFilePath, + noCharacterLimitConfig, + ); + + expect(typeof result.llmContent).toBe('string'); + const llmContent = result.llmContent as string; + expect(llmContent).toHaveLength(byteLength); + expect(llmContent).not.toContain('... [truncated]'); + expect(result.returnDisplay).toBe(''); + expect(result.isTruncated).toBe(false); + }); + + it('should still return an error if an inline media file exceeds 10MB', async () => { + mockMimeGetType.mockReturnValue('image/png'); + actualNodeFs.writeFileSync( + testImageFilePath, + Buffer.alloc(11 * 1024 * 1024), + ); + + const result = await processSingleFileContent( + testImageFilePath, + mockConfig, + ); + + expect(result.error).toContain('File size exceeds the 10MB limit'); + expect(result.returnDisplay).toContain( + 'File size exceeds the 10MB limit', + ); + expect(result.llmContent).toContain('File size exceeds the 10MB limit'); }); it('should allow explicit page ranges above the full-PDF text-extraction size cap', async () => { diff --git a/packages/core/src/utils/fileUtils.ts b/packages/core/src/utils/fileUtils.ts index c6e03416e3..6cb62bf205 100644 --- a/packages/core/src/utils/fileUtils.ts +++ b/packages/core/src/utils/fileUtils.ts @@ -18,7 +18,7 @@ import { ToolErrorType } from '../tools/tool-error.js'; import { BINARY_EXTENSIONS } from './ignorePatterns.js'; import type { Config } from '../config/config.js'; import { createDebugLogger } from './debugLogger.js'; -import { getErrorMessage, isNodeError } from './errors.js'; +import { getErrorMessage, isAbortError, isNodeError } from './errors.js'; import type { InputModalities } from '../core/contentGenerator.js'; import { detectEncodingFromBuffer } from './systemEncoding.js'; import { @@ -35,6 +35,11 @@ import { shouldRequirePDFPageRange, } from './pdf.js'; import { readNotebookWithMetadata } from './notebook.js'; +import { readTextRange } from './read-text-range.js'; +import { + DEFAULT_RANGE_READ_BYTES, + TEXT_RANGE_FAST_PATH_MAX_SIZE, +} from './text-range-constants.js'; const debugLogger = createDebugLogger('FILE_UTILS'); @@ -273,9 +278,13 @@ function bomEncodingToName(bomEncoding: UnicodeEncoding): string { */ export async function readFileWithEncodingInfo( filePath: string, + signal?: AbortSignal, ): Promise { // Read the file once; detect BOM and decode from the single buffer. - const full = await fs.promises.readFile(filePath); + const full = await fs.promises.readFile( + filePath, + signal === undefined ? undefined : { signal }, + ); return decodeBufferWithEncodingInfo(full); } @@ -299,14 +308,42 @@ export async function readFileWithLineAndLimit(params: { path: string; limit: number; line?: number; + maxOutputBytes?: number; + signal?: AbortSignal; + stats?: import('node:fs').Stats; }): Promise<{ content: string; bom?: boolean; encoding?: string; originalLineCount: number; + originalLineCountExact?: boolean; + lineEnding?: 'crlf' | 'lf'; + truncatedByBytes?: boolean; }> { - const { path: filePath, limit, line } = params; - const { content, encoding, bom } = await readFileWithEncodingInfo(filePath); + const { path: filePath, limit, line, maxOutputBytes, signal } = params; + const stats = params.stats ?? (await fs.promises.stat(filePath)); + if ( + (line !== undefined && line > 0) || + Number.isFinite(limit) || + maxOutputBytes !== undefined + ) { + return readTextRange({ + path: filePath, + offset: line || 0, + limit, + maxOutputBytes: normalizeRangeReadByteLimit(maxOutputBytes), + stats, + ...(signal !== undefined ? { signal } : {}), + }); + } + + signal?.throwIfAborted(); + + const { content, encoding, bom } = await readFileWithEncodingInfo( + filePath, + signal, + ); + signal?.throwIfAborted(); const lines = content.split('\n'); const originalLineCount = lines.length; const startLine = line || 0; @@ -321,6 +358,7 @@ export async function readFileWithLineAndLimit(params: { bom, encoding, originalLineCount, + originalLineCountExact: true, }; } @@ -829,6 +867,7 @@ export interface ProcessedFileReadResult { error?: string; // Optional error message for the LLM if file processing failed errorType?: ToolErrorType; // Structured error type originalLineCount?: number; // For text files, the total number of lines in the original file + originalLineCountExact?: boolean; // False when a large range read stopped before EOF isTruncated?: boolean; // Indicates if displayed content was truncated linesShown?: [number, number]; // For text files [startLine, endLine] (1-based for display) /** @@ -867,6 +906,7 @@ export interface ProcessSingleFileContentOptions { * sets this after deciding the vision bridge should handle the image. */ preserveUnsupportedImage?: boolean; + signal?: AbortSignal; /** * Large full-PDF text fallback returns a tool error by default. `@`-attached * PDFs use `reference` so the model gets guidance without a failed read. @@ -945,10 +985,12 @@ export async function processSingleFileContent( limit, pages, preserveUnsupportedImage = false, + signal, largePdfBehavior = 'error', } = options; const rootDirectory = config.getTargetDir(); try { + signal?.throwIfAborted(); let stats: import('node:fs').Stats; try { // Async stat doubles as the existence check — ENOENT is handled below @@ -1101,7 +1143,7 @@ export async function processSingleFileContent( }; } } - if (fileSizeInMB > 9.9 && !willExtractPdfText) { + if (fileSizeInMB > 9.9 && !willExtractPdfText && fileType !== 'text') { return { llmContent: 'File size exceeds the 10MB limit.', returnDisplay: 'File size exceeds the 10MB limit.', @@ -1182,11 +1224,25 @@ export async function processSingleFileContent( path: filePath, limit: limit ?? config.getTruncateToolOutputLines(), line: offset, + maxOutputBytes: getRangeReadByteLimit(config), + stats, + ...(signal !== undefined ? { signal } : {}), }); - const originalLineCount = - _meta?.originalLineCount ?? (await countFileLines(filePath)); const selectedLines = content.split('\n').map((line) => line.trimEnd()); const startLine = offset || 0; + const selectedLineCount = + content.length === 0 ? 0 : selectedLines.length; + const hasOriginalLineCount = _meta?.originalLineCount !== undefined; + const originalLineCount = + _meta?.originalLineCount ?? + (stats.size >= TEXT_RANGE_FAST_PATH_MAX_SIZE + ? startLine + selectedLineCount + : await countFileLines(filePath)); + const originalLineCountExact = + _meta?.originalLineCountExact === false + ? false + : hasOriginalLineCount || + stats.size < TEXT_RANGE_FAST_PATH_MAX_SIZE; const configCharLimit = config.getTruncateToolOutputThreshold(); // Apply character limit truncation @@ -1227,17 +1283,38 @@ export async function processSingleFileContent( linesIncluded = selectedLines.length; } + if (_meta?.truncatedByBytes === true) { + const marker = '... [truncated]'; + if (!llmContent.endsWith(marker)) { + const prefix = Number.isFinite(configCharLimit) + ? llmContent.slice( + 0, + Math.max(configCharLimit - marker.length - 1, 0), + ) + : llmContent; + const separator = + prefix.length === 0 || prefix.endsWith('\n') ? '' : '\n'; + llmContent = prefix + separator + marker; + } + contentLengthTruncated = true; + } + const actualEndLine = startLine + linesIncluded; const contentRangeTruncated = - startLine > 0 || actualEndLine < originalLineCount; + startLine > 0 || + !originalLineCountExact || + actualEndLine < originalLineCount; const isTruncated = contentRangeTruncated || contentLengthTruncated; + const lineCountLabel = originalLineCountExact + ? `${originalLineCount}` + : `at least ${originalLineCount}`; // By default, return nothing to streamline the common case of a successful read_file. let returnDisplay = ''; if (isTruncated) { returnDisplay = `Read lines ${ startLine + 1 - }-${actualEndLine} of ${originalLineCount} from ${relativePathForDisplay}`; + }-${actualEndLine} of ${lineCountLabel} from ${relativePathForDisplay}`; if (contentLengthTruncated) { returnDisplay += ' (truncated)'; } @@ -1248,6 +1325,7 @@ export async function processSingleFileContent( returnDisplay, isTruncated, originalLineCount, + originalLineCountExact, linesShown: [startLine + 1, actualEndLine], stats, }; @@ -1385,6 +1463,9 @@ export async function processSingleFileContent( } } } catch (error) { + if (signal?.aborted || isAbortError(error)) { + throw error; + } const errorMessage = getErrorMessage(error); const displayPath = path .relative(rootDirectory, filePath) @@ -1398,6 +1479,28 @@ export async function processSingleFileContent( } } +function getRangeReadByteLimit(config: Config): number { + const charLimit = config.getTruncateToolOutputThreshold(); + if (charLimit === Number.POSITIVE_INFINITY) { + return Number.POSITIVE_INFINITY; + } + if (!Number.isFinite(charLimit)) { + return DEFAULT_RANGE_READ_BYTES; + } + // Leave enough byte headroom for UTF-8 text so the character budget remains + // the primary truncation control for normal source/log content. + return Math.max(DEFAULT_RANGE_READ_BYTES, Math.floor(charLimit) * 4); +} + +function normalizeRangeReadByteLimit(maxOutputBytes: number | undefined) { + if (maxOutputBytes === Number.POSITIVE_INFINITY) { + return Number.POSITIVE_INFINITY; + } + return typeof maxOutputBytes === 'number' && Number.isFinite(maxOutputBytes) + ? maxOutputBytes + : DEFAULT_RANGE_READ_BYTES; +} + export async function fileExists(filePath: string): Promise { try { await fsPromises.access(filePath, fs.constants.F_OK); diff --git a/packages/core/src/utils/pathReader.test.ts b/packages/core/src/utils/pathReader.test.ts index c6b8040d39..5b5e951acb 100644 --- a/packages/core/src/utils/pathReader.test.ts +++ b/packages/core/src/utils/pathReader.test.ts @@ -428,8 +428,7 @@ describe('readPathFromWorkspace', () => { }, ); - it('should return an error string for files exceeding the size limit', async () => { - // Mock a file slightly larger than the 10MB limit defined in fileUtils.ts + it('should truncate text files exceeding 10MB instead of failing', async () => { const largeContent = 'a'.repeat(11 * 1024 * 1024); // 11MB mock({ [CWD]: { @@ -442,7 +441,7 @@ describe('readPathFromWorkspace', () => { const config = createMockConfig(CWD, [], mockFileService); const result = await readPathFromWorkspace('large.txt', config); const textResult = result[0] as string; - // The error message comes directly from processSingleFileContent - expect(textResult).toBe('File size exceeds the 10MB limit.'); + expect(textResult).toContain('a'.repeat(100)); + expect(textResult).toContain('... [truncated]'); }); }); diff --git a/packages/core/src/utils/read-text-range.test.ts b/packages/core/src/utils/read-text-range.test.ts new file mode 100644 index 0000000000..2812045580 --- /dev/null +++ b/packages/core/src/utils/read-text-range.test.ts @@ -0,0 +1,294 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import * as fs from 'node:fs/promises'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { iconvEncode } from './iconvHelper.js'; +import { LargeNonUtf8TextError, readTextRange } from './read-text-range.js'; + +describe('readTextRange', () => { + let tempDir: string; + + beforeEach(async () => { + tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'read-text-range-')); + }); + + afterEach(async () => { + await fs.rm(tempDir, { recursive: true, force: true }); + }); + + async function writeFile( + name: string, + data: string | Buffer, + ): Promise { + const filePath = path.join(tempDir, name); + await fs.writeFile(filePath, data); + return filePath; + } + + function largeUtf8Lines(lineCount: number): string { + return Array.from( + { length: lineCount }, + (_, index) => `line-${index + 1} ${'x'.repeat(180)}`, + ).join('\n'); + } + + it('preserves split newline semantics on the fast path', async () => { + const emptyPath = await writeFile('empty.txt', ''); + await expect( + readTextRange({ + path: emptyPath, + offset: 0, + limit: 10, + maxOutputBytes: 100, + }), + ).resolves.toMatchObject({ + content: '', + originalLineCount: 1, + truncatedByBytes: false, + }); + + const trailingPath = await writeFile('trailing.txt', 'a\n'); + await expect( + readTextRange({ + path: trailingPath, + offset: 0, + limit: 10, + maxOutputBytes: 100, + }), + ).resolves.toMatchObject({ + content: 'a\n', + originalLineCount: 2, + truncatedByBytes: false, + }); + }); + + it('streams a large UTF-8 file and returns the requested range', async () => { + const filePath = await writeFile('large.log', largeUtf8Lines(65_000)); + + const result = await readTextRange({ + path: filePath, + offset: 42_000, + limit: 3, + maxOutputBytes: 10_000, + }); + + expect(result.content.split('\n')).toEqual([ + expect.stringContaining('line-42001'), + expect.stringContaining('line-42002'), + expect.stringContaining('line-42003'), + ]); + expect(result.originalLineCount).toBeGreaterThanOrEqual(42_004); + expect(result.originalLineCount).toBeLessThan(65_000); + expect(result.originalLineCountExact).toBe(false); + expect(result.encoding).toBe('utf-8'); + expect(result.bom).toBe(false); + expect(result.truncatedByBytes).toBe(false); + }); + + it('streams a large UTF-8 file from the beginning when no range is provided', async () => { + const filePath = await writeFile('large.log', largeUtf8Lines(65_000)); + + const result = await readTextRange({ + path: filePath, + maxOutputBytes: 10_000, + }); + + expect(result.content).toContain('line-1'); + expect(result.content).toContain('line-2'); + expect(result.content).not.toContain('line-65000'); + expect(result.originalLineCount).toBeGreaterThan(1); + expect(result.originalLineCountExact).toBe(false); + expect(result.truncatedByBytes).toBe(true); + }); + + it('returns empty content when a large UTF-8 range starts beyond EOF', async () => { + const filePath = await writeFile('large.log', largeUtf8Lines(65_000)); + + const result = await readTextRange({ + path: filePath, + offset: 100_000, + limit: 10, + maxOutputBytes: 10_000, + }); + + expect(result.content).toBe(''); + expect(result.originalLineCount).toBe(65_000); + expect(result.originalLineCountExact).toBe(true); + expect(result.truncatedByBytes).toBe(false); + }); + + it('preserves CRLF content and line-ending metadata for large files', async () => { + const content = Array.from( + { length: 65_000 }, + (_, index) => `line-${index + 1} ${'x'.repeat(180)}`, + ).join('\r\n'); + const filePath = await writeFile('crlf.log', content); + + const result = await readTextRange({ + path: filePath, + offset: 1, + limit: 2, + maxOutputBytes: 10_000, + }); + + expect(result.content).toContain('\r\n'); + expect(result.content.split('\n')[0]).toMatch(/\r$/); + expect(result.lineEnding).toBe('crlf'); + expect(result.originalLineCount).toBeGreaterThanOrEqual(4); + expect(result.originalLineCount).toBeLessThan(65_000); + expect(result.originalLineCountExact).toBe(false); + }); + + it('detects CRLF when the pair crosses a stream chunk boundary', async () => { + const highWaterMark = 512 * 1024; + const firstChunk = `${'a'.repeat(highWaterMark - 1)}\r`; + const body = Buffer.concat([ + Buffer.from(firstChunk), + Buffer.from('\nsecond\n'), + Buffer.alloc(11 * 1024 * 1024, 'x'), + ]); + const filePath = await writeFile('split-crlf.log', body); + + const result = await readTextRange({ + path: filePath, + offset: 0, + limit: 2, + maxOutputBytes: highWaterMark + 100, + }); + + expect(result.lineEnding).toBe('crlf'); + expect(result.content).toContain('\r\nsecond'); + }); + + it('strips UTF-8 BOM from large file content and reports BOM metadata', async () => { + const body = largeUtf8Lines(65_000); + const filePath = await writeFile( + 'bom.log', + Buffer.concat([Buffer.from([0xef, 0xbb, 0xbf]), Buffer.from(body)]), + ); + + const result = await readTextRange({ + path: filePath, + offset: 0, + limit: 1, + maxOutputBytes: 10_000, + }); + + expect(result.content.charCodeAt(0)).not.toBe(0xfeff); + expect(result.content).toContain('line-1'); + expect(result.bom).toBe(true); + expect(result.encoding).toBe('utf-8'); + expect(result.originalLineCountExact).toBe(false); + }); + + it('does not split a UTF-8 character when byte-truncating', async () => { + const filePath = await writeFile('emoji.txt', `a🙂b`); + + const result = await readTextRange({ + path: filePath, + offset: 0, + limit: 1, + maxOutputBytes: 4, + }); + + expect(result.content).toBe('a'); + expect(result.content).not.toContain('\uFFFD'); + expect(result.truncatedByBytes).toBe(true); + expect(result.originalLineCountExact).toBe(true); + }); + + it('rejects large non-UTF-8 files with a targeted error', async () => { + const gbkLine = iconvEncode('中文日志行\n', 'gbk'); + const repeatCount = Math.ceil((11 * 1024 * 1024) / gbkLine.length); + const filePath = await writeFile( + 'gbk.log', + Buffer.concat(Array.from({ length: repeatCount }, () => gbkLine)), + ); + + await expect( + readTextRange({ + path: filePath, + offset: 0, + limit: 10, + maxOutputBytes: 10_000, + }), + ).rejects.toThrow(LargeNonUtf8TextError); + }); + + it('rejects large files with invalid UTF-8 beyond the encoding sample', async () => { + const mostlyAsciiThenGbk = Buffer.concat([ + Buffer.alloc(9 * 1024, 'a'), + iconvEncode('你好', 'gbk'), + Buffer.alloc(11 * 1024 * 1024, 'b'), + ]); + const filePath = await writeFile('late-gbk.log', mostlyAsciiThenGbk); + + const promise = readTextRange({ + path: filePath, + offset: 0, + limit: 500, + maxOutputBytes: 20_000, + }); + + await expect(promise).rejects.toThrow(LargeNonUtf8TextError); + await expect(promise).rejects.toThrow(/invalid UTF-8 byte sequence/); + await expect(promise).rejects.toMatchObject({ reason: 'invalid-utf8' }); + }); + + it('bounds selected output for a large single-line file', async () => { + const filePath = await writeFile( + 'single-line.log', + 'x'.repeat(11 * 1024 * 1024), + ); + + const result = await readTextRange({ + path: filePath, + offset: 0, + limit: 1, + maxOutputBytes: 1024, + }); + + expect(result.content).toBe('x'.repeat(1024)); + expect(result.originalLineCount).toBe(1); + expect(result.originalLineCountExact).toBe(false); + expect(result.truncatedByBytes).toBe(true); + }); + + it('propagates aborts before reading large files', async () => { + const filePath = await writeFile('large.log', largeUtf8Lines(65_000)); + const controller = new AbortController(); + controller.abort(); + + await expect( + readTextRange({ + path: filePath, + offset: 0, + limit: 10, + maxOutputBytes: 10_000, + signal: controller.signal, + }), + ).rejects.toThrow(/abort/i); + }); + + it('propagates aborts while streaming large files', async () => { + const filePath = await writeFile('large.log', largeUtf8Lines(80_000)); + const controller = new AbortController(); + const promise = readTextRange({ + path: filePath, + offset: 70_000, + limit: 10, + maxOutputBytes: 10_000, + signal: controller.signal, + }); + + setTimeout(() => controller.abort(), 0); + + await expect(promise).rejects.toThrow(/abort/i); + }); +}); diff --git a/packages/core/src/utils/read-text-range.ts b/packages/core/src/utils/read-text-range.ts new file mode 100644 index 0000000000..d675372893 --- /dev/null +++ b/packages/core/src/utils/read-text-range.ts @@ -0,0 +1,281 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { createReadStream, type Stats } from 'node:fs'; +import { stat } from 'node:fs/promises'; +import { TextDecoder } from 'node:util'; +import { detectFileEncoding, readFileWithEncodingInfo } from './fileUtils.js'; +import { isUtf8CompatibleEncoding } from './iconvHelper.js'; +import { + DEFAULT_RANGE_READ_BYTES, + TEXT_RANGE_FAST_PATH_MAX_SIZE, +} from './text-range-constants.js'; + +export interface ReadTextRangeRequest { + path: string; + offset?: number; + limit?: number; + maxOutputBytes: number; + signal?: AbortSignal; + stats?: Stats; +} + +export interface ReadTextRangeResult { + content: string; + originalLineCount: number; + encoding?: string; + bom?: boolean; + lineEnding?: 'crlf' | 'lf'; + originalLineCountExact: boolean; + truncatedByBytes: boolean; +} + +export class LargeNonUtf8TextError extends Error { + constructor( + readonly encoding: string, + readonly reason?: 'invalid-utf8', + ) { + super( + reason === 'invalid-utf8' + ? 'Large text file contains invalid UTF-8 byte sequence beyond the initial encoding sample. Convert or extract a smaller UTF-8 slice and read that instead.' + : `Large non-UTF-8 text files are not supported for streaming reads (detected ${encoding}). Convert or extract a smaller UTF-8 slice and read that instead.`, + ); + this.name = 'LargeNonUtf8TextError'; + } +} + +export async function readTextRange( + request: ReadTextRangeRequest, +): Promise { + request.signal?.throwIfAborted(); + const stats = request.stats ?? (await stat(request.path)); + const maxOutputBytes = normalizeMaxBytes(request.maxOutputBytes); + + if (stats.size < TEXT_RANGE_FAST_PATH_MAX_SIZE) { + const { content, encoding, bom } = await readFileWithEncodingInfo( + request.path, + request.signal, + ); + request.signal?.throwIfAborted(); + const range = sliceDecodedContent( + content, + request.offset, + request.limit, + maxOutputBytes, + ); + return { + ...range, + encoding, + bom, + lineEnding: detectLineEndingFromContent(content), + }; + } + + return readLargeUtf8Range(request, maxOutputBytes); +} + +function normalizeMaxBytes(maxOutputBytes: number): number { + if (maxOutputBytes === Number.POSITIVE_INFINITY) { + return Number.POSITIVE_INFINITY; + } + if (!Number.isFinite(maxOutputBytes)) { + return DEFAULT_RANGE_READ_BYTES; + } + return Math.max(0, Math.floor(maxOutputBytes)); +} + +function sliceDecodedContent( + content: string, + offset: number | undefined, + limit: number | undefined, + maxOutputBytes: number, +): Pick< + ReadTextRangeResult, + | 'content' + | 'originalLineCount' + | 'originalLineCountExact' + | 'truncatedByBytes' +> { + const lines = content.split('\n'); + const originalLineCount = lines.length; + const start = Math.min(Math.max(0, offset ?? 0), originalLineCount); + const end = + limit === undefined + ? originalLineCount + : Math.min(start + Math.max(0, limit), originalLineCount); + const selected = lines.slice(start, end).join('\n'); + const truncated = truncateUtf8(selected, maxOutputBytes); + + return { + content: truncated.content, + originalLineCount, + originalLineCountExact: true, + truncatedByBytes: truncated.truncated, + }; +} + +async function readLargeUtf8Range( + request: ReadTextRangeRequest, + maxOutputBytes: number, +): Promise { + const encoding = await detectFileEncoding(request.path); + if (!isUtf8CompatibleEncoding(encoding)) { + throw new LargeNonUtf8TextError(encoding); + } + + const offset = Math.max(0, request.offset ?? 0); + const endLine = + offset + Math.max(0, request.limit ?? Number.POSITIVE_INFINITY); + let currentLine = 0; + let output = ''; + let outputBytes = 0; + let truncatedByBytes = false; + let bom = false; + let firstChunk = true; + let lineEnding: 'crlf' | 'lf' = 'lf'; + let previousChunkEndedWithCR = false; + let originalLineCountExact = true; + let stoppedEarly = false; + const decoder = new TextDecoder('utf-8', { + fatal: true, + ignoreBOM: true, + }); + + const stream = createReadStream(request.path, { + highWaterMark: 512 * 1024, + signal: request.signal, + }); + + function appendSelected(fragment: string): void { + if (fragment.length === 0 || truncatedByBytes) { + return; + } + + const available = maxOutputBytes - outputBytes; + if (available <= 0) { + truncatedByBytes = true; + return; + } + + const truncated = truncateUtf8(fragment, available); + output += truncated.content; + outputBytes += Buffer.byteLength(truncated.content, 'utf8'); + if (truncated.truncated) { + truncatedByBytes = true; + } + } + + function isSelectedLine(): boolean { + return currentLine >= offset && currentLine < endLine; + } + + function decodeUtf8Chunk( + chunk?: Buffer, + options?: TextDecodeOptions, + ): string { + try { + return decoder.decode(chunk, options); + } catch { + throw new LargeNonUtf8TextError(encoding, 'invalid-utf8'); + } + } + + try { + for await (const rawChunk of stream) { + request.signal?.throwIfAborted(); + let chunk = decodeUtf8Chunk(rawChunk as Buffer, { stream: true }); + if (firstChunk) { + firstChunk = false; + if (chunk.charCodeAt(0) === 0xfeff) { + chunk = chunk.slice(1); + bom = true; + } + } + + if ( + (previousChunkEndedWithCR && chunk.startsWith('\n')) || + chunk.includes('\r\n') + ) { + lineEnding = 'crlf'; + } + previousChunkEndedWithCR = chunk.endsWith('\r'); + + let start = 0; + let newline = chunk.indexOf('\n', start); + while (newline !== -1) { + if (isSelectedLine()) { + appendSelected(chunk.slice(start, newline)); + if (currentLine + 1 < endLine) { + appendSelected('\n'); + } + } + currentLine++; + start = newline + 1; + if (currentLine >= endLine || truncatedByBytes) { + originalLineCountExact = false; + stoppedEarly = true; + break; + } + newline = chunk.indexOf('\n', start); + } + + if (start < chunk.length && isSelectedLine()) { + appendSelected(chunk.slice(start)); + } + if (currentLine >= endLine || truncatedByBytes) { + originalLineCountExact = false; + stoppedEarly = true; + break; + } + } + } finally { + stream.destroy(); + } + + if (!stoppedEarly) { + decodeUtf8Chunk(); + } + + return { + content: output, + originalLineCount: currentLine + 1, + encoding: 'utf-8', + bom, + lineEnding, + originalLineCountExact, + truncatedByBytes, + }; +} + +function truncateUtf8( + content: string, + maxBytes: number, +): { content: string; truncated: boolean } { + const bytes = Buffer.byteLength(content, 'utf8'); + if (bytes <= maxBytes) { + return { content, truncated: false }; + } + if (maxBytes <= 0) { + return { content: '', truncated: true }; + } + + const buffer = Buffer.from(content, 'utf8'); + let end = Math.min(maxBytes, buffer.length); + // `end` is the first excluded byte. If it lands inside a multi-byte UTF-8 + // sequence, the byte at `end` is a continuation byte, so back up until the + // prefix ends before the incomplete character. + while (end > 0 && (buffer[end] & 0xc0) === 0x80) { + end--; + } + return { + content: buffer.subarray(0, end).toString('utf8'), + truncated: true, + }; +} + +function detectLineEndingFromContent(content: string): 'crlf' | 'lf' { + return content.includes('\r\n') ? 'crlf' : 'lf'; +} diff --git a/packages/core/src/utils/readManyFiles.test.ts b/packages/core/src/utils/readManyFiles.test.ts index 20d654e253..4351432c80 100644 --- a/packages/core/src/utils/readManyFiles.test.ts +++ b/packages/core/src/utils/readManyFiles.test.ts @@ -147,6 +147,22 @@ describe('readManyFiles', () => { expect(content).toContain('--- End of content ---'); }); + it('should include truncated large text files instead of reporting a size error', async () => { + const relativePath = 'large.log'; + const absolutePath = path.join(tempRootDir, relativePath); + await fs.writeFile(absolutePath, 'x'.repeat(11 * 1024 * 1024), 'utf-8'); + const mockConfig = createMockConfig(tempRootDir); + + const result = await readManyFiles(mockConfig, { paths: [relativePath] }); + + const content = contentToString(result.contentParts); + expect(content).toContain('Showing lines 1-1 of at least 1 total lines'); + expect(content).toContain('... [truncated]'); + expect(result.files).toHaveLength(1); + expect(result.files[0]!.error).toBeUndefined(); + expect(result.files[0]!.filePath).toBe(absolutePath); + }); + it('should include truncated notebooks that do not expose text line ranges', async () => { const relativePath = 'large.ipynb'; const absolutePath = path.join(tempRootDir, relativePath); @@ -276,6 +292,20 @@ describe('readManyFiles', () => { expect(content).not.toContain('Content of file1.txt'); }); + it('should propagate aborts before reading a directory', async () => { + await createTestFile('mydir', 'file1.txt'); + const mockConfig = createMockConfig(tempRootDir); + const controller = new AbortController(); + controller.abort(); + + await expect( + readManyFiles(mockConfig, { + paths: ['mydir'], + signal: controller.signal, + }), + ).rejects.toThrow(/abort/i); + }); + it('should handle directory with trailing slash', async () => { await createTestFile('mydir', 'file1.txt'); const mockConfig = createMockConfig(tempRootDir); @@ -439,6 +469,20 @@ describe('readManyFiles', () => { }); describe('per-file error surfacing', () => { + it('should propagate aborts from file reads instead of returning an error message', async () => { + const { relativePath } = await createTestFile('cancel.txt'); + const mockConfig = createMockConfig(tempRootDir); + const controller = new AbortController(); + controller.abort(); + + await expect( + readManyFiles(mockConfig, { + paths: [relativePath], + signal: controller.signal, + }), + ).rejects.toThrow(/abort/i); + }); + it('should surface processSingleFileContent errors instead of silently skipping the file', async () => { // Trigger the >10MB file-size error path in processSingleFileContent. const relativePath = 'huge.bin'; diff --git a/packages/core/src/utils/readManyFiles.ts b/packages/core/src/utils/readManyFiles.ts index 80bc835cb1..7d12b26dca 100644 --- a/packages/core/src/utils/readManyFiles.ts +++ b/packages/core/src/utils/readManyFiles.ts @@ -8,7 +8,7 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import type { Part, PartListUnion } from '@google/genai'; import type { Config } from '../config/config.js'; -import { getErrorMessage } from './errors.js'; +import { getErrorMessage, isAbortError } from './errors.js'; import type { ProcessedFileReadResult } from './fileUtils.js'; import { isCacheableReadResult, @@ -104,7 +104,11 @@ export async function readManyFiles( config: Config, options: ReadManyFilesOptions, ): Promise { - const { paths: inputPatterns, preserveUnsupportedImageForBridge } = options; + const { + paths: inputPatterns, + preserveUnsupportedImageForBridge, + signal, + } = options; const seenFiles = new Set(); const contentParts: Part[] = []; @@ -114,6 +118,7 @@ export async function readManyFiles( const projectRoot = config.getProjectRoot(); for (const rawPattern of inputPatterns) { + signal?.throwIfAborted(); const normalizedPattern = rawPattern.replace(/\\/g, '/'); const fullPath = path.resolve(projectRoot, normalizedPattern); const stats = fs.existsSync(fullPath) ? fs.statSync(fullPath) : null; @@ -122,6 +127,7 @@ export async function readManyFiles( const { contentParts: dirParts, info } = await readDirectory( config, fullPath, + signal, ); contentParts.push(...dirParts); files.push(info); @@ -134,6 +140,7 @@ export async function readManyFiles( config, fullPath, preserveUnsupportedImageForBridge, + signal, ); if (readResult) { contentParts.push(...readResult.contentParts); @@ -142,6 +149,9 @@ export async function readManyFiles( } } } catch (error) { + if (signal?.aborted || isAbortError(error)) { + throw error; + } const errorMessage = `Error during file search: ${getErrorMessage(error)}`; return { contentParts: [errorMessage], @@ -165,11 +175,14 @@ export async function readManyFiles( async function readDirectory( config: Config, directoryPath: string, + signal?: AbortSignal, ): Promise<{ contentParts: Part[]; info: FileReadInfo }> { + signal?.throwIfAborted(); const structure = await getFolderStructure(directoryPath, { fileService: config.getFileService(), fileFilteringOptions: config.getFileFilteringOptions(), }); + signal?.throwIfAborted(); const contentParts: Part[] = [ { text: `\nContent from ${directoryPath}:\n` }, @@ -190,10 +203,12 @@ async function readFileContent( config: Config, filePath: string, preserveUnsupportedImage = false, + signal?: AbortSignal, ): Promise<{ contentParts: Part[]; info: FileReadInfo } | null> { try { const fileReadResult = await processSingleFileContent(filePath, config, { preserveUnsupportedImage, + ...(signal !== undefined ? { signal } : {}), largePdfBehavior: 'reference', }); @@ -233,7 +248,11 @@ async function readFileContent( ) { const [start, end] = fileReadResult.linesShown!; const total = fileReadResult.originalLineCount!; - fileContentForLlm = `Showing lines ${start}-${end} of ${total} total lines.\n---\n${fileReadResult.llmContent}`; + const totalLabel = + fileReadResult.originalLineCountExact === false + ? `at least ${total}` + : total; + fileContentForLlm = `Showing lines ${start}-${end} of ${totalLabel} total lines.\n---\n${fileReadResult.llmContent}`; } else { fileContentForLlm = fileReadResult.llmContent; } @@ -258,7 +277,10 @@ async function readFileContent( isDirectory: false, }, }; - } catch { + } catch (error) { + if (signal?.aborted || isAbortError(error)) { + throw error; + } return null; } } diff --git a/packages/core/src/utils/text-range-constants.ts b/packages/core/src/utils/text-range-constants.ts new file mode 100644 index 0000000000..57c2b2bc8b --- /dev/null +++ b/packages/core/src/utils/text-range-constants.ts @@ -0,0 +1,8 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +export const TEXT_RANGE_FAST_PATH_MAX_SIZE = 10 * 1024 * 1024; +export const DEFAULT_RANGE_READ_BYTES = 25_000; From 9bb68b323c04b8bf8d06b972bd610c4637a9ce7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=98=93=E8=89=AF?= <1204183885@qq.com> Date: Tue, 7 Jul 2026 19:49:19 +0800 Subject: [PATCH 04/11] fix(core): strip system-reminder blocks from session title and recap side-query prompts (#6435) * fix(core): strip system-reminder pollution from session title and recap prompts The `filterToDialog` function in sessionTitle.ts and sessionRecap.ts was including system-reminder blocks (skills list, CLAUDE.md, MCP announcements) in the prompt sent to the title/recap model. When the user's first message was short, `flattenToTail` would reach back into these injected entries, causing titles like "resolve-cr-comments" instead of reflecting the actual conversation. - Skip startup prelude entries via `getStartupContextLength` - Strip `` blocks from text parts - Add shared `stripSystemReminderBlocks` helper in environmentContext.ts - Add regression tests for both sessionTitle and sessionRecap Closes #6419 * fix(core): preserve mixed reminder prompt turns --- .../core/src/services/sessionRecap.test.ts | 75 ++++++++++++ packages/core/src/services/sessionRecap.ts | 29 +++-- .../core/src/services/sessionTitle.test.ts | 113 ++++++++++++++++++ packages/core/src/services/sessionTitle.ts | 29 +++-- .../core/src/utils/environmentContext.test.ts | 49 +++++--- packages/core/src/utils/environmentContext.ts | 31 +++-- 6 files changed, 285 insertions(+), 41 deletions(-) create mode 100644 packages/core/src/services/sessionRecap.test.ts diff --git a/packages/core/src/services/sessionRecap.test.ts b/packages/core/src/services/sessionRecap.test.ts new file mode 100644 index 0000000000..173d3059be --- /dev/null +++ b/packages/core/src/services/sessionRecap.test.ts @@ -0,0 +1,75 @@ +/** + * @license + * Copyright 2025 Qwen Code + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, expect, it, vi } from 'vitest'; +import type { Content } from '@google/genai'; +import type { Config } from '../config/config.js'; +import { + SYSTEM_REMINDER_CLOSE, + SYSTEM_REMINDER_OPEN, +} from '../utils/environmentContext.js'; +import { generateSessionRecap } from './sessionRecap.js'; + +const reminder = (body: string) => + `${SYSTEM_REMINDER_OPEN}\n${body}\n${SYSTEM_REMINDER_CLOSE}`; + +describe('generateSessionRecap', () => { + it('strips startup and mid-session system reminders from recap input', async () => { + const history: Content[] = [ + { + role: 'user', + parts: [ + { text: reminder('STARTUP_SKILL_LIST') }, + { text: 'fix session title pollution' }, + ], + }, + { role: 'model', parts: [{ text: 'I found the title service.' }] }, + { role: 'user', parts: [{ text: reminder('ADDED_MCP_TOOLS') }] }, + { + role: 'user', + parts: [ + { text: reminder('PLAN_MODE_REMINDER') }, + { + text: `continue with recap coverage\n${reminder('IDE_CONTEXT')}`, + }, + ], + }, + ]; + + let captured: Content[] | null = null; + const generateText = vi.fn(async (opts: { contents: Content[] }) => { + captured = opts.contents; + return { + text: 'Fixing session title pollution. Next: verify tests.', + usage: undefined, + }; + }); + const config = { + getFastModel: vi.fn(() => 'qwen-turbo'), + getModel: vi.fn(() => 'qwen-plus'), + getGeminiClient: vi.fn(() => ({ + getHistoryShallow: () => history, + })), + getBaseLlmClient: vi.fn(() => ({ generateText })), + getOutputLanguageFilePath: vi.fn(() => undefined), + } as unknown as Config; + + const result = await generateSessionRecap( + config, + new AbortController().signal, + ); + + expect(result).toBe('Fixing session title pollution. Next: verify tests.'); + expect(captured).not.toBeNull(); + const serialized = JSON.stringify(captured); + expect(serialized).not.toContain('STARTUP_SKILL_LIST'); + expect(serialized).not.toContain('ADDED_MCP_TOOLS'); + expect(serialized).not.toContain('PLAN_MODE_REMINDER'); + expect(serialized).not.toContain('IDE_CONTEXT'); + expect(serialized).toContain('fix session title pollution'); + expect(serialized).toContain('continue with recap coverage'); + }); +}); diff --git a/packages/core/src/services/sessionRecap.ts b/packages/core/src/services/sessionRecap.ts index 456dedb90e..21ea5ec637 100644 --- a/packages/core/src/services/sessionRecap.ts +++ b/packages/core/src/services/sessionRecap.ts @@ -4,9 +4,13 @@ * SPDX-License-Identifier: Apache-2.0 */ -import type { Content } from '@google/genai'; +import type { Content, Part } from '@google/genai'; import type { Config } from '../config/config.js'; import { createDebugLogger } from '../utils/debugLogger.js'; +import { + getStartupContextLength, + stripSystemReminderBlocks, +} from '../utils/environmentContext.js'; import { runSideQuery } from '../utils/sideQuery.js'; const debugLogger = createDebugLogger('SESSION_RECAP'); @@ -142,15 +146,22 @@ function extractRecap(raw: string): string { */ function filterToDialog(history: Content[]): Content[] { const out: Content[] = []; - for (const msg of history) { + for (const msg of history.slice(getStartupContextLength(history))) { if (msg.role !== 'user' && msg.role !== 'model') continue; - const textParts = (msg.parts ?? []).filter( - (part) => - typeof part?.text === 'string' && - part.text.trim() !== '' && - !part.thought && - !part.thoughtSignature, - ); + const textParts: Part[] = []; + for (const part of msg.parts ?? []) { + if ( + typeof part?.text !== 'string' || + part.text.trim() === '' || + part.thought || + part.thoughtSignature + ) { + continue; + } + const text = stripSystemReminderBlocks(part.text); + if (text.trim() === '') continue; + textParts.push({ ...part, text }); + } if (textParts.length === 0) continue; out.push({ role: msg.role, parts: textParts }); } diff --git a/packages/core/src/services/sessionTitle.test.ts b/packages/core/src/services/sessionTitle.test.ts index 2da674f754..393530d283 100644 --- a/packages/core/src/services/sessionTitle.test.ts +++ b/packages/core/src/services/sessionTitle.test.ts @@ -7,6 +7,10 @@ import { describe, expect, it, vi } from 'vitest'; import type { Content } from '@google/genai'; import type { Config } from '../config/config.js'; +import { + SYSTEM_REMINDER_CLOSE, + SYSTEM_REMINDER_OPEN, +} from '../utils/environmentContext.js'; import { sanitizeTitle, tryGenerateSessionTitle } from './sessionTitle.js'; interface MockOptions { @@ -50,6 +54,9 @@ const DIALOG_HISTORY: Content[] = [ }, ]; +const reminder = (body: string) => + `${SYSTEM_REMINDER_OPEN}\n${body}\n${SYSTEM_REMINDER_CLOSE}`; + describe('tryGenerateSessionTitle', () => { it('returns {ok:false, reason:"no_fast_model"} when fast model is absent', async () => { const { config } = makeConfig({ @@ -225,6 +232,112 @@ describe('tryGenerateSessionTitle', () => { expect(serialized).toContain('middleware stores tokens unsafely'); }); + it('skips startup context when building the title prompt', async () => { + const history: Content[] = [ + { role: 'user', parts: [{ text: reminder('STARTUP_SKILL_LIST') }] }, + { role: 'user', parts: [{ text: 'fix auto title for short prompts' }] }, + { role: 'model', parts: [{ text: 'I will inspect title generation.' }] }, + ]; + + let captured = ''; + const { config } = makeConfig({ + fastModel: 'qwen-turbo', + history, + generateJsonResult: async (opts: unknown) => { + captured = JSON.stringify((opts as { contents: Content[] }).contents); + return { title: 'Fix auto title prompts' }; + }, + }); + + await tryGenerateSessionTitle(config, new AbortController().signal); + + expect(captured).not.toContain('STARTUP_SKILL_LIST'); + expect(captured).toContain('fix auto title for short prompts'); + }); + + it('strips system-reminder parts while keeping the real user prompt', async () => { + const history: Content[] = [ + { role: 'user', parts: [{ text: 'start with titles' }] }, + { role: 'model', parts: [{ text: 'Ready.' }] }, + { + role: 'user', + parts: [ + { text: reminder('MID_SESSION_TOOL_METADATA') }, + { text: 'please add a regression test' }, + ], + }, + { role: 'model', parts: [{ text: 'I will add the test.' }] }, + ]; + + let captured = ''; + const { config } = makeConfig({ + fastModel: 'qwen-turbo', + history, + generateJsonResult: async (opts: unknown) => { + captured = JSON.stringify((opts as { contents: Content[] }).contents); + return { title: 'Add title regression test' }; + }, + }); + + await tryGenerateSessionTitle(config, new AbortController().signal); + + expect(captured).not.toContain('MID_SESSION_TOOL_METADATA'); + expect(captured).toContain('please add a regression test'); + }); + + it('drops pure system-reminder messages from the title prompt', async () => { + const history: Content[] = [ + { role: 'user', parts: [{ text: 'fix session titles' }] }, + { role: 'model', parts: [{ text: 'I found the title service.' }] }, + { role: 'user', parts: [{ text: reminder('ADDED_MCP_TOOLS') }] }, + ]; + + let captured = ''; + const { config } = makeConfig({ + fastModel: 'qwen-turbo', + history, + generateJsonResult: async (opts: unknown) => { + captured = JSON.stringify((opts as { contents: Content[] }).contents); + return { title: 'Fix session titles' }; + }, + }); + + await tryGenerateSessionTitle(config, new AbortController().signal); + + expect(captured).not.toContain('ADDED_MCP_TOOLS'); + expect(captured).toContain('fix session titles'); + }); + + it('strips IDE-merged system reminders from title prompt text', async () => { + const history: Content[] = [ + { role: 'user', parts: [{ text: 'inspect the selected file' }] }, + { role: 'model', parts: [{ text: 'I will inspect it.' }] }, + { + role: 'user', + parts: [ + { + text: `what does this function do?\n${reminder('IDE_SKILL_CONTEXT')}`, + }, + ], + }, + ]; + + let captured = ''; + const { config } = makeConfig({ + fastModel: 'qwen-turbo', + history, + generateJsonResult: async (opts: unknown) => { + captured = JSON.stringify((opts as { contents: Content[] }).contents); + return { title: 'Inspect selected function' }; + }, + }); + + await tryGenerateSessionTitle(config, new AbortController().signal); + + expect(captured).not.toContain('IDE_SKILL_CONTEXT'); + expect(captured).toContain('what does this function do?'); + }); + it('tail-slices conversations longer than 1000 characters', async () => { // A session that pivots mid-conversation — the final topic is what the // title should reflect. Feeding the head risks titling the session by diff --git a/packages/core/src/services/sessionTitle.ts b/packages/core/src/services/sessionTitle.ts index 0d2eb71554..db15bb2b50 100644 --- a/packages/core/src/services/sessionTitle.ts +++ b/packages/core/src/services/sessionTitle.ts @@ -4,9 +4,13 @@ * SPDX-License-Identifier: Apache-2.0 */ -import type { Content } from '@google/genai'; +import type { Content, Part } from '@google/genai'; import type { Config } from '../config/config.js'; import { createDebugLogger } from '../utils/debugLogger.js'; +import { + getStartupContextLength, + stripSystemReminderBlocks, +} from '../utils/environmentContext.js'; import { runSideQuery } from '../utils/sideQuery.js'; import { stripTerminalControlSequences } from '../utils/terminalSafe.js'; import { SESSION_TITLE_MAX_LENGTH } from './sessionService.js'; @@ -205,15 +209,22 @@ export function sanitizeTitle(s: string): string { */ function filterToDialog(history: Content[]): Content[] { const out: Content[] = []; - for (const msg of history) { + for (const msg of history.slice(getStartupContextLength(history))) { if (msg.role !== 'user' && msg.role !== 'model') continue; - const textParts = (msg.parts ?? []).filter( - (part) => - typeof part?.text === 'string' && - part.text.trim() !== '' && - !part.thought && - !part.thoughtSignature, - ); + const textParts: Part[] = []; + for (const part of msg.parts ?? []) { + if ( + typeof part?.text !== 'string' || + part.text.trim() === '' || + part.thought || + part.thoughtSignature + ) { + continue; + } + const text = stripSystemReminderBlocks(part.text); + if (text.trim() === '') continue; + textParts.push({ ...part, text }); + } if (textParts.length === 0) continue; out.push({ role: msg.role, parts: textParts }); } diff --git a/packages/core/src/utils/environmentContext.test.ts b/packages/core/src/utils/environmentContext.test.ts index 27ff34d6f7..d6f6d682d3 100644 --- a/packages/core/src/utils/environmentContext.test.ts +++ b/packages/core/src/utils/environmentContext.test.ts @@ -29,6 +29,7 @@ import { getInitialChatHistory, getStartupContextLength, isSystemReminderContent, + stripSystemReminderBlocks, stripStartupContext, formatDateForContext, SYSTEM_REMINDER_OPEN, @@ -325,30 +326,17 @@ describe('getInitialChatHistory', () => { }); it('places deferred-tools reminder last so stable prefix stays cacheable on KV-caching servers', async () => { - // Pin: deferred-tools part changes when tool_search reveals a tool. - // Placing it LAST keeps the stable prefix (MCP + skills + startup) - // cacheable; only the tail recomputes on prefix-caching servers. mockToolRegistry.getDeferredToolSummary.mockReturnValue([ { name: 'web_fetch', description: 'Fetches web pages' }, ]); const [history] = await getInitialChatHistory(mockConfig as Config); - expect(history).toHaveLength(1); const parts = history[0]?.parts ?? []; - expect(parts.length).toBeGreaterThanOrEqual(1); - - // The LAST text part should be the deferred-tools reminder. const lastText = parts[parts.length - 1]?.text; - expect(lastText).toBeDefined(); expect(lastText).toContain('reachable via `tool_search`'); expect(lastText).toContain('web_fetch'); - - // The FIRST text part should NOT be the deferred-tools reminder — - // it must be the stable MCP/skills/startup prefix. - const firstText = parts[0]?.text; - expect(firstText).toBeDefined(); - expect(firstText).not.toContain('reachable via `tool_search`'); + expect(parts[0]?.text).not.toContain('reachable via `tool_search`'); }); }); @@ -399,6 +387,20 @@ describe('stripStartupContext', () => { ).toEqual([{ role: 'user', parts: [{ text: 'Hello' }] }]); }); + it('keeps a first user turn that mixes a reminder part with a prompt part', () => { + const history: Content[] = [ + { + role: 'user', + parts: [ + { text: '\nctx\n' }, + { text: 'real prompt' }, + ], + }, + ]; + + expect(stripStartupContext(history)).toEqual(history); + }); + it('should round-trip with getInitialChatHistory', async () => { const mockConfig = { getSkipStartupContext: vi.fn().mockReturnValue(false), @@ -430,6 +432,25 @@ describe('stripStartupContext', () => { }); }); +describe('stripSystemReminderBlocks', () => { + it('strips complete reminder blocks and preserves surrounding text', () => { + expect( + stripSystemReminderBlocks('axb'), + ).toBe('ab'); + expect( + stripSystemReminderBlocks( + 'axbyc', + ), + ).toBe('abc'); + }); + + it('drops a trailing unclosed reminder block', () => { + expect(stripSystemReminderBlocks('keep secret')).toBe( + 'keep ', + ); + }); +}); + describe('formatDateForContext', () => { it('should format date in en-US locale regardless of system timezone', () => { expect(formatDateForContext(new Date('2026-06-05T12:00:00Z'))).toBe( diff --git a/packages/core/src/utils/environmentContext.ts b/packages/core/src/utils/environmentContext.ts index 94b20ad2af..b24d8f3d52 100644 --- a/packages/core/src/utils/environmentContext.ts +++ b/packages/core/src/utils/environmentContext.ts @@ -593,15 +593,7 @@ export function getStartupContextLength( ): number { const firstEntry = history[0]; if (firstEntry?.role !== 'user') return 0; - const firstText = firstEntry.parts?.[0]?.text; - // Open prefix, and close tag AT THE END (not merely present). Excludes a - // prompt quoting the literal tag, and — since IDE mode merges the reminder - // into the prompt's text part — a real first turn trailing after the close. - if ( - typeof firstText === 'string' && - firstText.startsWith(SYSTEM_REMINDER_OPEN) && - firstText.trimEnd().endsWith(SYSTEM_REMINDER_CLOSE) - ) { + if (isSystemReminderContent(firstEntry)) { if (options.includeCompressed) { const compressedLength = detectCompressedPrefixLength(history, 1); if (compressedLength > 0) return 1 + compressedLength; @@ -712,6 +704,27 @@ export function isSystemReminderContent(content: Content): boolean { ); } +export function stripSystemReminderBlocks(text: string): string { + let out = ''; + let offset = 0; + + while (offset < text.length) { + const open = text.indexOf(SYSTEM_REMINDER_OPEN, offset); + if (open === -1) return out + text.slice(offset); + + const close = text.indexOf( + SYSTEM_REMINDER_CLOSE, + open + SYSTEM_REMINDER_OPEN.length, + ); + if (close === -1) return out + text.slice(offset, open); + + out += text.slice(offset, open); + offset = close + SYSTEM_REMINDER_CLOSE.length; + } + + return out; +} + /** * Strip the leading startup context reminder from a chat history. Used when * forwarding a parent session's history to a child agent that will generate From df6be035e15f4cea8b6e228829b56c0c6d1ca9b6 Mon Sep 17 00:00:00 2001 From: Salman Chishti <13schishti@gmail.com> Date: Tue, 7 Jul 2026 20:17:22 +0800 Subject: [PATCH 05/11] Upgrade GitHub Actions for Node 24 compatibility (#5157) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Upgrade GitHub Actions for Node 24 compatibility Signed-off-by: Salman Muin Kayser Chishti <13schishti@gmail.com> * ci: update remaining checkout pins --------- Signed-off-by: Salman Muin Kayser Chishti <13schishti@gmail.com> Co-authored-by: qwen-code-dev-bot Co-authored-by: 易良 <1204183885@qq.com> Co-authored-by: yiliang114 --- .github/workflows/audio-capture-prebuilds.yml | 2 +- .github/workflows/build-and-publish-image.yml | 2 +- .github/workflows/ci.yml | 10 +++++----- .github/workflows/codeql.yml | 2 +- .github/workflows/desktop-release.yml | 4 ++-- .github/workflows/docs-page-action.yml | 2 +- .github/workflows/e2e.yml | 4 ++-- .github/workflows/gemini-scheduled-pr-triage.yml | 2 +- .github/workflows/qwen-autofix.yml | 4 ++-- .github/workflows/qwen-code-pr-review.yml | 4 ++-- .github/workflows/qwen-pr-safety-precheck.yml | 2 +- .github/workflows/qwen-triage.yml | 4 ++-- .github/workflows/release-sdk-python.yml | 2 +- .github/workflows/release-sdk.yml | 2 +- .github/workflows/release-vscode-companion.yml | 6 +++--- .github/workflows/release.yml | 10 +++++----- .github/workflows/sdk-python.yml | 2 +- .github/workflows/stale.yml | 2 +- .github/workflows/sync-cua-driver-to-oss.yml | 2 +- .github/workflows/sync-release-to-oss.yml | 2 +- .github/workflows/terminal-bench.yml | 2 +- 21 files changed, 36 insertions(+), 36 deletions(-) diff --git a/.github/workflows/audio-capture-prebuilds.yml b/.github/workflows/audio-capture-prebuilds.yml index 65f4ec4184..4716ca541d 100644 --- a/.github/workflows/audio-capture-prebuilds.yml +++ b/.github/workflows/audio-capture-prebuilds.yml @@ -50,7 +50,7 @@ jobs: runner: 'windows-2022' arch: 'x64' steps: - - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + - uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 - uses: 'actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e' # v6.4.0 with: node-version: '22' diff --git a/.github/workflows/build-and-publish-image.yml b/.github/workflows/build-and-publish-image.yml index b682771c75..0c0daba602 100644 --- a/.github/workflows/build-and-publish-image.yml +++ b/.github/workflows/build-and-publish-image.yml @@ -28,7 +28,7 @@ jobs: steps: - name: 'Checkout repository' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: '${{ github.ref }}' diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7b2ec4a01d..5d40f391a9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -136,7 +136,7 @@ jobs: - name: 'Checkout' id: 'checkout' if: "${{ needs.classify_pr.outputs.skip_ci != 'true' }}" - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: "${{ github.event.inputs.branch_ref || (github.event_name == 'pull_request' && format('refs/pull/{0}/head', github.event.pull_request.number)) || (github.event_name == 'merge_group' && github.event.merge_group.head_sha) || github.ref }}" # Shallow: nothing here walks git history (the verify guard below checks @@ -386,7 +386,7 @@ jobs: - name: 'Checkout' id: 'checkout' if: "${{ needs.classify_pr.outputs.skip_ci != 'true' }}" - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: "${{ github.event.inputs.branch_ref || (github.event_name == 'pull_request' && format('refs/pull/{0}/head', github.event.pull_request.number)) || (github.event_name == 'merge_group' && github.event.merge_group.head_sha) || github.ref }}" @@ -441,7 +441,7 @@ jobs: - name: 'Checkout' id: 'checkout' if: "${{ needs.classify_pr.outputs.skip_ci != 'true' }}" - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: "${{ github.event.inputs.branch_ref || (github.event_name == 'pull_request' && format('refs/pull/{0}/head', github.event.pull_request.number)) || (github.event_name == 'merge_group' && github.event.merge_group.head_sha) || github.ref }}" @@ -510,7 +510,7 @@ jobs: - '22.x' steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 - name: 'Download coverage reports artifact' uses: 'actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c' # v8.0.1 @@ -557,7 +557,7 @@ jobs: OPENAI_MODEL: '${{ secrets.OPENAI_MODEL }}' steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: "${{ github.event.inputs.branch_ref || (github.event_name == 'merge_group' && github.event.merge_group.head_sha) || github.ref }}" # Shallow, mirroring the Ubuntu gate: nothing here walks git history, diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index ded8856643..6b4c33c8fb 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -30,7 +30,7 @@ jobs: timeout-minutes: 30 steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 - name: 'Initialize CodeQL' uses: 'github/codeql-action/init@df559355d593797519d70b90fc8edd5db049e7a2' # ratchet:github/codeql-action/init@v3 diff --git a/.github/workflows/desktop-release.yml b/.github/workflows/desktop-release.yml index c730fbede1..1f8fe3f0b7 100644 --- a/.github/workflows/desktop-release.yml +++ b/.github/workflows/desktop-release.yml @@ -76,7 +76,7 @@ jobs: steps: - name: 'Check out source' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: fetch-depth: 0 @@ -234,7 +234,7 @@ jobs: steps: - name: 'Check out source' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: '${{ needs.release_metadata.outputs.release_ref }}' diff --git a/.github/workflows/docs-page-action.yml b/.github/workflows/docs-page-action.yml index 2a10ba62d9..0d2352285f 100644 --- a/.github/workflows/docs-page-action.yml +++ b/.github/workflows/docs-page-action.yml @@ -24,7 +24,7 @@ jobs: runs-on: 'ubuntu-latest' steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 - name: 'Setup Pages' uses: 'actions/configure-pages@45bfe0192ca1faeb007ade9deae92b16b8254a0d' # ratchet:actions/configure-pages@v6 diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index d6d13addb0..c5afbf1d38 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -44,7 +44,7 @@ jobs: - '22.x' steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 - name: 'Set up Node.js ${{ matrix.node-version }}' uses: 'actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e' # v6.4.0 @@ -109,7 +109,7 @@ jobs: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository }} steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 - name: 'Set up Node.js' uses: 'actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e' # v6.4.0 diff --git a/.github/workflows/gemini-scheduled-pr-triage.yml b/.github/workflows/gemini-scheduled-pr-triage.yml index bc6830c2e0..77119672cf 100644 --- a/.github/workflows/gemini-scheduled-pr-triage.yml +++ b/.github/workflows/gemini-scheduled-pr-triage.yml @@ -20,7 +20,7 @@ jobs: prs_needing_comment: '${{ steps.run_triage.outputs.prs_needing_comment }}' steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 - name: 'Run PR Triage Script' id: 'run_triage' diff --git a/.github/workflows/qwen-autofix.yml b/.github/workflows/qwen-autofix.yml index 851bbd62c6..53474e159e 100644 --- a/.github/workflows/qwen-autofix.yml +++ b/.github/workflows/qwen-autofix.yml @@ -215,7 +215,7 @@ jobs: AUTOFIX_ISSUE_EXCLUDES: 'no:assignee -linked:pr -label:autofix/skip -label:autofix/in-progress -label:status/need-information -label:status/need-retesting sort:created-desc' steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: fetch-depth: 0 persist-credentials: false @@ -997,7 +997,7 @@ jobs: WATERMARK: '${{ matrix.target.watermark }}' steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: fetch-depth: 0 persist-credentials: false diff --git a/.github/workflows/qwen-code-pr-review.yml b/.github/workflows/qwen-code-pr-review.yml index 7b7c4983eb..141772e813 100644 --- a/.github/workflows/qwen-code-pr-review.yml +++ b/.github/workflows/qwen-code-pr-review.yml @@ -387,7 +387,7 @@ jobs: # SECURITY: checkout trusted base code; /review fetches PR diff context. - name: 'Checkout base branch' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: '${{ github.event.repository.default_branch }}' fetch-depth: 0 @@ -895,7 +895,7 @@ jobs: echo "pr_number=${pr_number}" >> "$GITHUB_OUTPUT" - name: 'Checkout base branch' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: '${{ github.event.repository.default_branch }}' fetch-depth: 0 diff --git a/.github/workflows/qwen-pr-safety-precheck.yml b/.github/workflows/qwen-pr-safety-precheck.yml index 2bded93945..53222af08c 100644 --- a/.github/workflows/qwen-pr-safety-precheck.yml +++ b/.github/workflows/qwen-pr-safety-precheck.yml @@ -25,7 +25,7 @@ jobs: decision: '${{ steps.assess.outputs.decision }}' steps: - name: 'Checkout trusted precheck script' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: '${{ github.event.repository.default_branch }}' sparse-checkout: '.github/scripts/pr-safety-precheck.mjs' diff --git a/.github/workflows/qwen-triage.yml b/.github/workflows/qwen-triage.yml index a3c59ce7ec..8329324e82 100644 --- a/.github/workflows/qwen-triage.yml +++ b/.github/workflows/qwen-triage.yml @@ -244,7 +244,7 @@ jobs: echo "stale agent state cleaned" - name: 'Checkout repo' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: token: '${{ secrets.GITHUB_TOKEN }}' @@ -442,7 +442,7 @@ jobs: - name: 'Checkout PR merge ref' if: "steps.pr.outputs.decision == 'run'" - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: # Untrusted PR code — keep the token out of .git/config. persist-credentials: false diff --git a/.github/workflows/release-sdk-python.yml b/.github/workflows/release-sdk-python.yml index 7b1e63c274..27a343515d 100644 --- a/.github/workflows/release-sdk-python.yml +++ b/.github/workflows/release-sdk-python.yml @@ -100,7 +100,7 @@ jobs: fi - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: '${{ github.event.inputs.ref || github.sha }}' fetch-depth: 0 diff --git a/.github/workflows/release-sdk.yml b/.github/workflows/release-sdk.yml index 2cf54c66ec..25564a3aab 100644 --- a/.github/workflows/release-sdk.yml +++ b/.github/workflows/release-sdk.yml @@ -64,7 +64,7 @@ jobs: steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: '${{ github.event.inputs.ref || github.sha }}' fetch-depth: 0 diff --git a/.github/workflows/release-vscode-companion.yml b/.github/workflows/release-vscode-companion.yml index 5e38991711..d1fb8b01a7 100644 --- a/.github/workflows/release-vscode-companion.yml +++ b/.github/workflows/release-vscode-companion.yml @@ -60,7 +60,7 @@ jobs: steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: '${{ github.event.release.tag_name || github.event.inputs.ref || github.sha }}' fetch-depth: 0 @@ -200,7 +200,7 @@ jobs: steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: '${{ github.event.release.tag_name || github.event.inputs.ref || github.sha }}' fetch-depth: 0 @@ -285,7 +285,7 @@ jobs: steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: '${{ github.event.release.tag_name || github.event.inputs.ref || github.sha }}' diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 8eb0687272..096b5850ed 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -57,7 +57,7 @@ jobs: steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: '${{ github.event.inputs.ref || github.sha }}' fetch-depth: 0 @@ -154,7 +154,7 @@ jobs: steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: '${{ github.event.inputs.ref || github.sha }}' fetch-depth: 0 @@ -212,7 +212,7 @@ jobs: steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: '${{ github.event.inputs.ref || github.sha }}' fetch-depth: 0 @@ -259,7 +259,7 @@ jobs: steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: '${{ github.event.inputs.ref || github.sha }}' fetch-depth: 0 @@ -347,7 +347,7 @@ jobs: steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: # Persist the bot PAT for release-branch pushes so downstream CI # workflows are triggered. diff --git a/.github/workflows/sdk-python.yml b/.github/workflows/sdk-python.yml index b3abf07e94..ce53710e7d 100644 --- a/.github/workflows/sdk-python.yml +++ b/.github/workflows/sdk-python.yml @@ -72,7 +72,7 @@ jobs: python-version: ['3.10', '3.11', '3.12'] steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 - name: 'Set up Python' uses: 'actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405' # v6.2.0 diff --git a/.github/workflows/stale.yml b/.github/workflows/stale.yml index f419cfe8db..bf3401e657 100644 --- a/.github/workflows/stale.yml +++ b/.github/workflows/stale.yml @@ -18,7 +18,7 @@ jobs: group: '${{ github.workflow }}-stale' cancel-in-progress: true steps: - - uses: 'actions/stale@b5d41d4e1d5dceea10e7104786b73624c18a190f' # v10.2.0 + - uses: 'actions/stale@eb5cf3af3ac0a1aa4c9c45633dd1ae542a27a899' # v10.3.0 with: repo-token: '${{ secrets.GITHUB_TOKEN }}' # Issues are intentionally disabled here; a separate policy will diff --git a/.github/workflows/sync-cua-driver-to-oss.yml b/.github/workflows/sync-cua-driver-to-oss.yml index 777fc57423..652c05dd3c 100644 --- a/.github/workflows/sync-cua-driver-to-oss.yml +++ b/.github/workflows/sync-cua-driver-to-oss.yml @@ -45,7 +45,7 @@ jobs: contents: 'read' steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 - name: 'Resolve cua-driver version' id: 'meta' diff --git a/.github/workflows/sync-release-to-oss.yml b/.github/workflows/sync-release-to-oss.yml index c2eee4c5fb..12dc1183c3 100644 --- a/.github/workflows/sync-release-to-oss.yml +++ b/.github/workflows/sync-release-to-oss.yml @@ -30,7 +30,7 @@ jobs: steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: '${{ env.RELEASE_TAG }}' diff --git a/.github/workflows/terminal-bench.yml b/.github/workflows/terminal-bench.yml index 4897ad8aa5..e6daa65ecb 100644 --- a/.github/workflows/terminal-bench.yml +++ b/.github/workflows/terminal-bench.yml @@ -26,7 +26,7 @@ jobs: - 'swe-bench-astropy-1' steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: submodules: 'recursive' - name: 'Install uv and set the python version' From 010bedfc882bfe7ca5bfa09471a74db44926d614 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=98=93=E8=89=AF?= <1204183885@qq.com> Date: Tue, 7 Jul 2026 20:27:13 +0800 Subject: [PATCH 06/11] fix(autofix): report review handoff failures (#6415) * fix(autofix): report review handoff failures * fix(autofix): distinguish terminal review handoffs * fix(autofix): harden review handoff reporting * test(autofix): cover agent failure handoff marker * fix(autofix): avoid hanging on destroyed log stream * fix(autofix): harden handoff failure reporting * fix(autofix): detect split loop guard output * fix(autofix): kill timed-out qwen process group --- .github/workflows/qwen-autofix.yml | 36 ++++- .qwen/skills/autofix/scripts/run-agent.mjs | 126 ++++++++++++++-- scripts/tests/qwen-autofix-workflow.test.js | 157 +++++++++++++++++++- 3 files changed, 302 insertions(+), 17 deletions(-) diff --git a/.github/workflows/qwen-autofix.yml b/.github/workflows/qwen-autofix.yml index 53474e159e..861a537c48 100644 --- a/.github/workflows/qwen-autofix.yml +++ b/.github/workflows/qwen-autofix.yml @@ -1281,7 +1281,7 @@ jobs: if git rev-parse --verify "${BRANCH}" > /dev/null 2>&1; then git diff "origin/main...${BRANCH}" > "${WORKDIR}/pr.diff" || true fi - for f in feedback.md address-summary.md no-action.md failure.md pr.diff; do + for f in feedback.md address-summary.md no-action.md failure.md handoff.md pr.diff; do if [[ -f "${WORKDIR}/${f}" ]]; then echo "=============== ${f} ===============" cat "${WORKDIR}/${f}" @@ -1382,6 +1382,8 @@ jobs: OUTCOME: '${{ steps.verify.outputs.outcome }}' CONFLICT: '${{ steps.prepare.outputs.conflict }}' DRY_RUN: '${{ needs.route.outputs.dry_run }}' + GITHUB_TOKEN: '${{ secrets.CI_DEV_BOT_PAT }}' + NEWEST: '${{ steps.prepare.outputs.newest }}' run: |- SUFFIX='' [[ "${DRY_RUN}" == "true" ]] && SUFFIX=' (dry-run, nothing pushed)' @@ -1389,7 +1391,7 @@ jobs: echo "### PR #${PR} (issue #${ISSUE}) — outcome=${OUTCOME:-unknown}${SUFFIX}" echo "- Base conflict: ${CONFLICT:-unknown}" echo - for f in address-summary.md no-action.md failure.md; do + for f in address-summary.md no-action.md failure.md handoff.md; do if [[ -s "${WORKDIR}/${f}" ]]; then echo "**${f}:**" cat "${WORKDIR}/${f}" @@ -1397,3 +1399,33 @@ jobs: fi done } >> "${GITHUB_STEP_SUMMARY}" + + if [[ "${DRY_RUN}" != "true" && "${OUTCOME:-unknown}" == "failed" && -n "${NEWEST:-}" && -n "${GITHUB_TOKEN:-}" && -s "${WORKDIR}/handoff.md" ]]; then + api_error_file="$(mktemp)" + if ! bot_actor="$(GH_TOKEN="${GITHUB_TOKEN}" gh api user --jq '.login' 2>"${api_error_file}")"; then + api_error="$(tr '\r\n' ' ' < "${api_error_file}")" + rm -f "${api_error_file}" + echo "::error::Failed to verify CI_DEV_BOT_PAT identity with gh api user: ${api_error:-unknown error}." + exit 1 + fi + rm -f "${api_error_file}" + echo "CI_DEV_BOT_PAT authenticates as ${bot_actor}" + if [[ "${bot_actor}" != "${AUTOFIX_BOT}" ]]; then + echo "::error::CI_DEV_BOT_PAT authenticates as ${bot_actor}; expected ${AUTOFIX_BOT}." + exit 1 + fi + { + echo "🤖 Could not address the latest review feedback automatically." + echo + echo "The feedback was evaluated, but AutoFix failed before producing a verified commit. A human should take over this PR." + echo + echo "Run: ${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}/actions/runs/${GITHUB_RUN_ID}" + echo + echo "**Failure:**" + head -c 1500 "${WORKDIR}/failure.md" | sed 's///g' + echo + echo + echo "" + } > "${WORKDIR}/report.md" + gh pr comment "${PR}" --repo "${REPO}" --body-file "${WORKDIR}/report.md" || echo "::warning::Failed to post handoff comment on PR #${PR}" + fi diff --git a/.qwen/skills/autofix/scripts/run-agent.mjs b/.qwen/skills/autofix/scripts/run-agent.mjs index 5ea66360af..066445f111 100755 --- a/.qwen/skills/autofix/scripts/run-agent.mjs +++ b/.qwen/skills/autofix/scripts/run-agent.mjs @@ -1,7 +1,8 @@ #!/usr/bin/env node -import { spawnSync } from 'node:child_process'; +import { spawn } from 'node:child_process'; import { + createWriteStream, existsSync, mkdirSync, readFileSync, @@ -17,7 +18,7 @@ const skillPath = resolve( '..', 'SKILL.md', ); -const QWEN_TIMEOUT_MS = 50 * 60 * 1000; +const QWEN_TIMEOUT_MS = Number(process.env.QWEN_TIMEOUT_MS) || 50 * 60 * 1000; const specs = { 'assess-candidates': { inputs: ['candidates.json'], @@ -66,6 +67,87 @@ function writeFailure(workdir, message) { ); } +function writeHandoff(workdir, message) { + mkdirSync(workdir, { recursive: true }); + writeFileSync(file(workdir, 'handoff.md'), `${message}\n`); +} + +function isLoopGuardOutput(output) { + return ( + output.includes('turn_tool_call_cap') || + output.includes('Loop detection halted the run') + ); +} + +function killQwen(child, signal) { + try { + process.kill(-child.pid, signal); + } catch { + child.kill(signal); + } +} + +function runQwen(options, prompt) { + mkdirSync(options.workdir, { recursive: true }); + const log = createWriteStream(file(options.workdir, 'agent.log'), { + flags: 'w', + }); + log.on('error', () => {}); + let outputTail = ''; + let loopDetected = false; + let settled = false; + let timedOut = false; + let timer; + let killTimer; + + return new Promise((resolve) => { + const child = spawn(options.qwenBin, ['--yolo', '--prompt', prompt], { + stdio: ['inherit', 'pipe', 'pipe'], + detached: true, + }); + + const finish = (result) => { + if (settled) return; + settled = true; + clearTimeout(timer); + clearTimeout(killTimer); + const payload = { + ...result, + timedOut, + loopDetected: loopDetected || isLoopGuardOutput(outputTail), + }; + if (log.destroyed) { + resolve(payload); + } else { + log.end(() => resolve(payload)); + } + }; + + const record = (chunk, stream) => { + const text = chunk.toString('utf8'); + outputTail = (outputTail + text).slice(-20_000); + if (!loopDetected && isLoopGuardOutput(outputTail)) loopDetected = true; + log.write(chunk); + stream.write(chunk); + }; + + child.stdout.on('data', (chunk) => record(chunk, process.stdout)); + child.stderr.on('data', (chunk) => record(chunk, process.stderr)); + child.on('error', (error) => finish({ error, status: null, signal: null })); + child.on('close', (status, signal) => + finish({ error: null, status, signal }), + ); + + timer = setTimeout(() => { + timedOut = true; + killQwen(child, 'SIGTERM'); + killTimer = setTimeout(() => { + if (!settled) killQwen(child, 'SIGKILL'); + }, 10_000); + }, QWEN_TIMEOUT_MS); + }); +} + function promptFor(options, spec) { const skill = readFileSync(skillPath, 'utf8') .replace(/\r\n/g, '\n') @@ -123,22 +205,36 @@ if (missingInputs.length > 0) { ); } -const result = spawnSync(options.qwenBin, ['--yolo', '--prompt', prompt], { - stdio: 'inherit', - timeout: QWEN_TIMEOUT_MS, -}); +const result = await runQwen(options, prompt); if (result.error || result.signal || result.status !== 0) { const detail = result.error ? result.error.message - : result.signal - ? `signal ${result.signal}` - : `status ${String(result.status)}`; + : result.timedOut + ? `timeout (${QWEN_TIMEOUT_MS}ms)` + : result.signal + ? `signal ${result.signal}` + : `status ${String(result.status)}`; if (!existsSync(file(options.workdir, 'failure.md'))) { - writeFailure( - options.workdir, - `Qwen failed during ${options.mode}: ${detail}.`, - ); + if (result.loopDetected) { + writeFailure( + options.workdir, + `Qwen hit the tool-call loop guard during ${options.mode}. A human should take over this feedback batch.`, + ); + writeHandoff( + options.workdir, + 'Qwen hit the tool-call loop guard; a human should take over this feedback batch.', + ); + } else { + writeFailure( + options.workdir, + `Qwen failed during ${options.mode}: ${detail}.`, + ); + } } else { + writeHandoff( + options.workdir, + 'The agent wrote failure.md before qwen exited; a human should take over this feedback batch.', + ); console.error( `Qwen failed during ${options.mode}: ${detail}; preserving agent-written failure.md.`, ); @@ -148,6 +244,10 @@ if (result.error || result.signal || result.status !== 0) { if (existsSync(file(options.workdir, 'failure.md'))) { const content = readFileSync(file(options.workdir, 'failure.md'), 'utf8'); + writeHandoff( + options.workdir, + 'The agent wrote failure.md; a human should take over this feedback batch.', + ); console.error(`Autofix agent wrote failure.md:\n${content}`); process.exit(0); } diff --git a/scripts/tests/qwen-autofix-workflow.test.js b/scripts/tests/qwen-autofix-workflow.test.js index efdec99a33..907633d769 100644 --- a/scripts/tests/qwen-autofix-workflow.test.js +++ b/scripts/tests/qwen-autofix-workflow.test.js @@ -7,6 +7,7 @@ import { execFileSync, spawnSync } from 'node:child_process'; import { chmodSync, + existsSync, mkdtempSync, readFileSync, rmSync, @@ -895,6 +896,109 @@ describe('qwen-autofix workflow', () => { ); }); + it('posts a human-handoff marker when review addressing reaches a terminal handoff', () => { + expect(reviewAddressReportStep).toContain( + "GITHUB_TOKEN: '${{ secrets.CI_DEV_BOT_PAT }}'", + ); + expect(reviewAddressReportStep).toContain( + "NEWEST: '${{ steps.prepare.outputs.newest }}'", + ); + expect(reviewAddressReportStep).toContain('"${DRY_RUN}" != "true"'); + expect(reviewAddressReportStep).toContain('-s "${WORKDIR}/handoff.md"'); + expect(reviewAddressReportStep).toContain( + '', + ); + expect(reviewAddressReportStep).toContain( + 'Could not address the latest review feedback automatically', + ); + expect(reviewAddressReportStep).toContain('gh pr comment "${PR}"'); + expect(reviewAddressReportStep).toContain( + 'GH_TOKEN="${GITHUB_TOKEN}" gh api user --jq \'.login\'', + ); + expect(reviewAddressReportStep).toContain( + 'CI_DEV_BOT_PAT authenticates as ${bot_actor}', + ); + expect(reviewAddressReportStep).toContain( + '::warning::Failed to post handoff comment on PR #${PR}', + ); + expect(reviewAddressReportStep).toContain('human should take over'); + expect(reviewAddressReportStep).toContain("sed 's///g'"); + }); + + it('writes agent output to a log and marks loop guard failures for handoff', () => { + withRunnerDir((dir) => { + writeFileSync(join(dir, 'feedback.md'), 'feedback\n'); + const stub = writeQwenStub(dir, [ + "process.stderr.write('turn_tool_call_cap: too many tool calls\\n');", + 'process.exit(1);', + ]); + + const result = runAddressReview(dir, stub); + + expect(result.status).not.toBe(0); + expect(readFileSync(join(dir, 'agent.log'), 'utf8')).toContain( + 'turn_tool_call_cap', + ); + expect(readFileSync(join(dir, 'failure.md'), 'utf8')).toContain( + 'Qwen hit the tool-call loop guard', + ); + expect(readFileSync(join(dir, 'handoff.md'), 'utf8')).toContain( + 'human should take over', + ); + }); + }); + + it('handles agent log stream errors without crashing immediately', () => { + expect(readFileSync(autofixRunnerScriptPath, 'utf8')).toContain( + "log.on('error', () => {});", + ); + expect(readFileSync(autofixRunnerScriptPath, 'utf8')).toContain( + 'if (log.destroyed)', + ); + }); + + it('detects loop guard output before it falls out of the log tail', () => { + withRunnerDir((dir) => { + writeFileSync(join(dir, 'feedback.md'), 'feedback\n'); + const stub = writeQwenStub(dir, [ + "process.stderr.write('Loop detection halted the run\\n');", + "process.stdout.write('x'.repeat(21_000));", + 'process.exit(1);', + ]); + + const result = runAddressReview(dir, stub); + + expect(result.status).not.toBe(0); + expect(readFileSync(join(dir, 'failure.md'), 'utf8')).toContain( + 'Qwen hit the tool-call loop guard', + ); + expect(readFileSync(join(dir, 'handoff.md'), 'utf8')).toContain( + 'human should take over', + ); + }); + }); + + it('does not mark generic qwen subprocess failures for handoff', () => { + withRunnerDir((dir) => { + writeFileSync(join(dir, 'feedback.md'), 'feedback\n'); + const stub = writeQwenStub(dir, [ + "process.stderr.write('temporary upstream error\\n');", + 'process.exit(1);', + ]); + + const result = runAddressReview(dir, stub); + + expect(result.status).not.toBe(0); + expect(readFileSync(join(dir, 'agent.log'), 'utf8')).toContain( + 'temporary upstream error', + ); + expect(readFileSync(join(dir, 'failure.md'), 'utf8')).toContain( + 'Qwen failed during address-review', + ); + expect(existsSync(join(dir, 'handoff.md'))).toBe(false); + }); + }); + it('preserves agent-written failure details when the qwen subprocess fails', () => { withRunnerDir((dir) => { writeFileSync(join(dir, 'candidates.json'), '[]\n'); @@ -909,14 +1013,60 @@ describe('qwen-autofix workflow', () => { expect(readFileSync(join(dir, 'failure.md'), 'utf8')).toContain( 'agent detail', ); + expect(readFileSync(join(dir, 'handoff.md'), 'utf8')).toContain( + 'human should take over', + ); }); }); it('bounds qwen subprocess runtime', () => { const runner = readFileSync(autofixRunnerScriptPath, 'utf8'); - expect(runner).toContain('const QWEN_TIMEOUT_MS = 50 * 60 * 1000'); - expect(runner).toContain('timeout: QWEN_TIMEOUT_MS'); + expect(runner).toContain('50 * 60 * 1000'); + expect(runner).toContain('setTimeout(() =>'); + expect(runner).toContain("killQwen(child, 'SIGKILL')"); + expect(runner).toContain('}, QWEN_TIMEOUT_MS)'); + }); + + it('kills qwen subprocess descendants on timeout', () => { + withRunnerDir((dir) => { + writeFileSync(join(dir, 'feedback.md'), 'feedback\n'); + const stub = writeQwenStub(dir, [ + "import { spawn } from 'node:child_process';", + "spawn(process.execPath, ['-e', 'setTimeout(() => {}, 3000)'], {", + " stdio: ['ignore', 'inherit', 'inherit'],", + '});', + 'setTimeout(() => process.exit(0), 3000);', + ]); + + const result = spawnSync( + process.execPath, + [ + autofixRunnerScriptPath, + '--mode', + 'address-review', + '--pr', + '5678', + '--issue', + '1234', + '--workdir', + dir, + '--qwen-bin', + stub, + ], + { + encoding: 'utf8', + env: { ...process.env, QWEN_TIMEOUT_MS: '100' }, + timeout: 2000, + }, + ); + + expect(result.error).toBeUndefined(); + expect(result.status).not.toBe(0); + expect(readFileSync(join(dir, 'failure.md'), 'utf8')).toContain( + 'timeout (100ms)', + ); + }); }); it('reports external qwen subprocess signals without calling them timeouts', () => { @@ -976,6 +1126,9 @@ describe('qwen-autofix workflow', () => { expect(readFileSync(join(dir, 'failure.md'), 'utf8')).toContain( 'cannot proceed', ); + expect(readFileSync(join(dir, 'handoff.md'), 'utf8')).toContain( + 'human should take over', + ); }); }); From d1d53d71228c7950fd2b49c18d9a094662eb3943 Mon Sep 17 00:00:00 2001 From: han <2992336417@qq.com> Date: Tue, 7 Jul 2026 21:13:28 +0800 Subject: [PATCH 07/11] fix(monitor): preserve inherited git pager (#6429) --- packages/core/src/tools/monitor.test.ts | 49 ++++++++++++++++++- packages/core/src/tools/monitor.ts | 2 +- .../core/src/utils/shell-pager-env.test.ts | 22 +++++++++ 3 files changed, 70 insertions(+), 3 deletions(-) diff --git a/packages/core/src/tools/monitor.test.ts b/packages/core/src/tools/monitor.test.ts index 177d5f3719..48839ae826 100644 --- a/packages/core/src/tools/monitor.test.ts +++ b/packages/core/src/tools/monitor.test.ts @@ -191,8 +191,15 @@ describe('MonitorTool', () => { let monitorRegistry: MonitorRegistry; let mockChild: ReturnType; let mockIsPathWithinWorkspace: ReturnType; + let originalPager: string | undefined; + let originalGitPager: string | undefined; beforeEach(() => { + originalPager = process.env['PAGER']; + originalGitPager = process.env['GIT_PAGER']; + delete process.env['PAGER']; + delete process.env['GIT_PAGER']; + vi.clearAllMocks(); mockOsPlatform.mockReturnValue('linux'); @@ -229,6 +236,18 @@ describe('MonitorTool', () => { afterEach(() => { monitorRegistry.abortAll(); + + if (originalPager === undefined) { + delete process.env['PAGER']; + } else { + process.env['PAGER'] = originalPager; + } + + if (originalGitPager === undefined) { + delete process.env['GIT_PAGER']; + } else { + process.env['GIT_PAGER'] = originalGitPager; + } }); // Helper to access protected validateToolParamValues @@ -682,7 +701,33 @@ describe('MonitorTool', () => { const spawnOptions = mockSpawn.mock.calls[0][2]; expect(spawnOptions.env['PAGER']).toBe('cat'); - expect(spawnOptions.env['GIT_PAGER']).toBe('cat'); + expect(spawnOptions.env['GIT_PAGER']).toBeUndefined(); + }); + + it('preserves inherited git pager values for spawned processes', async () => { + process.env['GIT_PAGER'] = 'delta'; + const invocation = createInvocation({ + command: 'git log --oneline', + }); + + await invocation.execute(new AbortController().signal); + + const spawnOptions = mockSpawn.mock.calls[0][2]; + expect(spawnOptions.env['PAGER']).toBe('cat'); + expect(spawnOptions.env['GIT_PAGER']).toBe('delta'); + }); + + it('does not inject Unix pager defaults into Windows monitor env when unset', async () => { + mockOsPlatform.mockReturnValue('win32'); + const invocation = createInvocation({ + command: 'tail -f /var/log/app.log', + }); + + await invocation.execute(new AbortController().signal); + + const spawnOptions = mockSpawn.mock.calls[0][2]; + expect(spawnOptions.env['PAGER']).toBe(''); + expect(spawnOptions.env['GIT_PAGER']).toBeUndefined(); }); it('propagates explicit pager configuration to spawned processes', async () => { @@ -697,7 +742,7 @@ describe('MonitorTool', () => { const spawnOptions = mockSpawn.mock.calls[0][2]; expect(spawnOptions.env['PAGER']).toBe('more'); - expect(spawnOptions.env['GIT_PAGER']).toBe('more'); + expect(spawnOptions.env['GIT_PAGER']).toBeUndefined(); }); it('does not spawn when the turn signal is already aborted', async () => { diff --git a/packages/core/src/tools/monitor.ts b/packages/core/src/tools/monitor.ts index 0e5c1a4b5c..181ac8d942 100644 --- a/packages/core/src/tools/monitor.ts +++ b/packages/core/src/tools/monitor.ts @@ -369,7 +369,7 @@ class MonitorToolInvocation extends BaseToolInvocation< QWEN_CODE: '1', TERM: 'dumb', // no color codes for streaming ...getShellPagerEnv(this.config.getShellExecutionConfig().pager, { - includeGitPager: true, + includeGitPager: false, platform: os.platform(), }), ...getShellContextEnvVars(), diff --git a/packages/core/src/utils/shell-pager-env.test.ts b/packages/core/src/utils/shell-pager-env.test.ts index ef10aa8475..365b9e05b2 100644 --- a/packages/core/src/utils/shell-pager-env.test.ts +++ b/packages/core/src/utils/shell-pager-env.test.ts @@ -44,6 +44,28 @@ describe('shellPagerEnv', () => { }); }); + it('omits GIT_PAGER when git pager output is not requested', () => { + expect( + getShellPagerEnv('less', { + includeGitPager: false, + platform: 'linux', + }), + ).toEqual({ + PAGER: 'less', + }); + }); + + it('uses the default pager without GIT_PAGER when git pager output is not requested', () => { + expect( + getShellPagerEnv(undefined, { + includeGitPager: false, + platform: 'linux', + }), + ).toEqual({ + PAGER: 'cat', + }); + }); + it('treats an empty pager as an explicit request to disable pager env values', () => { expect( getShellPagerEnv('', { includeGitPager: true, platform: 'linux' }), From 40340ef50533068ae02ed9e4bd3608efa6e915db Mon Sep 17 00:00:00 2001 From: ytahdn <1294726970@qq.com> Date: Tue, 7 Jul 2026 21:38:56 +0800 Subject: [PATCH 08/11] fix(serve): classify interrupted model stream errors (#6422) * fix(serve): classify interrupted model streams * fix(serve): address interrupted stream review * test(webui): cover legacy terminated turn error fallback * fix(web-shell): preserve error message data shape * test(daemon): cover turn error fallback boundaries * fix(web-shell): preserve classified error data --------- Co-authored-by: ytahdn --- packages/acp-bridge/src/bridge.test.ts | 60 ++++++++- packages/acp-bridge/src/bridge.ts | 19 +++ packages/sdk-typescript/src/daemon/events.ts | 1 + packages/sdk-typescript/src/daemon/types.ts | 2 + .../src/daemon/ui/normalizer.ts | 2 + .../src/daemon/ui/transcript.ts | 3 + .../sdk-typescript/src/daemon/ui/types.ts | 1 + .../sdk-typescript/test/unit/daemonUi.test.ts | 97 ++++++++++++++ packages/web-shell/client/App.test.tsx | 43 ++++++ .../adapters/transcriptToMessages.test.ts | 125 ++++++++++++++++++ .../client/adapters/transcriptToMessages.ts | 44 ++++-- .../web-shell/client/hooks/useMessages.ts | 1 + packages/web-shell/client/i18n.tsx | 3 + .../src/daemon/transcriptAdapter.test.ts | 83 ++++++++++++ .../webui/src/daemon/transcriptAdapter.ts | 25 +++- 15 files changed, 495 insertions(+), 14 deletions(-) diff --git a/packages/acp-bridge/src/bridge.test.ts b/packages/acp-bridge/src/bridge.test.ts index ee46a1082a..027f5859e3 100644 --- a/packages/acp-bridge/src/bridge.test.ts +++ b/packages/acp-bridge/src/bridge.test.ts @@ -40,7 +40,11 @@ import { WorkspaceMismatchError, } from './bridgeErrors.js'; import { MAX_WORKSPACE_PATH_LENGTH } from './workspacePaths.js'; -import { extractErrorMessage, extractErrorCode } from './bridge.js'; +import { + classifyTurnErrorKind, + extractErrorMessage, + extractErrorCode, +} from './bridge.js'; import { SERVE_STATUS_EXT_METHODS } from './status.js'; import type { ChannelFactory } from './channel.js'; import type { BridgeTelemetry } from './bridgeOptions.js'; @@ -3376,6 +3380,42 @@ describe('createAcpSessionBridge', () => { await bridge.shutdown(); }); + it('adds structured errorKind when a turn ends with terminated', async () => { + const handle = makeChannel({ + promptImpl: () => { + throw new RequestError(-32603, 'terminated'); + }, + }); + const bridge = makeBridge({ channelFactory: async () => handle.channel }); + const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + const abort = new AbortController(); + const iter = bridge.subscribeEvents(session.sessionId, { + signal: abort.signal, + }); + const turnError = (async () => { + for await (const event of iter) { + if (event.type === 'turn_error') return event; + } + throw new Error('turn_error was not published'); + })(); + + await expect( + bridge.sendPrompt(session.sessionId, { + sessionId: session.sessionId, + prompt: [{ type: 'text', text: 'stream may fail' }], + }), + ).rejects.toThrow('terminated'); + + const event = await turnError; + expect(event.data).toMatchObject({ + message: 'terminated', + errorKind: 'model_stream_interrupted', + }); + + abort.abort(); + await bridge.shutdown(); + }); + it('echoes user_message_chunk to ALL session subscribers (cross-client sync)', async () => { // Cross-client sync fix: a prompt sent by client A must be visible // to every SSE subscriber of the same session — not just the @@ -10287,6 +10327,24 @@ describe('extractErrorCode', () => { }); }); +describe('classifyTurnErrorKind', () => { + it('classifies bare terminated errors as model stream interruptions', () => { + expect(classifyTurnErrorKind('terminated')).toBe( + 'model_stream_interrupted', + ); + expect(classifyTurnErrorKind('Terminated')).toBe( + 'model_stream_interrupted', + ); + expect(classifyTurnErrorKind(' terminated ')).toBe( + 'model_stream_interrupted', + ); + }); + + it('leaves unrelated turn errors unclassified', () => { + expect(classifyTurnErrorKind('API rate limit exceeded')).toBeUndefined(); + }); +}); + // --------------------------------------------------------------------------- // §2.3 side-channel state layer: publish helpers + reconciliation + snapshot // --------------------------------------------------------------------------- diff --git a/packages/acp-bridge/src/bridge.ts b/packages/acp-bridge/src/bridge.ts index 8fcfa8366c..e144e794a0 100644 --- a/packages/acp-bridge/src/bridge.ts +++ b/packages/acp-bridge/src/bridge.ts @@ -912,6 +912,14 @@ export function extractErrorCode(err: unknown): string | undefined { return undefined; } +export function classifyTurnErrorKind( + message: string, +): 'model_stream_interrupted' | undefined { + return message.trim().toLowerCase() === 'terminated' + ? 'model_stream_interrupted' + : undefined; +} + function broadcastTurnError( entry: SessionEntry, sessionId: string, @@ -921,6 +929,16 @@ function broadcastTurnError( ): void { const message = extractErrorMessage(err); const code = extractErrorCode(err); + const errorKind = classifyTurnErrorKind(message); + if (errorKind) { + writeServeDebugLine( + `turn_error classified session=${JSON.stringify(sessionId)} ` + + `message=${JSON.stringify(message)} ` + + `errorKind=${JSON.stringify(errorKind)}` + + (code ? ` code=${JSON.stringify(code)}` : '') + + (promptId ? ` promptId=${JSON.stringify(promptId)}` : ''), + ); + } entry.retryAllowed = true; try { entry.events.publish({ @@ -929,6 +947,7 @@ function broadcastTurnError( sessionId, message, ...(code ? { code } : {}), + ...(errorKind ? { errorKind } : {}), ...(promptId ? { promptId } : {}), }, ...(originatorClientId ? { originatorClientId } : {}), diff --git a/packages/sdk-typescript/src/daemon/events.ts b/packages/sdk-typescript/src/daemon/events.ts index 232a0d54a5..250ba0a48e 100644 --- a/packages/sdk-typescript/src/daemon/events.ts +++ b/packages/sdk-typescript/src/daemon/events.ts @@ -757,6 +757,7 @@ export interface DaemonTurnErrorData { sessionId: string; message: string; code?: string; + errorKind?: DaemonErrorKind | (string & {}); promptId?: string; [key: string]: unknown; } diff --git a/packages/sdk-typescript/src/daemon/types.ts b/packages/sdk-typescript/src/daemon/types.ts index fa7838f37d..fb4255f651 100644 --- a/packages/sdk-typescript/src/daemon/types.ts +++ b/packages/sdk-typescript/src/daemon/types.ts @@ -730,6 +730,8 @@ export const DAEMON_ERROR_KINDS = [ // An SSE writer's last successful flush was older than the daemon's // writer-idle deadline. 'writer_idle_timeout', + // The model response stream ended before a complete turn could be read. + 'model_stream_interrupted', ] as const; export type DaemonErrorKind = (typeof DAEMON_ERROR_KINDS)[number]; diff --git a/packages/sdk-typescript/src/daemon/ui/normalizer.ts b/packages/sdk-typescript/src/daemon/ui/normalizer.ts index d27d2b6c2c..37d697ea60 100644 --- a/packages/sdk-typescript/src/daemon/ui/normalizer.ts +++ b/packages/sdk-typescript/src/daemon/ui/normalizer.ts @@ -159,6 +159,7 @@ export function normalizeDaemonEvent( } case 'turn_error': { const code = getString(event.data, 'code'); + const errorKind = asDaemonErrorKind(getString(event.data, 'errorKind')); const promptId = getString(event.data, 'promptId'); return [ { @@ -167,6 +168,7 @@ export function normalizeDaemonEvent( source: 'turn_error', recoverable: true, ...(code ? { code } : {}), + ...(errorKind ? { errorKind } : {}), ...(promptId ? { promptId } : {}), text: getString(event.data, 'message') ?? diff --git a/packages/sdk-typescript/src/daemon/ui/transcript.ts b/packages/sdk-typescript/src/daemon/ui/transcript.ts index 2a619c1ceb..bdc8ac1a63 100644 --- a/packages/sdk-typescript/src/daemon/ui/transcript.ts +++ b/packages/sdk-typescript/src/daemon/ui/transcript.ts @@ -1016,6 +1016,9 @@ function appendStatusBlock( ...(event?.type === 'error' && event.promptId ? { promptId: event.promptId } : {}), + ...(event?.type === 'error' && event.errorKind + ? { errorKind: event.errorKind } + : {}), ...(event?.type === 'error' && event.source ? { source: event.source } : {}), diff --git a/packages/sdk-typescript/src/daemon/ui/types.ts b/packages/sdk-typescript/src/daemon/ui/types.ts index ef3b64bffb..0b7be64816 100644 --- a/packages/sdk-typescript/src/daemon/ui/types.ts +++ b/packages/sdk-typescript/src/daemon/ui/types.ts @@ -852,6 +852,7 @@ export interface DaemonStatusTranscriptBlock extends DaemonTranscriptBlockBase { text: string; code?: string; promptId?: string; + errorKind?: DaemonErrorKind; source?: string; data?: unknown; } diff --git a/packages/sdk-typescript/test/unit/daemonUi.test.ts b/packages/sdk-typescript/test/unit/daemonUi.test.ts index 10c90b9fa6..b45cca008c 100644 --- a/packages/sdk-typescript/test/unit/daemonUi.test.ts +++ b/packages/sdk-typescript/test/unit/daemonUi.test.ts @@ -1196,6 +1196,103 @@ describe('daemon UI normalizer and transcript reducer', () => { ); }); + it('preserves structured model stream interruption turn errors', () => { + expect( + normalizeDaemonEvent({ + id: 44, + v: 1, + type: 'turn_error', + data: { + sessionId: 'session-1', + message: 'terminated', + code: '-32603', + errorKind: 'model_stream_interrupted', + promptId: 'prompt-1', + }, + }), + ).toMatchObject([ + { + type: 'error', + source: 'turn_error', + recoverable: true, + code: '-32603', + errorKind: 'model_stream_interrupted', + promptId: 'prompt-1', + text: 'terminated', + }, + ]); + }); + + it('keeps structured model stream interruption on transcript blocks', () => { + const state = reduceDaemonTranscriptEvents( + createDaemonTranscriptState({ now: 100 }), + normalizeDaemonEvent({ + id: 46, + v: 1, + type: 'turn_error', + data: { + sessionId: 'session-1', + message: 'terminated', + code: '-32603', + errorKind: 'model_stream_interrupted', + promptId: 'prompt-1', + }, + }), + { now: 101 }, + ); + + expect(state.blocks).toMatchObject([ + { + kind: 'error', + source: 'turn_error', + text: 'terminated', + code: '-32603', + errorKind: 'model_stream_interrupted', + promptId: 'prompt-1', + }, + ]); + }); + + it('keeps older daemon terminated turn error text unchanged', () => { + expect( + normalizeDaemonEvent({ + id: 45, + v: 1, + type: 'turn_error', + data: { + sessionId: 'session-1', + message: 'terminated', + }, + }), + ).toMatchObject([ + { + type: 'error', + source: 'turn_error', + text: 'terminated', + }, + ]); + }); + + it('drops unrecognized errorKind from turn error events', () => { + const [event] = normalizeDaemonEvent({ + id: 47, + v: 1, + type: 'turn_error', + data: { + sessionId: 'session-1', + message: 'some error', + errorKind: 'some_future_kind', + }, + }); + + expect(event).toMatchObject({ + type: 'error', + source: 'turn_error', + text: 'some error', + }); + expect(event).not.toHaveProperty('errorKind'); + }); + it('normalizes daemon lifecycle and control events', () => { expect( normalizeDaemonEvent({ diff --git a/packages/web-shell/client/App.test.tsx b/packages/web-shell/client/App.test.tsx index 08e9cd8014..5bd3197ee2 100644 --- a/packages/web-shell/client/App.test.tsx +++ b/packages/web-shell/client/App.test.tsx @@ -808,6 +808,49 @@ describe('App session callbacks', () => { ); }); + it('allows manual retry after a model stream interrupted turn error', async () => { + const { container, rerender } = renderApp(); + await flush(); + + testState.prompt = 'recover this stream'; + await clickSubmit(container); + expect(mockSessionActions.sendPrompt).toHaveBeenCalledWith( + 'recover this stream', + expect.objectContaining({ retry: undefined }), + ); + + mockSessionActions.sendPrompt.mockClear(); + act(() => { + testState.blocks = [ + { + kind: 'error', + source: 'turn_error', + id: 'turn-error-stream-interrupted', + errorKind: 'model_stream_interrupted', + text: 'terminated', + }, + ]; + rerender(); + }); + + expect(container.querySelector('[data-testid="retry"]')).not.toBeNull(); + + await act(async () => { + container + .querySelector('[data-testid="retry"]') + ?.click(); + await Promise.resolve(); + }); + + expect(mockSessionActions.sendPrompt).toHaveBeenCalledWith( + 'recover this stream', + expect.objectContaining({ + optimisticUserMessage: false, + retry: true, + }), + ); + }); + it('gates queued submissions and only enqueues after approval', async () => { let approve: (() => void) | undefined; const onSubmitBefore = vi.fn( diff --git a/packages/web-shell/client/adapters/transcriptToMessages.test.ts b/packages/web-shell/client/adapters/transcriptToMessages.test.ts index bf935630ef..cf8cdcaf10 100644 --- a/packages/web-shell/client/adapters/transcriptToMessages.test.ts +++ b/packages/web-shell/client/adapters/transcriptToMessages.test.ts @@ -2179,6 +2179,131 @@ describe('transcriptBlocksToDaemonMessages', () => { ]); }); + it('renders model stream interruption errors from structured errorKind labels', () => { + const messages = transcriptBlocksToDaemonMessages( + [ + { + id: 'err-1', + kind: 'error' as const, + source: 'turn_error' as const, + errorKind: 'model_stream_interrupted' as const, + text: 'terminated', + data: { diagnosticId: 'abc' }, + clientReceivedAt: 1, + createdAt: 1, + updatedAt: 1, + }, + ], + { + labels: { + modelStreamInterrupted: 'Localized stream interruption.', + }, + }, + ); + + expect(messages).toEqual([ + { + id: 'err-1', + role: 'system', + content: 'Localized stream interruption.', + variant: 'error', + retryable: true, + source: 'turn_error', + data: { + diagnosticId: 'abc', + errorKind: 'model_stream_interrupted', + }, + timestamp: 1, + }, + ]); + }); + + it('upgrades older daemon terminated turn errors to localized text', () => { + const messages = transcriptBlocksToDaemonMessages( + [ + { + id: 'err-1', + kind: 'error' as const, + source: 'turn_error' as const, + text: 'terminated', + clientReceivedAt: 1, + createdAt: 1, + updatedAt: 1, + }, + ], + { + labels: { + modelStreamInterrupted: 'Localized stream interruption.', + }, + }, + ); + + expect(messages[0]).toMatchObject({ + content: 'Localized stream interruption.', + retryable: true, + source: 'turn_error', + }); + }); + + it('does not add data solely for structured errorKind labels', () => { + const messages = transcriptBlocksToDaemonMessages( + [ + { + id: 'err-1', + kind: 'error' as const, + source: 'turn_error' as const, + errorKind: 'model_stream_interrupted' as const, + text: 'terminated', + clientReceivedAt: 1, + createdAt: 1, + updatedAt: 1, + }, + ], + { + labels: { + modelStreamInterrupted: 'Localized stream interruption.', + }, + }, + ); + + expect(messages[0]).toMatchObject({ + content: 'Localized stream interruption.', + retryable: true, + source: 'turn_error', + }); + expect(messages[0]).not.toHaveProperty('data'); + }); + + it('preserves non-object error data when adding structured errorKind', () => { + const messages = transcriptBlocksToDaemonMessages( + [ + { + id: 'err-1', + kind: 'error' as const, + source: 'turn_error' as const, + errorKind: 'model_stream_interrupted' as const, + text: 'terminated', + data: 'diagnostic text', + clientReceivedAt: 1, + createdAt: 1, + updatedAt: 1, + }, + ], + { + labels: { + modelStreamInterrupted: 'Localized stream interruption.', + }, + }, + ); + + expect(messages[0]).toMatchObject({ + data: { + value: 'diagnostic text', + errorKind: 'model_stream_interrupted', + }, + }); + }); + it('converts debug blocks to system messages with info variant', () => { const messages = transcriptBlocksToDaemonMessages([ { diff --git a/packages/web-shell/client/adapters/transcriptToMessages.ts b/packages/web-shell/client/adapters/transcriptToMessages.ts index 3b09fedfca..f847bbc000 100644 --- a/packages/web-shell/client/adapters/transcriptToMessages.ts +++ b/packages/web-shell/client/adapters/transcriptToMessages.ts @@ -33,11 +33,6 @@ type DaemonPermissionTranscriptBlock = Extract< { kind: 'permission' } >; -type ExtendedDaemonStatusTranscriptBlock = DaemonStatusTranscriptBlock & { - source?: string; - data?: unknown; -}; - type ExtendedDaemonTextTranscriptBlock = DaemonTextTranscriptBlock & { meta?: { source?: unknown; @@ -50,6 +45,7 @@ interface TranscriptMessageLabels { promptCancelled?: string; branchSuccess?: (name: string) => string; midTurnInserted?: (message: string) => string; + modelStreamInterrupted?: string; } interface TranscriptMessageOptions { @@ -63,6 +59,35 @@ function isIgnoredWebShellStatus(text: string): boolean { ); } +function getErrorDisplayText( + block: DaemonStatusTranscriptBlock, + labels?: TranscriptMessageLabels, +): string { + if ( + block.errorKind === 'model_stream_interrupted' || + // Older daemons emit this turn_error before they know about errorKind. + (block.source === 'turn_error' && + block.text.trim().toLowerCase() === 'terminated') + ) { + return labels?.modelStreamInterrupted ?? block.text; + } + return block.text; +} + +function getErrorMessageData( + data: unknown, + errorKind: DaemonStatusTranscriptBlock['errorKind'], +): { data?: unknown } { + if (data === undefined) return {}; + if (!errorKind) return { data }; + return { + data: { + ...(getRecord(data) ?? { value: data }), + errorKind, + }, + }; +} + function getSessionBranchDisplayName(data: unknown): string | null { if (!data || typeof data !== 'object') return null; const branchData = data as { @@ -530,7 +555,7 @@ export function transcriptBlocksToDaemonMessages( case 'status': case 'debug': { - const statusBlock = block as ExtendedDaemonStatusTranscriptBlock; + const statusBlock = block; const branchDisplayName = statusBlock.source === 'session_branched' ? getSessionBranchDisplayName(statusBlock.data) @@ -575,16 +600,17 @@ export function transcriptBlocksToDaemonMessages( } case 'error': { - const errorBlock = block as ExtendedDaemonStatusTranscriptBlock; + const errorBlock = block; + const errorKind = errorBlock.errorKind; messages.push({ id: block.id, role: 'system', - content: errorBlock.text, + content: getErrorDisplayText(errorBlock, options.labels), variant: 'error', retryable: errorBlock.source === 'turn_error', timestamp: blockTime, ...(errorBlock.source ? { source: errorBlock.source } : {}), - ...(errorBlock.data !== undefined ? { data: errorBlock.data } : {}), + ...getErrorMessageData(errorBlock.data, errorKind), }); needsNewContentMessage = true; break; diff --git a/packages/web-shell/client/hooks/useMessages.ts b/packages/web-shell/client/hooks/useMessages.ts index 9959ea3253..ddda0d1f14 100644 --- a/packages/web-shell/client/hooks/useMessages.ts +++ b/packages/web-shell/client/hooks/useMessages.ts @@ -14,6 +14,7 @@ export function useMessages( promptCancelled: t('request.cancelled'), branchSuccess: (name) => t('branch.success', { name }), midTurnInserted: (message) => t('midTurn.inserted', { message }), + modelStreamInterrupted: t('error.modelStreamInterrupted'), }, }), [blocks, t], diff --git a/packages/web-shell/client/i18n.tsx b/packages/web-shell/client/i18n.tsx index f231784c97..5c689abb2a 100644 --- a/packages/web-shell/client/i18n.tsx +++ b/packages/web-shell/client/i18n.tsx @@ -827,6 +827,8 @@ const EN: Messages = { 'bug.submitted': 'Bug report opened in a new tab.', 'clear.blocked': 'Cannot clear while streaming — cancel first (Esc).', 'error.unknown': 'Unknown error', + 'error.modelStreamInterrupted': + 'Model response stream was interrupted. Please retry.', 'shell.command': 'Shell Command', 'compact.enabled': 'Compact mode enabled', 'compact.disabled': 'Compact mode disabled', @@ -2439,6 +2441,7 @@ const ZH: Messages = { 'bug.submitted': 'Bug 报告已在新标签页中打开。', 'clear.blocked': '流式输出中无法清屏 — 先按 Esc 取消。', 'error.unknown': '未知错误', + 'error.modelStreamInterrupted': '模型响应流已中断，请重试。', 'shell.command': 'Shell 命令', 'compact.enabled': '紧凑模式已开启', 'compact.disabled': '紧凑模式已关闭', diff --git a/packages/webui/src/daemon/transcriptAdapter.test.ts b/packages/webui/src/daemon/transcriptAdapter.test.ts index cf0d0476ce..d87300331f 100644 --- a/packages/webui/src/daemon/transcriptAdapter.test.ts +++ b/packages/webui/src/daemon/transcriptAdapter.test.ts @@ -31,6 +31,89 @@ describe('daemonTranscriptToUnifiedMessages', () => { }); }); + it('renders model stream interruption turn errors with user-facing text', () => { + const [message] = daemonTranscriptToUnifiedMessages([ + { + id: 'error-1', + kind: 'error', + source: 'turn_error', + errorKind: 'model_stream_interrupted', + text: 'terminated', + clientReceivedAt: 1, + createdAt: 1, + updatedAt: 1, + }, + ]); + + expect(message).toMatchObject({ + type: 'tool_call', + toolCall: { + kind: 'system_error', + rawOutput: 'Model response stream was interrupted. Please retry.', + content: [ + { + content: { + text: 'Model response stream was interrupted. Please retry.', + error: 'Model response stream was interrupted. Please retry.', + }, + }, + ], + }, + }); + }); + + it('renders older daemon terminated turn errors with user-facing text', () => { + const [message] = daemonTranscriptToUnifiedMessages([ + { + id: 'error-legacy', + kind: 'error', + source: 'turn_error', + text: 'terminated', + clientReceivedAt: 1, + createdAt: 1, + updatedAt: 1, + }, + ]); + + expect(message).toMatchObject({ + type: 'tool_call', + toolCall: { + kind: 'system_error', + rawOutput: 'Model response stream was interrupted. Please retry.', + content: [ + { + content: { + text: 'Model response stream was interrupted. Please retry.', + error: 'Model response stream was interrupted. Please retry.', + }, + }, + ], + }, + }); + }); + + it('passes non-terminated turn error text through unchanged', () => { + const [message] = daemonTranscriptToUnifiedMessages([ + { + id: 'error-other', + kind: 'error', + source: 'turn_error', + text: 'context deadline exceeded', + clientReceivedAt: 1, + createdAt: 1, + updatedAt: 1, + }, + ]); + + expect(message).toMatchObject({ + type: 'tool_call', + toolCall: { + kind: 'system_error', + rawOutput: 'context deadline exceeded', + }, + }); + }); + it('maps daemon tool statuses without leaving terminal states spinning', () => { const messages = daemonTranscriptToUnifiedMessages([ createToolBlock('cancelled-tool', 'cancelled'), diff --git a/packages/webui/src/daemon/transcriptAdapter.ts b/packages/webui/src/daemon/transcriptAdapter.ts index 8492f23bbf..9fe3135840 100644 --- a/packages/webui/src/daemon/transcriptAdapter.ts +++ b/packages/webui/src/daemon/transcriptAdapter.ts @@ -10,6 +10,7 @@ import { isDaemonUiSensitiveKey, sanitizeDaemonTerminalText, type DaemonTranscriptBlock, + type DaemonStatusTranscriptBlock, type DaemonToolTranscriptBlock, } from '@qwen-code/sdk/daemon'; import type { UnifiedMessage } from '../adapters/types.js'; @@ -43,6 +44,9 @@ export interface DaemonTranscriptAdapterOptions { enrichToolDetailsWithPreview?: boolean; } +const MODEL_STREAM_INTERRUPTED_MESSAGE = + 'Model response stream was interrupted. Please retry.'; + export function daemonTranscriptToUnifiedMessages( blocks: readonly DaemonTranscriptBlock[], options: DaemonTranscriptAdapterOptions = {}, @@ -145,7 +149,8 @@ export function daemonTranscriptToUnifiedMessages( isLast, }, ]; - case 'error': + case 'error': { + const text = getErrorDisplayText(block); return [ { id: block.id, @@ -156,14 +161,14 @@ export function daemonTranscriptToUnifiedMessages( kind: 'system_error', title: 'System error', status: 'failed', - rawOutput: sanitizeDisplayText(block.text), + rawOutput: sanitizeDisplayText(text), content: [ { type: 'content', content: { type: 'error', - text: sanitizeDisplayText(block.text), - error: sanitizeDisplayText(block.text), + text: sanitizeDisplayText(text), + error: sanitizeDisplayText(text), }, }, ], @@ -172,6 +177,7 @@ export function daemonTranscriptToUnifiedMessages( isLast, }, ]; + } case 'status': return [ { @@ -196,6 +202,17 @@ export function daemonTranscriptToUnifiedMessages( }); } +function getErrorDisplayText(block: DaemonStatusTranscriptBlock): string { + if ( + block.errorKind === 'model_stream_interrupted' || + (block.source === 'turn_error' && + block.text.trim().toLowerCase() === 'terminated') + ) { + return MODEL_STREAM_INTERRUPTED_MESSAGE; + } + return block.text; +} + function daemonToolBlockToToolCallData( block: DaemonToolTranscriptBlock, enrichDetails: boolean = false, From 1d19fe7172b5566da256ffc5cd0ebe0a31375782 Mon Sep 17 00:00:00 2001 From: ytahdn <1294726970@qq.com> Date: Tue, 7 Jul 2026 21:53:17 +0800 Subject: [PATCH 09/11] fix(web-shell): refine tool call summaries (#6450) * fix(web-shell): refine tool call summaries * fix(web-shell): address tool summary review feedback --------- Co-authored-by: ytahdn --- .../components/messages/ToolGroup.test.tsx | 228 ++++++++++++++++-- .../client/components/messages/ToolGroup.tsx | 139 +++++++++-- .../messages/toolFormatting.test.ts | 47 ++++ .../components/messages/toolFormatting.ts | 48 +++- .../messages/tools/ToolChrome.module.css | 72 +++++- packages/web-shell/client/i18n.tsx | 4 +- 6 files changed, 478 insertions(+), 60 deletions(-) diff --git a/packages/web-shell/client/components/messages/ToolGroup.test.tsx b/packages/web-shell/client/components/messages/ToolGroup.test.tsx index 03073352d6..d5544fe881 100644 --- a/packages/web-shell/client/components/messages/ToolGroup.test.tsx +++ b/packages/web-shell/client/components/messages/ToolGroup.test.tsx @@ -4,6 +4,7 @@ import { act } from 'react'; import { createRoot, type Root } from 'react-dom/client'; import type { ACPToolCall } from '../../adapters/types'; import { I18nProvider } from '../../i18n'; +import { WebShellCustomizationProvider } from '../../customization'; vi.mock('../../App', async () => { const { createContext } = await import('react'); @@ -19,7 +20,6 @@ const { extractDiff, fencedCodeBlock, formatSingleToolSummary, - formatRunningSingleToolSummary, formatToolGroupSummary, getActiveTool, getRawFileDiff, @@ -56,14 +56,17 @@ function makeTool(overrides: Partial = {}): ACPToolCall { }; } -function renderToolLine(tool: ACPToolCall): HTMLElement { +function renderToolLine( + tool: ACPToolCall, + props: Partial[0]> = {}, +): HTMLElement { const container = document.createElement('div'); document.body.appendChild(container); const root = createRoot(container); act(() => { root.render( - + , ); }); @@ -71,14 +74,19 @@ function renderToolLine(tool: ACPToolCall): HTMLElement { return container; } -function renderToolGroup(tools: ACPToolCall[]): HTMLElement { +function renderToolGroup( + tools: ACPToolCall[], + customization = {}, +): HTMLElement { const container = document.createElement('div'); document.body.appendChild(container); const root = createRoot(container); act(() => { root.render( - + + + , ); }); @@ -185,18 +193,51 @@ describe('tool group summary logic', () => { ); }); - it('formats a single tool summary from the tool itself', () => { + it('formats a single shell summary as only the semantic description', () => { + expect( + formatSingleToolSummary( + makeTool({ + toolName: 'run_shell_command', + args: { + command: 'dataworks-infra workspace list', + description: '查询用户工作空间列表', + timeout: 30000, + }, + }), + t, + ), + ).toBe('查询用户工作空间列表'); + }); + + it('falls back to command text for shell summaries without descriptions', () => { expect( formatSingleToolSummary( makeTool({ toolName: 'Shell', - args: { command: 'npm run build' }, + args: { command: 'npm run build', timeout: 30000 }, }), t, ), ).toBe('Shell npm run build'); }); + it('uses only skill names in single tool summaries', () => { + expect( + formatSingleToolSummary( + makeTool({ + toolName: 'skill', + title: + 'Skill: Use skill: "qc-helper" with args: "weather in Hangzhou next 5 days"', + args: { + skill: 'qc-helper', + args: 'weather in Hangzhou next 5 days', + }, + }), + t, + ), + ).toBe('Skill qc-helper'); + }); + it('uses action summaries for single todo and ask-user tools', () => { expect( formatSingleToolSummary(makeTool({ toolName: 'todo_write' }), t), @@ -206,20 +247,6 @@ describe('tool group summary logic', () => { ).toBe('Asked user'); }); - it('keeps running state and command details for a single active tool', () => { - expect( - formatRunningSingleToolSummary( - makeTool({ - toolName: 'Shell', - status: 'in_progress', - args: { command: 'npm run build' }, - }), - t, - '0:03', - ), - ).toBe('Running Shell npm run build 0:03'); - }); - it('truncates long single tool descriptions in the chat summary', () => { const summary = formatSingleToolSummary( makeTool({ @@ -232,6 +259,61 @@ describe('tool group summary logic', () => { expect(summary.length).toBeLessThan(140); expect(summary).toContain('...'); }); + + it('lets custom tool header extras render single-tool chat summaries', () => { + const container = renderToolGroup( + [ + makeTool({ + toolName: 'run_shell_command', + args: { + command: 'dataworks-infra workspace list', + description: '查询用户工作空间列表', + timeout: 30000, + }, + }), + ], + { + renderToolHeaderExtra: (info) => ( + + {info.kind}:{info.description} + + ), + }, + ); + + const summary = container.querySelector('button'); + expect(summary?.textContent).not.toContain('Shell'); + expect(summary?.textContent).toContain('shell:查询用户工作空间列表'); + expect(summary?.textContent).not.toContain('timeout: 30000ms'); + }); + + it('uses action descriptions for shell rows inside grouped summaries', () => { + const container = renderToolGroup([ + makeTool({ + callId: 'shell', + toolName: 'run_shell_command', + title: + 'Shell: dataworks-infra workspace list [timeout: 30000ms] (查询用户工作空间列表)', + args: { + command: 'dataworks-infra workspace list', + description: '查询用户工作空间列表', + timeout: 30000, + }, + }), + makeTool({ + callId: 'read', + toolName: 'read_file', + args: { file_path: 'README.md' }, + }), + ]); + + expect(container.textContent).toContain('Shell'); + expect(container.textContent).toContain('查询用户工作空间列表'); + expect(container.textContent).not.toContain( + 'dataworks-infra workspace list', + ); + expect(container.textContent).not.toContain('timeout: 30000ms'); + }); }); describe('tool expandability', () => { @@ -253,6 +335,32 @@ describe('tool expandability', () => { ), ).toBe(false); }); + + it('does not expand skill rows that only have the skill name', () => { + expect( + hasExpandableContent( + makeTool({ + toolName: 'skill', + title: 'Skill: Use skill: "review"', + args: { skill: 'review' }, + }), + ), + ).toBe(false); + expect( + hasExpandableContent( + makeTool({ + toolName: 'skill', + args: { skill: 'review' }, + content: [ + { + type: 'content', + content: { type: 'text', text: '# Code Review' }, + }, + ], + }), + ), + ).toBe(true); + }); }); describe('tool kind logic', () => { @@ -363,6 +471,84 @@ describe('tool row rendering', () => { expect(header.textContent).toContain(pattern); expect(header.textContent).toContain('packages/web-shell/client'); }); + + it('uses the shell tool name for expanded cards from action summaries', () => { + const container = renderToolLine( + makeTool({ + toolName: 'run_shell_command', + args: { + command: 'dataworks-infra workspace list', + description: '查询用户工作空间列表', + timeout: 30000, + }, + content: [ + { + type: 'content', + content: { type: 'text', text: 'failed\nwith details' }, + }, + ], + }), + { summaryOnly: true }, + ); + const header = container.querySelector('[role="button"]') as HTMLElement; + + expect(header.textContent).toContain('Shell'); + expect(header.textContent).toContain('查询用户工作空间列表'); + + act(() => header.click()); + + const cardTitle = container.querySelector('[class*="expandedCardTitle"]'); + expect(cardTitle?.textContent).toBe('Shell'); + }); + + it('shows complete skill content in the expanded card body', () => { + const container = renderToolLine( + makeTool({ + toolName: 'skill', + title: 'Skill: Use skill: "review" with args: "check the current diff"', + args: { + skill: 'review', + }, + content: [ + { + type: 'content', + content: { + type: 'text', + text: 'Base directory for this skill: /repo\n# Code Review', + }, + }, + ], + }), + ); + const header = container.querySelector('[role="button"]') as HTMLElement; + + expect(header.textContent).toContain('Skill'); + expect(header.textContent).toContain('review'); + expect(header.textContent).not.toContain('check the current diff'); + + act(() => header.click()); + + const output = container.querySelector('pre'); + expect(output?.textContent).toBe( + 'Base directory for this skill: /repo\n# Code Review', + ); + }); + + it('keeps running state for single todo summaries', () => { + const container = renderToolGroup([ + makeTool({ + toolName: 'todo_write', + status: 'in_progress', + args: { + todos: [{ id: '1', content: 'Check UI', status: 'in_progress' }], + }, + }), + ]); + const summary = container.querySelector('button'); + + expect(summary?.textContent).toContain('Running'); + expect(summary?.textContent).toContain('Updated task list'); + }); }); describe('tool output logic', () => { diff --git a/packages/web-shell/client/components/messages/ToolGroup.tsx b/packages/web-shell/client/components/messages/ToolGroup.tsx index 53e902f1bc..01d8c7e8f2 100644 --- a/packages/web-shell/client/components/messages/ToolGroup.tsx +++ b/packages/web-shell/client/components/messages/ToolGroup.tsx @@ -42,9 +42,12 @@ import { getAgentDisplayStatus, getAgentType, getTaskExecutionRecord, + getShellToolSemanticDescription, getToolDescription, + getToolSummaryDescription, getToolResultSummary, isAskUserQuestionToolName, + isSkillToolName, isShellToolName, toolContainsCallId, } from './toolFormatting'; @@ -77,6 +80,9 @@ export function hasExpandableContent(tool: ACPToolCall): boolean { const text = extractText(tool); return !!text && text.trim().length > 0 && text.split('\n').length > 1; } + if (isSkillToolName(name)) { + return !!getFirstToolContentText(tool); + } if (name === 'edit' || name === 'write' || name === 'editfile') { return hasEditContent(tool); } @@ -104,6 +110,7 @@ function hasDetailView(tool: ACPToolCall): boolean { name === 'read' || name === 'read_file' || name === 'readfile' || + isSkillToolName(name) || isAskUserQuestionToolName(tool.toolName) ); } @@ -477,6 +484,22 @@ function ExpandedAskUserQuestionOutput({ tool }: { tool: ACPToolCall }) { return 
{text}
; } +function ExpandedSkillOutput({ tool }: { tool: ACPToolCall }) { + const content = + getFirstToolContentText(tool) || + (typeof tool.args?.args === 'string' && tool.args.args.trim() + ? tool.args.args.trim() + : (tool.title ?? '')); + + return 
{content}
; +} + +function getFirstToolContentText(tool: ACPToolCall): string { + const block = tool.content?.[0]; + if (block?.type !== 'content') return ''; + return typeof block.content?.text === 'string' ? block.content.text : ''; +} + export function getToolHeaderKind(tool: ACPToolCall): ToolHeaderKind { const name = tool.toolName.toLowerCase(); if (isSubAgentToolCall(tool)) return 'agent'; @@ -575,22 +598,74 @@ export function formatSingleToolSummary( return t('toolGroup.summary.askedUser', { count: 1 }); } - const displayName = localizeToolDisplayName(tool.toolName, t); - const description = truncateText(getToolDescription(tool, workspaceCwd), 120); - return [displayName, description].filter(Boolean).join(' '); + const { displayName, description, hideDisplayName } = + getSingleToolSummaryInfo(tool, t, workspaceCwd); + return [hideDisplayName ? '' : displayName, description] + .filter(Boolean) + .join(' '); } -export function formatRunningSingleToolSummary( +function getSingleToolSummaryInfo( tool: ACPToolCall, t: ReturnType['t'], - duration?: string, workspaceCwd?: string, -): string { - return t('toolGroup.running', { - name: formatSingleToolSummary(tool, t, workspaceCwd), - count: 1, - duration: duration ?? '', - }); +): ToolHeaderExtraRenderInfo & { hideDisplayName: boolean } { + const displayName = localizeToolDisplayName(tool.toolName, t); + const description = truncateText( + getToolSummaryDescription(tool, workspaceCwd), + 120, + ); + return { + kind: getToolHeaderKind(tool), + tool, + displayName, + description, + elapsed: '', + workspaceCwd, + hideDisplayName: !!getShellToolSemanticDescription(tool), + }; +} + +function SingleToolSummary({ + tool, + runningDuration, + workspaceCwd, +}: { + tool: ACPToolCall; + runningDuration?: string; + workspaceCwd?: string; +}) { + const { t } = useI18n(); + const runningPrefix = + isActiveToolStatus(tool.status) && t('toolGroup.runningPrefix').trim(); + + if ( + isTodoWriteToolName(tool.toolName) || + isAskUserQuestionToolName(tool.toolName) + ) { + return ( + <> + {runningPrefix && {runningPrefix} } + {formatSingleToolSummary(tool, t, workspaceCwd)} + {runningDuration && {runningDuration}} + + ); + } + + const info = getSingleToolSummaryInfo(tool, t, workspaceCwd); + + return ( + <> + {runningPrefix && {runningPrefix} } + + {!info.hideDisplayName && ( + {info.displayName} + )} + + + {runningDuration && {runningDuration}} + + ); } function formatCompletedToolSummary( @@ -1046,6 +1121,12 @@ export const ToolLine = memo(function ToolLine({ workspaceCwd, }} /> +
)} {showExpanded && ( @@ -1061,8 +1142,12 @@ export const ToolLine = memo(function ToolLine({ ); } - const description = getToolDescription(tool, workspaceCwd); + const fullDescription = getToolDescription(tool, workspaceCwd); const result = getToolResultSummary(tool); + const summaryShell = summaryOnly && isShellToolName(tool.toolName); + const description = summaryShell + ? getToolSummaryDescription(tool, workspaceCwd) + : fullDescription; const displayName = localizeToolDisplayName(tool.toolName, t); const elapsed = isShellToolName(tool.toolName) || isWebFetchToolName(tool.toolName) @@ -1100,7 +1185,7 @@ export const ToolLine = memo(function ToolLine({ const useMarkdownDetail = isRead; const hideDescriptionInHeader = showDescriptionInDetail && !isShell && !isSearch && !isRead; - const expandedCardDetail = description; + const expandedCardDetail = fullDescription; const showExpandedSummaryPanel = !isTodo && expanded && !detailView && (showDescriptionInDetail || result); @@ -1158,6 +1243,14 @@ export const ToolLine = memo(function ToolLine({ workspaceCwd, }} /> + {expandable && ( + { ).toBe('cat ~/.qwen/settings.json (查看 ~/.qwen/settings.json 文件内容)'); }); + it('uses semantic shell descriptions for summaries', () => { + const shellTool = tool({ + toolName: 'run_shell_command', + title: + 'Shell: dataworks-infra workspace list [timeout: 30000ms] (查询用户工作空间列表)', + args: { + command: 'dataworks-infra workspace list', + description: '查询用户工作空间列表', + timeout: 30000, + }, + }); + + expect(getToolSummaryDescription(shellTool)).toBe('查询用户工作空间列表'); + expect(getToolDescription(shellTool)).toBe( + 'dataworks-infra workspace list [timeout: 30000ms] (查询用户工作空间列表)', + ); + }); + + it('falls back to shell commands in summaries without timeout metadata', () => { + expect( + getToolSummaryDescription( + tool({ + toolName: 'run_shell_command', + args: { + command: 'npm test', + timeout: 1000, + }, + }), + ), + ).toBe('npm test'); + }); + + it('describes skill calls from raw input', () => { + expect( + getToolDescription( + tool({ + toolName: 'skill', + args: { + skill: 'qc-helper', + args: 'weather in Hangzhou next 5 days', + }, + }), + ), + ).toBe('qc-helper'); + }); + it('summarizes read_file rawOutput by line count', () => { expect( getToolResultSummary( diff --git a/packages/web-shell/client/components/messages/toolFormatting.ts b/packages/web-shell/client/components/messages/toolFormatting.ts index 72b53ca072..8ca0d8e8e1 100644 --- a/packages/web-shell/client/components/messages/toolFormatting.ts +++ b/packages/web-shell/client/components/messages/toolFormatting.ts @@ -100,6 +100,10 @@ export function getToolDescription( tool: ACPToolCall, workspaceCwd?: string, ): string { + if (isSkillToolName(tool.toolName)) { + const skillName = getStringArg(tool.args, 'skill'); + if (skillName) return truncateText(skillName, MAX_DESCRIPTION_LENGTH); + } const fromTitle = getDescriptionFromTitle(tool, workspaceCwd); if (fromTitle) return truncateText(fromTitle, MAX_DESCRIPTION_LENGTH); const fromArgs = getDescriptionFromArgs(tool, workspaceCwd); @@ -107,6 +111,29 @@ export function getToolDescription( return ''; } +export function getToolSummaryDescription( + tool: ACPToolCall, + workspaceCwd?: string, +): string { + if (!isShellToolName(tool.toolName)) { + return getToolDescription(tool, workspaceCwd); + } + + const description = getStringArg(tool.args, 'description'); + if (description) return truncateText(description, MAX_DESCRIPTION_LENGTH); + + const fromArgs = getDescriptionFromArgs(tool, workspaceCwd, { + includeTimeout: false, + }); + if (fromArgs) return truncateText(fromArgs, MAX_DESCRIPTION_LENGTH); + return ''; +} + +export function getShellToolSemanticDescription(tool: ACPToolCall): string { + if (!isShellToolName(tool.toolName)) return ''; + return getStringArg(tool.args, 'description'); +} + export function extractText(tool: ACPToolCall): string | null { if (!tool.content) { return extractRawOutputText(tool.rawOutput); @@ -223,9 +250,11 @@ function parseGrepSummary(text: string): string | null { function getDescriptionFromArgs( tool: ACPToolCall, workspaceCwd?: string, + options: { includeTimeout?: boolean } = {}, ): string { const args = tool.args || {}; const name = tool.toolName.toLowerCase(); + const includeTimeout = options.includeTimeout ?? true; if (args.command) { let description = String(args.command); @@ -234,11 +263,12 @@ function getDescriptionFromArgs( } if (args.is_background) { description += ' [background]'; - } else if (args.timeout) { + } else if (includeTimeout && args.timeout) { description += ` [timeout: ${String(args.timeout)}ms]`; } - if (args.description) { - description += ` (${String(args.description).replace(/\n/g, ' ')})`; + const argDescription = getStringArg(args, 'description'); + if (argDescription) { + description += ` (${argDescription})`; } return truncateText(description, MAX_DESCRIPTION_LENGTH); } @@ -285,6 +315,14 @@ function getDescriptionFromArgs( return ''; } +function getStringArg( + args: Record | undefined, + key: string, +): string { + const value = args?.[key]; + return typeof value === 'string' ? value.trim().replace(/\n/g, ' ') : ''; +} + export function isShellToolName(name: string): boolean { const normalized = name.toLowerCase(); return ( @@ -295,6 +333,10 @@ export function isShellToolName(name: string): boolean { ); } +export function isSkillToolName(name: string): boolean { + return name.toLowerCase() === 'skill'; +} + export function toolContainsCallId( tool: ACPToolCall, toolCallId: string, diff --git a/packages/web-shell/client/components/messages/tools/ToolChrome.module.css b/packages/web-shell/client/components/messages/tools/ToolChrome.module.css index be33a4b9f9..0e9138ec5d 100644 --- a/packages/web-shell/client/components/messages/tools/ToolChrome.module.css +++ b/packages/web-shell/client/components/messages/tools/ToolChrome.module.css @@ -82,6 +82,16 @@ font-weight: 400; } +.chatSummaryInline { + display: inline-flex; + align-items: center; + gap: 4px; + min-width: 0; + max-width: 100%; + color: inherit; + vertical-align: bottom; +} + .chatSummaryTextActive { color: var(--muted-foreground); background-image: linear-gradient( @@ -193,12 +203,12 @@ align-items: center; gap: 8px; min-width: 0; -} - -.icon { color: var(--muted-foreground); font-size: 13px; font-weight: 400; +} + +.icon { flex-shrink: 0; white-space: nowrap; } @@ -208,15 +218,10 @@ } .lineName { - font-weight: 400; - color: var(--muted-foreground); - font-size: 13px; flex-shrink: 0; } .lineArg { - color: var(--muted-foreground); - font-size: 13px; min-width: 0; flex: 1; overflow: hidden; @@ -226,7 +231,6 @@ .lineElapsed { margin-left: auto; - color: var(--muted-foreground); font-size: 12px; flex-shrink: 0; } @@ -242,12 +246,53 @@ cursor: pointer; } -.lineExpandable:hover .lineName, -.lineExpandable:hover .lineArg, -.lineExpandable:hover .lineElapsed { +.lineExpandable .lineArg { + flex: 0 1 auto; +} + +.lineExpandable:hover, +.lineExpandable:focus-visible { color: var(--foreground); } +.lineChevronRight, +.lineChevronDown { + width: 12px; + height: 12px; + display: inline-block; + flex-shrink: 0; + opacity: 0; + transition: opacity 120ms ease; +} + +.lineChevronRight::before, +.lineChevronDown::before { + content: ''; + display: block; + width: 5px; + height: 5px; + border-right: 1.25px solid currentColor; + border-bottom: 1.25px solid currentColor; + border-radius: 0.5px; + margin: 3px 0 0 2px; +} + +.lineChevronRight::before { + transform: rotate(-45deg); +} + +.lineChevronDown::before { + margin: 2px 0 0 3px; + transform: rotate(45deg); +} + +.lineExpandable:hover .lineChevronRight, +.lineExpandable:hover .lineChevronDown, +.lineExpandable:focus-visible .lineChevronRight, +.lineExpandable:focus-visible .lineChevronDown { + opacity: 1; +} + .lineOutput { margin-left: 16px; font-size: 12px; @@ -386,6 +431,9 @@ align-items: baseline; gap: 8px; min-width: 0; + color: var(--muted-foreground); + font-size: 13px; + font-weight: 400; } .compactCount { diff --git a/packages/web-shell/client/i18n.tsx b/packages/web-shell/client/i18n.tsx index 5c689abb2a..8defe5be78 100644 --- a/packages/web-shell/client/i18n.tsx +++ b/packages/web-shell/client/i18n.tsx @@ -1552,6 +1552,7 @@ const EN: Messages = { `Running ${v?.name ?? 'tool'}${v?.duration ? ` ${v.duration}` : ''}${ Number(v?.count ?? 0) > 1 ? ` · ${v?.count ?? 0} tools` : '' }`, + 'toolGroup.runningPrefix': 'Running', 'thinking.expand': 'Expand thinking', 'thinking.collapse': 'Collapse thinking', 'thinking.running': (v) => `Thinking${v?.duration ? ` ${v.duration}` : ''}`, @@ -1643,7 +1644,7 @@ const ZH: Messages = { 'toolName.todo_write': '任务清单', 'toolName.save_memory': '保存记忆', 'toolName.agent': 'Agent', - 'toolName.skill': '技能', + 'toolName.skill': '查看技能', 'toolName.enter_plan_mode': '进入计划模式', 'toolName.exit_plan_mode': '退出计划模式', 'toolName.web_fetch': '网络搜索', @@ -3118,6 +3119,7 @@ const ZH: Messages = { `正在执行 ${v?.name ?? '工具'}${v?.duration ? ` ${v.duration}` : ''}${ Number(v?.count ?? 0) > 1 ? ` · 共 ${v?.count ?? 0} 个工具` : '' }`, + 'toolGroup.runningPrefix': '正在执行', 'thinking.expand': '展开思考', 'thinking.collapse': '收起思考', 'thinking.running': (v) => `正在思考${v?.duration ? ` ${v.duration}` : ''}`, From 55b2886909c8a8408033ca085703eb079a0bc69a Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Tue, 7 Jul 2026 22:08:59 +0800 Subject: [PATCH 10/11] fix(web-shell): split-view pane fixes (remove "current" badge, clear composer on send) (#6454) * fix(web-shell): remove meaningless "current" badge from split-view panes In the split view every pane is an equal, independently interactive session (its own DaemonSessionProvider, SSE, transcript, and approvals), so tagging one pane as the workspace's "current" session carried no operational meaning. It only leaked a single-view concept into a peer-of-equals layout and reliably prompted "what is this?" questions from users. Panes are already identified by their titles. Drop the isCurrent badge and its border highlight from ChatPane, and stop passing isCurrent from SplitView. The sidebar and session overview keep their "current" indicators, which are legitimate "you are here" navigation. currentSessionId is retained only to seed the initial pane. * fix(web-shell): clear the split-view composer on send, not at turn end A split-view pane kept the just-sent text sitting in its composer until the whole turn finished. handleSubmit committed the draft on the sendPrompt promise resolving, but that promise resolves via waitForAcceptedPromptCompletion (turn end), not at admission. Switch to the onAdmitted hook so the composer clears the moment the daemon accepts the prompt, matching the main view. A prompt rejected before admission still preserves the draft and surfaces the error. * fix(web-shell): pass commitAccepted directly as onAdmitted; cover admit-then-fail Review follow-up: - commitAccepted is already `() => void`, so pass it directly as the onAdmitted option instead of wrapping it in a redundant `() => commitAccepted?.()` closure. - Add a test for the turn failing after admission: the draft stays cleared (no second commit) and the error is still surfaced to onError. --- .../client/components/ChatPane.module.css | 14 ----- .../client/components/ChatPane.test.tsx | 55 +++++++++++++++++-- .../web-shell/client/components/ChatPane.tsx | 35 ++++++------ .../client/components/SplitView.test.tsx | 23 +++----- .../web-shell/client/components/SplitView.tsx | 6 +- 5 files changed, 77 insertions(+), 56 deletions(-) diff --git a/packages/web-shell/client/components/ChatPane.module.css b/packages/web-shell/client/components/ChatPane.module.css index 9e502a9212..b08a96e10f 100644 --- a/packages/web-shell/client/components/ChatPane.module.css +++ b/packages/web-shell/client/components/ChatPane.module.css @@ -10,10 +10,6 @@ overflow: hidden; } -.paneCurrent { - border-color: color-mix(in srgb, var(--primary) 60%, var(--border)); -} - .header { display: flex; align-items: center; @@ -52,16 +48,6 @@ color: var(--foreground); } -.currentBadge { - flex: 0 0 auto; - padding: 1px 7px; - border-radius: 999px; - font-size: 11px; - font-weight: 600; - color: var(--primary); - background: color-mix(in srgb, var(--primary) 14%, transparent); -} - .closeButton { flex: 0 0 auto; display: inline-flex; diff --git a/packages/web-shell/client/components/ChatPane.test.tsx b/packages/web-shell/client/components/ChatPane.test.tsx index 32923397e2..970cace7df 100644 --- a/packages/web-shell/client/components/ChatPane.test.tsx +++ b/packages/web-shell/client/components/ChatPane.test.tsx @@ -21,6 +21,7 @@ let latestOnSubmit: | undefined; let sendPromptResolve: (() => void) | undefined; let sendPromptReject: ((e: unknown) => void) | undefined; +let sendPromptAdmit: (() => void) | undefined; const sendPrompt = vi.fn(async () => ({}) as any); const submitPermission = vi.fn(async () => {}); const cancel = vi.fn(async () => {}); @@ -55,7 +56,9 @@ vi.mock('./StreamingStatus', () => ({ StreamingStatus: (props: any) => (
), @@ -121,10 +124,13 @@ beforeEach(() => { latestOnSubmit = undefined; sendPrompt.mockReset(); // Each sendPrompt returns a promise the test controls, so we can assert the - // draft is committed only after the prompt is accepted. + // draft is committed on admission (onAdmitted) rather than on turn completion + // (promise resolution). `sendPromptAdmit` captures the options.onAdmitted hook. + sendPromptAdmit = undefined; sendPrompt.mockImplementation( - () => + (_text?: string, options?: { onAdmitted?: () => void }) => new Promise((resolve, reject) => { + sendPromptAdmit = options?.onAdmitted; sendPromptResolve = () => resolve({}); sendPromptReject = (e) => reject(e); }), @@ -182,16 +188,26 @@ describe('ChatPane', () => { expect((sendPrompt.mock.calls[0] as unknown[])[0]).toBe('hello there'); }); - it('commits the draft only after the prompt is accepted', async () => { + it('commits the draft on admission, without waiting for the turn to finish', async () => { render(); const commit = vi.fn(); let returned: boolean | undefined; act(() => { returned = latestOnSubmit!('hi', undefined, commit); }); - // Returns false (keep the draft) and does NOT commit before acceptance. + // Returns false (keep the draft) and does NOT commit before admission. expect(returned).toBe(false); expect(commit).not.toHaveBeenCalled(); + // The daemon admits the prompt (onAdmitted). The draft clears now, even + // though the turn promise is still pending — a long response must not strand + // the sent text in the composer until the turn ends. + await act(async () => { + sendPromptAdmit!(); + await Promise.resolve(); + }); + expect(commit).toHaveBeenCalledTimes(1); + // The turn finishing later must NOT commit again — guards against regressing + // to committing on promise resolution (turn end) instead of admission. await act(async () => { sendPromptResolve!(); await Promise.resolve(); @@ -212,6 +228,29 @@ describe('ChatPane', () => { expect(commit).not.toHaveBeenCalled(); }); + it('keeps the draft cleared and still reports the error when the turn fails after admission', async () => { + const onError = vi.fn(); + render({ onError }); + const commit = vi.fn(); + act(() => { + latestOnSubmit!('hi', undefined, commit); + }); + // Admission clears the draft. + await act(async () => { + sendPromptAdmit!(); + await Promise.resolve(); + }); + expect(commit).toHaveBeenCalledTimes(1); + // The turn then fails mid-flight: the draft stays cleared (no second commit) + // and the failure is still surfaced to onError. + await act(async () => { + sendPromptReject!(new Error('turn crashed')); + await Promise.resolve(); + }); + expect(commit).toHaveBeenCalledTimes(1); + expect(onError).toHaveBeenCalled(); + }); + it('keeps pane approvals click-only (no global keyboard shortcuts)', () => { pendingPermission = { id: 'perm-1', toolName: 'write_file', rawInput: {} }; render(); @@ -237,7 +276,11 @@ describe('ChatPane', () => { new MouseEvent('click', { bubbles: true }), ), ); - expect(submitPermission).toHaveBeenCalledWith('perm-1', 'proceed', undefined); + expect(submitPermission).toHaveBeenCalledWith( + 'perm-1', + 'proceed', + undefined, + ); }); it('routes an AskUserQuestion permission to the AskUserQuestion overlay', () => { diff --git a/packages/web-shell/client/components/ChatPane.tsx b/packages/web-shell/client/components/ChatPane.tsx index f7f7e1c139..a640e694ce 100644 --- a/packages/web-shell/client/components/ChatPane.tsx +++ b/packages/web-shell/client/components/ChatPane.tsx @@ -30,8 +30,6 @@ const EMPTY_TOOLBAR: never[] = []; export interface ChatPaneProps { /** Header label; falls back to the session's own display name / id. */ title?: string; - /** Marks the pane bound to the window's primary (sidebar-selected) session. */ - isCurrent?: boolean; onClose?: () => void; onError?: (error: unknown, fallback: string) => void; } @@ -43,7 +41,7 @@ export interface ChatPaneProps { * state, approvals, and composer, and the browser scopes keyboard focus to the * pane the user clicks into — so there is no cross-pane approval arbitration. */ -export function ChatPane({ title, isCurrent, onClose, onError }: ChatPaneProps) { +export function ChatPane({ title, onClose, onError }: ChatPaneProps) { const { t } = useI18n(); const connection = useConnection(); const actions = useActions(); @@ -92,13 +90,19 @@ export function ChatPane({ title, isCurrent, onClose, onError }: ChatPaneProps) ): boolean => { const trimmed = text.trim(); if (!trimmed) return false; - // Keep the draft (return false) until the prompt is actually accepted: - // sendPrompt can reject (transcript still loading, session disconnected, - // or a turn already active), and committing first would silently drop the - // user's text. Commit only once it resolves. + // Keep the draft (return false) and clear it only once the daemon ADMITS + // the prompt. `onAdmitted` fires at acceptance; the sendPrompt promise + // itself resolves only when the whole (possibly long) turn finishes, so + // committing on resolution would strand the sent text in the composer for + // the entire response. If the prompt is rejected before admission + // (transcript still loading, session disconnected, or a turn already + // active) onAdmitted never fires, so the draft is preserved and the error + // is surfaced. actions - .sendPrompt(trimmed, images && images.length ? { images } : undefined) - .then(() => commitAccepted?.()) + .sendPrompt(trimmed, { + ...(images && images.length ? { images } : {}), + onAdmitted: commitAccepted, + }) .catch((error: unknown) => reportError(error, 'Failed to send prompt')); return false; }, @@ -119,7 +123,9 @@ export function ChatPane({ title, isCurrent, onClose, onError }: ChatPaneProps) const handleCancel = useCallback(() => { actions .cancel() - .catch((error: unknown) => reportError(error, 'Failed to cancel request')); + .catch((error: unknown) => + reportError(error, 'Failed to cancel request'), + ); }, [actions, reportError]); const headerLabel = @@ -127,9 +133,7 @@ export function ChatPane({ title, isCurrent, onClose, onError }: ChatPaneProps) return (
@@ -137,11 +141,6 @@ export function ChatPane({ title, isCurrent, onClose, onError }: ChatPaneProps) {headerLabel} - {isCurrent && ( - - {t('sessionsOverview.current')} - - )} {onClose && ( )} + {orderedVisibleColumnIndexes.length > 0 && ( + + )} {activeFilterCount > 0 && ( {t('markdownTable.filtersActive', { count: activeFilterCount })} @@ -1670,10 +1950,14 @@ export function EnhancedTable({ - - {visibleColumnIndexes.map((columnIndex) => { + {orderedVisibleColumnIndexes.map((columnIndex) => { const header = table.headers[columnIndex]; if (!header) return null; const isSorted = sort?.columnIndex === columnIndex; @@ -1704,12 +1988,18 @@ export function EnhancedTable({ const headerAlignStyle = header.textAlign ? { textAlign: header.textAlign } : undefined; + const isFrozenColumn = frozenColumnIndex === columnIndex; + const isActiveColumn = activeColumn === columnIndex; return ( ); })} @@ -1760,7 +2087,11 @@ export function EnhancedTable({ - - {visibleColumnIndexes.map((columnIndex) => { + {orderedVisibleColumnIndexes.map((columnIndex) => { const cell = row.cells[columnIndex]; if (!cell) return null; const cellAlignStyle = cell.textAlign ? { textAlign: cell.textAlign } : undefined; + const isFrozenColumn = frozenColumnIndex === columnIndex; return (
+ {t('markdownTable.actions')} dropColumn(event, columnIndex)} + style={columnStyle(columnIndex, headerAlignStyle)} >
+
+
+ @@ -1816,13 +2148,13 @@ export function EnhancedTable({
{t('markdownTable.detailsHeader')}
- {visibleColumnIndexes.map((columnIndex) => { + {orderedVisibleColumnIndexes.map((columnIndex) => { const header = table.headers[columnIndex]; const cell = row.cells[columnIndex]; if (!header || !cell) return null; @@ -1881,7 +2213,7 @@ export function EnhancedTable({ sort={sort} style={{ left: openFilterMenu.left, top: openFilterMenu.top }} menuRef={filterMenuRef} - canHideColumn={visibleColumnIndexes.length > 1} + canHideColumn={orderedVisibleColumnIndexes.length > 1} onApply={setColumnFilter} onClose={closeFilterMenu} onHideColumn={hideColumn} diff --git a/packages/web-shell/client/i18n.tsx b/packages/web-shell/client/i18n.tsx index 8defe5be78..06716cb6b6 100644 --- a/packages/web-shell/client/i18n.tsx +++ b/packages/web-shell/client/i18n.tsx @@ -265,11 +265,15 @@ const EN: Messages = { }, 'markdownTable.copyTsv': 'Copy TSV', 'markdownTable.copyVisible': 'Quick copy', + 'markdownTable.freezeFirstColumn': 'Freeze first column', + 'markdownTable.unfreezeFirstColumn': 'Unfreeze first column', 'markdownTable.hideColumn': 'Hide column', 'markdownTable.showHiddenColumns': (v) => { const count = Number(v?.count ?? 0); return `Show ${count} hidden column${count === 1 ? '' : 's'}`; }, + 'markdownTable.moveColumn': (v) => `Move ${v?.column ?? ''}`, + 'markdownTable.resizeColumn': (v) => `Resize ${v?.column ?? ''}`, 'markdownTable.rowDetails': 'Details', 'markdownTable.rowDetailsAria': (v) => `View details for row ${v?.index ?? ''}`, @@ -1910,8 +1914,12 @@ const ZH: Messages = { 'markdownTable.cellsSelected': (v) => `${v?.count ?? 0} 个单元格已选中`, 'markdownTable.copyTsv': '复制 TSV', 'markdownTable.copyVisible': '快捷复制', + 'markdownTable.freezeFirstColumn': '冻结首列', + 'markdownTable.unfreezeFirstColumn': '取消冻结首列', 'markdownTable.hideColumn': '隐藏列', 'markdownTable.showHiddenColumns': (v) => `显示 ${v?.count ?? 0} 个隐藏列`, + 'markdownTable.moveColumn': (v) => `移动 ${v?.column ?? ''}`, + 'markdownTable.resizeColumn': (v) => `调整 ${v?.column ?? ''} 列宽`, 'markdownTable.rowDetails': '详情', 'markdownTable.rowDetailsAria': (v) => `查看第 ${v?.index ?? ''} 行详情`, 'markdownTable.closeRowDetailsAria': (v) => `收起第 ${v?.index ?? ''} 行详情`,