Skip to content

fix: prevent API Test model persistence - #74

Merged
orangeboyChen merged 1 commit into
mainfrom
codex/fix-api-test-model-persistence
Aug 14, 2026
Merged

fix: prevent API Test model persistence#74
orangeboyChen merged 1 commit into
mainfrom
codex/fix-api-test-model-persistence

Conversation

@orangeboyChen

Copy link
Copy Markdown
Owner

Summary

  • Prevent API Test credential switching from calling the model-discovery endpoint that persists supported_models.
  • Load the saved per-credential model list through the read-only endpoint when entering API Test.

Root cause

Switching credentials in API Test used POST /admin-api/credentials/models, which writes the discovery result into the selected credential.

Validation

  • bun run lint
  • bun run format:check
  • bun run typecheck
  • bun run test:coverage (93.86% statements)
  • bun run build
  • bun run test:ci

@orangeboyChen
orangeboyChen marked this pull request as ready for review August 14, 2026 06:40
@codecov

codecov Bot commented Aug 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.67%. Comparing base (737603d) to head (38eb22d).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #74      +/-   ##
==========================================
+ Coverage   94.48%   94.67%   +0.18%     
==========================================
  Files          20       20              
  Lines        3429     3569     +140     
  Branches      947      971      +24     
==========================================
+ Hits         3240     3379     +139     
- Misses        189      190       +1     
Flag Coverage Δ
unittests 94.67% <ø> (+0.18%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

View 1 additional finding in Devin Review.

Open in Devin Review

Comment thread app/page-shell.tsx
Comment on lines 1951 to 1953
credentialFilename: value,
model: models[0] ?? '',
}));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Model dropdown can end up empty after switching credentials in API Test

Switching to another credential in API Test now only reads the previously saved per-credential model list (removal of the model fetch at app/page-shell.tsx:1953), so a credential with no saved list shows an empty model picker with no way to populate it from that screen.
Impact: Users who pick such a credential see no models to choose from and their test request goes out without a model choice.

How the empty list arises from the per-credential fallback

Before this change, onCredentialChange called refreshCredentialModels(value) which POSTed to /admin-api/credentials/models, performing live discovery for that credential and updating modelRows/apiTest.model. That call was removed, and the new loadCredentialModels (app/page-shell.tsx:510-544) is only invoked when initialData is absent (app/page-shell.tsx:1522-1528) — in the real app initialData is always provided by app/page.tsx:58, so no client fetch ever happens on the API Test tab.

The options list is then computed as credentials.modelRows[filename]?.models ?? (initialData.credentialModels[filename] ?? initialData.models) (app/page-shell.tsx:1936-1942). initialData.credentialModels is built for every eligible credential from persisted supported_models (app/page-loader.ts:153-158, lib/server/domain/credentials.ts:256-271), so a credential whose supported_models is empty maps to [], which is not nullish and therefore never falls back to initialData.models. Result: empty dropdown, apiTest.model === '', and the request at app/page-shell.tsx:1265 sends an empty model (the server then silently substitutes a default in lib/server/proxy/codebuddy.ts:686-689).

A fallback to the aggregate initialData.models when the per-credential list is empty would restore usable behavior without re-introducing the persisting POST.

(Refers to lines 1943-1953)

Prompt for agents
In app/page-shell.tsx the API Test tab no longer fetches models when the user switches credentials (the POST to /admin-api/credentials/models was intentionally removed to avoid persisting supported_models). The remaining sources are credentials.modelRows and initialData.credentialModels, both keyed by filename. Because initialData.credentialModels contains an entry for every eligible credential (empty array when the credential has no persisted supported_models), the `?? initialData.models` fallback at app/page-shell.tsx:1936-1942 and the `?? []` at app/page-shell.tsx:1945-1948 never trigger for such credentials, leaving the model dropdown empty and apiTest.model as an empty string. Consider treating an empty per-credential list as 'unknown' and falling back to the aggregate discovered list (initialData.models) — or fetching the read-only GET list on credential change — so the picker always offers selectable models.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@orangeboyChen
orangeboyChen merged commit dc45ba6 into main Aug 14, 2026
6 checks passed
@orangeboyChen
orangeboyChen deleted the codex/fix-api-test-model-persistence branch August 14, 2026 06:48
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.

2 participants