From cd1b9344baa0380ad189b538a98daea4811d6aaa Mon Sep 17 00:00:00 2001 From: Quickbeard Date: Fri, 11 Sep 2026 11:50:15 +0700 Subject: [PATCH] fix(desktop): route Skill Hub through the proxy-aware stack and refuse off-host project installs Hub browsing and installs in the desktop app went straight to Node's fetch, which ignores the system proxy and PAC settings that the gateway SSO calls already honor via Electron's net.fetch. @codev/skillhub now has an injectable transport (setHubTransport) that also carries the silent SSO refresh, the desktop passes net.fetch, and the controller waits out Chromium's proxy initialization once before the first hub request so it is not charged to the 15 s request timeout. Chromium's net::ERR_* codes classify as network errors. The install dialog offered every project of the connected server, but the main process writes to the host disk, so a WSL, sandbox, or remote project landed in the wrong place. "This project" is now disabled off the local server with a note naming the server and pointing at `codev skill pull`, and the controller checks the folder exists on the host before any hub traffic (new `missing-directory` error code). Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01QaEgv2JVUbep1sBYApZDFW --- CHANGELOG.md | 16 +++++ DIVERGENCES.md | 2 +- packages/app/src/i18n/en.ts | 3 + packages/app/src/i18n/es.ts | 3 + packages/app/src/i18n/vi.ts | 3 + packages/app/src/skillhub/dialog-install.tsx | 25 +++++-- packages/app/src/skillhub/types.ts | 7 +- packages/codev-skillhub/src/http.ts | 17 ++++- packages/codev-skillhub/src/index.ts | 2 + packages/codev-skillhub/src/skillhub.ts | 4 +- .../codev-skillhub/test/transport.test.ts | 66 +++++++++++++++++ packages/desktop/src/main/index.ts | 16 ++++- .../src/main/skillhub/controller.test.ts | 52 ++++++++++++++ .../desktop/src/main/skillhub/controller.ts | 71 ++++++++++++++++--- 14 files changed, 266 insertions(+), 21 deletions(-) create mode 100644 packages/codev-skillhub/test/transport.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f168171a..20a11a752 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -208,6 +208,22 @@ key_refused`; against an older backend the previous behaviour stands. the install dialog offers the browser sign-in right in the app. ([#331](https://github.com/quickbeard/codev-code/pull/331), [#335](https://github.com/quickbeard/codev-code/pull/335)) +- **Skill Hub works behind a corporate proxy** — browsing and installing from + the desktop app used to go straight to the network, ignoring the system + proxy and PAC settings that the app's own sign-in already honored, so on a + proxied network the Skill Hub page showed "Couldn't reach the Skill Hub" + while sign-in worked. Hub traffic, including the silent session refresh + behind it, now goes through the same proxy-aware network stack, and the app + waits for the proxy configuration to finish loading before the first hub + request instead of counting that wait against the request's own timeout. +- **Installing into a project on another machine is refused, not misplaced** + — when the app is connected to a WSL distro, a sandbox, or a remote server, + the install dialog no longer offers "This project": the app installs onto + the computer it runs on, so that folder is not here, and the previous + behavior created a stray folder on the wrong disk (or failed with an + unhelpful error). The dialog says which server you are connected to and + points at `codev skill pull` on that machine; the desktop side also checks + that a project folder really exists before it downloads anything. ### Language diff --git a/DIVERGENCES.md b/DIVERGENCES.md index 213b1be42..82f7b22e6 100644 --- a/DIVERGENCES.md +++ b/DIVERGENCES.md @@ -225,7 +225,7 @@ from the squash message first, or use a plain merge. | `packages/app/src/components/prompt-input-v2.tsx` | `onUnpaidClick` opens `DialogConnectProvider` (lazy import) instead of the free-models "unpaid" dialogs; their now-unused imports dropped. (The legacy `prompt-input.tsx` carried the same swap until its deletion — see the legacy prompt-input removal row.) | With the hosted providers hidden, "unpaid" means nothing is connected, and the free-models dialogs would render empty lists (this path is reachable on the web deployment; on desktop the gate covers it). | Keep the handler swap; take upstream's surrounding prompt-input changes. | | `packages/desktop/src/renderer/index.tsx` | Calls `installConsoleFormatting()` (fork-only `./console-format`) before anything else logs. | `spyRendererConsole` receives Chromium's flattened console-message string, so any renderer `console.*` with an object argument reached the terminal and the exported debug logs as `[object Object]`; the wrapper serializes structured arguments (Error-aware, circular-safe, truncated) at the source. | Keep the import and call; take upstream's surrounding entry changes. | | Desktop gateway SSO wiring: `packages/desktop/src/main/index.ts`, `packages/desktop/src/preload/{index,types}.ts`, `packages/desktop/src/renderer/index.tsx` | Main instantiates the fork-only `src/main/gateway-sso/` controller (wrapping `@codev/gateway` `login`/`silentSso` — node-only loopback OAuth — with `shell.openExternal` and a `net.fetch` wrapper) and registers its `gateway-sso-*` IPC channels next to the sandbox ones, passing an `onConfigured` callback that restarts the running sandboxes (`SandboxController.restartRunning`) because a guest is handed its codev config once at boot and cannot see credentials that appear afterwards; preload exposes the `gatewaySso` namespace (subscribe fan-out, sandbox pattern) with types re-exported from `@opencode-ai/app/gateway/types`; the renderer platform maps `gatewaySso: window.api.gatewaySso`. | SSO login for the desktop configure gate must run in the main process (`node:http` loopback server, `~/.codev/auth.json` writes); `net.fetch` so enterprise proxies/PAC apply (egress precedent). | Keep the wiring; take upstream's surrounding changes. Keep channel names `gateway-sso-*` stable. | -| Desktop Skill Hub wiring: `packages/desktop/src/main/index.ts`, `packages/desktop/src/preload/{index,types}.ts`, `packages/desktop/src/renderer/index.tsx`, `packages/desktop/package.json`, `packages/app/src/app.tsx`, `packages/app/src/i18n/{en,es}.ts` | Main registers the fork-only `src/main/skillhub/` controller (over the fork-only `@codev/skillhub` package) and its `skillhub-*` IPC channels; preload exposes the `skillhub` namespace (types aliased from `@opencode-ai/app/skillhub/types`); renderer maps `skillhub: window.api.skillhub` into the platform; `@codev/skillhub` added under desktop devDependencies (electron-vite bundles devDependency workspace packages); app.tsx registers lazy `/skills` + `/skills/:id` routes inside the new-layout ``; i18n adds `sidebar.skillhub` + the `skillhub.*` family (vi.ts is fork-only). | The Skill Hub page is native (fork-only `packages/app/src/{skillhub/**,pages/skill-hub*.tsx,components/skill-hub-nav.tsx}`); all hub HTTP must originate in the Electron main process because the hub serves no ACAO on /api/v1 and 403s cross-origin writes, so the renderer reaches it only over IPC. | Keep the registration/preload/renderer/package.json lines, the two route lines, and the i18n keys; take upstream's surrounding changes. | +| Desktop Skill Hub wiring: `packages/desktop/src/main/index.ts`, `packages/desktop/src/preload/{index,types}.ts`, `packages/desktop/src/renderer/index.tsx`, `packages/desktop/package.json`, `packages/app/src/app.tsx`, `packages/app/src/i18n/{en,es}.ts` | Main registers the fork-only `src/main/skillhub/` controller (over the fork-only `@codev/skillhub` package) and its `skillhub-*` IPC channels, passing a `net.fetch` wrapper as the hub transport (`setHubTransport`) and a `warmProxy` hook — `session.defaultSession.resolveProxy(SKILLHUB_URL)` — so hub traffic honors enterprise proxies/PAC like the gateway SSO calls do and Chromium's proxy initialization is waited out before the first hub request; preload exposes the `skillhub` namespace (types aliased from `@opencode-ai/app/skillhub/types`); renderer maps `skillhub: window.api.skillhub` into the platform; `@codev/skillhub` added under desktop devDependencies (electron-vite bundles devDependency workspace packages); app.tsx registers lazy `/skills` + `/skills/:id` routes inside the new-layout ``; i18n adds `sidebar.skillhub` + the `skillhub.*` family (vi.ts is fork-only). | The Skill Hub page is native (fork-only `packages/app/src/{skillhub/**,pages/skill-hub*.tsx,components/skill-hub-nav.tsx}`); all hub HTTP must originate in the Electron main process because the hub serves no ACAO on /api/v1 and 403s cross-origin writes, so the renderer reaches it only over IPC. | Keep the registration/preload/renderer/package.json lines, the two route lines, and the i18n keys; take upstream's surrounding changes. | | `packages/app/vite.js` | Added `optimizeDeps.entries`: the default `index.html` plus session-ui's `markdown.worker.ts` (absolute path derived from the plugin file, forward-slashed for fast-glob), so the dev-time dep scanner crawls the markdown worker at startup. | Vite's dep scanner does not crawl `?worker&url` entries, so the worker's deps (`@shikijs/stream`, `katex`, `marked`, `marked-shiki`, `remend`, `shiki`) were first discovered when the first markdown rendered; that mid-session re-optimize full-page reloads the dev app — e.g. a Skill Hub search reset itself the moment results rendered. A bare `optimizeDeps.include` list cannot express this fix: with bun's isolated linker the deps live only in the workspace packages' own `node_modules`, and no single include chain resolves from both consumers' roots (`packages/app` and the desktop renderer's `src/renderer`). Dev-only; production builds ignore `optimizeDeps`. | Keep the `optimizeDeps` block and keep `index.html` listed in it — setting `entries` replaces Vite's default entry inference, so dropping it unscans the app itself. If upstream moves or renames the worker file, update the path: a stale pattern matches nothing, logs nothing, and silently brings the reload back. | | `packages/app/package.json` | Added `"./gateway/types": "./src/gateway/types.ts"`, `"./skillhub/types": "./src/skillhub/types.ts"`, and `"./i18n/*": "./src/i18n/*.ts"` to `exports`. | The desktop preload aliases the app-owned gateway SSO wire types (sandbox/types pattern); the desktop main process imports the locale dicts to translate the macOS menu bar (`./i18n/*` mirrors how `@opencode-ai/ui` already exports its own — see the localized desktop menu row). | Keep both exports; take upstream's other manifest changes. | | `packages/app/src/i18n/{en,es}.ts` | Added the `gateway.configure.*` key family (configure-gate dialog) and reworded `sidebar.gettingStarted.line1` — the "includes free models" pitch no longer holds with the hosted providers hidden. | Strings for the fork's configure gate; the getting-started card must not advertise models the fork does not offer. | Keep our keys and copy; take upstream's surrounding i18n changes. | diff --git a/packages/app/src/i18n/en.ts b/packages/app/src/i18n/en.ts index fa2bac524..9bae07776 100644 --- a/packages/app/src/i18n/en.ts +++ b/packages/app/src/i18n/en.ts @@ -1080,6 +1080,8 @@ export const dict = { "skillhub.install.scope.label": "Install to", "skillhub.install.scope.project": "This project", "skillhub.install.scope.global": "Global (all projects)", + "skillhub.install.scope.remote": + "You are connected to {{server}}. The app installs skills on this computer, so a project install is unavailable and a global install will not be seen there. Run `codev skill pull` on that machine instead.", "skillhub.install.agents.label": "For which agents?", "skillhub.install.agents.locked": "Always included", "skillhub.install.overwrite.title": "Already installed", @@ -1093,6 +1095,7 @@ export const dict = { "skillhub.install.signin": "Sign in", "skillhub.install.signin.waiting": "Finish signing in in your browser…", "skillhub.install.error.failed": "The Skill Hub could not deliver this skill.", + "skillhub.install.error.missingDirectory": "That project folder is not on this computer.", "skillhub.install.success.title": "Installed {{name}}", "skillhub.install.done": "Done", "skillhub.desktopOnly.title": "Skill Hub is available in the desktop app", diff --git a/packages/app/src/i18n/es.ts b/packages/app/src/i18n/es.ts index 327d92b7e..a3e2e9a59 100644 --- a/packages/app/src/i18n/es.ts +++ b/packages/app/src/i18n/es.ts @@ -1074,6 +1074,8 @@ export const dict = { "skillhub.install.scope.label": "Instalar en", "skillhub.install.scope.project": "Este proyecto", "skillhub.install.scope.global": "Global (todos los proyectos)", + "skillhub.install.scope.remote": + "Estás conectado a {{server}}. La aplicación instala las habilidades en este equipo, así que la instalación en el proyecto no está disponible y una instalación global no se verá allí. Ejecuta `codev skill pull` en esa máquina.", "skillhub.install.agents.label": "¿Para qué agentes?", "skillhub.install.agents.locked": "Siempre incluido", "skillhub.install.overwrite.title": "Ya instalado", @@ -1087,6 +1089,7 @@ export const dict = { "skillhub.install.signin": "Iniciar sesión", "skillhub.install.signin.waiting": "Termina de iniciar sesión en tu navegador…", "skillhub.install.error.failed": "El Skill Hub no pudo entregar esta habilidad.", + "skillhub.install.error.missingDirectory": "Esa carpeta de proyecto no está en este equipo.", "skillhub.install.success.title": "{{name}} instalado", "skillhub.install.done": "Hecho", "skillhub.desktopOnly.title": "Skill Hub está disponible en la aplicación de escritorio", diff --git a/packages/app/src/i18n/vi.ts b/packages/app/src/i18n/vi.ts index 3cb534c62..9cfb40efc 100644 --- a/packages/app/src/i18n/vi.ts +++ b/packages/app/src/i18n/vi.ts @@ -1037,6 +1037,8 @@ export const dict = { "skillhub.install.scope.label": "Cài vào", "skillhub.install.scope.project": "Dự án này", "skillhub.install.scope.global": "Toàn cục (mọi dự án)", + "skillhub.install.scope.remote": + "Bạn đang kết nối tới {{server}}. Ứng dụng cài kỹ năng lên máy tính này, nên không thể cài vào dự án và cài toàn cục sẽ không hiện ở đó. Hãy chạy `codev skill pull` trên máy đó.", "skillhub.install.agents.label": "Cho những agent nào?", "skillhub.install.agents.locked": "Luôn được bao gồm", "skillhub.install.overwrite.title": "Đã được cài đặt", @@ -1050,6 +1052,7 @@ export const dict = { "skillhub.install.signin": "Đăng nhập", "skillhub.install.signin.waiting": "Hoàn tất đăng nhập trong trình duyệt…", "skillhub.install.error.failed": "Skill Hub không thể tải kỹ năng này về.", + "skillhub.install.error.missingDirectory": "Thư mục dự án đó không có trên máy tính này.", "skillhub.install.success.title": "Đã cài đặt {{name}}", "skillhub.install.done": "Xong", "skillhub.desktopOnly.title": "Skill Hub có sẵn trong ứng dụng desktop", diff --git a/packages/app/src/skillhub/dialog-install.tsx b/packages/app/src/skillhub/dialog-install.tsx index de405cd13..14eb27f36 100644 --- a/packages/app/src/skillhub/dialog-install.tsx +++ b/packages/app/src/skillhub/dialog-install.tsx @@ -11,6 +11,7 @@ import { createStore } from "solid-js/store" import { useLanguage } from "@/context/language" import { useLayout } from "@/context/layout" import { usePlatform } from "@/context/platform" +import { useServer } from "@/context/server" import { displayName } from "@/pages/layout/helpers" import { showToast } from "@/utils/toast" import type { SkillHubAgent, SkillHubInstallOutcome, SkillHubScope } from "./types" @@ -46,14 +47,20 @@ export function DialogSkillInstall(props: { id: string; name: string; version?: const language = useLanguage() const layout = useLayout() const platform = usePlatform() + const server = useServer() const projects = createMemo(() => layout.projects.list()) const preselected = createMemo( () => projects().find((project) => project.worktree === layout.home.selection().directory) ?? projects()[0], ) + // The main process installs onto this machine's disk, but the project list + // belongs to the connected server. A WSL distro, sandbox, or remote host has + // worktrees that are not here, so "This project" only makes sense against + // the local server; global still works but that server will not see it. + const local = createMemo(() => server.isLocal()) const [state, setState] = createStore({ - scope: (preselected() ? "project" : "global") as "project" | "global", + scope: (preselected() && local() ? "project" : "global") as "project" | "global", directory: preselected()?.worktree as string | undefined, agents: rememberedAgents(), phase: "form" as "form" | "installing" | "signing-in" | "overwrite" | "done", @@ -105,7 +112,12 @@ export function DialogSkillInstall(props: { id: string; name: string; version?: setState({ phase: "form", needsAuth: true, error: language.t("skillhub.install.error.auth") }) return } - const key = result.error.code === "network" ? "skillhub.error.network" : "skillhub.install.error.failed" + const key = + result.error.code === "network" + ? "skillhub.error.network" + : result.error.code === "missing-directory" + ? "skillhub.install.error.missingDirectory" + : "skillhub.install.error.failed" setState({ phase: "form", error: `${language.t(key)} (${result.error.message})` }) } @@ -154,10 +166,10 @@ export function DialogSkillInstall(props: { id: string; name: string; version?: > - 0}> + 0 && local()}> + +
+ {language.t("skillhub.install.scope.remote", { server: server.name })} +
+
diff --git a/packages/app/src/skillhub/types.ts b/packages/app/src/skillhub/types.ts index 21f62c3a8..1edbfc356 100644 --- a/packages/app/src/skillhub/types.ts +++ b/packages/app/src/skillhub/types.ts @@ -73,10 +73,11 @@ export type SkillHubInstallOutcome = { // Discriminated results instead of throws: ipcMain.handle rejections arrive as // mangled "Error invoking remote method" strings, while `already-installed` -// must stay machine-readable to drive the overwrite offer and `network` the -// hub-unreachable state. +// must stay machine-readable to drive the overwrite offer, `network` the +// hub-unreachable state, and `missing-directory` the case where the chosen +// project folder is not on the machine the app runs on. export type SkillHubError = { - code: "auth" | "not-found" | "already-installed" | "network" | "unknown" + code: "auth" | "not-found" | "already-installed" | "network" | "missing-directory" | "unknown" message: string } diff --git a/packages/codev-skillhub/src/http.ts b/packages/codev-skillhub/src/http.ts index c0b2c56ea..32f3b7f7a 100644 --- a/packages/codev-skillhub/src/http.ts +++ b/packages/codev-skillhub/src/http.ts @@ -8,12 +8,12 @@ import { applySystemCaCertsOnce, isCertError } from "./tls" // failed. async function hubFetch(endpoint: string, input: string | URL, init?: RequestInit): Promise { try { - return await fetch(input, init) + return await httpApi.transport(input, init) } catch (err) { if (!isCertError(err)) throw err const ca = applySystemCaCertsOnce() if (ca?.status !== "merged") throw err - return await fetch(input, init) + return await httpApi.transport(input, init) } } @@ -21,4 +21,17 @@ async function hubFetch(endpoint: string, input: string | URL, init?: RequestIni // ESM namespaces is unreliable). export const httpApi = { fetch: hubFetch, + // The socket-level transport behind every hub request, including the silent + // SSO refresh that authenticates them. Node's fetch ignores system proxies + // and PAC files, so a host with a proxy-aware stack swaps its own in (see + // setHubTransport); the CLI keeps the default. + transport: fetch as typeof fetch, +} + +// Route all hub traffic through `fetchImpl`. The desktop app's Electron main +// process passes Chromium's net.fetch so enterprise proxies, PAC scripts, and +// OS trust roots apply to Skill Hub browsing and installs the same way they +// already do to the gateway SSO calls. +export function setHubTransport(fetchImpl: typeof fetch) { + httpApi.transport = fetchImpl } diff --git a/packages/codev-skillhub/src/index.ts b/packages/codev-skillhub/src/index.ts index 277579be3..42aa3e36b 100644 --- a/packages/codev-skillhub/src/index.ts +++ b/packages/codev-skillhub/src/index.ts @@ -1,5 +1,7 @@ export { defaultAgents } from "./agents" export { pathExists } from "./archive" +export { SKILLHUB_URL } from "./const" +export { setHubTransport } from "./http" export { describeNetworkError } from "./tls" export { stripControlChars } from "./sanitize" export { AGENT_LABELS, ALWAYS_AGENT, SKILL_AGENTS, isSkillAgent, type Scope, type SkillAgent } from "./skill-dirs" diff --git a/packages/codev-skillhub/src/skillhub.ts b/packages/codev-skillhub/src/skillhub.ts index f2ed4afbf..bcad37ca6 100644 --- a/packages/codev-skillhub/src/skillhub.ts +++ b/packages/codev-skillhub/src/skillhub.ts @@ -29,7 +29,7 @@ async function skillhubAuthHeaders(optional: boolean): Promise { if (loadSkillhubCookie()) return true try { - return (await silentSso()) !== null + return (await silentSso({ fetchImpl: httpApi.transport })) !== null } catch { return false } diff --git a/packages/codev-skillhub/test/transport.test.ts b/packages/codev-skillhub/test/transport.test.ts new file mode 100644 index 000000000..1f0939e32 --- /dev/null +++ b/packages/codev-skillhub/test/transport.test.ts @@ -0,0 +1,66 @@ +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs" +import { tmpdir } from "node:os" +import { join } from "node:path" +import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test" +import { httpApi, setHubTransport } from "../src/http" +import { browseHubSkills } from "../src/hub" + +let tempDir: string +let savedHome: string | undefined +const savedTransport = httpApi.transport + +beforeEach(() => { + tempDir = mkdtempSync(join(tmpdir(), "codev-skillhub-transport-")) + savedHome = process.env.CODEV_TEST_HOME + process.env.CODEV_TEST_HOME = tempDir +}) + +afterEach(() => { + if (savedHome === undefined) delete process.env.CODEV_TEST_HOME + if (savedHome !== undefined) process.env.CODEV_TEST_HOME = savedHome + rmSync(tempDir, { recursive: true, force: true }) + setHubTransport(savedTransport) + mock.restore() +}) + +function json(body: unknown, status = 200): Response { + return new Response(JSON.stringify(body), { status, headers: { "Content-Type": "application/json" } }) +} + +describe("setHubTransport", () => { + test("hub requests go through the installed transport", async () => { + const transport = mock(async (_input: string | URL | Request, _init?: RequestInit) => + json({ success: true, data: [], pagination: { total: 0 } }), + ) + setHubTransport(transport as unknown as typeof fetch) + await browseHubSkills({ search: "pg" }) + expect(transport).toHaveBeenCalledTimes(1) + expect(String(transport.mock.calls[0]?.[0])).toContain("/api/v1/hub/skills?search=pg") + }) + + test("the silent SSO refresh behind a request uses the same transport", async () => { + // An expired access token with a refresh token forces silentSso to call + // the SSO token endpoint before the hub request. + mkdirSync(join(tempDir, ".codev"), { recursive: true }) + writeFileSync( + join(tempDir, ".codev", "auth.json"), + JSON.stringify({ access_token: "old", refresh_token: "r1", expires_at: 1 }), + ) + const transport = mock(async (input: string | URL | Request, _init?: RequestInit) => { + if (String(input).endsWith("/token")) { + return json({ access_token: "fresh", id_token: "id", refresh_token: "r2", expires_in: 3600 }) + } + if (String(input).endsWith("/userinfo")) { + return json({ sub: "u1", email: "u1@example.com" }) + } + return json({ success: true, data: [], pagination: { total: 0 } }) + }) + setHubTransport(transport as unknown as typeof fetch) + await browseHubSkills() + const urls = transport.mock.calls.map((call) => String(call[0])) + expect(urls.some((url) => url.endsWith("/token"))).toBe(true) + const hub = transport.mock.calls.find((call) => String(call[0]).includes("/api/v1/hub/skills")) + const headers = (hub?.[1] as RequestInit | undefined)?.headers as Record + expect(headers.Authorization).toBe("Bearer fresh") + }) +}) diff --git a/packages/desktop/src/main/index.ts b/packages/desktop/src/main/index.ts index c4ad5267f..8bae9ddde 100644 --- a/packages/desktop/src/main/index.ts +++ b/packages/desktop/src/main/index.ts @@ -7,6 +7,7 @@ import { join } from "node:path" import { getCACertificates, setDefaultCACertificates } from "node:tls" import type { Event } from "electron" import { app, BrowserWindow, net, Notification, session, shell } from "electron" +import { SKILLHUB_URL } from "@codev/skillhub" import { shouldNudgeSignIn } from "@codev/upload/nudge" import type { ServerSource as UploadServerSource } from "@codev/upload" @@ -417,7 +418,20 @@ const main = Effect.gen(function* () { }) registerWslIpcHandlers(wslServers) registerSandboxIpcHandlers(sandboxes) - registerSkillHubIpcHandlers(createSkillHubController()) + registerSkillHubIpcHandlers( + createSkillHubController({ + // Chromium's fetch so host proxies, PAC files, and enterprise roots + // apply to hub traffic exactly as they do to the gateway SSO calls below. + fetchImpl: (input, init) => + net.fetch(input instanceof URL ? input.toString() : input, init as Parameters[1]), + // Settles once Chromium's proxy initialization is done; logged so a + // report from a proxied network says which proxy the hub went through. + warmProxy: () => + session.defaultSession.resolveProxy(SKILLHUB_URL).then((proxy) => { + logger.log("skillhub proxy resolved", { proxy }) + }), + }), + ) registerGatewaySsoIpcHandlers( createGatewaySsoController({ openUrl: (url) => shell.openExternal(url), diff --git a/packages/desktop/src/main/skillhub/controller.test.ts b/packages/desktop/src/main/skillhub/controller.test.ts index 5d3167c58..19316d4c3 100644 --- a/packages/desktop/src/main/skillhub/controller.test.ts +++ b/packages/desktop/src/main/skillhub/controller.test.ts @@ -60,6 +60,7 @@ describe("skillhub controller", () => { }) test("project scope becomes an agents-at target rooted at the given directory", async () => { + spyOn(skillhubApi, "pathExists").mockResolvedValue(true) spyOn(skillhubApi, "getSkillMeta").mockResolvedValue(META) const install = spyOn(skillhubApi, "installResolvedSkill").mockResolvedValue({ name: "pg-tuner", @@ -102,7 +103,58 @@ describe("skillhub controller", () => { expect((await fail(new Error("boom"))).code).toBe("unknown") }) + test("a project folder that is not on this machine is refused before any hub traffic", async () => { + spyOn(skillhubApi, "pathExists").mockResolvedValue(false) + const meta = spyOn(skillhubApi, "getSkillMeta").mockResolvedValue(META) + const result = await controller.install({ + id: "id-1", + scope: { kind: "project", directory: "/home/wsl-user/proj" }, + agents: [], + force: false, + }) + if (result.ok) throw new Error("expected error") + expect(result.error.code).toBe("missing-directory") + expect(result.error.message).toContain("/home/wsl-user/proj") + expect(meta).not.toHaveBeenCalled() + }) + + test("Chromium network codes classify as network errors", async () => { + spyOn(skillhubApi, "getSkillDetail").mockRejectedValue(new Error("net::ERR_PROXY_CONNECTION_FAILED")) + const result = await controller.detail("x") + if (result.ok) throw new Error("expected error") + expect(result.error.code).toBe("network") + }) + + test("requests wait for the proxy warm-up once, then never again", async () => { + let release = () => {} + const warm = mock(() => new Promise((resolve) => (release = resolve))) + const browse = spyOn(skillhubApi, "browseHubSkills").mockResolvedValue({ + items: [], + pagination: { page: 1, limit: 24, total: 0, totalPages: 1 }, + }) + const warmed = createSkillHubController({ warmProxy: warm }) + const first = warmed.browse({}) + const second = warmed.browse({}) + await Promise.resolve() + expect(browse).not.toHaveBeenCalled() + release() + expect((await first).ok).toBe(true) + expect((await second).ok).toBe(true) + await warmed.detail("x").catch(() => {}) + expect(warm).toHaveBeenCalledTimes(1) + }) + + test("a failed proxy warm-up does not block requests", async () => { + spyOn(skillhubApi, "browseHubSkills").mockResolvedValue({ + items: [], + pagination: { page: 1, limit: 24, total: 0, totalPages: 1 }, + }) + const warmed = createSkillHubController({ warmProxy: () => Promise.reject(new Error("no proxy service")) }) + expect((await warmed.browse({})).ok).toBe(true) + }) + test("already-installed keeps the path in the message for the overwrite offer", async () => { + spyOn(skillhubApi, "pathExists").mockResolvedValue(true) spyOn(skillhubApi, "getSkillMeta").mockResolvedValue(META) spyOn(skillhubApi, "installResolvedSkill").mockRejectedValue( new Error("Already installed at /proj/.claude/skills/pg-tuner. Pass --force to overwrite."), diff --git a/packages/desktop/src/main/skillhub/controller.ts b/packages/desktop/src/main/skillhub/controller.ts index c5d98558d..5ffa17e7f 100644 --- a/packages/desktop/src/main/skillhub/controller.ts +++ b/packages/desktop/src/main/skillhub/controller.ts @@ -4,6 +4,8 @@ import { getSkillDetail, getSkillMeta, installResolvedSkill, + pathExists, + setHubTransport, SKILL_AGENTS, SkillhubAuthError, type InstallTarget, @@ -28,21 +30,51 @@ import type { // Auth is the shared @codev/gateway session file — the same one the CLIs use — // picked up inside @codev/skillhub; browsing and installs work anonymously. -// Stubbable seam so controller tests never touch the network (the package's -// own idiom — see codev-skillhub skill-install.ts). +// Stubbable seam so controller tests never touch the network or disk (the +// package's own idiom — see codev-skillhub skill-install.ts). export const skillhubApi = { browseHubSkills, getSkillDetail, getSkillMeta, installResolvedSkill, + pathExists, } export type SkillHubController = SkillHubPlatform -export function createSkillHubController(): SkillHubController { +// Chromium holds every request — the main process's net.fetch included — until +// its proxy initialization (PAC download / WPAD discovery) is done, which on +// corporate networks outlasts the hub's 15 s request timeout. The warm-up is +// awaited once, before the first request, so that wait is never charged to a +// request's own budget; past this bound the request goes ahead regardless. +const PROXY_WARM_TIMEOUT_MS = 60_000 + +export type SkillHubControllerDeps = { + // Transport for every hub request (and the silent SSO refresh behind it). + // The desktop passes Electron's net.fetch so enterprise proxies and PAC + // files apply; omitted, the package keeps Node's fetch. + fetchImpl?: typeof fetch + // Settles once the network stack behind fetchImpl is ready to send — the + // desktop passes session.resolveProxy. A failed warm-up never blocks. + warmProxy?: () => Promise +} + +export function createSkillHubController(deps: SkillHubControllerDeps = {}): SkillHubController { + if (deps.fetchImpl) setHubTransport(deps.fetchImpl) + + let warm: Promise | undefined + const ready = () => { + if (!deps.warmProxy) return Promise.resolve() + warm ??= Promise.race([ + deps.warmProxy().then(() => undefined), + new Promise((resolve) => setTimeout(resolve, PROXY_WARM_TIMEOUT_MS).unref()), + ]).catch(() => undefined) + return warm + } + return { browse: (params) => - run(async () => { + run(ready, async () => { const result = await skillhubApi.browseHubSkills({ search: params.search, page: params.page, @@ -56,7 +88,7 @@ export function createSkillHubController(): SkillHubController { } }), detail: (idOrName) => - run(async () => { + run(ready, async () => { const detail = await skillhubApi.getSkillDetail(idOrName) const card = toCard(detail) const full: SkillHubDetail = { @@ -70,7 +102,15 @@ export function createSkillHubController(): SkillHubController { return full }), install: (request) => - run(async () => { + run(ready, async () => { + // The install writes to this machine's disk, but the project list the + // dialog offers belongs to whichever server the window is connected + // to — a WSL distro or a remote host has worktrees that do not exist + // here (or resolve to an unrelated local path). Refuse before any hub + // traffic rather than create a stray folder. + if (request.scope.kind === "project" && !(await skillhubApi.pathExists(request.scope.directory))) { + throw new MissingDirectoryError(request.scope.directory) + } // CoDev Code is folded in here, never trusted from the renderer — the // dialog renders it locked, but the lock is enforced on this side. const agents = lockAgents(request.agents) @@ -94,8 +134,16 @@ export function createSkillHubController(): SkillHubController { } } -async function run(work: () => Promise): Promise> { +class MissingDirectoryError extends Error { + constructor(directory: string) { + super(`Project folder not found on this computer: ${directory}`) + this.name = "MissingDirectoryError" + } +} + +async function run(ready: () => Promise, work: () => Promise): Promise> { try { + await ready() return { ok: true, value: await work() } } catch (err) { return { ok: false, error: toError(err) } @@ -107,12 +155,19 @@ async function run(work: () => Promise): Promise> { // "Error invoking remote method" strings. function toError(err: unknown): SkillHubError { if (err instanceof SkillhubAuthError) return { code: "auth", message: err.message } + if (err instanceof MissingDirectoryError) return { code: "missing-directory", message: err.message } const message = err instanceof Error ? err.message : String(err) if (message.startsWith("Already installed at ")) return { code: "already-installed", message } if (message.includes("not found or not public")) return { code: "not-found", message } if (err instanceof Error) { + // Node's fetch reports "fetch failed" with the reason on `cause`; Electron's + // net.fetch reports Chromium's own codes ("net::ERR_PROXY_CONNECTION_FAILED"). const network = - err.name === "AbortError" || err.name === "TimeoutError" || message === "fetch failed" || err.cause !== undefined + err.name === "AbortError" || + err.name === "TimeoutError" || + message === "fetch failed" || + message.includes("net::ERR_") || + err.cause !== undefined if (network) return { code: "network", message: describeNetworkError(err) } } return { code: "unknown", message }