Desktop app frontend#117
Conversation
Greptile SummaryThis PR introduces the complete Electron desktop frontend for ReverbCode — a React + Vite renderer talking to the local
Confidence Score: 3/5Mostly safe to merge as a frontend scaffold, but the broken polling interval in SessionView means the session detail pane does not refresh independently. The useCallback dependency on the entire props object in SessionView causes the intended 2s refresh interval to tear down and restart on every parent re-render. Since App re-renders every 2s, the SessionView interval never fires on its own schedule — the session detail only refreshes when the parent renders, making the independent polling logic non-functional. frontend/src/renderer/components/SessionView.tsx — the hook dependency and interval reset issue; frontend/src/renderer/App.tsx — inline callbacks passed to SessionView should be stabilised with useCallback. Important Files Changed
Sequence DiagramsequenceDiagram
participant R as Renderer (React)
participant P as Preload (contextBridge)
participant M as Main Process (IPC)
participant D as Daemon (127.0.0.1)
R->>P: "window.ao.request({ method, path, body })"
P->>M: ipcRenderer.invoke("ao:request", req)
M->>M: discoverPort() — reads running.json or AO_PORT
M->>D: fetch(http://127.0.0.1:port/...)
D-->>M: HTTP response (JSON / text)
M-->>P: "AoResponse { ok, status, data }"
P-->>R: "Promise<AoResponse<T>>"
note over R: App polls every 2s via setInterval
note over R: SessionView also polls every 2s (interval resets on parent re-render)
R->>P: getHealth → /healthz
R->>P: listProjects → /api/v1/projects
R->>P: listSessions → /api/v1/sessions
Reviews (1): Last reviewed commit: "feat: compose desktop app root" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| setSession(await getSession(props.sessionId)); | ||
| } catch (err) { | ||
| props.onError(err instanceof Error ? err.message : String(err)); | ||
| } | ||
| }, [props]); | ||
|
|
||
| useEffect(() => { | ||
| void refresh(); | ||
| const id = window.setInterval(() => void refresh(), 2000); | ||
| return () => window.clearInterval(id); | ||
| }, [refresh]); | ||
|
|
||
| useEffect(() => { | ||
| const onKey = (e: KeyboardEvent) => e.key === "Escape" && props.onClose(); | ||
| window.addEventListener("keydown", onKey); | ||
| return () => window.removeEventListener("keydown", onKey); | ||
| }, [props]); | ||
|
|
||
| const run = async (label: string, fn: () => Promise<void>) => { | ||
| setBusy(label); |
There was a problem hiding this comment.
[props] dependency causes the polling interval to reset on every parent re-render
Both useCallback hooks use [props] as a dependency. Since App re-renders every 2 s (on each poll), every prop reference is recreated — so refresh gets a new identity on every parent render, causing the useEffect on line 46 to tear down and restart the interval immediately. In practice the 2 s interval for the session detail never fires on its own schedule; it resets before it can tick. Similarly, the keydown listener on line 52 is removed and re-added on every parent render, leaving a brief window where Escape doesn't close the overlay.
Destructure only the specific values used inside each closure, e.g. [props.sessionId, props.onError] and [props.onClose], so the callbacks only change when those values actually change. Inline arrow callbacks passed from App (like onClose={() => setDetailId(null)}) should also be wrapped in useCallback at the call-site to stabilise their references.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| <button | ||
| className="btn-ghost sm" | ||
| disabled={!!busy} | ||
| onClick={() => run("restore", () => restoreSession(props.sessionId).then())} |
There was a problem hiding this comment.
.then() with no arguments is unnecessary here. restoreSession already returns a Promise<Session>, which is assignable to Promise<void> in a callback position in TypeScript. The trailing .then() creates an extra microtask and could silently swallow a rejection that occurs after the first promise resolves.
| onClick={() => run("restore", () => restoreSession(props.sessionId).then())} | |
| onClick={() => run("restore", () => restoreSession(props.sessionId))} |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| const window = new BrowserWindow({ | ||
| width: 1200, | ||
| height: 800, | ||
| width: 1280, |
There was a problem hiding this comment.
The req coming from the renderer is typed as AoRequest but TypeScript types don't enforce anything at runtime. A compromised renderer (e.g. via a content script XSS in a future webview) could pass method: "DELETE" on any daemon path, including destructive endpoints like /api/v1/projects/:id, without any guard here. Since this is the sole network proxy for all daemon traffic, adding a minimal allowlist for req.method (e.g. ["GET","POST","PUT","PATCH","DELETE"]) and asserting req.path starts with / would narrow the attack surface without adding meaningful overhead.
| }, [projects]); | ||
|
|
||
| const up = !!health; | ||
| const withOverride = (list: Session[]): Session[] => | ||
| list.map((s) => | ||
| statusOverride[s.id] ? { ...s, status: statusOverride[s.id] } : s, |
There was a problem hiding this comment.
statusOverride from demo mode can bleed into live sessions
statusOverride is never cleared when demo mode is toggled off. If any real session happens to share an ID with a demo session (e.g. ao-204, int-7, …), it will display the demo's overridden status until the user navigates away or the component remounts. Resetting statusOverride when setDemo is called would prevent this.
| </div> | ||
|
|
||
| <div className="sb-foot"> | ||
| <button className="icon-btn sm"><Icon d={I.home} /></button> | ||
| <button className="icon-btn sm"><Icon d={I.check} /></button> | ||
| <button className="icon-btn sm"><Icon d={I.swap} /></button> |
There was a problem hiding this comment.
The four footer buttons (home, check, swap, moon) render without onClick, aria-label, or title attributes. They are inert — clicking them does nothing — and screen readers cannot identify their purpose. If these are intentional stubs, a title attribute and a disabled state would signal that more clearly and prevent user confusion.
2 participants
No description provided.