Skip to content

adopt-project.py: plan asks the leg-path, pin, date and value refusals of its flags, and an org/repo source's own name and derived id, before the work directory, git's reads of the source and gh repo clone, and says why it cannot clone into a --work-dir nobody can write in (#903) - #963

Merged
brettheap merged 3 commits into
mainfrom
fix/903-plan-flag-refusals-first
Oct 19, 2026

Conversation

@brettheap

@brettheap brettheap commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Lane: openreposhape-1 (openRepoShape-1)
Claim: #903 (comment)

A v1.2.0 draft, authored on the v1.1.1 release commit 50178305. Lane 3 filed #903 from its triage of #457 (L2-55, L4-37 and L1-67). It changes adopt-project.py, adds tests/test_adopt_plan_flag_refusals_first.py and adjusts four existing tests. The first commit is 11a60b71; the independent review (r903) found it READY with four SHOULDs and some NITs, and the second commit, 9e6ed9bb, answers them. The title is narrowed to what holds: it said "each refusal its flags alone decide", which four measured cases refuted (see Known limits).

What

  1. The refusals of the flags' own values come first. A new _checked_flag_inputs(args) runs in cmd_plan right after --out, before _source_spec, _work_root and Source.open: no work directory, no git on the source, no gh repo clone.
    • It asks, in the order _checked_plan_inputs asked them: --project; a passed --tracking-branch; adopt-bad-leg-path; the pin refusals (adopt-pin-collides, pin-malformed); adopt-leg-path-collides and the leg-path values; a passed --id (an empty one included) and --org; election-date-malformed; and adopt-unwritable-value for a passed --elected-by and --reference.
    • Each keeps its code and words; only when it is asked moved. The issue names five of them; the rest went too, since each is decided by a passed flag's value alone.
    • What needs the source, or the values as the plan will record them, stays in _checked_plan_inputs: the default tracking branch, a derived --org, adopt-not-the-source of a source read in place (its name is its origin), an elector read from git config user.name, the pins' digests, _check_names (naming-unclassified, naming-role-mismatch) and the manifest validator's refusals of the values as recorded (an empty --elected-by is adopt-manifest-invalid there). A --path-policy that is missing is yaml-unreadable, read once the source is open.
  2. An org/repo source is held to its name before the clone. _refuse_a_clone_of_another_repository asks, before _work_root, the id derived from --project (adopt-bad-id, added in the second commit: r903's SHOULD-1) and then adopt-not-the-source, in the words each has after a clone, and in main's order. _checked_plan_inputs asks both again of the clone, where they pass.
  3. plan gives an unwritable --work-dir its own reason, and a local source is not probed. _unwritable_work_dir takes an optional needed clause. plan passes _plan_clones, so the reason ends "and plan clones testorg/Thing into source-readonly/Thing there"; check and execute say what they said. A local source's directory is still made, since --out may lie in it, and refused where it cannot be made (adopt-project.py: a --work-dir that is a file or cannot be made ends plan, check and execute in a traceback, and --help says nothing of --work-dir or of where --org comes from #650) or where another user can rename in it (adopt-project.py: the clone plan or check makes and removes is claimed and checked by path, so a shared, writable --work-dir lets another local user turn the removal onto a directory of theirs, two runs on one --work-dir remove each other's clone, a skipped removal says nothing, and a source-readonly that is a file ends in a traceback #803). It is no longer probed for a write.

Why

Measured on 50178305, with a stub gh first on PATH (it logs its arguments and exits 97), a bogus GH_TOKEN, an empty GH_CONFIG_DIR, and TMPDIR and GIT_TRACE per run:

Case Before After
each of 11 flag refusals, org/repo source one gh repo clone, exit 2 source-unclonable; the refusal never said the refusal, exit 2; 0 gh, 0 git, no work directory
the same, local source 7 to 9 git commands, work directory made, then the refusal the same refusal; 0 gh, 0 git, no work directory
--source testorg/North_Wind --project North_Wind (derived id north_wind) one clone, source-unclonable 0 gh, adopt-bad-id, in the words a local source with that origin gets
--source testorg/Thing --project Other --org testorg one clone, source-unclonable 0 gh, adopt-not-the-source
unwritable --work-dir, org/repo reason ends "execute builds the spec leg, the code leg and the split there" reason ends as in item 3
the same directory, local source one write probe, exit 0 0 probes, exit 0
controls: a good plan exit 0 with 19 git commands; one clone the same

A passed --tracking-branch is the one flag that starts git first: git check-ref-format, with an empty temporary directory under TMPDIR as its --git-dir, never the source. It runs once for a branch git takes (--tracking-branch main) and twice for a..b, and the directory is removed.

Order

Two orders of refusal changed, and one more follows from the code:

  • election-date-malformed and adopt-unwritable-value (--reference, --elected-by) now come before adopt-not-the-source. On main adopt-not-the-source came first, so --project Other --elected-on 2026-9-2 is now the date refusal.
  • A flag refusal now beats source-unresolvable where both apply. No existing test met the pair.
  • (Read from the code, not probed: it needs a real clone.) For an org/repo source a derived adopt-bad-id is now asked before the clone, so before any refusal of the source's default branch, which is read from the clone: checked_value's unsafe-value (--tracking-branch (not given, so the source's default branch)), or git's adopt-bad-tracking-branch. A local source keeps main's order, the branch first.

The first commit had also put adopt-not-the-source before a derived adopt-bad-id for an org/repo source, where a local source says the id first. The second commit asks the id first for both, as main did.

Tests

tests/test_adopt_plan_flag_refusals_first.py, 81 tests: twelve refusals against four source and work-dir kinds, the tracking branch, seven pairs in main's order, --out still said first, the derived id of an org/repo source (the source's own name, and another name) with its words compared to a local source's, the not-the-source spellings, plan's own reason, the local source's missing probe, and the controls.

  • Without the fix: with 50178305's adopt-project.py put back, 70 fail and 11 pass, in a scratch copy of the tree. The 11 are the tests named control, and every test not so named fails. test_an_org_repo_source_still_probes_the_work_dir_it_clones_into was once named a control and failed on main, because it asserts plan's new words; the review ruled it renamed, and its docstring now says what it holds.
  • Mutants (r903's 20, and two for the derived id), each applied to a scratch copy of the tree and the new file run: 17 of r903's 20 fail it, M07 (--org '' widened), M16 (--id '' not asked with the flags) and M18 (the flags before --out) among them, which survived the first commit; so do the new M21 (the derived id dropped) and M22 (asked after not-the-source). M11 (the default branch's git check, logic that predates this PR), M15 (the derived elector's unwritable check; tests/test_adopt_git_reads.py::test_plan_refuses_a_user_name_that_is_not_utf8 fails it) and M17 (the pins left out of the pre-clone remedy) still survive the new file; M11 and M17 survive the suite files as well.
  • Four existing tests used a name that was not the source's to reach a later refusal: test_adopt_work_dir_refusal.py, test_adopt_temp_work_roots.py, test_adopt_source_clone_record.py and test_adopt_template_values.py. They now use the source's own, or a refusal that stays after the clone (the plan's write failing). test_adopt_work_dir_refusal.py also expects plan's own reason for an org/repo source into an unwritable folder (plan_cannot_write).
  • No --help text changed, so docs/cli.md is as it was.

Run with /usr/bin/python3 -m pytest, a stub gh first on PATH (it exits 1), a bogus GH_TOKEN, an empty GH_CONFIG_DIR, GIT_CONFIG_GLOBAL=/dev/null and GIT_SSH_COMMAND=false. On 9e6ed9bb, the files below at the same content:

  • test_adopt_plan_flag_refusals_first.py, test_adopt_work_dir_refusal.py, test_adopt_temp_work_roots.py, test_adopt_origin_repository.py, test_adopt_project_is_the_source.py: 175 passed.
  • test_adopt_source_clone_record.py, test_adopt_template_values.py, test_adopt_plan.py: 245 passed, 1 skipped.
  • test_derived_id_refusals.py, test_adopt_execute_refusals.py, test_github_name_limits.py: 266 passed.
  • test_shape_doctor.py: 445 passed, 3 skipped (PowerShell, a Windows host only). test_scaffold_leg_path_collisions.py: 104 passed.
  • test_cli_reference.py, test_agents_md_codes.py, test_repo_hygiene.py: 148 passed.

On 11a60b71, and not run again (the second commit changes two lines of code and comments): five groups of tests/test_adopt_*.py files (263 passed and 1 skipped; 144; 100; 547; 518), which cover test_adopt_leg_paths.py, test_adopt_manifest_values.py, test_adopt_leg_path_collisions.py, test_adopt_plan_yaml_quoting.py, test_adopt_tracking_branch.py, test_adopt_branch_names.py, test_adopt_plan_out_checked.py and test_adopt_plan_leg_path_help.py; and, before three last small edits, all 56 tests/test_adopt_*.py files in 8 shards, whose five failures are the ones the first commit fixes.

Known limits and follow-ups

Closes #903

🤖 Generated with Claude Code

Summary by Sourcery

Reorder plan validation so actionable flag and source-name refusals are reported before unnecessary filesystem, Git, and repository-clone work.

New Features:

  • Make plan report refusals determined solely by supplied flag values before accessing the source, creating work directories, or cloning repositories.
  • Validate an org/repo source against the derived project identity and repository name before cloning it.
  • Provide a plan-specific explanation when the work directory cannot be written for a required clone.

Bug Fixes:

  • Avoid probing local-source work directories for write access when plan only reads the source in place.
  • Preserve the intended refusal ordering and messages for flag validation, including empty explicitly supplied values.

Tests:

  • Add comprehensive coverage for early flag refusals, pre-clone repository validation, work-directory behavior, refusal ordering, and successful-plan controls.
  • Update existing adoption tests for the revised refusal ordering and plan-specific work-directory messages.

…d an `org/repo` source's own name, before the work directory, git's reads of the source and `gh repo clone`, and gives an unwritable `--work-dir` its own reason, so a bad `--spec-path` is no longer `source-unclonable` and a local source's `--work-dir` is no longer probed for nothing (#903)

What was wrong. `cmd_plan` asked `--pin-source` and `--out` first, then
`_source_spec`, `_work_root` and `Source.open`, and only then
`_checked_plan_inputs`, which holds every other refusal the flags alone
decide: `adopt-bad-leg-path`, `adopt-leg-path-collides`,
`adopt-pin-collides`, `pin-malformed`, `election-date-malformed`,
`adopt-unwritable-value` for `--reference` and `--elected-by`, a passed
`--project`, `--tracking-branch`, `--id` and `--org`. By then a local source
had run eight git commands and the work directory was made; an `org/repo`
source had run `gh repo clone`, so with a clone that failed each of them was
`source-unclonable`, with a remedy about `gh auth status`. For `--source
<org>/<repo>` `adopt-not-the-source` came after the clone too, though the
name is the one the flag gave. `plan`'s unwritable-folder refusal said what
`execute` builds there. And a local source's `--work-dir` was probed (a
directory made in it and removed) for an answer `Source.in_place` never read.

What happens now. `_checked_flag_inputs` (new, called after `--out`) asks
each of those in the order `_checked_plan_inputs` asked them, before
`_source_spec`: no work directory, no git, no gh. What needs the source stays
in `_checked_plan_inputs`: the default tracking branch, a derived id, a
derived organisation, `adopt-not-the-source` of a source read in place (its
name is its `origin`), an elector read from `git config user.name`, the pins'
digests, `_check_names` and the misread rules.
`_refuse_a_clone_of_another_repository` asks `_refuse_another_repository`'s
own question, in its words, of an `org/repo` source before `_work_root`, and
`_checked_plan_inputs` asks it again of the clone, where it passes.
`_unwritable_work_dir` takes what the command does there, and `plan` gives
the clone (`_plan_clones`): "... failed (Permission denied), and `plan`
clones testorg/Thing into `source-readonly/Thing` there"; `check` and
`execute` say what they said. A local source is no longer probed. Its work
directory is still made, because `--out` may lie in it
(`test_an_out_in_the_work_dir_the_run_makes_is_still_planned_into`), and
still refused where it cannot be made (#650); that part of `_work_root` is
not changed. The third nit of L1-67, a symlink loop's "(File exists)", is in
`_work_root`, and is left: measured, it is still `making it a directory
failed (File exists)`.

Measured, stub gh first on PATH (logs its arguments, exits 97),
GH_TOKEN=bogus, empty GH_CONFIG_DIR, GIT_CONFIG_GLOBAL=/dev/null,
GIT_SSH_COMMAND=false, TMPDIR and GIT_TRACE per run, an audit hook on mkdir,
rmdir and mkdtemp. Each of the eleven flag refusals above, for each of
an `org/repo` source, a local source and a local source with a missing
`--work-dir`:
- before: `org/repo`: `gh repo clone` once, exit 2 `source-unclonable`, the
  refusal never said. Local: 7 to 9 git commands in the source, the work
  directory made (the `--work-dir` left behind), then the refusal.
- after: the refusal, exit 2, 0 gh calls, 0 git commands, no work directory.
  The one exception is `--tracking-branch a..b`: 2 `git check-ref-format`
  commands in a throwaway repository in TMPDIR, which it removes.
`--source testorg/Thing --project Other --org testorg`: before 1 `gh repo
clone`, `source-unclonable`; after 0, `adopt-not-the-source`, in the words
it has after a clone. `--work-dir` nobody can write in, `org/repo` source:
before the reason ended "`execute` builds the spec leg, the code leg and the
split there, ..."; after it ends as above. Local source, same directory:
before 1 write probe and exit 0; after 0 and exit 0. Controls: a good plan
still plans (local: exit 0, 19 git commands as before), a good `org/repo`
plan still reaches the clone once.

Tests. tests/test_adopt_plan_flag_refusals_first.py, 72 tests: eleven
refusals (both leg paths, the pins, the date, the values, the names) against
four source/work-dir kinds with a fresh TMPDIR, a GIT_TRACE file, a work
directory that is not there and a gh that logs; the tracking branch; seven pairs of refusals in the order main asked them; the
three not-the-source spellings, with and without `--org`, before the clone;
plan's own reason; no probe for a local source; and the controls that pass on
main too. Four existing tests used a name that was not the source's to reach
a later refusal and are given the one that is, or another refusal that stays
after the clone (the plan's write failing): test_adopt_work_dir_refusal.py,
test_adopt_temp_work_roots.py, test_adopt_source_clone_record.py and
test_adopt_template_values.py. No `--help` text changed, so docs/cli.md is
as it was.

Lane: openreposhape-1 (openRepoShape-1)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@brettheap brettheap added this to the v1.2.0 milestone Oct 18, 2026
@sourcery-ai

sourcery-ai Bot commented Oct 18, 2026

Copy link
Copy Markdown

Reviewer's Guide

The PR restructures plan validation so flag-determined refusals and source-name mismatches occur before any work directory, git, or clone side effects, while making work-directory diagnostics accurately reflect whether plan clones or reads a local source; extensive tests verify ordering, messages, side effects, and unchanged successful behavior.

Sequence diagram for plan validation before side effects

sequenceDiagram
    participant User
    participant Plan as cmd_plan
    participant Flags as _checked_flag_inputs
    participant SourceSpec as _source_spec
    participant Naming as _refuse_a_clone_of_another_repository
    participant Work as _work_root
    participant Git as Source.open
    participant GH as gh_repo_clone

    User->>Plan: plan
    Plan->>Flags: _checked_flag_inputs(args)
    alt flag refusal
        Flags-->>User: Refusal
    else flags valid
        Plan->>SourceSpec: _source_spec(args.source)
        opt org/repo source
            Plan->>Naming: _refuse_a_clone_of_another_repository(args, repository, naming)
            alt source name mismatch
                Naming-->>User: adopt-not-the-source
            end
        end
        Plan->>Work: _work_root(args)
        Plan->>Git: Source.open(...)
        opt org/repo source
            Git->>GH: gh repo clone
        end
    end
Loading

Flow diagram for plan source handling and work-directory checks

flowchart TD
    A[cmd_plan] --> B[_checked_flag_inputs]
    B -->|refusal| R[Return refusal]
    B -->|valid| C[_source_spec]
    C --> D{org/repo source?}
    D -->|yes| E[_refuse_a_clone_of_another_repository]
    E -->|name mismatch| R
    E --> F[_work_root]
    D -->|no local source| F
    F --> G{work directory writable?}
    G -->|no| H[Refuse with _plan_clones reason]
    G -->|yes| I[Source.of]
    I --> J{org/repo source?}
    J -->|yes| K[Clone source]
    J -->|no| L[Read local source in place]
    K --> M[_checked_plan_inputs]
    L --> M
Loading

File-Level Changes

Change Details Files
Move all flag-only validation ahead of source access and filesystem or network work.
  • Add _checked_flag_inputs and invoke it immediately after --out.
  • Preserve the existing refusal order and messages while validating project, tracking branch, leg paths, pins, explicit identity fields, election date, and writable text values.
  • Keep validations requiring source-derived defaults, git metadata, or pin digests in _checked_plan_inputs.
  • Add coverage for refusal precedence, side-effect absence, tracking-branch validation, ordering, and successful controls.
adopt-project.py
tests/test_adopt_plan_flag_refusals_first.py
Reject mismatched GitHub source identities before cloning.
  • Parse the repository name from an org/repo or URL source and reuse the existing identity refusal through a name-only stand-in Source.
  • Run the identity check again after cloning, where it validates the cloned source as before.
  • Update affected tests to use the source's own name when exercising later-stage refusals.
adopt-project.py
tests/test_adopt_source_clone_record.py
tests/test_adopt_temp_work_roots.py
tests/test_adopt_template_values.py
Tailor work-directory validation to the operation and avoid probing local sources.
  • Allow _unwritable_work_dir to receive an operation-specific explanation.
  • Report that plan clones the source into the work directory, while retaining the existing execute/check explanation elsewhere.
  • Only probe writability for clone-based sources; continue creating and validating the work root for local sources without performing a write probe.
  • Add and adjust tests for clone-specific messaging, local-source behavior, and work-directory edge cases.
adopt-project.py
tests/test_adopt_work_dir_refusal.py
tests/test_adopt_plan_flag_refusals_first.py

Assessment against linked issues

Issue Objective Addressed Explanation
#903 Run every refusal decidable from plan flags before creating a work directory, reading the source with git, or attempting a source clone. ✅
#903 For an org/repo source, compare the source name with the project and organization names before cloning and report adopt-not-the-source when they differ. ✅
#903 Make plan's unwritable-work-directory refusal describe its clone requirement, and skip the write probe for local sources. ✅

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@brettheap

Copy link
Copy Markdown
Contributor Author

(lane openRepoShape-1, 2026-10-18T18:30:45Z) Independent review of #963 at 11a60b71, by lane 1's reviewer r903 (opus), who is not its writer: READY, with no MUST and four SHOULDs. Lane 1 takes them in a fix round now.

  • The three Direction points of adopt-project.py plan: refusals that need only the flags come after the work directory, the source's git reads and gh repo clone #903 are met:

    • the flag-only refusals come first, in base's order among themselves;
    • an org/repo source's name is held before the clone (0 gh calls);
    • plan gives its own reason for a folder it cannot clone into, and makes no probe for a local source.

    The branch merges cleanly with main.

  • Tests:

    • the new file: 72 passed;
    • the four other changed files: 220 passed;
    • the docs and codes files: 148 passed;
    • without the fix: 64 failed, and 8 passed, which are the controls.
  • Mutants: 13 of 20 are killed.

  • SHOULD-1: "every refusal the flags alone decide" overclaims. For an org/repo source, four cases still wait for the clone: a derived adopt-bad-id, naming-unclassified, an empty --elected-by, and a missing --path-policy.

    • r903's two-line patch refuses the derived id before the clone, and was tested with 191 passed and 1 skipped.
    • The words are narrowed to what then holds.
  • SHOULD-2: the test named a control fails on base, so it is renamed and re-documented.

  • SHOULD-3: two order changes go into the body.

  • SHOULD-4: L1-67's nit 2, the symlink loop, is unchanged. It goes to a follow-up issue.

  • NITs:

    • a passed --tracking-branch runs one git check-ref-format first, in a throwaway repository;
    • the _plan_clones docstring;
    • three test gaps (M07, M16 and M18), which the round closes.

Lane 1's writer f903 makes the round, then r903 re-checks the new head.

Lane: openreposhape-1 (openRepoShape-1)

… the clone, as a local source's is after git reads it, and the claims that `plan` asks every refusal its flags decide first say what still comes after the source: `naming-unclassified`, an empty `--elected-by`, a missing `--path-policy` (#903)

Review r903 found 11a60b7 READY with four SHOULDs; this is the round for
them, on the same branch.

SHOULD-1. `--source testorg/North_Wind --project North_Wind` derives the id
`north_wind`, which no validator takes. Its remedy needs only the name
`--source` gives, yet it was `source-unclonable` after a `gh repo clone`, as
on main. `_refuse_a_clone_of_another_repository` now asks
`_refuse_bad_id` of the derived id first and `adopt-not-the-source` next, in
the order `_checked_plan_inputs` asks them of a source it has read: 0 gh
calls, 0 git commands, no work directory, and the very words a local source
with that `origin` gets (a test compares them). That also gives an
`org/repo` source main's order, a derived id before not-the-source, which
11a60b7 had reversed. The claim that `plan` asks every refusal its flags
alone decide before the source is read is narrowed where it stood
(`_checked_plan_inputs` and `_checked_flag_inputs` docstrings, the comment in
`cmd_plan`, the comment on `--elected-by ''`, the test module's docstring)
to the refusals of a passed flag's own value, and each says what still comes
after the source, for an `org/repo` source as for a local one:
`naming-unclassified` and `naming-role-mismatch` (`_check_names`), an empty
`--elected-by` (a value `unwritable_values` holds; the manifest validator
refuses it, `adopt-manifest-invalid`) and a missing `--path-policy`
(`yaml-unreadable`). Measured, stub gh first on PATH: each of these three is
`source-unclonable` for an `org/repo` source, and for a local one comes after
git's reads.

SHOULD-2. `test_control_an_org_repo_source_still_probes_the_work_dir_it_clones_into`
failed on main, since it asserts plan's new words, so it was no control. It
is `test_an_org_repo_source_still_probes_the_work_dir_it_clones_into`, and its
docstring says what it holds: once, as on main, in plan's words, where main
gave execute's. Every test named `control` now passes on main, and every
other test fails there.

NIT-3, tests. `("--id", "")` is in FLAG_REFUSALS (an empty `--id` is a value
typed, refused before the clone); `--out` in a missing folder is still said
before `--spec-path spec/`, for both kinds of source; `--org ''` is read as
no `--org` and reaches the clone, as on main; two rows for the derived id
(the source's own name, and another name, which is the id first). Mutants of
r903's list that survived: M07 (`--org ''` widened), M16 (`--id ''` not asked
with the flags) and M18 (the flags before `--out`) now fail; two new ones,
dropping the derived id and asking it after not-the-source, fail too. M11,
M15 and M17 still survive the new file, as before.

NIT-1 and NIT-2, words. A passed `--tracking-branch` runs `git
check-ref-format` in a throwaway repository before the refusals after it, so
"no process starts" and "no `git`" are narrowed to "git reads nothing of the
source"; `_plan_clones` says a local source's work directory is still made and
refused where another user can rename in it, and that `--out` may be written
there.

Tests. tests/test_adopt_plan_flag_refusals_first.py is 81 tests (72 before).
With 5017830's adopt-project.py put back, 70 fail and the 11 controls pass.

Lane: openreposhape-1 (openRepoShape-1)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@brettheap brettheap changed the title adopt-project.py: plan asks each refusal its flags alone decide, and an org/repo source's own name, before the work directory, git's reads of the source and gh repo clone, and says why it cannot clone into a --work-dir nobody can write in (#903) adopt-project.py: plan asks the refusals its flag values decide, and an org/repo source's own name and derived id, before the work directory, git's reads of the source and gh repo clone, and says why it cannot clone into a --work-dir nobody can write in (#903) Oct 18, 2026
@brettheap brettheap changed the title adopt-project.py: plan asks the refusals its flag values decide, and an org/repo source's own name and derived id, before the work directory, git's reads of the source and gh repo clone, and says why it cannot clone into a --work-dir nobody can write in (#903) adopt-project.py: plan asks the leg-path, pin, date and value refusals of its flags, and an org/repo source's own name and derived id, before the work directory, git's reads of the source and gh repo clone, and says why it cannot clone into a --work-dir nobody can write in (#903) Oct 18, 2026
@brettheap

Copy link
Copy Markdown
Contributor Author

(lane openRepoShape-1, 2026-10-18T19:08:02Z) Re-check of #963 at 9e6ed9bb, by r903 (opus), who is not its writer: READY, with no MUST and no SHOULD.

Lane: openreposhape-1 (openRepoShape-1)

@brettheap

Copy link
Copy Markdown
Contributor Author

(lane openRepoShape-1, 2026-10-18T19:41:57Z) #963's three comment NITs (r903's NIT-a to NIT-c) are not taken in a round before landing. They are an item of #978, which already holds #903's other follow-up, so #963 lands at 9e6ed9bb, the head r903 found READY, in its turn once it is green and gated.

Lane: openreposhape-1 (openRepoShape-1)

@sonarqubecloud

Copy link
Copy Markdown

@brettheap
brettheap marked this pull request as ready for review October 19, 2026 12:43
@sourcery-ai

sourcery-ai Bot commented Oct 19, 2026

Copy link
Copy Markdown

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 2 days and 7 hours by commenting @sourcery-ai review. Upgrade to get a review now.

@brettheap
brettheap merged commit 2771b38 into main Oct 19, 2026
5 checks passed
@brettheap
brettheap deleted the fix/903-plan-flag-refusals-first branch October 19, 2026 12:43
brettheap added a commit that referenced this pull request Oct 20, 2026
…round a loop is refused in words that name the link and the loop, not "(File exists)", and a symlink to a file is named as one (#978) (#1065)

## What

`plan`, `check` and `execute` refused a `--work-dir` that is a symlink
round a loop as "making it a directory failed (File exists)". `os.mkdir`
of such a link fails with EEXIST, the error a file in the way gets, so the
person was told a file is there and found a link. The way out, "Pass a
--work-dir this user can write in", was the one for a folder this user
cannot write in, and said nothing of a link. #963's review (r903) left it
tracked as #978.

`_work_root` now asks a new `_unmade_work_dir` why the directory could not
be made. The code stays `adopt-work-dir-unwritable`, exit 2, and nothing
is made.

- The `--work-dir` is named as given, made absolute and not resolved. On
  base the reason named `os.path.realpath`'s reading of it, which for a
  link into a loop is another link, a path the person never typed.
- A loop is named only where `os.path.realpath` left a symlink in the path
  (it follows every other one) and `os.stat` of that link fails with ELOOP
  (`_the_loop_in`). So a link to a file, or a long chain of links, is
  never called a loop.
- The way out names the link: the `--work-dir` itself where it is a
  symlink, else the link in the loop.
- A file at the `--work-dir`, or above it, keeps the system's words and
  #524's way out, as the issue asks.
- A dangling link is no loop: `realpath` reads it as the path it leads
  to, which is made, as on base (the controls below).
- `_work_dir_refusal` takes the way out as `fix=`, #524's by default.
  Names are printed with `_spelled`. No `shell_quoted` call is added.

Measured with `plan` of a local source, `<A>` the directory that holds the
links, before on `1a47ed07` and after on this head (exit 2 in every row):

| `--work-dir` | Before | After |
| --- | --- | --- |
| `self`, a link to itself | `` --work-dir `<A>/self` cannot be made: making it a directory failed (File exists) `` | `` --work-dir `<A>/self` cannot be made: it is a symbolic link that leads round a loop `` |
| `a`, with `a -> b` and `b -> a` | `` --work-dir `<A>/a` cannot be made: making it a directory failed (File exists) `` | `` --work-dir `<A>/a` cannot be made: it is a symbolic link that leads round a loop `` |
| `into -> a`, into that pair | `` --work-dir `<A>/a` cannot be made: making it a directory failed (File exists) `` | `` --work-dir `<A>/into` cannot be made: it is a symbolic link that leads round a loop, at the symbolic link `<A>/a` `` |
| `through -> loop/sub`, with `loop -> loop` | `` --work-dir `<A>/loop/sub` cannot be made: making it a directory failed (Too many levels of symbolic links) `` | `` --work-dir `<A>/through` cannot be made: it is a symbolic link that leads round a loop, at the symbolic link `<A>/loop` `` |
| `loop/sub`, no link itself | `` --work-dir `<A>/loop/sub` cannot be made: making it a directory failed (Too many levels of symbolic links) `` | `` --work-dir `<A>/loop/sub` cannot be made: the way to it leads round a loop, at the symbolic link `<A>/loop` `` |
| `tofile -> a-file` | `` --work-dir `<A>/a-file` cannot be made: making it a directory failed (File exists) `` | `` --work-dir `<A>/tofile` cannot be made: it is a symbolic link that leads to `<A>/a-file`, which is not a directory `` |
| `a-file`, a file | `(File exists)` | unchanged |
| `a-file/under` | `(Not a directory)` | unchanged |

The way out:

- Before, in every row: "Remediation: Pass a --work-dir this user can
  write in, or none: without one, each run makes a new one."
- After, in the six symlink rows: "Remediation: Remove the symbolic link
  `<link>`, or make it lead to a directory, or pass another --work-dir, or
  none: without one, each run makes a new one." `<link>` is `<A>/self`,
  `<A>/a`, `<A>/into`, `<A>/through`, `<A>/loop` and `<A>/tofile`.
- `execute` still says "no leg repository was created and nothing was
  pushed." first. The two file rows keep the way out they had.

**Wider than the issue.** The issue asks for the EEXIST case, where the
`--work-dir` is itself the link. A path that runs through a loop fails
`os.mkdir` with ELOOP instead, and said only the system's words, with the
same way out for a folder this user cannot write in. It is named the same
way now: `--work-dir through` and `--work-dir loop/sub` reach the same
place. So the words `tests/test_adopt_clone_claimed.py` held for a loop
above the work directory change. That row's point, #650's code
`adopt-work-dir-unwritable` and not `adopt-work-dir-shared`, is unchanged.

### The comment wording in #963's code

r903's round-2 NITs a-c (#963 issuecomment-6253937080). Comments and
docstrings only.

- **(a)** `_ask_git_branch` passes an empty temporary directory as
  `--git-dir`, which is no repository.
  - `adopt-project.py`, `_checked_flag_inputs`: "That question is asked of
    a throwaway repository in the system temp folder, which it removes"
    is now "That question is asked with an empty temporary directory as
    its `--git-dir`, which `plan` removes".
  - `adopt-project.py`, `cmd_plan`: "runs `git check-ref-format` in a
    throwaway repository first" is now "runs `git check-ref-format` first,
    with an empty temporary directory as its `--git-dir`, which `plan`
    removes".
  - `tests/test_adopt_plan_flag_refusals_first.py`, the module docstring:
    "in a throwaway repository" is now "with an empty temporary directory
    as its `--git-dir`, which `plan` removes".
  - The same file's
    `test_the_branch_a_tracking_branch_names_is_asked_before_the_source`:
    "is asked of a throwaway repository in the temp folder ... and it
    removed what it made" is now "is asked of git with an empty temporary
    directory as its `--git-dir` ... and the directory was removed".
  - "which it removes" in r903's words is "which `plan` removes" here, so
    that it does not read as git removing it.
- **(b)** The refusals still made after the source are listed "among
  them", in `_checked_flag_inputs`' docstring and the test's module
  docstring: a `--project` of 251 to 255 characters is refused
  `unsafe-value` after git's reads.
- **(c)** The naming refusals come "with a remedy about the source where
  it names a repository".

## Tests

- `tests/test_adopt_work_dir_refusal.py`, 53 tests:
  - `test_a_work_dir_that_is_a_symlink_loop_is_refused` held "(File
    exists)" through `mkdir_says`. It now holds the new words, for a link
    to itself and for a pair, through `plan`, `check` and `execute`: 6.
  - New: a link into a pair and a link through a loop (6), a path under a
    loop (3), and a link to a file (3), each through the three commands;
    each asserts exit 2, the whole of stderr, no plan written, no leg, and
    nothing made or changed under the test's directory.
  - New: `_the_loop_in` asked directly, which calls a link to a file or to
    a directory no loop (1).
  - New controls: a dangling link to `target-a` and to
    `missing-dir/target-b`, through `plan` and `check`, exit 0 and the
    target made (4).
- `tests/test_adopt_clone_claimed.py`, 42 tests:
  `test_a_loop_or_a_file_above_the_work_dir_keeps_its_650_refusal`'s three
  `a-symlink-loop` rows hold the new words; its file rows keep the
  system's.
- `tests/test_adopt_plan_flag_refusals_first.py`: docstrings only.
- **Fail on base.** With `1a47ed07`'s `adopt-project.py` in this head's
  tree, the two changed files: 22 failed, 73 passed. The 22 are exactly
  the #978 rows: the 19 in `test_adopt_work_dir_refusal.py` and the 3
  `a-symlink-loop` rows. The 4 dangling-link controls pass on both. On
  this head: 95 passed.
- **The gate**, on this head, one basetemp, `EMAIL` exported, `LANES_LANE`
  unset, a stub `gh` first on PATH, `GH_TOKEN=bogus-w978`, an empty
  `GH_CONFIG_DIR`, `GIT_CONFIG_GLOBAL=/dev/null` and
  `GIT_SSH_COMMAND=false`, in 17 chunks and 4 shards, each under 9
  minutes:
  - 46 files: the 34 that name `adopt-work-dir-unwritable`, `_work_root`
    or `work_dir`; `test_adopt_plan_flag_refusals_first`,
    `test_repo_hygiene`, `test_agents_md_codes`, `test_cli_reference`,
    `test_readme_windows_quoting_sentence` and
    `test_scaffold_leg_path_collisions`; and the 7 that read
    `adopt-project.py`'s source (`test_git_output_decoding`,
    `test_no_auto_maintenance`, `test_non_utf8_git_failures`,
    `test_shape_advisory`, `test_pinned_copy_text`,
    `test_openreposhape_package`, `test_template_values_single_pass`):
    3444 passed, 2 skipped, in 17 chunks.
  - `test_shape_doctor.py`, all 448 tests in 4 shards: 445 passed and 3
    skipped.

**Mutants**, each applied alone to `adopt-project.py`, then
`tests/test_adopt_work_dir_refusal.py` run, then restored by sha256. All
12 killed. They ran on a file that differs from this head's only in two
comment lines ("which `plan` removes").

| Mutant | Failed | Killed by |
| --- | --- | --- |
| M1 ELOOP dropped from the errno check | 6 | through-a-loop, under-a-loop |
| M2 the same-link branch always taken | 6 | into-a-pair, through-a-loop |
| M3 the "is a link" test dropped from the link-to-a-file branch | 3 | the file AT the `--work-dir` (keeps `File exists`) |
| M4 the way out under a loop names no link | 3 | under-a-loop |
| M5 the `--work-dir` named as `realpath` reads it | 9 | into-a-pair, through-a-loop, link-to-a-file |
| M6 only the last part of the path asked for a link | 6 | through-a-loop, under-a-loop |
| M7 no loop ever found | 15 | every loop row |
| M8 the link named where its target goes | 3 | link-to-a-file |
| M9 the old way out kept | 18 | every symlink row |
| M10 `_same_entry` always false | 6 | to-itself, a-pair |
| M11 `realpath`'s reading passed for the path as typed | 9 | into-a-pair, through-a-loop, link-to-a-file |
| M12 any link left in the path called a loop, with no ELOOP | 1 | `test_only_a_symlink_that_loops_is_called_a_loop` |

M12 survives every command-level row: on Linux `realpath` leaves no link
in the path but one that loops. The direct test of `_the_loop_in` holds
the guard.

## Merge with main

`git merge-tree --write-tree` of this head with `main` at `eee089ca` is
clean (tree `02c1717d`). Of this branch's files, only `adopt-project.py`
moved on main. #1046 changed other functions. #1041 added the
`adopt-bad-project-name` refusal inside `_checked_flag_inputs`, below the
docstring and the comment this branch rewords. A `--project` the naming
policy refuses as well (`NUL.git`) is still refused only after the
source, and the docstring's list of what comes after the source, read
"among them", stays true. The branch was not merged, and the gate ran on
this head.

## Not in this PR

- A link to a path under a file (`w -> a-file/sub`) still names the path
  `realpath` reads, `<A>/a-file/sub`, with "(Not a directory)". True, but
  it does not name the link.
- Not measured on Windows, where a loop is named only if `os.stat` of the
  link reports ELOOP.

It lands by fast refresh (Brett Heap, 2026-10-15T22:48:35Z). Its reviewed head `a4e179dd` is green on Linux and SonarCloud. The branch merged main once and was never rebased; the last merge is update-branch's, clean, giving 228531a, which is green on SonarCloud and Git Bash. Lane 1's gate passed on its tree: the 47 test files that run `--work-dir` refusals and #650's claimed clones, read `adopt-project.py`'s source, and the doctor's, 3,898 passed and 3 skipped.

Closes #978

Lane: openreposhape-1 (openRepoShape-1)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.

adopt-project.py plan: refusals that need only the flags come after the work directory, the source's git reads and gh repo clone

1 participant