diff --git a/backend/scripts/sync_from_store.sh b/backend/scripts/sync_from_store.sh index 1e758e9a8..78ea05580 100755 --- a/backend/scripts/sync_from_store.sh +++ b/backend/scripts/sync_from_store.sh @@ -22,6 +22,11 @@ if [[ -z "${STORE:-}" || ! -d "$STORE/internal" ]]; then exit 1 fi +# Before the port into the BaseScanning monorepo the git toplevel was this +# package, so the toplevel held packet/, eval/ and the rest. The toplevel is +# now the monorepo root one level above backend/: git operations still run at +# the toplevel, but every copied file anchors under the package root. +PACKAGE="$(cd "$(dirname "$0")/.." && pwd)" REPO="$(git -C "$(dirname "$0")" rev-parse --show-toplevel)" cd "$REPO" @@ -44,16 +49,16 @@ RSYNC_OPTS=(-a --delete --max-size=5m --exclude=.pytest_cache --exclude=.ruff_cache --exclude=.DS_Store) mirror() { # mirror [extra rsync args] - local src="$STORE/$1" dst="$REPO/$2" + local src="$STORE/$1" dst="$PACKAGE/$2" shift 2 if [[ ! -d "$src" ]]; then echo "skip: $src missing"; return; fi mkdir -p "$dst" rsync "${RSYNC_OPTS[@]}" "$@" "$src/" "$dst/" - echo "synced $src -> ${dst#"$REPO"/}" + echo "synced $src -> ${dst#"$PACKAGE"/}" } copy_file() { # copy_file - local src="$STORE/$1" dst="$REPO/$2" + local src="$STORE/$1" dst="$PACKAGE/$2" if [[ ! -f "$src" ]]; then echo "skip: $src missing"; return; fi mkdir -p "$(dirname "$dst")" cp "$src" "$dst" diff --git a/backend/server/api/routes/result.py b/backend/server/api/routes/result.py index 282443825..a5154bd8c 100644 --- a/backend/server/api/routes/result.py +++ b/backend/server/api/routes/result.py @@ -5,7 +5,7 @@ import json from fastapi import FastAPI, Request -from fastapi.responses import HTMLResponse, JSONResponse +from fastapi.responses import HTMLResponse, JSONResponse, Response from pydantic import ValidationError from server.api.deps import get_deps, load_owned, require_principal @@ -121,3 +121,22 @@ def get_review(capture_id: str, request: Request): except FileNotFoundError as exc: raise fail(404, "result_not_ready", "/", "review is not ready") from exc return HTMLResponse(html) + + @app.get("/v1/captures/{capture_id}/preview.ply") + def get_preview(capture_id: str, request: Request): + """Serve the run's result/preview.ply so the review page's relative fetch works.""" + principal = require_principal(request) + deps = get_deps(request) + deps.charge(principal.label, "get", 120) + if deps.settings.HSS_AUTH_MODE != "none" and not principal.reviewer: + raise fail(403, "forbidden", "/", "review is for reviewers") + load_owned(deps, capture_id, principal) + run = _latest(deps, capture_id) + key = _artifacts(deps, run).get("result/preview.ply") if run else None + if not key: + raise fail(404, "result_not_ready", "/", "preview is not ready") + try: + data = deps.storage.get_bytes(key) + except FileNotFoundError as exc: + raise fail(404, "result_not_ready", "/", "preview is not ready") from exc + return Response(content=data, media_type="application/octet-stream") diff --git a/backend/server/api/tests/test_auth_none.py b/backend/server/api/tests/test_auth_none.py index 4767db421..ad8ed613e 100644 --- a/backend/server/api/tests/test_auth_none.py +++ b/backend/server/api/tests/test_auth_none.py @@ -105,6 +105,105 @@ def test_missing_authorization_is_the_demo_device(tmp_path) -> None: assert b"decision: reject" in review.content +def test_review_page_preview_ply_is_served_and_reviewer_gated(tmp_path) -> None: + """The review page's relative fetch("preview.ply") must resolve through the API. + + Served at GET /v1/captures/{id}/review, the browser resolves the page's + relative preview fetch to /v1/captures/{id}/preview.ply — PR #249 review + finding 1: nothing served that, so the point cloud never drew. + """ + + ply_bytes = ( + b"ply\nformat binary_little_endian 1.0\nelement vertex 1\n" + b"property float x\nproperty float y\nproperty float z\nend_header\n" + ) + b"\x00" * 12 + device_token = "d" * 40 + reviewer_token = "r" * 40 + + def _boot(settings: Settings): + app = create_app(settings=settings) + return app, TestClient(app) + + def _stage_preview(app, capture_id: str) -> None: + deps = app.state.deps + html_key = f"captures/{capture_id}/result/review.html" + ply_key = f"captures/{capture_id}/result/preview.ply" + deps.storage.put_bytes(html_key, b"

