Skip to content

Chore: remove stray output.txt from the repository root - #473

Open
AmaadMartin wants to merge 1 commit into
mainfrom
fix/delete-root-output-txt-artifact
Open

Chore: remove stray output.txt from the repository root#473
AmaadMartin wants to merge 1 commit into
mainfrom
fix/delete-root-output-txt-artifact

Conversation

@AmaadMartin

@AmaadMartin AmaadMartin commented Aug 1, 2026

Copy link
Copy Markdown
Owner

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):
    No existing issue. Related (the change that introduced the file): feat: skills: support script execution google/adk-js#276
  2. Or, if no issue exists, describe the change:
    Problem: A 5-byte file named output.txt, containing hello with no trailing newline, is tracked at the repository root. It was committed by accident in feat: skills: support script execution google/adk-js#276 ("feat: skills: support script execution"), where it appears in the diffstat as output.txt | 1 + amid ~34 legitimate source, test and fixture changes — an execution artifact the author's local test run materialized into the working directory. Nothing in the repository reads, writes, or resolves that path. git grep -n 'output\.txt' returns exactly four hits, none of which touch the repository root:
  • core/test/code_executors/unsafe_local_code_executor_test.ts:301,318 — a script string writing new_output.txt inside the executor's own mkdtemp sandbox, plus the assertion on that name.
  • core/test/tools/skills/run_skill_script_tool_test.ts:210 and core/test/tools/skills/run_skill_inline_script_tool_test.ts:226 — in-memory File object literals passed to a mocked materializeFiles.

No .gitignore, .prettierignore, eslint.config.js, tsconfig.json, typedoc.json, vitest.config.ts, workflow, script or package.json manifest mentions it (scripts/check_license.sh only scans *.js/*.ts, so it is invisible to the license check). The file is inert history that pollutes git status for any contributor whose tooling happens to write to ./output.txt.

Solution: Delete it with git rm output.txt, so the removal is staged deterministically. The entire diff is output.txt | 1 -, one deletion, nothing else. There is no runtime, API or behavioral change, and published npm tarballs are unaffected — the root workspace package is not published and each workspace packs only from its own directory.

Deliberately NOT adding an output.txt entry to .gitignore, for three reasons:

  1. A bare output.txt line is a repo-wide pattern — git matches an unanchored, slash-free pattern at every directory depth. It would silently hide a legitimate future output.txt anywhere in the tree, including under tests/integration/**, where the skills tests already use output.txt as their canonical example output-file name. That is a real footgun, not a hypothetical one.
  2. The existing .gitignore entries are all tool-generated directories (dist/, node_modules/, coverage/, dev/.cache/), editor droppings, or secrets (.env). A generic single-file name of an artifact that no supported workflow produces at the repo root does not fit that pattern.
  3. There is no recurrence mechanism to defend against. materializeFiles(files, dir = process.cwd()) in core/src/utils/file_utils.ts:15 is the only writer of agent output files; its call sites either pass an explicit tempDir (unsafe_local_code_executor.ts:182) or are mocked in the unit tests, and the integration suites that exercise them either spawn with cwd set to their own fixture directory or record "outputFiles": []. Deletion alone is sufficient and complete.

Duplicate-work disclosure (collision check). Per process I ran gh pr list --repo AmaadMartin/adk-js --state open --limit 100 before implementing. This change duplicates an existing open PR on this fork: #429 ("Chore: remove stray output.txt from the repository root"), whose three-dot delta is byte-identical to this one (D output.txt). Two further open PRs also remove the same path within broader work: #438 (the deletion plus isolating the cwd-writing skill-script integration tests) and #410 (the deletion plus routing skill script outputs to the artifact service). Only one of these should be merged — whichever lands first will make the others conflict. This PR is the minimal, single-purpose version; if #429 is preferred, close this one.

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.

No tests were added or changed. This change contributes zero lines of production code, so there is nothing to cover, and an assertion that a specific file does not exist would be unmaintainable noise. The verification obligation here is the inverse — to prove nothing depended on the file — which the commands below do.

Unit Tests:
[ ] I have added or updated unit tests for my change. — N/A: the change adds no code. See above.
[x] All unit tests pass locally. (One pre-existing, unrelated failure, reproduced on the unmodified base — see below.)

$ npm run build
> tsc --emitDeclarationOnly && node ./build.js
  dist/cjs/index.js 1.2kb | dist/esm/index.js 269b | dist/web/index.js 269b   ⚡ Done

$ npx vitest run --project unit:core --project unit:dev
 Test Files  1 failed | 181 passed (182)
      Tests  1 failed | 2573 passed (2574)

$ git status --porcelain      # nothing regenerated output.txt
(empty)

The single failure is dev/test/cli/cli_create_test.ts:214, which asserts on the Google Cloud project/region prompt and depends on local gcloud credentials that are absent in this environment. It is pre-existing and unrelated to this change — reproduced on the unmodified base commit with output.txt still present:

$ git checkout fork/main && npx vitest run --project unit:dev dev/test/cli/cli_create_test.ts
 Test Files  1 failed (1)
      Tests  1 failed | 10 passed (11)     <- same test, same line

$ git checkout fix/delete-root-output-txt-artifact && npx vitest run --project unit:dev dev/test/cli/cli_create_test.ts
 Test Files  1 failed (1)
      Tests  1 failed | 10 passed (11)     <- identical counts

Per scope discipline it is reported, not fixed here.

CI note. The first CI attempt showed run-tests (macos-latest) failing with Test timed out in 40000ms at tests/integration/app_loader/app_loader_test.ts:82, which cancelled the Windows job via matrix fail-fast. That is an unrelated flake, not a consequence of this diff: the test npm installs a fixture into tests/integration/app_loader/discovery inside a 40s cap and never touches the repository root, and re-running the failed jobs with no code change passed — run-tests (macos-latest) 6m55s, run-tests (windows-latest) 8m56s, run-tests (ubuntu-latest) 5m32s, all green.

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

git checkout fix/delete-root-output-txt-artifact
test -e output.txt && echo "STILL PRESENT (fail)" || echo "removed (pass)"   # -> removed (pass)
git ls-files output.txt                                                      # -> (no output)
git grep -n "output\.txt" -- .                                               # -> only the four mocked test literals
git diff --stat fork/main...HEAD                                             # -> output.txt | 1 -

Ran after the full unit:core + unit:dev run above: the worktree is clean and output.txt is absent, confirming no test recreates it at the repository root.

Checklist

[x] I have read the CONTRIBUTING.md document.
[x] I have performed a self-review of my own code.
[ ] I have commented my code, particularly in hard-to-understand areas. — N/A: no code, this is a file deletion.
[ ] I have added tests that prove my fix is effective or that my feature works. — N/A: no code is added; see the Testing Plan for the evidence that nothing depended on the file.
[x] New and existing unit tests pass locally with my changes.

Committed accidentally in google#276 alongside the skills script
execution change. Nothing in the repository reads, writes, or resolves this
path, and no supported workflow regenerates it at the repository root.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant