Skip to content

Fix: stop the published constraints files from downgrading google-adk - #77

Open
AmaadMartin wants to merge 6 commits into
fix/update-constraints-precommit-hookfrom
fix/publish-constraints-files
Open

Fix: stop the published constraints files from downgrading google-adk#77
AmaadMartin wants to merge 6 commits into
fix/update-constraints-precommit-hookfrom
fix/publish-constraints-files

Conversation

@AmaadMartin

@AmaadMartin AmaadMartin commented Jul 31, 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):
    Closes: #issue_number
    Related: #issue_number
  2. Or, if no issue exists, describe the change:

Stacked on #56 (fix/update-constraints-precommit-hook). Base branch is that PR, not main. Review #56 first; this PR is the delta on top of it.

Problem: The constraints files published by #56 pin google-adk itself, so the flow README.md documents — pip install google-adk -c constraints-<ver>.txtinstalls an older google-adk than the user asked for. This is live today, not hypothetical:

resolved google-adk
PyPI latest 2.6.0
pip install google-adk (no constraints) 2.6.0
pip install google-adk -c constraints-3.11.txt (as published by #56) 2.5.0 ← downgrade

Reproduced with a real install against real PyPI, no mocks:

$ curl -s https://pypi.org/pypi/google-adk/json | python3 -c "import json,sys;print(json.load(sys.stdin)['info']['version'])"
2.6.0
$ uv pip install --python /tmp/before/bin/python -c constraints-3.11.txt google-adk
 + google-adk==2.5.0
$ /tmp/before/bin/python -c "import google.adk; print(google.adk.__version__)"
2.5.0

Cause. --all-extras pulls in the community extra, whose google-adk-community depends back on google-adk, so uv emits a google-adk== pin into the output. Combined with the deliberate 4-day --exclude-newer supply-chain buffer and the fact that these files are only regenerated when pyproject.toml dependencies change, that pin is structurally guaranteed to lag the current release — it can never be anything but a downgrade or a no-op.

Solution: pass --no-emit-package google-adk to uv pip compile and regenerate. uv's own footer now records the exclusion:

-google-adk==2.5.0
-    # via
-    #   google-adk-community
-    #   toolbox-adk
+# The following packages were excluded from the output:
+# google-adk

google-adk-community deliberately stays pinned — it is a genuine transitive dependency, not the package under installation. The regenerated files are otherwise byte-identical apart from the header; no dependency version moved (git diff --stat is 9 lines per file), so this does not disturb #56's resolution.

Two loose ends from the same change are closed here:

  • .github/workflows/constraints-check.yml pins uv to 0.11.8. That job is a byte-for-byte diff of uv's output against the committed files, so an unpinned resolver means a future uv release turns it red with no dependency having drifted. 0.11.8 is the version that generated the committed files (uv --version), and setup-uv@37802adc accepts a version input at that pinned SHA. Comment tells the next person to bump it alongside a regeneration.
  • CONTRIBUTING.md documents the manual workflow. Fix: unbreak the permanently-failing update-constraints pre-commit hook #56 removed the update-constraints pre-commit hook (correctly), which was the only — broken — discovery mechanism for these files. The runbook now says when to regenerate, that update mode exits 1 whenever it rewrites (a real trip hazard), and the --check command CI runs.

Collision check (per contribution process). Run before writing any code:

gh pr list --repo AmaadMartin/adk-python --state open --limit 100 --json number,title,headRefName
gh pr diff <n> --repo AmaadMartin/adk-python --name-only   # for 68, 63, 56, 54, 49

Five open PRs touch this area:

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.

$ pytest tests/unittests/test_constraints_files.py -q      # this branch
27 passed
$ pytest tests/unittests/test_constraints_files.py -q      # #56's head, for comparison
17 passed

+10 cases: two new tests x five interpreters.

Two tests added to #56's tests/unittests/test_constraints_files.py, both parametrized over the five interpreters:

  • test_constraints_files_do_not_pin_google_adk — the artifact contract users consume.
  • test_constraints_files_still_pin_google_adk_community — guards the other direction, that --no-emit-package did not over-exclude and silently drop a real transitive pin.

Two existing tests were strengthened, not weakened. No test was deleted, skipped, or disabled.

  • _HEADER_LINE_2 in test_constraints_header_is_script_generated now additionally requires --no-emit-package google-adk. One more mandatory token in the same regex; every assertion it made before still holds.

  • _LEAKED_ANNOTATION in test_constraints_files_have_no_annotation_leakage was a vacuous guard — it could never fail. It was anchored ^#\s+-c .*\.tmp$, but uv writes the constraint source indented inside the # via block, so # is never at column 0:

    a2a-sdk==1.1.2
        # via
        #   -c constraints-3.11.txt.stable.tmp      <- 4-space indent
        #   google-adk (pyproject.toml)
    

    Against a genuinely --constraint-corrupted file the old pattern scored 0 matches; the corrected ^\s*#\s+-c .*$ scores 330–382 per file. Assuming a .tmp path was a second hole: passing the committed file to --constraint directly leaks its real name with no .tmp, which an anchor-only fix still misses (measured: 0 vs 382). Matching any -c source closes both, and the clean files contain none, so there is no false positive.

    This is load-bearing because the test is the only guard for that regression — CI cannot catch it. Proved by reintroducing --constraint in the generator and regenerating:

    result
    leaked annotations written into each committed file 330–382
    ./scripts/update_constraints.sh --check exit 0 — CI green on polluted files
    suite with the old ^# pattern 27 passed — the regression ships silently
    suite with the corrected pattern 5 failed, 22 passed — one per interpreter

    --check passes because a polluted committed file and its equally-polluted candidate compare equal — exactly the "fixed point after a second update run has polluted the files" state. The failure message now reports a count plus one sample instead of dumping all 382 matches.

Every test was proven able to fail. The headline mutations are real source-level reverts, not hand-edits:

Mutation Tests killed Failure message
git stash the --no-emit-package hunk out of scripts/update_constraints.sh and regenerate 10test_constraints_header_is_script_generated[3.10-3.14] + test_constraints_files_do_not_pin_google_adk[3.10-3.14] constraints-3.10.txt pins google-adk itself: ['google-adk==2.5.0']... and constraints-3.10.txt line 2 is not the command ./scripts/update_constraints.sh reconstructs...
Reintroduce --constraint "$STABLE_FILE" in the generator and regenerate 5test_constraints_files_have_no_annotation_leakage[3.10-3.14] constraints-3.11.txt carries 382 uv constraint-source annotations, e.g. ' # -c constraints-3.11.txt.stable.tmp'...
Swap in a real --constraint-generated file with no .tmp in the path test_constraints_files_have_no_annotation_leakage[3.12] ...e.g. ' # -c constraints-3.11.txt'
Hand-insert google-adk==2.5.0 into constraints-3.13.txt (bypassing the generator) test_constraints_files_do_not_pin_google_adk[3.13] constraints-3.13.txt pins google-adk itself: ['google-adk==2.5.0']
Delete the google-adk-community== line from constraints-3.14.txt test_constraints_files_still_pin_google_adk_community[3.14] constraints-3.14.txt no longer pins google-adk-community. --no-emit-package must exclude google-adk only.

Rows 1 and 2 are the important ones: both are reintroductions of the actual defect in the generator, confirming the guards catch the bug at its source rather than only a doctored artifact. The script and constraints files were restored after each mutation (git status --porcelain clean).

Coverage. No Python source changed — this is a Bash + YAML + Markdown change plus a regenerated artifact — so there is no new production line to cover. The added tests are themselves the coverage of the changed behaviour, and the Constraints Up To Date CI job is the permanent regression test.

Integration — real tooling, real PyPI, no mocks. Every command run on this branch:

# Command Result
1 bash -n scripts/update_constraints.sh exit 0
2 ./scripts/update_constraints.sh (regenerate) exit 1, five files rewritten — expected, update mode exits 1 when it writes
3 ./scripts/update_constraints.sh (again) exit 0, five ✅ — idempotent, wrote nothing
4 ./scripts/update_constraints.sh --check exit 0, five ✅
5 grep -E '^google-adk==' constraints-*.txt no matches
6 grep -c 'stable\.tmp' constraints-*.txt 0 for all five
7 grep -E ' +$' constraints-*.txt no matches
8 pre-commit run --all-files exit 0 — all 12 hooks pass, tree clean

Dead code removed from the generator, with the paths it touched re-exercised. Four post-run rm -f "$NEW_FILE" calls were already covered by the existing cleanup() { rm -f constraints-*.tmp; } + trap cleanup EXIT ($NEW_FILE is constraints-<ver>.txt.new.tmp), and the --output-file / --output-file= sed rewrites were unreachable because GENERATION_CMD is built in this script and always ends in -o $TARGET_FILE. Removing them is only safe if the trap really covers every path the rms used to, so each was exercised:

Path the removed rm used to handle Result
resolution failure (injected an unsatisfiable dependency) exit 1, 5/5 reported (does not abort on the first), no constraints-*.tmp, no committed file clobbered
check-mode OUT OF DATE exit 1, 5/5 reported, no constraints-*.tmp, 0 constraints files modified
update-mode rewrite exit 1, 5 rewrites, no constraints-*.tmp

The five committed files stayed byte-identical throughout, which is the proof that the dropped sed rewrites were genuine no-ops. The single rm before the seeding cp is kept: it covers the skipped-cp case, where a candidate left behind by a run killed before the trap could fire would otherwise seed uv.

The last of those sed rewrites is now gone too, at its root. The script built GENERATION_CMD itself and then regex-rewrote its own literal (s/-o [^ ]+/-o $NEW_FILE/) to retarget the output. That worked only because no other token in the command happened to contain -o ; a future flag that did would have silently redirected uv's output with no error. The destination-independent part is now built once as COMPILE_CMD, and each use appends its own -o — the header keeps -o $TARGET_FILE, the run appends -o $NEW_FILE:

-  RUN_CMD=$(echo "$GENERATION_CMD" | sed -E "s/-o [^ ]+/-o $NEW_FILE/")
-  echo "Running: $RUN_CMD"
-  if ! eval "$RUN_CMD"; then
+  echo "Running: $COMPILE_CMD -o $NEW_FILE"
+  if ! eval "$COMPILE_CMD -o $NEW_FILE"; then

The emitted invocation is byte-identical, and --check is the proof: it rebuilds each header from GENERATION_CMD and byte-diffs it against the committed file, so a single changed character would surface as OUT OF DATE. It reports exit 0, 5/5 up-to-date. The failure branch — the line actually edited — was re-exercised by injecting an unsatisfiable dependency into pyproject.toml: exit 1, all five reported (no abort on the first), no constraints-*.tmp residue, no committed file clobbered, pyproject.toml restored.

Test-only cleanup in the same commit. The module imported the private _find_pyproject from a sibling test module — the suite's only from tests.unittests.test_* import (verified by grep). It bought nothing here: its sibling-build-directory handling only reaches layouts where scripts/update_constraints.sh is absent, and the module already pytest.skips there, so both forms behave identically. _REPO_ROOT is now Path(__file__).parents[2], still lexical so the symlinked-test-tree safety that motivated avoiding .resolve() is preserved. To prove the new resolution really reads the committed files rather than passing vacuously, a third mutation was added: corrupting line 2 of constraints-3.14.txt (--all-extras--all-extra) fails test_constraints_header_is_script_generated[3.14]. All 27 tests pass; the two original mutations still kill their guards.

Drift detection still detects drift (the error path). Temporarily capping a dependency below its resolved pin (tenacity>=9,<10tenacity>=9,<9.1; resolved pin is 9.1.4):

$ ./scripts/update_constraints.sh --check
❌ constraints-3.10.txt is OUT OF DATE!
   Please run the update script locally to update it and commit the changes:
   $ ./scripts/update_constraints.sh
   (x5)                                        exit 1
$ git status --porcelain      # only pyproject.toml -- --check did NOT rewrite the tree
$ ./scripts/update_constraints.sh             # exit 1, 5 files, 1-line diff each
$ ./scripts/update_constraints.sh --check     # exit 0
$ grep -E '^google-adk==' constraints-*.txt   # still none after a repair cycle

Experiment reverted (git checkout -- pyproject.toml constraints-*.txt).

Note: a first attempt using the plan's suggested tenacity>=9,<10tenacity>=9.1,<10 did not produce drift, because the resolved tenacity==9.1.4 already satisfies the raised floor. A ceiling below the resolved pin is required to force a resolution change.

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

  1. The user-facing promise — no downgrade. Two real venvs, real PyPI:

    uv venv /tmp/before --python 3.11 && uv venv /tmp/after --python 3.11
    git show <#56-head>:constraints-3.11.txt > /tmp/before.txt
    
    uv pip install --python /tmp/before/bin/python -c /tmp/before.txt google-adk
    /tmp/before/bin/python -c "import google.adk; print(google.adk.__version__)"   # 2.5.0  <- downgrade
    
    uv pip install --python /tmp/after/bin/python  -c constraints-3.11.txt google-adk
    /tmp/after/bin/python  -c "import google.adk; print(google.adk.__version__)"   # 2.6.0  <- correct
  2. The constraints file still constrains — the fix must not turn it into a no-op. uvicorn latest on PyPI is 0.52.0; the file pins 0.51.0; the install above landed on uvicorn 0.51.0. Supply-chain pinning is intact — only google-adk itself is exempt. At the resolver level, uv pip compile 'google-adk[community]' -c constraints-3.11.txt yields google-adk==2.6.0 (uncapped) alongside google-adk-community==0.5.0 (pinned).

  3. CI — no workflow runs on this PR, by design of the stack. Both constraints-check.yml and continuous-integration.yml are pull_request: branches: [main], and this PR's base is fix/update-constraints-precommit-hook, so neither triggers. Only agent-triage-pull-request appears, and it skips. That is absent, not green — so the CI jobs were reproduced locally on the exact pushed commit 325c9ce instead:

    CI job Local equivalent Result
    Constraints Up To Date ./scripts/update_constraints.sh --check with uv 0.11.8 — the exact version this PR pins the job to exit 0, 5/5 ✅
    Pre-commit Linter pre-commit run --all-files exit 0, 12 hooks passed, 0 failed
    (unit tests) pytest tests/unittests/test_constraints_files.py 27 passed

    Working tree clean before and after all three. These jobs will run for real once the stack is flattened onto main.

  4. The stack's base PR Fix: unbreak the permanently-failing update-constraints pre-commit hook #56 does run the real jobs, and they confirm the fix. Fix: unbreak the permanently-failing update-constraints pre-commit hook #56 targets main, so continuous-integration.yml and constraints-check.yml both trigger there. On Fix: unbreak the permanently-failing update-constraints pre-commit hook #56's head (7f10983, the commit this PR is stacked on):

    Job Result
    Pre-commit Linter pass (45s)
    Constraints Up To Date pass (21s)
    Mypy Check (3.10/3.11/3.13) pass
    A2A v0.3 Tests (3.10–3.14) pass
    Unit Tests (3.10–3.14) fail — pre-existing, unrelated (see below)

    The Pre-commit Linter result is the direct evidence for the headline claim, because there is a control: open PRs branched from a main that lacks this stack (Fix: restore the mdformat-flattened YAML frontmatter in the local_environment_skill sample SKILL.md #121, Fix: correct the ListSessionsResponse docstring to match implemented state/events behaviour #122, Fix: do not record a group's help fallback as a failed CLI command run #124) all have Pre-commit Linter failing, and the failure is exactly the hook this stack removes —

    update-constraints.......................................................Failed
    - hook id: update-constraints
    Processing constraints-3.10.txt...
    Running: uv pip compile ... -o constraints-3.10.txt.new.tmp --constraint constraints-3.10.txt.stable.tmp
    ⚠️ Resolution failed with stable constraints. Retrying without constraints to allow upgrades...
    ❌ Resolution failed even without constraints.
    

    Same workflow, same runner image; the only difference is this stack. That is the lint gate going from red to green.

    The Unit Tests failures are not from this change and must not be attributed to it. They reproduce identically on unrelated PRs (Fix: restore the mdformat-flattened YAML frontmatter in the local_environment_skill sample SKILL.md #121, Fix: correct the ListSessionsResponse docstring to match implemented state/events behaviour #122, Fix: do not record a group's help fallback as a failed CLI command run #124), and none of them involve a constraints file — they are ImportError: Blocked import of regex from current working directory (nltk's CWD import guard, addressed separately) and click-8.2 CLI exit-code assertions. No test in tests/unittests/test_constraints_files.py appears anywhere in the failure log; that file's 27 cases pass.

Scope note — one documented deviation from the plan. The plan lists CONTRIBUTING.md as out of scope on the grounds that it "remains accurate after this change". It stays accurate, but it becomes incomplete: removing the update-constraints hook removes the only (broken) mechanism by which a contributor would ever discover that these files need regenerating. The 22-line runbook added here closes exactly the gap the hook removal opens, and nothing more. Every value in it is taken from the repo rather than from memory — the pinned uv 0.11.8 matches .github/workflows/constraints-check.yml, and the "exits 1 whenever it rewrites" warning is the script's own EXIT_CODE=1 behaviour.

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.

Amaad Martin added 5 commits August 1, 2026 07:01
--all-extras pulls in the `community` extra, whose google-adk-community
depends back on google-adk, so uv emitted a `google-adk==` pin into every
constraints file. README.md documents
`pip install google-adk -c constraints-<ver>.txt`, and these files are
regenerated only when pyproject.toml dependencies change, so the pin goes
stale on the next release and silently downgrades the very package the user
asked to install.

Pass --no-emit-package google-adk and regenerate. google-adk-community stays
pinned: it is a genuine transitive dependency, not the package under
installation.

The header guard test is tightened to require the new flag, and two tests are
added for the artifact contract itself: no google-adk self-pin, and
google-adk-community still pinned.
The Constraints Check job diffs uv's output byte-for-byte against the
committed files, so an unpinned resolver means a new uv release can turn the
job red without any dependency having drifted. Pin it to the version that
generated the committed files, with a note to bump it alongside a
regeneration.

Removing the pre-commit hook took away the (broken) discovery mechanism for
these files, so CONTRIBUTING.md now documents the manual workflow: when to
regenerate, that update mode exits 1 whenever it rewrites, and the --check
command CI runs.
Matches the other per-version guards in the module and reports the failing
interpreter individually instead of stopping at the first one.
The guard was anchored `^#\s+-c .*\.tmp$`, but uv writes the constraint source
*indented inside* the "# via" block ("    #   -c constraints-3.11.txt"), so `#`
is never at column 0. Against a genuinely corrupted file the pattern scored 0
matches while the corrected one scored 333-382 per file, i.e. the guard could
never fail.

Assuming a .tmp path was a second hole: passing the committed file to
--constraint directly leaks its real name and no .tmp, which an anchor-only fix
still misses. Match any `-c` source instead; the clean files contain none, so
there is no false positive.

This matters because the test is the only guard for this regression. Verified
by reintroducing --constraint in the generator and regenerating: the files pick
up 382 leaked annotations per interpreter, `--check` still exits 0 (a polluted
file and its polluted candidate compare equal, so CI stays green), and only
this test fails - 5 failures, one per interpreter, where the old pattern left
the suite at 27 passed.

Also report a count plus one sample rather than dumping all 382 matches.
…writes

Four post-run `rm -f "$NEW_FILE"` calls were already covered by
`cleanup() { rm -f constraints-*.tmp; }` + `trap cleanup EXIT`, since
$NEW_FILE is constraints-<ver>.txt.new.tmp. Removed. The one before the
seeding `cp` stays: it covers the skipped-cp case, where a candidate left by
a run killed before the trap fired would otherwise seed uv.

The `--output-file` and `--output-file=` sed rewrites were unreachable.
GENERATION_CMD is built in this script and always ends in `-o $TARGET_FILE`,
so only the `-o` rewrite ever fires.

Verified no temp file survives any path the removed rm calls used to handle:
resolution failure (unsatisfiable dependency, 5/5 reported, no clobbering),
check-mode OUT OF DATE, and update-mode rewrite all leave no constraints-*.tmp
and exit as before. The constraints files stay byte-identical, confirming the
dropped seds were no-ops.

CONTRIBUTING.md: the drift-check paragraph named the command inline and then
repeated it in a fenced block; collapsed to one sentence. It also now states
that regeneration must use the uv version CI pins, since the check is a
byte-for-byte diff of uv's output and a contributor on a different resolver
would otherwise hit drift they cannot reproduce.
The generator built GENERATION_CMD itself and then regex-rewrote its own
literal to retarget the output file. That only worked because no other
token in the command contained "-o "; a future flag that did would have
silently redirected uv's output with no error. Build the
destination-independent part once and let each use append its own -o.

Also drops a cross-test private import: the module pulled _find_pyproject
from a sibling test module, which was the suite's only
from tests.unittests.test_* import. It bought nothing here -- its sibling
build-directory handling only reaches layouts where the script is absent
and the module already skips -- so the repo root is now derived
lexically, keeping the symlink-safety that motivated avoiding resolve().

The emitted uv invocation is byte-identical, confirmed by --check still
diffing clean against all five committed files.
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