|
| 1 | +//! Cross-lane PR awareness via `gh pr list` + `git merge-base`. |
| 2 | +//! |
| 3 | +//! The fleet's `task_ready_for_agent` queue treats every candidate task as |
| 4 | +//! independent. In practice, lane A may produce a file that lane B's task |
| 5 | +//! depends on — and when A's PR isn't yet merged into B's base, B will pick |
| 6 | +//! up the work and immediately rediscover the dependency at PR time. This |
| 7 | +//! module is the data-layer half of the fix: it answers "which open PRs |
| 8 | +//! touch which files, and is any given branch already past PR N's head." |
| 9 | +//! |
| 10 | +//! The colony-side consumer (`readyScopeOverlapWarnings` in |
| 11 | +//! `colony/ready-queue.ts`) emits a `merge_pending` warning in `next_action` |
| 12 | +//! when a candidate's file scope intersects an open PR's fileset that the |
| 13 | +//! candidate's base has not yet absorbed. Per `task_claim_file`'s docstring, |
| 14 | +//! Colony's coordination is "soft — never blocks writes" — so this surfaces |
| 15 | +//! as a warning that the autopilot deprioritizes, not a hard block. |
| 16 | +//! |
| 17 | +//! ## Failure posture |
| 18 | +//! |
| 19 | +//! Mirrors [`crate::accounts`]: a missing `gh` binary, an unauthenticated |
| 20 | +//! CLI, or a network failure all return `Ok(vec![])` from |
| 21 | +//! [`open_prs_with_files`]. A dashboard without `gh` access should render |
| 22 | +//! no merge-pending warnings, not crash. For [`branch_contains_pr`], a |
| 23 | +//! lookup failure conservatively returns `Ok(false)` — "I cannot prove |
| 24 | +//! this PR is merged" is the safe default that produces a warning. |
| 25 | +
|
| 26 | +use serde::{Deserialize, Serialize}; |
| 27 | + |
| 28 | +/// One open PR's metadata + fileset, as far as the ready-queue cares. |
| 29 | +/// |
| 30 | +/// The fileset is *paths only* — `additions` / `deletions` counts from |
| 31 | +/// `gh pr list --json files` are dropped on parse, because cross-lane |
| 32 | +/// prereq detection only needs the set of paths. |
| 33 | +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] |
| 34 | +pub struct PrFileset { |
| 35 | + pub number: u64, |
| 36 | + pub head_ref: String, |
| 37 | + pub base_ref: String, |
| 38 | + pub files: Vec<String>, |
| 39 | +} |
| 40 | + |
| 41 | +impl PrFileset { |
| 42 | + /// `true` if any of this PR's files appears in the candidate scope. |
| 43 | + /// Both sides are matched exact-path; directory globbing is the |
| 44 | + /// caller's job (the colony side already normalizes to file paths). |
| 45 | + pub fn overlaps(&self, scope: &[String]) -> bool { |
| 46 | + self.files.iter().any(|f| scope.iter().any(|s| s == f)) |
| 47 | + } |
| 48 | +} |
| 49 | + |
| 50 | +/// Raw `gh pr list --json` shape — `files` is an array of `{path, additions, …}`. |
| 51 | +#[derive(Deserialize)] |
| 52 | +struct RawPr { |
| 53 | + number: u64, |
| 54 | + #[serde(rename = "headRefName")] |
| 55 | + head_ref: String, |
| 56 | + #[serde(rename = "baseRefName")] |
| 57 | + base_ref: String, |
| 58 | + files: Vec<RawFile>, |
| 59 | +} |
| 60 | + |
| 61 | +#[derive(Deserialize)] |
| 62 | +struct RawFile { |
| 63 | + path: String, |
| 64 | +} |
| 65 | + |
| 66 | +/// Parse `gh pr list --json number,headRefName,baseRefName,files` stdout |
| 67 | +/// into typed [`PrFileset`] rows. Pure — feed it captured JSON in tests. |
| 68 | +/// |
| 69 | +/// A malformed payload returns an empty Vec rather than an error; same |
| 70 | +/// posture as [`crate::accounts::parse`] — the dashboard's "no information" |
| 71 | +/// path is an empty fleet, not a panic. |
| 72 | +pub fn parse(stdout: &str) -> Vec<PrFileset> { |
| 73 | + let raws: Vec<RawPr> = serde_json::from_str(stdout).unwrap_or_default(); |
| 74 | + raws.into_iter() |
| 75 | + .map(|r| PrFileset { |
| 76 | + number: r.number, |
| 77 | + head_ref: r.head_ref, |
| 78 | + base_ref: r.base_ref, |
| 79 | + files: r.files.into_iter().map(|f| f.path).collect(), |
| 80 | + }) |
| 81 | + .collect() |
| 82 | +} |
| 83 | + |
| 84 | +/// `true` when `branch` is at or past `pr_head` — i.e. the PR's head commit |
| 85 | +/// is already an ancestor of `branch`. Wraps |
| 86 | +/// `git merge-base --is-ancestor <pr_head> <branch>` (exit 0 = ancestor). |
| 87 | +/// |
| 88 | +/// Returns `Ok(false)` on any failure path (unknown ref, no git on PATH) — |
| 89 | +/// for cross-lane warnings, "I cannot prove this PR is merged" is the safe |
| 90 | +/// default that produces a warning rather than silently suppressing it. |
| 91 | +pub fn branch_contains_pr(branch: &str, pr_head: &str) -> std::io::Result<bool> { |
| 92 | + let status = std::process::Command::new("git") |
| 93 | + .args(["merge-base", "--is-ancestor", pr_head, branch]) |
| 94 | + .stdout(std::process::Stdio::null()) |
| 95 | + .stderr(std::process::Stdio::null()) |
| 96 | + .status()?; |
| 97 | + Ok(status.success()) |
| 98 | +} |
| 99 | + |
| 100 | +/// Shell out to `gh pr list` and parse the result. |
| 101 | +/// |
| 102 | +/// Returns `Ok(vec![])` when `gh` is missing, unauthenticated, or returns a |
| 103 | +/// non-zero exit — a dashboard without `gh` access renders no warnings, not |
| 104 | +/// a crash. `--limit 100` is enough for any plausible fleet; a 100-PR |
| 105 | +/// backlog is itself a signal worth surfacing elsewhere. |
| 106 | +pub fn open_prs_with_files() -> std::io::Result<Vec<PrFileset>> { |
| 107 | + let out = match std::process::Command::new("gh") |
| 108 | + .args([ |
| 109 | + "pr", |
| 110 | + "list", |
| 111 | + "--state", |
| 112 | + "open", |
| 113 | + "--json", |
| 114 | + "number,headRefName,baseRefName,files", |
| 115 | + "--limit", |
| 116 | + "100", |
| 117 | + ]) |
| 118 | + .output() |
| 119 | + { |
| 120 | + Ok(o) => o, |
| 121 | + Err(_) => return Ok(Vec::new()), |
| 122 | + }; |
| 123 | + if !out.status.success() { |
| 124 | + return Ok(Vec::new()); |
| 125 | + } |
| 126 | + Ok(parse(&String::from_utf8_lossy(&out.stdout))) |
| 127 | +} |
| 128 | + |
| 129 | +fn cache() -> &'static crate::cache::TtlCache<Vec<PrFileset>> { |
| 130 | + static CACHE: std::sync::OnceLock<crate::cache::TtlCache<Vec<PrFileset>>> = |
| 131 | + std::sync::OnceLock::new(); |
| 132 | + // 45s TTL: `gh pr list` is rate-limited (5000/hr authenticated). |
| 133 | + // Four dashboard binaries each refreshing once per TTL window peak at |
| 134 | + // ~320 calls/hr — two orders of magnitude under the cap, while still |
| 135 | + // surfacing a freshly-opened PR within ~one tick of a minute. |
| 136 | + CACHE.get_or_init(|| crate::cache::TtlCache::new(std::time::Duration::from_secs(45))) |
| 137 | +} |
| 138 | + |
| 139 | +/// Cached variant of [`open_prs_with_files`]. The fleet's dashboard tick |
| 140 | +/// is 250 ms; calling the raw fn every tick would burn ~14k `gh` |
| 141 | +/// invocations per hour per binary and trip GitHub's rate limit. The 45 s |
| 142 | +/// TTL is tuned so a freshly-opened PR shows up within ~one minute and |
| 143 | +/// readers stay cheap. |
| 144 | +pub fn load_live_cached() -> std::io::Result<Vec<PrFileset>> { |
| 145 | + cache().get_or_refresh(open_prs_with_files) |
| 146 | +} |
| 147 | + |
| 148 | +/// Drop the cached PR list so the next [`load_live_cached`] re-shells. |
| 149 | +/// Useful after observing a `gh pr merge` / `gh pr close` locally, so the |
| 150 | +/// dashboard's warnings clear immediately rather than after the TTL. |
| 151 | +pub fn invalidate_cache() { |
| 152 | + cache().invalidate(); |
| 153 | +} |
| 154 | + |
| 155 | +/// Find every open PR whose fileset intersects `scope` and that has not yet |
| 156 | +/// been merged into `branch`'s history. This is the question |
| 157 | +/// `readyScopeOverlapWarnings` actually asks: "if lane B claims these |
| 158 | +/// files, what un-merged PRs already touch them and would conflict on |
| 159 | +/// rebase?" |
| 160 | +/// |
| 161 | +/// Returns `Ok(vec![])` when nothing matches, including the trivial "no |
| 162 | +/// open PRs" / "no `gh` on PATH" cases. The caller (colony) folds each |
| 163 | +/// returned PR into a `merge_pending` warning on the task's `next_action`. |
| 164 | +pub fn merge_pending_overlap( |
| 165 | + branch: &str, |
| 166 | + scope: &[String], |
| 167 | +) -> std::io::Result<Vec<PrFileset>> { |
| 168 | + let prs = load_live_cached()?; |
| 169 | + let mut out = Vec::new(); |
| 170 | + for pr in prs { |
| 171 | + if !pr.overlaps(scope) { |
| 172 | + continue; |
| 173 | + } |
| 174 | + // Skip PRs whose head is already an ancestor of `branch` — they're |
| 175 | + // either merged-and-pulled or this lane was cut from a tip that |
| 176 | + // already contained them. Either way, no merge-pending conflict. |
| 177 | + if branch_contains_pr(branch, &pr.head_ref).unwrap_or(false) { |
| 178 | + continue; |
| 179 | + } |
| 180 | + out.push(pr); |
| 181 | + } |
| 182 | + Ok(out) |
| 183 | +} |
| 184 | + |
| 185 | +#[cfg(test)] |
| 186 | +mod tests { |
| 187 | + use super::*; |
| 188 | + |
| 189 | + const FIXTURE: &str = r#"[ |
| 190 | + { |
| 191 | + "number": 24, |
| 192 | + "headRefName": "agent/claude/fleet-state-live-data-2026-05-14-20-19", |
| 193 | + "baseRefName": "main", |
| 194 | + "files": [ |
| 195 | + {"path": "rust/fleet-state/src/main.rs", "additions": 247, "deletions": 40}, |
| 196 | + {"path": "rust/fleet-data/src/fleet.rs", "additions": 419, "deletions": 0} |
| 197 | + ] |
| 198 | + }, |
| 199 | + { |
| 200 | + "number": 25, |
| 201 | + "headRefName": "agent/codex/overlays-phase5-sub-1-spotlight", |
| 202 | + "baseRefName": "main", |
| 203 | + "files": [ |
| 204 | + {"path": "rust/fleet-ui/src/overlay/spotlight.rs", "additions": 312, "deletions": 0} |
| 205 | + ] |
| 206 | + } |
| 207 | +]"#; |
| 208 | + |
| 209 | + #[test] |
| 210 | + fn parses_pr_list_json() { |
| 211 | + let prs = parse(FIXTURE); |
| 212 | + assert_eq!(prs.len(), 2); |
| 213 | + assert_eq!(prs[0].number, 24); |
| 214 | + assert_eq!( |
| 215 | + prs[0].head_ref, |
| 216 | + "agent/claude/fleet-state-live-data-2026-05-14-20-19" |
| 217 | + ); |
| 218 | + assert_eq!(prs[0].base_ref, "main"); |
| 219 | + assert_eq!( |
| 220 | + prs[0].files, |
| 221 | + vec![ |
| 222 | + "rust/fleet-state/src/main.rs".to_string(), |
| 223 | + "rust/fleet-data/src/fleet.rs".to_string(), |
| 224 | + ] |
| 225 | + ); |
| 226 | + assert_eq!(prs[1].number, 25); |
| 227 | + assert_eq!(prs[1].files.len(), 1); |
| 228 | + } |
| 229 | + |
| 230 | + #[test] |
| 231 | + fn malformed_json_yields_empty_vec() { |
| 232 | + assert!(parse("not json at all").is_empty()); |
| 233 | + assert!(parse("").is_empty()); |
| 234 | + assert!(parse("{}").is_empty(), "object, not array"); |
| 235 | + } |
| 236 | + |
| 237 | + #[test] |
| 238 | + fn missing_files_field_drops_the_row() { |
| 239 | + // `files` is required — a row without it fails the whole array parse, |
| 240 | + // which unwrap_or_default turns into an empty Vec. That's intentional: |
| 241 | + // a partial gh response shouldn't half-populate the warnings list. |
| 242 | + let prs = |
| 243 | + parse(r#"[{"number": 1, "headRefName": "x", "baseRefName": "main"}]"#); |
| 244 | + assert!(prs.is_empty()); |
| 245 | + } |
| 246 | + |
| 247 | + fn pr(number: u64, files: &[&str]) -> PrFileset { |
| 248 | + PrFileset { |
| 249 | + number, |
| 250 | + head_ref: format!("agent/x/{number}"), |
| 251 | + base_ref: "main".into(), |
| 252 | + files: files.iter().map(|s| s.to_string()).collect(), |
| 253 | + } |
| 254 | + } |
| 255 | + |
| 256 | + #[test] |
| 257 | + fn overlap_matches_exact_path() { |
| 258 | + let p = pr( |
| 259 | + 1, |
| 260 | + &["rust/fleet-data/src/fleet.rs", "rust/fleet-data/src/tmux.rs"], |
| 261 | + ); |
| 262 | + assert!(p.overlaps(&["rust/fleet-data/src/fleet.rs".to_string()])); |
| 263 | + assert!(p.overlaps(&[ |
| 264 | + "unrelated.txt".to_string(), |
| 265 | + "rust/fleet-data/src/tmux.rs".to_string() |
| 266 | + ])); |
| 267 | + assert!(!p.overlaps(&["rust/fleet-state/src/main.rs".to_string()])); |
| 268 | + assert!(!p.overlaps(&[])); |
| 269 | + } |
| 270 | + |
| 271 | + #[test] |
| 272 | + fn empty_pr_files_never_overlaps() { |
| 273 | + let p = pr(2, &[]); |
| 274 | + assert!(!p.overlaps(&["any.rs".to_string()])); |
| 275 | + } |
| 276 | + |
| 277 | + #[test] |
| 278 | + fn overlap_is_directional() { |
| 279 | + // Path strings must match exactly — `fleet-data/src/fleet.rs` doesn't |
| 280 | + // overlap `rust/fleet-data/src/fleet.rs`. Directory-prefix matching |
| 281 | + // is the caller's job; the colony side normalizes both before calling. |
| 282 | + let p = pr(3, &["rust/fleet-data/src/fleet.rs"]); |
| 283 | + assert!(!p.overlaps(&["fleet-data/src/fleet.rs".to_string()])); |
| 284 | + } |
| 285 | +} |
0 commit comments