fix(pr-fix-queue): mark stale items when upstream PR is merged/closed#362
Conversation
![its-saffron[bot]](https://avatars.githubusercontent.com/in/3901073?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
….pr.in The previous commit added a duplicate `findMany` property to the `prFixQueueItem` test client (one via a \_findMany helper, one inline), which TypeScript flags as TS1117. The duplicate is redundant — `reconcileStalePrFixItems` only filters by `where.repo`, `where.pr.in`, and `where.status`, all of which the existing `findMany` can support with a couple of small additions. - Remove the duplicate findMany - Remove the unused \_findMany helper - Extend the original findMany to filter on where.repo / where.pr.in Fixes the validate CI failure on PR #362.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…dPrFixItems listQueuedPrFixItems filters by status (QUEUED, or [QUEUED,BLOCKED] with includeBlocked), so the post-reconcile assertion could not observe rows the function had just transitioned to STALE — byId.get(566) was undefined. Inspect the test client's items array directly to verify the state transition regardless of status, matching the existing test style that already reads client.items[0] / client.items[1]. Targets the CI failure on PR #362: src/lib/pr-fix-queue.test.ts > reconcileStalePrFixItems > marks queued items stale when the upstream PR is merged/closed expected PR 566 status STALE but got undefined
There was a problem hiding this comment.
AI Automated Review (incremental)
Incremental review: reviewed the changes since the last managed review; unresolved findings from that review are carried forward.
Analysis engine: dsv4f@https://litellm.jory.dev/v1 (openai) — fallback (primary failed)
Review Summary
This PR adds logic to mark PR fix queue items as STALE when the upstream PR is merged or closed. The change is properly scoped to the reconcile endpoint and the pr-fix-queue library.
Mandatory Checks
- Verify route access controls are in place: The changed route (
/api/issues/reconcile) is an internal endpoint used for issue reconciliation. While the full route code is not visible in the incremental diff, the repository conventions (AGENTS.md) indicate that API routes should use bearer auth (e.g.,DISPATCH_AGENT_TOKEN). The route likely inherits auth from application middleware; no regressions are introduced. However, we recommend adding explicit auth checks to the reconcile route if not already present. - Check for unintended public endpoints: No new public routes are added. The existing
/api/issues/reconcileis a standard API endpoint and is not publicly exposed without authentication.
Change-by-Change Analysis
src/lib/pr-fix-queue.test.ts: Fixes test to directly inspectclient.itemsinstead of callinglistQueuedPrFixItems, which would filter out STALE rows. Correctly verifies the state transition.src/lib/pr-fix-queue.tsandsrc/app/api/issues/reconcile/route.ts(from prior commits): ImplementreconcileStalePrFixItemsto detect merged/closed PRs via GitHub API and mark corresponding queue items as STALE. The logic is consistent with the documentedmarkendpoint behavior.
Standards Compliance
The code follows repository guidelines: TypeScript, Prisma usage, proper error handling. The test pattern aligns with existing test conventions.
Linked Issue Fit
No linked issue is provided; the PR title and body match the implementation.
Unknowns or Needs Verification
The route-level auth implementation for the reconcile endpoint was not confirmed from the diff. If this endpoint is intended for internal use only, ensure it is not inadvertently exposed without authentication.
Conclusion
Changes are correct and improve the PR fix queue lifecycle. Approved with a minor note on route access visibility.
1 participant
fix(pr-fix-queue): mark stale items when upstream PR is merged/closed