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
106fn parse_number(raw: &str) -> Result<u64> {
107    raw.trim()
108        .parse()
109        .map_err(|_| Error::Invalid(format!("'{raw}' is not a PR number")))
110}
111
112#[derive(Debug, Clone, Deserialize)]
113struct GhUser {
114    #[serde(default)]
115    login: String,
116}
117
118/// One pull-request review — the summary a reviewer submits alongside (or
119/// instead of) inline comments (`gh api repos/o/r/pulls/N/reviews`). Only the
120/// fields the feedback body uses are named; anything else gh emits is ignored.
121#[derive(Debug, Clone, Deserialize)]
122struct PrReview {
123    #[serde(default)]
124    body: String,
125    #[serde(default)]
126    state: String,
127    #[serde(default)]
128    user: Option<GhUser>,
129}
130
131/// One inline review comment on the diff (`gh api repos/o/r/pulls/N/comments`).
132#[derive(Debug, Clone, Deserialize)]
133struct PrReviewComment {
134    #[serde(default)]
135    body: String,
136    #[serde(default)]
137    path: Option<String>,
138    #[serde(default)]
139    line: Option<i64>,
140    #[serde(default)]
141    user: Option<GhUser>,
142}
143
144fn login(user: &Option<GhUser>) -> &str {
145    user.as_ref()
146        .map(|u| u.login.as_str())
147        .filter(|l| !l.is_empty())
148        .unwrap_or("unknown")
149}
150
151/// Build a reject-with-feedback body from a PR's reviews and inline comments
152/// (DESIGN.md §11c), so a GitHub review reaches the agent without retyping.
153/// Reviews with an empty body are skipped; the inline comments carry their own
154/// text. Returns an empty string when there is nothing to relay.
155pub fn format_review_feedback(
156    pr: &PrRef,
157    reviews_json: &str,
158    comments_json: &str,
159) -> Result<String> {
160    let reviews: Vec<PrReview> = serde_json::from_str(reviews_json)
161        .map_err(|e| Error::Invalid(format!("invalid PR reviews JSON: {e}")))?;
162    let comments: Vec<PrReviewComment> = serde_json::from_str(comments_json)
163        .map_err(|e| Error::Invalid(format!("invalid PR comments JSON: {e}")))?;
164
165    let mut sections: Vec<String> = Vec::new();
166    for r in &reviews {
167        if r.body.trim().is_empty() {
168            continue;
169        }
170        let state = if r.state.is_empty() {
171            String::new()
172        } else {
173            format!(" ({})", r.state.to_lowercase())
174        };
175        sections.push(format!("@{}{state}: {}", login(&r.user), r.body.trim()));
176    }
177    for c in &comments {
178        if c.body.trim().is_empty() {
179            continue;
180        }
181        let loc = match (&c.path, c.line) {
182            (Some(path), Some(line)) => format!("`{path}:{line}` — "),
183            (Some(path), None) => format!("`{path}` — "),
184            _ => String::new(),
185        };
186        sections.push(format!("{loc}@{}: {}", login(&c.user), c.body.trim()));
187    }
188
189    if sections.is_empty() {
190        return Ok(String::new());
191    }
192    let mut body = format!("Review feedback from {}\n", pr.url);
193    for section in sections {
194        body.push('\n');
195        body.push_str(&section);
196        body.push('\n');
197    }
198    Ok(body)
199}
200
201/// Everything the forge needs to open a PR for a review task (DESIGN.md §8):
202/// the branch to push and the title and body of the pull request. Assembled by
203/// [`plan_pr`] once the task is proven PR-ready.
204#[derive(Debug, Clone, PartialEq, Eq)]
205pub struct PrPlan {
206    pub branch: String,
207    pub title: String,
208    pub body: String,
209}
210
211/// Validate that a task can have a PR opened from its done-time state, and if so
212/// assemble the [`PrPlan`] (DESIGN.md §8): a `review` task carrying both a branch
213/// (the work to push) and a completion summary (the PR body; the caller supplies
214/// the latest). Each gap fails naming what is missing — state, branch, or
215/// summary. Pure of I/O.
216pub fn plan_pr(task: &Task, latest_summary: Option<&str>) -> Result<PrPlan> {
217    if task.state != TaskState::Review {
218        return Err(Error::Invalid(format!(
219            "only a review task can have a PR opened from its summary; task {} is {}",
220            task.id, task.state
221        )));
222    }
223    let branch = task
224        .branch
225        .as_deref()
226        .map(str::trim)
227        .filter(|b| !b.is_empty())
228        .ok_or_else(|| {
229            Error::Invalid(format!(
230                "task {} has no branch to push — record one with `voro done --branch` or \
231                 `voro set --branch`",
232                task.id
233            ))
234        })?;
235    let body = latest_summary
236        .map(str::trim)
237        .filter(|s| !s.is_empty())
238        .ok_or_else(|| {
239            Error::Invalid(format!(
240                "task {} has no completion summary for the PR body — record one with \
241                 `voro set --summary`",
242                task.id
243            ))
244        })?;
245    Ok(PrPlan {
246        branch: branch.to_string(),
247        title: task.title.clone(),
248        body: body.to_string(),
249    })
250}
251
252#[cfg(test)]
253mod tests {
254    use super::*;
255
256    #[test]
257    fn parses_a_full_url() {
258        let pr = PrRef::parse("https://github.com/acme/widget/pull/42").unwrap();
259        assert_eq!(pr.host, "github.com");
260        assert_eq!(pr.owner, "acme");
261        assert_eq!(pr.repo, "widget");
262        assert_eq!(pr.number, 42);
263        assert_eq!(pr.nwo(), "acme/widget");
264        assert_eq!(pr.url, "https://github.com/acme/widget/pull/42");
265        assert_eq!(
266            pr.api_path("comments"),
267            "repos/acme/widget/pulls/42/comments"
268        );
269    }
270
271    #[test]
272    fn parses_url_without_scheme_and_with_extra_segments() {
273        let pr = PrRef::parse("github.com/acme/widget/pull/42/files?w=1#discussion").unwrap();
274        assert_eq!(pr.number, 42);
275        assert_eq!(pr.url, "https://github.com/acme/widget/pull/42");
276    }
277
278    #[test]
279    fn parses_the_owner_repo_shorthand() {
280        let pr = PrRef::parse("acme/widget#7").unwrap();
281        assert_eq!(pr.owner, "acme");
282        assert_eq!(pr.repo, "widget");
283        assert_eq!(pr.number, 7);
284        assert_eq!(pr.url, "https://github.com/acme/widget/pull/7");
285    }
286
287    #[test]
288    fn preserves_an_enterprise_host() {
289        let pr = PrRef::parse("https://git.example.com/acme/widget/pull/3").unwrap();
290        assert_eq!(pr.host, "git.example.com");
291        assert_eq!(pr.url, "https://git.example.com/acme/widget/pull/3");
292    }
293
294    #[test]
295    fn rejects_non_pr_references() {
296        assert!(PrRef::parse("https://github.com/acme/widget/issues/42").is_err());
297        assert!(PrRef::parse("https://github.com/acme/widget").is_err());
298        assert!(PrRef::parse("not a url").is_err());
299        assert!(PrRef::parse("acme/widget/extra#1").is_err());
300        assert!(PrRef::parse("").is_err());
301        assert!(PrRef::parse("https://github.com/acme/widget/pull/notanumber").is_err());
302    }
303
304    fn pr() -> PrRef {
305        PrRef::parse("https://github.com/acme/widget/pull/42").unwrap()
306    }
307
308    #[test]
309    fn formats_reviews_and_inline_comments() {
310        let reviews = r#"[
311            {"user": {"login": "alice"}, "state": "CHANGES_REQUESTED", "body": "Please fix the parser"},
312            {"user": {"login": "bob"}, "state": "APPROVED", "body": ""}
313        ]"#;
314        let comments = r#"[
315            {"user": {"login": "alice"}, "path": "src/lib.rs", "line": 12, "body": "off-by-one here"}
316        ]"#;
317        let body = format_review_feedback(&pr(), reviews, comments).unwrap();
318        assert!(body.contains("Review feedback from https://github.com/acme/widget/pull/42"));
319        assert!(body.contains("@alice (changes_requested): Please fix the parser"));
320        // the bare approval with no body is skipped
321        assert!(!body.contains("@bob"));
322        assert!(body.contains("`src/lib.rs:12` — @alice: off-by-one here"));
323    }
324
325    #[test]
326    fn empty_when_there_is_nothing_to_relay() {
327        let body = format_review_feedback(&pr(), "[]", "[]").unwrap();
328        assert!(body.is_empty());
329        // a lone approval with no comments is also nothing to relay
330        let reviews = r#"[{"user": {"login": "bob"}, "state": "APPROVED", "body": ""}]"#;
331        assert!(
332            format_review_feedback(&pr(), reviews, "[]")
333                .unwrap()
334                .is_empty()
335        );
336    }
337
338    #[test]
339    fn tolerates_missing_optional_fields() {
340        // a comment with no path/line/user still relays its body
341        let comments = r#"[{"body": "a general note"}]"#;
342        let body = format_review_feedback(&pr(), "[]", comments).unwrap();
343        assert!(body.contains("@unknown: a general note"));
344    }
345
346    #[test]
347    fn rejects_malformed_json() {
348        assert!(format_review_feedback(&pr(), "not json", "[]").is_err());
349        assert!(format_review_feedback(&pr(), "[]", "not json").is_err());
350    }
351
352    // --- plan_pr (DESIGN.md §8: opening a PR from a review task's summary) ---
353
354    use crate::model::Priority;
355
356    fn task(state: TaskState, branch: Option<&str>) -> Task {
357        Task {
358            id: 82,
359            project_id: 1,
360            title: "Extend pr to create the PR".into(),
361            body: String::new(),
362            priority: Priority::P1,
363            state,
364            agent: None,
365            human: false,
366            question: None,
367            pr_url: None,
368            branch: branch.map(str::to_string),
369            state_since: "2026-07-10 00:00:00".into(),
370            created_at: "2026-07-10 00:00:00".into(),
371            closed_at: None,
372        }
373    }
374
375    #[test]
376    fn plans_a_pr_from_a_review_task_with_branch_and_summary() {
377        let plan = plan_pr(
378            &task(TaskState::Review, Some("feat/pr")),
379            Some("Did the thing"),
380        )
381        .unwrap();
382        assert_eq!(plan.branch, "feat/pr");
383        assert_eq!(plan.title, "Extend pr to create the PR");
384        assert_eq!(plan.body, "Did the thing");
385    }
386
387    #[test]
388    fn plan_requires_the_review_state() {
389        for state in [
390            TaskState::Ready,
391            TaskState::Running,
392            TaskState::NeedsInput,
393            TaskState::Done,
394        ] {
395            let err = plan_pr(&task(state, Some("feat/pr")), Some("summary"))
396                .unwrap_err()
397                .to_string();
398            assert!(err.contains("review"), "{state}: {err}");
399        }
400    }
401
402    #[test]
403    fn plan_names_a_missing_branch() {
404        let err = plan_pr(&task(TaskState::Review, None), Some("summary"))
405            .unwrap_err()
406            .to_string();
407        assert!(err.contains("branch"), "{err}");
408        // a blank branch is treated as absent
409        let err = plan_pr(&task(TaskState::Review, Some("   ")), Some("summary"))
410            .unwrap_err()
411            .to_string();
412        assert!(err.contains("branch"), "{err}");
413    }
414
415    #[test]
416    fn plan_names_a_missing_summary() {
417        let err = plan_pr(&task(TaskState::Review, Some("feat/pr")), None)
418            .unwrap_err()
419            .to_string();
420        assert!(err.contains("summary"), "{err}");
421        let err = plan_pr(&task(TaskState::Review, Some("feat/pr")), Some("  "))
422            .unwrap_err()
423            .to_string();
424        assert!(err.contains("summary"), "{err}");
425    }
426}
voro_core/pr.rs

voro_core/
pr.rs
