diff --git a/core/src/code_executors/code_execution_utils.ts b/core/src/code_executors/code_execution_utils.ts index a2a80bb8a..55de6b3a9 100644 --- a/core/src/code_executors/code_execution_utils.ts +++ b/core/src/code_executors/code_execution_utils.ts @@ -114,6 +114,20 @@ export function getEncodedFileContent(data: string): string { return isBase64Encoded(data) ? data : base64Encode(data); } +/** + * Returns the file content as base64, honoring the file's declared + * `contentEncoding`. Content with no declared encoding is assumed to already be + * base64, which is what `AgentEngineSandboxCodeExecutor` produces. + * + * @param file The file whose content to encode. + * @return The file content as base64-encoded bytes. + */ +export function toBase64Content(file: File): string { + return file.contentEncoding === FileContentEncoding.UTF8 + ? base64Encode(file.content) + : file.content; +} + // Type to be used for regex matching of code blocks. interface CodeGroupMatch { groups?: {prefix?: string; codeStr?: string}; diff --git a/core/src/common.ts b/core/src/common.ts index 23f628165..f251ebf15 100644 --- a/core/src/common.ts +++ b/core/src/common.ts @@ -73,6 +73,7 @@ export type { SaveArtifactRequest, } from './artifacts/base_artifact_service.js'; export {InMemoryArtifactService} from './artifacts/in_memory_artifact_service.js'; +export {ScopedArtifactService} from './artifacts/scoped_artifact_service.js'; export type { SessionArtifactService, SessionLoadArtifactRequest, @@ -302,6 +303,10 @@ export type {SkillRegistry} from './skills/skill_registry.js'; export {ListSkillsTool} from './tools/skill/list_skills_tool.js'; export {LoadSkillResourceTool} from './tools/skill/load_skill_resource_tool.js'; export {LoadSkillTool} from './tools/skill/load_skill_tool.js'; +export type { + SavedOutputFile, + SkillScriptResponse, +} from './tools/skill/script_output_utils.js'; export {SearchSkillsTool} from './tools/skill/search_skills_tool.js'; export {SkillToolset} from './tools/skill/skill_toolset.js'; diff --git a/core/src/tools/skill/run_skill_inline_script_tool.ts b/core/src/tools/skill/run_skill_inline_script_tool.ts index 1c0ad2ea2..e3269ba3e 100644 --- a/core/src/tools/skill/run_skill_inline_script_tool.ts +++ b/core/src/tools/skill/run_skill_inline_script_tool.ts @@ -9,8 +9,8 @@ import {Context} from '../../agents/context.js'; import {isLlmAgent} from '../../agents/llm_agent.js'; import {CodeExecutionLanguage} from '../../code_executors/code_execution_utils.js'; import {experimental} from '../../utils/experimental.js'; -import {materializeFiles} from '../../utils/file_utils.js'; import {BaseTool, RunAsyncToolRequest} from '../base_tool.js'; +import {saveScriptOutputs} from './script_output_utils.js'; import {SkillToolset} from './skill_toolset.js'; /** @@ -140,10 +140,7 @@ export class RunSkillInlineScriptTool extends BaseTool { }, }); - // Final filename could be different if there was a collision, so update the result. - result.outputFiles = await materializeFiles(result.outputFiles); - - return result; + return saveScriptOutputs(toolContext, result); } catch (e: unknown) { return { error: `Failed to execute inline script: ${(e as Error).message}`, diff --git a/core/src/tools/skill/run_skill_script_tool.ts b/core/src/tools/skill/run_skill_script_tool.ts index 87e7b7a8d..903102d37 100644 --- a/core/src/tools/skill/run_skill_script_tool.ts +++ b/core/src/tools/skill/run_skill_script_tool.ts @@ -17,8 +17,8 @@ import { getMimeTypeAndEncoding, getScriptLanguageByExtension, } from '../../utils/file_extension_utils.js'; -import {materializeFiles} from '../../utils/file_utils.js'; import {BaseTool, RunAsyncToolRequest} from '../base_tool.js'; +import {saveScriptOutputs} from './script_output_utils.js'; import {SkillToolset} from './skill_toolset.js'; @experimental @@ -141,10 +141,7 @@ export class RunSkillScriptTool extends BaseTool { }, }); - // Final filename could be different if there was a collision, so update the result. - result.outputFiles = await materializeFiles(result.outputFiles); - - return result; + return saveScriptOutputs(toolContext, result); } catch (e: unknown) { return { error: `Failed to execute script '${scriptPath}': ${(e as Error).message}`, diff --git a/core/src/tools/skill/script_output_utils.ts b/core/src/tools/skill/script_output_utils.ts new file mode 100644 index 000000000..e3a9147da --- /dev/null +++ b/core/src/tools/skill/script_output_utils.ts @@ -0,0 +1,100 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import {Context} from '../../agents/context.js'; +import { + CodeExecutionResult, + toBase64Content, +} from '../../code_executors/code_execution_utils.js'; +import {logger} from '../../utils/logger.js'; + +/** + * An output file produced by a skill script and saved to the artifact service. + */ +export interface SavedOutputFile { + /** Artifact filename the file was saved under. */ + name: string; + mimeType: string; +} + +/** Response returned by the skill script execution tools. */ +export interface SkillScriptResponse { + stdout: string; + stderr: string; + /** + * Output files produced by the script. File bytes are deliberately omitted; + * load them from the artifact service (e.g. the `load_artifacts` tool). + */ + outputFiles: SavedOutputFile[]; + /** Set when output files could not be persisted. */ + warning?: string; +} + +/** + * Saves the output files of a skill script execution to the artifact service + * and returns a model-facing summary that never contains file bytes. + * + * When no artifact service is configured, the files cannot be persisted; the + * produced filenames are still reported alongside an explicit warning so the + * loss is never silent. + * + * @param toolContext The tool context owning the session's artifact service. + * @param result The result returned by the code executor. + * @return The model-facing response for the skill script tools. + */ +export async function saveScriptOutputs( + toolContext: Context, + {stdout, stderr, outputFiles}: CodeExecutionResult, +): Promise { + const names = outputFiles.map(({name, mimeType}) => ({name, mimeType})); + + if ( + outputFiles.length > 0 && + !toolContext.invocationContext.artifactService + ) { + const warning = + `No artifact service is configured; ${outputFiles.length} output ` + + `file(s) produced by the script were discarded.`; + logger.warn(warning); + return {stdout, stderr, outputFiles: names, warning}; + } + + const outcomes = await Promise.allSettled( + outputFiles.map((file) => + toolContext.saveArtifact(file.name, { + inlineData: {data: toBase64Content(file), mimeType: file.mimeType}, + }), + ), + ); + + const saved: SavedOutputFile[] = []; + const failed: string[] = []; + outcomes.forEach((outcome, index) => { + if (outcome.status === 'fulfilled') { + saved.push(names[index]); + return; + } + const {name} = names[index]; + failed.push(name); + logger.warn( + `Failed to save output file '${name}' to the artifact service.`, + outcome.reason, + ); + }); + + if (failed.length === 0) { + return {stdout, stderr, outputFiles: saved}; + } + + return { + stdout, + stderr, + outputFiles: saved, + warning: + `Failed to save ${failed.length} of ${outputFiles.length} output ` + + `file(s) to the artifact service: ${failed.join(', ')}.`, + }; +} diff --git a/core/src/utils/file_utils.ts b/core/src/utils/file_utils.ts index 54dc54a04..b62877abb 100644 --- a/core/src/utils/file_utils.ts +++ b/core/src/utils/file_utils.ts @@ -9,15 +9,21 @@ import * as path from 'node:path'; import {File} from '../code_executors/code_execution_utils.js'; /** - * Creates files with the given paths in the current working directory. - * @param files The files to materialize. + * Creates files with the given paths under `dir`. + * + * A name that collides with an existing file is written with a `_2`, `_3`, … + * suffix, and `file.name` is updated in place to the name actually used. + * + * @param files The files to materialize. Mutated: see above. + * @param dir The directory to create the files in. Required: an implicit + * default would write to whichever directory the host process happens to + * be running in. */ export async function materializeFiles( files: File[], - dir = process.cwd(), -): Promise { + dir: string, +): Promise { const resolvedBaseDir = path.resolve(dir); - const createdFiles: File[] = []; for (const file of files) { const fullPath = path.resolve(dir, file.name); @@ -62,14 +68,7 @@ export async function materializeFiles( finalPath, Buffer.from(file.content, file.contentEncoding), ); - - createdFiles.push({ - ...file, - name: path.relative(dir, finalPath), - }); } - - return createdFiles; } export const EXTENSION_TO_MIME_TYPE: Record = { diff --git a/core/test/code_executors/code_execution_utils_test.ts b/core/test/code_executors/code_execution_utils_test.ts index 91d469142..3fc5f0b7d 100644 --- a/core/test/code_executors/code_execution_utils_test.ts +++ b/core/test/code_executors/code_execution_utils_test.ts @@ -14,6 +14,7 @@ import { convertCodeExecutionParts, extractCodeAndTruncateContent, getEncodedFileContent, + toBase64Content, } from '../../src/code_executors/code_execution_utils.js'; import {base64Encode} from '../../src/utils/env_aware_utils.js'; @@ -39,6 +40,46 @@ describe('getEncodedFileContent', () => { }); }); +// --------------------------------------------------------------------------- +// toBase64Content +// --------------------------------------------------------------------------- +describe('toBase64Content', () => { + it('base64-encodes content declared as utf-8', () => { + expect( + toBase64Content({ + name: 'out.txt', + content: 'hello', + contentEncoding: FileContentEncoding.UTF8, + mimeType: 'text/plain', + }), + ).toBe(base64Encode('hello')); + }); + + it('returns content declared as base64 unchanged', () => { + expect( + toBase64Content({ + name: 'out.png', + content: 'aGVsbG8=', + contentEncoding: FileContentEncoding.BASE64, + mimeType: 'image/png', + }), + ).toBe('aGVsbG8='); + }); + + it('treats content with no declared encoding as base64', () => { + // AgentEngineSandboxCodeExecutor omits contentEncoding on already-base64 + // content; unlike getEncodedFileContent this must not sniff the payload, + // because plain text such as 'hello' is itself valid base64. + expect( + toBase64Content({ + name: 'out.bin', + content: 'aGVsbG8=', + mimeType: 'application/octet-stream', + }), + ).toBe('aGVsbG8='); + }); +}); + // --------------------------------------------------------------------------- // buildExecutableCodePart // --------------------------------------------------------------------------- diff --git a/core/test/tools/skills/run_skill_inline_script_tool_test.ts b/core/test/tools/skills/run_skill_inline_script_tool_test.ts index 36664b070..5bafbc2ea 100644 --- a/core/test/tools/skills/run_skill_inline_script_tool_test.ts +++ b/core/test/tools/skills/run_skill_inline_script_tool_test.ts @@ -10,21 +10,20 @@ import { CodeExecutionResult, Context, ExecuteCodeParams, - File, FileContentEncoding, + InMemoryArtifactService, InvocationContext, LlmAgent, RunSkillInlineScriptErrorCode, RunSkillInlineScriptTool, + ScopedArtifactService, + SessionArtifactService, + SkillScriptResponse, SkillToolset, } from '@google/adk'; -import {describe, expect, it, vi} from 'vitest'; +import * as fs from 'node:fs/promises'; +import {describe, expect, it} from 'vitest'; import {ToolConfirmation} from '../../../src/tools/tool_confirmation.js'; -import {materializeFiles} from '../../../src/utils/file_utils.js'; - -vi.mock('../../../src/utils/file_utils.js', () => ({ - materializeFiles: vi.fn().mockImplementation((files) => files), -})); class MockCodeExecutor extends BaseCodeExecutor { mockResult: CodeExecutionResult = { @@ -58,6 +57,7 @@ describe('RunSkillInlineScriptTool', () => { options: { functionCallId?: string; toolConfirmation?: ToolConfirmation; + artifactService?: SessionArtifactService; } = {}, ): Context { const agentObj: Record = {name: agentName}; @@ -70,6 +70,7 @@ describe('RunSkillInlineScriptTool', () => { invocationContext: { session: {state: {}}, agent: agentObj as unknown as LlmAgent, + artifactService: options.artifactService, } as unknown as InvocationContext, functionCallId: options.functionCallId, toolConfirmation: options.toolConfirmation, @@ -83,6 +84,32 @@ describe('RunSkillInlineScriptTool', () => { return new ToolConfirmation({confirmed: true}); } + function createSessionArtifactService(): SessionArtifactService { + return new ScopedArtifactService( + new InMemoryArtifactService(), + 'test-app', + 'test-user', + 'test-session', + ); + } + + function executorProducing(name: string): MockCodeExecutor { + const mockExecutor = new MockCodeExecutor(); + mockExecutor.mockResult = { + stdout: 'script stdout', + stderr: '', + outputFiles: [ + { + name, + content: 'hello', + contentEncoding: FileContentEncoding.UTF8, + mimeType: 'text/plain', + }, + ], + }; + return mockExecutor; + } + it('returns error if script content is missing', async () => { const toolset = new SkillToolset([]); const tool = new RunSkillInlineScriptTool(toolset); @@ -220,34 +247,84 @@ describe('RunSkillInlineScriptTool', () => { }); }); - it('calls materializeFiles with output files from executor', async () => { - const mockExecutor = new MockCodeExecutor(); - const testFile: File = { - name: 'output.txt', - content: 'hello', - contentEncoding: FileContentEncoding.UTF8, - mimeType: 'text/plain', - }; - mockExecutor.mockResult = { - stdout: '', + it('saves script output files to the artifact service and omits file bytes from the response', async () => { + const mockExecutor = executorProducing('output.txt'); + const artifactService = createSessionArtifactService(); + const toolContext = createMockContext('test-agent', undefined, { + toolConfirmation: confirmed(), + artifactService, + }); + const toolset = new SkillToolset([], {codeExecutor: mockExecutor}); + const tool = new RunSkillInlineScriptTool(toolset); + + const result = (await tool.runAsync({ + args: { + script_content: 'console.log("test");', + language: CodeExecutionLanguage.JAVASCRIPT, + }, + toolContext, + })) as SkillScriptResponse; + + expect(result).toEqual({ + stdout: 'script stdout', stderr: '', - outputFiles: [testFile], - }; + outputFiles: [{name: 'output.txt', mimeType: 'text/plain'}], + }); + const artifact = await artifactService.loadArtifact({ + filename: 'output.txt', + }); + expect(artifact?.inlineData?.data).toBe( + Buffer.from('hello', 'utf-8').toString('base64'), + ); + expect(toolContext.actions.artifactDelta).toEqual({'output.txt': 0}); + }); + it('does not write script output files to the process working directory', async () => { + const mockExecutor = executorProducing('cwd_regression_inline_output.txt'); const toolset = new SkillToolset([], {codeExecutor: mockExecutor}); const tool = new RunSkillInlineScriptTool(toolset); + const before = (await fs.readdir(process.cwd())).sort(); - await tool.runAsync({ + const result = (await tool.runAsync({ args: { script_content: 'console.log("test");', language: CodeExecutionLanguage.JAVASCRIPT, }, toolContext: createMockContext('test-agent', undefined, { toolConfirmation: confirmed(), + artifactService: createSessionArtifactService(), }), - }); + })) as SkillScriptResponse; + + expect((await fs.readdir(process.cwd())).sort()).toEqual(before); + expect(result.outputFiles).toEqual([ + {name: 'cwd_regression_inline_output.txt', mimeType: 'text/plain'}, + ]); + }); + + it('reports produced files with a warning when no artifact service is configured', async () => { + const mockExecutor = executorProducing('unsaved_output.txt'); + const toolset = new SkillToolset([], {codeExecutor: mockExecutor}); + const tool = new RunSkillInlineScriptTool(toolset); - expect(materializeFiles).toHaveBeenCalledWith([testFile]); + const result = (await tool.runAsync({ + args: { + script_content: 'console.log("test");', + language: CodeExecutionLanguage.JAVASCRIPT, + }, + toolContext: createMockContext('test-agent', undefined, { + toolConfirmation: confirmed(), + }), + })) as SkillScriptResponse; + + expect(result).toEqual({ + stdout: 'script stdout', + stderr: '', + outputFiles: [{name: 'unsaved_output.txt', mimeType: 'text/plain'}], + warning: + 'No artifact service is configured; 1 output file(s) produced by the ' + + 'script were discarded.', + }); }); it('successfully passes array arguments to code executor', async () => { diff --git a/core/test/tools/skills/run_skill_script_tool_test.ts b/core/test/tools/skills/run_skill_script_tool_test.ts index 15c796b05..9c8e004dd 100644 --- a/core/test/tools/skills/run_skill_script_tool_test.ts +++ b/core/test/tools/skills/run_skill_script_tool_test.ts @@ -10,19 +10,19 @@ import { CodeExecutionResult, Context, ExecuteCodeParams, - File, + FileContentEncoding, + InMemoryArtifactService, InvocationContext, LlmAgent, RunSkillScriptTool, + ScopedArtifactService, + SessionArtifactService, Skill, + SkillScriptResponse, SkillToolset, } from '@google/adk'; -import {describe, expect, it, vi} from 'vitest'; -import {materializeFiles} from '../../../src/utils/file_utils.js'; - -vi.mock('../../../src/utils/file_utils.js', () => ({ - materializeFiles: vi.fn(), -})); +import * as fs from 'node:fs/promises'; +import {describe, expect, it} from 'vitest'; class MockCodeExecutor extends BaseCodeExecutor { mockResult: CodeExecutionResult = { @@ -53,6 +53,7 @@ describe('RunSkillScriptTool', () => { function createMockContext( agentName = 'test-agent', agentExecutor?: BaseCodeExecutor, + artifactService?: SessionArtifactService, ): Context { const agentObj: Record = {name: agentName}; if (agentExecutor) { @@ -64,10 +65,37 @@ describe('RunSkillScriptTool', () => { invocationContext: { session: {state: {}}, agent: agentObj as unknown as LlmAgent, + artifactService, } as unknown as InvocationContext, }); } + function createSessionArtifactService(): SessionArtifactService { + return new ScopedArtifactService( + new InMemoryArtifactService(), + 'test-app', + 'test-user', + 'test-session', + ); + } + + function executorProducing(name: string): MockCodeExecutor { + const mockExecutor = new MockCodeExecutor(); + mockExecutor.mockResult = { + stdout: 'script stdout', + stderr: '', + outputFiles: [ + { + name, + content: 'hello', + contentEncoding: FileContentEncoding.UTF8, + mimeType: 'text/plain', + }, + ], + }; + return mockExecutor; + } + const mockSkill: Skill = { frontmatter: { name: 'test-skill', @@ -204,28 +232,74 @@ describe('RunSkillScriptTool', () => { expect(binaryFile?.contentEncoding).toBe('base64'); }); - it('calls materializeFiles with output files from executor', async () => { - const mockExecutor = new MockCodeExecutor(); - const testFile = { - name: 'output.txt', - content: 'hello', - contentEncoding: 'utf8', - mimeType: 'text/plain', - } as File; - mockExecutor.mockResult = { - stdout: '', + it('saves script output files to the artifact service and omits file bytes from the response', async () => { + const mockExecutor = executorProducing('output.txt'); + const artifactService = createSessionArtifactService(); + const toolContext = createMockContext( + 'test-agent', + undefined, + artifactService, + ); + const toolset = new SkillToolset([mockSkill], {codeExecutor: mockExecutor}); + const tool = new RunSkillScriptTool(toolset); + + const result = (await tool.runAsync({ + args: {skill_name: 'test-skill', script_path: 'scripts/setup.js'}, + toolContext, + })) as SkillScriptResponse; + + expect(result).toEqual({ + stdout: 'script stdout', stderr: '', - outputFiles: [testFile], - }; + outputFiles: [{name: 'output.txt', mimeType: 'text/plain'}], + }); + const artifact = await artifactService.loadArtifact({ + filename: 'output.txt', + }); + expect(artifact?.inlineData?.data).toBe( + Buffer.from('hello', 'utf-8').toString('base64'), + ); + expect(toolContext.actions.artifactDelta).toEqual({'output.txt': 0}); + }); + it('does not write script output files to the process working directory', async () => { + const mockExecutor = executorProducing('cwd_regression_output.txt'); const toolset = new SkillToolset([mockSkill], {codeExecutor: mockExecutor}); const tool = new RunSkillScriptTool(toolset); + const before = (await fs.readdir(process.cwd())).sort(); - await tool.runAsync({ + const result = (await tool.runAsync({ + args: {skill_name: 'test-skill', script_path: 'scripts/setup.js'}, + toolContext: createMockContext( + 'test-agent', + undefined, + createSessionArtifactService(), + ), + })) as SkillScriptResponse; + + expect((await fs.readdir(process.cwd())).sort()).toEqual(before); + expect(result.outputFiles).toEqual([ + {name: 'cwd_regression_output.txt', mimeType: 'text/plain'}, + ]); + }); + + it('reports produced files with a warning when no artifact service is configured', async () => { + const mockExecutor = executorProducing('unsaved_output.txt'); + const toolset = new SkillToolset([mockSkill], {codeExecutor: mockExecutor}); + const tool = new RunSkillScriptTool(toolset); + + const result = (await tool.runAsync({ args: {skill_name: 'test-skill', script_path: 'scripts/setup.js'}, toolContext: createMockContext(), - }); + })) as SkillScriptResponse; - expect(materializeFiles).toHaveBeenCalledWith([testFile]); + expect(result).toEqual({ + stdout: 'script stdout', + stderr: '', + outputFiles: [{name: 'unsaved_output.txt', mimeType: 'text/plain'}], + warning: + 'No artifact service is configured; 1 output file(s) produced by the ' + + 'script were discarded.', + }); }); }); diff --git a/core/test/tools/skills/script_output_utils_test.ts b/core/test/tools/skills/script_output_utils_test.ts new file mode 100644 index 000000000..c98eb2b25 --- /dev/null +++ b/core/test/tools/skills/script_output_utils_test.ts @@ -0,0 +1,278 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import {afterEach, describe, expect, it, vi} from 'vitest'; +import {Context} from '../../../src/agents/context.js'; +import {InvocationContext} from '../../../src/agents/invocation_context.js'; +import {LlmAgent} from '../../../src/agents/llm_agent.js'; +import {InMemoryArtifactService} from '../../../src/artifacts/in_memory_artifact_service.js'; +import {ScopedArtifactService} from '../../../src/artifacts/scoped_artifact_service.js'; +import {SessionArtifactService} from '../../../src/artifacts/session_artifact_service.js'; +import { + File, + FileContentEncoding, +} from '../../../src/code_executors/code_execution_utils.js'; +import {PluginManager} from '../../../src/plugins/plugin_manager.js'; +import {createSession} from '../../../src/sessions/session.js'; +import {saveScriptOutputs} from '../../../src/tools/skill/script_output_utils.js'; +import {logger} from '../../../src/utils/logger.js'; + +function createSessionArtifactService(): SessionArtifactService { + return new ScopedArtifactService( + new InMemoryArtifactService(), + 'test-app', + 'test-user', + 'test-session', + ); +} + +function createContext(artifactService?: SessionArtifactService): Context { + return new Context({ + invocationContext: new InvocationContext({ + invocationId: 'test-invocation', + agent: new LlmAgent({name: 'test_agent'}), + session: createSession({id: 'test-session', appName: 'test-app'}), + pluginManager: new PluginManager(), + artifactService, + }), + }); +} + +function textFile(name: string, content: string): File { + return { + name, + content, + contentEncoding: FileContentEncoding.UTF8, + mimeType: 'text/plain', + }; +} + +/** Reads back the base64 payload an artifact was saved with. */ +async function loadInlineData( + artifactService: SessionArtifactService, + filename: string, +): Promise { + const artifact = await artifactService.loadArtifact({filename}); + const data = artifact?.inlineData?.data; + if (data === undefined) { + expect.fail(`Artifact '${filename}' has no inline data.`); + } + return data; +} + +describe('saveScriptOutputs', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('returns an empty file list and saves nothing when the script produced no files', async () => { + const artifactService = createSessionArtifactService(); + const toolContext = createContext(artifactService); + + const response = await saveScriptOutputs(toolContext, { + stdout: 'done', + stderr: '', + outputFiles: [], + }); + + expect(response).toEqual({stdout: 'done', stderr: '', outputFiles: []}); + expect(await artifactService.listArtifactKeys()).toEqual([]); + expect(toolContext.actions.artifactDelta).toEqual({}); + }); + + it('does not warn about a missing artifact service when the script produced no files', async () => { + const warnSpy = vi.spyOn(logger, 'warn').mockImplementation(() => {}); + + const response = await saveScriptOutputs(createContext(), { + stdout: 'done', + stderr: '', + outputFiles: [], + }); + + expect(response).toEqual({stdout: 'done', stderr: '', outputFiles: []}); + expect(warnSpy).not.toHaveBeenCalled(); + }); + + it('saves each output file to the artifact service and returns only names and mime types', async () => { + const artifactService = createSessionArtifactService(); + const toolContext = createContext(artifactService); + + const response = await saveScriptOutputs(toolContext, { + stdout: 'script stdout', + stderr: 'script stderr', + outputFiles: [ + textFile('report.csv', 'a,b'), + { + name: 'chart.png', + content: 'AAEC', + contentEncoding: FileContentEncoding.BASE64, + mimeType: 'image/png', + }, + ], + }); + + expect(response).toEqual({ + stdout: 'script stdout', + stderr: 'script stderr', + outputFiles: [ + {name: 'report.csv', mimeType: 'text/plain'}, + {name: 'chart.png', mimeType: 'image/png'}, + ], + }); + // The model must never receive the file bytes. + expect(Object.keys(response.outputFiles[0])).toEqual(['name', 'mimeType']); + expect((await artifactService.listArtifactKeys()).sort()).toEqual([ + 'chart.png', + 'report.csv', + ]); + }); + + it('base64-encodes utf-8 file content before saving', async () => { + const artifactService = createSessionArtifactService(); + const toolContext = createContext(artifactService); + + await saveScriptOutputs(toolContext, { + stdout: '', + stderr: '', + outputFiles: [textFile('out.txt', 'hello')], + }); + + expect(await loadInlineData(artifactService, 'out.txt')).toBe( + Buffer.from('hello', 'utf-8').toString('base64'), + ); + }); + + it('passes base64 file content through unchanged', async () => { + const artifactService = createSessionArtifactService(); + const toolContext = createContext(artifactService); + + await saveScriptOutputs(toolContext, { + stdout: '', + stderr: '', + outputFiles: [ + { + name: 'out.bin', + content: 'aGVsbG8=', + contentEncoding: FileContentEncoding.BASE64, + mimeType: 'application/octet-stream', + }, + ], + }); + + expect(await loadInlineData(artifactService, 'out.bin')).toBe('aGVsbG8='); + }); + + it('treats content with no declared encoding as base64', async () => { + const artifactService = createSessionArtifactService(); + const toolContext = createContext(artifactService); + + await saveScriptOutputs(toolContext, { + stdout: '', + stderr: '', + outputFiles: [ + {name: 'out.png', content: 'aGVsbG8=', mimeType: 'image/png'}, + ], + }); + + expect(await loadInlineData(artifactService, 'out.png')).toBe('aGVsbG8='); + }); + + it('records an artifact delta for each saved file', async () => { + const artifactService = createSessionArtifactService(); + const toolContext = createContext(artifactService); + + await saveScriptOutputs(toolContext, { + stdout: '', + stderr: '', + outputFiles: [textFile('a.txt', 'a'), textFile('b.txt', 'b')], + }); + + expect(toolContext.actions.artifactDelta).toEqual({ + 'a.txt': 0, + 'b.txt': 0, + }); + }); + + it('saves a repeated filename as a new artifact version', async () => { + const artifactService = createSessionArtifactService(); + + await saveScriptOutputs(createContext(artifactService), { + stdout: '', + stderr: '', + outputFiles: [textFile('report.csv', 'first')], + }); + const secondContext = createContext(artifactService); + const response = await saveScriptOutputs(secondContext, { + stdout: '', + stderr: '', + outputFiles: [textFile('report.csv', 'second')], + }); + + expect(response.outputFiles).toEqual([ + {name: 'report.csv', mimeType: 'text/plain'}, + ]); + expect(secondContext.actions.artifactDelta).toEqual({'report.csv': 1}); + expect(await artifactService.listVersions('report.csv')).toEqual([0, 1]); + }); + + it('reports produced files with a warning when no artifact service is configured', async () => { + const warnSpy = vi.spyOn(logger, 'warn').mockImplementation(() => {}); + const toolContext = createContext(); + + const response = await saveScriptOutputs(toolContext, { + stdout: 'script stdout', + stderr: 'script stderr', + outputFiles: [textFile('out.txt', 'hello')], + }); + + expect(response).toEqual({ + stdout: 'script stdout', + stderr: 'script stderr', + outputFiles: [{name: 'out.txt', mimeType: 'text/plain'}], + warning: + 'No artifact service is configured; 1 output file(s) produced by the ' + + 'script were discarded.', + }); + expect(warnSpy).toHaveBeenCalledWith(response.warning); + }); + + it('returns the saved subset with a warning when an artifact save fails', async () => { + const warnSpy = vi.spyOn(logger, 'warn').mockImplementation(() => {}); + const artifactService = createSessionArtifactService(); + const reason = new Error('artifact backend unavailable'); + const save = artifactService.saveArtifact.bind(artifactService); + vi.spyOn(artifactService, 'saveArtifact').mockImplementation((request) => + request.filename === 'b.txt' ? Promise.reject(reason) : save(request), + ); + const toolContext = createContext(artifactService); + + const response = await saveScriptOutputs(toolContext, { + stdout: 'script stdout', + stderr: 'script stderr', + outputFiles: [ + textFile('a.txt', 'a'), + textFile('b.txt', 'b'), + textFile('c.txt', 'c'), + ], + }); + + expect(response).toEqual({ + stdout: 'script stdout', + stderr: 'script stderr', + outputFiles: [ + {name: 'a.txt', mimeType: 'text/plain'}, + {name: 'c.txt', mimeType: 'text/plain'}, + ], + warning: + 'Failed to save 1 of 3 output file(s) to the artifact service: b.txt.', + }); + expect(warnSpy).toHaveBeenCalledWith( + "Failed to save output file 'b.txt' to the artifact service.", + reason, + ); + expect(toolContext.actions.artifactDelta).toEqual({'a.txt': 0, 'c.txt': 0}); + }); +}); diff --git a/output.txt b/output.txt deleted file mode 100644 index b6fc4c620..000000000 --- a/output.txt +++ /dev/null @@ -1 +0,0 @@ -hello \ No newline at end of file diff --git a/tests/integration/skills/script_js/agent_test.ts b/tests/integration/skills/script_js/agent_test.ts index b17df2c86..ce8e69af4 100644 --- a/tests/integration/skills/script_js/agent_test.ts +++ b/tests/integration/skills/script_js/agent_test.ts @@ -5,6 +5,8 @@ */ import {exec, spawn} from 'node:child_process'; import * as fs from 'node:fs/promises'; +import * as os from 'node:os'; +import * as path from 'node:path'; import {promisify} from 'node:util'; import {afterAll, beforeAll, describe, expect, it} from 'vitest'; import {normalizeLineEndings, sendInput} from '../../test_case_utils.js'; @@ -13,6 +15,44 @@ const execAsync = promisify(exec); const dirname = process.cwd(); const PROJECT_PATH = `${dirname}/tests/integration/skills/script_js`; const TEST_EXECUTION_TIMEOUT = 60000; +const GENERATED_FILES = [ + 'ephemeral_entanglement.md', + 'index.html', + 'sketch.js', +]; + +/** Directory backing the CLI's file artifact service for this run. */ +let artifactRoot: string; + +/** + * Returns the content of a stored artifact, searched by filename anywhere + * under the artifact root so the test does not depend on the storage layout. + */ +async function findArtifact( + dir: string, + name: string, +): Promise { + for (const entry of await fs.readdir(dir, {withFileTypes: true})) { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) { + const found = await findArtifact(full, name); + if (found !== undefined) { + return found; + } + } else if (entry.name === name) { + return fs.readFile(full, 'utf-8'); + } + } + return undefined; +} + +async function readArtifact(name: string): Promise { + const content = await findArtifact(artifactRoot, name); + if (content === undefined) { + expect.fail(`Artifact '${name}' was not saved to the artifact service.`); + } + return content; +} /** * This integration test verifies that an agent equipped with script execution skills @@ -22,26 +62,35 @@ const TEST_EXECUTION_TIMEOUT = 60000; * 1. Starts the agent by running `npm run start` in the test project directory. * 2. Simulates user interaction by sending a prompt: "Let's create algorithmic art." * 3. Asserts that the agent's response matches the expected output, confirming it claims to have created the art and files. - * 4. Verifies that the expected files (`ephemeral_entanglement.md`, `index.html`, `sketch.js`) were actually generated in the file system. - * 5. Compares the content of these generated files with reference files in the `expected/` directory to ensure correctness. - * 6. Cleans up the generated files and installed dependencies after execution. + * 4. Verifies that the expected files (`ephemeral_entanglement.md`, `index.html`, `sketch.js`) were saved to the artifact service, and that none of them was written into the directory the agent process was started from. + * 5. Compares the content of these saved artifacts with reference files in the `expected/` directory to ensure correctness. + * 6. Cleans up the artifact store and installed dependencies after execution. * - * This test ensures the end-to-end flow of an agent using tools to generate and materialize files based on a high-level request. + * This test ensures the end-to-end flow of an agent using tools to generate files based on a high-level request and persist them where the session can reach them. */ describe('Agent with skills that generates JS script and runs it locally', () => { beforeAll(async () => { + artifactRoot = await fs.mkdtemp(path.join(os.tmpdir(), 'adk-artifacts-')); await execAsync('npm install', {cwd: PROJECT_PATH}); }, TEST_EXECUTION_TIMEOUT); it( 'should run agent with skills successfully', async () => { - const childProcess = spawn('npm', ['run', 'start'], { - cwd: PROJECT_PATH, - shell: true, - }); + // getArtifactServiceFromUri strips the scheme with a plain string split, + // so the path has to follow `file://` directly: a Windows drive letter + // does not survive the leading slash of a canonical `file:///` URL. + const artifactServiceUri = `file://${artifactRoot.split(path.sep).join('/')}`; + const childProcess = spawn( + 'npm', + ['run', 'start', '--', '--artifact_service_uri', artifactServiceUri], + { + cwd: PROJECT_PATH, + shell: true, + }, + ); - let response = await sendInput( + const response = await sendInput( childProcess, 'Let`s create algorithmic art.\n', ); @@ -49,22 +98,21 @@ describe('Agent with skills that generates JS script and runs it locally', () => 'I have created an original algorithmic art piece titled **"Ephemeral Entanglement"**.\n\nFollowing the generative art movement philosophy, I\'ve generated three files for you:\n\n1. **`ephemeral_entanglement.md`**: The algorithmic philosophy detailing the conceptual foundation of this piece. It explores the delicate dance between deterministic forces and stochastic drift, visualizing unseen connections in a dynamic system.\n2. **`index.html`**: The interactive viewer for the generative art. It includes a user interface to adjust parameters like particle count, connection radius, and noise scale, allowing you to explore the algorithm\'s emergent behavior.\n3. **`sketch.js`**: The meticulously crafted p5.js algorithm that brings the philosophy to life. It uses layered Perlin noise to drive a flow field, guiding particles that form ephemeral, glowing bonds when they come into proximity. \n\nYou can view the art by opening the `index.html` file in your web browser. Let the algorithmic dance begin!', ); - response = await sendInput(childProcess, 'exit\n'); - expect(response.toString()).toContain(''); + // Shut the CLI down so the session's artifacts are fully flushed. + await sendInput(childProcess, 'exit\n'); - // verify that files were created and have the expected content - const resultMdFile = await fs.readFile( - `${PROJECT_PATH}/ephemeral_entanglement.md`, - 'utf-8', - ); - const resultScriptFile = await fs.readFile( - `${PROJECT_PATH}/sketch.js`, - 'utf-8', - ); - const resultHtmlFile = await fs.readFile( - `${PROJECT_PATH}/index.html`, - 'utf-8', - ); + // The script's output belongs to the session, not to the directory the + // agent process happens to be running in. + for (const name of GENERATED_FILES) { + await expect(fs.access(`${PROJECT_PATH}/${name}`)).rejects.toThrow( + /ENOENT/, + ); + } + + // verify that the artifacts were saved and have the expected content + const resultMdFile = await readArtifact('ephemeral_entanglement.md'); + const resultScriptFile = await readArtifact('sketch.js'); + const resultHtmlFile = await readArtifact('index.html'); const expectedMdFile = await fs.readFile( `${PROJECT_PATH}/expected/ephemeral_entanglement.md`, @@ -93,12 +141,7 @@ describe('Agent with skills that generates JS script and runs it locally', () => ); afterAll(async () => { - // delete generated files - await fs - .rm(`${PROJECT_PATH}/ephemeral_entanglement.md`, {force: true}) - .catch(() => {}); - await fs.rm(`${PROJECT_PATH}/index.html`, {force: true}).catch(() => {}); - await fs.rm(`${PROJECT_PATH}/sketch.js`, {force: true}).catch(() => {}); + await fs.rm(artifactRoot, {recursive: true, force: true}).catch(() => {}); await fs .rm(`${PROJECT_PATH}/node_modules`, {recursive: true, force: true}) diff --git a/tests/integration/tools/artifact_service_test_utils.ts b/tests/integration/tools/artifact_service_test_utils.ts new file mode 100644 index 000000000..4f8b996ec --- /dev/null +++ b/tests/integration/tools/artifact_service_test_utils.ts @@ -0,0 +1,38 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { + InMemoryArtifactService, + ScopedArtifactService, + SessionArtifactService, +} from '@google/adk'; + +/** + * Builds the session-scoped in-memory artifact service that an invocation + * context carries. + */ +export function createSessionArtifactService(): SessionArtifactService { + return new ScopedArtifactService( + new InMemoryArtifactService(), + 'skill-script-integration-app', + 'skill-script-integration-user', + 'skill-script-integration-session', + ); +} + +/** + * Reads back the decoded text of the latest version of an artifact. + */ +export async function loadArtifactText( + artifactService: SessionArtifactService, + filename: string, +): Promise { + const artifact = await artifactService.loadArtifact({filename}); + const data = artifact?.inlineData?.data; + return data === undefined + ? undefined + : Buffer.from(data, 'base64').toString('utf-8'); +} diff --git a/tests/integration/tools/run_skill_inline_script_tool_test.ts b/tests/integration/tools/run_skill_inline_script_tool_test.ts index fcddc14d0..6e2f55d7d 100644 --- a/tests/integration/tools/run_skill_inline_script_tool_test.ts +++ b/tests/integration/tools/run_skill_inline_script_tool_test.ts @@ -10,6 +10,8 @@ import { Context, InvocationContext, RunSkillInlineScriptTool, + SessionArtifactService, + SkillScriptResponse, SkillToolset, ToolConfirmation, UnsafeLocalCodeExecutor, @@ -17,21 +19,44 @@ import { import * as fs from 'node:fs/promises'; import * as path from 'node:path'; import {describe, expect, it} from 'vitest'; +import { + createSessionArtifactService, + loadArtifactText, +} from './artifact_service_test_utils.js'; + +/** Content written by the output-file scripts under test. */ +const FILE_CONTENT = 'hello from output file'; describe('RunSkillInlineScriptTool Integration with UnsafeLocalCodeExecutor', () => { // These integration tests exercise real code execution, which is gated behind // a human-in-the-loop confirmation. Supply an already-confirmed confirmation // so the tool proceeds to execute (see run_skill_inline_script_tool.ts). - function createMockContext(agentName = 'test-agent') { + function createMockContext( + agentName = 'test-agent', + artifactService?: SessionArtifactService, + ) { return new Context({ invocationContext: { session: {state: {}}, agent: {name: agentName}, + artifactService, } as unknown as InvocationContext, toolConfirmation: new ToolConfirmation({confirmed: true}), }); } + async function cwdContains(filename: string): Promise { + return fs + .access(path.join(process.cwd(), filename)) + .then(() => true) + .catch(() => false); + } + + /** A script that writes `hello from output file` to the given filename. */ + function writeFileScript(filename: string): string { + return `const fs = require('fs'); fs.writeFileSync('${filename}', '${FILE_CONTENT}');`; + } + it('successfully executes a real JavaScript inline script', async () => { const executor = new UnsafeLocalCodeExecutor(); const toolset = new SkillToolset([], {codeExecutor: executor}); @@ -138,42 +163,52 @@ describe('RunSkillInlineScriptTool Integration with UnsafeLocalCodeExecutor', () expect(result.stderr).toContain('some python error'); }); - it('creates files in process.cwd returned from execution', async () => { + it('saves script output files to the artifact service', async () => { const executor = new UnsafeLocalCodeExecutor(); const toolset = new SkillToolset([], {codeExecutor: executor}); const tool = new RunSkillInlineScriptTool(toolset); - + const artifactService = createSessionArtifactService(); + const toolContext = createMockContext('test-agent', artifactService); const testFileName = `test_output_${Date.now()}.txt`; - const testFileContent = 'hello from output file'; const result = (await tool.runAsync({ args: { - script_content: `const fs = require('fs'); fs.writeFileSync('${testFileName}', '${testFileContent}');`, + script_content: writeFileScript(testFileName), language: CodeExecutionLanguage.JAVASCRIPT, }, - toolContext: createMockContext(), - })) as CodeExecutionResult; - - expect(result).toBeDefined(); - expect(result.outputFiles).toBeDefined(); - expect(result.outputFiles?.length).toBeGreaterThan(0); - - const outputFile = result.outputFiles?.find((f) => f.name === testFileName); - expect(outputFile).toBeDefined(); + toolContext, + })) as SkillScriptResponse; + + expect(result.outputFiles).toEqual([ + {name: testFileName, mimeType: 'text/plain'}, + ]); + expect(result.warning).toBeUndefined(); + expect(await loadArtifactText(artifactService, testFileName)).toBe( + FILE_CONTENT, + ); + expect(toolContext.actions.artifactDelta).toEqual({[testFileName]: 0}); + expect(await cwdContains(testFileName)).toBe(false); + }); - // Verify file was created in process.cwd() - const fullPath = path.join(process.cwd(), testFileName); - const exists = await fs - .access(fullPath) - .then(() => true) - .catch(() => false); - expect(exists).toBe(true); + it('reports output files with a warning when no artifact service is configured', async () => { + const executor = new UnsafeLocalCodeExecutor(); + const toolset = new SkillToolset([], {codeExecutor: executor}); + const tool = new RunSkillInlineScriptTool(toolset); + const testFileName = `test_unsaved_output_${Date.now()}.txt`; - const content = await fs.readFile(fullPath, 'utf-8'); - expect(content).toBe(testFileContent); + const result = (await tool.runAsync({ + args: { + script_content: writeFileScript(testFileName), + language: CodeExecutionLanguage.JAVASCRIPT, + }, + toolContext: createMockContext(), + })) as SkillScriptResponse; - // Clean up - await fs.unlink(fullPath); + expect(result.outputFiles).toEqual([ + {name: testFileName, mimeType: 'text/plain'}, + ]); + expect(result.warning).toMatch(/No artifact service is configured/); + expect(await cwdContains(testFileName)).toBe(false); }); it('successfully passes array arguments to a JavaScript inline script', async () => { @@ -212,48 +247,31 @@ describe('RunSkillInlineScriptTool Integration with UnsafeLocalCodeExecutor', () expect(result.stdout).toContain('--flag1 val1 --flag2 val2'); }); - it('handles file collisions by appending a numeric suffix', async () => { + it('creates a new artifact version instead of a renamed file on repeat runs', async () => { const executor = new UnsafeLocalCodeExecutor(); const toolset = new SkillToolset([], {codeExecutor: executor}); const tool = new RunSkillInlineScriptTool(toolset); - + const artifactService = createSessionArtifactService(); const testFileName = `test_inline_output_${Date.now()}.txt`; - const testFileContent = 'hello from output file'; - - // Pre-create the target file to force a collision - const targetFile = path.join(process.cwd(), testFileName); - await fs.writeFile(targetFile, 'existing content'); - + const args = { + script_content: writeFileScript(testFileName), + language: CodeExecutionLanguage.JAVASCRIPT, + }; + + await tool.runAsync({ + args, + toolContext: createMockContext('test-agent', artifactService), + }); const result = (await tool.runAsync({ - args: { - script_content: `const fs = require('fs'); fs.writeFileSync('${testFileName}', '${testFileContent}');`, - language: CodeExecutionLanguage.JAVASCRIPT, - }, - toolContext: createMockContext(), - })) as CodeExecutionResult; - - expect(result).toBeDefined(); - expect(result.outputFiles).toBeDefined(); - - const baseName = path.basename(testFileName, '.txt'); - const expectedName = `${baseName}_2.txt`; - - const outputFile = result.outputFiles?.find((f) => f.name === expectedName); - expect(outputFile).toBeDefined(); - - // Verify collision file was created in process.cwd() - const fullPath = path.join(process.cwd(), expectedName); - const exists = await fs - .access(fullPath) - .then(() => true) - .catch(() => false); - expect(exists).toBe(true); - - const content = await fs.readFile(fullPath, 'utf-8'); - expect(content).toBe(testFileContent); - - // Clean up both files - await fs.unlink(targetFile); - await fs.unlink(fullPath); + args, + toolContext: createMockContext('test-agent', artifactService), + })) as SkillScriptResponse; + + expect(result.outputFiles).toEqual([ + {name: testFileName, mimeType: 'text/plain'}, + ]); + expect(await artifactService.listVersions(testFileName)).toEqual([0, 1]); + const collisionName = `${path.basename(testFileName, '.txt')}_2.txt`; + expect(await cwdContains(collisionName)).toBe(false); }); }); diff --git a/tests/integration/tools/run_skill_script_tool_test.ts b/tests/integration/tools/run_skill_script_tool_test.ts index aa7f0ef6d..5f64bbd58 100644 --- a/tests/integration/tools/run_skill_script_tool_test.ts +++ b/tests/integration/tools/run_skill_script_tool_test.ts @@ -9,7 +9,9 @@ import { Context, InvocationContext, RunSkillScriptTool, + SessionArtifactService, Skill, + SkillScriptResponse, SkillToolset, UnsafeLocalCodeExecutor, } from '@google/adk'; @@ -17,6 +19,10 @@ import * as fs from 'node:fs/promises'; import * as os from 'node:os'; import * as path from 'node:path'; import {describe, expect, it} from 'vitest'; +import { + createSessionArtifactService, + loadArtifactText, +} from './artifact_service_test_utils.js'; const IS_WINDOWS = os.platform() === 'win32'; const IS_UNIX = os.platform() === 'linux' || os.platform() === 'darwin'; @@ -27,16 +33,39 @@ const IS_UNIX = os.platform() === 'linux' || os.platform() === 'darwin'; // core/src/code_executors/unsafe_local_code_executor.ts const TEST_EXECUTION_TIMEOUT = 40000; +/** + * The file `scripts/create_file.js` writes, as it appears on the tool response. + * + * Asserted by containment rather than equality: UnsafeLocalCodeExecutor skips + * input files by comparing `File.name` (which uses `/`) against an + * `fs.readdir({recursive: true})` entry (which uses `\` on Windows), so on + * Windows the skill's own input scripts are reported as outputs too. That is a + * separate executor defect, so these tests pin this tool's handling of the + * script's output rather than the executor's file count. + */ +const SCRIPT_OUTPUT = {name: 'output_from_script.txt', mimeType: 'text/plain'}; + describe('RunSkillScriptTool Integration with UnsafeLocalCodeExecutor', () => { - function createMockContext(agentName = 'test-agent') { + function createMockContext( + agentName = 'test-agent', + artifactService?: SessionArtifactService, + ) { return new Context({ invocationContext: { session: {state: {}}, agent: {name: agentName}, + artifactService, } as unknown as InvocationContext, }); } + async function cwdContains(filename: string): Promise { + return fs + .access(path.join(process.cwd(), filename)) + .then(() => true) + .catch(() => false); + } + const testSkill: Skill = { frontmatter: { name: 'test-skill', @@ -280,51 +309,60 @@ describe('RunSkillScriptTool Integration with UnsafeLocalCodeExecutor', () => { TEST_EXECUTION_TIMEOUT, ); - it('creates files in process.cwd returned from execution', async () => { + it('saves script output files to the artifact service', async () => { const executor = new UnsafeLocalCodeExecutor(); const toolset = new SkillToolset([testSkill], {codeExecutor: executor}); const tool = new RunSkillScriptTool(toolset); + const artifactService = createSessionArtifactService(); + const toolContext = createMockContext('test-agent', artifactService); const result = (await tool.runAsync({ args: { skill_name: 'test-skill', script_path: 'scripts/create_file.js', }, - toolContext: createMockContext(), - })) as CodeExecutionResult; - - expect(result).toBeDefined(); - expect(result.outputFiles).toBeDefined(); - expect(result.outputFiles?.length).toBeGreaterThan(0); - - const outputFile = result.outputFiles?.find( - (f) => f.name === 'output_from_script.txt', - ); - expect(outputFile).toBeDefined(); - - // Verify file was created in process.cwd() - const fullPath = path.join(process.cwd(), 'output_from_script.txt'); - const exists = await fs - .access(fullPath) - .then(() => true) - .catch(() => false); - expect(exists).toBe(true); - - const content = await fs.readFile(fullPath, 'utf-8'); - expect(content).toBe('hello from script file'); - - // Clean up - await fs.unlink(fullPath); + toolContext, + })) as SkillScriptResponse; + + expect(result.outputFiles).toContainEqual(SCRIPT_OUTPUT); + expect(result.warning).toBeUndefined(); + expect( + await loadArtifactText(artifactService, 'output_from_script.txt'), + ).toBe('hello from script file'); + expect(toolContext.actions.artifactDelta['output_from_script.txt']).toBe(0); + expect(await cwdContains('output_from_script.txt')).toBe(false); }); - it('handles file collisions by appending a numeric suffix', async () => { + it('creates a new artifact version instead of a renamed file on repeat runs', async () => { const executor = new UnsafeLocalCodeExecutor(); const toolset = new SkillToolset([testSkill], {codeExecutor: executor}); const tool = new RunSkillScriptTool(toolset); + const artifactService = createSessionArtifactService(); + const args = { + skill_name: 'test-skill', + script_path: 'scripts/create_file.js', + }; + + await tool.runAsync({ + args, + toolContext: createMockContext('test-agent', artifactService), + }); + const result = (await tool.runAsync({ + args, + toolContext: createMockContext('test-agent', artifactService), + })) as SkillScriptResponse; + + expect(result.outputFiles).toContainEqual(SCRIPT_OUTPUT); + expect( + await artifactService.listVersions('output_from_script.txt'), + ).toEqual([0, 1]); + expect(await cwdContains('output_from_script_2.txt')).toBe(false); + }); - // Pre-create the target file to force a collision - const targetFile = path.join(process.cwd(), 'output_from_script.txt'); - await fs.writeFile(targetFile, 'existing content'); + it('reports output files with a warning when no artifact service is configured', async () => { + const executor = new UnsafeLocalCodeExecutor(); + const toolset = new SkillToolset([testSkill], {codeExecutor: executor}); + const tool = new RunSkillScriptTool(toolset); const result = (await tool.runAsync({ args: { @@ -332,29 +370,10 @@ describe('RunSkillScriptTool Integration with UnsafeLocalCodeExecutor', () => { script_path: 'scripts/create_file.js', }, toolContext: createMockContext(), - })) as CodeExecutionResult; - - expect(result).toBeDefined(); - expect(result.outputFiles).toBeDefined(); - - const outputFile = result.outputFiles?.find( - (f) => f.name === 'output_from_script_2.txt', - ); - expect(outputFile).toBeDefined(); - - // Verify collision file was created in process.cwd() - const fullPath = path.join(process.cwd(), 'output_from_script_2.txt'); - const exists = await fs - .access(fullPath) - .then(() => true) - .catch(() => false); - expect(exists).toBe(true); - - const content = await fs.readFile(fullPath, 'utf-8'); - expect(content).toBe('hello from script file'); + })) as SkillScriptResponse; - // Clean up both files - await fs.unlink(targetFile); - await fs.unlink(fullPath); + expect(result.outputFiles).toContainEqual(SCRIPT_OUTPUT); + expect(result.warning).toMatch(/No artifact service is configured/); + expect(await cwdContains('output_from_script.txt')).toBe(false); }); });