diff --git a/gittensor/cli/issue_commands/helpers.py b/gittensor/cli/issue_commands/helpers.py index 84eb1eda..4a5a0da0 100644 --- a/gittensor/cli/issue_commands/helpers.py +++ b/gittensor/cli/issue_commands/helpers.py @@ -13,7 +13,7 @@ from contextlib import nullcontext from decimal import Decimal, InvalidOperation from pathlib import Path -from typing import Any, Callable, ContextManager, Dict, List, Optional, Tuple, TypeVar +from typing import Any, Callable, ContextManager, Dict, List, NoReturn, Optional, Tuple, TypeVar import click import requests @@ -197,7 +197,7 @@ def print_warning(message: str) -> None: err_console.print(f'\n[yellow]{message}[/yellow]\n', highlight=True) -def handle_exception(as_json: bool, message: str, error_type: str = 'cli_error') -> None: +def handle_exception(as_json: bool, message: str, error_type: str = 'cli_error') -> NoReturn: """Emit a CLI error in JSON or human format and exit non-zero.""" if as_json: emit_error_json(message, error_type=error_type) @@ -254,8 +254,13 @@ def fetch_open_issue_pull_requests( repository_full_name: str, issue_number: int, as_json: bool, -) -> list: - """Fetch open PR submissions for a GitHub issue.""" +) -> Optional[list]: + """Fetch open PR submissions for a GitHub issue. + + Returns a (possibly empty) list of PRs, or ``None`` when the GitHub lookup + fails. Callers must treat ``None`` as a failure (not "no submissions"); see + ``find_prs_for_issue``. + """ token = os.environ.get('GITTENSOR_MINER_PAT') or '' if not token and not as_json: print_warning('No GitHub token found; set GITTENSOR_MINER_PAT to fetch GitHub issue submissions') @@ -270,7 +275,8 @@ def fetch_open_issue_pull_requests( token=token or None, open_only=True, ) - # Intentionally return GitHub tool output as-is (no CLI schema mapping yet). + # Intentionally return GitHub tool output as-is (no CLI schema mapping yet); + # this includes the None failure sentinel, which the caller must handle. return prs except Exception as e: raise click.ClickException(f'Failed to fetch PR submissions from GitHub: {e}') diff --git a/gittensor/cli/issue_commands/submissions.py b/gittensor/cli/issue_commands/submissions.py index 560c9291..9d7dbfb9 100644 --- a/gittensor/cli/issue_commands/submissions.py +++ b/gittensor/cli/issue_commands/submissions.py @@ -81,6 +81,17 @@ def issues_submissions( except click.ClickException as e: handle_exception(as_json, str(e), click_error_type(e)) + # None is the GitHub lookup-failure sentinel (rate limit, network/GraphQL + # error). Surface it as an explicit error instead of reporting an empty + # submission list, which would be a false negative for monitoring/automation. + if pull_requests is None: + handle_exception( + as_json, + f'GitHub lookup failed for {repo_name}#{issue_number}; ' + 'submissions could not be determined. Please retry.', + 'github_lookup_failed', + ) + if as_json: submissions = [ { diff --git a/gittensor/utils/github_api_tools.py b/gittensor/utils/github_api_tools.py index 803969d1..8726cadf 100644 --- a/gittensor/utils/github_api_tools.py +++ b/gittensor/utils/github_api_tools.py @@ -329,14 +329,26 @@ def find_prs_for_issue( issue_number: int, open_only: bool = True, token: Optional[str] = None, -) -> List[PRInfo]: - """Find PRs that reference an issue via GraphQL cross-reference data.""" +) -> Optional[List[PRInfo]]: + """Find PRs that reference an issue via GraphQL cross-reference data. + + Returns a (possibly empty) list on success, or ``None`` when the GraphQL + lookup fails (rate limit, network error, GraphQL errors, missing issue + payload, or an exception). ``None`` is a failure sentinel that callers must + distinguish from ``[]`` ("no referencing PRs exist") — mirroring the + ``solver_lookup_failed`` signaling in ``find_solver_from_closure_event`` / + ``check_github_issue_closed``. The empty-list return for a falsy token is a + precondition-not-met case, not a transient failure. + """ if token: try: - prs = _search_issue_referencing_prs_graphql(repo, issue_number, token, open_only=open_only) - return prs or [] + # Propagate the None failure sentinel from the GraphQL helper as-is; + # collapsing it to [] would make a lookup failure indistinguishable + # from a genuinely empty submission list. + return _search_issue_referencing_prs_graphql(repo, issue_number, token, open_only=open_only) except Exception as exc: bt.logging.debug(f'GraphQL PR fetch failed for {repo}#{issue_number}: {exc}') + return None return [] diff --git a/tests/cli/test_issue_submission.py b/tests/cli/test_issue_submission.py index 7f6b099a..08a8adff 100644 --- a/tests/cli/test_issue_submission.py +++ b/tests/cli/test_issue_submission.py @@ -112,6 +112,49 @@ def test_submissions_human_no_open_prs_message(cli_root, runner, sample_issue): assert 'No open submissions available' in result.output +def test_submissions_json_lookup_failure_returns_structured_error(cli_root, runner, sample_issue): + """A GitHub lookup failure (find_prs_for_issue -> None, propagated as None by + fetch_open_issue_pull_requests) must surface as `success: false` with a + `github_lookup_failed` type — not a false-negative `submission_count: 0`.""" + with ( + patch('gittensor.cli.issue_commands.submissions.get_contract_address', return_value='0xabc'), + patch('gittensor.cli.issue_commands.submissions.resolve_network', return_value=('ws://x', 'test')), + patch('gittensor.cli.issue_commands.submissions.fetch_issue_from_contract', return_value=sample_issue), + patch('gittensor.cli.issue_commands.submissions.fetch_open_issue_pull_requests', return_value=None), + ): + result = runner.invoke( + cli_root, + ['issues', 'submissions', '--id', '42', '--json'], + catch_exceptions=False, + ) + + assert result.exit_code != 0 + payload = json.loads(result.stdout) + assert payload['success'] is False + assert payload['error']['type'] == 'github_lookup_failed' + assert 'submission_count' not in payload + + +def test_submissions_human_lookup_failure_errors_instead_of_no_submissions(cli_root, runner, sample_issue): + """In human mode a lookup failure must error out, not print the misleading + 'No open submissions available' message used for a genuinely empty list.""" + with ( + patch('gittensor.cli.issue_commands.submissions.get_contract_address', return_value='0xabc'), + patch('gittensor.cli.issue_commands.submissions.resolve_network', return_value=('ws://x', 'test')), + patch('gittensor.cli.issue_commands.submissions.fetch_issue_from_contract', return_value=sample_issue), + patch('gittensor.cli.issue_commands.submissions.fetch_open_issue_pull_requests', return_value=None), + ): + result = runner.invoke( + cli_root, + ['issues', 'submissions', '--id', '42'], + catch_exceptions=False, + ) + + assert result.exit_code != 0 + assert 'No open submissions available' not in result.output + assert 'GitHub lookup failed' in result.output + + def test_submissions_json_contract_read_failure_returns_structured_error(cli_root, runner): """`fetch_issue_from_contract` now converts contract-read failures to a `ClickException`, which `submissions` routes through `handle_exception` — diff --git a/tests/utils/test_github_api_tools.py b/tests/utils/test_github_api_tools.py index 5ed4c3ad..103c283b 100644 --- a/tests/utils/test_github_api_tools.py +++ b/tests/utils/test_github_api_tools.py @@ -172,12 +172,26 @@ def test_find_prs_returns_empty_when_graphql_empty(mock_graphql): @patch('gittensor.utils.github_api_tools._search_issue_referencing_prs_graphql') -def test_find_prs_returns_empty_when_graphql_errors(mock_graphql): +def test_find_prs_returns_none_when_graphql_errors(mock_graphql): + # An exception during the GraphQL lookup is a failure, not "no PRs" — it must + # surface as the None sentinel so callers can distinguish it from []. mock_graphql.side_effect = RuntimeError('boom') result = find_prs_for_issue('owner/repo', 12, open_only=True, token='fake_token') - assert result == [] + assert result is None + mock_graphql.assert_called_once_with('owner/repo', 12, 'fake_token', open_only=True) + + +@patch('gittensor.utils.github_api_tools._search_issue_referencing_prs_graphql') +def test_find_prs_returns_none_when_graphql_lookup_fails(mock_graphql): + # The GraphQL helper returns None on rate limit / network / GraphQL errors; + # find_prs_for_issue must propagate it rather than collapsing to []. + mock_graphql.return_value = None + + result = find_prs_for_issue('owner/repo', 12, open_only=True, token='fake_token') + + assert result is None mock_graphql.assert_called_once_with('owner/repo', 12, 'fake_token', open_only=True)