diff --git a/docs/specs/ops-specs-features/tasks.md b/docs/specs/ops-specs-features/tasks.md index c15c5ba01..91757100c 100644 --- a/docs/specs/ops-specs-features/tasks.md +++ b/docs/specs/ops-specs-features/tasks.md @@ -50,15 +50,15 @@ GET endpoints. Backend first; frontend follows. ## Phase 2 — Status-flip write-side -- [ ] **2.1** Add `PUT /api/specs/{slug}/{phase}/status` route +- [x] **2.1** Add `PUT /api/specs/{slug}/{phase}/status` route — same body as attune-gui's (`{"status": ""}`) -- [ ] **2.2** Reuse attune-gui's `_STATUS_RE` and +- [x] **2.2** Reuse attune-gui's `_STATUS_RE` and `_VALID_STATUSES` patterns directly (or copy with attribution comment) -- [ ] **2.3** Honor the `--read-only` flag added in PR #227 — +- [x] **2.3** Honor the `--read-only` flag added in PR #227 — status flip is a mutation, so blocked in read-only mode -- [ ] **2.4** Atomic write via existing +- [x] **2.4** Atomic write via existing `attune.ops` write helpers (or port `atomic_write` from attune-gui) diff --git a/src/attune/ops/routes/specs.py b/src/attune/ops/routes/specs.py index 6f7e655a4..7b897fb45 100644 --- a/src/attune/ops/routes/specs.py +++ b/src/attune/ops/routes/specs.py @@ -1,28 +1,33 @@ -"""Specs API — read-only endpoints for federated spec listing + drill-in. +"""Specs API — endpoints for federated spec listing, drill-in, and status flip. -Phase 1 of docs/specs/ops-specs-features/. Mirrors attune-gui's -`sidecar/attune_gui/routes/cowork_specs.py` GET endpoints for the -listing + read paths; status flip and other write tools come in -Phase 2. +Phases 1 + 2 of docs/specs/ops-specs-features/. Mirrors attune-gui's +`sidecar/attune_gui/routes/cowork_specs.py` GET endpoints (Phase 1) +and the PUT status-flip endpoint (Phase 2). The endpoints: - GET /api/specs — list all specs across configured roots - with phase status for each - GET /api/specs/{slug} — return content of phase files for one spec + GET /api/specs — list specs across roots + GET /api/specs/{slug} — read phase-file contents + PUT /api/specs/{slug}/{phase}/status — rewrite **Status** line Roots are configured via the `--specs-root` CLI flag on `attune ops` (defaults to `/docs/specs/`). Multiple roots are supported for federated listing across project boundaries. + +The status-flip endpoint is gated by ``config.allow_run`` — when the +server runs with ``--read-only``, mutations are rejected with 403. """ from __future__ import annotations +import os import re +import tempfile from dataclasses import asdict, dataclass from pathlib import Path +from typing import Any -from fastapi import APIRouter, HTTPException, Request +from fastapi import APIRouter, Body, HTTPException, Request router = APIRouter(prefix="/api/specs", tags=["specs"]) @@ -47,6 +52,26 @@ re.MULTILINE, ) +# Phase file names (without `.md`) accepted by the status-flip endpoint. +# Derived from `_PHASE_FILES` so the two stay in lockstep. +_VALID_PHASES: tuple[str, ...] = tuple(p.removesuffix(".md") for p in _PHASE_FILES) + +# Statuses we accept on a PUT — mirrors attune-gui's set so callers can use +# the same vocabulary across both dashboards. "completed" and "done" are +# accepted aliases for "complete". +_VALID_STATUSES: tuple[str, ...] = ( + "draft", + "in-review", + "approved", + "complete", + "completed", + "done", +) + +# Slug rule: lowercase letters/digits/dashes, must start with letter/digit, +# max 63 chars. Matches attune-gui's `_SLUG_RE` so the two endpoints agree. +_SLUG_RE = re.compile(r"^[a-z0-9][a-z0-9-]{0,62}$") + @dataclass(frozen=True) class SpecPhase: @@ -196,3 +221,142 @@ async def get_spec(slug: str, request: Request) -> dict: "contents": contents, } raise HTTPException(status_code=404, detail=f"spec '{slug}' not found in any configured root") + + +# --------------------------------------------------------------------------- +# Phase 2 — status-flip write API +# --------------------------------------------------------------------------- + + +def _validate_slug(slug: str) -> None: + """Reject slugs that fail the directory-name shape check.""" + if not _SLUG_RE.match(slug): + raise HTTPException( + status_code=400, + detail=( + "invalid slug: must be lowercase letters/digits/dashes, " + "start with a letter or digit, max 63 chars" + ), + ) + + +def _validate_phase_name(phase: str) -> None: + """Reject phase names not in the recognized set.""" + if phase not in _VALID_PHASES: + raise HTTPException( + status_code=400, + detail=f"unknown phase: {phase!r}. valid: {', '.join(_VALID_PHASES)}", + ) + + +def _validate_status_value(status: str) -> None: + """Reject status values outside the accepted vocabulary.""" + if status not in _VALID_STATUSES: + raise HTTPException( + status_code=400, + detail=f"invalid status: {status!r}. valid: {', '.join(_VALID_STATUSES)}", + ) + + +def _atomic_write(target: Path, text: str) -> None: + """Write ``text`` to ``target`` atomically via tempfile + os.replace. + + Mirrors attune-gui's `_fs.atomic_write` so a concurrent reader either + sees the old file or the new one, never a partial write. Cleans up the + temp file if anything raises before the rename lands. + """ + target.parent.mkdir(parents=True, exist_ok=True) + fd, tmp = tempfile.mkstemp(prefix=f".{target.name}.", suffix=".tmp", dir=str(target.parent)) + try: + with os.fdopen(fd, "w", encoding="utf-8") as f: + f.write(text) + os.replace(tmp, target) + except Exception: + Path(tmp).unlink(missing_ok=True) + raise + + +def _rewrite_status_line(original: str, status: str) -> str: + """Return ``original`` with its first **Status** line replaced by ``status``. + + If no recognized status line exists, inserts ``**Status:** {status}`` near + the top (after the first ``# `` heading if present). + """ + if _STATUS_RE.search(original): + return _STATUS_RE.sub(f"**Status:** {status}", original, count=1) + lines = original.splitlines() + insert_at = 1 if lines and lines[0].startswith("# ") else 0 + lines.insert(insert_at, f"\n**Status:** {status}\n") + new_text = "\n".join(lines) + if not new_text.endswith("\n"): + new_text += "\n" + return new_text + + +@router.put("/{slug}/{phase}/status") +async def update_phase_status( + slug: str, + phase: str, + request: Request, + body: dict[str, Any] = Body(...), # noqa: B008 +) -> dict[str, Any]: + """Rewrite the ``**Status**`` line in the named phase file. + + Body: ``{"status": ""}``. + + Gated on ``config.allow_run`` — when the server runs with + ``--read-only``, this returns 403. + """ + config = request.app.state.config + if not getattr(config, "allow_run", False): + raise HTTPException( + status_code=403, + detail="server is read-only; status flip is disabled", + ) + + _validate_slug(slug) + _validate_phase_name(phase) + + status = body.get("status") + if not isinstance(status, str): + raise HTTPException( + status_code=422, + detail="body must include `status` (string)", + ) + _validate_status_value(status) + + roots = _resolved_roots(config) + target: Path | None = None + matched_root: Path | None = None + for root in roots: + candidate = root / slug / f"{phase}.md" + if candidate.is_file(): + target = candidate + matched_root = root + break + if target is None or matched_root is None: + raise HTTPException( + status_code=404, + detail=f"{phase}.md not found for spec '{slug}' in any configured root", + ) + + try: + original = target.read_text(encoding="utf-8") + except OSError as exc: + raise HTTPException(status_code=500, detail=f"read failed: {exc}") from exc + + new_text = _rewrite_status_line(original, status) + + try: + _atomic_write(target, new_text) + except OSError as exc: + raise HTTPException(status_code=500, detail=f"write failed: {exc}") from exc + + return { + "slug": slug, + "phase": phase, + "file": f"{phase}.md", + "status": status, + "root": str(matched_root), + "path": str(target), + } diff --git a/tests/unit/ops/test_specs_routes.py b/tests/unit/ops/test_specs_routes.py index 362dfa4b8..ec0fef88a 100644 --- a/tests/unit/ops/test_specs_routes.py +++ b/tests/unit/ops/test_specs_routes.py @@ -37,8 +37,17 @@ def _make_spec(root: Path, slug: str, *, files: dict[str, str]) -> Path: return spec_dir -def _client(tmp_path: Path, *, specs_roots: tuple[Path, ...] | None = None) -> TestClient: - config = build_config(project_root=tmp_path, specs_roots=specs_roots) +def _client( + tmp_path: Path, + *, + specs_roots: tuple[Path, ...] | None = None, + allow_run: bool = False, +) -> TestClient: + config = build_config( + project_root=tmp_path, + specs_roots=specs_roots, + allow_run=allow_run, + ) return TestClient(create_app(config)) @@ -233,3 +242,173 @@ def test_config_accepts_multiple_specs_roots(tmp_path): b = tmp_path / "b" / "docs" / "specs" config = build_config(project_root=tmp_path, specs_roots=(a, b)) assert config.specs_roots == (a, b) + + +# --------------------------------------------------------------------------- +# PUT /api/specs/{slug}/{phase}/status — Phase 2 status flip +# --------------------------------------------------------------------------- + + +def test_put_status_flips_attune_ai_convention(tmp_path, monkeypatch): + """`**Status:** old` is rewritten to `**Status:** new` (colon-inside).""" + monkeypatch.chdir(tmp_path) + root = tmp_path / "specs" + _make_spec(root, "alpha", files={"tasks.md": "# Tasks\n\n**Status:** draft\n\nBody.\n"}) + client = _client(tmp_path, specs_roots=(root,), allow_run=True) + + r = client.put("/api/specs/alpha/tasks/status", json={"status": "approved"}) + + assert r.status_code == 200, r.text + payload = r.json() + assert payload["status"] == "approved" + assert payload["phase"] == "tasks" + assert payload["slug"] == "alpha" + updated = (root / "alpha" / "tasks.md").read_text(encoding="utf-8") + assert "**Status:** approved" in updated + assert "**Status:** draft" not in updated + assert "Body." in updated # body preserved + + +def test_put_status_flips_attune_gui_convention(tmp_path, monkeypatch): + """`**Status**: old` (colon-outside) is also recognized and rewritten.""" + monkeypatch.chdir(tmp_path) + root = tmp_path / "specs" + _make_spec(root, "beta", files={"design.md": "# Design\n\n**Status**: draft\n"}) + client = _client(tmp_path, specs_roots=(root,), allow_run=True) + + r = client.put("/api/specs/beta/design/status", json={"status": "complete"}) + + assert r.status_code == 200, r.text + updated = (root / "beta" / "design.md").read_text(encoding="utf-8") + assert "**Status:** complete" in updated + + +def test_put_status_inserts_when_missing(tmp_path, monkeypatch): + """Files without any **Status** line get one inserted near the top.""" + monkeypatch.chdir(tmp_path) + root = tmp_path / "specs" + _make_spec(root, "gamma", files={"requirements.md": "# Requirements\n\nNo status here.\n"}) + client = _client(tmp_path, specs_roots=(root,), allow_run=True) + + r = client.put("/api/specs/gamma/requirements/status", json={"status": "draft"}) + + assert r.status_code == 200, r.text + updated = (root / "gamma" / "requirements.md").read_text(encoding="utf-8") + assert "**Status:** draft" in updated + assert "# Requirements" in updated # H1 preserved + assert "No status here." in updated # body preserved + + +def test_put_status_accepts_done_and_completed_aliases(tmp_path, monkeypatch): + """`done` and `completed` are accepted alongside `complete`.""" + monkeypatch.chdir(tmp_path) + root = tmp_path / "specs" + _make_spec(root, "delta", files={"tasks.md": "**Status:** approved\n"}) + client = _client(tmp_path, specs_roots=(root,), allow_run=True) + + for status in ("done", "completed", "complete"): + r = client.put("/api/specs/delta/tasks/status", json={"status": status}) + assert r.status_code == 200, f"{status}: {r.text}" + + +def test_put_status_read_only_returns_403(tmp_path, monkeypatch): + """When allow_run is False (default --read-only), PUT is rejected.""" + monkeypatch.chdir(tmp_path) + root = tmp_path / "specs" + _make_spec(root, "alpha", files={"tasks.md": "**Status:** draft\n"}) + client = _client(tmp_path, specs_roots=(root,), allow_run=False) + + r = client.put("/api/specs/alpha/tasks/status", json={"status": "approved"}) + + assert r.status_code == 403, r.text + # File unchanged + assert (root / "alpha" / "tasks.md").read_text(encoding="utf-8") == "**Status:** draft\n" + + +def test_put_status_unknown_spec_returns_404(tmp_path, monkeypatch): + """Slug that doesn't exist in any root → 404.""" + monkeypatch.chdir(tmp_path) + root = tmp_path / "specs" + root.mkdir(parents=True) + client = _client(tmp_path, specs_roots=(root,), allow_run=True) + + r = client.put("/api/specs/missing/tasks/status", json={"status": "draft"}) + + assert r.status_code == 404, r.text + + +def test_put_status_unknown_phase_returns_400(tmp_path, monkeypatch): + """Phase name outside the recognized set → 400 before any FS work.""" + monkeypatch.chdir(tmp_path) + root = tmp_path / "specs" + _make_spec(root, "alpha", files={"tasks.md": "**Status:** draft\n"}) + client = _client(tmp_path, specs_roots=(root,), allow_run=True) + + r = client.put("/api/specs/alpha/notarealphase/status", json={"status": "draft"}) + + assert r.status_code == 400, r.text + assert "unknown phase" in r.json()["detail"].lower() + + +def test_put_status_invalid_status_value_returns_400(tmp_path, monkeypatch): + """Status value outside the accepted vocabulary → 400.""" + monkeypatch.chdir(tmp_path) + root = tmp_path / "specs" + _make_spec(root, "alpha", files={"tasks.md": "**Status:** draft\n"}) + client = _client(tmp_path, specs_roots=(root,), allow_run=True) + + r = client.put("/api/specs/alpha/tasks/status", json={"status": "WIP"}) + + assert r.status_code == 400, r.text + assert "invalid status" in r.json()["detail"].lower() + + +def test_put_status_missing_status_field_returns_422(tmp_path, monkeypatch): + """Body without a `status` string → 422 (Unprocessable Entity).""" + monkeypatch.chdir(tmp_path) + root = tmp_path / "specs" + _make_spec(root, "alpha", files={"tasks.md": "**Status:** draft\n"}) + client = _client(tmp_path, specs_roots=(root,), allow_run=True) + + r = client.put("/api/specs/alpha/tasks/status", json={"other": "value"}) + + assert r.status_code == 422, r.text + + +@pytest.mark.parametrize( + "slug", + ["../escape", "weird/slash", "back\\slash", "UPPERCASE", "-leading-dash", "with space"], +) +def test_put_status_rejects_invalid_slug(tmp_path, monkeypatch, slug): + """Slugs that don't match the directory-name shape are rejected as 400.""" + monkeypatch.chdir(tmp_path) + root = tmp_path / "specs" + root.mkdir(parents=True) + client = _client(tmp_path, specs_roots=(root,), allow_run=True) + + r = client.put(f"/api/specs/{slug}/tasks/status", json={"status": "draft"}) + + # FastAPI may also return 404 for some path-traversal-shaped slugs that + # don't reach the route handler — accept either rejection. + assert r.status_code in (400, 404), f"slug={slug!r} status={r.status_code} body={r.text}" + + +def test_put_status_first_root_wins_on_collision(tmp_path, monkeypatch): + """When the same slug exists in multiple roots, PUT writes to the first.""" + monkeypatch.chdir(tmp_path) + root_a = tmp_path / "a" + root_b = tmp_path / "b" + _make_spec(root_a, "shared", files={"tasks.md": "**Status:** draft\n"}) + _make_spec(root_b, "shared", files={"tasks.md": "**Status:** approved\n"}) + client = _client(tmp_path, specs_roots=(root_a, root_b), allow_run=True) + + r = client.put("/api/specs/shared/tasks/status", json={"status": "complete"}) + + assert r.status_code == 200, r.text + assert r.json()["root"] == str(root_a) + # Only root_a was rewritten + assert "**Status:** complete" in (root_a / "shared" / "tasks.md").read_text(encoding="utf-8") + # root_b is untouched + assert (root_b / "shared" / "tasks.md").read_text(encoding="utf-8") == ( + "**Status:** approved\n" + )