From 9f429f40b6910946ef88f6ae44268c816b17f076 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 16 Oct 2026 23:38:02 +0000 Subject: [PATCH 1/6] setup-project.py: a project name typed at the prompt is checked as `--project` is and an empty line says no name was typed, so `../x`, `a/b`, `..` and `--help` are refused by name, not by an `--id` hint, a help page or a probe write (#729) The name typed at the `project name` prompt was used as typed. The same value as `--project` or as the positional goes through `checked_value` at parse time and is refused there. Typed, it reached step (3), which builds the clone path from it and asks the disk about that path: `..` leaves `--into`, an absolute name replaces it, and #608's write probe is made in the nearest folder above it. Step (4) did not catch it either: the naming validator reads only the last part of what it is handed (`../x` passes as `x`), has no length bound and reads a leading `-` as its own option, so `--help` printed its help and exited 0. The scaffold's plan then refused `../x` as `scaffold-bad-id: --id '../x'`, an id nobody passed, and `-h` with a usage dump. Now `checked_value("project name", typed)` runs right after the `ask`, the check `take_positional` makes, so the refusal is the tool's own wording with `project name` as its subject, exit 2, before anything reads or writes a path. `a b` and `Bad_Name` still go to step (4): `NAME_RE` is permissive on purpose and the policy rules on those. Nothing in step (3) changes, and the validator's basename rule stays as it is. An empty typed line (and Ctrl-D) now says "no project name was typed; nothing was created.", exit 2, with no usage text. It said "no --project given, and no terminal to ask on." after the full usage text, which is the refusal for no terminal, and `can_ask()` still gives that one. tests/test_setup_project_typed_name.py types each name at a pseudo-terminal (POSIX only, as the other terminal tests are). Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- setup-project.py | 14 ++- tests/test_setup_project_typed_name.py | 136 +++++++++++++++++++++++++ 2 files changed, 149 insertions(+), 1 deletion(-) create mode 100644 tests/test_setup_project_typed_name.py diff --git a/setup-project.py b/setup-project.py index 1b2cab7f..b774914b 100755 --- a/setup-project.py +++ b/setup-project.py @@ -1477,7 +1477,19 @@ def resolve_project_and_elector(opts: Options, shape_root: Path) -> None: say("") say("(3) project") if not opts.project and can_ask(): - opts.project = ask(" project name (ONE CamelCase token, e.g. Atlas): ") + typed = ask(" project name (ONE CamelCase token, e.g. Atlas): ") + # AN EMPTY LINE IS A NAME NOBODY TYPED, NOT A MISSING TERMINAL (#729): + # `can_ask()` above is the gate that refuses for want of one, so this + # refusal says what happened and does not print the usage text. + if not typed: + die("no project name was typed; nothing was created.") + # THE TYPED NAME IS HELD TO THE CHECK THE SAME NAME GETS AS `--project` + # or as the positional (#729), before anything builds a path from it: + # `..` leaves `--into`, an absolute name replaces it, and step (4)'s + # validator reads only the last part of what it is handed. `a b` and + # `Bad_Name` pass here on purpose (`NAME_RE` is permissive); step (4) + # is where the policy rules on those. + opts.project = checked_value("project name", typed) if not opts.project: usage(sys.stderr) die("no --project given, and no terminal to ask on.") diff --git a/tests/test_setup_project_typed_name.py b/tests/test_setup_project_typed_name.py new file mode 100644 index 00000000..f1a3c26c --- /dev/null +++ b/tests/test_setup_project_typed_name.py @@ -0,0 +1,136 @@ +# SPDX-License-Identifier: Apache-2.0 +"""The project name typed at `setup-project.py`'s prompt (#729). + +A name that arrives as `--project` or as the positional is checked at parse +time, before step (1) prints, by `checked_value`. A name typed at the +`project name` prompt reached step (3) unchecked: `..` left `--into`, an +absolute name replaced it, `--help` printed the naming validator's help at +step (4) and exited 0, and the refusals that did come named an `--id` nobody +passed or dumped the usage text. These tests type each of those at a real +terminal and hold the run to the `--project` refusal, worded for what the +person did: the value is a `project name`. + +POSIX only, as every test that gives a run a terminal is (`PTY_SKIP`). +""" + +from __future__ import annotations + +import sys + +import pytest + +from conftest import PTY_SKIP, REPO, run_in_a_terminal +from test_setup_project_py import (SETUP_PROJECT, SETUP_SH, piped, run_entry, + terminal_env) + +PROMPT = "project name (ONE CamelCase token, e.g. Atlas): " + +#: What a terminal in canonical mode takes as "the next character is text": +#: ^V, which is how a control character is typed into a line. +LNEXT = chr(0x16) +BELL = chr(7) + +#: Names the check refuses, by case. `absolute` is a path under `tmp_path`, +#: so it is built by `typed_value`. +REFUSED = { + "up-and-out": "../x", + "slash": "a/b", + "dot-dot": "..", + "dot": ".", + "dash": "-x", + "help": "--help", + "control": "a" + BELL + "b", + "long": "x" * 256, + "blank": " ", + "absolute": None, +} + +#: Names step (4)'s policy rules on, as it did before: `NAME_RE` is +#: permissive on purpose. +LEFT_TO_STEP_FOUR = ("Bad_Name", "a b") + + +def typed_value(tmp_path, case): + value = REFUSED[case] + return str(tmp_path / "elsewhere" / "Name") if value is None else value + + +def at_the_prompt(tmp_path, typed, entry="py"): + """Setup, with no project, asked at a terminal and answered `typed` + (`"EOF"` is Ctrl-D). `entry` is the file run directly, that file with + its stdin a pipe (the terminal is then the controlling one), or + `setup.sh`. Returns the exit status and the screen.""" + flags = ["--org", "demoorg", "--local-remote-dir", + str(tmp_path / "remotes"), "--into", + str(tmp_path / "parent" / "into")] + argv = {"py": [sys.executable, str(SETUP_PROJECT), *flags], + "sh": ["bash", str(SETUP_SH), *flags], + "piped": piped("", *flags)}[entry] + if typed != "EOF": + typed = typed.replace(BELL, LNEXT + BELL) + "\n" + status, screen, answered = run_in_a_terminal( + argv, cwd=REPO, env=terminal_env(), answers=[typed], prompt=PROMPT, + timeout=60) + assert answered == 1, f"it never asked for a project name:\n{screen}" + assert "(3) project" in screen, screen + return status, screen + + +def the_refusal(text): + return text[text.index("REFUSED: "):].strip() + + +def assert_refused_at_the_prompt(tmp_path, value, entry="py"): + status, screen = at_the_prompt(tmp_path, value, entry) + assert status == 2, f"status {status}:\n{screen}" + assert f"project name is {ascii(value)}, which is not a value" in screen + assert "(4) naming policy" not in screen, screen + assert "usage: setup-project.py" not in screen, screen + assert "--id" not in screen, screen + assert list(tmp_path.iterdir()) == [], "the refused name left a trace" + return screen + + +@PTY_SKIP +@pytest.mark.parametrize("case", REFUSED) +def test_a_typed_project_name_is_refused_as_the_flag_refuses_it(tmp_path, + case): + value = typed_value(tmp_path, case) + screen = assert_refused_at_the_prompt(tmp_path, value) + flagged = run_entry("--project", value, "--org", "demoorg", + "--local-remote-dir", str(tmp_path / "remotes"), + "--into", str(tmp_path / "parent" / "into")) + assert flagged.returncode == 2, flagged.stderr + assert the_refusal(screen) == the_refusal(flagged.stderr).replace( + "REFUSED: --project is", "REFUSED: project name is", 1) + + +@PTY_SKIP +@pytest.mark.parametrize("entry", ["piped", "sh"]) +def test_a_typed_project_name_is_refused_on_every_way_in(tmp_path, entry): + """The same check on the one-liner's shape (stdin a pipe, the answer + read from the terminal) and through `setup.sh`, the shim.""" + assert_refused_at_the_prompt(tmp_path, "../x", entry) + + +@PTY_SKIP +@pytest.mark.parametrize("typed", ["", "EOF"]) +def test_an_empty_answer_is_refused_as_no_name_not_as_no_terminal(tmp_path, + typed): + status, screen = at_the_prompt(tmp_path, typed) + assert status == 2, f"status {status}:\n{screen}" + assert the_refusal(screen) == ( + "REFUSED: no project name was typed; nothing was created.") + assert "no terminal" not in screen, screen + assert "usage: setup-project.py" not in screen, screen + assert "(4) naming policy" not in screen, screen + assert list(tmp_path.iterdir()) == [] + + +@PTY_SKIP +@pytest.mark.parametrize("value", LEFT_TO_STEP_FOUR) +def test_a_name_the_policy_decides_is_left_to_step_four(tmp_path, value): + status, screen = at_the_prompt(tmp_path, value) + assert "(4) naming policy" in screen, screen + assert "project name is" not in screen, screen + assert status == 1, f"status {status}:\n{screen}" From b3c77cc2d22fea4d64fb7f4407a6fff6e74ee551 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 00:05:46 +0000 Subject: [PATCH 2/6] setup-project.py: the typed project name's prompt, empty-line refusal and check are one helper, and the tests pin where the check sits, so a check made after step (3) has asked the disk fails them (#729) Review round on 9f429f40 (rv729: approve with nits, two SHOULDs). SHOULD-1. SonarCloud reported python:S3776 on resolve_project_and_elector (cognitive complexity 16 of 15). The prompt, the empty-line refusal and the `checked_value("project name", typed)` call move into `ask_for_the_project_name()`, with their comments; the call site is `opts.project = ask_for_the_project_name()`. No behaviour changes. The function is back to 14 by rv729's estimator, and the helper is 1. SHOULD-2. The tests did not pin where the check sits: a check moved to just before `check_names`, or to after `resolve_family`, passed all 16 of them. Now every refused name is typed with a file standing where the folder above `--into` goes, so a check made after step (3) asks the disk would end in "is a file" and not "project name is"; and `[ok] project` and `[ok] legs` must not print. Only the seeded file may be on the disk afterwards, unchanged. Both moved checks now fail 12 tests each. Nits. The empty line and Ctrl-D are also typed through the one-liner's shape (stdin a pipe) and through `setup.sh`, 6 tests in place of 2. `(4) naming policy` is one constant. The module docstring no longer says `--help` "exited 0" of the run: the naming validator exited 0 at step (4), and the run ended at step (5) with exit 2. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- setup-project.py | 34 +++++++++------ tests/test_setup_project_typed_name.py | 59 +++++++++++++++++++------- 2 files changed, 64 insertions(+), 29 deletions(-) diff --git a/setup-project.py b/setup-project.py index b774914b..3ec1a195 100755 --- a/setup-project.py +++ b/setup-project.py @@ -1472,24 +1472,32 @@ def run_at_the_terminal(argv) -> subprocess.CompletedProcess: os.close(descriptor) +def ask_for_the_project_name() -> str: + """The `project name` prompt, and the name typed at it, checked (#729). + + AN EMPTY LINE IS A NAME NOBODY TYPED, NOT A MISSING TERMINAL: `can_ask()` + at the call is the gate that refuses for want of one, so this refusal says + what happened and does not print the usage text. + + THE TYPED NAME IS HELD TO THE CHECK THE SAME NAME GETS AS `--project` or + as the positional, before anything builds a path from it: `..` leaves + `--into`, an absolute name replaces it, and step (4)'s validator reads + only the last part of what it is handed. `a b` and `Bad_Name` pass here on + purpose (`NAME_RE` is permissive); step (4) is where the policy rules on + those. + """ + typed = ask(" project name (ONE CamelCase token, e.g. Atlas): ") + if not typed: + die("no project name was typed; nothing was created.") + return checked_value("project name", typed) + + def resolve_project_and_elector(opts: Options, shape_root: Path) -> None: """setup.sh:355-386, transcribed.""" say("") say("(3) project") if not opts.project and can_ask(): - typed = ask(" project name (ONE CamelCase token, e.g. Atlas): ") - # AN EMPTY LINE IS A NAME NOBODY TYPED, NOT A MISSING TERMINAL (#729): - # `can_ask()` above is the gate that refuses for want of one, so this - # refusal says what happened and does not print the usage text. - if not typed: - die("no project name was typed; nothing was created.") - # THE TYPED NAME IS HELD TO THE CHECK THE SAME NAME GETS AS `--project` - # or as the positional (#729), before anything builds a path from it: - # `..` leaves `--into`, an absolute name replaces it, and step (4)'s - # validator reads only the last part of what it is handed. `a b` and - # `Bad_Name` pass here on purpose (`NAME_RE` is permissive); step (4) - # is where the policy rules on those. - opts.project = checked_value("project name", typed) + opts.project = ask_for_the_project_name() if not opts.project: usage(sys.stderr) die("no --project given, and no terminal to ask on.") diff --git a/tests/test_setup_project_typed_name.py b/tests/test_setup_project_typed_name.py index f1a3c26c..d27eeb68 100644 --- a/tests/test_setup_project_typed_name.py +++ b/tests/test_setup_project_typed_name.py @@ -4,11 +4,17 @@ A name that arrives as `--project` or as the positional is checked at parse time, before step (1) prints, by `checked_value`. A name typed at the `project name` prompt reached step (3) unchecked: `..` left `--into`, an -absolute name replaced it, `--help` printed the naming validator's help at -step (4) and exited 0, and the refusals that did come named an `--id` nobody -passed or dumped the usage text. These tests type each of those at a real -terminal and hold the run to the `--project` refusal, worded for what the -person did: the value is a `project name`. +absolute name replaced it, `--help` made the naming validator print its help +and exit 0 at step (4) (the run itself ended at step (5), with the scaffold's +usage and exit 2), and the refusals that did come named an `--id` nobody passed +or dumped the usage text. These tests type each of those at a real terminal and +hold the run to the `--project` refusal, worded for what the person did: the +value is a `project name`. + +Two things pin WHERE the check sits, not only that it is made. Nothing of step +(3) may print (`[ok] project`, `[ok] legs`), and a FILE stands where the folder +above `--into` goes, so a check made after step (3) has asked the disk would +say "is a file" and not `project name is`. POSIX only, as every test that gives a run a terminal is (`PTY_SKIP`). """ @@ -25,6 +31,13 @@ PROMPT = "project name (ONE CamelCase token, e.g. Atlas): " +#: Step (4)'s heading, which no refused name may reach. +NAMING_POLICY = "(4) naming policy" + +#: A file where the folder above `--into` goes, so any step that asks the +#: disk about the clone path refuses with "is a file". +FILE_ABOVE_INTO = b"a file where the folder above --into goes\n" + #: What a terminal in canonical mode takes as "the next character is text": #: ^V, which is how a control character is typed into a line. LNEXT = chr(0x16) @@ -55,11 +68,14 @@ def typed_value(tmp_path, case): return str(tmp_path / "elsewhere" / "Name") if value is None else value -def at_the_prompt(tmp_path, typed, entry="py"): +def at_the_prompt(tmp_path, typed, entry="py", seeded=False): """Setup, with no project, asked at a terminal and answered `typed` (`"EOF"` is Ctrl-D). `entry` is the file run directly, that file with its stdin a pipe (the terminal is then the controlling one), or - `setup.sh`. Returns the exit status and the screen.""" + `setup.sh`. `seeded` puts `FILE_ABOVE_INTO` where a later step would + refuse it. Returns the exit status and the screen.""" + if seeded: + (tmp_path / "parent").write_bytes(FILE_ABOVE_INTO) flags = ["--org", "demoorg", "--local-remote-dir", str(tmp_path / "remotes"), "--into", str(tmp_path / "parent" / "into")] @@ -80,14 +96,23 @@ def the_refusal(text): return text[text.index("REFUSED: "):].strip() +def assert_nothing_was_made(tmp_path): + """Only the seeded file is on the disk, and it is as it was.""" + assert sorted(p.name for p in tmp_path.iterdir()) == ["parent"] + assert (tmp_path / "parent").read_bytes() == FILE_ABOVE_INTO + + def assert_refused_at_the_prompt(tmp_path, value, entry="py"): - status, screen = at_the_prompt(tmp_path, value, entry) + status, screen = at_the_prompt(tmp_path, value, entry, seeded=True) assert status == 2, f"status {status}:\n{screen}" assert f"project name is {ascii(value)}, which is not a value" in screen - assert "(4) naming policy" not in screen, screen + assert "is a file" not in screen, screen + assert "[ok] project" not in screen, screen + assert "[ok] legs" not in screen, screen + assert NAMING_POLICY not in screen, screen assert "usage: setup-project.py" not in screen, screen assert "--id" not in screen, screen - assert list(tmp_path.iterdir()) == [], "the refused name left a trace" + assert_nothing_was_made(tmp_path) return screen @@ -114,23 +139,25 @@ def test_a_typed_project_name_is_refused_on_every_way_in(tmp_path, entry): @PTY_SKIP +@pytest.mark.parametrize("entry", ["py", "piped", "sh"]) @pytest.mark.parametrize("typed", ["", "EOF"]) -def test_an_empty_answer_is_refused_as_no_name_not_as_no_terminal(tmp_path, - typed): - status, screen = at_the_prompt(tmp_path, typed) +def test_an_empty_answer_is_refused_as_no_name_not_as_no_terminal( + tmp_path, typed, entry): + status, screen = at_the_prompt(tmp_path, typed, entry, seeded=True) assert status == 2, f"status {status}:\n{screen}" assert the_refusal(screen) == ( "REFUSED: no project name was typed; nothing was created.") + assert "is a file" not in screen, screen assert "no terminal" not in screen, screen assert "usage: setup-project.py" not in screen, screen - assert "(4) naming policy" not in screen, screen - assert list(tmp_path.iterdir()) == [] + assert NAMING_POLICY not in screen, screen + assert_nothing_was_made(tmp_path) @PTY_SKIP @pytest.mark.parametrize("value", LEFT_TO_STEP_FOUR) def test_a_name_the_policy_decides_is_left_to_step_four(tmp_path, value): status, screen = at_the_prompt(tmp_path, value) - assert "(4) naming policy" in screen, screen + assert NAMING_POLICY in screen, screen assert "project name is" not in screen, screen assert status == 1, f"status {status}:\n{screen}" From 854f6ed95c890e3f47bab9e8ea4f032813f41a15 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 10:52:28 +0000 Subject: [PATCH 3/6] setup-project.py: a refused value names the check it failed, a project name over 95 characters is refused before step (4) where GitHub will hold the repositories, and the prompt takes the spaces off a typed name, so `../x` is no longer told about a leading `-`, a 100-character name no longer ends at step (5) or at GitHub, and `Atlas ` no longer hides in step (4) (#777) `refuse_value` gave every refusal the one closing sentence, and the sentence explained only a leading `-`: `../x`, `a/b`, ` Atlas`, `Atlas/` and a name with an accented letter were told about a `-` none of them has. It now takes the reason, and `why_refused` names the first check the value failed, in the order empty, leading `-` (which keeps its "git reads its own arguments" sentence), control character, then the pattern, said in words and as the pattern, as `repo_shape.checked_value` prints its own. `checked_ref` says `..` and `.lock` in their own sentences and `checked_passthrough` its control character. `repo_shape.checked_value` is not changed: it is a pinned copy, so a wording change there is a re-pin, and it already prints its pattern. A project name over 95 characters is refused before step (1) for `--project` and the positional, and before step (3) prints for the prompt, unless `--local-remote-dir` is given. GitHub keeps a repository name to at most 100 characters (its documentation: nothing here creates a real repository, so the number is not measured) and `-spec` and `-code` add five. Under `--local-remote-dir` the limit is the disk's and step (5) still asks it as `scaffold-name-too-long`. The check runs at the end of the parse, because `--local-remote-dir` can come after `--project` on the line. The prompt takes the spaces off the ends of what was typed. `Atlas ` passed `NAME_RE` where ` Atlas` was refused, and step (4) judged `Atlas -spec` with no space shown anywhere. A line of spaces is now an empty line. An argv value is deliberate and is held as typed. tests/test_setup_project_name_edges.py is new. #756's file moves its three-spaces case from the refusals to the empty answers, and a comment in tests/test_setup_project_py.py stops saying the two entry points refuse in the same words. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- setup-project.py | 138 +++++++-- tests/test_setup_project_name_edges.py | 395 +++++++++++++++++++++++++ tests/test_setup_project_py.py | 10 +- tests/test_setup_project_typed_name.py | 7 +- 4 files changed, 525 insertions(+), 25 deletions(-) create mode 100644 tests/test_setup_project_name_edges.py diff --git a/setup-project.py b/setup-project.py index 3ec1a195..dc95a73c 100755 --- a/setup-project.py +++ b/setup-project.py @@ -349,8 +349,53 @@ def program_version(argv, field: int) -> str: CONTROL_RE = re.compile(r"[\x00-\x1f\x7f]") -def refuse_value(what: str, text: str) -> None: - """ONE wording for every value this tool will not pass on. +#: A GITHUB REPOSITORY NAME IS AT MOST 100 CHARACTERS, and a project is three +#: repositories: ``, `-spec` and `-code`. The legs +#: add five characters (`-spec` and `-code` are as long as each other), so a +#: project name over 95 names repositories GitHub cannot create (#777). The +#: 100 is GitHub's own documented limit. No run of this repository has +#: measured it: none creates a real repository, and a probe against a real +#: organisation is another lane's. Asked only where a GitHub repository will +#: be made: under `--local-remote-dir` the limit is the disk's, which step (5) +#: asks as `scaffold-name-too-long` (#261). +GITHUB_NAME_MAX = 100 +PROJECT_NAME_MAX = GITHUB_NAME_MAX - len("-spec") + + +#: WHY A VALUE IS REFUSED, one sentence each, said after the fixed first +#: sentence of `refuse_value` (#777). The old refusal gave every value the same +#: closing sentence, which explained only a leading `-`: `../x`, `a/b` and an +#: accented name were told about a `-` none of them has. +WHY_EMPTY = "It is empty, and a value here needs at least one character." +WHY_DASH = ("It starts with `-`, and git reads its own arguments: a value " + "that starts with `-` is an option to git, not a name to us.") +WHY_CONTROL = ("It holds a control character, which the quotation shows as " + "an escape: a newline ends a line in every terminal and log " + "this output reaches, so a value carrying one could print a " + "line that reads as this tool's own.") +WHY_RANGE = ("It holds `..`, which git reads as a revision range and not as " + "a ref.") +WHY_LOCK = ("It ends in `.lock`, the name git gives the lock file it writes " + "beside a ref.") + +#: What each pattern asks, in words and then as the pattern itself, as the +#: sibling `repo_shape.checked_value` prints its own. `PATH_RE` has no row: it +#: refuses only an empty value and a control character, and both are named +#: before a pattern is asked; a pattern with no row is still named, as the +#: sibling names it, by `why_refused`. +WHY_PATTERN = { + NAME_RE: ("It is not spellable as a name: a letter or a digit first, " + "then letters, digits, spaces and `._'()&,+@-`, at most 255 " + "characters in all (`%s`)." % NAME_RE.pattern), + REF_RE: ("It is not spellable as a ref: a letter or a digit first, then " + "letters, digits and `._/-`, at most 255 characters in all " + "(`%s`)." % REF_RE.pattern), +} + + +def refuse_value(what: str, text: str, why: str) -> None: + """ONE first sentence for every value this tool will not pass on, and the + reason it failed as the second (#777). `ascii()`, never `%r`: `repr` leaves a printable non-ASCII character as itself, and this file's output is ASCII to the byte (see the module @@ -359,9 +404,26 @@ def refuse_value(what: str, text: str) -> None: PURE ASCII rule exists to prevent. """ die("%s is %s, which is not a value this tool will put on a `git` or `gh` " - "command line. git reads its own arguments: a value that starts with " - "`-` is an option to git, not a name to us; the rest must be spellable " - "as a name, a ref or a path." % (what, ascii(text))) + "command line. %s" % (what, ascii(text), why)) + + +def why_refused(text: str, pattern) -> str: + """The reason `checked_value` refuses `text`, or "" when it does not. + + Asked in the order a person would find them: nothing typed, a leading `-`, + a control character, and only then the pattern, so the sentence names the + first check the value failed and never one it passed. + """ + if not text: + return WHY_EMPTY + if text.startswith("-"): + return WHY_DASH + if CONTROL_RE.search(text): + return WHY_CONTROL + if not pattern.fullmatch(text): + return WHY_PATTERN.get(pattern) or ( + "It does not match `%s`." % pattern.pattern) + return "" def checked_value(flag: str, value: str, pattern=NAME_RE) -> str: @@ -376,18 +438,39 @@ def checked_value(flag: str, value: str, pattern=NAME_RE) -> str: write). THE HOUSE PATTERN, COPIED RATHER THAN IMPORTED. `scripts/repo_shape.py`'s - `checked_value` is the same check in the same words for the same reason, - and `scaffold-project.py` calls it on the values this file forwards. This + `checked_value` is the same check for the same reason, and + `scaffold-project.py` calls it on the values this file forwards. This entry point cannot import it: it runs BEFORE there is a checkout to import - it from, which is the whole of self-bootstrap mode. + it from, which is the whole of self-bootstrap mode. The two refuse the + same values; each says why in its own words, and this file's name the + check the value failed (`why_refused`), where the sibling prints one + remediation for all of them. """ text = str(value) - if (not text or text.startswith("-") or CONTROL_RE.search(text) - or not pattern.fullmatch(text)): - refuse_value(flag, text) + why = why_refused(text, pattern) + if why: + refuse_value(flag, text, why) return text +def checked_project_length(what: str, name: str, local_mode: bool) -> str: + """A project name that GitHub will hold three repositories under, or a + refusal before step (4) explains three names it will not use (#777). + + ASKED OF A NAME THAT HAS PASSED `checked_value`, so it is printable ASCII + and is quoted back as it is, both ends of it: the name is long, and a + name this long differs from its neighbours by its tail. + """ + if local_mode or len(name) <= PROJECT_NAME_MAX: + return name + die("%s is %d characters long (%s...%s), and a project is three " + "repositories, ``, `-spec` and `-code`: " + "GitHub keeps a repository name to at most %d characters, so a " + "project name is at most %d. Nothing was created. Use a shorter name." + % (what, len(name), name[:12], name[-12:], GITHUB_NAME_MAX, + PROJECT_NAME_MAX)) + + def checked_ref(flag: str, value: str) -> str: """`--shape-ref`: narrower than a name, because git reads more into one. @@ -397,8 +480,10 @@ def checked_ref(flag: str, value: str) -> str: confusing failure inside the temporary checkout. """ text = checked_value(flag, value, REF_RE) - if ".." in text or text.endswith(".lock"): - refuse_value(flag, text) + if ".." in text: + refuse_value(flag, text, WHY_RANGE) + if text.endswith(".lock"): + refuse_value(flag, text, WHY_LOCK) return text @@ -417,7 +502,8 @@ def checked_passthrough(rest: list) -> list: """ for item in rest: if CONTROL_RE.search(item): - refuse_value("a passthrough argument after `--`", item) + refuse_value("a passthrough argument after `--`", item, + WHY_CONTROL) return rest @@ -580,6 +666,12 @@ def parse_args(argv, invocation_dir: str) -> Options: if opts.visibility not in VISIBILITY_CHOICES: die("--visibility is %s; it must be 'private', 'public' or 'internal'" % opts.visibility) + # ASKED HERE, NOT WHERE `--project` IS TAKEN: `--local-remote-dir` can + # come after it on the line, and decides whether a GitHub repository + # will be made (#777). Still before step (1) prints. + if opts.project: + spelling = "" if state.positional else "--project" + checked_project_length(spelling, opts.project, opts.local_mode) return opts @@ -1472,7 +1564,7 @@ def run_at_the_terminal(argv) -> subprocess.CompletedProcess: os.close(descriptor) -def ask_for_the_project_name() -> str: +def ask_for_the_project_name(local_mode: bool) -> str: """The `project name` prompt, and the name typed at it, checked (#729). AN EMPTY LINE IS A NAME NOBODY TYPED, NOT A MISSING TERMINAL: `can_ask()` @@ -1485,11 +1577,21 @@ def ask_for_the_project_name() -> str: only the last part of what it is handed. `a b` and `Bad_Name` pass here on purpose (`NAME_RE` is permissive); step (4) is where the policy rules on those. + + THE SPACES AT THE ENDS ARE TAKEN OFF FIRST (#777). A trailing space + passed `NAME_RE` (a space is allowed after the first character, so `a b` + can reach step (4)) where a leading one was refused, and step (4) then + judged `Atlas `, `Atlas -spec` and `Atlas -code` and showed no space + anywhere. A stray space is what a prompt answer gets from a paste or a + slip of the thumb, and the run ends at the first mistake with no second + ask, so the prompt takes it off; an argv value is deliberate and is held + as typed. A line of nothing but spaces is then an empty line. """ - typed = ask(" project name (ONE CamelCase token, e.g. Atlas): ") + typed = ask(" project name (ONE CamelCase token, e.g. Atlas): ").strip() if not typed: die("no project name was typed; nothing was created.") - return checked_value("project name", typed) + return checked_project_length( + "project name", checked_value("project name", typed), local_mode) def resolve_project_and_elector(opts: Options, shape_root: Path) -> None: @@ -1497,7 +1599,7 @@ def resolve_project_and_elector(opts: Options, shape_root: Path) -> None: say("") say("(3) project") if not opts.project and can_ask(): - opts.project = ask_for_the_project_name() + opts.project = ask_for_the_project_name(opts.local_mode) if not opts.project: usage(sys.stderr) die("no --project given, and no terminal to ask on.") diff --git a/tests/test_setup_project_name_edges.py b/tests/test_setup_project_name_edges.py new file mode 100644 index 00000000..d7f53a4d --- /dev/null +++ b/tests/test_setup_project_name_edges.py @@ -0,0 +1,395 @@ +# SPDX-License-Identifier: Apache-2.0 +"""The edges of a project name typed at `setup-project.py` (#777). + +#729 made the name typed at the prompt answer to the check `--project` gets. +Three things were left, each measured on #729's head: + +* `refuse_value` gave every refusal one closing sentence, and the sentence + explained only a leading `-`, so `../x`, `a/b`, ` Atlas` and a name with an + accented letter were told about a `-` none of them has. Each refusal now + names the check the value failed: empty, a leading `-`, a control character, + or the pattern for a name or a ref, in words and as the pattern. +* A name of 96 to 255 characters passed the prompt and `--project` and ended at + step (5) or at GitHub, after step (4) printed its wall for three names that + could never be repositories. GitHub keeps a repository name to at most 100 + characters (its documentation: nothing here creates a real repository, so the + number is not measured), and `-spec` and `-code` add five, so a project name + is at most 95, asked before step (4) and only where a GitHub repository will + be made. Under `--local-remote-dir` the limit is the disk's, and step (5) + still asks it as `scaffold-name-too-long`. +* `Atlas ` passed `NAME_RE` where ` Atlas` was refused, and step (4) judged + `Atlas -spec` with no space shown anywhere. The prompt now takes the spaces + off the ends of what was typed; an argv value is deliberate and is held as + typed. + +POSIX only where a run needs a terminal or a `gh` stub (`PTY_SKIP`). +""" + +from __future__ import annotations + +import os +import re +import string +import sys + +import pytest + +from conftest import PTY_SKIP, REPO, run_in_a_terminal +from test_setup_project_py import (SETUP_PROJECT, entry_point_module, + run_entry, terminal_env) +from test_setup_project_typed_name import (BELL, LNEXT, NAMING_POLICY, PROMPT, + the_refusal) + +#: The first sentence every refusal says, then its reason. +FIRST = ("{what} is {shown}, which is not a value this tool will put on a " + "`git` or `gh` command line. ") + +EMPTY = "It is empty, and a value here needs at least one character." +DASH = ("It starts with `-`, and git reads its own arguments: a value that " + "starts with `-` is an option to git, not a name to us.") +CONTROL = ("It holds a control character, which the quotation shows as an " + "escape: a newline ends a line in every terminal and log this " + "output reaches, so a value carrying one could print a line that " + "reads as this tool's own.") +NAME = ("It is not spellable as a name: a letter or a digit first, then " + "letters, digits, spaces and `._'()&,+@-`, at most 255 characters in " + "all (`^[A-Za-z0-9][A-Za-z0-9 ._'()&,+@-]{0,254}$`).") +REF = ("It is not spellable as a ref: a letter or a digit first, then " + "letters, digits and `._/-`, at most 255 characters in all " + "(`^[A-Za-z0-9][A-Za-z0-9._/-]{0,254}$`).") +RANGE = ("It holds `..`, which git reads as a revision range and not as a " + "ref.") +LOCK = ("It ends in `.lock`, the name git gives the lock file it writes " + "beside a ref.") + +#: What a sentence for one check must never say about another. +NOT_THE_DASH = ("starts with", "option to git", "reads its own arguments") + +ACCENTED = "Caf\u00e9" + +#: Each refusal: the argv that carries the value (`{}` is the value), the +#: value, how the refusal names it, and the reason it gives. A value that +#: fails two checks is told the first, in the order empty, `-`, control +#: character, pattern. +REFUSALS = { + "org-empty": (("--org", "{}"), "", "--org", EMPTY), + "into-empty": (("--into", "{}"), "", "--into", EMPTY), + "org-dash": (("--org", "{}"), "-evil", "--org", DASH), + "into-dash": (("--into", "{}"), "-x", "--into", DASH), + "org-control": (("--org", "{}"), "a" + BELL + "b", "--org", CONTROL), + "into-control": (("--into", "{}"), "a" + BELL + "b", "--into", CONTROL), + "passthrough-control": (("--", "{}"), "a" + BELL + "b", + "a passthrough argument after `--`", CONTROL), + "project-up-and-out": (("--project", "{}"), "../x", "--project", NAME), + "project-slash": (("--project", "{}"), "a/b", "--project", NAME), + "project-dot-dot": (("--project", "{}"), "..", "--project", NAME), + "project-leading-space": (("--project", "{}"), " Atlas", "--project", + NAME), + "project-trailing-slash": (("--project", "{}"), "Atlas/", "--project", + NAME), + "project-accented": (("--project", "{}"), ACCENTED, "--project", NAME), + "project-blank": (("--project", "{}"), " ", "--project", NAME), + "org-dash-and-control": (("--org", "{}"), "-" + BELL, "--org", DASH), + "org-control-and-slash": (("--org", "{}"), "a" + BELL + "/b", "--org", + CONTROL), + "org-long": (("--org", "{}"), "x" * 256, "--org", NAME), + "positional": (("{}",), "../x", "", NAME), + "ref-space": (("--shape-ref", "{}"), "a b", "--shape-ref", REF), + "ref-long": (("--shape-ref", "{}"), "a" * 256, "--shape-ref", REF), + "ref-dash": (("--shape-ref", "{}"), "-x", "--shape-ref", DASH), + "ref-range": (("--shape-ref", "{}"), "a..b", "--shape-ref", RANGE), + "ref-lock": (("--shape-ref", "{}"), "topic.lock", "--shape-ref", LOCK), +} + + +def a_run_carrying(tmp_path, template, value): + """A run that would otherwise be valid, with `value` where `template` + puts it. A bare `` is the one carrier that cannot also be + given `--project`: two names are refused before either is checked.""" + carried = [part.replace("{}", value) for part in template] + context = ["--org", "demoorg", "--into", str(tmp_path / "work"), + "--local-remote-dir", str(tmp_path / "remotes")] + if template[0] != "{}": + context = ["--project", "Sample", *context] + return run_entry(*context, *carried) + + +@pytest.mark.parametrize("case", REFUSALS) +def test_a_refusal_names_the_check_the_value_failed(tmp_path, case): + template, value, what, why = REFUSALS[case] + result = a_run_carrying(tmp_path, template, value) + assert result.returncode == 2, result.stderr + expected = "REFUSED: " + FIRST.format(what=what, shown=ascii(value)) + why + assert the_refusal(result.stderr) == expected + if why != DASH: + for words in NOT_THE_DASH: + assert words not in result.stderr, result.stderr + assert not (tmp_path / "remotes").exists() + + +#: What typed at the prompt is refused, and why. An empty line is not here: +#: it is "no project name was typed", which #729's file pins. +TYPED = { + "dash": ("-x", DASH), + "control": ("a" + BELL + "b", CONTROL), + "up-and-out": ("../x", NAME), + "slash": ("a/b", NAME), + "trailing-slash": ("Atlas/", NAME), + "accented": (ACCENTED, NAME), + "long": ("x" * 256, NAME), +} + + +def gh_that_is_signed_in(tmp_path): + """The environment of a run that is allowed to call `gh` and gets the + three answers a preflight and step (3) read, and refuses the rest, so no + run here reaches a real GitHub (the real `gh` stays unreachable).""" + stub = tmp_path / "ghbin" + stub.mkdir(exist_ok=True) + gh = stub / "gh" + gh.write_bytes(b"#!/bin/sh\n" + b'case "$1" in\n' + b' --version) echo "gh version 2.50.0 (stub)" ;;\n' + b" auth) exit 0 ;;\n" + b" api) echo tester ;;\n" + b' *) echo "stub gh: refused $*" >&2; exit 97 ;;\n' + b"esac\n") + gh.chmod(0o755) + return {"PATH": str(stub) + os.pathsep + os.environ["PATH"]} + + +def flags_for(tmp_path, local): + """The flags of a run asked for its project: `--` and `--dry-run` last, + so a name that goes on ends at the plan and creates nothing.""" + return ["--org", "demoorg", "--elected-by", "Tester", "--into", + str(tmp_path / "parent" / "into"), + *(["--local-remote-dir", str(tmp_path / "remotes")] + if local else []), "--", "--dry-run"] + + +def at_the_prompt(tmp_path, typed, local=True): + """Setup asked for its project at a terminal and answered `typed`.""" + env = terminal_env() + env.update(gh_that_is_signed_in(tmp_path)) + status, screen, answered = run_in_a_terminal( + [sys.executable, str(SETUP_PROJECT), *flags_for(tmp_path, local)], + cwd=REPO, env=env, + answers=[typed.replace(BELL, LNEXT + BELL) + "\n"], prompt=PROMPT, + timeout=60) + assert answered == 1, f"it never asked for a project name:\n{screen}" + return status, screen + + +def assert_nothing_was_made(tmp_path): + for name in ("remotes", "parent"): + assert not (tmp_path / name).exists(), name + + +@PTY_SKIP +@pytest.mark.parametrize("case", TYPED) +def test_a_typed_name_is_refused_with_the_check_it_failed(tmp_path, case): + value, why = TYPED[case] + status, screen = at_the_prompt(tmp_path, value) + assert status == 2, f"status {status}:\n{screen}" + assert the_refusal(screen) == ( + "REFUSED: " + FIRST.format(what="project name", shown=ascii(value)) + + why) + if why != DASH: + for words in NOT_THE_DASH: + assert words not in screen, screen + assert "[ok] project" not in screen, screen + assert NAMING_POLICY not in screen, screen + assert_nothing_was_made(tmp_path) + + +# --- the bound --------------------------------------------------------------- + +#: GitHub's documented limit on a repository name, less `-spec` and `-code`. +BOUND = 95 + + +def a_name(length): + return "A" + "b" * (length - 1) + + +def too_long(what, name): + return ("REFUSED: %s is %d characters long (%s...%s), and a project is " + "three repositories, ``, `-spec` and " + "`-code`: GitHub keeps a repository name to at most " + "100 characters, so a project name is at most 95. Nothing was " + "created. Use a shorter name." + % (what, len(name), name[:12], name[-12:])) + + +def a_run_for(tmp_path, project, local, positional=False, late_local=False): + """The project given on the command line, and `-- --dry-run`.""" + flags = flags_for(tmp_path, local) + if late_local: + flags = [f for f in flags_for(tmp_path, False)] + flags[flags.index("--"):flags.index("--")] = [ + "--local-remote-dir", str(tmp_path / "remotes")] + given = [project] if positional else ["--project", project] + return run_entry(*given, *flags, extra_env=gh_that_is_signed_in(tmp_path)) + + +@PTY_SKIP +@pytest.mark.parametrize("how", ["flag", "positional"]) +@pytest.mark.parametrize("length", [BOUND + 1, 100, 101, 255]) +def test_a_name_over_the_bound_is_refused_before_anything_prints( + tmp_path, length, how): + name = a_name(length) + result = a_run_for(tmp_path, name, local=False, + positional=how == "positional") + assert result.returncode == 2, result.stderr + assert the_refusal(result.stderr) == too_long( + "" if how == "positional" else "--project", name) + assert result.stdout == "", result.stdout + assert_nothing_was_made(tmp_path) + + +@PTY_SKIP +def test_a_name_at_the_bound_goes_on_to_step_four(tmp_path): + result = a_run_for(tmp_path, a_name(BOUND), local=False) + assert NAMING_POLICY in result.stdout, result.stdout + result.stderr + assert "characters long" not in result.stderr, result.stderr + + +@PTY_SKIP +@pytest.mark.parametrize("length", [BOUND + 1, 100, 101, 246]) +def test_a_name_over_the_bound_is_not_asked_of_a_local_run(tmp_path, length): + """No GitHub repository is made under `--local-remote-dir`, so the limit + is the disk's, and step (5) still asks it: 247 characters and more end + there as `scaffold-name-too-long`.""" + result = a_run_for(tmp_path, a_name(length), local=True) + assert NAMING_POLICY in result.stdout, result.stdout + result.stderr + assert "characters long" not in result.stderr, result.stderr + assert result.returncode == 0, result.stderr + + +@PTY_SKIP +def test_the_disk_limit_is_still_step_fives_under_a_local_run(tmp_path): + result = a_run_for(tmp_path, a_name(247), local=True) + assert result.returncode == 2, result.stderr + assert "scaffold-name-too-long" in result.stderr, result.stderr + assert "characters long" not in result.stderr, result.stderr + + +@PTY_SKIP +def test_the_bound_waits_for_a_local_remote_dir_that_comes_later(tmp_path): + """`--local-remote-dir` decides whether a GitHub repository will be made, + and it can come after `--project` on the line.""" + result = a_run_for(tmp_path, a_name(BOUND + 1), local=False, + late_local=True) + assert NAMING_POLICY in result.stdout, result.stdout + result.stderr + assert "characters long" not in result.stderr, result.stderr + + +@PTY_SKIP +@pytest.mark.parametrize("length", [BOUND + 1, 100, 101, 255]) +def test_a_typed_name_over_the_bound_is_refused_at_the_prompt(tmp_path, + length): + name = a_name(length) + status, screen = at_the_prompt(tmp_path, name, local=False) + assert status == 2, f"status {status}:\n{screen}" + assert the_refusal(screen) == too_long("project name", name) + assert "[ok] project" not in screen, screen + assert NAMING_POLICY not in screen, screen + assert_nothing_was_made(tmp_path) + + +@PTY_SKIP +@pytest.mark.parametrize("local", [False, True], ids=["github", "local"]) +def test_a_typed_name_at_the_bound_goes_on_to_step_four(tmp_path, local): + status, screen = at_the_prompt(tmp_path, a_name(BOUND), local=local) + assert "[ok] project" in screen, screen + assert NAMING_POLICY in screen, screen + assert "characters long" not in screen, screen + + +@PTY_SKIP +def test_a_typed_name_over_the_bound_goes_on_in_a_local_run(tmp_path): + status, screen = at_the_prompt(tmp_path, a_name(BOUND + 1), local=True) + assert status == 0, f"status {status}:\n{screen}" + assert NAMING_POLICY in screen, screen + assert "characters long" not in screen, screen + + +# --- the spaces at the ends -------------------------------------------------- + +@PTY_SKIP +@pytest.mark.parametrize("typed", ["Atlas ", " Atlas", " Atlas "]) +def test_the_spaces_at_the_ends_of_a_typed_name_are_taken_off(tmp_path, typed): + status, screen = at_the_prompt(tmp_path, typed) + assert status == 0, f"status {status}:\n{screen}" + assert "\n [ok] project Atlas\n" in screen, screen + assert "\n [ok] legs Atlas-spec, Atlas-code\n" in screen, screen + assert "NO FAMILY" not in screen, screen + assert "naming-unclassified" not in screen, screen + assert f"clone into {tmp_path / 'parent' / 'into' / 'Atlas'}\n" in screen + + +@PTY_SKIP +def test_a_line_of_spaces_is_no_name_at_all(tmp_path): + status, screen = at_the_prompt(tmp_path, " ") + assert status == 2, f"status {status}:\n{screen}" + assert the_refusal(screen) == ( + "REFUSED: no project name was typed; nothing was created.") + assert "[ok] project" not in screen, screen + assert_nothing_was_made(tmp_path) + + +def test_an_argv_name_is_held_as_typed(tmp_path): + """The other half of the choice: an argv value is deliberate, so a + trailing space on `--project` reaches step (4) as it was given.""" + result = run_entry("--project", "Atlas ", "--org", "demoorg", + "--elected-by", "Tester", "--into", + str(tmp_path / "work"), "--local-remote-dir", + str(tmp_path / "remotes")) + assert result.returncode == 1, result.stderr + assert "FINDING naming-unclassified: Atlas , Atlas -spec, Atlas -code" \ + in result.stderr, result.stderr + + +# --- the sentences say what the checks are ------------------------------------- + +def test_the_name_sentence_says_what_the_pattern_holds(): + """The words and the pattern cannot drift: every printable character is + in the sentence's list exactly when `NAME_RE` takes it after a letter.""" + module = entry_point_module() + said = set(string.ascii_letters + string.digits + " ._'()&,+@-") + for char in string.printable: + if char in "\r\n\x0b\x0c": + continue + assert bool(module.NAME_RE.fullmatch("A" + char)) == (char in said), \ + repr(char) + assert module.NAME_RE.fullmatch("A" * 255) + assert not module.NAME_RE.fullmatch("A" * 256) + for char in string.digits + string.ascii_letters: + assert module.NAME_RE.fullmatch(char + "x") + assert not module.NAME_RE.fullmatch(" x") + + +def test_the_ref_sentence_says_what_the_pattern_holds(): + module = entry_point_module() + said = set(string.ascii_letters + string.digits + "._/-") + for char in string.printable: + if char in "\r\n\x0b\x0c": + continue + assert bool(module.REF_RE.fullmatch("a" + char)) == (char in said), \ + repr(char) + assert module.REF_RE.fullmatch("a" * 255) + assert not module.REF_RE.fullmatch("a" * 256) + + +def test_a_pattern_with_no_words_is_named_as_the_sibling_names_it(): + """`PATH_RE` has no row in `WHY_PATTERN`: its only failures are an empty + value and a control character, named first. A pattern that did get here + is still named, not a KeyError.""" + module = entry_point_module() + for text in ("", "a" + BELL + "b", "\n"): + assert not module.PATH_RE.fullmatch(text) + assert module.why_refused(text, module.PATH_RE) in ( + EMPTY, CONTROL), repr(text) + with pytest.raises(module.Refusal) as refused: + module.checked_value("--x", "ab", re.compile("^z$")) + assert str(refused.value).endswith(". It does not match `^z$`.") diff --git a/tests/test_setup_project_py.py b/tests/test_setup_project_py.py index 535aa995..dcb8c822 100644 --- a/tests/test_setup_project_py.py +++ b/tests/test_setup_project_py.py @@ -784,10 +784,12 @@ def test_a_flag_with_no_value_refuses(): # ARGUMENT injection, not shell injection: every command this entry point runs # is a list with `shell=False`, so there is no shell - but git reads its own # arguments, and a value that starts with `-` is an option to git rather than -# a name. `scaffold-project.py` refuses the same shape in the same words -# (`tests/test_scaffold_pin_and_reuse.py`), through `repo_shape.checked_value`; -# this file's subject carries its own copy because it runs BEFORE there is a -# checkout to import that from, which is the whole of self-bootstrap mode. +# a name. `scaffold-project.py` refuses the same values +# (`tests/test_scaffold_pin_and_reuse.py`), through `repo_shape.checked_value`, +# each in a sentence of its own: this file's subject names the check a value +# failed (`tests/test_setup_project_name_edges.py`). It carries its own copy +# because it runs BEFORE there is a checkout to import that from, which is the +# whole of self-bootstrap mode. # # Every run here passes `--local-remote-dir` even though none of them reaches # it: the refusal is a parse-time one, and the flag is what keeps diff --git a/tests/test_setup_project_typed_name.py b/tests/test_setup_project_typed_name.py index d27eeb68..cac6b3b8 100644 --- a/tests/test_setup_project_typed_name.py +++ b/tests/test_setup_project_typed_name.py @@ -44,7 +44,9 @@ BELL = chr(7) #: Names the check refuses, by case. `absolute` is a path under `tmp_path`, -#: so it is built by `typed_value`. +#: so it is built by `typed_value`. A line of spaces is not here: the prompt +#: takes the spaces off the ends of what was typed (#777), so it is an empty +#: line, and `--project ' '` is refused by the pattern. REFUSED = { "up-and-out": "../x", "slash": "a/b", @@ -54,7 +56,6 @@ "help": "--help", "control": "a" + BELL + "b", "long": "x" * 256, - "blank": " ", "absolute": None, } @@ -140,7 +141,7 @@ def test_a_typed_project_name_is_refused_on_every_way_in(tmp_path, entry): @PTY_SKIP @pytest.mark.parametrize("entry", ["py", "piped", "sh"]) -@pytest.mark.parametrize("typed", ["", "EOF"]) +@pytest.mark.parametrize("typed", ["", "EOF", " "]) def test_an_empty_answer_is_refused_as_no_name_not_as_no_terminal( tmp_path, typed, entry): status, screen = at_the_prompt(tmp_path, typed, entry, seeded=True) From b5def3f0562f0fb18820134151f7e972a6bdbf1f Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 16:12:40 +0000 Subject: [PATCH 4/6] tests/test_setup_project_name_edges.py: the typed runs give git a credential helper for github.com, and one run empties git's own config, so a GitHub run no longer waits on CI at a `gh auth setup-git` offer nothing answers (#777) On CI (run 39027235147, git 2.55.0) five of this file's tests were killed after 60 seconds: the four over-the-bound names typed at the prompt (96, 100, 101, 255) and the 95-character name typed in a GitHub run. Each screen ended in step (1): [??] git has no credential helper for github.com, so the first push over HTTPS will fail. Fix it once: gh auth setup-git Run `gh auth setup-git` now? Type yes to continue: A GitHub run (no `--local-remote-dir`) makes that offer in its preflight when git has no credential helper for github.com, before step (3) asks for the project. The tests type only at the project prompt, so nothing answered the offer, and the run waited. The bound played no part: the same names in local runs passed, and the 95-character GitHub run hung too. On the writer's machine /etc/gitconfig and ~/.gitconfig both name a helper, so the offer never came there. CI's runner names none. With git's system file skipped and its global file empty, the same five hang on that machine with CI's screen. The tool is right: the offer is `_check_credential_helper`'s documented warning and offer. The tests depended on the machine's git config. - `gh_that_is_signed_in` also gives git a credential helper for github.com, in git's command scope (`GIT_CONFIG_COUNT`, appended to what the environment holds): `! auth git-credential`, the line `gh auth setup-git` writes, pointed at the stub. The preflight finds a helper on every machine. No run here pushes, so git never asks it. - A new test, `test_a_github_run_here_is_not_held_by_a_git_with_no_helper` (95 and 96 characters), runs with git's system and global files empty, as CI's runner has them. A run that waits at the offer now fails on every machine, not on CI alone. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- tests/test_setup_project_name_edges.py | 38 ++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/tests/test_setup_project_name_edges.py b/tests/test_setup_project_name_edges.py index d7f53a4d..a2e2282c 100644 --- a/tests/test_setup_project_name_edges.py +++ b/tests/test_setup_project_name_edges.py @@ -29,6 +29,7 @@ import os import re +import shlex import string import sys @@ -143,7 +144,16 @@ def test_a_refusal_names_the_check_the_value_failed(tmp_path, case): def gh_that_is_signed_in(tmp_path): """The environment of a run that is allowed to call `gh` and gets the three answers a preflight and step (3) read, and refuses the rest, so no - run here reaches a real GitHub (the real `gh` stays unreachable).""" + run here reaches a real GitHub (the real `gh` stays unreachable). + + Its git has a credential helper for github.com: the line `gh auth + setup-git` writes, pointed at this stub, in git's command scope + (`GIT_CONFIG_COUNT`, appended to what the environment holds). Without + it, the machine's own git config decided whether a GitHub run's + preflight offered `gh auth setup-git`. A machine with a helper anywhere + was never offered it; CI's runner has none, and its runs waited at that + offer's `Type yes to continue:`, which nothing here answers, until they + were killed (#777's first CI run).""" stub = tmp_path / "ghbin" stub.mkdir(exist_ok=True) gh = stub / "gh" @@ -155,7 +165,12 @@ def gh_that_is_signed_in(tmp_path): b' *) echo "stub gh: refused $*" >&2; exit 97 ;;\n' b"esac\n") gh.chmod(0o755) - return {"PATH": str(stub) + os.pathsep + os.environ["PATH"]} + held = int(os.environ.get("GIT_CONFIG_COUNT") or 0) + return {"PATH": str(stub) + os.pathsep + os.environ["PATH"], + "GIT_CONFIG_COUNT": str(held + 1), + f"GIT_CONFIG_KEY_{held}": "credential.https://github.com.helper", + f"GIT_CONFIG_VALUE_{held}": + "!%s auth git-credential" % shlex.quote(str(gh))} def flags_for(tmp_path, local): @@ -306,6 +321,25 @@ def test_a_typed_name_at_the_bound_goes_on_to_step_four(tmp_path, local): assert "characters long" not in screen, screen +@PTY_SKIP +@pytest.mark.parametrize("length", [BOUND, BOUND + 1]) +def test_a_github_run_here_is_not_held_by_a_git_with_no_helper( + tmp_path, monkeypatch, length): + """A GitHub run's preflight offers `gh auth setup-git` to a machine whose + git has no credential helper for github.com, before step (3) asks for the + project. CI's runner is such a machine, and the typed runs above waited + there on it. Here git's system and global files are empty, as the + runner's are, so a run that would wait at that offer fails on every + machine, not on CI alone.""" + blank = tmp_path / "blank.gitconfig" + blank.write_bytes(b"") + for scope in ("GIT_CONFIG_SYSTEM", "GIT_CONFIG_GLOBAL"): + monkeypatch.setenv(scope, str(blank)) + _, screen = at_the_prompt(tmp_path, a_name(length), local=False) + assert "gh auth setup-git" not in screen, screen + assert ("characters long" in screen) == (length > BOUND), screen + + @PTY_SKIP def test_a_typed_name_over_the_bound_goes_on_in_a_local_run(tmp_path): status, screen = at_the_prompt(tmp_path, a_name(BOUND + 1), local=True) From 050abea6cc7e4fa25b8e3d83dc3b614b9b1bd730 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 16:28:58 +0000 Subject: [PATCH 5/6] setup-project.py: the prompt takes only the space off a typed name, and the name and ref sentences say ASCII, so a typed tab, form feed or no-break space is refused as `--project` refuses it, and `Caf\xe9` is no longer described as spellable (#777) Lane 2's review of #811 (rv811, at 854f6ed9), SHOULDs 2 and 3 and NITs 4 and 5. Its MUST 1 is b5def3f0. - The prompt's `.strip()` took off every kind of whitespace, not only the spaces its docstring names: a tab, VT, FF, `\x1c` to `\x1f`, NEL, NBSP and U+3000. Typed, each gave `[ok] project Atlas` and exit 0, where `--project` refuses the same value as a control character or by the name pattern, and where #756's head refused it at the prompt too. It is now `.strip(" ")`. A typed line holds no newline here (`ask` drops it), so nothing else needs taking off. The test file's typed-refusal test takes the nine values, with stdin the terminal and with stdin a pipe, and the argv refusals take them as `--project`. The same reason is given on every way in. Each control character is typed after ^V, so the terminal takes it as text (`\x1c` alone is the quit key). - The name sentence said "letters" and "spaces", but `NAME_RE` takes only ASCII letters and U+0020, so its words described `Caf\xe9` and a no-break space as values that pass. Both sentences now say "an ASCII letter or digit first, then ASCII letters, digits, the space and ..." (the ref sentence without the space), as `adopt-project.py`'s help says "ASCII letters, digits and ...". The drift guards also walk every character above ASCII and find none taken. README, the handbook, AGENTS.md and docs/cli.md do not state either sentence (`render-cli-reference.py --check` is ok). - A ref is refused for `.lock` only where it ends in it: a new test passes `topic.locked` and `a.lockb` through `checked_ref`. - `a_run_for`'s list comprehension that only copied is gone. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- setup-project.py | 27 +++++--- tests/test_setup_project_name_edges.py | 88 +++++++++++++++++++++----- 2 files changed, 90 insertions(+), 25 deletions(-) diff --git a/setup-project.py b/setup-project.py index dc95a73c..f1052b8d 100755 --- a/setup-project.py +++ b/setup-project.py @@ -382,14 +382,17 @@ def program_version(argv, field: int) -> str: #: sibling `repo_shape.checked_value` prints its own. `PATH_RE` has no row: it #: refuses only an empty value and a control character, and both are named #: before a pattern is asked; a pattern with no row is still named, as the -#: sibling names it, by `why_refused`. +#: sibling names it, by `why_refused`. The words say ASCII because the +#: patterns are: "letters" and "spaces" alone describe `Caf\xe9` and a +#: no-break space as values that pass (rv811). WHY_PATTERN = { - NAME_RE: ("It is not spellable as a name: a letter or a digit first, " - "then letters, digits, spaces and `._'()&,+@-`, at most 255 " - "characters in all (`%s`)." % NAME_RE.pattern), - REF_RE: ("It is not spellable as a ref: a letter or a digit first, then " - "letters, digits and `._/-`, at most 255 characters in all " - "(`%s`)." % REF_RE.pattern), + NAME_RE: ("It is not spellable as a name: an ASCII letter or digit " + "first, then ASCII letters, digits, the space and " + "`._'()&,+@-`, at most 255 characters in all (`%s`)." + % NAME_RE.pattern), + REF_RE: ("It is not spellable as a ref: an ASCII letter or digit first, " + "then ASCII letters, digits and `._/-`, at most 255 characters " + "in all (`%s`)." % REF_RE.pattern), } @@ -1586,8 +1589,16 @@ def ask_for_the_project_name(local_mode: bool) -> str: slip of the thumb, and the run ends at the first mistake with no second ask, so the prompt takes it off; an argv value is deliberate and is held as typed. A line of nothing but spaces is then an empty line. + + ONLY THE SPACE COMES OFF, NOT EVERY KIND OF WHITESPACE. `str.strip()` + with no argument also took off a tab, a form feed, `\\x1c` to `\\x1f`, a + no-break space and the ideographic space, so the prompt accepted each of + them where `--project` refuses it, as a control character or by the + pattern (rv811). Now only the space comes off, and the rest reach + `checked_value` as they were typed. """ - typed = ask(" project name (ONE CamelCase token, e.g. Atlas): ").strip() + typed = ask(" project name (ONE CamelCase token, e.g. Atlas): ") + typed = typed.strip(" ") if not typed: die("no project name was typed; nothing was created.") return checked_project_length( diff --git a/tests/test_setup_project_name_edges.py b/tests/test_setup_project_name_edges.py index a2e2282c..a43ff981 100644 --- a/tests/test_setup_project_name_edges.py +++ b/tests/test_setup_project_name_edges.py @@ -20,7 +20,8 @@ * `Atlas ` passed `NAME_RE` where ` Atlas` was refused, and step (4) judged `Atlas -spec` with no space shown anywhere. The prompt now takes the spaces off the ends of what was typed; an argv value is deliberate and is held as - typed. + typed. Only the space comes off: a tab, a no-break space or any other + whitespace is refused at the prompt as `--project` refuses it. POSIX only where a run needs a terminal or a `gh` stub (`PTY_SKIP`). """ @@ -37,7 +38,7 @@ from conftest import PTY_SKIP, REPO, run_in_a_terminal from test_setup_project_py import (SETUP_PROJECT, entry_point_module, - run_entry, terminal_env) + piped, run_entry, terminal_env) from test_setup_project_typed_name import (BELL, LNEXT, NAMING_POLICY, PROMPT, the_refusal) @@ -52,11 +53,11 @@ "escape: a newline ends a line in every terminal and log this " "output reaches, so a value carrying one could print a line that " "reads as this tool's own.") -NAME = ("It is not spellable as a name: a letter or a digit first, then " - "letters, digits, spaces and `._'()&,+@-`, at most 255 characters in " - "all (`^[A-Za-z0-9][A-Za-z0-9 ._'()&,+@-]{0,254}$`).") -REF = ("It is not spellable as a ref: a letter or a digit first, then " - "letters, digits and `._/-`, at most 255 characters in all " +NAME = ("It is not spellable as a name: an ASCII letter or digit first, " + "then ASCII letters, digits, the space and `._'()&,+@-`, at most 255 " + "characters in all (`^[A-Za-z0-9][A-Za-z0-9 ._'()&,+@-]{0,254}$`).") +REF = ("It is not spellable as a ref: an ASCII letter or digit first, then " + "ASCII letters, digits and `._/-`, at most 255 characters in all " "(`^[A-Za-z0-9][A-Za-z0-9._/-]{0,254}$`).") RANGE = ("It holds `..`, which git reads as a revision range and not as a " "ref.") @@ -68,6 +69,22 @@ ACCENTED = "Caf\u00e9" +#: Whitespace that is not the space. `str.strip()` took each off a typed name +#: where `--project` refuses the same value (rv811), as a control character +#: or by the name pattern. Only the space comes off at the prompt, so each is +#: refused on every way in, for the same reason. +NOT_THE_SPACE = { + "tab-after": ("Atlas\t", CONTROL), + "tab-before": ("\tAtlas", CONTROL), + "vertical-tab": ("Atlas\x0b", CONTROL), + "form-feed": ("Atlas\x0c", CONTROL), + "file-separator": ("\x1cAtlas", CONTROL), + "unit-separator": ("\x1fAtlas", CONTROL), + "no-break-space": ("Atlas\xa0", NAME), + "next-line": ("Atlas\x85", NAME), + "ideographic-space": ("Atlas\u3000", NAME), +} + #: Each refusal: the argv that carries the value (`{}` is the value), the #: value, how the refusal names it, and the reason it gives. A value that #: fails two checks is told the first, in the order empty, `-`, control @@ -100,6 +117,8 @@ "ref-dash": (("--shape-ref", "{}"), "-x", "--shape-ref", DASH), "ref-range": (("--shape-ref", "{}"), "a..b", "--shape-ref", RANGE), "ref-lock": (("--shape-ref", "{}"), "topic.lock", "--shape-ref", LOCK), + **{"project-" + case: (("--project", "{}"), value, "--project", why) + for case, (value, why) in NOT_THE_SPACE.items()}, } @@ -138,6 +157,7 @@ def test_a_refusal_names_the_check_the_value_failed(tmp_path, case): "trailing-slash": ("Atlas/", NAME), "accented": (ACCENTED, NAME), "long": ("x" * 256, NAME), + **NOT_THE_SPACE, } @@ -182,15 +202,26 @@ def flags_for(tmp_path, local): if local else []), "--", "--dry-run"] -def at_the_prompt(tmp_path, typed, local=True): - """Setup asked for its project at a terminal and answered `typed`.""" +def typed_as_text(typed): + """`typed` as keys at a terminal in canonical mode: ^V before each + control character, so the terminal takes it as text and not as an edit + or a signal (the file separator alone is the quit key, Ctrl-\\).""" + return "".join(LNEXT + char if ord(char) < 0x20 or char == "\x7f" + else char for char in typed) + + +def at_the_prompt(tmp_path, typed, local=True, entry="py"): + """Setup asked for its project at a terminal and answered `typed`. + `entry` is the file run directly, or that file with its stdin a pipe, + when the answer is read from the controlling terminal.""" env = terminal_env() env.update(gh_that_is_signed_in(tmp_path)) + flags = flags_for(tmp_path, local) + argv = {"py": [sys.executable, str(SETUP_PROJECT), *flags], + "piped": piped("", *flags)}[entry] status, screen, answered = run_in_a_terminal( - [sys.executable, str(SETUP_PROJECT), *flags_for(tmp_path, local)], - cwd=REPO, env=env, - answers=[typed.replace(BELL, LNEXT + BELL) + "\n"], prompt=PROMPT, - timeout=60) + argv, cwd=REPO, env=env, answers=[typed_as_text(typed) + "\n"], + prompt=PROMPT, timeout=60) assert answered == 1, f"it never asked for a project name:\n{screen}" return status, screen @@ -201,10 +232,12 @@ def assert_nothing_was_made(tmp_path): @PTY_SKIP +@pytest.mark.parametrize("entry", ["py", "piped"]) @pytest.mark.parametrize("case", TYPED) -def test_a_typed_name_is_refused_with_the_check_it_failed(tmp_path, case): +def test_a_typed_name_is_refused_with_the_check_it_failed(tmp_path, case, + entry): value, why = TYPED[case] - status, screen = at_the_prompt(tmp_path, value) + status, screen = at_the_prompt(tmp_path, value, entry=entry) assert status == 2, f"status {status}:\n{screen}" assert the_refusal(screen) == ( "REFUSED: " + FIRST.format(what="project name", shown=ascii(value)) @@ -240,7 +273,7 @@ def a_run_for(tmp_path, project, local, positional=False, late_local=False): """The project given on the command line, and `-- --dry-run`.""" flags = flags_for(tmp_path, local) if late_local: - flags = [f for f in flags_for(tmp_path, False)] + flags = flags_for(tmp_path, False) flags[flags.index("--"):flags.index("--")] = [ "--local-remote-dir", str(tmp_path / "remotes")] given = [project] if positional else ["--project", project] @@ -386,10 +419,21 @@ def test_an_argv_name_is_held_as_typed(tmp_path): # --- the sentences say what the checks are ------------------------------------- +def above_ascii_taken(pattern, letter): + """Every character above ASCII that `pattern` takes first or after + `letter`: none, as the sentences' "ASCII" says. A walk over + `string.printable` alone cannot see an accented letter (rv811).""" + return [hex(code) for code in range(0x80, sys.maxunicode + 1) + if pattern.fullmatch(letter + chr(code)) + or pattern.fullmatch(chr(code) + letter)] + + def test_the_name_sentence_says_what_the_pattern_holds(): """The words and the pattern cannot drift: every printable character is - in the sentence's list exactly when `NAME_RE` takes it after a letter.""" + in the sentence's list exactly when `NAME_RE` takes it after a letter, + and no character above ASCII is taken anywhere.""" module = entry_point_module() + assert above_ascii_taken(module.NAME_RE, "A") == [] said = set(string.ascii_letters + string.digits + " ._'()&,+@-") for char in string.printable: if char in "\r\n\x0b\x0c": @@ -405,6 +449,7 @@ def test_the_name_sentence_says_what_the_pattern_holds(): def test_the_ref_sentence_says_what_the_pattern_holds(): module = entry_point_module() + assert above_ascii_taken(module.REF_RE, "a") == [] said = set(string.ascii_letters + string.digits + "._/-") for char in string.printable: if char in "\r\n\x0b\x0c": @@ -415,6 +460,15 @@ def test_the_ref_sentence_says_what_the_pattern_holds(): assert not module.REF_RE.fullmatch("a" * 256) +def test_a_ref_is_refused_for_lock_only_at_its_end(): + """A ref that ends in `.lock` is refused; `topic.locked` and `a.lockb` + hold it inside a word, are refs git checks out (`git check-ref-format + --branch` takes both), and go on.""" + module = entry_point_module() + for text in ("topic.locked", "a.lockb"): + assert module.checked_ref("--shape-ref", text) == text + + def test_a_pattern_with_no_words_is_named_as_the_sibling_names_it(): """`PATH_RE` has no row in `WHY_PATTERN`: its only failures are an empty value and a control character, named first. A pattern that did get here From b6f56cad59315f3581b76d25d319a8ddd36f2cf4 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sun, 18 Oct 2026 02:50:08 +0000 Subject: [PATCH 6/6] tests/test_setup_project_name_edges.py: the credential helper's entry is appended through `appended_to_command_scope`, as #848 asks of every hand-worked count, so the scan in `test_no_auto_maintenance.py` passes again (#777) `gh_that_is_signed_in` worked its `GIT_CONFIG_COUNT` out by hand from the environment's own count (`held + 1`, `GIT_CONFIG_KEY_{held}` and `GIT_CONFIG_VALUE_{held}`). That sum was right, and #848's scan (`test_no_test_sets_a_git_config_count_of_its_own`) refuses it in a test file now: it named `test_setup_project_name_edges.py:190`. #848 landed first, and its second-lander note gives this conversion to whichever of #848 and #811 lands second. The three entries now come from `appended_to_command_scope(os.environ, ...)`, merged into the returned dict beside `PATH`. The same three variables get the same values, under an environment that holds two entries, none, or an empty count. The docstring names the helper. No other line of the file changes. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- tests/test_setup_project_name_edges.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/test_setup_project_name_edges.py b/tests/test_setup_project_name_edges.py index a43ff981..1f4aa53f 100644 --- a/tests/test_setup_project_name_edges.py +++ b/tests/test_setup_project_name_edges.py @@ -36,7 +36,8 @@ import pytest -from conftest import PTY_SKIP, REPO, run_in_a_terminal +from conftest import (PTY_SKIP, REPO, appended_to_command_scope, + run_in_a_terminal) from test_setup_project_py import (SETUP_PROJECT, entry_point_module, piped, run_entry, terminal_env) from test_setup_project_typed_name import (BELL, LNEXT, NAMING_POLICY, PROMPT, @@ -168,12 +169,12 @@ def gh_that_is_signed_in(tmp_path): Its git has a credential helper for github.com: the line `gh auth setup-git` writes, pointed at this stub, in git's command scope - (`GIT_CONFIG_COUNT`, appended to what the environment holds). Without - it, the machine's own git config decided whether a GitHub run's - preflight offered `gh auth setup-git`. A machine with a helper anywhere - was never offered it; CI's runner has none, and its runs waited at that - offer's `Type yes to continue:`, which nothing here answers, until they - were killed (#777's first CI run).""" + (`GIT_CONFIG_COUNT`, appended to what the environment holds, by + `appended_to_command_scope`). Without it, the machine's own git config + decided whether a GitHub run's preflight offered `gh auth setup-git`. A + machine with a helper anywhere was never offered it; CI's runner has + none, and its runs waited at that offer's `Type yes to continue:`, which + nothing here answers, until they were killed (#777's first CI run).""" stub = tmp_path / "ghbin" stub.mkdir(exist_ok=True) gh = stub / "gh" @@ -185,12 +186,11 @@ def gh_that_is_signed_in(tmp_path): b' *) echo "stub gh: refused $*" >&2; exit 97 ;;\n' b"esac\n") gh.chmod(0o755) - held = int(os.environ.get("GIT_CONFIG_COUNT") or 0) + helper = "!%s auth git-credential" % shlex.quote(str(gh)) return {"PATH": str(stub) + os.pathsep + os.environ["PATH"], - "GIT_CONFIG_COUNT": str(held + 1), - f"GIT_CONFIG_KEY_{held}": "credential.https://github.com.helper", - f"GIT_CONFIG_VALUE_{held}": - "!%s auth git-credential" % shlex.quote(str(gh))} + **appended_to_command_scope( + os.environ, + [("credential.https://github.com.helper", helper)])} def flags_for(tmp_path, local):