Skip to content

fix: treat transient GitHub PAT identity checks as inconclusive#1107

Merged
anderdc merged 3 commits into
entrius:testfrom
jakearmstrong59:fix/pat-handler-transient-user-failure
May 12, 2026
Merged

fix: treat transient GitHub PAT identity checks as inconclusive#1107
anderdc merged 3 commits into
entrius:testfrom
jakearmstrong59:fix/pat-handler-transient-user-failure

Conversation

@jakearmstrong59
Copy link
Copy Markdown
Contributor

Summary

Fix validator-side PAT broadcast/check handling for transient GitHub /user failures.

After #932, scoring uses validate_github_credentials_result() so transient GitHub identity lookup failures can be distinguished from invalid PATs. The validator PAT handlers still used the legacy (github_id, error) wrapper, which dropped transient_failure=True and treated temporary GitHub /user failures as normal validation errors.

Changes

  • Use validate_github_credentials_result() in PAT broadcast/check handlers.
  • Return a retry-oriented message for transient broadcast failures.
  • Return inconclusive check results for transient stored PAT validation:
    • has_pat = True
    • pat_valid = None
    • retry-oriented rejection_reason
  • Keep normal invalid PAT behavior unchanged.
  • Add regression tests for transient /user failures in both broadcast and check flows.

Testing

UV_CACHE_DIR=/tmp/uv-cache uv run python -m pytest tests/validator/test_pat_handler.py tests/validator/test_github_identity_fallback.py tests/validator/test_validator_cache_fallback.py tests/cli/test_miner_commands.py -q
UV_CACHE_DIR=/tmp/uv-cache uv run ruff check gittensor/ tests/
UV_CACHE_DIR=/tmp/uv-cache uv run ruff format --check gittensor/ tests/

Targeted pyright on the changed files with local venv paths: 0 errors.

Note: full validator-suite runs in this sandbox hit pre-existing hangs in mirror/issue-discovery tests before reaching this change.

Related

Closes #1106

@xiao-xiao-mao xiao-xiao-mao Bot added the bug Something isn't working label May 8, 2026
@jakearmstrong59 jakearmstrong59 force-pushed the fix/pat-handler-transient-user-failure branch from 2f2a006 to 3b1db7f Compare May 8, 2026 18:54
Copy link
Copy Markdown
Collaborator

@anderdc anderdc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please scope this down. Keep the check-handler change — pat_valid=None instead of False is the real semantic fix.

Drop the broadcast-handler change and its test: the _github_identity_retry_message helper, the new branch in handle_pat_broadcast, and test_transient_identity_lookup_rejected_with_retry_message. Both old and new broadcast paths reject with accepted=False and short-circuit before _test_pat_against_repo, so the new code only swaps the rejection string for a friendlier one — not enough to justify the added surface in PAT validation code.

…nto fix/pat-handler-transient-user-failure
@jakearmstrong59
Copy link
Copy Markdown
Contributor Author

Hi, @anderdc Could you please review again? Updated based on your feedback.
thank you

@anderdc anderdc merged commit c40de7a into entrius:test May 12, 2026
3 checks passed
Khaostica pushed a commit to Khaostica/gittensor that referenced this pull request May 21, 2026
Extend the transient-failure handling from entrius#1107 (identity check) to
`_test_pat_against_repo` (repo test query).

A GitHub 5xx burst or transport-layer hiccup (DNS, connection reset,
timeout) during the GraphQL test query currently flips `pat_valid` to
`False` and tells the miner their PAT failed — prompting them to rotate
a working PAT. The identity-check side of this same handler was already
fixed in entrius#1107 to surface transient failures as `pat_valid = None` with
a retry-oriented message; the test-query side kept the old "collapse
everything to invalid" behavior.

Changes:
- `_test_pat_against_repo` now returns a `PatTestResult` dataclass
  mirroring `GitHubCredentialValidation`, distinguishing transient
  failures (HTTP 5xx, `requests.RequestException`) from permanent ones
  (4xx, GraphQL errors, viewer:null scope rejection).
- `handle_pat_check` surfaces transient test-query failures as
  `pat_valid = None` with the same retry message established by entrius#1107
  for the identity path.
- `handle_pat_broadcast` rejects on transient with a retry-oriented
  message and does not store the PAT (write was already gated on
  success, so no behavior change there — the message just stops
  pointing at the PAT as the culprit).
- Tests updated for the new return type; new regression tests cover
  the transient 5xx path, transport-error path, and broadcast retry
  message.
jakearmstrong59 added a commit to jakearmstrong59/gittensor that referenced this pull request May 29, 2026
…r check

After entrius#1107 a validator emits has_pat=True, pat_valid=None on a transient
GitHub failure, but _pat_check_row_category had no branch for it and
collapsed it into 'no_response' — indistinguishable from a validator that
never answered, in both the table and --json output.

Add an 'inconclusive' category in the categorizer, the aggregate-counts
dict, and the check.py status-markup map (the table does a bare
_PAT_CHECK_STATUS_MARKUP[category] lookup, so the markup entry is required
to avoid a KeyError). success = valid_count > 0 is unchanged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] pat_handler treats transient GitHub /user failures as invalid PATs in validator-side broadcast/check handlers

2 participants