fix(openclaw): stop silently skipping retention on default agent:main:main sessions (#1276)

The default dynamicBankGranularity is ["agent","channel","user"] in deriveBankId,
but getIdentitySkipReason defaulted to false when the field was unset, causing
agent:main:main sessions to be silently skipped from retention and recall.

Align both paths: default agentBanking to true (matching the runtime default),
normalise dynamicBankGranularity at config-validation time, and extract a shared
DEFAULT_DYNAMIC_BANK_GRANULARITY constant.

Also adds throttled info-level logging for identity skip events so operators can
discover silent skips without enabling debug mode.

Closes #1215

Author: nicoloboschi

hindsight-integrations/openclaw/src/index.test.ts

@@ -974,12 +974,10 @@ describe("session identity helpers", () => {
     expect(bankId).toBe("main::direct%3A12345::12345");
   });
 
-  it("marks operational main sessions as skippable", () => {
+  it("allows agent:*:main sessions by default (default granularity includes 'agent')", () => {
     const result = getIdentitySkipReason({ sessionKey: "agent:main:main" });
-    expect(result.reason).toEqual({
-      kind: "final",
-      detail: "internal main session agent:main:main",
-    });
+    expect(result.reason).toBeUndefined();
+    expect(result.resolvedCtx?.senderId).toBe("agent-user:main");
   });
 
   it.each([
@@ -1079,19 +1077,38 @@ describe("session identity helpers", () => {
     expect(result.resolvedCtx?.senderId).toBe("agent-user:main");
   });
 
-  it("does not broaden the carve-out when dynamicBankId is false but bankId is missing", () => {
+  it("allows agent:*:main when dynamicBankId is false but bankId is missing (default granularity includes 'agent')", () => {
     const result = getIdentitySkipReason(
       { sessionKey: "agent:main:main" },
       { dynamicBankId: false }
     );
+    // Default agentBanking is true (default granularity includes 'agent'),
+    // so the session is allowed even without an explicit bankId.
+    expect(result.reason).toBeUndefined();
+  });
+
+  it("does not broaden the carve-out when granularity excludes 'agent' and bankId is missing", () => {
+    const result = getIdentitySkipReason(
+      { sessionKey: "agent:main:main" },
+      { dynamicBankId: false, dynamicBankGranularity: ["channel", "user"] }
+    );
     expect(result.reason).toEqual({
       kind: "final",
       detail: "internal main session agent:main:main",
     });
   });
 
-  it("preserves default skip behavior when agent banking is not enabled", () => {
+  it("allows agent:*:main sessions with empty config (default granularity includes 'agent')", () => {
     const result = getIdentitySkipReason({ sessionKey: "agent:main:main" }, {});
+    expect(result.reason).toBeUndefined();
+    expect(result.resolvedCtx?.senderId).toBe("agent-user:main");
+  });
+
+  it("skips agent:*:main sessions when granularity explicitly excludes 'agent'", () => {
+    const result = getIdentitySkipReason(
+      { sessionKey: "agent:main:main" },
+      { dynamicBankGranularity: ["channel", "user"] }
+    );
     expect(result.reason).toEqual({
       kind: "final",
       detail: "internal main session agent:main:main",

hindsight-integrations/openclaw/src/index.ts

@@ -390,6 +390,20 @@ const __dirname = dirname(__filename);
 // Default bank name (fallback when channel context not available)
 const DEFAULT_BANK_NAME = "openclaw";
 
+// Default granularity fields used by deriveBankId when not explicitly configured.
+// This constant is shared between getPluginConfig (normalisation) and deriveBankId
+// (fallback) so the skip-reason check and the bank-routing logic always agree.
+const DEFAULT_DYNAMIC_BANK_GRANULARITY: Array<"agent" | "provider" | "channel" | "user"> = [
+  "agent",
+  "channel",
+  "user",
+];
+
+// Throttle set: log an info-level skip message at most once per (sessionKey) per
+// process lifetime so operators can discover silent retention/recall skips without
+// flooding the log on every turn.
+const loggedSkipSessions = new Set<string>();
+
 function getConfiguredBankId(pluginConfig: PluginConfig): string | undefined {
   if (typeof pluginConfig.bankId !== "string") {
     return undefined;
@@ -794,6 +808,27 @@ function isRetryableIdentitySkipReason(reason: IdentitySkipReason | undefined):
   return reason?.kind === "retryable";
 }
 
+/**
+ * Log an identity-skip event at info level, throttled to once per session key
+ * per process lifetime. This makes silent skips visible to operators without
+ * flooding the log on every turn.
+ */
+function logSkipOnce(
+  operation: "recall" | "retain" | "dispatch" | "agent_start",
+  sessionKey: string | undefined,
+  reason: IdentitySkipReason
+): void {
+  if (!sessionKey) return;
+  const cacheKey = `${operation}:${sessionKey}`;
+  if (loggedSkipSessions.has(cacheKey)) return;
+  loggedSkipSessions.add(cacheKey);
+  const hint =
+    reason.kind === "final"
+      ? ". If unexpected, set dynamicBankGranularity to ['agent','channel','user'] or use static banking (dynamicBankId: false + bankId: '<name>')"
+      : "";
+  log.info(`Skipping ${operation} on session '${sessionKey}': ${reason.detail}${hint}`);
+}
+
 function cacheSessionIdentity(
   sessionKey: string | undefined,
   resolvedCtx: PluginHookAgentContext | undefined
@@ -886,11 +921,13 @@ export function getIdentitySkipReason(
   // exist to keep the default multi-tenant bank from being polluted by CLI/main
   // sessions that lack a stable identity. They should NOT fire when the user has
   // explicitly opted into a routing scheme that expects those sessions:
-  //   - dynamicBankGranularity includes 'agent' → each agent (including 'main')
-  //     gets its own bank
+  //   - dynamicBankGranularity includes 'agent' (the default) → each agent
+  //     (including 'main') gets its own bank
   //   - dynamicBankId === false with a configured bankId → user pinned a single
   //     named bank and wants every session retained into it
-  const agentBanking = pluginConfig?.dynamicBankGranularity?.includes("agent") ?? false;
+  // When dynamicBankGranularity is unset, the default is ["agent","channel","user"]
+  // which includes "agent", so agentBanking defaults to true to match deriveBankId.
+  const agentBanking = pluginConfig?.dynamicBankGranularity?.includes("agent") ?? true;
   const staticBanking =
     pluginConfig?.dynamicBankId === false &&
     typeof pluginConfig?.bankId === "string" &&
@@ -994,7 +1031,7 @@ export function deriveBankId(
   const resolvedCtx = resolveSessionIdentity(ctx);
   const fields = pluginConfig.dynamicBankGranularity?.length
     ? pluginConfig.dynamicBankGranularity
-    : ["agent", "channel", "user"];
+    : DEFAULT_DYNAMIC_BANK_GRANULARITY;
 
   // Validate field names at runtime — typos silently produce 'unknown' segments
   const validFields = new Set(["agent", "channel", "user", "provider"]);
@@ -1321,7 +1358,7 @@ function getPluginConfig(api: MoltbotPluginAPI): PluginConfig {
     autoRecall: config.autoRecall !== false, // Default: true (on) — backward compatible
     dynamicBankGranularity: Array.isArray(config.dynamicBankGranularity)
       ? config.dynamicBankGranularity
-      : undefined,
+      : DEFAULT_DYNAMIC_BANK_GRANULARITY,
     autoRetain: config.autoRetain !== false, // Default: true
     retainRoles: Array.isArray(config.retainRoles) ? config.retainRoles : undefined,
     retainFormat: config.retainFormat === "text" ? "text" : "json",
@@ -1802,6 +1839,7 @@ export default function (api: MoltbotPluginAPI) {
           debug(
             `[Hindsight] before_dispatch marked session ${sessionKey} to skip this turn: ${formatIdentitySkipReason(skipReason)}`
           );
+          logSkipOnce("dispatch", sessionKey, skipReason);
           return;
         }
         if (!resolvedCtx?.senderId || typeof resolvedCtx.senderId !== "string") {
@@ -1830,6 +1868,7 @@ export default function (api: MoltbotPluginAPI) {
           debug(
             `[Hindsight] before_agent_start skipping session ${sessionKey}: ${formatIdentitySkipReason(skipReason)}`
           );
+          logSkipOnce("agent_start", sessionKey, skipReason);
           return;
         }
         if (!resolvedCtx) {
@@ -1895,6 +1934,7 @@ export default function (api: MoltbotPluginAPI) {
           debug(
             `[Hindsight] Skipping recall for session ${sessionKeyForCache}: ${formatIdentitySkipReason(skipTurnReason)}`
           );
+          logSkipOnce("recall", sessionKeyForCache, skipTurnReason);
           return;
         }
 
@@ -1912,6 +1952,7 @@ export default function (api: MoltbotPluginAPI) {
           debug(
             `[Hindsight] Skipping recall for session ${sessionKeyForCache}: ${formatIdentitySkipReason(identitySkipReason)}`
           );
+          logSkipOnce("recall", sessionKeyForCache, identitySkipReason);
           return;
         }
 
@@ -2124,6 +2165,7 @@ ${memoriesFormatted}
           debug(
             `[Hindsight Hook] Skipping retain for session ${sessionKeyForLookup}: ${formatIdentitySkipReason(skipTurnReason)}`
           );
+          logSkipOnce("retain", sessionKeyForLookup, skipTurnReason);
           if (sessionKeyForLookup) {
             skipHindsightTurnBySession.delete(sessionKeyForLookup);
           }
@@ -2144,6 +2186,7 @@ ${memoriesFormatted}
           debug(
             `[Hindsight Hook] Skipping retain for session ${sessionKeyForLookup}: ${formatIdentitySkipReason(identitySkipReason)}`
           );
+          logSkipOnce("retain", sessionKeyForLookup, identitySkipReason);
           if (sessionKeyForLookup) {
             skipHindsightTurnBySession.delete(sessionKeyForLookup);
           }