diff --git a/apps/mobile/src/components/ui/AndroidScreenHeader.tsx b/apps/mobile/src/components/ui/AndroidScreenHeader.tsx index 7931717682fa1..3d7605b71b5fb 100644 --- a/apps/mobile/src/components/ui/AndroidScreenHeader.tsx +++ b/apps/mobile/src/components/ui/AndroidScreenHeader.tsx @@ -21,6 +21,7 @@ export interface AndroidHeaderAction { readonly disabled?: boolean; readonly loading?: boolean; readonly selected?: boolean; + readonly subtitle?: string; readonly destructive?: boolean; readonly overflowOnly?: boolean; } @@ -178,10 +179,11 @@ export function AndroidScreenHeader(props: { actions={overflowActions.map((action, index) => ({ id: String(index), title: action.accessibilityLabel, + subtitle: action.subtitle, + state: action.selected ? "on" : undefined, attributes: { disabled: Boolean(action.disabled), destructive: action.destructive, - state: action.selected ? "on" : undefined, }, }))} onPressAction={({ nativeEvent }) => diff --git a/apps/mobile/src/features/pull-requests/PullRequestRouteScreen.tsx b/apps/mobile/src/features/pull-requests/PullRequestRouteScreen.tsx index e0ad9c76e9bb4..80391ef4060eb 100644 --- a/apps/mobile/src/features/pull-requests/PullRequestRouteScreen.tsx +++ b/apps/mobile/src/features/pull-requests/PullRequestRouteScreen.tsx @@ -111,6 +111,7 @@ import { PullRequestReadStatus, } from "./PullRequestPresentation"; import { usePullRequestFilesSection } from "./PullRequestFiles"; +import { usePullRequestMaintainerEdits } from "./usePullRequestMaintainerEdits"; import { PullRequestDescription } from "./PullRequestDescription"; import { PullRequestMediaVisibleContext } from "./PullRequestMedia"; import { PullRequestMarkdownRepository } from "./PullRequestMarkdown"; @@ -416,6 +417,11 @@ export function PullRequestRouteScreen({ route }: StaticScreenProps pr && @@ -517,6 +523,19 @@ export function PullRequestRouteScreen({ route }: StaticScreenPropsEdit description ) : null} + {maintainerEdits.visible ? ( + + Allow edits from maintainers + + ) : null} {identity ? ( (null); + const kept = maintainerEditsChoiceInForce(choice, pr?.url ?? "", pr?.observedAt, saving); + if (kept !== choice) { + setChoice(kept); + } + const value = kept?.value ?? pr?.maintainerCanModify === true; + const save = async (next: boolean) => { + if (pending.current || pr === null || !canEdit) return; + pending.current = true; + const previousChoice = choice; + setSaving(true); + setChoice({ key: pr.url, observedAt: pr.observedAt, value: next }); + const response = await update({ + ...target, + input: { ...target.input, maintainerCanModify: next }, + }); + pending.current = false; + setSaving(false); + if (AsyncResult.isFailure(response)) { + setChoice(previousChoice); + const cause = squashAtomCommandFailure(response); + Dialog.alert( + "Could not save maintainer access", + cause instanceof Error ? cause.message : "Try again.", + ); + return; + } + refreshDetail(); + }; + return { + visible: canEdit, + value, + saving, + subtitle: saving + ? "Saving…" + : "Maintainers can push to this branch. If your fork has workflows, this can also give access to its secrets.", + toggle: () => void save(!value), + }; +} diff --git a/apps/mobile/src/features/pull-requests/usePullRequestPermissions.ts b/apps/mobile/src/features/pull-requests/usePullRequestPermissions.ts index 741b9c2ad0828..28a2bbb04cc97 100644 --- a/apps/mobile/src/features/pull-requests/usePullRequestPermissions.ts +++ b/apps/mobile/src/features/pull-requests/usePullRequestPermissions.ts @@ -2,6 +2,7 @@ import { useAtomValue } from "@effect/atom-react"; import { canEditPullRequestChangeRequest, canEditPullRequestComment, + canEditPullRequestMaintainerEdits, } from "@t3tools/client-runtime/pull-request-editing"; import type { PullRequestComment, PullRequestDetail } from "@t3tools/contracts"; import { usePullRequestConnected } from "./usePullRequestConnected"; @@ -49,6 +50,8 @@ export function usePullRequestPermissions(params: PullRequestParams, pr: PullReq canWrite: connected && canWrite, canEditChangeRequest: connected && canUpdate && pr !== null && canEditPullRequestChangeRequest(pr), + canEditMaintainerEdits: + connected && canUpdate && pr !== null && canEditPullRequestMaintainerEdits(pr), canEditComment: (comment: Pick) => connected && canUpdateComment && pr !== null && canEditPullRequestComment(pr, comment), canReact: connected && canSetReaction && pr?.capabilities.reactions === true, diff --git a/apps/server/src/pullRequest/PullRequestService.test.ts b/apps/server/src/pullRequest/PullRequestService.test.ts index 4b2d67738c20c..6c6c8215aa315 100644 --- a/apps/server/src/pullRequest/PullRequestService.test.ts +++ b/apps/server/src/pullRequest/PullRequestService.test.ts @@ -116,6 +116,65 @@ function hostedChangeRequest(body: string, additions = 1) { }; } +it.effect("reads maintainer edits and changes them through the update command", () => + Effect.gen(function* () { + let enabled = true; + const provider = fakePullRequestProvider(SourceControlProviderKind.make("github"), { + getChangeRequest: () => + Effect.succeed({ ...hostedChangeRequest("Body"), maintainerCanModify: enabled }), + updateChangeRequest: (input) => + Effect.sync(() => { + assert.strictEqual(input.title, undefined); + assert.strictEqual(input.body, undefined); + enabled = input.maintainerCanModify!; + }), + }); + const service = yield* makeService({ + projects: [project({ id: "p1", title: "web", workspaceRoot: "/w", repository: "acme/web" })], + providers: [ + { + ...provider, + capabilities: { + ...provider.capabilities, + edit: { changeRequest: true, comment: true, maintainerEdits: true }, + }, + }, + ], + }); + const ref = { projectId: "p1" as ProjectId, repository: "acme/web", number: 1 }; + assert.strictEqual((yield* service.detail(ref)).maintainerCanModify, true); + yield* service.update({ ...ref, maintainerCanModify: false }); + assert.strictEqual((yield* service.detail(ref)).maintainerCanModify, false); + }), +); + +it.effect("refuses maintainer edits on a host without the capability", () => + Effect.gen(function* () { + let writes = 0; + const service = yield* makeService({ + projects: [project({ id: "p1", title: "web", workspaceRoot: "/w", repository: "acme/web" })], + providers: [ + fakePullRequestProvider(SourceControlProviderKind.make("github"), { + updateChangeRequest: () => + Effect.sync(() => { + writes += 1; + }), + }), + ], + }); + const result = yield* service + .update({ + projectId: "p1" as ProjectId, + repository: "acme/web", + number: 1, + maintainerCanModify: false, + }) + .pipe(Effect.flip); + assert.strictEqual(result._tag, "PullRequestOperationError"); + assert.strictEqual(writes, 0); + }), +); + it.effect("does not wait for an in-flight detail read to display a preview", () => Effect.gen(function* () { const detailStarted = yield* Deferred.make(); diff --git a/apps/server/src/pullRequest/PullRequestService.ts b/apps/server/src/pullRequest/PullRequestService.ts index 5ee9b6274408a..854d182f155ec 100644 --- a/apps/server/src/pullRequest/PullRequestService.ts +++ b/apps/server/src/pullRequest/PullRequestService.ts @@ -1753,6 +1753,9 @@ export const make = Effect.gen(function* () { ...(changeRequest.autoMergeMethod === undefined ? {} : { autoMergeMethod: changeRequest.autoMergeMethod }), + ...(changeRequest.maintainerCanModify === undefined + ? {} + : { maintainerCanModify: changeRequest.maintainerCanModify }), ...(changeRequest.workflowApprovalsRequired === undefined ? {} : { workflowApprovalsRequired: changeRequest.workflowApprovalsRequired }), @@ -2138,7 +2141,22 @@ export const make = Effect.gen(function* () { }), ); } - if (input.title === undefined && input.body === undefined) { + if ( + input.maintainerCanModify !== undefined && + project.api.capabilities.edit?.maintainerEdits !== true + ) { + return Effect.fail( + new PullRequestOperationError({ + operation: "update", + detail: "This host does not support maintainer edits.", + }), + ); + } + if ( + input.title === undefined && + input.body === undefined && + input.maintainerCanModify === undefined + ) { return Effect.fail( new PullRequestOperationError({ operation: "update", @@ -2151,6 +2169,9 @@ export const make = Effect.gen(function* () { repository: project.repository, host: project.host, number: input.number, + ...(input.maintainerCanModify === undefined + ? {} + : { maintainerCanModify: input.maintainerCanModify }), ...(input.title === undefined ? {} : { title: input.title }), ...(input.body === undefined ? {} : { body: input.body }), }).pipe(Effect.mapError(toPullRequestError("update"))); diff --git a/apps/web/src/components/pullRequest/PullRequestActionsMenu.browser.test.tsx b/apps/web/src/components/pullRequest/PullRequestActionsMenu.browser.test.tsx new file mode 100644 index 0000000000000..521d9c47f316e --- /dev/null +++ b/apps/web/src/components/pullRequest/PullRequestActionsMenu.browser.test.tsx @@ -0,0 +1,212 @@ +import "~/index.css"; +import { EnvironmentId, ProjectId } from "@t3tools/contracts"; +import { sourceControlClients } from "@t3tools/client-runtime/source-control-clients"; +import { Cause } from "effect"; +import { expect, it, vi } from "vite-plus/test"; +import { page } from "vite-plus/test/browser"; + +const mocks = vi.hoisted(() => ({ + update: + vi.fn<(input: unknown) => Promise<{ _tag: "Success" } | { _tag: "Failure"; cause: unknown }>>(), + notify: vi.fn(), +})); +vi.mock( + import("~/state/pullRequests"), + () => ({ pullRequestEnvironment: { update: {} } }) as never, +); +vi.mock( + import("~/state/use-atom-command"), + () => ({ useAtomCommand: () => mocks.update }) as never, +); + +vi.mock(import("../ui/toast"), () => ({ toastManager: { add: mocks.notify } }) as never); +vi.mock(import("~/state/server"), async () => { + const { Atom } = await import("effect/reactivity"); + return { primaryServerKeybindingsAtom: Atom.make([]) } as never; +}); +vi.mock(import("./PullRequestThreadLinks"), () => ({ PullRequestThreadLinks: () => null })); +vi.mock( + import("./PullRequestDetailActions"), + () => ({ ActOnEnvironmentPicker: () => null }) as never, +); + +import { render } from "../../../test/render"; +import { PullRequestActionsMenu } from "./PullRequestActionsMenu"; + +const reference = { projectId: ProjectId.make("project"), repository: "owner/repo", number: 7 }; +const environmentId = EnvironmentId.make("environment"); + +type Detail = Parameters[0]["detail"]; + +const openDetail: Detail = { + url: "https://github.com/owner/repo/pull/7", + state: "open", + isDraft: false, + headBranch: "feature", + maintainerCanModify: true, + observedAt: 1, + capabilities: { + diff: true, + comment: true, + actions: [], + mergeMethods: [], + search: true, + review: { inlineComment: true, reply: true, resolve: true, verdicts: [] }, + reviewers: { request: true, listCandidates: true }, + edit: { changeRequest: true, comment: true, maintainerEdits: true }, + }, + viewerPermissions: { + actions: [], + comment: true, + resolve: true, + verdicts: [], + requestReviewers: true, + maintainerEdits: true, + }, +}; + +function setup() { + mocks.update.mockReset().mockResolvedValue({ _tag: "Success" }); + mocks.notify.mockReset(); + return (detail: Detail = openDetail) => ( + {}} + refreshing={false} + refreshFromHost={async () => {}} + handoff={null} + attachesToComposer={false} + askAboutPullRequest={() => {}} + explainPullRequest={() => {}} + canFixFindings={false} + startFixFindings={() => {}} + handoffLabels={{ fixFindings: "Fix review findings" }} + canWriteGuide={false} + guideRunning={false} + guideWritten={false} + writeGuide={() => {}} + pickableEnvironments={[]} + actingEnvironmentId={environmentId} + onActingEnvironmentChange={() => {}} + can={() => false} + autoMergeArmed={false} + showsDraftToggle={false} + showsMergeNow={false} + showsAutoMerge={false} + showsMergeMethods={false} + allowedMergeMethods={[]} + selectedMergeMethod="squash" + onSelectMergeMethod={() => {}} + showsDeleteBranch={false} + environmentConnected={true} + canWriteSourceControl={true} + onMaintainerEditsSaved={() => {}} + confirmDeleteBranch={async () => {}} + pendingAction={null} + perform={async () => true} + openOnHostText="Open on GitHub" + copyReference={() => {}} + /> + ); +} + +it("saves maintainer edits, locks while saving, and restores the saved value when a save fails", async () => { + const control = setup(); + await using screen = await render(control()); + await screen.getByRole("button", { name: "More pull request actions" }).click(); + const checkbox = page.getByRole("menuitemcheckbox", { name: "Allow edits from maintainers" }); + await expect.element(checkbox).toBeChecked(); + await checkbox.click(); + await expect.element(page.getByRole("status")).not.toBeInTheDocument(); + expect(mocks.update).toHaveBeenCalledExactlyOnceWith({ + environmentId, + input: { ...reference, maintainerCanModify: false }, + }); + + let settle!: (value: { _tag: "Failure"; cause: unknown }) => void; + mocks.update.mockReturnValueOnce( + new Promise((resolve) => { + settle = resolve; + }), + ); + await checkbox.click(); + await expect.element(checkbox).toBeChecked(); + await expect.element(checkbox).toBeDisabled(); + await expect.element(page.getByRole("status")).toHaveTextContent("Saving…"); + settle({ + _tag: "Failure", + cause: Cause.fail(new Error("Resource not accessible by integration")), + }); + await expect.element(checkbox).not.toBeChecked(); + expect(mocks.notify).toHaveBeenCalledExactlyOnceWith( + expect.objectContaining({ + type: "error", + title: "Could not save maintainer access", + description: "Resource not accessible by integration", + }), + ); +}); + +it("keeps a saved choice through a stale refresh, then follows the host", async () => { + const control = setup(); + await using screen = await render(control()); + await screen.getByRole("button", { name: "More pull request actions" }).click(); + const checkbox = page.getByRole("menuitemcheckbox", { name: "Allow edits from maintainers" }); + let settle!: (value: { _tag: "Success" }) => void; + mocks.update.mockReturnValueOnce( + new Promise((resolve) => { + settle = resolve; + }), + ); + await checkbox.click(); + await screen.rerender(control({ ...openDetail, observedAt: 2 })); + settle({ _tag: "Success" }); + await expect.element(page.getByRole("status")).not.toBeInTheDocument(); + await screen.rerender(control({ ...openDetail, observedAt: 2 })); + await expect.element(checkbox).not.toBeChecked(); + await screen.rerender(control({ ...openDetail, maintainerCanModify: false, observedAt: 3 })); + await expect.element(checkbox).not.toBeChecked(); + await screen.rerender(control({ ...openDetail, maintainerCanModify: true, observedAt: 4 })); + await expect.element(checkbox).toBeChecked(); +}); + +it("follows the host's first reading after a save, even when it matches the old value", async () => { + const control = setup(); + await using screen = await render(control()); + await screen.getByRole("button", { name: "More pull request actions" }).click(); + const checkbox = page.getByRole("menuitemcheckbox", { name: "Allow edits from maintainers" }); + await checkbox.click(); + await expect.element(page.getByRole("status")).not.toBeInTheDocument(); + await expect.element(checkbox).not.toBeChecked(); + await screen.rerender(control({ ...openDetail, observedAt: 2 })); + await expect.element(checkbox).toBeChecked(); +}); + +it.each<[string, Detail]>([ + ["closed", { ...openDetail, state: "closed" }], + [ + "not authored by the viewer", + { + ...openDetail, + viewerPermissions: { ...openDetail.viewerPermissions, maintainerEdits: false }, + }, + ], + [ + "on a host without maintainer edits", + { + ...openDetail, + capabilities: { ...openDetail.capabilities, edit: { changeRequest: true, comment: true } }, + }, + ], +])("hides maintainer edits on a pull request that is %s", async (_, detail) => { + const control = setup(); + await using screen = await render(control(detail)); + await screen.getByRole("button", { name: "More pull request actions" }).click(); + await expect.element(page.getByRole("menuitem", { name: "Open on GitHub" })).toBeVisible(); + expect(document.querySelector('[role="menuitemcheckbox"]')).toBeNull(); +}); diff --git a/apps/web/src/components/pullRequest/PullRequestActionsMenu.tsx b/apps/web/src/components/pullRequest/PullRequestActionsMenu.tsx index c920bbd3e19ff..df402185ce350 100644 --- a/apps/web/src/components/pullRequest/PullRequestActionsMenu.tsx +++ b/apps/web/src/components/pullRequest/PullRequestActionsMenu.tsx @@ -1,5 +1,11 @@ import { useAtomValue } from "@effect/atom-react"; +import { + canEditPullRequestMaintainerEdits, + maintainerEditsChoiceInForce, + type MaintainerEditsChoice, +} from "@t3tools/client-runtime/pull-request-editing"; import type { ChangeRequestTerminology } from "@t3tools/client-runtime/source-control-clients"; +import { squashAtomCommandFailure } from "@t3tools/client-runtime/state/runtime"; import type { EnvironmentId, PullRequestAction, @@ -20,13 +26,17 @@ import { SparklesIcon, Trash2Icon, } from "lucide-react"; +import { useId, useState } from "react"; import { shortcutLabelForCommand } from "~/keybindings"; import { readLocalApi } from "~/localApi"; +import { pullRequestEnvironment } from "~/state/pullRequests"; import { primaryServerKeybindingsAtom } from "~/state/server"; +import { useAtomCommand } from "~/state/use-atom-command"; import { Button } from "../ui/button"; import { Menu, + MenuCheckboxItem, MenuItem, MenuPopup, MenuRadioGroup, @@ -36,11 +46,13 @@ import { MenuTrigger, } from "../ui/menu"; import { RefreshIcon } from "../ui/refresh-icon"; +import { toastManager } from "../ui/toast"; import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; import { ActOnEnvironmentPicker, type PerformPullRequestAction } from "./PullRequestDetailActions"; import { PULL_REQUEST_MERGE_METHOD_LABELS, pullRequestActionMenuHasGroup, + readableFailure, } from "./pullRequestDetail.logic"; import { PullRequestGlyph } from "./pullRequestIcons"; import type { PickableEnvironment } from "./pullRequestProjectAssignment.logic"; @@ -83,6 +95,8 @@ export function PullRequestActionsMenu({ onSelectMergeMethod, showsDeleteBranch, environmentConnected, + canWriteSourceControl, + onMaintainerEditsSaved, confirmDeleteBranch, pendingAction, perform, @@ -92,7 +106,17 @@ export function PullRequestActionsMenu({ environmentId: EnvironmentId; reference: PullRequestRef; pullRequestKey: string; - detail: Pick; + detail: Pick< + PullRequestDetail, + | "url" + | "state" + | "isDraft" + | "headBranch" + | "capabilities" + | "viewerPermissions" + | "maintainerCanModify" + | "observedAt" + >; terminology: ChangeRequestTerminology; linkThreadRef: ScopedThreadRef | null; onThreadPickerOpenChange: (open: boolean) => void; @@ -123,6 +147,8 @@ export function PullRequestActionsMenu({ onSelectMergeMethod: (method: PullRequestMergeMethod) => void; showsDeleteBranch: boolean; environmentConnected: boolean; + canWriteSourceControl: boolean; + onMaintainerEditsSaved: () => void; confirmDeleteBranch: () => Promise; pendingAction: PullRequestAction | null; perform: PerformPullRequestAction; @@ -132,6 +158,47 @@ export function PullRequestActionsMenu({ const keybindings = useAtomValue(primaryServerKeybindingsAtom); const inlineConfirm = useInlineActionConfirm(pullRequestKey); const actionPending = pendingAction !== null; + const maintainerEditsHintId = useId(); + const updatePullRequest = useAtomCommand(pullRequestEnvironment.update, { + reportFailure: false, + }); + const [maintainerEditsChoice, setMaintainerEditsChoice] = useState( + null, + ); + const [maintainerEditsSaving, setMaintainerEditsSaving] = useState(false); + const maintainerEditsChoiceKept = maintainerEditsChoiceInForce( + maintainerEditsChoice, + pullRequestKey, + detail.observedAt, + maintainerEditsSaving, + ); + if (maintainerEditsChoiceKept !== maintainerEditsChoice) { + setMaintainerEditsChoice(maintainerEditsChoiceKept); + } + const maintainerCanModify = maintainerEditsChoiceKept?.value ?? detail.maintainerCanModify; + const canSaveMaintainerEdits = + canWriteSourceControl && environmentConnected && !maintainerEditsSaving; + const saveMaintainerEdits = async (value: boolean) => { + if (!canSaveMaintainerEdits) return; + const previousChoice = maintainerEditsChoice; + setMaintainerEditsChoice({ key: pullRequestKey, observedAt: detail.observedAt, value }); + setMaintainerEditsSaving(true); + const result = await updatePullRequest({ + environmentId, + input: { ...reference, maintainerCanModify: value }, + }); + setMaintainerEditsSaving(false); + if (result._tag === "Failure") { + setMaintainerEditsChoice(previousChoice); + toastManager.add({ + type: "error", + title: "Could not save maintainer access", + description: readableFailure(squashAtomCommandFailure(result), "Try again."), + }); + return; + } + onMaintainerEditsSaved(); + }; return ( @@ -218,6 +285,33 @@ export function PullRequestActionsMenu({ /> ) : null} + {canEditPullRequestMaintainerEdits(detail) ? ( + <> + void saveMaintainerEdits(value)} + > + + Allow edits from maintainers + + Maintainers can push to this branch. If your fork has workflows, this can also + give access to its secrets. + + {maintainerEditsSaving ? ( + + Saving… + + ) : null} + + + + + ) : null} {detail.state === "open" ? ( <> {showsDraftToggle ? ( diff --git a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx index 45a070b9806b1..135ae2970ceb9 100644 --- a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx +++ b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx @@ -1381,6 +1381,8 @@ export function PullRequestDetailPanel({ onSelectMergeMethod={selectMergeMethod} showsDeleteBranch={showsDeleteBranch} environmentConnected={environmentConnected} + canWriteSourceControl={canWriteSourceControl} + onMaintainerEditsSaved={refreshDetail} confirmDeleteBranch={confirmDeleteBranch} pendingAction={pendingAction} perform={perform} diff --git a/apps/web/src/components/ui/menu.tsx b/apps/web/src/components/ui/menu.tsx index 287c871beb8b0..95c459b8f8e8f 100644 --- a/apps/web/src/components/ui/menu.tsx +++ b/apps/web/src/components/ui/menu.tsx @@ -163,7 +163,7 @@ function MenuCheckboxItem({ variant = "default", ...props }: MenuPrimitive.CheckboxItem.Props & { - variant?: "default" | "switch"; + variant?: "default" | "icon" | "switch"; }) { return ( ) : ( <> - + , +): boolean { + return ( + detail.state === "open" && + detail.capabilities.edit?.maintainerEdits === true && + detail.viewerPermissions.maintainerEdits === true && + detail.maintainerCanModify !== undefined + ); +} + +export interface MaintainerEditsChoice { + readonly key: string; + readonly observedAt: number | undefined; + readonly value: boolean; +} + +/** + * A saved choice ends at the first host reading delivered after the save. Readers refresh once it + * succeeds, which drops reads still in flight, so that reading started after the write. + */ +export function maintainerEditsChoiceInForce( + choice: MaintainerEditsChoice | null, + key: string, + observedAt: number | undefined, + saving: boolean, +): MaintainerEditsChoice | null { + if (choice === null || choice.key !== key) return null; + if (saving) return choice.observedAt === observedAt ? choice : { ...choice, observedAt }; + return observedAt !== undefined && choice.observedAt === observedAt ? choice : null; +} + /** Hosts do not support rewriting review summaries through comment mutations. */ export function canEditPullRequestComment( detail: EditingSubject, diff --git a/packages/contracts/src/pullRequest.ts b/packages/contracts/src/pullRequest.ts index e38d5fa6cf95a..64111ab4742b1 100644 --- a/packages/contracts/src/pullRequest.ts +++ b/packages/contracts/src/pullRequest.ts @@ -244,6 +244,7 @@ export type PullRequestReviewCapabilities = typeof PullRequestReviewCapabilities export const PullRequestEditCapabilities = Schema.Struct({ changeRequest: Schema.Boolean, comment: Schema.Boolean, + maintainerEdits: Schema.optional(Schema.Boolean), }); export type PullRequestEditCapabilities = typeof PullRequestEditCapabilities.Type; @@ -295,6 +296,7 @@ export const PullRequestViewerPermissions = Schema.Struct({ updateMethods: Schema.optional(ForwardCompatibleArray(PullRequestUpdateMethod)), labels: Schema.optional(Schema.Boolean), editChangeRequest: Schema.optional(Schema.Boolean), + maintainerEdits: Schema.optional(Schema.Boolean), }); export type PullRequestViewerPermissions = typeof PullRequestViewerPermissions.Type; @@ -589,6 +591,7 @@ export const PullRequestDetail = Schema.Struct({ autoMergeEnabled: Schema.optional(Schema.Boolean), autoMergeMethod: Schema.optional(PullRequestMergeMethod), workflowApprovalsRequired: Schema.optional(NonNegativeInt), + maintainerCanModify: Schema.optional(Schema.Boolean), }); export type PullRequestDetail = typeof PullRequestDetail.Type; @@ -746,6 +749,7 @@ export const PullRequestUpdateInput = Schema.Struct({ ...PullRequestRef.fields, title: Schema.optional(TrimmedNonEmptyString.check(Schema.isMaxLength(1024))), body: Schema.optional(Schema.String.check(Schema.isMaxLength(65_536))), + maintainerCanModify: Schema.optional(Schema.Boolean), }); export type PullRequestUpdateInput = typeof PullRequestUpdateInput.Type; diff --git a/packages/source-control-core/src/server/PullRequestProvider.ts b/packages/source-control-core/src/server/PullRequestProvider.ts index b300f14afb0cb..46fc8fd48b0dc 100644 --- a/packages/source-control-core/src/server/PullRequestProvider.ts +++ b/packages/source-control-core/src/server/PullRequestProvider.ts @@ -182,6 +182,7 @@ export interface ProviderChangeRequestStat { } export interface ProviderChangeRequestDetail extends ProviderChangeRequest { + readonly maintainerCanModify?: boolean; readonly headSha?: string | null; readonly body: string; readonly changedFiles: number; @@ -462,6 +463,7 @@ export interface PullRequestProviderApi { /** Requires edit capability and at least one field. */ readonly updateChangeRequest?: ( input: ProviderRepositoryRef & { + readonly maintainerCanModify?: boolean | undefined; readonly number: number; readonly title?: string | undefined; readonly body?: string | undefined; diff --git a/packages/source-control-github/src/server/GitHubPullRequestApi.test.ts b/packages/source-control-github/src/server/GitHubPullRequestApi.test.ts index 3fd74e08130ec..a4b5eb8da4c22 100644 --- a/packages/source-control-github/src/server/GitHubPullRequestApi.test.ts +++ b/packages/source-control-github/src/server/GitHubPullRequestApi.test.ts @@ -68,6 +68,7 @@ const coreResponse = (pullRequest: Readonly> = {}) => ({ mergedAt: null, closedAt: null, isCrossRepository: false, + maintainerCanModify: true, headRepositoryOwner: null, headRepository: null, headRef: null, @@ -710,6 +711,25 @@ it.effect( ); layer("GitHubPullRequestApi.layer", (it) => { + it.effect("turns maintainer edits off without rewriting the pull request's text", () => + Effect.gen(function* () { + mockedExecute.mockImplementation((call) => + Effect.succeed( + output( + encodeJson( + call.kind === "graphql" && matches(call, NODE_ID_QUERY) ? nodeIdAnswer("PR_7") : {}, + ), + ), + ), + ); + const api = yield* GitHubPullRequestApi.GitHubPullRequestApi; + yield* api.updatePullRequest({ ...pullRequestInput, maintainerCanModify: false }); + expect(variablesOf("mutation UpdatePullRequest")).toEqual([ + { pullRequestId: "PR_7", maintainerCanModify: false }, + ]); + }), + ); + it.effect.each(["mixed", "merges", "unavailable"] as const)( "reads stats only for displayed non-merge commits (%s)", (kind) => diff --git a/packages/source-control-github/src/server/GitHubPullRequestApi.ts b/packages/source-control-github/src/server/GitHubPullRequestApi.ts index 4b7014a890744..16a7a1df18cdb 100644 --- a/packages/source-control-github/src/server/GitHubPullRequestApi.ts +++ b/packages/source-control-github/src/server/GitHubPullRequestApi.ts @@ -705,6 +705,7 @@ export class GitHubPullRequestApi extends Context.Service< }) => Effect.Effect; readonly updatePullRequest: (input: { + readonly maintainerCanModify?: boolean | undefined; readonly cwd: string; readonly repository: string; readonly host: string; @@ -2740,6 +2741,9 @@ export const make = Effect.gen(function* () { client .UpdatePullRequest({ pullRequestId, + ...(input.maintainerCanModify === undefined + ? {} + : { maintainerCanModify: input.maintainerCanModify }), ...(input.title === undefined ? {} : { title: input.title }), ...(input.body === undefined ? {} : { body: input.body }), }) diff --git a/packages/source-control-github/src/server/GitHubPullRequestProvider.test.ts b/packages/source-control-github/src/server/GitHubPullRequestProvider.test.ts index 81623592066e5..dc8ca69770e46 100644 --- a/packages/source-control-github/src/server/GitHubPullRequestProvider.test.ts +++ b/packages/source-control-github/src/server/GitHubPullRequestProvider.test.ts @@ -16,6 +16,8 @@ import { gitHubViewerPermissions, loginAvatarUrl, make } from "./GitHubPullReque import type { GitHubReviewThreadComments } from "./gitHubPullRequestJson.ts"; const coreFields = { + maintainerCanModify: true, + canEditMaintainerEdits: false, reviewRequestTeams: [], checksTruncated: false, comparison: { behindBy: 0, viewerCanUpdate: true }, @@ -205,6 +207,7 @@ describe("gitHubViewerPermissions", () => { }); expect(detail.viewerPermissions).toEqual({ + maintainerEdits: false, actions: ["ready", "draft", "close", "reopen"], comment: true, resolve: false, diff --git a/packages/source-control-github/src/server/GitHubPullRequestProvider.ts b/packages/source-control-github/src/server/GitHubPullRequestProvider.ts index f90941bf882ef..4f3490eec97b0 100644 --- a/packages/source-control-github/src/server/GitHubPullRequestProvider.ts +++ b/packages/source-control-github/src/server/GitHubPullRequestProvider.ts @@ -49,7 +49,7 @@ const CAPABILITIES: PullRequestCapabilities = { verdicts: ["comment", "approve", "request-changes"], }, reviewers: { request: true, listCandidates: true }, - edit: { changeRequest: true, comment: true }, + edit: { changeRequest: true, comment: true, maintainerEdits: true }, stacks: true, stackActions: true, labels: true, @@ -453,10 +453,13 @@ export const make = Effect.gen(function* () { ...pullRequest.reviewRequestTeams, ], mergeCapabilities: pullRequest.viewerAccess.mergeCapabilities, - viewerPermissions: gitHubViewerPermissions({ - ...pullRequest.viewerAccess, - canUpdateBranch: pullRequest.comparison?.viewerCanUpdate === true, - }), + viewerPermissions: { + ...gitHubViewerPermissions({ + ...pullRequest.viewerAccess, + canUpdateBranch: pullRequest.comparison?.viewerCanUpdate === true, + }), + maintainerEdits: pullRequest.canEditMaintainerEdits, + }, baseComparison: pullRequest.comparison === null || pullRequest.comparison.behindBy === null ? "unknown" @@ -666,6 +669,9 @@ export const make = Effect.gen(function* () { updateChangeRequest: (input) => cli .updatePullRequest({ + ...(input.maintainerCanModify === undefined + ? {} + : { maintainerCanModify: input.maintainerCanModify }), cwd: input.cwd, repository: input.repository, host: input.host, diff --git a/packages/source-control-github/src/server/gitHubPullRequestJson.test.ts b/packages/source-control-github/src/server/gitHubPullRequestJson.test.ts index 4e6321aa3c7ac..622b48db3b85d 100644 --- a/packages/source-control-github/src/server/gitHubPullRequestJson.test.ts +++ b/packages/source-control-github/src/server/gitHubPullRequestJson.test.ts @@ -362,6 +362,7 @@ describe("pull request detail decoding", () => { baseRefName: "main", headRefOid: "abc123", isCrossRepository: false, + maintainerCanModify: true, headRepositoryOwner: null, headRepository: null, headRef: null, @@ -394,6 +395,30 @@ describe("pull request detail decoding", () => { pullRequest?: Record, ) => expectSuccess(decodePullRequestCoreJson(coreJson(rollup, pullRequest))); + it.each([ + [{}, true], + [{ viewerDidAuthor: false }, false], + [{ isCrossRepository: false }, false], + [{ state: "CLOSED" }, false], + [{ headRepositoryOwner: { __typename: "Organization", login: "team" } }, false], + [{ headRepository: null }, false], + ])( + "reads maintainer edits and lets only the author of an open personal fork change them: %j", + (overrides, allowed) => { + const detail = detailOf([], { + maintainerCanModify: false, + isCrossRepository: true, + viewerDidAuthor: true, + headRepositoryOwner: { __typename: "User", login: "octocat" }, + headRepository: { nameWithOwner: "octocat/t3code", viewerPermission: "ADMIN" }, + headRef: { id: "REF_7" }, + ...overrides, + }); + expect(detail.maintainerCanModify).toBe(false); + expect(detail.canEditMaintainerEdits).toBe(allowed); + }, + ); + const detailJson = JSON.stringify({ comments: [{ id: "c1", body: "second", createdAt: "2026-07-04T00:00:00Z" }], reviews: [ diff --git a/packages/source-control-github/src/server/gitHubPullRequestJson.ts b/packages/source-control-github/src/server/gitHubPullRequestJson.ts index 965855a4350ac..e7c05a4aa8445 100644 --- a/packages/source-control-github/src/server/gitHubPullRequestJson.ts +++ b/packages/source-control-github/src/server/gitHubPullRequestJson.ts @@ -978,6 +978,8 @@ export function toPullRequestWatchFingerprint( } export interface GitHubPullRequestCore extends GitHubPullRequestDetail { + readonly maintainerCanModify: boolean; + readonly canEditMaintainerEdits: boolean; readonly reviewRequestTeams: ReadonlyArray; readonly viewerAccess: GitHubViewerAccess & GitHubRepositoryAccess; readonly comparison: GitHubBaseComparison | null; @@ -1023,6 +1025,14 @@ export function toPullRequestCore(result: PullRequestCore.Result): GitHubPullReq statusCheckRollup: toCheckContexts(nodesOf(contexts)), }), headRepositoryNameWithOwner: trimmed(pr.headRepository?.nameWithOwner), + maintainerCanModify: pr.maintainerCanModify, + canEditMaintainerEdits: + pr.state === "OPEN" && + pr.isCrossRepository && + pr.viewerDidAuthor && + pr.headRepositoryOwner?.__typename === "User" && + pr.headRepository !== null && + pr.headRef !== null, viewerAccess: { canWrite: toCanWrite(repository.viewerPermission), canDeleteBranch: toCanWrite(pr.headRepository?.viewerPermission) && pr.headRef !== null, diff --git a/packages/source-control-github/src/server/pullRequestMutations.graphql b/packages/source-control-github/src/server/pullRequestMutations.graphql index 4ba7854507af7..3e2d5b3133ad5 100644 --- a/packages/source-control-github/src/server/pullRequestMutations.graphql +++ b/packages/source-control-github/src/server/pullRequestMutations.graphql @@ -41,11 +41,23 @@ mutation UnresolveReviewThread($threadId: ID!) { } } -# Rewrites the pull request's own words. Both are nullable so that one document serves a change to -# the title, to the description, or to the two together: a variable the request does not send puts -# no entry in the input at all, which leaves that field as it was rather than clearing it. -mutation UpdatePullRequest($pullRequestId: ID!, $title: String, $body: String) { - updatePullRequest(input: { pullRequestId: $pullRequestId, title: $title, body: $body }) { +# Rewrites the pull request's own words or its maintainer-edit setting. Every field is nullable so +# that one document serves any combination: a variable the request does not send puts no entry in +# the input at all, which leaves that field as it was rather than clearing it. +mutation UpdatePullRequest( + $pullRequestId: ID! + $title: String + $body: String + $maintainerCanModify: Boolean +) { + updatePullRequest( + input: { + pullRequestId: $pullRequestId + title: $title + body: $body + maintainerCanModify: $maintainerCanModify + } + ) { pullRequest { id } diff --git a/packages/source-control-github/src/server/pullRequestMutations.graphql.ts b/packages/source-control-github/src/server/pullRequestMutations.graphql.ts index 05a67b81d4e5c..d11b2b1e69fcf 100644 --- a/packages/source-control-github/src/server/pullRequestMutations.graphql.ts +++ b/packages/source-control-github/src/server/pullRequestMutations.graphql.ts @@ -205,17 +205,19 @@ export const UpdatePullRequest = GraphQL.mutation("UpdatePullRequest", { variables: [ { name: "pullRequestId", body: "ID!" }, { name: "title", body: "String" }, - { name: "body", body: "String" } + { name: "body", body: "String" }, + { name: "maintainerCanModify", body: "Boolean" } ], selections: [ - { name: "updatePullRequest", body: ["(input:{pullRequestId:$", "pullRequestId", " title:$", "title", " body:$", "body", "}){pullRequest{id}}"] } + { name: "updatePullRequest", body: ["(input:{pullRequestId:$", "pullRequestId", " title:$", "title", " body:$", "body", " maintainerCanModify:$", "maintainerCanModify", "}){pullRequest{id}}"] } ], fragments: [] }, variables: { pullRequestId: Shared.ID, title: Schema.optional(Schema.NullOr(Schema.String)), - body: Schema.optional(Schema.NullOr(Schema.String)) + body: Schema.optional(Schema.NullOr(Schema.String)), + maintainerCanModify: Schema.optional(Schema.NullOr(Schema.Boolean)) }, result: Schema.Struct({ /** Update a pull request */ diff --git a/packages/source-control-github/src/server/pullRequestReads.graphql b/packages/source-control-github/src/server/pullRequestReads.graphql index d5a95256fc1a3..78e5efe0e4558 100644 --- a/packages/source-control-github/src/server/pullRequestReads.graphql +++ b/packages/source-control-github/src/server/pullRequestReads.graphql @@ -87,6 +87,7 @@ query PullRequestCore( baseRefName headRefOid isCrossRepository + maintainerCanModify headRepositoryOwner { login } diff --git a/packages/source-control-github/src/server/pullRequestReads.graphql.ts b/packages/source-control-github/src/server/pullRequestReads.graphql.ts index e261ec0240a73..5bc09fa4e2ce4 100644 --- a/packages/source-control-github/src/server/pullRequestReads.graphql.ts +++ b/packages/source-control-github/src/server/pullRequestReads.graphql.ts @@ -395,6 +395,8 @@ export const PullRequestCoreRepositoryPullRequestPullRequest = Schema.Struct({ headRefOid: Shared.GitObjectID, /** The head and base repositories are different. */ isCrossRepository: Schema.Boolean, + /** Indicates whether maintainers can modify the pull request. */ + maintainerCanModify: Schema.Boolean, /** The owner of the repository associated with this pull request's head Ref. */ headRepositoryOwner: Schema.NullOr(RepositoryOwner), /** The repository associated with this pull request's head Ref. */ @@ -457,7 +459,7 @@ export const PullRequestCore = GraphQL.query("PullRequestCore", { { name: "requestedReviewers", body: "Int!" } ], selections: [ - { name: "repository", body: ["(owner:$", "owner", " name:$", "name", "){mergeCommitAllowed squashMergeAllowed rebaseMergeAllowed autoMergeAllowed viewerPermission pullRequest(number:$", "number", "){number title url body state isDraft mergeable mergeStateStatus reviewDecision additions deletions changedFiles createdAt updatedAt mergedAt closedAt headRefName baseRefName headRefOid isCrossRepository headRepositoryOwner{__typename login}headRepository{nameWithOwner viewerPermission}headRef{id}author{__typename login avatarUrl...on User{id name}}autoMergeRequest{mergeMethod}viewerCanUpdate viewerDidAuthor viewerCanUpdateBranch baseRef{compare(headRef:$", "headRef", "){behindBy}}reviewRequests(first:$", "requestedReviewers", "){nodes{requestedReviewer{__typename...on User{login name}...on Bot{login}...on Team{slug combinedSlug teamName:name teamAvatarUrl:avatarUrl}}}}labels(first:100){nodes{name color}}commits(last:1){nodes{commit{statusCheckRollup{contexts(first:100){nodes{__typename...", "CheckContext", "}pageInfo{hasNextPage}}}}}}}}"] } + { name: "repository", body: ["(owner:$", "owner", " name:$", "name", "){mergeCommitAllowed squashMergeAllowed rebaseMergeAllowed autoMergeAllowed viewerPermission pullRequest(number:$", "number", "){number title url body state isDraft mergeable mergeStateStatus reviewDecision additions deletions changedFiles createdAt updatedAt mergedAt closedAt headRefName baseRefName headRefOid isCrossRepository maintainerCanModify headRepositoryOwner{__typename login}headRepository{nameWithOwner viewerPermission}headRef{id}author{__typename login avatarUrl...on User{id name}}autoMergeRequest{mergeMethod}viewerCanUpdate viewerDidAuthor viewerCanUpdateBranch baseRef{compare(headRef:$", "headRef", "){behindBy}}reviewRequests(first:$", "requestedReviewers", "){nodes{requestedReviewer{__typename...on User{login name}...on Bot{login}...on Team{slug combinedSlug teamName:name teamAvatarUrl:avatarUrl}}}}labels(first:100){nodes{name color}}commits(last:1){nodes{commit{statusCheckRollup{contexts(first:100){nodes{__typename...", "CheckContext", "}pageInfo{hasNextPage}}}}}}}}"] } ], fragments: [ { name: "CheckContext", body: ["on StatusCheckRollupContext{__typename...on StatusContext{context state targetUrl createdAt description isRequired(pullRequestNumber:$", "number", ")@include(if:$", "required", ")}...on CheckRun{name status conclusion startedAt completedAt detailsUrl isRequired(pullRequestNumber:$", "number", ")@include(if:$", "required", ")checkSuite{workflowRun{workflow{name}}}}}"] }