diff --git a/docs/admin-guide.md b/docs/admin-guide.md index bfede4cb..a894f33d 100644 --- a/docs/admin-guide.md +++ b/docs/admin-guide.md @@ -113,6 +113,8 @@ The full surface, in one table. | `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. | +| `tour_disabled` | bool | `False` | traitlet | Suppress the first-run tour. The "Show NBI tour" command still replays it. See [`docs/admin-tour-config.md`](admin-tour-config.md). | +| `NBI_TOUR_DISABLED` | bool | 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. | diff --git a/docs/admin-tour-config.md b/docs/admin-tour-config.md index 7c846624..28e2de5d 100644 --- a/docs/admin-tour-config.md +++ b/docs/admin-tour-config.md @@ -6,6 +6,20 @@ Point the `NBI_TOUR_CONFIG_PATH` environment variable (or the `NotebookIntelligence.tour_config_path` traitlet) at a YAML or JSON file and Notebook Intelligence will overlay your copy on the built-in steps. +## Turning the tour off + +Set `NBI_TOUR_DISABLED=1` (or `NotebookIntelligence.tour_disabled = True`) +to stop the tour from opening on first run. The environment variable +takes precedence over the traitlet. Users can still replay the tour from +the command palette ("Show NBI tour"). Unlike disabling every step in an +override file, this switch does not need updating when steps are added. + +The switch only stops the tour from opening on its own; it does not mark +the tour as completed. If you remove it later, users who never finished +the tour see it again on their next load. An unrecognized value (for +example `NBI_TOUR_DISABLED=ture`) logs a warning and falls back to the +traitlet instead of stopping the extension from loading. + ## Where to put the file Anywhere readable by the Jupyter server process. A common pattern on diff --git a/notebook_intelligence/extension.py b/notebook_intelligence/extension.py index 5d718966..f1241133 100644 --- a/notebook_intelligence/extension.py +++ b/notebook_intelligence/extension.py @@ -453,6 +453,20 @@ def _resolve_bool_with_env(env_var_name: str, fallback: bool | None) -> bool: ) +def _resolve_tour_disabled(traitlet_value: bool | None) -> bool: + """Resolve NBI_TOUR_DISABLED, falling back to the traitlet. + + Unlike the security gates this is a cosmetic switch, so an unrecognized + value warns and falls back instead of raising: a typo must not stop the + whole extension from loading. + """ + try: + return _resolve_bool_with_env("NBI_TOUR_DISABLED", traitlet_value) + except ValueError as exc: + log.warning("%s; falling back to the tour_disabled setting", exc) + return bool(traitlet_value) + + def _resolve_positive_int_with_env(env_var_name: str, traitlet_value: int) -> int: """Resolve a non-negative int tunable, falling back to the traitlet. @@ -769,6 +783,9 @@ class GetCapabilitiesHandler(APIHandler): # Resolved at extension init from NBI_TOUR_CONFIG_PATH (or the # tour_config_path traitlet). Empty string disables the override. tour_config_path = "" + # Resolved at extension init from NBI_TOUR_DISABLED (or the + # tour_disabled traitlet). Suppresses only the first-run auto-show. + tour_disabled = False @tornado.web.authenticated def get(self): @@ -966,6 +983,7 @@ def is_provider_enabled(provider_id: str) -> bool: # unset, so the cost is at most one stat call per request. The # path itself is pre-resolved at initialize_handlers time. response["tour_overrides"] = load_tour_config(self.tour_config_path) + response["tour_disabled"] = self.tour_disabled self.finish(json.dumps(response)) @@ -4530,6 +4548,17 @@ class NotebookIntelligence(ExtensionApp): config=True, ) + tour_disabled = Bool( + default_value=False, + help=""" + Suppress the in-app first-run tour. The tour no longer opens on its + own; the "Show NBI tour" command still replays it on demand. + Overridden by the NBI_TOUR_DISABLED environment variable. + """, + allow_none=True, + config=True, + ) + def initialize_settings(self): warm_tokenizer_encoding() @@ -4734,6 +4763,9 @@ def _setup_handlers(self, web_app, feature_policies: dict, string_overrides: dic os.environ.get("NBI_TOUR_CONFIG_PATH", "").strip() or (self.tour_config_path or "").strip() ) + GetCapabilitiesHandler.tour_disabled = _resolve_tour_disabled( + self.tour_disabled + ) SkillsBaseHandler.allow_github_skill_import = _resolve_bool_with_env( "NBI_ALLOW_GITHUB_SKILL_IMPORT", self.allow_github_skill_import ) diff --git a/src/api.ts b/src/api.ts index 450d75c0..21189b52 100644 --- a/src/api.ts +++ b/src/api.ts @@ -536,6 +536,13 @@ export class NBIConfig { return v && typeof v === 'object' ? v : EMPTY_TOUR_OVERRIDES; } + // True when an admin has suppressed the first-run auto-show + // (NBI_TOUR_DISABLED). Replaying the tour from the command palette + // still works. + get tourDisabled(): boolean { + return this.capabilities.tour_disabled === true; + } + get allowGithubSkillImport(): boolean { return this.capabilities.allow_github_skill_import !== false; } diff --git a/src/chat-sidebar.tsx b/src/chat-sidebar.tsx index c0f46e79..2e4bc05f 100644 --- a/src/chat-sidebar.tsx +++ b/src/chat-sidebar.tsx @@ -97,7 +97,7 @@ import { upsertToolCallContent } from './tool-call-stream'; import { TourOverlay } from './tour/tour-overlay'; import { TOUR_ANCHOR } from './tour/tour-anchors'; import { TOUR_START_EVENT, TOUR_STOP_EVENT } from './tour/tour-events'; -import { hasCompletedTour } from './tour/tour-state'; +import { hasCompletedTour, shouldAutoShowTour } from './tour/tour-state'; import { cancelInFlightTurns, registerInFlightTurn } from './chat-cancel'; import { recordStoppedTurn } from './chat-stopped-turn'; import { upsertMessageById } from './chat-transcript'; @@ -1303,7 +1303,7 @@ function SidebarComponent(props: any) { const [tourVisible, setTourVisible] = useState(false); const sidebarRootRef = useRef(null); useEffect(() => { - if (hasCompletedTour()) { + if (!shouldAutoShowTour(NBIAPI.config.tourDisabled)) { return; } // The sidebar React tree mounts even when the Lumino panel is diff --git a/src/tour/tour-state.ts b/src/tour/tour-state.ts index bba457e8..22b2f086 100644 --- a/src/tour/tour-state.ts +++ b/src/tour/tour-state.ts @@ -85,3 +85,12 @@ export function resetTour(): void { // See markTourCompleted. } } + +/** + * Whether the sidebar should open the tour on its own. An admin switch + * (`NBI_TOUR_DISABLED`) suppresses only this auto-show; the replay command + * starts the tour directly and is unaffected. + */ +export function shouldAutoShowTour(disabledByAdmin: boolean): boolean { + return !disabledByAdmin && !hasCompletedTour(); +} diff --git a/tests/test_tour_disabled.py b/tests/test_tour_disabled.py new file mode 100644 index 00000000..b6f67f55 --- /dev/null +++ b/tests/test_tour_disabled.py @@ -0,0 +1,119 @@ +"""The NBI_TOUR_DISABLED switch: env var wins over the traitlet, and a typo +never stops the extension from loading.""" + +import logging +from unittest.mock import MagicMock + +import pytest + +from notebook_intelligence import extension as ext_module + + +@pytest.fixture(autouse=True) +def _isolate(monkeypatch): + monkeypatch.delenv("NBI_TOUR_DISABLED", raising=False) + # _setup_handlers publishes onto the class; restore it so no test depends + # on the order the others ran in. + monkeypatch.setattr( + ext_module.GetCapabilitiesHandler, "tour_disabled", False, raising=False + ) + + +def test_default_is_enabled(): + assert ext_module.NotebookIntelligence().tour_disabled is False + assert ext_module._resolve_tour_disabled(False) is False + assert ext_module._resolve_tour_disabled(None) is False + + +def test_traitlet_disables(): + assert ext_module._resolve_tour_disabled(True) is True + + +@pytest.mark.parametrize("value", ["1", "true", "yes", "on"]) +def test_env_var_disables(monkeypatch, value): + monkeypatch.setenv("NBI_TOUR_DISABLED", value) + assert ext_module._resolve_tour_disabled(False) is True + + +def test_env_var_overrides_traitlet(monkeypatch): + monkeypatch.setenv("NBI_TOUR_DISABLED", "0") + assert ext_module._resolve_tour_disabled(True) is False + + +@pytest.mark.parametrize("traitlet", [True, False]) +def test_unrecognized_env_value_warns_and_uses_traitlet( + monkeypatch, caplog, traitlet +): + monkeypatch.setenv("NBI_TOUR_DISABLED", "maybe") + with caplog.at_level(logging.WARNING, logger=ext_module.log.name): + assert ext_module._resolve_tour_disabled(traitlet) is traitlet + assert "NBI_TOUR_DISABLED" in caplog.text + assert "true, false, 1, 0, yes, no, on, off" in caplog.text + + +@pytest.fixture +def restore_handler_class_state(): + """_setup_handlers publishes settings onto many handler classes at once + (some are security gates other tests expect closed), so put every plain + class attribute it could have touched back afterwards.""" + saved = [] + for cls in vars(ext_module).values(): + if isinstance(cls, type) and cls.__module__ == ext_module.__name__: + saved.append((cls, dict(vars(cls)))) + yield + for cls, attrs in saved: + for name, value in attrs.items(): + if name.startswith("__") or callable(value): + continue + setattr(cls, name, value) + for name in set(vars(cls)) - set(attrs): + if not name.startswith("__"): + delattr(cls, name) + + +def test_setup_handlers_publishes_the_resolved_value( + monkeypatch, restore_handler_class_state +): + monkeypatch.setenv("NBI_TOUR_DISABLED", "1") + # _setup_handlers reads config through module globals that only exist once + # the extension has started, and treats every *_env / list setting as data. + monkeypatch.setattr(ext_module, "ai_service_manager", MagicMock()) + monkeypatch.setattr(ext_module, "nbi_config", MagicMock(), raising=False) + web_app = MagicMock() + web_app.settings = {"base_url": "/"} + ext = MagicMock() + ext.tour_disabled = False + ext.tour_config_path = "" + ext.skill_max_archive_mb = 100 + ext.upload_max_mb = 25 + ext.upload_retention_hours = 24 + for name in ( + "disabled_tools", + "disabled_providers", + "disabled_coding_agent_launchers", + "additional_skipped_workspace_directories", + ): + setattr(ext, name, []) + ext_module.NotebookIntelligence._setup_handlers(ext, web_app, {}, {}) + assert ext_module.GetCapabilitiesHandler.tour_disabled is True + + +@pytest.mark.parametrize("disabled", [True, False]) +def test_capabilities_response_carries_the_flag(monkeypatch, disabled): + monkeypatch.setattr(ext_module, "ai_service_manager", MagicMock()) + # The response is one big dict of mostly-mocked values; capture it before + # it is serialized rather than mocking every field to be JSON-safe. + monkeypatch.setattr(ext_module.json, "dumps", lambda obj, *a, **k: obj) + handler = MagicMock() + handler.tour_config_path = "" + handler.tour_disabled = disabled + for name in ( + "disabled_tools", + "disabled_providers", + "disabled_coding_agent_launchers", + "additional_skipped_workspace_directories", + ): + setattr(handler, name, []) + ext_module.GetCapabilitiesHandler.get(handler) + response = handler.finish.call_args[0][0] + assert response["tour_disabled"] is disabled diff --git a/tests/ts/tour-state.test.ts b/tests/ts/tour-state.test.ts index 1adfeff2..dfd2da94 100644 --- a/tests/ts/tour-state.test.ts +++ b/tests/ts/tour-state.test.ts @@ -5,7 +5,8 @@ import { TOUR_VERSION, hasCompletedTour, markTourCompleted, - resetTour + resetTour, + shouldAutoShowTour } from '../../src/tour/tour-state'; describe('tour-state', () => { @@ -52,4 +53,25 @@ describe('tour-state', () => { const parsed = JSON.parse(raw as string); expect(parsed.version).toBe(TOUR_VERSION); }); + + describe('shouldAutoShowTour', () => { + it('shows on first run', () => { + expect(shouldAutoShowTour(false)).toBe(true); + }); + + it('does not show once completed', () => { + markTourCompleted(); + expect(shouldAutoShowTour(false)).toBe(false); + }); + + it('does not show when an admin disabled it, even on first run', () => { + expect(shouldAutoShowTour(true)).toBe(false); + }); + + it('leaves replay unaffected: resetTour still clears completion', () => { + markTourCompleted(); + resetTour(); + expect(hasCompletedTour()).toBe(false); + }); + }); });