fix(cli): report the real reason gitt miner post rejects a PAT - #1655
Open
tryeverything24 wants to merge 1 commit into
Open
fix(cli): report the real reason gitt miner post rejects a PAT#1655tryeverything24 wants to merge 1 commit into
tryeverything24 wants to merge 1 commit into
Conversation
_validate_pat_locally() collapses three different failures into a bare None, so miner_post always reports "GitHub PAT is invalid or expired": - A PAT that authenticates fine on /user but fails the GraphQL probe (the default for GitHub's fine-grained PATs without "Public Repositories (read-only)") printed the true cause to stderr only, then still exited with the generic invalid-or-expired verdict. In --json mode the machine-readable error on stdout carried only the wrong cause, telling automation to rotate a token that works. - A network failure reaching GitHub (requests.RequestException) was also reported as an invalid PAT, sending operators off to mint a new token when the fix is to retry. Return (login, reason) from _validate_pat_locally() and surface the specific reason through _error() in both human and --json output. The same verdict-vs-transient distinction was applied to issue submissions in entrius#1554; this applies it to the PAT broadcast path. Update the mocked call sites and add regression tests for the three failure causes plus the --json envelope.
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.
PR body — tryeverything24 → entrius/gittensor
Branch: fix/miner-post-pat-error-reason (commit c663b3c, in WSL /root/gtn-pr)
Base: test
Title: fix(cli): report the real reason
gitt miner postrejects a PATSummary
_validate_pat_locally()collapses three different failures into a bareNone, sominer_postalways reports the same verdict — "GitHub PAT is invalid or expired" — evenwhen that's provably not the cause:
/userreturns 200, the GraphQL probefails — the default outcome for GitHub's fine-grained PATs without the
"Public Repositories (read-only)" permission (the token type GitHub now steers new users
toward). The true cause was printed to stderr only, then the command still exited with
the generic invalid-or-expired message. In
--jsonmode the machine-readable envelope onstdout carried only the wrong cause, telling automation to rotate a token that works.
requests.RequestException) was also reported as aninvalid PAT — sending operators off to mint a new token when the fix is to retry.
This is the same verdict-vs-transient distinction #1554 applied to
issues submissions(don't report a lookup failure as an empty result); this applies it to the PAT broadcast path.
Before / after (live capture, unreachable network)
Fix
_validate_pat_locally()now returns(login, reason):/usernon-200 (or no login in the response) → "invalid or expired" (unchanged wording)through
_error()in both human and--jsonoutput instead of a stderr-only printrequests.RequestException→ "Could not reach GitHub … retry", distinct from a bad tokenminer_postpasses the specific reason to_error(...). No behavior change on the successpath; the broadcast flow is untouched.
Tests
Updated the four existing mocked call sites for the new return shape, plus new
TestValidatePatLocallyintests/cli/test_miner_commands.py:test_invalid_token_reports_invalid— 401 → invalid/expiredtest_missing_graphql_access_reports_graphql_cause— 200 + GraphQL 403 → GraphQL message, NOT invalid/expiredtest_network_failure_reports_connectivity_not_invalid— RequestException → connectivity message, NOT invalid/expiredtest_valid_token_returns_login_and_no_errortest_post_json_mode_surfaces_specific_reason—--jsonenvelope carries the real cause