feat: pull request review and conversation actions (A5a)

crates/client/src/lib.rs

@@ -1376,6 +1376,39 @@ mod tests {
         assert!(link.inner.pending.lock().unwrap().is_empty());
     }
 
+    /// A pull request write resent after its answer was lost could post twice, so a lost
+    /// connection fails it, and reconnecting sends nothing again.
+    #[test]
+    fn a_pull_request_action_in_flight_fails_on_disconnect_and_is_not_resent() {
+        let (to_host, outgoing) = async_channel::unbounded();
+        let (_incoming, from_host) = async_channel::unbounded();
+        let link = HostLink::new(to_host, from_host);
+        let mut action = std::pin::pin!(link.command(Command::RunPullRequestAction {
+            session_id: "one".into(),
+            key: serde_json::from_value(
+                serde_json::json!({"host": "github.com", "repository": "octo/repo", "number": 7}),
+            )
+            .unwrap(),
+            action: tcode_protocol::PullRequestAction::Comment {
+                body: "Once".into(),
+            },
+        }));
+        let mut cx = std::task::Context::from_waker(std::task::Waker::noop());
+        assert!(action.as_mut().poll(&mut cx).is_pending());
+        assert_eq!(request(&outgoing).key, None);
+        link.set_connection_state(ConnectionState::Reconnecting {
+            attempt: 1,
+            reason: None,
+        });
+        match action.as_mut().poll(&mut cx) {
+            std::task::Poll::Ready(Err(error)) => assert_eq!(error.code, "disconnected"),
+            other => panic!("the action must fail on disconnect, got {other:?}"),
+        }
+        link.set_connection_state(ConnectionState::Syncing { path: None });
+        assert!(outgoing.try_recv().is_err(), "nothing was resent");
+        assert!(link.pending_commands().is_empty());
+    }
+
     #[test]
     fn full_transport_queue_rejects_requests_without_leaving_a_waiter() {
         let (outgoing, _receiver) = outgoing::channel();

crates/core/src/project.rs

@@ -84,6 +84,9 @@ pub enum SettledOverride {
 pub struct SessionMeta {
     #[serde(default)]
     pub pull_requests: Vec<crate::pull_request::ThreadPullRequestLink>,
+    /// Reviews being written in this thread, one per pull request it shows.
+    #[serde(default, skip_serializing_if = "Vec::is_empty")]
+    pub pull_request_reviews: Vec<crate::pull_request::PullRequestReviewDraft>,
     pub id: String,
     pub title: String,
     pub provider: ProviderKind,
@@ -275,6 +278,7 @@ impl SessionMeta {
             id: uuid::Uuid::new_v4().to_string(),
             title: format!("New {} session", provider.display_name()),
             pull_requests: Vec::new(),
+            pull_request_reviews: Vec::new(),
             provider,
             profile_id: None,
             cwd,

crates/core/src/pull_request.rs

@@ -443,6 +443,253 @@ pub fn groups(links: &[ThreadPullRequestLink]) -> Vec<PullRequestGroup<'_>> {
     groups
 }
 
+/// A review being written on the host. GitHub sees none of it until it is submitted whole, so a
+/// draft outlives a client, a device and a moved head alike.
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
+pub struct PullRequestReviewDraft {
+    pub key: PullRequestKey,
+    /// The GitHub account writing it, as the conversation names it: a draft is another
+    /// account's work once the host reads as someone else, so it is kept and not shown.
+    #[serde(default, skip_serializing_if = "String::is_empty")]
+    pub account: String,
+    /// The head commit the comments are anchored at.
+    #[serde(default, skip_serializing_if = "String::is_empty")]
+    pub head: String,
+    #[serde(default, skip_serializing_if = "String::is_empty")]
+    pub body: String,
+    #[serde(default, skip_serializing_if = "Vec::is_empty")]
+    pub comments: Vec<PullRequestReviewDraftComment>,
+    /// Never reused, so an edit naming a removed comment cannot reach a later one.
+    #[serde(default)]
+    pub next_id: u64,
+    /// The last submission got no answer, so it may have been posted; set until the pull request
+    /// is read again.
+    #[serde(default, skip_serializing_if = "std::ops::Not::not")]
+    pub uncertain: bool,
+}
+
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
+pub struct PullRequestReviewDraftComment {
+    pub id: u64,
+    /// The commit whose text the comment's side shows: the base for the old side, the head for
+    /// the new.
+    pub revision: String,
+    pub path: String,
+    pub side: crate::session::ReviewSide,
+    pub start_line: u32,
+    pub end_line: u32,
+    pub body: String,
+    /// False once the head moved and its lines no longer read as they did: kept, never sent.
+    #[serde(default = "placed", skip_serializing_if = "is_placed")]
+    pub placed: bool,
+}
+
+fn placed() -> bool {
+    true
+}
+
+fn is_placed(placed: &bool) -> bool {
+    *placed
+}
+
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
+#[serde(tag = "type", content = "content", rename_all = "snake_case")]
+pub enum PullRequestReviewDraftEdit {
+    /// Lines of the diff read at `head`; a draft anchored at another head takes no new comments
+    /// until it is moved to this one.
+    AddComment {
+        head: String,
+        revision: String,
+        path: String,
+        side: crate::session::ReviewSide,
+        start_line: u32,
+        end_line: u32,
+        body: String,
+    },
+    EditComment {
+        id: u64,
+        body: String,
+    },
+    RemoveComment {
+        id: u64,
+    },
+    SetBody {
+        body: String,
+    },
+    /// Re-anchors the comments at the pull request's current head. The host applies it with
+    /// [`reanchor_review`], since only it can read whether each comment's lines changed.
+    MoveToHead,
+    Discard,
+}
+
+/// The account's draft of the pull request.
+pub fn review_draft<'a>(
+    drafts: &'a [PullRequestReviewDraft],
+    key: &PullRequestKey,
+    account: &str,
+) -> Option<&'a PullRequestReviewDraft> {
+    drafts
+        .iter()
+        .find(|draft| draft.key == *key && draft.account == account)
+}
+
+fn draft_index(
+    drafts: &[PullRequestReviewDraft],
+    key: &PullRequestKey,
+    account: &str,
+) -> Option<usize> {
+    drafts
+        .iter()
+        .position(|draft| draft.key == *key && draft.account == account)
+}
+
+/// Applies an edit to the account's draft; false when it changed nothing, was malformed, or was
+/// [`PullRequestReviewDraftEdit::MoveToHead`].
+pub fn edit_review_draft(
+    drafts: &mut Vec<PullRequestReviewDraft>,
+    key: &PullRequestKey,
+    account: &str,
+    edit: PullRequestReviewDraftEdit,
+) -> bool {
+    let index = match draft_index(drafts, key, account) {
+        Some(index) => index,
+        None if matches!(edit, PullRequestReviewDraftEdit::AddComment { .. })
+            || matches!(&edit, PullRequestReviewDraftEdit::SetBody { body } if !body.is_empty()) =>
+        {
+            drafts.push(PullRequestReviewDraft {
+                key: key.clone(),
+                account: account.to_owned(),
+                head: String::new(),
+                body: String::new(),
+                comments: Vec::new(),
+                next_id: 1,
+                uncertain: false,
+            });
+            drafts.len() - 1
+        }
+        None => return false,
+    };
+    let draft = &mut drafts[index];
+    let changed = match edit {
+        PullRequestReviewDraftEdit::AddComment {
+            head,
+            revision,
+            path,
+            side,
+            start_line,
+            end_line,
+            body,
+        } => {
+            let anchored = draft.comments.is_empty() || draft.head == head;
+            if !anchored
+                || head.is_empty()
+                || path.is_empty()
+                || start_line == 0
+                || start_line > end_line
+                || body.trim().is_empty()
+            {
+                false
+            } else {
+                draft.head = head;
+                draft.comments.push(PullRequestReviewDraftComment {
+                    id: draft.next_id,
+                    revision,
+                    path,
+                    side,
+                    start_line,
+                    end_line,
+                    body,
+                    placed: true,
+                });
+                draft.next_id += 1;
+                true
+            }
+        }
+        PullRequestReviewDraftEdit::EditComment { id, body } => {
+            match draft.comments.iter_mut().find(|comment| comment.id == id) {
+                Some(comment) if !body.trim().is_empty() && comment.body != body => {
+                    comment.body = body;
+                    true
+                }
+                _ => false,
+            }
+        }
+        PullRequestReviewDraftEdit::RemoveComment { id } => {
+            let before = draft.comments.len();
+            draft.comments.retain(|comment| comment.id != id);
+            draft.comments.len() != before
+        }
+        PullRequestReviewDraftEdit::SetBody { body } => {
+            body != std::mem::replace(&mut draft.body, body.clone())
+        }
+        PullRequestReviewDraftEdit::MoveToHead => false,
+        PullRequestReviewDraftEdit::Discard => {
+            drafts.remove(index);
+            return true;
+        }
+    };
+    if draft.body.is_empty() && draft.comments.is_empty() {
+        drafts.remove(index);
+    }
+    changed
+}
+
+/// Anchors the draft at `head`. `moved` names where each comment's lines are now, or `None` when
+/// they changed, which leaves the comment unplaced.
+pub fn reanchor_review(
+    drafts: &mut [PullRequestReviewDraft],
+    key: &PullRequestKey,
+    account: &str,
+    head: &str,
+    moved: impl Fn(&PullRequestReviewDraftComment) -> Option<String>,
+) -> bool {
+    let Some(index) = draft_index(drafts, key, account) else {
+        return false;
+    };
+    let draft = &mut drafts[index];
+    if draft.head == head {
+        return false;
+    }
+    draft.head = head.to_owned();
+    for comment in &mut draft.comments {
+        match moved(comment) {
+            Some(revision) if comment.placed => comment.revision = revision,
+            _ => comment.placed = false,
+        }
+    }
+    true
+}
+
+/// What a submission did to the draft. Once GitHub took it, its comments go by id, and its body
+/// only while it still reads as sent, since a body revised meanwhile is new work. An unanswered
+/// one marks the draft until the pull request is read again.
+pub fn submitted_review(
+    drafts: &mut Vec<PullRequestReviewDraft>,
+    key: &PullRequestKey,
+    account: &str,
+    sent: Option<(&[u64], &str)>,
+) -> bool {
+    let Some(index) = draft_index(drafts, key, account) else {
+        return false;
+    };
+    let draft = &mut drafts[index];
+    let Some((comments, body)) = sent else {
+        draft.uncertain = true;
+        return true;
+    };
+    draft
+        .comments
+        .retain(|comment| !comments.contains(&comment.id));
+    if draft.body == body {
+        draft.body.clear();
+    }
+    draft.uncertain = false;
+    if draft.body.is_empty() && draft.comments.is_empty() {
+        drafts.remove(index);
+    }
+    true
+}
+
 /// A menu visibility hint only; the host forge adapter validates and canonicalizes the target.
 pub fn is_pull_request_url(value: &str) -> bool {
     let value = value.split(['?', '#']).next().unwrap_or_default();

crates/protocol/src/command.rs

@@ -117,6 +117,18 @@ pub enum Command {
         paths: Vec<String>,
         viewed: bool,
     },
+    /// A write to a linked pull request, answered with [`CommandResponse::PullRequestAction`].
+    /// It is never retained for redelivery: a write resent after a lost answer could apply twice.
+    RunPullRequestAction {
+        session_id: String,
+        key: tcode_core::pull_request::PullRequestKey,
+        action: crate::PullRequestAction,
+    },
+    EditPullRequestReviewDraft {
+        session_id: String,
+        key: tcode_core::pull_request::PullRequestKey,
+        edit: tcode_core::pull_request::PullRequestReviewDraftEdit,
+    },
     SetProfileSecret {
         profile_id: String,
         name: String,
@@ -487,6 +499,7 @@ pub enum CommandResponse {
         key: tcode_core::pull_request::PullRequestKey,
         already_linked: bool,
     },
+    PullRequestAction(crate::PullRequestActionResult),
     ProjectId(Option<String>),
     SessionId(Option<String>),
     PendingRelaunchSection {
@@ -505,6 +518,8 @@ impl Command {
             | Self::WatchPullRequest { session_id, .. }
             | Self::SetPullRequestFilesViewed { session_id, .. }
             | Self::RefreshPullRequest { session_id, .. }
+            | Self::RunPullRequestAction { session_id, .. }
+            | Self::EditPullRequestReviewDraft { session_id, .. }
             | Self::OrchestrateTurn { session_id, .. }
             | Self::RunGitAction { session_id, .. }
             | Self::SetActiveAcpAgent { session_id, .. }
@@ -566,9 +581,10 @@ impl Command {
         )
     }
 
-    /// Idempotent controls and reads do not need retained delivery.
-    /// All other variants are retained writes, including settings assignments:
-    /// repeating an old assignment after a newer one would undo user intent.
+    /// Idempotent controls and reads do not need retained delivery, and a
+    /// pull request action must not have it. All other variants are retained
+    /// writes, including settings assignments: repeating an old assignment
+    /// after a newer one would undo user intent.
     pub fn requires_delivery_key(&self) -> bool {
         !matches!(
             self,
@@ -577,6 +593,7 @@ impl Command {
                 | Self::ShutdownAllAndFlush
                 | Self::OpenLatestSession
                 | Self::RefreshGitHubCredentials
+                | Self::RunPullRequestAction { .. }
                 | Self::RefreshProviderStatus
                 | Self::RefreshProviderUsage
                 | Self::CheckProviderVersions