From 15a86bb1fb1f5cee7b0fab6b6541fc4831ef901c Mon Sep 17 00:00:00 2001 From: Kalven Schraut Date: Fri, 4 Sep 2026 23:19:55 -0500 Subject: [PATCH 1/4] feat(pull-requests): detect Gitea fork capabilities --- .../src/pullRequest/GiteaConversation.test.ts | 2 +- .../src/pullRequest/GiteaConversation.ts | 12 +- .../pullRequest/GiteaForkCapabilities.test.ts | 31 +++ .../src/pullRequest/GiteaForkCapabilities.ts | 15 ++ .../pullRequest/GiteaPullRequestApi.test.ts | 90 ++++++-- .../src/pullRequest/GiteaPullRequestApi.ts | 126 ++++++++--- .../pullRequest/GiteaPullRequestProvider.ts | 11 +- .../src/pullRequest/PullRequestProvider.ts | 5 + .../src/pullRequest/PullRequestService.ts | 210 ++++++++++-------- 9 files changed, 359 insertions(+), 143 deletions(-) create mode 100644 apps/server/src/pullRequest/GiteaForkCapabilities.test.ts create mode 100644 apps/server/src/pullRequest/GiteaForkCapabilities.ts diff --git a/apps/server/src/pullRequest/GiteaConversation.test.ts b/apps/server/src/pullRequest/GiteaConversation.test.ts index b9aad258c90e..c36ceecf1d94 100644 --- a/apps/server/src/pullRequest/GiteaConversation.test.ts +++ b/apps/server/src/pullRequest/GiteaConversation.test.ts @@ -12,7 +12,7 @@ describe("GiteaConversation", () => { it("addresses ordinary and inline review remarks through the same issue-comment record", () => { expect(editableCommentId("issue:12")).toBe("12"); expect(editableCommentId("review-comment:34")).toBe("34"); - expect(reactionTarget("review:56")).toBeNull(); + expect(reactionTarget("review:56")).toEqual({ kind: "review", id: "56" }); }); it("distinguishes a pull request description from a comment and rejects malformed ids", () => { diff --git a/apps/server/src/pullRequest/GiteaConversation.ts b/apps/server/src/pullRequest/GiteaConversation.ts index 6fa274a540e1..2831c6cdd992 100644 --- a/apps/server/src/pullRequest/GiteaConversation.ts +++ b/apps/server/src/pullRequest/GiteaConversation.ts @@ -13,7 +13,8 @@ export const RawGiteaReaction = Schema.Struct({ export type GiteaConversationReactionTarget = | { readonly kind: "pull-request" } - | { readonly kind: "comment"; readonly id: string }; + | { readonly kind: "comment"; readonly id: string } + | { readonly kind: "review"; readonly id: string }; const reactionContent = new Map([ ["+1", "thumbs-up"], @@ -38,14 +39,17 @@ function commentId(subjectId: string): string | null { /** * Gitea stores inline review remarks as issue comments. Review summaries use a separate Review - * record, for which v1.27.3 intentionally exposes neither an edit nor a reaction endpoint. + * record, which the companion server extension exposes through a separate route. */ export function reactionTarget( subjectId: string | undefined, ): GiteaConversationReactionTarget | null { if (subjectId === undefined) return { kind: "pull-request" }; - const id = commentId(subjectId); - return id === null ? null : { kind: "comment", id }; + const [kind, reviewId] = subjectId.split(":", 2); + if (kind === "review" && reviewId && /^\d+$/.test(reviewId)) + return { kind: "review", id: reviewId }; + const comment = commentId(subjectId); + return comment === null ? null : { kind: "comment", id: comment }; } /** Returns the native issue-comment ID for both ordinary and inline-review remarks. */ diff --git a/apps/server/src/pullRequest/GiteaForkCapabilities.test.ts b/apps/server/src/pullRequest/GiteaForkCapabilities.test.ts new file mode 100644 index 000000000000..105a5869b177 --- /dev/null +++ b/apps/server/src/pullRequest/GiteaForkCapabilities.test.ts @@ -0,0 +1,31 @@ +import { describe, expect, it } from "@effect/vitest"; +import type { PullRequestCapabilities } from "@t3tools/contracts"; + +import { giteaForkCapabilities } from "./GiteaForkCapabilities.ts"; + +const base: PullRequestCapabilities = { + diff: true, + comment: true, + actions: [], + mergeMethods: [], + search: false, + reactions: false, + reactionSubjects: { + changeRequest: true, + issueComment: true, + reviewComment: true, + review: false, + }, + review: { inlineComment: true, reply: true, resolve: true, verdicts: [] }, + reviewers: { request: true, listCandidates: true }, +}; + +describe("GiteaForkCapabilities", () => { + it("enables review-summary reactions only for an advertising server", () => { + expect(giteaForkCapabilities(base, []).reactionSubjects?.review).toBe(false); + expect(giteaForkCapabilities(base, ["pull-review-reactions"]).reactionSubjects?.review).toBe( + true, + ); + expect(base.reactionSubjects?.review).toBe(false); + }); +}); diff --git a/apps/server/src/pullRequest/GiteaForkCapabilities.ts b/apps/server/src/pullRequest/GiteaForkCapabilities.ts new file mode 100644 index 000000000000..5dada5cd8536 --- /dev/null +++ b/apps/server/src/pullRequest/GiteaForkCapabilities.ts @@ -0,0 +1,15 @@ +import type { PullRequestCapabilities } from "@t3tools/contracts"; + +/** Capabilities added by the companion Gitea API extension, discovered lazily per server. */ +export function giteaForkCapabilities( + base: PullRequestCapabilities, + features: ReadonlyArray, +): PullRequestCapabilities { + return features.includes("pull-review-reactions") + ? { ...base, reactionSubjects: { ...base.reactionSubjects!, review: true } } + : base; +} + +export function giteaHasFeature(features: ReadonlyArray, feature: string): boolean { + return features.includes(feature); +} diff --git a/apps/server/src/pullRequest/GiteaPullRequestApi.test.ts b/apps/server/src/pullRequest/GiteaPullRequestApi.test.ts index a0cc182a06dd..c4794a1f6bdd 100644 --- a/apps/server/src/pullRequest/GiteaPullRequestApi.test.ts +++ b/apps/server/src/pullRequest/GiteaPullRequestApi.test.ts @@ -1141,6 +1141,7 @@ layer("GiteaPullRequestApi", (it) => { it.effect("loads reactions for the pull request and every issue-backed remark", () => Effect.gen(function* () { mockedRequest + .mockReturnValueOnce(Effect.succeed(response({ features: [] }))) .mockReturnValueOnce( Effect.succeed( response([ @@ -1171,14 +1172,44 @@ layer("GiteaPullRequestApi", (it) => { expect(reactions.bySubjectId.get("review-comment:34")).toEqual([]); expect(reactions.bySubjectId.has("review:21")).toBe(false); expect(mockedRequest.mock.calls.map((call) => call[0].path)).toEqual([ - "/repos/acme/web/issues/7/reactions", - "/repos/acme/web/issues/comments/12/reactions", - "/repos/acme/web/issues/comments/34/reactions", + "/settings/api", + "/repos/acme/web/issues/7/reactions?page=1&limit=100", + "/repos/acme/web/issues/comments/12/reactions?page=1&limit=100", + "/repos/acme/web/issues/comments/34/reactions?page=1&limit=100", ]); }), ); - it.effect("reports Gitea's missing review-summary reaction route without issuing a request", () => + it.effect("follows a reaction list when Gitea caps a requested page below its limit", () => + Effect.gen(function* () { + mockedRequest.mockImplementation((input) => { + if (input.path === "/settings/api") return Effect.succeed(response({ features: [] })); + if (input.path === "/repos/acme/web/issues/7/reactions?page=1&limit=100") + return Effect.succeed( + response([{ reaction: "heart", user: { login: "one" } }], { "x-total-count": "2" }), + ); + if (input.path === "/repos/acme/web/issues/7/reactions?page=2&limit=100") + return Effect.succeed( + response([{ reaction: "eyes", user: { login: "two" } }], { "x-total-count": "2" }), + ); + return Effect.die(`unexpected request: ${input.path}`); + }); + const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const reactions = yield* api.listConversationReactions({ + host: "forge.example.test", + repository: "acme/web", + number: 7, + viewer: "reader", + subjectIds: [], + }); + expect(reactions.pullRequest).toEqual([ + { content: "heart", count: 1, actors: ["one"], viewerHasReacted: false }, + { content: "eyes", count: 1, actors: ["two"], viewerHasReacted: false }, + ]); + }), + ); + + it.effect("reports Gitea's missing review-summary reaction route", () => Effect.gen(function* () { const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; const error = yield* api @@ -1193,7 +1224,29 @@ layer("GiteaPullRequestApi", (it) => { .pipe(Effect.flip); expect(error.detail).toContain("review summaries"); - assert.strictEqual(mockedRequest.mock.calls.length, 0); + assert.strictEqual(mockedRequest.mock.calls.length, 1); + assert.strictEqual(callAt(0).path, "/settings/api"); + }), + ); + + it.effect("uses the review-summary reaction route when the server advertises it", () => + Effect.gen(function* () { + mockedRequest + .mockReturnValueOnce(Effect.succeed(response({ features: ["pull-review-reactions"] }))) + .mockReturnValueOnce(Effect.succeed(response({}))); + const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + yield* api.setReaction({ + host: "forge.example.test", + repository: "acme/web", + number: 7, + subjectId: "review:21", + content: "eyes", + reacted: true, + }); + expect(callAt(1)).toMatchObject({ + method: "POST", + path: "/repos/acme/web/pulls/7/reviews/21/reactions", + }); }), ); @@ -1341,16 +1394,18 @@ layer("GiteaPullRequestApi", (it) => { it.effect("reads armed auto-merge state from Gitea's durable timeline events", () => Effect.gen(function* () { - mockedRequest.mockReturnValueOnce( - Effect.succeed( - response([ - { id: 10, type: "pull_scheduled_merge" }, - { id: 11, type: "comment" }, - { id: 12, type: "pull_cancel_scheduled_merge" }, - { id: 13, type: "pull_scheduled_merge" }, - ]), - ), - ); + mockedRequest + .mockReturnValueOnce(Effect.succeed(response({ features: [] }))) + .mockReturnValueOnce( + Effect.succeed( + response([ + { id: 10, type: "pull_scheduled_merge" }, + { id: 11, type: "comment" }, + { id: 12, type: "pull_cancel_scheduled_merge" }, + { id: 13, type: "pull_scheduled_merge" }, + ]), + ), + ); const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; assert.isTrue( @@ -1360,13 +1415,14 @@ layer("GiteaPullRequestApi", (it) => { number: 7, }), ); - expect(callAt(0).path).toBe("/repos/acme/web/issues/7/timeline?page=1&limit=50"); + expect(callAt(1).path).toBe("/repos/acme/web/issues/7/timeline?page=1&limit=50"); }), ); it.effect("paginates the timeline before deciding that auto-merge is armed", () => Effect.gen(function* () { mockedRequest + .mockReturnValueOnce(Effect.succeed(response({ features: [] }))) .mockReturnValueOnce( Effect.succeed( response( @@ -1387,7 +1443,7 @@ layer("GiteaPullRequestApi", (it) => { number: 7, }), ); - expect(callAt(1).path).toContain("page=2"); + expect(callAt(2).path).toContain("page=2"); }), ); diff --git a/apps/server/src/pullRequest/GiteaPullRequestApi.ts b/apps/server/src/pullRequest/GiteaPullRequestApi.ts index 6f42568a2a15..0febbfdce967 100644 --- a/apps/server/src/pullRequest/GiteaPullRequestApi.ts +++ b/apps/server/src/pullRequest/GiteaPullRequestApi.ts @@ -90,6 +90,8 @@ const RawPullRequest = Schema.Struct({ merged: Schema.optional(Schema.Boolean), mergeable: Schema.optional(Schema.NullOr(Schema.Boolean)), draft: Schema.optional(Schema.Boolean), + auto_merge_enabled: Schema.optional(Schema.NullOr(Schema.Boolean)), + auto_merge_method: Schema.optional(Schema.NullOr(Schema.String)), html_url: Schema.String, created_at: Schema.String, updated_at: Schema.String, @@ -231,6 +233,8 @@ export interface GiteaPullRequest { readonly reviewers: ReadonlyArray; readonly labels: ReadonlyArray; readonly commentCount: number; + readonly autoMergeEnabled?: boolean; + readonly autoMergeMethod?: PullRequestMergeMethod; } export interface GiteaRepositoryAccess { @@ -318,6 +322,12 @@ function pullRequest(value: RawPullRequest): GiteaPullRequest | null { return name ? [{ name, color: label.color?.trim() || null }] : []; }), commentCount: Math.max(0, value.comments ?? 0) + Math.max(0, value.review_comments ?? 0), + ...(value.auto_merge_enabled === null || value.auto_merge_enabled === undefined + ? {} + : { autoMergeEnabled: value.auto_merge_enabled }), + ...(["merge", "squash", "rebase"].includes(value.auto_merge_method ?? "") + ? { autoMergeMethod: value.auto_merge_method as PullRequestMergeMethod } + : {}), }; } @@ -436,6 +446,7 @@ export class GiteaPullRequestApi extends Context.Service< GiteaPullRequestApi, { readonly getViewer: () => Effect.Effect; + readonly getFeatures: () => Effect.Effect, GiteaPullRequestApiError>; readonly listPullRequests: (input: { readonly host: string; readonly repository: string; @@ -466,7 +477,7 @@ export class GiteaPullRequestApi extends Context.Service< host: string; repository: string; number: number; - }) => Effect.Effect; + }) => Effect.Effect; readonly listComments: (input: { host: string; repository: string; @@ -842,6 +853,21 @@ export const make = Effect.gen(function* () { readUnknownPage(input).pipe(Effect.map((page) => page.rows)), ); + const getFeatures = yield* Effect.cachedWithTTL( + gitea.request({ operation: "getFeatures", method: "GET", path: "/settings/api" }).pipe( + Effect.mapError((error) => failure("getFeatures", error)), + Effect.flatMap((response) => + decode( + "getFeatures", + Schema.Struct({ features: Schema.optional(Schema.Array(Schema.String)) }), + response, + ), + ), + Effect.map((settings) => settings.features ?? []), + ), + "1 minute", + ); + const readUnknownSlice = Effect.fn("GiteaPullRequestApi.readUnknownSlice")(function* (input: { operation: string; host: string; @@ -1341,6 +1367,11 @@ export const make = Effect.gen(function* () { const getAutoMergeEnabled = Effect.fn("GiteaPullRequestApi.getAutoMergeEnabled")( function* (input: { host: string; repository: string; number: number }) { + const features = yield* getFeatures.pipe(Effect.option); + if (Option.isNone(features)) return undefined; + if (features.value.includes("pull-auto-merge-state")) { + return (yield* getPullRequest(input)).autoMergeEnabled; + } const operation = "getAutoMergeEnabled"; const events: Array = []; let path = query(`${basePath(input.repository)}/issues/${input.number}/timeline`, { @@ -1385,6 +1416,17 @@ export const make = Effect.gen(function* () { number: number; action: Extract; }) { + const features = yield* getFeatures.pipe(Effect.orElseSucceed(() => [])); + if (features.includes("pull-draft")) { + return yield* write({ + operation: "runAction", + host: input.host, + repository: input.repository, + method: "PATCH", + path: `${basePath(input.repository)}/pulls/${input.number}`, + body: { draft: input.action === "draft" }, + }); + } const before = yield* getPullRequest(input); const title = GiteaLifecycle.titleForDraftAction({ action: input.action, @@ -1443,38 +1485,59 @@ export const make = Effect.gen(function* () { viewer: string; subjectIds: ReadonlyArray; }) { + const supportsReviewReactions = (yield* getFeatures.pipe( + Effect.orElseSucceed(() => []), + )).includes("pull-review-reactions"); const targets: Array<{ readonly subjectId: string | undefined; readonly target: GiteaConversationReactionTarget; }> = [{ subjectId: undefined, target: { kind: "pull-request" } }]; for (const subjectId of new Set(input.subjectIds)) { const target = reactionTarget(subjectId); - if (target !== null) targets.push({ subjectId, target }); + if (target !== null && (target.kind !== "review" || supportsReviewReactions)) + targets.push({ subjectId, target }); } const reactions = yield* Effect.all( targets.map((entry) => - readUnknownArray({ + readUnknownSlice({ operation: "listConversationReactions", host: input.host, repository: input.repository, path: entry.target.kind === "pull-request" - ? `${basePath(input.repository)}/issues/${input.number}/reactions` - : `${basePath(input.repository)}/issues/comments/${entry.target.id}/reactions`, + ? query(`${basePath(input.repository)}/issues/${input.number}/reactions`, { + page: 1, + limit: PAGE_SIZE, + }) + : entry.target.kind === "review" + ? query( + `${basePath(input.repository)}/pulls/${input.number}/reviews/${entry.target.id}/reactions`, + { page: 1, limit: PAGE_SIZE }, + ) + : query( + `${basePath(input.repository)}/issues/comments/${entry.target.id}/reactions`, + { + page: 1, + limit: PAGE_SIZE, + }, + ), + limit: PAGE_SIZE * MAX_PAGINATION_PAGES, }).pipe( - Effect.map((rows) => ({ + Effect.map((result) => ({ subjectId: entry.subjectId, - reactions: reactionsForViewer( - rows.flatMap((row) => { - const decoded = decodeReaction(row); - return Option.isSome(decoded) ? [decoded.value] : []; - }), - input.viewer, - ), + reactions: result.truncated + ? [] + : reactionsForViewer( + result.rows.flatMap((row) => { + const decoded = decodeReaction(row); + return Option.isSome(decoded) ? [decoded.value] : []; + }), + input.viewer, + ), })), ), ), - { concurrency: 10 }, + { concurrency: 4 }, ); return { pullRequest: reactions.find((entry) => entry.subjectId === undefined)?.reactions ?? [], @@ -1495,6 +1558,7 @@ export const make = Effect.gen(function* () { }); return GiteaPullRequestApi.of({ + getFeatures: () => getFeatures, getViewer: Effect.fn("GiteaPullRequestApi.getViewer")(function* () { const response = yield* gitea .request({ @@ -1890,17 +1954,29 @@ export const make = Effect.gen(function* () { }), ); } - const path = - target.kind === "pull-request" - ? `${basePath(input.repository)}/issues/${input.number}/reactions` - : `${basePath(input.repository)}/issues/comments/${target.id}/reactions`; - return write({ - operation: "setReaction", - host: input.host, - repository: input.repository, - method: input.reacted ? "POST" : "DELETE", - path, - body: { content: nativeReactionContent(input.content) }, + return Effect.gen(function* () { + const features = yield* getFeatures.pipe(Effect.orElseSucceed(() => [])); + if (target.kind === "review" && !features.includes("pull-review-reactions")) { + return yield* new GiteaPullRequestApiError({ + operation: "setReaction", + reason: "failed", + detail: "Gitea cannot react to pull request review summaries through this API.", + }); + } + const path = + target.kind === "pull-request" + ? `${basePath(input.repository)}/issues/${input.number}/reactions` + : target.kind === "review" + ? `${basePath(input.repository)}/pulls/${input.number}/reviews/${target.id}/reactions` + : `${basePath(input.repository)}/issues/comments/${target.id}/reactions`; + return yield* write({ + operation: "setReaction", + host: input.host, + repository: input.repository, + method: input.reacted ? "POST" : "DELETE", + path, + body: { content: nativeReactionContent(input.content) }, + }); }); }, }); diff --git a/apps/server/src/pullRequest/GiteaPullRequestProvider.ts b/apps/server/src/pullRequest/GiteaPullRequestProvider.ts index 6e7c8d65b285..6c8e1cbfdf6d 100644 --- a/apps/server/src/pullRequest/GiteaPullRequestProvider.ts +++ b/apps/server/src/pullRequest/GiteaPullRequestProvider.ts @@ -2,6 +2,7 @@ import * as Effect from "effect/Effect"; import type { PullRequestCapabilities, PullRequestViewerPermissions } from "@t3tools/contracts"; import * as GiteaPullRequestApi from "./GiteaPullRequestApi.ts"; +import { giteaForkCapabilities } from "./GiteaForkCapabilities.ts"; import { PullRequestProviderError, type PullRequestProviderApi, @@ -136,6 +137,11 @@ export const make = Effect.gen(function* () { const provider: PullRequestProviderApi = { kind: "gitea", capabilities: CAPABILITIES, + getCapabilities: ({ host }) => + api.getFeatures().pipe( + Effect.map((features) => giteaForkCapabilities(CAPABILITIES, features)), + Effect.mapError(fail(`getCapabilities:${host}`)), + ), getViewer: () => api.getViewer().pipe(Effect.mapError(fail("getViewer"))), @@ -184,7 +190,10 @@ export const make = Effect.gen(function* () { checks, mergeCapabilities: access.mergeCapabilities, baseComparison: giteaBaseComparison(pullRequest), - autoMergeEnabled, + ...(autoMergeEnabled === undefined ? {} : { autoMergeEnabled }), + ...(pullRequest.autoMergeMethod === undefined + ? {} + : { autoMergeMethod: pullRequest.autoMergeMethod }), viewerPermissions: permissions({ access, viewer, diff --git a/apps/server/src/pullRequest/PullRequestProvider.ts b/apps/server/src/pullRequest/PullRequestProvider.ts index 22028ced5ddf..adf6f8d15b2b 100644 --- a/apps/server/src/pullRequest/PullRequestProvider.ts +++ b/apps/server/src/pullRequest/PullRequestProvider.ts @@ -240,6 +240,11 @@ export interface ProviderRepositoryRef { export interface PullRequestProviderApi { readonly kind: SourceControlProviderKind; readonly capabilities: PullRequestCapabilities; + /** Host-discovered additions, read only when a caller needs to gate a capability. */ + readonly getCapabilities?: (input: { + readonly cwd: string; + readonly host: string; + }) => Effect.Effect; /** The signed-in account, which is what involvement filtering compares against. */ readonly getViewer: (input: { diff --git a/apps/server/src/pullRequest/PullRequestService.ts b/apps/server/src/pullRequest/PullRequestService.ts index 54d1f510c84b..101ebaa22b86 100644 --- a/apps/server/src/pullRequest/PullRequestService.ts +++ b/apps/server/src/pullRequest/PullRequestService.ts @@ -689,6 +689,15 @@ export const make = Effect.gen(function* () { }), ); + const capabilitiesOf = (project: SupportedProject) => { + const read = project.api.getCapabilities; + return read === undefined + ? Effect.succeed(project.api.capabilities) + : read({ cwd: project.project.workspaceRoot, host: project.host }).pipe( + Effect.orElseSucceed(() => project.api.capabilities), + ); + }; + /** * What the signed-in account may do with this change request, asked of the host itself. Every * write goes through it: the page hides what a viewer may not do, and a request that arrived @@ -1278,12 +1287,13 @@ export const make = Effect.gen(function* () { }) .pipe(Effect.mapError(toPullRequestError("detail"))), viewerOf(project), + capabilitiesOf(project), ], - { concurrency: 2 }, + { concurrency: 3 }, ).pipe( - Effect.map(([changeRequest, viewer]): PullRequestDetail => ({ + Effect.map(([changeRequest, viewer, capabilities]): PullRequestDetail => ({ provider: project.api.kind, - capabilities: project.api.capabilities, + capabilities, projectId: project.project.id, projectTitle: project.project.title, workspaceRoot: project.project.workspaceRoot, @@ -1432,85 +1442,91 @@ export const make = Effect.gen(function* () { const runAction = (input: PullRequestActionInput): Effect.Effect => requireProject(input).pipe( - Effect.flatMap((project): Effect.Effect => { - // The surface hides what a host cannot do, and this refuses it as well: a request that - // reached here anyway must not be handed to a provider that never claimed the action. - if (!project.api.capabilities.actions.includes(input.action)) { - return Effect.fail( - new PullRequestOperationError({ - operation: "runAction", - detail: `This host cannot ${input.action} a change request.`, - }), - ); - } - // A strategy the host does not offer must be refused rather than passed on: every - // provider maps an unrecognised method to its own default, so asking Azure DevOps to - // rebase would quietly merge instead of failing. - if ( - input.mergeMethod !== undefined && - !project.api.capabilities.mergeMethods.includes(input.mergeMethod) - ) { - return Effect.fail( - new PullRequestOperationError({ - operation: "runAction", - detail: `This host cannot merge with the ${input.mergeMethod} strategy.`, - }), - ); - } - // The same for the way a stale branch is brought up to date: a host that only merges - // must not be asked to rebase and left to pick something else. - if ( - input.updateMethod !== undefined && - !(project.api.capabilities.updateMethods ?? []).includes(input.updateMethod) - ) { - return Effect.fail( - new PullRequestOperationError({ - operation: "runAction", - detail: `This host cannot update a branch by ${input.updateMethod}.`, - }), - ); - } - // What the host can do and what this account may ask of it are two questions, and both - // have to say yes. The second is asked last, because it costs a request and the checks - // above do not. - return viewerPermissionsOf(project, input, "runAction").pipe( - Effect.flatMap((viewer): Effect.Effect => { - if (!viewer.actions.includes(input.action)) { + Effect.flatMap((project): Effect.Effect => + capabilitiesOf(project).pipe( + Effect.flatMap((capabilities): Effect.Effect => { + // The surface hides what a host cannot do, and this refuses it as well: a request that + // reached here anyway must not be handed to a provider that never claimed the action. + if (!capabilities.actions.includes(input.action)) { return Effect.fail( new PullRequestOperationError({ operation: "runAction", - detail: ACTION_ACCESS_REFUSALS[input.action], + detail: `This host cannot ${input.action} a change request.`, }), ); } + // A strategy the host does not offer must be refused rather than passed on: every + // provider maps an unrecognised method to its own default, so asking Azure DevOps to + // rebase would quietly merge instead of failing. if ( - input.updateMethod !== undefined && - !(viewer.updateMethods ?? []).includes(input.updateMethod) + input.mergeMethod !== undefined && + !capabilities.mergeMethods.includes(input.mergeMethod) ) { return Effect.fail( new PullRequestOperationError({ operation: "runAction", - detail: ACTION_ACCESS_REFUSALS["update-branch"], + detail: `This host cannot merge with the ${input.mergeMethod} strategy.`, }), ); } - return project.api - .runAction({ - cwd: project.project.workspaceRoot, - repository: project.repository, - host: project.host, - number: input.number, - action: input.action, - ...(input.mergeMethod === undefined ? {} : { mergeMethod: input.mergeMethod }), - ...(input.updateMethod === undefined ? {} : { updateMethod: input.updateMethod }), - }) - .pipe( - Effect.mapError(toPullRequestError("runAction")), - Effect.as(project.repository), + // The same for the way a stale branch is brought up to date: a host that only merges + // must not be asked to rebase and left to pick something else. + if ( + input.updateMethod !== undefined && + !(capabilities.updateMethods ?? []).includes(input.updateMethod) + ) { + return Effect.fail( + new PullRequestOperationError({ + operation: "runAction", + detail: `This host cannot update a branch by ${input.updateMethod}.`, + }), ); + } + // What the host can do and what this account may ask of it are two questions, and both + // have to say yes. The second is asked last, because it costs a request and the checks + // above do not. + return viewerPermissionsOf(project, input, "runAction").pipe( + Effect.flatMap((viewer): Effect.Effect => { + if (!viewer.actions.includes(input.action)) { + return Effect.fail( + new PullRequestOperationError({ + operation: "runAction", + detail: ACTION_ACCESS_REFUSALS[input.action], + }), + ); + } + if ( + input.updateMethod !== undefined && + !(viewer.updateMethods ?? []).includes(input.updateMethod) + ) { + return Effect.fail( + new PullRequestOperationError({ + operation: "runAction", + detail: ACTION_ACCESS_REFUSALS["update-branch"], + }), + ); + } + return project.api + .runAction({ + cwd: project.project.workspaceRoot, + repository: project.repository, + host: project.host, + number: input.number, + action: input.action, + ...(input.mergeMethod === undefined ? {} : { mergeMethod: input.mergeMethod }), + ...(input.updateMethod === undefined + ? {} + : { updateMethod: input.updateMethod }), + }) + .pipe( + Effect.mapError(toPullRequestError("runAction")), + Effect.as(project.repository), + ); + }), + ); }), - ); - }), + ), + ), ); const comment: PullRequestService["Service"]["comment"] = (input) => @@ -1771,35 +1787,39 @@ export const make = Effect.gen(function* () { */ const setReaction: PullRequestService["Service"]["setReaction"] = (input) => requireProject(input).pipe( - Effect.flatMap((project): Effect.Effect => { - const subject = - input.subjectId === undefined - ? "change-request" - : input.subjectId.startsWith("issue:") - ? "issue-comment" - : input.subjectId.startsWith("review-comment:") - ? "review-comment" - : "review"; - if (!pullRequestCanReact(project.api.capabilities, subject)) { - return Effect.fail( - new PullRequestOperationError({ - operation: "setReaction", - detail: "This host cannot react to this part of the conversation.", - }), - ); - } - return project.api - .setReaction({ - cwd: project.project.workspaceRoot, - repository: project.repository, - host: project.host, - number: input.number, - ...(input.subjectId === undefined ? {} : { subjectId: input.subjectId }), - content: input.content, - reacted: input.reacted, - }) - .pipe(Effect.mapError(toPullRequestError("setReaction"))); - }), + Effect.flatMap((project): Effect.Effect => + capabilitiesOf(project).pipe( + Effect.flatMap((capabilities): Effect.Effect => { + const subject = + input.subjectId === undefined + ? "change-request" + : input.subjectId.startsWith("issue:") + ? "issue-comment" + : input.subjectId.startsWith("review-comment:") + ? "review-comment" + : "review"; + if (!pullRequestCanReact(capabilities, subject)) { + return Effect.fail( + new PullRequestOperationError({ + operation: "setReaction", + detail: "This host cannot react to this part of the conversation.", + }), + ); + } + return project.api + .setReaction({ + cwd: project.project.workspaceRoot, + repository: project.repository, + host: project.host, + number: input.number, + ...(input.subjectId === undefined ? {} : { subjectId: input.subjectId }), + content: input.content, + reacted: input.reacted, + }) + .pipe(Effect.mapError(toPullRequestError("setReaction"))); + }), + ), + ), ); /** From 786a8a86b29b2d37d396256bc02726ee5b0669f9 Mon Sep 17 00:00:00 2001 From: Kalven Schraut Date: Fri, 4 Sep 2026 23:27:13 -0500 Subject: [PATCH 2/4] fix(pull-requests): defer Gitea feature discovery until requested --- .../pullRequest/GiteaPullRequestApi.test.ts | 125 +++++++++--------- .../src/pullRequest/GiteaPullRequestApi.ts | 5 +- 2 files changed, 63 insertions(+), 67 deletions(-) diff --git a/apps/server/src/pullRequest/GiteaPullRequestApi.test.ts b/apps/server/src/pullRequest/GiteaPullRequestApi.test.ts index c4794a1f6bdd..768325f909b7 100644 --- a/apps/server/src/pullRequest/GiteaPullRequestApi.test.ts +++ b/apps/server/src/pullRequest/GiteaPullRequestApi.test.ts @@ -11,19 +11,12 @@ const mockedRequest = vi.fn(); const decodeJson = Schema.decodeUnknownSync(Schema.fromJsonString(Schema.Unknown)); const layer = it.layer( - GiteaPullRequestApi.layer.pipe( - Layer.provide( - Layer.succeed( - GiteaApi.GiteaApi, - GiteaApi.GiteaApi.of({ - baseUrl: Option.some("https://forge.example.test/gitea"), - sshHosts: ["work-forge"], - request: mockedRequest, - probeAuth: Effect.die("not used"), - }), - ), - ), - ), + Layer.succeed(GiteaApi.GiteaApi, GiteaApi.GiteaApi.of({ + baseUrl: Option.some("https://forge.example.test/gitea"), + sshHosts: ["work-forge"], + request: mockedRequest, + probeAuth: Effect.die("not used"), + })), ); function response(value: unknown, headers: Readonly> = {}) { @@ -150,7 +143,7 @@ it.effect("keeps a search hydration transport failure fatal", () => layer("GiteaPullRequestApi", (it) => { it.effect("validates the requested host before making an HTTP request", () => Effect.gen(function* () { - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; const error = yield* api .getPullRequest({ host: "elsewhere.test", @@ -168,7 +161,7 @@ layer("GiteaPullRequestApi", (it) => { it.effect("accepts an SSH port when the remote names the configured hostname", () => Effect.gen(function* () { mockedRequest.mockReturnValueOnce(Effect.succeed(response(rawPullRequest(7)))); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; const pullRequest = yield* api.getPullRequest({ host: "forge.example.test:2222", repository: "acme/web", @@ -183,7 +176,7 @@ layer("GiteaPullRequestApi", (it) => { it.effect("accepts a configured SSH alias for pull request reads", () => Effect.gen(function* () { mockedRequest.mockReturnValueOnce(Effect.succeed(response(rawPullRequest(7)))); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; assert.strictEqual( (yield* api.getPullRequest({ host: "work-forge", repository: "acme/web", number: 7 })) .number, @@ -214,7 +207,7 @@ layer("GiteaPullRequestApi", (it) => { ]), ), ); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; const page = yield* api.listPullRequests({ host: "forge.example.test", repository: "acme/web", @@ -250,7 +243,7 @@ layer("GiteaPullRequestApi", (it) => { ), ) .mockReturnValueOnce(Effect.succeed(response([rawPullRequest(51)]))); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; const page = yield* api.listPullRequests({ host: "forge.example.test", repository: "acme/web", @@ -274,7 +267,7 @@ layer("GiteaPullRequestApi", (it) => { response(Array.from({ length: 50 }, (_, index) => rawPullRequest(index + 1))), ), ); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; const page = yield* api.listPullRequests({ host: "forge.example.test", repository: "acme/web", @@ -301,7 +294,7 @@ layer("GiteaPullRequestApi", (it) => { .mockReturnValueOnce(Effect.succeed(response([{ number: 7 }, { number: 8 }]))) .mockReturnValueOnce(Effect.succeed(response(rawPullRequest(7)))) .mockReturnValueOnce(Effect.succeed(response(rawPullRequest(8)))); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; const page = yield* api.listPullRequests({ host: "forge.example.test", repository: "acme/web", @@ -342,7 +335,7 @@ layer("GiteaPullRequestApi", (it) => { }), ); }); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; const page = yield* api.listPullRequests({ host: "forge.example.test", repository: "acme/web", @@ -393,7 +386,7 @@ layer("GiteaPullRequestApi", (it) => { ), ), ); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; const page = yield* api.listPullRequests({ host: "forge.example.test", repository: "acme/web", @@ -423,7 +416,7 @@ layer("GiteaPullRequestApi", (it) => { ) .mockReturnValueOnce(Effect.succeed(response(rawPullRequest(3)))) .mockReturnValueOnce(Effect.succeed(response(rawPullRequest(4)))); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; const page = yield* api.listPullRequests({ host: "forge.example.test", repository: "acme/web", @@ -452,7 +445,7 @@ layer("GiteaPullRequestApi", (it) => { mockedRequest.mockImplementation(() => Effect.succeed(response([{ number: "malformed" }], { "x-total-count": "101" })), ); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; const error = yield* api .listPullRequests({ host: "forge.example.test", @@ -492,7 +485,7 @@ layer("GiteaPullRequestApi", (it) => { ), ), ); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; const page = yield* api.listPullRequests({ host: "forge.example.test", repository: "acme/web", @@ -518,7 +511,7 @@ layer("GiteaPullRequestApi", (it) => { mockedRequest.mockImplementation(() => Effect.succeed(response([rawPullRequest(1)], { "x-total-count": "101" })), ); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; const error = yield* api .listPullRequests({ host: "forge.example.test", @@ -551,7 +544,7 @@ layer("GiteaPullRequestApi", (it) => { ), ), ); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; const pullRequest = yield* api.getPullRequest({ host: "forge.example.test", repository: "acme/web", @@ -581,7 +574,7 @@ layer("GiteaPullRequestApi", (it) => { }), ), ); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; const access = yield* api.getRepositoryAccess({ host: "forge.example.test", repository: "acme/web", @@ -602,7 +595,7 @@ layer("GiteaPullRequestApi", (it) => { it.effect("does not turn omitted repository permissions into a denial", () => Effect.gen(function* () { mockedRequest.mockReturnValueOnce(Effect.succeed(response({}))); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; const access = yield* api.getRepositoryAccess({ host: "forge.example.test", repository: "acme/web", @@ -665,7 +658,7 @@ layer("GiteaPullRequestApi", (it) => { ]), ), ); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; const activity = yield* api.listReviews({ host: "forge.example.test", repository: "acme/web", @@ -736,7 +729,7 @@ layer("GiteaPullRequestApi", (it) => { ]), ), ); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; const result = yield* api.listComments({ host: "forge.example.test", repository: "acme/web", @@ -770,7 +763,7 @@ layer("GiteaPullRequestApi", (it) => { ), ) .mockReturnValueOnce(Effect.succeed(response([]))); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; const result = yield* api.listReviews({ host: "forge.example.test", repository: "acme/web", @@ -788,7 +781,7 @@ layer("GiteaPullRequestApi", (it) => { it.effect("encodes an inline review with native old and new positions", () => Effect.gen(function* () { mockedRequest.mockReturnValueOnce(Effect.succeed(response({}))); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; yield* api.submitReview({ host: "forge.example.test", repository: "acme/web", @@ -851,7 +844,7 @@ layer("GiteaPullRequestApi", (it) => { }), ), ); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; const checks = yield* api.listChecks({ host: "forge.example.test", repository: "acme/web", @@ -870,7 +863,7 @@ layer("GiteaPullRequestApi", (it) => { it.effect("rejects repository and file traversal before any HTTP request", () => Effect.gen(function* () { - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; const repositoryError = yield* api .getPullRequest({ host: "forge.example.test", @@ -918,7 +911,7 @@ layer("GiteaPullRequestApi", (it) => { }), ); }); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; const files = yield* api.getDiffFileContents({ host: "forge.example.test", repository: "acme/web", @@ -972,7 +965,7 @@ layer("GiteaPullRequestApi", (it) => { }), ); }); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; const files = yield* api.getDiffFileContents({ host: "forge.example.test", repository: "acme/web", @@ -1001,7 +994,7 @@ layer("GiteaPullRequestApi", (it) => { mockedRequest.mockReturnValueOnce( Effect.succeed(response(rawPullRequest(7, { merge_base: "" }))), ); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; const error = yield* api .getDiffFileContents({ host: "forge.example.test", @@ -1030,7 +1023,7 @@ layer("GiteaPullRequestApi", (it) => { }), ), ); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; const error = yield* api .getDiffFileContents({ host: "forge.example.test", @@ -1051,7 +1044,7 @@ layer("GiteaPullRequestApi", (it) => { it.effect("posts a general pull request comment to its issue conversation", () => Effect.gen(function* () { mockedRequest.mockReturnValueOnce(Effect.succeed(response({}))); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; yield* api.comment({ host: "forge.example.test", repository: "acme/web", @@ -1076,7 +1069,7 @@ layer("GiteaPullRequestApi", (it) => { mockedRequest .mockReturnValueOnce(Effect.succeed(response({}))) .mockReturnValueOnce(Effect.succeed(response({}))); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; yield* api.updateComment({ host: "forge.example.test", repository: "acme/web", @@ -1108,7 +1101,7 @@ layer("GiteaPullRequestApi", (it) => { mockedRequest .mockReturnValueOnce(Effect.succeed(response({}))) .mockReturnValueOnce(Effect.succeed(response({}))); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; yield* api.setReaction({ host: "forge.example.test", repository: "acme/web", @@ -1154,7 +1147,7 @@ layer("GiteaPullRequestApi", (it) => { Effect.succeed(response([{ reaction: "heart", user: { login: "friend" } }])), ) .mockReturnValueOnce(Effect.succeed(response([]))); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; const reactions = yield* api.listConversationReactions({ host: "forge.example.test", repository: "acme/web", @@ -1173,9 +1166,9 @@ layer("GiteaPullRequestApi", (it) => { expect(reactions.bySubjectId.has("review:21")).toBe(false); expect(mockedRequest.mock.calls.map((call) => call[0].path)).toEqual([ "/settings/api", - "/repos/acme/web/issues/7/reactions?page=1&limit=100", - "/repos/acme/web/issues/comments/12/reactions?page=1&limit=100", - "/repos/acme/web/issues/comments/34/reactions?page=1&limit=100", + "/repos/acme/web/issues/7/reactions?page=1&limit=50", + "/repos/acme/web/issues/comments/12/reactions?page=1&limit=50", + "/repos/acme/web/issues/comments/34/reactions?page=1&limit=50", ]); }), ); @@ -1184,17 +1177,17 @@ layer("GiteaPullRequestApi", (it) => { Effect.gen(function* () { mockedRequest.mockImplementation((input) => { if (input.path === "/settings/api") return Effect.succeed(response({ features: [] })); - if (input.path === "/repos/acme/web/issues/7/reactions?page=1&limit=100") + if (input.path === "/repos/acme/web/issues/7/reactions?page=1&limit=50") return Effect.succeed( response([{ reaction: "heart", user: { login: "one" } }], { "x-total-count": "2" }), ); - if (input.path === "/repos/acme/web/issues/7/reactions?page=2&limit=100") + if (input.path === "/repos/acme/web/issues/7/reactions?page=2&limit=50") return Effect.succeed( response([{ reaction: "eyes", user: { login: "two" } }], { "x-total-count": "2" }), ); return Effect.die(`unexpected request: ${input.path}`); }); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; const reactions = yield* api.listConversationReactions({ host: "forge.example.test", repository: "acme/web", @@ -1211,7 +1204,8 @@ layer("GiteaPullRequestApi", (it) => { it.effect("reports Gitea's missing review-summary reaction route", () => Effect.gen(function* () { - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + mockedRequest.mockReturnValueOnce(Effect.succeed(response({features: []}))); + const api = yield* GiteaPullRequestApi.make; const error = yield* api .setReaction({ host: "forge.example.test", @@ -1234,7 +1228,7 @@ layer("GiteaPullRequestApi", (it) => { mockedRequest .mockReturnValueOnce(Effect.succeed(response({ features: ["pull-review-reactions"] }))) .mockReturnValueOnce(Effect.succeed(response({}))); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; yield* api.setReaction({ host: "forge.example.test", repository: "acme/web", @@ -1255,7 +1249,7 @@ layer("GiteaPullRequestApi", (it) => { mockedRequest .mockReturnValueOnce(Effect.succeed(response(rawPullRequest(7)))) .mockReturnValueOnce(Effect.succeed(response({}))); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; yield* api.setLabels({ host: "forge.example.test", repository: "acme/web", @@ -1279,7 +1273,7 @@ layer("GiteaPullRequestApi", (it) => { mockedRequest .mockReturnValueOnce(Effect.succeed(response(rawPullRequest(7)))) .mockReturnValueOnce(Effect.succeed(response({}))); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; yield* api.runAction({ host: "forge.example.test", repository: "acme/web", @@ -1303,6 +1297,7 @@ layer("GiteaPullRequestApi", (it) => { Effect.gen(function* () { mockedRequest .mockReturnValueOnce(Effect.succeed(response({}))) + .mockReturnValueOnce(Effect.succeed(response({features: []}))) .mockReturnValueOnce(Effect.succeed(response(rawPullRequest(7)))) .mockReturnValueOnce(Effect.succeed(response({}))) .mockReturnValueOnce( @@ -1327,7 +1322,7 @@ layer("GiteaPullRequestApi", (it) => { ) .mockReturnValueOnce(Effect.succeed(response({}))) .mockReturnValueOnce(Effect.succeed(response(rawPullRequest(7)))); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; yield* api.runAction({ host: "forge.example.test", repository: "acme/web", @@ -1349,19 +1344,20 @@ layer("GiteaPullRequestApi", (it) => { }); expect(callAt(0).path).toBe("/repos/acme/web/pulls/7/update?style=rebase"); - expect(decodeJson(callAt(2).body ?? "{}")).toEqual({ + expect(decodeJson(callAt(3).body ?? "{}")).toEqual({ title: "WIP: Pull request 7", }); - expect(decodeJson(callAt(5).body ?? "{}")).toEqual({ + expect(decodeJson(callAt(6).body ?? "{}")).toEqual({ title: "Pull request 7", }); - assert.strictEqual(mockedRequest.mock.calls.length, 7); + assert.strictEqual(mockedRequest.mock.calls.length, 8); }), ); it.effect("restores the title when Gitea does not recognize the configured draft prefix", () => Effect.gen(function* () { mockedRequest + .mockReturnValueOnce(Effect.succeed(response({features: []}))) .mockReturnValueOnce(Effect.succeed(response(rawPullRequest(7)))) .mockReturnValueOnce(Effect.succeed(response({}))) .mockReturnValueOnce( @@ -1375,7 +1371,7 @@ layer("GiteaPullRequestApi", (it) => { ), ) .mockReturnValueOnce(Effect.succeed(response({}))); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; const error = yield* api .runAction({ host: "forge.example.test", @@ -1386,7 +1382,7 @@ layer("GiteaPullRequestApi", (it) => { .pipe(Effect.flip); expect(error.detail).toContain("T3CODE_GITEA_DRAFT_PREFIXES"); - expect(decodeJson(callAt(3).body ?? "{}")).toEqual({ + expect(decodeJson(callAt(4).body ?? "{}")).toEqual({ title: "Pull request 7", }); }), @@ -1406,7 +1402,7 @@ layer("GiteaPullRequestApi", (it) => { ]), ), ); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; assert.isTrue( yield* api.getAutoMergeEnabled({ @@ -1434,7 +1430,7 @@ layer("GiteaPullRequestApi", (it) => { ), ) .mockReturnValueOnce(Effect.succeed(response([{ id: 51, type: "pull_scheduled_merge" }]))); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; assert.isTrue( yield* api.getAutoMergeEnabled({ @@ -1449,6 +1445,7 @@ layer("GiteaPullRequestApi", (it) => { it.effect("honors a server timeline page-size cap before reading the final merge state", () => Effect.gen(function* () { + mockedRequest.mockReturnValueOnce(Effect.succeed(response({features: []}))); mockedRequest.mockReturnValueOnce( Effect.succeed( response([{ id: 1, type: "pull_scheduled_merge" }], { "x-total-count": "2" }), @@ -1459,7 +1456,7 @@ layer("GiteaPullRequestApi", (it) => { response([{ id: 2, type: "pull_cancel_scheduled_merge" }], { "x-total-count": "2" }), ), ); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; assert.isFalse( yield* api.getAutoMergeEnabled({ host: "forge.example.test", @@ -1467,7 +1464,7 @@ layer("GiteaPullRequestApi", (it) => { number: 7, }), ); - expect(callAt(1).path).toContain("page=2"); + expect(callAt(2).path).toContain("page=2"); }), ); @@ -1477,7 +1474,7 @@ layer("GiteaPullRequestApi", (it) => { .mockReturnValueOnce(Effect.succeed(response(rawPullRequest(7)))) .mockReturnValueOnce(Effect.succeed(response({}))) .mockReturnValueOnce(Effect.succeed(response({}))); - const api = yield* GiteaPullRequestApi.GiteaPullRequestApi; + const api = yield* GiteaPullRequestApi.make; yield* api.runAction({ host: "forge.example.test", repository: "acme/web", diff --git a/apps/server/src/pullRequest/GiteaPullRequestApi.ts b/apps/server/src/pullRequest/GiteaPullRequestApi.ts index 0febbfdce967..99d44d1ea0e5 100644 --- a/apps/server/src/pullRequest/GiteaPullRequestApi.ts +++ b/apps/server/src/pullRequest/GiteaPullRequestApi.ts @@ -854,7 +854,7 @@ export const make = Effect.gen(function* () { ); const getFeatures = yield* Effect.cachedWithTTL( - gitea.request({ operation: "getFeatures", method: "GET", path: "/settings/api" }).pipe( + Effect.suspend(() => gitea.request({ operation: "getFeatures", method: "GET", path: "/settings/api" })).pipe( Effect.mapError((error) => failure("getFeatures", error)), Effect.flatMap((response) => decode( @@ -1955,8 +1955,7 @@ export const make = Effect.gen(function* () { ); } return Effect.gen(function* () { - const features = yield* getFeatures.pipe(Effect.orElseSucceed(() => [])); - if (target.kind === "review" && !features.includes("pull-review-reactions")) { + if (target.kind === "review" && !(yield* getFeatures.pipe(Effect.orElseSucceed(() => []))).includes("pull-review-reactions")) { return yield* new GiteaPullRequestApiError({ operation: "setReaction", reason: "failed", From a180e8fe7641aa597d2c6973700c64ccdf976090 Mon Sep 17 00:00:00 2001 From: Kalven Schraut Date: Fri, 4 Sep 2026 23:42:00 -0500 Subject: [PATCH 3/4] fix(pull-requests): preserve dynamic provider capabilities --- .../pullRequest/PullRequestService.test.ts | 56 +++++++++++++++++++ .../src/pullRequest/PullRequestService.ts | 3 + 2 files changed, 59 insertions(+) diff --git a/apps/server/src/pullRequest/PullRequestService.test.ts b/apps/server/src/pullRequest/PullRequestService.test.ts index 43ba73d325ef..0dd5a30fb396 100644 --- a/apps/server/src/pullRequest/PullRequestService.test.ts +++ b/apps/server/src/pullRequest/PullRequestService.test.ts @@ -1869,6 +1869,62 @@ it.effect("refuses to react on a host whose capabilities omit reactions entirely }), ); +it.effect("uses dynamic reaction capabilities after rate-limit wrapping", () => + Effect.gen(function* () { + let reactionCalls = 0; + const service = yield* makeService({ + projects: [ + project({ id: "p1", title: "t3code", workspaceRoot: "/a", repository: "pingdotgg/t3code" }), + ], + providers: [ + fakeProvider("github", { + capabilities: { + diff: true, + comment: true, + actions: ["merge"], + mergeMethods: ["merge"], + search: true, + reactions: false, + reactionSubjects: { + changeRequest: true, + issueComment: true, + reviewComment: true, + review: false, + }, + review: FULL_REVIEW, + reviewers: FULL_REVIEWERS, + }, + getCapabilities: () => + Effect.succeed({ + ...fakeProvider("github").capabilities, + reactions: false, + reactionSubjects: { + changeRequest: true, + issueComment: true, + reviewComment: true, + review: true, + }, + }), + setReaction: () => + Effect.sync(() => { + reactionCalls += 1; + }), + }), + ], + }); + + yield* service.setReaction({ + projectId: "p1" as ProjectId, + repository: "pingdotgg/t3code", + number: 1, + subjectId: "review:9", + content: "heart", + reacted: true, + }); + assert.strictEqual(reactionCalls, 1); + }), +); + it.effect("passes a reaction through with its subject id on a host that has them", () => Effect.gen(function* () { let received: { diff --git a/apps/server/src/pullRequest/PullRequestService.ts b/apps/server/src/pullRequest/PullRequestService.ts index 101ebaa22b86..f6a0405ec9b2 100644 --- a/apps/server/src/pullRequest/PullRequestService.ts +++ b/apps/server/src/pullRequest/PullRequestService.ts @@ -457,6 +457,9 @@ function withRateLimitBackoff( return { kind: api.kind, capabilities: api.capabilities, + ...(api.getCapabilities === undefined + ? {} + : { getCapabilities: wrap("getCapabilities", api.getCapabilities) }), getViewer: wrap("getViewer", api.getViewer), listChangeRequests: wrap("listChangeRequests", api.listChangeRequests), ...(api.listChangeRequestsAcross === undefined From 4ca2ad5c6a99e8e284ed3444ff2089f431905870 Mon Sep 17 00:00:00 2001 From: Kalven Schraut Date: Fri, 4 Sep 2026 23:43:26 -0500 Subject: [PATCH 4/4] fix(pull-requests): retain Gitea fallback when discovery fails --- apps/server/src/pullRequest/GiteaPullRequestApi.test.ts | 7 +++++++ apps/server/src/pullRequest/GiteaPullRequestApi.ts | 5 ++--- apps/server/src/pullRequest/GiteaPullRequestProvider.ts | 4 ++-- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/apps/server/src/pullRequest/GiteaPullRequestApi.test.ts b/apps/server/src/pullRequest/GiteaPullRequestApi.test.ts index 768325f909b7..f272999522e2 100644 --- a/apps/server/src/pullRequest/GiteaPullRequestApi.test.ts +++ b/apps/server/src/pullRequest/GiteaPullRequestApi.test.ts @@ -141,6 +141,13 @@ it.effect("keeps a search hydration transport failure fatal", () => ); layer("GiteaPullRequestApi", (it) => { + it.effect("reconstructs auto-merge from the timeline when discovery is unavailable", () => Effect.gen(function* () { + mockedRequest.mockReturnValueOnce(Effect.fail(new GiteaApi.GiteaApiError({operation: "getFeatures", reason: "failed", detail: "temporarily unavailable"}))).mockReturnValueOnce(Effect.succeed(response([{id: 1, type: "pull_scheduled_merge"}]))); + const api = yield* GiteaPullRequestApi.make; + expect(yield* api.getAutoMergeEnabled({host: "forge.example.test", repository: "acme/web", number: 7})).toBe(true); + expect(callAt(1).path).toContain("/timeline?"); + })); + it.effect("validates the requested host before making an HTTP request", () => Effect.gen(function* () { const api = yield* GiteaPullRequestApi.make; diff --git a/apps/server/src/pullRequest/GiteaPullRequestApi.ts b/apps/server/src/pullRequest/GiteaPullRequestApi.ts index 99d44d1ea0e5..8411ee84f9bb 100644 --- a/apps/server/src/pullRequest/GiteaPullRequestApi.ts +++ b/apps/server/src/pullRequest/GiteaPullRequestApi.ts @@ -1367,9 +1367,8 @@ export const make = Effect.gen(function* () { const getAutoMergeEnabled = Effect.fn("GiteaPullRequestApi.getAutoMergeEnabled")( function* (input: { host: string; repository: string; number: number }) { - const features = yield* getFeatures.pipe(Effect.option); - if (Option.isNone(features)) return undefined; - if (features.value.includes("pull-auto-merge-state")) { + const features = yield* getFeatures.pipe(Effect.orElseSucceed(() => [])); + if (features.includes("pull-auto-merge-state")) { return (yield* getPullRequest(input)).autoMergeEnabled; } const operation = "getAutoMergeEnabled"; diff --git a/apps/server/src/pullRequest/GiteaPullRequestProvider.ts b/apps/server/src/pullRequest/GiteaPullRequestProvider.ts index 6c8e1cbfdf6d..30e8664726a9 100644 --- a/apps/server/src/pullRequest/GiteaPullRequestProvider.ts +++ b/apps/server/src/pullRequest/GiteaPullRequestProvider.ts @@ -137,10 +137,10 @@ export const make = Effect.gen(function* () { const provider: PullRequestProviderApi = { kind: "gitea", capabilities: CAPABILITIES, - getCapabilities: ({ host }) => + getCapabilities: () => api.getFeatures().pipe( + Effect.orElseSucceed(() => []), Effect.map((features) => giteaForkCapabilities(CAPABILITIES, features)), - Effect.mapError(fail(`getCapabilities:${host}`)), ), getViewer: () => api.getViewer().pipe(Effect.mapError(fail("getViewer"))),