review

", "text/html") + deps.storage.put_bytes(ply_key, ply_bytes, "application/octet-stream") + deps.state.put_run( + RunRecord( + run_id=f"run_{capture_id}", + capture_id=capture_id, + packet_sha256="c" * 64, + status="done", + branch="image", + flow="photo", + tier="photo_only", + profile_default="C", + created_at="2026-09-26T00:00:00.000Z", + deadline_at="2026-09-26T01:00:00.000Z", + artifacts={"result/review.html": html_key, "result/preview.ply": ply_key}, + ) + ) + deps.state.upsert_stage( + StageRecord( + run_id=f"run_{capture_id}", + stage="result", + status="done", + attempt=1, + heartbeat="2026-09-26T00:00:00.000Z", + ) + ) + + # Auth-none demo mode: the page and its relative preview fetch both resolve. + app, client = _boot(_settings(tmp_path)) + created = client.post( + "/v1/captures", + json=create_body("packet-preview"), + headers={"Idempotency-Key": "packet-preview"}, + ) + assert created.status_code == 201, created.text + capture_id = created.json()["captureId"] + _stage_preview(app, capture_id) + + review = client.get(f"/v1/captures/{capture_id}/review") + assert review.status_code == 200, review.text + preview = client.get(f"/v1/captures/{capture_id}/preview.ply") + assert preview.status_code == 200, preview.text + assert preview.content == ply_bytes + assert preview.headers["content-type"] == "application/octet-stream" + + # Static mode: the artifact is reviewer-gated, same as the page that links it. + static_settings = _settings( + tmp_path, + HSS_AUTH_MODE="static", + CAPTURE_API_TOKENS=f"dev:{device_token},reviewer:rev:{reviewer_token}", + ) + app, client = _boot(static_settings) + created = client.post( + "/v1/captures", + json=create_body("packet-preview-static"), + headers={"Idempotency-Key": "packet-preview-static", "Authorization": f"Bearer {device_token}"}, + ) + assert created.status_code == 201, created.text + capture_id = created.json()["captureId"] + _stage_preview(app, capture_id) + + anonymous = client.get(f"/v1/captures/{capture_id}/preview.ply") + assert anonymous.status_code == 401, anonymous.text + device = client.get( + f"/v1/captures/{capture_id}/preview.ply", + headers={"Authorization": f"Bearer {device_token}"}, + ) + assert device.status_code == 403, device.text + reviewer = client.get( + f"/v1/captures/{capture_id}/preview.ply", + headers={"Authorization": f"Bearer {reviewer_token}"}, + ) + assert reviewer.status_code == 200, reviewer.text + assert reviewer.content == ply_bytes + + def test_replay_mini_arkit_without_a_token(tmp_path) -> None: app = create_app(settings=_settings(tmp_path)) client = TestClient(app) diff --git a/backend/server/result/review/template.html b/backend/server/result/review/template.html index 8063c17dd..f62bdb660 100644 --- a/backend/server/result/review/template.html +++ b/backend/server/result/review/template.html @@ -112,14 +112,58 @@

