Skip to content

Commit bcefea2

Browse files
authored
Merge pull request #14 from pylon-code/upstream/2026-08-13-pr-surfaces
feat(web): adopt upstream pull request surfaces
2 parents ab164e5 + cda705f commit bcefea2

86 files changed

Lines changed: 11987 additions & 627 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

apps/server/src/auth/RpcAuthorization.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,13 @@ export const RPC_REQUIRED_SCOPES = {
6161
[WS_METHODS.pullRequestsActivity]: AuthOrchestrationReadScope,
6262
[WS_METHODS.pullRequestsDiffFileContents]: AuthOrchestrationReadScope,
6363
[WS_METHODS.pullRequestsRunAction]: AuthOrchestrationOperateScope,
64+
[WS_METHODS.pullRequestsUpdate]: AuthOrchestrationOperateScope,
6465
[WS_METHODS.pullRequestsComment]: AuthOrchestrationOperateScope,
66+
[WS_METHODS.pullRequestsUpdateComment]: AuthOrchestrationOperateScope,
6567
[WS_METHODS.pullRequestsSubmitReview]: AuthOrchestrationOperateScope,
6668
[WS_METHODS.pullRequestsReplyToThread]: AuthOrchestrationOperateScope,
6769
[WS_METHODS.pullRequestsSetThreadResolution]: AuthOrchestrationOperateScope,
70+
[WS_METHODS.pullRequestsSetReaction]: AuthOrchestrationOperateScope,
6871
// Read scope like the reads it un-caches: refreshing is part of reading, and a read-only
6972
// client pressing refresh must not be told it may not look again.
7073
[WS_METHODS.pullRequestsInvalidate]: AuthOrchestrationReadScope,

apps/server/src/git/GitManager.test.ts

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,10 @@ import * as Duration from "effect/Duration";
99
import * as Effect from "effect/Effect";
1010
import * as FileSystem from "effect/FileSystem";
1111
import * as Layer from "effect/Layer";
12+
import * as Logger from "effect/Logger";
1213
import * as Option from "effect/Option";
1314
import * as PlatformError from "effect/PlatformError";
15+
import * as References from "effect/References";
1416
import * as Scope from "effect/Scope";
1517
import { ChildProcessSpawner } from "effect/unstable/process";
1618
import { expect } from "vite-plus/test";
@@ -1439,6 +1441,58 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => {
14391441
}),
14401442
);
14411443

