Skip to content

Commit 815f5aa

Browse files
authored
fix(coding-agents): write back only the new turns (append + idempotent retain) (#3336)
* feat(coding-agents): write back only the new turns (append + idempotent retain) The live write-back re-uploaded the WHOLE conversation on every Stop, and every N turns under the persistent-plugin runtime. A long session therefore re-sent its entire transcript each time, which is what turns a large session into an unretainable one rather than merely a slow one. Retains now carry a per-session cursor. A session that has already been written appends only the turns added since the last successful write, using the server's `update_mode: "append"` (supported since #932); the server concatenates them onto the stored document with "\n", which is exactly why the transcript is JSONL. Append is only correct while our view of the document matches the server's, so every uncertain case falls back to the full REPLACE this always did - no cursor, a transcript that was rewritten rather than extended (compaction, a truncated rollout), or a previous write whose outcome is unknown. Replace is idempotent by construction and so is always the safe recovery. Two supporting changes: - Conversation retains carry a deterministic v5 `operation_id`, so a resubmitted write is collapsed into the original operation instead of being applied twice. That is what makes append safe: the client aborts at 15s, and a server that committed the write anyway would otherwise get the same turns again. The field landed in v0.8.6 (#2937/#2947) and is silently IGNORED by anything older, so the append path is gated on a cached GET /version probe and older servers keep replacing. - `RetainOpts.async` is gone. Nothing ever passed `false`; retains are always async, and nothing in this plugin can afford to block a hook on extraction. Backfill, git, knowledge and survey retains are untouched: they keep replacing, and deliberately do not take a deterministic operation id, so re-retaining identical content after a document is deleted still restores it. * fix(coding-agents): serialise a session's write-backs so appends cannot overlap Found reviewing the append cursor: the runtime fires retains without awaiting them, and reading the cursor was not atomic with claiming it — the capability probe awaits in between. Two overlapping write-backs therefore both planned an append from the SAME position and submitted overlapping slices, duplicating turns inside the document: replace(REF-ID + turns 0-4), append(turns 5-7), append(turns 5-8) Serialising only the claim would not have fixed it either: that leaves an append racing a replace on the wire, where the order they land in decides the outcome. The whole read-plan-send-confirm cycle is now chained per session, so each write-back plans against the previous one's CONFIRMED cursor. The runtime's idle test now waits a tick before asserting on the fire-and-forget retain, as its sibling assertions already did — one extra microtask hop. * fix(coding-agents): key the write-back cursor to the bank it wrote to The cursor is keyed by (harness, session id), but the bank is re-derived from each hook event's cwd — so a session that moves between repos (#3133) keeps its id and changes bank. The new bank holds no document for that session, and the cursor still claimed a position in it: bank repo-a: replace(REF-ID + turns 0-4) bank repo-b: append(turns 5-7) <- turns 0-4 never existed here The cursor now records the bank it wrote to, and a mismatch replaces. Same reasoning as the fingerprint and dirty checks: anything that makes our view of the document unreliable falls back to the full write. Also covers a config change (mapPathToBank, an explicit bankId) that re-points a live session at a different bank. * fix(coding-agents): re-sync the whole document every 20 appends Review follow-up. Retains are async: the server acknowledges the submission and extracts later, so a write can be confirmed to us and still fail afterwards — and _resolve_retain_replay returns a prior operation whatever its status, so resubmitting the same payload will not redo it. Replacing everything used to be self-healing precisely because each write re-sent the whole document; appending gives that up, and a single lost write would otherwise cost the rest of the session. A full write every MAX_APPENDS_BEFORE_RESYNC appends bounds that to the turns since the last re-sync. A replace of any kind resets the count. Also from the review: - drop a session's chain entry once it settles, so a host that outlives many sessions (opencode runs for days) does not keep one resolved promise per session id forever - pin the version test to MIN_IDEMPOTENT_RETAIN_VERSION rather than repeating the literal, which also gives the exported constant a consumer
1 parent f8e588c commit 815f5aa

21 files changed

Lines changed: 910 additions & 38 deletions

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

Lines changed: 210 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { describe, expect, it, vi } from "vitest";
22
import type { HindsightClient } from "./hindsight";
33
import { renderSessionJsonl, retainLiveSession, type TransportTurn } from "./chat";
4+
import { memoryCursorStore, type RetainCursorStore } from "./retain-cursor";
45

56
describe("renderSessionJsonl", () => {
67
const turns: TransportTurn[] = [
@@ -68,11 +69,219 @@ describe("retainLiveSession", () => {
6869
expect(documentId).toBe("conversation:s2");
6970
expect(tags).toEqual(["source:chat"]);
7071
expect(strategy).toBe("conversation");
71-
expect(opts).toMatchObject({ async: true, timestamp: "2026-01-01T00:00:00Z" });
72+
expect(opts).toMatchObject({ timestamp: "2026-01-01T00:00:00Z" });
7273
expect(opts.metadata).toMatchObject({
7374
source: "chat",
7475
session_id: "s2",
7576
ref_id: "conversation:s2",
7677
});
7778
});
7879
});
80+
81+
describe("retainLiveSession — incremental write-back", () => {
82+
const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-5[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/;
83+
const turn = (i: number): TransportTurn => ({ role: "user", content: `turn ${i}` });
84+
const turns = (n: number) => Array.from({ length: n }, (_, i) => turn(i));
85+
86+
/** Client double: `supported` is what GET /version would have told us about operation_id. */
87+
const stubClient = (supported = true) => {
88+
const retain = vi.fn().mockResolvedValue(undefined);
89+
return {
90+
retain,
91+
client: {
92+
retain,
93+
bank: "coding-agent::repo",
94+
supportsIdempotentRetain: async () => supported,
95+
} as unknown as HindsightClient,
96+
};
97+
};
98+
99+
const write = (client: HindsightClient, turnList: TransportTurn[], cursors: RetainCursorStore) =>
100+
retainLiveSession(client, "s1", turnList, "2026-01-01T00:00:00Z", "codex", { cursors });
101+
102+
it("replaces on the first write, then appends only the new turns", async () => {
103+
const { retain, client } = stubClient();
104+
const cursors = memoryCursorStore();
105+
106+
await write(client, turns(2), cursors);
107+
const first = retain.mock.calls[0];
108+
expect(first[5].updateMode).toBeUndefined();
109+
expect(first[0]).toBe(renderSessionJsonl("conversation:s1", turns(2), "2026-01-01T00:00:00Z"));
110+
111+
await write(client, turns(5), cursors);
112+
const second = retain.mock.calls[1];
113+
expect(second[5].updateMode).toBe("append");
114+
// Only the three new turns, and no REF-ID header: the document already carries one.
115+
expect((second[0] as string).split("\n").map((l) => JSON.parse(l) as TransportTurn)).toEqual([
116+
turn(2),
117+
turn(3),
118+
turn(4),
119+
]);
120+
expect(second[2]).toBe("conversation:s1"); // same document id — append targets it
121+
});
122+
123+
it("sends a stable v5 operation_id so a resubmitted write is not applied twice", async () => {
124+
const a = stubClient();
125+
const b = stubClient();
126+
await write(a.client, turns(3), memoryCursorStore());
127+
await write(b.client, turns(3), memoryCursorStore());
128+
const opId = a.retain.mock.calls[0][5].operationId;
129+
expect(opId).toMatch(UUID_RE);
130+
expect(b.retain.mock.calls[0][5].operationId).toBe(opId);
131+
});
132+
133+
it("gives a different operation_id to a different payload", async () => {
134+
const { retain, client } = stubClient();
135+
const cursors = memoryCursorStore();
136+
await write(client, turns(2), cursors);
137+
await write(client, turns(5), cursors);
138+
expect(retain.mock.calls[0][5].operationId).not.toBe(retain.mock.calls[1][5].operationId);
139+
});
140+
141+
it("skips the write entirely when no turn was added", async () => {
142+
const { retain, client } = stubClient();
143+
const cursors = memoryCursorStore();
144+
await write(client, turns(3), cursors);
145+
await write(client, turns(3), cursors);
146+
expect(retain).toHaveBeenCalledTimes(1);
147+
});
148+
149+
it("replaces the whole document after a failed write, instead of appending onto an unknown state", async () => {
150+
const { retain, client } = stubClient();
151+
const cursors = memoryCursorStore();
152+
await write(client, turns(2), cursors);
153+
154+
retain.mockRejectedValueOnce(new Error("timeout"));
155+
await expect(write(client, turns(4), cursors)).rejects.toThrow("timeout");
156+
expect(cursors.read("s1")?.dirty).toBe(true);
157+
158+
await write(client, turns(6), cursors);
159+
const recovery = retain.mock.calls[2];
160+
expect(recovery[5].updateMode).toBeUndefined();
161+
expect(recovery[0]).toBe(
162+
renderSessionJsonl("conversation:s1", turns(6), "2026-01-01T00:00:00Z")
163+
);
164+
expect(cursors.read("s1")).toEqual({
165+
turns: 6,
166+
fingerprint: expect.any(String),
167+
bank: "coding-agent::repo",
168+
appends: 0, // the recovery replace re-established the document, restarting the re-sync count
169+
});
170+
});
171+
172+
it("never appends against a server that ignores operation_id", async () => {
173+
const { retain, client } = stubClient(false);
174+
const cursors = memoryCursorStore();
175+
await write(client, turns(2), cursors);
176+
await write(client, turns(5), cursors);
177+
expect(retain.mock.calls.map((c) => c[5].updateMode)).toEqual([undefined, undefined]);
178+
expect(retain.mock.calls[1][0]).toBe(
179+
renderSessionJsonl("conversation:s1", turns(5), "2026-01-01T00:00:00Z")
180+
);
181+
});
182+
183+
it("replaces (never appends) when no cursor store is supplied", async () => {
184+
const { retain, client } = stubClient();
185+
await retainLiveSession(client, "s1", turns(2), "2026-01-01T00:00:00Z", "codex");
186+
await retainLiveSession(client, "s1", turns(5), "2026-01-01T00:00:00Z", "codex");
187+
expect(retain.mock.calls.map((c) => c[5].updateMode)).toEqual([undefined, undefined]);
188+
});
189+
190+
it("serialises overlapping write-backs so neither appends a slice the other already sent", async () => {
191+
// The runtime fires retains without awaiting them: a turn-driven one and an idle-driven one can
192+
// overlap. Unserialised, both planned an append from the same cursor position and submitted
193+
// overlapping slices, duplicating turns inside the document.
194+
const submitted: { mode: string; turns: number }[] = [];
195+
const gates: (() => void)[] = [];
196+
const retain = vi.fn((content: string, ...rest: unknown[]) => {
197+
const o = rest[4] as { updateMode?: string };
198+
submitted.push({ mode: o.updateMode ?? "replace", turns: content.split("\n").length });
199+
return new Promise<void>((resolve) => gates.push(resolve));
200+
});
201+
const client = {
202+
retain,
203+
bank: "b",
204+
supportsIdempotentRetain: async () => true,
205+
} as unknown as HindsightClient;
206+
const cursors = memoryCursorStore();
207+
208+
const first = write(client, turns(5), cursors);
209+
await vi.waitFor(() => expect(gates).toHaveLength(1));
210+
gates[0]();
211+
await first;
212+
213+
const a = write(client, turns(8), cursors);
214+
const b = write(client, turns(9), cursors);
215+
// Only ONE request is in flight: the second write-back waits for the first to be confirmed.
216+
await vi.waitFor(() => expect(gates).toHaveLength(2));
217+
expect(submitted).toHaveLength(2);
218+
gates[1]();
219+
await a;
220+
await vi.waitFor(() => expect(gates).toHaveLength(3));
221+
gates[2]();
222+
await b;
223+
224+
// 6 = REF-ID + 5 turns, then turns 5-7, then turn 8 alone — every turn sent exactly once.
225+
expect(submitted).toEqual([
226+
{ mode: "replace", turns: 6 },
227+
{ mode: "append", turns: 3 },
228+
{ mode: "append", turns: 1 },
229+
]);
230+
});
231+
232+
it("keeps two sessions in the same directory independent", async () => {
233+
// Same repo => same bank, but each session owns its own document and its own cursor, so two
234+
// agents running side by side in one checkout never append into each other's conversation.
235+
const { retain, client } = stubClient();
236+
const cursors = memoryCursorStore();
237+
const writeAs = (id: string, list: TransportTurn[]) =>
238+
retainLiveSession(client, id, list, "2026-01-01T00:00:00Z", "codex", { cursors });
239+
240+
await writeAs("sess-a", turns(2));
241+
await writeAs("sess-b", turns(4));
242+
await writeAs("sess-a", turns(3));
243+
244+
const docs = retain.mock.calls.map((c) => c[2]);
245+
expect(docs).toEqual(["conversation:sess-a", "conversation:sess-b", "conversation:sess-a"]);
246+
// sess-b's longer transcript did not advance sess-a's cursor: its append is turn 2 alone.
247+
expect(retain.mock.calls[1][5].updateMode).toBeUndefined(); // first write for sess-b
248+
expect(retain.mock.calls[2][5].updateMode).toBe("append");
249+
expect((retain.mock.calls[2][0] as string).split("\n")).toHaveLength(1);
250+
expect(cursors.read("sess-a")?.turns).toBe(3);
251+
expect(cursors.read("sess-b")?.turns).toBe(4);
252+
});
253+
254+
it("replaces rather than appends when the session moved to another bank", async () => {
255+
const sent: { bank: string; mode: string }[] = [];
256+
const mk = (bank: string) =>
257+
({
258+
bank,
259+
supportsIdempotentRetain: async () => true,
260+
retain: vi.fn(async (_c: string, ...rest: unknown[]) => {
261+
sent.push({ bank, mode: (rest[4] as { updateMode?: string }).updateMode ?? "replace" });
262+
}),
263+
}) as unknown as HindsightClient;
264+
const cursors = memoryCursorStore();
265+
266+
await write(mk("repo-a"), turns(5), cursors);
267+
await write(mk("repo-b"), turns(8), cursors); // user cd'd into another repo mid-session
268+
expect(sent).toEqual([
269+
{ bank: "repo-a", mode: "replace" },
270+
{ bank: "repo-b", mode: "replace" },
271+
]);
272+
});
273+
274+
it("keeps the write-back when the capability probe itself fails", async () => {
275+
const retain = vi.fn().mockResolvedValue(undefined);
276+
const client = {
277+
retain,
278+
bank: "b",
279+
supportsIdempotentRetain: async () => {
280+
throw new Error("unreachable");
281+
},
282+
} as unknown as HindsightClient;
283+
await write(client, turns(2), memoryCursorStore());
284+
expect(retain).toHaveBeenCalledTimes(1);
285+
expect(retain.mock.calls[0][5].updateMode).toBeUndefined();
286+
});
287+
});

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

Lines changed: 102 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44
* the REF-ID tracer; every turn gets an ABSOLUTE timestamp.
55
*/
66
import type { HindsightClient } from "./hindsight";
7+
import { fingerprintTurns, planRetain, type RetainCursorStore } from "./retain-cursor";
78
import type { ChatSession } from "./types";
9+
import { uuidV5 } from "./uuid";
810
import { pool } from "./util";
911

1012
export interface TransportTurn {
@@ -86,32 +88,125 @@ export async function ingestChats(
8688
return failures;
8789
}
8890

91+
/** Capability probe for the append path. Any failure answers "no": a write-back must never be lost
92+
* because we couldn't work out whether the cheaper form of it was available. */
93+
async function supportsAppend(client: HindsightClient): Promise<boolean> {
94+
try {
95+
return await client.supportsIdempotentRetain();
96+
} catch {
97+
return false;
98+
}
99+
}
100+
101+
/**
102+
* One write-back at a time per session, keyed by the store the cursors live in.
103+
*
104+
* The persistent-plugin runtime fires retains without awaiting them (a turn-driven one and an
105+
* idle-driven one can overlap), and reading the cursor is not atomic with claiming it — there is an
106+
* await in between. Two overlapping calls therefore both planned an append from the SAME position
107+
* and submitted overlapping slices, duplicating turns inside the document; and even serialising the
108+
* claim alone would leave an append racing a replace on the wire, where the order they land in
109+
* decides whether the result is correct. Chaining the whole read-plan-send-confirm cycle is what
110+
* makes the cursor mean what it says: the second call sees the first call's CONFIRMED position.
111+
*/
112+
const writeBacks = new WeakMap<RetainCursorStore, Map<string, Promise<void>>>();
113+
114+
function serialize(
115+
cursors: RetainCursorStore,
116+
sessionId: string,
117+
write: () => Promise<void>
118+
): Promise<void> {
119+
const perSession = writeBacks.get(cursors) ?? new Map<string, Promise<void>>();
120+
writeBacks.set(cursors, perSession);
121+
// Chain off the previous write-back whether it succeeded or failed — a failure leaves the cursor
122+
// dirty, which the next call needs to see so it can replace rather than append.
123+
const next = (perSession.get(sessionId) ?? Promise.resolve()).then(write, write);
124+
const tail = next.catch(() => {});
125+
perSession.set(sessionId, tail);
126+
// Drop the entry once it is settled AND still the tail, so a host that outlives many sessions
127+
// (opencode runs for days) doesn't accumulate one resolved promise per session id forever.
128+
void tail.then(() => {
129+
if (perSession.get(sessionId) === tail) perSession.delete(sessionId);
130+
});
131+
return next;
132+
}
133+
89134
/**
90-
* Live write-back: upsert a running session under a stable document_id. Same id => Hindsight
91-
* reprocesses the FULL conversation, so the settled decision is extracted from the whole thing.
135+
* Live write-back: upsert a running session under a stable document_id, sending only what is new.
136+
*
137+
* Given a cursor store, a session that has already been written APPENDS the turns added since the
138+
* last successful write; the server concatenates them onto the stored document (with "\n", which is
139+
* why the transcript is JSONL) and re-chunks. Without one — first write, a rewritten transcript, an
140+
* unconfirmed previous write, or a server too old to be idempotent — it falls back to REPLACING the
141+
* whole document, which is what this always used to do. See core/retain-cursor.ts for why every
142+
* uncertain case resolves that way.
92143
*
93144
* Uses the same `conversation` strategy as backfilled chats — one strategy for all developer
94145
* conversations; the mission scales extraction to the substance. The content is a JSON transcript
95-
* (renderSessionJson) whose tool activity is compacted into `role:"action"` turns
146+
* (renderSessionJsonl) whose tool activity is compacted into `role:"action"` turns
96147
* (see core/transcript*.ts).
97148
*/
98149
export async function retainLiveSession(
99150
client: HindsightClient,
100151
sessionId: string,
101152
turns: TransportTurn[],
102153
startTs: string,
103-
harness?: string
154+
harness?: string,
155+
opts: { cursors?: RetainCursorStore } = {}
156+
): Promise<void> {
157+
const cursors = opts.cursors;
158+
if (!cursors) return writeSession(client, sessionId, turns, startTs, harness);
159+
// Serialised so the plan is made against the previous write-back's CONFIRMED cursor (see above).
160+
return serialize(cursors, sessionId, () =>
161+
writeSession(client, sessionId, turns, startTs, harness, cursors)
162+
);
163+
}
164+
165+
async function writeSession(
166+
client: HindsightClient,
167+
sessionId: string,
168+
turns: TransportTurn[],
169+
startTs: string,
170+
harness?: string,
171+
cursors?: RetainCursorStore
104172
): Promise<void> {
105173
const refId = `conversation:${sessionId}`;
174+
const appendSupported = Boolean(cursors) && (await supportsAppend(client));
175+
const prior = cursors?.read(sessionId);
176+
const plan = planRetain(turns, prior, { appendSupported, bank: client.bank });
177+
if (plan.mode === "skip") return;
178+
179+
const content =
180+
plan.mode === "append"
181+
? turns
182+
.slice(plan.fromTurn)
183+
.map((t) => JSON.stringify(t))
184+
.join("\n")
185+
: renderSessionJsonl(refId, turns, startTs);
186+
187+
// Claim the new position BEFORE the write and mark it unconfirmed, so a client that times out on
188+
// a request the server did commit replaces next time instead of appending the same turns twice.
189+
const next = {
190+
turns: turns.length,
191+
fingerprint: fingerprintTurns(turns, turns.length),
192+
bank: client.bank,
193+
// A full write re-establishes the document, so the re-sync countdown starts over.
194+
appends: plan.mode === "append" ? (prior?.appends ?? 0) + 1 : 0,
195+
};
196+
cursors?.write(sessionId, { ...next, dirty: true });
197+
106198
await client.retain(
107-
renderSessionJsonl(refId, turns, startTs),
199+
content,
108200
"coding agent session",
109201
refId,
110202
["source:chat", ...(harness ? [`harness:${harness}`] : [])],
111203
"conversation",
112204
{
113205
timestamp: startTs,
114-
async: true,
206+
updateMode: plan.mode === "append" ? "append" : undefined,
207+
// Identity of THIS payload: a resubmission of the same bytes is collapsed server-side into
208+
// the original operation instead of extracting (or appending) twice.
209+
operationId: uuidV5(`${client.bank}\n${refId}\n${plan.mode}\n${content}`),
115210
metadata: {
116211
source: "chat",
117212
session_id: sessionId,
@@ -120,4 +215,5 @@ export async function retainLiveSession(
120215
},
121216
}
122217
);
218+
cursors?.write(sessionId, next);
123219
}

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -193,8 +193,7 @@ export async function ingestGitLog(
193193
`git commit-message history (last ${n}) for ${repoName}`,
194194
`gitlog:${repoName}`,
195195
["source:git", "source:git-log", ...(head ? [`gitlog-head:${head}`] : [])],
196-
"gitlog",
197-
{ async: true }
196+
"gitlog"
198197
);
199198
log(`[gitlog] done: ${n} commit messages ingested as 1 document under strategy 'gitlog'`);
200199
return 0;

0 commit comments

Comments
 (0)