Views needed

Meter

Status {{ meter.status }} · value {{ meter.value }} · candidates {{ meter.candidates }}

-
+ + +
+
@@ -129,9 +173,151 @@

Preview

diff --git a/backend/server/result/tests/test_compose.py b/backend/server/result/tests/test_compose.py index e8ebd10ca..5ec67b449 100644 --- a/backend/server/result/tests/test_compose.py +++ b/backend/server/result/tests/test_compose.py @@ -302,7 +302,7 @@ def test_review_page_lists_profiles_criteria_and_offline_notice(tmp_path): assert name in html assert "three.js" in html assert "offline" in html - assert 'action="/captures/cap_test/confirmations"' in html + assert 'action="/v1/captures/cap_test/confirmations"' in html assert "TODO queue" in html assert ("SS" + "R") not in html diff --git a/backend/server/result/tests/test_review_page.py b/backend/server/result/tests/test_review_page.py new file mode 100644 index 000000000..a962f5b3f --- /dev/null +++ b/backend/server/result/tests/test_review_page.py @@ -0,0 +1,85 @@ +"""Regression tests for the reviewer page (PR #247 Greptile findings). + +The confirmation form must post JSON with the reviewer's bearer credentials to +the real /v1 route, and the point-cloud preview must load and draw preview.ply. +""" + +from __future__ import annotations + +from pathlib import Path + +from server.result.review.render import render_review + +THREE_URL = "https://unpkg.com/three@0.160.0/build/three.module.js" + + +def _context() -> dict: + return { + "capture_id": "cap_test", + "run_id": "run_test", + "profiles": [{"name": "A", "decision": "manual_review", "device": True}], + "true_decision": "manual_review", + "failure_code": None, + "degraded": [], + "scale": None, + "timings": {}, + "vlm_cost": None, + "decidability": None, + "photos": [], + "criteria": [], + "walls": [], + "views": [], + "meter": { + "status": "confirmed", + "value": "1234567", + "candidates": [], + "evidence": {"image": "still-1"}, + }, + "profile": "A", + "links": [], + } + + +def _render(tmp_path: Path) -> str: + out = tmp_path / "review.html" + render_review(_context(), out, three_url=THREE_URL) + return out.read_text() + + +def test_confirmation_form_posts_to_the_v1_route(tmp_path: Path) -> None: + html = _render(tmp_path) + assert 'action="/v1/captures/cap_test/confirmations"' in html + # The old page posted to /captures/..., a route this API does not serve. + assert 'action="/captures/' not in html + + +def test_confirmation_form_submits_json_with_bearer_credentials(tmp_path: Path) -> None: + html = _render(tmp_path) + assert '"Content-Type": "application/json"' in html + assert "Bearer " in html + assert "JSON.stringify" in html + # Every field the ConfirmationRequest contract requires is present. + for field in ("fieldId", "kind", "value", "imageId", "clientAt"): + assert f"{field}:" in html + + +def test_rejection_status_reads_the_errors_shape(tmp_path: Path) -> None: + html = _render(tmp_path) + # Every API failure path returns {"errors": [{"code", "pointer", "message"}]} + # (server.api.errors.error_response, and the RequestValidationError handler + # in server.api.app builds the same shape) — body.detail is never set, so + # reading it left the reviewer with only "Rejected: HTTP 422". + assert "body.errors[0].message" in html + assert "body.detail" not in html + + +def test_preview_loads_and_draws_preview_ply(tmp_path: Path) -> None: + html = _render(tmp_path) + assert 'fetch("preview.ply")' in html + assert "parseBinaryPly" in html + assert "THREE.Points" in html + assert "THREE.BufferGeometry" in html + # The fallback note is hidden only after the cloud was drawn, and the + # failure path surfaces a message instead of leaving a blank canvas. + assert "note.hidden = true" in html + assert "Point cloud preview unavailable" in html diff --git a/backend/tests/test_sync_from_store.py b/backend/tests/test_sync_from_store.py new file mode 100644 index 000000000..b6ac01857 --- /dev/null +++ b/backend/tests/test_sync_from_store.py @@ -0,0 +1,86 @@ +"""sync_from_store.sh must anchor its copies under backend/, not the git toplevel. + +Since the package moved into the BaseScanning monorepo (PR #247), the git +toplevel is the repository root one level above backend/. Anchoring the +mirrors at the toplevel would rsync --delete into the app's own top-level +packet/ and docs/ folders (Greptile finding on scripts/sync_from_store.sh:26). +""" + +from __future__ import annotations + +import shutil +import subprocess +from pathlib import Path + +BACKEND = Path(__file__).resolve().parents[1] +SCRIPT = BACKEND / "scripts" / "sync_from_store.sh" + + +def _run(cmd: list[str], cwd: Path) -> None: + subprocess.run(cmd, cwd=cwd, check=True, capture_output=True, text=True) + + +def test_sync_writes_inside_backend_only(tmp_path: Path) -> None: + if shutil.which("rsync") is None: # pragma: no cover - environment guard + import pytest + + pytest.skip("rsync is not installed") + mono = tmp_path / "mono" + (mono / "backend" / "scripts").mkdir(parents=True) + _run(["git", "init", "-q"], mono) + + # The app's own top-level folders at the monorepo root must never be touched. + root_packet = mono / "packet" + root_packet.mkdir() + (root_packet / "keep.txt").write_text("app file\n") + + # A stale backend file absent from the store: rsync --delete removes it, + # proving the mirror really landed in backend/packet/. + stale = mono / "backend" / "packet" + stale.mkdir() + (stale / "stale-only.txt").write_text("stale\n") + + store = tmp_path / "store" + for folder in ( + "internal/packet-generator", + "internal/eval-harness", + "internal/criteria", + "internal/data/scripts", + "docs", + ): + (store / folder).mkdir(parents=True) + (store / "internal/packet-generator/packet.json").write_text("{}\n") + (store / "internal/eval-harness/harness.py").write_text("# eval\n") + (store / "internal/criteria/criteria.yaml").write_text("criteria: {}\n") + (store / "internal/data/scripts/build.py").write_text("# data tooling\n") + (store / "internal/data/manifest.json").write_text("{}\n") + (store / "docs/capture-packet-spec.md").write_text("# spec\n") + (store / "internal/criteria/SSR_Prompts.md").write_text("private\n") + + shutil.copy2(SCRIPT, mono / "backend" / "scripts" / "sync_from_store.sh") + + result = subprocess.run( + ["bash", "backend/scripts/sync_from_store.sh", "--here"], + cwd=mono, + env={"PATH": "/usr/local/bin:/usr/bin:/bin", "STORE": str(store), "HOME": str(tmp_path)}, + capture_output=True, + text=True, + ) + assert result.returncode == 0, result.stderr + + # Copies anchor under the package root. + assert (mono / "backend" / "packet" / "packet.json").is_file() + assert (mono / "backend" / "eval" / "harness.py").is_file() + assert (mono / "backend" / "criteria" / "spec" / "criteria.yaml").is_file() + assert (mono / "backend" / "data" / "scripts" / "build.py").is_file() + assert (mono / "backend" / "data" / "manifest.json").is_file() + assert (mono / "backend" / "docs" / "capture-packet-spec.md").is_file() + # Base prompt material stays under the package's ignored private/ folder. + assert (mono / "backend" / "private" / "criteria" / "SSR_Prompts.md").is_file() + # rsync --delete applies inside the package mirror only. + assert not (mono / "backend" / "packet" / "stale-only.txt").exists() + # Nothing was written outside backend/. + assert (root_packet / "keep.txt").is_file() + assert list(root_packet.iterdir()) == [root_packet / "keep.txt"] + assert not (mono / "docs").exists() + assert not (mono / "eval").exists()