Problem Statement
ocr viewer can browse and compare past review sessions, but it is read-only. To start a review you still need a terminal: cd into the repository, remember the flags for the diff range, and pass --provider / --model if you want a model other than the configured default. Then you switch to the browser to read the result.
For people who already live in the viewer (or who are less comfortable with the CLI), it would be useful to do the whole loop in one place:
- choose a local repository,
- choose the diff range and the model,
- start the review and watch its progress live,
- land on the finished session page that the viewer already renders.
Proposed Solution
1. User-facing behaviour
- New opt-in flag:
ocr viewer --allow-run. Without it the viewer behaves exactly as today (read-only, no new routes registered).
- A New review page with a form:
- Repository path: absolute path to a local git work tree.
- Mode: workspace changes, range (
--from / --to), or single commit (--commit).
- Provider and model: a dropdown filled from the built-in provider/model list (
llm.ListProviders()), plus a free-text model field. It defaults to the configured provider and model.
- Optional: background text (
--background), effort (--effort), token budget (--max-tokens-budget).
- After Start, a live log view streams progress over Server-Sent Events (SSE), with a Cancel button.
- When the run finishes, the page links to the existing session page
/r/{repo}/{sessionID} (the session_id is taken from the JSON result).
2. Architecture
Each run is a child process of the same binary. The viewer starts os.Executable() with review ... --format json --audience human:
stderr (human progress) is read line by line and forwarded as SSE log events.
stdout (a single JSON document) is parsed at exit and sent as a final result event.
Why a child process instead of calling internal/agent in-process:
- Progress goes through the process-global writer in
internal/stdout, and stdout.Quiet() is documented as main-goroutine-only. Two concurrent in-process runs could not keep their output apart.
- The CLI already has interrupt handling (
interrupt.go) and process-group setup (procattr_*.go). Cancellation reuses both: SIGINT to the process group first, then a kill after a grace period.
- A panic or a hung run cannot take the viewer down.
- Sessions are persisted to
~/.opencodereview/sessions/ by the child exactly as a CLI run would, so the existing viewer pages work unchanged.
Job manager (new, inside internal/viewer):
- In-memory jobs with states
queued → running → succeeded | failed | cancelled.
- A bounded log buffer per job (for example 5,000 lines / 2 MB). A browser that reconnects replays the log from
Last-Event-ID.
- Finished jobs are evicted after a TTL. All child processes are killed when the viewer exits.
Routes (registered only with --allow-run):
| Method |
Path |
Purpose |
| GET |
/run |
New-review form |
| GET |
/api/models |
Provider names, model ids, which one is configured. No secrets. |
| POST |
/api/runs |
Validate input, start a job, return 202 {"id": ...} |
| GET |
/api/runs/{id}/events |
SSE stream: log, status, result |
| POST |
/api/runs/{id}/cancel |
Cancel a running job |
SSE rather than WebSocket: the stream is one-way, SSE works with plain net/http with no new dependency, and the browser reconnects on its own.
3. Security
Starting a review spends API tokens and runs git against arbitrary local paths, so the run feature needs a stricter model than the read-only viewer.
3.1 Off by default and loopback only
- The run routes exist only with
--allow-run.
--allow-run refuses to start if --addr binds a non-loopback address (for example :3000) or if OCR_VIEWER_ALLOWED_HOSTS is set. There is no auth layer suited to a network, so the feature is not offered there.
3.2 Per-launch secret token (similar to Jupyter)
- At startup the viewer generates 32 random bytes (
crypto/rand) and prints the URL as http://localhost:5483/?token=....
- The first request exchanges the token for an
HttpOnly; SameSite=Strict cookie and redirects to the same URL without the token, so the token does not stay in the address bar or history.
- Every
/run and /api/* request requires the cookie. The comparison is constant-time.
- This protects against other local users or processes on a shared machine, and together with the existing host guard it covers DNS rebinding.
3.3 CSRF
- State-changing routes are
POST only.
- They require an
Origin header equal to the viewer's own origin; a missing Origin is rejected.
- They require
Content-Type: application/json, which forces a CORS preflight from any other site. The viewer sends no CORS headers, so the preflight fails.
- The
SameSite=Strict cookie from 3.2 adds another layer.
3.4 Input validation (no shell, no free-form arguments)
- argv is built from a typed struct and run with
exec.Command. There is no shell and no pass-through field for arbitrary flags.
- Repository path: must be absolute. It is cleaned with
filepath.Clean and resolved with filepath.EvalSymlinks, must be a directory, and must equal git rev-parse --show-toplevel. An optional repeatable --allow-run-root <dir> restricts runs to repositories under given directories.
- Refs (
from / to / commit): reject anything starting with - (git option injection), limit the charset and length, and check with git rev-parse --verify before starting.
- Provider must be a known provider. Model is limited to
[A-Za-z0-9._:/@-] with a maximum length.
- Request bodies are capped with
http.MaxBytesReader (for example 64 KB), and the background text has its own length limit.
- Phase 1 does not accept
--rule, --tools, --background-file or -o, because those read or write arbitrary files. The run uses the repository's own .opencodereview/ config, as the CLI does.
3.5 Secrets stay in the CLI
- No route reads or writes config. API keys are never sent to the browser or accepted from it.
/api/models returns only names and a "configured" flag.
- The child inherits the viewer's environment, which is the same thing that happens when a user runs
ocr review in a terminal.
3.6 Resource limits
- A maximum number of concurrent jobs (default 1) and at most one running job per repository.
- The existing
--timeout applies. The form pre-fills a default --max-tokens-budget so a mistaken click does not start an unbounded run.
- Log buffers are bounded, and job records expire after the TTL.
3.7 Rendering untrusted output
- Log lines contain model output and source code, so they are treated as untrusted.
- They are sent as JSON-encoded SSE data. ANSI escapes are stripped on the server, and the client renders them only with
textContent, never innerHTML.
- The current CSP (
script-src 'self', form-action 'none', no unsafe-inline) stays unchanged. All new JS lives in internal/viewer/static/.
3.8 Audit
- Each run start, finish and cancel is logged on the viewer's stderr with repository, mode, provider/model and child PID.
4. Delivery plan (small, separate PRs)
- Backend: job manager, child-process runner, token and Origin middleware, the
/api/* routes, input validation. No UI.
- UI:
/run form and live log page in vanilla JS and Go templates, following the existing static/ and templates/ style (no build step, embedded assets).
- Docs: viewer doc page in all four locales, and the README feature list synced to the translations.
- (Follow-up, if wanted):
ocr scan from the UI, reusing the same runner.
Testing: httptest coverage for every security check (token, cookie, Origin, Content-Type, body size, path and ref validation, loopback refusal), and the job manager tested against a fake child process (the TestHelperProcess pattern) for streaming, reconnect replay, cancellation and timeouts. The 90% coverage threshold holds.
5. Open questions for maintainers
- Is an opt-in flag on
ocr viewer the right shape, or would you prefer a separate subcommand?
- Is a child process acceptable, or would you rather make
internal/stdout per-run so reviews can run in-process?
- Repository path: free input (optionally limited by
--allow-run-root), or only repositories that already have sessions under ~/.opencodereview/sessions/?
- Should
ocr scan be part of the first round?
Alternatives Considered
- A separate wrapper project that starts the CLI. This loses the integration with the existing session pages and repeats all of the security work outside the project.
- Running reviews in-process. This is blocked by the global
internal/stdout writer and the process-wide signal handling, and a panic in one review would stop the viewer.
- WebSocket instead of SSE. A two-way channel is not needed. SSE is simpler, reconnects on its own and needs no dependency.
Affected Area
WebUI Viewer, CLI / Commands
Additional Context
No response
Problem Statement
ocr viewercan browse and compare past review sessions, but it is read-only. To start a review you still need a terminal:cdinto the repository, remember the flags for the diff range, and pass--provider/--modelif you want a model other than the configured default. Then you switch to the browser to read the result.For people who already live in the viewer (or who are less comfortable with the CLI), it would be useful to do the whole loop in one place:
Proposed Solution
1. User-facing behaviour
ocr viewer --allow-run. Without it the viewer behaves exactly as today (read-only, no new routes registered).--from/--to), or single commit (--commit).llm.ListProviders()), plus a free-text model field. It defaults to the configured provider and model.--background), effort (--effort), token budget (--max-tokens-budget)./r/{repo}/{sessionID}(thesession_idis taken from the JSON result).2. Architecture
Each run is a child process of the same binary. The viewer starts
os.Executable()withreview ... --format json --audience human:stderr(human progress) is read line by line and forwarded as SSElogevents.stdout(a single JSON document) is parsed at exit and sent as a finalresultevent.Why a child process instead of calling
internal/agentin-process:internal/stdout, andstdout.Quiet()is documented as main-goroutine-only. Two concurrent in-process runs could not keep their output apart.interrupt.go) and process-group setup (procattr_*.go). Cancellation reuses both: SIGINT to the process group first, then a kill after a grace period.~/.opencodereview/sessions/by the child exactly as a CLI run would, so the existing viewer pages work unchanged.Job manager (new, inside
internal/viewer):queued → running → succeeded | failed | cancelled.Last-Event-ID.Routes (registered only with
--allow-run):/run/api/models/api/runs202 {"id": ...}/api/runs/{id}/eventslog,status,result/api/runs/{id}/cancelSSE rather than WebSocket: the stream is one-way, SSE works with plain
net/httpwith no new dependency, and the browser reconnects on its own.3. Security
Starting a review spends API tokens and runs
gitagainst arbitrary local paths, so the run feature needs a stricter model than the read-only viewer.3.1 Off by default and loopback only
--allow-run.--allow-runrefuses to start if--addrbinds a non-loopback address (for example:3000) or ifOCR_VIEWER_ALLOWED_HOSTSis set. There is no auth layer suited to a network, so the feature is not offered there.3.2 Per-launch secret token (similar to Jupyter)
crypto/rand) and prints the URL ashttp://localhost:5483/?token=....HttpOnly; SameSite=Strictcookie and redirects to the same URL without the token, so the token does not stay in the address bar or history./runand/api/*request requires the cookie. The comparison is constant-time.3.3 CSRF
POSTonly.Originheader equal to the viewer's own origin; a missingOriginis rejected.Content-Type: application/json, which forces a CORS preflight from any other site. The viewer sends no CORS headers, so the preflight fails.SameSite=Strictcookie from 3.2 adds another layer.3.4 Input validation (no shell, no free-form arguments)
exec.Command. There is no shell and no pass-through field for arbitrary flags.filepath.Cleanand resolved withfilepath.EvalSymlinks, must be a directory, and must equalgit rev-parse --show-toplevel. An optional repeatable--allow-run-root <dir>restricts runs to repositories under given directories.from/to/commit): reject anything starting with-(git option injection), limit the charset and length, and check withgit rev-parse --verifybefore starting.[A-Za-z0-9._:/@-]with a maximum length.http.MaxBytesReader(for example 64 KB), and the background text has its own length limit.--rule,--tools,--background-fileor-o, because those read or write arbitrary files. The run uses the repository's own.opencodereview/config, as the CLI does.3.5 Secrets stay in the CLI
/api/modelsreturns only names and a "configured" flag.ocr reviewin a terminal.3.6 Resource limits
--timeoutapplies. The form pre-fills a default--max-tokens-budgetso a mistaken click does not start an unbounded run.3.7 Rendering untrusted output
textContent, neverinnerHTML.script-src 'self',form-action 'none', nounsafe-inline) stays unchanged. All new JS lives ininternal/viewer/static/.3.8 Audit
4. Delivery plan (small, separate PRs)
/api/*routes, input validation. No UI./runform and live log page in vanilla JS and Go templates, following the existingstatic/andtemplates/style (no build step, embedded assets).ocr scanfrom the UI, reusing the same runner.Testing:
httptestcoverage for every security check (token, cookie, Origin, Content-Type, body size, path and ref validation, loopback refusal), and the job manager tested against a fake child process (theTestHelperProcesspattern) for streaming, reconnect replay, cancellation and timeouts. The 90% coverage threshold holds.5. Open questions for maintainers
ocr viewerthe right shape, or would you prefer a separate subcommand?internal/stdoutper-run so reviews can run in-process?--allow-run-root), or only repositories that already have sessions under~/.opencodereview/sessions/?ocr scanbe part of the first round?Alternatives Considered
internal/stdoutwriter and the process-wide signal handling, and a panic in one review would stop the viewer.Affected Area
WebUI Viewer, CLI / Commands
Additional Context
No response