Merge remote-tracking branch 'g1t/main' into queue-methods
29 files+264−3913 files not counted0/29 viewed
| 3398 | 3398 | ||
| 3399 | 3399 | [[package]] | |
| 3400 | 3400 | name = "rustls" | |
| 3401 | − | version = "0.23.45" | |
| 3401 | + | version = "0.23.46" | |
| 3402 | 3402 | source = "registry+https://github.com/rust-lang/crates.io-index" | |
| 3403 | − | checksum = "0d41d731c7d2f962d1ccc364cec258de3c0e93b38c2fb3ba97ac74513048d634" | |
| 3403 | + | checksum = "48e13bd8c0e9365c43cfa5c9e8f9ad49d3c8444926c9aac819e0e4dc503c8fdf" | |
| 3404 | 3404 | dependencies = [ | |
| 3405 | 3405 | "aws-lc-rs", | |
| 3406 | 3406 | "log", |
| 275 | 275 | Op::Security(SecurityOp::GetDependencyGraph), | |
| 276 | 276 | Op::Security(SecurityOp::GetSbom), | |
| 277 | 277 | Op::Security(SecurityOp::CompareDependencies), | |
| 278 | + | Op::Security(SecurityOp::RerunDependencyReview), | |
| 278 | 279 | ], | |
| 279 | 280 | ), | |
| 280 | 281 | ( |
| 807 | 807 | } | |
| 808 | 808 | ||
| 809 | 809 | impl Op { | |
| 810 | − | pub const ALL: [Op; 402] = [ | |
| 810 | + | pub const ALL: [Op; 403] = [ | |
| 811 | 811 | Op::Whoami, | |
| 812 | 812 | Op::GetWorkspace, | |
| 813 | 813 | Op::CreateWorkspace, | |
| ⋯ | |||
| 1040 | 1040 | Op::Security(SecurityOp::GetDependencyGraph), | |
| 1041 | 1041 | Op::Security(SecurityOp::GetSbom), | |
| 1042 | 1042 | Op::Security(SecurityOp::CompareDependencies), | |
| 1043 | + | Op::Security(SecurityOp::RerunDependencyReview), | |
| 1043 | 1044 | Op::Security(SecurityOp::GetSettings), | |
| 1044 | 1045 | Op::Security(SecurityOp::UpdateSettings), | |
| 1045 | 1046 | Op::Security(SecurityOp::GetWorkspaceSettings), | |
| 9286 | 9286 | "deny_licenses": [] | |
| 9287 | 9287 | } | |
| 9288 | 9288 | }, | |
| 9289 | + | "rerun_dependency_review": { | |
| 9290 | + | "params": { | |
| 9291 | + | "owner": "flagon-io", | |
| 9292 | + | "name": "hello", | |
| 9293 | + | "number": 12 | |
| 9294 | + | }, | |
| 9295 | + | "response": { | |
| 9296 | + | "base": "4b1d7c0e9a2f6e3d8c5b0a7f4e1d2c3b4a5f6e7d", | |
| 9297 | + | "head": "9f2c4e1a7b3d5f60812a4c6e8b0d2f4a6c8e0b13", | |
| 9298 | + | "changes": [ | |
| 9299 | + | { | |
| 9300 | + | "change_type": "added", | |
| 9301 | + | "manifest": "package-lock.json", | |
| 9302 | + | "ecosystem": "npm", | |
| 9303 | + | "name": "lodash", | |
| 9304 | + | "version": "4.17.21", | |
| 9305 | + | "relationship": "direct", | |
| 9306 | + | "development": false, | |
| 9307 | + | "license": "MIT", | |
| 9308 | + | "purl": "pkg:npm/lodash@4.17.21", | |
| 9309 | + | "vulnerabilities": [], | |
| 9310 | + | "denied_license": false, | |
| 9311 | + | "failing": false | |
| 9312 | + | }, | |
| 9313 | + | { | |
| 9314 | + | "change_type": "removed", | |
| 9315 | + | "manifest": "package-lock.json", | |
| 9316 | + | "ecosystem": "npm", | |
| 9317 | + | "name": "lodash", | |
| 9318 | + | "version": "4.17.20", | |
| 9319 | + | "relationship": "direct", | |
| 9320 | + | "development": false, | |
| 9321 | + | "license": "MIT", | |
| 9322 | + | "purl": "pkg:npm/lodash@4.17.20", | |
| 9323 | + | "vulnerabilities": [], | |
| 9324 | + | "denied_license": false, | |
| 9325 | + | "failing": false | |
| 9326 | + | } | |
| 9327 | + | ], | |
| 9328 | + | "passed": true, | |
| 9329 | + | "headline": "2 dependency changes, none vulnerable", | |
| 9330 | + | "fail_on": "high", | |
| 9331 | + | "deny_licenses": [] | |
| 9332 | + | }, | |
| 9333 | + | "notes": "The pull request's one dependency review comment is changed in place; nothing new is posted when the review says the same." | |
| 9334 | + | }, | |
| 9289 | 9335 | "get_security_settings": { | |
| 9290 | 9336 | "params": { | |
| 9291 | 9337 | "owner": "flagon-io", |
| 332 | 332 | route("GET", "/repos/:owner/:name/dependency-graph", Op::Security(SecurityOp::GetDependencyGraph), &[]), | |
| 333 | 333 | route("GET", "/repos/:owner/:name/dependency-graph/sbom", Op::Security(SecurityOp::GetSbom), &[]), | |
| 334 | 334 | route("GET", "/repos/:owner/:name/dependency-graph/compare/:basehead", Op::Security(SecurityOp::CompareDependencies), &[]), | |
| 335 | + | route("POST", "/repos/:owner/:name/pulls/:number/dependency-review/rerun", Op::Security(SecurityOp::RerunDependencyReview), &[]), | |
| 335 | 336 | route("GET", "/repos/:owner/:name/security/settings", Op::Security(SecurityOp::GetSettings), &[]), | |
| 336 | 337 | route("PATCH", "/repos/:owner/:name/security/settings", Op::Security(SecurityOp::UpdateSettings), &[]), | |
| 337 | 338 | route("GET", "/workspaces/:workspace/security/settings", Op::Security(SecurityOp::GetWorkspaceSettings), &[]), |
| 54 | 54 | GetDependencyGraph, | |
| 55 | 55 | GetSbom, | |
| 56 | 56 | CompareDependencies, | |
| 57 | + | RerunDependencyReview, | |
| 57 | 58 | GetSettings, | |
| 58 | 59 | UpdateSettings, | |
| 59 | 60 | GetWorkspaceSettings, | |
| ⋯ | |||
| 73 | 74 | /// Every one: `Op::ALL` lists each as `Op::Security(…)`, which a test | |
| 74 | 75 | /// checks against this. | |
| 75 | 76 | #[cfg(test)] | |
| 76 | − | pub const ALL: [SecurityOp; 38] = [ | |
| 77 | + | pub const ALL: [SecurityOp; 39] = [ | |
| 77 | 78 | SecurityOp::ListSecretAlerts, | |
| 78 | 79 | SecurityOp::GetSecretAlert, | |
| 79 | 80 | SecurityOp::UpdateSecretAlert, | |
| ⋯ | |||
| 99 | 100 | SecurityOp::GetDependencyGraph, | |
| 100 | 101 | SecurityOp::GetSbom, | |
| 101 | 102 | SecurityOp::CompareDependencies, | |
| 103 | + | SecurityOp::RerunDependencyReview, | |
| 102 | 104 | SecurityOp::GetSettings, | |
| 103 | 105 | SecurityOp::UpdateSettings, | |
| 104 | 106 | SecurityOp::GetWorkspaceSettings, | |
| ⋯ | |||
| 141 | 143 | SecurityOp::GetDependencyGraph => "get_dependency_graph", | |
| 142 | 144 | SecurityOp::GetSbom => "get_sbom", | |
| 143 | 145 | SecurityOp::CompareDependencies => "compare_dependencies", | |
| 146 | + | SecurityOp::RerunDependencyReview => "rerun_dependency_review", | |
| 144 | 147 | SecurityOp::GetSettings => "get_security_settings", | |
| 145 | 148 | SecurityOp::UpdateSettings => "update_security_settings", | |
| 146 | 149 | SecurityOp::GetWorkspaceSettings => "get_workspace_security_settings", | |
| ⋯ | |||
| 185 | 188 | SecurityOp::GetDependencyGraph => "Get the dependency graph", | |
| 186 | 189 | SecurityOp::GetSbom => "Export an SBOM", | |
| 187 | 190 | SecurityOp::CompareDependencies => "Compare dependencies", | |
| 191 | + | SecurityOp::RerunDependencyReview => "Re-run dependency review", | |
| 188 | 192 | SecurityOp::GetSettings => "Get a repository's security settings", | |
| 189 | 193 | SecurityOp::UpdateSettings => "Update a repository's security settings", | |
| 190 | 194 | SecurityOp::GetWorkspaceSettings => "Get a workspace's security settings", | |
| ⋯ | |||
| 228 | 232 | SecurityOp::GetDependencyGraph => "Get the dependency graph: every package the lockfiles on the default branch resolve, per lockfile, with whether it is direct or transitive (where the lockfile says), for development, its license when recorded, its package URL and its open vulnerability alerts.", | |
| 229 | 233 | SecurityOp::GetSbom => "Export the dependency graph as an SPDX 2.3 JSON document, in sbom. Every package is named by its package URL.", | |
| 230 | 234 | SecurityOp::CompareDependencies => "Compare the dependencies at two commits, branches or tags (basehead, as base...head): what was added and removed per lockfile, with the known vulnerabilities of what was added and whether it passes the repository's dependency review policy. Needs the g1t plan on a private repository.", | |
| 235 | + | SecurityOp::RerunDependencyReview => "Run the Dependency review check again on an open pull request's head, with the advisories known now: its status is set again and its one comment on the pull request is changed in place (or added, when it has dependency changes to show and none is there yet). Returns the review. g1t reviews a pull request by itself when it opens and when its head moves, carrying the result over when the head's lockfile changes are the ones reviewed before. Takes the Write role. Needs the g1t plan on a private repository.", | |
| 231 | 236 | SecurityOp::GetSettings => "Get a repository's security settings: when the Code scanning check fails, dependency review and its policy, whether secrets in the history open alerts, whether it takes private vulnerability reports, its workspace's settings, and whether the paid features are on for it.", | |
| 232 | 237 | SecurityOp::UpdateSettings => "Change a repository's security settings: code_scanning_gate (none, errors, critical, high, medium or any), dependency_review, review_fail_on (critical, high, medium, low or none), review_deny_licenses (SPDX ids), review_comment, history_alerts (whether secrets found in the history open alerts; push protection stays on) and private_vulnerability_reporting. Takes the Admin role, or a security manager of the workspace. Require the Code scanning and Dependency review checks in branch protection to gate merges on them.", | |
| 233 | 238 | SecurityOp::GetWorkspaceSettings => "Get a workspace's security settings (delegated bypass, validity checks) and whether it has Security and quality, which comes with the g1t plan.", | |
| ⋯ | |||
| 428 | 433 | &["repo", "id", "state"], | |
| 429 | 434 | ), | |
| 430 | 435 | SecurityOp::GetDependencyGraph | SecurityOp::GetSbom | SecurityOp::GetSettings => (json!({ "repo": repo() }), &["repo"]), | |
| 436 | + | SecurityOp::RerunDependencyReview => ( | |
| 437 | + | json!({ "repo": repo(), "number": { "type": "integer", "description": "The pull request's number." } }), | |
| 438 | + | &["repo", "number"], | |
| 439 | + | ), | |
| 431 | 440 | SecurityOp::CompareDependencies => ( | |
| 432 | 441 | json!({ | |
| 433 | 442 | "repo": repo(), | |
| ⋯ | |||
| 1037 | 1046 | }; | |
| 1038 | 1047 | pass(services, "dependency_review", &DependencyReviewArgs { viewer: viewer.clone(), repo, base, head }).await | |
| 1039 | 1048 | } | |
| 1049 | + | SecurityOp::RerunDependencyReview => { | |
| 1050 | + | let Some(repo) = repo else { return failed(FailureCode::Invalid, &need_repo()) }; | |
| 1051 | + | let Some(number) = whole(input, "number").filter(|number| *number > 0) else { | |
| 1052 | + | return failed(FailureCode::Invalid, "number is the pull request's number."); | |
| 1053 | + | }; | |
| 1054 | + | pass(services, "rerun_dependency_review", &PullScanningArgs { viewer: viewer.clone(), repo, number }).await | |
| 1055 | + | } | |
| 1040 | 1056 | SecurityOp::GetSettings => { | |
| 1041 | 1057 | let Some(repo) = repo else { return failed(FailureCode::Invalid, &need_repo()) }; | |
| 1042 | 1058 | pass(services, "security_settings", &SecuritySettingsArgs { viewer: viewer.clone(), repo }).await | |
| ⋯ | |||
| 1241 | 1257 | } | |
| 1242 | 1258 | assert_eq!(scope_for("get_sbom"), Some(Scope::SecurityRead)); | |
| 1243 | 1259 | assert_eq!(scope_for("upload_sarif"), Some(Scope::SecurityWrite)); | |
| 1260 | + | assert_eq!(scope_for("rerun_dependency_review"), Some(Scope::SecurityWrite)); | |
| 1244 | 1261 | // No run decides about security. | |
| 1245 | 1262 | for name in ["bypass_push_protection", "review_bypass_request", "update_security_settings"] { | |
| 1246 | 1263 | assert!(g1t_contracts::credentials::NEVER.contains(&name), "{name}"); | |
| 515 | 515 | a("dependency_graph", Op::Security(SecurityOp::GetDependencyGraph), "Every package the lockfiles resolve, direct or transitive"), | |
| 516 | 516 | a("sbom", Op::Security(SecurityOp::GetSbom), "The dependency graph as an SPDX 2.3 document"), | |
| 517 | 517 | a("compare_dependencies", Op::Security(SecurityOp::CompareDependencies), "What changes in dependencies between base...head"), | |
| 518 | + | a("rerun_dependency_review", Op::Security(SecurityOp::RerunDependencyReview), "Run a pull request's Dependency review check again"), | |
| 518 | 519 | a("settings", Op::Security(SecurityOp::GetSettings), "A repository's security settings"), | |
| 519 | 520 | a("update_settings", Op::Security(SecurityOp::UpdateSettings), "Change when checks fail and dependency review's policy"), | |
| 520 | 521 | a("workspace_settings", Op::Security(SecurityOp::GetWorkspaceSettings), "A workspace's delegated bypass and validity checks"), |
| 713 | 713 | checksum = "d045de693cb712d0b22c6a64be5b953f67b3ce00ab5ad3dd5d8b441886ab8e1a" | |
| 714 | 714 | dependencies = [ | |
| 715 | 715 | "quote", | |
| 716 | − | "syn 3.0.7", | |
| 716 | + | "syn 2.0.119", | |
| 717 | 717 | ] | |
| 718 | 718 | ||
| 719 | 719 | [[package]] | |
| ⋯ | |||
| 3487 | 3487 | ||
| 3488 | 3488 | [[package]] | |
| 3489 | 3489 | name = "rustls" | |
| 3490 | − | version = "0.23.45" | |
| 3490 | + | version = "0.23.46" | |
| 3491 | 3491 | source = "registry+https://github.com/rust-lang/crates.io-index" | |
| 3492 | − | checksum = "0d41d731c7d2f962d1ccc364cec258de3c0e93b38c2fb3ba97ac74513048d634" | |
| 3492 | + | checksum = "48e13bd8c0e9365c43cfa5c9e8f9ad49d3c8444926c9aac819e0e4dc503c8fdf" | |
| 3493 | 3493 | dependencies = [ | |
| 3494 | 3494 | "once_cell", | |
| 3495 | 3495 | "ring", | |
| 527 | 527 | ||
| 528 | 528 | A pull request that only has fallen behind its target, without conflicts, | |
| 529 | 529 | still merges: merging brings it up to date first, unless the repository | |
| 530 | − | requires pull requests to be up to date. | |
| 530 | + | requires pull requests to be up to date. Bringing it up to date makes a new | |
| 531 | + | commit, so when the target branch requires checks, the merge box says | |
| 532 | + | **Merging once checks pass** and the pull request lands when the required | |
| 533 | + | checks pass on that commit, with nothing more to press. If one of them fails | |
| 534 | + | there, it is not merged: the pull request says which check failed, and you | |
| 535 | + | push a fix and merge again. With no required checks, it lands as soon as it | |
| 536 | + | is up to date. | |
| 537 | + | ||
| 538 | + | If the repository does not answer in time while merging, the merge box says | |
| 539 | + | g1t couldn't reach the repository. Merge again: if the first merge went | |
| 540 | + | through meanwhile, merging again records it rather than merging twice. | |
| 531 | 541 | ||
| 532 | 542 | ## Keeping it up to date | |
| 533 | 543 |
| 80 | 80 | `g1t.sh/<owner>/<project>/security/pulls/<number>`. Require **Dependency | |
| 81 | 81 | review** under **Settings → Branches** to block merges on it. | |
| 82 | 82 | ||
| 83 | + | A pull request has one review comment. When a later review says something | |
| 84 | + | new, that comment is changed in place; when it says the same, the comment | |
| 85 | + | is left alone. A push whose lockfile changes are the ones already reviewed, | |
| 86 | + | such as catching the branch up with its base, is not reviewed again: the | |
| 87 | + | check's result carries over to the new head. | |
| 88 | + | ||
| 89 | + | Advisories are published every day, so a review can be made again: **Re-run | |
| 90 | + | dependency review** in the pull request's checks, or **Re-run** on its | |
| 91 | + | Security checks page, takes the Write role. It updates the check and the | |
| 92 | + | same comment. | |
| 93 | + | ||
| 83 | 94 | In the repository's **Security settings**: | |
| 84 | 95 | ||
| 85 | 96 | | Setting | Default | | | |
| ⋯ | |||
| 115 | 126 | | `GET /repos/{owner}/{name}/dependency-graph` | The graph, per lockfile. | `security:read` | | |
| 116 | 127 | | `GET /repos/{owner}/{name}/dependency-graph/sbom` | The SPDX document, in `sbom`. | `security:read` | | |
| 117 | 128 | | `GET /repos/{owner}/{name}/dependency-graph/compare/{basehead}` | What changed between two commits, and whether it passes review. | `security:read` | | |
| 129 | + | | `POST /repos/{owner}/{name}/pulls/{number}/dependency-review/rerun` | Runs a pull request's Dependency review check again and returns the review. | `security:write` | | |
| 118 | 130 | | `GET /repos/{owner}/{name}/vulnerability-alerts` | Lists alerts; filter with `state`, `severity`, `ecosystem`, `package`. | `security:read` | | |
| 119 | 131 | | `GET /workspaces/{workspace}/vulnerability-alerts` | The same across a workspace. | `security:read` | | |
| 120 | 132 | | `GET /repos/{owner}/{name}/vulnerability-alerts/{id}` | One alert. | `security:read` | | |
| 121 | 133 | | `PATCH /repos/{owner}/{name}/vulnerability-alerts/{id}` | `state` `dismissed` with `reason` and `comment`, or `open`. | `security:write` | | |
| 122 | 134 | ||
| 123 | 135 | Over MCP, the [`security` tool](/reference/mcp/#security) has | |
| 124 | − | `dependency_graph`, `sbom`, `compare_dependencies`, `vulnerability_alerts`, | |
| 136 | + | `dependency_graph`, `sbom`, `compare_dependencies`, `rerun_dependency_review`, `vulnerability_alerts`, | |
| 125 | 137 | `vulnerability_alert` and `update_vulnerability_alert`. Webhooks: | |
| 126 | 138 | `vulnerability_alert.created`, `.fixed`, `.dismissed` and `.reopened`. | |
| 618 | 618 | | [`dependency_graph`](/reference/api/supply-chain/get-dependency-graph/) | Every package the lockfiles resolve, direct or transitive, with licenses. | `repo` | `security:read` | | |
| 619 | 619 | | [`sbom`](/reference/api/supply-chain/get-sbom/) | The dependency graph as an SPDX 2.3 document, in `sbom`. | `repo` | `security:read` | | |
| 620 | 620 | | [`compare_dependencies`](/reference/api/supply-chain/compare-dependencies/) | What changes between `basehead` (`base...head`), and whether it passes dependency review. | `repo`, `basehead` | `security:read` | | |
| 621 | + | | [`rerun_dependency_review`](/reference/api/supply-chain/rerun-dependency-review/) | Runs a pull request's Dependency review check again; its one comment changes in place. Write role. | `repo`, `number` | `security:write` | | |
| 621 | 622 | | [`settings`](/reference/api/security-settings/get-security-settings/) | A repository's security settings, and whether the paid features are on. | `repo` | `security:read` | | |
| 622 | 623 | | [`update_settings`](/reference/api/security-settings/update-security-settings/) | Change `code_scanning_gate`, `dependency_review`, `review_fail_on`, `review_deny_licenses`, `review_comment`. Admin role, or a security manager. | `repo` | `security:write` | | |
| 623 | 624 | | [`workspace_settings`](/reference/api/security-settings/get-workspace-security-settings/) | A workspace's delegated bypass and validity checks. | `workspace` | `security:read` | |
Binary or large file; its contents are not shown.
| 219 | 219 | ); | |
| 220 | 220 | } | |
| 221 | 221 | ||
| 222 | − | /** Where one rule's review stands, in a few words. */ | |
| 223 | − | function ReviewStatus({ review }: { review: OwnerReview }) { | |
| 222 | + | /** | |
| 223 | + | * Where one rule's review stands, in a few words. When the ruleset does not | |
| 224 | + | * require code owners' approval (`required` false), a rule nobody approved | |
| 225 | + | * yet is said plainly, not as something merging waits for. | |
| 226 | + | */ | |
| 227 | + | function ReviewStatus({ review, required }: { review: OwnerReview; required: boolean }) { | |
| 224 | 228 | const names = (list: string[]) => list.join(", "); | |
| 225 | 229 | if (review.changes_requested_by.length > 0) { | |
| 226 | 230 | return ( | |
| ⋯ | |||
| 246 | 250 | </span> | |
| 247 | 251 | ); | |
| 248 | 252 | } | |
| 253 | + | if (!required) { | |
| 254 | + | return ( | |
| 255 | + | <span className="text-xs text-faint"> | |
| 256 | + | {review.approved_by.length > 0 ? `Approved by ${names(review.approved_by)}` : "No approval yet"} | |
| 257 | + | </span> | |
| 258 | + | ); | |
| 259 | + | } | |
| 249 | 260 | const more = Math.max(review.required - review.approved_by.length, 1); | |
| 250 | 261 | return ( | |
| 251 | 262 | <span className="flex items-center gap-1 text-xs text-warn"> | |
| ⋯ | |||
| 257 | 268 | } | |
| 258 | 269 | ||
| 259 | 270 | /** One rule: its section, pattern and line, its owners, the files it covers, and where its review stands. */ | |
| 260 | − | function OwnerReviewRow({ review, fileHref }: { review: OwnerReview; fileHref: (line: number) => string }) { | |
| 271 | + | function OwnerReviewRow({ | |
| 272 | + | review, | |
| 273 | + | required, | |
| 274 | + | fileHref, | |
| 275 | + | }: { | |
| 276 | + | review: OwnerReview; | |
| 277 | + | /** Whether the ruleset requires code owners' approval to merge. */ | |
| 278 | + | required: boolean; | |
| 279 | + | fileHref: (line: number) => string; | |
| 280 | + | }) { | |
| 261 | 281 | const many = review.files.length > 3; | |
| 262 | 282 | const files = ( | |
| 263 | 283 | <ul className="mt-1 space-y-0.5"> | |
| ⋯ | |||
| 277 | 297 | line {review.line} | |
| 278 | 298 | </Link> | |
| 279 | 299 | <span className="ml-auto"> | |
| 280 | − | <ReviewStatus review={review} /> | |
| 300 | + | <ReviewStatus review={review} required={required} /> | |
| 281 | 301 | </span> | |
| 282 | 302 | </div> | |
| 283 | 303 | <p className="mt-1 flex flex-wrap items-center gap-x-2 gap-y-0.5 text-xs text-muted"> | |
| ⋯ | |||
| 343 | 363 | ) : ( | |
| 344 | 364 | <ul className="divide-y divide-line"> | |
| 345 | 365 | {owners.reviews.map((review) => ( | |
| 346 | − | <OwnerReviewRow key={`${review.section ?? ""}:${review.line}`} review={review} fileHref={fileHref} /> | |
| 366 | + | <OwnerReviewRow key={`${review.section ?? ""}:${review.line}`} review={review} required={owners.required} fileHref={fileHref} /> | |
| 347 | 367 | ))} | |
| 348 | 368 | </ul> | |
| 349 | 369 | )} | |
| 21 | 21 | import { useMemo, useState } from "react"; | |
| 22 | 22 | import { Form, Link } from "react-router"; | |
| 23 | 23 | ||
| 24 | − | import type { CheckResult, CheckRun, CommitStatus, Job, Mergeable, Pull, RequiredCheck } from "@g1t/contracts"; | |
| 24 | + | import { | |
| 25 | + | type CheckResult, | |
| 26 | + | type CheckRun, | |
| 27 | + | type CommitStatus, | |
| 28 | + | DEPENDENCY_REVIEW_CHECK, | |
| 29 | + | type Job, | |
| 30 | + | type Mergeable, | |
| 31 | + | type Pull, | |
| 32 | + | type RequiredCheck, | |
| 33 | + | } from "@g1t/contracts"; | |
| 25 | 34 | ||
| 26 | 35 | import { useAddresses } from "../lib/addresses"; | |
| 27 | 36 | import { duration } from "./actions"; | |
| ⋯ | |||
| 406 | 415 | pull.fork ? `/${pull.fork.namespace}/${pull.fork.name}/commit/${sha}` : `${base}/commit/${sha}`; | |
| 407 | 416 | const older = run != null && pull.headCommit != null && run.headCommit !== pull.headCommit; | |
| 408 | 417 | const failedRuns = statuses.filter((status) => statusStanding(status) === "failed" && runIdOf(status)); | |
| 418 | + | // g1t's own Dependency review check, which anyone who may start runs can | |
| 419 | + | // have made again (after an advisory is published, say). | |
| 420 | + | const dependencyReview = statuses.some((status) => status.context === DEPENDENCY_REVIEW_CHECK && status.source === "security"); | |
| 409 | 421 | ||
| 410 | 422 | return ( | |
| 411 | 423 | <div aria-busy={loading || undefined}> | |
| ⋯ | |||
| 458 | 470 | </Hint> | |
| 459 | 471 | </Form> | |
| 460 | 472 | ))} | |
| 473 | + | {canRerunWorkflows && dependencyReview && !settled && ( | |
| 474 | + | <Form method="post"> | |
| 475 | + | <input type="hidden" name="action" value="rerun-dependency-review" /> | |
| 476 | + | <Hint label="Review its dependency changes again, with the advisories known now"> | |
| 477 | + | <SubmitButton variant="outline" match={{ action: "rerun-dependency-review" }} pending="Re-running…"> | |
| 478 | + | <RotateCw size={13} /> | |
| 479 | + | Re-run dependency review | |
| 480 | + | </SubmitButton> | |
| 481 | + | </Hint> | |
| 482 | + | </Form> | |
| 483 | + | )} | |
| 461 | 484 | </div> | |
| 462 | 485 | </div> | |
| 463 | 486 | {error && ( | |
| 402 | 402 | ); | |
| 403 | 403 | } | |
| 404 | 404 | ||
| 405 | − | /** Something that happened, as one line on the conversation's rail. */ | |
| 406 | − | function TimelineEvent({ comment, base, filesUrl }: { comment: Comment; base?: string; filesUrl?: string }) { | |
| 405 | + | /** | |
| 406 | + | * One line on the conversation's rail: a mark on the rail (a dot unless | |
| 407 | + | * `mark` says otherwise), who, what and when. | |
| 408 | + | */ | |
| 409 | + | function TimelineRow({ comment, mark, children }: { comment: Comment; mark?: ReactNode; children: ReactNode }) { | |
| 407 | 410 | return ( | |
| 408 | 411 | <div className="flex items-center gap-3 text-sm text-muted"> | |
| 409 | 412 | <span className="hidden w-8 shrink-0 justify-center sm:flex"> | |
| 410 | − | <span className="size-2 rounded-full border border-line-strong bg-bg" /> | |
| 413 | + | {mark ?? <span className="size-2 rounded-full border border-line-strong bg-bg" />} | |
| 411 | 414 | </span> | |
| 412 | 415 | <span className="flex min-w-0 flex-wrap items-center gap-x-1.5 gap-y-0.5"> | |
| 413 | − | <Avatar name={comment.author.username} size={16} /> | |
| 414 | − | <PersonLink name={comment.author.username} className="font-medium text-fg hover:underline" /> | |
| 415 | − | <span> | |
| 416 | − | <WithReferences text={comment.body} base={base} filesUrl={filesUrl} /> | |
| 417 | − | </span> | |
| 416 | + | {comment.agent ? ( | |
| 417 | + | <> | |
| 418 | + | <Avatar name={comment.agent.displayName} agent size={16} /> | |
| 419 | + | <AgentName agent={comment.agent} /> | |
| 420 | + | </> | |
| 421 | + | ) : ( | |
| 422 | + | <> | |
| 423 | + | <Avatar name={comment.author.username} size={16} /> | |
| 424 | + | <PersonLink name={comment.author.username} className="font-medium text-fg hover:underline" /> | |
| 425 | + | </> | |
| 426 | + | )} | |
| 427 | + | {children} | |
| 428 | + | {comment.actingFor && ( | |
| 429 | + | <span className="text-xs text-faint"> | |
| 430 | + | on behalf of{" "} | |
| 431 | + | <PersonLink name={comment.actingFor.username} className="hover:text-fg hover:underline"> | |
| 432 | + | @{comment.actingFor.username} | |
| 433 | + | </PersonLink> | |
| 434 | + | </span> | |
| 435 | + | )} | |
| 418 | 436 | <span className="text-faint"> | |
| 419 | 437 | · <TimeAgo at={comment.createdAt} /> | |
| 420 | 438 | </span> | |
| ⋯ | |||
| 423 | 441 | ); | |
| 424 | 442 | } | |
| 425 | 443 | ||
| 444 | + | /** Something that happened, as one line on the conversation's rail. */ | |
| 445 | + | function TimelineEvent({ comment, base, filesUrl }: { comment: Comment; base?: string; filesUrl?: string }) { | |
| 446 | + | return ( | |
| 447 | + | <TimelineRow comment={comment}> | |
| 448 | + | <span> | |
| 449 | + | <WithReferences text={comment.body} base={base} filesUrl={filesUrl} /> | |
| 450 | + | </span> | |
| 451 | + | </TimelineRow> | |
| 452 | + | ); | |
| 453 | + | } | |
| 454 | + | ||
| 455 | + | /** A review's verdict, approved or changes requested, in its own colour. */ | |
| 456 | + | function Verdict({ verdict }: { verdict: keyof typeof VERDICTS }) { | |
| 457 | + | const { label, style } = VERDICTS[verdict]; | |
| 458 | + | return ( | |
| 459 | + | <span className={`flex items-center gap-1 font-medium ${style}`}> | |
| 460 | + | {verdict === "approve" ? <CircleCheck size={14} /> : <CircleSlash size={14} />} | |
| 461 | + | {label} | |
| 462 | + | </span> | |
| 463 | + | ); | |
| 464 | + | } | |
| 465 | + | ||
| 466 | + | /** | |
| 467 | + | * A review that gave a verdict and said nothing else: one line on the rail, | |
| 468 | + | * not a comment with an empty body. There is nothing to edit, and a | |
| 469 | + | * verdict is not deleted, so it has no actions. | |
| 470 | + | */ | |
| 471 | + | function TimelineVerdict({ comment, verdict }: { comment: Comment; verdict: keyof typeof VERDICTS }) { | |
| 472 | + | return ( | |
| 473 | + | <TimelineRow | |
| 474 | + | comment={comment} | |
| 475 | + | mark={ | |
| 476 | + | verdict === "approve" ? ( | |
| 477 | + | <CircleCheck size={16} className="text-success" /> | |
| 478 | + | ) : ( | |
| 479 | + | <CircleSlash size={16} className="text-danger" /> | |
| 480 | + | ) | |
| 481 | + | } | |
| 482 | + | > | |
| 483 | + | <span className={VERDICTS[verdict].style}>{VERDICTS[verdict].label}</span> | |
| 484 | + | {comment.advisory && <AdvisoryChip />} | |
| 485 | + | </TimelineRow> | |
| 486 | + | ); | |
| 487 | + | } | |
| 488 | + | ||
| 426 | 489 | /** Edit and Delete, under a comment the viewer may change. */ | |
| 427 | 490 | function CommentActions({ | |
| 428 | 491 | comment, | |
| ⋯ | |||
| 521 | 584 | if (comment.kind === "event") { | |
| 522 | 585 | return <TimelineEvent key={comment.id} comment={comment} base={base} filesUrl={review?.changesUrl} />; | |
| 523 | 586 | } | |
| 524 | − | const verdict = comment.verdict && VERDICTS[comment.verdict]; | |
| 587 | + | const verdict = comment.verdict; | |
| 588 | + | // A verdict with nothing said and no line it is about is one line, not a card. | |
| 589 | + | if (verdict && !comment.body.trim() && !comment.path) { | |
| 590 | + | return <TimelineVerdict key={comment.id} comment={comment} verdict={verdict} />; | |
| 591 | + | } | |
| 525 | 592 | const can = mayChangeComment(comment, viewerId, canModerate); | |
| 526 | 593 | return ( | |
| 527 | 594 | <TimelineItem | |
| ⋯ | |||
| 537 | 604 | action={ | |
| 538 | 605 | <> | |
| 539 | 606 | {verdict ? ( | |
| 540 | − | <span className={`flex items-center gap-1 font-medium ${verdict.style}`}> | |
| 541 | − | {comment.verdict === "approve" ? ( | |
| 542 | − | <CircleCheck size={14} /> | |
| 543 | − | ) : ( | |
| 544 | − | <CircleSlash size={14} /> | |
| 545 | − | )} | |
| 546 | − | {verdict.label} | |
| 547 | − | </span> | |
| 607 | + | <Verdict verdict={verdict} /> | |
| 548 | 608 | ) : ( | |
| 549 | 609 | <span>{comment.advisory ? "reviewed" : "commented"}</span> | |
| 550 | 610 | )} | |
| 38 | 38 | type RepoSettings, | |
| 39 | 39 | type SessionEntry, | |
| 40 | 40 | type Viewer, | |
| 41 | + | fail, | |
| 41 | 42 | pullComparison, | |
| 42 | 43 | workOwner, | |
| 43 | 44 | } from "@g1t/contracts"; | |
| ⋯ | |||
| 89 | 90 | import { ViolationList } from "../../components/rules"; | |
| 90 | 91 | import { PullCodeOwnersPanel, TeamReviewer } from "../../components/codeowners"; | |
| 91 | 92 | import { notFound } from "../../lib/not-found.server"; | |
| 92 | − | import { actions, deployments, identity, inbox, repos, work } from "../../lib/services.server"; | |
| 93 | + | import { actions, deployments, identity, inbox, repos, securitySuite, work } from "../../lib/services.server"; | |
| 93 | 94 | import { commitChecksFor } from "../../lib/commit-checks.server"; | |
| 94 | 95 | import { CommitChecksBadge } from "../../components/commit-checks"; | |
| 95 | 96 | import { DeployLink } from "../../components/deploy"; | |
| ⋯ | |||
| 394 | 395 | const PULL_NEEDS: Record<string, Capability> = { | |
| 395 | 396 | "rerun-workflow": "run", | |
| 396 | 397 | "rerun-failed": "run", | |
| 398 | + | "rerun-dependency-review": "run", | |
| 397 | 399 | merge: "merge", | |
| 398 | 400 | unqueue: "merge", | |
| 399 | 401 | milestone: "triage", | |
| ⋯ | |||
| 487 | 489 | const rerun = await actions.rerun(user, path, String(form.get("run") ?? ""), true); | |
| 488 | 490 | return rerun.ok ? null : { error: rerun.error.message, action }; | |
| 489 | 491 | } | |
| 492 | + | // The Dependency review check, made again on its head: its comment | |
| 493 | + | // changes in place. Who may is the security service's call. | |
| 494 | + | if (action === "rerun-dependency-review") { | |
| 495 | + | const rerun = await securitySuite.rerunDependencyReview(user, path, number); | |
| 496 | + | return rerun.ok ? null : { error: rerun.error.message, action }; | |
| 497 | + | } | |
| 490 | 498 | // Every failed run on its head, run again: Mission control's quick action. | |
| 491 | 499 | if (action === "rerun-failed") { | |
| 492 | 500 | const found = await work.getPull(path, number, user); | |
| ⋯ | |||
| 516 | 524 | } | |
| 517 | 525 | const result = | |
| 518 | 526 | action === "merge" | |
| 519 | − | ? await work.mergePull(user, path, number, mergeOptions(form)) | |
| 527 | + | ? // A merge that did not answer in time (lib/deadline.ts) or broke | |
| 528 | + | // off says so in the merge box, not as an error page: trying again | |
| 529 | + | // is safe, as a merge that went through meanwhile is recognised. | |
| 530 | + | await work | |
| 531 | + | .mergePull(user, path, number, mergeOptions(form)) | |
| 532 | + | .catch(() => fail("unavailable", "g1t couldn't reach the repository to merge this. Try again in a moment.")) | |
| 520 | 533 | : action === "unqueue" | |
| 521 | 534 | ? await work.removeFromQueue(user, path, number) | |
| 522 | 535 | : action === "close" | |
| ⋯ | |||
| 688 | 701 | ); | |
| 689 | 702 | } | |
| 690 | 703 | ||
| 704 | + | /** The required checks a caught-up merge still waits for: "CI passes", "CI and Deploy pass". */ | |
| 705 | + | function awaitedChecks(checks: { name: string; state: string }[]): string { | |
| 706 | + | const names = checks.filter((check) => check.state !== "success").map((check) => check.name); | |
| 707 | + | if (names.length === 0) return "the required checks pass"; | |
| 708 | + | if (names.length === 1) return `the required check ${names[0]} passes`; | |
| 709 | + | return `the required checks ${names.slice(0, -1).join(", ")} and ${names[names.length - 1]} pass`; | |
| 710 | + | } | |
| 711 | + | ||
| 691 | 712 | export default function PullPage({ loaderData, actionData, params }: Route.ComponentProps) { | |
| 692 | 713 | const { | |
| 693 | 714 | pull, | |
| ⋯ | |||
| 753 | 774 | canRerunWorkflows={canMerge} | |
| 754 | 775 | settingsUrl={canProtect ? `${base}/settings/branches` : null} | |
| 755 | 776 | conflicting={mergeable === "conflicting" && !noChecks} | |
| 756 | − | error={actionData?.action === "rerun-failed" || actionData?.action === "rerun-workflow" ? actionData.error : null} | |
| 777 | + | error={actionData?.action === "rerun-failed" || actionData?.action === "rerun-workflow" || actionData?.action === "rerun-dependency-review" | |
| 778 | + | ? actionData.error | |
| 779 | + | : null} | |
| 757 | 780 | loading={loading} | |
| 758 | 781 | /> | |
| 759 | 782 | ); | |
| ⋯ | |||
| 1267 | 1290 | </StatusRow> | |
| 1268 | 1291 | )} | |
| 1269 | 1292 | {landing ? ( | |
| 1293 | + | // Brought up to date first, then landed once the | |
| 1294 | + | // required checks pass on that new commit. | |
| 1270 | 1295 | <StatusRow | |
| 1271 | 1296 | icon={<Loader size={16} className="animate-spin text-accent" />} | |
| 1272 | − | title="Merging" | |
| 1297 | + | title={!behind && unchecked ? "Merging once checks pass" : "Merging"} | |
| 1273 | 1298 | > | |
| 1274 | − | {defaultBranch} has moved, so it is being brought up to date first. It | |
| 1275 | − | lands as soon as that is done. | |
| 1299 | + | {behind | |
| 1300 | + | ? `${defaultBranch} has moved, so it is being brought up to date first. ${ | |
| 1301 | + | requiredChecks.length > 0 | |
| 1302 | + | ? "It lands once the required checks pass on that commit." | |
| 1303 | + | : "It lands as soon as that is done." | |
| 1304 | + | }` | |
| 1305 | + | : unchecked | |
| 1306 | + | ? `Brought up to date with ${defaultBranch}. It lands once ${awaitedChecks(requiredChecks)} on that commit.` | |
| 1307 | + | : `Brought up to date with ${defaultBranch}. Landing now.`} | |
| 1276 | 1308 | </StatusRow> | |
| 1277 | 1309 | ) : conflicting ? ( | |
| 1278 | 1310 | <ConflictsSection | |
| ⋯ | |||
| 1449 | 1481 | </Form> | |
| 1450 | 1482 | )} | |
| 1451 | 1483 | {actionData && | |
| 1452 | − | !["merge", "comment", "rerun-failed", "rerun-workflow", "update", "reviewers", "assign", "edit-comment", "delete-comment", "auto-merge-on", "auto-merge-off", "delete-branch", "restore-branch"].includes( | |
| 1484 | + | !["merge", "comment", "rerun-failed", "rerun-workflow", "rerun-dependency-review", "update", "reviewers", "assign", "edit-comment", "delete-comment", "auto-merge-on", "auto-merge-off", "delete-branch", "restore-branch"].includes( | |
| 1453 | 1485 | String(actionData.action), | |
| 1454 | 1486 | ) && <ErrorText>{actionData.error}</ErrorText>} | |
| 1455 | 1487 | </div> | |
Loading the diff…
Loading the diff…
Loading the diff…
Loading the diff…
Loading the diff…
Loading the diff…
Loading the diff…
Loading the diff…
Loading the diff…
Loading the diff…
Loading the diff…
Loading the diff…
Loading the diff…
Comments
Sign in to comment.