Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,7 @@ export const make = Effect.fn("makeAzureDevOpsPullRequestProvider")(function* ()
reviewRequestedLogins: row.reviewRequestedLogins,
// Azure keeps labels on work items rather than on the pull request.
labels: [],
autoMergeEnabled: row.autoMergeEnabled,
});

const provider: PullRequestProviderApi = {
Expand Down Expand Up @@ -342,7 +343,6 @@ export const make = Effect.fn("makeAzureDevOpsPullRequestProvider")(function* ()
checks: [],
baseComparison: "unknown" as const,
behindBy: null,
autoMergeEnabled: row.autoMergeEnabled,
})),
),

Expand Down
2 changes: 0 additions & 2 deletions apps/server/src/pullRequest/BitbucketPullRequestProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -332,8 +332,6 @@ export const make = Effect.fn("makeBitbucketPullRequestProvider")(function* () {
// Bitbucket compares no branch with its base.
baseComparison: "unknown" as const,
behindBy: null,
// Bitbucket has nothing that arms a merge to run on its own.
autoMergeEnabled: null,
})),
);
}),
Expand Down
61 changes: 56 additions & 5 deletions apps/server/src/pullRequest/GitHubPullRequestProvider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,6 @@ describe("GitHubPullRequestProvider.runAction", () => {
input: { action: "update-branch", updateMethod: "rebase" },
args: ["pr", "update-branch", "12", "--repo", "octocat/example-app", "--rebase"],
},
{
name: "arms auto-merge with the strategy the host stores alongside it",
input: { action: "enable-auto-merge", mergeMethod: "squash" },
args: ["pr", "merge", "12", "--repo", "octocat/example-app", "--auto", "--squash"],
},
{
name: "disarms auto-merge",
input: { action: "disable-auto-merge" },
Expand All @@ -68,6 +63,62 @@ describe("GitHubPullRequestProvider.runAction", () => {
}).pipe(Effect.provide(layer)),
);
}

/** The host answers the readiness read with one status and every write with nothing. */
const hostReports = (status: string) => {
mockExecute.mockImplementation((input) =>
Effect.succeed(
processOutput(input.args[1] === "view" ? JSON.stringify({ mergeStateStatus: status }) : ""),
),
);
};
const prCalls = () =>
calls()
.map((call) => call.args)
.filter((args) => args[0] === "pr");

it.effect("arms auto-merge only once the host says the merge would wait", () =>
Effect.gen(function* () {
hostReports("BLOCKED");
const provider = yield* GitHubPullRequestProvider.make();

yield* provider.runAction({
...repository,
number: 12,
action: "enable-auto-merge",
mergeMethod: "squash",
});

assert.deepStrictEqual(prCalls(), [
["pr", "view", "12", "--repo", "octocat/example-app", "--json", "mergeStateStatus"],
// The strategy travels with the standing instruction.
["pr", "merge", "12", "--repo", "octocat/example-app", "--auto", "--squash"],
]);
}).pipe(Effect.provide(layer)),
);

it.effect("refuses to arm a pull request gh would merge on the spot", () =>
Effect.gen(function* () {
hostReports("CLEAN");
const provider = yield* GitHubPullRequestProvider.make();

const error = yield* provider
.runAction({
...repository,
number: 12,
action: "enable-auto-merge",
mergeMethod: "squash",
})
.pipe(Effect.flip);

assert.equal(error.detail, "This pull request can merge right now. Use Merge instead.");
// Nothing ran but the read: the merge never reached the host.
assert.deepStrictEqual(
prCalls().map((args) => args[1]),
["view"],
);
}).pipe(Effect.provide(layer)),
);
});

