Fix frontend settings API configuration persistence - #34
Conversation
Co-authored-by: up2itnow0822 <up2itnow0822@gmail.com>
Co-authored-by: up2itnow0822 <up2itnow0822@gmail.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| export const setSessionAuthToken = (token: string) => { | ||
| const trimmedToken = token.trim(); | ||
| if (trimmedToken) { | ||
| sessionAuthToken = trimmedToken; | ||
| } | ||
| }; |
There was a problem hiding this comment.
setSessionAuthToken silently no-ops when called with an empty string, making it impossible to clear an active in-memory token through the settings UI. Both settings/page.tsx and SettingsModal.tsx call setSessionAuthToken(apiKeyInputRef.current?.value || '') on save; if a token is already set and the user saves with a blank API Key field (e.g., to update only the backend URL), the old token persists with no indication. The "Settings saved" confirmation fires regardless, leaving users no way to revoke their session token short of a full page reload.
| export const setSessionAuthToken = (token: string) => { | |
| const trimmedToken = token.trim(); | |
| if (trimmedToken) { | |
| sessionAuthToken = trimmedToken; | |
| } | |
| }; | |
| export const setSessionAuthToken = (token: string) => { | |
| sessionAuthToken = token.trim(); | |
| }; |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| for (const legacyKey of LEGACY_BACKEND_URL_STORAGE_KEYS) { | ||
| const legacyUrl = storage.getItem(legacyKey)?.trim(); | ||
| if (legacyUrl) { | ||
| storage.setItem(BACKEND_URL_STORAGE_KEY, legacyUrl); | ||
| storage.removeItem(legacyKey); | ||
| return legacyUrl; | ||
| } | ||
| } |
There was a problem hiding this comment.
backend_base_url and api_url) are present simultaneously — a realistic state since each was written by a different component before this PR — the migration loop migrates the first match but only removes that one key. The canonical key is written and subsequent calls return early, so api_url stays in localStorage indefinitely. It never affects the returned URL, but the old key is never cleaned up unless the user explicitly saves settings again via persistBackendBaseUrl.
| for (const legacyKey of LEGACY_BACKEND_URL_STORAGE_KEYS) { | |
| const legacyUrl = storage.getItem(legacyKey)?.trim(); | |
| if (legacyUrl) { | |
| storage.setItem(BACKEND_URL_STORAGE_KEY, legacyUrl); | |
| storage.removeItem(legacyKey); | |
| return legacyUrl; | |
| } | |
| } | |
| for (const legacyKey of LEGACY_BACKEND_URL_STORAGE_KEYS) { | |
| const legacyUrl = storage.getItem(legacyKey)?.trim(); | |
| if (legacyUrl) { | |
| storage.setItem(BACKEND_URL_STORAGE_KEY, legacyUrl); | |
| LEGACY_BACKEND_URL_STORAGE_KEYS.forEach((k) => storage.removeItem(k)); | |
| return legacyUrl; | |
| } | |
| } |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
2 participants
Summary
Bug and impact
Recent security-hardening commits changed settings storage keys and removed localStorage auth-token persistence, but
frontend/src/lib/api.tsstill read the oldapi_url/auth_tokenkeys. Configured users could save a backend URL/token in the UI, then authenticated API flows would hit the wrong backend and omit Authorization.Validation
pnpm --filter @agentnexus/frontend test -- --runInBandpnpm --filter @agentnexus/frontend type-checkpnpm --filter @agentnexus/frontend lint(0 errors, 3 existing warnings)pnpm --filter @agentnexus/frontend buildpython3 -m ruff check backend/src/agents/trading_bot agents/summarizer agents/paper-trader agent-runtime/docker/python-agentpython3 -m bandit -r backend/src/agents/trading_bot agents/summarizer agents/paper-trader agent-runtime/docker/python-agent -x agents/paper-trader/test_price_feed.pypython3 -m pytest agents/paper-trader/test_price_feed.pyagentnexus_backend_url, leaves API key field empty, andauth_tokenremains absent from localStorage.Greptile Summary
This PR fixes a key-mismatch bug where
frontend/src/lib/api.tswas readingapi_url/auth_tokenfrom localStorage while the Settings UI was writing to a different key (agentnexus_backend_url), causing authenticated API calls to target the wrong backend. It introduces a sharedapi-config.tsmodule with legacy key migration and keeps auth tokens in memory only.api-config.tswith a canonical storage key (agentnexus_backend_url), a one-time migration loop for the two legacy keys, and an in-memory-only session auth token — both settings surfaces (settings/page.tsxandSettingsModal.tsx) now read/write through this shared module.urlopen,usedforsecurity=Falsefor non-security md5, removes unused imports).Confidence Score: 4/5
Safe to merge once the open thread about token-clearing behaviour is resolved; the core storage-key fix is correct and well-tested.
The storage-key unification and request-interceptor wiring are correct. However, the
setSessionAuthTokenfunction inapi-config.tssilently ignores an empty string, so saving settings with a blank API Key field leaves the in-memory token unchanged even though the UI confirms the save — a real gap when a user needs to revoke a session token short of a full page reload. That unresolved behavioural issue (called out in a prior review thread) keeps this from a clean bill of health.frontend/src/lib/api-config.ts — specifically the
setSessionAuthTokenguard that prevents callers from clearing the in-memory token via an empty string.Important Files Changed
Sequence Diagram
sequenceDiagram participant UI as Settings UI (page / modal) participant AC as api-config.ts participant LS as localStorage participant API as api.ts (Axios) participant BE as Backend UI->>AC: persistBackendBaseUrl(url) AC->>LS: setItem('agentnexus_backend_url', url) AC->>LS: removeItem('backend_base_url') AC->>LS: removeItem('api_url') UI->>AC: setSessionAuthToken(token) AC-->>AC: "sessionAuthToken = token (memory only)" UI-->>UI: Settings saved Note over API,BE: On next API request API->>AC: getBackendBaseUrl() AC->>LS: getItem('agentnexus_backend_url') LS-->>AC: url AC-->>API: url API->>AC: getSessionAuthToken() AC-->>API: sessionAuthToken (memory) API->>BE: GET /api/... Authorization: Bearer token BE-->>API: 200 OKReviews (2): Last reviewed commit: "chore: refresh frontend lockfile for con..." | Re-trigger Greptile