From b9ae027875bc65c529050f0f6f5be52fbe88547d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=A7=A6=E5=A5=87?= Date: Tue, 28 Jul 2026 15:48:02 +0800 Subject: [PATCH 1/9] fix(core): auto-record write_file artifacts --- .../serve/routes/workspace-file-read.test.ts | 20 ++--- packages/core/src/tools/write-file.test.ts | 64 ++++++++++++--- packages/core/src/tools/write-file.ts | 78 ++++++++++++++++--- 3 files changed, 130 insertions(+), 32 deletions(-) diff --git a/packages/cli/src/serve/routes/workspace-file-read.test.ts b/packages/cli/src/serve/routes/workspace-file-read.test.ts index f14b81d1133..0a751051c99 100644 --- a/packages/cli/src/serve/routes/workspace-file-read.test.ts +++ b/packages/cli/src/serve/routes/workspace-file-read.test.ts @@ -557,18 +557,18 @@ describe('capability advertisement', () => { }); }); -// The `record_artifact` hint that `write_file` appends to a successful write -// names a `workspacePath`, and the model hands that exact string back to this -// route. Producer and consumer live in different packages, each with its own -// notion of what the path is relative to — `write_file` starts from the session -// cwd, the route resolves against the bound workspace root. They agree for an -// ordinary session and drifted apart for a worktree one, where every artifact -// preview 404'd. Neither side's unit tests could catch that: both were -// internally consistent. These pin the round trip instead, over real HTTP. -describe('record_artifact workspacePath contract (write_file ⇄ GET /file)', () => { +// The artifact metadata that `write_file` returns for successful writes names a +// `workspacePath`, and this route later resolves that exact string. Producer and +// consumer live in different packages, each with its own notion of what the path +// is relative to — `write_file` starts from the session cwd, the route resolves +// against the bound workspace root. They agree for an ordinary session and +// drifted apart for a worktree one, where every artifact preview 404'd. Neither +// side's unit tests could catch that: both were internally consistent. These pin +// the round trip instead, over real HTTP. +describe('artifact workspacePath contract (write_file ⇄ GET /file)', () => { const ARTIFACT = '

Quarterly Chart

