5 waves: system_settings DB + HKDF encryption (01), ProviderConfig + GenericOpenAIProvider + singleton client fix (02), Anthropic output_config + classifier wiring (03), Celery retry 30/90/270s + re-queue endpoint (04), admin AI panel + DocumentCard badge + human checkpoint (05). All 18 decisions D-01..D-18 covered. Plan checker passed after 1 revision round (4 blockers fixed: D-02/D-14 coverage, D-11 Vitest tests, Plan 05 files_modified). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
39 KiB
phase, plan, type, wave, depends_on, files_modified, autonomous, requirements, must_haves
| phase | plan | type | wave | depends_on | files_modified | autonomous | requirements | must_haves | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| 07-redo-and-optimize-llm-integration | 05 | execute | 5 |
|
|
false |
|
|
Purpose: Close D-08 (admin AI Providers panel with test-connection), D-11 frontend half (failed badge + re-analyze button), and D-15 (LMStudio base URL configurable through the panel). Enforce the admin-endpoint invariant that api_key_enc is never serialised in any response (Threat T-07-01 mitigation). Add Vitest coverage for the three new frontend API helpers and the reanalyze() handler so the D-11 frontend half satisfies CLAUDE.md's "every new function / component must have at least one test" rule.
Output: Backend admin endpoints (GET / PUT / test-connection), load_provider_config_by_id added to backend/services/ai_config.py, frontend API client helpers, AdminAiConfigTab.vue extended with a global System AI Providers section above the existing per-user table, DocumentCard.vue badge + button, Vitest test files for the new client helpers and DocumentCard.reanalyze, and the matching admin endpoint security tests promoted from xfail. Plan ends on a human checkpoint that exercises the panel and badge end-to-end.
<execution_context> @$HOME/.claude/get-shit-done/workflows/execute-plan.md @$HOME/.claude/get-shit-done/templates/summary.md </execution_context>
@.planning/PROJECT.md @.planning/phases/07-redo-and-optimize-llm-integration/07-CONTEXT.md @.planning/phases/07-redo-and-optimize-llm-integration/07-RESEARCH.md @.planning/phases/07-redo-and-optimize-llm-integration/07-PATTERNS.md @.planning/phases/07-redo-and-optimize-llm-integration/07-04-SUMMARY.md @backend/api/admin.py @backend/services/ai_config.py @backend/ai/__init__.py @backend/ai/provider_config.py @backend/services/storage.py @backend/deps/utils.py @frontend/src/api/client.js @frontend/src/components/admin/AdminAiConfigTab.vue @frontend/src/components/documents/DocumentCard.vueFrom backend/api/admin.py (existing patterns):
- Router prefix is "/api/admin" (or similar — verify); endpoints take Depends(get_db), Depends(get_current_admin)
_user_to_dict(user) -> dictis the whitelist helper pattern (see 07-PATTERNS.md "Admin Endpoint Whitelist Helper")await write_audit_log(session, event_type=..., user_id=..., actor_id=..., resource_id=..., ip_address=get_client_ip(request), metadata_={...})is used for admin actions- get_client_ip lives in backend/deps/utils.py — import from there
From backend/services/ai_config.py (introduced in Plan 01 + Plan 03):
encrypt_api_key(master_key: bytes, provider_id: str, api_key: str) -> strdecrypt_api_key(master_key: bytes, provider_id: str, ciphertext: str) -> strload_provider_config(session) -> ProviderConfig | None(per-row, active=True)- For PUT we ALSO need to load by provider_id explicitly (not just is_active) — Task 1 adds
async def load_provider_config_by_id(session, provider_id) -> ProviderConfig | Noneto this file
From backend/ai/__init__.py and backend/ai/provider_config.py (Plan 02):
get_provider(config: ProviderConfig) -> AIProviderPROVIDER_DEFAULTSdict — the 10 known provider_ids; PUT validates incoming provider_id against this list
From frontend/src/api/client.js (existing pattern, line ~277):
request(path, options)wrapper;adminUpdateAiConfig(id, provider, model)is the existing per-user PATCH patternclassifyDocument(docId)is the existing client function that POSTs /api/documents/{id}/classify (used by DocumentCard.reanalyze)
From frontend/src/components/admin/AdminAiConfigTab.vue (existing — per Pitfall 6):
- Existing per-user table powered by
usersref +configsreactive +saveConfig(userId)— KEEP UNTOUCHED - This plan adds a SECOND top section before/above the per-user table
From frontend/src/components/documents/DocumentCard.vue (existing):
docprop has status, is_shared fields; existing pattern at lines 31-34 shows the Shared badge
Frontend Vitest framework (existing project setup):
- Vitest is the unit-test runner (matching the project's vue-test-utils / Vitest convention used elsewhere)
- Run command:
cd frontend && npm run test -- --run(one-shot, no watch) - If frontend/tests/ does not yet exist, create it. Configure vite.config.js / vitest.config.js to pick up
tests/**/*.spec.js— if a config already exists, follow whatever globs are defined; otherwise addtest: { include: ['tests/**/*.spec.js'] }to the existing vite config.
Add new Pydantic request model alongside existing ones:
`class AiConfigUpdate(BaseModel): model_config = ConfigDict(extra="forbid"); provider_id: str; api_key: Optional[str] = None; base_url: Optional[str] = None; model_name: Optional[str] = None; context_chars: Optional[int] = None; is_active: Optional[bool] = None` — and a `@field_validator("provider_id")` that asserts the value is in PROVIDER_DEFAULTS.keys() (rejects unknown providers with 422).
Add `_ai_config_to_dict(row: SystemSettings) -> dict` whitelist helper (matches `_user_to_dict` pattern from lines 52-68): returns `{"provider_id": row.provider_id, "base_url": row.base_url, "model_name": row.model_name, "context_chars": row.context_chars, "is_active": row.is_active, "has_api_key": row.api_key_enc is not None, "updated_at": row.updated_at.isoformat() if row.updated_at else None}` — explicitly excludes api_key_enc.
Add GET endpoint: `@router.get("/ai-config") async def get_ai_config(session: AsyncSession = Depends(get_db), _admin: User = Depends(get_current_admin)) -> dict:` — selects all SystemSettings rows, returns `{"providers": [_ai_config_to_dict(r) for r in rows]}`. Also include rows from PROVIDER_DEFAULTS that have no DB record (synthesized as `{"provider_id": key, "base_url": defaults["base_url"], "model_name": defaults["model"], "context_chars": defaults["context_chars"], "is_active": False, "has_api_key": False, "updated_at": None}`) so the admin UI can show all 10 providers even before any have been saved.
Add PUT endpoint: `@router.put("/ai-config") async def update_ai_config(body: AiConfigUpdate, request: Request, session: AsyncSession = Depends(get_db), _admin: User = Depends(get_current_admin)) -> dict:` — load existing row via select(SystemSettings).where(provider_id == body.provider_id); if absent create new SystemSettings instance with defaults from PROVIDER_DEFAULTS[body.provider_id]; apply provided fields. For api_key: if body.api_key is None leave api_key_enc unchanged; if body.api_key == "" set api_key_enc = None; if body.api_key is a non-empty string set api_key_enc = encrypt_api_key(settings.cloud_creds_key.encode() if isinstance(settings.cloud_creds_key, str) else settings.cloud_creds_key, body.provider_id, body.api_key). session.add(row) if newly created. If body.is_active is True: execute `update(SystemSettings).values(is_active=(SystemSettings.provider_id == body.provider_id))` — atomic flip (no read-then-write). Then await write_audit_log with metadata_={"provider_id": body.provider_id, "fields_changed": [k for k in ("api_key","base_url","model_name","context_chars","is_active") if getattr(body, k, None) is not None]}. Commit. Return `_ai_config_to_dict(row)`.
Add test-connection endpoint: `@router.get("/ai-config/test-connection") async def test_ai_connection(provider_id: str, session: AsyncSession = Depends(get_db), _admin: User = Depends(get_current_admin)) -> dict:` — load config via load_provider_config_by_id; if None return {"ok": False, "provider_id": provider_id, "reason": "not_configured"}; build provider via get_provider(config); try: await provider.health_check(); return {"ok": True, "provider_id": provider_id}. except Exception as exc: return {"ok": False, "provider_id": provider_id, "reason": "health_check_failed"}.
Add `async def load_provider_config_by_id(session, provider_id: str) -> ProviderConfig | None` to backend/services/ai_config.py — mirrors load_provider_config but filters by provider_id and ignores is_active (so test-connection works on inactive rows too). (THIS FILE IS LISTED IN files_modified — Task 1 modifies it.)
Promote backend/tests/test_admin_ai_config.py: test_get_never_returns_key — call GET as admin, assert response status 200, assert "api_key_enc" not in response.text and "api_key" not in response.json()["providers"][0]. test_put_writes_active_provider — PUT with provider_id="openai", api_key="sk-test", is_active=True; assert response 200 and has_api_key=True; PUT another provider with is_active=True; query DB and assert exactly one row has is_active=True. Add a third test (recommended) test_put_admin_only — call PUT as a regular (non-admin) user, assert 403. Remove xfail decorators.
Edit frontend/src/components/admin/AdminAiConfigTab.vue. Do NOT alter the existing per-user table block (Pitfall 6). Add a new <section> at the top of the <template> placed above the existing block. The new section uses Tailwind classes matching the existing rounded-xl bordered card pattern. Structure: card header "System AI Providers (Global)"; explanatory paragraph "These settings apply to all classification jobs. Per-user overrides below take precedence when set."; loading spinner (reuse pattern from lines 2-9); error banner; main body — a list of accordion rows, one per provider_id from systemProviders. Each accordion row shows: provider_id (e.g., "OpenAI"), is_active badge ("Active" green when true), expand chevron. Expanded body has: API key input (type=password, placeholder switches between "(unchanged)" and "(not set)"), Base URL input (placeholder is the PROVIDER_DEFAULTS base_url), Model input (placeholder is the default model), Context chars number input (placeholder is default context_chars), three buttons in a row: "Set Active" (calls saveAiConfig with is_active=true), "Save" (calls saveAiConfig with only the modified fields), "Test Connection" (calls testAiConnection, shows ok/failed badge for 3s).
In <script setup> or the existing Options API style (match the file's current style — verify before editing), add: `const systemProviders = ref([])`, `const loadingSystem = ref(false)`, `const systemError = ref(null)`, `const openProviderId = ref(null)`, `const formByProvider = reactive({})` (per-provider edit buffer), `const testResults = reactive({})` (per-provider ok|failed|null), `const savingProvider = ref(null)`. async function `loadSystemProviders()`: set loadingSystem true, try await getAiConfig(), set systemProviders to result.providers, initialise formByProvider entries (api_key="", base_url=row.base_url||"", model_name=row.model_name||"", context_chars=row.context_chars||0); catch e: systemError = e.message; finally loadingSystem=false. async function `saveSystemProvider(providerId, opts={})`: savingProvider=providerId; build body = {provider_id: providerId}; for each editable field check formByProvider[providerId][field] and include only if changed (string non-empty for api_key, defined for the rest); if opts.activate: body.is_active = true; try await saveAiConfig(body); await loadSystemProviders(); catch e: systemError=e.message; finally savingProvider=null. async function `runTestConnection(providerId)`: try const r = await testAiConnection(providerId); testResults[providerId] = r.ok ? 'ok' : 'failed'; setTimeout(() => testResults[providerId] = null, 3000); catch: testResults[providerId] = 'failed'; setTimeout reset. Call loadSystemProviders inside the existing onMounted hook (preserve the existing per-user load call).
Visual rules: write-only api_key field (never pre-fill from server — verified by manual checkpoint), display has_api_key as the only indicator that a key is set; active provider gets a green border; PROVIDER_DEFAULTS values used as placeholder text (hardcoded in the component because there is no /api/ai/defaults endpoint — match the keys returned by GET /api/admin/ai-config).
Create frontend/tests/api.spec.js with the following Vitest tests (use `vi.mock('../src/api/client.js', ...)` is NOT what we want — instead, mock the underlying `fetch` / `request` mechanism). Implementation pattern: import { describe, it, expect, vi, beforeEach } from 'vitest'; use vi.stubGlobal('fetch', vi.fn(...)) to capture the fetch call OR — if request() is the wrapper that internally calls fetch — mock fetch and assert on the URL/method/body. Tests required: (1) `getAiConfig() issues GET /api/admin/ai-config` — mock fetch to return Response with `{providers: []}`, call getAiConfig, assert fetch.mock.calls[0][0] ends with '/api/admin/ai-config' and the init.method === 'GET' (case-insensitive); (2) `saveAiConfig(body) issues PUT with JSON body` — call saveAiConfig({provider_id: 'openai', api_key: 'sk-x'}), assert fetch was called with method 'PUT', headers include 'Content-Type: application/json', body parses to {provider_id: 'openai', api_key: 'sk-x'}; (3) `testAiConnection(providerId) issues GET with encoded provider_id` — call testAiConnection('open ai'), assert fetch URL contains '/api/admin/ai-config/test-connection?provider_id=open%20ai'. Use whatever vitest config the project already has — if frontend/tests/ does not exist, create it. If vite.config.js or vitest.config.js lacks a test section, add `test: { environment: 'jsdom', include: ['tests/**/*.spec.js'] }` (preserve existing config).
In <script>, add `classifyDocument` to the existing imports from `'../../api/client.js'` (or whatever the existing import path is — verify; 07-PATTERNS.md shows the existing moveDocument import on lines 97-98). Add `const reanalyzing = ref(false)` to the existing refs block. Add an emit definition `defineEmits(['reclassified'])` if missing (or extend the existing one). Add: `async function reanalyze() { reanalyzing.value = true; try { await classifyDocument(props.doc.id); emit('reclassified', props.doc.id); } catch (e) { console.error('Re-analyze failed:', e.message); } finally { setTimeout(() => { reanalyzing.value = false }, 500); } }`. Match the file's existing Options API or <script setup> style — verify which by reading the current header before editing.
No structural changes to other parts of the file. Existing tests and stories continue to work.
Create frontend/tests/DocumentCard.spec.js (Vitest + @vue/test-utils). Pattern: import { describe, it, expect, vi } from 'vitest'; import { mount } from '@vue/test-utils'; vi.mock('../src/api/client.js', () => ({ classifyDocument: vi.fn(() => Promise.resolve({document_id: 'd1', status: 'processing'})) })); import DocumentCard from '../src/components/documents/DocumentCard.vue'; import { classifyDocument } from '../src/api/client.js'. Tests required: (1) `renders Classification failed badge when status === 'classification_failed'` — mount DocumentCard with props {doc: {id: 'd1', status: 'classification_failed', name: 'x.pdf', ...minimum required props}}; expect wrapper.text().toContain('Classification failed'); expect wrapper.find('button').text()).toContain('Re-analyze'); (2) `does NOT render badge when status !== 'classification_failed'` — mount with status: 'ready'; expect wrapper.text() does not contain 'Classification failed'; (3) `Re-analyze button calls classifyDocument(doc.id) and emits 'reclassified'` — mount with status 'classification_failed'; await wrapper.find('button').trigger('click'); await flush promises; expect classifyDocument to have been called once with 'd1'; expect wrapper.emitted('reclassified')[0] to equal ['d1']. Use whatever the project's mount/global-stubs convention requires — check AdminUsersTab or any existing component test if one exists; if no precedent exists, use the @vue/test-utils default `mount(Component, {props: {...}})` invocation.
<threat_model>
Trust Boundaries
| Boundary | Description |
|---|---|
| admin browser → PUT /api/admin/ai-config | API key crosses untrusted boundary; encrypted on backend before DB write; never echoed back |
| admin browser ↔ GET /api/admin/ai-config | server-side whitelist _ai_config_to_dict excludes api_key_enc; has_api_key bool is the only signal |
| audit_log metadata → admin viewer (ADMIN-06) | metadata contains only provider_id + fields_changed list; never the api_key value |
STRIDE Threat Register
| Threat ID | Category | Component | Disposition | Mitigation Plan |
|---|---|---|---|---|
| T-07-01 | Information Disclosure | GET /api/admin/ai-config response body | mitigate | _ai_config_to_dict whitelist excludes api_key_enc and any decrypted value; integration test asserts response.text never contains "api_key_enc" or any stored key |
| T-07-02 | Elevation of Privilege | HKDF domain separation | mitigate | encrypt_api_key uses info=b"ai-provider-settings" (different from cloud b"cloud-credentials"); same master key cannot produce equal Fernet for both domains; Plan 01 test enforces |
| T-07-03 | Tampering | is_active dual-write race | mitigate | Single UPDATE SET is_active = (provider_id = $target) runs in one statement — atomic; integration test asserts COUNT(WHERE is_active) == 1 after two PUTs |
| T-07-13 | Tampering | Mass assignment via PUT body | mitigate | AiConfigUpdate Pydantic model uses extra="forbid"; unknown fields raise 422 |
| T-07-14 | Information Disclosure | Audit log metadata leak | mitigate | audit metadata records only {"provider_id", "fields_changed"} — never the api_key value itself; matches STATE.md "login_failed audit metadata_=None" precedent |
| T-07-15 | Elevation of Privilege | Non-admin PUT | mitigate | All three endpoints carry Depends(get_current_admin); test_put_admin_only asserts 403 for regular users |
| T-07-SC | Tampering | No new packages | accept | RESEARCH.md Package Legitimacy Audit confirms zero new packages; pip audit + npm audit re-run at phase gate |
| </threat_model> |
<success_criteria>
- Three admin AI-config endpoints land with whitelist, atomic flip, and audit logging.
- backend/services/ai_config.py gains load_provider_config_by_id (now listed in files_modified).
- Frontend client gains three helpers (getAiConfig / saveAiConfig / testAiConnection).
- AdminAiConfigTab.vue has a working global System AI Providers section ABOVE the untouched per-user table.
- DocumentCard.vue renders classification_failed badge + Re-analyze button driving POST /classify.
- 2-3 previously-xfailed admin tests now pass; full suite green.
- frontend/tests/api.spec.js and frontend/tests/DocumentCard.spec.js exist and pass (D-11 frontend half coverage per CLAUDE.md mandate).
- Human checkpoint approves end-to-end UAT flow; no api_key leak observed in network tab. </success_criteria>