fix(btw): stop persisting side questions

ngutman · ngutman · commit 7e821d4f02bb · 2026-03-14T18:23:22.000+02:00
diff --git a/src/agents/btw.test.ts b/src/agents/btw.test.ts
@@ -2,7 +2,6 @@ import { beforeEach, describe, expect, it, vi } from "vitest";
 import type { SessionEntry } from "../config/sessions.js";
 
 const streamSimpleMock = vi.fn();
-const appendCustomEntryMock = vi.fn();
 const buildSessionContextMock = vi.fn();
 const getLeafEntryMock = vi.fn();
 const branchMock = vi.fn();
@@ -13,11 +12,8 @@ const discoverModelsMock = vi.fn();
 const resolveModelWithRegistryMock = vi.fn();
 const getApiKeyForModelMock = vi.fn();
 const requireApiKeyMock = vi.fn();
-const acquireSessionWriteLockMock = vi.fn();
 const resolveSessionAuthProfileOverrideMock = vi.fn();
 const getActiveEmbeddedRunSnapshotMock = vi.fn();
-const waitForEmbeddedPiRunEndMock = vi.fn();
-const diagWarnMock = vi.fn();
 const diagDebugMock = vi.fn();
 
 vi.mock("@mariozechner/pi-ai", () => ({
@@ -31,7 +27,6 @@ vi.mock("@mariozechner/pi-coding-agent", () => ({
       branch: branchMock,
       resetLeaf: resetLeafMock,
       buildSessionContext: buildSessionContextMock,
-      appendCustomEntry: appendCustomEntryMock,
     }),
   },
 }));
@@ -54,13 +49,8 @@ vi.mock("./model-auth.js", () => ({
   requireApiKey: (...args: unknown[]) => requireApiKeyMock(...args),
 }));
 
