* 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
This commit is contained in:
Nicolò Boschi
@@ -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 |
|
||||
|
||||
@@ -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 |
|
||||
|
||||
@@ -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),
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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([]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -3,9 +3,11 @@
|
||||
* normalize it, and write it back into the bank so the session compounds into memory. The retain
|
||||
* half of the plugin (the `UserPromptSubmit` hook in core/hook.ts is the recall half).
|
||||
*
|
||||
* Write-back is ON by default for this hook harness — governed only by `disabled` — unlike the
|
||||
* opencode persistent-plugin's `retainSessions` flag, which is a separate opt-in concern for a
|
||||
* long-lived process retaining mid-session on a cadence (see core/runtime.ts).
|
||||
* Write-back is ON by default, and `retainSessions: false` turns it off for the scope it is set on
|
||||
* (global, `harnesses.<name>`, or the resolved bank's `banks.<id>` section) — the same flag the
|
||||
* opencode persistent-plugin honors for its mid-session cadence (see core/runtime.ts). It gates
|
||||
* ONLY the transcript write-back: recall, git ingest, seeding and the memory tools keep working,
|
||||
* which is what separates it from the `disabled` kill switch.
|
||||
*
|
||||
* The pure logic lives in `buildRetain` (path + client in, void out) so it's unit-testable
|
||||
* without stdin; `runRetainHook` is thin plumbing around it, mirroring `runHook`/`buildHookOutput`
|
||||
@@ -137,7 +139,14 @@ export async function runRetainHook(
|
||||
const resolved = applyBankConfig(cfg, deriveBankId(cfg, cwd, spec.harness, sessionRoot), cwd);
|
||||
cfg = resolved.cfg;
|
||||
const bankId = resolved.bankId;
|
||||
if (cfg.disabled) return;
|
||||
if (cfg.disabled) return; // per-bank opt-out (banks.<id> override)
|
||||
// Checked only HERE, after the bank is resolved, so a `banks.<id>` section can turn write-back
|
||||
// back on for one repo under a global `retainSessions: false` (and vice versa). Before the
|
||||
// daemon start below: a session that writes nothing has no reason to bring a server up.
|
||||
if (!cfg.retainSessions) {
|
||||
diag(spec.harness, "retain_disabled", { bank: bankId, session: sessionId });
|
||||
return;
|
||||
}
|
||||
// Last chance to get the daemon up: this is the write path, and a session whose daemon never
|
||||
// started would otherwise lose its whole conversation. The Stop hook has the longest budget of
|
||||
// any hook and nothing is waiting on its result, so it can afford the longer wait.
|
||||
|
||||
@@ -31,6 +31,7 @@ import { SURVEY_DOC_IDS } from "./core/survey";
|
||||
import { buildPageTrigger } from "./core/missions";
|
||||
import { HindsightClient } from "./core/hindsight";
|
||||
import { DEEPEN_DIFF_TARGET } from "./core/status";
|
||||
import type { ChatSession } from "./core/types";
|
||||
import { pool } from "./core/util";
|
||||
import { getHarness, HARNESS_NAMES } from "./harness/registry";
|
||||
import { diag } from "./core/diag";
|
||||
@@ -162,11 +163,23 @@ async function main() {
|
||||
|
||||
// chats FIRST: few, and they carry the decisions that make memory necessary — never starved
|
||||
// behind the git flood. Dedup against what's already in the bank (chat:<id>).
|
||||
const chatIds = await client.listDocumentIds("source:chat").catch(() => new Set<string>());
|
||||
const all = await harness.chatReader.read({ conversations: CONV, repo: REPO });
|
||||
const sessions = all.filter((s, i) => !chatIds.has(`chat:${s.id || `s${i}`}`));
|
||||
if (all.length !== sessions.length)
|
||||
log(`[chat] ${all.length - sessions.length} conversations already ingested — skipping those`);
|
||||
//
|
||||
// `retainSessions: false` covers THIS door too, not just the live write-back (#3596): history
|
||||
// import puts the very same conversations in the bank, one session later, so honoring the flag
|
||||
// in only one of the two places would leave the opt-out cosmetic. Git ingest, knowledge pages
|
||||
// and bank configuration below are untouched by it.
|
||||
let sessions: ChatSession[] = [];
|
||||
if (!cfg.retainSessions) {
|
||||
log("[chat] retainSessions: false — skipping conversation import");
|
||||
} else {
|
||||
const chatIds = await client.listDocumentIds("source:chat").catch(() => new Set<string>());
|
||||
const all = await harness.chatReader.read({ conversations: CONV, repo: REPO });
|
||||
sessions = all.filter((s, i) => !chatIds.has(`chat:${s.id || `s${i}`}`));
|
||||
if (all.length !== sessions.length)
|
||||
log(
|
||||
`[chat] ${all.length - sessions.length} conversations already ingested — skipping those`
|
||||
);
|
||||
}
|
||||
const chatFails = await ingestChats(client, sessions, {
|
||||
concurrency: cfg.maxParallelRetains,
|
||||
log,
|
||||
|
||||
@@ -387,7 +387,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 |
|
||||
|
||||
Reference in New Issue
Block a user