Replace Jinja UI with React frontend

[shiv-tyagi]

Should be merged after #249.

Renames the web package to backend, adds a React (Vite) frontend that talks to the existing API, and updates docker-compose, README, and flake8 so the new layout runs cleanly locally and in CI.

Made with Cursor

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Pull request overview

Copilot reviewed 72 out of 91 changed files in this pull request and generated no new comments.

Suppressed comments (5)

frontend/index.html:1

• The HTML references /favicon.ico, but this PR adds frontend/public/favicon.svg. Unless a .ico file is also present/generated elsewhere, browsers will 404 the icon. Either change the link to /favicon.svg (and set an appropriate type, e.g. image/svg+xml) or add a favicon.ico asset to frontend/public/.
  frontend/src/hooks/useConfigLoad.ts:1
• The non-null assertions (find(...)!) can crash at runtime if the backing array changes (e.g., async refresh) or an unexpected id is passed. Guard these lookups and handle the 'not found' case by setting an error + returning to idle (or re-opening the conflict state) instead of throwing.
  frontend/src/components/FeaturesModal.tsx:1
• This category grouping is recomputed on every render. In this modal, renders can be frequent (search input, toggles), and features can be large. Wrap this reduction in useMemo keyed on features to avoid repeated work and reduce UI jank.
  frontend/src/components/FeaturesModal.tsx:1
• Using a div with role=\"button\" as a click target that contains another interactive control (TriStateCheckbox renders a button) creates nested interactive elements, which is problematic for keyboard and assistive tech. Prefer making the outer container a real <button type=\"button\"> for the scroll action (or make it non-interactive and add a dedicated 'jump to category' button), and ensure the checkbox control is not nested inside another button-like element.
  frontend/src/featureDeps.ts:1
• reducedDependencyEdges calls reachableWithin(...) inside a nested loop, which can become expensive with many selected features and dependency edges (repeated DFS per candidate edge). Consider memoizing reachableWithin(start) per start for the current ids set, or computing reachability once per node before the inner loop to avoid repeated graph traversals.

[peterbarker]

The security thing here doesn't look great here... I'd definitely want that one explained first!

  ⎿  docker-compose.yml
       ●  58 [security]       The new frontend service publishes
                              "${WEB_PORT:-11080}:80" without the loopback
                              binding the removed app service had
                              ("127.0.0.1:${WEB_PORT:-8080}:8080"),
                              exposing the UI and proxied API on every host
                              interface.
     frontend/src/components/BuildsTable.tsx
       ● 277 [correctness]    Download is gated on `terminal` (which
                              includes FAILURE/ERROR/TIMED_OUT) rather than
                              state === 'SUCCESS', but get_artifact_path
                              (backend/services/builds.py:243) only returns
                              a path when the archive exists.
       ● 106 [error-handling] doFetch chains .then().finally() with no
                              .catch(), so a rejected fetchBuilds becomes
                              an unhandled promise rejection with no error
                              surfaced.
     frontend/nginx.conf
       ●   8 [correctness]    `proxy_pass http://backend:8080/api/;` uses a
                              literal hostname with no resolver or
                              upstream variable, so nginx resolves
                              `backend` once at startup and never
                              re-resolves.
       ●   7 [correctness]    The proxy block sets no proxy_read_timeout,
                              so nginx's 60s default now applies to API
                              calls that legitimately exceed it.
       ●  15 [correctness]    Only /api/ is proxied and the backend port is
                              no longer published, so /health falls
                              through to `try_files … /index.html`.
     frontend/src/buildConfig.ts
       ●  62 [correctness]    fetchSchema only checks res.ok, but nginx's
                              `try_files $uri $uri/ /index.html` returns
                              index.html with status 200 for any unknown
                              path, so a missing schema parses as JSON.
       ●  34 [correctness]    configFromQueryParams requires vehicle_id, so
                              the old UI's ?rebuild_from=<build_id> links
                              are ignored entirely.
     frontend/src/types.ts
       ●  14 [correctness]    backend/services/vehicles.py:79-84 maps
                              unknown release types to "custom"
                              (backend/schemas/vehicles.py:20), but
                              Version['type'] and BuildForm.tsx:43's
                              TYPE_ORDER omit it (same omission at
                              types.ts:62).
     frontend/src/components/StepComponents.tsx
       ● 198 [correctness]    The quick pills pick options.find(o => o.type
                              === qt.type) — the first match across all
                              remotes — and render only version.name with
                              no remote badge, unlike the "More" list.
     frontend/src/components/FeaturesGraphView.tsx
       ● 772 [correctness]    This document-level Escape handler and
                              ModalShell's (ModalShell.tsx:50) are both on
                              `document`; stopPropagation() does not stop
                              other listeners registered on the same
                              target.
     README.md
       ● 112 [documentation]  The PR removed the working `uvicorn
                              web.main:app` instruction, leaving `python3
                              backend/main.py` as the only documented way
                              to run the API locally; it fails immediately
                              because sys.path[0] is …/backend and the repo
                              root is never added.
     frontend/src/components/StepComponents.tsx
       ● 198 [correctness]    The quick pills pick options.find(o => o.type === qt.type) — the first match across all remotes — and render
                              only version.name with no remote badge, unlike the "More" list.
     frontend/src/components/FeaturesGraphView.tsx
       ● 772 [correctness]    This document-level Escape handler and ModalShell's (ModalShell.tsx:50) are both on `document`;
                              stopPropagation() does not stop other listeners registered on the same target.
     README.md
       ● 112 [documentation]  The PR removed the working `uvicorn web.main:app` instruction, leaving `python3 backend/main.py` as the only
                              documented way to run the API locally; it fails immediately because sys.path[0] is …/backend and the repo root
                              is never added.
     .github/workflows/linting.yml
       ● 198 [correctness]    The quick pills pick options.find(o => o.type === qt.type) — the first match across all remotes — and
                              render only version.name with no remote badge, unlike the "More" list.
     frontend/src/components/FeaturesGraphView.tsx
       ● 772 [correctness]    This document-level Escape handler and ModalShell's (ModalShell.tsx:50) are both on `document`;
                              stopPropagation() does not stop other listeners registered on the same target.
     README.md
       ● 112 [documentation]  The PR removed the working `uvicorn web.main:app` instruction, leaving `python3 backend/main.py` as the
                              only documented way to run the API locally; it fails immediately because sys.path[0] is …/backend and the
                              repo root is never added.
     .github/workflows/linting.yml
       ●   1 [test-coverage]  CI still runs only flake8 and pytest; tsc --noEmit, eslint, and vite build never run despite the PR adding
                              an eslint config and a large TypeScript frontend.

