fix(store): let every state registration fail for anything but "exists"

changelog.d/state-registration-errors.fixed.md

@@ -0,0 +1 @@
+- A state that can't be registered, for any reason but that it already exists, now fails the call that registers it, with an error that names the state (#385). Those calls are `AgentFramework.create()`, `addModule()`, the workspace's `initStore()`, the MCPL checkpoint and host states, and the tune-out coordinator's state when it's written. Most of the registrations behind them swallowed every failure, a closed store or an I/O error alike, so the failure surfaced at a later read or write of that state, if one came, under that call's name. Two of them leave nothing behind when they fail this way: `initStore()` takes no mount until a call succeeds, and a feature set whose host state couldn't be registered is registered by the next call for it, such as a reconnect's. A registration the store answers with "already exists", as on a reopened store, is still taken as done.

src/framework.ts

@@ -75,7 +75,7 @@ import { ProcessQueueImpl } from './queue.js';
 import { DEFAULT_SHUTDOWN_TIMEOUT_MS, validateShutdownTimeout, shutdownErrorMessage, waitForShutdown, ShutdownTimeoutError } from './shutdown.js';
 import { REFUSAL_REACTIONS, REFUSAL_REACTION_FALLBACK, REFUSAL_REACTION_BASELINE } from './refusal-reactions.js';
 import { Agent } from './agent.js';
-import { ModuleRegistry, checkedToolResult, isStateExistsError, storableToolResult, thrownResult } from './module-registry.js';
+import { ModuleRegistry, checkedToolResult, ensureStateRegistered, storableToolResult, thrownResult } from './module-registry.js';
 import { McplServerRegistry } from './mcpl/server-registry.js';
 import { logValue } from './mcpl/log-value.js';
 import { FeatureSetManager } from './mcpl/feature-set-manager.js';
