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>
381 lines
39 KiB
Markdown
381 lines
39 KiB
Markdown
---
|
|
phase: 07-redo-and-optimize-llm-integration
|
|
plan: 05
|
|
type: execute
|
|
wave: 5
|
|
depends_on:
|
|
- 07-04
|
|
files_modified:
|
|
- backend/api/admin.py
|
|
- backend/api/__init__.py
|
|
- backend/services/ai_config.py # Task 1 adds load_provider_config_by_id
|
|
- frontend/src/api/client.js
|
|
- frontend/src/components/admin/AdminAiConfigTab.vue
|
|
- frontend/src/components/documents/DocumentCard.vue
|
|
- backend/tests/test_admin_ai_config.py
|
|
- frontend/tests/api.spec.js # NEW Vitest tests for getAiConfig/saveAiConfig/testAiConnection (D-11 frontend coverage)
|
|
- frontend/tests/DocumentCard.spec.js # NEW Vitest test for reanalyze() (D-11 frontend coverage)
|
|
autonomous: false
|
|
requirements:
|
|
- D-05
|
|
- D-08
|
|
- D-11
|
|
- D-15
|
|
|
|
must_haves:
|
|
truths:
|
|
- "GET /api/admin/ai-config returns a list of provider configs with provider_id, base_url, model_name, context_chars, is_active, updated_at — never api_key_enc"
|
|
- "PUT /api/admin/ai-config accepts {provider_id, api_key?, base_url?, model?, context_chars?, is_active?}; when api_key is provided it is HKDF-encrypted via encrypt_api_key and stored; when omitted the existing api_key_enc is left untouched"
|
|
- "PUT /api/admin/ai-config atomically flips is_active via a single UPDATE: SET is_active = (provider_id = $target) when is_active=true is requested"
|
|
- "GET /api/admin/ai-config/test-connection?provider_id=X loads the row, builds ProviderConfig, get_provider(config).health_check(), returns {\"ok\": bool}"
|
|
- "load_provider_config_by_id(session, provider_id) is added to backend/services/ai_config.py — filters by provider_id, ignores is_active so test-connection works on inactive rows"
|
|
- "AdminAiConfigTab.vue adds a Global AI Providers section ABOVE the existing per-user assignment table; per-user table remains untouched"
|
|
- "DocumentCard.vue shows a red 'Classification failed' badge with a 'Re-analyze' button when doc.status === 'classification_failed'"
|
|
- "Re-analyze button posts to /api/documents/{id}/classify via the existing classifyDocument client function; spinner displayed while in-flight"
|
|
- "All admin AI-config endpoints require get_current_admin and write audit_log entries with metadata containing only provider_id (never api_key)"
|
|
- "frontend/tests/api.spec.js Vitest tests verify getAiConfig/saveAiConfig/testAiConnection call request() with the expected method, path, and JSON body shape (D-11 frontend coverage; CLAUDE.md mandate that every new function has at least one test)"
|
|
- "frontend/tests/DocumentCard.spec.js Vitest test verifies reanalyze() calls classifyDocument with props.doc.id and emits 'reclassified' (D-11 frontend coverage)"
|
|
artifacts:
|
|
- path: "backend/api/admin.py"
|
|
provides: "GET/PUT /api/admin/ai-config + test-connection endpoint + _ai_config_to_dict whitelist"
|
|
contains: "_ai_config_to_dict"
|
|
- path: "backend/services/ai_config.py"
|
|
provides: "load_provider_config_by_id (added in Task 1)"
|
|
contains: "load_provider_config_by_id"
|
|
- path: "frontend/src/api/client.js"
|
|
provides: "getAiConfig/saveAiConfig/testAiConnection helpers"
|
|
contains: "getAiConfig"
|
|
- path: "frontend/src/components/admin/AdminAiConfigTab.vue"
|
|
provides: "Global system provider config section + per-provider accordion + test-connection button"
|
|
contains: "System AI Providers"
|
|
- path: "frontend/src/components/documents/DocumentCard.vue"
|
|
provides: "Classification failed badge + Re-analyze button"
|
|
contains: "classification_failed"
|
|
- path: "frontend/tests/api.spec.js"
|
|
provides: "Vitest unit tests for getAiConfig/saveAiConfig/testAiConnection"
|
|
contains: "getAiConfig"
|
|
- path: "frontend/tests/DocumentCard.spec.js"
|
|
provides: "Vitest unit test for DocumentCard.reanalyze() (D-11 frontend half)"
|
|
contains: "reanalyze"
|
|
key_links:
|
|
- from: "backend/api/admin.py PUT /api/admin/ai-config"
|
|
to: "backend/services/ai_config.py::encrypt_api_key"
|
|
via: "HKDF encryption before DB write"
|
|
pattern: "encrypt_api_key"
|
|
- from: "backend/api/admin.py GET /api/admin/ai-config/test-connection"
|
|
to: "backend/services/ai_config.py::load_provider_config_by_id"
|
|
via: "fetch row by provider_id (ignoring is_active)"
|
|
pattern: "load_provider_config_by_id"
|
|
- from: "backend/api/admin.py PUT /api/admin/ai-config"
|
|
to: "PostgreSQL atomic is_active flip"
|
|
via: "single UPDATE SET is_active = (provider_id = $target)"
|
|
pattern: "is_active = \\(provider_id ="
|
|
- from: "frontend AdminAiConfigTab.vue saveSystemConfig"
|
|
to: "PUT /api/admin/ai-config"
|
|
via: "fetch wrapper request()"
|
|
pattern: "saveAiConfig"
|
|
- from: "frontend DocumentCard.vue reanalyze()"
|
|
to: "POST /api/documents/{id}/classify"
|
|
via: "existing classifyDocument client function"
|
|
pattern: "classifyDocument"
|
|
---
|
|
|
|
<objective>
|
|
Surface the new AI provider stack in the UI: an admin "AI Providers" configuration panel that drives the `system_settings` table (read/write/test) and a Document card "Classification failed" badge with a re-analyze button that triggers the Plan 04 re-queue endpoint.
|
|
|
|
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.
|
|
</objective>
|
|
|
|
<execution_context>
|
|
@$HOME/.claude/get-shit-done/workflows/execute-plan.md
|
|
@$HOME/.claude/get-shit-done/templates/summary.md
|
|
</execution_context>
|
|
|
|
<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.vue
|
|
|
|
<interfaces>
|
|
<!-- Contracts the executor needs -->
|
|
|
|
From 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) -> dict` is 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) -> str`
|
|
- `decrypt_api_key(master_key: bytes, provider_id: str, ciphertext: str) -> str`
|
|
- `load_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 | None` to this file
|
|
|
|
From backend/ai/__init__.py and backend/ai/provider_config.py (Plan 02):
|
|
- `get_provider(config: ProviderConfig) -> AIProvider`
|
|
- `PROVIDER_DEFAULTS` dict — 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 pattern
|
|
- `classifyDocument(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 `users` ref + `configs` reactive + `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):
|
|
- `doc` prop 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 add `test: { include: ['tests/**/*.spec.js'] }` to the existing vite config.
|
|
</interfaces>
|
|
</context>
|
|
|
|
<tasks>
|
|
|
|
<task type="auto" tdd="true">
|
|
<name>Task 1: Admin AI-config backend (GET, PUT, test-connection) + audit logging + load_provider_config_by_id</name>
|
|
<read_first>
|
|
backend/api/admin.py
|
|
backend/services/ai_config.py
|
|
backend/ai/__init__.py
|
|
backend/ai/provider_config.py
|
|
backend/deps/utils.py
|
|
backend/db/models.py
|
|
backend/tests/test_admin_ai_config.py
|
|
</read_first>
|
|
<behavior>
|
|
- `GET /api/admin/ai-config` returns 200 with body `{"providers": [...]}` where each entry is the whitelisted dict (provider_id, base_url, model_name, context_chars, is_active, has_api_key (bool), updated_at)
|
|
- `api_key_enc` and `api_key` are never present in any response body
|
|
- `PUT /api/admin/ai-config` accepts body `{provider_id: str, api_key?: str, base_url?: str|null, model?: str, context_chars?: int, is_active?: bool}` validated by a Pydantic model with extra="forbid"
|
|
- When body.api_key is a non-empty string → encrypt with encrypt_api_key(master_key=cloud_creds_key, provider_id, api_key) and store in api_key_enc
|
|
- When body.api_key is omitted (None) → leave existing api_key_enc untouched (do not overwrite to NULL)
|
|
- When body.api_key == "" → set api_key_enc to NULL (explicit clear)
|
|
- When body.is_active is True → single UPDATE: `UPDATE system_settings SET is_active = (provider_id = :target_id)` — atomically flips all rows
|
|
- If no row exists for provider_id, PUT inserts one (upsert behaviour) with the supplied fields, defaults from PROVIDER_DEFAULTS for omitted fields
|
|
- PUT writes an audit log entry: event_type="admin.ai_config_changed", actor_id=admin.id, user_id=None, resource_id=None, metadata_={"provider_id": body.provider_id, "fields_changed": [list of fields the body explicitly set, NEVER api_key value]}
|
|
- `GET /api/admin/ai-config/test-connection?provider_id=X` returns `{"ok": bool, "provider_id": str}`; loads the row, calls load_provider_config_by_id, builds via get_provider, awaits health_check(); returns ok=False if any exception or no row found (does not return 5xx for provider failures — surface as ok=False so the UI shows a clear status)
|
|
- All three endpoints depend on get_current_admin (admin-only)
|
|
- backend/services/ai_config.py gains `load_provider_config_by_id(session, provider_id)` — same shape as load_provider_config but filters by provider_id and does NOT require is_active=True
|
|
- Promote test_admin_ai_config.py::test_get_never_returns_key (asserts api_key_enc not in JSON body) and test_put_writes_active_provider (asserts atomic is_active flip)
|
|
</behavior>
|
|
<action>
|
|
Open backend/api/admin.py. Add imports: `from services.ai_config import encrypt_api_key, decrypt_api_key, load_provider_config, load_provider_config_by_id`, `from ai import get_provider`, `from ai.provider_config import ProviderConfig, PROVIDER_DEFAULTS`, `from db.models import SystemSettings`, `from sqlalchemy import select, update`. (Verify exact import paths against the existing admin.py style.)
|
|
|
|
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.
|
|
</action>
|
|
<verify>
|
|
<automated>cd backend && pytest tests/test_admin_ai_config.py -x -v</automated>
|
|
</verify>
|
|
<acceptance_criteria>
|
|
- Source assertion: backend/api/admin.py contains `_ai_config_to_dict`, `class AiConfigUpdate(BaseModel)`, `@router.get("/ai-config")`, `@router.put("/ai-config")`, `@router.get("/ai-config/test-connection")`
|
|
- Source assertion: backend/api/admin.py PUT body uses `update(SystemSettings).values(is_active=(SystemSettings.provider_id == body.provider_id))` (atomic flip)
|
|
- Source assertion: `_ai_config_to_dict` body contains no reference to `api_key_enc` (no leak)
|
|
- Source assertion: backend/services/ai_config.py contains `async def load_provider_config_by_id`
|
|
- Behavior: pytest backend/tests/test_admin_ai_config.py exits 0 (all promoted tests pass)
|
|
- Behavior: GET /api/admin/ai-config response body never contains the string "api_key_enc" or "api_key" value (assertion in test_get_never_returns_key)
|
|
- Behavior: After two PUTs with is_active=True on different provider_ids, SELECT COUNT(*) WHERE is_active = TRUE returns 1 (asserted in test_put_writes_active_provider)
|
|
- Behavior: PUT by non-admin user returns 403 (admin-only enforcement)
|
|
</acceptance_criteria>
|
|
<done>Three admin AI-config endpoints land; atomic is_active flip enforced; api_key_enc never serialised; audit log records changes without leaking secrets; load_provider_config_by_id added to backend/services/ai_config.py; three Wave-5 admin tests promoted.</done>
|
|
</task>
|
|
|
|
<task type="auto" tdd="true">
|
|
<name>Task 2: Frontend API client helpers + AdminAiConfigTab.vue global System AI Providers section + Vitest coverage</name>
|
|
<read_first>
|
|
frontend/src/api/client.js
|
|
frontend/src/components/admin/AdminAiConfigTab.vue
|
|
frontend/src/components/admin/AdminUsersTab.vue
|
|
frontend/src/components/admin/AdminQuotasTab.vue
|
|
frontend/vite.config.js
|
|
frontend/package.json
|
|
</read_first>
|
|
<behavior>
|
|
- frontend/src/api/client.js exports getAiConfig(), saveAiConfig(body), testAiConnection(providerId)
|
|
- getAiConfig calls request("/api/admin/ai-config", { method: "GET" })
|
|
- saveAiConfig(body) calls request("/api/admin/ai-config", { method: "PUT", headers: {"Content-Type": "application/json"}, body: JSON.stringify(body) })
|
|
- testAiConnection(providerId) calls request(`/api/admin/ai-config/test-connection?provider_id=${encodeURIComponent(providerId)}`, { method: "GET" })
|
|
- AdminAiConfigTab.vue gains a new top-of-template section titled "System AI Providers" placed ABOVE the existing per-user table; existing template, script, and saveConfig logic for per-user assignments is unchanged
|
|
- New section: active-provider selector (radio or styled buttons over the 10 PROVIDER_DEFAULTS keys), per-provider accordion (open one at a time) with masked API key field (input type="password", placeholder shows "(unchanged)" when has_api_key is true, otherwise "(not set)"), base URL text input, model name text input, context_chars number input, Save button per provider, Test Connection button per provider
|
|
- On mount: call getAiConfig() and populate a reactive systemProviders array; if API call fails, show a non-blocking error banner above the new section only
|
|
- Saving a provider: build body containing only the fields the admin actually modified (api_key only if user typed something), call saveAiConfig, refresh systemProviders from response
|
|
- Test Connection: call testAiConnection(providerId), show inline "OK" badge (green) or "Failed" badge (red) for 3 seconds
|
|
- Per-user assignment block (existing) remains untouched (Pitfall 6 mitigation)
|
|
- frontend/tests/api.spec.js Vitest test asserts getAiConfig/saveAiConfig/testAiConnection construct the correct fetch/request payloads (path, method, body, JSON serialization, encodeURIComponent for provider_id)
|
|
- `npm run test -- --run` exits 0 with the new api.spec.js tests passing
|
|
</behavior>
|
|
<action>
|
|
Edit frontend/src/api/client.js: append at the bottom (or near other admin endpoints around line 277): `export function getAiConfig() { return request('/api/admin/ai-config', { method: 'GET' }) }`; `export function saveAiConfig(body) { return request('/api/admin/ai-config', { method: 'PUT', headers: {'Content-Type': 'application/json'}, body: JSON.stringify(body) }) }`; `export function testAiConnection(providerId) { return request('/api/admin/ai-config/test-connection?provider_id=' + encodeURIComponent(providerId), { method: 'GET' }) }`.
|
|
|
|
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).
|
|
</action>
|
|
<verify>
|
|
<automated>cd frontend && npm run build 2>&1 | tail -20 && npm run test -- --run 2>&1 | tail -30</automated>
|
|
</verify>
|
|
<acceptance_criteria>
|
|
- Source assertion: frontend/src/api/client.js contains `getAiConfig`, `saveAiConfig`, `testAiConnection`
|
|
- Source assertion: frontend/src/components/admin/AdminAiConfigTab.vue contains the string "System AI Providers" and a `<section>` block placed before the existing per-user table block
|
|
- Source assertion: AdminAiConfigTab.vue contains `getAiConfig`, `saveAiConfig`, `testAiConnection` imports/calls
|
|
- Source assertion: AdminAiConfigTab.vue does NOT mutate the existing per-user `users` reactive ref or `saveConfig` function (grep confirms only additions, not deletions)
|
|
- Source assertion: api_key input is `type="password"` and there is no `v-model` binding that pre-fills it from server-returned data
|
|
- Source assertion: frontend/tests/api.spec.js exists and contains test names referencing `getAiConfig`, `saveAiConfig`, `testAiConnection`
|
|
- Behavior: npm run build exits 0 with no errors
|
|
- Behavior: `cd frontend && npm run test -- --run` exits 0; api.spec.js tests pass (D-11 frontend client coverage)
|
|
</acceptance_criteria>
|
|
<done>API client gains three helpers; AdminAiConfigTab.vue has a working global System AI Providers section above the untouched per-user table; build is green; Vitest tests for the three helpers pass (D-11 frontend half coverage per CLAUDE.md test mandate).</done>
|
|
</task>
|
|
|
|
<task type="auto" tdd="true">
|
|
<name>Task 3: DocumentCard.vue — Classification failed badge + Re-analyze button + Vitest coverage</name>
|
|
<read_first>
|
|
frontend/src/components/documents/DocumentCard.vue
|
|
frontend/src/api/client.js
|
|
frontend/package.json
|
|
</read_first>
|
|
<behavior>
|
|
- DocumentCard.vue renders a red pill badge with text "Classification failed" when props.doc.status === 'classification_failed'
|
|
- Beside the badge, a "Re-analyze" text button calls classifyDocument(doc.id) and emits an event "reclassified" (so parent views can refresh document list status)
|
|
- Button shows spinner text "Re-analyzing…" while in flight; on success shows "Re-analyzing…" then resets after parent emits or after 500ms timeout
|
|
- Button stops bubbling clicks (@click.stop) so it does not open the document
|
|
- Existing shared badge block (lines 31-34) is preserved unchanged; the new badge block is added immediately after it
|
|
- No new visual regression on docs whose status is "processing" or "ready"
|
|
- frontend/tests/DocumentCard.spec.js Vitest test mounts DocumentCard with a doc whose status is 'classification_failed', mocks classifyDocument, asserts the badge renders, clicking Re-analyze calls classifyDocument with props.doc.id, and 'reclassified' event is emitted
|
|
- `npm run test -- --run` exits 0 with the new DocumentCard.spec.js test passing
|
|
</behavior>
|
|
<action>
|
|
Edit frontend/src/components/documents/DocumentCard.vue. In the <template>, immediately after the existing `<div v-if="doc.is_shared" class="mt-2">` block (lines 31-34 per 07-PATTERNS.md), add a sibling block: `<div v-if="doc.status === 'classification_failed'" class="mt-2 flex items-center gap-2"><span class="bg-red-50 text-red-600 text-xs font-medium px-2 py-1 rounded-full">Classification failed</span><button @click.stop="reanalyze" :disabled="reanalyzing" class="text-xs text-indigo-600 hover:text-indigo-700 font-semibold disabled:opacity-50"><span v-if="reanalyzing">Re-analyzing…</span><span v-else>Re-analyze</span></button></div>`.
|
|
|
|
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.
|
|
</action>
|
|
<verify>
|
|
<automated>cd frontend && npm run build 2>&1 | tail -10 && npm run test -- --run 2>&1 | tail -30</automated>
|
|
</verify>
|
|
<acceptance_criteria>
|
|
- Source assertion: frontend/src/components/documents/DocumentCard.vue contains the string `classification_failed` and the string `Re-analyze`
|
|
- Source assertion: DocumentCard.vue contains `classifyDocument` import and a `reanalyze` function
|
|
- Source assertion: button uses `@click.stop="reanalyze"` (does not bubble to card click)
|
|
- Source assertion: existing `v-if="doc.is_shared"` block on lines around 31-34 is still present (no accidental deletion)
|
|
- Source assertion: frontend/tests/DocumentCard.spec.js exists and references `reanalyze` and `classifyDocument` in its test bodies (grep)
|
|
- Behavior: npm run build exits 0
|
|
- Behavior: `cd frontend && npm run test -- --run` exits 0; DocumentCard.spec.js tests pass (D-11 frontend half coverage)
|
|
</acceptance_criteria>
|
|
<done>DocumentCard renders the failed badge and re-analyze button; click triggers POST /classify via the existing client; build is green; Vitest test exists and passes (CLAUDE.md mandate for new components).</done>
|
|
</task>
|
|
|
|
<task type="checkpoint:human-verify" gate="blocking">
|
|
<name>Task 4: Human verification — Admin AI Providers panel + Re-analyze button end-to-end</name>
|
|
<read_first>
|
|
.planning/phases/07-redo-and-optimize-llm-integration/07-VALIDATION.md
|
|
</read_first>
|
|
<what-built>
|
|
- Backend GET/PUT /api/admin/ai-config + /api/admin/ai-config/test-connection (Plan 05 Task 1)
|
|
- backend/services/ai_config.py::load_provider_config_by_id (Plan 05 Task 1)
|
|
- Frontend AdminAiConfigTab.vue global System AI Providers section (Plan 05 Task 2)
|
|
- DocumentCard.vue classification-failed badge + Re-analyze button (Plan 05 Task 3)
|
|
- Vitest unit tests for the three new client helpers and DocumentCard.reanalyze (Plan 05 Tasks 2 & 3)
|
|
- Underlying Celery retry harness + re-queue endpoint from Plan 04
|
|
</what-built>
|
|
<how-to-verify>
|
|
1. Start the full stack: docker compose up. Wait for backend and celery-worker healthchecks to pass.
|
|
2. Log in as an admin user. Navigate to Admin → AI Config tab.
|
|
3. Confirm: the new "System AI Providers (Global)" section appears ABOVE the existing per-user assignment table. The per-user table still functions (try a quick edit and save to confirm no regression).
|
|
4. In the System section, expand the "OpenAI" accordion. Confirm the API key field is empty (write-only) and shows placeholder "(not set)" because no key is configured yet.
|
|
5. Enter a fake key "sk-test-1234567890". Click Save. Confirm the row refreshes and the indicator now says "(unchanged)" — the key was stored but is not returned.
|
|
6. Click "Set Active" on the OpenAI row. Confirm: OpenAI row shows "Active" badge; if any other row previously showed Active it must lose the badge (atomic flip).
|
|
7. Click "Test Connection" on OpenAI. Confirm a Failed badge appears (the fake key will not authenticate against OpenAI). Confirm no 5xx error appears in the browser console — the response was structured 200 with ok=false.
|
|
8. Open browser DevTools → Network → click on GET /api/admin/ai-config request. Confirm the response body does NOT contain the strings "api_key_enc" or "sk-test" anywhere. Confirm the providers list contains has_api_key=true for openai.
|
|
9. Open any existing document in the library that has status="classification_failed". (If none exists, manually set one via `psql`: UPDATE documents SET status='classification_failed' WHERE id=...). Reload the document list.
|
|
10. Confirm the DocumentCard shows the red "Classification failed" pill badge AND the "Re-analyze" button beside it.
|
|
11. Click "Re-analyze". Confirm: button text changes to "Re-analyzing…"; status badge transitions away (next refresh or polling cycle); celery worker logs show extract_and_classify enqueued for that doc id; doc.status becomes "processing" then resolves.
|
|
12. Confirm: cross-user reclassify still 404s — try opening the network request to POST /api/documents/{some_other_users_doc_id}/classify in DevTools and confirm the server returns 404.
|
|
</how-to-verify>
|
|
<resume-signal>Type "approved" or describe issues.</resume-signal>
|
|
</task>
|
|
|
|
</tasks>
|
|
|
|
<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>
|
|
|
|
<verification>
|
|
- pytest backend/tests/test_admin_ai_config.py exits 0; test_get_never_returns_key asserts "api_key_enc" not in response body.
|
|
- grep -F 'api_key_enc' backend/api/admin.py | grep -v '_ai_config_to_dict' returns no occurrences inside the response-construction code paths.
|
|
- grep -F 'async def load_provider_config_by_id' backend/services/ai_config.py returns the function definition (Task 1 modification).
|
|
- npm run build (frontend) exits 0.
|
|
- `cd frontend && npm run test -- --run` exits 0 — both frontend/tests/api.spec.js and frontend/tests/DocumentCard.spec.js pass (D-11 frontend half coverage).
|
|
- Human checkpoint #4 confirms end-to-end flow and absence of api_key in admin GET response.
|
|
- grep -F 'classification_failed' frontend/src/components/documents/DocumentCard.vue confirms the new badge block.
|
|
- Audit log entry for PUT /api/admin/ai-config contains only provider_id + fields_changed (no api_key value).
|
|
</verification>
|
|
|
|
<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>
|
|
|
|
<output>
|
|
Create `.planning/phases/07-redo-and-optimize-llm-integration/07-05-SUMMARY.md` when done.
|
|
</output>
|