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
3 changes: 3 additions & 0 deletions apps/server/src/auth/RpcAuthorization.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,9 @@ describe("RPC authorization scopes", () => {
it("reads the reviewer menu under the same scope as the pull request it belongs to", () => {
// The candidate list is a read like the detail beside it, and asking somebody for a review is
// a write like every other pull request operation.
expect(requiredScopeForRpcMethod(WS_METHODS.pullRequestsChecks)).toBe(
AuthOrchestrationReadScope,
);
expect(requiredScopeForRpcMethod(WS_METHODS.pullRequestsReviewerCandidates)).toBe(
requiredScopeForRpcMethod(WS_METHODS.pullRequestsDetail),
);
Expand Down
1 change: 1 addition & 0 deletions apps/server/src/auth/RpcAuthorization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ export const RPC_REQUIRED_SCOPES = {
[WS_METHODS.pullRequestsStack]: AuthOrchestrationReadScope,
[WS_METHODS.pullRequestsLinkedThreads]: AuthOrchestrationReadScope,
[WS_METHODS.pullRequestsDetail]: AuthOrchestrationReadScope,
[WS_METHODS.pullRequestsChecks]: AuthOrchestrationReadScope,
[WS_METHODS.pullRequestsActivity]: AuthOrchestrationReadScope,
[WS_METHODS.pullRequestsThreadComments]: AuthOrchestrationReadScope,
[WS_METHODS.pullRequestsDiffFileContents]: AuthOrchestrationReadScope,
Expand Down
1 change: 1 addition & 0 deletions apps/server/src/environment/ServerEnvironment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ export const make = Effect.gen(function* () {
questionAttachments: true,
fileAttachments: { maxUploadBytes: PROVIDER_SEND_TURN_MAX_FILE_BYTES },
pullRequests: true,
pullRequestChecks: true,
inlineMessageContext: true,
threadSettlement: true,
threadAutoSettlement: true,
Expand Down
125 changes: 124 additions & 1 deletion apps/server/src/pullRequest/BitbucketPullRequestProvider.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,134 @@
import { describe, expect, it } from "vite-plus/test";
import { describe, expect, it } from "@effect/vitest";
import * as Effect from "effect/Effect";
import * as Layer from "effect/Layer";
import * as Result from "effect/Result";

import * as BitbucketApi from "../sourceControl/BitbucketApi.ts";
import * as BitbucketPullRequestApi from "./BitbucketPullRequestApi.ts";
import { decodePullRequestJson } from "./bitbucketPullRequestJson.ts";
import {
bitbucketProviderFailure,
bitbucketViewerPermissions,
make,
} from "./BitbucketPullRequestProvider.ts";

for (const operation of [
"getMergeability",
"listChecks",
"getRepositoryPermission",
"listComments",
"listCommits",
] as const) {
it.effect.each(["response", "body read"])(
`preserves rate limits from ${operation} on %s errors while recovering other optional-read failures`,
(variant) =>
Effect.gen(function* () {
const pullRequest = Result.getOrThrow(
decodePullRequestJson(`{
"id": 1, "title": "Check polling", "state": "OPEN",
"source": { "branch": { "name": "feature" } },
"destination": { "branch": { "name": "main" } },
"created_on": "2026-09-16T00:00:00Z",
"updated_on": "2026-09-16T00:00:00Z",
"links": { "html": { "href": "https://bitbucket.org/acme/web/pull-requests/1" } }
}`),
);
for (const status of [429, 403]) {
const provider = yield* make.pipe(
Effect.provide(
Layer.mock(BitbucketPullRequestApi.BitbucketPullRequestApi)({
getPullRequest: () => Effect.succeed(pullRequest),
getDiffStat: () => Effect.succeed({ additions: 0, deletions: 0, changedFiles: 0 }),
getMergeability: () => Effect.succeed("unknown" as const),
listChecks: () => Effect.succeed([]),
getRepositoryPermission: () => Effect.succeed(true),
listComments: () => Effect.succeed({ comments: [], threads: [], truncated: false }),
listCommits: () => Effect.succeed([]),
[operation]: () =>
Effect.fail(
variant === "response"
? new BitbucketApi.BitbucketResponseError({
operation: "request",
status,
responseBodyLength: 0,
retryAt: 120_000,
})
: new BitbucketApi.BitbucketResponseBodyReadError({
operation: "request",
status,
cause: new Error("response stream failed"),
retryAt: 120_000,
}),
),
}),
),
);
const reference = {
cwd: "/repo",
repository: "acme/web",
number: 1,
host: "bitbucket.org",
};
const result = yield* operation === "listComments" || operation === "listCommits"
? Effect.result(provider.getChangeRequestActivity(reference))
: Effect.result(provider.getChangeRequest(reference));
if (status === 429) {
expect(result).toMatchObject({
_tag: "Failure",
failure: { reason: "rate-limited", retryAt: 120_000 },
});
} else {
expect(result._tag).toBe("Success");
}
}
}),
);
}

it.effect("reads checks and PR state without diff, mergeability, or permission requests", () =>
Effect.gen(function* () {
const pullRequest = Result.getOrThrow(
decodePullRequestJson(`{
"id": 1, "title": "Checks", "state": "OPEN",
"source": { "branch": { "name": "feature" } },
"destination": { "branch": { "name": "main" } },
"created_on": "2026-09-16T00:00:00Z", "updated_on": "2026-09-16T00:00:00Z",
"links": { "html": { "href": "https://bitbucket.org/acme/web/pull-requests/1" } }
}`),
);
let limited = false;
const provider = yield* make.pipe(
Effect.provide(
Layer.mock(BitbucketPullRequestApi.BitbucketPullRequestApi)({
getPullRequest: () => Effect.succeed(pullRequest),
listChecks: () =>
limited
? Effect.fail(
new BitbucketApi.BitbucketResponseError({
operation: "request",
status: 429,
responseBodyLength: 0,
retryAt: 120_000,
}),
)
: Effect.succeed([
{ name: "build", status: "failure" as const, description: null, url: null },
]),
}),
),
);
const read = provider.getChangeRequestChecks;
if (read === undefined) return yield* Effect.die("checks read missing");
const input = { cwd: "/repo", repository: "acme/web", host: "bitbucket.org", number: 1 };
expect((yield* read(input)).checks[0]?.status).toBe("failure");
limited = true;
expect(yield* Effect.flip(read(input))).toMatchObject({
reason: "rate-limited",
retryAt: 120_000,
});
}),
);

describe("bitbucketProviderFailure", () => {
it("treats only an HTTP 401 as unusable credentials", () => {
const responseError = (status: number) =>
Expand Down
48 changes: 40 additions & 8 deletions apps/server/src/pullRequest/BitbucketPullRequestProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,10 @@ export function bitbucketProviderFailure(
if (error._tag === "BitbucketResponseError" && error.status === 401) {
return { reason: "unauthenticated" };
}
if (error._tag === "BitbucketResponseError" && error.status === 429) {
if (
(error._tag === "BitbucketResponseError" || error._tag === "BitbucketResponseBodyReadError") &&
error.status === 429
) {
return {
reason: "rate-limited",
...(error.retryAt === undefined ? {} : { retryAt: error.retryAt }),
Expand Down Expand Up @@ -117,6 +120,31 @@ export const make = Effect.gen(function* () {
cause: error,
});

const recoverRead = <A>(
read: Effect.Effect<A, BitbucketPullRequestApi.BitbucketPullRequestApiError>,
fallback: A,
) => {
const recover = () => Effect.succeed(fallback);
return Effect.catchTags(read, {
BitbucketResponseError: (error) => (error.status === 429 ? Effect.fail(error) : recover()),
BitbucketUntrustedUrlError: recover,
BitbucketRepositoryLocatorError: recover,
BitbucketRequestError: recover,
BitbucketResponseBodyReadError: (error) =>
error.status === 429 ? Effect.fail(error) : recover(),
BitbucketResponseDecodeError: recover,
BitbucketRepositoryVcsResolveError: recover,
BitbucketRepositoryRemotesListError: recover,
BitbucketRepositoryRemoteNotFoundError: recover,
BitbucketPullRequestBodyReadError: recover,
BitbucketCheckoutError: recover,
BitbucketPullRequestReadError: recover,
BitbucketViewerUnavailableError: recover,
BitbucketRepositoryUnsupportedError: recover,
BitbucketDiffCommitError: recover,
});
};

const provider: PullRequestProviderApi = {
kind: "bitbucket",
capabilities: CAPABILITIES,
Expand Down Expand Up @@ -145,18 +173,24 @@ export const make = Effect.gen(function* () {
})),
),

getChangeRequestChecks: (input) =>
Effect.all([api.getPullRequest(input), api.listChecks(input)], { concurrency: 2 }).pipe(
Effect.map(([pullRequest, checks]) => ({ state: pullRequest.state, checks })),
Effect.mapError(fail("getChangeRequestChecks")),
),

getChangeRequest: (input) => {
const target = { repository: input.repository, number: input.number };
return Effect.all(
[
api.getPullRequest(target),
api.getDiffStat(target),
api.getMergeability(target).pipe(Effect.orElseSucceed(() => "unknown" as const)),
api.listChecks(target).pipe(Effect.orElseSucceed(() => [])),
recoverRead(api.getMergeability(target), "unknown" as const),
recoverRead(api.listChecks(target), []),
// A permission that could not be read is an unknown one, which is granted: a hidden
// Merge leaves someone entitled to it with no way through, and one Bitbucket refuses
// at least says why.
api.getRepositoryPermission(target).pipe(Effect.orElseSucceed(() => true)),
recoverRead(api.getRepositoryPermission(target), true),
],
{ concurrency: 5 },
).pipe(
Expand Down Expand Up @@ -195,10 +229,8 @@ export const make = Effect.gen(function* () {
// Reviews ride on the pull request itself, so this inexpensive core read is repeated
// here rather than making the core response wait for the conversation endpoints.
api.getPullRequest(target),
api
.listComments(target)
.pipe(Effect.orElseSucceed(() => ({ comments: [], threads: [], truncated: true }))),
api.listCommits(target).pipe(Effect.orElseSucceed(() => [])),
recoverRead(api.listComments(target), { comments: [], threads: [], truncated: true }),
recoverRead(api.listCommits(target), []),
],
{ concurrency: 3 },
).pipe(
Expand Down
13 changes: 13 additions & 0 deletions apps/server/src/pullRequest/ForgejoPullRequestProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,19 @@ export const make = Effect.gen(function* () {
},
),
getChangeRequestSummary: (input) => getPull(input).pipe(Effect.map(forgejoChangeRequest)),
getChangeRequestChecks: Effect.fn("ForgejoPullRequestProvider.getChangeRequestChecks")(
function* (input) {
const pr = yield* getPull(input);
const statuses = yield* page(
{
...input,
path: `${repoPath(input)}/statuses/${encodeURIComponent(pr.head.sha)}?sort=recentupdate`,
},
ForgejoStatus,
);
return { state: forgejoChangeRequest(pr).state, checks: forgejoChecks(statuses.items) };
},
),
Comment thread
coderabbitai[bot] marked this conversation as resolved.
getChangeRequest: Effect.fn("ForgejoPullRequestProvider.getChangeRequest")(function* (input) {
const [pr, repo, viewer] = yield* Effect.all(
[getPull(input), getRepo(input), getViewer(input)],
Expand Down
55 changes: 55 additions & 0 deletions apps/server/src/pullRequest/GitHubPullRequestProvider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ import { describe, expect, it } from "@effect/vitest";
import * as Effect from "effect/Effect";
import * as Layer from "effect/Layer";
import * as Redacted from "effect/Redacted";
import * as Result from "effect/Result";
import type { PullRequestReaction } from "@t3tools/contracts";

import { decodePullRequestDetailJson } from "./gitHubPullRequestJson.ts";
import * as GitHubCli from "../sourceControl/GitHubCli.ts";
import * as GitHubPullRequestCli from "./GitHubPullRequestCli.ts";
import { gitHubViewerPermissions, loginAvatarUrl, make } from "./GitHubPullRequestProvider.ts";
Expand Down Expand Up @@ -47,6 +49,54 @@ it.effect("maps credential verification failures without relabeling operation fa
}),
);

it.effect("refreshes checks without permissions or comparison reads", () =>
Effect.gen(function* () {
let reads = 0;
const snapshot = Result.getOrThrow(
decodePullRequestDetailJson(`{
"number": 7, "title": "Checks", "url": "https://github.com/acme/web/pull/7",
"headRefName": "feature", "baseRefName": "main", "state": "OPEN",
"createdAt": "2026-07-01T00:00:00Z", "updatedAt": "2026-07-01T00:00:00Z"
}`),
);
const provider = yield* make.pipe(
Effect.provide(
Layer.mock(GitHubPullRequestCli.GitHubPullRequestCli)({
getPullRequestDetail: () =>
Effect.sync(() => {
reads++;
return {
...snapshot,
state: reads === 3 ? ("merged" as const) : ("open" as const),
checks: [
{
name: "build",
status: reads === 1 ? ("pending" as const) : ("success" as const),
description: null,
url: null,
},
],
};
}),
}),
),
);
const read = provider.getChangeRequestChecks;
if (read === undefined) return yield* Effect.die("checks read missing");
for (let tick = 1; tick <= 3; tick++) {
const result = yield* read({
cwd: "/w",
repository: "acme/web",
host: "github.com",
number: 7,
});
expect(result.checks[0]?.status).toBe(tick === 1 ? "pending" : "success");
expect(result.state).toBe(tick === 3 ? "merged" : "open");
}
expect(reads).toBe(3);
}),
);

it.effect("uses one narrow read for a linked pull request summary", () =>
Effect.gen(function* () {
let summaryReads = 0;
Expand Down Expand Up @@ -338,6 +388,11 @@ describe("gitHubViewerPermissions", () => {
number: 7,
});

const readChecks = provider.getChangeRequestChecks;
if (readChecks === undefined) return yield* Effect.die("checks read missing");
expect(
yield* readChecks({ cwd: "/w", repository: "acme/web", host: "github.com", number: 7 }),
).toEqual({ state: detail.state, checks: detail.checks });
expect(detail.workflowApprovalsRequired).toBe(1);
expect(detail.checks).toEqual([
{
Expand Down
Loading
Loading