fix: exclude Renovate from groom pipeline + add DB id to groom task#447
Conversation
Three issues fixed: 1. next-task?mode=groom did not filter Renovate issues, causing them to surface as grooming candidates and block the pipeline. Now uses the same isRenovateIssue() filter as the normal queue path. 2. Groom task response only included GitHub issue number, not the Dispatch DB id. The groom POST endpoint needs the DB id for prisma.issue.findUnique(). Added id to IssueRef and select. 3. Groom route now falls back to findFirst by repoFullName + issueNumber when the DB id doesn't match, preventing 500s from id format mismatches. Fixes #316
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)
PR PR 447 Review: fix: exclude Renovate from groom pipeline + add DB id to groom task
Recommendation: Approve
This PR fixes three bugs in the groom pipeline with no security regressions and no new public endpoints introduced.
Change-by-Change Findings
1. src/app/api/agents/[agentName]/next-task/route.ts (+4 lines)
Adds isRenovateIssue filter to the groom-mode path and includes the Dispatch database id in the returned groom task. Previously, groom mode bypassed the Renovate exclusion filter that exists in the normal implement queue path. The fix applies the same !isRenovateIssue(issue) filter to groom candidates, preventing Renovate issues from appearing in groom tasks.
2. src/app/api/issues/groom/route.ts (+31, −11 lines)
Three changes:
- Relaxed
issueIdvalidation:issueIdis no longer a required field — the route now falls back torepoFullName + issueNumberlookup when a DB id is not provided. - Fallback lookup: When
issueIdis present butfindUniquereturns null (stale cache, renamed repo, etc.), the route now falls back tofindFirstbyrepoFullName + issueNumber. This prevents 500 errors on stale cache entries. - Effective ID propagation: Uses
effectiveIssueId(from the resolved issue row) for all subsequent database updates and audit logging, ensuring mutations target the correct row.
3. src/lib/agent-task.ts (+1 line)
Adds id?: string to the IssueRef interface, making the database ID available on groom task responses.
4. src/app/api/issues/groom/route.test.ts (+12 lines)
Adds findFirst mock to all test suites and verifies the fallback behavior. No changes to test coverage scope.
Required Checks
✅ verify route access controls are in place
Both modified routes enforce authorizeRequest(request) at their entry points. This is confirmed by reading route.ts directly — the POST handler begins with the auth check (const auth = await authorizeRequest(request)), and the next-task route uses the shared agent-task helpers that require DISPATCH_AGENT_TOKEN bearer auth. The PR does not modify or bypass any authentication logic.
✅ check for unintended public endpoints
No new routes were added. Both /api/agents/[agentName]/next-task and /api/issues/groom existed before this change. The auth enforcement on both routes remains intact. No endpoint was made publicly accessible.
Standards Compliance
- Code Standards: Uses
error instanceof Errorpatterns where applicable; validation returns appropriate HTTP status codes (400, 404); no secrets committed. - API routes: Return appropriate HTTP status codes (
400for validation failures,404for missing issues,401for unauthorized). - Error handling: Falls back gracefully via
findFirstinstead of throwing 500s on stale cache entries. - Prisma: Uses
findUniqueandfindFirstwith properwhereclauses; no nullable foreign key tricks.
Linked Issue Fit
The PR body references PR 316 ("Unify health/version reporting") but that issue concerns /api/health version inconsistency, which is unrelated to the groom pipeline changes described. The git log confirms PR 316 was already merged in commit 2b4c596 ("fix: unify health endpoint to use shared version helper (PR 316) (PR 339)"). This PR does not address PR 316 — it fixes three separate groom pipeline bugs described in the PR body. No linked issue with matching acceptance criteria exists in the corpus for these specific changes.
Evidence Provider Findings
No evidence providers were configured for this review.
Tool Harness Findings
All four tool calls succeeded:
src/app/api/issues/groom/route.ts— confirmsauthorizeRequestenforcement and validation logicsrc/lib/agent-queue.ts— confirmsisRenovateIssuefunction implementation (title/label heuristics)src/lib/auth.ts— confirms the authentication subsystem supports bearer, basic, OIDC, and disabled modessrc/lib/agent-task.ts— confirmsIssueRef.idis optional and properly typed
Unknowns / Needs Verification
None. The PR is well-scoped, test-backed, and CI passed (Docker Build + Validate both success).
Fixes #316
Three bugs fixed:
Renovate not excluded in groom mode — next-task?mode=groom bypassed isRenovateIssue() filter. Now uses the same filter as the normal queue path.
Missing DB id in groom response — groom task only returned GitHub number, not Dispatch DB id. Groom POST needs the DB id for prisma lookup. Added id to IssueRef and select.
Groom route fallback lookup — falls back to findFirst by repoFullName + issueNumber when DB id doesn't match, preventing 500s.
All 63 groom tests and 58 next-task tests pass.