diff --git a/.github/workflows/kanban-archive.yml b/.github/workflows/kanban-archive.yml index 25f3f16..e1b9e1d 100644 --- a/.github/workflows/kanban-archive.yml +++ b/.github/workflows/kanban-archive.yml @@ -267,6 +267,23 @@ jobs: # under suspicion here -- reading the baseline with it would put both # sides of the comparison back inside one view. GH_TOKEN: ${{ github.token }} + # `gh run download` TAKES NO --repo AND THIS JOB HAS NO CHECKOUT, so it + # resolved the repository from the git remote, found no `.git`, and died + # before making any API call (backend#2802). That is why the artifacts + # LISTING above worked -- it puts the repo in the URL path -- while the + # fetch failed, and the run went red having archived successfully. + # + # Reproduced outside Actions, gh 2.98.0: + # $ cd /tmp/empty && gh run download -n board-baseline + # failed to run git: fatal: not a git repository + # $ GH_REPO=tracebloc/.github gh run download -n board-baseline + # $ cat prev/board.total -> 574 + # + # `gh` does NOT read `GITHUB_REPOSITORY` (tested), so being on a runner + # does not supply it. GH_REPO is the variable it does read, and it is + # cheaper and narrower than adding a checkout to a job that needs no + # source. + GH_REPO: ${{ github.repository }} run: | set -uo pipefail : > prev.total @@ -313,7 +330,27 @@ jobs: else rid=$(printf '%s' "$arts" \ | jq -sr '[.[] | select(.expired == false)] | sort_by(.created_at) | last | .workflow_run.id') - if gh run download "$rid" -n board-baseline -D prev >/dev/null 2>&1 && [ -s prev/board.total ]; then + # KEEP THE DOWNLOAD'S STDERR. `>/dev/null 2>&1` is the reason this + # failure was diagnosable only by reproducing it by hand: the step + # reported "could not be downloaded" and threw away the one line + # that said why ("failed to run git: fatal: not a git repository"). + # Stdout still goes to /dev/null -- it is progress noise -- but the + # error text is captured and echoed by the refusal below. + # STDERR TO A SCRATCH FILE, PROMOTED TO prev.error ONLY ON FAILURE + # (Bugbot, High -- on my own fix). Writing straight into prev.error + # couples the two: the assert step at the bottom treats ANY non-empty + # prev.error as an unreadable baseline (`if [ -s prev.error ]`), so a + # SUCCESSFUL download that printed anything at all would mark the + # baseline unreadable and keep the job red after a good archive -- + # the exact failure this PR fixes, arriving from the other side. + # + # MEASURED, and stated honestly: `gh run download` writes 0 bytes to + # stderr on success on a non-TTY (tested, gh 2.98.0), so the finding + # as described does not reproduce today. The COUPLING is real + # regardless, and one deprecation notice on stderr in a future gh is + # all it would take. Decoupled unconditionally because it costs + # nothing: only the failure branch can now populate prev.error. + if gh run download "$rid" -n board-baseline -D prev >/dev/null 2>prev.stderr && [ -s prev/board.total ]; then cp prev/board.total prev.total echo "Baseline: run $rid saw $(cat prev.total) item(s)." # IS THIS JOB THE ONLY ARCHIVER SINCE? It is not, and assuming @@ -353,7 +390,14 @@ jobs: # this is a broken read, not an empty history. Refusing is the # whole point of having asked the two questions separately. # selftest:unreadable-path - printf 'a live board-baseline artifact exists (run %s) but could not be downloaded\n' "$rid" > prev.error + # APPEND, DO NOT CLOBBER. `gh run download`'s stderr is already in + # prev.error at this point, and it holds the only line that says + # WHY -- overwriting it with this summary is what made the last + # five red runs undiagnosable from their own logs. The summary + # goes first because it names the run; the tool's own words follow. + reason=$(tr '\n' ' ' < prev.stderr 2>/dev/null | cut -c1-300) + printf 'a live board-baseline artifact exists (run %s) but could not be downloaded. gh said: %s\n' \ + "$rid" "${reason:-}" > prev.error echo "::warning::the baseline exists but could not be read; the assert step will refuse." fi fi diff --git a/scripts/branch_owner.py b/scripts/branch_owner.py index bdde8e7..a602ed1 100644 --- a/scripts/branch_owner.py +++ b/scripts/branch_owner.py @@ -57,15 +57,48 @@ one that says "cannot tell": it looks like coverage. The first-commit author is derivable from the clone for every branch, forever, and is named for what it is. +THE PULL-REQUEST LIST IS READ TO THE END, NOT TO A CEILING (backend#2972) +------------------------------------------------------------------------ +This used to ask `gh pr list --limit 1000` and refuse when the answer came back +at the cap. `tracebloc/backend` has 1418 pull requests, so the tool refused on the +org's LARGEST repo -- the one it is most needed on, carrying 62 of the org's 99 +stale branches -- and every one of its 108 branches came back `unattributable` +when a complete list attributes 102 of them. + +A bigger number would only move the date this happens again. So the read pages to +the end (`gh api graphql --paginate`) and asks the repository for its OWN count on +the same connection, and "did the read finish?" is then a comparison of two +measured numbers rather than a threshold somebody picked: + + rows read == pullRequests.totalCount -> the list is complete + anything else -> refuse, naming both numbers + +THE CURSOR VARIABLE MUST BE NAMED `$endCursor`, EXACTLY. `gh --paginate` injects +the next page's cursor into a variable of that name and no other. Misname it and +gh re-requests page 1 forever; the secondary rate limit that loop trips is +invisible to `gh api rate_limit`, so the failure does not even look like one. The +name is asserted by the selftest against the query text rather than trusted. + FAIL CLOSED, AND SAY SO ----------------------- "No PR was found for this branch" is only a fact when the PR list was actually -read, and completely. If `gh` is missing, unauthenticated, failing, or returned a -list that hit its own `--limit` cap, then absence proves nothing -- and falling -through to the commit-author path in that state is precisely the misattribution -this module exists to prevent, arrived at from a clean read of the wrong thing. -So a PR-list problem makes EVERY branch `unattributable`, before any other -evidence is weighed. Same posture, and the same reasoning, as `scripts/git-reap`. +read, and completely. If `gh` is missing, unauthenticated, failing, or came back +with fewer rows than the repository says it has, then absence proves nothing -- +and falling through to the commit-author path in that state is precisely the +misattribution this module exists to prevent, arrived at from a clean read of the +wrong thing. So a PR-list problem makes EVERY branch `unattributable`, before any +other evidence is weighed. Same posture, and the same reasoning, as +`scripts/git-reap`. + +AND A REFUSAL MUST NOT WEAR THE SHAPE OF DATA. `108 branch(es): 0 attributed, 108 +unattributable` is also what a genuine "nobody can be named in this repo" answer +looks like, so the two were indistinguishable and a reader who trusted the tool +concluded that every branch in the largest repo was unowned. An INCOMPLETE +pull-request list is therefore not reported as N unattributable rows at all: +`main` names the repo, both counts and the shortfall, prints no rows, and exits +non-zero, so no caller can read it as an inventory. The other PR-list problems +still print rows -- each saying why it was refused -- and the summary line now +carries the one reason with them. Ambiguity is also a refusal, not a tie-break. Two PRs for one head name by two different people -- branch names get reused, `fix/typo` twice, months apart -- is @@ -112,10 +145,44 @@ # caller that forgets to check it gets nothing rather than a plausible name. UNATTRIBUTABLE = "" -# `gh pr list` truncates at --limit SILENTLY. A miss against a partial window is -# not an absence, and the old branches this tool is for sort out of a newest-first -# window first. Reaching the cap is therefore reported as a problem, not a result. -PR_LIMIT = 1000 +# GitHub's own maximum for a GraphQL connection page. Not a number to tune: `first:` +# above 100 is refused by the API, so this is simply the largest page the read can +# ask for, and the page COUNT is whatever the repo's size makes it. +PAGE_SIZE = 100 + +# The prefix on the one problem string that means "the list is SHORT -- fewer rows +# came back than the repository says it has". A caller cannot branch on an English +# sentence, and `main` has to tell THIS refusal apart from a dead network to answer +# differently, so the marker is a single constant that the seam writes and `main` +# matches rather than a phrase each end spells for itself. +INCOMPLETE_PR_LIST = "REFUSING: incomplete pull-request list --" + +# ONE QUERY, PAGED TO THE END, WITH THE REPOSITORY'S OWN COUNT BESIDE THE ROWS. +# +# `totalCount` sits on the SAME connection as `nodes`, so it counts exactly the set +# the pages walk -- which is what lets `pull_requests` derive "the read finished" +# instead of asserting it. Unfiltered, like the `--state all` it replaces. +# +# THE CURSOR VARIABLE IS `$endCursor` AND THE NAME IS LOAD-BEARING: `gh --paginate` +# injects the next cursor into that name and no other, and a misnamed one re-reads +# page 1 forever. The field names are also chosen to match `gh pr list --json +# number,author,headRefName,headRefOid,state,createdAt` one-for-one, so the rows the +# rule receives -- and any caller importing `attribute` -- keep the same shape and +# the same OPEN/CLOSED/MERGED vocabulary. +# +# MEASURED, 2026-09-01: 4 pages / 386 rows for `.github`, 15 pages / 1418 rows for +# `backend`, each run ending with rows == totalCount and no duplicate numbers. +PR_QUERY = """ +query($owner: String!, $name: String!, $endCursor: String) { + repository(owner: $owner, name: $name) { + pullRequests(first: %d, after: $endCursor) { + totalCount + pageInfo { hasNextPage endCursor } + nodes { number author { login } headRefName headRefOid state createdAt } + } + } +} +""" % PAGE_SIZE SIGNALS = ("pr-exact", "pr", "first-commit", "unattributable") @@ -267,29 +334,117 @@ def _run(args: list) -> "tuple[int, str]": return proc.returncode, proc.stdout.strip() +def repo_identity(repo: str = "") -> "tuple[str, str, str]": + """(owner, name, problem). The two halves the GraphQL read needs as variables. + + `gh pr list` accepted `owner/name` whole, or resolved the clone itself. GraphQL + takes the halves separately, so the resolution gh did implicitly happens here -- + and, being a read that can fail, it refuses out loud rather than guessing at a + repo. Splitting is deliberately strict: `owner/name/extra` is not a repository, + and quietly reading a DIFFERENT repo's pull requests is a misattribution of the + same shape as everything else this module refuses. + """ + if repo: + owner, slash, name = repo.partition("/") + if not owner or not slash or not name or "/" in name: + return "", "", (f"{repo!r} is not owner/name, so the pull-request list " + "could not be asked for") + return owner, name, "" + rc, out = _run(["gh", "repo", "view", "--json", "nameWithOwner", + "--jq", ".nameWithOwner"]) + if rc != 0 or not out: + return "", "", ("this clone's repository could not be identified -- no gh, " + "no auth, no network, or not a repo. Absence from a list " + "that was never read proves nothing") + owner, slash, name = out.partition("/") + if not owner or not slash or not name: + return "", "", (f"gh named this clone {out!r}, which is not owner/name") + return owner, name, "" + + def pull_requests(repo: str = "") -> "tuple[dict, str]": """(head ref -> [PR rows], problem). A non-empty problem refuses everything. - One query per repo, never one per branch: a per-branch call burns the API - rate limit on a repo with hundreds of branches. + THE WHOLE LIST OR A REFUSAL, NEVER A WINDOW (backend#2972). One paged query per + repo, never one per branch -- a per-branch call burns the API rate limit on a + repo with hundreds of branches -- read to the end by `gh --paginate` and then + CHECKED against the repository's own `totalCount`. Three ways the read can be + unsound, and each is a refusal rather than a short list: + + * fewer rows than `totalCount` -- pagination stopped early + * duplicate pull-request numbers -- the same page came back twice, which is + what a broken cursor looks like on the near side of an infinite loop + * `totalCount` disagreeing between pages -- the repo changed under the read, + so no single answer is a fact about it """ - args = ["gh", "pr", "list", "--state", "all", "--limit", str(PR_LIMIT), - "--json", "number,author,headRefName,headRefOid,state,createdAt"] - if repo: - args += ["--repo", repo] - rc, out = _run(args) + owner, name, problem = repo_identity(repo) + if problem: + return {}, problem + where = f"{owner}/{name}" + # `-f`, NEVER `-F`: `-F` types its value, so a numeric repo name arrives as an + # int against a `String!` variable and the query is rejected outright -- + # measured, `-F name=123` comes back "Could not coerce value 123 to String" + # while `-f name=123` resolves and reports the repo simply does not exist. + rc, out = _run(["gh", "api", "graphql", "--paginate", "--slurp", + "-f", f"owner={owner}", "-f", f"name={name}", + "-f", f"query={PR_QUERY}"]) if rc != 0 or not out: - return {}, ("`gh pr list` failed -- no gh, no auth, or no network. " + return {}, ("`gh api graphql` failed -- no gh, no auth, or no network. " "Absence from a list that was never read proves nothing") try: - rows = json.loads(out) + pages = json.loads(out) except json.JSONDecodeError as exc: return {}, f"the pull-request list did not parse as JSON ({exc})" - if not isinstance(rows, list): - return {}, f"the pull-request list is a {type(rows).__name__}, not a list" - if len(rows) >= PR_LIMIT: - return {}, (f"the pull-request list hit its --limit {PR_LIMIT} cap, so a " - "branch missing from it may simply be past the window") + # `--slurp` wraps the pages in ONE array, so a bare object here means the shape + # is not what was asked for -- not that there is one page. + if not isinstance(pages, list): + return {}, f"the pull-request list is a {type(pages).__name__}, not a list" + + rows: list = [] + totals = set() + for page in pages: + page = page if isinstance(page, dict) else {} + # A GRAPHQL ERROR IS A 200. `errors[]` beside a null `data` is how this API + # reports a bad field, a missing repo or a permissions problem, and reading + # only the exit code turns all three into "no pull requests". + errors = page.get("errors") + if errors: + first = errors[0] if isinstance(errors, list) and errors else errors + said = (first or {}).get("message") if isinstance(first, dict) else first + return {}, (f"the pull-request query returned an error for {where} " + f"({said!r}), so nothing was read") + conn = (((page.get("data") or {}).get("repository") or {}) + .get("pullRequests")) + if not isinstance(conn, dict): + return {}, (f"a page of {where}'s pull-request list carries no " + "repository.pullRequests, so the read cannot be trusted") + nodes = conn.get("nodes") + if not isinstance(nodes, list): + return {}, (f"a page's `nodes` is a {type(nodes).__name__}, not a list, " + "so the read cannot be trusted") + rows += nodes + totals.add(conn.get("totalCount")) + + if len(totals) != 1 or not isinstance(next(iter(totals)), int): + return {}, (f"{INCOMPLETE_PR_LIST} {where}'s pull-request count was " + f"{sorted(totals, key=repr)} across the pages read, so there is " + "no single number to check the list against -- either the repo " + "changed mid-read or the count was never returned") + total = next(iter(totals)) + + numbers = [row.get("number") for row in rows if isinstance(row, dict)] + if len(set(numbers)) != len(numbers): + return {}, (f"{INCOMPLETE_PR_LIST} {where} returned {len(numbers)} rows but " + f"only {len(set(numbers))} distinct pull requests -- a page came " + "back twice, which is what a broken `$endCursor` looks like, so " + "the read is not a list of the repo's pull requests") + if len(rows) != total: + return {}, (f"{INCOMPLETE_PR_LIST} {where} reports {total} pull request(s) " + f"but the paged read returned {len(rows)} -- {abs(total - len(rows))} " + "unaccounted for, so a branch missing from this list may simply " + "never have been read. This is a refusal, not an " + "'unattributable' finding") + by_head: dict = {} for row in rows: by_head.setdefault(row.get("headRefName") or "", []).append(row) @@ -339,10 +494,24 @@ def default_branch(repo: str = "") -> "tuple[str, str]": whoever created this branch. That is a misattribution of the same shape as the tip-author bug, one layer down, so it is refused rather than used. """ - args = ["gh", "repo", "view", "--json", "defaultBranchRef", - "--jq", ".defaultBranchRef.name"] + # THE REPOSITORY IS POSITIONAL HERE, AND THAT IS MEASURED, NOT STYLE. + # `gh repo view` HAS NO `--repo` FLAG -- `gh repo view [] [flags]` -- + # and the flag form exits 1 without asking anything: + # + # $ gh repo view --json defaultBranchRef --repo tracebloc/backend + # unknown flag: --repo (2026-09-01) + # + # This built the flag form, so THE AUTHORITATIVE LOOKUP WAS UNREACHABLE ON EVERY + # `--repo` RUN -- including the `--repo tracebloc/backend` invocation the tool + # exists for. It fell through to `origin/HEAD`, a clone-time cache, and the + # first-commit signal was then withheld for every branch while the message said + # the remote "could not be confirmed". The remote was never asked; the command + # was malformed. The no-argument form still resolves from the clone, which is + # the only reason the bug stayed invisible (backend#2972). + args = ["gh", "repo", "view"] if repo: - args += ["--repo", repo] + args.append(repo) + args += ["--json", "defaultBranchRef", "--jq", ".defaultBranchRef.name"] rc, out = _run(args) if rc == 0 and out: # THE REMOTE'S ANSWER IS AUTHORITATIVE ABOUT THE NAME, NOT ABOUT THIS @@ -430,6 +599,14 @@ def main(argv: "list[str] | None" = None) -> int: if note: sys.stderr.write(f"branch_owner: {note}\n") + # AN INCOMPLETE PR LIST IS A REFUSAL, SO DO NOT SERVE IT AS ROWS (backend#2972). + # Every row below would be `unattributable` for this one reason, and the tally + # they add up to is character-for-character what a repo with genuinely + # unattributable branches prints. Nothing is printed and the exit code carries + # the refusal, so no caller can mistake a short read for the repo. + if problem.startswith(INCOMPLETE_PR_LIST): + return 2 + refs, refs_problem = remote_branches(default) if refs_problem: # REFUSE, do not report zero. Without the ref list there is no tip for any @@ -478,8 +655,13 @@ def main(argv: "list[str] | None" = None) -> int: print(f"{r['branch']}\t{r['owner']}\t{r['signal']}\t{r['why']}") unattributed = sum(1 for r in out if r["signal"] == "unattributable") + # SAY WHEN THE TALLY IS NOT A MEASUREMENT. The short-read refusal above never + # reaches this line, but the other PR-list problems do, and they refuse every + # row for one reason -- which a bare count hides just as completely. Carrying + # the reason here is what stops the same misreading on the paths that do print. + because = f" -- every row refused for one reason: {problem}" if problem else "" print(f"\n{len(out)} branch(es): {len(out) - unattributed} attributed, " - f"{unattributed} unattributable", file=sys.stderr) + f"{unattributed} unattributable{because}", file=sys.stderr) return 0 diff --git a/scripts/tests/archive-baseline-mutations.py b/scripts/tests/archive-baseline-mutations.py index e32465f..c5c3e0f 100755 --- a/scripts/tests/archive-baseline-mutations.py +++ b/scripts/tests/archive-baseline-mutations.py @@ -127,7 +127,14 @@ # smaller blast radius and no way to notice it. ( "one of the lookup failure paths stops marking itself unreadable", - " printf 'a live board-baseline artifact exists (run %s) but could not be downloaded\\n' \"$rid\" > prev.error", + # RE-ANCHORED for backend#2802. The old anchor was the single-line + # `printf ... > prev.error`; that branch now captures `gh run download`'s + # stderr first and APPENDS the summary to it, so the literal no longer + # exists. The harness caught this itself -- "anchor matched 0 times" -- + # rather than reporting 0 uncaught about a premise nobody typed, which is + # the behaviour that made the re-anchor necessary instead of optional. + " printf 'a live board-baseline artifact exists (run %s) but could not be downloaded. gh said: %s\\n' \\\n" + " \"$rid\" \"${reason:-}\" > prev.error", ' echo "could not download the baseline"', ), # The marker/write pairing that lets the suite DERIVE how many failure paths diff --git a/scripts/tests/archive-baseline-selftest.py b/scripts/tests/archive-baseline-selftest.py index 53299c0..4ed8d1b 100755 --- a/scripts/tests/archive-baseline-selftest.py +++ b/scripts/tests/archive-baseline-selftest.py @@ -405,6 +405,54 @@ def wiring_failures() -> list: "nothing -- this ticket's own defect" ) + # THE DOWNLOAD NEEDS A REPO, AND THIS JOB HAS NO CHECKOUT (backend#2802). + # `gh run download` takes no `--repo`, so without one it resolves the + # repository from the git remote, finds no `.git`, and dies BEFORE any API + # call -- which is why the artifacts LISTING succeeded (repo in the URL path) + # while the fetch failed, and five scheduled runs went red having archived + # successfully. Reproduced outside Actions on gh 2.98.0; `gh` does not read + # `GITHUB_REPOSITORY`, so being on a runner does not supply it. + recall = [s for s in steps if "gh run download" in str(s.get("run", ""))] + if len(recall) != 1: + bad.append(f"expected exactly one step running `gh run download`, found " + f"{len(recall)}; the baseline recall cannot be checked") + else: + renv = recall[0].get("env") or {} + if "github.repository" not in str(renv.get("GH_REPO", "")): + bad.append( + "the baseline-recall step does not set `GH_REPO`, so `gh run download` " + "has no repository to resolve and fails before any API call -- the run " + "goes red having archived successfully (backend#2802)" + ) + run = str(recall[0].get("run", "")) + # AND THE FAILURE MUST BE ABLE TO NAME ITSELF. `>/dev/null 2>&1` on the + # download is why the red runs were diagnosable only by reproducing them + # by hand: the step reported "could not be downloaded" and discarded the + # one line that said why. + if "2>&1" in run and "gh run download" in run.split("2>&1")[0][-120:]: + bad.append( + "the baseline download still discards its stderr, so a failure cannot " + "name its own cause in the log" + ) + # CAPTURED, AND NOT INTO THE SENTINEL (Bugbot High, on the first cut of + # this fix). The assert step treats ANY non-empty `prev.error` as an + # unreadable baseline, so capturing stderr straight into it would let a + # SUCCESSFUL download that printed anything mark the baseline unreadable and + # keep the job red after a good archive. Measured: gh writes 0 bytes to + # stderr on success on a non-TTY, so that does not reproduce today -- but + # the coupling is real and one future deprecation notice is all it takes. + if "2>prev.stderr" not in run.replace("2> prev.stderr", "2>prev.stderr"): + bad.append( + "the baseline download does not capture stderr into `prev.stderr`, so " + "the refusal below cannot carry the reason" + ) + if "2>prev.error" in run.replace("2> prev.error", "2>prev.error"): + bad.append( + "the baseline download writes stderr straight into `prev.error`, which " + "the assert step reads as 'baseline unreadable' -- a successful " + "download that prints anything would keep the job red" + ) + upload = [s for s in steps if "upload-artifact" in str(s.get("uses", ""))] if len(upload) != 1: bad.append(f"expected exactly one upload-artifact step, found {len(upload)}; " diff --git a/scripts/tests/branch-owner-mutations.py b/scripts/tests/branch-owner-mutations.py index 7beb343..f044a23 100644 --- a/scripts/tests/branch-owner-mutations.py +++ b/scripts/tests/branch-owner-mutations.py @@ -91,18 +91,71 @@ ("the oldest line is no longer the one taken", " return out.splitlines()[0].strip()", " return out.splitlines()[-1].strip()"), - ("a silently truncated PR list is reported as complete", - " if len(rows) >= PR_LIMIT:", - " if False and len(rows) >= PR_LIMIT:"), + # --- THE READ MUST REACH THE END, AND SAY SO WHEN IT DOES NOT (backend#2972) + # + # `--limit 1000` was replaced by a paged read checked against the repository's + # own `totalCount`, so the mutations are the ways that check can be defeated: + # stop paging, stop slurping, misname the cursor gh injects, or simply not + # compare. Each is a mutation somebody would really write, and the first three + # are the ones that leave a SHORT list looking like a complete one. + ("the read stops after page 1, so a big repo comes back short", + ' rc, out = _run(["gh", "api", "graphql", "--paginate", "--slurp",', + ' rc, out = _run(["gh", "api", "graphql", "--slurp",'), + ("the pages are not slurped into one document, so the body will not parse", + ' rc, out = _run(["gh", "api", "graphql", "--paginate", "--slurp",', + ' rc, out = _run(["gh", "api", "graphql", "--paginate",'), + ("the query variables go back to -F, which types a numeric repo name", + ' "-f", f"owner={owner}", "-f", f"name={name}",', + ' "-F", f"owner={owner}", "-F", f"name={name}",'), + ("the cursor variable is renamed, which is an infinite page-1 loop in reality", + "query($owner: String!, $name: String!, $endCursor: String) {", + "query($owner: String!, $name: String!, $cursor: String) {"), + ("the query pages on a variable gh will never fill", + " pullRequests(first: %d, after: $endCursor) {", + " pullRequests(first: %d, after: $cursor) {"), + ("the repository is never asked for its own count, so nothing checks the read", + " totalCount\n pageInfo { hasNextPage endCursor }", + " pageInfo { hasNextPage endCursor }"), + ("a short read is accepted as the whole list", + " if len(rows) != total:", + " if False and len(rows) != total:"), + ("a short read loses its marker, so `main` serves rows out of it", + ' return {}, (f"{INCOMPLETE_PR_LIST} {where} reports {total} pull request(s) "', + ' return {}, (f"{where} reports {total} pull request(s) "'), + ("the marker is smeared onto a failed gh call, so it marks nothing", + ' return {}, ("`gh api graphql` failed -- no gh, no auth, or no network. "', + ' return {}, (f"{INCOMPLETE_PR_LIST} `gh api graphql` failed. "'), + ("a page that came back twice is counted as progress", + " if len(set(numbers)) != len(numbers):", + " if False and len(set(numbers)) != len(numbers):"), + ("a count that disagrees with itself between pages is picked from arbitrarily", + " if len(totals) != 1 or not isinstance(next(iter(totals)), int):", + " if False and (len(totals) != 1 or not isinstance(next(iter(totals)), int)):"), + ("a GraphQL errors[] payload at exit 0 is read as an empty repo", + " errors = page.get(\"errors\")\n if errors:", + " errors = page.get(\"errors\")\n if False:"), + ("main tallies an incomplete list into rows again, as the ticket found it", + " if problem.startswith(INCOMPLETE_PR_LIST):\n return 2", + " if False and problem.startswith(INCOMPLETE_PR_LIST):\n return 2"), + ("main refuses on ANY pull-request problem, not just an incomplete read", + " if problem.startswith(INCOMPLETE_PR_LIST):\n return 2", + " if problem:\n return 2"), + ("the tally drops the one reason every row was refused for", + ' because = f" -- every row refused for one reason: {problem}" if problem else ""', + ' because = ""'), + ("the repo to read is guessed at instead of refused when it is not owner/name", + " if not owner or not slash or not name or \"/\" in name:", + " if False:"), + ("a failed gh call reports no problem", - ' return {}, ("`gh pr list` failed -- no gh, no auth, or no network. "\n' + ' return {}, ("`gh api graphql` failed -- no gh, no auth, or no network. "\n' ' "Absence from a list that was never read proves nothing")', ' return {}, ""'), ("an unparseable PR list is reported as empty rather than unreadable", ' return {}, f"the pull-request list did not parse as JSON ({exc})"', ' return {}, ""'), ("a PR list that is not a list is accepted as empty", - ' return {}, f"the pull-request list is a {type(rows).__name__}, not a list"', + ' return {}, f"the pull-request list is a {type(pages).__name__}, not a list"', ' return {}, ""'), # --- the two defects the first live run turned up ---------------------- @@ -186,9 +239,22 @@ ' vrc, _ = _run(["git", "rev-parse", "--verify", "--quiet", ref])\n' ' if False:'), ("the remote is never asked, so the cache is the only source", + ' args = ["gh", "repo", "view"]\n if repo:\n args.append(repo)', + ' args = ["false"]\n if repo:\n args.append(repo)'), + # THE REGRESSION THAT WAS INVISIBLE FOR THE LIFE OF THE FILE: `gh repo view` + # has no `--repo` flag, so the flag form never asked the remote anything. The + # old case asserted the arg shape and passed either way. + ("the repo goes back to a --repo flag, which `gh repo view` refuses", + ' args = ["gh", "repo", "view"]\n if repo:\n args.append(repo)\n' + ' args += ["--json", "defaultBranchRef", "--jq", ".defaultBranchRef.name"]', ' args = ["gh", "repo", "view", "--json", "defaultBranchRef",\n' - ' "--jq", ".defaultBranchRef.name"]', - ' args = ["false"]'), + ' "--jq", ".defaultBranchRef.name"]\n if repo:\n' + ' args += ["--repo", repo]'), + ("the clone-identity lookup grows a --repo flag gh will not take", + ' rc, out = _run(["gh", "repo", "view", "--json", "nameWithOwner",\n' + ' "--jq", ".nameWithOwner"])', + ' rc, out = _run(["gh", "repo", "view", "--repo", "--json", "nameWithOwner",\n' + ' "--jq", ".nameWithOwner"])'), # --- the structural guarantee ----------------------------------------- ("a tip_author parameter is reintroduced", diff --git a/scripts/tests/branch-owner-selftest.py b/scripts/tests/branch-owner-selftest.py index 026c235..8f211f7 100644 --- a/scripts/tests/branch-owner-selftest.py +++ b/scripts/tests/branch-owner-selftest.py @@ -16,7 +16,9 @@ sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent.parent)) from branch_owner import ( # noqa: E402 - PR_LIMIT, + INCOMPLETE_PR_LIST, + PAGE_SIZE, + PR_QUERY, SIGNALS, UNATTRIBUTABLE, attribute, @@ -300,8 +302,65 @@ def run(args): return rc, out return run + import json as _json + + REPO = "tracebloc/backend" + + def graphql_pages(rows, total=None, per_page=None): + """A `gh api graphql --paginate --slurp` body: `rows` split into pages. + + `total` defaults to len(rows) -- a COMPLETE read. Passing a LARGER one is + how a short read is constructed: the repository says it has N and the pages + carry fewer, which is exactly the shape pagination stopping early leaves + behind, and nothing about it has to be assumed about gh's internals. + """ + per_page = PAGE_SIZE if per_page is None else per_page + total = len(rows) if total is None else total + chunks = [rows[i:i + per_page] for i in range(0, len(rows), per_page)] or [[]] + return _json.dumps([ + {"data": {"repository": {"pullRequests": { + "totalCount": total, + "pageInfo": {"hasNextPage": i < len(chunks) - 1, + "endCursor": f"cursor{i}"}, + "nodes": chunk}}}} + for i, chunk in enumerate(chunks)]) + + # THE CURSOR VARIABLE'S NAME IS THE TRAP, so it is a machine check on the query + # text rather than a comment asking the next editor to be careful. `gh + # --paginate` injects the next cursor into `$endCursor` and no other name; + # misname it and gh re-requests page 1 forever, and the secondary rate limit + # that loop trips does not show up in `gh api rate_limit` -- so the failure does + # not even look like a failure. Asserted on BOTH halves: the declaration and + # the use, because renaming either one alone is enough to break it. + eq("the paged query declares the cursor variable gh will inject", + "$endCursor: String" in PR_QUERY, True) + eq("the paged query actually pages on that variable", + "after: $endCursor" in PR_QUERY, True) + eq("no other cursor variable name is used", + [w for w in PR_QUERY.split() if w.startswith("$cursor")], []) + eq("the page size asked for is GitHub's connection maximum", PAGE_SIZE, 100) + eq("the query asks the repository for its own count, to check the read against", + "totalCount" in PR_QUERY, True) + + # ... and the read must actually ASK to be paged and slurped, or the query above + # returns exactly one page and every completeness check below is vacuous. + calls.clear() + _m._run = stub(0, graphql_pages([])) + _m.pull_requests(REPO) + args = calls[-1] + eq("the read pages to the end", "--paginate" in args, True) + eq("the pages come back as one document", "--slurp" in args, True) + eq("the repo is passed as the query's two halves", + [a for a in args if a.startswith(("owner=", "name="))], + ["owner=tracebloc", "name=backend"]) + # `-f` NOT `-F`: `-F` types its value, and a numeric repo name then arrives as + # an int against a `String!` variable. Measured 2026-09-01: `-F name=123` is + # refused with "Could not coerce value 123 to String", `-f name=123` is not. + eq("the variables are sent as strings, so a numeric repo name still resolves", + [a for a in args if a == "-F"], []) + _m._run = stub(1, "") - heads, problem = _m.pull_requests() + heads, problem = _m.pull_requests(REPO) eq("a failed gh call yields no heads", heads, {}) if problem and "proves nothing" in problem: ok("a failed gh call yields a problem that explains itself") @@ -309,50 +368,161 @@ def run(args): bad(f"a failed gh call did not fail closed: {problem!r}") _m._run = stub(0, "not json at all") - heads, problem = _m.pull_requests() + heads, problem = _m.pull_requests(REPO) if problem and "JSON" in problem: ok("an unparseable list is a problem, not an empty result") else: bad(f"an unparseable list was accepted: {problem!r}") + # `--slurp` wraps the pages in ONE array, so a bare object is a shape problem + # and not "a single page". _m._run = stub(0, '{"headRefName": "x"}') - heads, problem = _m.pull_requests() + heads, problem = _m.pull_requests(REPO) if problem and "not a list" in problem: - ok("a list that is not a list is a problem") + ok("a body that is not a list of pages is a problem") else: bad(f"a non-list was accepted: {problem!r}") - # THE SILENT-TRUNCATION CASE. Exactly PR_LIMIT rows means the window may be - # partial, so absence from it proves nothing -- derived from PR_LIMIT, not - # from a hand-typed number. - import json as _json - capped = _json.dumps([pr(i, "x", f"b{i}", "0" * 40) for i in range(PR_LIMIT)]) - _m._run = stub(0, capped) - heads, problem = _m.pull_requests() - if problem and str(PR_LIMIT) in problem: - ok(f"a list of exactly {PR_LIMIT} rows is reported as possibly truncated") + # A GRAPHQL ERROR IS AN HTTP 200. Reading only the exit code turns a bad field, + # a missing repo and a permissions failure alike into "no pull requests". + _m._run = stub(0, _json.dumps([{"data": None, "errors": [ + {"message": "Could not resolve to a Repository with the name 'x/y'."}]}])) + heads, problem = _m.pull_requests(REPO) + eq("an errors[] payload at exit 0 yields no heads", heads, {}) + if problem and "Could not resolve" in problem: + ok("an errors[] payload at exit 0 is a refusal that quotes the error") else: - bad(f"the --limit cap was not detected: {problem!r}") + bad(f"a GraphQL error at exit 0 was accepted: {problem!r}") - under = _json.dumps([pr(i, "x", f"b{i}", "0" * 40) for i in range(PR_LIMIT - 1)]) - _m._run = stub(0, under) - heads, problem = _m.pull_requests() - eq(f"a list of {PR_LIMIT - 1} rows is not truncated", problem, "") - eq("rows are grouped by head ref", len(heads), PR_LIMIT - 1) - - _m._run = stub(0, _json.dumps([pr(1, "a", "same", "0" * 40), - pr(2, "b", "same", "1" * 40)])) - heads, problem = _m.pull_requests() + # --- THE REGRESSION THIS TICKET IS FOR --------------------------------- + # + # A repo with MORE PULL REQUESTS THAN ONE PAGE STILL ATTRIBUTES. Under the old + # `--limit 1000` the largest repo in the org refused outright -- 108 branches, + # 0 attributed -- so "the read spans pages and the answer still comes back" is + # the case that has to hold, not merely "a big read is refused politely". + # PAGE_SIZE * 2 + 50 rows, so the last page is a PARTIAL one: an off-by-one in + # the paging would land exactly here. + many = [pr(i, "saqlainsyed007", f"b{i}", f"{i:040d}") + for i in range(PAGE_SIZE * 2 + 50)] + _m._run = stub(0, graphql_pages(many)) + heads, problem = _m.pull_requests(REPO) + eq(f"a {len(many)}-pull-request repo is read across pages without refusing", + problem, "") + eq("every row from every page arrives", sum(len(v) for v in heads.values()), + len(many)) + eq("rows are grouped by head ref", len(heads), len(many)) + # ... and the rows still carry what the RULE reads, so paging did not change + # the shape `attribute` was written against. + last = heads[f"b{len(many) - 1}"][0] + got = att(last["headRefName"], last["headRefOid"], [last]) + eq("a branch from the last page attributes from its PR author", + (got.owner, got.signal), ("saqlainsyed007", "pr-exact")) + + # --- AND THE BACKSTOP STILL REFUSES ------------------------------------ + # + # Pagination CAN stop early, and then the list is short. Derived from the + # repository's own totalCount, so this is two measured numbers disagreeing + # rather than a ceiling somebody picked -- and it is a MARKED refusal, because + # `main` exits non-zero on this and on nothing else. + short = graphql_pages(many[:PAGE_SIZE], total=len(many)) + _m._run = stub(0, short) + heads, problem = _m.pull_requests(REPO) + eq("a short read yields no heads at all", heads, {}) + eq("a short read is a marked refusal", problem.startswith(INCOMPLETE_PR_LIST), + True) + eq("the refusal names the repo it is about", REPO in problem, True) + for what, number in (("what the repo says it has", len(many)), + ("what the read returned", PAGE_SIZE), + ("the shortfall", len(many) - PAGE_SIZE)): + eq(f"the refusal names {what}", str(number) in problem, True) + if "not a" in problem and "unattributable" in problem: + ok("the refusal says outright it is not an 'unattributable' finding") + else: + bad(f"the refusal does not distinguish itself: {problem!r}") + + # A PAGE THAT CAME BACK TWICE is what a broken cursor looks like on the near + # side of the infinite loop, and it would otherwise pass the count check by + # accident once the duplicates make the totals line up. + dupes = many[:PAGE_SIZE] + many[:PAGE_SIZE] + _m._run = stub(0, graphql_pages(dupes, total=len(dupes))) + heads, problem = _m.pull_requests(REPO) + eq("a duplicated page is a marked refusal", + (heads, problem.startswith(INCOMPLETE_PR_LIST)), ({}, True)) + if "distinct" in problem and "endCursor" in problem: + ok("the duplicate refusal names the cursor as the likely cause") + else: + bad(f"a duplicated page was not diagnosed: {problem!r}") + + # A COUNT THAT DISAGREES WITH ITSELF between pages is a repo that changed + # mid-read: there is then no single number to check against, and "cannot tell" + # is a refusal rather than a pass. + pages = _json.loads(graphql_pages(many)) + pages[-1]["data"]["repository"]["pullRequests"]["totalCount"] = len(many) + 1 + _m._run = stub(0, _json.dumps(pages)) + heads, problem = _m.pull_requests(REPO) + eq("a totalCount that disagrees across pages is a marked refusal", + (heads, problem.startswith(INCOMPLETE_PR_LIST)), ({}, True)) + + # ... and so is a count that never came back at all, which would otherwise + # compare a list against None and take the mismatch for a short read. + pages = _json.loads(graphql_pages(many)) + for page in pages: + del page["data"]["repository"]["pullRequests"]["totalCount"] + _m._run = stub(0, _json.dumps(pages)) + eq("a missing totalCount is a marked refusal", + _m.pull_requests(REPO)[1].startswith(INCOMPLETE_PR_LIST), True) + + # --- THE MARKER MUST BE EXCLUSIVE -------------------------------------- + # + # `main` exits non-zero on the marked refusal and on no other problem, so a + # bare non-zero would leave a cap-hit and a dead network indistinguishable -- + # which is where this ticket started. That is a claim about the seam's WHOLE + # problem domain, so it is asserted against every other problem it can produce + # rather than about the one string above. + for label, answer in (("a failed gh call", (1, "")), + ("a missing gh", (127, "gh: not found")), + ("an unparseable body", (0, "not json at all")), + ("a body that is not a list", (0, '{"a": 1}')), + ("a page with no pullRequests", (0, '[{"data": {}}]')), + ("an errors[] payload", + (0, '[{"errors": [{"message": "nope"}]}]'))): + _m._run = stub(*answer) + _, other = _m.pull_requests(REPO) + eq(f"{label} is a problem, but NOT the incomplete-list refusal", + (bool(other), other.startswith(INCOMPLETE_PR_LIST)), (True, False)) + + _m._run = stub(0, graphql_pages([pr(1, "a", "same", "0" * 40), + pr(2, "b", "same", "1" * 40)])) + heads, problem = _m.pull_requests(REPO) eq("two PRs on one head land in one group", len(heads.get("same", [])), 2) - # `--repo` is only added when asked for, so the tool works in a bare clone. - calls.clear() - _m._run = stub(0, "[]") - _m.pull_requests("tracebloc/client") - eq("--repo is passed through", "tracebloc/client" in calls[-1], True) + # --- WHICH REPO IS BEING READ ----------------------------------------- + # + # GraphQL has no `{owner}/{repo}` placeholder, so the resolution `gh pr list` + # did implicitly now happens in the open -- and can fail, which is a refusal. + eq("owner/name is split for the query", _m.repo_identity("tracebloc/client"), + ("tracebloc", "client", "")) + for bad_repo in ("tracebloc", "/client", "tracebloc/", "a/b/c", ""): + if bad_repo == "": + continue + owner, name, why = _m.repo_identity(bad_repo) + eq(f"{bad_repo!r} is not a repository and is refused, not guessed at", + (owner, name, bool(why)), ("", "", True)) + # ... and with nothing passed, the clone is asked -- by a command that has no + # `--repo` flag to get wrong (see `default_branch`). calls.clear() - _m.pull_requests() - eq("--repo is absent by default", "--repo" in calls[-1], False) + _m._run = stub(0, "tracebloc/.github") + eq("a bare clone resolves itself", _m.repo_identity(), + ("tracebloc", ".github", "")) + eq("the clone is asked for nameWithOwner", "nameWithOwner" in calls[-1], True) + eq("no --repo flag is handed to `gh repo view`", "--repo" in calls[-1], False) + _m._run = stub(1, "") + owner, name, why = _m.repo_identity() + eq("a clone that cannot be identified is a refusal", (owner, name), ("", "")) + if why and "proves nothing" in why: + ok("an unidentifiable clone fails closed rather than reading some other repo") + else: + bad(f"an unidentifiable clone did not fail closed: {why!r}") # THE OLDEST COMMIT, NOT THE TIP. git prints oldest-first under --reverse, so # the first line is the one to take -- and the request must say --reverse and @@ -453,9 +623,43 @@ def run(args): eq("the named ref is verified to exist locally", any("rev-parse" in c and "origin/develop" in c for c in calls), True) + # --- WITH A REPO NAMED, THE REMOTE MUST ACTUALLY BE ASKED -------------- + # + # `gh repo view` HAS NO `--repo` FLAG. It takes the repository positionally + # (`gh repo view [] [flags]`) and the flag form exits 1 with + # `unknown flag: --repo` -- measured 2026-09-01. This seam built the flag form, + # so on EVERY `--repo` run the authoritative lookup failed unasked, the answer + # fell through to `origin/HEAD` (a clone-time cache), the first-commit signal + # was withheld for every branch, and the message blamed a remote that was never + # queried. On `tracebloc/backend` that alone cost 14 of 108 attributions + # (backend#2972). + # + # ASSERTING THE ARGUMENT SHAPE IS THE WEAK TEST, and its weakness is why this + # survived: a permissive stub answers a malformed command as happily as a good + # one, so a case named "--repo reaches the default-branch query" passed + # throughout. This stub instead MODELS gh's real accepted surface -- it refuses + # an unknown flag the way gh does -- and the assertion is that the problem + # string comes back EMPTY, which only a command gh would accept can achieve. + def gh_like(answer): + def run(args): + calls.append(args) + if args[:3] == ["gh", "repo", "view"] and "--repo" in args: + return 1, "" # exactly what gh does: unknown flag + return answer + return run + calls.clear() - _m.default_branch("tracebloc/client") - eq("--repo reaches the default-branch query", "tracebloc/client" in calls[0], True) + _m._run = gh_like((0, "develop")) + eq("a named repo reaches the AUTHORITATIVE default-branch lookup", + _m.default_branch("tracebloc/client"), ("origin/develop", "")) + eq("the repo is named to the command in the form that command accepts", + "tracebloc/client" in calls[0], True) + + # ... and the same surface, applied to the other seam that calls `gh repo view`. + calls.clear() + _m._run = gh_like((0, "tracebloc/client")) + eq("resolving a bare clone reaches the authoritative lookup too", + _m.repo_identity(), ("tracebloc", "client", "")) seq2 = [(0, "develop"), (1, "")] # gh answers; the local ref is absent _m._run = lambda args: seq2.pop(0) ref, why = _m.default_branch() @@ -512,7 +716,12 @@ def run(args): # argument reverts the whole fix while every isolated case stays green -- which # is exactly what a mutation run showed. So this drives the real entry point. def run_main(answers, argv=()): - """(exit code, stdout) with `_run` answering per command.""" + """(exit code, stdout, stderr) with `_run` answering per command. + + STDERR IS RETURNED, NOT DISCARDED: every refusal this entry point makes is + written there, so a case reading only the exit code cannot say WHICH refusal + it got -- and a short read and a dead network would look alike again. + """ def fake(args): joined = " ".join(args) for needle, reply in answers: @@ -520,15 +729,21 @@ def fake(args): return reply return 0, "" _m._run = fake - buf = io.StringIO() - with contextlib.redirect_stdout(buf), contextlib.redirect_stderr(io.StringIO()): + buf, ebuf = io.StringIO(), io.StringIO() + with contextlib.redirect_stdout(buf), contextlib.redirect_stderr(ebuf): code = _m.main(list(argv)) - return code, buf.getvalue() - - GOOD_HEAD = [("repo view", (0, "develop")), ("pr list", (0, "[]")), + return code, buf.getvalue(), ebuf.getvalue() + + # NEEDLES ARE MATCHED AGAINST THE JOINED COMMAND, so `gh repo view` has to be + # disambiguated BY THE FIELD IT ASKS FOR: two seams call it now, and a bare + # "repo view" needle answers whichever one asks first -- a fake that agrees with + # itself instead of with the module. + GOOD_HEAD = [("defaultBranchRef", (0, "develop")), + ("nameWithOwner", (0, "tracebloc/backend")), + ("graphql", (0, graphql_pages([]))), ("for-each-ref", (0, "origin/feat/x\tabc123"))] - code, out = run_main([*GOOD_HEAD, ("git log", (128, ""))]) + code, out, _ = run_main([*GOOD_HEAD, ("git log", (128, ""))]) eq("main exits 0 having reported the branch", code, 0) if "was not measured" in out and "git log" in out: ok("main carries a FAILED history read through to the row as unmeasured") @@ -541,7 +756,7 @@ def fake(args): # ... and with the same shape but a SUCCESSFUL empty history, the other # sentence is the right one. Pinned apart end-to-end, not only at the seam. - code, out = run_main([*GOOD_HEAD, ("git log", (0, ""))]) + code, out, _ = run_main([*GOOD_HEAD, ("git log", (0, ""))]) eq("main exits 0 on a genuinely empty history", code, 0) if "no commit on this branch" in out and "was not measured" not in out: ok("main renders a genuinely empty history as exactly that") @@ -550,15 +765,79 @@ def fake(args): # A FAILED ENUMERATION IS A NON-ZERO EXIT, not a report of zero branches -- # Saqlain's finding, asserted at the entry point he was reading. - code, out = run_main([("repo view", (0, "develop")), ("pr list", (0, "[]")), - ("for-each-ref", (128, ""))]) + code, out, _ = run_main([*GOOD_HEAD[:3], ("for-each-ref", (128, ""))]) eq("main refuses when the branch list could not be read", code, 2) eq("main prints no rows when it refuses", out.strip(), "") # ... while a genuinely empty remote is a clean, zero-row success. - code, out = run_main([("repo view", (0, "develop")), ("pr list", (0, "[]")), - ("for-each-ref", (0, ""))]) + code, out, _ = run_main([*GOOD_HEAD[:3], ("for-each-ref", (0, ""))]) eq("main exits 0 on a genuinely empty remote", code, 0) + + # --- MORE PULL REQUESTS THAN ONE PAGE, END TO END ---------------------- + # + # THE CASE THE TICKET WAS FILED FOR (backend#2972), driven through the entry + # point rather than described. `tracebloc/backend`'s 1418 pull requests against + # the old `--limit 1000` made the seam fail closed, and `main` printed + # + # 108 branch(es): 0 attributed, 108 unattributable + # + # and exited 0 -- indistinguishable from a repo whose branches genuinely cannot + # be attributed, in the repo where 102 of the 108 do attribute. A seam that + # pages correctly buys nothing if `main` cannot carry a multi-page answer, so + # this asserts the ANSWER, not merely the absence of a refusal. + spread = [pr(i, "shujaatTracebloc", f"old/{i}", f"{i:040d}") + for i in range(PAGE_SIZE * 2 + 7)] + spread.append(pr(9999, "waqaskhanroghani", "feat/x", "abc123")) + code, out, err = run_main( + [*GOOD_HEAD[:2], ("graphql", (0, graphql_pages(spread))), + ("for-each-ref", (0, "origin/feat/x\tabc123")), + ("git log", (0, "First Person "))], + argv=("--repo", "tracebloc/backend")) + eq(f"main attributes across a {len(spread)}-pull-request repo", code, 0) + eq("the owner comes from the PR author on a paged read", + [ln for ln in out.splitlines() if ln.startswith("feat/x\t")], + ["feat/x\twaqaskhanroghani\tpr-exact\tPR #9999 author, head oid matches the tip"]) + eq("the tally is a measurement, with nothing appended to it", + err.strip().endswith("1 branch(es): 1 attributed, 0 unattributable"), True) + + # --- AND AN INCOMPLETE READ IS A REFUSAL, NOT A ZERO-ROW ANSWER -------- + # + # The backstop survives the fix: pagination stopping early is now a real + # "could not read it all" rather than the routine condition it used to be, and + # it must not come back as rows. + code, out, err = run_main( + [*GOOD_HEAD[:2], + ("graphql", (0, graphql_pages(spread[:PAGE_SIZE], total=len(spread)))), + ("for-each-ref", (0, "origin/feat/x\tabc123"))], + argv=("--repo", "tracebloc/backend")) + eq("main refuses an incomplete PR list instead of reporting rows", code, 2) + eq("main prints no rows out of a short read", out.strip(), "") + # NOT A BARE NON-ZERO: that is also what a dead network returns, and a caller + # who cannot tell them apart is back where this ticket started. + eq("main's refusal is the incomplete-list one specifically", + INCOMPLETE_PR_LIST in err, True) + eq("main's refusal names the repo whose read fell short", + "tracebloc/backend" in err, True) + eq("main's refusal names both counts", + (str(len(spread)) in err, str(PAGE_SIZE) in err), (True, True)) + # ... and prints NO tally at all, because a tally is the shape of an answer. + eq("main prints no branch tally for a short read", "branch(es):" in err, False) + + # ... while the PR-list problems that are NOT a short read still report their + # rows -- with the one reason they were all refused for carried into the tally, + # since `0 attributed, N unattributable` alone is the same misreading one path + # over. Pinned in both directions, or "refuse on a short read" degrades into + # "refuse whenever the PR list is imperfect". + code, out, err = run_main([*GOOD_HEAD[:2], ("graphql", (1, "")), + ("for-each-ref", (0, "origin/feat/x\tabc123"))], + argv=("--repo", "tracebloc/backend")) + eq("a PR list that could not be read still reports its rows", code, 0) + eq("its rows are all refused for the unread list", + out.count("the pull-request list is not evidence"), 1) + if "every row refused for one reason" in err and "proves nothing" in err: + ok("the tally carries the one reason every row was refused for") + else: + bad(f"the tally hid the reason behind a bare count: {err.strip()[-200:]!r}") finally: _m._run = _real