diff --git a/.agents/skills/pr-gardening/scripts/find-candidates.mjs b/.agents/skills/pr-gardening/scripts/find-candidates.mjs index 99a132d65e..616ccb2e8c 100755 --- a/.agents/skills/pr-gardening/scripts/find-candidates.mjs +++ b/.agents/skills/pr-gardening/scripts/find-candidates.mjs @@ -4,6 +4,7 @@ import { chooseOriginatingIssue, extractPullRequestNumber, ghJson, + isMissingPullRequestError, issueSummary, normalizeRepository, paperclipGet, @@ -29,6 +30,7 @@ export async function findCandidates(options) { const contains = `github.com/${repository}/pull`; const limit = 200; + const matchesPerIssue = 200; const issueMap = new Map(); let offset = 0; let truncated = false; @@ -41,6 +43,7 @@ export async function findCandidates(options) { updatedWithin: `${days}d`, limit: String(limit), offset: String(offset), + matchesPerIssue: String(matchesPerIssue), }); const page = await getPaperclip(`/companies/${companyId}/search/extract?${query}`, { apiUrl, apiKey }); for (const issue of page.results) issueMap.set(issue.issueId, issue); @@ -89,16 +92,29 @@ export async function findCandidates(options) { const candidates = []; const closed = []; + const unavailable = []; for (const entry of [...pullRequests.values()].sort((left, right) => left.number - right.number)) { - const pullRequest = getGhJson([ - "pr", - "view", - String(entry.number), - "--repo", - repository, - "--json", - "number,url,title,state,isDraft,headRefOid,updatedAt", - ]); + let pullRequest; + try { + pullRequest = getGhJson([ + "pr", + "view", + String(entry.number), + "--repo", + repository, + "--json", + "number,url,title,state,isDraft,headRefOid,updatedAt", + ]); + } catch (error) { + if (!isMissingPullRequestError(error)) throw error; + unavailable.push({ + number: entry.number, + url: prUrl(repository, entry.number), + state: "unavailable", + reason: "GitHub could not resolve this pull request", + }); + continue; + } const sourceIssues = [...entry.issueMentions.values()].sort((left, right) => right.updatedAt.localeCompare(left.updatedAt)); const candidate = { number: pullRequest.number, @@ -131,6 +147,7 @@ export async function findCandidates(options) { distinctPullRequestCount: pullRequests.size, openPullRequestCount: candidates.length, droppedClosedPullRequests: closed, + droppedUnavailablePullRequests: unavailable, truncated: false, }, candidates, diff --git a/.agents/skills/pr-gardening/scripts/lib.mjs b/.agents/skills/pr-gardening/scripts/lib.mjs index 9f0c990cf0..77dfcbc374 100644 --- a/.agents/skills/pr-gardening/scripts/lib.mjs +++ b/.agents/skills/pr-gardening/scripts/lib.mjs @@ -1,9 +1,10 @@ -import { execFileSync } from "node:child_process"; +import { spawnSync } from "node:child_process"; import { readFileSync, writeFileSync } from "node:fs"; export const GREEN_CHECK_CONCLUSIONS = new Set(["SUCCESS", "NEUTRAL", "SKIPPED"]); export const GREEN_STATUS_STATES = new Set(["SUCCESS"]); export const TERMINAL_ISSUE_STATUSES = new Set(["done", "cancelled"]); +const GH_JSON_MAX_BUFFER_BYTES = 50 * 1024 * 1024; export function parseArgs(argv, defaults = {}) { const args = { ...defaults }; @@ -33,8 +34,31 @@ export function writeJson(path, value) { } export function ghJson(args) { - const output = execFileSync("gh", args, { encoding: "utf8", stdio: ["ignore", "pipe", "inherit"] }); - return JSON.parse(output); + const result = spawnSync("gh", args, { + encoding: "utf8", + maxBuffer: GH_JSON_MAX_BUFFER_BYTES, + stdio: ["ignore", "pipe", "pipe"], + }); + // Surface gh diagnostics (warnings, deprecation/auth notices, error output) + // on both success and failure — spawnSync captures stderr in every case. + if (result.stderr) process.stderr.write(result.stderr); + if (result.error) throw result.error; + if (result.status !== 0) { + const error = new Error(`gh ${args.join(" ")} exited with status ${result.status}`); + error.stderr = result.stderr; + error.status = result.status; + throw error; + } + return JSON.parse(result.stdout); +} + +export function isMissingPullRequestError(error) { + const detail = `${error?.message ?? ""}\n${error?.stderr ?? ""}`; + // Scope to the exact signals gh emits for a deleted/nonexistent PR: the GraphQL + // "Could not resolve to a PullRequest" message and REST "Not Found (HTTP 404)". + // A bare "Not Found" would over-match unrelated failures (e.g. "repository not + // found"), so we require the HTTP 404 marker for the REST case. + return /Could not resolve to a PullRequest|HTTP 404/i.test(detail); } export function normalizeRepository(value) { diff --git a/.agents/skills/pr-gardening/scripts/pr-gardening.test.mjs b/.agents/skills/pr-gardening/scripts/pr-gardening.test.mjs index c4d8abdabe..f697635ccc 100644 --- a/.agents/skills/pr-gardening/scripts/pr-gardening.test.mjs +++ b/.agents/skills/pr-gardening/scripts/pr-gardening.test.mjs @@ -2,7 +2,7 @@ import assert from "node:assert/strict"; import test from "node:test"; import { confidenceFor, readinessVerdict } from "./check-readiness.mjs"; import { findCandidates } from "./find-candidates.mjs"; -import { chooseOriginatingIssue, extractPullRequestNumber, normalizeCheck } from "./lib.mjs"; +import { chooseOriginatingIssue, extractPullRequestNumber, isMissingPullRequestError, normalizeCheck } from "./lib.mjs"; import { renderReport } from "./render-report.mjs"; test("extracts only pull requests from the requested repository", () => { @@ -38,8 +38,10 @@ test("origin selection prioritizes work products then comment mentions", () => { }); test("candidate discovery deduplicates mentions and drops closed PRs", async () => { + let extractPath = ""; const paperclipGet = async (path) => { if (path.includes("search/extract")) { + extractPath = path; return { hasMore: false, results: [ @@ -55,6 +57,7 @@ test("candidate discovery deduplicates mentions and drops closed PRs", async () { value: "https://github.com/paperclipai/paperclip/pull/1", field: "comment", label: "Comment", source: { type: "comment", commentId: "c1" } }, { value: "https://github.com/paperclipai/paperclip/pull/1", field: "document_body", label: "Document", source: { type: "document", documentId: "d1", documentKey: "plan" } }, { value: "https://github.com/paperclipai/paperclip/pull/2", field: "description", label: "Description", source: { type: "issue", issueId: "issue-1" } }, + { value: "https://github.com/paperclipai/paperclip/pull/3", field: "description", label: "Description", source: { type: "issue", issueId: "issue-1" } }, ], }, ], @@ -64,6 +67,7 @@ test("candidate discovery deduplicates mentions and drops closed PRs", async () }; const ghJson = (args) => { const number = Number(args[2]); + if (number === 3) throw new Error("GraphQL: Could not resolve to a PullRequest with the number of 3"); return { number, url: `https://github.com/paperclipai/paperclip/pull/${number}`, @@ -83,9 +87,21 @@ test("candidate discovery deduplicates mentions and drops closed PRs", async () gh_json: ghJson, }); assert.deepEqual(result.candidates.map((candidate) => candidate.number), [1]); + assert.equal(new URL(`http://paperclip.test${extractPath}`).searchParams.get("matchesPerIssue"), "200"); assert.equal(result.candidates[0].sourceIssues[0].mentions.length, 2); assert.equal(result.candidates[0].originatingIssue.selectionBasis, "pull_request_work_product"); assert.deepEqual(result.source.droppedClosedPullRequests.map((pullRequest) => pullRequest.number), [2]); + assert.deepEqual(result.source.droppedUnavailablePullRequests.map((pullRequest) => pullRequest.number), [3]); +}); + +test("missing-PR detection matches only deleted/nonexistent PR signals", () => { + // gh's real signals for a deleted/nonexistent PR: GraphQL resolution failure and REST 404. + assert.equal(isMissingPullRequestError(new Error("GraphQL: Could not resolve to a PullRequest with the number of 3")), true); + assert.equal(isMissingPullRequestError({ stderr: "gh: Not Found (HTTP 404)" }), true); + // Unrelated failures that merely contain "not found" must not be treated as skippable. + assert.equal(isMissingPullRequestError(new Error("repository not found")), false); + assert.equal(isMissingPullRequestError(new Error("could not connect to github.com")), false); + assert.equal(isMissingPullRequestError(undefined), false); }); test("normalizes check runs and status contexts", () => { diff --git a/doc/SPEC-implementation.md b/doc/SPEC-implementation.md index 4fff1753c8..c151566373 100644 --- a/doc/SPEC-implementation.md +++ b/doc/SPEC-implementation.md @@ -928,8 +928,8 @@ The current app also exposes V1-supporting surfaces for: - issue approvals, issue references/search, labels, read state, inbox/archive state, and work products - company search through `GET /companies/:companyId/search` plus agent-oriented bulk extraction through `GET /companies/:companyId/search/extract`; extraction accepts a server-escaped literal `contains`, optional - server-owned URL expansion, issue/comment/document scopes, status/date filters, issue-level pagination, and - explicit issue/match truncation flags + server-owned URL expansion, issue/comment/document scopes, status/date filters, issue-level pagination, a + bounded `matchesPerIssue` override for machine consumers, and explicit issue/match truncation flags - execution workspaces, project workspaces, workspace runtime services, and workspace operations - task watchdog configuration and reusable watchdog issue orchestration for explicitly watched issue subtrees - routines and scheduled/API/webhook triggers diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index 0802e3e238..7184a011ba 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -1481,6 +1481,7 @@ export { companySearchExtractQuerySchema, companySearchQuerySchema, COMPANY_SEARCH_EXTRACT_DEFAULT_LIMIT, + COMPANY_SEARCH_EXTRACT_DEFAULT_MATCHES_PER_ISSUE, COMPANY_SEARCH_EXTRACT_MAX_LIMIT, COMPANY_SEARCH_EXTRACT_MAX_MATCHES_PER_ISSUE, COMPANY_SEARCH_EXTRACT_MAX_OFFSET, diff --git a/packages/shared/src/types/search.ts b/packages/shared/src/types/search.ts index 1b812bba5d..481766eca5 100644 --- a/packages/shared/src/types/search.ts +++ b/packages/shared/src/types/search.ts @@ -146,6 +146,7 @@ export interface CompanySearchExtractResponse { scope: CompanySearchExtractScope; limit: number; offset: number; + matchesPerIssue: number; results: CompanySearchExtractIssueResult[]; hasMore: boolean; truncated: boolean; diff --git a/packages/shared/src/validators/index.ts b/packages/shared/src/validators/index.ts index 2fa890e13e..58853af7a9 100644 --- a/packages/shared/src/validators/index.ts +++ b/packages/shared/src/validators/index.ts @@ -421,6 +421,7 @@ export { export { COMPANY_SEARCH_EXTRACT_DEFAULT_LIMIT, + COMPANY_SEARCH_EXTRACT_DEFAULT_MATCHES_PER_ISSUE, COMPANY_SEARCH_EXTRACT_MAX_LIMIT, COMPANY_SEARCH_EXTRACT_MAX_MATCHES_PER_ISSUE, COMPANY_SEARCH_EXTRACT_MAX_OFFSET, diff --git a/packages/shared/src/validators/search.ts b/packages/shared/src/validators/search.ts index b54922c1e6..839f4cb529 100644 --- a/packages/shared/src/validators/search.ts +++ b/packages/shared/src/validators/search.ts @@ -16,7 +16,8 @@ export const COMPANY_SEARCH_MAX_OFFSET = 200; export const COMPANY_SEARCH_EXTRACT_DEFAULT_LIMIT = 100; export const COMPANY_SEARCH_EXTRACT_MAX_LIMIT = 200; export const COMPANY_SEARCH_EXTRACT_MAX_OFFSET = 5_000; -export const COMPANY_SEARCH_EXTRACT_MAX_MATCHES_PER_ISSUE = 20; +export const COMPANY_SEARCH_EXTRACT_DEFAULT_MATCHES_PER_ISSUE = 20; +export const COMPANY_SEARCH_EXTRACT_MAX_MATCHES_PER_ISSUE = 200; const UPDATED_WITHIN_RE = /^[1-9]\d{0,2}(h|d|w|m)$/; @@ -238,6 +239,16 @@ export const companySearchExtractQuerySchema = z.object({ offset: z.unknown() .optional() .transform((value, ctx) => parseIntegerQuery(value, ctx, "offset", 0, 0, COMPANY_SEARCH_EXTRACT_MAX_OFFSET)), + matchesPerIssue: z.unknown() + .optional() + .transform((value, ctx) => parseIntegerQuery( + value, + ctx, + "matchesPerIssue", + COMPANY_SEARCH_EXTRACT_DEFAULT_MATCHES_PER_ISSUE, + 1, + COMPANY_SEARCH_EXTRACT_MAX_MATCHES_PER_ISSUE, + )), status: z.unknown() .optional() .transform((value, ctx) => parseEnumList(value, ctx, "status", ISSUE_STATUSES)), diff --git a/server/src/__tests__/company-search-extract-routes.test.ts b/server/src/__tests__/company-search-extract-routes.test.ts index 67a7144c96..375247858b 100644 --- a/server/src/__tests__/company-search-extract-routes.test.ts +++ b/server/src/__tests__/company-search-extract-routes.test.ts @@ -17,6 +17,7 @@ function extractResponse(query: CompanySearchExtractQuery): CompanySearchExtract scope: query.scope, limit: query.limit, offset: query.offset, + matchesPerIssue: query.matchesPerIssue, results: [], hasMore: false, truncated: false, @@ -57,7 +58,13 @@ describe("company extract-search route", () => { const response = await request(app) .get("/api/companies/company-1/search/extract") - .query({ contains: "github.com/example/repo/pull", kind: "url", scope: "comments", limit: "200" }) + .query({ + contains: "github.com/example/repo/pull", + kind: "url", + scope: "comments", + limit: "200", + matchesPerIssue: "200", + }) .expect(200); expect(extract).toHaveBeenCalledWith("company-1", expect.objectContaining({ @@ -65,6 +72,7 @@ describe("company extract-search route", () => { kind: "url", scope: "comments", limit: 200, + matchesPerIssue: 200, })); expect(response.body).toMatchObject({ kind: "url", scope: "comments", truncated: false }); }); diff --git a/server/src/__tests__/company-search-extract-service.test.ts b/server/src/__tests__/company-search-extract-service.test.ts index e03b7c6de6..d50c0e149a 100644 --- a/server/src/__tests__/company-search-extract-service.test.ts +++ b/server/src/__tests__/company-search-extract-service.test.ts @@ -9,6 +9,7 @@ import { issues, } from "@paperclipai/db"; import { + COMPANY_SEARCH_EXTRACT_DEFAULT_MATCHES_PER_ISSUE, COMPANY_SEARCH_EXTRACT_MAX_MATCHES_PER_ISSUE, companySearchExtractQuerySchema, } from "@paperclipai/shared"; @@ -36,15 +37,18 @@ describe("extract-search query validation", () => { status: "in_progress,in_review", limit: "200", offset: "5000", + matchesPerIssue: "200", updatedWithin: "30d", }); expect(parsed.kind).toBe("url"); expect(parsed.scope).toBe("comments"); expect(parsed.status).toEqual(["in_progress", "in_review"]); + expect(parsed.matchesPerIssue).toBe(COMPANY_SEARCH_EXTRACT_MAX_MATCHES_PER_ISSUE); expect(() => companySearchExtractQuerySchema.parse({ contains: ".*", kind: "regex" })).toThrow(); expect(() => companySearchExtractQuerySchema.parse({ contains: "x" })).toThrow(); expect(() => companySearchExtractQuerySchema.parse({ contains: "needle", limit: "201" })).toThrow(); + expect(() => companySearchExtractQuerySchema.parse({ contains: "needle", matchesPerIssue: "201" })).toThrow(); expect(() => companySearchExtractQuerySchema.parse({ contains: "needle", updatedWithin: "30d", @@ -214,10 +218,10 @@ describeEmbeddedPostgres("companySearchExtractService", () => { expect(result.results.map((row) => row.issueId)).toEqual([recentId]); }); - it("caps distinct matches per issue and marks truncation explicitly", async () => { + it("uses the default distinct-match cap and marks truncation explicitly", async () => { const companyId = await createCompany(); const urls = Array.from( - { length: COMPANY_SEARCH_EXTRACT_MAX_MATCHES_PER_ISSUE + 1 }, + { length: COMPANY_SEARCH_EXTRACT_DEFAULT_MATCHES_PER_ISSUE + 1 }, (_, index) => `https://github.com/paperclipai/paperclip/pull/${index + 1}`, ); await createIssue(companyId, { description: urls.join(" ") }); @@ -227,11 +231,32 @@ describeEmbeddedPostgres("companySearchExtractService", () => { kind: "url", })); - expect(result.results[0]?.matches).toHaveLength(COMPANY_SEARCH_EXTRACT_MAX_MATCHES_PER_ISSUE); + expect(result.matchesPerIssue).toBe(COMPANY_SEARCH_EXTRACT_DEFAULT_MATCHES_PER_ISSUE); + expect(result.results[0]?.matches).toHaveLength(COMPANY_SEARCH_EXTRACT_DEFAULT_MATCHES_PER_ISSUE); expect(result.results[0]?.matchesTruncated).toBe(true); expect(result.truncated).toBe(true); }); + it("supports a bounded per-issue match cap for complete machine extraction", async () => { + const companyId = await createCompany(); + const urls = Array.from( + { length: COMPANY_SEARCH_EXTRACT_DEFAULT_MATCHES_PER_ISSUE + 1 }, + (_, index) => `https://github.com/paperclipai/paperclip/pull/${index + 1}`, + ); + await createIssue(companyId, { description: urls.join(" ") }); + + const result = await svc.extract(companyId, companySearchExtractQuerySchema.parse({ + contains: "github.com/paperclipai/paperclip/pull", + kind: "url", + matchesPerIssue: COMPANY_SEARCH_EXTRACT_MAX_MATCHES_PER_ISSUE, + })); + + expect(result.matchesPerIssue).toBe(COMPANY_SEARCH_EXTRACT_MAX_MATCHES_PER_ISSUE); + expect(result.results[0]?.matches).toHaveLength(urls.length); + expect(result.results[0]?.matchesTruncated).toBe(false); + expect(result.truncated).toBe(false); + }); + it("does not return matching issues from another company", async () => { const companyId = await createCompany(); const otherCompanyId = await createCompany("Other"); diff --git a/server/src/services/company-search-extract.ts b/server/src/services/company-search-extract.ts index 6462b33445..e8c59d17f1 100644 --- a/server/src/services/company-search-extract.ts +++ b/server/src/services/company-search-extract.ts @@ -3,7 +3,6 @@ import type { SQL, SQLWrapper } from "drizzle-orm"; import type { Db } from "@paperclipai/db"; import { documents, issueComments, issueDocuments, issues } from "@paperclipai/db"; import { - COMPANY_SEARCH_EXTRACT_MAX_MATCHES_PER_ISSUE, type CompanySearchExtractIssueResult, type CompanySearchExtractMatch, type CompanySearchExtractQuery, @@ -131,7 +130,7 @@ function extractMatches(sources: ExtractSource[], query: CompanySearchExtractQue const dedupeKey = occurrence.value.toLowerCase(); if (seen.has(dedupeKey)) continue; seen.add(dedupeKey); - if (matches.length >= COMPANY_SEARCH_EXTRACT_MAX_MATCHES_PER_ISSUE) { + if (matches.length >= query.matchesPerIssue) { matchesTruncated = true; continue; } @@ -340,6 +339,7 @@ export function companySearchExtractService(db: Db) { scope: query.scope, limit: query.limit, offset: query.offset, + matchesPerIssue: query.matchesPerIssue, results, hasMore, truncated: hasMore || results.some((result) => result.matchesTruncated),