Files
ducoterra 990c8adf13
Build and Push Containers / build-and-push-app (push) Successful in 2m12s
Build and Push Containers / build-and-push-db (push) Successful in 14s
phase: 115_doc_draft_discard
All green — this was the final verification pass; everything from the four completed tasks was already in the working tree and verified.

**Phase 115 — Doc drafts: Discard + DELETE route + title fix — verification report**

- Verified all 4 task deliverables present: DELETE route (`app/api/doc_drafts.py`), Discard UI (`doc-edit.html` + `doc-edit.js` + `.discard-draft` CSS), title fix (`defaultDocTitle(wrap)` pairing + `saveAsDoc` call site), and all test pins (integration, frontend unit, E2E). No code changes needed.
- **Completion criteria:**
  1. ✅ Orphaned draft discardable from edit screen; row gone — `test_delete_removes_row_and_invalidates_token` (204 → GET 404), unknown-token 404, admin-gate 403 on all routes, E2E `test_discard_draft_from_edit_screen` all pass.
  2. ✅ Title after retry redo = redone answer's own question — E2E `test_save_title_is_the_redo_question_after_retry` passes.
  3. ✅ Push flow byte-identical — `git diff` shows only the new DELETE route + module docstring; all 7 existing push tests green.
  4. ✅ `uv run pytest --cov=app` → **2457 passed**, app coverage **99%** (>90%); `uv run pytest tests/e2e/test_save_doc_session.py -v --no-cov` → **4 passed**; `uv run ruff check .` → clean; `uv run pyright` → 0 errors.
  5. ⏳ Commit + phase-dir move left to the harness (per executor rules, no `git` run; all changes left in the working tree).
- No defects found; no deviations.
- Next pending phase: none in `todo/` other than this one (`115_doc_draft_discard` is the last).
2026-09-15 05:35:05 -04:00

326 lines
13 KiB
Python

