Merge b32201979b into c4265a5f16

src/agents/pi-extensions/compaction-safeguard.test.ts

@@ -40,6 +40,9 @@ const {
   computeAdaptiveChunkRatio,
   isOversizedForSummary,
   readWorkspaceContextForSummary,
+  isRealConversationMessage,
+  hasMeaningfulText,
+  hasNearbyMeaningfulUserMessage,
   BASE_CHUNK_RATIO,
   MIN_CHUNK_RATIO,
   SAFETY_MARGIN,
@@ -1583,3 +1586,231 @@ describe("readWorkspaceContextForSummary", () => {
     },
   );
 });
+
+// ---------------------------------------------------------------------------
+// isRealConversationMessage — content-aware checks (issue #40727)
+// ---------------------------------------------------------------------------
+
+describe("isRealConversationMessage", () => {
+  it("accepts a user message with meaningful text", () => {
+    const msg = castAgentMessage({ role: "user", content: "Fix the bug", timestamp: 0 });
+    expect(isRealConversationMessage(msg)).toBe(true);
+  });
+
+  it("accepts an assistant message with meaningful text", () => {
+    const msg = castAgentMessage({
+      role: "assistant",
+      content: [{ type: "text", text: "Here is the fix" }],
+      timestamp: 0,
+    });
+    expect(isRealConversationMessage(msg)).toBe(true);
+  });
+
+  it("rejects a user message with empty content", () => {
+    const msg = castAgentMessage({ role: "user", content: "", timestamp: 0 });
+    expect(isRealConversationMessage(msg)).toBe(false);
+  });
+
+  it("rejects an assistant message that is only HEARTBEAT_OK", () => {
+    const msg = castAgentMessage({
+      role: "assistant",
+      content: "HEARTBEAT_OK",
+      timestamp: 0,
+    });
+    expect(isRealConversationMessage(msg)).toBe(false);
+  });
+
+  it("rejects an assistant message that is only NO_REPLY", () => {
+    const msg = castAgentMessage({
+      role: "assistant",
+      content: "NO_REPLY",
+      timestamp: 0,
+    });
+    expect(isRealConversationMessage(msg)).toBe(false);
+  });
+
+  it("rejects a user message with only whitespace", () => {
+    const msg = castAgentMessage({ role: "user", content: "   \n  ", timestamp: 0 });
+    expect(isRealConversationMessage(msg)).toBe(false);
+  });
+
+  it("accepts an assistant message with HEARTBEAT_OK embedded in real text", () => {
+    const msg = castAgentMessage({
+      role: "assistant",
+      content: "Status: HEARTBEAT_OK — also deployed v2",
+      timestamp: 0,
+    });
+    expect(isRealConversationMessage(msg)).toBe(true);
+  });
+
+  it("rejects a system message", () => {
+    const msg = castAgentMessage({ role: "system", content: "You are helpful", timestamp: 0 });
+    expect(isRealConversationMessage(msg)).toBe(false);
+  });
+
+  it("accepts a toolResult when no window is provided (conservative)", () => {
+    const msg = castAgentMessage({
+      role: "toolResult",
+      toolCallId: "tc1",
+      toolName: "read_file",
+      content: "file contents",
+      isError: false,
+      timestamp: 0,
+    });
+    expect(isRealConversationMessage(msg)).toBe(true);
+  });
+
+  it("accepts a toolResult with a nearby meaningful user message", () => {
+    const window: AgentMessage[] = [
+      castAgentMessage({ role: "user", content: "Please read main.ts", timestamp: 0 }),
+      castAgentMessage({
+        role: "assistant",
+        content: [{ type: "text", text: "Reading..." }],
+        timestamp: 0,
+      }),
+      castAgentMessage({
+        role: "toolResult",
+        toolCallId: "tc1",
+        toolName: "read_file",
+        content: "file data",
+        isError: false,
+        timestamp: 0,
+      }),
+    ];
+    expect(isRealConversationMessage(window[2], window, 2)).toBe(true);
+  });
+
+  it("rejects a toolResult when nearby user messages are heartbeat-only", () => {
+    const window: AgentMessage[] = [
+      castAgentMessage({ role: "user", content: "", timestamp: 0 }),
+      castAgentMessage({
+        role: "assistant",
+        content: "HEARTBEAT_OK",
+        timestamp: 0,
+      }),
+      castAgentMessage({
+        role: "toolResult",
+        toolCallId: "tc1",
+        toolName: "heartbeat_check",
+        content: "ok",
+        isError: false,
+        timestamp: 0,
+      }),
+    ];
+    expect(isRealConversationMessage(window[2], window, 2)).toBe(false);
+  });
+});
+
+// ---------------------------------------------------------------------------
+// hasMeaningfulText
+// ---------------------------------------------------------------------------
+
+describe("hasMeaningfulText", () => {
+  it("returns true for normal text", () => {
+    const msg = castAgentMessage({ role: "user", content: "Hello world", timestamp: 0 });
+    expect(hasMeaningfulText(msg)).toBe(true);
+  });
+
+  it("returns false for empty string", () => {
+    const msg = castAgentMessage({ role: "user", content: "", timestamp: 0 });
+    expect(hasMeaningfulText(msg)).toBe(false);
+  });
+
+  it("returns false for HEARTBEAT_OK only", () => {
+    const msg = castAgentMessage({ role: "assistant", content: "HEARTBEAT_OK", timestamp: 0 });
+    expect(hasMeaningfulText(msg)).toBe(false);
+  });
+
+  it("returns false for NO_REPLY only", () => {
+    const msg = castAgentMessage({ role: "assistant", content: "NO_REPLY", timestamp: 0 });
+    expect(hasMeaningfulText(msg)).toBe(false);
+  });
+
+  it("returns true when boilerplate is mixed with real text", () => {
+    const msg = castAgentMessage({
+      role: "assistant",
+      content: "Done. NO_REPLY",
+      timestamp: 0,
+    });
+    expect(hasMeaningfulText(msg)).toBe(true);
+  });
+
+  it("returns false for array content with no text blocks", () => {
+    const msg = castAgentMessage({
+      role: "assistant",
+      content: [{ type: "image", source: "data" }],
+      timestamp: 0,
+    });
+    expect(hasMeaningfulText(msg)).toBe(false);
+  });
+});
+
+// ---------------------------------------------------------------------------
+// hasNearbyMeaningfulUserMessage
+// ---------------------------------------------------------------------------
+
+describe("hasNearbyMeaningfulUserMessage", () => {
+  it("finds a meaningful user message within lookback window", () => {
+    const window: AgentMessage[] = [
+      castAgentMessage({ role: "user", content: "Do something", timestamp: 0 }),
+      castAgentMessage({ role: "assistant", content: "ok", timestamp: 0 }),
+      castAgentMessage({
+        role: "toolResult",
+        toolCallId: "tc1",
+        toolName: "x",
+        content: "y",
+        isError: false,
+        timestamp: 0,
+      }),
+    ];
+    expect(hasNearbyMeaningfulUserMessage(window, 2)).toBe(true);
+  });
+
+  it("returns false when user messages are empty", () => {
+    const window: AgentMessage[] = [
+      castAgentMessage({ role: "user", content: "", timestamp: 0 }),
+      castAgentMessage({
+        role: "toolResult",
+        toolCallId: "tc1",
+        toolName: "x",
+        content: "y",
+        isError: false,
+        timestamp: 0,
+      }),
+    ];
+    expect(hasNearbyMeaningfulUserMessage(window, 1)).toBe(false);
+  });
+
+  it("returns false when meaningful user message is beyond lookback", () => {
+    const window: AgentMessage[] = [
+      castAgentMessage({ role: "user", content: "Real message", timestamp: 0 }),
+      // 6 filler messages (beyond lookback of 5)
+      ...Array.from({ length: 6 }, () =>
+        castAgentMessage({ role: "assistant", content: "HEARTBEAT_OK", timestamp: 0 }),
+      ),
+      castAgentMessage({
+        role: "toolResult",
+        toolCallId: "tc1",
+        toolName: "x",
+        content: "y",
+        isError: false,
+        timestamp: 0,
+      }),
+    ];
+    expect(hasNearbyMeaningfulUserMessage(window, 7)).toBe(false);
+  });
+
+  it("returns false when index is 0 (no preceding messages)", () => {
+    const window: AgentMessage[] = [
+      castAgentMessage({
+        role: "toolResult",
+        toolCallId: "tc1",
+        toolName: "x",
+        content: "y",
+        isError: false,
+        timestamp: 0,
+      }),
+    ];
+    expect(hasNearbyMeaningfulUserMessage(window, 0)).toBe(false);
+  });
+});

