Skip to content

Commit 30f1194

Browse files
committed
fix: corrects tracked-user scoping, config wipe, upstream discovery
1 parent aeaeec7 commit 30f1194

7 files changed

Lines changed: 171 additions & 160 deletions

File tree

src/app/components/onboarding/OnboardingWizard.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ export default function OnboardingWizard() {
115115
showUpstreamDiscovery={true}
116116
upstreamRepos={upstreamRepos()}
117117
onUpstreamChange={setUpstreamRepos}
118+
trackedUsers={config.trackedUsers}
118119
/>
119120
</Match>
120121
</Switch>

src/app/components/onboarding/RepoSelector.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ interface RepoSelectorProps {
2525
showUpstreamDiscovery?: boolean;
2626
upstreamRepos?: RepoRef[];
2727
onUpstreamChange?: (repos: RepoRef[]) => void;
28+
trackedUsers?: { login: string; avatarUrl: string; name: string | null }[];
2829
}
2930

3031
interface OrgRepoState {
@@ -165,7 +166,7 @@ export default function RepoSelector(props: RepoSelectorProps) {
165166
allOrgFullNames.add(repo.fullName);
166167
}
167168
});
168-
void discoverUpstreamRepos(discoveryClient, currentUser.login, allOrgFullNames)
169+
void discoverUpstreamRepos(discoveryClient, currentUser.login, allOrgFullNames, props.trackedUsers)
169170
.then((repos) => {
170171
if (version !== effectVersion) return;
171172
setDiscoveredRepos(repos);

src/app/components/settings/SettingsPage.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,7 @@ export default function SettingsPage() {
323323
showUpstreamDiscovery={true}
324324
upstreamRepos={localUpstream()}
325325
onUpstreamChange={handleUpstreamChange}
326+
trackedUsers={config.trackedUsers}
326327
/>
327328
</div>
328329
</Show>

src/app/services/api.ts

Lines changed: 34 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -1200,52 +1200,6 @@ function mergeEnrichment(
12001200
});
12011201
}
12021202

1203-
/**
1204-
* Global (unscoped) light search for a single tracked user — no repo: qualifiers.
1205-
* Returns the same LightSearchResult shape as graphqlLightCombinedSearch.
1206-
*/
1207-
async function graphqlGlobalUserSearch(
1208-
octokit: GitHubOctokit,
1209-
userLogin: string
1210-
): Promise<LightSearchResult> {
1211-
if (!VALID_TRACKED_LOGIN.test(userLogin)) {
1212-
return {
1213-
issues: [],
1214-
pullRequests: [],
1215-
prNodeIds: [],
1216-
errors: [{ repo: "global-search", statusCode: null, message: "Invalid userLogin", retryable: false }],
1217-
};
1218-
}
1219-
1220-
const issueSeen = new Set<number>();
1221-
const issues: Issue[] = [];
1222-
const prMap = new Map<number, PullRequest>();
1223-
const nodeIdMap = new Map<number, string>();
1224-
const errors: ApiError[] = [];
1225-
const ISSUE_CAP = 1000;
1226-
const PR_CAP = 1000;
1227-
1228-
const issueQ = `is:issue is:open involves:${userLogin}`;
1229-
const prInvQ = `is:pr is:open involves:${userLogin}`;
1230-
const prRevQ = `is:pr is:open review-requested:${userLogin}`;
1231-
1232-
try {
1233-
await executeLightCombinedQuery(
1234-
octokit, issueQ, prInvQ, prRevQ, `global-search:${userLogin}`,
1235-
issueSeen, issues, prMap, nodeIdMap, errors, ISSUE_CAP, PR_CAP,
1236-
);
1237-
} catch (err) {
1238-
const { statusCode, message } = extractRejectionError(err);
1239-
errors.push({ repo: `global-search:${userLogin}`, statusCode, message, retryable: statusCode === null || statusCode >= 500 });
1240-
}
1241-
1242-
const pullRequests = [...prMap.values()];
1243-
if (pullRequests.length >= PR_CAP) pullRequests.splice(PR_CAP);
1244-
const prNodeIds = pullRequests.map((pr) => nodeIdMap.get(pr.id)).filter((id): id is string => id != null);
1245-
1246-
return { issues, pullRequests, prNodeIds, errors };
1247-
}
1248-
12491203
/**
12501204
* Merges tracked user search results into the main issue/PR maps.
12511205
* Items already present get the tracked user's login appended to surfacedBy.
@@ -1309,8 +1263,8 @@ export async function fetchIssuesAndPullRequests(
13091263

13101264
const hasTrackedUsers = (trackedUsers?.length ?? 0) > 0;
13111265

1312-
// Early exit when there is nothing to search
1313-
if (repos.length === 0 && !hasTrackedUsers) {
1266+
// Early exit — tracked user searches are scoped to repos, so no repos = no results
1267+
if (repos.length === 0) {
13141268
return { issues: [], pullRequests: [], errors: [] };
13151269
}
13161270

@@ -1353,9 +1307,9 @@ export async function fetchIssuesAndPullRequests(
13531307
? fetchPREnrichment(octokit, mainNodeIds)
13541308
: Promise.resolve({ enrichments: new Map<number, PREnrichmentData>(), errors: [] as ApiError[] });
13551309

1356-
// Tracked user global searches — run in parallel with main backfill
1357-
const trackedSearchPromise = hasTrackedUsers
1358-
? Promise.allSettled(trackedUsers!.map((u) => graphqlGlobalUserSearch(octokit, u.login)))
1310+
// Tracked user searches — scoped to the same repos as the main user
1311+
const trackedSearchPromise = hasTrackedUsers && repos.length > 0
1312+
? Promise.allSettled(trackedUsers!.map((u) => graphqlLightCombinedSearch(octokit, repos, u.login)))
13591313
: Promise.resolve([] as PromiseSettledResult<LightSearchResult>[]);
13601314

13611315
const [mainBackfill, trackedResults] = await Promise.all([mainBackfillPromise, trackedSearchPromise]);
@@ -2075,14 +2029,14 @@ export async function validateGitHubUser(
20752029
* Discovers repos the user participates in but doesn't own, via unscoped
20762030
* GraphQL search (no repo: qualifiers). Returns up to 100 repos sorted
20772031
* alphabetically, excluding any in the provided excludeRepos set.
2032+
* When trackedUsers is provided, also discovers repos those users participate in.
20782033
*/
20792034
export async function discoverUpstreamRepos(
20802035
octokit: GitHubOctokit,
20812036
userLogin: string,
2082-
excludeRepos: Set<string>
2037+
excludeRepos: Set<string>,
2038+
trackedUsers?: TrackedUser[]
20832039
): Promise<RepoRef[]> {
2084-
if (!VALID_TRACKED_LOGIN.test(userLogin)) return [];
2085-
20862040
const repoNames = new Set<string>();
20872041
const errors: ApiError[] = [];
20882042
const CAP = 100;
@@ -2095,21 +2049,32 @@ export async function discoverUpstreamRepos(
20952049
return true;
20962050
}
20972051

2098-
const issueQ = `is:issue is:open involves:${userLogin}`;
2099-
const prQ = `is:pr is:open involves:${userLogin}`;
2100-
2101-
await Promise.allSettled([
2102-
paginateGraphQLSearch<GraphQLIssueSearchResponse, GraphQLIssueNode>(
2103-
octokit, ISSUES_SEARCH_QUERY, issueQ, "upstream-issues", errors,
2104-
(node) => extractRepoName(node),
2105-
() => repoNames.size, CAP,
2106-
),
2107-
paginateGraphQLSearch<LightPRSearchResponse, GraphQLLightPRNode>(
2108-
octokit, LIGHT_PR_SEARCH_QUERY, prQ, "upstream-prs", errors,
2109-
(node) => extractRepoName(node),
2110-
() => repoNames.size, CAP,
2111-
),
2112-
]);
2052+
function discoverForUser(login: string) {
2053+
const issueQ = `is:issue is:open involves:${login}`;
2054+
const prQ = `is:pr is:open involves:${login}`;
2055+
return Promise.allSettled([
2056+
paginateGraphQLSearch<GraphQLIssueSearchResponse, GraphQLIssueNode>(
2057+
octokit, ISSUES_SEARCH_QUERY, issueQ, `upstream-issues:${login}`, errors,
2058+
(node) => extractRepoName(node),
2059+
() => repoNames.size, CAP,
2060+
),
2061+
paginateGraphQLSearch<LightPRSearchResponse, GraphQLLightPRNode>(
2062+
octokit, LIGHT_PR_SEARCH_QUERY, prQ, `upstream-prs:${login}`, errors,
2063+
(node) => extractRepoName(node),
2064+
() => repoNames.size, CAP,
2065+
),
2066+
]);
2067+
}
2068+
2069+
// Collect all valid logins to discover for
2070+
const logins: string[] = [];
2071+
if (VALID_TRACKED_LOGIN.test(userLogin)) logins.push(userLogin);
2072+
for (const u of trackedUsers ?? []) {
2073+
if (VALID_TRACKED_LOGIN.test(u.login)) logins.push(u.login);
2074+
}
2075+
if (logins.length === 0) return [];
2076+
2077+
await Promise.allSettled(logins.map((login) => discoverForUser(login)));
21132078

21142079
if (errors.length > 0) {
21152080
pushNotification(

tests/components/onboarding/RepoSelector.test.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -436,7 +436,8 @@ describe("RepoSelector — upstream discovery", () => {
436436
expect(api.discoverUpstreamRepos).toHaveBeenCalledWith(
437437
expect.anything(),
438438
"testuser",
439-
expect.any(Set)
439+
expect.any(Set),
440+
undefined
440441
);
441442
});
442443

tests/services/api-users.test.ts

Lines changed: 66 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,47 @@ describe("discoverUpstreamRepos", () => {
354354
expect(result).toHaveLength(1);
355355
expect(result[0]).toEqual({ owner: "my-org", name: "my-repo", fullName: "my-org/my-repo" });
356356
});
357+
358+
it("discovers repos from tracked users in addition to primary user", async () => {
359+
const octokit = makeOctokit(
360+
async () => ({}),
361+
async (_query: string, vars: unknown) => {
362+
const v = vars as { q: string };
363+
if (v.q.includes("involves:primary") && v.q.includes("is:issue")) {
364+
return makeSearchPage(["org/primary-repo"]);
365+
}
366+
if (v.q.includes("involves:tracked1") && v.q.includes("is:issue")) {
367+
return makeSearchPage(["org/tracked1-repo"]);
368+
}
369+
return makeSearchPage([]);
370+
}
371+
);
372+
373+
const trackedUsers = [makeTrackedUser("tracked1")];
374+
const result = await discoverUpstreamRepos(octokit as never, "primary", new Set(), trackedUsers);
375+
const names = result.map((r) => r.fullName);
376+
expect(names).toContain("org/primary-repo");
377+
expect(names).toContain("org/tracked1-repo");
378+
});
379+
380+
it("deduplicates repos found by both primary and tracked users", async () => {
381+
const octokit = makeOctokit(
382+
async () => ({}),
383+
async (_query: string, vars: unknown) => {
384+
const v = vars as { q: string };
385+
if (v.q.includes("is:issue")) {
386+
// Both users discover the same repo
387+
return makeSearchPage(["org/shared-repo"]);
388+
}
389+
return makeSearchPage([]);
390+
}
391+
);
392+
393+
const trackedUsers = [makeTrackedUser("tracked1")];
394+
const result = await discoverUpstreamRepos(octokit as never, "primary", new Set(), trackedUsers);
395+
expect(result).toHaveLength(1);
396+
expect(result[0].fullName).toBe("org/shared-repo");
397+
});
357398
});
358399

359400
// ── multi-user search (fetchIssuesAndPullRequests with trackedUsers) ───────────
@@ -471,16 +512,14 @@ describe("multi-user search", () => {
471512
async () => ({}),
472513
async (_query: string, vars: unknown) => {
473514
const v = vars as Record<string, unknown>;
474-
// Heavy backfill query has 'ids' variable
475515
if ("ids" in v) return makeBackfillResponse([]);
476-
// Light combined query: distinguish by issueQ content
516+
// Both main and tracked user searches are now repo-scoped;
517+
// distinguish by the involves: login in the query
477518
const issueQ = v["issueQ"] as string | undefined;
478-
if (issueQ?.includes("repo:")) {
479-
// Main user search (scoped to repos)
519+
if (issueQ?.includes("involves:mainuser") || issueQ?.includes("involves:trackeduser")) {
480520
return makeLightCombinedResponse([{ databaseId: sharedIssueId, repoFullName: "org/repo" }]);
481521
}
482-
// Tracked user search (unscoped)
483-
return makeLightCombinedResponse([{ databaseId: sharedIssueId, repoFullName: "org/repo" }]);
522+
return makeLightCombinedResponse([]);
484523
}
485524
);
486525

@@ -506,14 +545,17 @@ describe("multi-user search", () => {
506545
const v = vars as Record<string, unknown>;
507546
if ("ids" in v) return makeBackfillResponse([]);
508547
const issueQ = v["issueQ"] as string | undefined;
509-
if (issueQ?.includes("repo:")) {
548+
if (issueQ?.includes("involves:mainuser")) {
510549
return makeLightCombinedResponse([{ databaseId: mainIssueId, repoFullName: "org/repo" }]);
511550
}
512-
// Tracked user has both the shared one and a unique one
513-
return makeLightCombinedResponse([
514-
{ databaseId: mainIssueId, repoFullName: "org/repo" },
515-
{ databaseId: trackedOnlyIssueId, repoFullName: "org/other-repo" },
516-
]);
551+
if (issueQ?.includes("involves:trackeduser")) {
552+
// Tracked user has both the shared one and a unique one
553+
return makeLightCombinedResponse([
554+
{ databaseId: mainIssueId, repoFullName: "org/repo" },
555+
{ databaseId: trackedOnlyIssueId, repoFullName: "org/repo" },
556+
]);
557+
}
558+
return makeLightCombinedResponse([]);
517559
}
518560
);
519561

@@ -541,8 +583,7 @@ describe("multi-user search", () => {
541583
const v = vars as Record<string, unknown>;
542584
if ("ids" in v) return makeBackfillResponse([]);
543585
const issueQ = v["issueQ"] as string | undefined;
544-
if (issueQ?.includes("repo:")) {
545-
// Main user: only shared item
586+
if (issueQ?.includes("involves:mainuser")) {
546587
return makeLightCombinedResponse([{ databaseId: sharedId, repoFullName: "org/repo" }]);
547588
}
548589
if (issueQ?.includes("involves:usera")) {
@@ -609,7 +650,7 @@ describe("multi-user search", () => {
609650
const v = vars as Record<string, unknown>;
610651
if ("ids" in v) return makeBackfillResponse([]);
611652
const issueQ = v["issueQ"] as string | undefined;
612-
if (issueQ?.includes("repo:")) {
653+
if (issueQ?.includes("involves:mainuser")) {
613654
return makeLightCombinedResponse([{ databaseId: 1001, repoFullName: "org/repo" }]);
614655
}
615656
// Tracked user search fails
@@ -626,7 +667,6 @@ describe("multi-user search", () => {
626667
expect(result.issues).toHaveLength(1);
627668
expect(result.issues[0].id).toBe(1001);
628669
expect(result.issues[0].surfacedBy).toEqual(["mainuser"]);
629-
// Error is captured (graphqlGlobalUserSearch uses "global-search:" prefix internally)
630670
expect(result.errors.length).toBeGreaterThan(0);
631671
expect(callCount).toBeGreaterThan(0);
632672
});
@@ -641,16 +681,13 @@ describe("multi-user search", () => {
641681
async (_query: string, vars: unknown) => {
642682
const v = vars as Record<string, unknown>;
643683
if ("ids" in v) {
644-
// Heavy backfill: return enrichment for the PR
645684
return makeBackfillResponse([prId]);
646685
}
647686
const issueQ = v["issueQ"] as string | undefined;
648-
if (issueQ?.includes("repo:")) {
649-
// Main user light search: includes a PR
687+
if (issueQ?.includes("involves:mainuser") || issueQ?.includes("involves:trackeduser")) {
650688
return makeLightCombinedResponse([], [{ databaseId: prId, nodeId: prNodeId, repoFullName: "org/repo" }]);
651689
}
652-
// Tracked user search: same PR
653-
return makeLightCombinedResponse([], [{ databaseId: prId, nodeId: prNodeId, repoFullName: "org/repo" }]);
690+
return makeLightCombinedResponse([]);
654691
}
655692
);
656693

@@ -660,42 +697,30 @@ describe("multi-user search", () => {
660697

661698
expect(result.pullRequests).toHaveLength(1);
662699
const pr = result.pullRequests[0];
663-
// surfacedBy survives enrichment (spread preserves it since PREnrichmentData lacks surfacedBy)
664700
expect(pr.surfacedBy).toContain("mainuser");
665701
expect(pr.surfacedBy).toContain("trackeduser");
666-
// And it's fully enriched
667702
expect(pr.enriched).toBe(true);
668703
expect(pr.checkStatus).toBe("success");
669704
});
670705

671-
it("empty repos with tracked users still runs tracked user searches", async () => {
672-
const trackedIssueId = 9001;
673-
706+
it("empty repos returns empty results even with tracked users", async () => {
674707
const octokit = makeOctokit(
675-
async () => ({}),
676-
async (_query: string, vars: unknown) => {
677-
const v = vars as Record<string, unknown>;
678-
if ("ids" in v) return makeBackfillResponse([]);
679-
// Only tracked user search should run (no repo: qualifier)
680-
const issueQ = v["issueQ"] as string | undefined;
681-
if (issueQ && !issueQ.includes("repo:")) {
682-
return makeLightCombinedResponse([{ databaseId: trackedIssueId, repoFullName: "org/tracked-repo" }]);
683-
}
684-
return makeLightCombinedResponse([]);
685-
}
708+
async () => { throw new Error("should not be called"); },
709+
async () => { throw new Error("should not be called"); }
686710
);
687711

688712
const result = await fetchIssuesAndPullRequests(
689713
octokit as never,
690-
[], // empty repos — skip main user search
714+
[], // empty repos — tracked user searches are also repo-scoped
691715
"mainuser",
692716
undefined,
693717
[makeTrackedUser("trackeduser")]
694718
);
695719

696-
expect(result.issues).toHaveLength(1);
697-
expect(result.issues[0].id).toBe(trackedIssueId);
698-
expect(result.issues[0].surfacedBy).toEqual(["trackeduser"]);
720+
expect(result.issues).toEqual([]);
721+
expect(result.pullRequests).toEqual([]);
722+
expect(result.errors).toEqual([]);
723+
expect(octokit.graphql).not.toHaveBeenCalled();
699724
});
700725

701726
it("surfacedBy logins are always lowercase", async () => {

0 commit comments

Comments
 (0)