Skip to content

Fix: fail update_constraints.sh --check when the header records no single snapshot date - #213

Open
AmaadMartin wants to merge 2 commits into
mainfrom
fix/update-constraints-check-unreadable-snapshot-date
Open

Fix: fail update_constraints.sh --check when the header records no single snapshot date#213
AmaadMartin wants to merge 2 commits into
mainfrom
fix/update-constraints-check-unreadable-snapshot-date

Conversation

@AmaadMartin

@AmaadMartin AmaadMartin commented Aug 11, 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):

  2. Or, if no issue exists, describe the change:

Problem: update_constraints.sh --check re-resolves each constraints-<ver>.txt against the snapshot date recorded in the file's own header. When the header records no date, the substitution yields an empty string, the --exclude-newer flag is dropped, and the pins are re-resolved against the live index; every release published since then reads as drift. When the header records two dates, the two-line value expands into stray argv (2026-03-04: command not found). Both cases end in ❌ <file> is OUT OF DATE!, which is the wrong diagnosis and points the reader at a remedy that launders an unreadable header into a new supply-chain baseline.

Solution: In check mode the recovered value must match exactly one YYYY-MM-DD token. A file that fails the match gets its own message, sets the exit code and is skipped before any resolution runs, so the live index is never consulted for it. The remaining versions are still checked. Update mode owns the date and is unchanged, which makes the if [ -n "$date_to_use" ] guard dead code, so it is removed.

Collision check: I listed all 100 open pull requests on the fork. #149 rewrites the same block, but it also redesigns update mode, so this change stays independent of it and rebases cleanly in either order. #159, #163 and #195 touch other parts of the script.

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:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

pytest tests/unittests/scripts/test_update_constraints.py -q → 7 passed.

The tests drive the real script through bash against a stub uv on PATH, so nothing is mocked and no test reaches PyPI.

Proof the tests can fail. Against the unmodified script, 4 failed and 3 passed. The other three pin behaviour this change must preserve, so I mutated each separately:

Mutation Result
The whole change reverted 4 failed: every rejection case, e.g. assert 0 == 1 on the exit code
Shape check narrowed to ^[0-9]{4}-[0-9]{2}$ 5 failed, adding test_check_mode_resolves_against_the_recorded_snapshot_date
Update mode also reads the header 1 failed: test_update_mode_regenerates_a_file_with_a_malformed_header
Missing-file branch loses its continue 1 failed: test_check_mode_reports_missing_files_without_resolving

Update mode is unchanged, measured rather than asserted. I ran the old script and the new one over identical trees with a stub uv, one file carrying a stripped header. Both make 5 uv calls, both resolve against today - 4d, both rewrite all five files and exit 1. The two transcripts differ only in the diff -u timestamps.

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

cp constraints-3.12.txt /tmp/c312.bak
sed -i 's/ --exclude-newer [0-9-]*//' constraints-3.12.txt
./scripts/update_constraints.sh --check
cp /tmp/c312.bak constraints-3.12.txt

I ran this against the committed constraints-3.12.txt with no uv on PATH, so any resolution attempt would be visible. A stripped header and two concatenated headers both print ❌ constraints-3.12.txt has no single '--exclude-newer YYYY-MM-DD' snapshot date in its header,, exit 1 and leave the file byte-unchanged, with no uv call. The control run with the real header still reports Found generation command: ... --exclude-newer 2026-07-24 ..., so a well-formed file is not rejected.

CI on this PR: all 5 Unit Tests jobs, all 5 A2A jobs and all 4 Mypy jobs pass. The Pre-commit Linter job fails on the update-constraints hook with files were modified by this hook. That failure is pre-existing and unrelated: PR #212 reproduces it on a branch that touches neither the script nor the constraints files. The hook runs the script in update mode over the committed files, which is the concern of #163.

Checklist

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

Check mode reads the snapshot date out of the file's own header. When the
header records no date, or several, the extraction yields a value the shell
cannot use: an empty one drops --exclude-newer and re-resolves the pins
against the live index, and a multi-line one expands into stray argv. Both
report the file as OUT OF DATE, which is the wrong diagnosis and the wrong
remedy. Require exactly one YYYY-MM-DD in check mode, and reject the file
before any resolution runs. Update mode owns the date and is unchanged.
@AmaadMartin
AmaadMartin force-pushed the fix/update-constraints-check-unreadable-snapshot-date branch from 488c398 to 86e548b Compare August 11, 2026 02:57
@AmaadMartin AmaadMartin changed the title Fix: reject a constraints header that records no single snapshot date Fix: fail update_constraints.sh --check when the header records no single snapshot date Aug 11, 2026
@AmaadMartin
AmaadMartin changed the base branch from feat/restore-dependency-constraints to main August 11, 2026 02:57
The surrounding script comments are one line each. Keep the intent and
drop the notes about bash mechanics, which the echo block below already
states to the reader.
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