Skip to content
Closed
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
1 change: 1 addition & 0 deletions apps/server/src/auth/RpcAuthorization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ export const RPC_REQUIRED_SCOPES = {
[WS_METHODS.pullRequestsSubscribeRefreshes]: AuthOrchestrationReadScope,
[WS_METHODS.pullRequestsReviewerCandidates]: AuthOrchestrationReadScope,
[WS_METHODS.pullRequestsLabelCandidates]: AuthOrchestrationReadScope,
[WS_METHODS.pullRequestsAssigneeCandidates]: AuthOrchestrationReadScope,
[WS_METHODS.sourceControlLookupRepository]: AuthOrchestrationReadScope,
[WS_METHODS.subscribeProjectClones]: AuthOrchestrationReadScope,
[WS_METHODS.projectsListEntries]: AuthFilesystemReadScope,
Expand Down
2 changes: 2 additions & 0 deletions apps/server/src/observability/RpcInstrumentation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,8 @@ const RPC_AGGREGATES = {
[WS_METHODS.pullRequestsRequestReviewers]: "pull-requests",
[WS_METHODS.pullRequestsLabelCandidates]: "pull-requests",
[WS_METHODS.pullRequestsSetLabels]: "pull-requests",
[WS_METHODS.pullRequestsAssigneeCandidates]: "pull-requests",
[WS_METHODS.pullRequestsSetAssignees]: "pull-requests",
[WS_METHODS.sourceControlLookupRepository]: "source-control",
[WS_METHODS.sourceControlCloneRepository]: "source-control",
[WS_METHODS.sourceControlPublishRepository]: "source-control",
Expand Down
54 changes: 54 additions & 0 deletions apps/server/src/pullRequest/PullRequestService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3821,3 +3821,57 @@ it.effect("expands only files the change request changes, across every slice of
assert.deepStrictEqual(read, ["src/old.ts->src/new.ts", "gone.md->gone.md"]);
}),
);

