diff --git a/.github/workflows/validation.yaml b/.github/workflows/validation.yaml index 7bb00098e..10528a62c 100644 --- a/.github/workflows/validation.yaml +++ b/.github/workflows/validation.yaml @@ -40,6 +40,18 @@ jobs: - name: Run tests and check code coverage run: npm run test:coverage + - name: Check that tests left the working tree clean + # Tests must clean up after themselves; a leaked artifact once got + # committed from a local run (google/adk-js#276). + shell: bash + run: | + changes="$(git status --porcelain)" + if [ -n "$changes" ]; then + echo "::error::The test run left the working tree dirty. Tests must delete the files they create, or write them under a temp directory (fs.mkdtemp)." + echo "$changes" + exit 1 + fi + - name: Run lint check run: npm run lint diff --git a/core/src/code_executors/unsafe_local_code_executor.ts b/core/src/code_executors/unsafe_local_code_executor.ts index 97061719a..b54641817 100644 --- a/core/src/code_executors/unsafe_local_code_executor.ts +++ b/core/src/code_executors/unsafe_local_code_executor.ts @@ -178,9 +178,17 @@ export class UnsafeLocalCodeExecutor extends BaseCodeExecutor { const filePath = res.filePath; tempDir = res.tempDir; - if (params.codeExecutionInput.inputFiles) { - await materializeFiles(params.codeExecutionInput.inputFiles, tempDir); - } + // Recorded as resolved paths rather than names: `fs.readdir` reports + // platform separators (`subdir\file` on Windows) while `File.name` is + // always `/`-separated, so comparing the two as raw strings misses + // nested inputs and re-reports them as script output. + const inputFiles = await materializeFiles( + params.codeExecutionInput.inputFiles, + tempDir, + ); + const inputFilePaths = new Set( + inputFiles.map((f) => path.join(res.tempDir, f.name)), + ); let command = this.nodeCommandPath; let args = [filePath]; @@ -271,10 +279,7 @@ export class UnsafeLocalCodeExecutor extends BaseCodeExecutor { } // Skip input files - const isInputFile = params.codeExecutionInput.inputFiles?.some( - (f) => f.name === relativeFilePath, - ); - if (isInputFile) { + if (inputFilePaths.has(fullPath)) { continue; } diff --git a/core/test/code_executors/unsafe_local_code_executor_test.ts b/core/test/code_executors/unsafe_local_code_executor_test.ts index 82a0d2d01..e568274e6 100644 --- a/core/test/code_executors/unsafe_local_code_executor_test.ts +++ b/core/test/code_executors/unsafe_local_code_executor_test.ts @@ -7,6 +7,7 @@ import { CodeExecutionLanguage, ExecuteCodeParams, + FileContentEncoding, InvocationContext, LlmAgent, PluginManager, @@ -321,6 +322,39 @@ describe('UnsafeLocalCodeExecutor', () => { expect(result.outputFiles![0].mimeType).toBe('text/plain'); }); + it('should exclude nested input files from the returned output files', async () => { + const params: ExecuteCodeParams = { + invocationContext, + codeExecutionInput: { + code: 'const fs = require("fs"); fs.writeFileSync("new_output.txt", "hello from script");', + language: CodeExecutionLanguage.JAVASCRIPT, + // A nested input file is read back from disk as `scripts/hello.js` on + // POSIX and `scripts\hello.js` on Windows, and `./assets/logo.txt` is + // a third spelling of the same path. All of them denote an input and + // must not be reported as script output. + inputFiles: [ + { + name: 'scripts/hello.js', + content: 'console.log("hello");', + contentEncoding: FileContentEncoding.UTF8, + mimeType: 'text/javascript', + }, + { + name: './assets/logo.txt', + content: 'logo', + contentEncoding: FileContentEncoding.UTF8, + mimeType: 'text/plain', + }, + ], + }, + }; + + const result = await executor.executeCode(params); + + expect(result.stderr).toBe(''); + expect(result.outputFiles!.map((f) => f.name)).toEqual(['new_output.txt']); + }); + it('should infer correct mimeType for generated JSON files', async () => { const params: ExecuteCodeParams = { invocationContext,