From c26a32006a82d516eeb83640f8fc3a4f1243707b Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Tue, 3 Nov 2026 10:26:15 +0800 Subject: [PATCH] fix(serve): keep record_artifact guidance out of Managed Runtime worker results (#16148) * fix(serve): keep record_artifact guidance out of Managed Runtime worker results The worker builds its tools from a configuration that left artifact recording at the core default. A Hosted Session declares no record_artifact and the worker forwards only response parts, so every Shell result for a command not proven read-only told the model to call a tool it does not have, and write_file claimed artifact-like files were recorded when nothing records them. The reminder's read-only check also loaded the Bash AST parser in every sandbox for each successful Shell call. Turn artifact recording off for the worker's tools regardless of env flags, as the shared execution worker already does. The local Managed engine's provider worker, which hands artifacts to its client, is unchanged. Fixes #16147 * fix(serve): keep artifact guidance out of the CSI files worker too Add a Config option, artifactRecordingSupported, for hosts that expose no record_artifact tool and forward no artifact metadata. Unlike artifactEnabled, the artifact env flags cannot turn recording back on. The Managed Runtime worker, the CSI files worker and the shared execution worker now pass it instead of overriding isRecordArtifactEnabled on the instance. The CSI files worker's write_file and edit no longer tell the model that a file was recorded as a workspace artifact. The tests now derive that claim from formatRecordArtifactReminder() instead of a literal, cover edit, and assert that a worker Shell call makes no call into the Bash parser. --- .../serve/managed-csi-file-backend.test.ts | 43 ++++++- .../src/serve/managed-csi-file-composer.ts | 3 + .../managed-runtime-tool-executor.test.ts | 120 ++++++++++++++++++ .../serve/managed-runtime-tool-executor.ts | 4 + packages/core/src/config/config.test.ts | 4 + packages/core/src/config/config.ts | 9 ++ .../core/src/services/execution-worker.ts | 4 +- 7 files changed, 178 insertions(+), 9 deletions(-) diff --git a/packages/cli/src/serve/managed-csi-file-backend.test.ts b/packages/cli/src/serve/managed-csi-file-backend.test.ts index 991dd7f37bc..855880aca5f 100644 --- a/packages/cli/src/serve/managed-csi-file-backend.test.ts +++ b/packages/cli/src/serve/managed-csi-file-backend.test.ts @@ -17,6 +17,7 @@ import { CSI_FILES_RETIREMENT_CAPABILITY_DIGEST } from './managed-csi-file-profi import type { ManagedCsiMount } from './managed-csi-mount.js'; import type { ManagedCsiMountReceipt } from './managed-csi-envelope.js'; import { readCsiFileHistoryObservation } from './managed-csi-file-history-protocol.js'; +import { formatRecordArtifactReminder } from '@qwen-code/qwen-code-core/tools/artifact/workspace-auto-record.js'; import { isModifiableDeclarativeTool } from '@qwen-code/qwen-code-core/tools/modifiable-tool.js'; import type { AnyDeclarativeTool } from '@qwen-code/qwen-code-core/tools/tools.js'; @@ -823,6 +824,41 @@ function cases(mapped: boolean) { await expect(composed.close()).rejects.toThrow(); }); + // The profile declares no record_artifact and the Worker forwards no + // artifact metadata, so nothing records these files. + it.each(['', '1'])( + 'does not claim an actual Write or Edit was recorded as an artifact (QWEN_CODE_ENABLE_ARTIFACT=%j)', + async (flag) => { + vi.stubEnv('QWEN_CODE_ENABLE_ARTIFACT', flag); + onTestFinished(() => { + vi.unstubAllEnvs(); + }); + const owned = await fixture(mapped); + const composed = await owned.compose(); + const file = owned.file('report.md'); + const written = await composed.tools.tools + .get('write_file')! + .build({ file_path: file, content: '# Report\n' }) + .execute(signal); + const edited = await composed.tools.tools + .get('edit')! + .build({ + file_path: file, + old_string: '# Report', + new_string: '# Final report', + }) + .execute(signal); + expect(written.error).toBeUndefined(); + expect(edited.error).toBeUndefined(); + expect(await fs.readFile(file, 'utf8')).toBe('# Final report\n'); + const claim = formatRecordArtifactReminder('report.md'); + expect(String(written.llmContent)).not.toContain(claim); + expect(String(edited.llmContent)).not.toContain(claim); + expect(written.artifacts).toBeUndefined(); + expect(edited.artifacts).toBeUndefined(); + }, + ); + it('keeps the actual team-memory secret guard without private pathname discovery', async () => { const owned = await fixture(mapped); const composed = await owned.compose(); @@ -880,13 +916,6 @@ function cases(mapped: boolean) { }), ); expect(first.error).toBeUndefined(); - expect(first.artifacts).toEqual([ - expect.objectContaining({ - storage: 'workspace', - workspacePath: 'nested/created.html', - sizeBytes: Buffer.byteLength('alpha\r\nsecond\r\n'), - }), - ]); const before = await run(tools.get('read_file')!, { file_path: file }); expect(before.error).toBeUndefined(); expect(JSON.stringify(before.llmContent)).toContain('alpha'); diff --git a/packages/cli/src/serve/managed-csi-file-composer.ts b/packages/cli/src/serve/managed-csi-file-composer.ts index 4c04851caad..a1520e299d6 100644 --- a/packages/cli/src/serve/managed-csi-file-composer.ts +++ b/packages/cli/src/serve/managed-csi-file-composer.ts @@ -72,6 +72,9 @@ export async function composeManagedCsiFiles(options: { approvalMode: ApprovalMode.YOLO, fileCheckpointingEnabled: false, fileReadCacheDisabled: true, + // The CSI profile declares no record_artifact and its results carry + // no artifact metadata to the host. + artifactRecordingSupported: false, }); config.setFileSystemService(storage); const history = new ManagedRuntimeFileHistory( diff --git a/packages/cli/src/serve/managed-runtime-tool-executor.test.ts b/packages/cli/src/serve/managed-runtime-tool-executor.test.ts index ca722a330eb..8def0571efb 100644 --- a/packages/cli/src/serve/managed-runtime-tool-executor.test.ts +++ b/packages/cli/src/serve/managed-runtime-tool-executor.test.ts @@ -22,6 +22,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { Storage } from '@qwen-code/qwen-code-core/config/storage.js'; import type { ToolResultEnvelope } from '@qwen-code/qwen-code-core/managed-runtime/managed-tool-result.js'; import type { LocalShellCaptureRequest } from '@qwen-code/qwen-code-core/managed-runtime/managed-shell-result-session.js'; +import { formatRecordArtifactReminder } from '@qwen-code/qwen-code-core/tools/artifact/workspace-auto-record.js'; import { managedToolDigest } from '@qwen-code/qwen-code-core/tools/managed-tool-protocol.js'; import type { AnyDeclarativeTool } from '@qwen-code/qwen-code-core/tools/tools.js'; import { @@ -77,6 +78,27 @@ vi.mock( }, ); +// Records each call into the Bash parser module so a test can pin a Shell +// path that should never load the parser. Calls pass through unchanged. +const bashParserCalls = vi.hoisted(() => [] as string[]); +vi.mock( + '@qwen-code/qwen-code-core/utils/shellAstParser.js', + async (importOriginal) => { + const actual = await importOriginal>(); + return Object.fromEntries( + Object.entries(actual).map(([name, value]) => [ + name, + typeof value === 'function' + ? (...args: unknown[]) => { + bashParserCalls.push(name); + return (value as (...callArgs: unknown[]) => unknown)(...args); + } + : value, + ]), + ); + }, +); + // A long-running foreground command the Shell tool admits: a bare // `sleep N` trips its standalone-sleep refusal long before the budget. const LONG_RUN = `"${process.execPath}" -e 'setInterval(()=>{},1000)'`; @@ -2551,6 +2573,104 @@ describe('todo_write in the Worker', () => { }); }); +// A Hosted Session declares no record_artifact and the Worker forwards no +// artifact metadata, so no Worker result may ask the model to record one. +describe.skipIf(process.platform === 'win32')( + 'record_artifact guidance in the Worker', + () => { + let workspace: string; + let runtimeDir: string; + + function reference(callId: string, input: unknown): ManagedToolReference { + return { + sessionId: 'session-artifact', + promptId: 'prompt-artifact', + callId, + argsDigest: managedToolDigest(input as Record), + }; + } + + async function resultText( + exec: ManagedToolExecutor, + callId: string, + toolName: string, + input: Record, + ): Promise { + const result = await exec.execute( + reference(callId, input), + toolName, + input, + ); + expect(result.executionStatus).toBe('success'); + return (result.responseParts as Array<{ text?: string }>) + .map((part) => part.text ?? '') + .join(''); + } + + beforeEach(async () => { + workspace = await mkdtemp(path.join(tmpdir(), 'qwen-artifact-')); + runtimeDir = await mkdtemp(path.join(tmpdir(), 'qwen-artifact-runtime-')); + Storage.setRuntimeBaseDir(runtimeDir); + }); + + afterEach(async () => { + vi.unstubAllEnvs(); + Storage.setRuntimeBaseDir(undefined); + await rm(workspace, { recursive: true, force: true }); + await rm(runtimeDir, { recursive: true, force: true }); + }); + + it.each(['', '1'])( + 'leaves it out of a Shell result that wrote a file (QWEN_CODE_ENABLE_ARTIFACT=%j)', + async (flag) => { + vi.stubEnv('QWEN_CODE_ENABLE_ARTIFACT', flag); + const exec = ManagedToolExecutor.forWorkspace( + workspace, + 'session-artifact', + ); + bashParserCalls.length = 0; + const text = await resultText(exec, 'shell', 'run_shell_command', { + command: 'echo hello > notes.md', + }); + expect(await readFile(path.join(workspace, 'notes.md'), 'utf8')).toBe( + 'hello\n', + ); + expect(text).not.toContain('record_artifact'); + // The reminder was the only reason this path parsed the command, and + // loading the parser is what grows each resident Worker. + expect(bashParserCalls).toEqual([]); + await exec.close(); + }, + ); + + it.each(['', '1'])( + 'does not claim a written or edited Markdown file was recorded (QWEN_CODE_ENABLE_ARTIFACT=%j)', + async (flag) => { + vi.stubEnv('QWEN_CODE_ENABLE_ARTIFACT', flag); + const exec = ManagedToolExecutor.forWorkspace( + workspace, + 'session-artifact', + ); + const file = path.join(workspace, 'report.md'); + const written = await resultText(exec, 'write', 'write_file', { + file_path: file, + content: '# Report\n', + }); + const edited = await resultText(exec, 'edit', 'edit', { + file_path: file, + old_string: '# Report', + new_string: '# Final report', + }); + expect(await readFile(file, 'utf8')).toBe('# Final report\n'); + const claim = formatRecordArtifactReminder('report.md'); + expect(written).not.toContain(claim); + expect(edited).not.toContain(claim); + await exec.close(); + }, + ); + }, +); + // The route-level fixtures cannot express these geometries (a Session root is // always a deep unique tmpdir path), so the anchoring invariants are pinned // against the function directly. diff --git a/packages/cli/src/serve/managed-runtime-tool-executor.ts b/packages/cli/src/serve/managed-runtime-tool-executor.ts index 247dc71f550..36823a3959a 100644 --- a/packages/cli/src/serve/managed-runtime-tool-executor.ts +++ b/packages/cli/src/serve/managed-runtime-tool-executor.ts @@ -2809,6 +2809,10 @@ export function createManagedToolSet( // tool would give a managed Session a tool the same setting hides // elsewhere. todoWriteEnabled: true, + // These tools have no record_artifact and their results carry no artifact + // metadata to the host. Off, a Shell call also no longer parses its + // command only to decide on the record_artifact reminder. + artifactRecordingSupported: false, }); registerSessionProjectDir(sessionId, config.storage.getProjectDir()); return { diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index 0fb0db20225..f3645cb6efd 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -6876,6 +6876,8 @@ describe('Server Config (config.ts)', () => { afterEach(restoreEnv); const interactiveCli = { interactive: true, sdkMode: false }; + // A host with no record_artifact tool and no artifact metadata route. + const workerHost = { artifactRecordingSupported: false }; // `record` is the expected isRecordArtifactEnabled(), where asserted. it.each` title | env | params | artifact | record @@ -6887,6 +6889,8 @@ describe('Server Config (config.ts)', () => { ${'keeps the Artifact tool disabled for daemon CLI env enablement'} | ${{ QWEN_CODE_ENABLE_ARTIFACT: '1' }} | ${DAEMON} | ${false} | ${true} ${'lets daemon sessions record metadata by default without publishing'} | ${{}} | ${DAEMON} | ${false} | ${true} ${'lets QWEN_CODE_ENABLE_ARTIFACT force-enable interactive CLI use'} | ${{ QWEN_CODE_ENABLE_ARTIFACT: '1' }} | ${{ ...interactiveCli, artifactEnabled: false }} | ${true} | ${undefined} + ${'keeps recording off for hosts that do not support it'} | ${{}} | ${{ ...DAEMON, ...workerHost }} | ${false} | ${false} + ${'keeps recording off for those hosts even when force-enabled'} | ${{ QWEN_CODE_ENABLE_ARTIFACT: '1' }} | ${{ ...interactiveCli, ...workerHost }} | ${true} | ${false} `('%s', ({ env, params, artifact, record }) => { Object.assign(process.env, env); const config = makeConfig(params); diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 822927abcbf..0649d3e5e47 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -1448,6 +1448,12 @@ export interface ConfigParameters { /** Consent gate for the propose_goal tool; see ProposeGoalTool. */ modelProposedGoals?: ModelProposedGoalsMode; artifactEnabled?: boolean; + /** + * False for hosts that expose no `record_artifact` tool and forward no + * artifact metadata, such as tool workers. Unlike `artifactEnabled`, the + * artifact env flags cannot turn recording back on. + */ + artifactRecordingSupported?: boolean; artifactAutoOpen?: boolean; artifactPublisher?: 'local' | 'host' | 'oss'; artifactHost?: ArtifactHostConfig; @@ -3196,6 +3202,7 @@ export class Config { private readonly unifiedAgentLoopEnabled: boolean = false; private readonly agentCollaborationEnabled: boolean = false; private readonly artifactEnabled: boolean = true; + private readonly artifactRecordingSupported: boolean = true; private artifactSnapshotsEnabled = false; private readonly artifactAutoOpen: boolean = true; private readonly artifactPublisher: 'local' | 'host' | 'oss' = 'local'; @@ -3651,6 +3658,7 @@ export class Config { this.unifiedAgentLoopEnabled = params.unifiedAgentLoopEnabled ?? false; this.agentCollaborationEnabled = params.agentCollaborationEnabled ?? false; this.artifactEnabled = params.artifactEnabled ?? true; + this.artifactRecordingSupported = params.artifactRecordingSupported ?? true; this.artifactAutoOpen = params.artifactAutoOpen ?? true; this.artifactPublisher = params.artifactPublisher ?? 'local'; this.artifactHost = params.artifactHost; @@ -10576,6 +10584,7 @@ export class Config { } isRecordArtifactEnabled(): boolean { + if (!this.artifactRecordingSupported) return false; if (process.env['QWEN_CODE_DISABLE_ARTIFACT'] === '1') return false; if (this.sdkMode) return false; if (process.env['QWEN_CODE_ENABLE_ARTIFACT'] === '1') return true; diff --git a/packages/core/src/services/execution-worker.ts b/packages/core/src/services/execution-worker.ts index f27933f505e..4d8a61250ff 100644 --- a/packages/core/src/services/execution-worker.ts +++ b/packages/core/src/services/execution-worker.ts @@ -40,9 +40,9 @@ export function createExecutionWorkerEnvironment( deferTelemetryInitialization: true, usageStatisticsEnabled: false, mcpServers: {}, + // The host does not register worker artifacts. + artifactRecordingSupported: false, }); - // The host does not register worker artifacts, regardless of image env flags. - config.isRecordArtifactEnabled = () => false; if (options.outputDirectory) { config.storage.getProjectTempDir = () => options.outputDirectory!; config.storage.getToolResultsDir = () => options.outputDirectory!;