voro_core/
pr.rs

1//! Tracking a GitHub PR on a task (DESIGN.md §11c): parsing a PR reference into
2//! the pieces a `gh` call needs, and turning a PR's review comments into a
3//! reject-with-feedback body. Pure of I/O — the `gh` shell-out lives in the
4//! `voro` crate — so everything here is testable against canned strings.
5
6use serde::Deserialize;
7
8use crate::error::{Error, Result};
9use crate::model::{Task, TaskState};
10
11/// A parsed reference to a GitHub pull request. The base repo is recorded
12/// explicitly (`owner`/`repo`/`host`) rather than inferred from a checkout's
13/// `origin`, so a PR opened from a fork is still addressed against the repo
14/// where the diff and its review comments live (DESIGN.md §11c).
15#[derive(Debug, Clone, PartialEq, Eq)]
16pub struct PrRef {
17    /// Canonical `https://{host}/{owner}/{repo}/pull/{number}` URL.
18    pub url: String,
19    pub host: String,
20    pub owner: String,
21    pub repo: String,
22    pub number: u64,
23}
24
25impl PrRef {
26    /// Parse a PR reference from a URL (`https://github.com/o/r/pull/12`, with
27    /// or without scheme, extra path segments, or a query/fragment) or the
28    /// `owner/repo#number` shorthand. Enterprise hosts are preserved. The URL
29    /// is re-emitted in canonical form so tracking is idempotent regardless of
30    /// which form the operator pasted.
31    pub fn parse(input: &str) -> Result<PrRef> {
32        let s = input.trim();
33        if s.is_empty() {
34            return Err(Error::Invalid("a PR reference is required".into()));
35        }
36
37        // `owner/repo#number` shorthand: no scheme, exactly one slash.
38        if let Some((repo_part, num_part)) = s.split_once('#')
39            && !repo_part.contains("://")
40            && repo_part.matches('/').count() == 1
41        {
42            let (owner, repo) = repo_part.split_once('/').unwrap();
43            let number = parse_number(num_part)?;
44            return PrRef::build("github.com", owner, repo, number);
45        }
46
47        // URL form. Drop the scheme and any query/fragment, then walk the path
48        // segments looking for `.../pull/<n>`.
49        let no_scheme = s
50            .split_once("://")
51            .map(|(_, rest)| rest)
52            .unwrap_or(s)
53            .trim_start_matches('/');
54        let path = no_scheme
55            .split(['?', '#'])
56            .next()
57            .unwrap_or(no_scheme)
58            .trim_end_matches('/');
59        let segments: Vec<&str> = path.split('/').filter(|s| !s.is_empty()).collect();
60
61        // [host, owner, repo, ("pull"|"pulls"), number, ...]
62        if let Some(pos) = segments.iter().position(|s| *s == "pull" || *s == "pulls")
63            && pos >= 3
64            && let Some(num) = segments.get(pos + 1)
65        {
66            let host = segments[pos - 3];
67            let owner = segments[pos - 2];
68            let repo = segments[pos - 1];
69            let number = parse_number(num)?;
70            return PrRef::build(host, owner, repo, number);
71        }
72
73        Err(Error::Invalid(format!(
74            "'{input}' is not a GitHub PR reference — expected a URL like \
75             https://github.com/owner/repo/pull/12 or the shorthand owner/repo#12"
76        )))
77    }
78
79    fn build(host: &str, owner: &str, repo: &str, number: u64) -> Result<PrRef> {
80        if host.is_empty() || owner.is_empty() || repo.is_empty() {
81            return Err(Error::Invalid(
82                "a PR reference needs a host, owner, and repo".into(),
83            ));
84        }
85        Ok(PrRef {
86            url: format!("https://{host}/{owner}/{repo}/pull/{number}"),
87            host: host.to_string(),
88            owner: owner.to_string(),
89            repo: repo.to_string(),
90            number,
91        })
92    }
93
94    /// The `owner/repo` slug a `gh -R`/`gh api repos/...` call expects.
95    pub fn nwo(&self) -> String {
96        format!("{}/{}", self.owner, self.repo)
97    }
98
99    /// The API path segment for this PR's REST resources, e.g.
100    /// `repos/owner/repo/pulls/12`.
101    pub fn api_path(&self, resource: &str) -> String {
102        format!("repos/{}/pulls/{}/{resource}", self.nwo(), self.number)
103    }
104
105    /// The PR's diff narrowed to a commit range — GitHub's compare-within-a-PR
106    /// view (DESIGN.md §8), which is what makes a re-review proportional to the
107    /// rework rather than to the whole branch. It stays *inside* the PR, so the
108    /// review comments and the merge button are where they always were, and the
109    /// full diff remains one click away.
110    pub fn range_url(&self, since: &str, head: &str) -> String {
111        format!("{}/files/{since}..{head}", self.url)
112    }
113}
114
115fn parse_number(raw: &str) -> Result<u64> {
116    raw.trim()
117        .parse()
118        .map_err(|_| Error::Invalid(format!("'{raw}' is not a PR number")))
119}
120
121/// GitHub's mergeability verdict for a tracked PR (`gh pr view --json
122/// mergeable`, DESIGN.md §8): whether a review task's branch still merges
123/// cleanly with its base. Read fresh and never stored — the same
124/// rendered-not-stored shape as the incomplete-report flag — it is what turns a
125/// `CONFLICTING` PR into the informational `[branch conflicts]` marker.
126#[derive(Debug, Clone, Copy, PartialEq, Eq)]
127pub enum Mergeability {
128    /// The branch merges cleanly into its base — no marker.
129    Mergeable,
130    /// The branch conflicts with the moved base — surface the marker so the
131    /// operator knows to resolve it before merging.
132    Conflicting,
133    /// GitHub has not finished computing it, or gave no usable answer — no
134    /// signal. Never treated as a conflict.
135    Unknown,
136}
137
138impl Mergeability {
139    /// Whether to surface the conflict marker. Only a definite `Conflicting`
140    /// verdict does; `Mergeable` and `Unknown` show nothing, so a PR GitHub is
141    /// still recomputing never flickers a false conflict.
142    pub fn conflicts(self) -> bool {
143        matches!(self, Mergeability::Conflicting)
144    }
145}
146
147/// Read GitHub's mergeability verdict out of `gh pr view --json mergeable`
148/// output (`{"mergeable":"CONFLICTING"}`). Only the two definite verdicts map
149/// to themselves; anything else — `UNKNOWN`, a missing field, malformed JSON,
150/// or the empty string a missing or unauthenticated `gh` leaves behind —
151/// degrades to [`Mergeability::Unknown`], so no signal is ever mistaken for a
152/// conflict. Pure of I/O; the `gh` call lives in the `voro` crate.
153pub fn parse_mergeable(json: &str) -> Mergeability {
154    #[derive(Deserialize)]
155    struct View {
156        #[serde(default)]
157        mergeable: String,
158    }
159    match serde_json::from_str::<View>(json) {
160        Ok(view) => match view.mergeable.as_str() {
161            "MERGEABLE" => Mergeability::Mergeable,
162            "CONFLICTING" => Mergeability::Conflicting,
163            _ => Mergeability::Unknown,
164        },
165        Err(_) => Mergeability::Unknown,
166    }
167}
168
169#[derive(Debug, Clone, Deserialize)]
170struct GhUser {
171    #[serde(default)]
172    login: String,
173}
174
175/// One pull-request review — the summary a reviewer submits alongside (or
176/// instead of) inline comments (`gh api repos/o/r/pulls/N/reviews`). Only the
177/// fields the feedback body uses are named; anything else gh emits is ignored.
178#[derive(Debug, Clone, Deserialize)]
179struct PrReview {
180    #[serde(default)]
181    body: String,
182    #[serde(default)]
183    state: String,
184    #[serde(default)]
185    user: Option<GhUser>,
186}
187
188/// One inline review comment on the diff (`gh api repos/o/r/pulls/N/comments`).
189#[derive(Debug, Clone, Deserialize)]
190struct PrReviewComment {
191    #[serde(default)]
192    body: String,
193    #[serde(default)]
194    path: Option<String>,
195    #[serde(default)]
196    line: Option<i64>,
197    #[serde(default)]
198    user: Option<GhUser>,
199}
200
201fn login(user: &Option<GhUser>) -> &str {
202    user.as_ref()
203        .map(|u| u.login.as_str())
204        .filter(|l| !l.is_empty())
205        .unwrap_or("unknown")
206}
207
208/// Build a reject-with-feedback body from a PR's reviews and inline comments
209/// (DESIGN.md §11c), so a GitHub review reaches the agent without retyping.
210/// Reviews with an empty body are skipped; the inline comments carry their own
211/// text. Returns an empty string when there is nothing to relay.
212pub fn format_review_feedback(
213    pr: &PrRef,
214    reviews_json: &str,
215    comments_json: &str,
216) -> Result<String> {
217    let reviews: Vec<PrReview> = serde_json::from_str(reviews_json)
218        .map_err(|e| Error::Invalid(format!("invalid PR reviews JSON: {e}")))?;
219    let comments: Vec<PrReviewComment> = serde_json::from_str(comments_json)
220        .map_err(|e| Error::Invalid(format!("invalid PR comments JSON: {e}")))?;
221
222    let mut sections: Vec<String> = Vec::new();
223    for r in &reviews {
224        if r.body.trim().is_empty() {
225            continue;
226        }
227        let state = if r.state.is_empty() {
228            String::new()
229        } else {
230            format!(" ({})", r.state.to_lowercase())
231        };
232        sections.push(format!("@{}{state}: {}", login(&r.user), r.body.trim()));
233    }
234    for c in &comments {
235        if c.body.trim().is_empty() {
236            continue;
237        }
238        let loc = match (&c.path, c.line) {
239            (Some(path), Some(line)) => format!("`{path}:{line}` — "),
240            (Some(path), None) => format!("`{path}` — "),
241            _ => String::new(),
242        };
243        sections.push(format!("{loc}@{}: {}", login(&c.user), c.body.trim()));
244    }
245
246    if sections.is_empty() {
247        return Ok(String::new());
248    }
249    let mut body = format!("Review feedback from {}\n", pr.url);
250    for section in sections {
251        body.push('\n');
252        body.push_str(&section);
253        body.push('\n');
254    }
255    Ok(body)
256}
257
258/// Everything the forge needs to open a PR for a review task (DESIGN.md §8):
259/// the branch to push and the title and body of the pull request. Assembled by
260/// [`plan_pr`] once the task is proven PR-ready.
261#[derive(Debug, Clone, PartialEq, Eq)]
262pub struct PrPlan {
263    pub branch: String,
264    pub title: String,
265    pub body: String,
266}
267
268/// Validate that a task can have a PR opened from its done-time state, and if so
269/// assemble the [`PrPlan`] (DESIGN.md §8): a `review` task carrying both a branch
270/// (the work to push) and a completion summary (the PR body; the caller supplies
271/// the latest). Each gap fails naming what is missing — state, branch, or
272/// summary. The branch gap is the one no advertised verb leads to: a review task
273/// without one asks for *accept* rather than *pr* (DESIGN.md §6), so this
274/// refusal is reached only by an operator naming `pr` on it directly. Pure of
275/// I/O.
276pub fn plan_pr(task: &Task, latest_summary: Option<&str>) -> Result<PrPlan> {
277    if task.state != TaskState::Review {
278        return Err(Error::Invalid(format!(
279            "only a review task can have a PR opened from its summary; task {} is {}",
280            task.id, task.state
281        )));
282    }
283    let branch = task.branch_name().ok_or_else(|| {
284        Error::Invalid(format!(
285            "task {} has no branch to push — record one with `voro done --branch` or \
286             `voro set --branch`",
287            task.id
288        ))
289    })?;
290    let body = latest_summary
291        .map(str::trim)
292        .filter(|s| !s.is_empty())
293        .ok_or_else(|| {
294            Error::Invalid(format!(
295                "task {} has no completion summary for the PR body — record one with \
296                 `voro set --summary`",
297                task.id
298            ))
299        })?;
300    Ok(PrPlan {
301        branch: branch.to_string(),
302        title: task.title.clone(),
303        body: body.to_string(),
304    })
305}
306
307#[cfg(test)]
308mod tests {
309    use super::*;
310
311    #[test]
312    fn parses_a_full_url() {
313        let pr = PrRef::parse("https://github.com/acme/widget/pull/42").unwrap();
314        assert_eq!(pr.host, "github.com");
315        assert_eq!(pr.owner, "acme");
316        assert_eq!(pr.repo, "widget");
317        assert_eq!(pr.number, 42);
318        assert_eq!(pr.nwo(), "acme/widget");
319        assert_eq!(pr.url, "https://github.com/acme/widget/pull/42");
320        assert_eq!(
321            pr.api_path("comments"),
322            "repos/acme/widget/pulls/42/comments"
323        );
324    }
325
326    #[test]
327    fn narrows_a_pr_to_a_commit_range() {
328        assert_eq!(
329            PrRef::parse("https://github.com/acme/widget/pull/42")
330                .unwrap()
331                .range_url("abc1234", "def5678"),
332            "https://github.com/acme/widget/pull/42/files/abc1234..def5678"
333        );
334    }
335
336    #[test]
337    fn parses_url_without_scheme_and_with_extra_segments() {
338        let pr = PrRef::parse("github.com/acme/widget/pull/42/files?w=1#discussion").unwrap();
339        assert_eq!(pr.number, 42);
340        assert_eq!(pr.url, "https://github.com/acme/widget/pull/42");
341    }
342
343    #[test]
344    fn parses_the_owner_repo_shorthand() {
345        let pr = PrRef::parse("acme/widget#7").unwrap();
346        assert_eq!(pr.owner, "acme");
347        assert_eq!(pr.repo, "widget");
348        assert_eq!(pr.number, 7);
349        assert_eq!(pr.url, "https://github.com/acme/widget/pull/7");
350    }
351
352    #[test]
353    fn preserves_an_enterprise_host() {
354        let pr = PrRef::parse("https://git.example.com/acme/widget/pull/3").unwrap();
355        assert_eq!(pr.host, "git.example.com");
356        assert_eq!(pr.url, "https://git.example.com/acme/widget/pull/3");
357    }
358
359    #[test]
360    fn rejects_non_pr_references() {
361        assert!(PrRef::parse("https://github.com/acme/widget/issues/42").is_err());
362        assert!(PrRef::parse("https://github.com/acme/widget").is_err());
363        assert!(PrRef::parse("not a url").is_err());
364        assert!(PrRef::parse("acme/widget/extra#1").is_err());
365        assert!(PrRef::parse("").is_err());
366        assert!(PrRef::parse("https://github.com/acme/widget/pull/notanumber").is_err());
367    }
368
369    fn pr() -> PrRef {
370        PrRef::parse("https://github.com/acme/widget/pull/42").unwrap()
371    }
372
373    #[test]
374    fn formats_reviews_and_inline_comments() {
375        let reviews = r#"[
376            {"user": {"login": "alice"}, "state": "CHANGES_REQUESTED", "body": "Please fix the parser"},
377            {"user": {"login": "bob"}, "state": "APPROVED", "body": ""}
378        ]"#;
379        let comments = r#"[
380            {"user": {"login": "alice"}, "path": "src/lib.rs", "line": 12, "body": "off-by-one here"}
381        ]"#;
382        let body = format_review_feedback(&pr(), reviews, comments).unwrap();
383        assert!(body.contains("Review feedback from https://github.com/acme/widget/pull/42"));
384        assert!(body.contains("@alice (changes_requested): Please fix the parser"));
385        // the bare approval with no body is skipped
386        assert!(!body.contains("@bob"));
387        assert!(body.contains("`src/lib.rs:12` — @alice: off-by-one here"));
388    }
389
390    #[test]
391    fn empty_when_there_is_nothing_to_relay() {
392        let body = format_review_feedback(&pr(), "[]", "[]").unwrap();
393        assert!(body.is_empty());
394        // a lone approval with no comments is also nothing to relay
395        let reviews = r#"[{"user": {"login": "bob"}, "state": "APPROVED", "body": ""}]"#;
396        assert!(
397            format_review_feedback(&pr(), reviews, "[]")
398                .unwrap()
399                .is_empty()
400        );
401    }
402
403    #[test]
404    fn tolerates_missing_optional_fields() {
405        // a comment with no path/line/user still relays its body
406        let comments = r#"[{"body": "a general note"}]"#;
407        let body = format_review_feedback(&pr(), "[]", comments).unwrap();
408        assert!(body.contains("@unknown: a general note"));
409    }
410
411    #[test]
412    fn rejects_malformed_json() {
413        assert!(format_review_feedback(&pr(), "not json", "[]").is_err());
414        assert!(format_review_feedback(&pr(), "[]", "not json").is_err());
415    }
416
417    // --- parse_mergeable (DESIGN.md §8: detecting a stale review branch) ---
418
419    #[test]
420    fn conflicting_is_the_only_verdict_that_marks() {
421        assert_eq!(
422            parse_mergeable(r#"{"mergeable":"CONFLICTING"}"#),
423            Mergeability::Conflicting
424        );
425        assert!(parse_mergeable(r#"{"mergeable":"CONFLICTING"}"#).conflicts());
426        assert_eq!(
427            parse_mergeable(r#"{"mergeable":"MERGEABLE"}"#),
428            Mergeability::Mergeable
429        );
430        assert!(!parse_mergeable(r#"{"mergeable":"MERGEABLE"}"#).conflicts());
431    }
432
433    #[test]
434    fn unknown_and_unusable_answers_give_no_signal() {
435        // GitHub still recomputing
436        assert_eq!(
437            parse_mergeable(r#"{"mergeable":"UNKNOWN"}"#),
438            Mergeability::Unknown
439        );
440        // an unexpected value, a missing field, malformed JSON, and the empty
441        // string a missing/unauthenticated `gh` leaves all read as no signal
442        for raw in [
443            r#"{"mergeable":"WHATEVER"}"#,
444            r#"{"mergeable":""}"#,
445            "{}",
446            "not json",
447            "",
448        ] {
449            assert_eq!(parse_mergeable(raw), Mergeability::Unknown, "{raw}");
450            assert!(!parse_mergeable(raw).conflicts(), "{raw}");
451        }
452    }
453
454    // --- plan_pr (DESIGN.md §8: opening a PR from a review task's summary) ---
455
456    use crate::model::Priority;
457
458    fn task(state: TaskState, branch: Option<&str>) -> Task {
459        Task {
460            id: 82,
461            project_id: 1,
462            repo_id: None,
463            title: "Extend pr to create the PR".into(),
464            body: String::new(),
465            priority: Priority::P1,
466            state,
467            agent: None,
468            human: false,
469            deep: false,
470            question: None,
471            pr_url: None,
472            branch: branch.map(str::to_string),
473            state_since: "2026-07-10 00:00:00".into(),
474            created_at: "2026-07-10 00:00:00".into(),
475            closed_at: None,
476        }
477    }
478
479    #[test]
480    fn plans_a_pr_from_a_review_task_with_branch_and_summary() {
481        let plan = plan_pr(
482            &task(TaskState::Review, Some("feat/pr")),
483            Some("Did the thing"),
484        )
485        .unwrap();
486        assert_eq!(plan.branch, "feat/pr");
487        assert_eq!(plan.title, "Extend pr to create the PR");
488        assert_eq!(plan.body, "Did the thing");
489    }
490
491    #[test]
492    fn plan_requires_the_review_state() {
493        for state in [
494            TaskState::Ready,
495            TaskState::Running,
496            TaskState::NeedsInput,
497            TaskState::Done,
498        ] {
499            let err = plan_pr(&task(state, Some("feat/pr")), Some("summary"))
500                .unwrap_err()
501                .to_string();
502            assert!(err.contains("review"), "{state}: {err}");
503        }
504    }
505
506    #[test]
507    fn plan_names_a_missing_branch() {
508        let err = plan_pr(&task(TaskState::Review, None), Some("summary"))
509            .unwrap_err()
510            .to_string();
511        assert!(err.contains("branch"), "{err}");
512        // a blank branch is treated as absent
513        let err = plan_pr(&task(TaskState::Review, Some("   ")), Some("summary"))
514            .unwrap_err()
515            .to_string();
516        assert!(err.contains("branch"), "{err}");
517    }
518
519    #[test]
520    fn plan_names_a_missing_summary() {
521        let err = plan_pr(&task(TaskState::Review, Some("feat/pr")), None)
522            .unwrap_err()
523            .to_string();
524        assert!(err.contains("summary"), "{err}");
525        let err = plan_pr(&task(TaskState::Review, Some("feat/pr")), Some("  "))
526            .unwrap_err()
527            .to_string();
528        assert!(err.contains("summary"), "{err}");
529    }
530}
voro_core/pr.rs

voro_core/
pr.rs
