fix(coding-agents): honor retainSessions in the hook harnesses (#3596) (#3607)

* fix(coding-agents): honor retainSessions in the hook harnesses (#3596)

`retainSessions` was parsed, defaulted, env-mapped and accepted as a known
config key, but only `RuntimeCore` (opencode, Kilo, Cline, Prime Agent, dsh)
ever read it. The shared Stop-hook flow behind every hook harness — claude-code,
codex, cursor-cli, copilot-cli, devin-cli, grok-build, antigravity-cli — checked
`disabled` twice and `retainSessions` never, so `retainSessions: false` (global,
per-harness or in a `banks.<id>` section) wrote the transcript back anyway. The
comment claimed this was deliberate while the docs sold the flag as a general
write-back opt-out, including a per-bank example.

Gate the write-back in `runRetainHook`, after `applyBankConfig` so a bank
section can flip it either way, and before `ensureDaemon` — a session that
writes nothing has no reason to bring a server up. A `retain_disabled` diag
record replaces the `retain_ok` that used to appear, so the opt-out is
verifiable in the diagnostic log.

`deepen`'s conversation-history import is the same door one session later: it
reads the harness's own history files and files them as `chat:<id>`. Honoring
the flag in only one of the two places would have left the opt-out cosmetic, so
it skips the import too. Git ingest, seeding, knowledge pages, recall and the
memory tools are all untouched — that separation is what distinguishes this flag
from the `disabled` kill switch.

Tests: four end-to-end `runRetainHook` cases (default writes; global false
writes nothing and builds no client; a bank override opts one repo out; a bank
override re-enables under a global opt-out), two of which fail against the
pre-fix code. Plus a family-wide structural guard in the shape of
`daemon.test.ts`'s "every harness entrypoint reaches a daemon": every module
calling `retainLiveSession` or `ingestChats` must consult the flag. The path
that forgot is by definition the one with no test, so the guard is asserted over
the whole family rather than per-harness.

* chore(docs): re-sync the coding-agents page after the README reflow

Author: nicoloboschi

hindsight-docs/docs-integrations/coding-agents.md

@@ -392,7 +392,7 @@ hook by Codex...), so one shared config serves several agents side by side:
 | `codebaseSurvey`        | `true`                               | SessionStart: headless survey of a cold repo's structure, run under the current harness's own CLI (claude/codex/antigravity/opencode), falling back to any available agent                                                                                                                                                                                                                                                                        |
 | `surveyModel`           | `haiku`                              | model for the survey — Claude recipe only (`claude -p --model`); other agents use their configured default                                                                                                                                                                                                                                                                                                                                        |
 | `surveyBudgetUsd`       | `2`                                  | survey spend cap — Claude recipe only (`claude -p --max-budget-usd`); other agents rely on their read-only sandbox                                                                                                                                                                                                                                                                                                                                |
-| `retainSessions`        | `true`                               | plugin-harness write-back (opencode, Kilo): async upsert of the session transcript every turn, plus an idle flush that captures the reply the per-turn pass can't see (set `false` to opt out; hook harnesses always write on Stop)                                                                                                                                                                                                               |
+| `retainSessions`        | `true`                               | session write-back, honored by every harness: hook harnesses write the transcript on Stop, plugin harnesses (opencode, Kilo) upsert it every turn plus an idle flush that captures the reply the per-turn pass can't see. Set `false` — globally, per harness, or per bank — to stop writing transcripts (the background history import stops with it) while recall, git ingest and the memory tools keep working                                 |
 | `maxParallelRetains`    | `10`                                 | cap on concurrent retain-related requests: drain()'s per-op polls plus deepen's chat/git retain pools. The API rate-limits bursts, not single requests — if you see 429s, lower this rather than raising it                                                                                                                                                                                                                                       |
 | `logLevel`              | `"info"`                             | plugin-log verbosity (`"debug"` \| `"info"` \| `"warn"` \| `"error"`); `HINDSIGHT_LOG_LEVEL` env overrides                                                                                                                                                                                                                                                                                                                                        |
 | `gitIngest`             | `"message"`                          | git depth for seeding AND staying current (same engine): `"message"` = commit messages only (one doc, re-upserted when HEAD moves); `"full"` = messages + per-commit full diffs (progressive, newest first); `"none"` = git off                                                                                                                                                                                                                   |

hindsight-integrations/coding-agents/README.md

@@ -385,7 +385,7 @@ hook by Codex...), so one shared config serves several agents side by side:
 | `codebaseSurvey`        | `true`                               | SessionStart: headless survey of a cold repo's structure, run under the current harness's own CLI (claude/codex/antigravity/opencode), falling back to any available agent                                                                                                                                                                                                                                                                        |
 | `surveyModel`           | `haiku`                              | model for the survey — Claude recipe only (`claude -p --model`); other agents use their configured default                                                                                                                                                                                                                                                                                                                                        |
 | `surveyBudgetUsd`       | `2`                                  | survey spend cap — Claude recipe only (`claude -p --max-budget-usd`); other agents rely on their read-only sandbox                                                                                                                                                                                                                                                                                                                                |