[shiv-tyagi]

● 58 [security] The new frontend service publishes
"${WEB_PORT:-11080}:80" without the loopback
binding the removed app service had
("127.0.0.1:${WEB_PORT:-8080}:8080"),
exposing the UI and proxied API on every host
interface.

Yes, this one looks real. I will fix this and other points raised by you. This would have exposed custom build server to the world (bypassing apache), which is not catastrophic but not good either.

[shiv-tyagi]

@peterbarker Addressed all your points.

1. Frontend is now published as `127.0.0.1:${WEB_PORT:-11080}:80`, matching the old loopback bind so the UI and proxied API are not on every host interface.
2. Backend already served the archive whenever it existed (including failed builds); the UI now enables download from `artifact_available` on the build payload instead of any terminal state.
3. A rejected `fetchBuilds` is caught and shown as a full-width “Failed to load builds” row in the table, and the error clears on the next successful fetch.
4. Nginx only runs in Compose with `depends_on: backend`, so a one-shot resolve of the `backend` hostname at container start is acceptable here.
5. All `/api/` proxying uses a 120s `proxy_read_timeout` / `proxy_send_timeout`; a separate `/api/v1/builds/` location was dropped because it 301’d slashless URLs off port 11080.
6. We are not using `/health` yet, so leaving it unpublished behind nginx is fine.
7. `fetchSchema` now requires `Content-Type: application/json` (and a successful JSON parse) so nginx serving `index.html` with 200 cannot be treated as a schema.
8. We are cutting over immediately and will not keep `?rebuild_from=<build_id>`; rebuild stays on the in-app button via `fetchBuildConfig`.
9. Frontend `Version` / `BuildVersionInfo` types, sort order, and badges now include `custom`, matching the backend.
10. Quick pills are only official ArduPilot stable/beta/latest, so a remote badge there is unnecessary; the More list still shows remotes.
11. The graph Escape handler is on the capture phase and calls `stopImmediatePropagation` when it consumes the key, so it no longer races ModalShell on `document`.
12. Local API docs now start the server from the repo root with `PYTHONPATH=. uvicorn backend.main:app --host 0.0.0.0 --port 8080`.
13. CI has a frontend job that runs `npm ci`, `npm run lint`, and `npm run build` (tsc + Vite) on the same PR/push triggers as pytest.

[peterbarker]

README.mnd is trivial.

DId you mean to remove latest and use master?

New findings, ranked

1. README.md line 21 is corrupted. The word "your" now contains roughly 800 spaces, so the line reads "installed on yo ... r machine". Looks like an editor accident. Trivial fix, but it
   should not merge as is.
2. Version names changed in the public API. backend/services/vehicles.py:75 now hard-codes the name "master" for every "latest" release and drops the remote name from all titles. The
   sample remotes file has a "latest" release on a non-official remote pinned to a raw SHA, and it would be labelled "master". The tests were edited to lock in the new strings. The UI
   shows a remote badge so it looks fine there, but any other API consumer sees different names. This is scope creep beyond a rename plus new frontend and deserves a sentence in the PR
   description, or the remote name kept for non-official remotes.
3. Rate limit bypass via X-Forwarded-For (Codex, pre-existing). Uvicorn trusts forwarded headers from every address and nginx appends to whatever the client sends, so a caller can rotate
   the leading address and dodge the 10 builds per hour limit. This was already true with Apache in front of uvicorn on main, so it is not a regression. With a second proxy hop now in the
   chain it is worth setting FORWARDED_ALLOW_IPS to the actual proxy addresses in a follow-up.
4. Health endpoint unreachable through compose (Codex). A request to /health on the published port returns the SPA index with 200. The author already said nothing probes it. If Apache or
   any monitor ever does, add a location block for it.
5. Old rebuild links are dropped (Codex). The author stated this is a deliberate cutover. That is your call to confirm, not a bug.

Where I disagree with Codex. Its highest-severity finding was that the build-complete screen offers the download on SUCCESS before the archive exists. That is wrong. The progress updater
only reports SUCCESS after the archive file is on disk, see build_manager/progress_updater.py:187, so SUCCESS implies the file exists. The old auto-download in index.js relied on the same
guarantee. I would not raise this on the PR.