diff --git a/docs/superpowers/specs/2026-08-19-configurable-room-turn-timeout-design.md b/docs/superpowers/specs/2026-08-19-configurable-room-turn-timeout-design.md new file mode 100644 index 0000000000..dea6b17102 --- /dev/null +++ b/docs/superpowers/specs/2026-08-19-configurable-room-turn-timeout-design.md @@ -0,0 +1,111 @@ +# Configurable Room Turn Timeout + +## Summary + +OpenMausBot currently stops every room member turn after five minutes, even when the engine is still producing output. The duration and the error message are hard-coded in `server/index.ts`. This behavior is separate from the activity-based turn stall watchdog controlled by `OMB_TURN_STALL_MS`. + +Add one global, persisted room turn timeout setting. Keep five minutes as the default, expose the setting in the existing General settings UI, and use the configured value for room turns started after the setting is saved. + +## Goals + +- Let users configure the maximum duration of a room member turn. +- Preserve the current five-minute behavior for existing installations. +- Make the setting discoverable and editable in the existing app settings UI. +- Apply updates without restarting the server or reloading providers. +- Report the configured duration in timeout activity messages. +- Keep the room turn ceiling distinct from the inactivity-based turn stall watchdog. + +## Non-goals + +- Per-room or per-bot timeout overrides. +- Changing `OMB_TURN_STALL_MS` or the semantics of the stall watchdog. +- Changing provider-specific approval or RPC timeouts. +- Retiming room turns that are already running when the setting changes. +- Adding an environment-variable override for the room turn ceiling. + +## Configuration Model + +Add a `rooms` section to the persisted application configuration: + +```json +{ + "rooms": { + "turnTimeoutMinutes": 5 + } +} +``` + +`turnTimeoutMinutes` is a whole number from 1 through 1,440. Missing values resolve to 5, so existing configuration files retain the current behavior. Stored configuration and API patches reject non-numeric, fractional, out-of-range, and structurally invalid values. + +The public config status includes the effective value because it is non-secret: + +```json +{ + "rooms": { + "turnTimeoutMinutes": 5 + } +} +``` + +Saving only this section must not reload providers or interrupt active turns. The server updates its in-memory application config and broadcasts the new config status through the existing config event. + +## Server Behavior + +When a room member turn is dispatched, the server reads the effective global timeout and captures it for that turn. The timer uses that captured value, so changing the setting affects the next room turn and does not silently move the deadline of a turn already in progress. + +On timeout, the server keeps the existing interruption and room ownership behavior. Only the timer duration and activity text become dynamic. The message uses readable singular and plural forms, for example: + +- `Atlas's room turn exceeded 1 minute and was stopped` +- `Atlas's room turn exceeded 20 minutes and was stopped` + +The turn stall watchdog remains activity-based and independent. A room turn can therefore stop because it reaches the configured absolute ceiling or because it becomes inactive long enough for the existing watchdog to fire. + +## User Interface + +Add a `Room turns` card to `Settings > General`, alongside the existing global settings cards. The card follows the current `Card` and input styles instead of introducing a new settings pattern. + +The card contains: + +- A `Maximum turn length` label. +- A numeric input showing the current value. +- A `minutes` suffix. +- Supporting text explaining that the limit applies to every bot turn in rooms and that direct chats use the inactivity watchdog instead. + +The field accepts whole minutes from 1 through 1,440. It saves on blur, matching the Profile fields. Pressing Enter blurs the field and saves. Invalid input remains visible with the existing danger color treatment, shows a concise inline validation message, and is not sent to the server. A failed save also keeps the entered value visible and reports the server error inline. + +When a config status update arrives, the field synchronizes to the server value unless the user is actively editing it. This prevents a stale config event from replacing in-progress input. + +## Data Flow + +1. `GET /api/config` returns `rooms.turnTimeoutMinutes` with an effective default of 5. +2. The app store hydrates and folds config events with the `rooms` status included. +3. The General settings card edits the value and sends `PUT /api/config` with only the `rooms` patch. +4. The server validates and persists the patch, updates the live config object, and broadcasts the resulting config status. +5. Each new room member turn captures the effective duration and starts its absolute timeout timer. +6. If the timer fires first, the server interrupts the provider and records the dynamic timeout activity message. + +## Error Handling + +- Client-side validation prevents empty, fractional, non-numeric, and out-of-range values from being submitted. +- Server-side schema validation remains authoritative and returns HTTP 400 for invalid patches. +- Save failures appear next to the field without changing the last confirmed setting in application state. +- Existing room timeout cleanup and ownership safeguards remain unchanged. + +## Testing + +Add focused coverage for: + +- Stored configuration parsing with a valid room timeout. +- Defaulting missing room settings to five minutes in config status. +- Rejecting malformed and out-of-range room timeout patches. +- Persisting and returning an updated room timeout through `/api/config`. +- Folding the `rooms` section from config events into client state. +- Client validation and save behavior for the General settings field. +- The room turn timer using a configurable duration and formatting singular and plural timeout messages. +- Existing behavior remaining at five minutes when no setting is present. + +Run the focused tests first, followed by the repository typecheck, lint, and relevant full test suite before publishing the pull request. + +## Pull Request Scope + +The pull request will contain only the configuration contract, room timeout behavior, General settings UI, focused tests, and supporting documentation needed for this change. The pull request title, body, commits, code comments, UI copy, and tests will be written in English. diff --git a/server/config.test.ts b/server/config.test.ts index ccf722f736..1b4d1c618f 100644 --- a/server/config.test.ts +++ b/server/config.test.ts @@ -10,6 +10,7 @@ import { loadConfig, parseConfigPatch, parseStoredConfig, + roomTurnTimeoutMinutes, stripWorkspaceCredentialEnv, syncCredentialEnv, vpsSshAlias, @@ -48,6 +49,23 @@ describe("configuration boundaries", () => { expect(vpsSshAlias({ vps: { sshAlias: "production-vps" } })).toBe("production-vps"); expect(vpsSshAlias({ vps: { sshAlias: "-bad" } })).toBeNull(); }); + + it("accepts a persisted global room turn timeout and supplies the legacy default", () => { + expect(parseStoredConfig({ rooms: { turnTimeoutMinutes: 20 } })).toEqual({ + rooms: { turnTimeoutMinutes: 20 }, + }); + expect(roomTurnTimeoutMinutes({ rooms: { turnTimeoutMinutes: 20 } })).toBe(20); + expect(roomTurnTimeoutMinutes({})).toBe(5); + }); + + it.each([0, 1.5, 1441, "20", null])( + "rejects an invalid room turn timeout: %j", + (turnTimeoutMinutes) => { + expect(() => parseConfigPatch({ rooms: { turnTimeoutMinutes } })).toThrow( + "rooms.turnTimeoutMinutes", + ); + }, + ); }); describe("default fleet", () => { diff --git a/server/config.ts b/server/config.ts index 6879128163..842f04d28f 100644 --- a/server/config.ts +++ b/server/config.ts @@ -13,6 +13,10 @@ import { parseJson, schemaIssue, type JsonObject, type JsonValue } from "./schem const optionalText = z.string().optional(); const SSH_ALIAS = /^[A-Za-z0-9][A-Za-z0-9_.-]{0,127}$/; +export const DEFAULT_ROOM_TURN_TIMEOUT_MINUTES = 5; +export const MIN_ROOM_TURN_TIMEOUT_MINUTES = 1; +export const MAX_ROOM_TURN_TIMEOUT_MINUTES = 1_440; + export function isValidSshAlias(value: unknown): value is string { return typeof value === "string" && SSH_ALIAS.test(value); } @@ -36,6 +40,13 @@ const vpsConfigSchema = z.object({ message: "must be a simple SSH config alias", }).optional(), }); +const roomConfigSchema = z.object({ + turnTimeoutMinutes: z + .number() + .int() + .min(MIN_ROOM_TURN_TIMEOUT_MINUTES) + .max(MAX_ROOM_TURN_TIMEOUT_MINUTES), +}); const instanceConfigSchema = z.object({ driver: z.string().min(1), displayName: optionalText, @@ -58,6 +69,7 @@ const appConfigSchema = z.object({ tts: z.object({ key: optionalText, voice: optionalText }).optional(), /** Non-secret profile details shown in the sidebar. */ profile: z.object({ name: optionalText, email: optionalText }).optional(), + rooms: roomConfigSchema.optional(), instances: instanceConfigMapSchema.optional(), }); const appConfigPatchSchema = appConfigSchema.omit({ instances: true }); @@ -72,6 +84,7 @@ export interface AppConfig { opencodeGo?: { apiKey?: string }; tts?: { key?: string; voice?: string }; profile?: { name?: string; email?: string }; + rooms?: { turnTimeoutMinutes: number }; instances?: InstanceConfigMap; } export type ConfigPatch = z.output; @@ -94,6 +107,10 @@ export function vpsSshAlias(cfg: AppConfig): string | null { return isValidSshAlias(cfg.vps?.sshAlias) ? cfg.vps.sshAlias : null; } +export function roomTurnTimeoutMinutes(cfg: AppConfig): number { + return cfg.rooms?.turnTimeoutMinutes ?? DEFAULT_ROOM_TURN_TIMEOUT_MINUTES; +} + // OMB_DATA_DIR isolates test/soak rigs from the user's real fleet. export const DATA_DIR = process.env.OMB_DATA_DIR ?? join(homedir(), ".openmausbot"); const LEGACY_DATA_DIR = join(homedir(), ".opengrokbot"); @@ -193,7 +210,7 @@ export function saveConfig(patch: Partial): void { /* first write */ } const checkedPatch = appConfigSchema.partial().parse(patch); - for (const key of ["xai", "composio", "box", "opencodeGo", "tts", "profile"] as const) { + for (const key of ["xai", "composio", "box", "opencodeGo", "tts", "profile", "rooms"] as const) { const section = checkedPatch[key]; if (!section) continue; const current = jsonObjectSchema.safeParse(disk[key]); diff --git a/server/index.test.ts b/server/index.test.ts index 8142dde953..45ba3694a1 100644 --- a/server/index.test.ts +++ b/server/index.test.ts @@ -5,7 +5,7 @@ // the shadow-instance behavior end to end while it's at it. import { spawn, type ChildProcess } from "node:child_process"; import { createServer, request, type Server } from "node:http"; -import { mkdirSync, mkdtempSync, readFileSync, statSync, writeFileSync } from "node:fs"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, statSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; @@ -16,6 +16,7 @@ import { openSse } from "./testing/sse.ts"; const SERVER_DIR = dirname(fileURLToPath(import.meta.url)); const ROOT = join(SERVER_DIR, ".."); +const FAKE_CLAUDE_CLI = join(SERVER_DIR, "testing", "fake-claude-cli.ts"); const PORT = 18800 + Math.floor(Math.random() * 10_000); const BASE = `http://127.0.0.1:${PORT}`; const WEBHOOK_PORT = 39000 + Math.floor(Math.random() * 10_000); @@ -27,6 +28,7 @@ let boxStub: Server; let boxStubPort = 0; let home: string; let staticDir: string; +let fakeClaudeDump: string; let stderr = ""; const api = async (method: string, path: string, body?: unknown): Promise<{ status: number; body: any }> => { @@ -51,6 +53,7 @@ const statusWithHeaders = (headers: Record): Promise => beforeAll(async () => { home = mkdtempSync(join(tmpdir(), "omb-api-test-")); staticDir = join(home, "static"); + fakeClaudeDump = join(home, "fake-claude-dump.json"); // a fleet of exactly one unknown driver: no CLI probes, no network mkdirSync(join(home, ".openmausbot"), { recursive: true }); mkdirSync(join(staticDir, "assets"), { recursive: true }); @@ -58,7 +61,12 @@ beforeAll(async () => { writeFileSync(join(staticDir, "assets", "smoke.css"), "body { color: white; }"); writeFileSync( join(home, ".openmausbot", "config.json"), - JSON.stringify({ instances: { ghost: { driver: "not-a-real-driver", displayName: "Ghost" } } }), + JSON.stringify({ + instances: { + ghost: { driver: "not-a-real-driver", displayName: "Ghost" }, + claude: { driver: "claudeAgent", displayName: "Fixture Claude", config: { cli: FAKE_CLAUDE_CLI } }, + }, + }), ); writeFileSync( join(home, ".openmausbot", "groups.json"), @@ -163,6 +171,8 @@ beforeAll(async () => { OMB_BOX_API: `http://127.0.0.1:${boxStubPort}`, OMB_COMPOSIO_API: `http://127.0.0.1:${boxStubPort}/api/v3.1`, OMB_STATIC_DIR: staticDir, + FAKE_CLAUDE_MODE: "hang", + FAKE_CLAUDE_DUMP: fakeClaudeDump, }, stdio: ["ignore", "pipe", "pipe"], }); @@ -277,14 +287,19 @@ describe("harness HTTP API", () => { it("describes the configured fleet, shadows included", async () => { const { status, body } = await api("GET", "/api/instances"); expect(status).toBe(200); - expect(body.instances).toHaveLength(1); - expect(body.instances[0]).toMatchObject({ + const ghost = body.instances.find((instance: { instanceId: string }) => instance.instanceId === "ghost"); + expect(ghost).toMatchObject({ instanceId: "ghost", driverKind: "not-a-real-driver", displayName: "Ghost", snapshot: { state: "unavailable" }, }); - expect(body.instances[0].snapshot.reason).toContain("not-a-real-driver"); + expect(ghost.snapshot.reason).toContain("not-a-real-driver"); + expect(body.instances).toContainEqual(expect.objectContaining({ + instanceId: "claude", + driverKind: "claudeAgent", + displayName: "Fixture Claude", + })); }); it("searches transcripts and exports a conversation", async () => { @@ -832,6 +847,77 @@ describe("harness HTTP API", () => { expect(nothing.status).toBe(400); }); + it("validates and persists the global room turn timeout", async () => { + const before = await api("GET", "/api/config"); + expect(before.status).toBe(200); + expect(before.body.rooms).toEqual({ turnTimeoutMinutes: 5 }); + + for (const turnTimeoutMinutes of [0, 1.5, 1441, "20", null]) { + const invalid = await api("PUT", "/api/config", { rooms: { turnTimeoutMinutes } }); + expect(invalid.status).toBe(400); + expect(invalid.body.error).toContain("rooms.turnTimeoutMinutes"); + } + + const saved = await api("PUT", "/api/config", { rooms: { turnTimeoutMinutes: 20 } }); + expect(saved.status).toBe(200); + expect(saved.body.rooms).toEqual({ turnTimeoutMinutes: 20 }); + + const after = await api("GET", "/api/config"); + expect(after.body.rooms).toEqual({ turnTimeoutMinutes: 20 }); + + const disk = JSON.parse(readFileSync(join(home, ".openmausbot", "config.json"), "utf8")); + expect(disk.rooms).toEqual({ turnTimeoutMinutes: 20 }); + + await api("PUT", "/api/config", { rooms: { turnTimeoutMinutes: 5 } }); + }); + + it("keeps an active turn alive when only the room timeout changes", async () => { + const created = await api("POST", "/api/bots", {}); + const botId = created.body.bot.id; + const room = (await api("POST", "/api/groups", { + name: "Room timeout capture", + memberIds: [botId], + })).body.group; + try { + const selected = await api("PATCH", `/api/bots/${botId}`, { + modelSelection: { instanceId: "claude", model: "claude-sonnet-5" }, + }); + expect(selected.status).toBe(200); + + rmSync(fakeClaudeDump, { force: true }); + const sent = await api("POST", `/api/groups/${room.id}/messages`, { text: "stay active" }); + expect(sent.status).toBe(202); + await expect.poll(() => existsSync(fakeClaudeDump), { timeout: 5_000 }).toBe(true); + + const before = (await api("GET", "/api/bots")).body; + expect(before.bots.find((bot: { id: string }) => bot.id === botId)?.busy).toBe(true); + expect(before.groups.find((group: { id: string }) => group.id === room.id)?.busyBotId).toBe(botId); + + const saved = await api("PUT", "/api/config", { rooms: { turnTimeoutMinutes: 20 } }); + expect(saved.status).toBe(200); + + const after = (await api("GET", "/api/bots")).body; + expect(after.bots.find((bot: { id: string }) => bot.id === botId)?.busy).toBe(true); + const activeRoom = after.groups.find((group: { id: string }) => group.id === room.id); + expect(activeRoom?.busyBotId).toBe(botId); + expect(activeRoom.messages.some((message: { tool?: { name?: string } }) => + message.tool?.name?.includes("provider settings changed"), + )).toBe(false); + } finally { + await api("POST", `/api/groups/${room.id}/interrupt`); + await expect.poll(async () => { + const state = (await api("GET", "/api/bots")).body; + return { + botBusy: state.bots.find((bot: { id: string }) => bot.id === botId)?.busy, + roomBusyBotId: state.groups.find((group: { id: string }) => group.id === room.id)?.busyBotId, + }; + }, { timeout: 5_000 }).toEqual({ botBusy: false, roomBusyBotId: null }); + await api("DELETE", `/api/groups/${room.id}`); + await api("DELETE", `/api/bots/${botId}`); + await api("PUT", "/api/config", { rooms: { turnTimeoutMinutes: 5 } }); + } + }); + it("validates the non-secret VPS alias and keeps old bots on Box by default", async () => { const before = await api("GET", "/api/bots"); const bot = before.body.bots[0]; diff --git a/server/index.ts b/server/index.ts index 28883047a9..2cef2b3609 100644 --- a/server/index.ts +++ b/server/index.ts @@ -13,6 +13,7 @@ import { approvalKey, autoVerdict } from "./auto-approve.ts"; import { appendDecision, readDecisions } from "./decision-log.ts"; import { validateBotCwd } from "./bot-cwd.ts"; import { groupTurnCwd } from "./room-cwd.ts"; +import { RoomTurnStallRegistry, roomTurnTimeoutMessage, scheduleRoomTurnTimeout } from "./room-turn-timeout.ts"; import * as box from "./box.ts"; import { cloudBackendChangeError, vpsAliasChangeError } from "./cloud-backend.ts"; import * as composio from "./composio.ts"; @@ -30,6 +31,7 @@ import { instanceConfigs, loadConfig, parseConfigPatch, + roomTurnTimeoutMinutes, saveConfig, syncCredentialEnv, withInstanceCli, @@ -516,12 +518,13 @@ const turnUsage = new Map(); const repeats = new RepeatDetector({ thresholds: [5, 10, 20], maxKeysPerThread: 256 }); // ── stall watchdog ───────────────────────────────────────────────────── -// ask_bot has a 4-minute ceiling and room turns a 5-minute one; the main -// 1:1 path had none, so a wedged CLI left its bot busy forever. The -// watchdog stops a turn whose thread has emitted NOTHING for stallMs — +// ask_bot has a 4-minute ceiling, while room turns have a separately +// configurable absolute ceiling. The main 1:1 path had none, so a wedged CLI +// left its bot busy forever. The watchdog stops a turn whose thread has emitted NOTHING for stallMs — // activity-based, so an hour-long turn that keeps streaming is never // touched, and turns parked on a human approval are exempt. const TURN_STALL_MS = Math.max(60_000, Number(process.env.OMB_TURN_STALL_MS) || 20 * 60_000); +const roomStallCompletions = new RoomTurnStallRegistry(); const watchdog = new TurnWatchdog({ stallMs: TURN_STALL_MS, checkMs: 60_000, @@ -538,6 +541,7 @@ const watchdog = new TurnWatchdog({ }); finalizeDelegationWatch(turn.threadId, false, "", "Delegated turn stalled and was stopped"); turnUsage.delete(turn.threadId); + roomStallCompletions.stall(turn.threadId); // ACP interruption settles within five seconds; other adapters settle // sooner. Keep ownership during that grace period so another turn cannot // overlap the process we are stopping. The normal turn.completed fold @@ -1821,30 +1825,36 @@ async function runGroupMemberTurn( // run the turn and wait for it to settle, folding the reply text so a // chained @mention can be routed afterwards let replyText = ""; - const outcome = await new Promise<"settled" | "dispatch_failed" | "timed_out">((resolve) => { + const timeoutMinutes = roomTurnTimeoutMinutes(cfg); + const outcome = await new Promise<"settled" | "dispatch_failed" | "stalled" | "timed_out">((resolve) => { let done = false; - const finish = (value: "settled" | "dispatch_failed" | "timed_out") => { + let timer!: ReturnType; + let unsub = () => {}; + let unregisterStall = () => {}; + const finish = (value: "settled" | "dispatch_failed" | "stalled" | "timed_out") => { if (done) return; done = true; clearTimeout(timer); unsub(); + unregisterStall(); resolve(value); }; - const unsub = bus.subscribe((e: RuntimeEvent) => { + unsub = bus.subscribe((e: RuntimeEvent) => { if (e.threadId !== group.threadId) return; if (e.type === "item.completed" && e.itemType === "assistant_text") replyText += `\n${e.text}`; else if (e.type === "turn.completed") finish("settled"); }); - const timer = setTimeout(() => { + timer = scheduleRoomTurnTimeout(timeoutMinutes, () => { void instance.adapter.interruptTurn(group.threadId).catch(() => {}); store.appendMessage(group.threadId, { role: "bot", kind: "activity", from: { botId: bot.id, name: bot.name, color: bot.color }, - tool: { name: `${bot.name}'s room turn exceeded 5 minutes and was stopped`, ok: false }, + tool: { name: roomTurnTimeoutMessage(bot.name, timeoutMinutes), ok: false }, }); finish("timed_out"); - }, 5 * 60_000); + }); + unregisterStall = roomStallCompletions.register(group.threadId, () => finish("stalled")); watchdog.watch(group.threadId, bot.id); instance.adapter .sendTurn({ @@ -1869,7 +1879,7 @@ async function runGroupMemberTurn( // A timed-out provider still owns the room thread until its interrupt // produces turn.completed (or the stall watchdog's grace fallback runs). // Do not clear busy or start the next member on that same thread early. - if (outcome === "timed_out") return false; + if (outcome === "stalled" || outcome === "timed_out") return false; // turn.completed normally performs this cleanup. Only use the fallback // when this invocation still owns the room; otherwise it would emit a // duplicate group frame or clear a newer speaker's state. @@ -2115,6 +2125,7 @@ function configStatus() { tts: tts.describeVoice(cfg), // not a secret — the sidebar shows it profile: { name: cfg.profile?.name ?? "", email: cfg.profile?.email ?? "" }, + rooms: { turnTimeoutMinutes: roomTurnTimeoutMinutes(cfg) }, }; } @@ -3642,12 +3653,13 @@ const server = createServer(async (req, res) => { syncCredentialEnv(patch); Object.assign(cfg, loadConfig()); } - // provider keys change the fleet; a profile or voice edit must not - // kill in-flight turns with a pointless reload — no driver reads - // either, and picking a voice mid-turn should be free - // The VPS alias is consumed by lifecycle commands, not provider - // engines. Saving it must not interrupt an in-flight turn. - if (Object.keys(patch).some((k) => k !== "profile" && k !== "tts" && k !== "vps")) await reloadProviders(); + // Provider keys change the fleet. Profile, voice, VPS, and room timeout + // changes do not rebuild it: no driver reads them, and they should not + // interrupt in-flight turns. + const reloadKeys = Object.keys(patch).filter( + (key) => key !== "profile" && key !== "tts" && key !== "vps" && key !== "rooms", + ); + if (reloadKeys.length > 0) await reloadProviders(); const status = configStatus(); broadcast({ kind: "config", ...status }); return json(res, 200, status); diff --git a/server/room-turn-timeout.test.ts b/server/room-turn-timeout.test.ts new file mode 100644 index 0000000000..f836a77678 --- /dev/null +++ b/server/room-turn-timeout.test.ts @@ -0,0 +1,93 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { + RoomTurnStallRegistry, + roomTurnTimeoutMessage, + roomTurnTimeoutMs, + scheduleRoomTurnTimeout, +} from "./room-turn-timeout.ts"; + +afterEach(() => vi.useRealTimers()); + +describe("room turn timeout", () => { + it("converts configured minutes to milliseconds", () => { + expect(roomTurnTimeoutMs(20)).toBe(20 * 60_000); + }); + + it("fires exactly at the configured absolute deadline", async () => { + vi.useFakeTimers(); + const onTimeout = vi.fn(); + const timer = scheduleRoomTurnTimeout(20, onTimeout); + + await vi.advanceTimersByTimeAsync(20 * 60_000 - 1); + expect(onTimeout).not.toHaveBeenCalled(); + + await vi.advanceTimersByTimeAsync(1); + expect(onTimeout).toHaveBeenCalledOnce(); + clearTimeout(timer); + }); + + it("can be cancelled when the room turn settles", async () => { + vi.useFakeTimers(); + const onTimeout = vi.fn(); + const timer = scheduleRoomTurnTimeout(5, onTimeout); + clearTimeout(timer); + + await vi.advanceTimersByTimeAsync(5 * 60_000); + expect(onTimeout).not.toHaveBeenCalled(); + }); + + it("settles a stalled room turn once, cancels its ceiling, and can be reused", async () => { + vi.useFakeTimers(); + const stalls = new RoomTurnStallRegistry(); + const onTimeout = vi.fn(); + let resolveFinished!: () => void; + const finished = new Promise((resolve) => { + resolveFinished = resolve; + }); + let completions = 0; + let timer!: ReturnType; + let unregister = () => {}; + const finish = () => { + if (completions > 0) return; + completions += 1; + clearTimeout(timer); + unregister(); + resolveFinished(); + }; + timer = scheduleRoomTurnTimeout(20, () => { + onTimeout(); + finish(); + }); + unregister = stalls.register("room-thread", finish); + + expect(stalls.stall("room-thread")).toBe(true); + expect(stalls.stall("room-thread")).toBe(false); + await finished; + expect(completions).toBe(1); + + await vi.advanceTimersByTimeAsync(20 * 60_000); + expect(onTimeout).not.toHaveBeenCalled(); + expect(completions).toBe(1); + + const settledElsewhere = vi.fn(); + const cleanup = stalls.register("room-thread", settledElsewhere); + cleanup(); + expect(stalls.stall("room-thread")).toBe(false); + expect(settledElsewhere).not.toHaveBeenCalled(); + + const nextTurn = vi.fn(); + stalls.register("room-thread", nextTurn); + expect(stalls.stall("room-thread")).toBe(true); + expect(nextTurn).toHaveBeenCalledOnce(); + }); + + it("formats singular and plural timeout messages", () => { + expect(roomTurnTimeoutMessage("Atlas", 1)).toBe( + "Atlas's room turn exceeded 1 minute and was stopped", + ); + expect(roomTurnTimeoutMessage("Atlas", 20)).toBe( + "Atlas's room turn exceeded 20 minutes and was stopped", + ); + }); +}); diff --git a/server/room-turn-timeout.ts b/server/room-turn-timeout.ts new file mode 100644 index 0000000000..44d803d17e --- /dev/null +++ b/server/room-turn-timeout.ts @@ -0,0 +1,35 @@ +export function roomTurnTimeoutMs(minutes: number): number { + return minutes * 60_000; +} + +export function scheduleRoomTurnTimeout( + minutes: number, + onTimeout: () => void, +): ReturnType { + return setTimeout(onTimeout, roomTurnTimeoutMs(minutes)); +} + +/** Completes the active room turn when its activity watchdog stalls. */ +export class RoomTurnStallRegistry { + private handlers = new Map void>(); + + register(threadId: string, handler: () => void): () => void { + this.handlers.set(threadId, handler); + return () => { + if (this.handlers.get(threadId) === handler) this.handlers.delete(threadId); + }; + } + + stall(threadId: string): boolean { + const handler = this.handlers.get(threadId); + if (!handler) return false; + this.handlers.delete(threadId); + handler(); + return true; + } +} + +export function roomTurnTimeoutMessage(botName: string, minutes: number): string { + const unit = minutes === 1 ? "minute" : "minutes"; + return `${botName}'s room turn exceeded ${minutes} ${unit} and was stopped`; +} diff --git a/server/turn-watchdog.ts b/server/turn-watchdog.ts index 539193a71e..68983d9d73 100644 --- a/server/turn-watchdog.ts +++ b/server/turn-watchdog.ts @@ -1,7 +1,8 @@ // Stall watchdog for dispatched turns. // -// ask_bot has a 4-minute ceiling and room turns a 5-minute one, but the -// main 1:1 path had none: a wedged CLI (hung network call, dead MCP child, +// ask_bot has a 4-minute ceiling, while room turns have a separately +// configurable absolute ceiling. The main 1:1 path had none: a wedged CLI +// (hung network call, dead MCP child, // a provider that stops streaming without exiting) left its bot busy // forever — composer locked, screen poller running — until an interrupt or // an app restart. This watchdog watches ACTIVITY, not duration: a turn may diff --git a/src/components/RoomTurnTimeoutSettings.tsx b/src/components/RoomTurnTimeoutSettings.tsx new file mode 100644 index 0000000000..0bae109f5c --- /dev/null +++ b/src/components/RoomTurnTimeoutSettings.tsx @@ -0,0 +1,97 @@ +import { useEffect, useRef, useState } from "react"; + +import { + MAX_ROOM_TURN_TIMEOUT_MINUTES, + MIN_ROOM_TURN_TIMEOUT_MINUTES, + createExclusiveSaveGate, + saveRoomTurnTimeoutMinutes, +} from "@/lib/room-turn-timeout"; +import { api, useStore, type ConfigStatus } from "@/state/store"; + +export function RoomTurnTimeoutSettings() { + const { state, dispatch } = useStore(); + const confirmedMinutes = state.config?.rooms.turnTimeoutMinutes ?? 5; + const [value, setValue] = useState(String(confirmedMinutes)); + const [dirty, setDirty] = useState(false); + const [error, setError] = useState(""); + const [saving, setSaving] = useState(false); + const saveGateRef = useRef(createExclusiveSaveGate()); + + useEffect(() => { + if (!dirty) setValue(String(confirmedMinutes)); + }, [confirmedMinutes, dirty]); + + const save = async () => { + if (!dirty || !saveGateRef.current.tryStart()) return; + setSaving(true); + try { + let savedConfig: ConfigStatus | undefined; + const result = await saveRoomTurnTimeoutMinutes(value, async (minutes) => { + const config = await api("/api/config", { + method: "PUT", + body: JSON.stringify({ rooms: { turnTimeoutMinutes: minutes } }), + }); + savedConfig = config; + return config.rooms.turnTimeoutMinutes; + }); + + if (!result.ok) { + setError(result.error); + return; + } + + if (savedConfig) dispatch({ type: "configStatus", config: savedConfig }); + setValue(String(result.minutes)); + setDirty(false); + setError(""); + } finally { + saveGateRef.current.finish(); + setSaving(false); + } + }; + + return ( +
+ +
+ { + setValue(event.target.value); + setDirty(true); + setError(""); + }} + onBlur={() => void save()} + onKeyDown={(event) => { + if (event.key === "Enter") event.currentTarget.blur(); + }} + className="min-w-0 flex-1 bg-transparent px-3 py-2 text-[14px] tabular-nums text-ink focus:outline-none" + /> + minutes +
+

