Fix notebook kernel widget crash when toggling preview tabs. (#12915)

Addresses #12914 and #12464

When a notebook replaces a preview tab, the kernel status badge widget
can render before `attachView()` completes. At that point
`scopedContextKeyService` is still undefined, causing `useMenu` to crash
when it tries to create a menu.

This PR:
- Makes `KernelStatusBadge` defer menu creation until `container` is set
(the readiness signal for `attachView()`)
- Makes `useMenu` handle `undefined` `contextKeyService` gracefully and
synchronously mask stale menus when the service identity changes between
renders
- Fixes `editorActionBarFactory` to use the correct private
`_contextKeyService` field

### Before

_Opening up a new notebook in the same preview tab as a previous one
caused a crash of the widget._


https://github.com/user-attachments/assets/9e662fbd-dd71-4726-a4a8-b1acb2fae57f


### After

_Widget no longer crashes when swapping editors in a preview tab_


https://github.com/user-attachments/assets/60793fb0-7175-47d9-b619-fab198f1bb83



### Release Notes

#### New Features
- N/A

#### Bug Fixes
- Fixed a crash in the notebook kernel selector when a notebook replaces
another notebook in a preview tab (#12914)

### QA Notes

@:editor-action-bar @:positron-notebooks @:quarto

1. Single-click a notebook in the explorer to open in a preview tab
2. Double-click or edit the notebook so it replaces the preview tab
3. Verify no crash occurs and the kernel selector badge appears
correctly

Author: nstrayer

src/vs/workbench/contrib/positronNotebook/browser/KernelStatusBadge.tsx

@@ -81,7 +81,15 @@ export function KernelStatusBadge() {
 		// This shouldn't happen...
 		return '';
 	}));
