Repository navigation
repo_shape.py and bootstrap.py: COMMIT_RE, SHA256_RE and COMMIT_40 end in \Z, not $, so a pin's commit: or tree_sha256: that ends in a line break is refused as not 40 or 64 hex by every validator and adopt-project.py (#1017) - #1126
Conversation
…40` end in `\Z`, not `$`, so a pin's `commit:` or `tree_sha256:` that ends in a line break is refused as not 40 or 64 hex by every validator and `adopt-project.py` (#1017) `$` also matches just before a trailing line feed, and every caller reads these patterns with `.match`, so a `commit:` or `tree_sha256:` read as the hex and a line feed (the double-quoted escape for a line feed, or a block scalar) passed as well formed. On main, validate-manifest.py and validate-family.py passed one (`manifest ok`, `family ok`), validate-pins.py refused one for another reason (`pin-commit-unresolvable`, `pin-digest-mismatch`), and the assembly root's bootstrap.py, which holds a copy of the commit pattern as `COMMIT_40`, printed its upstream notice over one. `\Z` matches only at the very end of the value, so each caller's `.match` refuses a trailing line feed as it already refused a carriage return and line feed, and a space. Nothing else about the patterns changes: 40 and 64 digits, either case. Rows: tests/test_pin_pattern_line_break.py (the three patterns, the holder's gate, the upstream notice, the doctor's `pin_commit`), and a row each in test_pin_validator.py, test_manifest_validator.py, test_adopt_pins.py and test_shape_doctor.py. Each fails on main and passes with the change. Closes #1017 Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Reviewer's GuideThe PR fixes an anchoring defect where Python’s Flow diagram for strict pin validationflowchart LR
Input["commit or tree_sha256 value"] --> Pattern{Strict \Z pattern}
Pattern -->|"exact hex length"| Valid["Accepted"]
Pattern -->|"trailing line feed or other suffix"| Invalid["Rejected as malformed"]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Independent review of #1126 (#1017) at The fix is right. Nothing that worked before is refused now, and I found no other copy that needs the change. The only blocker is two composite asserts in the new test file, which SonarCloud has flagged on the PR. Splitting them is a two-line fix round. FindingsMUST: two composite asserts (fix round).
NIT: the body misstates what main does with
NIT: the merge-tree section is stale. Body lines 80-82 name
NIT: two callers have no row of their own, so a caller-side mutant survives (details under Rows and mutants). These callers are protected only through the shared pattern; that is what the PR changes, so they work. The gap is coverage, not a bug:
Checked and true1. The defect on main (
Per-tool outcomes on main, through the PR's own rows run against main's production files (17 failed, 319 passed in 330.72s):
A direct probe of 2. No caller relied on
No site where valid input is now refused. The only values that change are YAML values that end in a line feed. No tool writes one; it takes a double-quoted escape, or a block scalar written by hand. 3. Copies.
4. Rows and mutants. Run in a worktree at the predicted merge, with the new file (and target rows) only:
A caller's 5. Runs (all through
6. Title and body.
Sonar-style checks:
Could not verify
Follow-up assessment (item 7; input for the lane's issue, not findings against this PR)
Scratch left
Lane: openreposhape-5 (openRepoShape-5) |
…n's commit or digest, or a record's `source_commit:`, has a row, so `.rstrip` before the match in validate-pins.py or validate-manifest.py fails; each read asserts one value (#1017) Fix round 1 for #1126, from its independent review. `member_pin` and `holder_shape` ended in `assert commit and digest, text` (SonarCloud S9073): when one of the two reads found nothing, the message could not say which. Each value now has its own assert and a message that names it. Two callers of the patterns had no row of their own, so a `.rstrip("\n")` before the match survived every row: the neutral product's pin in validate-pins.py (`commit:` is refused as `neutral-pin-tag-only`, `tree_sha256:` is `neutral-pin-digest-malformed`) and the `source_commit:` of a `submodules:` or `answered_paths:` row in validate-manifest.py (`manifest-submodule-commit`, `manifest-answered-commit`). On main the first was the product's checkout refusing the name (`neutral-pin-source-unreadable`), the second a digest recompute (`neutral-pin-digest-mismatch`), the third `manifest ok`. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
Re-check of #1126 (#1017) fix round 1 at All five checks hold. There is no MUST or SHOULD, and two NITs, both about prose. The fix round does what rv1126 asked, and the SonarCloud S9073 issues are now closed. FindingsNIT 1: the test file's module docstring still says what the body no longer says.
NIT 2: the body's "Mutants" table is measured on the 42-row file. It reads 7/35, 4/38, 3/39 and 2/40, which sum to 42. The file now has 46 rows. I re-measured on the head:
Every pattern is still killed by a row of its own. M1 is the union of M2 and M3, so I did not run it. Nothing is wrong, the table is just stale. Main moved to Checked and true1. The MUST is fixed.
2. The four new rows.
No mutant survived. O5a and O5b show that both 3. Runs through
4. Merges.
5. The body's NITs.
Could not verify
Scratch I leave
Lane: openreposhape-5 (openRepoShape-5) |
Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…date-family.py` passed only the digest of `family.yaml`'s `shape:` on main, and gave `shape-pin-manifest-disagree` over the commit, not that it passed both (#1017) NIT 1 of rc1126's re-check of #1126 (issuecomment-6340973564). The docstring said validate-family.py passed `shape.commit` and `shape.digests.tree_sha256` in family.yaml on main. Only the digest passed (`family ok`, exit 0). The commit passed `COMMIT_RE` too, so `family-shape-commit` was not raised, but `contracts/shape-pin.yaml` holds the commit without the line feed, and the gate gave `shape-pin-manifest-disagree` with the same hex printed on both sides. The clause now says that. A docstring only: no code and no row changes. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
|
Re-check of #1126 (#1017) for lane openRepoShape-5: READY at I checked only the delta from Checked and true1. The merge is clean and adds nothing of its own.
2. The NIT commit
3. The new docstring is true of main.
3b. The mutants. NIT 2 of the same comment was the stale table. I applied each mutant alone to
4. Runs at
5. Merge-trees.
Body note (not a finding)The body's merge-tree bullet for fix round 2 is out of date:
Could not verify
Lane: openreposhape-5 (openRepoShape-5) |
|
Sorry @brettheap, you've used your own review budget of 250,000 diff characters for the last 7 days. You can request another review in 4 hours and 46 minutes by commenting |



