fix(harness): order Terminal binding writes through End [SAP-3292]

.changeset/terminal-binding-write-order.md

@@ -0,0 +1,5 @@
+---
+"@sapiom/harness": patch
+---
+
+Order Terminal binding writes with End and surface failed restart repairs.

packages/harness/docs/assistant-history.md

@@ -128,3 +128,8 @@ Missing native history offers a separate action to review the retained record.
 Brief reduction preserves the newest useful Assistant state and the user's task
 context, with explicit omission markers. Sibling legacy associations that claim
 the same native scope with contradictory workspaces fail closed before creation.
+
+Terminal binding persistence serializes actual sidecar writes with End, taking
+each snapshot when its write runs. A failed compensating repair is surfaced so
+cleanup can be retried. Resume and Start Terminal during shutdown return a
+conflict instead of an internal-server error.

packages/harness/src/core/session-manager-close.test.ts

@@ -237,12 +237,17 @@ describe("Terminal close admission and evidence", () => {
       },
       {},
     );
-    await held.entered;
-    await manager.close(id);
-    held.release();
-    await expect(creating).rejects.toMatchObject({
+    const cancelled = expect(creating).rejects.toMatchObject({
       code: "SESSION_PREPARATION_CANCELLED",
     });
+    await held.entered;
+    const closing = manager.close(id);
+    try {
+      expect(writes).toBe(1);
+    } finally {
+      held.release();
+      await Promise.all([closing, cancelled]);
+    }
     expect(spawnPty).not.toHaveBeenCalled();
   });
 

packages/harness/src/core/session-manager-retirement-review.test.ts

