Desktop: a failed knowledge-selection read is not an empty answer (/ palette, workflow capture)

ui/desktop/src/components/MentionPopover.privateChat.test.tsx

@@ -0,0 +1,186 @@
+import { render, screen } from '@testing-library/react';
+import { afterEach, beforeEach, describe, expect, it, vi, type MockInstance } from 'vitest';
+import MentionPopover from './MentionPopover';
+import { reachGatedGetActive, USER_ACTION_KEY } from '../test/reachGate';
+
+const mocks = vi.hoisted(() => ({
+  getActive: vi.fn(),
+  listBases: vi.fn(),
+  getSlashCommands: vi.fn(),
+  getSessionExtensions: vi.fn(),
+  // ONE array, as the real context's state is: a fresh `[]` per render
+  // recreates `loadReferenceItems` every render, and the palette reloads itself
+  // in a loop that detaches every row it has just drawn.
+  extensionsList: [] as never[],
+}));
+
+vi.mock('../api', () => ({
+  getActive: mocks.getActive,
+  listBases: mocks.listBases,
+  getSlashCommands: mocks.getSlashCommands,
+  getSessionExtensions: mocks.getSessionExtensions,
+}));
+
+vi.mock('./ConfigContext', () => ({
+  useConfig: () => ({ extensionsList: mocks.extensionsList }),
+}));
+
+vi.mock('./skills/useSkillCatalog', () => ({
+  fetchSkillCatalog: vi.fn(async () => ({ generation: 0, roots: [], skills: [], bundles: [] })),
+  pickerBundles: () => [],
+  standaloneSkills: () => [],
+}));
+
+const PRIVATE_CHAT = 'chat-private';
+
+function base(id: string, name: string) {
+  return { id, name, color: '#cf6d47', created_at: '', schema_version: 1, tier: 'public' };
+}
+
+function renderPalette() {
+  return render(
+    <MentionPopover
+      isOpen
+      isSlashCommand
+      query="kb"
+      sessionId={PRIVATE_CHAT}
+      workingDir="/w"
+      position={{ x: 0, y: 400 }}
+      selectedIndex={0}
+      onSelectedIndexChange={() => {}}
+      onSelect={() => {}}
+      onClose={() => {}}
+    />
+  );
+}
+
+/**
+ * Issue #56 Task 58: the `/` palette's knowledge-base rows come from the chat's
+ * selection, and `GET /knowledge/active` naming a PRIVATE chat is on the
+ * daemon's reach gate. Measured in the running desktop app on 2026-09-11: the
+ * read went out with no proof, was refused, and the palette fell back to "no
+ * base is hidden, none is primary" — offering a base the chat had hidden as
+ * "Knowledge base in this chat", and naming no primary at all.
+ */
+describe('the / palette in a private chat', () => {
+  let savedElectron: unknown;
+  let savedScrollIntoView: PropertyDescriptor | undefined;
+
+  beforeEach(() => {
+    vi.clearAllMocks();
+    savedElectron = (window as { electron?: unknown }).electron;
+    Object.assign(window, {
+      electron: { getUserActionKey: vi.fn(async () => USER_ACTION_KEY) },
+    });
+    // jsdom has no `scrollIntoView`, and the palette scrolls its selected row
+    // into view on every render — an effect that throws unmounts the palette.
+    savedScrollIntoView = Object.getOwnPropertyDescriptor(Element.prototype, 'scrollIntoView');
+    Object.defineProperty(Element.prototype, 'scrollIntoView', {
+      configurable: true,
+      writable: true,
+      value: vi.fn(),
+    });
+    mocks.getSlashCommands.mockResolvedValue({ data: { commands: [] } });
+    mocks.getSessionExtensions.mockResolvedValue({ data: { extensions: [] } });
+    mocks.listBases.mockResolvedValue({
+      data: [
+        base('soul', 'Soul'),
+        base('lab-notes', 'Lab notes'),
+        base('grant-drafts', 'Grant drafts'),
+      ],
+    });
+    mocks.getActive.mockImplementation(
+      reachGatedGetActive([PRIVATE_CHAT], (sessionId) =>
+        sessionId
+          ? {
+              kb_ids: ['lab-notes', 'soul'],
+              primary_kb: 'lab-notes',
+              active_kb: 'lab-notes',
+              hidden_kbs: ['grant-drafts'],
+            }
+          : { kb_ids: ['grant-drafts', 'lab-notes', 'soul'], primary_kb: 'soul', hidden_kbs: [] }
+      )
+    );
+  });
+
+  afterEach(() => {
+    Object.assign(window, { electron: savedElectron });
+    if (savedScrollIntoView) {
+      Object.defineProperty(Element.prototype, 'scrollIntoView', savedScrollIntoView);
+    } else {
+      delete (Element.prototype as { scrollIntoView?: unknown }).scrollIntoView;
+    }
+  });
+
+  it("offers this chat's knowledge bases, not every base, and names its primary", async () => {
+    renderPalette();
+
+    expect(await screen.findByText('kb:Lab notes')).toBeInTheDocument();
+    expect(screen.getByText('Primary knowledge base · lab-notes')).toBeInTheDocument();
+    expect(screen.getByText('kb:Soul')).toBeInTheDocument();
+    expect(screen.queryByText('kb:Grant drafts')).not.toBeInTheDocument();
+    expect(mocks.getActive).toHaveBeenCalledWith(
+      expect.objectContaining({
+        query: { session_id: PRIVATE_CHAT },
+        headers: { 'X-User-Action': USER_ACTION_KEY },
+      })
+    );
+  });
+
+  /**
+   * With the proof attached, a read that still fails is a genuine error: a
+   * surface that cannot prove the person, a dropped connection, an older
+   * daemon. None of those said "no base is hidden, none is primary", and the
+   * palette used to render exactly that — every base as "Knowledge base in
+   * this chat", Grant drafts included, and no primary.
+   *
+   * It keeps offering every base, because a reference names its base by id and
+   * an explicit id reaches a base whatever the chat's selection
+   * (`kb_id_or_primary` in the knowledge server). What it drops is the claim.
+   */
+  describe('when its selection cannot be read', () => {
+    let warn: MockInstance;
+
+    beforeEach(() => {
+      // The failure is reported, not swallowed; this keeps it out of the run's
+      // output and lets the test say so.
+      warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
+    });
+
+    afterEach(() => {
+      warn.mockRestore();
+    });
+
+    it.each<[string, () => void]>([
+      [
+        'the daemon refuses it',
+        // A preload with no bridge: `userActionHeaders()` sends no proof, and
+        // the gate answers the way it answers any caller that has none.
+        () => Object.assign(window, { electron: {} }),
+      ],
+      [
+        'the request fails in transit',
+        () => mocks.getActive.mockResolvedValue({ error: new TypeError('Failed to fetch') }),
+      ],
+      [
+        'the request throws',
+        // Before, this took the whole palette with it: `Promise.all` rejected
+        // and not one row, command or skill, was left to pick.
+        () => mocks.getActive.mockRejectedValue(new SyntaxError('Unexpected end of JSON input')),
+      ],
+    ])('offers every base and claims none of them for the chat when %s', async (_, fail) => {
+      fail();
+      renderPalette();
+
+      expect(await screen.findByText('kb:Grant drafts')).toBeInTheDocument();
+      expect(screen.getByText('kb:Lab notes')).toBeInTheDocument();
+      expect(screen.getByText('kb:Soul')).toBeInTheDocument();
+      for (const id of ['grant-drafts', 'lab-notes', 'soul']) {
+        expect(screen.getByText(`Knowledge base · ${id}`)).toBeInTheDocument();
+      }
+      expect(screen.queryAllByText(/Primary knowledge base/)).toHaveLength(0);
+      expect(screen.queryAllByText(/in this chat/)).toHaveLength(0);
+      expect(warn).toHaveBeenCalledWith('Knowledge selection not read:', expect.any(String));
+    });
+  });
+});

