Skip to content
Merged
35 changes: 18 additions & 17 deletions packages/cli/src/serve/routes/workspace-file-read.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -601,30 +602,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];
}

Expand Down
5 changes: 4 additions & 1 deletion packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
23 changes: 18 additions & 5 deletions packages/core/src/tools/record-artifact.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,13 @@ 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 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.`;

export const ARTIFACT_TITLE_MAX_LENGTH = 200;
export const ARTIFACT_WORKSPACE_PATH_MAX_LENGTH = 500;

class RecordArtifactInvocation extends BaseToolInvocation<
RecordArtifactParams,
ToolResult
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -318,7 +326,7 @@ function isDisplayField(field: string): boolean {
);
}

function hasControlCharacter(
export function hasControlCharacter(
value: string,
allowLineWhitespace = false,
): boolean {
Expand Down Expand Up @@ -346,7 +354,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,
Expand All @@ -356,7 +364,12 @@ 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;
}
Expand Down
Loading
Loading