diff --git a/apps/server/scripts/measure-pr-preview.ts b/apps/server/scripts/measure-pr-preview.ts new file mode 100644 index 0000000000000000000000000000000000000000..b7938edcac763a6e4e17d348575ce13d18ee0f90 --- /dev/null +++ b/apps/server/scripts/measure-pr-preview.ts @@ -0,0 +1,128 @@ +// Run with: node apps/server/scripts/measure-pr-preview.ts owner/repo 123 456 +// Numbers form a session with shared repository-permission caches. Browser and +// service caches are excluded. Uses real GitHub reads, without a server or database. +// CLI-generated GraphQL +// queries are replayed with rateLimit.cost, outside the timed section, to measure +// their cost without confusing other applications' traffic with this process's. +import * as NodeServices from "@effect/platform-node/NodeServices"; +import * as Effect from "effect/Effect"; +import * as Console from "effect/Console"; +import * as Layer from "effect/Layer"; +import * as Schema from "effect/Schema"; + +import * as GitHubPullRequestCli from "../src/pullRequest/GitHubPullRequestCli.ts"; +import * as GitHubPullRequestProvider from "../src/pullRequest/GitHubPullRequestProvider.ts"; +import * as GitHubCli from "../src/sourceControl/GitHubCli.ts"; +import * as VcsProcess from "../src/vcs/VcsProcess.ts"; + +const [repository, ...numbers] = process.argv.slice(2); +if (!repository || numbers.length === 0 || numbers.some((number) => !/^\d+$/.test(number))) { + throw new Error("Usage: node apps/server/scripts/measure-pr-preview.ts owner/repo number..."); +} + +type Read = { graphqlRequests: number; restRequests: number; cost: number; debug: string }; +const reads: Read[] = []; +const measuredProcess = Layer.effect( + VcsProcess.VcsProcess, + Effect.gen(function* () { + const vcs = yield* VcsProcess.VcsProcess; + return VcsProcess.VcsProcess.of({ + run: (input) => + vcs.run({ ...input, env: { ...input.env, GH_DEBUG: input.env?.GH_DEBUG ?? "api" } }).pipe( + Effect.tap((output) => + Effect.sync(() => { + const graphqlRequests = [...output.stderr.matchAll(/^> POST \/graphql /gm)].length; + const requests = [...output.stderr.matchAll(/^> (?:GET|POST) /gm)].length; + const cost = /"rateLimit"\s*:\s*\{[^}]*"cost"\s*:\s*(\d+)/.exec(output.stdout)?.[1]; + reads.push({ + graphqlRequests, + restRequests: requests - graphqlRequests, + cost: Number(cost ?? 0), + debug: output.stderr, + }); + }), + ), + ), + }); + }), +).pipe(Layer.provide(VcsProcess.layer), Layer.provide(NodeServices.layer)); + +const services = GitHubPullRequestCli.layer.pipe( + Layer.provide(GitHubCli.layer), + Layer.provideMerge(measuredProcess), +); + +const decodeJson = Schema.decodeSync(Schema.fromJsonString(Schema.Unknown)); +const encodeJson = Schema.encodeSync(Schema.fromJsonString(Schema.Unknown)); +const graphqlCost = Effect.fn("measurePrPreview.graphqlCost")(function* (read: Read) { + // The REST quota probe synthesizes rateLimit.cost for budget admission. + // It does not spend GraphQL points. + if (!read.graphqlRequests) return 0; + if (read.cost) return read.cost; + const vcs = yield* VcsProcess.VcsProcess; + let cost = 0; + let found = 0; + for (const match of read.debug.matchAll( + /GraphQL query:\n([\s\S]*?)\nGraphQL variables: (\{[^\n]*\})/g, + )) { + const query = match[1]!.replace(/(\bquery\b[^{]*\{)/, "$1 rateLimit { cost }"); + const output = yield* vcs.run({ + operation: "measurePrPreview.cost", + command: "gh", + cwd: process.cwd(), + args: [ + "api", + "graphql", + "--hostname", + "github.com", + "--input", + "-", + "--jq", + ".data.rateLimit.cost", + ], + stdin: encodeJson({ query, variables: decodeJson(match[2]!) }), + env: { GH_DEBUG: "" }, + }); + cost += Number(output.stdout.trim()); + found++; + } + if (found !== read.graphqlRequests) + throw new Error("Could not account for every GraphQL request"); + return cost; +}); + +for (const mode of ["detail", "preview"] as const) { + // Start each side cold, then retain the caches shared across PRs in one session. + const rows = await Effect.runPromise( + Effect.scoped( + Effect.gen(function* () { + const provider = yield* GitHubPullRequestProvider.make; + return yield* Effect.forEach(numbers.map(Number), (number) => + Effect.gen(function* () { + const input = { cwd: process.cwd(), repository, host: "github.com", number }; + reads.length = 0; + const start = performance.now(); + const value = yield* mode === "detail" + ? provider.getChangeRequest(input) + : provider.getChangeRequestPreview!(input); + const elapsedMs = Math.round(performance.now() - start); + const measured = [...reads]; + const costs = yield* Effect.forEach(measured, graphqlCost); + const points = costs.reduce((total, cost) => total + cost, 0); + return { + repository, + number, + mode, + state: value.state, + elapsedMs, + points, + graphqlRequests: measured.reduce((total, read) => total + read.graphqlRequests, 0), + restRequests: measured.reduce((total, read) => total + read.restRequests, 0), + }; + }), + ); + }), + ).pipe(Effect.provide(services)), + ); + for (const row of rows) await Effect.runPromise(Console.log(encodeJson(row))); +} diff --git a/apps/server/src/auth/RpcAuthorization.ts b/apps/server/src/auth/RpcAuthorization.ts index b9ae42f77e768080b68594b2ae48caf1f4e839a5..20f2893b83351e3cad6908766148904d19ae7376 100644 --- a/apps/server/src/auth/RpcAuthorization.ts +++ b/apps/server/src/auth/RpcAuthorization.ts @@ -73,6 +73,7 @@ export const RPC_REQUIRED_SCOPES = { [WS_METHODS.pullRequestsStack]: AuthOrchestrationReadScope, [WS_METHODS.pullRequestsLinkedThreads]: AuthOrchestrationReadScope, [WS_METHODS.pullRequestsDetail]: AuthOrchestrationReadScope, + [WS_METHODS.pullRequestsPreview]: AuthOrchestrationReadScope, [WS_METHODS.pullRequestsActivity]: AuthOrchestrationReadScope, [WS_METHODS.pullRequestsThreadComments]: AuthOrchestrationReadScope, [WS_METHODS.pullRequestsDiffFileContents]: AuthOrchestrationReadScope, diff --git a/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts b/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts index 23c14da3b48cfceb24f557d13226cde486f01fd9..fd5ab04e8a815f5dbadb750cc3da0256c7b11bba 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts @@ -1,5 +1,6 @@ import { afterEach, assert, expect, it, vi } from "@effect/vitest"; import * as Effect from "effect/Effect"; +import * as Clock from "effect/Clock"; import * as Deferred from "effect/Deferred"; import * as Fiber from "effect/Fiber"; import * as Layer from "effect/Layer"; @@ -16,6 +17,36 @@ import { BASE_COMPARISON_GRAPHQL_QUERY } from "./gitHubPullRequestJson.ts"; const encodeJson = Schema.encodeSync(Schema.fromJsonString(Schema.Unknown)); +const coreResponse = (pullRequest: Readonly> = {}) => ({ + data: { + repository: { + mergeCommitAllowed: true, + squashMergeAllowed: false, + rebaseMergeAllowed: true, + viewerPermission: "WRITE", + pullRequest: { + number: 7, + title: "Pull request 7", + url: "https://github.com/acme/web/pull/7", + headRefName: "feature", + baseRefName: "main", + headRefOid: "abc123", + state: "OPEN", + createdAt: "2026-07-01T00:00:00Z", + updatedAt: "2026-07-02T00:00:00Z", + viewerCanUpdate: true, + viewerDidAuthor: false, + viewerCanUpdateBranch: true, + baseRef: { compare: { behindBy: 2 } }, + reviewRequests: { nodes: [] }, + labels: { nodes: [] }, + commits: { nodes: [] }, + ...pullRequest, + }, + }, + }, +}); + const mockedExecute = vi.fn(); const mockedStackMemberships = vi.fn(() => Effect.succeed(output('{"data":{}}')), @@ -33,7 +64,7 @@ const layer = it.layer( getPullRequest: mockedGetPullRequest, }), ), - Layer.provide(GitHubGraphQlBudget.layer), + Layer.provideMerge(GitHubGraphQlBudget.layer), ), ); @@ -255,6 +286,118 @@ it.effect( ); layer("GitHubPullRequestCli.layer", (it) => { + it.effect("admits only one concurrent preview above the reserve and resumes after reset", () => + Effect.gen(function* () { + const now = yield* Clock.currentTimeMillis; + const budget = yield* GitHubGraphQlBudget.GitHubGraphQlBudget; + const resetAt = "2099-08-13T14:00:00Z"; + yield* budget.observe( + "preview-budget.example", + encodeJson({ data: { rateLimit: { cost: 1, limit: 5_000, remaining: 501, resetAt } } }), + ); + mockedExecute.mockReturnValue( + Effect.succeed( + output( + encodeJson({ + data: { + repository: { + pullRequest: { + number: 7, + title: "Fast previews", + url: "https://preview-budget.example/acme/web/pull/7", + state: "OPEN", + isDraft: false, + createdAt: "2026-07-01T00:00:00Z", + author: null, + }, + }, + rateLimit: { cost: 1, limit: 5_000, remaining: 500, resetAt }, + }, + }), + ), + ), + ); + const cli = yield* GitHubPullRequestCli.GitHubPullRequestCli; + const input = { + cwd: "/w", + repository: "acme/web", + host: "preview-budget.example", + number: 7, + }; + const results = yield* Effect.all( + Array.from({ length: 20 }, (_, index) => + cli.getPullRequestPreview({ ...input, number: index + 1 }).pipe(Effect.result), + ), + { concurrency: "unbounded" }, + ); + expect(results.filter((result) => result._tag === "Success")).toHaveLength(1); + for (const result of results) { + if (result._tag === "Failure") { + expect(result.failure._tag).toBe("SourceControlRateLimitPausedError"); + } + } + expect(mockedExecute).toHaveBeenCalledTimes(1); + yield* TestClock.setTime(Date.parse(resetAt)); + yield* cli.getPullRequestPreview(input); + expect(mockedExecute).toHaveBeenCalledTimes(2); + yield* TestClock.setTime(now); + }), + ); + + it.effect("loads the complete hover card with one GraphQL request", () => + Effect.gen(function* () { + mockedExecute.mockReturnValueOnce( + Effect.succeed( + output( + encodeJson({ + data: { + repository: { + pullRequest: { + number: 7, + title: "Fast previews", + url: "https://github.example/acme/web/pull/7", + state: "MERGED", + isDraft: false, + createdAt: "2026-07-01T00:00:00Z", + author: { + login: "octocat", + name: "Octo Cat", + avatarUrl: "https://github.example/avatar.png", + }, + }, + }, + }, + }), + ), + ), + ); + const cli = yield* GitHubPullRequestCli.GitHubPullRequestCli; + const preview = yield* cli.getPullRequestPreview({ + cwd: "/w", + repository: "acme/web", + host: "github.example", + number: 7, + }); + expect(preview).toEqual({ + number: 7, + title: "Fast previews", + url: "https://github.example/acme/web/pull/7", + state: "merged", + isDraft: false, + createdAt: "2026-07-01T00:00:00Z", + author: { + login: "octocat", + name: "Octo Cat", + avatarUrl: "https://github.example/avatar.png", + }, + }); + expect(mockedExecute).toHaveBeenCalledTimes(1); + expect(callAt(0).args).toEqual( + expect.arrayContaining(["api", "graphql", "--hostname", "github.example"]), + ); + }), + ); + it.effect("coalesces concurrent identity verification for the same host and credential", () => Effect.gen(function* () { mockedExecute.mockImplementation((input) => @@ -1741,19 +1884,20 @@ layer("GitHubPullRequestCli.layer", (it) => { mockedExecute.mockReturnValueOnce( Effect.succeed( output( - // @effect-diagnostics-next-line preferSchemaOverJson:off - canned gh response. - JSON.stringify({ - number: 7, - title: "Pull request 7", - url: "https://github.com/acme/web/pull/7", - headRefName: "feat/page", - headRefOid: "abc123", - isCrossRepository: false, - headRepositoryOwner: { login: "acme" }, - baseRefName: "main", - createdAt: "2026-07-01T00:00:00Z", - updatedAt: "2026-07-02T00:00:00Z", - }), + encodeJson( + coreResponse({ + number: 7, + title: "Pull request 7", + url: "https://github.com/acme/web/pull/7", + headRefName: "feat/page", + headRefOid: "abc123", + isCrossRepository: false, + headRepositoryOwner: { login: "acme" }, + baseRefName: "main", + createdAt: "2026-07-01T00:00:00Z", + updatedAt: "2026-07-02T00:00:00Z", + }), + ), ), ), ); @@ -1774,19 +1918,20 @@ layer("GitHubPullRequestCli.layer", (it) => { it.effect("finds and approves every workflow waiting on a maintainer", () => Effect.gen(function* () { const detail = output( - // @effect-diagnostics-next-line preferSchemaOverJson:off - canned gh response. - JSON.stringify({ - number: 7, - title: "Pull request 7", - url: "https://github.com/acme/web/pull/7", - headRefName: "feat/page", - headRefOid: "abc123", - isCrossRepository: true, - headRepositoryOwner: { login: "octocat" }, - baseRefName: "main", - createdAt: "2026-07-01T00:00:00Z", - updatedAt: "2026-07-02T00:00:00Z", - }), + encodeJson( + coreResponse({ + number: 7, + title: "Pull request 7", + url: "https://github.com/acme/web/pull/7", + headRefName: "feat/page", + headRefOid: "abc123", + isCrossRepository: true, + headRepositoryOwner: { login: "octocat" }, + baseRefName: "main", + createdAt: "2026-07-01T00:00:00Z", + updatedAt: "2026-07-02T00:00:00Z", + }), + ), ); const heads = output( // @effect-diagnostics-next-line preferSchemaOverJson:off - canned gh response. @@ -1902,7 +2047,7 @@ layer("GitHubPullRequestCli.layer", (it) => { updatedAt: "2026-07-02T00:00:00Z", }; for (const value of [ - detail, + coreResponse(detail), [ { number: 7, @@ -1912,7 +2057,7 @@ layer("GitHubPullRequestCli.layer", (it) => { }, ], [{ databaseId: 10, workflowName: "build", url: "https://example.com/10" }], - { ...detail, headRefOid: "def456" }, + coreResponse({ ...detail, headRefOid: "def456" }), ]) { mockedExecute.mockReturnValueOnce( Effect.succeed( @@ -1943,8 +2088,47 @@ layer("GitHubPullRequestCli.layer", (it) => { }), ); + it.effect("reads workflow runs and their pull request scope concurrently", () => + Effect.gen(function* () { + const headsStarted = yield* Deferred.make(); + const runsStarted = yield* Deferred.make(); + mockedExecute.mockImplementation(({ args }) => + args[0] === "pr" + ? Deferred.succeed(headsStarted, undefined).pipe( + Effect.andThen(Deferred.await(runsStarted)), + Effect.as( + output( + '[{"number":7,"headRefOid":"abc123","isCrossRepository":true,"headRepositoryOwner":{"login":"octocat"}}]', + ), + ), + ) + : Deferred.succeed(runsStarted, undefined).pipe( + Effect.andThen(Deferred.await(headsStarted)), + Effect.as( + output('[{"databaseId":10,"workflowName":"build","url":"https://example.com/10"}]'), + ), + ), + ); + const cli = yield* GitHubPullRequestCli.GitHubPullRequestCli; + + const runs = yield* cli.listWorkflowRunsRequiringApproval({ + cwd: "/w", + repository: "acme/web", + host: "github.com", + number: 7, + headSha: "abc123", + headBranch: "feat/page", + headRepositoryOwner: "octocat", + isCrossRepository: true, + }); + + expect(runs).toEqual([{ id: 10, name: "build", url: "https://example.com/10" }]); + }), + ); + it.effect("refuses workflow approval when one head belongs to several pull requests", () => Effect.gen(function* () { + mockedExecute.mockReturnValue(Effect.succeed(output("[]"))); mockedExecute.mockReturnValueOnce( Effect.succeed( output( @@ -1983,7 +2167,7 @@ layer("GitHubPullRequestCli.layer", (it) => { limit: 1_000, }); expect(error.detail).toContain("instead of uniquely matching #7"); - expect(mockedExecute).toHaveBeenCalledTimes(1); + expect(mockedExecute).toHaveBeenCalledTimes(2); }), ); @@ -1992,19 +2176,20 @@ layer("GitHubPullRequestCli.layer", (it) => { mockedExecute.mockReturnValueOnce( Effect.succeed( output( - // @effect-diagnostics-next-line preferSchemaOverJson:off - canned gh response. - JSON.stringify({ - number: 7, - title: "Pull request 7", - url: "https://github.com/acme/web/pull/7", - headRefName: "feat/page", - headRefOid: "abc123", - isCrossRepository: true, - headRepositoryOwner: null, - baseRefName: "main", - createdAt: "2026-07-01T00:00:00Z", - updatedAt: "2026-07-02T00:00:00Z", - }), + encodeJson( + coreResponse({ + number: 7, + title: "Pull request 7", + url: "https://github.com/acme/web/pull/7", + headRefName: "feat/page", + headRefOid: "abc123", + isCrossRepository: true, + headRepositoryOwner: null, + baseRefName: "main", + createdAt: "2026-07-01T00:00:00Z", + updatedAt: "2026-07-02T00:00:00Z", + }), + ), ), ), ); @@ -3041,19 +3226,20 @@ layer("GitHubPullRequestCli.layer", (it) => { mockedExecute.mockReturnValueOnce( Effect.succeed( output( - // @effect-diagnostics-next-line preferSchemaOverJson:off - JSON.stringify({ - number: 7, - title: "Progressive detail", - url: "https://github.com/acme/web/pull/7", - author: { login: "octocat" }, - headRefName: "feature", - baseRefName: "main", - createdAt: "2026-07-01T00:00:00Z", - updatedAt: "2026-07-02T00:00:00Z", - body: "Core body", - changedFiles: 2, - }), + encodeJson( + coreResponse({ + number: 7, + title: "Progressive detail", + url: "https://github.com/acme/web/pull/7", + author: { login: "octocat" }, + headRefName: "feature", + baseRefName: "main", + createdAt: "2026-07-01T00:00:00Z", + updatedAt: "2026-07-02T00:00:00Z", + body: "Core body", + changedFiles: 2, + }), + ), ), ), ); @@ -3083,13 +3269,216 @@ layer("GitHubPullRequestCli.layer", (it) => { expect(detail.body).toBe("Core body"); expect(activity.author?.login).toBe("octocat"); - expect(callAt(0).args.at(-1)).toBe( - "number,title,url,author,headRefName,baseRefName,state,isDraft,mergeable,reviewDecision,additions,deletions,createdAt,updatedAt,mergedAt,reviewRequests,labels,statusCheckRollup,body,changedFiles,closedAt,isCrossRepository,headRepositoryOwner,headRefOid,autoMergeRequest", - ); + expect(callAt(0).args).toContain("headRef=refs/pull/7/head"); + expect(callAt(0).args.at(-1)).toContain("viewerCanUpdateBranch"); + expect(detail.viewerAccess.mergeCapabilities).toEqual({ + merge: true, + squash: false, + rebase: true, + }); + expect(detail.comparison).toEqual({ behindBy: 2, viewerCanUpdate: true }); expect(callAt(1).args.at(-1)).toBe("author,comments,reviews,commits"); }), ); + it.effect("decodes reviewers, labels and workflow checks without another detail read", () => + Effect.gen(function* () { + mockedExecute.mockReturnValueOnce( + Effect.succeed( + output( + encodeJson( + coreResponse({ + baseRef: null, + reviewRequests: { + nodes: [ + { requestedReviewer: { login: "reviewer" } }, + { requestedReviewer: { slug: "maintainers", name: "Maintainers" } }, + ], + }, + labels: { nodes: [{ name: "bug", color: "ff0000" }] }, + commits: { + nodes: [ + { + commit: { + statusCheckRollup: { + contexts: { + nodes: [ + { + __typename: "CheckRun", + name: "build", + status: "COMPLETED", + conclusion: "SUCCESS", + checkSuite: { workflowRun: { workflow: { name: "linux" } } }, + }, + { + __typename: "CheckRun", + name: "build", + status: "COMPLETED", + conclusion: "FAILURE", + checkSuite: { workflowRun: { workflow: { name: "windows" } } }, + }, + ], + pageInfo: { hasNextPage: false }, + }, + }, + }, + }, + ], + }, + }), + ), + ), + ), + ); + const cli = yield* GitHubPullRequestCli.GitHubPullRequestCli; + const detail = yield* cli.getPullRequestDetail({ + cwd: "/w", + repository: "acme/web", + host: "github.com", + number: 7, + }); + expect(mockedExecute).toHaveBeenCalledOnce(); + expect(detail.comparison).toBeNull(); + expect(detail.reviewRequestLogins).toEqual(["reviewer"]); + expect(detail.hasTeamReviewRequest).toBe(true); + expect(detail.labels).toEqual([{ name: "bug", color: "ff0000" }]); + expect(detail.checks).toHaveLength(2); + expect(detail.checksState).toBe("failing"); + }), + ); + + it.effect("reads every check when the combined response has another page", () => + Effect.gen(function* () { + const response = coreResponse({ + commits: { + nodes: [ + { + commit: { + statusCheckRollup: { + contexts: { + nodes: [{ name: "first", status: "COMPLETED", conclusion: "SUCCESS" }], + pageInfo: { hasNextPage: true }, + }, + }, + }, + }, + ], + }, + }); + mockedExecute.mockReturnValueOnce(Effect.succeed(output(encodeJson(response)))); + mockedExecute.mockReturnValueOnce( + Effect.succeed( + output( + encodeJson({ + ...response.data.repository.pullRequest, + reviewRequests: [], + labels: [], + statusCheckRollup: [ + { name: "first", status: "COMPLETED", conclusion: "SUCCESS" }, + { name: "last", status: "COMPLETED", conclusion: "FAILURE" }, + ], + }), + ), + ), + ); + const cli = yield* GitHubPullRequestCli.GitHubPullRequestCli; + const detail = yield* cli.getPullRequestDetail({ + cwd: "/w", + repository: "acme/web", + host: "github.com", + number: 7, + }); + expect(detail.checks).toHaveLength(2); + expect(detail.checksState).toBe("failing"); + expect(detail.checksTruncated).toBe(false); + expect(callAt(1).args.slice(0, 2)).toEqual(["pr", "view"]); + }), + ); + + it.effect("refuses to combine checks from different head revisions", () => + Effect.gen(function* () { + const response = coreResponse({ + commits: { + nodes: [ + { + commit: { + statusCheckRollup: { + contexts: { + nodes: [], + pageInfo: { hasNextPage: true }, + }, + }, + }, + }, + ], + }, + }); + mockedExecute.mockReturnValueOnce(Effect.succeed(output(encodeJson(response)))); + mockedExecute.mockReturnValueOnce( + Effect.succeed( + output( + encodeJson({ + ...response.data.repository.pullRequest, + headRefOid: "new-head", + reviewRequests: [], + labels: [], + statusCheckRollup: [], + }), + ), + ), + ); + const cli = yield* GitHubPullRequestCli.GitHubPullRequestCli; + const error = yield* Effect.flip( + cli.getPullRequestDetail({ + cwd: "/w", + repository: "acme/web", + host: "github.com", + number: 7, + }), + ); + expect(error._tag).toBe("GitHubPullRequestReadError"); + }), + ); + + it.effect("preserves the reserve for automatic detail reads and allows manual checks", () => + Effect.gen(function* () { + const response = coreResponse(); + mockedExecute.mockReturnValue( + Effect.succeed( + output( + encodeJson({ + ...response, + data: { + ...response.data, + rateLimit: { + cost: 1, + limit: 5000, + remaining: 500, + resetAt: "2099-08-13T14:00:00Z", + }, + }, + }), + ), + ), + ); + const cli = yield* GitHubPullRequestCli.GitHubPullRequestCli; + const input = { + cwd: "/w", + repository: "acme/web", + host: "github.core-reserve.test", + number: 7, + }; + yield* cli.getPullRequestDetail(input); + const error = yield* Effect.flip(cli.getPullRequestDetail(input)); + expect(error._tag).toBe("SourceControlRateLimitPausedError"); + expect(mockedExecute).toHaveBeenCalledOnce(); + yield* cli + .getPullRequestDetail(input) + .pipe(Effect.provideService(GitHubCli.AllowGitHubReserve, true)); + expect(mockedExecute).toHaveBeenCalledTimes(2); + }), + ); + it.effect("fails a files page too large to read rather than calling the diff whole", () => Effect.gen(function* () { mockedExecute.mockReturnValueOnce(Effect.fail(diffRefused)); @@ -3271,6 +3660,9 @@ layer("GitHubPullRequestCli.layer", (it) => { JSON.stringify({ data: { repository: { + mergeCommitAllowed: true, + squashMergeAllowed: false, + rebaseMergeAllowed: true, viewerPermission: "READ", pullRequest: { viewerCanUpdate: true, viewerDidAuthor: true }, }, @@ -3291,7 +3683,11 @@ layer("GitHubPullRequestCli.layer", (it) => { // One request, because both answers hang off the same repository object. assert.strictEqual(mockedExecute.mock.calls.length, 1); expect(callAt(0).args).toContain("number=7"); + expect(callAt(0).args.at(-1)).toContain( + "mergeCommitAllowed squashMergeAllowed rebaseMergeAllowed", + ); expect(access).toEqual({ + mergeCapabilities: { merge: true, squash: false, rebase: true }, canWrite: false, canTriage: false, canUpdate: true, @@ -3433,6 +3829,9 @@ layer("GitHubPullRequestCli.layer", (it) => { JSON.stringify({ data: { repository: { + mergeCommitAllowed: true, + squashMergeAllowed: false, + rebaseMergeAllowed: true, viewerPermission: "READ", pullRequest: { viewerCanUpdate: true, viewerDidAuthor: true }, }, @@ -3460,6 +3859,7 @@ layer("GitHubPullRequestCli.layer", (it) => { assert.strictEqual(mockedExecute.mock.calls.length, 2); expect(access).toEqual({ + mergeCapabilities: { merge: true, squash: false, rebase: true }, canWrite: false, canTriage: false, canUpdate: true, @@ -3469,38 +3869,6 @@ layer("GitHubPullRequestCli.layer", (it) => { }), ); - it.effect("reads the viewer's role off the same call as the merge settings", () => - Effect.gen(function* () { - mockedExecute.mockReturnValue( - Effect.succeed( - output( - // @effect-diagnostics-next-line preferSchemaOverJson:off - JSON.stringify({ - mergeCommitAllowed: false, - squashMergeAllowed: true, - rebaseMergeAllowed: true, - viewerPermission: "WRITE", - }), - ), - ), - ); - const cli = yield* GitHubPullRequestCli.GitHubPullRequestCli; - - const access = yield* cli.getRepositoryAccess({ - cwd: "/w", - repository: "acme/web", - host: "github.com", - }); - - assert.strictEqual(mockedExecute.mock.calls.length, 1); - expect(callAt(0).args).toContain( - "mergeCommitAllowed,squashMergeAllowed,rebaseMergeAllowed,viewerPermission", - ); - assert.isTrue(access.canWrite); - expect(access.mergeCapabilities).toEqual({ merge: false, squash: true, rebase: true }); - }), - ); - it.effect("asks GitHub to review, naming the collection a request is added to", () => Effect.gen(function* () { mockedExecute.mockReturnValue(Effect.succeed(output("{}"))); diff --git a/apps/server/src/pullRequest/GitHubPullRequestCli.ts b/apps/server/src/pullRequest/GitHubPullRequestCli.ts index d68d77c38f6cfdbdb61f673fca6567d0ed1024bb..723fcfc4d9b959e57bc47c70d113f69b70e3286e 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestCli.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestCli.ts @@ -30,6 +30,7 @@ import { type PullRequestLabelCandidateList, type PullRequestThreadCommentsResult, type PullRequestUpdateMethod, + type PullRequestPreview, } from "@t3tools/contracts"; import * as GitHubCli from "../sourceControl/GitHubCli.ts"; @@ -44,6 +45,11 @@ import { decodeActorAvatarsJson, decodePullRequestActivityJson, decodePullRequestDetailJson, + decodePullRequestCoreJson, + PULL_REQUEST_CORE_GRAPHQL_QUERY, + type GitHubPullRequestCore, + decodePullRequestPreviewJson, + PULL_REQUEST_PREVIEW_GRAPHQL_QUERY, decodePullRequestFilesJson, decodePullRequestFilesViewedJson, decodePullRequestHeadsJson, @@ -53,7 +59,6 @@ import { decodePullRequestStacksJson, decodePullRequestStatsJson, decodeReactionSubjectScopeJson, - decodeRepositoryAccessJson, decodeReviewerCandidatesJson, decodeLabelCandidatesJson, buildLabelRequestJson, @@ -78,7 +83,6 @@ import { REMOVE_REACTION_GRAPHQL_MUTATION, REVERT_PULL_REQUEST_GRAPHQL_MUTATION, gitHubReactionContent, - REPOSITORY_ACCESS_JSON_FIELDS, RESOLVE_REVIEW_THREAD_GRAPHQL_MUTATION, REVIEWER_CANDIDATES_GRAPHQL_QUERY, REVIEW_THREAD_COMMENTS_GRAPHQL_QUERY, @@ -94,7 +98,6 @@ import { decodeViewerPermissionsJson, decodeWorkflowRunApprovalsJson, type GitHubBaseComparison, - type GitHubPullRequestDetail, type GitHubPullRequestActivity, type GitHubPullRequestHead, type GitHubPullRequestListItem, @@ -519,7 +522,17 @@ export class GitHubPullRequestCli extends Context.Service< readonly repository: string; readonly host: string; readonly number: number; - }) => Effect.Effect; + }) => Effect.Effect; + + readonly getPullRequestPreview: (input: { + readonly cwd: string; + readonly repository: string; + readonly host: string; + readonly number: number; + }) => Effect.Effect< + Omit, + GitHubPullRequestCliError + >; readonly listWorkflowRunsRequiringApproval: (input: { readonly cwd: string; @@ -639,13 +652,6 @@ export class GitHubPullRequestCli extends Context.Service< readonly cursor: string; }) => Effect.Effect; - /** One `gh repo view`, which answers what the repository allows and where the viewer stands. */ - readonly getRepositoryAccess: (input: { - readonly cwd: string; - readonly repository: string; - readonly host: string; - }) => Effect.Effect; - /** The viewer's standing on its own, for deciding a write without reading the whole detail. */ readonly getViewerAccess: (input: { readonly cwd: string; @@ -654,7 +660,7 @@ export class GitHubPullRequestCli extends Context.Service< readonly number: number; /** Manual action checks may use the quota held back from automatic reads. */ readonly allowReserve?: boolean | undefined; - }) => Effect.Effect; + }) => Effect.Effect; /** Who this pull request may be sent to, and who it has already been sent to. */ readonly listReviewerCandidates: (input: { @@ -1514,7 +1520,9 @@ export const make = Effect.gen(function* () { return { oldContents, newContents }; }); - const getPullRequestDetail: GitHubPullRequestCli["Service"]["getPullRequestDetail"] = (input) => + const readLegacyDetail = ( + input: Parameters[0], + ) => github .execute({ cwd: input.cwd, @@ -1543,6 +1551,53 @@ export const make = Effect.gen(function* () { }), ); + const getPullRequestDetail: GitHubPullRequestCli["Service"]["getPullRequestDetail"] = (input) => { + const { owner, name } = parseRepositorySelector(input.repository); + return GitHubCli.AllowGitHubReserve.pipe( + Effect.flatMap((allowReserve) => + graphqlRead({ + allowReserve, + cwd: input.cwd, + host: input.host, + operation: "getPullRequestDetail", + variables: [ + ["-f", `owner=${owner}`], + ["-f", `name=${name}`], + ["-F", `number=${input.number}`], + ["-f", `headRef=refs/pull/${input.number}/head`], + ], + query: PULL_REQUEST_CORE_GRAPHQL_QUERY, + decode: decodePullRequestCoreJson, + }), + ), + ).pipe( + Effect.flatMap((core) => { + if (!core.checksTruncated) return Effect.succeed(core); + // gh already pages check contexts. Keep its complete, deduplicated result for + // large check suites instead of letting the first 100 checks imply success. + return readLegacyDetail(input).pipe( + Effect.flatMap((detail) => + detail.headSha !== core.headSha + ? Effect.fail( + new GitHubPullRequestReadError({ + command: "gh", + cwd: input.cwd, + operation: "getPullRequestDetail", + cause: new Error("Pull request head changed while reading checks."), + }), + ) + : Effect.succeed({ + ...core, + checks: detail.checks, + checksState: detail.checksState, + checksTruncated: false, + }), + ), + ); + }), + ); + }; + const workflowApprovalLimit = 1_000; const workflowApprovalProbeLimit = String(workflowApprovalLimit + 1); const workflowApprovalReadError = (cwd: string, cause: unknown) => @@ -1554,71 +1609,74 @@ export const make = Effect.gen(function* () { }); const listWorkflowRunsRequiringApproval: GitHubPullRequestCli["Service"]["listWorkflowRunsRequiringApproval"] = (input) => - github - .execute({ - cwd: input.cwd, - args: [ - "pr", - "list", - ...repositoryArgs(input), - "--state", - "open", - "--head", - input.headBranch, - "--limit", - workflowApprovalProbeLimit, - "--json", - "number,headRefOid,isCrossRepository,headRepositoryOwner", - ], - }) - .pipe( - Effect.flatMap( - ( - result, - ): Effect.Effect< - GitHubPullRequestHead, - GitHubPullRequestReadError | GitHubWorkflowApprovalRefusedError - > => { - const decoded = decodePullRequestHeadsJson(result.stdout.trim()); - if (!Result.isSuccess(decoded)) { - return Effect.fail(workflowApprovalReadError(input.cwd, decoded.failure)); - } - const exactHeads = decoded.success.filter( - (pullRequest) => - pullRequest.headSha === input.headSha && - pullRequest.isCrossRepository === true && - pullRequest.headRepositoryOwner?.toLowerCase() === - input.headRepositoryOwner.toLowerCase(), - ); - if (decoded.success.length > workflowApprovalLimit) { - return Effect.fail( - new GitHubWorkflowApprovalRefusedError({ - command: "gh", - cwd: input.cwd, - number: input.number, - reason: "head-list-truncated", - observedCount: decoded.success.length, - limit: workflowApprovalLimit, - }), - ); - } - if (exactHeads.length !== 1 || exactHeads[0]?.number !== input.number) { - return Effect.fail( - new GitHubWorkflowApprovalRefusedError({ - command: "gh", - cwd: input.cwd, - number: input.number, - reason: "head-not-unique", - observedCount: exactHeads.length, - limit: workflowApprovalLimit, - }), - ); - } - return Effect.succeed(exactHeads[0]); - }, - ), - Effect.flatMap(() => - github.execute({ + Effect.all( + [ + github + .execute({ + cwd: input.cwd, + args: [ + "pr", + "list", + ...repositoryArgs(input), + "--state", + "open", + "--head", + input.headBranch, + "--limit", + workflowApprovalProbeLimit, + "--json", + "number,headRefOid,isCrossRepository,headRepositoryOwner", + ], + }) + .pipe( + Effect.flatMap( + ( + result, + ): Effect.Effect< + GitHubPullRequestHead, + GitHubPullRequestReadError | GitHubWorkflowApprovalRefusedError + > => { + const decoded = decodePullRequestHeadsJson(result.stdout.trim()); + if (!Result.isSuccess(decoded)) { + return Effect.fail(workflowApprovalReadError(input.cwd, decoded.failure)); + } + const exactHeads = decoded.success.filter( + (pullRequest) => + pullRequest.headSha === input.headSha && + pullRequest.isCrossRepository === true && + pullRequest.headRepositoryOwner?.toLowerCase() === + input.headRepositoryOwner.toLowerCase(), + ); + if (decoded.success.length > workflowApprovalLimit) { + return Effect.fail( + new GitHubWorkflowApprovalRefusedError({ + command: "gh", + cwd: input.cwd, + number: input.number, + reason: "head-list-truncated", + observedCount: decoded.success.length, + limit: workflowApprovalLimit, + }), + ); + } + if (exactHeads.length !== 1 || exactHeads[0]?.number !== input.number) { + return Effect.fail( + new GitHubWorkflowApprovalRefusedError({ + command: "gh", + cwd: input.cwd, + number: input.number, + reason: "head-not-unique", + observedCount: exactHeads.length, + limit: workflowApprovalLimit, + }), + ); + } + return Effect.succeed(exactHeads[0]); + }, + ), + ), + github + .execute({ cwd: input.cwd, args: [ "run", @@ -1637,34 +1695,37 @@ export const make = Effect.gen(function* () { "--json", "databaseId,workflowName,url", ], - }), - ), - Effect.flatMap( - ( - result, - ): Effect.Effect< - ReadonlyArray, - GitHubPullRequestReadError | GitHubWorkflowApprovalRefusedError - > => { - const decoded = decodeWorkflowRunApprovalsJson(result.stdout.trim()); - if (!Result.isSuccess(decoded)) { - return Effect.fail(workflowApprovalReadError(input.cwd, decoded.failure)); - } - return decoded.success.length > workflowApprovalLimit - ? Effect.fail( - new GitHubWorkflowApprovalRefusedError({ - command: "gh", - cwd: input.cwd, - number: input.number, - reason: "run-list-truncated", - observedCount: decoded.success.length, - limit: workflowApprovalLimit, - }), - ) - : Effect.succeed(decoded.success); - }, - ), - ); + }) + .pipe( + Effect.flatMap( + ( + result, + ): Effect.Effect< + ReadonlyArray, + GitHubPullRequestReadError | GitHubWorkflowApprovalRefusedError + > => { + const decoded = decodeWorkflowRunApprovalsJson(result.stdout.trim()); + if (!Result.isSuccess(decoded)) { + return Effect.fail(workflowApprovalReadError(input.cwd, decoded.failure)); + } + return decoded.success.length > workflowApprovalLimit + ? Effect.fail( + new GitHubWorkflowApprovalRefusedError({ + command: "gh", + cwd: input.cwd, + number: input.number, + reason: "run-list-truncated", + observedCount: decoded.success.length, + limit: workflowApprovalLimit, + }), + ) + : Effect.succeed(decoded.success); + }, + ), + ), + ], + { concurrency: 2 }, + ).pipe(Effect.map(([, runs]) => runs)); return GitHubPullRequestCli.of({ withVerifiedCredential, @@ -1924,6 +1985,21 @@ export const make = Effect.gen(function* () { ), getPullRequestDetail, + getPullRequestPreview: (input) => { + const { owner, name } = parseRepositorySelector(input.repository); + return graphqlRead({ + cwd: input.cwd, + host: input.host, + operation: "getPullRequestPreview", + variables: [ + ["-f", `owner=${owner}`], + ["-f", `name=${name}`], + ["-F", `number=${input.number}`], + ], + query: PULL_REQUEST_PREVIEW_GRAPHQL_QUERY, + decode: decodePullRequestPreviewJson, + }); + }, listWorkflowRunsRequiringApproval, getPullRequestStack: (input) => { @@ -2246,34 +2322,6 @@ export const make = Effect.gen(function* () { }); }, - getRepositoryAccess: (input) => - github - .execute({ - cwd: input.cwd, - args: [ - "repo", - "view", - `${input.host}/${input.repository}`, - "--json", - REPOSITORY_ACCESS_JSON_FIELDS, - ], - }) - .pipe( - Effect.flatMap((result) => { - const decoded = decodeRepositoryAccessJson(result.stdout.trim()); - return Result.isSuccess(decoded) - ? Effect.succeed(decoded.success) - : Effect.fail( - new GitHubPullRequestReadError({ - command: "gh", - cwd: input.cwd, - operation: "getRepositoryAccess", - cause: decoded.failure, - }), - ); - }), - ), - getViewerAccess: (input) => { const { owner, name } = parseRepositorySelector(input.repository); return graphqlRead({ diff --git a/apps/server/src/pullRequest/GitHubPullRequestProvider.test.ts b/apps/server/src/pullRequest/GitHubPullRequestProvider.test.ts index f2629f68b95670a2841291f2c33e6ab1979b90f7..1882b692f2b0608beb92d7342661e48c0db5ccdd 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestProvider.test.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestProvider.test.ts @@ -9,6 +9,18 @@ import * as GitHubPullRequestCli from "./GitHubPullRequestCli.ts"; import { gitHubViewerPermissions, loginAvatarUrl, make } from "./GitHubPullRequestProvider.ts"; import type { GitHubReviewThreadComments } from "./gitHubPullRequestJson.ts"; +const coreFields = { + checksTruncated: false, + comparison: { behindBy: 0, viewerCanUpdate: true }, + viewerAccess: { + canWrite: true, + canTriage: true, + canUpdate: true, + didAuthor: false, + mergeCapabilities: { merge: true, squash: true, rebase: true }, + }, +}; + it.effect("maps credential verification failures without relabeling operation failures", () => Effect.gen(function* () { let verificationFails = true; @@ -229,7 +241,7 @@ describe("gitHubViewerPermissions", () => { }); }); - it.effect("uses the small viewer-access read for core permissions", () => + it.effect("uses the permissions carried by the core read without a second request", () => Effect.gen(function* () { const provider = yield* make; const detail = yield* provider.getChangeRequest({ @@ -276,6 +288,9 @@ describe("gitHubViewerPermissions", () => { Layer.mock(GitHubPullRequestCli.GitHubPullRequestCli)({ getPullRequestDetail: () => Effect.succeed({ + ...coreFields, + comparison: null, + viewerAccess: { ...coreFields.viewerAccess, canWrite: false, canTriage: false }, authorId: null, number: 7, title: "Pull request 7", @@ -285,9 +300,9 @@ describe("gitHubViewerPermissions", () => { headRepositoryOwner: null, headBranch: "feat/page", baseBranch: "main", - state: "open", + state: "open" as const, isDraft: false, - mergeability: "mergeable", + mergeability: "mergeable" as const, reviewDecision: null, additions: 1, deletions: 1, @@ -304,25 +319,22 @@ describe("gitHubViewerPermissions", () => { checks: [], comments: [], commits: [], - }), - getRepositoryAccess: () => - GitHubCli.PinnedGitHubCredential.pipe( - Effect.map((credential) => ({ - canWrite: false, - mergeCapabilities: { - merge: true, - squash: credential?.credentialFingerprint !== "restricted", - rebase: true, - }, - })), + }).pipe( + Effect.flatMap((detail) => + GitHubCli.PinnedGitHubCredential.pipe( + Effect.map((credential) => ({ + ...detail, + viewerAccess: { + ...detail.viewerAccess, + mergeCapabilities: { + ...detail.viewerAccess.mergeCapabilities, + squash: credential?.credentialFingerprint !== "restricted", + }, + }, + })), + ), + ), ), - getViewerAccess: () => - Effect.succeed({ - canWrite: false, - canTriage: false, - canUpdate: true, - didAuthor: false, - }), }), ), ), @@ -364,6 +376,7 @@ describe("gitHubViewerPermissions", () => { Layer.mock(GitHubPullRequestCli.GitHubPullRequestCli)({ getPullRequestDetail: () => Effect.succeed({ + ...coreFields, authorId: null, number: 7, title: "Pull request 7", @@ -412,13 +425,14 @@ describe("gitHubViewerPermissions", () => { ]), getPullRequestBaseComparison: () => Effect.succeed({ behindBy: 0, viewerCanUpdate: true }), - getRepositoryAccess: () => + getViewerAccess: () => Effect.succeed({ canWrite: true, + canTriage: true, + canUpdate: true, + didAuthor: false, mergeCapabilities: { merge: true, squash: true, rebase: true }, }), - getViewerAccess: () => - Effect.succeed({ canWrite: true, canTriage: true, canUpdate: true, didAuthor: false }), }), ), ), @@ -426,6 +440,7 @@ describe("gitHubViewerPermissions", () => { }); const openDetail = { + ...coreFields, authorId: null, number: 7, title: "Pull request 7", @@ -457,6 +472,39 @@ const openDetail = { commits: [], }; +it.effect( + "uses the core comparison and permissions while preserving workflow approval checks", + () => + Effect.gen(function* () { + const provider = yield* make.pipe( + Effect.provide( + Layer.mock(GitHubPullRequestCli.GitHubPullRequestCli)({ + getPullRequestDetail: () => + Effect.succeed({ ...openDetail, comparison: { behindBy: 2, viewerCanUpdate: true } }), + listWorkflowRunsRequiringApproval: () => + Effect.succeed([{ id: 123, name: "tests", url: "https://example.com/runs/123" }]), + }), + ), + ); + + const detail = yield* provider.getChangeRequest({ + cwd: "/w", + repository: "acme/web", + host: "github.com", + number: 7, + }); + + expect(detail.behindBy).toBe(2); + expect(detail.workflowApprovalsRequired).toBe(1); + expect(detail.checks).toContainEqual({ + name: "tests", + status: "action-required", + description: "A maintainer must approve this workflow before it can run.", + url: "https://example.com/runs/123", + }); + }), +); + it.effect("does not classify same-repository gates as fork workflow approvals", () => Effect.gen(function* () { const provider = yield* make; @@ -476,13 +524,14 @@ it.effect("does not classify same-repository gates as fork workflow approvals", getPullRequestBaseComparison: () => Effect.succeed({ behindBy: 0, viewerCanUpdate: true }), listWorkflowRunsRequiringApproval: () => Effect.die("same-repository pull requests must not probe fork workflow approvals"), - getRepositoryAccess: () => + getViewerAccess: () => Effect.succeed({ canWrite: true, + canTriage: true, + canUpdate: true, + didAuthor: false, mergeCapabilities: { merge: true, squash: true, rebase: true }, }), - getViewerAccess: () => - Effect.succeed({ canWrite: true, canTriage: true, canUpdate: true, didAuthor: false }), }), ), ), @@ -523,13 +572,14 @@ it.effect("keeps an unsafe workflow approval scope visible as unknown", () => limit: 1_000, }), ), - getRepositoryAccess: () => + getViewerAccess: () => Effect.succeed({ canWrite: true, + canTriage: true, + canUpdate: true, + didAuthor: false, mergeCapabilities: { merge: true, squash: true, rebase: true }, }), - getViewerAccess: () => - Effect.succeed({ canWrite: true, canTriage: true, canUpdate: true, didAuthor: false }), }), ), ), @@ -562,13 +612,14 @@ it.effect("propagates workflow discovery rate limits", () => cause: new Error("rate limited"), }), ), - getRepositoryAccess: () => + getViewerAccess: () => Effect.succeed({ canWrite: true, + canTriage: true, + canUpdate: true, + didAuthor: false, mergeCapabilities: { merge: true, squash: true, rebase: true }, }), - getViewerAccess: () => - Effect.succeed({ canWrite: true, canTriage: true, canUpdate: true, didAuthor: false }), }), ), ), @@ -598,7 +649,13 @@ describe("getViewerPermissions", () => { getViewerAccess: () => Effect.sync(() => { accessReads++; - return { canWrite: true, canTriage: true, canUpdate: true, didAuthor: false }; + return { + canWrite: true, + canTriage: true, + canUpdate: true, + didAuthor: false, + mergeCapabilities: { merge: true, squash: true, rebase: true }, + }; }), }), ), @@ -615,7 +672,13 @@ describe("getViewerPermissions", () => { getPullRequestDetail: () => Effect.succeed(openDetail), getPullRequestBaseComparison: () => comparison, getViewerAccess: () => - Effect.succeed({ canWrite: true, canTriage: true, canUpdate: true, didAuthor: false }), + Effect.succeed({ + canWrite: true, + canTriage: true, + canUpdate: true, + didAuthor: false, + mergeCapabilities: { merge: true, squash: true, rebase: true }, + }), }); it.effect("offers update-branch when the comparison grants it", () => @@ -661,7 +724,13 @@ describe("getViewerPermissions", () => { getViewerAccess: (input) => Effect.sync(() => { viewerAllowReserve = input.allowReserve; - return { canWrite: true, canTriage: true, canUpdate: true, didAuthor: false }; + return { + canWrite: true, + canTriage: true, + canUpdate: true, + didAuthor: false, + mergeCapabilities: { merge: true, squash: true, rebase: true }, + }; }), }), ), @@ -696,7 +765,13 @@ describe("getViewerPermissions", () => { }), ), getViewerAccess: () => - Effect.succeed({ canWrite: true, canTriage: true, canUpdate: true, didAuthor: false }), + Effect.succeed({ + canWrite: true, + canTriage: true, + canUpdate: true, + didAuthor: false, + mergeCapabilities: { merge: true, squash: true, rebase: true }, + }), }), ), ), diff --git a/apps/server/src/pullRequest/GitHubPullRequestProvider.ts b/apps/server/src/pullRequest/GitHubPullRequestProvider.ts index e73e666f76cb9e41ba1f33e5d460394cddc689fb..0b76fdda88fc067f9e2bd1f6a31605646ad94ff4 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestProvider.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestProvider.ts @@ -1,7 +1,4 @@ -import * as Cache from "effect/Cache"; -import * as Duration from "effect/Duration"; import * as Effect from "effect/Effect"; -import * as Exit from "effect/Exit"; import type { PullRequestActor, PullRequestCapabilities, @@ -11,7 +8,6 @@ import type { } from "@t3tools/contracts"; import * as GitHubPullRequestCli from "./GitHubPullRequestCli.ts"; -import { PinnedGitHubCredential } from "../sourceControl/GitHubCli.ts"; import { PullRequestProviderError, type PullRequestProviderFailure, @@ -193,35 +189,6 @@ const rendersEmpty = (body: string): boolean => export const make = Effect.gen(function* () { const cli = yield* GitHubPullRequestCli.GitHubPullRequestCli; - const repositoryAccessCache = yield* Cache.makeWith( - (key: string) => { - const [cwd, repository, host] = JSON.parse(key) as [string, string, string]; - return cli.getRepositoryAccess({ cwd, repository, host }); - }, - { - capacity: 128, - timeToLive: (exit) => (Exit.isSuccess(exit) ? Duration.minutes(10) : Duration.zero), - }, - ); - const getRepositoryAccess = (input: { - readonly cwd: string; - readonly repository: string; - readonly host: string; - }) => - PinnedGitHubCredential.pipe( - Effect.flatMap((credential) => - Cache.get( - repositoryAccessCache, - JSON.stringify([ - input.cwd, - input.repository, - input.host, - credential?.credentialFingerprint ?? null, - ]), - ), - ), - ); - const fail = (operation: string) => (error: GitHubPullRequestCli.GitHubPullRequestCliError) => new PullRequestProviderError({ provider: "github", @@ -340,102 +307,80 @@ export const make = Effect.gen(function* () { getChangeRequestStack: (input) => cli.getPullRequestStack(input).pipe(Effect.mapError(fail("getChangeRequestStack"))), + getChangeRequestPreview: (input) => + cli.getPullRequestPreview(input).pipe(Effect.mapError(fail("getChangeRequestPreview"))), + getChangeRequest: (input) => - Effect.all( - [ - cli.getPullRequestDetail(input).pipe( - Effect.flatMap((pullRequest) => - Effect.all({ - // Only an open pull request can be behind anything worth saying so about, and - // only one whose head repository is known can be compared at all. - comparison: - pullRequest.state !== "open" || pullRequest.headRepositoryOwner === null - ? Effect.succeed(null) - : cli - .getPullRequestBaseComparison({ - ...input, - headRef: `${pullRequest.headRepositoryOwner}:${pullRequest.headBranch}`, - }) - .pipe(Effect.orElseSucceed(() => null)), - // GitHub omits a fork workflow that has not been approved from the normal check - // rollup. Read the action-required runs by head revision so "all passed" cannot - // be shown while a whole workflow is still waiting to start. - workflowApprovals: - pullRequest.state !== "open" || pullRequest.isCrossRepository !== true - ? Effect.succeed({ - runs: [] as ReadonlyArray, - unavailable: false, - }) - : pullRequest.headSha == null || pullRequest.headRepositoryOwner == null - ? Effect.succeed({ - runs: [] as ReadonlyArray, - unavailable: true, - }) - : cli - .listWorkflowRunsRequiringApproval({ - ...input, - headSha: pullRequest.headSha, - headBranch: pullRequest.headBranch, - headRepositoryOwner: pullRequest.headRepositoryOwner, - isCrossRepository: true, - }) - .pipe( - Effect.matchEffect({ - onFailure: (error) => - error._tag === "GitHubCliRateLimitError" || - error._tag === "SourceControlRateLimitPausedError" - ? Effect.fail(error) - : Effect.succeed({ - runs: [] as ReadonlyArray, - unavailable: true, - }), - onSuccess: (runs) => Effect.succeed({ runs, unavailable: false }), - }), - ), - }).pipe(Effect.map((extra) => ({ pullRequest, ...extra }))), - ), - ), - getRepositoryAccess({ - cwd: input.cwd, - repository: input.repository, - host: input.host, - }), - // A small permissions query replaces the deeply paginated review-thread walk on the - // core path. Writes ask again immediately before mutating, so this is presentation. - cli.getViewerAccess(input), - ], - { concurrency: 3 }, - ).pipe( + cli.getPullRequestDetail(input).pipe( + Effect.flatMap((pullRequest) => { + // Fork workflows awaiting approval are absent from the normal check rollup. + const approvals = + pullRequest.state !== "open" || pullRequest.isCrossRepository !== true + ? Effect.succeed({ + runs: [] as ReadonlyArray, + unavailable: false, + }) + : pullRequest.headSha == null || pullRequest.headRepositoryOwner == null + ? Effect.succeed({ + runs: [] as ReadonlyArray, + unavailable: true, + }) + : cli + .listWorkflowRunsRequiringApproval({ + ...input, + headSha: pullRequest.headSha, + headBranch: pullRequest.headBranch, + headRepositoryOwner: pullRequest.headRepositoryOwner, + isCrossRepository: true, + }) + .pipe( + Effect.matchEffect({ + onFailure: (error) => + error._tag === "GitHubCliRateLimitError" || + error._tag === "SourceControlRateLimitPausedError" + ? Effect.fail(error) + : Effect.succeed({ + runs: [] as ReadonlyArray, + unavailable: true, + }), + onSuccess: (runs) => Effect.succeed({ runs, unavailable: false }), + }), + ); + return approvals.pipe( + Effect.map((workflowApprovals): ProviderChangeRequestDetail => ({ + ...pullRequest, + author: withAvatar(pullRequest.author, new Map(), input.host), + checks: withWorkflowApprovals( + pullRequest.checks, + workflowApprovals.runs, + workflowApprovals.unavailable, + ), + ...(workflowApprovals.unavailable + ? {} + : { workflowApprovalsRequired: workflowApprovals.runs.length }), + reviewers: pullRequest.reviewRequestLogins.map((login) => ({ + login, + name: null, + avatarUrl: null, + })), + mergeCapabilities: pullRequest.viewerAccess.mergeCapabilities, + viewerPermissions: gitHubViewerPermissions({ + ...pullRequest.viewerAccess, + canUpdateBranch: pullRequest.comparison?.viewerCanUpdate === true, + }), + baseComparison: + pullRequest.comparison === null || pullRequest.comparison.behindBy === null + ? "unknown" + : pullRequest.comparison.behindBy > 0 + ? "behind" + : "up-to-date", + ...(pullRequest.comparison?.behindBy == null + ? {} + : { behindBy: pullRequest.comparison.behindBy }), + })), + ); + }), Effect.mapError(fail("getChangeRequest")), - Effect.map(([detail, repository, viewerAccess]): ProviderChangeRequestDetail => ({ - ...detail.pullRequest, - author: withAvatar(detail.pullRequest.author, new Map(), input.host), - checks: withWorkflowApprovals( - detail.pullRequest.checks, - detail.workflowApprovals.runs, - detail.workflowApprovals.unavailable, - ), - ...(detail.workflowApprovals.unavailable - ? {} - : { workflowApprovalsRequired: detail.workflowApprovals.runs.length }), - reviewers: detail.pullRequest.reviewRequestLogins.map((login) => ({ - login, - name: null, - avatarUrl: null, - })), - mergeCapabilities: repository.mergeCapabilities, - viewerPermissions: gitHubViewerPermissions({ - ...viewerAccess, - canUpdateBranch: detail.comparison?.viewerCanUpdate === true, - }), - baseComparison: - detail.comparison === null || detail.comparison.behindBy === null - ? "unknown" - : detail.comparison.behindBy > 0 - ? "behind" - : "up-to-date", - ...(detail.comparison?.behindBy == null ? {} : { behindBy: detail.comparison.behindBy }), - })), ), getChangeRequestActivity: (input) => diff --git a/apps/server/src/pullRequest/PullRequestProvider.ts b/apps/server/src/pullRequest/PullRequestProvider.ts index eb09f81417cc22707b2548503b335bb1aaf97673..b4361c125c38fd32a2d0d98bb79faf1d16c5559b 100644 --- a/apps/server/src/pullRequest/PullRequestProvider.ts +++ b/apps/server/src/pullRequest/PullRequestProvider.ts @@ -31,6 +31,7 @@ import type { PullRequestReviewerKind, PullRequestLabelCandidateList, PullRequestState, + PullRequestPreview, PullRequestUpdateMethod, PullRequestViewerPermissions, SourceControlProviderKind, @@ -400,6 +401,14 @@ export interface PullRequestProviderApi { input: ProviderRepositoryRef & { readonly number: number }, ) => Effect.Effect; + /** Hosts without a narrow read use their existing detail response for hover cards. */ + readonly getChangeRequestPreview?: ( + input: ProviderRepositoryRef & { readonly number: number }, + ) => Effect.Effect< + Omit, + PullRequestProviderError + >; + /** * The cheap live fields used by linked threads. Optional because a provider without a narrow * endpoint can fall back to its full detail read at the service boundary. diff --git a/apps/server/src/pullRequest/PullRequestService.test.ts b/apps/server/src/pullRequest/PullRequestService.test.ts index 4000d383642d514f4e95762eba571342e620713b..8e9262a081048c30e58d7fe380c26ae7bad06cb6 100644 --- a/apps/server/src/pullRequest/PullRequestService.test.ts +++ b/apps/server/src/pullRequest/PullRequestService.test.ts @@ -116,6 +116,215 @@ function hostedChangeRequest(body: string, additions = 1) { }; } +it.effect("caches narrow previews and invalidates them after refresh or mutation", () => + Effect.gen(function* () { + let reads = 0; + const service = yield* makeService({ + projects: [project({ id: "p1", title: "web", workspaceRoot: "/w", repository: "acme/web" })], + providers: [ + fakeProvider("github", { + getViewer: () => Effect.die("preview must not read the viewer"), + getChangeRequestPreview: () => + Effect.sync(() => { + reads += 1; + return changeRequest(1, "2026-07-02T00:00:00Z"); + }), + }), + ], + }); + const ref = { projectId: "p1" as ProjectId, repository: "acme/web", number: 1 }; + const previews = yield* Effect.all([service.preview(ref), service.preview(ref)], { + concurrency: 2, + }); + assert.deepStrictEqual(previews[0], { + ...ref, + title: "Change request 1", + url: "https://host/pull/1", + author: { login: "octocat", name: null, avatarUrl: null }, + state: "open", + isDraft: false, + createdAt: "2026-07-01T00:00:00Z", + }); + assert.strictEqual(reads, 1); + yield* service.invalidate({ reference: ref }); + yield* service.preview(ref); + assert.strictEqual(reads, 2); + yield* service.runAction({ ...ref, action: "close" }); + yield* service.preview(ref); + assert.strictEqual(reads, 3); + yield* service.refreshAfterTurn(ref.projectId); + yield* service.preview(ref); + assert.strictEqual(reads, 4); + const error = yield* Effect.flip(service.preview({ ...ref, repository: "another/repo" })); + assert.strictEqual(error._tag, "PullRequestOperationError"); + assert.strictEqual(reads, 4); + }), +); + +it.effect("keeps cached previews available and pauses uncached previews until quota resets", () => + Effect.gen(function* () { + let reads = 0; + const retryAt = Date.parse("2099-08-13T14:00:00Z"); + const service = yield* makeService({ + projects: [project({ id: "p1", title: "web", workspaceRoot: "/w", repository: "acme/web" })], + providers: [ + fakeProvider("github", { + getChangeRequestPreview: ({ number }) => + Effect.gen(function* () { + reads++; + if (reads === 2) + return yield* new PullRequestProviderError({ + provider: "github", + operation: "getChangeRequestPreview", + reason: "rate-limited", + detail: "GitHub requests are paused until the rate limit resets.", + retryAt, + }); + return changeRequest(number, "2026-07-02T00:00:00Z"); + }), + }), + ], + }); + const ref = { projectId: "p1" as ProjectId, repository: "acme/web", number: 1 }; + yield* service.preview(ref); + const limited = yield* Effect.flip(service.preview({ ...ref, number: 2 })); + assert.instanceOf(limited, PullRequestOperationError); + if (limited._tag !== "PullRequestOperationError") return; + assert.include(limited.detail, "paused"); + assert.strictEqual((yield* service.preview(ref)).number, 1); + for (const number of [2, 3, 4]) { + const paused = yield* Effect.flip(service.preview({ ...ref, number })); + assert.instanceOf(paused, PullRequestOperationError); + if (paused._tag !== "PullRequestOperationError") return; + assert.include(paused.detail, "paused"); + } + assert.strictEqual(reads, 2); + yield* TestClock.setTime(retryAt); + assert.strictEqual((yield* service.preview({ ...ref, number: 2 })).number, 2); + assert.strictEqual(reads, 3); + }), +); + +it.effect("reuses only unexpired detail for previews", () => + Effect.gen(function* () { + let previewReads = 0; + const service = yield* makeService({ + projects: [project({ id: "p1", title: "web", workspaceRoot: "/w", repository: "acme/web" })], + providers: [ + fakeProvider("github", { + getChangeRequest: () => Effect.succeed(hostedChangeRequest("Description")), + getChangeRequestPreview: () => + Effect.sync(() => { + previewReads++; + return { + ...changeRequest(1, "2026-07-02T00:00:00Z"), + title: "Updated title", + state: "closed" as const, + }; + }), + }), + ], + }); + const ref = { projectId: "p1" as ProjectId, repository: "acme/web", number: 1 }; + yield* service.detail(ref); + assert.strictEqual((yield* service.preview(ref)).title, "Change request 1"); + assert.strictEqual(previewReads, 0); + yield* TestClock.adjust("16 seconds"); + const preview = yield* service.preview(ref); + assert.strictEqual(preview.title, "Updated title"); + assert.strictEqual(preview.state, "closed"); + assert.strictEqual(previewReads, 1); + }), +); + +it.effect("does not wait for an in-flight detail read to display a preview", () => + Effect.gen(function* () { + const detailStarted = yield* Deferred.make(); + const releaseDetail = yield* Deferred.make(); + const service = yield* makeService({ + projects: [project({ id: "p1", title: "web", workspaceRoot: "/w", repository: "acme/web" })], + providers: [ + fakeProvider("github", { + getChangeRequest: () => + Deferred.succeed(detailStarted, undefined).pipe( + Effect.andThen(Deferred.await(releaseDetail)), + Effect.as(hostedChangeRequest("Description")), + ), + getChangeRequestPreview: () => Effect.succeed(changeRequest(1, "2026-07-02T00:00:00Z")), + }), + ], + }); + const ref = { projectId: "p1" as ProjectId, repository: "acme/web", number: 1 }; + const detail = yield* Effect.forkChild(service.detail(ref)); + yield* Deferred.await(detailStarted); + assert.strictEqual((yield* service.preview(ref)).title, "Change request 1"); + yield* Deferred.succeed(releaseDetail, undefined); + yield* Fiber.join(detail); + }), +); + +it.effect("keeps previews warm when another project finishes a turn", () => + Effect.gen(function* () { + const reads: string[] = []; + const service = yield* makeService({ + projects: [ + project({ id: "p1", title: "web", workspaceRoot: "/w", repository: "acme/web" }), + project({ id: "p2", title: "docs", workspaceRoot: "/d", repository: "acme/docs" }), + ], + providers: [ + fakeProvider("github", { + getChangeRequestPreview: (input) => + Effect.sync(() => { + reads.push(input.repository); + return changeRequest(1, "2026-07-02T00:00:00Z"); + }), + }), + ], + }); + const web = { projectId: "p1" as ProjectId, repository: "acme/web", number: 1 }; + const docs = { projectId: "p2" as ProjectId, repository: "acme/docs", number: 1 }; + yield* service.preview(web); + yield* service.preview(docs); + yield* service.preview({ ...web, host: "github.com" }); + assert.deepStrictEqual(reads, ["acme/web", "acme/docs"]); + yield* service.refreshAfterTurn(web.projectId); + yield* service.preview(web); + yield* service.preview(docs); + assert.deepStrictEqual(reads, ["acme/web", "acme/docs", "acme/web"]); + }), +); + +it.effect("uses full detail for hosts without a narrow preview", () => + Effect.gen(function* () { + let reads = 0; + const service = yield* makeService({ + projects: [ + project({ + id: "p1", + title: "web", + workspaceRoot: "/w", + repository: "acme/web", + provider: "gitlab", + }), + ], + providers: [ + fakeProvider("gitlab", { + getChangeRequest: () => + Effect.sync(() => { + reads += 1; + return hostedChangeRequest("Description"); + }), + }), + ], + }); + const ref = { projectId: "p1" as ProjectId, repository: "acme/web", number: 1 }; + const preview = yield* service.preview(ref); + assert.strictEqual(preview.title, "Change request 1"); + assert.strictEqual(reads, 1); + assert.ok(!("body" in preview)); + }), +); + function unusable(provider: SourceControlProviderKind, reason: "missing-tool" | "unauthenticated") { return new PullRequestProviderError({ provider, @@ -1840,6 +2049,13 @@ it.effect("routes explicit Forgejo HTTP authorities through SSH checkouts after state: "open", }); assert.strictEqual(listed.viewers["code.example:3000"], "bilal"); + const preview = yield* service.preview({ + projectId: "ssh" as ProjectId, + host: "code.example:3000", + repository: "team/repo", + number: 42, + }); + assert.strictEqual(preview.number, 42); const detail = yield* service.detail({ projectId: "ssh" as ProjectId, host: "code.example:3000", @@ -4071,7 +4287,7 @@ it.effect("keeps routed reads separate when the GitHub account changes", () => it.effect("isolates routed caches for two credentials belonging to the same account", () => Effect.gen(function* () { - for (const operation of ["summary", "detail", "diff", "filesViewed"] as const) { + for (const operation of ["summary", "detail", "diff", "preview", "filesViewed"] as const) { let credential = "broad"; let calls = 0; const read = () => @@ -4109,6 +4325,7 @@ it.effect("isolates routed caches for two credentials belonging to the same acco read().pipe( Effect.as({ patch: "private patch", truncated: false, nextCursor: null }), ), + getChangeRequestPreview: read, }), ], }); diff --git a/apps/server/src/pullRequest/PullRequestService.ts b/apps/server/src/pullRequest/PullRequestService.ts index f1369c21d418ddb0f2233f88237287533b914586..2cc1180bb826633d2d097ea8b3a204bc33cfbf2d 100644 --- a/apps/server/src/pullRequest/PullRequestService.ts +++ b/apps/server/src/pullRequest/PullRequestService.ts @@ -32,6 +32,7 @@ import { type PullRequestCommentInput, type PullRequestCommentUpdateInput, type PullRequestDetail, + type PullRequestPreview, type PullRequestDiffFileContentsInput, type PullRequestDiffFileContentsResult, type PullRequestDiffStat, @@ -202,6 +203,9 @@ export class PullRequestService extends Context.Service< readonly subscribeRefreshes: Stream.Stream; readonly refreshAfterTurn: (projectId: ProjectId) => Effect.Effect; readonly detail: (input: PullRequestRef) => Effect.Effect; + readonly preview: ( + input: PullRequestRef, + ) => Effect.Effect; readonly activity: ( input: PullRequestRef, ) => Effect.Effect; @@ -546,6 +550,9 @@ function withRateLimitBackoff( listChangeRequestStats: wrap("listChangeRequestStats", api.listChangeRequestStats), }), getChangeRequest: wrap("getChangeRequest", api.getChangeRequest), + ...(api.getChangeRequestPreview === undefined + ? {} + : { getChangeRequestPreview: wrap("getChangeRequestPreview", api.getChangeRequestPreview) }), ...(api.getChangeRequestSummary === undefined ? {} : { @@ -1679,6 +1686,38 @@ export const make = Effect.gen(function* () { ), ); + const previewFields = (value: PullRequestPreview): PullRequestPreview => ({ + projectId: value.projectId, + repository: value.repository, + number: value.number, + title: value.title, + url: value.url, + author: value.author, + state: value.state, + isDraft: value.isDraft, + createdAt: value.createdAt, + }); + const previewUncached: PullRequestService["Service"]["preview"] = (input) => + requireProject(input).pipe( + Effect.flatMap((project) => + (project.api.getChangeRequestPreview ?? project.api.getChangeRequest)({ + cwd: project.project.workspaceRoot, + repository: project.repository, + host: project.host, + number: input.number, + }).pipe( + Effect.mapError(toPullRequestError("preview")), + Effect.map((value) => + previewFields({ + ...value, + projectId: project.project.id, + repository: project.repository, + }), + ), + ), + ), + ); + const activityUncached: PullRequestService["Service"]["activity"] = (input) => requireProject(input).pipe( Effect.flatMap((project) => @@ -2885,6 +2924,27 @@ export const make = Effect.gen(function* () { timeToLive: (exit) => (Exit.isSuccess(exit) ? DETAIL_CACHE_TTL : Duration.zero), }, ); + + const previewCache = yield* Cache.makeWith( + (key: string) => { + return previewUncached(refOfCacheKey(key)); + }, + { + capacity: DETAIL_CACHE_CAPACITY, + timeToLive: (exit) => (Exit.isSuccess(exit) ? DETAIL_CACHE_TTL : Duration.zero), + }, + ); + const preview: PullRequestService["Service"]["preview"] = (input) => { + const key = refCacheKey(input); + return Cache.getSuccess(detailCache, key).pipe( + Effect.flatMap( + Option.match({ + onNone: () => Cache.get(previewCache, key), + onSome: (detail) => Effect.succeed(previewFields(detail)), + }), + ), + ); + }; const activity: PullRequestService["Service"]["activity"] = (input) => { const key = refCacheKey(input); return Cache.get(activityCache, key); @@ -3131,6 +3191,7 @@ export const make = Effect.gen(function* () { refreshAfterTurn, detail: credentialCached(detail), activity: credentialCached(activity), + preview: credentialCached(preview), threadComments, diff: credentialCached(diff), diffFileContents, diff --git a/apps/server/src/pullRequest/gitHubPullRequestJson.test.ts b/apps/server/src/pullRequest/gitHubPullRequestJson.test.ts index 8100a167808eaf81f4a209cd4a144e3a65ff5e2e..4591ac0437bf55f518470e744d8865516b4d1dc6 100644 --- a/apps/server/src/pullRequest/gitHubPullRequestJson.test.ts +++ b/apps/server/src/pullRequest/gitHubPullRequestJson.test.ts @@ -17,7 +17,6 @@ import { decodePullRequestSearchJson, decodePullRequestStacksJson, decodeLabelCandidatesJson, - decodeRepositoryAccessJson, decodeReviewerCandidatesJson, decodeReviewThreadCommentsJson, decodeReviewThreadsJson, @@ -810,31 +809,38 @@ describe("reaction decoding", () => { describe("repository access decoding", () => { const repositoryJson = (viewerPermission?: string | null) => JSON.stringify({ - mergeCommitAllowed: true, - squashMergeAllowed: false, - rebaseMergeAllowed: true, - ...(viewerPermission === undefined ? {} : { viewerPermission }), + data: { + repository: { + pullRequest: null, + mergeCommitAllowed: true, + squashMergeAllowed: false, + rebaseMergeAllowed: true, + ...(viewerPermission === undefined ? {} : { viewerPermission }), + }, + }, }); - it("reads the three settings gh reports", () => { + it("reads merge settings with viewer permissions", () => { expect( - expectSuccess(decodeRepositoryAccessJson(repositoryJson("ADMIN"))).mergeCapabilities, + expectSuccess(decodeViewerPermissionsJson(repositoryJson("ADMIN"))).mergeCapabilities, ).toEqual({ merge: true, squash: false, rebase: true }); }); it("fails rather than defaulting open when a setting is missing", () => { - const decoded = decodeRepositoryAccessJson(JSON.stringify({ mergeCommitAllowed: true })); + const decoded = decodeViewerPermissionsJson( + JSON.stringify({ data: { repository: { pullRequest: null, mergeCommitAllowed: true } } }), + ); expect(Result.isSuccess(decoded)).toBe(false); }); it("counts the roles that can push as write, and the ones that cannot as read", () => { for (const permission of ["ADMIN", "MAINTAIN", "WRITE"]) { - expect(expectSuccess(decodeRepositoryAccessJson(repositoryJson(permission))).canWrite).toBe( + expect(expectSuccess(decodeViewerPermissionsJson(repositoryJson(permission))).canWrite).toBe( true, ); } for (const permission of ["TRIAGE", "READ", "NONE"]) { - expect(expectSuccess(decodeRepositoryAccessJson(repositoryJson(permission))).canWrite).toBe( + expect(expectSuccess(decodeViewerPermissionsJson(repositoryJson(permission))).canWrite).toBe( false, ); } @@ -843,14 +849,23 @@ describe("repository access decoding", () => { it("withholds write where gh names no permission, which is not a standing it gave", () => { // The one place an unknown answer is not granted: a Merge button a reader cannot use wastes // the press, where a missing one still leaves the pull request open on its host. - expect(expectSuccess(decodeRepositoryAccessJson(repositoryJson())).canWrite).toBe(false); - expect(expectSuccess(decodeRepositoryAccessJson(repositoryJson(null))).canWrite).toBe(false); + expect(expectSuccess(decodeViewerPermissionsJson(repositoryJson())).canWrite).toBe(false); + expect(expectSuccess(decodeViewerPermissionsJson(repositoryJson(null))).canWrite).toBe(false); }); }); describe("viewer permission decoding", () => { const viewerJson = (repository: Record) => - JSON.stringify({ data: { repository } }); + JSON.stringify({ + data: { + repository: { + mergeCommitAllowed: true, + squashMergeAllowed: false, + rebaseMergeAllowed: true, + ...repository, + }, + }, + }); it("reads the repository's role and the pull request's own viewer fields together", () => { expect( @@ -862,7 +877,13 @@ describe("viewer permission decoding", () => { }), ), ), - ).toEqual({ canWrite: false, canTriage: false, canUpdate: true, didAuthor: true }); + ).toEqual({ + mergeCapabilities: { merge: true, squash: false, rebase: true }, + canWrite: false, + canTriage: false, + canUpdate: true, + didAuthor: true, + }); }); it("says no to a passer-by on a repository they can only read", () => { @@ -875,7 +896,13 @@ describe("viewer permission decoding", () => { }), ), ), - ).toEqual({ canWrite: false, canTriage: false, canUpdate: false, didAuthor: false }); + ).toEqual({ + mergeCapabilities: { merge: true, squash: false, rebase: true }, + canWrite: false, + canTriage: false, + canUpdate: false, + didAuthor: false, + }); }); it("reads silence as permission, but not as authorship", () => { @@ -883,6 +910,7 @@ describe("viewer permission decoding", () => { // answer grants it and lets the host refuse; authorship is a fact about who wrote the change, // and claiming it for someone who did not is how an author's own rules get handed out. expect(expectSuccess(decodeViewerPermissionsJson(viewerJson({ pullRequest: null })))).toEqual({ + mergeCapabilities: { merge: true, squash: false, rebase: true }, canWrite: false, canTriage: false, canUpdate: true, diff --git a/apps/server/src/pullRequest/gitHubPullRequestJson.ts b/apps/server/src/pullRequest/gitHubPullRequestJson.ts index 6229c142a15983e7e2d61c18fbb1a2340e91d6d1..a011702d685e4a460f0dbbebb9f5b69f3fc4e244 100644 --- a/apps/server/src/pullRequest/gitHubPullRequestJson.ts +++ b/apps/server/src/pullRequest/gitHubPullRequestJson.ts @@ -5,6 +5,7 @@ import * as Schema from "effect/Schema"; import type { PullRequestStackMembership, PullRequestActor, + PullRequestPreview, PullRequestCheck, PullRequestCheckStatus, PullRequestChecksState, @@ -596,6 +597,62 @@ const RawRepositoryAccessSchema = Schema.Struct({ viewerPermission: Schema.optional(Schema.NullOr(Schema.String)), }); +const RawCoreSchema = Schema.Struct({ + data: Schema.Struct({ + repository: Schema.Struct({ + ...RawRepositoryAccessSchema.fields, + pullRequest: Schema.Struct({ + ...RawDetailSchema.fields, + ...RawViewerFieldsSchema.fields, + viewerCanUpdateBranch: Schema.Boolean, + baseRef: Schema.NullOr( + Schema.Struct({ + compare: Schema.NullOr(Schema.Struct({ behindBy: Schema.Int })), + }), + ), + reviewRequests: Schema.Struct({ + nodes: Schema.Array( + Schema.Struct({ requestedReviewer: Schema.NullOr(RawReviewRequestSchema) }), + ), + }), + labels: Schema.Struct({ nodes: Schema.Array(RawLabelSchema) }), + commits: Schema.Struct({ + nodes: Schema.Array( + Schema.Struct({ + commit: Schema.Struct({ + statusCheckRollup: Schema.NullOr( + Schema.Struct({ + contexts: Schema.Struct({ + nodes: Schema.Array( + Schema.Struct({ + ...RawCheckSchema.fields, + checkSuite: Schema.optional( + Schema.NullOr( + Schema.Struct({ + workflowRun: Schema.NullOr( + Schema.Struct({ + workflow: Schema.NullOr(Schema.Struct({ name: Schema.String })), + }), + ), + }), + ), + ), + }), + ), + pageInfo: Schema.Struct({ hasNextPage: Schema.Boolean }), + }), + }), + ), + }), + }), + ), + }), + }), + }), + }), +}); +const decodeCore = decodeJsonResult(RawCoreSchema); + const RawPullRequestFileSchema = Schema.Struct({ filename: Schema.String, status: Schema.optional(Schema.NullOr(Schema.String)), @@ -644,6 +701,78 @@ export const PULL_REQUEST_LIST_JSON_FIELDS = "number,title,url,author,headRefName,baseRefName,state,isDraft,mergeable,reviewDecision,additions,deletions,createdAt,updatedAt,mergedAt,reviewRequests,labels,statusCheckRollup"; export const PULL_REQUEST_DETAIL_JSON_FIELDS = `${PULL_REQUEST_LIST_JSON_FIELDS},body,changedFiles,closedAt,isCrossRepository,headRepositoryOwner,headRefOid,autoMergeRequest`; + +/** Pull refs let the comparison share the detail read without first resolving a fork branch. */ +export const PULL_REQUEST_CORE_GRAPHQL_QUERY = `query($owner: String!, $name: String!, $number: Int!, $headRef: String!) { + repository(owner: $owner, name: $name) { + mergeCommitAllowed squashMergeAllowed rebaseMergeAllowed viewerPermission + pullRequest(number: $number) { + number title url body state isDraft mergeable reviewDecision + additions deletions changedFiles createdAt updatedAt mergedAt closedAt + headRefName baseRefName headRefOid isCrossRepository + headRepositoryOwner { login } + author { login avatarUrl ... on User { id name } } + autoMergeRequest { mergeMethod } + viewerCanUpdate viewerDidAuthor viewerCanUpdateBranch + baseRef { compare(headRef: $headRef) { behindBy } } + reviewRequests(first: 100) { + nodes { requestedReviewer { ... on User { login name } ... on Bot { login } ... on Team { slug name } } } + } + labels(first: 100) { nodes { name color } } + commits(last: 1) { + nodes { commit { statusCheckRollup { contexts(first: 100) { + nodes { + __typename + ... on StatusContext { context state targetUrl createdAt description } + ... on CheckRun { + name status conclusion startedAt completedAt detailsUrl + checkSuite { workflowRun { workflow { name } } } + } + } + pageInfo { hasNextPage } + } } } } + } + } + } +}`; + +export const PULL_REQUEST_PREVIEW_GRAPHQL_QUERY = `query($owner: String!, $name: String!, $number: Int!) { + repository(owner: $owner, name: $name) { + pullRequest(number: $number) { + number title url state isDraft createdAt + author { login avatarUrl ... on User { name } } + } + } +}`; + +const decodePullRequestPreview = decodeJsonResult( + Schema.Struct({ + data: Schema.Struct({ + repository: Schema.Struct({ + pullRequest: Schema.Struct({ + number: Schema.Int, + title: Schema.String, + url: Schema.String, + state: Schema.String, + isDraft: Schema.Boolean, + createdAt: Schema.String, + author: Schema.NullOr(RawActorSchema), + }), + }), + }), + }), +); + +export function decodePullRequestPreviewJson( + raw: string, +): Result.Result, DecodeFailure> { + return Result.map(decodePullRequestPreview(raw), ({ data }) => ({ + ...data.repository.pullRequest, + author: toActor(data.repository.pullRequest.author), + state: toState(data.repository.pullRequest), + })); +} + export const PULL_REQUEST_ACTIVITY_JSON_FIELDS = "author,comments,reviews,commits"; /** GitHub's own ceiling on a connection page, which is what both thread reads ask for. */ @@ -1034,14 +1163,6 @@ export function buildReviewSubmissionJson(input: { }); } -/** - * `viewerPermission` rides along with the merge settings rather than being asked for on its own: - * `gh repo view --json` serves both out of the same GraphQL repository object, so the viewer's - * standing on the repository costs no request of its own. - */ -export const REPOSITORY_ACCESS_JSON_FIELDS = - "mergeCommitAllowed,squashMergeAllowed,rebaseMergeAllowed,viewerPermission"; - export interface GitHubPullRequestListItem { readonly stack?: PullRequestStackMembership; /** The author's node id, kept so a batch can resolve the avatar the listing does not carry. */ @@ -1489,7 +1610,6 @@ const decodeWorkflowRunApprovals = decodeJsonResult(Schema.Array(RawWorkflowRunA const decodePullRequestHeads = decodeJsonResult(Schema.Array(RawPullRequestHeadSchema)); const decodeActivity = decodeJsonResult(RawActivitySchema); const decodeFileEntry = Schema.decodeUnknownExit(RawPullRequestFileSchema); -const decodeRepositoryAccess = decodeJsonResult(RawRepositoryAccessSchema); const decodeReviewThreads = decodeJsonResult(RawReviewThreadsSchema); const decodeReviewThreadComments = decodeJsonResult(RawReviewThreadCommentsSchema); @@ -1689,6 +1809,54 @@ export function decodePullRequestStatsJson( return Result.succeed(stats); } +export interface GitHubPullRequestCore extends GitHubPullRequestDetail { + readonly viewerAccess: GitHubViewerAccess & GitHubRepositoryAccess; + readonly comparison: GitHubBaseComparison | null; + readonly checksTruncated: boolean; +} + +export function decodePullRequestCoreJson( + raw: string, +): Result.Result { + const decoded = decodeCore(raw); + if (!Result.isSuccess(decoded)) return Result.fail(decoded.failure); + const repository = decoded.success.data.repository; + const pr = repository.pullRequest; + const contexts = pr.commits.nodes[0]?.commit.statusCheckRollup?.contexts; + return Result.succeed({ + ...toDetail({ + ...pr, + reviewRequests: pr.reviewRequests.nodes.flatMap(({ requestedReviewer }) => + requestedReviewer === null ? [] : [requestedReviewer], + ), + labels: pr.labels.nodes, + statusCheckRollup: + contexts?.nodes.map((check) => ({ + ...check, + workflowName: check.checkSuite?.workflowRun?.workflow?.name ?? null, + })) ?? [], + }), + viewerAccess: { + canWrite: toCanWrite(repository.viewerPermission), + canTriage: toCanTriage(repository.viewerPermission), + ...toPullRequestViewerFields(pr), + mergeCapabilities: { + merge: repository.mergeCommitAllowed, + squash: repository.squashMergeAllowed, + rebase: repository.rebaseMergeAllowed, + }, + }, + comparison: + pr.state !== "OPEN" || pr.baseRef?.compare == null + ? null + : { + behindBy: pr.baseRef.compare.behindBy, + viewerCanUpdate: pr.viewerCanUpdateBranch, + }, + checksTruncated: contexts?.pageInfo.hasNextPage === true, + }); +} + export function decodePullRequestDetailJson( raw: string, ): Result.Result { @@ -2052,7 +2220,7 @@ export function decodeReviewThreadCommentsJson(raw: string): Result.Result< }); } -/** What one `gh repo view` answers: what the repository allows, and where the viewer stands. */ +/** Repository settings returned alongside the pull request viewer permissions. */ export interface GitHubRepositoryAccess { readonly mergeCapabilities: PullRequestMergeCapabilities; readonly canWrite: boolean; @@ -2084,22 +2252,6 @@ function toCanTriage(viewerPermission: string | null | undefined): boolean { return viewerPermission?.trim().toUpperCase() === "TRIAGE" || toCanWrite(viewerPermission); } -export function decodeRepositoryAccessJson( - raw: string, -): Result.Result { - const decoded = decodeRepositoryAccess(raw); - return Result.isSuccess(decoded) - ? Result.succeed({ - mergeCapabilities: { - merge: decoded.success.mergeCommitAllowed, - squash: decoded.success.squashMergeAllowed, - rebase: decoded.success.rebaseMergeAllowed, - }, - canWrite: toCanWrite(decoded.success.viewerPermission), - }) - : Result.fail(decoded.failure); -} - /** * Who a review may be asked of, and who it has already been asked of, in one read. * @@ -2428,15 +2580,10 @@ export interface GitHubViewerAccess { readonly canUpdateBranch?: boolean; } -/** - * The viewer's standing, asked on its own. Only the write path needs this: reading a pull request - * already carries the same three fields on calls it was making anyway, and this exists so that a - * merge or a close is decided by what GitHub says now rather than by what the page was told when - * it loaded. - */ +/** Core detail and write checks share one read of permissions and merge settings. */ export const VIEWER_PERMISSIONS_GRAPHQL_QUERY = `query($owner: String!, $name: String!, $number: Int!) { repository(owner: $owner, name: $name) { - viewerPermission + mergeCommitAllowed squashMergeAllowed rebaseMergeAllowed viewerPermission pullRequest(number: $number) { viewerCanUpdate viewerDidAuthor } } }`; @@ -2444,7 +2591,7 @@ export const VIEWER_PERMISSIONS_GRAPHQL_QUERY = `query($owner: String!, $name: S const RawViewerPermissionsSchema = Schema.Struct({ data: Schema.Struct({ repository: Schema.Struct({ - viewerPermission: Schema.optional(Schema.NullOr(Schema.String)), + ...RawRepositoryAccessSchema.fields, /** Null for a number that names no pull request the viewer can see. */ pullRequest: Schema.NullOr(RawViewerFieldsSchema), }), @@ -2455,13 +2602,18 @@ const decodeViewerPermissions = decodeJsonResult(RawViewerPermissionsSchema); export function decodeViewerPermissionsJson( raw: string, -): Result.Result { +): Result.Result { const decoded = decodeViewerPermissions(raw); if (!Result.isSuccess(decoded)) { return Result.fail(decoded.failure); } const repository = decoded.success.data.repository; return Result.succeed({ + mergeCapabilities: { + merge: repository.mergeCommitAllowed, + squash: repository.squashMergeAllowed, + rebase: repository.rebaseMergeAllowed, + }, canWrite: toCanWrite(repository.viewerPermission), canTriage: toCanTriage(repository.viewerPermission), ...toPullRequestViewerFields(repository.pullRequest), diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts index 802a0396d68f99f8027502258987fe37211e0ca1..0b9bd1136bf34b2cd5b719c9e0de188696ed5fd7 100644 --- a/apps/server/src/ws.ts +++ b/apps/server/src/ws.ts @@ -2729,6 +2729,12 @@ const makeWsRpcLayer = ( "rpc.aggregate": "pull-requests", }, ), + [WS_METHODS.pullRequestsPreview]: (input) => + observeRpcEffect( + WS_METHODS.pullRequestsPreview, + withPullRequestViewer(input, pullRequests.preview(input)), + { "rpc.aggregate": "pull-requests" }, + ), [WS_METHODS.pullRequestsActivity]: (input) => observeRpcEffect( WS_METHODS.pullRequestsActivity, diff --git a/apps/web/src/components/ChatMarkdown.tsx b/apps/web/src/components/ChatMarkdown.tsx index 9136900fe0f79483ed3ade55c56e9652374561b2..abebe4841cbd37f161b5241c13c71cedbe5ad899 100644 --- a/apps/web/src/components/ChatMarkdown.tsx +++ b/apps/web/src/components/ChatMarkdown.tsx @@ -2905,6 +2905,7 @@ const CHAT_MARKDOWN_COMPONENTS = { environmentId, input: { projectId: pullRequestProject.id, + host: pullRequestCandidate.authority ?? pullRequestCandidate.host, repository: sourceControlRepositorySelector(pullRequestProject.repositoryIdentity) ?? pullRequestCandidate.repository, diff --git a/apps/web/src/components/pullRequest/PullRequestLinkPreview.tsx b/apps/web/src/components/pullRequest/PullRequestLinkPreview.tsx index 36a4c8b15c1c7e9eb1567c05c07aae905b1ef425..bbdbd9e16f99558664d06049a3efa88c372f0b23 100644 --- a/apps/web/src/components/pullRequest/PullRequestLinkPreview.tsx +++ b/apps/web/src/components/pullRequest/PullRequestLinkPreview.tsx @@ -42,11 +42,10 @@ export function PullRequestLinkPreview({ }) : null, ); - const readDetail = useAtomQueryRunner(pullRequestEnvironment.detail, { + const readPreview = useAtomQueryRunner(pullRequestEnvironment.preview, { reportFailure: false, reportDefect: false, }); - const trigger = confirmBeforeOpen ? cloneElement(link, { onClick: (event) => { @@ -56,7 +55,7 @@ export function PullRequestLinkPreview({ if (resolvingClick) return; setOpen(false); setResolvingClick(true); - void readDetail({ environmentId: target.environmentId, input: target.input }) + void readPreview(target) .then(async (result) => { if (isAtomCommandInterrupted(result)) return; if (result._tag === "Success" && onOpenPullRequest(result.value.url)) return; @@ -84,36 +83,38 @@ export function PullRequestLinkPreview({ return ( - - {detail === null ? ( -

