From abe10ca3a3ebca8d5c991ce0eafeff5e073cad37 Mon Sep 17 00:00:00 2001 From: PJ Doland Date: Fri, 12 Jun 2026 18:04:46 -0400 Subject: [PATCH 1/6] feat(chat): Claude permission-mode selector with gated bypass Adds a compact permission-mode picker next to the chat send button in Claude mode (issue #359): an icon button that opens a menu of Default / Accept Edits / Plan, replacing the /enter-plan-mode and /exit-plan-mode slash commands (kept as hidden, no-longer-autocompleted aliases for one release). The selected mode rides each request and is applied to the SDK client; plan approval and the aliases route through one apply helper so the tracked mode and a new backend->frontend notification can't drift. Bypass Permissions skips NBI's tool-call confirmation entirely, so it is gated as its own armed mode rather than a peer of the other three: - A new claude_bypass_permissions policy / NBI_CLAUDE_BYPASS_PERMISSIONS_POLICY defaulting to force-off (the only policy that does). user-choice exposes the option; the user still arms it per session through a confirm step, and it never survives a new session or a fresh SDK client. - The mode is clamped server-side at the websocket boundary, so a hand-rolled request can't escalate past what the selector offers; unknown modes (and dontAsk/auto) fail closed to default. - NBI defers to Claude Code's enterprise managed settings: permissions.disableBypassPermissionsMode refuses bypass regardless of the policy (corrupt file fails closed), and permissions.defaultMode seeds the selector's starting mode (bypass excepted). The armed state shows a red warning glyph plus aria-label (not color alone); the menu has roving arrow-key focus, Escape, and focus restoration. Documented in the README policy table and admin guide. Closes #359 --- CHANGELOG.md | 4 + README.md | 35 +-- docs/admin-guide.md | 112 +++++---- notebook_intelligence/api.py | 4 + notebook_intelligence/claude.py | 161 ++++++++++++- notebook_intelligence/extension.py | 88 ++++++- src/api.ts | 15 +- src/chat-sidebar.tsx | 66 ++++- src/components/permission-mode-select.tsx | 239 +++++++++++++++++++ src/icons.ts | 1 + src/tokens.ts | 1 + style/base.css | 89 +++++++ tests/conftest.py | 16 ++ tests/test_permission_modes.py | 251 ++++++++++++++++++++ tests/test_websocket_handler_integration.py | 109 +++++++++ tests/ts/permission-mode-select.test.tsx | 227 ++++++++++++++++++ 16 files changed, 1339 insertions(+), 79 deletions(-) create mode 100644 src/components/permission-mode-select.tsx create mode 100644 tests/test_permission_modes.py create mode 100644 tests/ts/permission-mode-select.test.tsx diff --git a/CHANGELOG.md b/CHANGELOG.md index c23db6c6..1d85c6e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,10 @@ For each release we list user-facing changes grouped as **Added**, **Changed**, ## [Unreleased] +### Added + +- **Claude permission-mode selector in the chat input** (#359). A dropdown next to the send button switches between Default, Accept Edits, and Plan; the selected mode rides each request and takes effect immediately, replacing the `/enter-plan-mode` and `/exit-plan-mode` slash commands (still working as hidden aliases for one release, but no longer autocompleted). "Bypass Permissions", which skips NBI's tool-call confirmation entirely, is gated behind the new `claude_bypass_permissions_policy` traitlet / `NBI_CLAUDE_BYPASS_PERMISSIONS_POLICY` env var defaulting to `force-off` (the only policy that does); when an admin sets `user-choice`, the option appears but must be armed through an explicit confirm step, shows a persistent red indicator while armed, and never survives a new session (it resets to default on `/clear` and on a fresh SDK client). The mode is clamped server-side on every request, and NBI defers to Claude Code's enterprise managed settings: `permissions.disableBypassPermissionsMode` refuses bypass regardless of the NBI policy, and `permissions.defaultMode` seeds the selector's starting mode (bypass excepted). + ## [5.1.0] - UNRELEASED 5.1.0 builds on 5.0.x with a focus on Claude-mode agent visibility. Tool calls the agent runs now render as persistent status cards with inline diffs and collapsible grouping, the generating indicator can cycle custom verbs, and cancelling a turn tears down the whole process tree the agent spawned instead of leaking it. It also adds two opt-in security guardrails (an MCP stdio-command allowlist and a default-token-password check on shared filesystems) and an always-visible mode for chat feedback. No traitlet, env-var, REST route, or on-disk-format renames or removals; every new admin surface is opt-in and listed below. diff --git a/README.md b/README.md index a163e1ad..ec484068 100644 --- a/README.md +++ b/README.md @@ -177,23 +177,24 @@ Most settings panel toggles can be locked by org administrators. Two shapes: **Boolean policies** use the `*_POLICY` suffix and accept three values: `user-choice` (default — user toggles freely), `force-on` (locked enabled), `force-off` (locked disabled). When forced, the panel control is disabled with a "Locked by your administrator" tooltip and any client-side write is ignored. -| Env var | Locks the Settings panel control for | -| ---------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------- | -| `NBI_EXPLAIN_ERROR_POLICY` | "Explain cell errors" | -| `NBI_OUTPUT_FOLLOWUP_POLICY` | "Ask about cell outputs" | -| `NBI_OUTPUT_TOOLBAR_POLICY` | "Show output toolbar" | -| `NBI_CLAUDE_MODE_POLICY` | "Enable Claude mode" | -| `NBI_CLAUDE_CONTINUE_CONVERSATION_POLICY` | "Remember conversation history" | -| `NBI_CLAUDE_CODE_TOOLS_POLICY` | "Claude Code tools" | -| `NBI_CLAUDE_JUPYTER_UI_TOOLS_POLICY` | "Jupyter UI tools" | -| `NBI_CLAUDE_SETTING_SOURCE_USER_POLICY` | Setting source: User | -| `NBI_CLAUDE_SETTING_SOURCE_PROJECT_POLICY` | Setting source: Project | -| `NBI_STORE_GITHUB_ACCESS_TOKEN_POLICY` | "Remember my GitHub Copilot access token" | -| `NBI_SKILLS_MANAGEMENT_POLICY` | The Skills tab (force-off hides it and 403s the API; also disables the managed-skills reconciler) | -| `NBI_CLAUDE_MCP_MANAGEMENT_POLICY` | The Claude-mode MCP Servers tab (force-off hides it and 403s `/claude-mcp/*`; independent of the non-Claude MCP Servers tab) | -| `NBI_CLAUDE_PLUGINS_MANAGEMENT_POLICY` | The Claude-mode Plugins tab (force-off hides it and 403s `/plugins/*`) | -| `NBI_TERMINAL_DRAG_DROP_POLICY` | Terminal drag-drop file attach feature | -| `NBI_REFRESH_OPEN_FILES_ON_DISK_CHANGE_POLICY` | "Refresh open files when changed on disk" | +| Env var | Locks the Settings panel control for | +| ---------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `NBI_EXPLAIN_ERROR_POLICY` | "Explain cell errors" | +| `NBI_OUTPUT_FOLLOWUP_POLICY` | "Ask about cell outputs" | +| `NBI_OUTPUT_TOOLBAR_POLICY` | "Show output toolbar" | +| `NBI_CLAUDE_MODE_POLICY` | "Enable Claude mode" | +| `NBI_CLAUDE_CONTINUE_CONVERSATION_POLICY` | "Remember conversation history" | +| `NBI_CLAUDE_CODE_TOOLS_POLICY` | "Claude Code tools" | +| `NBI_CLAUDE_JUPYTER_UI_TOOLS_POLICY` | "Jupyter UI tools" | +| `NBI_CLAUDE_SETTING_SOURCE_USER_POLICY` | Setting source: User | +| `NBI_CLAUDE_SETTING_SOURCE_PROJECT_POLICY` | Setting source: Project | +| `NBI_STORE_GITHUB_ACCESS_TOKEN_POLICY` | "Remember my GitHub Copilot access token" | +| `NBI_SKILLS_MANAGEMENT_POLICY` | The Skills tab (force-off hides it and 403s the API; also disables the managed-skills reconciler) | +| `NBI_CLAUDE_MCP_MANAGEMENT_POLICY` | The Claude-mode MCP Servers tab (force-off hides it and 403s `/claude-mcp/*`; independent of the non-Claude MCP Servers tab) | +| `NBI_CLAUDE_PLUGINS_MANAGEMENT_POLICY` | The Claude-mode Plugins tab (force-off hides it and 403s `/plugins/*`) | +| `NBI_CLAUDE_BYPASS_PERMISSIONS_POLICY` | "Bypass Permissions" in the Claude permission-mode selector (defaults to `force-off`, the only policy that does; `user-choice` exposes the option, which the user still arms per session) | +| `NBI_TERMINAL_DRAG_DROP_POLICY` | Terminal drag-drop file attach feature | +| `NBI_REFRESH_OPEN_FILES_ON_DISK_CHANGE_POLICY` | "Refresh open files when changed on disk" | The first three also have matching traitlets on `NotebookIntelligence` (`explain_error_policy`, `output_followup_policy`, `output_toolbar_policy`); add the others as needed in the same shape: diff --git a/docs/admin-guide.md b/docs/admin-guide.md index 88941616..b4df1e06 100644 --- a/docs/admin-guide.md +++ b/docs/admin-guide.md @@ -75,56 +75,56 @@ If users share a home directory across nodes (NFS-backed shared HPC, classroom l The full surface, in one table. -| Name | Type | Default | Source | Purpose | -| ------------------------------------------------ | ---- | --------------------------- | ---------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| `disabled_providers` | List | `[]` | traitlet on `NotebookIntelligence` | Hide providers from the user dropdown. Values: `github-copilot`, `ollama`, `litellm-compatible`, `openai-compatible`. | -| `allow_enabling_providers_with_env` | Bool | `False` | traitlet | If true, `NBI_ENABLED_PROVIDERS` re-enables hidden providers per pod. | -| `NBI_ENABLED_PROVIDERS` | csv | unset | env | Comma-separated provider IDs to re-enable. Effective only when `allow_enabling_providers_with_env=True`. | -| `disabled_tools` | List | `[]` | traitlet | Hide built-in tools from agent mode. Values listed in [Restricting features](#restricting-features-for-managed-deployments). | -| `allow_enabling_tools_with_env` | Bool | `False` | traitlet | If true, `NBI_ENABLED_BUILTIN_TOOLS` re-enables hidden tools per pod. | -| `NBI_ENABLED_BUILTIN_TOOLS` | csv | unset | env | Comma-separated tool IDs to re-enable. Effective only when `allow_enabling_tools_with_env=True`. | -| `disabled_coding_agent_launchers` | List | `[]` | traitlet | Hide JupyterLab launcher tiles for coding-agent CLIs even when the CLI is on `PATH`. Valid IDs: `claude-code`, `opencode`, `pi`, `github-copilot-cli`, `codex`. See [Disabling coding-agent launcher tiles](#disabling-coding-agent-launcher-tiles). | -| `allow_enabling_coding_agent_launchers_with_env` | Bool | `False` | traitlet | If true, `NBI_ENABLED_CODING_AGENT_LAUNCHERS` re-enables hidden tiles per pod. | -| `NBI_ENABLED_CODING_AGENT_LAUNCHERS` | csv | unset | env | Comma-separated launcher IDs to re-enable. Effective only when `allow_enabling_coding_agent_launchers_with_env=True`. | -| `enable_chat_feedback` | Bool | `False` | traitlet | Enables thumbs-up/down UI in chat and emits in-process `telemetry` events. | -| `enable_chat_feedback_always_visible` | Bool | `False` | traitlet | Renders thumbs-up/down buttons at full opacity on every assistant reply instead of revealing them only on hover. Requires `enable_chat_feedback=True`. | -| `additional_skipped_workspace_directories` | List | `[]` | traitlet | Extra directory names to skip in the chat-sidebar @-mention workspace file picker. Merged with the built-in skips (`__pycache__`, `node_modules`). Match is by directory name only, case-sensitive. | -| `NBI_ADDITIONAL_SKIPPED_WORKSPACE_DIRECTORIES` | csv | unset | env (appends to traitlet) | Comma-separated extra directory names. Resolved at server startup and concatenated with the traitlet value, so a spawn profile can add to (rather than replace) the org-wide list. | -| `allow_github_skill_import` | Bool | `True` | traitlet | When `False`, hides the **Import from GitHub** button in the Skills panel and rejects `/skills/import` POSTs with 403. Does not affect the managed-skills reconciler. | -| `NBI_ALLOW_GITHUB_SKILL_IMPORT` | bool | unset | env (overrides traitlet) | Per-pod override for `allow_github_skill_import`. Accepts `true`/`false`/`1`/`0`/`yes`/`no`/`on`/`off` (case-insensitive). Useful for varying the policy across spawn profiles. | -| `skills_manifest` | str | `""` | traitlet | URL or filesystem path to a managed-skills manifest, or a comma-separated list of either. Manifests are unioned with first-wins URL dedupe; name collisions surface as per-entry errors. See [`docs/skills.md`](skills.md#managed-skills-via-an-org-manifest). | -| `NBI_SKILLS_MANIFEST` | str | unset | env (overrides traitlet) | Same as above; env takes precedence. | -| `skills_manifest_interval` | int | `86400` | traitlet | Seconds between reconciles. | -| `NBI_SKILLS_MANIFEST_INTERVAL` | int | unset | env (overrides traitlet) | Same as above; env takes precedence. | -| `managed_skills_token` | str | `""` | traitlet | Bearer token for managed-skills GitHub fetches. | -| `NBI_MANAGED_SKILLS_TOKEN` | str | unset | env (overrides traitlet) | Same as above; env takes precedence. | -| `allow_github_plugin_import` | Bool | `True` | traitlet | When `False`, hides the "From GitHub" affordance in the Plugins panel and rejects `claude plugin marketplace add` requests whose source resolves as a GitHub URL or `owner/repo` shorthand. Local-path and arbitrary-URL sources remain available. | -| `NBI_ALLOW_GITHUB_PLUGIN_IMPORT` | bool | unset | env (overrides traitlet) | Per-pod override for `allow_github_plugin_import`. Accepts `true`/`false`/`1`/`0`/`yes`/`no`/`on`/`off` (case-insensitive). | -| `skill_max_archive_mb` | Int | `100` | traitlet | Per-archive on-wire size cap (megabytes) for skill bundles fetched from GitHub. Applies to both user imports and managed-skills tarballs. `0` disables the cap. | -| `NBI_SKILL_MAX_ARCHIVE_MB` | int | unset | env (overrides traitlet) | Same as above; env takes precedence. | -| `upload_max_mb` | Int | `50` | traitlet | Per-file size cap (megabytes) for the shared upload endpoint used by chat-sidebar attachments and terminal drag-drop. Requests over the cap return HTTP 413. `0` disables the cap. | -| `NBI_UPLOAD_MAX_MB` | int | unset | env (overrides traitlet) | Same as above; env takes precedence. | -| `upload_retention_hours` | Int | `24` | traitlet | How long staged uploads survive in the temp directory before the next upload sweeps them. `0` keeps only the atexit purge (uploads survive the session). | -| `NBI_UPLOAD_RETENTION_HOURS` | int | unset | env (overrides traitlet) | Same as above; env takes precedence. | -| `mcp_stdio_command_allowlist` | List | `[]` | traitlet | Regex allowlist for the stdio MCP server `command` field. Empty list (default) means no enforcement; non-empty list rejects any stdio MCP server whose command does not match. Matched with `re.search`; anchor (`^...$`) for literal equality. Applies at both Claude `mcp add` and `mcp.json` load. See [Restricting MCP stdio commands](#restricting-mcp-stdio-commands). | -| `NBI_MCP_STDIO_COMMAND_ALLOWLIST` | csv | unset | env (appends to traitlet) | Comma-separated regex patterns added to the traitlet at startup. Per-pod additions on an org baseline. | -| `tour_config_path` | str | `""` | traitlet | Filesystem path to a YAML/JSON file with admin overrides for the first-run sidebar tour copy. See [`docs/admin-tour-config.md`](admin-tour-config.md). | -| `NBI_TOUR_CONFIG_PATH` | str | unset | env (overrides traitlet) | Same as above; env takes precedence. | -| `NBI_GH_ACCESS_TOKEN_PASSWORD` | str | `nbi-access-token-password` | env | Password used to encrypt the stored Copilot token in `user-data.json`. **Change in multi-tenant deployments.** | -| `NBI_REFUSE_DEFAULT_TOKEN_PASSWORD_ON_SHARED_FS` | bool | unset | env | When set, refuse to write `user-data.json` if the default `NBI_GH_ACCESS_TOKEN_PASSWORD` is still in use AND `~/.jupyter/nbi/` is readable by group or other. Opt-in to preserve backwards compatibility on single-user deployments where the directory mode is incidental. | -| `NBI_ALLOW_DEFAULT_TOKEN_PASSWORD` | bool | unset | env | Per-pod opt-out that disengages the refuse-on-shared-fs guard above. Admins who knowingly accept the risk (e.g., during a transition before rolling out a per-user password) set this so writes continue. | -| `NBI_RULES_AUTO_RELOAD` | bool | `true` | env | When `false`, ruleset edits require a JupyterLab restart to take effect. | -| `NBI_CLAUDE_CLI_PATH` | str | unset | env | Absolute path to the Claude Code CLI binary. When unset, NBI looks up `claude` on `PATH`. | -| `NBI_OPENCODE_CLI_PATH` | str | unset | env | Absolute path to the opencode CLI. When unset, NBI looks up `opencode` on `PATH`. Gates the opencode launcher tile. | -| `NBI_PI_CLI_PATH` | str | unset | env | Absolute path to the Pi CLI. When unset, NBI looks up `pi` on `PATH`. Gates the Pi launcher tile. | -| `NBI_GITHUB_COPILOT_CLI_PATH` | str | unset | env | Absolute path to the GitHub Copilot CLI. When unset, NBI looks up `copilot` on `PATH`. Gates the GitHub Copilot launcher tile. | -| `NBI_CODEX_CLI_PATH` | str | unset | env | Absolute path to the OpenAI Codex CLI. When unset, NBI looks up `codex` on `PATH`. Gates the Codex launcher tile. | -| `NBI_GHE_SUBDOMAIN` | str | `""` | env | GitHub Enterprise subdomain for GitHub Copilot users on a GHE tenant. Empty selects github.com. | -| `NBI_GITHUB_ENTERPRISE_HOSTS` | csv | `""` | env | Comma-separated hostnames the plugin marketplace detector treats as GitHub. Cookie-domain shape: bare token (`github.acme.com`) matches exactly; leading-dot token (`.acme.com`) matches any subdomain of `acme.com`. Independent of `NBI_GHE_SUBDOMAIN`, which only configures the Copilot OAuth tenant. Required so `allow_github_plugin_import = False` actually gates GHE marketplace adds and so the `GITHUB_TOKEN` / `gh auth token` chain injects on GHE sources. | -| `NBI_LOG_LEVEL` | str | `INFO` | env | Python logging level for the `notebook_intelligence` logger. | -| `NBI_DISABLE_OUTPUT_SCRUB` | bool | unset | env | When set (`1` / `true` / `yes` / `on`), disables the shell-tool output scrubber so raw stdout/stderr (including any env-var values that leak) is sent through to chat. Default off; the scrubber redacts values for sensitive-named env vars (`TOKEN`, `SECRET`, `API_KEY`, ...) plus tokens with well-known credential prefixes (`ghp_`, `sk-ant-`, `AKIA`, ...). Opt out only when debugging credential helpers where the redaction interferes. | -| `GITHUB_TOKEN`, `GH_TOKEN` | str | unset | env | Used (in that order) by user-initiated skill imports and GitHub-sourced plugin marketplace adds for GitHub auth. Falls back to `gh` CLI auth. | -| `NBI_*_POLICY` | str | `user-choice` | env | Lock individual Settings panel toggles. See [README → Admin policies](../README.md#admin-policies) for the full list of `*_POLICY` env vars and matching traitlets, including `NBI_SKILLS_MANAGEMENT_POLICY`, `NBI_CLAUDE_MCP_MANAGEMENT_POLICY`, `NBI_CLAUDE_PLUGINS_MANAGEMENT_POLICY`, `NBI_TERMINAL_DRAG_DROP_POLICY`, and `NBI_REFRESH_OPEN_FILES_ON_DISK_CHANGE_POLICY`. | +| Name | Type | Default | Source | Purpose | +| ------------------------------------------------ | ---- | --------------------------- | ---------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `disabled_providers` | List | `[]` | traitlet on `NotebookIntelligence` | Hide providers from the user dropdown. Values: `github-copilot`, `ollama`, `litellm-compatible`, `openai-compatible`. | +| `allow_enabling_providers_with_env` | Bool | `False` | traitlet | If true, `NBI_ENABLED_PROVIDERS` re-enables hidden providers per pod. | +| `NBI_ENABLED_PROVIDERS` | csv | unset | env | Comma-separated provider IDs to re-enable. Effective only when `allow_enabling_providers_with_env=True`. | +| `disabled_tools` | List | `[]` | traitlet | Hide built-in tools from agent mode. Values listed in [Restricting features](#restricting-features-for-managed-deployments). | +| `allow_enabling_tools_with_env` | Bool | `False` | traitlet | If true, `NBI_ENABLED_BUILTIN_TOOLS` re-enables hidden tools per pod. | +| `NBI_ENABLED_BUILTIN_TOOLS` | csv | unset | env | Comma-separated tool IDs to re-enable. Effective only when `allow_enabling_tools_with_env=True`. | +| `disabled_coding_agent_launchers` | List | `[]` | traitlet | Hide JupyterLab launcher tiles for coding-agent CLIs even when the CLI is on `PATH`. Valid IDs: `claude-code`, `opencode`, `pi`, `github-copilot-cli`, `codex`. See [Disabling coding-agent launcher tiles](#disabling-coding-agent-launcher-tiles). | +| `allow_enabling_coding_agent_launchers_with_env` | Bool | `False` | traitlet | If true, `NBI_ENABLED_CODING_AGENT_LAUNCHERS` re-enables hidden tiles per pod. | +| `NBI_ENABLED_CODING_AGENT_LAUNCHERS` | csv | unset | env | Comma-separated launcher IDs to re-enable. Effective only when `allow_enabling_coding_agent_launchers_with_env=True`. | +| `enable_chat_feedback` | Bool | `False` | traitlet | Enables thumbs-up/down UI in chat and emits in-process `telemetry` events. | +| `enable_chat_feedback_always_visible` | Bool | `False` | traitlet | Renders thumbs-up/down buttons at full opacity on every assistant reply instead of revealing them only on hover. Requires `enable_chat_feedback=True`. | +| `additional_skipped_workspace_directories` | List | `[]` | traitlet | Extra directory names to skip in the chat-sidebar @-mention workspace file picker. Merged with the built-in skips (`__pycache__`, `node_modules`). Match is by directory name only, case-sensitive. | +| `NBI_ADDITIONAL_SKIPPED_WORKSPACE_DIRECTORIES` | csv | unset | env (appends to traitlet) | Comma-separated extra directory names. Resolved at server startup and concatenated with the traitlet value, so a spawn profile can add to (rather than replace) the org-wide list. | +| `allow_github_skill_import` | Bool | `True` | traitlet | When `False`, hides the **Import from GitHub** button in the Skills panel and rejects `/skills/import` POSTs with 403. Does not affect the managed-skills reconciler. | +| `NBI_ALLOW_GITHUB_SKILL_IMPORT` | bool | unset | env (overrides traitlet) | Per-pod override for `allow_github_skill_import`. Accepts `true`/`false`/`1`/`0`/`yes`/`no`/`on`/`off` (case-insensitive). Useful for varying the policy across spawn profiles. | +| `skills_manifest` | str | `""` | traitlet | URL or filesystem path to a managed-skills manifest, or a comma-separated list of either. Manifests are unioned with first-wins URL dedupe; name collisions surface as per-entry errors. See [`docs/skills.md`](skills.md#managed-skills-via-an-org-manifest). | +| `NBI_SKILLS_MANIFEST` | str | unset | env (overrides traitlet) | Same as above; env takes precedence. | +| `skills_manifest_interval` | int | `86400` | traitlet | Seconds between reconciles. | +| `NBI_SKILLS_MANIFEST_INTERVAL` | int | unset | env (overrides traitlet) | Same as above; env takes precedence. | +| `managed_skills_token` | str | `""` | traitlet | Bearer token for managed-skills GitHub fetches. | +| `NBI_MANAGED_SKILLS_TOKEN` | str | unset | env (overrides traitlet) | Same as above; env takes precedence. | +| `allow_github_plugin_import` | Bool | `True` | traitlet | When `False`, hides the "From GitHub" affordance in the Plugins panel and rejects `claude plugin marketplace add` requests whose source resolves as a GitHub URL or `owner/repo` shorthand. Local-path and arbitrary-URL sources remain available. | +| `NBI_ALLOW_GITHUB_PLUGIN_IMPORT` | bool | unset | env (overrides traitlet) | Per-pod override for `allow_github_plugin_import`. Accepts `true`/`false`/`1`/`0`/`yes`/`no`/`on`/`off` (case-insensitive). | +| `skill_max_archive_mb` | Int | `100` | traitlet | Per-archive on-wire size cap (megabytes) for skill bundles fetched from GitHub. Applies to both user imports and managed-skills tarballs. `0` disables the cap. | +| `NBI_SKILL_MAX_ARCHIVE_MB` | int | unset | env (overrides traitlet) | Same as above; env takes precedence. | +| `upload_max_mb` | Int | `50` | traitlet | Per-file size cap (megabytes) for the shared upload endpoint used by chat-sidebar attachments and terminal drag-drop. Requests over the cap return HTTP 413. `0` disables the cap. | +| `NBI_UPLOAD_MAX_MB` | int | unset | env (overrides traitlet) | Same as above; env takes precedence. | +| `upload_retention_hours` | Int | `24` | traitlet | How long staged uploads survive in the temp directory before the next upload sweeps them. `0` keeps only the atexit purge (uploads survive the session). | +| `NBI_UPLOAD_RETENTION_HOURS` | int | unset | env (overrides traitlet) | Same as above; env takes precedence. | +| `mcp_stdio_command_allowlist` | List | `[]` | traitlet | Regex allowlist for the stdio MCP server `command` field. Empty list (default) means no enforcement; non-empty list rejects any stdio MCP server whose command does not match. Matched with `re.search`; anchor (`^...$`) for literal equality. Applies at both Claude `mcp add` and `mcp.json` load. See [Restricting MCP stdio commands](#restricting-mcp-stdio-commands). | +| `NBI_MCP_STDIO_COMMAND_ALLOWLIST` | csv | unset | env (appends to traitlet) | Comma-separated regex patterns added to the traitlet at startup. Per-pod additions on an org baseline. | +| `tour_config_path` | str | `""` | traitlet | Filesystem path to a YAML/JSON file with admin overrides for the first-run sidebar tour copy. See [`docs/admin-tour-config.md`](admin-tour-config.md). | +| `NBI_TOUR_CONFIG_PATH` | str | unset | env (overrides traitlet) | Same as above; env takes precedence. | +| `NBI_GH_ACCESS_TOKEN_PASSWORD` | str | `nbi-access-token-password` | env | Password used to encrypt the stored Copilot token in `user-data.json`. **Change in multi-tenant deployments.** | +| `NBI_REFUSE_DEFAULT_TOKEN_PASSWORD_ON_SHARED_FS` | bool | unset | env | When set, refuse to write `user-data.json` if the default `NBI_GH_ACCESS_TOKEN_PASSWORD` is still in use AND `~/.jupyter/nbi/` is readable by group or other. Opt-in to preserve backwards compatibility on single-user deployments where the directory mode is incidental. | +| `NBI_ALLOW_DEFAULT_TOKEN_PASSWORD` | bool | unset | env | Per-pod opt-out that disengages the refuse-on-shared-fs guard above. Admins who knowingly accept the risk (e.g., during a transition before rolling out a per-user password) set this so writes continue. | +| `NBI_RULES_AUTO_RELOAD` | bool | `true` | env | When `false`, ruleset edits require a JupyterLab restart to take effect. | +| `NBI_CLAUDE_CLI_PATH` | str | unset | env | Absolute path to the Claude Code CLI binary. When unset, NBI looks up `claude` on `PATH`. | +| `NBI_OPENCODE_CLI_PATH` | str | unset | env | Absolute path to the opencode CLI. When unset, NBI looks up `opencode` on `PATH`. Gates the opencode launcher tile. | +| `NBI_PI_CLI_PATH` | str | unset | env | Absolute path to the Pi CLI. When unset, NBI looks up `pi` on `PATH`. Gates the Pi launcher tile. | +| `NBI_GITHUB_COPILOT_CLI_PATH` | str | unset | env | Absolute path to the GitHub Copilot CLI. When unset, NBI looks up `copilot` on `PATH`. Gates the GitHub Copilot launcher tile. | +| `NBI_CODEX_CLI_PATH` | str | unset | env | Absolute path to the OpenAI Codex CLI. When unset, NBI looks up `codex` on `PATH`. Gates the Codex launcher tile. | +| `NBI_GHE_SUBDOMAIN` | str | `""` | env | GitHub Enterprise subdomain for GitHub Copilot users on a GHE tenant. Empty selects github.com. | +| `NBI_GITHUB_ENTERPRISE_HOSTS` | csv | `""` | env | Comma-separated hostnames the plugin marketplace detector treats as GitHub. Cookie-domain shape: bare token (`github.acme.com`) matches exactly; leading-dot token (`.acme.com`) matches any subdomain of `acme.com`. Independent of `NBI_GHE_SUBDOMAIN`, which only configures the Copilot OAuth tenant. Required so `allow_github_plugin_import = False` actually gates GHE marketplace adds and so the `GITHUB_TOKEN` / `gh auth token` chain injects on GHE sources. | +| `NBI_LOG_LEVEL` | str | `INFO` | env | Python logging level for the `notebook_intelligence` logger. | +| `NBI_DISABLE_OUTPUT_SCRUB` | bool | unset | env | When set (`1` / `true` / `yes` / `on`), disables the shell-tool output scrubber so raw stdout/stderr (including any env-var values that leak) is sent through to chat. Default off; the scrubber redacts values for sensitive-named env vars (`TOKEN`, `SECRET`, `API_KEY`, ...) plus tokens with well-known credential prefixes (`ghp_`, `sk-ant-`, `AKIA`, ...). Opt out only when debugging credential helpers where the redaction interferes. | +| `GITHUB_TOKEN`, `GH_TOKEN` | str | unset | env | Used (in that order) by user-initiated skill imports and GitHub-sourced plugin marketplace adds for GitHub auth. Falls back to `gh` CLI auth. | +| `NBI_*_POLICY` | str | `user-choice` | env | Lock individual Settings panel toggles. See [README → Admin policies](../README.md#admin-policies) for the full list of `*_POLICY` env vars and matching traitlets, including `NBI_SKILLS_MANAGEMENT_POLICY`, `NBI_CLAUDE_MCP_MANAGEMENT_POLICY`, `NBI_CLAUDE_PLUGINS_MANAGEMENT_POLICY`, `NBI_TERMINAL_DRAG_DROP_POLICY`, and `NBI_REFRESH_OPEN_FILES_ON_DISK_CHANGE_POLICY`. One exception to the `user-choice` default: `NBI_CLAUDE_BYPASS_PERMISSIONS_POLICY` defaults to `force-off`; see [Allowing Bypass Permissions](#allowing-bypass-permissions-in-the-claude-permission-mode-selector). | Configure traitlets in `jupyter_server_config.py`: @@ -560,6 +560,18 @@ This env is **independent of** `NBI_GHE_SUBDOMAIN` (which only configures GitHub For air-gap deployments, marketplace-add inherits the JupyterLab process env, so the same `HTTPS_PROXY` / `HTTP_PROXY` / `NO_PROXY` / `NODE_EXTRA_CA_CERTS` settings documented in [Custom CA certs and corporate proxies](#custom-ca-certs-and-corporate-proxies) apply. Pre-installed plugins (under `~/.claude/plugins/`) keep loading without any network access. +### Allowing Bypass Permissions in the Claude permission-mode selector + +```python +c.NotebookIntelligence.claude_bypass_permissions_policy = "user-choice" +``` + +Or via env: `NBI_CLAUDE_BYPASS_PERMISSIONS_POLICY=user-choice`. + +The Claude permission-mode selector (Default / Accept Edits / Plan) always offers its three normal modes; all three keep NBI's tool-call confirmation flow. "Bypass Permissions" removes that confirmation entirely and runs every tool call with the Jupyter user's full access, so it is **disabled by default**: this is the only policy whose default is `force-off` rather than `user-choice`. Setting `user-choice` exposes the option, which the user must still arm explicitly per session through a confirm step; the armed state never persists across sessions or reconnects. `force-on` grants the same availability as `user-choice` and never auto-arms bypass. + +Independent of this policy, NBI refuses bypass whenever Claude Code's enterprise [managed settings](https://docs.anthropic.com/en/docs/claude-code/settings) set `permissions.disableBypassPermissionsMode`, and honors a managed `permissions.defaultMode` as the selector's starting mode (except bypass, which never starts armed). The clamp is enforced server-side at the request boundary, not just in the UI. + ### Disabling terminal drag-drop file attach ```python diff --git a/notebook_intelligence/api.py b/notebook_intelligence/api.py index 3129e8cc..e2fda1eb 100644 --- a/notebook_intelligence/api.py +++ b/notebook_intelligence/api.py @@ -46,6 +46,7 @@ class BackendMessageType(str, Enum): MCPServerStatusChange = 'mcp-server-status-change' ClaudeCodeStatusChange = 'claude-code-status-change' ClaudeCodeHeartbeat = 'claude-code-heartbeat' + ClaudePermissionModeChange = 'claude-permission-mode-change' SkillsReloaded = 'skills-reloaded' class ResponseStreamDataType(str, Enum): @@ -143,6 +144,9 @@ class ChatRequest: cancel_token: CancelToken = None # NEW: Add context for rule evaluation rule_context: Optional[RuleContext] = None + # Claude-mode permission mode, already clamped server-side against the + # bypass policy and managed settings at the websocket boundary. + permission_mode: str = 'default' @dataclass class ResponseStreamData: diff --git a/notebook_intelligence/claude.py b/notebook_intelligence/claude.py index 21829b20..95ce6f54 100644 --- a/notebook_intelligence/claude.py +++ b/notebook_intelligence/claude.py @@ -297,6 +297,141 @@ def get_current_claude_client() -> ClaudeSDKClient: global _current_claude_client return _current_claude_client +# The subset of the SDK's PermissionMode literals NBI exposes (issue #359). +# "dontAsk" and "auto" are deliberately excluded: both skip the +# human-in-the-loop gate the way bypass does, without the arming UX. +CLAUDE_PERMISSION_MODES = ("default", "acceptEdits", "plan", "bypassPermissions") +CLAUDE_BYPASS_PERMISSION_MODE = "bypassPermissions" +DEFAULT_PERMISSION_MODE = "default" + +# Claude Code's enterprise managed-settings locations, one per platform. +_MANAGED_SETTINGS_PATHS = ( + "/Library/Application Support/ClaudeCode/managed-settings.json", + "/etc/claude-code/managed-settings.json", + "C:\\ProgramData\\ClaudeCode\\managed-settings.json", +) + + +def _claude_managed_permissions() -> Optional[dict]: + """Best-effort read of the ``permissions`` block of Claude Code's + enterprise managed settings. + + Returns ``None`` when no managed-settings file exists. A file that + exists but cannot be parsed returns a block that disables bypass: + an admin clearly tried to manage this machine, so the safe reading + of a corrupt file is the most restrictive one. + """ + for path in _MANAGED_SETTINGS_PATHS: + if not os.path.isfile(path): + continue + try: + with open(path, encoding="utf-8") as f: + data = json.load(f) + permissions = data.get("permissions") if isinstance(data, dict) else None + return permissions if isinstance(permissions, dict) else {} + except Exception as exc: + log.warning( + f"Could not read Claude managed settings at {path}; treating bypass as disabled: {exc}" + ) + return {"disableBypassPermissionsMode": True} + return None + + +def claude_bypass_disabled_by_managed_settings() -> bool: + permissions = _claude_managed_permissions() + return bool(permissions and permissions.get("disableBypassPermissionsMode")) + + +def claude_managed_default_permission_mode() -> Optional[str]: + """The ``permissions.defaultMode`` from managed settings, when valid.""" + permissions = _claude_managed_permissions() or {} + mode = permissions.get("defaultMode") + return mode if mode in CLAUDE_PERMISSION_MODES else None + + +def resolve_permission_mode(requested: str, allow_bypass: bool) -> str: + """Clamp a client-requested permission mode at the request boundary. + + Fails closed: unknown modes collapse to default, and bypass is only + honored when both the NBI policy allows it and managed settings don't + forbid it, so a hand-rolled websocket request can't escalate past + what the selector UI offers. + """ + if requested not in CLAUDE_PERMISSION_MODES: + return DEFAULT_PERMISSION_MODE + if requested == CLAUDE_BYPASS_PERMISSION_MODE and ( + not allow_bypass or claude_bypass_disabled_by_managed_settings() + ): + return DEFAULT_PERMISSION_MODE + return requested + + +# These module globals are written only from the single Claude client +# thread (connect, the query loop, and the SDK-invoked permission handler +# all run there), mirroring the existing _current_claude_client / +# set_current_request singletons. The only cross-thread hop is +# write_message, which marshals onto the Tornado loop. A future +# multi-client or per-session refactor would need to revisit this. +_current_permission_mode = DEFAULT_PERMISSION_MODE +_permission_mode_notifier: Optional[ThreadSafeWebSocketConnector] = None + + +def set_permission_mode_notifier(connector: Optional[ThreadSafeWebSocketConnector]): + global _permission_mode_notifier + _permission_mode_notifier = connector + + +def get_current_permission_mode() -> str: + return _current_permission_mode + + +def _notify_permission_mode(mode: str): + if _permission_mode_notifier is None: + return + try: + _permission_mode_notifier.write_message({ + "type": BackendMessageType.ClaudePermissionModeChange, + "data": {"mode": mode} + }) + except Exception as e: + log.error(f"Error occurred while sending permission mode change to websocket: {str(e)}") + + +def reset_permission_mode_tracking(): + """A fresh SDK client starts in default mode; realign tracking and the UI. + + The notification carries the selector's *starting* value, which honors + managed settings' defaultMode; the next request applies it. Bypass never + survives a reset, even as a managed default: re-arming is always manual. + """ + global _current_permission_mode + _current_permission_mode = DEFAULT_PERMISSION_MODE + starting = claude_managed_default_permission_mode() or DEFAULT_PERMISSION_MODE + if starting == CLAUDE_BYPASS_PERMISSION_MODE: + starting = DEFAULT_PERMISSION_MODE + _notify_permission_mode(starting) + + +async def apply_permission_mode(sdk_client: ClaudeSDKClient, mode: str) -> bool: + """Switch the SDK's permission mode when it differs from the applied one. + + Every switch goes through here (selector requests, the hidden plan-mode + slash aliases, and the plan-approval reset) so the tracked mode and the + UI notification can't drift from what the SDK was actually told. + + This does NOT re-check the bypass policy: callers must pass a mode that + was already clamped by ``resolve_permission_mode`` at the request + boundary, or a hardcoded non-bypass literal. Bypass must never reach + here unclamped. + """ + global _current_permission_mode + if mode not in CLAUDE_PERMISSION_MODES or mode == _current_permission_mode: + return False + await sdk_client.set_permission_mode(mode) + _current_permission_mode = mode + _notify_permission_mode(mode) + return True + def tool_text_response(text: Any, *, is_error: bool = False) -> dict[str, Any]: """Shape an MCP tool result. Set ``is_error=True`` for rejection paths. @@ -768,6 +903,8 @@ async def _client_thread_func(self): self._set_status(ClaudeAgentClientStatus.Connected) self._connect_resolved.set() set_current_claude_client(client) + set_permission_mode_notifier(self._websocket_connector) + reset_permission_mode_tracking() while True: queue = self._client_queue @@ -784,6 +921,13 @@ async def _client_thread_func(self): set_current_request(request) set_current_response(response) + # Refresh per query: the websocket reconnects on + # every page reload while this client thread (and + # its agent subprocess) persists, so the connector + # captured at connect time goes stale. The + # participant pushes the live connector onto this + # client, so re-read it here. + set_permission_mode_notifier(self._websocket_connector) messages = request.chat_history query_lines = [] @@ -797,16 +941,27 @@ async def _client_thread_func(self): already_handled = False + # Hidden aliases kept for one release after the + # selector replaced them in the UI (issue #359); + # they're filtered out of autocomplete on the + # frontend but still work when typed. if client_query.startswith('/enter-plan-mode'): - await client.set_permission_mode("plan") + await apply_permission_mode(client, "plan") response.stream(MarkdownData("✓ Entered plan mode")) already_handled = True elif client_query.startswith('/exit-plan-mode'): - await client.set_permission_mode("default") + await apply_permission_mode(client, DEFAULT_PERMISSION_MODE) response.stream(MarkdownData("✓ Exit plan mode")) already_handled = True if not already_handled and not request.cancel_token.is_cancel_requested: + # The mode was clamped against the bypass + # policy and managed settings at the websocket + # boundary; here it only needs applying. + await apply_permission_mode( + client, + request.permission_mode or DEFAULT_PERMISSION_MODE, + ) await client.query(client_query) # Per-query map from tool_use_id to its # (title, kind) so the ToolResultBlock echo @@ -1401,7 +1556,7 @@ async def custom_permission_handler( )) user_input = await ChatResponse.wait_for_chat_user_input(response, callback_id) if user_input['confirmed'] == True: - await get_current_claude_client().set_permission_mode("default") + await apply_permission_mode(get_current_claude_client(), DEFAULT_PERMISSION_MODE) return PermissionResultAllow(updated_input={"message": "Plan approved", "approved": True}) else: return PermissionResultDeny(message="User did not confirm the plan", interrupt=True) diff --git a/notebook_intelligence/extension.py b/notebook_intelligence/extension.py index 75f52d29..dba70275 100644 --- a/notebook_intelligence/extension.py +++ b/notebook_intelligence/extension.py @@ -53,7 +53,15 @@ reject_dangerous_env_keys, validate_mcp_stdio_command, ) -from notebook_intelligence.claude import ClaudeCodeChatParticipant, fetch_claude_models +from notebook_intelligence.claude import ( + CLAUDE_BYPASS_PERMISSION_MODE, + ClaudeCodeChatParticipant, + DEFAULT_PERMISSION_MODE, + claude_bypass_disabled_by_managed_settings, + claude_managed_default_permission_mode, + fetch_claude_models, + resolve_permission_mode, +) from notebook_intelligence.claude_mcp_manager import ClaudeMCPManager from notebook_intelligence.plugin_manager import PluginManager from notebook_intelligence.tour_config import load_tour_config @@ -328,6 +336,11 @@ def _resolve_positive_int_with_env(env_var_name: str, traitlet_value: int) -> in "NBI_CLAUDE_PLUGINS_MANAGEMENT_POLICY", "claude_plugins_management_policy", ), + ( + "claude_bypass_permissions", + "NBI_CLAUDE_BYPASS_PERMISSIONS_POLICY", + "claude_bypass_permissions_policy", + ), ( "terminal_drag_drop", "NBI_TERMINAL_DRAG_DROP_POLICY", @@ -341,6 +354,12 @@ def _resolve_positive_int_with_env(env_var_name: str, traitlet_value: int) -> in ) FEATURE_POLICY_NAMES = tuple(name for name, _, _ in FEATURE_POLICY_SPEC) +# Fallback used when a policies dict hasn't been populated (handler classes +# before _setup_handlers, direct construction in tests). Everything defaults +# to user-choice except bypass, which must fail closed. +FEATURE_POLICY_DEFAULTS = {name: POLICY_USER_CHOICE for name in FEATURE_POLICY_NAMES} +FEATURE_POLICY_DEFAULTS["claude_bypass_permissions"] = POLICY_FORCE_OFF + # ``(setting_lock_name, env_var)`` pairs for the value-presence-locks. The # claude_api_key entry maps to ANTHROPIC_API_KEY (the SDK's native convention) # rather than an NBI-prefixed env var. Same for claude_base_url. @@ -388,6 +407,12 @@ def _build_feature_policies_response(policies: dict, nbi_config) -> dict: "skills_management": True, "claude_mcp_management": True, "claude_plugins_management": True, + # Gates only whether the Bypass Permissions option is offered in + # the permission-mode selector; the user still arms it per + # session. force-on grants the same availability as user-choice + # and never auto-arms bypass. Defaults to force-off, the only + # policy with a non-user-choice default. + "claude_bypass_permissions": True, "terminal_drag_drop": True, "refresh_open_files_on_disk_change": nbi_config.refresh_open_files_on_disk_change, } @@ -395,12 +420,33 @@ def _build_feature_policies_response(policies: dict, nbi_config) -> dict: response = {} for name in FEATURE_POLICY_NAMES: enabled, locked = resolve_feature_flag( - policies.get(name, POLICY_USER_CHOICE), user_values[name] + policies.get(name, FEATURE_POLICY_DEFAULTS[name]), user_values[name] ) response[name] = {"enabled": enabled, "locked": locked} + + # Defense in depth: Claude Code's enterprise managed settings can + # disable bypass independently of the NBI policy. Fold that into the + # one answer the frontend reads so the selector and the server's + # request clamp can't disagree. + if claude_bypass_disabled_by_managed_settings(): + response["claude_bypass_permissions"] = {"enabled": False, "locked": True} return response +def _resolve_default_permission_mode() -> str: + """Starting permission mode for the selector in a new Claude session. + + Honors managed settings' ``permissions.defaultMode`` except for bypass, + which never applies without the user arming it explicitly, no matter + who asks; a managed default of bypass collapses to default the same + way the reconnect reset does. + """ + managed_default = claude_managed_default_permission_mode() + if managed_default is None or managed_default == CLAUDE_BYPASS_PERMISSION_MODE: + return DEFAULT_PERMISSION_MODE + return managed_default + + def _build_setting_locks_response(string_overrides: dict) -> dict: """Surface lock state for non-boolean settings (model pickers, API key, base URL). @@ -580,6 +626,11 @@ def is_provider_enabled(provider_id: str) -> bool: self.feature_policies, nbi_config ), "setting_locks": _build_setting_locks_response(self.string_overrides), + # Starting mode for the permission-mode selector: managed + # settings' permissions.defaultMode when present and valid, + # else "default". Bypass never starts armed regardless of + # managed settings or policy, so it collapses to "default". + "claude_permission_default_mode": _resolve_default_permission_mode(), } for participant_id in ai_service_manager.chat_participants: participant = ai_service_manager.chat_participants[participant_id] @@ -2183,6 +2234,10 @@ class WebsocketCopilotHandler(WebSocketMixin, websocket.WebSocketHandler, Jupyte # leaving the default 10 MiB headroom for memory amplification. max_message_size = 4 * 1024 * 1024 + # Resolved from the claude_bypass_permissions policy in _setup_handlers; + # False (fail closed) until then. + claude_bypass_permissions_allowed = False + # Inheritance matches Jupyter's first-party WS handlers (e.g. # KernelWebsocketHandler): ``WebSocketMixin`` adds ping/pong # keepalive plus a ``prepare`` that routes through Jupyter's @@ -2255,6 +2310,12 @@ def on_message(self, message): mcp_server_tools=toolSelections.get('mcpServers', {}), extension_tools=toolSelections.get('extensions', {}) ) + # Clamp at the boundary so a hand-rolled request can't reach + # bypass when the policy or managed settings forbid it. + permission_mode = resolve_permission_mode( + data.get('permissionMode', DEFAULT_PERMISSION_MODE), + self.claude_bypass_permissions_allowed, + ) is_claude_code_mode = ai_service_manager.is_claude_code_mode chat_history = self.chat_history.get_history(chatId) @@ -2496,7 +2557,7 @@ def on_message(self, message): # last prompt is added later request_chat_history = chat_history[chat_history_initial_size:-1] if is_claude_code_mode else chat_history[:-1] - coro = ai_service_manager.handle_chat_request(ChatRequest(chat_mode=chat_mode, tool_selection=tool_selection, prompt=prompt, chat_history=request_chat_history, cancel_token=cancel_token, rule_context=rule_context), response_emitter) + coro = ai_service_manager.handle_chat_request(ChatRequest(chat_mode=chat_mode, tool_selection=tool_selection, prompt=prompt, chat_history=request_chat_history, cancel_token=cancel_token, rule_context=rule_context, permission_mode=permission_mode), response_emitter) thread = threading.Thread(target=self._run_request_thread, args=(coro, messageId)) thread.start() elif messageType == RequestDataType.GenerateCode: @@ -2923,6 +2984,24 @@ class NotebookIntelligence(ExtensionApp): config=True, ) + claude_bypass_permissions_policy = TraitletEnum( + list(VALID_POLICIES), + default_value=POLICY_FORCE_OFF, + help=""" + Org-wide policy for offering "Bypass Permissions" in the Claude + permission-mode selector. "force-off" (the default; the only + policy that doesn't default to user-choice) hides the option and + the server rejects bypass requests. "user-choice" exposes the + option; the user still has to arm it explicitly per session. + "force-on" grants the same availability as user-choice and never + auto-arms bypass. Independent of this policy, bypass is refused + whenever Claude Code's enterprise managed settings set + permissions.disableBypassPermissionsMode. Overridden by the + NBI_CLAUDE_BYPASS_PERMISSIONS_POLICY env var. + """, + config=True, + ) + terminal_drag_drop_policy = TraitletEnum( list(VALID_POLICIES), default_value=POLICY_USER_CHOICE, @@ -3258,6 +3337,9 @@ def _setup_handlers(self, web_app, feature_policies: dict, string_overrides: dic PluginsBaseHandler.claude_plugins_management_enabled = not is_force_off( feature_policies, "claude_plugins_management" ) + WebsocketCopilotHandler.claude_bypass_permissions_allowed = not is_force_off( + feature_policies, "claude_bypass_permissions" + ) PluginsBaseHandler.allow_github_plugin_import = _resolve_bool_with_env( "NBI_ALLOW_GITHUB_PLUGIN_IMPORT", self.allow_github_plugin_import ) diff --git a/src/api.ts b/src/api.ts index d20e348c..bfca84c4 100644 --- a/src/api.ts +++ b/src/api.ts @@ -276,6 +276,7 @@ export type FeaturePolicyName = | 'skills_management' | 'claude_mcp_management' | 'claude_plugins_management' + | 'claude_bypass_permissions' | 'terminal_drag_drop' | 'refresh_open_files_on_disk_change'; @@ -404,6 +405,10 @@ export class NBIConfig { return this.capabilities.claude_cli_available === true; } + get claudePermissionDefaultMode(): string { + return this.capabilities.claude_permission_default_mode ?? 'default'; + } + get isOpenCodeCliAvailable(): boolean { return this.capabilities.opencode_cli_available === true; } @@ -505,6 +510,7 @@ export class NBIConfig { 'skills_management', 'claude_mcp_management', 'claude_plugins_management', + 'claude_bypass_permissions', 'terminal_drag_drop', 'refresh_open_files_on_disk_change' ]; @@ -584,6 +590,7 @@ export class NBIAPI { // The chat sidebar uses it to drive the "Generating" indicator's pulse // and to swap to a "server may be slow" copy when the gap stretches. static claudeCodeHeartbeat = new Signal(this); + static claudePermissionModeChanged = new Signal(this); static async initialize() { await this.fetchCapabilities(); @@ -614,6 +621,8 @@ export class NBIAPI { this.skillsReloaded.emit(); } else if (msg.type === BackendMessageType.ClaudeCodeHeartbeat) { this.claudeCodeHeartbeat.emit(); + } else if (msg.type === BackendMessageType.ClaudePermissionModeChange) { + this.claudePermissionModeChanged.emit(msg.data?.mode ?? 'default'); } }); } @@ -1245,7 +1254,8 @@ export class NBIAPI { additionalContext: IContextItem[], chatMode: string, toolSelections: IToolSelections, - responseEmitter: IChatCompletionResponseEmitter + responseEmitter: IChatCompletionResponseEmitter, + permissionMode: string = 'default' ) { this._subscribeUntilStreamEnd(messageId, responseEmitter); this._webSocket.send( @@ -1260,7 +1270,8 @@ export class NBIAPI { filename, additionalContext, chatMode, - toolSelections + toolSelections, + permissionMode } }) ); diff --git a/src/chat-sidebar.tsx b/src/chat-sidebar.tsx index 5b4667ed..e7856f20 100644 --- a/src/chat-sidebar.tsx +++ b/src/chat-sidebar.tsx @@ -80,6 +80,10 @@ import { mcpServerSettingsToEnabledState } from './components/mcp-util'; import claudeSvgStr from '../style/icons/claude.svg'; import { AskUserQuestion } from './components/ask-user-question'; import { ClaudeSessionPicker } from './components/claude-session-picker'; +import { + BYPASS_PERMISSIONS_MODE, + PermissionModeSelect +} from './components/permission-mode-select'; import { ToolCallGroup } from './components/tool-call-group'; import { upsertToolCallContent } from './tool-call-stream'; import { TourOverlay } from './tour/tour-overlay'; @@ -114,6 +118,7 @@ export interface IRunChatCompletionRequest { additionalContext?: IContextItem[]; chatMode: string; toolSelections?: IToolSelections; + permissionMode?: string; // Optional id used by external listeners (e.g. the notebook toolbar // generation popover) to track progress when the chat sidebar is hidden. externalRequestId?: string; @@ -1187,7 +1192,8 @@ async function submitCompletionRequest( request.additionalContext || [], request.chatMode, request.toolSelections || {}, - responseEmitter + responseEmitter, + request.permissionMode || 'default' ); case RunChatCompletionType.ExplainThis: case RunChatCompletionType.FixThis: { @@ -1394,6 +1400,15 @@ function SidebarComponent(props: any) { const fileInputRef = useRef(null); const telemetryEmitter: ITelemetryEmitter = props.getTelemetryEmitter(); const [chatMode, setChatMode] = useState(NBIAPI.config.defaultChatMode); + const [permissionMode, setPermissionMode] = useState( + NBIAPI.config.claudePermissionDefaultMode + ); + const [bypassPermissionsAllowed, setBypassPermissionsAllowed] = useState( + NBIAPI.config.featurePolicies.claude_bypass_permissions.enabled + ); + // Ref mirror so memoized request handlers read the live mode without + // adding it to their dependency arrays. + const permissionModeRef = useRef(permissionMode); const [toolSelectionTitle, setToolSelectionTitle] = useState('Tool selection'); @@ -2321,6 +2336,36 @@ function SidebarComponent(props: any) { }; }, []); + useEffect(() => { + permissionModeRef.current = permissionMode; + }, [permissionMode]); + + // Track the bypass policy across capability refreshes (an armed mode must + // not outlive a policy flip) and follow server-driven mode changes (plan + // approval, the hidden plan-mode slash aliases, client restart) so the + // selector always shows what the next turn will use. + useEffect(() => { + const configHandler = () => { + const allowed = + NBIAPI.config.featurePolicies.claude_bypass_permissions.enabled; + setBypassPermissionsAllowed(allowed); + if (!allowed) { + setPermissionMode(mode => + mode === BYPASS_PERMISSIONS_MODE ? 'default' : mode + ); + } + }; + const modeHandler = (_: unknown, mode: string) => { + setPermissionMode(mode); + }; + NBIAPI.configChanged.connect(configHandler); + NBIAPI.claudePermissionModeChanged.connect(modeHandler); + return () => { + NBIAPI.configChanged.disconnect(configHandler); + NBIAPI.claudePermissionModeChanged.disconnect(modeHandler); + }; + }, []); + // Drive the elapsed-time counter while a request is in flight. The same // interval re-evaluates whether the heartbeat has gone stale so the // indicator copy can swap to "Still working..." without a second timer. @@ -2701,8 +2746,9 @@ function SidebarComponent(props: any) { prefixes.push(`/${command}`); } } - prefixes.push('/enter-plan-mode'); - prefixes.push('/exit-plan-mode'); + // /enter-plan-mode and /exit-plan-mode were replaced by the + // permission-mode selector; they still work when typed (hidden + // aliases, one-release deprecation) but no longer autocomplete. } else { if (chatMode === 'ask') { const chatParticipants = NBIAPI.config.chatParticipants; @@ -3041,7 +3087,8 @@ function SidebarComponent(props: any) { filename: activeDocInfo.filePath, additionalContext, chatMode, - toolSelections: toolSelections + toolSelections: toolSelections, + permissionMode: permissionModeRef.current }, { emit: async response => { @@ -3278,6 +3325,7 @@ function SidebarComponent(props: any) { resetPrefixSuggestions(); setPromptHistory([]); setPromptHistoryIndex(0); + setPermissionMode(NBIAPI.config.claudePermissionDefaultMode); }; const onPromptKeyDown = async (event: KeyboardEvent) => { @@ -3410,6 +3458,9 @@ function SidebarComponent(props: any) { (eventData: any) => { const request: IRunChatCompletionRequest = eventData.detail; request.chatId = chatId; + // Consolidated here so every entrypoint (sidebar input, toolbar + // popovers, extension-fired prompts) uses the selector's mode. + request.permissionMode = permissionModeRef.current; let message = ''; switch (request.type) { case RunChatCompletionType.ExplainThis: @@ -4337,6 +4388,13 @@ function SidebarComponent(props: any) { {selectedToolCount > 0 && <>{selectedToolCount}} )} + {NBIAPI.config.isInClaudeCodeMode && ( + + )} {NBIAPI.config.isInClaudeCodeMode && ( + +import React, { useEffect, useRef, useState } from 'react'; +import { VscCheck, VscShield, VscWarning } from '../icons'; + +export const BYPASS_PERMISSIONS_MODE = 'bypassPermissions'; + +const MODE_LABELS: ReadonlyArray<{ mode: string; label: string }> = [ + { mode: 'default', label: 'Default' }, + { mode: 'acceptEdits', label: 'Accept Edits' }, + { mode: 'plan', label: 'Plan' } +]; + +function labelFor(mode: string): string { + const found = MODE_LABELS.find(m => m.mode === mode); + if (found) { + return found.label; + } + return mode === BYPASS_PERMISSIONS_MODE ? 'Bypass Permissions' : 'Default'; +} + +export interface IPermissionModeSelectProps { + value: string; + bypassAllowed: boolean; + onModeChange: (mode: string) => void; +} + +/** + * Permission-mode picker for Claude mode (issue #359). + * + * A compact footer icon button that opens a menu of modes rather than a + * wide dropdown, to keep the input footer uncluttered. Default / Accept + * Edits / Plan switch immediately. Bypass Permissions is listed only when + * the admin policy allows it and never switches on the first click: + * choosing it opens a confirm-to-arm step. While armed, the button renders + * a red warning glyph as a persistent (non-color-only) indicator. + */ +export function PermissionModeSelect( + props: IPermissionModeSelectProps +): JSX.Element { + const [open, setOpen] = useState(false); + const [pendingBypass, setPendingBypass] = useState(false); + const bypassActive = props.value === BYPASS_PERMISSIONS_MODE; + const buttonRef = useRef(null); + const menuRef = useRef(null); + const containerRef = useRef(null); + const cancelRef = useRef(null); + + // Close the menu on an outside click, matching the @-autocomplete and + // tools popovers. + useEffect(() => { + if (!open) { + return; + } + const handleClickOutside = (event: MouseEvent) => { + if (!containerRef.current?.contains(event.target as Node)) { + setOpen(false); + } + }; + document.addEventListener('mousedown', handleClickOutside); + return () => document.removeEventListener('mousedown', handleClickOutside); + }, [open]); + + // Move focus into the menu when it opens (the current item) so keyboard + // and screen-reader users land on it. + useEffect(() => { + if (open) { + menuRef.current + ?.querySelector('[aria-checked="true"]') + ?.focus(); + } + }, [open]); + + // Focus Cancel when the confirm-to-arm dialog opens. + useEffect(() => { + if (pendingBypass) { + cancelRef.current?.focus(); + } + }, [pendingBypass]); + + const closeMenu = (restoreFocus = true) => { + setOpen(false); + if (restoreFocus) { + buttonRef.current?.focus(); + } + }; + + const chooseMode = (mode: string) => { + if (mode === BYPASS_PERMISSIONS_MODE) { + setOpen(false); + setPendingBypass(true); + return; + } + setOpen(false); + buttonRef.current?.focus(); + props.onModeChange(mode); + }; + + const dismissConfirm = () => { + setPendingBypass(false); + buttonRef.current?.focus(); + }; + + // Offer bypass when the policy allows it, and also when it is already the + // active mode (a policy flip mid-session shouldn't drop the armed value + // from the menu before the sidebar resets it). + const options = + props.bypassAllowed || bypassActive + ? [ + ...MODE_LABELS, + { mode: BYPASS_PERMISSIONS_MODE, label: 'Bypass Permissions' } + ] + : MODE_LABELS; + + return ( +
+ {pendingBypass && ( +
{ + if (event.key === 'Escape') { + event.stopPropagation(); + dismissConfirm(); + } + }} + > +
+ Bypass Permissions runs every tool call without asking for + confirmation, with your full user account access. Content the agent + reads can steer what it runs. Stays on until you switch modes or + start a new session. +
+
+ + +
+
+ )} + {open && ( +
{ + if (event.key === 'Escape') { + event.stopPropagation(); + closeMenu(); + return; + } + if (event.key === 'ArrowDown' || event.key === 'ArrowUp') { + // Roving focus between items, the expected role=menu behavior. + event.preventDefault(); + const items = Array.from( + menuRef.current?.querySelectorAll( + '[role="menuitemradio"]' + ) ?? [] + ); + const current = items.indexOf( + document.activeElement as HTMLButtonElement + ); + const delta = event.key === 'ArrowDown' ? 1 : -1; + const next = (current + delta + items.length) % items.length; + items[next]?.focus(); + } + }} + > + {options.map(({ mode, label }) => { + const selected = props.value === mode; + return ( + + ); + })} +
+ )} + +
+ ); +} diff --git a/src/icons.ts b/src/icons.ts index 6c983b09..d6978b57 100644 --- a/src/icons.ts +++ b/src/icons.ts @@ -54,6 +54,7 @@ export const VscPassFilled = asIcon(Vsc.VscPassFilled); export const VscRefresh = asIcon(Vsc.VscRefresh); export const VscSend = asIcon(Vsc.VscSend); export const VscSettingsGear = asIcon(Vsc.VscSettingsGear); +export const VscShield = asIcon(Vsc.VscShield); export const VscSparkle = asIcon(Vsc.VscSparkle); export const VscStopCircle = asIcon(Vsc.VscStopCircle); export const VscSync = asIcon(Vsc.VscSync); diff --git a/src/tokens.ts b/src/tokens.ts index b6c9596a..8998ca45 100644 --- a/src/tokens.ts +++ b/src/tokens.ts @@ -36,6 +36,7 @@ export enum BackendMessageType { MCPServerStatusChange = 'mcp-server-status-change', ClaudeCodeStatusChange = 'claude-code-status-change', ClaudeCodeHeartbeat = 'claude-code-heartbeat', + ClaudePermissionModeChange = 'claude-permission-mode-change', SkillsReloaded = 'skills-reloaded' } diff --git a/style/base.css b/style/base.css index 13620d75..8c534741 100644 --- a/style/base.css +++ b/style/base.css @@ -344,6 +344,95 @@ outline: none; } +.permission-mode-container { + position: relative; + display: flex; + align-items: center; +} + +/* The armed-bypass color must out-specify the shared footer-button + :hover and :focus-visible rules (the button takes focus after arming), + so each state is listed explicitly. */ +.permission-mode-button.permission-mode-button-bypass, +.permission-mode-button.permission-mode-button-bypass:hover, +.permission-mode-button.permission-mode-button-bypass:focus-visible { + color: var(--jp-error-color1); +} + +.permission-mode-menu { + position: absolute; + bottom: 32px; + right: 0; + z-index: 100; + min-width: 160px; + padding: 4px; + background-color: var(--jp-layout-color1); + border: 1px solid var(--jp-border-color1); + border-radius: 4px; + box-shadow: 0 2px 8px rgb(0 0 0 / 30%); +} + +.permission-mode-menu-item { + display: flex; + align-items: center; + gap: 6px; + width: 100%; + box-sizing: border-box; + padding: 4px 8px; + border: none; + background: none; + text-align: left; + cursor: pointer; + color: var(--jp-ui-font-color1); + font-size: var(--jp-ui-font-size1); + border-radius: 2px; +} + +.permission-mode-menu-item:hover, +.permission-mode-menu-item:focus-visible { + background-color: var(--jp-layout-color2); + outline: none; +} + +.permission-mode-menu-item-bypass { + color: var(--jp-error-color1); +} + +.permission-mode-menu-check { + display: inline-flex; + width: 16px; + flex-shrink: 0; +} + +.permission-mode-confirm { + position: absolute; + bottom: 32px; + right: 0; + z-index: 100; + + /* width capped so the dialog stays inside a narrow docked sidebar. */ + width: 280px; + max-width: calc(100vw - 32px); + box-sizing: border-box; + padding: 10px; + background-color: var(--jp-layout-color1); + border: 1px solid var(--jp-error-color1); + border-radius: 4px; + box-shadow: 0 2px 8px rgb(0 0 0 / 30%); +} + +.permission-mode-confirm-message { + color: var(--jp-ui-font-color1); + font-size: var(--jp-ui-font-size1); + margin-bottom: 8px; +} + +.permission-mode-confirm-buttons { + display: flex; + justify-content: flex-end; + gap: 6px; +} + .user-input-footer-button { width: 24px; height: 24px; diff --git a/tests/conftest.py b/tests/conftest.py index c0f0e170..c75600e7 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -10,6 +10,22 @@ from notebook_intelligence.config import NBIConfig +@pytest.fixture(autouse=True) +def _no_real_managed_settings(monkeypatch): + """Stop the Claude managed-settings probe from reading real OS paths. + + ``_build_feature_policies_response`` and ``resolve_permission_mode`` now + consult Claude Code's enterprise managed-settings files (issue #359). On + a developer machine or CI image that actually has one installed, that + would couple unrelated capabilities/policy tests to the host. Default + every test to "unmanaged" (empty path tuple); the permission-mode tests + re-point this to their own tmp file where they need a managed file. + """ + import notebook_intelligence.claude as claude_module + + monkeypatch.setattr(claude_module, "_MANAGED_SETTINGS_PATHS", ()) + + def stub_claude_subprocess( monkeypatch, *, diff --git a/tests/test_permission_modes.py b/tests/test_permission_modes.py new file mode 100644 index 00000000..3d4ad4d4 --- /dev/null +++ b/tests/test_permission_modes.py @@ -0,0 +1,251 @@ +# Copyright (c) Mehmet Bektas + +"""Tests for the Claude permission-mode selector backend (issue #359).""" + +import asyncio +import json +from unittest.mock import AsyncMock, MagicMock + +import pytest + +import notebook_intelligence.claude as claude_module +from notebook_intelligence.claude import ( + CLAUDE_BYPASS_PERMISSION_MODE, + CLAUDE_PERMISSION_MODES, + DEFAULT_PERMISSION_MODE, + apply_permission_mode, + claude_bypass_disabled_by_managed_settings, + claude_managed_default_permission_mode, + reset_permission_mode_tracking, + resolve_permission_mode, + set_permission_mode_notifier, +) +from notebook_intelligence.extension import ( + FEATURE_POLICY_DEFAULTS, + _build_feature_policies_response, + _resolve_default_permission_mode, +) +from notebook_intelligence.feature_flags import POLICY_FORCE_OFF, POLICY_USER_CHOICE + + +@pytest.fixture(autouse=True) +def _no_managed_settings(monkeypatch, tmp_path): + """Point the managed-settings probe at a tmp location by default. + + Without this, a developer machine that actually has enterprise managed + settings installed would bleed into every assertion below. Tests that + need a managed file write to ``managed_path`` and re-point the tuple. + """ + managed = tmp_path / "managed-settings.json" + monkeypatch.setattr(claude_module, "_MANAGED_SETTINGS_PATHS", (str(managed),)) + yield managed + + +@pytest.fixture(autouse=True) +def _reset_tracking(): + set_permission_mode_notifier(None) + claude_module._current_permission_mode = DEFAULT_PERMISSION_MODE + yield + set_permission_mode_notifier(None) + claude_module._current_permission_mode = DEFAULT_PERMISSION_MODE + + +class TestResolvePermissionMode: + @pytest.mark.parametrize("mode", ["default", "acceptEdits", "plan"]) + def test_normal_modes_pass_regardless_of_policy(self, mode): + assert resolve_permission_mode(mode, False) == mode + assert resolve_permission_mode(mode, True) == mode + + @pytest.mark.parametrize( + "requested", ["dontAsk", "auto", "", "DEFAULT", "Plan", None, 42] + ) + def test_unknown_modes_collapse_to_default(self, requested): + assert resolve_permission_mode(requested, True) == DEFAULT_PERMISSION_MODE + + def test_bypass_rejected_when_policy_disallows(self): + assert ( + resolve_permission_mode(CLAUDE_BYPASS_PERMISSION_MODE, False) + == DEFAULT_PERMISSION_MODE + ) + + def test_bypass_allowed_when_policy_allows(self): + assert ( + resolve_permission_mode(CLAUDE_BYPASS_PERMISSION_MODE, True) + == CLAUDE_BYPASS_PERMISSION_MODE + ) + + def test_bypass_rejected_when_managed_settings_disable_it( + self, _no_managed_settings + ): + _no_managed_settings.write_text( + json.dumps({"permissions": {"disableBypassPermissionsMode": True}}) + ) + assert ( + resolve_permission_mode(CLAUDE_BYPASS_PERMISSION_MODE, True) + == DEFAULT_PERMISSION_MODE + ) + + +class TestManagedSettings: + def test_absent_file_means_unmanaged(self): + assert claude_bypass_disabled_by_managed_settings() is False + assert claude_managed_default_permission_mode() is None + + def test_corrupt_file_fails_closed(self, _no_managed_settings): + _no_managed_settings.write_text("{not json") + assert claude_bypass_disabled_by_managed_settings() is True + + def test_managed_file_without_permissions_block(self, _no_managed_settings): + _no_managed_settings.write_text(json.dumps({"env": {}})) + assert claude_bypass_disabled_by_managed_settings() is False + assert claude_managed_default_permission_mode() is None + + def test_default_mode_read_when_valid(self, _no_managed_settings): + _no_managed_settings.write_text( + json.dumps({"permissions": {"defaultMode": "acceptEdits"}}) + ) + assert claude_managed_default_permission_mode() == "acceptEdits" + + def test_invalid_default_mode_ignored(self, _no_managed_settings): + _no_managed_settings.write_text( + json.dumps({"permissions": {"defaultMode": "yolo"}}) + ) + assert claude_managed_default_permission_mode() is None + + +class TestApplyPermissionMode: + def _sdk_client(self): + client = MagicMock() + client.set_permission_mode = AsyncMock() + return client + + def test_applies_and_tracks_new_mode(self): + client = self._sdk_client() + changed = asyncio.run(apply_permission_mode(client, "plan")) + assert changed is True + client.set_permission_mode.assert_awaited_once_with("plan") + assert claude_module.get_current_permission_mode() == "plan" + + def test_noop_when_mode_unchanged(self): + client = self._sdk_client() + changed = asyncio.run(apply_permission_mode(client, DEFAULT_PERMISSION_MODE)) + assert changed is False + client.set_permission_mode.assert_not_awaited() + + def test_rejects_unknown_mode(self): + client = self._sdk_client() + changed = asyncio.run(apply_permission_mode(client, "dontAsk")) + assert changed is False + client.set_permission_mode.assert_not_awaited() + + def test_notifies_websocket_on_change(self): + client = self._sdk_client() + notifier = MagicMock() + set_permission_mode_notifier(notifier) + asyncio.run(apply_permission_mode(client, "acceptEdits")) + notifier.write_message.assert_called_once() + payload = notifier.write_message.call_args.args[0] + assert payload["data"] == {"mode": "acceptEdits"} + + +class TestResetPermissionModeTracking: + def test_reset_realigns_tracking_and_notifies_default(self): + claude_module._current_permission_mode = "plan" + notifier = MagicMock() + set_permission_mode_notifier(notifier) + reset_permission_mode_tracking() + assert claude_module.get_current_permission_mode() == DEFAULT_PERMISSION_MODE + payload = notifier.write_message.call_args.args[0] + assert payload["data"] == {"mode": DEFAULT_PERMISSION_MODE} + + def test_reset_notification_honors_managed_default(self, _no_managed_settings): + _no_managed_settings.write_text( + json.dumps({"permissions": {"defaultMode": "plan"}}) + ) + notifier = MagicMock() + set_permission_mode_notifier(notifier) + reset_permission_mode_tracking() + # Tracking reflects the SDK's actual state (a fresh client starts in + # default); the notification carries the selector's starting value. + assert claude_module.get_current_permission_mode() == DEFAULT_PERMISSION_MODE + payload = notifier.write_message.call_args.args[0] + assert payload["data"] == {"mode": "plan"} + + def test_managed_bypass_default_never_arms_on_reset(self, _no_managed_settings): + _no_managed_settings.write_text( + json.dumps( + {"permissions": {"defaultMode": CLAUDE_BYPASS_PERMISSION_MODE}} + ) + ) + notifier = MagicMock() + set_permission_mode_notifier(notifier) + reset_permission_mode_tracking() + payload = notifier.write_message.call_args.args[0] + assert payload["data"] == {"mode": DEFAULT_PERMISSION_MODE} + + +class TestPolicyWiring: + def _nbi_config(self): + config = MagicMock() + config.claude_settings = {} + config.enable_explain_error = False + config.enable_output_followup = False + config.enable_output_toolbar = False + config.store_github_access_token = False + config.refresh_open_files_on_disk_change = True + return config + + def test_bypass_policy_defaults_to_force_off(self): + assert ( + FEATURE_POLICY_DEFAULTS["claude_bypass_permissions"] == POLICY_FORCE_OFF + ) + response = _build_feature_policies_response({}, self._nbi_config()) + assert response["claude_bypass_permissions"] == { + "enabled": False, + "locked": True, + } + + def test_bypass_policy_user_choice_offers_option(self): + response = _build_feature_policies_response( + {"claude_bypass_permissions": POLICY_USER_CHOICE}, self._nbi_config() + ) + assert response["claude_bypass_permissions"]["enabled"] is True + + def test_managed_settings_override_policy(self, _no_managed_settings): + _no_managed_settings.write_text( + json.dumps({"permissions": {"disableBypassPermissionsMode": True}}) + ) + response = _build_feature_policies_response( + {"claude_bypass_permissions": POLICY_USER_CHOICE}, self._nbi_config() + ) + assert response["claude_bypass_permissions"] == { + "enabled": False, + "locked": True, + } + + def test_default_permission_mode_from_managed_settings( + self, _no_managed_settings + ): + _no_managed_settings.write_text( + json.dumps({"permissions": {"defaultMode": "acceptEdits"}}) + ) + assert _resolve_default_permission_mode() == "acceptEdits" + + def test_default_permission_mode_never_starts_armed(self, _no_managed_settings): + _no_managed_settings.write_text( + json.dumps( + {"permissions": {"defaultMode": CLAUDE_BYPASS_PERMISSION_MODE}} + ) + ) + assert _resolve_default_permission_mode() == DEFAULT_PERMISSION_MODE + + def test_default_permission_mode_unmanaged(self): + assert _resolve_default_permission_mode() == DEFAULT_PERMISSION_MODE + + def test_exposed_modes_are_the_contract(self): + assert CLAUDE_PERMISSION_MODES == ( + "default", + "acceptEdits", + "plan", + "bypassPermissions", + ) diff --git a/tests/test_websocket_handler_integration.py b/tests/test_websocket_handler_integration.py index 5639754f..f3ba3c03 100644 --- a/tests/test_websocket_handler_integration.py +++ b/tests/test_websocket_handler_integration.py @@ -839,3 +839,112 @@ def test_on_message_claude_mode_no_selection_no_range_pointer( assert "@src/foo.py" in context_message assert "lines" not in context_message assert "selection" not in context_message + + +class TestPermissionModeClamp: + """The websocket boundary is the sole arbiter for bypass (issue #359). + + Hiding the option in the UI is convenience; a hand-rolled request must + still be clamped server-side against the resolved bypass policy. + """ + + def _create_mock_application(self): + app = Mock(spec=Application) + app.settings = {"jinja2_env": None, "headers": {}} + app.ui_methods = {} + app.ui_modules = {} + app.transforms = [] + return app + + def _create_mock_request(self): + request = Mock(spec=HTTPServerRequest) + request.connection = Mock() + return request + + def _handler(self, *, bypass_allowed): + mock_factory = Mock(spec=RuleContextFactory) + mock_factory.create.return_value = Mock(spec=RuleContext) + with patch('notebook_intelligence.extension.ThreadSafeWebSocketConnector'): + handler = WebsocketCopilotHandler( + self._create_mock_application(), + self._create_mock_request(), + context_factory=mock_factory, + ) + handler.claude_bypass_permissions_allowed = bypass_allowed + return handler + + def _send(self, handler, mock_ai_manager, mock_nb_intel, mode): + mock_nb_intel.root_dir = "/workspace" + mock_ai_manager.handle_chat_request = Mock() + data = { + 'chatId': 'c', + 'prompt': 'hi', + 'language': 'python', + 'filename': 'x.ipynb', + 'chatMode': 'ask', + 'toolSelections': {}, + 'additionalContext': [], + } + if mode is not None: + data['permissionMode'] = mode + message = {'id': 'm', 'type': 'chat-request', 'data': data} + handler.on_message(json.dumps(message)) + return mock_ai_manager.handle_chat_request.call_args[0][0] + + def test_class_default_fails_closed(self): + # Before _setup_handlers runs, the gate must be closed. + assert WebsocketCopilotHandler.claude_bypass_permissions_allowed is False + + @patch('notebook_intelligence.extension.ai_service_manager') + @patch('notebook_intelligence.extension.NotebookIntelligence') + @patch('notebook_intelligence.extension.threading.Thread') + def test_bypass_clamped_to_default_when_policy_forbids( + self, mock_thread, mock_nb_intel, mock_ai_manager + ): + handler = self._handler(bypass_allowed=False) + req = self._send( + handler, mock_ai_manager, mock_nb_intel, 'bypassPermissions' + ) + assert req.permission_mode == 'default' + + @patch('notebook_intelligence.extension.ai_service_manager') + @patch('notebook_intelligence.extension.NotebookIntelligence') + @patch('notebook_intelligence.extension.threading.Thread') + def test_bypass_passes_through_when_policy_allows( + self, mock_thread, mock_nb_intel, mock_ai_manager + ): + handler = self._handler(bypass_allowed=True) + req = self._send( + handler, mock_ai_manager, mock_nb_intel, 'bypassPermissions' + ) + assert req.permission_mode == 'bypassPermissions' + + @patch('notebook_intelligence.extension.ai_service_manager') + @patch('notebook_intelligence.extension.NotebookIntelligence') + @patch('notebook_intelligence.extension.threading.Thread') + def test_normal_mode_passes_through( + self, mock_thread, mock_nb_intel, mock_ai_manager + ): + handler = self._handler(bypass_allowed=False) + req = self._send(handler, mock_ai_manager, mock_nb_intel, 'plan') + assert req.permission_mode == 'plan' + + @patch('notebook_intelligence.extension.ai_service_manager') + @patch('notebook_intelligence.extension.NotebookIntelligence') + @patch('notebook_intelligence.extension.threading.Thread') + def test_unknown_mode_clamped_to_default( + self, mock_thread, mock_nb_intel, mock_ai_manager + ): + handler = self._handler(bypass_allowed=True) + req = self._send(handler, mock_ai_manager, mock_nb_intel, 'dontAsk') + assert req.permission_mode == 'default' + + @patch('notebook_intelligence.extension.ai_service_manager') + @patch('notebook_intelligence.extension.NotebookIntelligence') + @patch('notebook_intelligence.extension.threading.Thread') + def test_missing_mode_defaults( + self, mock_thread, mock_nb_intel, mock_ai_manager + ): + handler = self._handler(bypass_allowed=True) + req = self._send(handler, mock_ai_manager, mock_nb_intel, None) + assert req.permission_mode == 'default' diff --git a/tests/ts/permission-mode-select.test.tsx b/tests/ts/permission-mode-select.test.tsx new file mode 100644 index 00000000..9b38b2e9 --- /dev/null +++ b/tests/ts/permission-mode-select.test.tsx @@ -0,0 +1,227 @@ +import React from 'react'; +import { fireEvent, render, screen } from '@testing-library/react'; +import { + BYPASS_PERMISSIONS_MODE, + PermissionModeSelect +} from '../../src/components/permission-mode-select'; + +function openMenu() { + fireEvent.click(screen.getByRole('button', { name: /Permission mode/ })); +} + +describe('PermissionModeSelect', () => { + it('shows the current mode on the trigger button', () => { + render( + + ); + expect( + screen.getByRole('button', { name: 'Permission mode: Plan' }) + ).toBeInTheDocument(); + }); + + it('lists the three normal modes and hides bypass when not allowed', () => { + render( + + ); + openMenu(); + const items = screen.getAllByRole('menuitemradio').map(i => i.textContent); + expect(items).toEqual(['Default', 'Accept Edits', 'Plan']); + }); + + it('lists bypass when the policy allows it', () => { + render( + + ); + openMenu(); + expect( + screen.getByRole('menuitemradio', { name: 'Bypass Permissions' }) + ).toBeInTheDocument(); + }); + + it('marks the active mode as checked', () => { + render( + + ); + openMenu(); + expect( + screen.getByRole('menuitemradio', { name: 'Accept Edits' }) + ).toHaveAttribute('aria-checked', 'true'); + expect( + screen.getByRole('menuitemradio', { name: 'Default' }) + ).toHaveAttribute('aria-checked', 'false'); + }); + + it('switches normal modes immediately and closes the menu', () => { + const onModeChange = jest.fn(); + render( + + ); + openMenu(); + fireEvent.click(screen.getByRole('menuitemradio', { name: 'Plan' })); + expect(onModeChange).toHaveBeenCalledWith('plan'); + expect(screen.queryByRole('menu')).not.toBeInTheDocument(); + }); + + it('requires the confirm-to-arm step before bypass takes effect', () => { + const onModeChange = jest.fn(); + render( + + ); + openMenu(); + fireEvent.click( + screen.getByRole('menuitemradio', { name: 'Bypass Permissions' }) + ); + // Choosing bypass closes the menu and opens the confirm dialog without + // switching the mode. + expect(onModeChange).not.toHaveBeenCalled(); + expect(screen.queryByRole('menu')).not.toBeInTheDocument(); + expect(screen.getByRole('alertdialog')).toBeInTheDocument(); + + fireEvent.click(screen.getByText('Bypass permissions')); + expect(onModeChange).toHaveBeenCalledWith(BYPASS_PERMISSIONS_MODE); + }); + + it('cancelling the arm step leaves the mode unchanged', () => { + const onModeChange = jest.fn(); + render( + + ); + openMenu(); + fireEvent.click( + screen.getByRole('menuitemradio', { name: 'Bypass Permissions' }) + ); + fireEvent.click(screen.getByText('Cancel')); + expect(onModeChange).not.toHaveBeenCalled(); + expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument(); + }); + + it('Escape dismisses the confirm dialog without arming', () => { + const onModeChange = jest.fn(); + render( + + ); + openMenu(); + fireEvent.click( + screen.getByRole('menuitemradio', { name: 'Bypass Permissions' }) + ); + fireEvent.keyDown(screen.getByRole('alertdialog'), { key: 'Escape' }); + expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument(); + expect(onModeChange).not.toHaveBeenCalled(); + }); + + it('ArrowDown/ArrowUp move focus between menu items', () => { + render( + + ); + openMenu(); + // Opens with focus on the checked item (Default, index 0). + expect( + screen.getByRole('menuitemradio', { name: 'Default' }) + ).toHaveFocus(); + fireEvent.keyDown(screen.getByRole('menu'), { key: 'ArrowDown' }); + expect( + screen.getByRole('menuitemradio', { name: 'Accept Edits' }) + ).toHaveFocus(); + fireEvent.keyDown(screen.getByRole('menu'), { key: 'ArrowUp' }); + expect( + screen.getByRole('menuitemradio', { name: 'Default' }) + ).toHaveFocus(); + // Wraps from the first item up to the last. + fireEvent.keyDown(screen.getByRole('menu'), { key: 'ArrowUp' }); + expect(screen.getByRole('menuitemradio', { name: 'Plan' })).toHaveFocus(); + }); + + it('Escape closes the menu', () => { + render( + + ); + openMenu(); + fireEvent.keyDown(screen.getByRole('menu'), { key: 'Escape' }); + expect(screen.queryByRole('menu')).not.toBeInTheDocument(); + }); + + it('moves focus into the confirm dialog when it opens', () => { + render( + + ); + openMenu(); + fireEvent.click( + screen.getByRole('menuitemradio', { name: 'Bypass Permissions' }) + ); + expect(screen.getByText('Cancel')).toHaveFocus(); + }); + + it('shows the armed indicator and aria-label while bypass is active', () => { + render( + + ); + const button = screen.getByRole('button', { + name: 'Permission mode: Bypass Permissions is active' + }); + expect(button).toHaveClass('permission-mode-button-bypass'); + }); + + it('keeps bypass listed if the policy flips while it is active', () => { + // The sidebar resets the mode when the policy flips; until that lands, + // the active bypass must remain visible/selectable in the menu. + render( + + ); + openMenu(); + expect( + screen.getByRole('menuitemradio', { name: 'Bypass Permissions' }) + ).toHaveAttribute('aria-checked', 'true'); + }); +}); From d7164e92b3805c12cfe33cee27632f46344404e0 Mon Sep 17 00:00:00 2001 From: PJ Doland Date: Fri, 12 Jun 2026 19:57:00 -0400 Subject: [PATCH 2/6] docs: document the permission-mode selector's exact behavior The admin guide and policy table covered the bypass gating, but nothing described what each mode does, how the selector behaves, or the slash-command change for end users. Add a Permission modes subsection to the Claude mode docs: what Default / Accept Edits / Plan / Bypass do, that only Default/Accept Edits/Plan switch immediately while Bypass needs a confirm-to-arm step and shows a red indicator, that the mode rides each request and never persists across a new session or a fresh client, the managed-settings deference, and that the /enter-plan-mode and /exit-plan-mode commands still work but are deprecated out of autocomplete. --- README.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/README.md b/README.md index ec484068..b85cb648 100644 --- a/README.md +++ b/README.md @@ -90,6 +90,25 @@ If the Claude Code CLI is on `PATH`, NBI launches it automatically. To override Claude settings +#### Permission modes + +In Claude mode the chat input footer shows a shield-icon button (to the left of the send button) that sets the agent's permission mode for the chat panel, matching the modes in Claude Code and the Claude VS Code extension. Click it to choose: + +- **Default**: every tool call the agent wants to run goes through NBI's confirmation prompt. You approve or reject each one. This is the starting mode. +- **Accept Edits**: file edits the agent makes apply without a per-edit prompt; other tool calls (running commands, etc.) still go through the confirmation prompt. Useful for iterative work where you trust the edits but still want a gate on everything else. +- **Plan**: the agent researches and proposes a plan **without making any changes**, then presents it for approval. Approving runs the plan and returns the selector to **Default**; rejecting keeps it planning. This replaces the old `/enter-plan-mode` slash command. +- **Bypass Permissions**: NBI's confirmation prompt is skipped for **every** tool call, including the Claude Code CLI's own Bash / Write / Edit running in the agent subprocess. The agent runs everything with your full account access and no confirmation, and any untrusted content it reads can steer what it runs. See the gating notes below. + +Default, Accept Edits, and Plan switch the moment you pick them. The selected mode travels with each message you send and is applied to the agent before the turn runs; switching mid-conversation takes effect on your next message. + +The mode does not persist across sessions: starting a **New chat session** (or `/clear`) resets the selector to its starting mode, and a fresh Claude client always begins in Default. + +Choosing **Bypass Permissions** does not arm it immediately. It opens a confirmation step; only after you confirm does bypass take effect, and while it is active the shield turns into a red warning icon as a persistent indicator. Bypass must be re-armed each session: starting a new chat or restarting the Claude client drops back to Default. And because the server re-checks the requested mode on every message, an armed bypass can never outlive a policy that an administrator has since turned off. + +**Admin gating.** Bypass Permissions is **off by default** and hidden from the selector unless an administrator enables it: it is governed by the `claude_bypass_permissions` policy (`NBI_CLAUDE_BYPASS_PERMISSIONS_POLICY`), the only admin policy whose default is `force-off` rather than `user-choice`. The requested mode is also clamped on the server for every message, so the gate can't be bypassed by a hand-crafted request. Independently, NBI honors Claude Code's enterprise [managed settings](https://code.claude.com/docs/en/settings): `permissions.disableBypassPermissionsMode` removes the option regardless of the NBI policy, and `permissions.defaultMode` sets the selector's starting mode (Bypass excepted, since it never auto-arms). See [Allowing Bypass Permissions](docs/admin-guide.md#allowing-bypass-permissions-in-the-claude-permission-mode-selector) in the admin guide. + +The `/enter-plan-mode` and `/exit-plan-mode` slash commands still work if typed but are no longer offered in autocomplete; the selector replaces them and will retire the commands in a future release. + #### Resuming a previous Claude session When Claude mode is on, the chat sidebar shows a history icon next to the gear. Click it to list the Claude Code sessions recorded for the current working directory (the same transcripts the Claude Code CLI stores under `~/.claude/projects/`). Selecting a session reconnects via `resume`, so the next message you send continues that transcript with full prior context. A **New chat session** button next to the gear restarts the SDK client without typing `/clear`. From d0f8141895d2afb295c54b1d7dcfe5ebf3119520 Mon Sep 17 00:00:00 2001 From: PJ Doland Date: Tue, 16 Jun 2026 07:30:38 -0400 Subject: [PATCH 3/6] fix(chat): refine permission-mode selector indicator, reset, and bypass Addresses the maintainer's review feedback on the Claude permission-mode selector (PR #377). 1. Selected-mode indication. The footer button showed the same shield for Default / Accept Edits / Plan, so the active mode wasn't visible at a glance. Each mode now has a distinct glyph (shield / pencil / eye / warning) on the compact icon button, and the menu echoes the same glyphs so the mapping is learnable. A text label was tried first but doesn't fit the narrow footer (it overlapped the mode toggle and send button). 2. Selection reset on reconnect. reset_permission_mode_tracking notified the UI on every (re)connect, clobbering the user's selection back to default when a background connect completed or a skills/config reload reconnected. The reset notification now carries a `reset` flag; the selector applies it only to retire bypass (which still never survives a reset) and otherwise keeps an explicit non-bypass selection. The decision is a pure helper, nextPermissionModeOnNotification, with unit coverage. 3. Bypass Permissions error. Switching to Bypass failed with "the session was not launched with --dangerously-skip-permissions". Launching with that flag was rejected: live testing showed it disables ALL permission prompts for the whole session (Default / Accept Edits / Plan silently ran tools with no confirmation), and it can't be scoped to just the bypass turns. Bypass is now realized in NBI's own permission handler: apply_permission_mode keeps the CLI in default and the handler auto-allows every tool while the tracked mode is bypass, so the gate stays fully under NBI's control and non-bypass modes keep prompting. The request-boundary clamp still keeps bypass unreachable unless the policy allows it. 4. Confirm-dialog buttons. The confirm-to-arm Cancel / Bypass buttons used full-size jp-Dialog-button sizing; they're now compact while staying at least 24px tall for touch targets. Testing: pytest (incl. new TestBypassViaHandler and reset-flag assertions), jest (selector glyphs + the reset-decision helper), tsc, lint. Verified live in JupyterLab: Default and Accept Edits gate tool calls with a confirmation, Bypass runs tools without confirmation and without the launch error, and switching back to Default re-gates. The selector indicator updates instantly and no longer overlaps the footer controls. --- notebook_intelligence/claude.py | 46 ++++++++++---- src/api.ts | 10 ++- src/chat-sidebar.tsx | 10 ++- src/components/permission-mode-select.tsx | 56 ++++++++++++++--- style/base.css | 19 ++++++ tests/test_permission_modes.py | 75 +++++++++++++++++++++-- tests/ts/permission-mode-select.test.tsx | 55 +++++++++++++++++ 7 files changed, 245 insertions(+), 26 deletions(-) diff --git a/notebook_intelligence/claude.py b/notebook_intelligence/claude.py index 95ce6f54..d14bda2a 100644 --- a/notebook_intelligence/claude.py +++ b/notebook_intelligence/claude.py @@ -385,13 +385,13 @@ def get_current_permission_mode() -> str: return _current_permission_mode -def _notify_permission_mode(mode: str): +def _notify_permission_mode(mode: str, reset: bool = False): if _permission_mode_notifier is None: return try: _permission_mode_notifier.write_message({ "type": BackendMessageType.ClaudePermissionModeChange, - "data": {"mode": mode} + "data": {"mode": mode, "reset": reset} }) except Exception as e: log.error(f"Error occurred while sending permission mode change to websocket: {str(e)}") @@ -400,16 +400,20 @@ def _notify_permission_mode(mode: str): def reset_permission_mode_tracking(): """A fresh SDK client starts in default mode; realign tracking and the UI. - The notification carries the selector's *starting* value, which honors - managed settings' defaultMode; the next request applies it. Bypass never - survives a reset, even as a managed default: re-arming is always manual. + The notification is flagged ``reset`` and carries the selector's + *starting* value, which honors managed settings' defaultMode. The UI + applies it only to retire bypass (which never survives a reset, even as + a managed default: re-arming is always manual) and otherwise keeps the + user's explicit selection, so a mid-session reconnect (skills reload, a + config change) doesn't clobber the chosen mode (issue #377). The next + request applies whatever the selector then shows. """ global _current_permission_mode _current_permission_mode = DEFAULT_PERMISSION_MODE starting = claude_managed_default_permission_mode() or DEFAULT_PERMISSION_MODE if starting == CLAUDE_BYPASS_PERMISSION_MODE: starting = DEFAULT_PERMISSION_MODE - _notify_permission_mode(starting) + _notify_permission_mode(starting, reset=True) async def apply_permission_mode(sdk_client: ClaudeSDKClient, mode: str) -> bool: @@ -419,15 +423,28 @@ async def apply_permission_mode(sdk_client: ClaudeSDKClient, mode: str) -> bool: slash aliases, and the plan-approval reset) so the tracked mode and the UI notification can't drift from what the SDK was actually told. - This does NOT re-check the bypass policy: callers must pass a mode that - was already clamped by ``resolve_permission_mode`` at the request - boundary, or a hardcoded non-bypass literal. Bypass must never reach - here unclamped. + Bypass is realized in NBI's own ``custom_permission_handler`` rather than + the CLI's ``bypassPermissions`` mode: the CLI refuses that mode unless the + session was launched with ``--dangerously-skip-permissions``, and that flag + skips ALL permission prompts for the whole session (it can't be scoped to + the bypass turns only), which would silently un-gate default / acceptEdits + / plan too (issue #377). Instead bypass keeps the CLI in ``default`` and the + handler auto-allows every tool while ``_current_permission_mode`` is bypass, + so the gate stays fully under NBI's control. + + This does NOT re-check the bypass policy: callers must pass a mode that was + already clamped by ``resolve_permission_mode`` at the request boundary, or a + hardcoded non-bypass literal. """ global _current_permission_mode if mode not in CLAUDE_PERMISSION_MODES or mode == _current_permission_mode: return False - await sdk_client.set_permission_mode(mode) + cli_mode = ( + DEFAULT_PERMISSION_MODE + if mode == CLAUDE_BYPASS_PERMISSION_MODE + else mode + ) + await sdk_client.set_permission_mode(cli_mode) _current_permission_mode = mode _notify_permission_mode(mode) return True @@ -1523,6 +1540,13 @@ async def custom_permission_handler( log.debug(f"Custom permission handler called for tool {tool_name} with input {input_data} and context {context}") + # Bypass Permissions: auto-allow every tool without prompting. NBI realizes + # bypass here rather than via the CLI's bypassPermissions mode (which needs + # an un-scopable launch flag); the mode only reaches here clamped against + # the policy at the request boundary (issue #377). + if get_current_permission_mode() == CLAUDE_BYPASS_PERMISSION_MODE: + return PermissionResultAllow() + response = get_current_response() callback_id = str(uuid.uuid4()) diff --git a/src/api.ts b/src/api.ts index bfca84c4..203dd3a0 100644 --- a/src/api.ts +++ b/src/api.ts @@ -590,7 +590,10 @@ export class NBIAPI { // The chat sidebar uses it to drive the "Generating" indicator's pulse // and to swap to a "server may be slow" copy when the gap stretches. static claudeCodeHeartbeat = new Signal(this); - static claudePermissionModeChanged = new Signal(this); + static claudePermissionModeChanged = new Signal< + unknown, + { mode: string; reset: boolean } + >(this); static async initialize() { await this.fetchCapabilities(); @@ -622,7 +625,10 @@ export class NBIAPI { } else if (msg.type === BackendMessageType.ClaudeCodeHeartbeat) { this.claudeCodeHeartbeat.emit(); } else if (msg.type === BackendMessageType.ClaudePermissionModeChange) { - this.claudePermissionModeChanged.emit(msg.data?.mode ?? 'default'); + this.claudePermissionModeChanged.emit({ + mode: msg.data?.mode ?? 'default', + reset: msg.data?.reset ?? false + }); } }); } diff --git a/src/chat-sidebar.tsx b/src/chat-sidebar.tsx index e7856f20..7e766434 100644 --- a/src/chat-sidebar.tsx +++ b/src/chat-sidebar.tsx @@ -82,6 +82,7 @@ import { AskUserQuestion } from './components/ask-user-question'; import { ClaudeSessionPicker } from './components/claude-session-picker'; import { BYPASS_PERMISSIONS_MODE, + nextPermissionModeOnNotification, PermissionModeSelect } from './components/permission-mode-select'; import { ToolCallGroup } from './components/tool-call-group'; @@ -2355,8 +2356,13 @@ function SidebarComponent(props: any) { ); } }; - const modeHandler = (_: unknown, mode: string) => { - setPermissionMode(mode); + const modeHandler = ( + _: unknown, + notification: { mode: string; reset: boolean } + ) => { + setPermissionMode(current => + nextPermissionModeOnNotification(current, notification) + ); }; NBIAPI.configChanged.connect(configHandler); NBIAPI.claudePermissionModeChanged.connect(modeHandler); diff --git a/src/components/permission-mode-select.tsx b/src/components/permission-mode-select.tsx index df7c3d9b..220224da 100644 --- a/src/components/permission-mode-select.tsx +++ b/src/components/permission-mode-select.tsx @@ -1,7 +1,7 @@ // Copyright (c) Mehmet Bektas import React, { useEffect, useRef, useState } from 'react'; -import { VscCheck, VscShield, VscWarning } from '../icons'; +import { VscCheck, VscEdit, VscEye, VscShield, VscWarning } from '../icons'; export const BYPASS_PERMISSIONS_MODE = 'bypassPermissions'; @@ -19,6 +19,42 @@ function labelFor(mode: string): string { return mode === BYPASS_PERMISSIONS_MODE ? 'Bypass Permissions' : 'Default'; } +/** + * Resolve the selector mode after a server permission-mode notification. + * + * A `reset` notification (a fresh SDK session: skills reload, config change, + * restart) must retire bypass but must NOT clobber an explicit non-bypass + * selection the user already made, so a mid-session reconnect doesn't snap the + * selector back to default (issue #377). A non-reset notification is an + * authoritative server-driven switch (plan approval, the slash aliases) and + * applies unconditionally. + */ +export function nextPermissionModeOnNotification( + current: string, + notification: { mode: string; reset: boolean } +): string { + if (notification.reset) { + return current === BYPASS_PERMISSIONS_MODE ? notification.mode : current; + } + return notification.mode; +} + +// A distinct glyph per mode so the active selection reads at a glance in the +// compact footer (no room for a text label there), addressing #377. Bypass +// keeps the red warning glyph as its persistent, non-color-only indicator. +function iconFor(mode: string): JSX.Element { + switch (mode) { + case BYPASS_PERMISSIONS_MODE: + return