Skip to content

Commit 383b1d2

Browse files
authored
fix(coding-agents): say when a turn is running without memory (#3455)
Addresses #3443. When the once-per-session hook reflect fails, the turn proceeds with no memory injection and the session looks exactly like a healthy one. The failure IS recorded — log.warn to plugin.log and a reflect_failed diag line — but both are files nobody is tailing mid-session. The reporter found their own two failures only because they happened to open the diag log for an unrelated reason, and one of them ate the prompt in which they were asking about this very limit. diag.ts already states the goal in its header: "so a silently memory-less session can't masquerade as one that worked". A file achieves that only in a post-mortem. One terse line on the affected turn, naming the trail to open: Hindsight · no memory this turn — see /tmp/hindsight-plugin.log Deliberately not an explanation and not advice to the agent — the details are in the file it names, and it fires at most once per session, on the turn reflect ran. Later turns don't re-run reflect, so re-announcing a failure they never observed would just nag. An EMPTY answer is NOT a failure: reflect can legitimately have nothing to say on a sparse bank, which diag already records as its own reflect_empty event. Tying the line to a flag set only in the catch keeps "no relevant memory exists" distinct from "reflect broke" — collapsing them would report a breakage on exactly the fresh, sparse banks where nothing is broken, and that confusion is what the issue is about in the first place. The notice reaches the user on claude-code (systemMessage) and as a toast on opencode/kilo; harnesses whose hook schema has no user-visible channel ignore it, as they already do for the existing reflect notice. Not addressed here: reflect overrunning the 25s cap in the first place. The cap stays (it must remain under the harness hook timeout), and the reporter's own measurements point at reasoning tokens, which is provider-specific and already reachable through HINDSIGHT_API_REFLECT_LLM_EXTRA_BODY server-side.
1 parent 257df73 commit 383b1d2

3 files changed

Lines changed: 50 additions & 3 deletions

File tree

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,16 @@
1010
import { appendFileSync } from "node:fs";
1111
import { log } from "./log";
1212

13+
/** Where the JSONL trail lands. Exported so a failure notice can name the file to open. */
14+
export function diagFilePath(): string {
15+
return process.env.HINDSIGHT_DIAG_FILE || "/tmp/hindsight-plugin.log";
16+
}
17+
1318
export function diag(harness: string, event: string, extra: Record<string, unknown> = {}): void {
1419
log.debug(harness, `diag:${event}`, extra); // debug level shows the full story in ONE file
1520
try {
1621
appendFileSync(
17-
process.env.HINDSIGHT_DIAG_FILE || "/tmp/hindsight-plugin.log",
22+
diagFilePath(),
1823
JSON.stringify({ ts: new Date().toISOString(), harness, event, ...extra }) + "\n"
1924
);
2025
} catch {

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

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { join } from "node:path";
44
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
55
import { resolveConfig } from "./config";
66
import { buildHookOutput, runHook } from "./hook";
7+
import { diagFilePath } from "./diag";
78
import { buildReflectQuery } from "./inject";
89

910
let root: string;
@@ -138,7 +139,9 @@ describe("buildHookOutput", () => {
138139
});
139140
// Reflect failed -> no reflect block; pages are never auto-injected -> nothing to inject.
140141
expect(t1.context).toBeUndefined();
141-
expect(t1.notice).toBeUndefined();
142+
// ...but the turn is NOT silent: one line pointing at the diag trail (#3443).
143+
expect(t1.notice).toContain("no memory this turn");
144+
expect(t1.notice).toContain(diagFilePath());
142145
expect(JSON.parse(readFileSync(cacheFile, "utf8")).reflectAnswer).toBe("");
143146

144147
await buildHookOutput({
@@ -152,6 +155,34 @@ describe("buildHookOutput", () => {
152155
expect(client.reflect).toHaveBeenCalledTimes(1);
153156
});
154157

158+
it("the notice fires ONCE — the turn reflect failed, not on later turns", async () => {
159+
const cfg = resolveConfig({});
160+
const client = makeClient({
161+
reflect: vi.fn(async () => {
162+
throw new Error("reflect boom");
163+
}),
164+
});
165+
const args = { harness: "claude-code", prompt: UNRELATED_PROMPT, cfg, client, cacheFile };
166+
expect((await buildHookOutput(args)).notice).toContain("no memory this turn");
167+
// Turn 2 does not re-run reflect, so re-announcing a failure it did not observe would nag.
168+
expect((await buildHookOutput(args)).notice).toBeUndefined();
169+
});
170+
171+
it("an EMPTY answer is not a failure: no notice (reflect simply had nothing to say)", async () => {
172+
const cfg = resolveConfig({});
173+
// The real client returns (data.text || "").trim() — a 200 with no text yields "".
174+
const client = makeClient({ reflect: vi.fn(async () => "") });
175+
const result = await buildHookOutput({
176+
harness: "claude-code",
177+
prompt: UNRELATED_PROMPT,
178+
cfg,
179+
client,
180+
cacheFile,
181+
});
182+
expect(result.notice).toBeUndefined();
183+
expect(result.context).toBeUndefined();
184+
});
185+
155186
it("uses a bounded low-budget reflect and caps its timeout at 25000ms", async () => {
156187
const cfg = resolveConfig({}); // reflectTimeoutMs default 120000
157188
const client = makeClient();

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

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import { existsSync, readFileSync } from "node:fs";
2121
import { deriveBankId } from "./bank";
2222
import type { Config } from "./config";
2323
import { applyBankConfig, loadConfig } from "./config";
24-
import { diag } from "./diag";
24+
import { diag, diagFilePath } from "./diag";
2525
import { log, setLogLevel } from "./log";
2626
import { startBackgroundSeed } from "./seed";
2727
import type { ClientOpts } from "./hindsight";
@@ -102,6 +102,10 @@ export async function buildHookOutput(args: {
102102
// instructs the agent to call hindsight_reflect itself when a new goal is set.
103103
let reflectAnswer = cached.reflectAnswer;
104104
let reflectRanThisTurn = false;
105+
// Set ONLY by the catch below. An empty answer is not a failure: reflect can legitimately have
106+
// nothing to say on a sparse bank (diag records that as reflect_empty), and reporting it as a
107+
// failure would tell the user the plugin broke on exactly the sessions where it did not.
108+
let reflectFailed = false;
105109
const deferInitialReflect = cached.deferInitialReflect === true;
106110
if (deferInitialReflect) {
107111
// A new bank has no useful history yet. Do not burn the once-per-session synthesis on prompt
@@ -128,6 +132,7 @@ export async function buildHookOutput(args: {
128132
});
129133
} catch (e) {
130134
reflectAnswer = ""; // ran and failed — don't retry every turn; the diag trail records it
135+
reflectFailed = true;
131136
log.warn(harness, "reflect failed — session runs without memory", {
132137
error: String((e as Error)?.message || e).slice(0, 200),
133138
});
@@ -193,6 +198,12 @@ export async function buildHookOutput(args: {
193198
notice =
194199
`${brandWord()} · goal: recall this repo's past decisions about “${excerpt}”\n` +
195200
`↳ ${preview.length > 140 ? `${preview.slice(0, 140)}…` : preview}`;
201+
} else if (reflectFailed) {
202+
// The failure is already in the diag trail and plugin.log, but both are files nobody is
203+
// tailing mid-session, so a memory-less session looked exactly like a healthy one (#3443).
204+
// One terse line pointing at the trail — not an explanation, and not advice to the agent:
205+
// this fires at most once per session, on the turn reflect ran.
206+
notice = `${brandWord()} · no memory this turn — see ${diagFilePath()}`;
196207
}
197208

198209
return { context: kept.length ? kept.join("\n\n") : undefined, notice, pagesKnown: pages.length };

0 commit comments

Comments
 (0)