diff --git a/packages/@boringos/core/src/admin-routes.ts b/packages/@boringos/core/src/admin-routes.ts index dc1c0cc..34cf0a0 100644 --- a/packages/@boringos/core/src/admin-routes.ts +++ b/packages/@boringos/core/src/admin-routes.ts @@ -37,7 +37,7 @@ import { import type { AgentEngine, ToolRegistry } from "@boringos/agent"; import type { RuntimeRegistry } from "@boringos/runtime"; import type { StorageBackend } from "@boringos/drive"; -import { generateId } from "@boringos/shared"; +import { generateId, TASK_STATUSES, type TaskStatus } from "@boringos/shared"; import type { RealtimeBus } from "./realtime.js"; import type { EventBus } from "./event-bus.js"; import { syncArchive, syncStatusChange } from "./inbox-gmail-sync.js"; @@ -294,12 +294,47 @@ export function createAdminRoutes( values.reportsTo = body.reportsTo; } - // Task 07: Protect shell agents from modification - const agentRows = await db.select({ source: (await import("@boringos/db")).agents.source }) - .from((await import("@boringos/db")).agents).where(eq((await import("@boringos/db")).agents.id, agentId)).limit(1) as Array<{ source: string }>; + // Task 07: Shell-source agents (Copilot, Triage, Replier, etc.) + // are framework-owned — their identity (name, role, instructions, + // reportsTo) is immutable per release. BUT runtime/model swaps + // are benign: they change WHICH LLM executes the agent, not WHO + // the agent is. Allow that subset so operators can move the + // Copilot off Haiku without forking the framework. + // + // We also fetch the current `model` so the model-swap path below + // can detect an actual change vs. an idempotent PATCH that resends + // the same value. + const agentRows = await db + .select({ source: agents.source, model: agents.model }) + .from(agents) + .where(eq(agents.id, agentId)) + .limit(1) as Array<{ source: string; model: string | null }>; if (agentRows[0]?.source === "shell") { - return c.json({ error: "Framework agents (source='shell') cannot be modified" }, 403); + // Allowlist applies to body keys only; updatedAt is set by the + // server and never appears in `body`. Permissions and budgets + // are also tenant-controlled config (not identity), so they're + // included. + const SHELL_MUTABLE = new Set([ + "model", + "runtimeId", + "fallbackRuntimeId", + "status", + "permissions", + "budgetMonthlyCents", + ]); + const offending = Object.keys(body).filter((k) => !SHELL_MUTABLE.has(k)); + if (offending.length > 0) { + return c.json( + { + error: + `Framework agents (source='shell') only allow model/runtime/budget changes; ` + + `the following keys cannot be modified: ${offending.join(", ")}.`, + }, + 403, + ); + } } + const previousModel = agentRows[0]?.model ?? null; if (body.name !== undefined) values.name = body.name; if (body.role !== undefined) values.role = body.role; @@ -363,11 +398,128 @@ export function createAdminRoutes( ); const rows = await db.select().from(agents).where(eq(agents.id, c.req.param("id"))).limit(1); + // Session invalidation on model change. + // + // Background: `tasks.session_id` caches the Claude Code session + // an agent has been resuming for a given task. When the operator + // switches the agent's model, the engine still passes + // `--resume --model `, but Claude Code + // ignores `--model` on a resumed session and keeps the original + // model. Result: the model swap is silently a no-op. + // + // Fix: when `model` actually changed, NULL out `session_id` for + // every active task assigned to this agent so the next wake + // starts a fresh session under the new model. Tasks whose + // latest run is currently `running` are deferred — clearing + // session_id under a live run would race with the engine's + // post-run UPDATE. The next wake after that run finishes will + // pick up the cleared session_id. + let sessionInvalidation = { tasksCleared: 0, tasksDeferred: 0 }; + if (body.model !== undefined && body.model !== previousModel) { + // Active task statuses = anything that isn't terminal. We + // import the canonical list from @boringos/shared and filter + // out the terminal ones rather than hardcoding the in-flight + // set, so future statuses (e.g. 'in_review') are covered + // automatically. + const ACTIVE_TASK_STATUSES = TASK_STATUSES.filter( + (s): s is TaskStatus => s !== "done" && s !== "cancelled", + ); + const tenantId = c.get("tenantId"); + + // Candidate tasks: assigned to this agent, not terminal, + // currently carry a session_id (no point clearing nulls). + const candidates = await db + .select({ id: tasks.id }) + .from(tasks) + .where( + and( + eq(tasks.assigneeAgentId, c.req.param("id")), + eq(tasks.tenantId, tenantId), + sql`${tasks.status} = ANY(${ACTIVE_TASK_STATUSES})`, + sql`${tasks.sessionId} IS NOT NULL`, + ), + ); + + if (candidates.length > 0) { + // Latest-run-per-task: agent_runs link to tasks via + // wakeup_request_id → agent_wakeup_requests.task_id. We + // pull the most recent run per task and check its status. + const candidateIds = candidates.map((t) => t.id); + const wakeups = await db + .select({ id: agentWakeupRequests.id, taskId: agentWakeupRequests.taskId }) + .from(agentWakeupRequests) + .where( + sql`${agentWakeupRequests.taskId} = ANY(ARRAY[${sql.join( + candidateIds.map((id) => sql`${id}::uuid`), + sql`, `, + )}])`, + ); + + const wakeToTask = new Map(); + for (const w of wakeups) if (w.taskId) wakeToTask.set(w.id, w.taskId); + + const wakeIds = [...wakeToTask.keys()]; + const taskRunStatus = new Map(); // taskId → latest run status + if (wakeIds.length > 0) { + const runs = await db + .select({ + wakeupRequestId: agentRuns.wakeupRequestId, + status: agentRuns.status, + createdAt: agentRuns.createdAt, + }) + .from(agentRuns) + .where( + sql`${agentRuns.wakeupRequestId} = ANY(ARRAY[${sql.join( + wakeIds.map((id) => sql`${id}::uuid`), + sql`, `, + )}])`, + ) + .orderBy(desc(agentRuns.createdAt)); + for (const r of runs) { + const taskId = r.wakeupRequestId ? wakeToTask.get(r.wakeupRequestId) : undefined; + if (!taskId) continue; + // First-write-wins because the result set is desc(createdAt). + if (!taskRunStatus.has(taskId)) taskRunStatus.set(taskId, r.status); + } + } + + const toClear: string[] = []; + for (const t of candidates) { + if (taskRunStatus.get(t.id) === "running") { + sessionInvalidation.tasksDeferred += 1; + } else { + toClear.push(t.id); + } + } + + if (toClear.length > 0) { + await db + .update(tasks) + .set({ sessionId: null, updatedAt: new Date() }) + .where( + sql`${tasks.id} = ANY(ARRAY[${sql.join( + toClear.map((id) => sql`${id}::uuid`), + sql`, `, + )}])`, + ); + sessionInvalidation.tasksCleared = toClear.length; + } + } + } + if (body.reportsTo !== undefined) { emit("agent:reparented", c.get("tenantId"), { agentId: c.req.param("id"), reportsTo: values.reportsTo }); } - emit("agent:updated", c.get("tenantId"), { agentId: c.req.param("id") }); - return c.json(rows[0]); + emit("agent:updated", c.get("tenantId"), { + agentId: c.req.param("id"), + modelChanged: body.model !== undefined && body.model !== previousModel, + sessionInvalidation, + }); + // Attach `sessionInvalidation` so the frontend can branch its + // toast message between "next run uses Opus" and "current run + // finishes on Haiku, next run uses Opus". Older clients that + // ignore the field keep working. + return c.json({ ...rows[0], sessionInvalidation }); }); // Dedicated routing-tags endpoint: cheaper than round-tripping the diff --git a/packages/@boringos/shell/src/screens/Settings/AgentsPanel.tsx b/packages/@boringos/shell/src/screens/Settings/AgentsPanel.tsx index 8add73d..6dc908c 100644 --- a/packages/@boringos/shell/src/screens/Settings/AgentsPanel.tsx +++ b/packages/@boringos/shell/src/screens/Settings/AgentsPanel.tsx @@ -4,11 +4,13 @@ // Global pause toggle + per-agent controls: pause/resume, change model, view runs. import { useState } from "react"; +import { toast } from "sonner"; import { useAuth } from "../../auth/AuthProvider.js"; import { useAgents, useRuntimes, useSettings, useCosts } from "@boringos/ui"; import { Switch } from "../../components/ui/switch.js"; import { LoadingState, EmptyState } from "../_shared.js"; +import { ConfirmModelChangeDialog } from "./ConfirmModelChangeDialog.js"; // Mirrors `claudeRuntime.models` in @boringos/runtime. Picking any of // these on an agent sets `agents.model`; the engine then routes to the @@ -20,6 +22,19 @@ const CLAUDE_MODELS: Array<{ id: string; label: string }> = [ { id: "claude-opus-4-6", label: "Claude Opus 4.6" }, ]; +function modelLabel(id: string | null | undefined): string { + if (!id) return "Runtime default"; + const known = CLAUDE_MODELS.find((m) => m.id === id); + return known?.label ?? id; +} + +interface PendingModelChange { + agentId: string; + agentName: string; + newModel: string; // empty string means "Runtime default" + previousModel: string | null; +} + export function AgentsPanel() { const { user } = useAuth(); const { agents, isLoading: agentsLoading, updateAgent } = useAgents(); @@ -27,6 +42,7 @@ export function AgentsPanel() { const { settings, updateSettings } = useSettings(); const { costs } = useCosts(); const [error, setError] = useState(null); + const [pendingModelChange, setPendingModelChange] = useState(null); if (!user?.role || user.role !== "admin") { return ( @@ -70,12 +86,54 @@ export function AgentsPanel() { } }; - const handleModelChange = async (agentId: string, model: string) => { + // Two-phase model swap: opening this dialog is a no-op until the + // operator confirms. Confirming calls updateAgent and surfaces the + // server's `sessionInvalidation` payload through a sonner toast so + // the operator knows whether any in-flight runs are still on the + // old model. + const handleModelChange = (agentId: string, newModel: string) => { + const agent = agents?.find((a) => a.id === agentId); + if (!agent) return; + const previousModel = (agent as { model?: string | null }).model ?? null; + if ((newModel || null) === (previousModel || null)) return; // no-op + setError(null); + setPendingModelChange({ + agentId, + agentName: agent.name, + newModel, + previousModel, + }); + }; + + const commitModelChange = async () => { + if (!pendingModelChange) return; + const { agentId, newModel } = pendingModelChange; try { - setError(null); - await updateAgent({ agentId, data: { model: model || null } }); + const updated = (await updateAgent({ + agentId, + data: { model: newModel || null }, + })) as { sessionInvalidation?: { tasksCleared: number; tasksDeferred: number } }; + const inv = updated?.sessionInvalidation; + const niceLabel = modelLabel(newModel || null); + if (inv && inv.tasksDeferred > 0) { + toast.info( + `Model updated. ${inv.tasksDeferred} run${inv.tasksDeferred === 1 ? "" : "s"} ` + + `currently in progress will finish on the old model; subsequent runs use ${niceLabel}.`, + ); + } else if (inv && inv.tasksCleared > 0) { + toast.success( + `Model updated. ${inv.tasksCleared} task session${inv.tasksCleared === 1 ? "" : "s"} ` + + `reset; next run uses ${niceLabel}.`, + ); + } else { + toast.success(`Model updated. Next run uses ${niceLabel}.`); + } + setPendingModelChange(null); } catch (e) { - setError(e instanceof Error ? e.message : "Failed to update model"); + const msg = e instanceof Error ? e.message : "Failed to update model"; + setError(msg); + toast.error(msg); + setPendingModelChange(null); } }; @@ -194,6 +252,20 @@ export function AgentsPanel() { + + {pendingModelChange && ( + setPendingModelChange(null)} + /> + )} ); } diff --git a/packages/@boringos/shell/src/screens/Settings/ConfirmModelChangeDialog.tsx b/packages/@boringos/shell/src/screens/Settings/ConfirmModelChangeDialog.tsx new file mode 100644 index 0000000..55600b7 --- /dev/null +++ b/packages/@boringos/shell/src/screens/Settings/ConfirmModelChangeDialog.tsx @@ -0,0 +1,90 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +// +// Confirmation dialog for the per-agent model swap in +// Settings → Agents. +// +// Why this exists: changing `agents.model` does not just update +// a label — the framework also clears `tasks.session_id` for any +// task that was mid-conversation, because Claude Code's +// `--resume ` ignores `--model ` and keeps the +// original session's model. Clearing session_id is the only way +// to make the next wake actually use the new model. +// +// The dialog tells the operator that explicitly so the in-task +// context loss is a deliberate choice rather than a surprise. + +import { useState } from "react"; + +import { Button } from "../../components/ui/button.js"; +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from "../../components/ui/dialog.js"; + +export interface ConfirmModelChangeDialogProps { + /** Display name of the agent being changed (used in copy). */ + agentName: string; + /** Pretty label of the new model (e.g. "Claude Opus 4.6"). */ + newModelLabel: string; + /** Pretty label of the previous model, or null when previously unset. */ + previousModelLabel: string | null; + /** Called when the operator confirms. Errors thrown here surface back to the parent. */ + onConfirm: () => Promise; + /** Called when the dialog is dismissed without confirming. */ + onCancel: () => void; +} + +export function ConfirmModelChangeDialog({ + agentName, + newModelLabel, + previousModelLabel, + onConfirm, + onCancel, +}: ConfirmModelChangeDialogProps) { + const [busy, setBusy] = useState(false); + + const headline = previousModelLabel + ? `Switch ${agentName} from ${previousModelLabel} to ${newModelLabel}?` + : `Set ${agentName} to ${newModelLabel}?`; + + async function confirm() { + setBusy(true); + try { + await onConfirm(); + } finally { + setBusy(false); + } + } + + return ( + !open && !busy && onCancel()}> + + + {headline} + + Switching this agent's model starts a fresh Claude session on the next run. + Any in-task context from the current session will be lost. + + +
+

+ Tasks currently mid-run keep using the old model until that run completes; + every wake afterwards uses {newModelLabel}. +

+
+ + + + +
+
+ ); +} diff --git a/packages/@boringos/ui/src/client.ts b/packages/@boringos/ui/src/client.ts index 5b9e5ec..98960b4 100644 --- a/packages/@boringos/ui/src/client.ts +++ b/packages/@boringos/ui/src/client.ts @@ -295,12 +295,21 @@ export interface BoringOSClient { status?: string; runtimeId?: string | null; fallbackRuntimeId?: string | null; + /** + * Per-agent model override (e.g. `claude-sonnet-4-6`). When set, + * takes priority over the resolved runtime row's model at run + * time. Setting / changing this server-side also clears + * `tasks.session_id` for active tasks so the next wake starts + * a fresh session under the new model. The PATCH response + * carries `sessionInvalidation: { tasksCleared, tasksDeferred }`. + */ + model?: string | null; reportsTo?: string | null; routingTags?: string[]; permissions?: Record; budgetMonthlyCents?: number; }, - ): Promise; + ): Promise; patchAgentRoutingTags( agentId: string, data: { add?: string[]; remove?: string[]; set?: string[] },