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
10 changes: 7 additions & 3 deletions plugins/guided-review/server/forge/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ type FileCall = CallOptions & { maxBytes: number };
/** Long enough for a paginated file list on a large PR, short enough that a hung call ends. */
export const DEFAULT_CLI_TIMEOUT_MS = 120_000;

/** The CLI is not where the settings say, which for a reviewer on only the other forge means it is not installed. */
export class CliNotFoundError extends ForgeError {
override name = "CliNotFoundError";
}

/** One forge CLI, whose failures come back as a `ForgeError` in a sentence. */
export type Cli = {
text(args: readonly string[], options?: CallOptions): Promise<string>;
Expand Down Expand Up @@ -61,9 +66,8 @@ export function createCli(options: CliOptions): Cli {
}
if (result.spawnError !== null) {
if (/ENOENT|EACCES/.test(result.spawnError)) {
throw new ForgeError(
`Could not run ${options.name} at "${binary}" (${result.spawnError}). Set the ${options.name} path in the Guided Review settings.`,
);
const message = `Could not run ${options.name} at "${binary}" (${result.spawnError}). Set the ${options.name} path in the Guided Review settings.`;
throw /ENOENT/.test(result.spawnError) ? new CliNotFoundError(message) : new ForgeError(message);
}
throw new ForgeError(`${options.name} did not finish: ${result.spawnError}.`);
}
Expand Down
3 changes: 2 additions & 1 deletion plugins/guided-review/server/forge/github.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { readFileSync } from "node:fs";
import path from "node:path";
import test from "node:test";
import { fakeCommandRunner, type FakeCommandRunner, type ScriptedResult } from "../fake-command-runner.ts";
import { CliNotFoundError } from "./cli.ts";
import {
ADD_THREAD_MUTATION,
AUTHORED_SEARCH_QUERY,
Expand Down Expand Up @@ -182,7 +183,7 @@ test("a gh that cannot run points at the setting", async () => {

await assert.rejects(
forge.fetchChangeRequest(PR_105),
new ForgeError(
new CliNotFoundError(
'Could not run gh at "/opt/gh/bin/gh" (spawn /opt/gh/bin/gh ENOENT). Set the gh path in the Guided Review settings.',
),
);
Expand Down
9 changes: 8 additions & 1 deletion plugins/guided-review/server/forge/gitlab.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import test from "node:test";
import type { LineRef } from "../../shared/drafts.ts";
import { anchorAt } from "../anchors.ts";
import { fakeCommandRunner, type ScriptedResult } from "../fake-command-runner.ts";
import { CliNotFoundError } from "./cli.ts";
import { createGitLabForge, parseAuthStatus, parseMergeRequestUrl, REQUEST_CHANGES_MUTATION, REVIEW_LIST_QUERY, uploadOf } from "./gitlab.ts";
import { ForgeError, type AnchorLine, type ChangeRequestRef, type DraftAnchor, type DraftTarget } from "./port.ts";

Expand Down Expand Up @@ -290,7 +291,7 @@ test("a glab that cannot run points at the setting", async () => {

await assert.rejects(
forge.matchUrl(MR_3931.url),
new ForgeError('Could not run glab at "/usr/bin/glab" (spawn /usr/bin/glab ENOENT). Set the glab path in the Guided Review settings.'),
new CliNotFoundError('Could not run glab at "/usr/bin/glab" (spawn /usr/bin/glab ENOENT). Set the glab path in the Guided Review settings.'),
);
});

Expand Down Expand Up @@ -1219,6 +1220,12 @@ test("lists the open MRs the viewer reviews on each host glab is logged in to, a
});
});

test("lists no host when glab is not installed, so a GitHub-only reviewer's list still loads", async () => {
const { forge } = forgeReplaying([{ exitCode: null, spawnError: "spawn /usr/bin/glab ENOENT" }]);

assert.deepEqual(await forge.listChangeRequests(), []);
});

test("an MR reads by the viewer's own reviewer record, changed when the head commit is newer than it", async () => {
const { forge } = forgeReplaying(LIST_REVIEWS);

Expand Down
12 changes: 10 additions & 2 deletions plugins/guided-review/server/forge/gitlab.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type { Draft, DraftLocation, LineRef } from "../../shared/drafts.ts";
import type { CommandRunner } from "../command-runner.ts";
import type { Verdict } from "../../shared/submit.ts";
import type { AuthoredChangeRequest, CiState, ListedChangeRequest, ListedHost, ReviewerState, ReviewRequest } from "../../shared/inbox.ts";
import { createCli, type Cli } from "./cli.ts";
import { CliNotFoundError, createCli, type Cli } from "./cli.ts";
import { GITHUB_HOST } from "./github.ts";
import { CLONE_TIMEOUT_MS, COMMIT_READS_AT_ONCE, mapAtMost, MAX_BRANCH_CHANGE_REQUESTS, MAX_COMMITS, MAX_LINKED_ISSUES, latestOf, reviewStatusOf, unlistedHost, userOf } from "./common.ts";
import { SubmitSteps } from "./submit-steps.ts";
Expand Down Expand Up @@ -904,7 +904,15 @@ export function createGitLabForge(options: GitLabForgeOptions): Forge {
},

async listChangeRequests(): Promise<ListedHost[]> {
const statuses = parseAuthStatus(await glab.report(["auth", "status", "--all"]));
let report: string;
try {
report = await glab.report(["auth", "status", "--all"]);
} catch (error) {
// GitLab has no host of its own to report as unlisted, so a reviewer without glab has none.
if (error instanceof CliNotFoundError) return [];
throw error;
}
const statuses = parseAuthStatus(report);
return Promise.all(
statuses.map(async ({ host, loggedIn, problem }): Promise<ListedHost> => {
if (!loggedIn) {
Expand Down
Loading