Skip to content

WebUI: start a review from "ocr viewer" and stream its progress live (opt-in, loopback-only) #1582

Description

@xandner

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:

  1. choose a local repository,
  2. choose the diff range and the model,
  3. start the review and watch its progress live,
  4. 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)

  1. Backend: job manager, child-process runner, token and Origin middleware, the /api/* routes, input validation. No UI.
  2. 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).
  3. Docs: viewer doc page in all four locales, and the README feature list synced to the translations.
  4. (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

  1. Is an opt-in flag on ocr viewer the right shape, or would you prefer a separate subcommand?
  2. Is a child process acceptable, or would you rather make internal/stdout per-run so reviews can run in-process?
  3. Repository path: free input (optionally limited by --allow-run-root), or only repositories that already have sessions under ~/.opencodereview/sessions/?
  4. 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

Activity

  1. added theissue type on Sep 27, 2026
  2. Moon-Wrecker commented on Oct 3, 2026

    @Moon-Wrecker
    Contributor

    Hi @xandner , as I can't see if anybody or your mention to be working on this. I would like to take up this issue and work on it. Kindly notify on prior if otherwise.

  3. xandner commented on Oct 3, 2026

    @xandner
    Author

    Hi @xandner , as I can't see if anybody or your mention to be working on this. I would like to take up this issue and work on it. Kindly notify on prior if otherwise.

    Hi. I want to implement this system, but I don't know if anyone is working or not.

  4. Moon-Wrecker commented on Oct 3, 2026

    @Moon-Wrecker
    Contributor

    Hi @xandner , as I can't see if anybody or your mention to be working on this. I would like to take up this issue and work on it. Kindly notify on prior if otherwise.

    Hi. I want to implement this system, but I don't know if anyone is working or not.

    Sure, You can continue working on this. Let me know if you need any help at any point on this.

  5. xandner commented on Oct 3, 2026

    @xandner
    Author

    Hi @xandner , as I can't see if anybody or your mention to be working on this. I would like to take up this issue and work on it. Kindly notify on prior if otherwise.

    Hi. I want to implement this system, but I don't know if anyone is working or not.

    Sure, You can continue working on this. Let me know if you need any help at any point on this.

    Sure. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions