fix: prevent model picker race condition when switching agent types

[chr1syy]

Summary

• Fixed race condition in MainPanel.tsx model/effort pill fetching where stale async responses from a previous agent could overwrite the current agent's model list
• Added stale flag with cleanup function to the useEffect that fetches models, effort options, and agent defaults
• Added test verifying that late-arriving model responses from a superseded agent are discarded

Root Cause

When switching between agents (e.g., OpenCode → Claude), the useEffect fired async calls for both. OpenCode's model discovery (opencode models subprocess) is slower than Claude's (local file read), so the stale OpenCode response could resolve last and overwrite Claude's model list — causing OpenCode models like github-copilot/gpt-5-mini to appear in a Claude-only session.

Test plan

• Added test: Model/effort pill race condition > should discard stale model responses when switching agent types
• All 135 MainPanel tests pass
• TypeScript type check clean
• ESLint clean
• Prettier clean

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Prevented stale in-flight model requests from overwriting the current agent's model/effort selections when switching agent types, improving model-list stability.

• Tests

  • Added test coverage that simulates agent switching and verifies model loading race conditions are handled correctly.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a3845b0a-c310-4e32-9fa1-385df1886078

📥 Commits

Reviewing files that changed from the base of the PR and between edafdb8 and f1233a9.

📒 Files selected for processing (1)

• src/__tests__/renderer/components/MainPanel.test.tsx

---

📝 Walkthrough

Walkthrough

Implements a stale-flag guard in MainPanel to prevent late-resolving agent model-discovery promises from overwriting state after switching activeSession.toolType. Adds a test that simulates switching from an OpenCode agent (deferred discovery) to a Claude agent (immediate), ensuring the late OpenCode response does not replace Claude models.

Changes

Cohort / File(s)	Summary
Race Condition Prevention in Model Loading src/renderer/components/MainPanel/MainPanel.tsx	Added a local stale flag in the effect that loads agent-specific pillModels/pillEfforts and default config. Handlers now short-circuit when stale to avoid late promise responses overwriting state.
Race Condition Test Coverage src/__tests__/renderer/components/MainPanel.test.tsx	Updated mocked InputArea to expose data-available-models. Added a test that switches active sessions from opencode (deferred) to claude-code (immediate) and asserts the late opencode response does not replace the claude-code model list.

Sequence Diagram(s)

sequenceDiagram
  participant MainPanel
  participant MaestroAgents as window.maestro.agents
  participant InputArea
  Note over MainPanel,MaestroAgents: Start model discovery for OpenCode (deferred)
  MainPanel->>MaestroAgents: getModels('opencode') -> deferred Promise
  Note over MainPanel: User switches activeSession.toolType -> 'claude-code'
  MainPanel->>MaestroAgents: getModels('claude-code') -> resolves immediately
  MaestroAgents-->>MainPanel: resolves with claude models
  alt effect not stale
    MainPanel->>MainPanel: setPillModels/efforts with Claude models
    MainPanel->>InputArea: render data-available-models = Claude models
  end
  Note over MaestroAgents,MainPanel: Later, deferred OpenCode promise resolves
  MaestroAgents-->>MainPanel: resolves with opencode models
  alt effect is stale
    MainPanel--xMaestroAgents: ignore response (stale guard prevents setState)
  else
    MainPanel->>InputArea: (would overwrite models) but guarded
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~18 minutes

Poem

🐰 I nibbled on async threads today,
Put a stale flag where late promises play,
Switches now safe, no models betrayed,
Hops and tests cheer the tidy parade! 🥕

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly summarizes the main change: preventing a model picker race condition when switching agent types, which is the core fix in MainPanel.tsx and the primary focus of the test additions.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/model-picker-race-condition

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[greptile-apps]

Greptile Summary

This PR adds a stale flag with a cleanup function to the useEffect in MainPanel.tsx that fetches models, effort levels, and agent defaults, preventing late-arriving async responses from a superseded agent from overwriting the current agent's model list. The fix is correct and follows the standard React cancellation pattern.

