fix(agent): gate and log /btw reviews

This commit is contained in:
Nimrod Gutman
2026-03-13 22:45:35 +02:00
parent db956d59d8
commit d5aaf6802f
5 changed files with 91 additions and 2 deletions

View File

@@ -17,6 +17,7 @@ const acquireSessionWriteLockMock = vi.fn();
const resolveSessionAuthProfileOverrideMock = vi.fn();
const getActiveEmbeddedRunSnapshotMock = vi.fn();
const waitForEmbeddedPiRunEndMock = vi.fn();
const diagWarnMock = vi.fn();
vi.mock("@mariozechner/pi-ai", () => ({
streamSimple: (...args: unknown[]) => streamSimpleMock(...args),
@@ -66,6 +67,12 @@ vi.mock("./auth-profiles/session-override.js", () => ({
resolveSessionAuthProfileOverrideMock(...args),
}));
vi.mock("../logging/diagnostic.js", () => ({
diagnosticLogger: {
warn: (...args: unknown[]) => diagWarnMock(...args),
},
}));
const { BTW_CUSTOM_TYPE, runBtwSideQuestion } = await import("./btw.js");
function makeAsyncEvents(events: unknown[]) {
@@ -105,6 +112,7 @@ describe("runBtwSideQuestion", () => {
resolveSessionAuthProfileOverrideMock.mockReset();
getActiveEmbeddedRunSnapshotMock.mockReset();
waitForEmbeddedPiRunEndMock.mockReset();
diagWarnMock.mockReset();
buildSessionContextMock.mockReturnValue({
messages: [{ role: "user", content: [{ type: "text", text: "hi" }], timestamp: 1 }],
@@ -425,4 +433,58 @@ describe("runBtwSideQuestion", () => {
);
});
});
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"),
);
streamSimpleMock.mockReturnValue(
makeAsyncEvents([
{
type: "done",
reason: "stop",
message: {
role: "assistant",
content: [{ type: "text", text: "323" }],
provider: "anthropic",
api: "anthropic-messages",
model: "claude-sonnet-4-5",
stopReason: "stop",
usage: {
input: 1,
output: 2,
cacheRead: 0,
cacheWrite: 0,
totalTokens: 3,
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 },
},
timestamp: Date.now(),
},
},
]),
);
const result = await runBtwSideQuestion({
cfg: {} as never,
agentDir: "/tmp/agent",
provider: "anthropic",
model: "claude-sonnet-4-5",
question: "What is 17 * 19?",
sessionEntry: createSessionEntry(),
resolvedReasoningLevel: "off",
opts: {},
isNewSession: false,
});
expect(result).toEqual({ text: "323" });
await vi.waitFor(() => {
expect(diagWarnMock).toHaveBeenCalledWith(
expect.stringContaining("btw transcript persistence skipped: sessionId=session-1"),
);
});
});
});

View File

@@ -15,6 +15,7 @@ import {
resolveSessionFilePathOptions,
type SessionEntry,
} from "../config/sessions.js";
import { diagnosticLogger as diag } from "../logging/diagnostic.js";
import { resolveSessionAuthProfileOverride } from "./auth-profiles/session-override.js";
import { getApiKeyForModel, requireApiKey } from "./model-auth.js";
import { ensureOpenClawModelsJson } from "./models-config.js";
@@ -97,7 +98,7 @@ function deferBtwCustomEntryPersist(params: {
});
} catch (error) {
const message = error instanceof Error ? error.message : String(error);
console.warn(`[btw] skipped transcript persistence: ${message}`);
diag.warn(`btw transcript persistence skipped: sessionId=${params.sessionId} err=${message}`);
}
})();
}

View File

@@ -32,6 +32,23 @@ describe("handleBtwCommand", () => {
});
});
it("ignores /btw when text commands are disabled", async () => {
const result = await handleBtwCommand(buildParams("/btw what changed?"), false);
expect(result).toBeNull();
expect(runBtwSideQuestionMock).not.toHaveBeenCalled();
});
it("ignores /btw from unauthorized senders", async () => {
const params = buildParams("/btw what changed?");
params.command.isAuthorizedSender = false;
const result = await handleBtwCommand(params, true);
expect(result).toEqual({ shouldContinue: false });
expect(runBtwSideQuestionMock).not.toHaveBeenCalled();
});
it("requires an active session context", async () => {
const params = buildParams("/btw what changed?");
params.sessionEntry = undefined;

View File

@@ -1,13 +1,21 @@
import { runBtwSideQuestion } from "../../agents/btw.js";
import { rejectUnauthorizedCommand } from "./command-gates.js";
import type { CommandHandler } from "./commands-types.js";
const BTW_USAGE = "Usage: /btw <side question>";
export const handleBtwCommand: CommandHandler = async (params) => {
export const handleBtwCommand: CommandHandler = async (params, allowTextCommands) => {
if (!allowTextCommands) {
return null;
}
const match = params.command.commandBodyNormalized.match(/^\/btw(?:\s+(.*))?$/i);
if (!match) {
return null;
}
const unauthorized = rejectUnauthorizedCommand(params, "/btw");
if (unauthorized) {
return unauthorized;
}
const question = match[1]?.trim() ?? "";
if (!question) {

View File

@@ -37,6 +37,7 @@ function getBuiltinSlashCommands(): Set<string> {
return builtinSlashCommands;
}
builtinSlashCommands = listReservedChatSlashCommandNames([
"btw",
"think",
"verbose",
"reasoning",