Fix model draft desync - #331710
Conversation
There was a problem hiding this comment.
Pull request overview
Prevents agent-session draft synchronization from overwriting a remote model selection with a local fallback.
Changes:
- Adds model-aware draft synchronization state.
- Suppresses fallback publication for unavailable remote models.
- Adds focused synchronization tests.
Show a summary per file
| File | Description |
|---|---|
agentHostSessionHandler.ts |
Implements model-aware draft synchronization. |
draftSyncState.test.ts |
Tests fallback, publishing, and remote re-arming behavior. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Balanced
| if (this._appliedRemote | ||
| && sameDraftUserContent(outgoing, this._appliedRemote) | ||
| && !this._canResolveModel(this._appliedRemote) | ||
| ) { | ||
| return 'adopt'; |
There was a problem hiding this comment.
Good catch — fixed in ee85431.
_appliedRemote and the whole adopt/withhold mechanism are gone. The picker now reports why it holds a model (selectionReason), so an explicit pick carries UserSelection and is always published; only a model the input fell back to is held back. Nothing stays armed, so there is no state to consume.
Covered by draftSyncState.test.ts (userPicksModel → published, with the user's model).
| const modelId = this._toLanguageModelId(sessionResource, rawModelId); | ||
| return !!modelId && !!this._languageModelsService.lookupLanguageModel(modelId); |
There was a problem hiding this comment.
Agreed, and fixed in ee85431 — though by removing the predicate rather than extending it.
Mirroring the picker's session/mode eligibility here would duplicate logic that can drift out of sync (which is exactly how this gap appeared). Instead the input now records whether its model was picked by the user or fallen back to, and the sync reads that conclusion. So it no longer matters why the picker fell back — unregistered, invalid for the session, or unsupported in the current mode all behave the same, and the channel's model is preserved.
canResolveDraftModel is deleted; nothing in the handler re-derives model eligibility any more.
Have the picker say whether a model was chosen or fallen back to, instead of the draft sync guessing from whether it could look the model up. A window that can't offer the session's model shows its own default. Sending that default back overwrote the model another window's user had picked, and typing in that window did the same. Meanwhile a deliberate pick could be withheld, silently dropping the user's choice. The outbound sync now keeps the channel's model whenever the local one was only fallen back to, which removes the need to guess and covers every reason the picker falls back, not just an unknown model. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7b78dcd2-9eee-47cf-97a7-e0b60551e56e
No description provided.