feat(registry): read every gh account when building the repo map and discovering owners - #69
Merged
Merged
Conversation
…vering owners `gh api user/repos` and `gh api user/orgs` answer for whichever account is ACTIVE. On a machine with several logins reaching disjoint sets, the map held one set at a time and every owner the other logins reached produced `warning: no accessible repos for owner '<x>'` — the same warning a mistyped owner produces, so the two were indistinguishable. Discovery had it one level earlier and it cost more: the candidate list is the single question onboarding asks the operator, and an owner never offered never reaches the map at all. Both scripts now enumerate every account for the host and ask each one with its own token, through the new `scripts/lib/gh-accounts.sh`. Tokens are read by name with `gh auth token --user`, so nothing switches the account the operator's `gh` points at. A repo two accounts both reach is one repo. An account whose credential fails is named and skipped. No account list — an old `gh` with no `auth status --json`, or a `GH_TOKEN` already in the environment — falls back to the active session alone, which is the old behaviour. `scripts/check.sh shell` now runs `shellcheck -x` and covers `scripts/lib/*.sh`.
…l failure and silent drops
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.
Intent
Make the fleet control plane's repo map cover every gh account the machine holds, not only the active one, in both places that ask GitHub who the operator is.
The problem: 'gh api user/repos' (scripts/sync-registry.sh) and 'gh api user/orgs' (scripts/discover-owners.sh) answer for whichever gh account is ACTIVE. On a machine with three logins reaching disjoint sets of repositories, the generated map held one set at a time, and every owner the other logins reached produced 'warning: no accessible repos for owner ' — the SAME warning a mistyped owner produces, so the failure was indistinguishable from a typo. discover-owners.sh had the same blindness one level earlier and it cost more: its candidate list is the single question onboarding asks the operator, so four orgs were never offered, and an owner never offered never reaches the map.
The mechanism, verified on this machine: 'gh auth status --json hosts' lists every account for a host with a state and an active flag, and 'gh auth token --hostname --user ' hands over one account's token WITHOUT switching the active one.
Constraints and decisions:
Verification: the selftest sections were written first and watched to fail for the right reason, then the scripts were fixed. Offline with a stubbed gh and no network the selftest covers: every owner any account reaches present in the map, the typo warning still printed for an owner nothing reaches and for no other owner, a repo two accounts both reach counted once, the totals counting the merged set, no account switching, an expired credential and an unreadable token both named and skipped without aborting, every account failing refusing rather than erasing the map, and an older gh with no --json still writing a map from the active account alone. On the real machine: the map went from 67 repos across 2 owners with two spurious unreachable warnings to 90 repos across 4 owners with none, with the active gh account unchanged; discovery went from 4 candidates to 10. ./scripts/check.sh is green.
A previous run of this pipeline failed at the document step because its own agent hit a usage limit, not because of a finding; its review fix commit is already on the branch.
What Changed
scripts/lib/gh-accounts.shenumerates everyghlogin for a host fromgh auth status --json hosts, reads each one's token BY NAME withgh auth token --user(nevergh auth switch), and runs a call as that account by putting the token on theghcommand itself so nothing leaks into the calling shell. An account whose state is notsuccessis named on stderr and skipped; with no jq-parseable account list at all — an oldergh, orGH_TOKEN/GITHUB_TOKENalready set — it prints nothing and callers fall back to the active session, the previous behaviour.scripts/sync-registry.shnow runs oneuser/reposlisting per account and merges them deduped byfull_name, so a repo two logins both reach is counted once. A token it cannot read or a listing that fails is named and skipped rather than fatal; if not one account could be listed it dies and leavesregistry/repos.generated.yamluntouched instead of writing an empty map.scripts/discover-owners.shfolds each login'sgh api useranduser/orgsinto the candidate list, gates oncommand -v ghinstead ofgh auth status(which exits non-zero when any account anywhere has issues), names a login that cannot be read, and defers theread:orgscope hint until every account has answered. Theno accessible repos for ownerwarning is unchanged and now signals only a typo or an owner no login can see.scripts/onboarding-selftest.shgains §4 and §5, appended at the end of the file, driving both scripts offline against a stubbedghin a sandbox repo root: every owner any account reaches present in the map, the unreachable warning printed for the typo owner and no other, the shared repo counted once, merged totals, an expired credential and an unreadable token both named without aborting, all accounts failing refusing without erasing the map, an olderghwriting from the active account alone, andgh auth switchwired as a tripwire.scripts/check.shswitches the shell check toshellcheck -xoverscripts/*.sh scripts/lib/*.sh;AGENTS.md,README.md,CONTRIBUTING.mdand thefleet-onboardingskill describe the multi-login read.Risk Assessment
✅ Low: The change is well-bounded to two scripts plus a new sourced seam, every intent constraint is source-verifiable and satisfied, the one destructive failure mode (overwriting the operator's map with an empty one) is explicitly guarded in both code paths and covered by an offline regression test, and tokens are read by name without export, logging, or any switch of the active account.
Testing
The configured
./scripts/check.shbaseline had already passed; on top of it I drove the two changed scripts as an operator runs them, live against real GitHub on a machine that genuinely holds three gh logins, and compared each run against the base commit's version of the same script. The map grew from 29 repos across 1 owner (with two real owners reported by the same warning a typo produces) to 52 repos across 3 owners with only the typo owner warned, and the candidate list grew from 3 to 10 — with the active gh account and hosts.yml unchanged after every run. Adversarial runs injected faults at the network through a local CONNECT proxy and through an isolated gh config carrying one lapsed login: total listing failure refuses and leaves the map byte-identical, partial failure names the account and still publishes the rest, a login that cannot say who it is is named and non-fatal, and the lapsed login no longer costs discovery every healthy account (base printed a false "gh is not authenticated" and zero candidates, HEAD printed all ten). Two scenarios were not driven live at all: dedup across two accounts that both reach one repository, and the unreadable-token / older-gh-without---jsonfallbacks. These three logins reach strictly disjoint repo sets, and producing either case would mean writing to the operator's real GitHub org or altering their gh credentials and installed gh binary, so both are reported untested — the offline selftest exercises them with a stubbed gh, which is not a live result. This change has no UI surface; CLI transcripts are the reviewer-visible evidence, and all of them are redacted because this repository is public and the logins and orgs are the operator's own../scripts/sync-registry.shagainst real GitHub: base commit wrote 29 repos across 1 owner, HEAD wrote 52 repos across 3 owners — sync-registry-live.txtgh auth status --json hostsactive login andmd5sum ~/.config/gh/hosts.ymlidentical before and after repeated sync and discovery runs — sync-registry-live.txt / discover-owners-live.txt./scripts/discover-owners.sh <empty-root>: 3 candidates at base commit, 10 at HEAD (two further logins and their orgs) — discover-owners-live.txtgh auth statusexits 1; base printed "No candidate owners found" and a false "gh is not authenticated", HEA…gh api usercalls refused at the network: both warned by name, the healthy login's candidates still printed, exit 0 — adversarial-live.txt §DGH_TOKEN=<non-active login token>on both scripts: map written from that session alone (20 repos / 1 owner) with the unreachable owners warned again, discovery lists only that login and its orgs — e…./scripts/onboarding-selftest.sh§4c/§4d, which runs the sync script against a stubbed gh, so no live result was established. It…auth status --jsonstill writes a map from the active account alone./scripts/onboarding-selftest.sh§4f/§4g/§5c/§5d, which drives the scripts against a stubbed gh, so no live result was establish…Evidence: sync-registry driven live: base (29 repos / 1 owner, two false warnings) vs HEAD (52 repos / 3 owners, typo only)
Evidence: discover-owners driven live: 3 candidates before, 10 after, active account unchanged
Evidence: Adversarial live runs: total listing failure refuses and leaves the map byte-identical; partial failure names the account; a lapsed login no longer wipes discovery; two logins failing are named and non-fatal
Evidence: GH_TOKEN environment fallback: both scripts ask that one session alone, the pre-change behaviour
Evidence: Offline onboarding selftest §4/§5 (dedup, expired credential, unreadable token, older gh without --json) — stubbed gh, not a live run
Evidence: The map before and after the change, on the real machine
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 2 issues found → auto-fixed ✅
scripts/onboarding-selftest.sh:601- The new §4 and §5 sections do not scrub the gh environment they depend on, so./scripts/check.sh onboardingfails on an ordinary developer machine and silently exercises the wrong path where it does not fail.gh_accountsshort-circuits on two inherited variables (scripts/lib/gh-accounts.sh:40[ -z "${GH_TOKEN:-}${GITHUB_TOKEN:-}" ] || return 0, and scripts/sync-registry.sh:67 / scripts/discover-owners.sh:140 derive the host fromGH_HOST), but lines 601, 669, 701, 780 and 820 override onlyPATH,HOMEandFIXTURES. Concrete sequence withGH_TOKENexported (a common setup, and one the intent itself names as a supported fallback):gh_accountsreturns nothing, sync-registry takes the active-session branch, the §4 stub evaluates${GH_TOKEN:-tok-octo}to the operator's real token, hits its*) exit 1arm,set -eaborts the sync, and §4a reports exit != 0 with §4d/§4f failing on fixture-unrelated errors — while §4g and §5d still pass, but for the wrong reason (they are exercising the GH_TOKEN short-circuit, not theghtoo old for--jsonpath they claim to cover).GH_HOST=<self-hosted>produces the same divergence via the.hosts[$h]filter. Remedy:unset GH_TOKEN GITHUB_TOKEN GH_HOSTonce at the same boundary the file already clears the git environment for exactly this reason (scripts/onboarding-selftest.sh:66-71), and have §4g/§5d setGH_TOKENexplicitly if they want to cover that half of the fallback.scripts/discover-owners.sh:88- The comment asserts "The token never reaches this shell:gh_api_asscopes it to theghcall itself", but line 150 stores it in the caller's shell:tok="$(gh_account_token "$gh_host" "$acct")", andask_ghthen holds it again inlocal token. The load-bearing and accurate part of the claim is that no assignment persists after the call and no subshell is needed (which is whynote's associative array survives); the absolute "never reaches this shell" is wrong and, on a security-adjacent property, is the kind of comment AGENTS.md's shell guidance calls worse than none. Remedy: narrow the sentence to what is true — the token is never exported and the assignment lives exactly as long as the oneghcall.🔧 Fix applied.
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
./scripts/sync-registry.shagainst real GitHub: base commit wrote 29 repos across 1 owner, HEAD wrote 52 repos across 3 owners — sync-registry-live.txtgh auth status --json hostsactive login andmd5sum ~/.config/gh/hosts.ymlidentical before and after repeated sync and discovery runs — sync-registry-live.txt / discover-owners-live.txt./scripts/discover-owners.sh <empty-root>: 3 candidates at base commit, 10 at HEAD (two further logins and their orgs) — discover-owners-live.txtgh auth statusexits 1; base printed "No candidate owners found" and a false "gh is not authenticated", HEA…gh api usercalls refused at the network: both warned by name, the healthy login's candidates still printed, exit 0 — adversarial-live.txt §DGH_TOKEN=<non-active login token>on both scripts: map written from that session alone (20 repos / 1 owner) with the unreachable owners warned again, discovery lists only that login and its orgs — e…./scripts/onboarding-selftest.sh§4c/§4d, which runs the sync script against a stubbed gh, so no live result was established. It…auth status --jsonstill writes a map from the active account alone./scripts/onboarding-selftest.sh§4f/§4g/§5c/§5d, which drives the scripts against a stubbed gh, so no live result was establish…./scripts/check.sh./scripts/sync-registry.shagainst real GitHub with three live gh logins and an owners.txt spanning one owner per account plus a deliberate typo ownergit show e559816:scripts/sync-registry.shrun in a sandbox root against the same real accounts, to reproduce the pre-change one-account map./scripts/discover-owners.sh <empty-root>at HEAD vs the base-commit copy, clone scan pointed at an empty directory so only the gh evidence differsHTTPS_PROXY=<fault-injecting CONNECT proxy> ./scripts/sync-registry.shwith every repo listing refused (total-failure refusal) and with only one listing allowed (partial failure)HTTPS_PROXY=<fault-injecting CONNECT proxy> ./scripts/discover-owners.shwith two of three logins failing theirgh api usercallGH_CONFIG_DIR=<isolated copy + one lapsed login on another host>driving both the base and HEADdiscover-owners.sh, withgh auth statusexiting 1GH_TOKEN=<non-active login token> ./scripts/sync-registry.shand./scripts/discover-owners.sh(documented environment-token fallback)md5sum ~/.config/gh/hosts.ymland theactiveflag fromgh auth status --json hostsbefore and after every run (no account switching)./scripts/check.sh onboardingand./scripts/onboarding-selftest.sh(offline; §4 and §5 cover dedup, unreadable token, expired credential, older gh without --json)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.