fix(github): stop the open-issues GraphQL supplement when endCursor is missing#8410
Conversation
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
…s missing supplementOpenIssuesFromGraphQl broke pagination on hasNextPage alone, so a GitHub response claiming another page with a null/absent endCursor serialized into a malformed 'after: undefined' query. That request fails, the function throws, and supplementUnderCountIfNeeded's catch discards the entire supplement attempt including every issue already fetched on prior pages. Mirrors the guard its sibling supplementOpenPullRequestsFromGraphQl already has (added two days later as a close copy and never backported), so the issues path degrades gracefully and keeps what it fetched. Adds a regression test that fails without the guard. Closes JSONbored#8312
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8410 +/- ##
==========================================
- Coverage 92.42% 89.92% -2.50%
==========================================
Files 791 98 -693
Lines 79250 23996 -55254
Branches 23946 4369 -19577
==========================================
- Hits 73243 21579 -51664
+ Misses 4868 2194 -2674
+ Partials 1139 223 -916
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-24 12:15:32 UTC
Review summary Nits — 2 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Summary
supplementOpenIssuesFromGraphQl(src/github/backfill.ts:1831) ended its pagination loop onhasNextPagealone. If GitHub reportshasNextPage: truewith a null/absentendCursor,JSON.stringify(undefined)serializes to the stringundefined, producing a malformed, after: undefinedfollow-up query. That request fails, the function throws, andsupplementUnderCountIfNeeded'scatchdiscards the entire supplement attempt — including every issue already fetched on prior pages — leaving only a generic warning.supplementOpenPullRequestsFromGraphQl(:1897) already has:if (!issues?.pageInfo?.hasNextPage || !issues.pageInfo.endCursor) break;— the exact same condition shape. The PR-side function was added two days after the issues one as a close copy and picked up the guard; it was never backported.return supplemented;keeps whatever prior pages accumulated — the graceful partial stop the sibling already achieves. No other logic change; the/* v8 ignore start/stop */scope is untouched.test/unit/backfill.test.tsalongside the existing sparse-payload supplement test: page 1 returnspageInfo: { hasNextPage: true, endCursor: null }with one issue, and the follow-upafter:-bearing request is stubbed to fail (502). With the guard that request is never issued and the segment completespartialwithfetchedCount: 1, the issue persisted; without the guard the malformed query fires, throws, and the whole supplement is discarded.Closes #8312
Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #123) — a linked open issue is required for every contributor PR.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocally;codecov/patchrequires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateIf any required check was skipped, explain why:
src/github/backfill.ts(one line) andtest/unit/backfill.test.ts, soactionlint(no workflow change),build:mcp/test:mcp-pack(no MCP change),ui:*(no UI/OpenAPI change),test:workers(no worker change), andnpm audit(no dependency change) are not exercised by it./* v8 ignore start … stop */block — the file's established convention for defensive GraphQL-payload-normalization code, matching the sibling PR-supplement function's identical treatment. It is therefore intentionally outside Codecov's branch-count gate and should not be read as uncovered patch code. The behavioral coverage is the real regression test above, which is exactly how this file already tests other ignored-block code (e.g. thefetchLiveReviewThreadBlockerscursor-guard test inbackfill-2.test.ts).test/unit/backfill.test.tspasses in full (133 tests). I also verified the new test genuinely pins the fix: reverting the one-line guard makes it fail, restoring it makes it pass. Roottsc --noEmitis clean for the changed files.Safety
UI Evidencesection below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.UI Evidence
Not applicable — backend-only pagination guard in
src/github/backfill.ts; no visible UI, frontend, docs, or extension change.Notes
fetchLiveReviewThreadBlockers'sif (!nextCursor || …) break;.