fix: five correctness bugs (model-wipe, infinite retry loop, type gap)

[dylanneve1]

Summary

Deep code review surfaced five bugs, three of them confirmed critical through independent verification.

1. CRITICAL — fallback_model path permanently wipes modelByBackend (3 files)

Files: src/backend/shared/handle-retry.ts, src/backend/openai-agents/handler.ts, src/backend/codex/handler.ts

After migrateLegacyModelField runs at startup, getChatSettings(chatId).model is undefined (the legacy field is deleted). The fallback_model retry path saved this as originalModel, then the finally block called setChatModel(chatId, undefined). Per chat-settings.ts line 346–354, passing undefined to setChatModel is the "reset everything" semantic: it deletes both modelByBackend and model, permanently wiping every per-backend model the user had selected.

Fix: Pass the fallback model via params.model instead of mutating chat settings. The swap is now purely in-memory — no setChatModel calls, no try/finally, and user preferences survive the retry untouched.

2. CRITICAL — openai-agents flow-violation check causes infinite retry loop

File: src/backend/openai-agents/handler.ts

detectFlowViolation was called with retried: _retried (boolean) but without retryCount or maxRetries. With the new retryCount-based API in flow-violation.ts, retried: true maps to retryCount = inputs.retryCount ?? 1. Since 1 < FLOW_VIOLATION_MAX_RETRIES (3) is always true, shouldRetry never becomes false. On a model that persistently violates the flow contract, the handler recurses indefinitely.

Fix: Explicitly pass retryCount: (_retried ? 1 : 0) and maxRetries: 1, preserving the single-pass behaviour documented in the handler's own comment. Also adds the missing toolCalls: streamState.toolCalls parameter so tool-call-only violations (model calls tools but no terminator) are detected correctly.

3. MEDIUM — openai-agents flow-violation log always says "trailing prose"

File: src/backend/openai-agents/handler.ts

The log line hardcoded "trailing prose (N chars)" even when the violation was caused by tool calls without a terminator. Now uses violation.reason, which is already correctly phrased for both cases by flow-violation.ts.

4. MEDIUM — RemoteAssistantInfo missing id field; dedup cast was untyped

File: src/backend/remote-server/session-helpers.ts

listSessionMessages deduplicates messages by info.id, but RemoteAssistantInfo didn't declare an id field. The working dedup relied on an ad-hoc double-cast (message as Record<string, unknown>)?.info as { id?: string } that TypeScript couldn't validate. A future refactor could silently break the cast without any type error.

Fix: Add id?: string to RemoteAssistantInfo and simplify the dedup cast to the typed (message as { info?: RemoteAssistantInfo })?.info?.id.

5. PLAUSIBLE — events.ts scope filter uses truthiness instead of !== undefined

File: src/backend/remote-server/events.ts

if (evtSessionID && evtSessionID !== ctx.sessionId) treats sessionID: "" (empty string) as "no session scope", allowing a session.turn.close with a blank sessionID to prematurely stop any session's SSE loop. The defensive fix uses !== undefined so only a truly absent sessionID is treated as unscoped.

---

Test plan

• npm test passes (2997 tests, 0 failures) ✅ verified locally
• shared-handle-retry.test.ts — updated two test cases to assert the new correct behaviour (fallback model injected via params.model, chat settings untouched)
• Manually verify model picker persists across a rate-limit fallback by checking getChatSettings after a fallback_model retry — modelByBackend should be unchanged
• Verify flow-violation retry in openai-agents backend stops after exactly one reminder (previously would loop indefinitely on a stubborn model)

https://claude.ai/code/session_01JpQ53zUQ5RpZq16mEH1Tgj

---

Generated by Claude Code