Skip to content

Commit e7cbb1b

Browse files
authored
fix(miner): carry real assignees through the discovery-index supplement so the repo-owner exclusion applies (#7488)
discover-cli's supplementWithDiscoveryIndex stubbed assignees: [] on every hosted discovery-index candidate, so contribution-profile-filter's always-on repo-owner exclusion (#7040) -- which reads candidate.assignees -- could never fire for an index-sourced candidate, silently no-opping the safeguard for that entire source. Add an optional assignees field to DiscoveryIndexCandidate (normalized like labels, kept optional so 'never served' is distinguishable from 'served empty') and copy the real value through in supplementWithDiscoveryIndex, falling back to [] only when the served response genuinely omitted it. isAssignedToRepoOwner's logic is unchanged; the direct fan-out path already carried real assignees. Additive and backward-compatible. Adds contract + end-to-end discover-cli regression tests. Closes #7442
1 parent 87766eb commit e7cbb1b

5 files changed

Lines changed: 124 additions & 13 deletions

File tree

packages/loopover-engine/src/discovery-index-contract.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,11 @@ export type DiscoveryIndexCandidate = {
4848
issueNumber: number;
4949
title: string;
5050
labels: readonly string[];
51+
/** Repo login(s) the issue is assigned to on GitHub (#7442). OPTIONAL: an older, backward-compatible hosted
52+
* server build that doesn't populate it leaves the field absent, which lets a client distinguish "assignees
53+
* were never served" from "served, and empty" -- the former falls back to [] at the merge site rather than
54+
* silently no-opping contribution-profile-filter's repo-owner exclusion (#7040). Normalized like `labels`. */
55+
assignees?: readonly string[];
5156
commentsCount: number;
5257
createdAt: string | null;
5358
updatedAt: string | null;
@@ -197,13 +202,20 @@ export function normalizeDiscoveryIndexCandidate(raw: unknown): DiscoveryIndexCa
197202
const labels = Array.isArray(candidate.labels)
198203
? candidate.labels.filter((label): label is string => typeof label === "string" && label.trim() !== "").map((label) => label.trim())
199204
: [];
205+
// #7442: carry real assignees through when the server provides them (same string normalization as `labels`); an
206+
// older build omits the field, leaving `assignees` off the result (undefined) so the merge site falls back to []
207+
// -- fail-safe, never silently skipping the repo-owner exclusion filter.
208+
const assignees = Array.isArray(candidate.assignees)
209+
? candidate.assignees.filter((entry): entry is string => typeof entry === "string" && entry.trim() !== "").map((entry) => entry.trim())
210+
: undefined;
200211
return {
201212
owner,
202213
repo,
203214
repoFullName: canonical,
204215
issueNumber,
205216
title,
206217
labels,
218+
...(assignees !== undefined ? { assignees } : {}),
207219
commentsCount: typeof candidate.commentsCount === "number" && Number.isFinite(candidate.commentsCount) ? candidate.commentsCount : 0,
208220
createdAt: typeof candidate.createdAt === "string" ? candidate.createdAt : null,
209221
updatedAt: typeof candidate.updatedAt === "string" ? candidate.updatedAt : null,

packages/loopover-miner/lib/discover-cli.js

Lines changed: 10 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/loopover-miner/lib/discover-cli.ts

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

test/unit/discovery-index-contract.test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,18 @@ describe("discovery-index API contract (#4300)", () => {
122122
});
123123
});
124124

125+
it("normalizes assignees like labels, and omits the field entirely when the response never carries it (#7442)", () => {
126+
// Present: filtered + trimmed exactly like labels (non-strings and blanks dropped).
127+
expect(
128+
normalizeDiscoveryIndexCandidate({ repoFullName: "acme/widgets", issueNumber: 1, title: "t", assignees: ["acme", " ", 42, " maintainer "] })?.assignees,
129+
).toEqual(["acme", "maintainer"]);
130+
// Explicitly served empty: present as [] (a real "no assignees" signal).
131+
expect(normalizeDiscoveryIndexCandidate({ repoFullName: "acme/widgets", issueNumber: 1, title: "t", assignees: [] })?.assignees).toEqual([]);
132+
// Never served (older server build): the field is omitted, so a client can tell it apart from served-empty
133+
// and fall back to [] at the merge site rather than silently skipping the repo-owner exclusion filter.
134+
expect("assignees" in (normalizeDiscoveryIndexCandidate({ repoFullName: "acme/widgets", issueNumber: 1, title: "t" }) ?? {})).toBe(false);
135+
});
136+
125137
it("maps aiPolicySource and respects an explicit aiPolicyAllowed:false", () => {
126138
expect(normalizeDiscoveryIndexCandidate({ repoFullName: "o/r", issueNumber: 1, title: "t", aiPolicySource: "CONTRIBUTING.md", aiPolicyAllowed: false })).toMatchObject({
127139
aiPolicySource: "CONTRIBUTING.md",

test/unit/miner-discover-cli.test.ts

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,26 @@ function fanOutIssue(overrides: Record<string, unknown> = {}) {
8282
};
8383
}
8484

85+
// A hosted discovery-index candidate (#7168 shape). assignees is optional in the contract (#7442) — omit it to
86+
// simulate an older server build that never populated the field.
87+
function indexCandidate(overrides: Record<string, unknown> = {}) {
88+
return {
89+
owner: "acme",
90+
repo: "widgets",
91+
repoFullName: "acme/widgets",
92+
issueNumber: 2,
93+
title: "index-sourced issue",
94+
labels: ["help wanted"],
95+
commentsCount: 0,
96+
createdAt: null,
97+
updatedAt: null,
98+
htmlUrl: null,
99+
aiPolicyAllowed: true as const,
100+
aiPolicySource: "none" as const,
101+
...overrides,
102+
};
103+
}
104+
85105
afterEach(() => {
86106
for (const store of stores.splice(0)) store.close();
87107
closeDefaultPortfolioQueueStore();
@@ -469,6 +489,67 @@ describe("runDiscover (#4247)", () => {
469489
]);
470490
});
471491

492+
it("REGRESSION (#7442): a discovery-index candidate assigned to its own repo owner is excluded once real assignees flow through", async () => {
493+
const portfolioQueue = tempQueueStore();
494+
const fetchCandidateIssuesWithSummary = vi.fn(async () => ({
495+
issues: [fanOutIssue({ issueNumber: 1, title: "direct fan-out issue" })],
496+
warnings: [],
497+
rateLimitRemaining: 5000,
498+
rateLimitResetAt: "2026-07-09T13:00:00.000Z",
499+
}));
500+
// The hosted index supplies a DIFFERENT issue (so it survives dedupe) assigned to the repo owner "acme".
501+
const queryDiscoveryIndex = vi.fn(async () => ({ contractVersion: 1, candidates: [indexCandidate({ issueNumber: 2, assignees: ["acme"] })], nextCursor: null }));
502+
503+
const log = vi.spyOn(console, "log").mockImplementation(() => undefined);
504+
const exitCode = await runDiscover(["acme/widgets", "--json"], {
505+
nowMs: NOW,
506+
env: { ...process.env, LOOPOVER_MINER_DISCOVERY_PLANE: "1" },
507+
initPortfolioQueue: () => portfolioQueue,
508+
initPolicyDocCache: () => tempPolicyDocCacheStore(),
509+
initPolicyVerdictCache: () => tempPolicyVerdictCacheStore(),
510+
initRankedCandidatesStore: () => tempRankedCandidatesStore(),
511+
fetchCandidateIssuesWithSummary,
512+
queryDiscoveryIndex: queryDiscoveryIndex as never,
513+
});
514+
515+
expect(exitCode).toBe(0);
516+
expect(queryDiscoveryIndex).toHaveBeenCalled();
517+
const payload = JSON.parse(String(log.mock.calls[0]?.[0]));
518+
// The owner-assigned index candidate (issue 2) is excluded by the #7040 filter; only the direct fan-out issue survives.
519+
expect(payload.ranked.map((e: { issueNumber: number }) => e.issueNumber)).toEqual([1]);
520+
expect(portfolioQueue.listQueue("acme/widgets").map((e) => e.identifier)).toEqual(["issue:1"]);
521+
});
522+
523+
it("REGRESSION (#7442): a discovery-index response that omits assignees falls back to [] and still runs the filter (kept, not skipped)", async () => {
524+
const portfolioQueue = tempQueueStore();
525+
const fetchCandidateIssuesWithSummary = vi.fn(async () => ({
526+
issues: [fanOutIssue({ issueNumber: 1, title: "direct fan-out issue" })],
527+
warnings: [],
528+
rateLimitRemaining: 5000,
529+
rateLimitResetAt: "2026-07-09T13:00:00.000Z",
530+
}));
531+
// Older server build: the candidate carries NO assignees field at all.
532+
const queryDiscoveryIndex = vi.fn(async () => ({ contractVersion: 1, candidates: [indexCandidate({ issueNumber: 2 })], nextCursor: null }));
533+
534+
const log = vi.spyOn(console, "log").mockImplementation(() => undefined);
535+
const exitCode = await runDiscover(["acme/widgets", "--json"], {
536+
nowMs: NOW,
537+
env: { ...process.env, LOOPOVER_MINER_DISCOVERY_PLANE: "1" },
538+
initPortfolioQueue: () => portfolioQueue,
539+
initPolicyDocCache: () => tempPolicyDocCacheStore(),
540+
initPolicyVerdictCache: () => tempPolicyVerdictCacheStore(),
541+
initRankedCandidatesStore: () => tempRankedCandidatesStore(),
542+
fetchCandidateIssuesWithSummary,
543+
queryDiscoveryIndex: queryDiscoveryIndex as never,
544+
});
545+
546+
expect(exitCode).toBe(0);
547+
const payload = JSON.parse(String(log.mock.calls[0]?.[0]));
548+
// Fail-safe: the omitted field becomes [], the owner-exclusion filter still runs (finds no assignment), issue 2 is KEPT.
549+
expect(payload.ranked.map((e: { issueNumber: number }) => e.issueNumber).sort()).toEqual([1, 2]);
550+
expect(portfolioQueue.listQueue("acme/widgets").map((e) => e.identifier).sort()).toEqual(["issue:1", "issue:2"]);
551+
});
552+
472553
it("#4847: --dry-run performs the real fan-out/rank but never opens any local store", async () => {
473554
const initPortfolioQueue = vi.fn();
474555
const initPolicyDocCache = vi.fn();

0 commit comments

Comments
 (0)