fix: finish model list alias + heartbeat session (#1256) (thanks @zknicker)

This commit is contained in:
Peter Steinberger
2026-01-22 01:08:11 +00:00
parent 7725dd6795
commit 39073d5196
9 changed files with 259 additions and 117 deletions

View File

@@ -60,7 +60,7 @@ describe("directive behavior", () => {
vi.restoreAllMocks();
});
it("moves /model list to /models", async () => {
it("aliases /model list to /models", async () => {
await withTempHome(async (home) => {
vi.mocked(runEmbeddedPiAgent).mockReset();
const storePath = path.join(home, "sessions.json");
@@ -84,13 +84,15 @@ describe("directive behavior", () => {
);
const text = Array.isArray(res) ? res[0]?.text : res?.text;
expect(text).toContain("Model listing moved.");
expect(text).toContain("Use: /models (providers) or /models <provider> (models)");
expect(text).toContain("Providers:");
expect(text).toContain("- anthropic");
expect(text).toContain("- openai");
expect(text).toContain("Use: /models <provider>");
expect(text).toContain("Switch: /model <provider/model>");
expect(runEmbeddedPiAgent).not.toHaveBeenCalled();
});
});
it("shows summary on /model when catalog is unavailable", async () => {
it("shows current model when catalog is unavailable", async () => {
await withTempHome(async (home) => {
vi.mocked(runEmbeddedPiAgent).mockReset();
vi.mocked(loadModelCatalog).mockResolvedValueOnce([]);
@@ -122,10 +124,10 @@ describe("directive behavior", () => {
expect(runEmbeddedPiAgent).not.toHaveBeenCalled();
});
});
it("moves /model list to /models even when no allowlist is set", async () => {
it("includes catalog providers when no allowlist is set", async () => {
await withTempHome(async (home) => {
vi.mocked(runEmbeddedPiAgent).mockReset();
vi.mocked(loadModelCatalog).mockResolvedValueOnce([
vi.mocked(loadModelCatalog).mockResolvedValue([
{ id: "claude-opus-4-5", name: "Opus 4.5", provider: "anthropic" },
{ id: "gpt-4.1-mini", name: "GPT-4.1 Mini", provider: "openai" },
{ id: "grok-4", name: "Grok 4", provider: "xai" },
@@ -151,13 +153,15 @@ describe("directive behavior", () => {
);
const text = Array.isArray(res) ? res[0]?.text : res?.text;
expect(text).toContain("Model listing moved.");
expect(text).toContain("Use: /models (providers) or /models <provider> (models)");
expect(text).toContain("Switch: /model <provider/model>");
expect(text).toContain("Providers:");
expect(text).toContain("- anthropic");
expect(text).toContain("- openai");
expect(text).toContain("- xai");
expect(text).toContain("Use: /models <provider>");
expect(runEmbeddedPiAgent).not.toHaveBeenCalled();
});
});
it("moves /model list to /models even when catalog is present", async () => {
it("lists config-only providers when catalog is present", async () => {
await withTempHome(async (home) => {
vi.mocked(runEmbeddedPiAgent).mockReset();
// Catalog present but missing custom providers: /model should still include
@@ -173,7 +177,7 @@ describe("directive behavior", () => {
const storePath = path.join(home, "sessions.json");
const res = await getReplyFromConfig(
{ Body: "/model list", From: "+1222", To: "+1222", CommandAuthorized: true },
{ Body: "/models minimax", From: "+1222", To: "+1222", CommandAuthorized: true },
{},
{
agents: {
@@ -202,13 +206,12 @@ describe("directive behavior", () => {
);
const text = Array.isArray(res) ? res[0]?.text : res?.text;
expect(text).toContain("Model listing moved.");
expect(text).toContain("Use: /models (providers) or /models <provider> (models)");
expect(text).toContain("Switch: /model <provider/model>");
expect(text).toContain("Model set to minimax");
expect(text).toContain("minimax/MiniMax-M2.1");
expect(runEmbeddedPiAgent).not.toHaveBeenCalled();
});
});
it("moves /model list to /models without listing auth labels", async () => {
it("does not repeat missing auth labels on /model list", async () => {
await withTempHome(async (home) => {
vi.mocked(runEmbeddedPiAgent).mockReset();
const storePath = path.join(home, "sessions.json");
@@ -231,9 +234,7 @@ describe("directive behavior", () => {
);
const text = Array.isArray(res) ? res[0]?.text : res?.text;
expect(text).toContain("Model listing moved.");
expect(text).toContain("Use: /models (providers) or /models <provider> (models)");
expect(text).toContain("Switch: /model <provider/model>");
expect(text).toContain("Providers:");
expect(text).not.toContain("missing (missing)");
expect(runEmbeddedPiAgent).not.toHaveBeenCalled();
});

View File

@@ -215,6 +215,7 @@ describe("directive behavior", () => {
expect(text).toContain("Switch: /model <provider/model>");
expect(text).toContain("Browse: /models (providers) or /models <provider> (models)");
expect(text).toContain("More: /model status");
expect(text).not.toContain("openai/gpt-4.1-mini");
expect(runEmbeddedPiAgent).not.toHaveBeenCalled();
});
});

View File

@@ -123,7 +123,7 @@ describe("trigger handling", () => {
expect(normalized).not.toContain("image");
});
});
it("moves /model list to /models", async () => {
it("aliases /model list to /models", async () => {
await withTempHome(async (home) => {
const cfg = makeCfg(home);
const res = await getReplyFromConfig(
@@ -143,8 +143,8 @@ describe("trigger handling", () => {
const text = Array.isArray(res) ? res[0]?.text : res?.text;
const normalized = normalizeTestText(text ?? "");
expect(normalized).toContain("Model listing moved.");
expect(normalized).toContain("Use: /models (providers) or /models <provider> (models)");
expect(normalized).toContain("Providers:");
expect(normalized).toContain("Use: /models <provider>");
expect(normalized).toContain("Switch: /model <provider/model>");
});
});

View File

@@ -7,6 +7,7 @@ import {
resolveModelRefFromString,
} from "../../agents/model-selection.js";
import { DEFAULT_MODEL, DEFAULT_PROVIDER } from "../../agents/defaults.js";
import type { ClawdbotConfig } from "../../config/config.js";
import type { ReplyPayload } from "../types.js";
import type { CommandHandler } from "./commands-types.js";
@@ -68,10 +69,11 @@ function parseModelsArgs(raw: string): {
};
}
export const handleModelsCommand: CommandHandler = async (params, allowTextCommands) => {
if (!allowTextCommands) return null;
const body = params.command.commandBodyNormalized.trim();
export async function resolveModelsCommandReply(params: {
cfg: ClawdbotConfig;
commandBodyNormalized: string;
}): Promise<ReplyPayload | null> {
const body = params.commandBodyNormalized.trim();
if (!body.startsWith("/models")) return null;
const argText = body.replace(/^\/models\b/i, "").trim();
@@ -164,7 +166,7 @@ export const handleModelsCommand: CommandHandler = async (params, allowTextComma
"Use: /models <provider>",
"Switch: /model <provider/model>",
];
return { reply: { text: lines.join("\n") }, shouldContinue: false };
return { text: lines.join("\n") };
}
if (!byProvider.has(provider)) {
@@ -176,7 +178,7 @@ export const handleModelsCommand: CommandHandler = async (params, allowTextComma
"",
"Use: /models <provider>",
];
return { reply: { text: lines.join("\n") }, shouldContinue: false };
return { text: lines.join("\n") };
}
const models = [...(byProvider.get(provider) ?? new Set<string>())].sort();
@@ -189,7 +191,7 @@ export const handleModelsCommand: CommandHandler = async (params, allowTextComma
"Browse: /models",
"Switch: /model <provider/model>",
];
return { reply: { text: lines.join("\n") }, shouldContinue: false };
return { text: lines.join("\n") };
}
const effectivePageSize = all ? total : pageSize;
@@ -203,7 +205,7 @@ export const handleModelsCommand: CommandHandler = async (params, allowTextComma
`Try: /models ${provider} ${safePage}`,
`All: /models ${provider} all`,
];
return { reply: { text: lines.join("\n") }, shouldContinue: false };
return { text: lines.join("\n") };
}
const startIndex = (safePage - 1) * effectivePageSize;
@@ -226,5 +228,16 @@ export const handleModelsCommand: CommandHandler = async (params, allowTextComma
}
const payload: ReplyPayload = { text: lines.join("\n") };
return { reply: payload, shouldContinue: false };
return payload;
}
export const handleModelsCommand: CommandHandler = async (params, allowTextCommands) => {
if (!allowTextCommands) return null;
const reply = await resolveModelsCommandReply({
cfg: params.cfg,
commandBodyNormalized: params.command.commandBodyNormalized,
});
if (!reply) return null;
return { reply, shouldContinue: false };
};

View File

@@ -36,7 +36,7 @@ describe("/model chat UX", () => {
expect(reply?.text).toContain("Switch: /model <provider/model>");
});
it("suggests closest match for typos without switching", () => {
it("auto-applies closest match for typos", () => {
const directives = parseInlineDirectives("/model anthropic/claud-opus-4-5");
const cfg = { commands: { text: true } } as unknown as ClawdbotConfig;
@@ -52,9 +52,11 @@ describe("/model chat UX", () => {
provider: "anthropic",
});
expect(resolved.modelSelection).toBeUndefined();
expect(resolved.errorText).toContain("Did you mean:");
expect(resolved.errorText).toContain("anthropic/claude-opus-4-5");
expect(resolved.errorText).toContain("Try: /model anthropic/claude-opus-4-5");
expect(resolved.modelSelection).toEqual({
provider: "anthropic",
model: "claude-opus-4-5",
isDefault: true,
});
expect(resolved.errorText).toBeUndefined();
});
});

View File

@@ -20,6 +20,7 @@ import {
resolveProviderEndpointLabel,
} from "./directive-handling.model-picker.js";
import type { InlineDirectives } from "./directive-handling.parse.js";
import { resolveModelsCommandReply } from "./commands-models.js";
import { type ModelDirectiveSelection, resolveModelDirectiveSelection } from "./model-selection.js";
function buildModelPickerCatalog(params: {
@@ -185,14 +186,11 @@ export async function maybeHandleModelDirectiveInfo(params: {
});
if (wantsLegacyList) {
return {
text: [
"Model listing moved.",
"",
"Use: /models (providers) or /models <provider> (models)",
"Switch: /model <provider/model>",
].join("\n"),
};
const reply = await resolveModelsCommandReply({
cfg: params.cfg,
commandBodyNormalized: "/models",
});
return reply ?? { text: "No models available." };
}
if (wantsSummary) {
@@ -340,42 +338,7 @@ export function resolveModelSelectionFromDirective(params: {
}
if (resolved.selection) {
const suggestion = `${resolved.selection.provider}/${resolved.selection.model}`;
const rawHasSlash = raw.includes("/");
const shouldAutoSelect = (() => {
if (!rawHasSlash) return true;
const slash = raw.indexOf("/");
if (slash <= 0) return true;
const rawProvider = normalizeProviderId(raw.slice(0, slash));
const rawFragment = raw
.slice(slash + 1)
.trim()
.toLowerCase();
if (!rawFragment) return false;
const resolvedProvider = normalizeProviderId(resolved.selection.provider);
if (rawProvider !== resolvedProvider) return false;
const resolvedModel = resolved.selection.model.toLowerCase();
return (
resolvedModel.startsWith(rawFragment) ||
resolvedModel.includes(rawFragment) ||
rawFragment.startsWith(resolvedModel)
);
})();
if (shouldAutoSelect) {
modelSelection = resolved.selection;
} else {
return {
errorText: [
`Unrecognized model: ${raw}`,
"",
`Did you mean: ${suggestion}`,
`Try: /model ${suggestion}`,
"",
"Browse: /models or /models <provider>",
].join("\n"),
};
}
modelSelection = resolved.selection;
}
}

View File

@@ -478,7 +478,9 @@ describe("runHeartbeatOnce", () => {
peerKind: "group",
peerId: groupId,
});
cfg.agents?.defaults?.heartbeat && (cfg.agents.defaults.heartbeat.session = groupSessionKey);
if (cfg.agents?.defaults?.heartbeat) {
cfg.agents.defaults.heartbeat.session = groupSessionKey;
}
await fs.writeFile(
storePath,

View File

@@ -1,4 +1,5 @@
import { resolveAgentConfig, resolveDefaultAgentId } from "../agents/agent-scope.js";
import { resolveUserTimezone } from "../agents/date-time.js";
import { resolveEffectiveMessagesConfig } from "../agents/identity.js";
import {
DEFAULT_HEARTBEAT_ACK_MAX_CHARS,
@@ -14,8 +15,8 @@ import { parseDurationMs } from "../cli/parse-duration.js";
import type { ClawdbotConfig } from "../config/config.js";
import { loadConfig } from "../config/config.js";
import {
loadSessionStore,
canonicalizeMainSessionAlias,
loadSessionStore,
resolveAgentIdFromSessionKey,
resolveAgentMainSessionKey,
resolveStorePath,
@@ -26,6 +27,7 @@ import type { AgentDefaultsConfig } from "../config/types.agent-defaults.js";
import { formatErrorMessage } from "../infra/errors.js";
import { createSubsystemLogger } from "../logging/subsystem.js";
import { getQueueSize } from "../process/command-queue.js";
import { CommandLane } from "../process/lanes.js";
import { defaultRuntime, type RuntimeEnv } from "../runtime.js";
import { normalizeAgentId, toAgentStoreSessionKey } from "../routing/session-key.js";
import { emitHeartbeatEvent } from "./heartbeat-events.js";
@@ -70,6 +72,94 @@ export type HeartbeatSummary = {
};
const DEFAULT_HEARTBEAT_TARGET = "last";
const ACTIVE_HOURS_TIME_PATTERN = /^([01]\d|2[0-3]|24):([0-5]\d)$/;
function resolveActiveHoursTimezone(cfg: ClawdbotConfig, raw?: string): string {
const trimmed = raw?.trim();
if (!trimmed || trimmed === "user") {
return resolveUserTimezone(cfg.agents?.defaults?.userTimezone);
}
if (trimmed === "local") {
const host = Intl.DateTimeFormat().resolvedOptions().timeZone;
return host?.trim() || "UTC";
}
try {
new Intl.DateTimeFormat("en-US", { timeZone: trimmed }).format(new Date());
return trimmed;
} catch {
return resolveUserTimezone(cfg.agents?.defaults?.userTimezone);
}
}
function parseActiveHoursTime(opts: { allow24: boolean }, raw?: string): number | null {
if (!raw || !ACTIVE_HOURS_TIME_PATTERN.test(raw)) return null;
const [hourStr, minuteStr] = raw.split(":");
const hour = Number(hourStr);
const minute = Number(minuteStr);
if (!Number.isFinite(hour) || !Number.isFinite(minute)) return null;
if (hour === 24) {
if (!opts.allow24 || minute !== 0) return null;
return 24 * 60;
}
return hour * 60 + minute;
}
function resolveMinutesInTimeZone(nowMs: number, timeZone: string): number | null {
try {
const parts = new Intl.DateTimeFormat("en-US", {
timeZone,
hour: "2-digit",
minute: "2-digit",
hourCycle: "h23",
}).formatToParts(new Date(nowMs));
const map: Record<string, string> = {};
for (const part of parts) {
if (part.type !== "literal") map[part.type] = part.value;
}
const hour = Number(map.hour);
const minute = Number(map.minute);
if (!Number.isFinite(hour) || !Number.isFinite(minute)) return null;
return hour * 60 + minute;
} catch {
return null;
}
}
function isWithinActiveHours(
cfg: ClawdbotConfig,
heartbeat?: HeartbeatConfig,
nowMs?: number,
): boolean {
const active = heartbeat?.activeHours;
if (!active) return true;
const startMin = parseActiveHoursTime({ allow24: false }, active.start);
const endMin = parseActiveHoursTime({ allow24: true }, active.end);
if (startMin === null || endMin === null) return true;
if (startMin === endMin) return true;
const timeZone = resolveActiveHoursTimezone(cfg, active.timezone);
const currentMin = resolveMinutesInTimeZone(nowMs ?? Date.now(), timeZone);
if (currentMin === null) return true;
if (endMin > startMin) {
return currentMin >= startMin && currentMin < endMin;
}
return currentMin >= startMin || currentMin < endMin;
}
type HeartbeatAgentState = {
agentId: string;
heartbeat?: HeartbeatConfig;
intervalMs: number;
lastRunMs?: number;
nextDueMs: number;
};
export type HeartbeatRunner = {
stop: () => void;
updateConfig: (cfg: ClawdbotConfig) => void;
};
function hasExplicitHeartbeatAgents(cfg: ClawdbotConfig) {
const list = cfg.agents?.list ?? [];
@@ -365,12 +455,16 @@ export async function runHeartbeatOnce(opts: {
return { status: "skipped", reason: "disabled" };
}
const queueSize = (opts.deps?.getQueueSize ?? getQueueSize)("main");
const startedAt = opts.deps?.nowMs?.() ?? Date.now();
if (!isWithinActiveHours(cfg, heartbeat, startedAt)) {
return { status: "skipped", reason: "quiet-hours" };
}
const queueSize = (opts.deps?.getQueueSize ?? getQueueSize)(CommandLane.Main);
if (queueSize > 0) {
return { status: "skipped", reason: "requests-in-flight" };
}
const startedAt = opts.deps?.nowMs?.() ?? Date.now();
const { entry, sessionKey, storePath } = resolveHeartbeatSession(cfg, agentId, heartbeat);
const previousUpdatedAt = entry?.updatedAt;
const delivery = resolveHeartbeatDeliveryTarget({ cfg, entry, heartbeat });
@@ -576,24 +670,97 @@ export function startHeartbeatRunner(opts: {
cfg?: ClawdbotConfig;
runtime?: RuntimeEnv;
abortSignal?: AbortSignal;
}) {
const cfg = opts.cfg ?? loadConfig();
const heartbeatAgents = resolveHeartbeatAgents(cfg);
const intervals = heartbeatAgents
.map((agent) => resolveHeartbeatIntervalMs(cfg, undefined, agent.heartbeat))
.filter((value): value is number => typeof value === "number");
const intervalMs = intervals.length > 0 ? Math.min(...intervals) : null;
if (!intervalMs) {
log.info("heartbeat: disabled", { enabled: false });
}
runOnce?: typeof runHeartbeatOnce;
}): HeartbeatRunner {
const runtime = opts.runtime ?? defaultRuntime;
const lastRunByAgent = new Map<string, number>();
const runOnce = opts.runOnce ?? runHeartbeatOnce;
const state = {
cfg: opts.cfg ?? loadConfig(),
runtime,
agents: new Map<string, HeartbeatAgentState>(),
timer: null as NodeJS.Timeout | null,
stopped: false,
};
let initialized = false;
const resolveNextDue = (now: number, intervalMs: number, prevState?: HeartbeatAgentState) => {
if (typeof prevState?.lastRunMs === "number") {
return prevState.lastRunMs + intervalMs;
}
if (prevState && prevState.intervalMs === intervalMs && prevState.nextDueMs > now) {
return prevState.nextDueMs;
}
return now + intervalMs;
};
const scheduleNext = () => {
if (state.stopped) return;
if (state.timer) {
clearTimeout(state.timer);
state.timer = null;
}
if (state.agents.size === 0) return;
const now = Date.now();
let nextDue = Number.POSITIVE_INFINITY;
for (const agent of state.agents.values()) {
if (agent.nextDueMs < nextDue) nextDue = agent.nextDueMs;
}
if (!Number.isFinite(nextDue)) return;
const delay = Math.max(0, nextDue - now);
state.timer = setTimeout(() => {
requestHeartbeatNow({ reason: "interval", coalesceMs: 0 });
}, delay);
state.timer.unref?.();
};
const updateConfig = (cfg: ClawdbotConfig) => {
if (state.stopped) return;
const now = Date.now();
const prevAgents = state.agents;
const prevEnabled = prevAgents.size > 0;
const nextAgents = new Map<string, HeartbeatAgentState>();
const intervals: number[] = [];
for (const agent of resolveHeartbeatAgents(cfg)) {
const intervalMs = resolveHeartbeatIntervalMs(cfg, undefined, agent.heartbeat);
if (!intervalMs) continue;
intervals.push(intervalMs);
const prevState = prevAgents.get(agent.agentId);
const nextDueMs = resolveNextDue(now, intervalMs, prevState);
nextAgents.set(agent.agentId, {
agentId: agent.agentId,
heartbeat: agent.heartbeat,
intervalMs,
lastRunMs: prevState?.lastRunMs,
nextDueMs,
});
}
state.cfg = cfg;
state.agents = nextAgents;
const nextEnabled = nextAgents.size > 0;
if (!initialized) {
if (!nextEnabled) {
log.info("heartbeat: disabled", { enabled: false });
} else {
log.info("heartbeat: started", { intervalMs: Math.min(...intervals) });
}
initialized = true;
} else if (prevEnabled !== nextEnabled) {
if (!nextEnabled) {
log.info("heartbeat: disabled", { enabled: false });
} else {
log.info("heartbeat: started", { intervalMs: Math.min(...intervals) });
}
}
scheduleNext();
};
const run: HeartbeatWakeHandler = async (params) => {
if (!heartbeatsEnabled) {
return { status: "skipped", reason: "disabled" } satisfies HeartbeatRunResult;
}
if (heartbeatAgents.length === 0) {
if (state.agents.size === 0) {
return { status: "skipped", reason: "disabled" } satisfies HeartbeatRunResult;
}
@@ -603,52 +770,44 @@ export function startHeartbeatRunner(opts: {
const now = startedAt;
let ran = false;
for (const agent of heartbeatAgents) {
const agentIntervalMs = resolveHeartbeatIntervalMs(cfg, undefined, agent.heartbeat);
if (!agentIntervalMs) continue;
const lastRun = lastRunByAgent.get(agent.agentId);
if (isInterval && typeof lastRun === "number" && now - lastRun < agentIntervalMs) {
for (const agent of state.agents.values()) {
if (isInterval && now < agent.nextDueMs) {
continue;
}
const res = await runHeartbeatOnce({
cfg,
const res = await runOnce({
cfg: state.cfg,
agentId: agent.agentId,
heartbeat: agent.heartbeat,
reason,
deps: { runtime },
deps: { runtime: state.runtime },
});
if (res.status === "skipped" && res.reason === "requests-in-flight") {
return res;
}
if (res.status !== "skipped" || res.reason !== "disabled") {
lastRunByAgent.set(agent.agentId, now);
agent.lastRunMs = now;
agent.nextDueMs = now + agent.intervalMs;
}
if (res.status === "ran") ran = true;
}
scheduleNext();
if (ran) return { status: "ran", durationMs: Date.now() - startedAt };
return { status: "skipped", reason: isInterval ? "not-due" : "disabled" };
};
setHeartbeatWakeHandler(async (params) => run({ reason: params.reason }));
let timer: NodeJS.Timeout | null = null;
if (intervalMs) {
timer = setInterval(() => {
requestHeartbeatNow({ reason: "interval", coalesceMs: 0 });
}, intervalMs);
timer.unref?.();
log.info("heartbeat: started", { intervalMs });
}
updateConfig(state.cfg);
const cleanup = () => {
state.stopped = true;
setHeartbeatWakeHandler(null);
if (timer) clearInterval(timer);
timer = null;
if (state.timer) clearTimeout(state.timer);
state.timer = null;
};
opts.abortSignal?.addEventListener("abort", cleanup, { once: true });
return { stop: cleanup };
return { stop: cleanup, updateConfig };
}