desktop/windows: apps detail page, conversation fixes, Rewind + auto-update UX#8072
Open
andermont wants to merge 1 commit into
Open
desktop/windows: apps detail page, conversation fixes, Rewind + auto-update UX#8072andermont wants to merge 1 commit into
andermont wants to merge 1 commit into
Conversation
…update UX Brings the Windows app up to the current build: - Apps: per-app detail page (About/Prompt/Preview/Capabilities/Integration/Reviews), popularity-ranked marketplace, faster cached loading. - Conversations: render pending/local rows locally instead of 404ing; fix pending deletion. - Rewind: load full stored history, wheel-pan the activity bar without yanking, stick to the live edge on open. - Auto-update: opt-in native "Download?" dialog, native Task-Dialog progress (a .NET win-update-helper), silent in-place install, app version in Advanced settings, app icon (win.icon). - Keeps productName "Omi" / omi.exe; no publish block (releases handled by CI).
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
11 issues found across 91 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="desktop/windows/src/preload/index.ts">
<violation number="1" location="desktop/windows/src/preload/index.ts:140">
P1: Preload now exposes a consent-free automation execution IPC to web content. This allows renderer/XSS-triggered UI automation without the native approval dialog.</violation>
</file>
<file name="desktop/windows/src/main/ipc/automation.ts">
<violation number="1" location="desktop/windows/src/main/ipc/automation.ts:48">
P1: `automation:run` executes desktop UI automation without the native consent dialog, allowing the approval gate to be bypassed.</violation>
</file>
<file name="desktop/windows/src/renderer/src/components/Markdown.tsx">
<violation number="1" location="desktop/windows/src/renderer/src/components/Markdown.tsx:30">
P1: Unvalidated markdown href allows dangerous protocols to be clicked. In this Electron flow that can trigger external handler execution/SMB auth leakage from model-generated content.</violation>
</file>
<file name="desktop/windows/src/main/overlay/window.ts">
<violation number="1" location="desktop/windows/src/main/overlay/window.ts:208">
P2: Scale updates can be undone during an active height tween. Sync `tweenW` when applying scaled bounds so tween frames keep the new width.</violation>
</file>
<file name="desktop/windows/src/main/update/progressDialog.ts">
<violation number="1" location="desktop/windows/src/main/update/progressDialog.ts:28">
P2: Exit/error handlers are not bound to the spawned child instance, causing cross-process state races. A stale helper exit can null out a newer helper and suppress the dialog for the rest of the download.</violation>
</file>
<file name="desktop/windows/src/renderer/src/components/rewind/RewindTimelineBar.tsx">
<violation number="1" location="desktop/windows/src/renderer/src/components/rewind/RewindTimelineBar.tsx:78">
P2: Timeline auto-scroll no longer tracks explicit cursor changes from other controls. Seeking/playback can move the cursor into history while the bar stays locked to live edge until the user manually scrolls this bar.</violation>
</file>
<file name="desktop/windows/src/renderer/src/components/settings/LevelSlider.tsx">
<violation number="1" location="desktop/windows/src/renderer/src/components/settings/LevelSlider.tsx:52">
P2: Pointer move handling updates value for any pressed mouse-over, not only active slider drags. Gate moves on pointer capture to avoid accidental jumps.</violation>
</file>
<file name="desktop/windows/src/main/ipc/omiListen.ts">
<violation number="1" location="desktop/windows/src/main/ipc/omiListen.ts:99">
P2: Connection logging now includes `uid` in plaintext via the URL, which leaks account identifiers into logs.</violation>
</file>
<file name="desktop/windows/src/renderer/src/hooks/usePushToTalk.ts">
<violation number="1" location="desktop/windows/src/renderer/src/hooks/usePushToTalk.ts:143">
P2: Push-to-talk visualizer uses exact mic selection without stale-device fallback, so stale mic IDs can silently disable VAD/waveform while transcription still runs on fallback input.</violation>
</file>
<file name="desktop/windows/src/renderer/src/components/layout/MainViews.tsx">
<violation number="1" location="desktop/windows/src/renderer/src/components/layout/MainViews.tsx:69">
P2: Unprotected decodeURIComponent in route rendering can throw and crash the app-detail view on malformed URL input.</violation>
</file>
<file name="desktop/windows/src/renderer/src/components/overlay/OverlayApp.tsx">
<violation number="1" location="desktop/windows/src/renderer/src/components/overlay/OverlayApp.tsx:61">
P2: `notifyVoiceCaptured` now only fires on non-empty transcript commits, so completed hold-Space captures without transcript no longer unblock the onboarding voice step.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Partial review: This PR has more than 50 files, so cubic reviewed the highest-priority files first. During the trial, paid plans get a higher file limit.
You can try an ultrareview to bypass the file limit, comment @cubic-dev-ai ultrareview. Learn more.
Re-trigger cubic
| // which gates on a native approval dialog built in main from the real plan. | ||
| // Exposing a consent-free run primitive to web content would let any future | ||
| // renderer-side code (XSS, hostile navigation) silently drive Windows UI input. | ||
| automationRun: (plan: AutomationPlan) => ipcRenderer.invoke('automation:run', plan), |
There was a problem hiding this comment.
P1: Preload now exposes a consent-free automation execution IPC to web content. This allows renderer/XSS-triggered UI automation without the native approval dialog.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At desktop/windows/src/preload/index.ts, line 140:
<comment>Preload now exposes a consent-free automation execution IPC to web content. This allows renderer/XSS-triggered UI automation without the native approval dialog.</comment>
<file context>
@@ -134,11 +137,7 @@ const omi: OmiBridgeApi = {
- // which gates on a native approval dialog built in main from the real plan.
- // Exposing a consent-free run primitive to web content would let any future
- // renderer-side code (XSS, hostile navigation) silently drive Windows UI input.
+ automationRun: (plan: AutomationPlan) => ipcRenderer.invoke('automation:run', plan),
automationConfirmRun: (plan: AutomationPlan) => ipcRenderer.invoke('automation:confirmRun', plan),
onAutomationStep: (cb: (r: StepResult) => void) => {
</file context>
| // renderer but had no legitimate caller, and let web content drive Windows UI | ||
| // input with no approval. Per-step progress events aren't needed by the | ||
| // confirm flow (it resolves once on completion). | ||
| ipcMain.handle('automation:run', async (e, plan: AutomationPlan): Promise<PlanRunResult> => { |
There was a problem hiding this comment.
P1: automation:run executes desktop UI automation without the native consent dialog, allowing the approval gate to be bypassed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At desktop/windows/src/main/ipc/automation.ts, line 48:
<comment>`automation:run` executes desktop UI automation without the native consent dialog, allowing the approval gate to be bypassed.</comment>
<file context>
@@ -45,11 +45,12 @@ export function registerAutomationHandlers(): void {
- // renderer but had no legitimate caller, and let web content drive Windows UI
- // input with no approval. Per-step progress events aren't needed by the
- // confirm flow (it resolves once on completion).
+ ipcMain.handle('automation:run', async (e, plan: AutomationPlan): Promise<PlanRunResult> => {
+ const wc: WebContents = e.sender
+ return automationBridge.run(plan, (r) => {
</file context>
Comment on lines
+30
to
+35
| if (link) | ||
| return ( | ||
| <a key={i} href={link[2]} target="_blank" rel="noreferrer" className="underline"> | ||
| {link[1]} | ||
| </a> | ||
| ) |
There was a problem hiding this comment.
P1: Unvalidated markdown href allows dangerous protocols to be clicked. In this Electron flow that can trigger external handler execution/SMB auth leakage from model-generated content.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At desktop/windows/src/renderer/src/components/Markdown.tsx, line 30:
<comment>Unvalidated markdown href allows dangerous protocols to be clicked. In this Electron flow that can trigger external handler execution/SMB auth leakage from model-generated content.</comment>
<file context>
@@ -27,22 +27,12 @@ function renderInline(text: string): React.ReactNode[] {
- )
- return <span key={i}>{link[1]}</span>
- }
+ if (link)
+ return (
+ <a key={i} href={link[2]} target="_blank" rel="noreferrer" className="underline">
</file context>
Suggested change
| if (link) | |
| return ( | |
| <a key={i} href={link[2]} target="_blank" rel="noreferrer" className="underline"> | |
| {link[1]} | |
| </a> | |
| ) | |
| if (link) { | |
| const href = link[2].trim() | |
| if (/^(https?:|mailto:)/i.test(href)) | |
| return ( | |
| <a key={i} href={href} target="_blank" rel="noreferrer" className="underline"> | |
| {link[1]} | |
| </a> | |
| ) | |
| return <span key={i}>{link[1]}</span> | |
| } |
| if (win.isVisible()) { | ||
| const wa = | ||
| activeWorkArea ?? screen.getDisplayNearestPoint(screen.getCursorScreenPoint()).workArea | ||
| win.setBounds(computeOverlayBounds(wa, lastContentHeight, currentOverlayWidth())) |
There was a problem hiding this comment.
P2: Scale updates can be undone during an active height tween. Sync tweenW when applying scaled bounds so tween frames keep the new width.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At desktop/windows/src/main/overlay/window.ts, line 208:
<comment>Scale updates can be undone during an active height tween. Sync `tweenW` when applying scaled bounds so tween frames keep the new width.</comment>
<file context>
@@ -185,6 +177,38 @@ let activeWorkArea: { x: number; y: number; width: number; height: number } | nu
+ if (win.isVisible()) {
+ const wa =
+ activeWorkArea ?? screen.getDisplayNearestPoint(screen.getCursorScreenPoint()).workArea
+ win.setBounds(computeOverlayBounds(wa, lastContentHeight, currentOverlayWidth()))
+ }
+}
</file context>
Suggested change
| win.setBounds(computeOverlayBounds(wa, lastContentHeight, currentOverlayWidth())) | |
| const target = computeOverlayBounds(wa, lastContentHeight, currentOverlayWidth()) | |
| tweenW = target.width | |
| win.setBounds(target) |
| // NB: no `windowsHide` — it sets CREATE_NO_WINDOW, which suppresses the | ||
| // helper's Task Dialog from appearing. The helper is a WinExe, so there's no | ||
| // console window to hide anyway. | ||
| proc = spawn(exe, [version], { stdio: ['pipe', 'ignore', 'ignore'] }) |
There was a problem hiding this comment.
P2: Exit/error handlers are not bound to the spawned child instance, causing cross-process state races. A stale helper exit can null out a newer helper and suppress the dialog for the rest of the download.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At desktop/windows/src/main/update/progressDialog.ts, line 28:
<comment>Exit/error handlers are not bound to the spawned child instance, causing cross-process state races. A stale helper exit can null out a newer helper and suppress the dialog for the rest of the download.</comment>
<file context>
@@ -0,0 +1,67 @@
+ // NB: no `windowsHide` — it sets CREATE_NO_WINDOW, which suppresses the
+ // helper's Task Dialog from appearing. The helper is a WinExe, so there's no
+ // console window to hide anyway.
+ proc = spawn(exe, [version], { stdio: ['pipe', 'ignore', 'ignore'] })
+ proc.on('error', (e) => {
+ console.warn('[autoUpdate] progress helper spawn error:', e?.message ?? e)
</file context>
| setFromClientX(e.clientX) | ||
| } | ||
| const onPointerMove = (e: React.PointerEvent): void => { | ||
| if (e.buttons === 0) return // only while dragging |
There was a problem hiding this comment.
P2: Pointer move handling updates value for any pressed mouse-over, not only active slider drags. Gate moves on pointer capture to avoid accidental jumps.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At desktop/windows/src/renderer/src/components/settings/LevelSlider.tsx, line 52:
<comment>Pointer move handling updates value for any pressed mouse-over, not only active slider drags. Gate moves on pointer capture to avoid accidental jumps.</comment>
<file context>
@@ -0,0 +1,113 @@
+ setFromClientX(e.clientX)
+ }
+ const onPointerMove = (e: React.PointerEvent): void => {
+ if (e.buttons === 0) return // only while dragging
+ setFromClientX(e.clientX)
+ }
</file context>
| // Token rides in the Authorization header (not the URL), so the URL is safe to | ||
| // log. This surfaces the exact connect params + any close reason for diagnosing | ||
| // 1008s (trial_expired, "Bad uid", "language not supported", …). | ||
| console.log(`[omi-listen] connecting ${args.sessionId} src=${args.source} ${url}`) |
There was a problem hiding this comment.
P2: Connection logging now includes uid in plaintext via the URL, which leaks account identifiers into logs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At desktop/windows/src/main/ipc/omiListen.ts, line 99:
<comment>Connection logging now includes `uid` in plaintext via the URL, which leaks account identifiers into logs.</comment>
<file context>
@@ -59,6 +93,10 @@ function startSession(args: ListenStartArgs, owner: WebContents): void {
+ // Token rides in the Authorization header (not the URL), so the URL is safe to
+ // log. This surfaces the exact connect params + any close reason for diagnosing
+ // 1008s (trial_expired, "Bad uid", "language not supported", …).
+ console.log(`[omi-listen] connecting ${args.sessionId} src=${args.source} ${url}`)
const ws = new WebSocket(url, {
headers: { Authorization: `Bearer ${args.token}` }
</file context>
Suggested change
| console.log(`[omi-listen] connecting ${args.sessionId} src=${args.source} ${url}`) | |
| console.log(`[omi-listen] connecting ${args.sessionId} src=${args.source} ${url.replace(/([?&]uid=)[^&]*/i, '$1<redacted>')}`) |
Comment on lines
+143
to
+145
| const stream = await navigator.mediaDevices.getUserMedia({ | ||
| audio: micAudioConstraints(getPreferences().micDeviceId) | ||
| }) |
There was a problem hiding this comment.
P2: Push-to-talk visualizer uses exact mic selection without stale-device fallback, so stale mic IDs can silently disable VAD/waveform while transcription still runs on fallback input.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At desktop/windows/src/renderer/src/hooks/usePushToTalk.ts, line 143:
<comment>Push-to-talk visualizer uses exact mic selection without stale-device fallback, so stale mic IDs can silently disable VAD/waveform while transcription still runs on fallback input.</comment>
<file context>
@@ -142,7 +139,10 @@ export function usePushToTalk(opts: Options): PushToTalk {
try {
- const stream = await navigator.mediaDevices.getUserMedia({ audio: true })
+ // Visualize the SAME input the transcription stream uses (Settings → General).
+ const stream = await navigator.mediaDevices.getUserMedia({
+ audio: micAudioConstraints(getPreferences().micDeviceId)
+ })
</file context>
Suggested change
| const stream = await navigator.mediaDevices.getUserMedia({ | |
| audio: micAudioConstraints(getPreferences().micDeviceId) | |
| }) | |
| const constraint = micAudioConstraints(getPreferences().micDeviceId) | |
| let stream: MediaStream | |
| try { | |
| stream = await navigator.mediaDevices.getUserMedia({ audio: constraint }) | |
| } catch (e) { | |
| const err = e as Error | |
| if (constraint !== true && (err.name === 'OverconstrainedError' || err.name === 'NotFoundError')) { | |
| stream = await navigator.mediaDevices.getUserMedia({ audio: true }) | |
| } else { | |
| throw e | |
| } | |
| } |
|
|
||
| const appDetailMatch = pathname.match(/^\/apps\/([^/]+)$/) | ||
| if (appDetailMatch) { | ||
| return <AppDetail appId={decodeURIComponent(appDetailMatch[1])} /> |
There was a problem hiding this comment.
P2: Unprotected decodeURIComponent in route rendering can throw and crash the app-detail view on malformed URL input.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At desktop/windows/src/renderer/src/components/layout/MainViews.tsx, line 69:
<comment>Unprotected decodeURIComponent in route rendering can throw and crash the app-detail view on malformed URL input.</comment>
<file context>
@@ -63,6 +64,11 @@ export function MainViews(): React.JSX.Element {
+ const appDetailMatch = pathname.match(/^\/apps\/([^/]+)$/)
+ if (appDetailMatch) {
+ return <AppDetail appId={decodeURIComponent(appDetailMatch[1])} />
+ }
+
</file context>
| setDraft('') | ||
| enqueueSend(text) | ||
| // Let the onboarding voice step know a hold-Space capture completed. | ||
| window.omiOverlay.notifyVoiceCaptured() |
There was a problem hiding this comment.
P2: notifyVoiceCaptured now only fires on non-empty transcript commits, so completed hold-Space captures without transcript no longer unblock the onboarding voice step.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At desktop/windows/src/renderer/src/components/overlay/OverlayApp.tsx, line 61:
<comment>`notifyVoiceCaptured` now only fires on non-empty transcript commits, so completed hold-Space captures without transcript no longer unblock the onboarding voice step.</comment>
<file context>
@@ -54,12 +57,9 @@ function OverlayPanel({ replayEnter }: { replayEnter: () => void }): React.JSX.E
setDraft('')
enqueueSend(text)
+ // Let the onboarding voice step know a hold-Space capture completed.
+ window.omiOverlay.notifyVoiceCaptured()
},
- // Fires on every completed hold-Space capture, even when transcription was
</file context>
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.
Updates the Windows desktop app (
desktop/windows/) to the current build.Apps
/apps/:id): About, Prompt, Preview, Capabilities, Integration (triggers + setup), Reviews with developer responses.Conversations
Rewind
Auto-update UX
win-update-helper, same stdio-helper pattern aswin-ocr-helper).win.icon).Notes
productName: Omi/omi.exe.publishpoints atBasedHardware/omi(the release tag scheme is reconciled by CI at publish time).!app.isPackaged.