fix(03): revise plans based on checker feedback

.planning/phases/03-operator-experience/03-01-PLAN.md

@@ -12,12 +12,14 @@ files_modified:
   - packages/shared/shared/models/tenant.py
   - packages/shared/shared/api/billing.py
   - packages/shared/shared/api/channels.py
+  - packages/shared/shared/api/llm_keys.py
   - packages/shared/shared/api/usage.py
   - packages/shared/shared/crypto.py
   - packages/orchestrator/orchestrator/agents/runner.py
   - packages/orchestrator/orchestrator/audit/logger.py
   - migrations/versions/005_billing_and_usage.py
   - tests/unit/test_key_encryption.py
+  - tests/unit/test_llm_keys_crud.py
   - tests/unit/test_slack_oauth.py
   - tests/unit/test_stripe_webhooks.py
   - tests/unit/test_usage_aggregation.py
@@ -34,6 +36,7 @@ must_haves:
   truths:
     - "Audit events for LLM calls include prompt_tokens, completion_tokens, cost_usd, and provider in metadata"
     - "BYO API keys can be encrypted and decrypted without data loss using Fernet"
+    - "BYO API keys can be listed (redacted), created, and deleted via REST endpoints"
     - "Slack OAuth state can be HMAC-signed and verified for CSRF protection"
     - "Stripe webhook events can be processed idempotently"
     - "Token usage can be aggregated per agent and per provider from audit events"
@@ -45,6 +48,8 @@ must_haves:
       provides: "KeyEncryptionService with Fernet encrypt/decrypt/rotate"
     - path: "packages/shared/shared/api/channels.py"
       provides: "Slack OAuth state generation/verification, OAuth callback endpoint"
+    - path: "packages/shared/shared/api/llm_keys.py"
+      provides: "LLM key CRUD endpoints: GET (list, redacted), POST (encrypt + store), DELETE"
     - path: "packages/shared/shared/api/billing.py"
       provides: "Stripe webhook handler, checkout session creation, billing portal session"
     - path: "packages/shared/shared/api/usage.py"
@@ -64,13 +69,21 @@ must_haves:
       to: "PLATFORM_ENCRYPTION_KEY env var"
       via: "Fernet key loaded at init"
       pattern: "MultiFernet"
+    - from: "packages/shared/shared/api/llm_keys.py"
+      to: "packages/shared/shared/crypto.py"
+      via: "KeyEncryptionService.encrypt() on POST, never decrypts on GET"
+      pattern: "KeyEncryptionService"
+    - from: "packages/portal/app/(dashboard)/settings/api-keys/page.tsx"
+      to: "packages/shared/shared/api/llm_keys.py"
+      via: "GET/POST/DELETE /api/portal/tenants/{tenant_id}/llm-keys"
+      pattern: "tenants.*llm-keys"
 ---
 
 <objective>
-Backend foundation for Phase 3: database migrations, dependency installs, audit trail token metadata, encryption service, and all backend API endpoints for billing, channel connection, and usage aggregation.
+Backend foundation for Phase 3: database migrations, dependency installs, audit trail token metadata, encryption service, and all backend API endpoints for billing, channel connection, LLM key management, and usage aggregation.
 
 Purpose: Every portal UI feature in Phase 3 depends on backend APIs and database schema. This plan ships all backend infrastructure so Plans 02-04 can focus on frontend.
-Output: New DB tables/fields, billing + channel + usage API endpoints, encryption service, enhanced audit logger, and comprehensive test scaffolds.
+Output: New DB tables/fields, billing + channel + LLM key + usage API endpoints, encryption service, enhanced audit logger, and comprehensive test scaffolds.
 </objective>
 
 <execution_context>
@@ -217,7 +230,7 @@ portal_router = APIRouter(prefix="/api/portal")
        - CREATE TABLE tenant_llm_keys with RLS enabled (same FORCE ROW LEVEL SECURITY pattern as agents)
        - CREATE TABLE stripe_events (event_id TEXT PK, processed_at TIMESTAMPTZ DEFAULT now())
        - CREATE INDEX idx_audit_events_tenant_type_created ON audit_events (tenant_id, action_type, created_at DESC)
-       - GRANT SELECT, INSERT on tenant_llm_keys to konstruct_app
+       - GRANT SELECT, INSERT, DELETE on tenant_llm_keys to konstruct_app (DELETE needed for key removal)
        - GRANT SELECT, INSERT on stripe_events to konstruct_app
 
     9. Write test scaffolds:
@@ -233,7 +246,7 @@ portal_router = APIRouter(prefix="/api/portal")
     - TenantLlmKey and StripeEvent models exist in billing.py
     - KeyEncryptionService passes encrypt/decrypt/rotate tests
     - Budget alert threshold logic passes at all levels
-    - Alembic migration 005 exists with all schema changes
+    - Alembic migration 005 exists with all schema changes (including DELETE grant on tenant_llm_keys)
     - Config has all new settings fields
   </done>
 </task>
@@ -287,7 +300,7 @@ portal_router = APIRouter(prefix="/api/portal")
        - `GET /api/portal/usage/{tenant_id}/budget-alerts` — for each agent with budget_limit_usd, compare current month cost_usd sum against limit. Return status: "ok" (<80%), "warning" (80-99%), "exceeded" (>=100%).
        - Include the composite index from migration 005 for performance.
 
