From deb1a24f7d6d387c183ab2727ca6b2a3f490dd19 Mon Sep 17 00:00:00 2001 From: Alejandro Perez Date: Tue, 7 Jul 2026 17:30:25 +0100 Subject: [PATCH 1/4] docs: reviewer chat design spec --- cockpit-docs/REVIEWER_CHAT.md | 97 +++++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 cockpit-docs/REVIEWER_CHAT.md diff --git a/cockpit-docs/REVIEWER_CHAT.md b/cockpit-docs/REVIEWER_CHAT.md new file mode 100644 index 0000000..6dd6c2c --- /dev/null +++ b/cockpit-docs/REVIEWER_CHAT.md @@ -0,0 +1,97 @@ +# Reviewer Chat — design spec + +**Date:** 2026-07-06 · **Status:** approved for implementation + +A multi-turn chat where the reviewer talks to the pre-review agent about the PR +under review. The agent already knows the diff, intent, findings, and recap; it +answers questions and can draft review comments the reviewer adopts through the +existing guarded flow. + +## Decisions (locked with the user) + +- **Role:** answer + draft comments. Read-only on code; it never edits, and any + outward post rides the existing adopt → guarded-submit path (§9). +- **Placement:** workspace-level docked panel, toggled by an "✦ Ask agent" + header button, rendered over whatever tab is active (Briefing / Diff / CI). + One conversation, reachable from anywhere in the review. +- **Persistence:** one thread per PR, persisted with the review; **cleared when + the head moves**, alongside `review_findings` / `distilled_intent` + (answers about superseded code go stale). +- **Agent model:** stateless per-turn spawn. Each turn is a fresh one-shot + `claude` in the review's worktree; the prompt is assembled from the review + context + the prior transcript + the new question. Our store owns the + transcript. (Not a resumable CLI session — that hides conversation state in + CLI-managed files we can't cleanly persist or replay.) +- **Exploration:** free. Read-only worktree tools (`Read`/`Grep`/`Glob`), no + tight tool budget, best-grounded answers. No `Write`/`Edit` → no permission + prompts. + +## Data model (`cockpit-core`) + +```rust +struct ChatMessage { + id: ChatMessageId, // newtype + role: ChatRole, // enum User | Agent + text: String, // markdown + drafts: Vec, // agent turns only; usually empty + created_at: SystemTime, +} +struct DraftComment { path, range: (u32,u32), side: DiffSide, body: String } +``` + +`Review.chat: Vec` — `#[serde(default)]`, TS bindings `T | null` +where optional (never `ts(optional)`). Added to +`clear_head_anchored_annotations` so a new push clears it with the findings. + +## Prompt + +New `AgentMode`-adjacent prompt assembler `assemble_chat_prompt(ChatInput)`: +frames the agent as the read-only reviewer, embeds intent + findings + diff + +recap + the reviewer's local comments + GitHub conversation, then the prior +transcript, then the new question. Instructs: answer concisely, ground in the +diff, say when unsure; you may read files read-only; to draft a comment write +`{drafts:[{path,line_start,line_end,side,body}]}` to ``; never +edit code. Deterministic assembly with a golden test. + +## Streaming & lifecycle + +- **Command** `send_chat_message(pr, text)`: appends the user message, spawns + the turn, streams the answer, on completion parses the draft transport and + appends the agent message, persists. +- **Dedicated `chat-stream` event** `{ pr, message_id, delta }` — separate from + `agent-event` so chat never collides with the AgentPanel timeline. +- Turns are **transient spawns, not attached to `review.agent`**: they don't + appear in the fleet roster, don't count against `max_parallel_agents`, and + never touch the gate. +- One in-flight turn per PR (send disabled while streaming); a Stop cancels it. +- Fail-open (§0.1): a spawn/parse failure surfaces an error message in the + thread and never blocks the review. + +## Frontend + +- `ChatPanel` (workspace-level), toggled from the ReviewWorkspace header; + docked right, resizable (reuse the preview-rail width pattern), open/closed + persisted. User bubbles right; agent bubbles left with the brand rail; + answers via the shared safe ``; draft cards inline with an + "Add to review" button reusing the finding-adoption path (`add_comment`). +- One global `chat-stream` listener appends deltas to the in-flight message. + +## Guardrails + +- Read-only tools only; no code mutation from chat. +- Drafts are proposals; posting to GitHub still requires explicit adopt + + confirmed Mirror/Submit-review (§9). +- Chat is ephemeral like comments (§0.4-adjacent): head move clears it. + +## Testing + +- Rust: `ChatMessage`/`DraftComment` serde round-trip; `assemble_chat_prompt` + golden; draft-transport parse (reuse findings parser shape); head-move clears + chat; transient spawn does not set `review.agent`. +- TS: chat store slice (append user, stream deltas, finalize agent, adopt + draft); ChatPanel render (bubbles, draft card, streaming/disabled states). + +## Out of scope (v1) + +- Resumable CLI sessions; chat on the plan gate; chat acting on code; sharing a + thread to GitHub. Revisit if wanted. From 5bb1c4377cdc84c5c136197c8faa8bac56928a48 Mon Sep 17 00:00:00 2001 From: Alejandro Perez Date: Tue, 7 Jul 2026 18:19:26 +0100 Subject: [PATCH 2/4] =?UTF-8?q?docs:=20reviewer=20chat=20spec=20=E2=80=94?= =?UTF-8?q?=20inline=20drafts,=20epoch=20ms,=20turn=20registry,=20caps?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- cockpit-docs/REVIEWER_CHAT.md | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/cockpit-docs/REVIEWER_CHAT.md b/cockpit-docs/REVIEWER_CHAT.md index 6dd6c2c..b867f66 100644 --- a/cockpit-docs/REVIEWER_CHAT.md +++ b/cockpit-docs/REVIEWER_CHAT.md @@ -23,8 +23,9 @@ existing guarded flow. transcript. (Not a resumable CLI session — that hides conversation state in CLI-managed files we can't cleanly persist or replay.) - **Exploration:** free. Read-only worktree tools (`Read`/`Grep`/`Glob`), no - tight tool budget, best-grounded answers. No `Write`/`Edit` → no permission - prompts. + tight tool budget, best-grounded answers. The spawn hard-disallows + `Write`/`Edit`/`NotebookEdit`/`Bash`/`Task` (`--disallowedTools`), so the + turn is provably read-only and never triggers a permission prompt. ## Data model (`cockpit-core`) @@ -34,7 +35,7 @@ struct ChatMessage { role: ChatRole, // enum User | Agent text: String, // markdown drafts: Vec, // agent turns only; usually empty - created_at: SystemTime, + created_at_epoch_ms: u64, // repo precedent (TrajectorySummary), ts-safe } struct DraftComment { path, range: (u32,u32), side: DiffSide, body: String } ``` @@ -47,17 +48,24 @@ where optional (never `ts(optional)`). Added to New `AgentMode`-adjacent prompt assembler `assemble_chat_prompt(ChatInput)`: frames the agent as the read-only reviewer, embeds intent + findings + diff + -recap + the reviewer's local comments + GitHub conversation, then the prior -transcript, then the new question. Instructs: answer concisely, ground in the -diff, say when unsure; you may read files read-only; to draft a comment write -`{drafts:[{path,line_start,line_end,side,body}]}` to ``; never -edit code. Deterministic assembly with a golden test. +recap (capped) + the reviewer's local comments + GitHub conversation, then the +prior transcript (last 40 messages), then the new question. Instructs: answer +concisely, ground in the diff, say when unsure; you may read files read-only; +to draft a comment emit a fenced JSON block +`{"drafts":[{path,line_start,line_end,side,body}]}` **inline in the answer** +(no file transport — keeps the turn write-free); never edit code. Cockpit +parses the block out of the final text, renders the drafts as adopt-able +cards, and shows the remaining text as the answer. Deterministic assembly with +a golden test. ## Streaming & lifecycle - **Command** `send_chat_message(pr, text)`: appends the user message, spawns - the turn, streams the answer, on completion parses the draft transport and - appends the agent message, persists. + the turn, streams the answer, on completion parses the inline drafts block + out of the text and appends the agent message, persists. +- **Turn registry:** `AppState.chat_turns: Mutex>` — one + in-flight turn per PR; `cancel_chat_turn(pr)` kills it; a 5-minute hard + timeout kills a hung turn and appends an error message. - **Dedicated `chat-stream` event** `{ pr, message_id, delta }` — separate from `agent-event` so chat never collides with the AgentPanel timeline. - Turns are **transient spawns, not attached to `review.agent`**: they don't From 1f3b8541c7e3fd21c870fd64e82bd24e9c92d8f2 Mon Sep 17 00:00:00 2001 From: Alejandro Perez Date: Tue, 7 Jul 2026 18:43:16 +0100 Subject: [PATCH 3/4] =?UTF-8?q?core+app:=20reviewer=20chat=20=E2=80=94=20a?= =?UTF-8?q?sk=20the=20agent=20about=20the=20review,=20adopt=20drafted=20co?= =?UTF-8?q?mments?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements cockpit-docs/REVIEWER_CHAT.md: a workspace-level 'Ask agent' panel with one persistent thread per PR (cleared when the head moves). Each turn is a stateless one-shot spawn in the review's worktree, provably read-only (--disallowedTools Write/Edit/NotebookEdit/Bash/Task), transient (no session, no fleet row, no gate contact), streamed over dedicated chat-stream/chat-complete events with a per-PR turn registry and a 5-minute hard timeout. Draft comments come back inline as a {"drafts":[...]} block parsed out of the answer and rendered as adopt-able cards feeding the existing guarded comment path (§9). --- app/src-tauri/src/commands/chat.rs | 405 ++++++++++++++++++ app/src-tauri/src/commands/mod.rs | 1 + app/src-tauri/src/lib.rs | 15 + app/src-tauri/src/state.rs | 9 + app/src/App.tsx | 37 ++ app/src/bindings/ChatMessage.ts | 37 ++ app/src/bindings/ChatMessageId.ts | 6 + app/src/bindings/ChatRole.ts | 6 + app/src/bindings/DraftComment.ts | 27 ++ app/src/bindings/Review.ts | 11 +- app/src/components/ChatPanel.test.tsx | 125 ++++++ app/src/components/ChatPanel.tsx | 386 +++++++++++++++++ app/src/components/ReviewWorkspace.tsx | 41 +- app/src/lib/chat-panel.test.ts | 34 ++ app/src/lib/chat-panel.ts | 60 +++ app/src/lib/finding-comment.test.ts | 27 ++ app/src/lib/finding-comment.ts | 23 + app/src/store.test.ts | 74 ++++ app/src/store.ts | 78 ++++ app/src/test/fixtures.ts | 1 + crates/cockpit-core/src/adapters/agent.rs | 112 +++++ crates/cockpit-core/src/adapters/github.rs | 2 + crates/cockpit-core/src/chat.rs | 276 ++++++++++++ crates/cockpit-core/src/gate.rs | 1 + crates/cockpit-core/src/kickoff.rs | 3 + crates/cockpit-core/src/lib.rs | 1 + crates/cockpit-core/src/model.rs | 66 +++ crates/cockpit-core/src/persist.rs | 1 + crates/cockpit-core/src/prompt.rs | 348 ++++++++++++++- crates/cockpit-core/src/restack.rs | 1 + crates/cockpit-core/src/store.rs | 1 + .../cockpit-core/tests/batch_fan_out_e2e.rs | 1 + crates/cockpit-core/tests/e2e_round_trip.rs | 1 + crates/cockpit-core/tests/fix_loop_e2e.rs | 1 + .../cockpit-core/tests/golden/chat_prompt.txt | 59 +++ 35 files changed, 2273 insertions(+), 4 deletions(-) create mode 100644 app/src-tauri/src/commands/chat.rs create mode 100644 app/src/bindings/ChatMessage.ts create mode 100644 app/src/bindings/ChatMessageId.ts create mode 100644 app/src/bindings/ChatRole.ts create mode 100644 app/src/bindings/DraftComment.ts create mode 100644 app/src/components/ChatPanel.test.tsx create mode 100644 app/src/components/ChatPanel.tsx create mode 100644 app/src/lib/chat-panel.test.ts create mode 100644 app/src/lib/chat-panel.ts create mode 100644 crates/cockpit-core/src/chat.rs create mode 100644 crates/cockpit-core/tests/golden/chat_prompt.txt diff --git a/app/src-tauri/src/commands/chat.rs b/app/src-tauri/src/commands/chat.rs new file mode 100644 index 0000000..e4ea056 --- /dev/null +++ b/app/src-tauri/src/commands/chat.rs @@ -0,0 +1,405 @@ +//! Reviewer-chat commands — the multi-turn "Ask the reviewer" panel. +//! +//! Design: `cockpit-docs/REVIEWER_CHAT.md`. Each turn is a stateless one-shot +//! spawn in the review's worktree, provably read-only (`--disallowedTools`), +//! never attached to `Review::agent` (no fleet row, no `max_parallel_agents` +//! pressure, no Stop-hook session), streamed to the frontend over dedicated +//! `chat-stream` / `chat-complete` events so it can never collide with the +//! AgentPanel timeline. + +use std::sync::Arc; +use std::time::{Duration, SystemTime, UNIX_EPOCH}; + +use tauri::{Emitter, State}; +use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; + +use cockpit_core::adapters::agent::{self, SpawnConfig}; +use cockpit_core::adapters::agent_stream; +use cockpit_core::chat::parse_chat_reply; +use cockpit_core::config::Config; +use cockpit_core::model::{ChatMessage, ChatMessageId, ChatRole, PrRef, Review}; +use cockpit_core::prompt::{ChatInput, assemble_chat_prompt}; + +use crate::error::CommandError; +use crate::state::AppState; + +/// Hard wall-clock budget for one chat turn; a hung turn is killed and +/// surfaces as an error message so the PR's chat is never blocked forever. +const CHAT_TURN_TIMEOUT: Duration = Duration::from_secs(300); + +/// Tools denied to every chat turn. Deny rules outrank allows in the Claude +/// CLI, so the reviewer chat is *provably* read-only — and therefore can never +/// raise a permission prompt. +const CHAT_DENIED_TOOLS: &[&str] = &["Write", "Edit", "NotebookEdit", "Bash", "Task"]; + +/// A `chat-stream` event: one text delta of the in-flight answer. +#[derive(Debug, Clone, serde::Serialize)] +struct ChatStreamPayload { + /// PR ref the turn belongs to. + pr: String, + /// Id the finished agent message will carry. + message_id: String, + /// Appended answer text. + delta: String, +} + +/// A `chat-complete` event: the turn finished (the message is in the review). +#[derive(Debug, Clone, serde::Serialize)] +struct ChatCompletePayload { + /// PR ref the turn belonged to. + pr: String, + /// Id of the appended agent message. + message_id: String, + /// False when the turn failed (spawn error, timeout, or empty output). + ok: bool, +} + +/// Wall-clock now in epoch milliseconds (0 on a pre-epoch clock). +fn now_ms() -> u64 { + SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|d| u64::try_from(d.as_millis()).unwrap_or(u64::MAX)) + .unwrap_or(0) +} + +/// Send one reviewer-chat message and start the agent turn answering it. +/// +/// Appends the user message to the review's thread immediately (the returned +/// `Review` carries it), then drives the turn on a background task: stream +/// deltas via `chat-stream`, and on completion parse the reply (inline drafts +/// block extracted by [`parse_chat_reply`]), append the agent message, and +/// emit `chat-complete`. One turn per PR at a time. +/// +/// Fail-open (Invariant §0.1): every failure path appends an `error: true` +/// agent message rather than wedging the thread. +#[tauri::command] +pub async fn send_chat_message( + state: State<'_, Arc>, + app_handle: tauri::AppHandle, + pr: String, + text: String, +) -> Result { + let question = text.trim().to_owned(); + if question.is_empty() { + return Err(CommandError { + message: "Cannot send an empty message".into(), + }); + } + let pr_ref = PrRef::new(&pr); + let review = state.reviews.get(&pr_ref).ok_or_else(|| CommandError { + message: format!("Review not found: {pr}"), + })?; + + // One in-flight turn per PR. Registered with a placeholder pid (0) BEFORE + // any await so two rapid sends cannot both pass the check; the real pid + // replaces it right after spawn. + { + let mut turns = state.chat_turns.lock().expect("chat turns lock poisoned"); + if turns.contains_key(&pr) { + return Err(CommandError { + message: "A chat turn is already running for this review".into(), + }); + } + turns.insert(pr.clone(), 0); + } + + // Everything below must unregister on failure — a small guard tidies the + // early-return paths. + let result = start_chat_turn(&state, &app_handle, &pr, &pr_ref, &review, question).await; + if result.is_err() { + state + .chat_turns + .lock() + .expect("chat turns lock poisoned") + .remove(&pr); + } + result +} + +/// The body of [`send_chat_message`] after the turn slot is claimed. +async fn start_chat_turn( + state: &State<'_, Arc>, + app_handle: &tauri::AppHandle, + pr: &str, + pr_ref: &PrRef, + review: &Review, + question: String, +) -> Result { + let config = Config::load()?; + + // The transcript replayed to the agent is the thread BEFORE this question + // (the question is its own prompt section). + let transcript = review.chat.clone(); + + // Recap markdown, when one exists on disk (best-effort context). + let recap_mdx: Option = match cockpit_core::config::recap_dir_for_pr(pr_ref) { + Ok(dir) => { + let sha = review.recap_sha.clone(); + tokio::task::spawn_blocking(move || cockpit_core::recap::read_recap(&dir, sha).mdx) + .await + .unwrap_or(None) + } + Err(_) => None, + }; + + let prompt = assemble_chat_prompt(&ChatInput { + title: &review.title, + body: &review.body, + issue: review.issue.as_str(), + diff: &review.diff, + intent: review.distilled_intent.as_deref(), + findings: &review.review_findings, + recap_mdx: recap_mdx.as_deref(), + comments: &review.comments, + conversation: &review.conversation, + transcript: &transcript, + question: &question, + }); + + // Materialize the worktree (same path pre-review uses) so the agent can + // read the actual repository, not just the diff. + let worktree = super::ensure_worktree_for_review(state, pr_ref).await?; + + let mut spawn_config = SpawnConfig::from_config(&config).disallow_tools(CHAT_DENIED_TOOLS); + if let Some(model) = config + .review_model + .as_deref() + .filter(|m| !m.trim().is_empty()) + { + spawn_config = spawn_config.with_model(model); + } + + // Append the user's message before spawning, so even a spawn failure + // leaves the question (and the error reply) in the thread. + let user_message = ChatMessage { + id: ChatMessageId::new(format!("cm-{}", uuid::Uuid::new_v4())), + role: ChatRole::User, + text: question, + drafts: Vec::new(), + error: false, + created_at_epoch_ms: now_ms(), + }; + state.reviews.update(pr_ref, |r| { + r.chat.push(user_message.clone()); + }); + + let message_id = format!("cm-{}", uuid::Uuid::new_v4()); + + let turn = agent::spawn_chat_turn(&worktree, &prompt, &spawn_config) + .await + .map_err(|e| { + // Spawn failed: record the failure in the thread (fail-open). + append_agent_message( + state, + pr_ref, + &message_id, + format!("The reviewer agent could not start: {e}"), + true, + ); + CommandError { + message: format!("failed to spawn chat turn: {e}"), + } + })?; + + state + .chat_turns + .lock() + .expect("chat turns lock poisoned") + .insert(pr.to_owned(), turn.pid); + + // Drive the turn to completion in the background; the command returns the + // review with the user message appended so the UI updates instantly. + let task_state = Arc::clone(state.inner()); + let task_handle = app_handle.clone(); + let task_pr = pr.to_owned(); + let task_pr_ref = pr_ref.clone(); + tauri::async_runtime::spawn(async move { + drive_chat_turn( + task_state, + task_handle, + task_pr, + task_pr_ref, + message_id, + turn, + ) + .await; + }); + + state.reviews.get(pr_ref).ok_or_else(|| CommandError { + message: format!("Review not found: {pr}"), + }) +} + +/// Append an agent message to a review's thread (used by every finish path). +fn append_agent_message( + state: &AppState, + pr_ref: &PrRef, + message_id: &str, + text: String, + error: bool, +) { + let reply = if error { + cockpit_core::chat::ChatReply { + text, + drafts: Vec::new(), + } + } else { + parse_chat_reply(&text) + }; + let message = ChatMessage { + id: ChatMessageId::new(message_id), + role: ChatRole::Agent, + text: reply.text, + drafts: reply.drafts, + error, + created_at_epoch_ms: now_ms(), + }; + state.reviews.update(pr_ref, |r| { + r.chat.push(message); + }); +} + +/// Read the turn's stdout to completion, streaming text deltas, then append +/// the final agent message and emit `chat-complete`. +async fn drive_chat_turn( + state: Arc, + app_handle: tauri::AppHandle, + pr: String, + pr_ref: PrRef, + message_id: String, + mut turn: agent::ChatTurn, +) { + let stdout = turn.child.stdout.take(); + let outcome = match stdout { + Some(stdout) => { + let read = read_turn_output(&app_handle, &pr, &message_id, stdout, &turn.log_path); + match tokio::time::timeout(CHAT_TURN_TIMEOUT, read).await { + Ok(text) => TurnOutcome::Finished(text), + Err(_) => { + // Hung turn: kill it so the child doesn't linger. + let _ = agent::kill_agent(turn.pid).await; + TurnOutcome::TimedOut + } + } + } + None => TurnOutcome::NoStdout, + }; + // Reap the child (best-effort; it exited or was just killed). + let _ = turn.child.wait().await; + + state + .chat_turns + .lock() + .expect("chat turns lock poisoned") + .remove(&pr); + + let (text, error) = match outcome { + TurnOutcome::Finished(text) if !text.trim().is_empty() => (text, false), + TurnOutcome::Finished(_) => ( + "The reviewer agent produced no answer (the turn may have been stopped).".to_owned(), + true, + ), + TurnOutcome::TimedOut => ( + "The reviewer agent timed out after 5 minutes and was stopped.".to_owned(), + true, + ), + TurnOutcome::NoStdout => ( + "The reviewer agent produced no output stream.".to_owned(), + true, + ), + }; + let ok = !error; + append_agent_message(&state, &pr_ref, &message_id, text, error); + + let _ = app_handle.emit("chat-complete", ChatCompletePayload { pr, message_id, ok }); +} + +/// How a chat turn's read phase ended. +enum TurnOutcome { + /// Stream ended; carries the final answer text (possibly empty). + Finished(String), + /// The 5-minute budget elapsed. + TimedOut, + /// The child had no stdout pipe (should not happen). + NoStdout, +} + +/// Consume the turn's JSONL stdout: tee to the log, emit a `chat-stream` +/// delta per assistant text block, and return the final answer text +/// (`Complete.result_text` when present, else the accumulated blocks). +async fn read_turn_output( + app_handle: &tauri::AppHandle, + pr: &str, + message_id: &str, + stdout: tokio::process::ChildStdout, + log_path: &std::path::Path, +) -> String { + let log_file = tokio::fs::OpenOptions::new() + .create(true) + .append(true) + .open(log_path) + .await; + let mut log_writer = log_file.ok().map(tokio::io::BufWriter::new); + + let mut lines = BufReader::new(stdout).lines(); + let mut accumulated = String::new(); + let mut final_text: Option = None; + + while let Ok(Some(line)) = lines.next_line().await { + if let Some(ref mut writer) = log_writer { + let _ = writer.write_all(line.as_bytes()).await; + let _ = writer.write_all(b"\n").await; + let _ = writer.flush().await; + } + match agent_stream::parse_stream_line(&line) { + Some(agent_stream::Event::Text { content }) => { + if !accumulated.is_empty() { + accumulated.push_str("\n\n"); + } + accumulated.push_str(&content); + let _ = app_handle.emit( + "chat-stream", + ChatStreamPayload { + pr: pr.to_owned(), + message_id: message_id.to_owned(), + delta: content, + }, + ); + } + Some(agent_stream::Event::Complete { result_text, .. }) + if !result_text.trim().is_empty() => + { + final_text = Some(result_text); + } + _ => {} + } + } + if let Some(ref mut writer) = log_writer { + let _ = writer.flush().await; + } + + final_text.unwrap_or(accumulated) +} + +/// Stop the in-flight chat turn for a review (explicit user action). +/// +/// Kills the turn's process; the driver task then sees EOF and settles the +/// thread with whatever the turn produced (or an error message). +#[tauri::command] +pub async fn cancel_chat_turn( + state: State<'_, Arc>, + pr: String, +) -> Result<(), CommandError> { + let pid = { + let turns = state.chat_turns.lock().expect("chat turns lock poisoned"); + turns.get(&pr).copied() + }; + match pid { + // A placeholder (0) means the spawn is still in flight; nothing to kill + // yet — the send path's own error handling covers it. + Some(0) | None => Ok(()), + Some(pid) => agent::kill_agent(pid).await.map_err(|e| CommandError { + message: format!("failed to stop chat turn: {e}"), + }), + } +} diff --git a/app/src-tauri/src/commands/mod.rs b/app/src-tauri/src/commands/mod.rs index 342d8a6..31422ca 100644 --- a/app/src-tauri/src/commands/mod.rs +++ b/app/src-tauri/src/commands/mod.rs @@ -3,6 +3,7 @@ //! Commands parse params, call core, and map results through //! [`CommandError`](crate::error::CommandError). All logic lives in core. +pub mod chat; pub mod shell; use std::collections::HashSet; diff --git a/app/src-tauri/src/lib.rs b/app/src-tauri/src/lib.rs index 92bc8dd..67e657c 100644 --- a/app/src-tauri/src/lib.rs +++ b/app/src-tauri/src/lib.rs @@ -319,6 +319,8 @@ pub fn run() { Ok(()) }) .invoke_handler(tauri::generate_handler![ + commands::chat::send_chat_message, + commands::chat::cancel_chat_turn, commands::list_reviews, commands::get_frontier, commands::get_review, @@ -792,6 +794,9 @@ fn clear_head_anchored_annotations(review: &mut Review) { review.review_findings.clear(); review.distilled_intent = None; review.distilled_headline = None; + // The chat thread answers questions about the previous head; stale answers + // are worse than no answers (cockpit-docs/REVIEWER_CHAT.md). + review.chat.clear(); } /// Extract the PR number from a PR URL or reference string. @@ -1426,6 +1431,7 @@ mod tests { distilled_intent: None, distilled_headline: None, recap_sha: None, + chat: Vec::new(), } } @@ -1525,6 +1531,14 @@ mod tests { }]; review.distilled_intent = Some("intent".into()); review.recap_sha = Some("headsha9".into()); + review.chat = vec![cockpit_core::model::ChatMessage { + id: cockpit_core::model::ChatMessageId::new("cm-1"), + role: cockpit_core::model::ChatRole::User, + text: "q".into(), + drafts: Vec::new(), + error: false, + created_at_epoch_ms: 1, + }]; clear_head_anchored_annotations(&mut review); @@ -1536,6 +1550,7 @@ mod tests { review.distilled_intent.is_none(), "intent cleared on refresh" ); + assert!(review.chat.is_empty(), "chat cleared on refresh"); assert_eq!( review.recap_sha.as_deref(), Some("headsha9"), diff --git a/app/src-tauri/src/state.rs b/app/src-tauri/src/state.rs index e282673..9e5bdf0 100644 --- a/app/src-tauri/src/state.rs +++ b/app/src-tauri/src/state.rs @@ -51,6 +51,14 @@ pub struct AppState { /// is only ever held for trivial map lookups/inserts — never across an /// `.await`. pub lsp_bridges: Mutex>, + + /// In-flight reviewer-chat turns, PR ref → agent pid. + /// + /// Enforces one turn per PR and gives `cancel_chat_turn` its kill target. + /// Chat turns are transient (never attached to `Review::agent`), so this + /// registry is their only handle. The `std::sync::Mutex` is held only for + /// trivial map operations — never across an `.await`. + pub chat_turns: Mutex>, } impl AppState { @@ -67,6 +75,7 @@ impl AppState { completion_tx, permission_broker, lsp_bridges: Mutex::new(HashMap::new()), + chat_turns: Mutex::new(HashMap::new()), } } } diff --git a/app/src/App.tsx b/app/src/App.tsx index 2220612..6ba31ae 100644 --- a/app/src/App.tsx +++ b/app/src/App.tsx @@ -97,6 +97,20 @@ interface AgentEventEnvelope { readonly event: AgentStreamEvent; } +/** Payload of the reviewer-chat `"chat-stream"` event (hand-typed, no ts-rs). */ +interface ChatStreamPayload { + readonly pr: string; + readonly message_id: string; + readonly delta: string; +} + +/** Payload of the reviewer-chat `"chat-complete"` event (hand-typed, no ts-rs). */ +interface ChatCompletePayload { + readonly pr: string; + readonly message_id: string; + readonly ok: boolean; +} + type ReviewTab = "my-prs" | "review-requests" | "all"; const SIDEBAR_COLLAPSED_KEY = "cockpit-sidebar-collapsed"; @@ -603,7 +617,30 @@ function App() { }, ); + // Reviewer chat: stream deltas into the in-flight bubble; on completion + // settle the turn (the persisted agent message lands via review refresh). + const unlistenChatStream = listen( + "chat-stream", + (event) => { + useAppStore + .getState() + .applyChatDelta(event.payload.pr, event.payload.delta); + }, + ); + const unlistenChatComplete = listen( + "chat-complete", + (event) => { + void useAppStore.getState().completeChatTurn(event.payload.pr); + }, + ); + return () => { + void unlistenChatStream.then((f) => { + f(); + }); + void unlistenChatComplete.then((f) => { + f(); + }); void unlisten.then((f) => { f(); }); diff --git a/app/src/bindings/ChatMessage.ts b/app/src/bindings/ChatMessage.ts new file mode 100644 index 0000000..a85ed41 --- /dev/null +++ b/app/src/bindings/ChatMessage.ts @@ -0,0 +1,37 @@ +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. +import type { ChatMessageId } from "./ChatMessageId"; +import type { ChatRole } from "./ChatRole"; +import type { DraftComment } from "./DraftComment"; + +/** + * One message in a review's chat thread with the reviewer agent. + * + * The thread is head-anchored context like findings and the distilled + * intent: it is cleared when the head moves, since answers about superseded + * code go stale. + */ +export type ChatMessage = { +/** + * Locally-unique message id. + */ +id: ChatMessageId, +/** + * Who authored the message. + */ +role: ChatRole, +/** + * The message text (markdown; draft blocks already stripped). + */ +text: string, +/** + * Draft comments proposed by the agent in this turn (usually empty). + */ +drafts: Array, +/** + * Whether this agent message reports a failed turn (spawn/timeout/parse). + */ +error: boolean, +/** + * Wall-clock creation time in epoch milliseconds. + */ +created_at_epoch_ms: number, }; diff --git a/app/src/bindings/ChatMessageId.ts b/app/src/bindings/ChatMessageId.ts new file mode 100644 index 0000000..79b7aec --- /dev/null +++ b/app/src/bindings/ChatMessageId.ts @@ -0,0 +1,6 @@ +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. + +/** + * Locally-unique identifier for a [`ChatMessage`] in a review's thread. + */ +export type ChatMessageId = string; diff --git a/app/src/bindings/ChatRole.ts b/app/src/bindings/ChatRole.ts new file mode 100644 index 0000000..b595cf9 --- /dev/null +++ b/app/src/bindings/ChatRole.ts @@ -0,0 +1,6 @@ +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. + +/** + * Who authored a [`ChatMessage`] in a review's chat thread. + */ +export type ChatRole = "User" | "Agent"; diff --git a/app/src/bindings/DraftComment.ts b/app/src/bindings/DraftComment.ts new file mode 100644 index 0000000..1aafb6b --- /dev/null +++ b/app/src/bindings/DraftComment.ts @@ -0,0 +1,27 @@ +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. +import type { DiffSide } from "./DiffSide"; + +/** + * A review comment the chat agent proposes, for the reviewer to adopt. + * + * A draft is a *proposal only*: it becomes a real [`Comment`] exclusively + * through the reviewer's explicit adopt action, and reaches GitHub only via + * the guarded mirror / submit-review flows (Invariant §9). + */ +export type DraftComment = { +/** + * Path relative to the repo root. + */ +path: string, +/** + * Inclusive start and end line in the current head. + */ +range: [number, number], +/** + * Which side of the diff the range refers to. + */ +side: DiffSide, +/** + * The proposed comment body (markdown). + */ +body: string, }; diff --git a/app/src/bindings/Review.ts b/app/src/bindings/Review.ts index b4fed4e..83fea0b 100644 --- a/app/src/bindings/Review.ts +++ b/app/src/bindings/Review.ts @@ -1,5 +1,6 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. import type { AgentRun } from "./AgentRun"; +import type { ChatMessage } from "./ChatMessage"; import type { CiSummary } from "./CiSummary"; import type { Comment } from "./Comment"; import type { ConversationItem } from "./ConversationItem"; @@ -165,4 +166,12 @@ distilled_headline: string | null, * tab can nudge a regenerate. Deliberately NOT cleared when the head moves * (unlike `review_findings` / `distilled_intent`) so staleness is derivable. */ -recap_sha: string | null, }; +recap_sha: string | null, +/** + * The chat thread between the reviewer and the read-only reviewer agent. + * + * Head-anchored like `review_findings` / `distilled_intent`: cleared when + * the head moves. `#[serde(default)]` keeps legacy persisted reviews + * loading with an empty thread. + */ +chat: Array, }; diff --git a/app/src/components/ChatPanel.test.tsx b/app/src/components/ChatPanel.test.tsx new file mode 100644 index 0000000..9d7ba02 --- /dev/null +++ b/app/src/components/ChatPanel.test.tsx @@ -0,0 +1,125 @@ +import { describe, it, expect, vi } from "vitest"; +import { render, screen, fireEvent } from "@testing-library/react"; +import { makeReview } from "../test/fixtures"; +import { mockInvoke, callsFor } from "../test/tauri-mock"; +import type { ChatMessage } from "../bindings/ChatMessage"; + +vi.mock("@tauri-apps/api/core", async () => { + const mock = await import("../test/tauri-mock"); + return { invoke: mock.invoke }; +}); +vi.mock("@tauri-apps/api/event", async () => { + const mock = await import("../test/tauri-mock"); + return { listen: mock.listen }; +}); +vi.mock("@tauri-apps/plugin-opener", () => ({ + openUrl: vi.fn(() => Promise.resolve()), +})); + +const { useAppStore } = await import("../store"); +const { ChatPanel } = await import("./ChatPanel"); + +const agentMessage: ChatMessage = { + id: "cm-a1", + role: "Agent", + text: "The flag gates every entry point.", + drafts: [ + { + path: "src/a.py", + range: [57, 57], + side: "New", + body: "Use the no-retry policy.", + }, + ], + error: false, + created_at_epoch_ms: 2, +}; + +const userMessage: ChatMessage = { + id: "cm-u1", + role: "User", + text: "What does the flag gate?", + drafts: [], + error: false, + created_at_epoch_ms: 1, +}; + +describe("ChatPanel", () => { + it("renders the thread and adopts a draft via add_comment", () => { + const review = { + ...makeReview({ gate_state: "InReview" }), + chat: [userMessage, agentMessage], + }; + useAppStore.setState({ + activeReview: review, + view: { kind: "diff", pr: review.pr }, + }); + mockInvoke("add_comment", () => review); + + render( undefined} />); + + expect(screen.getByText("What does the flag gate?")).toBeDefined(); + expect( + screen.getByText("The flag gates every entry point."), + ).toBeDefined(); + + fireEvent.click(screen.getByRole("button", { name: "Add to review" })); + const calls = callsFor("add_comment"); + expect(calls).toHaveLength(1); + expect(calls[0]?.args).toMatchObject({ + file: "src/a.py", + lineStart: 57, + lineEnd: 57, + body: "Use the no-retry policy.", + side: "New", + }); + }); + + it("shows the adopted state instead of the button once a comment matches", () => { + const base = makeReview({ gate_state: "InReview" }); + const review = { + ...base, + chat: [agentMessage], + comments: [ + { + id: "c-1", + anchor: { + DiffLine: { + path: "src/a.py", + range: [57, 57] as [number, number], + side: "New" as const, + }, + }, + body: "Use the no-retry policy.", + origin: "Local" as const, + }, + ], + }; + render( undefined} />); + expect(screen.queryByRole("button", { name: "Add to review" })).toBeNull(); + expect(screen.getByText("Added ✓")).toBeDefined(); + }); + + it("disables send on empty input and marks error turns", () => { + const review = { + ...makeReview({ gate_state: "InReview" }), + chat: [ + { + ...agentMessage, + id: "cm-err", + drafts: [], + error: true, + text: "The reviewer agent timed out after 5 minutes and was stopped.", + }, + ], + }; + render( undefined} />); + const send = screen.getByRole("button", { name: "Send message" }); + expect((send as HTMLButtonElement).disabled).toBe(true); + expect( + screen.getByText( + "The reviewer agent timed out after 5 minutes and was stopped.", + ), + ).toBeDefined(); + }); +}); diff --git a/app/src/components/ChatPanel.tsx b/app/src/components/ChatPanel.tsx new file mode 100644 index 0000000..0e322c7 --- /dev/null +++ b/app/src/components/ChatPanel.tsx @@ -0,0 +1,386 @@ +/** + * The reviewer-chat panel — "Ask the reviewer" (cockpit-docs/REVIEWER_CHAT.md). + * + * A workspace-level docked panel: one persistent conversation per PR with the + * read-only reviewer agent, reachable from whatever tab is active. User + * bubbles right; agent bubbles left with the brand rail (the agent-voice + * identity); answers render through the shared safe Markdown. Agent turns can + * carry draft comments, shown as adopt-able cards that flow into the same + * guarded adopt → Mirror/Submit path as findings (§9). + * + * The panel owns its width (drag the left divider; persisted) and renders a + * streaming bubble while a turn is in flight (`chat-stream` deltas from the + * store). Send is disabled during a turn; Stop kills it. + */ + +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; +import { Loader2, Square, X } from "lucide-react"; +import type { Review } from "../bindings/Review"; +import type { ChatMessage } from "../bindings/ChatMessage"; +import type { DraftComment } from "../bindings/DraftComment"; +import { useAppStore } from "../store"; +import { Markdown } from "./Markdown"; +import { AgentMark } from "@/lib/agent-event"; +import { isDraftAdopted } from "@/lib/finding-comment"; +import { + CHAT_DEFAULT_WIDTH, + CHAT_KEY_STEP, + CHAT_MAX_WIDTH, + CHAT_MIN_WIDTH, + clampChatWidth, + loadChatWidth, + saveChatWidth, +} from "@/lib/chat-panel"; +import { cn } from "@/lib/utils"; + +interface ChatPanelProps { + readonly review: Review; + readonly onClose: () => void; +} + +/** One draft-comment card inside an agent bubble. */ +function DraftCard({ + draft, + adopted, + canAdopt, + onAdopt, +}: { + readonly draft: DraftComment; + readonly adopted: boolean; + /** False when the gate is not InReview (commenting unavailable). */ + readonly canAdopt: boolean; + readonly onAdopt: (draft: DraftComment) => void; +}) { + const location = + draft.range[0] > 0 + ? `${draft.path}:${String(draft.range[0])}${ + draft.range[1] !== draft.range[0] ? `–${String(draft.range[1])}` : "" + }` + : draft.path; + return ( +
+
+ Draft comment +
+
+ {location} +
+
+ +
+
+ {adopted ? ( + + Added ✓ + + ) : ( + + )} +
+
+ ); +} + +/** One chat bubble. */ +function Bubble({ + message, + review, + canAdopt, + onAdopt, +}: { + readonly message: ChatMessage; + readonly review: Review; + readonly canAdopt: boolean; + readonly onAdopt: (draft: DraftComment) => void; +}) { + const isUser = message.role === "User"; + return ( +
+
+ {isUser ? "You" : "Reviewer"} +
+
+ {message.error ? ( + {message.text} + ) : ( + + )} + {message.drafts.map((draft, i) => ( + + ))} +
+
+ ); +} + +/** The docked reviewer-chat panel (self-sizing; drag its left edge). */ +export function ChatPanel({ review, onClose }: ChatPanelProps) { + const pending = useAppStore((s) => s.chatPending[review.pr] === true); + const streamText = useAppStore((s) => s.chatStreamText[review.pr] ?? ""); + const sendChatMessage = useAppStore((s) => s.sendChatMessage); + const cancelChatTurn = useAppStore((s) => s.cancelChatTurn); + const addComment = useAppStore((s) => s.addComment); + + const [input, setInput] = useState(""); + const [width, setWidth] = useState(loadChatWidth); + const drag = useRef<{ startX: number; startWidth: number } | null>(null); + const scrollRef = useRef(null); + + const canAdopt = review.gate_state === "InReview"; + + // Keep the newest message in view as the thread grows or streams. + useEffect(() => { + const el = scrollRef.current; + if (el !== null) el.scrollTop = el.scrollHeight; + }, [review.chat.length, streamText, pending]); + + const handleSend = useCallback(() => { + const text = input.trim(); + if (text === "" || pending) return; + setInput(""); + void sendChatMessage(review.pr, text); + }, [input, pending, review.pr, sendChatMessage]); + + const handleAdopt = useCallback( + (draft: DraftComment) => { + void addComment( + draft.path, + draft.range[0], + draft.range[1], + draft.body, + draft.side, + ); + }, + [addComment], + ); + + // -- Left-edge resize (mirrors the board preview rail's interaction). -- + const onDragStart = useCallback( + (e: React.PointerEvent) => { + e.preventDefault(); + e.currentTarget.setPointerCapture(e.pointerId); + drag.current = { startX: e.clientX, startWidth: width }; + }, + [width], + ); + const onDragMove = useCallback((e: React.PointerEvent) => { + const d = drag.current; + if (d === null) return; + setWidth(clampChatWidth(d.startWidth + (d.startX - e.clientX))); + }, []); + const onDragEnd = useCallback((e: React.PointerEvent) => { + const d = drag.current; + if (d === null) return; + drag.current = null; + e.currentTarget.releasePointerCapture(e.pointerId); + saveChatWidth(clampChatWidth(d.startWidth + (d.startX - e.clientX))); + }, []); + const onDividerKey = useCallback( + (e: React.KeyboardEvent) => { + const delta = + e.key === "ArrowLeft" + ? CHAT_KEY_STEP + : e.key === "ArrowRight" + ? -CHAT_KEY_STEP + : null; + if (delta === null) return; + e.preventDefault(); + setWidth((w) => { + const next = clampChatWidth(w + delta); + saveChatWidth(next); + return next; + }); + }, + [], + ); + const resetWidth = useCallback(() => { + setWidth(CHAT_DEFAULT_WIDTH); + saveChatWidth(CHAT_DEFAULT_WIDTH); + }, []); + + const thread = review.chat; + const empty = thread.length === 0 && !pending; + const contextLine = useMemo(() => { + const parts = ["diff"]; + if (review.review_findings.length > 0) + parts.push(`${String(review.review_findings.length)} findings`); + if (review.distilled_intent !== null) parts.push("intent"); + if (review.recap_sha !== null) parts.push("recap"); + if (review.comments.length > 0) parts.push("your comments"); + return parts.join(" · "); + }, [review]); + + return ( + <> +
+