25260030712

Committed 02 May 2026 07:31PM UTC coverage: 64.412% (-0.3%) from 64.699%

Build # 25260030712

Build Type

push

github

Committed by

web-flow

Commit Message

chore: Add GitHub PR review workflow (#594)

Introduces an end-to-end AI-assisted pull request review workflow,
spanning a new `jp_attachment_github` crate, extensions to `jp_github`,
new project tooling, and a dedicated `pr-reviewer` persona.

**New `jp_attachment_github` crate**

Adds a `gh://` URI attachment handler with two resource types:

- `gh:pull/N/diff` — fetches a PR's title, description, and unified diff
as a single text attachment. The diff is filtered through a configurable
exclusion list (snapshots, lockfiles, minified JS, …) so generated noise
is kept out of the LLM context. Override via `?exclude=glob1,glob2` or
disable the defaults with `?no_defaults=true`.

- `gh:pull/N/reviews` — fetches all reviews and inline comments on a PR,
rendered as structured Markdown grouped by file and anchor line. Pending
(draft) reviews and comments are attributed to "you" rather than
exposing the reviewer's login.

Both resource types accept a shortform (`gh:pull/N/diff`) that is
project-rooted to `dcdpr/jp`, or a canonical form
(`gh://owner/repo/pull/N/diff`) for any repository.

**`jp_github` extensions**

Added new models (`Side`, `ReviewState`, `Review`, `DraftReviewComment`,
`GitRef`) and new `PullsHandler` methods:

- `list_reviews` — lists all reviews on a PR.
- `create_review` — builder API that creates a draft (`PENDING`) review.
- `delete_review` — deletes a pending review.
- `add_review_thread` — appends a single inline comment to an existing
pending review via the GraphQL `addPullRequestReviewThread` mutation,
since the REST API does not support appending to an existing draft.

`PullRequest` gains `node_id`, `head`, and `base` fields required for
fetching file content at the correct SHA.

**`github_pr_review_add_comment` tool**

A new project tool that queues one inline comment at a time to the
authenticated user's pending review. Before each comment is posted, the
tool renders a syntax-highlighted snippet from the PR's HEAD file plus
the p... (continued)

Coverage Stats

539 of 1036 new or added lines in 8 files covered. (52.03%)

2 existing lines in 1 file now uncovered.

24568 of 38142 relevant lines covered (64.41%)

194.82 hits per line

Source File
Press 'n' to go to next uncovered line, 'b' for previous

67.22

/crates/jp_github/src/client.rs

use std::sync::{Arc, Mutex, OnceLock};

use reqwest::{
    Client,
    header::{ACCEPT, AUTHORIZATION, HeaderMap, HeaderName, HeaderValue, USER_AGENT},
};
use serde::{Serialize, de::DeserializeOwned};
use serde_json::Value;

use crate::{
    Error, GitHubError, Page, Result, StatusCode,
    handlers::{CurrentHandler, IssuesHandler, PullsHandler, ReposHandler, SearchHandler},
};

#[derive(Clone)]
pub struct Octocrab {
    pub(crate) inner: Arc<Inner>,
}

pub(crate) struct Inner {
    pub(crate) client: Client,
    pub(crate) api_base: String,
    pub(crate) graphql_url: String,
}

pub struct OctocrabBuilder {
    token: Option<String>,
}

impl Octocrab {
    #[must_use]
    pub fn builder() -> OctocrabBuilder {
        OctocrabBuilder { token: None }
    }

    #[must_use]
    pub fn current(&self) -> CurrentHandler {
        CurrentHandler {
            client: self.clone(),
        }
    }

    #[must_use]
    pub fn issues(&self, owner: impl Into<String>, repo: impl Into<String>) -> IssuesHandler {
        IssuesHandler {
            client: self.clone(),
            owner: owner.into(),
            repo: repo.into(),
        }
    }

    #[must_use]
    pub fn pulls(&self, owner: impl Into<String>, repo: impl Into<String>) -> PullsHandler {
        PullsHandler {
            client: self.clone(),
            owner: owner.into(),
            repo: repo.into(),
        }
    }

    #[must_use]
    pub fn repos(&self, owner: impl Into<String>, repo: impl Into<String>) -> ReposHandler {
        ReposHandler {
            client: self.clone(),
            owner: owner.into(),
            repo: repo.into(),
        }
    }

    #[must_use]
    pub fn search(&self) -> SearchHandler {
        SearchHandler {
            client: self.clone(),
        }
    }

    pub async fn graphql<T: DeserializeOwned>(&self, body: &Value) -> Result<T> {
        let request = self.inner.client.post(&self.inner.graphql_url).json(body);
        self.send_json(request).await
    }

    #[allow(clippy::unused_async)] // Keep async for octocrab API compatibility at callsites.
    pub async fn all_pages<T>(&self, page: Page<T>) -> Result<Vec<T>> {
        Ok(page.items)
    }

    pub(crate) async fn get_json<T: DeserializeOwned>(
        &self,
        path: &str,
        query: &[(String, String)],
    ) -> Result<T> {
        let request = self
            .inner
            .client
            .get(format!("{}{}", self.inner.api_base, path))
            .query(query);

        self.send_json(request).await
    }

    pub(crate) async fn post_json<T: DeserializeOwned, B: Serialize>(
        &self,
        path: &str,
        body: &B,
    ) -> Result<T> {
        let request = self
            .inner
            .client
            .post(format!("{}{}", self.inner.api_base, path))
            .json(body);

        self.send_json(request).await
    }

    /// GET a path with a custom `Accept` header, returning the raw response
    /// body as a string. Used for endpoints that vary their content type by
    /// `Accept` (e.g. PR diffs via `application/vnd.github.diff`).
    pub(crate) async fn get_with_accept(&self, path: &str, accept: &str) -> Result<String> {
        let request = self
            .inner
            .client
            .get(format!("{}{}", self.inner.api_base, path))
            .header(reqwest::header::ACCEPT, accept);

        let response = request.send().await?;
        let status = response.status();
        let body = response.text().await?;

        if status.is_success() {
            return Ok(body);
        }

        let message = serde_json::from_str::<Value>(&body)
            .ok()
            .and_then(|value| {
                value
                    .get("message")
                    .and_then(Value::as_str)
                    .map(str::to_owned)
            })
            .unwrap_or_else(|| format!("request failed with status {}", status.as_u16()));

        Err(Error::GitHub {
            source: GitHubError {
                status_code: StatusCode::new(status.as_u16()),
                message,
            },
            body: Some(body),
        })
    }

    pub(crate) async fn delete_no_content(&self, path: &str) -> Result<()> {
        let request = self
            .inner
            .client
            .delete(format!("{}{}", self.inner.api_base, path));

        let response = request.send().await?;
        let status = response.status();

        if status.is_success() {
            return Ok(());
        }

        let body = response.text().await?;
        let message = serde_json::from_str::<Value>(&body)
            .ok()
            .and_then(|value| {
                value
                    .get("message")
                    .and_then(Value::as_str)
                    .map(str::to_owned)
            })
            .unwrap_or_else(|| format!("request failed with status {}", status.as_u16()));

        Err(Error::GitHub {
            source: GitHubError {
                status_code: StatusCode::new(status.as_u16()),
                message,
            },
            body: Some(body),
        })
    }

    pub(crate) async fn get_paginated<T: DeserializeOwned>(
        &self,
        path: &str,
        mut query: Vec<(String, String)>,
        per_page: u8,
    ) -> Result<Vec<T>> {
        let mut page = 1_u64;
        let mut out = vec![];

        loop {
            query.retain(|(key, _)| key != "page" && key != "per_page");
            query.push(("per_page".to_owned(), per_page.to_string()));
            query.push(("page".to_owned(), page.to_string()));

            let items: Vec<T> = self.get_json(path, &query).await?;
            let count = items.len();
            out.extend(items);

            if count == 0 || count < usize::from(per_page) {
                break;
            }

            page += 1;
        }

        Ok(out)
    }

    pub(crate) async fn get_search_paginated<T: DeserializeOwned>(
        &self,
        path: &str,
        mut query: Vec<(String, String)>,
        per_page: u8,
    ) -> Result<Vec<T>> {
        #[derive(serde::Deserialize)]
        struct SearchResponse<T> {
            items: Vec<T>,
        }

        let mut page = 1_u64;
        let mut out = vec![];

        loop {
            query.retain(|(key, _)| key != "page" && key != "per_page");
            query.push(("per_page".to_owned(), per_page.to_string()));
            query.push(("page".to_owned(), page.to_string()));

            let response: SearchResponse<T> = self.get_json(path, &query).await?;
            let count = response.items.len();
            out.extend(response.items);

            if count == 0 || count < usize::from(per_page) {
                break;
            }

            page += 1;
        }

        Ok(out)
    }

    async fn send_json<T: DeserializeOwned>(&self, request: reqwest::RequestBuilder) -> Result<T> {
        let response = request.send().await?;
        let status = response.status();
        let body = response.text().await?;

        if !status.is_success() {
            let message = serde_json::from_str::<Value>(&body)
                .ok()
                .and_then(|value| {
                    value
                        .get("message")
                        .and_then(Value::as_str)
                        .map(str::to_owned)
                })
                .unwrap_or_else(|| format!("request failed with status {}", status.as_u16()));

            return Err(Error::GitHub {
                source: GitHubError {
                    status_code: StatusCode::new(status.as_u16()),
                    message,
                },
                body: Some(body),
            });
        }

        serde_json::from_str(&body).map_err(Into::into)
    }

    #[cfg(test)]
    pub(crate) fn with_base_url(base_url: &str, token: Option<&str>) -> Self {
        let client = build_http_client(token).expect("test client to build");

        Self {
            inner: Arc::new(Inner {
                client,
                api_base: base_url.to_owned(),
                graphql_url: format!("{base_url}/graphql"),
            }),
        }
    }
}

impl OctocrabBuilder {
    #[must_use]
    pub fn personal_token(mut self, token: impl Into<String>) -> Self {
        self.token = Some(token.into());
        self
    }

    pub fn build(self) -> Result<Octocrab> {
        let client = build_http_client(self.token.as_deref())?;

        Ok(Octocrab {
            inner: Arc::new(Inner {
                client,
                api_base: "https://api.github.com".to_owned(),
                graphql_url: "https://api.github.com/graphql".to_owned(),
            }),
        })
    }
}

fn build_http_client(token: Option<&str>) -> Result<Client> {
    let mut headers = HeaderMap::new();
    headers.insert(USER_AGENT, HeaderValue::from_static("jp-github"));
    headers.insert(
        ACCEPT,
        HeaderValue::from_static("application/vnd.github+json"),
    );
    headers.insert(
        HeaderName::from_static("x-github-api-version"),
        HeaderValue::from_static("2022-11-28"),
    );

    if let Some(token) = token {
        let token = format!("Bearer {token}");
        headers.insert(AUTHORIZATION, HeaderValue::from_str(&token)?);
    }

    Client::builder()
        .default_headers(headers)
        .build()
        .map_err(|error| Error::Build(format!("{error:#}")))
}

static INSTANCE: OnceLock<Mutex<Option<Octocrab>>> = OnceLock::new();

pub fn initialise(client: Octocrab) {
    let mutex = INSTANCE.get_or_init(|| Mutex::new(None));
    if let Ok(mut lock) = mutex.lock() {
        *lock = Some(client);
    }
}

#[must_use]
/// Returns the globally initialized GitHub client.
///
/// # Panics
///
/// Panics if [`initialise`] has not been called successfully.
pub fn instance() -> Octocrab {
    let mutex = INSTANCE.get_or_init(|| Mutex::new(None));
    let lock = mutex.lock().ok();

    match lock.and_then(|guard| guard.as_ref().cloned()) {
        Some(client) => client,
        None => panic!("jp_github client not initialized"),
    }
}

1	use std::sync::{Arc, Mutex, OnceLock};
2
3	use reqwest::{
4	Client,
5	header::{ACCEPT, AUTHORIZATION, HeaderMap, HeaderName, HeaderValue, USER_AGENT},
6	};
7	use serde::{Serialize, de::DeserializeOwned};
8	use serde_json::Value;
9
10	use crate::{
11	Error, GitHubError, Page, Result, StatusCode,
12	handlers::{CurrentHandler, IssuesHandler, PullsHandler, ReposHandler, SearchHandler},
13	};
14
15	#[derive(Clone)]
16	pub struct Octocrab {
17	pub(crate) inner: Arc<Inner>,
18	}
19
20	pub(crate) struct Inner {
21	pub(crate) client: Client,
22	pub(crate) api_base: String,
23	pub(crate) graphql_url: String,
24	}
25
26	pub struct OctocrabBuilder {
27	token: Option<String>,
28	}
29
30	impl Octocrab {
31	#[must_use]
32	pub fn builder() -> OctocrabBuilder {	×
33	OctocrabBuilder { token: None }	×
34	}	×
35
36	#[must_use]
37	pub fn current(&self) -> CurrentHandler {	2✔
38	CurrentHandler {	2✔
39	client: self.clone(),	2✔
40	}	2✔
41	}	2✔
42
43	#[must_use]
44	pub fn issues(&self, owner: impl Into<String>, repo: impl Into<String>) -> IssuesHandler {	2✔
45	IssuesHandler {	2✔
46	client: self.clone(),	2✔
47	owner: owner.into(),	2✔
48	repo: repo.into(),	2✔
49	}	2✔
50	}	2✔
51
52	#[must_use]
53	pub fn pulls(&self, owner: impl Into<String>, repo: impl Into<String>) -> PullsHandler {	7✔
54	PullsHandler {	7✔
55	client: self.clone(),	7✔
56	owner: owner.into(),	7✔
57	repo: repo.into(),	7✔
58	}	7✔
59	}	7✔
60
61	#[must_use]
62	pub fn repos(&self, owner: impl Into<String>, repo: impl Into<String>) -> ReposHandler {	2✔
63	ReposHandler {	2✔
64	client: self.clone(),	2✔
65	owner: owner.into(),	2✔
66	repo: repo.into(),	2✔
67	}	2✔
68	}	2✔
69
70	#[must_use]
71	pub fn search(&self) -> SearchHandler {	1✔
72	SearchHandler {	1✔
73	client: self.clone(),	1✔
74	}	1✔
75	}	1✔
76
77	pub async fn graphql<T: DeserializeOwned>(&self, body: &Value) -> Result<T> {	4✔
78	let request = self.inner.client.post(&self.inner.graphql_url).json(body);	4✔
79	self.send_json(request).await	4✔
80	}	4✔
81
82	#[allow(clippy::unused_async)] // Keep async for octocrab API compatibility at callsites.
83	pub async fn all_pages<T>(&self, page: Page<T>) -> Result<Vec<T>> {	5✔
84	Ok(page.items)	5✔
85	}	5✔
86
87	pub(crate) async fn get_json<T: DeserializeOwned>(	9✔
88	&self,	9✔
89	path: &str,	9✔
90	query: &[(String, String)],	9✔
91	) -> Result<T> {	9✔
92	let request = self	9✔
93	.inner	9✔
94	.client	9✔
95	.get(format!("{}{}", self.inner.api_base, path))	9✔
96	.query(query);	9✔
97
98	self.send_json(request).await	9✔
99	}	9✔
100
101	pub(crate) async fn post_json<T: DeserializeOwned, B: Serialize>(	2✔
102	&self,	2✔
103	path: &str,	2✔
104	body: &B,	2✔
105	) -> Result<T> {	2✔
106	let request = self	2✔
107	.inner	2✔
108	.client	2✔
109	.post(format!("{}{}", self.inner.api_base, path))	2✔
110	.json(body);	2✔
111
112	self.send_json(request).await	2✔
113	}	2✔
114
115	/// GET a path with a custom `Accept` header, returning the raw response
116	/// body as a string. Used for endpoints that vary their content type by
117	/// `Accept` (e.g. PR diffs via `application/vnd.github.diff`).
NEW 118	pub(crate) async fn get_with_accept(&self, path: &str, accept: &str) -> Result<String> {	×
NEW 119	let request = self	×
NEW 120	.inner	×
NEW 121	.client	×
NEW 122	.get(format!("{}{}", self.inner.api_base, path))	×
NEW 123	.header(reqwest::header::ACCEPT, accept);	×
124
NEW 125	let response = request.send().await?;	×
NEW 126	let status = response.status();	×
NEW 127	let body = response.text().await?;	×
128
NEW 129	if status.is_success() {	×
NEW 130	return Ok(body);	×
NEW 131	}	×
132
NEW 133	let message = serde_json::from_str::<Value>(&body)	×
NEW 134	.ok()	×
NEW 135	.and_then(\|value\| {	×
NEW 136	value	×
NEW 137	.get("message")	×
NEW 138	.and_then(Value::as_str)	×
NEW 139	.map(str::to_owned)	×
NEW 140	})	×
NEW 141	.unwrap_or_else(\|\| format!("request failed with status {}", status.as_u16()));	×
142
NEW 143	Err(Error::GitHub {	×
NEW 144	source: GitHubError {	×
NEW 145	status_code: StatusCode::new(status.as_u16()),	×
NEW 146	message,	×
NEW 147	},	×
NEW 148	body: Some(body),	×
NEW 149	})	×
NEW 150	}	×
151
152	pub(crate) async fn delete_no_content(&self, path: &str) -> Result<()> {	1✔
153	let request = self	1✔
154	.inner	1✔
155	.client	1✔
156	.delete(format!("{}{}", self.inner.api_base, path));	1✔
157
158	let response = request.send().await?;	1✔
159	let status = response.status();	1✔
160
161	if status.is_success() {	1✔
162	return Ok(());	1✔
NEW 163	}	×
164
NEW 165	let body = response.text().await?;	×
NEW 166	let message = serde_json::from_str::<Value>(&body)	×
NEW 167	.ok()	×
NEW 168	.and_then(\|value\| {	×
NEW 169	value	×
NEW 170	.get("message")	×
NEW 171	.and_then(Value::as_str)	×
NEW 172	.map(str::to_owned)	×
NEW 173	})	×
NEW 174	.unwrap_or_else(\|\| format!("request failed with status {}", status.as_u16()));	×
175
NEW 176	Err(Error::GitHub {	×
NEW 177	source: GitHubError {	×
NEW 178	status_code: StatusCode::new(status.as_u16()),	×
NEW 179	message,	×
NEW 180	},	×
NEW 181	body: Some(body),	×
NEW 182	})	×
183	}	1✔
184
185	pub(crate) async fn get_paginated<T: DeserializeOwned>(	4✔
186	&self,	4✔
187	path: &str,	4✔
188	mut query: Vec<(String, String)>,	4✔
189	per_page: u8,	4✔
190	) -> Result<Vec<T>> {	4✔
191	let mut page = 1_u64;	4✔
192	let mut out = vec![];	4✔
193
194	loop {
195	query.retain(\|(key, _)\| key != "page" && key != "per_page");	5✔
196	query.push(("per_page".to_owned(), per_page.to_string()));	5✔
197	query.push(("page".to_owned(), page.to_string()));	5✔
198
199	let items: Vec<T> = self.get_json(path, &query).await?;	5✔
200	let count = items.len();	5✔
201	out.extend(items);	5✔
202
203	if count == 0 \|\| count < usize::from(per_page) {	5✔
204	break;	4✔
205	}	1✔
206
207	page += 1;	1✔
208	}
209
210	Ok(out)	4✔
211	}	4✔
212
213	pub(crate) async fn get_search_paginated<T: DeserializeOwned>(	1✔
214	&self,	1✔
215	path: &str,	1✔
216	mut query: Vec<(String, String)>,	1✔
217	per_page: u8,	1✔
218	) -> Result<Vec<T>> {	1✔
219	#[derive(serde::Deserialize)]
220	struct SearchResponse<T> {
221	items: Vec<T>,
222	}
223
224	let mut page = 1_u64;	1✔
225	let mut out = vec![];	1✔
226
227	loop {
228	query.retain(\|(key, _)\| key != "page" && key != "per_page");	1✔
229	query.push(("per_page".to_owned(), per_page.to_string()));	1✔
230	query.push(("page".to_owned(), page.to_string()));	1✔
231
232	let response: SearchResponse<T> = self.get_json(path, &query).await?;	1✔
233	let count = response.items.len();	1✔
234	out.extend(response.items);	1✔
235
236	if count == 0 \|\| count < usize::from(per_page) {	1✔
237	break;	1✔
238	}	×
239
240	page += 1;	×
241	}
242
243	Ok(out)	1✔
244	}	1✔
245
246	async fn send_json<T: DeserializeOwned>(&self, request: reqwest::RequestBuilder) -> Result<T> {	15✔
247	let response = request.send().await?;	15✔
248	let status = response.status();	15✔
249	let body = response.text().await?;	15✔
250
251	if !status.is_success() {	15✔
252	let message = serde_json::from_str::<Value>(&body)	1✔
253	.ok()	1✔
254	.and_then(\|value\| {	1✔
255	value	1✔
256	.get("message")	1✔
257	.and_then(Value::as_str)	1✔
258	.map(str::to_owned)	1✔
259	})	1✔
260	.unwrap_or_else(\|\| format!("request failed with status {}", status.as_u16()));	1✔
261
262	return Err(Error::GitHub {	1✔
263	source: GitHubError {	1✔
264	status_code: StatusCode::new(status.as_u16()),	1✔
265	message,	1✔
266	},	1✔
267	body: Some(body),	1✔
268	});	1✔
269	}	14✔
270
271	serde_json::from_str(&body).map_err(Into::into)	14✔
272	}	15✔
273
274	#[cfg(test)]
275	pub(crate) fn with_base_url(base_url: &str, token: Option<&str>) -> Self {	15✔
276	let client = build_http_client(token).expect("test client to build");	15✔
277
278	Self {	15✔
279	inner: Arc::new(Inner {	15✔
280	client,	15✔
281	api_base: base_url.to_owned(),	15✔
282	graphql_url: format!("{base_url}/graphql"),	15✔
283	}),	15✔
284	}	15✔
285	}	15✔
286	}
287
288	impl OctocrabBuilder {
289	#[must_use]
290	pub fn personal_token(mut self, token: impl Into<String>) -> Self {	×
291	self.token = Some(token.into());	×
292	self	×
293	}	×
294
295	pub fn build(self) -> Result<Octocrab> {	×
296	let client = build_http_client(self.token.as_deref())?;	×
297
298	Ok(Octocrab {	×
299	inner: Arc::new(Inner {	×
300	client,	×
301	api_base: "https://api.github.com".to_owned(),	×
302	graphql_url: "https://api.github.com/graphql".to_owned(),	×
303	}),	×
304	})	×
305	}	×
306	}
307
308	fn build_http_client(token: Option<&str>) -> Result<Client> {	15✔
309	let mut headers = HeaderMap::new();	15✔
310	headers.insert(USER_AGENT, HeaderValue::from_static("jp-github"));	15✔
311	headers.insert(	15✔
312	ACCEPT,	15✔
313	HeaderValue::from_static("application/vnd.github+json"),	15✔
314	);
315	headers.insert(	15✔
316	HeaderName::from_static("x-github-api-version"),	15✔
317	HeaderValue::from_static("2022-11-28"),	15✔
318	);
319
320	if let Some(token) = token {	15✔
321	let token = format!("Bearer {token}");	1✔
322	headers.insert(AUTHORIZATION, HeaderValue::from_str(&token)?);	1✔
323	}	14✔
324
325	Client::builder()	15✔
326	.default_headers(headers)	15✔
327	.build()	15✔
328	.map_err(\|error\| Error::Build(format!("{error:#}")))	15✔
329	}	15✔
330
331	static INSTANCE: OnceLock<Mutex<Option<Octocrab>>> = OnceLock::new();
332
333	pub fn initialise(client: Octocrab) {	×
334	let mutex = INSTANCE.get_or_init(\|\| Mutex::new(None));	×
335	if let Ok(mut lock) = mutex.lock() {	×
336	*lock = Some(client);	×
337	}	×
338	}	×
339
340	#[must_use]
341	/// Returns the globally initialized GitHub client.
342	///
343	/// # Panics
344	///
345	/// Panics if [`initialise`] has not been called successfully.
346	pub fn instance() -> Octocrab {	×
347	let mutex = INSTANCE.get_or_init(\|\| Mutex::new(None));	×
348	let lock = mutex.lock().ok();	×
349
350	match lock.and_then(\|guard\| guard.as_ref().cloned()) {	×
351	Some(client) => client,	×
352	None => panic!("jp_github client not initialized"),	×
353	}
354	}	×

dcdpr / jp / 25260030712

Source File Press 'n' to go to next uncovered line, 'b' for previous

Source File
Press 'n' to go to next uncovered line, 'b' for previous