feat(web): vouch review-ui — browser-based review console (mvp slice)#195
feat(web): vouch review-ui — browser-based review console (mvp slice)#195plind-junior wants to merge 2 commits into
Conversation
mvp slice of #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.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a localhost-only browser review console: a new ChangesReview Console Web Layer
Sequence DiagramsequenceDiagram
participant Browser
participant FastAPI
participant KBStore
participant ProposalsMod
participant AuditLog
Browser->>FastAPI: GET /
FastAPI->>KBStore: load pending proposals
FastAPI->>Browser: render queue.html
Browser->>FastAPI: POST /approve/{id}
FastAPI->>ProposalsMod: approve(proposal_id, reviewer)
ProposalsMod->>KBStore: update proposal status
ProposalsMod->>AuditLog: write review gate event
FastAPI->>Browser: redirect to /
Browser->>FastAPI: GET /audit
FastAPI->>AuditLog: read review events
FastAPI->>Browser: render audit.html
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
pyproject.toml (1)
34-34: ⚡ Quick winConsider adding an upper version bound for python-multipart.
The dependency
python-multipart>=0.0.9has no upper bound, which may introduce breaking changes if the library releases a new major version. Consider constraining it similarly to the other web dependencies, e.g.,python-multipart>=0.0.9,<1.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pyproject.toml` at line 34, The dependency declaration for python-multipart lacks an upper bound; update the pyproject.toml dependency entry for "python-multipart" to add a safe upper bound (e.g., change "python-multipart>=0.0.9" to "python-multipart>=0.0.9,<1") so it follows the same version constraint pattern as the other web dependencies and prevents unexpected breaking changes from a future major release.src/vouch/web/__init__.py (1)
15-31: ⚡ Quick winConsider checking for python-multipart in
_require_web_extra.The function validates
fastapiandjinja2, butpython-multipartis also a required dependency for FastAPI'sFormhandling (used in the approve/reject endpoints). If it's missing, users will encounter a less clear runtime error when posting forms. Adding a check here would provide earlier, clearer feedback.♻️ Proposed addition
try: import jinja2 # noqa: F401 except ImportError: missing.append("jinja2") + try: + import multipart # noqa: F401 + except ImportError: + missing.append("python-multipart") if missing:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/vouch/web/__init__.py` around lines 15 - 31, The _require_web_extra function currently checks for fastapi and jinja2 but not python-multipart, which FastAPI needs for Form handling in the approve/reject endpoints; add a try/except ImportError block in _require_web_extra that attempts to import multipart (e.g. import multipart # noqa: F401) and on ImportError append "python-multipart" (or "python-multipart (multipart)") to the missing list so the raised ImportError message will list that dependency as well.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pyproject.toml`:
- Around line 105-111: The mypy overrides block in pyproject.toml omits jinja2,
so mypy fails when scanning src/vouch/web/__init__.py; update the
[[tool.mypy.overrides]] entry that currently lists ["fastapi", "fastapi.*",
"uvicorn", "uvicorn.*", "starlette", "starlette.*"] to also include "jinja2" and
"jinja2.*" (i.e., add jinja2 to the module list for the mypy overrides) so
missing-imports for the templating package are ignored during type checking.
In `@src/vouch/cli.py`:
- Around line 808-828: The host allowlist check fails for bracketed IPv6
literals like "[::1]:7780"; after splitting bind into host and port_str (the
variables in this snippet), strip surrounding brackets from host if host starts
with "[" and ends with "]" (i.e., host = host[1:-1]) before comparing against
("127.0.0.1", "localhost", "::1") so bracketed IPv6 localhost is accepted.
In `@src/vouch/web/server.py`:
- Around line 42-46: _update the _whoami function to match the CLI's fallback
chain: use VOUCH_AGENT if set, otherwise VOUCH_USER, and finally fall back to
the system user via getpass.getuser(); locate the _whoami() function in
server.py and replace the current single-env fallback ("web-reviewer") with the
same sequence used by the CLI (VOUCH_AGENT or VOUCH_USER or getpass.getuser())
so audit-log attribution is consistent across web and CLI surfaces.
In `@src/vouch/web/templates/queue.html`:
- Around line 26-32: The approve/reject POST forms lack CSRF protection; add a
server-generated CSRF token to the template forms (e.g. include a hidden input
named csrf_token inside the forms that submit to /approve/{{ item.id }} and
/reject/{{ item.id }}) and ensure the POST handlers that process these routes
(the approve and reject request handlers) validate the token on every
state-changing request (or alternatively perform strict Origin/Referer checks)
before performing the action; update the template rendering code to supply the
token (e.g. csrf_token) and update the approve/reject handler functions to
reject requests with missing/invalid tokens.
In `@tests/test_web.py`:
- Line 96: The assertion uses a weak OR that often passes; replace it with a
concrete check for the rationale marker from the seeded proposal by asserting
the exact rationale key or value in the response body (e.g., assert
'"rationale":' and/or the seeded rationale string is present or absent as
appropriate). Locate the assertion that inspects r.text (the response object
named r) and change it to assert the specific rationale JSON key/value for the
seeded proposal (or assert its absence) so the test verifies rationale rendering
deterministically.
---
Nitpick comments:
In `@pyproject.toml`:
- Line 34: The dependency declaration for python-multipart lacks an upper bound;
update the pyproject.toml dependency entry for "python-multipart" to add a safe
upper bound (e.g., change "python-multipart>=0.0.9" to
"python-multipart>=0.0.9,<1") so it follows the same version constraint pattern
as the other web dependencies and prevents unexpected breaking changes from a
future major release.
In `@src/vouch/web/__init__.py`:
- Around line 15-31: The _require_web_extra function currently checks for
fastapi and jinja2 but not python-multipart, which FastAPI needs for Form
handling in the approve/reject endpoints; add a try/except ImportError block in
_require_web_extra that attempts to import multipart (e.g. import multipart #
noqa: F401) and on ImportError append "python-multipart" (or "python-multipart
(multipart)") to the missing list so the raised ImportError message will list
that dependency as well.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ccca1784-3ec1-4398-9ad3-a486a478708c
📒 Files selected for processing (10)
pyproject.tomlsrc/vouch/cli.pysrc/vouch/web/__init__.pysrc/vouch/web/server.pysrc/vouch/web/static/app.csssrc/vouch/web/templates/audit.htmlsrc/vouch/web/templates/base.htmlsrc/vouch/web/templates/claim.htmlsrc/vouch/web/templates/queue.htmltests/test_web.py
| if ":" not in bind: | ||
| raise click.ClickException( | ||
| f"--bind must be host:port (got {bind!r})" | ||
| ) | ||
| host, _, port_str = bind.rpartition(":") | ||
| try: | ||
| port = int(port_str) | ||
| except ValueError as e: | ||
| raise click.ClickException(f"invalid port in --bind: {port_str!r}") from e | ||
|
|
||
| # The full design (issue #194) refuses 0.0.0.0 without --auth bearer. | ||
| # The MVP slice does not ship the Bearer-auth layer yet (depends on | ||
| # the HTTP-transport feature), so we refuse non-localhost binds | ||
| # outright — a clearer error than silently exposing an unauthenticated | ||
| # approve surface on the network. | ||
| if host not in ("127.0.0.1", "localhost", "::1"): | ||
| raise click.ClickException( | ||
| f"--bind {bind!r}: review-ui is localhost-only in the MVP slice. " | ||
| "Bearer-auth + non-loopback binding land alongside the HTTP " | ||
| "transport feature." | ||
| ) |
There was a problem hiding this comment.
Handle IPv6 localhost addresses with brackets.
The current parsing uses rpartition(":") which correctly extracts the port, but IPv6 addresses may be provided with brackets (e.g., [::1]:7780). In that case, host would be "[::1]" (with brackets) and would not match the allowlist check at line 823 that expects "::1".
🛡️ Proposed fix to strip brackets from IPv6 addresses
host, _, port_str = bind.rpartition(":")
+ # Strip brackets from IPv6 addresses like [::1]:7780
+ if host.startswith("[") and host.endswith("]"):
+ host = host[1:-1]
try:
port = int(port_str)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/vouch/cli.py` around lines 808 - 828, The host allowlist check fails for
bracketed IPv6 literals like "[::1]:7780"; after splitting bind into host and
port_str (the variables in this snippet), strip surrounding brackets from host
if host starts with "[" and ends with "]" (i.e., host = host[1:-1]) before
comparing against ("127.0.0.1", "localhost", "::1") so bracketed IPv6 localhost
is accepted.
| def _whoami() -> str: | ||
| """Reviewer identity. Without the Bearer-auth layer from #1 this falls | ||
| back to the same env var the CLI uses, so audit-log attribution stays | ||
| consistent across surfaces.""" | ||
| return os.environ.get("VOUCH_AGENT", "web-reviewer") |
There was a problem hiding this comment.
Make _whoami() consistent with the CLI version for audit-log attribution.
The comment states that reviewer identity should match the CLI behavior for consistent audit logs, but the implementation differs:
- CLI (lines 75-84 in
cli.py):VOUCH_AGENT or VOUCH_USER or getpass.getuser() - Web (line 46):
VOUCH_AGENTor fallback"web-reviewer"
If VOUCH_USER is set but VOUCH_AGENT is not, the CLI will use VOUCH_USER while the web UI will use "web-reviewer", breaking attribution consistency.
🔧 Proposed fix to match CLI behavior
+import getpass
+
def _whoami() -> str:
"""Reviewer identity. Without the Bearer-auth layer from `#1` this falls
back to the same env var the CLI uses, so audit-log attribution stays
consistent across surfaces."""
- return os.environ.get("VOUCH_AGENT", "web-reviewer")
+ return (
+ os.environ.get("VOUCH_AGENT")
+ or os.environ.get("VOUCH_USER")
+ or getpass.getuser()
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _whoami() -> str: | |
| """Reviewer identity. Without the Bearer-auth layer from #1 this falls | |
| back to the same env var the CLI uses, so audit-log attribution stays | |
| consistent across surfaces.""" | |
| return os.environ.get("VOUCH_AGENT", "web-reviewer") | |
| import getpass | |
| def _whoami() -> str: | |
| """Reviewer identity. Without the Bearer-auth layer from `#1` this falls | |
| back to the same env var the CLI uses, so audit-log attribution stays | |
| consistent across surfaces.""" | |
| return ( | |
| os.environ.get("VOUCH_AGENT") | |
| or os.environ.get("VOUCH_USER") | |
| or getpass.getuser() | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/vouch/web/server.py` around lines 42 - 46, _update the _whoami function
to match the CLI's fallback chain: use VOUCH_AGENT if set, otherwise VOUCH_USER,
and finally fall back to the system user via getpass.getuser(); locate the
_whoami() function in server.py and replace the current single-env fallback
("web-reviewer") with the same sequence used by the CLI (VOUCH_AGENT or
VOUCH_USER or getpass.getuser()) so audit-log attribution is consistent across
web and CLI surfaces.
| <form method="post" action="/approve/{{ item.id }}" class="inline"> | ||
| <button type="submit" class="approve">approve</button> | ||
| </form> | ||
| <form method="post" action="/reject/{{ item.id }}" class="inline"> | ||
| <input type="text" name="reason" placeholder="reason (required)" required> | ||
| <button type="submit" class="reject">reject</button> | ||
| </form> |
There was a problem hiding this comment.
Add CSRF protection for approve/reject form posts.
At Line 26 and Line 29, these state-changing POST forms can be triggered cross-site from a malicious page targeting localhost. Please enforce CSRF protection (token validation and/or strict Origin/Referer checks) before accepting approve/reject actions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/vouch/web/templates/queue.html` around lines 26 - 32, The approve/reject
POST forms lack CSRF protection; add a server-generated CSRF token to the
template forms (e.g. include a hidden input named csrf_token inside the forms
that submit to /approve/{{ item.id }} and /reject/{{ item.id }}) and ensure the
POST handlers that process these routes (the approve and reject request
handlers) validate the token on every state-changing request (or alternatively
perform strict Origin/Referer checks) before performing the action; update the
template rendering code to supply the token (e.g. csrf_token) and update the
approve/reject handler functions to reject requests with missing/invalid tokens.
| assert r.status_code == 200 | ||
| assert pid in r.text | ||
| assert "detail view shows the full payload" in r.text | ||
| assert "rationale" not in r.text or "agent-A" in r.text |
There was a problem hiding this comment.
Tighten this assertion; current check is effectively non-diagnostic.
At Line 96, assert "rationale" not in r.text or "agent-A" in r.text will usually pass regardless of rationale rendering because agent-A appears in normal metadata. Assert a concrete rationale-specific marker instead (e.g., absence/presence of the rationale term/value for the seeded proposal).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_web.py` at line 96, The assertion uses a weak OR that often
passes; replace it with a concrete check for the rationale marker from the
seeded proposal by asserting the exact rationale key or value in the response
body (e.g., assert '"rationale":' and/or the seeded rationale string is present
or absent as appropriate). Locate the assertion that inspects r.text (the
response object named r) and change it to assert the specific rationale JSON
key/value for the seeded proposal (or assert its absence) so the test verifies
rationale rendering deterministically.
…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>
…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>
ci installs the [dev] extra only (no [web]), so jinja2 isn't on the python path when mypy scans src/vouch/web/__init__.py — the guard import inside _require_web_extra() trips import-not-found even though the runtime story (raise ImportError if missing) is correct. local mypy missed it because my venv has jinja2 installed for the test suite; ci doesn't. add jinja2 to the same override block that already covers fastapi/uvicorn/starlette.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 21432e89c8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| import pytest | ||
| from click.testing import CliRunner | ||
| from fastapi.testclient import TestClient |
There was a problem hiding this comment.
In the GitHub CI test job I checked, the install step uses pip install -e '.[dev]' and then runs python -m pytest, but [dev] now only adds httpx while fastapi remains only in the [web] extra. Because this new test module imports fastapi.testclient during collection, a normal dev/CI install without [web] fails before running any tests with ModuleNotFoundError. Either include the web extra dependencies in the dev/CI install or skip these tests when the web stack is absent.
Useful? React with 👍 / 👎.
| """Reviewer identity. Without the Bearer-auth layer from #1 this falls | ||
| back to the same env var the CLI uses, so audit-log attribution stays | ||
| consistent across surfaces.""" | ||
| return os.environ.get("VOUCH_AGENT", "web-reviewer") |
There was a problem hiding this comment.
When a human files a proposal via the CLI without VOUCH_AGENT, proposed_by is VOUCH_USER or the OS user, but the web UI falls back to the literal web-reviewer. In that default local workflow, approving from vouch review-ui no longer matches the proposer and bypasses proposals.approve's forbidden-self-approval guard, recording the wrong actor in the audit log. Mirror the CLI fallback or require an explicit reviewer so self-approval remains blocked.
Useful? React with 👍 / 👎.
| proposals_mod.approve( | ||
| store, proposal_id, approved_by=_whoami(), reason=reason | ||
| ) | ||
| except (proposals_mod.ProposalError, ArtifactNotFoundError) as e: |
There was a problem hiding this comment.
For proposals that were valid when filed but become invalid before approval, such as a cited source/evidence or a page's referenced claim being deleted, proposals_mod.approve() can raise ValueError from the storage put_* methods. This handler only catches ProposalError and ArtifactNotFoundError, so the browser path returns an internal server error while the CLI's _cli_errors reports a clean user error for the same condition. Include ValueError in the handled errors.
Useful? React with 👍 / 👎.
| @app.post("/approve/{proposal_id}") | ||
| def approve(proposal_id: str, reason: str | None = Form(default=None)) -> Any: |
There was a problem hiding this comment.
Because these approve/reject routes are unauthenticated and accept plain form POSTs, a malicious web page can submit a cross-site form to a locally running review UI if it has or can infer a proposal id, causing an approval or rejection under the user's local reviewer identity. Localhost binding does not stop browser CSRF; add an origin/token check for these mutation routes while keeping the no-JS form flow.
Useful? React with 👍 / 👎.
| url = f"http://{host}:{port}/" | ||
| click.echo(f"vouch review-ui running at {url}") | ||
| threading.Timer(0.5, lambda: webbrowser.open(url)).start() | ||
| else: | ||
| click.echo(f"vouch review-ui running at http://{host}:{port}/") |
There was a problem hiding this comment.
When the accepted loopback bind is ::1:7780, this interpolation prints and opens http://::1:7780/, which URL parsers treat as an invalid authority instead of the IPv6 literal host. Users binding to IPv6 loopback need http://[::1]:7780/ (and the same formatting in the no-open branch) for the browser URL to work.
Useful? React with 👍 / 👎.
1 participant
Summary
mvp slice of #194: a fastapi + jinja viewport over the existing
proposals+auditsurface. ships behind a new[web]optional extra; localhost-only. every approve / reject routes throughvouch.proposals.approve/vouch.proposals.reject, so the audit-log entry is identical regardless of whether the action came fromvouch approve(cli) or the browser. closes the part of #194 that doesn't depend on the http-transport (#1) or multi-dim-scopes (#2) work.What changed
src/vouch/web/__init__.py— package shim, raises a cleanImportErrorline if the[web]extra is missing.src/vouch/web/server.py— fastapi app factory + routes (/,/claim/<id>,POST /approve/<id>,POST /reject/<id>,/audit,/api/pending,/healthz). audit-log writes go through the sharedproposals.*code path; no parallel data path.src/vouch/web/templates/{base,queue,claim,audit}.html— jinja templates. no js framework; plain form-post approve/reject works without javascript.src/vouch/web/static/app.css— hand-rolled monograph-adjacent styling, single sheet, no tailwind.src/vouch/cli.py—vouch review-uisubcommand. binds127.0.0.1:7780by default, refuses any non-loopback bind in this mvp slice (bearer-auth lands alongside chore(deps): bump actions/setup-python from 5 to 6 #1).pyproject.toml— adds the[web]extra (fastapi,jinja2,python-multipart,uvicorn);httpxadded to[dev]so the test client works in ci; mypy overrides for the new optional imports.tests/test_web.py— 15 tests: queue render, empty state, claim detail, 404 path, approve round-trip (asserts the durable claim lands + theproposal.claim.approveaudit entry), reject landsproposal.claim.rejectwith reason, missing-reason rejected, plain-form-post flow without js, audit timeline view, healthz, bring-up error if no.vouch/, and--bind 0.0.0.0:...is refused.Why
the review gate is vouch's load-bearing primitive but only the terminal can drive it today; that doesn't scale past a single reviewer or a ~20-item queue. this gives reviewers a browser-shaped surface without rebuilding any of the underlying gate logic — same code path, same audit entries, same forbidden-self-approval guard.
scope was deliberately kept small (no websocket, no bearer-auth, no spa) because the full design depends on transport features that haven't shipped. follow-ups stack cleanly on top of this slice.
Tests
wheel build confirmed the templates + static dir ship in
vouch_kb-0.1.0-py3-none-any.whl.Not included (follow-ups)
/session/<id>and/sources/<id>views from the specj/k/a/r) and optimistic uiCloses #194 (mvp slice — follow-ups tracked in subsequent PRs).
Summary by CodeRabbit
New Features
vouch review-uicommand and browser-based review console for managing proposalsChores
weboptional dependency group and expandeddevextras for web/testingTests