-vi.mock("./session-write-lock.js", () => ({
-  acquireSessionWriteLock: (...args: unknown[]) => acquireSessionWriteLockMock(...args),
-}));
-
 vi.mock("./pi-embedded-runner/runs.js", () => ({
   getActiveEmbeddedRunSnapshot: (...args: unknown[]) => getActiveEmbeddedRunSnapshotMock(...args),
-  waitForEmbeddedPiRunEnd: (...args: unknown[]) => waitForEmbeddedPiRunEndMock(...args),
 }));
 
 vi.mock("./auth-profiles/session-override.js", () => ({
@@ -70,12 +60,11 @@ vi.mock("./auth-profiles/session-override.js", () => ({
 
 vi.mock("../logging/diagnostic.js", () => ({
   diagnosticLogger: {
-    warn: (...args: unknown[]) => diagWarnMock(...args),
     debug: (...args: unknown[]) => diagDebugMock(...args),
   },
 }));
 
-const { BTW_CUSTOM_TYPE, runBtwSideQuestion } = await import("./btw.js");
+const { runBtwSideQuestion } = await import("./btw.js");
 
 function makeAsyncEvents(events: unknown[]) {
   return {
@@ -99,7 +88,6 @@ function createSessionEntry(overrides: Partial<SessionEntry> = {}): SessionEntry
 describe("runBtwSideQuestion", () => {
   beforeEach(() => {
     streamSimpleMock.mockReset();
-    appendCustomEntryMock.mockReset();
     buildSessionContextMock.mockReset();
     getLeafEntryMock.mockReset();
     branchMock.mockReset();
@@ -110,11 +98,8 @@ describe("runBtwSideQuestion", () => {
     resolveModelWithRegistryMock.mockReset();
     getApiKeyForModelMock.mockReset();
     requireApiKeyMock.mockReset();
-    acquireSessionWriteLockMock.mockReset();
     resolveSessionAuthProfileOverrideMock.mockReset();
     getActiveEmbeddedRunSnapshotMock.mockReset();
-    waitForEmbeddedPiRunEndMock.mockReset();
-    diagWarnMock.mockReset();
     diagDebugMock.mockReset();
 
     buildSessionContextMock.mockReturnValue({
@@ -128,15 +113,11 @@ describe("runBtwSideQuestion", () => {
     });
     getApiKeyForModelMock.mockResolvedValue({ apiKey: "secret", mode: "api-key", source: "test" });
     requireApiKeyMock.mockReturnValue("secret");
-    acquireSessionWriteLockMock.mockResolvedValue({
-      release: vi.fn().mockResolvedValue(undefined),
-    });
     resolveSessionAuthProfileOverrideMock.mockResolvedValue("profile-1");
     getActiveEmbeddedRunSnapshotMock.mockReturnValue(undefined);
-    waitForEmbeddedPiRunEndMock.mockResolvedValue(true);
   });
 
-  it("streams blocks and persists a non-context custom entry", async () => {
+  it("streams blocks without persisting BTW data to disk", async () => {
     const onBlockReply = vi.fn().mockResolvedValue(undefined);
     streamSimpleMock.mockReturnValue(
       makeAsyncEvents([
@@ -212,17 +193,6 @@ describe("runBtwSideQuestion", () => {
       text: "Side answer.",
       btw: { question: "What changed?" },
     });
-    await vi.waitFor(() => {
-      expect(appendCustomEntryMock).toHaveBeenCalledWith(
-        BTW_CUSTOM_TYPE,
-        expect.objectContaining({
-          question: "What changed?",
-          answer: "Side answer.",
-          provider: "anthropic",
-          model: "claude-sonnet-4-5",
-        }),
-      );
-    });
   });
 
   it("returns a final payload when block streaming is unavailable", async () => {
@@ -641,14 +611,7 @@ describe("runBtwSideQuestion", () => {
     );
   });
 
-  it("returns the BTW answer and retries transcript persistence after a session lock", async () => {
-    acquireSessionWriteLockMock
-      .mockRejectedValueOnce(
-        new Error("session file locked (timeout 250ms): pid=123 /tmp/session.lock"),
-      )
-      .mockResolvedValueOnce({
-        release: vi.fn().mockResolvedValue(undefined),
-      });
+  it("returns the BTW answer without appending transcript custom entries", async () => {
     streamSimpleMock.mockReturnValue(
       makeAsyncEvents([
         {
@@ -688,26 +651,10 @@ describe("runBtwSideQuestion", () => {
     });
 
     expect(result).toEqual({ text: "323" });
-    await vi.waitFor(() => {
-      expect(waitForEmbeddedPiRunEndMock).toHaveBeenCalledWith("session-1", 30000);
-      expect(appendCustomEntryMock).toHaveBeenCalledWith(
-        BTW_CUSTOM_TYPE,
-        expect.objectContaining({
-          question: "What is 17 * 19?",
-          answer: "323",
-        }),
-      );
-    });
+    expect(buildSessionContextMock).toHaveBeenCalled();
   });
 
-  it("logs deferred persistence failures through the diagnostic logger", async () => {
-    acquireSessionWriteLockMock
-      .mockRejectedValueOnce(
-        new Error("session file locked (timeout 250ms): pid=123 /tmp/session.lock"),
-      )
-      .mockRejectedValueOnce(
-        new Error("session file locked (timeout 10000ms): pid=123 /tmp/session.lock"),
-      );
+  it("does not log transcript persistence warnings because BTW no longer writes to disk", async () => {
     streamSimpleMock.mockReturnValue(
       makeAsyncEvents([
         {
@@ -747,11 +694,9 @@ describe("runBtwSideQuestion", () => {
     });
 
     expect(result).toEqual({ text: "323" });
-    await vi.waitFor(() => {
-      expect(diagWarnMock).toHaveBeenCalledWith(
-        expect.stringContaining("btw transcript persistence skipped: sessionId=session-1"),
-      );
-    });
+    expect(diagDebugMock).not.toHaveBeenCalledWith(
+      expect.stringContaining("btw transcript persistence skipped"),
+    );
   });
 
   it("excludes tool results from BTW context to avoid replaying raw tool output", async () => {
diff --git a/src/agents/btw.ts b/src/agents/btw.ts
@@ -21,19 +21,10 @@ import { getApiKeyForModel, requireApiKey } from "./model-auth.js";
 import { ensureOpenClawModelsJson } from "./models-config.js";
 import { EmbeddedBlockChunker, type BlockReplyChunking } from "./pi-embedded-block-chunker.js";
 import { resolveModelWithRegistry } from "./pi-embedded-runner/model.js";
-import {
-  getActiveEmbeddedRunSnapshot,
-  waitForEmbeddedPiRunEnd,
-} from "./pi-embedded-runner/runs.js";
+import { getActiveEmbeddedRunSnapshot } from "./pi-embedded-runner/runs.js";
 import { mapThinkingLevel } from "./pi-embedded-runner/utils.js";
 import { discoverAuthStorage, discoverModels } from "./pi-model-discovery.js";
 import { stripToolResultDetails } from "./session-transcript-repair.js";
-import { acquireSessionWriteLock } from "./session-write-lock.js";
-
-const BTW_CUSTOM_TYPE = "openclaw:btw";
-const BTW_PERSIST_TIMEOUT_MS = 250;
-const BTW_PERSIST_RETRY_WAIT_MS = 30_000;
-const BTW_PERSIST_RETRY_LOCK_MS = 10_000;
 
 type SessionManagerLike = {
   getLeafEntry?: () => {
@@ -47,97 +38,6 @@ type SessionManagerLike = {
   buildSessionContext: () => { messages?: unknown[] };
 };
 
-type BtwCustomEntryData = {
-  timestamp: number;
-  question: string;
-  answer: string;
-  provider: string;
-  model: string;
-  thinkingLevel: ThinkLevel | "off";
-  reasoningLevel: ReasoningLevel;
-  sessionKey?: string;
-  authProfileId?: string;
-  authProfileIdSource?: "auto" | "user";
-  usage?: unknown;
-};
-
-async function appendBtwCustomEntry(params: {
-  sessionFile: string;
-  timeoutMs: number;
-  entry: BtwCustomEntryData;
-}) {
-  const lock = await acquireSessionWriteLock({
-    sessionFile: params.sessionFile,
-    timeoutMs: params.timeoutMs,
-    allowReentrant: false,
-  });
-  try {
-    const persisted = SessionManager.open(params.sessionFile);
-    persisted.appendCustomEntry(BTW_CUSTOM_TYPE, params.entry);
-  } finally {
-    await lock.release();
-  }
-}
-
-function isSessionLockError(error: unknown): boolean {
-  const message = error instanceof Error ? error.message : String(error);
-  return message.includes("session file locked");
-}
-
-function deferBtwCustomEntryPersist(params: {
-  sessionId: string;
-  sessionFile: string;
-  entry: BtwCustomEntryData;
-}) {
-  void (async () => {
-    try {
-      await waitForEmbeddedPiRunEnd(params.sessionId, BTW_PERSIST_RETRY_WAIT_MS);
-      await appendBtwCustomEntry({
-        sessionFile: params.sessionFile,
-        timeoutMs: BTW_PERSIST_RETRY_LOCK_MS,
-        entry: params.entry,
-      });
-    } catch (error) {
-      const message = error instanceof Error ? error.message : String(error);
-      diag.warn(`btw transcript persistence skipped: sessionId=${params.sessionId} err=${message}`);
-    }
-  })();
-}
-
-async function persistBtwCustomEntry(params: {
-  sessionId: string;
-  sessionFile: string;
-  entry: BtwCustomEntryData;
-}) {
-  try {
-    await appendBtwCustomEntry({
-      sessionFile: params.sessionFile,
-      timeoutMs: BTW_PERSIST_TIMEOUT_MS,
-      entry: params.entry,
-    });
-  } catch (error) {
-    if (!isSessionLockError(error)) {
-      throw error;
-    }
-    deferBtwCustomEntryPersist({
-      sessionId: params.sessionId,
-      sessionFile: params.sessionFile,
-      entry: params.entry,
-    });
-  }
-}
-
-function persistBtwCustomEntryInBackground(params: {
-  sessionId: string;
-  sessionFile: string;
-  entry: BtwCustomEntryData;
-}) {
-  void persistBtwCustomEntry(params).catch((error) => {
-    const message = error instanceof Error ? error.message : String(error);
-    diag.warn(`btw transcript persistence skipped: sessionId=${params.sessionId} err=${message}`);
-  });
-}
-
 function collectTextContent(content: Array<{ type?: string; text?: string }>): string {
   return content
     .filter((part): part is { type: "text"; text: string } => part.type === "text")
@@ -347,7 +247,7 @@ export async function runBtwSideQuestion(
     throw new Error("No active session context.");
   }
 
-  const { model, authProfileId, authProfileIdSource } = await resolveRuntimeModel({
+  const { model, authProfileId } = await resolveRuntimeModel({
     cfg: params.cfg,
     provider: params.provider,
     model: params.model,
@@ -483,31 +383,9 @@ export async function runBtwSideQuestion(
     throw new Error("No BTW response generated.");
   }
 
-  const customEntry = {
-    timestamp: Date.now(),
-    question: params.question,
-    answer,
-    provider: model.provider,
-    model: model.id,
-    thinkingLevel: params.resolvedThinkLevel ?? "off",
-    reasoningLevel: params.resolvedReasoningLevel,
-    sessionKey: params.sessionKey,
-    authProfileId,
-    authProfileIdSource,
-    usage: finalMessage?.usage,
-  } satisfies BtwCustomEntryData;
-
-  persistBtwCustomEntryInBackground({
-    sessionId,
-    sessionFile,
-    entry: customEntry,
-  });
-
   if (emittedBlocks > 0) {
     return undefined;
   }
 
   return { text: answer };
 }
-
-export { BTW_CUSTOM_TYPE };
