fix: the full-codebase review findings — data loss, a CSRF boundary, secrets in the graph, and 19 root causes

CHANGELOG.md

@@ -4,6 +4,86 @@ All notable changes to MeMesh are documented here.
 
 ## [Unreleased]
 
+### Removed
+
+- **`user_patterns` no longer reports `toolPreferences` or
+  `workflow.avgSessionMinutes`.** Both were parsed out of observation text in
+  a format nothing has ever written: `patterns.ts` looked for observations
+  beginning `[FOCUS]` containing `Top tools: …`, and for `[SESSION]` lines
+  containing `Duration: Nm`. Neither string occurs anywhere else in the
+  repository, and `signal-scorer.ts` — which reads the one `[SESSION]` shape
+  that did exist — shows it recorded **seconds**, not minutes. So
+  `toolPreferences` was permanently `[]` and `avgSessionMinutes` permanently
+  `0`, on the MCP tool output, the dashboard's Analytics tab, and in the
+  documented response shape. The dashboard rendered the tool list only when
+  non-empty (never) and the session figure as `—` (always).
+
+  They are removed rather than implemented. The test for them was
+  `expect(result.toolPreferences).toEqual([])` — a test asserting the
+  deadness. `commitsPerSession`, `totalSessions` and `totalCommits` are
+  computed from real rows and are unchanged.
+
+  If you passed `"toolPreferences"` in `categories`, that value is no longer
+  accepted; it previously returned an empty list.
+
+- **`DEFAULT_SIGNAL_THRESHOLD` is gone.** Its docstring described a dashboard
+  filter users could override in Settings. Nothing imported it but its own
+  test, and no such filter exists. The signal score itself is real and widely
+  used — the dreamer's compactable range, `kg-backfill`'s Rule 3 floor, the
+  briefing — and is unchanged.
+
+### Changed
+
+- **`memesh export` now tells you when the bundle is only part of your
+  graph.** The `--limit` default is 1000 and always has been, but nothing
+  said so: on a real graph of 1272 memories the command printed
+  `✅ Exported 1000 entities` and produced a bundle missing 21% of what it was
+  taken to preserve. `entity_count` could not distinguish that from a graph
+  that happens to be exactly 1000. The result now carries `truncated`
+  (`true`/`false`) — visible to the MCP and HTTP callers as well as the CLI —
+  and the CLI prints a warning on **stderr**, so `memesh export > b.json`
+  still writes a clean bundle. For a full backup, pass a limit above your
+  graph size: `memesh export --limit 100000 -o backup.json`.
+
+- **`memesh import` no longer exits 1 for a relation that points outside the
+  bundle.** *(Behaviour change — scripts that check the exit code are
+  affected.)* Those relations are real information loss and are still
+  reported, by name, in a new `skipped_relations` field and on stderr. But
+  they are not an error: every bundle narrowed by `--tag`, `--namespace` or
+  `--limit` has them, so counting them as errors made
+  `memesh export > b.json && memesh import b.json` — the round trip this
+  project's own help text recommends — a failing command on a restore that
+  did exactly what it should. Measured: a full backup of a 1272-memory graph
+  restored 1000 entities and 142 of 151 relations, and exited 1. `errors`
+  still means an entry that genuinely failed, and still exits 1.
+
+- **The export bundle is version `3.1.0`, and it is now actually a backup.**
+  Three things a restore needs were missing from it, and a fourth was thrown
+  away on the way back in:
+
+  - `created_at` was not exported, so a restore stamped every memory with the
+    day of the restore. That is not cosmetic: creation time drives recency in
+    ranking, the dreamer's weekly clustering, `memesh why`, and every "what was
+    I doing then" question. Restored only for entities the import creates, and
+    only when `parseSqliteUtcMs` can read the value.
+  - **Archived entities were skipped**, so `memesh forget` followed by an
+    export and a restore brought the memory back to life.
+  - `metadata` was not exported, losing `signal_score`, `task_state`, the demo
+    marker and provenance. It round-trips now, minus `guard` — that field
+    controls what memesh warns about on your tool calls, and a bundle you were
+    sent must be able to bring memories, not to change what memesh does.
+  - **Relations were dropped on import.** They were created inside the
+    per-entity loop and skipped when the target "may not have been imported
+    yet". That is not an edge case: `export` writes newest-first and relations
+    point newer → older, so the target was almost always still further down the
+    file. A backup of a graph with relations restored with none of them and
+    reported success. They are created in a second pass now, after every entity
+    exists, and one that still cannot be created — a target genuinely outside
+    the bundle — is reported in `errors` instead of swallowed.
+
+  Bundles written by earlier versions import unchanged; every added field is
+  optional.
+
 ### Fixed
 
 - **`memesh doctor` no longer counts the Install ID row as something it