"""Doc-drafts API — the draft lifecycle the edit screen runs on
(phase 59, task 02).
The "Save as doc" action (task 05) POSTs a completed answer here, the
edit screen (task 06) GETs/PUTs it by token, and the push endpoint
(task 04) commits + pushes it. A draft's long answer body lives on the
**server** — never in a URL: the row is keyed by an unguessable 128-bit
``uuid4`` ``token`` (the edit screen's URL credential,
``/doc-edit.html?draft=<token>`` — the share-token trust model,
phase 51).
The whole router sits behind :func:`app.core.auth.require_admin`
(router-wide ``dependencies`` — the :mod:`app.api.steering` pattern):
drafts are admin-only, so anonymous callers get 403 on every route.
Routes: ``POST /api/doc-drafts`` (create — 201, ``token = uuid4``,
``status = "draft"``), ``GET /api/doc-drafts/{token}`` (fetch by
token — 404 ``draft not found`` when unknown), ``PUT
/api/doc-drafts/{token}`` (partial update — absent fields unchanged,
``updated_at`` bumped; editing a ``pushed`` draft resets ``status``
back to ``draft`` — the stored sha no longer describes the current
body, so the next push re-commits; phase 59 D3 ASSUMPTION), ``DELETE
/api/doc-drafts/{token}`` (discard — 204 No Content; the row is
removed, so every subsequent GET/PUT/push 404s; phase 115, the edit
screen's Discard control), ``POST
/api/doc-drafts/{token}/push`` (the single mutation the edit screen
triggers — commit + ``git push`` the draft's file to the
``BOR_DOCS_REPO`` ``BOR_DOCS_BRANCH`` via
:func:`app.core.docs_push.push_document`; success records
``status`` / ``branch`` / ``commit_sha`` on the row and returns
``DocDraftPushed``; 409 while unconfigured, 422 on a path that no
longer passes the guard-rails, 502 on git failure with the
SANITIZED git stderr in the detail (credential ``user:pass@``
userinfo masked by :func:`app.core.errors.sanitize_error` — phase
84, SEC-08) — the row untouched).
Every ``path`` (create, update **and** push) passes the shared
:func:`validate_draft_path` guard, so no draft can ever be created,
edited or pushed with a path that escapes the repo root.
"""
from __future__ import annotations
import uuid
from pathlib import Path
from fastapi import APIRouter, Depends, HTTPException
from sqlalchemy import select, text
from sqlalchemy.orm import Session
from app.config import Settings, get_settings
from app.core.auth import require_admin
from app.core.docs_push import DocsPushError, push_document
from app.core.errors import sanitize_error
from app.db import get_db
from app.models import DocDraft
from app.schemas import DocDraft as DocDraftOut
from app.schemas import (
DocDraftCreate,
DocDraftPushed,
DocDraftUpdate,
)
router = APIRouter(
prefix="/doc-drafts",
tags=["doc-drafts"],
dependencies=[Depends(require_admin)], # phase 59: drafts are admin-only
)
def validate_draft_path(raw: str) -> str:
"""Guard-rail an in-repo doc path (phase 59, task 02).
Shared by the create/update routes **and** the push endpoint (task
04): the file will later be written inside the ``BOR_DOCS_REPO``
checkout, so a path that escapes the repo root is a security hole,
not a typo. Rules (first violation wins, each 422 names its rule):
* non-empty after strip — a blank path names no file;
* not absolute — ``/etc/passwd`` would leave the checkout;
* no ``.``/``..`` components — ``../x.md`` and ``a/b/../c.md`` walk
out of the checkout (checked via ``Path(p).parts``, so the
traversal is rejected wherever it sits);
* carries a file suffix (``Path(p).suffix`` non-empty) — the
committed file must be a real file, e.g. ``docs/note.md``.
Returns the stripped path (the value that gets stored); raises
:class:`fastapi.HTTPException` (422) on the first violated rule.
"""
path = raw.strip()
if not path:
raise HTTPException(status_code=422, detail="path must not be empty")
if Path(path).is_absolute():
raise HTTPException(status_code=422, detail="path must not be an absolute path")
if any(part in (".", "..") for part in Path(path).parts):
raise HTTPException(
status_code=422,
detail="path must not contain '.' or '..' path components",
)
if not Path(path).suffix:
raise HTTPException(
status_code=422, detail="path must carry a file suffix (e.g. docs/note.md)"
)
return path
def _to_out(row: DocDraft) -> DocDraftOut:
"""The full-payload response shape (create/get/put)."""
return DocDraftOut(
token=row.token,
title=row.title,
path=row.path,
body=row.body,
status=row.status,
branch=row.branch,
commit_sha=row.commit_sha,
created_at=row.created_at,
updated_at=row.updated_at,
)
def _get_draft_or_404(db: Session, token: uuid.UUID) -> DocDraft:
"""One draft by its URL credential; 404 when the token is unknown."""
row = db.execute(select(DocDraft).where(DocDraft.token == token)).scalars().first()
if row is None:
raise HTTPException(status_code=404, detail="draft not found")
return row
@router.post("", response_model=DocDraftOut, status_code=201)
def create_draft(
payload: DocDraftCreate,
db: Session = Depends(get_db), # noqa: B008
) -> DocDraftOut:
"""Store one completed answer as a draft (201).
``title`` / ``body`` must be non-empty after strip (422 — the
``SteeringNoteIn`` pattern; pydantic's ``min_length=1`` alone would
let a whitespace-only value through). ``path`` passes
:func:`validate_draft_path` (422 naming the violated rule).
``token`` is a fresh ``uuid4`` — set on the PENDING row, so it ships
in the same INSERT (the ``share_token`` precedent, phase 51);
``status`` starts as ``draft``.
"""
title = payload.title.strip()
if not title:
raise HTTPException(status_code=422, detail="title must not be empty")
body = payload.body.strip()
if not body:
raise HTTPException(status_code=422, detail="body must not be empty")
path = validate_draft_path(payload.path)
row = DocDraft(
token=uuid.uuid4(), # the URL credential — set before INSERT
title=title,
path=path,
body=body,
)
db.add(row)
db.commit()
db.refresh(row)
return _to_out(row)
@router.get("/{token}", response_model=DocDraftOut)
def get_draft(
token: uuid.UUID,
db: Session = Depends(get_db), # noqa: B008
) -> DocDraftOut:
"""One draft by its URL credential (the edit screen's load); 404
``draft not found`` when the token is unknown (no enumeration — one
message for every unknown token)."""
return _to_out(_get_draft_or_404(db, token))
@router.put("/{token}", response_model=DocDraftOut)
def update_draft(
token: uuid.UUID,
payload: DocDraftUpdate,
db: Session = Depends(get_db), # noqa: B008
) -> DocDraftOut:
"""Partial edit of a draft (the edit screen's Save); 404 when the
token is unknown.
Each field is replaced only when supplied (absent keeps the row's
current value; present must be non-empty after strip — 422). A
supplied ``path`` re-runs :func:`validate_draft_path`; the same
guard-rails apply on update as on create. ``updated_at`` is bumped
on every PUT — via the model's ``onupdate=func.now()`` when a stored
value changes, via an explicit raw ``UPDATE`` when the PUT is a
no-op (empty body, or every supplied value identical — the ORM
flushes nothing, so the onupdate default never fires).
Editing a ``pushed`` draft resets ``status`` to ``draft`` (phase 59
D3 ASSUMPTION): the stored ``commit_sha`` no longer describes the
current body, so the next push re-commits. The last push's
``branch``/``commit_sha`` stay visible (the previous state) until
the next push overwrites them.
"""
row = _get_draft_or_404(db, token)
# Validate everything BEFORE assigning anything, so a 422 on one
# field can never leave a half-applied edit pending.
new_title = payload.title.strip() if payload.title is not None else None
if payload.title is not None and not new_title:
raise HTTPException(status_code=422, detail="title must not be empty")
new_path = validate_draft_path(payload.path) if payload.path is not None else None
new_body = payload.body.strip() if payload.body is not None else None
if payload.body is not None and not new_body:
raise HTTPException(status_code=422, detail="body must not be empty")
changed = False
if new_title is not None and row.title != new_title:
row.title = new_title
changed = True
if new_path is not None and row.path != new_path:
row.path = new_path
changed = True
if new_body is not None and row.body != new_body:
row.body = new_body
changed = True
if row.status == "pushed":
# D3 ASSUMPTION — see the docstring above.
row.status = "draft"
changed = True
if not changed:
# No-op PUT: the onupdate default does not fire for a flush that
# changed nothing, so bump updated_at explicitly (raw SQL — the
# ORM object is already in its final state).
db.execute(
text("UPDATE doc_drafts SET updated_at = now() WHERE id = :id"),
{"id": row.id},
)
db.commit()
db.refresh(row)
return _to_out(row)
@router.delete("/{token}", status_code=204)
def delete_draft(
token: uuid.UUID,
db: Session = Depends(get_db), # noqa: B008
) -> None:
"""Discard a draft (the edit screen's Discard control, phase 115).
Admin-gated like the whole router (phase 59); the uuid4 token is
the screen's credential — after a successful discard, GET/PUT/push
all 404. 204 No Content (no body): the token is a one-way
credential and nothing else references the row (no FK targets, no
push-side state — the git push happens only on push).
"""
row = _get_draft_or_404(db, token)
db.delete(row)
db.commit()
@router.post("/{token}/push", response_model=DocDraftPushed)
def push_doc_draft(
token: uuid.UUID,
db: Session = Depends(get_db), # noqa: B008
settings: Settings = Depends(get_settings), # noqa: B008
) -> DocDraftPushed:
"""Commit + push the draft's file (phase 59, task 04).
The single mutation the edit screen (task 06) triggers: take the
draft's (title, path, body), commit + push through
:func:`app.core.docs_push.push_document`, and record the outcome
on the draft. Outcomes (checked in this order):
1. unknown token → 404 ``draft not found`` (same message as GET/
PUT — no enumeration);
2. ``settings.docs_configured`` false → 409 naming
``BOR_DOCS_REPO`` (D3: the feature is inert by default — the
optional-feature pattern of the git-sources env fallback);
3. the stored ``path`` re-runs :func:`validate_draft_path` → 422
on the first violated rule (a row must not be pushable into a
bad path, whatever wrote it);
4. :class:`DocsPushError` → 502 with the SANITIZED git stderr in
the detail (phase 84, SEC-08): ``str(exc)`` runs through
:func:`app.core.errors.sanitize_error`, which masks any
``user:pass@`` userinfo git's stderr may echo (a
``BOR_DOCS_REPO`` URL with embedded credentials) — the failing
repo and git's reason stay readable, and the
``GitSyncError`` → ``detail`` mapping from
:mod:`app.api.git_sources` is otherwise kept. Only a SUCCESS
mutates the row: the failed push leaves ``status`` / ``branch``
/ ``commit_sha`` exactly as found (no partial commit).
On success the row becomes ``status = "pushed"`` with the landed
``branch`` and ``commit_sha`` (must equal ``git rev-parse
<branch>`` in the repo) and a bumped ``updated_at`` (the column's
``onupdate`` fires — a successful push always rewrites at least
``commit_sha``, so a flush always happens); the response is
``DocDraftPushed(status="pushed", branch, commit_sha=sha)`` — the
edit screen's branch + sha feedback (D3: no PR, no URL — the
owner opens the PR themselves).
"""
row = _get_draft_or_404(db, token)
if not settings.docs_configured:
raise HTTPException(
status_code=409, detail="docs repo not configured (BOR_DOCS_REPO)"
)
path = validate_draft_path(row.path)
try:
branch, sha = push_document(
repo=settings.docs_repo,
base_branch=settings.docs_base_branch,
branch=settings.docs_branch,
work_dir=settings.docs_work_dir,
rel_path=path,
content=row.body,
commit_message=f"docs: {row.title}",
)
except DocsPushError as exc:
# Phase 84 (SEC-08): git's stderr may echo the remote URL with
# embedded credentials — the RESPONSE is where the secret would
# leak, so it is sanitized here (the DocsPushError itself keeps
# carrying the full stderr for logs/inspection).
raise HTTPException(status_code=502, detail=sanitize_error(str(exc))) from None
row.status = "pushed"
row.branch = branch
row.commit_sha = sha
db.commit()
db.refresh(row)
return DocDraftPushed(status="pushed", branch=branch, commit_sha=sha)