Revert "🤖 Remove interrupt sentinel - models resume naturally (#133)"

ammario · ammario · commit df5673e87e8f · 2025-10-10T14:33:49.000-05:00
This reverts commit e732a87.
diff --git a/src/services/aiService.ts b/src/services/aiService.ts
@@ -19,6 +19,7 @@ import { log } from "./log";
 import {
   transformModelMessages,
   validateAnthropicCompliance,
+  addInterruptedSentinel,
   filterEmptyAssistantMessages,
 } from "@/utils/messages/modelMessageTransform";
 import { applyCacheControl } from "@/utils/ai/cacheStrategy";
@@ -439,10 +440,13 @@ export class AIService extends EventEmitter {
         log.debug("Keeping reasoning parts for OpenAI (fetch wrapper handles item_references)");
       }
 
+      // Add [INTERRUPTED] sentinel to partial messages (for model context)
+      const messagesWithSentinel = addInterruptedSentinel(filteredMessages);
+
       // Convert CmuxMessage to ModelMessage format using Vercel AI SDK utility
       // Type assertion needed because CmuxMessage has custom tool parts for interrupted tools
       // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-argument
-      const modelMessages = convertToModelMessages(filteredMessages as any);
+      const modelMessages = convertToModelMessages(messagesWithSentinel as any);
       log.debug_obj(`${workspaceId}/2_model_messages.json`, modelMessages);
 
       // Apply ModelMessage transforms based on provider requirements
diff --git a/src/utils/messages/modelMessageTransform.test.ts b/src/utils/messages/modelMessageTransform.test.ts
@@ -1,6 +1,11 @@
 import { describe, it, expect } from "@jest/globals";
 import type { ModelMessage, AssistantModelMessage, ToolModelMessage } from "ai";
