From 5ac2103a4b530e6ea6727889aa9cc987b0202716 Mon Sep 17 00:00:00 2001 From: root Date: Sat, 20 Jun 2026 18:09:00 -0500 Subject: [PATCH] docs: reconcile PRD + fix-wave with current code (ea0ba33) The audit anchors had drifted from the implementation. Corrected against the live source as of commit ea0ba33: - PRD invariant #1: mode registry is now 24 handlers in _VALID_MODES, not "only extract". Added invariant #2: unknown modes fail with HTTP 400 + valid_modes (was 200-with-error-body). Added SECURE to the session-cookie invariant. - PRD known-gaps + change-axis: marked the mode-registration and unknown-mode items RESOLVED; flagged the live codereview-vs-code_review naming mismatch that still 400s lakehouse observer escalations. - SCRUM_FIX_WAVE items 1-2 struck through as DONE; recorded the cookie + nginx security fixes under "Recently landed". Machine-generated .memory/ state left untouched (regenerated by the scrum pipeline, not hand-edited). Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/PRD.md | 99 ++++++++++++++++++++++++++++++++++++++++++ docs/SCRUM_FIX_WAVE.md | 58 +++++++++++++++++++++++++ 2 files changed, 157 insertions(+) create mode 100644 docs/PRD.md create mode 100644 docs/SCRUM_FIX_WAVE.md diff --git a/docs/PRD.md b/docs/PRD.md new file mode 100644 index 0000000..58c3201 --- /dev/null +++ b/docs/PRD.md @@ -0,0 +1,99 @@ +# LLM Team UI — Product Requirements (for scrum-master audits) + +This doc is the anchor the lakehouse scrum-master uses when auditing +this codebase. It is the "ground truth" — every scrum finding is +evaluated against what this doc says the system should be. Written +2026-04-24 to give the scrum pipeline a target surface; keep it tight +and aligned with what the code actually tries to do. Last reconciled +2026-06-20 against commit `ea0ba33` — the mode registry and unknown-mode +handling described below were verified against the live source on that date. + +## What LLM Team UI is + +Flask-based web UI that lets a user configure and run multi-model +"teams" — pipelines of LLMs (OpenRouter + Ollama Cloud + local Ollama) +that collaborate on a task with roles like executor, reviewer, critic, +sentinel. Lives at devop.live:5000 behind nginx. Main file +`llm_team_ui.py` handles HTTP routes, session auth, model orchestration, +SSE streaming. Backed by PostgreSQL for users + runs + audit. + +## Core invariants (scrum should flag if violated) + +1. **Every mode accepted by `/api/run` must be in the `_VALID_MODES` + registry and have a matching `run_` handler.** As of 2026-06-20 + there are 24 registered modes (brainstorm, pipeline, debate, validator, + roundrobin, redteam, consensus, codereview, ladder, tournament, + evolution, blindassembly, staircase, drift, mesh, hallucination, + timeloop, research, eval, extract, refine, adaptive, deep_analysis, + distill), each with a `run_` function. `run_team()` validates + `mode` against `_VALID_MODES` before opening the SSE stream and rejects + anything else with HTTP 400 (see invariant below). The registry is kept + in sync with the handlers manually — adding a `run_` without + listing it in `_VALID_MODES` (or vice-versa) is the hard fail to flag. +2. **Unknown modes must fail fast with HTTP 400 + the valid-mode list.** + `run_team()` returns `{"error": "Unknown mode: X", "valid_modes": [...]}` + with status 400 *before* streaming. Returning 200-with-error-body (the + old behavior) is a hard fail — it forces callers to parse SSE to learn + the request was bad. +3. **All authenticated routes must call `@login_required` decorator.** + The SESSION_COOKIE_HTTPONLY + SAMESITE=Lax + SECURE defaults must hold + (SECURE added 2026-06-20; the app sits behind nginx TLS). Any route + that mutates state without auth is a hard fail. +4. **Security logging goes to `/var/log/llm-team-security.log`** via + the `sec_log` logger. Fail2ban watches this file. Any new auth path + must emit there on failure. +5. **PostgreSQL connections come from `DB_URL` env.** Direct literal + connection strings are a hard fail. +6. **SMTP alerts via `send_security_alert(subject, body)`** — never + log secrets in the body. +7. **SSE streams must flush incrementally** — if an endpoint returns + all-at-once, it should not claim SSE in the Content-Type. +8. **Password storage: bcrypt only.** Any plain-text / SHA256 password + path is a hard fail. +9. **Rate limiting enforced** on /login, /register, and model-invocation + endpoints. No unbounded loops that invoke paid APIs. + +## Known gaps (scrum can work on these) + +- **~~Only `extract` mode is registered.~~ RESOLVED (2026-06-20).** The + mode set grew from one handler to 24 (`run_brainstorm` … `run_distill`), + all listed in `_VALID_MODES` and dispatched by `run_team()`. Note the + mode named in code is `codereview` (not `code_review`); callers using the + underscored name still get a 400. Historical context: observer.ts in + lakehouse was escalating to `/api/run?mode=code_review` and silently hit + "Unknown mode" for weeks (see lakehouse memory + `reference_llm_team_modes.md`) — the underscore-vs-no-underscore mismatch + is the remaining footgun, not the missing handler. +- **File is 13K lines.** Cohesive modules (auth, teams, runs, alerts, + SSE, templates) should be separated — but that's a refactor, out of + scope for individual scrum-master reviews. +- **Audit log schema is thin.** The PostgreSQL `runs` table stores the + fact of a run but not fine-grained per-step outcomes. Downstream + correlation is manual. +- **Templates inline in Python as `render_template_string(...)`.** + Makes HTML review impossible via the scrum pipeline — HTML is hidden + inside Python string literals. + +## Change proposal axis (what the scrum should optimize for) + +1. **~~Every Unknown-mode endpoint should either be registered or return + 404/not-a-mode.~~ DONE (2026-06-20).** `run_team()` now returns HTTP 400 + (not 404 — chosen so it reads as "bad request / wrong mode" rather than + "route missing") with a `valid_modes` list, before streaming. Callers + can now distinguish a wrong call from a missing mode. Remaining: see the + `codereview` vs `code_review` naming footgun in Known gaps. +2. **Auth decorators should be typed** — `@login_required` is a + runtime check; adding an `Annotated[User, Depends(auth)]`-style + type hint (possible via Flask's `g` object) would make review + mechanical. +3. **Security logging should be structured** — current `sec_log.warning( + f"...")` loses fields. Move to `logger.warning(msg, extra={...})` + so fail2ban and external tools can parse. +4. **Secret handling** — `FLASK_SECRET` in env is correct, but the + fallback to `~/.llm-team-secret` means dev mode silently generates + a persistent secret. Not bad, but should log once. + +## How scrum-master audits this repo + +The lakehouse scrum pipeline is repo-agnostic via env vars. See +`/root/llm-team-ui/CLAUDE.md` for the exact runbook. diff --git a/docs/SCRUM_FIX_WAVE.md b/docs/SCRUM_FIX_WAVE.md new file mode 100644 index 0000000..f0a80a4 --- /dev/null +++ b/docs/SCRUM_FIX_WAVE.md @@ -0,0 +1,58 @@ +# LLM Team scrum-master fix wave + +Concrete changes the scrum-master should propose when auditing this +repo. Evaluated at ~confidence level per item. Keep focused — only +things that are safely landable per review, not the full refactor. + +Last reconciled 2026-06-20 against commit `ea0ba33`. Items 1–2 below have +shipped since this list was written; they're kept (struck through) so the +numbering of 3–8 stays stable. + +## Recently landed (verified 2026-06-20) + +- **Session cookie hardening** — `SESSION_COOKIE_SECURE` added (was only + HTTPONLY + SAMESITE=Lax). Commit `ea0ba33`. +- **nginx ban IP validation** — `_nginx_ban` now validates the IP via + `ipaddress.ip_address()` before writing the `deny` rule, closing a + config-injection path. Commit `ea0ba33`. + +## High priority (start here) + +1. ~~**Register the missing modes** — `run_code_review`, `run_patch`, + `run_refactor`.~~ **DONE.** The mode set is now 24 handlers + (`run_brainstorm` … `run_distill`) in `_VALID_MODES`. NB: the code mode + is `codereview`, not `code_review` — lakehouse observer escalations + using the underscored name still 400. Closing that naming gap is the + only remaining work here. + +2. ~~**Mode-dispatch should 404 when mode is unknown.**~~ **DONE (as 400).** + `run_team()` validates `mode` against `_VALID_MODES` and returns HTTP + 400 + `valid_modes` before streaming. Implemented as 400 rather than the + proposed 404 (reads as "bad request", not "route missing"). + +3. **Structured security logging.** Replace `sec_log.warning(f"...")` + with `sec_log.warning("event", extra={"fields": ...})` so the log + format is JSON-parseable by fail2ban + external tools. + +## Medium priority + +4. **Rate-limit decorators on login/register.** Currently an attacker + can hammer /login at full request rate. Add per-IP + per-user rate + limits via `flask-limiter` or a simple in-memory sliding window. + +5. **Audit log gains per-step fields.** Extend the `runs` table with + per-phase timestamps and outcomes so downstream analysis can + reconstruct what happened without replaying the full run. + +6. **Templates → separate files.** Extract `render_template_string(...)` + HTML into `templates/*.html` so HTML/CSS review can happen + separately from Python logic. + +## Low priority (nice to have, not urgent) + +7. **Type hints on all public routes.** Adds IDE completion + makes + auditor pattern-match easier. + +8. **Split llm_team_ui.py into modules.** 13K lines in one file makes + scrum-master reviews expensive (must tree-split). Modules: `auth.py`, + `teams.py`, `runs.py`, `alerts.py`, `sse.py`, `templates.py`. -- 2.47.2