ui/desktop/src/components/MentionPopover.tsx

@@ -10,8 +10,9 @@ import {
 import { createPortal } from 'react-dom';
 import { ItemIcon } from './ItemIcon';
 import BuiltInBadge from './ui/BuiltInBadge';
-import { CommandType, getActive, getSessionExtensions, getSlashCommands, listBases } from '../api';
+import { CommandType, getSessionExtensions, getSlashCommands, listBases } from '../api';
 import type { CatalogView } from '../api';
+import { readKnowledgeSelection, type KnowledgeSelection } from './knowledge/knowledgeSelection';
 import { getInitialWorkingDir } from '../utils/workingDir';
 import { IMAGE_EXTENSIONS } from '../utils/imageFormats';
 import { labelledRefTag, refTag, type RefKind } from '../utils/resourceRefs';
@@ -99,6 +100,20 @@ const REFERENCE_KIND: Partial<Record<DisplayItemType, RefKind>> = {
   Extension: 'extension',
 };
 
+/**
+ * What a knowledge-base row may say about the chat, given what the daemon said.
+ *
+ * With no selection the read failed, and a failure answers neither question the
+ * other two labels claim to — is this base in the chat, is it the primary — so
+ * the row names only what it is. It is still offered: a reference names its
+ * base by id, and an explicit id reaches a base whatever the chat's selection
+ * (`kb_id_or_primary` in the knowledge server).
+ */
+const knowledgeBaseRole = (selection: KnowledgeSelection | null, kbId: string) => {
+  if (!selection) return 'Knowledge base';
+  return selection.primaryKbId === kbId ? 'Primary knowledge base' : 'Knowledge base in this chat';
+};
+
 /**
  * The resource `item` names, or `null` if it is not a resource reference.
  *
@@ -543,16 +558,16 @@ const MentionPopover = forwardRef<
 
     const loadReferenceItems = useCallback(
       async (includeCommands: boolean) => {
-        const [commandsResponse, basesResponse, activeResponse, skillsResult, sessionExtensions] =
+        const [commandsResponse, basesResponse, selection, skillsResult, sessionExtensions] =
           await Promise.all([
             includeCommands
               ? getSlashCommands({ throwOnError: true })
               : Promise.resolve({ data: { commands: [] } }),
             listBases({ throwOnError: false }),
-            getActive({
-              query: sessionId ? { session_id: sessionId } : undefined,
-              throwOnError: false,
-            }),
+            // Issue #56 Task 58: sent with the user's proof, which a GET naming
+            // a PRIVATE chat needs. `null` when the read failed anyway, and
+            // `knowledgeBaseRole` then claims nothing about the chat.
+            readKnowledgeSelection(sessionId),
             // The daemon's catalog, not a renderer scan: a skill bundled inside
             // an installed extension was loadable by the model and absent from
             // this list, so `@skill:word` completed to nothing (#113).
@@ -588,14 +603,11 @@ const MentionPopover = forwardRef<
           }
         }
 
-        const hiddenKbIds = new Set(activeResponse.data?.hidden_kbs ?? []);
-        const primaryKbId =
-          activeResponse.data?.primary_kb ?? activeResponse.data?.active_kb ?? null;
         for (const base of basesResponse.data ?? []) {
-          if (hiddenKbIds.has(base.id)) continue;
+          if (selection?.hiddenKbIds.has(base.id)) continue;
           commandItems.push({
             name: `kb:${base.name}`,
-            extra: `${primaryKbId === base.id ? 'Primary knowledge base' : 'Knowledge base in this chat'} · ${base.id}`,
+            extra: `${knowledgeBaseRole(selection, base.id)} · ${base.id}`,
             itemType: 'KnowledgeBase',
             relativePath: base.id,
           });

ui/desktop/src/components/knowledge/KnowledgeContext.test.tsx

@@ -1,7 +1,8 @@
 import { act, render, screen, waitFor } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
-import { beforeEach, describe, expect, it, vi } from 'vitest';
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 import { KnowledgeProvider, useKnowledge } from './KnowledgeContext';
+import { reachGatedGetActive, USER_ACTION_KEY } from '../../test/reachGate';
 
 /** A promise the test resolves by hand, so "after the response settled" is a fact, not a race. */
 function deferred<T>() {
@@ -583,4 +584,91 @@ describe('KnowledgeContext', () => {
       expect(mocks.setActive).not.toHaveBeenCalled();
     });
   });
