refactor(config): CapabilitySet for recall-operation feature gates (#1523) (#1567)

* refactor(config): CapabilitySet for recall-operation feature gates (#1523)

* fix(config): keep CapabilitySet helpers backward-compatible (#1523)

* fix(config): move CapabilitySet param last to preserve positional call shape (#1523)

Author: joshuaswarren

packages/remnic-core/src/capabilities.test.ts

@@ -0,0 +1,97 @@
+import assert from "node:assert/strict";
+import test from "node:test";
+
+import { parseConfig } from "./config.js";
+import { resolveCapabilities, type CapabilitySet } from "./capabilities.js";
+
+/**
+ * Characterization tests for the recall-operation CapabilitySet (issue #1523).
+ *
+ * These guard against composition drift: every capability field must project
+ * from its `<field>Enabled` config flag, so a future edit to
+ * `resolveCapabilities` that accidentally maps a field to the wrong flag (the
+ * rule-39 gate-divergence class, moved up one layer) fails loudly here.
+ */
+
+/**
+ * Map of CapabilitySet field → the PluginConfig flag it projects from.
+ * Kept explicit (rather than derived by string concat) so the two graph flags
+ * with non-`<field>Enabled` names are covered too.
+ */
+const FIELD_TO_FLAG: Record<keyof CapabilitySet, string> = {
+  rerankCache: "rerankCacheEnabled",
+  recallDirectAnswer: "recallDirectAnswerEnabled",
+  recallMemoryWorthFilter: "recallMemoryWorthFilterEnabled",
+  recallMmr: "recallMmrEnabled",
+  recallReasoningTraceBoost: "recallReasoningTraceBoostEnabled",
+  recallPlannerLlm: "recallPlannerLlmEnabled",
+  recallPlanner: "recallPlannerEnabled",
+  recallConfidenceGate: "recallConfidenceGateEnabled",
+  graphRecall: "graphRecallEnabled",
+  graphAssistInFullMode: "graphAssistInFullModeEnabled",
+  graphExpandedIntent: "graphExpandedIntentEnabled",
+};
+
+const FIELDS = Object.keys(FIELD_TO_FLAG) as Array<keyof CapabilitySet>;
+
+test("resolveCapabilities projects every field from its <field>Enabled flag (true variant)", () => {
+  // Build a config where every migrated flag is explicitly true.
+  const overrides: Record<string, boolean> = {};
+  for (const flag of Object.values(FIELD_TO_FLAG)) overrides[flag] = true;
+  const config = parseConfig(overrides);
+  const caps = resolveCapabilities(config);
+
+  for (const field of FIELDS) {
+    const flag = FIELD_TO_FLAG[field];
+    assert.equal(
+      caps[field],
+      (config as unknown as Record<string, boolean>)[flag],
+      `caps.${field} must equal config.${flag} (true variant)`,
+    );
+    assert.equal(caps[field], true, `caps.${field} should be true here`);
+  }
+});
+
+test("resolveCapabilities projects every field from its <field>Enabled flag (false variant)", () => {
+  const overrides: Record<string, boolean> = {};
+  for (const flag of Object.values(FIELD_TO_FLAG)) overrides[flag] = false;
+  const config = parseConfig(overrides);
+  const caps = resolveCapabilities(config);
+
+  for (const field of FIELDS) {
+    const flag = FIELD_TO_FLAG[field];
+    // The two optional graph flags carry default-when-undefined semantics, but
+    // when explicitly set to a concrete boolean the projection must match it.
+    assert.equal(
+      caps[field],
+      (config as unknown as Record<string, boolean>)[flag],
+      `caps.${field} must equal config.${flag} (false variant)`,
+    );
+    assert.equal(caps[field], false, `caps.${field} should be false here`);
+  }
+});
+
+test("resolveCapabilities preserves optional-flag defaults when the flag is undefined", () => {
+  // parseConfig with no overrides exercises the documented defaults. The two
+  // optional graph flags encode asymmetric defaults on purpose:
+  //   graphAssistInFullModeEnabled → default-ON  (`!== false`)
+  //   graphExpandedIntentEnabled    → default-OFF (`=== true`)
+  const config = parseConfig({});
+  const caps = resolveCapabilities(config);
+
+  assert.equal(
+    caps.graphAssistInFullMode,
+    config.graphAssistInFullModeEnabled !== false,
+    "graphAssistInFullMode must be default-on unless explicitly false",
+  );
+  assert.equal(
+    caps.graphExpandedIntent,
+    config.graphExpandedIntentEnabled === true,
+    "graphExpandedIntent must be default-off unless explicitly true",
+  );
+});
+
+test("resolveCapabilities returns a frozen object", () => {
+  const caps = resolveCapabilities(parseConfig({}));
+  assert.equal(Object.isFrozen(caps), true, "CapabilitySet must be frozen");
+});

packages/remnic-core/src/capabilities.ts

@@ -0,0 +1,86 @@
+/**
+ * CapabilitySet — recall-operation feature gates resolved once, then threaded.
+ *
+ * Issue #1523 (Phase 1 of epic #1520). Root cause this addresses: 161+
+ * scattered `config.<flag>Enabled` reads mean each gate is re-derived at every
+ * call site, and reviews keep finding parallel code paths where one branch
+ * checks a gate the other forgot (CLAUDE.md rule 39 — the "gate divergence"
+ * defect class). The fix is to resolve a frozen capability projection ONCE at
+ * the top of the recall operation and pass it down explicitly.
+ *
+ * Scope of THIS module (first migration PR): only the recall-operation-scoped
+ * flags below. Flags that are also read in graph construction, writes, CLI, or
+ * the summarizer are deliberately deferred to a follow-up so we never leave a
+ * single flag half-migrated (some sites on `caps.`, some on `config.`).
+ *
+ * Field naming: each field is the config flag name with the trailing `Enabled`
+ * removed (`recallMmrEnabled` → `recallMmr`).
+ *
+ * This is plumbing, not a feature — there is deliberately NO `enabled` gate for
+ * the CapabilitySet itself (rule 30 governs behavior changes; resolving and
+ * threading a capability projection must stay behavior-preserving).
+ */
+
+import type { PluginConfig } from "./types.js";
+
+/**
+ * Frozen projection of recall-operation feature gates.
+ *
+ * Every field is `readonly boolean`. The composition that maps a config flag to
+ * a capability (including default-when-undefined semantics for optional flags)
+ * lives ONLY in {@link resolveCapabilities} — call sites must read the
+ * capability, never re-derive it from raw config.
+ */
+export interface CapabilitySet {
+  /** `rerankCacheEnabled` — cache reranker scores across recall passes. */
+  readonly rerankCache: boolean;
+  /** `recallDirectAnswerEnabled` — observation-mode direct-answer tier. */
+  readonly recallDirectAnswer: boolean;
+  /** `recallMemoryWorthFilterEnabled` — Memory-Worth score reweighting. */
+  readonly recallMemoryWorthFilter: boolean;
+  /** `recallMmrEnabled` — maximal-marginal-relevance diversification. */
+  readonly recallMmr: boolean;
+  /** `recallReasoningTraceBoostEnabled` — boost reasoning-trace memories. */
+  readonly recallReasoningTraceBoost: boolean;
+  /** `recallPlannerLlmEnabled` — LLM-backed recall-mode planner. */
+  readonly recallPlannerLlm: boolean;
+  /** `recallPlannerEnabled` — recall-mode planner (heuristic + optional LLM). */
+  readonly recallPlanner: boolean;
+  /** `recallConfidenceGateEnabled` — Synapse-style confidence gate. */
+  readonly recallConfidenceGate: boolean;
+  /** `graphRecallEnabled` — graph-mode recall tier (gates planner graph mode). */
+  readonly graphRecall: boolean;
+  /** `graphAssistInFullModeEnabled` — graph-assist overlay in full mode. */
+  readonly graphAssistInFullMode: boolean;
+  /** `graphExpandedIntentEnabled` — promote broad-intent asks to graph mode. */
+  readonly graphExpandedIntent: boolean;
+}
+
+/**
+ * Resolve the recall-operation {@link CapabilitySet} from parsed config.
+ *
+ * Call this ONCE per recall operation (at the `recall()` / `recallInternal`
+ * entry) and thread the result down. Composition lives here and only here.
+ *
+ * Session toggles are intentionally not a parameter yet: `session-toggles.ts`
+ * is agent-scoped (per session/agent enable-disable of the whole plugin), not
+ * flag-scoped — none of the flags projected here have a per-session override,
+ * so there is nothing for a toggle argument to compose at this layer.
+ */
+export function resolveCapabilities(config: PluginConfig): CapabilitySet {
+  return Object.freeze({
+    rerankCache: config.rerankCacheEnabled,
+    recallDirectAnswer: config.recallDirectAnswerEnabled,
+    recallMemoryWorthFilter: config.recallMemoryWorthFilterEnabled,
+    recallMmr: config.recallMmrEnabled,
+    recallReasoningTraceBoost: config.recallReasoningTraceBoostEnabled,
+    recallPlannerLlm: config.recallPlannerLlmEnabled,
+    recallPlanner: config.recallPlannerEnabled,
+    recallConfidenceGate: config.recallConfidenceGateEnabled,
+    graphRecall: config.graphRecallEnabled,
+    // Optional flags: preserve the exact default-when-undefined semantics the
+    // migrated call sites used (`!== false` = default-on, `=== true` = default-off).
+    graphAssistInFullMode: config.graphAssistInFullModeEnabled !== false,
+    graphExpandedIntent: config.graphExpandedIntentEnabled === true,
+  });
+}

packages/remnic-core/src/direct-answer-wiring.test.ts

@@ -7,7 +7,7 @@ import {
   type DirectAnswerWiringInput,
 } from "./direct-answer-wiring.js";
 import { DEFAULT_TAXONOMY } from "./taxonomy/default-taxonomy.js";
-import type { MemoryFile, PluginConfig } from "./types.js";
+import type { MemoryFile } from "./types.js";
 import type { TrustZoneName } from "./trust-zones.js";
 
 type WiringConfig = DirectAnswerWiringInput["config"];
@@ -94,7 +94,8 @@ test("tryDirectAnswer disabled-path does not call any source accessor", async ()
   const result = await tryDirectAnswer({
     query: "does not matter",
     namespace: "default",
-    config: { ...BASE_CONFIG, recallDirectAnswerEnabled: false },
+    config: BASE_CONFIG,
+    enabled: false,
     sources,
   });
   assert.equal(result.eligible, false);
@@ -104,6 +105,42 @@ test("tryDirectAnswer disabled-path does not call any source accessor", async ()
   assert.deepEqual(sources.calls.importance, []);
 });
 
+// ── Backward-compat (#1523): omit top-level `enabled`, fall back to config ──
+
+test("tryDirectAnswer falls back to config.recallDirectAnswerEnabled when `enabled` is omitted (disabled)", async () => {
+  // Old input shape: config carries recallDirectAnswerEnabled: false and no
+  // top-level `enabled`. Must short-circuit as "disabled" (identical to the
+  // pre-#1523 behavior) rather than treating undefined as enabled.
+  const sources = makeMockSources({ memories: [makeMemory()] });
+  const result = await tryDirectAnswer({
+    query: "does not matter",
+    namespace: "default",
+    config: { ...BASE_CONFIG, recallDirectAnswerEnabled: false },
+    sources,
+  });
+  assert.equal(result.eligible, false);
+  assert.equal(result.reason, "disabled");
+  assert.equal(sources.calls.listCandidates, 0);
+});
+
+test("tryDirectAnswer falls back to config.recallDirectAnswerEnabled when `enabled` is omitted (enabled)", async () => {
+  // Old input shape with recallDirectAnswerEnabled: true and no `enabled` must
+  // NOT short-circuit — it proceeds to materialize candidates.
+  const sources = makeMockSources({
+    memories: [makeMemory({ tags: ["pnpm"], content: "remnic uses pnpm" })],
+    trustZones: { m1: "trusted" },
+    importance: { m1: 0.9 },
+  });
+  const result = await tryDirectAnswer({
+    query: "package manager remnic",
+    namespace: "default",
+    config: BASE_CONFIG, // recallDirectAnswerEnabled: true
+    sources,
+  });
+  assert.notEqual(result.reason, "disabled");
+  assert.equal(sources.calls.listCandidates, 1);
+});
+
 // ── Empty-query short-circuit: no I/O ───────────────────────────────────────
 
 test("tryDirectAnswer skips all I/O when query normalizes to zero searchable tokens", async () => {
@@ -119,6 +156,7 @@ test("tryDirectAnswer skips all I/O when query normalizes to zero searchable tok
     query: "? !!!  ",
     namespace: "default",
     config: BASE_CONFIG,
+    enabled: true,
     sources,
   });
   assert.equal(result.reason, "empty-query");
@@ -135,6 +173,7 @@ test("tryDirectAnswer with empty memory list returns no-candidates", async () =>
     query: "package manager remnic",
     namespace: "default",
     config: BASE_CONFIG,
+    enabled: true,
     sources,
   });
   assert.equal(result.reason, "no-candidates");
@@ -158,6 +197,7 @@ test("tryDirectAnswer skips importance resolution for non-trusted memories", asy
     query: "package manager remnic",
     namespace: "default",
     config: BASE_CONFIG,
+    enabled: true,
     sources,
   });
   assert.equal(result.eligible, false);
@@ -182,6 +222,7 @@ test("tryDirectAnswer skips importance for quarantine-zone memories", async () =
     query: "package manager remnic",
     namespace: "default",
     config: BASE_CONFIG,
+    enabled: true,
     sources,
   });
   assert.equal(result.eligible, false);
@@ -203,6 +244,7 @@ test("tryDirectAnswer skips importance when trust zone is missing (null)", async
     query: "package manager remnic",
     namespace: "default",
     config: BASE_CONFIG,
+    enabled: true,
     sources,
   });
   assert.equal(result.eligible, false);
@@ -229,6 +271,7 @@ test("tryDirectAnswer skips importance when taxonomy bucket is not eligible", as
     query: "package manager remnic",
     namespace: "default",
     config: BASE_CONFIG,
+    enabled: true,
     sources,
   });
   assert.equal(result.eligible, false);
