commit ae2b22ebf90dd9ecb2dcee8833f37b63b428ec75
parent 5dce90475d13a9cc16c369023eb0c444d3711087
Author: William Casarin <jb55@jb55.com>
Date: Wed, 18 Feb 2026 11:07:31 -0800
refactor: unify permission resolution into PermissionTracker::resolve()
handle_permission_response() and handle_question_response() each
independently updated req.response, permissions.responded, and the
oneshot channel — three redundant state locations that had to stay in
sync. When 950b43012 fixed permission responses to always publish nostr
events, the Q&A path was missed, requiring a catch-up fix (4c05d075).
Add PermissionTracker::resolve() as the single method that updates all
resolution state. Both handlers now funnel through it, so future
changes to resolution logic only need to touch one place.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Diffstat:
2 files changed, 66 insertions(+), 60 deletions(-)
diff --git a/crates/notedeck_dave/src/session.rs b/crates/notedeck_dave/src/session.rs
@@ -6,7 +6,8 @@ use crate::agent_status::AgentStatus;
use crate::config::AiMode;
use crate::git_status::GitStatusCache;
use crate::messages::{
- CompactionInfo, PermissionResponse, QuestionAnswer, SessionInfo, SubagentStatus,
+ AnswerSummary, CompactionInfo, PermissionResponse, PermissionResponseType, QuestionAnswer,
+ SessionInfo, SubagentStatus,
};
use crate::session_events::ThreadingState;
use crate::{DaveApiResponse, Message};
@@ -65,6 +66,48 @@ impl PermissionTracker {
!self.pending.is_empty()
}
+ /// Resolve a permission request. This is the ONLY place resolution state
+ /// is updated — both `handle_permission_response` and
+ /// `handle_question_response` funnel through here.
+ pub fn resolve(
+ &mut self,
+ chat: &mut [Message],
+ request_id: Uuid,
+ response_type: PermissionResponseType,
+ answer_summary: Option<AnswerSummary>,
+ is_remote: bool,
+ oneshot_response: Option<PermissionResponse>,
+ ) {
+ // 1. Update the PermissionRequest message in chat
+ for msg in chat.iter_mut() {
+ if let Message::PermissionRequest(req) = msg {
+ if req.id == request_id {
+ req.response = Some(response_type);
+ if answer_summary.is_some() {
+ req.answer_summary = answer_summary;
+ }
+ break;
+ }
+ }
+ }
+
+ // 2. Update PermissionTracker state
+ if is_remote {
+ self.responded.insert(request_id);
+ } else if let Some(response) = oneshot_response {
+ if let Some(sender) = self.pending.remove(&request_id) {
+ if sender.send(response).is_err() {
+ tracing::error!(
+ "failed to send permission response for request {}",
+ request_id
+ );
+ }
+ } else {
+ tracing::warn!("no pending permission found for request {}", request_id);
+ }
+ }
+ }
+
/// Merge loaded permission state from restored events.
pub fn merge_loaded(&mut self, responded: HashSet<Uuid>, request_note_ids: HashMap<Uuid, [u8; 32]>) {
self.responded = responded;
diff --git a/crates/notedeck_dave/src/update.rs b/crates/notedeck_dave/src/update.rs
@@ -204,15 +204,12 @@ pub fn handle_permission_response(
let is_remote = session.is_remote();
- // Record the response type in the message for UI display
let response_type = match &response {
PermissionResponse::Allow { .. } => crate::messages::PermissionResponseType::Allowed,
PermissionResponse::Deny { .. } => crate::messages::PermissionResponseType::Denied,
};
// Extract relay-publish info before we move `response`.
- // Both local and remote sessions publish permission response events
- // so that resolved state persists across session reloads.
let allowed = matches!(&response, PermissionResponse::Allow { .. });
let message = match &response {
PermissionResponse::Allow { message } => message.clone(),
@@ -231,32 +228,16 @@ pub fn handle_permission_response(
agentic.permission_message_state = PermissionMessageState::None;
}
- for msg in &mut session.chat {
- if let Message::PermissionRequest(req) = msg {
- if req.id == request_id {
- req.response = Some(response_type);
- break;
- }
- }
- }
-
+ // Resolve through the single unified path
if let Some(agentic) = &mut session.agentic {
- if is_remote {
- // Remote: mark as responded, signal relay publish needed
- agentic.permissions.responded.insert(request_id);
- } else {
- // Local: send through oneshot channel to Claude process
- if let Some(sender) = agentic.permissions.pending.remove(&request_id) {
- if sender.send(response).is_err() {
- tracing::error!(
- "Failed to send permission response for request {}",
- request_id
- );
- }
- } else {
- tracing::warn!("No pending permission found for request {}", request_id);
- }
- }
+ agentic.permissions.resolve(
+ &mut session.chat,
+ request_id,
+ response_type,
+ None,
+ is_remote,
+ Some(response),
+ );
}
Some(PermissionPublish {
@@ -359,41 +340,23 @@ pub fn handle_question_response(
)
};
- // Mark the request as allowed in the UI and store the summary for display
- for msg in &mut session.chat {
- if let Message::PermissionRequest(req) = msg {
- if req.id == request_id {
- req.response = Some(crate::messages::PermissionResponseType::Allowed);
- req.answer_summary = answer_summary.clone();
- break;
- }
- }
- }
-
- // Clean up transient answer state and send response (agentic only)
+ // Clean up transient answer state
if let Some(agentic) = &mut session.agentic {
agentic.question_answers.remove(&request_id);
agentic.question_index.remove(&request_id);
- if is_remote {
- // Remote: mark as responded, signal relay publish needed
- agentic.permissions.responded.insert(request_id);
- } else {
- // Local: send through oneshot channel to Claude process
- if let Some(sender) = agentic.permissions.pending.remove(&request_id) {
- let response = PermissionResponse::Allow {
- message: Some(formatted_response.clone()),
- };
- if sender.send(response).is_err() {
- tracing::error!(
- "Failed to send question response for request {}",
- request_id
- );
- }
- } else {
- tracing::warn!("No pending permission found for request {}", request_id);
- }
- }
+ // Resolve through the single unified path
+ let oneshot_response = PermissionResponse::Allow {
+ message: Some(formatted_response.clone()),
+ };
+ agentic.permissions.resolve(
+ &mut session.chat,
+ request_id,
+ crate::messages::PermissionResponseType::Allowed,
+ answer_summary,
+ is_remote,
+ Some(oneshot_response),
+ );
}
Some(PermissionPublish {
