fix(core): auto-record artifact file writes

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';
@@ -557,30 +558,30 @@ 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 = '<!doctype html><h1>Quarterly Chart</h1>';
 
-  /** 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(
-      {
-        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];
   }
 

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,

packages/core/src/tools/record-artifact.ts

@@ -28,7 +28,7 @@ export interface RecordArtifactParams {
   metadata?: Record<string, string | number | boolean | null>;
 }
 
-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.`;
 
@@ -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,

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 = '<!doctype html><html><body>Weather</body></html>';
       const params = {
         file_path: filePath,
-        content: '<!doctype html><html><body>Weather</body></html>',
+        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,96 @@ 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.each([
+      ['page.htm', 'html'],
+      ['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 remind for ordinary source files', async () => {
+    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');
+      const params = {
+        file_path: filePath,
+        content: '<!doctype html><html><body>Weather</body></html>',
+      };
+
+      const result = await tool.build(params).execute(abortSignal);
+
+      expect(result.llmContent).not.toContain('automatically recorded');
+      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',
+        '<img src=x onerror=alert(1)>.html',
+      );
+      const params = {
+        file_path: filePath,
+        content: '<!doctype html><html><body>XSS</body></html>',
+      };
+
+      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');
       const params = {
@@ -490,10 +583,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 +597,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 +621,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;
       }