-	const menu = useMenu(MenuId.PositronNotebookKernelSubmenu, notebookInstance.scopedContextKeyService);
+	// scopedContextKeyService is only available after attachView() is called.
+	// When a notebook replaces a preview tab, the widget may render before
+	// attachView() completes. Use container (set last in attachView) as the
+	// readiness signal.
+	const container = useObservedValue(notebookInstance.container);
+	const menu = useMenu(
+		MenuId.PositronNotebookKernelSubmenu,
+		container ? notebookInstance.scopedContextKeyService : undefined
+	);
 
 	// Callback to load actions from the menu
 	const getActions = React.useCallback(() => {

src/vs/workbench/contrib/positronNotebook/browser/useMenu.tsx

@@ -43,14 +43,22 @@ export function useMenu(
 	const [menu, setMenu] = React.useState<IMenu | undefined>();
 	const [version, setVersion] = React.useState(0);
 
+	// Track which contextKeyService created the current menu so we can
+	// synchronously mask stale menus when the service identity changes
+	// (not just when it becomes undefined).
+	const menuOwnerRef = React.useRef<IContextKeyService | undefined>(undefined);
+
 	// Main effect
 	React.useEffect(() => {
 		if (!contextKeyService) {
+			setMenu(undefined);
+			menuOwnerRef.current = undefined;
 			return;
 		}
 
 		const menu = menuService.createMenu(menuId, contextKeyService, options);
 		setMenu(menu);
+		menuOwnerRef.current = contextKeyService;
 		setVersion(0);
 
 		const disposable = combinedDisposable(
@@ -60,5 +68,10 @@ export function useMenu(
 		return () => disposable.dispose();
 	}, [menuService, contextKeyService, menuId, options]);
 
-	return React.useMemo(() => ({ current: menu, version }), [menu, version]);
+	// Synchronously mask the menu whenever the contextKeyService doesn't
+	// match the service that created it. This covers both the undefined
+	// case and the case where the service changes identity (e.g. during
+	// notebook tab reuse: service A -> undefined -> service B).
+	const current = (contextKeyService && contextKeyService === menuOwnerRef.current) ? menu : undefined;
+	return React.useMemo(() => ({ current, version }), [current, version]);
 }

src/vs/workbench/contrib/positronNotebook/browser/useMenuActions.tsx

@@ -3,7 +3,7 @@
  *  Licensed under the Elastic License 2.0. See LICENSE.txt for license information.
  *--------------------------------------------------------------------------------------------*/
 import React from 'react';
-import { MenuItemAction, SubmenuItemAction, IMenuActionOptions } from '../../../../platform/actions/common/actions.js';
+import { IMenuActionOptions } from '../../../../platform/actions/common/actions.js';
 import { IVersionedMenu } from './useMenu.js';
 
 /**
@@ -17,16 +17,8 @@ export function useMenuActions(
 	menu: IVersionedMenu,
 	options?: IMenuActionOptions,
 ) {
-	// State
-	const [actions, setActions] = React.useState<[string, (MenuItemAction | SubmenuItemAction)[]][]>([]);
-
-	// Main effect
-	React.useEffect(() => {
-		if (!menu.current) {
-			return setActions([]);
-		}
-		setActions(menu.current.getActions(options));
-	}, [menu, options]);
-
-	return actions;
+	return React.useMemo(
+		() => menu.current ? menu.current.getActions(options) : [],
+		[menu, options],
+	);
 }

src/vs/workbench/contrib/positronNotebook/test/browser/notebookCells/CellOutputActionBar.test.tsx

@@ -115,13 +115,11 @@ suite('CellOutputActionBar', () => {
 			</PositronReactServicesContext.Provider>
 		);
 
-		// TODO: Render three times to settle chained useEffect state updates
-		// in useMenu and useMenuActions. Each hook sets state in an effect,
-		// requiring a separate render cycle to propagate.
-		// https://github.com/posit-dev/positron/issues/12464
+		// Render twice: once for the initial render, once for the useMenu
+		// effect to create the menu and trigger a re-render. useMenuActions
+		// now uses useMemo so it resolves in the same cycle as useMenu.
 		const container = render(element);
 		render(element);
-		render(element);
 		return new CellOutputActionBarFixture(container);
 	}
 

src/vs/workbench/contrib/positronNotebook/test/browser/useMenuActions.test.tsx

@@ -0,0 +1,259 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (C) 2026 Posit Software, PBC. All rights reserved.
+ *  Licensed under the Elastic License 2.0. See LICENSE.txt for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+/* eslint-disable local/code-no-dangerous-type-assertions */
+
+import assert from 'assert';
+import sinon from 'sinon';
+import { Emitter } from '../../../../../base/common/event.js';
+import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js';
+import { setupReactRenderer } from '../../../../../base/test/browser/react.js';
+import { IMenu, IMenuChangeEvent, IMenuService, MenuId, MenuItemAction, SubmenuItemAction } from '../../../../../platform/actions/common/actions.js';
+import { IVersionedMenu, useMenu } from '../../browser/useMenu.js';
+import { useMenuActions } from '../../browser/useMenuActions.js';
+import { MockContextKeyService } from '../../../../../platform/keybinding/test/common/mockKeybindingService.js';
+import { PositronReactServicesContext } from '../../../../../base/browser/positronReactRendererContext.js';
+import { PositronReactServices } from '../../../../../base/browser/positronReactServices.js';
+
+type Actions = [string, (MenuItemAction | SubmenuItemAction)[]][];
+
+/** Harness that renders useMenuActions with a directly controlled IVersionedMenu. */
+function UseMenuActionsHarness({ menu, onActions }: {
+	menu: IVersionedMenu;
+	onActions: (a: Actions) => void;
+}) {
+	const actions = useMenuActions(menu);
+	onActions(actions);
+	return null;
+}
+
+/** Harness that composes useMenu + useMenuActions the way real components do. */
+function ComposedHarness({ contextKeyService, onActions }: {
+	contextKeyService: MockContextKeyService | undefined;
+	onActions: (a: Actions) => void;
+}) {
+	const menu = useMenu(MenuId.CommandPalette, contextKeyService);
+	const actions = useMenuActions(menu);
+	onActions(actions);
+	return null;
+}
+
+suite('useMenuActions', () => {
+	const { render } = setupReactRenderer();
+	const disposables = ensureNoDisposablesAreLeakedInTestSuite();
+
+	const action = (id: string) => ({ id }) as MenuItemAction;
+
+	suite('standalone', () => {
+		test('returns empty array when menu.current is undefined', () => {
+			let captured: Actions = [];
+			const menu: IVersionedMenu = { current: undefined, version: 0 };
+
+			render(<UseMenuActionsHarness menu={menu} onActions={a => { captured = a; }} />);
+			assert.deepStrictEqual(captured, []);
+		});
+
+		test('returns actions when menu.current is present', () => {
+			let captured: Actions = [];
+			const expected: Actions = [['group', [action('a1'), action('a2')]]];
+			const menu: IVersionedMenu = {
+				current: { getActions: () => expected } as unknown as IMenu,
+				version: 0,
+			};
+
+			render(<UseMenuActionsHarness menu={menu} onActions={a => { captured = a; }} />);
+			assert.deepStrictEqual(captured, expected);
+		});
+
+		test('updates when version changes', () => {
+			let captured: Actions = [];
+			const actionsV0: Actions = [['g', [action('old')]]];
+			const actionsV1: Actions = [['g', [action('new')]]];
+			let currentActions = actionsV0;
+
+			const mockMenu = { getActions: () => currentActions } as unknown as IMenu;
+
+			render(<UseMenuActionsHarness
+				menu={{ current: mockMenu, version: 0 }}
+				onActions={a => { captured = a; }}
+			/>);
+			assert.deepStrictEqual(captured, actionsV0);
+
+			currentActions = actionsV1;
+			render(<UseMenuActionsHarness
+				menu={{ current: mockMenu, version: 1 }}
+				onActions={a => { captured = a; }}
+			/>);
+			assert.deepStrictEqual(captured, actionsV1);
+		});
+	});
+
+	suite('composed with useMenu', () => {
+		let contextKeyService: MockContextKeyService;
+		let menuActions: Actions;
+		let onDidChange: Emitter<IMenuChangeEvent>;
+
+		setup(() => {
+			contextKeyService = disposables.add(new MockContextKeyService());
+			menuActions = [];
+			onDidChange = disposables.add(new Emitter<IMenuChangeEvent>());
+		});
+
+		/** Tracks all menus created by the mock service. */
+		let createdMenus: { menu: IMenu; dispose: sinon.SinonSpy }[];
+
+		function createServices(): PositronReactServices {
+			createdMenus = [];
+			const menuService: IMenuService = {
+				_serviceBrand: undefined,
+				createMenu: () => {
+					const dispose = sinon.spy();
+					const menu: IMenu = {
+						onDidChange: onDidChange.event,
+						dispose,
+						getActions: () => menuActions,
+					};
+					createdMenus.push({ menu, dispose });
+					return menu;
+				},
+				getMenuActions: () => [],
+				getMenuContexts: () => new Set(),
+				resetHiddenStates: () => { },
+			};
+			return {
+				get: (id: any) => {
+					if (id === IMenuService) { return menuService; }
+					throw new Error(`Unexpected service: ${id}`);
+				},
+			} as unknown as PositronReactServices;
+		}
+
+		test('resolves actions in the same render as useMenu', () => {
+			menuActions = [['g', [action('a1')]]];
+			let captured: Actions = [];
+			const services = createServices();
+
+			const element = (
+				<PositronReactServicesContext.Provider value={services}>
+					<ComposedHarness
+						contextKeyService={contextKeyService}
+						onActions={a => { captured = a; }}
+					/>
+				</PositronReactServicesContext.Provider>
+			);
+
+			// Render 1: initial (effect queued, menu undefined)
+			render(element);
+			assert.deepStrictEqual(captured, [], 'no actions before effect settles');
+
+			// Render 2: effect creates menu; useMemo resolves actions in same cycle
+			render(element);
+			assert.deepStrictEqual(captured, [['g', [action('a1')]]]);
+		});
+
+		test('updates actions when menu fires onDidChange', () => {
+			menuActions = [['g', [action('v0')]]];
+			let captured: Actions = [];
+			const services = createServices();
+
+			const element = (
+				<PositronReactServicesContext.Provider value={services}>
+					<ComposedHarness
+						contextKeyService={contextKeyService}
+						onActions={a => { captured = a; }}
+					/>
+				</PositronReactServicesContext.Provider>
+			);
+
+			render(element);
+			render(element);
+			assert.deepStrictEqual(captured, [['g', [action('v0')]]]);
+
+			// Simulate menu content change
+			menuActions = [['g', [action('v1')]]];
+			onDidChange.fire({} as IMenuChangeEvent);
+
+			// One render to pick up the version bump
+			render(element);
+			assert.deepStrictEqual(captured, [['g', [action('v1')]]]);
+		});
+
+		test('clears actions when contextKeyService becomes undefined', () => {
+			menuActions = [['g', [action('a1')]]];
+			let captured: Actions = [];
+			const services = createServices();
+
+			const withService = (
+				<PositronReactServicesContext.Provider value={services}>
+					<ComposedHarness
+						contextKeyService={contextKeyService}
+						onActions={a => { captured = a; }}
+					/>
+				</PositronReactServicesContext.Provider>
+			);
+			render(withService);
+			render(withService);
+			assert.deepStrictEqual(captured, [['g', [action('a1')]]]);
+
+			// Remove contextKeyService -- actions must clear synchronously
+			render(
+				<PositronReactServicesContext.Provider value={services}>
+					<ComposedHarness
+						contextKeyService={undefined}
+						onActions={a => { captured = a; }}
+					/>
+				</PositronReactServicesContext.Provider>
+			);
+			assert.deepStrictEqual(captured, [], 'actions must clear when service disappears');
+		});
+
+		test('clears stale actions and disposes old menu on contextKeyService identity swap', () => {
+			menuActions = [['g', [action('a1')]]];
+			let captured: Actions = [];
+			const services = createServices();
+
+			const serviceA = contextKeyService;
+			const serviceB = disposables.add(new MockContextKeyService());
+
+			// Mount with service A and settle
+			const withA = (
+				<PositronReactServicesContext.Provider value={services}>
+					<ComposedHarness
+						contextKeyService={serviceA}
+						onActions={a => { captured = a; }}
+					/>
+				</PositronReactServicesContext.Provider>
+			);
+			render(withA);
+			render(withA);
+			assert.deepStrictEqual(captured, [['g', [action('a1')]]]);
+			assert.strictEqual(createdMenus.length, 1, 'one menu created for service A');
+
+			// Swap to service B -- stale actions must not leak on first render
+			render(
+				<PositronReactServicesContext.Provider value={services}>
+					<ComposedHarness
+						contextKeyService={serviceB}
+						onActions={a => { captured = a; }}
+					/>
+				</PositronReactServicesContext.Provider>
+			);
+			assert.deepStrictEqual(captured, [], 'stale actions from service A must not appear');
+
+			// Settle the effect for service B
+			render(
+				<PositronReactServicesContext.Provider value={services}>
+					<ComposedHarness
+						contextKeyService={serviceB}
+						onActions={a => { captured = a; }}
+					/>
+				</PositronReactServicesContext.Provider>
+			);
+			assert.deepStrictEqual(captured, [['g', [action('a1')]]], 'actions from service B menu');
+			assert.strictEqual(createdMenus.length, 2, 'a new menu was created for service B');
+			sinon.assert.calledOnce(createdMenus[0].dispose);
+		});
+	});
+});