+ Applies to every bot turn in rooms. Direct chats use the inactivity watchdog instead. +

+ {error ? ( + + ) : null} +
+ ); +} diff --git a/src/components/SettingsModal.tsx b/src/components/SettingsModal.tsx index a614a5def2..ebe0ed7b00 100644 --- a/src/components/SettingsModal.tsx +++ b/src/components/SettingsModal.tsx @@ -14,6 +14,7 @@ import { Card } from "./SettingsPrimitives"; import { UsageSection } from "./UsageSection"; import { VoiceSettings } from "./VoiceSettings"; import { SkinPicker } from "./SkinPicker"; +import { RoomTurnTimeoutSettings } from "./RoomTurnTimeoutSettings"; import { cn } from "@/lib/cn"; const SECTIONS: Array<{ id: AppSettingsSection; label: string; icon: typeof User }> = [ @@ -206,6 +207,9 @@ export function SettingsModal() { + + + )} diff --git a/src/lib/room-turn-timeout.test.ts b/src/lib/room-turn-timeout.test.ts new file mode 100644 index 0000000000..934e53783f --- /dev/null +++ b/src/lib/room-turn-timeout.test.ts @@ -0,0 +1,95 @@ +import { describe, expect, it } from "vitest"; + +import { + createExclusiveSaveGate, + parseRoomTurnTimeoutMinutes, + saveRoomTurnTimeoutMinutes, +} from "./room-turn-timeout"; + +describe("room turn timeout input", () => { + it.each(["", " ", "0", "1.5", "1441", "twenty"])( + "rejects %j", + (value) => { + expect(parseRoomTurnTimeoutMinutes(value)).toEqual({ + ok: false, + error: "Enter a whole number from 1 to 1,440.", + }); + }, + ); + + it.each([ + ["1", 1], + ["20", 20], + ["1440", 1440], + ])("accepts %s", (value, expected) => { + expect(parseRoomTurnTimeoutMinutes(value)).toEqual({ ok: true, minutes: expected }); + }); + + it("does not persist invalid input", async () => { + const persist = async (_minutes: number) => { + throw new Error("Persistence should not be called"); + }; + + await expect(saveRoomTurnTimeoutMinutes("1.5", persist)).resolves.toEqual({ + ok: false, + error: "Enter a whole number from 1 to 1,440.", + }); + }); + + it("persists parsed minutes and returns the confirmed value", async () => { + let receivedMinutes: number | undefined; + + const result = await saveRoomTurnTimeoutMinutes(" 20 ", async (minutes) => { + receivedMinutes = minutes; + return 25; + }); + + expect(receivedMinutes).toBe(20); + expect(result).toEqual({ ok: true, minutes: 25 }); + }); + + it("preserves an Error message when persistence fails", async () => { + const result = await saveRoomTurnTimeoutMinutes("20", async () => { + throw new Error("The server rejected this setting."); + }); + + expect(result).toEqual({ ok: false, error: "The server rejected this setting." }); + }); + + it("uses a fallback error for non-Error persistence failures", async () => { + const result = await saveRoomTurnTimeoutMinutes("20", async () => { + throw "unavailable"; + }); + + expect(result).toEqual({ ok: false, error: "Could not save the room turn limit." }); + }); + + it("blocks a second save while the first is pending and allows a later save", async () => { + let resolveFirstSave = () => {}; + const firstSaveDeferred = new Promise((resolve) => { + resolveFirstSave = resolve; + }); + const gate = createExclusiveSaveGate(); + let startedOperations = 0; + + const save = async (operation: Promise) => { + if (!gate.tryStart()) return false; + startedOperations += 1; + try { + await operation; + return true; + } finally { + gate.finish(); + } + }; + + const firstSave = save(firstSaveDeferred); + await expect(save(Promise.resolve())).resolves.toBe(false); + expect(startedOperations).toBe(1); + + resolveFirstSave(); + await expect(firstSave).resolves.toBe(true); + await expect(save(Promise.resolve())).resolves.toBe(true); + expect(startedOperations).toBe(2); + }); +}); diff --git a/src/lib/room-turn-timeout.ts b/src/lib/room-turn-timeout.ts new file mode 100644 index 0000000000..26c1fe7638 --- /dev/null +++ b/src/lib/room-turn-timeout.ts @@ -0,0 +1,53 @@ +export const MIN_ROOM_TURN_TIMEOUT_MINUTES = 1; +export const MAX_ROOM_TURN_TIMEOUT_MINUTES = 1_440; +export const ROOM_TURN_TIMEOUT_INPUT_ERROR = "Enter a whole number from 1 to 1,440."; + +export type RoomTurnTimeoutInput = + | { ok: true; minutes: number } + | { ok: false; error: string }; + +export type RoomTurnTimeoutSaveResult = RoomTurnTimeoutInput; + +export interface ExclusiveSaveGate { + tryStart: () => boolean; + finish: () => void; +} + +export function createExclusiveSaveGate(): ExclusiveSaveGate { + let active = false; + + return { + tryStart: () => { + if (active) return false; + active = true; + return true; + }, + finish: () => { + active = false; + }, + }; +} + +export function parseRoomTurnTimeoutMinutes(value: string): RoomTurnTimeoutInput { + const trimmed = value.trim(); + if (!/^\d+$/.test(trimmed)) return { ok: false, error: ROOM_TURN_TIMEOUT_INPUT_ERROR }; + const minutes = Number(trimmed); + if (minutes < MIN_ROOM_TURN_TIMEOUT_MINUTES || minutes > MAX_ROOM_TURN_TIMEOUT_MINUTES) { + return { ok: false, error: ROOM_TURN_TIMEOUT_INPUT_ERROR }; + } + return { ok: true, minutes }; +} + +export async function saveRoomTurnTimeoutMinutes( + value: string, + persist: (minutes: number) => Promise, +): Promise { + const parsed = parseRoomTurnTimeoutMinutes(value); + if (!parsed.ok) return parsed; + + try { + return { ok: true, minutes: await persist(parsed.minutes) }; + } catch (cause) { + return { ok: false, error: cause instanceof Error ? cause.message : "Could not save the room turn limit." }; + } +} diff --git a/src/state/store.test.ts b/src/state/store.test.ts index 2449aed512..baf24a6b49 100644 --- a/src/state/store.test.ts +++ b/src/state/store.test.ts @@ -1,6 +1,32 @@ import { describe, expect, it } from "vitest"; -import { initialState, reducer, type Bot, type Message } from "./store"; +import { configStatusFromFrame, initialState, reducer, type Bot, type Message } from "./store"; + +describe("config status frames", () => { + it("keeps the room turn timeout with the existing config fields", () => { + expect( + configStatusFromFrame({ + xai: { configured: true }, + composio: { configured: true, mode: "managed" }, + box: { configured: false }, + vps: { configured: true, sshAlias: "homelab" }, + rooms: { turnTimeoutMinutes: 20 }, + opencodeGo: { configured: true }, + tts: { configured: true, ready: true, voice: "Ada" }, + profile: { name: "Ian", email: "ian@example.test" }, + }), + ).toEqual({ + xai: { configured: true }, + composio: { configured: true, mode: "managed" }, + box: { configured: false }, + vps: { configured: true, sshAlias: "homelab" }, + rooms: { turnTimeoutMinutes: 20 }, + opencodeGo: { configured: true }, + tts: { configured: true, ready: true, voice: "Ada" }, + profile: { name: "Ian", email: "ian@example.test" }, + }); + }); +}); describe("cross-client bot creation", () => { it("adds an announced bot before its greeting frames arrive", () => { diff --git a/src/state/store.tsx b/src/state/store.tsx index 2f556dd0c6..339fb86ecc 100644 --- a/src/state/store.tsx +++ b/src/state/store.tsx @@ -214,6 +214,7 @@ export interface ConfigStatus { composio: { configured: boolean; mode?: "managed" | "self-hosted" | "unavailable" }; box: { configured: boolean }; vps: { configured: boolean; sshAlias: string }; + rooms: { turnTimeoutMinutes: number }; opencodeGo?: { configured: boolean }; /** Voice (ElevenLabs). `configured` = a key is saved; `ready` = a key AND * a voice, which is what it takes to actually speak. The key itself is @@ -223,6 +224,24 @@ export interface ConfigStatus { profile?: { name: string; email: string }; } +export type ConfigStatusFrame = Pick< + ConfigStatus, + "xai" | "composio" | "box" | "vps" | "rooms" | "opencodeGo" | "tts" | "profile" +>; + +export function configStatusFromFrame(frame: ConfigStatusFrame): ConfigStatus { + return { + xai: frame.xai, + composio: frame.composio, + box: frame.box, + vps: frame.vps, + rooms: frame.rooms, + opencodeGo: frame.opencodeGo, + tts: frame.tts, + profile: frame.profile, + }; +} + /** How an engine gets installed — declared by its driver, mirrors * EngineInstall in server/contracts.ts. Absent for engines that need no * local binary. `command` omits platforms that have no one-liner. */ @@ -1454,14 +1473,7 @@ export function StoreProvider({ children }: { children: ReactNode }) { case "config": rawDispatch({ type: "configStatus", - config: { - xai: frame.xai, - composio: frame.composio, - box: frame.box, - vps: frame.vps, - tts: frame.tts, - profile: frame.profile, - }, + config: configStatusFromFrame(frame), }); api("/api/instances") .then(({ instances }) => rawDispatch({ type: "instances", instances }))