feat: persist thinking level default per agent
When using /think <level>, the thinking level is now persisted to the agent's config entry (thinkingDefault field). This ensures the thinking level survives session resets (/new, /reset) for the same agent. Changes: - Added thinkingDefault field to AgentConfig type - Added thinkingDefault to AgentEntrySchema (zod validation) - Added thinkingDefault to ResolvedAgentConfig in agent-scope.ts - Added persistAgentThinkingDefault() helper in directive-handling.impl.ts Implementation improvements (based on code review): - Fire-and-forget: config write doesn't block the /think response - Uses parsed config (preserves user's JSON structure) - Uses normalizeAgentId for consistent agent ID matching - Explicitly sets 'off' value instead of deleting the field - Logs warnings on write failures (best-effort, non-blocking) - Does NOT persist runtime downgrades (e.g., xhigh -> high fallback) to preserve user's intent when model capabilities change Includes unit tests for persistence behavior.
This commit is contained in:
parent
035ece4732
commit
be0ad69efb
@ -20,6 +20,7 @@ type ResolvedAgentConfig = {
|
||||
workspace?: string;
|
||||
agentDir?: string;
|
||||
model?: AgentEntry["model"];
|
||||
thinkingDefault?: AgentEntry["thinkingDefault"];
|
||||
memorySearch?: AgentEntry["memorySearch"];
|
||||
humanDelay?: AgentEntry["humanDelay"];
|
||||
heartbeat?: AgentEntry["heartbeat"];
|
||||
@ -103,6 +104,7 @@ export function resolveAgentConfig(
|
||||
typeof entry.model === "string" || (entry.model && typeof entry.model === "object")
|
||||
? entry.model
|
||||
: undefined,
|
||||
thinkingDefault: entry.thinkingDefault,
|
||||
memorySearch: entry.memorySearch,
|
||||
humanDelay: entry.humanDelay,
|
||||
heartbeat: entry.heartbeat,
|
||||
|
||||
@ -6,9 +6,11 @@ import {
|
||||
import type { ModelAliasIndex } from "../../agents/model-selection.js";
|
||||
import { resolveSandboxRuntimeStatus } from "../../agents/sandbox.js";
|
||||
import type { MoltbotConfig } from "../../config/config.js";
|
||||
import { createConfigIO } from "../../config/io.js";
|
||||
import { type SessionEntry, updateSessionStore } from "../../config/sessions.js";
|
||||
import type { ExecAsk, ExecHost, ExecSecurity } from "../../infra/exec-approvals.js";
|
||||
import { enqueueSystemEvent } from "../../infra/system-events.js";
|
||||
import { normalizeAgentId } from "../../routing/session-key.js";
|
||||
import { applyVerboseOverride } from "../../sessions/level-overrides.js";
|
||||
import { applyModelOverrideToSessionEntry } from "../../sessions/model-overrides.js";
|
||||
import { formatThinkingLevels, formatXHighModelHint, supportsXHighThinking } from "../thinking.js";
|
||||
@ -59,6 +61,59 @@ function resolveExecDefaults(params: {
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Persists the thinking level default to the agent's config entry.
|
||||
* This ensures the thinking level persists across session resets for the same agent.
|
||||
*
|
||||
* This function is fire-and-forget to avoid blocking the directive response.
|
||||
* It reads the raw config (preserving user formatting) and only updates the
|
||||
* specific agent's thinkingDefault field.
|
||||
*/
|
||||
function persistAgentThinkingDefault(agentId: string, thinkLevel: ThinkLevel): void {
|
||||
// Validate agentId
|
||||
if (!agentId?.trim()) return;
|
||||
const normalizedId = normalizeAgentId(agentId);
|
||||
|
||||
// Fire-and-forget: don't block the response
|
||||
void (async () => {
|
||||
try {
|
||||
const io = createConfigIO();
|
||||
const snapshot = await io.readConfigFileSnapshot();
|
||||
|
||||
// Use parsed (raw JSON) to preserve user's config structure
|
||||
if (!snapshot.exists || !snapshot.parsed) return;
|
||||
const cfg = snapshot.parsed as MoltbotConfig;
|
||||
|
||||
const agents = cfg.agents?.list;
|
||||
if (!Array.isArray(agents)) return;
|
||||
|
||||
// Use normalizeAgentId for consistent matching with rest of codebase
|
||||
const agentIndex = agents.findIndex(
|
||||
(a) => a && typeof a === "object" && normalizeAgentId(a.id) === normalizedId,
|
||||
);
|
||||
if (agentIndex === -1) return;
|
||||
|
||||
const agent = agents[agentIndex];
|
||||
if (!agent) return;
|
||||
|
||||
// Skip if already set to the same value (avoid unnecessary writes)
|
||||
if (agent.thinkingDefault === thinkLevel) return;
|
||||
|
||||
// Explicitly set the value (including "off") for clarity
|
||||
// This ensures the user's intent is always persisted
|
||||
agent.thinkingDefault = thinkLevel;
|
||||
|
||||
await io.writeConfigFile(cfg);
|
||||
} catch (err) {
|
||||
// Log warning but don't fail - this is best-effort persistence
|
||||
console.warn(
|
||||
`[directive-handling] Failed to persist thinkingDefault for agent "${agentId}":`,
|
||||
err instanceof Error ? err.message : err,
|
||||
);
|
||||
}
|
||||
})();
|
||||
}
|
||||
|
||||
export async function handleDirectiveOnly(params: {
|
||||
cfg: MoltbotConfig;
|
||||
directives: InlineDirectives;
|
||||
@ -304,8 +359,13 @@ export async function handleDirectiveOnly(params: {
|
||||
if (directives.hasThinkDirective && directives.thinkLevel) {
|
||||
if (directives.thinkLevel === "off") delete sessionEntry.thinkingLevel;
|
||||
else sessionEntry.thinkingLevel = directives.thinkLevel;
|
||||
// Persist user's explicit choice to agent config (fire-and-forget)
|
||||
// This survives session resets (/new, /reset)
|
||||
persistAgentThinkingDefault(activeAgentId, directives.thinkLevel);
|
||||
}
|
||||
if (shouldDowngradeXHigh) {
|
||||
// Runtime downgrade only affects session, NOT persisted config
|
||||
// User's preference (xhigh) is preserved for when model supports it
|
||||
sessionEntry.thinkingLevel = "high";
|
||||
}
|
||||
if (directives.hasVerboseDirective && directives.verboseLevel) {
|
||||
|
||||
@ -0,0 +1,251 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type { MoltbotConfig } from "../../config/config.js";
|
||||
|
||||
// Track calls to writeConfigFile
|
||||
const writeConfigFileMock = vi.fn();
|
||||
const readConfigFileSnapshotMock = vi.fn();
|
||||
|
||||
// Mock the config IO module
|
||||
vi.mock("../../config/io.js", () => ({
|
||||
createConfigIO: vi.fn(() => ({
|
||||
readConfigFileSnapshot: readConfigFileSnapshotMock,
|
||||
writeConfigFile: writeConfigFileMock,
|
||||
})),
|
||||
}));
|
||||
|
||||
// Mock other dependencies
|
||||
vi.mock("../../agents/agent-scope.js", () => ({
|
||||
resolveAgentConfig: vi.fn(() => ({})),
|
||||
resolveAgentDir: vi.fn(() => "/tmp/agent"),
|
||||
resolveSessionAgentId: vi.fn(() => "main"),
|
||||
}));
|
||||
|
||||
vi.mock("../../agents/sandbox.js", () => ({
|
||||
resolveSandboxRuntimeStatus: vi.fn(() => ({ sandboxed: false })),
|
||||
}));
|
||||
|
||||
vi.mock("../../config/sessions.js", () => ({
|
||||
updateSessionStore: vi.fn(async () => {}),
|
||||
}));
|
||||
|
||||
vi.mock("../../infra/system-events.js", () => ({
|
||||
enqueueSystemEvent: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("../../routing/session-key.js", () => ({
|
||||
normalizeAgentId: vi.fn((id: string) => id?.toLowerCase?.() ?? ""),
|
||||
}));
|
||||
|
||||
import { parseInlineDirectives } from "./directive-handling.js";
|
||||
import { handleDirectiveOnly } from "./directive-handling.impl.js";
|
||||
|
||||
function createMockConfig(agents: Array<{ id: string; thinkingDefault?: string }>): MoltbotConfig {
|
||||
return {
|
||||
agents: {
|
||||
defaults: {},
|
||||
list: agents,
|
||||
},
|
||||
} as unknown as MoltbotConfig;
|
||||
}
|
||||
|
||||
function createMockSnapshot(cfg: MoltbotConfig, valid = true) {
|
||||
return {
|
||||
exists: true,
|
||||
valid,
|
||||
parsed: cfg,
|
||||
config: cfg,
|
||||
};
|
||||
}
|
||||
|
||||
describe("thinkingDefault persistence", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it("persists thinkingDefault when /think <level> is used", async () => {
|
||||
const cfg = createMockConfig([{ id: "myagent" }]);
|
||||
readConfigFileSnapshotMock.mockResolvedValue(createMockSnapshot(cfg));
|
||||
writeConfigFileMock.mockResolvedValue(undefined);
|
||||
|
||||
const directives = parseInlineDirectives("/think high");
|
||||
const sessionEntry = { updatedAt: Date.now() };
|
||||
|
||||
await handleDirectiveOnly({
|
||||
cfg,
|
||||
directives,
|
||||
sessionEntry: sessionEntry as any,
|
||||
sessionStore: {},
|
||||
sessionKey: "agent:myagent:test",
|
||||
elevatedEnabled: false,
|
||||
elevatedAllowed: false,
|
||||
defaultProvider: "anthropic",
|
||||
defaultModel: "claude-sonnet-4-20250514",
|
||||
aliasIndex: { byAlias: new Map(), byKey: new Map() },
|
||||
allowedModelKeys: new Set(),
|
||||
allowedModelCatalog: [],
|
||||
resetModelOverride: false,
|
||||
provider: "anthropic",
|
||||
model: "claude-sonnet-4-20250514",
|
||||
initialModelLabel: "anthropic/claude-sonnet-4-20250514",
|
||||
formatModelSwitchEvent: () => "",
|
||||
});
|
||||
|
||||
// Give fire-and-forget time to execute
|
||||
await new Promise((r) => setTimeout(r, 50));
|
||||
|
||||
expect(writeConfigFileMock).toHaveBeenCalled();
|
||||
const writtenConfig = writeConfigFileMock.mock.calls[0][0];
|
||||
expect(writtenConfig.agents.list[0].thinkingDefault).toBe("high");
|
||||
});
|
||||
|
||||
it("persists 'off' explicitly instead of deleting the field", async () => {
|
||||
const cfg = createMockConfig([{ id: "myagent", thinkingDefault: "high" }]);
|
||||
readConfigFileSnapshotMock.mockResolvedValue(createMockSnapshot(cfg));
|
||||
writeConfigFileMock.mockResolvedValue(undefined);
|
||||
|
||||
const directives = parseInlineDirectives("/think off");
|
||||
const sessionEntry = { updatedAt: Date.now() };
|
||||
|
||||
await handleDirectiveOnly({
|
||||
cfg,
|
||||
directives,
|
||||
sessionEntry: sessionEntry as any,
|
||||
sessionStore: {},
|
||||
sessionKey: "agent:myagent:test",
|
||||
elevatedEnabled: false,
|
||||
elevatedAllowed: false,
|
||||
defaultProvider: "anthropic",
|
||||
defaultModel: "claude-sonnet-4-20250514",
|
||||
aliasIndex: { byAlias: new Map(), byKey: new Map() },
|
||||
allowedModelKeys: new Set(),
|
||||
allowedModelCatalog: [],
|
||||
resetModelOverride: false,
|
||||
provider: "anthropic",
|
||||
model: "claude-sonnet-4-20250514",
|
||||
initialModelLabel: "anthropic/claude-sonnet-4-20250514",
|
||||
formatModelSwitchEvent: () => "",
|
||||
});
|
||||
|
||||
await new Promise((r) => setTimeout(r, 50));
|
||||
|
||||
expect(writeConfigFileMock).toHaveBeenCalled();
|
||||
const writtenConfig = writeConfigFileMock.mock.calls[0][0];
|
||||
// Should explicitly set "off", not delete the field
|
||||
expect(writtenConfig.agents.list[0].thinkingDefault).toBe("off");
|
||||
});
|
||||
|
||||
it("skips write if thinkingDefault is already set to the same value", async () => {
|
||||
const cfg = createMockConfig([{ id: "myagent", thinkingDefault: "high" }]);
|
||||
readConfigFileSnapshotMock.mockResolvedValue(createMockSnapshot(cfg));
|
||||
writeConfigFileMock.mockResolvedValue(undefined);
|
||||
|
||||
const directives = parseInlineDirectives("/think high");
|
||||
const sessionEntry = { updatedAt: Date.now() };
|
||||
|
||||
await handleDirectiveOnly({
|
||||
cfg,
|
||||
directives,
|
||||
sessionEntry: sessionEntry as any,
|
||||
sessionStore: {},
|
||||
sessionKey: "agent:myagent:test",
|
||||
elevatedEnabled: false,
|
||||
elevatedAllowed: false,
|
||||
defaultProvider: "anthropic",
|
||||
defaultModel: "claude-sonnet-4-20250514",
|
||||
aliasIndex: { byAlias: new Map(), byKey: new Map() },
|
||||
allowedModelKeys: new Set(),
|
||||
allowedModelCatalog: [],
|
||||
resetModelOverride: false,
|
||||
provider: "anthropic",
|
||||
model: "claude-sonnet-4-20250514",
|
||||
initialModelLabel: "anthropic/claude-sonnet-4-20250514",
|
||||
formatModelSwitchEvent: () => "",
|
||||
});
|
||||
|
||||
await new Promise((r) => setTimeout(r, 50));
|
||||
|
||||
// Should not write if value is the same
|
||||
expect(writeConfigFileMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not persist when agent is not found in config", async () => {
|
||||
const cfg = createMockConfig([{ id: "otheragent" }]);
|
||||
readConfigFileSnapshotMock.mockResolvedValue(createMockSnapshot(cfg));
|
||||
writeConfigFileMock.mockResolvedValue(undefined);
|
||||
|
||||
const directives = parseInlineDirectives("/think high");
|
||||
const sessionEntry = { updatedAt: Date.now() };
|
||||
|
||||
await handleDirectiveOnly({
|
||||
cfg,
|
||||
directives,
|
||||
sessionEntry: sessionEntry as any,
|
||||
sessionStore: {},
|
||||
sessionKey: "agent:myagent:test",
|
||||
elevatedEnabled: false,
|
||||
elevatedAllowed: false,
|
||||
defaultProvider: "anthropic",
|
||||
defaultModel: "claude-sonnet-4-20250514",
|
||||
aliasIndex: { byAlias: new Map(), byKey: new Map() },
|
||||
allowedModelKeys: new Set(),
|
||||
allowedModelCatalog: [],
|
||||
resetModelOverride: false,
|
||||
provider: "anthropic",
|
||||
model: "claude-sonnet-4-20250514",
|
||||
initialModelLabel: "anthropic/claude-sonnet-4-20250514",
|
||||
formatModelSwitchEvent: () => "",
|
||||
});
|
||||
|
||||
await new Promise((r) => setTimeout(r, 50));
|
||||
|
||||
// Agent not found, should not write
|
||||
expect(writeConfigFileMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("logs warning on write failure but does not throw", async () => {
|
||||
const cfg = createMockConfig([{ id: "myagent" }]);
|
||||
readConfigFileSnapshotMock.mockResolvedValue(createMockSnapshot(cfg));
|
||||
writeConfigFileMock.mockRejectedValue(new Error("Write failed"));
|
||||
|
||||
const consoleWarnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
|
||||
const directives = parseInlineDirectives("/think high");
|
||||
const sessionEntry = { updatedAt: Date.now() };
|
||||
|
||||
// Should not throw
|
||||
await expect(
|
||||
handleDirectiveOnly({
|
||||
cfg,
|
||||
directives,
|
||||
sessionEntry: sessionEntry as any,
|
||||
sessionStore: {},
|
||||
sessionKey: "agent:myagent:test",
|
||||
elevatedEnabled: false,
|
||||
elevatedAllowed: false,
|
||||
defaultProvider: "anthropic",
|
||||
defaultModel: "claude-sonnet-4-20250514",
|
||||
aliasIndex: { byAlias: new Map(), byKey: new Map() },
|
||||
allowedModelKeys: new Set(),
|
||||
allowedModelCatalog: [],
|
||||
resetModelOverride: false,
|
||||
provider: "anthropic",
|
||||
model: "claude-sonnet-4-20250514",
|
||||
initialModelLabel: "anthropic/claude-sonnet-4-20250514",
|
||||
formatModelSwitchEvent: () => "",
|
||||
}),
|
||||
).resolves.not.toThrow();
|
||||
|
||||
await new Promise((r) => setTimeout(r, 50));
|
||||
|
||||
expect(consoleWarnSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining("Failed to persist thinkingDefault"),
|
||||
expect.any(String),
|
||||
);
|
||||
|
||||
consoleWarnSpy.mockRestore();
|
||||
});
|
||||
});
|
||||
@ -24,6 +24,8 @@ export type AgentConfig = {
|
||||
workspace?: string;
|
||||
agentDir?: string;
|
||||
model?: AgentModelConfig;
|
||||
/** Default thinking level for this agent when no /think directive is present. */
|
||||
thinkingDefault?: "off" | "minimal" | "low" | "medium" | "high" | "xhigh";
|
||||
memorySearch?: MemorySearchConfig;
|
||||
/** Human-like delay between block replies for this agent. */
|
||||
humanDelay?: HumanDelayConfig;
|
||||
|
||||
@ -421,6 +421,16 @@ export const AgentEntrySchema = z
|
||||
workspace: z.string().optional(),
|
||||
agentDir: z.string().optional(),
|
||||
model: AgentModelSchema.optional(),
|
||||
thinkingDefault: z
|
||||
.union([
|
||||
z.literal("off"),
|
||||
z.literal("minimal"),
|
||||
z.literal("low"),
|
||||
z.literal("medium"),
|
||||
z.literal("high"),
|
||||
z.literal("xhigh"),
|
||||
])
|
||||
.optional(),
|
||||
memorySearch: MemorySearchSchema,
|
||||
humanDelay: HumanDelaySchema.optional(),
|
||||
heartbeat: HeartbeatSchema,
|
||||
|
||||
Loading…
Reference in New Issue
Block a user