From 427237a4c51cb3caf9bb4cec248186c37ac1f0a5 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Tue, 13 Oct 2026 11:21:59 +0000 Subject: [PATCH 1/3] setup-project.py: the clone path `/` is refused in step (3), before anything is created, when anything at all stands there, naming the path and telling a rehearsal's leftover clone apart as one to move or delete, and the plan names the clone path before the yes, so rehearsing and then running for real in the same folder - README's own order - no longer creates three repositories and then refuses with a hand clone that fails on the same directory (#498) README's "Rehearse first, creating nothing" runs the scaffold against `--local-remote-dir ./rehearsal` and leaves `./Atlas` behind, "both of which you can delete". A person who kept it and ran the real command in the same folder went through the preflight, the naming check, the plan and the yes; step (6) created and pushed all three repositories; and only then did step (7) find `./Atlas` and refuse. Its printed `git clone` failed on the same directory, the `cd Atlas` after it entered the rehearsal's project, and a re-run was refused `scaffold-remote-exists`. The path was decided in step (3), by `resolve_family`, and nothing read it before step (7). - Step (3), right after `resolve_family`: `refuse_a_taken_clone_path` refuses when anything at all stands at the clone path. `os.path.lexists`, so a symbolic link to nothing counts: `exists` reads it as absent and `git clone` still refuses it. An EMPTY directory is refused too, though `git clone` would take one: #498's option 1, put to Brett Heap on 2026-10-13, who did not object. The refusal, exit 2, says " already exists, and that is where this project would be cloned, so the run stops here and nothing was created.", then what is there where it can tell (`what_stands_at`), then the way out. - A rehearsal's clone is told apart by all three of: an `origin` on this disk, named `.git`, and a `project.yaml` at its root. "It is the clone a rehearsal left behind: its `origin` is , a repository on this disk, as `--local-remote-dir` makes one. Move or delete it, then run this again." An empty directory is offered for deletion too. Anything else is somebody's and is never offered for deletion: a clone of GitHub (named, with any `user:token@` dropped), a local mirror under another name, a directory that is not a clone, a file. "Move it, or choose another --into, then run this again." - The `origin` is read with `git config --file /.git/config`, never `git -C `, which walks up from an empty or broken `.git` and reports the `origin` of whatever repository holds ``. A value carrying a control character is not echoed. - Step (5): the plan prints `clone into ` after the scaffold's dry run, on every run, `--yes` and no terminal included. The plan named the three repositories and never the one thing the run writes on this disk; it is also what an assistant relays to the person deciding. - Step (7) keeps its check as the backstop for a path that appears while the run is going, now `lexists` too. Its way out moves the thing aside first and runs the hand clone from the clone's parent, where it used to print a `git clone` that met the same directory. `setup.sh` hands over to `setup-project.py`, so both entry points refuse alike. The runs through each are asserted once, in two helpers in `tests/conftest.py`, with a one-call wrapper per file. README.md, AGENTS.md and docs/handbook.html are #483's (lane 5) and are not changed here; no `--help` text changed, so docs/cli.md stands. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 --- setup-project.py | 150 ++++++++++++++++++++++++++++++-- tests/conftest.py | 95 ++++++++++++++++++++ tests/test_setup_project_py.py | 154 ++++++++++++++++++++++++++++++++- tests/test_setup_sh.py | 17 ++++ 4 files changed, 407 insertions(+), 9 deletions(-) diff --git a/setup-project.py b/setup-project.py index d820d8f8..2a71d5f3 100755 --- a/setup-project.py +++ b/setup-project.py @@ -1662,6 +1662,122 @@ def resolve_family(opts: Options, shape_root: Path) -> Landing: return Landing(landing, family, landing / family) +# --------------------------------------------------------------------------- +# 3c. the clone path, while nothing exists +# --------------------------------------------------------------------------- + +#: THE TWO WAYS OUT OF A TAKEN CLONE PATH. Deleting is offered only for what +#: this tool can tell is worth nothing: an empty directory, and the clone a +#: rehearsal left behind. Anything else is somebody's, and is moved, or the +#: project is put somewhere else; this tool deletes and moves nothing itself. +DISCARD_IT = "Move or delete it, then run this again." +KEEP_IT = "Move it, or choose another --into, then run this again." + + +def origin_recorded_in(clone: str) -> str: + """The `origin` URL `/.git/config` records, or "" when it records + none this can read. + + THE FILE, NEVER A DISCOVERY. `git -C ` would look for a + repository and, finding none in `` itself - an empty `.git`, a + broken one - would walk up and answer for whatever repository holds + ``, naming somebody else's `origin` as this directory's. `git + config --file` reads that one file, with no includes and no URL + rewriting. A `.git` FILE (a worktree, a submodule) keeps its config + elsewhere, so there is nothing here to read and the caller says less. + + A value carrying a control character is not echoed: it would print a + line this tool did not write (`CONTROL_RE`). + """ + config = os.path.join(clone, ".git", "config") + if not os.path.isfile(config): + return "" + proc = run(["git", "config", "--file", config, "--get", + "remote.origin.url"], capture=True) + url = (proc.stdout or "").strip() if proc.returncode == 0 else "" + return "" if CONTROL_RE.search(url) else url + + +def is_on_this_disk(url: str) -> bool: + """An `origin` that is a path rather than a host: what `git clone` of a + local repository records (it makes the path absolute), which is what a + `--local-remote-dir` rehearsal's clone has.""" + return url.startswith("file:") or os.path.isabs(url) + + +def without_credentials(url: str) -> str: + """`url` with any `user:token@` after its scheme dropped: an `origin` is + echoed into a refusal an assistant may relay, and a token is not part of + what is there.""" + return re.sub(r"(?<=://)[^/@]*@", "", url, count=1) + + +def what_stands_at(clone: str, project: str) -> tuple: + """`(one sentence saying what is at clone, whether it can be thrown + away)`, or `("", False)` when this cannot tell. + + A REHEARSAL'S CLONE IS THE CASE THIS EXISTS FOR (#498). README's + "Rehearse first" leaves `./` behind, cloned from the bare + `/.git` that `--local-remote-dir` made, and the real run + that follows it in the same folder lands on exactly that path. It is + told apart by all three of: an `origin` on this disk, named + `.git`, and a `project.yaml` at its root. A clone of a + repository on GitHub, or of a local mirror under another name, is + somebody's working copy and is never offered for deletion. + """ + try: + if not os.path.isdir(clone): + return "", False + if not os.path.lexists(os.path.join(clone, ".git")): + if os.listdir(clone): + return "It is a directory that is not a git clone.", False + return "It is an empty directory.", True + except OSError: + return "", False + origin = origin_recorded_in(clone) + if not origin: + return "It is a git repository.", False + if (is_on_this_disk(origin) + and os.path.basename(origin.rstrip("/\\")) == project + ".git" + and os.path.isfile(os.path.join(clone, "project.yaml"))): + return ("It is the clone a rehearsal left behind: its `origin` is %s, " + "a repository on this disk, as `--local-remote-dir` makes " + "one." % origin), True + return "It is a clone of %s." % without_credentials(origin), False + + +def refuse_a_taken_clone_path(clone: str, project: str) -> None: + """Refuse, HERE, a clone path that anything at all already stands at. + + THE QUESTION IS ASKED WHERE THE ANSWER IS DECIDED (#498). `resolve_family` + has just fixed `/` (or `//`), and + nothing exists yet. Asked only in step (7), as it used to be, the same + question came after step (6) had created all three repositories and + pushed them: a person who rehearsed and then ran for real in the same + folder - README's own order - got three new repositories on GitHub, no + project, a hand clone that failed on the same directory, and a re-run + refused `scaffold-remote-exists`. + + ANY ENTRY AT THAT NAME, `lexists`: a directory, a file, and a symbolic + link to nothing, which `exists` reads as absent and `git clone` still + refuses. An EMPTY directory is refused too, although `git clone` would + take one (#498's option 1, put to Brett Heap on 2026-10-13, who did not + object): the rule is "nothing there", said once, rather than one more + case to explain. + + The refusal says nothing was created, names the path, and says what is + there where `what_stands_at` can tell. Step (7) keeps its own check, for + a path that appears while the run is going. + """ + if not os.path.lexists(clone): + return + what, disposable = what_stands_at(clone, project) + die(" ".join(part for part in ( + "%s already exists, and that is where this project would be cloned, " + "so the run stops here and nothing was created." % clone, + what, DISCARD_IT if disposable else KEEP_IT) if part)) + + # --------------------------------------------------------------------------- # 4. the names, before anything exists # --------------------------------------------------------------------------- @@ -1741,7 +1857,7 @@ def scaffold_args(opts: Options, org: str) -> list: def plan_and_confirm(opts: Options, shape_root: Path, org: str, - args: list) -> None: + args: list, clone: str) -> None: say("") say("(5) the plan") planned = run([PYTHON, str(shape_root / SCAFFOLD), *args, @@ -1750,6 +1866,18 @@ def plan_and_confirm(opts: Options, shape_root: Path, org: str, raise Refusal("the plan could not be produced (see above).", shell_status(planned.returncode)) + # THE ONE THING THE RUN WRITES ON THIS DISK IS IN THE PLAN TOO (#498). + # The scaffold's plan names the three repositories; the clone path - set + # by `--into`, by the developer path's `..`, by a family folder - was + # said before the `yes` only by `--family`'s `lands at`, far above, so a + # person learned where the project went from `DONE`. Step (3) has + # already refused a path that is taken, so this line is where to look + # afterwards, not a warning; it is printed on every run, `--yes` and no + # terminal included, since the plan is what an assistant relays to the + # person deciding. + say("") + say("clone into %s" % clone) + if opts.assume_yes: return # THE QUESTION GOES TO THE PERSON'S TERMINAL, NEVER TO A PIPE (#492). @@ -1863,12 +1991,20 @@ def clone_and_bootstrap(opts: Options, org: str, landing: Landing) -> dict: # The three repositories now EXIST. Everything from here is recoverable by # hand, so a failure names the command that finishes the job rather than # leaving the person wondering what was created. - if os.path.exists(clone): - die("%s already exists, so there is nowhere to clone into. The three " - "repositories WERE created. Finish by hand:\n" + # + # THE BACKSTOP (#498). `refuse_a_taken_clone_path` asked this in step (3), + # before anything existed, so a path taken HERE appeared while the run + # was going. `lexists` for the same reason it is there. The way out moves + # the thing aside FIRST and runs the clone from the clone's parent: as it + # used to read, the hand clone met the same directory and failed, and + # `cd ` from anywhere else entered whatever was there. + if os.path.lexists(clone): + die("%s already exists - it was not there when step (3) looked - so " + "there is nowhere to clone into. The three repositories WERE " + "created. Move it aside, then finish by hand in %s:\n" " git clone --recurse-submodules %s\n" " cd %s && %s %s" - % (clone, urls["assembly"], opts.project, PYTHON_CMD, + % (clone, parent, urls["assembly"], opts.project, PYTHON_CMD, BOOTSTRAP)) say("") @@ -2049,9 +2185,11 @@ def _main(argv, invocation_dir: str) -> int: org = resolve_org(opts, invocation_dir) resolve_project_and_elector(opts, shape_root) landing = resolve_family(opts, shape_root) + clone = landing.clone_path(opts.project) + refuse_a_taken_clone_path(clone, opts.project) check_names(shape_root, opts.project, opts.family) args = scaffold_args(opts, org) - plan_and_confirm(opts, shape_root, org, args) + plan_and_confirm(opts, shape_root, org, args, clone) scaffold(shape_root, args) created = clone_and_bootstrap(opts, org, landing) hand_over(opts.project, created["clone"], created["urls"], org, landing) diff --git a/tests/conftest.py b/tests/conftest.py index 82b96e45..2ea85e5b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -841,6 +841,101 @@ def assert_refuses_for_want_of_a_terminal(stderr: str) -> None: f"the exit that keeps the human in charge comes first: {stderr!r}") +# --- a clone path that is already taken (#498) ------------------------------- +# +# ONE READING, TWO ENTRY POINTS. `tests/test_setup_sh.py` and +# `tests/test_setup_project_py.py` each hand their own runner (`run_setup`, +# `run_entry`) to the two helpers below, so the same runs are asserted once +# whichever name a person typed. Every run here passes `--local-remote-dir`, +# which is what those files' `test_no_test_here_depends_on_gh_being_ +# authenticated` looks for in each call it can see; it cannot see these, so +# they keep it themselves. + +#: What can stand where the project would be cloned, as +#: `assert_a_taken_clone_path_is_refused` makes each one. +TAKEN_CLONE_PATHS = ("empty directory", "file", "directory") + +#: The refusal's two ways out: deleting is offered only for what holds +#: nothing of anybody's. +DISCARD_IT = "Move or delete it, then run this again." +KEEP_IT = "Move it, or choose another --into, then run this again." + + +def refused_in_step_three(result, clone: str) -> str: + """`result` refused in step (3), before the naming check, the plan or + the scaffold, naming `clone` and saying nothing was created; returns the + refusal's text after that first sentence.""" + head = (f"REFUSED: {clone} already exists, and that is where this project " + "would be cloned, so the run stops here and nothing was created.") + assert result.returncode == 2, result.stderr + result.stdout + assert head in result.stderr, result.stderr + assert "(3) project" in result.stdout, result.stdout + for later in ("(4) naming policy", "(5) the plan", "(6) scaffold"): + assert later not in result.stdout, f"the run reached {later}" + return result.stderr.split(head, 1)[1].strip() + + +def assert_a_rehearsals_clone_stops_the_real_run(run, tmp_path: Path) -> None: + """README's own order, in one folder: rehearse with `--local-remote-dir + ./rehearsal`, then run for real (a second `--local-remote-dir` stands in + for GitHub). The rehearsal names its clone path in the plan and leaves + `./Sample` behind; the real run refuses in step (3), names that clone + as the rehearsal's and offers to delete it, and creates nothing. On main + it created all three repositories and only then refused, in step (7).""" + here = tmp_path / "here" + clone = os.path.abspath(str(here / "Sample")) + rehearsal = run("--project", "Sample", "--yes", "--org", "demoorg", + "--local-remote-dir", str(here / "rehearsal"), + "--into", str(here)) + assert rehearsal.returncode == 0, rehearsal.stderr + rehearsal.stdout + out = rehearsal.stdout + assert (out.index("--dry-run: nothing was created.") + < out.index(f"\nclone into {clone}\n") + < out.index("(6) scaffold")), out + real = run("--project", "Sample", "--yes", "--org", "demoorg", + "--local-remote-dir", str(here / "real"), "--into", str(here)) + rest = refused_in_step_three(real, clone) + assert rest.startswith("It is the clone a rehearsal left behind: its " + "`origin` is "), rest + assert ("a repository on this disk, as `--local-remote-dir` makes one. " + + DISCARD_IT) in rest, rest + assert not (here / "real").exists(), "the real run created repositories" + assert (here / "Sample" / "project.yaml").is_file() + + +def assert_a_taken_clone_path_is_refused(run, tmp_path: Path, + kind: str) -> None: + """Anything at `/` - one of `TAKEN_CLONE_PATHS` - is + refused in step (3) of a run told `--yes`, with what is there said where + the tool can tell, and left exactly as it was. An EMPTY directory too, + though `git clone` would take one: #498's option 1. On main each of + these created all three repositories first.""" + work = tmp_path / "work" + taken = work / "Sample" + if kind == "file": + work.mkdir() + taken.write_bytes(b"not a project\n") + said, way_out = "", KEEP_IT + elif kind == "directory": + taken.mkdir(parents=True) + (taken / "notes.txt").write_bytes(b"somebody's\n") + said, way_out = "It is a directory that is not a git clone. ", KEEP_IT + else: + taken.mkdir(parents=True) + said, way_out = "It is an empty directory. ", DISCARD_IT + before = sorted(p.name for p in taken.iterdir()) if taken.is_dir() else ( + taken.read_bytes()) + result = run("--project", "Sample", "--yes", "--org", "demoorg", + "--local-remote-dir", str(tmp_path / "remotes"), + "--into", str(work)) + rest = refused_in_step_three(result, os.path.abspath(str(taken))) + assert rest == said + way_out, rest + assert not (tmp_path / "remotes").exists() + after = sorted(p.name for p in taken.iterdir()) if taken.is_dir() else ( + taken.read_bytes()) + assert after == before, f"{taken} was changed by a run that refused" + + #: THE PERSON'S OWN TERMINAL, IN A TEST (#492). The one-liner reads its yes #: from the CONTROLLING terminal when its stdin is the pipe the script came #: down, so a test that says it asks has to give the run one: a diff --git a/tests/test_setup_project_py.py b/tests/test_setup_project_py.py index acb23366..967df02b 100644 --- a/tests/test_setup_project_py.py +++ b/tests/test_setup_project_py.py @@ -40,6 +40,10 @@ from conftest import PTY_SKIP, run_in_a_terminal +from conftest import (KEEP_IT, TAKEN_CLONE_PATHS, + assert_a_rehearsals_clone_stops_the_real_run, + assert_a_taken_clone_path_is_refused) + SETUP_PROJECT = REPO / "setup-project.py" SETUP_SH = REPO / "setup.sh" DEGRADE_LINE = "authority is not wallet-carried in this org" @@ -462,6 +466,19 @@ def test_passthrough_reaches_the_scaffold(tmp_path): assert 'reference: "a-staged-fragment.md"' in manifest +def test_a_rehearsals_clone_stops_the_real_run_before_anything(tmp_path): + """#498, on an interpreter alone: `conftest.assert_a_rehearsals_clone_ + stops_the_real_run`.""" + assert_a_rehearsals_clone_stops_the_real_run(run_entry, tmp_path) + + +@pytest.mark.parametrize("kind", TAKEN_CLONE_PATHS) +def test_a_taken_clone_path_is_refused_before_anything(tmp_path, kind): + """#498, on an interpreter alone: `conftest.assert_a_taken_clone_path_ + is_refused`.""" + assert_a_taken_clone_path_is_refused(run_entry, tmp_path, kind) + + def test_anything_but_yes_refuses_and_creates_nothing(monkeypatch, tmp_path): """The one `yes` is EXACT: `no`, `y`, `Yes` and an empty line all refuse. @@ -483,14 +500,15 @@ def isatty(self): args=[], returncode=0, stdout="", stderr="")) opts = module.parse_args(["--project", "Sample", "--org", "demoorg"], str(tmp_path)) + clone = str(tmp_path / "Sample") for answer in ("no", "y", "Yes", ""): monkeypatch.setattr(module, "ask", lambda prompt, a=answer: a) with pytest.raises(module.Refusal) as refused: - module.plan_and_confirm(opts, REPO, "demoorg", []) + module.plan_and_confirm(opts, REPO, "demoorg", [], clone) assert refused.value.code == 1, answer assert "not confirmed; nothing was created." in str(refused.value) monkeypatch.setattr(module, "ask", lambda prompt: "yes") - module.plan_and_confirm(opts, REPO, "demoorg", []) + module.plan_and_confirm(opts, REPO, "demoorg", [], clone) def test_no_test_here_depends_on_gh_being_authenticated(): @@ -1657,10 +1675,139 @@ def test_a_rehearsal_that_stops_at_the_plan_makes_no_family_folder(tmp_path): assert "no terminal to confirm on" in result.stderr assert "lands at %s" % (work / FAMILY / "Sample") in result.stdout assert "--dry-run: nothing was created." in result.stdout + # The plan an assistant relays names the clone path too, family level + # included (#498), though the confirm refuses. + assert "clone into %s" % (work / FAMILY / "Sample") in result.stdout assert not work.exists(), f"{work} was created by a run that created nothing" assert not (tmp_path / "remotes").exists() +# --- what stands at a taken clone path (#498) -------------------------------- +# +# The runs through both entry points are `conftest`'s, wrapped above. These +# ask the pieces in-process: what the refusal can tell about a clone, a link +# to nothing, and the step (7) backstop, which a run reaches only when the +# path appears while it is going. + +#: An `origin` carrying a token, as an HTTPS clone can record one. +WITH_A_TOKEN = "https://dana:ghp_notreal@github.com/Northwind/Sample.git" + +REHEARSALS = ("It is the clone a rehearsal left behind: its `origin` is %s, a " + "repository on this disk, as `--local-remote-dir` makes one.") + + +def _a_clone(taken: Path, origin=None, shape_root: bool = False) -> None: + """`taken` as `git init` leaves it, with `origin` and a `project.yaml` + if asked; no commit, so no identity is needed.""" + taken.mkdir(parents=True) + git("init", "-q", cwd=taken) + if origin is not None: + git("remote", "add", "origin", str(origin), cwd=taken) + if shape_root: + (taken / "project.yaml").write_bytes(b"schema_version: 1\n") + + +@pytest.mark.parametrize("case", [ + "github", "the-rehearsals", "the-rehearsals-as-a-file-url", + "a-mirror-under-another-name", "rehearsal-shaped-with-no-project-yaml", + "no-origin", "a-git-file", "an-empty-git-inside-another-repository"]) +def test_what_stands_at_a_taken_clone_path(tmp_path, case): + """`what_stands_at`: a sentence and whether deleting is offered. + + Deleting is offered for a rehearsal's clone alone, told by all three of + an `origin` on this disk, its name `.git`, and a `project.yaml`: + a mirror under another name and a clone with no manifest are somebody's. + A token in an `origin` is not echoed. And the `origin` is read from the + directory's OWN `.git/config`: an empty `.git` inside a repository that + has one must not be reported as a clone of THAT repository, which is + what `git -C` finds by walking up.""" + module = entry_point_module() + taken = tmp_path / "here" / "Sample" + rehearsal = tmp_path / "here" / "rehearsal" / "Sample.git" + mirror = tmp_path / "mirrors" / "Mirror.git" + if case == "github": + _a_clone(taken, WITH_A_TOKEN, shape_root=True) + expected = ("It is a clone of " + "https://github.com/Northwind/Sample.git.", False) + elif case == "the-rehearsals": + _a_clone(taken, rehearsal, shape_root=True) + expected = (REHEARSALS % rehearsal, True) + elif case == "the-rehearsals-as-a-file-url": + _a_clone(taken, rehearsal.as_uri(), shape_root=True) + expected = (REHEARSALS % rehearsal.as_uri(), True) + elif case == "a-mirror-under-another-name": + _a_clone(taken, mirror, shape_root=True) + expected = ("It is a clone of %s." % mirror, False) + elif case == "rehearsal-shaped-with-no-project-yaml": + _a_clone(taken, rehearsal) + expected = ("It is a clone of %s." % rehearsal, False) + elif case == "no-origin": + _a_clone(taken) + expected = ("It is a git repository.", False) + elif case == "a-git-file": + taken.mkdir(parents=True) + (taken / ".git").write_bytes(b"gitdir: ../elsewhere/.git\n") + expected = ("It is a git repository.", False) + else: + _a_clone(taken.parent, "https://example.invalid/Outer.git") + (taken / ".git").mkdir(parents=True) + expected = ("It is a git repository.", False) + assert module.what_stands_at(str(taken), "Sample") == expected + assert "ghp_notreal" not in expected[0] + + +def test_a_link_to_nothing_takes_the_clone_path(tmp_path): + """`os.path.exists` reads a symbolic link to nothing as absent, and + `git clone` still refuses it (`could not create work tree dir`), so + step (3) asks `lexists`. A free path is the control: it is not refused.""" + module = entry_point_module() + link = tmp_path / "Sample" + try: + os.symlink(tmp_path / "nowhere", link) + except (NotImplementedError, OSError) as exc: + pytest.skip(f"this platform cannot create a symlink here: {exc}") + assert module.refuse_a_taken_clone_path(str(tmp_path / "Free"), + "Free") is None + with pytest.raises(module.Refusal) as refused: + module.refuse_a_taken_clone_path(str(link), "Sample") + assert refused.value.code == 2 + assert str(refused.value) == ( + "%s already exists, and that is where this project would be cloned, " + "so the run stops here and nothing was created. %s" % (link, KEEP_IT)) + + +def test_a_path_taken_after_step_three_is_the_backstops(monkeypatch, + tmp_path): + """Step (7) still asks, for a path that appeared while the run was going, + after all three repositories exist. Its way out moves the thing aside + and clones from the clone's PARENT: as it used to read, the printed + `git clone` met the same directory, and `cd ` from anywhere + else entered whatever stood there. Nothing is cloned.""" + module = entry_point_module() + runs = _Runs() + monkeypatch.setattr(module, "run", runs) + opts = module.parse_args(["--project", "Sample", "--org", "demoorg", + "--local-remote-dir", str(tmp_path / "remotes"), + "--into", str(tmp_path / "work")], + str(tmp_path)) + parent = Path(opts.into) + (parent / "Sample").mkdir(parents=True) + with pytest.raises(module.Refusal) as refused: + module.clone_and_bootstrap(opts, "demoorg", + module.Landing(parent, "", None)) + assert refused.value.code == 2 + assert str(refused.value) == ( + "%s already exists - it was not there when step (3) looked - so " + "there is nowhere to clone into. The three repositories WERE " + "created. Move it aside, then finish by hand in %s:\n" + " git clone --recurse-submodules %s\n" + " cd Sample && %s scripts/bootstrap.py" + % (parent / "Sample", parent, + os.path.join(opts.local_remote_dir, "Sample.git"), + module.PYTHON_CMD)) + assert runs.calls == [], f"something was run: {runs.calls}" + + # --- setup.sh is a shim, not a second flag list ----------------------------- #: The only flags `setup.sh` may still NAME, and why each of them is about the @@ -1867,7 +2014,8 @@ def test_a_controlling_terminal_that_is_a_file_never_answers(monkeypatch, opts = module.parse_args(["--project", "Sample", "--org", "demoorg"], str(tmp_path)) with pytest.raises(module.Refusal) as refused: - module.plan_and_confirm(opts, REPO, "demoorg", []) + module.plan_and_confirm(opts, REPO, "demoorg", [], + str(tmp_path / "Sample")) assert refused.value.code == 2 assert_refuses_for_want_of_a_terminal(str(refused.value)) diff --git a/tests/test_setup_sh.py b/tests/test_setup_sh.py index 0a1cce63..07e56c3c 100644 --- a/tests/test_setup_sh.py +++ b/tests/test_setup_sh.py @@ -28,6 +28,10 @@ from conftest import PTY_SKIP, run_in_a_terminal +from conftest import (TAKEN_CLONE_PATHS, + assert_a_rehearsals_clone_stops_the_real_run, + assert_a_taken_clone_path_is_refused) + SETUP = REPO / "setup.sh" DEGRADE_LINE = "authority is not wallet-carried in this org" @@ -393,6 +397,19 @@ def test_passthrough_reaches_the_scaffold(tmp_path): assert 'reference: "a-staged-fragment.md"' in manifest +def test_a_rehearsals_clone_stops_the_real_run_before_anything(tmp_path): + """#498, through the shim: `conftest.assert_a_rehearsals_clone_stops_ + the_real_run`.""" + assert_a_rehearsals_clone_stops_the_real_run(run_setup, tmp_path) + + +@pytest.mark.parametrize("kind", TAKEN_CLONE_PATHS) +def test_a_taken_clone_path_is_refused_before_anything(tmp_path, kind): + """#498, through the shim: `conftest.assert_a_taken_clone_path_is_ + refused`.""" + assert_a_taken_clone_path_is_refused(run_setup, tmp_path, kind) + + def test_no_test_here_depends_on_gh_being_authenticated(): """Every invocation either stops before the preflight or runs offline. From 121c204bf6d68182073ee7d042427a82337390a2 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Tue, 13 Oct 2026 11:48:14 +0000 Subject: [PATCH 2/3] README.md, AGENTS.md and docs/handbook.html: a clone path anything already stands at is refused up front, so the rehearsal's `./Atlas` is no longer optional to delete before a real run in the same folder, AGENTS.md names it as the third refusal an assistant can foresee before anything is created, and "What setup.sh does" no longer says the flow has no behaviour of its own (#498) The tool half of #498 refuses `/` in step (3) when anything stands there. The documents still said otherwise. Brett Heap ruled at 2026-10-13T11:30Z that each lane-4 fix pull request carries its own doc lines, so they are corrected here, on the text #505 (af01068b) left. Each file keeps its line count: README.md stays at its cap of 1422, AGENTS.md at 490, and docs/handbook.html at 1933. - README.md:306-313. The rehearsal "leaves behind ... all of which you can delete" now reads "all deletable; a real run here refuses up front while `./Atlas` is there". The rest of that paragraph is rewrapped with its words unchanged. - README.md:524-525. "What setup.sh does" no longer says the flow runs its commands "with no behaviour of its own". It says that before the second command the flow refuses, creating nothing, if anything stands at `/Atlas`. - AGENTS.md:88-96. "Two you can foresee from your flags" becomes "Three you can foresee". The third is anything already at `/`, a rehearsal's clone included. The assistant asks the human to move or delete it and never does either itself. Both refusal codes are still spelled as before. - AGENTS.md:110-113. The steps by hand begin with "a free clone path". - docs/handbook.html:869-872, 956-957 and 1818-1825 are the README's and AGENTS.md's twins of the above. The footer at 1872-1875 adds "a taken clone path refused up front, added (#498)" and sets the README blob to `git hash-object README.md`. README.md:445 ("clones the root as `./Atlas` where you ran it") is left as it is. Saying more there fits only by rewrapping all ten lines from 436 to 445, which belong to other findings. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 26 +++++++++++++------------- README.md | 20 ++++++++++---------- docs/handbook.html | 36 ++++++++++++++++++------------------ 3 files changed, 41 insertions(+), 41 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 4b94a0a7..0719e9ab 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -85,15 +85,15 @@ to confirm on`. Give the human the same command for THEIR OWN terminal, where it asks them, the one-liner too: setup reads the answer from that terminal and never from the pipe. Never pass `--yes` yourself. -If it refuses, read the refusal: each one names what to run instead, or, for -some, the VALUE to change. Two you can foresee from your flags, both before -anything is created: `scaffold-leg-path-collides` (a `--spec-path` or -`--code-path` that is, holds or lies inside a FILE the shape writes into the -assembly root: `project.yaml` is one, and `contracts` holds -`contracts/spec-pin.yaml`; the defaults `spec` and `code` pass) and -`scaffold-pin-collides` (a `--pin` the shape cannot write, below). Both go -after `--`. Relay the refusal and ask the human for the value; never choose -another yourself. Do not retry with `--force`; there is none, deliberately. +If it refuses, read the refusal: each one names what to run instead, or, for some, the +VALUE to change. Three you can foresee, all before anything is created: +`scaffold-leg-path-collides` (a `--spec-path` or `--code-path` that is, holds or lies +inside a FILE the shape writes into the assembly root: `project.yaml` is one, and +`contracts` holds `contracts/spec-pin.yaml`; the defaults `spec` and `code` pass) and +`scaffold-pin-collides` (a `--pin` the shape cannot write, below), both after `--`; +and anything already at `/`, a rehearsal's clone included. Relay the +refusal and ask the human for the value, or to move or delete what is there; never do +either yourself. Do not retry with `--force`; there is none, deliberately. ## 3. It finishes the job @@ -107,10 +107,10 @@ copied and digest-pinned — and you read that before touching the project. ## If setup.sh cannot be used -The same steps by hand, in order: `validate-repository-naming.py --explain` on -the three names, `scaffold-project.py --dry-run`, an explicit human yes, -`scaffold-project.py` for real, then `git clone --recurse-submodules` and -`python3 scripts/bootstrap.py`. README's "What setup.sh does" carries the exact commands. +The same steps by hand, in order: a free clone path, `validate-repository-naming.py +--explain` on the three names, `scaffold-project.py --dry-run`, an explicit human yes, +`scaffold-project.py` for real, then `git clone --recurse-submodules` and `python3 +scripts/bootstrap.py`. README's "What setup.sh does" carries the exact commands. ## Adopting an existing repository diff --git a/README.md b/README.md index 1417b9ec..43df8f47 100644 --- a/README.md +++ b/README.md @@ -303,14 +303,14 @@ checkout instead and nothing touches the network at all. On Windows the same flag goes on the Python entry point — `py setup-project.py Atlas --org --local-remote-dir .\rehearsal` — which, run from a download, still clones the standard into a temporary directory first, and from a checkout clones -nothing. What it leaves behind is `./Atlas`, three bare repositories in -`./rehearsal` and the scaffold's three working trees, in the temporary directory -its `Working trees are at` line names, all of which you can delete. `python3 -setup-project.py --help` and, from a checkout, `setup.sh --help` list every -flag, this one included. From a download, `setup.sh --help` clones nothing and -lists none: it names `setup-project.py --help`, which answers offline from that -one file too, and [`docs/cli.md`](docs/cli.md), every tool's help and every -subcommand's in one file, generated from the tools themselves. +nothing. It leaves `./Atlas`, three bare repositories in `./rehearsal` and the +scaffold's three working trees, in the temporary directory its `Working trees are +at` line names, all deletable; a real run here refuses up front while `./Atlas` +is there. `python3 setup-project.py --help` and, from a checkout, `setup.sh +--help` list every flag, this one included. From a download, `setup.sh --help` +clones nothing and lists none: it names `setup-project.py --help`, which answers +offline from that one file too, and [`docs/cli.md`](docs/cli.md), every tool's +help and every subcommand's in one file, generated from the tools themselves. Without installing anything — no fork, no manual clone — one command: @@ -521,8 +521,8 @@ confers nothing and stays neutral. `setup.sh` is a shim that finds a Python 3.9 or newer, clones this standard into a temporary directory when you are not already in a checkout of it, and hands over to `setup-project.py`, which IS the flow. These are the commands that flow runs, in -order, with no behaviour of its own (the first only in self-bootstrap mode, from the -one-liner above): +order (the first only in self-bootstrap mode, from the one-liner above); before +the second it refuses, creating nothing, if anything stands at `/Atlas`: ```sh git clone --depth 1 https://github.com/opensoft/openRepoShape.git diff --git a/docs/handbook.html b/docs/handbook.html index ee0b9cc7..20af4f15 100644 --- a/docs/handbook.html +++ b/docs/handbook.html @@ -866,10 +866,10 @@

Rehearse first, creating nothing

Honestly: it still reaches GitHub before the run starts — to fetch setup.sh, then for the self-bootstrap clone of this standard into a temporary directory. Run the same flag on ./setup.sh from a checkout - instead and nothing touches the network at all. What it leaves behind is - ./Atlas, three bare repositories in ./rehearsal, and the - scaffold’s three working trees in the temporary directory its - Working trees are at line names, all of which you can delete.

+ instead and nothing touches the network at all. It leaves ./Atlas, three + bare repositories in ./rehearsal, and the scaffold’s three working + trees in the temporary directory its Working trees are at line names, all + deletable; a real run here refuses up front while ./Atlas is there.

Honestly: it still reaches GitHub before the run starts — to fetch setup.sh, then for the self-bootstrap clone of this standard into a temporary directory. Run the same flag on ./setup.sh from a checkout @@ -953,8 +953,8 @@

What setup.sh actually runs

It is a shim that finds a Python 3.9 or newer, clones this standard into a temporary directory when you are not already in a checkout of it, and hands over to setup-project.py, which IS the flow. These are the commands that flow runs, - in order, with no behaviour of its own (the first only in self-bootstrap mode, from the - one-liner):

+ in order (the first only in self-bootstrap mode, from the one-liner); before the second + it refuses, creating nothing, if anything stands at <into>/Atlas:

git clone --depth 1 https://github.com/opensoft/openRepoShape.git <tmp-dir>
 python3 scripts/validate-repository-naming.py --explain Atlas Atlas-spec Atlas-code
 python3 scaffold-project.py --org <your-org> --project Atlas --dry-run
@@ -1815,14 +1815,14 @@ 

For AI assistants

it asks them, the one-liner too — setup reads the answer from that terminal and never from the pipe — and never pass --yes yourself. Read a refusal rather than retrying: each one names what to run instead, or, for some, the VALUE to change — - among them a --spec-path or - --code-path that is, holds or lies inside a file the shape writes, and a - --pin the shape cannot write, both before anything is created — and there - is no --force, deliberately. Relay the finished block — the clone - path, the three repository URLs and the next commands — and say that the line - authority is not wallet-carried in this org is a report and not a fault. The new - project carries AGENTS-shape.md at its assembly root, and you read that before - touching the project.

+ among them a --spec-path or --code-path that is, holds or lies + inside a file the shape writes, a --pin the shape cannot write, and anything + already at <into>/<Project>, for the human to move or delete, all + before anything is created — and there is no --force, deliberately. Relay + the finished block — the clone path, the three repository URLs and the next commands + — and say that the line authority is not wallet-carried in this org is a + report and not a fault. The new project carries AGENTS-shape.md at its assembly + root, and you read that before touching the project.

What you must not tell them: that the shape confers anything — or that membership of a family does — or that a <user>-wip workspace repository does, which holds no code, elects nothing and is membership of nothing. Electing @@ -1869,10 +1869,10 @@

For AI assistants

added (#411); the doctor’s merge row, and INVALID (merge) above drift, added (#424); the assistant’s route when its shell has no terminal, added (#476); the one-liner asking at the person’s own terminal, never reading the answer - from the pipe, added (#492); the statements the v1.0.0 accuracy pass found false, - corrected here as in the README, added (#483) — a page cannot name the commit - that contains it, and the blob below is the fact a test checks. - README.md blob 1417b9ec0235675da8f73ca023756411bc3fbe8a. Every command, flag, exit code + from the pipe, added (#492); the statements the v1.0.0 accuracy pass found false, corrected + here as in the README, added (#483); a taken clone path refused up front, added (#498) — + a page cannot name the commit that contains it, and the blob below is the fact a test checks. + README.md blob 43df8f47b8a70678a2ff1836f01468a686e41eb6. Every command, flag, exit code and file name on this page is quoted from that README and from AGENTS.md.

From 7a97d53b952ca0fae73d9ddc4dceff4e798e1816 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Tue, 13 Oct 2026 11:52:15 +0000 Subject: [PATCH 3/3] setup-project.py: a taken clone path that only looks like a rehearsal's clone is said to LOOK like one and is offered for deletion only "if nothing in it is yours", a symbolic link there is named as one and never offered for deletion, and a credential is cut at the last `@`, so a real working clone of a local or network repository with unpushed work is no longer told "It is the clone a rehearsal left behind ... Move or delete it" (#498) The independent review of #509 at 427237a found this (MUST-1). The three signs `what_stands_at` reads are an `origin` on this disk, named `.git`, and a `project.yaml` at the root. A working clone of a repository on a local path, a NAS mount or a UNC share meets all three, and so does a rehearsal's clone somebody has since worked in. The refusal stated a guess as a fact and offered deletion outright, and an assistant relaying "delete it" could destroy unpushed work. - The rehearsal sentence now reads "It looks like the clone a rehearsal left behind: its `origin` is ..., a repository on this disk, as `--local-remote-dir` makes one." The way out is "Move it, or delete it if nothing in it is yours, then run this again." Only an empty directory still gets a plain "Move or delete it". - The `origin` is still read with `git config --file`, and no git runs inside a directory this tool did not make: its config can name a command (`core.fsmonitor`) that `git status` executes. That is why unpushed work is not checked for, and why the wording carries the condition instead. - `what_stands_at` returns its way out with its sentence, so the three strengths (`DISCARD_IT`, `DISCARD_IT_IF_UNUSED`, `KEEP_IT`) are chosen where the thing is identified. - A symbolic link at the clone path is named first: "It is a symbolic link to .", with KEEP_IT. `os.path.isdir` follows a link, so a link to an empty directory used to be offered "Move or delete it", and `rm -r Sample/` acts on the target. That was NIT-2. - A directory whose `origin` cannot be read says "It is a directory holding a `.git`." rather than "It is a git repository.". An empty `.git`, or a `.git` file naming a missing gitdir, is not a repository (NIT-2). - `without_credentials` cuts up to the LAST `@` before the path, so `https://dana:p@SECRET3@github.com/...` no longer leaves `SECRET3` showing. It now applies to the rehearsal sentence too (NIT-1). Tests, in tests/test_setup_project_py.py, with conftest's rehearsal assertion following the new wording: - a working clone of a local bare repository setup never made, holding an unpushed commit, is told only that it looks like a rehearsal's clone, and never "Move or delete it" (MUST-1); - an `origin` carrying an ESC sequence and a newline before a forged `REFUSED:` line. It is written into `.git/config` as bytes and read back by git first, and is never echoed (SHOULD-1, which pins `CONTROL_RE`); - the step (7) backstop with a symbolic link to nothing at the path, which pins its `lexists` (SHOULD-2); - a password holding `@`, and a `file://` origin with credentials (NIT-1); - a link to nothing and a link to an empty directory, each named as a link with KEEP_IT (NIT-2). Each change reverted alone fails these tests. The old rehearsal wording fails 4. Dropping the control-character filter fails 1. The backstop asking `exists` fails 1. The first-`@` cut fails 1. Dropping the link branch fails 2. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 --- setup-project.py | 69 ++++++++----- tests/conftest.py | 22 ++-- tests/test_setup_project_py.py | 179 +++++++++++++++++++++++++-------- 3 files changed, 193 insertions(+), 77 deletions(-) diff --git a/setup-project.py b/setup-project.py index 2a71d5f3..1f226080 100755 --- a/setup-project.py +++ b/setup-project.py @@ -1666,11 +1666,17 @@ def resolve_family(opts: Options, shape_root: Path) -> Landing: # 3c. the clone path, while nothing exists # --------------------------------------------------------------------------- -#: THE TWO WAYS OUT OF A TAKEN CLONE PATH. Deleting is offered only for what -#: this tool can tell is worth nothing: an empty directory, and the clone a -#: rehearsal left behind. Anything else is somebody's, and is moved, or the -#: project is put somewhere else; this tool deletes and moves nothing itself. +#: THE WAYS OUT OF A TAKEN CLONE PATH. This tool deletes and moves nothing +#: itself, and it offers deleting only as strongly as it KNOWS that nothing +#: is lost. An empty directory holds nothing: move or delete it. What looks +#: like a rehearsal's clone is a GUESS - a real working clone of a local, NAS +#: or UNC repository named `.git` looks the same, unpushed commits +#: and all - so deleting is offered on the person's own condition (#498, +#: review round 1). Anything else is somebody's: move it, or put the project +#: somewhere else. DISCARD_IT = "Move or delete it, then run this again." +DISCARD_IT_IF_UNUSED = ("Move it, or delete it if nothing in it is yours, then " + "run this again.") KEEP_IT = "Move it, or choose another --into, then run this again." @@ -1685,6 +1691,8 @@ def origin_recorded_in(clone: str) -> str: config --file` reads that one file, with no includes and no URL rewriting. A `.git` FILE (a worktree, a submodule) keeps its config elsewhere, so there is nothing here to read and the caller says less. + And no git runs INSIDE a directory this tool did not make: its config + can name a command (`core.fsmonitor`) that `git status` executes. A value carrying a control character is not echoed: it would print a line this tool did not write (`CONTROL_RE`). @@ -1708,42 +1716,55 @@ def is_on_this_disk(url: str) -> bool: def without_credentials(url: str) -> str: """`url` with any `user:token@` after its scheme dropped: an `origin` is echoed into a refusal an assistant may relay, and a token is not part of - what is there.""" - return re.sub(r"(?<=://)[^/@]*@", "", url, count=1) + what is there. Up to the LAST `@` before the path, because a password + holding a bare `@` would otherwise leave its tail on the screen.""" + return re.sub(r"(?<=://)[^/]*@", "", url, count=1) def what_stands_at(clone: str, project: str) -> tuple: - """`(one sentence saying what is at clone, whether it can be thrown - away)`, or `("", False)` when this cannot tell. + """`(one sentence saying what is at clone, the way out)`; the sentence + is "" when this cannot tell. A REHEARSAL'S CLONE IS THE CASE THIS EXISTS FOR (#498). README's "Rehearse first" leaves `./` behind, cloned from the bare `/.git` that `--local-remote-dir` made, and the real run - that follows it in the same folder lands on exactly that path. It is - told apart by all three of: an `origin` on this disk, named - `.git`, and a `project.yaml` at its root. A clone of a - repository on GitHub, or of a local mirror under another name, is - somebody's working copy and is never offered for deletion. + that follows it in the same folder lands on exactly that path. It LOOKS + like one when all three hold: an `origin` on this disk, named + `.git`, and a `project.yaml` at its root. That is said as a + look, never as a fact, and deleting it is offered only "if nothing in + it is yours": a working clone of a local or network repository meets + all three, and this tool cannot see its unpushed work without running + git inside it (`origin_recorded_in` says why it does not). + + A SYMBOLIC LINK IS SAID TO BE ONE, first, and never offered for + deletion: `os.path.isdir` follows it, and `rm -r /` on the + advice "delete it" would act on its target. """ try: + if os.path.islink(clone): + target = os.readlink(clone) + if not target or CONTROL_RE.search(target): + return "It is a symbolic link.", KEEP_IT + return "It is a symbolic link to %s." % target, KEEP_IT if not os.path.isdir(clone): - return "", False + return "", KEEP_IT if not os.path.lexists(os.path.join(clone, ".git")): if os.listdir(clone): - return "It is a directory that is not a git clone.", False - return "It is an empty directory.", True + return "It is a directory that is not a git clone.", KEEP_IT + return "It is an empty directory.", DISCARD_IT except OSError: - return "", False + return "", KEEP_IT origin = origin_recorded_in(clone) if not origin: - return "It is a git repository.", False + return "It is a directory holding a `.git`.", KEEP_IT + shown = without_credentials(origin) if (is_on_this_disk(origin) and os.path.basename(origin.rstrip("/\\")) == project + ".git" and os.path.isfile(os.path.join(clone, "project.yaml"))): - return ("It is the clone a rehearsal left behind: its `origin` is %s, " - "a repository on this disk, as `--local-remote-dir` makes " - "one." % origin), True - return "It is a clone of %s." % without_credentials(origin), False + return ("It looks like the clone a rehearsal left behind: its `origin` " + "is %s, a repository on this disk, as `--local-remote-dir` " + "makes one." % shown), DISCARD_IT_IF_UNUSED + return "It is a clone of %s." % shown, KEEP_IT def refuse_a_taken_clone_path(clone: str, project: str) -> None: @@ -1771,11 +1792,11 @@ def refuse_a_taken_clone_path(clone: str, project: str) -> None: """ if not os.path.lexists(clone): return - what, disposable = what_stands_at(clone, project) + what, way_out = what_stands_at(clone, project) die(" ".join(part for part in ( "%s already exists, and that is where this project would be cloned, " "so the run stops here and nothing was created." % clone, - what, DISCARD_IT if disposable else KEEP_IT) if part)) + what, way_out) if part)) # --------------------------------------------------------------------------- diff --git a/tests/conftest.py b/tests/conftest.py index 2ea85e5b..f6672b8c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -855,9 +855,12 @@ def assert_refuses_for_want_of_a_terminal(stderr: str) -> None: #: `assert_a_taken_clone_path_is_refused` makes each one. TAKEN_CLONE_PATHS = ("empty directory", "file", "directory") -#: The refusal's two ways out: deleting is offered only for what holds -#: nothing of anybody's. +#: The refusal's ways out: deleting is offered outright only for what holds +#: nothing of anybody's, and for what only LOOKS like a rehearsal's clone on +#: the person's own condition (#498, review round 1). DISCARD_IT = "Move or delete it, then run this again." +DISCARD_IT_IF_UNUSED = ("Move it, or delete it if nothing in it is yours, then " + "run this again.") KEEP_IT = "Move it, or choose another --into, then run this again." @@ -879,9 +882,10 @@ def assert_a_rehearsals_clone_stops_the_real_run(run, tmp_path: Path) -> None: """README's own order, in one folder: rehearse with `--local-remote-dir ./rehearsal`, then run for real (a second `--local-remote-dir` stands in for GitHub). The rehearsal names its clone path in the plan and leaves - `./Sample` behind; the real run refuses in step (3), names that clone - as the rehearsal's and offers to delete it, and creates nothing. On main - it created all three repositories and only then refused, in step (7).""" + `./Sample` behind; the real run refuses in step (3), says that clone + LOOKS like the rehearsal's, offers deleting it only if nothing in it is + the person's, and creates nothing. On main it created all three + repositories and only then refused, in step (7).""" here = tmp_path / "here" clone = os.path.abspath(str(here / "Sample")) rehearsal = run("--project", "Sample", "--yes", "--org", "demoorg", @@ -895,10 +899,10 @@ def assert_a_rehearsals_clone_stops_the_real_run(run, tmp_path: Path) -> None: real = run("--project", "Sample", "--yes", "--org", "demoorg", "--local-remote-dir", str(here / "real"), "--into", str(here)) rest = refused_in_step_three(real, clone) - assert rest.startswith("It is the clone a rehearsal left behind: its " - "`origin` is "), rest - assert ("a repository on this disk, as `--local-remote-dir` makes one. " - + DISCARD_IT) in rest, rest + assert rest.startswith("It looks like the clone a rehearsal left behind: " + "its `origin` is "), rest + assert rest.endswith("a repository on this disk, as `--local-remote-dir` " + "makes one. " + DISCARD_IT_IF_UNUSED), rest assert not (here / "real").exists(), "the real run created repositories" assert (here / "Sample" / "project.yaml").is_file() diff --git a/tests/test_setup_project_py.py b/tests/test_setup_project_py.py index cad3eaad..95481f11 100644 --- a/tests/test_setup_project_py.py +++ b/tests/test_setup_project_py.py @@ -40,7 +40,8 @@ from conftest import PTY_SKIP, run_in_a_terminal -from conftest import (KEEP_IT, TAKEN_CLONE_PATHS, +from conftest import (DISCARD_IT, DISCARD_IT_IF_UNUSED, KEEP_IT, + TAKEN_CLONE_PATHS, assert_a_rehearsals_clone_stops_the_real_run, assert_a_taken_clone_path_is_refused) @@ -1685,15 +1686,22 @@ def test_a_rehearsal_that_stops_at_the_plan_makes_no_family_folder(tmp_path): # --- what stands at a taken clone path (#498) -------------------------------- # # The runs through both entry points are `conftest`'s, wrapped above. These -# ask the pieces in-process: what the refusal can tell about a clone, a link -# to nothing, and the step (7) backstop, which a run reaches only when the +# ask the pieces in-process: what the refusal can tell about a clone, a +# symbolic link, and the step (7) backstop, which a run reaches only when the # path appears while it is going. #: An `origin` carrying a token, as an HTTPS clone can record one. WITH_A_TOKEN = "https://dana:ghp_notreal@github.com/Northwind/Sample.git" -REHEARSALS = ("It is the clone a rehearsal left behind: its `origin` is %s, a " - "repository on this disk, as `--local-remote-dir` makes one.") +#: A password holding a bare `@`: a malformed URL, and still no part of it +#: is echoed (#509's review, NIT-1). +WITH_AN_AT = "https://dana:p@SECRET3@github.com/Northwind/Sample.git" + +REHEARSALS = ("It looks like the clone a rehearsal left behind: its `origin` " + "is %s, a repository on this disk, as `--local-remote-dir` " + "makes one.") + +HOLDS_A_GIT = "It is a directory holding a `.git`." def _a_clone(taken: Path, origin=None, shape_root: bool = False) -> None: @@ -1707,63 +1715,132 @@ def _a_clone(taken: Path, origin=None, shape_root: bool = False) -> None: (taken / "project.yaml").write_bytes(b"schema_version: 1\n") +def _an_origin_with_controls(taken: Path) -> None: + """`taken` with a `project.yaml` and an `origin` named `Sample.git` on + this disk whose value holds an ESC sequence and a newline before a + forged `REFUSED:` line. Appended to `.git/config` as BYTES, so no argv + and no code page carries it; git reads it back as written, which the + test asserts first, so the filter and not a parse failure is what keeps + it off the screen.""" + _a_clone(taken, shape_root=True) + with open(taken / ".git" / "config", "ab") as config: + config.write(b'[remote "origin"]\n' + b'\turl = "/srv\x1b[2J\\nREFUSED: forged/Sample.git"\n') + + @pytest.mark.parametrize("case", [ - "github", "the-rehearsals", "the-rehearsals-as-a-file-url", + "github", "a-password-with-an-at", "the-rehearsals", + "the-rehearsals-as-a-file-url", "a-file-url-with-credentials", "a-mirror-under-another-name", "rehearsal-shaped-with-no-project-yaml", - "no-origin", "a-git-file", "an-empty-git-inside-another-repository"]) + "no-origin", "a-git-file", "an-empty-git-inside-another-repository", + "an-origin-with-control-characters"]) def test_what_stands_at_a_taken_clone_path(tmp_path, case): - """`what_stands_at`: a sentence and whether deleting is offered. - - Deleting is offered for a rehearsal's clone alone, told by all three of - an `origin` on this disk, its name `.git`, and a `project.yaml`: - a mirror under another name and a clone with no manifest are somebody's. - A token in an `origin` is not echoed. And the `origin` is read from the - directory's OWN `.git/config`: an empty `.git` inside a repository that - has one must not be reported as a clone of THAT repository, which is - what `git -C` finds by walking up.""" + """`what_stands_at`: a sentence, and the way out it offers. + + What LOOKS like a rehearsal's clone - an `origin` on this disk, named + `.git`, and a `project.yaml` - is said to look like one, and + deleting it is offered only if nothing in it is the person's (#509's + review, MUST-1). A mirror under another name and a clone with no + manifest are somebody's, and so is anything this cannot read. No + credential and no control character is echoed. And the `origin` is + read from the directory's OWN `.git/config`: an empty `.git` inside a + repository that has one must not be reported as a clone of THAT + repository, which is what `git -C` finds by walking up.""" module = entry_point_module() taken = tmp_path / "here" / "Sample" rehearsal = tmp_path / "here" / "rehearsal" / "Sample.git" mirror = tmp_path / "mirrors" / "Mirror.git" + on_disk = "/" + rehearsal.as_posix().lstrip("/") + github = ("It is a clone of https://github.com/Northwind/Sample.git.", + KEEP_IT) if case == "github": _a_clone(taken, WITH_A_TOKEN, shape_root=True) - expected = ("It is a clone of " - "https://github.com/Northwind/Sample.git.", False) + expected = github + elif case == "a-password-with-an-at": + _a_clone(taken, WITH_AN_AT, shape_root=True) + expected = github elif case == "the-rehearsals": _a_clone(taken, rehearsal, shape_root=True) - expected = (REHEARSALS % rehearsal, True) + expected = (REHEARSALS % rehearsal, DISCARD_IT_IF_UNUSED) elif case == "the-rehearsals-as-a-file-url": _a_clone(taken, rehearsal.as_uri(), shape_root=True) - expected = (REHEARSALS % rehearsal.as_uri(), True) + expected = (REHEARSALS % rehearsal.as_uri(), DISCARD_IT_IF_UNUSED) + elif case == "a-file-url-with-credentials": + _a_clone(taken, "file://dana:SECRET7@" + on_disk, shape_root=True) + expected = (REHEARSALS % ("file://" + on_disk), DISCARD_IT_IF_UNUSED) elif case == "a-mirror-under-another-name": _a_clone(taken, mirror, shape_root=True) - expected = ("It is a clone of %s." % mirror, False) + expected = ("It is a clone of %s." % mirror, KEEP_IT) elif case == "rehearsal-shaped-with-no-project-yaml": _a_clone(taken, rehearsal) - expected = ("It is a clone of %s." % rehearsal, False) + expected = ("It is a clone of %s." % rehearsal, KEEP_IT) elif case == "no-origin": _a_clone(taken) - expected = ("It is a git repository.", False) + expected = (HOLDS_A_GIT, KEEP_IT) elif case == "a-git-file": taken.mkdir(parents=True) (taken / ".git").write_bytes(b"gitdir: ../elsewhere/.git\n") - expected = ("It is a git repository.", False) - else: + expected = (HOLDS_A_GIT, KEEP_IT) + elif case == "an-empty-git-inside-another-repository": _a_clone(taken.parent, "https://example.invalid/Outer.git") (taken / ".git").mkdir(parents=True) - expected = ("It is a git repository.", False) - assert module.what_stands_at(str(taken), "Sample") == expected - assert "ghp_notreal" not in expected[0] - - -def test_a_link_to_nothing_takes_the_clone_path(tmp_path): - """`os.path.exists` reads a symbolic link to nothing as absent, and - `git clone` still refuses it (`could not create work tree dir`), so - step (3) asks `lexists`. A free path is the control: it is not refused.""" + expected = (HOLDS_A_GIT, KEEP_IT) + else: + _an_origin_with_controls(taken) + read = git("config", "--file", str(taken / ".git" / "config"), + "--get", "remote.origin.url", cwd=tmp_path).stdout + assert "\x1b[2J" in read and "\nREFUSED: forged" in read, repr(read) + expected = (HOLDS_A_GIT, KEEP_IT) + said = module.what_stands_at(str(taken), "Sample") + assert said == expected + for leaked in ("ghp_notreal", "SECRET3", "SECRET7", "\x1b", "forged"): + assert leaked not in said[0], said + + +def test_a_working_clone_of_a_local_repository_is_not_called_a_rehearsals( + tmp_path): + """#509's review, MUST-1: a clone of a local bare repository setup never + made, holding a commit nobody has pushed, meets every sign a rehearsal's + clone does - an `origin` on this disk named `Sample.git`, and a + `project.yaml`. It may be somebody's work, so the refusal says only that + it LOOKS like a rehearsal's clone, and offers deleting it only if + nothing in it is theirs: never "It is the clone a rehearsal left + behind", and never the bare "Move or delete it".""" + module = entry_point_module() + bare = tmp_path / "srv" / "Sample.git" + git("init", "-q", "--bare", str(bare), cwd=tmp_path) + work = tmp_path / "here" / "Sample" + work.parent.mkdir() + git("clone", "-q", str(bare), str(work), cwd=tmp_path) + (work / "project.yaml").write_bytes(b"schema_version: 1\n") + git("add", "project.yaml", cwd=work) + git("commit", "-q", "-m", "real work, not pushed", cwd=work) + with pytest.raises(module.Refusal) as refused: + module.refuse_a_taken_clone_path(str(work), "Sample") + said = str(refused.value) + assert ("It looks like the clone a rehearsal left behind: its `origin` " + "is ") in said, said + assert said.endswith("makes one. " + DISCARD_IT_IF_UNUSED), said + assert "It is the clone a rehearsal left behind" not in said, said + assert DISCARD_IT not in said, said + + +@pytest.mark.parametrize("target", ["nowhere", "an-empty-directory"]) +def test_a_symbolic_link_takes_the_clone_path_and_is_said_to_be_one(tmp_path, + target): + """`os.path.exists` reads a link to nothing as absent, and `git clone` + still refuses it (`could not create work tree dir`), so step (3) asks + `lexists`. A link is said to be a link, and is never offered for + deletion: `os.path.isdir` follows one to an empty directory, and `rm -r + Sample/` on "delete it" would act on the target (#509's review, NIT-2). + A free path is the control: it is not refused.""" module = entry_point_module() + aim = tmp_path / target + if target != "nowhere": + aim.mkdir() link = tmp_path / "Sample" try: - os.symlink(tmp_path / "nowhere", link) + os.symlink(aim, link, target_is_directory=True) except (NotImplementedError, OSError) as exc: pytest.skip(f"this platform cannot create a symlink here: {exc}") assert module.refuse_a_taken_clone_path(str(tmp_path / "Free"), @@ -1771,18 +1848,24 @@ def test_a_link_to_nothing_takes_the_clone_path(tmp_path): with pytest.raises(module.Refusal) as refused: module.refuse_a_taken_clone_path(str(link), "Sample") assert refused.value.code == 2 - assert str(refused.value) == ( + said = str(refused.value) + assert said.startswith( "%s already exists, and that is where this project would be cloned, " - "so the run stops here and nothing was created. %s" % (link, KEEP_IT)) + "so the run stops here and nothing was created. It is a symbolic " + "link to " % link), said + assert said.endswith("%s. %s" % (target, KEEP_IT)), said -def test_a_path_taken_after_step_three_is_the_backstops(monkeypatch, - tmp_path): +@pytest.mark.parametrize("taken", ["a-directory", "a-link-to-nothing"]) +def test_a_path_taken_after_step_three_is_the_backstops(monkeypatch, tmp_path, + taken): """Step (7) still asks, for a path that appeared while the run was going, - after all three repositories exist. Its way out moves the thing aside - and clones from the clone's PARENT: as it used to read, the printed - `git clone` met the same directory, and `cd ` from anywhere - else entered whatever stood there. Nothing is cloned.""" + after all three repositories exist, and asks `lexists` as step (3) does: + a link to nothing is a path `git clone` refuses (#509's review, + SHOULD-2). Its way out moves the thing aside and clones from the clone's + PARENT: as it used to read, the printed `git clone` met the same + directory, and `cd ` from anywhere else entered whatever stood + there. Nothing is cloned.""" module = entry_point_module() runs = _Runs() monkeypatch.setattr(module, "run", runs) @@ -1791,7 +1874,15 @@ def test_a_path_taken_after_step_three_is_the_backstops(monkeypatch, "--into", str(tmp_path / "work")], str(tmp_path)) parent = Path(opts.into) - (parent / "Sample").mkdir(parents=True) + parent.mkdir(parents=True) + if taken == "a-directory": + (parent / "Sample").mkdir() + else: + try: + os.symlink(tmp_path / "nowhere", parent / "Sample", + target_is_directory=True) + except (NotImplementedError, OSError) as exc: + pytest.skip(f"this platform cannot create a symlink here: {exc}") with pytest.raises(module.Refusal) as refused: module.clone_and_bootstrap(opts, "demoorg", module.Landing(parent, "", None))