From 8954de1c4617f6ef9650ae283131f6fac5b6679b Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 10:46:20 -0700 Subject: [PATCH 1/3] fix(desktop): prove the person on every read of a chat's knowledge selection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `GET /knowledge/active` naming a chat is on the reach gate's list (`session_reach.rs`), exactly like the POST beside it. The POST in `syncSelection` has always sent `userActionHeaders()`; the reads did not — the KnowledgeProvider hydrate and its recovery re-read, the `/` palette's knowledge-base rows, and the create-workflow modal. So the desktop's own daemon answered each of them as a public caller and refused every private chat, whatever model was bound. Measured on 2026-09-11 in a sandboxed dev instance on versa_azure (GPT-5.5): one turn ratcheted a new chat private, and its selection was set to primary `lab-notes` with `grant-drafts` hidden. - `GET /knowledge/active?session_id=…` without the proof: 403, "That chat is private, or there is no chat with that id. This request was made on a public model …" — for a chat bound to a private model. With `X-User-Action`: 200 and the chat's selection. - The console printed "Knowledge selection not hydrated: That chat is private, or there is no chat with that id." and the chip showed all three bases on: the renderer's cache, not the daemon's selection. - One click on another base in that stale chip wrote the stale set back; the daemon then had `grant-drafts` visible to the chat again. - The `/` palette offered `kb:Grant drafts` as "Knowledge base in this chat" and named no primary. With the proof on the reads, the same chat hydrates to the daemon's selection, the chip shows Grant drafts off, the same click leaves it hidden, and the palette offers Lab notes (as the primary) and Soul only. This reaches nothing the renderer could not already read: the selection is a strict subset of the transcript `getSession` reads with the same proof. And `userActionHeaders()` is the one place the surface is decided, so on a browser surface these reads state the host's model once SD-9's change to that helper lands (#229), with no further change here. The KnowledgeContext comment that called the refusal "a correct outcome" for "a private chat opened while a public model is bound" is corrected: the read was refused for every private chat, and what followed was not correct. Fail-before: all five new tests — the hydrate, the recovery re-read, the toggle that re-exposed a hidden base, the palette and the workflow capture — fail with the production change reverted. `src/test/reachGate.ts` is the one model of the gate they share. --- .../MentionPopover.privateChat.test.tsx | 129 ++++++++++++++++++ ui/desktop/src/components/MentionPopover.tsx | 16 ++- .../knowledge/KnowledgeContext.test.tsx | 90 +++++++++++- .../components/knowledge/KnowledgeContext.tsx | 29 +++- .../CreateWorkflowFromSessionModal.tsx | 8 +- .../CreateWorkflowFromSessionModal.test.tsx | 94 ++++++++++++- ui/desktop/src/test/reachGate.ts | 55 ++++++++ 7 files changed, 407 insertions(+), 14 deletions(-) create mode 100644 ui/desktop/src/components/MentionPopover.privateChat.test.tsx create mode 100644 ui/desktop/src/test/reachGate.ts diff --git a/ui/desktop/src/components/MentionPopover.privateChat.test.tsx b/ui/desktop/src/components/MentionPopover.privateChat.test.tsx new file mode 100644 index 000000000..a079dec8e --- /dev/null +++ b/ui/desktop/src/components/MentionPopover.privateChat.test.tsx @@ -0,0 +1,129 @@ +import { render, screen } from '@testing-library/react'; +import { afterEach, beforeEach, describe, expect, it, vi } 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( + {}} + 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 }, + }) + ); + }); +}); diff --git a/ui/desktop/src/components/MentionPopover.tsx b/ui/desktop/src/components/MentionPopover.tsx index b49b56b7f..21ecb6ffe 100644 --- a/ui/desktop/src/components/MentionPopover.tsx +++ b/ui/desktop/src/components/MentionPopover.tsx @@ -12,6 +12,7 @@ import { ItemIcon } from './ItemIcon'; import BuiltInBadge from './ui/BuiltInBadge'; import { CommandType, getActive, getSessionExtensions, getSlashCommands, listBases } from '../api'; import type { CatalogView } from '../api'; +import { userActionHeaders } from '../utils/userAction'; import { getInitialWorkingDir } from '../utils/workingDir'; import { IMAGE_EXTENSIONS } from '../utils/imageFormats'; import { labelledRefTag, refTag, type RefKind } from '../utils/resourceRefs'; @@ -549,10 +550,17 @@ const MentionPopover = forwardRef< ? getSlashCommands({ throwOnError: true }) : Promise.resolve({ data: { commands: [] } }), listBases({ throwOnError: false }), - getActive({ - query: sessionId ? { session_id: sessionId } : undefined, - throwOnError: false, - }), + // Issue #56 Task 58: a GET naming a PRIVATE chat needs the user's + // proof, as `setActive` sends it. Refused, the rows below fell back + // to "nothing hidden, nothing primary" and offered every base as + // one this chat has. + userActionHeaders().then((headers) => + getActive({ + query: sessionId ? { session_id: sessionId } : undefined, + headers, + throwOnError: false, + }) + ), // 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). diff --git a/ui/desktop/src/components/knowledge/KnowledgeContext.test.tsx b/ui/desktop/src/components/knowledge/KnowledgeContext.test.tsx index 7221f8e20..760d9fadd 100644 --- a/ui/desktop/src/components/knowledge/KnowledgeContext.test.tsx +++ b/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() { @@ -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(); + 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']); + }); + }); }); diff --git a/ui/desktop/src/components/knowledge/KnowledgeContext.tsx b/ui/desktop/src/components/knowledge/KnowledgeContext.tsx index 34b513075..03333859f 100644 --- a/ui/desktop/src/components/knowledge/KnowledgeContext.tsx +++ b/ui/desktop/src/components/knowledge/KnowledgeContext.tsx @@ -186,6 +186,8 @@ export function KnowledgeProvider({ try { const res = await getActive({ query: sessionId ? { session_id: sessionId } : undefined, + // The same proof the hydrate below sends, for the same reason. + headers: await userActionHeaders(), throwOnError: true, }); if (generation !== selectionGenerationRef.current) return; @@ -455,6 +457,12 @@ export function KnowledgeProvider({ try { const res = await getActive({ query: sessionId ? { session_id: sessionId } : undefined, + // Issue #56 Task 58: a GET naming a PRIVATE chat is on the reach + // gate's list exactly as the POST in `syncSelection` is, and the + // desktop gets through it the same way — by proving the person. This + // selection is a strict subset of what `getSession` already reads + // with that proof. + headers: await userActionHeaders(), throwOnError: true, }); if (cancelled || generation !== selectionGenerationRef.current) return; @@ -462,12 +470,21 @@ export function KnowledgeProvider({ applyHidden(readHidden(res.data) ?? []); } catch (err) { if (cancelled) return; - // ⚠ One quiet line, deliberately. A private chat opened while a - // public model is bound refuses this read with a ~900-character - // paragraph addressed to an AI agent, and printing it here made a - // correct outcome — the chat opened and rendered in full — look like a - // crash. The person's answer is the composer's pinned-model note; see - // `selectionWarning.ts` for the full reasoning. + // ⚠ One quiet line, deliberately: the gate's refusal is ~900 + // characters addressed to an AI agent, and its first sentence is all a + // person reading the console needs (`selectionWarning.ts`). + // + // ⚠ Until 2026-09-11 this comment called that refusal "a correct + // outcome" for "a private chat opened while a public model is bound". + // Neither half held. This read carried no proof, so the daemon refused + // it for EVERY private chat, whatever model was bound — measured with a + // chat on its own private model — and what followed was not correct: + // the chip showed this renderer's cache instead of the daemon's + // selection, and the next toggle wrote that stale set back over it. A + // refusal still lands here for a caller that genuinely has neither the + // proof nor a private model (a daemon started without a user-action + // key, a browser tab not running a private model), and only there is + // it the right answer. console.warn('Knowledge selection not hydrated:', briefSelectionFailure(err)); setPrimaryKbIdState(local); setHiddenKbIdsState(localHidden); diff --git a/ui/desktop/src/components/workflows/CreateWorkflowFromSessionModal.tsx b/ui/desktop/src/components/workflows/CreateWorkflowFromSessionModal.tsx index b17c85653..b79582b70 100644 --- a/ui/desktop/src/components/workflows/CreateWorkflowFromSessionModal.tsx +++ b/ui/desktop/src/components/workflows/CreateWorkflowFromSessionModal.tsx @@ -6,6 +6,7 @@ import { Button } from '../ui/button'; import { WorkflowFormFields } from './shared/WorkflowFormFields'; import { WorkflowFormData } from './shared/workflowFormSchema'; import { createWorkflow, getActive, getSessionExtensions, listBases } from '../../api/sdk.gen'; +import { userActionHeaders } from '../../utils/userAction'; import { WorkflowParameter } from './shared/workflowFormSchema'; import { toastError } from '../../toasts'; import { saveWorkflow } from '../../workflow/workflow_management'; @@ -107,7 +108,12 @@ export default function CreateWorkflowFromSessionModal({ Promise.all([ listBases({ throwOnError: false }), - getActive({ query: { session_id: sessionId }, throwOnError: false }), + // Issue #56 Task 58: this modal opens from the chat it names, and a GET + // naming a PRIVATE chat needs the user's proof. Refused, the capture + // took every base, with whichever came first as its default. + userActionHeaders().then((headers) => + getActive({ query: { session_id: sessionId }, headers, throwOnError: false }) + ), ]).then(([basesRes, activeRes]) => { if (cancelled) return; const bases: Manifest[] = basesRes.data ?? []; diff --git a/ui/desktop/src/components/workflows/__tests__/CreateWorkflowFromSessionModal.test.tsx b/ui/desktop/src/components/workflows/__tests__/CreateWorkflowFromSessionModal.test.tsx index 2a4cfa209..aa23765f1 100644 --- a/ui/desktop/src/components/workflows/__tests__/CreateWorkflowFromSessionModal.test.tsx +++ b/ui/desktop/src/components/workflows/__tests__/CreateWorkflowFromSessionModal.test.tsx @@ -1,10 +1,11 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import CreateWorkflowFromSessionModal from '../CreateWorkflowFromSessionModal'; -import { createWorkflow, skillCatalogHandler } from '../../../api/sdk.gen'; +import { createWorkflow, getActive, listBases, skillCatalogHandler } from '../../../api/sdk.gen'; import type { CreateWorkflowResponse } from '../../../api/types.gen'; import { saveWorkflow } from '../../../workflow/workflow_management'; +import { reachGatedGetActive, USER_ACTION_KEY } from '../../../test/reachGate'; vi.mock('../../../api/sdk.gen', () => ({ createWorkflow: vi.fn(), @@ -70,6 +71,8 @@ vi.mock('../../../workflow/workflow_management', () => ({ })); const mockCreateWorkflow = vi.mocked(createWorkflow); +const mockGetActive = vi.mocked(getActive); +const mockListBases = vi.mocked(listBases); const mockSkillCatalog = vi.mocked(skillCatalogHandler); const mockSaveWorkflow = vi.mocked(saveWorkflow); @@ -483,4 +486,91 @@ describe('CreateWorkflowFromSessionModal', () => { expect(createButton).toBeDisabled(); }); }); + + /** + * Issue #56 Task 58: the modal reads the chat's knowledge-base selection with + * `GET /knowledge/active`, and naming a PRIVATE chat there is on the daemon's + * reach gate. The read carried no proof, was refused, and the modal fell back + * to "nothing is hidden, nothing is primary" — so a workflow captured from a + * private chat took EVERY base, with whichever came first as its default. + */ + describe('from a private chat', () => { + const PRIVATE_CHAT = 'private-session-id'; + const kb = (id: string) => ({ id, name: id, color: '#cf6d47', created_at: '' }); + + beforeEach(() => { + Object.assign(window, { + electron: { + listSkillDirs: vi.fn().mockResolvedValue([]), + getUserActionKey: vi.fn(async () => USER_ACTION_KEY), + }, + }); + mockListBases.mockResolvedValue({ + data: [kb('soul'), kb('lab-notes'), kb('grant-drafts')], + error: undefined, + } as never); + mockGetActive.mockImplementation( + reachGatedGetActive([PRIVATE_CHAT], () => ({ + kb_ids: ['lab-notes', 'soul'], + primary_kb: 'lab-notes', + active_kb: 'lab-notes', + hidden_kbs: ['grant-drafts'], + })) as never + ); + // A generation with no knowledge-base block of its own, so the chat's + // selection is the only place the saved workflow can take its bases from. + mockCreateWorkflow.mockResolvedValue({ + data: { + workflow: { + title: 'Analyzed Workflow Title', + description: 'Analyzed description', + instructions: 'Analyzed instructions', + }, + error: undefined, + }, + error: undefined, + request: new globalThis.Request('http://localhost/test'), + response: new globalThis.Response(), + }); + }); + + afterEach(() => { + // `clearAllMocks` keeps implementations, so put back what the rest of the + // file expects rather than leak this chat's gate into it. + mockListBases.mockResolvedValue({ data: [], error: undefined } as never); + mockGetActive.mockResolvedValue({ + data: { active_kb: null, hidden_kbs: [] }, + error: undefined, + } as never); + }); + + it("captures the chat's knowledge bases and its primary, not every base", async () => { + const user = userEvent.setup(); + render(); + + await waitFor( + () => { + expect(screen.getByTestId('create-workflow-button')).toBeEnabled(); + }, + { timeout: 2000 } + ); + await user.click(screen.getByTestId('create-workflow-button')); + await waitFor(() => { + expect(mockSaveWorkflow).toHaveBeenCalled(); + }); + + expect(mockSaveWorkflow).toHaveBeenCalledWith( + expect.objectContaining({ + knowledge_bases: { default: 'lab-notes', visible: ['soul', 'lab-notes'] }, + }), + null + ); + expect(mockGetActive).toHaveBeenCalledWith( + expect.objectContaining({ + query: { session_id: PRIVATE_CHAT }, + headers: { 'X-User-Action': USER_ACTION_KEY }, + }) + ); + }); + }); }); diff --git a/ui/desktop/src/test/reachGate.ts b/ui/desktop/src/test/reachGate.ts new file mode 100644 index 000000000..c4f7a74c4 --- /dev/null +++ b/ui/desktop/src/test/reachGate.ts @@ -0,0 +1,55 @@ +/** + * The daemon's session reach gate, as a renderer test meets it on + * `GET /knowledge/active` — one model of it, so the tests that need it cannot + * drift into three different ideas of what the daemon does. + * + * The real gate is `refuse_unless_reachable` in + * `crates/biorouter-server/src/routes/session_reach.rs`: a request naming a + * PRIVATE chat is answered only for a caller whose stated capability covers it + * or that carries the user's proof. The desktop app never states a capability + * — it proves the person, with the `X-User-Action` header `userActionHeaders()` + * builds from the preload bridge — so on the desktop the proof is the only way + * through, and that is all this models. A chat that is not private, and a + * request naming no chat at all, pass untouched: the gate is inert there. + * + * A refusal arrives the way the generated client delivers this route's + * `text/plain` 403 (`api/client/client.gen.ts`): THROWN as a bare string under + * `throwOnError: true`, and returned as `{ error }` with no `data` otherwise. + */ + +/** What the stubbed preload bridge hands `userActionHeaders()`. */ +export const USER_ACTION_KEY = 'renderer-test-user-action-key'; + +/** + * The first sentence of `SESSION_OUT_OF_REACH` in `session_reach.rs`. The rest + * of that paragraph is addressed to a model and never matters to a renderer + * assertion; this sentence is the one the console line keeps. + */ +export const SESSION_OUT_OF_REACH = 'That chat is private, or there is no chat with that id.'; + +type GetActiveOptions = { + query?: { session_id?: string }; + headers?: Record; + throwOnError?: boolean; +}; + +/** + * A `getActive` implementation that refuses a private chat to a request without + * the user's proof, and otherwise answers with `answer(sessionId)`. + */ +export function reachGatedGetActive( + privateSessionIds: readonly string[], + answer: (sessionId: string | undefined) => unknown +) { + return (options?: GetActiveOptions) => { + const sessionId = options?.query?.session_id; + const namesPrivateChat = sessionId !== undefined && privateSessionIds.includes(sessionId); + const proven = options?.headers?.['X-User-Action'] === USER_ACTION_KEY; + if (namesPrivateChat && !proven) { + return options?.throwOnError + ? Promise.reject(SESSION_OUT_OF_REACH) + : Promise.resolve({ data: undefined, error: SESSION_OUT_OF_REACH }); + } + return Promise.resolve({ data: answer(sessionId) }); + }; +} From 4befa3bc50c1527d8841f208447e5fef24935a2e Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 10:46:26 -0700 Subject: [PATCH 2/3] docs(desktop): the knowledge-selection refusal was never about the bound model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `selectionWarning.ts` and its test carried the premise the previous commit corrected in KnowledgeContext: that the desktop app was refused `GET /knowledge/active` for a private chat "while a public model is bound", as "a normal, correct outcome". It was refused for every private chat, because its read carried no proof. What the module does is still right — the refusal is prose addressed to a model, and a console's reader is a person — so only the reasoning changes. The pointer to the composer's pinned-model note goes with it: that note answers why a chosen model is not in effect, which this refusal never was about. --- .../knowledge/selectionWarning.test.ts | 13 ++++--- .../components/knowledge/selectionWarning.ts | 34 ++++++++++--------- 2 files changed, 26 insertions(+), 21 deletions(-) diff --git a/ui/desktop/src/components/knowledge/selectionWarning.test.ts b/ui/desktop/src/components/knowledge/selectionWarning.test.ts index 8c33a0422..ebf1b96d5 100644 --- a/ui/desktop/src/components/knowledge/selectionWarning.test.ts +++ b/ui/desktop/src/components/knowledge/selectionWarning.test.ts @@ -2,11 +2,14 @@ import { describe, expect, it } from 'vitest'; import { briefSelectionFailure } from './selectionWarning'; /** - * F14. Opening a private chat while a public model is bound refuses - * `GET /knowledge/active` with a paragraph addressed to an AI agent. The - * refusal is correct and stays exactly as it is; what was wrong is that it was - * printed in full in the devtools console of a desktop app, where the chat had - * opened and rendered correctly and the only reader is a person. + * F14. A request naming a private chat on `GET /knowledge/active` with neither + * the user's proof nor a private model is refused with a paragraph addressed to + * an AI agent. The refusal stays exactly as it is; what was wrong is that it was + * printed in full in a devtools console, whose only reader is a person. + * + * (F14 took the desktop app's refusal for a correct one, on a chat bound to a + * public model. It was not: the desktop's read carried no proof, and was refused + * for every private chat — see `KnowledgeContext.tsx`.) */ const AGENT_REFUSAL = 'That chat is private, or there is no chat with that id. This request was made on a public ' + diff --git a/ui/desktop/src/components/knowledge/selectionWarning.ts b/ui/desktop/src/components/knowledge/selectionWarning.ts index cfe619d52..f5a056c8a 100644 --- a/ui/desktop/src/components/knowledge/selectionWarning.ts +++ b/ui/desktop/src/components/knowledge/selectionWarning.ts @@ -10,23 +10,25 @@ const MAX_REASON_CHARS = 160; * A failed knowledge-selection read, reduced to one line for the console. * * ⚠ **The paragraph this trims is deliberate, correct, and written for somebody - * else.** Opening a private chat while a public model is bound refuses - * `GET /knowledge/active` with ~900 characters addressed to an AI AGENT — "Do - * not retry as you are… If this task genuinely needs that chat, stop and ask - * the user to open it for you." That text is privacy-critical, pinned by - * repo-grep tests in `crates/biorouter-server/src/routes/session_reach.rs`, and - * nothing here changes it or should. + * else.** A request naming a private chat on `GET /knowledge/active` with + * neither the user's proof nor a private model behind it is refused with ~900 + * characters addressed to an AI AGENT — "Do not retry as you are… If this task + * genuinely needs that chat, stop and ask the user to open it for you." That + * text is privacy-critical, pinned by repo-grep tests in + * `crates/biorouter-server/src/routes/session_reach.rs`, and nothing here + * changes it or should. In a console its only reader is a person, who has no + * task, is not an agent and cannot open the chat "for" anybody, so the console + * keeps a one-line record: the read did not settle, and which one. * - * What was wrong is where it landed: in the devtools console of a **desktop** - * app, where the chat had opened correctly and rendered in full, and where the - * only reader is a person who has no task, is not an agent, and cannot open the - * chat "for" anybody. A normal, correct outcome printed as a wall of red-flag - * prose reads as a crash. - * - * So the console keeps a one-line record — the read did not settle, and which - * one — and the user-facing answer lives where a person will actually see it: - * the composer's own note about the model this chat is pinned to - * (`privacy/PinnedModelNote`). + * ⚠ **Until 2026-09-11 this said the refusal was "a normal, correct outcome" + * for a private chat opened in the desktop app while a public model was bound. + * It was neither.** The desktop's reads carried no proof, so the daemon refused + * every private chat whatever model was bound (measured with a chat on its own + * private model), and the renderer went on to show — and write back — the + * selection it had cached. The reads carry `userActionHeaders()` now, as the + * write always did (`KnowledgeContext.tsx`). The refusal still reaches a + * console from a caller that genuinely has neither: a daemon started without a + * user-action key, or a browser tab not running a private model. * * Trimming to the first sentence rather than to a fixed prefix is what keeps * this useful for the failures that are NOT the refusal: "Failed to fetch" and From 15b8a901643e65fc076bfb3dcf2213747ab904d2 Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 10:51:08 -0700 Subject: [PATCH 3/3] docs(desktop): say what the proof already reaches, not that the selection is part of it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hydrate's comment called a chat's knowledge-base selection "a strict subset of what `getSession` already reads". It is not part of that response at all — the selection lives beside the session store, not in the transcript. The point the sentence was making is about reach: the same proof already reads the whole transcript, which discloses far more than which bases the chat uses. --- ui/desktop/src/components/knowledge/KnowledgeContext.tsx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/ui/desktop/src/components/knowledge/KnowledgeContext.tsx b/ui/desktop/src/components/knowledge/KnowledgeContext.tsx index 03333859f..3a615dcbb 100644 --- a/ui/desktop/src/components/knowledge/KnowledgeContext.tsx +++ b/ui/desktop/src/components/knowledge/KnowledgeContext.tsx @@ -459,9 +459,10 @@ export function KnowledgeProvider({ query: sessionId ? { session_id: sessionId } : undefined, // Issue #56 Task 58: a GET naming a PRIVATE chat is on the reach // gate's list exactly as the POST in `syncSelection` is, and the - // desktop gets through it the same way — by proving the person. This - // selection is a strict subset of what `getSession` already reads - // with that proof. + // desktop gets through it the same way — by proving the person. It + // reaches nothing new: the same proof already reads the chat's whole + // transcript through `getSession`, which says far more than which + // knowledge bases the chat uses. headers: await userActionHeaders(), throwOnError: true, });