Lane: openreposhape-5 (openRepoShape-5)
Claim: #1017 (comment)
Goal
scripts/repo_shape.pyendsCOMMIT_REandSHA256_REin$, and every caller reads them with.match. In Python$also matches just before a trailing line feed, so a pin'scommit:ortree_sha256:that ends in a line break passed as well formed. This makes both patterns refuse it, wherever a value is decided to be valid, and changes nothing else about them.What still held on main
Measured on main (
2a119f28, then re-checked on6da757cbe; the use sites of both patterns are the same on823624d93, and no new copy of either pattern was added). Python 3.12.3:.matchon mainCOMMIT_RE/SHA256_REA line feed reaches the patterns through YAML: the double-quoted escape
"<hex>\n"and a block scalar both read as the hex and a line feed. On main:validate-manifest.py:shape.commitandshape.digests.tree_sha256ending in a line feed ->manifest ok, exit 0.validate-family.py:shape.digests.tree_sha256infamily.yamlending in a line feed passed (family ok, exit 0).shape.commitdid not pass: it exited 1 withshape-pin-manifest-disagree, becausecontracts/shape-pin.yamlandfamily.yaml'sshape.commitdiffer only by the line feed, so the same 40 hex is printed on both sides; the finding that names the commit as not 40 hex,family-shape-commit, was not raised. A member pin'stree_sha256ended in a line feed gavemember-digest-mismatch, not the malformed finding, and a member pin'scommitgavemember-commit-unresolvable, notmember-pin-commit.validate-pins.py: a leg pin's commit gave exit 2pin-commit-unresolvable, notpin-tag-only; a leg digest gavepin-digest-mismatch, notpin-digest-malformed; a shape pin's commit gave onlyshape-pin-manifest-disagree, notshape-pin-commit.adopt-project.py check(_pin_row_problem): the row read as well formed, with noplan-pin-malformed.templates/assembly-root/scripts/bootstrap.pyholds a copy of the commit pattern,COMMIT_40, with the same defect:shape_upstream_noticeprinted its notice for a pin ending in a line feed.shape-doctor.pypin_commitreturned"<hex>\n"for such a leg pin.Checked and not affected:
bump-leg.py --toandfamily.py --at/--togo throughchecked_value(afullmatchthat already refuses a line feed);bump-leg.pyline 2212 strips first;update-shape.pyline 558 readsgit_out, which is stripped;shape-doctor'srecorded_gitlinksplits fields;src/openreposhape/standard.pyusesfullmatch. The@(?P<sha>[0-9a-fA-F]{40})patterns inbump-leg.pyandvalidate-pins.pyare unanchored workflow-ref patterns.What changed
scripts/repo_shape.py:COMMIT_REandSHA256_REend in\Z, not$(comment added).\Zmatches only at the very end of the value, so each caller's.matchrefuses a line feed as it already refused a carriage return and line feed, and a space. 40 and 64 digits, either case, are unchanged.templates/assembly-root/scripts/bootstrap.py:COMMIT_40likewise (comment added).AGENTS.mdtext pins the old pattern bytes (grepped), andtests/test_pinned_copy_text.pyand everytests/test_bootstrap*.pypass unchanged (shard tB and shard tA).tests/test_pin_pattern_line_break.py(46 rows: 42 at first, and 4 added in fix round 1, below): the three patterns x case, length, after-suffix (line feed, carriage return and line feed, space, two line feeds, carriage return, tab) and before-prefix; the holder's gate (four rows and a baseline); the bootstrap upstream notice (two rows); the doctor'spin_commit(two rows);tests/test_pin_validator.py: 3 rows (leg commit, leg digest, shape-pin commit);tests/test_manifest_validator.py: 2 rows (shape.commit, shape digest) and a helpershape_of;tests/test_adopt_pins.py: 2malformed()cases (commit-line-break,digest-line-break);tests/test_shape_doctor.py: 1ROOT_PIN_FAULTSparam (a-commit-ending-in-a-line-break).Rows that fail on main
Run with the two production files of main checked out, on a head with the same tree as this one (git 2.43.0,
EMAIL=service@opensoft.dev):test_pin_validator.py,test_manifest_validator.py,test_adopt_pins.py: 16 failed, 307 passed in 294.83s. The 16:line-feed-COMMIT_RE,line-feed-SHA256_RE,line-feed-bootstrap-COMMIT_40,test_a_member_pin_commit_that_ends_in_a_line_break_is_refused,test_a_member_pin_digest_that_ends_in_a_line_break_is_malformed,test_a_family_shape_commit_that_ends_in_a_line_break_is_a_finding,test_a_family_shape_digest_that_ends_in_a_line_break_is_a_finding,test_the_upstream_notice_is_silent_over_a_pin_that_ends_in_a_line_break,test_the_doctor_reads_a_pins_commit_as_the_validators_do[40-hex-and-a-line-feed];test_pin_validator.py(3): a commit, a digest and a shape-pin commit that end in a line break;test_manifest_validator.py(2): a shape commit and a shape digest;test_adopt_pins.py(2):plan-pin-malformed-commit-line-break,plan-pin-malformed-digest-line-break;test_shape_doctor.py -k a_root_pin_that_does_not_read: 1 failed, 12 passed, 436 deselected in 57.84s (a-commit-ending-in-a-line-break).With the change the same files pass (see the runs).
Mutants (the new file only,
-rf)repo_shapepatterns back to$COMMIT_REback to$SHA256_REback to$COMMIT_40back to$Re-measured in fix round 2, at
4c1f80cf, on the 46-row file. The first round measured its 42 rows: 7/35, 4/38, 3/39 and 2/40. M1 is M2 and M3 together.Each is killed by at least one row, and each pattern is killed by a row of its own. Every mutant was undone: the production files are byte for byte (
cmp) their backups of this change.Runs with the change (head
ae30dbd0e0eecfb7092c5ef257da2367d114fc62, worktree clean at each START)git 2.43.0 (
/usr/bin/git):tests/test_repo_hygiene.py tests/test_agents_md_codes.py tests/test_cli_reference.py): 170 passed in 37.48s, rc=0.The two shards are 74 targeted files: every file that names a pin code (
pin-tag-only,pin-digest-*,shape-pin-*,manifest-shape-*,family-shape-*,member-pin-*,update-pin-*,neutral-pin-*,plan-pin-*,not 40/64 hex) or one ofCOMMIT_RE,SHA256_RE,COMMIT_40, everytests/test_bootstrap*.py(13 of 13),test_shape_doctor.py, 6 of the 19test_update_shape*.pyfiles,test_family.py,test_scaffold_e2e.py, and the pin tests. They replaced a wide run (226 importer files, about 27k tests) that was killed by PID under load (load average 72 on 32 cores, 2% in 11 minutes); that run is not evidence. The new file and the four edited files are all inside the shards. The other 13test_update_shape*.pyfiles, which name no pin code, ran as a third run (480 passed, 3 skipped in 441.06s, rc=0), andtests/test_scaffold_leg_path_collisions.pyas a fourth (104 passed in 4.35s, rc=0). No full suite was run.git 2.47.1 (
git --versionprinted first in each log):test_pin_validator.py,test_manifest_validator.py,test_adopt_pins.py: 323 passed in 208.37s, rc=0.test_shape_doctor.py -k a_root_pin_that_does_not_read: 13 passed, 436 deselected in 52.85s, rc=0.Merge-trees
Refreshed after the fix round, against the re-fetched origin/main:
038dafba9dbedd9405b316cc749a04dffe02bb36(tests/test_path_guard_hygiene.py and shape-doctor.py: a bare is_file/is_dir/exists guard in a shipped tool fails by file and line unless its_is_aor a reasoned allowlist covers it, and aproject.yamllinked into an unsearchable folder isyaml-unreadable(#430) #1111) is merged into the branch asc7da27dc2, cleanly.4c1f80cf4db6215d8861bd43cd57f72df957d82amerges with main at its own tree,9884a99aeba2a55e486a355ae3483d8c932b4f68: the seven files of this PR.checkandexecuteask a plan'sorg:andid:right after its three repository names, so a bad one beside an unanswered entry, a moved source, a refused name or another repository is that one refusal in both, and apins:entry that is no pin is said as_read_assays it (#1020) #1125 (head2a53a8e6) still touchestests/test_adopt_pins.py, and its merge-tree with this head is clean, treea026a192669a86abfaed59fbcce2fd1179bb0894.git merge-tree --write-tree origin/main HEADagainst4ed7c7bf581dceb75f816f30f76b776c2b8917f0(merge base6da757cbe), with this PR's head03b2b97b9d3cccc42b4703cb2f7c978693488266: clean, rc=0, tree95d242489e6a8d6ea2be8c454a5e6655922d5712. The difference between that tree and origin/main is exactly the seven files of this PR.6da757cbe. Since then three PRs landed. scaffold-project.py: the--reuse-empty-repohelp, its docs and each remedy that sends a person into that re-run say ssh, a credential helper or askpass can still make it wait, and how to keep it from waiting (#915) #1101 and templates/assembly-root/project.yaml and templates/family-root/family.yaml: the comment abovevisibility:says it is the visibility the run asked for, and that a repository the run reuses keeps its own, where it said "this is all three repositories' visibility" (#913) #1102 touchedREADME.md,docs/cli.md,docs/handbook.html,scaffold-project.py,templates/assembly-root/project.yaml,templates/family-root/family.yaml,tests/readme-cap.d/915.txt,tests/test_scaffold_reuse_seeded_legs.pyandtests/test_visibility_field_words.py. adopt-project.py:checknames what the clone's.githolds as git keeps refs, and asks a pin's file through the links the root keeps, so a link into.gitor onto a pin's file is refused before any leg or adopted with the copy beside it (#904) #1091 (4ed7c7bf5) touchedadopt-project.py,tests/test_adopt_root_shape_paths.pyandtests/test_adopt_root_symlinks.py. None is a file of this PR, andadopt-project.pyholds the same 8 uses ofCOMMIT_REandSHA256_REas before.gh pr list --state open --json number,headRefName,headRefOid,files, this one included), one other touches a file of this PR: adopt-project.py:checkandexecuteask a plan'sorg:andid:right after its three repository names, so a bad one beside an unanswered entry, a moved source, a refused name or another repository is that one refusal in both, and apins:entry that is no pin is said as_read_assays it (#1020) #1125 (head2a53a8e64a0cce17ab84cd1bf5949bcb6fbd4484) touchestests/test_adopt_pins.py.git merge-tree --write-tree 2a53a8e64a0cce17ab84cd1bf5949bcb6fbd4484 HEADis clean, rc=0, tree5fbd1eaafdf94004ead7142ae9132dc79dc4d1e5.Fix round 1
One new commit on top,
03b2b97b9d3cccc42b4703cb2f7c978693488266, after the independent review ofae30dbd0e0eecfb7092c5ef257da2367d114fc62(not ready on one must). It changestests/test_pin_pattern_line_break.pyand nothing else; no production file changes.Composite asserts (SonarCloud
python:S9073, the two issues at lines 164 and 173).member_pinandholder_shapeended inassert commit and digest, text. Each value now has its own assert, with a message that names it (fixture drift: no member pin commit,no member pin tree_sha256,no shape commit,no shape tree_sha256).Two callers had no row of their own, so a
.rstrip("\n")before the match in either survived every row. The new rows (the file now has 46):test_a_neutral_pin_commit_that_ends_in_a_line_break_is_refused: the neutral product's pin invalidate-pins.py, exit 2,REFUSED neutral-pin-tag-only. On main the commit passedCOMMIT_REand the run exited 1 withneutral-pin-source-unreadable, git refusing the name;test_a_neutral_pin_digest_that_ends_in_a_line_break_is_malformed: the same pin'stree_sha256, exit 1,neutral-pin-digest-malformed. On main it was recomputed and compared (neutral-pin-digest-mismatch);test_a_recorded_source_commit_that_ends_in_a_line_break_is_a_finding, two rows (submodules,answered-paths): a record row'ssource_commit:invalidate-manifest.py, exit 1,manifest-submodule-commitandmanifest-answered-commit. On main:manifest ok.The four rows fail on main. The new file with main's two production files checked out: 13 failed, 33 passed (the nine of the first round, and these four).
Mutants, each applied alone to the committed head, the file restored afterwards (sha256
94f125c5f5e4b8a0forvalidate-pins.pyand260c4742fe422597forvalidate-manifest.py, the same as before the mutant, and clean against HEAD;repo_shape.pyandbootstrap.pyunchanged at623af12131447f95and4102d375f4c9bc38). The new file only:.rstrip("\n")on the neutral pin's commit before the match (validate-pins.pyline 1790): 1 failed, 45 passed, killed by the neutral-commit row;.rstrip("\n")onsource_commitbefore the match (validate-manifest.pyline 1264): 2 failed, 44 passed, killed by bothsource_commitrows;Runs, at head
03b2b97b9d3cccc42b4703cb2f7c978693488266, worktree clean at each START:tests/test_pin_validator.pyandtests/test_manifest_validator.py: 175 passed in 58.03s, rc=0;tests/test_neutral_pin_validator.py(the new rows use its helpers): 39 passed in 99.62s, rc=0;git --versionprinted first): the new file: 46 passed in 19.32s, rc=0.ae30dbd0e; the two production files and the four other test files are the same at this head.Fix round 2
This round is one merge of main and one new commit, after rc1126's re-check: READY at
03b2b97b, with two prose NITs (#1126 issuecomment-6340973564).038dafba(tests/test_path_guard_hygiene.py and shape-doctor.py: a bare is_file/is_dir/exists guard in a shipped tool fails by file and line unless its_is_aor a reasoned allowlist covers it, and aproject.yamllinked into an unsearchable folder isyaml-unreadable(#430) #1111), asc7da27dc2, with no conflict.4c1f80cf4changes only the module docstring oftests/test_pin_pattern_line_break.py. It now says what main did:validate-manifest.pypassed both ashape.commitand ashape.digests.tree_sha256that end in a line feed;validate-family.pypassed only the digest. Its commit gaveshape-pin-manifest-disagreewith the same hex on both sides, becausecontracts/shape-pin.yamlholds the commit without the line feed.4c1f80cf4, git 2.43.0:tests/test_path_guard_hygiene.py: 95 passed (46 + 49);Not changed
Other
^...$patterns are used with.matchon values read from files or arguments:PROJECT_ID_RE,DATE_REandQUALIFIED_RE(validate-manifest.py,validate-family.py),REPOSITORY_RE(update-shape.py),PIN_ARG_RE(shape_materialize.py), andNAME_RE,REF_REandPATH_RE(setup-project.py). They have the same$property and are outside the two patterns this issue names, so they are left for #1139, filed from this PR's review.Closes #1017
🤖 Generated with Claude Code
Summary by Sourcery
Require pin identifiers and tree digests to end exactly at the expected hexadecimal value, preventing trailing line breaks from being accepted as valid pins.
Bug Fixes:
Enhancements:
Tests: