From 5ae709fdda9963c681c3ea8b997fe6d48a31372f Mon Sep 17 00:00:00 2001
From: govind arora <aroraa5810@gmail.com>
Date: Sun, 17 May 2026 15:23:36 +0530
Subject: [PATCH] fix(agents): clear stuck Claude session on model swap + UX
 confirm
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

PATCH `/api/admin/agents/:id` now invalidates the per-task Claude
session when the model changes, so the next wake actually starts a
fresh CLI run on the new model instead of silently resuming the
previous session (which would keep using the old model regardless of
what the agent row says).

Backend (`admin-routes.ts`):

- When `body.model` changes, scan tasks assigned to this agent in
  active states whose latest `agent_runs.status` is **not** `running`,
  and `UPDATE tasks SET session_id = NULL`. In-flight runs are left
  alone — those finish on the old model, then the next wake takes the
  new model. Status enums imported from `@boringos/shared` instead of
  hardcoded strings.
- The PATCH response now carries a `sessionInvalidation: { tasksCleared,
  tasksDeferred }` block so the UI can render an accurate post-swap
  toast.
- The shell-agent guard is relaxed from "block all writes" to "allow
  model / runtimeId / fallbackRuntimeId / status / permissions /
  budgetMonthlyCents only", with everything else still blocked at 403.
  Without this, the model dropdown returned 403 on framework-source
  agents (copilot, replier, triage) which is exactly where the bug
  matters most.

Frontend (`AgentsPanel.tsx` + new `ConfirmModelChangeDialog.tsx`):

- The model `<select>` `onChange` now opens a confirmation dialog
  warning that the in-task Claude context will be lost. Pattern
  follows `ForceDeleteDialog` (no generic AlertDialog primitive yet).
- After the PATCH succeeds, render a `sonner` toast branched on the
  `sessionInvalidation` payload — distinguishes "next run uses X"
  from "N run(s) currently in progress will finish on the old model".

API typing (`client.ts`):

- `updateAgent` data shape was missing `model?: string | null` and the
  new `sessionInvalidation` return field; added both. Fixes silent
  drift between the runtime and the typed client.

Closes #17.
---
 packages/@boringos/core/src/admin-routes.ts   | 166 +++++++++++++++++-
 .../src/screens/Settings/AgentsPanel.tsx      |  80 ++++++++-
 .../Settings/ConfirmModelChangeDialog.tsx     |  90 ++++++++++
 packages/@boringos/ui/src/client.ts           |  11 +-
 4 files changed, 335 insertions(+), 12 deletions(-)
 create mode 100644 packages/@boringos/shell/src/screens/Settings/ConfirmModelChangeDialog.tsx

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 <oldSessionId> --model <new>`, 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<string, string>();
+        for (const w of wakeups) if (w.taskId) wakeToTask.set(w.id, w.taskId);
+
+        const wakeIds = [...wakeToTask.keys()];
+        const taskRunStatus = new Map<string, string>(); // 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<string | null>(null);
+  const [pendingModelChange, setPendingModelChange] = useState<PendingModelChange | null>(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() {
           </table>
         </div>
       </div>
+
+      {pendingModelChange && (
+        <ConfirmModelChangeDialog
+          agentName={pendingModelChange.agentName}
+          newModelLabel={modelLabel(pendingModelChange.newModel || null)}
+          previousModelLabel={
+            pendingModelChange.previousModel
+              ? modelLabel(pendingModelChange.previousModel)
+              : null
+          }
+          onConfirm={commitModelChange}
+          onCancel={() => setPendingModelChange(null)}
+        />
+      )}
     </div>
   );
 }
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 <sessionId>` ignores `--model <new>` 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<void>;
+  /** 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 (
+    <Dialog open onOpenChange={(open) => !open && !busy && onCancel()}>
+      <DialogContent>
+        <DialogHeader>
+          <DialogTitle>{headline}</DialogTitle>
+          <DialogDescription>
+            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.
+          </DialogDescription>
+        </DialogHeader>
+        <div className="text-xs text-muted-strong space-y-2">
+          <p>
+            Tasks currently mid-run keep using the old model until that run completes;
+            every wake afterwards uses {newModelLabel}.
+          </p>
+        </div>
+        <DialogFooter>
+          <Button variant="ghost" onClick={onCancel} disabled={busy}>
+            Cancel
+          </Button>
+          <Button variant="destructive" onClick={confirm} disabled={busy}>
+            {busy ? "Updating…" : `Switch to ${newModelLabel}`}
+          </Button>
+        </DialogFooter>
+      </DialogContent>
+    </Dialog>
+  );
+}
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<string, unknown>;
       budgetMonthlyCents?: number;
     },
-  ): Promise<Agent>;
+  ): Promise<Agent & { sessionInvalidation?: { tasksCleared: number; tasksDeferred: number } }>;
   patchAgentRoutingTags(
     agentId: string,
     data: { add?: string[]; remove?: string[]; set?: string[] },