feat(web): vouch review-ui — full #194 spec (WebSocket sync, sessions, sources, bearer auth, pagination)#198
Merged
Conversation
mvp slice of vouchdev#194: a fastapi + jinja viewport over the existing proposals + audit surface. every approve / reject goes through proposals.approve / proposals.reject so the audit-log entry is identical to the cli code path. localhost-only; bearer-auth + websocket sync land alongside the http-transport feature. ships behind the [web] optional extra so the base install stays lean. wheel includes the templates and static directory.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughImplements ChangesBrowser-Based Review Console
Sequence DiagramsequenceDiagram
participant CLI as vouch CLI
participant WebApp as FastAPI App
participant Auth as Auth Dependency
participant Storage as KBStore
participant WS as WebSocket Hub
participant Browser as Browser Client
CLI->>CLI: Parse --bind, --auth, --reviewer
CLI->>CLI: Resolve auth token (literal/generate/env)
CLI->>CLI: Validate loopback or require --auth
CLI->>WebApp: create_app(kb_root, auth_token, auth_label)
WebApp->>Storage: Load KB, read proposals
Storage-->>WebApp: Pending proposals
CLI->>Browser: Open 127.0.0.1:7780/?token=ABC (if auth)
Browser->>WebApp: GET /?token=ABC
WebApp->>Auth: Check query token
Auth-->>WebApp: Valid → redirect
WebApp-->>Browser: 303 to / (Set HttpOnly Cookie)
Browser->>WebApp: GET / (with HttpOnly cookie)
WebApp->>Auth: Check cookie
Auth-->>WebApp: Authenticated
WebApp->>Storage: Read paginated proposals
Storage-->>WebApp: Page of 20 proposals
WebApp-->>Browser: queue.html + live.js
Browser->>WebApp: WebSocket /ws
WebApp->>WS: Register client
Browser->>WebApp: POST /approve/prop-123
WebApp->>Storage: Approve proposal, write claim, log audit
Storage-->>WebApp: Claim created, audit entry written
WebApp->>WS: Broadcast refresh frame
WS-->>Browser: {type: refresh, view: queue}
Browser->>Browser: Debounced reload
WebApp-->>Browser: 303 redirect to /
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes The PR introduces a substantial new web subsystem with 1900+ lines of new code spanning FastAPI routes, auth mechanics (Bearer token, HttpOnly cookie bootstrap), WebSocket hub, Jinja2 templates, client-side JavaScript, CSS, CLI integration, and 700+ lines of E2E test coverage. The complexity is distributed across multiple files with interdependencies (server depends on auth, templates depend on server context, tests exercise full flows), dense logic around auth bootstrap redirect and mutation workflows, and several novel patterns (in-process WebSocket broadcast hub, pagination with YAML file bounds, constant-time token comparison). The homogeneity is moderate: most changes are coherent to the single feature, though routes, auth, and views each involve distinct reasoning. Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…urces, auth, pagination) Completes the browser review console started by the MVP slice (vouchdev#195) so it satisfies every acceptance criterion in vouchdev#194. Still a pure viewport: every approve / reject / contradict routes through vouch.proposals / vouch.lifecycle, so the audit log is identical to the CLI. Zero new on-disk schema. Adds on top of the MVP slice: - WebSocket realtime sync (/ws): a single channel per KB broadcasts a refresh signal after every mutation; a second reviewer's queue updates in <1s (measured ~50ms). Frame is a signal, not data — clients re-fetch through the same routes, so there's one rendering path. Each send is bounded by a per-client timeout so one slow/dead socket can't stall the decision handler. - Server-side pagination at the storage layer: only the requested page of proposal files is parsed (proven by test: 500 pending -> 50 parsed). First page renders in ~30ms (was ~260ms loading the whole queue), under the budget. A corrupt proposal file is skipped + logged, not 500'd. - /session/<id> (proposals grouped by agent run) and /sources/<id> (reverse index: which durable claims cite a source). - /contradict gate action; keyboard shortcuts (j/k/a/r/?) and live-refresh as a progressive-enhancement layer — every action is still a plain form POST that works with JavaScript disabled. Auth (team mode): - A non-loopback bind requires --auth (a literal token, 'generate', or 'env'); loopback stays tokenless. Reviewer identity comes from the token label and is recorded in the audit log. - Credentials: Authorization: Bearer header (CLI/API) or an HttpOnly, SameSite=Strict cookie (browser). A ?token= query param is a one-time GET bootstrap only — moved into the cookie and 303-redirected away so the bare token never lingers in a URL or access log. Token comparison is constant-time (secrets.compare_digest), HTTP and WebSocket alike. JS never touches the token. Tests / CI: - tests/test_web_e2e.py (the file vouchdev#194's acceptance names): approve flow end to end asserting the audit.log.jsonl entry; ws sync (with <1s timing assertion), session/source views, deterministic pagination (parses one page of 500) + malformed-file resilience, served-static-asset check, and the auth-hardening cases (cookie bootstrap, constant-time, wrong-cookie/ws rejection). - tests/test_web.py updated for the paginated /api/pending envelope and the auth-required-for-non-loopback behaviour; both web test modules importorskip the [web] extra so they skip cleanly without it. - CI installs .[dev,web] so the web suite actually runs (it imports fastapi). - pyproject: websockets added to the [web] extra (uvicorn needs it for /ws). - docs/review-ui.md documents the surface incl. the auth model. All web assets ship inside the wheel (verified by building it). Full suite + mypy src + ruff clean. Hardening verified live against a real uvicorn server. Closes vouchdev#194. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
dripsmvcp
force-pushed
the
feat/194-review-ui-full
branch
from
c991c42 to
44c30e5
Compare
Collaborator
|
LGTM! |
Merged
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Completes the
vouch review-uiconsole so it satisfies every acceptance criterion in #194. Stacked on top of #195 (plind-junior's MVP slice): this branch contains #195's commit plus one commit that adds the deferred spec features, so it merges without conflicting and credits the MVP. It is a strict superset — nothing the MVP did is removed.Still a pure viewport — every approve/reject/contradict routes through
vouch.proposals/vouch.lifecycle, so the audit log is identical to the CLI. Zero new on-disk schema, zero newkb.*RPC methods.What this adds on top of the MVP slice (#195)
/wshub, measured ~50ms, asserted <1s/session/<id>(proposals by agent run)/sources/<id>(reverse index)0.0.0.0requires auth--auth; allowed with ittests/test_web_e2e.pydrives approve, asserts audit entryPlus
/contradict; keyboard shortcuts (j/k/a/r/?); reviewer identity in the audit log.Security (hardened, then adversarially reviewed)
A multi-agent review pass over the diff (auth-bypass, correctness, async, packaging, acceptance lenses) surfaced and I fixed:
secrets.compare_digest) on HTTP and WebSocket — no timing oracle.Authorization: Bearerheader or an HttpOnly, SameSite=Strict cookie.?token=is a one-time GET bootstrap that's moved into the cookie and303-redirected away, so the bare token never lingers in a URL or access log. JS never touches the token.broadcast()can't be stalled by one slow/dead WebSocket — each send is bounded by a per-client timeout and failures are pruned.run_in_threadpool, so a write can't block the event loop / other reviewers..[dev,web]so the web suite actually runs (it imports FastAPI); both web test modulesimportorskipthe extra for local runs without it.Verification
pytest --ignore=tests/embeddings) → 132 passed (41 web) ·mypy srcclean (31 files) ·ruff check src testsclean.SameSite=Strictcookie,?token=stripped via 303; cookie-authed approve attributed to the token label.Test Plan
tests/test_web_e2e.py,tests/test_web.pypytest --ignore=tests/embeddingspasses (132)mypy src+ruff check src testscleanCloses #194. Supersedes #195 (or merge #195 first — this rebases cleanly on top).
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
vouch review-uicommand with pending queue, claim details, session grouping, and audit timeline pages.j/kfor navigation,ato approve,rto reject).Documentation