-import { transformModelMessages, validateAnthropicCompliance } from "./modelMessageTransform";
+import {
+  transformModelMessages,
+  validateAnthropicCompliance,
+  addInterruptedSentinel,
+} from "./modelMessageTransform";
+import type { CmuxMessage } from "@/types/message";
 
 describe("modelMessageTransform", () => {
   describe("transformModelMessages", () => {
@@ -396,6 +401,122 @@ describe("modelMessageTransform", () => {
     });
   });
 
+  describe("addInterruptedSentinel", () => {
+    it("should insert user message after partial assistant message", () => {
+      const messages: CmuxMessage[] = [
+        {
+          id: "user-1",
+          role: "user",
+          parts: [{ type: "text", text: "Hello" }],
+          metadata: { timestamp: 1000 },
+        },
+        {
+          id: "assistant-1",
+          role: "assistant",
+          parts: [{ type: "text", text: "Let me help..." }],
+          metadata: { timestamp: 2000, partial: true },
+        },
+      ];
+
+      const result = addInterruptedSentinel(messages);
+
+      // Should have 3 messages: user, assistant, [INTERRUPTED] user
+      expect(result).toHaveLength(3);
+      expect(result[0].id).toBe("user-1");
+      expect(result[1].id).toBe("assistant-1");
+      expect(result[2].id).toBe("interrupted-assistant-1");
+      expect(result[2].role).toBe("user");
+      expect(result[2].parts).toEqual([{ type: "text", text: "[INTERRUPTED]" }]);
+      expect(result[2].metadata?.synthetic).toBe(true);
+      expect(result[2].metadata?.timestamp).toBe(2000);
+    });
+
+    it("should not insert sentinel for non-partial assistant messages", () => {
+      const messages: CmuxMessage[] = [
+        {
+          id: "user-1",
+          role: "user",
+          parts: [{ type: "text", text: "Hello" }],
+          metadata: { timestamp: 1000 },
+        },
+        {
+          id: "assistant-1",
+          role: "assistant",
+          parts: [{ type: "text", text: "Complete response" }],
+          metadata: { timestamp: 2000, partial: false },
+        },
+      ];
+
+      const result = addInterruptedSentinel(messages);
+
+      // Should remain unchanged (no sentinel)
+      expect(result).toHaveLength(2);
+      expect(result).toEqual(messages);
+    });
+
+    it("should insert sentinel for reasoning-only partial messages", () => {
+      const messages: CmuxMessage[] = [
+        {
+          id: "user-1",
+          role: "user",
+          parts: [{ type: "text", text: "Calculate something" }],
+          metadata: { timestamp: 1000 },
+        },
+        {
+          id: "assistant-1",
+          role: "assistant",
+          parts: [{ type: "reasoning", text: "Let me think about this..." }],
+          metadata: { timestamp: 2000, partial: true },
+        },
+      ];
+
+      const result = addInterruptedSentinel(messages);
+
+      // Should have 3 messages: user, assistant (reasoning only), [INTERRUPTED] user
+      expect(result).toHaveLength(3);
+      expect(result[2].role).toBe("user");
+      expect(result[2].parts).toEqual([{ type: "text", text: "[INTERRUPTED]" }]);
+    });
+
+    it("should handle multiple partial messages", () => {
+      const messages: CmuxMessage[] = [
+        {
+          id: "user-1",
+          role: "user",
+          parts: [{ type: "text", text: "First" }],
+          metadata: { timestamp: 1000 },
+        },
+        {
+          id: "assistant-1",
+          role: "assistant",
+          parts: [{ type: "text", text: "Response 1..." }],
+          metadata: { timestamp: 2000, partial: true },
+        },
+        {
+          id: "user-2",
+          role: "user",
+          parts: [{ type: "text", text: "Second" }],
+          metadata: { timestamp: 3000 },
+        },
+        {
+          id: "assistant-2",
+          role: "assistant",
+          parts: [{ type: "text", text: "Response 2..." }],
+          metadata: { timestamp: 4000, partial: true },
+        },
+      ];
+
+      const result = addInterruptedSentinel(messages);
+
+      // Should have 6 messages (4 original + 2 sentinels)
+      expect(result).toHaveLength(6);
+      expect(result[2].id).toBe("interrupted-assistant-1");
+      expect(result[2].role).toBe("user");
+      expect(result[5].id).toBe("interrupted-assistant-2");
+      expect(result[5].role).toBe("user");
+    });
+  });
+
   describe("reasoning part stripping for OpenAI", () => {
     it("should strip reasoning parts for OpenAI provider", () => {
       const messages: ModelMessage[] = [
diff --git a/src/utils/messages/modelMessageTransform.ts b/src/utils/messages/modelMessageTransform.ts
@@ -61,6 +61,40 @@ export function stripReasoningForOpenAI(messages: CmuxMessage[]): CmuxMessage[]
   });
 }
 
+/**
+ * Add [INTERRUPTED] sentinel to partial messages by inserting a user message.
+ * This helps the model understand that a message was interrupted and incomplete.
+ * The sentinel is ONLY for model context, not shown in UI.
+ *
+ * We insert a separate user message instead of modifying the assistant message
+ * because if the assistant message only has reasoning (no text), it will be
+ * filtered out, and we'd lose the interruption context. A user message always
+ * survives filtering.
+ */
+export function addInterruptedSentinel(messages: CmuxMessage[]): CmuxMessage[] {
+  const result: CmuxMessage[] = [];
+
+  for (const msg of messages) {
+    result.push(msg);
+
+    // If this is a partial assistant message, insert [INTERRUPTED] user message after it
+    if (msg.role === "assistant" && msg.metadata?.partial) {
+      result.push({
+        id: `interrupted-${msg.id}`,
+        role: "user",
+        parts: [{ type: "text", text: "[INTERRUPTED]" }],
+        metadata: {
+          timestamp: msg.metadata.timestamp,
+          // Mark as synthetic so it can be identified if needed
+          synthetic: true,
+        },
+      });
+    }
+  }
+
+  return result;
+}
+
 /**
  * Split assistant messages with mixed text and tool calls into separate messages
  * to comply with Anthropic's requirement that tool_use blocks must be immediately
diff --git a/tests/ipcMain/resumeStream.test.ts b/tests/ipcMain/resumeStream.test.ts
@@ -14,7 +14,7 @@ const describeIntegration = shouldRunIntegrationTests() ? describe : describe.sk
 
 // Validate API keys before running tests
 if (shouldRunIntegrationTests()) {
-  validateApiKeys(["ANTHROPIC_API_KEY", "OPENAI_API_KEY"]);
+  validateApiKeys(["ANTHROPIC_API_KEY"]);
 }
 
 describeIntegration("IpcMain resumeStream integration tests", () => {
@@ -23,29 +23,19 @@ describeIntegration("IpcMain resumeStream integration tests", () => {
     jest.retryTimes(3, { logErrorsBeforeRetry: true });
   }
 
-  test.concurrent.each([
-    {
-      provider: "anthropic" as const,
-      model: "claude-sonnet-4-5",
-      expectedWord: "RESUMPTION_TEST_SUCCESS",
-    },
-    {
-      provider: "openai" as const,
-      model: "gpt-4o",
-      expectedWord: "RESUMPTION_TEST_OPENAI_SUCCESS",
-    },
-  ])(
-    "should resume interrupted stream without new user message ($provider)",
-    async ({ provider, model, expectedWord }) => {
-      const { env, workspaceId, cleanup } = await setupWorkspace(provider);
+  test.concurrent(
+    "should resume interrupted stream without new user message",
+    async () => {
+      const { env, workspaceId, cleanup } = await setupWorkspace("anthropic");
       try {
         // Start a stream with a bash command that outputs a specific word
+        const expectedWord = "RESUMPTION_TEST_SUCCESS";
         void sendMessageWithModel(
           env.mockIpcRenderer,
           workspaceId,
           `Run this bash command: sleep 5 && echo '${expectedWord}'`,
-          provider,
-          model
+          "anthropic",
+          "claude-sonnet-4-5"
         );
 
         // Wait for stream to start
@@ -70,8 +60,8 @@ describeIntegration("IpcMain resumeStream integration tests", () => {
           env.mockIpcRenderer,
           workspaceId,
           "",
-          provider,
-          model
+          "anthropic",
+          "claude-sonnet-4-5"
         );
         expect(interruptResult.success).toBe(true);
 
@@ -100,7 +90,7 @@ describeIntegration("IpcMain resumeStream integration tests", () => {
         const resumeResult = (await env.mockIpcRenderer.invoke(
           IPC_CHANNELS.WORKSPACE_RESUME_STREAM,
           workspaceId,
-          { model: `${provider}:${model}` }
+          { model: "anthropic:claude-sonnet-4-5" }
         )) as Result<void, SendMessageError>;
         expect(resumeResult.success).toBe(true);
 
