Repository navigation
fix(git): retry a PR lookup without CI fields only when gh names that field - #825
Open
eric-engberg wants to merge 2 commits into
Open
eric-engberg wants to merge 2 commits into
eric-engberg wants to merge 2 commits into
Conversation
… field With a Git CI widget on the line, the PR lookup asks `gh pr view` for statusCheckRollup and runs again without it when gh can't give that field. The check searched the whole error text for "statuscheckrollup". execFileSync starts that error's message with the command line, which asks for statusCheckRollup, so every failure matched: no PR for the branch, an auth error, a network error. Each failing lookup ran gh twice inside the one 5s budget that the lookup with gh's own repo resolution and the one pinned to origin with `--repo` share. On a branch with no PR that was four gh runs instead of two. When gh takes about 1.7s or more, the pinned lookup, which is what finds a PR opened from a fork, ran out of time and the lookup was cached as no PR. Decide from gh's stderr alone. gh names the field when it can't give it: its --json check prints `Unknown JSON field: "statusCheckRollup"`, and a GraphQL error on the field gives the field's path. The tests' stand-in gh failures now carry the command line in their message and gh's output in stderr, as execFileSync's errors do.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
When a Git CI widget is on the line and
gh pr viewfails for an ordinary reason (no PR for the branch, an auth or network error), the PR lookup no longer runs gh a second time without the CI fields. Each failed gh run used to be followed by a second one, which doubled the time the lookup spent inside its 5s budget. Now a PR opened from a fork is found with a slow gh instead of being cached as "no PR". The run without CI fields still happens when gh can't give the CI field.Why
With a CI widget, the lookup asks for
--json url,number,title,state,reviewDecision,statusCheckRollup. If that fails andisCiFieldUnavailableErrorsays gh couldn't give the field, the lookup runs again withoutstatusCheckRollup. That check searched the error's message and stderr together forstatuscheckrollup.execFileSyncstarts the message with the command line, and the command line asks forstatusCheckRollup. So every failure matched.What
execFileSyncthrows for a gh that exits 1 withno pull requests found for branch "feature/x"(the same under Node 26.10.0 and Bun 1.4.2):What gh itself prints when it can't give a field:
--jsoncheck (pkg/cmdutil/json_flags.go) buildsfmt.Errorf("Unknown JSON field: %q\nAvailable fields:\n %s", ...). gh 2.102.0, run offline with a scratch config and a dummy token, printsUnknown JSON field: "statusCheckRollupX", thenAvailable fields:and the list. The wording is the same in v1.9.0, the April 2021 release that added--json, andstatusCheckRollupwas already in its PR fields then.GraphQL: <message> (<path>), from go-gh'sGraphQLError.Error()). gh queries the field asstatusCheckRollup: commits(last: 1) { … }, so an error on it namesstatusCheckRollup.--jsonat all, so the run without CI fields would fail the same way.Measured with a stand-in gh first on
PATHthat logs every call and sleeps before failing, a scratch repo on branchfeature/no-prwith anhttps://github.com/…origin, and the background refresh a CI widget starts (ccstatusline --internal-refresh-git-review-cache <repo> checks <lock>). The lookup asks gh twice: first with gh's own repo resolution, then pinned to origin with--repo.maingh pr viewtakes 1s--repolookup finds it; each run takes 1.8sUnknown JSON field: "statusCheckRollup"for the CI fieldsA "no PR" result stays cached for 30s, so in the fork case the Git PR widget showed nothing until the next refresh, which then ran out of time the same way.
How
statuscheckrollup, which gh'sUnknown JSON fielderror and a GraphQL error on the field both contain, orresource not accessible by integration.git-review-cache.test.ts, the stand-in gh failures are now shaped likeexecFileSync's errors: the message starts with the command line asking forstatusCheckRollup, and gh's output is instderr. "does not retry metadata-only for ordinary CI lookup failures" uses gh'sno pull requests found for branch "…"and now fails onmain(3 gh runs instead of 2). The two tests for the retry use gh'sUnknown JSON field: "statusCheckRollup"and a GraphQLResource not accessible by integrationerror on the field's path.Left as is:
Available fields:list namesstatusCheckRollup, so that error would still match. It can't happen here: every field the lookup asks for has been in gh since 1.9.0 added--json.Overlap with open upstream PRs: #618 (caching confirmed absent PRs) also changes
git-review-cache.tsand conflicts with this one.Demo
No GIFs: the line only differs when gh is slow enough to cut off the fork lookup (the 1.8s rows above), so the timings and gh run counts above show the change.
Testing
mainas of 2.2.34 (0c94df7): it merges cleanly.bun run lintis clean, andbun teston Linux CI gives 4403 pass, 0 fail (4403 onmain). The built CLI under Bun 1.4.4 and Node 26.13.0 agrees across runtimes and matchesmainin plain and Powerline mode, and the TUI opens and exits cleanly under both. It adds no JavaScript or Node API newer than Node 14, the build's target.git-review-cache.test.tschanged (no new tests). One of them fails onmainunder both Bun and Node vitest. The two retry tests pass on both and fail when the retry is turned off.bun run lintis clean.git-review-cache.test.tsunder vitest on Node 26.10.0: 33 of 33 pass, as onmain.mainin plain and Powerline mode, and Bun 1.4.2 and Node 26.10.0 give the same output. The TUI opens and exits cleanly under both.git-review-cache.test.tspasses 59 of 59.