diff --git a/CLAUDE.md b/CLAUDE.md index d00981305..b072afbfa 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -338,7 +338,7 @@ The Knowledge feature (built across Plans 1-6 in `docs/history/knowledge-base-bu - **HTTP routes:** `crates/biorouter-server/src/routes/knowledge.rs` covers `/knowledge/bases`, `/ingest` (SSE), `/graph`, `/history`, `/preview`, `/restore`, `/page`, `/active`, `/export`, `/import`. - **Frontend:** `ui/desktop/src/components/knowledge/` (view shell, KB selector, ingest panel, force-graph + change-log drawer). The chat-side KB chip lives at `ui/desktop/src/components/bottom_menu/BottomMenuKnowledgeSelection.tsx`. - **Storage layout:** `~/.config/biorouter/knowledge//` with `raw/`, `knowledge/`, `index.md`, `log.md`, `schema.md`, and a hidden `.git/`. -- **One axis, one pointer.** A session's knowledge bases are the *visible* set — everything not in `.hidden-kbs` (machine-wide) or `.hidden-kb-sessions/` (per session; an empty `[]` means "hide nothing", not "inherit"). KB-less search spans this set with per-hit `kb_id` attribution. Its **primary** is the write target, default single-base read target and Knowledge view's subject. Soul is the product default when the user has expressed no preference. A missing `.active-kb` / `.active-kb-sessions/` inherits; a bare id pins a choice; a blank file explicitly chooses no primary and must not fall back to Soul. KB-less writes with that explicit no-primary state fail with the candidate list. The primary must remain visible; the daemon repairs selection when its base is hidden or deleted. `kb_set_active` changes the primary without narrowing search. Set-only edits send neither `primary_kb` nor `clear_primary`; see [`docs/knowledge-base/multi-kb-implementation-plan.md`](docs/knowledge-base/multi-kb-implementation-plan.md). +- **One axis, one pointer.** A session's knowledge bases are the *visible* set — everything not in `.hidden-kbs` (machine-wide) or `.hidden-kb-sessions/` (per session; an empty `[]` means "hide nothing", not "inherit"). KB-less search spans this set with per-hit `kb_id` attribution. Its **primary** is the write target, default single-base read target and Knowledge view's subject. Soul is the product default when the user has expressed no preference. A missing `.active-kb` / `.active-kb-sessions/` inherits; a bare id pins a choice; a blank file explicitly chooses no primary and must not fall back to Soul. KB-less writes with that explicit no-primary state fail with the candidate list. The primary must remain visible; the daemon repairs selection when its base is hidden or deleted — and the two repairs differ (D2): hiding *promotes* to the first remaining base, deleting *clears* every pointer that named the base to that explicit blank, and a chat that only inherited the pointer is left inheriting. A blank `.active-kb` after a delete is the repair, not its absence. ⚠ The renderer adopts these repairs by re-reading; it never writes one (no `clear_primary` after a delete, no prune against its own base list), and every selection read from the desktop carries `userActionHeaders()`, because the gate refuses a private chat's selection without it. `kb_set_active` changes the primary without narrowing search. Set-only edits send neither `primary_kb` nor `clear_primary`; see [`docs/knowledge-base/multi-kb-implementation-plan.md`](docs/knowledge-base/multi-kb-implementation-plan.md). - **Sub-agent loop:** `crates/biorouter-mcp/src/knowledge/subagent/loop_.rs` drives ingest / query / lint macros. Mutating tools accept an optional `txn` so a macro's tool calls commit as one logical change. When working on the Knowledge feature: diff --git a/crates/biorouter-mcp/src/knowledge/git.rs b/crates/biorouter-mcp/src/knowledge/git.rs index ff3877485..b1e0b9e1d 100644 --- a/crates/biorouter-mcp/src/knowledge/git.rs +++ b/crates/biorouter-mcp/src/knowledge/git.rs @@ -100,7 +100,16 @@ impl GitRepo { pub fn log(&self, limit: usize) -> Result> { let mut walk = self.inner.revwalk()?; walk.push_head()?; - walk.set_sorting(git2::Sort::TIME)?; + // ⚠ TOPOLOGICAL, not `TIME` alone. Commit times have one-second + // resolution and a digest makes several commits inside one second — + // `add_raw_source`, the squash commit, a lint autofix — and libgit2's + // time sort leaves equal timestamps in no useful order: measured, it + // listed HEAD and then the rest of the tie OLDEST first, so the Change + // log put a base's `create` above the ingests made after it (QA + // 2026-09-10 F13 asked for the log to match `git log`). Topological + // order never lists a parent before its child; `TIME` only breaks ties + // between branches, which a squash-committed history never has. + walk.set_sorting(git2::Sort::TOPOLOGICAL | git2::Sort::TIME)?; let mut out = Vec::new(); for oid in walk.flatten().take(limit) { let commit = self.inner.find_commit(oid)?; @@ -775,6 +784,69 @@ mod tests { assert_eq!(log[1].summary, "one"); } + /// QA 2026-09-10 F13 asked the history route to match `git log`, and a + /// digest is exactly where it did not: `add_raw_source`, the squash commit + /// and a lint autofix land within one second, and `Sort::TIME` alone leaves + /// commits with EQUAL timestamps in no particular order — measured, it + /// listed a base's `create` commit above the two ingests made after it. + /// The timestamps here are pinned equal, so the tie is certain rather than + /// a matter of how fast this machine commits. + #[test] + fn log_keeps_commit_order_when_commits_share_a_timestamp() { + let dir = tempfile::tempdir().unwrap(); + let repo = GitRepo::init(dir.path()).unwrap(); + let sig = git2::Signature::new( + "Biorouter Knowledge", + "knowledge@biorouter.local", + &git2::Time::new(1_789_000_000, 0), + ) + .unwrap(); + let mut expected = Vec::new(); + for (step, kind) in [ + ChangeKind::Manual, + ChangeKind::Ingest, + ChangeKind::Ingest, + ChangeKind::Lint, + ChangeKind::Restore, + ] + .into_iter() + .enumerate() + { + std::fs::write(dir.path().join(format!("{step}.md")), step.to_string()).unwrap(); + let mut index = repo.inner.index().unwrap(); + stage_all(&mut index).unwrap(); + index.write().unwrap(); + let tree = repo.inner.find_tree(index.write_tree().unwrap()).unwrap(); + let parent = repo.inner.head().ok().and_then(|h| h.peel_to_commit().ok()); + let parents: Vec<&git2::Commit> = parent.iter().collect(); + let summary = format!("step {step}"); + let oid = repo + .inner + .commit( + Some("HEAD"), + &sig, + &sig, + &render_message(kind, &summary, None), + &tree, + &parents, + ) + .unwrap(); + expected.push((oid.to_string(), summary)); + } + expected.reverse(); + + let listed: Vec<(String, String)> = repo + .log(10) + .unwrap() + .into_iter() + .map(|entry| (entry.commit_sha, entry.summary)) + .collect(); + assert_eq!( + listed, expected, + "newest first, and never a parent before its child" + ); + } + #[test] fn txn_lifecycle_squash_merges_into_main() { let dir = tempfile::tempdir().unwrap(); diff --git a/crates/biorouter-server/src/routes/session_reach.rs b/crates/biorouter-server/src/routes/session_reach.rs index 09dbb875e..74d17a4c0 100644 --- a/crates/biorouter-server/src/routes/session_reach.rs +++ b/crates/biorouter-server/src/routes/session_reach.rs @@ -2207,4 +2207,147 @@ mod bypass_tests { "this 400 did not come from `set_selection`: only it echoes the kb id: {body}" ); } + + /// `DELETE /knowledge/bases/{id}` through the real router tree, with the + /// proof — as the Knowledge view sends it. + async fn delete_knowledge_base(state: Arc, kb_id: &str) -> (StatusCode, String) { + let app = crate::routes::configure(state, "task-58-secret".to_string()); + let res = app + .oneshot( + Request::builder() + .method("DELETE") + .uri(format!("/knowledge/bases/{kb_id}")) + .header("X-User-Action", TEST_USER_ACTION_KEY) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + let status = res.status(); + let bytes = to_bytes(res.into_body(), usize::MAX).await.unwrap(); + (status, String::from_utf8_lossy(&bytes).into_owned()) + } + + fn selection_json(body: &str) -> serde_json::Value { + serde_json::from_str(body).unwrap_or_else(|_| panic!("not a selection: {body}")) + } + + /// QA 2026-09-10 F14, the daemon's half, end to end through the real router, + /// the reach gate and a PRIVATE chat — the configuration every chat on a + /// UCSF install is in. + /// + /// QA read the blank `.active-kb` it found after deleting the primary as + /// "the daemon does not repair the selection". The blank IS the repair for a + /// delete (D2 in `docs/knowledge-base/multi-kb-implementation-plan.md`): + /// hiding promotes to the next base, deleting clears to the explicit + /// no-primary, and a chat that merely inherited keeps inheriting. What this + /// pins is the rest of the contract the Knowledge view now relies on instead + /// of re-deriving it: nothing is left pointing at the deleted base, in any + /// scope, and the person at the keyboard can choose again — for a private + /// chat — and have it stick. + #[tokio::test(flavor = "multi_thread")] + #[serial] + async fn deleting_the_primary_leaves_no_pointer_at_it_and_the_user_can_choose_again() { + use biorouter_mcp::knowledge::service::PrimaryUpdate; + + install_test_user_action_key(); + // A throwaway knowledge root: this test creates bases and moves + // pointers, which it must never do in a real one. + let knowledge_root = tempfile::tempdir().unwrap(); + let state = AppState::new_with_knowledge_root(knowledge_root.path().to_path_buf()) + .await + .unwrap(); + let svc = state.knowledge_service.clone(); + let pinning = seed_private_chat(&state, "F14 pinning chat (test fixture)").await; + let inheriting = seed_private_chat(&state, "F14 inheriting chat (test fixture)").await; + svc.create_base("soul", "Soul", None).unwrap(); + svc.create_base("doomed", "Doomed", None).unwrap(); + + // The machine default names the base about to go, so the inheriting + // chat shows it as its primary too; the other chat pins it itself — as + // the person does, with the proof, through the gate. + svc.set_selection(None, None, PrimaryUpdate::Set("doomed")) + .unwrap(); + let (status, body) = post_knowledge_active( + state.clone(), + serde_json::json!({ "session_id": pinning.id(), "primary_kb": "doomed" }), + Some(TEST_USER_ACTION_KEY), + ) + .await; + assert_eq!(status, StatusCode::OK, "{body}"); + for chat in [pinning.id(), inheriting.id()] { + let (status, body) = + get_knowledge_active(state.clone(), chat, Some(TEST_USER_ACTION_KEY)).await; + assert_eq!(status, StatusCode::OK, "{body}"); + assert_eq!(selection_json(&body)["primary_kb"], "doomed", "{body}"); + } + + let (status, body) = delete_knowledge_base(state.clone(), "doomed").await; + assert_eq!(status, StatusCode::NO_CONTENT, "{body}"); + + // No scope reports the deleted base, as primary or as a member… + for chat in [pinning.id(), inheriting.id()] { + let (status, body) = + get_knowledge_active(state.clone(), chat, Some(TEST_USER_ACTION_KEY)).await; + assert_eq!(status, StatusCode::OK, "{body}"); + let selection = selection_json(&body); + assert!(selection["primary_kb"].is_null(), "{body}"); + assert_eq!(selection["kb_ids"], serde_json::json!(["soul"]), "{body}"); + } + let machine = svc.selection(None).unwrap(); + assert_eq!(machine.primary_kb, None); + + // …and none is left STORING it. The two pointers that named it are the + // explicit no-primary — a blank file, which must not fall back to Soul — + // and the chat that only inherited was left inheriting: no file of its + // own was invented for it. + let active_kb = std::fs::read_to_string(knowledge_root.path().join(".active-kb")).unwrap(); + assert_eq!( + active_kb.trim(), + "", + "the machine pointer still names something" + ); + let sessions = knowledge_root.path().join(".active-kb-sessions"); + let stored: Vec = std::fs::read_dir(&sessions) + .unwrap() + .map(|entry| std::fs::read_to_string(entry.unwrap().path()).unwrap()) + .collect(); + assert_eq!( + stored, + vec![String::new()], + "exactly one chat pinned the base, and its pointer must now be blank" + ); + assert_eq!(svc.get_primary_for_session(inheriting.id()).unwrap(), None); + + // The person chooses again — for a PRIVATE chat, which needs the proof — + // and it sticks: in the answer, in a fresh read, and on disk. + let (status, body) = post_knowledge_active( + state.clone(), + serde_json::json!({ "session_id": inheriting.id(), "primary_kb": "soul" }), + Some(TEST_USER_ACTION_KEY), + ) + .await; + assert_eq!(status, StatusCode::OK, "{body}"); + assert_eq!(selection_json(&body)["primary_kb"], "soul", "{body}"); + let (status, body) = + get_knowledge_active(state.clone(), inheriting.id(), Some(TEST_USER_ACTION_KEY)).await; + assert_eq!(status, StatusCode::OK, "{body}"); + assert_eq!(selection_json(&body)["primary_kb"], "soul", "{body}"); + assert_eq!( + svc.get_primary_for_session(inheriting.id()) + .unwrap() + .as_deref(), + Some("soul") + ); + + // The same write without the proof is still refused, and moves nothing. + let (status, _) = post_knowledge_active( + state.clone(), + serde_json::json!({ "session_id": pinning.id(), "primary_kb": "soul" }), + None, + ) + .await; + assert_eq!(status, StatusCode::FORBIDDEN); + assert_eq!(svc.get_primary_for_session(pinning.id()).unwrap(), None); + } } diff --git a/crates/biorouter-server/tests/knowledge_routes.rs b/crates/biorouter-server/tests/knowledge_routes.rs index 7c1a9f337..7ac850c46 100644 --- a/crates/biorouter-server/tests/knowledge_routes.rs +++ b/crates/biorouter-server/tests/knowledge_routes.rs @@ -720,6 +720,134 @@ async fn history_write_restore_roundtrip() { ); } +/// QA 2026-09-10 F13: after a digest the Change log listed only `create +/// knowledge base …`, while `git log` in the base held the two `[ingest]` +/// commits that wrote every page. The drawer was at fault — it read once, +/// before the digest (`ui/desktop/src/components/knowledge/hooks/useHistory.ts`) +/// — but the obvious suspect was this route: does it filter by a commit-message +/// prefix, or read a side-log instead of git? It does neither, and this pins +/// that it keeps not doing so. Every commit the knowledge write paths make, of +/// every kind, comes back, in `git log`'s order, the ingest pair included. +#[tokio::test] +async fn history_lists_every_commit_git_holds_ingest_included() { + use biorouter_mcp::knowledge::{git::GitRepo, types::ChangeKind}; + + let (_d, root, app) = build_test_router_with_root(); + create_kb(app.clone(), "hist-f13", "History F13").await; + + // What a digest does first: stage the source, which commits + // `[ingest] ingested `. + let res = app + .clone() + .oneshot( + Request::builder() + .method("POST") + .uri("/bases/hist-f13/raw") + .header("content-type", "application/json") + .body(Body::from( + serde_json::to_vec(&serde_json::json!({ + "text": "Metformin is a biguanide used as first-line therapy.", + "title": "Pasted knowledge" + })) + .unwrap(), + )) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(res.status(), 200); + let staged: serde_json::Value = serde_json::from_slice( + &axum::body::to_bytes(res.into_body(), usize::MAX) + .await + .unwrap(), + ) + .unwrap(); + let source_id = staged["source_id"].as_str().unwrap().to_string(); + + // …then the digest itself: pages written on a transaction branch and + // squash-committed onto main as ONE commit, the way the ingest and lint + // macros commit. + let kb_root = root.join("hist-f13"); + let repo = GitRepo::open(&kb_root).unwrap(); + for (kind, label, summary, page) in [ + ( + ChangeKind::Ingest, + "ingest", + "ingest pasted-knowledge", + "metformin", + ), + (ChangeKind::Lint, "lint", "lint autofix", "biguanide"), + ] { + let txn = repo.begin_txn(label).unwrap(); + let notes = kb_root.join("knowledge").join("notes"); + std::fs::create_dir_all(¬es).unwrap(); + std::fs::write( + notes.join(format!("{page}.md")), + valid_page("note", page, &format!("# {page}")), + ) + .unwrap(); + repo.commit_on_txn(&txn, "work in progress").unwrap(); + repo.commit_txn(&txn, kind, summary, Some("+1 page")) + .unwrap(); + } + + let res = app + .oneshot( + Request::builder() + .uri("/bases/hist-f13/history?limit=200") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(res.status(), 200); + let history: Vec = serde_json::from_slice( + &axum::body::to_bytes(res.into_body(), usize::MAX) + .await + .unwrap(), + ) + .unwrap(); + let listed: Vec<(String, String)> = history + .iter() + .map(|entry| { + ( + entry["kind"].as_str().unwrap().to_string(), + entry["summary"].as_str().unwrap().to_string(), + ) + }) + .collect(); + let expected: Vec<(String, String)> = [ + ("lint", "lint autofix".to_string()), + ("ingest", "ingest pasted-knowledge".to_string()), + ("ingest", format!("ingested {source_id}")), + ("manual", "create knowledge base hist-f13".to_string()), + ] + .into_iter() + .map(|(kind, summary)| (kind.to_string(), summary)) + .collect(); + assert_eq!(listed, expected, "the history route must list every commit"); + + // …sha for sha, in the order `git log` prints them. The CLI is the ground + // truth QA compared against; it is optional here only so a machine without + // it does not fail a test about something else. + let shas: Vec<&str> = history + .iter() + .map(|entry| entry["commit_sha"].as_str().unwrap()) + .collect(); + match std::process::Command::new("git") + .arg("-C") + .arg(&kb_root) + .args(["log", "--format=%H"]) + .output() + { + Ok(out) if out.status.success() => { + let log = String::from_utf8(out.stdout).unwrap(); + assert_eq!(shas, log.lines().collect::>()); + } + _ => eprintln!("git CLI unavailable; compared with the commits this test made instead"), + } +} + // ────────────────────────────────────────────────────────────────────────────── // Task 8: POST /bases/:id/raw // ────────────────────────────────────────────────────────────────────────────── diff --git a/docs/knowledge-base/multi-kb-implementation-plan.md b/docs/knowledge-base/multi-kb-implementation-plan.md index 85eaccca5..8221e014b 100644 --- a/docs/knowledge-base/multi-kb-implementation-plan.md +++ b/docs/knowledge-base/multi-kb-implementation-plan.md @@ -162,6 +162,14 @@ It is shown **only when following the default would visibly change something**: The gesture carries **no optimistic pointer** and **omits `hidden_kbs`**. The daemon resolves which base the chat lands on, so guessing would guess at the rule the gesture defers to; and a chat may be inheriting the machine-wide hidden list, so echoing the resolved list back would install a set override from a gesture that means "stop overriding here". +**Amendment (2026-09-11, QA F14): the renderer never writes a selection on its own initiative.** D12 already said the GUI takes the primary from the daemon's answer rather than re-deriving the repair in TypeScript; three code paths still re-derived it by *writing*. Two effects in `KnowledgeContext` "repaired" the renderer's cache — `clear_primary` for a primary missing from its base list, and a pruned `hidden_kbs` — and `useKnowledgeBases.remove` sent `clear_primary` after every delete of the primary. Each installed a durable, session-scoped override from whatever list one renderer held, in every open window, including in a chat the daemon had deliberately left inheriting (D2). Now: + +- `refresh` re-reads the base list **and** the selection, and is what a delete, a rename, the view's mount, opening the chat chip and the end of every turn (`message-stream-finished`) call. The last two exist for QA F6: the agent creates and deletes bases from inside a turn, sometimes from `execute_code` where no knowledge tool call is visible, and the chip read "2 visible" over three bases until a remount. The daemon's repair is adopted, never re-made. +- A pointer at a base the list lacks is *shown* as no primary and is not persisted as one. +- `localStorage` holds only what the daemon confirmed. A write that fails is reported to the person, and the view falls back to the daemon's re-read, or to that last confirmed value. +- "Make primary" sends `hidden_kbs` only when it has to un-hide the base, for the reason the paragraph above gives. +- Every selection read carries `userActionHeaders()`. `GET /knowledge/active` naming a private chat is gated exactly as the POST is, and the reads used to go unproven, so on a UCSF install, where every chat is private, the renderer's cache stood in for the chat's selection. + ### Policies this plan deliberately does **not** change - **KB-less `kb_search` keeps meaning "every base in this session."** Under the merged model that sentence is both today's behaviour and the literal ask. No `scope` parameter, no narrowing, no regression. diff --git a/ui/desktop/src/components/bottom_menu/BottomMenuKnowledgeSelection.followsDaemon.test.tsx b/ui/desktop/src/components/bottom_menu/BottomMenuKnowledgeSelection.followsDaemon.test.tsx new file mode 100644 index 000000000..67ba6859b --- /dev/null +++ b/ui/desktop/src/components/bottom_menu/BottomMenuKnowledgeSelection.followsDaemon.test.tsx @@ -0,0 +1,161 @@ +import { act, render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import { KnowledgeProvider } from '../knowledge/KnowledgeContext'; +import { BottomMenuKnowledgeSelection } from './BottomMenuKnowledgeSelection'; + +/** + * QA 2026-09-10 F6: the chip read "Manage knowledge bases (2 visible)" over + * three bases on disk, none hidden, until navigating to the Knowledge view and + * back remounted it. The agent had created the third base from inside the chat + * — from `execute_code`, where no knowledge tool call reaches the renderer — and + * nothing told the chip. + * + * Unlike `BottomMenuKnowledgeSelection.test.tsx`, which stubs the context to + * pin how the chip DRIVES it, this runs the real `KnowledgeProvider` over a + * mocked daemon: the defect is in what the provider listens to. + */ + +const mocks = vi.hoisted(() => ({ + listBases: vi.fn(), + getActive: vi.fn(), + setActive: vi.fn(), +})); + +vi.mock('../../api', () => ({ + listBases: mocks.listBases, + getActive: mocks.getActive, + setActive: mocks.setActive, +})); + +vi.mock('../../toasts', () => ({ toastError: vi.fn() })); + +function base(id: string) { + return { id, name: id, color: '#cf6d47', created_at: '', schema_version: 3, tier: 'public' }; +} + +/** What the daemon holds. Tests move it the way the agent would. */ +const daemon = { + bases: [base('soul'), base('brainstorm')], + selection: { + kb_ids: ['brainstorm', 'soul'], + primary_kb: 'soul' as string | null, + active_kb: 'soul' as string | null, + hidden_kbs: [] as string[], + }, +}; + +beforeAll(() => { + vi.stubGlobal( + 'ResizeObserver', + class { + observe() {} + unobserve() {} + disconnect() {} + } + ); +}); + +afterAll(() => { + vi.unstubAllGlobals(); +}); + +beforeEach(() => { + localStorage.clear(); + vi.clearAllMocks(); + daemon.bases = [base('soul'), base('brainstorm')]; + daemon.selection = { + kb_ids: ['brainstorm', 'soul'], + primary_kb: 'soul', + active_kb: 'soul', + hidden_kbs: [], + }; + mocks.listBases.mockImplementation(() => Promise.resolve({ data: [...daemon.bases] })); + mocks.getActive.mockImplementation(() => Promise.resolve({ data: { ...daemon.selection } })); +}); + +function renderChip() { + return render( + + + + ); +} + +function chipLabel() { + return screen.getByRole('button', { name: /Manage knowledge bases/ }).getAttribute('aria-label'); +} + +/** The app's end-of-turn signal, as `ChatStreamController` dispatches it. */ +async function finishATurn() { + await act(async () => { + window.dispatchEvent(new CustomEvent('message-stream-finished')); + await Promise.resolve(); + }); +} + +describe('BottomMenuKnowledgeSelection follows the daemon', () => { + it('counts a base the agent created once the turn ends, without a remount', async () => { + renderChip(); + await waitFor(() => expect(chipLabel()).toBe('Manage knowledge bases (2 visible)')); + + // The agent creates a base during the turn. + daemon.bases = [...daemon.bases, base('f6-chip-probe')]; + daemon.selection = { ...daemon.selection, kb_ids: ['brainstorm', 'f6-chip-probe', 'soul'] }; + await finishATurn(); + + await waitFor(() => expect(chipLabel()).toBe('Manage knowledge bases (3 visible)')); + }); + + it('drops a base the agent deleted once the turn ends', async () => { + renderChip(); + await waitFor(() => expect(chipLabel()).toBe('Manage knowledge bases (2 visible)')); + + daemon.bases = [base('soul')]; + daemon.selection = { ...daemon.selection, kb_ids: ['soul'] }; + await finishATurn(); + + await waitFor(() => expect(chipLabel()).toBe('Manage knowledge bases (1 visible)')); + }); + + // The selection rides with the list: the agent can hide a base from this chat + // (`kb_set_active`, `workspace_set_tools`) in the same turn it creates one. + it("follows the chat's own set when the agent changed it", async () => { + renderChip(); + await waitFor(() => expect(chipLabel()).toBe('Manage knowledge bases (2 visible)')); + + daemon.selection = { ...daemon.selection, kb_ids: ['soul'], hidden_kbs: ['brainstorm'] }; + await finishATurn(); + + await waitFor(() => expect(chipLabel()).toBe('Manage knowledge bases (1 visible)')); + }); + + // A base created from the CLI or another window ends no turn here. Opening + // the chip is when the person asks, so it asks the daemon then. + it('re-reads the bases when the chip is opened', async () => { + renderChip(); + await waitFor(() => expect(chipLabel()).toBe('Manage knowledge bases (2 visible)')); + + daemon.bases = [...daemon.bases, base('from-the-cli')]; + daemon.selection = { ...daemon.selection, kb_ids: ['brainstorm', 'from-the-cli', 'soul'] }; + await userEvent.click(screen.getByRole('button', { name: /Manage knowledge bases/ })); + + await waitFor(() => expect(screen.getByText('from-the-cli')).toBeInTheDocument()); + expect(chipLabel()).toBe('Manage knowledge bases (3 visible)'); + }); + + // pin-outranks-the-row, for knowledge: a refresh must never be what resets the + // chat's primary. A list that arrives without the primary's base (stale, or a + // daemon that filters it) used to trigger a durable `clear_primary` write. + it('never writes the selection on its own initiative', async () => { + renderChip(); + await waitFor(() => expect(chipLabel()).toBe('Manage knowledge bases (2 visible)')); + + daemon.bases = [base('brainstorm')]; + await finishATurn(); + await waitFor(() => expect(chipLabel()).toBe('Manage knowledge bases (1 visible)')); + + expect(mocks.setActive).not.toHaveBeenCalled(); + expect(localStorage.getItem('knowledge_active_kb:chat-1')).toBe('soul'); + }); +}); diff --git a/ui/desktop/src/components/bottom_menu/BottomMenuKnowledgeSelection.test.tsx b/ui/desktop/src/components/bottom_menu/BottomMenuKnowledgeSelection.test.tsx index aead85111..c97138335 100644 --- a/ui/desktop/src/components/bottom_menu/BottomMenuKnowledgeSelection.test.tsx +++ b/ui/desktop/src/components/bottom_menu/BottomMenuKnowledgeSelection.test.tsx @@ -13,6 +13,7 @@ const mocks = vi.hoisted(() => ({ setHiddenKbIds: vi.fn(), hideAllKnowledgeBases: vi.fn(), showAllKnowledgeBases: vi.fn(), + refresh: vi.fn(async () => {}), state: { bases: [] as { id: string; name: string }[], hiddenKbIds: [] as string[] }, })); @@ -46,6 +47,7 @@ vi.mock('../knowledge/KnowledgeContext', () => ({ mocks.showAllKnowledgeBases(); commit([]); }, + refresh: mocks.refresh, }; }, })); diff --git a/ui/desktop/src/components/bottom_menu/BottomMenuKnowledgeSelection.tsx b/ui/desktop/src/components/bottom_menu/BottomMenuKnowledgeSelection.tsx index 609f6499c..3fba3e31e 100644 --- a/ui/desktop/src/components/bottom_menu/BottomMenuKnowledgeSelection.tsx +++ b/ui/desktop/src/components/bottom_menu/BottomMenuKnowledgeSelection.tsx @@ -22,6 +22,7 @@ export function BottomMenuKnowledgeSelection() { toggleKbHidden, hideAllKnowledgeBases, showAllKnowledgeBases, + refresh, } = useKnowledge(); const [open, setOpen] = useState(false); const [searchQuery, setSearchQuery] = useState(''); @@ -68,6 +69,10 @@ export function BottomMenuKnowledgeSelection() { onOpenChange={(nextOpen) => { setOpen(nextOpen); if (!nextOpen) setSearchQuery(''); + // Re-read on open, as the Knowledge view's picker and manager do: the + // provider already follows a turn's end, but a base created from the + // CLI or another window changes nothing this renderer can hear. + if (nextOpen) void refresh(); }} > diff --git a/ui/desktop/src/components/knowledge/KBSelector/KBManagerDialog.tsx b/ui/desktop/src/components/knowledge/KBSelector/KBManagerDialog.tsx index ef9bcf283..9b3745f98 100644 --- a/ui/desktop/src/components/knowledge/KBSelector/KBManagerDialog.tsx +++ b/ui/desktop/src/components/knowledge/KBSelector/KBManagerDialog.tsx @@ -169,10 +169,12 @@ export function KBManagerDialog({ open, onOpenChange, startInCreate = false }: P try { if (draftMode?.kind === 'rename') { setBusyId(draftMode.base.id); - const manifest = await rename(draftMode.base.id, trimmed); - if (primaryKbId === draftMode.base.id) { - setPrimaryKbId(manifest.id); - } + // No `setPrimaryKbId(manifest.id)` after this: a rename moves every + // pointer that named the base, machine default and chats alike, and + // `rename` ends by re-reading the selection. Re-pinning it from here + // turned a chat that only inherited the renamed base into one that + // pinned it. + await rename(draftMode.base.id, trimmed); } resetDraft(); await refresh(); diff --git a/ui/desktop/src/components/knowledge/KnowledgeContext.test.tsx b/ui/desktop/src/components/knowledge/KnowledgeContext.test.tsx index 760d9fadd..52f8af704 100644 --- a/ui/desktop/src/components/knowledge/KnowledgeContext.test.tsx +++ b/ui/desktop/src/components/knowledge/KnowledgeContext.test.tsx @@ -1,8 +1,9 @@ import { act, render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { KnowledgeProvider, useKnowledge } from './KnowledgeContext'; -import { reachGatedGetActive, USER_ACTION_KEY } from '../../test/reachGate'; +import { KnowledgeProvider, SELECTION_NOT_SAVED_TITLE, useKnowledge } from './KnowledgeContext'; +import { useKnowledgeBases } from './hooks/useKnowledgeBases'; +import { reachGatedGetActive, SESSION_OUT_OF_REACH, 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() { @@ -28,14 +29,21 @@ const mocks = vi.hoisted(() => ({ listBases: vi.fn(), getActive: vi.fn(), setActive: vi.fn(), + createBase: vi.fn(), + deleteBase: vi.fn(), + toastError: vi.fn(), })); vi.mock('../../api', () => ({ listBases: mocks.listBases, getActive: mocks.getActive, setActive: mocks.setActive, + createBase: mocks.createBase, + deleteBase: mocks.deleteBase, })); +vi.mock('../../toasts', () => ({ toastError: mocks.toastError })); + function base(id: string) { return { id, name: id, color: '#cf6d47', created_at: '', schema_version: 1 }; } @@ -73,6 +81,7 @@ function Probe() { toggleKbHidden, refresh, } = useKnowledge(); + const { remove } = useKnowledgeBases(); return (
{primaryKbId ?? 'none'} @@ -100,6 +109,9 @@ function Probe() { +
); } @@ -143,6 +155,7 @@ beforeEach(() => { mocks.setActive.mockResolvedValue({ data: { kb_ids: ['alpha', 'beta'], primary_kb: 'beta', active_kb: 'beta', hidden_kbs: [] }, }); + mocks.deleteBase.mockResolvedValue({}); }); describe('KnowledgeContext', () => { @@ -291,6 +304,11 @@ describe('KnowledgeContext', () => { expect(mocks.getActive).toHaveBeenCalledTimes(2); expect(screen.getByTestId('primary').textContent).toBe('alpha'); expect(screen.getByTestId('hidden').textContent).toBe('beta'); + // …and the person who clicked is told the click did not land. + expect(mocks.toastError).toHaveBeenCalledWith({ + title: SELECTION_NOT_SAVED_TITLE, + msg: 'network down', + }); }); // Same divergence by the other door: the client resolves, but with an error @@ -308,6 +326,10 @@ describe('KnowledgeContext', () => { expect(mocks.getActive).toHaveBeenCalledTimes(2); expect(screen.getByTestId('primary').textContent).toBe('alpha'); expect(screen.getByTestId('hidden').textContent).toBe('beta'); + expect(mocks.toastError).toHaveBeenCalledWith({ + title: SELECTION_NOT_SAVED_TITLE, + msg: 'primary_kb is not a member', + }); }); // Two clicks, two writes, answers out of order. The older answer describes a @@ -358,21 +380,25 @@ describe('KnowledgeContext', () => { expect(screen.getByTestId('hidden').textContent).toBe('beta'); }); - it('clears a stale primary after an empty base list has arrived', async () => { + // A pointer at a base the list no longer holds must not be SHOWN — the view, + // the ingest target and the graph would all aim at a base that is gone — and + // must not be WRITTEN back as "no primary" either. That write used to happen + // here: a durable, session-scoped override derived from whatever list this + // renderer held, installed in a chat that may only have inherited the pointer + // and that the daemon had deliberately left alone (D2; QA 2026-09-10 F14). + it('hides a primary whose base is gone without writing a durable clear', async () => { mocks.listBases.mockResolvedValue({ data: [] }); daemon.session.hidden_kbs = []; - mocks.setActive.mockResolvedValue({ - data: { kb_ids: [], primary_kb: null, active_kb: null, hidden_kbs: [] }, - }); renderProvider(); + await waitFor(() => expect(mocks.listBases).toHaveBeenCalled()); await waitFor(() => expect(screen.getByTestId('primary')).toHaveTextContent('none')); - await waitFor(() => expect(mocks.setActive).toHaveBeenCalled()); - expect(mocks.setActive.mock.calls[0]?.[0]?.body).toMatchObject({ - clear_primary: true, - session_id: 'chat-1', - }); + await settle(() => {}); + expect(mocks.setActive).not.toHaveBeenCalled(); + // …and the daemon's answer is what `localStorage` keeps: the renderer did + // not invent a different one to persist. + expect(localStorage.getItem('knowledge_active_kb:chat-1')).toBe('alpha'); }); // Same, by the other door: a list request that fails is not a list of zero @@ -420,6 +446,132 @@ describe('KnowledgeContext', () => { expect(screen.getByTestId('bases-error').textContent).not.toBe(''); }); + // QA 2026-09-10 F14. Around the refused reads (see 'a private chat' below), the + // renderer made selection writes nobody asked for, which is how a click could + // look saved and not be. It now writes only what a person clicked, persists + // only what the daemon confirmed, and says so when a write does not land. + describe('writes only what the daemon confirmed', () => { + // The renderer believed the write succeeded while the daemon had refused + // it: `localStorage` took the guess before the POST went out, and nothing on + // screen ever said the click had not landed. + it('says a refused write did not land, and keeps localStorage on what the daemon confirmed', async () => { + const write = deferred(); + renderProvider(); + await waitFor(() => expect(screen.getByTestId('primary')).toHaveTextContent('alpha')); + expect(localStorage.getItem('knowledge_active_kb:chat-1')).toBe('alpha'); + + mocks.setActive.mockReturnValue(write.promise); + // …and the recovery read cannot reach the daemon either. + mocks.getActive.mockRejectedValue(new Error('Failed to fetch')); + await userEvent.click(screen.getByRole('button', { name: 'make beta primary' })); + + // Optimistic on screen, never in storage. + expect(screen.getByTestId('primary').textContent).toBe('beta'); + expect(localStorage.getItem('knowledge_active_kb:chat-1')).toBe('alpha'); + + const listsBefore = mocks.listBases.mock.calls.length; + await settle(() => write.resolve({ error: SESSION_OUT_OF_REACH })); + + await waitFor(() => expect(mocks.toastError).toHaveBeenCalledTimes(1)); + expect(mocks.toastError).toHaveBeenCalledWith({ + title: SELECTION_NOT_SAVED_TITLE, + msg: 'That chat is private, or there is no chat with that id.', + }); + // The list is re-read as well: a choice is most often refused because + // the base it named went away somewhere else. + expect(mocks.listBases.mock.calls.length).toBeGreaterThan(listsBefore); + await waitFor(() => expect(screen.getByTestId('primary')).toHaveTextContent('alpha')); + expect(screen.getByTestId('hidden').textContent).toBe('beta'); + expect(localStorage.getItem('knowledge_active_kb:chat-1')).toBe('alpha'); + expect(localStorage.getItem('knowledge_hidden_kbs:chat-1')).toBe('["beta"]'); + }); + + // A base the chat already uses needs no set edit. Echoing the resolved + // hidden list back installed a session-scoped override on a chat that was + // inheriting the machine-wide one. + it('makes a base already in the chat primary without re-sending the set', async () => { + daemon.session = { + kb_ids: ['alpha', 'beta'], + primary_kb: 'alpha', + active_kb: 'alpha', + hidden_kbs: [], + }; + renderProvider(); + await waitFor(() => expect(screen.getByTestId('primary')).toHaveTextContent('alpha')); + + await userEvent.click(screen.getByRole('button', { name: 'make beta primary' })); + + await waitFor(() => expect(mocks.setActive).toHaveBeenCalled()); + const body = mocks.setActive.mock.calls[0]?.[0]?.body; + expect(body.primary_kb).toBe('beta'); + expect(body.hidden_kbs).toBeUndefined(); + }); + + // The delete IS the repair (D2): the daemon clears every pointer that named + // the base. The renderer used to write `clear_primary` on top of it — a + // durable "no primary" in a chat that may only have inherited the pointer. + it("follows the daemon's repair after a delete instead of writing a clear", async () => { + renderProvider(); + await waitFor(() => expect(screen.getByTestId('primary')).toHaveTextContent('alpha')); + // What the daemon holds once `alpha` is gone: it cleared this chat's pin. + mocks.listBases.mockResolvedValue({ data: [base('beta')] }); + daemon.session = { kb_ids: [], primary_kb: null, active_kb: null, hidden_kbs: ['beta'] }; + const readsBefore = mocks.getActive.mock.calls.length; + + await userEvent.click(screen.getByRole('button', { name: 'delete alpha' })); + + await waitFor(() => + expect(mocks.deleteBase).toHaveBeenCalledWith( + expect.objectContaining({ path: { id: 'alpha' } }) + ) + ); + await waitFor(() => expect(mocks.getActive.mock.calls.length).toBeGreaterThan(readsBefore)); + await waitFor(() => expect(localStorage.getItem('knowledge_active_kb:chat-1')).toBeNull()); + expect(screen.getByTestId('primary').textContent).toBe('none'); + expect(mocks.setActive).not.toHaveBeenCalled(); + }); + + // `refresh` now re-reads the selection, so it must never land on top of a + // click: a read answered before the write commits describes the selection + // the user just moved away from. + it('never lets a background refresh overwrite a write that is still out', async () => { + renderProvider(); + await waitFor(() => expect(screen.getByTestId('primary')).toHaveTextContent('alpha')); + + // A refresh whose selection read is answered late, with the old selection… + const staleRead = deferred(); + mocks.getActive.mockReturnValueOnce(staleRead.promise); + await userEvent.click(screen.getByRole('button', { name: 'refresh' })); + + // …then a click. + const write = deferred(); + mocks.setActive.mockReturnValue(write.promise); + await userEvent.click(screen.getByRole('button', { name: 'make beta primary' })); + + // A refresh while the write is out does not even ask. + const readsDuringWrite = mocks.getActive.mock.calls.length; + await userEvent.click(screen.getByRole('button', { name: 'refresh' })); + await settle(() => {}); + expect(mocks.getActive.mock.calls.length).toBe(readsDuringWrite); + + await settle(() => staleRead.resolve({ data: daemon.session })); + expect(screen.getByTestId('primary').textContent).toBe('beta'); + + await settle(() => + write.resolve({ + data: { + kb_ids: ['alpha', 'beta'], + primary_kb: 'beta', + active_kb: 'beta', + hidden_kbs: [], + }, + }) + ); + expect(screen.getByTestId('primary').textContent).toBe('beta'); + expect(localStorage.getItem('knowledge_active_kb:chat-1')).toBe('beta'); + }); + }); + // The fourth intent. `clear` writes a *durable* "this chat has no primary", // and deleting the base a chat had pinned installs exactly that — so without // a way to drop the chat's own pointer, such a chat could never follow the diff --git a/ui/desktop/src/components/knowledge/KnowledgeContext.tsx b/ui/desktop/src/components/knowledge/KnowledgeContext.tsx index 2201920b2..e621b3944 100644 --- a/ui/desktop/src/components/knowledge/KnowledgeContext.tsx +++ b/ui/desktop/src/components/knowledge/KnowledgeContext.tsx @@ -8,10 +8,16 @@ import { useRef, useState, } from 'react'; -import { listBases, getActive, setActive } from '../../api'; -import { readHidden, readPrimary } from './knowledgeSelection'; +import { listBases, setActive } from '../../api'; +import { + fetchKnowledgeSelection, + readHidden, + readPrimary, + type SelectionPayload, +} from './knowledgeSelection'; import { briefSelectionFailure } from './selectionWarning'; import { userActionHeaders } from '../../utils/userAction'; +import { toastError } from '../../toasts'; /** * `KbListEntry` is `Manifest & { tier }` — the manifest the daemon stores plus * the privacy tier, which lives in `.kb-tiers` and not in `manifest.yaml` @@ -53,6 +59,13 @@ type PrimaryUpdate = | { kind: 'inherit' } | { kind: 'set'; id: string }; +/** + * The title of the one error a person sees when a selection change they made + * did not land. A console line is not a report: the person clicked, the chip + * moved, and without this nothing on screen would ever say it moved back. + */ +export const SELECTION_NOT_SAVED_TITLE = 'Knowledge base selection not saved'; + interface KnowledgeContextType { bases: KbListEntry[]; /** The session's knowledge bases — the one axis. Searchable, readable, usable. */ @@ -111,13 +124,16 @@ export function KnowledgeProvider({ }) { const [bases, setBases] = useState([]); // Has a base list ever arrived? Until it has, `bases` being empty says nothing - // about which bases exist, so nothing may be pruned against it. + // about which bases exist, so no pointer may be judged missing against it. const [basesLoaded, setBasesLoaded] = useState(false); const [loading, setLoading] = useState(true); const [basesError, setBasesError] = useState(null); const storageKey = useMemo(() => storageKeyForSession(sessionId), [sessionId]); const hiddenStorageKey = useMemo(() => hiddenStorageKeyForSession(sessionId), [sessionId]); - const [primaryKbId, setPrimaryKbIdState] = useState(() => + // The pointer as last adopted — from the daemon, or optimistically from a + // click. What consumers see is `primaryKbId` below, which also hides a + // pointer at a base the list no longer holds. + const [storedPrimaryKbId, setPrimaryKbIdState] = useState(() => localStorage.getItem(storageKeyForSession(sessionId)) ); const [hiddenKbIds, setHiddenKbIdsState] = useState(() => { @@ -139,11 +155,23 @@ export function KnowledgeProvider({ // would spend a request per chat switch on a value most chats never show. const [defaultPrimaryKbId, setDefaultPrimaryKbId] = useState(null); const graphRefreshRef = useRef<(() => Promise) | null>(null); - // Every selection round-trip — a write, its recovery read, a hydrate — takes a - // generation. Only the newest may write state back, so a slow answer cannot - // reinstate a selection the user has already clicked past. + // Every selection round-trip a USER starts — a write, its recovery read, the + // hydrate on a chat switch — takes a generation. Only the newest may write + // state back, so a slow answer cannot reinstate a selection the user has + // already clicked past. A background re-read (`resyncSelection`) takes none of + // its own; it may only land while the generation it started under is current. const selectionGenerationRef = useRef(0); + // How many user writes are waiting on the daemon. A background re-read that + // starts while one is out could be answered before the write commits, and + // would then put the pre-write selection back over the write's answer. + const writesInFlightRef = useRef(0); + /** + * Adopt a selection the DAEMON reported. `localStorage` is written here and + * nowhere else: it holds the last selection the daemon confirmed, never a + * guess, which is what lets a failed write fall back to it (QA 2026-09-10 + * F14 found it holding `soul` while the daemon had never stored it). + */ const applyPrimary = useCallback( (primary: string | null) => { setPrimaryKbIdState(primary); @@ -161,30 +189,106 @@ export function KnowledgeProvider({ [hiddenStorageKey] ); - /** Re-read the authoritative selection after a write that did not land. */ - const rehydrateSelection = useCallback( - async (generation: number) => { + /** Put back the last selection the daemon confirmed for this scope. */ + const restoreConfirmedSelection = useCallback(() => { + setPrimaryKbIdState(localStorage.getItem(storageKey)); + try { + const parsed: unknown = JSON.parse(localStorage.getItem(hiddenStorageKey) ?? '[]'); + setHiddenKbIdsState( + Array.isArray(parsed) ? parsed.filter((id): id is string => typeof id === 'string') : [] + ); + } catch { + setHiddenKbIdsState([]); + } + }, [hiddenStorageKey, storageKey]); + + /** + * Re-read this scope's selection and adopt it, WITHOUT superseding anything + * the user is doing: skipped while a write is out, and dropped if a write or + * a chat switch starts while the read is. + * + * This is how the renderer follows a selection that moved underneath it — a + * base deleted (the daemon clears every pointer that named it), the agent's + * `kb_set_active`, another window — and it replaces two effects that used to + * "repair" this renderer's cache by WRITING: clearing a primary missing from + * the base list, and pruning the hidden set against it. Both installed a + * durable, session-scoped override the user never asked for, from whatever + * list this renderer happened to hold, in every window at once. The daemon + * already made the one repair the model allows (D2); the renderer reads it. + */ + const resyncSelection = useCallback(async () => { + if (writesInFlightRef.current > 0) return; + const generation = selectionGenerationRef.current; + try { + const data = await fetchKnowledgeSelection(sessionId); + if (generation !== selectionGenerationRef.current) return; + applyPrimary(readPrimary(data)); + const hidden = readHidden(data); + if (hidden) applyHidden(hidden); + } catch (err) { + // A failed read changes nothing on screen: it is not evidence the + // selection moved, and the next refresh or hydrate asks again. + console.warn('Knowledge selection not re-read:', briefSelectionFailure(err)); + } + }, [applyHidden, applyPrimary, sessionId]); + + const refreshBases = useCallback(async () => { + setLoading(true); + try { + // With the proof, for the reason `fetchKnowledgeSelection` gives: a daemon that + // filters what an unproven caller may see would otherwise hand this list + // back with the user's own private bases missing. + const res = await listBases({ headers: await userActionHeaders(), throwOnError: true }); + setBases(res.data || []); + setBasesLoaded(true); + setBasesError(null); + } catch (err) { + // Keep the list we already had. A failed request is not a list of zero + // bases, and a consumer reading it as one would conclude every base it + // knows about is gone. + console.error('listBases failed:', err); + // …and say that it is stale, in a value that is never falsy on failure: + // an error reported as '' reads as "no failure" at every call site. + const message = err instanceof Error ? err.message : String(err); + setBasesError(message || 'Could not load knowledge bases.'); + } finally { + setLoading(false); + } + }, []); + + /** + * A write that did not land: say so, then show the truth. + * + * The daemon's answer is re-read (with the proof, so a private chat is not + * refused a second time for a different reason) and adopted. When even that + * fails, the last selection the daemon CONFIRMED comes back from + * `localStorage` — never the optimistic value, which is the one thing known + * not to have been stored. The list is re-read too: the likeliest reason a + * choice is refused is that the base it named went away in another window. + */ + const recoverFromFailedWrite = useCallback( + async (generation: number, failure: unknown) => { + toastError({ + title: SELECTION_NOT_SAVED_TITLE, + msg: briefSelectionFailure(failure), + }); + void refreshBases(); 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, - }); + const data = await fetchKnowledgeSelection(sessionId); if (generation !== selectionGenerationRef.current) return; - applyPrimary(readPrimary(res.data)); - const hidden = readHidden(res.data); + applyPrimary(readPrimary(data)); + const hidden = readHidden(data); if (hidden) applyHidden(hidden); } catch (err) { - // Both the write and the recovery read failed. Keep what is on screen — - // there is nothing better to show, and the next hydrate will settle it. + if (generation !== selectionGenerationRef.current) return; console.warn( 'Knowledge selection not re-read after a failed write:', briefSelectionFailure(err) ); + restoreConfirmedSelection(); } }, - [applyHidden, applyPrimary, sessionId] + [applyHidden, applyPrimary, refreshBases, restoreConfirmedSelection, sessionId] ); /** @@ -221,18 +325,23 @@ export function KnowledgeProvider({ ); } if (nextHiddenKbIds) setHiddenKbIdsState(nextHiddenKbIds); - if (primary.kind === 'set') localStorage.setItem(storageKey, primary.id); - if (primary.kind === 'clear') localStorage.removeItem(storageKey); - if (nextHiddenKbIds) localStorage.setItem(hiddenStorageKey, JSON.stringify(nextHiddenKbIds)); - // Issue #56 Task 58: this POST names a chat, and repointing a PRIVATE - // chat's knowledge bases needs the proof-of-user. `userActionHeaders` - // resolves to `{}` rather than rejecting when there is no bridge, so the - // chain below is unchanged in shape and the `.catch` still means what it - // meant. - void userActionHeaders() - .then((headers) => - setActive({ - headers, + // ⚠ **Nothing is written to `localStorage` here.** It used to take the + // optimistic value before the POST went out, so a write the daemon refused + // left it holding a selection the daemon never stored — and a later + // hydrate that could not reach the daemon then restored that guess as if + // it were the chat's (QA 2026-09-10 F14). `applyPrimary`/`applyHidden` + // persist the daemon's ANSWER, and only they do. + writesInFlightRef.current += 1; + void (async () => { + let data: SelectionPayload = undefined; + let failure: unknown = null; + try { + const res = await setActive({ + // Issue #56 Task 58: this POST names a chat, and repointing a + // PRIVATE chat's knowledge bases needs the proof-of-user. + // `userActionHeaders` resolves to `{}` rather than rejecting when + // there is no bridge, and the daemon then refuses in words. + headers: await userActionHeaders(), body: { // The three primary gestures are mutually exclusive on the wire: // two of them in one body is a 400 naming both fields, not a @@ -244,48 +353,55 @@ export function KnowledgeProvider({ session_id: sessionId || undefined, }, throwOnError: false, - }) - ) - .then((res) => { - // A newer edit is already in flight (or has already answered): this - // answer describes a selection the user has clicked past, so applying - // it would silently undo their newer choice. - if (generation !== selectionGenerationRef.current) return; - // The daemon owns the "primary must be a member" repair: hiding the - // primary promotes to the first remaining base, hiding everything - // clears it. Adopt its answer instead of re-implementing that rule - // here, where the two would silently drift apart. - const data = res?.data; - if (!data) { - // The write did not land. Keeping the optimistic value would leave - // the chip, the Knowledge view and the ingest target describing a - // selection the daemon never applied, with nothing to correct it — - // so re-read the truth instead of guessing at it. - console.warn('setActive (server sync) returned no selection:', res?.error); - void rehydrateSelection(generation); - return; - } - applyPrimary(readPrimary(data)); - // Adopt the set too, not just the pointer: the repair moves both, and - // taking one half of it is how the renderer ends up holding a primary - // that is not a member of its own visible set. - const appliedHidden = readHidden(data); - if (appliedHidden) applyHidden(appliedHidden); - }) - .catch((err) => { - if (generation !== selectionGenerationRef.current) return; - console.warn('setActive (server sync) failed:', err); - void rehydrateSelection(generation); - }); + }); + data = res?.data; + // An error envelope instead of a selection is a write that did not + // land, however politely it arrived. + if (!data) failure = res?.error ?? 'The daemon answered without a selection.'; + } catch (err) { + failure = err; + } finally { + writesInFlightRef.current -= 1; + } + // A newer edit is already in flight (or has already answered): this + // answer describes a selection the user has clicked past, so applying + // it — or reporting it — would silently undo their newer choice. + if (generation !== selectionGenerationRef.current) return; + if (failure !== null) { + // Keeping the optimistic value would leave the chip, the Knowledge + // view and the ingest target describing a selection the daemon never + // applied, with nothing to correct it and nothing on screen to say so. + void recoverFromFailedWrite(generation, failure); + return; + } + // The daemon owns the "primary must be a member" repair: hiding the + // primary promotes to the first remaining base, hiding everything + // clears it. Adopt its answer instead of re-implementing that rule + // here, where the two would silently drift apart. + applyPrimary(readPrimary(data)); + // Adopt the set too, not just the pointer: the repair moves both, and + // taking one half of it is how the renderer ends up holding a primary + // that is not a member of its own visible set. + const appliedHidden = readHidden(data); + if (appliedHidden) applyHidden(appliedHidden); + })(); }, - [applyHidden, applyPrimary, hiddenStorageKey, rehydrateSelection, sessionId, storageKey] + [applyHidden, applyPrimary, recoverFromFailedWrite, sessionId] ); const setPrimaryKbId = useCallback( (id: string | null) => { - // The primary must be a member of the set, so making a base primary adds - // it to this chat in the same request — one gesture, one POST. - const nextHidden = id ? hiddenKbIds.filter((hiddenId) => hiddenId !== id) : hiddenKbIds; + // The primary must be a member of the set, so making a HIDDEN base + // primary adds it to this chat in the same request — one gesture, one + // POST, validated by the daemon against the state it produces. + // + // ⚠ Only then does the set travel. A base already in the chat needs no + // set edit, and echoing the resolved hidden list back would install a + // session-scoped override on a chat that is inheriting the machine-wide + // list — the very thing `syncSelection`'s `null` exists to avoid. It did + // exactly that on every "make primary" until 2026-09-11. + const nextHidden = + id && hiddenKbIds.includes(id) ? hiddenKbIds.filter((hiddenId) => hiddenId !== id) : null; syncSelection(id ? { kind: 'set', id } : { kind: 'clear' }, nextHidden); }, [hiddenKbIds, syncSelection] @@ -306,8 +422,7 @@ export function KnowledgeProvider({ return; } try { - const res = await getActive({ query: undefined, throwOnError: true }); - setDefaultPrimaryKbId(readPrimary(res.data)); + setDefaultPrimaryKbId(readPrimary(await fetchKnowledgeSelection(null))); } catch (err) { // Keep the last known default: a failed read is not evidence that there // is none, and inventing one would offer a base nobody chose. @@ -353,26 +468,25 @@ export function KnowledgeProvider({ setHiddenKbIds([]); }, [setHiddenKbIds]); + // `refresh` stays referentially stable across chat switches: half a dozen + // consumers run it from an effect keyed on its identity, and one of them (the + // manager dialog) resets a half-typed form whenever that effect re-runs. + const resyncSelectionRef = useRef(resyncSelection); + resyncSelectionRef.current = resyncSelection; + + /** + * Re-read the daemon: the base list and this scope's selection, together. + * + * This is the Knowledge feature's one change signal — the view calls it when + * it mounts, the picker and the manager when they open, every create, delete, + * rename and import when it lands, and the provider itself when a turn ends + * (below). The selection rides with the list because anything that moved one + * can have moved the other: deleting a base clears every pointer that named + * it, and the agent can create a base and pin it in one turn. + */ const refresh = useCallback(async () => { - setLoading(true); - try { - const res = await listBases({ throwOnError: true }); - setBases(res.data || []); - setBasesLoaded(true); - setBasesError(null); - } catch (err) { - // Keep the list we already had. A failed request is not a list of zero - // bases, and emptying it here is what let the prune below read "every - // stored id names a base that no longer exists". - console.error('listBases failed:', err); - // …and say that it is stale, in a value that is never falsy on failure: - // an error reported as '' reads as "no failure" at every call site. - const message = err instanceof Error ? err.message : String(err); - setBasesError(message || 'Could not load knowledge bases.'); - } finally { - setLoading(false); - } - }, []); + await Promise.all([refreshBases(), resyncSelectionRef.current()]); + }, [refreshBases]); const registerGraphRefresh = useCallback((fn: (() => Promise) | null) => { graphRefreshRef.current = fn; @@ -383,30 +497,25 @@ export function KnowledgeProvider({ }, []); useEffect(() => { - void refresh(); - }, [refresh]); - - useEffect(() => { - // A primary that names a base which no longer exists is cleared, not - // promoted — deleting a base is destructive, so re-pointing the write - // target at an unrelated one is the wrong default (D2). - if (basesLoaded && primaryKbId && !bases.some((b) => b.id === primaryKbId)) { - setPrimaryKbId(null); - } - }, [basesLoaded, primaryKbId, bases, setPrimaryKbId]); + // The list only: the hydrate below owns the selection on mount, and a + // second read racing it would be answered by the same daemon twice. + void refreshBases(); + }, [refreshBases]); useEffect(() => { - // Drop ids naming bases that no longer exist — but only once a list has - // actually arrived. Pruning against the empty list this starts out with - // would persist an empty set on every mount, erasing the session's working - // set before the daemon had said a word about which bases exist. - if (!basesLoaded) return; - const validIds = new Set(bases.map((base) => base.id)); - const nextHiddenKbIds = hiddenKbIds.filter((id) => validIds.has(id)); - if (nextHiddenKbIds.length !== hiddenKbIds.length) { - setHiddenKbIds(nextHiddenKbIds); - } - }, [basesLoaded, bases, hiddenKbIds, setHiddenKbIds]); + // QA 2026-09-10 F6. The agent creates, deletes and merges knowledge bases in + // a chat — directly, or from inside `execute_code`, where no knowledge tool + // call is visible to the renderer at all — and before this nothing told the + // composer's chip, which read "2 visible" over three bases until a remount. + // `message-stream-finished` is the app's existing end-of-turn signal: the + // sidebar, the extension chip and the tool count already re-read on it, so + // this is one more reader of a signal rather than a second subscription. + // Mounted with the provider, once per renderer — a subscription belongs to a + // mount, not to a lookup. + const onTurnFinished = () => void refresh(); + window.addEventListener('message-stream-finished', onTurnFinished); + return () => window.removeEventListener('message-stream-finished', onTurnFinished); + }, [refresh]); useEffect(() => { const local = localStorage.getItem(storageKey); @@ -436,20 +545,10 @@ export function KnowledgeProvider({ void (async () => { 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. 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, - }); + const data = await fetchKnowledgeSelection(sessionId); if (cancelled || generation !== selectionGenerationRef.current) return; - applyPrimary(readPrimary(res.data)); - applyHidden(readHidden(res.data) ?? []); + applyPrimary(readPrimary(data)); + applyHidden(readHidden(data) ?? []); } catch (err) { if (cancelled) return; // ⚠ One quiet line, deliberately: the gate's refusal is ~900 @@ -477,6 +576,16 @@ export function KnowledgeProvider({ }; }, [applyHidden, applyPrimary, hiddenStorageKey, sessionId, storageKey]); + // A pointer at a base the list does not hold is shown as no primary — the + // view, the ingest target and the graph must never aim at a base that is gone + // — but it is NOT written back as one. The daemon already cleared every + // pointer the delete touched (D2), and a durable "no primary" derived from + // this renderer's list would override a chat that merely inherits, from a + // list that may be the stale one. `refresh` re-reads the truth instead. + const primaryKbId = + basesLoaded && storedPrimaryKbId && !bases.some((b) => b.id === storedPrimaryKbId) + ? null + : storedPrimaryKbId; const primaryKb = useMemo( () => bases.find((b) => b.id === primaryKbId) ?? null, [bases, primaryKbId] diff --git a/ui/desktop/src/components/knowledge/changelog/ChangeLogDrawer.history.test.tsx b/ui/desktop/src/components/knowledge/changelog/ChangeLogDrawer.history.test.tsx new file mode 100644 index 000000000..afd0868c3 --- /dev/null +++ b/ui/desktop/src/components/knowledge/changelog/ChangeLogDrawer.history.test.tsx @@ -0,0 +1,141 @@ +import { act, render, screen, waitFor, within } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import type { HistoryEntry } from '../../../api/types.gen'; +import { ChangeLogDrawer } from './ChangeLogDrawer'; + +/** + * QA 2026-09-10 F13: after digesting a paragraph into a new base the Change log + * listed only `create knowledge base …`, while `git log` held the two `[ingest]` + * commits that wrote every page. The history route reads git and was right; the + * drawer — mounted with the Knowledge view and merely hidden — had read it once, + * before the digest, and never again. + * + * `ChangeLogDrawer.test.tsx` stubs `useHistory` to pin the drawer's markup; this + * file runs the real hook over a mocked route, because the defect was in WHEN + * the hook reads. + */ + +const mocks = vi.hoisted(() => ({ + listHistory: vi.fn(), + restoreState: vi.fn(), + primaryKbId: 'probe' as string | null, +})); + +vi.mock('../../../api', () => ({ + listHistory: mocks.listHistory, + restoreState: mocks.restoreState, +})); + +vi.mock('../KnowledgeContext', () => ({ + useKnowledge: () => ({ primaryKbId: mocks.primaryKbId, triggerGraphRefresh: vi.fn() }), +})); + +vi.mock('../../../toasts', () => ({ toastError: vi.fn() })); + +function commit(sha: string, kind: HistoryEntry['kind'], summary: string): HistoryEntry { + return { commit_sha: sha.padEnd(40, '0'), kind, summary, timestamp: '2026-09-10T19:30:00Z' }; +} + +const CREATED = commit('2dc4141', 'manual', 'create knowledge base probe'); +const INGESTED_SOURCE = commit('0058ad9', 'ingest', 'ingested pasted-knowledge-b2c5a1'); +const INGESTED_PAGES = commit('6f5b82e', 'ingest', 'ingest pasted-knowledge-b2c5a1'); + +/** What `git log` in the base says, newest first. Tests move it. */ +let gitLog: HistoryEntry[] = []; + +function deferred() { + let resolve!: (value: T) => void; + const promise = new Promise((res) => { + resolve = res; + }); + return { promise, resolve }; +} + +beforeEach(() => { + vi.clearAllMocks(); + mocks.primaryKbId = 'probe'; + gitLog = [CREATED]; + mocks.listHistory.mockImplementation(() => Promise.resolve({ data: [...gitLog] })); +}); + +function drawer(open: boolean) { + return ( + undefined} + onPreview={() => undefined} + onRestored={() => undefined} + /> + ); +} + +function listedSummaries() { + return within(screen.getByRole('dialog')) + .queryAllByText(/knowledge base probe|pasted-knowledge/) + .map((node) => node.textContent); +} + +describe('ChangeLogDrawer — what it lists is what git holds', () => { + it('lists the ingest commits a digest made while the drawer was shut', async () => { + // The view mounts with the drawer shut; the base is brand new. + const { rerender } = render(drawer(false)); + + // A digest lands: two `[ingest]` commits on top of the create. + gitLog = [INGESTED_PAGES, INGESTED_SOURCE, CREATED]; + + rerender(drawer(true)); + + await waitFor(() => + expect(listedSummaries()).toEqual([ + 'ingest pasted-knowledge-b2c5a1', + 'ingested pasted-knowledge-b2c5a1', + 'create knowledge base probe', + ]) + ); + }); + + it('reads again every time it is opened', async () => { + const { rerender } = render(drawer(true)); + await waitFor(() => expect(listedSummaries()).toEqual(['create knowledge base probe'])); + + rerender(drawer(false)); + gitLog = [INGESTED_PAGES, CREATED]; + rerender(drawer(true)); + + await waitFor(() => + expect(listedSummaries()).toEqual([ + 'ingest pasted-knowledge-b2c5a1', + 'create knowledge base probe', + ]) + ); + }); + + // Nothing to show while shut, so nothing is read while shut. + it('does not read while it is shut', async () => { + render(drawer(false)); + await Promise.resolve(); + expect(mocks.listHistory).not.toHaveBeenCalled(); + }); + + // The subject can change while a read is out. The older base's answer must + // not land over the newer one's. + it("never shows another base's history that answered late", async () => { + const slow = deferred<{ data: HistoryEntry[] }>(); + mocks.listHistory.mockImplementationOnce(() => slow.promise); + const { rerender } = render(drawer(true)); + + mocks.primaryKbId = 'other'; + const otherLog = [commit('aaaaaaa', 'manual', 'create knowledge base probe-two')]; + mocks.listHistory.mockImplementation(() => Promise.resolve({ data: otherLog })); + rerender(drawer(true)); + await waitFor(() => expect(listedSummaries()).toEqual(['create knowledge base probe-two'])); + + // Let the late answer land completely — resolution, state update, render. + await act(async () => { + slow.resolve({ data: [INGESTED_PAGES, INGESTED_SOURCE, CREATED] }); + await slow.promise; + await new Promise((settle) => setTimeout(settle, 0)); + }); + expect(listedSummaries()).toEqual(['create knowledge base probe-two']); + }); +}); diff --git a/ui/desktop/src/components/knowledge/changelog/ChangeLogDrawer.tsx b/ui/desktop/src/components/knowledge/changelog/ChangeLogDrawer.tsx index 01fa3c017..a8fa48d02 100644 --- a/ui/desktop/src/components/knowledge/changelog/ChangeLogDrawer.tsx +++ b/ui/desktop/src/components/knowledge/changelog/ChangeLogDrawer.tsx @@ -55,7 +55,9 @@ function relativeTime(iso: string): string { */ export function ChangeLogDrawer({ open, onOpenChange, onPreview, onRestored }: Props) { const { primaryKbId, triggerGraphRefresh } = useKnowledge(); - const { history, loading, error, restore } = useHistory(primaryKbId); + // Read on every open, not once at mount: the drawer stays mounted, hidden, + // for as long as the Knowledge view is, and a digest lands while it is shut. + const { history, loading, error, restore } = useHistory(primaryKbId, open); const [activeKinds, setActiveKinds] = useState>(new Set(ALL_KINDS)); const [restoring, setRestoring] = useState(null); const [entryToRestore, setEntryToRestore] = useState(null); diff --git a/ui/desktop/src/components/knowledge/hooks/useHistory.ts b/ui/desktop/src/components/knowledge/hooks/useHistory.ts index 0d569d1f2..c2042045a 100644 --- a/ui/desktop/src/components/knowledge/hooks/useHistory.ts +++ b/ui/desktop/src/components/knowledge/hooks/useHistory.ts @@ -1,5 +1,6 @@ -import { useCallback, useEffect, useState } from 'react'; +import { useCallback, useEffect, useRef, useState } from 'react'; import { listHistory, restoreState } from '../../../api'; +import { userActionHeaders } from '../../../utils/userAction'; import type { HistoryEntry, RestoreResponse } from '../../../api/types.gen'; export interface UseHistoryResult { @@ -10,14 +11,34 @@ export interface UseHistoryResult { restore: (commitSha: string) => Promise; // returns new commit sha } -export function useHistory(kbId: string | null): UseHistoryResult { +/** + * A knowledge base's commit history, as the Change log shows it. + * + * @param enabled read only while this is true, and read AGAIN every time it + * turns true. The Change log passes its `open` state. + * + * ⚠ **It used to read once per base, at mount, and never again.** The drawer + * is mounted with the Knowledge view and merely hidden, so its one read ran + * when the base became the subject — before anything had been digested into + * it — and every ingest, lint, merge and restore after that was invisible until + * the base changed. QA (2026-09-10 F13) digested a paragraph into a new base and + * the log listed only `create knowledge base`, while `git log` held the two + * `[ingest]` commits that wrote all ten pages. The route was never wrong: it + * reads git. What the drawer showed was simply old. + */ +export function useHistory(kbId: string | null, enabled = true): UseHistoryResult { const [history, setHistory] = useState([]); const [loading, setLoading] = useState(false); const [error, setError] = useState(null); + // Only the newest read may land. Reopening the log, or the subject changing + // while a read is out, must not let an older answer — another base's — win. + const requestRef = useRef(0); const refresh = useCallback(async () => { + const request = ++requestRef.current; if (!kbId) { setHistory([]); + setLoading(false); return; } setLoading(true); @@ -26,17 +47,21 @@ export function useHistory(kbId: string | null): UseHistoryResult { const res = await listHistory({ path: { id: kbId }, query: { limit: 200 }, + // The Knowledge view is the person at the keyboard, and says so the way + // every other Knowledge request does (`KnowledgeContext.readSelection`). + headers: await userActionHeaders(), throwOnError: true, }); + if (request !== requestRef.current) return; // ListHistoryResponses[200] is typed `unknown` in the generated SDK, // but the route returns `Vec`. - const data = (res.data ?? []) as HistoryEntry[]; - setHistory(data); + setHistory((res.data ?? []) as HistoryEntry[]); } catch (err) { + if (request !== requestRef.current) return; setError(err instanceof Error ? err.message : String(err)); setHistory([]); } finally { - setLoading(false); + if (request === requestRef.current) setLoading(false); } }, [kbId]); @@ -56,8 +81,9 @@ export function useHistory(kbId: string | null): UseHistoryResult { ); useEffect(() => { + if (!enabled) return; void refresh(); - }, [refresh]); + }, [enabled, refresh]); return { history, loading, error, refresh, restore }; } diff --git a/ui/desktop/src/components/knowledge/hooks/useKnowledgeBases.ts b/ui/desktop/src/components/knowledge/hooks/useKnowledgeBases.ts index b9be11a26..76d0a796f 100644 --- a/ui/desktop/src/components/knowledge/hooks/useKnowledgeBases.ts +++ b/ui/desktop/src/components/knowledge/hooks/useKnowledgeBases.ts @@ -5,7 +5,7 @@ import type { KbFormat, Manifest } from '../../../api/types.gen'; import { knowledgeFetch } from './knowledgeRequest'; export function useKnowledgeBases() { - const { refresh, setPrimaryKbId, primaryKbId } = useKnowledge(); + const { refresh, setPrimaryKbId } = useKnowledge(); /** * Create a base. @@ -60,13 +60,24 @@ export function useKnowledgeBases() { [refresh] ); + /** + * Delete a base, then read back what the daemon made of the selection. + * + * ⚠ **No `setPrimaryKbId(null)` here, and there must not be one.** The delete + * itself is the repair (D2): the daemon clears every pointer that named the + * base — the machine default and each chat that had pinned it — to the + * explicit "no primary", and leaves a chat that merely inherited following the + * default. Writing `clear_primary` from here on top of that installed a + * durable "this chat has no primary" in a chat that never pinned the base, + * from a pointer this renderer may only have had cached (QA 2026-09-10 F14). + * `refresh` re-reads both the list and the selection. + */ const remove = useCallback( async (id: string): Promise => { await apiDelete({ throwOnError: true, path: { id } }); - if (primaryKbId === id) setPrimaryKbId(null); await refresh(); }, - [refresh, primaryKbId, setPrimaryKbId] + [refresh] ); const exportArchive = useCallback(async (id: string, name: string): Promise => { diff --git a/ui/desktop/src/components/knowledge/knowledgeSelection.ts b/ui/desktop/src/components/knowledge/knowledgeSelection.ts index 299f3ff8e..2c0180173 100644 --- a/ui/desktop/src/components/knowledge/knowledgeSelection.ts +++ b/ui/desktop/src/components/knowledge/knowledgeSelection.ts @@ -3,7 +3,7 @@ import { userActionHeaders } from '../../utils/userAction'; import { briefSelectionFailure } from './selectionWarning'; /** The shape both selection endpoints answer with — GET /active and POST /active. */ -type SelectionPayload = +export type SelectionPayload = | { primary_kb?: string | null; active_kb?: string | null; hidden_kbs?: string[] | null } | undefined; @@ -28,6 +28,40 @@ export interface KnowledgeSelection { hiddenKbIds: ReadonlySet; } +/** + * THE request for a knowledge-base selection: the chat's when `sessionId` names + * one, the machine-wide default otherwise, read as the person at the keyboard. + * Resolves to the daemon's answer and REJECTS when there is none, so a caller + * can never mistake a failure for a selection. + * + * ⚠ **Every selection read in the renderer goes through here**: the + * `KnowledgeProvider` hydrate, its re-reads and its machine-default read, and + * `readKnowledgeSelection` below for the `/` palette and the create-workflow + * modal. Issue #56 Task 58: `GET /knowledge/active` naming a PRIVATE chat is on + * the reach gate's list exactly as the POST is, and the desktop gets through it + * the only way it can — `userActionHeaders()`, the one helper that decides how + * this surface proves a person. The reads used to go without it, and on a UCSF + * install, where every chat is private, the daemon refused every one: the + * Knowledge view, the chip and the ingest target then showed the renderer's + * cache as the chat's selection (QA 2026-09-10 F14). The proof reaches nothing + * new; it already reads the chat's whole transcript through `getSession`. + * + * It carries the proof at machine scope too, where the gate is inert today, so + * that a daemon which filters what an unproven caller may see never hands this + * surface a selection with the user's own private bases missing from it. + */ +export async function fetchKnowledgeSelection( + sessionId: string | null | undefined +): Promise> { + const res = await getActive({ + query: sessionId ? { session_id: sessionId } : undefined, + headers: await userActionHeaders(), + throwOnError: false, + }); + if (!res.data) throw res.error ?? new Error('The daemon answered without a selection.'); + return res.data; +} + /** * Read the selection of the chat `sessionId` names, or the machine-wide one * when it names none, for a surface that shows the selection or saves it. @@ -52,21 +86,10 @@ export async function readKnowledgeSelection( sessionId: string | null | undefined ): Promise { 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, and the desktop gets through it by proving the person — as the - // hydrate in `KnowledgeContext` does, and as `setActive` always has. - headers: await userActionHeaders(), - throwOnError: false, - }); - if (!res.data) { - console.warn('Knowledge selection not read:', briefSelectionFailure(res.error)); - return null; - } + const data = await fetchKnowledgeSelection(sessionId); return { - primaryKbId: readPrimary(res.data), - hiddenKbIds: new Set(readHidden(res.data) ?? []), + primaryKbId: readPrimary(data), + hiddenKbIds: new Set(readHidden(data) ?? []), }; } catch (err) { console.warn('Knowledge selection not read:', briefSelectionFailure(err));