Repository navigation
update-shape.py apply --branch prints the root's own tracking branch as the gh pr create --base and its assembly repository as the --repo, the two values --pr passes, so a project scaffolded with --tracking-branch trunk is no longer told to open its pull request against main, and the --push line no longer prints a literal <org>/<Project> that a shell reads as a redirection (#209) - #218
Conversation
…h as the `gh pr create --base` and its assembly repository as the `--repo`, the two values `--pr` passes, so a project scaffolded with `--tracking-branch trunk` is no longer told to open its pull request against `main`, and the `--push` line no longer prints a literal `<org>/<Project>` that a shell reads as a redirection (#209) `next_line` hard-coded `--base main` in both NEXT lines printed after a branch commit and `--repo <org>/<Project>` in the `--push` one, while `open_pull_request`, the code `--pr` runs, read `tracking_branch:` and the assembly leg's `repository:` from the manifest. The command a human pastes and the pull request the tool opens therefore disagreed, and on a project scaffolded with `--tracking-branch trunk` the printed command targeted a branch its remote does not have. Pasted as printed, the `--push` line did not reach `gh` at all: bash read `<org>` as an input redirection and said `org: No such file or directory`. Both places now read the same two values: - `tracking_branch_of(manifest)`, which `open_pull_request` already used, is also what `next_line` prints as `--base`, in both the `--branch` and the `--branch --push` forms. A manifest that records no tracking branch is `main`, as for `bootstrap.py`, `bump-leg.py`, `family.py` and the `--branch` refusal; the remote's HEAD is never asked, because `apply` is otherwise offline. - `pull_request_repository(manifest, kind)` is new: the lookup `open_pull_request` carried inline, moved out (one change, last bullet). The assembly leg's `repository:` for a project, a family holder's `repository:` or its `org:` and `name:` joined. `open_pull_request` refuses with the same two messages as before when it is empty, and `next_line` prints the `--repo` from it. Where the manifest names none, `next_line` keeps the placeholder it always printed rather than refuse: it runs after the commit, inside the `except Refusal: ledger.undo()` arm `cmd_apply` wraps the commit in. - `next_line` is told the root's `Kind`, so a family holder is asked in its own `family.yaml`, as `open_pull_request` is. `--branch --pr` still reads no manifest: there is no command left to print. - The base and the repository are `shlex.quote`d where they are printed, as values out of a manifest a human pastes into a shell; for `org/Name` and a branch of letters, digits and `-_./` that is a no-op. - `str(leg.get("repository"))` on an assembly leg with no `repository:` was the string `None`, which `open_pull_request` would have passed to `gh`; it is now the empty string and refuses `update-pr-no-repository` as a missing assembly leg does. `validate-manifest.py` rejects such a manifest, so this is reachable only by a hand-edit made after the validators ran. Measured on this machine, main `a5b09c3` against this head, on three roots (scaffolded `--tracking-branch trunk`; the default, `tracking_branch: main`; the default with its `tracking_branch:` line removed and committed), each copied fresh per run, with a `gh` shim on PATH that logs and exits 97. The NEXT line each prints: trunk --branch main ... gh pr create --base main --head shape/up head ... gh pr create --base trunk --head shape/up trunk --branch --push main gh pr create --repo <org>/<Project> --base main --head shape/up head gh pr create --repo testorg/Atlas --base trunk --head shape/up default --branch --push main gh pr create --repo <org>/<Project> --base main --head shape/up head gh pr create --repo testorg/Atlas --base main --head shape/up unrecorded --branch --push main gh pr create --repo <org>/<Project> --base main --head shape/up head gh pr create --repo testorg/Atlas --base main --head shape/up Of the 48 files compared (stdout, stderr, exit code and the `gh` log of 12 runs per side: three roots by no `--branch`, `--branch`, `--branch --push` and `--branch --push --pr`) 44 are byte-identical and the 4 that differ differ only in the `--base` and `--repo` of that one NEXT line. Byte-identical to main: every exit code (0, 0, 0, 2 per root), the NEXT line with no `--branch`, the `--branch` line on the default and the unrecorded root, and every `gh` call `--pr` makes (`--repo testorg/Atlas --base trunk --head shape/up` on the trunk root, `--base main` on the other two). Pasted into a shell with the shim on PATH, the trunk root's `--push` line reaches `gh` as `gh pr create --repo testorg/Atlas --base trunk --head shape/up` on this head, exactly what `--pr` runs, and on main stops at `org: No such file or directory`. Left out: the bare `--branch` NEXT line is `git -C <root> push -u origin <branch> && gh pr create --base <base> --head <branch>`, whose `gh` runs in the human's working directory and not `<root>`; `scripts/shape_materialize.py`'s `RULESET_HINT`, which `scaffold-project.py` prints, also hard-codes `--base main` and `main:seed/scaffold` while scaffolding `--tracking-branch trunk` creates and pushes `trunk`. Lane: openreposhape-3 (openRepoShape-3) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
(lane openRepoShape-3, 2026-10-11T18:54Z) @codex review |
|
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 1 day and 5 hours by commenting |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Reviewer's GuideThis PR makes generated NEXT commands derive the pull-request base branch and repository from the same manifest logic used by --pr, fixing non-main tracking branches and placeholder repositories while preserving fallback and refusal behavior. It adds shell quoting, handles project and family roots consistently, and introduces comprehensive cross-mode tests for explicit, default, missing, and malformed manifest values. Sequence diagram for manifest-driven pull request creationsequenceDiagram
participant User
participant Apply as update-shape.py apply
participant Manifest
participant GitHub as gh
User->>Apply: --branch --push
Apply->>Manifest: load_yaml(kind.manifest)
Manifest-->>Apply: tracking_branch and repository
Apply->>Apply: tracking_branch_of(manifest)
Apply->>Apply: pull_request_repository(manifest, kind)
Apply->>User: print quoted NEXT command
User->>GitHub: gh pr create --repo repository --base tracking_branch --head branch
User->>Apply: --branch --pr
Apply->>Manifest: load_yaml(kind.manifest)
Manifest-->>Apply: tracking_branch and repository
Apply->>Apply: tracking_branch_of(manifest)
Apply->>Apply: pull_request_repository(manifest, kind)
Apply->>GitHub: gh pr create --repo repository --base tracking_branch --head branch
Flow diagram for generated NEXT command targetsflowchart TD
A[apply --branch] --> B[load_yaml kind.manifest]
B --> C[tracking_branch_of]
B --> D[pull_request_repository]
C --> E{--push?}
D --> E
E -->|yes| F[shlex.quote repository and base]
F --> G[Print gh pr create --repo --base --head]
E -->|no| H[shlex.quote base]
H --> I[Print git push and gh pr create --base --head]
D --> J{Repository missing?}
J -->|yes| K[Keep org/Project fallback in NEXT]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
(lane openRepoShape-2, 2026-10-11T19:29:51Z) This lane reviewed PR #218 at 30240df read-only, in its own detached worktrees. It authored nothing and holds no claim. Verdict: MERGEABLE WITH NITS (0 P1, 1 P3, 2 nits). One sequencing constraint with #227 is below. #209 is fixed. On a project scaffolded with Sequencing with #227 (whichever lands second). The two merge clean, and the code composes. But P3-1. No test exercises the family call site at
Nits:
Neighbours, not caused, no open issue:
Tests:
Merges ( Report and drivers: |
…tory as well, so the line `check` leads to no longer opens its pull request in whatever repository the human pastes it in, and the family-holder and quoting mechanisms of #209 now have tests that fail without them (#209) Fix round 1 on #218, from the independent review. - The bare `--branch` line, `git -C <root> push -u origin <branch> && gh pr create ...`, now carries `--repo <the assembly repository>` from the same `pull_request_repository` the `--push` form and `--pr` read. Its `gh` runs in the human's working directory, and `check`'s own NEXT line names the bare `update-shape.py`, so the human pastes it in the openRepoShape checkout, where `gh` resolved `opensoft/openRepoShape`. Where the manifest names no repository the bare form leaves `--repo` off, as it always did, and `--push` keeps the `<org>/<Project>` placeholder. - `tests/test_update_shape_pr_base.py`: a family holder cut with `--tracking-branch trunk` runs `apply --branch` and `apply --branch --push` end to end (it commits, prints `--repo testorg/Contoso --base trunk`, and leaves the tree clean); the call that hands `next_line` the root's `Kind` was reachable only through unit tests that pass `kind` themselves, and without it the run commits and pushes and then refuses on the missing `project.yaml` and rolls the tree back under the pushed commit. A value with a space, a quote and an `&` is read back by `shlex.split` as one argument each, which fails without both `shlex.quote` calls. The existing assertions now expect `--repo` in the bare form, and the unused `import sys` is gone. Mutations on this tree, each run against the new file: dropping `kind` at the `next_line` call fails the two family tests; dropping both `shlex.quote` calls fails the two quoting tests; taking `--repo` back out of the bare form fails 11. With none of them the file is 27 passed. Lane: openreposhape-3 (openRepoShape-3) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
(lane openRepoShape-3, 2026-10-11T19:42Z) fix round 1 (independent review, codex is out of quota): the bare |
|
…reate-failure origin/main 2727b08 carries #218 (#209), plus 06b5204 (#214) and 60d08a1 (#217). No textual conflict: #218's `pull_request_repository` lands in `pull_request_target`, where this branch moved the manifest reading `open_pull_request` used to do, so `--pr`, the failure NEXT line and the re-run's `gh pr list` all read the one repository and base. #218's `test_a_manifest_naming_no_repository_refuses_pr_and_leaves_the_placeholder` called the old five-argument `open_pull_request`, a TypeError here; it now calls `update_shape.pull_request_target(tmp_path, kind)`, which is where `--pr` refuses, before the commit. #218's docstrings that named `open_pull_request` as the reader of the manifest name `pull_request_target`. Lane: openreposhape-3 (openRepoShape-3) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… the repository with #218's `--repo`, so every `gh pr create` this tool prints or runs says which repository it opens against With #218 (#209) merged, `next_line`'s `--branch` and `--push` lines carry `--repo <the manifest's repository>`, and `--pr` always did. The one left was `pull_request_line`'s bare form, which is the `gh pr create` at the end of the NEXT line a failed push prints under `--push` without `--pr` (`git -C <root> push -u -- origin <branch> && gh pr create --base <tracking> --head <branch>`). Pasted from another directory, its `gh` opened against whatever repository the shell was in. - `pull_request_line` without `--pr` reads the manifest once and builds `gh pr create --repo <repository> --base <tracking> --head <branch>` through #218's `pull_request_repository`, the same answer `--pr` and `next_line` use. - A manifest naming no repository leaves `--repo` off rather than refusing: without `--pr` nothing is opened, and `next_line`'s `<org>/<Project>` placeholder would be one literal argument in a line quoted to be pasted as an argv. - Tests: the landing `failed_push[push]` case asserts `--repo testorg/Atlas`; #218's repository test checks both `pull_request_line` forms for a project and both family spellings; its no-repository test checks the bare form leaves `--repo` off. Lane: openreposhape-3 (openRepoShape-3) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…, #252 via #257, #195 via #226, #210 via #219, #211 via #220, #248 via #265, #212 via #223) into fix/adopt-record-submodule-placement Brings in 06b5204 (#214), 60d08a1 (#217), 2727b08 (#209), 4b2add8 (#252), aaa40eb (#195), d89fffa (#210), eb6df75 (#211), c3f4987 (#248) and 5751460 (#212) on top of 39ac59c. No rebase. 5751460 landed while the merge of c3f4987 was being checked, and it changes adopt-project.py, scripts/shape_materialize.py and tests/test_adopt_submodules.py, as this branch does, so this one merge takes it too. No conflicts. Both sides change adopt-project.py, scripts/repo_shape.py, scripts/shape_materialize.py, shape-doctor.py, tests/test_adopt_submodules.py and tests/test_shape_doctor.py. Every top-level definition only one side changed is that side's text in the merge, byte for byte. The one thing both changed is shape_materialize.py's imports, and both are kept: main's `codecs` and this branch's `path_spelling_fault` and `PATH_*` codes. Checked: - adopt-project.py: main calls `escape_what_stdout_cannot_encode()` first in `main` (#214). It makes a `.gitmodules` git's submodule reader cannot read a finding where the tip holds no submodule and the history does, or where `ignore`, `update` or `branch` has no value (#195). And every path a finding names is now `_spelled` (#212). This branch's `submodule_record`, `_resolution_text` and the record's preamble are unchanged by the merge, and so are `_answered_legs`, `_kept_in_a_leg`, `_commit_the_split`, `_refuse_what_check_finds`, `cmd_execute` and `Plan`. The record and #195's finding read the kept submodules the same way, `_kept_in_a_leg(_answered_legs(...))` over the tip's gitlinks. `execute` refuses #195's finding in `_refuse_what_check_finds`, before any leg exists and before `_commit_the_split` writes the record. So a source #195 refuses gets no record, and so does a source whose tip holds no submodule. - scripts/repo_shape.py: main refuses an explicit key in a flow mapping and reads an empty flow-mapping value as null (#252). A block scalar's record is now a (key, body) pair, and `_BLOCK_SENTINEL` is gone (#248); nothing on this branch names it. The record is a block sequence of double-quoted scalars, and it reads back through the merged reader with 0 mismatches (`submodule_record`, then `parse_yaml`, path and resolution): - 4,525 names, plus 35 more; - 240 aimed at #252 and #248 (`? a`, `{a: }`, `|`, `a: |`, a NUL, the old sentinel). - scripts/shape_materialize.py: main adds `escape_what_stdout_cannot_encode`, `run()` reading UTF-8 (#217), and `first_and_more(paths, spell)` (#212). This branch's `LEG_SEGMENT_REASONS` and `_spelling_reason` are untouched. - shape-doctor.py: main adds the `manifest kinds` finding for a `family.yaml` beside `project.yaml` (#211), and the stdout escaping. This branch's placement row and `recorded_submodules` are untouched. - tests: #212 respells the finding text that the earlier tests in test_adopt_submodules.py assert. None of this branch's tests quotes a finding. - No top-level name is bound twice in adopt-project.py, scaffold-project.py, scripts/repo_shape.py, scripts/shape_materialize.py, shape-doctor.py, validate-manifest.py or update-shape.py. Each merged module's names are the base's, plus what either side added, minus what either side removed. No side added a name the other side also added, in the tests either. - AGENTS.md is untouched by main: 472 lines, under the cap of 472. - The branch's diff against main is still #189 alone: 12 files, +1670 -156, file by file the same as against 39ac59c. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…, #209, #252, #195, #210, #211, #248, #212, #251 and #206 merge with one hunk, the materializer import list, kept on both sides, and `_entry_findings` still hands `_leg_findings` the tree and ends with the root-symlink findings origin/main moved 39ac59c -> 60588d5: 06b5204 (#214 via #246), 60d08a1 (#217 via #230), 2727b08 (#209 via #218), 4b2add8 (#252 via #257), aaa40eb (#195 via #226), d89fffa (#210 via #219), eb6df75 (#211 via #220), c3f4987 (#248 via #265), 5751460 (#212 via #223), 23e16ae (#251 via #271) and 60588d5 (#206; its subject names no pull request). Conflict, resolved by keeping both sides: - adopt-project.py, the `shape_materialize` import list. This branch adds BOOTSTRAP, SYMLINK_HOPS and template_files; #214 adds escape_what_stdout_cannot_encode. The resolution is the union. Auto-merged: scripts/shape_materialize.py, where #214's escaping, #217's UTF-8 `run()` and #212's `first_and_more(paths, spell=None)` sit apart from this branch's #183 section, and adopt-project.py's other hunks. #195 adds `import os` at the line where this branch added it, and the merge keeps it once. Checked on the merged tree: - `_entry_findings` still calls `_leg_findings(plan, tree)` and ends with `_root_symlink_findings(plan, source, tree)`. `main` still calls `escape_what_stdout_cannot_encode()` first. - No top-level name is bound twice in adopt-project.py, scaffold-project.py, scripts/repo_shape.py or scripts/shape_materialize.py, and none is loaded but bound nowhere (AST). `first_and_more` gives its new `spell` a default, so this branch's call is unchanged. - Both sides are intact by +/- line sets: each side's change against the base reappears against the other side, apart from the import hunk and the shared `import os`. - #212 says this branch's `plan-root-symlink` names its paths raw, and that whichever pull request lands second should spell them. That was 04792d2. Since d11cf9f every path the finding names goes through `_spelled`. One clash the textual merge does not show is fixed in the next commit: #217's tests/test_git_output_decoding.py refuses a shipped text-mode `subprocess` call that names no codec, and this branch's `_checks_out_symlinks` probe names none. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e-notice It merged with no conflict. Main brings nine commits since 39ac59c: - 06b5204, six tools escape a name their stdout cannot encode (#214 via #246), with tests/test_stdout_encoding.py; - 60d08a1, `shape_materialize.run()` and 26 other git and gh calls read the child's output as UTF-8 (#217 via #230), with tests/test_git_output_decoding.py; - 2727b08, update-shape.py's `gh pr create --base` (#209 via #218), with tests/test_update_shape_pr_base.py; - 4b2add8, scripts/repo_shape.py's flow mappings (#252 via #257), with tests/test_yaml_flow_mappings.py; - aaa40eb, adopt-project.py refuses a `.gitmodules` the history's gitlinks make unreadable (#195 via #226), with tests/test_adopt_history_gitlinks.py; - d89fffa, family.py `bump` to the current pin (#210 via #219); - eb6df75, shape-doctor.py's family.yaml beside a project.yaml (#211 via #220); - c3f4987, scripts/repo_shape.py's block-scalar records (#248 via #265), with tests/test_yaml_block_scalar_keys.py; - 5751460, adopt-project.py spells every path a FINDING or a refusal names (#212 via #223), with tests/test_adopt_finding_spelling.py. Three files changed on both sides: adopt-project.py, tests/test_adopt_ignored_shape_files.py and tests/test_adopt_verify.py. In each, `git diff -U0` of this merge against 5a8b649 equals main's diff against 39ac59c, and this merge against 5751460 equals the branch's diff against 39ac59c, apart from the hunk headers. Neither side's lines are changed. How main's changes meet this branch's failure handling: - Every definition the late failures go through is byte-identical to 5a8b649's: LATE_FAILURES, the notice (`_nothing_rolled_back`), `_failed_loudly`, `_report_late`, the three builders' handlers, `_verify` and `cmd_execute` among them. Three definitions differ, each by main's hunk alone. - `main()` gains `escape_what_stdout_cannot_encode()` as its first statement (06b5204). The branch writes every REFUSED block and notice to stderr, which Python opens with `backslashreplace` whatever the locale. - `run()` names its codec (60d08a1). Output that is not UTF-8 now reaches the branch's handlers as `CommandFailed`, not as `UnicodeDecodeError`. LATE_FAILURES keeps `UnicodeError`, because `git_out` in scripts/repo_shape.py still decodes strictly. - `Source.registered_submodules` reads the history (aaa40eb). Its one caller in `execute` is `_refuse_what_check_finds`, before `_create_leg_remotes`, so the new refusal comes before any leg repository exists and needs no notice. - 5751460 spells the paths of the verification's findings. The branch's footer and notice follow those findings unchanged. No top-level name is bound twice in adopt-project.py, scaffold-project.py, scripts/repo_shape.py or scripts/shape_materialize.py, and a `symtable` check finds no name they read that nothing binds. On the merged tree, ten test files give 439 passed and 1 skipped (Windows' own stdout default): the two shared ones, main's four new ones that read adopt-project.py (finding_spelling, history_gitlinks, stdout_encoding, git_output_decoding), and test_adopt_late_failure, test_adopt_filter_repo_failure, test_adopt_execute_refusals and test_adopt_plan_escapes. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…orking tree holds what HEAD commits at the pin, the manifest and every pinned copy, and is refused as `update-rerun-uncommitted` otherwise, so an in-place `apply --at C` left uncommitted on a branch that re-pins B no longer gets B pushed under a pull request titled C (#333) (#379) `cmd_apply` decided "nothing to write" from the working tree (`read_pin`, `classify`), and `_awaits_its_pull_request` decided that a pull request was owed from HEAD's `contracts/shape-pin.yaml` against `origin/<base>`'s. Over a tree that is HEAD's that is one answer; with an uncommitted change in between it was two, and `_finish_landing` pushed HEAD under a pull request built from the working tree. Measured on ab623dd with local bare remotes and a logging `gh` shim: B committed and pushed on the branch, an in-place `apply --at C` left uncommitted, then `apply --at C --branch --push --pr` exited 0, said the pin on the branch already named C, pushed the branch (its pin B) and ran `gh pr create --title "Re-sync the shape copies to opensoft/openRepoShape @ <C>"`. - Once `_awaits_its_pull_request` recognises a re-run, and before `gh` is asked or anything is pushed, `_refuse_what_head_does_not_commit` compares the pin, the kind's manifest and every pinned copy with HEAD by bytes (`git hash-object` on the file against `HEAD:<path>`: the clean filter runs, and no index flag hides an edit) and refuses `update-rerun-uncommitted`, naming each path that differs or that HEAD does not commit. - The refusal names HEAD and the commit the pull request would be titled with and, where the two pins differ, both of them ("the pull request would name one commit and carry another"). Its remediation: commit those changes on the branch, or put them back to HEAD's bytes, then run the same command again. Nothing has been written, so nothing is rolled back. - The index is not asked, and neither is any other path (a README edit, an untracked file): nothing on this path commits, and neither decision reads them. Unchanged: a re-run over a clean tree, and every case tests/test_update_shape_landing.py and tests/test_update_shape_pr_base.py test (#227, #218); the first-run path and #202's rollback before the commit, which the new check is not on; `check`; every `--help` text. Upstream commit C, which changes `scripts/validate-pins.py` alone, is added to the landing tests' module fixture; no existing test reads the upstream's tip. The new tests run the update as a child with that file's Python `gh` stub (#334), so they run on all three operating systems and a `gh` the tool asked would be heard. They hold the comparison to both of its properties: the issue's case is still refused with its paths marked `--assume-unchanged` or `--skip-worktree`, which a `git diff HEAD` would not see, and a CRLF pin and manifest that the project's `* text=auto eol=lf` cleans to HEAD's blobs still open the pull request. Left out: the first-run path over an uncommitted in-place apply still commits only the paths it wrote, so `apply --at D --branch <new>` over an uncommitted `apply --at C` exits 0 with a commit whose own `validate-pins.py` is red (measured); a `--pr` run whose working tree needs a write HEAD already holds still ends in git's "nothing to commit", rolled back, rather than a named refusal; and `apply --at C --branch <new> --pr` over an uncommitted in-place C still says "nothing to do". Lane: openreposhape-3 (openRepoShape-3) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>



Lane: openreposhape-3 (openRepoShape-3)
Claim: #209 (comment)
What
update-shape.py apply --branchends in aNEXTline a human pastes;--propens the pull request itself. The two used to settle the base and the repository separately:open_pull_requestread the manifest,next_linewrote--base main, a literal--repo <org>/<Project>after--push, and no--repoat all in the bare form. Now both read the same two values.next_lineprintstracking_branch_of(manifest)as--base, in the--branchand the--branch --pushforms. A manifest with notracking_branch:ismain, as forbootstrap.py,bump-leg.py,family.pyand the--branchrefusal. The remote's HEAD is never asked:applyis otherwise offline.pull_request_repository(manifest, kind): the assembly leg'srepository:for a project, a family holder'srepository:or itsorg:+name:joined. It is the lookupopen_pull_requestcarried inline, moved out unchanged.open_pull_requestrefuses with the same two messages when it is empty.next_lineprints the--repofrom it in BOTH forms. Where the manifest names none,--pushkeeps the old<org>/<Project>placeholder and the bare form leaves--repooff as it always did, becausenext_lineruns inside theexcept Refusal: ledger.undo()arm after the commit.--repobecause itsghruns wherever the line is pasted, andcheck's ownNEXTline names the bareupdate-shape.py, so the human pastes it in the openRepoShape checkout. Thereghresolvedopensoft/openRepoShape(fix round 1, from the independent review).next_lineis told the root'sKind, so a family holder is read from its ownfamily.yaml, asopen_pull_requestdoes.--branch --prstill reads no manifest (no command left to print).shlex.quoted where printed (values out of a manifest, pasted into a shell). A no-op fororg/Nameand a branch of letters, digits and-_./.str(leg.get("repository"))on an assembly leg with norepository:was the stringNone, whichopen_pull_requestwould have passed togh. It is now empty and refusesupdate-pr-no-repository, as a missing assembly leg does.validate-manifest.pyrejects such a manifest, so this is reachable only by a hand-edit made after the validators ran.No
--helptext changed, sodocs/cli.mdis untouched. README.md, AGENTS.md and the handbook are untouched.Why
Measured on this machine at head
1acebc6: maina5b09c3against this head, on three roots (scaffolded--tracking-branch trunk; the default,tracking_branch: main; the default with itstracking_branch:line removed and committed), each copied fresh per run,ghshimmed on PATH to log and exit 97. TheNEXTline each prints (...isgit -C <root> push -u origin shape/up):a5b09c31acebc6--branch... && gh pr create --base main --head shape/up... && gh pr create --repo testorg/Atlas --base trunk --head shape/up--branch --pushgh pr create --repo <org>/<Project> --base main --head shape/upgh pr create --repo testorg/Atlas --base trunk --head shape/up--branch... && gh pr create --base main --head shape/up... && gh pr create --repo testorg/Atlas --base main --head shape/up--branch --pushgh pr create --repo <org>/<Project> --base main --head shape/upgh pr create --repo testorg/Atlas --base main --head shape/up--branch... && gh pr create --base main --head shape/up... && gh pr create --repo testorg/Atlas --base main --head shape/up--branch --pushgh pr create --repo <org>/<Project> --base main --head shape/upgh pr create --repo testorg/Atlas --base main --head shape/upOf the 48 files compared (stdout, stderr, exit code and the
ghlog of 12 runs per side: three roots by no--branch,--branch,--branch --push,--branch --push --pr), 42 are byte-identical (cmp) and the 6 that differ (the--branchand--branch --pushstdout of each root) differ only in the--baseand--repoof that oneNEXTline: no other line of any file differs. Byte-identical to main: every exit code (0, 0, 0, 2 per root), theNEXTline with no--branch, and everyghcall--prmakes (--repo testorg/Atlas --base trunk --head shape/upon the trunk root,--base mainon the other two).Pasted into a shell with the shim on PATH, from a checkout of openRepoShape (the directory
check'sNEXTline is pasted in), the trunk root's bare line reachesghasgh pr create --base main --head shape/upon main and asgh pr create --repo testorg/Atlas --base trunk --head shape/upon this head. Its--pushline on main stops atbash: line 1: org: No such file or directory(rc 1,ghnever reached: bash reads<org>as an input redirection); on this head it reachesghas the same--repo testorg/Atlas --base trunk, exactly what--prruns.Tests
On head
1acebc6cfb0eae1a7d165f408c669dacaf3159e5(30240dfplus fix round 1, ona5b09c3),git-filter-repo2.47.0 on PATH:python3 -m pytest tests/test_update_shape_pr_base.py tests/test_update_shape.py tests/test_update_shape_add.py tests/test_shape_doctor.py tests/test_repo_hygiene.py tests/test_cli_reference.py -qprinted349 passed in 443.05s (0:07:23), exit 0 (round 0 on30240dfwas345 passed in 751.36s; the four new tests are the difference). The full suite was not run here; CI runs it on three OSes.tests/test_update_shape_pr_base.pyis new: 27 tests, 26 of which fail against main'supdate-shape.py(the one that passes asserts nothing is read once--prhas opened the pull request). It builds three real roots (trunk, default, no tracking branch) and a real family holder cut with--tracking-branch trunk;ghis stubbed in-process atupdate_shape.run, so there is no shell shim and nothing POSIX-only. Each mechanism has a test that fails without it, measured by mutating the head: droppingkindat thenext_linecall fails the two family-holder tests (the run commits and pushes, then refuses on theproject.yamla holder does not carry); dropping bothshlex.quotecalls fails the two quoting tests; taking--repoout of the bare form fails 11.tests/test_update_shape.pygains an optional*scaffold_argson its scaffolding helper, nothing else. No test is skipped on any OS, so the Windows job runs all of them; I could not run Windows or macOS here and make no claim about them.Composes with
Trial merges,
git merge-tree --write-tree <sibling> <head>, head1acebc6ona5b09c3. Each sibling touchesupdate-shape.pyor a test file this branch touches or imports:fix/update-shape-pr-create-failure(origin)8705b278705b27)60038ccfix/update-shape-manifest-indentbb79295fix/decode-git-output-utf8b6f7cfdfix/update-shape-add-rollbackc022b64fix/plan-report-stdout-encoding(local, no PR yet)3006303fix/family-add-member-case, #219fix/family-bump-nothing-to-do(touchtests/test_family.py, which the new file importsupstream_clonefrom)9725bd2,c7e8f5d#227 (the old #204) breaks three of this PR's tests once both are merged, though git merges them cleanly. #227 changes
open_pull_request's signature, sotest_a_manifest_naming_no_repository_refuses_pr_and_leaves_the_placeholder(its three parameters) raisesTypeError: open_pull_request() takes 3 positional arguments but 5 were given. The same call on the merged tree,update_shape.pull_request_target(tmp_path, kind)in place of theopen_pull_request(...)call, gives 27 passed on both8705b27and60038cc; on the merged treepull_request_targetis the refusal_landreaches before the commit, so the test still covers the refusal--prmakes. Whichever of #218 and #227 lands second makes that one-line change intests/test_update_shape_pr_base.py(it cannot be made here:pull_request_targetdoes not exist on main). #227's owntests/test_update_shape_landing.pypasses on the merged tree with8705b27(28 passed). #227'spull_request_targetcalls this PR'spull_request_repositoryandtracking_branch_of, so under--pra failedgh pr create'sNEXTline names the same base and repository as--prdoes.One disagreement the merged tests do not catch. Without
--pr, #227'spull_request_linebuilds its own baregh pr create --base <tracking> --head <branch>(no--repo) for theNEXTline a failed push prints, and asserts exactly that attests/test_update_shape_landing.py:362. After both land, that failure line and this PR's bare--branchline differ by the--repo. Whichever lands second adds--repo <repository>to that arm (frompull_request_repository, left off where it names none) and the one expected string in that test.The three other
update-shape.pysiblings merge with this head and no test of theirs asserts the old--base mainor<org>/<Project>text; #233 and #227 conflict with each other inupdate-shape.pywith or without this change.Left out
scripts/shape_materialize.py'sRULESET_HINT, whichscaffold-project.pyprints, hard-codes--base mainandmain:seed/scaffoldwhile scaffolding--tracking-branch trunkcreates and pushestrunk; adjacent to RULESET_HINT advisespush -u origin ...from an extracted leg whoseoriginfilter-repo removed, and says "listed above" of lines a combined log prints after it #177, which is about the same hint'soriginadvice, not its base.next_linenow reads the manifest after the commit, inside the arm that undoes the tree on aRefusal, so a manifest that stops loading there would roll the tree back under a pushed commit; it is unreachable today becausecommit_on_branchhas just loaded the same file, and reading base and repository once before the commit (in_land) would remove the hazard whatever the merge order.git -C <root>and--head <branch>are not quoted, a root under a path with a space breaks every shell, andshlex.quoteis a POSIX shell's quoting, notcmd.exe's; update-shape.py: a push orgh pr createthat fails afterapply --branchcommitted is named as that step, keeps the commit and ends on a NEXT line, and a re-run with--pron the branch opens the missing pull request, so the tool no longer refuses "committing the re-synced shape" and then answers "nothing to do" (#204) #227'slanding_failedquotes the root withshlex.join, so after both land the twoNEXTforms quote the root differently.gh pr createthat fails afterapply --branchcommitted is named as that step, keeps the commit and ends on a NEXT line, and a re-run with--pron the branch opens the missing pull request, so the tool no longer refuses "committing the re-synced shape" and then answers "nothing to do" (#204) #227 lands,next_linecould readpull_request_targetinstead of callingtracking_branch_ofandpull_request_repositoryitself; it was left alone because that helper refuses on a missing repository andnext_linemust not refuse after the commit.Closes #209
🤖 Generated with Claude Code
Summary by Sourcery
Make generated pull-request commands use the same manifest-defined base branch and repository as automatic pull-request creation.
Bug Fixes:
Nonewhen opening pull requests.Enhancements:
Tests: