feat(win): Windows support — build, package, publish, and platform-aware UX#47
Merged
Conversation
Make the bundled-runtime resolvers Windows-aware and add a branch CI job that packages an NSIS installer, so a Windows build can actually be produced. - runtime-env: add exe() helper (appends .exe on win32) - llm/imagegen/rag-extractors: resolve llama-server/sd-cli/whisper-cli/ffmpeg via exe(); prepend binDir to PATH on Windows so co-located DLLs load - scripts/fetch-win-binaries.ps1: rewrite to pull win64 llama/whisper/sd/ffmpeg from each project's LATEST GitHub release (no more stale pins), laid out in the per-runtime subdirs the resolvers expect, with sd.exe->sd-cli.exe rename - .github/workflows/windows-build.yml: windows-2022 + Python 3.12, npm ci, fetch binaries, build, package --win, upload installer artifact (no version bump / no Release publish from the branch) - README: document the LFS prereq and the Windows build path mflux (MLX), coreml-sd, and ocr (Apple Vision) are macOS-only and degrade gracefully when their binaries are absent on Windows. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ULeJgUZHdRQsXPbieBYEZq
…han cleanup Hardens against the "server up but /v1/models empty" bug (seen on macOS) and ensures it can't recur on Windows, where the old Unix-only cleanup was a no-op. Root cause: an orphaned llama-server from a previous run keeps holding port 8439. The new spawn can't bind, and waitForReady — which only probed /health — then talked to the ORPHAN (serving no/stale model), marking us ready with an empty model list. The orphan-killer used lsof/ps/SIGKILL (macOS/Linux only), so on Windows there was zero cleanup. - waitForReady: require /v1/models to list a model (not just /health 200) before declaring ready, and bail immediately if the server process exits (failed load) instead of waiting out the full timeout - killOrphansOnPort: cross-platform helper — lsof/ps on macOS+Linux, netstat/tasklist/taskkill on Windows; still only kills a process identified as our own llama-server - gateway /v1/models: fall back to the on-disk active model (LLMService .activeModelInfo()) when upstream hasn't loaded one — so the chat model is listed for an idle app / headless gateway, matching image/speech/STT behaviour Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ULeJgUZHdRQsXPbieBYEZq
Adds a manual recovery control to the Models page for when the server gets into a bad state (e.g. up but no model loaded). Useful during development and as a quick fix without restarting the whole app. - llm: add restart() — clears paused, reloads the active model, waits until it's actually loaded; throws if it can't come back up - ipc: llm:stop and llm:restart handlers (alongside the existing llm:status) - preload: expose getServerStatus / stopServer / restartServer - ModelsScreen: header control with a live status dot (polls llm:status every 4s) plus Restart and Stop buttons with in-flight state and a result message Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ULeJgUZHdRQsXPbieBYEZq
…er copy The binaries downloaded and copied fine, but the final "now contains" listing used $_.FullName.Substring($bin.Length+1) where $bin still held the uncollapsed 'scripts\..\resources\bin' path — longer than the copied files' canonical paths, so Substring threw "startIndex cannot be larger than length of string" and, under ErrorActionPreference=Stop, exited the script 1 *after* a successful copy. The CI fetch step and local runs both went red, leaving llama-server.exe seemingly absent. - canonicalize $bin via [IO.Path]::GetFullPath so paths line up - list with a safe .Replace($bin,'') instead of Substring - add an explicit post-fetch check: llama-server.exe REQUIRED (hard fail with a clear message), whisper/sd/ffmpeg optional (warn only) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ULeJgUZHdRQsXPbieBYEZq
Carry the latest core (macOS) work into the Windows-support branch. main had
advanced 217 commits and refactored several files the Windows branch had
modified, so the conflicts were reconciled to keep BOTH main's newer structure
and the branch's cross-platform code:
- llm.ts: kept main's submodule split + activeModelInfo/validateGguf/handleCrash;
re-applied exe('llama-server') resolution and folded the Windows dev-control
restart() (clear paused + throw on failure) into main's canonical restart().
- imagegen.ts: kept main's sd-server/args refactor + exe('sd-cli').
- model-server.ts: kept main's tagLlmEntries() + the on-disk active-model
fallback; removed a duplicate llm import from the auto-merge.
- extractors.ts: adopted main's re-export from transcription/whisper-cli.
- ModelsScreen.tsx: kept main's redesigned header + injected the model-server
Stop/Restart dev control.
The whisper/ffmpeg/sd resolvers main moved into new modules lacked the exe()
Windows fix the branch previously had in extractors.ts, so re-applied it plus
the win32 PATH-for-DLL spawn pattern to transcription/whisper-cli.ts,
transcription/whisper-server.ts, and sd-server.ts.
Verified: npm run typecheck and OFFGRID_FORCE_CORE=1 npm run build both pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016wZRqu5sTN3AR1x6csoZ65
Untracked working docs for the Windows-support effort. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wZRqu5sTN3AR1x6csoZ65
Windows chat with MCP connectors failed with "failed to parse grammar" (and a follow-on 400) because the Windows llama-server.exe was fetched from llama.cpp's LATEST release while macOS builds from source at ref b9838 (scripts/build-llama.sh). Different engine versions handle the native tool-call schema->GBNF conversion differently, and the floating latest build rejected the grammar generated from the connectors' tool schemas. Pin fetch-win-binaries.ps1 to b9838 (env-overridable via LLAMA_REF) so both platforms run the identical engine. Generalized Get-LatestAssetUrl -> Get-AssetUrl with an optional tag; whisper/sd/ffmpeg stay on latest. Verified the b9838 release publishes llama-b9838-bin-win-cpu-x64.zip. Takes effect after re-running the fetch + repackaging the Windows build. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wZRqu5sTN3AR1x6csoZ65
…2/B3) Two fixes for the "chat 400s with many MCP connectors" problem the merge exposed. B3 — tool-schema budget (src/main/tools/tool-budget.ts, wired into toolChat): llama-server inlines every tool schema into the prompt AND compiles it to a grammar, so a large connector set overflowed the context window (22.8k tokens vs a 16384 ctx) and the server rejected the whole turn. budgetTools() now fits the payload into ~45% of the effective (RAM-clamped) context: it first PRUNES grammar-bloating, low-value schema keywords (numeric/string range constraints — the same ones that broke tool-call grammars — plus examples/format/long descriptions), then DROPS connector tools from the end only if still over, never below the built-ins. Never a silent cap: it logs what was dropped and tells the model via a system hint. B2 — actionable server errors (describeServerError in llm/http-post.ts): the two streaming request paths discarded the error body (res.resume()) and rejected with a bare "LLM Server Error: 400". They now read the small error body and map the common cases — context overflow, ungrammatical tool schema — to plain, user-fixable messages; other cases fall back to the server's own message. Verified: typecheck + OFFGRID_FORCE_CORE=1 build pass; budgetTools exercised on a synthetic oversized tool set (prunes, drops connectors, preserves built-ins, passes small sets through untouched). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wZRqu5sTN3AR1x6csoZ65
…t, cosine arity Three fixes for "chat is slow / errors with connectors on" on Windows. 1) Windows GPU acceleration with a safe fallback. The bundled Windows llama-server was the CPU-only build, so -ngl was a no-op and prompt processing of the tool payload crawled on CPU. fetch-win-binaries.ps1 now ships the Vulkan (GPU) build as bin/llama (offloads to any Vulkan device incl. AMD/Intel iGPUs, and still runs on CPU when no device is present) plus a CPU-only build in bin/llama-cpu. llm.ts resolves an ORDERED engine list and _doInit falls through Vulkan -> CPU via a new launchServer() helper if the primary can't start (e.g. no Vulkan loader). A failed engine is torn down with its close handler neutralized so the fallback is never misread as a crash. macOS/Linux see a single engine — no behaviour change. 2) Absolute tool-token cap (tools.ts). gemma-4's sliding-window attention defeats the prompt cache, so the full tool prompt is re-processed every tool round; the old 45%-of-context budget allowed ~9k tool tokens. Cap at ~4k (min(ctx*0.4, 4000)) to roughly halve per-round prompt cost while keeping a useful connector set. 3) cosine_similarity arity (database.ts). It was registered with a rest param (...args), whose fn.length is 0, so better-sqlite3 registered a 0-arg function and every "cosine_similarity(embedding, ?)" call threw "wrong number of arguments". Declare the two params explicitly. (The observation_fts errors are a separate, harmless matter: that table is Pro-only and absent on core builds.) Verified: typecheck + OFFGRID_FORCE_CORE=1 build pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wZRqu5sTN3AR1x6csoZ65
Two related Windows-support changes on one reusable platform helper (src/shared/device.ts, resolved in the renderer via lib/device.ts): 1. Naming: say "Mac" only on macOS, "device" on Windows/Linux. Swaps the device-context copy in SetupPanel/Onboarding/ModelsScreen/PermissionGate/ proCatalog + the main-process setup + llama-error OOM message. Code comments, the spoofed web User-Agent, and "macOS" refs left as-is. 2. Pro gating: Pro is macOS-tested only for now, so a Pro subscriber on a non-Mac platform gets a "coming soon" screen (license still works on Mac + phone) instead of the untested feature. Applies to the Pro feature tabs (App.tsx via proFeatureComingSoon) and the backend-dependent Settings sections (proactive delivery, learned prefs), while keeping plan management + identity. Free users unaffected (still see the upgrade upsell). Also: windows-build.yml checks out the private pro repo (mirrors release.yml) so the Windows exe bundles + can exercise Pro; vitest.config gains the renderer path aliases so alias-importing modules resolve in tests. Tests: device helpers (shared + renderer), proComingSoonHere / proFeatureComingSoon, platform-aware llama-error OOM case. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017mcAb498trk7GGDtCFQbyX
…ly missing The Windows binary fetch matched `bin-win-avx2-x64.zip`, which upstream stable-diffusion.cpp no longer publishes. The sd fetch is optional (verify only WARNs), so Get-AssetUrl threw "no asset matching", the try/catch swallowed it, and the Windows package shipped with NO image binary -> runtime error "Image generation binary (sd-cli) not found in resources/bin/sd". Point it at the current `bin-win-cpu-x64.zip`. CPU on purpose: the default path is the one-shot sd-cli (resident sd-server is opt-in), which reports a single exit code and so can't tell "GPU binary won't load" from "generation failed" - no launch-failure ladder like llm.ts. The bundled binary must load unconditionally; only the CPU build does. Vulkan-primary + CPU-fallback is a future upgrade once the resident readiness seam can detect load failure. Add a source-reading regression guard so a stale/dead asset name can't silently ship again. macOS is unaffected (its sd binaries ship from Git LFS, not this script). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U2S44L2JTARo75RQNRXH54
…downloading every run) transformers.js defaults env.cacheDir to `<its own module folder>/.cache`, which in a packaged app resolves INSIDE the read-only app.asar. FileCache.put() then fails every write with ENOTDIR (asar is a single file, not a directory), catches it non-fatally, and falls back to the in-memory buffer. Net effect: the ~23MB MiniLM embedding model is NEVER persisted, so every embedding operation re-downloads it from HuggingFace. On a slower/flakier link than the dev box that repeated full download surfaces as the reported "embeddings model timeout" when uploading a file to a project. Confirmed on a Windows machine: download, onnxruntime load, network, and proxy were ALL clean; the only failure was 8x ENOTDIR from FileCache.put per operation, with nothing cached to disk. So this is a cache-persistence bug, not a download/engine/ network one - bundling the model would NOT have fixed it. Point cacheDir at the same writable userData/models dir already used for localModelPath, so the model downloads once and is read from disk thereafter. Cross-platform: the same asar is read-only on macOS too (the signed .app), so a packaged Mac has the identical latent bug (masked by fast network). This fixes both; userData/models is writable on both platforms, so no regression. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U2S44L2JTARo75RQNRXH54
electron-builder.yml set mac.icon but left the win block with no icon key, so the Windows exe, NSIS installer, and window/taskbar fell back to the default Electron icon. Point win.icon at the existing 512x512 resources/icon.png (electron-builder auto-converts to a multi-size ICO), and extend the runtime BrowserWindow icon from Linux-only to Windows too. macOS is untouched (its own mac.icon + dock path stand). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U2S44L2JTARo75RQNRXH54
The Models page carried a dev/recovery control (status pill + Stop + Restart) added in 5862dcd. It is not meant for release, so remove it ahead of merging to main - the UI block, its state/handlers, and the now-orphaned IPC + preload methods it was the sole caller of (llm:status / llm:stop / llm:restart, getServerStatus / stopServer / restartServer). App.tsx's own model-server status pill is unaffected - it reads systemHealth, not these endpoints. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U2S44L2JTARo75RQNRXH54
Pro is macOS-tested only for now, but the Pro buy screen showed "Get Pro" on
Windows/Linux with no caveat - a user could buy expecting it to run locally. On
!isMac() the upgrade variant now leads with a "coming soon to your device" notice
that reassures them the purchase works today on Mac + the phone app (up to 5
devices). The buy CTA stays (the license is valid on Mac/phone). Reuses the
existing isMac()/deviceNoun() helpers; gated so macOS is unchanged.
Regression guard added (source-reading, matching the repo's no-render-harness
pattern): asserts the !isMac() gating, the copy, that the buy CTA remains, and
brand voice (" - " not em dash).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U2S44L2JTARo75RQNRXH54
… auto-updating) windows-build.yml only uploads a branch artifact - no version bump, no publish, no auto-update. Add a build-win job to release.yml that mirrors build-mac: it consumes the same resolved version + channel from the version job and publishes the NSIS installer plus the Windows electron-updater feed (latest.yml stable / beta.yml nightly) to the SAME GitHub release, so Windows and macOS ship as one version with working auto-update. Runs after build-mac to avoid a release-creation race. Windows runtimes come from fetch-win-binaries.ps1 (not the macOS LFS binaries). Code signing is wired but optional: CSC_LINK/CSC_KEY_PASSWORD from WIN_CSC_* secrets, a no-op when absent (unsigned build still publishes; SmartScreen warns until a cert is added). windows-build.yml stays for feature-branch artifact testing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U2S44L2JTARo75RQNRXH54
The versioned NSIS installer name changes every release, so a stable /releases/latest/download/<name> URL can't target it. Mirror the Mac OffGrid-latest.dmg alias for Windows: upload a constant-named copy (OffGrid-latest-setup.exe on stable, OffGrid-nightly-setup.exe on the rolling nightly) so there's a permanent "Download for Windows" URL. The versioned installer + latest.yml updater feed are untouched, so auto-update is unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U2S44L2JTARo75RQNRXH54
The app now builds, installs, and auto-updates on Windows (x64), so drop the "macOS only for now." line, add a Windows entry to Install, add a Windows badge, and widen the header download label. Dev "Build for Windows" section already existed and is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U2S44L2JTARo75RQNRXH54
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughThis PR adds Windows build and packaging workflows, Windows runtime acquisition, cross-platform executable handling, LLM startup and tool-budget safeguards, platform-aware Pro UI behavior, and Windows support and testing documentation. ChangesWindows support and cross-platform runtime
Estimated code review effort: 4 (Complex) | ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/transcription/whisper-server.ts (1)
172-174: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winImplement cross-platform orphan cleanup.
The PR objective mentions cross-platform orphan-process cleanup, but
killOrphanOnPort()(called here) relies entirely onlsofandps, which do not exist natively on Windows.Because the underlying implementation quietly swallows the
execSyncerror whenlsoffails, Windows orphans will silently retain control of the port. This will prevent a new instance from starting and effectively break transcription functionality until the user manually intervenes.Please update
killOrphanOnPort()(lines 281-297) to support Windows (e.g., usingnetstat -ano | findstror PowerShell'sGet-CimInstance), similar to how cross-platform process cleanup was implemented for the other local runtimes. Do you want me to generate the cross-platform equivalent for you?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/transcription/whisper-server.ts` around lines 172 - 174, Update killOrphanOnPort() to implement a Windows-specific process and port lookup, using an available native approach such as netstat/findstr or PowerShell, while retaining the existing lsof/ps behavior on Unix-like systems. Ensure the method identifies and terminates only the orphaned whisper-server process for the configured port and does not silently leave Windows orphans running when command execution fails.
🧹 Nitpick comments (1)
docs/WINDOWS_SUPPORT.md (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove em dashes across the Windows documentation.
All three files use em dashes despite the shared Markdown brand-voice rule. Replace every occurrence in each file with punctuation allowed by the style guide.
docs/WINDOWS_SUPPORT.md#L1-L1: update the title and remaining em-dash usage.docs/WINDOWS_TEST_MODELS.md#L1-L1: update the title and remaining em-dash usage.docs/WINDOWS_TEST_PLAN.md#L1-L1: update the title and remaining em-dash usage.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/WINDOWS_SUPPORT.md` at line 1, Replace every em dash in the Windows documentation with punctuation permitted by the Markdown style guide, including the title and remaining usages in docs/WINDOWS_SUPPORT.md (lines 1-1), docs/WINDOWS_TEST_MODELS.md (lines 1-1), and docs/WINDOWS_TEST_PLAN.md (lines 1-1).Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/windows-build.yml:
- Around line 30-34: Add persist-credentials: false to both actions/checkout
steps in .github/workflows/windows-build.yml:30-34 and
.github/workflows/windows-build.yml:41-47, covering the main repository checkout
and the pro repository checkout while preserving their existing configuration.
In `@docs/WINDOWS_SUPPORT.md`:
- Line 34: Update the Windows runtime binaries fetch documentation to
distinguish the pinned llama-server/llama.cpp version b9838 from the dynamically
resolved whisper-cli, sd-cli, and ffmpeg assets. Preserve the existing
build-time fetch and failure behavior descriptions while clarifying that only
the non-llama assets float.
- Around line 13-15: Synchronize the Windows validation documentation with the
completed real-hardware testing: update the overall validation disclaimer at
docs/WINDOWS_SUPPORT.md lines 13-15, revise the CI-build evidence row at lines
32-32, and remove or update the obsolete top-risk entry at lines 68-72 so none
claims Windows hardware testing is unverified.
In `@docs/WINDOWS_TEST_PLAN.md`:
- Line 227: Update the PowerShell curl.exe gateway command in the Windows test
plan to send valid JSON by removing the unnecessary backslash escapes inside the
single-quoted request body. Preserve the existing model, messages, and endpoint
values.
- Line 6: Update the bug-report section link near the top of
WINDOWS_TEST_PLAN.md to target the generated Markdown fragment for the heading
“3. How to report a bug,” ensuring it matches the heading’s numbered anchor and
passes markdownlint.
In `@README.md`:
- Line 227: Replace the em dashes in README.md at lines 227-227, 234-234, and
248-249 with hyphens or equivalent wording, preserving the existing
documentation meaning and formatting.
In `@src/main/llm.ts`:
- Around line 517-555: Update killOrphansOnPort to verify each candidate’s
executable path is within the paths returned by binRoots() before force-killing
it. Apply this ownership check in both Windows and non-Windows branches, retain
the existing non-llama safeguard, and leave unverified or externally installed
processes untouched.
- Around line 594-615: Update waitForReady so each /health and /v1/models fetch
uses an AbortController or equivalent per-request timeout, capped by the
remaining overall startup timeout. Ensure the probes are aborted when that
deadline expires, while preserving the existing readiness checks, retry loop,
and fallback error behavior.
- Around line 474-476: Reset intentionalStop when installing the replacement
process in the restart/reload flow, before assigning it to this.server, so
stop() state from the previous process cannot suppress recovery for a later
unexpected exit. Update the relevant replacement-assignment logic near the guard
and its corresponding path around lines 913–919, and add a regression test
covering restart() followed by an unexpected replacement-process exit.
- Around line 283-291: Update activeModelInfo() to accept cfg.id only when it is
a string; otherwise retain the filename-derived id. Preserve the existing
fallback behavior and return the declared { id: string; vision: boolean }
contract.
In `@src/main/llm/http-post.ts`:
- Around line 28-33: Replace em dashes and curly apostrophes with
brand-compliant ASCII punctuation in the user-facing messages near the
context-size and grammar/schema handling in http-post.ts. Apply the same
punctuation cleanup to the method documentation in src/main/llm.ts lines 145-146
and the strategy documentation in src/main/tools/tool-budget.ts lines 10-15; do
not otherwise alter the behavior or wording.
In `@src/main/tools.ts`:
- Line 323: Update the LLM test double used by tools-stream tests to implement
effectiveContextSize() with a deterministic value, so toolChat() can execute
without failures. Add regression assertions covering context pruning and
connector dropping, using the fake’s configured context size to verify the exact
behavior.
In `@src/main/tools/tool-budget.ts`:
- Around line 67-74: The built-in tools can remain over budget after pruning, so
expose that state from budgetTools and prevent toolChat from sending an
oversized schema. In src/main/tools/tool-budget.ts lines 67-74, add an explicit
overflow or fits result when the retained minimum still exceeds maxTokens; in
src/main/tools.ts lines 330-336, consume that result and handle the overflow
before sending tools, preserving normal sending behavior when the budget fits.
In `@src/renderer/src/App.tsx`:
- Around line 165-167: Update the URL initialization effect in App to map the
root path `/` to the platform-aware default, selecting `day` only for Pro on Mac
and `models` otherwise, so it does not overwrite the initial state on Pro
Windows. Add a regression test covering a Pro Windows launch at `/` and
asserting the Models view remains selected.
In
`@src/renderer/src/components/pro/__tests__/UpgradeScreen.windows-notice.test.ts`:
- Around line 27-40: Update the tests around the upgrade-screen source fixture
to extract the non-comingSoon upgrade branch, then assert !isMac(), Get Pro, and
PRO_PAY_URL within that captured block rather than matching the whole file. Keep
the existing notice-content assertions scoped appropriately, and ensure the
regression test verifies the purchase CTA and Mac gating belong to the same
upgrade variant.
In `@src/renderer/src/components/pro/proCatalog.ts`:
- Line 124: Update the Voice card description in proCatalog to use the
accelerator from the existing dictation settings instead of hard-coding
“Option+Space,” so platform-specific shortcuts are displayed correctly. Also
replace the em dash in that copy with the project’s supported separator while
preserving the remaining description.
In `@src/renderer/src/components/pro/UpgradeScreen.tsx`:
- Line 179: Update the app-facing product names in UpgradeScreen.tsx at lines
179-179 and PermissionGate.tsx at lines 323-323 to use “Off Grid AI Desktop”
exactly: change the Mac installation text and permission configuration text as
specified.
- Line 208: Replace the em dash in the app-facing copy in
src/renderer/src/components/pro/UpgradeScreen.tsx at lines 208-208 with a period
or approved " - " separator, and make the same punctuation change before “fine
on a 16 GB...” in src/renderer/src/components/setup/SetupPanel.tsx at lines
189-189. Ensure neither string contains an em dash.
---
Outside diff comments:
In `@src/main/transcription/whisper-server.ts`:
- Around line 172-174: Update killOrphanOnPort() to implement a Windows-specific
process and port lookup, using an available native approach such as
netstat/findstr or PowerShell, while retaining the existing lsof/ps behavior on
Unix-like systems. Ensure the method identifies and terminates only the orphaned
whisper-server process for the configured port and does not silently leave
Windows orphans running when command execution fails.
---
Nitpick comments:
In `@docs/WINDOWS_SUPPORT.md`:
- Line 1: Replace every em dash in the Windows documentation with punctuation
permitted by the Markdown style guide, including the title and remaining usages
in docs/WINDOWS_SUPPORT.md (lines 1-1), docs/WINDOWS_TEST_MODELS.md (lines 1-1),
and docs/WINDOWS_TEST_PLAN.md (lines 1-1).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a07fad27-62f7-4e5b-ad48-30ee5c1ec4e0
📒 Files selected for processing (44)
.github/workflows/release.yml.github/workflows/windows-build.ymlREADME.mddocs/WINDOWS_SUPPORT.mddocs/WINDOWS_TEST_MODELS.mddocs/WINDOWS_TEST_PLAN.mdelectron-builder.ymlscripts/fetch-win-binaries.ps1src/main/__tests__/embeddings-cachedir.test.tssrc/main/__tests__/fetch-win-binaries.test.tssrc/main/__tests__/llama-error.test.tssrc/main/database.tssrc/main/embeddings.tssrc/main/imagegen.tssrc/main/index.tssrc/main/ipc.tssrc/main/llama-error.tssrc/main/llm.tssrc/main/llm/http-post.tssrc/main/model-server.tssrc/main/runtime-env.tssrc/main/sd-server.tssrc/main/setup.tssrc/main/tools.tssrc/main/tools/tool-budget.tssrc/main/transcription/whisper-cli.tssrc/main/transcription/whisper-server.tssrc/preload/index.tssrc/renderer/src/App.tsxsrc/renderer/src/components/ModelsScreen.tsxsrc/renderer/src/components/Onboarding.tsxsrc/renderer/src/components/PermissionGate.tsxsrc/renderer/src/components/Settings.tsxsrc/renderer/src/components/pro/UpgradeScreen.tsxsrc/renderer/src/components/pro/__tests__/UpgradeScreen.windows-notice.test.tssrc/renderer/src/components/pro/proCatalog.tssrc/renderer/src/components/setup/SetupPanel.tsxsrc/renderer/src/env.d.tssrc/renderer/src/lib/__tests__/device.test.tssrc/renderer/src/lib/__tests__/proCatalog.lookup.test.tssrc/renderer/src/lib/device.tssrc/shared/__tests__/device.test.tssrc/shared/device.tsvitest.config.ts
💤 Files with no reviewable changes (1)
- src/main/ipc.ts
…gate)
tools.ts:323 calls llm.effectiveContextSize() to budget tool schemas to the
context window, but tools-stream.test.ts mocked ../llm with only { init,
streamChat } - so all 13 tests threw "effectiveContextSize is not a function".
Add it to the mock (returns a roomy 8192 so budgeting takes its normal no-prune
path). This is CI's hard gate (npm test); lint stays advisory in ci.yml.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U2S44L2JTARo75RQNRXH54
CodeRabbit review on #47, the valid + in-scope items: - security: persist-credentials: false on the Windows checkouts in windows-build.yml and release.yml build-win (they don't push, so the token needn't sit in .git/config for npm postinstall scripts; the pro checkout protects PRO_REPO_TOKEN). - docs/WINDOWS_TEST_PLAN.md: fix the bug-report anchor (#3-how-to-report-a-bug) and the PowerShell curl body (single-quoted string -> plain JSON, no literal backslashes). - docs/WINDOWS_SUPPORT.md: sync the now-completed real-hardware test status, note the llama-server b9838 pin vs the floating whisper/sd/ffmpeg assets, flip auto-update to published (release.yml build-win), refresh the top risks. - README + UpgradeScreen: brand copy - drop em dashes, use the full product name "Off Grid AI Desktop". - UpgradeScreen test: scope assertions to the upgrade branch so they can't pass if the notice/CTA drifts into the coming-soon variant. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U2S44L2JTARo75RQNRXH54
|
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
Brings the desktop app to Windows (x64) end-to-end: cross-platform binary resolution, a Windows build/publish pipeline folded into
release.yml, and platform-aware UX. Also fixes three Windows-specific runtime bugs surfaced by testing on real hardware, plus one latent cross-platform bug.What's in this branch
Build & runtime
scripts/fetch-win-binaries.ps1, theexe()helper).llama-serverpinned tob9838(parity with the macOS engine); GPU (Vulkan) primary + CPU fallback.bin-win-avx2-x64asset, so the Windows package silently shipped with nosd-cli(optional fetch → warning only) and image generation failed at runtime with "Image generation binary (sd-cli) not found". Now fetches the currentbin-win-cpu-x64build (chosen because the one-shotsd-clipath has no load-failure fallback, so the binary must load unconditionally).@xenova/transformerscached the MiniLM model inside the read-onlyapp.asar(defaultcacheDir), soFileCache.putfailed withENOTDIRevery run and the ~23 MB model re-downloaded from HuggingFace on every embed; on a slower link that reads as a timeout. PinnedcacheDirto the writableuserData/models/.cache. Cross-platform: the signed.appis read-only on macOS too, so this was a latent Mac bug masked by fast network — fixed on both.Release
release.ymlgains abuild-winjob that shares theversionjob (one version across platforms) and publishes the NSIS installer + Windows updater feed (latest.ymlstable /beta.ymlnightly) to the same GitHub release as macOS. Runs afterbuild-macto avoid a release-creation race.OffGrid-latest-setup.exe(parity with Mac'sOffGrid-latest.dmg).WIN_CSC_LINK/WIN_CSC_KEY_PASSWORD) — a no-op when absent (unsigned build still publishes).Platform-aware UX
win.iconwas unset).llm:status/llm:stop/llm:restartIPC/preload) — it wasn't meant to ship.Docs
docs/WINDOWS_SUPPORT.md.Verification
npx tsc --noEmiton bothtsconfig.node.jsonandtsconfig.web.json: clean.npm test: 892 passed, 0 failed, 6 skipped. (ci.ymlhard gates are typecheck + test; lint is advisory /continue-on-error.)llmmock intools-stream.test.tsthat was throwingeffectiveContextSize is not a function(13 failures) — pre-existing, now green.fetch-win-binaries(sd asset),embeddings-cachedir,UpgradeScreen.windows-notice— all passing.Evidence / screenshots
The changed Windows-facing surfaces can't be captured from the macOS dev/CI environment:
!isMac(), so it does not render on macOS; verified on Windows.Green Windows build (app verified there): https://github.com/off-grid-ai/OGAD/actions/runs/29492185685
Notes / follow-ups (not in this PR)
WIN_CSC_*secrets are set (SmartScreen warns). Pathway is wired for when a cloud-signing cert is procured.onnxruntime-nodeunder@huggingface/transformers(kokoro-js TTS) is missingonnxruntime_providers_shared.dllin its unpacked dir.🤖 Generated with Claude Code
https://claude.ai/code/session_01U2S44L2JTARo75RQNRXH54
Summary by CodeRabbit
New Features
Bug Fixes
Documentation