| author | |
| committer | |
| log | 1eae9c2efec43b1d27e60d967001f5721c4a0429 |
| tree | 2306e4b3ca30fdc4c725965d1eb794f72f74b8bf |
| parent | 9ac8f33f1685d1d6e843b657829d0d7462a13813 |
| signature | Signed by PGP key B5690EEEBB952194 |
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>7 files changed, 452 insertions(+), 239 deletions(-)
apps/server/scripts/measure-pr-preview.ts+5-1| ... | ... | @@ -73,7 +73,11 @@ const measuredApi = Layer.effect( |
| 73 | 73 | Layer.provide(NodeServices.layer), |
| 74 | 74 | ); |
| 75 | 75 | |
| 76 | const services = GitHubPullRequestCli.layer.pipe(Layer.provideMerge(measuredApi)); | |
| 76 | const services = GitHubPullRequestCli.layer.pipe( | |
| 77 | Layer.provideMerge(measuredApi), | |
| 78 | Layer.provideMerge(VcsProcess.layer), | |
| 79 | Layer.provideMerge(NodeServices.layer), | |
| 80 | ); | |
| 77 | 81 | |
| 78 | 82 | const encodeJson = Schema.encodeSync(Schema.fromJsonString(Schema.Unknown)); |
| 79 | 83 |
apps/server/src/pullRequest/GitHubPullRequestCli.test.ts+5| ... | ... | @@ -1,3 +1,4 @@ |
| 1 | import * as NodeServices from "@effect/platform-node/NodeServices"; | |
| 1 | 2 | import * as NodeCrypto from "@effect/platform-node/NodeCrypto"; |
| 2 | 3 | import { afterEach, assert, expect, it, vi } from "@effect/vitest"; |
| 3 | 4 | import * as Effect from "effect/Effect"; |
| ... | ... | @@ -16,6 +17,7 @@ import * as GitHubApi from "../sourceControl/GitHubApi.ts"; |
| 16 | 17 | import * as GitHubCredentials from "../sourceControl/GitHubCredentials.ts"; |
| 17 | 18 | import * as GitHubGraphQlBudget from "../sourceControl/githubGraphQlBudget.ts"; |
| 18 | 19 | import * as SourceControlRateLimit from "../sourceControl/SourceControlRateLimit.ts"; |
| 20 | import * as VcsProcess from "../vcs/VcsProcess.ts"; | |
| 19 | 21 | import * as GitHubPullRequestCli from "./GitHubPullRequestCli.ts"; |
| 20 | 22 | import { KnownWorkflowRuns } from "./gitHubConditionalChecks.ts"; |
| 21 | 23 | |
| ... | ... | @@ -98,6 +100,7 @@ const layer = it.layer( |
| 98 | 100 | Layer.provideMerge(mockApi), |
| 99 | 101 | Layer.provideMerge(GitHubGraphQlBudget.layer), |
| 100 | 102 | Layer.provide(NodeCrypto.layer), |
| 103 | Layer.provide(VcsProcess.layer.pipe(Layer.provideMerge(NodeServices.layer))), | |
| 101 | 104 | ), |
| 102 | 105 | ); |
| 103 | 106 | |
| ... | ... | @@ -492,6 +495,8 @@ it.effect( |
| 492 | 495 | Layer.provide(Layer.mergeAll(credentials, http)), |
| 493 | 496 | Layer.provide(GitHubGraphQlBudget.layer), |
| 494 | 497 | Layer.provide(SourceControlRateLimit.layer), |
| 498 | Layer.merge(VcsProcess.layer), | |
| 499 | Layer.provideMerge(NodeServices.layer), | |
| 495 | 500 | ), |
| 496 | 501 | ), |
| 497 | 502 | ); |
apps/server/src/pullRequest/GitHubPullRequestCli.ts+6| ... | ... | @@ -6,6 +6,7 @@ import * as Context from "effect/Context"; |
| 6 | 6 | import * as Clock from "effect/Clock"; |
| 7 | 7 | import * as Duration from "effect/Duration"; |
| 8 | 8 | import * as Effect from "effect/Effect"; |
| 9 | import * as FileSystem from "effect/FileSystem"; | |
| 9 | 10 | import * as Layer from "effect/Layer"; |
| 10 | 11 | import * as Exit from "effect/Exit"; |
| 11 | 12 | import * as Option from "effect/Option"; |
| ... | ... | @@ -43,6 +44,7 @@ import { |
| 43 | 44 | |
| 44 | 45 | import { AllowGitHubReserve } from "../sourceControl/GitHubCli.ts"; |
| 45 | 46 | import * as GitHubApi from "../sourceControl/GitHubApi.ts"; |
| 47 | import * as VcsProcess from "../vcs/VcsProcess.ts"; | |
| 46 | 48 | import * as SourceControlRateLimit from "../sourceControl/SourceControlRateLimit.ts"; |
| 47 | 49 | import { |
| 48 | 50 | ACTOR_AVATARS_GRAPHQL_QUERY, |
| ... | ... | @@ -1113,6 +1115,8 @@ const SIMPLE_ACTION_MUTATIONS = { |
| 1113 | 1115 | /** @public Service construction is part of the canonical Effect module API. */ |
| 1114 | 1116 | export const make = Effect.gen(function* () { |
| 1115 | 1117 | const api = yield* GitHubApi.GitHubApi; |
| 1118 | const vcsProcess = yield* VcsProcess.VcsProcess; | |
| 1119 | const fileSystem = yield* FileSystem.FileSystem; | |
| 1116 | 1120 | const revalidateChecks = yield* makeChecksRevalidator; |
| 1117 | 1121 | const routingIdentities = new Map< |
| 1118 | 1122 | string, |
| ... | ... | @@ -2495,6 +2499,8 @@ export const make = Effect.gen(function* () { |
| 2495 | 2499 | if (input.stackNumber !== undefined) |
| 2496 | 2500 | return runGitHubStackAction({ ...input, stackNumber: input.stackNumber }).pipe( |
| 2497 | 2501 | Effect.provideService(GitHubApi.GitHubApi, api), |
| 2502 | Effect.provideService(VcsProcess.VcsProcess, vcsProcess), | |
| 2503 | Effect.provideService(FileSystem.FileSystem, fileSystem), | |
| 2498 | 2504 | ); |
| 2499 | 2505 | if (input.action === "revert") { |
| 2500 | 2506 | return pullRequestNodeId({ ...input, operation: "revertPullRequest" }).pipe( |
apps/server/src/pullRequest/githubStackActions.test.ts+89-135| ... | ... | @@ -4,7 +4,11 @@ import * as Effect from "effect/Effect"; |
| 4 | 4 | import * as Fiber from "effect/Fiber"; |
| 5 | 5 | import * as Schema from "effect/Schema"; |
| 6 | 6 | import * as TestClock from "effect/testing/TestClock"; |
| 7 | import * as FileSystem from "effect/FileSystem"; | |
| 8 | import * as Redacted from "effect/Redacted"; | |
| 9 | import { ChildProcessSpawner } from "effect/process"; | |
| 7 | 10 | import * as GitHubApi from "../sourceControl/GitHubApi.ts"; |
| 11 | import * as VcsProcess from "../vcs/VcsProcess.ts"; | |
| 8 | 12 | import { runGitHubStackAction as runStackAction } from "./githubStackActions.ts"; |
| 9 | 13 | |
| 10 | 14 | /** |
| ... | ... | @@ -17,32 +21,57 @@ type Send = ( |
| 17 | 21 | kind: "graphql" | "rest", |
| 18 | 22 | ) => Effect.Effect<string, GitHubApi.GitHubApiError>; |
| 19 | 23 | |
| 24 | /** Every git command the cascade ran, in order; none actually runs. */ | |
| 25 | let gitCalls: Array<ReadonlyArray<string>> = []; | |
| 26 | const fakeGit = Layer.mergeAll( | |
| 27 | Layer.mock(VcsProcess.VcsProcess)({ | |
| 28 | run: (input) => | |
| 29 | Effect.sync(() => { | |
| 30 | gitCalls.push(input.args); | |
| 31 | const stdout = | |
| 32 | input.args[0] === "rev-parse" || input.args[0] === "merge-base" ? "new-sha\n" : ""; | |
| 33 | return { | |
| 34 | exitCode: ChildProcessSpawner.ExitCode(0), | |
| 35 | stdout, | |
| 36 | stderr: "", | |
| 37 | stdoutTruncated: false, | |
| 38 | stderrTruncated: false, | |
| 39 | }; | |
| 40 | }), | |
| 41 | }), | |
| 42 | FileSystem.layerNoop({ makeTempDirectoryScoped: () => Effect.succeed("/tmp/scratch") }), | |
| 43 | ); | |
| 44 | ||
| 20 | 45 | const runGitHubStackAction = (send: Send, input: Parameters<typeof runStackAction>[0]) => |
| 21 | 46 | runStackAction(input).pipe( |
| 22 | 47 | Effect.provide( |
| 23 | Layer.mock(GitHubApi.GitHubApi)({ | |
| 24 | graphql: (request) => { | |
| 25 | // GitHub refuses a document that declares a variable it never uses. | |
| 26 | const declared = [...request.query.matchAll(/\$(\w+)\s*:/g)].map((match) => match[1]!); | |
| 27 | const unused = declared.filter( | |
| 28 | (name) => !new RegExp(`\\$${name}(?!\\w)(?!\\s*:)`).test(request.query), | |
| 29 | ); | |
| 30 | if (unused.length > 0) { | |
| 31 | return Effect.die(new Error(`Variables declared but not used: ${unused.join(", ")}`)); | |
| 32 | } | |
| 33 | return send(words(request.query, request.variables), "graphql"); | |
| 34 | }, | |
| 35 | rest: (request) => | |
| 36 | send(words(request.path, request.body), "rest").pipe( | |
| 37 | Effect.map((body) => ({ | |
| 38 | status: 200, | |
| 39 | headers: {}, | |
| 40 | body, | |
| 41 | truncated: false, | |
| 42 | invalidUtf8: false, | |
| 43 | })), | |
| 44 | ), | |
| 45 | }), | |
| 48 | Layer.mergeAll( | |
| 49 | fakeGit, | |
| 50 | Layer.mock(GitHubApi.GitHubApi)({ | |
| 51 | credential: () => Effect.succeed({ token: Redacted.make("token"), fingerprint: "fp" }), | |
| 52 | graphql: (request) => { | |
| 53 | // GitHub refuses a document that declares a variable it never uses. | |
| 54 | const declared = [...request.query.matchAll(/\$(\w+)\s*:/g)].map((match) => match[1]!); | |
| 55 | const unused = declared.filter( | |
| 56 | (name) => !new RegExp(`\\$${name}(?!\\w)(?!\\s*:)`).test(request.query), | |
| 57 | ); | |
| 58 | if (unused.length > 0) { | |
| 59 | return Effect.die(new Error(`Variables declared but not used: ${unused.join(", ")}`)); | |
| 60 | } | |
| 61 | return send(words(request.query, request.variables), "graphql"); | |
| 62 | }, | |
| 63 | rest: (request) => | |
| 64 | send(words(request.path, request.body), "rest").pipe( | |
| 65 | Effect.map((body) => ({ | |
| 66 | status: 200, | |
| 67 | headers: {}, | |
| 68 | body, | |
| 69 | truncated: false, | |
| 70 | invalidUtf8: false, | |
| 71 | })), | |
| 72 | ), | |
| 73 | }), | |
| 74 | ), | |
| 46 | 75 | ), |
| 47 | 76 | ); |
| 48 | 77 | |
| ... | ... | @@ -250,41 +279,6 @@ it.effect("refuses a changed stack before performing any mutation", () => |
| 250 | 279 | }), |
| 251 | 280 | ); |
| 252 | 281 | |
| 253 | it.effect("rebases unmerged layers bottom to top without local git commands", () => | |
| 254 | Effect.gen(function* () { | |
| 255 | const api = fake([stack, access, ...rebaseResponses]); | |
| 256 | yield* runGitHubStackAction(api.execute, { ...input, action: "update-branch" }); | |
| 257 | const mutations = api.calls.filter(isMutation); | |
| 258 | expect(mutations).toHaveLength(2); | |
| 259 | expect(mutations[0]).toContain("id=PR_2"); | |
| 260 | expect(mutations[0]).toContain("sha=bbb"); | |
| 261 | expect(mutations[1]).toContain("id=PR_3"); | |
| 262 | expect(mutations[1]).toContain("sha=ccc"); | |
| 263 | }), | |
| 264 | ); | |
| 265 | ||
| 266 | it.effect("does not update later layers after a rebase failure", () => | |
| 267 | Effect.gen(function* () { | |
| 268 | const api = fake([stack, access, branch(2, "bbb")]); | |
| 269 | const execute: Send = (call, kind) => | |
| 270 | !isMutation(call) | |
| 271 | ? api.execute(call, kind) | |
| 272 | : Effect.fail( | |
| 273 | new GitHubApi.GitHubApiAuthenticationError({ | |
| 274 | host: "github.com", | |
| 275 | operation: "runGitHubStackAction", | |
| 276 | }), | |
| 277 | ); | |
| 278 | const result = yield* runGitHubStackAction(execute, { ...input, action: "update-branch" }).pipe( | |
| 279 | Effect.result, | |
| 280 | ); | |
| 281 | expect(result).toMatchObject({ | |
| 282 | _tag: "Failure", | |
| 283 | failure: { _tag: "GitHubStackRebaseFailedError", number: 2, completed: 0 }, | |
| 284 | }); | |
| 285 | }), | |
| 286 | ); | |
| 287 | ||
| 288 | 282 | it.effect("refuses the entire rebase before mutation when a later fork denies write access", () => |
| 289 | 283 | Effect.gen(function* () { |
| 290 | 284 | const api = fake([ |
| ... | ... | @@ -325,7 +319,7 @@ it.effect("allows a fork that explicitly permits maintainer updates", () => |
| 325 | 319 | ...rebaseResponses, |
| 326 | 320 | ]); |
| 327 | 321 | yield* runGitHubStackAction(api.execute, { ...input, action: "update-branch" }); |
| 328 | expect(api.calls.at(-1)).toContain("id=PR_3"); | |
| 322 | expect(gitCalls.filter((args) => args[0] === "push")).toHaveLength(2); | |
| 329 | 323 | }), |
| 330 | 324 | ); |
| 331 | 325 | |
| ... | ... | @@ -348,92 +342,52 @@ it.effect("bounds polling and reports a still-running merge without claiming suc |
| 348 | 342 | }), |
| 349 | 343 | ); |
| 350 | 344 | |
| 351 | it.effect("rejects a push after preflight without rebasing the new revision", () => | |
| 345 | it.effect("rebases the open layers bottom to top in a scratch clone, each onto the one below", () => | |
| 352 | 346 | Effect.gen(function* () { |
| 353 | const api = fake([stack, access, branch(2, "new-head")]); | |
| 354 | const result = yield* runGitHubStackAction(api.execute, { | |
| 355 | ...input, | |
| 356 | action: "update-branch", | |
| 357 | }).pipe(Effect.result); | |
| 358 | expect(result).toMatchObject({ | |
| 359 | _tag: "Failure", | |
| 360 | failure: { _tag: "GitHubStackChangedError", number: 2, completed: 0 }, | |
| 361 | }); | |
| 362 | expect(api.calls).toHaveLength(3); | |
| 363 | }), | |
| 364 | ); | |
| 365 | ||
| 366 | it.effect("skips current layers without submitting a rebase mutation", () => | |
| 367 | Effect.gen(function* () { | |
| 368 | const api = fake([stack, access, branch(2, "bbb", 0), branch(3, "ccc", 0, ["bbb"])]); | |
| 347 | gitCalls = []; | |
| 348 | const api = fake([stack, access]); | |
| 369 | 349 | yield* runGitHubStackAction(api.execute, { ...input, action: "update-branch" }); |
| 370 | expect(api.calls.some((args) => isMutation(args))).toBe(false); | |
| 371 | }), | |
| 372 | ); | |
| 373 | ||
| 374 | it.effect("keeps earlier progress and stops after a later layer fails", () => | |
| 375 | Effect.gen(function* () { | |
| 376 | const api = fake([ | |
| 377 | stack, | |
| 378 | access, | |
| 379 | branch(2, "bbb"), | |
| 380 | rebased, | |
| 381 | branch(3, "ccc", 1, ["rebased-sha"]), | |
| 382 | { data: { updatePullRequestBranch: null } }, | |
| 350 | // GitHub is only read; every change is a git push with a lease on the reviewed head. | |
| 351 | expect(api.calls.some(isMutation)).toBe(false); | |
| 352 | const rebases = gitCalls.filter((args) => args[0] === "rebase"); | |
| 353 | expect(rebases).toEqual([ | |
| 354 | ["rebase", "--quiet", "--onto", "origin/main", "new-sha"], | |
| 355 | ["rebase", "--quiet", "--onto", "new-sha", "bbb"], | |
| 383 | 356 | ]); |
| 384 | const result = yield* runGitHubStackAction(api.execute, { | |
| 385 | ...input, | |
| 386 | action: "update-branch", | |
| 387 | }).pipe(Effect.result); | |
| 388 | expect(result).toMatchObject({ | |
| 389 | _tag: "Failure", | |
| 390 | failure: { _tag: "GitHubStackRebaseFailedError", number: 3, completed: 1 }, | |
| 391 | }); | |
| 392 | }), | |
| 393 | ); | |
| 394 | ||
| 395 | it.effect("reports partial progress when a later head changes during the rebase", () => | |
| 396 | Effect.gen(function* () { | |
| 397 | const api = fake([ | |
| 398 | stack, | |
| 399 | access, | |
| 400 | branch(2, "bbb"), | |
| 401 | rebased, | |
| 402 | branch(3, "concurrent-head", 1, ["rebased-sha"]), | |
| 357 | expect(gitCalls.filter((args) => args[0] === "push")).toEqual([ | |
| 358 | [ | |
| 359 | "push", | |
| 360 | "--quiet", | |
| 361 | "--force-with-lease=refs/heads/middle:bbb", | |
| 362 | "origin", | |
| 363 | "new-sha:refs/heads/middle", | |
| 364 | ], | |
| 365 | [ | |
| 366 | "push", | |
| 367 | "--quiet", | |
| 368 | "--force-with-lease=refs/heads/top:ccc", | |
| 369 | "origin", | |
| 370 | "new-sha:refs/heads/top", | |
| 371 | ], | |
| 403 | 372 | ]); |
| 404 | const result = yield* runGitHubStackAction(api.execute, { | |
| 405 | ...input, | |
| 406 | action: "update-branch", | |
| 407 | }).pipe(Effect.result); | |
| 408 | expect(result).toMatchObject({ | |
| 409 | _tag: "Failure", | |
| 410 | failure: { _tag: "GitHubStackChangedError", number: 3, completed: 1 }, | |
| 411 | }); | |
| 412 | if (result._tag === "Failure") { | |
| 413 | expect(result.failure.message).toContain("Earlier updates remain on GitHub"); | |
| 414 | } | |
| 415 | expect(api.calls.filter((args) => isMutation(args))).toHaveLength(1); | |
| 416 | 373 | }), |
| 417 | 374 | ); |
| 418 | 375 | |
| 419 | it.effect.each([false, true])("rejects a push to a processed layer, rebased=%s", (rebasedParent) => | |
| 376 | it.effect("checks every layer's write access before any git runs", () => | |
| 420 | 377 | Effect.gen(function* () { |
| 378 | gitCalls = []; | |
| 421 | 379 | const api = fake([ |
| 422 | 380 | stack, |
| 423 | access, | |
| 424 | branch(2, "bbb", rebasedParent ? 1 : 0), | |
| 425 | ...(rebasedParent ? [rebased] : []), | |
| 426 | branch(3, "ccc", 1, ["concurrent-parent-head"]), | |
| 381 | { | |
| 382 | data: { | |
| 383 | repository: { | |
| 384 | ...access.data.repository, | |
| 385 | pr3: { headRepository: { viewerPermission: "READ" }, maintainerCanModify: false }, | |
| 386 | }, | |
| 387 | }, | |
| 388 | }, | |
| 427 | 389 | ]); |
| 428 | const result = yield* runGitHubStackAction(api.execute, { | |
| 429 | ...input, | |
| 430 | action: "update-branch", | |
| 431 | }).pipe(Effect.result); | |
| 432 | expect(result).toMatchObject({ | |
| 433 | _tag: "Failure", | |
| 434 | failure: { _tag: "GitHubStackChangedError", number: 2, completed: 1 }, | |
| 435 | }); | |
| 436 | expect(api.calls.at(-1)?.includes('ids=["PR_2"]')).toBe(true); | |
| 437 | expect(api.calls.filter((args) => isMutation(args))).toHaveLength(rebasedParent ? 1 : 0); | |
| 390 | yield* Effect.flip(runGitHubStackAction(api.execute, { ...input, action: "update-branch" })); | |
| 391 | expect(gitCalls).toEqual([]); | |
| 438 | 392 | }), |
| 439 | 393 | ); |
apps/server/src/pullRequest/githubStackActions.ts+30-103| ... | ... | @@ -10,6 +10,7 @@ import * as Schema from "effect/Schema"; |
| 10 | 10 | |
| 11 | 11 | import * as GitHubApi from "../sourceControl/GitHubApi.ts"; |
| 12 | 12 | import { decodePullRequestStacksJson } from "./gitHubPullRequestJson.ts"; |
| 13 | import { cascadeRebaseStack } from "./githubStackRebase.ts"; | |
| 13 | 14 | |
| 14 | 15 | const stackErrorIdentity = { |
| 15 | 16 | repository: Schema.String, |
| ... | ... | @@ -78,7 +79,10 @@ export class GitHubStackRebaseFailedError extends Schema.TaggedError<GitHubStack |
| 78 | 79 | { ...stackErrorIdentity, completed: Schema.Int, cause: Schema.Defect() }, |
| 79 | 80 | ) { |
| 80 | 81 | override get message(): string { |
| 81 | return `Stack rebase stopped at PR #${this.number} after ${this.completed} layers. Earlier updates remain on GitHub; resolve the failing layer before retrying.`; | |
| 82 | // A conflict or a refused push already says which layer and what to do about it. | |
| 83 | return this.cause instanceof Error && this.cause.message !== "" | |
| 84 | ? this.cause.message | |
| 85 | : `Stack rebase stopped at PR #${this.number} after ${this.completed} layers. Earlier updates remain on GitHub; resolve the failing layer before retrying.`; | |
| 82 | 86 | } |
| 83 | 87 | } |
| 84 | 88 | |
| ... | ... | @@ -121,39 +125,9 @@ const decodeBranchAccess = Schema.decodeEffect( |
| 121 | 125 | ), |
| 122 | 126 | ); |
| 123 | 127 | |
| 124 | const decodeRebaseBranch = Schema.decodeEffect( | |
| 125 | Schema.fromJsonString( | |
| 126 | Schema.Struct({ | |
| 127 | data: Schema.Struct({ | |
| 128 | processed: Schema.optional( | |
| 129 | Schema.Array(Schema.NullOr(Schema.Struct({ headRefOid: Schema.String }))), | |
| 130 | ), | |
| 131 | repository: Schema.Struct({ | |
| 132 | pullRequest: Schema.Struct({ | |
| 133 | id: Schema.String, | |
| 134 | headRefOid: Schema.String, | |
| 135 | baseRef: Schema.Struct({ compare: Schema.Struct({ behindBy: Schema.Int }) }), | |
| 136 | }), | |
| 137 | }), | |
| 138 | }), | |
| 139 | }), | |
| 140 | ), | |
| 141 | ); | |
| 142 | const decodeRebaseResponse = Schema.decodeEffect( | |
| 143 | Schema.fromJsonString( | |
| 144 | Schema.Struct({ | |
| 145 | data: Schema.Struct({ | |
| 146 | updatePullRequestBranch: Schema.Struct({ | |
| 147 | pullRequest: Schema.Struct({ headRefOid: Schema.String }), | |
| 148 | }), | |
| 149 | }), | |
| 150 | }), | |
| 151 | ), | |
| 152 | ); | |
| 153 | ||
| 154 | 128 | const decodeMergeResponse = Schema.decodeEffect(Schema.fromJsonString(MergeResponse)); |
| 155 | 129 | |
| 156 | /** Remote-only updates: a stack rebase never switches or rewrites the environment's checkout. */ | |
| 130 | /** Remote-only updates: a stack rebase works in a scratch clone, never the environment's checkout. */ | |
| 157 | 131 | export const runGitHubStackAction = Effect.fn("runGitHubStackAction")(function* (input: { |
| 158 | 132 | cwd: string; |
| 159 | 133 | repository: string; |
| ... | ... | @@ -242,77 +216,30 @@ export const runGitHubStackAction = Effect.fn("runGitHubStackAction")(function* |
| 242 | 216 | }) |
| 243 | 217 | ) |
| 244 | 218 | return yield* new GitHubStackPermissionError({ ...identity }); |
| 245 | const processed: Array<{ id: string; number: number; headSha: string }> = []; | |
| 246 | for (const [index, layer] of open.entries()) { | |
| 247 | yield* Effect.gen(function* () { | |
| 248 | const read = yield* api.graphql({ | |
| 249 | host: input.host, | |
| 250 | operation: "runGitHubStackAction", | |
| 251 | allowReserve: true, | |
| 252 | variables: { | |
| 253 | owner, | |
| 254 | name, | |
| 255 | number: layer.number, | |
| 256 | sha: layer.headSha, | |
| 257 | ids: processed.map((head) => head.id), | |
| 258 | }, | |
| 259 | // GitHub rejects a declared variable the document never uses, so `$ids` is always | |
| 260 | // selected; an empty list asks for nothing. | |
| 261 | query: `query($owner:String!,$name:String!,$number:Int!,$sha:String!,$ids:[ID!]!){processed:nodes(ids:$ids){... on PullRequest{headRefOid}} repository(owner:$owner,name:$name){pullRequest(number:$number){id headRefOid baseRef{compare(headRef:$sha){behindBy}}}}}`, | |
| 262 | }); | |
| 263 | const { | |
| 264 | data: { | |
| 265 | processed: observed, | |
| 266 | repository: { pullRequest: pr }, | |
| 267 | }, | |
| 268 | } = yield* decodeRebaseBranch(read); | |
| 269 | // A push to an earlier layer must not silently become the next layer's new base. | |
| 270 | const changed = processed.find( | |
| 271 | (head, index) => observed?.[index]?.headRefOid !== head.headSha, | |
| 272 | ); | |
| 273 | if (changed !== undefined) | |
| 274 | return yield* new GitHubStackChangedError({ | |
| 275 | ...identity, | |
| 276 | number: changed.number, | |
| 277 | completed: index, | |
| 278 | }); | |
| 279 | if (pr.headRefOid !== layer.headSha) | |
| 280 | return yield* new GitHubStackChangedError({ | |
| 281 | ...identity, | |
| 282 | number: layer.number, | |
| 283 | completed: index, | |
| 284 | }); | |
| 285 | if (pr.baseRef.compare.behindBy === 0) { | |
| 286 | processed.push({ id: pr.id, number: layer.number, headSha: pr.headRefOid }); | |
| 287 | return; | |
| 288 | } | |
| 289 | // Pass the reviewed revision to GitHub, including when a push races this read. | |
| 290 | const updated = yield* api.graphql({ | |
| 291 | host: input.host, | |
| 292 | operation: "runGitHubStackAction", | |
| 293 | variables: { id: pr.id, sha: layer.headSha }, | |
| 294 | query: | |
| 295 | "mutation($id:ID!,$sha:GitObjectID!){updatePullRequestBranch(input:{pullRequestId:$id,expectedHeadOid:$sha,updateMethod:REBASE}){pullRequest{headRefOid}}}", | |
| 296 | }); | |
| 297 | const response = yield* decodeRebaseResponse(updated); | |
| 298 | processed.push({ | |
| 299 | id: pr.id, | |
| 300 | number: layer.number, | |
| 301 | headSha: response.data.updatePullRequestBranch.pullRequest.headRefOid, | |
| 302 | }); | |
| 303 | }).pipe( | |
| 304 | Effect.mapError((cause) => | |
| 305 | cause._tag === "GitHubStackChangedError" | |
| 306 | ? cause | |
| 307 | : new GitHubStackRebaseFailedError({ | |
| 308 | ...identity, | |
| 309 | number: layer.number, | |
| 310 | completed: index, | |
| 311 | cause, | |
| 312 | }), | |
| 313 | ), | |
| 314 | ); | |
| 315 | } | |
| 219 | // GitHub's own "Rebase stack" has no API, and its per-PR "update branch" replays the old | |
| 220 | // copy of every lower layer into the one above it. The cascade moves each layer's own commits. | |
| 221 | yield* cascadeRebaseStack({ | |
| 222 | host: input.host, | |
| 223 | repository: input.repository, | |
| 224 | base: stack.base, | |
| 225 | layers: open.map((layer) => ({ | |
| 226 | number: layer.number, | |
| 227 | headBranch: layer.headBranch, | |
| 228 | headSha: layer.headSha!, | |
| 229 | })), | |
| 230 | }).pipe( | |
| 231 | Effect.mapError((cause) => | |
| 232 | cause._tag === "GitHubStackRebaseConflictError" || | |
| 233 | cause._tag === "GitHubStackRebaseGitError" | |
| 234 | ? new GitHubStackRebaseFailedError({ | |
| 235 | ...identity, | |
| 236 | number: cause.number, | |
| 237 | completed: cause.completed, | |
| 238 | cause, | |
| 239 | }) | |
| 240 | : cause, | |
| 241 | ), | |
| 242 | ); | |
| 316 | 243 | return; |
| 317 | 244 | } |
| 318 | 245 | if (open.some((layer) => layer.isDraft)) |
apps/server/src/pullRequest/githubStackRebase.test.ts created+162| ... | ... | @@ -0,0 +1,162 @@ |
| 1 | import * as NodeServices from "@effect/platform-node/NodeServices"; | |
| 2 | import { assert, expect, it } from "@effect/vitest"; | |
| 3 | import * as Effect from "effect/Effect"; | |
| 4 | import * as FileSystem from "effect/FileSystem"; | |
| 5 | import * as Layer from "effect/Layer"; | |
| 6 | import * as Redacted from "effect/Redacted"; | |
| 7 | ||
| 8 | import * as GitHubApi from "../sourceControl/GitHubApi.ts"; | |
| 9 | import * as VcsProcess from "../vcs/VcsProcess.ts"; | |
| 10 | import { cascadeRebaseStack } from "./githubStackRebase.ts"; | |
| 11 | ||
| 12 | const layer = Layer.mergeAll( | |
| 13 | Layer.mock(GitHubApi.GitHubApi)({ | |
| 14 | credential: () => Effect.succeed({ token: Redacted.make("token"), fingerprint: "fp" }), | |
| 15 | }), | |
| 16 | VcsProcess.layer, | |
| 17 | ).pipe(Layer.provideMerge(NodeServices.layer)); | |
| 18 | ||
| 19 | /** | |
| 20 | * A bare "GitHub" holding the live-run stack: `main` moved ahead after the stack was cut, and a | |
| 21 | * second layer sits on the first. Returns each branch's head. | |
| 22 | */ | |
| 23 | const setup = Effect.gen(function* () { | |
| 24 | const fs = yield* FileSystem.FileSystem; | |
| 25 | const process = yield* VcsProcess.VcsProcess; | |
| 26 | const root = yield* fs.makeTempDirectoryScoped({ prefix: "t3-cascade-test-" }); | |
| 27 | const remote = `${root}/remote.git`; | |
| 28 | const work = `${root}/work`; | |
| 29 | const git = (cwd: string, ...args: string[]) => | |
| 30 | process | |
| 31 | .run({ | |
| 32 | operation: "test", | |
| 33 | command: "git", | |
| 34 | args, | |
| 35 | cwd, | |
| 36 | env: { | |
| 37 | GIT_AUTHOR_NAME: "t", | |
| 38 | GIT_AUTHOR_EMAIL: "t@t", | |
| 39 | GIT_COMMITTER_NAME: "t", | |
| 40 | GIT_COMMITTER_EMAIL: "t@t", | |
| 41 | }, | |
| 42 | }) | |
| 43 | .pipe(Effect.map((out) => out.stdout.trim())); | |
| 44 | const write = (path: string, text: string) => fs.writeFileString(`${work}/${path}`, text); | |
| 45 | yield* git(root, "init", "--quiet", "--bare", "-b", "main", remote); | |
| 46 | yield* git(root, "clone", "--quiet", remote, work); | |
| 47 | yield* write("index.ts", "export const answer = 41;\n"); | |
| 48 | yield* git(work, "add", "-A"); | |
| 49 | yield* git(work, "commit", "--quiet", "-m", "Initial"); | |
| 50 | yield* git(work, "switch", "--quiet", "-c", "feat/answer-42"); | |
| 51 | yield* write("index.ts", "export const answer = 42;\n"); | |
| 52 | yield* git(work, "commit", "--quiet", "-am", "A: set the answer to 42"); | |
| 53 | yield* git(work, "switch", "--quiet", "-c", "feat/answer-doc"); | |
| 54 | yield* write("index.ts", "/** The answer. */\nexport const answer = 42;\n"); | |
| 55 | yield* git(work, "commit", "--quiet", "-am", "B: document the answer"); | |
| 56 | yield* git(work, "switch", "--quiet", "main"); | |
| 57 | yield* write("README.md", "main moved ahead\n"); | |
| 58 | yield* git(work, "add", "-A"); | |
| 59 | yield* git(work, "commit", "--quiet", "-m", "Docs"); | |
| 60 | yield* git(work, "push", "--quiet", "origin", "main", "feat/answer-42", "feat/answer-doc"); | |
| 61 | const head = (branch: string) => git(remote, "rev-parse", `refs/heads/${branch}`); | |
| 62 | return { | |
| 63 | remote, | |
| 64 | head, | |
| 65 | git: (...args: string[]) => git(remote, ...args), | |
| 66 | layers: [ | |
| 67 | { number: 1, headBranch: "feat/answer-42", headSha: yield* head("feat/answer-42") }, | |
| 68 | { number: 2, headBranch: "feat/answer-doc", headSha: yield* head("feat/answer-doc") }, | |
| 69 | ], | |
| 70 | }; | |
| 71 | }); | |
| 72 | ||
| 73 | it.layer(layer)("cascadeRebaseStack", (it) => { | |
| 74 | it.effect("moves each layer's own commits onto the rebased layer below it", () => | |
| 75 | Effect.gen(function* () { | |
| 76 | const repo = yield* setup; | |
| 77 | const completed = yield* cascadeRebaseStack({ | |
| 78 | host: "github.com", | |
| 79 | repository: "acme/web", | |
| 80 | base: "main", | |
| 81 | layers: repo.layers, | |
| 82 | remote: repo.remote, | |
| 83 | }); | |
| 84 | assert.strictEqual(completed, 2); | |
| 85 | ||
| 86 | const main = yield* repo.head("main"); | |
| 87 | const bottom = yield* repo.head("feat/answer-42"); | |
| 88 | const top = yield* repo.head("feat/answer-doc"); | |
| 89 | // Bottom sits on the new main; top sits on the new bottom with only its own commit. | |
| 90 | expect(yield* repo.git("rev-parse", `${bottom}~1`)).toBe(main); | |
| 91 | expect(yield* repo.git("rev-parse", `${top}~1`)).toBe(bottom); | |
| 92 | expect(yield* repo.git("log", "--format=%s", `${bottom}..${top}`)).toBe( | |
| 93 | "B: document the answer", | |
| 94 | ); | |
| 95 | }).pipe(Effect.scoped), | |
| 96 | ); | |
| 97 | ||
| 98 | it.effect("refuses to overwrite a layer pushed after it was reviewed", () => | |
| 99 | Effect.gen(function* () { | |
| 100 | const repo = yield* setup; | |
| 101 | const stale = [ | |
| 102 | repo.layers[0]!, | |
| 103 | // Reviewed at the bottom layer's head, but the branch is really at its own commit. | |
| 104 | { ...repo.layers[1]!, headSha: repo.layers[0]!.headSha }, | |
| 105 | ]; | |
| 106 | const error = yield* Effect.flip( | |
| 107 | cascadeRebaseStack({ | |
| 108 | host: "github.com", | |
| 109 | repository: "acme/web", | |
| 110 | base: "main", | |
| 111 | layers: stale, | |
| 112 | remote: repo.remote, | |
| 113 | }), | |
| 114 | ); | |
| 115 | expect(error).toMatchObject({ _tag: "GitHubStackRebaseGitError", number: 2, completed: 1 }); | |
| 116 | expect(yield* repo.head("feat/answer-doc")).toBe(repo.layers[1]!.headSha); | |
| 117 | }).pipe(Effect.scoped), | |
| 118 | ); | |
| 119 | ||
| 120 | it.effect("stops at a conflicting layer and leaves it untouched", () => | |
| 121 | Effect.gen(function* () { | |
| 122 | const repo = yield* setup; | |
| 123 | // Make main conflict with the bottom layer's change to the same line. | |
| 124 | const fs = yield* FileSystem.FileSystem; | |
| 125 | const process = yield* VcsProcess.VcsProcess; | |
| 126 | const scratch = yield* fs.makeTempDirectoryScoped({ prefix: "t3-cascade-conflict-" }); | |
| 127 | const run = (...args: string[]) => | |
| 128 | process.run({ | |
| 129 | operation: "test", | |
| 130 | command: "git", | |
| 131 | args, | |
| 132 | cwd: scratch, | |
| 133 | env: { | |
| 134 | GIT_AUTHOR_NAME: "t", | |
| 135 | GIT_AUTHOR_EMAIL: "t@t", | |
| 136 | GIT_COMMITTER_NAME: "t", | |
| 137 | GIT_COMMITTER_EMAIL: "t@t", | |
| 138 | }, | |
| 139 | }); | |
| 140 | yield* run("clone", "--quiet", repo.remote, "."); | |
| 141 | yield* fs.writeFileString(`${scratch}/index.ts`, "export const answer = 43;\n"); | |
| 142 | yield* run("commit", "--quiet", "-am", "Conflict"); | |
| 143 | yield* run("push", "--quiet", "origin", "main"); | |
| 144 | ||
| 145 | const error = yield* Effect.flip( | |
| 146 | cascadeRebaseStack({ | |
| 147 | host: "github.com", | |
| 148 | repository: "acme/web", | |
| 149 | base: "main", | |
| 150 | layers: repo.layers, | |
| 151 | remote: repo.remote, | |
| 152 | }), | |
| 153 | ); | |
| 154 | expect(error).toMatchObject({ | |
| 155 | _tag: "GitHubStackRebaseConflictError", | |
| 156 | number: 1, | |
| 157 | completed: 0, | |
| 158 | }); | |
| 159 | expect(yield* repo.head("feat/answer-42")).toBe(repo.layers[0]!.headSha); | |
| 160 | }).pipe(Effect.scoped), | |
| 161 | ); | |
| 162 | }); |
apps/server/src/pullRequest/githubStackRebase.ts created+155| ... | ... | @@ -0,0 +1,155 @@ |
| 1 | import * as Effect from "effect/Effect"; | |
| 2 | import * as FileSystem from "effect/FileSystem"; | |
| 3 | import * as Redacted from "effect/Redacted"; | |
| 4 | import * as Schema from "effect/Schema"; | |
| 5 | import * as Base64 from "effect/encoding/Base64"; | |
| 6 | ||
| 7 | import * as GitHubApi from "../sourceControl/GitHubApi.ts"; | |
| 8 | import * as VcsProcess from "../vcs/VcsProcess.ts"; | |
| 9 | ||
| 10 | /** One open layer of a stack, bottom to top, with the head the reader reviewed. */ | |
| 11 | export interface CascadeLayer { | |
| 12 | readonly number: number; | |
| 13 | readonly headBranch: string; | |
| 14 | readonly headSha: string; | |
| 15 | } | |
| 16 | ||
| 17 | /** A layer's rebase hit a conflict; the layers below it are already rebased on GitHub. */ | |
| 18 | export class GitHubStackRebaseConflictError extends Schema.TaggedError<GitHubStackRebaseConflictError>()( | |
| 19 | "GitHubStackRebaseConflictError", | |
| 20 | { number: Schema.Int, completed: Schema.Int }, | |
| 21 | ) { | |
| 22 | override get message(): string { | |
| 23 | return `PR #${this.number} conflicts with the layer below it. ${this.completed} layers were rebased; resolve #${this.number} with \`gh stack rebase\` and push.`; | |
| 24 | } | |
| 25 | } | |
| 26 | ||
| 27 | /** Git failed for a reason other than a conflict: the fetch, or a push GitHub refused. */ | |
| 28 | export class GitHubStackRebaseGitError extends Schema.TaggedError<GitHubStackRebaseGitError>()( | |
| 29 | "GitHubStackRebaseGitError", | |
| 30 | { step: Schema.String, number: Schema.Int, completed: Schema.Int, cause: Schema.Defect() }, | |
| 31 | ) { | |
| 32 | override get message(): string { | |
| 33 | return `Stack rebase stopped at PR #${this.number} while ${this.step}. ${this.completed} layers were rebased.`; | |
| 34 | } | |
| 35 | } | |
| 36 | ||
| 37 | /** The git URL of a repository on a GitHub host, which is also where its token is good. */ | |
| 38 | function remoteUrl(host: string, repository: string): string { | |
| 39 | return `https://${host.trim().toLowerCase()}/${repository}.git`; | |
| 40 | } | |
| 41 | ||
| 42 | /** | |
| 43 | * GitHub's own "Rebase stack": every open layer, bottom to top, onto the new head of the layer | |
| 44 | * below it (the bottom one onto the stack's base). Each layer moves only its own commits — | |
| 45 | * `git rebase --onto <new parent> <old parent>` — so a rebased lower layer is never replayed into | |
| 46 | * the one above it, which is what GitHub's per-PR "update branch" does and why it conflicts. | |
| 47 | * | |
| 48 | * It works in a throwaway clone, so the environment's checkout never moves, and pushes each | |
| 49 | * layer with a lease on the head that was reviewed, so a push that landed meanwhile is refused | |
| 50 | * rather than overwritten. The stacks API has no rebase endpoint to call instead. | |
| 51 | */ | |
| 52 | export const cascadeRebaseStack = Effect.fn("cascadeRebaseStack")(function* (input: { | |
| 53 | readonly host: string; | |
| 54 | readonly repository: string; | |
| 55 | readonly base: string; | |
| 56 | readonly layers: ReadonlyArray<CascadeLayer>; | |
| 57 | /** Where to fetch and push; the repository's own URL on its host unless a test points elsewhere. */ | |
| 58 | readonly remote?: string; | |
| 59 | }) { | |
| 60 | const api = yield* GitHubApi.GitHubApi; | |
| 61 | const process = yield* VcsProcess.VcsProcess; | |
| 62 | const fileSystem = yield* FileSystem.FileSystem; | |
| 63 | const { token } = yield* api.credential(input.host); | |
| 64 | const first = input.layers[0]; | |
| 65 | if (first === undefined) return 0; | |
| 66 | const directory = yield* fileSystem.makeTempDirectoryScoped({ prefix: "t3-stack-rebase-" }).pipe( | |
| 67 | Effect.mapError( | |
| 68 | (cause) => | |
| 69 | new GitHubStackRebaseGitError({ | |
| 70 | step: "preparing", | |
| 71 | number: first.number, | |
| 72 | completed: 0, | |
| 73 | cause, | |
| 74 | }), | |
| 75 | ), | |
| 76 | ); | |
| 77 | // The token rides in a header for this process only: never in the URL, argv, or git config. | |
| 78 | const remote = input.remote ?? remoteUrl(input.host, input.repository); | |
| 79 | const authorization = `AUTHORIZATION: basic ${Base64.encode(`x-access-token:${Redacted.value(token)}`)}`; | |
| 80 | const env = { | |
| 81 | GIT_TERMINAL_PROMPT: "0", | |
| 82 | GIT_CONFIG_COUNT: "3", | |
| 83 | GIT_CONFIG_KEY_0: `http.${remote}.extraheader`, | |
| 84 | GIT_CONFIG_VALUE_0: authorization, | |
| 85 | GIT_CONFIG_KEY_1: "user.name", | |
| 86 | GIT_CONFIG_VALUE_1: "T3 Code", | |
| 87 | GIT_CONFIG_KEY_2: "user.email", | |
| 88 | GIT_CONFIG_VALUE_2: "noreply@t3.codes", | |
| 89 | }; | |
| 90 | const git = (args: ReadonlyArray<string>, allowNonZeroExit = false) => | |
| 91 | process.run({ | |
| 92 | operation: "cascadeRebaseStack", | |
| 93 | command: "git", | |
| 94 | args, | |
| 95 | cwd: directory, | |
| 96 | env, | |
| 97 | timeoutMs: 120_000, | |
| 98 | ...(allowNonZeroExit ? { allowNonZeroExit: true } : {}), | |
| 99 | }); | |
| 100 | const failed = (step: string, number: number, completed: number) => (cause: unknown) => | |
| 101 | new GitHubStackRebaseGitError({ step, number, completed, cause }); | |
| 102 | ||
| 103 | yield* git(["init", "--quiet"]).pipe(Effect.mapError(failed("preparing", first.number, 0))); | |
| 104 | yield* git(["remote", "add", "origin", remote]).pipe( | |
| 105 | Effect.mapError(failed("preparing", first.number, 0)), | |
| 106 | ); | |
| 107 | yield* git([ | |
| 108 | "fetch", | |
| 109 | "--quiet", | |
| 110 | "--no-tags", | |
| 111 | "origin", | |
| 112 | `+refs/heads/${input.base}:refs/remotes/origin/${input.base}`, | |
| 113 | ...input.layers.map( | |
| 114 | (layer) => `+refs/heads/${layer.headBranch}:refs/remotes/origin/${layer.headBranch}`, | |
| 115 | ), | |
| 116 | ]).pipe(Effect.mapError(failed("fetching", first.number, 0))); | |
| 117 | ||
| 118 | let parentOld = `origin/${input.base}`; | |
| 119 | let parentNew = `origin/${input.base}`; | |
| 120 | for (const [index, layer] of input.layers.entries()) { | |
| 121 | // The bottom layer's old parent is where it forked from the base; above that it is the | |
| 122 | // reviewed head of the layer below, which is exactly the commits this layer must not replay. | |
| 123 | const upstream = | |
| 124 | index === 0 | |
| 125 | ? (yield* git(["merge-base", parentOld, layer.headSha]).pipe( | |
| 126 | Effect.mapError(failed("reading the fork point", layer.number, index)), | |
| 127 | )).stdout.trim() | |
| 128 | : parentOld; | |
| 129 | yield* git(["checkout", "--quiet", "--detach", layer.headSha]).pipe( | |
| 130 | Effect.mapError(failed("checking out", layer.number, index)), | |
| 131 | ); | |
| 132 | const rebase = yield* git(["rebase", "--quiet", "--onto", parentNew, upstream], true).pipe( | |
| 133 | Effect.mapError(failed("rebasing", layer.number, index)), | |
| 134 | ); | |
| 135 | if (rebase.exitCode !== 0) { | |
| 136 | yield* git(["rebase", "--abort"], true).pipe(Effect.ignore); | |
| 137 | return yield* new GitHubStackRebaseConflictError({ number: layer.number, completed: index }); | |
| 138 | } | |
| 139 | const rebased = (yield* git(["rev-parse", "HEAD"]).pipe( | |
| 140 | Effect.mapError(failed("rebasing", layer.number, index)), | |
| 141 | )).stdout.trim(); | |
| 142 | if (rebased !== layer.headSha) { | |
| 143 | yield* git([ | |
| 144 | "push", | |
| 145 | "--quiet", | |
| 146 | `--force-with-lease=refs/heads/${layer.headBranch}:${layer.headSha}`, | |
| 147 | "origin", | |
| 148 | `${rebased}:refs/heads/${layer.headBranch}`, | |
| 149 | ]).pipe(Effect.mapError(failed("pushing", layer.number, index))); | |
| 150 | } | |
| 151 | parentOld = layer.headSha; | |
| 152 | parentNew = rebased; | |
| 153 | } | |
| 154 | return input.layers.length; | |
| 155 | }, Effect.scoped); |