feat: Permission system for client tool calls#3370
Draft
BinaryMuse wants to merge 14 commits intomainfrom
Draft
Conversation
…oundary This change extracts four new abstractions and sets a clear architectural rule that will guide subsequent refactoring: **Mutation boundary (Phase 1a):** AppState no longer owns an mpsc::Sender. State is passive data; the event loop (or stream task) is the only thing that emits events. handle_client_tool_call is now a pure mutation — the CheckToolCallPermission event is sent by the caller. **AppContext (Phase 1b):** Session-scoped API configuration (endpoint, token, send_cwd) is now a single Clone struct instead of three individually-cloned bindings in the event loop. **ClientContext + ChatRequest (Phase 1c):** Machine identity (OS, shell, distro) is computed once per session via ClientContext::detect() instead of on every SSE request. ChatRequest wraps the per-turn message/session payload, replacing the inline request body construction in create_chat_stream. **ToolDescriptor (Phase 1d):** Centralizes tool metadata — canonical names, display verbs, progressive/past verbs, and client/server classification — into static descriptors. Replaces four separate name-to-text match sites and fixes a bug where is_client was checked against 'file_read' (wrong) instead of 'read_file' (correct). Also fixes several clippy warnings in modified code.
…rames **State decomposition (Phase 2a):** AppState is replaced by three types with clear ownership: - Conversation: owns the event log and session_id. All pure query and event-manipulation methods (events_to_messages, current_command, has_any_command, etc.) move here. - Interaction: owns ephemeral UI state (mode, is_input_blank, confirmation_pending, streaming_status, error). - Session: the top-level type containing conversation, interaction, pending_tool_calls, exit_action, and stream_abort. Cross-cutting lifecycle methods (start_streaming, cancel_streaming, add_tool_call, etc.) stay here. **Dispatch extraction (Phase 2b):** The 400-line match event in the spawn_blocking loop is now a 5-line dispatch call. All 12 handlers are named functions in a new tui/dispatch.rs module. inline.rs shrinks from ~640 lines to ~240 lines. **Stream launch centralization (Phase 2c):** The duplicated 8-line stream launch ritual (present in ContinueAfterTools, SubmitInput, and Retry) is replaced by a single launch_stream function that takes a setup callback for pre-work. Each handler collapses to one line. **Stream frame split (Phase 2d):** ChatStreamEvent is replaced by StreamFrame with explicit Content (TextChunk, ToolCall, ToolResult) and Control (Done, Error, StatusChanged) variants. run_chat_stream now dispatches on frame type, with apply_content_frame and apply_control_frame as separate functions.
Extracts three abstractions that untangle the ~180-line on_check_tool_permission handler: **PermissionResolver (permissions/resolver.rs):** Composes the PermissionWalker + PermissionChecker into a single new/check API. The handler no longer imports Walker, Checker, or Request directly. **ToolOutcome + ClientToolCall::execute (tools/mod.rs):** Each tool variant owns its execution logic. ReadToolCall::execute handles both directory listing and file reading (previously only directory listing worked — file reading was a no-op). Returns ToolOutcome::Success or ToolOutcome::Error, replacing the inline match arms. **PendingToolCall state transitions (tools/mod.rs):** mark_asking(), mark_executing(), mark_denied() methods formalize the state machine instead of direct enum variant assignment. **Session::complete_tool_call (tui/state.rs):** Combines add_tool_result + pending_tool_calls.retain into one method, replacing the repeated cleanup pattern in the handler. The handler drops from ~180 lines to ~60 lines.
Add the history database to AppContext and plumb it through dispatch so client tools can run async database queries. AtuinHistory::execute uses atuin-client's Database::search with fuzzy matching, the first filter mode from the tool call, and configurable limit. Also fix two pre-existing bugs that prevented client tools from working end-to-end: AtuinHistory::matches_rule had a todo!() panic that crashed the permission check task, and on_select_permission Allow was discarding the tool call instead of executing it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR implements a permission system for client-side tool calls. It uses a model similar to Claude Code:
ReadWrite(src/**/*)Shell(git commit *)config.toml.atuin/permissions.ai.toml