it.effect("hands an assignment to the host only where the host and the viewer both allow it", () =>
Effect.gen(function* () {
const received: Array<{ assignees: ReadonlyArray<string>; assigned: boolean }> = [];
const github = SourceControlProviderKind.make("github");
const setup = (options: { capability: boolean; allowed: boolean }) =>
makeService({
projects: [
project({ id: "p1", title: "web", workspaceRoot: "/a", repository: "acme/web" }),
],
providers: [
fakeProvider(github, {
capabilities: {
...fakeProvider(github).capabilities,
...(options.capability ? { assignees: true } : {}),
},
getViewerPermissions: () =>
Effect.succeed({
actions: [],
comment: true,
resolve: false,
verdicts: [],
requestReviewers: false,
assignees: options.allowed,
}),
setAssignees: (input) => {
received.push({ assignees: input.assignees, assigned: input.assigned });
return Effect.void;
},
}),
],
});
const change = {
projectId: "p1" as ProjectId,
repository: "acme/web",
number: 4,
assignees: ["octocat"],
assigned: true,
};

const unsupported = yield* setup({ capability: false, allowed: true });
const unsupportedError = yield* Effect.flip(unsupported.setAssignees(change));
assert.include(unsupportedError.message, "cannot change who a change request is assigned to");

const refused = yield* setup({ capability: true, allowed: false });
const refusedError = yield* Effect.flip(refused.setAssignees(change));
assert.include(refusedError.message, "You need triage access on this repository");
assert.deepStrictEqual(received, []);

const allowed = yield* setup({ capability: true, allowed: true });
yield* allowed.setAssignees(change);
assert.deepStrictEqual(received, [{ assignees: ["octocat"], assigned: true }]);
}),
);
104 changes: 102 additions & 2 deletions apps/server/src/pullRequest/PullRequestService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
PullRequestUnavailableError,
pullRequestHostOf,
pullRequestProviderRequirement,
matchesPullRequestAssigneeFilter,
resolvePullRequestAuthorFilter,
type OrchestrationProjectShell,
type ProjectId,
Expand Down Expand Up @@ -63,6 +64,9 @@ import {
type PullRequestReviewerRequestInput,
type PullRequestLabelCandidateList,
type PullRequestLabelChangeInput,
type PullRequestAssigneeCandidateList,
type PullRequestAssigneeCandidatesInput,
type PullRequestAssigneeChangeInput,
type PullRequestSetFilesViewedInput,
type PullRequestState,
type PullRequestSubmitReviewInput,
Expand Down Expand Up @@ -265,6 +269,12 @@ export class PullRequestService extends Context.Service<
readonly setLabels: (
input: PullRequestLabelChangeInput,
) => Effect.Effect<void, PullRequestError>;
readonly assigneeCandidates: (
input: PullRequestAssigneeCandidatesInput,
) => Effect.Effect<PullRequestAssigneeCandidateList, PullRequestError>;
readonly setAssignees: (
input: PullRequestAssigneeChangeInput,
) => Effect.Effect<void, PullRequestError>;
readonly invalidate: (
input: PullRequestInvalidateInput,
options?: { readonly notifyReaders?: boolean },
Expand Down Expand Up @@ -302,6 +312,8 @@ const ACTION_ACCESS_REFUSALS: Record<PullRequestAction, string> = {

const REVIEWER_REQUEST_REFUSAL = "You need write access on this repository to ask for a review.";
const LABEL_CHANGE_REFUSAL = "You need triage access on this repository to change its labels.";
const ASSIGNEE_CHANGE_REFUSAL =
"You need triage access on this repository to change who it is assigned to.";

export interface SupportedProject {
readonly cursorKey: string;
Expand Down Expand Up @@ -566,6 +578,14 @@ function withRateLimitBackoff(
? {}
: { listLabelCandidates: interactive("listLabelCandidates", api.listLabelCandidates) }),
...(api.setLabels === undefined ? {} : { setLabels: interactive("setLabels", api.setLabels) }),
...(api.listAssigneeCandidates === undefined
? {}
: {
listAssigneeCandidates: interactive("listAssigneeCandidates", api.listAssigneeCandidates),
}),
...(api.setAssignees === undefined
? {}
: { setAssignees: interactive("setAssignees", api.setAssignees) }),
replyToThread: interactive("replyToThread", api.replyToThread),
setReaction: interactive("setReaction", api.setReaction),
setThreadResolution: interactive("setThreadResolution", api.setThreadResolution),
Expand Down Expand Up @@ -1031,7 +1051,8 @@ export const make = Effect.gen(function* () {
(filters.excludedLabels === undefined || !filters.excludedLabels.some(holds)) &&
(filters.author === undefined ||
item.author?.login.toLowerCase() ===
resolvePullRequestAuthorFilter(filters.author, viewer).toLowerCase())
resolvePullRequestAuthorFilter(filters.author, viewer).toLowerCase()) &&
matchesPullRequestAssigneeFilter(item.assignees, filters.assignee, viewer)
);
};

Expand Down Expand Up @@ -1070,6 +1091,7 @@ export const make = Effect.gen(function* () {
input.item.author?.login.toLowerCase() !== viewer &&
input.item.reviewRequestLogins.some((login) => login.toLowerCase() === viewer),
labels: input.item.labels,
...(input.item.assignees === undefined ? {} : { assignees: input.item.assignees }),
...(input.item.reviewDecision === undefined || input.item.reviewDecision === null
? {}
: { reviewDecision: input.item.reviewDecision }),
Expand Down Expand Up @@ -1593,6 +1615,9 @@ export const make = Effect.gen(function* () {
mergedAt: changeRequest.mergedAt,
closedAt: changeRequest.closedAt,
reviewers: changeRequest.reviewers,
...(changeRequest.assignees === undefined
? {}
: { assignees: changeRequest.assignees }),
labels: changeRequest.labels,
checks: changeRequest.checks,
mergeCapabilities: changeRequest.mergeCapabilities,
Expand Down Expand Up @@ -2350,6 +2375,77 @@ export const make = Effect.gen(function* () {
}),
);

/** Guarded like the labels: the list is wanted only by somebody about to change it. */
const assigneeCandidates: PullRequestService["Service"]["assigneeCandidates"] = (input) =>
requireProject(input).pipe(
Effect.flatMap(
(project): Effect.Effect<PullRequestAssigneeCandidateList, PullRequestError> => {
const list = project.api.listAssigneeCandidates;
if (project.api.capabilities.assignees !== true || list === undefined) {
return Effect.fail(
new PullRequestOperationError({
operation: "assigneeCandidates",
detail: "This host cannot change who a change request is assigned to.",
}),
);
}
return viewerPermissionsOf(project, input, "assigneeCandidates").pipe(
Effect.flatMap(
(viewer): Effect.Effect<PullRequestAssigneeCandidateList, PullRequestError> =>
viewer.assignees === false
? Effect.fail(
new PullRequestOperationError({
operation: "assigneeCandidates",
detail: ASSIGNEE_CHANGE_REFUSAL,
}),
)
: list({
cwd: project.project.workspaceRoot,
repository: project.repository,
host: project.host,
number: input.number,
query: input.query,
}).pipe(Effect.mapError(toPullRequestError("assigneeCandidates"))),
),
);
},
),
);

const setAssignees: PullRequestService["Service"]["setAssignees"] = (input) =>
requireProject(input).pipe(
Effect.flatMap((project): Effect.Effect<void, PullRequestError> => {
const change = project.api.setAssignees;
if (project.api.capabilities.assignees !== true || change === undefined) {
return Effect.fail(
new PullRequestOperationError({
operation: "setAssignees",
detail: "This host cannot change who a change request is assigned to.",
}),
);
}
return viewerPermissionsOf(project, input, "setAssignees").pipe(
Effect.flatMap((viewer): Effect.Effect<void, PullRequestError> =>
viewer.assignees === false
? Effect.fail(
new PullRequestOperationError({
operation: "setAssignees",
detail: ASSIGNEE_CHANGE_REFUSAL,
}),
)
: change({
cwd: project.project.workspaceRoot,
repository: project.repository,
host: project.host,
number: input.number,
assignees: input.assignees,
assigned: input.assigned,
}).pipe(Effect.mapError(toPullRequestError("setAssignees"))),
),
);
}),
);

/**
* The line counts for rows already on the page, which the listing left out because on GitHub
* they cost more than everything else on the row put together.
Expand Down Expand Up @@ -2620,14 +2716,15 @@ export const make = Effect.gen(function* () {
string | ReadonlyArray<string> | ReadonlyArray<ReadonlyArray<string>> | null
>,
): PullRequestListFilters => {
const [draft, review, checks, author, labels, excludedLabels] = slots;
const [draft, review, checks, author, labels, excludedLabels, assignee] = slots;
return {
...(typeof draft === "string" ? { draft: draft as "only" | "hide" } : {}),
...(typeof review === "string" ? { review: review as PullRequestListFilters["review"] } : {}),
...(typeof checks === "string" ? { checks: checks as PullRequestListFilters["checks"] } : {}),
...(typeof author === "string" ? { author } : {}),
...(Array.isArray(labels) ? { labels: labels as ReadonlyArray<ReadonlyArray<string>> } : {}),
...(Array.isArray(excludedLabels) ? { excludedLabels } : {}),
...(typeof assignee === "string" ? { assignee } : {}),
};
};

Expand Down Expand Up @@ -2755,6 +2852,7 @@ export const make = Effect.gen(function* () {
input.filters.author ?? null,
input.filters.labels ?? null,
input.filters.excludedLabels ?? null,
input.filters.assignee ?? null,
],
input.projectId ?? null,
input.projectIds === undefined ? null : [...input.projectIds].sort(),
Expand Down Expand Up @@ -3415,6 +3513,8 @@ export const make = Effect.gen(function* () {
requestReviewers: invalidatedByMutation(requestReviewers),
labelCandidates,
setLabels: invalidatedByMutation(setLabels),
assigneeCandidates,
setAssignees: invalidatedByMutation(setAssignees),
invalidate,
});
});
Expand Down
4 changes: 4 additions & 0 deletions apps/server/src/ws.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2544,6 +2544,10 @@ export const layerWsRpc = (
withPullRequestViewer(input, pullRequests.labelCandidates(input)),
[WS_METHODS.pullRequestsSetLabels]: (input) =>
withPullRequestViewer(input, pullRequests.setLabels(input)),
[WS_METHODS.pullRequestsAssigneeCandidates]: (input) =>
withPullRequestViewer(input, pullRequests.assigneeCandidates(input)),
[WS_METHODS.pullRequestsSetAssignees]: (input) =>
withPullRequestViewer(input, pullRequests.setAssignees(input)),
[WS_METHODS.sourceControlLookupRepository]: (input) =>
sourceControlRepositories.lookupRepository(input),
[WS_METHODS.sourceControlCloneRepository]: (input) =>
Expand Down
Loading
Loading