-| `retainSessions`        | `true`                               | plugin-harness write-back (opencode, Kilo): async upsert of the session transcript every turn, plus an idle flush that captures the reply the per-turn pass can't see (set `false` to opt out; hook harnesses always write on Stop)                                                                                                                                                                                                               |
+| `retainSessions`        | `true`                               | session write-back, honored by every harness: hook harnesses write the transcript on Stop, plugin harnesses (opencode, Kilo) upsert it every turn plus an idle flush that captures the reply the per-turn pass can't see. Set `false` — globally, per harness, or per bank — to stop writing transcripts (the background history import stops with it) while recall, git ingest and the memory tools keep working                                 |
 | `maxParallelRetains`    | `10`                                 | cap on concurrent retain-related requests: drain()'s per-op polls plus deepen's chat/git retain pools. The API rate-limits bursts, not single requests — if you see 429s, lower this rather than raising it                                                                                                                                                                                                                                       |
 | `logLevel`              | `"info"`                             | plugin-log verbosity (`"debug"` \| `"info"` \| `"warn"` \| `"error"`); `HINDSIGHT_LOG_LEVEL` env overrides                                                                                                                                                                                                                                                                                                                                        |
 | `gitIngest`             | `"message"`                          | git depth for seeding AND staying current (same engine): `"message"` = commit messages only (one doc, re-upserted when HEAD moves); `"full"` = messages + per-commit full diffs (progressive, newest first); `"none"` = git off                                                                                                                                                                                                                   |

hindsight-integrations/coding-agents/skill/SKILL.md

@@ -77,11 +77,12 @@ Layering, later wins: defaults → file → `harnesses.<name>` → `banks.<resol
 ```
 
 Key behavioral fields (any of them valid per-harness or per-bank): `disabled`,
-`retainSessions` (write-back opt-out), `gitIngest`, `reflectTimeoutMs` (AUTOMATIC session reflect,
-default 120000; hooks cap at 25s), `reflectToolTimeoutMs`/`reflectBudget` (the agent-invoked
-`hindsight_reflect` tool: default 330000 — above the server's 300s reflect wall timeout — and
-"high"), `autoReflect` (true; false = no injected first-prompt synthesis — the agent is instead
-told to call `hindsight_reflect` on new goals), `pageRefreshEveryTurns` (10),
+`retainSessions` (transcript write-back opt-out, history import included — recall and git ingest keep working), `gitIngest`,
+`reflectTimeoutMs` (AUTOMATIC session reflect, default 120000; hooks cap at 25s),
+`reflectToolTimeoutMs`/`reflectBudget` (the agent-invoked `hindsight_reflect` tool: default 330000 —
+above the server's 300s reflect wall timeout — and "high"), `autoReflect` (true; false = no injected
+first-prompt synthesis — the agent is instead told to call `hindsight_reflect` on new goals),
+`pageRefreshEveryTurns` (10),
 `pageTriggerType`/`pageTriggerCron` (when NEW knowledge pages refresh: `auto-refresh` (default) after
 each consolidation, `cron` on a schedule, `manual` never — existing pages keep the trigger they were
 created with), `autoSeed`/`seedLimit` (true/300),

hindsight-integrations/coding-agents/src/core/config.ts