describe("GitHubPullRequestProvider.submitReview", () => {
Expand Down
57 changes: 51 additions & 6 deletions apps/server/src/pullRequest/GitHubPullRequestProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
import {
decodeGitHubPullRequestActivityJson,
decodeGitHubPullRequestDetailJson,
decodeGitHubImmediatelyMergeableJson,
decodeGitHubRepositoryJson,
GITHUB_PULL_REQUEST_ACTIVITY_FIELDS,
GITHUB_PULL_REQUEST_DETAIL_FIELDS,
Expand Down Expand Up @@ -259,6 +260,43 @@ export const make = Effect.fn("makeGitHubPullRequestProvider")(function* () {
stdin: encodeGraphQlRequestJson({ query: input.query, variables: input.variables }),
});

/**
* Fails where GitHub would merge the pull request the moment it is armed. The
* refusal is this app's own sentence, since the host never gets asked.
*/
const refuseIfImmediatelyMergeable = (
input: ProviderRepositoryRef & { readonly number: number },
) =>
run({
operation: "runAction",
cwd: input.cwd,
args: [
"pr",
"view",
String(input.number),
...repositoryArgs(input),
"--json",
"mergeStateStatus",
],
}).pipe(
Effect.flatMap((output) => {
const decoded = decodeGitHubImmediatelyMergeableJson(output.stdout.trim());
if (!Result.isSuccess(decoded)) {
return Effect.fail(decodeError("runAction", "merge state", decoded.failure));
}
return decoded.success
? Effect.fail(
new PullRequestProviderError({
provider: PROVIDER_KIND,
operation: "runAction",
reason: "failed",
detail: "This pull request can merge right now. Use Merge instead.",
}),
)
: Effect.void;
}),
);

const graphqlRead = <A>(input: {
readonly operation: string;
readonly cwd: string;
Expand Down Expand Up @@ -626,12 +664,19 @@ export const make = Effect.fn("makeGitHubPullRequestProvider")(function* () {
updateMethod: input.updateMethod,
deleteBranch: input.deleteBranch,
});
return run({
operation: "runAction",
cwd: input.cwd,
args: ["pr", subcommand, String(input.number), ...repositoryArgs(input), ...flags],
...(input.action === "merge" ? { timeoutMs: MERGE_TIMEOUT_MS } : {}),
}).pipe(Effect.asVoid);
const action = () =>
run({
operation: "runAction",
cwd: input.cwd,
args: ["pr", subcommand, String(input.number), ...repositoryArgs(input), ...flags],
...(input.action === "merge" ? { timeoutMs: MERGE_TIMEOUT_MS } : {}),
}).pipe(Effect.asVoid);
// `gh pr merge --auto` merges outright when nothing is pending, which is
// not what someone arming a merge asked for. The readiness is read first
// and a ready pull request is sent back to the Merge button instead.
return input.action === "enable-auto-merge"
? refuseIfImmediatelyMergeable(input).pipe(Effect.flatMap(action))
: action();
},

comment: (input) =>
Expand Down
9 changes: 7 additions & 2 deletions apps/server/src/pullRequest/PullRequestProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ export interface ProviderChangeRequest {
readonly mergeability?: PullRequestMergeability;
/** Absent where there are no checks, or where the listing did not ask for them. */
readonly checksState?: PullRequestChecksState;
/** Whether the host is armed to merge it on its own; absent where the host does not say. */
readonly autoMergeEnabled?: boolean;
}

/**
Expand Down Expand Up @@ -122,8 +124,6 @@ export interface ProviderChangeRequestDetail extends ProviderChangeRequest {
readonly baseComparison: PullRequestBaseComparison;
/** Null where the host could not compare the branch with its base. */
readonly behindBy: number | null;
/** Null where the host does not say whether it is armed to merge on its own. */
readonly autoMergeEnabled: boolean | null;
}

/**
Expand All @@ -135,6 +135,11 @@ export interface ProviderRepositoryAccess {
readonly mergeMethods: ReadonlyArray<PullRequestMergeMethod>;
/** What a pull request has to target not to be stacked on other work. */
readonly defaultBranch: string | null;
/**
* Whether the repository lets a pull request be armed to merge on its own.
* Absent where the host has no such switch, or did not say.
*/
readonly autoMergeAllowed?: boolean;
}

/**
Expand Down
8 changes: 7 additions & 1 deletion apps/server/src/pullRequest/PullRequestService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,13 +214,15 @@ const repositoryJson = (input?: {
readonly merge?: boolean;
readonly squash?: boolean;
readonly rebase?: boolean;
readonly autoMerge?: boolean;
}) =>
JSON.stringify({
name: "example-app",
permissions: { admin: false, push: input?.push ?? true, pull: true },
allow_merge_commit: input?.merge ?? true,
allow_squash_merge: input?.squash ?? true,
allow_rebase_merge: input?.rebase ?? true,
allow_auto_merge: input?.autoMerge ?? true,
});

const detailJson = (input?: { readonly author?: string; readonly isDraft?: boolean }) =>
Expand Down Expand Up @@ -846,7 +848,7 @@ describe("PullRequestService pull request reads", () => {
onlyGitHubProject();
let author = "hubot";
hostAnswers({
repository: repositoryJson({ push: false, merge: false }),
repository: repositoryJson({ push: false, merge: false, autoMerge: false }),
detail: () => detailJson({ author }),
});

Expand All @@ -859,6 +861,10 @@ describe("PullRequestService pull request reads", () => {
canManage: false,
});
assert.deepStrictEqual(theirs.mergeMethods, ["squash", "rebase"]);
// A repository with auto-merge switched off is never offered the switch;
// everything else the host can do stays.
assert.equal(theirs.capabilities.actions.includes("enable-auto-merge"), false);
assert.equal(theirs.capabilities.actions.includes("merge"), true);

author = "OctoCat";
const mine = yield* service.detail({ ...reference, force: true });
Expand Down
15 changes: 12 additions & 3 deletions apps/server/src/pullRequest/PullRequestService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,7 @@ function toEntry(input: {
...(row.mergeability === undefined || row.mergeability === "unknown"
? {}
: { mergeability: row.mergeability }),
...(row.autoMergeEnabled === undefined ? {} : { autoMergeEnabled: row.autoMergeEnabled }),
labels: row.labels,
origin: input.origin,
};
Expand Down Expand Up @@ -474,11 +475,19 @@ function toDetail(input: {
},
mergeMethods: input.repository.mergeMethods,
// What the host supports in general, narrowed to what this repository
// actually allows, so the client never offers a merge the host refuses.
capabilities: { ...input.capabilities, mergeMethods: input.repository.mergeMethods },
// actually allows, so the client never offers a merge the host refuses,
// nor an auto-merge switch the repository has turned off.
capabilities: {
...input.capabilities,
mergeMethods: input.repository.mergeMethods,
actions:
input.repository.autoMergeAllowed === false
? input.capabilities.actions.filter((action) => action !== "enable-auto-merge")
: input.capabilities.actions,
},
baseComparison: row.baseComparison,
behindBy: row.behindBy,
autoMergeEnabled: row.autoMergeEnabled,
autoMergeEnabled: row.autoMergeEnabled ?? null,
isStacked: defaultBranch !== null && row.baseBranch !== defaultBranch,
defaultBranch,
};
Expand Down
8 changes: 7 additions & 1 deletion apps/server/src/pullRequest/gitHubPullRequestDetail.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,9 +200,15 @@ describe("decodeGitHubRepositoryJson", () => {
allow_merge_commit: false,
allow_squash_merge: true,
allow_rebase_merge: true,
allow_auto_merge: false,
default_branch: "main",
}),
{ canWrite: true, mergeMethods: ["squash", "rebase"], defaultBranch: "main" },
{
canWrite: true,
mergeMethods: ["squash", "rebase"],
defaultBranch: "main",
autoMergeAllowed: false,
},
);
});

Expand Down
42 changes: 37 additions & 5 deletions apps/server/src/pullRequest/gitHubPullRequestDetail.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ export interface GitHubRepositoryAccess {
readonly mergeMethods: ReadonlyArray<PullRequestMergeMethod>;
/** What a pull request has to target not to be stacked on other work. */
readonly defaultBranch: string | null;
/** Absent on a host too old to report the auto-merge switch. */
readonly autoMergeAllowed?: boolean;
}

/** The order the detail surface offers the allowed merge methods in. */
Expand All @@ -79,9 +81,24 @@ const GitHubRepositorySchema = Schema.Struct({
allow_merge_commit: Schema.optional(Schema.NullOr(Schema.Boolean)),
allow_squash_merge: Schema.optional(Schema.NullOr(Schema.Boolean)),
allow_rebase_merge: Schema.optional(Schema.NullOr(Schema.Boolean)),
allow_auto_merge: Schema.optional(Schema.NullOr(Schema.Boolean)),
default_branch: Schema.optional(Schema.NullOr(Schema.String)),
});

/**
* `gh pr view --json mergeStateStatus`: the one word GitHub sums a pull
* request's readiness into.
*/
const GitHubMergeStateSchema = Schema.Struct({
mergeStateStatus: Schema.optional(Schema.NullOr(Schema.String)),
});

/**
* The states `gh pr merge --auto` merges outright instead of arming, copied
* from the CLI: nothing is pending, so there is nothing to wait for.
*/
const IMMEDIATELY_MERGEABLE_STATES = new Set(["CLEAN", "HAS_HOOKS", "UNSTABLE"]);

/** A `gh pr view` row: everything a list row carries, plus the detail fields. */
export interface GitHubPullRequestDetailRow extends GitHubPullRequestListRow {
readonly body: string;
Expand All @@ -93,8 +110,6 @@ export interface GitHubPullRequestDetailRow extends GitHubPullRequestListRow {
readonly checks: ReadonlyArray<PullRequestCheck>;
/** Absent while the host is still deciding after a push, or on a host too old to say. */
readonly mergeGate?: PullRequestMergeGate;
/** Null on a host too old to report an auto-merge instruction at all. */
readonly autoMergeEnabled: boolean | null;
/** Qualifies the head branch when it lives on a fork. */
readonly headRepositoryOwnerLogin: string | null;
}
Expand Down Expand Up @@ -373,6 +388,7 @@ function normalizeCommits(
const decodeDetailPayload = decodeJsonResult(GitHubPullRequestDetailRowSchema);
const decodeActivityPayload = decodeJsonResult(GitHubPullRequestActivitySchema);
const decodeRepositoryPayload = decodeJsonResult(GitHubRepositorySchema);
const decodeMergeStatePayload = decodeJsonResult(GitHubMergeStateSchema);

/**
* Decodes `gh api repos/<owner>/<name>` into the viewer's access and the merge
Expand All @@ -398,9 +414,28 @@ export function decodeGitHubRepositoryJson(
(_method, index) => !reported || switches[index] === true,
),
defaultBranch: nonEmptyText(row.default_branch),
...(typeof row.allow_auto_merge === "boolean"
? { autoMergeAllowed: row.allow_auto_merge }
: {}),
});
}

/**
* Whether GitHub would merge the pull request this instant, read from
* `gh pr view --json mergeStateStatus`. A status the host did not name is not
* ready: arming then waits, which is the safe way to be wrong.
*/
export function decodeGitHubImmediatelyMergeableJson(
raw: string,
): Result.Result<boolean, Cause.Cause<Schema.SchemaError>> {
const payload = decodeMergeStatePayload(raw);
if (!Result.isSuccess(payload)) {
return Result.fail(payload.failure);
}
const status = nonEmptyText(payload.success.mergeStateStatus);
return Result.succeed(status !== null && IMMEDIATELY_MERGEABLE_STATES.has(status));
}

/** Decodes `gh pr view --json <detail fields>` into the header the panel renders. */
export function decodeGitHubPullRequestDetailJson(
raw: string,
Expand Down Expand Up @@ -429,9 +464,6 @@ export function decodeGitHubPullRequestDetailJson(
}),
checks: normalizeChecks(row.statusCheckRollup),
...(mergeGate === undefined ? {} : { mergeGate }),
// A CLI too old for the field leaves it absent, which is "the host did not
// say" rather than "auto-merge is off".
autoMergeEnabled: row.autoMergeRequest === undefined ? null : row.autoMergeRequest !== null,
headRepositoryOwnerLogin: nonEmptyText(row.headRepositoryOwner?.login),
});
}
Expand Down
2 changes: 2 additions & 0 deletions apps/server/src/pullRequest/gitHubPullRequestGraphql.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@ describe("decodeGitHubAuthoredPullRequestsJson", () => {
labels: { nodes: [{ name: "bug", color: "d73a4a" }] },
reviewRequests: { nodes: [{ requestedReviewer: { login: "hubot" } }] },
commits: { nodes: [{ commit: { statusCheckRollup: { state: "FAILURE" } } }] },
autoMergeRequest: { enabledAt: "2026-08-31T09:00:00Z" },
...overrides,
});

Expand Down Expand Up @@ -303,6 +304,7 @@ describe("decodeGitHubAuthoredPullRequestsJson", () => {
reviewDecision: "changes-requested",
checksState: "failure",
mergeability: "conflicting",
autoMergeEnabled: true,
labels: [{ name: "bug", color: "d73a4a" }],
repository: "openai/codex",
},
Expand Down
Loading
Loading