Updates that g1t opened a "needs code changes" issue for are put where updates go now: the sweep closes the issue as not planned, saying where it is tracked, and marks the update failed, so a security update shows why on its alert and is tried again, and a version update opens again as a pull request with its checks. An alert's history no longer repeats an entry that says the same as its latest one, so an update that fails the same way each day adds it once.
5 files+121−70/5 viewed
| 336 | 336 | | Merged | The pull request merged. | | |
| 337 | 337 | | Closed | The pull request was closed without merging. | | |
| 338 | 338 | | Superseded | A newer security update for the same package replaced it, or the alerts it fixes were fixed another way or dismissed. | | |
| 339 | − | | Needs code changes | An earlier update that g1t opened an issue for. New updates no longer reach this status. | | |
| 339 | + | | Needs code changes | An earlier update that g1t opened an issue for. g1t closes that issue, and the update shows as failed. | | |
| 340 | 340 | | Failed | The update could not be made. | | |
| 341 | 341 | ||
| 342 | 342 | ### When an update is no longer needed |
| 22 | 22 | use g1t_contracts::repos::RepoPath; | |
| 23 | 23 | use g1t_contracts::security::{BumpArgs, UpdateState, update_branch}; | |
| 24 | 24 | use g1t_contracts::time::rfc3339; | |
| 25 | − | use g1t_contracts::work::{OpenPullArgs, Pull, PullActionArgs, PullDetail, PullStatus, Runtime, ViewArgs}; | |
| 25 | + | use g1t_contracts::work::{IssueActionArgs, IssueReason, OpenPullArgs, Pull, PullActionArgs, PullDetail, PullStatus, Runtime, ViewArgs}; | |
| 26 | 26 | use g1t_contracts::{Outcome, User}; | |
| 27 | 27 | use g1t_kit::now_ms; | |
| 28 | 28 | use g1t_scan::osv::{self, Severity}; | |
| ⋯ | |||
| 76 | 76 | } | |
| 77 | 77 | } | |
| 78 | 78 | ||
| 79 | + | /// Why an update that never pushed failed, as its alert shows it. | |
| 80 | + | pub fn cannot_raise(package: &str, target: &str) -> String { | |
| 81 | + | format!( | |
| 82 | + | "`{package}` could not be raised to {target} on its own: a package that depends on it may hold it back, or the upgrade needs code changes." | |
| 83 | + | ) | |
| 84 | + | } | |
| 85 | + | ||
| 79 | 86 | /// The pull request's title. | |
| 80 | 87 | pub fn pull_title(package: &str, target: &str) -> String { | |
| 81 | 88 | format!("Upgrade {package} to {target}").chars().take(200).collect() | |
| ⋯ | |||
| 250 | 257 | Ok(matches!(closed, Outcome::Ok(_))) | |
| 251 | 258 | } | |
| 252 | 259 | ||
| 253 | − | /// Records `action` on every open alert of an update's package. | |
| 260 | + | /// Closes an issue g1t opened for an update before it stopped opening | |
| 261 | + | /// them, saying why. One already closed is left as it is. | |
| 262 | + | pub(crate) async fn close_issue_with(&self, repo: &RepoRow, number: u32, why: String) -> Result<()> { | |
| 263 | + | let system = User::system(&repo.namespace); | |
| 264 | + | let path = Self::path_of(repo); | |
| 265 | + | let closed: Outcome<Value> = g1t_kit::call( | |
| 266 | + | &self.work, | |
| 267 | + | "close_issue", | |
| 268 | + | &IssueActionArgs { actor: system.clone(), repo: path.clone(), number, reason: Some(IssueReason::NotPlanned) }, | |
| 269 | + | ) | |
| 270 | + | .await?; | |
| 271 | + | if matches!(closed, Outcome::Ok(_)) { | |
| 272 | + | self.comment(&system, &path, number, why).await?; | |
| 273 | + | } | |
| 274 | + | Ok(()) | |
| 275 | + | } | |
| 276 | + | ||
| 277 | + | /// Records `action` on every open alert of an update's package, except | |
| 278 | + | /// on an alert whose latest entry already says the same. | |
| 254 | 279 | async fn note(&self, row: &UpdateRow, action: &str, actor: Option<&str>, number: Option<u32>, comment: Option<&str>) -> Result<()> { | |
| 255 | 280 | let ids = self.store.open_ids(&row.repo_id, &row.ecosystem, &row.package).await?; | |
| 281 | + | let latest = self.store.latest_activity(&row.repo_id, &ids).await?; | |
| 256 | 282 | let activity: Vec<Activity> = ids | |
| 257 | 283 | .iter() | |
| 284 | + | .filter(|id| latest.get(*id).is_none_or(|(last, said)| last != action || said.as_deref() != comment)) | |
| 258 | 285 | .map(|id| Activity { alert_id: id, action, actor, reason: None, comment, number }) | |
| 259 | 286 | .collect(); | |
| 260 | 287 | self.store.record(&row.repo_id, &activity).await | |
| ⋯ | |||
| 618 | 645 | self.store.set_update(&row, UpdateState::Failed, row.pull(), None, Some(&why)).await?; | |
| 619 | 646 | continue; | |
| 620 | 647 | } | |
| 621 | − | let why = format!( | |
| 622 | − | "`{}` could not be raised to {} on its own: a package that depends on it may hold it back, or the upgrade needs code changes.", | |
| 623 | − | row.package, row.target | |
| 624 | − | ); | |
| 648 | + | let why = cannot_raise(&row.package, &row.target); | |
| 649 | + | self.store.set_update(&row, UpdateState::Failed, row.pull(), None, Some(&why)).await?; | |
| 650 | + | self.note(&row, "update_failed", Some(g1t_contracts::system::USERNAME), None, Some(&why)).await?; | |
| 651 | + | } | |
| 652 | + | // Left from when g1t opened an issue for an upgrade it could not | |
| 653 | + | // make: the issue closes, and the update fails as one would now. | |
| 654 | + | for row in self.store.needs_code_updates(10).await? { | |
| 655 | + | let why = cannot_raise(&row.package, &row.target); | |
| 656 | + | if let (Some(repo), Some(number)) = (self.store.repo(&row.repo_id).await?, row.issue()) { | |
| 657 | + | let path = Self::path_of(&repo); | |
| 658 | + | let alerts = format!("/{}/{}/security/vulnerabilities", path.namespace, path.name); | |
| 659 | + | self.close_issue_with(&repo, number, format!( | |
| 660 | + | "g1t no longer opens issues for upgrades it cannot make on its own. This one is on [its alert]({alerts}), \ | |
| 661 | + | which says why and is tried again later. To have an agent make the change, use **Fix with g1t** there." | |
| 662 | + | )) | |
| 663 | + | .await?; | |
| 664 | + | } | |
| 625 | 665 | self.store.set_update(&row, UpdateState::Failed, row.pull(), None, Some(&why)).await?; | |
| 626 | 666 | self.note(&row, "update_failed", Some(g1t_contracts::system::USERNAME), None, Some(&why)).await?; | |
| 627 | 667 | } | |
| 902 | 902 | Ok(()) | |
| 903 | 903 | } | |
| 904 | 904 | ||
| 905 | + | /// Each of `alert_ids`' latest action and comment, for the alerts that | |
| 906 | + | /// have any. | |
| 907 | + | pub async fn latest_activity(&self, repo_id: &str, alert_ids: &[String]) -> Result<HashMap<String, (String, Option<String>)>> { | |
| 908 | + | #[derive(Deserialize)] | |
| 909 | + | struct Latest { | |
| 910 | + | alert_id: String, | |
| 911 | + | action: String, | |
| 912 | + | comment: Option<String>, | |
| 913 | + | } | |
| 914 | + | if alert_ids.is_empty() { | |
| 915 | + | return Ok(HashMap::new()); | |
| 916 | + | } | |
| 917 | + | let marks = vec!["?"; alert_ids.len()].join(", "); | |
| 918 | + | let mut binds: Vec<JsValue> = vec![repo_id.into()]; | |
| 919 | + | binds.extend(alert_ids.iter().map(|id| JsValue::from(id.as_str()))); | |
| 920 | + | let rows = self | |
| 921 | + | .db | |
| 922 | + | .prepare(format!( | |
| 923 | + | "SELECT alert_id, action, comment FROM alert_activity WHERE repo_id = ? AND alert_id IN ({marks}) | |
| 924 | + | ORDER BY at DESC, id DESC" | |
| 925 | + | )) | |
| 926 | + | .bind(&binds)? | |
| 927 | + | .all() | |
| 928 | + | .await? | |
| 929 | + | .results::<Latest>()?; | |
| 930 | + | let mut latest = HashMap::new(); | |
| 931 | + | for row in rows { | |
| 932 | + | latest.entry(row.alert_id).or_insert((row.action, row.comment)); | |
| 933 | + | } | |
| 934 | + | Ok(latest) | |
| 935 | + | } | |
| 936 | + | ||
| 905 | 937 | /// What happened to a repository's alerts, newest first. | |
| 906 | 938 | pub async fn activity(&self, repo_id: &str, limit: u32) -> Result<Vec<AlertActivity>> { | |
| 907 | 939 | Ok(self | |
| ⋯ | |||
| 952 | 984 | } | |
| 953 | 985 | ||
| 954 | 986 | /// Updates asked of a sandbox before `before` that never pushed. | |
| 987 | + | /// Updates left in `needs_code`, which g1t opened an issue for before | |
| 988 | + | /// it stopped doing that. | |
| 989 | + | pub async fn needs_code_updates(&self, limit: u32) -> Result<Vec<UpdateRow>> { | |
| 990 | + | self.db | |
| 991 | + | .prepare("SELECT * FROM updates WHERE state = 'needs_code' ORDER BY updated_at LIMIT ?") | |
| 992 | + | .bind(&[limit.into()])? | |
| 993 | + | .all() | |
| 994 | + | .await? | |
| 995 | + | .results::<UpdateRow>() | |
| 996 | + | } | |
| 997 | + | ||
| 955 | 998 | pub async fn stalled_updates(&self, before: &str, limit: u32) -> Result<Vec<UpdateRow>> { | |
| 956 | 999 | self.db | |
| 957 | 1000 | .prepare("SELECT * FROM updates WHERE state = 'requested' AND updated_at < ? ORDER BY updated_at LIMIT ?") | |
| 42 | 42 | pub reviewers: String, | |
| 43 | 43 | pub state: String, | |
| 44 | 44 | pub pull: Option<i64>, | |
| 45 | + | /// The issue g1t opened for it, from before it stopped doing that. | |
| 46 | + | #[serde(default)] | |
| 47 | + | pub issue: Option<i64>, | |
| 45 | 48 | pub head: Option<String>, | |
| 46 | 49 | pub merge_by: Option<String>, | |
| 47 | 50 | pub error: Option<String>, | |
| ⋯ | |||
| 57 | 60 | self.pull.map(|n| n.max(0) as u32) | |
| 58 | 61 | } | |
| 59 | 62 | ||
| 63 | + | pub fn issue(&self) -> Option<u32> { | |
| 64 | + | self.issue.map(|n| n.max(0) as u32) | |
| 65 | + | } | |
| 66 | + | ||
| 60 | 67 | pub fn dependencies(&self) -> Vec<UpdatedDependency> { | |
| 61 | 68 | serde_json::from_str(&self.dependencies).unwrap_or_default() | |
| 62 | 69 | } | |
| ⋯ | |||
| 252 | 259 | } | |
| 253 | 260 | ||
| 254 | 261 | /// Update pull requests asked of a sandbox before `before` that never pushed. | |
| 262 | + | /// Version updates left in `needs_code`, which g1t opened an issue for | |
| 263 | + | /// before it stopped doing that. | |
| 264 | + | pub async fn needs_code_update_pulls(&self, limit: u32) -> Result<Vec<PullRow>> { | |
| 265 | + | self.db | |
| 266 | + | .prepare("SELECT * FROM update_pulls WHERE state = 'needs_code' ORDER BY updated_at LIMIT ?") | |
| 267 | + | .bind(&[limit.into()])? | |
| 268 | + | .all() | |
| 269 | + | .await? | |
| 270 | + | .results::<PullRow>() | |
| 271 | + | } | |
| 272 | + | ||
| 255 | 273 | pub async fn stalled_update_pulls(&self, before: &str, limit: u32) -> Result<Vec<PullRow>> { | |
| 256 | 274 | self.db | |
| 257 | 275 | .prepare("SELECT * FROM update_pulls WHERE state = 'requested' AND updated_at < ? ORDER BY updated_at LIMIT ?") | |
| 399 | 399 | let error = "The update could not be made in a sandbox: its packages may need code changes to move, or a registry could not be reached."; | |
| 400 | 400 | self.store.set_update_pull(&row.id, UpdateState::Failed, row.pull(), None, Some(error)).await?; | |
| 401 | 401 | } | |
| 402 | + | // Left from when g1t opened an issue for an update whose checks | |
| 403 | + | // failed: the issue closes, and the next run opens the update again | |
| 404 | + | // as a pull request that stays open with its failing checks. | |
| 405 | + | for row in self.store.needs_code_update_pulls(10).await? { | |
| 406 | + | if let (Some(repo), Some(number)) = (self.store.repo(&row.repo_id).await?, row.issue()) { | |
| 407 | + | let why = "g1t no longer opens issues for updates whose checks fail. The entry's next run opens this update again \ | |
| 408 | + | as a pull request, which stays open with its checks for you to decide on."; | |
| 409 | + | self.close_issue_with(&repo, number, why.to_owned()).await?; | |
| 410 | + | } | |
| 411 | + | let error = "Raising the versions failed this branch's required checks."; | |
| 412 | + | self.store.set_update_pull(&row.id, UpdateState::Failed, row.pull(), None, Some(error)).await?; | |
| 413 | + | } | |
| 402 | 414 | Ok(()) | |
| 403 | 415 | } | |
| 404 | 416 | ||
| ⋯ | |||
| 1567 | 1579 | reviewers: "[]".into(), | |
| 1568 | 1580 | state: state.into(), | |
| 1569 | 1581 | pull: Some(3), | |
| 1582 | + | issue: None, | |
| 1570 | 1583 | head: None, | |
| 1571 | 1584 | merge_by: None, | |
| 1572 | 1585 | error: None, | |