fix: prevent ordinary prompt text from being interpreted as a mode switch (#3387)

[Hmbown]

Rebased, credit-preserving replacement for #3455 (which was red only due to a stale base; the diff itself is clean). Cherry-picked @mvanhorn's two commits onto current main so CI runs against the live tree.

Closes #3387.

What

RuntimeThreadManager previously coerced any unrecognized per-turn mode override to Agent via parse_mode. A stray prompt fragment in the mode slot could silently flip the thread's mode. This splits resolution:

• parse_mode_opt resolves only explicit tokens (agent/plan/yolo) and numeric aliases (1/2/3); anything else returns None.
• An unrecognized override now falls back to the thread's persisted mode instead of coercing to Agent.

This aligns with AGENTS.md: route/mode changes must come from explicit user choice, not from interpreting raw prompt text.

Testing

• cargo fmt -p codewhale-tui -- --check ✅
• cargo test -p codewhale-tui --bin codewhale-tui -- parse_mode → 6 passed ✅ (incl. new parse_mode_opt_resolves_explicit_tokens_and_aliases, parse_mode_opt_rejects_prompt_fragments, parse_mode_wrapper_defaults_and_resolves_numeric_aliases)

Harvested from #3455.

Co-authored-by: Matt Van Horn 455140+mvanhorn@users.noreply.github.com

[claude]

Claude finished @Hmbown's task in 1m 57s —— View job

---

PR Review: fix: prevent ordinary prompt text from being interpreted as a mode switch (#3387)

Tasks

• Gather context (PR metadata, branch diff)
• Read changed files
• Analyze correctness and logic
• Check provider/model/route safety invariants
• Review test coverage
• Post final review

---

Summary

The fix is correct and the approach is clean. The split into parse_mode_opt / parse_mode is idiomatic and the comment at the call site explains the invariant clearly. One pre-existing follow-up worth noting, one minor test observation.

---

Correctness

The core fix is sound. (runtime_threads.rs, lines 2033–2037)

Old: parse_mode(req.mode.as_deref().unwrap_or(&thread.mode)) — a stray prompt fragment like "plan a trip to Tokyo" fell through to the _ arm and silently coerced the mode to AppMode::Agent, even if the thread was already in Plan or Yolo.

New: req.mode.as_deref().and_then(parse_mode_opt).unwrap_or_else(|| parse_mode(&thread.mode)) — unrecognized overrides return None and fall back to the persisted thread mode. No silent coercion.

The numeric aliases ("1"/"2"/"3") are a bonus that the old parse_mode also lacked; they're correctly placed in parse_mode_opt so both the strict resolver and the fallback wrapper inherit them.

Pre-existing follow-up (not introduced here): update_thread and create_thread still write arbitrary strings to ThreadRecord::mode without calling parse_mode_opt for validation. After this PR, parse_mode(&thread.mode) at the start_turn call site handles any garbage value safely (returns AppMode::Agent), so there's no regression. But a client sending PATCH /v1/threads/{id} with {"mode": "gibberish"} will persist that string. Consider adding a parse_mode_opt(mode).is_some() guard in update_thread for parity. Fix this →

---

Provider/Model/Route Safety

Not touched by this PR. No code path infers a provider or model from freeform text. ✅

---

Tests

Coverage is solid:

• parse_mode_opt_resolves_explicit_tokens_and_aliases — hits all 6 recognized tokens including whitespace/casing (" PLAN ").
• parse_mode_opt_rejects_prompt_fragments — a representative set of multi-word inputs.
• parse_mode_wrapper_defaults_and_resolves_numeric_aliases — confirms the parse_mode shim inherits alias resolution and defaults to Agent.

Minor: the test name parse_mode_wrapper_defaults_and_resolves_numeric_aliases slightly undersells what it covers — parse_mode("plan a trip to Tokyo") is also tested there and is the bug's precise repro. Not blocking.

The existing parse_mode_defaults_to_agent test at line 2464 still passes because parse_mode("unknown") now routes through parse_mode_opt → None → AppMode::Agent. Consistent.

---

Good Choices

• Inverting the return type to Option<AppMode> rather than adding a sentinel value keeps callers honest.
• The doc-comment on parse_mode_opt naming the explicit entry points (Tab cycle, /mode, mode picker, config) is useful for future readers enforcing the invariant.
• Keeping parse_mode as a thin wrapper avoids touching the other call sites that legitimately want a default.

Verdict: Approve. The fix is minimal, correct, and well-tested. The update_thread validation gap is pre-existing and can be addressed separately.