fix(ai): Ollama/LM Studio/vLLM desktop discovery + PWA CORS noise (#266) + vendor-fork audit (#60)#269
Open
qnbs wants to merge 14 commits into
Open
fix(ai): Ollama/LM Studio/vLLM desktop discovery + PWA CORS noise (#266) + vendor-fork audit (#60)#269qnbs wants to merge 14 commits into
qnbs wants to merge 14 commits into
Conversation
Root-cause analysis for Ollama/LM Studio/vLLM discovery on Tauri and the PWA CORS noise: WebView fetch to localhost is cross-origin (CORS/PNA), CSP was never the blocker. Decision: route local-server HTTP through the Tauri HTTP plugin; PWA stays strictly desktop-only. Help articles updated (Tauri desktop, provider FAQ).
New thin services/localServerHttp.ts: runtime-aware fetch (native @tauri-apps/plugin-http on desktop — CORS-free, browser fetch on web), shared base-URL normalization, timeout composition via AbortSignal.any, and unreachable/timeout error classification. User aborts rethrow unchanged (cancel != failure). ollamaService (test/list/stream/pull) and scanLocalOpenAiCompatibleEndpoints now use it; the scan returns per-endpoint state (ok/unreachable/timeout/http) for actionable UI badges. Public signatures and legacy error strings are preserved; new unit tests cover both fetch branches and classification.
…dges (#266) - The settings auto-effect and model loading no longer probe localhost in the PWA (that was the CORS console noise); a quiet 'desktop app required' banner with a download CTA replaces it. testAIConnection's desktop-only guard is unchanged. - Desktop scan results render classified status badges (reachable / no response / timeout / HTTP error) with aria-live, plus a one-click 'Use this URL' action that adopts the found base URL + matching preset. - Tauri http capability: http:default alone grants NO url scope (plugin allow-list denied every call, incl. desktop cloud AI). Scope now allows loopback any-port + the cloud endpoints mirroring the Tauri CSP.
New settings.ai keys (desktop-only banner title/body, download CTA, scan timeout/HTTP states, use-this-URL) plus a corrected ollamaHint (no CORS setup needed on desktop), translated for all 19 locales; runtime bundles rebuilt (53827 keys).
… dep (#60) Full-file diff of packages/collab-transport against upstream y-webrtc 10.3.0: no deviations beyond the three documented SC patches (PBKDF2 600k, extractable:false, return-before-reject) + DataChannel E2E encryption. Recorded in packages/collab-transport/AUDIT.md; fork bumped to -sc2. Cleanup: removed the deprecated patches/y-webrtc@10.3.0.patch and the dead root y-webrtc dependency (zero imports). Created the previously-referenced- but-missing scripts/verify-vendor-fork.mjs invariant guard and wired it into package.json (verify:vendor) + the CI security job.
- Experimental-category search hints referenced retired/promoted flags (plot board, codex, cross project); replaced with current features. - ProForgeDashboard uses the catalog-driven MaturityBadge instead of a hard-coded Experimental pill (v1.24 maturity-label convention). - AGENTS.md drift: six opt-in flags (enableProForge), 19 locales.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 8 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (56)
📝 WalkthroughWalkthroughThis PR adds runtime-aware local AI server connectivity, desktop-only Ollama probing, classified endpoint scan results, localized setup and status messaging, and Tauri HTTP capability rules. It also audits and verifies the vendored y-webrtc fork in CI, updates ProForge maturity labeling, and removes stale search hints. ChangesLocal AI connectivity
Vendored fork security
Settings and dashboard hygiene
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant AiProviderCard
participant aiProviderService
participant localServerHttp
participant TauriHTTP
User->>AiProviderCard: Scan local ports
AiProviderCard->>aiProviderService: scanLocalOpenAiCompatibleEndpoints()
aiProviderService->>localServerHttp: localServerFetch(endpoint, timeoutMs)
localServerHttp->>TauriHTTP: Fetch through Tauri plugin on desktop
TauriHTTP-->>localServerHttp: HTTP response or transport error
localServerHttp-->>aiProviderService: Classified result
aiProviderService-->>AiProviderCard: Render status and usable URL
User->>AiProviderCard: Use URL
AiProviderCard-->>User: Apply Ollama base URL and preset
Possibly related issues
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/ollamaService.ts (1)
192-207: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPreserve caller cancellation in
streamOllama.
localServerFetchrethrowsAbortError, but this catch wraps it as a generic connection failure. A user cancellation can therefore enter fallback/error handling instead of stopping the request. Re-throw aborts before wrapping.Proposed fix
} catch (err) { + if (isAbortError(err)) throw err; const error = new Error( `Ollama not reachable (${baseUrl}). Make sure Ollama is running: ollama serve`, { cause: err as Error },🤖 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 `@services/ollamaService.ts` around lines 192 - 207, Update the catch block in streamOllama around localServerFetch to detect an AbortError and re-throw it unchanged before constructing the wrapped Ollama connection error. Preserve the existing wrapping behavior for non-abort failures and keep the current callback contract intact.
🧹 Nitpick comments (1)
tests/unit/localServerHttp.test.ts (1)
1-19: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd the required QNBS-v3 rationale marker.
This non-trivial test module adds runtime-routing and transport-error contract coverage but has no single-line
QNBS-v3comment explaining that purpose.As per coding guidelines, “For non-trivial changes, add one single-line
QNBS-v3comment explaining why the change was made.”🤖 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 `@tests/unit/localServerHttp.test.ts` around lines 1 - 19, Add one single-line QNBS-v3 comment near the imports in the localServerHttp test module, briefly explaining that the tests cover runtime routing and transport-error contracts. Keep the comment focused on the purpose of this non-trivial test coverage.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 `@AGENTS.md`:
- Line 390: Synchronize the feature-flag documentation so the opt-in count and
flag list match across the sections near the feature flag overview and the “23
flags” statement. Use the authoritative six opt-in flags—enableRtlLayout,
enableVoiceSupport, enableProForge, enableVoiceWasm, enableGlobalCopilot, and
enableLocalFirstSync—and remove the stale five-flag wording.
In `@CHANGELOG.md`:
- Around line 128-132: Update the feature-count statement in the changelog
bullet to reflect the final UI state of 22 flags, or explicitly label 23 flags
as an intermediate state; keep the surrounding catalog/default-state and test
details unchanged.
In `@components/settings/AiProviderCard.tsx`:
- Around line 75-80: Gate the user-triggered Ollama controls in AiProviderCard
using the existing isDesktop state: hide or disable both the “Load models”
action and the shared “Test connection” button when !isDesktop. Update the
relevant rendering paths around the Ollama model-loading handler and the shared
connection action, while preserving their current desktop behavior.
- Around line 121-133: Update handleUseScannedUrl and its surrounding follow-up
flow so adopting a scanned URL cannot trigger Ollama-specific model loading or
testing for lm_studio or vllm presets. Either restrict the action to Ollama URLs
or branch the subsequent checks by the selected localBackendPreset, while
preserving the existing Ollama behavior.
In `@locales/es/settings.json`:
- Around line 209-211: Remove the incorrect CSP attribution from the
settings.ai.ollamaDesktopOnlyBody translation while retaining the browser
CORS/private-network explanation. Apply the same wording update in
locales/es/settings.json (209-211), public/locales/is/bundle.json (1817-1819),
public/locales/it/bundle.json (1817-1819), public/locales/ja/bundle.json
(1817-1819), public/locales/ko/bundle.json (1817-1819), and
public/locales/pt/bundle.json (1817-1819), keeping all locale copies aligned.
In `@packages/collab-transport/src/crypto.js`:
- Around line 4-13: Update the vendor header comments to use the SC patch
terminology, reference the implementation’s decrypt() symbol instead of
encryptMessageContent, and add the required single-line QNBS-v3 rationale for
the JavaScript change. Keep the existing patch and audit metadata unchanged.
In `@scripts/verify-vendor-fork.mjs`:
- Around line 41-45: Update the DataChannel invariant check in the verification
script to validate encryption and decryption at the actual data-flow sites, not
merely anywhere in webrtcSrc. Assert that sendWebrtcConn and broadcastWebrtcConn
encrypt payloads before peer.send, and that the peer.on('data')/readPeerMessage
path decrypts inbound payloads; use targeted regexes or a structured parser
check.
- Around line 50-54: Expand Invariant 6 in the verification script beyond
rootPkg and the single y-webrtc@10.3.0.patch path: scan every workspace package
manifest, pnpm-lock.yaml, and the patches directory for any y-webrtc dependency,
patchedDependency, lockfile entry, or y-webrtc@*.patch file. Keep the invariant
passing only when no stale upstream wiring exists anywhere in these dependency
surfaces.
In `@tests/unit/aiProviderService.test.ts`:
- Around line 685-713: Update the scanLocalOpenAiCompatibleEndpoints tests to
isolate each global fetch mock by using vi.stubGlobal and restoring globals with
vi.unstubAllGlobals, or by saving and restoring the original globalThis.fetch.
Ensure cleanup runs between tests so the HTTP, timeout, and network-failure
cases cannot affect one another.
In `@VENDOR-FORKS.md`:
- Around line 39-40: Update the maintenance instruction around the
advisory-recording guidance to replace the stale VENDOR-DIFF.md destination with
packages/collab-transport/AUDIT.md, keeping the release-record workflow
consistent with the earlier statement.
---
Outside diff comments:
In `@services/ollamaService.ts`:
- Around line 192-207: Update the catch block in streamOllama around
localServerFetch to detect an AbortError and re-throw it unchanged before
constructing the wrapped Ollama connection error. Preserve the existing wrapping
behavior for non-abort failures and keep the current callback contract intact.
---
Nitpick comments:
In `@tests/unit/localServerHttp.test.ts`:
- Around line 1-19: Add one single-line QNBS-v3 comment near the imports in the
localServerHttp test module, briefly explaining that the tests cover runtime
routing and transport-error contracts. Keep the comment focused on the purpose
of this non-trivial test coverage.
🪄 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
Run ID: a5716990-2af4-4e31-bcaf-5f6a01f2f192
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (62)
.github/workflows/ci.ymlAGENTS.mdCHANGELOG.mdVENDOR-FORKS.mdcomponents/proForge/ProForgeDashboard.tsxcomponents/settings/AiProviderCard.tsxdocs/LOCAL-AI.mddocs/adr/0012-local-server-connectivity-tauri-http.mdlocales/ar/settings.jsonlocales/de/settings.jsonlocales/el/settings.jsonlocales/en/help.jsonlocales/en/settings.jsonlocales/es/settings.jsonlocales/eu/settings.jsonlocales/fa/settings.jsonlocales/fi/settings.jsonlocales/fr/settings.jsonlocales/he/settings.jsonlocales/hu/settings.jsonlocales/is/settings.jsonlocales/it/settings.jsonlocales/ja/settings.jsonlocales/ko/settings.jsonlocales/pt/settings.jsonlocales/ru/settings.jsonlocales/sv/settings.jsonlocales/zh/settings.jsonpackage.jsonpackages/collab-transport/AUDIT.mdpackages/collab-transport/package.jsonpackages/collab-transport/src/crypto.jspatches/y-webrtc@10.3.0.patchpublic/locales/ar/bundle.jsonpublic/locales/de/bundle.jsonpublic/locales/el/bundle.jsonpublic/locales/en/bundle.jsonpublic/locales/es/bundle.jsonpublic/locales/eu/bundle.jsonpublic/locales/fa/bundle.jsonpublic/locales/fi/bundle.jsonpublic/locales/fr/bundle.jsonpublic/locales/he/bundle.jsonpublic/locales/hu/bundle.jsonpublic/locales/is/bundle.jsonpublic/locales/it/bundle.jsonpublic/locales/ja/bundle.jsonpublic/locales/ko/bundle.jsonpublic/locales/pt/bundle.jsonpublic/locales/ru/bundle.jsonpublic/locales/sv/bundle.jsonpublic/locales/zh/bundle.jsonscripts/verify-vendor-fork.mjsservices/aiProviderService.tsservices/localServerHttp.tsservices/ollamaService.tsservices/settingsSearchHints.tssrc-tauri/capabilities/default.jsontests/unit/aiProviderService.test.tstests/unit/localServerHttp.test.tstests/unit/ollamaService.test.tstests/unit/settings/AiProviderCard.test.tsx
💤 Files with no reviewable changes (1)
- patches/y-webrtc@10.3.0.patch
🏁 CodeAnt Quality Gate ResultsCommit: ✅ Overall Status: PASSEDQuality Gate Details
|
The CI security gate (pnpm audit --audit-level=high) started failing on every PR: 9 high advisories in transitive dev paths. Overrides raised to the patched lines (bounded to avoid unvetted major jumps): adm-zip >=0.6.0, axios >=1.18.0, brace-expansion >=5.0.8, fast-uri >=3.1.4 <4, js-yaml >=4.3.0 <5, postcss >=8.5.18, sharp >=0.35.0. Also reverts an accidental packageManager pin to pnpm@11.17.0 (back to 11.5.2).
The npm audit bulk endpoint intermittently returns an undecoded gzip body (ERR_PNPM_AUDIT_BAD_RESPONSE) — a registry/CDN-side flake, not a finding. 3 attempts with 20s backoff; real advisories still fail the gate.
…gate, vendor checks - AiProviderCard: disable 'Load models' + 'Test connection' for ollama in PWA - i18n: ollamaDesktopOnlyBody blames CORS/PNA, not CSP (ADR-0012) — all 19 locales + bundles - ci: pnpm audit step now advisory (continue-on-error) — registry serves force-gzipped body pnpm cannot decode (ERR_PNPM_AUDIT_BAD_RESPONSE, deterministic per CDN edge); OSV scan of both lockfiles remains the enforced gate - verify-vendor-fork: path-sensitive DataChannel encrypt/decrypt checks + full surface scan (workspace manifests, pnpm-lock, patches glob) - ollamaService.streamOllama: rethrow AbortError before wrapping (caller cancel ≠ unreachable) - collab-transport crypto.js header: SC patch naming, decrypt() fix, QNBS-v3 tag - tests: scan tests use vi.stubGlobal/unstubAllGlobals; localServerHttp header comment - docs: flag count truth 22 (16 on / 6 off) across AGENTS.md, CLAUDE.md, copilot-instructions.md, FEATURE-PARITY.md, CHANGELOG; VENDOR-FORKS.md AUDIT.md ref
Owner
Author
|
Review round addressed (commits e985623 + 0342b98): Outside-diff comment (services/ollamaService.ts — Major): Nitpick (tests/unit/localServerHttp.test.ts): QNBS-v3 rationale comment added near the imports. All 10 actionable inline comments: fixed and replied individually above. Local gates green: CI note: the Security Audit job was red because |
…ity gate - Cargo.lock: quick-xml 0.39.4 → 0.41.0 (via plist 1.10.0; RUSTSEC-2026-0194/0195, CVSS 7.5), anyhow → 1.0.104, crossbeam-epoch → 0.9.20, serde_with → 3.21.0 - pnpm overrides: dompurify → 3.4.12, protobufjs → 8.7.1, body-parser → 1.20.6 (<2) - osv-scanner.toml: drop 2 stale ignores whose advisories no longer match the lockfile - CHANGELOG: record the remediation batch + advisory pnpm audit step
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
…e source (#60) The 'patch file exists' smoke test went stale when patches/y-webrtc@10.3.0.patch was deleted in the vendor-fork cleanup; the invariant is now the file's absence.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
…RAGE-POLICY.md Patch coverage is informational (no codecov.yml target, check passes); the 8 missing/partial lines are recorded as open follow-ups with a ≥90% backfill target.
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
Fixes #266 · Refs #60
#266 — Local AI server connectivity (root cause: CORS, not CSP)
ollamaService,scanLocalOpenAiCompatibleEndpoints) used the WebViewfetch; inside the Tauri shell cross-originlocalhostcalls died on CORS/PNA, and the PWA settings auto-effect probedlocalhost:11434on every visit (loud CORS console errors).services/localServerHttp.ts: native@tauri-apps/plugin-httpon desktop (CORS-free), browser fetch on web; shared URL normalization, timeout composition,unreachable/timeoutclassification; user aborts propagate unchanged.http:defaultgrants no URL scope — every plugin-http call (incl. desktop cloud AI viafetchAdapter) was silently denied. Capability now allows loopback any-port + the cloud endpoints mirroring the Tauri CSP.#60 — y-webrtc vendor-fork audit
y-webrtc@10.3.0: only the 3 documented SC patches + DataChannel E2E encryption →packages/collab-transport/AUDIT.md, bump10.3.0-sc2.patches/y-webrtc@10.3.0.patch+ dead rooty-webrtcdep; created the missingscripts/verify-vendor-fork.mjsinvariant guard, wired asverify:vendorinto the CI security job.Hygiene
ProForgeDashboarduses catalog-drivenMaturityBadge; AGENTS.md drift (6 opt-in flags, 19 locales).How to verify
pnpm run lint && pnpm run typecheck && pnpm run i18n:check— all green locally; unit tests for every changed path (localServerHttp, ollamaService, aiProviderService scan/testAIConnection, AiProviderCard PWA+desktop branches, settingsSearchHints, proForge suite).tauri-build.ymlon this branch → Settings → AI → Ollama → Scan common local ports shows live badges with a runningollama serve; Use this URL adopts it; model list + test connection succeed withoutOLLAMA_ORIGINS.pnpm run verify:vendor→ all 8 invariants ✓ (also runs in the security job).Notes
[Unreleased](Fixed / Security / Changed).Summary by CodeRabbit
New Features
Bug Fixes
Documentation