Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions packages/loopover-engine/src/discovery-index-contract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ export type DiscoveryIndexCandidate = {
issueNumber: number;
title: string;
labels: readonly string[];
/** Repo login(s) the issue is assigned to on GitHub (#7442). OPTIONAL: an older, backward-compatible hosted
* server build that doesn't populate it leaves the field absent, which lets a client distinguish "assignees
* were never served" from "served, and empty" -- the former falls back to [] at the merge site rather than
* silently no-opping contribution-profile-filter's repo-owner exclusion (#7040). Normalized like `labels`. */
assignees?: readonly string[];
commentsCount: number;
createdAt: string | null;
updatedAt: string | null;
Expand Down Expand Up @@ -197,13 +202,20 @@ export function normalizeDiscoveryIndexCandidate(raw: unknown): DiscoveryIndexCa
const labels = Array.isArray(candidate.labels)
? candidate.labels.filter((label): label is string => typeof label === "string" && label.trim() !== "").map((label) => label.trim())
: [];
// #7442: carry real assignees through when the server provides them (same string normalization as `labels`); an
// older build omits the field, leaving `assignees` off the result (undefined) so the merge site falls back to []
// -- fail-safe, never silently skipping the repo-owner exclusion filter.
const assignees = Array.isArray(candidate.assignees)
? candidate.assignees.filter((entry): entry is string => typeof entry === "string" && entry.trim() !== "").map((entry) => entry.trim())
: undefined;
return {
owner,
repo,
repoFullName: canonical,
issueNumber,
title,
labels,
...(assignees !== undefined ? { assignees } : {}),
commentsCount: typeof candidate.commentsCount === "number" && Number.isFinite(candidate.commentsCount) ? candidate.commentsCount : 0,
createdAt: typeof candidate.createdAt === "string" ? candidate.createdAt : null,
updatedAt: typeof candidate.updatedAt === "string" ? candidate.updatedAt : null,
Expand Down
17 changes: 10 additions & 7 deletions packages/loopover-miner/lib/discover-cli.js

Large diffs are not rendered by default.

15 changes: 9 additions & 6 deletions packages/loopover-miner/lib/discover-cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,11 @@ function dedupeKey(repoFullName: string, issueNumber: number): string {
* no-op (returns `fanOut` unchanged) unless the plane is enabled, so a run with the flag unset behaves exactly
* as before this feature existed. Local results always win on a duplicate issue (the discovery-index candidate
* is dropped, not merged over it) -- this instance's own live fan-out is more current than a cached shared
* index entry. Discovery-index candidates lack `assignees` (not part of the public contract), so they're
* annotated with an empty array to match opportunity-fanout.js's own candidate shape; contribution-profile-
* filter.js's assignee-exclusion rule treats that identically to "no assignees on this issue".
* index entry. Discovery-index candidates now carry their real `assignees` when the hosted contract supplies them
* (#7442): the value flows through so contribution-profile-filter.js's repo-owner exclusion (#7040) engages for
* index-sourced candidates exactly as it does for direct fan-out ones. A candidate whose response omitted the
* field (older discovery-index build) falls back to `[]` -- fail-safe: the filter still runs, it just can't detect
* an owner-assignment it was never told about, rather than the check being silently skipped.
*/
async function supplementWithDiscoveryIndex(
fanOut: DiscoverFanOutSummary,
Expand All @@ -181,9 +183,10 @@ async function supplementWithDiscoveryIndex(
const seen = new Set(fanOut.issues.map((issue) => dedupeKey(issue.repoFullName, issue.issueNumber)));
const supplemented = response.candidates
.filter((candidate) => !seen.has(dedupeKey(candidate.repoFullName, candidate.issueNumber)))
// DiscoveryIndexCandidate is a near-superset of RawCandidateIssue; assignees is absent from the hosted
// contract (#7168) so we annotate [] — cast preserves pre-existing runtime shape rather than re-mapping.
.map((candidate) => ({ ...candidate, assignees: [], labels: [...candidate.labels] }) as RawCandidateIssue);
// DiscoveryIndexCandidate is a near-superset of RawCandidateIssue; copy the real assignees through when the
// hosted contract carried them (#7442), falling back to [] only when the served response genuinely omitted the
// field — cast preserves pre-existing runtime shape rather than re-mapping.
.map((candidate) => ({ ...candidate, assignees: [...(candidate.assignees ?? [])], labels: [...candidate.labels] }) as RawCandidateIssue);
if (supplemented.length === 0) return fanOut;
return { ...fanOut, issues: [...fanOut.issues, ...supplemented] };
}
Expand Down
12 changes: 12 additions & 0 deletions test/unit/discovery-index-contract.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,18 @@ describe("discovery-index API contract (#4300)", () => {
});
});

it("normalizes assignees like labels, and omits the field entirely when the response never carries it (#7442)", () => {
// Present: filtered + trimmed exactly like labels (non-strings and blanks dropped).
expect(
normalizeDiscoveryIndexCandidate({ repoFullName: "acme/widgets", issueNumber: 1, title: "t", assignees: ["acme", " ", 42, " maintainer "] })?.assignees,
).toEqual(["acme", "maintainer"]);
// Explicitly served empty: present as [] (a real "no assignees" signal).
expect(normalizeDiscoveryIndexCandidate({ repoFullName: "acme/widgets", issueNumber: 1, title: "t", assignees: [] })?.assignees).toEqual([]);
// Never served (older server build): the field is omitted, so a client can tell it apart from served-empty
// and fall back to [] at the merge site rather than silently skipping the repo-owner exclusion filter.
expect("assignees" in (normalizeDiscoveryIndexCandidate({ repoFullName: "acme/widgets", issueNumber: 1, title: "t" }) ?? {})).toBe(false);
});

it("maps aiPolicySource and respects an explicit aiPolicyAllowed:false", () => {
expect(normalizeDiscoveryIndexCandidate({ repoFullName: "o/r", issueNumber: 1, title: "t", aiPolicySource: "CONTRIBUTING.md", aiPolicyAllowed: false })).toMatchObject({
aiPolicySource: "CONTRIBUTING.md",
Expand Down
81 changes: 81 additions & 0 deletions test/unit/miner-discover-cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,26 @@ function fanOutIssue(overrides: Record<string, unknown> = {}) {
};
}

// A hosted discovery-index candidate (#7168 shape). assignees is optional in the contract (#7442) — omit it to
// simulate an older server build that never populated the field.
function indexCandidate(overrides: Record<string, unknown> = {}) {
return {
owner: "acme",
repo: "widgets",
repoFullName: "acme/widgets",
issueNumber: 2,
title: "index-sourced issue",
labels: ["help wanted"],
commentsCount: 0,
createdAt: null,
updatedAt: null,
htmlUrl: null,
aiPolicyAllowed: true as const,
aiPolicySource: "none" as const,
...overrides,
};
}

afterEach(() => {
for (const store of stores.splice(0)) store.close();
closeDefaultPortfolioQueueStore();
Expand Down Expand Up @@ -469,6 +489,67 @@ describe("runDiscover (#4247)", () => {
]);
});

it("REGRESSION (#7442): a discovery-index candidate assigned to its own repo owner is excluded once real assignees flow through", async () => {
const portfolioQueue = tempQueueStore();
const fetchCandidateIssuesWithSummary = vi.fn(async () => ({
issues: [fanOutIssue({ issueNumber: 1, title: "direct fan-out issue" })],
warnings: [],
rateLimitRemaining: 5000,
rateLimitResetAt: "2026-07-09T13:00:00.000Z",
}));
// The hosted index supplies a DIFFERENT issue (so it survives dedupe) assigned to the repo owner "acme".
const queryDiscoveryIndex = vi.fn(async () => ({ contractVersion: 1, candidates: [indexCandidate({ issueNumber: 2, assignees: ["acme"] })], nextCursor: null }));

const log = vi.spyOn(console, "log").mockImplementation(() => undefined);
const exitCode = await runDiscover(["acme/widgets", "--json"], {
nowMs: NOW,
env: { ...process.env, LOOPOVER_MINER_DISCOVERY_PLANE: "1" },
initPortfolioQueue: () => portfolioQueue,
initPolicyDocCache: () => tempPolicyDocCacheStore(),
initPolicyVerdictCache: () => tempPolicyVerdictCacheStore(),
initRankedCandidatesStore: () => tempRankedCandidatesStore(),
fetchCandidateIssuesWithSummary,
queryDiscoveryIndex: queryDiscoveryIndex as never,
});

expect(exitCode).toBe(0);
expect(queryDiscoveryIndex).toHaveBeenCalled();
const payload = JSON.parse(String(log.mock.calls[0]?.[0]));
// The owner-assigned index candidate (issue 2) is excluded by the #7040 filter; only the direct fan-out issue survives.
expect(payload.ranked.map((e: { issueNumber: number }) => e.issueNumber)).toEqual([1]);
expect(portfolioQueue.listQueue("acme/widgets").map((e) => e.identifier)).toEqual(["issue:1"]);
});

it("REGRESSION (#7442): a discovery-index response that omits assignees falls back to [] and still runs the filter (kept, not skipped)", async () => {
const portfolioQueue = tempQueueStore();
const fetchCandidateIssuesWithSummary = vi.fn(async () => ({
issues: [fanOutIssue({ issueNumber: 1, title: "direct fan-out issue" })],
warnings: [],
rateLimitRemaining: 5000,
rateLimitResetAt: "2026-07-09T13:00:00.000Z",
}));
// Older server build: the candidate carries NO assignees field at all.
const queryDiscoveryIndex = vi.fn(async () => ({ contractVersion: 1, candidates: [indexCandidate({ issueNumber: 2 })], nextCursor: null }));

const log = vi.spyOn(console, "log").mockImplementation(() => undefined);
const exitCode = await runDiscover(["acme/widgets", "--json"], {
nowMs: NOW,
env: { ...process.env, LOOPOVER_MINER_DISCOVERY_PLANE: "1" },
initPortfolioQueue: () => portfolioQueue,
initPolicyDocCache: () => tempPolicyDocCacheStore(),
initPolicyVerdictCache: () => tempPolicyVerdictCacheStore(),
initRankedCandidatesStore: () => tempRankedCandidatesStore(),
fetchCandidateIssuesWithSummary,
queryDiscoveryIndex: queryDiscoveryIndex as never,
});

expect(exitCode).toBe(0);
const payload = JSON.parse(String(log.mock.calls[0]?.[0]));
// Fail-safe: the omitted field becomes [], the owner-exclusion filter still runs (finds no assignment), issue 2 is KEPT.
expect(payload.ranked.map((e: { issueNumber: number }) => e.issueNumber).sort()).toEqual([1, 2]);
expect(portfolioQueue.listQueue("acme/widgets").map((e) => e.identifier).sort()).toEqual(["issue:1", "issue:2"]);
});

it("#4847: --dry-run performs the real fan-out/rank but never opens any local store", async () => {
const initPortfolioQueue = vi.fn();
const initPolicyDocCache = vi.fn();
Expand Down