authorgravatar for julius0216@outlook.comJulius Marminge <julius0216@outlook.com> 2026-10-09 23:05:38-07:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-10-09 23:05:38-07:00
logc3243f05088c3c20e9439f285fc6d7b0bfe1747d
tree723acaca318b1ea7bb28d18ceb1c32359180ff14
parent0be20c86c2962b7b581dfc79df9ab34b4fbdab14
signature Signed by PGP key B5690EEEBB952194

feat(pull-requests): hosts can report edit and resolve permissions per item (#17667)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

4 files changed, 121 insertions(+), 8 deletions(-)

apps/web/src/components/pullRequest/PullRequestCodeTab.tsx+7-3
...@@ -33,7 +33,7 @@ import { useClientSettings, useUpdateClientSettings } from "~/hooks/useSettings"...@@ -33,7 +33,7 @@ import { useClientSettings, useUpdateClientSettings } from "~/hooks/useSettings"
33import { useTheme } from "~/hooks/useTheme";33import { useTheme } from "~/hooks/useTheme";
34import { areAllDiffFilesCollapsed } from "~/lib/diffCollapse";34import { areAllDiffFilesCollapsed } from "~/lib/diffCollapse";
35import { pullRequestFindingKey, type PullRequestFinding } from "./pullRequestDetail.logic";35import { pullRequestFindingKey, type PullRequestFinding } from "./pullRequestDetail.logic";
36import { canEditPullRequestComment } from "./pullRequestEditing.logic";36import { canEditPullRequestComment, canResolvePullRequestThread } from "./pullRequestEditing.logic";
37import { orderDiffFiles } from "./pullRequestFileOrder.logic";37import { orderDiffFiles } from "./pullRequestFileOrder.logic";
38import {38import {
39 buildFileDiffRenderKey,39 buildFileDiffRenderKey,
...@@ -934,7 +934,7 @@ function PullRequestCodeTab({...@@ -934,7 +934,7 @@ function PullRequestCodeTab({
934 thread={thread}934 thread={thread}
935 workspaceRoot={detail.workspaceRoot}935 workspaceRoot={detail.workspaceRoot}
936 canReply={review.reply}936 canReply={review.reply}
937 canResolve={review.resolve}937 canResolve={review.resolve && canResolvePullRequestThread(detail, thread)}
938 canReact={detail.capabilities.reactions === true}938 canReact={detail.capabilities.reactions === true}
939 environmentId={environmentId}939 environmentId={environmentId}
940 reference={reference}940 reference={reference}
...@@ -966,7 +966,11 @@ function PullRequestCodeTab({...@@ -966,7 +966,11 @@ function PullRequestCodeTab({
966 }966 }
967 // A conversation on a line is made of review comments, whatever the host filed them as.967 // A conversation on a line is made of review comments, whatever the host filed them as.
968 canEditComment={(comment) =>968 canEditComment={(comment) =>
969 canEditPullRequestComment(detail, { author: comment.author, kind: "review-comment" })969 canEditPullRequestComment(detail, {
970 author: comment.author,
971 kind: "review-comment",
972 canEdit: comment.canEdit,
973 })
970 }974 }
971 onEditComment={(commentId, body) =>975 onEditComment={(commentId, body) =>
972 runThreadCommand("The comment could not be saved", () =>976 runThreadCommand("The comment could not be saved", () =>
apps/web/src/components/pullRequest/pullRequestEditing.logic.test.ts+66
...@@ -10,6 +10,7 @@ import { describe, expect, it } from "vite-plus/test";...@@ -10,6 +10,7 @@ import { describe, expect, it } from "vite-plus/test";
10import {10import {
11 canEditPullRequestChangeRequest,11 canEditPullRequestChangeRequest,
12 canEditPullRequestComment,12 canEditPullRequestComment,
13 canResolvePullRequestThread,
13} from "./pullRequestEditing.logic";14} from "./pullRequestEditing.logic";
1415
15type Subject = Pick<PullRequestDetail, "author" | "capabilities" | "viewer" | "viewerPermissions">;16type Subject = Pick<PullRequestDetail, "author" | "capabilities" | "viewer" | "viewerPermissions">;
...@@ -89,6 +90,33 @@ describe("canEditPullRequestChangeRequest", () => {...@@ -89,6 +90,33 @@ describe("canEditPullRequestChangeRequest", () => {
89 ).toBe(true);90 ).toBe(true);
90 });91 });
9192
93 it("follows the host's own answer about the reader over authorship and merge access", () => {
94 expect(
95 canEditPullRequestChangeRequest(
96 subject({
97 author: actor("someone-else"),
98 viewerPermissions: permissions({ editChangeRequest: true }),
99 }),
100 ),
101 ).toBe(true);
102 expect(
103 canEditPullRequestChangeRequest(
104 subject({
105 viewerPermissions: permissions({ actions: ["merge"], editChangeRequest: false }),
106 }),
107 ),
108 ).toBe(false);
109 // The host can't rewrite change requests at all, so the reader's answer can't widen that.
110 expect(
111 canEditPullRequestChangeRequest(
112 subject({
113 capabilities: capabilities({ changeRequest: false, comment: true }),
114 viewerPermissions: permissions({ editChangeRequest: true }),
115 }),
116 ),
117 ).toBe(false);
118 });
119
92 it("refuses a reader who neither wrote it nor may merge it", () => {120 it("refuses a reader who neither wrote it nor may merge it", () => {
93 expect(121 expect(
94 canEditPullRequestChangeRequest(122 canEditPullRequestChangeRequest(
...@@ -146,6 +174,23 @@ describe("canEditPullRequestComment", () => {...@@ -146,6 +174,23 @@ describe("canEditPullRequestComment", () => {
146 );174 );
147 });175 });
148176
177 it("follows the host's own answer about a remark over authorship", () => {
178 expect(canEditPullRequestComment(subject(), comment({ canEdit: false }))).toBe(false);
179 expect(
180 canEditPullRequestComment(
181 subject(),
182 comment({ author: actor("someone-else"), canEdit: true }),
183 ),
184 ).toBe(true);
185 // The host can't rewrite remarks at all, so its per-remark answer can't widen that.
186 expect(
187 canEditPullRequestComment(
188 subject({ capabilities: capabilities({ changeRequest: true, comment: false }) }),
189 comment({ canEdit: true }),
190 ),
191 ).toBe(false);
192 });
193
149 it("refuses a remark the host attributes to nobody", () => {194 it("refuses a remark the host attributes to nobody", () => {
150 expect(canEditPullRequestComment(subject(), comment({ author: null }))).toBe(false);195 expect(canEditPullRequestComment(subject(), comment({ author: null }))).toBe(false);
151 });196 });
...@@ -170,3 +215,24 @@ describe("canEditPullRequestComment", () => {...@@ -170,3 +215,24 @@ describe("canEditPullRequestComment", () => {
170 expect(canEditPullRequestComment(subject({ viewer: undefined }), comment())).toBe(false);215 expect(canEditPullRequestComment(subject({ viewer: undefined }), comment())).toBe(false);
171 });216 });
172});217});
218
219describe("canResolvePullRequestThread", () => {
220 it("uses the reader's repository-wide permission where the host says nothing per thread", () => {
221 expect(canResolvePullRequestThread(subject(), {})).toBe(true);
222 expect(
223 canResolvePullRequestThread(
224 subject({ viewerPermissions: permissions({ resolve: false }) }),
225 {},
226 ),
227 ).toBe(false);
228 });
229
230 it("only narrows the reader's permission with the host's answer for one thread", () => {
231 expect(canResolvePullRequestThread(subject(), { canResolve: false })).toBe(false);
232 expect(
233 canResolvePullRequestThread(subject({ viewerPermissions: permissions({ resolve: false }) }), {
234 canResolve: true,
235 }),
236 ).toBe(false);
237 });
238});
apps/web/src/components/pullRequest/pullRequestEditing.logic.ts+25-5
...@@ -1,4 +1,8 @@...@@ -1,4 +1,8 @@
1import type { PullRequestComment, PullRequestDetail } from "@t3tools/contracts";1import type {
2 PullRequestComment,
3 PullRequestDetail,
4 PullRequestReviewThread,
5} from "@t3tools/contracts";
26
3/** Only the parts of a detail either answer reads, so a caller can pass a whole detail view. */7/** Only the parts of a detail either answer reads, so a caller can pass a whole detail view. */
4type EditingSubject = Pick<8type EditingSubject = Pick<
...@@ -14,12 +18,15 @@ function sameLogin(one: string | null | undefined, other: string | null | undefi...@@ -14,12 +18,15 @@ function sameLogin(one: string | null | undefined, other: string | null | undefi
1418
15/**19/**
16 * Whether the title and description can be rewritten from here. Beyond the host being able to at20 * Whether the title and description can be rewritten from here. Beyond the host being able to at
17 * all, either the reader wrote the change request or they may merge it — merging is the one action21 * all, a host that reports `editChangeRequest` answers directly. Otherwise either the reader wrote
18 * every host here grants with write access and withholds without it, so it stands in for the22 * the change request or they may merge it: merging is the one action every host here grants with
19 * permission none of them publishes by name.23 * write access and withholds without it, so it stands in for the permission they don't publish.
20 */24 */
21export function canEditPullRequestChangeRequest(detail: EditingSubject): boolean {25export function canEditPullRequestChangeRequest(detail: EditingSubject): boolean {
22 if (detail.capabilities.edit?.changeRequest !== true) return false;26 if (detail.capabilities.edit?.changeRequest !== true) return false;
27 if (detail.viewerPermissions.editChangeRequest !== undefined) {
28 return detail.viewerPermissions.editChangeRequest;
29 }
23 return (30 return (
24 sameLogin(detail.viewer, detail.author?.login) ||31 sameLogin(detail.viewer, detail.author?.login) ||
25 detail.viewerPermissions.actions.includes("merge")32 detail.viewerPermissions.actions.includes("merge")
...@@ -33,9 +40,22 @@ export function canEditPullRequestChangeRequest(detail: EditingSubject): boolean...@@ -33,9 +40,22 @@ export function canEditPullRequestChangeRequest(detail: EditingSubject): boolean
33 */40 */
34export function canEditPullRequestComment(41export function canEditPullRequestComment(
35 detail: EditingSubject,42 detail: EditingSubject,
36 comment: Pick<PullRequestComment, "author" | "kind">,43 comment: Pick<PullRequestComment, "author" | "canEdit" | "kind">,
37): boolean {44): boolean {
38 if (detail.capabilities.edit?.comment !== true) return false;45 if (detail.capabilities.edit?.comment !== true) return false;
39 if (comment.kind !== "issue-comment" && comment.kind !== "review-comment") return false;46 if (comment.kind !== "issue-comment" && comment.kind !== "review-comment") return false;
47 if (comment.canEdit !== undefined) return comment.canEdit;
40 return sameLogin(detail.viewer, comment.author?.login);48 return sameLogin(detail.viewer, comment.author?.login);
41}49}
50
51/**
52 * Whether the reader may resolve or reopen one review thread. The host's answer for this thread
53 * can only narrow the reader's repository-wide `resolve` permission: a thread can't grant what the
54 * repository withholds.
55 */
56export function canResolvePullRequestThread(
57 detail: Pick<PullRequestDetail, "viewerPermissions">,
58 thread: Pick<PullRequestReviewThread, "canResolve">,
59): boolean {
60 return detail.viewerPermissions.resolve && thread.canResolve !== false;
61}
packages/contracts/src/pullRequest.ts+23
...@@ -209,6 +209,12 @@ export const PullRequestComment = Schema.Struct({...@@ -209,6 +209,12 @@ export const PullRequestComment = Schema.Struct({
209 reviewState: Schema.NullOr(Schema.String),209 reviewState: Schema.NullOr(Schema.String),
210 /** Absent from a host with no reactions at all, which is a different thing from none on this. */210 /** Absent from a host with no reactions at all, which is a different thing from none on this. */
211 reactions: Schema.optional(Schema.Array(PullRequestReaction)),211 reactions: Schema.optional(Schema.Array(PullRequestReaction)),
212 /**
213 * The host's own answer to whether this reader may rewrite this remark. Absent where the host
214 * doesn't say, which leaves the page to guess from authorship. A remark on a line appears both
215 * here and in its review thread; a host that sets this sets it on both copies alike.
216 */
217 canEdit: Schema.optional(Schema.Boolean),
212});218});
213export type PullRequestComment = typeof PullRequestComment.Type;219export type PullRequestComment = typeof PullRequestComment.Type;
214220
...@@ -232,6 +238,12 @@ export const PullRequestThreadComment = Schema.Struct({...@@ -232,6 +238,12 @@ export const PullRequestThreadComment = Schema.Struct({
232 editedAt: Schema.optional(Schema.NullOr(IsoDateTime)),238 editedAt: Schema.optional(Schema.NullOr(IsoDateTime)),
233 url: Schema.NullOr(Schema.String),239 url: Schema.NullOr(Schema.String),
234 reactions: Schema.optional(Schema.Array(PullRequestReaction)),240 reactions: Schema.optional(Schema.Array(PullRequestReaction)),
241 /**
242 * The host's own answer to whether this reader may rewrite this remark. Absent where the host
243 * doesn't say, which leaves the page to guess from authorship. A remark on a line appears both
244 * here and in its review thread; a host that sets this sets it on both copies alike.
245 */
246 canEdit: Schema.optional(Schema.Boolean),
235});247});
236export type PullRequestThreadComment = typeof PullRequestThreadComment.Type;248export type PullRequestThreadComment = typeof PullRequestThreadComment.Type;
237249
...@@ -257,6 +269,12 @@ export const PullRequestReviewThread = Schema.Struct({...@@ -257,6 +269,12 @@ export const PullRequestReviewThread = Schema.Struct({
257 commentCount: Schema.optional(NonNegativeInt),269 commentCount: Schema.optional(NonNegativeInt),
258 /** Opaque cursor for the next comment page. Absent once this thread is whole. */270 /** Opaque cursor for the next comment page. Absent once this thread is whole. */
259 nextCommentsCursor: Schema.optional(TrimmedNonEmptyString),271 nextCommentsCursor: Schema.optional(TrimmedNonEmptyString),
272 /**
273 * The host's own answer to whether this reader may resolve this thread. It can only narrow the
274 * reader's repository-wide `resolve` permission, never widen it. Absent where the host
275 * doesn't say per thread.
276 */
277 canResolve: Schema.optional(Schema.Boolean),
260});278});
261export type PullRequestReviewThread = typeof PullRequestReviewThread.Type;279export type PullRequestReviewThread = typeof PullRequestReviewThread.Type;
262280
...@@ -481,6 +499,11 @@ export const PullRequestViewerPermissions = Schema.Struct({...@@ -481,6 +499,11 @@ export const PullRequestViewerPermissions = Schema.Struct({
481 * changed on this host at all.499 * changed on this host at all.
482 */500 */
483 labels: Schema.optional(Schema.Boolean),501 labels: Schema.optional(Schema.Boolean),
502 /**
503 * This viewer may rewrite the change request's title and description. Absent where the host
504 * doesn't say, which leaves the page to guess from authorship and merge access.
505 */
506 editChangeRequest: Schema.optional(Schema.Boolean),
484});507});
485export type PullRequestViewerPermissions = typeof PullRequestViewerPermissions.Type;508export type PullRequestViewerPermissions = typeof PullRequestViewerPermissions.Type;
486509