| author | |
| committer | |
| log | 443b4ebfe83fcfe64c34b09ecb5a5fffdebb85c7 |
| tree | de97bf11921339582f599d9f1eee2f074e3214f9 |
| parent | 4ba39a6f408bdee468df2e3ffdf7d5dc08e7b59d |
| signature | Signed by PGP key B5690EEEBB952194 |
Co-authored-by: maria-rcks <254055478+maria-rcks@users.noreply.github.com>34 files changed, 2241 insertions(+), 205 deletions(-)
apps/server/src/pullRequest/AzureDevOpsPullRequestCli.test.ts+1-1| ... | @@ -414,7 +414,7 @@ layer("AzureDevOpsPullRequestCli.layer", (it) => { | ... | @@ -414,7 +414,7 @@ layer("AzureDevOpsPullRequestCli.layer", (it) => { |
| 414 | ); | 414 | ); |
| 415 | 415 | ||
| 416 | it.effect.each([ | 416 | it.effect.each([ |
| 417 | { action: "enable-auto-merge", expected: ["--auto-complete", "true", "--squash", "false"] }, | 417 | { action: "enable-auto-merge", expected: ["--auto-complete", "true"] }, |
| 418 | { action: "disable-auto-merge", expected: ["--auto-complete", "false"] }, | 418 | { action: "disable-auto-merge", expected: ["--auto-complete", "false"] }, |
| 419 | { action: "draft", expected: ["--draft", "true"] }, | 419 | { action: "draft", expected: ["--draft", "true"] }, |
| 420 | { action: "ready", expected: ["--draft", "false"] }, | 420 | { action: "ready", expected: ["--draft", "false"] }, |
apps/server/src/pullRequest/AzureDevOpsPullRequestCli.ts+11-1| ... | @@ -224,7 +224,13 @@ function actionArgs( | ... | @@ -224,7 +224,13 @@ function actionArgs( |
| 224 | // Auto-complete is Azure's own name for it: the pull request stays active and Azure completes | 224 | // Auto-complete is Azure's own name for it: the pull request stays active and Azure completes |
| 225 | // it once its policies pass. The squash choice is stored with it, as it is for a merge now. | 225 | // it once its policies pass. The squash choice is stored with it, as it is for a merge now. |
| 226 | case "enable-auto-merge": | 226 | case "enable-auto-merge": |
| 227 | return ["--auto-complete", "true", "--squash", mergeMethod === "squash" ? "true" : "false"]; | 227 | return [ |
| 228 | "--auto-complete", | ||
| 229 | "true", | ||
| 230 | ...(mergeMethod === undefined | ||
| 231 | ? [] | ||
| 232 | : ["--squash", mergeMethod === "squash" ? "true" : "false"]), | ||
| 233 | ]; | ||
| 228 | case "disable-auto-merge": | 234 | case "disable-auto-merge": |
| 229 | return ["--auto-complete", "false"]; | 235 | return ["--auto-complete", "false"]; |
| 230 | case "ready": | 236 | case "ready": |
| ... | @@ -238,6 +244,10 @@ function actionArgs( | ... | @@ -238,6 +244,10 @@ function actionArgs( |
| 238 | return []; | 244 | return []; |
| 239 | case "reopen": | 245 | case "reopen": |
| 240 | return ["--status", "active"]; | 246 | return ["--status", "active"]; |
| 247 | // Never reached: this host does not declare the action, so the service refuses it first. | ||
| 248 | case "revert": | ||
| 249 | case "approve-workflows": | ||
| 250 | throw new Error(`Azure DevOps pull request action ${action} is unsupported`); | ||
| 241 | } | 251 | } |
| 242 | } | 252 | } |
| 243 | 253 |
apps/server/src/pullRequest/AzureDevOpsPullRequestProvider.ts+3| ... | @@ -169,6 +169,9 @@ export const make = Effect.gen(function* () { | ... | @@ -169,6 +169,9 @@ export const make = Effect.gen(function* () { |
| 169 | mergeCapabilities: { merge: true, squash: true, rebase: false }, | 169 | mergeCapabilities: { merge: true, squash: true, rebase: false }, |
| 170 | viewerPermissions: AZURE_DEVOPS_VIEWER_PERMISSIONS, | 170 | viewerPermissions: AZURE_DEVOPS_VIEWER_PERMISSIONS, |
| 171 | autoMergeEnabled: pullRequest.autoMergeEnabled, | 171 | autoMergeEnabled: pullRequest.autoMergeEnabled, |
| 172 | ...(pullRequest.autoMergeMethod === undefined | ||
| 173 | ? {} | ||
| 174 | : { autoMergeMethod: pullRequest.autoMergeMethod }), | ||
| 172 | }), | 175 | }), |
| 173 | ), | 176 | ), |
| 174 | ), | 177 | ), |
apps/server/src/pullRequest/GitHubPullRequestCli.test.ts+384-1| ... | @@ -1200,6 +1200,389 @@ layer("GitHubPullRequestCli.layer", (it) => { | ... | @@ -1200,6 +1200,389 @@ layer("GitHubPullRequestCli.layer", (it) => { |
| 1200 | }), | 1200 | }), |
| 1201 | ); | 1201 | ); |
| 1202 | 1202 | ||
| 1203 | it.effect("opens a pull request that reverts a merged pull request", () => | ||
| 1204 | Effect.gen(function* () { | ||
| 1205 | mockedExecute.mockReturnValueOnce( | ||
| 1206 | Effect.succeed( | ||
| 1207 | output( | ||
| 1208 | // @effect-diagnostics-next-line preferSchemaOverJson:off - canned gh GraphQL response. | ||
| 1209 | JSON.stringify({ | ||
| 1210 | data: { repository: { pullRequest: { id: "PR_7" } } }, | ||
| 1211 | }), | ||
| 1212 | ), | ||
| 1213 | ), | ||
| 1214 | ); | ||
| 1215 | mockedExecute.mockReturnValueOnce(Effect.succeed(output("{}"))); | ||
| 1216 | const cli = yield* GitHubPullRequestCli.GitHubPullRequestCli; | ||
| 1217 | |||
| 1218 | yield* cli.runPullRequestAction({ | ||
| 1219 | cwd: "/w", | ||
| 1220 | repository: "acme/web", | ||
| 1221 | host: "github.com", | ||
| 1222 | number: 7, | ||
| 1223 | action: "revert", | ||
| 1224 | }); | ||
| 1225 | |||
| 1226 | expect(callAt(0).args).toContain("owner=acme"); | ||
| 1227 | expect(callAt(0).args).toContain("name=web"); | ||
| 1228 | expect(callAt(0).args).toContain("number=7"); | ||
| 1229 | expect(callAt(1).args).toEqual([ | ||
| 1230 | "api", | ||
| 1231 | "graphql", | ||
| 1232 | "--hostname", | ||
| 1233 | "github.com", | ||
| 1234 | "--input", | ||
| 1235 | "-", | ||
| 1236 | ]); | ||
| 1237 | expect(callAt(1).stdin).toContain("revertPullRequest"); | ||
| 1238 | expect(callAt(1).stdin).toContain('"pullRequestId":"PR_7"'); | ||
| 1239 | }), | ||
| 1240 | ); | ||
| 1241 | |||
| 1242 | it.effect("does not approve action-required runs for a same-repository pull request", () => | ||
| 1243 | Effect.gen(function* () { | ||
| 1244 | mockedExecute.mockReturnValueOnce( | ||
| 1245 | Effect.succeed( | ||
| 1246 | output( | ||
| 1247 | // @effect-diagnostics-next-line preferSchemaOverJson:off - canned gh response. | ||
| 1248 | JSON.stringify({ | ||
| 1249 | number: 7, | ||
| 1250 | title: "Pull request 7", | ||
| 1251 | url: "https://github.com/acme/web/pull/7", | ||
| 1252 | headRefName: "feat/page", | ||
| 1253 | headRefOid: "abc123", | ||
| 1254 | isCrossRepository: false, | ||
| 1255 | headRepositoryOwner: { login: "acme" }, | ||
| 1256 | baseRefName: "main", | ||
| 1257 | createdAt: "2026-07-01T00:00:00Z", | ||
| 1258 | updatedAt: "2026-07-02T00:00:00Z", | ||
| 1259 | }), | ||
| 1260 | ), | ||
| 1261 | ), | ||
| 1262 | ); | ||
| 1263 | const cli = yield* GitHubPullRequestCli.GitHubPullRequestCli; | ||
| 1264 | |||
| 1265 | yield* cli.runPullRequestAction({ | ||
| 1266 | cwd: "/w", | ||
| 1267 | repository: "acme/web", | ||
| 1268 | host: "github.com", | ||
| 1269 | number: 7, | ||
| 1270 | action: "approve-workflows", | ||
| 1271 | }); | ||
| 1272 | |||
| 1273 | expect(mockedExecute).toHaveBeenCalledTimes(1); | ||
| 1274 | }), | ||
| 1275 | ); | ||
| 1276 | |||
| 1277 | it.effect("finds and approves every workflow waiting on a maintainer", () => | ||
| 1278 | Effect.gen(function* () { | ||
| 1279 | const detail = output( | ||
| 1280 | // @effect-diagnostics-next-line preferSchemaOverJson:off - canned gh response. | ||
| 1281 | JSON.stringify({ | ||
| 1282 | number: 7, | ||
| 1283 | title: "Pull request 7", | ||
| 1284 | url: "https://github.com/acme/web/pull/7", | ||
| 1285 | headRefName: "feat/page", | ||
| 1286 | headRefOid: "abc123", | ||
| 1287 | isCrossRepository: true, | ||
| 1288 | headRepositoryOwner: { login: "octocat" }, | ||
| 1289 | baseRefName: "main", | ||
| 1290 | createdAt: "2026-07-01T00:00:00Z", | ||
| 1291 | updatedAt: "2026-07-02T00:00:00Z", | ||
| 1292 | }), | ||
| 1293 | ); | ||
| 1294 | const heads = output( | ||
| 1295 | // @effect-diagnostics-next-line preferSchemaOverJson:off - canned gh response. | ||
| 1296 | JSON.stringify([ | ||
| 1297 | { | ||
| 1298 | number: 7, | ||
| 1299 | headRefOid: "abc123", | ||
| 1300 | isCrossRepository: true, | ||
| 1301 | headRepositoryOwner: { login: "octocat" }, | ||
| 1302 | }, | ||
| 1303 | ]), | ||
| 1304 | ); | ||
| 1305 | const runs = output( | ||
| 1306 | // @effect-diagnostics-next-line preferSchemaOverJson:off - canned gh response. | ||
| 1307 | JSON.stringify([ | ||
| 1308 | { databaseId: 10, workflowName: "build", url: "https://example.com/10" }, | ||
| 1309 | { databaseId: 11, workflowName: "test", url: "https://example.com/11" }, | ||
| 1310 | ]), | ||
| 1311 | ); | ||
| 1312 | for (const result of [ | ||
| 1313 | detail, | ||
| 1314 | heads, | ||
| 1315 | runs, | ||
| 1316 | detail, | ||
| 1317 | heads, | ||
| 1318 | runs, | ||
| 1319 | output(""), | ||
| 1320 | detail, | ||
| 1321 | heads, | ||
| 1322 | runs, | ||
| 1323 | output(""), | ||
| 1324 | ]) { | ||
| 1325 | mockedExecute.mockReturnValueOnce(Effect.succeed(result)); | ||
| 1326 | } | ||
| 1327 | const cli = yield* GitHubPullRequestCli.GitHubPullRequestCli; | ||
| 1328 | |||
| 1329 | yield* cli.runPullRequestAction({ | ||
| 1330 | cwd: "/w", | ||
| 1331 | repository: "acme/web", | ||
| 1332 | host: "github.com", | ||
| 1333 | number: 7, | ||
| 1334 | action: "approve-workflows", | ||
| 1335 | }); | ||
| 1336 | |||
| 1337 | expect(callAt(1).args).toEqual([ | ||
| 1338 | "pr", | ||
| 1339 | "list", | ||
| 1340 | "--repo", | ||
| 1341 | "github.com/acme/web", | ||
| 1342 | "--state", | ||
| 1343 | "open", | ||
| 1344 | "--head", | ||
| 1345 | "feat/page", | ||
| 1346 | "--limit", | ||
| 1347 | "1001", | ||
| 1348 | "--json", | ||
| 1349 | "number,headRefOid,isCrossRepository,headRepositoryOwner", | ||
| 1350 | ]); | ||
| 1351 | expect(callAt(2).args).toEqual([ | ||
| 1352 | "run", | ||
| 1353 | "list", | ||
| 1354 | "--repo", | ||
| 1355 | "github.com/acme/web", | ||
| 1356 | "--commit", | ||
| 1357 | "abc123", | ||
| 1358 | "--branch", | ||
| 1359 | "feat/page", | ||
| 1360 | "--event", | ||
| 1361 | "pull_request", | ||
| 1362 | "--status", | ||
| 1363 | "action_required", | ||
| 1364 | "--limit", | ||
| 1365 | "1001", | ||
| 1366 | "--json", | ||
| 1367 | "databaseId,workflowName,url", | ||
| 1368 | ]); | ||
| 1369 | expect([callAt(6).args, callAt(10).args]).toEqual([ | ||
| 1370 | [ | ||
| 1371 | "api", | ||
| 1372 | "--method", | ||
| 1373 | "POST", | ||
| 1374 | "--hostname", | ||
| 1375 | "github.com", | ||
| 1376 | "repos/acme/web/actions/runs/10/approve", | ||
| 1377 | "--silent", | ||
| 1378 | ], | ||
| 1379 | [ | ||
| 1380 | "api", | ||
| 1381 | "--method", | ||
| 1382 | "POST", | ||
| 1383 | "--hostname", | ||
| 1384 | "github.com", | ||
| 1385 | "repos/acme/web/actions/runs/11/approve", | ||
| 1386 | "--silent", | ||
| 1387 | ], | ||
| 1388 | ]); | ||
| 1389 | expect(mockedExecute).toHaveBeenCalledTimes(11); | ||
| 1390 | }), | ||
| 1391 | ); | ||
| 1392 | |||
| 1393 | it.effect("refuses a stale workflow approval after the pull request head changes", () => | ||
| 1394 | Effect.gen(function* () { | ||
| 1395 | const detail = { | ||
| 1396 | number: 7, | ||
| 1397 | title: "Pull request 7", | ||
| 1398 | url: "https://github.com/acme/web/pull/7", | ||
| 1399 | headRefName: "feat/page", | ||
| 1400 | headRefOid: "abc123", | ||
| 1401 | isCrossRepository: true, | ||
| 1402 | headRepositoryOwner: { login: "octocat" }, | ||
| 1403 | baseRefName: "main", | ||
| 1404 | createdAt: "2026-07-01T00:00:00Z", | ||
| 1405 | updatedAt: "2026-07-02T00:00:00Z", | ||
| 1406 | }; | ||
| 1407 | for (const value of [ | ||
| 1408 | detail, | ||
| 1409 | [ | ||
| 1410 | { | ||
| 1411 | number: 7, | ||
| 1412 | headRefOid: "abc123", | ||
| 1413 | isCrossRepository: true, | ||
| 1414 | headRepositoryOwner: { login: "octocat" }, | ||
| 1415 | }, | ||
| 1416 | ], | ||
| 1417 | [{ databaseId: 10, workflowName: "build", url: "https://example.com/10" }], | ||
| 1418 | { ...detail, headRefOid: "def456" }, | ||
| 1419 | ]) { | ||
| 1420 | mockedExecute.mockReturnValueOnce( | ||
| 1421 | Effect.succeed( | ||
| 1422 | output( | ||
| 1423 | // @effect-diagnostics-next-line preferSchemaOverJson:off - canned gh response. | ||
| 1424 | JSON.stringify(value), | ||
| 1425 | ), | ||
| 1426 | ), | ||
| 1427 | ); | ||
| 1428 | } | ||
| 1429 | const cli = yield* GitHubPullRequestCli.GitHubPullRequestCli; | ||
| 1430 | |||
| 1431 | const error = yield* Effect.flip( | ||
| 1432 | cli.runPullRequestAction({ | ||
| 1433 | cwd: "/w", | ||
| 1434 | repository: "acme/web", | ||
| 1435 | host: "github.com", | ||
| 1436 | number: 7, | ||
| 1437 | action: "approve-workflows", | ||
| 1438 | }), | ||
| 1439 | ); | ||
| 1440 | |||
| 1441 | expect(error).toMatchObject({ | ||
| 1442 | _tag: "GitHubWorkflowApprovalHeadChangedError", | ||
| 1443 | number: 7, | ||
| 1444 | }); | ||
| 1445 | expect(mockedExecute).toHaveBeenCalledTimes(4); | ||
| 1446 | }), | ||
| 1447 | ); | ||
| 1448 | |||
| 1449 | it.effect("refuses workflow approval when one head belongs to several pull requests", () => | ||
| 1450 | Effect.gen(function* () { | ||
| 1451 | mockedExecute.mockReturnValueOnce( | ||
| 1452 | Effect.succeed( | ||
| 1453 | output( | ||
| 1454 | // @effect-diagnostics-next-line preferSchemaOverJson:off - canned gh response. | ||
| 1455 | JSON.stringify( | ||
| 1456 | [7, 8].map((number) => ({ | ||
| 1457 | number, | ||
| 1458 | headRefOid: "abc123", | ||
| 1459 | isCrossRepository: true, | ||
| 1460 | headRepositoryOwner: { login: "octocat" }, | ||
| 1461 | })), | ||
| 1462 | ), | ||
| 1463 | ), | ||
| 1464 | ), | ||
| 1465 | ); | ||
| 1466 | const cli = yield* GitHubPullRequestCli.GitHubPullRequestCli; | ||
| 1467 | |||
| 1468 | const error = yield* Effect.flip( | ||
| 1469 | cli.listWorkflowRunsRequiringApproval({ | ||
| 1470 | cwd: "/w", | ||
| 1471 | repository: "acme/web", | ||
| 1472 | host: "github.com", | ||
| 1473 | number: 7, | ||
| 1474 | headSha: "abc123", | ||
| 1475 | headBranch: "feat/page", | ||
| 1476 | headRepositoryOwner: "octocat", | ||
| 1477 | isCrossRepository: true, | ||
| 1478 | }), | ||
| 1479 | ); | ||
| 1480 | |||
| 1481 | expect(error).toMatchObject({ | ||
| 1482 | _tag: "GitHubWorkflowApprovalRefusedError", | ||
| 1483 | reason: "head-not-unique", | ||
| 1484 | number: 7, | ||
| 1485 | observedCount: 2, | ||
| 1486 | limit: 1_000, | ||
| 1487 | }); | ||
| 1488 | expect(error.detail).toContain("instead of uniquely matching #7"); | ||
| 1489 | expect(mockedExecute).toHaveBeenCalledTimes(1); | ||
| 1490 | }), | ||
| 1491 | ); | ||
| 1492 | |||
| 1493 | it.effect("refuses workflow approval when GitHub omits the head repository", () => | ||
| 1494 | Effect.gen(function* () { | ||
| 1495 | mockedExecute.mockReturnValueOnce( | ||
| 1496 | Effect.succeed( | ||
| 1497 | output( | ||
| 1498 | // @effect-diagnostics-next-line preferSchemaOverJson:off - canned gh response. | ||
| 1499 | JSON.stringify({ | ||
| 1500 | number: 7, | ||
| 1501 | title: "Pull request 7", | ||
| 1502 | url: "https://github.com/acme/web/pull/7", | ||
| 1503 | headRefName: "feat/page", | ||
| 1504 | headRefOid: "abc123", | ||
| 1505 | isCrossRepository: true, | ||
| 1506 | headRepositoryOwner: null, | ||
| 1507 | baseRefName: "main", | ||
| 1508 | createdAt: "2026-07-01T00:00:00Z", | ||
| 1509 | updatedAt: "2026-07-02T00:00:00Z", | ||
| 1510 | }), | ||
| 1511 | ), | ||
| 1512 | ), | ||
| 1513 | ); | ||
| 1514 | const cli = yield* GitHubPullRequestCli.GitHubPullRequestCli; | ||
| 1515 | |||
| 1516 | const error = yield* Effect.flip( | ||
| 1517 | cli.runPullRequestAction({ | ||
| 1518 | cwd: "/w", | ||
| 1519 | repository: "acme/web", | ||
| 1520 | host: "github.com", | ||
| 1521 | number: 7, | ||
| 1522 | action: "approve-workflows", | ||
| 1523 | }), | ||
| 1524 | ); | ||
| 1525 | |||
| 1526 | expect(error).toMatchObject({ | ||
| 1527 | _tag: "GitHubWorkflowApprovalHeadUnavailableError", | ||
| 1528 | number: 7, | ||
| 1529 | }); | ||
| 1530 | expect(mockedExecute).toHaveBeenCalledTimes(1); | ||
| 1531 | }), | ||
| 1532 | ); | ||
| 1533 | |||
| 1534 | it.effect("surfaces a workflow run list beyond the safe approval bound", () => | ||
| 1535 | Effect.gen(function* () { | ||
| 1536 | mockedExecute.mockReturnValueOnce( | ||
| 1537 | Effect.succeed( | ||
| 1538 | output( | ||
| 1539 | // @effect-diagnostics-next-line preferSchemaOverJson:off - canned gh response. | ||
| 1540 | JSON.stringify([ | ||
| 1541 | { | ||
| 1542 | number: 7, | ||
| 1543 | headRefOid: "abc123", | ||
| 1544 | isCrossRepository: true, | ||
| 1545 | headRepositoryOwner: { login: "octocat" }, | ||
| 1546 | }, | ||
| 1547 | ]), | ||
| 1548 | ), | ||
| 1549 | ), | ||
| 1550 | ); | ||
| 1551 | mockedExecute.mockReturnValueOnce( | ||
| 1552 | Effect.succeed( | ||
| 1553 | output( | ||
| 1554 | // @effect-diagnostics-next-line preferSchemaOverJson:off - canned gh response. | ||
| 1555 | JSON.stringify(Array.from({ length: 1_001 }, (_, id) => ({ databaseId: id + 1 }))), | ||
| 1556 | ), | ||
| 1557 | ), | ||
| 1558 | ); | ||
| 1559 | const cli = yield* GitHubPullRequestCli.GitHubPullRequestCli; | ||
| 1560 | |||
| 1561 | const error = yield* Effect.flip( | ||
| 1562 | cli.listWorkflowRunsRequiringApproval({ | ||
| 1563 | cwd: "/w", | ||
| 1564 | repository: "acme/web", | ||
| 1565 | host: "github.com", | ||
| 1566 | number: 7, | ||
| 1567 | headSha: "abc123", | ||
| 1568 | headBranch: "feat/page", | ||
| 1569 | headRepositoryOwner: "octocat", | ||
| 1570 | isCrossRepository: true, | ||
| 1571 | }), | ||
| 1572 | ); | ||
| 1573 | |||
| 1574 | expect(error).toMatchObject({ | ||
| 1575 | _tag: "GitHubWorkflowApprovalRefusedError", | ||
| 1576 | reason: "run-list-truncated", | ||
| 1577 | number: 7, | ||
| 1578 | observedCount: 1_001, | ||
| 1579 | limit: 1_000, | ||
| 1580 | }); | ||
| 1581 | expect(error.detail).toContain("more than 1000 workflow runs"); | ||
| 1582 | expect(mockedExecute).toHaveBeenCalledTimes(2); | ||
| 1583 | }), | ||
| 1584 | ); | ||
| 1585 | |||
| 1203 | it.effect("returns a pull request to draft by undoing ready", () => | 1586 | it.effect("returns a pull request to draft by undoing ready", () => |
| 1204 | Effect.gen(function* () { | 1587 | Effect.gen(function* () { |
| 1205 | mockedExecute.mockReturnValue(Effect.succeed(output(""))); | 1588 | mockedExecute.mockReturnValue(Effect.succeed(output(""))); |
| ... | @@ -2136,7 +2519,7 @@ layer("GitHubPullRequestCli.layer", (it) => { | ... | @@ -2136,7 +2519,7 @@ layer("GitHubPullRequestCli.layer", (it) => { |
| 2136 | expect(detail.body).toBe("Core body"); | 2519 | expect(detail.body).toBe("Core body"); |
| 2137 | expect(activity.author?.login).toBe("octocat"); | 2520 | expect(activity.author?.login).toBe("octocat"); |
| 2138 | expect(callAt(0).args.at(-1)).toBe( | 2521 | expect(callAt(0).args.at(-1)).toBe( |
| 2139 | "number,title,url,author,headRefName,baseRefName,state,isDraft,mergeable,reviewDecision,additions,deletions,createdAt,updatedAt,mergedAt,reviewRequests,labels,statusCheckRollup,body,changedFiles,closedAt,headRepositoryOwner,autoMergeRequest", | 2522 | "number,title,url,author,headRefName,baseRefName,state,isDraft,mergeable,reviewDecision,additions,deletions,createdAt,updatedAt,mergedAt,reviewRequests,labels,statusCheckRollup,body,changedFiles,closedAt,isCrossRepository,headRepositoryOwner,headRefOid,autoMergeRequest", |
| 2140 | ); | 2523 | ); |
| 2141 | expect(callAt(1).args.at(-1)).toBe("author,comments,reviews,commits"); | 2524 | expect(callAt(1).args.at(-1)).toBe("author,comments,reviews,commits"); |
| 2142 | }), | 2525 | }), |
apps/server/src/pullRequest/GitHubPullRequestCli.ts+342-28| ... | @@ -34,6 +34,7 @@ import { | ... | @@ -34,6 +34,7 @@ import { |
| 34 | decodePullRequestActivityJson, | 34 | decodePullRequestActivityJson, |
| 35 | decodePullRequestDetailJson, | 35 | decodePullRequestDetailJson, |
| 36 | decodePullRequestFilesJson, | 36 | decodePullRequestFilesJson, |
| 37 | decodePullRequestHeadsJson, | ||
| 37 | decodePullRequestListJson, | 38 | decodePullRequestListJson, |
| 38 | decodePullRequestNodeIdJson, | 39 | decodePullRequestNodeIdJson, |
| 39 | decodePullRequestSearchJson, | 40 | decodePullRequestSearchJson, |
| ... | @@ -56,6 +57,7 @@ import { | ... | @@ -56,6 +57,7 @@ import { |
| 56 | PULL_REQUEST_NODE_ID_GRAPHQL_QUERY, | 57 | PULL_REQUEST_NODE_ID_GRAPHQL_QUERY, |
| 57 | REACTION_SUBJECT_PULL_REQUEST_GRAPHQL_QUERY, | 58 | REACTION_SUBJECT_PULL_REQUEST_GRAPHQL_QUERY, |
| 58 | REMOVE_REACTION_GRAPHQL_MUTATION, | 59 | REMOVE_REACTION_GRAPHQL_MUTATION, |
| 60 | REVERT_PULL_REQUEST_GRAPHQL_MUTATION, | ||
| 59 | gitHubReactionContent, | 61 | gitHubReactionContent, |
| 60 | REPOSITORY_ACCESS_JSON_FIELDS, | 62 | REPOSITORY_ACCESS_JSON_FIELDS, |
| 61 | RESOLVE_REVIEW_THREAD_GRAPHQL_MUTATION, | 63 | RESOLVE_REVIEW_THREAD_GRAPHQL_MUTATION, |
| ... | @@ -71,13 +73,16 @@ import { | ... | @@ -71,13 +73,16 @@ import { |
| 71 | UPDATE_REVIEW_COMMENT_GRAPHQL_MUTATION, | 73 | UPDATE_REVIEW_COMMENT_GRAPHQL_MUTATION, |
| 72 | VIEWER_PERMISSIONS_GRAPHQL_QUERY, | 74 | VIEWER_PERMISSIONS_GRAPHQL_QUERY, |
| 73 | decodeViewerPermissionsJson, | 75 | decodeViewerPermissionsJson, |
| 76 | decodeWorkflowRunApprovalsJson, | ||
| 74 | type GitHubBaseComparison, | 77 | type GitHubBaseComparison, |
| 75 | type GitHubPullRequestDetail, | 78 | type GitHubPullRequestDetail, |
| 76 | type GitHubPullRequestActivity, | 79 | type GitHubPullRequestActivity, |
| 80 | type GitHubPullRequestHead, | ||
| 77 | type GitHubPullRequestListItem, | 81 | type GitHubPullRequestListItem, |
| 78 | type GitHubPullRequestSearchItem, | 82 | type GitHubPullRequestSearchItem, |
| 79 | type GitHubReviewThreadComments, | 83 | type GitHubReviewThreadComments, |
| 80 | type GitHubRepositoryAccess, | 84 | type GitHubRepositoryAccess, |
| 85 | type GitHubWorkflowRunApproval, | ||
| 81 | type GitHubReviewThreadEntry, | 86 | type GitHubReviewThreadEntry, |
| 82 | type GitHubReviewThreadPage, | 87 | type GitHubReviewThreadPage, |
| 83 | type GitHubViewerAccess, | 88 | type GitHubViewerAccess, |
| ... | @@ -259,6 +264,69 @@ export class GitHubSubjectScopeError extends Schema.TaggedErrorClass<GitHubSubje | ... | @@ -259,6 +264,69 @@ export class GitHubSubjectScopeError extends Schema.TaggedErrorClass<GitHubSubje |
| 259 | } | 264 | } |
| 260 | } | 265 | } |
| 261 | 266 | ||
| 267 | /** GitHub answered successfully, but approving every returned workflow would be unsafe. */ | ||
| 268 | export class GitHubWorkflowApprovalRefusedError extends Schema.TaggedErrorClass<GitHubWorkflowApprovalRefusedError>()( | ||
| 269 | "GitHubWorkflowApprovalRefusedError", | ||
| 270 | { | ||
| 271 | command: Schema.Literal("gh"), | ||
| 272 | cwd: Schema.String, | ||
| 273 | number: Schema.Int, | ||
| 274 | reason: Schema.Literals(["head-list-truncated", "head-not-unique", "run-list-truncated"]), | ||
| 275 | observedCount: Schema.Int, | ||
| 276 | limit: Schema.Int, | ||
| 277 | }, | ||
| 278 | ) { | ||
| 279 | get detail(): string { | ||
| 280 | if (this.reason === "head-list-truncated") { | ||
| 281 | return `GitHub returned more than ${this.limit} pull requests for this head branch.`; | ||
| 282 | } | ||
| 283 | if (this.reason === "head-not-unique") { | ||
| 284 | return `The head revision matched ${this.observedCount} pull requests instead of uniquely matching #${this.number}.`; | ||
| 285 | } | ||
| 286 | return `GitHub returned more than ${this.limit} workflow runs awaiting approval.`; | ||
| 287 | } | ||
| 288 | |||
| 289 | override get message(): string { | ||
| 290 | return `GitHub CLI refused listWorkflowRunsRequiringApproval: ${this.detail}`; | ||
| 291 | } | ||
| 292 | } | ||
| 293 | |||
| 294 | /** GitHub omitted the immutable head identity needed to scope an approval safely. */ | ||
| 295 | export class GitHubWorkflowApprovalHeadUnavailableError extends Schema.TaggedErrorClass<GitHubWorkflowApprovalHeadUnavailableError>()( | ||
| 296 | "GitHubWorkflowApprovalHeadUnavailableError", | ||
| 297 | { | ||
| 298 | command: Schema.Literal("gh"), | ||
| 299 | cwd: Schema.String, | ||
| 300 | number: Schema.Int, | ||
| 301 | }, | ||
| 302 | ) { | ||
| 303 | get detail(): string { | ||
| 304 | return `GitHub did not report a complete head revision for #${this.number}.`; | ||
| 305 | } | ||
| 306 | |||
| 307 | override get message(): string { | ||
| 308 | return `GitHub CLI refused approve-workflows: ${this.detail}`; | ||
| 309 | } | ||
| 310 | } | ||
| 311 | |||
| 312 | /** The pull request moved after its approval candidates were read. */ | ||
| 313 | export class GitHubWorkflowApprovalHeadChangedError extends Schema.TaggedErrorClass<GitHubWorkflowApprovalHeadChangedError>()( | ||
| 314 | "GitHubWorkflowApprovalHeadChangedError", | ||
| 315 | { | ||
| 316 | command: Schema.Literal("gh"), | ||
| 317 | cwd: Schema.String, | ||
| 318 | number: Schema.Int, | ||
| 319 | }, | ||
| 320 | ) { | ||
| 321 | get detail(): string { | ||
| 322 | return `The head revision of #${this.number} changed before its workflows could be approved.`; | ||
| 323 | } | ||
| 324 | |||
| 325 | override get message(): string { | ||
| 326 | return `GitHub CLI refused approve-workflows: ${this.detail}`; | ||
| 327 | } | ||
| 328 | } | ||
| 329 | |||
| 262 | export type GitHubPullRequestCliError = | 330 | export type GitHubPullRequestCliError = |
| 263 | | GitHubCli.GitHubCliError | 331 | | GitHubCli.GitHubCliError |
| 264 | | GitHubPullRequestReadError | 332 | | GitHubPullRequestReadError |
| ... | @@ -268,6 +336,9 @@ export type GitHubPullRequestCliError = | ... | @@ -268,6 +336,9 @@ export type GitHubPullRequestCliError = |
| 268 | | GitHubDiffFileContentsUnavailableError | 336 | | GitHubDiffFileContentsUnavailableError |
| 269 | | GitHubRepositorySelectorError | 337 | | GitHubRepositorySelectorError |
| 270 | | GitHubSubjectScopeError | 338 | | GitHubSubjectScopeError |
| 339 | | GitHubWorkflowApprovalRefusedError | ||
| 340 | | GitHubWorkflowApprovalHeadUnavailableError | ||
| 341 | | GitHubWorkflowApprovalHeadChangedError | ||
| 271 | | SourceControlRateLimit.SourceControlRateLimitPausedError | 342 | | SourceControlRateLimit.SourceControlRateLimitPausedError |
| 272 | | GitHubViewerLoginUnavailableError | 343 | | GitHubViewerLoginUnavailableError |
| 273 | | GitHubPullRequestUpdatedAtUnavailableError; | 344 | | GitHubPullRequestUpdatedAtUnavailableError; |
| ... | @@ -405,6 +476,17 @@ export class GitHubPullRequestCli extends Context.Service< | ... | @@ -405,6 +476,17 @@ export class GitHubPullRequestCli extends Context.Service< |
| 405 | readonly number: number; | 476 | readonly number: number; |
| 406 | }) => Effect.Effect<GitHubPullRequestDetail, GitHubPullRequestCliError>; | 477 | }) => Effect.Effect<GitHubPullRequestDetail, GitHubPullRequestCliError>; |
| 407 | 478 | ||
| 479 | readonly listWorkflowRunsRequiringApproval: (input: { | ||
| 480 | readonly cwd: string; | ||
| 481 | readonly repository: string; | ||
| 482 | readonly host: string; | ||
| 483 | readonly number: number; | ||
| 484 | readonly headSha: string; | ||
| 485 | readonly headBranch: string; | ||
| 486 | readonly headRepositoryOwner: string; | ||
| 487 | readonly isCrossRepository: true; | ||
| 488 | }) => Effect.Effect<ReadonlyArray<GitHubWorkflowRunApproval>, GitHubPullRequestCliError>; | ||
| 489 | |||
| 408 | /** | 490 | /** |
| 409 | * How far the branch trails its base, and whether this viewer may update it. Its own read | 491 | * How far the branch trails its base, and whether this viewer may update it. Its own read |
| 410 | * because the comparison needs the head ref the detail answers with — a fork's branch is not | 492 | * because the comparison needs the head ref the detail answers with — a fork's branch is not |
| ... | @@ -864,6 +946,12 @@ function actionArgs( | ... | @@ -864,6 +946,12 @@ function actionArgs( |
| 864 | return ["close"]; | 946 | return ["close"]; |
| 865 | case "reopen": | 947 | case "reopen": |
| 866 | return ["reopen"]; | 948 | return ["reopen"]; |
| 949 | case "revert": | ||
| 950 | throw new Error("Revert requires a GraphQL mutation"); | ||
| 951 | // Handled separately because it may approve several workflow runs rather than mutate the | ||
| 952 | // pull request itself. | ||
| 953 | case "approve-workflows": | ||
| 954 | throw new Error("Workflow approval requires run discovery"); | ||
| 867 | } | 955 | } |
| 868 | } | 956 | } |
| 869 | 957 | ||
| ... | @@ -1194,6 +1282,158 @@ export const make = Effect.gen(function* () { | ... | @@ -1194,6 +1282,158 @@ export const make = Effect.gen(function* () { |
| 1194 | return { oldContents, newContents }; | 1282 | return { oldContents, newContents }; |
| 1195 | }); | 1283 | }); |
| 1196 | 1284 | ||
| 1285 | const getPullRequestDetail: GitHubPullRequestCli["Service"]["getPullRequestDetail"] = (input) => | ||
| 1286 | github | ||
| 1287 | .execute({ | ||
| 1288 | cwd: input.cwd, | ||
| 1289 | args: [ | ||
| 1290 | "pr", | ||
| 1291 | "view", | ||
| 1292 | String(input.number), | ||
| 1293 | ...repositoryArgs(input), | ||
| 1294 | "--json", | ||
| 1295 | PULL_REQUEST_DETAIL_JSON_FIELDS, | ||
| 1296 | ], | ||
| 1297 | }) | ||
| 1298 | .pipe( | ||
| 1299 | Effect.flatMap((result) => { | ||
| 1300 | const decoded = decodePullRequestDetailJson(result.stdout.trim()); | ||
| 1301 | return Result.isSuccess(decoded) | ||
| 1302 | ? Effect.succeed(decoded.success) | ||
| 1303 | : Effect.fail( | ||
| 1304 | new GitHubPullRequestReadError({ | ||
| 1305 | command: "gh", | ||
| 1306 | cwd: input.cwd, | ||
| 1307 | operation: "getPullRequestDetail", | ||
| 1308 | cause: decoded.failure, | ||
| 1309 | }), | ||
| 1310 | ); | ||
| 1311 | }), | ||
| 1312 | ); | ||
| 1313 | |||
| 1314 | const workflowApprovalLimit = 1_000; | ||
| 1315 | const workflowApprovalProbeLimit = String(workflowApprovalLimit + 1); | ||
| 1316 | const workflowApprovalReadError = (cwd: string, cause: unknown) => | ||
| 1317 | new GitHubPullRequestReadError({ | ||
| 1318 | command: "gh", | ||
| 1319 | cwd, | ||
| 1320 | operation: "listWorkflowRunsRequiringApproval", | ||
| 1321 | cause, | ||
| 1322 | }); | ||
| 1323 | const listWorkflowRunsRequiringApproval: GitHubPullRequestCli["Service"]["listWorkflowRunsRequiringApproval"] = | ||
| 1324 | (input) => | ||
| 1325 | github | ||
| 1326 | .execute({ | ||
| 1327 | cwd: input.cwd, | ||
| 1328 | args: [ | ||
| 1329 | "pr", | ||
| 1330 | "list", | ||
| 1331 | ...repositoryArgs(input), | ||
| 1332 | "--state", | ||
| 1333 | "open", | ||
| 1334 | "--head", | ||
| 1335 | input.headBranch, | ||
| 1336 | "--limit", | ||
| 1337 | workflowApprovalProbeLimit, | ||
| 1338 | "--json", | ||
| 1339 | "number,headRefOid,isCrossRepository,headRepositoryOwner", | ||
| 1340 | ], | ||
| 1341 | }) | ||
| 1342 | .pipe( | ||
| 1343 | Effect.flatMap( | ||
| 1344 | ( | ||
| 1345 | result, | ||
| 1346 | ): Effect.Effect< | ||
| 1347 | GitHubPullRequestHead, | ||
| 1348 | GitHubPullRequestReadError | GitHubWorkflowApprovalRefusedError | ||
| 1349 | > => { | ||
| 1350 | const decoded = decodePullRequestHeadsJson(result.stdout.trim()); | ||
| 1351 | if (!Result.isSuccess(decoded)) { | ||
| 1352 | return Effect.fail(workflowApprovalReadError(input.cwd, decoded.failure)); | ||
| 1353 | } | ||
| 1354 | const exactHeads = decoded.success.filter( | ||
| 1355 | (pullRequest) => | ||
| 1356 | pullRequest.headSha === input.headSha && | ||
| 1357 | pullRequest.isCrossRepository === true && | ||
| 1358 | pullRequest.headRepositoryOwner?.toLowerCase() === | ||
| 1359 | input.headRepositoryOwner.toLowerCase(), | ||
| 1360 | ); | ||
| 1361 | if (decoded.success.length > workflowApprovalLimit) { | ||
| 1362 | return Effect.fail( | ||
| 1363 | new GitHubWorkflowApprovalRefusedError({ | ||
| 1364 | command: "gh", | ||
| 1365 | cwd: input.cwd, | ||
| 1366 | number: input.number, | ||
| 1367 | reason: "head-list-truncated", | ||
| 1368 | observedCount: decoded.success.length, | ||
| 1369 | limit: workflowApprovalLimit, | ||
| 1370 | }), | ||
| 1371 | ); | ||
| 1372 | } | ||
| 1373 | if (exactHeads.length !== 1 || exactHeads[0]?.number !== input.number) { | ||
| 1374 | return Effect.fail( | ||
| 1375 | new GitHubWorkflowApprovalRefusedError({ | ||
| 1376 | command: "gh", | ||
| 1377 | cwd: input.cwd, | ||
| 1378 | number: input.number, | ||
| 1379 | reason: "head-not-unique", | ||
| 1380 | observedCount: exactHeads.length, | ||
| 1381 | limit: workflowApprovalLimit, | ||
| 1382 | }), | ||
| 1383 | ); | ||
| 1384 | } | ||
| 1385 | return Effect.succeed(exactHeads[0]); | ||
| 1386 | }, | ||
| 1387 | ), | ||
| 1388 | Effect.flatMap(() => | ||
| 1389 | github.execute({ | ||
| 1390 | cwd: input.cwd, | ||
| 1391 | args: [ | ||
| 1392 | "run", | ||
| 1393 | "list", | ||
| 1394 | ...repositoryArgs(input), | ||
| 1395 | "--commit", | ||
| 1396 | input.headSha, | ||
| 1397 | "--branch", | ||
| 1398 | input.headBranch, | ||
| 1399 | "--event", | ||
| 1400 | "pull_request", | ||
| 1401 | "--status", | ||
| 1402 | "action_required", | ||
| 1403 | "--limit", | ||
| 1404 | workflowApprovalProbeLimit, | ||
| 1405 | "--json", | ||
| 1406 | "databaseId,workflowName,url", | ||
| 1407 | ], | ||
| 1408 | }), | ||
| 1409 | ), | ||
| 1410 | Effect.flatMap( | ||
| 1411 | ( | ||
| 1412 | result, | ||
| 1413 | ): Effect.Effect< | ||
| 1414 | ReadonlyArray<GitHubWorkflowRunApproval>, | ||
| 1415 | GitHubPullRequestReadError | GitHubWorkflowApprovalRefusedError | ||
| 1416 | > => { | ||
| 1417 | const decoded = decodeWorkflowRunApprovalsJson(result.stdout.trim()); | ||
| 1418 | if (!Result.isSuccess(decoded)) { | ||
| 1419 | return Effect.fail(workflowApprovalReadError(input.cwd, decoded.failure)); | ||
| 1420 | } | ||
| 1421 | return decoded.success.length > workflowApprovalLimit | ||
| 1422 | ? Effect.fail( | ||
| 1423 | new GitHubWorkflowApprovalRefusedError({ | ||
| 1424 | command: "gh", | ||
| 1425 | cwd: input.cwd, | ||
| 1426 | number: input.number, | ||
| 1427 | reason: "run-list-truncated", | ||
| 1428 | observedCount: decoded.success.length, | ||
| 1429 | limit: workflowApprovalLimit, | ||
| 1430 | }), | ||
| 1431 | ) | ||
| 1432 | : Effect.succeed(decoded.success); | ||
| 1433 | }, | ||
| 1434 | ), | ||
| 1435 | ); | ||
| 1436 | |||
| 1197 | return GitHubPullRequestCli.of({ | 1437 | return GitHubPullRequestCli.of({ |
| 1198 | getViewerLogin: (input) => | 1438 | getViewerLogin: (input) => |
| 1199 | github.execute({ cwd: input.cwd, args: ["api", "user", "--jq", ".login"] }).pipe( | 1439 | github.execute({ cwd: input.cwd, args: ["api", "user", "--jq", ".login"] }).pipe( |
| ... | @@ -1393,34 +1633,8 @@ export const make = Effect.gen(function* () { | ... | @@ -1393,34 +1633,8 @@ export const make = Effect.gen(function* () { |
| 1393 | ), | 1633 | ), |
| 1394 | ), | 1634 | ), |
| 1395 | 1635 | ||
| 1396 | getPullRequestDetail: (input) => | 1636 | getPullRequestDetail, |
| 1397 | github | 1637 | listWorkflowRunsRequiringApproval, |
| 1398 | .execute({ | ||
| 1399 | cwd: input.cwd, | ||
| 1400 | args: [ | ||
| 1401 | "pr", | ||
| 1402 | "view", | ||
| 1403 | String(input.number), | ||
| 1404 | ...repositoryArgs(input), | ||
| 1405 | "--json", | ||
| 1406 | PULL_REQUEST_DETAIL_JSON_FIELDS, | ||
| 1407 | ], | ||
| 1408 | }) | ||
| 1409 | .pipe( | ||
| 1410 | Effect.flatMap((result) => { | ||
| 1411 | const decoded = decodePullRequestDetailJson(result.stdout.trim()); | ||
| 1412 | return Result.isSuccess(decoded) | ||
| 1413 | ? Effect.succeed(decoded.success) | ||
| 1414 | : Effect.fail( | ||
| 1415 | new GitHubPullRequestReadError({ | ||
| 1416 | command: "gh", | ||
| 1417 | cwd: input.cwd, | ||
| 1418 | operation: "getPullRequestDetail", | ||
| 1419 | cause: decoded.failure, | ||
| 1420 | }), | ||
| 1421 | ); | ||
| 1422 | }), | ||
| 1423 | ), | ||
| 1424 | 1638 | ||
| 1425 | getPullRequestBaseComparison: (input) => { | 1639 | getPullRequestBaseComparison: (input) => { |
| 1426 | const { owner, name } = parseRepositorySelector(input.repository); | 1640 | const { owner, name } = parseRepositorySelector(input.repository); |
| ... | @@ -1764,6 +1978,106 @@ export const make = Effect.gen(function* () { | ... | @@ -1764,6 +1978,106 @@ export const make = Effect.gen(function* () { |
| 1764 | }, | 1978 | }, |
| 1765 | 1979 | ||
| 1766 | runPullRequestAction: (input) => { | 1980 | runPullRequestAction: (input) => { |
| 1981 | if (input.action === "revert") { | ||
| 1982 | return pullRequestNodeId({ ...input, operation: "revertPullRequest" }).pipe( | ||
| 1983 | Effect.flatMap((pullRequestId) => | ||
| 1984 | graphql({ | ||
| 1985 | cwd: input.cwd, | ||
| 1986 | host: input.host, | ||
| 1987 | query: REVERT_PULL_REQUEST_GRAPHQL_MUTATION, | ||
| 1988 | variables: { pullRequestId }, | ||
| 1989 | }), | ||
| 1990 | ), | ||
| 1991 | ); | ||
| 1992 | } | ||
| 1993 | if (input.action === "approve-workflows") { | ||
| 1994 | const { owner, name } = parseRepositorySelector(input.repository); | ||
| 1995 | return getPullRequestDetail(input).pipe( | ||
| 1996 | Effect.flatMap((detail) => { | ||
| 1997 | if (detail.isCrossRepository !== true) return Effect.void; | ||
| 1998 | if (detail.headSha == null || detail.headRepositoryOwner == null) { | ||
| 1999 | return Effect.fail( | ||
| 2000 | new GitHubWorkflowApprovalHeadUnavailableError({ | ||
| 2001 | command: "gh", | ||
| 2002 | cwd: input.cwd, | ||
| 2003 | number: input.number, | ||
| 2004 | }), | ||
| 2005 | ); | ||
| 2006 | } | ||
| 2007 | const expectedHeadSha = detail.headSha; | ||
| 2008 | const expectedHeadBranch = detail.headBranch; | ||
| 2009 | const expectedHeadRepositoryOwner = detail.headRepositoryOwner; | ||
| 2010 | return listWorkflowRunsRequiringApproval({ | ||
| 2011 | ...input, | ||
| 2012 | headSha: expectedHeadSha, | ||
| 2013 | headBranch: expectedHeadBranch, | ||
| 2014 | headRepositoryOwner: expectedHeadRepositoryOwner, | ||
| 2015 | isCrossRepository: true, | ||
| 2016 | }).pipe( | ||
| 2017 | Effect.flatMap((runs) => | ||
| 2018 | Effect.forEach( | ||
| 2019 | runs, | ||
| 2020 | (run) => | ||
| 2021 | getPullRequestDetail(input).pipe( | ||
| 2022 | Effect.flatMap((current) => { | ||
| 2023 | if (current.headSha == null || current.headRepositoryOwner == null) { | ||
| 2024 | return Effect.fail( | ||
| 2025 | new GitHubWorkflowApprovalHeadUnavailableError({ | ||
| 2026 | command: "gh", | ||
| 2027 | cwd: input.cwd, | ||
| 2028 | number: input.number, | ||
| 2029 | }), | ||
| 2030 | ); | ||
| 2031 | } | ||
| 2032 | if ( | ||
| 2033 | current.isCrossRepository !== true || | ||
| 2034 | current.headSha !== expectedHeadSha || | ||
| 2035 | current.headBranch !== expectedHeadBranch || | ||
| 2036 | current.headRepositoryOwner.toLowerCase() !== | ||
| 2037 | expectedHeadRepositoryOwner.toLowerCase() | ||
| 2038 | ) { | ||
| 2039 | return Effect.fail( | ||
| 2040 | new GitHubWorkflowApprovalHeadChangedError({ | ||
| 2041 | command: "gh", | ||
| 2042 | cwd: input.cwd, | ||
| 2043 | number: input.number, | ||
| 2044 | }), | ||
| 2045 | ); | ||
| 2046 | } | ||
| 2047 | return listWorkflowRunsRequiringApproval({ | ||
| 2048 | ...input, | ||
| 2049 | headSha: current.headSha, | ||
| 2050 | headBranch: current.headBranch, | ||
| 2051 | headRepositoryOwner: current.headRepositoryOwner, | ||
| 2052 | isCrossRepository: true, | ||
| 2053 | }); | ||
| 2054 | }), | ||
| 2055 | Effect.flatMap((currentRuns) => | ||
| 2056 | currentRuns.some((current) => current.id === run.id) | ||
| 2057 | ? github | ||
| 2058 | .execute({ | ||
| 2059 | cwd: input.cwd, | ||
| 2060 | args: [ | ||
| 2061 | "api", | ||
| 2062 | "--method", | ||
| 2063 | "POST", | ||
| 2064 | "--hostname", | ||
| 2065 | input.host, | ||
| 2066 | `repos/${owner}/${name}/actions/runs/${run.id}/approve`, | ||
| 2067 | "--silent", | ||
| 2068 | ], | ||
| 2069 | }) | ||
| 2070 | .pipe(Effect.asVoid) | ||
| 2071 | : Effect.void, | ||
| 2072 | ), | ||
| 2073 | ), | ||
| 2074 | { concurrency: 1, discard: true }, | ||
| 2075 | ), | ||
| 2076 | ), | ||
| 2077 | ); | ||
| 2078 | }), | ||
| 2079 | ); | ||
| 2080 | } | ||
| 1767 | const [subcommand, ...flags] = actionArgs( | 2081 | const [subcommand, ...flags] = actionArgs( |
| 1768 | input.action, | 2082 | input.action, |
| 1769 | input.mergeMethod, | 2083 | input.mergeMethod, |
apps/server/src/pullRequest/GitHubPullRequestProvider.test.ts+256-30| ... | @@ -3,6 +3,7 @@ import * as Effect from "effect/Effect"; | ... | @@ -3,6 +3,7 @@ import * as Effect from "effect/Effect"; |
| 3 | import * as Layer from "effect/Layer"; | 3 | import * as Layer from "effect/Layer"; |
| 4 | import type { PullRequestReaction } from "@t3tools/contracts"; | 4 | import type { PullRequestReaction } from "@t3tools/contracts"; |
| 5 | 5 | ||
| 6 | import * as GitHubCli from "../sourceControl/GitHubCli.ts"; | ||
| 6 | import * as GitHubPullRequestCli from "./GitHubPullRequestCli.ts"; | 7 | import * as GitHubPullRequestCli from "./GitHubPullRequestCli.ts"; |
| 7 | import { gitHubViewerPermissions, loginAvatarUrl, make } from "./GitHubPullRequestProvider.ts"; | 8 | import { gitHubViewerPermissions, loginAvatarUrl, make } from "./GitHubPullRequestProvider.ts"; |
| 8 | import type { GitHubReviewThreadComments } from "./gitHubPullRequestJson.ts"; | 9 | import type { GitHubReviewThreadComments } from "./gitHubPullRequestJson.ts"; |
| ... | @@ -52,6 +53,8 @@ describe("gitHubViewerPermissions", () => { | ... | @@ -52,6 +53,8 @@ describe("gitHubViewerPermissions", () => { |
| 52 | "merge", | 53 | "merge", |
| 53 | "enable-auto-merge", | 54 | "enable-auto-merge", |
| 54 | "disable-auto-merge", | 55 | "disable-auto-merge", |
| 56 | "revert", | ||
| 57 | "approve-workflows", | ||
| 55 | "ready", | 58 | "ready", |
| 56 | "draft", | 59 | "draft", |
| 57 | "close", | 60 | "close", |
| ... | @@ -108,6 +111,13 @@ describe("gitHubViewerPermissions", () => { | ... | @@ -108,6 +111,13 @@ describe("gitHubViewerPermissions", () => { |
| 108 | verdicts: ["comment", "approve", "request-changes"], | 111 | verdicts: ["comment", "approve", "request-changes"], |
| 109 | requestReviewers: false, | 112 | requestReviewers: false, |
| 110 | }); | 113 | }); |
| 114 | expect(detail.workflowApprovalsRequired).toBeUndefined(); | ||
| 115 | expect(detail.checks).toContainEqual({ | ||
| 116 | name: "Workflow approval status", | ||
| 117 | status: "action-required", | ||
| 118 | description: "GitHub could not determine whether workflows are awaiting approval.", | ||
| 119 | url: null, | ||
| 120 | }); | ||
| 111 | }).pipe( | 121 | }).pipe( |
| 112 | Effect.provide( | 122 | Effect.provide( |
| 113 | Layer.mock(GitHubPullRequestCli.GitHubPullRequestCli)({ | 123 | Layer.mock(GitHubPullRequestCli.GitHubPullRequestCli)({ |
| ... | @@ -118,6 +128,7 @@ describe("gitHubViewerPermissions", () => { | ... | @@ -118,6 +128,7 @@ describe("gitHubViewerPermissions", () => { |
| 118 | title: "Pull request 7", | 128 | title: "Pull request 7", |
| 119 | url: "https://github.com/acme/web/pull/7", | 129 | url: "https://github.com/acme/web/pull/7", |
| 120 | author: null, | 130 | author: null, |
| 131 | isCrossRepository: true, | ||
| 121 | headRepositoryOwner: null, | 132 | headRepositoryOwner: null, |
| 122 | headBranch: "feat/page", | 133 | headBranch: "feat/page", |
| 123 | baseBranch: "main", | 134 | baseBranch: "main", |
| ... | @@ -152,39 +163,254 @@ describe("gitHubViewerPermissions", () => { | ... | @@ -152,39 +163,254 @@ describe("gitHubViewerPermissions", () => { |
| 152 | ), | 163 | ), |
| 153 | ), | 164 | ), |
| 154 | ); | 165 | ); |
| 166 | |||
| 167 | it.effect("keeps fork workflows awaiting approval out of the passing state", () => | ||
| 168 | Effect.gen(function* () { | ||
| 169 | const provider = yield* make; | ||
| 170 | const detail = yield* provider.getChangeRequest({ | ||
| 171 | cwd: "/w", | ||
| 172 | repository: "acme/web", | ||
| 173 | host: "github.com", | ||
| 174 | number: 7, | ||
| 175 | }); | ||
| 176 | |||
| 177 | expect(detail.workflowApprovalsRequired).toBe(1); | ||
| 178 | expect(detail.checks).toEqual([ | ||
| 179 | { | ||
| 180 | name: "manual gate", | ||
| 181 | status: "action-required", | ||
| 182 | description: null, | ||
| 183 | url: "https://example.com/manual-gate", | ||
| 184 | }, | ||
| 185 | { | ||
| 186 | name: "build", | ||
| 187 | status: "success", | ||
| 188 | description: null, | ||
| 189 | url: null, | ||
| 190 | }, | ||
| 191 | { | ||
| 192 | name: "contributor tests", | ||
| 193 | status: "action-required", | ||
| 194 | description: "A maintainer must approve this workflow before it can run.", | ||
| 195 | url: "https://github.com/acme/web/actions/runs/123", | ||
| 196 | }, | ||
| 197 | ]); | ||
| 198 | }).pipe( | ||
| 199 | Effect.provide( | ||
| 200 | Layer.mock(GitHubPullRequestCli.GitHubPullRequestCli)({ | ||
| 201 | getPullRequestDetail: () => | ||
| 202 | Effect.succeed({ | ||
| 203 | authorId: null, | ||
| 204 | number: 7, | ||
| 205 | title: "Pull request 7", | ||
| 206 | url: "https://github.com/acme/web/pull/7", | ||
| 207 | author: null, | ||
| 208 | isCrossRepository: true, | ||
| 209 | headRepositoryOwner: "octocat", | ||
| 210 | headSha: "abc123", | ||
| 211 | headBranch: "feat/page", | ||
| 212 | baseBranch: "main", | ||
| 213 | state: "open", | ||
| 214 | isDraft: false, | ||
| 215 | mergeability: "mergeable", | ||
| 216 | reviewDecision: null, | ||
| 217 | additions: 1, | ||
| 218 | deletions: 1, | ||
| 219 | createdAt: "2026-07-01T00:00:00Z", | ||
| 220 | updatedAt: "2026-07-02T00:00:00Z", | ||
| 221 | reviewRequestLogins: [], | ||
| 222 | hasTeamReviewRequest: false, | ||
| 223 | checksState: "passing", | ||
| 224 | labels: [], | ||
| 225 | body: "", | ||
| 226 | changedFiles: 1, | ||
| 227 | mergedAt: null, | ||
| 228 | closedAt: null, | ||
| 229 | checks: [ | ||
| 230 | { | ||
| 231 | name: "manual gate", | ||
| 232 | status: "action-required", | ||
| 233 | description: null, | ||
| 234 | url: "https://example.com/manual-gate", | ||
| 235 | }, | ||
| 236 | { name: "build", status: "success", description: null, url: null }, | ||
| 237 | ], | ||
| 238 | comments: [], | ||
| 239 | commits: [], | ||
| 240 | }), | ||
| 241 | listWorkflowRunsRequiringApproval: () => | ||
| 242 | Effect.succeed([ | ||
| 243 | { | ||
| 244 | id: 123, | ||
| 245 | name: "contributor tests", | ||
| 246 | url: "https://github.com/acme/web/actions/runs/123", | ||
| 247 | }, | ||
| 248 | ]), | ||
| 249 | getPullRequestBaseComparison: () => | ||
| 250 | Effect.succeed({ behindBy: 0, viewerCanUpdate: true }), | ||
| 251 | getRepositoryAccess: () => | ||
| 252 | Effect.succeed({ | ||
| 253 | canWrite: true, | ||
| 254 | mergeCapabilities: { merge: true, squash: true, rebase: true }, | ||
| 255 | }), | ||
| 256 | getViewerAccess: () => | ||
| 257 | Effect.succeed({ canWrite: true, canUpdate: true, didAuthor: false }), | ||
| 258 | }), | ||
| 259 | ), | ||
| 260 | ), | ||
| 261 | ); | ||
| 155 | }); | 262 | }); |
| 156 | 263 | ||
| 157 | describe("getViewerPermissions", () => { | 264 | const openDetail = { |
| 158 | const openDetail = { | 265 | authorId: null, |
| 159 | authorId: null, | 266 | number: 7, |
| 160 | number: 7, | 267 | title: "Pull request 7", |
| 161 | title: "Pull request 7", | 268 | url: "https://github.com/acme/web/pull/7", |
| 162 | url: "https://github.com/acme/web/pull/7", | 269 | author: null, |
| 163 | author: null, | 270 | isCrossRepository: true, |
| 164 | headRepositoryOwner: "acme", | 271 | headRepositoryOwner: "acme", |
| 165 | headBranch: "feat/page", | 272 | headSha: "abc123", |
| 166 | baseBranch: "main", | 273 | headBranch: "feat/page", |
| 167 | state: "open" as const, | 274 | baseBranch: "main", |
| 168 | isDraft: false, | 275 | state: "open" as const, |
| 169 | mergeability: "mergeable" as const, | 276 | isDraft: false, |
| 170 | reviewDecision: null, | 277 | mergeability: "mergeable" as const, |
| 171 | additions: 1, | 278 | reviewDecision: null, |
| 172 | deletions: 1, | 279 | additions: 1, |
| 173 | createdAt: "2026-07-01T00:00:00Z", | 280 | deletions: 1, |
| 174 | updatedAt: "2026-07-02T00:00:00Z", | 281 | createdAt: "2026-07-01T00:00:00Z", |
| 175 | reviewRequestLogins: [], | 282 | updatedAt: "2026-07-02T00:00:00Z", |
| 176 | hasTeamReviewRequest: false, | 283 | reviewRequestLogins: [], |
| 177 | checksState: null, | 284 | hasTeamReviewRequest: false, |
| 178 | labels: [], | 285 | checksState: null, |
| 179 | body: "", | 286 | labels: [], |
| 180 | changedFiles: 1, | 287 | body: "", |
| 181 | mergedAt: null, | 288 | changedFiles: 1, |
| 182 | closedAt: null, | 289 | mergedAt: null, |
| 183 | checks: [], | 290 | closedAt: null, |
| 184 | comments: [], | 291 | checks: [], |
| 185 | commits: [], | 292 | comments: [], |
| 186 | }; | 293 | commits: [], |
| 294 | }; | ||
| 295 | |||
| 296 | it.effect("does not classify same-repository gates as fork workflow approvals", () => | ||
| 297 | Effect.gen(function* () { | ||
| 298 | const provider = yield* make; | ||
| 299 | const detail = yield* provider.getChangeRequest({ | ||
| 300 | cwd: "/w", | ||
| 301 | repository: "acme/web", | ||
| 302 | host: "github.com", | ||
| 303 | number: 7, | ||
| 304 | }); | ||
| 187 | 305 | ||
| 306 | expect(detail.workflowApprovalsRequired).toBe(0); | ||
| 307 | expect(detail.checks).toEqual([]); | ||
| 308 | }).pipe( | ||
| 309 | Effect.provide( | ||
| 310 | Layer.mock(GitHubPullRequestCli.GitHubPullRequestCli)({ | ||
| 311 | getPullRequestDetail: () => Effect.succeed({ ...openDetail, isCrossRepository: false }), | ||
| 312 | getPullRequestBaseComparison: () => Effect.succeed({ behindBy: 0, viewerCanUpdate: true }), | ||
| 313 | listWorkflowRunsRequiringApproval: () => | ||
| 314 | Effect.die("same-repository pull requests must not probe fork workflow approvals"), | ||
| 315 | getRepositoryAccess: () => | ||
| 316 | Effect.succeed({ | ||
| 317 | canWrite: true, | ||
| 318 | mergeCapabilities: { merge: true, squash: true, rebase: true }, | ||
| 319 | }), | ||
| 320 | getViewerAccess: () => | ||
| 321 | Effect.succeed({ canWrite: true, canUpdate: true, didAuthor: false }), | ||
| 322 | }), | ||
| 323 | ), | ||
| 324 | ), | ||
| 325 | ); | ||
| 326 | |||
| 327 | it.effect("keeps an unsafe workflow approval scope visible as unknown", () => | ||
| 328 | Effect.gen(function* () { | ||
| 329 | const provider = yield* make; | ||
| 330 | const detail = yield* provider.getChangeRequest({ | ||
| 331 | cwd: "/w", | ||
| 332 | repository: "acme/web", | ||
| 333 | host: "github.com", | ||
| 334 | number: 7, | ||
| 335 | }); | ||
| 336 | |||
| 337 | expect(detail.workflowApprovalsRequired).toBeUndefined(); | ||
| 338 | expect(detail.checks).toEqual([ | ||
| 339 | { | ||
| 340 | name: "Workflow approval status", | ||
| 341 | status: "action-required", | ||
| 342 | description: "GitHub could not determine whether workflows are awaiting approval.", | ||
| 343 | url: null, | ||
| 344 | }, | ||
| 345 | ]); | ||
| 346 | }).pipe( | ||
| 347 | Effect.provide( | ||
| 348 | Layer.mock(GitHubPullRequestCli.GitHubPullRequestCli)({ | ||
| 349 | getPullRequestDetail: () => Effect.succeed(openDetail), | ||
| 350 | getPullRequestBaseComparison: () => Effect.succeed({ behindBy: 0, viewerCanUpdate: true }), | ||
| 351 | listWorkflowRunsRequiringApproval: () => | ||
| 352 | Effect.fail( | ||
| 353 | new GitHubPullRequestCli.GitHubWorkflowApprovalRefusedError({ | ||
| 354 | command: "gh", | ||
| 355 | cwd: "/w", | ||
| 356 | number: 7, | ||
| 357 | reason: "head-not-unique", | ||
| 358 | observedCount: 2, | ||
| 359 | limit: 1_000, | ||
| 360 | }), | ||
| 361 | ), | ||
| 362 | getRepositoryAccess: () => | ||
| 363 | Effect.succeed({ | ||
| 364 | canWrite: true, | ||
| 365 | mergeCapabilities: { merge: true, squash: true, rebase: true }, | ||
| 366 | }), | ||
| 367 | getViewerAccess: () => | ||
| 368 | Effect.succeed({ canWrite: true, canUpdate: true, didAuthor: false }), | ||
| 369 | }), | ||
| 370 | ), | ||
| 371 | ), | ||
| 372 | ); | ||
| 373 | |||
| 374 | it.effect("propagates workflow discovery rate limits", () => | ||
| 375 | Effect.gen(function* () { | ||
| 376 | const provider = yield* make; | ||
| 377 | const error = yield* provider | ||
| 378 | .getChangeRequest({ | ||
| 379 | cwd: "/w", | ||
| 380 | repository: "acme/web", | ||
| 381 | host: "github.com", | ||
| 382 | number: 7, | ||
| 383 | }) | ||
| 384 | .pipe(Effect.flip); | ||
| 385 | |||
| 386 | expect(error.operation).toBe("getChangeRequest"); | ||
| 387 | expect(error.reason).toBe("rate-limited"); | ||
| 388 | }).pipe( | ||
| 389 | Effect.provide( | ||
| 390 | Layer.mock(GitHubPullRequestCli.GitHubPullRequestCli)({ | ||
| 391 | getPullRequestDetail: () => Effect.succeed(openDetail), | ||
| 392 | getPullRequestBaseComparison: () => Effect.succeed({ behindBy: 0, viewerCanUpdate: true }), | ||
| 393 | listWorkflowRunsRequiringApproval: () => | ||
| 394 | Effect.fail( | ||
| 395 | new GitHubCli.GitHubCliRateLimitError({ | ||
| 396 | command: "gh", | ||
| 397 | cwd: "/w", | ||
| 398 | cause: new Error("rate limited"), | ||
| 399 | }), | ||
| 400 | ), | ||
| 401 | getRepositoryAccess: () => | ||
| 402 | Effect.succeed({ | ||
| 403 | canWrite: true, | ||
| 404 | mergeCapabilities: { merge: true, squash: true, rebase: true }, | ||
| 405 | }), | ||
| 406 | getViewerAccess: () => | ||
| 407 | Effect.succeed({ canWrite: true, canUpdate: true, didAuthor: false }), | ||
| 408 | }), | ||
| 409 | ), | ||
| 410 | ), | ||
| 411 | ); | ||
| 412 | |||
| 413 | describe("getViewerPermissions", () => { | ||
| 188 | const layerWithComparison = ( | 414 | const layerWithComparison = ( |
| 189 | comparison: Effect.Effect<{ | 415 | comparison: Effect.Effect<{ |
| 190 | readonly behindBy: number | null; | 416 | readonly behindBy: number | null; |
apps/server/src/pullRequest/GitHubPullRequestProvider.ts+106-16| ... | @@ -5,6 +5,7 @@ import * as Exit from "effect/Exit"; | ... | @@ -5,6 +5,7 @@ import * as Exit from "effect/Exit"; |
| 5 | import type { | 5 | import type { |
| 6 | PullRequestActor, | 6 | PullRequestActor, |
| 7 | PullRequestCapabilities, | 7 | PullRequestCapabilities, |
| 8 | PullRequestCheck, | ||
| 8 | PullRequestReaction, | 9 | PullRequestReaction, |
| 9 | PullRequestViewerPermissions, | 10 | PullRequestViewerPermissions, |
| 10 | } from "@t3tools/contracts"; | 11 | } from "@t3tools/contracts"; |
| ... | @@ -17,7 +18,7 @@ import { | ... | @@ -17,7 +18,7 @@ import { |
| 17 | type ProviderChangeRequestDetail, | 18 | type ProviderChangeRequestDetail, |
| 18 | type PullRequestProviderApi, | 19 | type PullRequestProviderApi, |
| 19 | } from "./PullRequestProvider.ts"; | 20 | } from "./PullRequestProvider.ts"; |
| 20 | import type { GitHubViewerAccess } from "./gitHubPullRequestJson.ts"; | 21 | import type { GitHubViewerAccess, GitHubWorkflowRunApproval } from "./gitHubPullRequestJson.ts"; |
| 21 | 22 | ||
| 22 | const CAPABILITIES: PullRequestCapabilities = { | 23 | const CAPABILITIES: PullRequestCapabilities = { |
| 23 | diff: true, | 24 | diff: true, |
| ... | @@ -31,6 +32,8 @@ const CAPABILITIES: PullRequestCapabilities = { | ... | @@ -31,6 +32,8 @@ const CAPABILITIES: PullRequestCapabilities = { |
| 31 | "update-branch", | 32 | "update-branch", |
| 32 | "enable-auto-merge", | 33 | "enable-auto-merge", |
| 33 | "disable-auto-merge", | 34 | "disable-auto-merge", |
| 35 | "revert", | ||
| 36 | "approve-workflows", | ||
| 34 | ], | 37 | ], |
| 35 | mergeMethods: ["merge", "squash", "rebase"], | 38 | mergeMethods: ["merge", "squash", "rebase"], |
| 36 | updateMethods: ["merge", "rebase"], | 39 | updateMethods: ["merge", "rebase"], |
| ... | @@ -68,7 +71,15 @@ export function gitHubViewerPermissions(access: GitHubViewerAccess): PullRequest | ... | @@ -68,7 +71,15 @@ export function gitHubViewerPermissions(access: GitHubViewerAccess): PullRequest |
| 68 | actions: [ | 71 | actions: [ |
| 69 | // Arming a merge and taking the arming back are the merge, deferred: whoever may not | 72 | // Arming a merge and taking the arming back are the merge, deferred: whoever may not |
| 70 | // merge here may not leave an instruction to merge later either. | 73 | // merge here may not leave an instruction to merge later either. |
| 71 | ...(access.canWrite ? (["merge", "enable-auto-merge", "disable-auto-merge"] as const) : []), | 74 | ...(access.canWrite |
| 75 | ? ([ | ||
| 76 | "merge", | ||
| 77 | "enable-auto-merge", | ||
| 78 | "disable-auto-merge", | ||
| 79 | "revert", | ||
| 80 | "approve-workflows", | ||
| 81 | ] as const) | ||
| 82 | : []), | ||
| 72 | ...(access.canUpdate ? (["ready", "draft", "close", "reopen"] as const) : []), | 83 | ...(access.canUpdate ? (["ready", "draft", "close", "reopen"] as const) : []), |
| 73 | // Whether this viewer may update the branch is GitHub's own answer, read with the | 84 | // Whether this viewer may update the branch is GitHub's own answer, read with the |
| 74 | // comparison; without it the action is offered to nobody rather than to everybody. | 85 | // comparison; without it the action is offered to nobody rather than to everybody. |
| ... | @@ -117,6 +128,43 @@ function withAvatar( | ... | @@ -117,6 +128,43 @@ function withAvatar( |
| 117 | return avatarUrl === null ? actor : { ...actor, avatarUrl }; | 128 | return avatarUrl === null ? actor : { ...actor, avatarUrl }; |
| 118 | } | 129 | } |
| 119 | 130 | ||
| 131 | function withWorkflowApprovals( | ||
| 132 | checks: ReadonlyArray<PullRequestCheck>, | ||
| 133 | runs: ReadonlyArray<GitHubWorkflowRunApproval>, | ||
| 134 | unavailable: boolean, | ||
| 135 | ): ReadonlyArray<PullRequestCheck> { | ||
| 136 | const representedRunIds = new Set<number>(); | ||
| 137 | for (const check of checks) { | ||
| 138 | if (check.status !== "action-required" || check.url === null) continue; | ||
| 139 | const id = check.url.match(/\/actions\/runs\/(\d+)(?:\/|$)/)?.[1]; | ||
| 140 | if (id !== undefined) representedRunIds.add(Number(id)); | ||
| 141 | } | ||
| 142 | const approvalChecks = runs | ||
| 143 | .filter((run) => !representedRunIds.has(run.id)) | ||
| 144 | .map( | ||
| 145 | (run): PullRequestCheck => ({ | ||
| 146 | name: run.name, | ||
| 147 | status: "action-required", | ||
| 148 | description: "A maintainer must approve this workflow before it can run.", | ||
| 149 | url: run.url, | ||
| 150 | }), | ||
| 151 | ); | ||
| 152 | return [ | ||
| 153 | ...checks, | ||
| 154 | ...approvalChecks, | ||
| 155 | ...(unavailable | ||
| 156 | ? [ | ||
| 157 | { | ||
| 158 | name: "Workflow approval status", | ||
| 159 | status: "action-required" as const, | ||
| 160 | description: "GitHub could not determine whether workflows are awaiting approval.", | ||
| 161 | url: null, | ||
| 162 | }, | ||
| 163 | ] | ||
| 164 | : []), | ||
| 165 | ]; | ||
| 166 | } | ||
| 167 | |||
| 120 | /** | 168 | /** |
| 121 | * Null for anything that is not a plain user login: an app posts as `dependabot[bot]`, which | 169 | * Null for anything that is not a plain user login: an app posts as `dependabot[bot]`, which |
| 122 | * names no page, and a guessed URL that 404s is worse than the initials it would replace. | 170 | * names no page, and a guessed URL that 404s is worse than the initials it would replace. |
| ... | @@ -250,20 +298,54 @@ export const make = Effect.gen(function* () { | ... | @@ -250,20 +298,54 @@ export const make = Effect.gen(function* () { |
| 250 | [ | 298 | [ |
| 251 | cli.getPullRequestDetail(input).pipe( | 299 | cli.getPullRequestDetail(input).pipe( |
| 252 | Effect.flatMap((pullRequest) => | 300 | Effect.flatMap((pullRequest) => |
| 253 | // Only an open pull request can be behind anything worth saying so about, and only | 301 | Effect.all({ |
| 254 | // one whose head repository is known can be compared at all. A comparison that | 302 | // Only an open pull request can be behind anything worth saying so about, and |
| 255 | // fails is left unknown: the banner is an offer, never a blocker. | 303 | // only one whose head repository is known can be compared at all. |
| 256 | pullRequest.state !== "open" || pullRequest.headRepositoryOwner === null | 304 | comparison: |
| 257 | ? Effect.succeed({ pullRequest, comparison: null }) | 305 | pullRequest.state !== "open" || pullRequest.headRepositoryOwner === null |
| 258 | : cli | 306 | ? Effect.succeed(null) |
| 259 | .getPullRequestBaseComparison({ | 307 | : cli |
| 260 | ...input, | 308 | .getPullRequestBaseComparison({ |
| 261 | headRef: `${pullRequest.headRepositoryOwner}:${pullRequest.headBranch}`, | 309 | ...input, |
| 262 | }) | 310 | headRef: `${pullRequest.headRepositoryOwner}:${pullRequest.headBranch}`, |
| 263 | .pipe( | 311 | }) |
| 264 | Effect.map((comparison) => ({ pullRequest, comparison })), | 312 | .pipe(Effect.orElseSucceed(() => null)), |
| 265 | Effect.orElseSucceed(() => ({ pullRequest, comparison: null })), | 313 | // GitHub omits a fork workflow that has not been approved from the normal check |
| 266 | ), | 314 | // rollup. Read the action-required runs by head revision so "all passed" cannot |
| 315 | // be shown while a whole workflow is still waiting to start. | ||
| 316 | workflowApprovals: | ||
| 317 | pullRequest.state !== "open" || pullRequest.isCrossRepository !== true | ||
| 318 | ? Effect.succeed({ | ||
| 319 | runs: [] as ReadonlyArray<GitHubWorkflowRunApproval>, | ||
| 320 | unavailable: false, | ||
| 321 | }) | ||
| 322 | : pullRequest.headSha == null || pullRequest.headRepositoryOwner == null | ||
| 323 | ? Effect.succeed({ | ||
| 324 | runs: [] as ReadonlyArray<GitHubWorkflowRunApproval>, | ||
| 325 | unavailable: true, | ||
| 326 | }) | ||
| 327 | : cli | ||
| 328 | .listWorkflowRunsRequiringApproval({ | ||
| 329 | ...input, | ||
| 330 | headSha: pullRequest.headSha, | ||
| 331 | headBranch: pullRequest.headBranch, | ||
| 332 | headRepositoryOwner: pullRequest.headRepositoryOwner, | ||
| 333 | isCrossRepository: true, | ||
| 334 | }) | ||
| 335 | .pipe( | ||
| 336 | Effect.matchEffect({ | ||
| 337 | onFailure: (error) => | ||
| 338 | error._tag === "GitHubCliRateLimitError" || | ||
| 339 | error._tag === "SourceControlRateLimitPausedError" | ||
| 340 | ? Effect.fail(error) | ||
| 341 | : Effect.succeed({ | ||
| 342 | runs: [] as ReadonlyArray<GitHubWorkflowRunApproval>, | ||
| 343 | unavailable: true, | ||
| 344 | }), | ||
| 345 | onSuccess: (runs) => Effect.succeed({ runs, unavailable: false }), | ||
| 346 | }), | ||
| 347 | ), | ||
| 348 | }).pipe(Effect.map((extra) => ({ pullRequest, ...extra }))), | ||
| 267 | ), | 349 | ), |
| 268 | ), | 350 | ), |
| 269 | getRepositoryAccess({ | 351 | getRepositoryAccess({ |
| ... | @@ -281,6 +363,14 @@ export const make = Effect.gen(function* () { | ... | @@ -281,6 +363,14 @@ export const make = Effect.gen(function* () { |
| 281 | Effect.map( | 363 | Effect.map( |
| 282 | ([detail, repository, viewerAccess]): ProviderChangeRequestDetail => ({ | 364 | ([detail, repository, viewerAccess]): ProviderChangeRequestDetail => ({ |
| 283 | ...detail.pullRequest, | 365 | ...detail.pullRequest, |
| 366 | checks: withWorkflowApprovals( | ||
| 367 | detail.pullRequest.checks, | ||
| 368 | detail.workflowApprovals.runs, | ||
| 369 | detail.workflowApprovals.unavailable, | ||
| 370 | ), | ||
| 371 | ...(detail.workflowApprovals.unavailable | ||
| 372 | ? {} | ||
| 373 | : { workflowApprovalsRequired: detail.workflowApprovals.runs.length }), | ||
| 284 | reviewers: detail.pullRequest.reviewRequestLogins.map((login) => ({ | 374 | reviewers: detail.pullRequest.reviewRequestLogins.map((login) => ({ |
| 285 | login, | 375 | login, |
| 286 | name: null, | 376 | name: null, |
apps/server/src/pullRequest/GitLabPullRequestCli.ts+4| ... | @@ -504,6 +504,10 @@ function actionArgs( | ... | @@ -504,6 +504,10 @@ function actionArgs( |
| 504 | return ["rebase"]; | 504 | return ["rebase"]; |
| 505 | case "reopen": | 505 | case "reopen": |
| 506 | return ["reopen"]; | 506 | return ["reopen"]; |
| 507 | // Never reached: this host does not declare the action, so the service refuses it first. | ||
| 508 | case "revert": | ||
| 509 | case "approve-workflows": | ||
| 510 | throw new Error(`GitLab merge request action ${action} is unsupported`); | ||
| 507 | } | 511 | } |
| 508 | } | 512 | } |
| 509 | 513 |
apps/server/src/pullRequest/PullRequestProvider.ts+4| ... | @@ -177,6 +177,10 @@ export interface ProviderChangeRequestDetail extends ProviderChangeRequest { | ... | @@ -177,6 +177,10 @@ export interface ProviderChangeRequestDetail extends ProviderChangeRequest { |
| 177 | readonly behindBy?: number; | 177 | readonly behindBy?: number; |
| 178 | /** Absent from a host that does not report whether it is armed to merge this on its own. */ | 178 | /** Absent from a host that does not report whether it is armed to merge this on its own. */ |
| 179 | readonly autoMergeEnabled?: boolean; | 179 | readonly autoMergeEnabled?: boolean; |
| 180 | /** The strategy stored with an armed auto-merge, where the host reports it. */ | ||
| 181 | readonly autoMergeMethod?: PullRequestMergeMethod; | ||
| 182 | /** Workflow runs on this head commit that still need a maintainer's approval. */ | ||
| 183 | readonly workflowApprovalsRequired?: number; | ||
| 180 | } | 184 | } |
| 181 | 185 | ||
| 182 | /** The conversation-shaped half of a detail, loaded after the core can already render. */ | 186 | /** The conversation-shaped half of a detail, loaded after the core can already render. */ |
apps/server/src/pullRequest/PullRequestService.ts+9| ... | @@ -202,6 +202,9 @@ const ACTION_ACCESS_REFUSALS: Record<PullRequestAction, string> = { | ... | @@ -202,6 +202,9 @@ const ACTION_ACCESS_REFUSALS: Record<PullRequestAction, string> = { |
| 202 | "You need write access on this repository to have it merged for you once it is ready.", | 202 | "You need write access on this repository to have it merged for you once it is ready.", |
| 203 | "disable-auto-merge": | 203 | "disable-auto-merge": |
| 204 | "You need write access on this repository to stop it being merged for you once it is ready.", | 204 | "You need write access on this repository to stop it being merged for you once it is ready.", |
| 205 | revert: "You need write access on this repository to open a revert pull request.", | ||
| 206 | "approve-workflows": | ||
| 207 | "You need write access on this repository to approve workflows from a fork pull request.", | ||
| 205 | }; | 208 | }; |
| 206 | 209 | ||
| 207 | /** | 210 | /** |
| ... | @@ -1290,6 +1293,12 @@ export const make = Effect.gen(function* () { | ... | @@ -1290,6 +1293,12 @@ export const make = Effect.gen(function* () { |
| 1290 | ...(changeRequest.autoMergeEnabled === undefined | 1293 | ...(changeRequest.autoMergeEnabled === undefined |
| 1291 | ? {} | 1294 | ? {} |
| 1292 | : { autoMergeEnabled: changeRequest.autoMergeEnabled }), | 1295 | : { autoMergeEnabled: changeRequest.autoMergeEnabled }), |
| 1296 | ...(changeRequest.autoMergeMethod === undefined | ||
| 1297 | ? {} | ||
| 1298 | : { autoMergeMethod: changeRequest.autoMergeMethod }), | ||
| 1299 | ...(changeRequest.workflowApprovalsRequired === undefined | ||
| 1300 | ? {} | ||
| 1301 | : { workflowApprovalsRequired: changeRequest.workflowApprovalsRequired }), | ||
| 1293 | }), | 1302 | }), |
| 1294 | ), | 1303 | ), |
| 1295 | ), | 1304 | ), |
apps/server/src/pullRequest/azureDevOpsPullRequestJson.test.ts+28| ... | @@ -159,6 +159,34 @@ describe("decodePullRequestJson", () => { | ... | @@ -159,6 +159,34 @@ describe("decodePullRequestJson", () => { |
| 159 | ); | 159 | ); |
| 160 | }); | 160 | }); |
| 161 | 161 | ||
| 162 | it("keeps the strategy stored with auto-complete", () => { | ||
| 163 | const armed = expectSuccess( | ||
| 164 | decodePullRequestJson( | ||
| 165 | asJson( | ||
| 166 | pullRequest({ | ||
| 167 | autoCompleteSetBy: { displayName: "Bilal Hassan" }, | ||
| 168 | completionOptions: { mergeStrategy: "squash" }, | ||
| 169 | }), | ||
| 170 | ), | ||
| 171 | ), | ||
| 172 | ); | ||
| 173 | |||
| 174 | expect(armed).toMatchObject({ autoMergeEnabled: true, autoMergeMethod: "squash" }); | ||
| 175 | |||
| 176 | const unspecified = expectSuccess( | ||
| 177 | decodePullRequestJson( | ||
| 178 | asJson( | ||
| 179 | pullRequest({ | ||
| 180 | autoCompleteSetBy: { displayName: "Bilal Hassan" }, | ||
| 181 | completionOptions: { squashMerge: false }, | ||
| 182 | }), | ||
| 183 | ), | ||
| 184 | ), | ||
| 185 | ); | ||
| 186 | expect(unspecified?.autoMergeEnabled).toBe(true); | ||
| 187 | expect(unspecified?.autoMergeMethod).toBeUndefined(); | ||
| 188 | }); | ||
| 189 | |||
| 162 | it("works out where the conversation lives from what Azure returned", () => { | 190 | it("works out where the conversation lives from what Azure returned", () => { |
| 163 | const detail = expectSuccess(decodePullRequestJson(asJson(pullRequest()))); | 191 | const detail = expectSuccess(decodePullRequestJson(asJson(pullRequest()))); |
| 164 | 192 |
apps/server/src/pullRequest/azureDevOpsPullRequestJson.ts+30| ... | @@ -5,6 +5,7 @@ import * as Schema from "effect/Schema"; | ... | @@ -5,6 +5,7 @@ import * as Schema from "effect/Schema"; |
| 5 | import type { | 5 | import type { |
| 6 | PullRequestActor, | 6 | PullRequestActor, |
| 7 | PullRequestComment, | 7 | PullRequestComment, |
| 8 | PullRequestMergeMethod, | ||
| 8 | PullRequestMergeability, | 9 | PullRequestMergeability, |
| 9 | PullRequestState, | 10 | PullRequestState, |
| 10 | } from "@t3tools/contracts"; | 11 | } from "@t3tools/contracts"; |
| ... | @@ -41,6 +42,14 @@ const RawPullRequestSchema = Schema.Struct({ | ... | @@ -41,6 +42,14 @@ const RawPullRequestSchema = Schema.Struct({ |
| 41 | * entirely once nobody has. So its presence is the answer, and there is no third state. | 42 | * entirely once nobody has. So its presence is the answer, and there is no third state. |
| 42 | */ | 43 | */ |
| 43 | autoCompleteSetBy: Schema.optional(Schema.NullOr(RawIdentitySchema)), | 44 | autoCompleteSetBy: Schema.optional(Schema.NullOr(RawIdentitySchema)), |
| 45 | completionOptions: Schema.optional( | ||
| 46 | Schema.NullOr( | ||
| 47 | Schema.Struct({ | ||
| 48 | mergeStrategy: Schema.optional(Schema.NullOr(Schema.String)), | ||
| 49 | squashMerge: Schema.optional(Schema.NullOr(Schema.Boolean)), | ||
| 50 | }), | ||
| 51 | ), | ||
| 52 | ), | ||
| 44 | mergeStatus: Schema.optional(Schema.NullOr(Schema.String)), | 53 | mergeStatus: Schema.optional(Schema.NullOr(Schema.String)), |
| 45 | createdBy: Schema.optional(Schema.NullOr(RawIdentitySchema)), | 54 | createdBy: Schema.optional(Schema.NullOr(RawIdentitySchema)), |
| 46 | reviewers: Schema.optional(Schema.NullOr(Schema.Array(RawIdentitySchema))), | 55 | reviewers: Schema.optional(Schema.NullOr(Schema.Array(RawIdentitySchema))), |
| ... | @@ -132,6 +141,8 @@ export interface AzureDevOpsPullRequest { | ... | @@ -132,6 +141,8 @@ export interface AzureDevOpsPullRequest { |
| 132 | readonly threadsUrl: string | null; | 141 | readonly threadsUrl: string | null; |
| 133 | /** Whether Azure is set to complete this on its own once its policies pass. */ | 142 | /** Whether Azure is set to complete this on its own once its policies pass. */ |
| 134 | readonly autoMergeEnabled: boolean; | 143 | readonly autoMergeEnabled: boolean; |
| 144 | /** The completion strategy Azure stored with auto-complete, where it reported one. */ | ||
| 145 | readonly autoMergeMethod?: PullRequestMergeMethod; | ||
| 135 | } | 146 | } |
| 136 | 147 | ||
| 137 | function trimmed(value: string | null | undefined): string | null { | 148 | function trimmed(value: string | null | undefined): string | null { |
| ... | @@ -188,6 +199,23 @@ function toThreadsUrl(raw: Schema.Schema.Type<typeof RawPullRequestSchema>): str | ... | @@ -188,6 +199,23 @@ function toThreadsUrl(raw: Schema.Schema.Type<typeof RawPullRequestSchema>): str |
| 188 | return `${base}/${encodeURIComponent(project)}/_apis/git/repositories/${encodeURIComponent(repository)}/pullRequests/${raw.pullRequestId}/threads`; | 199 | return `${base}/${encodeURIComponent(project)}/_apis/git/repositories/${encodeURIComponent(repository)}/pullRequests/${raw.pullRequestId}/threads`; |
| 189 | } | 200 | } |
| 190 | 201 | ||
| 202 | function toAutoMergeMethod( | ||
| 203 | raw: Schema.Schema.Type<typeof RawPullRequestSchema>, | ||
| 204 | ): PullRequestMergeMethod | undefined { | ||
| 205 | if (raw.autoCompleteSetBy == null) return undefined; | ||
| 206 | switch (raw.completionOptions?.mergeStrategy?.trim().toLowerCase()) { | ||
| 207 | case "squash": | ||
| 208 | return "squash"; | ||
| 209 | case "rebase": | ||
| 210 | case "rebasemerge": | ||
| 211 | return "rebase"; | ||
| 212 | case "nofastforward": | ||
| 213 | return "merge"; | ||
| 214 | default: | ||
| 215 | return raw.completionOptions?.squashMerge === true ? "squash" : undefined; | ||
| 216 | } | ||
| 217 | } | ||
| 218 | |||
| 191 | /** | 219 | /** |
| 192 | * Null when Azure said too little to place the pull request: a row with no browser url and no | 220 | * Null when Azure said too little to place the pull request: a row with no browser url and no |
| 193 | * branch left after its prefix is dropped cannot be rendered or opened, and the wire contract | 221 | * branch left after its prefix is dropped cannot be rendered or opened, and the wire contract |
| ... | @@ -196,6 +224,7 @@ function toThreadsUrl(raw: Schema.Schema.Type<typeof RawPullRequestSchema>): str | ... | @@ -196,6 +224,7 @@ function toThreadsUrl(raw: Schema.Schema.Type<typeof RawPullRequestSchema>): str |
| 196 | function toPullRequest( | 224 | function toPullRequest( |
| 197 | raw: Schema.Schema.Type<typeof RawPullRequestSchema>, | 225 | raw: Schema.Schema.Type<typeof RawPullRequestSchema>, |
| 198 | ): AzureDevOpsPullRequest | null { | 226 | ): AzureDevOpsPullRequest | null { |
| 227 | const autoMergeMethod = toAutoMergeMethod(raw); | ||
| 199 | const reviewers = (raw.reviewers ?? []).flatMap((reviewer) => { | 228 | const reviewers = (raw.reviewers ?? []).flatMap((reviewer) => { |
| 200 | const actor = toActor(reviewer); | 229 | const actor = toActor(reviewer); |
| 201 | return actor === null ? [] : [actor]; | 230 | return actor === null ? [] : [actor]; |
| ... | @@ -232,6 +261,7 @@ function toPullRequest( | ... | @@ -232,6 +261,7 @@ function toPullRequest( |
| 232 | reviewers, | 261 | reviewers, |
| 233 | threadsUrl: toThreadsUrl(raw), | 262 | threadsUrl: toThreadsUrl(raw), |
| 234 | autoMergeEnabled: (raw.autoCompleteSetBy ?? null) !== null, | 263 | autoMergeEnabled: (raw.autoCompleteSetBy ?? null) !== null, |
| 264 | ...(autoMergeMethod === undefined ? {} : { autoMergeMethod }), | ||
| 235 | }; | 265 | }; |
| 236 | } | 266 | } |
| 237 | 267 |
apps/server/src/pullRequest/gitHubPullRequestJson.test.ts+75-6| ... | @@ -16,6 +16,7 @@ import { | ... | @@ -16,6 +16,7 @@ import { |
| 16 | decodeReviewThreadCommentsJson, | 16 | decodeReviewThreadCommentsJson, |
| 17 | decodeReviewThreadsJson, | 17 | decodeReviewThreadsJson, |
| 18 | decodeViewerPermissionsJson, | 18 | decodeViewerPermissionsJson, |
| 19 | decodeWorkflowRunApprovalsJson, | ||
| 19 | reviewThreadConversation, | 20 | reviewThreadConversation, |
| 20 | REVIEW_THREADS_GRAPHQL_QUERY, | 21 | REVIEW_THREADS_GRAPHQL_QUERY, |
| 21 | } from "./gitHubPullRequestJson.ts"; | 22 | } from "./gitHubPullRequestJson.ts"; |
| ... | @@ -223,18 +224,56 @@ describe("pull request detail decoding", () => { | ... | @@ -223,18 +224,56 @@ describe("pull request detail decoding", () => { |
| 223 | ]); | 224 | ]); |
| 224 | }); | 225 | }); |
| 225 | 226 | ||
| 226 | it("reads an auto-merge request as armed, its null as off and its absence as neither", () => { | 227 | it("keeps a workflow waiting for approval out of the passing state", () => { |
| 228 | const raw = JSON.parse(detailJson) as Record<string, unknown>; | ||
| 229 | const detail = expectSuccess( | ||
| 230 | decodePullRequestDetailJson( | ||
| 231 | JSON.stringify({ | ||
| 232 | ...raw, | ||
| 233 | statusCheckRollup: [ | ||
| 234 | { __typename: "CheckRun", name: "build", status: "COMPLETED", conclusion: "SUCCESS" }, | ||
| 235 | { | ||
| 236 | __typename: "CheckRun", | ||
| 237 | name: "contributor tests", | ||
| 238 | status: "COMPLETED", | ||
| 239 | conclusion: "ACTION_REQUIRED", | ||
| 240 | }, | ||
| 241 | ], | ||
| 242 | }), | ||
| 243 | ), | ||
| 244 | ); | ||
| 245 | |||
| 246 | expect(detail.checks.map((check) => check.status)).toEqual(["success", "action-required"]); | ||
| 247 | expect(detail.checksState).toBe("pending"); | ||
| 248 | }); | ||
| 249 | |||
| 250 | it("decodes workflow runs that can be approved", () => { | ||
| 251 | expect( | ||
| 252 | expectSuccess( | ||
| 253 | decodeWorkflowRunApprovalsJson( | ||
| 254 | JSON.stringify([ | ||
| 255 | { databaseId: 10, workflowName: "contributor tests", url: "https://example.com/10" }, | ||
| 256 | { databaseId: 11, workflowName: null, url: null }, | ||
| 257 | ]), | ||
| 258 | ), | ||
| 259 | ), | ||
| 260 | ).toEqual([ | ||
| 261 | { id: 10, name: "contributor tests", url: "https://example.com/10" }, | ||
| 262 | { id: 11, name: "Workflow run 11", url: null }, | ||
| 263 | ]); | ||
| 264 | }); | ||
| 265 | |||
| 266 | it("reads an auto-merge request and strategy, its null as off and its absence as neither", () => { | ||
| 227 | const raw = JSON.parse(detailJson) as Record<string, unknown>; | 267 | const raw = JSON.parse(detailJson) as Record<string, unknown>; |
| 228 | const armed = (entry: Record<string, unknown>) => | 268 | const armed = (entry: Record<string, unknown>) => |
| 229 | expectSuccess(decodePullRequestDetailJson(JSON.stringify({ ...raw, ...entry }))) | 269 | expectSuccess(decodePullRequestDetailJson(JSON.stringify({ ...raw, ...entry }))); |
| 230 | .autoMergeEnabled; | ||
| 231 | 270 | ||
| 232 | expect( | 271 | expect( |
| 233 | armed({ autoMergeRequest: { enabledBy: { login: "octocat" }, mergeMethod: "SQUASH" } }), | 272 | armed({ autoMergeRequest: { enabledBy: { login: "octocat" }, mergeMethod: "SQUASH" } }), |
| 234 | ).toBe(true); | 273 | ).toMatchObject({ autoMergeEnabled: true, autoMergeMethod: "squash" }); |
| 235 | expect(armed({ autoMergeRequest: null })).toBe(false); | 274 | expect(armed({ autoMergeRequest: null }).autoMergeEnabled).toBe(false); |
| 236 | // `gh` not answering for the field at all is not GitHub saying the merge is unarmed. | 275 | // `gh` not answering for the field at all is not GitHub saying the merge is unarmed. |
| 237 | expect(armed({})).toBeUndefined(); | 276 | expect(armed({}).autoMergeEnabled).toBeUndefined(); |
| 238 | }); | 277 | }); |
| 239 | 278 | ||
| 240 | it("shows a re-running check once, as the run that is happening now", () => { | 279 | it("shows a re-running check once, as the run that is happening now", () => { |
| ... | @@ -419,6 +458,36 @@ describe("review thread decoding", () => { | ... | @@ -419,6 +458,36 @@ describe("review thread decoding", () => { |
| 419 | ]); | 458 | ]); |
| 420 | }); | 459 | }); |
| 421 | 460 | ||
| 461 | it("omits misleading line counts from merge commits", () => { | ||
| 462 | const result = expectSuccess( | ||
| 463 | decodeReviewThreadsJson( | ||
| 464 | JSON.stringify({ | ||
| 465 | data: { | ||
| 466 | repository: { | ||
| 467 | pullRequest: { | ||
| 468 | reviewThreads: { totalCount: 0, nodes: [] }, | ||
| 469 | commits: { | ||
| 470 | nodes: [ | ||
| 471 | { | ||
| 472 | commit: { | ||
| 473 | oid: "merge123", | ||
| 474 | additions: 36_858, | ||
| 475 | deletions: 12_928, | ||
| 476 | parents: { totalCount: 2 }, | ||
| 477 | }, | ||
| 478 | }, | ||
| 479 | ], | ||
| 480 | }, | ||
| 481 | }, | ||
| 482 | }, | ||
| 483 | }, | ||
| 484 | }), | ||
| 485 | ), | ||
| 486 | ); | ||
| 487 | |||
| 488 | expect([...result.commitStats]).toEqual([]); | ||
| 489 | }); | ||
| 490 | |||
| 422 | it("decodes the newest commits off the same connection, oldest to newest", () => { | 491 | it("decodes the newest commits off the same connection, oldest to newest", () => { |
| 423 | const result = expectSuccess( | 492 | const result = expectSuccess( |
| 424 | decodeReviewThreadsJson( | 493 | decodeReviewThreadsJson( |
apps/server/src/pullRequest/gitHubPullRequestJson.ts+115-11| ... | @@ -11,6 +11,7 @@ import type { | ... | @@ -11,6 +11,7 @@ import type { |
| 11 | PullRequestCommit, | 11 | PullRequestCommit, |
| 12 | PullRequestLabel, | 12 | PullRequestLabel, |
| 13 | PullRequestMergeCapabilities, | 13 | PullRequestMergeCapabilities, |
| 14 | PullRequestMergeMethod, | ||
| 14 | PullRequestOmittedFileStat, | 15 | PullRequestOmittedFileStat, |
| 15 | PullRequestMergeability, | 16 | PullRequestMergeability, |
| 16 | PullRequestReaction, | 17 | PullRequestReaction, |
| ... | @@ -359,17 +360,32 @@ const RawCommitSchema = Schema.Struct({ | ... | @@ -359,17 +360,32 @@ const RawCommitSchema = Schema.Struct({ |
| 359 | 360 | ||
| 360 | const RawDetailSchema = Schema.Struct({ | 361 | const RawDetailSchema = Schema.Struct({ |
| 361 | ...RawListItemSchema.fields, | 362 | ...RawListItemSchema.fields, |
| 363 | /** GitHub's explicit distinction between a fork head and a branch in the base repository. */ | ||
| 364 | isCrossRepository: Schema.optional(Schema.Boolean), | ||
| 362 | /** Names the fork a pull request came from, which is what qualifies its head ref. */ | 365 | /** Names the fork a pull request came from, which is what qualifies its head ref. */ |
| 363 | headRepositoryOwner: Schema.optional(Schema.NullOr(Schema.Struct({ login: Schema.String }))), | 366 | headRepositoryOwner: Schema.optional(Schema.NullOr(Schema.Struct({ login: Schema.String }))), |
| 367 | /** The exact head revision, used to find workflow runs that GitHub has not started yet. */ | ||
| 368 | headRefOid: Schema.optional(Schema.NullOr(Schema.String)), | ||
| 364 | body: Schema.optional(Schema.String), | 369 | body: Schema.optional(Schema.String), |
| 365 | changedFiles: Schema.optional(Schema.Int), | 370 | changedFiles: Schema.optional(Schema.Int), |
| 366 | closedAt: Schema.optional(Schema.NullOr(Schema.String)), | 371 | closedAt: Schema.optional(Schema.NullOr(Schema.String)), |
| 367 | /** | 372 | /** The standing instruction and strategy GitHub will use once its requirements are met. */ |
| 368 | * The standing instruction to merge once GitHub's own requirements are met, which is an object | 373 | autoMergeRequest: Schema.optional( |
| 369 | * describing who armed it and how, and a JSON null where nobody has. Nothing inside it is read: | 374 | Schema.NullOr(Schema.Struct({ mergeMethod: Schema.optional(Schema.NullOr(Schema.String)) })), |
| 370 | * the question the page asks is whether one exists. | 375 | ), |
| 371 | */ | 376 | }); |
| 372 | autoMergeRequest: Schema.optional(Schema.NullOr(Schema.Unknown)), | 377 | |
| 378 | const RawWorkflowRunApprovalSchema = Schema.Struct({ | ||
| 379 | databaseId: Schema.Int, | ||
| 380 | workflowName: Schema.optional(Schema.NullOr(Schema.String)), | ||
| 381 | url: Schema.optional(Schema.NullOr(Schema.String)), | ||
| 382 | }); | ||
| 383 | |||
| 384 | const RawPullRequestHeadSchema = Schema.Struct({ | ||
| 385 | number: Schema.Int, | ||
| 386 | headRefOid: Schema.String, | ||
| 387 | isCrossRepository: Schema.optional(Schema.Boolean), | ||
| 388 | headRepositoryOwner: Schema.optional(Schema.NullOr(Schema.Struct({ login: Schema.String }))), | ||
| 373 | }); | 389 | }); |
| 374 | 390 | ||
| 375 | const RawActivitySchema = Schema.Struct({ | 391 | const RawActivitySchema = Schema.Struct({ |
| ... | @@ -508,6 +524,9 @@ const RawReviewThreadsSchema = Schema.Struct({ | ... | @@ -508,6 +524,9 @@ const RawReviewThreadsSchema = Schema.Struct({ |
| 508 | committedDate: Schema.optional(Schema.NullOr(Schema.String)), | 524 | committedDate: Schema.optional(Schema.NullOr(Schema.String)), |
| 509 | additions: Schema.optional(Schema.Int), | 525 | additions: Schema.optional(Schema.Int), |
| 510 | deletions: Schema.optional(Schema.Int), | 526 | deletions: Schema.optional(Schema.Int), |
| 527 | parents: Schema.optional( | ||
| 528 | Schema.NullOr(Schema.Struct({ totalCount: Schema.optional(Schema.Int) })), | ||
| 529 | ), | ||
| 511 | authors: Schema.optional( | 530 | authors: Schema.optional( |
| 512 | Schema.NullOr( | 531 | Schema.NullOr( |
| 513 | Schema.Struct({ | 532 | Schema.Struct({ |
| ... | @@ -599,7 +618,7 @@ export function decodeActorAvatarsJson( | ... | @@ -599,7 +618,7 @@ export function decodeActorAvatarsJson( |
| 599 | export const PULL_REQUEST_LIST_JSON_FIELDS = | 618 | export const PULL_REQUEST_LIST_JSON_FIELDS = |
| 600 | "number,title,url,author,headRefName,baseRefName,state,isDraft,mergeable,reviewDecision,additions,deletions,createdAt,updatedAt,mergedAt,reviewRequests,labels,statusCheckRollup"; | 619 | "number,title,url,author,headRefName,baseRefName,state,isDraft,mergeable,reviewDecision,additions,deletions,createdAt,updatedAt,mergedAt,reviewRequests,labels,statusCheckRollup"; |
| 601 | 620 | ||
| 602 | export const PULL_REQUEST_DETAIL_JSON_FIELDS = `${PULL_REQUEST_LIST_JSON_FIELDS},body,changedFiles,closedAt,headRepositoryOwner,autoMergeRequest`; | 621 | export const PULL_REQUEST_DETAIL_JSON_FIELDS = `${PULL_REQUEST_LIST_JSON_FIELDS},body,changedFiles,closedAt,isCrossRepository,headRepositoryOwner,headRefOid,autoMergeRequest`; |
| 603 | export const PULL_REQUEST_ACTIVITY_JSON_FIELDS = "author,comments,reviews,commits"; | 622 | export const PULL_REQUEST_ACTIVITY_JSON_FIELDS = "author,comments,reviews,commits"; |
| 604 | 623 | ||
| 605 | /** GitHub's own ceiling on a connection page, which is what both thread reads ask for. */ | 624 | /** GitHub's own ceiling on a connection page, which is what both thread reads ask for. */ |
| ... | @@ -731,6 +750,7 @@ export const REVIEW_THREADS_GRAPHQL_QUERY = `query($owner: String!, $name: Strin | ... | @@ -731,6 +750,7 @@ export const REVIEW_THREADS_GRAPHQL_QUERY = `query($owner: String!, $name: Strin |
| 731 | committedDate | 750 | committedDate |
| 732 | additions | 751 | additions |
| 733 | deletions | 752 | deletions |
| 753 | parents(first: 1) { totalCount } | ||
| 734 | authors(first: 3) { nodes { name avatarUrl user { login } } } | 754 | authors(first: 3) { nodes { name avatarUrl user { login } } } |
| 735 | } | 755 | } |
| 736 | } | 756 | } |
| ... | @@ -879,6 +899,13 @@ export const UPDATE_PULL_REQUEST_GRAPHQL_MUTATION = `mutation($pullRequestId: ID | ... | @@ -879,6 +899,13 @@ export const UPDATE_PULL_REQUEST_GRAPHQL_MUTATION = `mutation($pullRequestId: ID |
| 879 | } | 899 | } |
| 880 | }`; | 900 | }`; |
| 881 | 901 | ||
| 902 | /** Creates a new pull request that reverses a merged pull request. */ | ||
| 903 | export const REVERT_PULL_REQUEST_GRAPHQL_MUTATION = `mutation($pullRequestId: ID!) { | ||
| 904 | revertPullRequest(input: { pullRequestId: $pullRequestId }) { | ||
| 905 | revertPullRequest { id } | ||
| 906 | } | ||
| 907 | }`; | ||
| 908 | |||
| 882 | /** | 909 | /** |
| 883 | * The two comment mutations name their comment differently. The variable is spelled the same in | 910 | * The two comment mutations name their comment differently. The variable is spelled the same in |
| 884 | * both, so a rewrite sends one set of variables whichever kind of remark it is. | 911 | * both, so a rewrite sends one set of variables whichever kind of remark it is. |
| ... | @@ -1016,8 +1043,11 @@ export interface GitHubPullRequestListItem { | ... | @@ -1016,8 +1043,11 @@ export interface GitHubPullRequestListItem { |
| 1016 | } | 1043 | } |
| 1017 | 1044 | ||
| 1018 | export interface GitHubPullRequestDetail extends GitHubPullRequestListItem { | 1045 | export interface GitHubPullRequestDetail extends GitHubPullRequestListItem { |
| 1046 | /** True only when GitHub says the head belongs to another repository. */ | ||
| 1047 | readonly isCrossRepository?: boolean; | ||
| 1019 | /** The owner of the head branch's repository; null where `gh` did not say. */ | 1048 | /** The owner of the head branch's repository; null where `gh` did not say. */ |
| 1020 | readonly headRepositoryOwner: string | null; | 1049 | readonly headRepositoryOwner: string | null; |
| 1050 | readonly headSha?: string | null; | ||
| 1021 | readonly body: string; | 1051 | readonly body: string; |
| 1022 | readonly changedFiles: number; | 1052 | readonly changedFiles: number; |
| 1023 | readonly mergedAt: string | null; | 1053 | readonly mergedAt: string | null; |
| ... | @@ -1025,6 +1055,21 @@ export interface GitHubPullRequestDetail extends GitHubPullRequestListItem { | ... | @@ -1025,6 +1055,21 @@ export interface GitHubPullRequestDetail extends GitHubPullRequestListItem { |
| 1025 | readonly checks: ReadonlyArray<PullRequestCheck>; | 1055 | readonly checks: ReadonlyArray<PullRequestCheck>; |
| 1026 | /** Absent where `gh` did not answer for auto-merge at all, which is not the same as off. */ | 1056 | /** Absent where `gh` did not answer for auto-merge at all, which is not the same as off. */ |
| 1027 | readonly autoMergeEnabled?: boolean; | 1057 | readonly autoMergeEnabled?: boolean; |
| 1058 | /** Absent where auto-merge is off or GitHub did not report the stored strategy. */ | ||
| 1059 | readonly autoMergeMethod?: PullRequestMergeMethod; | ||
| 1060 | } | ||
| 1061 | |||
| 1062 | export interface GitHubWorkflowRunApproval { | ||
| 1063 | readonly id: number; | ||
| 1064 | readonly name: string; | ||
| 1065 | readonly url: string | null; | ||
| 1066 | } | ||
| 1067 | |||
| 1068 | export interface GitHubPullRequestHead { | ||
| 1069 | readonly number: number; | ||
| 1070 | readonly headSha: string; | ||
| 1071 | readonly isCrossRepository?: boolean; | ||
| 1072 | readonly headRepositoryOwner: string | null; | ||
| 1028 | } | 1073 | } |
| 1029 | 1074 | ||
| 1030 | export interface GitHubPullRequestActivity { | 1075 | export interface GitHubPullRequestActivity { |
| ... | @@ -1114,6 +1159,19 @@ function toMergeability(value: string | null | undefined): PullRequestMergeabili | ... | @@ -1114,6 +1159,19 @@ function toMergeability(value: string | null | undefined): PullRequestMergeabili |
| 1114 | } | 1159 | } |
| 1115 | } | 1160 | } |
| 1116 | 1161 | ||
| 1162 | function toMergeMethod(value: string | null | undefined): PullRequestMergeMethod | undefined { | ||
| 1163 | switch (value?.trim().toUpperCase()) { | ||
| 1164 | case "MERGE": | ||
| 1165 | return "merge"; | ||
| 1166 | case "SQUASH": | ||
| 1167 | return "squash"; | ||
| 1168 | case "REBASE": | ||
| 1169 | return "rebase"; | ||
| 1170 | default: | ||
| 1171 | return undefined; | ||
| 1172 | } | ||
| 1173 | } | ||
| 1174 | |||
| 1117 | function toReviewDecision(value: string | null | undefined): PullRequestReviewDecision | null { | 1175 | function toReviewDecision(value: string | null | undefined): PullRequestReviewDecision | null { |
| 1118 | switch (value?.trim().toUpperCase()) { | 1176 | switch (value?.trim().toUpperCase()) { |
| 1119 | case "APPROVED": | 1177 | case "APPROVED": |
| ... | @@ -1169,12 +1227,12 @@ function toCheckStatus(raw: Schema.Schema.Type<typeof RawCheckSchema>): PullRequ | ... | @@ -1169,12 +1227,12 @@ function toCheckStatus(raw: Schema.Schema.Type<typeof RawCheckSchema>): PullRequ |
| 1169 | switch ((raw.conclusion ?? raw.state)?.trim().toUpperCase()) { | 1227 | switch ((raw.conclusion ?? raw.state)?.trim().toUpperCase()) { |
| 1170 | case "SUCCESS": | 1228 | case "SUCCESS": |
| 1171 | return "success"; | 1229 | return "success"; |
| 1230 | case "ACTION_REQUIRED": | ||
| 1231 | return "action-required"; | ||
| 1172 | case "FAILURE": | 1232 | case "FAILURE": |
| 1173 | case "ERROR": | 1233 | case "ERROR": |
| 1174 | case "TIMED_OUT": | 1234 | case "TIMED_OUT": |
| 1175 | case "STARTUP_FAILURE": | 1235 | case "STARTUP_FAILURE": |
| 1176 | // A completed check asking for manual intervention is blocking, not neutral. | ||
| 1177 | case "ACTION_REQUIRED": | ||
| 1178 | return "failure"; | 1236 | return "failure"; |
| 1179 | case "CANCELLED": | 1237 | case "CANCELLED": |
| 1180 | return "cancelled"; | 1238 | return "cancelled"; |
| ... | @@ -1253,7 +1311,7 @@ function rollupChecksState( | ... | @@ -1253,7 +1311,7 @@ function rollupChecksState( |
| 1253 | ]; | 1311 | ]; |
| 1254 | if (statuses.length === 0) return null; | 1312 | if (statuses.length === 0) return null; |
| 1255 | if (statuses.includes("failure")) return "failing"; | 1313 | if (statuses.includes("failure")) return "failing"; |
| 1256 | if (statuses.includes("pending")) return "pending"; | 1314 | if (statuses.includes("pending") || statuses.includes("action-required")) return "pending"; |
| 1257 | return statuses.includes("success") ? "passing" : null; | 1315 | return statuses.includes("success") ? "passing" : null; |
| 1258 | } | 1316 | } |
| 1259 | 1317 | ||
| ... | @@ -1361,9 +1419,14 @@ function toListItem(raw: Schema.Schema.Type<typeof RawListItemSchema>): GitHubPu | ... | @@ -1361,9 +1419,14 @@ function toListItem(raw: Schema.Schema.Type<typeof RawListItemSchema>): GitHubPu |
| 1361 | } | 1419 | } |
| 1362 | 1420 | ||
| 1363 | function toDetail(raw: Schema.Schema.Type<typeof RawDetailSchema>): GitHubPullRequestDetail { | 1421 | function toDetail(raw: Schema.Schema.Type<typeof RawDetailSchema>): GitHubPullRequestDetail { |
| 1422 | const autoMergeMethod = toMergeMethod(raw.autoMergeRequest?.mergeMethod); | ||
| 1364 | return { | 1423 | return { |
| 1365 | ...toListItem(raw), | 1424 | ...toListItem(raw), |
| 1425 | ...(typeof raw.isCrossRepository === "boolean" | ||
| 1426 | ? { isCrossRepository: raw.isCrossRepository } | ||
| 1427 | : {}), | ||
| 1366 | headRepositoryOwner: trimmed(raw.headRepositoryOwner?.login), | 1428 | headRepositoryOwner: trimmed(raw.headRepositoryOwner?.login), |
| 1429 | headSha: trimmed(raw.headRefOid), | ||
| 1367 | body: raw.body ?? "", | 1430 | body: raw.body ?? "", |
| 1368 | changedFiles: raw.changedFiles ?? 0, | 1431 | changedFiles: raw.changedFiles ?? 0, |
| 1369 | mergedAt: trimmed(raw.mergedAt), | 1432 | mergedAt: trimmed(raw.mergedAt), |
| ... | @@ -1374,6 +1437,7 @@ function toDetail(raw: Schema.Schema.Type<typeof RawDetailSchema>): GitHubPullRe | ... | @@ -1374,6 +1437,7 @@ function toDetail(raw: Schema.Schema.Type<typeof RawDetailSchema>): GitHubPullRe |
| 1374 | ...(raw.autoMergeRequest === undefined | 1437 | ...(raw.autoMergeRequest === undefined |
| 1375 | ? {} | 1438 | ? {} |
| 1376 | : { autoMergeEnabled: raw.autoMergeRequest !== null }), | 1439 | : { autoMergeEnabled: raw.autoMergeRequest !== null }), |
| 1440 | ...(autoMergeMethod === undefined ? {} : { autoMergeMethod }), | ||
| 1377 | }; | 1441 | }; |
| 1378 | } | 1442 | } |
| 1379 | 1443 | ||
| ... | @@ -1391,6 +1455,8 @@ const decodeSearch = decodeJsonResult(RawSearchSchema); | ... | @@ -1391,6 +1455,8 @@ const decodeSearch = decodeJsonResult(RawSearchSchema); |
| 1391 | const decodeSearchItem = Schema.decodeUnknownExit(RawSearchItemSchema); | 1455 | const decodeSearchItem = Schema.decodeUnknownExit(RawSearchItemSchema); |
| 1392 | const decodeStats = decodeJsonResult(RawStatsSchema); | 1456 | const decodeStats = decodeJsonResult(RawStatsSchema); |
| 1393 | const decodeDetail = decodeJsonResult(RawDetailSchema); | 1457 | const decodeDetail = decodeJsonResult(RawDetailSchema); |
| 1458 | const decodeWorkflowRunApprovals = decodeJsonResult(Schema.Array(RawWorkflowRunApprovalSchema)); | ||
| 1459 | const decodePullRequestHeads = decodeJsonResult(Schema.Array(RawPullRequestHeadSchema)); | ||
| 1394 | const decodeActivity = decodeJsonResult(RawActivitySchema); | 1460 | const decodeActivity = decodeJsonResult(RawActivitySchema); |
| 1395 | const decodeFileEntry = Schema.decodeUnknownExit(RawPullRequestFileSchema); | 1461 | const decodeFileEntry = Schema.decodeUnknownExit(RawPullRequestFileSchema); |
| 1396 | const decodeRepositoryAccess = decodeJsonResult(RawRepositoryAccessSchema); | 1462 | const decodeRepositoryAccess = decodeJsonResult(RawRepositoryAccessSchema); |
| ... | @@ -1551,6 +1617,37 @@ export function decodePullRequestDetailJson( | ... | @@ -1551,6 +1617,37 @@ export function decodePullRequestDetailJson( |
| 1551 | : Result.fail(decoded.failure); | 1617 | : Result.fail(decoded.failure); |
| 1552 | } | 1618 | } |
| 1553 | 1619 | ||
| 1620 | export function decodeWorkflowRunApprovalsJson( | ||
| 1621 | raw: string, | ||
| 1622 | ): Result.Result<ReadonlyArray<GitHubWorkflowRunApproval>, DecodeFailure> { | ||
| 1623 | const decoded = decodeWorkflowRunApprovals(raw); | ||
| 1624 | if (!Result.isSuccess(decoded)) return Result.fail(decoded.failure); | ||
| 1625 | return Result.succeed( | ||
| 1626 | decoded.success.map((run) => ({ | ||
| 1627 | id: run.databaseId, | ||
| 1628 | name: trimmed(run.workflowName) ?? `Workflow run ${run.databaseId}`, | ||
| 1629 | url: trimmed(run.url), | ||
| 1630 | })), | ||
| 1631 | ); | ||
| 1632 | } | ||
| 1633 | |||
| 1634 | export function decodePullRequestHeadsJson( | ||
| 1635 | raw: string, | ||
| 1636 | ): Result.Result<ReadonlyArray<GitHubPullRequestHead>, DecodeFailure> { | ||
| 1637 | const decoded = decodePullRequestHeads(raw); | ||
| 1638 | if (!Result.isSuccess(decoded)) return Result.fail(decoded.failure); | ||
| 1639 | return Result.succeed( | ||
| 1640 | decoded.success.map((pullRequest) => ({ | ||
| 1641 | number: pullRequest.number, | ||
| 1642 | headSha: pullRequest.headRefOid, | ||
| 1643 | ...(typeof pullRequest.isCrossRepository === "boolean" | ||
| 1644 | ? { isCrossRepository: pullRequest.isCrossRepository } | ||
| 1645 | : {}), | ||
| 1646 | headRepositoryOwner: trimmed(pullRequest.headRepositoryOwner?.login), | ||
| 1647 | })), | ||
| 1648 | ); | ||
| 1649 | } | ||
| 1650 | |||
| 1554 | export function decodePullRequestActivityJson( | 1651 | export function decodePullRequestActivityJson( |
| 1555 | raw: string, | 1652 | raw: string, |
| 1556 | ): Result.Result<GitHubPullRequestActivity, DecodeFailure> { | 1653 | ): Result.Result<GitHubPullRequestActivity, DecodeFailure> { |
| ... | @@ -1788,7 +1885,14 @@ export function decodeReviewThreadsJson( | ... | @@ -1788,7 +1885,14 @@ export function decodeReviewThreadsJson( |
| 1788 | const commit = node.commit; | 1885 | const commit = node.commit; |
| 1789 | const oid = trimmed(commit.oid); | 1886 | const oid = trimmed(commit.oid); |
| 1790 | if (oid === null) continue; | 1887 | if (oid === null) continue; |
| 1791 | if (commit.additions !== undefined && commit.deletions !== undefined) { | 1888 | // GitHub measures a merge commit against its first parent, so merging the base into the head |
| 1889 | // reports every upstream change as if it belonged to the pull request. There is no useful | ||
| 1890 | // per-commit stat to show for that integration commit without another comparison request. | ||
| 1891 | if ( | ||
| 1892 | (commit.parents?.totalCount ?? 1) <= 1 && | ||
| 1893 | commit.additions !== undefined && | ||
| 1894 | commit.deletions !== undefined | ||
| 1895 | ) { | ||
| 1792 | commitStats.set(oid, { | 1896 | commitStats.set(oid, { |
| 1793 | additions: Math.max(0, commit.additions), | 1897 | additions: Math.max(0, commit.additions), |
| 1794 | deletions: Math.max(0, commit.deletions), | 1898 | deletions: Math.max(0, commit.deletions), |
apps/server/src/pullRequest/gitLabMergeRequestJson.test.ts+22| ... | @@ -192,6 +192,28 @@ describe("decodeMergeRequestDetailJson", () => { | ... | @@ -192,6 +192,28 @@ describe("decodeMergeRequestDetailJson", () => { |
| 192 | expect(armed({})).toBeUndefined(); | 192 | expect(armed({})).toBeUndefined(); |
| 193 | }); | 193 | }); |
| 194 | 194 | ||
| 195 | it("keeps the squash choice stored with auto-merge", () => { | ||
| 196 | expect( | ||
| 197 | expectSuccess( | ||
| 198 | decodeMergeRequestDetailJson( | ||
| 199 | detailJson({ auto_merge_enabled: true, squash_on_merge: true }), | ||
| 200 | ), | ||
| 201 | ), | ||
| 202 | ).toMatchObject({ autoMergeEnabled: true, autoMergeMethod: "squash" }); | ||
| 203 | |||
| 204 | expect( | ||
| 205 | expectSuccess( | ||
| 206 | decodeMergeRequestDetailJson( | ||
| 207 | detailJson({ | ||
| 208 | auto_merge_enabled: true, | ||
| 209 | squash: true, | ||
| 210 | squash_on_merge: false, | ||
| 211 | }), | ||
| 212 | ), | ||
| 213 | ).autoMergeMethod, | ||
| 214 | ).toBeUndefined(); | ||
| 215 | }); | ||
| 216 | |||
| 195 | it("keeps a divergence GitLab did not count apart from a divergence of none", () => { | 217 | it("keeps a divergence GitLab did not count apart from a divergence of none", () => { |
| 196 | const behind = (entry: Record<string, unknown>) => | 218 | const behind = (entry: Record<string, unknown>) => |
| 197 | expectSuccess(decodeMergeRequestDetailJson(detailJson(entry))).divergedCommits; | 219 | expectSuccess(decodeMergeRequestDetailJson(detailJson(entry))).divergedCommits; |
apps/server/src/pullRequest/gitLabMergeRequestJson.ts+9| ... | @@ -11,6 +11,7 @@ import type { | ... | @@ -11,6 +11,7 @@ import type { |
| 11 | PullRequestLabel, | 11 | PullRequestLabel, |
| 12 | PullRequestMergeability, | 12 | PullRequestMergeability, |
| 13 | PullRequestMergeCapabilities, | 13 | PullRequestMergeCapabilities, |
| 14 | PullRequestMergeMethod, | ||
| 14 | PullRequestReaction, | 15 | PullRequestReaction, |
| 15 | PullRequestReactionContent, | 16 | PullRequestReactionContent, |
| 16 | PullRequestReviewThread, | 17 | PullRequestReviewThread, |
| ... | @@ -80,6 +81,9 @@ const RawMergeRequestSchema = Schema.Struct({ | ... | @@ -80,6 +81,9 @@ const RawMergeRequestSchema = Schema.Struct({ |
| 80 | */ | 81 | */ |
| 81 | merge_when_pipeline_succeeds: Schema.optional(Schema.NullOr(Schema.Boolean)), | 82 | merge_when_pipeline_succeeds: Schema.optional(Schema.NullOr(Schema.Boolean)), |
| 82 | auto_merge_enabled: Schema.optional(Schema.NullOr(Schema.Boolean)), | 83 | auto_merge_enabled: Schema.optional(Schema.NullOr(Schema.Boolean)), |
| 84 | /** The merge request's stored squash choice, including project-policy overrides. */ | ||
| 85 | squash_on_merge: Schema.optional(Schema.NullOr(Schema.Boolean)), | ||
| 86 | squash: Schema.optional(Schema.NullOr(Schema.Boolean)), | ||
| 83 | /** | 87 | /** |
| 84 | * How far the target branch has moved on since this one left it, which is the same number | 88 | * How far the target branch has moved on since this one left it, which is the same number |
| 85 | * GitLab's own "out of date" wording counts. It costs a walk of the two branches, so GitLab | 89 | * GitLab's own "out of date" wording counts. It costs a walk of the two branches, so GitLab |
| ... | @@ -228,6 +232,8 @@ export interface GitLabMergeRequestDetail extends GitLabMergeRequestListItem { | ... | @@ -228,6 +232,8 @@ export interface GitLabMergeRequestDetail extends GitLabMergeRequestListItem { |
| 228 | readonly reviewerIds: ReadonlyArray<number>; | 232 | readonly reviewerIds: ReadonlyArray<number>; |
| 229 | /** Absent where GitLab named neither auto-merge field, which is not the same as off. */ | 233 | /** Absent where GitLab named neither auto-merge field, which is not the same as off. */ |
| 230 | readonly autoMergeEnabled?: boolean; | 234 | readonly autoMergeEnabled?: boolean; |
| 235 | /** GitLab only exposes the stored strategy separately when that strategy is squash. */ | ||
| 236 | readonly autoMergeMethod?: PullRequestMergeMethod; | ||
| 231 | /** | 237 | /** |
| 232 | * Absent where GitLab did not count, which is not the same as a branch that has nothing behind | 238 | * Absent where GitLab did not count, which is not the same as a branch that has nothing behind |
| 233 | * it: an install too old to answer must not be read as saying the branch is current. | 239 | * it: an install too old to answer must not be read as saying the branch is current. |
| ... | @@ -379,6 +385,9 @@ function toDetail(raw: Schema.Schema.Type<typeof RawMergeRequestSchema>): GitLab | ... | @@ -379,6 +385,9 @@ function toDetail(raw: Schema.Schema.Type<typeof RawMergeRequestSchema>): GitLab |
| 379 | reviewer.id === undefined ? [] : [reviewer.id], | 385 | reviewer.id === undefined ? [] : [reviewer.id], |
| 380 | ), | 386 | ), |
| 381 | ...(autoMerge === undefined ? {} : { autoMergeEnabled: autoMerge }), | 387 | ...(autoMerge === undefined ? {} : { autoMergeEnabled: autoMerge }), |
| 388 | ...(autoMerge === true && raw.squash_on_merge === true | ||
| 389 | ? { autoMergeMethod: "squash" as const } | ||
| 390 | : {}), | ||
| 382 | ...(raw.diverged_commits_count == null ? {} : { divergedCommits: raw.diverged_commits_count }), | 391 | ...(raw.diverged_commits_count == null ? {} : { divergedCommits: raw.diverged_commits_count }), |
| 383 | }; | 392 | }; |
| 384 | } | 393 | } |
apps/web/src/components/pullRequest/PullRequestChecksPopover.tsx+2-1| ... | @@ -65,7 +65,7 @@ function ChecksBody({ checks }: { checks: ReadonlyArray<PullRequestCheck> }) { | ... | @@ -65,7 +65,7 @@ function ChecksBody({ checks }: { checks: ReadonlyArray<PullRequestCheck> }) { |
| 65 | <TooltipPopup side="top">{check.description ?? check.name}</TooltipPopup> | 65 | <TooltipPopup side="top">{check.description ?? check.name}</TooltipPopup> |
| 66 | </Tooltip> | 66 | </Tooltip> |
| 67 | <span className="shrink-0 text-muted-foreground"> | 67 | <span className="shrink-0 text-muted-foreground"> |
| 68 | {pullRequestCheckStatusLabel(check.status)} | 68 | {pullRequestCheckStatusLabel(check)} |
| 69 | </span> | 69 | </span> |
| 70 | {check.url === null ? null : ( | 70 | {check.url === null ? null : ( |
| 71 | <button | 71 | <button |
| ... | @@ -112,6 +112,7 @@ export function PullRequestChecksPopover({ | ... | @@ -112,6 +112,7 @@ export function PullRequestChecksPopover({ |
| 112 | not valid inside one. The click is stopped here so opening the checks does not also | 112 | not valid inside one. The click is stopped here so opening the checks does not also |
| 113 | select the row it sits on. */} | 113 | select the row it sits on. */} |
| 114 | <PopoverTrigger | 114 | <PopoverTrigger |
| 115 | nativeButton={false} | ||
| 115 | render={ | 116 | render={ |
| 116 | <span | 117 | <span |
| 117 | role="button" | 118 | role="button" |
apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx+181-43| ... | @@ -33,7 +33,9 @@ import { | ... | @@ -33,7 +33,9 @@ import { |
| 33 | MoreHorizontalIcon, | 33 | MoreHorizontalIcon, |
| 34 | PanelRightIcon, | 34 | PanelRightIcon, |
| 35 | PencilIcon, | 35 | PencilIcon, |
| 36 | PlayIcon, | ||
| 36 | RefreshCwIcon, | 37 | RefreshCwIcon, |
| 38 | RotateCcwIcon, | ||
| 37 | ServerIcon, | 39 | ServerIcon, |
| 38 | TriangleAlertIcon, | 40 | TriangleAlertIcon, |
| 39 | } from "lucide-react"; | 41 | } from "lucide-react"; |
| ... | @@ -118,6 +120,7 @@ import { | ... | @@ -118,6 +120,7 @@ import { |
| 118 | pullRequestFindingKey, | 120 | pullRequestFindingKey, |
| 119 | pullRequestHandoffLabels, | 121 | pullRequestHandoffLabels, |
| 120 | readableFailure, | 122 | readableFailure, |
| 123 | resolvePullRequestPrimaryControl, | ||
| 121 | resolveBaseFreshness, | 124 | resolveBaseFreshness, |
| 122 | type PullRequestFinding, | 125 | type PullRequestFinding, |
| 123 | shouldRefreshPullRequestActivity, | 126 | shouldRefreshPullRequestActivity, |
| ... | @@ -154,6 +157,8 @@ const ACTION_SUCCESS_LABELS: Record<PullRequestAction, string> = { | ... | @@ -154,6 +157,8 @@ const ACTION_SUCCESS_LABELS: Record<PullRequestAction, string> = { |
| 154 | "enable-auto-merge": | 157 | "enable-auto-merge": |
| 155 | "Auto-merge turned on — merges as soon as this is ready, sooner if it already is", | 158 | "Auto-merge turned on — merges as soon as this is ready, sooner if it already is", |
| 156 | "disable-auto-merge": "Auto-merge turned off", | 159 | "disable-auto-merge": "Auto-merge turned off", |
| 160 | revert: "Revert pull request opened", | ||
| 161 | "approve-workflows": "Workflows approved", | ||
| 157 | }; | 162 | }; |
| 158 | 163 | ||
| 159 | const MERGE_METHOD_LABELS: Record<PullRequestMergeMethod, string> = { | 164 | const MERGE_METHOD_LABELS: Record<PullRequestMergeMethod, string> = { |
| ... | @@ -172,6 +177,8 @@ const ACTION_FAILURE_LABELS: Record<PullRequestAction, string> = { | ... | @@ -172,6 +177,8 @@ const ACTION_FAILURE_LABELS: Record<PullRequestAction, string> = { |
| 172 | "update-branch": "Could not update this branch", | 177 | "update-branch": "Could not update this branch", |
| 173 | "enable-auto-merge": "Could not turn on auto-merge", | 178 | "enable-auto-merge": "Could not turn on auto-merge", |
| 174 | "disable-auto-merge": "Could not turn off auto-merge", | 179 | "disable-auto-merge": "Could not turn off auto-merge", |
| 180 | revert: "Could not open a revert pull request", | ||
| 181 | "approve-workflows": "Could not approve workflows", | ||
| 175 | }; | 182 | }; |
| 176 | 183 | ||
| 177 | /** What to try, for the times the host says only that it refused. */ | 184 | /** What to try, for the times the host says only that it refused. */ |
| ... | @@ -193,6 +200,10 @@ const ACTION_FAILURE_HINTS: Record<PullRequestAction, string> = { | ... | @@ -193,6 +200,10 @@ const ACTION_FAILURE_HINTS: Record<PullRequestAction, string> = { |
| 193 | "The host refused it. Check that this repository allows auto-merge, that you have write access, and that there is something left for it to wait on.", | 200 | "The host refused it. Check that this repository allows auto-merge, that you have write access, and that there is something left for it to wait on.", |
| 194 | "disable-auto-merge": | 201 | "disable-auto-merge": |
| 195 | "The host refused it. Check that you have write access, and that the merge has not already happened.", | 202 | "The host refused it. Check that you have write access, and that the merge has not already happened.", |
| 203 | revert: | ||
| 204 | "The host refused it. Check that you have write access and that this pull request was merged on the host.", | ||
| 205 | "approve-workflows": | ||
| 206 | "The host refused it. Check that you have Actions write access and that these workflow runs are still awaiting approval.", | ||
| 196 | }; | 207 | }; |
| 197 | 208 | ||
| 198 | /** | 209 | /** |
| ... | @@ -518,10 +529,18 @@ export function PullRequestDetailPanel({ | ... | @@ -518,10 +529,18 @@ export function PullRequestDetailPanel({ |
| 518 | compensationRef.current = null; | 529 | compensationRef.current = null; |
| 519 | if (scroller) scroller.scrollTop = Math.max(0, scroller.scrollTop + delta); | 530 | if (scroller) scroller.scrollTop = Math.max(0, scroller.scrollTop + delta); |
| 520 | }, [condensed]); | 531 | }, [condensed]); |
| 521 | const [mergeMethod, setMergeMethod] = useState<PullRequestMergeMethod>("merge"); | 532 | const [mergeMethodSelection, setMergeMethodSelection] = useState<{ |
| 533 | readonly pullRequestKey: string; | ||
| 534 | readonly method: PullRequestMergeMethod; | ||
| 535 | }>(() => ({ pullRequestKey, method: "merge" })); | ||
| 536 | const mergeMethod = | ||
| 537 | mergeMethodSelection.pullRequestKey === pullRequestKey ? mergeMethodSelection.method : "merge"; | ||
| 538 | const setMergeMethod = (method: PullRequestMergeMethod) => { | ||
| 539 | setMergeMethodSelection({ pullRequestKey, method }); | ||
| 540 | }; | ||
| 522 | const [confirmation, setConfirmation] = useState<{ | 541 | const [confirmation, setConfirmation] = useState<{ |
| 523 | readonly open: boolean; | 542 | readonly open: boolean; |
| 524 | readonly action: "merge" | "close" | "enable-auto-merge"; | 543 | readonly action: "merge" | "close" | "enable-auto-merge" | "revert" | "approve-workflows"; |
| 525 | }>({ open: false, action: "merge" }); | 544 | }>({ open: false, action: "merge" }); |
| 526 | const confirmAction = confirmation.action; | 545 | const confirmAction = confirmation.action; |
| 527 | // Which handoff is preparing, keyed so a per-finding button can say "Preparing..." on itself | 546 | // Which handoff is preparing, keyed so a per-finding button can say "Preparing..." on itself |
| ... | @@ -558,6 +577,9 @@ export function PullRequestDetailPanel({ | ... | @@ -558,6 +577,9 @@ export function PullRequestDetailPanel({ |
| 558 | }, | 577 | }, |
| 559 | [activity, coreDetail], | 578 | [activity, coreDetail], |
| 560 | ); | 579 | ); |
| 580 | useEffect(() => { | ||
| 581 | if (detail?.autoMergeMethod !== undefined) setMergeMethod(detail.autoMergeMethod); | ||
| 582 | }, [detail?.autoMergeMethod, pullRequestKey]); | ||
| 561 | const repositoryUrl = detail === null ? null : changeRequestRepositoryUrl(detail.url); | 583 | const repositoryUrl = detail === null ? null : changeRequestRepositoryUrl(detail.url); |
| 562 | const checkoutCommand = detail | 584 | const checkoutCommand = detail |
| 563 | ? pullRequestCheckoutCommand( | 585 | ? pullRequestCheckoutCommand( |
| ... | @@ -636,6 +658,7 @@ export function PullRequestDetailPanel({ | ... | @@ -636,6 +658,7 @@ export function PullRequestDetailPanel({ |
| 636 | void refreshFromHost(); | 658 | void refreshFromHost(); |
| 637 | }, [forcedRefreshToken, refreshFromHost]); | 659 | }, [forcedRefreshToken, refreshFromHost]); |
| 638 | const runAction = useAtomCommand(pullRequestEnvironment.runAction, { reportFailure: false }); | 660 | const runAction = useAtomCommand(pullRequestEnvironment.runAction, { reportFailure: false }); |
| 661 | const postComment = useAtomCommand(pullRequestEnvironment.comment, { reportFailure: false }); | ||
| 639 | // Which action is in flight, not merely that one is: every control here is disabled while any | 662 | // Which action is in flight, not merely that one is: every control here is disabled while any |
| 640 | // of them runs, but only the button that was pressed may say what it is doing. | 663 | // of them runs, but only the button that was pressed may say what it is doing. |
| 641 | const [pendingAction, setPendingAction] = useState<PullRequestAction | null>(null); | 664 | const [pendingAction, setPendingAction] = useState<PullRequestAction | null>(null); |
| ... | @@ -689,13 +712,11 @@ export function PullRequestDetailPanel({ | ... | @@ -689,13 +712,11 @@ export function PullRequestDetailPanel({ |
| 689 | cwd: acting?.workspaceRoot ?? detail?.workspaceRoot ?? null, | 712 | cwd: acting?.workspaceRoot ?? detail?.workspaceRoot ?? null, |
| 690 | }); | 713 | }); |
| 691 | 714 | ||
| 692 | const perform = async ( | 715 | const finishAction = async ( |
| 693 | action: PullRequestAction, | 716 | action: PullRequestAction, |
| 694 | method?: PullRequestMergeMethod, | 717 | method?: PullRequestMergeMethod, |
| 695 | updateMethod?: PullRequestUpdateMethod, | 718 | updateMethod?: PullRequestUpdateMethod, |
| 696 | ) => { | 719 | ) => { |
| 697 | if (pendingAction !== null) return; | ||
| 698 | setPendingAction(action); | ||
| 699 | const result = await runAction({ | 720 | const result = await runAction({ |
| 700 | environmentId, | 721 | environmentId, |
| 701 | input: { | 722 | input: { |
| ... | @@ -723,7 +744,7 @@ export function PullRequestDetailPanel({ | ... | @@ -723,7 +744,7 @@ export function PullRequestDetailPanel({ |
| 723 | title: ACTION_FAILURE_LABELS[action], | 744 | title: ACTION_FAILURE_LABELS[action], |
| 724 | description: readableFailure(failure, hint), | 745 | description: readableFailure(failure, hint), |
| 725 | }); | 746 | }); |
| 726 | return; | 747 | return false; |
| 727 | } | 748 | } |
| 728 | toastManager.add({ type: "success", title: ACTION_SUCCESS_LABELS[action] }); | 749 | toastManager.add({ type: "success", title: ACTION_SUCCESS_LABELS[action] }); |
| 729 | // A branch update moves the head commit, which leaves the diff atom pointed at a comparison | 750 | // A branch update moves the head commit, which leaves the diff atom pointed at a comparison |
| ... | @@ -737,6 +758,36 @@ export function PullRequestDetailPanel({ | ... | @@ -737,6 +758,36 @@ export function PullRequestDetailPanel({ |
| 737 | refreshDetail(); | 758 | refreshDetail(); |
| 738 | } | 759 | } |
| 739 | onActed?.(); | 760 | onActed?.(); |
| 761 | return true; | ||
| 762 | }; | ||
| 763 | |||
| 764 | const perform = async ( | ||
| 765 | action: PullRequestAction, | ||
| 766 | method?: PullRequestMergeMethod, | ||
| 767 | updateMethod?: PullRequestUpdateMethod, | ||
| 768 | ) => { | ||
| 769 | if (pendingAction !== null) return false; | ||
| 770 | setPendingAction(action); | ||
| 771 | return finishAction(action, method, updateMethod); | ||
| 772 | }; | ||
| 773 | |||
| 774 | const performCommentAction = async (body: string, action: "close" | "reopen") => { | ||
| 775 | if (pendingAction !== null) return { commentPosted: false }; | ||
| 776 | setPendingAction(action); | ||
| 777 | const commentResult = await postComment({ | ||
| 778 | environmentId, | ||
| 779 | input: { ...reference, body }, | ||
| 780 | }); | ||
| 781 | if (commentResult._tag === "Failure") { | ||
| 782 | setPendingAction(null); | ||
| 783 | toastManager.add({ type: "error", title: "Could not post the comment" }); | ||
| 784 | return { commentPosted: false }; | ||
| 785 | } | ||
| 786 | const actionSucceeded = await finishAction(action); | ||
| 787 | // The comment is durable even if the state change was refused, so make it visible while the | ||
| 788 | // shared action failure explains why the pull request stayed where it was. | ||
| 789 | if (!actionSucceeded) refreshDetail(); | ||
| 790 | return { commentPosted: true }; | ||
| 740 | }; | 791 | }; |
| 741 | 792 | ||
| 742 | const saveTitle = async (next: string) => { | 793 | const saveTitle = async (next: string) => { |
| ... | @@ -1094,10 +1145,17 @@ export function PullRequestDetailPanel({ | ... | @@ -1094,10 +1145,17 @@ export function PullRequestDetailPanel({ |
| 1094 | ? mergeMethod | 1145 | ? mergeMethod |
| 1095 | : (allowedMergeMethods[0] ?? "merge"); | 1146 | : (allowedMergeMethods[0] ?? "merge"); |
| 1096 | const selectedMergeMethodLabel = MERGE_METHOD_LABELS[selectedMergeMethod]; | 1147 | const selectedMergeMethodLabel = MERGE_METHOD_LABELS[selectedMergeMethod]; |
| 1148 | const pendingAutoMergeLabel = `Auto-merge (${selectedMergeMethodLabel.toLowerCase()})`; | ||
| 1097 | const conflicting = detail?.state === "open" && detail.mergeability === "conflicting"; | 1149 | const conflicting = detail?.state === "open" && detail.mergeability === "conflicting"; |
| 1098 | // Only an outright yes arms it. A host that reports nothing has not said the merge is already | 1150 | // Only an outright yes arms it. A host that reports nothing has not said the merge is already |
| 1099 | // spoken for, and an off switch for something that may not be on says the wrong thing twice. | 1151 | // spoken for, and an off switch for something that may not be on says the wrong thing twice. |
| 1100 | const autoMergeArmed = detail?.state === "open" && detail.autoMergeEnabled === true; | 1152 | const autoMergeArmed = detail?.state === "open" && detail.autoMergeEnabled === true; |
| 1153 | const armedMergeMethod = detail?.autoMergeMethod; | ||
| 1154 | const armedAutoMergeLabel = armedMergeMethod | ||
| 1155 | ? `Auto-merge (${MERGE_METHOD_LABELS[armedMergeMethod].toLowerCase()})` | ||
| 1156 | : "Auto-merge"; | ||
| 1157 | const workflowApprovalsRequired = | ||
| 1158 | detail?.state === "open" ? (detail.workflowApprovalsRequired ?? 0) : 0; | ||
| 1101 | // Out of date with the base, and still cleanly mergeable — the one pairing an update button | 1159 | // Out of date with the base, and still cleanly mergeable — the one pairing an update button |
| 1102 | // exists for. Null everywhere else, including hosts that cannot compare at all. | 1160 | // exists for. Null everywhere else, including hosts that cannot compare at all. |
| 1103 | const freshness = detail === null ? null : resolveBaseFreshness(detail); | 1161 | const freshness = detail === null ? null : resolveBaseFreshness(detail); |
| ... | @@ -1118,20 +1176,22 @@ export function PullRequestDetailPanel({ | ... | @@ -1118,20 +1176,22 @@ export function PullRequestDetailPanel({ |
| 1118 | const can = (action: PullRequestAction) => | 1176 | const can = (action: PullRequestAction) => |
| 1119 | detail?.capabilities.actions.includes(action) === true && | 1177 | detail?.capabilities.actions.includes(action) === true && |
| 1120 | detail.viewerPermissions.actions.includes(action); | 1178 | detail.viewerPermissions.actions.includes(action); |
| 1121 | // One live action holds the slot. Conflicts take priority because every other completion action | 1179 | const checksState = detail ? pullRequestChecksState(detail.checks) : null; |
| 1122 | // depends on resolving them first, even for a reader who cannot merge on the host themselves. | 1180 | // The merge state remains in one stable slot from waiting through completion. Conflicts take |
| 1123 | const primaryAction = | 1181 | // the slot while they need a person; the armed badge remains beside them so that state is not lost. |
| 1124 | detail === null || detail.state !== "open" | 1182 | const primaryAction = detail |
| 1125 | ? null | 1183 | ? resolvePullRequestPrimaryControl({ |
| 1126 | : conflicting | 1184 | state: detail.state, |
| 1127 | ? "resolve" | 1185 | isDraft: detail.isDraft, |
| 1128 | : detail.isDraft && can("ready") | 1186 | mergeability: detail.mergeability, |
| 1129 | ? "ready" | 1187 | checksState, |
| 1130 | : !can("merge") | 1188 | autoMergeEnabled: detail.autoMergeEnabled, |
| 1131 | ? null | 1189 | hasMergeMethod: allowedMergeMethods.length > 0, |
| 1132 | : allowedMergeMethods.length > 0 | 1190 | canMerge: can("merge"), |
| 1133 | ? "merge" | 1191 | canMarkReady: can("ready"), |
| 1134 | : null; | 1192 | canEnableAutoMerge: can("enable-auto-merge"), |
| 1193 | }) | ||
| 1194 | : null; | ||
| 1135 | // What the menu's action group holds. Named once so the separators around it are drawn from | 1195 | // What the menu's action group holds. Named once so the separators around it are drawn from |
| 1136 | // the same answer as its contents, rather than on the assumption that it has any. | 1196 | // the same answer as its contents, rather than on the assumption that it has any. |
| 1137 | const showsDraftToggle = | 1197 | const showsDraftToggle = |
| ... | @@ -1142,10 +1202,18 @@ export function PullRequestDetailPanel({ | ... | @@ -1142,10 +1202,18 @@ export function PullRequestDetailPanel({ |
| 1142 | detail?.state === "open" && | 1202 | detail?.state === "open" && |
| 1143 | ((autoMergeArmed && can("disable-auto-merge")) || | 1203 | ((autoMergeArmed && can("disable-auto-merge")) || |
| 1144 | (!autoMergeArmed && | 1204 | (!autoMergeArmed && |
| 1205 | primaryAction !== "enable-auto-merge" && | ||
| 1145 | !detail.isDraft && | 1206 | !detail.isDraft && |
| 1146 | !conflicting && | 1207 | !conflicting && |
| 1147 | can("enable-auto-merge") && | 1208 | can("enable-auto-merge") && |
| 1148 | allowedMergeMethods.length > 0)); | 1209 | allowedMergeMethods.length > 0)); |
| 1210 | const showsMergeNow = | ||
| 1211 | detail?.state === "open" && | ||
| 1212 | (primaryAction === "enable-auto-merge" || primaryAction === "auto-merge-armed") && | ||
| 1213 | can("merge") && | ||
| 1214 | !detail.isDraft && | ||
| 1215 | !conflicting && | ||
| 1216 | allowedMergeMethods.length > 0; | ||
| 1149 | const showsMergeMethods = | 1217 | const showsMergeMethods = |
| 1150 | detail?.state === "open" && | 1218 | detail?.state === "open" && |
| 1151 | can("merge") && | 1219 | can("merge") && |
| ... | @@ -1158,7 +1226,6 @@ export function PullRequestDetailPanel({ | ... | @@ -1158,7 +1226,6 @@ export function PullRequestDetailPanel({ |
| 1158 | ? resolvePullRequestState({ state: detail.state, isDraft: detail.isDraft }) | 1226 | ? resolvePullRequestState({ state: detail.state, isDraft: detail.isDraft }) |
| 1159 | : null; | 1227 | : null; |
| 1160 | const checksSummary = detail ? summarizePullRequestChecks(detail.checks) : null; | 1228 | const checksSummary = detail ? summarizePullRequestChecks(detail.checks) : null; |
| 1161 | const checksState = detail ? pullRequestChecksState(detail.checks) : null; | ||
| 1162 | // Approvals that still stand, and only those. A superseded one is dimmed beside the reviewer | 1229 | // Approvals that still stand, and only those. A superseded one is dimmed beside the reviewer |
| 1163 | // who gave it, so counting it here would have the header assert in a number what the row next | 1230 | // who gave it, so counting it here would have the header assert in a number what the row next |
| 1164 | // to it has just qualified. | 1231 | // to it has just qualified. |
| ... | @@ -1335,18 +1402,28 @@ export function PullRequestDetailPanel({ | ... | @@ -1335,18 +1402,28 @@ export function PullRequestDetailPanel({ |
| 1335 | </MenuPopup> | 1402 | </MenuPopup> |
| 1336 | </Menu> | 1403 | </Menu> |
| 1337 | ) : null} | 1404 | ) : null} |
| 1405 | {workflowApprovalsRequired > 0 && can("approve-workflows") ? ( | ||
| 1406 | <Button | ||
| 1407 | size="xs" | ||
| 1408 | variant="warning-outline" | ||
| 1409 | disabled={actionPending} | ||
| 1410 | onClick={() => setConfirmation({ open: true, action: "approve-workflows" })} | ||
| 1411 | > | ||
| 1412 | <PlayIcon className="size-3.5" /> | ||
| 1413 | {pendingAction === "approve-workflows" | ||
| 1414 | ? "Approving..." | ||
| 1415 | : "Approve workflows to run"} | ||
| 1416 | </Button> | ||
| 1417 | ) : null} | ||
| 1338 | {/* Said where the Merge button is, because it is the answer to why nobody has | 1418 | {/* Said where the Merge button is, because it is the answer to why nobody has |
| 1339 | pressed it: the merge is already asked for, and the host is holding it. */} | 1419 | pressed it: the merge is already asked for, and the host is holding it. */} |
| 1340 | {autoMergeArmed ? ( | 1420 | {autoMergeArmed && primaryAction !== "auto-merge-armed" ? ( |
| 1341 | <Tooltip> | 1421 | <Tooltip> |
| 1342 | <TooltipTrigger | 1422 | <TooltipTrigger |
| 1343 | render={ | 1423 | render={ |
| 1344 | <Badge | 1424 | <Badge size="control" variant="info"> |
| 1345 | variant="info" | 1425 | <GitMergeIcon aria-hidden className="size-3.5" /> |
| 1346 | className="h-5 shrink-0 gap-1 rounded px-1.5 text-[10px]" | 1426 | {armedAutoMergeLabel} |
| 1347 | > | ||
| 1348 | <GitMergeIcon aria-hidden className="size-3" /> | ||
| 1349 | Auto-merge | ||
| 1350 | </Badge> | 1427 | </Badge> |
| 1351 | } | 1428 | } |
| 1352 | /> | 1429 | /> |
| ... | @@ -1369,6 +1446,29 @@ export function PullRequestDetailPanel({ | ... | @@ -1369,6 +1446,29 @@ export function PullRequestDetailPanel({ |
| 1369 | <Button size="xs" disabled={actionPending} onClick={() => void perform("ready")}> | 1446 | <Button size="xs" disabled={actionPending} onClick={() => void perform("ready")}> |
| 1370 | Ready for review | 1447 | Ready for review |
| 1371 | </Button> | 1448 | </Button> |
| 1449 | ) : primaryAction === "enable-auto-merge" ? ( | ||
| 1450 | <Button | ||
| 1451 | size="xs" | ||
| 1452 | disabled={actionPending} | ||
| 1453 | onClick={() => setConfirmation({ open: true, action: "enable-auto-merge" })} | ||
| 1454 | > | ||
| 1455 | <GitMergeIcon className="size-3.5" /> | ||
| 1456 | {pendingAction === "enable-auto-merge" ? "Enabling..." : pendingAutoMergeLabel} | ||
| 1457 | </Button> | ||
| 1458 | ) : primaryAction === "auto-merge-armed" ? ( | ||
| 1459 | <Tooltip> | ||
| 1460 | <TooltipTrigger | ||
| 1461 | render={ | ||
| 1462 | <Badge size="control" variant="info"> | ||
| 1463 | <GitMergeIcon className="size-3.5" /> | ||
| 1464 | {armedAutoMergeLabel} | ||
| 1465 | </Badge> | ||
| 1466 | } | ||
| 1467 | /> | ||
| 1468 | <TooltipPopup side="top"> | ||
| 1469 | The host will merge this on its own once its requirements are met | ||
| 1470 | </TooltipPopup> | ||
| 1471 | </Tooltip> | ||
| 1372 | ) : primaryAction === "merge" ? ( | 1472 | ) : primaryAction === "merge" ? ( |
| 1373 | <Button | 1473 | <Button |
| 1374 | size="xs" | 1474 | size="xs" |
| ... | @@ -1377,6 +1477,12 @@ export function PullRequestDetailPanel({ | ... | @@ -1377,6 +1477,12 @@ export function PullRequestDetailPanel({ |
| 1377 | > | 1477 | > |
| 1378 | {pendingAction === "merge" ? "Merging..." : selectedMergeMethodLabel} | 1478 | {pendingAction === "merge" ? "Merging..." : selectedMergeMethodLabel} |
| 1379 | </Button> | 1479 | </Button> |
| 1480 | ) : (primaryAction === "merged" || primaryAction === "closed") && | ||
| 1481 | statePresentation !== null ? ( | ||
| 1482 | <Badge size="control" variant="outline" className={statePresentation.toneClassName}> | ||
| 1483 | <statePresentation.Icon className="size-3.5" /> | ||
| 1484 | {statePresentation.label} | ||
| 1485 | </Badge> | ||
| 1380 | ) : null} | 1486 | ) : null} |
| 1381 | <Menu> | 1487 | <Menu> |
| 1382 | <MenuTrigger | 1488 | <MenuTrigger |
| ... | @@ -1447,11 +1553,18 @@ export function PullRequestDetailPanel({ | ... | @@ -1447,11 +1553,18 @@ export function PullRequestDetailPanel({ |
| 1447 | {detail.isDraft ? "Ready for review" : "Convert to draft"} | 1553 | {detail.isDraft ? "Ready for review" : "Convert to draft"} |
| 1448 | </MenuItem> | 1554 | </MenuItem> |
| 1449 | ) : null} | 1555 | ) : null} |
| 1450 | {/* The same merge, left with the host to carry out once the things it | 1556 | {showsMergeNow ? ( |
| 1451 | waits on are done. It is offered beside the merge rather than instead | 1557 | <MenuItem |
| 1452 | of it, because the reader who can wait and the reader who cannot are | 1558 | disabled={actionPending} |
| 1453 | the same person on different days — and a conflicting branch is neither, | 1559 | onClick={() => setConfirmation({ open: true, action: "merge" })} |
| 1454 | since nothing the host waits for will clear it. */} | 1560 | > |
| 1561 | <GitMergeIcon className="size-3.5" /> | ||
| 1562 | Merge now | ||
| 1563 | </MenuItem> | ||
| 1564 | ) : null} | ||
| 1565 | {/* The same merge, left with the host to carry out once its requirements | ||
| 1566 | pass. A conflicting branch cannot be armed because nothing the host | ||
| 1567 | waits for will clear the conflict. */} | ||
| 1455 | {autoMergeArmed && can("disable-auto-merge") ? ( | 1568 | {autoMergeArmed && can("disable-auto-merge") ? ( |
| 1456 | <MenuItem | 1569 | <MenuItem |
| 1457 | disabled={actionPending} | 1570 | disabled={actionPending} |
| ... | @@ -1460,11 +1573,7 @@ export function PullRequestDetailPanel({ | ... | @@ -1460,11 +1573,7 @@ export function PullRequestDetailPanel({ |
| 1460 | <GitMergeIcon className="size-3.5" /> | 1573 | <GitMergeIcon className="size-3.5" /> |
| 1461 | Disable auto-merge | 1574 | Disable auto-merge |
| 1462 | </MenuItem> | 1575 | </MenuItem> |
| 1463 | ) : !autoMergeArmed && | 1576 | ) : showsAutoMerge ? ( |
| 1464 | !detail.isDraft && | ||
| 1465 | !conflicting && | ||
| 1466 | can("enable-auto-merge") && | ||
| 1467 | allowedMergeMethods.length > 0 ? ( | ||
| 1468 | <MenuItem | 1577 | <MenuItem |
| 1469 | disabled={actionPending} | 1578 | disabled={actionPending} |
| 1470 | onClick={() => | 1579 | onClick={() => |
| ... | @@ -1485,7 +1594,9 @@ export function PullRequestDetailPanel({ | ... | @@ -1485,7 +1594,9 @@ export function PullRequestDetailPanel({ |
| 1485 | {/* Only below the draft control. A host with no draft of its own, or | 1594 | {/* Only below the draft control. A host with no draft of its own, or |
| 1486 | a draft whose control is already the header button, would leave | 1595 | a draft whose control is already the header button, would leave |
| 1487 | this against the separator that opened the group. */} | 1596 | this against the separator that opened the group. */} |
| 1488 | {showsDraftToggle ? <MenuSeparator /> : null} | 1597 | {showsDraftToggle || showsMergeNow || showsAutoMerge ? ( |
| 1598 | <MenuSeparator /> | ||
| 1599 | ) : null} | ||
| 1489 | <MenuRadioGroup | 1600 | <MenuRadioGroup |
| 1490 | value={selectedMergeMethod} | 1601 | value={selectedMergeMethod} |
| 1491 | onValueChange={(method) => | 1602 | onValueChange={(method) => |
| ... | @@ -1507,7 +1618,7 @@ export function PullRequestDetailPanel({ | ... | @@ -1507,7 +1618,7 @@ export function PullRequestDetailPanel({ |
| 1507 | ) : null} | 1618 | ) : null} |
| 1508 | {pullRequestActionMenuHasGroup( | 1619 | {pullRequestActionMenuHasGroup( |
| 1509 | showsDraftToggle, | 1620 | showsDraftToggle, |
| 1510 | showsAutoMerge, | 1621 | showsAutoMerge || showsMergeNow, |
| 1511 | showsMergeMethods, | 1622 | showsMergeMethods, |
| 1512 | ) ? ( | 1623 | ) ? ( |
| 1513 | <MenuSeparator /> | 1624 | <MenuSeparator /> |
| ... | @@ -1542,6 +1653,17 @@ export function PullRequestDetailPanel({ | ... | @@ -1542,6 +1653,17 @@ export function PullRequestDetailPanel({ |
| 1542 | Reopen pull request | 1653 | Reopen pull request |
| 1543 | </MenuItem> | 1654 | </MenuItem> |
| 1544 | </> | 1655 | </> |
| 1656 | ) : detail.state === "merged" && can("revert") ? ( | ||
| 1657 | <> | ||
| 1658 | <MenuSeparator /> | ||
| 1659 | <MenuItem | ||
| 1660 | disabled={actionPending} | ||
| 1661 | onClick={() => setConfirmation({ open: true, action: "revert" })} | ||
| 1662 | > | ||
| 1663 | <RotateCcwIcon className="size-3.5" /> | ||
| 1664 | Revert changes | ||
| 1665 | </MenuItem> | ||
| 1666 | </> | ||
| 1545 | ) : null} | 1667 | ) : null} |
| 1546 | </MenuPopup> | 1668 | </MenuPopup> |
| 1547 | </Menu> | 1669 | </Menu> |
| ... | @@ -1987,6 +2109,8 @@ export function PullRequestDetailPanel({ | ... | @@ -1987,6 +2109,8 @@ export function PullRequestDetailPanel({ |
| 1987 | fixFindingLabel={handoffLabels.fixFinding} | 2109 | fixFindingLabel={handoffLabels.fixFinding} |
| 1988 | fixCheckLabel={handoffLabels.fixCheck} | 2110 | fixCheckLabel={handoffLabels.fixCheck} |
| 1989 | onFixFinding={startFixFinding} | 2111 | onFixFinding={startFixFinding} |
| 2112 | actionPending={actionPending} | ||
| 2113 | onCommentAction={performCommentAction} | ||
| 1990 | onRefresh={refreshDetail} | 2114 | onRefresh={refreshDetail} |
| 1991 | /> | 2115 | /> |
| 1992 | </div> | 2116 | </div> |
| ... | @@ -2049,7 +2173,11 @@ export function PullRequestDetailPanel({ | ... | @@ -2049,7 +2173,11 @@ export function PullRequestDetailPanel({ |
| 2049 | ? "Merge pull request?" | 2173 | ? "Merge pull request?" |
| 2050 | : confirmAction === "enable-auto-merge" | 2174 | : confirmAction === "enable-auto-merge" |
| 2051 | ? "Enable auto-merge?" | 2175 | ? "Enable auto-merge?" |
| 2052 | : "Close pull request?"} | 2176 | : confirmAction === "revert" |
| 2177 | ? "Revert these changes?" | ||
| 2178 | : confirmAction === "approve-workflows" | ||
| 2179 | ? "Approve workflows to run?" | ||
| 2180 | : "Close pull request?"} | ||
| 2053 | </AlertDialogTitle> | 2181 | </AlertDialogTitle> |
| 2054 | <AlertDialogDescription> | 2182 | <AlertDialogDescription> |
| 2055 | {confirmAction === "merge" | 2183 | {confirmAction === "merge" |
| ... | @@ -2059,7 +2187,11 @@ export function PullRequestDetailPanel({ | ... | @@ -2059,7 +2187,11 @@ export function PullRequestDetailPanel({ |
| 2059 | // may be immediately — there is no telling from here whether anything is | 2187 | // may be immediately — there is no telling from here whether anything is |
| 2060 | // still outstanding. | 2188 | // still outstanding. |
| 2061 | `This merges #${reference.number} using ${selectedMergeMethod} as soon as the host considers it ready, which may be immediately.` | 2189 | `This merges #${reference.number} using ${selectedMergeMethod} as soon as the host considers it ready, which may be immediately.` |
| 2062 | : `This closes #${reference.number} without merging it.`} | 2190 | : confirmAction === "revert" |
| 2191 | ? `This opens a new pull request that reverses the changes merged by #${reference.number}.` | ||
| 2192 | : confirmAction === "approve-workflows" | ||
| 2193 | ? `This allows ${workflowApprovalsRequired} ${workflowApprovalsRequired === 1 ? "workflow" : "workflows"} from #${reference.number} to run. Review the code and workflow changes first.` | ||
| 2194 | : `This closes #${reference.number} without merging it.`} | ||
| 2063 | </AlertDialogDescription> | 2195 | </AlertDialogDescription> |
| 2064 | </AlertDialogHeader> | 2196 | </AlertDialogHeader> |
| 2065 | <AlertDialogFooter> | 2197 | <AlertDialogFooter> |
| ... | @@ -2076,6 +2208,8 @@ export function PullRequestDetailPanel({ | ... | @@ -2076,6 +2208,8 @@ export function PullRequestDetailPanel({ |
| 2076 | if (action === "merge") void perform("merge", selectedMergeMethod); | 2208 | if (action === "merge") void perform("merge", selectedMergeMethod); |
| 2077 | if (action === "enable-auto-merge") | 2209 | if (action === "enable-auto-merge") |
| 2078 | void perform("enable-auto-merge", selectedMergeMethod); | 2210 | void perform("enable-auto-merge", selectedMergeMethod); |
| 2211 | if (action === "revert") void perform("revert"); | ||
| 2212 | if (action === "approve-workflows") void perform("approve-workflows"); | ||
| 2079 | if (action === "close") void perform("close"); | 2213 | if (action === "close") void perform("close"); |
| 2080 | }} | 2214 | }} |
| 2081 | > | 2215 | > |
| ... | @@ -2083,7 +2217,11 @@ export function PullRequestDetailPanel({ | ... | @@ -2083,7 +2217,11 @@ export function PullRequestDetailPanel({ |
| 2083 | ? selectedMergeMethodLabel | 2217 | ? selectedMergeMethodLabel |
| 2084 | : confirmAction === "enable-auto-merge" | 2218 | : confirmAction === "enable-auto-merge" |
| 2085 | ? "Enable auto-merge" | 2219 | ? "Enable auto-merge" |
| 2086 | : "Close"} | 2220 | : confirmAction === "revert" |
| 2221 | ? "Create revert PR" | ||
| 2222 | : confirmAction === "approve-workflows" | ||
| 2223 | ? "Approve and run" | ||
| 2224 | : "Close"} | ||
| 2087 | </Button> | 2225 | </Button> |
| 2088 | </AlertDialogFooter> | 2226 | </AlertDialogFooter> |
| 2089 | </AlertDialogPopup> | 2227 | </AlertDialogPopup> |
apps/web/src/components/pullRequest/PullRequestListFilters.tsx+1-1| ... | @@ -524,7 +524,7 @@ export function PullRequestFiltersMenu({ | ... | @@ -524,7 +524,7 @@ export function PullRequestFiltersMenu({ |
| 524 | <ListFilterIcon className="size-4" /> | 524 | <ListFilterIcon className="size-4" /> |
| 525 | <span>Filters</span> | 525 | <span>Filters</span> |
| 526 | {filterCount > 0 ? ( | 526 | {filterCount > 0 ? ( |
| 527 | <span className="rounded-full bg-primary/10 px-1.5 text-xs text-primary tabular-nums"> | 527 | <span className="rounded-full bg-muted px-1.5 text-xs text-muted-foreground tabular-nums"> |
| 528 | {filterCount} | 528 | {filterCount} |
| 529 | </span> | 529 | </span> |
| 530 | ) : null} | 530 | ) : null} |
apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx+67-11| ... | @@ -9,9 +9,11 @@ import { | ... | @@ -9,9 +9,11 @@ import { |
| 9 | ArrowDownUpIcon, | 9 | ArrowDownUpIcon, |
| 10 | ChevronDownIcon, | 10 | ChevronDownIcon, |
| 11 | ChevronRightIcon, | 11 | ChevronRightIcon, |
| 12 | GitPullRequestClosedIcon, | ||
| 12 | HammerIcon, | 13 | HammerIcon, |
| 13 | MessageSquareIcon, | 14 | MessageSquareIcon, |
| 14 | PencilIcon, | 15 | PencilIcon, |
| 16 | RotateCcwIcon, | ||
| 15 | SendIcon, | 17 | SendIcon, |
| 16 | TagIcon, | 18 | TagIcon, |
| 17 | UsersIcon, | 19 | UsersIcon, |
| ... | @@ -308,20 +310,43 @@ function Section({ | ... | @@ -308,20 +310,43 @@ function Section({ |
| 308 | function CommentComposer({ | 310 | function CommentComposer({ |
| 309 | environmentId, | 311 | environmentId, |
| 310 | detail, | 312 | detail, |
| 313 | actionPending, | ||
| 314 | onCommentAction, | ||
| 311 | onCommented, | 315 | onCommented, |
| 312 | }: { | 316 | }: { |
| 313 | environmentId: EnvironmentId; | 317 | environmentId: EnvironmentId; |
| 314 | detail: PullRequestDetailView; | 318 | detail: PullRequestDetailView; |
| 319 | actionPending: boolean; | ||
| 320 | onCommentAction: ( | ||
| 321 | body: string, | ||
| 322 | action: "close" | "reopen", | ||
| 323 | ) => Promise<{ readonly commentPosted: boolean }>; | ||
| 315 | onCommented: () => void; | 324 | onCommented: () => void; |
| 316 | }) { | 325 | }) { |
| 317 | const [body, setBody] = useState(""); | 326 | const [body, setBody] = useState(""); |
| 318 | const [posting, setPosting] = useState(false); | 327 | const [submitting, setSubmitting] = useState<"comment" | "close" | "reopen" | null>(null); |
| 319 | const postComment = useAtomCommand(pullRequestEnvironment.comment, { reportFailure: false }); | 328 | const postComment = useAtomCommand(pullRequestEnvironment.comment, { reportFailure: false }); |
| 329 | const followUpAction = | ||
| 330 | detail.state === "open" && | ||
| 331 | detail.capabilities.actions.includes("close") && | ||
| 332 | detail.viewerPermissions.actions.includes("close") | ||
| 333 | ? ("close" as const) | ||
| 334 | : detail.state === "closed" && | ||
| 335 | detail.capabilities.actions.includes("reopen") && | ||
| 336 | detail.viewerPermissions.actions.includes("reopen") | ||
| 337 | ? ("reopen" as const) | ||
| 338 | : null; | ||
| 320 | 339 | ||
| 321 | const submit = async () => { | 340 | const submit = async (action: "comment" | "close" | "reopen") => { |
| 322 | const trimmed = body.trim(); | 341 | const trimmed = body.trim(); |
| 323 | if (trimmed.length === 0 || posting) return; | 342 | if (trimmed.length === 0 || submitting !== null || actionPending) return; |
| 324 | setPosting(true); | 343 | setSubmitting(action); |
| 344 | if (action !== "comment") { | ||
| 345 | const result = await onCommentAction(trimmed, action); | ||
| 346 | if (result.commentPosted) setBody(""); | ||
| 347 | setSubmitting(null); | ||
| 348 | return; | ||
| 349 | } | ||
| 325 | const result = await postComment({ | 350 | const result = await postComment({ |
| 326 | environmentId, | 351 | environmentId, |
| 327 | input: { | 352 | input: { |
| ... | @@ -331,12 +356,13 @@ function CommentComposer({ | ... | @@ -331,12 +356,13 @@ function CommentComposer({ |
| 331 | body: trimmed, | 356 | body: trimmed, |
| 332 | }, | 357 | }, |
| 333 | }); | 358 | }); |
| 334 | setPosting(false); | ||
| 335 | if (result._tag === "Failure") { | 359 | if (result._tag === "Failure") { |
| 360 | setSubmitting(null); | ||
| 336 | toastManager.add({ type: "error", title: "Could not post the comment" }); | 361 | toastManager.add({ type: "error", title: "Could not post the comment" }); |
| 337 | return; | 362 | return; |
| 338 | } | 363 | } |
| 339 | setBody(""); | 364 | setBody(""); |
| 365 | setSubmitting(null); | ||
| 340 | onCommented(); | 366 | onCommented(); |
| 341 | }; | 367 | }; |
| 342 | 368 | ||
| ... | @@ -345,22 +371,43 @@ function CommentComposer({ | ... | @@ -345,22 +371,43 @@ function CommentComposer({ |
| 345 | <Textarea | 371 | <Textarea |
| 346 | // Locked while posting: the body is cleared on success, which would otherwise throw | 372 | // Locked while posting: the body is cleared on success, which would otherwise throw |
| 347 | // away a new draft typed while the request was still in flight. | 373 | // away a new draft typed while the request was still in flight. |
| 348 | disabled={posting} | 374 | disabled={submitting !== null || actionPending} |
| 349 | value={body} | 375 | value={body} |
| 350 | rows={3} | 376 | rows={3} |
| 351 | placeholder="Leave a comment" | 377 | placeholder="Leave a comment" |
| 352 | aria-label="Comment on this pull request" | 378 | aria-label="Comment on this pull request" |
| 353 | onChange={(event) => setBody(event.target.value)} | 379 | onChange={(event) => setBody(event.target.value)} |
| 354 | /> | 380 | /> |
| 355 | <div className="flex justify-end"> | 381 | <div className="flex justify-end gap-2"> |
| 382 | {followUpAction === null ? null : ( | ||
| 383 | <Button | ||
| 384 | size="xs" | ||
| 385 | variant={followUpAction === "close" ? "destructive-outline" : "outline"} | ||
| 386 | disabled={body.trim().length === 0 || submitting !== null || actionPending} | ||
| 387 | onClick={() => void submit(followUpAction)} | ||
| 388 | > | ||
| 389 | {followUpAction === "close" ? ( | ||
| 390 | <GitPullRequestClosedIcon className="size-3.5" /> | ||
| 391 | ) : ( | ||
| 392 | <RotateCcwIcon className="size-3.5" /> | ||
| 393 | )} | ||
| 394 | {submitting === followUpAction | ||
| 395 | ? followUpAction === "close" | ||
| 396 | ? "Closing..." | ||
| 397 | : "Reopening..." | ||
| 398 | : followUpAction === "close" | ||
| 399 | ? "Close with comment" | ||
| 400 | : "Reopen with comment"} | ||
| 401 | </Button> | ||
| 402 | )} | ||
| 356 | <Button | 403 | <Button |
| 357 | size="xs" | 404 | size="xs" |
| 358 | variant="outline" | 405 | variant="outline" |
| 359 | disabled={body.trim().length === 0 || posting} | 406 | disabled={body.trim().length === 0 || submitting !== null || actionPending} |
| 360 | onClick={() => void submit()} | 407 | onClick={() => void submit("comment")} |
| 361 | > | 408 | > |
| 362 | <SendIcon className="size-3.5" /> | 409 | <SendIcon className="size-3.5" /> |
| 363 | {posting ? "Posting..." : "Comment"} | 410 | {submitting === "comment" ? "Posting..." : "Comment"} |
| 364 | </Button> | 411 | </Button> |
| 365 | </div> | 412 | </div> |
| 366 | </div> | 413 | </div> |
| ... | @@ -383,6 +430,8 @@ export function PullRequestSummaryTab({ | ... | @@ -383,6 +430,8 @@ export function PullRequestSummaryTab({ |
| 383 | fixFindingLabel = "Fix in a thread", | 430 | fixFindingLabel = "Fix in a thread", |
| 384 | fixCheckLabel = "Fix", | 431 | fixCheckLabel = "Fix", |
| 385 | onFixFinding, | 432 | onFixFinding, |
| 433 | actionPending, | ||
| 434 | onCommentAction, | ||
| 386 | onRefresh, | 435 | onRefresh, |
| 387 | }: { | 436 | }: { |
| 388 | environmentId: EnvironmentId; | 437 | environmentId: EnvironmentId; |
| ... | @@ -395,6 +444,11 @@ export function PullRequestSummaryTab({ | ... | @@ -395,6 +444,11 @@ export function PullRequestSummaryTab({ |
| 395 | fixFindingLabel?: string; | 444 | fixFindingLabel?: string; |
| 396 | fixCheckLabel?: string; | 445 | fixCheckLabel?: string; |
| 397 | onFixFinding?: (finding: PullRequestFinding) => void; | 446 | onFixFinding?: (finding: PullRequestFinding) => void; |
| 447 | actionPending: boolean; | ||
| 448 | onCommentAction: ( | ||
| 449 | body: string, | ||
| 450 | action: "close" | "reopen", | ||
| 451 | ) => Promise<{ readonly commentPosted: boolean }>; | ||
| 398 | onRefresh: () => void; | 452 | onRefresh: () => void; |
| 399 | }) { | 453 | }) { |
| 400 | // Keyed by the pull request, so opening another one starts at the end of its conversation | 454 | // Keyed by the pull request, so opening another one starts at the end of its conversation |
| ... | @@ -718,7 +772,7 @@ export function PullRequestSummaryTab({ | ... | @@ -718,7 +772,7 @@ export function PullRequestSummaryTab({ |
| 718 | <PullRequestCheckStatusIcon status={check.status} /> | 772 | <PullRequestCheckStatusIcon status={check.status} /> |
| 719 | <span className="min-w-0 flex-1 truncate">{check.name}</span> | 773 | <span className="min-w-0 flex-1 truncate">{check.name}</span> |
| 720 | <span className="shrink-0 text-muted-foreground"> | 774 | <span className="shrink-0 text-muted-foreground"> |
| 721 | {pullRequestCheckStatusLabel(check.status)} | 775 | {pullRequestCheckStatusLabel(check)} |
| 722 | </span> | 776 | </span> |
| 723 | </button> | 777 | </button> |
| 724 | {/* Only where there is something to fix. A passing check has no failure to | 778 | {/* Only where there is something to fix. A passing check has no failure to |
| ... | @@ -919,6 +973,8 @@ export function PullRequestSummaryTab({ | ... | @@ -919,6 +973,8 @@ export function PullRequestSummaryTab({ |
| 919 | key={`${environmentId}:${detail.projectId}/${detail.repository}#${detail.number}`} | 973 | key={`${environmentId}:${detail.projectId}/${detail.repository}#${detail.number}`} |
| 920 | environmentId={environmentId} | 974 | environmentId={environmentId} |
| 921 | detail={detail} | 975 | detail={detail} |
| 976 | actionPending={actionPending} | ||
| 977 | onCommentAction={onCommentAction} | ||
| 922 | onCommented={onRefresh} | 978 | onCommented={onRefresh} |
| 923 | /> | 979 | /> |
| 924 | ) : null} | 980 | ) : null} |
apps/web/src/components/pullRequest/pullRequestChecks.test.tsx+30-3| ... | @@ -5,10 +5,17 @@ import { describe, expect, it } from "vite-plus/test"; | ... | @@ -5,10 +5,17 @@ import { describe, expect, it } from "vite-plus/test"; |
| 5 | import { PullRequestChecksPopover } from "./PullRequestChecksPopover"; | 5 | import { PullRequestChecksPopover } from "./PullRequestChecksPopover"; |
| 6 | import type { EnvironmentPullRequestEntry } from "./pullRequestList.logic"; | 6 | import type { EnvironmentPullRequestEntry } from "./pullRequestList.logic"; |
| 7 | import { PullRequestRow } from "./PullRequestRow"; | 7 | import { PullRequestRow } from "./PullRequestRow"; |
| 8 | import { pullRequestChecksState } from "./pullRequestPresentation"; | 8 | import { |
| 9 | pullRequestChecksState, | ||
| 10 | pullRequestCheckStatusLabel, | ||
| 11 | summarizePullRequestChecks, | ||
| 12 | } from "./pullRequestPresentation"; | ||
| 9 | 13 | ||
| 10 | function check(status: PullRequestCheck["status"]): PullRequestCheck { | 14 | function check( |
| 11 | return { name: `check-${status}`, status, description: null, url: null }; | 15 | status: PullRequestCheck["status"], |
| 16 | overrides: Partial<PullRequestCheck> = {}, | ||
| 17 | ): PullRequestCheck { | ||
| 18 | return { name: `check-${status}`, status, description: null, url: null, ...overrides }; | ||
| 12 | } | 19 | } |
| 13 | 20 | ||
| 14 | describe("pullRequestChecksState", () => { | 21 | describe("pullRequestChecksState", () => { |
| ... | @@ -18,11 +25,31 @@ describe("pullRequestChecksState", () => { | ... | @@ -18,11 +25,31 @@ describe("pullRequestChecksState", () => { |
| 18 | ); | 25 | ); |
| 19 | expect(pullRequestChecksState([check("success"), check("cancelled")])).toBe("failing"); | 26 | expect(pullRequestChecksState([check("success"), check("cancelled")])).toBe("failing"); |
| 20 | expect(pullRequestChecksState([check("success"), check("pending")])).toBe("pending"); | 27 | expect(pullRequestChecksState([check("success"), check("pending")])).toBe("pending"); |
| 28 | expect(pullRequestChecksState([check("success"), check("action-required")])).toBe("pending"); | ||
| 21 | expect(pullRequestChecksState([check("success")])).toBe("passing"); | 29 | expect(pullRequestChecksState([check("success")])).toBe("passing"); |
| 22 | // Skipped and neutral are neither a pass nor a failure, so they are no verdict at all. | 30 | // Skipped and neutral are neither a pass nor a failure, so they are no verdict at all. |
| 23 | expect(pullRequestChecksState([check("skipped"), check("neutral")])).toBe(null); | 31 | expect(pullRequestChecksState([check("skipped"), check("neutral")])).toBe(null); |
| 24 | expect(pullRequestChecksState([])).toBe(null); | 32 | expect(pullRequestChecksState([])).toBe(null); |
| 25 | }); | 33 | }); |
| 34 | |||
| 35 | it("names workflow approval instead of claiming every check passed", () => { | ||
| 36 | const workflow = check("action-required", { | ||
| 37 | url: "https://github.com/acme/web/actions/runs/42/job/7", | ||
| 38 | }); | ||
| 39 | const manualGate = check("action-required", { url: "https://example.com/manual-gate" }); | ||
| 40 | expect(pullRequestCheckStatusLabel(workflow)).toBe("Awaiting approval"); | ||
| 41 | expect(pullRequestCheckStatusLabel(manualGate)).toBe("Awaiting action"); | ||
| 42 | expect(summarizePullRequestChecks([check("success"), workflow])).toBe( | ||
| 43 | "1 workflow awaiting approval", | ||
| 44 | ); | ||
| 45 | expect(summarizePullRequestChecks([check("failure"), workflow])).toBe("1 of 2 failing"); | ||
| 46 | expect(summarizePullRequestChecks([check("success"), manualGate])).toBe( | ||
| 47 | "1 check awaiting action", | ||
| 48 | ); | ||
| 49 | expect(summarizePullRequestChecks([workflow, manualGate])).toBe( | ||
| 50 | "1 workflow and 1 check awaiting action", | ||
| 51 | ); | ||
| 52 | }); | ||
| 26 | }); | 53 | }); |
| 27 | 54 | ||
| 28 | /** Every element of the tree the row returned, so a nested indicator can be looked for. */ | 55 | /** Every element of the tree the row returned, so a nested indicator can be looked for. */ |
apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts+53-1| ... | @@ -31,6 +31,7 @@ import { | ... | @@ -31,6 +31,7 @@ import { |
| 31 | pullRequestHandoffLabels, | 31 | pullRequestHandoffLabels, |
| 32 | pullRequestReviewOutcome, | 32 | pullRequestReviewOutcome, |
| 33 | readableFailure, | 33 | readableFailure, |
| 34 | resolvePullRequestPrimaryControl, | ||
| 34 | shouldRefreshPullRequestActivity, | 35 | shouldRefreshPullRequestActivity, |
| 35 | resolveBaseFreshness, | 36 | resolveBaseFreshness, |
| 36 | buildPullRequestTimeline, | 37 | buildPullRequestTimeline, |
| ... | @@ -146,6 +147,54 @@ describe("pull request action menu", () => { | ... | @@ -146,6 +147,54 @@ describe("pull request action menu", () => { |
| 146 | }); | 147 | }); |
| 147 | }); | 148 | }); |
| 148 | 149 | ||
| 150 | describe("pull request primary control", () => { | ||
| 151 | const open = { | ||
| 152 | state: "open" as const, | ||
| 153 | isDraft: false, | ||
| 154 | mergeability: "mergeable" as const, | ||
| 155 | checksState: "passing" as const, | ||
| 156 | autoMergeEnabled: false, | ||
| 157 | hasMergeMethod: true, | ||
| 158 | canMerge: true, | ||
| 159 | canMarkReady: true, | ||
| 160 | canEnableAutoMerge: true, | ||
| 161 | }; | ||
| 162 | |||
| 163 | it("moves pending and failing checks to auto-merge", () => { | ||
| 164 | expect(resolvePullRequestPrimaryControl({ ...open, checksState: "pending" })).toBe( | ||
| 165 | "enable-auto-merge", | ||
| 166 | ); | ||
| 167 | expect(resolvePullRequestPrimaryControl({ ...open, checksState: "failing" })).toBe( | ||
| 168 | "enable-auto-merge", | ||
| 169 | ); | ||
| 170 | }); | ||
| 171 | |||
| 172 | it("does not offer auto-merge while the host state is unknown", () => { | ||
| 173 | expect( | ||
| 174 | resolvePullRequestPrimaryControl({ | ||
| 175 | ...open, | ||
| 176 | checksState: "pending", | ||
| 177 | autoMergeEnabled: undefined, | ||
| 178 | }), | ||
| 179 | ).toBe("merge"); | ||
| 180 | }); | ||
| 181 | |||
| 182 | it("keeps armed and terminal states in the merge button slot", () => { | ||
| 183 | expect(resolvePullRequestPrimaryControl({ ...open, autoMergeEnabled: true })).toBe( | ||
| 184 | "auto-merge-armed", | ||
| 185 | ); | ||
| 186 | expect(resolvePullRequestPrimaryControl({ ...open, state: "merged" })).toBe("merged"); | ||
| 187 | expect(resolvePullRequestPrimaryControl({ ...open, state: "closed" })).toBe("closed"); | ||
| 188 | }); | ||
| 189 | |||
| 190 | it("keeps conflicts and drafts actionable before merge", () => { | ||
| 191 | expect(resolvePullRequestPrimaryControl({ ...open, mergeability: "conflicting" })).toBe( | ||
| 192 | "resolve", | ||
| 193 | ); | ||
| 194 | expect(resolvePullRequestPrimaryControl({ ...open, isDraft: true })).toBe("ready"); | ||
| 195 | }); | ||
| 196 | }); | ||
| 197 | |||
| 149 | describe("pull request state description", () => { | 198 | describe("pull request state description", () => { |
| 150 | it("keeps draft and conflicts orthogonal to the terminal states", () => { | 199 | it("keeps draft and conflicts orthogonal to the terminal states", () => { |
| 151 | expect(describePullRequestState("open", true)).toBe("Draft"); | 200 | expect(describePullRequestState("open", true)).toBe("Draft"); |
| ... | @@ -1243,7 +1292,9 @@ describe("which actions need the host read again after they run", () => { | ... | @@ -1243,7 +1292,9 @@ describe("which actions need the host read again after they run", () => { |
| 1243 | // Imported from the contract rather than hand-listed, so a new PullRequestAction fails this | 1292 | // Imported from the contract rather than hand-listed, so a new PullRequestAction fails this |
| 1244 | // test until somebody decides which side of the diff it belongs on. | 1293 | // test until somebody decides which side of the diff it belongs on. |
| 1245 | expect(PullRequestAction.literals.map(pullRequestActionNeedsHostRefresh)).toEqual( | 1294 | expect(PullRequestAction.literals.map(pullRequestActionNeedsHostRefresh)).toEqual( |
| 1246 | PullRequestAction.literals.map((action) => action === "update-branch"), | 1295 | PullRequestAction.literals.map( |
| 1296 | (action) => action === "update-branch" || action === "approve-workflows", | ||
| 1297 | ), | ||
| 1247 | ); | 1298 | ); |
| 1248 | }); | 1299 | }); |
| 1249 | 1300 | ||
| ... | @@ -1260,6 +1311,7 @@ describe("which actions need the host read again after they run", () => { | ... | @@ -1260,6 +1311,7 @@ describe("which actions need the host read again after they run", () => { |
| 1260 | "enable-auto-merge", | 1311 | "enable-auto-merge", |
| 1261 | "disable-auto-merge", | 1312 | "disable-auto-merge", |
| 1262 | "merge", | 1313 | "merge", |
| 1314 | "revert", | ||
| 1263 | ] as const) { | 1315 | ] as const) { |
| 1264 | expect(pullRequestActionNeedsHostRefresh(action)).toBe(false); | 1316 | expect(pullRequestActionNeedsHostRefresh(action)).toBe(false); |
| 1265 | } | 1317 | } |
apps/web/src/components/pullRequest/pullRequestDetail.logic.ts+45-5| ... | @@ -3,6 +3,7 @@ import type { | ... | @@ -3,6 +3,7 @@ import type { |
| 3 | PullRequestActor, | 3 | PullRequestActor, |
| 4 | PullRequestBaseComparison, | 4 | PullRequestBaseComparison, |
| 5 | PullRequestCheck, | 5 | PullRequestCheck, |
| 6 | PullRequestChecksState, | ||
| 6 | PullRequestComment, | 7 | PullRequestComment, |
| 7 | PullRequestCommit, | 8 | PullRequestCommit, |
| 8 | PullRequestDetailView, | 9 | PullRequestDetailView, |
| ... | @@ -20,6 +21,45 @@ import { inferReviewCommentFenceLanguage, type ReviewCommentContext } from "~/re | ... | @@ -20,6 +21,45 @@ import { inferReviewCommentFenceLanguage, type ReviewCommentContext } from "~/re |
| 20 | const safeShellArgument = /^[A-Za-z0-9._/@+=,-]+$/; | 21 | const safeShellArgument = /^[A-Za-z0-9._/@+=,-]+$/; |
| 21 | const bitbucketRepositoryName = /^[A-Za-z0-9._-]+\/[A-Za-z0-9._-]+$/; | 22 | const bitbucketRepositoryName = /^[A-Za-z0-9._-]+\/[A-Za-z0-9._-]+$/; |
| 22 | 23 | ||
| 24 | export type PullRequestPrimaryControl = | ||
| 25 | | "resolve" | ||
| 26 | | "ready" | ||
| 27 | | "merge" | ||
| 28 | | "enable-auto-merge" | ||
| 29 | | "auto-merge-armed" | ||
| 30 | | "merged" | ||
| 31 | | "closed" | ||
| 32 | | null; | ||
| 33 | |||
| 34 | /** The one merge-area state shown in the header, including terminal and deferred states. */ | ||
| 35 | export function resolvePullRequestPrimaryControl(input: { | ||
| 36 | readonly state: PullRequestState; | ||
| 37 | readonly isDraft: boolean; | ||
| 38 | readonly mergeability: PullRequestMergeability; | ||
| 39 | readonly checksState: PullRequestChecksState | null; | ||
| 40 | readonly autoMergeEnabled: boolean | undefined; | ||
| 41 | readonly hasMergeMethod: boolean; | ||
| 42 | readonly canMerge: boolean; | ||
| 43 | readonly canMarkReady: boolean; | ||
| 44 | readonly canEnableAutoMerge: boolean; | ||
| 45 | }): PullRequestPrimaryControl { | ||
| 46 | if (input.state === "merged") return "merged"; | ||
| 47 | if (input.state === "closed") return "closed"; | ||
| 48 | if (input.mergeability === "conflicting") return "resolve"; | ||
| 49 | if (input.isDraft) return input.canMarkReady ? "ready" : null; | ||
| 50 | if (input.autoMergeEnabled) return "auto-merge-armed"; | ||
| 51 | if (!input.hasMergeMethod) return null; | ||
| 52 | if ( | ||
| 53 | input.autoMergeEnabled === false && | ||
| 54 | input.checksState !== null && | ||
| 55 | input.checksState !== "passing" && | ||
| 56 | input.canEnableAutoMerge | ||
| 57 | ) { | ||
| 58 | return "enable-auto-merge"; | ||
| 59 | } | ||
| 60 | return input.canMerge ? "merge" : null; | ||
| 61 | } | ||
| 62 | |||
| 23 | export function pullRequestCheckoutCommand( | 63 | export function pullRequestCheckoutCommand( |
| 24 | provider: SourceControlProviderKind, | 64 | provider: SourceControlProviderKind, |
| 25 | number: number, | 65 | number: number, |
| ... | @@ -949,11 +989,9 @@ export function resolveBaseFreshness(detail: { | ... | @@ -949,11 +989,9 @@ export function resolveBaseFreshness(detail: { |
| 949 | } | 989 | } |
| 950 | 990 | ||
| 951 | /** | 991 | /** |
| 952 | * Whether a completed action leaves the diff atom pointed at a comparison that no longer exists, | 992 | * Whether a completed action needs the uncached host read rather than the cheaper detail refresh. |
| 953 | * the same staleness the manual refresh button fixes. Only `update-branch` moves the head commit; | 993 | * Updating a branch moves the diff's head. Approving workflows changes data GitHub omits from the |
| 954 | * a merge moves the branch too, but it also closes the pull request, where the diff is no longer | 994 | * normal pull-request detail. Written as a `Record` so every new action makes that choice here. |
| 955 | * what anyone is looking at. Written as a `Record` so a new `PullRequestAction` fails to compile | ||
| 956 | * here until somebody decides which side of the diff it belongs on. | ||
| 957 | */ | 995 | */ |
| 958 | const ACTION_NEEDS_HOST_REFRESH: Record<PullRequestAction, boolean> = { | 996 | const ACTION_NEEDS_HOST_REFRESH: Record<PullRequestAction, boolean> = { |
| 959 | "update-branch": true, | 997 | "update-branch": true, |
| ... | @@ -964,6 +1002,8 @@ const ACTION_NEEDS_HOST_REFRESH: Record<PullRequestAction, boolean> = { | ... | @@ -964,6 +1002,8 @@ const ACTION_NEEDS_HOST_REFRESH: Record<PullRequestAction, boolean> = { |
| 964 | reopen: false, | 1002 | reopen: false, |
| 965 | "enable-auto-merge": false, | 1003 | "enable-auto-merge": false, |
| 966 | "disable-auto-merge": false, | 1004 | "disable-auto-merge": false, |
| 1005 | revert: false, | ||
| 1006 | "approve-workflows": true, | ||
| 967 | }; | 1007 | }; |
| 968 | 1008 | ||
| 969 | export function pullRequestActionNeedsHostRefresh(action: PullRequestAction): boolean { | 1009 | export function pullRequestActionNeedsHostRefresh(action: PullRequestAction): boolean { |
apps/web/src/components/pullRequest/pullRequestList.logic.test.ts+167| ... | @@ -21,6 +21,7 @@ import { | ... | @@ -21,6 +21,7 @@ import { |
| 21 | readPullRequestListSnapshot, | 21 | readPullRequestListSnapshot, |
| 22 | writePullRequestListSnapshot, | 22 | writePullRequestListSnapshot, |
| 23 | rankPullRequestMatches, | 23 | rankPullRequestMatches, |
| 24 | rankPullRequestsByMergeReadiness, | ||
| 24 | scorePullRequestMatch, | 25 | scorePullRequestMatch, |
| 25 | retainVisiblePullRequestStatsBatches, | 26 | retainVisiblePullRequestStatsBatches, |
| 26 | withDiffStat, | 27 | withDiffStat, |
| ... | @@ -29,6 +30,11 @@ import { | ... | @@ -29,6 +30,11 @@ import { |
| 29 | resolveSelectedEnvironmentId, | 30 | resolveSelectedEnvironmentId, |
| 30 | type EnvironmentPullRequestEntry, | 31 | type EnvironmentPullRequestEntry, |
| 31 | } from "./pullRequestList.logic"; | 32 | } from "./pullRequestList.logic"; |
| 33 | import { | ||
| 34 | pullRequestListPreferences, | ||
| 35 | readPullRequestListPreferences, | ||
| 36 | writePullRequestListPreferences, | ||
| 37 | } from "./pullRequestListPreferences"; | ||
| 32 | 38 | ||
| 33 | const VIEWERS = { "github.com": "Bilal" } as const; | 39 | const VIEWERS = { "github.com": "Bilal" } as const; |
| 34 | const NO_VIEWERS = {} as const; | 40 | const NO_VIEWERS = {} as const; |
| ... | @@ -656,6 +662,99 @@ describe("ranking what a search found", () => { | ... | @@ -656,6 +662,99 @@ describe("ranking what a search found", () => { |
| 656 | }); | 662 | }); |
| 657 | }); | 663 | }); |
| 658 | 664 | ||
| 665 | describe("default merge-readiness ranking", () => { | ||
| 666 | it("puts ready work first, finished work after open work, and every conflict last", () => { | ||
| 667 | const conflict = entry({ | ||
| 668 | number: 1, | ||
| 669 | mergeability: "conflicting", | ||
| 670 | checksState: "passing", | ||
| 671 | reviewDecision: "approved", | ||
| 672 | updatedAt: "2026-09-01T00:00:00Z", | ||
| 673 | }); | ||
| 674 | const other = entry({ | ||
| 675 | number: 2, | ||
| 676 | checksState: "pending", | ||
| 677 | updatedAt: "2026-08-01T00:00:00Z", | ||
| 678 | }); | ||
| 679 | const green = entry({ | ||
| 680 | number: 3, | ||
| 681 | checksState: "passing", | ||
| 682 | reviewDecision: "review-required", | ||
| 683 | updatedAt: "2026-07-01T00:00:00Z", | ||
| 684 | }); | ||
| 685 | const approved = entry({ | ||
| 686 | number: 4, | ||
| 687 | checksState: "passing", | ||
| 688 | reviewDecision: "approved", | ||
| 689 | updatedAt: "2026-06-01T00:00:00Z", | ||
| 690 | }); | ||
| 691 | const draft = entry({ | ||
| 692 | number: 5, | ||
| 693 | isDraft: true, | ||
| 694 | checksState: "passing", | ||
| 695 | reviewDecision: "approved", | ||
| 696 | updatedAt: "2026-09-02T00:00:00Z", | ||
| 697 | }); | ||
| 698 | const finished = entry({ | ||
| 699 | number: 6, | ||
| 700 | state: "merged", | ||
| 701 | checksState: "passing", | ||
| 702 | reviewDecision: "approved", | ||
| 703 | updatedAt: "2026-09-03T00:00:00Z", | ||
| 704 | }); | ||
| 705 | |||
| 706 | expect( | ||
| 707 | rankPullRequestsByMergeReadiness([conflict, other, green, approved, draft, finished]).map( | ||
| 708 | (row) => row.number, | ||
| 709 | ), | ||
| 710 | ).toEqual([4, 3, 5, 2, 6, 1]); | ||
| 711 | }); | ||
| 712 | |||
| 713 | it("uses recency inside one readiness tier", () => { | ||
| 714 | const older = entry({ number: 1, checksState: "passing" }); | ||
| 715 | const newer = entry({ | ||
| 716 | number: 2, | ||
| 717 | checksState: "passing", | ||
| 718 | updatedAt: "2026-08-01T00:00:00Z", | ||
| 719 | }); | ||
| 720 | |||
| 721 | expect(rankPullRequestsByMergeReadiness([older, newer]).map((row) => row.number)).toEqual([ | ||
| 722 | 2, 1, | ||
| 723 | ]); | ||
| 724 | }); | ||
| 725 | |||
| 726 | it("puts the smallest measured change first inside a readiness tier", () => { | ||
| 727 | const larger = entry({ | ||
| 728 | number: 1, | ||
| 729 | checksState: "passing", | ||
| 730 | reviewDecision: "approved", | ||
| 731 | additions: 40, | ||
| 732 | deletions: 10, | ||
| 733 | updatedAt: "2026-09-01T00:00:00Z", | ||
| 734 | }); | ||
| 735 | const smaller = entry({ | ||
| 736 | number: 2, | ||
| 737 | checksState: "passing", | ||
| 738 | reviewDecision: "approved", | ||
| 739 | additions: 3, | ||
| 740 | deletions: 2, | ||
| 741 | updatedAt: "2026-08-01T00:00:00Z", | ||
| 742 | }); | ||
| 743 | const unknown = entry({ | ||
| 744 | number: 3, | ||
| 745 | checksState: "passing", | ||
| 746 | reviewDecision: "approved", | ||
| 747 | additions: 0, | ||
| 748 | deletions: 0, | ||
| 749 | updatedAt: "2026-09-02T00:00:00Z", | ||
| 750 | }); | ||
| 751 | |||
| 752 | expect( | ||
| 753 | rankPullRequestsByMergeReadiness([larger, unknown, smaller]).map((row) => row.number), | ||
| 754 | ).toEqual([2, 1, 3]); | ||
| 755 | }); | ||
| 756 | }); | ||
| 757 | |||
| 659 | describe("line counts that arrive after the rows", () => { | 758 | describe("line counts that arrive after the rows", () => { |
| 660 | const stats = new Map([["env-1 project-1 7", { additions: 42, deletions: 3 }]]); | 759 | const stats = new Map([["env-1 project-1 7", { additions: 42, deletions: 3 }]]); |
| 661 | 760 | ||
| ... | @@ -858,6 +957,74 @@ describe("the list snapshot across a reload", () => { | ... | @@ -858,6 +957,74 @@ describe("the list snapshot across a reload", () => { |
| 858 | }); | 957 | }); |
| 859 | }); | 958 | }); |
| 860 | 959 | ||
| 960 | describe("remembered pull request list controls", () => { | ||
| 961 | const makeStorage = () => { | ||
| 962 | const held = new Map<string, string>(); | ||
| 963 | return { | ||
| 964 | getItem: (key: string) => held.get(key) ?? null, | ||
| 965 | setItem: (key: string, value: string) => void held.set(key, value), | ||
| 966 | }; | ||
| 967 | }; | ||
| 968 | |||
| 969 | it("restores every filter and the selected sort", () => { | ||
| 970 | const storage = makeStorage(); | ||
| 971 | const preferences = { | ||
| 972 | involvement: "reviewing", | ||
| 973 | state: "merged", | ||
| 974 | environmentId: "env-1" as EnvironmentId, | ||
| 975 | projectId: "project-1" as ProjectId, | ||
| 976 | host: "github.com", | ||
| 977 | q: "workflow", | ||
| 978 | draft: "hide", | ||
| 979 | review: "approved", | ||
| 980 | checks: "passing", | ||
| 981 | author: "octocat", | ||
| 982 | labels: ["bug", "priority"], | ||
| 983 | sort: "largest", | ||
| 984 | } as const; | ||
| 985 | |||
| 986 | writePullRequestListPreferences(preferences, storage); | ||
| 987 | expect(readPullRequestListPreferences(storage)).toEqual(preferences); | ||
| 988 | }); | ||
| 989 | |||
| 990 | it("omits the default sort from storage", () => { | ||
| 991 | expect( | ||
| 992 | pullRequestListPreferences({ | ||
| 993 | involvement: "all", | ||
| 994 | state: "open", | ||
| 995 | sort: "ready", | ||
| 996 | }), | ||
| 997 | ).toEqual({ involvement: "all", state: "open" }); | ||
| 998 | }); | ||
| 999 | |||
| 1000 | it("keeps an explicit recency sort", () => { | ||
| 1001 | expect( | ||
| 1002 | pullRequestListPreferences({ involvement: "all", state: "open", sort: "updated" }), | ||
| 1003 | ).toEqual({ involvement: "all", state: "open", sort: "updated" }); | ||
| 1004 | }); | ||
| 1005 | |||
| 1006 | it("falls back to the default controls when storage is corrupt", () => { | ||
| 1007 | const storage = makeStorage(); | ||
| 1008 | storage.setItem("t3.pullRequests.preferences", "{not json"); | ||
| 1009 | expect(readPullRequestListPreferences(storage)).toEqual({ involvement: "all", state: "open" }); | ||
| 1010 | }); | ||
| 1011 | |||
| 1012 | it("falls back when browser policy denies storage", () => { | ||
| 1013 | const denied = { | ||
| 1014 | getItem: () => { | ||
| 1015 | throw new Error("storage denied"); | ||
| 1016 | }, | ||
| 1017 | setItem: () => { | ||
| 1018 | throw new Error("storage denied"); | ||
| 1019 | }, | ||
| 1020 | }; | ||
| 1021 | expect(readPullRequestListPreferences(denied)).toEqual({ involvement: "all", state: "open" }); | ||
| 1022 | expect(() => | ||
| 1023 | writePullRequestListPreferences({ involvement: "all", state: "open" }, denied), | ||
| 1024 | ).not.toThrow(); | ||
| 1025 | }); | ||
| 1026 | }); | ||
| 1027 | |||
| 861 | const ENV_1 = "env-1" as EnvironmentId; | 1028 | const ENV_1 = "env-1" as EnvironmentId; |
| 862 | const ENV_2 = "env-2" as EnvironmentId; | 1029 | const ENV_2 = "env-2" as EnvironmentId; |
| 863 | 1030 |
apps/web/src/components/pullRequest/pullRequestList.logic.ts+30-1| ... | @@ -985,7 +985,7 @@ export function scorePullRequestMatch(entry: PullRequestListEntry, query: string | ... | @@ -985,7 +985,7 @@ export function scorePullRequestMatch(entry: PullRequestListEntry, query: string |
| 985 | 985 | ||
| 986 | /** | 986 | /** |
| 987 | * Search results in the order they answer the question, most convincing first, and by recency | 987 | * Search results in the order they answer the question, most convincing first, and by recency |
| 988 | * among equals. Only for a search: without one, a listing is a timeline and recency is the order. | 988 | * among equals. Without a search, preserve the host order for the caller's browse-time ranking. |
| 989 | */ | 989 | */ |
| 990 | export function rankPullRequestMatches<Entry extends PullRequestListEntry>( | 990 | export function rankPullRequestMatches<Entry extends PullRequestListEntry>( |
| 991 | entries: ReadonlyArray<Entry>, | 991 | entries: ReadonlyArray<Entry>, |
| ... | @@ -998,6 +998,35 @@ export function rankPullRequestMatches<Entry extends PullRequestListEntry>( | ... | @@ -998,6 +998,35 @@ export function rankPullRequestMatches<Entry extends PullRequestListEntry>( |
| 998 | }); | 998 | }); |
| 999 | } | 999 | } |
| 1000 | 1000 | ||
| 1001 | /** | ||
| 1002 | * The default review queue: work that is green and approved, then green work still waiting on a | ||
| 1003 | * verdict, then everything else still open. Drafts stay in that third tier because their author | ||
| 1004 | * has not made them mergeable yet. Finished work follows open work when all states are visible. A | ||
| 1005 | * known conflict is never ready, whatever its checks, review or state say, so it stays at the | ||
| 1006 | * bottom. Smaller measured changes come first within a tier; recency only breaks a remaining tie. | ||
| 1007 | */ | ||
| 1008 | export function rankPullRequestsByMergeReadiness<Entry extends PullRequestListEntry>( | ||
| 1009 | entries: ReadonlyArray<Entry>, | ||
| 1010 | hasMeasuredSize: (entry: Entry) => boolean = (entry) => entry.additions + entry.deletions > 0, | ||
| 1011 | ): ReadonlyArray<Entry> { | ||
| 1012 | const tier = (entry: Entry) => { | ||
| 1013 | if (entry.mergeability === "conflicting") return 4; | ||
| 1014 | if (entry.state !== "open") return 3; | ||
| 1015 | if (entry.isDraft) return 2; | ||
| 1016 | if (entry.checksState === "passing" && entry.reviewDecision === "approved") return 0; | ||
| 1017 | if (entry.checksState === "passing") return 1; | ||
| 1018 | return 2; | ||
| 1019 | }; | ||
| 1020 | return entries.toSorted((left, right) => { | ||
| 1021 | const byTier = tier(left) - tier(right); | ||
| 1022 | if (byTier !== 0) return byTier; | ||
| 1023 | const byMeasurement = Number(hasMeasuredSize(right)) - Number(hasMeasuredSize(left)); | ||
| 1024 | if (byMeasurement !== 0) return byMeasurement; | ||
| 1025 | const bySize = left.additions + left.deletions - (right.additions + right.deletions); | ||
| 1026 | return bySize !== 0 ? bySize : right.updatedAt.localeCompare(left.updatedAt); | ||
| 1027 | }); | ||
| 1028 | } | ||
| 1029 | |||
| 1001 | /** | 1030 | /** |
| 1002 | * A row with the line counts that arrived after it did. Only where the host left them out — a | 1031 | * A row with the line counts that arrived after it did. Only where the host left them out — a |
| 1003 | * listing that carried them is not second-guessed — and only where they have arrived, since a row | 1032 | * listing that carried them is not second-guessed — and only where they have arrived, since a row |
apps/web/src/components/pullRequest/pullRequestListPreferences.ts created+122| ... | @@ -0,0 +1,122 @@ | ||
| 1 | import * as Schema from "effect/Schema"; | ||
| 2 | |||
| 3 | import { | ||
| 4 | EnvironmentId, | ||
| 5 | ProjectId, | ||
| 6 | PullRequestInvolvement, | ||
| 7 | PullRequestListFilters, | ||
| 8 | PullRequestListState, | ||
| 9 | } from "@t3tools/contracts"; | ||
| 10 | |||
| 11 | export const PullRequestListSort = Schema.Literals([ | ||
| 12 | "ready", | ||
| 13 | "updated", | ||
| 14 | "newest", | ||
| 15 | "oldest", | ||
| 16 | "largest", | ||
| 17 | "smallest", | ||
| 18 | ]); | ||
| 19 | export type PullRequestListSort = typeof PullRequestListSort.Type; | ||
| 20 | |||
| 21 | export interface PullRequestListPreferences { | ||
| 22 | readonly involvement: PullRequestInvolvement; | ||
| 23 | readonly state: PullRequestListState; | ||
| 24 | readonly environmentId?: EnvironmentId; | ||
| 25 | readonly projectId?: ProjectId; | ||
| 26 | readonly host?: string; | ||
| 27 | readonly q?: string; | ||
| 28 | readonly draft?: "only" | "hide"; | ||
| 29 | readonly review?: NonNullable<PullRequestListFilters["review"]>; | ||
| 30 | readonly checks?: NonNullable<PullRequestListFilters["checks"]>; | ||
| 31 | readonly author?: string; | ||
| 32 | readonly labels?: ReadonlyArray<string>; | ||
| 33 | readonly sort?: PullRequestListSort; | ||
| 34 | } | ||
| 35 | |||
| 36 | export type PullRequestListPreferencePatch = { | ||
| 37 | [Key in keyof PullRequestListPreferences]?: PullRequestListPreferences[Key] | undefined; | ||
| 38 | }; | ||
| 39 | |||
| 40 | export const DEFAULT_PULL_REQUEST_LIST_PREFERENCES = { | ||
| 41 | involvement: "all", | ||
| 42 | state: "open", | ||
| 43 | } as const satisfies PullRequestListPreferences; | ||
| 44 | |||
| 45 | const BoundedPreference = Schema.String.check(Schema.isMaxLength(200)); | ||
| 46 | const PullRequestListPreferencesSchema = Schema.Struct({ | ||
| 47 | involvement: PullRequestInvolvement, | ||
| 48 | state: PullRequestListState, | ||
| 49 | environmentId: Schema.optional(EnvironmentId), | ||
| 50 | projectId: Schema.optional(ProjectId), | ||
| 51 | host: Schema.optional(BoundedPreference), | ||
| 52 | q: Schema.optional(BoundedPreference), | ||
| 53 | draft: PullRequestListFilters.fields.draft, | ||
| 54 | review: PullRequestListFilters.fields.review, | ||
| 55 | checks: PullRequestListFilters.fields.checks, | ||
| 56 | author: Schema.optional(BoundedPreference), | ||
| 57 | labels: Schema.optional(Schema.Array(BoundedPreference).check(Schema.isMaxLength(10))), | ||
| 58 | sort: Schema.optional(PullRequestListSort), | ||
| 59 | }); | ||
| 60 | |||
| 61 | const decodePullRequestListPreferences = Schema.decodeUnknownOption( | ||
| 62 | PullRequestListPreferencesSchema, | ||
| 63 | ); | ||
| 64 | const PULL_REQUEST_LIST_PREFERENCES_STORAGE_KEY = "t3.pullRequests.preferences"; | ||
| 65 | type PreferenceStorage = Pick<Storage, "getItem" | "setItem">; | ||
| 66 | |||
| 67 | function resolvePreferenceStorage( | ||
| 68 | storage: PreferenceStorage | undefined, | ||
| 69 | ): PreferenceStorage | undefined { | ||
| 70 | return storage ?? (typeof window === "undefined" ? undefined : window.localStorage); | ||
| 71 | } | ||
| 72 | |||
| 73 | /** Only list controls are remembered. The selected row remains a URL and right-panel concern. */ | ||
| 74 | export function pullRequestListPreferences( | ||
| 75 | search: PullRequestListPreferences | Schema.Schema.Type<typeof PullRequestListPreferencesSchema>, | ||
| 76 | ): PullRequestListPreferences { | ||
| 77 | return { | ||
| 78 | involvement: search.involvement, | ||
| 79 | state: search.state, | ||
| 80 | ...(search.environmentId ? { environmentId: search.environmentId } : {}), | ||
| 81 | ...(search.projectId ? { projectId: search.projectId } : {}), | ||
| 82 | ...(search.host ? { host: search.host } : {}), | ||
| 83 | ...(search.q ? { q: search.q } : {}), | ||
| 84 | ...(search.draft ? { draft: search.draft } : {}), | ||
| 85 | ...(search.review ? { review: search.review } : {}), | ||
| 86 | ...(search.checks ? { checks: search.checks } : {}), | ||
| 87 | ...(search.author ? { author: search.author } : {}), | ||
| 88 | ...(search.labels && search.labels.length > 0 ? { labels: search.labels } : {}), | ||
| 89 | ...(search.sort && search.sort !== "ready" ? { sort: search.sort } : {}), | ||
| 90 | }; | ||
| 91 | } | ||
| 92 | |||
| 93 | export function readPullRequestListPreferences( | ||
| 94 | storage?: PreferenceStorage, | ||
| 95 | ): PullRequestListPreferences { | ||
| 96 | try { | ||
| 97 | const raw = resolvePreferenceStorage(storage)?.getItem( | ||
| 98 | PULL_REQUEST_LIST_PREFERENCES_STORAGE_KEY, | ||
| 99 | ); | ||
| 100 | if (!raw) return DEFAULT_PULL_REQUEST_LIST_PREFERENCES; | ||
| 101 | const decoded = decodePullRequestListPreferences(JSON.parse(raw)); | ||
| 102 | return decoded._tag === "Some" | ||
| 103 | ? pullRequestListPreferences(decoded.value) | ||
| 104 | : DEFAULT_PULL_REQUEST_LIST_PREFERENCES; | ||
| 105 | } catch { | ||
| 106 | return DEFAULT_PULL_REQUEST_LIST_PREFERENCES; | ||
| 107 | } | ||
| 108 | } | ||
| 109 | |||
| 110 | export function writePullRequestListPreferences( | ||
| 111 | preferences: PullRequestListPreferences, | ||
| 112 | storage?: PreferenceStorage, | ||
| 113 | ): void { | ||
| 114 | try { | ||
| 115 | resolvePreferenceStorage(storage)?.setItem( | ||
| 116 | PULL_REQUEST_LIST_PREFERENCES_STORAGE_KEY, | ||
| 117 | JSON.stringify(preferences), | ||
| 118 | ); | ||
| 119 | } catch { | ||
| 120 | // Storage can be full or denied; the URL remains the source of truth for this visit. | ||
| 121 | } | ||
| 122 | } | ||
apps/web/src/components/pullRequest/pullRequestPresentation.tsx+35-6| ... | @@ -120,6 +120,11 @@ export function PullRequestStateGlyph({ | ... | @@ -120,6 +120,11 @@ export function PullRequestStateGlyph({ |
| 120 | 120 | ||
| 121 | const CHECK_STATUS_PRESENTATION = { | 121 | const CHECK_STATUS_PRESENTATION = { |
| 122 | pending: { label: "Running", Icon: LoaderIcon, toneClassName: "animate-spin text-amber-500" }, | 122 | pending: { label: "Running", Icon: LoaderIcon, toneClassName: "animate-spin text-amber-500" }, |
| 123 | "action-required": { | ||
| 124 | label: "Awaiting action", | ||
| 125 | Icon: CircleDotIcon, | ||
| 126 | toneClassName: "text-amber-600 dark:text-amber-400/90", | ||
| 127 | }, | ||
| 123 | success: { | 128 | success: { |
| 124 | label: "Passed", | 129 | label: "Passed", |
| 125 | Icon: CircleCheckIcon, | 130 | Icon: CircleCheckIcon, |
| ... | @@ -134,8 +139,20 @@ const CHECK_STATUS_PRESENTATION = { | ... | @@ -134,8 +139,20 @@ const CHECK_STATUS_PRESENTATION = { |
| 134 | { label: string; Icon: typeof CircleCheckIcon; toneClassName: string } | 139 | { label: string; Icon: typeof CircleCheckIcon; toneClassName: string } |
| 135 | >; | 140 | >; |
| 136 | 141 | ||
| 137 | export function pullRequestCheckStatusLabel(status: PullRequestCheckStatus): string { | 142 | function isWorkflowApprovalCheck(check: Pick<PullRequestCheck, "status" | "url">): boolean { |
| 138 | return CHECK_STATUS_PRESENTATION[status].label; | 143 | return ( |
| 144 | check.status === "action-required" && | ||
| 145 | check.url !== null && | ||
| 146 | /\/actions\/runs\/\d+(?:\/|$)/u.test(check.url) | ||
| 147 | ); | ||
| 148 | } | ||
| 149 | |||
| 150 | export function pullRequestCheckStatusLabel( | ||
| 151 | check: Pick<PullRequestCheck, "status" | "url">, | ||
| 152 | ): string { | ||
| 153 | return isWorkflowApprovalCheck(check) | ||
| 154 | ? "Awaiting approval" | ||
| 155 | : CHECK_STATUS_PRESENTATION[check.status].label; | ||
| 139 | } | 156 | } |
| 140 | 157 | ||
| 141 | export function PullRequestCheckStatusIcon({ status }: { status: PullRequestCheckStatus }) { | 158 | export function PullRequestCheckStatusIcon({ status }: { status: PullRequestCheckStatus }) { |
| ... | @@ -187,10 +204,10 @@ export function pullRequestChecksState( | ... | @@ -187,10 +204,10 @@ export function pullRequestChecksState( |
| 187 | checks: ReadonlyArray<PullRequestCheck>, | 204 | checks: ReadonlyArray<PullRequestCheck>, |
| 188 | ): PullRequestChecksState | null { | 205 | ): PullRequestChecksState | null { |
| 189 | if (checks.length === 0) return null; | 206 | if (checks.length === 0) return null; |
| 190 | const statuses = checks.map((check) => check.status); | 207 | const statuses = new Set(checks.map((check) => check.status)); |
| 191 | if (statuses.includes("failure") || statuses.includes("cancelled")) return "failing"; | 208 | if (statuses.has("failure") || statuses.has("cancelled")) return "failing"; |
| 192 | if (statuses.includes("pending")) return "pending"; | 209 | if (statuses.has("pending") || statuses.has("action-required")) return "pending"; |
| 193 | return statuses.includes("success") ? "passing" : null; | 210 | return statuses.has("success") ? "passing" : null; |
| 194 | } | 211 | } |
| 195 | 212 | ||
| 196 | /** | 213 | /** |
| ... | @@ -435,12 +452,24 @@ export function PullRequestMetaLine({ | ... | @@ -435,12 +452,24 @@ export function PullRequestMetaLine({ |
| 435 | 452 | ||
| 436 | export function summarizePullRequestChecks(checks: ReadonlyArray<PullRequestCheck>): string { | 453 | export function summarizePullRequestChecks(checks: ReadonlyArray<PullRequestCheck>): string { |
| 437 | if (checks.length === 0) return "No checks reported"; | 454 | if (checks.length === 0) return "No checks reported"; |
| 455 | const actionRequired = checks.filter((check) => check.status === "action-required"); | ||
| 456 | const workflowApprovalRequired = actionRequired.filter(isWorkflowApprovalCheck).length; | ||
| 457 | const otherActionRequired = actionRequired.length - workflowApprovalRequired; | ||
| 438 | const failed = checks.filter( | 458 | const failed = checks.filter( |
| 439 | (check) => check.status === "failure" || check.status === "cancelled", | 459 | (check) => check.status === "failure" || check.status === "cancelled", |
| 440 | ).length; | 460 | ).length; |
| 441 | const pending = checks.filter((check) => check.status === "pending").length; | 461 | const pending = checks.filter((check) => check.status === "pending").length; |
| 442 | const passed = checks.filter((check) => check.status === "success").length; | 462 | const passed = checks.filter((check) => check.status === "success").length; |
| 443 | if (failed > 0) return `${failed} of ${checks.length} failing`; | 463 | if (failed > 0) return `${failed} of ${checks.length} failing`; |
| 464 | if (workflowApprovalRequired > 0 && otherActionRequired > 0) { | ||
| 465 | return `${workflowApprovalRequired} ${workflowApprovalRequired === 1 ? "workflow" : "workflows"} and ${otherActionRequired} ${otherActionRequired === 1 ? "check" : "checks"} awaiting action`; | ||
| 466 | } | ||
| 467 | if (workflowApprovalRequired > 0) { | ||
| 468 | return `${workflowApprovalRequired} ${workflowApprovalRequired === 1 ? "workflow" : "workflows"} awaiting approval`; | ||
| 469 | } | ||
| 470 | if (otherActionRequired > 0) { | ||
| 471 | return `${otherActionRequired} ${otherActionRequired === 1 ? "check" : "checks"} awaiting action`; | ||
| 472 | } | ||
| 444 | if (pending > 0) return `${pending} of ${checks.length} running`; | 473 | if (pending > 0) return `${pending} of ${checks.length} running`; |
| 445 | return passed === checks.length ? "All checks passed" : `${passed} of ${checks.length} passing`; | 474 | return passed === checks.length ? "All checks passed" : `${passed} of ${checks.length} passing`; |
| 446 | } | 475 | } |
apps/web/src/components/sidebar/SidebarChrome.tsx+5-1| ... | @@ -30,6 +30,7 @@ import { | ... | @@ -30,6 +30,7 @@ import { |
| 30 | useSidebar, | 30 | useSidebar, |
| 31 | } from "../ui/sidebar"; | 31 | } from "../ui/sidebar"; |
| 32 | import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; | 32 | import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; |
| 33 | import { readPullRequestListPreferences } from "../pullRequest/pullRequestListPreferences"; | ||
| 33 | import { SidebarProviderUpdatePill } from "./SidebarProviderUpdatePill"; | 34 | import { SidebarProviderUpdatePill } from "./SidebarProviderUpdatePill"; |
| 34 | import { SidebarUpdateArchitectureWarning, SidebarUpdatePill } from "./SidebarUpdatePill"; | 35 | import { SidebarUpdateArchitectureWarning, SidebarUpdatePill } from "./SidebarUpdatePill"; |
| 35 | 36 | ||
| ... | @@ -157,7 +158,10 @@ export const SidebarUtilityMenu = memo(function SidebarUtilityMenu() { | ... | @@ -157,7 +158,10 @@ export const SidebarUtilityMenu = memo(function SidebarUtilityMenu() { |
| 157 | }, [isMobile, setOpenMobile]); | 158 | }, [isMobile, setOpenMobile]); |
| 158 | const handlePullRequestsClick = useCallback(() => { | 159 | const handlePullRequestsClick = useCallback(() => { |
| 159 | closeMobileSidebar(); | 160 | closeMobileSidebar(); |
| 160 | void navigate({ to: "/pull-requests", search: { involvement: "all", state: "open" } }); | 161 | void navigate({ |
| 162 | to: "/pull-requests", | ||
| 163 | search: readPullRequestListPreferences(), | ||
| 164 | }); | ||
| 161 | }, [closeMobileSidebar, navigate]); | 165 | }, [closeMobileSidebar, navigate]); |
| 162 | const handleSettingsClick = useCallback(() => { | 166 | const handleSettingsClick = useCallback(() => { |
| 163 | closeMobileSidebar(); | 167 | closeMobileSidebar(); |
apps/web/src/components/ui/badge.tsx+2| ... | @@ -15,6 +15,8 @@ const badgeVariants = cva( | ... | @@ -15,6 +15,8 @@ const badgeVariants = cva( |
| 15 | }, | 15 | }, |
| 16 | variants: { | 16 | variants: { |
| 17 | size: { | 17 | size: { |
| 18 | control: | ||
| 19 | "h-7 min-w-7 rounded-[var(--control-radius)] px-[calc(--spacing(2)-1px)] text-sm sm:h-6 sm:min-w-6 sm:text-xs", | ||
| 18 | default: | 20 | default: |
| 19 | "h-5.5 min-w-5.5 px-[calc(--spacing(1)-1px)] text-sm sm:h-4.5 sm:min-w-4.5 sm:text-xs", | 21 | "h-5.5 min-w-5.5 px-[calc(--spacing(1)-1px)] text-sm sm:h-4.5 sm:min-w-4.5 sm:text-xs", |
| 20 | lg: "h-6.5 min-w-6.5 px-[calc(--spacing(1.5)-1px)] text-base sm:h-5.5 sm:min-w-5.5 sm:text-sm", | 22 | lg: "h-6.5 min-w-6.5 px-[calc(--spacing(1.5)-1px)] text-base sm:h-5.5 sm:min-w-5.5 sm:text-sm", |
apps/web/src/components/ui/button.tsx+2| ... | @@ -53,6 +53,8 @@ const buttonVariants = cva( | ... | @@ -53,6 +53,8 @@ const buttonVariants = cva( |
| 53 | "[--control-icon-color:var(--contrast-muted-foreground)] border-input bg-popover not-dark:bg-clip-padding text-foreground shadow-xs/5 not-disabled:not-active:not-data-pressed:before:shadow-[0_1px_--theme(--color-black/4%)] dark:bg-input/32 dark:not-disabled:before:shadow-[0_-1px_--theme(--color-white/2%)] dark:not-disabled:not-active:not-data-pressed:before:shadow-[0_-1px_--theme(--color-white/6%)] [:disabled,:active,[data-pressed]]:shadow-none [:hover,[data-pressed]]:bg-accent/50 dark:[:hover,[data-pressed]]:bg-input/64", | 53 | "[--control-icon-color:var(--contrast-muted-foreground)] border-input bg-popover not-dark:bg-clip-padding text-foreground shadow-xs/5 not-disabled:not-active:not-data-pressed:before:shadow-[0_1px_--theme(--color-black/4%)] dark:bg-input/32 dark:not-disabled:before:shadow-[0_-1px_--theme(--color-white/2%)] dark:not-disabled:not-active:not-data-pressed:before:shadow-[0_-1px_--theme(--color-white/6%)] [:disabled,:active,[data-pressed]]:shadow-none [:hover,[data-pressed]]:bg-accent/50 dark:[:hover,[data-pressed]]:bg-input/64", |
| 54 | secondary: | 54 | secondary: |
| 55 | "border-transparent bg-secondary text-secondary-foreground [:active,[data-pressed]]:bg-secondary/80 [:hover,[data-pressed]]:bg-secondary/90", | 55 | "border-transparent bg-secondary text-secondary-foreground [:active,[data-pressed]]:bg-secondary/80 [:hover,[data-pressed]]:bg-secondary/90", |
| 56 | "warning-outline": | ||
| 57 | "border-warning/32 bg-warning-surface text-warning-foreground shadow-xs/5 [:disabled,:active,[data-pressed]]:shadow-none [:hover,[data-pressed]]:border-warning/40 [:hover,[data-pressed]]:bg-warning/16 dark:[:hover,[data-pressed]]:bg-warning/24", | ||
| 56 | }, | 58 | }, |
| 57 | }, | 59 | }, |
| 58 | }, | 60 | }, |
apps/web/src/routes/_chat.pull-requests.tsx+49-34| ... | @@ -25,6 +25,7 @@ import { | ... | @@ -25,6 +25,7 @@ import { |
| 25 | GitPullRequestClosedIcon, | 25 | GitPullRequestClosedIcon, |
| 26 | GitPullRequestIcon, | 26 | GitPullRequestIcon, |
| 27 | LayersIcon, | 27 | LayersIcon, |
| 28 | ListChecksIcon, | ||
| 28 | PenLineIcon, | 29 | PenLineIcon, |
| 29 | LoaderIcon, | 30 | LoaderIcon, |
| 30 | Maximize2Icon, | 31 | Maximize2Icon, |
| ... | @@ -57,6 +58,7 @@ import { | ... | @@ -57,6 +58,7 @@ import { |
| 57 | pullRequestEntryKey, | 58 | pullRequestEntryKey, |
| 58 | pullRequestEntryViewer, | 59 | pullRequestEntryViewer, |
| 59 | rankPullRequestMatches, | 60 | rankPullRequestMatches, |
| 61 | rankPullRequestsByMergeReadiness, | ||
| 60 | pullRequestEnvironmentSetKey, | 62 | pullRequestEnvironmentSetKey, |
| 61 | readPullRequestListSnapshot, | 63 | readPullRequestListSnapshot, |
| 62 | resolveProjectScope, | 64 | resolveProjectScope, |
| ... | @@ -76,6 +78,13 @@ import { | ... | @@ -76,6 +78,13 @@ import { |
| 76 | type PullRequestStatsScope, | 78 | type PullRequestStatsScope, |
| 77 | type PullRequestPartitionsSnapshot, | 79 | type PullRequestPartitionsSnapshot, |
| 78 | } from "../components/pullRequest/pullRequestList.logic"; | 80 | } from "../components/pullRequest/pullRequestList.logic"; |
| 81 | import { | ||
| 82 | pullRequestListPreferences, | ||
| 83 | type PullRequestListPreferencePatch, | ||
| 84 | type PullRequestListPreferences, | ||
| 85 | type PullRequestListSort, | ||
| 86 | writePullRequestListPreferences, | ||
| 87 | } from "../components/pullRequest/pullRequestListPreferences"; | ||
| 79 | import { assignProjectsToEnvironments } from "../components/pullRequest/pullRequestProjectAssignment.logic"; | 88 | import { assignProjectsToEnvironments } from "../components/pullRequest/pullRequestProjectAssignment.logic"; |
| 80 | import { PullRequestDetailPanel } from "../components/pullRequest/PullRequestDetailPanel"; | 89 | import { PullRequestDetailPanel } from "../components/pullRequest/PullRequestDetailPanel"; |
| 81 | import { | 90 | import { |
| ... | @@ -129,9 +138,7 @@ import { useAtomCommand } from "../state/use-atom-command"; | ... | @@ -129,9 +138,7 @@ import { useAtomCommand } from "../state/use-atom-command"; |
| 129 | import { cn } from "~/lib/utils"; | 138 | import { cn } from "~/lib/utils"; |
| 130 | import { getSourceControlPresentationForKind } from "~/sourceControlPresentation"; | 139 | import { getSourceControlPresentationForKind } from "~/sourceControlPresentation"; |
| 131 | 140 | ||
| 132 | export interface PullRequestsSearch { | 141 | export interface PullRequestsSearch extends PullRequestListPreferences { |
| 133 | readonly involvement: PullRequestInvolvement; | ||
| 134 | readonly state: PullRequestListState; | ||
| 135 | /** | 142 | /** |
| 136 | * Narrows the list to one server. Absent means every connected one, which is the default the | 143 | * Narrows the list to one server. Absent means every connected one, which is the default the |
| 137 | * page has now — so a link written before servers could be chosen still opens the whole list. | 144 | * page has now — so a link written before servers could be chosen still opens the whole list. |
| ... | @@ -153,21 +160,8 @@ export interface PullRequestsSearch { | ... | @@ -153,21 +160,8 @@ export interface PullRequestsSearch { |
| 153 | * link without it still opens, resolved by project id alone where that is unambiguous. | 160 | * link without it still opens, resolved by project id alone where that is unambiguous. |
| 154 | */ | 161 | */ |
| 155 | readonly selectedEnvironmentId?: EnvironmentId; | 162 | readonly selectedEnvironmentId?: EnvironmentId; |
| 156 | readonly q?: string; | ||
| 157 | /** | ||
| 158 | * The narrowings beyond state and involvement, each absent when that group is unfiltered. Flat | ||
| 159 | * in the URL because a link is read and edited by hand; folded into one record for the listing. | ||
| 160 | */ | ||
| 161 | readonly draft?: "only" | "hide"; | ||
| 162 | readonly review?: NonNullable<PullRequestListFilters["review"]>; | ||
| 163 | readonly checks?: NonNullable<PullRequestListFilters["checks"]>; | ||
| 164 | readonly author?: string; | ||
| 165 | readonly labels?: ReadonlyArray<string>; | ||
| 166 | readonly sort?: PullRequestListSort; | ||
| 167 | } | 163 | } |
| 168 | 164 | ||
| 169 | type PullRequestListSort = "updated" | "newest" | "oldest" | "largest" | "smallest"; | ||
| 170 | |||
| 171 | // The state filters wear the same glyphs the rows do, so the two read as one vocabulary. | 165 | // The state filters wear the same glyphs the rows do, so the two read as one vocabulary. |
| 172 | const INVOLVEMENT_TABS = [ | 166 | const INVOLVEMENT_TABS = [ |
| 173 | { value: "all", label: "All", Icon: LayersIcon }, | 167 | { value: "all", label: "All", Icon: LayersIcon }, |
| ... | @@ -183,6 +177,7 @@ const STATE_TABS = [ | ... | @@ -183,6 +177,7 @@ const STATE_TABS = [ |
| 183 | ] as const satisfies ReadonlyArray<PullRequestFilterOption<PullRequestListState>>; | 177 | ] as const satisfies ReadonlyArray<PullRequestFilterOption<PullRequestListState>>; |
| 184 | 178 | ||
| 185 | const SORT_OPTIONS = [ | 179 | const SORT_OPTIONS = [ |
| 180 | { value: "ready", label: "Merge readiness", Icon: ListChecksIcon }, | ||
| 186 | { value: "updated", label: "Recently updated", Icon: ClockIcon }, | 181 | { value: "updated", label: "Recently updated", Icon: ClockIcon }, |
| 187 | { value: "newest", label: "Newest shown", Icon: CalendarArrowDownIcon }, | 182 | { value: "newest", label: "Newest shown", Icon: CalendarArrowDownIcon }, |
| 188 | { value: "oldest", label: "Oldest shown", Icon: CalendarArrowUpIcon }, | 183 | { value: "oldest", label: "Oldest shown", Icon: CalendarArrowUpIcon }, |
| ... | @@ -287,9 +282,9 @@ export const Route = createFileRoute("/_chat/pull-requests")({ | ... | @@ -287,9 +282,9 @@ export const Route = createFileRoute("/_chat/pull-requests")({ |
| 287 | 282 | ||
| 288 | function PullRequestsRouteView() { | 283 | function PullRequestsRouteView() { |
| 289 | const search = Route.useSearch(); | 284 | const search = Route.useSearch(); |
| 290 | const sort = search.sort ?? "updated"; | 285 | const sort = search.sort ?? "ready"; |
| 291 | const statsPolicy: PullRequestStatsPolicy = | 286 | const statsPolicy: PullRequestStatsPolicy = |
| 292 | sort === "largest" || sort === "smallest" ? "eager" : "visible"; | 287 | sort === "ready" || sort === "largest" || sort === "smallest" ? "eager" : "visible"; |
| 293 | const navigate = useNavigate({ from: Route.fullPath }); | 288 | const navigate = useNavigate({ from: Route.fullPath }); |
| 294 | const { environments } = useEnvironments(); | 289 | const { environments } = useEnvironments(); |
| 295 | // Every connected environment that has said it can list pull requests. Sorted, so the query | 290 | // Every connected environment that has said it can list pull requests. Sorted, so the query |
| ... | @@ -487,7 +482,7 @@ function PullRequestsRouteView() { | ... | @@ -487,7 +482,7 @@ function PullRequestsRouteView() { |
| 487 | return { | 482 | return { |
| 488 | involvement: next.involvement ?? previous.involvement, | 483 | involvement: next.involvement ?? previous.involvement, |
| 489 | state: next.state ?? previous.state, | 484 | state: next.state ?? previous.state, |
| 490 | ...(next.sort && next.sort !== "updated" ? { sort: next.sort } : {}), | 485 | ...(next.sort && next.sort !== "ready" ? { sort: next.sort } : {}), |
| 491 | ...(next.repository ? { repository: next.repository } : {}), | 486 | ...(next.repository ? { repository: next.repository } : {}), |
| 492 | ...(next.number ? { number: next.number } : {}), | 487 | ...(next.number ? { number: next.number } : {}), |
| 493 | ...(next.projectId ? { projectId: next.projectId } : {}), | 488 | ...(next.projectId ? { projectId: next.projectId } : {}), |
| ... | @@ -510,22 +505,25 @@ function PullRequestsRouteView() { | ... | @@ -510,22 +505,25 @@ function PullRequestsRouteView() { |
| 510 | [navigate], | 505 | [navigate], |
| 511 | ); | 506 | ); |
| 512 | 507 | ||
| 513 | // Changing what the list contains must not leave a selection from the previous view open. | ||
| 514 | // The project filter is untouched: it is the user's scope, not part of the selection. | ||
| 515 | const clearedSelection = { | 508 | const clearedSelection = { |
| 516 | repository: undefined, | 509 | repository: undefined, |
| 517 | number: undefined, | 510 | number: undefined, |
| 518 | selectedProjectId: undefined, | 511 | selectedProjectId: undefined, |
| 519 | selectedEnvironmentId: undefined, | 512 | selectedEnvironmentId: undefined, |
| 520 | }; | 513 | }; |
| 521 | const updateListScope = (patch: { | 514 | // List controls change the rows behind the detail, not the independent selected surface. The |
| 522 | [Key in keyof PullRequestsSearch]?: PullRequestsSearch[Key] | undefined; | 515 | // reader can keep working in that panel while narrowing, sorting, or switching projects. |
| 523 | }) => { | 516 | const updateListScope = (patch: PullRequestListPreferencePatch) => { |
| 524 | if (rightPanelRef !== null) { | 517 | const currentPreferences = pullRequestListPreferences({ |
| 525 | // Hide the old selection while retaining peer PR tabs for parallel reviews. | 518 | ...search, |
| 526 | useRightPanelStore.getState().close(rightPanelRef); | 519 | ...patch, |
| 527 | } | 520 | involvement: patch.involvement ?? search.involvement, |
| 528 | updateSearch({ ...patch, ...clearedSelection }); | 521 | state: patch.state ?? search.state, |
| 522 | }); | ||
| 523 | // Opening a selected-only link does not save its default all/open list. Once a list control | ||
| 524 | // changes, the entire visible scope is intentional and becomes the next sidebar destination. | ||
| 525 | writePullRequestListPreferences(currentPreferences); | ||
| 526 | updateSearch(patch); | ||
| 529 | }; | 527 | }; |
| 530 | 528 | ||
| 531 | // Searching asks the hosts, which takes a round trip, so the text is held for a moment before | 529 | // Searching asks the hosts, which takes a round trip, so the text is held for a moment before |
| ... | @@ -1420,7 +1418,24 @@ function PullRequestsRouteView() { | ... | @@ -1420,7 +1418,24 @@ function PullRequestsRouteView() { |
| 1420 | ...group, | 1418 | ...group, |
| 1421 | entries: group.entries.map((entry) => withDiffStat(entry, statsByRow)), | 1419 | entries: group.entries.map((entry) => withDiffStat(entry, statsByRow)), |
| 1422 | })); | 1420 | })); |
| 1423 | if (sort === "updated") return enriched; | 1421 | if (sort === "ready" && typedParsed.text.length === 0) { |
| 1422 | return [ | ||
| 1423 | { | ||
| 1424 | key: "others" as const, | ||
| 1425 | label: "", | ||
| 1426 | entries: rankPullRequestsByMergeReadiness( | ||
| 1427 | enriched.flatMap((group) => group.entries), | ||
| 1428 | (entry) => | ||
| 1429 | entry.additions + entry.deletions > 0 || | ||
| 1430 | statsByRow.has(pullRequestDiffStatKey(entry)), | ||
| 1431 | ), | ||
| 1432 | }, | ||
| 1433 | ]; | ||
| 1434 | } | ||
| 1435 | // Searching keeps its relevance order and priority groups unless the reader explicitly asks | ||
| 1436 | // for another sort. The readiness queue is the default browse order, not a way to bury a | ||
| 1437 | // closer text match. | ||
| 1438 | if (sort === "ready" || sort === "updated") return enriched; | ||
| 1424 | const entries = enriched.flatMap((group) => group.entries); | 1439 | const entries = enriched.flatMap((group) => group.entries); |
| 1425 | const hasSize = (entry: (typeof entries)[number]) => | 1440 | const hasSize = (entry: (typeof entries)[number]) => |
| 1426 | entry.additions + entry.deletions > 0 || statsByRow.has(pullRequestDiffStatKey(entry)); | 1441 | entry.additions + entry.deletions > 0 || statsByRow.has(pullRequestDiffStatKey(entry)); |
| ... | @@ -1448,7 +1463,7 @@ function PullRequestsRouteView() { | ... | @@ -1448,7 +1463,7 @@ function PullRequestsRouteView() { |
| 1448 | }), | 1463 | }), |
| 1449 | }, | 1464 | }, |
| 1450 | ]; | 1465 | ]; |
| 1451 | }, [groups, sort, statsByRow]); | 1466 | }, [groups, sort, statsByRow, typedParsed.text]); |
| 1452 | 1467 | ||
| 1453 | const linkedSelection = useMemo( | 1468 | const linkedSelection = useMemo( |
| 1454 | () => | 1469 | () => |
| ... | @@ -1567,7 +1582,7 @@ function PullRequestsRouteView() { | ... | @@ -1567,7 +1582,7 @@ function PullRequestsRouteView() { |
| 1567 | <PullRequestSearchInput | 1582 | <PullRequestSearchInput |
| 1568 | value={search.q ?? ""} | 1583 | value={search.q ?? ""} |
| 1569 | busy={typedQuery.length > 0 && (!querySettled || showingCarried)} | 1584 | busy={typedQuery.length > 0 && (!querySettled || showingCarried)} |
| 1570 | onChange={(query) => updateSearch({ q: query || undefined })} | 1585 | onChange={(query) => updateListScope({ q: query || undefined })} |
| 1571 | /> | 1586 | /> |
| 1572 | ); | 1587 | ); |
| 1573 | const panelToggleControls = ( | 1588 | const panelToggleControls = ( |
| ... | @@ -1635,7 +1650,7 @@ function PullRequestsRouteView() { | ... | @@ -1635,7 +1650,7 @@ function PullRequestsRouteView() { |
| 1635 | searching={typedQuery.length > 0 && (!querySettled || showingCarried)} | 1650 | searching={typedQuery.length > 0 && (!querySettled || showingCarried)} |
| 1636 | canLoadMore={listData?.truncated === true && (canContinue || pageSize < MAX_PAGE_SIZE)} | 1651 | canLoadMore={listData?.truncated === true && (canContinue || pageSize < MAX_PAGE_SIZE)} |
| 1637 | loadingMore={loadingMore} | 1652 | loadingMore={loadingMore} |
| 1638 | onClearQuery={() => updateSearch({ q: undefined })} | 1653 | onClearQuery={() => updateListScope({ q: undefined })} |
| 1639 | onLoadMore={loadMore} | 1654 | onLoadMore={loadMore} |
| 1640 | /> | 1655 | /> |
| 1641 | ) : ( | 1656 | ) : ( |
| ... | @@ -1739,7 +1754,7 @@ function PullRequestsRouteView() { | ... | @@ -1739,7 +1754,7 @@ function PullRequestsRouteView() { |
| 1739 | outlined | 1754 | outlined |
| 1740 | value={sort} | 1755 | value={sort} |
| 1741 | options={SORT_OPTIONS} | 1756 | options={SORT_OPTIONS} |
| 1742 | onChange={(next) => updateSearch({ sort: next })} | 1757 | onChange={(next) => updateListScope({ sort: next })} |
| 1743 | /> | 1758 | /> |
| 1744 | ); | 1759 | ); |
| 1745 | const filtersMenu = ( | 1760 | const filtersMenu = ( |
docs/user/source-control.md+14-2| ... | @@ -41,9 +41,19 @@ T3 Code works with the platforms your team already uses: | ... | @@ -41,9 +41,19 @@ T3 Code works with the platforms your team already uses: |
| 41 | 41 | ||
| 42 | - See if your current branch already has an open PR/MR | 42 | - See if your current branch already has an open PR/MR |
| 43 | - Open several reviews from the **Pull requests** page as tabs in the right panel | 43 | - Open several reviews from the **Pull requests** page as tabs in the right panel |
| 44 | - By default, see passing and approved reviews first, passing reviews awaiting approval next, and | ||
| 45 | conflicting reviews last. Smaller changes come first within each readiness group, and finished | ||
| 46 | reviews follow open work when all states are visible. | ||
| 44 | - Filter the list by author or labels, rank authors by merges in the loaded results, see label and | 47 | - Filter the list by author or labels, rank authors by merges in the loaded results, see label and |
| 45 | change-size context on each row, and sort the results currently shown by update time, creation | 48 | change-size context on each row, and sort the results currently shown by readiness, update time, |
| 46 | time, or change size | 49 | creation time, or change size. Your filters, search, scope, and sort are restored when you return. |
| 50 | - Merge now, or on GitHub, GitLab, and Azure DevOps, leave an auto-merge instruction with a chosen | ||
| 51 | strategy while checks are outstanding; see the completed state in the same control after the | ||
| 52 | pull request merges | ||
| 53 | - On GitHub, approve fork workflows that are waiting to run and open a revert pull request for a | ||
| 54 | merged change | ||
| 55 | - Timeline line counts stay hidden on merge commits, where GitHub's totals include upstream changes | ||
| 56 | brought in from the base branch | ||
| 47 | - While working in a thread, open linked reviews in the same compact right-panel tabs without | 57 | - While working in a thread, open linked reviews in the same compact right-panel tabs without |
| 48 | leaving the conversation | 58 | leaving the conversation |
| 49 | - Enable **Settings → General → Proactive panels** to open a newly linked review automatically and | 59 | - Enable **Settings → General → Proactive panels** to open a newly linked review automatically and |
| ... | @@ -56,6 +66,8 @@ T3 Code works with the platforms your team already uses: | ... | @@ -56,6 +66,8 @@ T3 Code works with the platforms your team already uses: |
| 56 | 66 | ||
| 57 | **Fix what you wrote, in place** | 67 | **Fix what you wrote, in place** |
| 58 | 68 | ||
| 69 | - Comment while closing an open pull request or reopening a closed one when the host offers that | ||
| 70 | action | ||
| 59 | - Rewrite a pull request's title and description from the review itself, in Markdown, with a | 71 | - Rewrite a pull request's title and description from the review itself, in Markdown, with a |
| 60 | preview before you save | 72 | preview before you save |
| 61 | - Rewrite your own comments the same way, wherever they are shown | 73 | - Rewrite your own comments the same way, wherever they are shown |
packages/contracts/src/pullRequest.test.ts+28-2| ... | @@ -13,6 +13,7 @@ import { | ... | @@ -13,6 +13,7 @@ import { |
| 13 | const decodeListResult = Schema.decodeUnknownSync(PullRequestListResult); | 13 | const decodeListResult = Schema.decodeUnknownSync(PullRequestListResult); |
| 14 | const decodeListInput = Schema.decodeUnknownSync(PullRequestListInput); | 14 | const decodeListInput = Schema.decodeUnknownSync(PullRequestListInput); |
| 15 | const decodeReviewerRequest = Schema.decodeUnknownSync(PullRequestReviewerRequestInput); | 15 | const decodeReviewerRequest = Schema.decodeUnknownSync(PullRequestReviewerRequestInput); |
| 16 | const decodeAction = Schema.decodeUnknownSync(PullRequestActionInput); | ||
| 16 | 17 | ||
| 17 | const LIST_RESULT: PullRequestListResult = { | 18 | const LIST_RESULT: PullRequestListResult = { |
| 18 | viewers: { "github.com": "bilal", "gitlab.com": "bilal.hassan" }, | 19 | viewers: { "github.com": "bilal", "gitlab.com": "bilal.hassan" }, |
| ... | @@ -158,7 +159,6 @@ describe("PullRequestReviewerRequestInput", () => { | ... | @@ -158,7 +159,6 @@ describe("PullRequestReviewerRequestInput", () => { |
| 158 | }); | 159 | }); |
| 159 | 160 | ||
| 160 | describe("updating a branch that has fallen behind its base", () => { | 161 | describe("updating a branch that has fallen behind its base", () => { |
| 161 | const decodeAction = Schema.decodeUnknownSync(PullRequestActionInput); | ||
| 162 | const ref = { projectId: "project-1", repository: "acme/web", number: 7 }; | 162 | const ref = { projectId: "project-1", repository: "acme/web", number: 7 }; |
| 163 | 163 | ||
| 164 | it("carries the way the branch should be brought up to date", () => { | 164 | it("carries the way the branch should be brought up to date", () => { |
| ... | @@ -182,7 +182,6 @@ describe("updating a branch that has fallen behind its base", () => { | ... | @@ -182,7 +182,6 @@ describe("updating a branch that has fallen behind its base", () => { |
| 182 | }); | 182 | }); |
| 183 | 183 | ||
| 184 | describe("leaving a merge for the host to make once it is ready", () => { | 184 | describe("leaving a merge for the host to make once it is ready", () => { |
| 185 | const decodeAction = Schema.decodeUnknownSync(PullRequestActionInput); | ||
| 186 | const ref = { projectId: "project-1", repository: "acme/web", number: 7 }; | 185 | const ref = { projectId: "project-1", repository: "acme/web", number: 7 }; |
| 187 | 186 | ||
| 188 | it("carries the strategy the deferred merge should use, as merging now does", () => { | 187 | it("carries the strategy the deferred merge should use, as merging now does", () => { |
| ... | @@ -196,6 +195,33 @@ describe("leaving a merge for the host to make once it is ready", () => { | ... | @@ -196,6 +195,33 @@ describe("leaving a merge for the host to make once it is ready", () => { |
| 196 | }); | 195 | }); |
| 197 | }); | 196 | }); |
| 198 | 197 | ||
| 198 | describe("reverting a merged pull request", () => { | ||
| 199 | it("carries the revert action without merge options", () => { | ||
| 200 | const action = decodeAction({ | ||
| 201 | projectId: "project-1", | ||
| 202 | repository: "acme/web", | ||
| 203 | number: 7, | ||
| 204 | action: "revert", | ||
| 205 | }); | ||
| 206 | |||
| 207 | expect(action.action).toBe("revert"); | ||
| 208 | expect(action.mergeMethod).toBeUndefined(); | ||
| 209 | }); | ||
| 210 | }); | ||
| 211 | |||
| 212 | describe("approving fork workflows", () => { | ||
| 213 | it("carries workflow approval as its own action", () => { | ||
| 214 | const action = decodeAction({ | ||
| 215 | projectId: "project-1", | ||
| 216 | repository: "acme/web", | ||
| 217 | number: 7, | ||
| 218 | action: "approve-workflows", | ||
| 219 | }); | ||
| 220 | |||
| 221 | expect(action.action).toBe("approve-workflows"); | ||
| 222 | }); | ||
| 223 | }); | ||
| 224 | |||
| 199 | describe("PullRequestCapabilities", () => { | 225 | describe("PullRequestCapabilities", () => { |
| 200 | const decodeCapabilities = Schema.decodeUnknownSync(PullRequestCapabilities); | 226 | const decodeCapabilities = Schema.decodeUnknownSync(PullRequestCapabilities); |
| 201 | const base = { | 227 | const base = { |
packages/contracts/src/pullRequest.ts+9| ... | @@ -93,6 +93,10 @@ export const PullRequestAction = Schema.Literals([ | ... | @@ -93,6 +93,10 @@ export const PullRequestAction = Schema.Literals([ |
| 93 | "enable-auto-merge", | 93 | "enable-auto-merge", |
| 94 | /** Take the standing instruction back, which leaves the change request where it was. */ | 94 | /** Take the standing instruction back, which leaves the change request where it was. */ |
| 95 | "disable-auto-merge", | 95 | "disable-auto-merge", |
| 96 | /** Open a new change request that reverses a merged one. */ | ||
| 97 | "revert", | ||
| 98 | /** Allow Actions workflows from a fork pull request to begin running. */ | ||
| 99 | "approve-workflows", | ||
| 96 | ]); | 100 | ]); |
| 97 | export type PullRequestAction = typeof PullRequestAction.Type; | 101 | export type PullRequestAction = typeof PullRequestAction.Type; |
| 98 | 102 | ||
| ... | @@ -131,6 +135,7 @@ export type PullRequestLabel = typeof PullRequestLabel.Type; | ... | @@ -131,6 +135,7 @@ export type PullRequestLabel = typeof PullRequestLabel.Type; |
| 131 | 135 | ||
| 132 | export const PullRequestCheckStatus = Schema.Literals([ | 136 | export const PullRequestCheckStatus = Schema.Literals([ |
| 133 | "pending", | 137 | "pending", |
| 138 | "action-required", | ||
| 134 | "success", | 139 | "success", |
| 135 | "failure", | 140 | "failure", |
| 136 | "skipped", | 141 | "skipped", |
| ... | @@ -709,6 +714,10 @@ export const PullRequestDetail = Schema.Struct({ | ... | @@ -709,6 +714,10 @@ export const PullRequestDetail = Schema.Struct({ |
| 709 | * arm something that is already armed, and a second arming is a write nobody asked for. | 714 | * arm something that is already armed, and a second arming is a write nobody asked for. |
| 710 | */ | 715 | */ |
| 711 | autoMergeEnabled: Schema.optional(Schema.Boolean), | 716 | autoMergeEnabled: Schema.optional(Schema.Boolean), |
| 717 | /** The strategy the host will use for an armed auto-merge, where it reports one. */ | ||
| 718 | autoMergeMethod: Schema.optional(PullRequestMergeMethod), | ||
| 719 | /** GitHub Actions runs on this head commit that are waiting for a maintainer's approval. */ | ||
| 720 | workflowApprovalsRequired: Schema.optional(NonNegativeInt), | ||
| 712 | }); | 721 | }); |
| 713 | export type PullRequestDetail = typeof PullRequestDetail.Type; | 722 | export type PullRequestDetail = typeof PullRequestDetail.Type; |
| 714 | 723 |