From aa2147a947ff13b81e1fc901bf6ccc6490567e68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felipe=20St=C3=A9ffano?= Date: Mon, 21 Sep 2026 18:14:11 -0300 Subject: [PATCH 1/2] feat(tour): add NBI_TOUR_DISABLED to stop the first-run tour from opening The tour could only be silenced by writing an override file that disabled every step id, which breaks whenever a step is added. Add a real switch: the NBI_TOUR_DISABLED env var (or tour_disabled traitlet, env wins) is published in the capabilities response and the sidebar skips its first-run auto-show. The "Show NBI tour" command still replays the tour on demand. Co-Authored-By: Claude Sonnet 5 --- docs/admin-guide.md | 2 ++ docs/admin-tour-config.md | 8 +++++++ notebook_intelligence/extension.py | 18 +++++++++++++++ src/api.ts | 7 ++++++ src/chat-sidebar.tsx | 4 ++-- src/tour/tour-state.ts | 9 ++++++++ tests/test_tour_disabled.py | 36 ++++++++++++++++++++++++++++++ tests/ts/tour-state.test.ts | 24 +++++++++++++++++++- 8 files changed, 105 insertions(+), 3 deletions(-) create mode 100644 tests/test_tour_disabled.py 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..1294a22b 100644 --- a/docs/admin-tour-config.md +++ b/docs/admin-tour-config.md @@ -6,6 +6,14 @@ 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. + ## 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..191ba395 100644 --- a/notebook_intelligence/extension.py +++ b/notebook_intelligence/extension.py @@ -769,6 +769,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 +969,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 +4534,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 +4749,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_bool_with_env( + "NBI_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..1cf8426d --- /dev/null +++ b/tests/test_tour_disabled.py @@ -0,0 +1,36 @@ +"""The NBI_TOUR_DISABLED switch: env var wins over the traitlet.""" + +import pytest + +from notebook_intelligence import extension as ext_module + + +@pytest.fixture(autouse=True) +def _clear_env(monkeypatch): + monkeypatch.delenv("NBI_TOUR_DISABLED", raising=False) + + +def test_default_is_enabled(): + assert ext_module.NotebookIntelligence().tour_disabled is False + assert ext_module.GetCapabilitiesHandler.tour_disabled is False + + +def test_traitlet_disables(): + assert ext_module._resolve_bool_with_env("NBI_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_bool_with_env("NBI_TOUR_DISABLED", False) is True + + +def test_env_var_overrides_traitlet(monkeypatch): + monkeypatch.setenv("NBI_TOUR_DISABLED", "0") + assert ext_module._resolve_bool_with_env("NBI_TOUR_DISABLED", True) is False + + +def test_unrecognized_env_value_fails_loudly(monkeypatch): + monkeypatch.setenv("NBI_TOUR_DISABLED", "maybe") + with pytest.raises(ValueError): + ext_module._resolve_bool_with_env("NBI_TOUR_DISABLED", False) 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); + }); + }); }); From e5d1d203879955d468733ff2e7601ffa7172d696 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felipe=20St=C3=A9ffano?= Date: Mon, 21 Sep 2026 18:35:44 -0300 Subject: [PATCH 2/2] fix(tour): tolerate a mistyped NBI_TOUR_DISABLED and test the wiring A cosmetic switch should not be able to stop the extension from loading, so an unrecognized NBI_TOUR_DISABLED value now warns and falls back to the tour_disabled traitlet instead of raising. The strict resolver stays in place for the security gates. Add tests that _setup_handlers publishes the resolved value, that the capabilities response carries it, and that the lenient fallback warns, and document that the switch does not mark the tour completed. Co-Authored-By: Claude Sonnet 5 --- docs/admin-tour-config.md | 6 ++ notebook_intelligence/extension.py | 18 ++++- tests/test_tour_disabled.py | 101 ++++++++++++++++++++++++++--- 3 files changed, 114 insertions(+), 11 deletions(-) diff --git a/docs/admin-tour-config.md b/docs/admin-tour-config.md index 1294a22b..28e2de5d 100644 --- a/docs/admin-tour-config.md +++ b/docs/admin-tour-config.md @@ -14,6 +14,12 @@ 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 191ba395..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. @@ -4749,8 +4763,8 @@ 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_bool_with_env( - "NBI_TOUR_DISABLED", self.tour_disabled + 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/tests/test_tour_disabled.py b/tests/test_tour_disabled.py index 1cf8426d..b6f67f55 100644 --- a/tests/test_tour_disabled.py +++ b/tests/test_tour_disabled.py @@ -1,4 +1,8 @@ -"""The NBI_TOUR_DISABLED switch: env var wins over the traitlet.""" +"""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 @@ -6,31 +10,110 @@ @pytest.fixture(autouse=True) -def _clear_env(monkeypatch): +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.GetCapabilitiesHandler.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_bool_with_env("NBI_TOUR_DISABLED", True) is True + 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_bool_with_env("NBI_TOUR_DISABLED", False) is True + 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_bool_with_env("NBI_TOUR_DISABLED", True) is False + assert ext_module._resolve_tour_disabled(True) is False -def test_unrecognized_env_value_fails_loudly(monkeypatch): +@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 pytest.raises(ValueError): - ext_module._resolve_bool_with_env("NBI_TOUR_DISABLED", False) + 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