-    4. Register new routers in the appropriate main.py files. Add channels_router, billing_router, and usage_router to the FastAPI app. The stripe webhook route should be on a separate prefix (/api/webhooks/stripe) without auth.
+    4. Register new routers in the appropriate main.py files. Add channels_router, billing_router, llm_keys_router, and usage_router to the FastAPI app. The stripe webhook route should be on a separate prefix (/api/webhooks/stripe) without auth.
 
     5. Enhance audit logger — in `packages/orchestrator/orchestrator/agents/runner.py`, extend the metadata dict passed to `log_llm_call()` to include:
        - prompt_tokens: extracted from LiteLLM response usage object
@@ -319,23 +332,68 @@ portal_router = APIRouter(prefix="/api/portal")
   </done>
 </task>
 
+<task type="auto" tdd="true">
+  <name>Task 3: LLM key CRUD API endpoints</name>
+  <files>
+    packages/shared/shared/api/llm_keys.py,
+    tests/unit/test_llm_keys_crud.py
+  </files>
+  <behavior>
+    - test_create_llm_key: POST with {provider, label, api_key} encrypts key and returns {id, provider, label, created_at} (no key in response)
+    - test_list_llm_keys_redacted: GET returns list with provider, label, created_at, key_hint (last 4 chars) — never the full key
+    - test_delete_llm_key: DELETE removes key, subsequent GET no longer includes it
+    - test_create_duplicate_provider: POST with same tenant_id+provider returns 409 Conflict (UNIQUE constraint)
+    - test_delete_nonexistent_key: DELETE with unknown key_id returns 404
+  </behavior>
+  <action>
+    1. Create `packages/shared/shared/api/llm_keys.py`:
+       - `llm_keys_router = APIRouter(prefix="/api/portal/tenants/{tenant_id}/llm-keys")`
+       - `GET /api/portal/tenants/{tenant_id}/llm-keys` — list all BYO keys for tenant. Return list of {id, provider, label, key_hint, created_at}. key_hint = last 4 characters of the original key (stored alongside encrypted_key in a separate column, or computed at creation and stored in label metadata). NEVER decrypt the key for listing.
+       - `POST /api/portal/tenants/{tenant_id}/llm-keys` — accepts {provider: str, label: str, api_key: str}. Encrypt api_key using KeyEncryptionService.encrypt(). Store key_hint (last 4 chars of api_key) for display. Insert into tenant_llm_keys table. Return {id, provider, label, key_hint, created_at} with 201 status. Handle UNIQUE(tenant_id, provider) conflict -> return 409.
+       - `DELETE /api/portal/tenants/{tenant_id}/llm-keys/{key_id}` — delete the key row. Verify tenant_id matches to prevent cross-tenant deletion. Return 204 on success, 404 if not found.
+       - Use the same dependency injection pattern as existing portal endpoints (get_db session, tenant authorization).
+
+    2. Update migration 005 if needed: add `key_hint` column (VARCHAR(4), nullable=True) to tenant_llm_keys table for storing the last 4 chars safely without decryption on list.
+
+    3. Write tests in `tests/unit/test_llm_keys_crud.py`:
+       - test_create_llm_key: verify encrypted_key is stored (not plaintext), response has no api_key field
+       - test_list_llm_keys_redacted: verify response never contains encrypted_key or plaintext key, only key_hint
+       - test_delete_llm_key: verify removal and 204 status
+       - test_create_duplicate_provider: verify 409 on UNIQUE violation
+       - test_delete_nonexistent_key: verify 404
+  </action>
+  <verify>
+    <automated>cd /home/adelorenzo/repos/konstruct && pytest tests/unit/test_llm_keys_crud.py -x -v</automated>
+  </verify>
+  <done>
+    - GET /api/portal/tenants/{tenant_id}/llm-keys returns redacted key list (provider, label, key_hint, created_at)
+    - POST creates encrypted key and returns 201 with no secret in response
+    - DELETE removes key and returns 204
+    - Duplicate provider per tenant returns 409
+    - Cross-tenant deletion prevented by tenant_id check
+    - All 5 tests pass
+  </done>
+</task>
+
 </tasks>
 
 <verification>
-All Wave 0 test scaffolds created and passing:
+All test scaffolds created and passing:
 - `pytest tests/unit/test_key_encryption.py -x` — Fernet encrypt/decrypt/rotate
 - `pytest tests/unit/test_budget_alerts.py -x` — threshold logic
 - `pytest tests/unit/test_slack_oauth.py -x` — OAuth state HMAC
 - `pytest tests/unit/test_stripe_webhooks.py -x` — idempotency, status updates, cancellation
 - `pytest tests/unit/test_usage_aggregation.py -x` — SQL aggregates
+- `pytest tests/unit/test_llm_keys_crud.py -x` — LLM key CRUD operations
 - `pytest tests/unit -x -q` — full unit suite still green
 </verification>
 
 <success_criteria>
-- All 5 test files pass with 0 failures
+- All 6 test files pass with 0 failures
 - Alembic migration 005 exists and is syntactically valid
-- New API routers registered and importable
+- New API routers registered and importable (including llm_keys_router)
 - KeyEncryptionService encrypt/decrypt roundtrip works
+- LLM key CRUD endpoints return redacted data (never expose plaintext keys)
 - Audit logger metadata includes prompt_tokens, completion_tokens, cost_usd, provider
 - Existing test suite remains green
 </success_criteria>