Skip to content

Keep composer mode preferences stable#1658

Open
boudra wants to merge 2 commits into
mainfrom
workspace-mode-archive-reset
Open

Keep composer mode preferences stable#1658
boudra wants to merge 2 commits into
mainfrom
workspace-mode-archive-reset

Conversation

@boudra

@boudra boudra commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

Linked issue

N/A

Type of change

  • Bug fix
  • New feature (with prior issue + design alignment)
  • Refactor / code improvement
  • Docs

What does this PR do

The new workspace composer no longer silently drops a saved mode preference when provider metadata is temporarily incomplete or when a later partial preference update omits mode.

Provider preference updates now treat undefined as "leave this saved value alone," while real values still update the selected model, mode, thinking choice, and feature values. The form resolver also preserves explicit saved mode selections instead of defaulting them just because the current mode list cannot validate them.

How did you verify it

Reproduced the failure with red unit tests first:

  • saved Codex full-access mode was reset to auto when a loading provider snapshot omitted modes
  • a partial preference update with mode: undefined erased the saved mode

Confirmed green after the fix with:

  • npx vitest run packages/app/src/provider-selection/resolve-agent-form.test.ts packages/app/src/create-agent-preferences/preferences.test.ts packages/app/src/hooks/use-form-preferences.test.ts --bail=1
  • npm run lint -- packages/app/src/create-agent-preferences/preferences.ts packages/app/src/create-agent-preferences/preferences.test.ts packages/app/src/provider-selection/resolve-agent-form.ts packages/app/src/provider-selection/resolve-agent-form.test.ts
  • npm run typecheck
  • npm run format:files -- packages/app/src/create-agent-preferences/preferences.ts packages/app/src/create-agent-preferences/preferences.test.ts packages/app/src/provider-selection/resolve-agent-form.ts packages/app/src/provider-selection/resolve-agent-form.test.ts

The pre-commit hook also reran lint, format check, and typecheck successfully.

Checklist

  • One focused change. Unrelated cleanups split out.
  • npm run typecheck passes
  • npm run lint passes
  • npm run format ran (Biome)
  • UI changes include screenshots or video for every affected platform
  • Tests added or updated where it made sense

Treat undefined provider preference updates as no-ops, and keep saved mode selections instead of silently defaulting them when provider metadata is incomplete or does not list the saved mode.
@greptile-apps

greptile-apps Bot commented Jun 22, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes two bugs where the workspace composer could silently drop a saved mode preference: (1) a partial preference update with mode: undefined was spreading through to erase the stored value, and (2) form resolution was discarding a valid saved mode whenever the provider's modes list was absent (loading state) or didn't include the saved mode ID.

  • preferences.ts: Introduces applyProviderPreferenceUpdates that treats undefined field values as "leave existing value alone," replacing the previous ...existing, ...updates spread that would clobber saved fields with undefined.
  • resolve-agent-form.ts: Replaces validModeIds.includes(…) guards in resolveModeId, pickNextModeForProvider, and pickNextModeForProviderAndModel with a new resolvePreferredModeId helper that always honours a non-empty saved mode ID, deferring unknown-mode validation to the API submission boundary.

Confidence Score: 5/5

Safe to merge — two tightly scoped fixes with targeted regression tests for each scenario.

Both changes are narrow and well-bounded: the preference merge fix eliminates an undefined-spread clobber, and the form-resolver fix removes mode-list validation that was prematurely discarding saved user intent. Each fix is covered by a new regression test that would have caught the original bug. No new cross-cutting concerns, no data-loss paths, and the existing test suite was extended rather than worked around.

No files require special attention.

Important Files Changed

Filename Overview
packages/app/src/create-agent-preferences/preferences.ts Adds mergeDefinedRecord and applyProviderPreferenceUpdates helpers to replace the spread-based merge that could clobber saved fields with undefined; logic is correct and well-tested.
packages/app/src/provider-selection/resolve-agent-form.ts Extracts resolvePreferredModeId to preserve saved mode IDs without validating against the provider's current mode list; three call sites updated consistently.
packages/app/src/create-agent-preferences/preferences.test.ts Adds a targeted regression test covering the partial-update-with-undefined-mode scenario; tests cross the module interface and assert the full output shape.
packages/app/src/provider-selection/resolve-agent-form.test.ts Adds two regression tests — one for the loading-snapshot case (no modes in definition) and one for a saved mode absent from the current mode list — both asserting specific modeId values.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Preference update arrives\nwith mode: undefined] --> B{applyProviderPreferenceUpdates}
    B --> C{updates.mode !== undefined?}
    C -- No --> D[Keep existing mode\nsaved value intact]
    C -- Yes --> E[Write new mode value]

    F[Form resolution triggered\neg. provider snapshot arrives] --> G{resolveModeId}
    G --> H{userModified.modeId?}
    H -- Yes --> I[Return currentModeId as-is]
    H -- No --> J[resolvePreferredModeId]
    J --> K{initialModeId non-empty?}
    K -- Yes --> L[Return initialModeId\nno mode-list check]
    K -- No --> M{preferredModeId non-empty?}
    M -- Yes --> N[Return preferredModeId\nno mode-list check]
    M -- No --> O[Return providerDef.defaultModeId\nor first mode or empty]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[Preference update arrives\nwith mode: undefined] --> B{applyProviderPreferenceUpdates}
    B --> C{updates.mode !== undefined?}
    C -- No --> D[Keep existing mode\nsaved value intact]
    C -- Yes --> E[Write new mode value]

    F[Form resolution triggered\neg. provider snapshot arrives] --> G{resolveModeId}
    G --> H{userModified.modeId?}
    H -- Yes --> I[Return currentModeId as-is]
    H -- No --> J[resolvePreferredModeId]
    J --> K{initialModeId non-empty?}
    K -- Yes --> L[Return initialModeId\nno mode-list check]
    K -- No --> M{preferredModeId non-empty?}
    M -- Yes --> N[Return preferredModeId\nno mode-list check]
    M -- No --> O[Return providerDef.defaultModeId\nor first mode or empty]
Loading

Reviews (2): Last reviewed commit: "fix(app): document mode preference prese..." | Re-trigger Greptile

Comment thread packages/app/src/provider-selection/resolve-agent-form.ts
Comment thread packages/app/src/provider-selection/resolve-agent-form.ts Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant