fix(sync): fetch closed issues so closed⇒done enforcement actually runs#544
Conversation
closedIssueStatusFix (#521) only applies to issues in the sync fetch set, and every caller fetched state=open — so closed issues never flowed through it and kept stale claimable labels (a closed issue sat status/ready and got claimed by the bridge). The gap was masked by the heartbeat cron, whose reconcileClosedIssues did its own includeClosed fetch; retiring it (in-app scheduler cutover) exposed this. All three sync callers now fetch state=all.
There was a problem hiding this comment.
AI Automated Review
Full PR review.
Analysis engine: MiniMax-M2.7@https://litellm.jory.dev/v1 (anthropic) — routed smart (risk match: public_route_changes)
Recommendation: Approve
This PR fixes a regression where closed issues were not being fetched during sync operations, causing the closed=>done enforcement (from PR 521) to silently not run. The fix consistently applies includeClosed: true across all three sync entry points.
Change-by-Change Findings
src/app/api/sync/route.ts
- Added
fetchAllStateIssueswrapper that passes{ includeClosed: true }tofetchIssues - Replaced direct
fetchIssuesusage withfetchAllStateIssuesinsyncIssuesForReposcall - Comment explains the regression: heartbeat cron previously handled this via its own
reconcileClosedIssues, but that code path was retired
src/app/api/sync/scheduled/route.ts
- Identical fix:
fetchAllStateIssueswrapper withincludeClosed: true - Replaces direct
fetchIssuesin the scheduled sync path
src/lib/heartbeat.ts
- Identical fix for the heartbeat sync path
src/app/api/sync/scheduled/route.test.ts
- New regression test verifies
fetchIssuesis called with{ includeClosed: true }
Required Checks
✅ verify route access controls are in place
The modified sync routes (/api/sync and /api/sync/scheduled) both retain their existing authorizeRequest guards:
if (!(await authorizeRequest(request)).authorized) {
return NextResponse.json({ error: "Unauthorized" }, { status: 401 });
}No changes were made to authentication or authorization logic. The heartbeat.ts library is not a route; it is called by authorized callers and does not expose endpoints directly.
✅ check for unintended public endpoints
No new routes are introduced. The diff touches only:
- Two existing API route handlers (path unchanged)
- One internal library module (
heartbeat.ts)
The PR is a pure logic fix (fetch closed issues so enforcement can run) with no routing surface changes.
Standards Compliance
- Code Standards: No violations. Error handling uses
instanceof Errorpatterns where applicable. No secrets committed. - API routes: Existing routes return appropriate 401 status codes.
- No Prisma schema changes.
- Tests pass, lint passes, typecheck passes, build passes (authoritative CI results).
Linked Issue Fit
This PR directly addresses the root cause described in the PR body: closed issues were stuck at status/ready (claimable) because the sync fetch was state=open. Issues PR 510 and PR 507 (mentioned in the body as examples of the problem) are unrelated refactoring work but serve as evidence that closed issues can incorrectly remain claimable.
The fix restores the intended behavior from PR 521 ("feat(sync): closed issues always move to status/done") by ensuring closed issues are included in the sync fetch set where the enforcement logic runs.
Evidence Provider Findings
No evidence providers configured.
Tool Harness Findings
No tool calls were issued; reviewing corpus directly.
Unknowns or Needs Verification
None. The fix is well-documented with inline comments explaining the regression root cause, includes a regression test, and all CI checks pass.
Summary
syncIssuesForReposcallers (scheduled/manual/heartbeat) now fetch with{ includeClosed: true }.closedIssueStatusFix(#521) only runs on issues in the fetch set — and the fetch wasstate=open, so closed issues never passed through it. Seen in prod: closed #510 stuck atstatus/ready(claimable — the bridge actually claimed closed #507 this way). The gap was masked by the heartbeat cron'sreconcileClosedIssues(which did its ownincludeClosedfetch); retiring the heartbeat in the scheduler cutover exposed it.Verification
vitest runon sync/heartbeat/issue-sync → 80 passed (incl. new regression test assertingincludeClosed: true);tsc+eslintclean.Notes
as anyon $transaction adapter #510) tostatus/doneon GitHub — expected, self-limiting.state=allpaginates full issue history per repo per sync. Fine at current repo sizes; if it ever matters, switch to a bounded recently-updated-closed fetch likefetchClosedPullRequests.