From 57162bab3a7234b017de20ff3af11f9df2ff5319 Mon Sep 17 00:00:00 2001 From: Yash Date: Mon, 14 Sep 2026 23:11:23 +0000 Subject: [PATCH] fix(harness): retain terminal end markers after failed writes --- .../terminal-end-binding-reconciliation.md | 5 + packages/harness/docs/assistant-history.md | 4 + .../session-manager-retirement-review.test.ts | 99 ++++++++++++++++++- packages/harness/src/core/session-manager.ts | 14 ++- 4 files changed, 117 insertions(+), 5 deletions(-) create mode 100644 .changeset/terminal-end-binding-reconciliation.md diff --git a/.changeset/terminal-end-binding-reconciliation.md b/.changeset/terminal-end-binding-reconciliation.md new file mode 100644 index 000000000..d14dbe282 --- /dev/null +++ b/.changeset/terminal-end-binding-reconciliation.md @@ -0,0 +1,5 @@ +--- +"@sapiom/harness": patch +--- + +Fence both compensation paths by their original close generation and exact proposed marker. Preserve End's retained tombstone or coordinator-confirmed removal and keep sibling sessions untouched. diff --git a/packages/harness/docs/assistant-history.md b/packages/harness/docs/assistant-history.md index 7fd3c2629..e9e2dff34 100644 --- a/packages/harness/docs/assistant-history.md +++ b/packages/harness/docs/assistant-history.md @@ -181,3 +181,7 @@ Restored pending children are hidden before the server starts accepting requests An exact attested no-reply seed can be removed from an older empty user-only checkpoint without inflating task counts. Clearing a seed-only record requires the exact previous revision; compacted tasks and recorded real replies remain. + +A failed Terminal binding publication can roll back only its original close +generation and exact proposed marker. A queued End retains its durable tombstone +or confirmed removal, and other sessions keep their own process and binding. diff --git a/packages/harness/src/core/session-manager-retirement-review.test.ts b/packages/harness/src/core/session-manager-retirement-review.test.ts index 570be5651..42c20c3ac 100644 --- a/packages/harness/src/core/session-manager-retirement-review.test.ts +++ b/packages/harness/src/core/session-manager-retirement-review.test.ts @@ -39,7 +39,7 @@ describe("retained Terminal cleanup and bound restart cancellation", () => { await rm(root, { recursive: true, force: true }); }); - function fixture() { + function fixture(closeRecorded = true) { const request = { cwd: root, harness: "claude-code" as const }; const path = join(root, "sessions.json"); const writeBinding = vi.fn(async (file: string, text: string) => { @@ -88,7 +88,7 @@ describe("retained Terminal cleanup and bound restart cancellation", () => { adapters: { "claude-code": adapter }, resolveAgentMapIdentity: resolveIdentity, writeSubsessionBindingRegistry: writeBinding, - onSubsessionUserClosed: async () => {}, + onSubsessionUserClosed: closeRecorded ? async () => {} : undefined, spawnPty, isPidAlive, }); @@ -315,4 +315,99 @@ describe("retained Terminal cleanup and bound restart cancellation", () => { expect(manager.getSubsessionBinding(id)).toBeNull(); }, ); + + it.each([ + ["create", false], + ["create", true], + ["resume", false], + ["resume", true], + ] as const)( + "failed %s publication cannot roll back queued End (coordinator recorded=%s)", + async (kind, closeRecorded) => { + const { request, path, writeBinding, canResume, spawnPty } = + fixture(closeRecorded); + const id = "00000000-0000-4000-8000-000000000335"; + const siblingId = "00000000-0000-4000-8000-000000000336"; + const marker = { + sessionId: id, + projectId: "project-1", + parentSessionId: "parent-1", + bindingId: "binding-primary", + incarnation: 1, + spawnEpoch: 1, + }; + const next = { ...marker, incarnation: 2, spawnEpoch: 2 }; + const siblingMarker = { + ...marker, + sessionId: siblingId, + bindingId: "binding-sibling", + }; + const trusted = { + agentMapIdentity: (sessionId: string) => ({ + sessionId, + projectId: "project-1", + userId: "user-1", + }), + }; + if (kind === "resume") { + await manager.createReserved(id, request, marker, trusted); + await manager.setAgentSessionId( + id, + "vendor-1", + "startup", + manager.getRuntimeEpoch(id)!, + ); + spawns[0]!.exit(); + await manager.flush(); + canResume.mockResolvedValue(true); + } + await manager.createReserved(siblingId, request, siblingMarker, trusted); + const siblingEpoch = manager.getRuntimeEpoch(siblingId); + const siblingSpawn = spawns.at(-1)!; + const spawnCount = spawns.length; + const held = gate(); + const failure = new Error("binding publication acknowledgement failed"); + writeBinding.mockImplementationOnce(async (file, text) => { + await held.wait(); + await writeFile(file, text); + throw failure; + }); + const preparing = + kind === "create" + ? manager.createReserved(id, request, marker, trusted) + : manager.resumeBound(id, marker, next); + const rejected = expect(preparing).rejects.toBe(failure); + await held.entered; + const closing = manager.close(id); + const endedMarker = kind === "create" ? marker : next; + expect(manager.wasSubsessionClosedByUser(endedMarker)).toBe(true); + held.release(); + await Promise.all([closing, rejected]); + + const expectedMarker = closeRecorded ? null : endedMarker; + expect(manager.getSubsessionBinding(id)).toEqual(expectedMarker); + expect(manager.getRuntimeEpoch(siblingId)).toBe(siblingEpoch); + expect(siblingSpawn.kill).not.toHaveBeenCalled(); + expect(spawnPty).toHaveBeenCalledTimes(spawnCount); + expect( + JSON.parse(await readFile(`${path}.subsession-bindings.json`, "utf8")), + ).toEqual({ + version: 1, + markers: { + [siblingId]: siblingMarker, + ...(expectedMarker ? { [id]: expectedMarker } : {}), + }, + closedSessionIds: closeRecorded ? [] : [id], + }); + siblingSpawn.exit(); + await manager.flush(); + fixture(closeRecorded); + await manager.init(); + expect(manager.getSubsessionBinding(id)).toEqual(expectedMarker); + expect(manager.getSubsessionBinding(siblingId)).toEqual(siblingMarker); + expect(manager.wasSubsessionClosedByUser(endedMarker)).toBe( + !closeRecorded, + ); + }, + ); }); diff --git a/packages/harness/src/core/session-manager.ts b/packages/harness/src/core/session-manager.ts index acbc4f9d3..727dfd6ab 100644 --- a/packages/harness/src/core/session-manager.ts +++ b/packages/harness/src/core/session-manager.ts @@ -1959,11 +1959,16 @@ export class SessionManager { // A failed spawn may leave the exact next marker durably committed. // Retrying that same transition must not require the old marker again. if (!sameSubsessionBinding(current, bindingTransition.next)) { - this.subsessionBindings.set(id, bindingTransition.next); + const next = bindingTransition.next; + this.subsessionBindings.set(id, next); try { await this.persistSubsessionBindings(); } catch (error) { - this.subsessionBindings.set(id, current); + if ( + (this.closeGenerations.get(id) ?? 0) === closeGeneration && + this.subsessionBindings.get(id) === next + ) + this.subsessionBindings.set(id, current); throw error; } } @@ -4042,7 +4047,10 @@ export class SessionManager { try { await this.persistSubsessionBindings(); } catch (error) { - if (this.subsessionBindings.get(reservedSessionId) === marker) + if ( + (this.closeGenerations.get(reservedSessionId) ?? 0) === closeGeneration && + this.subsessionBindings.get(reservedSessionId) === marker + ) this.subsessionBindings.delete(reservedSessionId); throw error; }