fix(skills): surface skill market install failures instead of disabling install

apps/desktop/electron/main/public-https-fetch.ts

@@ -1,5 +1,11 @@
 import { lookup as dnsLookup } from "node:dns/promises";
-import { isPublicIpLiteral, isSafePublicHttpsUrl } from "@pi-desktop/shared";
+import {
+  ErrorCodes,
+  PUBLIC_NETWORK_POLICY_ERROR,
+  isPublicIpLiteral,
+  isPublicNetworkPolicyFailure,
+  isSafePublicHttpsUrl,
+} from "@pi-desktop/shared";
 
 const DEFAULT_TIMEOUT_MS = 8_000;
 const MAX_HOPS = 5;
@@ -14,14 +20,19 @@ export type PublicHttpsLookup = (host: string) => Promise<Array<{ address: strin
 
 export class PublicNetworkPolicyError extends Error {
   readonly code = "PUBLIC_NETWORK_POLICY";
+  /**
+   * Stable code the IPC wrapper forwards to the renderer, so a policy refusal
+   * can be told apart from an ordinary network failure (spec 08 §3.1).
+   */
+  readonly errorCode = ErrorCodes.NETWORK_POLICY_BLOCKED;
   constructor(message: string) {
     super(message);
-    this.name = "PublicNetworkPolicyError";
+    this.name = PUBLIC_NETWORK_POLICY_ERROR;
   }
 }
 
 export function isPublicNetworkPolicyError(error: unknown): boolean {
-  return error instanceof PublicNetworkPolicyError || (error instanceof Error && error.name === "PublicNetworkPolicyError");
+  return error instanceof PublicNetworkPolicyError || isPublicNetworkPolicyFailure(error);
 }
 
 export type PublicHttpsClient = {

apps/desktop/electron/main/skill-market-scan.ts

@@ -5,6 +5,7 @@
  * (the public-HTTPS client). One failing source only costs itself.
  */
 import {
+  isPublicNetworkPolicyFailure,
   isSafeSkillSourceUrl,
   sanitizeSkillCatalogId,
   splitSkillDocument,
@@ -17,9 +18,21 @@ import {
 
 export type CatalogRequest = (url: string, kind: "json" | "text") => Promise<unknown>;
 
+/** Why a source failed: the public-network guard refused it, or it errored. */
+export type SkillMarketFailureKind = "policy" | "network";
+
 export type SkillMarketSearchResult = {
   entries: SourcedSkillEntry[];
   failedSources: string[];
+  /**
+   * `failedSources` alone cannot tell the user why the market went quiet. Keyed
+   * by the same display name so the panel can explain a policy refusal (the
+   * local DNS lookup could not classify the host, which is what a proxy that
+   * answers DNS itself produces) apart from a source that is merely
+   * unreachable. Repeated names collapse, exactly as they already do in
+   * `failedSources`.
+   */
+  failureKinds: Record<string, SkillMarketFailureKind>;
 };
 
 export type SkillMarketDocument = {
@@ -34,6 +47,12 @@ const GITHUB_REPO = /^https:\/\/github\.com\/([\w.-]+)\/([\w.-]+?)(?:\.git)?(?:[
 const JSDELIVR_GH = /^https:\/\/cdn\.jsdelivr\.net\/gh\/([^/]+)\/([^/]+)@([^/]+)\/(.+)$/;
 const SKILL_FILE = /(?:^|\/)SKILL\.md$/;
 
+function classifyFailure(error: unknown): SkillMarketFailureKind {
+  // Structural check so this module keeps no dependency on the client's
+  // `node:dns` import and stays testable as a pure module.
+  return isPublicNetworkPolicyFailure(error) ? "policy" : "network";
+}
+
 const SKILL_CATEGORY_KEYWORDS: ReadonlyArray<readonly [SkillCatalogCategory, string[]]> = [
   ["data", ["data", "sql", "database", "postgres", "mongo", "redis", "analytics", "dataset", "spreadsheet", "excel", "xlsx", "csv", "dashboard", "chart", "visualization"]],
   ["workflow", ["workflow", "review", "planning", "brainstorm", "checklist", "process", "sop", "handoff", "standup", "retro", "discernment", "verification", "triage", "audit", "security", "incident"]],
@@ -152,9 +171,20 @@ export function createSkillMarketAggregator(request: CatalogRequest) {
   async function search(query: string, sources: SkillMarketSource[]): Promise<SkillMarketSearchResult> {
     const trimmed = query.trim().toLocaleLowerCase();
     const usable = sources.filter((source) => isSafeSkillSourceUrl(source.url));
-    const failedSources = sources
-      .filter((source) => !isSafeSkillSourceUrl(source.url))
-      .map((source) => source.name);
+    // A source the syntactic guard never let out is a policy refusal, not a
+    // transport failure, and it must not be reported as an unreachable host.
+    const refused = sources.filter((source) => !isSafeSkillSourceUrl(source.url));
+    const failedSources = refused.map((source) => source.name);
+    // Two sources can carry the same display name (a default source and a user
+    // source with the same label), and `failedSources` already cannot tell such
+    // a pair apart. The refusal is the one worth surfacing, so `policy` wins
+    // the merge. A `Map` plus `Object.fromEntries` also keeps a source called
+    // `__proto__` from disappearing into the prototype.
+    const failures = new Map<string, SkillMarketFailureKind>();
+    const markFailure = (name: string, kind: SkillMarketFailureKind) => {
+      failures.set(name, failures.get(name) === "policy" || kind === "policy" ? "policy" : "network");
+    };
+    for (const source of refused) markFailure(source.name, "policy");
     const settled = await Promise.allSettled(
       usable.map(async (source) => {
         const entries = await loadSource(source);
@@ -170,7 +200,9 @@ export function createSkillMarketAggregator(request: CatalogRequest) {
     const seen = new Set<string>();
     settled.forEach((result, index) => {
       if (result.status === "rejected") {
-        failedSources.push(usable[index].name);
+        const name = usable[index].name;
+        failedSources.push(name);
+        markFailure(name, classifyFailure(result.reason));
         return;
       }
       for (const entry of result.value) {
@@ -179,7 +211,7 @@ export function createSkillMarketAggregator(request: CatalogRequest) {
         entries.push(entry);
       }
     });
-    return { entries, failedSources };
+    return { entries, failedSources, failureKinds: Object.fromEntries(failures) };
   }
 
   async function fetchEntryDocument(entry: SkillCatalogEntry): Promise<SkillMarketDocument> {

apps/desktop/src/components/settings/SkillMarketPanel.tsx

@@ -27,6 +27,12 @@ import {
 } from "../icons";
 import { Field, Input, TooltipButton, cx } from "../ui";
 import { LatestWinsGate } from "../../lib/latest-wins";
+import {
+  classifySkillMarketFailure,
+  hasPolicyFailure,
+  skillMarketFailureDetail,
+  type SkillMarketFailureKind,
+} from "../../lib/skill-market-failure";
 
 const CATEGORIES: readonly SkillCatalogCategory[] = [
   "workflow",
@@ -61,9 +67,22 @@ type RemoteState = {
   status: "idle" | "loading" | "ready" | "error";
   entries: MarketItem[];
   failed: string[];
+  /** Why each named source failed, so a policy/DNS refusal can be explained. */
+  failureKinds: Record<string, SkillMarketFailureKind>;
+  /**
+   * Query-level failure reason (bridge or preload unavailable). Such a rejection
+   * carries no per-source detail, and it used to be discarded with no trace.
+   */
+  queryError: string;
 };
 
-const REMOTE_IDLE: RemoteState = { status: "idle", entries: [], failed: [] };
+const REMOTE_IDLE: RemoteState = {
+  status: "idle",
+  entries: [],
+  failed: [],
+  failureKinds: {},
+  queryError: "",
+};
 
 const SOURCES_STORAGE_KEY = "pi.skill-market.sources.v1";
 
@@ -121,6 +140,10 @@ export function SkillMarketPanel({
   const [installFor, setInstallFor] = useState<MarketItem | null>(null);
   const [documentBody, setDocumentBody] = useState<string | null>(null);
   const [documentTooLarge, setDocumentTooLarge] = useState(false);
+  const [previewFailure, setPreviewFailure] = useState<{
+    kind: SkillMarketFailureKind;
+    detail: string;
+  } | null>(null);
   const [installing, setInstalling] = useState(false);
   const previewGate = useRef(new LatestWinsGate());
   const [sources, setSources] = useState<SkillMarketSource[]>(loadSources);
@@ -148,7 +171,13 @@ export function SkillMarketPanel({
     const timer = setTimeout(() => {
       setRemote((current) =>
         current.status === "idle" || current.status === "ready"
-          ? { status: "loading", entries: current.entries, failed: current.failed }
+          ? {
+              status: "loading",
+              entries: current.entries,
+              failed: current.failed,
+              failureKinds: current.failureKinds,
+              queryError: current.queryError,
+            }
           : current,
       );
       api
@@ -161,11 +190,21 @@ export function SkillMarketPanel({
               status: entries.length === 0 && failed.length > 0 ? "error" : "ready",
               entries,
               failed,
+              failureKinds: result.failureKinds ?? {},
+              queryError: "",
             });
           }
         })
-        .catch(() => {
-          if (!cancelled) setRemote({ status: "error", entries: [], failed: [] });
+        .catch((error: unknown) => {
+          if (!cancelled) {
+            setRemote({
+              status: "error",
+              entries: [],
+              failed: [],
+              failureKinds: {},
+              queryError: skillMarketFailureDetail(error),
+            });
+          }
         });
     }, 350);
     return () => {
@@ -201,11 +240,11 @@ export function SkillMarketPanel({
     [visible, currentPage],
   );
 
-  const openInstall = (entry: MarketItem) => {
+  const loadDocument = (entry: MarketItem) => {
     const token = previewGate.current.begin();
-    setInstallFor(entry);
     setDocumentBody(null);
     setDocumentTooLarge(false);
+    setPreviewFailure(null);
     api
       .fetchSkillMarketDocument(entry)
       .then((document) => {
@@ -214,14 +253,27 @@ export function SkillMarketPanel({
         setDocumentBody(assembled.body);
         setDocumentTooLarge(assembled.tooLarge);
       })
-      .catch(() => {
-        if (previewGate.current.isCurrent(token)) {
-          setDocumentBody(null);
-          setDocumentTooLarge(false);
-        }
+      .catch((error: unknown) => {
+        if (!previewGate.current.isCurrent(token)) return;
+        // Swallowing this left the sheet on a null body, so the install button
+        // sat disabled behind the word "Loading…" with no reason and no way to
+        // try again — the dead end issue #419 reports.
+        setPreviewFailure({
+          kind: classifySkillMarketFailure(error),
+          detail: skillMarketFailureDetail(error),
+        });
       });
   };
 
+  const openInstall = (entry: MarketItem) => {
+    setInstallFor(entry);
+    loadDocument(entry);
+  };
+
+  const retryPreview = () => {
+    if (installFor) loadDocument(installFor);
+  };
+
   const install = async () => {
     if (!installFor || installing) return;
     setInstalling(true);
@@ -272,6 +324,14 @@ export function SkillMarketPanel({
     setDraftSource({ name: "", url: "" });
   };
 
+  // A bare `remoteError` could not tell a policy refusal from a dead host, so
+  // the two now carry different copy and the policy case gets the proxy hint.
+  const remoteErrorText = () => {
+    if (remote.queryError) return t("settings.sklm.remoteErrorQuery");
+    if (hasPolicyFailure(remote.failureKinds)) return t("settings.sklm.remoteErrorPolicy");
+    return t("settings.sklm.remoteError");
+  };
+
   const sourcesSheet = sourcesOpen ? (
     <div
       className="overlay ext-sheet-overlay sklm-overlay"
@@ -418,7 +478,36 @@ export function SkillMarketPanel({
 
           <div className="ext-field-group">
             <div className="ext-field-label">{t("settings.sklm.preview")}</div>
-            <pre className="sklm-preview">{documentBody ?? t("common.loading")}</pre>
+            {previewFailure ? (
+              <p className="sklm-note is-error" role="alert">
+                {t(
+                  previewFailure.kind === "policy"
+                    ? "settings.sklm.previewPolicyError"
+                    : "settings.sklm.previewError",
+                )}
+              </p>
+            ) : (
+              <pre className="sklm-preview">{documentBody ?? t("common.loading")}</pre>
+            )}
+            {previewFailure?.kind === "policy" ? (
+              <p className="sklm-note">{t("settings.sklm.proxyHint")}</p>
+            ) : null}
+            {previewFailure?.detail ? (
+              <p className="sklm-note">
+                <span className="sklm-note-label">{t("settings.sklm.failureDetail")}</span>
+                {previewFailure.detail}
+              </p>
+            ) : null}
+            {previewFailure ? (
+              <button
+                type="button"
+                className="sklm-install is-ghost"
+                onClick={retryPreview}
+                disabled={installing}
+              >
+                {t("settings.sklm.retryPreview")}
+              </button>
+            ) : null}
             {documentTooLarge ? <p className="sklm-note">{t("settings.sklm.documentTooLarge")}</p> : null}
           </div>
 
@@ -492,10 +581,31 @@ export function SkillMarketPanel({
         </p>
       ) : null}
       {remote.status === "error" ? (
-        <p className="sklm-status is-error" role="status">
-          {t("settings.sklm.remoteError")}
-          {remote.failed.length ? ` (${remote.failed.join(", ")})` : ""}
-        </p>
+        <>
+          <p className="sklm-status is-error" role="status">
+            {remoteErrorText()}
+            {remote.failed.length ? ` (${remote.failed.join(", ")})` : ""}
+          </p>
+          {remote.queryError ? (
+            <p className="sklm-status">
+              <span className="sklm-note-label">{t("settings.sklm.failureDetail")}</span>
+              {remote.queryError}
+            </p>
+          ) : null}
+          {hasPolicyFailure(remote.failureKinds) ? (
+            <p className="sklm-status">{t("settings.sklm.proxyHint")}</p>
+          ) : null}
+        </>
+      ) : null}
+      {remote.status === "ready" && remote.failed.length ? (
+        <>
+          <p className="sklm-status">
+            {t("settings.sklm.remotePartial", { names: remote.failed.join(", ") })}
+          </p>
+          {hasPolicyFailure(remote.failureKinds) ? (
+            <p className="sklm-status">{t("settings.sklm.proxyHint")}</p>
+          ) : null}
+        </>
       ) : null}
 
       <div className="sklm-cats" role="tablist" aria-label={t("settings.sklm.title")}>

apps/desktop/src/lib/api.ts

@@ -719,10 +719,15 @@ export const api = {
 
   // --- Skill market ----------------------------------------------------------
   searchSkillMarket: (query: string, sources: { id: string; name: string; url: string }[]) =>
-    invoke<{ entries: SkillCatalogEntry[]; failedSources?: string[] }>(
-      IPC.invoke.skillMarketSearch,
-      { query, sources },
-    ),
+    invoke<{
+      entries: SkillCatalogEntry[];
+      failedSources?: string[];
+      /**
+       * Why each named source failed, so the market can explain a policy/DNS
+       * refusal instead of reporting every source as merely unreachable.
+       */
+      failureKinds?: Record<string, "policy" | "network">;
+    }>(IPC.invoke.skillMarketSearch, { query, sources }),
   /** Fetch one catalog document (frontmatter split off) for preview/install. */
   fetchSkillMarketDocument: (entry: SkillCatalogEntry) =>
     invoke<{ name?: string; description?: string; body: string; resources?: Array<{ path: string; body: string }> }>(