Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion crates/adapter-zarvis/src/interactive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3563,7 +3563,14 @@ async fn run_one_tool(
let mut denied = false;
loop {
term.approval(&call.name, &args_summary, tool.risk(), allow_auto_review);
match wait_for_approval(inbox, &call.id, approval_mode).await {
let mode_before_approval = *approval_mode;
let approval_outcome = wait_for_approval(inbox, &call.id, approval_mode).await;
if *approval_mode != mode_before_approval {
emit.emit(SessionEvent::ApprovalModeChanged {
mode: *approval_mode,
});
}
match approval_outcome {
ApprovalOutcome::Stop => {
emit.emit(SessionEvent::ToolApprovalResolved {
call_id: call.id.clone(),
Expand Down
1 change: 1 addition & 0 deletions crates/cli/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ fn chat_scroll_kind(ev: &SessionEvent) -> ChatScrollKind {
| SessionEvent::EditorState { .. }
| SessionEvent::ClientCommand { .. }
| SessionEvent::ToolApprovalResolved { .. }
| SessionEvent::ApprovalModeChanged { .. }
| SessionEvent::AgentStatus(_) => ChatScrollKind::Hidden,
SessionEvent::Message { role, text }
if should_render_chat_message_for_scroll(*role, text) =>
Expand Down
1 change: 1 addition & 0 deletions crates/cli/src/matrix_rain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,7 @@ fn word_for_event(event: &SessionEvent) -> Option<(&'static str, FlashTone, u8)>
| SessionEvent::Diff { .. }
| SessionEvent::Pty { .. }
| SessionEvent::PtyResize { .. }
| SessionEvent::ApprovalModeChanged { .. }
| SessionEvent::EditorState { .. }
| SessionEvent::BrowserPreview(_)
| SessionEvent::UiPanel(_)
Expand Down
5 changes: 5 additions & 0 deletions crates/cli/src/ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5610,6 +5610,7 @@ fn chat_event_kind(ev: &SessionEvent) -> ChatEventKind {
// `slash::COMMANDS[id].render` and shows SystemNote breadcrumbs.
| SessionEvent::ClientCommand { .. }
| SessionEvent::ToolApprovalResolved { .. }
| SessionEvent::ApprovalModeChanged { .. }
| SessionEvent::AgentStatus(_) => ChatEventKind::Hidden,
SessionEvent::Message { role, text } if should_render_chat_message(*role, text) => {
if *role == MessageRole::Assistant {
Expand Down Expand Up @@ -5728,6 +5729,7 @@ fn format_chat_event_body(theme: &Theme, ev: &SessionEvent) -> Vec<Span<'static>
| SessionEvent::EditorState { .. }
| SessionEvent::ClientCommand { .. }
| SessionEvent::ToolApprovalResolved { .. }
| SessionEvent::ApprovalModeChanged { .. }
| SessionEvent::AgentStatus(_) => Vec::new(),
SessionEvent::Message { role, text } => {
let role_label = match role {
Expand Down Expand Up @@ -6427,6 +6429,9 @@ pub fn short_event_label(ev: &SessionEvent) -> String {
SessionEvent::Done { exit_code } => format!("done (exit {exit_code})"),
SessionEvent::Pty { data } => format!("pty: {} bytes", data.len()),
SessionEvent::ToolApprovalRequest { tool, .. } => format!("approve? {tool}"),
SessionEvent::ApprovalModeChanged { mode } => {
format!("approval-mode {}", mode.badge().unwrap_or("manual"))
}
SessionEvent::TaskStart { tool, call_id, .. } => format!("task-start {tool} {call_id}"),
SessionEvent::TaskBackgrounded { call_id } => format!("task-bg {call_id}"),
SessionEvent::TaskEnd { call_id, ok, .. } => format!("task-end {call_id} ok={ok}"),
Expand Down
60 changes: 60 additions & 0 deletions crates/daemon/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2015,6 +2015,18 @@ impl SessionManager {
}));
return;
}
// ApprovalModeChanged updates durable per-session state. The state
// notification is enough for clients; do not record a transcript row.
if let SessionEvent::ApprovalModeChanged { mode } = &event {
if let Err(e) = self.persist_approval_mode(entry, *mode).await {
tracing::warn!(
session = %entry.id,
error = ?e,
"persist approval mode from adapter event failed"
);
}
return;
}
// PTY events take a fast path: append to the on-disk pty.log + a
// live broadcast. A copy was also appended to the transcript above
// as an ordering marker. Replay reads back from `pty.log` directly
Expand Down Expand Up @@ -2102,6 +2114,7 @@ impl SessionManager {
| SessionEvent::ToolApprovalRequest { .. }
// Transient; handled by the broadcast-only fast path above.
| SessionEvent::ToolApprovalResolved { .. }
| SessionEvent::ApprovalModeChanged { .. }
| SessionEvent::TaskStart { .. }
| SessionEvent::TaskBackgrounded { .. }
| SessionEvent::TaskEnd { .. }
Expand Down Expand Up @@ -3998,6 +4011,53 @@ mod tests {
);
}

/// Inline PTY approval prompts can change the approval mode locally
/// inside the adapter (`a` / `f`). The adapter reports that state change
/// back to the daemon with `ApprovalModeChanged`; the daemon must update
/// the session summary so modelines and other clients stop showing the
/// stale mode, without recording a transcript row.
#[tokio::test]
async fn approval_mode_changed_updates_summary_without_transcript_row() {
use tempfile::tempdir;

let tmp = tempdir().expect("tempdir");
let storage =
Arc::new(crate::storage::Storage::new(tmp.path().join("data")).expect("storage"));
let config = Arc::new(crate::config::Config::default());
let (mgr, _remote_rx, _restart_rx) =
SessionManager::new(storage.clone(), config, tmp.path().join("run"))
.await
.expect("session manager");

let id = "sapprovalmode";
let entry = synthetic_entry(id, agentd_protocol::SessionKind::User, 0);
mgr.sessions.write().await.insert(id.into(), entry.clone());

mgr.handle_event(
&entry,
SessionEvent::ApprovalModeChanged {
mode: agentd_protocol::ApprovalMode::UnsafeAuto,
},
)
.await;

let summary = storage.load_summary(id).expect("summary");
assert_eq!(
summary.approval_mode,
agentd_protocol::ApprovalMode::UnsafeAuto
);
let transcript = storage
.read_transcript(id, 0, None)
.expect("read transcript");
assert!(
!transcript
.events
.iter()
.any(|e| matches!(e.event, SessionEvent::ApprovalModeChanged { .. })),
"ApprovalModeChanged must not be written to the transcript"
);
}

/// Regression for the post-#69 "all sessions go to `done` after
/// graceful daemon restart" bug: when `shutdown_adapters` is in
/// flight, any `SessionEvent::Done` or `AdapterMessage::Closed`
Expand Down
6 changes: 6 additions & 0 deletions crates/protocol/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,12 @@ pub enum SessionEvent {
ToolApprovalResolved {
call_id: String,
},
/// Adapter changed the session's approval mode internally, typically
/// because the user answered an inline PTY approval prompt with an
/// action that changes future approval behavior.
ApprovalModeChanged {
mode: ApprovalMode,
},
/// Tool lifecycle: adapter started running a tool. Carries the
/// canonical `call_id` (unlike [`ToolUse`] which doesn't) so the
/// daemon's per-session task registry can match this against the
Expand Down
2 changes: 2 additions & 0 deletions specs/0015-approval-modes.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ Approval prompts expose `a` for `auto_review` and `f` for `unsafe_auto`. `unsafe

When a session renders its own inline approval prompt, clients should not also open a global minibuffer approval prompt for the same request. The user's approval keystrokes should go to the session that asked for approval, and other sessions should not lose input focus because a background session needs a decision.

When an inline approval prompt changes the session's future approval mode, the adapter must report that change back to the daemon so all clients render the current mode from the shared session summary.

## Reason

A boolean automode conflated two different needs: high-throughput trusted operation and model-mediated review. Naming the modes separately lets users choose a guarded middle ground without hiding the risk of fully bypassing approvals.
Expand Down
Loading