@@ -254,6 +297,7 @@ test("tryDirectAnswer returns eligible for a single trusted user-confirmed decis
     query: "package manager remnic",
     namespace: "default",
     config: BASE_CONFIG,
+    enabled: true,
     sources,
   });
   assert.equal(result.eligible, true);
@@ -284,6 +328,7 @@ test("tryDirectAnswer defers to hybrid when two trusted candidates are within am
     query: "package manager remnic",
     namespace: "default",
     config: BASE_CONFIG,
+    enabled: true,
     sources,
   });
   assert.equal(result.eligible, false);
@@ -327,6 +372,7 @@ test("tryDirectAnswer throws AbortError when signal aborts mid-loop", async () =
         query: "package manager remnic",
         namespace: "default",
         config: BASE_CONFIG,
+        enabled: true,
         sources,
         abortSignal: controller.signal,
       }),
@@ -363,6 +409,7 @@ test("tryDirectAnswer throws when abort lands during trustZoneFor on the only me
         query: "package manager remnic",
         namespace: "default",
         config: BASE_CONFIG,
+        enabled: true,
         sources,
         abortSignal: controller.signal,
       }),
@@ -391,6 +438,7 @@ test("tryDirectAnswer throws when abort lands during trustZoneFor on the last of
         query: "package manager remnic",
         namespace: "default",
         config: BASE_CONFIG,
+        enabled: true,
         sources,
         abortSignal: controller.signal,
       }),
@@ -408,6 +456,7 @@ test("tryDirectAnswer throws when signal is already aborted before I/O", async (
         query: "anything",
         namespace: "default",
         config: BASE_CONFIG,
+        enabled: true,
         sources,
         abortSignal: controller.signal,
       }),
@@ -434,6 +483,7 @@ test("tryDirectAnswer passes the requested namespace to listCandidateMemories",
     query: "anything",
     namespace: "project-x",
     config: BASE_CONFIG,
+    enabled: true,
     sources,
   });
   assert.equal(observedNamespace, "project-x");
@@ -465,6 +515,7 @@ test("tryDirectAnswer forwards queryEntityRefs to the eligibility gate", async (
     query: "package manager remnic",
     namespace: "default",
     config: BASE_CONFIG,
+    enabled: true,
     sources,
     queryEntityRefs: ["remnic"],
   });