Fix #1299: Picker platforms values are not checked against the known process.platform set: a manifest typo silently withholds the row everywhere

llp/0368-picker-rows-declare-their-platforms.decision.md

@@ -7,6 +7,7 @@
 **Date:** 2026-09-03
 **Related:** LLP 0130 (#picker-block: the declarative picker row this extends), LLP 0011 (#autodetect-vs-default: a probe pre-checks, it never forces and never hides), LLP 0202 (#hidden-rows: display filtering is never a catalog deletion), LLP 0139 (#macos-only: the Desktop commands already refuse off macOS), LLP 0297 (#problem: the first reading of this gap), LLP 0358 (the decision that made the Desktop row visible again), hyparam/hypaware#1283
 **Extends:** LLP 0130
+**Extended-by:** LLP 0369 (a `platforms` value outside the known `process.platform` set warns at load rather than failing the plugin)
 
 > LLP 0130 settled that a picker row is declarative manifest data and that
 > `detect` seeds its checkbox. LLP 0011 settled that detection may only

llp/0369-unrecognized-picker-platforms-warn.decision.md

@@ -0,0 +1,70 @@
+# LLP 0369: An unrecognized picker platform warns rather than fails the plugin
+
+**Type:** Decision
+**Status:** Accepted
+**Systems:** Plugins, Onboarding
+**Author:** Phil / Claude
+**Date:** 2026-09-03
+**Related:** LLP 0368 (#platform-gate: the gate this reports on), LLP 0130 (#picker-block: the declarative picker row), LLP 0329 (#stderr-mirror: the channel a diagnostic reaches an unconfigured install by), hyparam/hypaware#1299
+**Extends:** LLP 0368
+
+> LLP 0368 settled that `validatePickerContributions` accepts any non-empty
+> array of non-empty strings as a row's `platforms` gate. That leaves a typo
+> (`"macos"`, `"Darwin"`, `"win"`) valid, and a valid gate that matches no
+> platform withholds its row everywhere in silence. This adds the missing
+> diagnostic without turning the typo into a rejection.
+
+## The problem {#problem}
+
+`visiblePickerDescriptors` compares a row's `platforms` against
+`process.platform` by string equality. Nothing anywhere compares those values
+against the set `process.platform` can actually report, so `["macos"]`
+validates, loads, and then matches nothing. The row is offered on no platform
+at all, which is worse than the mistake it came from and looks exactly like a
+row the author forgot to write. The author's own machine gives them no signal,
+because the failure is the absence of a row.
+
+## Warn, do not reject {#warn-not-reject}
+
+A manifest validation failure is fatal to the whole plugin: `loadManifest`
+returns a `FailedManifest` and every contribution the plugin makes (sources,
+sinks, datasets, commands) goes with it. Rejecting an unrecognized `platforms`
+value would therefore trade one withheld picker row for a dead plugin, which
+is a strictly larger failure than the one being fixed, and it would arm on the
+one input nobody can enumerate in advance: a `process.platform` value Node
+adds after this release.
+
+So the shape LLP 0368 `#consequences` settled is kept exactly as written, and
+the report is added beside it. `loadManifest` emits one WARN per offending
+row, `manifest.picker_platform_unrecognized`, naming the manifest path, the
+plugin, the row, and the unrecognized values. It mirrors to stderr (LLP 0329
+`#stderr-mirror`), because the reader is a plugin author on a default install
+with no telemetry provider, where an unmirrored WARN is constructed and
+dropped.
+
+The warning fires at load, not at picker display: a gated row is withheld
+before it reaches any menu, so the display path is the one place the mistake
+cannot be observed from.
+
+## The known set is the diagnostic's, not the gate's {#known-set}
+
+`KNOWN_PLATFORMS` holds the values `process.platform` can report (`aix`,
+`android`, `cygwin`, `darwin`, `freebsd`, `haiku`, `linux`, `netbsd`,
+`openbsd`, `sunos`, `win32`): the `NodeJS.Platform` union, not the shorter
+list the prose docs give, because `netbsd`, `cygwin`, and `haiku` are real
+today and warning on a gate that is correct is the one cost this diagnostic
+must not pay.
+It is deliberately not consulted by the gate itself, only by the report, which
+is what makes it safe to be wrong: a platform Node adds later costs one
+spurious warning line on a manifest that is in fact correct, and the row still
+renders where it should. Had the same list gated validation, being out of date
+would have cost the author their plugin.
+
+## Consequences {#consequences}
+
+- `loadManifest` warns per picker row carrying a `platforms` value outside the
+  known set; `validateManifest` is unchanged and still accepts it.
+- The warning mirrors to stderr, so it is visible on an install that
+  configured no telemetry.
+- No bundled manifest emits it: `@hypaware/claude-desktop`'s `["darwin"]` is
+  the only gate that ships, and a test pins that the bundled set stays quiet.

src/core/manifest.js

@@ -25,8 +25,10 @@ const MANIFEST_BASENAME = 'hypaware.plugin.json'
  */
 export async function loadManifest(rootDir) {
   const manifestPath = path.join(rootDir, MANIFEST_BASENAME)
+  /** @type {PluginManifest} */
+  let manifest
   try {
-    const manifest = await withSpan(
+    manifest = await withSpan(
       'manifest.load',
       {
         [Attr.OPERATION]: 'manifest.load',
@@ -62,7 +64,6 @@ export async function loadManifest(rootDir) {
       },
       { component: 'manifest' }
     )
-    return { ok: true, manifest, manifestPath, rootDir }
   } catch (err) {
     const errorKind = /** @type {ManifestErrorKind} */ (
       (err && /** @type {{hypErrorKind?: string}} */ (err).hypErrorKind) || 'manifest_invalid'
@@ -75,6 +76,65 @@ export async function loadManifest(rootDir) {
     })
     return { ok: false, errorKind, message, manifestPath, rootDir }
   }
+  // Outside the try above on purpose, and inside one of its own. Inside that
+  // try a throw from the diagnostic is caught as a manifest rejection and
+  // takes the whole plugin with it; outside it with no guard at all, the same
+  // throw rejects a promise this function has never rejected, and
+  // `loadManifests` fans out over `Promise.all`, so it would fail every
+  // plugin rather than one. A diagnostic may cost neither.
+  try {
+    warnUnrecognizedPickerPlatforms(manifest, manifestPath)
+  } catch {}
+  return { ok: true, manifest, manifestPath, rootDir }
+}
+
+/**
+ * The values `process.platform` can report (the `NodeJS.Platform` union, which
+ * is wider than the list the prose docs give). A picker row's `platforms` gate
+ * is matched against `process.platform` by string equality, so a value outside
+ * this set names no running platform.
+ *
+ * @ref LLP 0369#known-set: this list is the diagnostic's vocabulary, not the
+ * gate's, so a value Node adds later costs a spurious warning and nothing more.
+ */
+const KNOWN_PLATFORMS = new Set([
+  'aix', 'android', 'cygwin', 'darwin', 'freebsd', 'haiku',
+  'linux', 'netbsd', 'openbsd', 'sunos', 'win32',
+])
+
+/**
+ * Report picker `platforms` entries that name no known platform.
+ *
+ * Validation accepts them, because a rejection is fatal to the whole plugin
+ * and one mistyped display gate is not worth that. Unreported, the typo is
+ * invisible: the row is offered on no platform, and an absent row is what a
+ * row the author never wrote looks like too. The warning mirrors to stderr
+ * because the reader is a plugin author with no telemetry configured.
+ *
+ * @ref LLP 0369#warn-not-reject [implements]: an unrecognized platform is a load-time warning, never a refusal.
+ * @param {PluginManifest} manifest
+ * @param {string} manifestPath
+ */
+function warnUnrecognizedPickerPlatforms(manifest, manifestPath) {
+  for (const row of manifest.contributes?.picker ?? []) {
+    const platforms = row.platforms ?? []
+    const unrecognized = platforms.filter((p) => !KNOWN_PLATFORMS.has(p))
+    if (unrecognized.length === 0) continue
+    // Only a gate that is unrecognized end to end withholds the row
+    // everywhere. `["darwin", "win"]` carries the same typo but still renders
+    // on macOS, and telling that author the row is offered nowhere sends them
+    // hunting for a row they can see, which is the confusion this removes.
+    const detail = unrecognized.length === platforms.length
+      ? `picker row is gated to ${unrecognized.join(', ')}, which no platform reports, so the row is offered nowhere`
+      : `picker row names ${unrecognized.join(', ')} in its gate, which no platform reports, so the row is offered only where the rest of the gate matches`
+    getLogger('manifest', { mirrorStderr: true }).warn('manifest.picker_platform_unrecognized', {
+      [Attr.PLUGIN]: manifest.name,
+      hyp_manifest_path: manifestPath,
+      hyp_picker_row: row.name,
+      hyp_unrecognized_platforms: unrecognized.join(','),
+      detail,
+    })
+  }
 }
 
 /**
@@ -268,6 +328,7 @@ function validatePickerContributions(picker) {
       return invalid('contributes.picker hidden must be a boolean when present')
     }
     // @ref LLP 0368#platform-gate [implements]: a row may name the platforms it is offered on, which `detect` cannot say because a probe may only pre-check
+    // @ref LLP 0369#warn-not-reject [constrained-by]: the string values stay unchecked here; an unrecognized one is reported by `warnUnrecognizedPickerPlatforms`, not refused
     if (r.platforms !== undefined && !(Array.isArray(r.platforms) && r.platforms.length > 0 && r.platforms.every(isNonEmptyString))) {
       return invalid('contributes.picker platforms must be a non-empty array of process.platform values when present')
     }

test/core/picker-platform-unknown-warning.test.js

@@ -0,0 +1,117 @@
+// @ts-check
+
+// A `platforms` gate is compared against `process.platform` by string
+// equality, so a typo ("macos", "Darwin", "win") matches nothing and withholds
+// the row on every platform. Manifest validation accepts the shape (LLP 0369:
+// a closed enum would kill the whole plugin over one display gate), so the
+// load-time warning is all that stands between the author and a row that
+// silently never appears.
+//
+// The line is read off the real `process.stderr` rather than off a logger
+// provider, because a default install has no provider at all (LLP
+// 0329#dark-substrate) and this diagnostic has to reach an author who
+// configured nothing.
+//
+// @ref LLP 0369#warn-not-reject [tests]: an unrecognized platform warns and still loads.
+
+import assert from 'node:assert/strict'
+import fs from 'node:fs/promises'
+import os from 'node:os'
+import path from 'node:path'
+import test from 'node:test'
+
+import { loadManifest } from '../../src/core/manifest.js'
+import { discoverBundledPlugins } from '../../src/core/runtime/bundled.js'
+import { stderrTextFrom } from '../helpers/stderr_lines.js'
+
+const WARNING = 'picker_platform_unrecognized'
+
+/**
+ * Write a one-row plugin manifest carrying `platforms` into a fresh
+ * directory and return that directory.
+ *
+ * @param {unknown} platforms
+ * @returns {Promise<string>}
+ */
+async function pluginDirWithPickerPlatforms(platforms) {
+  const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'hyp-picker-platforms-'))
+  const manifest = {
+    schema_version: 1,
+    name: '@acme/gated',
+    version: '1.0.0',
+    hypaware_api: '^1.0.0',
+    runtime: 'node',
+    entrypoint: './src/index.js',
+    contributes: {
+      picker: [{ name: 'gated', label: 'Gated', ...(platforms === undefined ? {} : { platforms }) }],
+    },
+  }
+  await fs.writeFile(path.join(dir, 'hypaware.plugin.json'), JSON.stringify(manifest), 'utf8')
+  return dir
+}
+
+/**
+ * Load the plugin in `dir`, returning the result and what it wrote to stderr.
+ *
+ * @param {string} dir
+ * @returns {Promise<{ ok: boolean, stderr: string }>}
+ */
+async function loadCapturingStderr(dir) {
+  let ok = false
+  const stderr = await stderrTextFrom(async () => {
+    ok = (await loadManifest(dir)).ok
+  })
+  return { ok, stderr }
+}
+
+test('a picker platforms value outside the known set warns on stderr and still loads the plugin', async (t) => {
+  const dir = await pluginDirWithPickerPlatforms(['macos'])
+  t.after(() => fs.rm(dir, { recursive: true, force: true }))
+
+  const { ok, stderr } = await loadCapturingStderr(dir)
+
+  // The gate is one row's display filter, not a reason to lose the plugin.
+  assert.equal(ok, true)
+
+  const warned = stderr.split('\n').filter((line) => line.includes(WARNING))
+  assert.equal(warned.length, 1, `expected one warning, got ${JSON.stringify(warned)}`)
+  // The author has to be able to find the typo from the line alone.
+  assert.match(warned[0], /WARN/)
+  assert.equal(warned[0].includes(path.join(dir, 'hypaware.plugin.json')), true)
+  assert.equal(warned[0].includes('gated'), true)
+  assert.equal(warned[0].includes('macos'), true)
+})
+
+test('a partly unrecognized gate is not reported as withholding the row everywhere', async (t) => {
+  // "darwin" still matches, so the row does render on macOS. The line has to
+  // name the half that is wrong without claiming the row went missing, or it
+  // sends the author looking for a row that is in front of them.
+  const dir = await pluginDirWithPickerPlatforms(['darwin', 'win'])
+  t.after(() => fs.rm(dir, { recursive: true, force: true }))
+
+  const { ok, stderr } = await loadCapturingStderr(dir)
+
+  assert.equal(ok, true)
+  const warned = stderr.split('\n').filter((line) => line.includes(WARNING))
+  assert.equal(warned.length, 1, `expected one warning, got ${JSON.stringify(warned)}`)
+  assert.equal(warned[0].includes('win'), true)
+  assert.equal(warned[0].includes('offered nowhere'), false, warned[0])
+})
+
+test('a picker platforms gate naming real platforms warns about nothing', async (t) => {
+  // `netbsd` is in the set for the same reason the others are: it is a value
+  // `process.platform` reports, so gating on it is correct and must stay quiet.
+  for (const platforms of [['darwin', 'linux'], ['win32'], ['netbsd'], undefined]) {
+    const dir = await pluginDirWithPickerPlatforms(platforms)
+    t.after(() => fs.rm(dir, { recursive: true, force: true }))
+
+    const { ok, stderr } = await loadCapturingStderr(dir)
+    assert.equal(ok, true)
+    assert.equal(stderr.includes(WARNING), false, `warned for ${JSON.stringify(platforms)}`)
+  }
+})
+
+test('every bundled picker row names only known platforms', async () => {
+  const stderr = await stderrTextFrom(() => discoverBundledPlugins())
+  assert.equal(stderr.includes(WARNING), false, stderr)
+})