Skip to content

fix: prevent API Test model persistence #74

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
orangeboyChen merged 1 commit into from
Aug 14, 2026
Merged

fix: prevent API Test model persistence #74

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 40 additions & 53 deletions app/page-shell.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -507,58 +507,41 @@ const AdminPageLayoutContent = ({
}));
}, [setCredentials]);

const refreshCredentialModels = useCallback(
async (filename?: string) => {
setCredentials((current) => ({ ...current, modelsLoading: true }));
const result = await requestJson<CredentialModelsResponse>(
'/admin-api/credentials/models',
filename
? {
body: JSON.stringify({ filename }),
headers: { 'Content-Type': 'application/json' },
method: 'POST',
}
: undefined,
);
const modelRows = Object.fromEntries(
Object.entries(result.data?.models ?? {}).map(([key, value]) => [
key,
{
error: value.error ?? null,
models: (value.models ?? [])
.map((model) => model.id)
.filter((model): model is string => Boolean(model)),
},
]),
);

setCredentials((current) => ({
...current,
modelRows: filename
? { ...current.modelRows, ...modelRows }
: modelRows,
modelsLoading: false,
}));
setApiTest((current) => {
if (filename && current.credentialFilename !== filename) {
return current;
}
const loadCredentialModels = useCallback(async () => {
setCredentials((current) => ({ ...current, modelsLoading: true }));
const result = await requestJson<CredentialModelsResponse>(
'/admin-api/credentials/models',
);
const modelRows = Object.fromEntries(
Object.entries(result.data?.models ?? {}).map(([key, value]) => [
key,
{
error: value.error ?? null,
models: (value.models ?? [])
.map((model) => model.id)
.filter((model): model is string => Boolean(model)),
},
]),
);

const selectedFilename = filename ?? current.credentialFilename;
const models = modelRows[selectedFilename]?.models;
setCredentials((current) => ({
...current,
modelRows,
modelsLoading: false,
}));
setApiTest((current) => {
const models = modelRows[current.credentialFilename]?.models;

if (!models) return current;
if (!models) return current;

return {
...current,
model: models.includes(current.model)
? current.model
: (models[0] ?? ''),
};
});
},
[setApiTest, setCredentials],
);
return {
...current,
model: models.includes(current.model)
? current.model
: (models[0] ?? ''),
};
});
}, [setApiTest, setCredentials]);

const refreshCredentialList = useCallback(async () => {
setCredentials((current) => ({
Expand Down Expand Up @@ -829,8 +812,8 @@ const AdminPageLayoutContent = ({

const refreshAdminData = useCallback(async () => {
await Promise.all([loadDashboard(), loadCredentials()]);
await refreshCredentialModels();
}, [loadCredentials, loadDashboard, refreshCredentialModels]);
await loadCredentialModels();
}, [loadCredentialModels, loadCredentials, loadDashboard]);

const clearUsageHistory = async () => {
setUsage((current) => ({
Expand Down Expand Up @@ -1538,7 +1521,11 @@ const AdminPageLayoutContent = ({
useEffect(() => {
if (!initialData) {
if (activeTab === 'api-test') {
void Promise.all([loadCredentials(), loadSettings()]);
void Promise.all([
loadCredentialModels(),
loadCredentials(),
loadSettings(),
]);
} else if (activeTab === 'credentials') {
void loadCredentials();
} else if (activeTab === 'dashboard') {
Expand All @@ -1562,6 +1549,7 @@ const AdminPageLayoutContent = ({
clearDebugAutoRefreshTimer,
initialData,
loadCredentials,
loadCredentialModels,
loadDashboard,
loadDebug,
loadSettings,
Expand Down Expand Up @@ -1963,7 +1951,6 @@ const AdminPageLayoutContent = ({
credentialFilename: value,
model: models[0] ?? '',
}));
Comment on lines 1951 to 1953

@devin-ai-integration devin-ai-integration Bot Aug 14, 2026

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.

void refreshCredentialModels(value);
},
onMessageChange: (value) => {
setApiTest((current) => ({ ...current, message: value }));
Expand Down
Loading