863 lines
91 KiB
Markdown
863 lines
91 KiB
Markdown
# Refactoring & Security Plan — adiuvAI Workspace
|
||
|
||
**Generated:** 2026-06-12 · **Tree state:** `main` @ `315c5d0` (clean). Line numbers reference this state — execute file-split refactors (QUAL-*) **last**, they invalidate line references.
|
||
**Scope:** `adiuvAI/` (Electron), `api/` (FastAPI), `waitlist/`, `website/`.
|
||
**Method:** 8 parallel read-only audit agents (security ×2, dead-code/deps ×2, perf/correctness ×2, quality/types, waitlist/website), tool-assisted (`knip`, `npm audit`, `ruff`, `tsc --noEmit`, import-graph). Every finding verified against actual code.
|
||
|
||
**Legend:** `⚠️ REVIEW` = needs human or strong-model validation before execution. Do NOT hand these to a low-capability executor unattended.
|
||
|
||
---
|
||
|
||
## Summary Table
|
||
|
||
| ID | Category | Severity | File | Description |
|
||
|----|----------|----------|------|-------------|
|
||
| SEC-01 | Security | **Critical** | adiuvAI/src/main/api/drizzle-executor.ts | Backend can read ANY file on user's disk via generic FS tool handlers |
|
||
| SEC-02 | Security | High | api/app/api/middleware/rate_limit.py | Zero rate limiting on login/register/refresh (unauthenticated bypass) |
|
||
| SEC-03 | Security | High | api/app/models.py | Per-user Fernet key stored plaintext beside the ciphertext it protects |
|
||
| SEC-04 | Security | High | api/app/core/deep_agent.py +3 | Raw user content/PII sent to Langfuse (GDPR / L.132/2025) |
|
||
| SEC-05 | Security | High | adiuvAI/src/main/api/drizzle-executor.ts | Mass assignment: backend sets arbitrary columns on insert/update |
|
||
| SEC-06 | Security | High | adiuvAI/src/main/index.ts | No will-navigate / setWindowOpenHandler guards on main window |
|
||
| SEC-07 | Security | High | adiuvAI (app-wide) | No Content-Security-Policy |
|
||
| SEC-08 | Security | High | waitlist/app/rate_limit.py | Rate limit keyed on spoofable CF-Connecting-IP / X-Forwarded-For |
|
||
| SEC-09 | Security | High | waitlist/app/config.py | CONFIRM_SECRET defaults to per-process random → broken across workers |
|
||
| SEC-10 | Security | Medium | api/app/api/routes/auth.py | Login 500-crashes for social-only accounts (account-type oracle) |
|
||
| SEC-11 | Security | Medium | api/app/api/routes/auth.py | No password strength validation on register |
|
||
| SEC-12 | Security | Medium | api/app/api/routes/auth.py | No logout endpoint / refresh-token revocation |
|
||
| SEC-13 | Security | Medium | api + adiuvAI (WS handshake) | JWT in WebSocket URL query string (both sides) |
|
||
| SEC-14 | Security | Medium | api/app/config/settings.py | Insecure defaults (JWT_SECRET) with no prod startup guard |
|
||
| SEC-15 | Security | Medium | api (device_ws, scout_runner, deep_agent) | User message content in INFO logs (GDPR) |
|
||
| SEC-16 | Security | Medium | api/app/models.py | Relational-memory entity labels stored plaintext (third-party PII) |
|
||
| SEC-17 | Security | Medium | api/app/api/middleware/rate_limit.py | In-memory limiter state → ×4 bypass under gunicorn -w 4 |
|
||
| SEC-18 | Security | Medium | api (auth.py, scouts.py) | In-memory OAuth state stores break multi-worker; strand Gmail tokens |
|
||
| SEC-19 | Security | Medium | api/app/api/routes/scout_webhooks.py | Gmail Pub/Sub webhook fail-open when audience unset |
|
||
| SEC-20 | Security | Medium | adiuvAI/src/main/ipc.ts | IPC bridge doesn't validate sender; all procedures public |
|
||
| SEC-21 | Security | Medium | adiuvAI/src/main/ai/token.ts | Plaintext token + backup-key fallback in electron-store |
|
||
| SEC-22 | Security | Medium | adiuvAI/src/renderer/lib/httpLink.ts | Web SPA keeps JWT in localStorage |
|
||
| SEC-23 | Security | Medium | waitlist/app/security.py | Origin validation bypass via startswith |
|
||
| SEC-24 | Security | Medium | waitlist/app/routes.py | GDPR erasure + confirm triggered by bare GET (mail scanners) |
|
||
| SEC-25 | Security | Medium | waitlist/app/routes.py | Unauthenticated email re-send = mail-bombing vector |
|
||
| SEC-26 | Security | Medium | website/index.html | Third-party scripts without SRI; lucide@latest from unpkg |
|
||
| SEC-27 | Security | Medium | api/app/api/routes/device_ws.py | _index_sessions: no ownership check + never purged on disconnect |
|
||
| SEC-28 | Security | Medium | adiuvAI (db, attachments) | Local SQLite + attachments unencrypted at rest (product decision) |
|
||
| SEC-29 | Security | Low | api/app/api/routes/auth.py | Email enumeration via register 409 |
|
||
| SEC-30 | Security | Low | api/app/api/routes/device_ws.py | Internal exception strings returned to WS clients |
|
||
| SEC-31 | Security | Low | adiuvAI/src/main/router/index.ts | attachments.create accepts arbitrary sourcePath |
|
||
| SEC-32 | Security | Low | adiuvAI/src/main/index.ts | Scout Gmail deep link forwarded without local state check |
|
||
| SEC-33 | Security | Low | waitlist/app/routes.py | Partial email fragments in logs |
|
||
| SEC-34 | Security | Low | website (all pages) | No CSP / security headers on static site |
|
||
| SEC-35 | Security | Low | website/i18n.js | innerHTML i18n sink (latent XSS) |
|
||
| SEC-36 | Security | Low | adiuvAI/src/main/index.ts | sandbox not explicitly enabled in webPreferences |
|
||
| CORR-01 | Correctness | **Critical** | api/app/api/routes/device_ws.py | Chat tool calls: no timeout + send-before-register race → agent hangs forever |
|
||
| CORR-02 | Correctness | High | api/app/core/device_manager.py | WS unregister clobbers a freshly reconnected connection |
|
||
| CORR-03 | Correctness | High | api/app/core/memory_maintenance.py | Free-tier extraction drains with empty content — feature is a paid no-op |
|
||
| CORR-04 | Correctness | High | adiuvAI (renderer chat contexts) | Backend errors returned as success payloads ignored → chat wedged in "streaming" + listener leaks |
|
||
| CORR-05 | Correctness | High | adiuvAI/src/main/router/index.ts + drizzle-executor.ts | Cascade-delete gaps, zero transactions; AI-issued deletes orphan rows + disk files |
|
||
| CORR-06 | Correctness | High | adiuvAI (backend-client.ts + files/indexer.ts) | WS drop mid-index permanently wedges project in 'scanning' |
|
||
| CORR-07 | Correctness | Medium | adiuvAI/src/main/api/backend-client.ts | WS close handler clobbers newly opened connection (logout→login race) |
|
||
| CORR-08 | Correctness | Medium | api/app/core/memory_maintenance.py | Confidence decay applied per invocation, not per period |
|
||
| CORR-09 | Correctness | Medium | api (models + memory_middleware) | Check-then-insert races: memory_core / memory_relations lack unique constraints |
|
||
| CORR-10 | Correctness | Medium | api/app/api/routes/device_ws.py | Fire-and-forget create_task: no refs, no per-user concurrency cap |
|
||
| CORR-11 | Correctness | Medium | adiuvAI/src/main/router/index.ts | tasks.update unconditionally destroys briefing + chat history |
|
||
| CORR-12 | Correctness | Medium | adiuvAI/src/main/ai/orchestrator.ts | Stream errors emitted as plain stream_end → blank persisted messages |
|
||
| CORR-13 | Correctness | Medium | adiuvAI/src/main/api/backend-client.ts | withRetry retries non-idempotent POSTs (duplicate scout runs) |
|
||
| CORR-14 | Correctness | Medium | api/app/billing/stripe_service.py | Sync Stripe SDK calls block the event loop |
|
||
| CORR-15 | Correctness | Medium | api/app/api/routes/device_ws.py | run_home_stream swallows exceptions; client never gets error stream_end |
|
||
| CORR-16 | Correctness | Medium | api/app/api/routes/auth.py | delete_account: Stripe commit then DB deletes, broad except pass |
|
||
| CORR-17 | Correctness | Medium | adiuvAI/src/main/files + projectFolders.ts | Index-session TOCTOU + stale _active entry |
|
||
| CORR-18 | Correctness | Low | api/app/api/routes/scouts.py | Scout trigger TOCTOU → duplicate concurrent runs |
|
||
| CORR-19 | Correctness | Low | adiuvAI (router, drizzle-executor, auth-manager) | Swallowed-error triage: silent [] returns, bare catches, unguarded JSON.parse |
|
||
| CORR-20 | Correctness | Low | api/app/api/routes/auth.py | Naive/aware datetime kludge on refresh expiry |
|
||
| CORR-21 | Correctness | Low | adiuvAI (misc) | LanguageSync inside App body; fake AbortSignal; startsWith containment |
|
||
| PERF-01 | Performance | High | api/app/api/routes/auth.py | bcrypt runs on the event loop (rate-limit-exempt login path) |
|
||
| PERF-02 | Performance | High | api/app/core/deep_agent.py | Home channel buffers ALL tokens — streaming endpoint doesn't stream |
|
||
| PERF-03 | Performance | High | api/app/core/memory_maintenance.py | Hourly mining duplicates proactive rows; _load_proactive has no LIMIT |
|
||
| PERF-04 | Performance | Medium | adiuvAI/src/main/db/schema.ts | No SQLite indexes on any user-data table (sync full scans block IPC) |
|
||
| PERF-05 | Performance | Medium | api/app/models.py | Missing PG composite/pgvector indexes; refresh_tokens never purged |
|
||
| PERF-06 | Performance | Medium | adiuvAI (orchestrator + ChatSurface) | Per-token IPC message + full markdown re-parse per chunk |
|
||
| PERF-07 | Performance | Medium | api/app/core/deep_agent.py | Tool calls executed serially per LLM step (each a WS round-trip) |
|
||
| PERF-08 | Performance | Medium | api (deep_agent, scout_runner, langfuse_client) | Blocking lf.flush() per request; sync get_prompt on cache miss |
|
||
| PERF-09 | Performance | Medium | api/app/api/middleware/auth.py | get_current_user: 3 queries + full Fernet decrypt on EVERY request |
|
||
| PERF-10 | Performance | Medium | api/app/core/memory_middleware.py | Same User row re-fetched up to 4×/message; update_core commits per key |
|
||
| PERF-11 | Performance | Medium | adiuvAI/src/main/ai/orchestrator.ts | Health-check HTTP round-trip before every chat message |
|
||
| PERF-12 | Performance | Medium | adiuvAI/src/main/router/index.ts | Paid LLM brief regeneration fired by every week-relevant mutation |
|
||
| PERF-13 | Performance | Medium | adiuvAI (vite configs, ChatChartBlock, notes route) | No route code-splitting; recharts + Milkdown eager in initial bundle |
|
||
| PERF-14 | Performance | Medium | adiuvAI/src/renderer/index.tsx | Default QueryClient → refetch storms into sync SQLite on window focus |
|
||
| PERF-15 | Performance | Medium | adiuvAI (projectFolders.ts + indexer.ts) | Double full-folder walk; no WS backpressure (memory spike) |
|
||
| PERF-16 | Performance | Medium | api (scouts/engine.py, device_ws.py) | DB sessions held open across LLM calls (pool starvation) |
|
||
| PERF-17 | Performance | Medium | api/app/core/scout_runner.py | Per-file get_file_metadata WS round-trip during scans |
|
||
| PERF-18 | Performance | Low | api (embeddings.py, llm.py) | Fresh AsyncOpenAI client per call, never closed |
|
||
| PERF-19 | Performance | Low | adiuvAI/src/renderer/components/tasks/TaskListView.tsx | tasks.list returns all rows; pagination client-side |
|
||
| PERF-20 | Performance | Low | api/app/api/middleware/rate_limit.py | Limiter dict never sheds keys; tier read from JWT not DB |
|
||
| PERF-21 | Performance | Low | adiuvAI/src/main/api/drizzle-executor.ts | Unbounded page-details tables; uncapped pdf/docx base64 reads |
|
||
| PERF-22 | Performance | Low | adiuvAI/src/renderer/context | HeaderContext unmemoized value; ContextualChat re-renders route per token |
|
||
| DEAD-01 | Dead code | High | api/requirements.txt | 5 dead deps: pinecone, qdrant-client, boto3, moto[s3], google-auth-oauthlib |
|
||
| DEAD-02 | Dead code | High | api/app/core/scout_registry.py | Fully orphaned module (BaseAgent, zero importers) |
|
||
| DEAD-03 | Dead code | Medium | api/app/api/middleware/rate_limit.py | slowapi Limiter exported, zero decorated routes — dep removable |
|
||
| DEAD-04 | Dead code | Medium | api/app/api/routes/chat.py | HTTP /chat routes unused by Electron client (WS is the channel) |
|
||
| DEAD-05 | Dead code | Medium | adiuvAI/package.json | 4 unused deps: next-themes, mammoth, pdf-parse, @hello-pangea/dnd |
|
||
| DEAD-06 | Dead code | Medium | adiuvAI/src (5 files) | Dead modules: batch-types, useChatStream, useTaskBriefCache, ScoutRunLog, blocks barrel |
|
||
| DEAD-07 | Dead code | Medium | adiuvAI/src/main/auth/backup-key.ts | Documented architecture but zero importers — wire or remove |
|
||
| DEAD-08 | Dead code | Medium | adiuvAI/knip.json | Missing web-SPA entries → knip false-flags live files |
|
||
| DEAD-09 | Dead code | Medium | api/requirements.txt | langchain meta-package → langchain-core; redundant websockets pin |
|
||
| DEAD-10 | Dead code | Low | api (13 files) | 13 ruff F401 unused imports (auto-fixable) |
|
||
| DEAD-11 | Dead code | Low | adiuvAI/src (7 exports) | Dead utility exports (getRawSqlite, parseDateRange, formatTime, …) |
|
||
| DEAD-12 | Dead code | Low | api/requirements.txt + backend-client.ts | Dev deps in runtime requirements; stale endpoint doc comments |
|
||
| DEPS-01 | Dependencies | High | adiuvAI/package.json | ws 8.19.0 vulnerable (GHSA-58qx-3vcg-4xpx) — only runtime-reachable vuln |
|
||
| DEPS-02 | Dependencies | High | api/requirements.txt | python-jose floor 3.3.0 permits CVE-2024-33663/33664 versions |
|
||
| DEPS-03 | Dependencies | Medium | api/requirements.txt | cryptography floor 42.0.0 permits CVE-2024-26130 versions |
|
||
| DEPS-04 | Dependencies | Medium | api/ | No lock file — `>=` floors make installs unauditable |
|
||
| DEPS-05 | Dependencies | Medium | adiuvAI/package.json | eslint 8 + @typescript-eslint 5 both EOL |
|
||
| DEPS-06 | Dependencies | Medium | adiuvAI/package.json | Electron 40 → 42 (two majors of Chromium security patches behind) |
|
||
| DEPS-07 | Dependencies | Low | adiuvAI (transitive) | tmp/tar/esbuild advisories in forge/drizzle-kit toolchain — no fix, monitor |
|
||
| DEPS-08 | Dependencies | Low | adiuvAI/package.json | @types/ws in dependencies instead of devDependencies |
|
||
| QUAL-01 | Quality | High | adiuvAI/src/main/router/index.ts | God file: 1967 LOC, 15 sub-routers — split per domain |
|
||
| QUAL-02 | Quality | High | api/app/core/deep_agent.py | _run_single_agent vs _run_single_agent_stream ~90% duplicated |
|
||
| QUAL-03 | Quality | High | api (4 files) | Four hand-rolled LLM tool loops — extract one |
|
||
| QUAL-04 | Quality | High | adiuvAI/src/main/router/index.ts | CRUD repetition ×8 tables — extract factory |
|
||
| QUAL-05 | Quality | High | api/app/core/deep_agent.py | God file: 1329 LOC — split prompt-context / tools / runner |
|
||
| QUAL-06 | Quality | Medium | adiuvAI/src/main/api/backend-client.ts | God file 1191 LOC; 230-line openDeviceWebSocket; send* boilerplate ×6 |
|
||
| QUAL-07 | Quality | Medium | api/app/core/scout_runner.py | 1051 LOC; run_local_agent/run_cloud_agent 210-line twins |
|
||
| QUAL-08 | Quality | Medium | api/app/api/routes/device_ws.py | Protocol + business logic mixed — extract ws_handlers package |
|
||
| QUAL-09 | Quality | Medium | adiuvAI/src/renderer/components/projects/ProjectSidebar.tsx | 1292 LOC, 25+ useState — extract dialogs |
|
||
| QUAL-10 | Quality | Medium | api/app/api/routes/scouts.py | Ownership-check + 404 repeated 6× — FastAPI dependency |
|
||
| QUAL-11 | Quality | Medium | adiuvAI/src/main/router/index.ts | Three competing error-return styles, zero TRPCError |
|
||
| QUAL-12 | Quality | Medium | api/app/api/routes/device_ws.py | Raw-dict frames bypass pydantic schemas (also kills 7 type:ignores) |
|
||
| QUAL-13 | Quality | Medium | api + adiuvAI (WS boundary) | camelCase/snake_case contract untrusted (dual-read hedges) |
|
||
| QUAL-14 | Quality | Medium | api (auth.py, scouts.py) | OAuth route blocks → dedicated files; shared TTLStateStore |
|
||
| QUAL-15 | Quality | Low | api/app/agents | Copy-pasted _is_uuid + row formatting across agents |
|
||
| TYPE-01 | Type safety | High | adiuvAI/src/renderer/components/ai/blocks (3 files) | Broken relative imports — block types silently `any` |
|
||
| TYPE-02 | Type safety | High | api/ | No Python type checker configured at all |
|
||
| TYPE-03 | Type safety | Medium | adiuvAI/ | tsc --noEmit does not pass; no CI typecheck |
|
||
| TYPE-04 | Type safety | Medium | api/app/core/deep_agent.py | context: dict[str, Any] threaded everywhere — RequestContext model |
|
||
| TYPE-05 | Type safety | Medium | adiuvAI/src/main/api/drizzle-executor.ts | Cast cluster: table-as-Record ×5, bogus Boolean predicate casts |
|
||
| TYPE-06 | Type safety | Low | adiuvAI/src/renderer | (window as any).electronAI ×4 — global declaration file |
|
||
| TYPE-07 | Type safety | Low | api + adiuvAI | Misc: ToolResult TypedDict, 11 missing py annotations, vite/client types |
|
||
|
||
---
|
||
|
||
## Execution Order
|
||
|
||
Line numbers reference the current tree — **do mechanical fixes before file splits**.
|
||
|
||
**Phase 0 — Mechanical quick wins (safe for low-capability executor, no review needed):**
|
||
TYPE-01 → DEPS-01 → DEAD-10 → DEAD-01 → DEAD-02 → DEAD-05 → DEAD-08 → DEPS-08 → DEAD-11 → DEAD-12 → PERF-14 → PERF-18 → CORR-20
|
||
|
||
**Phase 1 — Critical & High security (ALL ⚠️ REVIEW, human validates each diff):**
|
||
SEC-01 → SEC-05 → SEC-06 → SEC-07 → SEC-20 (these four+one harden the Electron trust boundary together) → SEC-02 → SEC-14 → SEC-19 (share a startup-guard mechanism: do SEC-14 first) → SEC-09 → SEC-08 → SEC-04 → SEC-15 → SEC-03 (needs migration design)
|
||
|
||
**Phase 2 — Critical & High correctness:**
|
||
CORR-01 → CORR-02 (same files, same review session) → CORR-04 → CORR-12 (CORR-04 depends on CORR-12's error-frame contract — implement together) → CORR-06 → CORR-05 ⚠️ → CORR-03
|
||
|
||
**Phase 3 — Remaining Medium security + correctness:**
|
||
SEC-10 → SEC-11 → SEC-12 → SEC-13 (coordinated api+electron change) → SEC-17/SEC-18 (one Redis introduction covers both) → SEC-27 → SEC-21 → remaining SEC Mediums → CORR-07..CORR-17
|
||
|
||
**Phase 4 — Performance:**
|
||
PERF-01 → PERF-02 ⚠️ → PERF-03 → PERF-04 → PERF-05 (migrations) → PERF-09/PERF-10 (same subsystem) → PERF-06 → rest
|
||
|
||
**Phase 5 — Dependencies & tooling:**
|
||
DEPS-02 → DEPS-03 → DEPS-04 → TYPE-02 → TYPE-03 → DEPS-05 ⚠️ → DEPS-06 ⚠️
|
||
|
||
**Phase 6 — Structural refactors (after all above; invalidates line numbers):**
|
||
QUAL-02 ⚠️ → QUAL-03 ⚠️ → QUAL-01 → QUAL-04 ⚠️ → QUAL-05 → QUAL-06..QUAL-15 → TYPE-04..TYPE-07 → DEAD-04 ⚠️ / DEAD-07 ⚠️ (owner decisions)
|
||
|
||
---
|
||
|
||
# 1. Security
|
||
|
||
> Per instruction: **every SEC item is ⚠️ REVIEW** unless explicitly noted "mechanical". A botched auth check or sanitizer is worse than the original bug.
|
||
|
||
### SEC-01 — Backend can read ANY file on the user's disk ⚠️ REVIEW
|
||
- **File:** `adiuvAI/src/main/api/drizzle-executor.ts` — `handleListDirectory` L375-398, `handleReadFileContent` L400-436, `handleGetFileMetadata` L438-461
|
||
- **Severity:** Critical · OWASP A01:2021 Broken Access Control
|
||
- **Problem:** These handlers take a backend-supplied `path`, resolve it (`fs.promises.realpath(path.resolve(dirPath))`) and read it with **no allowlist or root containment**. `read_file_content` returns up to 500 KB of any file (`~/.ssh/id_rsa`, browser cookie DBs). The backend is semi-trusted by design; if compromised, blast radius = the entire user filesystem. The backend adds no guard either (`api/app/agents/filesystem_agent.py:24-33` passes absolute paths straight through). Contrast: `handleReadProjectFolderFile` (L511-529) does containment correctly.
|
||
- **Fix:** After realpath, assert the resolved path is inside an allowed root: the set of configured `projects.folderPath` values (query the projects table) plus nothing else. Use `const rel = path.relative(root, resolved); if (rel.startsWith('..') || path.isAbsolute(rel)) throw new ExecutorError('Access denied')`. Apply identically in all three handlers. Also fix the existing `startsWith` containment in `handleReadProjectFolderFile` L527 to use the same `path.relative` check (defeats `C:\proj\foo-evil` vs `C:\proj\foo`).
|
||
- **Risk:** Local filesystem scouts or agent flows that legitimately read outside project folders will break — verify which directories local scout configs reference and add those roots to the allowlist before locking down. Test: agent file-read inside project folder still works; read of `C:\Windows\win.ini` rejected.
|
||
|
||
### SEC-02 — No rate limiting on login/register/refresh ⚠️ REVIEW
|
||
- **File:** `api/app/api/middleware/rate_limit.py:41-48` (`_EXEMPT_PATHS`), `:88-91` (no-token pass-through)
|
||
- **Severity:** High · OWASP A07:2021 Identification & Authentication Failures
|
||
- **Problem:** Limiter only acts on requests with a valid Bearer JWT; `if not token: return await call_next(request)`. Login/register/refresh are unauthenticated → **never rate-limited**. Unlimited password guessing and refresh-token guessing.
|
||
- **Fix:** Remove `/api/v1/auth/login` and `/api/v1/auth/register` from `_EXEMPT_PATHS`. In `dispatch()`, when no token is present and `request.url.path.startswith("/api/v1/auth/")`, apply an IP-keyed sliding window (reuse the existing `_window` mechanism keyed on client IP, fixed limit e.g. 5/min for login/register/refresh, 429 on excess). Note this remains per-process until SEC-17 lands Redis.
|
||
- **Risk:** Shared-NAT users may hit limits; size window accordingly. Test: 6th login attempt in a minute → 429; authenticated traffic unaffected.
|
||
|
||
### SEC-03 — Fernet key stored in plaintext beside its ciphertext ⚠️ REVIEW
|
||
- **File:** `api/app/models.py:83` (`User.encryption_key`), read at `api/app/core/memory_middleware.py:559-566`
|
||
- **Severity:** High · OWASP A02:2021 Cryptographic Failures
|
||
- **Problem:** The per-user key that encrypts all memory tiers sits as a plaintext column in the same PostgreSQL DB. Any DB-level compromise or backup leak decrypts everything — the "encrypted at rest" zero-trust claim collapses.
|
||
- **Fix:** Introduce a KEK from env/KMS (model: the existing `OAUTH_ENCRYPTION_KEY` pattern in `api/app/integrations/__init__.py:90-102`). Store `Fernet(KEK).encrypt(user_key)` in the column; unwrap in `_get_fernet`. One-time Alembic data migration re-wrapping existing keys; keep a dual-read window (try unwrap, fall back to raw) during rollout, then remove fallback.
|
||
- **Risk:** Migration must be reversible and tested against a DB snapshot; losing the KEK = losing all memory data. Requires key-rotation runbook. This is design work — human required.
|
||
|
||
### SEC-04 — Raw user content & PII sent to Langfuse ⚠️ REVIEW
|
||
- **Files:** `api/app/core/deep_agent.py:912,926,933,975` · `api/app/scouts/engine.py:260-268` (email subject/sender/2000-char body) · `api/app/core/scout_runner.py:266-278`
|
||
- **Severity:** High · OWASP A09:2021 / GDPR Art. 5, 28, 44 / Italian Law 132/2025
|
||
- **Problem:** Full conversation history, tool I/O, and triaged **email content** are transmitted to Langfuse when configured. `hash_user_id()` only pseudonymizes the identifier; bodies go in clear. Requires Langfuse as contracted sub-processor; possible international transfer depending on `LANGFUSE_BASE_URL`.
|
||
- **Fix:** Add `LANGFUSE_CAPTURE_CONTENT: bool = False` to settings. Wrap every `input=`/`output=` content assignment in the three files: when flag is false, pass `{"redacted": True, "chars": len(...)}` instead. Keep metadata/token counts. Document Langfuse as sub-processor in the privacy policy regardless.
|
||
- **Risk:** Reduced debugging richness. Verify Langfuse prompt management (`get_prompt`) is unaffected — it's a separate path.
|
||
|
||
### SEC-05 — Mass assignment in drizzle-executor insert/update ⚠️ REVIEW
|
||
- **File:** `adiuvAI/src/main/api/drizzle-executor.ts` — `handleInsert` L268-290, `handleUpdate` L292-327
|
||
- **Severity:** High · OWASP A08:2021
|
||
- **Problem:** Table name is allowlisted, columns are not: `{ id: crypto.randomUUID(), ...data, createdAt: now }` spreads backend keys verbatim — any real column writable. Also: `...data` is spread **after** `id`, so a backend-supplied `id` overrides the executor's generated one (contradicting the code's own comment).
|
||
- **Fix:** Define a per-table writable-column allowlist (object map table→string[]). Before building `values`/`withTimestamp`, pick only allowlisted keys; always strip `id` and `createdAt` from incoming `data`/`updates`. Reorder insert to `{ ...picked, id: crypto.randomUUID(), createdAt: now, updatedAt: now }`.
|
||
- **Risk:** Additive validation. Test every agent CRUD flow (create task/note/event, update, propose_note_edit) still works — the allowlist must cover all fields agents legitimately set.
|
||
|
||
### SEC-06 — No navigation / window-open guards ⚠️ REVIEW
|
||
- **File:** `adiuvAI/src/main/index.ts` — `createWindow` L85-117 (handlers absent repo-wide)
|
||
- **Severity:** High · OWASP A05:2021
|
||
- **Problem:** No `setWindowOpenHandler`, no `will-navigate` guard. Any navigation to remote content lands in the privileged renderer (which has `window.electronTRPC`/`electronAI`).
|
||
- **Fix:** In `createWindow` add:
|
||
```ts
|
||
mainWindow.webContents.setWindowOpenHandler(({ url }) => {
|
||
if (url.startsWith('https://')) shell.openExternal(url);
|
||
return { action: 'deny' };
|
||
});
|
||
mainWindow.webContents.on('will-navigate', (e, url) => {
|
||
const ok = (MAIN_WINDOW_VITE_DEV_SERVER_URL && url.startsWith(MAIN_WINDOW_VITE_DEV_SERVER_URL)) || url.startsWith('file://');
|
||
if (!ok) e.preventDefault();
|
||
});
|
||
```
|
||
- **Risk:** Minimal — app uses internal TanStack routing. Test external links in chat still open in OS browser.
|
||
|
||
### SEC-07 — No Content-Security-Policy ⚠️ REVIEW
|
||
- **File:** `adiuvAI/index.html` (no meta CSP); `src/main/index.ts` (no onHeadersReceived)
|
||
- **Severity:** High · OWASP A05:2021
|
||
- **Problem:** Zero CSP → no defense-in-depth for any future XSS sink; `connect-src` unrestricted (compromised renderer exfiltrates anywhere).
|
||
- **Fix:** In production only (`!IS_DEV`), register `session.defaultSession.webRequest.onHeadersReceived` injecting: `default-src 'self'; script-src 'self'; connect-src 'self' https://<backend-host> wss://<backend-host>; img-src 'self' data: https:; style-src 'self' 'unsafe-inline'; object-src 'none'; frame-ancestors 'none'`. Backend host from existing config.
|
||
- **Risk:** Tailwind/shadcn need `style-src 'unsafe-inline'`; Vite dev needs exemption (gate on IS_DEV). Test packaged build fully: chat, charts, Milkdown editor, images.
|
||
|
||
### SEC-08 — Waitlist rate limit keyed on spoofable headers ⚠️ REVIEW
|
||
- **File:** `waitlist/app/rate_limit.py:19-32`
|
||
- **Severity:** High · OWASP A04:2021 / CWE-348
|
||
- **Problem:** `_get_client_ip` trusts `cf-connecting-ip` then `x-forwarded-for` unconditionally. Random header per request = fresh 5/min bucket → unlimited signup spam, Brevo quota burn, poisoned stored `ip_address`.
|
||
- **Fix:** Add `TRUSTED_PROXY_IPS: str = ""` setting. Trust `cf-connecting-ip` only when `request.client.host` is in that list; otherwise use `request.client.host`. Also firewall origin to Cloudflare ranges at infra level.
|
||
- **Risk:** Misconfigured trusted-proxy list rate-limits Cloudflare itself (all users share CF egress IPs) — must be deployed together with correct env value.
|
||
|
||
### SEC-09 — Waitlist CONFIRM_SECRET random default ⚠️ REVIEW
|
||
- **File:** `waitlist/app/config.py:19`; `waitlist/Dockerfile:21` (`-w 2`)
|
||
- **Severity:** High · OWASP A05:2021
|
||
- **Problem:** `CONFIRM_SECRET: str = secrets.token_hex(32)` — with 2 gunicorn workers and no env var, each worker has a different HMAC secret: ~50% of confirm/unsubscribe links fail silently; all tokens die on restart. Breaks double opt-in AND the GDPR erasure path.
|
||
- **Fix:** Remove default: `CONFIRM_SECRET: str` (required field — pydantic fails startup if unset). Update `.env.example` comment.
|
||
- **Risk:** Deploys without the env var now fail loudly at boot — that's the point. Coordinate with deploy.
|
||
|
||
### SEC-10 — Login 500 for social-only accounts ⚠️ REVIEW
|
||
- **File:** `api/app/api/routes/auth.py:169` (and `_verify_password` L73-74)
|
||
- **Severity:** Medium · OWASP A07:2021
|
||
- **Problem:** OAuth-only users have `password_hash=None`; `hashed.encode()` → uncaught `AttributeError` → 500. Differing responses (500 vs 401) = account-type oracle + crash.
|
||
- **Fix:** Replace the check with: `if user is None or user.password_hash is None or not _verify_password(body.password, user.password_hash): raise HTTPException(401, "Invalid credentials")`.
|
||
- **Risk:** None. Test: password login against a Google-only account → 401.
|
||
|
||
### SEC-11 — No password strength on register ⚠️ REVIEW (mechanical)
|
||
- **File:** `api/app/api/routes/auth.py:100-104`
|
||
- **Severity:** Medium · OWASP A07:2021
|
||
- **Problem:** `password: str` unconstrained (1-char/empty accepted); `_ChangePasswordRequest` already requires `min_length=8`. `email` is plain `str`, not `EmailStr`.
|
||
- **Fix:** Line 102: `password: str = Field(min_length=8)`; change `email: str` to `email: EmailStr` (import from pydantic). Mirror what change-password already does.
|
||
- **Risk:** None for new accounts. Existing weak-password accounts unaffected.
|
||
|
||
### SEC-12 — No logout / refresh-token revocation ⚠️ REVIEW
|
||
- **File:** `api/app/api/routes/auth.py` (endpoint absent; rotation only at L209)
|
||
- **Severity:** Medium · OWASP A07:2021
|
||
- **Problem:** No server-side logout; stolen refresh token valid 30 days; no reuse detection on rotated tokens.
|
||
- **Fix:** Add `POST /api/v1/auth/logout` (authenticated): body `{refresh_token: str}`, delete the matching `RefreshToken` row (SHA-256 the input, filter by hash + `user_id`). Optional `all: bool` to delete all the user's tokens. Wire Electron `auth-manager.ts` logout to call it before clearing local tokens.
|
||
- **Risk:** Electron + backend change together. Test logout then refresh → 401.
|
||
|
||
### SEC-13 — JWT in WebSocket URL query string ⚠️ REVIEW
|
||
- **Files:** `api/app/api/routes/device_ws.py:78` · `adiuvAI/src/main/api/backend-client.ts:895`
|
||
- **Severity:** Medium · OWASP A09:2021
|
||
- **Problem:** `?token=<jwt>` lands in reverse-proxy access logs, APM traces. Token grants full account access for 30 min.
|
||
- **Fix:** The Electron client uses the `ws` package which supports handshake headers: pass `{ headers: { Authorization: \`Bearer ${token}\` } }` as the second arg to `new WebSocket(url, opts)`. Backend: read `websocket.headers.get("authorization")` first, fall back to `?token=` for one release, then remove fallback. Interim mitigation: strip query strings for `/api/v1/ws/device` in proxy access logs.
|
||
- **Risk:** Coordinated two-repo change; old clients break when fallback removed. Web SPA (browser WebSocket) cannot set headers — if web ever uses the WS, it needs the ticket pattern instead.
|
||
|
||
### SEC-14 — Insecure config defaults, no prod guard ⚠️ REVIEW
|
||
- **File:** `api/app/config/settings.py:6-7`; `api/docker-compose.yml:22-23`
|
||
- **Severity:** Medium · OWASP A05:2021
|
||
- **Problem:** `JWT_SECRET = "change-me-in-production"` and `postgres:postgres` defaults; nothing refuses to start in prod with defaults → forgeable JWTs.
|
||
- **Fix:** Add a pydantic `model_validator` on `Settings`: if `ENV == "prod"` and (`JWT_SECRET == "change-me-in-production"` or `len(JWT_SECRET) < 32`), raise `ValueError` at import. Same guard hook used by SEC-19 (`GMAIL_PUBSUB_AUDIENCE` empty → fail in prod).
|
||
- **Risk:** Prod deploys with missing env now fail at boot (intended). Dev unaffected.
|
||
|
||
### SEC-15 — User message content in INFO logs ⚠️ REVIEW
|
||
- **Files:** `api/app/api/routes/device_ws.py:258-265,343` · `api/app/core/scout_runner.py:292-311` · `api/app/core/deep_agent.py:958-984` · `api/app/core/memory_middleware.py:528,554`
|
||
- **Severity:** Medium · OWASP A09:2021 / GDPR
|
||
- **Problem:** Chat messages (`message[:200]`), tool args/outputs (`[:800]`/`[:1200]`), memory queries logged at INFO — uncontrolled PII sink in centralized logs.
|
||
- **Fix:** Replace content interpolations with length+ID only (e.g. `logger.info("home_request user=%s chars=%d", user_id, len(message))`). Gate full content behind `logger.debug` + a `LOG_CONTENT=false` setting.
|
||
- **Risk:** Less debuggable ops. Each call site is a one-line change; review the full grep list before executing.
|
||
|
||
### SEC-16 — Relational-memory labels stored plaintext ⚠️ REVIEW
|
||
- **File:** `api/app/models.py:432-435`; predicates at `api/app/api/routes/memory.py:34-45`
|
||
- **Severity:** Medium · OWASP A02:2021 / GDPR
|
||
- **Problem:** `subject_label`/`object_label` hold names of user's contacts/employers (`works_at`, `reports_to`) in plaintext — third-party personal data. Only `notes_encrypted` protected.
|
||
- **Fix (decision):** Either encrypt labels with the user Fernet key (breaks label-match queries at `memory_middleware.py:421-428` — would need deterministic encryption or hashing for lookups) or formally document in the processing record and rely on SEC-03's KEK to mitigate DB-leak. **Recommend the latter** + SEC-03. Human decision required.
|
||
- **Risk:** Encrypting breaks `upsert_relation` matching — substantial rework. Don't hand to low-capability model.
|
||
|
||
### SEC-17 — In-memory rate limiter ×4 bypass ⚠️ REVIEW
|
||
- **File:** `api/app/api/middleware/rate_limit.py:67,81`
|
||
- **Severity:** Medium · OWASP A04:2021
|
||
- **Problem:** Per-process dicts under `gunicorn -w 4` → effective limit up to 4× tier limit, nondeterministic.
|
||
- **Fix:** Back the sliding window with Redis (`INCR` + `EXPIRE` or sorted-set window) keyed `rl:{user_id}:{minute}`. docker-compose already has a commented Redis service — enable it. Do together with SEC-18 (one Redis introduction).
|
||
- **Risk:** New infra dependency; need connection-failure fallback policy (fail-open vs fail-closed — recommend fail-open with warning log).
|
||
|
||
### SEC-18 — In-memory OAuth state stores ⚠️ REVIEW
|
||
- **Files:** `api/app/api/routes/auth.py:62` (`_pending_states`) · `api/app/api/routes/scouts.py:458,694-697` (`_pending_scout_oauth_states`, holds encrypted Gmail token between callback and finalize)
|
||
- **Severity:** Medium · OWASP A04:2021
|
||
- **Problem:** Multi-worker: authorize and callback can hit different workers → random 401s; scout flow strands the Gmail token; all state lost on redeploy.
|
||
- **Fix:** Move both to Redis with TTL (in-code comments already prescribe this). Implement one `TTLStateStore` (see QUAL-14) with `set(key, dict, ttl)` / `pop(key)`; Redis-backed in prod, dict-backed in tests.
|
||
- **Risk:** Prereq: Redis from SEC-17. Test full OAuth login + scout Gmail connect flows.
|
||
|
||
### SEC-19 — Gmail Pub/Sub webhook fail-open ⚠️ REVIEW
|
||
- **File:** `api/app/api/routes/scout_webhooks.py:48-52`; default at `api/app/config/settings.py:69`
|
||
- **Severity:** Medium · OWASP A05:2021/A07:2021
|
||
- **Problem:** Empty `GMAIL_PUBSUB_AUDIENCE` → `_verify_pubsub_jwt` returns True for ANY unauthenticated POST; attacker-supplied `emailAddress` triggers scout runs (LLM cost) for arbitrary users. Also unauthenticated → not rate-limited (SEC-02).
|
||
- **Fix:** Fail closed: `if not settings.GMAIL_PUBSUB_AUDIENCE: if settings.ENV == "prod": return False; logger.warning(...); return True`. Tie into SEC-14's startup guard.
|
||
- **Risk:** Prod deploys without the env var stop receiving Gmail push (intended — they were unverified anyway).
|
||
|
||
### SEC-20 — IPC bridge doesn't validate sender ⚠️ REVIEW
|
||
- **Files:** `adiuvAI/src/main/ipc.ts:44-87` · `adiuvAI/src/main/index.ts:122-131` (dialog + scope handlers)
|
||
- **Severity:** Medium · OWASP A01:2021
|
||
- **Problem:** `ipcMain.on('trpc', …)` dispatches without checking `event.senderFrame`; all router procedures are `publicProcedure`. Any frame that runs in the renderer gets full CRUD/auth/scout access. The cheap control that contains SEC-06/SEC-07 failures.
|
||
- **Fix:** At the top of each `ipcMain.on/handle` callback: validate `event.senderFrame.url` starts with the dev-server URL (dev) or the app's `file://` index (prod); silently drop otherwise. Extract a `isTrustedSender(event)` helper used by all four registration points (`trpc`, `ai:contextual-scope-update`, `dialog:showOpenDialog`, any others found at execution time).
|
||
- **Risk:** None today (no sub-frames). Test dev + packaged.
|
||
|
||
### SEC-21 — Plaintext token/backup-key fallback ⚠️ REVIEW
|
||
- **Files:** `adiuvAI/src/main/ai/token.ts:37-47` · `adiuvAI/src/main/auth/backup-key.ts:24-35`
|
||
- **Severity:** Medium · OWASP A02:2021
|
||
- **Problem:** When `safeStorage.isEncryptionAvailable()` is false (keyring-less Linux/WSL), JWTs and the AES backup key are written plaintext to electron-store JSON. `readFromStore` also silently returns raw string on decrypt failure (downgrade masking).
|
||
- **Fix:** (1) Tag values with `enc:`/`plain:` prefix on write; on read, never attempt plaintext interpretation of an `enc:` value. (2) When safeStorage unavailable: keep tokens in-memory only (re-login per launch) and emit a renderer-visible warning. Never persist the backup key plaintext — fail backup-key creation instead.
|
||
- **Risk:** Keyring-less Linux users lose session persistence. Migration: existing stored values have no prefix — treat unprefixed as legacy-plaintext, re-encrypt on first read.
|
||
|
||
### SEC-22 — Web SPA token in localStorage ⚠️ REVIEW
|
||
- **File:** `adiuvAI/src/renderer/lib/httpLink.ts:19`
|
||
- **Severity:** Medium (web build only) · OWASP A07:2021
|
||
- **Problem:** `localStorage.getItem('adiuvai-token')` — readable by any XSS.
|
||
- **Fix (decision):** httpOnly secure cookie set by backend (needs CSRF protection + CORS review) or in-memory + silent refresh. If deferring, at minimum pair with strict CSP on the web deployment. Product/infra decision — don't execute blindly.
|
||
|
||
### SEC-23 — Waitlist Origin startswith bypass ⚠️ REVIEW (mechanical)
|
||
- **File:** `waitlist/app/security.py:50-51`
|
||
- **Severity:** Medium · OWASP A01:2021 / CWE-346
|
||
- **Problem:** `origin.startswith(o)` → `https://adiuvai.com.evil.example` passes.
|
||
- **Fix:** `origin in allowed` exact match; for Referer, compare `urllib.parse.urlsplit(referer)._replace(path="", query="", fragment="")` scheme+host against allowed origins.
|
||
- **Risk:** None; existing tests cover the endpoint.
|
||
|
||
### SEC-24 — GDPR erasure + confirm on bare GET ⚠️ REVIEW
|
||
- **File:** `waitlist/app/routes.py:87` (confirm), `:125-154` (unsubscribe → `_anonymize_entry`)
|
||
- **Severity:** Medium · CWE-650
|
||
- **Problem:** Mail scanners (SafeLinks, Mimecast) follow GETs → can anonymize a subscriber or auto-confirm a bot-planted email before any human clicks.
|
||
- **Fix:** Both GET handlers render a minimal HTML page with `<form method="POST">` + the token in a hidden field; move the state change to new POST handlers (`POST /waitlist/confirm`, `POST /waitlist/unsubscribe`). Keep token validation identical.
|
||
- **Risk:** Email templates unchanged (links still GET the page). Update the 19-test suite: existing GET tests now assert the form page; add POST tests.
|
||
|
||
### SEC-25 — Unauthenticated re-send mail bombing ⚠️ REVIEW
|
||
- **File:** `waitlist/app/routes.py:52-62`
|
||
- **Severity:** Medium · OWASP API4:2023
|
||
- **Problem:** Re-POSTing a known email re-triggers `send_confirmation_email`. With SEC-08's spoof bypass: mail-bomb any subscribed address via your Brevo account.
|
||
- **Fix:** Add `last_email_sent_at` column (Alembic migration). In the existing-entry branch, skip the send if `last_email_sent_at` < 15 min ago (still return the generic success response — preserves anti-enumeration).
|
||
- **Risk:** Migration + test. Depends on SEC-08 for full mitigation.
|
||
|
||
### SEC-26 — Website scripts without SRI; lucide@latest ⚠️ REVIEW (mechanical)
|
||
- **File:** `website/index.html:44-48`
|
||
- **Severity:** Medium · OWASP A08:2021 / CWE-353
|
||
- **Problem:** GSAP + ScrollTrigger (cdnjs) and `lucide@latest` (unpkg) loaded with no `integrity`/`crossorigin`; `@latest` ships whatever unpkg serves — supply-chain risk on the page hosting the PII signup form.
|
||
- **Fix:** Pin lucide to an exact version; add `integrity="sha384-…" crossorigin="anonymous"` to all three script tags (compute hashes from the pinned files), or self-host under `website/assets/`.
|
||
- **Risk:** Hash must match exact file; test animations + icons after.
|
||
|
||
### SEC-27 — _index_sessions: no ownership check, never purged ⚠️ REVIEW
|
||
- **File:** `api/app/api/routes/device_ws.py:65` (registry), `:700` (cancel), `:732` (batch)
|
||
- **Severity:** Medium · OWASP A01:2021
|
||
- **Problem:** `_handle_index_file_batch` / `_handle_index_session_cancel` look up by `sessionId` without verifying `session["user_id"] == user_id` — a user who learns another's session UUID can cancel it or corrupt it. Sessions also leak forever on mid-index disconnect (no TTL, no cleanup in WS finally).
|
||
- **Fix:** (1) In both handlers, after lookup: `if session is None or session["user_id"] != user_id: send error frame, return`. (2) In the WS `finally` block, drop all `_index_sessions` entries whose `user_id == user_id` and `ws is websocket`.
|
||
- **Risk:** None — pure tightening. Test indexing E2E (test_ws_index_session suite exists).
|
||
|
||
### SEC-28 — Local data unencrypted at rest ⚠️ REVIEW (product decision)
|
||
- **Files:** `adiuvAI/src/main/db/index.ts:94` (plain SQLite), `adiuvAI/src/main/attachments/storage.ts:17-19`
|
||
- **Severity:** Medium (informational) · OWASP A02:2021
|
||
- **Problem:** All local user content (clients, projects, notes, attachments) plaintext on disk. For a data-sovereignty product, a deliberate gap.
|
||
- **Fix (decision):** Options: SQLCipher (better-sqlite3-multiple-ciphers fork), or document reliance on OS full-disk encryption, or encrypt attachments with the existing device backup key (DEAD-07). Roberto decides; do not execute without direction.
|
||
|
||
### SEC-29 — Register email enumeration · Low ⚠️ REVIEW
|
||
- **File:** `api/app/api/routes/auth.py:127` — 409 "Email already registered". Acceptable UX trade-off for most SaaS; mitigate via SEC-02 rate limiting. **Fix:** no code change now; revisit if email verification flow lands.
|
||
|
||
### SEC-30 — Internal exception strings to WS clients · Low ⚠️ REVIEW (mechanical)
|
||
- **File:** `api/app/api/routes/device_ws.py:467-468, 503-505, 539-541, 578-580, 625-631, 653-659`
|
||
- **Problem:** `error=str(exc)` / f-string exception text sent to renderer — can leak DB errors, paths.
|
||
- **Fix:** Replace each with a generic message (`"Internal error"` + the request_id); keep the existing server-side `logger.exception`. 6 call sites, mechanical.
|
||
|
||
### SEC-31 — attachments.create arbitrary sourcePath · Low ⚠️ REVIEW
|
||
- **File:** `adiuvAI/src/main/router/index.ts:1893-1915` → `attachments/storage.ts:25-37`
|
||
- **Problem:** Renderer-supplied absolute path copied without restriction; defense-in-depth issue (matters only post-SEC-06/20 compromise).
|
||
- **Fix:** Track paths returned by the `dialog:showOpenDialog` handler in a per-session `Set`; `attachments.create` rejects `sourcePath` not in the set. Cap file size (e.g. 100 MB).
|
||
- **Risk:** Drag-and-drop flows (if any) bypass the dialog — verify before enforcing.
|
||
|
||
### SEC-32 — Scout Gmail deep link no state check · Low ⚠️ REVIEW
|
||
- **File:** `adiuvAI/src/main/index.ts:46-54`
|
||
- **Problem:** `adiuvai://scout/oauth/gmail/callback` code+state forwarded to renderer with no pending-flow check (login OAuth path does this correctly in `auth-manager.ts:321-359`). Backend validates state server-side, limiting impact.
|
||
- **Fix:** Mirror `_pendingOAuth`: main process records pending Gmail-OAuth state when the flow starts; `handleDeepLink` drops callbacks whose state doesn't match.
|
||
|
||
### SEC-33 — Waitlist partial email in logs · Low
|
||
- **File:** `waitlist/app/routes.py:75`. `email[:3]` can reveal short local-parts. **Fix:** log entry `id` only. Mechanical.
|
||
|
||
### SEC-34 — Website missing CSP/security headers · Low ⚠️ REVIEW
|
||
- **Fix:** At the reverse proxy serving `website/`: add `Content-Security-Policy` (allowlist cdnjs/unpkg or self-hosted after SEC-26), `X-Content-Type-Options: nosniff`, `Referrer-Policy: strict-origin-when-cross-origin`. Infra change, not repo code.
|
||
|
||
### SEC-35 — Website innerHTML i18n sink · Low ⚠️ REVIEW (mechanical)
|
||
- **File:** `website/i18n.js:461-464`
|
||
- **Problem:** `el.innerHTML = t[key]` — not exploitable today (hardcoded dictionary) but latent; `?lang=` is user-controlled.
|
||
- **Fix:** Use `textContent` for all keys except an explicit whitelist of the 4 `data-i18n-html` keys that genuinely contain markup; never derive `t[key]` content from URL/user input.
|
||
|
||
### SEC-36 — sandbox not explicit · Low ⚠️ REVIEW
|
||
- **File:** `adiuvAI/src/main/index.ts:97-98` (webPreferences)
|
||
- **Problem:** `contextIsolation: true`, `nodeIntegration: false` are set (good), fuses are strong, but `sandbox` not enabled. Preload only uses `contextBridge`/`ipcRenderer`, so sandbox should be compatible.
|
||
- **Fix:** Add `sandbox: true` to webPreferences; full regression pass on packaged build (preload behavior changes under sandbox).
|
||
|
||
---
|
||
|
||
# 2. Correctness & Safety
|
||
|
||
### CORR-01 — Chat tool calls: no timeout + send-before-register race ⚠️ REVIEW
|
||
- **File:** `api/app/api/routes/device_ws.py:238-245` (`_executor`); contrast correct pattern `api/app/core/scout_runner.py:190-198`
|
||
- **Severity:** Critical
|
||
- **Problem:** (1) `await future` with no `asyncio.wait_for` — if Electron never replies (renderer crash, swallowed executor exception) the agent run hangs forever. (2) Frame is sent **before** `create_pending_call` registers the future; a fast `tool_result` arriving first no-ops on unknown id → permanent hang.
|
||
- **Fix:** Reorder: `future = device_manager.create_pending_call(user_id, payload["id"])` THEN `await websocket.send_text(...)`. Wrap: `return await asyncio.wait_for(future, timeout=30)` with `except asyncio.TimeoutError:` → pop the entry from `device_manager` `pending_calls` and return `{"error": "tool call timed out"}` so the LLM loop can continue. Also pop the dict entry on timeout in the scout path (`device_manager.py:145` leaks it).
|
||
- **Risk:** 30 s must exceed slowest legitimate executor op (large folder file reads) — confirm against `read_project_folder_file` worst case. Test: kill renderer mid-tool-call → agent run ends with error within 30 s.
|
||
|
||
### CORR-02 — WS unregister clobbers reconnected connection ⚠️ REVIEW
|
||
- **Files:** `api/app/api/routes/device_ws.py:133-145` · `api/app/core/device_manager.py:73-81`
|
||
- **Severity:** High
|
||
- **Problem:** Message loop ends silently on disconnect; heartbeat keeps the handler alive up to 30 s. If the client reconnects in that window, `register()` stores the new connection, then the OLD handler's `finally` calls `unregister(user_id)` — **popping the new healthy connection** and cancelling its pending futures. Also user looks online during the zombie window (scout runs dispatched into void).
|
||
- **Fix:** (1) `device_manager.unregister(self, user_id, ws)`: only pop when `self._connections.get(user_id)` and `self._connections[user_id].ws is ws`. Update both call sites. (2) End the heartbeat promptly: replace `asyncio.gather(loop, heartbeat)` with `done, pending = await asyncio.wait({...}, return_when=FIRST_COMPLETED)` then cancel pending.
|
||
- **Risk:** Core connection lifecycle — test reconnect-within-30s, normal disconnect, heartbeat failure (test_device_ws suite exists).
|
||
|
||
### CORR-03 — Free-tier extraction drains with empty content
|
||
- **File:** `api/app/core/memory_maintenance.py:120-154`
|
||
- **Severity:** High
|
||
- **Problem:** `run_extraction(..., last_user_msg="", last_assistant_msg="", ...)` — `row.episode_id` never used. Every hourly tick: one wasted gpt-4o-mini call per queue row over `"User: \nAssistant: "`, then row deleted. Free-tier memory extraction has never worked, and it costs money.
|
||
- **Fix:** For each row, load `MemoryEpisodic` by `row.episode_id` (+ `user_id`), decrypt `summary` with the user's Fernet (reuse `memory_middleware._get_fernet` or pass middleware instance), pass decrypted content as the extraction input. Group rows per user into a single call. Skip+delete rows whose episode is missing.
|
||
- **Risk:** Test asserting extracted candidates reference episode content (extend test_memory_extraction). Watch cost: now real extractions run.
|
||
|
||
### CORR-04 — Backend errors as success payloads ignored → chat wedged
|
||
- **Files:** `adiuvAI/src/main/router/index.ts:957-960` (returns `{response:'', error}`) · `adiuvAI/src/renderer/context/ContextualChatContext.tsx:160,210-240` · `adiuvAI/src/renderer/components/brief/TaskBriefChat.tsx:125,144` · `adiuvAI/src/renderer/components/ai/AIChatPanel.tsx:313-319`. Correct template: `useAIChat.ts:226-256`.
|
||
- **Severity:** High
|
||
- **Problem:** When offline/unauthenticated, no stream frames ever arrive; the three listed call sites never check `data.error` on mutation success → `isStreaming` stays true forever, `send()` no-ops, stream listeners leak (each leaked listener receives every future `ai:stream` frame).
|
||
- **Fix:** Preferred (cleaner, pre-1.0): make `aiRouter.chat`/brief/research procedures **throw TRPCError** instead of returning `{error}` (ipcLink already propagates errors, `ipcLink.ts:76`); then `onError` is the single error path — update `useAIChat` accordingly. Each renderer call site: in the error path, call the active unsubscribe, push an error bubble, reset `isStreaming`/`isResearching`. Add `useEffect` unmount cleanup for `TaskBriefChat`'s send listener (L144). Delete dead `hooks/useChatStream.ts` (DEAD-06).
|
||
- **Risk:** Touches every chat surface. Test: stop backend → send in contextual sidebar, task brief, home chat → error bubble shown, re-send works. Implement together with CORR-12 (shared error-frame contract).
|
||
|
||
### CORR-05 — Cascade-delete gaps; zero transactions ⚠️ REVIEW
|
||
- **Files:** `adiuvAI/src/main/router/index.ts` — `projects.delete` L211-220, `clients.deleteWithCascade` L119-151, `timelineEvents.delete` L711-723, `notes.delete` L835-840, `tasks.delete` L511-534 · `adiuvAI/src/main/api/drizzle-executor.ts` `handleDelete` L329-340
|
||
- **Severity:** High (data integrity)
|
||
- **Problem:** No FK constraints, hand-rolled deletes, no `db.transaction()` anywhere in src/main: project delete orphans timelineEvents/notes/projectFolderFiles/noteEdits; event delete leaves dangling dependency edges that **poison the cycle-guard BFS** (L756-772, falsely rejects new deps); AI-issued deletes (`handleDelete`) cascade nothing — orphan rows + attachment files on disk; `tasks.delete` interleaves file unlinks between row deletes (partial state on crash).
|
||
- **Fix:** Pre-1.0 clean option (preferred per Roberto's preference): add `references(..., { onDelete: 'cascade' })` to schema + enable `PRAGMA foreign_keys = ON` in `db/index.ts`, generate migration. Alternative: shared `cascadeDelete{Task,Project,Client,Note,TimelineEvent}` helpers in a new `src/main/db/cascade.ts`, used by BOTH tRPC routers and `handleDelete`, wrapped in `db.transaction()` (better-sqlite3 sync transactions), file unlinks after commit.
|
||
- **Risk:** FK route requires verifying every insert ordering satisfies constraints, and migration against existing user DBs with already-orphaned rows (clean orphans first). Test matrix: delete project with events+deps+notes+pending edits; agent-delete task with attachments → no orphan rows, files removed. Human review required — this deletes data.
|
||
|
||
### CORR-06 — WS drop wedges folder indexing in 'scanning'
|
||
- **Files:** `adiuvAI/src/main/api/backend-client.ts:1099-1100` (listeners cleared, `onDone` never called) · `adiuvAI/src/main/files/indexer.ts:106-117` (only `finalize` resets status) · consumers refusing rescan: `drizzle-executor.ts:486`, `files/daily-rescan.ts:22`, `router/projectFolders.ts:61`
|
||
- **Severity:** High
|
||
- **Problem:** WS close clears `indexListeners` without invoking callbacks → `folderLastScanStatus` stays `'scanning'` forever → all future scans refuse to run. Recovery currently requires manual DB surgery.
|
||
- **Fix:** (1) In the WS close handler, before `indexListeners.clear()`: iterate and call each `onDone('error')`. (2) Startup recovery in `initDb()` or app-ready: `UPDATE projects SET folder_last_scan_status='error' WHERE folder_last_scan_status='scanning'`.
|
||
- **Risk:** None meaningful. Test: kill backend mid-index → status becomes 'error', rescan works.
|
||
|
||
### CORR-07 — backend-client close handler clobbers new socket
|
||
- **File:** `adiuvAI/src/main/api/backend-client.ts:1079-1104`
|
||
- **Severity:** Medium-High
|
||
- **Problem:** `ws.on('close')` unconditionally nulls `persistentWs`, stops heartbeat, rejects+clears ALL listener maps. Logout→login: the old socket's deferred close event destroys the new connection's state and schedules a duplicate reconnect.
|
||
- **Fix:** Capture `ws` in closure; first line of the close and error handlers: `if (this.persistentWs !== ws) return;`. Same guard inside the heartbeat interval callback.
|
||
- **Risk:** Low. Test logout+immediate login → exactly one socket, in-flight chat on new socket survives.
|
||
|
||
### CORR-08 — Decay applied per invocation, not per period
|
||
- **File:** `api/app/core/memory_maintenance.py:280-309` (proactive, runs hourly), `:61-104` (relations)
|
||
- **Severity:** Medium
|
||
- **Problem:** `periods = days_elapsed // PERIOD; new_confidence = confidence * FACTOR**periods` recomputed from immutable `created_at` and re-applied every tick — a row ≥8 days old loses 10% **per hour** (intended: per 7 days).
|
||
- **Fix:** Compute confidence as a pure function of age each read, OR add `last_decayed_at` column (migration) and decay only `(now - last_decayed_at) // PERIOD` periods, updating the column. Pure-function approach avoids the migration — preferred.
|
||
- **Risk:** Recompute approach changes stored semantics — confirm nothing else mutates `confidence` (confirmation boosts do; then `last_decayed_at` is the correct route). ⚠️ REVIEW the choice.
|
||
|
||
### CORR-09 — Check-then-insert races, missing unique constraints ⚠️ REVIEW
|
||
- **Files:** `api/app/core/memory_middleware.py:232-247` (`update_core`), `:421-450` (`upsert_relation`); `api/app/api/routes/auth.py:125-127` (register IntegrityError→500)
|
||
- **Severity:** Medium
|
||
- **Problem:** No `UNIQUE(user_id, key)` on memory_core, none on memory_relations `(user_id, subject_label, predicate, object_label)`. Concurrent writers (documented fire-and-forget language seeding + onboarding PUT) create duplicates; `scalar_one_or_none()` then raises `MultipleResultsFound`. Register duplicate-email race returns 500 instead of 409.
|
||
- **Fix:** Alembic migration adding both unique constraints (dedupe existing rows first: keep newest per key). Convert both writers to PostgreSQL `INSERT ... ON CONFLICT DO UPDATE` (copy the savepoint pattern from `scouts/engine.py:123-135`). Register: catch `IntegrityError` → 409.
|
||
- **Risk:** Migration fails if production data has duplicates — dedupe step mandatory. Test concurrent update_core calls.
|
||
|
||
### CORR-10 — Fire-and-forget create_task: no refs, no concurrency cap
|
||
- **Files:** `api/app/api/routes/device_ws.py:120,130,171-215`; also `scouts.py:247-249`, `memory_middleware.py:201`, `note_agent.py:84,112`
|
||
- **Severity:** Medium
|
||
- **Problem:** Bare `asyncio.create_task` — loop holds weak refs (GC can collect mid-flight); exceptions surface only at GC; a frame-spamming client spawns unbounded concurrent agent runs (WS path has no rate limit).
|
||
- **Fix:** Per-connection `tasks: set[asyncio.Task]`; on spawn: `t = create_task(...); tasks.add(t); t.add_done_callback(tasks.discard)`. In WS `finally`: cancel all. Cap: if ≥3 in-flight stream-producing requests for the connection, reply immediately with `stream_end` error frame "busy".
|
||
- **Risk:** Cap value is judgment — confirm 3 is right for legitimate parallel use (home + contextual + brief). ⚠️ REVIEW the cap.
|
||
|
||
### CORR-11 — tasks.update wipes briefing + chat history on every update
|
||
- **File:** `adiuvAI/src/main/router/index.ts:504-506`; hash helper exists at `:55-64`; staleness check at `:976-989`
|
||
- **Severity:** Medium (silent user-data loss)
|
||
- **Problem:** Every update — including a kanban status drag — deletes `taskBriefings` + `taskBriefChats`.
|
||
- **Fix:** Load the task before update; compute `hashTaskForBriefing(prev)` vs hash of merged next state; run the two deletes only when hashes differ.
|
||
- **Risk:** Verify `hashTaskForBriefing` covers exactly the fields the briefing depends on. Test: status drag preserves brief chat; title edit invalidates it.
|
||
|
||
### CORR-12 — Stream errors emitted as plain stream_end
|
||
- **File:** `adiuvAI/src/main/ai/orchestrator.ts:105,111,141,147,192,198`; renderer persistence of empty message `ContextualChatContext.tsx:187-204`
|
||
- **Severity:** Medium
|
||
- **Problem:** `onError` sends a normal `stream_end` (no error flag); catch blocks send `stream_end` with `requestId: ''` (matches nothing). Mid-stream failure looks like success → blank assistant bubble **persisted to SQLite**.
|
||
- **Fix:** Extend the stream-end frame with optional `error?: string` in `src/shared` types (mirror backend's existing error-capable `WsStreamEnd`). orchestrator: pass real requestId + error message; delete the `requestId: ''` sends. Renderers: on `error`, show error bubble, skip `appendMessage` persistence when content empty. Backend twin fix: CORR-15.
|
||
- **Risk:** Frame contract change across main↔renderer — do together with CORR-04.
|
||
|
||
### CORR-13 — withRetry retries non-idempotent POSTs ⚠️ REVIEW
|
||
- **File:** `adiuvAI/src/main/api/backend-client.ts:700-723`
|
||
- **Severity:** Medium
|
||
- **Problem:** `proxyPost` retried up to 3× on timeout/5xx/429 — a timed-out-but-processed `/scouts/trigger` or scout create duplicates server work; immediate retry on 429 worsens it.
|
||
- **Fix:** Add `{ retry?: boolean }` option to `proxyPost` defaulting false; enable only for verified-idempotent endpoints (reads proxied via POST, if any). Exclude `RateLimitError` from retry entirely (or respect Retry-After). GET/DELETE-by-id keep retry.
|
||
- **Risk:** Requires classifying every proxyPost call site idempotent-or-not — review the list.
|
||
|
||
### CORR-14 — Sync Stripe calls block the event loop
|
||
- **File:** `api/app/billing/stripe_service.py:71-79, 197, 225-238` (auto_paging_iter paginates network synchronously)
|
||
- **Severity:** Medium
|
||
- **Fix:** Wrap each Stripe call in `await asyncio.to_thread(...)`; for `auto_paging_iter`, collect inside the thread: `await asyncio.to_thread(lambda: list(itertools.islice(it, 100)))`.
|
||
- **Risk:** None behavioral. Mechanical once pattern set.
|
||
|
||
### CORR-15 — run_home_stream swallows exceptions; no error stream_end
|
||
- **File:** `api/app/api/routes/device_ws.py:295-307`; correct pattern at `:498-505` (brief)
|
||
- **Severity:** Medium
|
||
- **Problem:** On exception: logged, partial chunks stored as an episode, client never receives `stream_end` with error → renderer hangs (pairs with CORR-04).
|
||
- **Fix:** In the except block: send `WsStreamEnd(request_id=..., error="Internal error")` (generic per SEC-30); skip `store_episode` when no response text was produced.
|
||
- **Risk:** None. Test: raise inside agent → client gets error frame.
|
||
|
||
### CORR-16 — delete_account: partial transaction + broad except ⚠️ REVIEW
|
||
- **File:** `api/app/api/routes/auth.py:759-795`
|
||
- **Severity:** Medium
|
||
- **Problem:** Stripe cancel commits its own transaction, then memory deletes, then user delete + final commit; failure mid-way = cancelled subscription with live account. `except Exception: pass` at L786-787 hides genuine failures. (DB-level `ondelete="CASCADE"` makes the manual memory deletes redundant.)
|
||
- **Fix:** Reorder: all DB deletes in one transaction first (drop the redundant manual memory-table deletes — FK cascade covers them; verify each memory table's FK has `ondelete="CASCADE"` in migrations before relying on it), commit, THEN Stripe cancel; on Stripe failure log + enqueue retry rather than failing the deletion. Narrow the except to expected types.
|
||
- **Risk:** Account deletion is irreversible — human review + test with seeded data.
|
||
|
||
### CORR-17 — Index-session TOCTOU + stale _active entry
|
||
- **Files:** `adiuvAI/src/main/files/indexer.ts:67` (`startIndexSession`) · `adiuvAI/src/main/router/projectFolders.ts:84-99` · trigger sites `drizzle-executor.ts:487-501`, `files/daily-rescan.ts:22`
|
||
- **Severity:** Medium
|
||
- **Problem:** Three concurrent triggers can all pass the `!== 'scanning'` check before any writes 'scanning'. `_active.set` runs after `await startIndexSession` returns — a zero-delta session completes before the entry exists, leaving stale `{status:'starting'}`.
|
||
- **Fix:** Module-level `const _inFlight = new Set<string>()` in indexer.ts; first synchronous statement of `startIndexSession`: `if (_inFlight.has(projectId)) throw new Error('Scan already in progress'); _inFlight.add(projectId)`; remove in `finalize`'s finally. In projectFolders.ts, register the `_active` entry before awaiting.
|
||
- **Risk:** Low. Also add `.catch(console.error)` to the `void startIndexSession(...)` fire-and-forget sites (daily-rescan.ts:25, drizzle-executor.ts:497).
|
||
|
||
### CORR-18 — Scout trigger TOCTOU · Low
|
||
- **File:** `api/app/api/routes/scouts.py:225-249` (+ `scout_runner.py:577`)
|
||
- **Fix:** Add `stable_agent_id` to `_running_agents` synchronously in the route before `db.commit()`/`create_task`; discard on spawn failure. Keep the in-task add idempotent.
|
||
|
||
### CORR-19 — Swallowed-error triage · Low
|
||
- **Files/fixes (mechanical batch):**
|
||
- `adiuvAI/src/main/router/index.ts:1156-1164, 1276-1294, 1374-1382, 1429-1434` — scout list/labels/catalog/runs return `[]` on any error → return `{ data, error }` envelope (or throw, per QUAL-11's chosen convention).
|
||
- `adiuvAI/src/main/api/drizzle-executor.ts:566-568` — include `error: message` in the `kind:'error'` result.
|
||
- `adiuvAI/src/main/auth/auth-manager.ts:594` — wrap `JSON.parse(text)` in try/catch, return `{}` on failure.
|
||
- Documented-intentional `.catch(() => {})` seeding (router/index.ts:1577,1585) — leave as is.
|
||
|
||
### CORR-20 — Naive/aware datetime kludge · Low
|
||
- **File:** `api/app/api/routes/auth.py:205` — unconditional `.replace(tzinfo=timezone.utc)` would mislabel a non-UTC value. **Fix:** `if rt.expires_at.tzinfo is None: rt_exp = rt.expires_at.replace(tzinfo=timezone.utc) else: rt_exp = rt.expires_at` (pattern from `scout_runner.py:166-167`).
|
||
|
||
### CORR-21 — Renderer/main misc · Low
|
||
- `adiuvAI/src/renderer/index.tsx:22-30` — move `LanguageSync` component definition to module scope (currently declared inside `App`, remount trap).
|
||
- `adiuvAI/src/main/ipc.ts:68` — `undefined as unknown as AbortSignal`: make the field optional in the local type (see TYPE-05 family).
|
||
|
||
---
|
||
|
||
# 3. Performance
|
||
|
||
### PERF-01 — bcrypt on the event loop
|
||
- **File:** `api/app/api/routes/auth.py:69-74`, called at `:134, :169, :656-659`
|
||
- **Severity:** High
|
||
- **Problem:** 100-300 ms CPU per call freezes ALL WS streams/tool round-trips; login is currently rate-limit-exempt (SEC-02) → credential-stuffing stalls the whole server.
|
||
- **Fix:** `return await asyncio.to_thread(bcrypt.hashpw, password.encode(), bcrypt.gensalt())` etc. — make `_hash_password`/`_verify_password` async, update 3 call sites.
|
||
- **Risk:** None. Verify tests still pass (auth suite exists).
|
||
|
||
### PERF-02 — Home channel buffers all tokens (fake streaming) ⚠️ REVIEW
|
||
- **File:** `api/app/core/deep_agent.py:1219-1227` (token withholding), `:1075` (`ainvoke` not `astream`)
|
||
- **Severity:** High (UX latency)
|
||
- **Problem:** `run_home_stream` accumulates every token and yields ONE blob after the whole run (including tool loops). User sees nothing until the end. Root cause: `_normalize_tagged_list_lines` needs the full text.
|
||
- **Fix:** Switch the LLM call to `llm_with_tools.astream()` accumulating tool-call deltas; make `_normalize_tagged_list_lines` line-buffered (process and flush complete lines as they arrive, hold only the current partial line). Measure time-to-first-frame before/after.
|
||
- **Risk:** Tool-call delta accumulation across LangChain/LiteLLM is fiddly; the comment near L1080-1100 documents a past double-LLM-call bug that must not regress. Strong-model territory.
|
||
|
||
### PERF-03 — Hourly mining duplicates + unbounded prompt injection
|
||
- **File:** `api/app/core/memory_maintenance.py:175-253`; `api/app/core/memory_middleware.py:703-718` (`_load_proactive`, no LIMIT); cron at `api/app/main.py:46-77`
|
||
- **Severity:** High (cost + correctness)
|
||
- **Problem:** Hourly re-mining of the same 30-day window, 3-5 new rows per tick, no dedup → ~100 near-duplicate rows/user/day, all decrypted into every chat prompt.
|
||
- **Fix:** (1) `_load_proactive`: add `.order_by(confidence.desc()).limit(20)`. (2) Mine only when episodes newer than the user's latest `MemoryProactive.created_at` exist. (3) Dedup new patterns against existing (reuse `decide_action`). (4) Consider daily cadence for the mining portion of the cron.
|
||
- **Risk:** (1) is mechanical and urgent; (2)-(4) need the memory test suites re-run.
|
||
|
||
### PERF-04 — No SQLite indexes (Electron)
|
||
- **File:** `adiuvAI/src/main/db/schema.ts` (only ai_chat tables indexed, migration 0006)
|
||
- **Severity:** Medium (High at scale — scans are synchronous, blocking all IPC)
|
||
- **Fix:** Add Drizzle `index()` on: `tasks.projectId`, `tasks.dueDate`, `taskComments.taskId`, `taskAttachments.taskId`, `notes.projectId`, `noteEdits.noteId`, `timelineEvents.projectId`, `timelineEventDependencies.fromEventId`+`.toEventId`, `scoutRunActions.runId`; `uniqueIndex` on `projectFolderFiles(projectId, relativePath)` (indexer does a per-file SELECT by that pair — `files/indexer.ts:128-136`, O(n²)). `npx drizzle-kit generate`.
|
||
- **Risk:** Migration must apply cleanly on existing user DBs (bootstrapMigrationsLedger path) — test against a populated dev DB.
|
||
|
||
### PERF-05 — Missing PG indexes; refresh_tokens never purged
|
||
- **File:** `api/app/models.py` (+ alembic migration needed)
|
||
- **Severity:** Medium
|
||
- **Fix:** One Alembic migration adding: `memory_core(user_id, key)` (unique — shared with CORR-09), `memory_episodic(user_id, created_at DESC)`, `memory_relations(user_id, subject_label, predicate, object_label)` (unique — CORR-09), `scout_run_logs(user_id, started_at)`, HNSW index on `memory_associative.embedding` (`USING hnsw (embedding vector_cosine_ops)`), `refresh_tokens(expires_at)`. Add a daily cleanup to the existing cron: `DELETE FROM refresh_tokens WHERE expires_at < now()`.
|
||
- **Risk:** HNSW build time on large tables; coordinate with CORR-09 dedupe.
|
||
|
||
### PERF-06 — Per-token IPC + full markdown re-parse
|
||
- **Files:** `adiuvAI/src/main/ai/orchestrator.ts:103,139,180` · `adiuvAI/src/renderer/components/ai/ChatSurface.tsx:197-199,352-358` · `useAIChat.ts:185-188`, `ContextualChatContext.tsx:183-185`, `TaskBriefChat.tsx:98-101`
|
||
- **Severity:** Medium
|
||
- **Problem:** token → IPC message → setState → full ReactMarkdown re-parse of the accumulated text + smooth scroll, per token. Quadratic on long answers.
|
||
- **Fix:** Coalesce in main: buffer chunks per requestId, flush on 40 ms timer (and on stream_end). Renderer: render the in-flight message as plain text (whitespace-pre-wrap), run ReactMarkdown only on `stream_end`; throttle autoscroll with requestAnimationFrame.
|
||
- **Risk:** Ensure stream_end flushes the tail buffer; visual check that markdown "pops in" acceptably at end.
|
||
|
||
### PERF-07 — Serial tool calls per LLM step
|
||
- **File:** `api/app/core/deep_agent.py:953-986, 1103-1137`; `api/app/core/scout_runner.py:289-312`
|
||
- **Severity:** Medium
|
||
- **Fix:** `results = await asyncio.gather(*(t.ainvoke(args) for ...))`, then append `ToolMessage`s preserving the original tool_call id order. Apply in all three loops (or once, after QUAL-03's shared loop).
|
||
- **Risk:** Concurrent tool calls hit the Electron executor concurrently — drizzle-executor handlers are async over a sync driver, fine, but verify pending-call map handles parallel ids (it's keyed by id — fine). Order preservation matters for the LLM.
|
||
|
||
### PERF-08 — Blocking Langfuse flush/get_prompt
|
||
- **Files:** `api/app/core/deep_agent.py:1009-1010,1165-1166` · `scout_runner.py:323-324` · `memory_extraction.py:319-322` · `langfuse_client.py:99`
|
||
- **Severity:** Medium
|
||
- **Fix:** Delete the per-request `lf.flush()` calls (SDK background-flushes; keep one flush in app shutdown lifespan). Wrap `lf.get_prompt(...)` in `asyncio.to_thread` at the call sites in langfuse_client, or pre-warm the prompt cache at startup.
|
||
- **Risk:** Traces may lag a few seconds in Langfuse UI — acceptable.
|
||
|
||
### PERF-09 — get_current_user does 3 queries + full memory decrypt per request
|
||
- **File:** `api/app/api/middleware/auth.py:62-91`
|
||
- **Severity:** Medium
|
||
- **Fix:** Split dependencies: `get_current_user` (user+tier in ONE query via outerjoin on subscriptions, no memory) and `get_current_user_with_memory` (adds core-block decrypt). Switch only the routes that actually read `.memory` (chat/profile) to the latter; scout CRUD, billing, etc. use the light one.
|
||
- **Risk:** Grep every `current_user.memory` access before switching routes. Test suite covers auth paths.
|
||
|
||
### PERF-10 — Redundant memory-middleware queries; per-key commits
|
||
- **Files:** `api/app/core/memory_middleware.py` (`_get_fernet`/`_get_user_debug` per method) · `api/app/api/routes/auth.py:560-575` (`update_memory` loop)
|
||
- **Severity:** Medium
|
||
- **Fix:** Cache `(user, fernet, tier)` on the middleware instance per request (it's constructed per call — add lazy `_ctx` loaded once). `update_memory`: add a `update_core_many(dict)` method doing one SELECT of existing keys, one bulk write, one commit.
|
||
- **Risk:** Instance lifetime check: confirm MemoryMiddleware isn't a long-lived singleton (it takes `db` per call — if constructed per request, caching is safe).
|
||
|
||
### PERF-11 — Health check before every chat
|
||
- **File:** `adiuvAI/src/main/ai/orchestrator.ts:76-87`
|
||
- **Fix:** In `checkConnectivity`, return early-success when `backendClient.persistentWs?.readyState === OPEN` (expose a `isWsConnected()` getter); keep the HTTP health check only as fallback when WS down.
|
||
- **Risk:** None — `sendHomeRequest` already rejects with OfflineError when WS is down.
|
||
|
||
### PERF-12 — Paid brief regen per mutation
|
||
- **File:** `adiuvAI/src/main/router/index.ts:447,502,532,668,706,721` → `orchestrator.ts:240-254` (1.5 s debounce)
|
||
- **Severity:** Medium (LLM cost)
|
||
- **Fix:** Replace eager regeneration with dirty-flag: `invalidateBriefCache()` only marks the cache stale; regeneration happens lazily on next `dailyBrief` read or the existing scheduler slot tick. Delete `scheduleBriefRegeneration`.
|
||
- **Risk:** First home-page visit after edits pays the generation latency — show the stale brief while regenerating (cache already stores it).
|
||
|
||
### PERF-13 — No code-splitting; heavy eager imports
|
||
- **Files:** `adiuvAI/vite.renderer.config.mts:13-16`, `vite.web.config.mts:22` · `ChatChartBlock.tsx:20` (recharts) · `routes/notes.$noteId.tsx:27` (Milkdown)
|
||
- **Fix:** Add `autoCodeSplitting: true` to TanStackRouterVite in both configs. `React.lazy` + Suspense for `ChatChartBlock` (renders only on `<chart>` tag) and `MilkdownEditor`.
|
||
- **Risk:** Test chart block + notes editor render after splitting (Suspense fallback flash).
|
||
|
||
### PERF-14 — Default QueryClient → focus refetch storms (mechanical)
|
||
- **File:** `adiuvAI/src/renderer/index.tsx:15` (contrast `web-main.tsx:24`)
|
||
- **Fix:** `new QueryClient({ defaultOptions: { queries: { staleTime: 30_000, refetchOnWindowFocus: false } } })` — local writes already invalidate explicitly.
|
||
- **Risk:** Multi-window staleness (single-window app — fine).
|
||
|
||
### PERF-15 — Double folder walk; no WS backpressure
|
||
- **Files:** `adiuvAI/src/main/router/projectFolders.ts:65` + `files/indexer.ts:67,191-215`
|
||
- **Fix:** Pass the pre-flight `ScanDelta` into `startIndexSession` (skip re-walk). In the batch loop, check `ws.bufferedAmount` and await drain (or gate on `onProgress` acks) before sending the next batch.
|
||
- **Risk:** Backpressure logic needs a stall timeout; test with a large folder.
|
||
|
||
### PERF-16 — DB sessions across LLM calls
|
||
- **Files:** `api/app/scouts/engine.py:48-70` · `api/app/api/routes/device_ws.py:736-829`; pool config `api/app/db.py:24-28`
|
||
- **Fix:** Restructure to short sessions: load needed rows → close → LLM work → new session for writes. Interim: raise `pool_size`/`max_overflow` and set explicit `pool_timeout`.
|
||
- **Risk:** Detached-instance errors after session close — copy needed attrs to plain objects first.
|
||
|
||
### PERF-17 — Per-file metadata WS round-trips
|
||
- **File:** `api/app/core/scout_runner.py:386-405`
|
||
- **Fix:** Extend the Electron `list_directory` executor result to include `mtimeMs`/`size` per entry (same `fs.stat` it already does for dirents — `drizzle-executor.ts:375-398`); backend reads mtimes from the listing, drops per-file `get_file_metadata` calls.
|
||
- **Risk:** Two-repo protocol change; version-skew tolerance (backend falls back to old path if field absent).
|
||
|
||
### PERF-18 — Per-call AsyncOpenAI clients (mechanical)
|
||
- **Files:** `api/app/core/embeddings.py:23` · `api/app/core/llm.py:154`
|
||
- **Fix:** Module-level `_client: AsyncOpenAI | None` lazy singleton; reuse across calls.
|
||
|
||
### PERF-19 — tasks.list client-side pagination · Low
|
||
- **Files:** `adiuvAI/src/main/router/index.ts:314-317` · `TaskListView.tsx:80,101`
|
||
- **Fix:** Pass `limit/offset` from TaskPager state; add `statusIn` filter server-side for the `active` pseudo-filter (L94-95). Defer until data sizes warrant.
|
||
|
||
### PERF-20 — Limiter dict growth; tier from JWT · Low
|
||
- **File:** `api/app/api/middleware/rate_limit.py:81,97-98,107-128`
|
||
- **Fix:** Delete empty lists after trim; superseded entirely if SEC-17's Redis lands. Tier-from-JWT staleness: document or read from DB (cheap once PERF-09's joined query exists).
|
||
|
||
### PERF-21 — Unbounded reads in drizzle-executor · Low
|
||
- **File:** `adiuvAI/src/main/api/drizzle-executor.ts:623-630` (page details), `:535-548` (pdf/docx/image base64 uncapped)
|
||
- **Fix:** Apply default limit 50 (as `handleSelect` does) to the `_all` page-detail queries; enforce the 500 KB cap on binary reads too (reject larger with explicit error).
|
||
|
||
### PERF-22 — Context re-render hotspots · Low
|
||
- **Files:** `adiuvAI/src/renderer/context/HeaderContext.tsx:30` (unmemoized value) · `ContextualChatContext.tsx:253-270` + `hooks/useContextualScope.ts:5`
|
||
- **Problem:** Every route re-renders per streamed sidebar token (scope consumers subscribe to the same context as `streamingContent`).
|
||
- **Fix:** `useMemo` the HeaderContext value. Split ContextualChatContext into stable-actions context (`setScope`, `open`, `toggle`) and volatile stream context (`messages`, `streamingContent`, `isStreaming`); `useContextualScope` consumes only the stable one.
|
||
|
||
---
|
||
|
||
# 4. Dead Code
|
||
|
||
### DEAD-01 — Five dead Python deps · High (mechanical)
|
||
- **File:** `api/requirements.txt` lines 12, 25, 26, 27, 31
|
||
- **Problem:** `boto3`, `moto[s3]`, `pinecone`, `qdrant-client`, `google-auth-oauthlib` — zero imports/refs anywhere (grep-verified; pgvector is the live vector path; `oauth_providers.py:71` explicitly states it does NOT use google-auth-oauthlib).
|
||
- **Fix:** Delete those 5 lines. Run `pytest` after. Keep `python-dotenv` (pydantic-settings `env_file` needs it) and `google-auth-httplib2` (transitive need of googleapiclient).
|
||
|
||
### DEAD-02 — Orphaned scout_registry.py · High (mechanical)
|
||
- **File:** `api/app/core/scout_registry.py` — `BaseAgent`, zero importers, zero subclasses, references deleted vector-store era. **Fix:** delete file; run pytest.
|
||
|
||
### DEAD-03 — slowapi dead · Medium (mechanical)
|
||
- **Files:** `api/app/api/middleware/rate_limit.py:26-28,52,66-67` · `middleware/__init__.py:10,17` · `requirements.txt:13`
|
||
- **Problem:** `Limiter` exported "for optional route-level decoration" — zero `@limiter` decorated routes exist.
|
||
- **Fix:** Remove the Limiter creation/exports and the requirement. **Sequencing:** do AFTER SEC-02 (which may or may not reuse slowapi — recommended fix reuses the custom window, so slowapi still goes).
|
||
|
||
### DEAD-04 — chat.py HTTP routes production-unused ⚠️ REVIEW (owner decision)
|
||
- **Files:** `api/app/api/routes/chat.py:40,63,105`; stale comments `adiuvAI/src/main/api/backend-client.ts:9,14`
|
||
- **Problem:** Electron client hits none of `POST /chat`, `/chat/brief`, `/chat/embed` (all traffic on `WS /api/v1/ws/device`). Routes are test-covered but production-dead — unless an external consumer exists.
|
||
- **Fix:** Roberto confirms no external consumers → delete routes + their tests (`test_brief_agent.py`/`test_middleware.py` portions that exercise them — keep middleware tests by retargeting another POST route). Either way, fix the stale doc comments in backend-client.ts (mentions nonexistent `WS /api/v1/chat/stream`) and `adiuvAI/src/shared/api-types.ts:338,390,425` (references nonexistent `/agents/*` HTTP routes). Note CLAUDE.md path drift: actual WS route is `/api/v1/ws/device`, docs say `/api/v1/device`.
|
||
|
||
### DEAD-05 — Four unused npm deps · Medium (mechanical)
|
||
- **File:** `adiuvAI/package.json:56,76,77,78`
|
||
- **Fix:** `npm uninstall next-themes mammoth pdf-parse @hello-pangea/dnd` (knip + grep verified zero imports; mammoth/pdf-parse are LanceDB-era leftovers).
|
||
|
||
### DEAD-06 — Dead Electron modules · Medium
|
||
- **Files (knip-verified, delete):** `adiuvAI/src/shared/batch-types.ts` (~5 KB), `src/renderer/hooks/useChatStream.ts` (has the CORR-04 bug — delete, don't fix), `src/renderer/hooks/useTaskBriefCache.ts`, `src/renderer/components/agents/ScoutRunLog.tsx`, `src/renderer/components/ai/blocks/index.tsx` (dead barrel).
|
||
- **⚠️ REVIEW one:** `blocks/ChatTableBlock.tsx` is dead only because the barrel is — check `api/app/core/output_formatter.py` first: if the backend emits table segments, wire ChatTableBlock into ChatSurface's segment switch instead of deleting (missing feature, not dead code).
|
||
|
||
### DEAD-07 — backup-key.ts unwired ⚠️ REVIEW (owner decision)
|
||
- **File:** `adiuvAI/src/main/auth/backup-key.ts` — zero importers, but CLAUDE.md documents it as deliberate architecture. **Fix:** Roberto decides: wire into the backup feature roadmap or delete (recoverable from git). Interacts with SEC-21 (its storage fallback) and SEC-28 (could encrypt attachments).
|
||
|
||
### DEAD-08 — knip.json missing web entries · Medium (mechanical)
|
||
- **File:** `adiuvAI/knip.json`
|
||
- **Problem:** `web-main.tsx` + `lib/httpLink.ts` false-flagged (live via `web.html:11`).
|
||
- **Fix:** Add `"web.html"` and `"src/renderer/web-main.tsx"` to the `entry` array; optionally add ignore rule for TanStack `Route` exports. Do this FIRST so subsequent knip runs are trustworthy.
|
||
|
||
### DEAD-09 — langchain meta-package; websockets pin · Medium (mechanical)
|
||
- **File:** `api/requirements.txt:4,20`
|
||
- **Fix:** Replace `langchain>=0.3.0` with `langchain-core>=0.3.0` (only `langchain_core`/`langchain_openai`/`langchain_litellm` are imported). Drop explicit `websockets` pin (uvicorn[standard] bundles it). Run pytest.
|
||
|
||
### DEAD-10 — Ruff F401 batch · Low (mechanical)
|
||
- **Fix:** `cd api && ruff check . --select F401 --fix` (13 findings: quota.py:7, deep_agent.py:1247, 11 test files). Keep the ERA001-flagged comment blocks (scouts.py:438-457, email_html.py:22 — documentation, add `# noqa: ERA001` if noisy).
|
||
|
||
### DEAD-11 — Dead TS exports · Low (mechanical)
|
||
- **Fix:** Remove `export` (or delete bodies): `getDbPath`/`getRawSqlite`/`closeDb` (`src/main/db/index.ts:118-137`), `generateAndCacheBrief` (`orchestrator.ts:257` — superseded if PERF-12 lands; coordinate), `attachmentsRoot` (`attachments/storage.ts:17`), `parseMutationsToEntityTags` (`useAIChat.ts:76`), `formatTime` (`lib/date.ts:72`), `parseDateRange` (`lib/parseDate.ts:151`). Keep error classes exported (instanceof checks); keep `webPlatform` until DEAD-08 confirms.
|
||
|
||
### DEAD-12 — Dev deps in runtime requirements · Low (mechanical)
|
||
- **File:** `api/requirements.txt:22-25,41`
|
||
- **Fix:** Move `pytest`, `pytest-asyncio`, `aiosqlite`, `ruff` to a new `requirements-dev.txt`; update Dockerfile to install only runtime file; CI installs both.
|
||
|
||
---
|
||
|
||
# 5. Dependencies
|
||
|
||
### DEPS-01 — ws 8.19.0 vulnerable · High (mechanical)
|
||
- **File:** `adiuvAI/package.json` — GHSA-58qx-3vcg-4xpx (uninitialized memory disclosure, affects ≤8.20.0). Only runtime-reachable vuln of the 58 npm audit hits. **Fix:** `npm audit fix` (→ ^8.21.0, semver-compatible).
|
||
|
||
### DEPS-02 — python-jose floor permits CVE versions · High (mechanical)
|
||
- **File:** `api/requirements.txt:10` — floor 3.3.0 permits CVE-2024-33663 (algorithm confusion) / CVE-2024-33664 (JWE DoS), fixed in 3.4.0. App pins HS256 with explicit `algorithms=[...]` everywhere (mitigates the confusion path), but raise anyway. **Fix:** `python-jose[cryptography]>=3.4.0`. **⚠️ REVIEW (later):** consider migrating to PyJWT (python-jose barely maintained) — separate ticket.
|
||
|
||
### DEPS-03 — cryptography floor · Medium (mechanical)
|
||
- **File:** `api/requirements.txt:34` — floor 42.0.0 permits CVE-2024-26130 versions. **Fix:** `cryptography>=43.0.1`.
|
||
|
||
### DEPS-04 — No Python lock file · Medium
|
||
- **Problem:** All `>=` floors; installed versions unauditable. **Fix:** Adopt `uv lock` or `pip-compile`; commit the lock; run `pip-audit` against it in CI. ⚠️ REVIEW tool choice.
|
||
|
||
### DEPS-05 — eslint 8 + @typescript-eslint 5 EOL ⚠️ REVIEW
|
||
- **File:** `adiuvAI/package.json:38-39` — ts-eslint v5 predates TS 5.x support; lint coverage degraded. **Fix:** Dedicated PR: eslint 9/10 flat-config migration + typescript-eslint 8. Medium effort (config rewrite).
|
||
|
||
### DEPS-06 — Electron 40 → 42 ⚠️ REVIEW
|
||
- **Fix:** Dedicated PR bumping electron; rebuild/retest better-sqlite3 (forge cross-compilation hooks download platform binaries — verify versions), full smoke of packaged app.
|
||
|
||
### DEPS-07 — Transitive toolchain advisories · Low (no action)
|
||
- `tmp`/`tar` via @electron-forge 7.x, `esbuild` via drizzle-kit — build-machine-only, **no fix available**. Do NOT run `npm audit fix --force` (downgrades forge/drizzle-kit). Monitor forge releases.
|
||
|
||
### DEPS-08 — @types/ws placement · Low (mechanical)
|
||
- **File:** `adiuvAI/package.json:65` — move to devDependencies.
|
||
|
||
---
|
||
|
||
# 6. Code Quality
|
||
|
||
> All QUAL items execute in **Phase 6**, after behavior fixes, because they move code and invalidate line references. Items marked ⚠️ involve behavior-preservation judgment.
|
||
|
||
### QUAL-01 — Split router/index.ts (1967 LOC) · High
|
||
- **Fix:** Split along existing boundaries into `src/main/router/{trpc.ts,clients.ts,projects.ts,tasks.ts,timeline.ts,notes.ts,ai.ts,scout.ts,auth.ts,memory.ts,settings.ts,attachments.ts}`; boundaries: clients 71-152, projects 154-228, tasks 230-560, timelineEvents 561-725, deps 726-787, notes 788-842, taskComments 843-877, settings 878-922, ai 923-1090, scout* 1091-1516, auth 1517-1729, memory 1730-1784, noteEdits 1785-1867, taskAttachments 1868-1946. `trpc.ts` holds the `t`/`router`/`publicProcedure` factory; `index.ts` becomes the `appRouter` merge + `AppRouter` export. Move brief helpers (L25-65) to `src/main/ai/brief-cache.ts` (with PERF-12).
|
||
- **Risk:** Low — routers are independent consts. Verify `AppRouter` type output unchanged (renderer compiles).
|
||
|
||
### QUAL-02 — Merge _run_single_agent twins ⚠️ REVIEW · High
|
||
- **File:** `api/app/core/deep_agent.py:873-1010` vs `:1013-1166` (~90% identical)
|
||
- **Fix:** Keep only the streaming generator; non-stream variant = join the tokens. **Preserve** the documented behavior at L1080-1100 (stream version intentionally avoids a second LLM call — past bug) as the single canonical path.
|
||
- **Risk:** Every chat path runs through this. Run full agent test suites; manual chat smoke.
|
||
|
||
### QUAL-03 — One LLM tool loop, four implementations ⚠️ REVIEW · High
|
||
- **Files:** `deep_agent.py:873,1013` · `scout_runner.py:222-324` · `scout_setup.py:237-349`
|
||
- **Fix:** New `api/app/core/tool_loop.py`: `async def run_tool_loop(llm, tools, messages, *, max_steps, lf_name, on_token=None)` yielding events. Four call sites become configuration. Use `contextlib.ExitStack` for Langfuse observations (kills the manual `__enter__`/`__exit__`). Parametrize observation names per site. Do AFTER QUAL-02 (reduces to three sites).
|
||
- **Risk:** Subtle per-site differences (Langfuse naming, error handling) — diff behavior carefully; PERF-07's gather lands here once.
|
||
|
||
### QUAL-04 — tRPC CRUD factory ⚠️ REVIEW · High
|
||
- **File:** `adiuvAI/src/main/router/index.ts` (8 tables repeat list/get/create/update/delete + `if (input.x !== undefined)` set-builders)
|
||
- **Fix:** `router/crud-factory.ts`: `makeCrudRouter(table, { createSchema, updateSchema, defaultOrder })`; plus `pickDefined(input, keys)` helper for update-set building. Prototype on `clients` first; domain procedures spread in.
|
||
- **Risk:** Generic factories can degrade tRPC type inference (`AppRouter` must stay exact for the renderer). If inference breaks, keep per-table routers and extract only `pickDefined` + create/update helpers.
|
||
|
||
### QUAL-05 — Split deep_agent.py (1329 LOC) · High
|
||
- **Fix:** (a) `app/core/prompt_context.py` ← `_*_injection` functions, `_request_context_block`, `format_folder_manifest`, `_prepare_context` (L43-519); (b) `app/core/agent_tools.py` ← `_memory_tools` (684-826), `_read_only_memory_tools`, `_brief_research_tools`, `_all_tools_for_user`, `_contextual_tools`, `get_page_details` (521-871); (c) `deep_agent.py` keeps runner + `run_*` entries. Pure moves; do after QUAL-02/03.
|
||
|
||
### QUAL-06 — backend-client.ts split + dispatch extract ⚠️ REVIEW · Medium
|
||
- **Fix:** (a) `backend-errors.ts` ← 5 error classes (L134-203); (b) extract `private dispatchFrame(frame)` + per-frame handlers from the 230-line `openDeviceWebSocket` (L876-1106); (c) `sendStreamFrame(frameType, payload, callbacks)` helper deduping the six `send*` methods (L276-660); (d) move `recordRunAction` (L55, SQLite write) to `src/main/scouts/scout-run-log.ts`. Full split into connection/requests modules optional second step.
|
||
- **Risk:** Shared mutable state (`streamListeners`, `persistentWs`) — extract as methods first, modules later. Do after CORR-07's guards land.
|
||
|
||
### QUAL-07 — scout_runner.py split + twin dedupe ⚠️ REVIEW · Medium
|
||
- **Fix:** `app/core/scouts/{local_runner.py,cloud_runner.py,runner_common.py}`; extract `_guard_device_online()` and `_process_items(items, process_one) -> RunStats` shared by the 210-line twins (`run_local_agent` 556-766 / `run_cloud_agent` 774-983).
|
||
- **Risk:** Per-item error semantics differ slightly between local/cloud — write characterization tests first (suites exist).
|
||
|
||
### QUAL-08 — device_ws.py handlers → package · Medium
|
||
- **Fix:** Keep `device_ws.py` as protocol layer (endpoint, `_message_loop`, `_heartbeat_loop`, `_mark_runs_disconnected`); move the 9 `_handle_*` to `app/api/ws_handlers/{chat,brief,journey,indexing}.py`. Handlers already take `(websocket, user_id, payload)`. Do with QUAL-12.
|
||
|
||
### QUAL-09 — ProjectSidebar.tsx (1292 LOC, 25+ useState) · Medium
|
||
- **Fix:** Extract `ProjectEditDialog.tsx`, `ClientManageDialogs.tsx`, `NewProjectDialog.tsx`, and a shared `ClientSelect` (the client+sub-client+create-inline combo duplicated between edit and new-project dialogs with parallel state pairs). Pure UI extraction.
|
||
|
||
### QUAL-10 — Ownership-check dependency · Medium (mechanical)
|
||
- **File:** `api/app/api/routes/scouts.py:344,372,391,407,702` (pattern ×6)
|
||
- **Fix:** `async def get_owned_cloud_scout(scout_id: UUID, current_user=Depends(get_current_user), db=Depends(get_session)) -> CloudScoutConfig` raising 404; use as route param. Same pattern available for other resource routes.
|
||
|
||
### QUAL-11 — Standardize tRPC error contract ⚠️ REVIEW · Medium
|
||
- **File:** `adiuvAI/src/main/router/index.ts` — three styles: `{error}` (L109,113), `{success:true}`, `{data,error}` envelopes (1157-1232), zero `TRPCError`.
|
||
- **Fix:** Standardize on throwing `TRPCError` (ipcLink propagates, `ipcLink.ts:76`); migrate procedure-by-procedure WITH their renderer consumers (each `useQuery/useMutation` error handling). Coordinates with CORR-04/CORR-19. Do during QUAL-01 split.
|
||
- **Risk:** Every consumer touched — easy to miss one. Grep all `\.error` accesses on tRPC results.
|
||
|
||
### QUAL-12 — Typed WS frames server-side · Medium
|
||
- **File:** `api/app/api/routes/device_ws.py:424,620,704,767-820` (raw `json.dumps` dicts) vs pydantic frames elsewhere; 7× `# type: ignore[union-attr]` (L293-294,382-383,491,567-568)
|
||
- **Fix:** Define pydantic models for index/journey reply frames in `app/schemas`; use a discriminated union (`Field(discriminator="type")`) for inbound frames, `model_validate` at dispatch, error frame on ValidationError. Kills the type:ignores and the hand-rolled snake/camel fallbacks (L673-676,746-750).
|
||
|
||
### QUAL-13 — Casing contract at WS boundary · Medium
|
||
- **Evidence:** `api/app/agents/note_agent.py:25` reads `row.get("aiSummary") or row.get("ai_summary")` — convention untrusted.
|
||
- **Fix:** Document the contract ("frames snake_case, tool-result rows camelCase") in `api/app/core/ws_context.py` docstring + `adiuvAI/src/shared/casing.ts`; delete the dual-read hedges; add a TypedDict for rows (TYPE-07).
|
||
|
||
### QUAL-14 — OAuth blocks → own files; shared TTLStateStore · Medium
|
||
- **Fix:** `app/api/routes/oauth.py` ← auth.py L45-67+299-505 (incl. extracting `link_or_create_oauth_user()` from the 124-line `oauth_callback`); `app/api/routes/scout_gmail_oauth.py` ← scouts.py L456-807. One `app/auth/state_store.py` `TTLStateStore` replacing both `_pending_states` dicts — the seam where SEC-18's Redis backend plugs in. Sequence: build TTLStateStore during SEC-18, move files in Phase 6.
|
||
|
||
### QUAL-15 — Agent helpers dedupe · Low (mechanical)
|
||
- **Fix:** `api/app/agents/_common.py` with `is_uuid()` (copies at task_agent.py:14-20, note_agent.py:15-22) and `format_rows(rows, line_fn, empty_msg)`.
|
||
|
||
---
|
||
|
||
# 7. Type Safety
|
||
|
||
### TYPE-01 — Broken relative imports, types silently `any` · High (mechanical)
|
||
- **Files:** `adiuvAI/src/renderer/components/ai/blocks/ChatEntityBlock.tsx:13`, `ChatChartBlock.tsx:27`, `ChatTimelineBlock.tsx:5`
|
||
- **Problem:** `'../../../../../shared/api-types'` (5 levels = repo root) — one `../` too many. `import type` so esbuild strips it without resolving; tsc confirms TS2307. Block data types are `any` throughout.
|
||
- **Fix:** Remove one `../` in each of the three imports (→ `'../../../../shared/api-types'`).
|
||
|
||
### TYPE-02 — No Python type checker · High ⚠️ REVIEW (config choices)
|
||
- **Fix:** Add `api/pyproject.toml` with `[tool.mypy]`: start `check_untyped_defs = true`, `warn_return_any = true`; per-module strict for `app/core`, `app/api`. Wire into CI beside `ruff check`. First run will surface real bugs — budget a cleanup pass.
|
||
|
||
### TYPE-03 — tsc doesn't pass; no CI typecheck · Medium
|
||
- **Evidence:** pre-existing errors at `backend-client.ts:963`, `drizzle-executor.ts:367`, `OnboardingFlow.tsx:157/165/291` (+ TYPE-01 cascade).
|
||
- **Fix:** After TYPE-01, fix the remaining tsc errors (inspect each — may be real bugs), then add `"typecheck": "tsc --noEmit"` script + CI step.
|
||
|
||
### TYPE-04 — RequestContext model for deep_agent · Medium
|
||
- **File:** `api/app/core/deep_agent.py` (19× `dict[str, Any]`; `context` threaded through L43-592+)
|
||
- **Fix:** `RequestContext` TypedDict/pydantic model in `app/schemas/` (trace_id, session_id, format_prefs, proactive hints, relational memory, …); `_trace_id_from_context`/`_session_id_from_context` (L574-590) become attribute access and get deleted. Do with QUAL-05.
|
||
|
||
### TYPE-05 — drizzle-executor cast cluster · Medium (mechanical)
|
||
- **File:** `adiuvAI/src/main/api/drizzle-executor.ts:73,159,262,317,335` (table-as-Record ×5), `:230,249` (`Boolean as unknown as (x) => x is SQL` — dead weight, conditions already `SQL[]`)
|
||
- **Fix:** One helper using drizzle's `getTableColumns()`: `getColumn(table, name): SQLiteColumn | undefined`; replace the record casts. Delete the Boolean predicate casts (`and(...conditions)` suffices).
|
||
|
||
### TYPE-06 — window.electronAI escapes · Low (mechanical)
|
||
- **Files:** `ContextualChatContext.tsx:148-150`, `CloudScoutConfigPanel.tsx:39`, `CloudScoutCreationFlow.tsx:47`, `httpLink.ts:17`
|
||
- **Fix:** `src/renderer/types/electron-api.d.ts` with `declare global { interface Window { electronAI?: ElectronAI; electronTRPC?: …; electronDialog?: … } }`; add `"types": ["vite/client"]` to tsconfig (fixes `(import.meta as any).env`).
|
||
|
||
### TYPE-07 — Boundary typing misc · Low
|
||
- `ToolResult` TypedDict in `api/app/core/ws_context.py` (`rows`, `row`, `error`) — types the most-trafficked boundary; pairs with QUAL-13.
|
||
- 11 missing return annotations: `api/app/api/routes/scouts.py` cloud CRUD handlers (302, 313, 337, 366, 758), `device_ws.get_session_buffer:321` — add once TYPE-02's checker exists.
|
||
- `Provider` Protocol in `app/integrations/__init__.py` for `fetch_messages`/`fetch_emails` (kills 2 type:ignores in scout_runner.py:851+).
|
||
- `adiuvAI/src/main/ipc.ts:55` `(response: any)` → type the tRPC response envelope; `SerializedTRPCResponse` in `src/shared/` closes the ipcLink seam (`ipcLink.ts:66,76`).
|
||
|
||
---
|
||
|
||
## Notes for the Executor
|
||
|
||
1. **Line numbers reference `main` @ `315c5d0`.** Re-verify each quoted snippet exists before editing; if it moved, find it by content, not line.
|
||
2. **Never execute ⚠️ REVIEW items unattended.** Security fixes especially: a sanitizer with a bypass or a broken auth check is worse than the original bug.
|
||
3. **After each Phase, run:** `cd api && ruff check . && pytest` · `cd adiuvAI && npm run lint && npx tsc --noEmit && npm run knip` (tsc becomes meaningful after TYPE-01/TYPE-03). There is no Electron test suite — manual smoke for renderer-affecting changes.
|
||
4. **Two-repo changes** (SEC-13, PERF-17, CORR-12, QUAL-13) must land backward-compatibly: backend accepts both old and new shapes for one release.
|
||
5. **Run `graphify update .` after each modifying session** (project rule).
|
||
6. Items needing **owner decisions before any execution:** SEC-16, SEC-22, SEC-28, DEAD-04, DEAD-07, CORR-08 (approach choice), DEPS-04 (tool choice).
|