From 4522e5b1cf393d6f8b0fd742c6c06857e9f2da4d Mon Sep 17 00:00:00 2001 From: Shay Date: Mon, 15 Jun 2026 01:05:50 -0700 Subject: [PATCH] workbench(vault): frame empty and unavailable vault states honestly (#760) --- .../src/app/routeConformance.test.tsx | 10 +- .../src/app/vault/VaultRoute.test.tsx | 51 ++++++- workbench-ui/src/app/vault/VaultRoute.tsx | 139 +++++++++++++----- .../design/components/states/EmptyState.tsx | 8 +- .../design/components/states/states.test.tsx | 9 +- 5 files changed, 168 insertions(+), 49 deletions(-) diff --git a/workbench-ui/src/app/routeConformance.test.tsx b/workbench-ui/src/app/routeConformance.test.tsx index 2391d610..a6839912 100644 --- a/workbench-ui/src/app/routeConformance.test.tsx +++ b/workbench-ui/src/app/routeConformance.test.tsx @@ -23,7 +23,7 @@ import { } from "./lived-life/LivedLifeRoute"; import { PacksRoute } from "./packs/PacksRoute"; import { LogosRoute } from "./logos/LogosRoute"; -import { VaultRoute } from "./vault/VaultRoute"; +import { VaultRoute, VAULT_ABSENCE_STATEMENT, VAULT_ABSENCE_ACTION } from "./vault/VaultRoute"; import { CalibrationRoute } from "./calibration/CalibrationRoute"; import { SettingsRoute } from "./settings/SettingsRoute"; @@ -227,10 +227,10 @@ const MOUNT_ROUTES: MountRouteSpec[] = [ path: "/vault", initialEntry: "/vault", loadingLabel: "Loading vault...", - // Fail-closed: absence of a persisted vault is the primary contract. - emptyStatement: - "No persisted vault. Session memory is held in-process and discarded on exit; persistence is opt-in via RuntimeConfig.persist_session_state.", - emptyCommand: "Set RuntimeConfig.persist_session_state = true", + // Fail-closed: absence of a persisted vault snapshot is the primary contract. + // The next action is the real daemon command, not a fake config-flag button. + emptyStatement: VAULT_ABSENCE_STATEMENT, + emptyCommand: VAULT_ABSENCE_ACTION, }, { name: "Calibration", diff --git a/workbench-ui/src/app/vault/VaultRoute.test.tsx b/workbench-ui/src/app/vault/VaultRoute.test.tsx index 17bc9fa3..e14487e5 100644 --- a/workbench-ui/src/app/vault/VaultRoute.test.tsx +++ b/workbench-ui/src/app/vault/VaultRoute.test.tsx @@ -96,6 +96,26 @@ function stubVault({ persisted = true }: { persisted?: boolean } = {}) { ); } +// A persisted snapshot that holds zero entries — distinct from absence. The +// entries query stays disabled (entry_count === 0), so /vault/entries is never +// hit; an unexpected hit fails the stub loudly. +function stubVaultPersistedEmpty() { + vi.stubGlobal( + "fetch", + vi.fn((input: unknown) => { + const path = new URL(String(input)).pathname; + if (path === "/vault/summary") { + return Promise.resolve({ + json: () => Promise.resolve(okBody({ ...summary, entry_count: 0 })), + }); + } + return Promise.resolve({ + json: () => Promise.resolve(evidenceUnavailable(`unexpected ${path}`)), + }); + }), + ); +} + const offsetDescriptors = { offsetHeight: Object.getOwnPropertyDescriptor(HTMLElement.prototype, "offsetHeight"), offsetWidth: Object.getOwnPropertyDescriptor(HTMLElement.prototype, "offsetWidth"), @@ -124,21 +144,40 @@ describe("VaultRoute", () => { localStorage.clear(); }); - it("fail-closed: an unpersisted vault renders the honest absence card, not an error", async () => { + it("fail-closed: an absent vault renders the honest absence card inside Vault chrome, not an error", async () => { stubVault({ persisted: false }); renderRoute(); expect( - await screen.findByText(/No persisted vault\./), + await screen.findByText(/No persisted vault snapshot is available\./), ).toBeInTheDocument(); - // both the statement and the config-pointer action name the opt-in flag - expect( - screen.getAllByText(/RuntimeConfig\.persist_session_state/).length, - ).toBeGreaterThan(0); + // framed as the Vault route, not a context-free card floating in a blank + // surface (the "nothing comes up" symptom) + expect(screen.getByRole("heading", { name: "Vault" })).toBeInTheDocument(); + // the statement still names the opt-in flag + expect(screen.getByText(/RuntimeConfig\.persist_session_state/)).toBeInTheDocument(); + // the next action is a real, runnable command (copyable), not a dead button + expect(screen.getByText("core always-on")).toBeInTheDocument(); + expect(screen.getByRole("button", { name: /copy/i })).toBeInTheDocument(); // it is NOT the generic error contract expect(screen.queryByText("What failed")).not.toBeInTheDocument(); }); + it("persisted-but-empty: distinguishes 'no entries yet' from absence, still framed as Vault", async () => { + stubVaultPersistedEmpty(); + renderRoute(); + + expect( + await screen.findByText(/Vault snapshot exists, but no entries have been stored yet\./), + ).toBeInTheDocument(); + expect(screen.getByRole("heading", { name: "Vault" })).toBeInTheDocument(); + // it is a DIFFERENT statement from absence, and not the generic error + expect( + screen.queryByText(/No persisted vault snapshot is available\./), + ).not.toBeInTheDocument(); + expect(screen.queryByText("What failed")).not.toBeInTheDocument(); + }); + it("renders the summary strip and entries when persisted", async () => { stubVault(); renderRoute(); diff --git a/workbench-ui/src/app/vault/VaultRoute.tsx b/workbench-ui/src/app/vault/VaultRoute.tsx index c9c7af15..290263c4 100644 --- a/workbench-ui/src/app/vault/VaultRoute.tsx +++ b/workbench-ui/src/app/vault/VaultRoute.tsx @@ -12,11 +12,30 @@ import { LoadingState } from "../../design/components/states/LoadingState"; import type { VaultEntry, VaultSummary } from "../../types/api"; import { useEvidenceSubject } from "../evidenceContext"; -// Vault persistence is opt-in; absence is the common, honest state. This is -// the fail-closed primary contract, not an error. -const FAIL_CLOSED_STATEMENT = - "No persisted vault. Session memory is held in-process and discarded on exit; persistence is opt-in via RuntimeConfig.persist_session_state."; -const FAIL_CLOSED_ACTION = "Set RuntimeConfig.persist_session_state = true"; +// Vault persistence is opt-in; absence of a snapshot is the common, honest +// primary state — the fail-closed contract, not an error. +// +// The next action must be TRUE and runnable: `core always-on` is the daemon +// that forces persist_session_state=True (chat/always_on_daemon.py) and writes +// engine_state/session_state.json. There is no `core chat --persist-session-state` +// flag today, so we do not invent one — that would be the same dishonesty as a +// dead button. Exported so route-conformance asserts the exact contract string. +export const VAULT_ABSENCE_STATEMENT = + "No persisted vault snapshot is available. Session memory is held in-process " + + "and discarded on exit; persistence is opt-in (RuntimeConfig.persist_session_state). " + + "The always-on daemon writes the snapshot."; +export const VAULT_ABSENCE_ACTION = "core always-on"; +const VAULT_PERSIST_CLI = { kind: "cli", command: VAULT_ABSENCE_ACTION } as const; + +// A persisted snapshot that holds zero entries is a DIFFERENT honest state from +// absence: the vault exists, it just has not stored anything yet. +const VAULT_EMPTY_STATEMENT = + "Vault snapshot exists, but no entries have been stored yet."; + +// Filtering is the operator's own action; the remedy is to relax the filter, +// not to enable persistence. Static guidance, not a command. +const VAULT_FILTER_EMPTY_STATEMENT = "No vault entries match this filter."; +const VAULT_FILTER_EMPTY_ACTION = "Clear the filter to see all entries."; function errorMessage(error: unknown) { return error instanceof WorkbenchApiError ? error.message : "Vault request failed."; @@ -103,9 +122,29 @@ function VaultSummaryStrip({ summary }: { summary: VaultSummary }) { ); } -function FailClosed() { +// Route identity must survive every data state. The success path was the only +// branch that painted "Vault" chrome, so loading/absent/empty/error read as a +// context-free card floating in a blank surface ("nothing comes up"). Wrapping +// every branch in the same Panel keeps the route legible regardless of state. +function VaultFrame({ + toolbar, + children, +}: { + toolbar?: React.ReactNode; + children: React.ReactNode; +}) { return ( - + + {children} + + ); +} + +function EntryCountToolbar({ count }: { count: number }) { + return ( + + {count} entries + ); } @@ -148,37 +187,69 @@ export function VaultRoute() { } if (summaryQuery.isLoading) { - return ; - } - - // Fail-closed: no persisted vault is the expected primary state, not an error. - if (summaryQuery.isError) { - if (summaryQuery.error.code === "evidence_unavailable") { - return ; - } return ( - + + + ); } - if (!hasEntries) { - return ; + // Fail-closed: no persisted vault snapshot is the expected primary state, not + // an error. The summary reader returns evidence_unavailable when the snapshot + // is absent (persistence is opt-in). + if (summaryQuery.isError) { + if (summaryQuery.error.code === "evidence_unavailable") { + return ( + + + + ); + } + return ( + + + + ); + } + + // react-query guarantees data is present once the query is neither loading + // nor errored, but the `summary` alias was captured before that narrowing. + // Guard explicitly — a missing summary is treated as honest absence, framed. + if (!summary) { + return ( + + + + ); + } + + // summary is defined past this point. A snapshot whose `persisted` flag is not + // set is treated as absence — the same honest fail-closed card. + if (!summary.persisted) { + return ( + + + + ); + } + + // A persisted snapshot with zero entries is a DISTINCT state from absence: the + // vault exists, it just has not stored anything yet. + if (summary.entry_count === 0) { + return ( + }> + + + ); } return ( - - {summary.entry_count} entries - - } - > + }>
) : filteredEntries.length === 0 ? ( ) : ( )}
- + ); } diff --git a/workbench-ui/src/design/components/states/EmptyState.tsx b/workbench-ui/src/design/components/states/EmptyState.tsx index 1b8d7f24..f6b8256f 100644 --- a/workbench-ui/src/design/components/states/EmptyState.tsx +++ b/workbench-ui/src/design/components/states/EmptyState.tsx @@ -33,9 +33,13 @@ export function EmptyState({

{statement}

{typeof nextAction === "string" ? ( - +

) : (
diff --git a/workbench-ui/src/design/components/states/states.test.tsx b/workbench-ui/src/design/components/states/states.test.tsx index 484e8e15..4ef22d57 100644 --- a/workbench-ui/src/design/components/states/states.test.tsx +++ b/workbench-ui/src/design/components/states/states.test.tsx @@ -6,10 +6,15 @@ import { ErrorState } from "./ErrorState"; import { LoadingState } from "./LoadingState"; describe("state components", () => { - it("requires empty-state statement and next action", () => { + it("renders a string nextAction as static guidance, not a dead button", () => { render(); expect(screen.getByText("No trace selected.")).toBeInTheDocument(); - expect(screen.getByRole("button", { name: "Select a trace." })).toBeInTheDocument(); + // a plain-string action is guidance, not an interactive control — a + // click-dead button reads as "I clicked and nothing happened" + expect(screen.getByText("Select a trace.")).toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: "Select a trace." }), + ).not.toBeInTheDocument(); }); it("renders cli-form nextAction as mono command row with copy button", async () => {