Skip to content

feat(desktop): simplify provider cards in Settings → API 服务 (re-open)#117

Open
hqhq1025 wants to merge 2 commits intomainfrom
wt/settings-ui-simplify
Open

feat(desktop): simplify provider cards in Settings → API 服务 (re-open)#117
hqhq1025 wants to merge 2 commits intomainfrom
wt/settings-ui-simplify

Conversation

@hqhq1025
Copy link
Copy Markdown
Collaborator

Reopen of #116 after #113 squash-merge auto-closed it. Rebased onto current main; first commit (the one duplicated by #113's squash) was dropped via rebase --skip. The 2 fixes from codex re-review are included.

Summary

  • Provider card collapsed to one header row: label + masked key + baseUrl + (active badge OR "设为当前") + ··· overflow menu
  • Active card: model rendered as click-to-edit chip → inline NativeSelect commits and closes on change. Optimistic update with rollback — on save failure the chip reverts to its previous value
  • Non-active cards: model selector hidden entirely
  • Overflow menu: Test connection (active only), Re-enter key, Delete with inline confirm
  • "Test connection" wraps the IPC call in try/catch — bridge errors surface as toasts, not unhandled rejections

Test plan

  • vitest 337 pass
  • typecheck clean
  • biome clean
  • DCO signed

🤖 Generated with Claude Code

Collapse each provider card to one header row (label, masked key, optional
base URL). The "Active" badge or a compact "set active" button sits on the
right; the model selector is hidden on non-active providers and rendered as
a click-to-edit chip on the active card. Per-row actions (Test connection,
Re-enter key, Delete) move into a ··· overflow menu, and the duplicated
dashed "Add provider" button at the bottom of the list is removed.

Signed-off-by: hqhq1025 <1506751656@qq.com>
… failure

Codex flagged two Major issues in #116:
- handleTestConnection awaited testActive() without try/catch, leaking
  bridge errors as unhandled promise rejections.
- Model chip optimistically set new value before save completed; on save
  failure the UI showed an unsaved model. Now save() returns boolean and
  handleChange rolls back primary to its previous value when save fails.

Signed-off-by: hqhq1025 <1506751656@qq.com>
Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • [Major] Silent bridge fallback violates the no-silent-fallback constraint — both new code paths return early when window.codesign is unavailable, with no surfaced error (apps/desktop/src/renderer/src/components/Settings.tsx:657, apps/desktop/src/renderer/src/components/Settings.tsx:768). This can fail user actions without UI feedback.
    Suggested fix:

    if (!window.codesign) {
      pushToast({
        variant: 'error',
        title: t('settings.providers.toast.connectionFailed'),
        description: t('settings.common.unknownError'),
      });
      return;
    }
  • [Major] Optimistic model rollback can revert to stale value on overlapping saves — handleChange captures prev per call and rolls back on failure (apps/desktop/src/renderer/src/components/Settings.tsx:786). If two saves overlap, a late failure from an earlier selection can overwrite a later successful change.
    Suggested fix:

    const saveSeq = useRef(0);
    
    function handleChange(v: string) {
      const prev = primary;
      const seq = ++saveSeq.current;
      setPrimary(v);
      setEditing(false);
      void save(v).then((ok) => {
        if (!ok && seq === saveSeq.current) setPrimary(prev);
      });
    }

Summary

  • Review mode: initial
  • 2 issues found in modified lines (silent fallback + stale rollback race).
  • docs/VISION.md and docs/PRINCIPLES.md were Not found in repo/docs in this workspace, so principle checks were done against available CLAUDE.md constraints plus the diffed files.
  • Residual risk: no test updates for new overflow-menu actions and optimistic rollback behavior.

Testing

  • Not run (automation)

open-codesign Bot

: 'border-[var(--color-border)] bg-[var(--color-surface)]';

async function handleTestConnection() {
if (!window.codesign) return;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (!window.codesign) return; silently drops the action. Project constraints require surfaced errors/no silent fallback. Please toast or throw with context here.

if (saveTimeout.current !== null) clearTimeout(saveTimeout.current);
saveTimeout.current = setTimeout(() => void save(v), 400);
setEditing(false);
void save(v).then((ok) => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rollback is race-prone across overlapping saves: a late failure from an earlier change can reset a newer successful selection. Please gate rollback with a request sequence/id (or cancel stale requests).

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