+
+  // Issue #56 Task 58. `GET /knowledge/active` naming a PRIVATE chat is on the
+  // reach gate's list, and the desktop app gets through it the way `setActive`
+  // already does: with the user's proof. The reads carried none, so the daemon
+  // refused every private chat's selection — whatever model was bound — and the
+  // chip fell back to whatever this renderer had cached, which the daemon may
+  // have moved past (the agent's `kb_set_active`, the CLI, another window).
+  describe('a private chat', () => {
+    let savedElectron: unknown;
+
+    beforeEach(() => {
+      savedElectron = (window as { electron?: unknown }).electron;
+      Object.assign(window, {
+        electron: { getUserActionKey: vi.fn(async () => USER_ACTION_KEY) },
+      });
+      mocks.getActive.mockImplementation(
+        reachGatedGetActive(['chat-1'], (sessionId) =>
+          sessionId ? daemon.session : daemon.machine
+        )
+      );
+    });
+
+    afterEach(() => {
+      Object.assign(window, { electron: savedElectron });
+    });
+
+    it('hydrates its selection from the daemon, not from what this renderer cached', async () => {
+      localStorage.setItem('knowledge_active_kb:chat-1', 'beta');
+      localStorage.setItem('knowledge_hidden_kbs:chat-1', '[]');
+
+      renderProvider();
+
+      await waitFor(() => expect(screen.getByTestId('primary').textContent).toBe('alpha'));
+      expect(screen.getByTestId('hidden').textContent).toBe('beta');
+      expect(mocks.getActive).toHaveBeenCalledWith(
+        expect.objectContaining({
+          query: { session_id: 'chat-1' },
+          headers: { 'X-User-Action': USER_ACTION_KEY },
+        })
+      );
+    });
+
+    // Seeded to match the daemon, so the first read cannot be what puts the
+    // selection back: only the recovery read can.
+    it('re-reads its selection with the proof when a write does not land', async () => {
+      localStorage.setItem('knowledge_active_kb:chat-1', 'alpha');
+      localStorage.setItem('knowledge_hidden_kbs:chat-1', '["beta"]');
+      const pending = deferred<unknown>();
+      renderProvider();
+      await waitFor(() => expect(mocks.getActive).toHaveBeenCalledTimes(1));
+      await settle(() => {});
+
+      mocks.setActive.mockReturnValue(pending.promise);
+      await userEvent.click(screen.getByRole('button', { name: 'make beta primary' }));
+      expect(screen.getByTestId('primary').textContent).toBe('beta');
+
+      await settle(() => pending.reject(new Error('network down')));
+
+      await waitFor(() => expect(mocks.getActive).toHaveBeenCalledTimes(2));
+      expect(mocks.getActive.mock.calls[1]?.[0]).toMatchObject({
+        query: { session_id: 'chat-1' },
+        headers: { 'X-User-Action': USER_ACTION_KEY },
+      });
+      await waitFor(() => expect(screen.getByTestId('primary').textContent).toBe('alpha'));
+      expect(screen.getByTestId('hidden').textContent).toBe('beta');
+    });
+
+    // What the refused read cost, measured in the running app on 2026-09-11:
+    // the chip showed a hidden base as switched on, and one click on another
+    // base wrote that stale set back — the hidden base was in the chat again.
+    // A set-only edit is only as right as the set it starts from.
+    it('toggles from the set the daemon holds, so a toggle cannot re-expose a hidden base', async () => {
+      localStorage.setItem('knowledge_hidden_kbs:chat-1', '[]');
+      renderProvider();
+      await waitFor(() => expect(mocks.getActive).toHaveBeenCalled());
+      // Let the hydrate's answer land, whichever way the daemon answered it.
+      await act(async () => {
+        await Promise.allSettled(mocks.getActive.mock.results.map((result) => result.value));
+      });
+
+      await userEvent.click(screen.getByRole('button', { name: 'toggle alpha' }));
+
+      await waitFor(() => expect(mocks.setActive).toHaveBeenCalled());
+      const calls = mocks.setActive.mock.calls;
+      expect(calls[calls.length - 1]?.[0]?.body.hidden_kbs).toEqual(['alpha', 'beta']);
+    });
+  });
 });