← Back to PRs

#19328: Fix: preserve modelOverride in agent handler (#5369)

by CodeReclaimers open 2026-02-17 17:50 View on GitHub →
gateway size: M
## Summary Fixes the intermittent race condition where sub-agent model overrides were silently ignored (~3% failure rate). - When `sessions_spawn` sets a model override via `sessions.patch`, the agent handler could read stale cached data and overwrite the fresh `modelOverride` when writing back to the session store - The fix moves session entry construction inside the `updateSessionStore` mutator, ensuring it always uses fresh store data (loaded with `skipCache: true`) ## Root Cause The agent handler was: 1. Reading session entry early via `loadSessionEntry()` (could return cached/stale data) 2. Building `nextEntry` with `modelOverride: entry?.modelOverride` (potentially undefined) 3. Calling `updateSessionStore()` which correctly re-reads with `skipCache: true` 4. **But the mutator ignored the fresh store data** and just wrote `store[key] = nextEntry` This meant freshly-written `modelOverride` values from `sessions.patch` could be overwritten. ## Changes - `src/gateway/server-methods/agent.ts`: Build session entry inside `updateSessionStore` using fresh store data - `src/gateway/server-methods/agent.test.ts`: Add 4 comprehensive tests for the fix ## Test plan - [x] Existing tests updated and passing - [x] New test: `issue #5369: agent handler preserves fresh modelOverride from store` - [x] New test: `issue #5369: request params override fresh store values for label/spawnedBy` - [x] New test: `issue #5369: creates new entry when store has no existing entry` - [x] New test: `issue #5369: preserves all important fields from fresh store` - [x] Full test suite passes (5,043 tests) Fixes #5369 Supersedes #8398 (rebased onto latest main to resolve conflicts). <!-- greptile_comment --> <h3>Greptile Summary</h3> Fixes race condition where sub-agent `modelOverride` values set via `sessions.patch` could be silently overwritten. The agent handler previously built session entries using potentially stale cached data, then overwrote fresh store values. The fix moves session entry construction inside the `updateSessionStore` mutator, ensuring it always reads and uses fresh store data (loaded with `skipCache: true`). - Moved session entry construction from outside to inside `updateSessionStore` mutator (src/gateway/server-methods/agent.ts:391-443) - Uses `freshEntry` from store instead of stale `entry` for all fields that `sessions.patch` can modify - Adds proper `sendPolicy` denial handling inside the mutator with structured return value - Handles both `storePath` and no-storePath (fallback) cases correctly - Added 4 comprehensive regression tests specifically for issue #5369 - Updated existing tests to handle new mutator return value structure <h3>Confidence Score: 5/5</h3> - Safe to merge - well-tested fix for documented race condition - The fix correctly addresses the root cause by ensuring session entry construction uses fresh store data. The implementation is clean, properly handles edge cases (storePath vs no-storePath, sendPolicy denial), and includes comprehensive test coverage specifically targeting the race condition. The changes are localized to the affected code path with minimal risk of side effects. - No files require special attention <sub>Last reviewed commit: ffe4cde</sub> <!-- greptile_other_comments_section --> <sub>(5/5) You can turn off certain types of comments like style [here](https://app.greptile.com/review/github)!</sub> <!-- /greptile_comment -->

Most Similar PRs