Conversation
A turn that settles while the daemon-server WebSocket is down emits its authoritative session.state idle into a dead socket: live timeline events are control-plane (trySend drops them silently, no queue/replay), so the server and every browser keep rendering the session as working forever, and queued composer sends stay held. Observed live on deck_sub_26624c1t: idle at 23:46:27 never left the daemon; the phone showed "Agent working" plus a long-consumed queued message until 01:43, and three stops later. On every ServerLink RE-connect, re-broadcast each transport session's current authoritative session.state (byte-identical payload builder as the live onStatusChange path, so authoritative-idle shape guarantees hold) and re-push the session record through the persist callback so a store PUT lost in the same outage heals too. First connect is excluded - startup already runs a full session sync. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Desktop status bar now shows per-partition disk usage from the daemon: a new fs.statfs-based collector (no df/wmic subprocess) enumerates real mounts on Linux (/proc/self/mounts with pseudo-fs filtering, device dedupe and \040 unescaping), macOS (/ plus /Volumes with firmlink dedupe) and Windows (26 fail-fast drive probes), cached 15s with lazy background refresh, and rides along in daemon.stats. Mobile hides it; more than two partitions collapse into the fullest one plus a tooltip listing all, in shared GB/TB units. Full-disk uploads now fail with a localized "insufficient capacity" error instead of a generic upload failure: the daemon tags ENOSPC with a shared error code, the server maps it to HTTP 507, and the web matches it via one shared isInsufficientCapacityError helper on BOTH the real composer upload path (SessionControls) and the attachment download/preview button - the original branch only covered the latter, which an independent audit flagged as missing the actual requirement. All seven locales gain upload.insufficient_capacity. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A working session could render as idle. hasActiveTimelineTurn() scans the timeline tail backwards for activity evidence, and any event type it did not know fell through to `return false`. memory.context is exactly such an event and it is emitted right after the user.message of every context-injecting send - so for the whole window between the send and the first tool call (the model is only emitting agent.status thinking labels) the scan reported "no active turn". SubSessionWindow feeds that into resolveTimelineBackedSessionState, whose stale-running guard then overrides the daemon's session.state running with the store snapshot, and the footer falls asleep while the agent is demonstrably editing files. Observed on deck_sub_704c1g42: running at 08:53:32, 34 s of thinking with a memory.context tail, footer idle the whole time; the same shape hid the elapsed-thinking timer because getActiveThinkingTs stopped on memory.context too. Introduce one shared TIMELINE_METADATA_EVENT_TYPES list (memory.*, file.change, peer_audit.*, transport.queue.*, execution clone terminal) and spread it into both scans so telemetry is walked through instead of treated as a turn boundary. The two lists can no longer drift apart. An authoritative clean idle still wins: metadata now leads the scan to it rather than short-circuiting before it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Scrolling up to the end of the loaded history made the view jump a whole section back into older messages. The near-top auto-trigger and the "load older" button both call revealHiddenOlderItems(), which raises the render limit by CHAT_RENDER_ITEM_INCREMENT (250) items - all mounted ABOVE the viewport. Only the HTTP load-older sibling saved a scroll anchor, so the reveal path kept scrollTop while the content above it grew by thousands of pixels: the same offset now pointed a full chunk higher in the list. It also left the viewport inside the scrollTop < 100 trigger zone, so the next cooldown tick revealed and jumped again - the "often" in the report. Capture the pre-reveal scrollHeight inside revealHiddenOlderItems() itself, so both call sites are anchored identically instead of repeating the bookkeeping, and the existing restore effect re-adds the delta. The reading position now holds still and the revealed history stays one scroll-up away. The previous behaviour was pinned by a test asserting scrollTop stayed 0; it entered the tree inside an unrelated combined dev snapshot rather than as a deliberate UX choice, so it is re-pointed at the anchored position and joined by a near-top (scrollTop = 50) repro of the mobile case. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
macOS CI failed on the maxAuditLoops=1 rework test with call[1] holding the audit-orchestration prompt instead of the rework brief. The test paced itself with fixed `await sleep(25)` between steps, so on a loaded runner it completed the delegated audit before the run had even reached the `auditing` phase - the audit dispatch is async (broker decision plus filesystem baseline discovery) - and the whole send sequence shifted under the index-based assertions. Wait on the state each step actually depends on, reusing the file's existing waitForRunPhase helper, and add the missing waitForRunEnd so the "second REWORK ends the run" case can assert teardown before asserting the send count never grew. No product code changes. The failing run's daemon tree was byte-identical to the previous fully green run (the commit in between touched only web/), so this was a pre-existing pacing flake rather than a regression. I could not reproduce it locally even under synthetic CPU load, so this is a structural fix for the observed failure signature, not a verified repro. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Answering an AskUserQuestion often did nothing. handleAskAnswer already placed the answer at the queue FRONT, but front placement only wins against other QUEUED messages - it still waits for the active turn to settle, and that turn is the very one paused ON the question. The answer waited for the turn while the turn waited for the answer: a deadlock, not just a delay. Only claude-code-sdk escapes it via answerPendingQuestion; codex-sdk, gemini, qwen, opencode, copilot, grok, kimi and openclaw have no in-place resolver, so every dialog on those providers hit this. A dialog answer is a control response, so per the send-queue contract it must use the priority path. cancel() IS that path: it cuts in line, settles the active turn locally without waiting on the provider, keeps queued messages intact, and drains - which makes the front-placed answer the next turn immediately. That is the "force-interrupt that re-steers the model" the branch already claimed to do. The drain emits the visible user.message, so the handler must not emit it again. Cancel only fires when the send actually came back queued; a directly delivered answer must not abort unrelated work. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
"The notification arrived but the chat is empty until I press force refresh" was not a gap-creation bug, it was a gap-HEALING bug. The automatic backfill anchors its cursor to the newest local cursor-eligible event minus TIMELINE_HISTORY_AFTER_TS_OVERLAP_MS, i.e. 1 millisecond. Any event that went missing below that point is unreachable by every automatic request forever; only the manual refresh button, which sends no lower bound, could recover it. agent.status and usage.update are cursor-eligible and fire about once a second during a turn, so a gap became invisible almost immediately - hence "often". Events do go missing regularly. The server relays timeline events only to currently-subscribed viewers, so a backgrounded app has them discarded outright, with no queue and no replay - and a push notification arriving is itself proof the app was backgrounded. Independently, live timeline events are control-plane, so ServerLink.trySend drops them silently whenever the daemon link is flapping, and the reconnect resync re-broadcasts only session.state, never assistant.text / tool.* / user.message. Both windows end with an observable event: app activation and daemon reconnect. Make those two heal properly by requesting the full newest window with no lower bound - the same request the refresh button already makes, and the same treatment the low-completeness seed and empty-IDB paths above already apply for this exact symptom. Cooldowns are untouched, so this adds no request volume beyond what already fired. Deliberately NOT done: seq-discontinuity gap detection. seq is not contiguous in the daemon's own persisted timeline (one live session was missing 629 of 2066, another 1359 of 1950), so it would false-positive constantly and turn every check into an unbounded fetch. The two app-resume tests pinned afterTs at the old tail cursor; their subject is that the resume chain fires, so they now assert the no-lower- bound contract instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Regression I introduced in 1d3e2f2. After a server/daemon reconnect the user's phone received hundreds of push notifications, some quoting messages that had been queued two days earlier. lifecycle.ts subscribes to the timeline and treats ANY session.state idle as a finished turn: it calls notifySessionIdle() and drainQueue(). The resync re-broadcasts idle for EVERY transport session on EVERY reconnect, so each reconnect replayed a "turn finished" edge across all of them and drained every stale queue at once. Every agent answered its days-old queued message, every answer fired an idle hook, and each hook became a push. With 30+ transport sessions and a flapping link this multiplies fast. The resync exists to inform the SERVER and BROWSERS of state the daemon already knew; it carries no new information and must not drive local side effects. Stamp it with a shared decisionReason constant and have the daemon-local timeline listener ignore those events - remote consumers still receive them, so the stuck-"working" fix it was written for still works. Also skips liveContextIngestion for resync events: duplicate idle edges were re-triggering memory processing on every reconnect too. Known test gap: the payload marking and the predicate are covered, but the one-line lifecycle listener guard is not - that listener lives inside the large startup() body and cannot be reached without refactoring it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Bug A fix only repaired the RECOVERY layer: on activation and on daemon
reconnect the browser now asks for the newest window with no lower bound, so a
gap heals itself instead of waiting for a manual force-refresh. The underlying
drop rate did not change at all — which means that fix also removed the only
signal we had. A regression that doubles the drop rate would now look exactly
like a healthy system, just with more backfill traffic.
So make both silent-discard points count what they throw away:
1. Daemon → server. Live timeline events are control-plane, so trySend()
returns false and drops them whenever the socket is not OPEN. There is no
queue and no replay; only the data plane has dataPlaneSendQueue.
2. Server → browser. sendJsonToSessionSubscribers() writes only to sockets
subscribed to that session. With zero subscribers both loops execute zero
bodies — the normal state of a backgrounded app, and the reason "the push
notification arrived but the chat was empty" reproduced so reliably.
Only content-bearing events are counted. agent.status and usage.update fire
about once a second during a turn, so counting them would bury the signal under
noise; unknown types fail closed so a future noisy event type cannot silently
inflate the series. Counter names live in shared/ because both sides emit them
and drifting names would split the series and hide a regression in plain sight.
sendJsonToSessionSubscribers() now returns the recipient count instead of the
telemetry re-parsing the serialized event: the relay call site already holds the
parsed event, and that path runs for every event of every session.
Deliberately NOT included: seq-gap detection. The daemon's own JSONL shows seq
is not contiguous in practice (one session had 2066 events missing seq 629,
another 1950 missing 1359), so a gap detector would false-positive constantly
and teach us to ignore it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The cap exists to stop a caller that labels by something unbounded (a session id, a file path) from growing the map forever; 1000 was low enough that a handful of label dimensions could reach it, and past the cap NEW keys are silently dropped — so hitting it means later metrics go missing without any complaint. Raising it meant editing the same constant in two places, which is the tell: the counter store existed as two near-identical copies that had ALREADY drifted — only the server copy ever grew addCounter(). So the implementation moves to shared/metrics.ts and both util/metrics.ts files become re-exports, keeping every existing import path working. Also documents the scope these numbers actually have, because it is easy to over-trust them: nothing is persisted, the map is per-process and resets on restart, and since the server runs multiple replicas a server-side counter reflects ONE pod's traffic. Anything needing a durable or cluster-wide total has to be written to the database explicitly. The store had no tests at all despite being the drift victim; it now has them, including that the cap holds, that an already-known key keeps counting past the cap (established series must not freeze because something noisy filled the map), and that the daemon entry point is the SAME store rather than a second copy. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reported as: in Safari, scroll to the very bottom, refresh, and the log still sits a little above the bottom every time. The chat list sets `overflow-anchor: none` (styles.css), so the engine never compensates when something shrinks the pane — the ResizeObserver re-pin is the only thing that can. That observer consults `bannerToggleAtRef` to skip re-pinning when the shift was the pinned "Last sent" banner mounting its own ~60px (re-pinning there would snap the user back down, re-hide the banner, and start a height oscillation). But the stamp was written by an effect keyed on `[pinnedAboveViewport]`, and such an effect also runs ON MOUNT — so every mount armed a 300ms blackout over the single most layout-unstable moment there is: the first frames after a page load, when `--vvh` is applied from an effect, the composer rehydrates its saved draft, and the sub-session bar runs its 200ms max-height transition. The blackout is not merely a delay. The observer records the new height BEFORE consulting the suppression, so a change landing inside that window is consumed and unrecoverable — no later resize sees a difference. That is why the offset was permanent and reproduced on every refresh rather than self-correcting. Mount is not a toggle, so compare against the previous value instead of trusting the dep change. This also stays correct under StrictMode's double-invoked effects, where a "skip the first run" flag would arm on the second pass and stamp anyway. The counterfactual is pinned: with the old effect restored the new test fails with `expected 1040 to be 1200` — the view stranded 160px up — while the paired test asserting a genuine banner toggle must NOT re-pin keeps passing, so the guard's original anti-jitter purpose is still enforced rather than traded away. KNOWN REMAINING GAP (not addressed here): the observer reacts to `clientHeight` only, so post-paint CONTENT growth that leaves the pane height alone still cannot re-pin — async image previews (78px skeleton → up to 260px), markdown <img> with no intrinsic dimensions, and ToolBlockFold's post-paint clamp. An image-heavy chat can therefore still drift after a refresh. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Follow-up to 32a26a3, which fixed the pane-HEIGHT half of "refresh and the log sits a little above the bottom". The reporter confirmed the affected chat has no images, which ruled out the async image-preview growth I had flagged as the remaining gap and pointed at a different source I had lumped in with it: blocks that mount ABOVE every message. The "Load older" row appears once `hasOlderHistory` resolves from the first history response; the loading-older status row, the agent todo list (first child of the scroller) and the tool-chooser banner behind an async preference fetch all do the same. Growth above the viewport with `scrollTop` frozen slides the entire log down, so the user ends up looking at older content — the reported 上弹. None of it changes `renderedRevision`, so the content-update layout effect never re-pins; the ResizeObserver only watches `clientHeight`, so it is blind to it; and `overflow-anchor: none` means the engine will not compensate. Nothing corrected it. So for a bounded window after mount/session-change, re-assert the bottom when the total height moves. The window is deliberately narrow, because the danger here is becoming the "auto-update fights my scrolling" bug that 4ed20fb and 605269f were written to kill: it stands down the moment follow is disengaged by a gentle scroll-up, it never engages follow itself, it stays clear of the load-older anchor, and a quiet mount costs nothing because it only acts on a real change. It also stands down whenever `events` changed. My first attempt did not, and it broke an existing test ("does not force bottom scroll for non-rendered status updates") — correctly, because data-driven updates belong to the content-update layout effect, which deliberately declines to follow some of them. This window only handles height moving under UNCHANGED data. Keeping that boundary is what makes the new behaviour additive instead of a policy change. Each fix is pinned by its own counterfactual: strip the settle window and only the content-growth test fails (`expected 1200 to be 1320`); revert 32a26a3's mount-stamp fix and only the viewport-shrink test fails (`expected 1040 to be 1200`) — so neither test is passing for the other's reason. Two of my own tests were flaky rather than the product being wrong, and both are now deterministic instead of merely re-run until green: the banner-suppression test raced a 300ms wall-clock window (frozen clock), and the harness did not drain the `requestAnimationFrame` the mount effect queues, so under full-suite parallel load that late callback re-pinned mid-scenario. Verified with two consecutive full-suite runs, 194/194 files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Injected memory carried only a summary, while the guidance shipped beside it told the agent to "call get_memory_sources with that ref" — there was no ref to call with, so a summary that looked relevant was a dead end. Emit a compact handle on every injected line across all nine recall/startup injection sites, registering it so it actually resolves.
Also rewrite the handle derivation. It kept the first 10 hex-looking characters of the id, scavenged from anywhere in the string, so structured ids sharing a constant prefix (md-ingest:<ns>:<file>:<sha256>) collapsed onto ONE handle — their discriminating hash sits past the 10th kept character. Resolution then returned whichever colliding record was newest, i.e. the wrong memory, silently. Derive from md5 instead, base32-encoded and truncated to 13 chars (~65 bits): uniform for any id shape, and a pure function of the id, so the ref->id table stays a rebuildable index rather than the only source of truth.
Summaries were condensed with split('\n')[0], which rendered whole blocks as a bare "- [recent] ## Problem". Keep the heading as a label and pull the first real content line up beside it.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Handles were persisted by rewriting a whole JSON file on every registration, and the write error was swallowed. Once the disk filled up the file silently stopped being written, so every handle issued after that point died on the next daemon restart — the failure was invisible precisely when it mattered. Upsert only the touched rows into the context store, and warm the in-memory index from it once at startup. Resolution stays synchronous because it runs inside render paths, so the store is read once rather than per lookup, and the write is fire-and-forget so registration never blocks on I/O. IMCODES_MEMORY_SHORT_REF_PATH still selects the JSON file for hermetic tests and a debuggable dump. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Moving handles off the JSON file was meant to fix a write whose errors were swallowed — a full disk silently stopped persisting handles, and nobody found out until they died on the next restart. The store write then swallowed its own failures the same way, so the same blind spot survived in a new location: a write rejected while the store worker is still starting, or on SQLITE_BUSY, left handles unpersisted with no signal at all. Report the counter and rate-limited warning this file's own store module already uses for exactly this case. Registration stays non-fatal and fire-and-forget, since it is called from synchronous render paths. Also use BEGIN IMMEDIATE like every other write transaction here: both the worker and the startup cold-fallback path can hold a write connection, and a deferred transaction upgrading to a write under WAL fails with SQLITE_BUSY rather than waiting. Guard the ROLLBACK so a failing rollback cannot mask the original error. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
On the 43 deployment a gateway blip dropped four daemons' WebSockets. All four re-authenticated within two seconds, and 276ms later ~90 push dispatches went out inside a 500ms window — a phone full of "Task complete — ready for input", several quoting work that had finished long before. The measurement is the damning part: 181 push dispatches in twelve hours, of which 91 landed in the 06:00 minute and 90 in the 23:22 minute — the two daemon-reconnect bursts. Every single notification in half a day came from this, and not one came from work actually completing. `resyncTransportSessionStatesAfterLinkRestore()` re-states every session's CURRENT state after the link returns, so browsers stop rendering a "working" spinner for a turn that ended during the outage. Those events are re-announcements, never completions — but nothing told the push path that. Both existing guards missed: `suppressPush` was not set, and PUSH_TIMELINE_EVENT_MAX_AGE_MS cannot help because a resync event is newly minted (ts = now) even though the state it reports is old. This is the unfixed half of 8493a61. That commit stopped the resync from re-firing DAEMON-LOCAL idle side effects (notifySessionIdle, drainQueue) and even added `isServerLinkResyncStatePayload` for the purpose — but the events still went to the server, and the server had never been taught the marker. I fixed the half I had reproduced and did not follow the event across the process boundary. Fixed on both sides, which is deliberate rather than redundant: - Daemon: the resync payload now carries `suppressPush`, so it is self-describing at the source. - Server: the resync marker is also honoured in the push branch. Daemons are user-installed and upgrade on their own schedule, so a daemon-only fix would leave every daemon already in the field storming until it happened to update. This makes a server deploy sufficient to stop it now. Suppressing the push must not weaken the idle — un-sticking a stale "working" footer is the whole reason the resync exists — so a test asserts the payload still satisfies the authoritative-idle shape validator. Counterfactuals pin each layer separately: revert the server guard and the old-daemon cases fail, one of them reproducing the storm's exact shape (`expected 25 to be +0` — 25 resyncing sessions, 25 queued pushes); revert the daemon flag and only the payload test fails. A third test asserts a genuine live idle STILL pushes, so the fix cannot pass by silencing notifications outright. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The path resolver returned ~/.imcodes/memory-short-refs.json for any process that was not a test, which inverted the intended routing: tests took the store path while real daemons kept writing the JSON file. The migration off that file — and the failure reporting added alongside it — therefore never executed anywhere it mattered, leaving the original bug fully intact in production: a full disk still silently dropped handles, which then died on the next restart. Only an explicit IMCODES_MEMORY_SHORT_REF_PATH selects the file now. Handles are a pure function of the id and re-register on their next injection, so the legacy file needs no import. Report the two remaining silent paths as well — a failed JSON write, and a failed warm-load whose 0 was indistinguishable from "nothing stored" — under a metric named for what it measures rather than reusing the startup one, since registration also happens during injection and MCP search. The regression test drops VITEST/NODE_ENV before asserting: with the test markers left in place, the old resolver takes the store branch too and the test passes against the very bug it is meant to catch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The test strips VITEST/NODE_ENV so it exercises the real production route. That is what makes it able to catch the routing regression — but it also means that if the regression returns, the file branch reactivates and writes the runner's actual ~/.imcodes/memory-short-refs.json before the assertion fires. A red test must not touch real user data, and the true-negative procedure for this file triggers exactly that path. Stub the filesystem writes so the failure state is hermetic, and assert positively that no file is written when no path override is set. Verified both ways: restoring the old resolver still turns this one test red, and the real home file is byte-identical before and after that red run. Report two further silent paths while here — a JSON cache that exists but cannot be read or parsed (ENOENT stays quiet, since a missing file is the normal first run), and a warm-load response that violates its contract — both of which stranded handles with no signal. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Routing handle persistence to the store orphaned the file it used to write. The handles in it are still valid — this machine's is 337KB — so they would otherwise stay unresolvable until each memory happened to be injected again, a user-visible gap for anyone who upgrades across those versions. Carry them into the store on the first warm-load. The file is strictly a read-only fallback now: never written again, and never deleted, so it stays available as a manual recovery point. A counter records each import, so the fallback can be dropped once it stops finding anything. The legacy path is overridable, and the store tests pin it at a nonexistent path — otherwise they would read whatever the developer running them happens to have in their real home directory. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two records deriving the same handle in one namespace resolved to whichever was seen last, so the caller received a real but wrong memory with no signal — the precise failure the handle scheme exists to prevent, reachable again the moment a digest collides. That guess made sense when the derivation collided routinely: the retired cache on this machine has one handle covering 40 distinct records. It does not survive the switch to a 65-bit digest, where a same-namespace collision should never occur — so treat it as unresolvable and report it. Callers then fall back to search rather than acting on the wrong record. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Refusing to resolve a collision kept the caller from acting on the wrong memory, but it also threw away information: the answer was in one of those records, and the reader is capable of telling which. Expand all of them and mark the response ambiguous, so it can never be mistaken for a single authoritative answer. Only read expansion does this. Callers that act on the result — archive, delete, update — still refuse, because a destructive operation must never run on a guess. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The ambiguous branch returned early, skipping the kind check the single-candidate path applies, so a request naming the wrong kind got candidates instead of a validation error. Enforce it against every candidate. Expansion is capped at four, yet the comment, the schema and the commit message all claimed every record was returned — a caller could stop looking while the answer sat in an omitted record. Report candidateCount and truncated, and describe the bound honestly. The tool description is what the model actually reads, and it still described only the old single-record shape. Say what an ambiguous reply means there, including that an empty is not "no memory". Warm-load discarded malformed rows one by one and returned zero, which is indistinguishable from an empty store while every earlier handle stops resolving; count those discards. Register the handle counters so they are collectable rather than living only in a map a restart clears, and log the legacy import, since that signal is what tells us the fallback can be retired. The two regressions are covered at the MCP handler, not the resolver — resolver-level tests could not see either. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Discard reporting covered only the store warm-load. The legacy import and the explicit JSON load skipped malformed rows one at a time in silence, and a row whose stored namespace could no longer be parsed was worse: it degraded to a namespace-less entry and counted as loaded, so it looked healthy while being unresolvable for every namespaced caller. Count all three sources, and drop a row whose namespace fails to parse rather than loading a crippled one. The tool description still told the model that candidates held every match while the implementation expanded at most four, so it could stop looking with the answer sitting in an omitted record. Say "up to four", and point at candidateCount / truncated. The bounded-expansion test injected only the projection getter, so the handler fell through to the real orchestrator and built a SQLite store, WAL and log in the runner's home directory; its sole assertion, candidates.length < 5, also passed on an empty array. Inject the orchestrator, assert the exact four ids and their expanded content, add an untruncated two-candidate case, and point handle persistence at a scratch file for the whole suite. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A namespace only had to be an object with a non-empty string scope, so an illegal scope or a corrupt identity field was silently replaced with undefined and the row loaded namespace-less. Such an entry looks healthy and counts as loaded, yet can never resolve for a namespaced caller — the handle disappears with no signal, which is the exact failure this module exists to prevent. The previous commit closed only the store's syntactically broken JSON and claimed more than it delivered: the legacy import and the explicit JSON load still degraded these rows. Decode namespaces through one strict path shared by all three sources, validate the scope against the known set along with identity field types, and discard the whole row when it fails. Storing the row also has to agree with itself, so compare namespace_key against the namespace that namespace_json decodes to and discard on mismatch. That check surfaced a real corruption: the key was built by joining fields with NUL, which SQLite's NUL-terminated TEXT truncates to just the scope — degrading the (ref, kind, id, namespace_key) primary key so two namespaces sharing a scope could overwrite one another. Persist a JSON tuple instead. Cover all of it, plus the tool description that tells the model an ambiguous reply is bounded: reverting either the validation or the description now fails. That protection was missing, which is how the overstated fix went unnoticed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A namespace that named no reachable location still loaded. Scope was checked
against the known set, but not against the identity a scope requires, so
{"scope":"personal"} with no userId or projectId passed validation and produced
an entry no caller can ever match. Run the same scope/identity validation the
MCP caller already uses.
A cache file on the current schema whose entries are not an array was treated
like an empty cache and returned in silence, even though every handle in it has
just vanished. Separate that from the expected quiet rejection of an older
schema, and report it.
Stub the throttled-warning module in the MCP handler suite so collision cases
stop writing warning text into a real home, and correct the setup comment: it
claimed nothing there touches a real home, while the daemon logger still creates
its file on import and a pre-existing case still builds the context store.
Neither is this suite's to fix, so they are named rather than papered over.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The key-consistency check added with the JSON tuple discarded every row written before it, because a NUL-separated key reads back from node:sqlite as just its leading scope — the driver stops at the first NUL converting TEXT to a JS string. Warm-load therefore returned nothing and no handle survived the upgrade, defeating the durability this persistence exists for. Accept the legacy shape so those rows keep resolving; they are rewritten in the new format as their memories are registered again. The explanation shipped with that change was also wrong. It claimed SQLite truncated the stored value and collapsed the (ref, kind, id, namespace_key) primary key. Measurement says otherwise: two rows differing only by a NUL-containing key both insert, the stored bytes are intact, and the key is whole on disk. Only the read-back conversion is lossy. The comment now says that, and no longer justifies dropping old rows with a defect that does not exist. A namespace column that is present but unparseable was also still decoding as "no namespace" and loading, which strands the handle for every namespaced caller. Only an absent column may mean absent. Covered against a real SQLite store, since the behaviour under test is how node:sqlite round-trips the key and a mocked client cannot reproduce it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Both existing signals stay inside the daemon. The counter lives in a process-local map that a restart clears and that nothing exports, and the throttled warning ends up in the daemon log — on the same disk whose exhaustion is usually the failure being reported, with destination errors swallowed. In the disk-full incident that motivated this persistence work, neither signal could leave the box, so "persistence failures are observable" was not actually true. Hold the last failure in memory and publish it on the control-plane heartbeat, which already leaves the process over the WebSocket every few seconds. It is sticky rather than edge-triggered and repeats on each beat, so an operator who connects after the incident still learns that handles stopped persisting, and a reconnect does not lose it. Failures accumulate, keeping a stuck disk distinguishable from a single blip. The test stubs out the counter and the warning entirely, so it only passes if the failure is reportable through neither of them. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The health record reached the pod and stopped there. The bridge rebuilt daemon.stats from a fixed field list, so it was dropped before any browser saw it: no UI, no alert, no query — the operator visibility the previous commit claimed did not exist. Forward it, type it on the client, and ignore a non-object value rather than passing junk along. The daemon-side test could never have caught that, because it only reads an in-process getter; deleting every line of the wiring left it green. The new test asserts the frame crossing to the browser instead, from both daemon.stats and heartbeat, and fails when the forward is removed. A namespace column holding the JSON text null also still loaded as namespace-less. It parses cleanly, so guarding only against parse failure let it through, and the resulting handle is unreachable for every namespaced caller. Only a value that is absent may mean absent. The identity check added earlier had no regression either: every existing case paired an invalid namespace with a mismatched key, so the key comparison masked whether identity was ever validated. The new cases pair each namespace with the key it would really be stored under, leaving the scope rules as the only thing that can reject them. Correcting b46a62e: legacy rows are not rewritten on re-registration. The primary key includes namespace_key, so the old and new key forms coexist as two rows. Reads are unaffected and namespaces stay isolated; the cost is a duplicate row and some of the warm-load budget until old rows are cleaned up separately. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The health record reached the browser but nothing rendered it, so nobody would ever find out. Surface it next to the other daemon stats as a warning marker, with the stage, failure count, time and error in the tooltip, and say plainly what it means: handles issued now will not resolve after a restart. Discarded rows now report through the same channel. A row dropped as unusable loses that memory's handle exactly as a failed write does, so leaving it in a process-local counter hid the same class of loss the off-box signal exists for. Namespace field typing also covers localTenant and canonicalRepoId, so an unexpected shape in either is rejected rather than carried along. They stay out of the storage key deliberately: the key mirrors namespaceKey(), which defines namespace equality, and adding fields there would both diverge from that equality and invalidate every stored key again. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The health record already had tests for producing it and for relaying it through the bridge, but nothing asserted it becomes something a person sees. Removing either render site, or the field mapping off the daemon.stats frame, left the suite fully green — so the visible outlet was only as good as a typecheck. Covers both status-bar layouts, asserts a healthy daemon shows no marker (a permanently-lit warning is ignored precisely when it matters), and checks the tooltip names the stage, count and underlying error rather than just alarming. The locale check reads the real files: the t() mock renders any key handed to it, so it would stay green against a key that ships nowhere, which is how a warning turns into a raw i18n path in production. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reported as⚠️ "记忆句柄未保存". The handles were in fact written: 302 rows sat in memory_short_refs. 290 of them (96%) were unusable, and the store says why — namespace_key `["personal","","<project>","",""]` for every injected handle against `["personal","daemon-local","<project>","",""]` for the ones that worked. One field apart. Probed live before touching anything: four refs injected into a session all redeemed to `sources: []` while search-provided refs from the same session redeemed fine. Recall items carry `scope` and `projectId` but never `userId`, so registration derived an owner-less namespace while the MCP server resolves personal / user_private memory as `daemon-local`, and resolveMemoryShortRef refuses a cross-namespace match on purpose. It was worse than unresolvable. `personal` declares `requiredIdentityFields: ['user_id', 'project_id']`, so those rows FAILED validation in decodeNamespace and the warm loader discarded them — every daemon start destroyed them rather than merely missing them. My first two attempts at this got that wrong in both directions, so both halves are now pinned separately: 1. Registration makes the owner explicit, so newly minted rows are policy-valid. 2. The warm loader backfills the owner on already-stored owner-less rows instead of dropping them, and accepts their pre-backfill namespace_key shape — which is what rescues the existing 290 instead of only fixing future ones. Root cause of the root cause: `daemon-local` was copy-pasted into four modules while shared/memory-namespace.ts already exported LEGACY_DAEMON_LOCAL_USER_ID. Because nobody imported the shared one, the rule that fills the owner in existed in exactly one of the two places that needed it. That is the duplication CLAUDE.md forbids, so the helper now lives beside the sentinel and both sides derive from it. Deliberately NOT done: dropping userId from the local identity instead. It is tempting — this store is per-device and a device has one owner, so the field carries no information locally (its only real values here were `''` 254× and the sentinel 43×). But `namespaceKey` is also what legacy NUL-separated rows are matched by, and rewriting it broke the legacy-key migration outright; and the scope policy requires an owner for owner-private memory regardless. Making the single owner explicit satisfies both. Boundary kept: SHARED scopes still isolate by owner, because sharing does not transfer ownership. The sentinel stands for "this device's owner", never for "anybody" — a test asserts a real user id and the sentinel stay distinct. Counterfactuals, each isolating one half: revert registration and the two redemption tests fail (`expected undefined to be 'proj-id-1'`); revert the loader rescue and the legacy row loads as `expected +0 to be 1` — the discard, reproduced. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Follow-up to edbe525, from the independent audit of it. That commit taught the warm loader to accept a row whose persisted namespace_key was written before the owner became explicit — otherwise the key-consistency check rejects the very rows the owner backfill exists to rescue. But it offered that alternative for EVERY decoded namespace, so it also excused a row whose namespace_json names an explicit owner (or a shared scope) while its key is the owner-less tuple. That is a genuine key/JSON disagreement, which is exactly the corruption the check is there to catch — the guard had become broader than its own comment claimed. Those rows could never cross-resolve (they are filed under the validated JSON namespace, and every namespaced lookup compares the full tuple), so this was not an isolation hole. It was an integrity check quietly not doing its job. `decodeNamespace` now reports whether it actually backfilled an owner, and only then is the pre-backfill key shape accepted. Pinned by a test that feeds a row with an owner-less key but an explicit `user-42` in the JSON: it must be discarded. Counterfactual: with the narrowing reverted that row loads (`expected 1 to be +0`). Also finishes the de-duplication that caused the original bug. `daemon-local` was copy-pasted into four modules while shared/memory-namespace.ts already exported LEGACY_DAEMON_LOCAL_USER_ID; edbe525 only fixed one of them. memory-read-tools.ts was the starkest case — it imported the shared constant and then shadowed it with its own literal of the same value. Both now alias the shared sentinel, so no memory module defines that string again. command-handler.ts keeps its own DAEMON_LOCAL_PREFERENCE_USER_ID: same text, different domain (preferences, not memory), and merging domains that may legitimately diverge would be a worse bug than the duplication. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Follow-up to 96cd604, from the second independent audit — which found this with a real-SQLite probe rather than a mock, and was right. The loader accepts a row whose persisted namespace_key predates the owner being made explicit, but it rebuilt that key by ASSUMING the missing owner had been the empty string. `normalizeDaemonLocalMemoryNamespace` also treats a whitespace-only owner as missing, so such a row IS backfilled — and then the reconstruction produced `''` where the row had stored `" "`, the keys disagreed, and the row was discarded as corrupt. Exactly the destruction this whole line of work exists to stop, in a shape I had not considered. `decodeNamespace` now hands back the pre-backfill namespace itself rather than a boolean, so the loader compares against the bytes the row was actually written with. That removes the assumption instead of extending it: any owner spelling the normalizer accepts as "missing" is now automatically handled, including ones nobody has thought of yet. Counterfactual: with the reconstruction restored, the new whitespace-owner test fails `expected +0 to be 1` — the row discarded, reproducing the auditor's real-SQLite finding. Also worth recording why the audit caught this and my own tests did not: mine mock the context-store client, so they only ever exercised the shapes I imagined. The auditor wrote adversarial rows into a real SQLite database and probed five shapes (owner-less JSON tuple, full NUL key, truncated scope-only key, explicit-owner mismatch, project mismatch). Three loaded, two were correctly refused, and the whitespace case fell outside all five of my mocked scenarios. Real-store coverage for this rescue remains a gap worth closing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validation
Hygiene