Skip to content
Closed
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
17 changes: 17 additions & 0 deletions plugins/github/server.rpc.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -348,4 +348,21 @@ describe("github plugin RPC behavior", () => {
},
});
});
it("warns about an extraRepos entry that cannot track anything", async () => {
const host = createFakePluginHost({
pluginId: "github",
settings: { extraRepos: "acme/widgets, ACME/*" },
});
await plugin(host.bb);

// The valid sibling still syncs; the glob contributes no repo.
await expect(host.harness.callRpc("refresh")).resolves.toMatchObject({
repos: 1,
});
expect(
host.harness.logEntries.filter(
(entry) => entry.level === "warn" && entry.message.includes("ACME/*"),
),
).not.toHaveLength(0);
});
});
20 changes: 20 additions & 0 deletions plugins/github/server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
fetchRepoItems,
githubRpcContract,
parsePaginatedGhApi,
parseExtraRepos,
validateGithubCliArgs,
} from "./server";

Expand Down Expand Up @@ -141,6 +142,25 @@ describe("GitHub RPC contract", () => {
);
});

it("separates trackable extraRepos entries from wildcards and typos", () => {
expect(parseExtraRepos("get-bb/bb, owner/other")).toEqual({
valid: ["get-bb/bb", "owner/other"],
invalid: [],
});
// A glob parsed to nothing at all, with no warning and no tracked repo.
expect(parseExtraRepos("get-bb/*")).toEqual({
valid: [],
invalid: ["get-bb/*"],
});
expect(parseExtraRepos("get-bb/bb, get-bb/*, nope")).toEqual({
valid: ["get-bb/bb"],
invalid: ["get-bb/*", "nope"],
});
// Blank and separator-only settings are empty, not invalid.
expect(parseExtraRepos("")).toEqual({ valid: [], invalid: [] });
expect(parseExtraRepos(" , ")).toEqual({ valid: [], invalid: [] });
});

it("infers parsed handler inputs and frontend results", () => {
expectTypeOf<
Parameters<GithubRpcHandlers["createIssue"]>[0]
Expand Down
34 changes: 30 additions & 4 deletions plugins/github/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,25 @@ function isRepoName(value: unknown): value is string {
return typeof value === "string" && /^[\w.-]+\/[\w.-]+$/.test(value);
}

/**
* Split the extraRepos setting into repository names this plugin can track and
* entries it cannot. The setting is an explicit list, so a wildcard such as
* "owner/*" is invalid rather than an owner-wide match.
*/
export function parseExtraRepos(raw: string): {
valid: string[];
invalid: string[];
} {
const valid: string[] = [];
const invalid: string[] = [];
for (const entry of raw.split(/[\s,]+/)) {
if (entry.length === 0) continue;
if (isRepoName(entry)) valid.push(entry);
else invalid.push(entry);
}
return { valid, invalid };
}

function run(
file: string,
args: string[],
Expand Down Expand Up @@ -609,10 +628,17 @@ export default async function plugin(bb: BbPluginApi) {
);
}
const { extraRepos } = await settings.get();
for (const raw of extraRepos.split(/[\s,]+/)) {
if (isRepoName(raw) && !byRepo.has(raw)) {
byRepo.set(raw, { repo: raw, projectId: null });
}
const extra = parseExtraRepos(extraRepos);
for (const raw of extra.valid) {
if (!byRepo.has(raw)) byRepo.set(raw, { repo: raw, projectId: null });
}
// An unusable entry used to be dropped in silence, so a typo or a glob
// looked identical to a setting that worked.
for (const bad of extra.invalid) {
bb.log.warn(
`extraRepos entry "${bad}" is not an owner/repo name and tracks nothing. ` +
"List each repository explicitly, for example \"owner/repo\".",
);
}
const repos = [...byRepo.values()];
repoCache = { repos, fetchedAt: Date.now() };
Expand Down
Loading