fix(search): honor extract match limits + harden pr-gardening candidate discovery (#9652)

## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - The `/pr-gardening` skill drives a bundled agent that scans a
company's issues for those linked to open GitHub PRs, then reports on
their state; it relies on the server's company-search **extract**
endpoint to pull PR references out of issue bodies
> - Two gaps surfaced during end-to-end QA of the gardening workflow:
the extract service silently ignored a per-issue match cap, so callers
could not bound how many matches came back per issue, and the skill's
candidate-discovery scripts fell over on large repos and on issues that
referenced deleted PRs
> - Left unaddressed, the gardener either truncated its scan
unpredictably or aborted outright, so it could not reliably enumerate PR
candidates
> - This pull request honors an explicit `matchesPerIssue` limit in the
extract search API and hardens the skill's candidate discovery against
missing/unavailable PRs and oversized `gh` output
> - The benefit is a PR-gardening workflow that scans deterministically
and finishes cleanly on real-world companies

## Linked Issues or Issue Description

No pre-existing public GitHub issue — describing the bug in-PR following
the bug report template (`.github/ISSUE_TEMPLATE/bug_report.yml`).

### What happened?

The company-search extract endpoint accepted a per-issue match limit but
did not apply it, returning matches capped only by the old hardcoded
constant regardless of the caller's request. Separately, the
`/pr-gardening` skill's candidate-discovery scripts crashed when a
scanned issue referenced a deleted PR (GitHub `Not Found (HTTP 404)` /
GraphQL `Could not resolve to a PullRequest`) and could exceed the
default `gh` output buffer on large result sets, aborting the whole
scan.

### Expected behavior

The extract API bounds matches per issue when a caller passes
`matchesPerIssue` (default 20, max 200), and omitting it preserves the
previous default. The gardening scripts skip PRs that are
deleted/unavailable and tolerate large `gh` responses without aborting
the scan.

### Steps to reproduce

1. Call the company-search extract endpoint with a `matchesPerIssue`
value against an issue containing many PR references — previously the
value was ignored.
2. Run the pr-gardening candidate scan against a company whose issues
reference a since-deleted PR — previously the scan threw instead of
skipping that PR.

### Paperclip version or commit

`master` at the base of this PR (branch cut from current
`origin/master`).

### Deployment mode

Local Paperclip instance / self-hosted.

## What Changed

- **Extract search honors `matchesPerIssue`**: added the
`matchesPerIssue` field to the shared search validator/types and applied
the cap in `company-search-extract` so results are bounded per issue
(`packages/shared`, `server/src/services/company-search-extract.ts`,
`doc/SPEC-implementation.md`).
- **Hardened pr-gardening candidate discovery**: `find-candidates.mjs` /
`lib.mjs` now request `matchesPerIssue=200`, treat missing/unavailable
PRs (deleted PR → `isMissingPullRequestError` / `unavailable`) as skips
instead of fatal errors, and raise the `gh` `maxBuffer` to 50 MB for
large repos.
- **Tests**: expanded `company-search-extract-{routes,service}.test.ts`
for the new limit and added coverage in `pr-gardening.test.mjs`.

## Verification

Re-run on a fresh worktree cherry-picked onto current `master`:

- `node --test
.agents/skills/pr-gardening/scripts/pr-gardening.test.mjs` → 8/8 pass
- `pnpm vitest run
server/src/__tests__/company-search-extract-routes.test.ts
server/src/__tests__/company-search-extract-service.test.ts` → 10/10
pass

## Risks

Low risk. `matchesPerIssue` is optional and backward-compatible
(omitting it preserves prior behavior). The skill changes only add
skip/tolerance paths and a larger buffer; no schema or migration
changes.

## Model Used

Claude — Opus 4.8 (`claude-opus-4-8`), extended thinking, tool use /
code execution via the Claude Agent SDK.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Dotta 2026-07-15 21:44:21 -05:00 committed by GitHub
parent 5588ddf681
commit ea0e899905
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 126 additions and 22 deletions

View File

@ -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,

View File

@ -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) {

View File

@ -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", () => {

View File

@ -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

View File

@ -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,

View File

@ -146,6 +146,7 @@ export interface CompanySearchExtractResponse {
scope: CompanySearchExtractScope;
limit: number;
offset: number;
matchesPerIssue: number;
results: CompanySearchExtractIssueResult[];
hasMore: boolean;
truncated: boolean;

View File

@ -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,

View File

@ -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)),

View File

@ -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 });
});

View File

@ -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");

View File

@ -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),