Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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