From 84132d30257da37c51250e733a6274bf40ab038e Mon Sep 17 00:00:00 2001
From: Claudia <noreply@anthropic.com>
Date: Fri, 7 Aug 2026 14:52:44 +0200
Subject: [PATCH] fix(ws): pin the withheld ACK with a test, and correct the
 changeset
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two blockers from the pre-submission gate, both reproduced before fixing.

1. The changeset claimed the non-mux POST branch answers OPERATION_FAILED. The
   code says the opposite in as many words ("NOT an error response,
   deliberately"), the commit message says response codes are unchanged, and the
   test asserts the 200. It was a leftover sentence from an earlier iteration that
   would have shipped into the CHANGELOG announcing an API contract change that
   does not exist — and errorCode values are SemVer-relevant per
   docs/versioning-policy.md.

2. The WebSocket half of the fix had no test protection: reverting ws-routes.ts to
   master left all 9 tests green, while the commit message sells "plus the whole
   WebSocket path" as part of the fix. Three tests added against the real WS
   route — ACK on delivery, ACK withheld and seq re-opened when the write did not
   land, and a deduplicated frame still ACKed so the client can drop it. Verified
   the other way round: with ws-routes.ts reverted, the middle one fails.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 .changeset/input-delivery-retryable.md |  7 ++--
 test/routes/ws-routes.test.ts          | 49 ++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/.changeset/input-delivery-retryable.md b/.changeset/input-delivery-retryable.md
index c9ec5357..596c41e9 100644
--- a/.changeset/input-delivery-retryable.md
+++ b/.changeset/input-delivery-retryable.md
@@ -14,8 +14,11 @@ been delivered.
 
 The bookkeeping is now rolled back on failure and the WebSocket ACK withheld, so
 the client redelivers. `Session.write()` reports whether it reached a PTY at all
-instead of silently swallowing the data, and the non-mux POST branch — whose
-response has not gone out yet — answers `OPERATION_FAILED` rather than a cheerful 200.
+instead of silently swallowing the data.
+
+Response codes are unchanged: a session can legitimately have no PTY yet (created
+but not started), so turning that into a failure status would be a contract change
+of its own.
 
 Note this does not remove the root cause: the POST still answers 200 before the
 mux write is attempted, so a client that treats any 2xx as final still cannot
diff --git a/test/routes/ws-routes.test.ts b/test/routes/ws-routes.test.ts
index 5b722036..76efc37d 100644
--- a/test/routes/ws-routes.test.ts
+++ b/test/routes/ws-routes.test.ts
@@ -208,6 +208,55 @@ describe('ws-routes', () => {
       }
     });
 
+    it('ACKs a delivered input and burns its seq', async () => {
+      const ws = await connectWs('/ws/sessions/ws-test-session/terminal');
+      try {
+        const session = ctx._session;
+        ws.send(JSON.stringify({ t: 'i', d: 'ok\r', cid: 'c1', seq: 1 }));
+
+        expect(await nextMessage(ws)).toEqual({ t: 'ia', seq: 1 });
+        expect(session.shouldApplyInput('c1', 1)).toBe(false);
+      } finally {
+        ws.close();
+      }
+    });
+
+    it('withholds the ACK and re-opens the seq when the write did not land', async () => {
+      // A session whose PTY is gone swallows the write. ACKing anyway told the
+      // client to drop the frame from its durable queue while the seq stayed
+      // burnt, so the retry that reliable delivery exists for was rejected as a
+      // duplicate — the input was lost for good.
+      const ws = await connectWs('/ws/sessions/ws-test-session/terminal');
+      try {
+        const session = ctx._session;
+        session.failWrites = true;
+
+        ws.send(JSON.stringify({ t: 'i', d: 'lost\r', cid: 'c1', seq: 1 }));
+
+        await expect(nextMessage(ws, 600)).rejects.toThrow(/timeout/);
+        expect(session.shouldApplyInput('c1', 1)).toBe(true);
+      } finally {
+        ws.close();
+      }
+    });
+
+    it('still ACKs a duplicate frame the server deliberately skipped', async () => {
+      // Dedup must stay silent-but-acknowledged: the client has to be able to
+      // drop a frame it already delivered once.
+      const ws = await connectWs('/ws/sessions/ws-test-session/terminal');
+      try {
+        const session = ctx._session;
+        session.shouldApplyInput('c1', 7); // pretend seq 7 already landed
+
+        ws.send(JSON.stringify({ t: 'i', d: 'again\r', cid: 'c1', seq: 7 }));
+
+        expect(await nextMessage(ws)).toEqual({ t: 'ia', seq: 7 });
+        expect(session.writeBuffer).not.toContain('again\r');
+      } finally {
+        ws.close();
+      }
+    });
+
     it('ignores input exceeding MAX_INPUT_LENGTH', async () => {
       const ws = await connectWs('/ws/sessions/ws-test-session/terminal');
       try {