@@ -77,7 +77,11 @@ export interface RawConfig {
   optInPaths?: string[];
   harness?: string; // runtime adapter (default "opencode")
   disabled?: boolean; // hard off-switch — inert plugin, for a no-memory baseline (default false)
-  retainSessions?: boolean; // opencode plugin write-back (default true; set false to opt out). Hook harnesses always write back on Stop and ignore this flag.
+  /** Session transcript write-back (default true; set false to opt out). Honored by every
+   *  harness: the hook harnesses' Stop write-back and the persistent plugins' per-turn
+   *  cadence alike. Gates ONLY the transcript — recall, git ingest, seeding and the memory
+   *  tools keep working (that is `disabled`'s job). */
+  retainSessions?: boolean;
   /** Cap on concurrent retain-related requests the client sends to the API (default 10):
    *  drain()'s per-operation polls and deepen's chat/git retain pools. A single request returning
    *  200 while bursts get 429s means the server is rate-limiting concurrency, not total volume —
@@ -299,7 +303,7 @@ export function resolveConfig(raw: RawConfig = {}): Config {
       : [],
     harness: raw.harness ?? "opencode",
     disabled: raw.disabled ?? false,
-    retainSessions: raw.retainSessions ?? true, // opencode: write back by default (parity with hook-harness Stop)
+    retainSessions: raw.retainSessions ?? true, // write sessions back by default, every harness
     maxParallelRetains: raw.maxParallelRetains || 10,
     reflectTimeoutMs: raw.reflectTimeoutMs || 120000,
     // Inherit an explicitly-raised reflectTimeoutMs (that is what users reaching for a longer

hindsight-integrations/coding-agents/src/core/retain-hook.test.ts

@@ -1,11 +1,33 @@
-import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
+import { mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from "node:fs";
 import { tmpdir } from "node:os";
-import { join } from "node:path";
+import { basename, join } from "node:path";
+import { fileURLToPath } from "node:url";
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import { deriveBankId } from "./bank";
+import { type RawConfig, resolveConfig } from "./config";
 import type { HindsightClient } from "./hindsight";
 import { buildRetain, runRetainHook } from "./retain-hook";
 import { memoryCursorStore, type RetainCursorStore } from "./retain-cursor";
 
+/** The Stop event `runRetainHook` reads from fd 0; every other read stays real. */
+let stdin = "";
+vi.mock("node:fs", async (importOriginal) => {
+  const actual = await importOriginal<typeof import("node:fs")>();
+  return {
+    ...actual,
+    readFileSync: (target: unknown, ...rest: unknown[]) =>
+      target === 0 ? stdin : (actual.readFileSync as (...a: unknown[]) => unknown)(target, ...rest),
+  };
+});
+
+/** What the hook's `loadConfig` returns — the real resolver over a per-test raw config, so bank
+ *  overrides and defaults behave exactly as they do against a real config file. */
+let rawConfig: RawConfig = {};
+vi.mock("./config", async (importOriginal) => {
+  const actual = await importOriginal<typeof import("./config")>();
+  return { ...actual, loadConfig: () => actual.resolveConfig(rawConfig) };
+});
+
 let root: string;
 let file: string;
 
@@ -240,3 +262,129 @@ describe("buildRetain — incremental write-back across Stop hooks", () => {
     expect((retain.mock.calls[2][0] as string).split("\n")).toHaveLength(5);
   });
 });
