From 38cb07df3176b599d95449b9066a26bda2fd944a Mon Sep 17 00:00:00 2001 From: wsp Date: Sat, 18 Jul 2026 12:42:12 +0800 Subject: [PATCH 01/34] refactor(permission): remove confirmation-time input mutation - Remove updated_input from tool confirmation requests and platform APIs - Delete backend task-argument mutation during tool confirmation - Stop Web UI and HarmonyOS clients from forwarding or rewriting tool input - Preserve existing confirm and reject behavior across all product surfaces - Update serialization, runtime, remote-connect, desktop, and frontend tests --- src/apps/cli/src/agent/core_adapter.rs | 7 +------ src/apps/cli/src/agent/mod.rs | 6 +----- src/apps/cli/src/modes/chat/input.rs | 5 ++--- src/apps/cli/src/modes/chat/run.rs | 2 +- src/apps/cli/src/modes/exec.rs | 3 +-- src/apps/cli/src/peer_host/commands/dialog.rs | 2 -- src/apps/desktop/src/api/agentic_api.rs | 9 +-------- src/apps/desktop/src/api/tool_api.rs | 2 -- .../entry/src/main/ets/model/RemoteModels.ets | 1 - .../entry/src/main/ets/pages/Index.ets | 8 ++++---- .../pages/components/ChatMessageBubble.ets | 6 +++--- .../ets/pages/components/ChatTimeline.ets | 6 +++--- .../main/ets/pages/components/ChatView.ets | 6 +++--- .../ets/pages/components/ToolStatusList.ets | 2 +- .../ets/services/RemoteCommandFactory.ets | 8 ++------ .../ets/services/RemoteSessionManager.ets | 4 ++-- .../entry/src/test/LocalUnit.test.ets | 7 +------ src/apps/server/src/rpc_dispatcher.rs | 3 +-- .../src/agentic/coordination/coordinator.rs | 13 +++--------- .../agentic/tools/pipeline/state_manager.rs | 13 ------------ .../agentic/tools/pipeline/tool_pipeline.rs | 12 +---------- .../core/src/agentic/tools/pipeline/types.rs | 6 ------ .../core/src/service_agent_runtime.rs | 8 ++------ .../execution/agent-runtime/src/runtime.rs | 4 +--- .../tests/interaction_response_contracts.rs | 4 ---- .../interfaces/acp/src/runtime/prompt.rs | 1 - .../src/remote_connect.rs | 20 ++++--------------- .../tests/remote_connect_contracts.rs | 1 - .../flow_chat/components/FlowItemRenderer.tsx | 2 +- .../src/flow_chat/components/FlowToolCard.tsx | 8 +++----- .../components/ToolApprovalBar.test.tsx | 2 +- .../flow_chat/components/ToolApprovalBar.tsx | 5 ++--- .../modern/ExploreGroupRenderer.tsx | 4 ++-- .../components/modern/FlowChatContext.tsx | 2 +- .../components/modern/ModelRoundItem.tsx | 4 ++-- .../modern/useFlowChatToolActions.ts | 19 ------------------ src/web-ui/src/flow_chat/hooks/useFlowChat.ts | 2 +- .../tool-cards/AcpPermissionActions.tsx | 6 ++---- src/web-ui/src/flow_chat/types/flow-chat.ts | 2 +- .../utils/toolInvocationIdentity.test.ts | 16 --------------- .../flow_chat/utils/toolInvocationIdentity.ts | 16 --------------- .../infrastructure/api/service-api/ToolAPI.ts | 3 +-- .../src/shared/services/agent-service.ts | 2 -- 43 files changed, 55 insertions(+), 207 deletions(-) diff --git a/src/apps/cli/src/agent/core_adapter.rs b/src/apps/cli/src/agent/core_adapter.rs index 740353c562..3c490d20c6 100644 --- a/src/apps/cli/src/agent/core_adapter.rs +++ b/src/apps/cli/src/agent/core_adapter.rs @@ -497,16 +497,11 @@ impl Agent for CoreAgentAdapter { Ok(()) } - async fn confirm_tool( - &self, - tool_id: &str, - updated_input: Option, - ) -> Result<()> { + async fn confirm_tool(&self, tool_id: &str) -> Result<()> { tracing::info!("Confirming tool execution: {}", tool_id); self.runtime .confirm_tool(AgentToolConfirmationRequest { tool_id: tool_id.to_string(), - updated_input, }) .await .map_err(|e| anyhow::anyhow!("Confirm tool failed: {}", e.into_message())) diff --git a/src/apps/cli/src/agent/mod.rs b/src/apps/cli/src/agent/mod.rs index 7ce5b44063..c837092a21 100644 --- a/src/apps/cli/src/agent/mod.rs +++ b/src/apps/cli/src/agent/mod.rs @@ -29,11 +29,7 @@ pub(crate) trait Agent: Send + Sync { async fn restore_session(&self, session_id: &str) -> Result<()>; /// Confirm tool execution (allow once) - async fn confirm_tool( - &self, - tool_id: &str, - updated_input: Option, - ) -> Result<()>; + async fn confirm_tool(&self, tool_id: &str) -> Result<()>; /// Reject tool execution async fn reject_tool(&self, tool_id: &str, reason: String) -> Result<()>; diff --git a/src/apps/cli/src/modes/chat/input.rs b/src/apps/cli/src/modes/chat/input.rs index e6fb3fbda1..23f5a0bdad 100644 --- a/src/apps/cli/src/modes/chat/input.rs +++ b/src/apps/cli/src/modes/chat/input.rs @@ -25,7 +25,7 @@ impl ChatMode { let agent = self.agent.clone(); tracing::info!("User allowed tool once: {}", tool_id); match tokio::task::block_in_place(|| { - rt_handle.block_on(agent.confirm_tool(&tool_id, None)) + rt_handle.block_on(agent.confirm_tool(&tool_id)) }) { Ok(()) => { chat_state.permission_prompt = None; @@ -48,7 +48,7 @@ impl ChatMode { tool_name ); match tokio::task::block_in_place(|| { - rt_handle.block_on(agent.confirm_tool(&tool_id, None)) + rt_handle.block_on(agent.confirm_tool(&tool_id)) }) { Ok(()) => { self.runtime.approval_controller().allow_always(&tool_name); @@ -612,5 +612,4 @@ impl ChatMode { } Ok(outcome) } - } diff --git a/src/apps/cli/src/modes/chat/run.rs b/src/apps/cli/src/modes/chat/run.rs index 23cccb0ea4..77912420a0 100644 --- a/src/apps/cli/src/modes/chat/run.rs +++ b/src/apps/cli/src/modes/chat/run.rs @@ -455,7 +455,7 @@ impl ChatMode { let agent = self.agent.clone(); let tool_id = identity.tool_id.clone(); match tokio::task::block_in_place(|| { - rt_handle.block_on(agent.confirm_tool(&tool_id, None)) + rt_handle.block_on(agent.confirm_tool(&tool_id)) }) { Ok(()) => continue, Err(error) => tracing::error!( diff --git a/src/apps/cli/src/modes/exec.rs b/src/apps/cli/src/modes/exec.rs index 2f78372989..b99a65a3c5 100644 --- a/src/apps/cli/src/modes/exec.rs +++ b/src/apps/cli/src/modes/exec.rs @@ -857,8 +857,7 @@ impl ExecMode { terminal_outcome = Some(Err(anyhow::anyhow!(message))); break; } else { - if let Err(error) = self.agent.confirm_tool(tool_id, None).await - { + if let Err(error) = self.agent.confirm_tool(tool_id).await { let mut message = format!( "Failed to approve tool request for {tool_name}: {error}" ); diff --git a/src/apps/cli/src/peer_host/commands/dialog.rs b/src/apps/cli/src/peer_host/commands/dialog.rs index b847e8f4d9..a64dd27fd2 100644 --- a/src/apps/cli/src/peer_host/commands/dialog.rs +++ b/src/apps/cli/src/peer_host/commands/dialog.rs @@ -175,12 +175,10 @@ pub(crate) async fn confirm_tool_execution( state.turns.restore_confirmation(tool_id, ownership); return Err("Tool confirmation session or turn does not match its Peer owner".to_string()); } - let updated_input = request.get("updatedInput").cloned(); if let Err(error) = state .agent_runtime .confirm_tool(AgentToolConfirmationRequest { tool_id: tool_id.clone(), - updated_input, }) .await { diff --git a/src/apps/desktop/src/api/agentic_api.rs b/src/apps/desktop/src/api/agentic_api.rs index baa64a33ba..84bcdabd87 100644 --- a/src/apps/desktop/src/api/agentic_api.rs +++ b/src/apps/desktop/src/api/agentic_api.rs @@ -713,7 +713,6 @@ pub struct ListSessionsRequest { pub struct ConfirmToolRequest { pub session_id: String, pub tool_id: String, - pub updated_input: Option, } #[derive(Debug, Deserialize)] @@ -2312,7 +2311,6 @@ pub async fn confirm_tool_execution( .agent_runtime() .confirm_tool(AgentToolConfirmationRequest { tool_id: request.tool_id, - updated_input: request.updated_input, }) .await .map_err(|error| format!("Confirm tool failed: {}", error.into_message())) @@ -2604,8 +2602,7 @@ mod tests { .expect("cancel request"); let confirm: ConfirmToolRequest = serde_json::from_value(json!({ "sessionId": "session-1", - "toolId": "tool-1", - "updatedInput": { "path": "updated.txt" } + "toolId": "tool-1" })) .expect("confirm request"); let reject: RejectToolRequest = serde_json::from_value(json!({ @@ -2618,10 +2615,6 @@ mod tests { assert_eq!(cancel.session_id, "session-1"); assert_eq!(cancel.dialog_turn_id, "turn-1"); assert_eq!(confirm.tool_id, "tool-1"); - assert_eq!( - confirm.updated_input, - Some(json!({ "path": "updated.txt" })) - ); assert_eq!(reject.reason.as_deref(), Some("Use a read-only path")); assert_eq!( serde_json::to_value(StartDialogTurnResponse { diff --git a/src/apps/desktop/src/api/tool_api.rs b/src/apps/desktop/src/api/tool_api.rs index 5ab103c701..4815e6ef26 100644 --- a/src/apps/desktop/src/api/tool_api.rs +++ b/src/apps/desktop/src/api/tool_api.rs @@ -105,8 +105,6 @@ pub struct ToolConfirmationRequest { pub action: String, #[serde(alias = "task_id")] pub task_id: Option, - #[serde(alias = "updated_input")] - pub updated_input: Option, } #[derive(Debug, Clone, Serialize, Deserialize)] diff --git a/src/apps/mobile/harmonyos/entry/src/main/ets/model/RemoteModels.ets b/src/apps/mobile/harmonyos/entry/src/main/ets/model/RemoteModels.ets index 690417f62d..b80b32d0b4 100644 --- a/src/apps/mobile/harmonyos/entry/src/main/ets/model/RemoteModels.ets +++ b/src/apps/mobile/harmonyos/entry/src/main/ets/model/RemoteModels.ets @@ -117,7 +117,6 @@ export interface RemoteCommand { model_id?: string; reason?: string; answers?: Object; - updated_input?: Object; image_contexts?: RemoteImageContext[]; images?: ImageAttachment[]; } diff --git a/src/apps/mobile/harmonyos/entry/src/main/ets/pages/Index.ets b/src/apps/mobile/harmonyos/entry/src/main/ets/pages/Index.ets index 2e08152a59..a5b5927668 100644 --- a/src/apps/mobile/harmonyos/entry/src/main/ets/pages/Index.ets +++ b/src/apps/mobile/harmonyos/entry/src/main/ets/pages/Index.ets @@ -319,8 +319,8 @@ struct Index { onLoadOlder: () => { this.loadOlderMessages(); }, - onApproveTool: (toolId: string, updatedInput?: Object) => { - this.approveTool(toolId, updatedInput); + onApproveTool: (toolId: string) => { + this.approveTool(toolId); }, onRejectTool: (toolId: string) => { this.rejectTool(toolId); @@ -1577,9 +1577,9 @@ struct Index { this.sendChatMessage(); } - private async approveTool(toolId: string, updatedInput?: Object): Promise { + private async approveTool(toolId: string): Promise { await this.runToolAction(toolId, RemoteI18n.t('status.approvingTool'), RemoteI18n.t('status.toolApproved'), () => { - return this.sessionManager.confirmTool(toolId, updatedInput); + return this.sessionManager.confirmTool(toolId); }); } diff --git a/src/apps/mobile/harmonyos/entry/src/main/ets/pages/components/ChatMessageBubble.ets b/src/apps/mobile/harmonyos/entry/src/main/ets/pages/components/ChatMessageBubble.ets index bebe727b26..603aebbbd7 100644 --- a/src/apps/mobile/harmonyos/entry/src/main/ets/pages/components/ChatMessageBubble.ets +++ b/src/apps/mobile/harmonyos/entry/src/main/ets/pages/components/ChatMessageBubble.ets @@ -47,7 +47,7 @@ export struct ChatMessageBubble { @Prop downloadingFilePath: string = ''; @Prop downloadedFilePath: string = ''; @Prop fileDownloadStatus: string = ''; - onApproveTool: (toolId: string, updatedInput?: Object) => void = (_toolId: string, _updatedInput?: Object) => {}; + onApproveTool: (toolId: string) => void = (_toolId: string) => {}; onRejectTool: (toolId: string) => void = (_toolId: string) => {}; onCancelTool: (toolId: string) => void = (_toolId: string) => {}; onAnswerQuestion: (toolId: string, answers: RemoteQuestionAnswerPayload) => void = @@ -348,8 +348,8 @@ export struct ChatMessageBubble { ToolStatusList({ tools, isBusy: this.isBusy, - onApproveTool: (toolId: string, updatedInput?: Object) => { - this.onApproveTool(toolId, updatedInput); + onApproveTool: (toolId: string) => { + this.onApproveTool(toolId); }, onRejectTool: (toolId: string) => { this.onRejectTool(toolId); diff --git a/src/apps/mobile/harmonyos/entry/src/main/ets/pages/components/ChatTimeline.ets b/src/apps/mobile/harmonyos/entry/src/main/ets/pages/components/ChatTimeline.ets index 8e402b7f31..90d307b1fe 100644 --- a/src/apps/mobile/harmonyos/entry/src/main/ets/pages/components/ChatTimeline.ets +++ b/src/apps/mobile/harmonyos/entry/src/main/ets/pages/components/ChatTimeline.ets @@ -15,7 +15,7 @@ export struct ChatTimeline { @Prop downloadedFilePath: string = ''; @Prop fileDownloadStatus: string = ''; onLoadOlder: () => void = () => {}; - onApproveTool: (toolId: string, updatedInput?: Object) => void = (_toolId: string, _updatedInput?: Object) => {}; + onApproveTool: (toolId: string) => void = (_toolId: string) => {}; onRejectTool: (toolId: string) => void = (_toolId: string) => {}; onCancelTool: (toolId: string) => void = (_toolId: string) => {}; onAnswerQuestion: (toolId: string, answers: RemoteQuestionAnswerPayload) => void = @@ -99,8 +99,8 @@ export struct ChatTimeline { downloadingFilePath: this.downloadingFilePath, downloadedFilePath: this.downloadedFilePath, fileDownloadStatus: this.fileDownloadStatus, - onApproveTool: (toolId: string, updatedInput?: Object) => { - this.onApproveTool(toolId, updatedInput); + onApproveTool: (toolId: string) => { + this.onApproveTool(toolId); }, onRejectTool: (toolId: string) => { this.onRejectTool(toolId); diff --git a/src/apps/mobile/harmonyos/entry/src/main/ets/pages/components/ChatView.ets b/src/apps/mobile/harmonyos/entry/src/main/ets/pages/components/ChatView.ets index ea0374a39b..ade23b2e06 100644 --- a/src/apps/mobile/harmonyos/entry/src/main/ets/pages/components/ChatView.ets +++ b/src/apps/mobile/harmonyos/entry/src/main/ets/pages/components/ChatView.ets @@ -39,7 +39,7 @@ export struct ChatView { onBack: () => void = () => {}; onStop: () => void = () => {}; onLoadOlder: () => void = () => {}; - onApproveTool: (toolId: string, updatedInput?: Object) => void = (_toolId: string, _updatedInput?: Object) => {}; + onApproveTool: (toolId: string) => void = (_toolId: string) => {}; onRejectTool: (toolId: string) => void = (_toolId: string) => {}; onCancelTool: (toolId: string) => void = (_toolId: string) => {}; onAnswerQuestion: (toolId: string, answers: RemoteQuestionAnswerPayload) => void = @@ -123,8 +123,8 @@ export struct ChatView { onLoadOlder: () => { this.onLoadOlder(); }, - onApproveTool: (toolId: string, updatedInput?: Object) => { - this.onApproveTool(toolId, updatedInput); + onApproveTool: (toolId: string) => { + this.onApproveTool(toolId); }, onRejectTool: (toolId: string) => { this.onRejectTool(toolId); diff --git a/src/apps/mobile/harmonyos/entry/src/main/ets/pages/components/ToolStatusList.ets b/src/apps/mobile/harmonyos/entry/src/main/ets/pages/components/ToolStatusList.ets index f911ddad4f..8151330292 100644 --- a/src/apps/mobile/harmonyos/entry/src/main/ets/pages/components/ToolStatusList.ets +++ b/src/apps/mobile/harmonyos/entry/src/main/ets/pages/components/ToolStatusList.ets @@ -53,7 +53,7 @@ interface ToolRenderEntry { export struct ToolStatusList { @Prop tools: RemoteToolStatusResponse[] = []; @Prop isBusy: boolean = false; - onApproveTool: (toolId: string, updatedInput?: Object) => void = (_toolId: string, _updatedInput?: Object) => {}; + onApproveTool: (toolId: string) => void = (_toolId: string) => {}; onRejectTool: (toolId: string) => void = (_toolId: string) => {}; onCancelTool: (toolId: string) => void = (_toolId: string) => {}; onAnswerQuestion: (toolId: string, answers: RemoteQuestionAnswerPayload) => void = diff --git a/src/apps/mobile/harmonyos/entry/src/main/ets/services/RemoteCommandFactory.ets b/src/apps/mobile/harmonyos/entry/src/main/ets/services/RemoteCommandFactory.ets index 450f5ea133..457ef0bf83 100644 --- a/src/apps/mobile/harmonyos/entry/src/main/ets/services/RemoteCommandFactory.ets +++ b/src/apps/mobile/harmonyos/entry/src/main/ets/services/RemoteCommandFactory.ets @@ -129,15 +129,11 @@ export class RemoteCommandFactory { }; } - static confirmTool(toolId: string, updatedInput?: Object): RemoteCommand { - const command: RemoteCommand = { + static confirmTool(toolId: string): RemoteCommand { + return { cmd: 'confirm_tool', tool_id: toolId }; - if (updatedInput) { - command.updated_input = updatedInput; - } - return command; } static rejectTool(toolId: string, reason: string): RemoteCommand { diff --git a/src/apps/mobile/harmonyos/entry/src/main/ets/services/RemoteSessionManager.ets b/src/apps/mobile/harmonyos/entry/src/main/ets/services/RemoteSessionManager.ets index ae0d2ae2a1..a5411ee345 100644 --- a/src/apps/mobile/harmonyos/entry/src/main/ets/services/RemoteSessionManager.ets +++ b/src/apps/mobile/harmonyos/entry/src/main/ets/services/RemoteSessionManager.ets @@ -207,8 +207,8 @@ export class RemoteSessionManager { await this.send(RemoteCommandFactory.deleteSession(sessionId)); } - async confirmTool(toolId: string, updatedInput?: Object): Promise { - await this.send(RemoteCommandFactory.confirmTool(toolId, updatedInput)); + async confirmTool(toolId: string): Promise { + await this.send(RemoteCommandFactory.confirmTool(toolId)); } async rejectTool(toolId: string, reason: string): Promise { diff --git a/src/apps/mobile/harmonyos/entry/src/test/LocalUnit.test.ets b/src/apps/mobile/harmonyos/entry/src/test/LocalUnit.test.ets index ee34ca66be..0674edf3c3 100644 --- a/src/apps/mobile/harmonyos/entry/src/test/LocalUnit.test.ets +++ b/src/apps/mobile/harmonyos/entry/src/test/LocalUnit.test.ets @@ -14,10 +14,6 @@ import { TimeFormat } from '../main/ets/services/TimeFormat'; import { X25519 } from '../main/ets/services/X25519'; import { ChatMessage, ImageAttachment, PollSessionResult, RemoteQuestionAnswerPayload } from '../main/ets/model/RemoteModels'; -interface ToolInputFixture { - safe: boolean; -} - interface CryptoImageFixture { id: string; data_url: string; @@ -338,10 +334,9 @@ export default function localUnitTest() { }); it('builds tool and file commands', 0, () => { - const toolInput: ToolInputFixture = { safe: true }; const answers: RemoteQuestionAnswerPayload = { answer: 'yes', '0': 'yes' }; - expectCommandJson(RemoteCommandFactory.confirmTool('tool-1', toolInput), '{"cmd":"confirm_tool","tool_id":"tool-1","updated_input":{"safe":true}}'); + expectCommandJson(RemoteCommandFactory.confirmTool('tool-1'), '{"cmd":"confirm_tool","tool_id":"tool-1"}'); expectCommandJson(RemoteCommandFactory.rejectTool('tool-1', 'no'), '{"cmd":"reject_tool","tool_id":"tool-1","reason":"no"}'); expectCommandJson(RemoteCommandFactory.cancelTool('tool-1', 'stop'), '{"cmd":"cancel_tool","tool_id":"tool-1","reason":"stop"}'); expectCommandJson(RemoteCommandFactory.answerQuestion('tool-2', answers), '{"cmd":"answer_question","tool_id":"tool-2","answers":{"0":"yes","answer":"yes"}}'); diff --git a/src/apps/server/src/rpc_dispatcher.rs b/src/apps/server/src/rpc_dispatcher.rs index bafe7a5aee..a393d93621 100644 --- a/src/apps/server/src/rpc_dispatcher.rs +++ b/src/apps/server/src/rpc_dispatcher.rs @@ -512,10 +512,9 @@ pub async fn dispatch( "confirm_tool_execution" => { let request = extract_request(¶ms)?; let tool_id = get_string(&request, "toolId")?; - let updated_input = request.get("updatedInput").cloned(); state .coordinator - .confirm_tool(&tool_id, updated_input) + .confirm_tool(&tool_id) .await .map_err(|e| anyhow!("{}", e))?; Ok(serde_json::json!({ "success": true })) diff --git a/src/crates/assembly/core/src/agentic/coordination/coordinator.rs b/src/crates/assembly/core/src/agentic/coordination/coordinator.rs index 1911de52f5..03b5624121 100644 --- a/src/crates/assembly/core/src/agentic/coordination/coordinator.rs +++ b/src/crates/assembly/core/src/agentic/coordination/coordinator.rs @@ -4675,14 +4675,8 @@ Update the persona files and delete BOOTSTRAP.md as soon as bootstrap is complet } /// Confirm tool execution - pub async fn confirm_tool( - &self, - tool_id: &str, - updated_input: Option, - ) -> BitFunResult<()> { - self.tool_pipeline - .confirm_tool(tool_id, updated_input) - .await + pub async fn confirm_tool(&self, tool_id: &str) -> BitFunResult<()> { + self.tool_pipeline.confirm_tool(tool_id).await } /// Reject tool execution @@ -8025,7 +8019,7 @@ impl bitfun_agent_runtime::sdk::AgentInteractionResponsePort for ConversationCoo &self, request: bitfun_agent_runtime::sdk::AgentToolConfirmationRequest, ) -> bitfun_runtime_ports::PortResult<()> { - self.confirm_tool(&request.tool_id, request.updated_input) + self.confirm_tool(&request.tool_id) .await .map_err(runtime_port_error_preserving_message) } @@ -8449,7 +8443,6 @@ mod tests { &coordinator, AgentToolConfirmationRequest { tool_id: missing_tool_id.clone(), - updated_input: Some(serde_json::json!({ "path": "updated.txt" })), }, ) .await diff --git a/src/crates/assembly/core/src/agentic/tools/pipeline/state_manager.rs b/src/crates/assembly/core/src/agentic/tools/pipeline/state_manager.rs index ad1e00e266..04c1518cdd 100644 --- a/src/crates/assembly/core/src/agentic/tools/pipeline/state_manager.rs +++ b/src/crates/assembly/core/src/agentic/tools/pipeline/state_manager.rs @@ -92,19 +92,6 @@ impl ToolStateManager { self.tasks.get(tool_id).map(|t| t.clone()) } - /// Update task arguments - pub fn update_task_arguments(&self, tool_id: &str, new_arguments: serde_json::Value) { - if let Some(mut task) = self.tasks.get_mut(tool_id) { - debug!( - "Updated tool arguments: tool_id={}, old_args={:?}, new_args={:?}", - tool_id, - task.effective_arguments(), - new_arguments - ); - task.update_effective_arguments(new_arguments); - } - } - /// Get all tasks of a session pub fn get_session_tasks(&self, session_id: &str) -> Vec { self.tasks diff --git a/src/crates/assembly/core/src/agentic/tools/pipeline/tool_pipeline.rs b/src/crates/assembly/core/src/agentic/tools/pipeline/tool_pipeline.rs index e3d77a1284..627841eaa3 100644 --- a/src/crates/assembly/core/src/agentic/tools/pipeline/tool_pipeline.rs +++ b/src/crates/assembly/core/src/agentic/tools/pipeline/tool_pipeline.rs @@ -1568,11 +1568,7 @@ impl ToolPipeline { } /// Confirm tool execution - pub async fn confirm_tool( - &self, - tool_id: &str, - updated_input: Option, - ) -> BitFunResult<()> { + pub async fn confirm_tool(&self, tool_id: &str) -> BitFunResult<()> { let task = self .state_manager .get_task(tool_id) @@ -1586,12 +1582,6 @@ impl ToolPipeline { ))); } - // If the user modified the parameters, update the task parameters first - if let Some(new_args) = updated_input { - debug!("User updated tool arguments: tool_id={}", tool_id); - self.state_manager.update_task_arguments(tool_id, new_args); - } - // Get sender from map and send confirmation response if self.confirmation_channels.confirm(tool_id) { info!("User confirmed tool execution: tool_id={}", tool_id); diff --git a/src/crates/assembly/core/src/agentic/tools/pipeline/types.rs b/src/crates/assembly/core/src/agentic/tools/pipeline/types.rs index d805794e56..b19c0645ce 100644 --- a/src/crates/assembly/core/src/agentic/tools/pipeline/types.rs +++ b/src/crates/assembly/core/src/agentic/tools/pipeline/types.rs @@ -140,12 +140,6 @@ impl ToolTask { pub fn effective_arguments(&self) -> &serde_json::Value { &self.invocation.effective_arguments } - - pub fn update_effective_arguments(&mut self, arguments: serde_json::Value) { - self.invocation - .replace_effective_arguments(arguments.clone()); - self.tool_call.arguments = self.invocation.wire_arguments.clone(); - } } /// Tool execution result wrapper diff --git a/src/crates/assembly/core/src/service_agent_runtime.rs b/src/crates/assembly/core/src/service_agent_runtime.rs index d435d76355..308a1148eb 100644 --- a/src/crates/assembly/core/src/service_agent_runtime.rs +++ b/src/crates/assembly/core/src/service_agent_runtime.rs @@ -1550,13 +1550,9 @@ impl RemotePollRuntimeHost for CoreRemotePollRuntimeHost<'_> { #[async_trait::async_trait] impl RemoteInteractionRuntimeHost for CoreRemoteInteractionRuntimeHost { - async fn confirm_tool( - &self, - tool_id: &str, - updated_input: Option, - ) -> Result<(), String> { + async fn confirm_tool(&self, tool_id: &str) -> Result<(), String> { self.coordinator()? - .confirm_tool(tool_id, updated_input) + .confirm_tool(tool_id) .await .map(|_| ()) .map_err(|error| error.to_string()) diff --git a/src/crates/execution/agent-runtime/src/runtime.rs b/src/crates/execution/agent-runtime/src/runtime.rs index ac72dc712e..2cb1e37d52 100644 --- a/src/crates/execution/agent-runtime/src/runtime.rs +++ b/src/crates/execution/agent-runtime/src/runtime.rs @@ -99,11 +99,9 @@ pub trait AgentSessionRestorePort: Send + Sync { #[derive(Debug, Clone, PartialEq, serde::Serialize, serde::Deserialize)] #[serde(rename_all = "camelCase")] -/// Confirms a pending tool call, optionally replacing its input before execution. +/// Confirms a pending tool call. pub struct AgentToolConfirmationRequest { pub tool_id: String, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub updated_input: Option, } #[derive(Debug, Clone, PartialEq, serde::Serialize, serde::Deserialize)] diff --git a/src/crates/execution/agent-runtime/tests/interaction_response_contracts.rs b/src/crates/execution/agent-runtime/tests/interaction_response_contracts.rs index a484c392e7..a59b5cfa54 100644 --- a/src/crates/execution/agent-runtime/tests/interaction_response_contracts.rs +++ b/src/crates/execution/agent-runtime/tests/interaction_response_contracts.rs @@ -83,7 +83,6 @@ async fn sdk_forwards_typed_interaction_responses_without_losing_payloads() { let confirmation = AgentToolConfirmationRequest { tool_id: "tool-1".to_string(), - updated_input: Some(json!({ "path": "updated.txt" })), }; let rejection = AgentToolRejectionRequest { tool_id: "tool-2".to_string(), @@ -127,7 +126,6 @@ async fn sdk_reports_a_missing_interaction_response_port() { let error = runtime .confirm_tool(AgentToolConfirmationRequest { tool_id: "tool-1".to_string(), - updated_input: None, }) .await .expect_err("missing port must be explicit"); @@ -140,12 +138,10 @@ fn interaction_response_requests_keep_camel_case_wire_fields() { assert_eq!( serde_json::to_value(AgentToolConfirmationRequest { tool_id: "tool-1".to_string(), - updated_input: Some(json!({ "command": "safe" })), }) .expect("serialize confirmation request"), json!({ "toolId": "tool-1", - "updatedInput": { "command": "safe" }, }) ); assert_eq!( diff --git a/src/crates/interfaces/acp/src/runtime/prompt.rs b/src/crates/interfaces/acp/src/runtime/prompt.rs index 680ab4b5d6..7498fe5919 100644 --- a/src/crates/interfaces/acp/src/runtime/prompt.rs +++ b/src/crates/interfaces/acp/src/runtime/prompt.rs @@ -344,7 +344,6 @@ async fn handle_permission_request( .agent_runtime .confirm_tool(AgentToolConfirmationRequest { tool_id: tool_id.to_string(), - updated_input: None, }) .await .map_err(BitfunAcpRuntime::runtime_error)?; diff --git a/src/crates/services/services-integrations/src/remote_connect.rs b/src/crates/services/services-integrations/src/remote_connect.rs index 0318b315cb..b064743fe7 100644 --- a/src/crates/services/services-integrations/src/remote_connect.rs +++ b/src/crates/services/services-integrations/src/remote_connect.rs @@ -1465,11 +1465,7 @@ where #[async_trait::async_trait] pub trait RemoteInteractionRuntimeHost: Send + Sync { - async fn confirm_tool( - &self, - tool_id: &str, - updated_input: Option, - ) -> Result<(), String>; + async fn confirm_tool(&self, tool_id: &str) -> Result<(), String>; async fn reject_tool(&self, tool_id: &str, reason: String) -> Result<(), String>; async fn cancel_tool(&self, tool_id: &str, reason: String) -> Result<(), String>; fn answer_question(&self, tool_id: &str, answers: serde_json::Value) -> Result<(), String>; @@ -1483,13 +1479,10 @@ where H: RemoteInteractionRuntimeHost + ?Sized, { match command { - RemoteCommand::ConfirmTool { - tool_id, - updated_input, - } => remote_interaction_accepted_response( + RemoteCommand::ConfirmTool { tool_id } => remote_interaction_accepted_response( "confirm_tool", tool_id.clone(), - host.confirm_tool(tool_id, updated_input.clone()).await, + host.confirm_tool(tool_id).await, ), RemoteCommand::RejectTool { tool_id, reason } => { let reject_reason = reason @@ -2105,7 +2098,6 @@ pub enum RemoteCommand { }, ConfirmTool { tool_id: String, - updated_input: Option, }, RejectTool { tool_id: String, @@ -3719,11 +3711,7 @@ mod tests { #[async_trait::async_trait] impl RemoteInteractionRuntimeHost for FakeInteractionHost { - async fn confirm_tool( - &self, - _tool_id: &str, - _updated_input: Option, - ) -> Result<(), String> { + async fn confirm_tool(&self, _tool_id: &str) -> Result<(), String> { Ok(()) } diff --git a/src/crates/services/services-integrations/tests/remote_connect_contracts.rs b/src/crates/services/services-integrations/tests/remote_connect_contracts.rs index 0bf97046dc..ca05582d87 100644 --- a/src/crates/services/services-integrations/tests/remote_connect_contracts.rs +++ b/src/crates/services/services-integrations/tests/remote_connect_contracts.rs @@ -984,7 +984,6 @@ async fn remote_connect_command_owner_preserves_cancel_and_group_routing() { &host, &RemoteCommand::ConfirmTool { tool_id: "tool-1".to_string(), - updated_input: None, }, RemoteConnectSubmissionSource::Relay, ) diff --git a/src/web-ui/src/flow_chat/components/FlowItemRenderer.tsx b/src/web-ui/src/flow_chat/components/FlowItemRenderer.tsx index 553730d908..3a41a47c8e 100644 --- a/src/web-ui/src/flow_chat/components/FlowItemRenderer.tsx +++ b/src/web-ui/src/flow_chat/components/FlowItemRenderer.tsx @@ -15,7 +15,7 @@ interface FlowItemRendererProps { item: FlowItem; onFileViewRequest?: (filePath: string) => void; onTabOpen?: (tabInfo: any) => void; - onConfirm?: (toolId: string, updatedInput?: any, permissionOptionId?: string, approve?: boolean) => void; + onConfirm?: (toolId: string, permissionOptionId?: string, approve?: boolean) => void; onReject?: (toolId: string, options?: ToolRejectOptions) => void; sessionId?: string; } diff --git a/src/web-ui/src/flow_chat/components/FlowToolCard.tsx b/src/web-ui/src/flow_chat/components/FlowToolCard.tsx index 34a67687bd..cd5e48ef27 100644 --- a/src/web-ui/src/flow_chat/components/FlowToolCard.tsx +++ b/src/web-ui/src/flow_chat/components/FlowToolCard.tsx @@ -18,7 +18,7 @@ const log = createLogger('FlowToolCard'); interface FlowToolCardProps { toolItem: FlowToolItem; - onConfirm?: (toolId: string, updatedInput?: any, permissionOptionId?: string, approve?: boolean) => void; + onConfirm?: (toolId: string, permissionOptionId?: string, approve?: boolean) => void; onReject?: (toolId: string, options?: ToolRejectOptions) => void; onOpenInEditor?: (filePath: string) => void; onOpenInPanel?: (panelType: string, data: any) => void; @@ -53,16 +53,14 @@ export const FlowToolCard: React.FC = React.memo(({ ? 'chat-browser-tool-card' : undefined; - const handleConfirm = React.useCallback((updatedInput?: any, permissionOptionId?: string, approve?: boolean) => { + const handleConfirm = React.useCallback((permissionOptionId?: string, approve?: boolean) => { log.debug('handleConfirm called', { toolId: toolItem.id, toolName: effectiveToolItem.toolName, - hasUpdatedInput: updatedInput !== undefined, - updatedInputKeys: updatedInput ? Object.keys(updatedInput) : [], hasPermissionOption: Boolean(permissionOptionId), approve }); - onConfirm?.(toolItem.id, updatedInput, permissionOptionId, approve); + onConfirm?.(toolItem.id, permissionOptionId, approve); }, [effectiveToolItem.toolName, toolItem.id, onConfirm]); const handleReject = React.useCallback((options?: ToolRejectOptions) => { diff --git a/src/web-ui/src/flow_chat/components/ToolApprovalBar.test.tsx b/src/web-ui/src/flow_chat/components/ToolApprovalBar.test.tsx index ec0d264b6e..0b0df0ac66 100644 --- a/src/web-ui/src/flow_chat/components/ToolApprovalBar.test.tsx +++ b/src/web-ui/src/flow_chat/components/ToolApprovalBar.test.tsx @@ -158,7 +158,7 @@ describe('ToolApprovalBar', () => { rejectButton.dispatchEvent(new dom.window.MouseEvent('click', { bubbles: true })); }); - expect(onConfirm).toHaveBeenCalledWith(input); + expect(onConfirm).toHaveBeenCalledWith(); expect(onReject).toHaveBeenCalledWith(); }); diff --git a/src/web-ui/src/flow_chat/components/ToolApprovalBar.tsx b/src/web-ui/src/flow_chat/components/ToolApprovalBar.tsx index 0039efb001..95b3d11252 100644 --- a/src/web-ui/src/flow_chat/components/ToolApprovalBar.tsx +++ b/src/web-ui/src/flow_chat/components/ToolApprovalBar.tsx @@ -14,7 +14,7 @@ const log = createLogger('ToolApprovalBar'); interface ToolApprovalBarProps { toolItem: FlowToolItem; - onConfirm?: (updatedInput?: any, permissionOptionId?: string, approve?: boolean) => void; + onConfirm?: (permissionOptionId?: string, approve?: boolean) => void; onReject?: (options?: ToolRejectOptions) => void; } @@ -97,7 +97,7 @@ export const ToolApprovalBar: React.FC = ({ return; } - onConfirm?.(input); + onConfirm?.(); }; const handleReject = (event: React.MouseEvent) => { @@ -172,7 +172,6 @@ export const ToolApprovalBar: React.FC = ({ {hasPermissionOptions ? ( (({ item, turnId sessionId, } = useFlowChatContext(); - const handleConfirm = useCallback(async (toolId: string, updatedInput?: any, permissionOptionId?: string, approve?: boolean) => { + const handleConfirm = useCallback(async (toolId: string, permissionOptionId?: string, approve?: boolean) => { if (onToolConfirm) { - await onToolConfirm(toolId, updatedInput, permissionOptionId, approve); + await onToolConfirm(toolId, permissionOptionId, approve); } }, [onToolConfirm]); diff --git a/src/web-ui/src/flow_chat/components/modern/FlowChatContext.tsx b/src/web-ui/src/flow_chat/components/modern/FlowChatContext.tsx index e19f4fbe5f..f34a985fdf 100644 --- a/src/web-ui/src/flow_chat/components/modern/FlowChatContext.tsx +++ b/src/web-ui/src/flow_chat/components/modern/FlowChatContext.tsx @@ -17,7 +17,7 @@ export interface FlowChatContextValue { onSwitchToChatPanel?: () => void; // Tool actions - onToolConfirm?: (toolId: string, updatedInput?: any, permissionOptionId?: string, approve?: boolean) => Promise; + onToolConfirm?: (toolId: string, permissionOptionId?: string, approve?: boolean) => Promise; onToolReject?: (toolId: string, options?: ToolRejectOptions) => Promise; // Session info diff --git a/src/web-ui/src/flow_chat/components/modern/ModelRoundItem.tsx b/src/web-ui/src/flow_chat/components/modern/ModelRoundItem.tsx index 3d85cadd40..abd517b7cd 100644 --- a/src/web-ui/src/flow_chat/components/modern/ModelRoundItem.tsx +++ b/src/web-ui/src/flow_chat/components/modern/ModelRoundItem.tsx @@ -893,9 +893,9 @@ const FlowItemRenderer: React.FC = ({
{ + onConfirm={async (toolId: string, permissionOptionId?: string, approve?: boolean) => { if (onToolConfirm) { - await onToolConfirm(toolId, updatedInput, permissionOptionId, approve); + await onToolConfirm(toolId, permissionOptionId, approve); } }} onReject={async (_toolId: string, options?: ToolRejectOptions) => { diff --git a/src/web-ui/src/flow_chat/components/modern/useFlowChatToolActions.ts b/src/web-ui/src/flow_chat/components/modern/useFlowChatToolActions.ts index bdd84647e2..fa4bb62349 100644 --- a/src/web-ui/src/flow_chat/components/modern/useFlowChatToolActions.ts +++ b/src/web-ui/src/flow_chat/components/modern/useFlowChatToolActions.ts @@ -3,10 +3,6 @@ */ import { useCallback } from 'react'; -import { - effectiveToolInvocation, - replaceEffectiveToolInput, -} from '../../utils/toolInvocationIdentity'; import { notificationService } from '@/shared/notification-system'; import { createLogger } from '@/shared/utils/logger'; import { @@ -62,7 +58,6 @@ function resolveToolContext(toolId: string): ResolvedToolContext { export function useFlowChatToolActions() { const handleToolConfirm = useCallback(async ( toolId: string, - updatedInput?: any, permissionOptionId?: string, approve = true, ) => { @@ -74,21 +69,9 @@ export function useFlowChatToolActions() { return; } - const effective = effectiveToolInvocation(toolItem.toolName, toolItem.toolCall?.input); - const finalInput = updatedInput || effective.input; - const finalWireInput = replaceEffectiveToolInput( - toolItem.toolName, - toolItem.toolCall?.input, - finalInput, - ); - flowChatStore.updateModelRoundItem(sessionId, turnId, toolId, { userConfirmed: approve, status: approve ? 'confirmed' : 'rejected', - toolCall: { - ...toolItem.toolCall, - input: finalWireInput, - }, ...(approve ? {} : { requiresConfirmation: false, acpPermission: undefined, @@ -117,7 +100,6 @@ export function useFlowChatToolActions() { sessionId, toolId, 'confirm', - finalInput, ); } catch (error) { log.error('Tool confirmation failed', error); @@ -163,7 +145,6 @@ export function useFlowChatToolActions() { sessionId, toolId, 'reject', - undefined, options?.instruction, ); } catch (error) { diff --git a/src/web-ui/src/flow_chat/hooks/useFlowChat.ts b/src/web-ui/src/flow_chat/hooks/useFlowChat.ts index 95c2c2fa50..19a3a7dcb8 100644 --- a/src/web-ui/src/flow_chat/hooks/useFlowChat.ts +++ b/src/web-ui/src/flow_chat/hooks/useFlowChat.ts @@ -421,7 +421,7 @@ export const useFlowChat = () => { processingLock.current = false; }, []); - const confirmTool = useCallback((_toolId: string, _updatedInput?: any) => { + const confirmTool = useCallback((_toolId: string) => { }, []); const rejectTool = useCallback((_toolId: string) => { diff --git a/src/web-ui/src/flow_chat/tool-cards/AcpPermissionActions.tsx b/src/web-ui/src/flow_chat/tool-cards/AcpPermissionActions.tsx index 89e5b6a495..17245510a7 100644 --- a/src/web-ui/src/flow_chat/tool-cards/AcpPermissionActions.tsx +++ b/src/web-ui/src/flow_chat/tool-cards/AcpPermissionActions.tsx @@ -16,12 +16,11 @@ const KIND_ORDER: Record = { interface AcpPermissionActionsProps { toolItem: FlowToolItem; - input?: any; disabled?: boolean; presentation?: 'icon' | 'text'; className?: string; buttonClassName?: string; - onConfirm?: (updatedInput?: any, permissionOptionId?: string, approve?: boolean) => void; + onConfirm?: (permissionOptionId?: string, approve?: boolean) => void; onReject?: (options?: ToolRejectOptions) => void; } @@ -64,7 +63,6 @@ function buttonVariant(kind: AcpPermissionOption['kind']): 'success' | 'danger' export const AcpPermissionActions: React.FC = ({ toolItem, - input, disabled = false, presentation = 'icon', className = '', @@ -94,7 +92,7 @@ export const AcpPermissionActions: React.FC = ({ event.stopPropagation(); if (approve) { - onConfirm?.(input, option.optionId, true); + onConfirm?.(option.optionId, true); } else { onReject?.({ permissionOptionId: option.optionId }); } diff --git a/src/web-ui/src/flow_chat/types/flow-chat.ts b/src/web-ui/src/flow_chat/types/flow-chat.ts index 10364f7180..c2f99eeaec 100644 --- a/src/web-ui/src/flow_chat/types/flow-chat.ts +++ b/src/web-ui/src/flow_chat/types/flow-chat.ts @@ -586,7 +586,7 @@ export interface FlowChatActions { sendMessage: (message: string, sessionId?: string) => Promise; createSession: (config?: Partial) => Promise; switchSession: (sessionId: string) => void; - confirmTool: (toolId: string, updatedInput?: any) => void; + confirmTool: (toolId: string) => void; rejectTool: (toolId: string) => void; clearSession: (sessionId?: string) => void; deleteSession: (sessionId: string) => Promise; // Now async. diff --git a/src/web-ui/src/flow_chat/utils/toolInvocationIdentity.test.ts b/src/web-ui/src/flow_chat/utils/toolInvocationIdentity.test.ts index 5a5d425704..da38af01cc 100644 --- a/src/web-ui/src/flow_chat/utils/toolInvocationIdentity.test.ts +++ b/src/web-ui/src/flow_chat/utils/toolInvocationIdentity.test.ts @@ -3,7 +3,6 @@ import { describe, expect, it } from 'vitest'; import { effectiveToolInvocation, projectEffectiveToolItem, - replaceEffectiveToolInput, } from './toolInvocationIdentity'; describe('toolInvocationIdentity', () => { @@ -55,19 +54,4 @@ describe('toolInvocationIdentity', () => { expect(item.toolName).toBe('CallDeferredTool'); expect(item.toolCall.input).toHaveProperty('tool_name', 'Write'); }); - - it('writes edited effective input back into deferred args', () => { - const wireInput = { - tool_name: 'Write', - args: { file_path: 'README.md', content: 'before' }, - }; - - expect(replaceEffectiveToolInput('CallDeferredTool', wireInput, { - file_path: 'README.md', - content: 'after', - })).toEqual({ - tool_name: 'Write', - args: { file_path: 'README.md', content: 'after' }, - }); - }); }); diff --git a/src/web-ui/src/flow_chat/utils/toolInvocationIdentity.ts b/src/web-ui/src/flow_chat/utils/toolInvocationIdentity.ts index a39d2df0bb..deef81b52b 100644 --- a/src/web-ui/src/flow_chat/utils/toolInvocationIdentity.ts +++ b/src/web-ui/src/flow_chat/utils/toolInvocationIdentity.ts @@ -61,19 +61,3 @@ export function projectEffectiveToolItem(toolItem: FlowToolItem): FlowToolItem { }, }; } - -export function replaceEffectiveToolInput( - wireToolName: string, - wireInput: unknown, - effectiveInput: unknown, -): unknown { - const current = effectiveToolInvocation(wireToolName, wireInput); - if (!current.isDeferred || wireInput === null || typeof wireInput !== 'object') { - return effectiveInput; - } - - return { - ...(wireInput as Record), - args: effectiveInput, - }; -} diff --git a/src/web-ui/src/infrastructure/api/service-api/ToolAPI.ts b/src/web-ui/src/infrastructure/api/service-api/ToolAPI.ts index 5c4f4f7850..5d934416f4 100644 --- a/src/web-ui/src/infrastructure/api/service-api/ToolAPI.ts +++ b/src/web-ui/src/infrastructure/api/service-api/ToolAPI.ts @@ -69,8 +69,7 @@ export class ToolAPI { const confirmRequest = { sessionId: request.sessionId, - toolId: request.toolId, - updatedInput: request.updatedInput || null + toolId: request.toolId }; const result = await api.invoke('confirm_tool_execution', { request: confirmRequest }); diff --git a/src/web-ui/src/shared/services/agent-service.ts b/src/web-ui/src/shared/services/agent-service.ts index 6dc49e1db7..f8d598f679 100644 --- a/src/web-ui/src/shared/services/agent-service.ts +++ b/src/web-ui/src/shared/services/agent-service.ts @@ -630,14 +630,12 @@ export class AgentService { sessionId: string, toolUseId: string, action: 'confirm' | 'reject', - updatedInput?: any, rejectReason?: string ): Promise { const requestPayload = { sessionId, toolId: toolUseId, action, - updatedInput: updatedInput || null, rejectReason }; From 3cc6c6c29e39abc992171fbfe49063711187f724 Mon Sep 17 00:00:00 2001 From: wsp Date: Sat, 18 Jul 2026 12:54:54 +0800 Subject: [PATCH 02/34] feat(permission): add v2 rule contracts and evaluator - Add allow, ask, and deny permission rule contracts - Support ordered global, project, and agent rule merging - Implement wildcard matching with normalized path separators - Support case-insensitive Windows resource matching - Apply last-matching-rule and default-ask semantics - Add atomic multi-resource deny, ask, and allow evaluation - Cover serialization, wildcard, ordering, and multi-resource behavior with tests --- .../contracts/product-domains/src/lib.rs | 1 + .../product-domains/src/tool_permissions.rs | 199 ++++++++++++++++++ .../tests/tool_permission_contracts.rs | 165 +++++++++++++++ 3 files changed, 365 insertions(+) create mode 100644 src/crates/contracts/product-domains/src/tool_permissions.rs create mode 100644 src/crates/contracts/product-domains/tests/tool_permission_contracts.rs diff --git a/src/crates/contracts/product-domains/src/lib.rs b/src/crates/contracts/product-domains/src/lib.rs index c3e985eb9a..f4d15eec77 100644 --- a/src/crates/contracts/product-domains/src/lib.rs +++ b/src/crates/contracts/product-domains/src/lib.rs @@ -4,6 +4,7 @@ //! the full BitFun core runtime assembly. pub mod canvas; +pub mod tool_permissions; #[cfg(feature = "external-sources")] pub mod external_sources; diff --git a/src/crates/contracts/product-domains/src/tool_permissions.rs b/src/crates/contracts/product-domains/src/tool_permissions.rs new file mode 100644 index 0000000000..446d454f11 --- /dev/null +++ b/src/crates/contracts/product-domains/src/tool_permissions.rs @@ -0,0 +1,199 @@ +//! Pure domain contracts and evaluation rules for tool-call permissions. +//! +//! This module intentionally has no runtime, persistence, or interaction +//! responsibilities. Product assembly and execution owners may consume these +//! decisions in later integration phases. + +use serde::{Deserialize, Serialize}; + +/// The effect produced by a matching permission rule. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum PermissionEffect { + Allow, + Ask, + Deny, +} + +/// An ordered action/resource permission rule. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct PermissionRule { + pub action: String, + pub resource: String, + pub effect: PermissionEffect, +} + +impl PermissionRule { + pub fn new( + action: impl Into, + resource: impl Into, + effect: PermissionEffect, + ) -> Self { + Self { + action: action.into(), + resource: resource.into(), + effect, + } + } +} + +/// A rule list whose order is significant: later matching rules win. +pub type PermissionRuleset = Vec; + +/// Controls resource matching for local or remote workspace path semantics. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum PermissionResourceCaseSensitivity { + Sensitive, + Insensitive, +} + +/// Pure evaluator for ordered tool permission rules. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct PermissionEvaluator { + resource_case_sensitivity: PermissionResourceCaseSensitivity, +} + +impl PermissionEvaluator { + pub const fn new(resource_case_sensitivity: PermissionResourceCaseSensitivity) -> Self { + Self { + resource_case_sensitivity, + } + } + + pub const fn case_sensitive() -> Self { + Self::new(PermissionResourceCaseSensitivity::Sensitive) + } + + pub const fn windows_compatible() -> Self { + Self::new(PermissionResourceCaseSensitivity::Insensitive) + } + + pub const fn for_current_platform() -> Self { + if cfg!(windows) { + Self::windows_compatible() + } else { + Self::case_sensitive() + } + } + + /// Returns the effect of the last rule matching both action and resource. + /// Unmatched requests default to `ask`. + pub fn evaluate_resource( + &self, + action: &str, + resource: &str, + rules: &[PermissionRule], + ) -> PermissionEffect { + rules + .iter() + .rev() + .find(|rule| { + wildcard_matches( + action, + &rule.action, + PermissionResourceCaseSensitivity::Sensitive, + ) && wildcard_matches(resource, &rule.resource, self.resource_case_sensitivity) + }) + .map(|rule| rule.effect) + .unwrap_or(PermissionEffect::Ask) + } + + /// Evaluates every resource in one tool call atomically. + /// + /// Any denied resource denies the call. Otherwise any resource that still + /// requires confirmation makes the call ask. Only an all-allow result is + /// allowed. A request without resources fails closed as `ask`. + pub fn evaluate_resources( + &self, + action: &str, + resources: &[String], + rules: &[PermissionRule], + ) -> PermissionEffect { + if resources.is_empty() { + return PermissionEffect::Ask; + } + + let mut aggregate = PermissionEffect::Allow; + for resource in resources { + match self.evaluate_resource(action, resource, rules) { + PermissionEffect::Deny => return PermissionEffect::Deny, + PermissionEffect::Ask => aggregate = PermissionEffect::Ask, + PermissionEffect::Allow => {} + } + } + aggregate + } +} + +impl Default for PermissionEvaluator { + fn default() -> Self { + Self::for_current_platform() + } +} + +/// Merges global, project, and agent rule layers without changing their order. +pub fn merge_permission_rule_layers(layers: &[&[PermissionRule]]) -> PermissionRuleset { + let capacity = layers.iter().map(|layer| layer.len()).sum(); + let mut merged = Vec::with_capacity(capacity); + for layer in layers { + merged.extend_from_slice(layer); + } + merged +} + +/// Matches `*` and `?` wildcards after normalizing path separators. +/// +/// Like the OpenCode V2 reference, a pattern ending in ` *` also matches the +/// prefix without a trailing argument (for example, `git *` matches `git`). +pub fn wildcard_matches( + input: &str, + pattern: &str, + case_sensitivity: PermissionResourceCaseSensitivity, +) -> bool { + let input = normalize_wildcard_value(input, case_sensitivity); + let pattern = normalize_wildcard_value(pattern, case_sensitivity); + + if pattern + .strip_suffix(" *") + .is_some_and(|prefix| input == prefix) + { + return true; + } + + glob_matches(&input, &pattern) +} + +fn normalize_wildcard_value( + value: &str, + case_sensitivity: PermissionResourceCaseSensitivity, +) -> String { + let normalized = value.replace('\\', "/"); + match case_sensitivity { + PermissionResourceCaseSensitivity::Sensitive => normalized, + PermissionResourceCaseSensitivity::Insensitive => normalized.to_lowercase(), + } +} + +fn glob_matches(input: &str, pattern: &str) -> bool { + let input: Vec = input.chars().collect(); + let mut previous = vec![false; input.len() + 1]; + previous[0] = true; + + for pattern_char in pattern.chars() { + let mut current = vec![false; input.len() + 1]; + if pattern_char == '*' { + current[0] = previous[0]; + } + + for (index, input_char) in input.iter().enumerate() { + current[index + 1] = match pattern_char { + '*' => previous[index + 1] || current[index], + '?' => previous[index], + literal => previous[index] && literal == *input_char, + }; + } + previous = current; + } + + previous[input.len()] +} diff --git a/src/crates/contracts/product-domains/tests/tool_permission_contracts.rs b/src/crates/contracts/product-domains/tests/tool_permission_contracts.rs new file mode 100644 index 0000000000..4e9f549abe --- /dev/null +++ b/src/crates/contracts/product-domains/tests/tool_permission_contracts.rs @@ -0,0 +1,165 @@ +use bitfun_product_domains::tool_permissions::{ + merge_permission_rule_layers, wildcard_matches, PermissionEffect, PermissionEvaluator, + PermissionResourceCaseSensitivity, PermissionRule, +}; +use serde_json::json; + +fn rule(action: &str, resource: &str, effect: PermissionEffect) -> PermissionRule { + PermissionRule::new(action, resource, effect) +} + +#[test] +fn permission_rule_uses_stable_wire_values() { + let value = serde_json::to_value(rule("read", "src/*", PermissionEffect::Ask)) + .expect("serialize permission rule"); + + assert_eq!( + value, + json!({ + "action": "read", + "resource": "src/*", + "effect": "ask", + }) + ); + assert_eq!( + serde_json::from_value::(value).expect("deserialize permission rule"), + rule("read", "src/*", PermissionEffect::Ask) + ); +} + +#[test] +fn wildcard_matching_supports_star_question_and_normalized_separators() { + let sensitive = PermissionResourceCaseSensitivity::Sensitive; + + assert!(wildcard_matches("src/main.rs", "src/*.rs", sensitive)); + assert!(wildcard_matches("src/main.rs", "src/mai?.rs", sensitive)); + assert!(wildcard_matches( + r"src\nested\main.rs", + "src/*/main.rs", + sensitive + )); + assert!(wildcard_matches("git", "git *", sensitive)); + assert!(wildcard_matches("git status", "git *", sensitive)); + assert!(!wildcard_matches("src/main.ts", "src/*.rs", sensitive)); + assert!(!wildcard_matches( + "src/deep/main.rs", + "src/????.rs", + sensitive + )); +} + +#[test] +fn windows_compatible_matching_is_case_insensitive_for_resources() { + let evaluator = PermissionEvaluator::windows_compatible(); + let rules = vec![rule( + "read", + r"C:\Users\Developer\Project\*", + PermissionEffect::Allow, + )]; + + assert_eq!( + evaluator.evaluate_resource("read", r"c:\users\developer\project\SRC\main.rs", &rules,), + PermissionEffect::Allow + ); + assert_eq!( + PermissionEvaluator::case_sensitive().evaluate_resource( + "read", + r"c:\users\developer\project\SRC\main.rs", + &rules, + ), + PermissionEffect::Ask + ); +} + +#[test] +fn last_matching_action_and_resource_rule_wins() { + let evaluator = PermissionEvaluator::case_sensitive(); + let rules = vec![ + rule("*", "*", PermissionEffect::Ask), + rule("read", "src/*", PermissionEffect::Allow), + rule("read", "src/private/*", PermissionEffect::Deny), + rule("read", "src/private/public.txt", PermissionEffect::Allow), + ]; + + assert_eq!( + evaluator.evaluate_resource("read", "src/lib.rs", &rules), + PermissionEffect::Allow + ); + assert_eq!( + evaluator.evaluate_resource("read", "src/private/key.txt", &rules), + PermissionEffect::Deny + ); + assert_eq!( + evaluator.evaluate_resource("read", "src/private/public.txt", &rules), + PermissionEffect::Allow + ); + assert_eq!( + evaluator.evaluate_resource("edit", "src/lib.rs", &rules), + PermissionEffect::Ask + ); +} + +#[test] +fn merged_layers_preserve_global_project_agent_override_order() { + let global = vec![rule("*", "*", PermissionEffect::Ask)]; + let project = vec![rule("read", "*", PermissionEffect::Allow)]; + let agent = vec![rule("read", "secrets/*", PermissionEffect::Deny)]; + let merged = merge_permission_rule_layers(&[&global, &project, &agent]); + let evaluator = PermissionEvaluator::case_sensitive(); + + assert_eq!(merged, [global, project, agent].concat()); + assert_eq!( + evaluator.evaluate_resource("read", "README.md", &merged), + PermissionEffect::Allow + ); + assert_eq!( + evaluator.evaluate_resource("read", "secrets/token.txt", &merged), + PermissionEffect::Deny + ); +} + +#[test] +fn unmatched_and_empty_resource_requests_default_to_ask() { + let evaluator = PermissionEvaluator::case_sensitive(); + let rules = vec![rule("read", "src/*", PermissionEffect::Allow)]; + + assert_eq!( + evaluator.evaluate_resource("edit", "src/lib.rs", &rules), + PermissionEffect::Ask + ); + assert_eq!( + evaluator.evaluate_resources("read", &[], &rules), + PermissionEffect::Ask + ); +} + +#[test] +fn multi_resource_decision_is_atomic_with_deny_then_ask_precedence() { + let evaluator = PermissionEvaluator::case_sensitive(); + let rules = vec![ + rule("edit", "src/*", PermissionEffect::Allow), + rule("edit", "src/generated/*", PermissionEffect::Ask), + rule("edit", "src/secrets/*", PermissionEffect::Deny), + ]; + + assert_eq!( + evaluator.evaluate_resources("edit", &["src/lib.rs".into(), "src/main.rs".into()], &rules,), + PermissionEffect::Allow + ); + assert_eq!( + evaluator.evaluate_resources( + "edit", + &["src/lib.rs".into(), "src/generated/api.rs".into()], + &rules, + ), + PermissionEffect::Ask + ); + assert_eq!( + evaluator.evaluate_resources( + "edit", + &["src/generated/api.rs".into(), "src/secrets/key.rs".into(),], + &rules, + ), + PermissionEffect::Deny + ); +} From 5de0c4ff85fa5aa3e9c55ef7a4d0cb8e671de119 Mon Sep 17 00:00:00 2001 From: wsp Date: Sat, 18 Jul 2026 14:04:04 +0800 Subject: [PATCH 03/34] feat(permission): add pending requests and project grant store - Add permission request, reply, grant, source, and audit contracts - Define grant, audit, and atomic reply persistence ports - Implement process-local pending request and waiting-channel management - Support once, always, reject, request cancellation, and session cancellation - Clear remaining session requests when a permission is rejected - Persist project-scoped remembered grants with unique action-resource keys - Commit always grants and reply audit records atomically - Keep pending requests unresolved when persistence fails - Verify restart persistence, project isolation, audit idempotency, and fail-closed behavior --- scripts/core-boundaries/rules/crate-rules.mjs | 1 - .../core-boundaries/rules/feature-rules.mjs | 8 + .../product-domains/src/tool_permissions.rs | 108 ++++++ .../tests/tool_permission_contracts.rs | 24 +- src/crates/contracts/runtime-ports/Cargo.toml | 5 + src/crates/contracts/runtime-ports/src/lib.rs | 12 + .../runtime-ports/src/permission_v2.rs | 36 ++ src/crates/execution/agent-runtime/Cargo.toml | 2 +- src/crates/execution/agent-runtime/src/lib.rs | 1 + .../agent-runtime/src/permission_v2.rs | 320 ++++++++++++++++ .../tests/permission_v2_contracts.rs | 349 ++++++++++++++++++ src/crates/services/services-core/Cargo.toml | 1 + src/crates/services/services-core/src/lib.rs | 2 + .../services-core/src/permission_store.rs | 288 +++++++++++++++ .../tests/permission_store_contracts.rs | 180 +++++++++ 15 files changed, 1334 insertions(+), 3 deletions(-) create mode 100644 src/crates/contracts/runtime-ports/src/permission_v2.rs create mode 100644 src/crates/execution/agent-runtime/src/permission_v2.rs create mode 100644 src/crates/execution/agent-runtime/tests/permission_v2_contracts.rs create mode 100644 src/crates/services/services-core/src/permission_store.rs create mode 100644 src/crates/services/services-core/tests/permission_store_contracts.rs diff --git a/scripts/core-boundaries/rules/crate-rules.mjs b/scripts/core-boundaries/rules/crate-rules.mjs index 119aa73e1c..8f930db647 100644 --- a/scripts/core-boundaries/rules/crate-rules.mjs +++ b/scripts/core-boundaries/rules/crate-rules.mjs @@ -81,7 +81,6 @@ export const lightweightBoundaryRules = [ 'bitfun-services-integrations', 'bitfun-agent-tools', 'bitfun-tool-packs', - 'bitfun-product-domains', 'bitfun-transport', 'terminal-core', 'tool-runtime', diff --git a/scripts/core-boundaries/rules/feature-rules.mjs b/scripts/core-boundaries/rules/feature-rules.mjs index 48a26b1d63..5408a0a084 100644 --- a/scripts/core-boundaries/rules/feature-rules.mjs +++ b/scripts/core-boundaries/rules/feature-rules.mjs @@ -1,6 +1,14 @@ // Boundary rules for feature assembly and optional dependency ownership. export const optionalDependencyFeatureOwnerRules = [ + { + crateName: 'runtime-ports', + reason: + 'runtime-ports may expose product-domain permission ports only through the explicit permission-v2 contract slice', + dependencies: [ + { depName: 'bitfun-product-domains', ownerFeatures: ['permission-v2'] }, + ], + }, { crateName: 'core', reason: diff --git a/src/crates/contracts/product-domains/src/tool_permissions.rs b/src/crates/contracts/product-domains/src/tool_permissions.rs index 446d454f11..2e9fe269e5 100644 --- a/src/crates/contracts/product-domains/src/tool_permissions.rs +++ b/src/crates/contracts/product-domains/src/tool_permissions.rs @@ -5,6 +5,7 @@ //! decisions in later integration phases. use serde::{Deserialize, Serialize}; +use serde_json::{Map, Value}; /// The effect produced by a matching permission rule. #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] @@ -40,6 +41,113 @@ impl PermissionRule { /// A rule list whose order is significant: later matching rules win. pub type PermissionRuleset = Vec; +/// Identifies the boundary that originated a permission request. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum PermissionRequestSourceKind { + ToolCall, + Provider, + Extension, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct PermissionRequestSource { + pub kind: PermissionRequestSourceKind, + pub identity: String, +} + +/// A process-local permission request projected to an interactive surface. +/// +/// Resource and display values stored here must already be safe for user +/// presentation and audit persistence. Raw secrets and unrestricted command +/// payloads must remain outside this DTO. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct PermissionRequest { + pub request_id: String, + pub project_id: String, + pub session_id: String, + pub agent_id: String, + pub action: String, + pub resources: Vec, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub save_resources: Vec, + pub source: PermissionRequestSource, + #[serde(default, skip_serializing_if = "Map::is_empty")] + pub display_metadata: Map, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(tag = "reply", rename_all = "snake_case")] +pub enum PermissionReply { + Once, + Always, + Reject { + #[serde(default, skip_serializing_if = "Option::is_none")] + feedback: Option, + }, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum PermissionReplySource { + User, + AutoApprove, + System, +} + +/// A remembered allow scoped by project, action, and resource. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct PermissionGrant { + pub project_id: String, + pub action: String, + pub resource: String, + pub created_at_ms: i64, +} + +impl PermissionGrant { + pub fn key(&self) -> PermissionGrantKey { + PermissionGrantKey { + project_id: self.project_id.clone(), + action: self.action.clone(), + resource: self.resource.clone(), + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct PermissionGrantKey { + pub project_id: String, + pub action: String, + pub resource: String, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(tag = "event", rename_all = "snake_case")] +pub enum PermissionAuditEvent { + Requested, + Replied { + reply: PermissionReply, + source: PermissionReplySource, + }, + Cancelled { + reason: String, + }, +} + +/// An append-only audit fact containing only presentation-safe request data. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct PermissionAuditRecord { + pub audit_id: String, + pub request: PermissionRequest, + pub event: PermissionAuditEvent, + pub timestamp_ms: i64, +} + /// Controls resource matching for local or remote workspace path semantics. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum PermissionResourceCaseSensitivity { diff --git a/src/crates/contracts/product-domains/tests/tool_permission_contracts.rs b/src/crates/contracts/product-domains/tests/tool_permission_contracts.rs index 4e9f549abe..c6ca3041d7 100644 --- a/src/crates/contracts/product-domains/tests/tool_permission_contracts.rs +++ b/src/crates/contracts/product-domains/tests/tool_permission_contracts.rs @@ -1,6 +1,6 @@ use bitfun_product_domains::tool_permissions::{ merge_permission_rule_layers, wildcard_matches, PermissionEffect, PermissionEvaluator, - PermissionResourceCaseSensitivity, PermissionRule, + PermissionReply, PermissionResourceCaseSensitivity, PermissionRule, }; use serde_json::json; @@ -27,6 +27,28 @@ fn permission_rule_uses_stable_wire_values() { ); } +#[test] +fn permission_reply_uses_stable_tagged_wire_values() { + assert_eq!( + serde_json::to_value(PermissionReply::Once).expect("serialize once reply"), + json!({ "reply": "once" }) + ); + assert_eq!( + serde_json::to_value(PermissionReply::Always).expect("serialize always reply"), + json!({ "reply": "always" }) + ); + assert_eq!( + serde_json::to_value(PermissionReply::Reject { + feedback: Some("Use a read-only path".to_string()), + }) + .expect("serialize reject reply"), + json!({ + "reply": "reject", + "feedback": "Use a read-only path", + }) + ); +} + #[test] fn wildcard_matching_supports_star_question_and_normalized_separators() { let sensitive = PermissionResourceCaseSensitivity::Sensitive; diff --git a/src/crates/contracts/runtime-ports/Cargo.toml b/src/crates/contracts/runtime-ports/Cargo.toml index e6962a9712..31cf7fa221 100644 --- a/src/crates/contracts/runtime-ports/Cargo.toml +++ b/src/crates/contracts/runtime-ports/Cargo.toml @@ -12,11 +12,16 @@ crate-type = ["rlib"] [dependencies] anyhow = { workspace = true } async-trait = { workspace = true } +bitfun-product-domains = { path = "../product-domains", default-features = false, optional = true } serde = { workspace = true } serde_json = { workspace = true } tokio = { workspace = true } tokio-util = { workspace = true } +[features] +default = [] +permission-v2 = ["dep:bitfun-product-domains"] + [dev-dependencies] tokio = { workspace = true } diff --git a/src/crates/contracts/runtime-ports/src/lib.rs b/src/crates/contracts/runtime-ports/src/lib.rs index 1dcc1c668f..c802ff05f1 100644 --- a/src/crates/contracts/runtime-ports/src/lib.rs +++ b/src/crates/contracts/runtime-ports/src/lib.rs @@ -11,8 +11,20 @@ use std::sync::Arc; use tokio::sync::mpsc; use tokio_util::sync::CancellationToken; +#[cfg(feature = "permission-v2")] +mod permission_v2; mod plugin; mod script_tool; +#[cfg(feature = "permission-v2")] +pub use bitfun_product_domains::tool_permissions::{ + PermissionAuditEvent, PermissionAuditRecord, PermissionGrant, PermissionGrantKey, + PermissionReply, PermissionReplySource, PermissionRequest as PermissionV2Request, + PermissionRequestSource, PermissionRequestSourceKind, +}; +#[cfg(feature = "permission-v2")] +pub use permission_v2::{ + PermissionAuditStorePort, PermissionGrantStorePort, PermissionReplyStorePort, +}; pub use plugin::{ validate_plugin_dispatch_response, validate_plugin_runtime_read_response, DisabledPluginRuntimeClient, ExtensionCapabilityAvailability, PermissionPromptDenyState, diff --git a/src/crates/contracts/runtime-ports/src/permission_v2.rs b/src/crates/contracts/runtime-ports/src/permission_v2.rs new file mode 100644 index 0000000000..aacc0f3243 --- /dev/null +++ b/src/crates/contracts/runtime-ports/src/permission_v2.rs @@ -0,0 +1,36 @@ +use crate::{PortResult, RuntimeServicePort}; +use async_trait::async_trait; +use bitfun_product_domains::tool_permissions::{ + PermissionAuditRecord, PermissionGrant, PermissionGrantKey, +}; + +/// Persistent remembered-grant storage. Pending requests do not belong here. +#[async_trait] +pub trait PermissionGrantStorePort: RuntimeServicePort { + async fn list_project_grants(&self, project_id: &str) -> PortResult>; + + async fn add_project_grants(&self, grants: Vec) -> PortResult<()>; + + async fn remove_project_grant(&self, key: PermissionGrantKey) -> PortResult; +} + +/// Append-only permission audit persistence over presentation-safe DTOs. +#[async_trait] +pub trait PermissionAuditStorePort: RuntimeServicePort { + async fn append_permission_audit(&self, record: PermissionAuditRecord) -> PortResult<()>; + + async fn list_project_permission_audit( + &self, + project_id: &str, + ) -> PortResult>; +} + +/// Atomically commits the durable effects of one permission reply. +#[async_trait] +pub trait PermissionReplyStorePort: RuntimeServicePort { + async fn commit_permission_reply( + &self, + grants: Vec, + audit: Vec, + ) -> PortResult<()>; +} diff --git a/src/crates/execution/agent-runtime/Cargo.toml b/src/crates/execution/agent-runtime/Cargo.toml index 7c8c174847..1e2f01d7f2 100644 --- a/src/crates/execution/agent-runtime/Cargo.toml +++ b/src/crates/execution/agent-runtime/Cargo.toml @@ -19,7 +19,7 @@ bitfun-agent-tools = { path = "../tool-contracts" } bitfun-core-types = { path = "../../contracts/core-types" } bitfun-events = { path = "../../contracts/events" } bitfun-harness = { path = "../harness" } -bitfun-runtime-ports = { path = "../../contracts/runtime-ports" } +bitfun-runtime-ports = { path = "../../contracts/runtime-ports", features = ["permission-v2"] } bitfun-runtime-services = { path = "../runtime-services" } dashmap = { workspace = true } log = { workspace = true } diff --git a/src/crates/execution/agent-runtime/src/lib.rs b/src/crates/execution/agent-runtime/src/lib.rs index a462a8d7b7..af9250df5c 100644 --- a/src/crates/execution/agent-runtime/src/lib.rs +++ b/src/crates/execution/agent-runtime/src/lib.rs @@ -19,6 +19,7 @@ pub mod events; pub mod evidence_ledger; pub mod file_read_state; pub mod output_surface; +pub mod permission_v2; pub mod post_call_hooks; pub mod prompt; pub mod prompt_cache; diff --git a/src/crates/execution/agent-runtime/src/permission_v2.rs b/src/crates/execution/agent-runtime/src/permission_v2.rs new file mode 100644 index 0000000000..dda4d8ded8 --- /dev/null +++ b/src/crates/execution/agent-runtime/src/permission_v2.rs @@ -0,0 +1,320 @@ +//! Process-local pending permission requests and reply coordination. +//! +//! This owner is intentionally not connected to the legacy tool confirmation +//! pipeline yet. It persists remembered grants and audit facts only when an +//! explicit V2 reply is delivered through this standalone contract. + +use bitfun_runtime_ports::{ + ClockPort, PermissionAuditEvent, PermissionAuditRecord, PermissionAuditStorePort, + PermissionGrant, PermissionReply, PermissionReplySource, PermissionReplyStorePort, + PermissionV2Request, PortError, +}; +use dashmap::mapref::entry::Entry; +use dashmap::DashMap; +use std::collections::HashSet; +use std::sync::Arc; +use tokio::sync::{oneshot, Mutex}; + +#[derive(Debug, Clone, PartialEq)] +pub enum PermissionWaitOutcome { + Replied(PermissionReply), + Cancelled { reason: String }, +} + +#[derive(Debug)] +pub struct PendingPermissionReceiver { + request_id: String, + receiver: oneshot::Receiver, +} + +impl PendingPermissionReceiver { + pub fn request_id(&self) -> &str { + &self.request_id + } + + pub async fn wait(self) -> PermissionWaitOutcome { + self.receiver + .await + .unwrap_or_else(|_| PermissionWaitOutcome::Cancelled { + reason: "Permission request channel closed".to_string(), + }) + } +} + +#[derive(Debug, Clone, PartialEq)] +pub struct PermissionReplyResolution { + pub request: PermissionV2Request, + pub reply: PermissionReply, + pub saved_grants: Vec, + pub resolved_request_ids: Vec, +} + +#[derive(Debug, thiserror::Error)] +pub enum PermissionRequestManagerError { + #[error("Duplicate pending permission request: {0}")] + DuplicateRequest(String), + #[error("Pending permission request not found: {0}")] + RequestNotFound(String), + #[error("Failed to persist permission reply: {0}")] + ReplyStore(#[source] PortError), + #[error("Failed to persist permission audit: {0}")] + AuditStore(#[source] PortError), +} + +#[derive(Debug)] +struct PendingPermission { + request: PermissionV2Request, + sender: oneshot::Sender, +} + +#[derive(Clone)] +pub struct PermissionRequestManager { + pending: Arc>, + operations: Arc>, + audit_store: Arc, + reply_store: Arc, + clock: Arc, +} + +impl std::fmt::Debug for PermissionRequestManager { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("PermissionRequestManager") + .field("pending_count", &self.pending.len()) + .finish_non_exhaustive() + } +} + +impl PermissionRequestManager { + pub fn new( + audit_store: Arc, + reply_store: Arc, + clock: Arc, + ) -> Self { + Self { + pending: Arc::new(DashMap::new()), + operations: Arc::new(Mutex::new(())), + audit_store, + reply_store, + clock, + } + } + + pub async fn register( + &self, + request: PermissionV2Request, + ) -> Result { + let _operation = self.operations.lock().await; + let request_id = request.request_id.clone(); + let (sender, receiver) = oneshot::channel(); + + match self.pending.entry(request_id.clone()) { + Entry::Occupied(_) => { + return Err(PermissionRequestManagerError::DuplicateRequest(request_id)); + } + Entry::Vacant(entry) => { + entry.insert(PendingPermission { + request: request.clone(), + sender, + }); + } + } + + let audit = PermissionAuditRecord { + audit_id: audit_id(&request_id, "requested"), + request, + event: PermissionAuditEvent::Requested, + timestamp_ms: self.clock.now_unix_millis(), + }; + if let Err(error) = self.audit_store.append_permission_audit(audit).await { + self.pending.remove(&request_id); + return Err(PermissionRequestManagerError::AuditStore(error)); + } + + Ok(PendingPermissionReceiver { + request_id, + receiver, + }) + } + + pub fn pending_requests(&self) -> Vec { + let mut requests = self + .pending + .iter() + .map(|entry| entry.request.clone()) + .collect::>(); + requests.sort_by(|left, right| left.request_id.cmp(&right.request_id)); + requests + } + + pub async fn reply( + &self, + request_id: &str, + reply: PermissionReply, + source: PermissionReplySource, + ) -> Result { + let _operation = self.operations.lock().await; + let request = self + .pending + .get(request_id) + .map(|entry| entry.request.clone()) + .ok_or_else(|| { + PermissionRequestManagerError::RequestNotFound(request_id.to_string()) + })?; + let timestamp_ms = self.clock.now_unix_millis(); + let grants = grants_for_reply(&request, &reply, timestamp_ms); + + let resolutions = if matches!(reply, PermissionReply::Reject { .. }) { + let mut requests = self + .pending + .iter() + .filter(|entry| entry.request.session_id == request.session_id) + .map(|entry| entry.request.clone()) + .collect::>(); + requests.sort_by(|left, right| left.request_id.cmp(&right.request_id)); + requests + .into_iter() + .map(|pending_request| { + let pending_reply = if pending_request.request_id == request_id { + reply.clone() + } else { + PermissionReply::Reject { feedback: None } + }; + (pending_request, pending_reply) + }) + .collect::>() + } else { + vec![(request.clone(), reply.clone())] + }; + + let audit = resolutions + .iter() + .map(|(pending_request, pending_reply)| PermissionAuditRecord { + audit_id: audit_id(&pending_request.request_id, "replied"), + request: pending_request.clone(), + event: PermissionAuditEvent::Replied { + reply: pending_reply.clone(), + source, + }, + timestamp_ms, + }) + .collect::>(); + self.reply_store + .commit_permission_reply(grants.clone(), audit) + .await + .map_err(PermissionRequestManagerError::ReplyStore)?; + + let resolved_request_ids = resolutions + .iter() + .map(|(pending_request, _)| pending_request.request_id.clone()) + .collect::>(); + for (pending_request, pending_reply) in resolutions { + if let Some((_, pending)) = self.pending.remove(&pending_request.request_id) { + let _ = pending + .sender + .send(PermissionWaitOutcome::Replied(pending_reply)); + } + } + + Ok(PermissionReplyResolution { + request, + reply, + saved_grants: grants, + resolved_request_ids, + }) + } + + pub async fn cancel_request( + &self, + request_id: &str, + reason: impl Into, + ) -> Result { + let _operation = self.operations.lock().await; + let Some(request) = self + .pending + .get(request_id) + .map(|entry| entry.request.clone()) + else { + return Ok(false); + }; + self.cancel_requests(vec![request], reason.into()).await?; + Ok(true) + } + + pub async fn cancel_session( + &self, + session_id: &str, + reason: impl Into, + ) -> Result, PermissionRequestManagerError> { + let _operation = self.operations.lock().await; + let mut requests = self + .pending + .iter() + .filter(|entry| entry.request.session_id == session_id) + .map(|entry| entry.request.clone()) + .collect::>(); + requests.sort_by(|left, right| left.request_id.cmp(&right.request_id)); + let request_ids = requests + .iter() + .map(|request| request.request_id.clone()) + .collect(); + self.cancel_requests(requests, reason.into()).await?; + Ok(request_ids) + } + + async fn cancel_requests( + &self, + requests: Vec, + reason: String, + ) -> Result<(), PermissionRequestManagerError> { + let timestamp_ms = self.clock.now_unix_millis(); + for request in &requests { + self.audit_store + .append_permission_audit(PermissionAuditRecord { + audit_id: audit_id(&request.request_id, "cancelled"), + request: request.clone(), + event: PermissionAuditEvent::Cancelled { + reason: reason.clone(), + }, + timestamp_ms, + }) + .await + .map_err(PermissionRequestManagerError::AuditStore)?; + } + + for request in requests { + if let Some((_, pending)) = self.pending.remove(&request.request_id) { + let _ = pending.sender.send(PermissionWaitOutcome::Cancelled { + reason: reason.clone(), + }); + } + } + Ok(()) + } +} + +fn grants_for_reply( + request: &PermissionV2Request, + reply: &PermissionReply, + created_at_ms: i64, +) -> Vec { + if !matches!(reply, PermissionReply::Always) { + return Vec::new(); + } + + let mut unique = HashSet::new(); + request + .save_resources + .iter() + .filter(|resource| unique.insert((*resource).clone())) + .map(|resource| PermissionGrant { + project_id: request.project_id.clone(), + action: request.action.clone(), + resource: resource.clone(), + created_at_ms, + }) + .collect() +} + +fn audit_id(request_id: &str, event: &str) -> String { + format!("{request_id}:{event}") +} diff --git a/src/crates/execution/agent-runtime/tests/permission_v2_contracts.rs b/src/crates/execution/agent-runtime/tests/permission_v2_contracts.rs new file mode 100644 index 0000000000..101dd0b109 --- /dev/null +++ b/src/crates/execution/agent-runtime/tests/permission_v2_contracts.rs @@ -0,0 +1,349 @@ +use async_trait::async_trait; +use bitfun_agent_runtime::permission_v2::{ + PermissionRequestManager, PermissionRequestManagerError, PermissionWaitOutcome, +}; +use bitfun_runtime_ports::{ + ClockPort, PermissionAuditRecord, PermissionAuditStorePort, PermissionGrant, + PermissionGrantKey, PermissionGrantStorePort, PermissionReply, PermissionReplySource, + PermissionReplyStorePort, PermissionRequestSource, PermissionRequestSourceKind, + PermissionV2Request, PortError, PortErrorKind, PortResult, RuntimeServiceCapability, + RuntimeServicePort, +}; +use serde_json::Map; +use std::sync::{Arc, Mutex}; + +#[derive(Debug, Default)] +struct RecordingPermissionStore { + grants: Mutex>, + audit: Mutex>, + fail_grants: Mutex, + fail_audit: Mutex, +} + +impl RuntimeServicePort for RecordingPermissionStore { + fn capability(&self) -> RuntimeServiceCapability { + RuntimeServiceCapability::Permission + } +} + +#[async_trait] +impl PermissionGrantStorePort for RecordingPermissionStore { + async fn list_project_grants(&self, project_id: &str) -> PortResult> { + Ok(self + .grants + .lock() + .unwrap() + .iter() + .filter(|grant| grant.project_id == project_id) + .cloned() + .collect()) + } + + async fn add_project_grants(&self, grants: Vec) -> PortResult<()> { + if *self.fail_grants.lock().unwrap() { + return Err(PortError::new( + PortErrorKind::Backend, + "grant store unavailable", + )); + } + let mut stored = self.grants.lock().unwrap(); + for grant in grants { + if !stored.iter().any(|existing| existing.key() == grant.key()) { + stored.push(grant); + } + } + Ok(()) + } + + async fn remove_project_grant(&self, key: PermissionGrantKey) -> PortResult { + let mut stored = self.grants.lock().unwrap(); + let previous_len = stored.len(); + stored.retain(|grant| grant.key() != key); + Ok(stored.len() != previous_len) + } +} + +#[async_trait] +impl PermissionAuditStorePort for RecordingPermissionStore { + async fn append_permission_audit(&self, record: PermissionAuditRecord) -> PortResult<()> { + if *self.fail_audit.lock().unwrap() { + return Err(PortError::new( + PortErrorKind::Backend, + "audit store unavailable", + )); + } + let mut stored = self.audit.lock().unwrap(); + if !stored + .iter() + .any(|existing| existing.audit_id == record.audit_id) + { + stored.push(record); + } + Ok(()) + } + + async fn list_project_permission_audit( + &self, + project_id: &str, + ) -> PortResult> { + Ok(self + .audit + .lock() + .unwrap() + .iter() + .filter(|record| record.request.project_id == project_id) + .cloned() + .collect()) + } +} + +#[async_trait] +impl PermissionReplyStorePort for RecordingPermissionStore { + async fn commit_permission_reply( + &self, + grants: Vec, + audit: Vec, + ) -> PortResult<()> { + if *self.fail_grants.lock().unwrap() { + return Err(PortError::new( + PortErrorKind::Backend, + "reply store unavailable", + )); + } + if *self.fail_audit.lock().unwrap() { + return Err(PortError::new( + PortErrorKind::Backend, + "audit store unavailable", + )); + } + self.add_project_grants(grants).await?; + for record in audit { + self.append_permission_audit(record).await?; + } + Ok(()) + } +} + +#[derive(Debug)] +struct FixedClock(i64); + +impl RuntimeServicePort for FixedClock { + fn capability(&self) -> RuntimeServiceCapability { + RuntimeServiceCapability::Clock + } +} + +impl ClockPort for FixedClock { + fn now_unix_millis(&self) -> i64 { + self.0 + } +} + +fn request(request_id: &str, session_id: &str) -> PermissionV2Request { + PermissionV2Request { + request_id: request_id.to_string(), + project_id: "project-a".to_string(), + session_id: session_id.to_string(), + agent_id: "agentic".to_string(), + action: "edit".to_string(), + resources: vec!["src/lib.rs".to_string()], + save_resources: vec!["src/*".to_string(), "src/*".to_string()], + source: PermissionRequestSource { + kind: PermissionRequestSourceKind::ToolCall, + identity: format!("tool-{request_id}"), + }, + display_metadata: Map::new(), + } +} + +fn manager() -> (PermissionRequestManager, Arc) { + let store = Arc::new(RecordingPermissionStore::default()); + ( + PermissionRequestManager::new(store.clone(), store.clone(), Arc::new(FixedClock(123))), + store, + ) +} + +#[tokio::test] +async fn once_releases_only_the_selected_request_and_records_audit() { + let (manager, store) = manager(); + let receiver = manager + .register(request("request-1", "session-a")) + .await + .expect("register request"); + + let resolution = manager + .reply( + "request-1", + PermissionReply::Once, + PermissionReplySource::User, + ) + .await + .expect("reply once"); + + assert_eq!( + receiver.wait().await, + PermissionWaitOutcome::Replied(PermissionReply::Once) + ); + assert_eq!(resolution.resolved_request_ids, vec!["request-1"]); + assert!(resolution.saved_grants.is_empty()); + assert!(manager.pending_requests().is_empty()); + assert_eq!(store.audit.lock().unwrap().len(), 2); +} + +#[tokio::test] +async fn always_persists_unique_project_grants_without_releasing_other_pending_requests() { + let (manager, store) = manager(); + let receiver = manager + .register(request("request-1", "session-a")) + .await + .expect("register request"); + let other = manager + .register(request("request-2", "session-b")) + .await + .expect("register other request"); + + let resolution = manager + .reply( + "request-1", + PermissionReply::Always, + PermissionReplySource::User, + ) + .await + .expect("reply always"); + + assert_eq!( + receiver.wait().await, + PermissionWaitOutcome::Replied(PermissionReply::Always) + ); + assert_eq!(resolution.saved_grants.len(), 1); + assert_eq!(store.grants.lock().unwrap().len(), 1); + assert_eq!( + manager + .pending_requests() + .iter() + .map(|request| request.request_id.as_str()) + .collect::>(), + vec!["request-2"] + ); + + manager + .cancel_request("request-2", "test cleanup") + .await + .expect("cancel other request"); + assert_eq!( + other.wait().await, + PermissionWaitOutcome::Cancelled { + reason: "test cleanup".to_string() + } + ); +} + +#[tokio::test] +async fn reject_clears_every_pending_request_in_the_same_session_only() { + let (manager, _) = manager(); + let target = manager + .register(request("request-1", "session-a")) + .await + .expect("register target"); + let sibling = manager + .register(request("request-2", "session-a")) + .await + .expect("register sibling"); + let other_session = manager + .register(request("request-3", "session-b")) + .await + .expect("register other session"); + + let reply = PermissionReply::Reject { + feedback: Some("Use a read-only path".to_string()), + }; + let resolution = manager + .reply("request-1", reply.clone(), PermissionReplySource::User) + .await + .expect("reject session"); + + assert_eq!(target.wait().await, PermissionWaitOutcome::Replied(reply)); + assert_eq!( + sibling.wait().await, + PermissionWaitOutcome::Replied(PermissionReply::Reject { feedback: None }) + ); + assert_eq!( + resolution.resolved_request_ids, + vec!["request-1", "request-2"] + ); + assert_eq!(manager.pending_requests().len(), 1); + + manager + .cancel_session("session-b", "disconnected") + .await + .expect("cancel other session"); + assert_eq!( + other_session.wait().await, + PermissionWaitOutcome::Cancelled { + reason: "disconnected".to_string() + } + ); +} + +#[tokio::test] +async fn grant_persistence_failure_keeps_the_request_pending_and_waiting() { + let (manager, store) = manager(); + let _receiver = manager + .register(request("request-1", "session-a")) + .await + .expect("register request"); + *store.fail_grants.lock().unwrap() = true; + + let error = manager + .reply( + "request-1", + PermissionReply::Always, + PermissionReplySource::User, + ) + .await + .expect_err("grant failure must fail closed"); + + assert!(matches!( + error, + PermissionRequestManagerError::ReplyStore(_) + )); + assert_eq!(manager.pending_requests().len(), 1); +} + +#[tokio::test] +async fn audit_persistence_failure_keeps_the_request_pending_and_waiting() { + let (manager, store) = manager(); + let _receiver = manager + .register(request("request-1", "session-a")) + .await + .expect("register request"); + *store.fail_audit.lock().unwrap() = true; + + let error = manager + .reply( + "request-1", + PermissionReply::Once, + PermissionReplySource::User, + ) + .await + .expect_err("audit failure must fail closed"); + + assert!(matches!( + error, + PermissionRequestManagerError::ReplyStore(_) + )); + assert_eq!(manager.pending_requests().len(), 1); +} + +#[tokio::test] +async fn pending_requests_are_process_local_and_not_restored_by_a_new_manager() { + let (manager, store) = manager(); + let _receiver = manager + .register(request("request-1", "session-a")) + .await + .expect("register request"); + + let restarted = PermissionRequestManager::new(store.clone(), store, Arc::new(FixedClock(456))); + assert!(restarted.pending_requests().is_empty()); +} diff --git a/src/crates/services/services-core/Cargo.toml b/src/crates/services/services-core/Cargo.toml index 7214f70b1e..b7c49b8461 100644 --- a/src/crates/services/services-core/Cargo.toml +++ b/src/crates/services/services-core/Cargo.toml @@ -40,6 +40,7 @@ default = ["lsp"] lsp = ["dep:anyhow", "dep:notify", "dep:zip"] markdown = ["dep:serde_yaml"] workspace-runtime = ["dep:anyhow", "dep:async-trait", "dep:bitfun-runtime-ports"] +permission-v2 = ["dep:async-trait", "dep:bitfun-runtime-ports", "bitfun-runtime-ports/permission-v2"] [dev-dependencies] filetime = { workspace = true } diff --git a/src/crates/services/services-core/src/lib.rs b/src/crates/services/services-core/src/lib.rs index 6f08ec5c33..c18613a42b 100644 --- a/src/crates/services/services-core/src/lib.rs +++ b/src/crates/services/services-core/src/lib.rs @@ -12,6 +12,8 @@ pub mod lsp; pub mod managed_runtime; #[cfg(feature = "markdown")] pub mod markdown; +#[cfg(feature = "permission-v2")] +pub mod permission_store; pub mod persistence; pub mod process_manager; pub mod session; diff --git a/src/crates/services/services-core/src/permission_store.rs b/src/crates/services/services-core/src/permission_store.rs new file mode 100644 index 0000000000..17764ea9a6 --- /dev/null +++ b/src/crates/services/services-core/src/permission_store.rs @@ -0,0 +1,288 @@ +//! File-backed project permission grants and append-only audit facts. + +use crate::json_store::JsonFileStore; +use bitfun_runtime_ports::{ + PermissionAuditRecord, PermissionAuditStorePort, PermissionGrant, PermissionGrantKey, + PermissionGrantStorePort, PermissionReplyStorePort, PortError, PortErrorKind, PortResult, + RuntimeServiceCapability, RuntimeServicePort, +}; +use serde::{Deserialize, Serialize}; +use std::path::{Path, PathBuf}; + +const PERMISSION_STORE_SCHEMA_VERSION: u32 = 1; +const PERMISSION_STORE_FILE_NAME: &str = "tool-permissions-v2.json"; + +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +struct PersistedPermissionState { + schema_version: u32, + grants: Vec, + audit: Vec, +} + +impl Default for PersistedPermissionState { + fn default() -> Self { + Self { + schema_version: PERMISSION_STORE_SCHEMA_VERSION, + grants: Vec::new(), + audit: Vec::new(), + } + } +} + +#[derive(Debug, Clone)] +pub struct ProjectPermissionFileStore { + path: PathBuf, + json: JsonFileStore, +} + +impl ProjectPermissionFileStore { + pub fn new(base_dir: impl Into) -> Self { + Self { + path: base_dir.into().join(PERMISSION_STORE_FILE_NAME), + json: JsonFileStore, + } + } + + pub fn path(&self) -> &Path { + &self.path + } + + async fn read_state(&self) -> PortResult { + let state = self + .json + .read_locked_optional::(&self.path) + .await + .map_err(store_error)? + .unwrap_or_default(); + validate_schema(&state)?; + Ok(state) + } + + async fn update_state( + &self, + update: impl FnOnce(&mut PersistedPermissionState) -> PortResult, + ) -> PortResult { + let (result, _) = self + .json + .update_locked(&self.path, PersistedPermissionState::default(), |state| { + validate_schema(state)?; + update(state) + }) + .await + .map_err(store_error)?; + result + } +} + +impl RuntimeServicePort for ProjectPermissionFileStore { + fn capability(&self) -> RuntimeServiceCapability { + RuntimeServiceCapability::Permission + } +} + +#[async_trait::async_trait] +impl PermissionGrantStorePort for ProjectPermissionFileStore { + async fn list_project_grants(&self, project_id: &str) -> PortResult> { + let mut grants = self + .read_state() + .await? + .grants + .into_iter() + .filter(|grant| grant.project_id == project_id) + .collect::>(); + sort_grants(&mut grants); + Ok(grants) + } + + async fn add_project_grants(&self, grants: Vec) -> PortResult<()> { + if grants.is_empty() { + return Ok(()); + } + validate_grants(&grants)?; + + self.update_state(|state| { + for grant in grants { + if !state.grants.iter().any(|existing| { + existing.project_id == grant.project_id + && existing.action == grant.action + && existing.resource == grant.resource + }) { + state.grants.push(grant); + } + } + sort_grants(&mut state.grants); + Ok(()) + }) + .await + } + + async fn remove_project_grant(&self, key: PermissionGrantKey) -> PortResult { + self.update_state(|state| { + let previous_len = state.grants.len(); + state.grants.retain(|grant| grant.key() != key); + Ok(state.grants.len() != previous_len) + }) + .await + } +} + +#[async_trait::async_trait] +impl PermissionReplyStorePort for ProjectPermissionFileStore { + async fn commit_permission_reply( + &self, + grants: Vec, + audit: Vec, + ) -> PortResult<()> { + validate_grants(&grants)?; + if audit.iter().any(|record| { + record.audit_id.trim().is_empty() || record.request.project_id.trim().is_empty() + }) { + return Err(PortError::new( + PortErrorKind::InvalidRequest, + "Permission audit ID and project ID must be non-empty", + )); + } + + self.update_state(|state| { + for record in &audit { + if let Some(existing) = state + .audit + .iter() + .find(|existing| existing.audit_id == record.audit_id) + { + if !same_audit_event(existing, record) { + return Err(audit_conflict(&record.audit_id)); + } + } + } + + for grant in grants { + if !state.grants.iter().any(|existing| { + existing.project_id == grant.project_id + && existing.action == grant.action + && existing.resource == grant.resource + }) { + state.grants.push(grant); + } + } + for record in audit { + if !state + .audit + .iter() + .any(|existing| existing.audit_id == record.audit_id) + { + state.audit.push(record); + } + } + sort_grants(&mut state.grants); + sort_audit(&mut state.audit); + Ok(()) + }) + .await + } +} + +#[async_trait::async_trait] +impl PermissionAuditStorePort for ProjectPermissionFileStore { + async fn append_permission_audit(&self, record: PermissionAuditRecord) -> PortResult<()> { + if record.audit_id.trim().is_empty() || record.request.project_id.trim().is_empty() { + return Err(PortError::new( + PortErrorKind::InvalidRequest, + "Permission audit ID and project ID must be non-empty", + )); + } + + self.update_state(|state| { + if let Some(existing) = state + .audit + .iter() + .find(|existing| existing.audit_id == record.audit_id) + { + if !same_audit_event(existing, &record) { + return Err(audit_conflict(&record.audit_id)); + } + return Ok(()); + } + state.audit.push(record); + sort_audit(&mut state.audit); + Ok(()) + }) + .await + } + + async fn list_project_permission_audit( + &self, + project_id: &str, + ) -> PortResult> { + Ok(self + .read_state() + .await? + .audit + .into_iter() + .filter(|record| record.request.project_id == project_id) + .collect()) + } +} + +fn validate_schema(state: &PersistedPermissionState) -> PortResult<()> { + if state.schema_version != PERMISSION_STORE_SCHEMA_VERSION { + return Err(PortError::new( + PortErrorKind::InvalidRequest, + format!( + "Unsupported permission store schema version: {}", + state.schema_version + ), + )); + } + Ok(()) +} + +fn sort_grants(grants: &mut [PermissionGrant]) { + grants.sort_by(|left, right| { + left.project_id + .cmp(&right.project_id) + .then_with(|| left.action.cmp(&right.action)) + .then_with(|| left.resource.cmp(&right.resource)) + }); +} + +fn sort_audit(audit: &mut [PermissionAuditRecord]) { + audit.sort_by(|left, right| { + left.timestamp_ms + .cmp(&right.timestamp_ms) + .then_with(|| left.audit_id.cmp(&right.audit_id)) + }); +} + +fn validate_grants(grants: &[PermissionGrant]) -> PortResult<()> { + if grants.iter().any(|grant| { + grant.project_id.trim().is_empty() + || grant.action.trim().is_empty() + || grant.resource.trim().is_empty() + }) { + return Err(PortError::new( + PortErrorKind::InvalidRequest, + "Permission grant project, action, and resource must be non-empty", + )); + } + Ok(()) +} + +fn audit_conflict(audit_id: &str) -> PortError { + PortError::new( + PortErrorKind::InvalidRequest, + format!("Permission audit ID already exists with different content: {audit_id}"), + ) +} + +fn same_audit_event(left: &PermissionAuditRecord, right: &PermissionAuditRecord) -> bool { + left.audit_id == right.audit_id && left.request == right.request && left.event == right.event +} + +fn store_error(error: impl std::fmt::Display) -> PortError { + PortError::new( + PortErrorKind::Backend, + format!("Permission store operation failed: {error}"), + ) +} diff --git a/src/crates/services/services-core/tests/permission_store_contracts.rs b/src/crates/services/services-core/tests/permission_store_contracts.rs new file mode 100644 index 0000000000..4dd73420ab --- /dev/null +++ b/src/crates/services/services-core/tests/permission_store_contracts.rs @@ -0,0 +1,180 @@ +#![cfg(feature = "permission-v2")] + +use bitfun_runtime_ports::{ + PermissionAuditEvent, PermissionAuditRecord, PermissionAuditStorePort, PermissionGrant, + PermissionGrantKey, PermissionGrantStorePort, PermissionReply, PermissionReplySource, + PermissionReplyStorePort, PermissionRequestSource, PermissionRequestSourceKind, + PermissionV2Request, +}; +use bitfun_services_core::permission_store::ProjectPermissionFileStore; +use serde_json::Map; + +fn request(request_id: &str, project_id: &str) -> PermissionV2Request { + PermissionV2Request { + request_id: request_id.to_string(), + project_id: project_id.to_string(), + session_id: "session-1".to_string(), + agent_id: "agentic".to_string(), + action: "read".to_string(), + resources: vec!["README.md".to_string()], + save_resources: vec!["README.md".to_string()], + source: PermissionRequestSource { + kind: PermissionRequestSourceKind::ToolCall, + identity: "tool-1".to_string(), + }, + display_metadata: Map::new(), + } +} + +#[tokio::test] +async fn project_grants_are_idempotent_isolated_and_survive_store_recreation() { + let root = tempfile::tempdir().expect("temp permission store"); + let store = ProjectPermissionFileStore::new(root.path()); + let project_a = PermissionGrant { + project_id: "project-a".to_string(), + action: "read".to_string(), + resource: "README.md".to_string(), + created_at_ms: 10, + }; + let project_b = PermissionGrant { + project_id: "project-b".to_string(), + action: "edit".to_string(), + resource: "src/*".to_string(), + created_at_ms: 20, + }; + + store + .add_project_grants(vec![project_a.clone(), project_a.clone(), project_b]) + .await + .expect("persist grants"); + + let reopened = ProjectPermissionFileStore::new(root.path()); + assert_eq!( + reopened + .list_project_grants("project-a") + .await + .expect("list project grants"), + vec![project_a.clone()] + ); + assert_eq!( + reopened + .list_project_grants("project-b") + .await + .expect("list other project grants") + .len(), + 1 + ); + + assert!(reopened + .remove_project_grant(PermissionGrantKey { + project_id: "project-a".to_string(), + action: "read".to_string(), + resource: "README.md".to_string(), + }) + .await + .expect("remove grant")); + assert!(reopened + .list_project_grants("project-a") + .await + .expect("list removed project grants") + .is_empty()); +} + +#[tokio::test] +async fn audit_records_are_idempotent_project_scoped_and_persistent() { + let root = tempfile::tempdir().expect("temp permission store"); + let store = ProjectPermissionFileStore::new(root.path()); + let record = PermissionAuditRecord { + audit_id: "request-1:replied".to_string(), + request: request("request-1", "project-a"), + event: PermissionAuditEvent::Replied { + reply: PermissionReply::Once, + source: PermissionReplySource::User, + }, + timestamp_ms: 100, + }; + + store + .append_permission_audit(record.clone()) + .await + .expect("append audit"); + store + .append_permission_audit(record.clone()) + .await + .expect("repeat audit idempotently"); + store + .append_permission_audit(PermissionAuditRecord { + timestamp_ms: 101, + ..record.clone() + }) + .await + .expect("repeat audit after retry timestamp change"); + store + .append_permission_audit(PermissionAuditRecord { + audit_id: "request-2:requested".to_string(), + request: request("request-2", "project-b"), + event: PermissionAuditEvent::Requested, + timestamp_ms: 90, + }) + .await + .expect("append other project audit"); + + let reopened = ProjectPermissionFileStore::new(root.path()); + assert_eq!( + reopened + .list_project_permission_audit("project-a") + .await + .expect("list project audit"), + vec![record] + ); + assert_eq!( + reopened + .list_project_permission_audit("project-b") + .await + .expect("list other project audit") + .len(), + 1 + ); +} + +#[tokio::test] +async fn reply_transaction_persists_grants_and_audit_in_one_state_update() { + let root = tempfile::tempdir().expect("temp permission store"); + let store = ProjectPermissionFileStore::new(root.path()); + let grant = PermissionGrant { + project_id: "project-a".to_string(), + action: "read".to_string(), + resource: "README.md".to_string(), + created_at_ms: 100, + }; + let audit = PermissionAuditRecord { + audit_id: "request-1:replied".to_string(), + request: request("request-1", "project-a"), + event: PermissionAuditEvent::Replied { + reply: PermissionReply::Always, + source: PermissionReplySource::User, + }, + timestamp_ms: 100, + }; + + store + .commit_permission_reply(vec![grant.clone()], vec![audit.clone()]) + .await + .expect("commit reply transaction"); + + let reopened = ProjectPermissionFileStore::new(root.path()); + assert_eq!( + reopened + .list_project_grants("project-a") + .await + .expect("list committed grants"), + vec![grant] + ); + assert_eq!( + reopened + .list_project_permission_audit("project-a") + .await + .expect("list committed audit"), + vec![audit] + ); +} From 269afe851c5f415ec48827d3dd03461bce6a1f41 Mon Sep 17 00:00:00 2001 From: wsp Date: Sat, 18 Jul 2026 15:20:46 +0800 Subject: [PATCH 04/34] feat(permission): add response API and UI projection - expose pending permission requests, lifecycle events, and respond_permission through the runtime SDK - wire the shared permission request manager into Desktop and CLI runtimes - add Desktop commands and Web UI for listing, presenting, and replying to permission requests - add interactive and non-interactive CLI permission handling - preserve the legacy tool confirmation gate and response APIs --- src/apps/cli/src/chat_state.rs | 4 +- src/apps/cli/src/modes/chat.rs | 4 +- src/apps/cli/src/modes/chat/input.rs | 27 ++- src/apps/cli/src/modes/chat/run.rs | 53 +++++ src/apps/cli/src/modes/exec.rs | 94 +++++++- src/apps/cli/src/ui/chat/render.rs | 4 +- src/apps/cli/src/ui/chat/state.rs | 2 +- src/apps/cli/src/ui/permission.rs | 225 +++++++++++++++++- src/apps/desktop/src/api/agentic_api.rs | 82 ++++++- .../src/api/remote_workspace_policy.rs | 12 + src/apps/desktop/src/lib.rs | 3 + src/apps/desktop/src/runtime/mod.rs | 56 ++++- src/crates/assembly/core/Cargo.toml | 3 +- .../assembly/core/src/product_runtime.rs | 52 ++++ .../core/src/service_agent_runtime.rs | 17 +- .../product-domains/src/tool_permissions.rs | 18 ++ src/crates/contracts/runtime-ports/src/lib.rs | 2 +- .../agent-runtime/src/permission_v2.rs | 180 +++++++++++++- .../execution/agent-runtime/src/runtime.rs | 51 ++++ src/crates/execution/agent-runtime/src/sdk.rs | 47 +++- .../modern/ModernFlowChatContainer.tsx | 14 ++ .../modern/PermissionRequestPanel.scss | 96 ++++++++ .../modern/PermissionRequestPanel.tsx | 85 +++++++ .../modern/usePermissionRequests.ts | 64 +++++ .../api/service-api/AgentAPI.test.ts | 12 + .../api/service-api/AgentAPI.ts | 61 +++++ src/web-ui/src/locales/en-US/flow-chat.json | 11 + src/web-ui/src/locales/zh-CN/flow-chat.json | 11 + src/web-ui/src/locales/zh-TW/flow-chat.json | 11 + 29 files changed, 1273 insertions(+), 28 deletions(-) create mode 100644 src/web-ui/src/flow_chat/components/modern/PermissionRequestPanel.scss create mode 100644 src/web-ui/src/flow_chat/components/modern/PermissionRequestPanel.tsx create mode 100644 src/web-ui/src/flow_chat/components/modern/usePermissionRequests.ts diff --git a/src/apps/cli/src/chat_state.rs b/src/apps/cli/src/chat_state.rs index a14b5cd0b6..85398c5346 100644 --- a/src/apps/cli/src/chat_state.rs +++ b/src/apps/cli/src/chat_state.rs @@ -11,7 +11,7 @@ use bitfun_agent_runtime::sdk::{SessionTranscript, TranscriptContent, Transcript use bitfun_agent_tools::effective_tool_invocation; use bitfun_events::ToolEventData; -use crate::ui::permission::PermissionPrompt; +use crate::ui::permission::{PermissionPrompt, PermissionV2Prompt}; use crate::ui::question::QuestionPrompt; // ============ Display Status Types ============ @@ -327,6 +327,7 @@ pub(crate) struct ChatState { // -- Permission state -- /// Current pending permission prompt (if a tool needs user confirmation) pub permission_prompt: Option, + pub permission_v2_prompt: Option, // -- Question state -- /// Current pending question prompt (if AskUserQuestion tool is waiting for answers) @@ -354,6 +355,7 @@ impl ChatState { tool_index: HashMap::new(), is_processing: false, permission_prompt: None, + permission_v2_prompt: None, question_prompt: None, } } diff --git a/src/apps/cli/src/modes/chat.rs b/src/apps/cli/src/modes/chat.rs index 806fc81c2e..fbdd7034c0 100644 --- a/src/apps/cli/src/modes/chat.rs +++ b/src/apps/cli/src/modes/chat.rs @@ -38,7 +38,9 @@ use crate::ui::mcp_add_dialog::McpAddAction; use crate::ui::mcp_selector::McpItem; use crate::ui::model_config_form::{ModelFormAction, ModelFormResult}; use crate::ui::model_selector::ModelItem; -use crate::ui::permission::{PermissionAction, ALLOW_ALWAYS_RUNTIME_SCOPE}; +use crate::ui::permission::{ + PermissionAction, PermissionV2Action, PermissionV2Prompt, ALLOW_ALWAYS_RUNTIME_SCOPE, +}; use crate::ui::provider_selector::ProviderSelection; use crate::ui::question::QuestionAction; use crate::ui::session_selector::{SessionAction, SessionItem}; diff --git a/src/apps/cli/src/modes/chat/input.rs b/src/apps/cli/src/modes/chat/input.rs index 23f5a0bdad..26f48f72fc 100644 --- a/src/apps/cli/src/modes/chat/input.rs +++ b/src/apps/cli/src/modes/chat/input.rs @@ -16,7 +16,31 @@ impl ChatMode { return self.dispatch_action(action, modal_state, chat_view, chat_state, rt_handle); } - // ── Permission prompt intercepts all keys when active ── + if let Some(ref mut prompt) = chat_state.permission_v2_prompt { + match prompt.handle_key_event(key) { + PermissionV2Action::Reply(reply) => { + let request_id = prompt.request.request_id.clone(); + let runtime = self.runtime.agent_runtime().clone(); + let result = tokio::task::block_in_place(|| { + rt_handle.block_on(runtime.respond_permission(&request_id, reply)) + }); + match result { + Ok(()) => { + chat_state.permission_v2_prompt = None; + chat_view.set_status(Some("Permission response sent".to_string())); + } + Err(error) => { + tracing::error!("Failed to respond to permission request: {error}"); + chat_view.set_status(Some(format!("Error: {error}"))); + } + } + } + PermissionV2Action::None => {} + } + return Ok(None); + } + + // ── Legacy permission prompt intercepts all keys when active ── if let Some(ref mut prompt) = chat_state.permission_prompt { let action = prompt.handle_key_event(key); match action { @@ -598,6 +622,7 @@ impl ChatMode { } else if context.chat_view.login_form_visible() { context.chat_view.login_form_insert_paste(&text); } else if context.chat_state.permission_prompt.is_none() + && context.chat_state.permission_v2_prompt.is_none() && context.chat_state.question_prompt.is_none() && !context.this.any_popup_visible(context.chat_view) { diff --git a/src/apps/cli/src/modes/chat/run.rs b/src/apps/cli/src/modes/chat/run.rs index 77912420a0..7bf9acfc03 100644 --- a/src/apps/cli/src/modes/chat/run.rs +++ b/src/apps/cli/src/modes/chat/run.rs @@ -151,6 +151,19 @@ impl ChatMode { } let mut event_rx = self.agent.event_source().subscribe(); + let mut permission_rx = self + .runtime + .agent_runtime() + .subscribe_permission_requests() + .ok(); + if let Ok(pending) = self.runtime.agent_runtime().pending_permission_requests() { + if let Some(request) = pending + .into_iter() + .find(|request| request.session_id == session_id) + { + chat_state.permission_v2_prompt = Some(PermissionV2Prompt::new(request)); + } + } // Send initial prompt if provided (from startup page input) if let Some(prompt) = self.initial_prompt.take() { @@ -216,6 +229,46 @@ impl ChatMode { needs_redraw = true; } + if let Some(receiver) = permission_rx.as_mut() { + for _ in 0..4 { + match receiver.try_recv() { + Ok(bitfun_agent_runtime::sdk::PermissionRequestEvent::Asked { + request, + }) if request.session_id == session_id => { + if chat_state.permission_v2_prompt.is_none() { + chat_state.permission_v2_prompt = + Some(PermissionV2Prompt::new(request)); + needs_redraw = true; + } + } + Ok(bitfun_agent_runtime::sdk::PermissionRequestEvent::Replied { + request_id, + .. + }) + | Ok(bitfun_agent_runtime::sdk::PermissionRequestEvent::Cancelled { + request_id, + .. + }) => { + if chat_state + .permission_v2_prompt + .as_ref() + .is_some_and(|prompt| prompt.request.request_id == request_id) + { + chat_state.permission_v2_prompt = None; + needs_redraw = true; + } + } + Err(TryRecvError::Empty) => break, + Err(TryRecvError::Lagged(_)) => continue, + Err(TryRecvError::Closed) => { + permission_rx = None; + break; + } + Ok(_) => {} + } + } + } + let mut external_source_closed = false; if let Some(receiver) = external_source_rx.as_mut() { let mut latest = None; diff --git a/src/apps/cli/src/modes/exec.rs b/src/apps/cli/src/modes/exec.rs index b99a65a3c5..dc11268944 100644 --- a/src/apps/cli/src/modes/exec.rs +++ b/src/apps/cli/src/modes/exec.rs @@ -5,12 +5,13 @@ use anyhow::Result; use clap::ValueEnum; use serde::Serialize; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet, VecDeque}; use std::io::Write; use std::path::PathBuf; use std::sync::Arc; use std::time::Duration; +use bitfun_agent_runtime::sdk::{PermissionReply, PermissionReplySource, PermissionRequestEvent}; use bitfun_agent_tools::effective_tool_invocation; use bitfun_events::{AgenticEvent, ToolEventIdentity}; use tokio::time::{sleep, Instant}; @@ -309,7 +310,7 @@ pub(crate) struct ExecMode { message: String, agent_type: String, agent: Arc, - _runtime: Arc, + runtime: Arc, workspace_path: Option, /// None: no patch output, Some("-"): output to stdout, Some(path): save to file output_patch: Option, @@ -344,7 +345,7 @@ impl ExecMode { message, agent_type, agent, - _runtime: runtime, + runtime, workspace_path, output_patch, output_format, @@ -557,7 +558,6 @@ impl ExecMode { }; tracing::info!(session_id = %session_id, "Session ready"); let mut event_rx = self.agent.event_source().subscribe(); - self.print_text(|| { eprintln!("Executing: {}", self.message); eprintln!(); @@ -586,6 +586,78 @@ impl ExecMode { }; tracing::info!(session_id = %session_id, turn_id = %turn_id, "Message sent"); + let mut permission_rx = self + .runtime + .agent_runtime() + .subscribe_permission_requests() + .map_err(|error| anyhow::anyhow!(error.into_message()))?; + let permission_runtime = self.runtime.agent_runtime().clone(); + let permission_session_id = session_id.clone(); + let permission_mode = self.approval_mode; + let (permission_action_tx, mut permission_action_rx) = tokio::sync::mpsc::channel(1); + let permission_task = tokio::spawn(async move { + let mut initial_requests = permission_runtime + .pending_permission_requests() + .unwrap_or_default() + .into_iter() + .collect::>(); + let mut handled_request_ids = HashSet::new(); + loop { + let request = if let Some(request) = initial_requests.pop_front() { + request + } else { + let Ok(event) = permission_rx.recv().await else { + break; + }; + let PermissionRequestEvent::Asked { request } = event else { + continue; + }; + request + }; + if request.session_id != permission_session_id { + continue; + } + if !handled_request_ids.insert(request.request_id.clone()) { + continue; + } + let (reply, action_required, source) = match permission_mode { + ExecApprovalMode::Auto => ( + PermissionReply::Once, + false, + PermissionReplySource::AutoApprove, + ), + ExecApprovalMode::Reject => ( + PermissionReply::Reject { + feedback: Some( + "Non-interactive execution requires an explicit permission policy" + .to_string(), + ), + }, + true, + PermissionReplySource::System, + ), + }; + if let Err(error) = permission_runtime + .respond_permission_with_source(&request.request_id, reply, source) + .await + { + let _ = permission_action_tx + .send(format!("Failed to respond to permission request: {error}")) + .await; + break; + } + if action_required { + let _ = permission_action_tx + .send(format!( + "action-required: permission needed for {} ({})", + request.action, request.request_id + )) + .await; + break; + } + } + }); + // Observe the shared Agentic event stream without consuming other clients' events. let mut total_tool_calls = 0usize; let mut subagent_parent_turns: HashMap = HashMap::new(); @@ -633,6 +705,18 @@ impl ExecMode { break 'event_loop; } }, + permission = permission_action_rx.recv() => { + let message = permission.unwrap_or_else(|| { + "Permission response channel closed before execution settled".to_string() + }); + if let Err(error) = self.agent.cancel_current_turn().await { + tracing::error!("Failed to cancel after permission action: {error}"); + } + terminal_status = Some(ExecTerminalStatus::Error); + terminal_message = Some(message.clone()); + terminal_outcome = Some(Err(anyhow::anyhow!(message))); + break 'event_loop; + } signal = tokio::signal::ctrl_c() => { let interrupted = signal.is_ok(); let mut message = match signal { @@ -1028,6 +1112,8 @@ impl ExecMode { } } + permission_task.abort(); + if let Err(error) = self.wait_for_turn_settlement(&session_id, &turn_id).await { let message = match terminal_message.take() { Some(existing) => format!("{existing}; {error}"), diff --git a/src/apps/cli/src/ui/chat/render.rs b/src/apps/cli/src/ui/chat/render.rs index 08f98d4d7e..f4121115d8 100644 --- a/src/apps/cli/src/ui/chat/render.rs +++ b/src/apps/cli/src/ui/chat/render.rs @@ -107,7 +107,9 @@ impl ChatView { self.render_shortcuts(frame, chunks[4], chat_state); // Render permission overlay on top of messages area if active (highest priority) - if let Some(ref prompt) = chat_state.permission_prompt { + if let Some(ref prompt) = chat_state.permission_v2_prompt { + render_permission_v2_overlay(frame, prompt, &self.theme, chunks[1]); + } else if let Some(ref prompt) = chat_state.permission_prompt { render_permission_overlay(frame, prompt, &self.theme, chunks[1]); } // Render question overlay (second priority, only if no permission prompt) diff --git a/src/apps/cli/src/ui/chat/state.rs b/src/apps/cli/src/ui/chat/state.rs index d572952a42..bb585d1d6a 100644 --- a/src/apps/cli/src/ui/chat/state.rs +++ b/src/apps/cli/src/ui/chat/state.rs @@ -17,7 +17,7 @@ use super::mcp_add_dialog::{McpAddAction, McpAddDialogState}; use super::mcp_selector::{McpAction, McpItem, McpSelectorState}; use super::model_config_form::{ModelConfigFormState, ModelFormAction}; use super::model_selector::{ModelItem, ModelSelectorState}; -use super::permission::render_permission_overlay; +use super::permission::{render_permission_overlay, render_permission_v2_overlay}; use super::provider_selector::{ProviderSelection, ProviderSelectorState}; use super::question::render_question_overlay; use super::session_selector::{SessionAction, SessionItem, SessionSelectorState}; diff --git a/src/apps/cli/src/ui/permission.rs b/src/apps/cli/src/ui/permission.rs index ac33f42e46..93b928f722 100644 --- a/src/apps/cli/src/ui/permission.rs +++ b/src/apps/cli/src/ui/permission.rs @@ -16,9 +16,86 @@ use ratatui::{ use super::string_utils::truncate_str; use super::theme::{tool_icon, StyleKind, Theme}; +use bitfun_agent_runtime::sdk::{PermissionReply, PermissionV2Request}; pub(crate) const ALLOW_ALWAYS_RUNTIME_SCOPE: &str = "until this CLI runtime exits"; +#[derive(Debug, Clone)] +pub(crate) struct PermissionV2Prompt { + pub(crate) request: PermissionV2Request, + pub(crate) selected_option: usize, + reject_feedback: String, + editing_reject_feedback: bool, +} + +#[derive(Debug, Clone, PartialEq)] +pub(crate) enum PermissionV2Action { + None, + Reply(PermissionReply), +} + +impl PermissionV2Prompt { + pub(crate) fn new(request: PermissionV2Request) -> Self { + Self { + request, + selected_option: 0, + reject_feedback: String::new(), + editing_reject_feedback: false, + } + } + + pub(crate) fn handle_key_event(&mut self, key: KeyEvent) -> PermissionV2Action { + if key.kind != KeyEventKind::Press && key.kind != KeyEventKind::Repeat { + return PermissionV2Action::None; + } + if self.editing_reject_feedback { + return match (key.code, key.modifiers) { + (KeyCode::Enter, _) => PermissionV2Action::Reply(PermissionReply::Reject { + feedback: match self.reject_feedback.trim() { + "" => None, + feedback => Some(feedback.to_string()), + }, + }), + (KeyCode::Esc, _) => { + self.editing_reject_feedback = false; + PermissionV2Action::None + } + (KeyCode::Backspace, _) => { + self.reject_feedback.pop(); + PermissionV2Action::None + } + (KeyCode::Char(character), KeyModifiers::NONE | KeyModifiers::SHIFT) + if !character.is_control() => + { + self.reject_feedback.push(character); + PermissionV2Action::None + } + _ => PermissionV2Action::None, + }; + } + match key.code { + KeyCode::Left | KeyCode::Char('h') => { + self.selected_option = self.selected_option.saturating_sub(1); + PermissionV2Action::None + } + KeyCode::Right | KeyCode::Char('l') => { + self.selected_option = (self.selected_option + 1).min(2); + PermissionV2Action::None + } + KeyCode::Esc => PermissionV2Action::Reply(PermissionReply::Reject { feedback: None }), + KeyCode::Enter => match self.selected_option { + 0 => PermissionV2Action::Reply(PermissionReply::Once), + 1 => PermissionV2Action::Reply(PermissionReply::Always), + _ => { + self.editing_reject_feedback = true; + PermissionV2Action::None + } + }, + _ => PermissionV2Action::None, + } + } +} + fn allow_always_confirmation_text(tool_name: &str) -> String { format!("This will auto-approve '{tool_name}' tool calls {ALLOW_ALWAYS_RUNTIME_SCOPE}.") } @@ -191,6 +268,98 @@ impl PermissionPrompt { // ============ Rendering ============ +pub(super) fn render_permission_v2_overlay( + frame: &mut Frame, + prompt: &PermissionV2Prompt, + theme: &Theme, + area: Rect, +) { + let overlay_height = 11u16.min(area.height.saturating_sub(2)); + let overlay_area = Rect { + x: area.x, + y: area.y + area.height.saturating_sub(overlay_height), + width: area.width, + height: overlay_height, + }; + frame.render_widget(Clear, overlay_area); + let chunks = Layout::default() + .direction(Direction::Vertical) + .constraints([Constraint::Min(4), Constraint::Length(2)]) + .split(overlay_area); + let content_block = Block::default() + .borders(Borders::LEFT | Borders::TOP | Borders::RIGHT) + .border_style(Style::default().fg(theme.warning)) + .style(Style::default().bg(theme.background_panel)); + let inner = content_block.inner(chunks[0]); + frame.render_widget(content_block, chunks[0]); + + let request = &prompt.request; + let resources = request + .resources + .iter() + .map(|resource| truncate_str(resource, 80)) + .collect::>() + .join(", "); + let save_scope = if request.save_resources.is_empty() { + "No remembered scope".to_string() + } else { + format!( + "Always saves {} project resource(s)", + request.save_resources.len() + ) + }; + let risk = request + .display_metadata + .get("riskDescription") + .or_else(|| request.display_metadata.get("risk")) + .and_then(serde_json::Value::as_str) + .unwrap_or("No additional risk information"); + let lines = vec![ + Line::from(Span::styled( + "Permission required", + Style::default() + .fg(theme.warning) + .add_modifier(Modifier::BOLD), + )), + Line::from(format!( + "Action: {} Source: {:?}:{}", + request.action, request.source.kind, request.source.identity + )), + Line::from(format!("Resources: {resources}")), + Line::from(format!("Project: {} {save_scope}", request.project_id)), + Line::from(format!("Risk: {}", truncate_str(risk, 100))), + if prompt.editing_reject_feedback { + Line::from(format!( + "Rejection feedback (optional): {}_", + prompt.reject_feedback + )) + } else { + Line::from("") + }, + ]; + frame.render_widget(Paragraph::new(lines).wrap(Wrap { trim: true }), inner); + render_button_bar( + frame, + chunks[1], + theme, + if prompt.editing_reject_feedback { + &["Submit reject"] + } else { + &["Allow once", "Always allow", "Reject"] + }, + if prompt.editing_reject_feedback { + 0 + } else { + prompt.selected_option + }, + if prompt.editing_reject_feedback { + "Enter submit Esc back" + } else { + "\u{21c6} select Enter confirm Esc reject" + }, + ); +} + /// Render the permission overlay on top of the message area. /// /// This renders at the bottom of the given area, taking up a fixed height. @@ -565,7 +734,29 @@ fn extract_first_param(params: &serde_json::Value) -> String { #[cfg(test)] mod tests { - use super::allow_always_confirmation_text; + use super::{allow_always_confirmation_text, PermissionV2Action, PermissionV2Prompt}; + use bitfun_agent_runtime::sdk::{ + PermissionReply, PermissionRequestSource, PermissionRequestSourceKind, PermissionV2Request, + }; + use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; + use serde_json::Map; + + fn request() -> PermissionV2Request { + PermissionV2Request { + request_id: "request-1".to_string(), + project_id: "project-1".to_string(), + session_id: "session-1".to_string(), + agent_id: "agentic".to_string(), + action: "edit".to_string(), + resources: vec!["src/main.rs".to_string()], + save_resources: vec!["src/main.rs".to_string()], + source: PermissionRequestSource { + kind: PermissionRequestSourceKind::ToolCall, + identity: "write_file".to_string(), + }, + display_metadata: Map::new(), + } + } #[test] fn allow_always_copy_describes_cli_runtime_lifetime() { @@ -574,4 +765,36 @@ mod tests { assert!(text.contains("until this CLI runtime exits")); assert!(!text.contains("session")); } + + #[test] + fn v2_prompt_returns_project_always_reply_without_using_legacy_runtime_scope() { + let mut prompt = PermissionV2Prompt::new(request()); + prompt.handle_key_event(KeyEvent::new(KeyCode::Right, KeyModifiers::NONE)); + + assert_eq!( + prompt.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)), + PermissionV2Action::Reply(PermissionReply::Always) + ); + } + + #[test] + fn v2_prompt_collects_optional_rejection_feedback() { + let mut prompt = PermissionV2Prompt::new(request()); + prompt.handle_key_event(KeyEvent::new(KeyCode::Right, KeyModifiers::NONE)); + prompt.handle_key_event(KeyEvent::new(KeyCode::Right, KeyModifiers::NONE)); + assert_eq!( + prompt.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)), + PermissionV2Action::None + ); + for character in "read only".chars() { + prompt.handle_key_event(KeyEvent::new(KeyCode::Char(character), KeyModifiers::NONE)); + } + + assert_eq!( + prompt.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)), + PermissionV2Action::Reply(PermissionReply::Reject { + feedback: Some("read only".to_string()), + }) + ); + } } diff --git a/src/apps/desktop/src/api/agentic_api.rs b/src/apps/desktop/src/api/agentic_api.rs index 84bcdabd87..dc68632b86 100644 --- a/src/apps/desktop/src/api/agentic_api.rs +++ b/src/apps/desktop/src/api/agentic_api.rs @@ -14,7 +14,7 @@ use crate::startup_trace::DesktopStartupTrace; use bitfun_agent_runtime::sdk::{ AgentDialogTurnRequest, AgentInputAttachment, AgentSessionModelUpdateRequest, AgentSubmissionSource, AgentToolConfirmationRequest, AgentToolRejectionRequest, - AgentTurnCancellationRequest, + AgentTurnCancellationRequest, PermissionReply, PermissionV2Request, }; use bitfun_core::agentic::agents::AgentSource; use bitfun_core::agentic::coordination::{ @@ -723,6 +723,66 @@ pub struct RejectToolRequest { pub reason: Option, } +#[derive(Debug, Clone, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct PermissionResponseRequest { + pub request_id: String, + pub reply: PermissionReplyKind, + pub feedback: Option, +} + +#[derive(Debug, Clone, Copy, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum PermissionReplyKind { + Once, + Always, + Reject, +} + +fn permission_reply(request: PermissionResponseRequest) -> PermissionReply { + match request.reply { + PermissionReplyKind::Once => PermissionReply::Once, + PermissionReplyKind::Always => PermissionReply::Always, + PermissionReplyKind::Reject => PermissionReply::Reject { + feedback: request.feedback, + }, + } +} + +#[tauri::command] +pub fn list_pending_permission_requests( + runtime: State<'_, DesktopRuntimeContext>, +) -> Result, String> { + runtime + .agent_runtime() + .pending_permission_requests() + .map_err(|error| error.into_message()) +} + +#[tauri::command] +pub fn subscribe_permission_requests( + app: AppHandle, + runtime: State<'_, DesktopRuntimeContext>, +) -> Result<(), String> { + runtime + .start_permission_event_forwarding(app) + .map_err(|error| error.into_message()) +} + +#[tauri::command] +pub async fn respond_permission( + runtime: State<'_, DesktopRuntimeContext>, + request: PermissionResponseRequest, +) -> Result<(), String> { + let request_id = request.request_id.clone(); + let reply = permission_reply(request); + runtime + .agent_runtime() + .respond_permission(&request_id, reply) + .await + .map_err(|error| error.into_message()) +} + #[derive(Debug, Deserialize)] #[serde(rename_all = "camelCase")] pub struct GenerateSessionTitleRequest { @@ -2593,6 +2653,26 @@ mod tests { ); } + #[test] + fn permission_response_dto_uses_stable_camel_case_wire_shape() { + let request: PermissionResponseRequest = serde_json::from_value(json!({ + "requestId": "permission-1", + "reply": "reject", + "feedback": "Use a read-only path" + })) + .expect("permission response request"); + + assert_eq!(request.request_id, "permission-1"); + assert!(matches!(request.reply, PermissionReplyKind::Reject)); + assert_eq!(request.feedback.as_deref(), Some("Use a read-only path")); + assert_eq!( + permission_reply(request), + PermissionReply::Reject { + feedback: Some("Use a read-only path".to_string()), + } + ); + } + #[test] fn desktop_interaction_dtos_keep_existing_camel_case_shape() { let cancel: CancelDialogTurnRequest = serde_json::from_value(json!({ diff --git a/src/apps/desktop/src/api/remote_workspace_policy.rs b/src/apps/desktop/src/api/remote_workspace_policy.rs index 6edf3191ca..8b68e631cb 100644 --- a/src/apps/desktop/src/api/remote_workspace_policy.rs +++ b/src/apps/desktop/src/api/remote_workspace_policy.rs @@ -718,6 +718,10 @@ pub const REMOTE_WORKSPACE_COMMAND_POLICIES: &[(&str, RemoteWorkspacePolicy)] = "list_background_command_activities", RemoteWorkspacePolicy::LegacyUnaudited, ), + ( + "list_pending_permission_requests", + RemoteWorkspacePolicy::WorkspaceAgnostic, + ), ("list_cron_jobs", RemoteWorkspacePolicy::LegacyUnaudited), ( "list_directory_files", @@ -1079,6 +1083,10 @@ pub const REMOTE_WORKSPACE_COMMAND_POLICIES: &[(&str, RemoteWorkspacePolicy)] = ), ("reject_file", RemoteWorkspacePolicy::LegacyUnaudited), ("reject_operation", RemoteWorkspacePolicy::LegacyUnaudited), + ( + "respond_permission", + RemoteWorkspacePolicy::WorkspaceAgnostic, + ), ( "reject_tool_execution", RemoteWorkspacePolicy::LegacyUnaudited, @@ -1434,6 +1442,10 @@ pub const REMOTE_WORKSPACE_COMMAND_POLICIES: &[(&str, RemoteWorkspacePolicy)] = RemoteWorkspacePolicy::LegacyUnaudited, ), ("start_dialog_turn", RemoteWorkspacePolicy::LegacyUnaudited), + ( + "subscribe_permission_requests", + RemoteWorkspacePolicy::WorkspaceAgnostic, + ), ("start_file_watch", RemoteWorkspacePolicy::LegacyUnaudited), ( "start_mcp_remote_oauth", diff --git a/src/apps/desktop/src/lib.rs b/src/apps/desktop/src/lib.rs index e144064f9e..99d40aeea2 100644 --- a/src/apps/desktop/src/lib.rs +++ b/src/apps/desktop/src/lib.rs @@ -918,6 +918,9 @@ pub async fn run() { webdriver_bridge_result, get_startup_native_trace, api::agentic_api::list_sessions, + api::agentic_api::list_pending_permission_requests, + api::agentic_api::subscribe_permission_requests, + api::agentic_api::respond_permission, api::agentic_api::confirm_tool_execution, api::agentic_api::reject_tool_execution, api::agentic_api::cancel_tool, diff --git a/src/apps/desktop/src/runtime/mod.rs b/src/apps/desktop/src/runtime/mod.rs index ed81f548dd..b642df45da 100644 --- a/src/apps/desktop/src/runtime/mod.rs +++ b/src/apps/desktop/src/runtime/mod.rs @@ -1,8 +1,10 @@ +use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; use bitfun_agent_runtime::sdk::{ AgentDialogTurnPort, AgentInteractionResponsePort, AgentRuntime, AgentRuntimeBuilder, - AgentSessionModelPort, AgentSubmissionPort, AgentTurnCancellationPort, RuntimeBuildError, + AgentSessionModelPort, AgentSubmissionPort, AgentTurnCancellationPort, PermissionRequestEvent, + RuntimeBuildError, }; use bitfun_core::agentic::coordination::{ConversationCoordinator, DialogScheduler}; @@ -14,6 +16,7 @@ use bitfun_core::agentic::coordination::{ConversationCoordinator, DialogSchedule /// Desktop delivery profile or its product services have been assembled. pub struct DesktopRuntimeContext { agent_runtime: AgentRuntime, + permission_events_started: AtomicBool, } impl DesktopRuntimeContext { @@ -32,14 +35,63 @@ impl DesktopRuntimeContext { .with_dialog_turn_port(dialog_turn) .with_cancellation_port(cancellation) .with_interaction_response_port(interaction_response) + .with_permission_request_manager( + bitfun_core::product_runtime::core_permission_request_manager() + .map_err(RuntimeBuildError::PermissionRequestManagerUnavailable)?, + ) .build()?; - Ok(Self { agent_runtime }) + Ok(Self { + agent_runtime, + permission_events_started: AtomicBool::new(false), + }) } pub(crate) fn agent_runtime(&self) -> &AgentRuntime { &self.agent_runtime } + + pub(crate) fn start_permission_event_forwarding( + &self, + app: tauri::AppHandle, + ) -> Result<(), bitfun_agent_runtime::sdk::RuntimeError> { + if self.permission_events_started.swap(true, Ordering::AcqRel) { + return Ok(()); + } + + let mut receiver = match self.agent_runtime.subscribe_permission_requests() { + Ok(receiver) => receiver, + Err(error) => { + self.permission_events_started + .store(false, Ordering::Release); + return Err(error); + } + }; + let runtime = self.agent_runtime.clone(); + tauri::async_runtime::spawn(async move { + use tauri::Emitter; + + loop { + match receiver.recv().await { + Ok(event) => { + let _ = app.emit("permission://event", event); + } + Err(tokio::sync::broadcast::error::RecvError::Lagged(_)) => { + if let Ok(requests) = runtime.pending_permission_requests() { + for request in requests { + let _ = app.emit( + "permission://event", + PermissionRequestEvent::Asked { request }, + ); + } + } + } + Err(tokio::sync::broadcast::error::RecvError::Closed) => break, + } + } + }); + Ok(()) + } } #[cfg(test)] diff --git a/src/crates/assembly/core/Cargo.toml b/src/crates/assembly/core/Cargo.toml index bec0e5b7d5..2412858a85 100644 --- a/src/crates/assembly/core/Cargo.toml +++ b/src/crates/assembly/core/Cargo.toml @@ -99,6 +99,7 @@ bitfun-services-core = { path = "../../services/services-core", default-features "lsp", "markdown", "workspace-runtime", + "permission-v2", ] } # Integration service owner crate @@ -138,7 +139,7 @@ bitfun-relay-service = { path = "../../services/relay-service", optional = true # Event layer dependency (lowest layer) bitfun-core-types = { path = "../../contracts/core-types" } bitfun-events = { path = "../../contracts/events" } -bitfun-runtime-ports = { path = "../../contracts/runtime-ports" } +bitfun-runtime-ports = { path = "../../contracts/runtime-ports", features = ["permission-v2"] } bitfun-runtime-services = { path = "../../execution/runtime-services" } # Reviewed product-full plugin composition root. diff --git a/src/crates/assembly/core/src/product_runtime.rs b/src/crates/assembly/core/src/product_runtime.rs index e7966219ad..7c20c941ac 100644 --- a/src/crates/assembly/core/src/product_runtime.rs +++ b/src/crates/assembly/core/src/product_runtime.rs @@ -8,11 +8,16 @@ mod runtime_services; use std::path::{Path, PathBuf}; use std::sync::Arc; +use std::sync::OnceLock; +use std::time::{SystemTime, UNIX_EPOCH}; +use bitfun_agent_runtime::permission_v2::PermissionRequestManager; use bitfun_agent_runtime::sdk::{AgentEventSource, AgentRuntime}; use bitfun_harness::HarnessRegistry; +use bitfun_runtime_ports::{ClockPort, RuntimeServiceCapability, RuntimeServicePort}; use bitfun_runtime_ports::{SessionStoragePathRequest, SessionStorePort, SessionViewRestoreTiming}; use bitfun_runtime_services::RuntimeServices; +use bitfun_services_core::permission_store::ProjectPermissionFileStore; use crate::agentic::coordination::{ ConversationCoordinator, DialogScheduler, SessionMaintenancePermit, @@ -36,6 +41,53 @@ use crate::util::errors::{BitFunError, BitFunResult}; pub use bitfun_product_capabilities::ProductRuntimeAssembly as CoreProductRuntimeAssembly; pub use runtime_services::CoreRuntimeServicesProvider; +#[derive(Debug, Clone, Copy, Default)] +struct SystemPermissionClock; + +impl RuntimeServicePort for SystemPermissionClock { + fn capability(&self) -> RuntimeServiceCapability { + RuntimeServiceCapability::Clock + } +} + +impl ClockPort for SystemPermissionClock { + fn now_unix_millis(&self) -> i64 { + SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|duration| i64::try_from(duration.as_millis()).unwrap_or(i64::MAX)) + .unwrap_or_default() + } +} + +static PERMISSION_REQUEST_MANAGER: OnceLock> = OnceLock::new(); + +/// Returns the process-shared V2 permission request owner used by product +/// surfaces. Pending requests remain process-local; only remembered grants and +/// audit facts are written to the user data directory. +pub fn core_permission_request_manager() -> Result, String> { + if let Some(manager) = PERMISSION_REQUEST_MANAGER.get() { + return Ok(manager.clone()); + } + + let path_manager = crate::infrastructure::PathManager::new() + .map_err(|error| format!("Failed to initialize permission path manager: {error}"))?; + let store = Arc::new(ProjectPermissionFileStore::new( + path_manager.user_data_dir().join("permissions"), + )); + let audit_store: Arc = store.clone(); + let reply_store: Arc = store; + let manager = Arc::new(PermissionRequestManager::new( + audit_store, + reply_store, + Arc::new(SystemPermissionClock), + )); + let _ = PERMISSION_REQUEST_MANAGER.set(manager); + PERMISSION_REQUEST_MANAGER + .get() + .cloned() + .ok_or_else(|| "Failed to initialize shared permission request manager".to_string()) +} + /// Serializes one compatibility mutation with Core's session lifecycle. pub struct CoreSessionMutationPermit { _guard: KeyedAsyncLockGuard, diff --git a/src/crates/assembly/core/src/service_agent_runtime.rs b/src/crates/assembly/core/src/service_agent_runtime.rs index 308a1148eb..dc6607a8c8 100644 --- a/src/crates/assembly/core/src/service_agent_runtime.rs +++ b/src/crates/assembly/core/src/service_agent_runtime.rs @@ -403,10 +403,10 @@ fn core_agent_runtime_builder( thread_goal_management: Arc, cancellation: Arc, interaction_response: Arc, -) -> AgentRuntimeBuilder { +) -> Result { let agent_registry: Arc = crate::agentic::agents::get_agent_registry(); - AgentRuntimeBuilder::new() + Ok(AgentRuntimeBuilder::new() .with_submission_port(submission) .with_session_management_port(session_management) .with_session_model_port(session_model) @@ -415,7 +415,8 @@ fn core_agent_runtime_builder( .with_thread_goal_management_port(thread_goal_management) .with_cancellation_port(cancellation) .with_interaction_response_port(interaction_response) - .with_agent_registry(agent_registry) + .with_permission_request_manager(crate::product_runtime::core_permission_request_manager()?) + .with_agent_registry(agent_registry)) } #[derive(Clone)] @@ -806,7 +807,7 @@ impl CoreServiceAgentRuntime { thread_goal_management, cancellation, interaction_response, - ) + )? .build() .map_err(|error| error.to_string()) } @@ -836,7 +837,7 @@ impl CoreServiceAgentRuntime { thread_goal_management, cancellation, interaction_response, - ) + )? .with_dialog_turn_port(dialog_turn) .with_lifecycle_delivery_port(lifecycle_delivery) .build() @@ -867,7 +868,7 @@ impl CoreServiceAgentRuntime { thread_goal_management, cancellation, interaction_response, - ) + )? .with_lifecycle_delivery_port(lifecycle_delivery) .build() .map_err(|error| error.to_string()) @@ -898,7 +899,7 @@ impl CoreServiceAgentRuntime { thread_goal_management, cancellation, interaction_response, - ) + )? .with_dialog_turn_port(dialog_turn) .with_lifecycle_delivery_port(lifecycle_delivery) .build() @@ -970,7 +971,7 @@ impl CoreServiceAgentRuntime { thread_goal_management, cancellation, interaction_response, - ) + )? .with_dialog_turn_port(dialog_turn) .with_lifecycle_delivery_port(lifecycle_delivery); let builder = match event_source { diff --git a/src/crates/contracts/product-domains/src/tool_permissions.rs b/src/crates/contracts/product-domains/src/tool_permissions.rs index 2e9fe269e5..1391e6c111 100644 --- a/src/crates/contracts/product-domains/src/tool_permissions.rs +++ b/src/crates/contracts/product-domains/src/tool_permissions.rs @@ -97,6 +97,24 @@ pub enum PermissionReplySource { System, } +/// Process-local lifecycle event projected to interactive permission surfaces. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[serde(tag = "event", rename_all = "snake_case")] +pub enum PermissionRequestEvent { + Asked { + request: PermissionRequest, + }, + Replied { + request_id: String, + reply: PermissionReply, + source: PermissionReplySource, + }, + Cancelled { + request_id: String, + reason: String, + }, +} + /// A remembered allow scoped by project, action, and resource. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] diff --git a/src/crates/contracts/runtime-ports/src/lib.rs b/src/crates/contracts/runtime-ports/src/lib.rs index c802ff05f1..dbca8517c0 100644 --- a/src/crates/contracts/runtime-ports/src/lib.rs +++ b/src/crates/contracts/runtime-ports/src/lib.rs @@ -19,7 +19,7 @@ mod script_tool; pub use bitfun_product_domains::tool_permissions::{ PermissionAuditEvent, PermissionAuditRecord, PermissionGrant, PermissionGrantKey, PermissionReply, PermissionReplySource, PermissionRequest as PermissionV2Request, - PermissionRequestSource, PermissionRequestSourceKind, + PermissionRequestEvent, PermissionRequestSource, PermissionRequestSourceKind, }; #[cfg(feature = "permission-v2")] pub use permission_v2::{ diff --git a/src/crates/execution/agent-runtime/src/permission_v2.rs b/src/crates/execution/agent-runtime/src/permission_v2.rs index dda4d8ded8..60dd5e5207 100644 --- a/src/crates/execution/agent-runtime/src/permission_v2.rs +++ b/src/crates/execution/agent-runtime/src/permission_v2.rs @@ -7,13 +7,17 @@ use bitfun_runtime_ports::{ ClockPort, PermissionAuditEvent, PermissionAuditRecord, PermissionAuditStorePort, PermissionGrant, PermissionReply, PermissionReplySource, PermissionReplyStorePort, - PermissionV2Request, PortError, + PermissionRequestEvent, PermissionV2Request, PortError, }; use dashmap::mapref::entry::Entry; use dashmap::DashMap; use std::collections::HashSet; use std::sync::Arc; -use tokio::sync::{oneshot, Mutex}; +use tokio::sync::{broadcast, oneshot, Mutex}; + +const PERMISSION_EVENT_CAPACITY: usize = 128; + +pub type PermissionRequestEventReceiver = broadcast::Receiver; #[derive(Debug, Clone, PartialEq)] pub enum PermissionWaitOutcome { @@ -74,6 +78,7 @@ pub struct PermissionRequestManager { audit_store: Arc, reply_store: Arc, clock: Arc, + events: broadcast::Sender, } impl std::fmt::Debug for PermissionRequestManager { @@ -90,15 +95,21 @@ impl PermissionRequestManager { reply_store: Arc, clock: Arc, ) -> Self { + let (events, _) = broadcast::channel(PERMISSION_EVENT_CAPACITY); Self { pending: Arc::new(DashMap::new()), operations: Arc::new(Mutex::new(())), audit_store, reply_store, clock, + events, } } + pub fn subscribe(&self) -> PermissionRequestEventReceiver { + self.events.subscribe() + } + pub async fn register( &self, request: PermissionV2Request, @@ -121,7 +132,7 @@ impl PermissionRequestManager { let audit = PermissionAuditRecord { audit_id: audit_id(&request_id, "requested"), - request, + request: request.clone(), event: PermissionAuditEvent::Requested, timestamp_ms: self.clock.now_unix_millis(), }; @@ -130,6 +141,10 @@ impl PermissionRequestManager { return Err(PermissionRequestManagerError::AuditStore(error)); } + let _ = self.events.send(PermissionRequestEvent::Asked { + request: request.clone(), + }); + Ok(PendingPermissionReceiver { request_id, receiver, @@ -211,7 +226,12 @@ impl PermissionRequestManager { if let Some((_, pending)) = self.pending.remove(&pending_request.request_id) { let _ = pending .sender - .send(PermissionWaitOutcome::Replied(pending_reply)); + .send(PermissionWaitOutcome::Replied(pending_reply.clone())); + let _ = self.events.send(PermissionRequestEvent::Replied { + request_id: pending_request.request_id, + reply: pending_reply, + source, + }); } } @@ -286,6 +306,10 @@ impl PermissionRequestManager { let _ = pending.sender.send(PermissionWaitOutcome::Cancelled { reason: reason.clone(), }); + let _ = self.events.send(PermissionRequestEvent::Cancelled { + request_id: request.request_id, + reason: reason.clone(), + }); } } Ok(()) @@ -318,3 +342,151 @@ fn grants_for_reply( fn audit_id(request_id: &str, event: &str) -> String { format!("{request_id}:{event}") } + +#[cfg(test)] +mod tests { + use super::*; + use bitfun_runtime_ports::{ + PermissionAuditStorePort, PermissionReplyStorePort, PortResult, RuntimeServiceCapability, + RuntimeServicePort, + }; + use serde_json::Map; + use std::sync::Mutex as StdMutex; + + #[derive(Debug, Default)] + struct MemoryPermissionStore { + audit: StdMutex>, + } + + impl RuntimeServicePort for MemoryPermissionStore { + fn capability(&self) -> RuntimeServiceCapability { + RuntimeServiceCapability::Permission + } + } + + #[async_trait::async_trait] + impl PermissionAuditStorePort for MemoryPermissionStore { + async fn append_permission_audit(&self, record: PermissionAuditRecord) -> PortResult<()> { + self.audit.lock().unwrap().push(record); + Ok(()) + } + + async fn list_project_permission_audit( + &self, + project_id: &str, + ) -> PortResult> { + Ok(self + .audit + .lock() + .unwrap() + .iter() + .filter(|record| record.request.project_id == project_id) + .cloned() + .collect()) + } + } + + #[async_trait::async_trait] + impl PermissionReplyStorePort for MemoryPermissionStore { + async fn commit_permission_reply( + &self, + _grants: Vec, + audit: Vec, + ) -> PortResult<()> { + self.audit.lock().unwrap().extend(audit); + Ok(()) + } + } + + #[derive(Debug)] + struct FixedClock; + + impl RuntimeServicePort for FixedClock { + fn capability(&self) -> RuntimeServiceCapability { + RuntimeServiceCapability::Clock + } + } + + impl ClockPort for FixedClock { + fn now_unix_millis(&self) -> i64 { + 42 + } + } + + fn request() -> PermissionV2Request { + PermissionV2Request { + request_id: "request-1".to_string(), + project_id: "project-1".to_string(), + session_id: "session-1".to_string(), + agent_id: "agentic".to_string(), + action: "edit".to_string(), + resources: vec!["src/main.rs".to_string()], + save_resources: vec!["src/main.rs".to_string()], + source: bitfun_runtime_ports::PermissionRequestSource { + kind: bitfun_runtime_ports::PermissionRequestSourceKind::ToolCall, + identity: "write_file".to_string(), + }, + display_metadata: Map::new(), + } + } + + #[tokio::test] + async fn request_events_project_asked_and_replied_lifecycle() { + let store = Arc::new(MemoryPermissionStore::default()); + let manager = PermissionRequestManager::new(store.clone(), store, Arc::new(FixedClock)); + let mut events = manager.subscribe(); + + let pending = manager.register(request()).await.expect("register request"); + assert_eq!( + events.recv().await.expect("asked event"), + PermissionRequestEvent::Asked { request: request() } + ); + assert_eq!(manager.pending_requests(), vec![request()]); + + manager + .reply( + "request-1", + PermissionReply::Once, + PermissionReplySource::User, + ) + .await + .expect("reply request"); + assert_eq!( + events.recv().await.expect("replied event"), + PermissionRequestEvent::Replied { + request_id: "request-1".to_string(), + reply: PermissionReply::Once, + source: PermissionReplySource::User, + } + ); + assert_eq!( + pending.wait().await, + PermissionWaitOutcome::Replied(PermissionReply::Once) + ); + assert!(manager.pending_requests().is_empty()); + + let cancelled = manager + .register(PermissionV2Request { + request_id: "request-2".to_string(), + ..request() + }) + .await + .expect("register second request"); + let _ = events.recv().await.expect("second asked event"); + manager + .cancel_request("request-2", "session closed") + .await + .expect("cancel request"); + assert!(matches!( + events.recv().await.expect("cancelled event"), + PermissionRequestEvent::Cancelled { request_id, reason } + if request_id == "request-2" && reason == "session closed" + )); + assert_eq!( + cancelled.wait().await, + PermissionWaitOutcome::Cancelled { + reason: "session closed".to_string() + } + ); + } +} diff --git a/src/crates/execution/agent-runtime/src/runtime.rs b/src/crates/execution/agent-runtime/src/runtime.rs index 2cb1e37d52..8980490625 100644 --- a/src/crates/execution/agent-runtime/src/runtime.rs +++ b/src/crates/execution/agent-runtime/src/runtime.rs @@ -25,7 +25,9 @@ use bitfun_runtime_ports::{ use bitfun_runtime_services::RuntimeServices; use crate::event_source::{AgentEventReceiver, AgentEventSource, AgentSessionEventReceiver}; +use crate::permission_v2::{PermissionRequestEventReceiver, PermissionRequestManager}; use crate::post_call_hooks::RuntimeHookRegistry; +use bitfun_runtime_ports::{PermissionReply, PermissionReplySource, PermissionV2Request}; #[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] pub enum RuntimeBuildError { @@ -33,6 +35,8 @@ pub enum RuntimeBuildError { MissingSubmissionPort, #[error("plugin runtime client binding must report executable host availability")] UnsupportedPluginRuntimeHostBinding, + #[error("permission request manager is unavailable: {0}")] + PermissionRequestManagerUnavailable(String), } #[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] @@ -57,6 +61,10 @@ pub enum RuntimeError { MissingEventSink, #[error("agent event source is not registered")] MissingEventSource, + #[error("permission request manager is not registered")] + MissingPermissionRequestManager, + #[error("permission request failed: {0}")] + PermissionRequest(String), #[error(transparent)] Port(#[from] PortError), } @@ -190,6 +198,7 @@ pub struct AgentRuntime { lifecycle_delivery: Option>, cancellation: Option>, interaction_response: Option>, + permission_requests: Option>, services: Option, event_stream: Option, event_source: Option, @@ -325,6 +334,7 @@ pub struct AgentRuntimeBuilder { lifecycle_delivery: Option>, cancellation: Option>, interaction_response: Option>, + permission_requests: Option>, services: Option, event_stream: Option, event_source: Option, @@ -405,6 +415,14 @@ impl AgentRuntimeBuilder { self } + pub fn with_permission_request_manager( + mut self, + manager: Arc, + ) -> Self { + self.permission_requests = Some(manager); + self + } + pub fn with_services(mut self, services: RuntimeServices) -> Self { self.services = Some(services); self @@ -457,6 +475,7 @@ impl AgentRuntimeBuilder { lifecycle_delivery, cancellation, interaction_response, + permission_requests, services, event_stream, event_source, @@ -482,6 +501,7 @@ impl AgentRuntimeBuilder { lifecycle_delivery, cancellation, interaction_response, + permission_requests, services, event_stream, event_source, @@ -608,6 +628,37 @@ impl AgentRuntime { .ok_or(RuntimeError::MissingEventSource) } + pub fn pending_permission_requests(&self) -> Result, RuntimeError> { + self.permission_requests + .as_ref() + .map(|manager| manager.pending_requests()) + .ok_or(RuntimeError::MissingPermissionRequestManager) + } + + pub fn subscribe_permission_requests( + &self, + ) -> Result { + self.permission_requests + .as_ref() + .map(|manager| manager.subscribe()) + .ok_or(RuntimeError::MissingPermissionRequestManager) + } + + pub async fn respond_permission( + &self, + request_id: &str, + reply: PermissionReply, + source: PermissionReplySource, + ) -> Result<(), RuntimeError> { + self.permission_requests + .as_ref() + .ok_or(RuntimeError::MissingPermissionRequestManager)? + .reply(request_id, reply, source) + .await + .map(|_| ()) + .map_err(|error| RuntimeError::PermissionRequest(error.to_string())) + } + pub fn services(&self) -> Option<&RuntimeServices> { self.services.as_ref() } diff --git a/src/crates/execution/agent-runtime/src/sdk.rs b/src/crates/execution/agent-runtime/src/sdk.rs index fc2f482cc5..e00058e217 100644 --- a/src/crates/execution/agent-runtime/src/sdk.rs +++ b/src/crates/execution/agent-runtime/src/sdk.rs @@ -36,6 +36,10 @@ impl AgentRuntimeSdkCompatibility { pub use crate::context_profile::{ContextProfile, ContextProfilePolicy, ModelCapabilityProfile}; pub use crate::event_source::{AgentEventReceiver, AgentEventSource, AgentSessionEventReceiver}; +pub use crate::permission_v2::{ + PermissionReplyResolution, PermissionRequestEventReceiver, PermissionRequestManager, + PermissionRequestManagerError, +}; pub use crate::post_call_hooks::{ RuntimeHookErrorPolicy, RuntimeHookKind, RuntimeHookPlan, RuntimeHookRegistry, RuntimeHookRegistryBuildError, @@ -64,7 +68,9 @@ pub use bitfun_runtime_ports::{ AgentThreadGoalManagementPort, AgentThreadGoalUpdateStatusRequest, AgentTurnCancellationPort, AgentTurnCancellationRequest, AgentTurnCancellationResult, ClockPort, DialogSubmissionPolicy, DialogSubmitOutcome, FileSystemPort, GitPort, McpCatalogPort, NetworkPort, PermissionDecision, - PermissionPort, PermissionRequest, PortError, PortResult, RemoteAssistantWorkspaceFacts, + PermissionPort, PermissionReply, PermissionReplySource, PermissionRequest, + PermissionRequestEvent, PermissionRequestSource, PermissionRequestSourceKind, + PermissionV2Request, PortError, PortResult, RemoteAssistantWorkspaceFacts, RemoteCapabilityPort, RemoteConnectionPort, RemoteProjectionPort, RemoteRecentWorkspaceFacts, RemoteWorkspaceFacts, RemoteWorkspaceFileRuntimeHost, RemoteWorkspaceKind, RemoteWorkspacePort, RemoteWorkspaceRuntimeHost, RemoteWorkspaceUpdate, RuntimeEventEnvelope, RuntimeEventSink, @@ -166,6 +172,14 @@ impl AgentRuntimeBuilder { self } + pub fn with_permission_request_manager( + mut self, + manager: Arc, + ) -> Self { + self.inner = self.inner.with_permission_request_manager(manager); + self + } + pub fn with_services(mut self, services: RuntimeServices) -> Self { self.inner = self.inner.with_services(services); self @@ -218,6 +232,37 @@ impl AgentRuntime { self.inner.subscribe_session_events(session_id) } + pub fn pending_permission_requests(&self) -> Result, RuntimeError> { + self.inner.pending_permission_requests() + } + + pub fn subscribe_permission_requests( + &self, + ) -> Result { + self.inner.subscribe_permission_requests() + } + + pub async fn respond_permission( + &self, + request_id: &str, + reply: PermissionReply, + ) -> Result<(), RuntimeError> { + self.inner + .respond_permission(request_id, reply, PermissionReplySource::User) + .await + } + + pub async fn respond_permission_with_source( + &self, + request_id: &str, + reply: PermissionReply, + source: PermissionReplySource, + ) -> Result<(), RuntimeError> { + self.inner + .respond_permission(request_id, reply, source) + .await + } + pub fn services(&self) -> Option<&RuntimeServices> { self.inner.services() } diff --git a/src/web-ui/src/flow_chat/components/modern/ModernFlowChatContainer.tsx b/src/web-ui/src/flow_chat/components/modern/ModernFlowChatContainer.tsx index a9c1236374..f5fe846490 100644 --- a/src/web-ui/src/flow_chat/components/modern/ModernFlowChatContainer.tsx +++ b/src/web-ui/src/flow_chat/components/modern/ModernFlowChatContainer.tsx @@ -74,6 +74,8 @@ import { type BackgroundSubagentActivityItem, } from '../../utils/backgroundSubagentActivity'; import './ModernFlowChatContainer.scss'; +import { PermissionRequestPanel } from './PermissionRequestPanel'; +import { usePermissionRequests } from './usePermissionRequests'; interface ModernFlowChatContainerProps { className?: string; @@ -224,6 +226,9 @@ export const ModernFlowChatContainer: React.FC = ( const { t } = useTranslation('flow-chat'); const virtualItems = useVirtualItems(); const activeSession = useActiveSession(); + const { requests: permissionRequests, respond: respondPermission } = usePermissionRequests( + activeSession?.sessionId, + ); const visibleTurnInfo = useVisibleTurnInfo(); const [pendingHeaderTurnId, setPendingHeaderTurnId] = useState(null); const [queuedHeaderTurnPinId, setQueuedHeaderTurnPinId] = useState(null); @@ -1476,6 +1481,15 @@ export const ModernFlowChatContainer: React.FC = ( onSend={handleSendBackgroundCommandInput} /> + {permissionRequests[0] && ( + + respondPermission(permissionRequests[0].requestId, reply, feedback) + } + /> + )} +
Promise; +} + +export function PermissionRequestPanel({ request, onRespond }: PermissionRequestPanelProps) { + const { t } = useTranslation('flow-chat'); + const [feedback, setFeedback] = useState(''); + const [responding, setResponding] = useState(false); + const [error, setError] = useState(false); + const risk = [request.displayMetadata?.riskDescription, request.displayMetadata?.risk].find( + (value): value is string => typeof value === 'string' && value.trim().length > 0, + ); + + const respond = async (reply: PermissionReplyKind) => { + setResponding(true); + setError(false); + try { + await onRespond(reply, reply === 'reject' ? feedback : undefined); + } catch { + setError(true); + } finally { + setResponding(false); + } + }; + + return ( +
+
+
+
+ {request.resources.map((resource, index) => ( + {resource} + ))} +
+ {risk &&

{risk}

} +

+ {request.saveResources?.length + ? t('permissionV2.scope', { project: request.projectId }) + : t('permissionV2.scopeOnce')} +

+ {error &&

{t('permissionV2.responseFailed')}

} +