Chore(ci): fail validation when the test run leaves the working tree dirty

[AmaadMartin]

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):
   Closes: #issue_number
   Related: feat: skills: support script execution google/adk-js#276 — the PR whose local run left the stray 5-byte output.txt at the repository root that is still tracked today.
2. Or, if no issue exists, describe the change:
   Problem: Several tests materialize real files into process.cwd() (the repository root on CI) and delete them only on the happy path — the fs.unlink calls live at the end of the it() body, so any assertion that fails earlier leaks the artifact:

Test	Written into process.cwd()	Cleanup
tests/integration/tools/run_skill_script_tool_test.ts	output_from_script.txt, output_from_script_2.txt	fs.unlink inside the it() body
tests/integration/tools/run_skill_inline_script_tool_test.ts	test_output_<ts>.txt, test_inline_output_<ts>*.txt	fs.unlink inside the it() body
tests/integration/skills/script_js/agent_test.ts	ephemeral_entanglement.md, index.html, sketch.js	afterAll

Nothing in validation.yaml inspects the working tree afterwards. npm run lint and npm run format:check only glob **/*.ts, and typedoc --emit none writes nothing, so a leaked artifact keeps the job green and can then be git add-ed and merged by accident. That is exactly how the root output.txt got in (google#276).

Solution: two commits.

1. chore(ci) — the gate. One step in .github/workflows/validation.yaml, immediately after Run tests and check code coverage and before Run lint check, that fails the job when git status --porcelain is non-empty, emitting a GitHub ::error:: annotation plus the verbatim porcelain listing so the offending paths are named. 12 lines of YAML, no new action or npm dependency, no new script.

Design points worth reviewing:

• Placement — directly after the test step, so dirt is attributed to the test run and not to the later lint/format/docs steps.
• shell: bash — GitHub defaults run: to pwsh on windows-latest. Declaring bash (present on all three runner images) is what makes the same four lines work across the matrix; it reuses the idiom already in .github/workflows/cross-language-integration.yml. The Windows run log confirms it: shell: C:\Program Files\Git\bin\bash.EXE --noprofile --norc -e -o pipefail {0}.
• git status --porcelain, not git diff --exit-code — untracked files are the primary leak signal and git diff does not see them. The default -unormal behaviour is kept deliberately: a leaked directory collapses to a single line instead of spamming thousands of paths (verified locally as scenario 6, and in CI where the leaked references/ tree printed as one line).
• --ignored is deliberately not passed — dist/, node_modules/, coverage/, .husky/_/ and the fixture lockfiles are gitignored build/test output and must not trip the gate (verified, scenario 4 below).
• No if: always() and no continue-on-error — if an earlier step failed the job is already red; running the gate afterwards would only muddy failure attribution.
• The step never mutates the tree it inspects — no git clean, git checkout, git stash or auto-commit. Its only effect is the exit code.

2. fix — the pre-existing Windows leak the gate immediately caught. The gate was expected to be green on arrival; ubuntu-latest and macos-latest were, and windows-latest was not. It reported 112 dirty paths: references/ plus 111 files under scripts/ (hello.{js,py,sh,ps1,bat}, fail.*, create_file.js, double.js, each with _2 … _10 collision suffixes).

Root cause, in core/src/code_executors/unsafe_local_code_executor.ts. After running a script, the executor scans its scratch directory and reports everything that is not the script itself as output, skipping the input files it materialized:

const isInputFile = params.codeExecutionInput.inputFiles?.some(
  (f) => f.name === relativeFilePath,
);

relativeFilePath comes from fs.readdir(tempDir, {recursive: true}), which joins with the platform separator, while File.name is always /-separated. On POSIX the two spellings coincide and the skip works; on Windows 'scripts/hello.js' !== 'scripts\hello.js', so every nested skill resource was reported as script output, and RunSkillScriptTool then materialized the whole set into process.cwd() (run_skill_script_tool.ts:145) — on CI, the repository root; for a real user, wherever their agent runs. The _2 … _10 suffixes are materializeFiles' collision renaming, one round per test case.

The fix records the resolved path of each input at the moment it is materialized and matches on that instead, so the comparison no longer depends on separator spelling:

const inputFiles = await materializeFiles(
  params.codeExecutionInput.inputFiles,
  tempDir,
);
const inputFilePaths = new Set(
  inputFiles.map((f) => path.join(res.tempDir, f.name)),
);

Deriving the set from materializeFiles' return value uses the names it actually wrote, which are the post-collision-rename ones. The if (params.codeExecutionInput.inputFiles) guard that used to wrap that call is gone: CodeExecutionInput.inputFiles is a required File[] and all four in-repo callers pass it, so the guard was unreachable defensive code. Dropping it removes a branch v8 can never cover and takes the file's branch coverage from 86.15% to 87.30%.

Scope note: this is a product bug, not a test bug, so it is fixed at the root rather than by adjusting the tests — but it is unrelated to the CI gate in every way except that the gate is what surfaced it, and the gate cannot land while it makes a matrix leg red. It is a separate commit for that reason. Deliberately still out of scope: deleting the root output.txt and moving the process.cwd() writers onto fs.mkdtemp directories; the gate is correct and green with or without that work, in either merge order. Nothing was added to .gitignore — ignoring a leaked artifact would defeat the gate. One adjacent latent defect is left alone: on Windows a genuine nested output file is still reported with a \-separated File.name; fixing that is a naming change with its own blast radius and no bearing on the leak.

Collision check: gh pr list --repo AmaadMartin/adk-js --state open --limit 1000 (163 open PRs), then gh pr diff --name-only on every plausible candidate. 23 open PRs touch .github/workflows/validation.yaml; grepping their diffs for git status / porcelain / working tree / --exit-code found no PR that adds a working-tree-clean gate. The nearest neighbours are #507, #467 and #345, which each add a narrower git diff --exit-code -- package-lock.json lockfile-drift check attached to the install step — a different concern at a different point in the file. For the executor fix I also grepped the six open PRs touching the skill-script/executor area (#305, #353, #355, #410, #437, #516) for a separator or win32 fix and found none; #355 (fs.mkdtemp scratch dir) touches the same function but a different statement. This PR therefore branches from main rather than stacking.

Testing Plan

Please describe the tests that you ran to verify your changes. This is required for all PRs that are not small documentation or typo fixes.
Unit Tests:
[x] I have added or updated unit tests for my change.
[x] All unit tests pass locally.

For the CI gate: no unit test, and none should be added — the change adds no TypeScript source line, so there is nothing for vitest to cover and the vitest.config.ts coverage thresholds are unaffected. A test that re-read and parsed validation.yaml to assert the step exists would assert the file's own contents rather than any behaviour. The gate is verified by executing it (below).

For the executor fix: one new test in core/test/code_executors/unsafe_local_code_executor_test.ts, 'should exclude nested input files from the returned output files'. It passes a nested input (scripts/hello.js, the shape that breaks on Windows) and a second spelling of the same idea (./assets/logo.txt), and asserts the returned output files are exactly ['new_output.txt']. It runs the real subprocess — no mocks.

• npx vitest run --project unit:core core/test/code_executors/unsafe_local_code_executor_test.ts → 19 passed.
• Mutation proof: with the source hunk reverted (git stash push -- core/src/code_executors/unsafe_local_code_executor.ts) the new test FAILS on Linux:

  - Expected  ["new_output.txt"]
  + Received  ["new_output.txt", "assets/logo.txt"]
  core/test/code_executors/unsafe_local_code_executor_test.ts:355
  

  The scripts/hello.js half of the test cannot fail on Linux by construction (the string comparison happens to work when the separator is /); that half is proven by the windows-latest leg of this PR going from 112 dirty paths to clean.
• Coverage of core/src/code_executors/unsafe_local_code_executor.ts from that file alone: 94.78% lines / 87.30% branches. No line or branch added by this change is uncovered — every remaining gap is pre-existing and platform-gated (the Windows .bat/.ps1 extension selection at lines 103–113, the Exit code N fallback, the readdir error handler).
• No existing test was modified, skipped, or deleted.
• Downstream suites re-run green after the fix: core/test/tools/skills/ + core/test/utils/file_utils_test.ts (84 passed), and tests/integration/tools/{run_skill_script_tool,run_skill_inline_script_tool,skills_registry_integration}_test.ts (19 passed, 4 skipped).
• npx eslint and npx prettier --check clean on both changed files. No suppression (any, @ts-expect-error, eslint-disable) was added anywhere in this PR.

Manual End-to-End (E2E) Tests:
Please provide instructions on how to manually test your changes, including any necessary setup or configuration.

Every scenario below ran the real run: body extracted from the committed .github/workflows/validation.yaml (not a retyped copy) under bash --noprofile --norc -eo pipefail, which is how GitHub invokes a shell: bash step. To reproduce, from the repo root:

python3 -c "import yaml; d=yaml.safe_load(open('.github/workflows/validation.yaml')); \
  s=next(x for x in d['jobs']['run-tests']['steps'] if x['name'].startswith('Check that tests left')); \
  open('/tmp/gate.sh','w').write(s['run'])"
bash --noprofile --norc -eo pipefail /tmp/gate.sh; echo "exit=$?"

The negative scenarios are the proof that the gate can actually fail: on a clean tree it exits 0 silently, and each kind of dirt flips it to 1 with the path named.

#	Setup	Expected	Observed
1	clean checkout	exit 0, no output	exit=0, no output
2	: > output_from_script.txt (the real leak name from run_skill_script_tool_test.ts)	exit 1, ?? output_from_script.txt	::error::The test run left the working tree dirty. … then ?? output_from_script.txt, exit=1
3	printf '\n' >> README.md (modified tracked file)	exit 1, M README.md	annotation then M README.md, exit=1
4	mkdir -p coverage && : > coverage/x.json (gitignored)	exit 0, no output	exit=0, no output — confirms --ignored is not passed
5	mv output.txt … (deleted tracked file)	exit 1, D output.txt	annotation then D output.txt, exit=1
6	leaked directory with 3 nested files	exit 1, one line	annotation then ?? leaked_dir/, exit=1 — confirms -unormal collapsing

Each scenario restored the tree afterwards (rm, git checkout --, mv back); git status --porcelain was empty again before the next one.

Pipeline rehearsal on Linux (Node v22.22.2, npm 9.2.0), checking git status --porcelain after each stage:

1. npm install → tree clean (empty porcelain).
2. npm run build → tree clean.
3. npx vitest run --project integration <the three suites that write into process.cwd()> → all pass, tree clean, gate exit=0. The script_js fixture's in-place node_modules/ is removed by its own afterAll; git status --porcelain --ignored on that directory is empty afterwards.
4. npm run test:integration (the whole integration project — it contains every known process.cwd() writer) → git status --porcelain empty afterwards, gate exit=0. Three suites failed locally for an unrelated environment reason (a mirroring npm registry makes the fixtures' in-place npm install exceed the 60s beforeAll budget); the same suites pass individually and all three legs of CI run them clean.

Note on package-lock.json: installing through a mirroring registry rewrites the resolved URLs and shows the lockfile as modified locally. Installing from the public registry leaves it byte-identical, which is what CI does — the lockfile is lockfileVersion: 3 and already carries the win32-* and darwin-* optional platform entries, so no churn occurred on any of the three runners.

Three-OS CI confirmation: the new Check that tests left the working tree clean step ran (completed/success, not skipped) on ubuntu-latest, windows-latest and macos-latest, and all three run-tests legs are green. On the first commit it passed on ubuntu and macos and correctly failed on Windows with the 112-path listing described above, which is the end-to-end proof that the gate both fires and clears. The Windows leg also hit the known tests/integration/app_loader/app_loader_test.ts 40s discovery timeout once (unrelated to this diff — it fails the test step, before the gate step runs, and passed on re-run).

Checklist

[x] I have read the CONTRIBUTING.md document.
[x] I have performed a self-review of my own code.
[x] I have commented my code, particularly in hard-to-understand areas.
[x] I have added tests that prove my fix is effective or that my feature works.
[x] New and existing unit tests pass locally with my changes.