From d093c26f8c1f46918e468696446d5d24f3867bdf Mon Sep 17 00:00:00 2001 From: Jory Irving Date: Fri, 19 Jun 2026 09:21:19 -0600 Subject: [PATCH 1/2] Fix dispatch cluster issue filters and sync --- docs/worker-execution-contract.md | 12 ++-- src/app/agents/page.tsx | 9 ++- .../api/agents/[agentName]/next-task/route.ts | 12 ++-- .../agents/[agentName]/work-summary/route.ts | 12 ++-- src/app/api/issues/route.test.ts | 58 ++++++++++++---- src/app/api/issues/route.ts | 17 +++-- src/app/api/issues/untriaged/route.test.ts | 4 ++ src/app/api/issues/untriaged/route.ts | 4 ++ src/app/automation/page.tsx | 41 +++++++++-- src/app/board/page.tsx | 22 +++++- src/app/page.tsx | 8 ++- src/app/projects/page.tsx | 3 +- src/lib/agent-queue-fetch.ts | 11 +-- src/lib/agent-queue.ts | 26 +------ src/lib/github.test.ts | 21 +++++- src/lib/github.ts | 26 +++++-- src/lib/issue-filters.test.ts | 68 +++++++++++++++++-- src/lib/issue-filters.ts | 55 ++++++++++++++- 18 files changed, 316 insertions(+), 93 deletions(-) diff --git a/docs/worker-execution-contract.md b/docs/worker-execution-contract.md index 3fb11c5..c2f3747 100644 --- a/docs/worker-execution-contract.md +++ b/docs/worker-execution-contract.md @@ -199,19 +199,14 @@ BACKLOG issues are excluded from the normal agent queue by default. ### Renovate Issue Exclusion -Renovate-created issues (dependency dashboards, update PRs, etc.) are **visible in Dispatch** but **excluded from agent queues by default**. This prevents agents from consuming cycles on dependency bookkeeping instead of normal issue work. +Renovate-created issues (dependency dashboards, update PRs, etc.) are **filtered out of Dispatch issue surfaces**. This keeps the Board, Projects, lane summaries, grooming intake, and agent queues focused on normal issue work instead of dependency bookkeeping. Detection heuristics (author detection is not available since the Issue model does not store author): - Title contains `Dependency Dashboard` - Title starts with `Update dependency`, `Update image`, or similar Renovate patterns - Labels: `renovate`, `dependencies`, `automated` -To explicitly include Renovate issues in queue results, pass `includeRenovate=true`: -```bash -GET /api/agents//queue?lane=normal&includeRenovate=true -``` - -Renovate exclusion applies to issue queue items only, not PR review-fix queue items. Issues excluded as Renovate remain visible on the Board and Projects pages. +Renovate exclusion applies to issue queue items only, not PR review-fix queue items. --- @@ -230,6 +225,7 @@ Workers using the canonical `next-task` endpoint automatically receive PR-fix it ## History - **2026-05-16** — Created to document generic worker execution contract and PR completion gates (Issue #65). Consolidates existing normal-worker behavior into a reusable, agent-agnostic specification. -- **2026-05-19** — Added Renovate issue exclusion section: Renovate issues are visible in Dispatch but excluded from agent queues by default; `includeRenovate=true` opt-in available (Issue #129). +- **2026-06-19** — Updated Renovate issue exclusion: Renovate issues are filtered from Dispatch issue surfaces, including Board, Projects, lane summaries, grooming intake, and agent queues. +- **2026-05-19** — Added Renovate issue exclusion section: Renovate issues are excluded from agent queues by default (Issue #129). - **2026-05-19** — Added five-column workflow with Ready status (Issue #140): Backlog → Ready → In Progress → In Review → Done. Agents pick from Ready by default; Backlog excluded unless explicitly requested. - **2026-05-20** — Marked `lane=gpt` as deprecated compatibility alias in canonical docs; linked openclaw-agent-mc-workflow.md as historical (Issue #117). diff --git a/src/app/agents/page.tsx b/src/app/agents/page.tsx index e30125f..5e83f32 100644 --- a/src/app/agents/page.tsx +++ b/src/app/agents/page.tsx @@ -4,6 +4,7 @@ import { Table, TableBody, TableCell, TableHead, TableRow } from "@/components/u import { Badge } from "@/components/ui/badge"; import { AGENT_PREFIX } from "@/types"; import AgentWorkPanel from "@/app/agents/agent-work-panel"; +import { applyRenovateIssueExclusion } from "@/lib/issue-filters"; export const dynamic = "force-dynamic"; @@ -25,14 +26,12 @@ interface AgentStats { } async function getAgentStats() { - const issues = await prisma.issue.findMany({ - where: { state: "open", repository: { enabled: true } }, - }); - const agentMap: Record = {}; + const where: Record = { repository: { enabled: true } }; + applyRenovateIssueExclusion(where); const agentIssues = await prisma.issue.findMany({ - where: { repository: { enabled: true } }, + where, select: { labels: true }, }); diff --git a/src/app/api/agents/[agentName]/next-task/route.ts b/src/app/api/agents/[agentName]/next-task/route.ts index 0ed99fa..44ff2ba 100644 --- a/src/app/api/agents/[agentName]/next-task/route.ts +++ b/src/app/api/agents/[agentName]/next-task/route.ts @@ -10,6 +10,7 @@ import { import { isBacklogLane, getBacklogLane } from "@/lib/lane-config"; import { isRenovateIssue } from "@/lib/agent-queue"; import { fetchAgentQueueData } from "@/lib/agent-queue-fetch"; +import { applyRenovateIssueExclusion } from "@/lib/issue-filters"; export async function GET( request: Request, @@ -31,11 +32,14 @@ export async function GET( try { // Groom mode: return exactly one issue to triage/enrich if (mode === "groom") { + const issueWhere: Record = { + state: "open", + repository: { enabled: true }, + }; + applyRenovateIssueExclusion(issueWhere); + const issues = await prisma.issue.findMany({ - where: { - state: "open", - repository: { enabled: true }, - }, + where: issueWhere, select: { id: true, number: true, diff --git a/src/app/api/agents/[agentName]/work-summary/route.ts b/src/app/api/agents/[agentName]/work-summary/route.ts index 776bd53..50987d1 100644 --- a/src/app/api/agents/[agentName]/work-summary/route.ts +++ b/src/app/api/agents/[agentName]/work-summary/route.ts @@ -3,6 +3,7 @@ import { authorizeRequest } from "@/lib/auth"; import { prisma, asPrFixQueueClient } from "@/lib/prisma"; import { listQueuedPrFixItems } from "@/lib/pr-fix-queue"; import { getConfiguredLanes, getDefaultClaimableLane, resolveLaneId } from "@/lib/lane-config"; +import { applyRenovateIssueExclusion } from "@/lib/issue-filters"; type WorkSummaryLaneCounts = { queued: number; inProgress: number }; type PrFixLaneCounts = { total: number; blocked: number }; @@ -32,11 +33,14 @@ export async function GET(request: Request, { params }: { params: Promise<{ agen } try { + const issueWhere: Record = { + state: "open", + repository: { enabled: true }, + }; + applyRenovateIssueExclusion(issueWhere); + const issues = await prisma.issue.findMany({ - where: { - state: "open", - repository: { enabled: true }, - }, + where: issueWhere, select: { labels: true, currentLane: true, diff --git a/src/app/api/issues/route.test.ts b/src/app/api/issues/route.test.ts index e0f12de..041a1ed 100644 --- a/src/app/api/issues/route.test.ts +++ b/src/app/api/issues/route.test.ts @@ -172,23 +172,31 @@ describe("GET /api/issues — visible issue filtering", () => { await makeRequest("http://localhost/api/issues?untriaged=true"); const call = mocks.findManyIssues.mock.calls[0][0]; - expect(call.where.labels).toBeDefined(); - expect(call.where.labels.hasNone).toBeDefined(); - expect(call.where.labels.hasNone).toContain("status/backlog"); - expect(call.where.labels.hasNone).toContain("status/ready"); - expect(call.where.labels.hasNone).toContain("status/in-progress"); - expect(call.where.labels.hasNone).toContain("status/in-review"); - expect(call.where.labels.hasNone).toContain("status/done"); + expect(call.where.AND).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + NOT: { labels: { hasSome: expect.arrayContaining([ + "status/backlog", + "status/ready", + "status/in-progress", + "status/in-review", + "status/done", + ]) } }, + }), + ]), + ); }); - it("does not add hasNone filter when untriaged is not true", async () => { + it("does not add no-status filter when untriaged is not true", async () => { await makeRequest("http://localhost/api/issues?untriaged=false"); const call = mocks.findManyIssues.mock.calls[0][0]; - // labels may have other filters but should not have hasNone from noStatus - if (call.where.labels) { - expect(call.where.labels.hasNone).toBeUndefined(); - } + const andClauses = call.where.AND ?? []; + expect(andClauses).not.toEqual( + expect.arrayContaining([ + expect.objectContaining({ NOT: { labels: { hasSome: expect.arrayContaining(["status/ready"]) } } }), + ]), + ); }); it("combines untriaged filter with agent filter", async () => { @@ -196,8 +204,13 @@ describe("GET /api/issues — visible issue filtering", () => { const call = mocks.findManyIssues.mock.calls[0][0]; expect(call.where.labels.has).toBe("agent/alpha"); - expect(call.where.labels.hasNone).toBeDefined(); - expect(call.where.labels.hasNone).toContain("status/ready"); + expect(call.where.AND).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + NOT: { labels: { hasSome: expect.arrayContaining(["status/ready"]) } }, + }), + ]), + ); }); it("orders by updatedAt descending", async () => { @@ -268,6 +281,23 @@ describe("GET /api/issues — visible issue filtering", () => { const call = mocks.findManyIssues.mock.calls[0][0]; expect(call.where.currentLane).toEqual({ in: ["backlog"] }); }); + + it("excludes Renovate issues from API results", async () => { + await makeRequest("http://localhost/api/issues"); + + const call = mocks.findManyIssues.mock.calls[0][0]; + expect(call.where.AND).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + NOT: expect.objectContaining({ + OR: expect.arrayContaining([ + { labels: { hasSome: ["renovate", "dependencies", "automated"] } }, + ]), + }), + }), + ]), + ); + }); }); describe("GET /api/issues — lane aliases", () => { diff --git a/src/app/api/issues/route.ts b/src/app/api/issues/route.ts index 687a1d7..0cceb78 100644 --- a/src/app/api/issues/route.ts +++ b/src/app/api/issues/route.ts @@ -1,7 +1,15 @@ import { NextResponse } from "next/server"; import { authorizeRequest } from "@/lib/auth"; import { prisma } from "@/lib/prisma"; -import { buildLabelWhere, buildVisibleIssueWhere, toProjectLabel, buildExcludedLabelWhere, buildNoStatusWhere } from "@/lib/issue-filters"; +import { + appendIssueWhere, + applyRenovateIssueExclusion, + buildExcludedLabelWhere, + buildLabelWhere, + buildNoStatusWhere, + buildVisibleIssueWhere, + toProjectLabel, +} from "@/lib/issue-filters"; import { parseExcludedLabels } from "@/lib/config"; import { isValidLane, getLaneIds, resolveRequestLane, getLaneAliases } from "@/lib/lane-config"; @@ -25,6 +33,7 @@ export async function GET(request: Request) { const where: Record = { repository: { enabled: true } }; buildVisibleIssueWhere(where, { includeClosed: includeClosed === "true" }); + applyRenovateIssueExclusion(where); if (repo) { where.repository = { ...(where.repository as object), fullName: repo }; @@ -41,13 +50,11 @@ export async function GET(request: Request) { const excludedLabels = parseExcludedLabels(process.env.DISPATCH_EXCLUDED_LABELS); const excludedLabelFilter = buildExcludedLabelWhere(excludedLabels); - if (excludedLabelFilter) { - where.labels = { ...(where.labels as object), ...excludedLabelFilter }; - } + appendIssueWhere(where, excludedLabelFilter); // Filter for untriaged issues (no status/* label) — grooming intake const noStatusFilter = buildNoStatusWhere(untriaged === "true"); - if (noStatusFilter) where.labels = { ...(where.labels as object), ...noStatusFilter }; + appendIssueWhere(where, noStatusFilter); // Filter by execution lane if (lane) { diff --git a/src/app/api/issues/untriaged/route.test.ts b/src/app/api/issues/untriaged/route.test.ts index 01c1fa8..7c2769e 100644 --- a/src/app/api/issues/untriaged/route.test.ts +++ b/src/app/api/issues/untriaged/route.test.ts @@ -24,6 +24,10 @@ vi.mock("@/lib/prisma", () => ({ vi.mock("@/types", () => ({ STATUS_LABELS: ["status/backlog", "status/ready", "status/in-progress", "status/in-review", "status/done"], + AGENT_PREFIX: "agent/", + OWNER_PREFIX: "owner/", + isAgentLabel: (label: string) => label.startsWith("agent/"), + isOwnerLabel: (label: string) => label.startsWith("owner/"), })); vi.mock("@/lib/agent-queue", () => ({ diff --git a/src/app/api/issues/untriaged/route.ts b/src/app/api/issues/untriaged/route.ts index 916d418..7668ef0 100644 --- a/src/app/api/issues/untriaged/route.ts +++ b/src/app/api/issues/untriaged/route.ts @@ -2,6 +2,7 @@ import { NextResponse } from "next/server"; import { prisma } from "@/lib/prisma"; import { STATUS_LABELS } from "@/types"; import { isRenovateIssue } from "@/lib/agent-queue"; +import { applyRenovateIssueExclusion } from "@/lib/issue-filters"; /** * GET /api/issues/untriaged @@ -43,6 +44,9 @@ export async function GET(request: Request) { state: "open", repository: { enabled: true }, }; + if (excludeRenovate) { + applyRenovateIssueExclusion(where); + } if (repoFilter) { where = { diff --git a/src/app/automation/page.tsx b/src/app/automation/page.tsx index f0f62cb..486a5f8 100644 --- a/src/app/automation/page.tsx +++ b/src/app/automation/page.tsx @@ -180,6 +180,7 @@ export default function AutomationOverview() { const [showAddForm, setShowAddForm] = useState(false); const [newRepo, setNewRepo] = useState(""); const [addError, setAddError] = useState(""); + const [loadError, setLoadError] = useState(""); const [addLoading, setAddLoading] = useState(false); useEffect(() => { @@ -187,9 +188,21 @@ export default function AutomationOverview() { .then((res) => res.json()) .catch(() => ({})); authedFetch("/api/automation/repos") - .then((res) => res.json()) - .then((data) => setRepos(data)) - .catch(() => setRepos([])) + .then(async (res) => { + const data = await res.json().catch(() => null); + if (!res.ok) { + throw new Error(data?.error || "Failed to load repositories"); + } + if (!Array.isArray(data)) { + throw new Error("Repository API returned an unexpected response"); + } + setRepos(data); + setLoadError(""); + }) + .catch((error) => { + setRepos([]); + setLoadError(error instanceof Error ? error.message : "Failed to load repositories"); + }) .finally(() => setLoading(false)); }, []); @@ -199,7 +212,16 @@ export default function AutomationOverview() { await authedFetch("/api/automation/sync", { method: "POST" }); const res = await authedFetch("/api/automation/repos"); const data = await res.json(); + if (!res.ok) { + throw new Error(data?.error || "Failed to load repositories"); + } + if (!Array.isArray(data)) { + throw new Error("Repository API returned an unexpected response"); + } setRepos(data); + setLoadError(""); + } catch (error) { + setLoadError(error instanceof Error ? error.message : "Failed to sync repositories"); } finally { setSyncing(false); } @@ -224,7 +246,10 @@ export default function AutomationOverview() { setShowAddForm(false); const res2 = await authedFetch("/api/automation/repos"); const data2 = await res2.json(); - setRepos(data2); + if (res2.ok && Array.isArray(data2)) { + setRepos(data2); + setLoadError(""); + } } catch { setAddError("Failed to add repo"); } finally { @@ -269,6 +294,12 @@ export default function AutomationOverview() { + {loadError && ( + + {loadError} + + )} + {showAddForm && ( @@ -308,4 +339,4 @@ export default function AutomationOverview() { )} ); -} \ No newline at end of file +} diff --git a/src/app/board/page.tsx b/src/app/board/page.tsx index c96b79f..d178e6d 100644 --- a/src/app/board/page.tsx +++ b/src/app/board/page.tsx @@ -4,7 +4,13 @@ import { FilterBar } from "@/components/filter-bar"; import { SyncIssuesButton } from "@/components/sync-issues-button"; import { Card, CardContent } from "@/components/ui/card"; import { getTrackedRepos } from "@/lib/config"; -import { buildLabelWhere, buildVisibleIssueWhere, discoverLabelFilterOptions, getDoneRetentionDays } from "@/lib/issue-filters"; +import { + applyRenovateIssueExclusion, + buildLabelWhere, + buildVisibleIssueWhere, + discoverLabelFilterOptions, + getDoneRetentionDays, +} from "@/lib/issue-filters"; import { getConfiguredLanes, isValidLane } from "@/lib/lane-config"; export const dynamic = "force-dynamic"; @@ -13,6 +19,7 @@ async function getIssues(repo?: string, agent?: string, owner?: string, priority const where: Record = { repository: { enabled: true } }; buildVisibleIssueWhere(where, { includeClosed }); + applyRenovateIssueExclusion(where); if (repo) where.repository = { ...(where.repository as object), fullName: repo }; @@ -38,19 +45,28 @@ async function getRepos() { } async function getFilterOptions() { + const where: Record = { repository: { enabled: true } }; + applyRenovateIssueExclusion(where); + const issues = await prisma.issue.findMany({ - where: { repository: { enabled: true } }, + where, select: { labels: true }, }); return discoverLabelFilterOptions(issues); } +function buildVisibleIssueStatsWhere() { + const where: Record = { repository: { enabled: true } }; + applyRenovateIssueExclusion(where); + return where; +} + async function getIssueSyncStatus() { const [trackedRepos, issueStats] = await Promise.all([ getTrackedRepos(), prisma.issue.aggregate({ - where: { repository: { enabled: true } }, + where: buildVisibleIssueStatsWhere(), _count: { _all: true }, _max: { lastSyncedAt: true }, }), diff --git a/src/app/page.tsx b/src/app/page.tsx index 9cc1efd..4cf24da 100644 --- a/src/app/page.tsx +++ b/src/app/page.tsx @@ -2,13 +2,17 @@ import { prisma } from "@/lib/prisma"; import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; import { Table, TableBody, TableCell, TableHead, TableRow } from "@/components/ui/table"; import { STATUS_LABELS } from "@/types"; +import { applyRenovateIssueExclusion } from "@/lib/issue-filters"; export const dynamic = "force-dynamic"; async function getStats() { + const issueWhere: Record = { state: "open", repository: { enabled: true } }; + applyRenovateIssueExclusion(issueWhere); + const [issues, recentRuns, recentLogs] = await Promise.all([ prisma.issue.findMany({ - where: { state: "open", repository: { enabled: true } }, + where: issueWhere, include: { repository: true }, }), prisma.agentRun.findMany({ @@ -203,4 +207,4 @@ export default async function OverviewPage() { ); -} \ No newline at end of file +} diff --git a/src/app/projects/page.tsx b/src/app/projects/page.tsx index 80552ad..2d5692d 100644 --- a/src/app/projects/page.tsx +++ b/src/app/projects/page.tsx @@ -2,7 +2,7 @@ import { prisma } from "@/lib/prisma"; import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; import { Badge } from "@/components/ui/badge"; import { BOARD_COLUMNS, STATUS_LABELS } from "@/types"; -import { buildVisibleIssueWhere } from "@/lib/issue-filters"; +import { applyRenovateIssueExclusion, buildVisibleIssueWhere } from "@/lib/issue-filters"; import { getProjectIssueStatus, groupIssuesByProject } from "@/lib/projects"; export const dynamic = "force-dynamic"; @@ -16,6 +16,7 @@ async function getProjects() { // to match Board page retention semantics. const where: Record = { repository: { enabled: true } }; buildVisibleIssueWhere(where); + applyRenovateIssueExclusion(where); const issues = await prisma.issue.findMany({ where, diff --git a/src/lib/agent-queue-fetch.ts b/src/lib/agent-queue-fetch.ts index a7aa80d..535adf2 100644 --- a/src/lib/agent-queue-fetch.ts +++ b/src/lib/agent-queue-fetch.ts @@ -4,6 +4,7 @@ import { listQueuedPrFixItems, toAgentQueuePrFixItem } from "@/lib/pr-fix-queue" import { findLeasedIssueIds } from "@/lib/lease"; import { parseExcludedLabels } from "@/lib/config"; import { resolveRequestLane, getLaneIds } from "@/lib/lane-config"; +import { applyRenovateIssueExclusion } from "@/lib/issue-filters"; import type { RankedIssue } from "@/lib/agent-queue"; /** @@ -53,13 +54,15 @@ export async function fetchAgentQueueData( params: AgentQueueFetchParams, ): Promise { const { agentName, lane, excludeDecomposed, includeClaimed, includeRenovate } = params; + const issueWhere: Record = { + state: "open", + repository: { enabled: true }, + }; + applyRenovateIssueExclusion(issueWhere); // Fetch all open issues from enabled repos (GitHub Issues as source of truth) const issues = await prisma.issue.findMany({ - where: { - state: "open", - repository: { enabled: true }, - }, + where: issueWhere, select: { id: true, number: true, diff --git a/src/lib/agent-queue.ts b/src/lib/agent-queue.ts index ec09b6b..548cf7b 100644 --- a/src/lib/agent-queue.ts +++ b/src/lib/agent-queue.ts @@ -6,9 +6,11 @@ import { getAgentFromLabels, getPriorityFromLabels, } from "@/types"; -import { isIssueExcludedByLabels } from "@/lib/issue-filters"; +import { isIssueExcludedByLabels, isRenovateIssue } from "@/lib/issue-filters"; import { isBacklogLane, resolveLaneId, laneMatchesConfigured } from "@/lib/lane-config"; +export { isRenovateIssue } from "@/lib/issue-filters"; + const DONE_STATUS: string = "status/done"; const IN_PROGRESS_STATUS: string = "status/in-progress"; const IN_REVIEW_STATUS: string = "status/in-review"; @@ -47,28 +49,6 @@ export interface RankedIssue { linkedPrHealth?: QueueLinkedPrHealth | null; } -/** - * Detect Renovate issues by title/label heuristics. - * Author detection is not available since the Issue model does not store author. - */ -export function isRenovateIssue(issue: { title: string; labels: string[] }): boolean { - const title = issue.title.toLowerCase(); - const labels = issue.labels.map((l) => l.toLowerCase()); - - // Title-based heuristics - if (title.includes("dependency dashboard")) return true; - if (title.includes("renovate dashboard")) return true; - if (/^update (?:dependency|image|deps?)/.test(title)) return true; - - // Label-based heuristics - const renovateLabels = ["renovate", "dependencies", "automated"]; - for (const label of labels) { - if (renovateLabels.includes(label)) return true; - } - - return false; -} - /** * Score an issue for a given agent. Lower score = higher priority. */ diff --git a/src/lib/github.test.ts b/src/lib/github.test.ts index 9f50cfc..ee556c2 100644 --- a/src/lib/github.test.ts +++ b/src/lib/github.test.ts @@ -8,7 +8,7 @@ vi.mock("./github", async (importOriginal) => { return { ...actual }; }); -function makeResponse(data: unknown[], hasNext = false): Response { +function makeResponse(data: unknown, hasNext = false): Response { const headers: Record = { "Content-Type": "application/json" }; if (hasNext) { headers.Link = '; rel="next"'; @@ -92,6 +92,25 @@ describe("fetchPaginated", () => { expect(result).toEqual([]); }); + it("extracts items from wrapped GitHub responses", async () => { + fetchMock.mockResolvedValueOnce(makeResponse({ total_count: 2, workflow_runs: [{ id: 1 }, { id: 2 }] })); + + const result = await fetchPaginated<{ id: number }>( + "https://example.com/api", + Infinity, + (data) => (data as { workflow_runs?: { id: number }[] }).workflow_runs ?? [], + ); + + expect(result).toEqual([{ id: 1 }, { id: 2 }]); + }); + + it("throws a useful error when an unwrapped response is not an array", async () => { + fetchMock.mockResolvedValueOnce(makeResponse({ message: "not an array" })); + + await expect(fetchPaginated<{ id: number }>("https://example.com/api")) + .rejects.toThrow("expected array response"); + }); + it("handles maxItems of 0", async () => { const result = await fetchPaginated<{ id: number }>("https://example.com/api", 0); diff --git a/src/lib/github.ts b/src/lib/github.ts index 5c6f3b9..6682e56 100644 --- a/src/lib/github.ts +++ b/src/lib/github.ts @@ -237,9 +237,15 @@ function getNextLink(response: Response): string | null { * * @param url - Initial API URL (must include per_page query param). * @param maxItems - Hard cap on total items returned. Defaults to Infinity (exhaust all pages). + * @param extractPageItems - Optional extractor for GitHub endpoints that wrap arrays + * in an object, such as Actions runs. * @returns All collected items across pages. */ -export async function fetchPaginated(url: string, maxItems = Infinity): Promise { +export async function fetchPaginated( + url: string, + maxItems = Infinity, + extractPageItems?: (data: unknown) => T[], +): Promise { const all: T[] = []; let currentUrl: string | null = url; @@ -251,7 +257,11 @@ export async function fetchPaginated(url: string, maxItems = Infinity): Promi throw new Error(`GitHub API error: ${response.status} ${text}`); } - const page = (await response.json()) as T[]; + const data = await response.json(); + const page = extractPageItems ? extractPageItems(data) : data; + if (!Array.isArray(page)) { + throw new Error(`GitHub API error: expected array response from ${currentUrl}`); + } const remaining = maxItems - all.length; all.push(...page.slice(0, remaining)); @@ -439,13 +449,21 @@ export interface GithubWorkflowRun { export async function fetchWorkflowRuns(repoFullName: string, workflowId: number, perPage = 20): Promise { // Bounded to 50 runs — enough to see recent history without excessive API usage. const url = `${GITHUB_API}/repos/${repoFullName}/actions/workflows/${workflowId}/runs?per_page=${perPage}`; - return fetchPaginated(url, 50); + return fetchPaginated( + url, + 50, + (data) => (data as { workflow_runs?: GithubWorkflowRun[] }).workflow_runs ?? [], + ); } export async function fetchRecentRunsAllWorkflows(repoFullName: string, perPage = 30): Promise { // Bounded to 100 runs across all workflows — recent history cap. const url = `${GITHUB_API}/repos/${repoFullName}/actions/runs?per_page=${perPage}`; - return fetchPaginated(url, 100); + return fetchPaginated( + url, + 100, + (data) => (data as { workflow_runs?: GithubWorkflowRun[] }).workflow_runs ?? [], + ); } export interface GithubJob { diff --git a/src/lib/issue-filters.test.ts b/src/lib/issue-filters.test.ts index 833cabb..2d692a5 100644 --- a/src/lib/issue-filters.test.ts +++ b/src/lib/issue-filters.test.ts @@ -1,5 +1,17 @@ import { describe, expect, it, vi, beforeEach } from "vitest"; -import { buildLabelWhere, discoverLabelFilterOptions, toProjectLabel, buildVisibleIssueWhere, getDoneRetentionDays, DEFAULT_DONE_RETENTION_DAYS, buildNoStatusWhere } from "./issue-filters"; +import { + appendIssueWhere, + applyRenovateIssueExclusion, + buildExcludedLabelWhere, + buildLabelWhere, + buildNoStatusWhere, + buildVisibleIssueWhere, + DEFAULT_DONE_RETENTION_DAYS, + discoverLabelFilterOptions, + getDoneRetentionDays, + isRenovateIssue, + toProjectLabel, +} from "./issue-filters"; describe("issue filter helpers", () => { it("discovers sorted agent and owner options from labels only", () => { @@ -47,14 +59,56 @@ describe("buildNoStatusWhere", () => { expect(buildNoStatusWhere(false)).toBeUndefined(); }); - it("returns hasNone filter with STATUS_LABELS when includeUntriaged is true", () => { + it("returns a NOT hasSome filter with STATUS_LABELS when includeUntriaged is true", () => { const result = buildNoStatusWhere(true); expect(result).toBeDefined(); - expect(result!.hasNone).toContain("status/backlog"); - expect(result!.hasNone).toContain("status/ready"); - expect(result!.hasNone).toContain("status/in-progress"); - expect(result!.hasNone).toContain("status/in-review"); - expect(result!.hasNone).toContain("status/done"); + const labels = result!.NOT.labels.hasSome; + expect(labels).toContain("status/backlog"); + expect(labels).toContain("status/ready"); + expect(labels).toContain("status/in-progress"); + expect(labels).toContain("status/in-review"); + expect(labels).toContain("status/done"); + }); +}); + +describe("issue exclusion filters", () => { + it("builds excluded label filters with Prisma-supported scalar-list syntax", () => { + expect(buildExcludedLabelWhere(["renovate"])).toEqual({ + NOT: { labels: { hasSome: ["renovate"] } }, + }); + }); + + it("appends issue where clauses with AND", () => { + const where: Record = { repository: { enabled: true } }; + appendIssueWhere(where, { NOT: { labels: { hasSome: ["renovate"] } } }); + appendIssueWhere(where, { NOT: { labels: { hasSome: ["status/ready"] } } }); + + expect(where.AND).toEqual([ + { NOT: { labels: { hasSome: ["renovate"] } } }, + { NOT: { labels: { hasSome: ["status/ready"] } } }, + ]); + }); + + it("adds Renovate exclusion as an AND clause", () => { + const where: Record = { repository: { enabled: true } }; + applyRenovateIssueExclusion(where); + + expect(where.AND).toEqual([ + expect.objectContaining({ + NOT: expect.objectContaining({ + OR: expect.arrayContaining([ + { labels: { hasSome: ["renovate", "dependencies", "automated"] } }, + ]), + }), + }), + ]); + }); + + it("detects Renovate issues by shared heuristics", () => { + expect(isRenovateIssue({ title: "Dependency Dashboard", labels: [] })).toBe(true); + expect(isRenovateIssue({ title: "Update image node to v20", labels: [] })).toBe(true); + expect(isRenovateIssue({ title: "Bump lodash", labels: ["renovate"] })).toBe(true); + expect(isRenovateIssue({ title: "Fix login bug", labels: ["bug"] })).toBe(false); }); }); diff --git a/src/lib/issue-filters.ts b/src/lib/issue-filters.ts index b600a21..75b9e9c 100644 --- a/src/lib/issue-filters.ts +++ b/src/lib/issue-filters.ts @@ -1,5 +1,7 @@ import { AGENT_PREFIX, isAgentLabel, isOwnerLabel, OWNER_PREFIX, STATUS_LABELS } from "@/types"; +export const RENOVATE_LABELS = ["renovate", "dependencies", "automated"] as const; + export interface VisibleIssueWhereOptions { includeClosed?: boolean; doneRetentionDays?: number; @@ -46,22 +48,69 @@ export function isIssueExcludedByLabels(issueLabels: string[], excludedLabels: s export function buildExcludedLabelWhere(excludedLabels: string[]) { if (excludedLabels.length === 0) return undefined; - return { hasNone: excludedLabels }; + return { NOT: { labels: { hasSome: excludedLabels } } }; } /** * Build a Prisma where clause that matches issues with no status/* label. * Used for grooming intake — surfaces untriaged open issues. */ -export function buildNoStatusWhere(includeUntriaged: boolean): { hasNone: string[] } | undefined { +export function buildNoStatusWhere(includeUntriaged: boolean) { if (!includeUntriaged) return undefined; - return { hasNone: STATUS_LABELS }; + return { NOT: { labels: { hasSome: STATUS_LABELS } } }; } export function toProjectLabel(project: string | null | undefined) { return project ? `project/${project}` : undefined; } +export function appendIssueWhere(where: Record, clause: Record | undefined): void { + if (!clause) return; + + const existing = where.AND; + if (Array.isArray(existing)) { + existing.push(clause); + } else if (existing) { + where.AND = [existing, clause]; + } else { + where.AND = [clause]; + } +} + +/** + * Detect Renovate issues by title/label heuristics. + * Author detection is not available since the Issue model does not store author. + */ +export function isRenovateIssue(issue: { title: string; labels: string[] }): boolean { + const title = issue.title.toLowerCase(); + const labels = issue.labels.map((l) => l.toLowerCase()); + + if (title.includes("dependency dashboard")) return true; + if (title.includes("renovate dashboard")) return true; + if (/^update (?:dependency|image|deps?)/.test(title)) return true; + + return labels.some((label) => (RENOVATE_LABELS as readonly string[]).includes(label)); +} + +export function buildRenovateIssueExclusionWhere() { + return { + NOT: { + OR: [ + { labels: { hasSome: [...RENOVATE_LABELS] } }, + { title: { contains: "dependency dashboard", mode: "insensitive" } }, + { title: { contains: "renovate dashboard", mode: "insensitive" } }, + { title: { startsWith: "update dependency", mode: "insensitive" } }, + { title: { startsWith: "update image", mode: "insensitive" } }, + { title: { startsWith: "update dep", mode: "insensitive" } }, + ], + }, + }; +} + +export function applyRenovateIssueExclusion(where: Record): void { + appendIssueWhere(where, buildRenovateIssueExclusionWhere()); +} + export const DEFAULT_DONE_RETENTION_DAYS = 7; export function getDoneRetentionDays(): number { From 8a856028e9874cbffd42c6725bbf1a0434f7de7d Mon Sep 17 00:00:00 2001 From: Jory Irving Date: Fri, 19 Jun 2026 09:28:35 -0600 Subject: [PATCH 2/2] Protect untriaged issues API --- src/app/api/issues/untriaged.test.ts | 59 ++++++++++++++++------ src/app/api/issues/untriaged/route.test.ts | 23 ++++++--- src/app/api/issues/untriaged/route.ts | 5 ++ 3 files changed, 65 insertions(+), 22 deletions(-) diff --git a/src/app/api/issues/untriaged.test.ts b/src/app/api/issues/untriaged.test.ts index 75ff451..839ca35 100644 --- a/src/app/api/issues/untriaged.test.ts +++ b/src/app/api/issues/untriaged.test.ts @@ -1,6 +1,16 @@ // @vitest-environment node import { describe, expect, it, beforeEach, vi } from "vitest"; +const mockToken = "test-agent-token"; +process.env.DISPATCH_AGENT_TOKEN = mockToken; + +vi.mock("@/lib/dispatch-env", () => ({ + isAuthorizedAgentToken: vi.fn((token) => token === mockToken), + isAuthorizedBearerToken: vi.fn((token) => token === mockToken), + getAcceptedAgentTokens: vi.fn(() => [mockToken]), + resetCaches: vi.fn(), +})); + // Mock prisma FIRST — simulate Prisma's where/orderBy behavior interface MockIssue { id?: string; @@ -70,6 +80,7 @@ vi.doMock("@/types", () => ({ // Dynamic imports after doMock setup. const { prisma } = await import("@/lib/prisma"); +const { resetAuthCaches } = await import("@/lib/auth"); const mockFindMany = vi.mocked(prisma.issue.findMany); // Import route AFTER all mocks are set up @@ -87,12 +98,28 @@ const makeIssue = (overrides: Partial = {}) => ({ repository: { fullName: overrides.repository?.fullName ?? "test/repo" }, }) satisfies MockIssue; +function request(urlString: string, includeAuth = true) { + const headers: Record = {}; + if (includeAuth) headers.Authorization = `Bearer ${mockToken}`; + return new Request(urlString, { headers }); +} + beforeEach(() => { - mockFindManyData = []; - }); + delete process.env.DISPATCH_AUTH_MODE; + resetAuthCaches(); + vi.clearAllMocks(); + mockFindManyData = []; +}); + +describe("GET /api/issues/untriaged", () => { + it("returns 401 without authentication", async () => { + const response = await GET(request("http://localhost/api/issues/untriaged", false)); + const body = await response.json(); - describe("GET /api/issues/untriaged", () => { - // Reset mock data before each test + expect(response.status).toBe(401); + expect(body.error).toBe("Unauthorized"); + expect(mockFindMany).not.toHaveBeenCalled(); + }); it("returns only issues with no status/* label", async () => { mockFindManyData = [ @@ -102,7 +129,7 @@ beforeEach(() => { makeIssue({ number: 4, labels: ["status/backlog"] }), // has status — excluded ]; - const response = await GET(new Request("http://localhost/api/issues/untriaged")); + const response = await GET(request("http://localhost/api/issues/untriaged")); const body = await response.json(); expect(response.status).toBe(200); @@ -117,7 +144,7 @@ beforeEach(() => { makeIssue({ number: 2, labels: ["bug"], state: "closed" }), ]; - const response = await GET(new Request("http://localhost/api/issues/untriaged")); + const response = await GET(request("http://localhost/api/issues/untriaged")); const body = await response.json(); expect(body).toHaveLength(1); @@ -133,7 +160,7 @@ beforeEach(() => { makeIssue({ number: 4, labels: [] }), // truly unlabelled — should be included ]; - const response = await GET(new Request("http://localhost/api/issues/untriaged")); + const response = await GET(request("http://localhost/api/issues/untriaged")); const body = await response.json(); expect(body).toHaveLength(1); @@ -144,7 +171,7 @@ beforeEach(() => { it("respects limit parameter (default 50)", async () => { mockFindManyData = Array.from({ length: 100 }, (_, i) => makeIssue({ number: i + 1, labels: ["bug"] })); - const response = await GET(new Request("http://localhost/api/issues/untriaged")); + const response = await GET(request("http://localhost/api/issues/untriaged")); const body = await response.json(); expect(body).toHaveLength(50); // default limit @@ -154,7 +181,7 @@ beforeEach(() => { it("respects custom limit parameter", async () => { mockFindManyData = Array.from({ length: 100 }, (_, i) => makeIssue({ number: i + 1, labels: ["bug"] })); - const response = await GET(new Request("http://localhost/api/issues/untriaged?limit=10")); + const response = await GET(request("http://localhost/api/issues/untriaged?limit=10")); const body = await response.json(); expect(body).toHaveLength(10); @@ -164,7 +191,7 @@ beforeEach(() => { it("caps limit at 200", async () => { mockFindManyData = Array.from({ length: 500 }, (_, i) => makeIssue({ number: i + 1, labels: ["bug"] })); - const response = await GET(new Request("http://localhost/api/issues/untriaged?limit=999")); + const response = await GET(request("http://localhost/api/issues/untriaged?limit=999")); const body = await response.json(); expect(body).toHaveLength(200); // hard cap @@ -177,7 +204,7 @@ beforeEach(() => { makeIssue({ number: 2, labels: ["bug"], repository: { fullName: "other/repo" } }), ]; - const response = await GET(new Request("http://localhost/api/issues/untriaged?repo=test/repo")); + const response = await GET(request("http://localhost/api/issues/untriaged?repo=test/repo")); const body = await response.json(); expect(body).toHaveLength(1); @@ -191,7 +218,7 @@ beforeEach(() => { makeIssue({ number: 2, title: "Fix critical bug", labels: ["bug"] }), ]; - const response = await GET(new Request("http://localhost/api/issues/untriaged")); + const response = await GET(request("http://localhost/api/issues/untriaged")); const body = await response.json(); expect(body).toHaveLength(1); @@ -205,7 +232,7 @@ beforeEach(() => { makeIssue({ number: 2, title: "Fix critical bug", labels: ["bug"] }), ]; - const response = await GET(new Request("http://localhost/api/issues/untriaged?excludeRenovate=false")); + const response = await GET(request("http://localhost/api/issues/untriaged?excludeRenovate=false")); const body = await response.json(); expect(body).toHaveLength(2); @@ -218,7 +245,7 @@ beforeEach(() => { makeIssue({ number: 2, labels: ["status/backlog"] }), ]; - const response = await GET(new Request("http://localhost/api/issues/untriaged")); + const response = await GET(request("http://localhost/api/issues/untriaged")); const body = await response.json(); expect(body).toEqual([]); @@ -228,7 +255,7 @@ beforeEach(() => { it("returns correct issue shape with all fields", async () => { mockFindManyData = [makeIssue({ number: 42, title: "Untriaged bug", labels: ["bug"] })]; - const response = await GET(new Request("http://localhost/api/issues/untriaged")); + const response = await GET(request("http://localhost/api/issues/untriaged")); const body = await response.json(); expect(body).toHaveLength(1); @@ -251,7 +278,7 @@ beforeEach(() => { makeIssue({ number: 3, labels: ["bug"], updatedAt: new Date(baseDate.getTime() + 2000) }), ]; - const response = await GET(new Request("http://localhost/api/issues/untriaged")); + const response = await GET(request("http://localhost/api/issues/untriaged")); const body = await response.json(); expect(body.map((i: { number: number }) => i.number)).toEqual([3, 1, 2]); diff --git a/src/app/api/issues/untriaged/route.test.ts b/src/app/api/issues/untriaged/route.test.ts index 7c2769e..d8c1c59 100644 --- a/src/app/api/issues/untriaged/route.test.ts +++ b/src/app/api/issues/untriaged/route.test.ts @@ -35,21 +35,32 @@ vi.mock("@/lib/agent-queue", () => ({ })); import { GET } from "./route"; +import { resetAuthCaches } from "@/lib/auth"; -function request(urlString: string) { - return new Request(urlString, { headers: {} }); +function request(urlString: string, includeAuth = true) { + const headers: Record = {}; + if (includeAuth) headers.Authorization = `Bearer ${mockToken}`; + return new Request(urlString, { headers }); } describe("GET /api/issues/untriaged", () => { - // NOTE: This route is intentionally unauthenticated. It returns open issues - // with no status/* label to any caller. This is an intake view for grooming. - // In production deployments behind a firewall or auth gateway this is acceptable. beforeEach(() => { + delete process.env.DISPATCH_AUTH_MODE; + resetAuthCaches(); vi.clearAllMocks(); mocks.issueFindMany.mockResolvedValue([]); }); - it("returns untriaged issues without authentication", async () => { + it("returns 401 without authentication", async () => { + const res = await GET(request("http://localhost/api/issues/untriaged", false)); + + expect(res.status).toBe(401); + const body = await res.json(); + expect(body.error).toBe("Unauthorized"); + expect(mocks.issueFindMany).not.toHaveBeenCalled(); + }); + + it("returns untriaged issues with authentication", async () => { mocks.issueFindMany.mockResolvedValue([ { id: "i1", diff --git a/src/app/api/issues/untriaged/route.ts b/src/app/api/issues/untriaged/route.ts index 7668ef0..d89dad8 100644 --- a/src/app/api/issues/untriaged/route.ts +++ b/src/app/api/issues/untriaged/route.ts @@ -3,6 +3,7 @@ import { prisma } from "@/lib/prisma"; import { STATUS_LABELS } from "@/types"; import { isRenovateIssue } from "@/lib/agent-queue"; import { applyRenovateIssueExclusion } from "@/lib/issue-filters"; +import { authorizeRequest } from "@/lib/auth"; /** * GET /api/issues/untriaged @@ -30,6 +31,10 @@ interface UntriagedIssue { } export async function GET(request: Request) { + if (!(await authorizeRequest(request)).authorized) { + return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); + } + try { const { searchParams } = new URL(request.url); const limit = Math.min(