fix(agents): clear stuck Claude session on model swap + UX confirm

[Govindaroraaa]

Summary

PATCH /api/admin/agents/:id now invalidates the per-task Claude session when the model changes, so the next wake starts a fresh CLI run on the new model. Without this the agent row reads "claude-sonnet-4-5" while the actual run keeps resuming the previous Haiku session via --resume <session_id>. Closes #17.

Backend (admin-routes.ts):

• On model change: scan tasks assigned to the agent in active statuses whose latest agent_runs.status is not running, and UPDATE tasks SET session_id = NULL. In-flight runs finish on the old model, then the next wake takes the new model.
• Response now carries sessionInvalidation: { tasksCleared, tasksDeferred } for accurate UX.
• Shell-source agent guard relaxed: model / runtimeId / fallbackRuntimeId / status / permissions / budgetMonthlyCents are now allowed (instead of returning 403 on every write). Everything else still 403s.

Frontend (AgentsPanel.tsx, new ConfirmModelChangeDialog.tsx):

• Confirmation dialog (mirroring ForceDeleteDialog) warns the user that in-task context will be lost.
• sonner toast after success branches on the sessionInvalidation payload — distinguishes "next run uses X" from "N in-flight run(s) finish on the old model".

API typing (client.ts):

• updateAgent was missing model?: string | null and the new return field; both added.

Test plan

• Trigger a triage run so a task gets a session_id.
• Switch the agent's model in Settings → Agents — confirmation dialog appears.
• DB inspection: matching tasks have session_id = NULL; tasks whose latest run is running keep their session_id.
• Next wake: new run uses the new model (verified in cost_events.model).
• Toast wording reflects which path was taken.
• Framework / shell-source agents (copilot etc.) no longer return 403 on model change.

Made with Cursor

[parag]

Requesting changes — but to be clear, this isn't an "address comments and re-push" request. The architectural decision behind this PR is wrong for the problem it's solving, and I'd like you to abandon this branch, open a fresh branch, and submit a new PR with the approach below. Leaving this PR open as a reference while the new one lands; we'll close this when the replacement merges.

The bug is real. The fix shape isn't. Here's the long version of why, in plain terms.

---

What tasks.session_id actually is

Every task carries one column: session_id. When the engine wakes an agent on a task, it reads that column, finds a UUID, and spawns the Claude CLI with --resume <that uuid>. Claude continues the conversation where it left off — same message history, same tools loaded, same model.

That session_id is a cache. Not a foreign key, not a hard reference. The engine treats it as "if there's a usable Claude session for this task, here's its ID." Claude itself stores the actual session content on disk. The DB column is just a pointer.

When a run finishes, the engine writes the new session_id back (engine.ts:353-358). When the next wake on that task happens, the engine reads it and passes --resume. That's the entire mechanism.

What a cache needs

Every cache has a key. The key has to capture everything the cached value depends on. If your cached value depends on (X, Y, Z) but your key is only X, then when Y or Z changes you silently serve stale data. Standard cache-invalidation lesson: the bug isn't "we forgot to invalidate," it's "our key didn't reflect our actual dependencies."

What does a Claude session actually depend on?

• The task — yes, that's what the current key captures. Each task has its own conversation thread.
• The model — a session created with Sonnet 4.6 is a Sonnet 4.6 session. Claude bakes the model into the session on disk. --resume ignores --model because the session already knows which model it belongs to.
• The runtime — a Claude session UUID is meaningless to Codex. They speak different session formats. Passing a Claude UUID to Codex is like handing someone the wrong house key.

So the real cache key is (task, runtime, model). But today's column is keyed on just task. That's the bug. Everything else is a symptom.

What this PR is doing, in those terms

The PR sees the symptom and tries to manually invalidate the cache from a sibling system: "when the agent's model changes, go reach into the tasks table and NULL out the cached session_id." Right intent — pop the cache entry — but wrong place.

Manual invalidation from another component has three classic problems, all of which this PR hits:

1. Race conditions. Between "I checked nothing's using it" and "I cleared the entry," a reader can come along and grab the entry. The PR's "deferred running runs" logic is trying to dodge this, but there's a window: a queued wake can transition to running between the status check (admin-routes.ts:489-517) and the UPDATE. The engine reads the old session_id at engine.ts:181-186, then completes and writes it back at engine.ts:353-358, overwriting our NULL. Standard cache-invalidation hazard.

