Repository navigation
Fix Poetry lock writer read three ways (#1847) - #1865
Mikola Lysenko (mikolalysenko) wants to merge 5 commits into
Conversation
Assisted-by: Claude Code:claude-opus-5-5
Hosted redirects, vendored wiring and the source-pin refusal each decided "was this poetry.lock written by an old Poetry" with their own copy of the same rule, while the helper built for that question had no caller. Add utils::poetry_lock::lock_writer, which reads the @generated stamp, else the lock format, and route all three through it so the warnings and refusals can't drift apart. No change for any lock a real Poetry writes. Fixes #1847 Assisted-by: Claude Code:claude-opus-5-5
A poetry.lock in format 0, 1.0 or 1.1 is only ever written by Poetry older than 1.3. If a hand-merged or copied header claimed Poetry 1.4+, the shared writer reader trusted the header and dropped the "written by Poetry < 1.4" install warnings that hosted and vendored modes gave before. Read the format first, as those warnings always did, and test a stamp that disagrees with the format. Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
|
bugbot run Generated by Claude Code |
1 similar comment
|
bugbot run Generated by Claude Code |
|
Burn-down agent: labeled Ready for review at
Generated by Claude Code |
|
Ready for review at
Generated by Claude Code |
The PR's one behaviour change is that a 1.0/1.1 lock carrying a hand-merged 1.4+ stamp now reads as its format (and source_constraint_break refuses it) instead of trusting the stamp. The description says this is covered, but no test row exercised it. Add rows to lock_writer's and source_constraint_break's tests; the latter fail on main, where the stamp decided. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Final reviewer: the logic is sound, but the description says the one behaviour change (a 1.0/1.1 lock with a hand-merged 1.4+ stamp now reads as its old format, so bugbot run Generated by Claude Code |
|
BugBot review Generated by Claude Code |
|
Ready for review at
Slack announcement not sent this run (the Slack send tool was unavailable); a later run will retry. Generated by Claude Code |
lock_writer parsed the lock with an uncached DocumentMut parse. Before this PR, source_constraint_break skipped parsing for any stamped lock, and the hosted pre-1.4 check used lock_fragments::parse_lock. Now every source-constrained package re-parsed the whole lock, and the PARSES counter that many_patches_render_the_lock_once relies on never saw it. Use the shared parse_lock so the parse is shared and counted. Also fix LockWriter::Format's doc (a format now wins over a stamp), and cover the six Poetry fixture locks the writer table skipped. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011f9aeS41XjBLw8LQsUUTYU
|
bugbot run Final reviewer: I pushed
Behaviour note for the description (non-blocking): one case now goes the loosening way. A format-0 lock ( I removed Generated by Claude Code |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 3143a07. Configure here.
|
Ready for review at
Generated by Claude Code |
Final review brief (
|
LLM Description written by Claude Code:claude-opus-5-5
Fixes #1847
Summary
"Which Poetry wrote this
poetry.lock" was answered three ways:redirect::poetry::pre_1_4_writer(hosted), an inline match invendor/pypi_poetry.rs(vendored), andsource_constraint_breakre-deriving it inline. Meanwhile
utils::poetry_lock::lock_writer_version,which was built for this question, had no caller.
This PR replaces
lock_writer_versionwith one reader,utils::poetry_lock::lock_writer(lock_text, known_format). It returns:LockWriter::Format("0" | "1.0" | "1.1")for a pre-2.0 format. OnlyPoetry < 1.3 writes those, and the format decides even when a stamp
says otherwise. That ordering is the one both pre-1.4 advisories had.
LockWriter::Poetry(major, minor)from the@generated by Poetry X.Ystamp, or(1, 3)for an unstampedlock-version = "2.0".Nonefor an unstamped 2.1+ lock or an unreadable one.LockWriter::is_pre_1_4()drives the hostedredirect_poetry_stale_install_riskand vendoredpypi_poetry_integrity_unverifiedwarnings.source_constraint_breaknow matches on the writer. The inline copies are deleted, and
generated_by_versionis nowpub(crate).No change for any lock a real Poetry writes. The only behaviour change is
for a hand-edited lock:
source_constraint_breaknow refuses a 1.0/1.1lock whose first line carries a 1.4–2.2 stamp. Before, the stamp alone
decided. This is the safe direction, since every Poetry from 1.3 on
re-solves those formats.
A second hand-edited case goes the other way. A legacy format-0 lock (with
a
[metadata.hashes]table) whose first line was hand-edited to namePoetry 1.3 – 2.2 used to be refused by
source_constraint_break. It isnow admitted, the same as every unstamped format-0 lock already was.
Third case: a 2.1+ lock whose first line was hand-edited to name a Poetry
older than 1.4 now gets the pre-1.4 install warnings. Before, only a 2.0
lock with such a stamp did. These are warnings only.
Tests
utils::poetry_lock::tests::lock_writer_reads_old_formats_then_the_stampcovers all 15 Poetry fixture locks (0.12 to 2.4), with and without a
known format. It also covers an unstamped 2.0 lock, a versionless stamp,
stamp-only text, stamped 2.0 and 2.1 locks, a 1.4+ stamp on a 0/1.0/1.1
lock, unstamped 2.1/2.2 locks, unreadable text, and CRLF variants.
It doesn't compile without the fix because the reader is new. The
stamp-vs-format rows would fail on the first commit (6abc8c5), which
checked the stamp first; /code-review caught that and 672ab67 fixes it.
integrity_advisory_tracks_pre_1_4_writers_not_the_bare_format,source_constraint_breaks_1_3_to_2_2_writers_and_pre_2_1_locksand thehosted stale-install / source-pin matrix.
Run locally on 672ab67:
cargo fmt --all -- --check: okcargo clippy --workspace --all-features -- -D warnings: okcargo test -p socket-patch-core --all-features --no-fail-fast: lib 6995passed and 4 failed; integration suites 658 passed. The 4 failures
(
copy_tree::relax_loop_must_not_traverse_symlinked_root,vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry,pypi_poetry::wire_write_failure_maps_error_and_leaves_lock_untouched,pypi_requirements::wire_failure_rolls_back_already_written_files) arepermission-denial tests. The sandbox runs as root, so the denial never
happens.
wire_write_failure_…fails the same way onmainhere. CIruns as non-root.
cargo test -p socket-patch-cli --all-features --test in_process_redirect_poetry --test mode_migration_pypi:83 passed. A full
cargo test --workspacedidn't fit the sandbox's diskallowance, so CI covers the remaining CLI suites. No wrapper under
npm/,pypi/orgem/reads Poetry locks, so they're untouched.Per-issue checklist:
lock_writer).pre_1_4_writer, the vendoredinline match and
source_constraint_breakare expressed through it.The inline copies are deleted. Test:
lock_writer_reads_old_formats_then_the_stamp./code-review high findings not applied
before (The #1282 source-pin guard misses Poetry 1.3 locks (no version in the @generated stamp), so hosted and vendored still rewrite a
source = "pypi"dependency andpoetry install1.3 fails with a solver error #1447): every previous copy read it that way, and the warningsit drives are advisories.
Format(&'static str)is stringly typed. Onlylock_writerbuildsit, from three literals. A
LockFormatenum would add more code thanthis refactor removes, so it's left as a possible follow-up.
https://claude.ai/code/session_011f9aeRpvEGL78RFpkUC4tZ
Note
Low Risk
Refactor of advisory/refusal heuristics on lock metadata only; real Poetry-generated locks are unchanged, with a stricter edge case for tampered headers on legacy formats.
Overview
Unifies how the codebase decides who wrote a
poetry.lockby replacing duplicated logic in hosted redirect, vendored PyPI, andsource_constraint_breakwithlock_writerand aLockWriterenum (Poetry(major, minor)or legacyFormat("0"|"1.0"|"1.1")).LockWriter::is_pre_1_4()now drives stale-install and integrity-unverified warnings; the localpre_1_4_writerhelper andlock_writer_versionare removed, andgenerated_by_versionispub(crate).Resolution order: pre-2.0
lock-versionwins over the@generated by Poetrystamp (so a hand-merged 1.4+ header cannot hide a 1.0/1.1 lock).source_constraint_breakis refactored to match onlock_writer; the only intentional behavior change is refusing source-pin breaks on stamped-but-1.0/1.1 locks that previously followed the stamp alone.Tests add
lock_writer_reads_old_formats_then_the_stamp(fixtures, CRLF, stamp-vs-format cases) and extendsource_constraint_breakcases for a 1.4 stamp on 1.0/1.1 formats.Reviewed by Cursor Bugbot for commit 3143a07. Configure here.
Generated by Claude Code