'; - /** The workspacePath the model is told to send, from the real producer. */ + /** The workspacePath emitted by the real producer. */ function emittedWorkspacePath(sessionCwd: string, filePath: string): string { const reminder = buildRecordArtifactReminder( { diff --git a/packages/core/src/tools/write-file.test.ts b/packages/core/src/tools/write-file.test.ts index 4e67dcb55d3..6b3ea7ed841 100644 --- a/packages/core/src/tools/write-file.test.ts +++ b/packages/core/src/tools/write-file.test.ts @@ -447,24 +447,34 @@ describe('WriteFileTool', () => { ); }); - it('reminds the model to record artifact-like workspace files', async () => { + it('records artifact-like workspace files in the tool result', async () => { mockConfigInternal.isRecordArtifactEnabled.mockReturnValue(true); const filePath = path.join(rootDir, 'reports', 'weather.html'); + const content = 'Weather'; const params = { file_path: filePath, - content: 'Weather', + content, }; const result = await tool.build(params).execute(abortSignal); - expect(result.llmContent).toContain('record_artifact'); + expect(result.llmContent).toContain('automatically recorded'); expect(result.llmContent).toContain( 'workspacePath "reports/weather.html"', ); - expect(result.artifacts).toBeUndefined(); + expect(result.artifacts).toEqual([ + { + title: 'weather.html', + kind: 'html', + storage: 'workspace', + workspacePath: 'reports/weather.html', + mimeType: 'text/html', + sizeBytes: Buffer.byteLength(content), + }, + ]); }); - it('reminds for case-insensitive artifact extensions', async () => { + it('records case-insensitive artifact extensions', async () => { mockConfigInternal.isRecordArtifactEnabled.mockReturnValue(true); const filePath = path.join(rootDir, 'reports', 'dashboard.HTML'); const params = { @@ -474,13 +484,34 @@ describe('WriteFileTool', () => { const result = await tool.build(params).execute(abortSignal); - expect(result.llmContent).toContain('record_artifact'); + expect(result.llmContent).toContain('automatically recorded'); expect(result.llmContent).toContain( 'workspacePath "reports/dashboard.HTML"', ); + expect(result.artifacts?.[0]).toMatchObject({ + title: 'dashboard.HTML', + kind: 'html', + storage: 'workspace', + workspacePath: 'reports/dashboard.HTML', + mimeType: 'text/html', + }); + }); + + it('does not record artifact-like files when artifact recording is disabled', async () => { + mockConfigInternal.isRecordArtifactEnabled.mockReturnValue(false); + const filePath = path.join(rootDir, 'reports', 'weather.html'); + const params = { + file_path: filePath, + content: 'Weather', + }; + + const result = await tool.build(params).execute(abortSignal); + + expect(result.llmContent).not.toContain('automatically recorded'); + expect(result.artifacts).toBeUndefined(); }); - it('does not remind for ordinary source files', async () => { + it('does not record ordinary source files as artifacts', async () => { mockConfigInternal.isRecordArtifactEnabled.mockReturnValue(true); const filePath = path.join(rootDir, 'src', 'index.ts'); const params = { @@ -490,10 +521,11 @@ describe('WriteFileTool', () => { const result = await tool.build(params).execute(abortSignal); - expect(result.llmContent).not.toContain('record_artifact'); + expect(result.llmContent).not.toContain('automatically recorded'); + expect(result.artifacts).toBeUndefined(); }); - it('does not remind for files outside the workspace', async () => { + it('does not record files outside the workspace as artifacts', async () => { mockConfigInternal.isRecordArtifactEnabled.mockReturnValue(true); const filePath = path.join(tempDir, 'outside.html'); const params = { @@ -503,10 +535,11 @@ describe('WriteFileTool', () => { const result = await tool.build(params).execute(abortSignal); - expect(result.llmContent).not.toContain('record_artifact'); + expect(result.llmContent).not.toContain('automatically recorded'); + expect(result.artifacts).toBeUndefined(); }); - it('suggests workspace-root-relative path inside a worktree', async () => { + it('records workspace-root-relative path inside a worktree', async () => { mockConfigInternal.isRecordArtifactEnabled.mockReturnValue(true); const worktreeDir = path.join( rootDir, @@ -526,10 +559,17 @@ describe('WriteFileTool', () => { const result = await tool.build(params).execute(abortSignal); - expect(result.llmContent).toContain('record_artifact'); + expect(result.llmContent).toContain('automatically recorded'); expect(result.llmContent).toContain( 'workspacePath ".qwen/worktrees/my-feature/report.html"', ); + expect(result.artifacts?.[0]).toMatchObject({ + title: 'report.html', + kind: 'html', + storage: 'workspace', + workspacePath: '.qwen/worktrees/my-feature/report.html', + mimeType: 'text/html', + }); } finally { mockConfigInternal.getTargetDir = originalGetTargetDir; } diff --git a/packages/core/src/tools/write-file.ts b/packages/core/src/tools/write-file.ts index 63e6154af68..cc3eb772b35 100644 --- a/packages/core/src/tools/write-file.ts +++ b/packages/core/src/tools/write-file.ts @@ -12,6 +12,8 @@ import { isAnyAutoMemPath, isTeamAutoMemPath } from '../memory/paths.js'; import { checkTeamMemorySecrets } from '../memory/team-memory-secret-guard.js'; import type { FileDiff, + ToolArtifact, + ToolArtifactKind, ToolCallConfirmationDetails, ToolEditConfirmationDetails, ToolInvocation, @@ -520,8 +522,10 @@ class WriteFileToolInvocation extends BaseToolInvocation< // pre-write placeholder. Best-effort: a stat failure here does // not undo the successful write — the next Read will re-stat // and either see fresh content or treat the entry as stale. + let postWriteSizeBytes: number | undefined; try { const postWriteStats = fs.statSync(file_path); + postWriteSizeBytes = postWriteStats.size; this.config.getFileReadCache().recordWrite(file_path, postWriteStats); } catch { // Non-fatal: leaving a stale entry is preferable to failing @@ -561,10 +565,14 @@ class WriteFileToolInvocation extends BaseToolInvocation< `User modified the \`content\` to be: ${content}`, ); } - const artifactReminder = buildRecordArtifactReminder( + const artifact = buildWorkspaceArtifactMetadata( this.config, file_path, + postWriteSizeBytes, ); + const artifactReminder = artifact + ? buildRecordArtifactReminder(this.config, file_path) + : null; if (artifactReminder) { llmSuccessMessageParts.push(artifactReminder); } @@ -601,6 +609,7 @@ class WriteFileToolInvocation extends BaseToolInvocation< return { llmContent: llmSuccessMessageParts.join(' '), returnDisplay: displayResult, + ...(artifact ? { artifacts: [artifact] } : {}), }; } catch (error) { // Capture detailed error information for debugging @@ -644,19 +653,53 @@ class WriteFileToolInvocation extends BaseToolInvocation< } /** - * Builds the `record_artifact` hint appended to a successful write. + * Builds the artifact-recording note appended to a successful write. * - * Exported because the string it produces is a CONTRACT with the daemon's + * Exported because the workspacePath it produces is a CONTRACT with the daemon's * `GET /file` route: the `workspacePath` computed here is later resolved by * `resolveWithinWorkspace` against the bound workspace root. The two sides live * in different packages and drifted apart once already (a worktree session - * emitted a worktree-relative path that the route resolved against the + * emitted a session-cwd-relative path that the route resolved against the * workspace root, so every artifact preview 404'd). `workspace-file-read.test.ts` * pins the round-trip; keep this exported so it can keep doing so. */ export function buildRecordArtifactReminder( config: Config, filePath: string, +): string | null { + const workspacePath = getRecordArtifactWorkspacePath(config, filePath); + if (!workspacePath) { + return null; + } + return ( + `This file was automatically recorded as a workspace artifact with ` + + `workspacePath "${workspacePath}". No extra artifact registration step ` + + `is needed.` + ); +} + +function buildWorkspaceArtifactMetadata( + config: Config, + filePath: string, + sizeBytes?: number, +): ToolArtifact | null { + const workspacePath = getRecordArtifactWorkspacePath(config, filePath); + if (!workspacePath) { + return null; + } + return { + title: path.basename(filePath), + kind: inferWorkspaceArtifactKind(filePath), + storage: 'workspace', + workspacePath, + mimeType: getSpecificMimeType(filePath), + sizeBytes, + }; +} + +function getRecordArtifactWorkspacePath( + config: Config, + filePath: string, ): string | null { if (!config.isRecordArtifactEnabled()) { return null; @@ -683,12 +726,27 @@ export function buildRecordArtifactReminder( ) { return null; } - const workspacePath = relativePath.split(path.sep).join('/'); - return ( - `If this file is a reusable user-facing artifact, call ` + - `record_artifact with workspacePath "${workspacePath}" before telling ` + - `the user it is available in the artifacts panel.` - ); + return relativePath.split(path.sep).join('/'); +} + +function inferWorkspaceArtifactKind(filePath: string): ToolArtifactKind { + switch (path.extname(filePath).toLowerCase()) { + case '.htm': + case '.html': + return 'html'; + case '.ipynb': + return 'notebook'; + case '.pdf': + return 'pdf'; + case '.jpeg': + case '.jpg': + case '.png': + case '.svg': + case '.webp': + return 'image'; + default: + return 'file'; + } } /** From 866fe07f88dd29b781b5a91342ef6a952ec3d509 Mon Sep 17 00:00:00 2001 From: qwen-code-ci-bot Date: Tue, 28 Jul 2026 09:51:10 +0000 Subject: [PATCH 2/9] test(core): cover all inferWorkspaceArtifactKind kind groups (#7914) Co-authored-by: Qwen-Coder --- packages/core/src/tools/write-file.test.ts | 26 ++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/packages/core/src/tools/write-file.test.ts b/packages/core/src/tools/write-file.test.ts index 6b3ea7ed841..8d4271a0624 100644 --- a/packages/core/src/tools/write-file.test.ts +++ b/packages/core/src/tools/write-file.test.ts @@ -497,6 +497,32 @@ describe('WriteFileTool', () => { }); }); + it.each([ + ['notebook.ipynb', 'notebook'], + ['paper.pdf', 'pdf'], + ['photo.png', 'image'], + ['photo.jpeg', 'image'], + ['photo.jpg', 'image'], + ['diagram.svg', 'image'], + ['photo.webp', 'image'], + ])('infers artifact kind for %s as %s', async (fileName, expectedKind) => { + mockConfigInternal.isRecordArtifactEnabled.mockReturnValue(true); + const filePath = path.join(rootDir, 'reports', fileName); + const params = { + file_path: filePath, + content: 'artifact content', + }; + + const result = await tool.build(params).execute(abortSignal); + + expect(result.artifacts?.[0]).toMatchObject({ + title: fileName, + kind: expectedKind, + storage: 'workspace', + workspacePath: `reports/${fileName}`, + }); + }); + it('does not record artifact-like files when artifact recording is disabled', async () => { mockConfigInternal.isRecordArtifactEnabled.mockReturnValue(false); const filePath = path.join(rootDir, 'reports', 'weather.html'); From 8a65624ddf4283da031bbbe3ad556af88d21372f Mon Sep 17 00:00:00 2001 From: Qwen Code Date: Tue, 28 Jul 2026 11:55:03 +0000 Subject: [PATCH 3/9] test(core): cover .htm extension in artifact kind inference (#7914) --- packages/core/src/tools/write-file.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/core/src/tools/write-file.test.ts b/packages/core/src/tools/write-file.test.ts index 8d4271a0624..dcf89e903f9 100644 --- a/packages/core/src/tools/write-file.test.ts +++ b/packages/core/src/tools/write-file.test.ts @@ -498,6 +498,7 @@ describe('WriteFileTool', () => { }); it.each([ + ['page.htm', 'html'], ['notebook.ipynb', 'notebook'], ['paper.pdf', 'pdf'], ['photo.png', 'image'], From d31994da241f99c70ca0cbb75e26e1866b42a0e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=A7=A6=E5=A5=87?= Date: Tue, 28 Jul 2026 20:49:16 +0800 Subject: [PATCH 4/9] fix(core): clarify artifact registration guidance --- packages/core/src/tools/record-artifact.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/tools/record-artifact.ts b/packages/core/src/tools/record-artifact.ts index 3e627e620ad..10449ad2878 100644 --- a/packages/core/src/tools/record-artifact.ts +++ b/packages/core/src/tools/record-artifact.ts @@ -28,7 +28,7 @@ export interface RecordArtifactParams { metadata?: Record; } -const DESCRIPTION = `Registers a session artifact so clients can show it in an artifacts panel. Use it after creating a useful file, URL, image, report, notebook, or other intermediate result that the user may want to open later. +const DESCRIPTION = `Registers a session artifact so clients can show it in an artifacts panel. Use it after creating a useful file, URL, image, report, notebook, or other intermediate result that the user may want to open later, unless the producing tool already returned artifact metadata. For example, write_file automatically records artifact-like workspace files, so do not call record_artifact again for the same workspacePath. This tool only records metadata. It does not publish, upload, read, write, or verify the referenced resource. Provide exactly one locator: workspacePath, managedId, or url. Use the Artifact tool, not record_artifact, for published interactive HTML artifacts.`; From ac377675a5f73800964cae6de3344cc7bcd1762f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=A7=A6=E5=A5=87?= Date: Tue, 28 Jul 2026 20:58:03 +0800 Subject: [PATCH 5/9] fix(core): keep artifact extension kinds in sync --- packages/core/src/tools/write-file.ts | 56 +++++++++++---------------- 1 file changed, 23 insertions(+), 33 deletions(-) diff --git a/packages/core/src/tools/write-file.ts b/packages/core/src/tools/write-file.ts index cc3eb772b35..1895c453206 100644 --- a/packages/core/src/tools/write-file.ts +++ b/packages/core/src/tools/write-file.ts @@ -55,16 +55,16 @@ import { CommitAttributionService } from '../services/commitAttribution.js'; import { createDebugLogger } from '../utils/debugLogger.js'; const debugLogger = createDebugLogger('WRITE_FILE'); -const ARTIFACT_LIKE_EXTENSIONS = new Set([ - '.htm', - '.html', - '.ipynb', - '.jpeg', - '.jpg', - '.pdf', - '.png', - '.svg', - '.webp', +const ARTIFACT_KIND_BY_EXTENSION = new Map([ + ['.htm', 'html'], + ['.html', 'html'], + ['.ipynb', 'notebook'], + ['.jpeg', 'image'], + ['.jpg', 'image'], + ['.pdf', 'pdf'], + ['.png', 'image'], + ['.svg', 'image'], + ['.webp', 'image'], ]); /** @@ -570,11 +570,10 @@ class WriteFileToolInvocation extends BaseToolInvocation< file_path, postWriteSizeBytes, ); - const artifactReminder = artifact - ? buildRecordArtifactReminder(this.config, file_path) - : null; - if (artifactReminder) { - llmSuccessMessageParts.push(artifactReminder); + if (artifact) { + llmSuccessMessageParts.push( + formatRecordArtifactReminder(artifact.workspacePath), + ); } // Log file operation for telemetry (without diff_stat to avoid double-counting) @@ -671,6 +670,10 @@ export function buildRecordArtifactReminder( if (!workspacePath) { return null; } + return formatRecordArtifactReminder(workspacePath); +} + +function formatRecordArtifactReminder(workspacePath: string): string { return ( `This file was automatically recorded as a workspace artifact with ` + `workspacePath "${workspacePath}". No extra artifact registration step ` + @@ -704,7 +707,7 @@ function getRecordArtifactWorkspacePath( if (!config.isRecordArtifactEnabled()) { return null; } - if (!ARTIFACT_LIKE_EXTENSIONS.has(path.extname(filePath).toLowerCase())) { + if (!ARTIFACT_KIND_BY_EXTENSION.has(path.extname(filePath).toLowerCase())) { return null; } // The daemon's file-read route resolves workspacePath against the @@ -730,23 +733,10 @@ function getRecordArtifactWorkspacePath( } function inferWorkspaceArtifactKind(filePath: string): ToolArtifactKind { - switch (path.extname(filePath).toLowerCase()) { - case '.htm': - case '.html': - return 'html'; - case '.ipynb': - return 'notebook'; - case '.pdf': - return 'pdf'; - case '.jpeg': - case '.jpg': - case '.png': - case '.svg': - case '.webp': - return 'image'; - default: - return 'file'; - } + return ( + ARTIFACT_KIND_BY_EXTENSION.get(path.extname(filePath).toLowerCase()) ?? + 'file' + ); } /** From 16af7b82f0d68231a3e9da0deffc328be9b85a7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=A7=A6=E5=A5=87?= Date: Tue, 28 Jul 2026 21:06:10 +0800 Subject: [PATCH 6/9] fix(core): type workspace write artifacts --- packages/core/src/tools/write-file.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/core/src/tools/write-file.ts b/packages/core/src/tools/write-file.ts index 1895c453206..597a34cd298 100644 --- a/packages/core/src/tools/write-file.ts +++ b/packages/core/src/tools/write-file.ts @@ -67,6 +67,11 @@ const ARTIFACT_KIND_BY_EXTENSION = new Map([ ['.webp', 'image'], ]); +type WorkspaceToolArtifact = ToolArtifact & { + storage: 'workspace'; + workspacePath: string; +}; + /** * Parameters for the WriteFile tool */ @@ -685,7 +690,7 @@ function buildWorkspaceArtifactMetadata( config: Config, filePath: string, sizeBytes?: number, -): ToolArtifact | null { +): WorkspaceToolArtifact | null { const workspacePath = getRecordArtifactWorkspacePath(config, filePath); if (!workspacePath) { return null; From 2674aa4a12e16dc594a7bc51ba1fb9ef70d52f2f Mon Sep 17 00:00:00 2001 From: qwen-code-bot Date: Tue, 28 Jul 2026 14:55:47 +0000 Subject: [PATCH 7/9] fix(core): guard artifact title safety and fill ipynb mimeType (#7914) --- .../serve/routes/workspace-file-read.test.ts | 15 ++++---- packages/core/src/index.ts | 5 ++- packages/core/src/tools/record-artifact.ts | 2 +- packages/core/src/tools/write-file.test.ts | 35 +++++++++++++++++++ packages/core/src/tools/write-file.ts | 17 +++++++-- 5 files changed, 62 insertions(+), 12 deletions(-) diff --git a/packages/cli/src/serve/routes/workspace-file-read.test.ts b/packages/cli/src/serve/routes/workspace-file-read.test.ts index 0a751051c99..9bbbc77c4e9 100644 --- a/packages/cli/src/serve/routes/workspace-file-read.test.ts +++ b/packages/cli/src/serve/routes/workspace-file-read.test.ts @@ -12,6 +12,7 @@ import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import request from 'supertest'; import { buildRecordArtifactReminder, + buildWorkspaceArtifactMetadata, Ignore, type Config, } from '@qwen-code/qwen-code-core'; @@ -570,17 +571,17 @@ describe('artifact workspacePath contract (write_file ⇄ GET /file)', () => { /** The workspacePath emitted by the real producer. */ function emittedWorkspacePath(sessionCwd: string, filePath: string): string { - const reminder = buildRecordArtifactReminder( - { - isRecordArtifactEnabled: () => true, - getTargetDir: () => sessionCwd, - } as unknown as Config, - filePath, - ); + const config = { + isRecordArtifactEnabled: () => true, + getTargetDir: () => sessionCwd, + } as unknown as Config; + const reminder = buildRecordArtifactReminder(config, filePath); const match = /workspacePath "([^"]+)"/.exec(reminder ?? ''); if (!match?.[1]) { throw new Error(`no workspacePath in reminder: ${reminder ?? 'null'}`); } + const artifact = buildWorkspaceArtifactMetadata(config, filePath); + expect(artifact?.workspacePath).toBe(match[1]); return match[1]; } diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 8e95bb9b44a..24c080e251f 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -199,7 +199,10 @@ export type { WriteFileTool, WriteFileToolParams } from './tools/write-file.js'; // Exported for the cross-package contract test in packages/cli (see the // function's own doc comment) — the daemon's file-read route must resolve the // workspacePath this produces. -export { buildRecordArtifactReminder } from './tools/write-file.js'; +export { + buildRecordArtifactReminder, + buildWorkspaceArtifactMetadata, +} from './tools/write-file.js'; export type { ArtifactTool, ArtifactToolParams, diff --git a/packages/core/src/tools/record-artifact.ts b/packages/core/src/tools/record-artifact.ts index 10449ad2878..6898bf367c4 100644 --- a/packages/core/src/tools/record-artifact.ts +++ b/packages/core/src/tools/record-artifact.ts @@ -346,7 +346,7 @@ function hasControlCharacter( return false; } -function hasUnsafeDisplayPayload(value: string): boolean { +export function hasUnsafeDisplayPayload(value: string): boolean { return ( /<\s*\/?[a-z!]|&(?:#[0-9]+|#x[0-9a-f]+|[a-z][a-z0-9]+);|javascript\s*:|data\s*:\s*(?:text\/(?:html|javascript)|application\/javascript|image\/svg\+xml)/i.test( value, diff --git a/packages/core/src/tools/write-file.test.ts b/packages/core/src/tools/write-file.test.ts index dcf89e903f9..0097d35a5ec 100644 --- a/packages/core/src/tools/write-file.test.ts +++ b/packages/core/src/tools/write-file.test.ts @@ -524,6 +524,22 @@ describe('WriteFileTool', () => { }); }); + it('sets application/x-ipynb+json mimeType for notebooks', async () => { + mockConfigInternal.isRecordArtifactEnabled.mockReturnValue(true); + const filePath = path.join(rootDir, 'notes', 'analysis.ipynb'); + const params = { + file_path: filePath, + content: '{"cells":[]}', + }; + + const result = await tool.build(params).execute(abortSignal); + + expect(result.artifacts?.[0]).toMatchObject({ + kind: 'notebook', + mimeType: 'application/x-ipynb+json', + }); + }); + it('does not record artifact-like files when artifact recording is disabled', async () => { mockConfigInternal.isRecordArtifactEnabled.mockReturnValue(false); const filePath = path.join(rootDir, 'reports', 'weather.html'); @@ -538,6 +554,25 @@ describe('WriteFileTool', () => { expect(result.artifacts).toBeUndefined(); }); + it('does not record artifacts whose filename contains unsafe markup', async () => { + mockConfigInternal.isRecordArtifactEnabled.mockReturnValue(true); + const filePath = path.join( + rootDir, + 'reports', + '.html', + ); + const params = { + file_path: filePath, + content: 'XSS', + }; + + const result = await tool.build(params).execute(abortSignal); + + expect(result.llmContent).toContain('Successfully created'); + expect(result.llmContent).not.toContain('automatically recorded'); + expect(result.artifacts).toBeUndefined(); + }); + it('does not record ordinary source files as artifacts', async () => { mockConfigInternal.isRecordArtifactEnabled.mockReturnValue(true); const filePath = path.join(rootDir, 'src', 'index.ts'); diff --git a/packages/core/src/tools/write-file.ts b/packages/core/src/tools/write-file.ts index 597a34cd298..1cebb321e67 100644 --- a/packages/core/src/tools/write-file.ts +++ b/packages/core/src/tools/write-file.ts @@ -53,6 +53,7 @@ import { import { getLanguageFromFilePath } from '../utils/language-detection.js'; import { CommitAttributionService } from '../services/commitAttribution.js'; import { createDebugLogger } from '../utils/debugLogger.js'; +import { hasUnsafeDisplayPayload } from './record-artifact.js'; const debugLogger = createDebugLogger('WRITE_FILE'); const ARTIFACT_KIND_BY_EXTENSION = new Map([ @@ -686,7 +687,7 @@ function formatRecordArtifactReminder(workspacePath: string): string { ); } -function buildWorkspaceArtifactMetadata( +export function buildWorkspaceArtifactMetadata( config: Config, filePath: string, sizeBytes?: number, @@ -695,12 +696,22 @@ function buildWorkspaceArtifactMetadata( if (!workspacePath) { return null; } + const title = path.basename(filePath); + // The daemon store rejects titles with unsafe markup; skip the artifact + // rather than tell the model it was recorded when it will be dropped. + if (hasUnsafeDisplayPayload(title)) { + return null; + } return { - title: path.basename(filePath), + title, kind: inferWorkspaceArtifactKind(filePath), storage: 'workspace', workspacePath, - mimeType: getSpecificMimeType(filePath), + mimeType: + getSpecificMimeType(filePath) ?? + (filePath.toLowerCase().endsWith('.ipynb') + ? 'application/x-ipynb+json' + : undefined), sizeBytes, }; } From f6afd485d3da52109c8135c94207d7ed4545da1f Mon Sep 17 00:00:00 2001 From: Qwen Code Autofix Date: Wed, 29 Jul 2026 07:43:30 +0000 Subject: [PATCH 8/9] fix(core): align artifact guard with store rules and fix Windows test (#7914) --- packages/core/src/tools/record-artifact.ts | 4 +-- packages/core/src/tools/write-file.test.ts | 35 +++++++++++++++++++- packages/core/src/tools/write-file.ts | 38 ++++++++++++---------- 3 files changed, 56 insertions(+), 21 deletions(-) diff --git a/packages/core/src/tools/record-artifact.ts b/packages/core/src/tools/record-artifact.ts index 6898bf367c4..222ca25230b 100644 --- a/packages/core/src/tools/record-artifact.ts +++ b/packages/core/src/tools/record-artifact.ts @@ -28,7 +28,7 @@ export interface RecordArtifactParams { metadata?: Record; } -const DESCRIPTION = `Registers a session artifact so clients can show it in an artifacts panel. Use it after creating a useful file, URL, image, report, notebook, or other intermediate result that the user may want to open later, unless the producing tool already returned artifact metadata. For example, write_file automatically records artifact-like workspace files, so do not call record_artifact again for the same workspacePath. +const DESCRIPTION = `Registers a session artifact so clients can show it in an artifacts panel. Use it after creating a useful file, URL, image, report, notebook, or other intermediate result that the user may want to open later, unless the producing tool already returned artifact metadata. For example, write_file automatically records HTML, image, PDF, and notebook files it writes inside the workspace, so do not call record_artifact again for the same workspacePath; still call it for other formats such as Markdown, CSV, JSON, and plain text, and for files produced outside write_file. This tool only records metadata. It does not publish, upload, read, write, or verify the referenced resource. Provide exactly one locator: workspacePath, managedId, or url. Use the Artifact tool, not record_artifact, for published interactive HTML artifacts.`; @@ -318,7 +318,7 @@ function isDisplayField(field: string): boolean { ); } -function hasControlCharacter( +export function hasControlCharacter( value: string, allowLineWhitespace = false, ): boolean { diff --git a/packages/core/src/tools/write-file.test.ts b/packages/core/src/tools/write-file.test.ts index 0097d35a5ec..ced8b57e471 100644 --- a/packages/core/src/tools/write-file.test.ts +++ b/packages/core/src/tools/write-file.test.ts @@ -559,7 +559,7 @@ describe('WriteFileTool', () => { const filePath = path.join( rootDir, 'reports', - '.html', + 'chart onerror=alert(1).html', ); const params = { file_path: filePath, @@ -573,6 +573,39 @@ describe('WriteFileTool', () => { expect(result.artifacts).toBeUndefined(); }); + it('does not record artifacts whose title exceeds 200 characters', async () => { + mockConfigInternal.isRecordArtifactEnabled.mockReturnValue(true); + const longName = 'a'.repeat(196) + '.html'; + const filePath = path.join(rootDir, 'reports', longName); + const params = { + file_path: filePath, + content: 'Long', + }; + + const result = await tool.build(params).execute(abortSignal); + + expect(result.llmContent).toContain('Successfully created'); + expect(result.llmContent).not.toContain('automatically recorded'); + expect(result.artifacts).toBeUndefined(); + }); + + it('does not record artifacts whose workspace path contains unsafe markup', async () => { + mockConfigInternal.isRecordArtifactEnabled.mockReturnValue(true); + const dir = path.join(rootDir, 'Q&A'); + fs.mkdirSync(dir, { recursive: true }); + const filePath = path.join(dir, 'summary.html'); + const params = { + file_path: filePath, + content: 'Summary', + }; + + const result = await tool.build(params).execute(abortSignal); + + expect(result.llmContent).toContain('Successfully created'); + expect(result.llmContent).not.toContain('automatically recorded'); + expect(result.artifacts).toBeUndefined(); + }); + it('does not record ordinary source files as artifacts', async () => { mockConfigInternal.isRecordArtifactEnabled.mockReturnValue(true); const filePath = path.join(rootDir, 'src', 'index.ts'); diff --git a/packages/core/src/tools/write-file.ts b/packages/core/src/tools/write-file.ts index 1cebb321e67..3cb96736fea 100644 --- a/packages/core/src/tools/write-file.ts +++ b/packages/core/src/tools/write-file.ts @@ -53,7 +53,10 @@ import { import { getLanguageFromFilePath } from '../utils/language-detection.js'; import { CommitAttributionService } from '../services/commitAttribution.js'; import { createDebugLogger } from '../utils/debugLogger.js'; -import { hasUnsafeDisplayPayload } from './record-artifact.js'; +import { + hasControlCharacter, + hasUnsafeDisplayPayload, +} from './record-artifact.js'; const debugLogger = createDebugLogger('WRITE_FILE'); const ARTIFACT_KIND_BY_EXTENSION = new Map([ @@ -658,25 +661,16 @@ class WriteFileToolInvocation extends BaseToolInvocation< } /** - * Builds the artifact-recording note appended to a successful write. - * - * Exported because the workspacePath it produces is a CONTRACT with the daemon's - * `GET /file` route: the `workspacePath` computed here is later resolved by - * `resolveWithinWorkspace` against the bound workspace root. The two sides live - * in different packages and drifted apart once already (a worktree session - * emitted a session-cwd-relative path that the route resolved against the - * workspace root, so every artifact preview 404'd). `workspace-file-read.test.ts` - * pins the round-trip; keep this exported so it can keep doing so. + * Kept for the cross-package contract test in `workspace-file-read.test.ts`: + * the daemon's `GET /file` route resolves the `workspacePath` this produces. + * Delegates to `buildWorkspaceArtifactMetadata` so the two agree by construction. */ export function buildRecordArtifactReminder( config: Config, filePath: string, ): string | null { - const workspacePath = getRecordArtifactWorkspacePath(config, filePath); - if (!workspacePath) { - return null; - } - return formatRecordArtifactReminder(workspacePath); + const artifact = buildWorkspaceArtifactMetadata(config, filePath); + return artifact ? formatRecordArtifactReminder(artifact.workspacePath) : null; } function formatRecordArtifactReminder(workspacePath: string): string { @@ -697,9 +691,17 @@ export function buildWorkspaceArtifactMetadata( return null; } const title = path.basename(filePath); - // The daemon store rejects titles with unsafe markup; skip the artifact - // rather than tell the model it was recorded when it will be dropped. - if (hasUnsafeDisplayPayload(title)) { + // The daemon store rejects titles and paths that are too long, carry control + // characters, or contain markup; skip the artifact rather than tell the model + // it was recorded when it will be dropped. + if ( + title.length > 200 || + hasControlCharacter(title) || + hasUnsafeDisplayPayload(title) || + workspacePath.length > 500 || + hasControlCharacter(workspacePath) || + hasUnsafeDisplayPayload(workspacePath) + ) { return null; } return { From 9379f5b6a79d5c5d7bcf965bc5fce83da97bf9c6 Mon Sep 17 00:00:00 2001 From: Qwen Code Autofix Date: Wed, 29 Jul 2026 10:27:45 +0000 Subject: [PATCH 9/9] fix(core): share artifact length limits and pin guard clauses (#7914) --- packages/core/src/tools/record-artifact.ts | 17 +++++++- packages/core/src/tools/write-file.test.ts | 45 +++++++++++++++++++++- packages/core/src/tools/write-file.ts | 9 ++++- 3 files changed, 66 insertions(+), 5 deletions(-) diff --git a/packages/core/src/tools/record-artifact.ts b/packages/core/src/tools/record-artifact.ts index 222ca25230b..6a16f952b1f 100644 --- a/packages/core/src/tools/record-artifact.ts +++ b/packages/core/src/tools/record-artifact.ts @@ -32,6 +32,9 @@ const DESCRIPTION = `Registers a session artifact so clients can show it in an a This tool only records metadata. It does not publish, upload, read, write, or verify the referenced resource. Provide exactly one locator: workspacePath, managedId, or url. Use the Artifact tool, not record_artifact, for published interactive HTML artifacts.`; +export const ARTIFACT_TITLE_MAX_LENGTH = 200; +export const ARTIFACT_WORKSPACE_PATH_MAX_LENGTH = 500; + class RecordArtifactInvocation extends BaseToolInvocation< RecordArtifactParams, ToolResult @@ -158,7 +161,12 @@ export class RecordArtifactTool extends BaseDeclarativeTool< params: RecordArtifactParams, ): string | null { params.title = (params.title ?? '').trim(); - const titleError = validateString(params.title, 'title', 200, true); + const titleError = validateString( + params.title, + 'title', + ARTIFACT_TITLE_MAX_LENGTH, + true, + ); if (titleError) { return titleError; } @@ -356,7 +364,12 @@ export function hasUnsafeDisplayPayload(value: string): boolean { function validateWorkspacePath(value: string): string | null { const trimmed = value.trim(); - const stringError = validateString(trimmed, 'workspacePath', 500, true); + const stringError = validateString( + trimmed, + 'workspacePath', + ARTIFACT_WORKSPACE_PATH_MAX_LENGTH, + true, + ); if (stringError) { return stringError; } diff --git a/packages/core/src/tools/write-file.test.ts b/packages/core/src/tools/write-file.test.ts index ced8b57e471..6e3482dcc38 100644 --- a/packages/core/src/tools/write-file.test.ts +++ b/packages/core/src/tools/write-file.test.ts @@ -14,7 +14,11 @@ import { type Mocked, } from 'vitest'; import type { WriteFileToolParams } from './write-file.js'; -import { WriteFileTool } from './write-file.js'; +import { + WriteFileTool, + buildRecordArtifactReminder, + buildWorkspaceArtifactMetadata, +} from './write-file.js'; import { ToolErrorType } from './tool-error.js'; import type { FileDiff, ToolEditConfirmationDetails } from './tools.js'; import { ToolConfirmationOutcome } from './tools.js'; @@ -1624,3 +1628,42 @@ describe('WriteFileTool', () => { }); }); }); + +describe('workspace artifact metadata guard', () => { + beforeEach(() => { + mockConfigInternal.isRecordArtifactEnabled.mockReturnValue(true); + }); + + // Pins the delegation: buildRecordArtifactReminder must agree with + // buildWorkspaceArtifactMetadata. If the reminder is reverted to compute the + // path independently (without the safety guard), it would still emit a hint + // for this markup-bearing filename while the artifact is correctly skipped, + // reintroducing the false "automatically recorded" claim. + it('keeps the reminder and the artifact in lockstep when the guard rejects', () => { + const rejected = path.resolve( + rootDir, + 'reports', + 'chart onerror=alert(1).html', + ); + expect(buildWorkspaceArtifactMetadata(mockConfig, rejected)).toBeNull(); + expect(buildRecordArtifactReminder(mockConfig, rejected)).toBeNull(); + }); + + it('skips artifacts whose workspace path exceeds the store limit', () => { + // A short filename buried in a deep directory: the workspace path blows + // past the 500-char store limit while the title stays well under its own, + // so this exercises the path-length clause on its own. + const deepDir = 'a'.repeat(510); + const filePath = path.resolve(rootDir, deepDir, 'x.html'); + expect(buildWorkspaceArtifactMetadata(mockConfig, filePath)).toBeNull(); + }); + + it('skips artifacts whose workspace path contains a control character', () => { + // The control character sits in a directory segment, not the basename: the + // title is path.basename(filePath), so a control character in the title + // would also appear in the path and could not prove the path-side check on + // its own. + const filePath = path.resolve(rootDir, 'reports\u000b', 'chart.html'); + expect(buildWorkspaceArtifactMetadata(mockConfig, filePath)).toBeNull(); + }); +}); diff --git a/packages/core/src/tools/write-file.ts b/packages/core/src/tools/write-file.ts index 3cb96736fea..81237f15a9c 100644 --- a/packages/core/src/tools/write-file.ts +++ b/packages/core/src/tools/write-file.ts @@ -54,6 +54,8 @@ import { getLanguageFromFilePath } from '../utils/language-detection.js'; import { CommitAttributionService } from '../services/commitAttribution.js'; import { createDebugLogger } from '../utils/debugLogger.js'; import { + ARTIFACT_TITLE_MAX_LENGTH, + ARTIFACT_WORKSPACE_PATH_MAX_LENGTH, hasControlCharacter, hasUnsafeDisplayPayload, } from './record-artifact.js'; @@ -695,13 +697,16 @@ export function buildWorkspaceArtifactMetadata( // characters, or contain markup; skip the artifact rather than tell the model // it was recorded when it will be dropped. if ( - title.length > 200 || + title.length > ARTIFACT_TITLE_MAX_LENGTH || hasControlCharacter(title) || hasUnsafeDisplayPayload(title) || - workspacePath.length > 500 || + workspacePath.length > ARTIFACT_WORKSPACE_PATH_MAX_LENGTH || hasControlCharacter(workspacePath) || hasUnsafeDisplayPayload(workspacePath) ) { + debugLogger.debug('workspace artifact skipped (safety checks)', { + path: filePath, + }); return null; } return {