fix(gh): distinguish an unresolvable GH_TOKEN from a mismatched one

[twistedmelonman]

Follow-up to #302, found the hard way: a real token expired mid-session and the guard from #302 blamed the wrong thing.

The bug

#302 resolves the token's identity with gh api user --jq .login and treats an empty result as the failure signal. But gh api writes its error body to stdout, not stderr — so 2>/dev/null doesn't suppress it, and the command substitution captures the JSON:

$ GH_TOKEN=<expired> gh api user --jq .login 2>/dev/null; echo "rc=$?"
{
  "message": "Bad credentials",
  "documentation_url": "https://docs.github.com/rest",
  "status": "401"
}
rc=1

Non-empty. So the could not be resolved branch was unreachable, and an expired token fell through to the mismatch comparison:

[gh] ERROR: GH_TOKEN authenticates as '{
  "message": "Bad credentials",
  "documentation_url": "https://docs.github.com/rest",
  "status": "401"
}' but repo owner 'smartwatermelon' requires 'smartwatermelon'
[gh] Fix: unset GH_TOKEN to use the keyring identity for this repo.

Note the tell: it claims a mismatch while printing the same name on both sides.

Not a security hole — it still failed closed, which is why this is a follow-up and not a revert. But it names the wrong cause, and the suggested fix ("unset GH_TOKEN") is wrong for the situation that actually triggers it. The right fix is to rotate the token. This matters on a fixed schedule: the current PAT expires 2026-12-02, and this is exactly what will greet whoever hits it first.

The fix

Decide on exit status, not output emptiness. if api_out="$(...)" keeps the value only when the call actually succeeded.

Reject anything not shaped like a login. A GitHub username is ^[A-Za-z0-9-]+$. Anything multi-line or punctuated that reaches the comparison is an error body arriving by some other path, and is discarded before it can be compared against desired. Belt and braces — the exit-status check is the real fix, this is the backstop for the class.

Say what is probably wrong. The resolution-failure message now names token expiry as the likely cause and gives the command to check it:

[gh] ERROR: GH_TOKEN is set but its identity could not be resolved
[gh] Most likely the token is expired or revoked. Check with:
[gh]   gh api -i user | grep -i token-expiration
[gh] Fix: rotate the token, or unset GH_TOKEN to use the keyring identity.

Testing

Case 4 uses a stub gh on PATH whose output shape was verified against the real binary (gh 2.99.0) rather than assumed — JSON body on stdout, human-readable line on stderr, exit 1. That keeps the case hermetic; no network, no real credential.

All three new assertions were observed failing against the pre-fix code:

PASS: expired GH_TOKEN: fails closed
FAIL: expired GH_TOKEN: should report a resolution failure, got: [gh] ERROR: GH_TOKEN authenticates as '{
FAIL: expired GH_TOKEN: leaked the raw API error body into the message

The third assertion is worth keeping on its own: it pins that the raw API error body never reaches the user-facing message, independent of which branch produces it.

Full suite 24 passed, 0 failed. shellcheck -S info clean, no disable directives. Live check: the valid-token path is unaffected — gh repo view still succeeds on-org.

Note on the test fixture

The first version of case 4 used ghp_expired000... as the fake token and the pre-commit Semgrep secrets scan blocked it — a ghp_-prefixed string reads as a hard-coded PAT no matter what follows. Changed to expired-token-fixture; the guard only cares that GH_TOKEN is non-empty, so the shape was never load-bearing. Flagging it because the next person writing a token fixture here will hit the same wall.

How this got through #302

Its Step 9 exercised the valid-token path only, and no reviewer ran the code against a bad credential — the failure mode lives in gh's stdout/stderr split, which is invisible when you reason about the source instead of executing it. Same lesson as the two false-OK findings recorded in that PR's plan: a check that never sees a known-bad case proves nothing.

Follow-up to #302. Does not touch #303 (identity-lookup caching), which remains open.

https://claude.ai/code/session_01RBvPRMFfep4ktSDfGe9uHq

[claude]

The PR fixes a real bug: `gh api` writes its error body to STDOUT, so a failed command substitution still produced a non-empty `token_login`, causing an expired token to be reported as an identity mismatch rather than a resolution failure. The fix correctly gates assignment on exit status and adds a format-guard regex. The test case exercises the exact failure path with a hermetic stub. No correctness, reliability, security, or data-loss issues in scope.

VERDICT: PASS