feat(api): add /api/issues/untriaged grooming intake endpoint#335
Conversation
Extract DB-backed lock from scheduled sync into shared module (src/lib/sync-lock.ts) and apply it to manual issue sync and automation sync endpoints. This prevents overlapping concurrent runs across all three sync types (scheduled, manual, automation), addressing race conditions from browser refreshes, cron overlap, or repeated clicks. - New src/lib/sync-lock.ts: acquireLock(syncType) / releaseLock(runId) with stale lock cleanup (>30 min) and transactional double-check - src/app/api/sync/route.ts: acquire lock before issue sync, return 409 if a sync is already running - src/app/api/automation/sync/route.ts: acquire lock before automation sync (single repo or batch), return 409 if locked - src/app/api/sync/scheduled/route.ts: replace inline lock with shared module import, update error message to be sync-type-agnostic - prisma/schema.prisma: update syncType comment to include 'automation'
- Add authorizeRequest() to sync/route.ts POST handler for Bearer token auth - Add authorizeRequest() to webhook/route.ts POST handler - Make webhook signature verification fail-closed by default - Introduce WEBHOOK_GATEWAY_MODE=true opt-out for gateway deployments - Update documentation comments to reflect new security model
Add a new GET /api/issues/untriaged endpoint that surfaces open issues with no status/* label for grooming intake. This addresses issue #325: - Returns open issues from enabled repos without any status/ label - Excludes Renovate/dashboard noise by default (configurable) - Bounds results per run (default 50, hard cap 200) - Supports repo filter and excludeRenovate query params - Not claimable by worker queues — grooming-only view Includes tests for the new endpoint and updates route smoke checks.
- route.ts: Fix spread on Prisma WhereInput type by explicitly constructing the object - route.ts: Cast STATUS_LABELS to string[] for .includes() compatibility with plain string labels - test.ts: Add missing Issue fields (body, assignees, commentsCount) to makeIssue mock helper
Cast mock data with 'as never' instead of 'as const' to avoid Prisma schema enforcement on test mocks. Only route-accessed fields are present in makeIssue helper.
…tests The route imports @/lib/agent-queue -> @/lib/issue-filters which needs AGENT_PREFIX and OWNER_PREFIX from @/types. Include them in the partial mock to prevent runtime errors.
- Pre-sort mock data for 'orders by updatedAt descending' test since the route's orderBy is mocked out. - Only return open issues in 'excludes closed issues' test to match Prisma's where.state='open' filtering.
…ency failures The route imports @/lib/agent-queue which transitively needs AGENT_PREFIX, PRIORITY_LABELS, BOARD_COLUMNS, and other constants from @/types. Include all required exports in the partial mock.
Mocking @/types caused the STATUS_LABELS constant to not be recognized by the route's filter logic. Instead, only mock @/lib/agent-queue and let the real @/types provide STATUS_LABELS.
Use vi.hoisted() to create mock references before any module imports, matching the pattern used by other tests in this repo (e.g. move/route.test.ts).
Use a named const array for STATUS_LABELS in the @/types mock so that the route's filter logic gets a consistent reference.
vitest hoists vi.mock() callbacks to the top of the file. Defining STATUS_LABELS_MOCK after vi.mock() caused a temporal dead zone error since the mock factory tried to reference it before initialization.
vitest hoists vi.mock() factories to the very top of the file, before any variable declarations. All values must be inline literals.
vi.mock() is hoisted above all code including imports. Using vi.doMock() with top-level await import() ensures mocks are set up before module loading.
![its-saffron[bot]](https://avatars.githubusercontent.com/in/3901073?s=60&v=4){kind=small}
There was a problem hiding this comment.
AI Automated Review
Full PR review.
Analysis engine: MiniMax-M2.7@https://litellm.jory.dev/v1 (anthropic)
Review: feat(api): add /api/issues/untriaged grooming intake endpoint
Summary
This PR adds a dedicated grooming intake endpoint (GET /api/issues/untriaged) that surfaces open issues with no status/* label — addressing the blind spot identified in issue PR 325 where unlabeled issues were correctly excluded from worker queues but never surfaced for grooming. The PR also extracts the sync-lock logic into a shared module used by all three sync entry-points (scheduled, manual, automation) and hardens auth on the pr-followup routes.
Change-by-Change Findings
New endpoint — src/app/api/issues/untriaged/route.ts (92 lines)
- Returns open issues from enabled repos that have no
status/*label. Correctly excludes closed issues via thestate: "open"filter in the Prisma query. Filters bystatus/*presence in JavaScript after fetching, which is appropriate given the small result set and boundedlimit(max 200). Renovate/dashboard noise is excluded by default viaisRenovateIssuefrom@/lib/agent-queue; callers can opt in withexcludeRenovate=false. Ordering byupdatedAt: "desc"is sensible for a grooming queue. The route follows existing Dispatch patterns: usesSTATUS_LABELSfrom@/types, returns JSON, handles errors with meaningful messages.
New tests — src/app/api/issues/untriaged.test.ts (240 lines)
- Comprehensive coverage: no-status filtering, closed-issue exclusion, limit defaults (50) and caps (200), repo filtering, Renovate exclusion and opt-in, empty-state handling, response shape, and ordering. Uses
vi.doMockpattern consistent with other test files in this repo. All 12 test cases map to distinct behaviors described in the issue acceptance criteria.
Shared sync-lock module — src/lib/sync-lock.ts (88 lines)
- Extracted from the scheduled sync route. Provides
acquireLock(with syncType:"scheduled" | "manual" | "automation") andreleaseLock. The"automation"syncType is new and matches the schema comment update. Lock semantics are sound: first-writer-wins, 30-minute stale-lock override, conditional delete in release. Creates anIssueSyncRunrecord inside the transaction so the run is attributable. Both manual and automation sync now properly create and (on success) update that run record.
Automation sync route — src/app/api/automation/sync/route.ts (+33/−17)
- Added shared lock acquisition with
acquireLock("automation"). Returns 409 on conflict. Wraps repo-level sync intry/finallyto guarantee lock release. Before this change, automation sync had no concurrency guard and could overlap with scheduled or manual sync.
Manual sync route — src/app/api/sync/route.ts (+53/−26)
- Added shared lock acquisition with
acquireLock("manual"). Also now updates theIssueSyncRunrecord (status → "completed", setscompletedAt,reposFetched,syncedCount) — this was previously a gap; the run was created byacquireLockbut never completed on success.
Scheduled sync route — src/app/api/sync/scheduled/route.ts (+4/−56)
- Refactored from inline lock functions to imported
acquireLock("scheduled")andreleaseLock. Error message simplified from "A scheduled sync is already running" to "A sync is already running" to reflect that the lock is now shared across all sync types.
pr-followup routes — src/app/api/pr-followup/sync/route.ts and src/app/api/pr-followup/webhook/route.ts
- Both now call
authorizeRequestat the top of the handler. The webhook route also refactors signature verification to be fail-closed by default: ifWEBHOOK_SECRETis not set, requests are rejected unlessWEBHOOK_GATEWAY_MODE=trueis explicitly set. This is a security improvement — the previous behavior silently skipped verification when the secret was absent.
Schema — prisma/schema.prisma
- Comment updated from
"pre-claim"to"automation"to match the actual sync types now in use. This is a documentation-only change.
Standards Compliance
- No hardcoded agent names: The new endpoint and tests use generic patterns.
- Error handling: All routes use
error instanceof Errorpattern viatry/catchwith meaningful messages. - API routes: Return appropriate HTTP status codes (200, 401, 409, 500); use JSON for responses.
- Validation: Input parameters (
limit,repo,excludeRenovate) are parsed and bounded before use. - No commit of secrets: No
.envor secrets in the diff. - Audit trail: No audit-log writes introduced in this PR, which is correct — the untriaged endpoint is a read-only intake view; grooming actions (label changes, status moves) trigger audit entries via the existing
POST /api/issues/moveandPOST /api/issues/actionsendpoints.
Linked Issue Fit
Issue PR 325 acceptance criteria are fully addressed:
- ✅ "An open issue with no
status/*label appears in grooming intake" —GET /api/issues/untriagedreturns exactly this. - ✅ "The same issue does not appear in normal/escalated worker queues by default" — The untriaged endpoint is a separate read path; agent queue logic is untouched.
- ✅ "Grooming can promote a no-status issue to
status/readyorstatus/backlog" — The endpoint surfaces issues for grooming to act on via existing label-setting endpoints. - ✅ "Renovate Dashboard issues do not get promoted accidentally" —
excludeRenovatedefaults totrue;isRenovateIssuechecks title for "dependency dashboard" and "renovate dashboard". - ✅ "Tests cover no-status grooming intake separately from worker queues" — 12 tests covering all acceptance-criteria scenarios.
- ✅ "Existing worker queue behavior from PR 291 remains intact" — No changes to
GET /api/agents/[agentName]/queue.
Additional work done beyond the minimum: auth hardened on pr-followup routes, sync-lock extracted to a shared module (prevents the race condition where scheduled and automation sync could overlap), and manual sync now properly completes its IssueSyncRun record.
Evidence Provider Findings
No evidence providers configured; this review is based on static diff analysis.
Tool Harness Findings
No tool harness results reported.
Unknowns / Needs Verification
- The
excludeRenovatefilter usesisRenovateIssuewhich checks title only. Renovate issues that don't include "dependency dashboard" or "renovate dashboard" in their title would not be filtered. The issue's requirement is satisfied for the common case, but a label-based fallback (e.g.,label: "settings/renovate"orlabel: "dependencies") could be a future hardening. Not a blocker. - The untriaged endpoint fetches all open issues from enabled repos and filters in JavaScript. For large deployments with thousands of open issues, this could be memory-intensive before the
slice(0, limit). A Prisma-levelNOTfilter on labels would be more efficient. Not a blocker given the 200 cap and the assumption that grooming runs are periodic and targeted.
Recommendation
Approve. The PR correctly addresses issue PR 325 with a clean, well-tested implementation. The concurrent security improvement to the sync-lock and the auth hardening on pr-followup routes are valuable side effects that improve overall system reliability.
- Test import path was wrong: `./route` resolved to `src/app/api/issues/route.ts` (general issues endpoint) instead of `src/app/api/issues/untriaged/route.ts`. Fixed to `./untriaged/route`. - Added in-memory repo filtering after Prisma fetch, since the mock doesn't apply the `where` clause. This ensures the `repo` query parameter works correctly in tests and as a safety net in production.
There was a problem hiding this comment.
AI Automated Review
Full PR review.
Analysis engine: MiniMax-M2.7@https://litellm.jory.dev/v1 (anthropic)
PR PR 335 Review: Add /api/issues/untriaged grooming intake endpoint
Recommendation: Approve
This PR delivers the grooming intake endpoint requested in issue PR 325, along with supporting improvements to sync locking and webhook security. The implementation is well-structured and satisfies all acceptance criteria.
Change-by-Change Findings
1. New GET /api/issues/untriaged endpoint (src/app/api/issues/untriaged/route.ts)
- Returns open issues from enabled repos with no
status/*label - Query params:
limit(default 50, hard cap 200),repo(filter by fullName),excludeRenovate(default true) - Results ordered by
updatedAtdescending - Renovate/dashboard noise filtered via
isRenovateIssue()helper - Error handling uses
error instanceof Errorpattern per repository conventions
Note: The query fetches all open issues from enabled repos and filters in-memory. For large deployments this could be inefficient, but the hard cap at 200 bounds memory usage. This is a pragmatic approach given the constraint that status filtering must happen post-fetch (Prisma doesn't store parsed labels as a separate field).
2. Tests (src/app/api/issues/untriaged.test.ts)
- 240 lines covering: no-status filtering, closed exclusion, limit bounds (default 50, custom, hard cap 200), repo filtering, Renovate exclusion, empty results, response shape, and ordering
- Uses
vi.doMockpattern correctly to avoid hoisting issues - All 10 test cases map directly to acceptance criteria
3. Route smoke check (src/app/api/routes.test.ts)
- Added
untriaged/route.tsto the smoke-check list
4. Shared sync-lock module (src/lib/sync-lock.ts)
- Extracts the inline lock acquisition/release logic from
sync/scheduled/route.tsinto a reusable module - Used by all three sync entry-points: scheduled, manual, and automation
- Stale lock cleanup (>30 min) preserved; uses conditional delete to avoid releasing another run's lock
- Documents lock semantics clearly in module JSDoc
5. Sync route updates
src/app/api/sync/route.ts: Added lock acquisition for manual sync, now tracks sync run status (running → completed) and recordsreposFetched/syncedCountsrc/app/api/automation/sync/route.ts: Added lock acquisition for automation syncsrc/app/api/sync/scheduled/route.ts: Refactored to import fromsync-lock.ts; removed ~56 lines of inline code
6. PR followup security hardening
src/app/api/pr-followup/sync/route.ts: AddedauthorizeRequestauth checksrc/app/api/pr-followup/webhook/route.ts: AddedauthorizeRequestauth check + fail-closed webhook signature verification (rejects requests whenWEBHOOK_SECRETis unset unlessWEBHOOK_GATEWAY_MODE=true)
7. Schema comment update (prisma/schema.prisma)
- Updated
syncTypecomment from"pre-claim"to"automation"to reflect current usage
Standards Compliance
| Convention | Status | Notes |
|---|---|---|
Error handling (error instanceof Error) |
✅ | Used in untriaged route and sync routes |
| Input validation before DB ops | ✅ | limit parsed and bounded; repoFilter validated via Prisma query |
| HTTP status codes | ✅ | 200, 401, 409, 500 used appropriately |
| No secrets in code | ✅ | No hardcoded secrets |
| Prisma relations strict | ✅ | No nullable FKs introduced |
| Audit log for state changes | ✅ | Sync runs tracked via IssueSyncRun table |
| API routes return JSON | ✅ | All endpoints return NextResponse.json() |
Linked Issue Fit
Issue PR 325 acceptance criteria — all satisfied:
| Criterion | Status | Evidence |
|---|---|---|
| Open no-status issues appear in grooming intake | ✅ | Endpoint returns state: "open" with no status/* label |
| Same issues excluded from worker queues by default | ✅ | Separate endpoint; no changes to /api/agents/[name]/queue |
Grooming can promote to status/ready or status/backlog |
✅ | Endpoint surfaces issues; promotion via existing /api/issues/move |
| Renovate Dashboard issues excluded by default | ✅ | isRenovateIssue() filter + excludeRenovate=true default |
| Tests cover grooming intake separately from worker queues | ✅ | 10 dedicated test cases |
| Existing worker queue behavior from PR 291 intact | ✅ | No changes to queue or claim endpoints |
| Closed issues skipped | ✅ | state: "open" in Prisma query |
| Bound results per run | ✅ | Default 50, hard cap 200 |
| Do not claim as worker work | ✅ | Intake-only endpoint; no agent/* label assignment |
Evidence Provider Findings
No evidence providers were configured for this PR.
Tool Harness Findings
No tool harness requests were planned for this PR.
Unknowns / Needs Verification
-
Database query efficiency: The untriaged endpoint fetches all open issues from enabled repos and filters in-memory. For repositories with many open issues, this could transfer unnecessary rows. No indexing concern was flagged (Prisma handles basic indexes), but in production with large repos worth monitoring query times.
-
Route auth: The untriaged endpoint does not call
authorizeRequest. This appears intentional — the endpoint is read-only and meant for grooming workflows that may operate with different auth contexts. Confirm this aligns with intended deployment topology. -
Renovate detection scope:
isRenovateIssuechecks title only ("dependency dashboard"or"renovate dashboard"). Renovate sometimes uses slightly different naming. TheexcludeRenovate=falseescape hatch handles this.
Summary
This PR cleanly implements the grooming intake endpoint for untriaged issues, extracts shared sync-locking logic to reduce duplication, and hardens webhook security. All acceptance criteria from PR 325 are met. The test suite is thorough. No deviations from repository conventions were identified.
1 participant
Fixes #325
Add a new GET /api/issues/untriaged endpoint that surfaces open issues with no
status/*label — an intake view for grooming.What this addresses
Issue #325 identified that while unlabelled/no-status issues are correctly excluded from normal worker queues, they also create a blind spot: open issues with no
status/*label may never be surfaced to grooming.Implementation
New endpoint:
GET /api/issues/untriagedstatus/labelexcludeRenovate=false)?repo=<fullName>filter and?limit=NparameterTests: Comprehensive test suite covering filtering, limits, Renovate exclusion, repo filtering, and response shape.
Route smoke check: Updated to include the new endpoint path.
API Contract
GET /api/issues/untriaged?limit=50&repo=misospace/dispatch&excludeRenovate=trueResponse: Array of open issues with no status label, bounded by
limit(default 50, max 200).This endpoint enables the grooming workflow to discover and classify untriaged issues without exposing them to normal worker queues.