Skip to content

fix(github): harden cross-reference PR parser against null GraphQL fields#1575

Open
codevector96 wants to merge 1 commit into
entrius:testfrom
codevector96:fix/harden-graphql-null-fields
Open

fix(github): harden cross-reference PR parser against null GraphQL fields#1575
codevector96 wants to merge 1 commit into
entrius:testfrom
codevector96:fix/harden-graphql-null-fields

Conversation

@codevector96

Copy link
Copy Markdown

Summary

_search_issue_referencing_prs_graphql read the PR base repo with
pr.get('baseRepository', {}).get('nameWithOwner', ''). The {} default only
applies when the key is absent — GitHub's GraphQL API returns the key
present with an explicit null (e.g. when a referencing PR's base repository
was deleted or is inaccessible). In that case the chained .get runs on None
and raises AttributeError.

find_prs_for_issue wraps this call in except Exception -> return None
(the lookup-failure sentinel). So a single unresolvable timeline node made the
entire issue's submission lookup report as "GitHub lookup failed" instead of
skipping that one node and returning the valid PRs — the exact failure mode the
recent read-failed/empty-submissions work (#1492/#1554) was tightening.

The sibling closure-event helpers in the same file already guard these nulls
correctly (_select_current_close_event uses if node and ... and
.get('nodes', []) or []; _solver_from_closed_event uses
((closer.get('baseRepository') or {}).get('nameWithOwner') or '')). This PR
brings the cross-reference parser in line with that convention:

  • null timelineItems → treated as no nodes
  • null timeline node → skipped
  • null baseRepository → skipped (not fatal)

Related Issues

None — surfaced while reading the issue-submission lookup path.

Type of Change

  • Bug fix

Testing

Adds TestSearchIssueReferencingPRsGraphQL covering each null path directly:
a baseRepository: null PR node is skipped while valid PRs still return, a
null timeline node is skipped, and a null timelineItems payload yields []
instead of raising. Existing tests unchanged. ruff check / ruff format
clean.

Checklist

  • Follows repository conventions (mirrors the null-guarding already used by the sibling closure-event helpers)
  • Self-reviewed
  • No unrelated files touched (1 source file + its test)

…elds

`_search_issue_referencing_prs_graphql` read `baseRepository` with
`pr.get('baseRepository', {}).get('nameWithOwner', '')`. GitHub's GraphQL
API returns the key present with an explicit `null` (e.g. when a referencing
PR's base repo was deleted or is inaccessible), so the chained `.get` raised
`AttributeError`. `find_prs_for_issue` swallows that into the lookup-failure
sentinel, so one unresolvable timeline node made the whole issue's submission
lookup report as failed instead of skipping that node.

Mirror the null-guarding the sibling closure-event helpers already use
(`_select_current_close_event`, `_solver_from_closed_event`): guard null
timeline nodes, null `timelineItems`, and null `baseRepository`.

Adds regression tests exercising each null path directly.
@xiao-xiao-mao xiao-xiao-mao Bot added the bug Something isn't working label Jul 3, 2026
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.

1 participant