@@ -2381,59 +2381,34 @@ export class AgentFramework {
     building: { framework?: AgentFramework },
   ): Promise<AgentFramework> {
     // Register framework states
-    try {
-      store.registerState({ id: FRAMEWORK_STATE_ID, strategy: 'snapshot' });
-    } catch {
-      // Already registered
-    }
-    try {
-      store.registerState({ id: DEFERRED_WRITES_ID, strategy: 'snapshot' });
-    } catch {
-      // Already registered
-    }
+    ensureStateRegistered(store, { id: FRAMEWORK_STATE_ID, strategy: 'snapshot' });
+    ensureStateRegistered(store, { id: DEFERRED_WRITES_ID, strategy: 'snapshot' });
     for (const id of [COALESCING_STATE_ID, COALESCING_RECENT_ID]) {
-      try {
-        store.registerState({ id, strategy: 'snapshot' });
-      } catch {
-        // Already registered
-      }
-    }
-
-    try {
-      store.registerState({
-        id: INFERENCE_LOG_ID,
-        strategy: 'append_log',
-        deltaSnapshotEvery: 100,
-        fullSnapshotEvery: 20,
-      });
-    } catch {
-      // Already registered
+      ensureStateRegistered(store, { id, strategy: 'snapshot' });
     }
+    ensureStateRegistered(store, {
+      id: INFERENCE_LOG_ID,
+      strategy: 'append_log',
+      deltaSnapshotEvery: 100,
+      fullSnapshotEvery: 20,
+    });
 
     // The legacy single-map checkpoint state (TURN_CHECKPOINTS_ID) is no longer
     // registered for new stores — it's read-only fallback data in old ones.
-    try {
-      store.registerState({ id: TURN_CHECKPOINTS_TREE_ID, strategy: 'tree' });
-    } catch (error) {
-      if (!isStateExistsError(error)) throw error;
-    }
+    ensureStateRegistered(store, { id: TURN_CHECKPOINTS_TREE_ID, strategy: 'tree' });
 
     // Process logging config (default: disabled)
     const processLoggingPersist = config.processLogging?.persist ?? false;
     const processLoggingBroadcast = config.processLogging?.broadcast ?? false;
 
     // Register process log state only if persistence is enabled
     if (processLoggingPersist) {
-      try {
-        store.registerState({
-          id: PROCESS_LOG_ID,
-          strategy: 'append_log',
-          deltaSnapshotEvery: 100,
-          fullSnapshotEvery: 20,
-        });
-      } catch {
-        // Already registered
-      }
+      ensureStateRegistered(store, {
+        id: PROCESS_LOG_ID,
+        strategy: 'append_log',
+        deltaSnapshotEvery: 100,
+        fullSnapshotEvery: 20,
+      });
     }
 
     const discordAwarenessOutboxPath = config.discordAwarenessOutboxPath
@@ -2546,11 +2521,7 @@ export class AgentFramework {
       // Generation counters persist across restarts — reusing generation 1's
       // agent name after a restart would reopen (and re-seed) the previous
       // engagement's Chronicle namespace.
-      try {
-        store.registerState({ id: CONVERSATION_ROUTER_STATE_ID, strategy: 'snapshot' });
-      } catch {
-        // Already registered
-      }
+      ensureStateRegistered(store, { id: CONVERSATION_ROUTER_STATE_ID, strategy: 'snapshot' });
       try {
         const data = store.getStateJson(CONVERSATION_ROUTER_STATE_ID) as
           { generations?: Record<string, number> } | null;

src/mcpl/channel-registry.ts

@@ -16,6 +16,7 @@
 import type { ContentBlock } from '@animalabs/membrane';
 import { INLINE_WITHHELD_TEXT, isInlineContradiction, referenceStubOrNull } from './references.js';
 import type { JsStore } from '@animalabs/chronicle';
+import { ensureStateRegistered } from '../module-registry.js';
 
 import type {
   ChannelDescriptor,
@@ -1570,13 +1571,7 @@ export class ChannelRegistry {
   private initializeLifecycleStore(): void {
     if (!this.store) return;
 
-    try {
-      this.store.registerState({ id: CHANNEL_LIFECYCLE_LOG_ID, strategy: 'append_log' });
-    } catch (error) {
-      if (!(error instanceof Error) || !error.message.includes('State already exists')) {
-        throw error;
-      }
-    }
+    ensureStateRegistered(this.store, { id: CHANNEL_LIFECYCLE_LOG_ID, strategy: 'append_log' });
 
     const raw = this.store.getStateJson(CHANNEL_LIFECYCLE_LOG_ID);
     if (!Array.isArray(raw)) return;
@@ -1646,13 +1641,7 @@ export class ChannelRegistry {
   private initializeLabelHistoryStore(): void {
     if (!this.store) return;
 
-    try {
-      this.store.registerState({ id: CHANNEL_LABEL_HISTORY_LOG_ID, strategy: 'append_log' });
-    } catch (error) {
-      if (!(error instanceof Error) || !error.message.includes('State already exists')) {
-        throw error;
-      }
-    }
+    ensureStateRegistered(this.store, { id: CHANNEL_LABEL_HISTORY_LOG_ID, strategy: 'append_log' });
 
     const raw = this.store.getStateJson(CHANNEL_LABEL_HISTORY_LOG_ID);
     if (!Array.isArray(raw)) return;
@@ -2102,17 +2091,13 @@ export class ChannelRegistry {
    */
   readCoordinatorState(stateId: string): unknown {
     if (!this.store) return null;
-    try {
-      this.store.registerState({ id: stateId, strategy: 'snapshot' });
-    } catch { /* already registered */ }
+    ensureStateRegistered(this.store, { id: stateId, strategy: 'snapshot' });
     return this.store.getStateJson(stateId);
   }
 
   writeCoordinatorState(stateId: string, value: unknown): void {
     if (!this.store) return;
-    try {
-      this.store.registerState({ id: stateId, strategy: 'snapshot' });
-    } catch { /* already registered */ }
+    ensureStateRegistered(this.store, { id: stateId, strategy: 'snapshot' });
     this.store.setStateJson(stateId, value);
   }
 

src/mcpl/checkpoint-manager.ts

@@ -13,6 +13,7 @@
  */
 
 import type { JsStore } from '@animalabs/chronicle';
+import { ensureStateRegistered } from '../module-registry.js';
 
 import type { StateCheckpoint, JsonPatchOperation } from './types.js';
 
@@ -218,11 +219,7 @@ export class CheckpointManager {
     this.emitTraceFn = emitTraceFn;
 
     // Register the checkpoint metadata state slot
-    try {
-      store.registerState({ id: CHECKPOINTS_STATE_ID, strategy: 'snapshot' });
-    } catch {
-      // Already registered (e.g. framework restart)
-    }
+    ensureStateRegistered(store, { id: CHECKPOINTS_STATE_ID, strategy: 'snapshot' });
 
     // Load existing tree from Chronicle
     this.loadFromStore();
@@ -253,8 +250,9 @@ export class CheckpointManager {
       currentState: undefined,
       nodes: new Map(),
     };
-    // Read before the tree is held, so a registration that fails here is
-    // made whole by the next one.
+    // The state's slot is registered, and its stored value read, before the
+    // tree is held, so a registration that fails here leaves nothing behind
+    // and is made whole by the next one (a reconnect's re-run).
     if (opts.hostState) this.loadHostState(serverId, featureSet, tree, 'registration');
     this.trees.set(key, tree);
 
@@ -559,30 +557,27 @@ export class CheckpointManager {
   }
 
   /** A host-managed feature set's stored state, read into its tree; nothing
-   *  stored leaves it unset. A read that fails throws, naming the state:
-   *  taken as nothing stored, the next patch would apply to nothing and
-   *  store over it. `when` says what the refusal stops: a restored tree's
-   *  read comes before its server connects, and a feature set with no tree
-   *  is read when it registers, once its server has connected. */
+   *  stored leaves it unset. A registration or read that fails throws,
+   *  naming the state: taken as nothing stored, the next patch would apply
+   *  to nothing and store over it. `when` says what the refusal stops: a
+   *  restored tree's read comes before its server connects, and a feature
+   *  set with no tree is read when it registers, once its server has
+   *  connected. */
   private loadHostState(
     serverId: string,
     featureSet: string,
     tree: FeatureSetState,
     when: 'before-connect' | 'registration',
   ): void {
     const stateId = this.stateId(serverId, featureSet);
-    try {
-      this.store.registerState({ id: stateId, strategy: 'snapshot' });
-    } catch {
-      // Already registered
-    }
     let existing: unknown;
     try {
+      ensureStateRegistered(this.store, { id: stateId, strategy: 'snapshot' });
       existing = this.store.getStateJson(stateId);
     } catch (error) {
       throw new Error(
         `the host-managed state of feature set "${featureSet}" on MCPL server "${serverId}" (${stateId}) could not ` +
-          `be read: ${thrownText(error)}. ` +
+          `be registered or read: ${thrownText(error)}. ` +
           (when === 'before-connect'
             ? `Neither create() with the server configured nor a connectMcplServer() of it goes on without it, since ` +
               `the feature set's next checkpoint would store over it. It's unchanged, and they go on once it can be read.`

src/module-registry.ts

@@ -1,4 +1,4 @@
-import type { JsStore } from '@animalabs/chronicle';
+import type { JsStateRegistration, JsStore } from '@animalabs/chronicle';
 import type { ContentBlock } from '@animalabs/membrane';
 import type {
   MessageId,
@@ -44,6 +44,25 @@ export function isStateExistsError(err: unknown): boolean {
   return err instanceof Error && err.message.includes('State already exists');
 }
 
+/**
+ * Register a state, treating "already exists" as done: a store reopened after
+ * a restart already holds it. Every other failure is thrown here, where it
+ * happened, naming the state, with the store's own error as its cause. Each
+ * state the framework registers goes through this.
+ */
+export function ensureStateRegistered(
+  store: Pick<JsStore, 'registerState'>,
+  registration: JsStateRegistration,
+): void {
+  try {
+    store.registerState(registration);
+  } catch (error) {
+    if (isStateExistsError(error)) return;
+    const reason = error instanceof Error ? error.message : String(error);
+    throw new Error(`Registering state '${registration.id}' failed: ${reason}`, { cause: error });
+  }
+}
+
 /**
  * Hard ceiling for a single module's gatherContext budget, regardless of the
  * `contextTimeoutMs` it declares. gatherContext runs on the critical path of
@@ -146,14 +165,7 @@ export class ModuleRegistry {
 
     // Register module state in store
     const stateId = this.getStateId(module.name);
-    try {
-      this.store.registerState({
-        id: stateId,
-        strategy: 'snapshot',
-      });
-    } catch {
-      // State already registered (from previous run)
-    }
+    ensureStateRegistered(this.store, { id: stateId, strategy: 'snapshot' });
 
     // Check if this is a restart (state exists)
     const existingState = this.store.getStateJson(stateId);
@@ -640,18 +652,12 @@ class ModuleContextImpl implements ModuleContext {
     name: string,
     opts?: { deltaSnapshotEvery?: number; fullSnapshotEvery?: number }
   ): void {
-    const id = this.logStateId(name);
-    try {
-      this.store.registerState({
-        id,
-        strategy: 'append_log',
-        deltaSnapshotEvery: opts?.deltaSnapshotEvery ?? 100,
-        fullSnapshotEvery: opts?.fullSnapshotEvery ?? 20,
-      });
-    } catch (err) {
-      if (!isStateExistsError(err)) throw err;
-      // Already registered (previous run) — idempotent path.
-    }
+    ensureStateRegistered(this.store, {
+      id: this.logStateId(name),
+      strategy: 'append_log',
+      deltaSnapshotEvery: opts?.deltaSnapshotEvery ?? 100,
+      fullSnapshotEvery: opts?.fullSnapshotEvery ?? 20,
+    });
     this.registeredLogStates.add(name);
   }
 

src/modules/workspace/index.ts

@@ -41,7 +41,7 @@ import { hashContent, DEFAULT_MAX_FILE_SIZE } from './sync.js';
 import { DiskAgreement, sameRootIdentity } from './disk-agreement.js';
 import { BranchIntents } from './branch-intent.js';
 import { IGNORED, OTHER_MOUNT, filesystemTrustsCtime, inOtherMount, locateRoot, passedOverAt, readPrefix } from './observe.js';
-import { TOOL_BATCH_HOOK_TIMEOUT_MS } from '../../module-registry.js';
+import { TOOL_BATCH_HOOK_TIMEOUT_MS, ensureStateRegistered } from '../../module-registry.js';
 import {
   type MountRuntime,
   type PassOptions,
@@ -751,27 +751,27 @@ export class WorkspaceModule implements Module {
    * second. `ensureRunning()` gates on `ctx && mounts-populated`.
    */
   initStore(store: JsStore): void {
-    this.store = store;
+    // Every mount's states are registered before the module takes the store
+    // or any mount, so a registration that fails leaves the module as it was
+    // before the call, knowing no mount, and a later initStore starts clean.
+    const mounts = new Map<string, MountState>();
+    const intents = new Map<string, BranchIntents>();
 
     // Register tree states for each mount: the workspace tree, and the
     // branch-local intent that qualifies it.
     for (const mount of this.config.mounts) {
       const treeStateId = `workspace/${mount.name}/tree`;
       const intentTreeStateId = `workspace/${mount.name}/intent`;
       for (const id of [treeStateId, intentTreeStateId]) {
-        try {
-          store.registerState({
-            id,
-            strategy: 'tree',
-            deltaSnapshotEvery: this.config.deltaSnapshotEvery ?? 50,
-            fullSnapshotEvery: this.config.fullSnapshotEvery ?? 10,
-          });
-        } catch {
-          // State already registered (restart scenario)
-        }
+        ensureStateRegistered(store, {
+          id,
+          strategy: 'tree',
+          deltaSnapshotEvery: this.config.deltaSnapshotEvery ?? 50,
+          fullSnapshotEvery: this.config.fullSnapshotEvery ?? 10,
+        });
       }
 
-      const mountState: MountState = {
+      mounts.set(mount.name, {
         config: mount,
         treeStateId,
         intentTreeStateId,
@@ -781,11 +781,14 @@ export class WorkspaceModule implements Module {
         lastMaterializedBranchId: null,
         watcherReadyAt: null,
         watcherError: null,
-      };
-      this.mounts.set(mount.name, mountState);
-      this.intents.set(mount.name, new BranchIntents(store, intentTreeStateId));
+      });
+      intents.set(mount.name, new BranchIntents(store, intentTreeStateId));
     }
 
+    this.store = store;
+    for (const [name, state] of mounts) this.mounts.set(name, state);
+    for (const [name, intent] of intents) this.intents.set(name, intent);
+
     // What disk last agreed with, per mount and path: global records, so a
     // branch switch never rewinds it.
     this.agreement = new DiskAgreement(store, { modesObserved: () => this.modesObserved });

src/tool-result-guard.ts

@@ -3,7 +3,7 @@ import { isDeepStrictEqual } from 'node:util';
 import type { ContextManager, MessageId, MessageMetadata } from '@animalabs/context-manager';
 import type { ContentBlock, NormalizedMessage, ToolResult } from '@animalabs/membrane';
 import type { CompletedToolCall } from './types/index.js';
-import { isStateExistsError } from './module-registry.js';
+import { ensureStateRegistered } from './module-registry.js';
 import { toolResultDataToHistoryString } from './tool-result-history.js';
 import { safeSlice } from './safe-slice.js';
 
@@ -295,11 +295,7 @@ export class ToolResultGuard {
   private write(record: Record<string, unknown>): void {
     const store = this.cm.getStore();
     if (!this.registered) {
-      try {
-        store.registerState({ id: TOOL_RESULT_GUARD_AUDIT_STATE, strategy: 'append_log' });
-      } catch (error) {
-        if (!isStateExistsError(error)) throw error;
-      }
+      ensureStateRegistered(store, { id: TOOL_RESULT_GUARD_AUDIT_STATE, strategy: 'append_log' });
       this.registered = true;
     }
     store.appendToStateJson(TOOL_RESULT_GUARD_AUDIT_STATE, record);