Files
kite/.planning/phases/09-admin-panel-rearchitecture/09-05-PLAN.md
T
curo1305andClaude Sonnet 4.6 ba30ada87a docs(09): create phase 9 plan — Admin Panel Rearchitecture
5 plans across 4 waves covering ADMIN-08..12, CODE-06, CODE-09:

- 09-01 (W1): backend GET /api/admin/overview + 8 ADMIN-11 tests
- 09-02 (W1): AdminLayout + AdminSidebar + AdminOverviewView + getAdminOverview API
- 09-03 (W2): extract 4 admin tab components to standalone views
- 09-04 (W3): nested /admin route + to.matched.some guard + D-08/D-09/D-10 redirects + Tailwind safelist (sky+amber) + login redirect + delete AdminView/tabs
- 09-05 (W4): CODE-09 comment purge for Phase 9 + retroactive Phase 8 backend sub-packages + human UAT checkpoint

All locked decisions D-01..D-16 mapped to tasks; safelist corrected to include sky (OneDrive) and amber (audit badges) per RESEARCH.md Pattern 7.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-06-12 15:34:31 +02:00

259 lines
19 KiB
Markdown

---
phase: 09-admin-panel-rearchitecture
plan: 05
type: execute
wave: 4
depends_on:
- 09-01
- 09-02
- 09-03
- 09-04
files_modified:
- backend/api/admin/overview.py
- backend/api/admin/__init__.py
- backend/api/admin/users.py
- backend/api/admin/quotas.py
- backend/api/admin/ai.py
- backend/api/admin/shared.py
- backend/api/documents/__init__.py
- backend/api/documents/upload.py
- backend/api/documents/content.py
- backend/api/documents/crud.py
- backend/api/documents/shared.py
- backend/api/auth/__init__.py
- backend/api/auth/tokens.py
- backend/api/auth/totp.py
- backend/api/auth/password.py
- backend/api/auth/shared.py
- frontend/src/layouts/AdminLayout.vue
- frontend/src/components/admin/AdminSidebar.vue
- frontend/src/views/admin/AdminOverviewView.vue
- frontend/src/views/admin/AdminUsersView.vue
- frontend/src/views/admin/AdminQuotasView.vue
- frontend/src/views/admin/AdminAiView.vue
- frontend/src/views/admin/AdminAuditView.vue
- frontend/src/router/index.js
- frontend/src/views/auth/LoginView.vue
- frontend/tailwind.config.js
- frontend/src/api/admin.js
autonomous: false
requirements:
- CODE-09
user_setup: []
must_haves:
truths:
- "Every file touched in Phase 9 contains no 'what' comments — only 'why' comments remain"
- "Every Phase 8 backend sub-package file (api/admin/*.py, api/documents/*.py, api/auth/*.py) has been comment-purged using the D-16 criterion"
- "The 'NO prefix' constraint comments in the three package __init__.py files are PRESERVED — they are non-obvious invariants per D-16"
- "Full backend pytest suite is green and frontend build succeeds after the purge"
artifacts:
- path: "backend/api/admin/__init__.py"
provides: "constraint comment about NO prefix preserved (D-16 'why' comment exemption)"
contains: "NO prefix"
key_links:
- from: "purged file set"
to: "test suite"
via: "pytest -v"
pattern: "0 failed"
---
<objective>
Apply the CODE-09 comment purge across (a) every file touched in Phase 9 (both backend and frontend) and (b) retroactively across the Phase 8 backend sub-packages (`backend/api/admin/`, `backend/api/documents/`, `backend/api/auth/`). Remove every "what" comment (generic docstrings, "this does X" inline notes). Keep every "why" comment (constraints, pitfall notes, non-obvious invariants — D-16). The `prefix="/api/admin"` / "NO prefix" constraint comments are the canonical "why" exemplars and MUST be preserved.
Purpose: CODE-09 requires that no comment describes WHAT code does. D-15 mandates a single dedicated plan at the end of Phase 9 covering Phase 9 files plus Phase 8 backend sub-packages.
Output: cleaner code; same behavior; full test suite green; frontend build green.
</objective>
<execution_context>
@$HOME/.claude/get-shit-done/workflows/execute-plan.md
@$HOME/.claude/get-shit-done/templates/summary.md
</execution_context>
<context>
@.planning/phases/09-admin-panel-rearchitecture/09-CONTEXT.md
@.planning/phases/09-admin-panel-rearchitecture/09-RESEARCH.md
@.planning/phases/09-admin-panel-rearchitecture/09-PATTERNS.md
@CLAUDE.md
@backend/api/admin/__init__.py
@backend/api/auth/__init__.py
@backend/api/documents/__init__.py
<interfaces>
From D-16 (purge criterion):
- **REMOVE** comments that describe WHAT the code does: generic function docstrings like `"""Return all users."""`, inline `# Loop over users` notes, `// Set state to loading` annotations, comment blocks restating the obvious from the next line.
- **KEEP** comments that explain WHY: constraint notes (e.g., "NO prefix — parent carries /api/admin"), pitfall references (e.g., "T-08-04-04 prevention"), non-obvious invariants (e.g., "constant-time comparison required for token validation"), security boundary notes (e.g., "never include credentials_enc here").
From RESEARCH.md §Pitfall 6 (explicit exemption):
- The constraint docstring at the top of `backend/api/admin/__init__.py` ("sub-routers carry NO prefix") reads like a docstring but is actually a non-obvious invariant comment. PRESERVE IT.
- Equivalent constraint comments at the top of `backend/api/auth/__init__.py` and `backend/api/documents/__init__.py` follow the same rule — if they document the prefix invariant or a cross-module import constraint, PRESERVE.
From CLAUDE.md (Code Standards):
- "Comments exist only where the *why* is non-obvious. Never explain what the code does."
Phase 8 backend file inventory (from RESEARCH.md Open Question 3 + repo audit):
- backend/api/admin/: __init__.py, shared.py, users.py, quotas.py, ai.py
- backend/api/documents/: __init__.py, shared.py, upload.py, content.py, crud.py
- backend/api/auth/: __init__.py, shared.py, tokens.py, totp.py, password.py
- (Note: roadmap mentions `sessions.py` for auth, but the repo only contains the four files above — purge what exists.)
Phase 9 backend files (created in 09-01):
- backend/api/admin/overview.py
- (backend/api/admin/__init__.py — already in Phase 8 set above; touched again in 09-01)
Phase 9 frontend files (created/modified in 09-02, 09-03, 09-04):
- frontend/src/layouts/AdminLayout.vue
- frontend/src/components/admin/AdminSidebar.vue
- frontend/src/views/admin/AdminOverviewView.vue
- frontend/src/views/admin/AdminUsersView.vue
- frontend/src/views/admin/AdminQuotasView.vue
- frontend/src/views/admin/AdminAiView.vue
- frontend/src/views/admin/AdminAuditView.vue
- frontend/src/router/index.js
- frontend/src/views/auth/LoginView.vue
- frontend/tailwind.config.js
- frontend/src/api/admin.js
</interfaces>
</context>
<tasks>
<task type="auto">
<name>Task 1: Backend purge — Phase 8 sub-packages + Phase 9 overview.py</name>
<files>backend/api/admin/__init__.py, backend/api/admin/shared.py, backend/api/admin/users.py, backend/api/admin/quotas.py, backend/api/admin/ai.py, backend/api/admin/overview.py, backend/api/documents/__init__.py, backend/api/documents/shared.py, backend/api/documents/upload.py, backend/api/documents/content.py, backend/api/documents/crud.py, backend/api/auth/__init__.py, backend/api/auth/shared.py, backend/api/auth/tokens.py, backend/api/auth/totp.py, backend/api/auth/password.py</files>
<read_first>
- backend/api/admin/__init__.py (anchor file for the "NO prefix" invariant comment — confirm before touching anything else)
- backend/api/auth/__init__.py (same — confirm equivalent constraint comment exists and keep)
- backend/api/documents/__init__.py (same)
- One representative file per package to calibrate the purge: backend/api/admin/users.py, backend/api/documents/upload.py, backend/api/auth/tokens.py
- backend/api/admin/overview.py (new from 09-01 — verify the NO-prefix WHY comment exists and is preserved)
</read_first>
<behavior>
- For each file in the list: read top-to-bottom; for each comment / docstring, decide "WHAT" (remove) vs "WHY" (keep) per D-16.
- Module-level docstrings that only restate the filename (e.g., `"""Admin users API."""`) are WHAT — remove.
- Module-level docstrings that document a constraint or invariant (e.g., the admin `__init__.py` "NO prefix" block) are WHY — KEEP.
- Function docstrings of the form `"""Return all users."""` or `"""Get the user by ID."""` — WHAT, remove.
- Function docstrings that describe a non-obvious contract (e.g., `"""Decrement quota atomically; raises QuotaExceeded if new total > limit."""`) — WHY (invariant), KEEP.
- Inline `#` comments restating the next line (e.g., `# Build the query` above `query = select(User)`) — WHAT, remove.
- Inline `#` comments documenting why a step is needed (e.g., `# atomic UPDATE; never read-then-write (CLAUDE.md)`) — WHY, KEEP.
- TODOs are WHY-adjacent — KEEP unless the underlying TODO is already resolved.
- Bandit suppression annotations (`# noqa: S105`, `# nosec`) with rationale — KEEP as written.
- Behavior of every endpoint and helper is UNCHANGED. No code paths added or removed.
</behavior>
<action>For each file in `<files>`, apply the WHAT-vs-WHY judgment per `<behavior>`. Edit in place via the Edit tool — never rewrite entire files. Specific exemptions to PRESERVE verbatim: (1) the constraint docstring at the top of `backend/api/admin/__init__.py` documenting the `prefix="/api/admin"` invariant and sub-router NO-prefix rule; (2) any equivalent constraint comment at the top of `backend/api/auth/__init__.py` and `backend/api/documents/__init__.py`; (3) the "T-08-04-04 prevention" note (or whichever threat-ID note exists) tying the prefix rule to the Phase 8 threat model; (4) any HKDF domain-separation `info=...` rationale comments in `auth/*.py`; (5) any "constant-time comparison" notes around `hmac.compare_digest`; (6) any atomic-UPDATE-RETURNING quota notes; (7) the "NO prefix" single-line comment in `backend/api/admin/overview.py` from 09-01. After purging each file, run `cd backend && python -c "from api.admin import router; from api.documents import router; from api.auth import router"` to confirm the imports still resolve (catches accidentally-removed `from x import y` lines). After all files are purged, run `cd backend && pytest -v` and fail the task if any new test failure appears compared to the green baseline at the end of 09-04. Do NOT touch any `# type: ignore` comments (these are tool directives, not human comments).</action>
<verify>
<automated>cd backend && python -c "from api.admin import router as a; from api.documents import router as d; from api.auth import router as u; print(len(a.routes), len(d.routes), len(u.routes))" && grep -c "NO prefix" api/admin/__init__.py && grep -c "NO prefix" api/admin/overview.py && pytest -v --tb=no -q | tail -20</automated>
</verify>
<acceptance_criteria>
- Module imports still succeed for all three admin packages (`from api.admin import router` etc.).
- `grep -c "NO prefix" backend/api/admin/__init__.py` returns at least 1 (constraint comment preserved).
- `grep -c "NO prefix" backend/api/admin/overview.py` returns at least 1 (Phase 9 WHY comment preserved).
- `cd backend && pytest -v --tb=no` reports the same passed count as the green baseline at the end of 09-04 (record both counts in SUMMARY).
- `cd backend && bandit -r api/admin/ api/documents/ api/auth/ -q` produces zero new HIGH findings vs. baseline.
- For each file purged, line count delta is non-positive (purging cannot add lines).
- No `from <module> import <name>` line was removed by the purge (verify with diff that no `import` lines are gone).
- Spot check 3 random `# `-prefixed comments still present in the files — each one passes the "could a reader figure this out from the next line of code?" test → if yes, it must be removed; if no (non-obvious WHY), it stays.
</acceptance_criteria>
<done>All Phase 8 backend sub-packages + Phase 9 `overview.py` purged of WHAT comments; invariant comments preserved; full backend suite green.</done>
</task>
<task type="auto">
<name>Task 2: Frontend purge — Phase 9 Vue + JS files</name>
<files>frontend/src/layouts/AdminLayout.vue, frontend/src/components/admin/AdminSidebar.vue, frontend/src/views/admin/AdminOverviewView.vue, frontend/src/views/admin/AdminUsersView.vue, frontend/src/views/admin/AdminQuotasView.vue, frontend/src/views/admin/AdminAiView.vue, frontend/src/views/admin/AdminAuditView.vue, frontend/src/router/index.js, frontend/src/views/auth/LoginView.vue, frontend/tailwind.config.js, frontend/src/api/admin.js</files>
<read_first>
- The 11 files in `<files>` (skim each; identify WHAT vs WHY comments)
- CLAUDE.md §Code Standards (the comment rule)
</read_first>
<behavior>
- Same WHAT-vs-WHY rule as Task 1.
- Vue `<!-- HTML comments -->` follow the same rule.
- JS `// inline` comments and `/* block */` comments follow the same rule.
- JSDoc / TSDoc `/** … */` blocks describing return value or generic param meaning are WHAT — remove unless they document a non-obvious invariant.
- Tailwind config has no comments to purge by default but check.
- Tag-level comments in `.vue` files like `<!-- Logo -->`, `<!-- Nav section -->`, `<!-- Footer -->` are WHAT (the section is obvious from the markup) — remove unless they document a non-obvious wiring constraint (e.g., `<!-- inside aside, NOT inside main — admin scope only -->`).
- The four extracted views (`AdminUsersView`, `AdminQuotasView`, `AdminAiView`, `AdminAuditView`) inherit comments from their Phase 8 source tab components — apply the purge to those inherited comments as part of this task.
- Behavior unchanged. `npm run build` continues to succeed.
</behavior>
<action>For each file in `<files>`, apply WHAT-vs-WHY judgment. Specific preservations: any constraint note about Vue Router 4 meta inheritance (`to.matched.some(...)`) — KEEP; any note documenting why `request()` is reused via `utils.js` (Phase 8 CODE-04 invariant) — KEEP if present; any safelist rationale documenting why `sky` + `amber` were added — KEEP; any D-XX decision references inline (e.g., `// D-08: admin login redirect`) — KEEP (these are anchor notes linking back to CONTEXT.md). Tag-section comments in the Vue templates that just label a layout section (`<!-- Stat cards -->`) — remove. After purging, run `cd frontend && npm run build` and the existing frontend test command (Vitest if configured) to confirm no regression.</action>
<verify>
<automated>cd frontend && npm run build && grep -c "to.matched.some" src/router/index.js</automated>
</verify>
<acceptance_criteria>
- `cd frontend && npm run build` succeeds.
- `grep -c "to.matched.some" frontend/src/router/index.js` still returns at least 1 (guard not accidentally erased).
- The "Admin" subtitle text in `AdminSidebar.vue` is still present (verify with `grep -c "Admin</p>" frontend/src/components/admin/AdminSidebar.vue` returns 1 — the visible badge text is not a comment).
- The safelist regex in `tailwind.config.js` is still present (`grep -c "amber" frontend/tailwind.config.js` ≥ 1; `grep -c "sky" frontend/tailwind.config.js` ≥ 1).
- For each purged file, line count delta is non-positive.
- No `import` line was removed.
- Spot check 3 surviving `<!-- … -->` comments — each one passes the "could a reader figure this out from the next markup line?" test.
</acceptance_criteria>
<done>All 11 Phase 9 frontend files purged of WHAT comments; build green; behavior unchanged.</done>
</task>
<task type="checkpoint:human-verify" gate="blocking">
<name>Task 3: Human verification — full Phase 9 UX + comment purge spot check</name>
<files>(no files — human verification only)</files>
<action>Follow the steps in `<how-to-verify>` below; record any failures in feedback; respond per `<resume-signal>`.</action>
<verify><human-check>see how-to-verify below</human-check></verify>
<done>Human approves Phase 9 end-to-end: admin URLs route correctly, login redirect honors role, guard blocks non-admin from /admin/*, redirects admin from user routes, OneDrive (sky) + audit (amber) badges render in production build, and surviving comments are genuine WHY notes.</done>
<what-built>
Phase 9 is complete after this checkpoint. The user verifies that:
- The five admin URLs render the correct view with the admin sidebar.
- Login as admin lands at `/admin`; login as regular user lands at `/`.
- A non-admin navigating to `/admin/users` is bounced to `/`.
- An admin navigating to `/` is bounced to `/admin`.
- OneDrive provider chip renders in sky color in production build; audit action badges render in amber.
- Spot-checking three random purged files confirms that the surviving comments are all genuine WHY notes.
</what-built>
<how-to-verify>
1. Backend suite: `cd backend && pytest -v` — record passed count; must equal the 09-04 baseline.
2. Frontend build: `cd frontend && npm run build` — must succeed.
3. Start the stack: `docker compose up -d` (or `cd backend && uvicorn main:app --reload` + `cd frontend && npm run dev`).
4. As an **admin** user: log in. Confirm you land at `/admin`. Click each of the 5 nav links and confirm: Overview shows 4 stat cards + a 10-row recent-activity table; Users/Quotas/AI/Audit show their full pre-Phase-9 functionality.
5. Open browser dev tools, paste `/topics` into the URL bar. Confirm the guard redirects you back to `/admin`.
6. Log out. Log in as a **non-admin** user. Confirm you land at `/` (or `?redirect=` target).
7. Paste `/admin/users` into the URL bar. Confirm the guard redirects you to `/`.
8. Production build visual check: with `npm run build && npm run preview`, look at an account with OneDrive connected (or open the Cloud sidebar) — the OneDrive chip should render in sky color, NOT default gray. Open the audit log — the action-type badges should show their amber/blue/purple colors.
9. Open three random files from the Task 1 and Task 2 file lists (your pick). For each, scan the remaining comments: each comment must be a WHY note. If any comment merely restates the next line of code, the purge missed it — record the file + line in your feedback.
</how-to-verify>
<resume-signal>Type "approved" if all steps pass. Otherwise describe what failed (e.g., "OneDrive chip is gray in production build" or "/admin/audit 404s after admin login").</resume-signal>
</task>
</tasks>
<threat_model>
## Trust Boundaries
| Boundary | Description |
|----------|-------------|
| Comment purge → invariant erasure | Code that depends on a constraint may silently break if the constraint comment is removed and a future change violates the rule |
## STRIDE Threat Register
| Threat ID | Category | Component | Disposition | Mitigation Plan |
|-----------|----------|-----------|-------------|-----------------|
| T-09-05-01 | Tampering | Constraint comment erasure | mitigate | Explicit preservation list in Task 1 action prose (NO-prefix invariants, HKDF domain separation, constant-time comparison, atomic UPDATE-RETURNING); acceptance criteria assert specific grep results on the preserved anchor comments |
| T-09-05-02 | Denial-of-Service | Accidental import removal | mitigate | After purge, Task 1 runs `python -c "from api.admin import router; …"` to confirm package imports resolve; full pytest -v is the second gate |
| T-09-05-03 | Information Disclosure | Comment leaking implementation details | accept | Purge removes more than it adds; no new comments introduced; existing WHY comments already reviewed in Phase 8 security agent runs |
</threat_model>
<verification>
- `cd backend && pytest -v` — same passed count as 09-04 baseline; zero new failures.
- `cd frontend && npm run build` — succeeds.
- Anchor comments still present (NO prefix, etc.) — verified by grep in acceptance criteria.
- Human checkpoint (Task 3) records visual confirmation of admin UX + dynamic color rendering + spot-check of preserved WHY comments.
</verification>
<success_criteria>
- CODE-09: No file in the Phase 9 / Phase 8 backend purge set contains a comment that describes WHAT the code does.
- D-15: One dedicated plan covered Phase 9 files + retroactive Phase 8 backend sub-packages — this is that plan.
- D-16: WHY comments preserved; the "NO prefix" anchors are the canonical exemplars.
- Backend suite green; frontend build green; human UAT approved.
</success_criteria>
<output>
Create `.planning/phases/09-admin-panel-rearchitecture/09-05-SUMMARY.md` when done. Include: backend pytest passed count (before vs after), list of files purged, list of anchor comments explicitly preserved, and the human-checkpoint approval line.
</output>