From e016994872ef6ce64f0f6e5ae6b12d96334448e7 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sun, 18 Oct 2026 18:15:46 +0000 Subject: [PATCH 1/2] scaffold-project.py, `family.py init` and shape_materialize.py: the plan names the tracking branch, and on disk the visibility, and says "not found" is what this token can see; a root or reused leg whose visibility GitHub did not give is said to be unread, the root before the plan and read once; every holder gets the members' credential NOTE; and an internal root's unread plan is no longer tied to the Free plan (#913) - U163, L2-92: `_reused_leg_visibility` returns None for a read that failed, where it returned the run's visibility, and `_private_leg_note` names each such leg in a NOTE of its own, goes on by the legs it could read, and asks the Free-plan note when a leg is closed or unread. - U167, L2-96: `_leg_visibility` reads the root once, through `repository_visibility_answer`, which tells gh's not-found answer apart, and says before the plan when the read failed, so `--visibility` can be passed before any leg exists; `_create_remotes` takes that read instead of asking GitHub again. - U164, L2-93: an internal root whose organisation's plan is unread is told `FREE_PLAN_UNREAD_INTERNAL`, which says the plan is not what the hint hangs on. - U166, L2-95: `init` prints the credential NOTE for every holder on GitHub, worded about the private or internal members a person adds. - U168, L2-97: the `not-found-on-stdout-alone` row holds gh's not-found words on stdout. - U010, AT-scaffold N3: the scaffold's and `init`'s plans have a `branch` row, and under `--local-remote-dir` a `visibility` row. - U060, L2-61: the `remotes` row says it creates what gh does not find with this token, and that a private repository it cannot see is not found either. U039 ("Needs a ruling": what `visibility:` records) is left as it is. Co-Authored-By: Claude Opus 5.5 --- scaffold-project.py | 189 +++++++++++++++---- scripts/family.py | 50 +++-- scripts/shape_materialize.py | 104 ++++++++--- tests/test_free_plan_hint_reads.py | 201 +++++++++++++------- tests/test_plan_visibility_notes.py | 275 ++++++++++++++++++++++++++++ 5 files changed, 680 insertions(+), 139 deletions(-) create mode 100644 tests/test_plan_visibility_notes.py diff --git a/scaffold-project.py b/scaffold-project.py index c382fa87..30b05f62 100755 --- a/scaffold-project.py +++ b/scaffold-project.py @@ -107,8 +107,10 @@ # The root's visibility and the Free-plan secrets note, asked as `family.py` # asks them of its holder (#660). from shape_materialize import ( # noqa: E402 - free_plan_secrets_note, repository_visibility, + free_plan_secrets_note, repository_visibility_answer, ) +# What the plan says "not found" means against GitHub (#913, L2-61). +from shape_materialize import NOT_FOUND_WITH_THIS_TOKEN # noqa: E402 # What the new project's own reader and `validate-manifest.py` would make of # the values written into its manifest, and whether a repository's name can # be a directory's: asked before anything is created (#258, #261). @@ -245,11 +247,15 @@ def remote_is_empty(repository: str) -> bool | None: def remote_visibility(repository: str) -> str | None: """`/`'s GitHub visibility (`private`, `public`, `internal`), - lowercased, or None if it cannot be read — the repository does not exist, - `gh` cannot see it, or it answered a word that is not one of the three. - It is `repository_visibility`, the reader `family.py` asks of its holder - (#660). Never called for a `--local-remote-dir` repository: a bare - repository on disk has no visibility at all. + lowercased; `REPOSITORY_NOT_FOUND` where gh's answer is that this token + finds no such repository; or None if it cannot be read — `gh repo view` + failed otherwise, or answered a word that is not one of the three. It is + `repository_visibility_answer`, the reader `family.py` asks of its + holder (#660), which tells not-found apart (#913): `_leg_visibility` + says before its plan that a root it could not read is unread, and a root + not there yet is the ordinary run. Never called for a + `--local-remote-dir` repository: a bare repository on disk has no + visibility at all. NO `gh` ON PATH IS `gh-not-found` (#537), as `remote_is_empty`'s is, and not a `FileNotFoundError` traceback: `--reuse-empty-repo` reads the root's @@ -258,7 +264,7 @@ def remote_visibility(repository: str) -> str | None: has to come from here too or the run never reaches the one that has it. """ try: - return repository_visibility(repository) + return repository_visibility_answer(repository) except FileNotFoundError as exc: raise _gh_not_found( f"what visibility {repository} has on GitHub", exc) from exc @@ -1278,9 +1284,29 @@ def _repository_urls(args, names: dict) -> tuple[bool, dict, dict]: return local, repositories, urls -def _leg_visibility(args, assembly_repository: str, local: bool) -> str: +#: What the scaffold says before its plan where `--reuse-empty-repo` could not +#: read the root's visibility (#913, L2-96): `ROOT_VISIBILITY_UNREAD`'s leg +#: twin, said while `--visibility` can still choose what a leg it creates +#: is. The first is said with no `--visibility`, the second with one. +ROOT_UNREAD_LEGS_DEFAULT = ( + "NOTE this run could not read {root}'s visibility from GitHub, so a leg " + "this run creates is created private, the default, and does not inherit " + "{root}'s; a leg it reuses keeps its own. Pass --visibility to choose " + "one before any leg exists; `gh repo view {root} --json visibility` " + "says which {root} is.") +ROOT_UNREAD_LEGS_CHOSEN = ( + "NOTE this run could not read {root}'s visibility from GitHub, so it " + "cannot say whether --visibility {visibility} agrees with it. A leg this " + "run creates is created {visibility}, as you asked; a leg it reuses " + "keeps its own. `gh repo view {root} --json visibility` says which " + "{root} is.") + + +def _leg_visibility(args, assembly_repository: str, + local: bool) -> tuple[str, str | None]: """(h) The visibility a leg this run CREATES is created at: explicit, or - inherited from a reused empty root. + inherited from a reused empty root; and the root's own, as GitHub gave + it, or None. Split out of `_scaffold` for #138. An organisation that created the assembly root first (--reuse-empty-repo) generally wants the legs this @@ -1296,26 +1322,47 @@ def _leg_visibility(args, assembly_repository: str, local: bool) -> str: there; each line now says what happens to a leg this run creates, and that a reused leg keeps its own visibility, which `_reused_leg_visibility` reads and warns about leg by leg. + + A ROOT IT COULD NOT READ IS SAID TO BE UNREAD, HERE (#913, L2-96). The + legs it creates were made private with no word, `--dry-run` included, + and only a real run's `_private_leg_note` said anything about the read. + `ROOT_UNREAD_LEGS_DEFAULT` or `ROOT_UNREAD_LEGS_CHOSEN` is printed before + the plan, so a person can pass `--visibility` before any leg exists. A + root gh does not find is the ordinary run, which creates it: nothing is + said of it. + + THE ROOT IS READ ONCE (#913, L2-96). `_create_remotes` asked GitHub again + for a reused root, and the two reads could disagree; the second element + of the answer is this read, the root's visibility where GitHub gave one, + and `_create_remotes` takes it. It is None where nothing was read (no + `--reuse-empty-repo`, or `--local-remote-dir`), where gh does not find + the root, and where the read failed. """ - reused_visibility = None - if args.reuse_empty_repo and not local: - reused_visibility = remote_visibility(assembly_repository) + if not args.reuse_empty_repo or local: + return args.visibility or "private", None + seen = remote_visibility(assembly_repository) + if seen is None: + unread = (ROOT_UNREAD_LEGS_DEFAULT if args.visibility is None + else ROOT_UNREAD_LEGS_CHOSEN) + print(unread.format(root=assembly_repository, + visibility=args.visibility)) + root = seen if seen in VISIBILITY_CHOICES else None if args.visibility is None: - if reused_visibility: - print(f"NOTE {assembly_repository} is already {reused_visibility} " + if root: + print(f"NOTE {assembly_repository} is already {root} " "on GitHub; a leg this run creates inherits that " "visibility, and a leg it reuses keeps its own (pass " "--visibility to choose one explicitly).") - return reused_visibility or "private" - if reused_visibility and reused_visibility != args.visibility: + return root or "private", root + if root and root != args.visibility: print(f"WARNING --visibility {args.visibility} disagrees with the " - f"reused {assembly_repository}, which is {reused_visibility} " + f"reused {assembly_repository}, which is {root} " "on GitHub. A leg this run creates is created " f"{args.visibility} because that is what you asked for, not " "because it is what the assembly root already is; a leg it " "reuses keeps its own.", file=sys.stderr) - return args.visibility + return args.visibility, root def _print_plan(args, values: dict, names: dict, repositories: dict, @@ -1385,15 +1432,36 @@ def _print_declared_classification(policy, names: dict, declared_pins: set, def _print_plan_actions(args, topic: str, local: bool) -> None: - """(i″) The four lines that say what this run will DO — create the - remotes, reuse an empty root, push, set the topic. Split out of + """(i″) The lines that say what this run will DO — create the remotes, + reuse an empty root, the branch, push, set the topic. Split out of `_print_plan` for #138. + + THE BRANCH AND, ON DISK, THE VISIBILITY ARE NAMED (#913, AT-scaffold N3). + The plan a person approves, here and in setup's step (5), named neither + the tracking branch the three repositories are created on nor, under + `--local-remote-dir`, the visibility the manifest records: against + GitHub `gh repo create --` names it, and a bare repository + on disk has none. And "NOT FOUND" IS WHAT THIS TOKEN CAN SEE (#913, + L2-61): only gh's not-found answer reads as absent (#609), and GitHub + gives it for a private repository the token cannot see, so the real run + would create the legs and then fail at `gh repo create` for a root that + exists. gh's answer cannot tell the two apart, so the row says which + repositories it creates in those words (`NOT_FOUND_WITH_THIS_TOKEN`). """ - print("remotes " + ("bare repositories on disk (no network)" if local - else f"gh repo create --{args.visibility}")) + if local: + print(f"visibility {args.visibility}, which project.yaml records; " + "a bare repository on disk has none") + print("remotes bare repositories on disk (no network)") + else: + print(f"remotes gh repo create --{args.visibility}, for each of " + "the three gh does not find") + for line in NOT_FOUND_WITH_THIS_TOKEN: + print(f" {line}") if args.reuse_empty_repo: print("reuse each of the three that EXISTS with zero commits " "is used, and a leg whose one commit is this scaffold's seed") + print(f"branch {args.tracking_branch}, the tracking branch each of " + "the three is created on") print("push " + ("SKIPPED (--no-push)" if args.no_push else "yes")) print("topics " + ("skipped for local remotes" if local else f"gh repo edit --add-topic {topic}")) @@ -1451,7 +1519,8 @@ def _refuse_unmakeable_folders(args, work_root: Path | None, names: dict, def _create_remotes(args, values: dict, repositories: dict, urls: dict, - local: bool, reused: dict) -> dict[str, str]: + local: bool, reused: dict, + root_seen: str | None) -> dict[str, str | None]: """(k) The three remotes, spec and code first so a failure costs the cheapest thing. Split out of `_scaffold` for #138. @@ -1462,13 +1531,14 @@ def _create_remotes(args, values: dict, repositories: dict, urls: dict, Returns each repository's visibility as it now stands, by role, for `_private_leg_note` (#542): the run's for one created here, and for a - reused one what GitHub says of it. A reused leg's is what - `_reused_leg_visibility` read, with its WARNING. The reused ROOT's is read - here without one, because `_leg_visibility` has already said where the - run's flag disagrees with it, and is None where GitHub does not say - (#660): the run's `--visibility` is not the root's, so a public root - whose read failed was given the Free-plan hint as private, and - `_private_leg_note` says now that it could not be read. + reused one what GitHub says of it, or None where GitHub does not say. A + reused leg's is what `_reused_leg_visibility` read, with its WARNING. + The reused ROOT's is `root_seen`, the one read `_leg_visibility` made + before the plan and said what it found (#913, L2-96): it was read a + second time here, and the two reads could disagree. It is None where + GitHub did not say (#660): the run's `--visibility` is not the root's, + so a public root whose read failed was given the Free-plan hint as + private, and `_private_leg_note` says now that it could not be read. """ display = values["PROJECT_NAME"] project_id = values["PROJECT_ID"] @@ -1481,7 +1551,7 @@ def _create_remotes(args, values: dict, repositories: dict, urls: dict, + ("zero commits)" if reused[role] is None else f"holding this scaffold's seed {reused[role][:12]})")) if not local and role == "assembly": - standing[role] = remote_visibility(repositories[role]) + standing[role] = root_seen elif not local: standing[role] = _reused_leg_visibility(args, repositories[role]) @@ -1502,7 +1572,7 @@ def _create_remotes(args, values: dict, repositories: dict, urls: dict, return standing -def _reused_leg_visibility(args, repository: str) -> str: +def _reused_leg_visibility(args, repository: str) -> str | None: """(k″) A reused leg keeps the visibility the run that created it gave it (#462), and a re-run whose visibility disagrees is told so, as `_leg_visibility` tells it about a reused root. Nothing here changes a @@ -1510,16 +1580,22 @@ def _reused_leg_visibility(args, repository: str) -> str: which has none. Returns the visibility GitHub reports for the leg, which is the one - `_private_leg_note` goes by (#542), or the run's when GitHub does not - say: the one value this run has for it then. + `_private_leg_note` goes by (#542), or None when GitHub does not say + (#913, L2-92): a read that failed, or gh not finding a leg it found a + moment before. It returned the run's then, so the leg NOTE and the + Free-plan hint went by a value GitHub did not give, hiding a true hint + or printing a false NOTE, and nothing said the read failed; the root's + rule since #660, for each leg. """ seen = remote_visibility(repository) - if seen and seen != args.visibility: + if seen not in VISIBILITY_CHOICES: + return None + if seen != args.visibility: print(f"WARNING {repository} is reused and is {seen} on GitHub, " f"where this run's visibility is {args.visibility}. It keeps " "the visibility it has: this run changes no repository's " "visibility.", file=sys.stderr) - return seen or args.visibility + return seen def _private_leg_note(args, repositories: dict, local: bool, @@ -1557,9 +1633,17 @@ def _private_leg_note(args, repositories: dict, local: bool, the run's `--visibility`, and a closed root whose organisation's plan could not be read is `FREE_PLAN_UNREAD`, where it was silent as for a paid plan. + + AND A REUSED LEG IT COULD NOT READ (#913, L2-92), `standing` None, is + named by `_unread_legs_note`, where it was taken at the run's + visibility: a public leg under a private run was called private, and a + private one under a public run got no NOTE. The legs it could read are + named as before, and the Free-plan note follows either NOTE, since an + unread leg may be closed. """ closed = [role for role in ("spec", "code") if standing[role] in ("private", "internal")] + unread = [role for role in ("spec", "code") if standing[role] is None] if not local and closed: both = len(closed) == 2 who = " and ".join(repositories[role] for role in closed) @@ -1574,12 +1658,39 @@ def _private_leg_note(args, repositories: dict, local: bool, "SHAPE_LEGS_APP_PRIVATE_KEY, preferred) or a SHAPE_LEGS_TOKEN " f"PAT (contents:read on {which}, fallback) — or the `validate` " f"check cannot check {them} out.") + if not local and unread: + print(_unread_legs_note(repositories, unread)) + if not local and (closed or unread): note = free_plan_secrets_note(args.org, repositories["assembly"], standing["assembly"]) if note: print(note) +def _unread_legs_note(repositories: dict, unread: list) -> str: + """(k‴) The NOTE for each reused leg whose visibility GitHub did not give + (#913, L2-92), as `ROOT_VISIBILITY_UNREAD` names a root: the question + that went unanswered, what the answer would mean, and the command that + answers it. `unread` is the roles, `spec` before `code`.""" + root = repositories["assembly"] + legs = [repositories[role] for role in unread] + if len(legs) == 1: + whose, them = f"{legs[0]}'s visibility", "it" + who, which = "if it is", "that leg" + asks = f"`gh repo view {legs[0]} --json visibility` says which it is." + else: + whose, them = f"the visibility of {legs[0]} and {legs[1]}", "them" + who, which = "for each that is", "the legs" + asks = (f"`gh repo view {legs[0]} --json visibility` and `gh repo " + f"view {legs[1]} --json visibility` say which each is.") + return (f"NOTE this run could not read {whose} from GitHub, so it cannot " + f"say whether {root} needs a way to read {them}: {who} private " + f"or internal, give {root} a GitHub App (SHAPE_LEGS_APP_ID + " + "SHAPE_LEGS_APP_PRIVATE_KEY, preferred) or a SHAPE_LEGS_TOKEN " + f"PAT (contents:read on {which}, fallback), or the `validate` " + f"check cannot check it out. {asks}") + + def _take_held_seed(work: Path, url: str, seed: str, tracking: str) -> str: """(l′) A leg whose remote already holds this scaffold's seed (#462): the work clone, its files just written, is put AT that commit rather than @@ -1953,7 +2064,8 @@ def _scaffold(args) -> int: args.tracking_branch) shape_commit, shape_tree = _shape_revision() - args.visibility = _leg_visibility(args, repositories["assembly"], local) + args.visibility, root_seen = _leg_visibility( + args, repositories["assembly"], local) values = { "PROJECT": project, @@ -2034,7 +2146,8 @@ def _scaffold(args) -> int: make_folder(FOLDER_REFUSAL, "--local-remote-dir", args.local_remote_dir.resolve()) - standing = _create_remotes(args, values, repositories, urls, local, reused) + standing = _create_remotes(args, values, repositories, urls, local, reused, + root_seen) _private_leg_note(args, repositories, local, standing) leg_values = _seed_legs(args, values, names, repositories, urls, work_root, diff --git a/scripts/family.py b/scripts/family.py index e8ed8bd6..c2dd8ac5 100755 --- a/scripts/family.py +++ b/scripts/family.py @@ -229,6 +229,9 @@ _both_ends, refuse_a_topic_github_cannot_set, refuse_an_owner_github_cannot_have, refuse_an_owner_github_cannot_spell, ) +# What `init`'s plan says "not found" means against GitHub, in the +# scaffold's words (#913, L2-61). +from shape_materialize import NOT_FOUND_WITH_THIS_TOKEN # noqa: E402 NAMING_POLICY = SHAPE_ROOT / "contracts" / "repository-naming.yaml" MANIFEST = "family.yaml" @@ -2706,6 +2709,14 @@ def _print_init_plan(values: dict, repository: str, url: str, a line break and then `WARNING forged-line: pwned` printed that line as a line of the plan, on a dry run and on a run refused after it, and an ESC reached the terminal. A printable path is named byte for byte. + + THE BRANCH AND, ON DISK, THE VISIBILITY ARE NAMED (#913), as the + scaffold's plan names them: the plan named neither the tracking branch + the holder is created on nor, under `--local-remote-dir`, the + visibility `family.yaml` records. And "not found" is what this token can + see (#913, L2-61): GitHub answers a private repository the token cannot + see as not found, so the `remotes` row says when the holder is created + in the scaffold's words (`NOT_FOUND_WITH_THIS_TOKEN`). """ print(f"family {values['FAMILY_NAME']} ({values['FAMILY_ID']})") print(f"shape {SHAPE_REPOSITORY} @ {shape_commit[:12]} " @@ -2722,11 +2733,20 @@ def _print_init_plan(values: dict, repository: str, url: str, "it") print(f"members mounted under {MEMBERS_DIR}/; none yet — " "`family.py add` puts one there") - print("remotes " + ("a bare repository on disk (no network)" if local - else f"gh repo create --{values['VISIBILITY']}")) + if local: + print(f"visibility {values['VISIBILITY']}, which {MANIFEST} " + "records; a bare repository on disk has none") + print("remotes a bare repository on disk (no network)") + else: + print(f"remotes gh repo create --{values['VISIBILITY']}, if gh " + f"does not find {repository}") + for line in NOT_FOUND_WITH_THIS_TOKEN: + print(f" {line}") if reuse_empty_repo: print("reuse an EXISTING with zero commits is used as " "the holder") + print(f"branch {values['TRACKING_BRANCH']}, the tracking branch the " + "holder is created on") print("topics " + ("skipped for local remotes" if local else f"gh repo edit --add-topic {topic}")) @@ -2917,12 +2937,21 @@ def _create_holder_worktree(args, work: Path, values: dict, shape_commit: str, def _finish_holder_remote(args, repository: str, topic: str, local: bool, reuse: bool) -> None: - """Set the holder's topic and print the private-visibility NOTE and the + """Set the holder's topic and print the members' credential NOTE and the Free-plan secrets note. Neither applies to a local (no-network) remote. Split from `cmd_init` for #136. + THE CREDENTIAL NOTE IS ABOUT THE MEMBERS, AND EVERY HOLDER GETS IT (#913, + L2-95). The `validate` workflow's default GITHUB_TOKEN cannot read a + private or internal member whatever the holder is, so a public holder + needs the App or the PAT as much as a private one. The NOTE was printed + only for a holder that is private or internal, in words that gave the + holder's visibility as the reason: a public holder whose members are + private was never told, and its `validate` check then could not check + them out. `add`, which knows each member, is not this function's. + BY THE HOLDER AS IT STANDS (#660). The holder is the repository whose `validate` workflow reads the secrets. One this run created has the run's `--visibility`; one `--reuse-empty-repo` reused keeps its own, @@ -2930,8 +2959,8 @@ def _finish_holder_remote(args, repository: str, topic: str, local: bool, (`repository_visibility`). Both lines went by `--visibility` alone, so a public holder reused under the default `private` was called private and given the Free-plan hint. A reused holder whose visibility GitHub - does not give is `ROOT_VISIBILITY_UNREAD` and gets no NOTE: whether it - is private is the very question that went unanswered. + does not give is `ROOT_VISIBILITY_UNREAD`: whether it is private is the + very question that went unanswered. NO MEMBER IS NAMED THAT DOES NOT EXIST (#660). `init` mounts none, and `add` mounts a member of any visibility, so the NOTE's "and its members @@ -2953,12 +2982,11 @@ def _finish_holder_remote(args, repository: str, topic: str, local: bool, visibility = repository_visibility(repository) except OSError: visibility = None - if visibility in ("private", "internal"): - print(f"NOTE {repository} is {visibility}: give it a way to read the " - "members you add that are private or internal — a GitHub App " - "(SHAPE_LEGS_APP_ID + SHAPE_LEGS_APP_PRIVATE_KEY, preferred) or " - "a SHAPE_LEGS_TOKEN PAT with contents:read on the members AND " - "their legs — or the `validate` check cannot check them out.") + print("NOTE a private or internal member you add needs a way for " + f"{repository}, public or not, to read it — a GitHub App " + "(SHAPE_LEGS_APP_ID + SHAPE_LEGS_APP_PRIVATE_KEY, preferred) or a " + "SHAPE_LEGS_TOKEN PAT with contents:read on the members AND their " + "legs — or the `validate` check cannot check that member out.") note = free_plan_secrets_note(args.org, repository, visibility) if note: print(note) diff --git a/scripts/shape_materialize.py b/scripts/shape_materialize.py index 922f791a..b95a7da2 100644 --- a/scripts/shape_materialize.py +++ b/scripts/shape_materialize.py @@ -1788,8 +1788,9 @@ def refuse_unless_not_found(repository: str, probe) -> None: "`validate` workflow and must be set as REPOSITORY secrets on {root} " "instead. `gh repo view {root} --json visibility` says which it is.") -#: The same where the repository is private or internal and GitHub would not -#: name its organisation's plan (#660). `{visibility}` is the repository's. +#: The same where the repository is private and GitHub would not name its +#: organisation's plan (#660); an internal one is told +#: `FREE_PLAN_UNREAD_INTERNAL` (#913). `{visibility}` is the repository's. #: The last sentence rests on GitHub's REST page for `GET /orgs/{org}`: an #: organisation's full details are shown to its owners. FREE_PLAN_UNREAD = ( @@ -1802,6 +1803,39 @@ def refuse_unless_not_found(repository: str, probe) -> None: "orgs/{org} --jq .plan.name`, run with an owner's login, says which plan " "it is on.") +#: `FREE_PLAN_UNREAD` for an INTERNAL repository (#913, L2-93). GitHub gives +#: internal visibility only to a repository of an organisation an enterprise +#: account owns ("About repositories", GitHub's documentation), and such an +#: organisation is not on the Free plan, so `FREE_PLAN_UNREAD`'s "it applies +#: when {org} is on the GitHub Free plan" named a condition that cannot hold +#: of it. The NOTE is printed where it always was; only its words say that +#: the plan is not what the hint hangs on. +FREE_PLAN_UNREAD_INTERNAL = ( + "NOTE this run could not read {org}'s plan from GitHub, but the " + "Free-plan secrets hint does not hang on it: {root} is internal, which " + "GitHub gives only to a repository of an organisation an enterprise " + "account owns, and that organisation is not on the GitHub Free plan. " + "Org-level SHAPE_LEGS_APP_ID / SHAPE_LEGS_APP_PRIVATE_KEY reach {root}'s " + "`validate` workflow where each secret's repository access includes " + "{root}.") + +#: What a creating tool's plan says under its `gh repo create` row (#913, +#: L2-61). Only gh's not-found answer reads as absent (#609), and GitHub +#: gives that answer for a private repository the token cannot see too, so +#: the two cannot be told apart from gh's answer: the plan says that "not +#: found" is what this token can see, and what the real run then meets. The +#: two lines follow a row that ends "gh does not find" and its object. +NOT_FOUND_WITH_THIS_TOKEN = ( + "with this token: a private repository it cannot see is not found", + "either, and `gh repo create` then fails for it") + +#: What `repository_visibility_answer` answers where gh says GitHub has no +#: such repository (#913): exit 1 with `GH_NOT_FOUND_WORDS` on stderr, the +#: reading `refuse_unless_not_found` gives `gh repo view` (#609). GitHub +#: gives a private repository this token cannot see the same answer +#: (`NOT_FOUND_WITH_THIS_TOKEN`). +REPOSITORY_NOT_FOUND = "not-found" + def visibility_in(answer: str) -> str | None: """The visibility a `gh repo view --json visibility` answer names, one of @@ -1818,26 +1852,50 @@ def visibility_in(answer: str) -> str | None: return seen if seen in VISIBILITY_CHOICES else None -def repository_visibility(repository: str) -> str | None: - """`repository`'s visibility on GitHub, one of `VISIBILITY_CHOICES`, or - None when it cannot be read: `gh repo view` failed (a repository the - token cannot see, a rate limit, an outage such as an `HTTP 502`) or - answered anything `visibility_in` does not read as one of the three. +def repository_visibility_answer(repository: str) -> str | None: + """`repository`'s visibility on GitHub, one of `VISIBILITY_CHOICES`; + `REPOSITORY_NOT_FOUND` where gh's own answer is that this token finds no + such repository (`gh_said_not_found`); or None when it cannot be read: + `gh repo view` failed otherwise (a rate limit, a token an organisation's + SAML enforcement refuses, an outage such as an `HTTP 502`) or answered + anything `visibility_in` does not read as one of the three. THE ONE READER of the scaffold and `family.py` (#660), on the model of - `adopt-project.py`'s `_root_visibility` (#602). A `gh` that cannot be - started is NOT answered here: its `OSError` reaches the caller, because - the scaffold refuses a missing `gh` as `gh-not-found` before it creates - anything (#537), and `family.py`, which asks only after `gh` has made or - edited the holder, takes it as unread. Never asked of a - `--local-remote-dir` repository, which has no visibility. + `adopt-project.py`'s `_root_visibility` (#602); `repository_visibility` + is this reading with not-found taken as unread. NOT-FOUND IS TOLD APART + (#913) for the scaffold's root under `--reuse-empty-repo`, whose + visibility the legs it creates inherit: a root not there yet is the + ordinary run, which creates it, and a root whose read failed is one + whose visibility nobody knows, which the scaffold says before its plan. + The command is run here rather than by `run`, as `organisation_plan`'s + is, because gh's answer is read from its own stderr. + + A `gh` that cannot be started is NOT answered here: its `OSError` + reaches the caller, because the scaffold refuses a missing `gh` as + `gh-not-found` before it creates anything (#537), and `family.py`, which + asks only after `gh` has made or edited the holder, takes it as unread. + Never asked of a `--local-remote-dir` repository, which has no + visibility. """ - try: - answer = run(["gh", "repo", "view", repository, "--json", - "visibility"]) - except CommandFailed: - return None - return visibility_in(answer) + probe = subprocess.run( + ["gh", "repo", "view", repository, "--json", "visibility"], + capture_output=True, text=True, encoding="utf-8", errors="replace", + check=False) + if probe.returncode != 0: + return REPOSITORY_NOT_FOUND if gh_said_not_found(probe) else None + return visibility_in(probe.stdout or "") + + +def repository_visibility(repository: str) -> str | None: + """`repository`'s visibility on GitHub, one of `VISIBILITY_CHOICES`, or + None when it cannot be read: `repository_visibility_answer`, with gh's + not-found answer taken as unread too. Its callers ask it of a repository + they have just found, or made, so not finding it is not "absent": it is + a repository the token cannot see, or a read that went wrong (#660). + A `gh` that cannot be started raises, as there. + """ + seen = repository_visibility_answer(repository) + return None if seen == REPOSITORY_NOT_FOUND else seen def organisation_plan(org: str) -> str | None: @@ -1894,8 +1952,10 @@ def free_plan_secrets_note(org: str, root: str, - Private or internal: the plan is asked. On `free` it is `FREE_PLAN_HINT`, with `root` alone as the repository that is private, as the scaffold and adopt name it since #602 and `family.py init` since - #660; where the plan is unread, `FREE_PLAN_UNREAD`; on any other - plan, and where the owner is not an organisation, None. + #660; where the plan is unread, `FREE_PLAN_UNREAD`, or for an internal + root `FREE_PLAN_UNREAD_INTERNAL`, which says the plan cannot be what + makes the hint apply to it (#913); on any other plan, and where the + owner is not an organisation, None. The scaffold passes the run's visibility for a root it created and GitHub's for one it reused, adopt GitHub's for the root it adopted, and @@ -1909,6 +1969,8 @@ def free_plan_secrets_note(org: str, root: str, plan = organisation_plan(org) if plan == "free": return FREE_PLAN_HINT.format(org=org, repo=root, what=f"{root} is") + if plan is None and visibility == "internal": + return FREE_PLAN_UNREAD_INTERNAL.format(org=org, root=root) if plan is None: return FREE_PLAN_UNREAD.format(org=org, root=root, visibility=visibility) diff --git a/tests/test_free_plan_hint_reads.py b/tests/test_free_plan_hint_reads.py index 9ef08847..dcc4c107 100644 --- a/tests/test_free_plan_hint_reads.py +++ b/tests/test_free_plan_hint_reads.py @@ -21,6 +21,12 @@ see was told nothing: the silent failure the hint exists for. - `init`'s hint said " and its members are private" (L2-71), and its NOTE "and its members will be too", of members `init` never makes. +- #913: `init`'s NOTE was printed only for a holder that is private or + internal, so a public holder whose members are private was never told + they need the credential (L2-95); an internal root whose plan could not + be read was told the hint applies when its organisation is on the Free + plan, which cannot hold of it (L2-93); and the `not-found-on-stdout-alone` + row did not hold gh's not-found words on stdout (L2-97). WHAT IS PINNED. One table, the three tools: a root or holder GitHub calls PUBLIC (no hint, no plan asked), PRIVATE (the hint, naming it alone), one @@ -187,31 +193,35 @@ def test_each_tool_says_what_github_said_of_the_root_and_its_plan( assert "its members" not in printed, printed -#: The NOTE `init` prints for a closed holder, after its subject. -HOLDER_NOTE = (": give it a way to read the members you add that are private " - "or internal — a GitHub App (SHAPE_LEGS_APP_ID + " - "SHAPE_LEGS_APP_PRIVATE_KEY, preferred) or a SHAPE_LEGS_TOKEN " - "PAT with contents:read on the members AND their legs — or the " - "`validate` check cannot check them out.") +def member_note(holder: str) -> str: + """The NOTE `init` prints for every holder on GitHub (#913, L2-95): about + the members a person adds, whatever the holder's own visibility.""" + return ("NOTE a private or internal member you add needs a way for " + f"{holder}, public or not, to read it — a GitHub App " + "(SHAPE_LEGS_APP_ID + SHAPE_LEGS_APP_PRIVATE_KEY, preferred) or a " + "SHAPE_LEGS_TOKEN PAT with contents:read on the members AND their " + "legs — or the `validate` check cannot check that member out.") @WINDOWS_SKIP -@pytest.mark.parametrize("seen, errors, noted", [ - ("PUBLIC", None, None), - ("PRIVATE", None, "private"), - ("INTERNAL", None, "internal"), - ("PRIVATE", {HOLDER: BAD_GATEWAY}, None), - ("PRIVATE", {HOLDER: NOT_FOUND}, None), +@pytest.mark.parametrize("seen, errors", [ + ("PUBLIC", None), + ("PRIVATE", None), + ("INTERNAL", None), + ("PRIVATE", {HOLDER: BAD_GATEWAY}), + ("PRIVATE", {HOLDER: NOT_FOUND}), ], ids=["public", "private", "internal", "visibility-502", "visibility-404"]) -def test_init_notes_a_reused_holder_at_its_own_visibility( - tmp_path, monkeypatch, capsys, seen, errors, noted): - """L2-58 and the NOTE's half of L2-71. `init`'s NOTE goes by the reused - holder as GitHub reports it, and names no member: `init` mounts none, and - `add` mounts a member of any visibility, so "and its members will be - too" said what nothing decides. An unread holder gets no NOTE, since - whether it is private is the question that went unanswered. On main the - NOTE said "is private and its members will be too" of every holder the - default `--visibility private` reused, the public one included. +def test_init_notes_the_members_credential_for_every_reused_holder( + tmp_path, monkeypatch, capsys, seen, errors): + """L2-58, the NOTE's half of L2-71, and L2-95. `init`'s NOTE names no + member: `init` mounts none, and `add` mounts a member of any visibility, + so "and its members will be too" said what nothing decides. And it is + about the members, for every holder (#913): the `validate` workflow's + default token cannot read a private or internal member whatever the + holder is. On main it was printed only for a holder GitHub called + private or internal, "NOTE is : give it a way to + read the members ...", so a public holder whose members are private was + never told, and neither was one whose visibility went unread. family.yaml keeps the RUN's `--visibility`, as it always has, which is the decision #660 records: the table that writes it is checked before @@ -219,37 +229,38 @@ def test_init_notes_a_reused_holder_at_its_own_visibility( printed, _calls = run_tool("family", tmp_path, monkeypatch, capsys, None, seen, plan="team", visibility_errors=errors) holder = ROOTS["family"] - if noted is None: - assert f"NOTE {holder} is " not in printed, printed - else: - assert f"NOTE {holder} is {noted}{HOLDER_NOTE}" in printed, printed + assert printed.count(member_note(holder)) == 1, printed + assert f"NOTE {holder} is " not in printed, printed assert "will be too" not in printed, printed manifest = load_yaml(tmp_path / "work" / HOLDER / "family.yaml") assert manifest["visibility"] == "private" @WINDOWS_SKIP -def test_init_asks_nothing_of_a_holder_it_creates(tmp_path): +@pytest.mark.parametrize("visibility", ["private", "public"]) +def test_init_asks_nothing_of_a_holder_it_creates(tmp_path, visibility): """The control: a holder this run creates is created at the run's `--visibility`, which is what it is, so GitHub is not asked; it gets the - NOTE and, on Free, the hint naming it alone.""" + members' NOTE, a public one too (#913, L2-95), and, private on Free, the + hint naming it alone.""" env, log = fake_gh(tmp_path, {}, plan="free") - result = init_on_github(tmp_path, env) + result = init_on_github(tmp_path, env, "--visibility", visibility) assert result.returncode == 0, result.stderr + result.stdout holder = ROOTS["family"] - assert f"NOTE {holder} is private{HOLDER_NOTE}" in result.stdout - assert f"{holder} is private, so org-level" in result.stdout + assert member_note(holder) in result.stdout + assert (f"{holder} is private, so org-level" in result.stdout) == ( + visibility == "private"), result.stdout calls = log.read_text(encoding="utf-8").splitlines() - assert f"repo create {holder} --private" in " | ".join(calls), calls + assert f"repo create {holder} --{visibility}" in " | ".join(calls), calls assert f"repo view {holder} --json visibility" not in calls, calls def test_init_takes_a_holder_gh_cannot_be_asked_about_as_unread( monkeypatch, capsys): """A reused holder whose visibility read cannot start `gh` (an `OSError`) - is unread, as a failed read is: `ROOT_VISIBILITY_UNREAD`, and no NOTE - and no hint at the run's `--visibility`, which is not the holder's - (#660's F8). In process, so it runs on every platform: the topic edit and + is unread, as a failed read is: `ROOT_VISIBILITY_UNREAD`, and no hint at + the run's `--visibility`, which is not the holder's (#660's F8); the + members' NOTE, which goes by no visibility (#913). In process, so it runs on every platform: the topic edit and the plan are answered here, and nothing reaches `gh`.""" family = load_without_bytecode(FAMILY, "family_free_plan_hint_reads") edits: list = [] @@ -269,6 +280,7 @@ def cannot_start(repository): printed = capsys.readouterr().out assert unread_root(holder)[0] in printed, printed assert f"NOTE {holder} is " not in printed, printed + assert member_note(holder) in printed, printed assert HINT not in printed, printed assert edits == [["gh", "repo", "edit", holder, "--add-topic", "xf-project-orbit"]] @@ -276,22 +288,6 @@ def cannot_start(repository): # --- the readers, on every platform ----------------------------------------- -def answering(monkeypatch, answer) -> list: - """`shape_materialize.run` answering every command with `answer`, or - raising it when it is an exception; returns the commands it was asked.""" - asked: list = [] - - def run(args, *_rest, **_keywords): - asked.append(args) - if isinstance(answer, BaseException): - raise answer - return answer - - monkeypatch.setattr(shape_materialize, "run", run) - return asked - - -FAILED = shape_materialize.CommandFailed(["gh"], None, 1, BAD_GATEWAY) NO_GH = FileNotFoundError(2, "No such file or directory", "gh") @@ -309,7 +305,7 @@ def run(args, *_rest, **_keywords): ((1, "", "gh: Resource protected by organization SAML enforcement. (HTTP " "403)\n"), None), ((1, "", f"gh: {BAD_GATEWAY}\n"), None), - ((1, NOT_FOUND_BODY, f"gh: {BAD_GATEWAY}\n"), None), + ((1, NOT_AN_ORGANISATION + "\n", f"gh: {BAD_GATEWAY}\n"), None), (NO_GH, None), ], ids=["free", "free-spaced", "team", "enterprise", "unnamed", "blank-line", "not-an-organisation", "saml-403", "failed-502", @@ -320,7 +316,10 @@ def test_the_plan_is_free_other_or_unread(monkeypatch, answer, plan): organisation (the reading `refuse_unless_not_found` gives, #609); and None where it names no plan, never "other" for a plan it did not name. The command is asked of `subprocess.run`, faked here, since the reading - is of stderr alone.""" + is of stderr alone: the `not-found-on-stdout-alone` row holds gh's + not-found words on stdout, so a reading that searched stdout too would + take it for `not-an-organisation` (#913, L2-97; its stdout was the REST + body, which does not hold them).""" asked: list = [] def gh(args, *_rest, **_keywords): @@ -334,46 +333,110 @@ def gh(args, *_rest, **_keywords): assert asked == [["gh", "api", f"orgs/{ORG}", "--jq", ".plan.name"]] -def test_a_visibility_read_is_one_of_three_or_unread(monkeypatch): - """`repository_visibility`: what `visibility_in` reads, None for a failed - `gh repo view`, and a `gh` that cannot be started left to the caller, - which the scaffold refuses as `gh-not-found` (#537).""" +#: `gh repo view`'s answer for a repository GitHub does not show this token. +NOT_FOUND_IN_GRAPHQL = ("GraphQL: Could not resolve to a Repository with the " + f"name '{ROOTS['scaffold']}'. (repository)\n") + + +@pytest.mark.parametrize("answer, seen", [ + ((0, '{"visibility": "Internal"}\n', ""), "internal"), + ((0, '{"visibility": "PUBLIC"}', ""), "public"), + ((0, '{"visibility": "SECRET"}\n', ""), None), + ((0, "", ""), None), + ((1, "", NOT_FOUND_IN_GRAPHQL), "not-found"), + ((1, NOT_FOUND_IN_GRAPHQL, f"{BAD_GATEWAY}\n"), None), + ((4, "", NOT_FOUND_IN_GRAPHQL), None), + ((1, "", f"{BAD_GATEWAY}\n"), None), + ((1, "", SAML_REFUSED + "\n"), None), +], ids=["internal", "public", "not-a-visibility", "empty", "not-found", + "not-found-on-stdout-alone", "not-found-words-exit-4", "failed-502", + "saml-403"]) +def test_a_visibility_read_is_one_of_three_not_found_or_unread( + monkeypatch, answer, seen): + """`repository_visibility_answer`: what `visibility_in` reads; + `REPOSITORY_NOT_FOUND` for gh's own not-found answer, exit 1 with its + words on stderr, the reading `refuse_unless_not_found` gives (#609), so + the scaffold tells a root not there yet from one it could not read + (#913); and None for every other failure. `repository_visibility` is the + same reading with not-found taken as unread, as its callers ask it of a + repository that is there. Asked of `subprocess.run`, faked here, since + the reading is of stderr alone.""" root = ROOTS["scaffold"] - asked = answering(monkeypatch, '{"visibility": "Internal"}') - assert shape_materialize.repository_visibility(root) == "internal" - assert asked == [["gh", "repo", "view", root, "--json", "visibility"]] - answering(monkeypatch, '{"visibility": "SECRET"}') - assert shape_materialize.repository_visibility(root) is None - answering(monkeypatch, FAILED) - assert shape_materialize.repository_visibility(root) is None - answering(monkeypatch, NO_GH) - with pytest.raises(FileNotFoundError): - shape_materialize.repository_visibility(root) + asked: list = [] + + def gh(args, *_rest, **_keywords): + asked.append(list(args)) + return subprocess.CompletedProcess(args, *answer) + + monkeypatch.setattr(shape_materialize.subprocess, "run", gh) + assert shape_materialize.repository_visibility_answer(root) == seen + assert shape_materialize.repository_visibility(root) == ( + None if seen == shape_materialize.REPOSITORY_NOT_FOUND else seen) + assert asked == [["gh", "repo", "view", root, "--json", "visibility"]] * 2 + + +def test_a_visibility_read_leaves_a_gh_that_cannot_start_to_the_caller( + monkeypatch): + """A `gh` that cannot be started is left to the caller, which the + scaffold refuses as `gh-not-found` (#537) and `family.py` takes as + unread.""" + def gh(args, *_rest, **_keywords): + raise NO_GH + + monkeypatch.setattr(shape_materialize.subprocess, "run", gh) + for read in (shape_materialize.repository_visibility_answer, + shape_materialize.repository_visibility): + with pytest.raises(FileNotFoundError): + read(ROOTS["scaffold"]) + + +#: What a run says of an INTERNAL root whose organisation's plan it could not +#: read (#913, L2-93): the plan is not what the hint hangs on. +def unread_plan_internal(root: str) -> str: + return (f"NOTE this run could not read {ORG}'s plan from GitHub, but the " + "Free-plan secrets hint does not hang on it: " + f"{root} is internal, which GitHub gives only to a repository of " + "an organisation an enterprise account owns, and that " + "organisation is not on the GitHub Free plan. Org-level " + "SHAPE_LEGS_APP_ID / SHAPE_LEGS_APP_PRIVATE_KEY reach " + f"{root}'s `validate` workflow where each secret's repository " + f"access includes {root}.") @pytest.mark.parametrize("visibility, plan, said", [ (None, "free", "root"), ("public", "free", None), ("private", "free", "hint"), ("internal", "free", "hint"), ("private", "other", None), ("private", None, "plan"), + ("internal", None, "internal"), ("internal", "other", None), ("private", "not-an-organisation", None), ], ids=["unread-root", "public", "private-free", "internal-free", - "private-paid", "private-unread-plan", "private-not-an-organisation"]) + "private-paid", "private-unread-plan", "internal-unread-plan", + "internal-paid", "private-not-an-organisation"]) def test_the_note_goes_by_both_answers(monkeypatch, visibility, plan, said): """`free_plan_secrets_note`, the one decision the three tools ask: the plan is asked only of a closed root, and what neither GitHub answer - gives is said.""" + gives is said. An internal root whose plan is unread is told the plan + is not what the hint hangs on (#913, L2-93): it was told the hint + "applies when" its organisation "is on the GitHub Free plan", which an + organisation that holds internal repositories is not.""" asked: list = [] monkeypatch.setattr(shape_materialize, "organisation_plan", lambda org: asked.append(org) or plan) root = ROOTS["adopt"] note = shape_materialize.free_plan_secrets_note(ORG, root, visibility) first = {"hint": HINT, "root": unread_root(root)[0], - "plan": f"NOTE this run could not read {ORG}'s plan"} + "plan": f"NOTE this run could not read {ORG}'s plan", + "internal": unread_plan_internal(root)} if said is None: assert note is None else: assert note.startswith(first[said]), note if said == "hint": assert f"{root} is private, so org-level" in note, note + if said == "plan": + assert note == unread_plan(root)[0], note + if said == "internal": + assert note == unread_plan_internal(root), note + assert "applies when" not in note, note assert asked == ([ORG] if visibility in ("private", "internal") else []), asked diff --git a/tests/test_plan_visibility_notes.py b/tests/test_plan_visibility_notes.py new file mode 100644 index 00000000..92bc0c7b --- /dev/null +++ b/tests/test_plan_visibility_notes.py @@ -0,0 +1,275 @@ +# SPDX-License-Identifier: Apache-2.0 +"""What the scaffold's and `family.py init`'s plan and NOTEs say about the +remotes and their visibility (#913). + +WHAT WAS WRONG, in #459's lines that #913 groups: +- U167, L2-96. Under `--reuse-empty-repo` a root whose `--json visibility` + read failed had the legs this run creates made private with no word, + `--dry-run` included, and only a real run's last NOTE said the read + failed; and the root was read twice, before the plan and again while the + remotes were made, so the two reads could disagree. +- U163, L2-92. A reused leg whose read failed was taken at the run's + visibility, so the leg NOTE and the Free-plan hint went by a value GitHub + did not give: a private leg under a public run got no NOTE and its + private root no hint, and a public leg under a private run was called + private, and nothing said a read failed. +- U010, AT-scaffold N3. The plan a person approves, the scaffold's and + `init`'s, named neither the tracking branch nor, under + `--local-remote-dir`, the visibility the manifest records. +- U060, L2-61. Only gh's not-found answer reads as absent (#609), and + GitHub gives it for a private repository the token cannot see, so the + plan said a repository would be created that the real run then fails to + create. gh's answer cannot tell the two apart, so the plan says "not + found" is what this token can see. + +`gh` is `test_scaffold_reuse_seeded_legs.py`'s stub, first on PATH: it knows +each repository at its visibility, names the plan, and fails the `--json +visibility` reads it is told to. Nothing reaches GitHub. It is a `#!` +script, so the runs that start it are `WINDOWS_SKIP`ped; the plans under +`--local-remote-dir` start no `gh` and run everywhere. +""" + +from __future__ import annotations + +import pytest + +from conftest import ORG, PROJECT, REPO, SCAFFOLD, WINDOWS_SKIP, run_script +from test_scaffold_reuse_seeded_legs import fake_gh, on_github + +FAMILY = REPO / "scripts" / "family.py" +HOLDER = "Orbit" +ROOT = f"{ORG}/{PROJECT}" +SPEC, CODE = f"{ROOT}-spec", f"{ROOT}-code" +BAD_GATEWAY = "HTTP 502: Bad Gateway (https://api.github.com/graphql)" +PLAN_STARTS = "project " +DRY_RUN_DONE = "--dry-run: nothing was created." +VISIBILITY_ASKED = f"repo view {ROOT} --json visibility" + +#: What the scaffold says before its plan of a root it could not read, with +#: no `--visibility` and with `--visibility public`. +ROOT_UNREAD_DEFAULT = ( + f"NOTE this run could not read {ROOT}'s visibility from GitHub, so a leg " + "this run creates is created private, the default, and does not " + f"inherit {ROOT}'s; a leg it reuses keeps its own. Pass --visibility to " + f"choose one before any leg exists; `gh repo view {ROOT} --json " + f"visibility` says which {ROOT} is.") +ROOT_UNREAD_PUBLIC = ( + f"NOTE this run could not read {ROOT}'s visibility from GitHub, so it " + "cannot say whether --visibility public agrees with it. A leg this run " + "creates is created public, as you asked; a leg it reuses keeps its " + f"own. `gh repo view {ROOT} --json visibility` says which {ROOT} is.") +#: What a real run says at its end of a reused root it could not read. +ROOT_UNREAD_AT_THE_END = ( + f"NOTE this run could not read {ROOT}'s visibility from GitHub, so it " + "cannot say whether the Free-plan secrets hint applies.") +#: What the plan says under its `gh repo create` row. +NOT_FOUND_LINES = ( + " with this token: a private repository it cannot see is " + "not found\n" + " either, and `gh repo create` then fails for it\n") + + +def unread_legs(*legs: str) -> str: + """The NOTE a real run prints for the reused legs it could not read.""" + if len(legs) == 1: + whose, them, who, which = (f"{legs[0]}'s visibility", "it", + "if it is", "that leg") + asks = f"`gh repo view {legs[0]} --json visibility` says which it is." + else: + whose, them, who, which = ( + f"the visibility of {legs[0]} and {legs[1]}", "them", + "for each that is", "the legs") + asks = (f"`gh repo view {legs[0]} --json visibility` and `gh repo " + f"view {legs[1]} --json visibility` say which each is.") + return (f"NOTE this run could not read {whose} from GitHub, so it cannot " + f"say whether {ROOT} needs a way to read {them}: {who} private or " + f"internal, give {ROOT} a GitHub App (SHAPE_LEGS_APP_ID + " + "SHAPE_LEGS_APP_PRIVATE_KEY, preferred) or a SHAPE_LEGS_TOKEN PAT " + f"(contents:read on {which}, fallback), or the `validate` check " + f"cannot check it out. {asks}") + + +def calls_of(log) -> list: + return log.read_text(encoding="utf-8").splitlines() + + +# --- U167: a root it could not read, said before the plan, and read once ---- + +@WINDOWS_SKIP +@pytest.mark.parametrize("flags, note", [ + ((), ROOT_UNREAD_DEFAULT), + (("--visibility", "public"), ROOT_UNREAD_PUBLIC), +], ids=["default", "chosen"]) +def test_a_root_it_could_not_read_is_said_before_the_plan(tmp_path, flags, + note): + """L2-96: a dry run over a reused root whose visibility read fails says + so before the plan, while `--visibility` can still choose what the legs + it creates are, and asks the root's visibility once. On main it printed + nothing about the read and planned the legs private.""" + env, log = fake_gh(tmp_path, {PROJECT: "PRIVATE"}, + visibility_errors={PROJECT: BAD_GATEWAY}) + result = on_github(tmp_path, "--reuse-empty-repo", "--dry-run", *flags, + env=env) + assert result.returncode == 0, result.stderr + result.stdout + assert DRY_RUN_DONE in result.stdout + assert result.stdout.count(note) == 1, result.stdout + assert result.stdout.index(note) < result.stdout.index(PLAN_STARTS) + assert calls_of(log).count(VISIBILITY_ASKED) == 1, calls_of(log) + + +@WINDOWS_SKIP +@pytest.mark.parametrize("repos", [{}, {PROJECT: "PRIVATE"}], + ids=["not-found", "read"]) +def test_a_root_gh_does_not_find_or_reads_is_not_said_to_be_unread( + tmp_path, repos): + """The controls: a root gh does not find is the ordinary + `--reuse-empty-repo` run, which creates it, and one it reads is + inherited as before; neither is said to be unread.""" + env, _log = fake_gh(tmp_path, repos) + result = on_github(tmp_path, "--reuse-empty-repo", "--dry-run", env=env) + assert result.returncode == 0, result.stderr + result.stdout + assert "could not read" not in result.stdout + result.stderr + assert (f"NOTE {ROOT} is already private on GitHub" in result.stdout) == ( + bool(repos)), result.stdout + + +@WINDOWS_SKIP +@pytest.mark.parametrize("errors", [None, {PROJECT: BAD_GATEWAY}], + ids=["read", "unread"]) +def test_a_real_run_reads_the_reused_root_once(tmp_path, errors): + """L2-96: the real run asks the root's visibility once, before the plan, + and its last NOTE goes by that read: the root's visibility for the + Free-plan hint, or `ROOT_VISIBILITY_UNREAD`. On main `_create_remotes` + asked again, and the two reads could disagree.""" + env, log = fake_gh(tmp_path, {PROJECT: "PRIVATE"}, plan="free", + visibility_errors=errors) + result = on_github(tmp_path, "--reuse-empty-repo", env=env) + assert result.returncode == 0, result.stderr + result.stdout + assert calls_of(log).count(VISIBILITY_ASKED) == 1, calls_of(log) + if errors: + assert ROOT_UNREAD_DEFAULT in result.stdout, result.stdout + assert ROOT_UNREAD_AT_THE_END in result.stdout, result.stdout + assert f"NOTE {ORG} is on the GitHub FREE plan" not in result.stdout + else: + assert f"{ROOT} is private, so org-level" in result.stdout + assert "could not read" not in result.stdout + + +# --- U163: a reused leg it could not read ----------------------------------- + +@WINDOWS_SKIP +@pytest.mark.parametrize("repos, flags, plan, unread, said, hint", [ + # L2-92's first run: everything private, a public run, a Free plan. On + # main: no leg NOTE and no hint, though the root is private on Free. + ({PROJECT: "PRIVATE", f"{PROJECT}-spec": "PRIVATE", + f"{PROJECT}-code": "PRIVATE"}, ("--visibility", "public"), "free", + ("spec", "code"), (), True), + # Its second: everything public, a private run. On main the NOTE called + # both legs private. + ({PROJECT: "PUBLIC", f"{PROJECT}-spec": "PUBLIC", + f"{PROJECT}-code": "PUBLIC"}, ("--visibility", "private"), "free", + ("spec", "code"), (), False), + # One leg unread, the other read private: the one read is named at its + # visibility, the other said to be unread, and a paid plan has no hint. + ({PROJECT: "PRIVATE", f"{PROJECT}-spec": "PUBLIC", + f"{PROJECT}-code": "PRIVATE"}, (), "team", ("spec",), + (f"NOTE {CODE} is private: give {ROOT} a way to read it",), False), +], ids=["private-legs-under-a-public-run", "public-legs-under-a-private-run", + "one-leg-unread"]) +def test_a_reused_leg_it_could_not_read_is_said_to_be_unread( + tmp_path, repos, flags, plan, unread, said, hint): + """L2-92: a reused leg whose visibility GitHub does not give is said to + be unread, by name, with what its answer would mean, and the legs it + could read are named as before; the Free-plan hint goes by the root. On + main the run's visibility stood in for each such leg.""" + legs = {"spec": f"{PROJECT}-spec", "code": f"{PROJECT}-code"} + env, _log = fake_gh(tmp_path, repos, plan=plan, visibility_errors={ + legs[role]: BAD_GATEWAY for role in unread}) + result = on_github(tmp_path, "--reuse-empty-repo", *flags, env=env) + assert result.returncode == 0, result.stderr + result.stdout + printed = result.stdout + result.stderr + assert printed.count(unread_legs(*(f"{ORG}/{legs[role]}" + for role in unread))) == 1, printed + notes = [line for line in result.stdout.splitlines() + if line.startswith("NOTE ") and ": give " in line] + assert len(notes) == len(said), notes + for line in said: + assert line in printed, printed + assert (f"{ROOT} is private, so org-level" in printed) == hint, printed + for role in unread: + assert f"WARNING {ORG}/{legs[role]} is reused" not in printed, printed + + +# --- U010 and U060: the plan's rows ----------------------------------------- + +def scaffold_on_disk(tmp_path, *extra: str): + return run_script(SCAFFOLD, "--org", ORG, "--project", PROJECT, + "--elected-by", "Test Human", "--dry-run", + "--local-remote-dir", str(tmp_path / "remotes"), + "--work-dir", str(tmp_path / "work"), *extra) + + +def init_run(tmp_path, *extra: str, env: dict | None = None): + return run_script(FAMILY, "init", "--org", ORG, "--family", HOLDER, + "--created-by", "Test Human", "--dry-run", + "--work-dir", str(tmp_path / "work"), *extra, env=env) + + +@pytest.mark.parametrize("tool", ["scaffold", "family"]) +@pytest.mark.parametrize("flags, branch, visibility", [ + ((), "main", "private"), + (("--tracking-branch", "trunk", "--visibility", "public"), "trunk", + "public"), +], ids=["defaults", "chosen"]) +def test_the_plan_on_disk_names_the_branch_and_the_visibility( + tmp_path, tool, flags, branch, visibility): + """AT-scaffold N3: under `--local-remote-dir` the plan names the tracking + branch the repositories are created on and the visibility the manifest + records, which a bare repository on disk does not have; on main it + named neither.""" + if tool == "scaffold": + result = scaffold_on_disk(tmp_path, *flags) + manifest, created = "project.yaml", "each of the three is created on" + remotes = "remotes bare repositories on disk (no network)\n" + else: + result = init_run(tmp_path, "--local-remote-dir", + str(tmp_path / "remotes"), *flags) + manifest, created = "family.yaml", "the holder is created on" + remotes = "remotes a bare repository on disk (no network)\n" + assert result.returncode == 0, result.stderr + result.stdout + out = result.stdout + row = (f"visibility {visibility}, which {manifest} records; a bare " + "repository on disk has none\n") + assert out.count(row) == 1, out + assert out.index(row) < out.index(remotes), out + assert out.count(f"branch {branch}, the tracking branch " + f"{created}\n") == 1, out + assert "with this token" not in out, out + + +@WINDOWS_SKIP +@pytest.mark.parametrize("tool", ["scaffold", "family"]) +def test_the_plan_on_github_says_not_found_is_what_this_token_sees( + tmp_path, tool): + """L2-61 and AT-scaffold N3 against GitHub: the `gh repo create` row + says it creates what gh does not find, and that a private repository + this token cannot see is not found either, so its create then fails; + the branch is named; the visibility is the `gh repo create` flag, and + no row of its own.""" + env, _log = fake_gh(tmp_path, {}) + if tool == "scaffold": + result = on_github(tmp_path, "--dry-run", "--tracking-branch", + "trunk", env=env) + row = ("remotes gh repo create --private, for each of the three " + "gh does not find\n") + branch = "branch trunk, the tracking branch each of the three " + else: + result = init_run(tmp_path, "--tracking-branch", "trunk", env=env) + row = ("remotes gh repo create --private, if gh does not find " + f"{ORG}/{HOLDER}\n") + branch = "branch trunk, the tracking branch the holder " + assert result.returncode == 0, result.stderr + result.stdout + assert DRY_RUN_DONE in result.stdout + assert result.stdout.count(row + NOT_FOUND_LINES) == 1, result.stdout + assert result.stdout.count(branch) == 1, result.stdout + assert "visibility " not in result.stdout, result.stdout From f3802d6fd452ffbd7deb9b92f4f286342ba2c8ab Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sun, 18 Oct 2026 19:05:38 +0000 Subject: [PATCH 2/2] scaffold-project.py and `family.py init`: the NOTE for two reused legs it could not read ends "cannot check them out", `init`'s members' NOTE names a member that is, or has a leg that is, private or internal, and a reused leg gh does not find is pinned as unread, so it is no longer taken at the run's visibility (#913) Fix round 1 on RV969's review of #969. - SHOULD 1: `_private_leg_note` returns first for a `--local-remote-dir` run and drops its three `not local and` guards, which brings its cognitive complexity under SonarCloud's 15. Nothing it prints changes. - SHOULD 2: `test_a_reused_leg_it_could_not_read_is_said_to_be_unread` has a row whose leg read is gh's not-found answer, which fails where `_reused_leg_visibility` takes that read at the run's visibility. - NIT 1: the same test counts the unread-leg NOTE in stdout alone, so the NOTE on stderr is caught. - NIT 2: a 115-character docstring line in test_free_plan_hint_reads.py is wrapped. - NIT 4: `init`'s members' NOTE names "a member you add that is, or has a leg that is, private or internal": a public member with private legs needs the credential too, since the holder's `validate` runs `--recursive`. - NIT 5: `_unread_legs_note` says "cannot check them out" for two legs. Co-Authored-By: Claude Sonnet 5.5 --- scaffold-project.py | 10 ++++---- scripts/family.py | 11 +++++---- tests/test_free_plan_hint_reads.py | 16 +++++++------ tests/test_plan_visibility_notes.py | 37 ++++++++++++++++++++--------- 4 files changed, 47 insertions(+), 27 deletions(-) diff --git a/scaffold-project.py b/scaffold-project.py index 30b05f62..b3af3609 100755 --- a/scaffold-project.py +++ b/scaffold-project.py @@ -1641,10 +1641,12 @@ def _private_leg_note(args, repositories: dict, local: bool, named as before, and the Free-plan note follows either NOTE, since an unread leg may be closed. """ + if local: + return closed = [role for role in ("spec", "code") if standing[role] in ("private", "internal")] unread = [role for role in ("spec", "code") if standing[role] is None] - if not local and closed: + if closed: both = len(closed) == 2 who = " and ".join(repositories[role] for role in closed) are = "are" if both else "is" @@ -1658,9 +1660,9 @@ def _private_leg_note(args, repositories: dict, local: bool, "SHAPE_LEGS_APP_PRIVATE_KEY, preferred) or a SHAPE_LEGS_TOKEN " f"PAT (contents:read on {which}, fallback) — or the `validate` " f"check cannot check {them} out.") - if not local and unread: + if unread: print(_unread_legs_note(repositories, unread)) - if not local and (closed or unread): + if closed or unread: note = free_plan_secrets_note(args.org, repositories["assembly"], standing["assembly"]) if note: @@ -1688,7 +1690,7 @@ def _unread_legs_note(repositories: dict, unread: list) -> str: f"or internal, give {root} a GitHub App (SHAPE_LEGS_APP_ID + " "SHAPE_LEGS_APP_PRIVATE_KEY, preferred) or a SHAPE_LEGS_TOKEN " f"PAT (contents:read on {which}, fallback), or the `validate` " - f"check cannot check it out. {asks}") + f"check cannot check {them} out. {asks}") def _take_held_seed(work: Path, url: str, seed: str, tracking: str) -> str: diff --git a/scripts/family.py b/scripts/family.py index c2dd8ac5..0e5c8a10 100755 --- a/scripts/family.py +++ b/scripts/family.py @@ -2982,11 +2982,12 @@ def _finish_holder_remote(args, repository: str, topic: str, local: bool, visibility = repository_visibility(repository) except OSError: visibility = None - print("NOTE a private or internal member you add needs a way for " - f"{repository}, public or not, to read it — a GitHub App " - "(SHAPE_LEGS_APP_ID + SHAPE_LEGS_APP_PRIVATE_KEY, preferred) or a " - "SHAPE_LEGS_TOKEN PAT with contents:read on the members AND their " - "legs — or the `validate` check cannot check that member out.") + print("NOTE a member you add that is, or has a leg that is, private or " + f"internal needs a way for {repository}, public or not, to read " + "it — a GitHub App (SHAPE_LEGS_APP_ID + SHAPE_LEGS_APP_PRIVATE_KEY, " + "preferred) or a SHAPE_LEGS_TOKEN PAT with contents:read on the " + "members AND their legs — or the `validate` check cannot check " + "that member out.") note = free_plan_secrets_note(args.org, repository, visibility) if note: print(note) diff --git a/tests/test_free_plan_hint_reads.py b/tests/test_free_plan_hint_reads.py index dcc4c107..3df3d778 100644 --- a/tests/test_free_plan_hint_reads.py +++ b/tests/test_free_plan_hint_reads.py @@ -196,11 +196,12 @@ def test_each_tool_says_what_github_said_of_the_root_and_its_plan( def member_note(holder: str) -> str: """The NOTE `init` prints for every holder on GitHub (#913, L2-95): about the members a person adds, whatever the holder's own visibility.""" - return ("NOTE a private or internal member you add needs a way for " - f"{holder}, public or not, to read it — a GitHub App " - "(SHAPE_LEGS_APP_ID + SHAPE_LEGS_APP_PRIVATE_KEY, preferred) or a " - "SHAPE_LEGS_TOKEN PAT with contents:read on the members AND their " - "legs — or the `validate` check cannot check that member out.") + return ("NOTE a member you add that is, or has a leg that is, private or " + f"internal needs a way for {holder}, public or not, to read " + "it — a GitHub App (SHAPE_LEGS_APP_ID + " + "SHAPE_LEGS_APP_PRIVATE_KEY, preferred) or a SHAPE_LEGS_TOKEN " + "PAT with contents:read on the members AND their legs — or the " + "`validate` check cannot check that member out.") @WINDOWS_SKIP @@ -260,8 +261,9 @@ def test_init_takes_a_holder_gh_cannot_be_asked_about_as_unread( """A reused holder whose visibility read cannot start `gh` (an `OSError`) is unread, as a failed read is: `ROOT_VISIBILITY_UNREAD`, and no hint at the run's `--visibility`, which is not the holder's (#660's F8); the - members' NOTE, which goes by no visibility (#913). In process, so it runs on every platform: the topic edit and - the plan are answered here, and nothing reaches `gh`.""" + members' NOTE, which goes by no visibility (#913). In process, so it runs + on every platform: the topic edit and the plan are answered here, and + nothing reaches `gh`.""" family = load_without_bytecode(FAMILY, "family_free_plan_hint_reads") edits: list = [] monkeypatch.setattr(family, "run", diff --git a/tests/test_plan_visibility_notes.py b/tests/test_plan_visibility_notes.py index 92bc0c7b..95d7206c 100644 --- a/tests/test_plan_visibility_notes.py +++ b/tests/test_plan_visibility_notes.py @@ -41,6 +41,9 @@ ROOT = f"{ORG}/{PROJECT}" SPEC, CODE = f"{ROOT}-spec", f"{ROOT}-code" BAD_GATEWAY = "HTTP 502: Bad Gateway (https://api.github.com/graphql)" +#: gh's answer for a repository it does not find, which is not an answer +#: about its visibility either. +NOT_FOUND = "GraphQL: Could not resolve to a Repository" PLAN_STARTS = "project " DRY_RUN_DONE = "--dry-run: nothing was created." VISIBILITY_ASKED = f"repo view {ROOT} --json visibility" @@ -86,7 +89,7 @@ def unread_legs(*legs: str) -> str: f"internal, give {ROOT} a GitHub App (SHAPE_LEGS_APP_ID + " "SHAPE_LEGS_APP_PRIVATE_KEY, preferred) or a SHAPE_LEGS_TOKEN PAT " f"(contents:read on {which}, fallback), or the `validate` check " - f"cannot check it out. {asks}") + f"cannot check {them} out. {asks}") def calls_of(log) -> list: @@ -158,38 +161,50 @@ def test_a_real_run_reads_the_reused_root_once(tmp_path, errors): # --- U163: a reused leg it could not read ----------------------------------- @WINDOWS_SKIP -@pytest.mark.parametrize("repos, flags, plan, unread, said, hint", [ +@pytest.mark.parametrize("repos, flags, plan, unread, said, hint, error", [ # L2-92's first run: everything private, a public run, a Free plan. On # main: no leg NOTE and no hint, though the root is private on Free. ({PROJECT: "PRIVATE", f"{PROJECT}-spec": "PRIVATE", f"{PROJECT}-code": "PRIVATE"}, ("--visibility", "public"), "free", - ("spec", "code"), (), True), + ("spec", "code"), (), True, BAD_GATEWAY), # Its second: everything public, a private run. On main the NOTE called # both legs private. ({PROJECT: "PUBLIC", f"{PROJECT}-spec": "PUBLIC", f"{PROJECT}-code": "PUBLIC"}, ("--visibility", "private"), "free", - ("spec", "code"), (), False), + ("spec", "code"), (), False, BAD_GATEWAY), # One leg unread, the other read private: the one read is named at its # visibility, the other said to be unread, and a paid plan has no hint. ({PROJECT: "PRIVATE", f"{PROJECT}-spec": "PUBLIC", f"{PROJECT}-code": "PRIVATE"}, (), "team", ("spec",), - (f"NOTE {CODE} is private: give {ROOT} a way to read it",), False), + (f"NOTE {CODE} is private: give {ROOT} a way to read it",), False, + BAD_GATEWAY), + # The same, where gh's answer for the leg is not-found rather than a + # failure: it is no answer about the leg's visibility either, so the run + # does not take the leg at its own. A leg found a moment before and not + # now, as a token's access changing mid-run, reads so. + ({PROJECT: "PRIVATE", f"{PROJECT}-spec": "PRIVATE", + f"{PROJECT}-code": "PRIVATE"}, ("--visibility", "public"), "team", + ("spec",), + (f"NOTE {CODE} is private: give {ROOT} a way to read it",), False, + NOT_FOUND), ], ids=["private-legs-under-a-public-run", "public-legs-under-a-private-run", - "one-leg-unread"]) + "one-leg-unread", "one-leg-not-found"]) def test_a_reused_leg_it_could_not_read_is_said_to_be_unread( - tmp_path, repos, flags, plan, unread, said, hint): + tmp_path, repos, flags, plan, unread, said, hint, error): """L2-92: a reused leg whose visibility GitHub does not give is said to be unread, by name, with what its answer would mean, and the legs it could read are named as before; the Free-plan hint goes by the root. On - main the run's visibility stood in for each such leg.""" + main the run's visibility stood in for each such leg. A leg gh does not + find (`error` the not-found answer) is unread as one whose read failed + is, and not taken at the run's visibility.""" legs = {"spec": f"{PROJECT}-spec", "code": f"{PROJECT}-code"} env, _log = fake_gh(tmp_path, repos, plan=plan, visibility_errors={ - legs[role]: BAD_GATEWAY for role in unread}) + legs[role]: error for role in unread}) result = on_github(tmp_path, "--reuse-empty-repo", *flags, env=env) assert result.returncode == 0, result.stderr + result.stdout printed = result.stdout + result.stderr - assert printed.count(unread_legs(*(f"{ORG}/{legs[role]}" - for role in unread))) == 1, printed + unread_note = unread_legs(*(f"{ORG}/{legs[role]}" for role in unread)) + assert result.stdout.count(unread_note) == 1, printed notes = [line for line in result.stdout.splitlines() if line.startswith("NOTE ") and ": give " in line] assert len(notes) == len(said), notes