story-kit: merge 91_bug_permissions_dialog_never_triggers_in_web_ui
This commit is contained in:
@@ -5,7 +5,17 @@ use crate::store::JsonFileStore;
|
||||
use crate::workflow::WorkflowState;
|
||||
use poem::http::StatusCode;
|
||||
use std::sync::Arc;
|
||||
use tokio::sync::broadcast;
|
||||
use tokio::sync::{broadcast, mpsc, oneshot};
|
||||
|
||||
/// A permission request forwarded from the MCP `prompt_permission` tool to the
|
||||
/// active WebSocket session. The MCP handler blocks on `response_tx` until the
|
||||
/// user approves or denies via the frontend dialog.
|
||||
pub struct PermissionForward {
|
||||
pub request_id: String,
|
||||
pub tool_name: String,
|
||||
pub tool_input: serde_json::Value,
|
||||
pub response_tx: oneshot::Sender<bool>,
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
pub struct AppContext {
|
||||
@@ -16,6 +26,13 @@ pub struct AppContext {
|
||||
/// Broadcast channel for filesystem watcher events. WebSocket handlers
|
||||
/// subscribe to this to push lifecycle notifications to connected clients.
|
||||
pub watcher_tx: broadcast::Sender<WatcherEvent>,
|
||||
/// Sender for permission requests originating from the MCP
|
||||
/// `prompt_permission` tool. The MCP handler sends a [`PermissionForward`]
|
||||
/// and awaits the oneshot response.
|
||||
pub perm_tx: mpsc::UnboundedSender<PermissionForward>,
|
||||
/// Receiver for permission requests. The active WebSocket handler locks
|
||||
/// this and polls for incoming permission forwards.
|
||||
pub perm_rx: Arc<tokio::sync::Mutex<mpsc::UnboundedReceiver<PermissionForward>>>,
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
@@ -25,12 +42,15 @@ impl AppContext {
|
||||
*state.project_root.lock().unwrap() = Some(project_root.clone());
|
||||
let store_path = project_root.join(".story_kit_store.json");
|
||||
let (watcher_tx, _) = broadcast::channel(64);
|
||||
let (perm_tx, perm_rx) = mpsc::unbounded_channel();
|
||||
Self {
|
||||
state: Arc::new(state),
|
||||
store: Arc::new(JsonFileStore::new(store_path).unwrap()),
|
||||
workflow: Arc::new(std::sync::Mutex::new(WorkflowState::default())),
|
||||
agents: Arc::new(AgentPool::new(3001)),
|
||||
watcher_tx,
|
||||
perm_tx,
|
||||
perm_rx: Arc::new(tokio::sync::Mutex::new(perm_rx)),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -760,6 +760,24 @@ fn handle_tools_list(id: Option<Value>) -> JsonRpcResponse {
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
{
|
||||
"name": "prompt_permission",
|
||||
"description": "Present a permission request to the user via the web UI. Used by Claude Code's --permission-prompt-tool to delegate permission decisions to the frontend dialog. Returns on approval; returns an error on denial.",
|
||||
"inputSchema": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"tool_name": {
|
||||
"type": "string",
|
||||
"description": "The tool requesting permission (e.g. 'Bash', 'Write')"
|
||||
},
|
||||
"input": {
|
||||
"type": "object",
|
||||
"description": "The tool's input arguments"
|
||||
}
|
||||
},
|
||||
"required": ["tool_name", "input"]
|
||||
}
|
||||
}
|
||||
]
|
||||
}),
|
||||
@@ -818,6 +836,8 @@ async fn handle_tools_call(
|
||||
"request_qa" => tool_request_qa(&args, ctx).await,
|
||||
// Diagnostics
|
||||
"get_server_logs" => tool_get_server_logs(&args),
|
||||
// Permission bridge (Claude Code → frontend dialog)
|
||||
"prompt_permission" => tool_prompt_permission(&args, ctx).await,
|
||||
_ => Err(format!("Unknown tool: {tool_name}")),
|
||||
};
|
||||
|
||||
@@ -1550,6 +1570,49 @@ fn tool_get_server_logs(args: &Value) -> Result<String, String> {
|
||||
Ok(recent.join("\n"))
|
||||
}
|
||||
|
||||
/// MCP tool called by Claude Code via `--permission-prompt-tool`.
|
||||
///
|
||||
/// Forwards the permission request through the shared channel to the active
|
||||
/// WebSocket session, which presents a dialog to the user. Blocks until the
|
||||
/// user approves or denies (with a 5-minute timeout).
|
||||
async fn tool_prompt_permission(args: &Value, ctx: &AppContext) -> Result<String, String> {
|
||||
let tool_name = args
|
||||
.get("tool_name")
|
||||
.and_then(|v| v.as_str())
|
||||
.unwrap_or("unknown")
|
||||
.to_string();
|
||||
let tool_input = args
|
||||
.get("input")
|
||||
.cloned()
|
||||
.unwrap_or(json!({}));
|
||||
|
||||
let request_id = uuid::Uuid::new_v4().to_string();
|
||||
let (response_tx, response_rx) = tokio::sync::oneshot::channel();
|
||||
|
||||
ctx.perm_tx
|
||||
.send(crate::http::context::PermissionForward {
|
||||
request_id: request_id.clone(),
|
||||
tool_name: tool_name.clone(),
|
||||
tool_input,
|
||||
response_tx,
|
||||
})
|
||||
.map_err(|_| "No active WebSocket session to receive permission request".to_string())?;
|
||||
|
||||
let approved = tokio::time::timeout(
|
||||
std::time::Duration::from_secs(300),
|
||||
response_rx,
|
||||
)
|
||||
.await
|
||||
.map_err(|_| format!("Permission request for '{tool_name}' timed out after 5 minutes"))?
|
||||
.map_err(|_| "Permission response channel closed unexpectedly".to_string())?;
|
||||
|
||||
if approved {
|
||||
Ok(format!("Permission granted for '{tool_name}'"))
|
||||
} else {
|
||||
Err(format!("User denied permission for '{tool_name}'"))
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -1647,7 +1710,8 @@ mod tests {
|
||||
assert!(names.contains(&"move_story_to_merge"));
|
||||
assert!(names.contains(&"request_qa"));
|
||||
assert!(names.contains(&"get_server_logs"));
|
||||
assert_eq!(tools.len(), 27);
|
||||
assert!(names.contains(&"prompt_permission"));
|
||||
assert_eq!(tools.len(), 28);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -10,7 +10,7 @@ use poem::web::websocket::{Message as WsMessage, WebSocket};
|
||||
use serde::{Deserialize, Serialize};
|
||||
use std::collections::HashMap;
|
||||
use std::sync::Arc;
|
||||
use tokio::sync::mpsc;
|
||||
use tokio::sync::{mpsc, oneshot};
|
||||
|
||||
#[derive(Deserialize)]
|
||||
#[serde(tag = "type", rename_all = "snake_case")]
|
||||
@@ -155,11 +155,11 @@ pub async fn ws_handler(ws: WebSocket, ctx: Data<&Arc<AppContext>>) -> impl poem
|
||||
}
|
||||
});
|
||||
|
||||
// Channel for permission requests flowing from the PTY thread to this handler.
|
||||
let (perm_req_tx, mut perm_req_rx) =
|
||||
mpsc::unbounded_channel::<crate::llm::providers::claude_code::PermissionReqMsg>();
|
||||
// Map of pending permission request_id → one-shot responder.
|
||||
let mut pending_perms: HashMap<String, std::sync::mpsc::SyncSender<bool>> = HashMap::new();
|
||||
// Map of pending permission request_id → oneshot responder.
|
||||
// Permission requests arrive from the MCP `prompt_permission` tool via
|
||||
// `ctx.perm_rx` and are forwarded to the client as `PermissionRequest`.
|
||||
// When the client responds, we resolve the corresponding oneshot.
|
||||
let mut pending_perms: HashMap<String, oneshot::Sender<bool>> = HashMap::new();
|
||||
|
||||
loop {
|
||||
// Outer loop: wait for the next WebSocket message.
|
||||
@@ -174,7 +174,6 @@ pub async fn ws_handler(ws: WebSocket, ctx: Data<&Arc<AppContext>>) -> impl poem
|
||||
let tx_tokens = tx.clone();
|
||||
let tx_activity = tx.clone();
|
||||
let ctx_clone = ctx.clone();
|
||||
let perm_tx = perm_req_tx.clone();
|
||||
|
||||
// Build the chat future without driving it yet so we can
|
||||
// interleave it with permission-request forwarding.
|
||||
@@ -198,26 +197,29 @@ pub async fn ws_handler(ws: WebSocket, ctx: Data<&Arc<AppContext>>) -> impl poem
|
||||
tool_name: tool_name.to_string(),
|
||||
});
|
||||
},
|
||||
Some(perm_tx),
|
||||
);
|
||||
tokio::pin!(chat_fut);
|
||||
|
||||
// Lock the permission receiver for the duration of this chat
|
||||
// session. Permission requests from the MCP tool arrive here.
|
||||
let mut perm_rx = ctx.perm_rx.lock().await;
|
||||
|
||||
// Inner loop: drive the chat while concurrently handling
|
||||
// permission requests and WebSocket messages.
|
||||
// permission requests (from MCP) and WebSocket messages.
|
||||
let chat_result = loop {
|
||||
tokio::select! {
|
||||
result = &mut chat_fut => break result,
|
||||
|
||||
// Forward permission requests from PTY to the client.
|
||||
Some(perm_req) = perm_req_rx.recv() => {
|
||||
// Forward permission requests from MCP tool to the client.
|
||||
Some(perm_fwd) = perm_rx.recv() => {
|
||||
let _ = tx.send(WsResponse::PermissionRequest {
|
||||
request_id: perm_req.request_id.clone(),
|
||||
tool_name: perm_req.tool_name.clone(),
|
||||
tool_input: perm_req.tool_input.clone(),
|
||||
request_id: perm_fwd.request_id.clone(),
|
||||
tool_name: perm_fwd.tool_name.clone(),
|
||||
tool_input: perm_fwd.tool_input.clone(),
|
||||
});
|
||||
pending_perms.insert(
|
||||
perm_req.request_id,
|
||||
perm_req.response_tx,
|
||||
perm_fwd.request_id,
|
||||
perm_fwd.response_tx,
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -177,7 +177,6 @@ pub fn set_anthropic_api_key(store: &dyn StoreOps, api_key: String) -> Result<()
|
||||
set_anthropic_api_key_impl(store, &api_key)
|
||||
}
|
||||
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub async fn chat<F, U, A>(
|
||||
messages: Vec<Message>,
|
||||
config: ProviderConfig,
|
||||
@@ -186,11 +185,6 @@ pub async fn chat<F, U, A>(
|
||||
mut on_update: F,
|
||||
mut on_token: U,
|
||||
mut on_activity: A,
|
||||
permission_tx: Option<
|
||||
tokio::sync::mpsc::UnboundedSender<
|
||||
crate::llm::providers::claude_code::PermissionReqMsg,
|
||||
>,
|
||||
>,
|
||||
) -> Result<ChatResult, String>
|
||||
where
|
||||
F: FnMut(&[Message]) + Send,
|
||||
@@ -247,7 +241,6 @@ where
|
||||
config.session_id.as_deref(),
|
||||
&mut cancel_rx,
|
||||
|token| on_token(token),
|
||||
permission_tx,
|
||||
)
|
||||
.await
|
||||
.map_err(|e| format!("Claude Code Error: {e}"))?;
|
||||
|
||||
@@ -1,24 +1,12 @@
|
||||
use crate::slog;
|
||||
use portable_pty::{CommandBuilder, PtySize, native_pty_system};
|
||||
use std::io::{BufRead, BufReader, Write};
|
||||
use std::io::{BufRead, BufReader};
|
||||
use std::sync::Arc;
|
||||
use std::sync::atomic::{AtomicBool, Ordering};
|
||||
use tokio::sync::watch;
|
||||
|
||||
use crate::llm::types::{FunctionCall, Message, Role, ToolCall};
|
||||
|
||||
/// A permission request emitted by Claude Code that must be resolved by the user.
|
||||
pub struct PermissionReqMsg {
|
||||
/// Unique identifier for this request (used to correlate the response).
|
||||
pub request_id: String,
|
||||
/// The tool that is requesting permission (e.g. "Bash", "Write").
|
||||
pub tool_name: String,
|
||||
/// The tool's input arguments (e.g. `{"command": "git push"}`).
|
||||
pub tool_input: serde_json::Value,
|
||||
/// One-shot channel to send the user's decision back to the PTY thread.
|
||||
pub response_tx: std::sync::mpsc::SyncSender<bool>,
|
||||
}
|
||||
|
||||
/// Result from a Claude Code session containing structured messages.
|
||||
pub struct ClaudeCodeResult {
|
||||
/// The conversation messages produced by Claude Code, including assistant
|
||||
@@ -37,6 +25,10 @@ pub struct ClaudeCodeResult {
|
||||
/// Supports session resumption: if a `session_id` is provided, passes
|
||||
/// `--resume <id>` so Claude Code loads the prior conversation transcript
|
||||
/// from disk and continues with full context.
|
||||
///
|
||||
/// Permissions are delegated to the MCP `prompt_permission` tool via
|
||||
/// `--permission-prompt-tool`, so Claude Code calls back into the server
|
||||
/// when a tool requires user approval. The frontend dialog handles the UX.
|
||||
pub struct ClaudeCodeProvider;
|
||||
|
||||
impl ClaudeCodeProvider {
|
||||
@@ -51,7 +43,6 @@ impl ClaudeCodeProvider {
|
||||
session_id: Option<&str>,
|
||||
cancel_rx: &mut watch::Receiver<bool>,
|
||||
mut on_token: F,
|
||||
permission_tx: Option<tokio::sync::mpsc::UnboundedSender<PermissionReqMsg>>,
|
||||
) -> Result<ClaudeCodeResult, String>
|
||||
where
|
||||
F: FnMut(&str) + Send,
|
||||
@@ -85,7 +76,6 @@ impl ClaudeCodeProvider {
|
||||
token_tx,
|
||||
msg_tx,
|
||||
sid_tx,
|
||||
permission_tx,
|
||||
)
|
||||
});
|
||||
|
||||
@@ -115,7 +105,10 @@ impl ClaudeCodeProvider {
|
||||
/// Sends streaming text tokens via `token_tx` for real-time display, and
|
||||
/// complete structured `Message` values via `msg_tx` for the final message
|
||||
/// history (assistant turns with tool_calls, and tool result turns).
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
///
|
||||
/// Permission handling is delegated to the MCP `prompt_permission` tool
|
||||
/// via `--permission-prompt-tool`. Claude Code calls the MCP tool when it
|
||||
/// needs user approval, and the server bridges the request to the frontend.
|
||||
fn run_pty_session(
|
||||
user_message: &str,
|
||||
cwd: &str,
|
||||
@@ -124,7 +117,6 @@ fn run_pty_session(
|
||||
token_tx: tokio::sync::mpsc::UnboundedSender<String>,
|
||||
msg_tx: std::sync::mpsc::Sender<Message>,
|
||||
sid_tx: tokio::sync::oneshot::Sender<String>,
|
||||
permission_tx: Option<tokio::sync::mpsc::UnboundedSender<PermissionReqMsg>>,
|
||||
) -> Result<(), String> {
|
||||
let pty_system = native_pty_system();
|
||||
|
||||
@@ -147,6 +139,11 @@ fn run_pty_session(
|
||||
cmd.arg("--output-format");
|
||||
cmd.arg("stream-json");
|
||||
cmd.arg("--verbose");
|
||||
// Delegate permission decisions to the MCP prompt_permission tool.
|
||||
// Claude Code will call this tool via the story-kit MCP server when
|
||||
// a tool requires user approval, instead of using PTY stdin/stdout.
|
||||
cmd.arg("--permission-prompt-tool");
|
||||
cmd.arg("mcp__story-kit__prompt_permission");
|
||||
cmd.cwd(cwd);
|
||||
// Keep TERM reasonable but disable color
|
||||
cmd.env("NO_COLOR", "1");
|
||||
@@ -154,7 +151,7 @@ fn run_pty_session(
|
||||
cmd.env("CLAUDECODE", "");
|
||||
|
||||
slog!(
|
||||
"[pty-debug] Spawning: claude -p \"{}\" {} --output-format stream-json --verbose",
|
||||
"[pty-debug] Spawning: claude -p \"{}\" {} --output-format stream-json --verbose --permission-prompt-tool mcp__story-kit__prompt_permission",
|
||||
user_message,
|
||||
resume_session_id
|
||||
.map(|s| format!("--resume {s}"))
|
||||
@@ -177,11 +174,9 @@ fn run_pty_session(
|
||||
.try_clone_reader()
|
||||
.map_err(|e| format!("Failed to clone PTY reader: {e}"))?;
|
||||
|
||||
// Keep a writer handle so we can respond to permission_request events.
|
||||
let mut pty_writer = pair
|
||||
.master
|
||||
.take_writer()
|
||||
.map_err(|e| format!("Failed to take PTY writer: {e}"))?;
|
||||
// We no longer need the writer — permission responses flow through MCP,
|
||||
// not PTY stdin. Drop it so the PTY sees EOF on stdin when appropriate.
|
||||
drop(pair.master);
|
||||
|
||||
// Read NDJSON lines from stdout
|
||||
let (line_tx, line_rx) = std::sync::mpsc::channel::<Option<String>>();
|
||||
@@ -276,53 +271,6 @@ fn run_pty_session(
|
||||
"system" => {}
|
||||
// Rate limit info — suppress noisy notification
|
||||
"rate_limit_event" => {}
|
||||
// Claude Code is requesting user approval before executing a tool.
|
||||
// Forward the request to the async context via permission_tx and
|
||||
// block until the user responds (or a 5-minute timeout elapses).
|
||||
"permission_request" => {
|
||||
let request_id = json
|
||||
.get("id")
|
||||
.and_then(|v| v.as_str())
|
||||
.unwrap_or("")
|
||||
.to_string();
|
||||
let tool_name = json
|
||||
.get("tool_name")
|
||||
.and_then(|v| v.as_str())
|
||||
.unwrap_or("unknown")
|
||||
.to_string();
|
||||
let tool_input = json
|
||||
.get("input")
|
||||
.cloned()
|
||||
.unwrap_or(serde_json::Value::Object(serde_json::Map::new()));
|
||||
|
||||
if let Some(ref ptx) = permission_tx {
|
||||
let (resp_tx, resp_rx) = std::sync::mpsc::sync_channel(1);
|
||||
let _ = ptx.send(PermissionReqMsg {
|
||||
request_id: request_id.clone(),
|
||||
tool_name,
|
||||
tool_input,
|
||||
response_tx: resp_tx,
|
||||
});
|
||||
// Block until the user responds or a 5-minute timeout elapses.
|
||||
let approved = resp_rx
|
||||
.recv_timeout(std::time::Duration::from_secs(300))
|
||||
.unwrap_or(false);
|
||||
let response = serde_json::json!({
|
||||
"type": "permission_response",
|
||||
"id": request_id,
|
||||
"approved": approved,
|
||||
});
|
||||
let _ = writeln!(pty_writer, "{}", response);
|
||||
} else {
|
||||
// No handler configured — deny by default.
|
||||
let response = serde_json::json!({
|
||||
"type": "permission_response",
|
||||
"id": request_id,
|
||||
"approved": false,
|
||||
});
|
||||
let _ = writeln!(pty_writer, "{}", response);
|
||||
}
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -66,6 +66,9 @@ async fn main() -> Result<(), std::io::Error> {
|
||||
}
|
||||
}
|
||||
|
||||
// Permission channel: MCP prompt_permission → WebSocket handler.
|
||||
let (perm_tx, perm_rx) = tokio::sync::mpsc::unbounded_channel();
|
||||
|
||||
// Capture project root and agents Arc before ctx is consumed by build_routes.
|
||||
let startup_root: Option<PathBuf> = app_state.project_root.lock().unwrap().clone();
|
||||
let startup_agents = Arc::clone(&agents);
|
||||
@@ -76,6 +79,8 @@ async fn main() -> Result<(), std::io::Error> {
|
||||
workflow,
|
||||
agents,
|
||||
watcher_tx,
|
||||
perm_tx,
|
||||
perm_rx: Arc::new(tokio::sync::Mutex::new(perm_rx)),
|
||||
};
|
||||
|
||||
let app = build_routes(ctx);
|
||||
|
||||
Reference in New Issue
Block a user