• The accompanying test's final assertions (toHaveBeenCalledWith('opencode') / toHaveBeenCalledWith('claude-code')) only verify that IPC calls fired; they do not assert on the resulting pillModels state, so the test would pass unchanged if the stale guard were removed. The comment "stale OpenCode models should NOT appear" describes intent that the code never actually checks.

Confidence Score: 5/5

Safe to merge — the production fix is correct and the only remaining finding is a P2 test-assertion gap.

The stale-flag implementation in MainPanel.tsx is correct and covers all three async calls. The sole finding is that the new test's assertions don't actually verify the race-condition behavior, which is a quality-of-test concern but not a production defect.

src/tests/renderer/components/MainPanel.test.tsx — assertions should be strengthened to actually catch a regression if the stale guard is removed.

Important Files Changed

Filename	Overview
src/renderer/components/MainPanel/MainPanel.tsx	Adds a stale flag + cleanup function to the model/effort useEffect; all three async callbacks (getModels, getConfigOptions, getConfig) correctly gate their setters behind if (!stale), fixing the race condition.
src/tests/renderer/components/MainPanel.test.tsx	Adds a race-condition test, but the final assertions only verify that getModels was called for both agents — they do not assert the resulting model list state, so the test would pass even without the fix.

Sequence Diagram

sequenceDiagram
    participant U as User
    participant MP as MainPanel useEffect
    participant OC as opencode getModels (slow)
    participant CC as claude-code getModels (fast)

    U->>MP: Render with opencode session
    MP->>OC: getModels('opencode') [stale=false]
    Note over OC: subprocess running…

    U->>MP: Switch to claude-code session
    MP-->>MP: cleanup() → stale=true (opencode effect)
    MP->>CC: getModels('claude-code') [new stale=false]
    CC-->>MP: ['sonnet','opus','haiku'] → setPillModels ✓

    OC-->>MP: ['gpt-5-mini','llama3'] (arrives late)
    MP-->>MP: stale=true → DISCARD, setPillModels NOT called ✓

Loading

_{Reviews (1): Last reviewed commit: "fix: prevent model picker race condition..." | Re-trigger Greptile}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/__tests__/renderer/components/MainPanel.test.tsx`:
- Around line 3542-3556: The test currently only checks both getModels calls
happened but doesn't verify Claude's models weren't overwritten by the late
opencode response; after calling resolveOpenCodeModels(openCodeModels) (the late
resolver), add an assertion that the component/state still reflects the Claude
models (e.g. assert rendered model names or the internal model list equals the
expected claudeModels) and assert that the opencode payload was not applied (for
example, ensure no subsequent call applied opencode models or that UI does not
contain any openCode model names); use the existing resolveOpenCodeModels,
openCodeModels and the mocked window.maestro.agents.getModels/response helpers
to locate where to add these assertions.

In `@src/renderer/components/MainPanel/MainPanel.tsx`:
- Around line 266-291: The three .catch handlers for the async calls to
window.maestro.agents (the getModels / getConfigOptions / getConfig promise
chains that call setPillModels, setPillEfforts, setAgentDefaultModel and
setAgentDefaultEffort) currently swallow errors; update each catch to report the
error via the Sentry utilities (import captureException or captureMessage from
src/utils/sentry.ts) with contextual info (agentId and which call: models,
effort/options, or config), then retain the existing fallback state updates
(e.g., setPillModels([]), setPillEfforts([]), setAgentDefaultModel(''),
setAgentDefaultEffort('')) so UI degrades while telemetry is recorded.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ba047a35-3cba-4de4-bffd-c243a8389fdb

📥 Commits

Reviewing files that changed from the base of the PR and between 3d52b3f and edafdb8.

📒 Files selected for processing (2)

• src/__tests__/renderer/components/MainPanel.test.tsx
• src/renderer/components/MainPanel/MainPanel.tsx