2. Cross-module knowledge. The PATCH endpoint in admin-routes.ts now has to know about tasks.session_id, agent_runs.status, agent_wakeup_requests, and the engine's session-resume mechanism. The admin route is reaching across module boundaries to clean up the engine's cache. The next time anyone changes the engine's caching strategy, they have to remember to update this PATCH handler too — and they won't, because admin routes have nothing to do with the run lifecycle in any other respect.

3. Incomplete invalidation. It fires only on model change, not on runtimeId change — even though the PR does allowlist runtimeId for shell agents (admin-routes.ts:314). Why? Because the author thought about model and didn't think about runtime. That's exactly what happens when invalidation is a manual checklist instead of a property of the cache itself.

The architectural shift

Move the responsibility from "the writer remembers to invalidate" to "the reader checks if the entry is still valid." Same change a CPU cache makes when it adds a tag to each line.

Concretely: store the model + runtime that the session was created under, alongside the session_id. One column, called something like session_fingerprint, holds a string like "claude:claude-sonnet-4-6".

Two places in the engine change:

On write (when a run completes and we cache the session_id):

• Store the session_id and the fingerprint of what just ran. "This session belongs to Claude runtime, Sonnet 4.6."

On read (when starting the next wake):

• Read the session_id and the fingerprint.
• Compute the current fingerprint from the agent's current model/runtime.
• If they match, pass --resume. If they don't, ignore the cached session_id and start fresh.

That's it. The admin PATCH endpoint goes back to being a dumb UPDATE agents SET model = ?. It doesn't need to know anything about session caching. The engine knows.

Why this is dramatically simpler

• No race condition. There's no moment of "I'm clearing this." The reader does a single SELECT, compares two strings, and decides. If a concurrent run is starting, it does the same SELECT-and-compare and reaches the same decision. No interleaving hazard.
• No "deferred" state. In-flight runs were going to finish on the old model regardless — you can't reach into a running CLI process and swap its model mid-conversation. The only thing the PR's "deferred" logic does is not interfere with that. Removing the manual invalidation entirely removes the need to think about deferring.
• No UX confirmation dialog. The mental model is now "I changed the model; next run uses it." Nothing got destroyed. The previous session still exists on Claude's disk — it's just not reused because it's the wrong model. If we ever want to be fancy, switching back to the old model could re-resume the old session (since its fingerprint would match again).
• Codex integration falls out for free. The fingerprint includes runtime. Day one, swapping Claude → Codex correctly starts a fresh session, because claude:sonnet ≠ codex:o1. No additional code, no second invalidation path.
• Future model/runtime additions need zero work. Add Haiku 5, add Gemini, add a custom runtime — the cache key absorbs them automatically.

The mental model in one sentence

This PR is asking "when the model changes, what should we clean up?" The right question is "what does a session belong to, and how do we make that obvious to the reader?" Once you store the second answer in the same row as the session_id, the first question disappears.

Why this matters beyond this one fix

The PR's solution scales poorly. Every future thing that affects which model actually runs — runtime config changes, permission changes that gate model access, tenant-level model overrides, A/B tests routing traffic to different models — would each need its own invalidation hook in PATCH-land. The fingerprint approach absorbs all of them: whatever the engine actually resolves to at wake time becomes the cache key, and any divergence from the stored fingerprint is a miss. The cache is keyed on the thing that actually matters (what runs), not on the thing that probably matters (which column got UPDATEd).

That's the difference between fixing a bug and fixing the class of bug.

---

What I'd like you to do

1. Leave this PR open for now as the reference for the bug + the discussion.
2. Open a fresh branch off main (suggested name: fix/issue-17-session-fingerprint) and submit a new PR with:
   • One migration adding tasks.session_fingerprint (text, nullable).
   • ~5 lines in engine.ts at the write site (engine.ts:353-358) to also persist the fingerprint.
   • ~5 lines in engine.ts at the read site (engine.ts:181-186) to compare fingerprints and only set previousSessionId on a match.
   • A unit test covering: same-fingerprint → resumes; different model → fresh session; different runtime → fresh session; null fingerprint (legacy row) → fresh session.
   • No changes to admin-routes.ts.
   • No changes to the shell UI (no confirm dialog, no toast branching). If we want to communicate "your next run starts a new session" in the model dropdown, that's a separate, optional 5-line tooltip PR after this lands.
3. Once the new PR merges, close this one.

The fingerprint approach is small enough to land in one tight PR with tests. Happy to pair on the engine read/write sites if helpful, but this should be a half-day at most. Issue #17 stays open as the tracking issue until the replacement merges.

Appreciate the work that went into this branch — the root-cause analysis (sessions get pinned to the model that created them) is correct, and that insight carries straight over to the new approach.