dashboard/src/components/AnalyticsTab.tsx

@@ -77,11 +77,9 @@ export function isPatternsRenderable(p: PatternsData | null): p is PatternsData
   return (
     Array.isArray(p?.workSchedule?.hourDistribution) &&
     Array.isArray(p.workSchedule?.dayDistribution) &&
-    Array.isArray(p.toolPreferences) &&
     Array.isArray(p.focusAreas) &&
     Array.isArray(p.strengths) &&
     Array.isArray(p.learningAreas) &&
-    typeof p.workflow?.avgSessionMinutes === 'number' &&
     typeof p.workflow?.totalSessions === 'number' &&
     typeof p.workflow?.commitsPerSession === 'number'
   );

dashboard/src/components/GraphTab.tsx

@@ -1506,11 +1506,43 @@ export function GraphTab() {
   // Search match count — over the SAME haystack the canvas highlights on
   // (name + headline). Counted over the machine name alone it reported "0
   // matches" for a query that the user took straight off this canvas.
-  const matchCount = searchQuery
-    ? [...displayIndex.values()].filter((d) =>
-        d.search.includes(searchQuery.toLowerCase()),
-      ).length
-    : 0;
+  // Memoized: this scans every node, and it used to run on every render with
+  // `searchQuery.toLowerCase()` recomputed per node.
+  const searchMatches = useMemo(() => {
+    if (!searchQuery) return [];
+    const q = searchQuery.toLowerCase();
+    return [...displayIndex.entries()].filter(([, d]) => d.search.includes(q));
+  }, [displayIndex, searchQuery]);
+  const matchCount = searchMatches.length;
+
+  /**
+   * Select the node the search has narrowed to, from the keyboard.
+   *
+   * Node selection was pointer-only: the click handler was the ONLY way to
+   * enter ego mode or open the evidence drill-down, so a keyboard user could
+   * reach the canvas (it is focusable and labelled) and read the summary,
+   * and could not open a single node. Search highlighted matches and stopped
+   * there.
+   *
+   * Deliberately narrow: only when the query has narrowed to exactly ONE
+   * node, and driven from the search box the user is already typing in. Full
+   * node-to-node traversal is still deferred — it is a large-graph
+   * interaction that needs its own design, as the canvas comment says — but
+   * "find it and open it" is the thing the mouse does that the keyboard
+   * could not do at all.
+   */
+  const selectSoleMatch = () => {
+    if (searchMatches.length !== 1) return;
+    const [name] = searchMatches[0];
+    setEgoNodeId(name);
+    // The REAL node, not a two-field stand-in. `{ id, display } as GNode`
+    // left twelve of fourteen fields undefined on a node reached by keyboard
+    // and populated on the same node reached by click — nothing reads them
+    // today, and the next reader of `evidenceNode` would get `undefined` with
+    // no type error to warn them.
+    const node = nodesRef.current.find((n) => n.id === name);
+    if (node) setEvidenceNode(node);
+  };
 
   // Ego node name for banner
   const egoEntity = egoNodeId
@@ -1697,6 +1729,8 @@ export function GraphTab() {
             placeholder={t('graph.search')}
             value={searchQuery}
             onInput={(e) => setSearchQuery((e.target as HTMLInputElement).value)}
+            onKeyDown={(e) => { if (e.key === 'Enter') selectSoleMatch(); }}
+            aria-describedby="graph-search-hint"
             style={{
               flex: '1 1 160px',
               minWidth: 120,
@@ -1712,13 +1746,18 @@ export function GraphTab() {
           />
           {searchQuery && (
             <span
+              id="graph-search-hint"
+              role="status"
               style={{
                 fontSize: 11,
                 fontFamily: 'var(--mono)',
                 color: 'var(--text-2)',
               }}
             >
               {matchCount} {t('graph.matches')}
+              {/* The hint appears exactly when the action is available, so it
+                  never promises something Enter will not do. */}
+              {matchCount === 1 && ` — ${t('graph.enterToOpen')}`}
             </span>
           )}
           <button

dashboard/src/components/InsightsTab.tsx

@@ -233,7 +233,15 @@ export function InsightsTab() {
     }
   }, [refresh]);
 
+  // Confirmed, because rejection is one click and permanent. The dreamer
+  // deliberately never re-proposes a rejected cluster (dreamer.ts:226) — that
+  // is what the status is FOR — and there is no un-reject on any surface. So
+  // a mis-click on a ghost button destroys a digest the user paid an LLM call
+  // for, with nothing to undo it. The sibling irreversible action in this
+  // dashboard, `OnboardingBanner.runReset`, already confirms; accept does not
+  // and should not, because an accepted memory can be forgotten.
   const reject = useCallback(async (id: number) => {
+    if (!confirm(t('insights.rejectConfirm'))) return;
     markBusy(id);
     try {
       await api('POST', `/v1/dream/proposals/${id}/reject`, { reason: 'rejected via dashboard' });

dashboard/src/components/PmAnalyticsPanel.tsx

@@ -1,6 +1,7 @@
 import { useState, useEffect } from 'preact/hooks';
 import { api } from '../lib/api';
 import { t } from '../lib/i18n';
+import { classifyLoadError, failureMessage, type LoadFailure } from '../lib/failure';
 
 interface PmAnalytics {
   velocity: { decisionsPerWeek: number; releasesPerMonth: number; windowDays: number };
@@ -21,7 +22,21 @@ export function isPmAnalyticsRenderable(d: PmAnalytics | null): d is PmAnalytics
 
 export function PmAnalyticsPanel() {
   const [data, setData] = useState<PmAnalytics | null>(null);
-  const [error, setError] = useState<string | null>(null);
+  // The dashboard's shared classifier, not a raw `String(e)`. It already draws
+  // the distinction this panel's states need — `unreachable` / `unreadable` /
+  // `ratelimited` — and every sibling on this tab renders through it, so a 429
+  // and a dead server stop printing the same sentence.
+  const [failure, setFailure] = useState<LoadFailure | null>(null);
+  // Three outcomes used to render the same thing — nothing.
+  //
+  // A failed request, a request still in flight, and a reply this bundle
+  // cannot read all ended at `return null`, so the card simply was not
+  // there. The user cannot tell "still loading" from "the server is down"
+  // from "your dashboard is older than your server" when all three look like
+  // an absence, and there is nothing to click, retry or report. The sibling
+  // on the same tab (`AnalyticsTab`) already renders a spinner and an
+  // `role="alert"` box for exactly these two cases.
+  const [loading, setLoading] = useState(true);
 
   useEffect(() => {
     // `api()` already unwraps the {success, data} envelope and returns
@@ -39,20 +54,30 @@ export function PmAnalyticsPanel() {
         }
         setData(r);
       })
-      .catch((e) => setError(String(e)));
+      .catch((e) => setFailure(classifyLoadError(e)))
+      .finally(() => setLoading(false));
   }, []);
 
-  if (error) {
-    console.warn('[PmAnalyticsPanel]', error);
-    return null;
+  if (loading) return <div class="empty"><div class="loading" /></div>;
+
+  if (failure) {
+    // role="alert" per DESIGN.md: a box that stands in for content has to
+    // announce itself to a screen reader rather than repaint silently.
+    return <div class="error-box" role="alert">{failureMessage(failure)}</div>;
   }
   // Guard the LEAVES, not the groups. `{}` is truthy, so checking that
   // `velocity` / `staleness` / `connectedness` merely exist admits a payload
   // whose groups are all present and all empty — and the next lines call
   // `data.velocity.decisionsPerWeek.toFixed(1)` on `undefined`. Two earlier
   // versions of this guard tightened one level at a time (`!data`, then the
   // three groups) and each was still one level short of the read.
-  if (!isPmAnalyticsRenderable(data)) return null;
+  if (!isPmAnalyticsRenderable(data)) {
+    // The request SUCCEEDED and the reply is unreadable — a stale bundle
+    // against a newer server, or the reverse. Saying so is the difference
+    // between "reload the page" and "memesh is broken": the console warning
+    // above is for whoever opens the console, and this is for everyone else.
+    return <div class="error-box" role="alert">{failureMessage('unreadable')}</div>;
+  }
 
   const orphanPct = (data.connectedness.orphanRate * 100).toFixed(1);
   const orphanColor =

dashboard/src/components/UserPatterns.tsx

@@ -7,7 +7,7 @@ interface Props {
 }
 
 export function UserPatterns({ data }: Props) {
-  const { workSchedule, toolPreferences, focusAreas, workflow, strengths, learningAreas } = data;
+  const { workSchedule, focusAreas, workflow, strengths, learningAreas } = data;
 
   // Build hour heatmap data (0-23)
   const hourMap = new Map<number, number>();
@@ -81,33 +81,6 @@ export function UserPatterns({ data }: Props) {
         </div>
       </div>
 
-      {/* Top Tools */}
-      {toolPreferences.length > 0 && (
-        <div style={{ marginBottom: 16 }}>
-          <div style={{
-            fontSize: 11,
-            fontWeight: 600,
-            color: 'var(--text-3)',
-            textTransform: 'uppercase',
-            letterSpacing: '.06em',
-            marginBottom: 8,
-          }}>
-            {t('patterns.tools')}
-          </div>
-          <div style={{ display: 'flex', flexWrap: 'wrap', gap: 6 }}>
-            {toolPreferences.slice(0, 10).map((tp) => (
-              <span
-                key={tp.tool}
-                class="tag"
-                style={{ fontSize: 11, padding: '2px 8px' }}
-              >
-                {tp.tool} <span style={{ opacity: 0.5 }}>({tp.sessions})</span>
-              </span>
-            ))}
-          </div>
-        </div>
-      )}
-
       {/* Workflow Stats */}
       <div style={{ marginBottom: 16 }}>
         <div style={{
@@ -121,12 +94,6 @@ export function UserPatterns({ data }: Props) {
           {t('patterns.workflow')}
         </div>
         <div style={{ display: 'grid', gridTemplateColumns: '1fr 1fr', gap: 8 }}>
-          <div class="stat" style={{ padding: 12 }}>
-            <div class="stat-val" style={{ fontSize: 18 }}>
-              {workflow.avgSessionMinutes > 0 ? `${Math.round(workflow.avgSessionMinutes)}m` : '—'}
-            </div>
-            <div class="stat-lbl">{t('patterns.avgSession')}</div>
-          </div>
           <div class="stat" style={{ padding: 12 }}>
             <div class="stat-val" style={{ fontSize: 18 }}>
               {workflow.totalSessions > 0 ? workflow.commitsPerSession.toFixed(1) : '—'}

dashboard/src/lib/api.ts

@@ -306,9 +306,8 @@ export interface PatternsData {
     // names are rendered client-side via the patterns.day.<n> catalogue keys.
     dayDistribution: Array<{ dayNum: number; count: number }>;
   };
-  toolPreferences: Array<{ tool: string; sessions: number }>;
   focusAreas: Array<{ type: string; count: number }>;
-  workflow: { avgSessionMinutes: number; commitsPerSession: number; totalSessions: number; totalCommits: number };
+  workflow: { commitsPerSession: number; totalSessions: number; totalCommits: number };
   strengths: Array<{ type: string; avgConfidence: number; count: number }>;
   learningAreas: Array<{ tag: string; count: number }>;
 }