+
+/**
+ * The `retainSessions: false` opt-out (#3596): the flag was parsed and env-mapped but no Stop-hook
+ * path ever read it, so hook harnesses wrote every transcript back regardless. These drive the
+ * real `runRetainHook` — stdin event in, config resolved through the real loader — because the bug
+ * was precisely a missing wire between the two, which a test of either half alone cannot see.
+ */
+describe("runRetainHook honors retainSessions", () => {
+  const event = () => ({
+    // A fresh session id per test: `sessionRootDir` caches a session's starting directory in /tmp
+    // for the life of the session, so reusing one id would pin every test to the first test's temp
+    // dir — and so to the wrong bank.
+    session_id: `sess-gate-${basename(root)}`,
+    transcript_path: file,
+    cwd: root,
+  });
+
+  const spec = {
+    harness: "claude-code",
+    hostTimeoutSec: 60,
+    parse: (ev: Record<string, unknown>) => ({
+      sessionId: ev.session_id as string,
+      transcriptPath: ev.transcript_path as string,
+      cwd: ev.cwd as string,
+    }),
+  };
+
+  beforeEach(() => {
+    vi.stubEnv("HINDSIGHT_DIAG_FILE", join(root, "diag.log"));
+    rawConfig = {};
+    writeFileSync(
+      file,
+      JSON.stringify({
+        type: "user",
+        timestamp: "2026-01-01T00:00:00Z",
+        message: { role: "user", content: "we use zod for validation" },
+      })
+    );
+    stdin = JSON.stringify(event());
+  });
+
+  afterEach(() => {
+    vi.unstubAllEnvs();
+  });
+
+  const stubClient = () => {
+    const retain = vi.fn().mockResolvedValue(undefined);
+    const makeClient = vi.fn(() => ({
+      retain,
+      supportsIdempotentRetain: async () => false,
+    })) as unknown as Parameters<typeof runRetainHook>[1];
+    return { retain, makeClient };
+  };
+
+  it("writes the transcript back by default", async () => {
+    const { retain, makeClient } = stubClient();
+    await runRetainHook(spec, makeClient);
+    expect(retain).toHaveBeenCalledTimes(1);
+  });
+
+  it("retainSessions: false -> no write-back, and no client is even built", async () => {
+    rawConfig = { retainSessions: false };
+    const { retain, makeClient } = stubClient();
+    await runRetainHook(spec, makeClient);
+    expect(retain).not.toHaveBeenCalled();
+    expect(makeClient).not.toHaveBeenCalled();
+  });
+
+  it("a banks.<id> override opts one repo out while the global default still writes", async () => {
+    const bankId = deriveBankId(resolveConfig(), root, spec.harness, root);
+    rawConfig = { banks: { [bankId]: { retainSessions: false } } };
+    const optedOut = stubClient();
+    await runRetainHook(spec, optedOut.makeClient);
+    expect(optedOut.retain).not.toHaveBeenCalled();
+
+    rawConfig = { banks: { "some-other-bank": { retainSessions: false } } };
+    const untouched = stubClient();
+    await runRetainHook(spec, untouched.makeClient);
+    expect(untouched.retain).toHaveBeenCalledTimes(1);
+  });
+
+  it("a banks.<id> override turns write-back back on under a global opt-out", async () => {
+    const bankId = deriveBankId(resolveConfig(), root, spec.harness, root);
+    rawConfig = { retainSessions: false, banks: { [bankId]: { retainSessions: true } } };
+    const { retain, makeClient } = stubClient();
+    await runRetainHook(spec, makeClient);
+    expect(retain).toHaveBeenCalledTimes(1);
+  });
+});
+
+/**
+ * Family-wide guard, in the shape of `daemon.test.ts`'s "every harness entrypoint reaches a
+ * daemon". #3596 was not a broken line of code but a MISSING one: the persistent-plugin path
+ * honored `retainSessions` and the hook path silently didn't, and no test failed because the path
+ * that forgot is by definition the one nobody wrote a test for. So assert over the whole family:
+ * every module that puts a conversation in the bank must consult the flag — the live write-back
+ * (`retainLiveSession`) and deepen's history import (`ingestChats`) alike, since either one alone
+ * would leave the opt-out cosmetic.
+ */
+describe("every session write-back path honors retainSessions", () => {
+  const SRC = fileURLToPath(new URL("..", import.meta.url));
+  const WRITERS = ["retainLiveSession(", "ingestChats("];
+  /** The modules that DEFINE the writers — the callers are what must gate. */
+  const DEFINITIONS = ["core/chat.ts"];
+
+  function sourceFiles(dir: string, prefix = ""): string[] {
+    return readdirSync(dir, { withFileTypes: true }).flatMap((entry) => {
+      const rel = prefix ? `${prefix}/${entry.name}` : entry.name;
+      if (entry.isDirectory())
+        return entry.name === "e2e" ? [] : sourceFiles(join(dir, entry.name), rel);
+      return entry.name.endsWith(".ts") && !entry.name.includes(".test.") ? [rel] : [];
+    });
+  }
+
+  it("has no conversation-writing module that ignores the flag", () => {
+    const ungated = sourceFiles(SRC).filter((rel) => {
+      if (DEFINITIONS.includes(rel)) return false;
+      const src = readFileSync(join(SRC, rel), "utf8");
+      if (!WRITERS.some((w) => src.includes(w))) return false;
+      // `writeBackEnabled` is RuntimeCore's own reading of the same flag, shared by every
+      // persistent-plugin host.
+      return !src.includes("cfg.retainSessions") && !src.includes("writeBackEnabled");
+    });
+    expect(ungated).toEqual([]);
+  });
+});