UX critique improvements, server mode over the tailnet, and phone layout - #28
Merged
Merged
Conversation
gh pr list --limit 100 with 'commits' requested up to 1M nodes (cap 500k), so every PR list load failed. Drop 'commits' and the unused commitsCount; skip per-PR mergeability in the bulk list (it times out on busy repos). Single-PR view/forBranch still fetch mergeable/mergeStateStatus.
Non-progress stderr lines (a pre-push hook, remote messages) become the progress description, so a long hook no longer leaves the UI on "Starting…"; the percentage stays empty until git reports a transfer phase.
Extracts an Electron-free core (handlers, event bus, HostCapabilities) that both the desktop app and a new node:http + WebSocket server mount. The server binds 127.0.0.1 behind `tailscale serve`, requires a bearer token, holds every forwarded request (page and bridge included) to the node owner's Tailscale login, answers only loopback/*.ts.net hosts, confines paths to allowlisted roots, serializes per-repo mutations and rate-limits clients. The desktop app can run as a client of a server (server-url / GITGOOD_SERVER_URL) and warns once on a version mismatch (GET /version). `npm run build` now builds the server so installers ship it; `npm run install:service` writes a systemd user unit; CI builds the bundle. The launch watched-folder scan now waits for tool discovery, since registering candidates runs git.
Below 768px the UI shows one pane at a time with a bottom tab bar; tapping a row drills in and the browser back gesture returns. Pickers, dialogs and context menus become bottom sheets, long-press opens context menus (iOS Safari never fires contextmenu), touch targets grow to 44px and line selection works with touch. A web manifest and icons make it installable to the home screen. Desktop layouts are unchanged.
Scenarios started acting while the renderer was still waiting for tool discovery, so settings, update and watched-folder scenarios saw unloaded state and failed (on main too).
- Path confinement follows each method's signature: repository paths, path parameters and path fields of option objects (repos.create/clone directory, worktree path, settings folders) are checked, `..` is resolved and relative paths are refused; file contents, .gitignore lines and comments starting with "/" are no longer rejected. - Trash works on the server: items go to <userData>/trash/, so discards that asked for the trash no longer hard-delete, and repos.remove trashes before unregistering. - Per-client state: each client (X-GitGood-Client, stable per tab) has its own repository watcher and history load, so a desktop and a phone no longer stop each other's live updates or cancel each other's history. A disconnected client releases its watcher after 60s. - The browser bridge reports focus/visibility and, after a reconnect, re-opens its repository and refreshes. - Long network/AI calls (fetch, push, AI review/triage) no longer hold the per-repository lock. - WebSocket: unmasked frames or frames over 125 bytes close the connection before anything is buffered. - Malformed URLs get 400; an uncaught exception exits for systemd to restart.
History entries now record how many entries were pushed instead of inferring it from the pane's depth; the review view drills from the list straight to a file in one entry, so a forward swipe followed by a tab switch rewound past the app's own page.
The tablet and phone layouts are for browsers. Their media rules are now scoped to html:not([data-desktop]); the preload marks both desktop modes (gitgoodDesktop), which also turns off the phone drill-down in JS. CSS nesting is flattened at build time (cssTarget) for older phone Safari.
Touch pointers are implicitly captured by the cell they start on, so no other cell saw pointerenter; the capture is released on pointerdown.
… paths - The invoke rate limit is keyed by X-Forwarded-For (set by tailscale serve) or the socket address instead of the caller-chosen X-GitGood-Client, so rotating the header no longer bypasses it. - In desktop client mode, open/reveal/trash of a path the server's page asks for is first checked against the server's path confinement.
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
1 participant
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
Three bodies of work, 18 commits:
UX critique improvements
Server mode (web access over the tailnet)
HostCapabilities) mounted by both the desktop app and a newnode:http+ WebSocket server (src/server/)tailscale serve; bearer token; every forwarded request must carry the allowedTailscale-User-Login(default: node owner); host allowlist against DNS rebinding; per-method path confinement; per-repo mutation lock; rate limit by source address<userData>/trash/server-url/GITGOOD_SERVER_URL), warns on version mismatchnpm run buildincludes the server (installers ship it);npm run install:servicewrites a systemd user unit; CI builds the server bundleopenspec/specs/server-mode/spec.mdPhone/tablet layout (web only)
Fixes found along the way
main)Review
Security and code reviews ran against the server/mobile work; all 15 findings are fixed in the last five commits (path confinement bypass via option objects, discard-to-trash hard-deleting in web mode, clients stopping each other's watchers and history loads, unbounded WebSocket buffering, rate-limit bypass, reconnect resync, and others).
Testing
npm run typecheck: cleannpm test: 1117 passed, 4 skippednpm run test:smoke: 35/35tailscale serveingress (status, diff, commit, history, live events); forged host/identity headers refused (421/403); two browser tabs on different repositories keep their own live updates, including after a server restart; iPhone 13 emulation for drill-in/back, long-press menu and touch drag-select; packaged Linux build runs the server withELECTRON_RUN_AS_NODE=1Not verified: a real phone (touch behaviour was tested in emulation only), and the desktop-layout guard in an actual narrowed/zoomed Electron window (verified in a browser by toggling the marker).