1444+
it.effect("status logs actionable provider detail without exposing the upstream cause", () =>
1445+
Effect.gen(function* () {
1446+
const repoDir = yield* makeTempDir("t3code-git-manager-");
1447+
yield* initRepo(repoDir);
1448+
yield* runGit(repoDir, ["checkout", "-b", "feature/status-rate-limited"]);
1449+
const remoteDir = yield* createBareRemote();
1450+
yield* runGit(repoDir, ["remote", "add", "origin", remoteDir]);
1451+
yield* runGit(repoDir, ["push", "-u", "origin", "feature/status-rate-limited"]);
1452+
1453+
const upstreamCause = "GraphQL rate limit for user ID 51714798 and token secret-value";
1454+
const { manager } = yield* makeManager({
1455+
ghScenario: {
1456+
failWith: new GitHubCli.GitHubCliRateLimitError({
1457+
command: "gh",
1458+
cwd: repoDir,
1459+
cause: new Error(upstreamCause),
1460+
}),
1461+
},
1462+
});
1463+
const logs: Array<{ message: string; annotations: Record<string, unknown> }> = [];
1464+
const logger = Logger.make<unknown, void>(({ fiber, message }) => {
1465+
logs.push({
1466+
message: String(message),
1467+
annotations: { ...fiber.getRef(References.CurrentLogAnnotations) },
1468+
});
1469+
});
1470+
1471+
const status = yield* manager
1472+
.status({ cwd: repoDir })
1473+
.pipe(Effect.provide(Logger.layer([logger], { mergeWithExisting: false })));
1474+
1475+
expect(status.pr).toBeNull();
1476+
const warning = logs.find((entry) => entry.message.includes("PR lookup failed"));
1477+
expect(warning?.annotations).toMatchObject({
1478+
operation: "lookupStatusPr",
1479+
branch: "feature/status-rate-limited",
1480+
errorTag: "SourceControlProviderError",
1481+
provider: "github",
1482+
providerOperation: "listChangeRequests",
1483+
providerCommand: "gh",
1484+
errorDetail:
1485+
"GitHub API rate limit exceeded. Run `gh api rate_limit` to inspect the quota and reset time.",
1486+
});
1487+
const loggedText = [
1488+
warning?.message ?? "",
1489+
...Object.values(warning?.annotations ?? {}).map(String),
1490+
].join("\n");
1491+
expect(loggedText).not.toContain(upstreamCause);
1492+
expect(loggedText).not.toContain("secret-value");
1493+
}),
1494+
);
1495+
14421496
it.effect("status keeps the last known PR when a later lookup fails", () =>
14431497
Effect.gen(function* () {
14441498
const repoDir = yield* makeTempDir("t3code-git-manager-");

apps/server/src/git/GitManager.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import * as Option from "effect/Option";
1212
import * as Order from "effect/Order";
1313
import * as Path from "effect/Path";
1414
import * as Ref from "effect/Ref";
15+
import * as Schema from "effect/Schema";
1516
import {
1617
GitActionProgressEvent,
1718
GitActionProgressPhase,
@@ -28,6 +29,7 @@ import {
2829
type VcsStatusRemoteResult,
2930
VcsStatusResult,
3031
ModelSelection,
32+
SourceControlProviderError,
3133
type SourceControlWritingStyleSettings,
3234
} from "@t3tools/contracts";
3335
import {
@@ -113,6 +115,7 @@ const PR_LOOKUP_CACHE_TTL = Duration.minutes(2);
113115
const PR_LOOKUP_FAILURE_BASE_TTL = Duration.seconds(20);
114116
const PR_LOOKUP_FAILURE_MAX_TTL = Duration.minutes(15);
115117
const PR_LOOKUP_CACHE_CAPACITY = 2_048;
118+
const isSourceControlProviderError = Schema.is(SourceControlProviderError);
116119

117120
/**
118121
* How long a failed PR lookup is cached, given the number of consecutive
@@ -1066,6 +1069,14 @@ export const make = Effect.gen(function* () {
10661069
typeof error === "object" && error !== null && "_tag" in error
10671070
? String(error._tag)
10681071
: typeof error,
1072+
...(isSourceControlProviderError(error)
1073+
? {
1074+
provider: error.provider,
1075+
providerOperation: error.operation,
1076+
providerCommand: error.command ?? "unknown",
1077+
errorDetail: error.detail,
1078+
}
1079+
: {}),
10691080
}),
10701081
Effect.andThen(resolveBranchHeadContext(cwd, details)),
10711082
Effect.map((headContext) =>

apps/server/src/pullRequest/AzureDevOpsPullRequestCli.test.ts

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,40 @@ layer("AzureDevOpsPullRequestCli.layer", (it) => {
342342
}),
343343
);
344344

345+
it.effect("stores the squash choice with an auto-completion, as a merge now does", () =>
346+
Effect.gen(function* () {
347+
mockedExecute.mockReturnValue(Effect.succeed(output("{}")));
348+
const cli = yield* AzureDevOpsPullRequestCli.AzureDevOpsPullRequestCli;
349+
350+
yield* cli.runPullRequestAction({
351+
cwd: "/w",
352+
number: 42,
353+
action: "enable-auto-merge",
354+
mergeMethod: "squash",
355+
});
356+
357+
expect(argsOfCall(0)).toEqual([
358+
"repos",
359+
"pr",
360+
"update",
361+
"--detect",
362+
"true",
363+
"--id",
364+
"42",
365+
"--auto-complete",
366+
"true",
367+
"--squash",
368+
"true",
369+
"--only-show-errors",
370+
"--output",
371+
"json",
372+
]);
373+
}),
374+
);
375+
345376
it.effect.each([
377+
{ action: "enable-auto-merge", expected: ["--auto-complete", "true", "--squash", "false"] },
378+
{ action: "disable-auto-merge", expected: ["--auto-complete", "false"] },
346379
{ action: "draft", expected: ["--draft", "true"] },
347380
{ action: "ready", expected: ["--draft", "false"] },
348381
{ action: "close", expected: ["--status", "abandoned"] },
@@ -370,6 +403,79 @@ layer("AzureDevOpsPullRequestCli.layer", (it) => {
370403
}),
371404
);
372405

406+
it.effect.each([
407+
{ name: "a title", rewrite: { title: "Add the page" }, expected: ["--title=Add the page"] },
408+
{
409+
name: "a description",
410+
rewrite: { body: "Why the page changed" },
411+
expected: ["--description=Why the page changed"],
412+
},
413+
{
414+
name: "both",
415+
rewrite: { title: "Add the page", body: "Why the page changed" },
416+
expected: ["--title=Add the page", "--description=Why the page changed"],
417+
},
418+
] as const)("rewrites $name, sending nothing it was not given", ({ rewrite, expected }) =>
419+
Effect.gen(function* () {
420+
mockedExecute.mockReturnValue(Effect.succeed(output("{}")));
421+
const cli = yield* AzureDevOpsPullRequestCli.AzureDevOpsPullRequestCli;
422+
423+
yield* cli.updatePullRequest({ cwd: "/w", number: 42, ...rewrite });
424+
425+
expect(argsOfCall(0)).toEqual([
426+
"repos",
427+
"pr",
428+
"update",
429+
"--detect",
430+
"true",
431+
"--id",
432+
"42",
433+
...expected,
434+
"--only-show-errors",
435+
"--output",
436+
"json",
437+
]);
438+
}),
439+
);
440+
441+
it.effect("sends a description that starts with a dash as one value, not as a flag", () =>
442+
Effect.gen(function* () {
443+
mockedExecute.mockReturnValue(Effect.succeed(output("{}")));
444+
const cli = yield* AzureDevOpsPullRequestCli.AzureDevOpsPullRequestCli;
445+
446+
yield* cli.updatePullRequest({
447+
cwd: "/w",
448+
number: 42,
449+
body: "- rewrote the page\n- kept the rest",
450+
});
451+
452+
// One argument, so the leading dash of an ordinary bullet list never reaches az as a flag,
453+
// and the whole text stays together where `--description` would otherwise take several.
454+
expect(argsOfCall(0)).toContain("--description=- rewrote the page\n- kept the rest");
455+
}),
456+
);
457+
458+
it.effect("rewrites through the provider, which says it takes one", () =>
459+
Effect.gen(function* () {
460+
mockedExecute.mockReturnValue(Effect.succeed(output("{}")));
461+
const provider = yield* AzureDevOpsPullRequestProvider.make;
462+
463+
// False for a remark because nothing here can post one, so there is none to rewrite.
464+
expect(provider.capabilities.edit).toEqual({ changeRequest: true, comment: false });
465+
assert.isDefined(provider.updateChangeRequest);
466+
yield* provider.updateChangeRequest({
467+
cwd: "/w",
468+
repository: "web",
469+
host: "dev.azure.com",
470+
number: 42,
471+
title: "Add the page",
472+
});
473+
474+
expect(argsOfCall(0)).toContain("--title=Add the page");
475+
expect(argsOfCall(0)).not.toContain("--description");
476+
}),
477+
);
478+
373479
it.effect("reads the conversation through the REST API, pinned to a version", () =>
374480
Effect.gen(function* () {
375481
mockedExecute.mockReturnValueOnce(

apps/server/src/pullRequest/AzureDevOpsPullRequestCli.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,14 @@ export class AzureDevOpsPullRequestCli extends Context.Service<
159159
readonly mergeMethod?: PullRequestMergeMethod;
160160
}) => Effect.Effect<void, AzureDevOpsPullRequestCliError>;
161161

162+
/** Rewrites the pull request's own words, through the same command that moves it. */
163+
readonly updatePullRequest: (input: {
164+
readonly cwd: string;
165+
readonly number: number;
166+
readonly title?: string | undefined;
167+
readonly body?: string | undefined;
168+
}) => Effect.Effect<void, AzureDevOpsPullRequestCliError>;
169+
162170
/**
163171
* Adds reviewers to a pull request, or takes them off it. `az repos pr reviewer` is the whole
164172
* of what Azure offers here: it adds and removes named identities, and has no counterpart that
@@ -212,12 +220,21 @@ function actionArgs(
212220
switch (action) {
213221
case "merge":
214222
return ["--status", "completed", "--squash", mergeMethod === "squash" ? "true" : "false"];
223+
// Auto-complete is Azure's own name for it: the pull request stays active and Azure completes
224+
// it once its policies pass. The squash choice is stored with it, as it is for a merge now.
225+
case "enable-auto-merge":
226+
return ["--auto-complete", "true", "--squash", mergeMethod === "squash" ? "true" : "false"];
227+
case "disable-auto-merge":
228+
return ["--auto-complete", "false"];
215229
case "ready":
216230
return ["--draft", "false"];
217231
case "draft":
218232
return ["--draft", "true"];
219233
case "close":
220234
return ["--status", "abandoned"];
235+
// Never reached: this host does not declare the action, so nothing offers it.
236+
case "update-branch":
237+
return [];
221238
case "reopen":
222239
return ["--status", "active"];
223240
}
@@ -481,6 +498,29 @@ export const make = Effect.gen(function* () {
481498
],
482499
})
483500
.pipe(Effect.asVoid),
501+
502+
updatePullRequest: (input) =>
503+
azure
504+
.execute({
505+
cwd: input.cwd,
506+
args: [
507+
"repos",
508+
"pr",
509+
"update",
510+
...detectArgs,
511+
"--id",
512+
String(input.number),
513+
// One argument rather than a flag and a value beside it: a description usually opens
514+
// with a bullet, and az reads a dash in the next argv slot as a flag of its own.
515+
// `--description` also takes several strings, and this keeps the whole text as one.
516+
...(input.title === undefined ? [] : [`--title=${input.title}`]),
517+
...(input.body === undefined ? [] : [`--description=${input.body}`]),
518+
"--only-show-errors",
519+
"--output",
520+
"json",
521+
],
522+
})
523+
.pipe(Effect.asVoid),
484524
});
485525
});
486526

apps/server/src/pullRequest/AzureDevOpsPullRequestProvider.test.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,15 @@ describe("azure devops viewer permissions", () => {
99
// and an unknown permission is granted rather than guessed away. Azure refuses the ones it
1010
// will not allow, at the moment they are taken, in words this could not have written.
1111
expect(AZURE_DEVOPS_VIEWER_PERMISSIONS).toEqual({
12-
actions: ["merge", "ready", "draft", "close", "reopen"],
12+
actions: [
13+
"merge",
14+
"ready",
15+
"draft",
16+
"close",
17+
"reopen",
18+
"enable-auto-merge",
19+
"disable-auto-merge",
20+
],
1321
// False because the host itself cannot post one, not because this viewer may not.
1422
comment: false,
1523
resolve: false,

apps/server/src/pullRequest/AzureDevOpsPullRequestProvider.ts

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,18 +18,31 @@ const CAPABILITIES: PullRequestCapabilities = {
1818
// Reading a conversation is a plain REST read, but posting one is not something this can
1919
// claim without having run it, so the composer stays hidden.
2020
comment: false,
21-
actions: ["merge", "ready", "draft", "close", "reopen"],
21+
actions: [
22+
"merge",
23+
"ready",
24+
"draft",
25+
"close",
26+
"reopen",
27+
"enable-auto-merge",
28+
"disable-auto-merge",
29+
],
2230
// Azure squashes as a completion option; it has no rebase strategy of its own.
2331
mergeMethods: ["merge", "squash"],
2432
// `az repos pr list` filters by status, creator, reviewer and branch, and by no text at all.
2533
search: false,
34+
reactions: false,
2635
// With no patch to show there are no lines to write against, so nothing here is offered.
2736
review: { inlineComment: false, reply: false, resolve: false, verdicts: [] },
2837
// `az repos pr reviewer add` and `remove` name identities, and nothing anywhere in `az repos`
2938
// lists the ones this repository could name — that lives behind the identity and graph APIs, a
3039
// different service with its own permissions. So the page takes a name here rather than being
3140
// handed a menu built out of a guess.
3241
reviewers: { request: true, listCandidates: false },
42+
// A new title and description travel on the same `az repos pr update` that moves a pull request.
43+
// Rewriting a remark is false for the same reason posting one is: this cannot put a remark on
44+
// Azure DevOps at all, so there is nothing here it could rewrite either.
45+
edit: { changeRequest: true, comment: false },
3346
};
3447

3548
/**
@@ -153,6 +166,7 @@ export const make = Effect.gen(function* () {
153166
checks: [],
154167
mergeCapabilities: { merge: true, squash: true, rebase: false },
155168
viewerPermissions: AZURE_DEVOPS_VIEWER_PERMISSIONS,
169+
autoMergeEnabled: pullRequest.autoMergeEnabled,
156170
}),
157171
),
158172
),
@@ -206,6 +220,16 @@ export const make = Effect.gen(function* () {
206220
})
207221
.pipe(Effect.mapError(fail("runAction"))),
208222

223+
updateChangeRequest: (input) =>
224+
cli
225+
.updatePullRequest({
226+
cwd: input.cwd,
227+
number: input.number,
228+
title: input.title,
229+
body: input.body,
230+
})
231+
.pipe(Effect.mapError(fail("updateChangeRequest"))),
232+
209233
// Never called: `capabilities.reviewers.listCandidates` is false, and the service refuses the
210234
// list without it.
211235
listReviewerCandidates: () =>
@@ -240,6 +264,8 @@ export const make = Effect.gen(function* () {
240264
replyToThread: () => unsupported("replyToThread"),
241265

242266
setThreadResolution: () => unsupported("setThreadResolution"),
267+
268+
setReaction: () => unsupported("setReaction"),
243269
};
244270

245271
return provider;

0 commit comments

Comments
 (0)