src/agents/pi-extensions/compaction-safeguard.ts

@@ -179,8 +179,72 @@ function formatToolFailuresSection(failures: ToolFailure[]): string {
   return `\n\n## Tool Failures\n${lines.join("\n")}`;
 }
 
-function isRealConversationMessage(message: AgentMessage): boolean {
-  return message.role === "user" || message.role === "assistant" || message.role === "toolResult";
+/** Boilerplate tokens that do not represent real user/assistant conversation. */
+const BOILERPLATE_TOKENS = new Set(["HEARTBEAT_OK", "NO_REPLY"]);
+
+/**
+ * Returns `true` when `message` represents genuine conversation content that
+ * justifies keeping a compaction window.
+ *
+ * The old implementation only checked `role`, which let heartbeat polls
+ * (empty user content) and HEARTBEAT_OK / NO_REPLY assistant replies slip
+ * through — see issue #40727.
+ *
+ * Rules:
+ *  • user / assistant — must contain meaningful text (non-empty after trimming
+ *    boilerplate tokens).
+ *  • toolResult — only counts when a nearby user message in the same window
+ *    carries meaningful text (prevents heartbeat-only tool-use chains from
+ *    being treated as real conversation).
+ */
+function isRealConversationMessage(
+  message: AgentMessage,
+  window?: AgentMessage[],
+  index?: number,
+): boolean {
+  if (message.role === "user" || message.role === "assistant") {
+    return hasMeaningfulText(message);
+  }
+  if (message.role === "toolResult") {
+    // Without a surrounding window we cannot verify context — be conservative
+    // and accept the message.
+    if (!window || index === undefined) {
+      return true;
+    }
+    return hasNearbyMeaningfulUserMessage(window, index);
+  }
+  return false;
+}
+
+/** Check whether a message carries non-boilerplate text content. */
+function hasMeaningfulText(message: AgentMessage): boolean {
+  const text = extractMessageText(message);
+  if (text.length === 0) {
+    return false;
+  }
+  // Strip known boilerplate tokens and see if anything remains.
+  let stripped = text;
+  for (const token of BOILERPLATE_TOKENS) {
+    stripped = stripped.replaceAll(token, "");
+  }
+  return stripped.trim().length > 0;
+}
+
+/**
+ * Scan backwards (up to 5 messages) from `index` looking for a user message
+ * that contains meaningful text.  This lets tool-result messages inherit
+ * "realness" from the user turn that triggered them.
+ */
+function hasNearbyMeaningfulUserMessage(window: AgentMessage[], index: number): boolean {
+  const lookback = 5;
+  const start = Math.max(0, index - lookback);
+  for (let i = index - 1; i >= start; i--) {
+    const msg = window[i];
+    if (msg.role === "user" && hasMeaningfulText(msg)) {
+      return true;
+    }
+  }
+  return false;
 }
 
 function computeFileLists(fileOps: FileOperations): {
@@ -702,7 +766,8 @@ async function readWorkspaceContextForSummary(): Promise<string> {
 export default function compactionSafeguardExtension(api: ExtensionAPI): void {
   api.on("session_before_compact", async (event, ctx) => {
     const { preparation, customInstructions: eventInstructions, signal } = event;
-    if (!preparation.messagesToSummarize.some(isRealConversationMessage)) {
+    const allMessages = [...preparation.turnPrefixMessages, ...preparation.messagesToSummarize];
+    if (!allMessages.some((msg, idx, arr) => isRealConversationMessage(msg, arr, idx))) {
       log.warn(
         "Compaction safeguard: cancelling compaction with no real conversation messages to summarize.",
       );
@@ -1005,6 +1070,9 @@ export const __testing = {
   computeAdaptiveChunkRatio,
   isOversizedForSummary,
   readWorkspaceContextForSummary,
+  isRealConversationMessage,
+  hasMeaningfulText,
+  hasNearbyMeaningfulUserMessage,
   BASE_CHUNK_RATIO,
   MIN_CHUNK_RATIO,
   SAFETY_MARGIN,