@@ -230,13 +230,24 @@ describe("retained Terminal cleanup and bound restart cancellation", () => {
         trusted,
         recorded,
       );
-      await held.entered;
-      await manager.close(id);
-      expect(manager.getSubsessionBinding(id)).toBeNull();
-      held.release();
-      await expect(restarting).rejects.toMatchObject({
+      const cancelled = expect(restarting).rejects.toMatchObject({
         code: "SESSION_PREPARATION_CANCELLED",
       });
+      await held.entered;
+      const closing = manager.close(id);
+      try {
+        if (stage === "binding-write") {
+          // The initial reservation and held restart are the only writes.
+          // End must wait for the older write before persisting its tombstone.
+          expect(writeBinding).toHaveBeenCalledTimes(2);
+        } else {
+          await closing;
+          expect(manager.getSubsessionBinding(id)).toBeNull();
+        }
+      } finally {
+        held.release();
+        await Promise.all([closing, cancelled]);
+      }
       expect(manager.getSubsessionBinding(id)).toBeNull();
       expect(
         JSON.parse(await readFile(`${path}.subsession-bindings.json`, "utf8")),
@@ -245,6 +256,63 @@ describe("retained Terminal cleanup and bound restart cancellation", () => {
       expect(adapter.launch).toHaveBeenCalledOnce();
       expect(manager.get(id)?.status).toBe("exited");
       if (stage === "vendor-probe") expect(recorded).not.toHaveBeenCalled();
+      fixture();
+      await manager.init();
+      expect(manager.getSubsessionBinding(id)).toBeNull();
+    },
+  );
+
+  it.each([false, true])(
+    "surfaces failed marker repair after commit=%s and permits durable close retry",
+    async (committed) => {
+      const { request, path, writeBinding, spawnPty } = fixture();
+      const id = "00000000-0000-4000-8000-000000000334";
+      const marker = {
+        sessionId: id,
+        projectId: "project-1",
+        parentSessionId: "parent-1",
+        bindingId: "binding-1",
+        incarnation: 1,
+        spawnEpoch: 1,
+      };
+      const trusted = {
+        agentMapIdentity: () => ({
+          sessionId: id,
+          projectId: "project-1",
+          userId: "user-1",
+        }),
+      };
+      await manager.createReserved(id, request, marker, trusted);
+      spawns[0]!.exit();
+      await manager.flush();
+      const repairFailure = Object.assign(new Error("marker repair failed"), {
+        code: "ENOSPC",
+      });
+      writeBinding
+        .mockImplementationOnce(async (file, text) => {
+          if (committed) await writeFile(file, text);
+          throw new Error("marker write outcome uncertain");
+        })
+        .mockRejectedValueOnce(repairFailure);
+      await expect(
+        manager.restartFreshBound(
+          id,
+          marker,
+          { ...marker, incarnation: 2, spawnEpoch: 2 },
+          trusted,
+          async () => false,
+        ),
+      ).rejects.toBe(repairFailure);
+      expect(spawnPty).toHaveBeenCalledOnce();
+      // A failed write must not poison the queue or conceal the repair error.
+      // A later successful End must remove even a possibly committed marker.
+      await manager.close(id);
+      expect(
+        JSON.parse(await readFile(`${path}.subsession-bindings.json`, "utf8")),
+      ).toEqual({ version: 1, markers: {}, closedSessionIds: [] });
+      fixture();
+      await manager.init();
+      expect(manager.getSubsessionBinding(id)).toBeNull();
     },
   );
 });

packages/harness/src/core/session-manager.ts

@@ -1195,6 +1195,7 @@ export class SessionManager {
   private agentSessionIdentityQueue: Promise<void> = Promise.resolve();
   private agentSessionOwnerWriteSeq = 0;
   private subsessionBindingWriteSeq = 0;
+  private subsessionBindingWriteQueue: Promise<void> = Promise.resolve();
   private subsessionBindingQueue: Promise<void> = Promise.resolve();
   private initialized = false;
 
@@ -3262,6 +3263,7 @@ export class SessionManager {
     }
     await this.agentSessionIdentityQueue;
     await this.subsessionBindingQueue;
+    await this.subsessionBindingWriteQueue;
     await this.writeQueue;
   }
 
@@ -3836,7 +3838,19 @@ export class SessionManager {
     }
   }
 
-  private async persistSubsessionBindings(): Promise<void> {
+  private persistSubsessionBindings(): Promise<void> {
+    // End may overtake asynchronous restart preparation. Serialize the actual
+    // sidecar writes and snapshot at execution so an older write cannot land
+    // after End has durably removed the binding. Keep this separate from the
+    // operation queue: End must not wait on held launch preparation.
+    const next = this.subsessionBindingWriteQueue.then(() =>
+      this.writeSubsessionBindings(),
+    );
+    this.subsessionBindingWriteQueue = next.catch(() => {});
+    return next;
+  }
+
+  private async writeSubsessionBindings(): Promise<void> {
     const markers = Object.fromEntries(
       [...this.subsessionBindings.entries()]
         .sort(([left], [right]) => left.localeCompare(right))
@@ -4358,7 +4372,10 @@ export class SessionManager {
           this.subsessionBindings.get(id) === next
         )
           this.subsessionBindings.set(id, current);
-        await this.persistSubsessionBindings().catch(() => {});
+        // Report cancellation only after its durable repair succeeds. A failed
+        // repair is actionable storage uncertainty, including a post-rename
+        // failure that may have committed the advanced marker.
+        await this.persistSubsessionBindings();
         throw error;
       }
     }

packages/harness/src/server/rest.test.ts

@@ -352,9 +352,10 @@ describe("createRestRouter", () => {
   });
 
   it.each([
+    new SessionManagerClosingError(),
     new SessionPreparationCancelledError(),
     new SessionCleanupUnconfirmedError(),
-  ])("returns 409 for cancelled or unconfirmed Terminal resume: $code", async (error) => {
+  ])("returns 409 for Terminal resume lifecycle conflicts: $code", async (error) => {
     const sessionManager = fakeSessionManager([exitedSession()]);
     vi.mocked(sessionManager.resume).mockRejectedValue(error);
     vi.mocked(sessionManager.restartForMcpCredentials).mockRejectedValue(error);
@@ -390,6 +391,14 @@ describe("createRestRouter", () => {
       vi.mocked(sessionManager.activateDormant).mockRejectedValueOnce(error);
       expect((await request()).status).toBe(409);
     }
+    const shutdown = new SessionManagerClosingError();
+    vi.mocked(sessionManager.activateDormant).mockRejectedValueOnce(shutdown);
+    const denied = await request();
+    expect(denied.status).toBe(409);
+    expect(await denied.json()).toEqual({
+      error: shutdown.message,
+      code: shutdown.code,
+    });
     vi.mocked(sessionManager.activateDormant).mockRejectedValueOnce(new UnknownSessionError("sess-1"));
     expect((await request()).status).toBe(404);
   });

packages/harness/src/server/rest.ts

@@ -732,6 +732,7 @@ export function createRestRouter(options: RestRouterOptions): Router {
       err instanceof McpSessionRestartUnavailableError ||
       err instanceof ProjectSessionScopeUnavailableError ||
       err instanceof SessionAlreadyLiveError ||
+      err instanceof SessionManagerClosingError ||
       err instanceof SessionPreparationCancelledError ||
       err instanceof SessionCleanupUnconfirmedError ||
       err instanceof SessionNotDormantError ||