From 0e9fc26aeaf7930009464a4ecb0b6550f65ee9b5 Mon Sep 17 00:00:00 2001 From: xuan2261 Date: Tue, 6 Oct 2026 11:40:50 +0700 Subject: [PATCH 1/4] fix(daemon): harden approval response correlation Co-Authored-By: JeikCode --- crates/jeikcode-capabilities/src/notify.rs | 104 ++- crates/jeikcode-coding/src/runtime.rs | 38 +- crates/jeikcode-daemon/src/lib.rs | 844 +++++++++++------- crates/jeikcode-daemon/src/live_api.rs | 484 +++++++--- crates/jeikcode-daemon/src/live_hub.rs | 55 ++ crates/jeikcode-daemon/src/native_live.rs | 53 ++ .../jeikcode-daemon/src/permission_bridge.rs | 415 ++++++++- .../jetbrains/daemon/JeikCodeApiClient.kt | 4 +- .../jetbrains/daemon/JeikCodeDaemonClient.kt | 2 + .../jetbrains/daemon/JeikCodeDaemonTypes.kt | 1 + .../jeikcode/jetbrains/daemon/SseParser.kt | 1 + .../services/JeikCodeProjectService.kt | 3 +- .../jetbrains/ui/JeikCodeChatPanel.kt | 4 +- .../jetbrains/daemon/SseParserTest.kt | 3 +- extensions/vscode/src/chat/provider.ts | 8 +- extensions/vscode/src/daemon/client.ts | 3 + extensions/vscode/src/daemon/types.ts | 4 +- .../src/components/PermissionRequest.tsx | 10 +- .../webview-ui/src/state/ChatProvider.tsx | 12 +- .../webview-ui/src/state/permissionBridge.ts | 17 + .../vscode/webview-ui/src/state/reducer.ts | 25 +- .../vscode/webview-ui/src/state/types.ts | 11 +- .../test/daemon-client-error.test.ts | 2 + .../test/provider-queue-regression.test.ts | 15 +- .../test/rendering-regression.test.ts | 104 ++- webui/src/api.test.ts | 55 ++ webui/src/api.ts | 23 +- webui/src/app.tsx | 10 +- webui/src/components/Chat.tsx | 15 +- webui/src/components/NotificationDock.tsx | 115 ++- webui/src/components/PermissionCard.tsx | 11 +- webui/src/lib/pendingPermission.ts | 18 + webui/src/lib/sessionNotify.test.ts | 44 + webui/src/lib/sessionNotify.ts | 64 +- 34 files changed, 1978 insertions(+), 599 deletions(-) create mode 100644 extensions/vscode/webview-ui/src/state/permissionBridge.ts diff --git a/crates/jeikcode-capabilities/src/notify.rs b/crates/jeikcode-capabilities/src/notify.rs index 7c5c1c9cf..4ae16d97a 100644 --- a/crates/jeikcode-capabilities/src/notify.rs +++ b/crates/jeikcode-capabilities/src/notify.rs @@ -155,11 +155,30 @@ pub fn focus_launch(port: u16, secret: &str, session_id: &str) -> Option accepted_focus_launch(&format!("jeikcode-focus:{port}:{secret}:{session_id}")) } +/// Correlated `/chat` approval launch target. The approval id is included only when +/// it is safe for the protocol URI; otherwise callers still get a normal +/// click-to-focus notification without inline Approve/Deny actions. +pub fn focus_launch_for_permission( + port: u16, + secret: &str, + session_id: &str, + approval_id: &str, +) -> Option { + if port == 0 { + return None; + } + let session_id = sanitize_focus_session_id(session_id)?; + let approval_id = sanitize_focus_session_id(approval_id)?; + accepted_focus_launch(&format!( + "jeikcode-focus:{port}:{secret}:{session_id}:approval:{approval_id}" + )) +} + fn accepted_focus_launch(raw: &str) -> Option { let raw = raw.trim(); let rest = raw.strip_prefix("jeikcode-focus:")?; let (port, rest) = rest.split_once(':')?; - let (secret, session) = rest.split_once(':')?; + let (secret, rest) = rest.split_once(':')?; if port.is_empty() || port.len() > 5 || !port.bytes().all(|b| b.is_ascii_digit()) @@ -170,8 +189,18 @@ fn accepted_focus_launch(raw: &str) -> Option { if secret.len() != 32 || !secret.bytes().all(|b| b.is_ascii_hexdigit()) { return None; } - let session = sanitize_focus_session_id(session)?; - Some(format!("jeikcode-focus:{port}:{secret}:{session}")) + let mut parts = rest.split(':'); + let session = sanitize_focus_session_id(parts.next()?)?; + match (parts.next(), parts.next(), parts.next()) { + (None, None, None) => Some(format!("jeikcode-focus:{port}:{secret}:{session}")), + (Some("approval"), Some(approval_id), None) => { + let approval_id = sanitize_focus_session_id(approval_id)?; + Some(format!( + "jeikcode-focus:{port}:{secret}:{session}:approval:{approval_id}" + )) + } + _ => None, + } } #[cfg(target_os = "windows")] @@ -241,7 +270,8 @@ fn windows_toast_xml(title: &str, body: &str, launch: Option<&str>) -> String { || lower_title.contains("ask") || title.contains("回答") || title.contains("提问")); - let actions = match (launch, is_approval, is_question) { + let approval_is_correlated = launch.is_some_and(|uri| uri.contains(":approval:")); + let actions = match (launch, is_approval && approval_is_correlated, is_question) { (Some(uri), true, _) => { let allow_uri = format!("{}:allow", xml_escape(uri)); let deny_uri = format!("{}:deny", xml_escape(uri)); @@ -878,23 +908,21 @@ $ErrorActionPreference = 'Continue' try { if ([string]::IsNullOrWhiteSpace($Uri)) { exit 0 } $Uri = $Uri.Trim().Trim('"').Trim("'") - if ($Uri -notmatch '^jeikcode-focus:(\d{1,5}):([0-9a-fA-F]{32}):([A-Za-z0-9_-]{1,128})(?::([A-Za-z0-9_]{1,32}))?$') { exit 0 } + if ($Uri -notmatch '^jeikcode-focus:(\d{1,5}):([0-9a-fA-F]{32}):([A-Za-z0-9_-]{1,128})(?::approval:([A-Za-z0-9_-]{1,128}))?(?::(allow|deny))?$') { exit 0 } $port = $Matches[1] $secret = $Matches[2] $session = $Matches[3] - $action = $Matches[4] + $approvalId = $Matches[4] + $action = $Matches[5] $payload = '{"session_id":"' + $session + '","secret":"' + $secret + '"}' try { Invoke-RestMethod -Method Post -Uri ("http://127.0.0.1:" + $port + "/notify-focus") -ContentType 'application/json; charset=utf-8' -Body $payload -TimeoutSec 3 | Out-Null } catch {} - if (-not [string]::IsNullOrWhiteSpace($action)) { - $permPayload = '{"session_id":"' + $session + '","decision":"' + $action + '"}' + if (-not [string]::IsNullOrWhiteSpace($action) -and -not [string]::IsNullOrWhiteSpace($approvalId)) { + $permPayload = '{"session_id":"' + $session + '","approval_id":"' + $approvalId + '","decision":"' + $action + '"}' try { Invoke-RestMethod -Method Post -Uri ("http://127.0.0.1:" + $port + "/chat/permission") -ContentType 'application/json; charset=utf-8' -Body $permPayload -TimeoutSec 3 | Out-Null } catch {} - try { - Invoke-RestMethod -Method Post -Uri ("http://127.0.0.1:" + $port + "/live/permission") -ContentType 'application/json; charset=utf-8' -Body $permPayload -TimeoutSec 3 | Out-Null - } catch {} } # 安全激活已有的 JeikCode 桌面窗口，使用 Windows 标准安全 COM 组件，绝不包含底层 C# 注入和键盘模拟，根除杀软误报 @@ -1176,10 +1204,19 @@ focus_uri() { secret=${rest%%:*} rest=${rest#*:} session=${rest%%:*} + approval_id="" + action="" if [ "$session" != "$rest" ]; then - action=${rest#*:} - else - action="" + tail=${rest#*:} + case "$tail" in + approval:*) + tail=${tail#approval:} + approval_id=${tail%%:*} + if [ "$approval_id" != "$tail" ]; then + action=${tail#*:} + fi + ;; + esac fi case "$port" in ''|0*|*[!0-9]*) return 0 ;; @@ -1199,6 +1236,14 @@ focus_uri() { if [ "${#session}" -gt 128 ]; then return 0 fi + if [ -n "$approval_id" ]; then + case "$approval_id" in + *[!A-Za-z0-9_-]*) return 0 ;; + esac + if [ "${#approval_id}" -gt 128 ]; then + return 0 + fi + fi payload=$(printf '{"session_id":"%s","secret":"%s"}' "$session" "$secret") url="http://127.0.0.1:${port}/notify-focus" if command -v curl >/dev/null 2>&1; then @@ -1206,8 +1251,8 @@ focus_uri() { elif command -v wget >/dev/null 2>&1; then wget -q -T 3 -O /dev/null --header='Content-Type: application/json' --post-data="$payload" "$url" >/dev/null 2>&1 || true fi - if [ -n "$action" ]; then - act_payload=$(printf '{"session_id":"%s","decision":"%s"}' "$session" "$action") + if [ -n "$action" ] && [ -n "$approval_id" ]; then + act_payload=$(printf '{"session_id":"%s","approval_id":"%s","decision":"%s"}' "$session" "$approval_id" "$action") act_url="http://127.0.0.1:${port}/chat/permission" if command -v curl >/dev/null 2>&1; then curl -fsS -m 3 -X POST -H 'Content-Type: application/json' --data "$act_payload" "$act_url" >/dev/null 2>&1 || true @@ -1259,8 +1304,12 @@ case "$cmd" in uri=${3:-} action="" is_appr=0 - case "$title $body" in - *approval*|*审核*|*review*) is_appr=1 ;; + case "$uri" in + jeikcode-focus:*:approval:*) + case "$title" in + *approval*|*审核*|*review*) is_appr=1 ;; + esac + ;; esac if command -v notify-send >/dev/null 2>&1; then if [ "$is_appr" -eq 1 ]; then @@ -1719,8 +1768,22 @@ mod tests { #[test] fn windows_toast_approval_includes_action_buttons() { let secret = "0123456789abcdef0123456789abcdef"; - let launch = focus_launch(13457, secret, "550e8400-e29b-41d4-a716-446655440000") + let plain_launch = focus_launch(13457, secret, "550e8400-e29b-41d4-a716-446655440000") .expect("uuid session"); + let plain_xml = + windows_toast_xml("JeikCode needs approval", "write_file", Some(&plain_launch)); + assert!( + !plain_xml.contains(""), + "uncorrelated approval notifications must be click-to-focus only" + ); + + let launch = focus_launch_for_permission( + 13457, + secret, + "550e8400-e29b-41d4-a716-446655440000", + "approval_abc123", + ) + .expect("correlated approval launch"); let xml = windows_toast_xml("JeikCode needs approval", "write_file", Some(&launch)); assert!(xml.contains("")); assert!(xml.contains("content=\"Approve\"")); @@ -1729,6 +1792,7 @@ mod tests { assert!(!xml.contains("拒绝")); assert!(xml.contains(&format!("arguments=\"{launch}:allow\""))); assert!(xml.contains(&format!("arguments=\"{launch}:deny\""))); + assert!(launch.contains(":approval:approval_abc123")); let zh_xml = windows_toast_xml("JeikCode 等待审核", "write_file", Some(&launch)); assert!(zh_xml.contains("")); @@ -1777,6 +1841,8 @@ mod tests { assert!(script.contains("linux-notify")); assert!(script.contains("-A default=")); assert!(script.contains("-t 25000")); + assert!(script.contains("jeikcode-focus:*:approval:*")); + assert!(script.contains("case \"$title\" in")); assert!(script.contains("JeikCode Desktop")); assert!(script.contains("wmctrl")); assert_eq!(shell_single_quote("a b's"), "'a b'\\''s'"); diff --git a/crates/jeikcode-coding/src/runtime.rs b/crates/jeikcode-coding/src/runtime.rs index 996008565..635766dca 100644 --- a/crates/jeikcode-coding/src/runtime.rs +++ b/crates/jeikcode-coding/src/runtime.rs @@ -1233,10 +1233,28 @@ impl CodingRuntimeHandle { value: serde_json::Value, ) -> Result<(), RuntimeError> { let state = self.state.load(Ordering::Acquire); + self.respond_for_generation( + RuntimeGeneration(runtime_state_generation(state)), + id, + value, + ) + .await + } + + /// Respond to an exact runtime generation. Callers that surface a native + /// request outside the runtime (for example `/live/permission`) must echo + /// the generation they observed so a delayed response cannot be rebound to + /// a later runtime generation that reused the same request id. + pub async fn respond_for_generation( + &self, + generation: RuntimeGeneration, + id: RequestId, + value: serde_json::Value, + ) -> Result<(), RuntimeError> { let (done, result) = oneshot::channel(); self.tx .send(CodingRuntimeControl::Respond { - generation: runtime_state_generation(state), + generation: generation.0, id, value, done, @@ -12183,7 +12201,23 @@ mod tests { )); assert_eq!(handle.status().phase, RuntimePhase::WaitingApproval); - handle.respond(43, serde_json::Value::Null).await.unwrap(); + let generation = handle.status().generation; + let stale_generation = generation.saturating_add(1); + assert!(matches!( + handle + .respond_for_generation( + RuntimeGeneration(stale_generation), + 43, + serde_json::Value::Null, + ) + .await, + Err(RuntimeError::Unavailable) + )); + assert_eq!(handle.status().phase, RuntimePhase::WaitingApproval); + handle + .respond_for_generation(RuntimeGeneration(generation), 43, serde_json::Value::Null) + .await + .unwrap(); assert!(matches!( kernel_commands.recv().await, Some(AgentCommand::Respond { id: 43, .. }) diff --git a/crates/jeikcode-daemon/src/lib.rs b/crates/jeikcode-daemon/src/lib.rs index 2f992e8d6..22131e8f4 100644 --- a/crates/jeikcode-daemon/src/lib.rs +++ b/crates/jeikcode-daemon/src/lib.rs @@ -4700,12 +4700,13 @@ pub enum ChatEvent { #[serde(skip_serializing_if = "Option::is_none")] message: Option, }, - /// A tool requires user approval. The browser must POST the decision - /// back to `/chat/permission` keyed by `session_id`. The decider blocks - /// until the decision arrives (or the turn is cancelled). + /// A tool requires user approval. The browser must POST the decision back + /// to `/chat/permission`, correlated by `session_id` and `approval_id`. + /// The decider blocks until the decision arrives (or the turn is cancelled). #[serde(rename = "permission_request")] PermissionRequest { session_id: String, + approval_id: String, tool_name: String, reason: String, call_id: String, @@ -5155,6 +5156,7 @@ mod chat_event_type_tests { id: 42, kind: jeikcode_capabilities::tools::APPROVAL_KIND.into(), payload: serde_json::json!({ + "approval_id": "approval-42", "call_id": "call-42", "tool": "bash", "args": "{\"command\":\"git status\"}" @@ -5170,10 +5172,14 @@ mod chat_event_type_tests { events.as_slice(), [ChatEvent::PermissionRequest { session_id, + approval_id, call_id, tool_name, .. - }] if session_id == "session-1" && call_id == "call-42" && tool_name == "bash" + }] if session_id == "session-1" + && approval_id == "approval-42" + && call_id == "call-42" + && tool_name == "bash" )); } @@ -5670,12 +5676,22 @@ impl ChatRuntimeProjector { if request.kind != APPROVAL_KIND { return Vec::new(); } + let approval_id = request + .payload + .get("approval_id") + .and_then(|value| value.as_str()) + .unwrap_or_default() + .to_string(); + if approval_id.is_empty() { + return Vec::new(); + } let Ok(approval) = serde_json::from_value::(request.payload) else { return Vec::new(); }; vec![ChatEvent::PermissionRequest { session_id: permission_session_id.to_string(), + approval_id, tool_name: approval.tool, reason: "Requires approval".into(), call_id: approval.call_id, @@ -6421,7 +6437,7 @@ async fn process_chat_request( // Turn never ran — the turn task (which registers the responder) never // spawned, so this is a defensive no-op cleanup for interactive modes. if registered_permission_responder { - pending_permissions.unregister(&perm_session_key); + pending_permissions.unregister_session(&perm_session_key); } pending_user_inputs.unregister_session(&perm_session_key); return Ok(()); @@ -6443,16 +6459,10 @@ async fn process_chat_request( == crate::approval_mode::ApprovalMode::Auto || (approval_mode == crate::approval_mode::ApprovalMode::Build && !interactive_permission); - // Interactive approval: route /chat/permission decisions to the native runtime - // request waiting for this turn. - let perm_rx = if registered_permission_responder { - let (tx, rx) = - mpsc::unbounded_channel::(); - pending_permissions.register(perm_session_key.clone(), tx); - Some(rx) - } else { - None - }; + // Interactive approval: each native approval request registers its exact + // `(session_id, call_id)` route immediately before the card is published. + let permission_responders = + registered_permission_responder.then(|| pending_permissions.clone()); let conv = conversation.clone(); let cancel = cancel_token.clone(); let runtime_session_id = perm_session_key.clone(); @@ -6467,7 +6477,7 @@ async fn process_chat_request( runtime_event_tx, cancel, runtime_cfg, - perm_rx, + permission_responders, if interactive_user_input { Some(runtime_user_inputs) } else { @@ -6523,7 +6533,7 @@ async fn process_chat_request( // Drop the permission // registration so it doesn't leak. Only registered in interactive prompt modes. if registered_permission_responder { - pending_permissions.unregister(&perm_session_key); + pending_permissions.unregister_session(&perm_session_key); } pending_user_inputs.unregister_session(&perm_session_key); Ok(()) @@ -7130,6 +7140,10 @@ struct SystemNotifyBody { /// Session to open when the OS toast is clicked. Empty for a bare notice. #[serde(default)] session_id: String, + /// Exact `/chat` approval identity. When present, the OS toast may expose + /// correlated Approve/Deny actions; otherwise review toasts are focus-only. + #[serde(default)] + approval_id: String, } fn notify_focus_port() -> &'static std::sync::atomic::AtomicU16 { @@ -7161,8 +7175,22 @@ fn publish_notify_focus_port(port: u16) { jeikcode_capabilities::notify::ensure_focus_protocol(); } -fn system_notify_launch(port: u16, secret: &str, session_id: &str) -> Option { - jeikcode_capabilities::notify::focus_launch(port, secret, session_id) +fn system_notify_launch( + port: u16, + secret: &str, + session_id: &str, + approval_id: Option<&str>, +) -> Option { + match approval_id.filter(|id| !id.trim().is_empty()) { + Some(approval_id) => jeikcode_capabilities::notify::focus_launch_for_permission( + port, + secret, + session_id, + approval_id, + ) + .or_else(|| jeikcode_capabilities::notify::focus_launch(port, secret, session_id)), + None => jeikcode_capabilities::notify::focus_launch(port, secret, session_id), + } } fn notifications_enabled() -> bool { @@ -7225,6 +7253,7 @@ async fn system_notify(Json(req): Json) -> impl IntoResponse { notify_focus_port().load(std::sync::atomic::Ordering::Relaxed), notify_focus_secret(), &req.session_id, + Some(req.approval_id.as_str()), ); jeikcode_capabilities::notify::notify_system_launch(title, body, launch.as_deref()); ( @@ -7359,6 +7388,7 @@ async fn chat_pending( permission.map(|ev| match ev { ChatEvent::PermissionRequest { session_id, + approval_id, tool_name, reason, call_id, @@ -7366,6 +7396,7 @@ async fn chat_pending( } => serde_json::json!({ "type": "permission_request", "session_id": session_id, + "approval_id": approval_id, "tool_name": tool_name, "reason": reason, "call_id": call_id, @@ -7374,7 +7405,8 @@ async fn chat_pending( _ => serde_json::Value::Null, }) }; - if !auto_mode + if active + && !auto_mode && (permission_json.is_none() || permission_json .as_ref() @@ -7394,6 +7426,7 @@ async fn chat_pending( permission_json = Some(serde_json::json!({ "type": "permission_request", "session_id": pending.session_id, + "approval_id": permission_bridge::approval_id_for_pending(&pending), "tool_name": pending.tool_name, "reason": pending.reason, "call_id": pending.call_id, @@ -7435,6 +7468,8 @@ async fn chat_pending( #[derive(Debug, serde::Deserialize)] pub struct PermissionDecisionRequest { pub session_id: String, + /// Stable identity of the exact `permission_request` card. + pub approval_id: String, /// "allow" | "deny" | "always_allow" | "allow_persist" pub decision: String, /// Full MCP tool name (`mcp__{server}__{tool}`); required for `allow_persist`. @@ -7446,316 +7481,91 @@ async fn chat_permission( State(state): State, Json(req): Json, ) -> impl IntoResponse { - use jeikcode_capabilities::tools::{parse_permission_decision, PermissionDecision}; - let project_dir = state.project.read().await.working_dir.clone(); - if req.decision == "allow_persist" { - if let Some(full) = req.tool_name.as_deref() { - let reg = state.mcp_registry.read().await.clone(); - let session_pool = jeikcode_capabilities::mcp::SessionMcpPool::global(); - let session_reg = session_pool - .cached_registry(&project_dir, &req.session_id) - .await; - let split = if let Some(pair) = reg.split_tool_name(full).await { - Some(pair) - } else if let Some(sreg) = &session_reg { - sreg.split_tool_name(full).await - } else { - full.strip_prefix("mcp__") - .and_then(|s| s.split_once("__")) - .map(|(s, t)| (s.to_string(), t.to_string())) - }; - if let Some((server, tool)) = split { - if let Err(e) = jeikcode_capabilities::mcp::config::add_auto_approved_tool( - &project_dir, - &server, - &tool, - ) { - tracing::warn!("[permission] persist autoApprove failed: {e}"); - } - reg.mark_tool_auto_approved(full); - state - .mcp_pool - .registry(&project_dir) - .await - .mark_tool_auto_approved(full); - session_pool - .mark_tool_auto_approved(&project_dir, full) - .await; - let snapshot = - jeikcode_capabilities::mcp::refresh_session_mcp_schema(&project_dir).await; - session_pool.hydrate_project(&project_dir, &snapshot).await; - } - } - let ok = state - .pending_permissions - .deliver(&req.session_id, PermissionDecision::AllowAlways); - if ok { - return Json(serde_json::json!({ "success": true })); - } + use jeikcode_capabilities::tools::parse_permission_decision; + let requested_session_id = req.session_id.trim(); + let approval_id = req.approval_id.trim(); + if requested_session_id.is_empty() || approval_id.is_empty() { + return Json(serde_json::json!({ + "success": false, + "error": "session_id and approval_id are required" + })); } let decision = parse_permission_decision(&req.decision); - if let Some(full) = req.tool_name.as_deref() { - if decision == PermissionDecision::AllowAlways { - state - .mcp_registry - .read() - .await - .mark_tool_auto_approved(full); - state - .mcp_pool - .registry(&project_dir) - .await - .mark_tool_auto_approved(full); - jeikcode_capabilities::mcp::SessionMcpPool::global() - .mark_tool_auto_approved(&project_dir, full) - .await; - } - } - if state.pending_permissions.deliver(&req.session_id, decision) { - return Json(serde_json::json!({ "success": true })); - } - // 容错 1：根据 active_chats 查找对应 session 的 operation 别名并交付 - if let Some(op_id) = state + let active_operation = state .active_chats - .operation_for_session(&req.session_id) - .await - { - if state.pending_permissions.deliver(&op_id, decision) { - return Json(serde_json::json!({ "success": true })); + .operation_for_session(requested_session_id) + .await; + let canonical_session_id = if let Some(operation_id) = active_operation.as_deref() { + state + .active_chats + .session_id(operation_id) + .await + .unwrap_or_else(|| requested_session_id.to_string()) + } else { + requested_session_id.to_string() + }; + + match state.pending_permissions.deliver( + &canonical_session_id, + approval_id, + permission_bridge::PermissionSubmission { + decision, + persist: req.decision == "allow_persist", + }, + ) { + permission_bridge::PermissionDelivery::Submitted { accepted, .. } => { + // A queued oneshot value can still lose a race with the driver's + // request timeout. Report success only after the driver consumes + // this exact decision; timeout/cancel drops the acknowledgement. + return match accepted.await { + Ok(()) => Json(serde_json::json!({ "success": true })), + Err(_) => Json(serde_json::json!({ + "success": false, + "error": "permission request expired" + })), + }; } - } - // 容错 2：若当前仅有唯一待审批会话，直接交付（彻底杜绝桌面端/Webview会话ID轻微差异导致无法审批） - if state.pending_permissions.deliver_any(decision) { - return Json(serde_json::json!({ "success": true })); - } - // 容错 3：若会话在 native_live 注册表中等待审批，交付给 native_live - let approval_resp = match decision { - PermissionDecision::AllowOnce => jeikcode_capabilities::tools::ApprovalResponse::allow(), - PermissionDecision::AllowAlways => { - jeikcode_capabilities::tools::ApprovalResponse::allow_always() + permission_bridge::PermissionDelivery::Expired => { + return Json(serde_json::json!({ + "success": false, + "error": "permission request expired" + })); } - _ => jeikcode_capabilities::tools::ApprovalResponse::deny(), - }; - let approval_val = serde_json::to_value(approval_resp).unwrap_or(serde_json::Value::Null); - if crate::native_live::resolve_pending_kind_via_registry( - &req.session_id, - jeikcode_capabilities::tools::APPROVAL_KIND, - approval_val.clone(), - ) - .is_ok() - { - return Json(serde_json::json!({ "success": true })); + permission_bridge::PermissionDelivery::Absent => {} } - if crate::native_live::respond_pending_kind_confirmed( - jeikcode_capabilities::tools::APPROVAL_KIND, - approval_val, - ) - .await - .is_ok() - { - return Json(serde_json::json!({ "success": true })); + + // An active `/chat` operation owns approval routing exclusively. If its exact + // `(session_id, approval_id)` route is absent, this POST is stale/duplicate; never + // fall through to a different live transport or an on-disk request. + if active_operation.is_some() { + return Json(serde_json::json!({ + "success": false, + "error": "stale or unknown permission request" + })); } - { - // Live turn is not running in memory (e.g. daemon restarted or turn completed/crashed). - // Try recovering and resolving the persisted pending permission from disk. - use jeikcode_capabilities::session::SessionManager; - let Some(manager) = SessionManager::find_manager_for_session(&req.session_id) else { - return Json( - serde_json::json!({ "success": false, "error": "no pending permission for session" }), - ); - }; - let pending = match manager.load_pending_permission(&req.session_id) { - Ok(Some(p)) => p, - _ => { - return Json( - serde_json::json!({ "success": false, "error": "no pending permission for session" }), - ); - } - }; - let lease = match manager.acquire_lease(&req.session_id) { - Ok(l) => l, - Err(e) => { - return Json( - serde_json::json!({ "success": false, "error": format!("lease conflict: {e}") }), - ); - } - }; - let (loaded, _) = match manager.load_native_session_for_resume(&lease) { - Ok(s) => s, - Err(e) => { - return Json( - serde_json::json!({ "success": false, "error": format!("failed to load session: {e}") }), - ); - } - }; - let working_dir = loaded.meta.working_dir.clone(); - let (tool_result_content, is_error) = match decision { - PermissionDecision::Deny => ( - format!( - "[Permission Denied] User declined execution of tool '{}'", - pending.tool_name - ), - true, - ), - _ => { - let mut reg = jeikcode_kernel::tool::ToolRegistry::new(); - jeikcode_capabilities::tools::register_coding_tools(&mut reg); - let ctx = jeikcode_kernel::tool::ToolContext { - working_dir: std::path::PathBuf::from(&working_dir), - cancel: tokio_util::sync::CancellationToken::new(), - progress: jeikcode_kernel::tool::ProgressSink::noop(), - requester: None, - }; - let args_str = if let serde_json::Value::String(s) = &pending.arguments { - s.clone() - } else { - pending.arguments.to_string() - }; - let mounted = reg.mount(&[&pending.tool_name]); - if let Some(tool) = mounted.get(&pending.tool_name) { - let res = tool.execute(&args_str, &ctx).await; - (res.content, res.is_error) - } else if pending.tool_name.starts_with("mcp__") { - let mcp_reg = state - .mcp_pool - .registry(std::path::Path::new(&working_dir)) - .await; - let split = - if let Some(pair) = mcp_reg.split_tool_name(&pending.tool_name).await { - Some(pair) - } else { - pending - .tool_name - .strip_prefix("mcp__") - .and_then(|s| s.split_once("__")) - .map(|(s, t)| (s.to_string(), t.to_string())) - }; - if let Some((server, tool)) = split { - match mcp_reg - .call_tool(&server, &tool, pending.arguments.clone()) - .await - { - Ok(content) => (content, false), - Err(e) => (e.to_string(), true), - } - } else { - (format!("MCP tool '{}' not found", pending.tool_name), true) - } - } else { - ( - format!("Tool '{}' not found in registry", pending.tool_name), - true, - ) - } + // A persisted checkpoint without an active turn is not an executable capability. + // The daemon cannot prove whether the prior owner timed out, was cancelled, or + // crashed after a side effect but before durable history was committed. Replaying + // that call here can therefore duplicate destructive work. Fail closed and require + // the user to retry the operation in a fresh active turn. If this exact stale + // checkpoint still exists, consume the sidecar best-effort so it cannot resurrect + // a ghost card later; a mismatched approval id must never consume a newer checkpoint. + use jeikcode_capabilities::session::SessionManager; + if let Some(manager) = SessionManager::find_manager_for_session(&canonical_session_id) { + if let Ok(Some(pending)) = manager.load_pending_permission(&canonical_session_id) { + if permission_bridge::approval_id_for_pending(&pending) != approval_id { + return Json(serde_json::json!({ + "success": false, + "error": "stale or unknown permission request" + })); } - }; - let mut native_snapshot = loaded.snapshot; - let tool_msg = jeikcode_kernel::message::Message::tool_result( - pending.call_id.clone(), - tool_result_content, - is_error, - ); - native_snapshot.messages.push(tool_msg); - let message_count = u32::try_from(native_snapshot.messages.len()).unwrap_or(0); - let updated_at = jeikcode_capabilities::session::now_ms(); - if let Err(e) = - manager.commit_native_runtime_mutation(&lease, &native_snapshot, move |_, meta, _| { - meta.message_count = message_count; - meta.updated_at = updated_at; - Ok(()) - }) - { - tracing::error!("Failed to commit resumed permission decision: {e}"); + manager.clear_pending_permission(&canonical_session_id); } - manager.clear_pending_permission(&req.session_id); - drop(lease); - - // Resume the turn: spawn continuation so model continues with tool result - let spawn_state = state.clone(); - let session_id_clone = req.session_id.clone(); - let wd_path = std::path::PathBuf::from(working_dir); - tokio::spawn(async move { - let req = ChatRequest { - message: "继续".into(), - session_id: Some(session_id_clone.clone()), - provider: None, - approval_mode: None, - working_dir: Some(wd_path), - extra_system_append: None, - session_title: None, - images: Vec::new(), - request_id: None, - }; - let admission = match spawn_state - .active_chats - .admit(Some(&session_id_clone), None) - .await - { - Ok(a) => a, - Err(e) => { - tracing::warn!( - "Failed to admit continuation turn after permission resolve: {e:?}" - ); - return; - } - }; - let (client_tx, _rx) = mpsc::unbounded_channel::(); - let operation_id = admission.operation_id.clone(); - let cancel_token = admission.cancellation; - let event_bus = spawn_state.active_chats.event_bus(&operation_id).await; - let replay = spawn_state - .active_chats - .event_bus_with_replay(&operation_id) - .await - .map(|(_, r)| r); - let fan_tx = fanout_chat_events_for_session( - client_tx, - event_bus.unwrap_or_else(|| tokio::sync::broadcast::channel(16).0), - replay, - Some(session_id_clone.clone()), - ); - let active_chats = spawn_state.active_chats.clone(); - let mcp_pool = spawn_state.mcp_pool.clone(); - let telemetry = spawn_state.telemetry.clone(); - let pending_permissions = spawn_state.pending_permissions.clone(); - let pending_user_inputs = spawn_state.pending_user_inputs.clone(); - let terminal_sent = Arc::new(std::sync::atomic::AtomicBool::new(false)); - let cleanup_op = operation_id.clone(); - let cleanup_chats = active_chats.clone(); - let chat_session_id = session_id_clone.clone(); - let inner_fan_tx = fan_tx.clone(); - let inner_terminal_sent = terminal_sent.clone(); - let inner = tokio::spawn(async move { - process_chat_request( - req, - inner_fan_tx, - cancel_token, - operation_id, - active_chats, - mcp_pool, - telemetry, - pending_permissions, - pending_user_inputs, - true, - true, - inner_terminal_sent, - false, - ) - .await - }); - finalize_chat_task( - inner, - &fan_tx, - &cleanup_chats, - &cleanup_op, - &chat_session_id, - &terminal_sent, - ) - .await; - }); - - Json(serde_json::json!({ "success": true, "resumed": true })) } + Json(serde_json::json!({ + "success": false, + "error": "permission request is no longer active; retry the operation" + })) } #[derive(Debug, serde::Deserialize)] @@ -10343,6 +10153,396 @@ mod tests { ); } + #[tokio::test(flavor = "current_thread")] + async fn stale_chat_permission_cannot_consume_or_persist_a_reused_call_id() { + let home = ScopedChatHome::new(); + let state = chat_test_state(&home); + let session_id = "11111111-1111-4111-8111-111111111111"; + let tool_name = "mcp__srv__query"; + let admission = state + .active_chats + .admit(Some(session_id), None) + .await + .unwrap(); + state + .active_chats + .bind_session(&admission.operation_id, session_id) + .await + .unwrap(); + + let old_pending = jeikcode_capabilities::session::PendingPermission { + session_id: session_id.into(), + call_id: "ollama_call_0".into(), + tool_name: tool_name.into(), + reason: "Requires approval".into(), + arguments: serde_json::json!({ "query": "old" }), + created_at: 1_000, + }; + let mut current_pending = old_pending.clone(); + current_pending.arguments = serde_json::json!({ "query": "current" }); + current_pending.created_at = 2_000; + let old_approval_id = permission_bridge::approval_id_for_pending(&old_pending); + let current_approval_id = permission_bridge::approval_id_for_pending(¤t_pending); + assert_ne!(old_approval_id, current_approval_id); + + let (tx, rx) = tokio::sync::oneshot::channel(); + state.pending_permissions.register( + session_id.into(), + current_approval_id.clone(), + tool_name.into(), + tx, + ); + + let stale = chat_permission( + State(state.clone()), + Json(PermissionDecisionRequest { + session_id: session_id.into(), + approval_id: old_approval_id, + decision: "allow_persist".into(), + tool_name: Some(tool_name.into()), + }), + ) + .await + .into_response(); + let stale_bytes = axum::body::to_bytes(stale.into_body(), usize::MAX) + .await + .unwrap(); + let stale_body: serde_json::Value = serde_json::from_slice(&stale_bytes).unwrap(); + assert_eq!(stale_body["success"], false); + assert_eq!(stale_body["error"], "stale or unknown permission request"); + + assert!( + !state + .mcp_registry + .read() + .await + .is_tool_auto_approved(tool_name), + "a stale allow_persist must not mutate approval policy" + ); + let stale_always = chat_permission( + State(state.clone()), + Json(PermissionDecisionRequest { + session_id: session_id.into(), + approval_id: permission_bridge::approval_id_for_pending(&old_pending), + decision: "always_allow".into(), + tool_name: Some(tool_name.into()), + }), + ) + .await + .into_response(); + let stale_always_bytes = axum::body::to_bytes(stale_always.into_body(), usize::MAX) + .await + .unwrap(); + let stale_always_body: serde_json::Value = + serde_json::from_slice(&stale_always_bytes).unwrap(); + assert_eq!(stale_always_body["success"], false); + assert!( + !state + .mcp_registry + .read() + .await + .is_tool_auto_approved(tool_name), + "a stale always_allow must not mutate approval policy" + ); + let delivery = state.pending_permissions.deliver( + session_id, + ¤t_approval_id, + permission_bridge::PermissionSubmission { + decision: jeikcode_capabilities::tools::PermissionDecision::Deny, + persist: false, + }, + ); + let permission_bridge::PermissionDelivery::Submitted { accepted, .. } = delivery else { + panic!("the current approval route must remain pending after a stale POST") + }; + let submission = rx.await.unwrap().accept(); + assert!(accepted.await.is_ok()); + assert_eq!( + submission.decision, + jeikcode_capabilities::tools::PermissionDecision::Deny + ); + } + + #[tokio::test(flavor = "current_thread")] + async fn chat_permission_success_requires_driver_consumption_ack() { + let home = ScopedChatHome::new(); + let state = chat_test_state(&home); + let session_id = "12121212-1212-4212-8212-121212121212"; + let admission = state + .active_chats + .admit(Some(session_id), None) + .await + .unwrap(); + state + .active_chats + .bind_session(&admission.operation_id, session_id) + .await + .unwrap(); + + let (tx, rx) = tokio::sync::oneshot::channel(); + state.pending_permissions.register( + session_id.into(), + "approval-accept".into(), + "bash".into(), + tx, + ); + let accepted_request = tokio::spawn(chat_permission( + State(state.clone()), + Json(PermissionDecisionRequest { + session_id: session_id.into(), + approval_id: "approval-accept".into(), + decision: "allow".into(), + tool_name: None, + }), + )); + let envelope = rx.await.unwrap(); + assert!( + !accepted_request.is_finished(), + "HTTP success must wait until the driver consumes the queued decision" + ); + let submission = envelope.submission(); + assert!( + !accepted_request.is_finished(), + "reading the submission is not a runtime-consumption acknowledgement" + ); + assert_eq!( + submission.decision, + jeikcode_capabilities::tools::PermissionDecision::AllowOnce + ); + envelope.acknowledge(); + let accepted_response = accepted_request.await.unwrap().into_response(); + let accepted_bytes = axum::body::to_bytes(accepted_response.into_body(), usize::MAX) + .await + .unwrap(); + let accepted_body: serde_json::Value = serde_json::from_slice(&accepted_bytes).unwrap(); + assert_eq!(accepted_body["success"], true); + + let (tx, rx) = tokio::sync::oneshot::channel(); + state.pending_permissions.register( + session_id.into(), + "approval-drop".into(), + "bash".into(), + tx, + ); + let dropped_request = tokio::spawn(chat_permission( + State(state.clone()), + Json(PermissionDecisionRequest { + session_id: session_id.into(), + approval_id: "approval-drop".into(), + decision: "allow".into(), + tool_name: None, + }), + )); + drop(rx.await.unwrap()); + let dropped_response = dropped_request.await.unwrap().into_response(); + let dropped_bytes = axum::body::to_bytes(dropped_response.into_body(), usize::MAX) + .await + .unwrap(); + let dropped_body: serde_json::Value = serde_json::from_slice(&dropped_bytes).unwrap(); + assert_eq!(dropped_body["success"], false); + assert_eq!(dropped_body["error"], "permission request expired"); + + state.active_chats.complete(&admission.operation_id).await; + } + + #[tokio::test(flavor = "current_thread")] + async fn persisted_chat_permission_requires_an_active_turn_and_exact_approval_id() { + use jeikcode_capabilities::session::{ + PendingPermission, PresentationFile, SessionManager, SessionMeta, StorageOwner, + }; + + let home = ScopedChatHome::new(); + let state = chat_test_state(&home); + let working_dir = home._dir.path().to_path_buf(); + let session_id = "22222222-2222-4222-8222-222222222222"; + let manager = SessionManager::for_project(&working_dir); + let lease = manager.acquire_lease(session_id).unwrap(); + let mut meta = SessionMeta::new(session_id, working_dir.to_string_lossy(), 1); + meta.owner = StorageOwner::Native; + manager + .commit_native_import( + &lease, + Some(&jeikcode_kernel::message::SessionSnapshot::new(Vec::new())), + Some(&PresentationFile::default()), + &meta, + ) + .unwrap(); + drop(lease); + + let pending = PendingPermission { + session_id: session_id.into(), + call_id: "ollama_call_0".into(), + tool_name: "mcp__srv__query".into(), + reason: "Requires approval".into(), + arguments: serde_json::json!({ "query": "current" }), + created_at: 2_000, + }; + manager + .save_pending_permission(session_id, &pending) + .unwrap(); + let exact_approval_id = permission_bridge::approval_id_for_pending(&pending); + let mut old_pending = pending.clone(); + old_pending.created_at = 1_000; + let stale_approval_id = permission_bridge::approval_id_for_pending(&old_pending); + assert_ne!(stale_approval_id, exact_approval_id); + + let stale = chat_permission( + State(state.clone()), + Json(PermissionDecisionRequest { + session_id: session_id.into(), + approval_id: stale_approval_id, + decision: "allow_persist".into(), + tool_name: Some(pending.tool_name.clone()), + }), + ) + .await + .into_response(); + let stale_bytes = axum::body::to_bytes(stale.into_body(), usize::MAX) + .await + .unwrap(); + let stale_body: serde_json::Value = serde_json::from_slice(&stale_bytes).unwrap(); + assert_eq!(stale_body["success"], false); + assert_eq!(stale_body["error"], "stale or unknown permission request"); + assert_eq!( + manager.load_pending_permission(session_id).unwrap(), + Some(pending.clone()), + "a stale persisted approval must not consume the current checkpoint" + ); + assert!( + !state + .mcp_registry + .read() + .await + .is_tool_auto_approved(&pending.tool_name), + "a stale persisted allow_persist must not mutate approval policy" + ); + + let restored = chat_pending( + State(state.clone()), + axum::extract::Query(ChatPendingQuery { + session_id: session_id.into(), + }), + ) + .await + .into_response(); + let restored_bytes = axum::body::to_bytes(restored.into_body(), usize::MAX) + .await + .unwrap(); + let restored_body: serde_json::Value = serde_json::from_slice(&restored_bytes).unwrap(); + assert!( + restored_body["permission"].is_null(), + "an inactive persisted checkpoint must not resurrect an actionable card" + ); + + let exact = chat_permission( + State(state.clone()), + Json(PermissionDecisionRequest { + session_id: session_id.into(), + approval_id: exact_approval_id, + decision: "allow_persist".into(), + tool_name: Some("caller-controlled-name".into()), + }), + ) + .await + .into_response(); + let exact_bytes = axum::body::to_bytes(exact.into_body(), usize::MAX) + .await + .unwrap(); + let exact_body: serde_json::Value = serde_json::from_slice(&exact_bytes).unwrap(); + assert_eq!(exact_body["success"], false); + assert_eq!( + exact_body["error"], + "permission request is no longer active; retry the operation" + ); + assert!( + !manager + .pending_permission_path(session_id) + .unwrap() + .exists(), + "an exact inactive checkpoint should be retired so it cannot resurrect" + ); + assert!( + !state + .mcp_registry + .read() + .await + .is_tool_auto_approved(&pending.tool_name), + "an inactive persisted allow_persist must not mutate approval policy" + ); + assert_eq!( + manager.read_meta(session_id).unwrap().message_count, + 0, + "inactive permission recovery must not synthesize or execute a tool result" + ); + } + + #[tokio::test(flavor = "current_thread")] + async fn stale_live_permission_cannot_mutate_auto_approval_policy() { + let home = ScopedChatHome::new(); + let state = chat_test_state(&home); + let session_id = "33333333-3333-4333-8333-333333333333".to_string(); + let tool_name = "mcp__srv__query"; + let registry = jeikcode_coding::session_runtime_registry::SessionRuntimeRegistry::global(); + registry + .open_or_attach(session_id.clone(), home._dir.path().to_path_buf()) + .unwrap(); + + let response = live_api::live_permission( + State(state.clone()), + Json(live_api::LivePermissionReq { + decision: "allow_persist".into(), + generation: 1, + request_id: 41, + tool_name: Some(tool_name.into()), + session_id: session_id.clone(), + }), + ) + .await + .into_response(); + let bytes = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .unwrap(); + let body: serde_json::Value = serde_json::from_slice(&bytes).unwrap(); + + assert_eq!(body["accepted"], false); + assert!( + !state + .mcp_registry + .read() + .await + .is_tool_auto_approved(tool_name), + "a stale live permission must not mutate approval policy" + ); + registry.force_remove(&session_id); + } + + #[test] + fn live_permission_json_requires_exact_session_and_request_identity() { + let missing_session = + serde_json::from_value::(serde_json::json!({ + "decision": "allow", + "generation": 1, + "request_id": 41, + })); + assert!(missing_session.is_err()); + + let missing_request = + serde_json::from_value::(serde_json::json!({ + "decision": "allow", + "generation": 1, + "session_id": "33333333-3333-4333-8333-333333333333", + })); + assert!(missing_request.is_err()); + + let missing_generation = + serde_json::from_value::(serde_json::json!({ + "decision": "allow", + "session_id": "33333333-3333-4333-8333-333333333333", + "request_id": 41, + })); + assert!(missing_generation.is_err()); + } + #[tokio::test(flavor = "current_thread")] async fn chat_admission_rejects_the_same_session_across_request_aliases() { let home = ScopedChatHome::new(); @@ -12163,6 +12363,7 @@ mod channel_mode_tests { }, ChatEvent::PermissionRequest { session_id: "s1".into(), + approval_id: "approval-c1".into(), tool_name: "task".into(), reason: "Requires approval".into(), call_id: "c1".into(), @@ -12182,6 +12383,7 @@ mod channel_mode_tests { let events = vec![ ChatEvent::PermissionRequest { session_id: "s1".into(), + approval_id: "approval-c1".into(), tool_name: "bash".into(), reason: "Requires approval".into(), call_id: "c1".into(), @@ -12204,6 +12406,7 @@ mod channel_mode_tests { let events = vec![ ChatEvent::PermissionRequest { session_id: "s1".into(), + approval_id: "approval-c1".into(), tool_name: "edit_file".into(), reason: "Requires approval".into(), call_id: "c1".into(), @@ -12225,6 +12428,7 @@ mod channel_mode_tests { let events = vec![ ChatEvent::PermissionRequest { session_id: "s1".into(), + approval_id: "approval-c1".into(), tool_name: "bash".into(), reason: "Requires approval".into(), call_id: "c1".into(), diff --git a/crates/jeikcode-daemon/src/live_api.rs b/crates/jeikcode-daemon/src/live_api.rs index 738bd7bfb..df9eff254 100644 --- a/crates/jeikcode-daemon/src/live_api.rs +++ b/crates/jeikcode-daemon/src/live_api.rs @@ -695,6 +695,66 @@ async fn await_chat_user_input_response( } } +enum ChatPermissionWaitOutcome { + Submission(crate::permission_bridge::PermissionSubmissionEnvelope), + TimedOut, + Closed, +} + +async fn await_chat_permission_response( + rx: tokio::sync::oneshot::Receiver, + request_timeout: Option, +) -> ChatPermissionWaitOutcome { + match request_timeout { + Some(timeout) => match tokio::time::timeout(timeout, rx).await { + Ok(Ok(submission)) => ChatPermissionWaitOutcome::Submission(submission), + Ok(Err(_)) => ChatPermissionWaitOutcome::Closed, + Err(_) => ChatPermissionWaitOutcome::TimedOut, + }, + None => match rx.await { + Ok(submission) => ChatPermissionWaitOutcome::Submission(submission), + Err(_) => ChatPermissionWaitOutcome::Closed, + }, + } +} + +async fn persist_chat_mcp_approval( + project_dir: &Path, + shared_registry: Option<&Arc>, + full_name: &str, +) { + if !full_name.starts_with("mcp__") { + return; + } + let session_pool = jeikcode_capabilities::mcp::SessionMcpPool::global(); + let split = match shared_registry { + Some(registry) => registry.split_tool_name(full_name).await, + None => None, + } + .or_else(|| { + full_name + .strip_prefix("mcp__") + .and_then(|value| value.split_once("__")) + .map(|(server, tool)| (server.to_string(), tool.to_string())) + }); + let Some((server, tool)) = split else { + return; + }; + if let Err(error) = + jeikcode_capabilities::mcp::config::add_auto_approved_tool(project_dir, &server, &tool) + { + tracing::warn!(%error, tool = %full_name, "persist MCP auto-approval failed"); + } + if let Some(registry) = shared_registry { + registry.mark_tool_auto_approved(full_name); + } + session_pool + .mark_tool_auto_approved(project_dir, full_name) + .await; + let snapshot = jeikcode_capabilities::mcp::refresh_session_mcp_schema(project_dir).await; + session_pool.hydrate_project(project_dir, &snapshot).await; +} + fn phase_blocks_new_turn(phase: jeikcode_coding::RuntimePhase) -> bool { matches!( phase, @@ -812,17 +872,27 @@ pub(crate) async fn steer_into_running_turn( } /// Drive a native runtime over `conv` and forward its native events to the shared -/// `/chat` consumer. `perm_rx` carries interactive approval decisions from `/chat/permission` -/// (`None` = apply [`fallback_approval_decision`] for the selected mode). The kernel +/// `/chat` consumer. `permission_responders` registers exact `(session_id, approval_id)` +/// approval routes for `/chat/permission` (`None` = apply [`fallback_approval_decision`] +/// for the selected mode). The kernel /// snapshot is written back to `conv` so the caller persists the completed turn. /// `steer_rx` folds follow-ups into this same turn at the next round boundary. +fn configure_chat_runtime_interactivity( + runtime_cfg: &mut jeikcode_coding::CodingRuntimeConfig, + has_interactive_driver: bool, +) { + if has_interactive_driver { + runtime_cfg.interactive = true; + } +} + pub(crate) async fn run_chat_turn_v2( session_id: String, conv: Arc>>, runtime_event_tx: mpsc::UnboundedSender, cancel: CancellationToken, - runtime_cfg: jeikcode_coding::CodingRuntimeConfig, - mut perm_rx: Option>, + mut runtime_cfg: jeikcode_coding::CodingRuntimeConfig, + permission_responders: Option, user_input_responders: Option, approval_mode: ApprovalMode, mut steer_rx: mpsc::UnboundedReceiver, @@ -830,6 +900,18 @@ pub(crate) async fn run_chat_turn_v2( use jeikcode_capabilities::tools::{ApprovalRequest, ApprovalResponse, APPROVAL_KIND}; use jeikcode_coding::TurnCompletion; + // When this daemon has a real interactive response route, the daemon owns + // cancellation/liveness at the transport seam. Disable the kernel's second, + // equal request timeout so it cannot expire the RequestCtx first and then + // race a later HTTP decision that the daemon still considers pending. + configure_chat_runtime_interactivity( + &mut runtime_cfg, + permission_responders.is_some() || user_input_responders.is_some(), + ); + + let runtime_working_dir = runtime_cfg.working_dir.clone(); + let shared_mcp_registry = runtime_cfg.shared_mcp_registry.clone(); + // Split the just-submitted user input from the persisted prefix before runtime // startup. The buffer already holds kernel messages (cold summaries inline as // synthetic messages), so the prefix IS a `SessionSnapshot` of the remaining @@ -924,6 +1006,7 @@ pub(crate) async fn run_chat_turn_v2( let mut cancelled = false; let mut steer_open = true; + let mut persist_after_start: HashMap = HashMap::new(); let final_messages = loop { let ev = tokio::select! { _ = cancel.cancelled(), if !cancelled => { @@ -959,18 +1042,58 @@ pub(crate) async fn run_chat_turn_v2( }; match ev { event @ CodingRuntimeEvent::Agent(_) => { + if let CodingRuntimeEvent::Agent( + jeikcode_kernel::event::AgentEvent::ToolStarted { call }, + ) = &event + { + if let Some(tool_name) = persist_after_start.remove(&call.id) { + persist_chat_mcp_approval( + &runtime_working_dir, + shared_mcp_registry.as_ref(), + &tool_name, + ) + .await; + } + } let _ = runtime_event_tx.send(event); } CodingRuntimeEvent::Request(request) if request.kind == APPROVAL_KIND => { - if serde_json::from_value::(request.payload.clone()).is_err() { + let Ok(approval) = + serde_json::from_value::(request.payload.clone()) + else { + let _ = handle.respond(request.id, serde_json::Value::Null).await; + continue; + }; + if approval.call_id.trim().is_empty() { let _ = handle.respond(request.id, serde_json::Value::Null).await; continue; } + let manager = jeikcode_capabilities::session::SessionManager::for_project( + &coding_cfg.working_dir, + ); + let pending = manager + .load_pending_permission(&session_id) + .ok() + .flatten() + .filter(|pending| { + pending.call_id == approval.call_id && pending.tool_name == approval.tool + }); + let Some(pending) = pending else { + tracing::warn!( + session_id = %session_id, + call_id = %approval.call_id, + "run_chat_turn_v2: approval checkpoint missing or mismatched; denying" + ); + let _ = handle.respond(request.id, serde_json::Value::Null).await; + continue; + }; + let approval_id = + crate::permission_bridge::approval_id_for_pending(&pending); tracing::warn!( - has_interactive_responder = perm_rx.is_some(), + has_interactive_responder = permission_responders.is_some(), approval_mode = ?approval_mode, payload = %log_truncate_live(&request.payload.to_string(), 300), - "run_chat_turn_v2: approval round-trip (perm_rx Some => permission_request emitted & waits for WebUI; None => fallback decision)" + "run_chat_turn_v2: approval round-trip (responder Some => permission_request emitted & waits for WebUI; None => fallback decision)" ); // Surface the permission modal BEFORE waiting for the decision. // The WebUI card is what prompts the human to POST /chat/permission, @@ -979,45 +1102,115 @@ pub(crate) async fn run_chat_turn_v2( // Re-read live mode so a mid-turn switch to Auto skips the card // and unparks a waiter within the poll interval. let current_mode = live_current_approval_mode(); - let decision = match &mut perm_rx { - None => fallback_approval_decision(current_mode), + let mut accepted_envelope = None; + let (decision, persist_requested) = match &permission_responders { + None => (fallback_approval_decision(current_mode), false), Some(_) if current_mode == ApprovalMode::Auto => { let _ = handle.set_mode(jeikcode_coding::RuntimeMode::Auto).await; - PermissionDecision::AllowOnce + (PermissionDecision::AllowOnce, false) } - Some(rx) => { - let _ = runtime_event_tx - .send(CodingRuntimeEvent::Request(request.clone())); - loop { - tokio::select! { - _ = cancel.cancelled(), if !cancelled => { - cancelled = true; - let _ = handle.cancel().await; - break PermissionDecision::Deny; - } - decision = rx.recv() => { - if live_current_approval_mode() == ApprovalMode::Auto { - let _ = handle.set_mode(jeikcode_coding::RuntimeMode::Auto).await; + Some(responders) => { + let (tx, rx) = tokio::sync::oneshot::channel(); + responders.register( + session_id.clone(), + approval_id.clone(), + approval.tool.clone(), + tx, + ); + let mut surfaced_request = request.clone(); + if let Some(payload) = surfaced_request.payload.as_object_mut() { + payload.insert( + "approval_id".into(), + serde_json::Value::String(approval_id.clone()), + ); + } + if runtime_event_tx + .send(CodingRuntimeEvent::Request(surfaced_request)) + .is_err() + { + responders.expire(&session_id, &approval_id); + jeikcode_capabilities::session::SessionManager::clear_pending_permission_any_project( + &session_id, + ); + (PermissionDecision::Deny, false) + } else { + let response = + await_chat_permission_response(rx, coding_cfg.request_timeout); + tokio::pin!(response); + loop { + tokio::select! { + _ = cancel.cancelled(), if !cancelled => { + cancelled = true; + let _ = handle.cancel().await; + responders.expire(&session_id, &approval_id); + jeikcode_capabilities::session::SessionManager::clear_pending_permission_any_project( + &session_id, + ); + break (PermissionDecision::Deny, false); } - break decision.unwrap_or(PermissionDecision::Deny); - } - _ = tokio::time::sleep(std::time::Duration::from_millis(100)) => { - if live_current_approval_mode() == ApprovalMode::Auto { - let _ = handle.set_mode(jeikcode_coding::RuntimeMode::Auto).await; - break PermissionDecision::AllowOnce; + outcome = &mut response => { + match outcome { + ChatPermissionWaitOutcome::Submission(envelope) => { + // Do not acknowledge HTTP yet. Winning the bridge receive + // branch is not enough: cancellation or the runtime request + // boundary can still reject this decision. Keep the envelope + // alive and ACK only after `handle.respond` succeeds below. + let submission = envelope.submission(); + accepted_envelope = Some(envelope); + if live_current_approval_mode() == ApprovalMode::Auto { + let _ = handle.set_mode(jeikcode_coding::RuntimeMode::Auto).await; + } + let persist = submission.persist + && submission.decision == PermissionDecision::AllowAlways; + break (submission.decision, persist); + } + ChatPermissionWaitOutcome::TimedOut => { + responders.expire(&session_id, &approval_id); + jeikcode_capabilities::session::SessionManager::clear_pending_permission_any_project( + &session_id, + ); + break (PermissionDecision::Deny, false); + } + ChatPermissionWaitOutcome::Closed => { + break (PermissionDecision::Deny, false); + } + } + } + _ = tokio::time::sleep(std::time::Duration::from_millis(100)) => { + if live_current_approval_mode() == ApprovalMode::Auto { + let _ = handle.set_mode(jeikcode_coding::RuntimeMode::Auto).await; + responders.unregister(&session_id, &approval_id); + break (PermissionDecision::AllowOnce, false); + } } } } } } }; + if persist_requested { + persist_after_start.insert(approval.call_id.clone(), approval.tool.clone()); + } let response = match decision { PermissionDecision::AllowOnce => ApprovalResponse::allow(), PermissionDecision::AllowAlways => ApprovalResponse::allow_always(), _ => ApprovalResponse::deny(), }; let value = serde_json::to_value(response).unwrap_or(serde_json::Value::Null); - let _ = handle.respond(request.id, value).await; + match handle.respond(request.id, value).await { + Ok(()) => { + if let Some(envelope) = accepted_envelope.take() { + envelope.acknowledge(); + } + } + Err(_) => { + persist_after_start.remove(&approval.call_id); + // Dropping an unacknowledged envelope closes the HTTP ACK + // channel, so `/chat/permission` reports failure instead of + // claiming that a rejected/cancelled request was accepted. + accepted_envelope.take(); + } + } } CodingRuntimeEvent::Request(request) if request.kind @@ -1254,6 +1447,8 @@ pub(crate) enum LiveWireEvent { PersistenceWarning { message: String }, #[serde(rename = "permission_request")] PermissionRequest { + generation: u64, + request_id: u64, tool_name: String, reason: String, call_id: String, @@ -1320,7 +1515,16 @@ struct NativeLiveWireProjector { } impl NativeLiveWireProjector { + #[cfg(test)] fn project(&mut self, event: crate::live_hub::LiveViewEvent) -> Option { + self.project_at_generation(0, event) + } + + fn project_at_generation( + &mut self, + generation: u64, + event: crate::live_hub::LiveViewEvent, + ) -> Option { use jeikcode_capabilities::tools::{ request_user_input::REQUEST_USER_INPUT_KIND, ApprovalRequest, APPROVAL_KIND, }; @@ -1466,6 +1670,8 @@ impl NativeLiveWireProjector { if request.kind == APPROVAL_KIND { let approval: ApprovalRequest = serde_json::from_value(request.payload).ok()?; LiveWireEvent::PermissionRequest { + generation, + request_id: request.id, tool_name: approval.tool, reason: "Requires approval".into(), call_id: approval.call_id, @@ -1729,6 +1935,7 @@ fn project_registry_event( sequenced: jeikcode_coding::session_runtime_registry::SequencedSessionEvent, ) -> Option { use jeikcode_coding::session_runtime_registry::SessionViewEvent; + let generation = sequenced.generation; if let Some(view) = sequenced.view { let live = match view { SessionViewEvent::InputAccepted { @@ -1754,11 +1961,12 @@ fn project_registry_event( crate::live_hub::LiveViewEvent::RequestResolved { request_id, kind } } }; - return projector.project(live); + return projector.project_at_generation(generation, live); } - sequenced - .runtime - .and_then(|runtime| projector.project(crate::live_hub::LiveViewEvent::Runtime(runtime))) + sequenced.runtime.and_then(|runtime| { + projector + .project_at_generation(generation, crate::live_hub::LiveViewEvent::Runtime(runtime)) + }) } fn live_stream_from_registry( @@ -1936,7 +2144,8 @@ fn live_stream_from_hub_join(join: crate::live_hub::LiveJoin) -> axum::response: ..Default::default() }; for observation in join.replay { - if let Some(w) = projector.project(observation.event) { + if let Some(w) = projector.project_at_generation(observation.generation, observation.event) + { let _ = tx.send(w); } } @@ -1946,7 +2155,9 @@ fn live_stream_from_hub_join(join: crate::live_hub::LiveJoin) -> axum::response: loop { match rx.recv().await { Ok(observation) if observation.binding_id == binding_id => { - if let Some(w) = projector.project(observation.event) { + if let Some(w) = + projector.project_at_generation(observation.generation, observation.event) + { if tx.send(w).is_err() { break; } @@ -3025,12 +3236,16 @@ pub(crate) async fn live_reasoning_effort( #[derive(serde::Deserialize)] pub(crate) struct LivePermissionReq { pub decision: String, // "allow" | "deny" | "always_allow" | "allow_persist" + /// Exact runtime generation that emitted the approval request. + pub generation: u64, + /// Exact native runtime request id from the `permission_request` event. + pub request_id: u64, /// Full MCP tool name (`mcp__{server}__{tool}`); required for `allow_persist`. #[serde(default)] pub tool_name: Option, - /// Target session when the hub is bound to a different view. - #[serde(default)] - pub session_id: Option, + /// Exact session that emitted this approval request. Required so a stale + /// browser card cannot fall through to a different hub/registry runtime. + pub session_id: String, } /// POST /live/permission — Deliver a permission decision for a pending live-session tool-approval @@ -3041,73 +3256,16 @@ pub(crate) struct LivePermissionReq { /// "always_allow" → PermissionDecision::AllowAlways (persisted for the session) /// anything else → PermissionDecision::Deny pub(crate) async fn live_permission( - State(state): State, + State(_state): State, Json(req): Json, ) -> impl IntoResponse { use jeikcode_capabilities::tools::{parse_permission_decision, PermissionDecision}; - let decision = if req.decision == "allow_persist" { - if let Some(full) = req.tool_name.as_deref() { - let reg = state.mcp_registry.read().await.clone(); - let project_dir = state.project.read().await.working_dir.clone(); - let session_pool = jeikcode_capabilities::mcp::SessionMcpPool::global(); - let session_reg = match req.session_id.as_deref() { - Some(sid) => session_pool.cached_registry(&project_dir, sid).await, - None => None, - }; - let split = if let Some(pair) = reg.split_tool_name(full).await { - Some(pair) - } else if let Some(sreg) = &session_reg { - sreg.split_tool_name(full).await - } else { - full.strip_prefix("mcp__") - .and_then(|s| s.split_once("__")) - .map(|(s, t)| (s.to_string(), t.to_string())) - }; - if let Some((server, tool)) = split { - if let Err(e) = jeikcode_capabilities::mcp::config::add_auto_approved_tool( - &project_dir, - &server, - &tool, - ) { - tracing::warn!("[permission] persist autoApprove failed: {e}"); - } - reg.mark_tool_auto_approved(full); - state - .mcp_pool - .registry(&project_dir) - .await - .mark_tool_auto_approved(full); - session_pool - .mark_tool_auto_approved(&project_dir, full) - .await; - let snapshot = - jeikcode_capabilities::mcp::refresh_session_mcp_schema(&project_dir).await; - session_pool.hydrate_project(&project_dir, &snapshot).await; - } - } - PermissionDecision::AllowAlways - } else { - let d = parse_permission_decision(&req.decision); - if let Some(full) = req.tool_name.as_deref() { - if d == PermissionDecision::AllowAlways { - let project_dir = state.project.read().await.working_dir.clone(); - state - .mcp_registry - .read() - .await - .mark_tool_auto_approved(full); - state - .mcp_pool - .registry(&project_dir) - .await - .mark_tool_auto_approved(full); - jeikcode_capabilities::mcp::SessionMcpPool::global() - .mark_tool_auto_approved(&project_dir, full) - .await; - } - } - d - }; + + let session_id = req.session_id.trim(); + if session_id.is_empty() { + return Json(serde_json::json!({ "accepted": false })); + } + let decision = parse_permission_decision(&req.decision); let response = match decision { PermissionDecision::AllowOnce => jeikcode_capabilities::tools::ApprovalResponse::allow(), PermissionDecision::AllowAlways => { @@ -3116,34 +3274,30 @@ pub(crate) async fn live_permission( _ => jeikcode_capabilities::tools::ApprovalResponse::deny(), }; let value = serde_json::to_value(response).unwrap_or(serde_json::Value::Null); - let session_id = parse_session_id(req.session_id); - let mut ok = if let Some(session_id) = session_id - .as_ref() - .filter(|id| crate::native_live::prefer_registry_live_stream(id)) - { - crate::native_live::resolve_pending_kind_via_registry( + let accepted = if crate::native_live::prefer_registry_live_stream(session_id) { + crate::native_live::resolve_via_registry_confirmed( session_id, - jeikcode_capabilities::tools::APPROVAL_KIND, + req.generation, + req.request_id, value, - ) - .is_ok() - } else { - crate::native_live::respond_pending_kind_confirmed( jeikcode_capabilities::tools::APPROVAL_KIND, - value, ) .await .is_ok() + } else if crate::native_live::live_execution_session_id().as_deref() == Some(session_id) { + crate::native_live::respond_confirmed_for_generation(req.generation, req.request_id, value) + .await + .is_ok() + } else { + false }; - if !ok { - if let Some(ref sid) = session_id { - ok = state.pending_permissions.deliver(sid, decision); - } - if !ok { - ok = state.pending_permissions.deliver_any(decision); - } - } - Json(serde_json::json!({ "accepted": ok })) + + // The live permission route deliberately has no daemon-level persistence side effect. + // Caller-provided tool metadata must not update MCP/config policy directly. + // AllowAlways is honored by the runtime approval middleware only after consuming + // this exact `(session_id, request_id)` response. + let _reported_tool_name = req.tool_name.as_deref(); + Json(serde_json::json!({ "accepted": accepted })) } #[derive(Debug, serde::Deserialize)] @@ -3904,6 +4058,59 @@ mod tests { assert_eq!(batch["responses"][1]["text"], "wait for next message"); } + #[tokio::test] + async fn chat_permission_wait_reports_driver_timeout() { + let (_tx, rx) = tokio::sync::oneshot::channel(); + let outcome = + await_chat_permission_response(rx, Some(std::time::Duration::from_millis(1))).await; + + assert!(matches!(outcome, ChatPermissionWaitOutcome::TimedOut)); + } + + #[test] + fn interactive_chat_driver_disables_the_kernel_request_timeout() { + let config = jeikcode_config::config::Config::default(); + let mut runtime_cfg = jeikcode_coding::CodingRuntimeConfig::from_config( + &config, + std::path::Path::new("."), + None, + None, + false, + false, + ); + assert!(runtime_cfg.agent_config().request_timeout.is_some()); + configure_chat_runtime_interactivity(&mut runtime_cfg, true); + assert!(runtime_cfg.agent_config().request_timeout.is_none()); + } + + #[tokio::test] + async fn chat_permission_wait_returns_the_received_decision() { + let (tx, rx) = tokio::sync::oneshot::channel(); + tx.send( + crate::permission_bridge::PermissionSubmissionEnvelope::without_ack( + crate::permission_bridge::PermissionSubmission { + decision: PermissionDecision::AllowOnce, + persist: false, + }, + ), + ) + .unwrap(); + + let outcome = + await_chat_permission_response(rx, Some(std::time::Duration::from_secs(1))).await; + + let ChatPermissionWaitOutcome::Submission(envelope) = outcome else { + panic!("queued decision must be returned") + }; + assert_eq!( + envelope.accept(), + crate::permission_bridge::PermissionSubmission { + decision: PermissionDecision::AllowOnce, + persist: false, + } + ); + } + #[tokio::test] async fn chat_user_input_wait_degrades_to_null_at_driver_timeout() { let (_tx, rx) = tokio::sync::oneshot::channel(); @@ -4160,6 +4367,35 @@ mod tests { .is_none()); } + #[test] + fn native_live_permission_projects_runtime_generation_with_request_id() { + use jeikcode_capabilities::tools::{ApprovalRequest, APPROVAL_KIND}; + + let mut projector = NativeLiveWireProjector::default(); + let request = jeikcode_coding::RuntimeRequest { + id: 1, + kind: APPROVAL_KIND.into(), + payload: serde_json::to_value(ApprovalRequest { + call_id: "call-1".into(), + tool: "write_file".into(), + args: "{\"path\":\"README.md\"}".into(), + }) + .unwrap(), + snapshot: None, + }; + let wire = projector + .project_at_generation( + 9, + crate::live_hub::LiveViewEvent::Runtime(CodingRuntimeEvent::Request(request)), + ) + .expect("approval request must reach the live wire"); + let json = serde_json::to_value(wire).unwrap(); + assert_eq!(json["type"], "permission_request"); + assert_eq!(json["generation"], 9); + assert_eq!(json["request_id"], 1); + assert_eq!(json["call_id"], "call-1"); + } + #[test] fn native_live_projector_exposes_exact_steered_inputs() { let mut projector = NativeLiveWireProjector::default(); diff --git a/crates/jeikcode-daemon/src/live_hub.rs b/crates/jeikcode-daemon/src/live_hub.rs index 9a71e2ea6..d74ed99e7 100644 --- a/crates/jeikcode-daemon/src/live_hub.rs +++ b/crates/jeikcode-daemon/src/live_hub.rs @@ -1089,6 +1089,61 @@ impl LiveViewHub { Ok(()) } + /// Confirm a response against the exact runtime generation that emitted the + /// request. A delayed browser response from an older generation must never + /// be rebound to a replacement runtime that happens to reuse the same + /// kernel request id. + pub async fn respond_confirmed_for_generation( + &self, + expected_generation: u64, + id: RequestId, + value: serde_json::Value, + ) -> Result<(), HubError> { + { + let state = self.state.lock().unwrap_or_else(|error| error.into_inner()); + let current = state.binding.as_ref().ok_or(HubError::Unbound)?; + if current.identity.generation != expected_generation { + return Err(HubError::RuntimeGenerationChanged { + expected: expected_generation, + actual: current.identity.generation, + }); + } + if !state.pending_requests.contains_key(&id) { + return Err(HubError::UnknownRequest(id)); + } + } + let (binding, handle) = self.bound_handle()?; + if binding.generation != expected_generation { + return Err(HubError::RuntimeGenerationChanged { + expected: expected_generation, + actual: binding.generation, + }); + } + handle + .respond_for_generation( + jeikcode_coding::RuntimeGeneration(expected_generation), + id, + value, + ) + .await + .map_err(|error| HubError::RuntimeRejected(error.to_string()))?; + let mut state = self.state.lock().unwrap_or_else(|error| error.into_inner()); + let current = state.binding.as_ref().ok_or(HubError::Unbound)?; + if current.identity.id != binding.id { + return Err(HubError::StaleBinding); + } + if current.identity.generation != expected_generation { + return Err(HubError::RuntimeGenerationChanged { + expected: expected_generation, + actual: current.identity.generation, + }); + } + if state.pending_requests.contains_key(&id) { + self.resolve_request_locked(&mut state, id)?; + } + Ok(()) + } + pub fn respond_pending_kind( &self, kind: &str, diff --git a/crates/jeikcode-daemon/src/native_live.rs b/crates/jeikcode-daemon/src/native_live.rs index 5d462c826..6eb4dc3d3 100644 --- a/crates/jeikcode-daemon/src/native_live.rs +++ b/crates/jeikcode-daemon/src/native_live.rs @@ -37,6 +37,13 @@ pub fn live_running_session_id() -> Option { hub().running_session_id() } +/// Session that owns the embedded live runtime execution. Unlike the current +/// projected/view session, this identity is authoritative for routing an exact +/// native request response back to the hub driver. +pub fn live_execution_session_id() -> Option { + hub().execution_session_id() +} + /// Session currently projected in the live WebUI/TUI, including while its /// runtime is idle. Status panels must use this rather than /// `live_running_session_id`, otherwise session-owned resources falsely appear @@ -558,6 +565,42 @@ pub fn resolve_via_registry( Ok(()) } +/// Deliver an exact response to a registry-owned runtime and wait until the +/// runtime accepts that request id before projecting it as resolved. This is +/// the approval-safe counterpart to [`resolve_via_registry`]: callers must not +/// report success merely because a command was enqueued while the runtime has +/// already timed out or advanced to another pending request. +pub async fn resolve_via_registry_confirmed( + session_id: &str, + generation: u64, + id: jeikcode_kernel::event::RequestId, + value: serde_json::Value, + kind: &str, +) -> Result<(), String> { + let reg = jeikcode_coding::session_runtime_registry::SessionRuntimeRegistry::global(); + let key = session_id.to_string(); + let handle = reg + .handle(&key) + .ok_or_else(|| format!("session {session_id} has no live registry handle for response"))?; + handle + .respond_for_generation(jeikcode_coding::RuntimeGeneration(generation), id, value) + .await + .map_err(|error| format!("registry response rejected: {error}"))?; + let working_dir = reg + .lookup(&key) + .map(|entry| entry.working_dir) + .unwrap_or_else(|| PathBuf::from(".")); + let _ = reg.ensure_and_push_view( + key, + working_dir, + jeikcode_coding::session_runtime_registry::SessionViewEvent::RequestResolved { + request_id: id, + kind: kind.to_string(), + }, + ); + Ok(()) +} + /// Respond to the latest pending request on a registry session. pub fn resolve_pending_kind_via_registry( session_id: &str, @@ -686,6 +729,16 @@ pub async fn respond_confirmed( hub().respond_confirmed(id, value).await } +pub async fn respond_confirmed_for_generation( + generation: u64, + id: jeikcode_kernel::event::RequestId, + value: serde_json::Value, +) -> Result<(), HubError> { + hub() + .respond_confirmed_for_generation(generation, id, value) + .await +} + pub async fn respond_pending_kind_confirmed( kind: &str, value: serde_json::Value, diff --git a/crates/jeikcode-daemon/src/permission_bridge.rs b/crates/jeikcode-daemon/src/permission_bridge.rs index 6a1cb0877..5ac055d9e 100644 --- a/crates/jeikcode-daemon/src/permission_bridge.rs +++ b/crates/jeikcode-daemon/src/permission_bridge.rs @@ -1,19 +1,112 @@ //! 把 daemon `/chat` 的交互式权限决策桥接到 HTTP。 //! -//! `InteractivePermissionDecider` 阻塞在 `response_rx` 上等待决定； -//! `/chat/permission` 收到前端决定后，经 `PermissionResponders` 按 -//! session_id 路由到对应的 `response_tx`，唤醒 decider。 +//! `/chat` approvals are correlated by `(session_id, approval_id)`. The exact +//! route is consumed before a decision is delivered, so duplicate/stale POSTs +//! cannot fall through to a later approval in the same session. use jeikcode_capabilities::tools::PermissionDecision; use std::collections::HashMap; use std::sync::{Arc, RwLock}; -use tokio::sync::mpsc::UnboundedSender; use tokio::sync::oneshot; -/// session_id -> decider 的 response 发送端。 +struct PendingPermissionResponder { + tx: oneshot::Sender, + tool_name: String, +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct PermissionSubmission { + pub decision: PermissionDecision, + pub persist: bool, +} + +/// One exact HTTP approval submission plus an acknowledgement that is resolved +/// only when the runtime driver actually consumes the decision. A successful +/// oneshot send alone is not enough: the driver's request timeout can win the +/// same scheduling race after the value was queued but before it was observed. +#[derive(Debug)] +pub struct PermissionSubmissionEnvelope { + submission: PermissionSubmission, + accepted: Option>, +} + +impl PermissionSubmissionEnvelope { + fn with_ack(submission: PermissionSubmission, accepted: oneshot::Sender<()>) -> Self { + Self { + submission, + accepted: Some(accepted), + } + } + + pub(crate) fn without_ack(submission: PermissionSubmission) -> Self { + Self { + submission, + accepted: None, + } + } + + pub fn submission(&self) -> PermissionSubmission { + self.submission + } + + /// Acknowledge only after the runtime has consumed this exact decision. + /// Queueing the envelope at the HTTP bridge is not sufficient: timeout or + /// cancellation may still win before the runtime request boundary accepts it. + pub fn acknowledge(mut self) -> PermissionSubmission { + if let Some(accepted) = self.accepted.take() { + let _ = accepted.send(()); + } + self.submission + } + + /// Back-compat helper for tests/callers that already established runtime + /// consumption by some other means. + pub fn accept(self) -> PermissionSubmission { + self.acknowledge() + } +} + +enum PermissionResponderEntry { + Pending(PendingPermissionResponder), + /// The runtime already timed out/cancelled this exact approval. Keep a + /// per-turn tombstone until normal turn cleanup so a racing late POST is + /// rejected rather than falling through to another transport. + Expired, +} + +pub enum PermissionDelivery { + Submitted { + tool_name: String, + accepted: oneshot::Receiver<()>, + }, + Expired, + Absent, +} + +/// Stable identity for one persisted pending approval. The runtime writes the +/// checkpoint before publishing the request, so the same id can be reconstructed +/// after refresh/restart while a later turn that reuses the provider call id gets +/// a different id from `created_at`. +pub fn approval_id_for_pending( + pending: &jeikcode_capabilities::session::PendingPermission, +) -> String { + use sha2::{Digest, Sha256}; + + let mut digest = Sha256::new(); + digest.update(pending.session_id.as_bytes()); + digest.update([0]); + digest.update(pending.created_at.to_le_bytes()); + digest.update([0]); + digest.update(pending.call_id.as_bytes()); + digest.update([0]); + digest.update(pending.tool_name.as_bytes()); + format!("{:x}", digest.finalize()) +} + +/// Exact `/chat` approval routes keyed by `(session_id, approval_id)`. #[derive(Clone, Default)] pub struct PermissionResponders { - inner: Arc>>>, + inner: Arc>>, } /// A pending `/chat` structured-input request, keyed by the native runtime request id. @@ -69,48 +162,116 @@ impl PermissionResponders { Self::default() } - /// 登记某 session 的决定发送端（`/chat` 启动时调用）。 - pub fn register(&self, session_id: String, tx: UnboundedSender) { - self.inner.write().unwrap().insert(session_id, tx); + /// Register exactly one approval round-trip. + pub fn register( + &self, + session_id: String, + approval_id: String, + tool_name: String, + tx: oneshot::Sender, + ) { + self.inner.write().unwrap().insert( + (session_id, approval_id), + PermissionResponderEntry::Pending(PendingPermissionResponder { tx, tool_name }), + ); } - /// `/chat` 结束时清理。 - pub fn unregister(&self, session_id: &str) { - self.inner.write().unwrap().remove(session_id); + /// Remove one exact pending approval after it was resolved locally (for + /// example by switching to Auto). Timeout/cancel should call [`Self::expire`] + /// instead so a racing stale POST sees an explicit tombstone. + pub fn unregister(&self, session_id: &str, approval_id: &str) { + self.inner + .write() + .unwrap() + .remove(&(session_id.to_owned(), approval_id.to_owned())); } - /// 把决定送给对应 session 的 decider。返回是否成功（session 是否在等待）。 - pub fn deliver(&self, session_id: &str, decision: PermissionDecision) -> bool { - let clean = session_id.trim(); - if let Some(tx) = self.inner.read().unwrap().get(clean) { - tx.send(decision).is_ok() - } else { - false + /// Mark one exact approval as expired while preserving a short-lived + /// tombstone until turn cleanup. + pub fn expire(&self, session_id: &str, approval_id: &str) { + let key = (session_id.to_owned(), approval_id.to_owned()); + let mut guard = self.inner.write().unwrap(); + if guard.contains_key(&key) { + guard.insert(key, PermissionResponderEntry::Expired); } } - /// 容错兜底：若仅有唯一一个待决策的 session，直接向其交付决定（防止桌面端/Webview 别名或ID轻微差异导致决策丢失） - pub fn deliver_any(&self, decision: PermissionDecision) -> bool { - let guard = self.inner.read().unwrap(); - if guard.len() == 1 { - if let Some(tx) = guard.values().next() { - return tx.send(decision).is_ok(); + pub fn unregister_session(&self, session_id: &str) { + self.inner + .write() + .unwrap() + .retain(|(registered, _), _| registered != session_id); + } + + /// Submit one exact approval. Removing the pending route and queuing the + /// decision claims the HTTP route, but the caller must still await the + /// returned `accepted` receiver before reporting success to the user. The + /// runtime resolves that acknowledgement only after it actually consumes + /// the decision rather than timing out/cancelling concurrently. + pub fn deliver( + &self, + session_id: &str, + approval_id: &str, + submission: PermissionSubmission, + ) -> PermissionDelivery { + let key = (session_id.to_owned(), approval_id.to_owned()); + let mut guard = self.inner.write().unwrap(); + match guard.remove(&key) { + None => PermissionDelivery::Absent, + Some(PermissionResponderEntry::Expired) => { + guard.insert(key, PermissionResponderEntry::Expired); + PermissionDelivery::Expired + } + Some(PermissionResponderEntry::Pending(pending)) => { + let (accepted_tx, accepted_rx) = oneshot::channel(); + match pending.tx.send(PermissionSubmissionEnvelope::with_ack( + submission, + accepted_tx, + )) { + Ok(()) => PermissionDelivery::Submitted { + tool_name: pending.tool_name, + accepted: accepted_rx, + }, + Err(_) => { + guard.insert(key, PermissionResponderEntry::Expired); + PermissionDelivery::Expired + } + } } } - false } - /// 把决定广播送给所有当前正在等待的 session（例如切换为 Auto 模式时立即放行解冻）。 - /// 返回成功送达的 session 数量。 + /// Resolve every approval that is pending *now* (used when switching to Auto). + /// Expired tombstones remain until turn cleanup. pub fn deliver_all(&self, decision: PermissionDecision) -> usize { - let senders: Vec<_> = self.inner.read().unwrap().values().cloned().collect(); - let mut count = 0; - for tx in senders { - if tx.send(decision).is_ok() { - count += 1; - } - } - count + let mut guard = self.inner.write().unwrap(); + let keys: Vec<_> = guard + .iter() + .filter_map(|(key, value)| { + matches!(value, PermissionResponderEntry::Pending(_)).then(|| key.clone()) + }) + .collect(); + let pending: Vec<_> = keys + .into_iter() + .filter_map(|key| match guard.remove(&key) { + Some(PermissionResponderEntry::Pending(pending)) => Some(pending.tx), + _ => None, + }) + .collect(); + drop(guard); + pending + .into_iter() + .map(|tx| { + tx.send(PermissionSubmissionEnvelope::without_ack( + PermissionSubmission { + decision, + persist: false, + }, + )) + .is_ok() + }) + .filter(|sent| *sent) + .count() } } @@ -119,37 +280,193 @@ mod tests { use super::*; use jeikcode_capabilities::tools::PermissionDecision; + #[test] + fn approval_id_distinguishes_reused_provider_call_ids_across_checkpoints() { + let first = jeikcode_capabilities::session::PendingPermission { + session_id: "sess-1".into(), + call_id: "ollama_call_0".into(), + tool_name: "bash".into(), + reason: "Requires approval".into(), + arguments: serde_json::json!({ "command": "git status" }), + created_at: 1000, + }; + let mut second = first.clone(); + second.created_at = 2000; + + assert_eq!( + approval_id_for_pending(&first), + approval_id_for_pending(&first) + ); + assert_ne!( + approval_id_for_pending(&first), + approval_id_for_pending(&second) + ); + } + #[tokio::test] async fn routes_decision_to_registered_session() { let reg = PermissionResponders::new(); - let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel(); - reg.register("sess-1".into(), tx); + let (tx, rx) = tokio::sync::oneshot::channel(); + reg.register("sess-1".into(), "call-1".into(), "bash".into(), tx); - assert!(reg.deliver("sess-1", PermissionDecision::AllowOnce)); + let delivery = reg.deliver( + "sess-1", + "call-1", + PermissionSubmission { + decision: PermissionDecision::AllowOnce, + persist: false, + }, + ); + let PermissionDelivery::Submitted { + tool_name, + mut accepted, + } = delivery + else { + panic!("registered approval must be submitted") + }; + assert_eq!(tool_name, "bash"); assert!(matches!( - rx.recv().await, - Some(PermissionDecision::AllowOnce) + accepted.try_recv(), + Err(tokio::sync::oneshot::error::TryRecvError::Empty) )); + let envelope = rx.await.unwrap(); + let submission = envelope.accept(); + assert_eq!( + submission, + PermissionSubmission { + decision: PermissionDecision::AllowOnce, + persist: false, + } + ); + assert!(accepted.await.is_ok()); + } + + #[tokio::test] + async fn submitted_decision_is_rejected_if_runtime_drops_it_before_consuming() { + let reg = PermissionResponders::new(); + let (tx, rx) = tokio::sync::oneshot::channel(); + reg.register("sess-1".into(), "call-1".into(), "bash".into(), tx); + + let delivery = reg.deliver( + "sess-1", + "call-1", + PermissionSubmission { + decision: PermissionDecision::AllowOnce, + persist: false, + }, + ); + let PermissionDelivery::Submitted { accepted, .. } = delivery else { + panic!("registered approval must be submitted") + }; + drop(rx.await.unwrap()); + assert!( + accepted.await.is_err(), + "HTTP acknowledgement must fail when the runtime drops a queued decision" + ); } #[test] - fn deliver_to_unknown_session_returns_false() { + fn stale_or_duplicate_approval_id_is_rejected() { let reg = PermissionResponders::new(); - assert!(!reg.deliver("nope", PermissionDecision::Deny)); + let (tx, _rx) = tokio::sync::oneshot::channel(); + reg.register("sess-1".into(), "call-new".into(), "bash".into(), tx); + assert!(matches!( + reg.deliver( + "sess-1", + "call-old", + PermissionSubmission { + decision: PermissionDecision::AllowOnce, + persist: false, + }, + ), + PermissionDelivery::Absent + )); + assert!(matches!( + reg.deliver( + "sess-1", + "call-new", + PermissionSubmission { + decision: PermissionDecision::Deny, + persist: false, + }, + ), + PermissionDelivery::Submitted { .. } + )); + assert!(matches!( + reg.deliver( + "sess-1", + "call-new", + PermissionSubmission { + decision: PermissionDecision::AllowOnce, + persist: false, + }, + ), + PermissionDelivery::Absent + )); + } + + #[test] + fn expired_approval_remains_a_tombstone_until_session_cleanup() { + let reg = PermissionResponders::new(); + let (tx, _rx) = tokio::sync::oneshot::channel(); + reg.register("sess-1".into(), "call-1".into(), "bash".into(), tx); + reg.expire("sess-1", "call-1"); + + assert!(matches!( + reg.deliver( + "sess-1", + "call-1", + PermissionSubmission { + decision: PermissionDecision::AllowOnce, + persist: false, + }, + ), + PermissionDelivery::Expired + )); + assert!(matches!( + reg.deliver( + "sess-1", + "call-1", + PermissionSubmission { + decision: PermissionDecision::AllowOnce, + persist: false, + }, + ), + PermissionDelivery::Expired + )); + + reg.unregister_session("sess-1"); + assert!(matches!( + reg.deliver( + "sess-1", + "call-1", + PermissionSubmission { + decision: PermissionDecision::AllowOnce, + persist: false, + }, + ), + PermissionDelivery::Absent + )); } #[tokio::test] async fn deliver_all_wakes_all_waiting_sessions() { let reg = PermissionResponders::new(); - let (tx1, mut rx1) = tokio::sync::mpsc::unbounded_channel(); - let (tx2, mut rx2) = tokio::sync::mpsc::unbounded_channel(); - reg.register("sess-1".into(), tx1); - reg.register("sess-2".into(), tx2); + let (tx1, rx1) = tokio::sync::oneshot::channel(); + let (tx2, rx2) = tokio::sync::oneshot::channel(); + reg.register("sess-1".into(), "call-1".into(), "bash".into(), tx1); + reg.register("sess-2".into(), "call-2".into(), "write".into(), tx2); let count = reg.deliver_all(PermissionDecision::AllowOnce); assert_eq!(count, 2); - assert_eq!(rx1.recv().await, Some(PermissionDecision::AllowOnce)); - assert_eq!(rx2.recv().await, Some(PermissionDecision::AllowOnce)); + assert_eq!( + rx1.await.unwrap().accept().decision, + PermissionDecision::AllowOnce + ); + assert_eq!( + rx2.await.unwrap().accept().decision, + PermissionDecision::AllowOnce + ); } #[tokio::test] diff --git a/extensions/jetbrains/src/main/kotlin/com/jeikcode/jetbrains/daemon/JeikCodeApiClient.kt b/extensions/jetbrains/src/main/kotlin/com/jeikcode/jetbrains/daemon/JeikCodeApiClient.kt index 5a03d03fd..b6fe40dfb 100644 --- a/extensions/jetbrains/src/main/kotlin/com/jeikcode/jetbrains/daemon/JeikCodeApiClient.kt +++ b/extensions/jetbrains/src/main/kotlin/com/jeikcode/jetbrains/daemon/JeikCodeApiClient.kt @@ -11,6 +11,7 @@ interface JeikCodeApiClient { fun streamChat(request: ChatRequest, onEvent: (ChatEvent) -> Unit): CompletableFuture fun sendPermissionDecision( sessionId: String, + approvalId: String, decision: String, toolName: String? = null, ): CompletableFuture @@ -36,8 +37,9 @@ class ExistingDaemonApiClient( override fun sendPermissionDecision( sessionId: String, + approvalId: String, decision: String, toolName: String?, ): CompletableFuture = - delegate.sendPermissionDecision(sessionId, decision, toolName) + delegate.sendPermissionDecision(sessionId, approvalId, decision, toolName) } diff --git a/extensions/jetbrains/src/main/kotlin/com/jeikcode/jetbrains/daemon/JeikCodeDaemonClient.kt b/extensions/jetbrains/src/main/kotlin/com/jeikcode/jetbrains/daemon/JeikCodeDaemonClient.kt index fb97ca775..3258fe705 100644 --- a/extensions/jetbrains/src/main/kotlin/com/jeikcode/jetbrains/daemon/JeikCodeDaemonClient.kt +++ b/extensions/jetbrains/src/main/kotlin/com/jeikcode/jetbrains/daemon/JeikCodeDaemonClient.kt @@ -231,12 +231,14 @@ class JeikCodeDaemonClient( fun sendPermissionDecision( sessionId: String, + approvalId: String, decision: String, toolName: String? = null, ): CompletableFuture { val body = buildString { append("{") append("\"session_id\":${sessionId.jsonQuoted()},") + append("\"approval_id\":${approvalId.jsonQuoted()},") append("\"decision\":${decision.jsonQuoted()}") toolName?.takeIf { it.isNotBlank() }?.let { append(",\"tool_name\":${it.jsonQuoted()}") diff --git a/extensions/jetbrains/src/main/kotlin/com/jeikcode/jetbrains/daemon/JeikCodeDaemonTypes.kt b/extensions/jetbrains/src/main/kotlin/com/jeikcode/jetbrains/daemon/JeikCodeDaemonTypes.kt index 89c0f2550..27f0fe72f 100644 --- a/extensions/jetbrains/src/main/kotlin/com/jeikcode/jetbrains/daemon/JeikCodeDaemonTypes.kt +++ b/extensions/jetbrains/src/main/kotlin/com/jeikcode/jetbrains/daemon/JeikCodeDaemonTypes.kt @@ -237,6 +237,7 @@ sealed interface ChatEvent { data class ArtifactEnd(val id: String) : ChatEvent data class PermissionRequest( val sessionId: String, + val approvalId: String, val toolName: String, val reason: String, val callId: String, diff --git a/extensions/jetbrains/src/main/kotlin/com/jeikcode/jetbrains/daemon/SseParser.kt b/extensions/jetbrains/src/main/kotlin/com/jeikcode/jetbrains/daemon/SseParser.kt index 11ea205b2..cb511f85b 100644 --- a/extensions/jetbrains/src/main/kotlin/com/jeikcode/jetbrains/daemon/SseParser.kt +++ b/extensions/jetbrains/src/main/kotlin/com/jeikcode/jetbrains/daemon/SseParser.kt @@ -79,6 +79,7 @@ class SseParser { "artifact_end" -> ChatEvent.ArtifactEnd(json.string("id").orEmpty()) "permission_request" -> ChatEvent.PermissionRequest( json.string("session_id").orEmpty(), + json.string("approval_id").orEmpty(), json.string("tool_name").orEmpty(), json.string("reason").orEmpty(), json.string("call_id").orEmpty(), diff --git a/extensions/jetbrains/src/main/kotlin/com/jeikcode/jetbrains/services/JeikCodeProjectService.kt b/extensions/jetbrains/src/main/kotlin/com/jeikcode/jetbrains/services/JeikCodeProjectService.kt index a174a5bbf..96aadf031 100644 --- a/extensions/jetbrains/src/main/kotlin/com/jeikcode/jetbrains/services/JeikCodeProjectService.kt +++ b/extensions/jetbrains/src/main/kotlin/com/jeikcode/jetbrains/services/JeikCodeProjectService.kt @@ -352,11 +352,12 @@ class JeikCodeProjectService(private val project: Project) : Disposable { fun respondToPermission( sessionId: String, + approvalId: String, decision: String, toolName: String? = null, ): CompletableFuture { val client = getOrCreateClient() - return client.sendPermissionDecision(sessionId, decision, toolName).thenApply { + return client.sendPermissionDecision(sessionId, approvalId, decision, toolName).thenApply { if (!it.success && !it.error.isNullOrBlank()) { throw IllegalStateException(it.error) } diff --git a/extensions/jetbrains/src/main/kotlin/com/jeikcode/jetbrains/ui/JeikCodeChatPanel.kt b/extensions/jetbrains/src/main/kotlin/com/jeikcode/jetbrains/ui/JeikCodeChatPanel.kt index 512ff96c1..a8a02de17 100644 --- a/extensions/jetbrains/src/main/kotlin/com/jeikcode/jetbrains/ui/JeikCodeChatPanel.kt +++ b/extensions/jetbrains/src/main/kotlin/com/jeikcode/jetbrains/ui/JeikCodeChatPanel.kt @@ -1190,7 +1190,7 @@ class JeikCodeChatPanel( val isDestructive = event.toolName in setOf("bash", "execute_command", "write_to_file", "replace_in_file", "delete_files") if (!isDestructive) { addSystemMessage("[Permission] auto-allowed: ${event.toolName}") - service.respondToPermission(event.sessionId, "allow", event.toolName) + service.respondToPermission(event.sessionId, event.approvalId, "allow", event.toolName) return } @@ -1210,7 +1210,7 @@ class JeikCodeChatPanel( ) val decision = when (choice) { 0 -> "allow"; 2 -> "allow_persist"; else -> "deny" } addSystemMessage("[Permission] $decision") - service.respondToPermission(event.sessionId, decision, event.toolName).whenComplete { ok, error -> + service.respondToPermission(event.sessionId, event.approvalId, decision, event.toolName).whenComplete { ok, error -> SwingUtilities.invokeLater { if (error != null) addErrorMessage("Permission error: ${error.cause?.message ?: error.message ?: "failed"}") else if (ok != true) addErrorMessage("no pending permission for this session") diff --git a/extensions/jetbrains/src/test/kotlin/com/jeikcode/jetbrains/daemon/SseParserTest.kt b/extensions/jetbrains/src/test/kotlin/com/jeikcode/jetbrains/daemon/SseParserTest.kt index e7ea40f74..f939c8167 100644 --- a/extensions/jetbrains/src/test/kotlin/com/jeikcode/jetbrains/daemon/SseParserTest.kt +++ b/extensions/jetbrains/src/test/kotlin/com/jeikcode/jetbrains/daemon/SseParserTest.kt @@ -62,12 +62,13 @@ class SseParserTest { fun parsesPermissionRequest() { val parser = SseParser() val events = parser.feed( - """data: {"type":"permission_request","session_id":"s1","tool_name":"mcp__repo__edit","reason":"Modify file","call_id":"c1","arguments":"{\"path\":\"README.md\"}"}${"\n\n"}""", + """data: {"type":"permission_request","session_id":"s1","approval_id":"approval-1","tool_name":"mcp__repo__edit","reason":"Modify file","call_id":"c1","arguments":"{\"path\":\"README.md\"}"}${"\n\n"}""", ) assertEquals( ChatEvent.PermissionRequest( sessionId = "s1", + approvalId = "approval-1", toolName = "mcp__repo__edit", reason = "Modify file", callId = "c1", diff --git a/extensions/vscode/src/chat/provider.ts b/extensions/vscode/src/chat/provider.ts index dc272ad0d..7afff157c 100644 --- a/extensions/vscode/src/chat/provider.ts +++ b/extensions/vscode/src/chat/provider.ts @@ -1300,6 +1300,7 @@ export class ChatViewProvider implements vscode.WebviewViewProvider { const msg = { sessionId: request.sessionId, id: request.callId, + approvalId: request.approvalId, toolName: request.toolName, reason: request.reason, args: request.args, @@ -1562,12 +1563,13 @@ export class ChatViewProvider implements vscode.WebviewViewProvider { private async _handlePermissionResponse(msg: { sessionId?: string; id?: string; + approvalId?: string; toolName?: string; decision?: unknown; allowed?: boolean; persist?: boolean; }) { - if (!msg.sessionId || !msg.id) return; + if (!msg.sessionId || !msg.id || !msg.approvalId) return; const decision: PermissionDecision | undefined = isPermissionDecision(msg.decision) ? msg.decision : typeof msg.allowed === 'boolean' @@ -1578,6 +1580,7 @@ export class ChatViewProvider implements vscode.WebviewViewProvider { this._postMessageForSession(msg.sessionId, { type: 'permissionResponseResult', id: msg.id, + approvalId: msg.approvalId, success: false, message: 'Invalid permission decision', }); @@ -1587,12 +1590,14 @@ export class ChatViewProvider implements vscode.WebviewViewProvider { try { const result = await this._client.sendPermissionDecision( msg.sessionId, + msg.approvalId, decision, msg.toolName, ); this._postMessageForSession(msg.sessionId, { type: 'permissionResponseResult', id: msg.id, + approvalId: msg.approvalId, success: result.success, message: result.error, }); @@ -1600,6 +1605,7 @@ export class ChatViewProvider implements vscode.WebviewViewProvider { this._postMessageForSession(msg.sessionId, { type: 'permissionResponseResult', id: msg.id, + approvalId: msg.approvalId, success: false, message: this._messageFromError(e), }); diff --git a/extensions/vscode/src/daemon/client.ts b/extensions/vscode/src/daemon/client.ts index de906d36d..1d02bfde3 100644 --- a/extensions/vscode/src/daemon/client.ts +++ b/extensions/vscode/src/daemon/client.ts @@ -247,11 +247,13 @@ export class DaemonClient { sendPermissionDecision( sessionId: string, + approvalId: string, decision: PermissionDecision, toolName?: string, ): Promise { return this.post('/chat/permission', { session_id: sessionId, + approval_id: approvalId, decision, ...(toolName ? { tool_name: toolName } : {}), }); @@ -560,6 +562,7 @@ export class DaemonClient { case 'permission_request': callbacks.onPermissionRequest({ sessionId: event.session_id, + approvalId: event.approval_id, toolName: event.tool_name, reason: event.reason, callId: event.call_id, diff --git a/extensions/vscode/src/daemon/types.ts b/extensions/vscode/src/daemon/types.ts index e36caef27..04d0bce41 100644 --- a/extensions/vscode/src/daemon/types.ts +++ b/extensions/vscode/src/daemon/types.ts @@ -50,7 +50,7 @@ export type ChatEvent = | { type: 'artifact_start'; id: string; artifact_type: string; language?: string; title?: string } | { type: 'artifact_content'; id: string; content: string } | { type: 'artifact_end'; id: string } - | { type: 'permission_request'; session_id: string; tool_name: string; reason: string; call_id: string; arguments: string } + | { type: 'permission_request'; session_id: string; approval_id: string; tool_name: string; reason: string; call_id: string; arguments: string } | { type: 'warning'; message: string } | { type: 'persistence_warning'; message: string } | { type: 'rate_limited'; message: string; retry_after_seconds?: number; attempt?: number; max_attempts?: number } @@ -322,7 +322,7 @@ export interface ChatStreamCallbacks { onToolStart: (id: string | undefined, name: string, args: string) => void; onToolProgress: (id: string, progress: string) => void; onToolResult: (id: string | undefined, name: string, output: string, success: boolean, durationMs: number) => void; - onPermissionRequest: (request: { sessionId: string; toolName: string; reason: string; callId: string; args: string }) => void; + onPermissionRequest: (request: { sessionId: string; approvalId: string; toolName: string; reason: string; callId: string; args: string }) => void; onTokens: (prompt: number, completion: number, total: number) => void; onArtifactStart: (id: string, type: string, language?: string, title?: string) => void; onArtifactContent: (id: string, content: string) => void; diff --git a/extensions/vscode/webview-ui/src/components/PermissionRequest.tsx b/extensions/vscode/webview-ui/src/components/PermissionRequest.tsx index 5f7c72d66..313ce66b9 100644 --- a/extensions/vscode/webview-ui/src/components/PermissionRequest.tsx +++ b/extensions/vscode/webview-ui/src/components/PermissionRequest.tsx @@ -14,15 +14,21 @@ export function PermissionRequest({ request }: PermissionRequestProps) { const t = useT(); const handleRespond = useCallback((decision: PermissionDecision) => { - dispatch({ type: 'PERMISSION_RESPOND', id: request.id, decision }); + dispatch({ + type: 'PERMISSION_RESPOND', + id: request.id, + approvalId: request.approvalId, + decision, + }); postMessage({ type: 'permissionResponse', sessionId: request.sessionId, id: request.id, + approvalId: request.approvalId, toolName: request.toolName, decision, }); - }, [request.id, request.sessionId, request.toolName, dispatch]); + }, [request.id, request.approvalId, request.sessionId, request.toolName, dispatch]); if (request.status === 'allowed' || request.status === 'denied') return null; diff --git a/extensions/vscode/webview-ui/src/state/ChatProvider.tsx b/extensions/vscode/webview-ui/src/state/ChatProvider.tsx index f52568e8f..41f28508a 100644 --- a/extensions/vscode/webview-ui/src/state/ChatProvider.tsx +++ b/extensions/vscode/webview-ui/src/state/ChatProvider.tsx @@ -1,6 +1,7 @@ import React, { createContext, useContext, useReducer, useEffect, useCallback, useRef } from 'react'; import { ChatState, ChatAction, ExtensionMessage, ImageData, ApprovalMode } from './types'; import { chatReducer, initialState } from './reducer'; +import { permissionRequestAction } from './permissionBridge'; import { postMessage, getVSCodeApi } from '../vscode'; import { createTranslator } from '../i18n'; import { shouldShowIdleNotice } from '../utils/streamStatus'; @@ -283,20 +284,13 @@ export function ChatProvider({ children }: { children: React.ReactNode }) { break; case 'permissionRequest': markStreamActivity(); - dispatch({ - type: 'PERMISSION_REQUEST', - id: msg.id, - sessionId: msg.sessionId, - toolName: msg.toolName, - reason: msg.reason, - args: msg.args, - isDestructive: msg.isDestructive, - }); + dispatch(permissionRequestAction(msg)); break; case 'permissionResponseResult': dispatch({ type: 'PERMISSION_RESPONSE_RESULT', id: msg.id, + approvalId: msg.approvalId, success: msg.success, message: msg.message, }); diff --git a/extensions/vscode/webview-ui/src/state/permissionBridge.ts b/extensions/vscode/webview-ui/src/state/permissionBridge.ts new file mode 100644 index 000000000..b36113d09 --- /dev/null +++ b/extensions/vscode/webview-ui/src/state/permissionBridge.ts @@ -0,0 +1,17 @@ +import type { ChatAction, ExtensionMessage } from './types'; + +type PermissionRequestMessage = Extract; +type PermissionRequestAction = Extract; + +export function permissionRequestAction(msg: PermissionRequestMessage): PermissionRequestAction { + return { + type: 'PERMISSION_REQUEST', + id: msg.id, + approvalId: msg.approvalId, + sessionId: msg.sessionId, + toolName: msg.toolName, + reason: msg.reason, + args: msg.args, + isDestructive: msg.isDestructive, + }; +} diff --git a/extensions/vscode/webview-ui/src/state/reducer.ts b/extensions/vscode/webview-ui/src/state/reducer.ts index bc8d67141..1a2b961c9 100644 --- a/extensions/vscode/webview-ui/src/state/reducer.ts +++ b/extensions/vscode/webview-ui/src/state/reducer.ts @@ -265,10 +265,18 @@ function completeArtifactBlock(message: ChatMessage, id: string): ChatMessage { function upsertPermissionBlock(message: ChatMessage, request: PermissionRequestData): ChatMessage { const blocks = currentBlocks(message); - const existing = blocks.findIndex((block) => block.type === 'permission' && block.request.id === request.id); + const existing = blocks.findIndex((block) => + block.type === 'permission' + && block.request.id === request.id + && block.request.approvalId === request.approvalId + ); const nextBlocks = existing >= 0 ? blocks.map((block, index) => index === existing && block.type === 'permission' ? { ...block, request } : block) - : [...blocks, { id: `${message.id}-permission-${request.id}`, type: 'permission' as const, request }]; + : [...blocks, { + id: `${message.id}-permission-${request.id}-${request.approvalId}`, + type: 'permission' as const, + request, + }]; return { ...message, blocks: nextBlocks }; } @@ -372,12 +380,17 @@ function mergeTerminalIntoHistory( function updatePermissionBlock( message: ChatMessage, id: string, + approvalId: string, update: (request: PermissionRequestData) => PermissionRequestData, ): ChatMessage { const blocks = currentBlocks(message); let updatedRequest: PermissionRequestData | undefined; const nextBlocks = blocks.map((block) => { - if (block.type !== 'permission' || block.request.id !== id) return block; + if ( + block.type !== 'permission' + || block.request.id !== id + || block.request.approvalId !== approvalId + ) return block; updatedRequest = update(block.request); return { ...block, request: updatedRequest }; }); @@ -386,6 +399,7 @@ function updatePermissionBlock( ...message, blocks: nextBlocks, permissionRequest: message.permissionRequest?.id === id + && message.permissionRequest?.approvalId === approvalId ? updatedRequest : message.permissionRequest, }; @@ -1175,6 +1189,7 @@ function chatReducerInner(state: ChatState, action: ChatAction): ChatState { ); const request: PermissionRequestData = { id: action.id, + approvalId: action.approvalId, sessionId: action.sessionId, toolName: action.toolName, reason: action.reason, @@ -1199,7 +1214,7 @@ function chatReducerInner(state: ChatState, action: ChatAction): ChatState { const msgs = [...state.messages]; const last = msgs[msgs.length - 1]; if (last?.role === 'assistant') { - msgs[msgs.length - 1] = updatePermissionBlock(last, action.id, (request) => ({ + msgs[msgs.length - 1] = updatePermissionBlock(last, action.id, action.approvalId, (request) => ({ ...request, status: 'submitting', decision: action.decision, @@ -1213,7 +1228,7 @@ function chatReducerInner(state: ChatState, action: ChatAction): ChatState { const msgs = [...state.messages]; const last = msgs[msgs.length - 1]; if (last?.role === 'assistant') { - msgs[msgs.length - 1] = updatePermissionBlock(last, action.id, (request) => action.success + msgs[msgs.length - 1] = updatePermissionBlock(last, action.id, action.approvalId, (request) => action.success ? { ...request, status: request.decision === 'deny' ? 'denied' : 'allowed', diff --git a/extensions/vscode/webview-ui/src/state/types.ts b/extensions/vscode/webview-ui/src/state/types.ts index 20c999709..348d1c793 100644 --- a/extensions/vscode/webview-ui/src/state/types.ts +++ b/extensions/vscode/webview-ui/src/state/types.ts @@ -112,6 +112,7 @@ export interface ArtifactData { export interface PermissionRequestData { id: string; + approvalId: string; sessionId: string; toolName: string; reason: string; @@ -260,9 +261,9 @@ export type ChatAction = | { type: 'CLEAR_CONTEXT' } | { type: 'TOGGLE_HISTORY' } | { type: 'TOGGLE_SETTINGS' } - | { type: 'PERMISSION_REQUEST'; id: string; sessionId: string; toolName: string; reason: string; args: string; isDestructive: boolean } - | { type: 'PERMISSION_RESPOND'; id: string; decision: PermissionDecision } - | { type: 'PERMISSION_RESPONSE_RESULT'; id: string; success: boolean; message?: string } + | { type: 'PERMISSION_REQUEST'; id: string; approvalId: string; sessionId: string; toolName: string; reason: string; args: string; isDestructive: boolean } + | { type: 'PERMISSION_RESPOND'; id: string; approvalId: string; decision: PermissionDecision } + | { type: 'PERMISSION_RESPONSE_RESULT'; id: string; approvalId: string; success: boolean; message?: string } | { type: 'SET_SEARCH_QUERY'; query: string } | { type: 'TOGGLE_SEARCH' } | { type: 'SEARCH_NEXT' } @@ -316,8 +317,8 @@ export type ExtensionMessage = | { type: 'context'; filePath: string; fileName: string; selection?: string; language?: string; startLine?: number; endLine?: number } | { type: 'insertText'; text: string } | { type: 'skills'; skills: SkillInfo[] } - | { type: 'permissionRequest'; sessionId: string; id: string; toolName: string; reason: string; args: string; isDestructive: boolean } - | { type: 'permissionResponseResult'; id: string; success: boolean; message?: string } + | { type: 'permissionRequest'; sessionId: string; id: string; approvalId: string; toolName: string; reason: string; args: string; isDestructive: boolean } + | { type: 'permissionResponseResult'; id: string; approvalId: string; success: boolean; message?: string } | { type: 'resumeStreaming' } | { type: 'setDraft'; text: string } | { type: 'chromeFont'; value: string | null }; diff --git a/extensions/vscode/webview-ui/test/daemon-client-error.test.ts b/extensions/vscode/webview-ui/test/daemon-client-error.test.ts index 0f1b48ab1..05febb810 100644 --- a/extensions/vscode/webview-ui/test/daemon-client-error.test.ts +++ b/extensions/vscode/webview-ui/test/daemon-client-error.test.ts @@ -57,6 +57,7 @@ function testPermissionRequestSseIsForwardedToCallback() { .handleSSEData(JSON.stringify({ type: 'permission_request', session_id: 'session-1', + approval_id: 'approval-1', tool_name: 'write_file', reason: 'Modify workspace file', call_id: 'call-1', @@ -65,6 +66,7 @@ function testPermissionRequestSseIsForwardedToCallback() { assert.deepEqual(received, { sessionId: 'session-1', + approvalId: 'approval-1', toolName: 'write_file', reason: 'Modify workspace file', callId: 'call-1', diff --git a/extensions/vscode/webview-ui/test/provider-queue-regression.test.ts b/extensions/vscode/webview-ui/test/provider-queue-regression.test.ts index 58ba50e52..6e10d938d 100644 --- a/extensions/vscode/webview-ui/test/provider-queue-regression.test.ts +++ b/extensions/vscode/webview-ui/test/provider-queue-regression.test.ts @@ -1940,6 +1940,7 @@ async function testPermissionRequestFromStreamIsForwardedToPanel() { streamChat: (_request: unknown, callbacks: { onPermissionRequest: (request: { sessionId: string; + approvalId: string; toolName: string; reason: string; callId: string; @@ -1948,6 +1949,7 @@ async function testPermissionRequestFromStreamIsForwardedToPanel() { }) => { callbacks.onPermissionRequest({ sessionId: 'session-a', + approvalId: 'approval-1', toolName: 'write_file', reason: 'Modify workspace file', callId: 'call-1', @@ -1974,6 +1976,7 @@ async function testPermissionRequestFromStreamIsForwardedToPanel() { type: 'permissionRequest', sessionId: 'session-a', id: 'call-1', + approvalId: 'approval-1', toolName: 'write_file', reason: 'Modify workspace file', args: '{"path":"README.md"}', @@ -2002,14 +2005,16 @@ async function testPermissionResponsePostsDecisionToDaemon() { await unsafeProvider._handlePermissionResponse({ sessionId: 'session-a', id: 'call-1', + approvalId: 'approval-1', toolName: 'write_file', allowed: true, }); - assert.deepEqual(calls, [['session-a', 'allow', 'write_file']]); + assert.deepEqual(calls, [['session-a', 'approval-1', 'allow', 'write_file']]); assert.deepEqual(posted, [{ type: 'permissionResponseResult', id: 'call-1', + approvalId: 'approval-1', success: true, message: undefined, }]); @@ -2036,28 +2041,32 @@ async function testPermissionResponsePostsExplicitDecisionToDaemon() { await unsafeProvider._handlePermissionResponse({ sessionId: 'session-a', id: 'call-1', + approvalId: 'approval-1', toolName: 'mcp__server__tool', decision: 'allow_persist', }); await unsafeProvider._handlePermissionResponse({ sessionId: 'session-a', id: 'call-2', + approvalId: 'approval-2', toolName: 'write_file', decision: 'always_allow', }); assert.deepEqual(calls, [ - ['session-a', 'allow_persist', 'mcp__server__tool'], - ['session-a', 'always_allow', 'write_file'], + ['session-a', 'approval-1', 'allow_persist', 'mcp__server__tool'], + ['session-a', 'approval-2', 'always_allow', 'write_file'], ]); assert.deepEqual(posted, [{ type: 'permissionResponseResult', id: 'call-1', + approvalId: 'approval-1', success: true, message: undefined, }, { type: 'permissionResponseResult', id: 'call-2', + approvalId: 'approval-2', success: true, message: undefined, }]); diff --git a/extensions/vscode/webview-ui/test/rendering-regression.test.ts b/extensions/vscode/webview-ui/test/rendering-regression.test.ts index d75ee1ebb..5711ace79 100644 --- a/extensions/vscode/webview-ui/test/rendering-regression.test.ts +++ b/extensions/vscode/webview-ui/test/rendering-regression.test.ts @@ -18,6 +18,7 @@ import { } from '../src/components/vscodeFileLinks'; import { formatToolDuration } from '../src/utils/format'; import { shouldShowIdleNotice } from '../src/utils/streamStatus'; +import { permissionRequestAction } from '../src/state/permissionBridge'; declare const require: { (id: string): typeof import('../src/state/reducer'); @@ -395,6 +396,7 @@ function testPermissionRequestMarksMatchingToolWaitingAndAddsPermissionBlock() { type: 'PERMISSION_REQUEST', id: 'call-1', sessionId: 'session-1', + approvalId: 'approval-1', toolName: 'write_file', reason: 'Modify workspace file', args: '{"path":"README.md"}', @@ -406,9 +408,34 @@ function testPermissionRequestMarksMatchingToolWaitingAndAddsPermissionBlock() { assert.equal(message.toolCalls?.[0]?.status, 'waiting_approval'); assert.equal(message.permissionRequest?.id, 'call-1'); assert.equal(message.permissionRequest?.sessionId, 'session-1'); + assert.equal(message.permissionRequest?.approvalId, 'approval-1'); assert.equal(message.permissionRequest?.reason, 'Modify workspace file'); } +function testPermissionRequestBridgePreservesApprovalIdentity() { + const action = permissionRequestAction({ + type: 'permissionRequest', + sessionId: 'session-1', + id: 'call-reused', + approvalId: 'approval-new', + toolName: 'write_file', + reason: 'Modify workspace file', + args: '{"path":"README.md"}', + isDestructive: true, + }); + assert.equal(action.approvalId, 'approval-new'); + + let state = startAssistantState(); + state = chatReducer(state, { + type: 'TOOL_START', + id: 'call-reused', + name: 'write_file', + args: '{"path":"README.md"}', + }); + state = chatReducer(state, action); + assert.equal(state.messages[0].permissionRequest?.approvalId, 'approval-new'); +} + function testConsecutivePermissionResponsesUpdateOriginalBlockOnly() { let state = startAssistantState(); state = chatReducer(state, { @@ -421,12 +448,18 @@ function testConsecutivePermissionResponsesUpdateOriginalBlockOnly() { type: 'PERMISSION_REQUEST', id: 'call-1', sessionId: 'session-1', + approvalId: 'approval-1', toolName: 'write_file', reason: 'Modify first file', args: '{"path":"README.md"}', isDestructive: true, }); - state = chatReducer(state, { type: 'PERMISSION_RESPOND', id: 'call-1', decision: 'allow' }); + state = chatReducer(state, { + type: 'PERMISSION_RESPOND', + id: 'call-1', + approvalId: 'approval-1', + decision: 'allow', + }); state = chatReducer(state, { type: 'TOOL_START', id: 'call-2', @@ -437,12 +470,18 @@ function testConsecutivePermissionResponsesUpdateOriginalBlockOnly() { type: 'PERMISSION_REQUEST', id: 'call-2', sessionId: 'session-1', + approvalId: 'approval-2', toolName: 'write_file', reason: 'Modify second file', args: '{"path":"CHANGELOG.md"}', isDestructive: true, }); - state = chatReducer(state, { type: 'PERMISSION_RESPONSE_RESULT', id: 'call-1', success: true }); + state = chatReducer(state, { + type: 'PERMISSION_RESPONSE_RESULT', + id: 'call-1', + approvalId: 'approval-1', + success: true, + }); const message = state.messages[0]; const permissionBlocks = message.blocks?.filter((block) => block.type === 'permission') ?? []; @@ -451,6 +490,57 @@ function testConsecutivePermissionResponsesUpdateOriginalBlockOnly() { [['call-1', 'allowed'], ['call-2', 'pending']], ); assert.equal(message.permissionRequest?.id, 'call-2'); + assert.equal(message.permissionRequest?.approvalId, 'approval-2'); + assert.equal(message.permissionRequest?.status, 'pending'); +} + +function testLatePermissionResultCannotOverwriteReusedCallIdApproval() { + let state = startAssistantState(); + state = chatReducer(state, { + type: 'PERMISSION_REQUEST', + id: 'call-reused', + sessionId: 'session-1', + approvalId: 'approval-a', + toolName: 'write_file', + reason: 'First approval', + args: '{"path":"A.md"}', + isDestructive: true, + }); + state = chatReducer(state, { + type: 'PERMISSION_RESPOND', + id: 'call-reused', + approvalId: 'approval-a', + decision: 'allow', + }); + + state = chatReducer(state, { + type: 'PERMISSION_REQUEST', + id: 'call-reused', + sessionId: 'session-1', + approvalId: 'approval-b', + toolName: 'write_file', + reason: 'Second approval', + args: '{"path":"B.md"}', + isDestructive: true, + }); + state = chatReducer(state, { + type: 'PERMISSION_RESPONSE_RESULT', + id: 'call-reused', + approvalId: 'approval-a', + success: true, + }); + + const message = state.messages[0]; + const permissionBlocks = message.blocks?.filter((block) => block.type === 'permission') ?? []; + assert.deepEqual( + permissionBlocks.map((block) => + block.type === 'permission' + ? [block.request.approvalId, block.request.status] + : undefined + ), + [['approval-a', 'allowed'], ['approval-b', 'pending']], + ); + assert.equal(message.permissionRequest?.approvalId, 'approval-b'); assert.equal(message.permissionRequest?.status, 'pending'); } @@ -460,12 +550,18 @@ function testPermissionRespondStoresExplicitDecision() { type: 'PERMISSION_REQUEST', id: 'call-1', sessionId: 'session-1', + approvalId: 'approval-1', toolName: 'mcp__server__tool', reason: 'Run MCP tool', args: '{}', isDestructive: false, }); - state = chatReducer(state, { type: 'PERMISSION_RESPOND', id: 'call-1', decision: 'allow_persist' }); + state = chatReducer(state, { + type: 'PERMISSION_RESPOND', + id: 'call-1', + approvalId: 'approval-1', + decision: 'allow_persist', + }); const request = state.messages[0].permissionRequest; assert.equal(request?.status, 'submitting'); @@ -1175,7 +1271,9 @@ testTypedCodeArtifactDoesNotStripDifferentLanguageLookingCodeLine(); testPlainCodeFenceArtifactDoesNotRenderArtifactChrome(); testToolBlocksStayBetweenTextChunks(); testPermissionRequestMarksMatchingToolWaitingAndAddsPermissionBlock(); +testPermissionRequestBridgePreservesApprovalIdentity(); testConsecutivePermissionResponsesUpdateOriginalBlockOnly(); +testLatePermissionResultCannotOverwriteReusedCallIdApproval(); testPermissionRespondStoresExplicitDecision(); testHistoryAttachedSelectionMessageDisplaysOnlyUserQuestion(); testHistoryMissingImagePlaceholderIsPreserved(); diff --git a/webui/src/api.test.ts b/webui/src/api.test.ts index 4eb53bf6d..c1a1e7b03 100644 --- a/webui/src/api.test.ts +++ b/webui/src/api.test.ts @@ -135,6 +135,61 @@ test('postLiveUserInput rejects an answer the runtime did not accept', async () } }); +test('postLivePermission carries exact session and native request identity', async () => { + const calls: Array<{ url: string; init?: RequestInit }> = []; + const originalFetch = globalThis.fetch; + globalThis.fetch = (async (url: RequestInfo | URL, init?: RequestInit) => { + calls.push({ url: String(url), init }); + return new Response('{"accepted":true}', { status: 200 }); + }) as typeof fetch; + + try { + const { postLivePermission } = await import('./api.ts'); + await postLivePermission('allow', 'mcp__srv__query', 'session-1', 5, 73); + + assert.equal(calls[0].url, '/live/permission'); + assert.deepEqual(JSON.parse(String(calls[0].init?.body)), { + decision: 'allow', + tool_name: 'mcp__srv__query', + session_id: 'session-1', + generation: 5, + request_id: 73, + }); + await assert.rejects( + () => postLivePermission('allow', 'mcp__srv__query', null, 5, 73), + /missing live approval identity/i, + ); + await assert.rejects( + () => postLivePermission('allow', 'mcp__srv__query', 'session-1'), + /missing live approval identity/i, + ); + await assert.rejects( + () => postLivePermission('allow', 'mcp__srv__query', 'session-1', undefined, 73), + /missing live approval identity/i, + ); + } finally { + globalThis.fetch = originalFetch; + } +}); + +test('postLivePermission rejects when the runtime did not consume the approval', async () => { + const originalFetch = globalThis.fetch; + globalThis.fetch = (async () => new Response( + JSON.stringify({ accepted: false }), + { status: 200, headers: { 'Content-Type': 'application/json' } }, + )) as typeof fetch; + + try { + const { postLivePermission } = await import('./api.ts'); + await assert.rejects( + () => postLivePermission('allow', 'write_file', 'session-1', 5, 73), + /did not accept permission/i, + ); + } finally { + globalThis.fetch = originalFetch; + } +}); + test('postChatUserInput correlates the answer by session and native request id', async () => { const calls: Array<{ url: string; init?: RequestInit }> = []; const originalFetch = globalThis.fetch; diff --git a/webui/src/api.ts b/webui/src/api.ts index 591a62577..6c31e2709 100644 --- a/webui/src/api.ts +++ b/webui/src/api.ts @@ -163,7 +163,7 @@ export type SSEEvent = | { type: 'tool_progress'; id: string; progress: string } | { type: 'tool_result'; id: string; name: string; output: string; success: boolean; duration_ms: number } | { type: 'tokens'; prompt: number; completion: number; total: number; cached?: number; cached_estimated?: boolean; reasoning?: number } - | { type: 'permission_request'; session_id: string; tool_name: string; reason: string; call_id: string; arguments: unknown } + | { type: 'permission_request'; session_id: string; approval_id: string; tool_name: string; reason: string; call_id: string; arguments: unknown } | UserInputRequestEvent | { type: 'user_input_resolved'; request_id: number } | { type: 'steered'; count: number; inputs: { text: string; images?: ImageData[] }[] } @@ -336,6 +336,7 @@ export async function postSystemNotify(input: { body: string; tag?: string; sessionId?: string; + approvalId?: string; }): Promise { const resp = await apiFetch('/system-notify', { method: 'POST', @@ -345,6 +346,7 @@ export async function postSystemNotify(input: { body: input.body, tag: input.tag, session_id: input.sessionId, + approval_id: input.approvalId, }), }); if (!resp.ok && resp.status !== 400) { @@ -370,6 +372,7 @@ export interface ChatPendingInteractive { permission: { type: 'permission_request'; session_id: string; + approval_id: string; tool_name: string; reason: string; call_id: string; @@ -584,6 +587,7 @@ export async function streamChat( export async function respondPermission( sessionId: string, + approvalId: string, decision: 'allow' | 'deny' | 'always_allow' | 'allow_persist', toolName?: string, ): Promise<{ success: boolean }> { @@ -593,7 +597,7 @@ export async function respondPermission( 'Content-Type': 'application/json', ...authHeaders(), }, - body: JSON.stringify({ session_id: sessionId, decision, tool_name: toolName }), + body: JSON.stringify({ session_id: sessionId, approval_id: approvalId, decision, tool_name: toolName }), }); return resp.json(); } @@ -1413,7 +1417,7 @@ export type LiveWireEvent = | { type: 'warning'; message: string } | { type: 'persistence_warning'; message: string } | { type: 'rate_limited'; reset_at_display: string; reset_label: string; secs_until_reset: number | null; auto_resuming: boolean; server_message?: string | null } - | { type: 'permission_request'; session_id?: string; tool_name: string; reason: string; call_id: string; arguments: string } + | { type: 'permission_request'; session_id?: string; generation: number; request_id: number; tool_name: string; reason: string; call_id: string; arguments: string } | { type: 'user_input_request'; session_id?: string; request_id: number; header: string; question: string; mode: 'single' | 'multiple' | 'text'; options: { label: string; description?: string }[] } | { type: 'user_input_resolved'; request_id: number } | { type: 'steered'; count: number; inputs: { text: string; images: ImageData[] }[]; client_input_ids: Array } @@ -1669,18 +1673,27 @@ export async function postLivePermission( decision: 'allow' | 'deny' | 'always_allow' | 'allow_persist', toolName?: string, sessionId?: string | null, + generation?: number, + requestId?: number, ): Promise<{ accepted: boolean }> { + if (!sessionId || generation === undefined || requestId === undefined) { + throw new Error('missing live approval identity'); + } const resp = await apiFetch('/live/permission', { method: 'POST', headers: { 'Content-Type': 'application/json', ...authHeaders() }, body: JSON.stringify({ decision, tool_name: toolName, - ...(sessionId ? { session_id: sessionId } : {}), + session_id: sessionId, + generation, + request_id: requestId, }), }); if (!resp.ok) throw new Error(`answer live permission failed: ${resp.status}`); - return resp.json(); + const body = await resp.json() as { accepted?: boolean }; + if (!body.accepted) throw new Error('live runtime did not accept permission'); + return { accepted: true }; } export interface UserInputQuestion { diff --git a/webui/src/app.tsx b/webui/src/app.tsx index b5cb69679..fcfbe2c60 100644 --- a/webui/src/app.tsx +++ b/webui/src/app.tsx @@ -65,7 +65,7 @@ export function App() { const [pending, setPending] = useState(null); const [liveReview, setLiveReview] = useState(null); const dismissLiveReview = useRef({ - permission: (_callId: string) => {}, + permission: (_generation: number, _requestId: number, _callId: string) => {}, userInput: () => {}, }); const onLiveReview = useCallback((review: LiveReviewState) => { @@ -73,7 +73,8 @@ export function App() { if ( prev && prev.sessionId === review.sessionId - && (prev.permission?.call_id ?? '') === (review.permission?.call_id ?? '') + && (prev.permission?.generation ?? -1) === (review.permission?.generation ?? -1) + && (prev.permission?.request_id ?? -1) === (review.permission?.request_id ?? -1) && (prev.userInput?.request_id ?? -1) === (review.userInput?.request_id ?? -1) ) { return prev; @@ -82,7 +83,7 @@ export function App() { }); }, []); const onBindReviewDismiss = useCallback((fns: { - permission: (callId: string) => void; + permission: (generation: number, requestId: number, callId: string) => void; userInput: () => void; }) => { dismissLiveReview.current = fns; @@ -1362,7 +1363,8 @@ export function App() { chatPermission={pending} activeSession={activeSession} onDismissChatPermission={() => setPending(null)} - onDismissLivePermission={(callId) => dismissLiveReview.current.permission(callId)} + onDismissLivePermission={(generation, requestId, callId) => + dismissLiveReview.current.permission(generation, requestId, callId)} onDismissLiveUserInput={() => dismissLiveReview.current.userInput()} onFocusSession={(id) => { if (!id || id === sessionId) return; diff --git a/webui/src/components/Chat.tsx b/webui/src/components/Chat.tsx index bf042a300..f40557256 100644 --- a/webui/src/components/Chat.tsx +++ b/webui/src/components/Chat.tsx @@ -621,6 +621,7 @@ interface TokenUsage { interface PermissionRequestEvent { type: 'permission_request'; session_id: string; + approval_id: string; tool_name: string; reason: string; call_id: string; @@ -675,12 +676,12 @@ interface ChatProps { /** 当前会话的审批 / 提问，交给右下角通知栈，而不是居中弹层。 */ onLiveReview?: (review: { sessionId: string | null; - permission: { tool_name: string; reason: string; call_id: string; arguments: unknown } | null; + permission: { generation: number; request_id: number; tool_name: string; reason: string; call_id: string; arguments: unknown } | null; userInput: UserInputRequestEvent | null; }) => void; /** 通知栈提交后清掉本会话的实时卡片。 */ onBindReviewDismiss?: (fns: { - permission: (callId: string) => void; + permission: (generation: number, requestId: number, callId: string) => void; userInput: () => void; }) => void; } @@ -1279,13 +1280,14 @@ export function Chat({ const syncRef = useRef(false); // Pending live-session permission request (shown as PermissionCard, calls /live/permission). // Kept separate from the non-sync `onPermission` prop so the /chat path is untouched. - const [livePending, setLivePending] = useState<{ tool_name: string; reason: string; call_id: string; arguments: string } | null>(null); + const [livePending, setLivePending] = useState<{ generation: number; request_id: number; tool_name: string; reason: string; call_id: string; arguments: string } | null>(null); // Pending structured input from either transport. The event's optional session_id // selects `/chat/user-input`; live requests answer the bound `/live` runtime. const [userInputReq, setUserInputReq] = useState(null); useEffect(() => { onBindReviewDismiss?.({ - permission: (callId) => setLivePending((cur) => resolvePendingAfterDecision(cur, callId)), + permission: (generation, requestId, callId) => + setLivePending((cur) => resolvePendingAfterDecision(cur, callId, requestId, generation)), userInput: () => setUserInputReq(null), }); }, [onBindReviewDismiss]); @@ -3384,7 +3386,7 @@ export function Chat({ break; } updateToolInLastAssistant(e.call_id, { status: 'waiting_approval' }); - setLivePending({ tool_name: e.tool_name, reason: e.reason, call_id: e.call_id, arguments: e.arguments }); + setLivePending({ generation: e.generation, request_id: e.request_id, tool_name: e.tool_name, reason: e.reason, call_id: e.call_id, arguments: e.arguments }); const folder = (effectiveWorkingDir ?? '').split(/[\\/]/).filter((part) => part.length > 0).pop() ?? ''; const sessionName = activeSession?.name || folder || 'JeikCode'; const sid = e.session_id || activeIdRef.current; @@ -3392,7 +3394,7 @@ export function Chat({ title: t('notify.review.title'), body: t('notify.review.body', { session: sessionName, detail: e.tool_name }), sessionId: sid, - tag: `${sid}:review:${e.call_id}`, + tag: `${sid}:review:${e.generation}:${e.request_id}`, postSystemNotifyFn: postSystemNotify, }); break; @@ -4842,6 +4844,7 @@ export function Chat({ body: t('notify.review.body', { session: sessionName, detail: event.tool_name }), sessionId: sid, tag: `${sid}:review:${event.call_id}`, + approvalId: event.approval_id, postSystemNotifyFn: postSystemNotify, }); } diff --git a/webui/src/components/NotificationDock.tsx b/webui/src/components/NotificationDock.tsx index 8a828495a..32c44f983 100644 --- a/webui/src/components/NotificationDock.tsx +++ b/webui/src/components/NotificationDock.tsx @@ -34,6 +34,8 @@ import { TerminalKind, TerminalNoticeContext, isSessionNoticeSuppressed, + permissionInstanceKey, + samePermissionInstance, } from '../lib/sessionNotify'; import { useT } from '../settings'; import { PermissionCard } from './PermissionCard'; @@ -42,6 +44,8 @@ import { UserInputCard } from './UserInputCard'; export interface LiveReviewState { sessionId: string | null; permission: { + generation: number; + request_id: number; tool_name: string; reason: string; call_id: string; @@ -52,6 +56,7 @@ export interface LiveReviewState { interface ChatPermission { session_id: string; + approval_id: string; tool_name: string; reason: string; call_id: string; @@ -84,13 +89,6 @@ function windowAway(): boolean { return isWindowAway(); } -function samePermission( - a: { call_id: string } | null | undefined, - b: { call_id: string } | null | undefined, -): boolean { - return !!a && !!b && a.call_id === b.call_id; -} - export function NotificationDock({ liveReview, chatPermission, @@ -104,7 +102,7 @@ export function NotificationDock({ chatPermission: ChatPermission | null; activeSession?: { id: string; name: string; working_dir?: string } | null; onDismissChatPermission: () => void; - onDismissLivePermission: (callId: string) => void; + onDismissLivePermission: (generation: number, requestId: number, callId: string) => void; onDismissLiveUserInput: () => void; onFocusSession: (sessionId: string) => void; }) { @@ -150,12 +148,24 @@ export function NotificationDock({ })); }, [activeSession?.id, activeSession?.name, activeSession?.working_dir]); - function dismissCardLocally(cardKey: string, sessionId: string, callOrReqId: string | number) { + function dismissCardLocally( + cardKey: string, + sessionId: string, + callOrReqId: string | number, + approvalId?: string, + requestId?: number, + generation?: number, + ) { const idStr = String(callOrReqId); + const identityKey = approvalId + ? permissionInstanceKey(sessionId, { call_id: idStr, approval_id: approvalId }) + : requestId !== undefined + ? permissionInstanceKey(sessionId, { call_id: idStr, request_id: requestId, generation }) + : `${sessionId}:${idStr}`; setDismissedKeys((prev) => { const next = new Set(prev); next.add(cardKey); - next.add(`${sessionId}:${idStr}`); + next.add(identityKey); return next; }); setPolled((prev) => @@ -163,7 +173,12 @@ export function NotificationDock({ (p) => !( p.sessionId === sessionId && - (p.permission?.call_id === idStr || + ((p.permission && + (approvalId + ? p.permission.approval_id === approvalId + : requestId !== undefined + ? false + : p.permission.call_id === idStr)) || (p.userInput?.request_id != null && String(p.userInput.request_id) === idStr)) ), ), @@ -217,7 +232,13 @@ export function NotificationDock({ }); } - function pingReview(tag: string, sessionId: string, detail: string, ask: boolean) { + function pingReview( + tag: string, + sessionId: string, + detail: string, + ask: boolean, + approvalId?: string, + ) { if (modeRef.current == null) return; if (sentReview.current.has(tag)) return; if (!shouldOsNotifyReview(modeRef.current, windowAway())) { @@ -231,6 +252,7 @@ export function NotificationDock({ body: tRef.current(ask ? 'notify.ask.body' : 'notify.review.body', { session, detail }), sessionId, tag, + approvalId, postSystemNotifyFn: postSystemNotify, }); } @@ -367,24 +389,28 @@ export function NotificationDock({ const permissionCards: Array<{ key: string; sessionId: string; + approval_id?: string; + generation?: number; + request_id?: number; tool_name: string; reason: string; call_id: string; arguments: unknown; live: boolean; }> = []; - if (livePermission && liveSessionId) { + if (livePermission && liveSessionId && !samePermissionInstance(chatPerm, livePermission)) { permissionCards.push({ - key: `live:${livePermission.call_id}`, + key: `live:${permissionInstanceKey(liveSessionId, livePermission)}`, sessionId: liveSessionId, ...livePermission, live: true, }); } - if (chatPerm && !samePermission(chatPerm, livePermission)) { + if (chatPerm) { permissionCards.push({ - key: `chat:${chatPerm.call_id}`, + key: `chat:${permissionInstanceKey(chatPerm.session_id, chatPerm)}`, sessionId: chatPerm.session_id, + approval_id: chatPerm.approval_id, tool_name: chatPerm.tool_name, reason: chatPerm.reason, call_id: chatPerm.call_id, @@ -395,15 +421,19 @@ export function NotificationDock({ for (const item of polled) { const perm = item.permission; if (!allowPermission || !perm) continue; - if (dismissedKeys.has(`poll:${item.sessionId}:${perm.call_id}`) || dismissedKeys.has(`${item.sessionId}:${perm.call_id}`)) { + const identityKey = permissionInstanceKey(item.sessionId, perm); + if (dismissedKeys.has(`poll:${identityKey}`) || dismissedKeys.has(identityKey)) { continue; } - if (permissionCards.some((card) => card.call_id === perm.call_id && card.sessionId === item.sessionId)) { + if (permissionCards.some((card) => + card.sessionId === item.sessionId && samePermissionInstance(card, perm) + )) { continue; } permissionCards.push({ - key: `poll:${item.sessionId}:${perm.call_id}`, + key: `poll:${identityKey}`, sessionId: item.sessionId, + approval_id: perm.approval_id, tool_name: perm.tool_name, reason: perm.reason, call_id: perm.call_id, @@ -443,7 +473,13 @@ export function NotificationDock({ useEffect(() => { for (const card of permissionCards) { - pingReview(`perm:${card.sessionId}:${card.call_id}`, card.sessionId, card.tool_name, false); + pingReview( + `perm:${permissionInstanceKey(card.sessionId, card)}`, + card.sessionId, + card.tool_name, + false, + card.live ? undefined : card.approval_id, + ); } for (const card of questionCards) { if (!showPermissionNotice(modeRef.current)) continue; @@ -645,6 +681,7 @@ export function NotificationDock({ dock req={{ session_id: currentCard.card.sessionId, + approval_id: currentCard.card.approval_id, tool_name: currentCard.card.tool_name, reason: currentCard.card.reason, call_id: currentCard.card.call_id, @@ -655,29 +692,45 @@ export function NotificationDock({ currentCard.card.key, currentCard.card.sessionId, currentCard.card.call_id, + currentCard.card.approval_id, + currentCard.card.request_id, + currentCard.card.generation, ); - if (currentCard.card.live) onDismissLivePermission(currentCard.card.call_id); - else if (chatPerm && chatPerm.call_id === currentCard.card.call_id) + if ( + currentCard.card.live + && currentCard.card.generation !== undefined + && currentCard.card.request_id !== undefined + ) { + onDismissLivePermission( + currentCard.card.generation, + currentCard.card.request_id, + currentCard.card.call_id, + ); + } + else if ( + chatPerm && + chatPerm.approval_id === currentCard.card.approval_id + ) onDismissChatPermission(); }} onDecide={async (decision, toolName) => { - dismissCardLocally( - currentCard.card.key, - currentCard.card.sessionId, - currentCard.card.call_id, - ); if (currentCard.card.live) { - await postLivePermission(decision, toolName, currentCard.card.sessionId); + await postLivePermission( + decision, + toolName, + currentCard.card.sessionId, + currentCard.card.generation, + currentCard.card.request_id, + ); return; } const result = await respondPermission( currentCard.card.sessionId, + currentCard.card.approval_id ?? '', decision, toolName, ); - if (!result.success) { - await postLivePermission(decision, toolName, currentCard.card.sessionId); - } + if (!result.success) throw new Error('permission request is no longer pending'); }} /> diff --git a/webui/src/components/PermissionCard.tsx b/webui/src/components/PermissionCard.tsx index c4eb76864..5e8ba0a2a 100644 --- a/webui/src/components/PermissionCard.tsx +++ b/webui/src/components/PermissionCard.tsx @@ -7,6 +7,7 @@ import { formatToolPayload } from '../lib/toolDisplay'; interface PermissionRequest { session_id: string; + approval_id?: string; tool_name: string; reason: string; call_id: string; @@ -46,13 +47,17 @@ export function PermissionCard({ req, onDone, onDecide, dock }: PermissionCardPr if (onDecide) { await onDecide(decision, req.tool_name); } else { - await respondPermission(req.session_id, decision, req.tool_name); + if (!req.approval_id) throw new Error('missing approval identity'); + const result = await respondPermission(req.session_id, req.approval_id, decision, req.tool_name); + if (!result.success) throw new Error('permission request is no longer pending'); } + onDone(); } catch { - // Best-effort; proceed to dismiss regardless + // Keep the exact approval visible and actionable when delivery was not + // confirmed. Poll/SSE reconciliation can still retire a genuinely stale + // request, while transient transport failures remain retryable. } finally { setLoading(false); - onDone(); } } diff --git a/webui/src/lib/pendingPermission.ts b/webui/src/lib/pendingPermission.ts index 06e6deece..c011a7f8e 100644 --- a/webui/src/lib/pendingPermission.ts +++ b/webui/src/lib/pendingPermission.ts @@ -16,11 +16,29 @@ export interface PendingLike { call_id: string; + request_id?: number; + generation?: number; } export function resolvePendingAfterDecision( current: T | null, decidedCallId: string, + decidedRequestId?: number, + decidedGeneration?: number, ): T | null { + if ( + current + && decidedRequestId !== undefined + && current.request_id !== undefined + ) { + if ( + decidedGeneration !== undefined + && current.generation !== undefined + && current.generation !== decidedGeneration + ) { + return current; + } + return current.request_id === decidedRequestId ? null : current; + } return current && current.call_id === decidedCallId ? null : current; } diff --git a/webui/src/lib/sessionNotify.test.ts b/webui/src/lib/sessionNotify.test.ts index 9128034e8..36bd979de 100644 --- a/webui/src/lib/sessionNotify.test.ts +++ b/webui/src/lib/sessionNotify.test.ts @@ -2,6 +2,8 @@ import assert from 'node:assert/strict'; import test from 'node:test'; import { dispatchSystemNotification, + permissionInstanceKey, + samePermissionInstance, sessionNoticeLabel, shouldEmitNotice, shouldOsNotifyReview, @@ -13,6 +15,48 @@ import { terminalKindFromDone, } from './sessionNotify.ts'; +test('permission identity distinguishes reused call ids by approval id', () => { + const first = { call_id: 'ollama_call_0', approval_id: 'approval-a' }; + const second = { call_id: 'ollama_call_0', approval_id: 'approval-b' }; + assert.notEqual( + permissionInstanceKey('session-1', first), + permissionInstanceKey('session-1', second), + ); + assert.equal(samePermissionInstance(first, second), false); + assert.equal( + samePermissionInstance(first, { call_id: 'ollama_call_0' }), + false, + 'a strong approval identity must not collapse into a legacy raw call id', + ); +}); + +test('live permission identity distinguishes reused call ids by native request id', () => { + const first = { call_id: 'ollama_call_0', request_id: 11 }; + const second = { call_id: 'ollama_call_0', request_id: 12 }; + assert.notEqual( + permissionInstanceKey('session-1', first), + permissionInstanceKey('session-1', second), + ); + assert.equal(samePermissionInstance(first, second), false); +}); + +test('live permission identity distinguishes reused request ids across runtime generations', () => { + const first = { call_id: 'call-reused', request_id: 1, generation: 7 }; + const second = { call_id: 'call-reused', request_id: 1, generation: 8 }; + assert.notEqual( + permissionInstanceKey('session-1', first), + permissionInstanceKey('session-1', second), + ); + assert.equal(samePermissionInstance(first, second), false); +}); + +test('strong chat and live identities never collapse merely because call id is reused', () => { + const chat = { call_id: 'ollama_call_0', approval_id: 'approval-old' }; + const live = { call_id: 'ollama_call_0', request_id: 73 }; + assert.equal(samePermissionInstance(chat, live), false); + assert.equal(samePermissionInstance(live, chat), false); +}); + test('build and plan notify on each review; auto does not', () => { assert.equal(showPermissionNotice('build'), true); assert.equal(showPermissionNotice('plan'), true); diff --git a/webui/src/lib/sessionNotify.ts b/webui/src/lib/sessionNotify.ts index 55cb7c1d1..e16926e55 100644 --- a/webui/src/lib/sessionNotify.ts +++ b/webui/src/lib/sessionNotify.ts @@ -224,7 +224,68 @@ export interface SystemNotificationOptions { body: string; tag?: string; sessionId?: string; + approvalId?: string; }) => Promise; + approvalId?: string; +} + +export interface PermissionNoticeIdentity { + call_id: string; + approval_id?: string | null; + request_id?: number | null; + generation?: number | null; +} + +export function permissionInstanceKey( + sessionId: string, + permission: PermissionNoticeIdentity, +): string { + const approvalId = permission.approval_id?.trim(); + if (approvalId) return `${sessionId}:approval:${approvalId}`; + if (permission.request_id !== undefined && permission.request_id !== null) { + const generation = permission.generation; + return generation !== undefined && generation !== null + ? `${sessionId}:generation:${generation}:request:${permission.request_id}` + : `${sessionId}:request:${permission.request_id}`; + } + return `${sessionId}:call:${permission.call_id}`; +} + +export function samePermissionInstance( + a: PermissionNoticeIdentity | null | undefined, + b: PermissionNoticeIdentity | null | undefined, +): boolean { + if (!a || !b) return false; + const aApproval = a.approval_id?.trim(); + const bApproval = b.approval_id?.trim(); + if (aApproval || bApproval) { + return Boolean(aApproval && bApproval && aApproval === bApproval); + } + const aRequest = a.request_id; + const bRequest = b.request_id; + if (aRequest !== undefined && aRequest !== null || bRequest !== undefined && bRequest !== null) { + if (!(aRequest !== undefined + && aRequest !== null + && bRequest !== undefined + && bRequest !== null)) { + return false; + } + const aGeneration = a.generation; + const bGeneration = b.generation; + if ( + aGeneration !== undefined && aGeneration !== null + || bGeneration !== undefined && bGeneration !== null + ) { + return aGeneration !== undefined + && aGeneration !== null + && bGeneration !== undefined + && bGeneration !== null + && aGeneration === bGeneration + && aRequest === bRequest; + } + return aRequest === bRequest; + } + return a.call_id === b.call_id; } /** @@ -233,7 +294,7 @@ export interface SystemNotificationOptions { * Browser Web Notifications are deliberately excluded to avoid duplicate toasts. */ export function dispatchSystemNotification(opts: SystemNotificationOptions): void { - const { title, body, sessionId, tag, postSystemNotifyFn } = opts; + const { title, body, sessionId, tag, approvalId, postSystemNotifyFn } = opts; // 统一通过后端分发操作系统级原生弹窗通知（Windows / macOS / Linux） if (postSystemNotifyFn) { void postSystemNotifyFn({ @@ -241,6 +302,7 @@ export function dispatchSystemNotification(opts: SystemNotificationOptions): voi body, tag, sessionId: sessionId || undefined, + ...(approvalId ? { approvalId } : {}), }).catch(() => {}); } } From 3b3a4258e6f8b605c3fe1eb9e349ea302f6acd0d Mon Sep 17 00:00:00 2001 From: xuan2261 Date: Tue, 6 Oct 2026 12:05:11 +0700 Subject: [PATCH 2/4] fix(daemon): preserve interactive request timeout Co-Authored-By: JeikCode --- crates/jeikcode-daemon/src/live_api.rs | 17 ++++++---- crates/jeikcode-daemon/src/live_hub.rs | 43 ++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/crates/jeikcode-daemon/src/live_api.rs b/crates/jeikcode-daemon/src/live_api.rs index df9eff254..9b43e1225 100644 --- a/crates/jeikcode-daemon/src/live_api.rs +++ b/crates/jeikcode-daemon/src/live_api.rs @@ -880,10 +880,12 @@ pub(crate) async fn steer_into_running_turn( fn configure_chat_runtime_interactivity( runtime_cfg: &mut jeikcode_coding::CodingRuntimeConfig, has_interactive_driver: bool, -) { +) -> Option { + let driver_request_timeout = runtime_cfg.agent_config().request_timeout; if has_interactive_driver { runtime_cfg.interactive = true; } + driver_request_timeout } pub(crate) async fn run_chat_turn_v2( @@ -904,7 +906,7 @@ pub(crate) async fn run_chat_turn_v2( // cancellation/liveness at the transport seam. Disable the kernel's second, // equal request timeout so it cannot expire the RequestCtx first and then // race a later HTTP decision that the daemon still considers pending. - configure_chat_runtime_interactivity( + let driver_request_timeout = configure_chat_runtime_interactivity( &mut runtime_cfg, permission_responders.is_some() || user_input_responders.is_some(), ); @@ -1135,7 +1137,7 @@ pub(crate) async fn run_chat_turn_v2( (PermissionDecision::Deny, false) } else { let response = - await_chat_permission_response(rx, coding_cfg.request_timeout); + await_chat_permission_response(rx, driver_request_timeout); tokio::pin!(response); loop { tokio::select! { @@ -1243,7 +1245,7 @@ pub(crate) async fn run_chat_turn_v2( // Register before publishing the SSE event so a very fast browser answer // cannot race the response route and be rejected as stale. let _ = runtime_event_tx.send(CodingRuntimeEvent::Request(request.clone())); - let answer = await_chat_user_input_response(rx, coding_cfg.request_timeout); + let answer = await_chat_user_input_response(rx, driver_request_timeout); tokio::pin!(answer); let value = tokio::select! { _ = cancel.cancelled(), if !cancelled => { @@ -4078,8 +4080,11 @@ mod tests { false, false, ); - assert!(runtime_cfg.agent_config().request_timeout.is_some()); - configure_chat_runtime_interactivity(&mut runtime_cfg, true); + let configured_timeout = runtime_cfg.agent_config().request_timeout; + assert!(configured_timeout.is_some()); + let driver_request_timeout = configure_chat_runtime_interactivity(&mut runtime_cfg, true); + assert_eq!(driver_request_timeout, configured_timeout); + assert!(driver_request_timeout.is_some()); assert!(runtime_cfg.agent_config().request_timeout.is_none()); } diff --git a/crates/jeikcode-daemon/src/live_hub.rs b/crates/jeikcode-daemon/src/live_hub.rs index d74ed99e7..cad0a08e8 100644 --- a/crates/jeikcode-daemon/src/live_hub.rs +++ b/crates/jeikcode-daemon/src/live_hub.rs @@ -1930,6 +1930,49 @@ mod tests { ); } + #[tokio::test] + async fn confirmed_response_rejects_stale_generation_before_reused_request_id() { + let hub = LiveViewHub::new(); + let (control, commands) = control(); + let binding = hub + .bind("session-1", PathBuf::from("/one"), snapshot("one"), control) + .unwrap(); + hub.publish( + &binding, + SequencedRuntimeEvent { + generation: 1, + sequence: 1, + event: CodingRuntimeEvent::Request(jeikcode_coding::RuntimeRequest { + id: 42, + kind: "approval".into(), + payload: serde_json::json!({}), + snapshot: None, + }), + }, + ) + .unwrap(); + + assert_eq!( + hub.respond_confirmed_for_generation(0, 42, serde_json::Value::Null) + .await + .unwrap_err(), + HubError::RuntimeGenerationChanged { + expected: 0, + actual: 1, + } + ); + assert!(commands.lock().unwrap().is_empty()); + assert!(hub + .join() + .unwrap() + .replay + .iter() + .any(|observation| matches!( + &observation.event, + LiveViewEvent::Runtime(CodingRuntimeEvent::Request(request)) if request.id == 42 + ))); + } + #[test] fn terminal_snapshot_replaces_replay_atomically() { let hub = LiveViewHub::new(); From 401e8f5e1e9a9a2248bf0defc91bb25c2fb46b91 Mon Sep 17 00:00:00 2001 From: xuan2261 Date: Tue, 6 Oct 2026 14:19:22 +0700 Subject: [PATCH 3/4] fix(daemon): close approval runtime race windows Co-Authored-By: JeikCode --- crates/jeikcode-coding/src/runtime.rs | 22 + .../src/session_runtime_registry.rs | 464 +++++++++++++++++- crates/jeikcode-daemon/src/lib.rs | 113 ++++- crates/jeikcode-daemon/src/live_api.rs | 190 ++++++- crates/jeikcode-daemon/src/live_hub.rs | 158 ++++++ crates/jeikcode-daemon/src/native_live.rs | 68 ++- crates/jeikcode-tuix/src/event_loop/mod.rs | 24 +- webui/src/api.test.ts | 13 +- webui/src/api.ts | 6 +- webui/src/app.tsx | 9 +- webui/src/components/Chat.tsx | 20 +- webui/src/components/NotificationDock.tsx | 49 +- webui/src/lib/pendingPermission.test.ts | 31 ++ webui/src/lib/pendingPermission.ts | 18 + webui/src/lib/sessionNotify.test.ts | 41 ++ webui/src/lib/sessionNotify.ts | 29 ++ 16 files changed, 1158 insertions(+), 97 deletions(-) create mode 100644 webui/src/lib/pendingPermission.test.ts diff --git a/crates/jeikcode-coding/src/runtime.rs b/crates/jeikcode-coding/src/runtime.rs index 635766dca..988e5f623 100644 --- a/crates/jeikcode-coding/src/runtime.rs +++ b/crates/jeikcode-coding/src/runtime.rs @@ -1012,6 +1012,10 @@ pub struct CodingRuntimeHandle { state: Arc, provider_unavailable_reason: Arc, terminal: watch::Receiver>, + /// Stable identity for this concrete runtime owner. Generations are scoped to + /// one owner and may restart when a runtime is replaced, so external + /// correlation must pair them with this value. + instance_id: Arc, } #[derive(Clone, Debug)] @@ -1035,6 +1039,10 @@ pub enum DeferredRuntimeState { } impl CodingRuntimeHandle { + pub fn instance_id(&self) -> &str { + &self.instance_id + } + pub fn is_stopped(&self) -> bool { self.terminal.borrow().is_some() || self.tx.is_closed() } @@ -2239,12 +2247,14 @@ pub fn coding_runtime_control_channel() -> (CodingRuntimeHandle, CodingRuntimeCo // this flag at spawn time when startup produced only a degraded placeholder. let state = Arc::new(AtomicU64::new(runtime_state(0, true))); let provider_unavailable_reason = Arc::new(AtomicU8::new(0)); + let instance_id: Arc = Arc::from(uuid::Uuid::new_v4().simple().to_string()); ( CodingRuntimeHandle { tx, state: Arc::clone(&state), provider_unavailable_reason: Arc::clone(&provider_unavailable_reason), terminal, + instance_id, }, CodingRuntimeControlReceiver { rx, @@ -10593,6 +10603,18 @@ mod tests { adapter.shutdown().await.unwrap(); } + #[test] + fn runtime_control_channel_instance_identity_is_stable_per_owner() { + let (first, _first_controls) = coding_runtime_control_channel(); + let first_clone = first.clone(); + let (second, _second_controls) = coding_runtime_control_channel(); + + assert_eq!(first.instance_id(), first_clone.instance_id()); + assert_ne!(first.instance_id(), second.instance_id()); + assert_eq!(first.status().generation, 0); + assert_eq!(second.status().generation, 0); + } + #[tokio::test] async fn stopped_or_degraded_runtime_rejects_compaction() { let (agent, _commands, _events) = fake_agent(); diff --git a/crates/jeikcode-coding/src/session_runtime_registry.rs b/crates/jeikcode-coding/src/session_runtime_registry.rs index 5d8ac1a97..89fd5215f 100644 --- a/crates/jeikcode-coding/src/session_runtime_registry.rs +++ b/crates/jeikcode-coding/src/session_runtime_registry.rs @@ -76,6 +76,10 @@ pub enum OpenOutcome { #[derive(Debug, Clone)] pub struct SequencedSessionEvent { pub session_id: SessionKey, + /// Concrete runtime owner that emitted this observation. Unlike the runtime + /// generation, this changes when a session is rebound to a replacement + /// runtime, even if that replacement restarts its generation counter. + pub runtime_instance_id: Option, pub generation: u64, pub sequence: u64, pub activity: RuntimeActivity, @@ -117,6 +121,8 @@ pub struct SessionRuntimeEntry { pub working_dir: PathBuf, pub activity: RuntimeActivity, pub generation: u64, + /// Concrete runtime owner currently bound to this session. + pub runtime_instance_id: Option, /// Last known conversation snapshot (optional; filled by drivers). pub snapshot: Option, /// Transport id used by TUI event fan-in (optional). @@ -128,13 +134,21 @@ pub struct SessionRuntimeEntry { pub last_terminal: Option, } +#[derive(Debug, Clone, PartialEq, Eq)] +struct PendingRuntimeRequest { + runtime_instance_id: Option, + generation: u64, + request_id: RequestId, + kind: String, +} + struct LiveInner { meta: SessionRuntimeEntry, handle: Option, journal: VecDeque, next_sequence: u64, event_tx: broadcast::Sender, - pending_request_id: Option, + pending_request: Option, /// Recent agent-observation fingerprints. An observer echoing the journal /// back (A,B,A,B…) is dropped before it can peg a core. recent_runtime_keys: VecDeque, @@ -147,7 +161,7 @@ impl std::fmt::Debug for LiveInner { .field("has_handle", &self.handle.is_some()) .field("journal_len", &self.journal.len()) .field("next_sequence", &self.next_sequence) - .field("pending_request_id", &self.pending_request_id) + .field("pending_request", &self.pending_request) .finish() } } @@ -161,6 +175,7 @@ impl LiveInner { working_dir, activity: RuntimeActivity::Starting, generation: 1, + runtime_instance_id: None, snapshot: None, runtime_id: None, terminal_seq: 0, @@ -170,13 +185,19 @@ impl LiveInner { journal: VecDeque::new(), next_sequence: 0, event_tx, - pending_request_id: None, + pending_request: None, recent_runtime_keys: VecDeque::new(), } } fn push_activity(&mut self, activity: RuntimeActivity) -> SequencedSessionEvent { - self.push(activity, None, None) + self.push( + activity, + self.meta.runtime_instance_id.clone(), + self.meta.generation, + None, + None, + ) } fn push_runtime( @@ -188,7 +209,24 @@ impl LiveInner { self.meta.generation = generation; } let activity = activity_from_runtime_event(&event, self.meta.activity); - self.push(activity, Some(event), None) + self.push(activity, None, self.meta.generation, Some(event), None) + } + + fn push_runtime_for_instance( + &mut self, + runtime_instance_id: &str, + generation: u64, + event: crate::runtime::CodingRuntimeEvent, + ) -> SequencedSessionEvent { + self.meta.generation = generation; + let activity = activity_from_runtime_event(&event, self.meta.activity); + self.push( + activity, + Some(runtime_instance_id.to_string()), + generation, + Some(event), + None, + ) } fn push_view(&mut self, view: SessionViewEvent) -> SequencedSessionEvent { @@ -199,12 +237,42 @@ impl LiveInner { SessionViewEvent::RequestResolved { .. } => RuntimeActivity::Running, SessionViewEvent::CommandOutput(_) => self.meta.activity, }; - self.push(activity, None, Some(view)) + self.push( + activity, + self.meta.runtime_instance_id.clone(), + self.meta.generation, + None, + Some(view), + ) + } + + fn push_view_for_instance( + &mut self, + runtime_instance_id: &str, + generation: u64, + view: SessionViewEvent, + ) -> SequencedSessionEvent { + let activity = match &view { + SessionViewEvent::InputAccepted { .. } | SessionViewEvent::Steered { .. } => { + RuntimeActivity::Running + } + SessionViewEvent::RequestResolved { .. } => RuntimeActivity::Running, + SessionViewEvent::CommandOutput(_) => self.meta.activity, + }; + self.push( + activity, + Some(runtime_instance_id.to_string()), + generation, + None, + Some(view), + ) } fn push( &mut self, activity: RuntimeActivity, + runtime_instance_id: Option, + generation: u64, runtime: Option, view: Option, ) -> SequencedSessionEvent { @@ -219,7 +287,8 @@ impl LiveInner { self.next_sequence = self.next_sequence.wrapping_add(1); let event = SequencedSessionEvent { session_id: self.meta.session_id.clone(), - generation: self.meta.generation, + runtime_instance_id, + generation, sequence: seq, activity, runtime, @@ -446,6 +515,22 @@ impl SessionRuntimeRegistry { let Some(inner) = guard.get_mut(key) else { return false; }; + let incoming_instance_id = handle.instance_id().to_string(); + if let Some(existing) = inner.handle.as_ref() { + if existing.instance_id() != incoming_instance_id && !existing.is_stopped() { + return false; + } + if existing.instance_id() != incoming_instance_id { + // A replacement runtime is only accepted after the previous owner + // has stopped. Its pending request and replay window are scoped to + // that old owner and must never survive into the replacement. + inner.pending_request = None; + inner.journal.clear(); + inner.recent_runtime_keys.clear(); + } + } + inner.meta.runtime_instance_id = Some(incoming_instance_id); + inner.meta.generation = handle.status().generation; inner.handle = Some(handle); inner.meta.runtime_id = runtime_id; if matches!( @@ -534,7 +619,7 @@ impl SessionRuntimeRegistry { .read() .unwrap_or_else(|e| e.into_inner()) .get(key) - .and_then(|e| e.pending_request_id) + .and_then(|e| e.pending_request.as_ref().map(|pending| pending.request_id)) } pub fn set_pending_request(&self, key: &SessionKey, id: Option) -> bool { @@ -542,8 +627,13 @@ impl SessionRuntimeRegistry { let Some(inner) = guard.get_mut(key) else { return false; }; - inner.pending_request_id = id; - if id.is_some() { + inner.pending_request = id.map(|request_id| PendingRuntimeRequest { + runtime_instance_id: inner.meta.runtime_instance_id.clone(), + generation: inner.meta.generation, + request_id, + kind: "approval".to_string(), + }); + if inner.pending_request.is_some() { inner.push_activity(RuntimeActivity::WaitingApproval); } true @@ -581,8 +671,12 @@ impl SessionRuntimeRegistry { self.dispatch(key, DriverCommand::Respond { id, value })?; let mut guard = self.entries.write().unwrap_or_else(|e| e.into_inner()); if let Some(inner) = guard.get_mut(key) { - if inner.pending_request_id == Some(id) { - inner.pending_request_id = None; + if inner + .pending_request + .as_ref() + .is_some_and(|pending| pending.request_id == id) + { + inner.pending_request = None; } } Ok(()) @@ -622,10 +716,19 @@ impl SessionRuntimeRegistry { inner.meta.snapshot = Some((**snapshot).clone()); } if let crate::runtime::CodingRuntimeEvent::Request(request) = &event { - inner.pending_request_id = Some(request.id); + inner.pending_request = Some(PendingRuntimeRequest { + runtime_instance_id: None, + generation: if generation > 0 { + generation + } else { + inner.meta.generation + }, + request_id: request.id, + kind: request.kind.clone(), + }); } if let crate::runtime::CodingRuntimeEvent::TurnFinished(_) = &event { - inner.pending_request_id = None; + inner.pending_request = None; } // Snapshot already has the completed turn. Replaying TextDelta / ToolStart // on top of it duplicates the last assistant (text + tools) after a @@ -652,6 +755,70 @@ impl SessionRuntimeRegistry { true } + /// Append an observation only when it came from the concrete runtime that + /// still owns this session. The producer supplies its immutable instance id; + /// never infer it from the registry row after an async rebind. + pub fn push_runtime_event_for_runtime( + &self, + key: &SessionKey, + expected_runtime_instance_id: &str, + generation: u64, + event: crate::runtime::CodingRuntimeEvent, + ) -> bool { + let mut guard = self.entries.write().unwrap_or_else(|e| e.into_inner()); + let Some(inner) = guard.get_mut(key) else { + return false; + }; + if inner.meta.runtime_instance_id.as_deref() != Some(expected_runtime_instance_id) { + return false; + } + let Some(handle) = inner.handle.as_ref() else { + return false; + }; + if handle.instance_id() != expected_runtime_instance_id + || handle.status().generation != generation + { + return false; + } + if let crate::runtime::CodingRuntimeEvent::TurnFinished( + crate::runtime::TurnCompletion::Completed { snapshot, .. }, + ) = &event + { + inner.meta.snapshot = Some((**snapshot).clone()); + } + if let crate::runtime::CodingRuntimeEvent::Request(request) = &event { + inner.pending_request = Some(PendingRuntimeRequest { + runtime_instance_id: Some(expected_runtime_instance_id.to_string()), + generation, + request_id: request.id, + kind: request.kind.clone(), + }); + } + if let crate::runtime::CodingRuntimeEvent::TurnFinished(_) = &event { + inner.pending_request = None; + } + if matches!( + &event, + crate::runtime::CodingRuntimeEvent::TurnFinished(_) + | crate::runtime::CodingRuntimeEvent::RuntimeStopped(_) + ) { + inner.journal.clear(); + inner.recent_runtime_keys.clear(); + } + if let Some(key) = consecutive_runtime_key(&event) { + if inner.recent_runtime_keys.iter().any(|seen| seen == &key) { + return false; + } + const RECENT_KEY_CAP: usize = 64; + if inner.recent_runtime_keys.len() >= RECENT_KEY_CAP { + inner.recent_runtime_keys.pop_front(); + } + inner.recent_runtime_keys.push_back(key); + } + inner.push_runtime_for_instance(expected_runtime_instance_id, generation, event); + true + } + /// Fan out a view-layer event (InputAccepted / Steered / RequestResolved / CommandOutput). pub fn push_view_event(&self, key: &SessionKey, view: SessionViewEvent) -> bool { let mut guard = self.entries.write().unwrap_or_else(|e| e.into_inner()); @@ -659,14 +826,95 @@ impl SessionRuntimeRegistry { return false; }; if let SessionViewEvent::RequestResolved { request_id, .. } = &view { - if inner.pending_request_id == Some(*request_id) { - inner.pending_request_id = None; + if inner + .pending_request + .as_ref() + .is_some_and(|pending| pending.request_id == *request_id) + { + inner.pending_request = None; } } inner.push_view(view); true } + /// Push a view event only while the same concrete runtime owner is still + /// bound. This prevents a response completed on an old runtime from clearing + /// a reused request id on a replacement runtime. + pub fn push_view_event_for_runtime( + &self, + key: &SessionKey, + expected_runtime_instance_id: &str, + expected_generation: u64, + view: SessionViewEvent, + ) -> bool { + let mut guard = self.entries.write().unwrap_or_else(|e| e.into_inner()); + let Some(inner) = guard.get_mut(key) else { + return false; + }; + if inner.meta.runtime_instance_id.as_deref() != Some(expected_runtime_instance_id) { + return false; + } + let Some(handle) = inner.handle.as_ref() else { + return false; + }; + if handle.instance_id() != expected_runtime_instance_id + || handle.status().generation != expected_generation + { + return false; + } + if inner.meta.generation != expected_generation { + return false; + } + if let SessionViewEvent::RequestResolved { request_id, kind } = &view { + let matches = inner.pending_request.as_ref().is_some_and(|pending| { + pending.runtime_instance_id.as_deref() == Some(expected_runtime_instance_id) + && pending.generation == expected_generation + && pending.request_id == *request_id + && pending.kind == *kind + }); + if !matches { + return false; + } + inner.pending_request = None; + } + inner.push_view_for_instance(expected_runtime_instance_id, expected_generation, view); + true + } + + pub fn pending_request_matches( + &self, + key: &SessionKey, + expected_runtime_instance_id: &str, + expected_generation: u64, + request_id: RequestId, + kind: &str, + ) -> bool { + let guard = self.entries.read().unwrap_or_else(|e| e.into_inner()); + let Some(inner) = guard.get(key) else { + return false; + }; + if inner.meta.runtime_instance_id.as_deref() != Some(expected_runtime_instance_id) + || inner.meta.generation != expected_generation + { + return false; + } + let Some(handle) = inner.handle.as_ref() else { + return false; + }; + if handle.instance_id() != expected_runtime_instance_id + || handle.status().generation != expected_generation + { + return false; + } + inner.pending_request.as_ref().is_some_and(|pending| { + pending.runtime_instance_id.as_deref() == Some(expected_runtime_instance_id) + && pending.generation == expected_generation + && pending.request_id == request_id + && pending.kind == kind + }) + } + /// Ensure the session exists, then push a view event. pub fn ensure_and_push_view( &self, @@ -695,6 +943,15 @@ impl SessionRuntimeRegistry { let Some(key) = key else { return false; }; + if let Some(handle) = self.handle(&key) { + let runtime_instance_id = handle.instance_id().to_string(); + return self.push_runtime_event_for_runtime( + &key, + &runtime_instance_id, + generation, + event, + ); + } self.push_runtime_event(&key, generation, event) } @@ -704,6 +961,22 @@ impl SessionRuntimeRegistry { guard.get(key).and_then(|e| e.handle.clone()) } + /// Bound handle only when it is still the exact runtime instance named by + /// an external correlation token. + pub fn handle_for_runtime_instance( + &self, + key: &SessionKey, + expected_runtime_instance_id: &str, + ) -> Option { + let guard = self.entries.read().unwrap_or_else(|e| e.into_inner()); + let inner = guard.get(key)?; + if inner.meta.runtime_instance_id.as_deref() != Some(expected_runtime_instance_id) { + return None; + } + let handle = inner.handle.clone()?; + (handle.instance_id() == expected_runtime_instance_id).then_some(handle) + } + /// Look up session id by TUI transport runtime id. pub fn session_id_for_runtime_id(&self, runtime_id: u64) -> Option { let guard = self.entries.read().unwrap_or_else(|e| e.into_inner()); @@ -879,6 +1152,165 @@ mod tests { assert_eq!(e1.session_id, e2.session_id); } + #[test] + fn bound_runtime_generation_zero_is_authoritative_for_correlated_requests() { + let reg = SessionRuntimeRegistry::new(); + let key = "s1".to_string(); + reg.open_or_attach(key.clone(), PathBuf::from("/proj")) + .unwrap(); + let (handle, _control) = crate::runtime::coding_runtime_control_channel(); + let runtime_instance_id = handle.instance_id().to_string(); + assert_eq!(handle.status().generation, 0); + assert!(reg.bind_handle(&key, handle, None)); + + let bound = reg.lookup(&key).expect("bound registry entry"); + assert_eq!(bound.generation, 0); + assert_eq!( + bound.runtime_instance_id.as_deref(), + Some(runtime_instance_id.as_str()) + ); + assert!(reg.push_runtime_event_for_runtime( + &key, + &runtime_instance_id, + 0, + crate::runtime::CodingRuntimeEvent::Request(crate::runtime::RuntimeRequest { + id: 1, + kind: "approval".into(), + payload: serde_json::json!({}), + snapshot: None, + }), + )); + assert!(reg.pending_request_matches(&key, &runtime_instance_id, 0, 1, "approval")); + let (replay, _rx) = reg.subscribe(&key, None).unwrap(); + assert!(replay.iter().any(|event| { + event.runtime_instance_id.as_deref() == Some(runtime_instance_id.as_str()) + && event.generation == 0 + && matches!( + &event.runtime, + Some(crate::runtime::CodingRuntimeEvent::Request(request)) if request.id == 1 + ) + })); + } + + #[test] + fn replacement_runtime_rejects_old_owner_with_reused_generation_and_request_id() { + let reg = SessionRuntimeRegistry::new(); + let key = "s1".to_string(); + reg.open_or_attach(key.clone(), PathBuf::from("/proj")) + .unwrap(); + + let (old_handle, old_control) = crate::runtime::coding_runtime_control_channel(); + let old_instance_id = old_handle.instance_id().to_string(); + assert!(reg.bind_handle(&key, old_handle.clone(), None)); + drop(old_control); + assert!(old_handle.is_stopped()); + + let (new_handle, _new_control) = crate::runtime::coding_runtime_control_channel(); + let new_instance_id = new_handle.instance_id().to_string(); + assert_ne!(old_instance_id, new_instance_id); + assert_eq!(new_handle.status().generation, 0); + assert!(reg.bind_handle(&key, new_handle, None)); + assert!(reg.push_runtime_event_for_runtime( + &key, + &new_instance_id, + 0, + crate::runtime::CodingRuntimeEvent::Request(crate::runtime::RuntimeRequest { + id: 1, + kind: "approval".into(), + payload: serde_json::json!({}), + snapshot: None, + }), + )); + + assert!(!reg.push_runtime_event_for_runtime( + &key, + &old_instance_id, + 0, + crate::runtime::CodingRuntimeEvent::Request(crate::runtime::RuntimeRequest { + id: 1, + kind: "approval".into(), + payload: serde_json::json!({}), + snapshot: None, + }), + )); + assert!(!reg.pending_request_matches(&key, &old_instance_id, 0, 1, "approval")); + assert!(reg.pending_request_matches(&key, &new_instance_id, 0, 1, "approval")); + assert_eq!( + reg.lookup(&key).unwrap().runtime_instance_id.as_deref(), + Some(new_instance_id.as_str()) + ); + } + + #[test] + fn active_runtime_cannot_be_rebound_to_a_different_owner() { + let reg = SessionRuntimeRegistry::new(); + let key = "s1".to_string(); + reg.open_or_attach(key.clone(), PathBuf::from("/proj")) + .unwrap(); + let (first, _first_control) = crate::runtime::coding_runtime_control_channel(); + let first_instance_id = first.instance_id().to_string(); + let (second, _second_control) = crate::runtime::coding_runtime_control_channel(); + + assert!(reg.bind_handle(&key, first, None)); + assert!(!reg.bind_handle(&key, second, None)); + assert_eq!( + reg.lookup(&key).unwrap().runtime_instance_id.as_deref(), + Some(first_instance_id.as_str()) + ); + } + + #[test] + fn runtime_scoped_resolution_requires_exact_generation_request_and_kind() { + let reg = SessionRuntimeRegistry::new(); + let key = "s1".to_string(); + reg.open_or_attach(key.clone(), PathBuf::from("/proj")) + .unwrap(); + let (handle, _control) = crate::runtime::coding_runtime_control_channel(); + let runtime_instance_id = handle.instance_id().to_string(); + assert!(reg.bind_handle(&key, handle, None)); + assert!(reg.push_runtime_event_for_runtime( + &key, + &runtime_instance_id, + 0, + crate::runtime::CodingRuntimeEvent::Request(crate::runtime::RuntimeRequest { + id: 9, + kind: "approval".into(), + payload: serde_json::json!({}), + snapshot: None, + }), + )); + + assert!(!reg.push_view_event_for_runtime( + &key, + &runtime_instance_id, + 1, + SessionViewEvent::RequestResolved { + request_id: 9, + kind: "approval".into(), + }, + )); + assert!(!reg.push_view_event_for_runtime( + &key, + &runtime_instance_id, + 0, + SessionViewEvent::RequestResolved { + request_id: 9, + kind: "user_input".into(), + }, + )); + assert!(reg.pending_request_matches(&key, &runtime_instance_id, 0, 9, "approval")); + assert!(reg.push_view_event_for_runtime( + &key, + &runtime_instance_id, + 0, + SessionViewEvent::RequestResolved { + request_id: 9, + kind: "approval".into(), + }, + )); + assert!(!reg.pending_request_matches(&key, &runtime_instance_id, 0, 9, "approval")); + } + #[tokio::test] async fn list_activity_scopes_by_working_dir() { let reg = SessionRuntimeRegistry::new(); diff --git a/crates/jeikcode-daemon/src/lib.rs b/crates/jeikcode-daemon/src/lib.rs index 22131e8f4..8b68210ae 100644 --- a/crates/jeikcode-daemon/src/lib.rs +++ b/crates/jeikcode-daemon/src/lib.rs @@ -809,6 +809,10 @@ struct ActiveChatOperation { session_id: Option, aliases: Vec, cancellation: CancellationToken, + /// Linearizes user stop against approval consumption. A stop must mark this + /// gate before signalling the cancellation token; approval delivery holds + /// the same gate until the runtime has consumed (or rejected) its response. + stop_gate: Arc>, /// Signalled in `complete` so a preempting request can wait until the /// cancelled turn has persisted its snapshot and left the registry. finished: Arc, @@ -919,6 +923,7 @@ impl ActiveChatRegistry { let operation_id = uuid::Uuid::new_v4().to_string(); let cancellation = CancellationToken::new(); + let stop_gate = Arc::new(Mutex::new(false)); // Capacity: lagging watchers drop oldest; primary SSE is separate. let (event_bus, _) = tokio::sync::broadcast::channel(512); // Clone for waking standby watchers (the original moves into the operation). @@ -945,6 +950,7 @@ impl ActiveChatRegistry { session_id: session_id.clone(), aliases, cancellation: cancellation.clone(), + stop_gate, finished: Arc::new(Notify::new()), stopped: false, terminal_reached: false, @@ -1204,19 +1210,14 @@ impl ActiveChatRegistry { /// Mark and cooperatively cancel an operation addressed by either alias. async fn stop_alias(&self, alias: &str) -> bool { - let cancellation = { - let mut index = self.inner.write().await; + let operation_id = { + let index = self.inner.read().await; let Some(operation_id) = index.aliases.get(alias).cloned() else { return false; }; - let Some(operation) = index.operations.get_mut(&operation_id) else { - return false; - }; - operation.stopped = true; - operation.cancellation.clone() + operation_id }; - cancellation.cancel(); - true + self.stop_operation(&operation_id).await } /// Stop the occupant of `alias` (same path as WebUI `/chat/stop`) and wait @@ -1259,21 +1260,52 @@ impl ActiveChatRegistry { } async fn stop_alias_owned(&self, alias: String) -> bool { - let cancellation = { - let mut index = self.inner.write().await; + let operation_id = { + let index = self.inner.read().await; let Some(operation_id) = index.aliases.get(&alias).cloned() else { return false; }; - let Some(operation) = index.operations.get_mut(&operation_id) else { + operation_id + }; + self.stop_operation(&operation_id).await + } + + async fn stop_operation(&self, operation_id: &str) -> bool { + let (cancellation, stop_gate) = { + let index = self.inner.read().await; + let Some(operation) = index.operations.get(operation_id) else { return false; }; - operation.stopped = true; - operation.cancellation.clone() + (operation.cancellation.clone(), operation.stop_gate.clone()) }; + + // This is the stop/approval linearization point. If an approval already + // holds the gate, its runtime consumption wins and this stop follows it. + // Otherwise mark stopped before the cancellation token becomes visible, + // so a later approval cannot cross the runtime boundary. + { + let mut stopped = stop_gate.lock().await; + *stopped = true; + } + { + let mut index = self.inner.write().await; + if let Some(operation) = index.operations.get_mut(operation_id) { + operation.stopped = true; + } + } cancellation.cancel(); true } + async fn stop_gate(&self, operation_id: &str) -> Option>> { + self.inner + .read() + .await + .operations + .get(operation_id) + .map(|operation| operation.stop_gate.clone()) + } + /// Compat latest-wins: if the session is busy, cancel the running turn /// (bash/tools included), wait for snapshot persist + `complete`, then admit. /// Retries until this caller occupies the aliases or `timeout` elapses. @@ -1425,15 +1457,16 @@ impl ActiveChatRegistry { #[cfg(test)] async fn cancel_all(&self) { - let cancellations: Vec = self + let cancellations: Vec<(CancellationToken, Arc>)> = self .inner .read() .await .operations .values() - .map(|operation| operation.cancellation.clone()) + .map(|operation| (operation.cancellation.clone(), operation.stop_gate.clone())) .collect(); - for cancellation in cancellations { + for (cancellation, stop_gate) in cancellations { + *stop_gate.lock().await = true; cancellation.cancel(); } } @@ -6466,6 +6499,10 @@ async fn process_chat_request( let conv = conversation.clone(); let cancel = cancel_token.clone(); let runtime_session_id = perm_session_key.clone(); + let stop_gate = active_chats + .stop_gate(&operation_id) + .await + .ok_or_else(|| anyhow::anyhow!("chat operation is no longer active"))?; let runtime_user_inputs = pending_user_inputs.clone(); let (steer_tx, steer_rx) = mpsc::unbounded_channel(); active_chats.install_steer(&operation_id, steer_tx).await; @@ -6476,6 +6513,7 @@ async fn process_chat_request( conv, runtime_event_tx, cancel, + stop_gate, runtime_cfg, permission_responders, if interactive_user_input { @@ -10491,6 +10529,7 @@ mod tests { State(state.clone()), Json(live_api::LivePermissionReq { decision: "allow_persist".into(), + runtime_instance_id: "stale-runtime".into(), generation: 1, request_id: 41, tool_name: Some(tool_name.into()), @@ -10521,6 +10560,7 @@ mod tests { let missing_session = serde_json::from_value::(serde_json::json!({ "decision": "allow", + "runtime_instance_id": "runtime-1", "generation": 1, "request_id": 41, })); @@ -10529,6 +10569,7 @@ mod tests { let missing_request = serde_json::from_value::(serde_json::json!({ "decision": "allow", + "runtime_instance_id": "runtime-1", "generation": 1, "session_id": "33333333-3333-4333-8333-333333333333", })); @@ -10537,10 +10578,20 @@ mod tests { let missing_generation = serde_json::from_value::(serde_json::json!({ "decision": "allow", + "runtime_instance_id": "runtime-1", "session_id": "33333333-3333-4333-8333-333333333333", "request_id": 41, })); assert!(missing_generation.is_err()); + + let missing_runtime_instance = + serde_json::from_value::(serde_json::json!({ + "decision": "allow", + "generation": 1, + "session_id": "33333333-3333-4333-8333-333333333333", + "request_id": 41, + })); + assert!(missing_runtime_instance.is_err()); } #[tokio::test(flavor = "current_thread")] @@ -10782,6 +10833,33 @@ mod tests { registry.complete(&first.operation_id).await; } + #[tokio::test] + async fn chat_stop_linearizes_after_inflight_permission_consumption() { + let registry = ActiveChatRegistry::default(); + let admission = registry.admit(Some("session-1"), None).await.unwrap(); + let stop_gate = registry + .stop_gate(&admission.operation_id) + .await + .expect("admitted operation has a stop gate"); + let gate_guard = stop_gate.lock().await; + + let registry_for_stop = registry.clone(); + let mut stop = tokio::spawn(async move { registry_for_stop.stop_alias("session-1").await }); + assert!( + tokio::time::timeout(Duration::from_millis(20), &mut stop) + .await + .is_err(), + "stop must wait while approval consumption owns the linearization gate" + ); + assert!(!admission.cancellation.is_cancelled()); + + drop(gate_guard); + assert!(stop.await.unwrap()); + assert!(admission.cancellation.is_cancelled()); + assert!(*stop_gate.lock().await); + registry.complete(&admission.operation_id).await; + } + #[tokio::test] async fn chat_panic_cleanup_removes_operation_and_allows_resubmit() { let active_chats = ActiveChatRegistry::default(); @@ -11399,6 +11477,7 @@ mod tests { assert_eq!(location.project_bucket, historical_bucket); let binding = crate::live_hub::LiveBinding { id: 1, + runtime_instance_id: "runtime-1".into(), generation: 1, session_id: "resumed-session".into(), working_dir, diff --git a/crates/jeikcode-daemon/src/live_api.rs b/crates/jeikcode-daemon/src/live_api.rs index 9b43e1225..5b4433e2d 100644 --- a/crates/jeikcode-daemon/src/live_api.rs +++ b/crates/jeikcode-daemon/src/live_api.rs @@ -701,6 +701,12 @@ enum ChatPermissionWaitOutcome { Closed, } +enum ChatPermissionDriverEvent { + Cancelled, + Response(ChatPermissionWaitOutcome), + ModePoll, +} + async fn await_chat_permission_response( rx: tokio::sync::oneshot::Receiver, request_timeout: Option, @@ -718,6 +724,47 @@ async fn await_chat_permission_response( } } +async fn next_chat_permission_driver_event( + cancel: &CancellationToken, + cancelled: bool, + response: std::pin::Pin<&mut F>, +) -> ChatPermissionDriverEvent +where + F: std::future::Future, +{ + tokio::select! { + // A user stop is fail-closed. If cancellation and an approval response + // are already ready on the same poll, cancellation must win rather than + // authorizing a tool nondeterministically. + biased; + _ = cancel.cancelled(), if !cancelled => ChatPermissionDriverEvent::Cancelled, + outcome = response => ChatPermissionDriverEvent::Response(outcome), + _ = tokio::time::sleep(std::time::Duration::from_millis(100)) => { + ChatPermissionDriverEvent::ModePoll + } + } +} + +async fn consume_chat_permission_if_running( + stop_gate: &Arc>, + cancel: &CancellationToken, + consume: F, +) -> Option +where + F: std::future::Future, +{ + // Hold the per-turn gate across runtime consumption. `/chat/stop` acquires + // the same gate before it marks the turn stopped and signals cancellation. + // Therefore exactly one side linearizes first: a stop that wins prevents + // the approval future from being polled; an approval that wins is consumed + // before the stop is allowed to become visible. + let stopped = stop_gate.lock().await; + if *stopped || cancel.is_cancelled() { + return None; + } + Some(consume.await) +} + async fn persist_chat_mcp_approval( project_dir: &Path, shared_registry: Option<&Arc>, @@ -893,6 +940,7 @@ pub(crate) async fn run_chat_turn_v2( conv: Arc>>, runtime_event_tx: mpsc::UnboundedSender, cancel: CancellationToken, + stop_gate: Arc>, mut runtime_cfg: jeikcode_coding::CodingRuntimeConfig, permission_responders: Option, user_input_responders: Option, @@ -1140,8 +1188,14 @@ pub(crate) async fn run_chat_turn_v2( await_chat_permission_response(rx, driver_request_timeout); tokio::pin!(response); loop { - tokio::select! { - _ = cancel.cancelled(), if !cancelled => { + match next_chat_permission_driver_event( + &cancel, + cancelled, + response.as_mut(), + ) + .await + { + ChatPermissionDriverEvent::Cancelled => { cancelled = true; let _ = handle.cancel().await; responders.expire(&session_id, &approval_id); @@ -1150,7 +1204,7 @@ pub(crate) async fn run_chat_turn_v2( ); break (PermissionDecision::Deny, false); } - outcome = &mut response => { + ChatPermissionDriverEvent::Response(outcome) => { match outcome { ChatPermissionWaitOutcome::Submission(envelope) => { // Do not acknowledge HTTP yet. Winning the bridge receive @@ -1178,7 +1232,7 @@ pub(crate) async fn run_chat_turn_v2( } } } - _ = tokio::time::sleep(std::time::Duration::from_millis(100)) => { + ChatPermissionDriverEvent::ModePoll => { if live_current_approval_mode() == ApprovalMode::Auto { let _ = handle.set_mode(jeikcode_coding::RuntimeMode::Auto).await; responders.unregister(&session_id, &approval_id); @@ -1199,13 +1253,32 @@ pub(crate) async fn run_chat_turn_v2( _ => ApprovalResponse::deny(), }; let value = serde_json::to_value(response).unwrap_or(serde_json::Value::Null); - match handle.respond(request.id, value).await { - Ok(()) => { + let runtime_response = consume_chat_permission_if_running( + &stop_gate, + &cancel, + handle.respond(request.id, value), + ) + .await; + match runtime_response { + None => { + cancelled = true; + let _ = handle.cancel().await; + if let Some(responders) = permission_responders.as_ref() { + responders.expire(&session_id, &approval_id); + } + jeikcode_capabilities::session::SessionManager::clear_pending_permission_any_project( + &session_id, + ); + persist_after_start.remove(&approval.call_id); + accepted_envelope.take(); + continue; + } + Some(Ok(())) => { if let Some(envelope) = accepted_envelope.take() { envelope.acknowledge(); } } - Err(_) => { + Some(Err(_)) => { persist_after_start.remove(&approval.call_id); // Dropping an unacknowledged envelope closes the HTTP ACK // channel, so `/chat/permission` reports failure instead of @@ -1449,6 +1522,7 @@ pub(crate) enum LiveWireEvent { PersistenceWarning { message: String }, #[serde(rename = "permission_request")] PermissionRequest { + runtime_instance_id: String, generation: u64, request_id: u64, tool_name: String, @@ -1519,11 +1593,12 @@ struct NativeLiveWireProjector { impl NativeLiveWireProjector { #[cfg(test)] fn project(&mut self, event: crate::live_hub::LiveViewEvent) -> Option { - self.project_at_generation(0, event) + self.project_at_generation(None, 0, event) } fn project_at_generation( &mut self, + runtime_instance_id: Option<&str>, generation: u64, event: crate::live_hub::LiveViewEvent, ) -> Option { @@ -1672,6 +1747,7 @@ impl NativeLiveWireProjector { if request.kind == APPROVAL_KIND { let approval: ApprovalRequest = serde_json::from_value(request.payload).ok()?; LiveWireEvent::PermissionRequest { + runtime_instance_id: runtime_instance_id?.to_string(), generation, request_id: request.id, tool_name: approval.tool, @@ -1937,6 +2013,7 @@ fn project_registry_event( sequenced: jeikcode_coding::session_runtime_registry::SequencedSessionEvent, ) -> Option { use jeikcode_coding::session_runtime_registry::SessionViewEvent; + let runtime_instance_id = sequenced.runtime_instance_id.clone(); let generation = sequenced.generation; if let Some(view) = sequenced.view { let live = match view { @@ -1963,11 +2040,14 @@ fn project_registry_event( crate::live_hub::LiveViewEvent::RequestResolved { request_id, kind } } }; - return projector.project_at_generation(generation, live); + return projector.project_at_generation(runtime_instance_id.as_deref(), generation, live); } sequenced.runtime.and_then(|runtime| { - projector - .project_at_generation(generation, crate::live_hub::LiveViewEvent::Runtime(runtime)) + projector.project_at_generation( + runtime_instance_id.as_deref(), + generation, + crate::live_hub::LiveViewEvent::Runtime(runtime), + ) }) } @@ -2145,9 +2225,13 @@ fn live_stream_from_hub_join(join: crate::live_hub::LiveJoin) -> axum::response: session_id: join.binding.session_id.clone(), ..Default::default() }; + let runtime_instance_id = join.binding.runtime_instance_id.clone(); for observation in join.replay { - if let Some(w) = projector.project_at_generation(observation.generation, observation.event) - { + if let Some(w) = projector.project_at_generation( + Some(&runtime_instance_id), + observation.generation, + observation.event, + ) { let _ = tx.send(w); } } @@ -2157,9 +2241,11 @@ fn live_stream_from_hub_join(join: crate::live_hub::LiveJoin) -> axum::response: loop { match rx.recv().await { Ok(observation) if observation.binding_id == binding_id => { - if let Some(w) = - projector.project_at_generation(observation.generation, observation.event) - { + if let Some(w) = projector.project_at_generation( + Some(&runtime_instance_id), + observation.generation, + observation.event, + ) { if tx.send(w).is_err() { break; } @@ -3238,6 +3324,8 @@ pub(crate) async fn live_reasoning_effort( #[derive(serde::Deserialize)] pub(crate) struct LivePermissionReq { pub decision: String, // "allow" | "deny" | "always_allow" | "allow_persist" + /// Exact concrete runtime owner that emitted the approval request. + pub runtime_instance_id: String, /// Exact runtime generation that emitted the approval request. pub generation: u64, /// Exact native runtime request id from the `permission_request` event. @@ -3264,7 +3352,8 @@ pub(crate) async fn live_permission( use jeikcode_capabilities::tools::{parse_permission_decision, PermissionDecision}; let session_id = req.session_id.trim(); - if session_id.is_empty() { + let runtime_instance_id = req.runtime_instance_id.trim(); + if session_id.is_empty() || runtime_instance_id.is_empty() { return Json(serde_json::json!({ "accepted": false })); } let decision = parse_permission_decision(&req.decision); @@ -3279,6 +3368,7 @@ pub(crate) async fn live_permission( let accepted = if crate::native_live::prefer_registry_live_stream(session_id) { crate::native_live::resolve_via_registry_confirmed( session_id, + runtime_instance_id, req.generation, req.request_id, value, @@ -3287,9 +3377,15 @@ pub(crate) async fn live_permission( .await .is_ok() } else if crate::native_live::live_execution_session_id().as_deref() == Some(session_id) { - crate::native_live::respond_confirmed_for_generation(req.generation, req.request_id, value) - .await - .is_ok() + crate::native_live::respond_confirmed_for_runtime( + runtime_instance_id, + req.generation, + req.request_id, + jeikcode_capabilities::tools::APPROVAL_KIND, + value, + ) + .await + .is_ok() } else { false }; @@ -4069,6 +4165,58 @@ mod tests { assert!(matches!(outcome, ChatPermissionWaitOutcome::TimedOut)); } + #[tokio::test] + async fn chat_permission_cancel_wins_when_submission_is_already_ready() { + let responders = crate::permission_bridge::PermissionResponders::new(); + let (tx, rx) = tokio::sync::oneshot::channel(); + responders.register("session-1".into(), "approval-1".into(), "bash".into(), tx); + let delivery = responders.deliver( + "session-1", + "approval-1", + crate::permission_bridge::PermissionSubmission { + decision: PermissionDecision::AllowOnce, + persist: false, + }, + ); + let crate::permission_bridge::PermissionDelivery::Submitted { accepted, .. } = delivery + else { + panic!("approval submission must be queued") + }; + + let cancel = CancellationToken::new(); + cancel.cancel(); + let event = { + let response = + await_chat_permission_response(rx, Some(std::time::Duration::from_secs(1))); + tokio::pin!(response); + next_chat_permission_driver_event(&cancel, false, response.as_mut()).await + }; + + assert!(matches!(event, ChatPermissionDriverEvent::Cancelled)); + assert!( + accepted.await.is_err(), + "a cancellation-winning approval race must not acknowledge HTTP success" + ); + } + + #[tokio::test] + async fn chat_stop_gate_prevents_runtime_consumption_after_stop_linearizes() { + let stop_gate = Arc::new(tokio::sync::Mutex::new(true)); + let cancel = CancellationToken::new(); + cancel.cancel(); + let consumed = Arc::new(std::sync::atomic::AtomicBool::new(false)); + let consumed_by_future = consumed.clone(); + + let result = consume_chat_permission_if_running(&stop_gate, &cancel, async move { + consumed_by_future.store(true, std::sync::atomic::Ordering::SeqCst); + 7usize + }) + .await; + + assert_eq!(result, None); + assert!(!consumed.load(std::sync::atomic::Ordering::SeqCst)); + } + #[test] fn interactive_chat_driver_disables_the_kernel_request_timeout() { let config = jeikcode_config::config::Config::default(); @@ -4390,12 +4538,14 @@ mod tests { }; let wire = projector .project_at_generation( + Some("runtime-1"), 9, crate::live_hub::LiveViewEvent::Runtime(CodingRuntimeEvent::Request(request)), ) .expect("approval request must reach the live wire"); let json = serde_json::to_value(wire).unwrap(); assert_eq!(json["type"], "permission_request"); + assert_eq!(json["runtime_instance_id"], "runtime-1"); assert_eq!(json["generation"], 9); assert_eq!(json["request_id"], 1); assert_eq!(json["call_id"], "call-1"); diff --git a/crates/jeikcode-daemon/src/live_hub.rs b/crates/jeikcode-daemon/src/live_hub.rs index cad0a08e8..bfc0b6e46 100644 --- a/crates/jeikcode-daemon/src/live_hub.rs +++ b/crates/jeikcode-daemon/src/live_hub.rs @@ -16,6 +16,9 @@ pub trait LiveRuntimeControl: Send + Sync { fn status(&self) -> RuntimeStatus; fn dispatch(&self, command: DriverCommand) -> Result<(), RuntimeUnavailable>; fn handle(&self) -> Option; + /// Authoritative identity of the concrete runtime owner. Controls that are + /// not yet backed by a runtime cannot be bound into the live hub. + fn runtime_instance_id(&self) -> Option; } impl LiveRuntimeControl for CodingRuntimeHandle { @@ -30,11 +33,19 @@ impl LiveRuntimeControl for CodingRuntimeHandle { fn handle(&self) -> Option { Some(self.clone()) } + + fn runtime_instance_id(&self) -> Option { + Some(CodingRuntimeHandle::instance_id(self).to_string()) + } } #[derive(Clone, Debug, PartialEq, Eq)] pub struct LiveBinding { pub id: u64, + /// Stable identity of the concrete runtime owner behind this binding. + /// Generations and kernel request ids are scoped to one runtime and may be + /// reused after replacement. + pub runtime_instance_id: String, pub generation: u64, pub session_id: String, pub working_dir: PathBuf, @@ -346,8 +357,12 @@ impl LiveViewHub { return Err(HubError::ActiveTurn); } state.next_binding_id += 1; + let runtime_instance_id = control + .runtime_instance_id() + .ok_or(HubError::RuntimeUnavailable)?; let identity = LiveBinding { id: state.next_binding_id, + runtime_instance_id, generation: status.generation, session_id: session_id.into(), working_dir, @@ -1144,6 +1159,76 @@ impl LiveViewHub { Ok(()) } + /// Confirm a response against the exact concrete runtime instance and + /// generation that emitted the request. Runtime generations restart when a + /// session is rebound, so generation + request id alone is not a durable + /// browser correlation key. + pub async fn respond_confirmed_for_runtime( + &self, + expected_runtime_instance_id: &str, + expected_generation: u64, + id: RequestId, + expected_kind: &str, + value: serde_json::Value, + ) -> Result<(), HubError> { + { + let state = self.state.lock().unwrap_or_else(|error| error.into_inner()); + let current = state.binding.as_ref().ok_or(HubError::Unbound)?; + if current.identity.runtime_instance_id != expected_runtime_instance_id { + return Err(HubError::StaleBinding); + } + if current.identity.generation != expected_generation { + return Err(HubError::RuntimeGenerationChanged { + expected: expected_generation, + actual: current.identity.generation, + }); + } + if state.pending_requests.get(&id).map(String::as_str) != Some(expected_kind) { + return Err(HubError::UnknownRequest(id)); + } + } + let (binding, handle) = self.bound_handle()?; + if binding.runtime_instance_id != expected_runtime_instance_id + || handle.instance_id() != expected_runtime_instance_id + { + return Err(HubError::StaleBinding); + } + if binding.generation != expected_generation { + return Err(HubError::RuntimeGenerationChanged { + expected: expected_generation, + actual: binding.generation, + }); + } + handle + .respond_for_generation( + jeikcode_coding::RuntimeGeneration(expected_generation), + id, + value, + ) + .await + .map_err(|error| HubError::RuntimeRejected(error.to_string()))?; + let mut state = self.state.lock().unwrap_or_else(|error| error.into_inner()); + let current = state.binding.as_ref().ok_or(HubError::Unbound)?; + if current.identity.id != binding.id + || current.identity.runtime_instance_id != expected_runtime_instance_id + { + return Err(HubError::StaleBinding); + } + if current.identity.generation != expected_generation { + return Err(HubError::RuntimeGenerationChanged { + expected: expected_generation, + actual: current.identity.generation, + }); + } + if let Some(kind) = state.pending_requests.get(&id) { + if kind != expected_kind { + return Err(HubError::UnknownRequest(id)); + } + self.resolve_request_locked(&mut state, id)?; + } + Ok(()) + } + pub fn respond_pending_kind( &self, kind: &str, @@ -1681,6 +1766,7 @@ mod tests { struct FakeControl { status: Arc>, commands: Arc>>, + runtime_instance_id: Option, } impl LiveRuntimeControl for FakeControl { @@ -1696,6 +1782,10 @@ mod tests { fn handle(&self) -> Option { None } + + fn runtime_instance_id(&self) -> Option { + self.runtime_instance_id.clone() + } } fn control() -> (Arc, Arc>>) { @@ -1707,6 +1797,7 @@ mod tests { phase: RuntimePhase::Ready, })), commands: commands.clone(), + runtime_instance_id: Some(uuid::Uuid::new_v4().simple().to_string()), }), commands, ) @@ -1720,6 +1811,7 @@ mod tests { fn same_generation_and_identity_is_a_noop_session_change() { let binding = LiveBinding { id: 1, + runtime_instance_id: "runtime-1".into(), generation: 7, session_id: "session-1".into(), working_dir: PathBuf::from("/project"), @@ -1973,6 +2065,52 @@ mod tests { ))); } + #[tokio::test] + async fn confirmed_runtime_response_rejects_stale_instance_before_reused_request_id() { + let hub = LiveViewHub::new(); + let (control, commands) = control(); + let binding = hub + .bind("session-1", PathBuf::from("/one"), snapshot("one"), control) + .unwrap(); + hub.publish( + &binding, + SequencedRuntimeEvent { + generation: 1, + sequence: 1, + event: CodingRuntimeEvent::Request(jeikcode_coding::RuntimeRequest { + id: 42, + kind: "approval".into(), + payload: serde_json::json!({}), + snapshot: None, + }), + }, + ) + .unwrap(); + + assert_eq!( + hub.respond_confirmed_for_runtime( + "stale-runtime", + 1, + 42, + "approval", + serde_json::Value::Null, + ) + .await + .unwrap_err(), + HubError::StaleBinding + ); + assert!(commands.lock().unwrap().is_empty()); + assert!(hub + .join() + .unwrap() + .replay + .iter() + .any(|observation| matches!( + &observation.event, + LiveViewEvent::Runtime(CodingRuntimeEvent::Request(request)) if request.id == 42 + ))); + } + #[test] fn terminal_snapshot_replaces_replay_atomically() { let hub = LiveViewHub::new(); @@ -2161,6 +2299,25 @@ mod tests { assert_eq!(error, HubError::ActiveTurn); } + #[test] + fn binding_requires_an_authoritative_runtime_instance_identity() { + let hub = LiveViewHub::new(); + let control = Arc::new(FakeControl { + status: Arc::new(Mutex::new(RuntimeStatus { + generation: 0, + phase: RuntimePhase::Ready, + })), + commands: Arc::new(Mutex::new(Vec::new())), + runtime_instance_id: None, + }); + + assert_eq!( + hub.bind("session-1", PathBuf::from("/one"), snapshot("old"), control,) + .unwrap_err(), + HubError::RuntimeUnavailable + ); + } + #[test] fn runtime_already_in_turn_cannot_be_bound_without_replay_state() { let hub = LiveViewHub::new(); @@ -2170,6 +2327,7 @@ mod tests { phase: RuntimePhase::InTurn, })), commands: Arc::new(Mutex::new(Vec::new())), + runtime_instance_id: Some(uuid::Uuid::new_v4().simple().to_string()), }); assert_eq!( diff --git a/crates/jeikcode-daemon/src/native_live.rs b/crates/jeikcode-daemon/src/native_live.rs index 6eb4dc3d3..f736e8744 100644 --- a/crates/jeikcode-daemon/src/native_live.rs +++ b/crates/jeikcode-daemon/src/native_live.rs @@ -397,7 +397,14 @@ pub async fn ensure_registry_runner( let reg = jeikcode_coding::session_runtime_registry::SessionRuntimeRegistry::global(); let _ = reg.open_or_attach(session_id.clone(), working_dir.clone()); - let _ = reg.bind_handle(&session_id, handle.clone(), None); + if !reg.bind_handle(&session_id, handle.clone(), None) { + let _ = handle.shutdown().await; + let _ = task.await; + return Err(format!( + "session {session_id} already owns another live runtime" + )); + } + let forward_runtime_instance_id = handle.instance_id().to_string(); // Cache the freshly bound provider identity so a follow-up `/live/message` // can detect a stale-bound request without reaching into runtime config. reg.set_provider_fingerprint(&session_id, Some(provider_fingerprint.clone())); @@ -419,7 +426,12 @@ pub async fn ensure_registry_runner( name, ); } - let _ = reg.push_runtime_event(&forward_id, envelope.generation, envelope.event); + let _ = reg.push_runtime_event_for_runtime( + &forward_id, + &forward_runtime_instance_id, + envelope.generation, + envelope.event, + ); } let _ = task.await; }); @@ -572,6 +584,7 @@ pub fn resolve_via_registry( /// already timed out or advanced to another pending request. pub async fn resolve_via_registry_confirmed( session_id: &str, + runtime_instance_id: &str, generation: u64, id: jeikcode_kernel::event::RequestId, value: serde_json::Value, @@ -579,25 +592,33 @@ pub async fn resolve_via_registry_confirmed( ) -> Result<(), String> { let reg = jeikcode_coding::session_runtime_registry::SessionRuntimeRegistry::global(); let key = session_id.to_string(); + if !reg.pending_request_matches(&key, runtime_instance_id, generation, id, kind) { + return Err(format!( + "session {session_id} no longer has the exact pending {kind} request" + )); + } let handle = reg - .handle(&key) - .ok_or_else(|| format!("session {session_id} has no live registry handle for response"))?; + .handle_for_runtime_instance(&key, runtime_instance_id) + .ok_or_else(|| { + format!("session {session_id} no longer owns runtime instance {runtime_instance_id}") + })?; handle .respond_for_generation(jeikcode_coding::RuntimeGeneration(generation), id, value) .await .map_err(|error| format!("registry response rejected: {error}"))?; - let working_dir = reg - .lookup(&key) - .map(|entry| entry.working_dir) - .unwrap_or_else(|| PathBuf::from(".")); - let _ = reg.ensure_and_push_view( - key, - working_dir, + if !reg.push_view_event_for_runtime( + &key, + runtime_instance_id, + generation, jeikcode_coding::session_runtime_registry::SessionViewEvent::RequestResolved { request_id: id, kind: kind.to_string(), }, - ); + ) { + return Err(format!( + "session {session_id} runtime changed before response confirmation" + )); + } Ok(()) } @@ -633,7 +654,16 @@ fn dual_write_runtime_event_to_registry( let reg = jeikcode_coding::session_runtime_registry::SessionRuntimeRegistry::global(); let _ = reg.open_or_attach(session_id.clone(), working_dir); if let Ok(handle) = hub().execution_handle() { - let _ = reg.bind_handle(&session_id, handle, None); + let runtime_instance_id = handle.instance_id().to_string(); + if reg.bind_handle(&session_id, handle, None) { + let _ = reg.push_runtime_event_for_runtime( + &session_id, + &runtime_instance_id, + generation, + event, + ); + } + return; } let _ = reg.push_runtime_event(&session_id, generation, event); } @@ -739,6 +769,18 @@ pub async fn respond_confirmed_for_generation( .await } +pub async fn respond_confirmed_for_runtime( + runtime_instance_id: &str, + generation: u64, + id: jeikcode_kernel::event::RequestId, + kind: &str, + value: serde_json::Value, +) -> Result<(), HubError> { + hub() + .respond_confirmed_for_runtime(runtime_instance_id, generation, id, kind, value) + .await +} + pub async fn respond_pending_kind_confirmed( kind: &str, value: serde_json::Value, diff --git a/crates/jeikcode-tuix/src/event_loop/mod.rs b/crates/jeikcode-tuix/src/event_loop/mod.rs index 15d9ddae5..fda812043 100644 --- a/crates/jeikcode-tuix/src/event_loop/mod.rs +++ b/crates/jeikcode-tuix/src/event_loop/mod.rs @@ -3270,6 +3270,19 @@ impl jeikcode_daemon::live_hub::LiveRuntimeControl for RuntimeControl { }, } } + + fn runtime_instance_id(&self) -> Option { + match self { + Self::Ready(ready) => Some(ready.handle.instance_id().to_string()), + Self::Deferred(deferred) => match &*deferred.state.borrow() { + jeikcode_coding::DeferredRuntimeState::Ready(handle) => { + Some(handle.instance_id().to_string()) + } + jeikcode_coding::DeferredRuntimeState::Starting + | jeikcode_coding::DeferredRuntimeState::Failed(_) => None, + }, + } + } } /// A newly spawned runtime endpoint and its single-consumer ordered event stream. @@ -19573,7 +19586,16 @@ fn publish_registry_runtime_event( ctx.bg_manager.handle_for_runtime(runtime_id) }; if let Some(handle) = handle { - let _ = reg.bind_handle(&session_id, handle, Some(runtime_id.as_u64())); + let runtime_instance_id = handle.instance_id().to_string(); + if reg.bind_handle(&session_id, handle, Some(runtime_id.as_u64())) { + let _ = reg.push_runtime_event_for_runtime( + &session_id, + &runtime_instance_id, + generation, + coding, + ); + } + return; } let _ = reg.push_runtime_event(&session_id, generation, coding); } diff --git a/webui/src/api.test.ts b/webui/src/api.test.ts index c1a1e7b03..049f581b9 100644 --- a/webui/src/api.test.ts +++ b/webui/src/api.test.ts @@ -145,18 +145,19 @@ test('postLivePermission carries exact session and native request identity', asy try { const { postLivePermission } = await import('./api.ts'); - await postLivePermission('allow', 'mcp__srv__query', 'session-1', 5, 73); + await postLivePermission('allow', 'mcp__srv__query', 'session-1', 'runtime-1', 5, 73); assert.equal(calls[0].url, '/live/permission'); assert.deepEqual(JSON.parse(String(calls[0].init?.body)), { decision: 'allow', tool_name: 'mcp__srv__query', session_id: 'session-1', + runtime_instance_id: 'runtime-1', generation: 5, request_id: 73, }); await assert.rejects( - () => postLivePermission('allow', 'mcp__srv__query', null, 5, 73), + () => postLivePermission('allow', 'mcp__srv__query', null, 'runtime-1', 5, 73), /missing live approval identity/i, ); await assert.rejects( @@ -164,7 +165,11 @@ test('postLivePermission carries exact session and native request identity', asy /missing live approval identity/i, ); await assert.rejects( - () => postLivePermission('allow', 'mcp__srv__query', 'session-1', undefined, 73), + () => postLivePermission('allow', 'mcp__srv__query', 'session-1', null, 5, 73), + /missing live approval identity/i, + ); + await assert.rejects( + () => postLivePermission('allow', 'mcp__srv__query', 'session-1', 'runtime-1', undefined, 73), /missing live approval identity/i, ); } finally { @@ -182,7 +187,7 @@ test('postLivePermission rejects when the runtime did not consume the approval', try { const { postLivePermission } = await import('./api.ts'); await assert.rejects( - () => postLivePermission('allow', 'write_file', 'session-1', 5, 73), + () => postLivePermission('allow', 'write_file', 'session-1', 'runtime-1', 5, 73), /did not accept permission/i, ); } finally { diff --git a/webui/src/api.ts b/webui/src/api.ts index 6c31e2709..d0c41bc9e 100644 --- a/webui/src/api.ts +++ b/webui/src/api.ts @@ -1417,7 +1417,7 @@ export type LiveWireEvent = | { type: 'warning'; message: string } | { type: 'persistence_warning'; message: string } | { type: 'rate_limited'; reset_at_display: string; reset_label: string; secs_until_reset: number | null; auto_resuming: boolean; server_message?: string | null } - | { type: 'permission_request'; session_id?: string; generation: number; request_id: number; tool_name: string; reason: string; call_id: string; arguments: string } + | { type: 'permission_request'; session_id?: string; runtime_instance_id: string; generation: number; request_id: number; tool_name: string; reason: string; call_id: string; arguments: string } | { type: 'user_input_request'; session_id?: string; request_id: number; header: string; question: string; mode: 'single' | 'multiple' | 'text'; options: { label: string; description?: string }[] } | { type: 'user_input_resolved'; request_id: number } | { type: 'steered'; count: number; inputs: { text: string; images: ImageData[] }[]; client_input_ids: Array } @@ -1673,10 +1673,11 @@ export async function postLivePermission( decision: 'allow' | 'deny' | 'always_allow' | 'allow_persist', toolName?: string, sessionId?: string | null, + runtimeInstanceId?: string | null, generation?: number, requestId?: number, ): Promise<{ accepted: boolean }> { - if (!sessionId || generation === undefined || requestId === undefined) { + if (!sessionId || !runtimeInstanceId || generation === undefined || requestId === undefined) { throw new Error('missing live approval identity'); } const resp = await apiFetch('/live/permission', { @@ -1686,6 +1687,7 @@ export async function postLivePermission( decision, tool_name: toolName, session_id: sessionId, + runtime_instance_id: runtimeInstanceId, generation, request_id: requestId, }), diff --git a/webui/src/app.tsx b/webui/src/app.tsx index fcfbe2c60..bf542ced5 100644 --- a/webui/src/app.tsx +++ b/webui/src/app.tsx @@ -65,7 +65,7 @@ export function App() { const [pending, setPending] = useState(null); const [liveReview, setLiveReview] = useState(null); const dismissLiveReview = useRef({ - permission: (_generation: number, _requestId: number, _callId: string) => {}, + permission: (_runtimeInstanceId: string, _generation: number, _requestId: number, _callId: string) => {}, userInput: () => {}, }); const onLiveReview = useCallback((review: LiveReviewState) => { @@ -73,6 +73,7 @@ export function App() { if ( prev && prev.sessionId === review.sessionId + && (prev.permission?.runtime_instance_id ?? '') === (review.permission?.runtime_instance_id ?? '') && (prev.permission?.generation ?? -1) === (review.permission?.generation ?? -1) && (prev.permission?.request_id ?? -1) === (review.permission?.request_id ?? -1) && (prev.userInput?.request_id ?? -1) === (review.userInput?.request_id ?? -1) @@ -83,7 +84,7 @@ export function App() { }); }, []); const onBindReviewDismiss = useCallback((fns: { - permission: (generation: number, requestId: number, callId: string) => void; + permission: (runtimeInstanceId: string, generation: number, requestId: number, callId: string) => void; userInput: () => void; }) => { dismissLiveReview.current = fns; @@ -1363,8 +1364,8 @@ export function App() { chatPermission={pending} activeSession={activeSession} onDismissChatPermission={() => setPending(null)} - onDismissLivePermission={(generation, requestId, callId) => - dismissLiveReview.current.permission(generation, requestId, callId)} + onDismissLivePermission={(runtimeInstanceId, generation, requestId, callId) => + dismissLiveReview.current.permission(runtimeInstanceId, generation, requestId, callId)} onDismissLiveUserInput={() => dismissLiveReview.current.userInput()} onFocusSession={(id) => { if (!id || id === sessionId) return; diff --git a/webui/src/components/Chat.tsx b/webui/src/components/Chat.tsx index f40557256..c7715cf30 100644 --- a/webui/src/components/Chat.tsx +++ b/webui/src/components/Chat.tsx @@ -676,12 +676,12 @@ interface ChatProps { /** 当前会话的审批 / 提问，交给右下角通知栈，而不是居中弹层。 */ onLiveReview?: (review: { sessionId: string | null; - permission: { generation: number; request_id: number; tool_name: string; reason: string; call_id: string; arguments: unknown } | null; + permission: { runtime_instance_id: string; generation: number; request_id: number; tool_name: string; reason: string; call_id: string; arguments: unknown } | null; userInput: UserInputRequestEvent | null; }) => void; /** 通知栈提交后清掉本会话的实时卡片。 */ onBindReviewDismiss?: (fns: { - permission: (generation: number, requestId: number, callId: string) => void; + permission: (runtimeInstanceId: string, generation: number, requestId: number, callId: string) => void; userInput: () => void; }) => void; } @@ -1280,14 +1280,20 @@ export function Chat({ const syncRef = useRef(false); // Pending live-session permission request (shown as PermissionCard, calls /live/permission). // Kept separate from the non-sync `onPermission` prop so the /chat path is untouched. - const [livePending, setLivePending] = useState<{ generation: number; request_id: number; tool_name: string; reason: string; call_id: string; arguments: string } | null>(null); + const [livePending, setLivePending] = useState<{ runtime_instance_id: string; generation: number; request_id: number; tool_name: string; reason: string; call_id: string; arguments: string } | null>(null); // Pending structured input from either transport. The event's optional session_id // selects `/chat/user-input`; live requests answer the bound `/live` runtime. const [userInputReq, setUserInputReq] = useState(null); useEffect(() => { onBindReviewDismiss?.({ - permission: (generation, requestId, callId) => - setLivePending((cur) => resolvePendingAfterDecision(cur, callId, requestId, generation)), + permission: (runtimeInstanceId, generation, requestId, callId) => + setLivePending((cur) => resolvePendingAfterDecision( + cur, + callId, + runtimeInstanceId, + requestId, + generation, + )), userInput: () => setUserInputReq(null), }); }, [onBindReviewDismiss]); @@ -3386,7 +3392,7 @@ export function Chat({ break; } updateToolInLastAssistant(e.call_id, { status: 'waiting_approval' }); - setLivePending({ generation: e.generation, request_id: e.request_id, tool_name: e.tool_name, reason: e.reason, call_id: e.call_id, arguments: e.arguments }); + setLivePending({ runtime_instance_id: e.runtime_instance_id, generation: e.generation, request_id: e.request_id, tool_name: e.tool_name, reason: e.reason, call_id: e.call_id, arguments: e.arguments }); const folder = (effectiveWorkingDir ?? '').split(/[\\/]/).filter((part) => part.length > 0).pop() ?? ''; const sessionName = activeSession?.name || folder || 'JeikCode'; const sid = e.session_id || activeIdRef.current; @@ -3394,7 +3400,7 @@ export function Chat({ title: t('notify.review.title'), body: t('notify.review.body', { session: sessionName, detail: e.tool_name }), sessionId: sid, - tag: `${sid}:review:${e.generation}:${e.request_id}`, + tag: `${sid}:review:${e.runtime_instance_id}:${e.generation}:${e.request_id}`, postSystemNotifyFn: postSystemNotify, }); break; diff --git a/webui/src/components/NotificationDock.tsx b/webui/src/components/NotificationDock.tsx index 32c44f983..f16641fda 100644 --- a/webui/src/components/NotificationDock.tsx +++ b/webui/src/components/NotificationDock.tsx @@ -44,6 +44,7 @@ import { UserInputCard } from './UserInputCard'; export interface LiveReviewState { sessionId: string | null; permission: { + runtime_instance_id: string; generation: number; request_id: number; tool_name: string; @@ -102,7 +103,7 @@ export function NotificationDock({ chatPermission: ChatPermission | null; activeSession?: { id: string; name: string; working_dir?: string } | null; onDismissChatPermission: () => void; - onDismissLivePermission: (generation: number, requestId: number, callId: string) => void; + onDismissLivePermission: (runtimeInstanceId: string, generation: number, requestId: number, callId: string) => void; onDismissLiveUserInput: () => void; onFocusSession: (sessionId: string) => void; }) { @@ -155,12 +156,18 @@ export function NotificationDock({ approvalId?: string, requestId?: number, generation?: number, + runtimeInstanceId?: string, ) { const idStr = String(callOrReqId); const identityKey = approvalId ? permissionInstanceKey(sessionId, { call_id: idStr, approval_id: approvalId }) : requestId !== undefined - ? permissionInstanceKey(sessionId, { call_id: idStr, request_id: requestId, generation }) + ? permissionInstanceKey(sessionId, { + call_id: idStr, + runtime_instance_id: runtimeInstanceId, + request_id: requestId, + generation, + }) : `${sessionId}:${idStr}`; setDismissedKeys((prev) => { const next = new Set(prev); @@ -390,6 +397,7 @@ export function NotificationDock({ key: string; sessionId: string; approval_id?: string; + runtime_instance_id?: string; generation?: number; request_id?: number; tool_name: string; @@ -688,6 +696,30 @@ export function NotificationDock({ arguments: currentCard.card.arguments, }} onDone={() => { + if (currentCard.card.live) { + const runtimeInstanceId = currentCard.card.runtime_instance_id; + const generation = currentCard.card.generation; + const requestId = currentCard.card.request_id; + if (!runtimeInstanceId || generation === undefined || requestId === undefined) { + return; + } + dismissCardLocally( + currentCard.card.key, + currentCard.card.sessionId, + currentCard.card.call_id, + undefined, + requestId, + generation, + runtimeInstanceId, + ); + onDismissLivePermission( + runtimeInstanceId, + generation, + requestId, + currentCard.card.call_id, + ); + return; + } dismissCardLocally( currentCard.card.key, currentCard.card.sessionId, @@ -695,19 +727,9 @@ export function NotificationDock({ currentCard.card.approval_id, currentCard.card.request_id, currentCard.card.generation, + currentCard.card.runtime_instance_id, ); if ( - currentCard.card.live - && currentCard.card.generation !== undefined - && currentCard.card.request_id !== undefined - ) { - onDismissLivePermission( - currentCard.card.generation, - currentCard.card.request_id, - currentCard.card.call_id, - ); - } - else if ( chatPerm && chatPerm.approval_id === currentCard.card.approval_id ) @@ -719,6 +741,7 @@ export function NotificationDock({ decision, toolName, currentCard.card.sessionId, + currentCard.card.runtime_instance_id, currentCard.card.generation, currentCard.card.request_id, ); diff --git a/webui/src/lib/pendingPermission.test.ts b/webui/src/lib/pendingPermission.test.ts new file mode 100644 index 000000000..5d911fde4 --- /dev/null +++ b/webui/src/lib/pendingPermission.test.ts @@ -0,0 +1,31 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; +import { resolvePendingAfterDecision } from './pendingPermission.ts'; + +test('old live decision cannot clear a replacement runtime permission card', () => { + const current = { + call_id: 'call-reused', + runtime_instance_id: 'runtime-new', + generation: 0, + request_id: 1, + }; + + assert.equal( + resolvePendingAfterDecision(current, 'call-reused', 'runtime-old', 1, 0), + current, + ); +}); + +test('strong live permission requires the complete runtime identity to clear', () => { + const current = { + call_id: 'call-reused', + runtime_instance_id: 'runtime-new', + generation: 0, + request_id: 1, + }; + + assert.equal(resolvePendingAfterDecision(current, 'call-reused', undefined, 1, 0), current); + assert.equal(resolvePendingAfterDecision(current, 'call-reused', 'runtime-new', 1, undefined), current); + assert.equal(resolvePendingAfterDecision(current, 'call-reused', 'runtime-new', undefined, 0), current); + assert.equal(resolvePendingAfterDecision(current, 'call-reused', 'runtime-new', 1, 0), null); +}); diff --git a/webui/src/lib/pendingPermission.ts b/webui/src/lib/pendingPermission.ts index c011a7f8e..591314caf 100644 --- a/webui/src/lib/pendingPermission.ts +++ b/webui/src/lib/pendingPermission.ts @@ -16,6 +16,7 @@ export interface PendingLike { call_id: string; + runtime_instance_id?: string; request_id?: number; generation?: number; } @@ -23,9 +24,26 @@ export interface PendingLike { export function resolvePendingAfterDecision( current: T | null, decidedCallId: string, + decidedRuntimeInstanceId?: string, decidedRequestId?: number, decidedGeneration?: number, ): T | null { + if (current?.runtime_instance_id !== undefined) { + if ( + decidedRuntimeInstanceId === undefined + || decidedRequestId === undefined + || decidedGeneration === undefined + || current.request_id === undefined + || current.generation === undefined + ) { + return current; + } + return current.runtime_instance_id === decidedRuntimeInstanceId + && current.generation === decidedGeneration + && current.request_id === decidedRequestId + ? null + : current; + } if ( current && decidedRequestId !== undefined diff --git a/webui/src/lib/sessionNotify.test.ts b/webui/src/lib/sessionNotify.test.ts index 36bd979de..726d28da7 100644 --- a/webui/src/lib/sessionNotify.test.ts +++ b/webui/src/lib/sessionNotify.test.ts @@ -50,6 +50,47 @@ test('live permission identity distinguishes reused request ids across runtime g assert.equal(samePermissionInstance(first, second), false); }); +test('live permission identity distinguishes reused generation and request id across runtime owners', () => { + const first = { + call_id: 'call-reused', + runtime_instance_id: 'runtime-old', + request_id: 1, + generation: 0, + }; + const second = { + call_id: 'call-reused', + runtime_instance_id: 'runtime-new', + request_id: 1, + generation: 0, + }; + assert.notEqual( + permissionInstanceKey('session-1', first), + permissionInstanceKey('session-1', second), + ); + assert.equal(samePermissionInstance(first, second), false); +}); + +test('strong live runtime identity never collapses into an instance-less legacy card', () => { + const strong = { + call_id: 'call-reused', + runtime_instance_id: 'runtime-new', + request_id: 1, + generation: 0, + }; + const legacy = { + call_id: 'call-reused', + request_id: 1, + generation: 0, + }; + + assert.notEqual( + permissionInstanceKey('session-1', strong), + permissionInstanceKey('session-1', legacy), + ); + assert.equal(samePermissionInstance(strong, legacy), false); + assert.equal(samePermissionInstance(legacy, strong), false); +}); + test('strong chat and live identities never collapse merely because call id is reused', () => { const chat = { call_id: 'ollama_call_0', approval_id: 'approval-old' }; const live = { call_id: 'ollama_call_0', request_id: 73 }; diff --git a/webui/src/lib/sessionNotify.ts b/webui/src/lib/sessionNotify.ts index e16926e55..87bb60e91 100644 --- a/webui/src/lib/sessionNotify.ts +++ b/webui/src/lib/sessionNotify.ts @@ -232,6 +232,7 @@ export interface SystemNotificationOptions { export interface PermissionNoticeIdentity { call_id: string; approval_id?: string | null; + runtime_instance_id?: string | null; request_id?: number | null; generation?: number | null; } @@ -242,6 +243,15 @@ export function permissionInstanceKey( ): string { const approvalId = permission.approval_id?.trim(); if (approvalId) return `${sessionId}:approval:${approvalId}`; + const runtimeInstanceId = permission.runtime_instance_id?.trim(); + if (runtimeInstanceId) { + const generation = permission.generation; + const requestId = permission.request_id; + if (generation === undefined || generation === null || requestId === undefined || requestId === null) { + return `${sessionId}:runtime:${runtimeInstanceId}:incomplete:${permission.call_id}`; + } + return `${sessionId}:runtime:${runtimeInstanceId}:generation:${generation}:request:${requestId}`; + } if (permission.request_id !== undefined && permission.request_id !== null) { const generation = permission.generation; return generation !== undefined && generation !== null @@ -261,6 +271,25 @@ export function samePermissionInstance( if (aApproval || bApproval) { return Boolean(aApproval && bApproval && aApproval === bApproval); } + const aRuntime = a.runtime_instance_id?.trim(); + const bRuntime = b.runtime_instance_id?.trim(); + if (aRuntime || bRuntime) { + return Boolean( + aRuntime + && bRuntime + && aRuntime === bRuntime + && a.generation !== undefined + && a.generation !== null + && b.generation !== undefined + && b.generation !== null + && a.generation === b.generation + && a.request_id !== undefined + && a.request_id !== null + && b.request_id !== undefined + && b.request_id !== null + && a.request_id === b.request_id + ); + } const aRequest = a.request_id; const bRequest = b.request_id; if (aRequest !== undefined && aRequest !== null || bRequest !== undefined && bRequest !== null) { From 413423c01a54fa21c73273ea8e21b02d5c711992 Mon Sep 17 00:00:00 2001 From: xuan2261 Date: Tue, 6 Oct 2026 14:40:02 +0700 Subject: [PATCH 4/4] test(tuix): carry live runtime identity Co-Authored-By: JeikCode --- crates/jeikcode-tuix/src/event_loop/commands.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/jeikcode-tuix/src/event_loop/commands.rs b/crates/jeikcode-tuix/src/event_loop/commands.rs index 804a402e0..95a9cff5e 100644 --- a/crates/jeikcode-tuix/src/event_loop/commands.rs +++ b/crates/jeikcode-tuix/src/event_loop/commands.rs @@ -794,6 +794,7 @@ mod bg_live_guard_tests { fn failed_live_unbind_preserves_the_local_guard_binding() { let original = jeikcode_daemon::live_hub::LiveBinding { id: 7, + runtime_instance_id: "runtime-1".into(), generation: 3, session_id: "session".into(), working_dir: PathBuf::from("/project"),