- {detailQuery.isPending ? "Loading pull request details…" : originalUrl} -

- ) : ( -
-
- {detail.repository} - #{detail.number} - · - {state === null ? null : ( - - - {state.label} - - )} -
-

- {detail.title} + {detail !== null || detailQuery.error !== null ? ( + + {detail === null ? ( +

+ {originalUrl}

-
- - {authorLabel} - · - opened {formatRelativeTimeLabel(detail.createdAt)} + ) : ( +
+
+ {detail.repository} + #{detail.number} + · + {state === null ? null : ( + + + {state.label} + + )} +
+

+ {detail.title} +

+
+ + {authorLabel} + · + opened {formatRelativeTimeLabel(detail.createdAt)} +
-
- )} - + )} + + ) : null} ); } diff --git a/knip.jsonc b/knip.jsonc index 0eae85a29acb7927b0f74d4116e3522c426c517e..a06e0885a2f8c18d9f177cf8c9b76171450fafa9 100644 --- a/knip.jsonc +++ b/knip.jsonc @@ -23,6 +23,7 @@ "src/claude-history-worker.ts!", "scripts/cli.ts", "scripts/evaluate-thread-titles.ts", + "scripts/measure-pr-preview.ts", "src/provider/testFixtures/*.mjs", ], // Keep the transitive Effect runtime pinned for standalone npm installs. diff --git a/packages/client-runtime/src/state/pullRequestRouting.ts b/packages/client-runtime/src/state/pullRequestRouting.ts index 64ccb9cdad898b9f2b7388072a73307414cd9931..af625bbad56e01013e9affde4f7d9de909cf8972 100644 --- a/packages/client-runtime/src/state/pullRequestRouting.ts +++ b/packages/client-runtime/src/state/pullRequestRouting.ts @@ -29,6 +29,7 @@ const reads = new Set([ WS_METHODS.pullRequestsSummary, WS_METHODS.pullRequestsStack, WS_METHODS.pullRequestsDetail, + WS_METHODS.pullRequestsPreview, WS_METHODS.pullRequestsActivity, WS_METHODS.pullRequestsThreadComments, WS_METHODS.pullRequestsDiffFileContents, diff --git a/packages/client-runtime/src/state/pullRequests.test.ts b/packages/client-runtime/src/state/pullRequests.test.ts index 6fa1141669af1e13503e3262830faba86e254402..75e6b38f4a2e5ec20184853fa3e75a1773e4530b 100644 --- a/packages/client-runtime/src/state/pullRequests.test.ts +++ b/packages/client-runtime/src/state/pullRequests.test.ts @@ -887,6 +887,84 @@ it.effect("keeps concurrent diff file reads on different hosts separate", () => ), ); +it.effect("keeps hover previews fresh after edits and turns", () => + Effect.scoped( + Effect.gen(function* () { + const refreshEvents = yield* PubSub.unbounded(); + let title = "Original title"; + let reads = 0; + const read = (input: unknown) => + Effect.sync(() => { + expect(input).toEqual(reference); + reads++; + return { ...reference, title }; + }); + const reference = { + projectId: ProjectId.make("project-1"), + repository: "acme/web", + number: 1, + host: "github.example.com", + }; + const client = { + [WS_METHODS.pullRequestsSubscribeRefreshes]: () => Stream.fromPubSub(refreshEvents), + [WS_METHODS.pullRequestsPreview]: read, + [WS_METHODS.pullRequestsDetail]: read, + [WS_METHODS.pullRequestsUpdate]: (input: { title: string }) => + Effect.sync(() => { + title = input.title; + }), + [WS_METHODS.pullRequestsRunAction]: () => + Effect.sync(() => { + title = "Closed"; + }), + [WS_METHODS.pullRequestsInvalidate]: () => Effect.void, + } as unknown as WsRpcProtocolClient; + const { atoms, registry } = yield* makeTestRuntime(client); + const target = { environmentId: TARGET.environmentId, input: reference }; + const preview = atoms.preview(target); + const unmount = registry.mount(preview); + yield* Effect.addFinalizer(() => Effect.sync(unmount)); + expect( + (yield* AtomRegistry.getResult(registry, preview, { suspendOnWaiting: true })).title, + ).toBe(title); + yield* Effect.promise(() => executeAtomQuery(registry, preview)); + expect(reads).toBe(1); + const edited = yield* Effect.promise(() => + atoms.update.run(registry, { ...target, input: { ...reference, title: "Edited" } }), + ); + expect(AsyncResult.isSuccess(edited)).toBe(true); + expect( + (yield* AtomRegistry.getResult(registry, preview, { suspendOnWaiting: true })).title, + ).toBe("Edited"); + yield* Effect.promise(() => + atoms.runAction.run(registry, { ...target, input: { ...reference, action: "close" } }), + ); + expect( + (yield* AtomRegistry.getResult(registry, preview, { suspendOnWaiting: true })).title, + ).toBe("Closed"); + title = "Refreshed"; + yield* Effect.promise(() => + atoms.invalidate.run(registry, { + environmentId: target.environmentId, + input: { reference }, + }), + ); + expect( + (yield* AtomRegistry.getResult(registry, preview, { suspendOnWaiting: true })).title, + ).toBe("Refreshed"); + const refreshed = Latch.makeUnsafe(); + const stop = registry.subscribe(preview, (result) => { + if (AsyncResult.isSuccess(result) && result.value.title === "After turn") + refreshed.openUnsafe(); + }); + yield* Effect.addFinalizer(() => Effect.sync(stop)); + title = "After turn"; + yield* PubSub.publish(refreshEvents, 1); + yield* refreshed.await; + }), + ), +); + it.effect("shares close, reopen, and merge with an untouched client's mounted PR readers", () => Effect.scoped( Effect.gen(function* () { diff --git a/packages/client-runtime/src/state/pullRequests.ts b/packages/client-runtime/src/state/pullRequests.ts index d8c000863ab135649fbb4fc3701e9d2840796dc0..34768ed3c4b591854d42af3b7ea870880027b035 100644 --- a/packages/client-runtime/src/state/pullRequests.ts +++ b/packages/client-runtime/src/state/pullRequests.ts @@ -176,6 +176,15 @@ export function createPullRequestEnvironmentAtoms( refreshTrigger: ({ environmentId }) => refreshes({ environmentId, input: {} }), }), ); + const preview = writableQueryFamily( + createEnvironmentRpcQueryAtomFamily(runtime, { + label: "environment-data:pull-requests:preview", + tag: WS_METHODS.pullRequestsPreview, + execute: (input) => routedRequest(WS_METHODS.pullRequestsPreview, input), + staleTimeMs: 60_000, + refreshTrigger: ({ environmentId }) => refreshes({ environmentId, input: {} }), + }), + ); const labelCandidates = writableQueryFamily( createEnvironmentRpcQueryAtomFamily(runtime, { label: "environment-data:pull-requests:label-candidates", @@ -221,6 +230,7 @@ export function createPullRequestEnvironmentAtoms( refreshTrigger: ({ environmentId }) => refreshes({ environmentId, input: {} }), }), detail, + preview, activity, threadComments: createEnvironmentRpcCommand(runtime, { label: "environment-data:pull-requests:thread-comments", @@ -295,6 +305,7 @@ export function createPullRequestEnvironmentAtoms( label: "environment-data:pull-requests:run-action", tag: WS_METHODS.pullRequestsRunAction, execute: (input) => routedRequest(WS_METHODS.pullRequestsRunAction, input), + onSuccess: (target, registry) => Effect.sync(() => registry.refresh(preview(target))), scheduler: commandScheduler, concurrency: serialPerEnvironment, }), @@ -302,6 +313,7 @@ export function createPullRequestEnvironmentAtoms( label: "environment-data:pull-requests:update", tag: WS_METHODS.pullRequestsUpdate, execute: (input) => routedRequest(WS_METHODS.pullRequestsUpdate, input), + onSuccess: (target, registry) => Effect.sync(() => registry.refresh(preview(target))), scheduler: commandScheduler, concurrency: serialPerEnvironment, }), @@ -485,6 +497,12 @@ export function createPullRequestEnvironmentAtoms( label: "environment-data:pull-requests:invalidate", tag: WS_METHODS.pullRequestsInvalidate, execute: (input) => routedRequest(WS_METHODS.pullRequestsInvalidate, input), + onSuccess: ({ environmentId, input }, registry) => + Effect.sync(() => { + if (input.reference !== undefined) { + registry.refresh(preview({ environmentId, input: input.reference })); + } + }), scheduler: commandScheduler, concurrency: serialPerEnvironment, }), diff --git a/packages/contracts/src/pullRequest.ts b/packages/contracts/src/pullRequest.ts index 41c9250f2c636a8f94d0b95ee30c1ba13f125d60..b146810a8aec9841e6d13984226f0599409848ff 100644 --- a/packages/contracts/src/pullRequest.ts +++ b/packages/contracts/src/pullRequest.ts @@ -703,6 +703,20 @@ export const PullRequestLinkedThreadsResult = Schema.Struct({ }); export type PullRequestLinkedThreadsResult = typeof PullRequestLinkedThreadsResult.Type; +/** The complete hover card, without checks, permissions, or branch comparisons. */ +export const PullRequestPreview = Schema.Struct({ + projectId: ProjectId, + repository: TrimmedNonEmptyString, + number: PositiveInt, + title: TrimmedNonEmptyString, + url: TrimmedNonEmptyString, + author: Schema.NullOr(PullRequestActor), + state: PullRequestState, + isDraft: Schema.Boolean, + createdAt: IsoDateTime, +}); +export type PullRequestPreview = typeof PullRequestPreview.Type; + /** * The small live shape a linked thread needs. Keeping it separate from detail means a sidebar * status check never loads permissions, repository settings, checks, or base comparison data. diff --git a/packages/contracts/src/rpc.ts b/packages/contracts/src/rpc.ts index dec42aac1530a2048e15a8d02b75763cf55e2742..41af6d9f1e9e69831b6920379efdea491633c871 100644 --- a/packages/contracts/src/rpc.ts +++ b/packages/contracts/src/rpc.ts @@ -111,6 +111,7 @@ import { PullRequestCommentInput, PullRequestCommentUpdateInput, PullRequestDetail, + PullRequestPreview, PullRequestDiffFileContentsInput, PullRequestDiffFileContentsResult, PullRequestFilesViewedResult, @@ -395,6 +396,7 @@ export const WS_METHODS = { pullRequestsStack: "pullRequests.stack", pullRequestsLinkedThreads: "pullRequests.linkedThreads", pullRequestsDetail: "pullRequests.detail", + pullRequestsPreview: "pullRequests.preview", pullRequestsActivity: "pullRequests.activity", pullRequestsThreadComments: "pullRequests.threadComments", pullRequestsDiffFileContents: "pullRequests.diffFileContents", @@ -737,6 +739,12 @@ const WsPullRequestsDetailRpc = Rpc.make(WS_METHODS.pullRequestsDetail, { error: PullRequestRpcError, }); +const WsPullRequestsPreviewRpc = Rpc.make(WS_METHODS.pullRequestsPreview, { + payload: PullRequestRef, + success: PullRequestPreview, + error: PullRequestRpcError, +}); + const WsPullRequestsActivityRpc = Rpc.make(WS_METHODS.pullRequestsActivity, { payload: PullRequestRef, success: PullRequestActivity, @@ -1418,6 +1426,7 @@ export const WsRpcGroup = RpcGroup.make( WsPullRequestsStackRpc, WsPullRequestsLinkedThreadsRpc, WsPullRequestsDetailRpc, + WsPullRequestsPreviewRpc, WsPullRequestsActivityRpc, WsPullRequestsThreadCommentsRpc, WsPullRequestsDiffFileContentsRpc,