From f39ed111b35f914d374884d0e8d7110f3b06af9c Mon Sep 17 00:00:00 2001 From: Jared Ping <13906915+JaredP94@users.noreply.github.com> Date: Mon, 2 Nov 2026 16:20:40 +0200 Subject: [PATCH 1/7] Tighten CSP I don't see any pdf.js code, so I presume we don't need this. (cherry picked from commit 5365a486c3e9291960b59377b42638cd7ac352b9) Co-authored-by: Kaylee Lubick Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com> --- desktop/src/electron/csp.ts | 10 ++++------ desktop/tests/modular/csp.test.ts | 20 ++++++++++++++++++++ 2 files changed, 24 insertions(+), 6 deletions(-) create mode 100644 desktop/tests/modular/csp.test.ts diff --git a/desktop/src/electron/csp.ts b/desktop/src/electron/csp.ts index 06414b71e..f7070d369 100644 --- a/desktop/src/electron/csp.ts +++ b/desktop/src/electron/csp.ts @@ -2,13 +2,14 @@ // SPDX-License-Identifier: Apache-2.0 export function buildCspHeader(): string { - const connectSrc = ["'self'", 'http://127.0.0.1:*', 'http://localhost:*'] + // https://cheatsheetseries.owasp.org/cheatsheets/Content_Security_Policy_Cheat_Sheet.html + // https://www.w3.org/TR/CSP/ return ( [ "default-src 'none'", "script-src 'self'", "style-src 'self' 'unsafe-inline'", - `connect-src ${connectSrc.join(' ')}`, + "connect-src 'self'", // `blob:` is needed so the SVG rasterizer can assign a blob URL // to `` for decode (see `extract-svg.ts`). `data:` covers // the inline previews we generate from base64 attachments. @@ -18,10 +19,7 @@ export function buildCspHeader(): string { // fallbacks only, so no remote font origin is needed. "font-src 'self'", "manifest-src 'self'", - // `'self'` covers the Vite-served pdf.js worker module; `blob:` - // is the pdf.js fallback when the worker module can't be loaded - // directly (some Chromium builds spin up a blob-URL shim). - "worker-src 'self' blob:", + "worker-src 'none'", "object-src 'none'", "frame-src 'none'", "base-uri 'self'", diff --git a/desktop/tests/modular/csp.test.ts b/desktop/tests/modular/csp.test.ts new file mode 100644 index 000000000..a5a27e65d --- /dev/null +++ b/desktop/tests/modular/csp.test.ts @@ -0,0 +1,20 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it } from 'vitest' +import { buildCspHeader } from '@/electron/csp' + +describe('content security policy', () => { + it('keeps security-critical directives restrictive', () => { + const directives = buildCspHeader() + .split(';') + .map(directive => directive.trim()) + + expect(directives).toContain("default-src 'none'") + expect(directives).toContain("script-src 'self'") + expect(directives).toContain("connect-src 'self'") + expect(directives).toContain("worker-src 'none'") + expect(directives).toContain("base-uri 'self'") + expect(directives).toContain("object-src 'none'") + }) +}) From c435a55014bd3a23108ef786a9839ba32e539c13 Mon Sep 17 00:00:00 2001 From: Jared Ping <13906915+JaredP94@users.noreply.github.com> Date: Mon, 2 Nov 2026 16:20:40 +0200 Subject: [PATCH 2/7] fix(ui): show every GPU on a node card, not just the first (NVIDIA/Personal-AI-Router#34) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes NVIDIA/Personal-AI-Router#33. ## Problem `NodeCardDetails.tsx:276` passed only the first GPU to the node card header: ```tsx gpuLabel={gpuInfo.length > 0 ? gpuInfo[0].name : undefined} ``` `NodeLabel` took a single `gpuLabel: string | undefined`, so every GPU after the first was unrepresentable by construction. A host with a Tesla P100 **and** an RTX 3060 rendered identically to one with only the 3060, with no count or overflow hint that anything was missing. The data was never wrong — `/v1/node-info` reports every GPU, `inferenceHardwareIds` was absent (which by the contract in `gpu-inference.ts` means "show every GPU"), and the detailed performance view already charts all of them. Only the header dropped them. ## Change Emit **one label segment per GPU**. Segments are the right unit rather than a joined string: `NodeLabel` applies wrapping, `·` separators and truncation *per segment*, so `gpus.join(' · ')` would be a single segment that truncates after the first name on a narrow card — the same blind spot in a new shape. Per-GPU segments keep the existing layout behavior and let each name wrap and truncate on its own. Three supporting details: - The segment builder moves to `ui/utils/node-label-segments.ts`. The unit project runs `environment: 'node'` with no DOM and deliberately excludes components, so a pure helper is testable there without adding jsdom/testing-library to the tree. It also lands inside the existing coverage scope ("the renderer's helper and constant modules"). - `gpuLabels` is memoized in `NodeCardDetails`. Building the array inline gave it a new identity every render, and `NodeLabel` observes its segments — that would have torn down and rebuilt the `ResizeObserver` on every render. - Blank GPU names are skipped, so a backend reporting an empty name cannot render a separator with nothing after it. Behavior for 0- and 1-GPU hosts is unchanged. ## Tests `desktop/tests/modular/node-label-segments.test.ts` — 13 cases: - name/address ordering, and the address-as-primary fallback - single-GPU host unchanged - the reported dual-GPU regression, asserting topology order - `it.each([3, 4, 5, 6, 7, 8])` — every GPU present, in order, at each count; 8-GPU servers are the case that matters most here - unique, stable `gpu-N` ids across an 8-GPU host so React keys do not collide - blank names dropped without disturbing the rest - no GPU segments for a host reporting none ## Verification Run from `desktop/`, all green on this branch: | Gate | Result | |---|---| | `npm run lint` | exit 0 (one pre-existing warning in `node-info-poller.ts`, untouched here) | | `npm run typecheck` | exit 0 | | `npm run dead-code:check` | exit 0 — no dead code | | `npm run test:unit` | 38 files, 221 tests passed | --- *Proudly Made in Nebraska. Go Big Red! 🌽 * (cherry picked from commit 54d2fe33067ce66cc59e1070fb0bc3a3ed87160b) Co-authored-by: Aaron K. Clark Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com> --- .../components/NodeList/NodeCardDetails.tsx | 7 +- .../src/ui/components/NodeList/NodeLabel.tsx | 25 ++---- desktop/src/ui/utils/node-label-segments.ts | 42 ++++++++++ .../tests/modular/node-label-segments.test.ts | 77 +++++++++++++++++++ 4 files changed, 132 insertions(+), 19 deletions(-) create mode 100644 desktop/src/ui/utils/node-label-segments.ts create mode 100644 desktop/tests/modular/node-label-segments.test.ts diff --git a/desktop/src/ui/components/NodeList/NodeCardDetails.tsx b/desktop/src/ui/components/NodeList/NodeCardDetails.tsx index 3784e0737..9793dcea0 100644 --- a/desktop/src/ui/components/NodeList/NodeCardDetails.tsx +++ b/desktop/src/ui/components/NodeList/NodeCardDetails.tsx @@ -97,6 +97,11 @@ function NodeCardDetails({ node }: NodeCardDetailsProps) { }) }, [hasGpu, node, nodeMetrics]) + // Memoized so the label's segment list keeps a stable identity across + // renders; NodeLabel observes its segments and would otherwise rebuild the + // ResizeObserver every render. + const gpuLabels = useMemo(() => gpuInfo.map(gpu => gpu.name), [gpuInfo]) + const cpuFallbackInfo: CpuFallbackInfo | null = useMemo(() => { if (hasGpu) return null const ramTotal = node.topology.ram @@ -301,7 +306,7 @@ function NodeCardDetails({ node }: NodeCardDetailsProps) { 0 ? gpuInfo[0].name : undefined} + gpuLabels={gpuLabels} isLocal={isLocal} /> diff --git a/desktop/src/ui/components/NodeList/NodeLabel.tsx b/desktop/src/ui/components/NodeList/NodeLabel.tsx index 219ee88e0..b43ea8e88 100644 --- a/desktop/src/ui/components/NodeList/NodeLabel.tsx +++ b/desktop/src/ui/components/NodeList/NodeLabel.tsx @@ -4,37 +4,26 @@ import { Flex, Text } from '@nvidia/foundations-react-core' import { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react' import { LocalBadge } from '@/ui/components/LocalBadge' - -interface LabelSegment { - id: string - text: string -} +import { buildNodeLabelSegments } from '@/ui/utils/node-label-segments' export default function NodeLabel({ name, ipAddress, - gpuLabel, + gpuLabels, isLocal }: { name: string ipAddress: string - gpuLabel: string | undefined + gpuLabels: string[] isLocal?: boolean }) { const segmentRefs = useRef<(HTMLDivElement | null)[]>([]) const containerRef = useRef(null) const [separatorAfter, setSeparatorAfter] = useState([]) - const segments = useMemo(() => { - const primaryText = (name || ipAddress).toUpperCase() - const nextSegments: LabelSegment[] = [{ id: 'primary', text: primaryText }] - if (name && ipAddress) { - nextSegments.push({ id: 'ip', text: ipAddress }) - } - if (gpuLabel) { - nextSegments.push({ id: 'gpu', text: gpuLabel }) - } - return nextSegments - }, [gpuLabel, ipAddress, name]) + const segments = useMemo( + () => buildNodeLabelSegments(name, ipAddress, gpuLabels), + [gpuLabels, ipAddress, name] + ) const updateSeparators = useCallback(() => { const next = segments.map((_segment, index) => { diff --git a/desktop/src/ui/utils/node-label-segments.ts b/desktop/src/ui/utils/node-label-segments.ts new file mode 100644 index 000000000..c4d11acb3 --- /dev/null +++ b/desktop/src/ui/utils/node-label-segments.ts @@ -0,0 +1,42 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +/** A single text run in a node card's header label. */ +interface LabelSegment { + id: string + text: string +} + +/** + * Header-label segments for a node card: name, address, then one segment per + * GPU. + * + * Each GPU gets its OWN segment rather than being folded into one joined + * string, because `NodeLabel` applies wrapping, `·` separators and truncation + * per segment. A joined string is a single segment, so on a narrow card it + * truncates after the first card name and the rest of a multi-GPU host becomes + * invisible — which is the failure this shape exists to prevent. + * + * Empty GPU names are dropped so a backend that reports a blank name cannot + * render a stray separator with nothing after it. + */ +export function buildNodeLabelSegments( + name: string, + ipAddress: string, + gpuLabels: string[] +): LabelSegment[] { + const primaryText = (name || ipAddress).toUpperCase() + const segments: LabelSegment[] = [{ id: 'primary', text: primaryText }] + + if (name && ipAddress) { + segments.push({ id: 'ip', text: ipAddress }) + } + + gpuLabels.forEach((gpuLabel, index) => { + if (gpuLabel) { + segments.push({ id: `gpu-${index}`, text: gpuLabel }) + } + }) + + return segments +} diff --git a/desktop/tests/modular/node-label-segments.test.ts b/desktop/tests/modular/node-label-segments.test.ts new file mode 100644 index 000000000..1bc7231b0 --- /dev/null +++ b/desktop/tests/modular/node-label-segments.test.ts @@ -0,0 +1,77 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it } from 'vitest' +import { buildNodeLabelSegments } from '@/ui/utils/node-label-segments' + +// The node card header previously rendered `gpus[0].name` alone, so a host with +// more than one GPU was represented by its first card and every other card was +// invisible in the summary view. `buildNodeLabelSegments` emits one segment per +// GPU instead. These tests pin that every GPU survives, in order, at the counts +// real hosts actually have — including the 8-GPU servers this has to hold for. + +const gpuTexts = (name: string, ip: string, gpus: string[]) => + buildNodeLabelSegments(name, ip, gpus) + .filter(segment => segment.id.startsWith('gpu-')) + .map(segment => segment.text) + +describe('buildNodeLabelSegments', () => { + it('puts the name first, uppercased, followed by the address', () => { + const segments = buildNodeLabelSegments('pluto.local.tld', '172.16.24.46', []) + expect(segments).toEqual([ + { id: 'primary', text: 'PLUTO.LOCAL.TLD' }, + { id: 'ip', text: '172.16.24.46' } + ]) + }) + + it('falls back to the address as the primary segment when there is no name', () => { + const segments = buildNodeLabelSegments('', '172.16.24.46', []) + expect(segments).toEqual([{ id: 'primary', text: '172.16.24.46' }]) + }) + + it('renders a single-GPU host unchanged', () => { + expect(gpuTexts('TELESTO', '172.16.29.18', ['NVIDIA GeForce GTX 1060 6GB'])).toEqual([ + 'NVIDIA GeForce GTX 1060 6GB' + ]) + }) + + it('renders BOTH cards of a dual-GPU host, in topology order', () => { + // The reported case: only the first card was ever shown. + expect( + gpuTexts('pluto.local.tld', '172.16.24.46', [ + 'Tesla P100-PCIE-16GB', + 'NVIDIA GeForce RTX 3060' + ]) + ).toEqual(['Tesla P100-PCIE-16GB', 'NVIDIA GeForce RTX 3060']) + }) + + it.each([3, 4, 5, 6, 7, 8])('renders every GPU on a %i-GPU host', count => { + const gpus = Array.from({ length: count }, (_, index) => `NVIDIA H100 80GB HBM3 #${index}`) + const texts = gpuTexts('dgx-01', '10.0.0.10', gpus) + + expect(texts).toHaveLength(count) + expect(texts).toEqual(gpus) + }) + + it('keeps ids unique and stable across an 8-GPU host so React can key them', () => { + const gpus = Array.from({ length: 8 }, (_, index) => `NVIDIA A100-SXM4-80GB #${index}`) + const ids = buildNodeLabelSegments('dgx-02', '10.0.0.11', gpus).map(segment => segment.id) + + expect(new Set(ids).size).toBe(ids.length) + expect(ids).toEqual([ + 'primary', + 'ip', + ...Array.from({ length: 8 }, (_, index) => `gpu-${index}`) + ]) + }) + + it('drops blank GPU names without disturbing the indices of the rest', () => { + // A blank name would otherwise render a separator with nothing after it. + const texts = gpuTexts('mixed', '10.0.0.12', ['Tesla P100-PCIE-16GB', '', 'RTX 3060']) + expect(texts).toEqual(['Tesla P100-PCIE-16GB', 'RTX 3060']) + }) + + it('emits no GPU segments for a host that reports none', () => { + expect(gpuTexts('cpu-only', '10.0.0.13', [])).toEqual([]) + }) +}) From 2853bb6c3f51c2932fa54b195fa336aabfa89ee7 Mon Sep 17 00:00:00 2001 From: Jared Ping <13906915+JaredP94@users.noreply.github.com> Date: Mon, 2 Nov 2026 16:20:40 +0200 Subject: [PATCH 3/7] Preserve IPv6 scope zone in pairing addresses and peer URLs When the inviter was reached over IPv6 link-local, outboundIP dropped the scope zone when advertising its address, so the joiner's Completion Exchange POST had no route and pairing failed after the PIN step. Keep the zone, and render peer addresses into URLs via net/url so the zone is percent-encoded per RFC 6874 (a raw % is rejected as an invalid URL escape). (cherry picked from commit 93076e62d92753c3bbcda6678d12970631fc980a) Co-authored-by: Mallikh Kalkere Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com> --- services/nvpair-cluster-manager/invite.go | 27 +++++++- services/nvpair-cluster-manager/mtls.go | 2 +- .../pairing_linklocal_test.go | 61 +++++++++++++++++++ .../nvpair-cluster-manager/roster_http.go | 2 +- 4 files changed, 88 insertions(+), 4 deletions(-) create mode 100644 services/nvpair-cluster-manager/pairing_linklocal_test.go diff --git a/services/nvpair-cluster-manager/invite.go b/services/nvpair-cluster-manager/invite.go index 388acef19..76c523afb 100644 --- a/services/nvpair-cluster-manager/invite.go +++ b/services/nvpair-cluster-manager/invite.go @@ -15,6 +15,7 @@ import ( "log" "net" "net/http" + "net/url" "strconv" "time" @@ -331,7 +332,7 @@ func postPairingBlob(client *http.Client, target, inviteID, phase string, blob [ if err != nil { return nil, err } - resp, err := client.Post("http://"+target+pairingPath, "application/json", bytes.NewReader(body)) + resp, err := client.Post(peerURL("http", target, pairingPath), "application/json", bytes.NewReader(body)) if err != nil { return nil, err } @@ -495,6 +496,10 @@ func newInviteID() (string, error) { // outboundIP returns the local IP that routes to target, so the inviter can tell // the joiner where to POST the Completion Exchange. Returns "" if it can't be // determined (the joiner then falls back to the source address). +// +// A link-local target (fe80::/10) is only dialable with its scope zone, so the +// zone is preserved on the returned address: dropping it made the joiner's +// return POST unroutable and pairing failed after the PIN step (#69). func outboundIP(target string) string { host, _, err := net.SplitHostPort(target) if err != nil { @@ -506,7 +511,25 @@ func outboundIP(target string) string { } defer conn.Close() if ua, ok := conn.LocalAddr().(*net.UDPAddr); ok { - return ua.IP.String() + return scopedHost(ua) } return "" } + +// scopedHost renders the IP of a UDP address, preserving the scope zone that +// link-local addresses (fe80::/10) require to be dialable. net.IP.String() +// drops the zone; without it the address has no route. +func scopedHost(ua *net.UDPAddr) string { + if ua.Zone != "" { + return ua.IP.String() + "%" + ua.Zone + } + return ua.IP.String() +} + +// peerURL renders scheme://hostport + path for a peer address. The host may +// carry an IPv6 scope zone (fe80::…%en0); net/url percent-encodes it (%25en0, +// RFC 6874) because a raw % is rejected as an invalid URL escape and the peer +// would otherwise be undialable (#69). +func peerURL(scheme, hostport, path string) string { + return (&url.URL{Scheme: scheme, Host: hostport, Path: path}).String() +} diff --git a/services/nvpair-cluster-manager/mtls.go b/services/nvpair-cluster-manager/mtls.go index 2eff4c673..43d6cca3c 100644 --- a/services/nvpair-cluster-manager/mtls.go +++ b/services/nvpair-cluster-manager/mtls.go @@ -159,7 +159,7 @@ func (m *Manager) notifyPeerRemoval(addr, peerUUID string, proof RemovalProof) { return } body, _ := json.Marshal(membersRemoveRequest{NodeUUID: m.identity.NodeUUID, Proof: proof}) - resp, err := client.Post("https://"+addr+membersRemovePath, "application/json", bytes.NewReader(body)) + resp, err := client.Post(peerURL("https", addr, membersRemovePath), "application/json", bytes.NewReader(body)) if err != nil { log.Printf("notify removal to %s (%s): %v", addr, peerUUID, err) return diff --git a/services/nvpair-cluster-manager/pairing_linklocal_test.go b/services/nvpair-cluster-manager/pairing_linklocal_test.go new file mode 100644 index 000000000..f6807bc7a --- /dev/null +++ b/services/nvpair-cluster-manager/pairing_linklocal_test.go @@ -0,0 +1,61 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "net" + "net/url" + "testing" +) + +// The joiner's Completion Exchange POST targets the inviter address carried in +// PairingInfo.Addr. When the inviter was reached over IPv6 link-local, that +// address is only dialable with its scope zone (#69). +func TestScopedHostPreservesZone(t *testing.T) { + ua := &net.UDPAddr{IP: net.ParseIP("fe80::c16:bee4:c6a1:3fd"), Zone: "en0"} + if got := scopedHost(ua); got != "fe80::c16:bee4:c6a1:3fd%en0" { + t.Fatalf("scopedHost = %q, want the scope zone preserved", got) + } +} + +func TestScopedHostWithoutZoneUnchanged(t *testing.T) { + for _, ip := range []string{"192.168.86.10", "fe80::1"} { + ua := &net.UDPAddr{IP: net.ParseIP(ip)} + if got := scopedHost(ua); got != ip { + t.Errorf("scopedHost(%q) = %q, want unchanged", ip, got) + } + } +} + +// outboundIP against loopback must keep returning the bare IP (no zone). +func TestOutboundIPLoopbackHasNoZone(t *testing.T) { + if got := outboundIP("127.0.0.1:9"); got != "127.0.0.1" { + t.Fatalf("outboundIP = %q, want 127.0.0.1", got) + } +} + +// A raw % in the URL host is rejected as an invalid URL escape, so the zone +// must be percent-encoded (RFC 6874) for the joiner's return POST to parse. +func TestPeerURLZoneEncoding(t *testing.T) { + got := peerURL("http", "[fe80::c16:bee4:c6a1:3fd%en0]:14321", "/v1/cluster/pairing") + want := "http://[fe80::c16:bee4:c6a1:3fd%25en0]:14321/v1/cluster/pairing" + if got != want { + t.Fatalf("peerURL = %q, want %q", got, want) + } + u, err := url.Parse(got) + if err != nil { + t.Fatalf("peerURL output does not parse: %v", err) + } + if u.Hostname() != "fe80::c16:bee4:c6a1:3fd%en0" || u.Port() != "14321" { + t.Fatalf("reparsed host = %q port = %q, zone lost", u.Hostname(), u.Port()) + } +} + +func TestPeerURLPlainHostportUnchanged(t *testing.T) { + got := peerURL("https", "192.168.86.10:14321", "/v1/cluster/members/remove") + want := "https://192.168.86.10:14321/v1/cluster/members/remove" + if got != want { + t.Fatalf("peerURL = %q, want %q", got, want) + } +} diff --git a/services/nvpair-cluster-manager/roster_http.go b/services/nvpair-cluster-manager/roster_http.go index 8157d729e..f78425b91 100644 --- a/services/nvpair-cluster-manager/roster_http.go +++ b/services/nvpair-cluster-manager/roster_http.go @@ -265,7 +265,7 @@ func confirmedFirst(addrs []string, confirmed string) []string { // reconcileWith to try the next candidate; every actual response — including a // rejection — is that peer's answer and ends the walk. func (m *Manager) reconcileOnce(ctx context.Context, client *http.Client, body []byte, addr, peerUUID string) (outcome reconcileOutcome, provenRemoval, answered bool) { - req, err := http.NewRequestWithContext(ctx, http.MethodPost, "https://"+addr+rosterPath, bytes.NewReader(body)) + req, err := http.NewRequestWithContext(ctx, http.MethodPost, peerURL("https", addr, rosterPath), bytes.NewReader(body)) if err != nil { log.Printf("roster: build reconcile request for %s (%s): %v", addr, peerUUID, err) return reconcileUnreachable, false, false From 44cde224468d3aa92023284b6b7e443768df3947 Mon Sep 17 00:00:00 2001 From: Jared Ping <13906915+JaredP94@users.noreply.github.com> Date: Mon, 2 Nov 2026 16:20:40 +0200 Subject: [PATCH 4/7] fix(desktop): swallow EPIPE from structured logger stdout File append was already best-effort. Console write was not, so a closed stdout pipe (parent gone, or a launch that inherited a pipe that later closed) raised uncaught write EPIPE from afterWriteDispatched. Electron then showed "A JavaScript error occurred in the main process". Treat stdio the same as the log file: try/catch around write, and attach error listeners so the async EPIPE event is not an uncaughtException. (cherry picked from commit b82fe49de89b797c69b3e24039fc85b4daf5f3f6) Co-authored-by: Michael Gannotti Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com> --- desktop/src/shared/utils/log.ts | 20 +++++++- .../modular/structured-log-stdout.test.ts | 49 +++++++++++++++++++ 2 files changed, 67 insertions(+), 2 deletions(-) create mode 100644 desktop/tests/modular/structured-log-stdout.test.ts diff --git a/desktop/src/shared/utils/log.ts b/desktop/src/shared/utils/log.ts index b3c8ff7fd..40131827c 100644 --- a/desktop/src/shared/utils/log.ts +++ b/desktop/src/shared/utils/log.ts @@ -97,11 +97,18 @@ function writeEntry(scope: string, level: LogLevel, payload: StructuredLogPayloa } } - // Console output with scope prefix + // Console output with scope prefix. Best-effort: a closed stdout (parent + // process gone, launched from a pipe that later closed) raises write EPIPE + // from afterWriteDispatched. That is an uncaughtException in Electron and + // surfaces as "A JavaScript error occurred in the main process". const prefix = `(${scope})`.padEnd(24) const msg = payload.message ?? '' const dataStr = payload.data ? ` ${JSON.stringify(payload.data)}` : '' - process.stdout.write(`${now.toLocaleTimeString()} ${prefix} > ${msg}${dataStr}\n`) + try { + process.stdout.write(`${now.toLocaleTimeString()} ${prefix} > ${msg}${dataStr}\n`) + } catch { + /* best-effort */ + } } // --------------------------------------------------------------------------- @@ -141,6 +148,15 @@ export function initFileLogger(paths: PathProvider): void { } catch { approxFileSize = 0 } + + // Swallow async write errors on stdio. try/catch around write() only + // covers the synchronous throw; EPIPE from a broken pipe is emitted later. + if (process.stdout.listenerCount('error') === 0) { + process.stdout.on('error', () => {}) + } + if (process.stderr.listenerCount('error') === 0) { + process.stderr.on('error', () => {}) + } } export function createStructuredLogger(scope: string): StructuredLogger { diff --git a/desktop/tests/modular/structured-log-stdout.test.ts b/desktop/tests/modular/structured-log-stdout.test.ts new file mode 100644 index 000000000..65710fc61 --- /dev/null +++ b/desktop/tests/modular/structured-log-stdout.test.ts @@ -0,0 +1,49 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { readFileSync } from 'fs' +import { join } from 'path' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { createStructuredLogger, initFileLogger } from '@/shared/utils/log' +import { assertIsolated } from '../fixtures/isolation' +import { createTmpUserData } from '../fixtures/tmpdir' + +/** + * File append is already best-effort. Console write was not, so a closed + * stdout pipe became an uncaughtException in the Electron main process + * ("A JavaScript error occurred in the main process" / write EPIPE). + */ +describe('structured logger stdout', () => { + const tmp = createTmpUserData() + + afterEach(() => { + vi.restoreAllMocks() + }) + + function paths() { + return { + getUserData: () => tmp.dir, + getTemp: () => tmp.dir, + getResourcesPath: () => process.cwd(), + getAppName: () => 'Personal AI Router' + } + } + + it('does not throw when stdout.write fails with EPIPE', () => { + assertIsolated() + initFileLogger(paths()) + vi.spyOn(process.stdout, 'write').mockImplementation(() => { + throw new Error('write EPIPE') + }) + const log = createStructuredLogger('app') + expect(() => log.info({ sublevel: 'lifecycle', message: 'ping' })).not.toThrow() + const onDisk = readFileSync(join(tmp.dir, 'logs', 'nvpair.jsonl'), 'utf8') + expect(onDisk).toContain('"message":"ping"') + }) + + it('does not turn a later stdout error event into an uncaughtException', () => { + assertIsolated() + initFileLogger(paths()) + expect(() => process.stdout.emit('error', new Error('write EPIPE'))).not.toThrow() + }) +}) From 450b232f1519c7084da54225cec528254d09ac75 Mon Sep 17 00:00:00 2001 From: Jared Ping <13906915+JaredP94@users.noreply.github.com> Date: Mon, 2 Nov 2026 16:20:40 +0200 Subject: [PATCH 5/7] Model hub: resolve unlisted Ollama models by exact name The Ollama model hub browses a committed snapshot (desktop/src/electron/model-hub/ollama-models.json) regenerated by hand, so a model published after that snapshot is invisible in the hub even though the engine can pull it. The pull path never had this limit: pull_model forwards whatever name it is given and never consulted the catalog, so the snapshot gated discovery alone. When a search matches nothing locally and the query is a well-formed model reference, resolve that one exact name against registry.ollama.ai and fold the answer into the list as an ordinary result row. The registry speaks the same OCI distribution API ollama pull uses, so this asks the source the engine would ask rather than scraping rendered HTML. It answers 404 for /tags/list, so browsing still comes from the snapshot and this is an exact-name lookup rather than a search. A manifest carries no publication date and the registry sends no last-modified, so ModelEntry.updatedAt becomes optional and a resolved row omits its age rather than claiming it was updated just now. Lookups arrive over IPC, so the reference is validated in the main process rather than trusting the renderer's own check: without it "foo/.." walks out of the namespace and "foo/bar#z" truncates the request path. Refs NVIDIA/Personal-AI-Router#21 (cherry picked from commit 3fefdb176c58f7eeb88ab3e105e2781845fa9487) Co-authored-by: Shaun Y Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com> --- desktop/src/electron/model-hub/index.ts | 40 ++- .../src/electron/model-hub/ollama-registry.ts | 239 ++++++++++++++++++ .../electron/service-bridge/empty-handlers.ts | 4 +- desktop/src/shared/types/engine-api.ts | 9 + desktop/src/shared/types/ws-channels.ts | 7 + desktop/src/ui/api/engine-api.ts | 5 + .../components/ModelHub/EngineHubListRow.tsx | 14 +- .../components/ModelHub/ModelHubContent.tsx | 68 ++++- .../ui/components/ModelHub/ModelHubList.tsx | 7 +- desktop/src/ui/types/model-hub.ts | 7 +- desktop/src/ui/utils/model-hub-lookup.ts | 68 +++++ desktop/src/ui/utils/model-hub-search.ts | 26 +- .../tests/modular/model-hub-lookup.test.ts | 74 ++++++ .../modular/ollama-registry-lookup.test.ts | 238 +++++++++++++++++ 14 files changed, 786 insertions(+), 20 deletions(-) create mode 100644 desktop/src/electron/model-hub/ollama-registry.ts create mode 100644 desktop/src/ui/utils/model-hub-lookup.ts create mode 100644 desktop/tests/modular/model-hub-lookup.test.ts create mode 100644 desktop/tests/modular/ollama-registry-lookup.test.ts diff --git a/desktop/src/electron/model-hub/index.ts b/desktop/src/electron/model-hub/index.ts index d1fda727c..ef6882e0b 100644 --- a/desktop/src/electron/model-hub/index.ts +++ b/desktop/src/electron/model-hub/index.ts @@ -2,8 +2,13 @@ // SPDX-License-Identifier: Apache-2.0 import type { EngineType } from '@/shared/types/engines' -import type { EngineHubModel, EngineHubSearchResponse } from '@/shared/types/engine-api' +import type { + EngineHubLookupResponse, + EngineHubModel, + EngineHubSearchResponse +} from '@/shared/types/engine-api' import { loadOllamaModels, type OllamaTagsModel } from '@/electron/model-hub/ollama-library' +import { lookupOllamaModel } from '@/electron/model-hub/ollama-registry' import { lmStudioCatalogCache, type LmStudioCatalogModel @@ -15,11 +20,16 @@ function ollamaToHubModel(m: OllamaTagsModel): EngineHubModel { id: m.name, name: m.name, author: '', - url: `https://ollama.com/library/${base}`, + // A namespaced name addresses that namespace's page; a bare one is a + // first-party model under `library`. + url: base.includes('/') + ? `https://ollama.com/${base}` + : `https://ollama.com/library/${base}`, size: m.size > 0 ? m.size : undefined, downloads: 0, likes: 0, - updatedAt: m.modified_at || new Date().toISOString(), + // Empty means the source reported no date; the row omits its age. + updatedAt: m.modified_at, tags: [], family: m.details.family || undefined, parameterSize: m.details.parameter_size || undefined @@ -57,6 +67,30 @@ export async function getEngineHubModels(engineType: EngineType): Promise { + switch (engineType) { + case 'ollama': { + const found = await lookupOllamaModel(name) + return { model: found ? ollamaToHubModel(found) : null } + } + default: + return { model: null } + } +} + /** * Kick a background refresh of the live engine hub caches so the first modal * open is instant. Ollama needs no warming (it is a committed static list); diff --git a/desktop/src/electron/model-hub/ollama-registry.ts b/desktop/src/electron/model-hub/ollama-registry.ts new file mode 100644 index 000000000..62b495512 --- /dev/null +++ b/desktop/src/electron/model-hub/ollama-registry.ts @@ -0,0 +1,239 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import axios, { isAxiosError } from 'axios' +import { createStructuredLogger } from '@/shared/utils/log' +import getErrorString from '@/shared/utils/get-error-string' +import type { OllamaTagsModel } from '@/electron/model-hub/ollama-library' + +const log = createStructuredLogger('ollama-registry') + +/** + * Resolve one Ollama model by exact name, for names the committed catalog + * snapshot (`ollama-models.json`) does not carry. + * + * `registry.ollama.ai` speaks the OCI distribution API that `ollama pull` uses, + * so this asks the same source the engine would. It only reads manifests: the + * registry answers 404 for `/tags/list`, so enumerating a catalog still needs + * the snapshot and a lookup is an exact-name check rather than a search. + */ + +const REGISTRY_BASE = 'https://registry.ollama.ai/v2' +const USER_AGENT = 'PAIR/1.0' + +/** + * Bounds the whole request rather than just the response. `axios`'s `timeout` is + * a socket timeout and does not start until a socket exists, so it misses a + * wedged DNS resolver; an `AbortSignal` covers every phase. `lmstudio-catalog.ts` + * carries the same reasoning at more length. + */ +const HTTP_TIMEOUT_MS = 8_000 + +/** What `ollama pull ` resolves a bare name to. */ +const DEFAULT_TAG = 'latest' + +/** Ollama's namespace for first-party models, implied by a bare name. */ +const DEFAULT_NAMESPACE = 'library' + +/** + * Characters a repository segment or tag may contain. This is the only guard on + * what reaches the request URL, so it has to hold on its own: the renderer runs + * its own check before asking, but a lookup arrives over IPC and main-process + * code does not treat the renderer as trusted. Without it, `foo/..` walks out of + * the namespace and `foo/bar#z` truncates the path. + */ +const SAFE_SEGMENT = /^[a-zA-Z0-9][a-zA-Z0-9._-]*$/ + +/** + * Positive and negative results both cached, because the hub looks names up as + * the user types and would otherwise re-request on every keystroke past a + * resolved name. Bounded so a long session cannot grow it without limit. + */ +const CACHE_LIMIT = 256 +const cache = new Map() + +function remember(key: string, value: OllamaTagsModel | null): OllamaTagsModel | null { + if (cache.size >= CACHE_LIMIT) { + // Map iteration is insertion-ordered, so the first key is the oldest. + const oldest = cache.keys().next() + if (!oldest.done) cache.delete(oldest.value) + } + cache.set(key, value) + return value +} + +/** + * Split `[namespace/]name[:tag]` into a registry path and tag, or null for a + * reference this registry cannot answer. + * + * OCI clients read a dot in the first of *several* segments as a registry + * hostname, which is how `hf.co//` is recognized as someone else's + * registry and declined rather than reported as missing. That rule must not + * reach a lone segment: bare model names legitimately carry dots — `qwen3.8`, + * `llama3.1`, `phi3.5` — and are always first-party names under `library`. + */ +function parseReference(name: string): { path: string; tag: string } | null { + const colon = name.lastIndexOf(':') + const slash = name.lastIndexOf('/') + // A colon ahead of the last slash belongs to a host:port, not a tag. + const hasTag = colon > slash + const repository = hasTag ? name.slice(0, colon) : name + const tag = hasTag ? name.slice(colon + 1) : DEFAULT_TAG + if (!SAFE_SEGMENT.test(tag)) return null + + const segments = repository.split('/') + if (!segments.every(segment => SAFE_SEGMENT.test(segment))) return null + if (segments.length === 1) return { path: `${DEFAULT_NAMESPACE}/${segments[0]}`, tag } + if (segments.length > 2 || segments[0].includes('.')) return null + return { path: repository, tag } +} + +function isRecord(v: unknown): v is Record { + return v !== null && typeof v === 'object' +} + +function manifestLayers(raw: unknown): Record[] { + if (!isRecord(raw) || !Array.isArray(raw.layers)) return [] + return raw.layers.filter(isRecord) +} + +/** + * Total bytes the engine downloads: every layer, not just the weights. The + * others (template, params, license) are a few kilobytes each, but the sum is + * what lands on disk. + */ +function totalLayerBytes(raw: unknown): number { + let total = 0 + for (const layer of manifestLayers(raw)) { + if (typeof layer.size === 'number' && layer.size > 0) total += layer.size + } + return total +} + +/** + * Capability chip for the row. `details.family` carries a capability rather than + * a model family here, because that is what the catalog scraper puts there and + * what the row renders. + * + * Vision is the only one a manifest proves: a multimodal model carries an extra + * `projector` layer for its vision encoder and nothing else does. Tools, + * thinking and embedding exist only on ollama.com's rendered page, so a model + * with those and no projector gets no chip instead of a wrong one. + */ +function capabilityFromLayers(raw: unknown): string { + const vision = manifestLayers(raw).some( + layer => layer.mediaType === 'application/vnd.ollama.image.projector' + ) + return vision ? 'vision' : '' +} + +function configDigest(raw: unknown): string { + if (!isRecord(raw) || !isRecord(raw.config)) return '' + return typeof raw.config.digest === 'string' ? raw.config.digest : '' +} + +/** The 12-character form Ollama shows in its own model lists. */ +function shortenDigest(digest: string): string { + return digest.replace(/^sha256:/, '').slice(0, 12) +} + +/** + * What the model says about itself, read from the config blob the manifest + * points at. Worth the extra request because a tag like `27b-coding-mxfp8` is a + * label the publisher chose, while these are the real figures. + */ +interface OllamaModelConfig { + format: string + families: string[] | null + parameterSize: string + quantizationLevel: string +} + +/** Resolves to null on any failure; the row still renders, with less on it. */ +async function fetchConfig(path: string, digest: string): Promise { + if (!digest) return null + try { + const { data } = await axios.get(`${REGISTRY_BASE}/${path}/blobs/${digest}`, { + headers: { 'User-Agent': USER_AGENT, Accept: 'application/json' }, + signal: AbortSignal.timeout(HTTP_TIMEOUT_MS) + }) + if (!isRecord(data)) return null + const families = Array.isArray(data.model_families) + ? data.model_families.filter((f): f is string => typeof f === 'string') + : [] + return { + format: typeof data.model_format === 'string' ? data.model_format : '', + families: families.length > 0 ? families : null, + parameterSize: typeof data.model_type === 'string' ? data.model_type : '', + quantizationLevel: typeof data.file_type === 'string' ? data.file_type : '' + } + } catch { + return null + } +} + +/** + * Look one exact model name up in Ollama's registry. + * + * Null covers every "not available" case alike — a reference this registry does + * not serve, a model or tag that does not exist, a timeout, an offline machine. + * None of them surface as an error: the lookup is speculative, and the user + * asked to search rather than to make this request. + */ +export async function lookupOllamaModel(name: string): Promise { + const cached = cache.get(name) + if (cached !== undefined) return cached + + const reference = parseReference(name) + if (!reference) return remember(name, null) + + try { + const { data } = await axios.get( + `${REGISTRY_BASE}/${reference.path}/manifests/${reference.tag}`, + { + headers: { + 'User-Agent': USER_AGENT, + Accept: 'application/vnd.docker.distribution.manifest.v2+json' + }, + signal: AbortSignal.timeout(HTTP_TIMEOUT_MS) + } + ) + const size = totalLayerBytes(data) + if (size === 0) return remember(name, null) + + const digest = configDigest(data) + const config = await fetchConfig(reference.path, digest) + + log.info({ sublevel: 'lookup', message: `resolved ${name} outside the catalog snapshot` }) + return remember(name, { + name, + model: name, + // Neither the manifest nor its headers carry a publication date. + // Empty means unknown, and the row omits its age rather than + // claiming the model was updated just now. + modified_at: '', + size, + digest: shortenDigest(digest), + details: { + parent_model: '', + format: config?.format ?? '', + family: capabilityFromLayers(data), + families: config?.families ?? null, + // Falls back to the tag, which is what the catalog shows here. + parameter_size: + config?.parameterSize || (reference.tag === DEFAULT_TAG ? '' : reference.tag), + quantization_level: config?.quantizationLevel ?? '' + } + }) + } catch (err) { + const status = isAxiosError(err) ? err.response?.status : undefined + // 404 is the ordinary answer for a name that does not exist. + if (status !== 404) { + log.warn({ + sublevel: 'http', + message: `lookup for ${name} failed: ${status ?? ''} ${getErrorString(err)}`.trim() + }) + } + return remember(name, null) + } +} diff --git a/desktop/src/electron/service-bridge/empty-handlers.ts b/desktop/src/electron/service-bridge/empty-handlers.ts index 0bae0f5a5..1a4c6303d 100644 --- a/desktop/src/electron/service-bridge/empty-handlers.ts +++ b/desktop/src/electron/service-bridge/empty-handlers.ts @@ -20,7 +20,7 @@ import { MODULAR_ENGINE_LIFECYCLE_CALL_TIMEOUT_MS } from '@/shared/constants/modular-runtime' import getErrorString from '@/shared/utils/get-error-string' -import { getEngineHubModels } from '@/electron/model-hub' +import { getEngineHubModels, lookupEngineHubModel } from '@/electron/model-hub' import { getModularSupervisor } from './modular-supervisor' import { getModularBridgeState, @@ -1180,6 +1180,8 @@ const EMPTY_SERVICE_BRIDGE_HANDLERS: BridgeHandlerMap = { 'engine:command': payload => handleEngineCommand(payload), 'engine:search-hub': payload => payload ? getEngineHubModels(payload.engineType) : { models: [] }, + 'engine:lookup-hub-model': payload => + payload ? lookupEngineHubModel(payload.engineType, payload.name) : { model: null }, 'errors:get-initial': () => handleErrorsGetInitial(), 'errors:clear': payload => (payload ? handleErrorsClear(payload) : null), diff --git a/desktop/src/shared/types/engine-api.ts b/desktop/src/shared/types/engine-api.ts index 351f5c5be..208caf2ab 100644 --- a/desktop/src/shared/types/engine-api.ts +++ b/desktop/src/shared/types/engine-api.ts @@ -37,6 +37,15 @@ export interface EngineHubSearchResponse { models: EngineHubModel[] } +/** + * Response for the `engine:lookup-hub-model` channel. `null` means the name is + * not available, whether because it does not exist, the engine has no registry + * to ask, or that registry could not be reached. + */ +export interface EngineHubLookupResponse { + model: EngineHubModel | null +} + /** Engine command discriminator. */ export type EngineCommandType = | 'toggle' diff --git a/desktop/src/shared/types/ws-channels.ts b/desktop/src/shared/types/ws-channels.ts index 7e66c85f2..111a34719 100644 --- a/desktop/src/shared/types/ws-channels.ts +++ b/desktop/src/shared/types/ws-channels.ts @@ -25,6 +25,7 @@ */ import type { EngineCommandPayload, + EngineHubLookupResponse, EngineHubSearchResponse, EngineInitialState, EngineStatePatch @@ -116,6 +117,12 @@ export interface WsInvokeChannelMap { 'engines:get-initial': { request: void; response: EngineInitialState } 'engine:command': { request: EngineCommandPayload; response: null } 'engine:search-hub': { request: { engineType: EngineType }; response: EngineHubSearchResponse } + // Exact-name lookup, used only when a search matches nothing, so the + // catalog the app ships with cannot hide a model the engine could pull. + 'engine:lookup-hub-model': { + request: { engineType: EngineType; name: string } + response: EngineHubLookupResponse + } // Errors 'errors:get-initial': { request: void; response: ServiceError[] } diff --git a/desktop/src/ui/api/engine-api.ts b/desktop/src/ui/api/engine-api.ts index 69fd71a24..e691ba981 100644 --- a/desktop/src/ui/api/engine-api.ts +++ b/desktop/src/ui/api/engine-api.ts @@ -9,6 +9,7 @@ */ import type { PreloadServiceTransport as ServiceTransport } from '@/shared/types/service-bridge' import type { + EngineHubLookupResponse, EngineHubSearchResponse, EngineInitialState, EngineStatePatch, @@ -49,6 +50,8 @@ export interface IEngineApi { setModelExpiry(engineType: EngineType, nodeId: string, model: string, expiry: string): void /** Search the model registry/hub for available models. */ searchHub(engineType: EngineType): Promise + /** Resolve one exact model name the hub catalog does not list. */ + lookupHubModel(engineType: EngineType, name: string): Promise /** Durable engine state changed. Prefer this for new renderer state. */ onStateChanged(callback: (patch: EngineStatePatch) => void): () => void @@ -103,6 +106,8 @@ export function createEngineApi(transport: ServiceTransport): IEngineApi { expiry }), searchHub: engineType => transport.invoke('engine:search-hub', { engineType }), + lookupHubModel: (engineType, name) => + transport.invoke('engine:lookup-hub-model', { engineType, name }), onStateChanged: cb => transport.subscribePush('engines:state-changed', cb), onProgress: cb => transport.subscribePush('engines:progress-changed', cb), diff --git a/desktop/src/ui/components/ModelHub/EngineHubListRow.tsx b/desktop/src/ui/components/ModelHub/EngineHubListRow.tsx index 10c0ccdb2..17bba0431 100644 --- a/desktop/src/ui/components/ModelHub/EngineHubListRow.tsx +++ b/desktop/src/ui/components/ModelHub/EngineHubListRow.tsx @@ -75,12 +75,14 @@ const EngineHubListRow = memo(function EngineHubListRow({ )} - - Updated {formatRelativeTime(new Date(model.updatedAt).getTime())} - + {!!model.updatedAt && ( + + Updated {formatRelativeTime(model.updatedAt.getTime())} + + )} {!!model.url && (