docs(08): create phase 8 plan — 8 plans, 3 waves (stack upgrade + backend decomposition)
Wave 0: CR-01/02/03 test stubs + api/schemas.py (CloudConnectionOut migration) Wave 1: useToastStore stub + Phase 7.1 frontend completion Wave 2 (parallel): api/admin/, api/documents/, api/auth/ package splits + client.js decomposition + PERF-01 deps Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
curo1305
co-authored by
Claude Sonnet 4.6
Claude Sonnet 4.6
parent
c20a5d8913
commit
25e568973f
@@ -0,0 +1,219 @@
|
||||
---
|
||||
phase: 08-stack-upgrade-backend-decomposition
|
||||
plan: 05
|
||||
type: execute
|
||||
wave: 2
|
||||
depends_on: [08-03]
|
||||
files_modified:
|
||||
- backend/api/documents/__init__.py
|
||||
- backend/api/documents/upload.py
|
||||
- backend/api/documents/crud.py
|
||||
- backend/api/documents/content.py
|
||||
- backend/api/documents/shared.py
|
||||
- backend/api/documents.py
|
||||
autonomous: true
|
||||
requirements: [CODE-02, CODE-08]
|
||||
tags: [backend-decomposition, documents, sub-router]
|
||||
must_haves:
|
||||
truths:
|
||||
- "backend/api/documents/ is a Python package with sub-modules upload.py, crud.py, content.py, shared.py and __init__.py"
|
||||
- "All document endpoints respond on the same URL paths as before"
|
||||
- "Every document endpoint enforces get_current_user (regular user) and the ownership assertion `resource.user_id == current_user.id`"
|
||||
- "_CLOUD_PROVIDERS frozenset and shared Pydantic models live in shared.py, not duplicated across sub-modules"
|
||||
- "No sub-router declares a prefix; only api/documents/__init__.py carries prefix=/api/documents"
|
||||
- "Old backend/api/documents.py monolith is deleted"
|
||||
- "main.py import `from api.documents import router as documents_router` continues to work"
|
||||
artifacts:
|
||||
- path: "backend/api/documents/__init__.py"
|
||||
provides: "Router aggregator with prefix=/api/documents; includes upload_router, crud_router, content_router"
|
||||
contains: "router = APIRouter(prefix"
|
||||
- path: "backend/api/documents/upload.py"
|
||||
provides: "Presigned URL + direct upload + confirm handlers (POST /upload-url, POST /upload, POST /{id}/confirm)"
|
||||
contains: "async def request_upload_url"
|
||||
- path: "backend/api/documents/crud.py"
|
||||
provides: "List/get/patch/delete + re-classify endpoint per D-08"
|
||||
contains: "async def list_documents"
|
||||
- path: "backend/api/documents/content.py"
|
||||
provides: "Range-aware content streaming endpoint + _parse_range helper"
|
||||
contains: "async def stream_document_content"
|
||||
- path: "backend/api/documents/shared.py"
|
||||
provides: "UploadUrlRequest, DocumentPatch Pydantic models, _CLOUD_PROVIDERS constant"
|
||||
contains: "_CLOUD_PROVIDERS = frozenset"
|
||||
key_links:
|
||||
- from: "backend/main.py"
|
||||
to: "backend/api/documents/__init__.py"
|
||||
via: "from api.documents import router as documents_router"
|
||||
pattern: "from api.documents import router as documents_router"
|
||||
- from: "backend/api/documents/upload.py and crud.py"
|
||||
to: "backend/api/documents/shared.py"
|
||||
via: "from api.documents.shared import _CLOUD_PROVIDERS, UploadUrlRequest, DocumentPatch"
|
||||
pattern: "from api.documents.shared import"
|
||||
---
|
||||
|
||||
<objective>
|
||||
Decompose the 852-line `backend/api/documents.py` monolith into the focused package `backend/api/documents/` per locked D-06 (4 sub-modules) and D-08 (re-classify endpoint placement is researcher/planner choice — placed in `crud.py` because it operates on an existing document with the same ownership-check pattern as get/patch/delete). Zero URL changes, zero behavior changes.
|
||||
|
||||
Purpose: CODE-02 (decomposition) + CODE-08 (single-definition discipline for `UploadUrlRequest`, `DocumentPatch`, `_CLOUD_PROVIDERS`).
|
||||
|
||||
Output: `backend/api/documents/` package; `backend/api/documents.py` monolith deleted.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@$HOME/.claude/get-shit-done/workflows/execute-plan.md
|
||||
@$HOME/.claude/get-shit-done/templates/summary.md
|
||||
</execution_context>
|
||||
|
||||
<context>
|
||||
@.planning/PROJECT.md
|
||||
@.planning/ROADMAP.md
|
||||
@.planning/STATE.md
|
||||
@.planning/phases/08-stack-upgrade-backend-decomposition/08-CONTEXT.md
|
||||
@.planning/phases/08-stack-upgrade-backend-decomposition/08-RESEARCH.md
|
||||
@.planning/phases/08-stack-upgrade-backend-decomposition/08-PATTERNS.md
|
||||
@backend/api/documents.py
|
||||
@backend/api/folders.py
|
||||
@backend/main.py
|
||||
@CLAUDE.md
|
||||
|
||||
<interfaces>
|
||||
Endpoint to sub-module map (locked per RESEARCH.md §"api/documents.py → api/documents/ package"):
|
||||
|
||||
upload.py: request_upload_url (POST /upload-url, line 93-135), upload_document (POST /upload, line 137-296),
|
||||
confirm_upload (POST /{id}/confirm, line 298-404)
|
||||
crud.py: list_documents (GET "", line 406-529), get_document (GET /{id}, line 531-576),
|
||||
patch_document (PATCH /{id}, line 578-630), delete_document (DELETE /{id}, line 632-705),
|
||||
classify_document (POST /{id}/classify, line 707-742) — placed here per D-08
|
||||
content.py: stream_document_content (GET /{id}/content, line 765 onwards) + _parse_range helper (line 744-760)
|
||||
|
||||
Pydantic model + constant to file map:
|
||||
|
||||
shared.py: _CLOUD_PROVIDERS frozenset (line 59), UploadUrlRequest (line 66-69), DocumentPatch (line 71-88)
|
||||
|
||||
Package aggregator pattern for backend/api/documents/__init__.py:
|
||||
|
||||
from fastapi import APIRouter
|
||||
from api.documents.upload import router as upload_router
|
||||
from api.documents.crud import router as crud_router
|
||||
from api.documents.content import router as content_router
|
||||
|
||||
router = APIRouter(prefix="/api/documents", tags=["documents"])
|
||||
router.include_router(upload_router)
|
||||
router.include_router(crud_router)
|
||||
router.include_router(content_router)
|
||||
</interfaces>
|
||||
</context>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 1: Create backend/api/documents/ package — shared.py, upload.py, crud.py, content.py, __init__.py</name>
|
||||
<files>backend/api/documents/__init__.py, backend/api/documents/shared.py, backend/api/documents/upload.py, backend/api/documents/crud.py, backend/api/documents/content.py</files>
|
||||
<read_first>
|
||||
- backend/api/documents.py lines 1-852 (full source — every endpoint, every Pydantic model, every helper, every import)
|
||||
- backend/api/folders.py (ownership assertion pattern: `if doc is None or doc.user_id != current_user.id: raise HTTPException(status_code=404, ...)`)
|
||||
- .planning/phases/08-stack-upgrade-backend-decomposition/08-PATTERNS.md sections for `backend/api/documents/*` (exact import lists, sub-router declaration, ownership assertion pattern)
|
||||
- CLAUDE.md §"Key Architectural Rules" (atomic quota UPDATE pattern, ownership checks; preserved verbatim)
|
||||
</read_first>
|
||||
<action>
|
||||
This task creates the package and ALL five files. Execute in this order so the package is in a valid state on disk after each sub-step:
|
||||
|
||||
(A) Create `backend/api/documents/shared.py`. Add `from __future__ import annotations`. Import `from typing import Optional` and `from pydantic import BaseModel, field_validator`. Move the constant `_CLOUD_PROVIDERS = frozenset({"google_drive", "onedrive", "nextcloud", "webdav"})` from `documents.py` line 59. Move `UploadUrlRequest` (line 66-69) and `DocumentPatch` (line 71-88) verbatim, including the `filename_no_path_separators` validator which is a security validator at the API boundary (RESEARCH.md confirms it stays in the Pydantic model — D-11 analysis).
|
||||
|
||||
(B) Create `backend/api/documents/upload.py`. Header: `from __future__ import annotations`. Imports per PATTERNS.md §"backend/api/documents/upload.py" plus `from api.documents.shared import UploadUrlRequest, _CLOUD_PROVIDERS`. `router = APIRouter()` — NO prefix (D-04). Move handler functions verbatim including decorators: `request_upload_url` (lines 93-135), `upload_document` (lines 137-296), `confirm_upload` (lines 298-404). Every handler injects `current_user: User = Depends(get_current_user)` and asserts ownership on any pre-existing document lookup using the pattern `if doc is None or doc.user_id != current_user.id: raise HTTPException(status_code=404, ...)`.
|
||||
|
||||
(C) Create `backend/api/documents/crud.py`. Header + imports per PATTERNS.md §"backend/api/documents/crud.py" plus `from api.documents.shared import DocumentPatch, _CLOUD_PROVIDERS`. `router = APIRouter()` — NO prefix. Move handlers verbatim: `list_documents` (406-529), `get_document` (531-576), `patch_document` (578-630), `delete_document` (632-705), `classify_document` (707-742) — placed here per D-08. Each handler injects `current_user: User = Depends(get_current_user)` and includes the ownership assertion.
|
||||
|
||||
(D) Create `backend/api/documents/content.py`. Header: `from __future__ import annotations`. Imports per PATTERNS.md §"backend/api/documents/content.py" (FastAPI APIRouter, Depends, HTTPException, Request; FastAPI responses StreamingResponse; SQLAlchemy AsyncSession; deps.auth.get_current_user; deps.db.get_db). `router = APIRouter()` — NO prefix. Move `_parse_range(range_header: str, file_size: int)` helper verbatim from line 744-760 as a private module-level function. Move `stream_document_content` handler verbatim from line 765 onwards. The handler injects `current_user: User = Depends(get_current_user)` and asserts ownership before serving bytes (DOC-04 / SEC-04).
|
||||
|
||||
(E) Create `backend/api/documents/__init__.py`. Contents per PATTERNS.md §"backend/api/documents/__init__.py": import APIRouter from fastapi, import `router as upload_router`, `router as crud_router`, `router as content_router` from the three sub-modules, declare `router = APIRouter(prefix="/api/documents", tags=["documents"])`, call `router.include_router(upload_router)`, `router.include_router(crud_router)`, `router.include_router(content_router)`. ONLY aggregation — no helpers, no models, no logic.
|
||||
|
||||
To avoid Python's package-vs-module name collision, immediately after creating the package directory, rename `backend/api/documents.py` to `backend/api/documents_OLD_REMOVE_IN_TASK_2.py`. Task 2 verifies tests pass and then deletes the renamed file.
|
||||
|
||||
The route registration order matters: `crud.py` registers `GET ""` (matches `/api/documents`), and `crud.py` registers `GET /{doc_id}` which could shadow `upload.py`'s `POST /upload-url` if route precedence is wrong. FastAPI matches by exact path + method, so HTTP-method differentiation prevents collision; verify by listing routes after include.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd backend && python -c "from api.documents import router; paths = sorted({(r.path, list(r.methods)[0] if r.methods else '') for r in router.routes}); print('\n'.join(f'{m} {p}' for p, m in paths))"</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- Files exist: `backend/api/documents/__init__.py`, `backend/api/documents/shared.py`, `backend/api/documents/upload.py`, `backend/api/documents/crud.py`, `backend/api/documents/content.py`
|
||||
- `grep -v '^#' backend/api/documents/__init__.py | grep -c 'router = APIRouter(prefix="/api/documents"'` returns 1
|
||||
- `grep -v '^#' backend/api/documents/upload.py | grep -c 'router = APIRouter()'` returns 1 (NO prefix)
|
||||
- `grep -v '^#' backend/api/documents/crud.py | grep -c 'router = APIRouter()'` returns 1 (NO prefix)
|
||||
- `grep -v '^#' backend/api/documents/content.py | grep -c 'router = APIRouter()'` returns 1 (NO prefix)
|
||||
- `grep -c "_CLOUD_PROVIDERS = frozenset" backend/api/documents/shared.py` returns 1
|
||||
- `grep -rn "_CLOUD_PROVIDERS = frozenset" backend/api/documents/` returns exactly one match (in shared.py)
|
||||
- `grep -c "Depends(get_current_user)" backend/api/documents/upload.py` ≥ 3
|
||||
- `grep -c "Depends(get_current_user)" backend/api/documents/crud.py` ≥ 5
|
||||
- `grep -c "Depends(get_current_user)" backend/api/documents/content.py` ≥ 1
|
||||
- `grep -c "doc.user_id != current_user.id\\|user_id != current_user.id" backend/api/documents/crud.py` ≥ 3 (ownership checks on get/patch/delete and classify)
|
||||
- `cd backend && python -c "from api.documents import router; assert len(router.routes) >= 9"` exits 0 (3 upload + 5 crud + 1 content = 9)
|
||||
</acceptance_criteria>
|
||||
<done>Package exists with all 5 files; sub-router count totals 9; no sub-router carries a prefix; ownership checks preserved; monolith renamed (not yet deleted).</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 2: Run URL regression suite, then delete the old monolith</name>
|
||||
<files>backend/api/documents.py</files>
|
||||
<read_first>
|
||||
- backend/tests/test_documents.py (full file — confirms which endpoints are exercised)
|
||||
- backend/api/documents_OLD_REMOVE_IN_TASK_2.py (renamed monolith from task 1)
|
||||
</read_first>
|
||||
<action>
|
||||
Run `cd backend && pytest tests/test_documents.py -x -v` to confirm every document URL still responds correctly (URLs unchanged, response shapes unchanged, ownership 404s preserved, quota behavior preserved). If ANY test fails, do NOT delete the renamed monolith — diagnose, fix, and re-run. Only after `tests/test_documents.py` and `tests/test_shares.py` (because shares.py uses Document lookups) both pass, delete `backend/api/documents_OLD_REMOVE_IN_TASK_2.py`. Re-run the full backend suite `cd backend && pytest -v` to confirm no other tests regressed.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd backend && pytest tests/test_documents.py tests/test_shares.py -x -v 2>&1 | tail -15 && test ! -f backend/api/documents.py && test ! -f backend/api/documents_OLD_REMOVE_IN_TASK_2.py && echo "old monolith deleted"</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `test ! -f backend/api/documents.py` exits 0
|
||||
- `test ! -f backend/api/documents_OLD_REMOVE_IN_TASK_2.py` exits 0
|
||||
- `test -d backend/api/documents` exits 0
|
||||
- `cd backend && pytest tests/test_documents.py -x` exits 0
|
||||
- `cd backend && pytest tests/test_shares.py -x` exits 0
|
||||
- `cd backend && pytest -v` exits 0 with no new failures vs. baseline
|
||||
- `grep -rn "^class UploadUrlRequest" backend/api/documents/` returns exactly one match (in shared.py)
|
||||
- `grep -rn "^class DocumentPatch" backend/api/documents/` returns exactly one match (in shared.py)
|
||||
</acceptance_criteria>
|
||||
<done>Old documents.py file deleted; full backend suite green; Pydantic models defined exactly once.</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<threat_model>
|
||||
## Trust Boundaries
|
||||
|
||||
| Boundary | Description |
|
||||
|----------|-------------|
|
||||
| Regular user → document endpoints | Every handler enforces `get_current_user` AND `resource.user_id == current_user.id` to prevent IDOR (DOC-04 / SEC-04) |
|
||||
| Browser → MinIO presigned PUT URL | Generated server-side scoped to `{user_id}/{document_id}/{uuid4}`; never includes admin or other user paths |
|
||||
| Range-header → byte offset arithmetic | `_parse_range` must reject malformed input; copied verbatim from monolith |
|
||||
|
||||
## STRIDE Threat Register
|
||||
|
||||
| Threat ID | Category | Component | Disposition | Mitigation Plan |
|
||||
|-----------|----------|-----------|-------------|-----------------|
|
||||
| T-08-05-01 | Spoofing/Elevation | IDOR on document endpoints | mitigate | All handlers copied verbatim including `current_user.id` ownership assertion. Acceptance criterion grep counts ≥ 3 ownership checks in crud.py. |
|
||||
| T-08-05-02 | Tampering | Sub-router prefix doubling | mitigate | All three sub-routers declare `router = APIRouter()` with NO prefix; only `__init__.py` carries `prefix="/api/documents"`. |
|
||||
| T-08-05-03 | Information Disclosure | DocumentPatch.filename validator | mitigate | `filename_no_path_separators` validator preserved verbatim in shared.py (path traversal defense). |
|
||||
| T-08-05-04 | Tampering | Atomic quota UPDATE invariant | mitigate | `confirm_upload` and `delete_document` copied verbatim — atomic `UPDATE quotas SET used_bytes = used_bytes + $delta WHERE …` pattern preserved (CLAUDE.md non-negotiable). |
|
||||
| T-08-05-05 | Denial of Service | Circular import via `__init__.py` | mitigate | `__init__.py` does ONLY aggregation; shared models in `shared.py`. |
|
||||
| T-08-05-SC | Supply Chain | No new packages | accept | This plan installs zero new packages. |
|
||||
</threat_model>
|
||||
|
||||
<verification>
|
||||
- `cd backend && pytest tests/test_documents.py tests/test_shares.py -x -v` — zero failures
|
||||
- `cd backend && pytest -v` — full suite zero failures
|
||||
- `cd backend && python -c "from main import app; doc_paths = sorted({r.path for r in app.routes if r.path.startswith('/api/documents')}); print('\\n'.join(doc_paths))"` lists at minimum: `/api/documents`, `/api/documents/upload-url`, `/api/documents/upload`, `/api/documents/{doc_id}`, `/api/documents/{doc_id}/confirm`, `/api/documents/{doc_id}/classify`, `/api/documents/{doc_id}/content`
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- `backend/api/documents/` package with 5 files
|
||||
- `backend/api/documents.py` deleted
|
||||
- All document URL paths unchanged, response shapes unchanged
|
||||
- Every handler preserves ownership assertion
|
||||
- All tests pass
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create `.planning/phases/08-stack-upgrade-backend-decomposition/08-05-SUMMARY.md` when done. Include: (a) full list of document paths emitted by `app.routes` before vs. after (must be identical), (b) `tests/test_documents.py` test count, (c) confirmation that the `_CLOUD_PROVIDERS` constant exists exactly once in the package.
|
||||
</output>
|
||||
Reference in New Issue
Block a user