Skip to content

scripts/check_new_py_files.py misses added files on Windows under jj and hg #7029

Description

@iam-kira

get_vcs_added_files() in scripts/check_new_py_files.py builds absolute paths for the jj and hg backends with os.path.join, which inserts os.sep. On Windows that yields a mixed-separator path:

/workspace\src/google/adk/agents/_jj_agent.py
Every downstream comparison in the module uses forward slashes. The module already normalises at three other sites — lines 144, 151, 321 and 409 all call .replace(os.sep, '/'). The jj (line 215) and hg (line 225) branches are the two that missed it.

The git backend is unaffected because git emits forward slashes natively, which is why this is invisible in the common case.

Consequence: on Windows under jj or hg, newly added .py files are not matched, so the script reports nothing to check and silently passes.

Steps to Reproduce
Clone the repo on Windows and install dev dependencies.
Run the script's own test suite:
pytest tests/unittests/tools/... -k check_new_py_files
(the suite is test_check_new_py_files.py)
Observe the jj and hg cases fail on path-separator mismatch in the mock assertions.
Expected Behavior
Paths returned by get_vcs_added_files() use forward slashes for every VCS backend, so the downstream comparisons match — the same normalisation the module already applies elsewhere.

Observed Behavior
jj and hg return -separated paths on Windows, which never match, so added files go undetected.

test_check_new_py_files.py on Windows: 6 failed / 23 passed
Two of those six are this bug. The other four are separate problems and are not part of this report: one is symlink-privilege (WinError 1314) and two shell out to a POSIX sh forwarder.

Environment Details
ADK Library Version (pip show google-adk): N/A — the defect is in scripts/, not the
installed package; reproduced from a source checkout of main
Desktop OS: Windows 11 (Windows-11-10.0.26200-SP0)
Python Version (python -V): Python 3.13.15
Model Information
Are you using LiteLLM: N/A
Which model is being used: N/A
This is a repository tooling bug; no model is involved.

🟡 Optional Information
Regression
Unknown — the normalisation is present at the other four sites, so the jj/hg branches look like they were added later without it rather than having regressed.

Logs
Nothing is logged. The script exits successfully having found no added files, which is the failure mode: it does not error, it under-reports.

Additional Context
No workflow runs this suite on Windows, so nothing upstream could have caught it. This matches two Windows path bugs already fixed here — #6415 and #6419 (adk eval mis-handling Windows paths).

Minimal Reproduction Code
import os

what the jj branch does today, on Windows:

jj_root = "/workspace"
p = "src/google/adk/agents/_jj_agent.py"
print(os.path.join(jj_root, p))

-> /workspace\src/google/adk/agents/_jj_agent.py (mixed separators)

what every other site in the module does:

print(os.path.join(jj_root, p).replace(os.sep, "/"))

-> /workspace/src/google/adk/agents/_jj_agent.py

How often has this issue occurred?
Always (100%) — on Windows with jj or hg.

Suggested fix
Apply the module's existing normalisation to the two branches that lack it:

jj branch (line 215)

p = os.path.join(jj_root, p).replace(os.sep, '/')

hg branch (line 225)

os.path.join(hg_root, f.strip()).replace(os.sep, '/')
Five lines changed, two of them comments. Verified on Windows: 6 failed / 23 passed → 4 failed / 25 passed, with the remaining four being the unrelated problems noted above.

Patch: patches/adk-python/0001-fix-scripts-normalize-VCS-reported-paths-to-forward-.patch (applies cleanly to main as of 2026-09-06).

Happy to open a PR once the CLA is signed.

Activity

self-assigned this
on Sep 7, 2026
added theissue type on Sep 7, 2026

claxman commented on Sep 7, 2026

@claxman
Contributor

get_vcs_added_files joins jj and hg roots with os.path.join and never calls .replace(os.sep, '/'). A snippet mocked os.sep to \\ and os.path.join to join on that sep, then called get_vcs_added_files('.') with a fake jj diff --summary of A src/google/adk/agents/_jj_agent.py. It printed added {'/workspace\\src/google/adk/agents/_jj_agent.py'} and match False. I will add .replace(os.sep, '/') on those jj and hg paths.

llalitkumarrr commented on Sep 8, 2026

@llalitkumarrr
Collaborator

Hello @iam-kira,

Thank you for bringing this issue to our attention. Based on our understanding this should not have any functional impact on the google-adk or cause your agentic app to fail. Could you please share more details on how this is affecting your system? This will help us better understand the problem.

added
request clarification[Status] The maintainer need clarification or more information from the author
on Sep 8, 2026

iam-kira commented on Sep 13, 2026

@iam-kira
Author

@llalitkumarrr You're right that it has no functional impact on google-adk or on agentic apps — scripts/check_new_py_files.py is contributor tooling and never ships in the package.

The impact is only on the local pre-commit check. On Windows under jj or hg, the added-file paths come back with mixed separators, so they never match src/google/adk/, and the check exits 0 without applying the private-by-default or unit-guide rules to new files. CI isn't affected, since it uses baseline-diff mode, so a contributor in that setup just finds out later than intended.

Worth noting it's the exact case the script's docstring says exit code 3 exists to prevent ("could not determine the added files" being read as "no violations"), just reached by a different route.

Low priority is completely reasonable. @claxman has offered to add the .replace(os.sep, '/') on those two paths, which is the whole fix.

GWeale commented on Sep 29, 2026

@GWeale
Collaborator

fixed on main by fe69c0b from #7046 (thanks @claxman), which normalizes the jj and hg paths to forward slashes. the script isn't part of the package, so no release is needed to pick it up. please open a new issue if this is still happening.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

request clarification[Status] The maintainer need clarification or more information from the authortools[Component] This issue is related to tools

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions