Merge pull request #3537 from open-gsd/feat/2873-cross-scope-shadowing

src/health-diagnostic-rules/install-surface-shadowing.cts

@@ -0,0 +1,110 @@
+/**
+ * Install Surface Shadowing rule (#2873, epic #2866 Phase 4a — governed by
+ * `.gsd/phase/feat-2873-cross-scope-shadowing/40-design.md`).
+ *
+ * One code, W028, surfacing `install-shadow-report.cts`'s `ShadowReport` as
+ * a `/gsd-health` diagnostic — the design doc's row #4 ("one diagnostic,
+ * severity WARNING, same projection" as the install-time report). WARNING
+ * severity, ADVISE remedy with `risk: NONE`: shadowing is never auto-fixable
+ * (there is no single correct scope to remove) so this is advisory only,
+ * mirroring `agent-install.cts`'s W010 shape.
+ *
+ * Reaches OUTSIDE the planning snapshot the same way `agent-install.cts`
+ * does for W010: `resolveRuntime(cwd)` (`runtime-slash.cjs`) resolves the
+ * runtime id from `process.env.GSD_RUNTIME` / `config.runtime` / the
+ * `'claude'` default (never throws), using `snapshot.cwd`
+ * (`planning-snapshot.cts`'s `buildPlanningSnapshot` — `path.resolve(cwd)`)
+ * as the project directory. `buildShadowReport` is then called with that
+ * `cwd` so the local scope resolves against the project actually being
+ * health-checked, while `home` is left un-injected so the resolver defaults
+ * to `os.homedir()` — the real machine, same production call shape the
+ * installer uses.
+ *
+ * `check(snapshot)` degrades to `[]` (never throws) whenever there is
+ * nothing installable to report: `buildShadowReport` itself already
+ * degrades an unresolvable runtime (`configHome.kind === 'none'`, e.g.
+ * vscode — design row #7) to `reason: RESOLVER_UNAVAILABLE`, which
+ * `renderShadowReport` renders as `[]`; the try/catch around both calls
+ * below additionally absorbs any other unexpected throw (design row D5),
+ * since this rule is advisory and must never make `/gsd-health` itself
+ * fail.
+ *
+ * Design: .gsd/phase/feat-2873-cross-scope-shadowing/40-design.md
+ *
+ * ADR-457 build-at-publish: source in
+ * src/health-diagnostic-rules/install-surface-shadowing.cts, compiled to
+ * gsd-core/bin/lib/health-diagnostic-rules/install-surface-shadowing.cjs
+ * (gitignored).
+ */
+
+// eslint-disable-next-line @typescript-eslint/no-require-imports -- type-only; erased at compile time, no runtime require emitted
+import type planningSnapshotMod = require('../planning-snapshot.cjs');
+type PlanningSnapshot = ReturnType<typeof planningSnapshotMod.buildPlanningSnapshot>;
+
+// eslint-disable-next-line @typescript-eslint/no-require-imports
+import healthDiagnosticMod = require('../health-diagnostic-types.cjs');
+const { SEVERITY, adviseRemedy } = healthDiagnosticMod;
+type Rule = healthDiagnosticMod.Rule;
+type Diagnostic = healthDiagnosticMod.Diagnostic;
+
+// eslint-disable-next-line @typescript-eslint/no-require-imports
+import installShadowReportMod = require('../install-shadow-report.cjs');
+const { buildShadowReport, renderShadowReport } = installShadowReportMod;
+
+// eslint-disable-next-line @typescript-eslint/no-require-imports
+import runtimeSlashMod = require('../runtime-slash.cjs');
+const { resolveRuntime } = runtimeSlashMod;
+
+import { PACKAGE_NAME } from '../package-identity.cjs';
+
+/**
+ * `check(snapshot)` for W028 — see module header for the degrade-to-`[]`
+ * cases and the runtime-resolution mechanism reused from `agent-install.cts`.
+ */
+function checkInstallSurfaceShadowing(snapshot: PlanningSnapshot): Diagnostic[] {
+  let lines: string[];
+  let runtime: string;
+  try {
+    runtime = resolveRuntime(snapshot.cwd);
+    const report = buildShadowReport(runtime, { cwd: snapshot.cwd });
+    // `renderShadowReport` returns `[]` for every reason other than
+    // SCOPE_SHADOWED (design rows #1, #2, #6, #7, #8, #11), and sanitizes
+    // every `declaredRuntime` it interpolates via `sanitizeForRender`
+    // internally (`install-shadow-report.cts`) — this rule's `message`
+    // reuses that render path rather than re-sanitizing, so the
+    // sanitize-at-the-render-seam guarantee (design row #13) holds here too.
+    // Trigger names are already SAFE_STEM-gated upstream
+    // (`installed-surface-resolver.cts`'s `deriveStemsForKindEntry`) before
+    // they ever reach a rendered line — no re-gating needed here either.
+    lines = renderShadowReport(report);
+  } catch {
+    // Advisory rule: an unresolvable runtime, a `configHome.kind === 'none'`
+    // runtime, or any other unexpected failure degrades to "no finding",
+    // never a thrown exception that would break `/gsd-health` itself
+    // (design row D5).
+    return [];
+  }
+
+  if (lines.length === 0) return [];
+
+  return [
+    {
+      code: 'W028',
+      severity: SEVERITY.WARNING,
+      message: lines.join(' '),
+      remedy: adviseRemedy(`Review install scopes for ${runtime}: npx ${PACKAGE_NAME}@latest`),
+    },
+  ];
+}
+
+const RULES: Rule[] = [
+  {
+    code: 'W028',
+    severity: SEVERITY.WARNING,
+    description: 'A GSD-owned install scope shadows another on this machine',
+    repairable: false,
+    check: checkInstallSurfaceShadowing,
+  },
+];
+
+export = { RULES };

src/health-diagnostic.cts

@@ -83,6 +83,8 @@ import worktreeHealthMod = require('./health-diagnostic-rules/worktree-health.cj
 import milestoneArchiveHygieneMod = require('./health-diagnostic-rules/milestone-archive-hygiene.cjs');
 // eslint-disable-next-line @typescript-eslint/no-require-imports
 import consistencyMod = require('./health-diagnostic-rules/consistency.cjs');
+// eslint-disable-next-line @typescript-eslint/no-require-imports
+import installSurfaceShadowingMod = require('./health-diagnostic-rules/install-surface-shadowing.cjs');
 
 const RULES: Rule[] = [
   ...rootExistenceMod.RULES,
@@ -93,6 +95,7 @@ const RULES: Rule[] = [
   ...roadmapDiskConsistencyMod.RULES,
   ...worktreeHealthMod.RULES,
   ...milestoneArchiveHygieneMod.RULES,
+  ...installSurfaceShadowingMod.RULES,
 ];
 
 /**

src/install-shadow-report.cts

@@ -0,0 +1,457 @@
+/**
+ * install-shadow-report.cts — Cross-Scope Shadow Report Module (#2873, epic
+ * #2866 Phase 4a — governed by
+ * `.gsd/phase/feat-2873-cross-scope-shadowing/40-design.md`).
+ *
+ * A read-only PROJECTION over `resolveInstalledSurfaces`
+ * (`installed-surface-resolver.cts`, #2872 Phase 3). That module answers
+ * "what is installed"; it is documented there as read-only, and rendering
+ * plus sanitization are a different concern with a different consumer set
+ * (installer + `/gsd-health`) — the design doc's "Rejected" #5 is why this is
+ * a separate leaf module rather than a second export bolted onto the
+ * resolver.
+ *
+ * ── What "shadowed" means here ──────────────────────────────────────────────
+ * A trigger is shadowed when `resolveTriggerSurface` (via the resolver)
+ * recorded a non-null `shadowedBy` for it: two scopes both installed a
+ * trigger-bearing artifact under the SAME trigger name, and only one wins.
+ * For claude (`skills`@global vs `commands`@local) the KINDS differ, so the
+ * loser's entire spec tree becomes unreachable through the trigger — the bug
+ * #2218 diagnosed. For the 12 both-scopes-`skills` runtimes the kinds are the
+ * SAME on both sides, so the loser is merely overridden, not vanished
+ * (design row #5 / "Not-corruption"). `kindsDiffer` on `ShadowReport` is what
+ * lets `renderShadowReport` word the two cases correctly.
+ *
+ * ── Report, don't correct (mirrors the resolver's own law) ─────────────────
+ * `mismatches` surfaces a declared runtime/scope that disagrees with the
+ * probed one (Postel's Law, design doc: liberal in what is accepted, but the
+ * mismatch is never silently absorbed). This module never substitutes a
+ * declared value for a probed one; it only reports the disagreement the
+ * resolver already computed.
+ *
+ * ── Sanitize at the render seam ─────────────────────────────────────────────
+ * `declaredRuntime` is attacker-influenceable (it comes from a manifest that
+ * may live inside a merely-cloned repository) and length-bounded but
+ * deliberately NOT charset-gated by the reader (`declaredRuntimeMatchesProbe`
+ * needs the raw value there). THIS module is what renders it to an operator,
+ * so this module owns the guard — `sanitizeForRender` strips ANSI escapes,
+ * C0/C1 controls, and Unicode bidi overrides/isolates, then collapses
+ * whitespace. It never truncates: `readInstallManifest` already caps at 64
+ * chars, and a second truncation here would double-truncate.
+ *
+ * Trigger names, by contrast, are already `SAFE_STEM`-gated upstream
+ * (`installed-surface-resolver.cts`'s `deriveStemsForKindEntry`) before they
+ * ever reach a `TriggerSurface` — this module does not re-gate them.
+ *
+ * ── Per-scope truth filter (why this lives HERE, not in the resolver) ──────
+ * `resolveOneRuntime` (`installed-surface-resolver.cts`) builds ONE union of
+ * every installed scope's `stems` and hands that single list to
+ * `resolveTriggerSurface`, which then synthesizes a candidate trigger for
+ * EVERY stem at EVERY installed scope's trigger-bearing kind entry —
+ * regardless of whether that specific scope's own manifest actually shipped
+ * that stem. Concretely: a global `full`-profile install (stems a, b, c)
+ * alongside a local `core`-profile install (stem a only) unions to
+ * `{a, b, c}`, and `resolveTriggerSurface` then reports `commands@local`
+ * candidates for b and c too — trigger names for artifacts that do not exist
+ * on disk at that scope. Left unfiltered, this module would tell the user
+ * `/gsd-b` and `/gsd-c` are shadowed local commands when there is no local
+ * artifact for either at all — over-reporting that is not cosmetic, since
+ * the whole point of this report is to make a real failure legible.
+ *
+ * `resolveTriggerSurface`'s API takes ONE stem list shared by every scope it
+ * is asked about, so per-scope truth cannot be expressed through it without
+ * either widening a shipped Phase-2 contract other callers may depend on, or
+ * calling it once per scope and re-implementing its winner computation
+ * (`isHigherPriority`) here as a second, driftable copy. `resolveOneRuntime`
+ * / `resolveInstalledSurfaces` (Phase 3, #2872) is likewise a shipped module
+ * this task deliberately leaves untouched. This module already receives the
+ * full `InstalledRuntimeSurface`, including each scope's own REAL `stems`
+ * list (`installed-surface-resolver.cts`'s `deriveStemsFromManifest`) — so
+ * the correction belongs here, as a filter over `resolveTriggerSurface`'s
+ * already-computed `shadowedBy` groups: a trigger is reported as shadowed
+ * only when its underlying stem is present in BOTH the winner's scope's own
+ * `stems` AND the shadowed side's scope's own `stems` — i.e. an artifact
+ * genuinely exists at both scopes, not merely "some stem exists somewhere in
+ * the union".
+ *
+ * `TriggerSurface` does not carry the originating stem OR the composing
+ * prefix on its output — only the already-composed `trigger` string
+ * (`${prefix}${stem}`) — so the stem cannot be read off it directly. Rather
+ * than hand-roll a fixed-offset `trigger.slice(4)` (which would silently
+ * assume every runtime's prefix is exactly `gsd-` — true today, but not a
+ * contract this module owns), the prefix is recovered the honest way: by
+ * re-resolving that scope's `ArtifactKind` layout (`resolveRuntimeArtifactLayout`
+ * / `resolveRuntimeArtifactLayoutFromRegistry`, the SAME layout descriptor
+ * `resolveTriggerSurface` itself reads its `entry.prefix` from) for the
+ * winner's and shadowed side's own `(scope, kind)`, and reading `.prefix`
+ * off the matching kind entry. This is metadata-only (constructing an
+ * `ArtifactKind` never touches the filesystem — see
+ * `runtime-artifact-layout.cts`'s kind-builder functions), so it costs
+ * nothing beyond a small per-`(scope,kind)` memo. If a prefix cannot be
+ * resolved at all (a `TypeError` from an unexpected registry shape), the
+ * trigger is conservatively DROPPED rather than kept — the same
+ * report-nothing-you-cannot-prove posture as the rest of this filter.
+ *
+ * ── Pure with respect to caller-visible state ───────────────────────────────
+ * `buildShadowReport` builds a fresh `ShadowReport` (fresh arrays, fresh
+ * objects) on every call, exactly as the resolver documents for itself
+ * (`installed-surface-resolver.cts`'s "Pure with respect to caller-visible
+ * state" paragraph) — no shared or cached state between calls.
+ */
+
+import { type InstallScope } from './install-scope.cjs';
+import {
+  resolveInstalledSurfaces,
+  type ResolveInstalledSurfacesOptions,
+  type InstalledRuntimeSurface,
+  type InstalledScopeRecord,
+} from './installed-surface-resolver.cjs';
+
+// eslint-disable-next-line @typescript-eslint/no-require-imports
+import runtimeArtifactLayoutMod = require('./runtime-artifact-layout.cjs');
+const { resolveRuntimeArtifactLayout, resolveRuntimeArtifactLayoutFromRegistry } = runtimeArtifactLayoutMod;
+
+/** The registry shape `resolveRuntimeArtifactLayoutFromRegistry` accepts as
+ *  its first argument — reused (not re-typed) so `opts.registry` can be
+ *  forwarded to it, mirroring `installed-surface-resolver.cts`'s own
+ *  `LayoutRegistryLike`. */
+type LayoutRegistryLike = Parameters<typeof resolveRuntimeArtifactLayoutFromRegistry>[0];
+
+/** `installed-surface-resolver.cts` does not export `TriggerSurface` by
+ *  name (only via `InstalledRuntimeSurface.triggers`'s element type) —
+ *  derived here rather than re-declared as a second, driftable shape. */
+type TriggerSurface = InstalledRuntimeSurface['triggers'][number];
+
+// ── Reason enum ─────────────────────────────────────────────────────────
+
+export const SHADOW_REASON = Object.freeze({
+  NOT_SHADOWED: 'not_shadowed',
+  SCOPE_SHADOWED: 'scope_shadowed',
+  RESOLVER_UNAVAILABLE: 'resolver_unavailable',
+});
+
+// ── Public types ────────────────────────────────────────────────────────
+
+export interface ShadowedTrigger {
+  trigger: string;
+  winnerKind: string;
+  winnerScope: InstallScope;
+  shadowedKind: string;
+  shadowedScope: InstallScope;
+}
+
+export interface DeclarationMismatch {
+  scope: InstallScope;
+  declaredRuntime: string | null;
+  declaredRuntimeMatchesProbe: boolean | null;
+  declaredScope: InstallScope | null;
+  declaredScopeMatchesProbe: boolean | null;
+}
+
+export interface ShadowReport {
+  runtime: string;
+  reason: string;
+  shadowed: boolean;
+  winner: { kind: string; scope: InstallScope } | null;
+  shadowedSide: { kind: string; scope: InstallScope } | null;
+  kindsDiffer: boolean;
+  triggers: ShadowedTrigger[];
+  mismatches: DeclarationMismatch[];
+}
+
+// ── Sanitization ────────────────────────────────────────────────────────
+
+/** CSI (`\x1b[...final`) and OSC (`\x1b]...BEL-or-ST`) sequences. An
+ *  unterminated/malformed sequence is left for the C0-control strip below to
+ *  remove the bare `\x1b` byte — liberal, never a throw. */
+const ANSI_RE = /\x1b(?:\[[0-?]*[ -/]*[@-~]|\][^\x07\x1b]*(?:\x07|\x1b\\))/g;
+
+/** C0 controls (`\x00`-`\x1f`, including any `\x1b` the ANSI strip above did
+ *  not consume) and DEL/C1 (`\x7f`-`\x9f`). */
+const CONTROL_RE = /[\x00-\x1f\x7f-\x9f]/g;
+
+/** Unicode bidi embedding/override controls (U+202A-U+202E) and bidi
+ *  isolates (U+2066-U+2069) — the RTL-spoofing class the design doc's row
+ *  #13 names. */
+const BIDI_RE = /[\u{202A}-\u{202E}\u{2066}-\u{2069}]/gu;
+
+/** Combining marks (U+0300-U+036F) — "zalgo" text. Stacked onto the
+ *  preceding base character, an unbounded run visually overflows into
+ *  adjacent terminal cells/rows even though the string stays within the
+ *  64-char cap `readInstallManifest` enforces. Written as `\u{...}` escapes
+ *  (not literal combining characters) so the source itself stays plain
+ *  ASCII and does not visually combine in editors/diffs. */
+const COMBINING_MARK_RE = /[\u{0300}-\u{036F}]/gu;
+
+/** Zero-width characters: ZWSP (U+200B), ZWNJ (U+200C), ZWJ (U+200D), and
+ *  BOM/ZWNBSP (U+FEFF). None of these are JS `\s`, so they survive both the
+ *  char-count cap and the whitespace-collapse step below undetected. */
+const ZERO_WIDTH_RE = /[\u{200B}-\u{200D}\u{FEFF}]/gu;
+
+/**
+ * Sanitize a `declaredRuntime` (or any similarly attacker-influenceable
+ * string) for terminal/console rendering. `null` passes through as `null`;
+ * `''` passes through as `''`. Strips ANSI escapes, C0/C1 controls, Unicode
+ * bidi overrides/isolates, combining marks (zalgo), and zero-width
+ * characters (replacing each stripped run with nothing — never a space),
+ * then collapses any remaining whitespace run (including adjacent spaces
+ * left behind by a removed newline) to a single space and trims.
+ *
+ * Idempotent by construction: once ANSI/control/bidi/combining/zero-width
+ * bytes are gone and whitespace is collapsed to single internal spaces with
+ * no leading/trailing space, a second pass finds nothing left to strip or
+ * collapse. Pure character-class filter — never truncates; `readInstallManifest`
+ * already caps at 64 chars.
+ */
+export function sanitizeForRender(value: string | null): string | null {
+  if (value === null) return null;
+  const stripped = value
+    .replace(ANSI_RE, '')
+    .replace(CONTROL_RE, '')
+    .replace(BIDI_RE, '')
+    .replace(COMBINING_MARK_RE, '')
+    .replace(ZERO_WIDTH_RE, '');
+  return stripped.replace(/\s+/g, ' ').trim();
+}
+
+// ── Per-scope truth filter helpers ─────────────────────────────────────
+
+/**
+ * Build a `(scope, kind) -> prefix | null` lookup for one runtime, memoized
+ * per call to `buildShadowReport` (never shared across calls — matches this
+ * module's "fresh objects on every call" contract). `null` means "could not
+ * be resolved" (unknown scope record, or a `TypeError` from the layout
+ * resolver) — the caller treats that as "cannot honestly attribute this
+ * trigger to a real stem here", not as "assume it is fine".
+ */
+function buildPrefixLookup(
+  runtime: string,
+  scopeRecords: Map<InstallScope, InstalledScopeRecord>,
+  opts: ResolveInstalledSurfacesOptions,
+): (scope: InstallScope, kind: string) => string | null {
+  const cache = new Map<string, string | null>();
+  return (scope: InstallScope, kind: string): string | null => {
+    const key = `${scope}:${kind}`;
+    if (cache.has(key)) return cache.get(key) as string | null;
+    const record = scopeRecords.get(scope);
+    let prefix: string | null = null;
+    if (record) {
+      try {
+        const layout = opts.registry !== undefined
+          ? resolveRuntimeArtifactLayoutFromRegistry(opts.registry as LayoutRegistryLike, runtime, record.configHome, record.scope)
+          : resolveRuntimeArtifactLayout(runtime, record.configHome, record.scope);
+        const kindEntry = (layout.kinds as Array<{ kind: string; prefix: string }>).find((k) => k.kind === kind);
+        prefix = kindEntry ? kindEntry.prefix : null;
+      } catch {
+        // Unknown runtime / malformed registry — degrade to "cannot resolve",
+        // never throw out of a report builder (matches this module's own
+        // RESOLVER_UNAVAILABLE degrade-not-propagate posture above).
+        prefix = null;
+      }
+    }
+    cache.set(key, prefix);
+    return prefix;
+  };
+}
+
+/** `trigger` minus `prefix`, or `null` when `prefix` is unknown, does not
+ *  actually prefix `trigger`, or the remainder would be empty (a `prefix`
+ *  covering the whole trigger string is not a real stem). */
+function stemFromTrigger(trigger: string, prefix: string | null): string | null {
+  if (prefix === null || !trigger.startsWith(prefix)) return null;
+  const stem = trigger.slice(prefix.length);
+  return stem === '' ? null : stem;
+}
+
+/**
+ * True when `t` (a `resolveTriggerSurface`-reported shadowed trigger) is a
+ * REAL cross-scope shadow: its stem is present in the winner's OWN scope
+ * `stems` and, independently, in the shadowed side's OWN scope `stems`. See
+ * the module-level "Per-scope truth filter" comment for why this check
+ * exists and why it lives here rather than in the resolver.
+ */
+function isGenuinelyShadowed(
+  t: TriggerSurface,
+  scopeRecords: Map<InstallScope, InstalledScopeRecord>,
+  prefixFor: (scope: InstallScope, kind: string) => string | null,
+): boolean {
+  if (t.shadowedBy === null) return false;
+  const winnerRecord = scopeRecords.get(t.shadowedBy.scope);
+  const shadowedRecord = scopeRecords.get(t.scope);
+  const winnerStem = stemFromTrigger(t.trigger, prefixFor(t.shadowedBy.scope, t.shadowedBy.kind));
+  const shadowedStem = stemFromTrigger(t.trigger, prefixFor(t.scope, t.kind));
+  if (winnerStem === null || shadowedStem === null) return false;
+  return (winnerRecord?.stems ?? []).includes(winnerStem) && (shadowedRecord?.stems ?? []).includes(shadowedStem);
+}
+
+// ── Report builder ──────────────────────────────────────────────────────
+
+/**
+ * Build a shadow report for one runtime. `opts` is forwarded VERBATIM to
+ * `resolveInstalledSurfaces` — this function adds no option of its own.
+ * Production call shape: `buildShadowReport('claude', { home, cwd })`.
+ *
+ * A `resolveInstalledSurfaces` `TypeError` (unknown runtime, or
+ * `configHome.kind === 'none'`, e.g. vscode — design row #7) degrades to
+ * `reason: RESOLVER_UNAVAILABLE` rather than propagating: an install-time or
+ * `/gsd-health` caller must never crash because a runtime has no installable
+ * config directory. Any other error type is rethrown — mirrors the
+ * resolver's own `TypeError` narrowing (`resolveInstalledSurfaces`'s sweep
+ * catch, and `buildScopeRecord`'s stem-derivation catch) so the two cannot
+ * drift apart.
+ */
+export function buildShadowReport(runtime: string, opts: ResolveInstalledSurfacesOptions = {}): ShadowReport {
+  let surfaces: InstalledRuntimeSurface[];
+  try {
+    surfaces = resolveInstalledSurfaces(runtime, opts);
+  } catch (error) {
+    if (!(error instanceof TypeError)) throw error;
+    return {
+      runtime,
+      reason: SHADOW_REASON.RESOLVER_UNAVAILABLE,
+      shadowed: false,
+      winner: null,
+      shadowedSide: null,
+      kindsDiffer: false,
+      triggers: [],
+      mismatches: [],
+    };
+  }
+
+  // resolveInstalledSurfaces(runtime, opts) with an explicit string `runtime`
+  // always returns exactly one element (see its own doc comment).
+  const surface = surfaces[0];
+
+  // Per-scope truth filter (see module comment): `surface.triggers` may
+  // contain candidates synthesized from the CROSS-SCOPE stem union
+  // (`installed-surface-resolver.cts`'s `stemUnion`) that do not correspond
+  // to a real artifact at one or both scopes. Only a trigger whose stem is
+  // provably present in BOTH the winner's own `stems` and the shadowed
+  // side's own `stems` is reported.
+  const scopeRecords = new Map<InstallScope, InstalledScopeRecord>(surface.scopes.map((r) => [r.scope, r] as const));
+  const prefixFor = buildPrefixLookup(runtime, scopeRecords, opts);
+  const shadowedSurfaces = surface.triggers.filter((t) => isGenuinelyShadowed(t, scopeRecords, prefixFor));
+  const triggers: ShadowedTrigger[] = shadowedSurfaces
+    .map((t) => ({
+      trigger: t.trigger,
+      // `shadowedBy` is non-null by construction of the filter above.
+      winnerKind: t.shadowedBy!.kind,
+      winnerScope: t.shadowedBy!.scope,
+      shadowedKind: t.kind,
+      shadowedScope: t.scope,
+    }))
+    .sort((a, b) => (a.trigger < b.trigger ? -1 : a.trigger > b.trigger ? 1 : 0));
+
+  const mismatches: DeclarationMismatch[] = [];
+  for (const record of surface.scopes) {
+    if (record.declaredRuntimeMatchesProbe === false || record.declaredScopeMatchesProbe === false) {
+      mismatches.push({
+        scope: record.scope,
+        // Postel's Law (design doc): sanitized here because this is the
+        // render seam — never silently absorbed, always surfaced.
+        declaredRuntime: sanitizeForRender(record.declaredRuntime),
+        declaredRuntimeMatchesProbe: record.declaredRuntimeMatchesProbe,
+        declaredScope: record.declaredScope,
+        declaredScopeMatchesProbe: record.declaredScopeMatchesProbe,
+      });
+    }
+  }
+
+  if (triggers.length === 0) {
+    return {
+      runtime,
+      reason: SHADOW_REASON.NOT_SHADOWED,
+      shadowed: false,
+      winner: null,
+      shadowedSide: null,
+      kindsDiffer: false,
+      triggers: [],
+      mismatches,
+    };
+  }
+
+  // Winner/shadowedSide are the (kind,scope) pair of the FIRST shadowed
+  // trigger (post-sort, for the same determinism reason the array itself is
+  // sorted). They are asserted-by-construction uniform across the whole set
+  // for every runtime this module has seen (every trigger shadowed by the
+  // SAME scope, with the SAME two kinds, on one machine) — but if a future
+  // registry shape ever produced a non-uniform set, this still returns the
+  // first pair rather than throwing; every distinct (kind,scope) pair is
+  // already visible per-entry in `triggers` itself, so nothing is lost.
+  const first = triggers[0];
+  const winner = { kind: first.winnerKind, scope: first.winnerScope };
+  const shadowedSide = { kind: first.shadowedKind, scope: first.shadowedScope };
+
+  return {
+    runtime,
+    reason: SHADOW_REASON.SCOPE_SHADOWED,
+    shadowed: true,
+    winner,
+    shadowedSide,
+    kindsDiffer: winner.kind !== shadowedSide.kind,
+    triggers,
+    mismatches,
+  };
+}
+
+// ── Renderer ────────────────────────────────────────────────────────────
+
+/**
+ * Render a `ShadowReport` to plain lines — no ANSI, no color, no leading
+ * indent. The caller (installer console output, `/gsd-health` text mode)
+ * owns terminal formatting; this keeps the module free of terminal concerns
+ * and testable without a spawned process. Structured (`--json`) health
+ * output (design row #17) consumes the typed `ShadowReport` directly and
+ * never calls this function.
+ *
+ * `reason !== SCOPE_SHADOWED` renders nothing — there is nothing to report
+ * (design rows #1, #2, #6, #7, #8, #11).
+ */
+export function renderShadowReport(report: ShadowReport, opts: { sampleLimit?: number } = {}): string[] {
+  if (report.reason !== SHADOW_REASON.SCOPE_SHADOWED || report.winner === null || report.shadowedSide === null) {
+    return [];
+  }
+
+  const sampleLimit = opts.sampleLimit ?? 5;
+  const count = report.triggers.length;
+  const plural = count === 1 ? '' : 's';
+  const { winner, shadowedSide, kindsDiffer } = report;
+
+  const lines: string[] = [];
+  lines.push(
+    kindsDiffer
+      ? `${count} trigger${plural} shadowed: the ${shadowedSide.scope} ${shadowedSide.kind} surface is unreachable through ${count === 1 ? 'that trigger' : 'those triggers'} — ${winner.scope} ${winner.kind} wins instead.`
+      : `${count} trigger${plural} shadowed: the ${shadowedSide.scope} ${shadowedSide.kind} ${count === 1 ? 'entry is' : 'entries are'} overridden by ${winner.scope} ${winner.kind}.`,
+  );
+
+  // Trigger names in `report.triggers` are already SAFE_STEM-gated upstream
+  // (installed-surface-resolver.cts's deriveStemsForKindEntry) — no re-gating
+  // needed here.
+  const sample = report.triggers.slice(0, sampleLimit);
+  for (const t of sample) {
+    lines.push(`  - ${t.trigger}: ${t.shadowedScope}/${t.shadowedKind} shadowed by ${t.winnerScope}/${t.winnerKind}`);
+  }
+  const remaining = count - sample.length;
+  if (remaining > 0) {
+    lines.push(`  ...and ${remaining} more`);
+  }
+
+  for (const m of report.mismatches) {
+    // Re-sanitized defensively: `buildShadowReport` already sanitizes
+    // `declaredRuntime` before it reaches a `ShadowReport`, and
+    // `sanitizeForRender` is idempotent, so this is a no-op in the normal
+    // path and a real guard against a hand-built `ShadowReport` (e.g. a
+    // renderer-only test) that skipped it.
+    const declaredRuntime = sanitizeForRender(m.declaredRuntime);
+    const parts: string[] = [];
+    if (m.declaredRuntimeMatchesProbe === false) {
+      parts.push(`declared runtime "${declaredRuntime}" does not match this runtime`);
+    }
+    if (m.declaredScopeMatchesProbe === false) {
+      parts.push(`declared scope "${m.declaredScope}" does not match the probed ${m.scope} scope`);
+    }
+    lines.push(`Note: ${m.scope} scope manifest mismatch — ${parts.join('; ')}.`);
+  }
+
+  return lines;
+}

src/installed-surface-resolver.cts

@@ -53,6 +53,8 @@
  * than re-deriving either rule as a fourth independent copy.
  */
 
+import fs from 'node:fs';
+import path from 'node:path';
 import { resolveScope, SCOPE_ORDER, type InstallScope } from './install-scope.cjs';
 import { posixNormalize } from './shell-command-projection.cjs';
 
@@ -68,7 +70,7 @@ const {
 
 // eslint-disable-next-line @typescript-eslint/no-require-imports
 import installerMigrationsMod = require('./installer-migrations.cjs');
-const { readInstallManifest } = installerMigrationsMod;
+const { readInstallManifest, MANIFEST_NAME } = installerMigrationsMod;
 
 // In .cts (CommonJS output) files, `require` is available as a global.
 const _require: NodeRequire = require;
@@ -130,6 +132,12 @@ export interface ResolveInstalledSurfacesOptions {
   cwd?: string;
   env?: Record<string, string | undefined>;
   existsSync?: (p: string) => boolean;
+  /** Injected for tests, matching `existsSync` above; defaults to
+   *  `node:fs`'s `lstatSync`. Used to refuse a manifest read when the
+   *  scope's config dir or manifest file is a symlink — see the
+   *  `buildScopeRecord` comment for why this is a deliberate hardening,
+   *  not an oversight. */
+  lstatSync?: (p: string) => { isSymbolicLink(): boolean };
   registry?: unknown;
   /** Injected for tests; defaults to installer-migrations' readInstallManifest. */
   readManifest?: (configDir: string) => {
@@ -252,6 +260,45 @@ function deriveStemsFromManifest(
   return [...stems].sort();
 }
 
+/**
+ * True when `p` is a symlink. An `lstatSync` throw (ENOENT — nothing at this
+ * path) is NOT evidence of a symlink; it is treated as "not a symlink" here
+ * and left for `readManifest` to classify (it already owns the absent-file
+ * case, per C14 above).
+ */
+function isSymlinkPath(p: string, lstatSync: (p: string) => { isSymbolicLink(): boolean }): boolean {
+  try {
+    return lstatSync(p).isSymbolicLink();
+  } catch {
+    return false;
+  }
+}
+
+/**
+ * The shared "not installed" degraded shape (C14's EACCES path, and this
+ * phase's new symlink-guard path). A FACTORY, not a module-level constant
+ * object: a `const` object spread at each return site would still share the
+ * same `stems` ARRAY reference across every call (`...` shallow-copies the
+ * object but not the array a property points at), which would violate this
+ * module's own "builds fresh arrays/objects on every call" contract (C15) —
+ * a caller mutating one degraded record's `stems` must never be visible on
+ * another's.
+ */
+function notInstalledScopeRecordFields(): Pick<
+  InstalledScopeRecord,
+  'installed' | 'manifestVersion' | 'declaredRuntime' | 'declaredScope' | 'declaredScopeMatchesProbe' | 'declaredRuntimeMatchesProbe' | 'stems'
+> {
+  return {
+    installed: false,
+    manifestVersion: null,
+    declaredRuntime: null,
+    declaredScope: null,
+    declaredScopeMatchesProbe: null,
+    declaredRuntimeMatchesProbe: null,
+    stems: [],
+  };
+}
+
 /**
  * Build one scope's record. `resolvedConfigHome` has already been probed
  * successfully by the time this is called (a `resolveScope` `TypeError` is
@@ -278,6 +325,26 @@ function buildScopeRecord(
   resolvedConfigHome: string,
   opts: ResolveInstalledSurfacesOptions,
 ): InstalledScopeRecord {
+  // Hardening requirement 2 (#2873 design doc, "Hardening requirements
+  // claimed from #2873's comment"): `readInstallManifest` -> `readJsonIfPresent`
+  // uses `existsSync` + `readFileSync` and therefore FOLLOWS symlinks, and
+  // this module resolves `local` against `process.cwd()` — a directory that,
+  // as of this phase, becomes reachable from an arbitrary cloned repository
+  // (this is the same phase that makes the local scope's manifest a first
+  // read target, not merely a write target). The in-tree precedent is
+  // `getAgentsDir` (`agent-install-check.cts`), which probes with
+  // `fs.lstatSync(...).isDirectory()`/`.isFile()` and deliberately does not
+  // follow. #2872 left this resolver's read un-guarded only because the path
+  // had zero callers at the time; refusing to follow a symlinked config dir
+  // or manifest here closes that asymmetry rather than carrying it forward.
+  // Degrading to `installed: false` reuses the SAME shape the EACCES catch
+  // below already returns — no new failure shape is introduced.
+  const lstatSync = opts.lstatSync ?? fs.lstatSync;
+  const manifestPath = path.join(resolvedConfigHome, MANIFEST_NAME);
+  if (isSymlinkPath(resolvedConfigHome, lstatSync) || isSymlinkPath(manifestPath, lstatSync)) {
+    return { scope: scopeId, configHome: resolvedConfigHome, ...notInstalledScopeRecordFields() };
+  }
+
   let manifest: {
     manifestVersion: number | null;
     runtime: string | null;
@@ -293,17 +360,7 @@ function buildScopeRecord(
     // read failure at all (EACCES, ENOENT-after-race, a corrupt filesystem)
     // legitimately means "cannot tell whether installed" (design row C14), so
     // there is no error TYPE here that should instead propagate.
-    return {
-      scope: scopeId,
-      configHome: resolvedConfigHome,
-      installed: false,
-      manifestVersion: null,
-      declaredRuntime: null,
-      declaredScope: null,
-      declaredScopeMatchesProbe: null,
-      declaredRuntimeMatchesProbe: null,
-      stems: [],
-    };
+    return { scope: scopeId, configHome: resolvedConfigHome, ...notInstalledScopeRecordFields() };
   }
 
   const installed = manifest.manifestVersion !== null; // C9: presence, never the new fields

src/installer-migrations.cts

@@ -246,7 +246,14 @@ function normalizeManifestVersion(raw: unknown): number {
 
 function readInstallManifest(configDir: string): InstallManifest {
   const manifest = readJsonIfPresent(path.join(configDir, MANIFEST_NAME), null);
-  if (!manifest || typeof manifest !== 'object') {
+  // `typeof [] === 'object'` in JS, so a bare `typeof !== 'object'` guard lets
+  // a top-level JSON array (valid JSON, but not the manifest's documented
+  // object shape) fall through to the field reads below — `m.manifestVersion`
+  // reads `undefined` off an array, which `normalizeManifestVersion` then
+  // reports as `1` (a v1 manifest), misclassifying "not an object" as
+  // "installed". `Array.isArray` closes that gap explicitly rather than
+  // relying on the object-shape checks below to catch it incidentally.
+  if (!manifest || typeof manifest !== 'object' || Array.isArray(manifest)) {
     return {
       version: null,
       timestamp: null,

src/runtime-artifact-conversion.cts

@@ -27,6 +27,7 @@ import capabilityRegistry = require('./capability-registry.cjs');
 import hostIntegration = require('./host-integration.cjs');
 import { posixNormalize } from './shell-command-projection.cjs';
 import { escapeRegex as escapeRegExp } from './pattern.cjs';
+import { scanFencedBlocks } from './markdown-sectionizer.cjs';
 // #2870: install-scope.cts is a leaf-tier sibling (imports only
 // runtime-homes.cjs + node builtins, never this module) — no cycle. See the
 // isGlobal sites below for why the boolean projection is centralized here too.
@@ -504,6 +505,90 @@ function convertClaudeCommandToClaudeSkill(content, skillName, runtime = null, c
   return `${fm}\n${normalizedBody}`;
 }
 
+// #2873 (4b) — spec-root reachability. Matches ONLY a line that is a real
+// `@~/.claude/gsd-core/workflows/<stem>.md` include: line-start `@`, exact
+// spec-root shape, nothing else on the line. This is deliberately narrower
+// than "any line mentioning gsd-core/workflows" so prose mentions and
+// `references/`/`templates/`/`@.planning/...` includes are never touched
+// (rows 24/25). CRLF-safe: an optional trailing `\r` is captured and
+// preserved rather than dropped.
+const WORKFLOW_SPEC_ROOT_INCLUDE_RE = /^@~\/\.claude\/gsd-core\/workflows\/([A-Za-z0-9._-]+)\.md[ \t]*(\r?)$/gm;
+
+/**
+ * Rewrite a static global-scope Claude skill `@`-include of the command's own
+ * workflow spec into an imperative two-step resolution the agent performs at
+ * runtime: prefer the project-local spec (cwd-relative), fall back to the
+ * global spec, and treat "neither exists" as a visible failure rather than a
+ * silent no-spec proceed.
+ *
+ * WHY this can't stay a static `@`-include (even a relative one): Claude Code
+ * documents relative `@`-paths as resolving against the file *containing* the
+ * import, which for a global skill is `~/.claude/skills/gsd-<stem>/` — not
+ * the project's working directory. `@./.claude/...` would therefore always
+ * resolve inside the skill's own install directory, never the project, so
+ * there is no static include syntax that can express "prefer local, fall
+ * back to global". This function exists precisely so that resolution can be
+ * performed by the agent, not the host's pre-expansion.
+ *
+ * Scope-free by design: this function does not know or care whether it is
+ * being applied to a global or local artifact, or which runtime — that
+ * judgment belongs to the caller (`skillsKind` in
+ * `runtime-artifact-layout.cts`, the one site that knows install scope).
+ * Applying it to a body with no workflow include is a no-op (row 26); a body
+ * with two independent workflow includes has each rewritten independently
+ * (row 27); an include inside a fenced code block or wrapped in inline
+ * backticks is left untouched (the backtick case is already excluded by the
+ * line-start anchor, since a backtick-wrapped line does not begin with `@`).
+ * Idempotent: the replacement text never begins with `@` and never matches
+ * `WORKFLOW_SPEC_ROOT_INCLUDE_RE`, so re-applying this function to its own
+ * output is a no-op.
+ *
+ * Fence detection reuses `scanFencedBlocks` (markdown-sectionizer.cts) — the
+ * same CommonMark-correct state machine `stripFencedCode`/`extractFencedBlock`
+ * are built on — instead of a hand-rolled "any delimiter line toggles
+ * open/closed" tracker. A naive toggle is wrong under CommonMark: a fence
+ * opened with ``` is NOT closed by a ~~~ line (closer must share the
+ * opener's delimiter character and have run length >= the opener's), so a
+ * mismatched delimiter is fence CONTENT, not a boundary. #2873 review.
+ */
+function resolveSpecRootReference(body) {
+  if (typeof body !== 'string' || body.length === 0) return body;
+  if (!body.includes('@~/.claude/gsd-core/workflows/')) return body;
+
+  // Collect [start, end) character-offset ranges covered by fenced code
+  // blocks so matches inside them are skipped. An unterminated trailing
+  // fence covers to the end of the string (still "inside a fence").
+  const lines = body.split('\n');
+  const lineStartOffsets = [];
+  {
+    let offset = 0;
+    for (const line of lines) {
+      lineStartOffsets.push(offset);
+      offset += line.length + 1; // +1 for the '\n' separator
+    }
+  }
+  const fenceRanges = scanFencedBlocks(lines).map(({ openLineIdx, closeLineIdx }) => {
+    const start = lineStartOffsets[openLineIdx];
+    const end = closeLineIdx === -1
+      ? body.length
+      : lineStartOffsets[closeLineIdx] + lines[closeLineIdx].length;
+    return [start, end];
+  });
+  const isInsideFence = (offset) => fenceRanges.some(([start, end]) => offset >= start && offset < end);
+
+  return body.replace(WORKFLOW_SPEC_ROOT_INCLUDE_RE, (match, stem, cr, offset) => {
+    if (isInsideFence(offset)) return match;
+    return (
+      `To load this command's workflow spec: check for ` +
+      `\`.claude/gsd-core/workflows/${stem}.md\` relative to the current working ` +
+      `directory first (project-local); if it is not there, fall back to ` +
+      `\`~/.claude/gsd-core/workflows/${stem}.md\` (the global install). If ` +
+      `neither file exists, stop — a workflow spec is required and none was found.` +
+      cr
+    );
+  });
+}
+
 function normalizeKimiSkillName(skillName) {
   let text = String(skillName || '').trim().toLowerCase();
   if (text.startsWith('/')) text = text.slice(1);
@@ -2995,6 +3080,38 @@ function applyRuntimeContentRewritesForCommandsInPlace(stagedDir, runtime, pathP
   return tempDir;
 }
 
+/**
+ * #2873 (4b) — second pass over a staged skills directory, run strictly AFTER
+ * `applyRuntimeContentRewritesInPlace`. That pass's `case 'claude':` branch
+ * unconditionally rewrites any bare (non-`@`-prefixed) `~/.claude/` substring
+ * in the body to the computed pathPrefix (`$HOME/.claude/` for a global
+ * install) and restores ONLY the `@`-prefixed form back to `~`
+ * (`@$HOME/.claude/` → `@~/.claude/`). `resolveSpecRootReference`'s
+ * replacement text is deliberately imperative prose containing a literal,
+ * non-`@`-prefixed `~/.claude/gsd-core/workflows/<stem>.md` — running it
+ * BEFORE the pass above would let that literal tilde text get silently
+ * mangled into the undocumented `$HOME/` form the design explicitly rejects.
+ * Running it here, after, means it only ever sees the FINAL
+ * `@~/.claude/gsd-core/workflows/<stem>.md` include line (which survives the
+ * pass above intact via its own `@`-guarded restore).
+ */
+function applySpecRootReferenceToStagedSkills(stagedDir) {
+  if (!fs.existsSync(stagedDir)) return;
+  const walk = (dir) => {
+    for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
+      const fullPath = path.join(dir, entry.name);
+      if (entry.isDirectory()) {
+        walk(fullPath);
+      } else if (entry.name === 'SKILL.md') {
+        const content = fs.readFileSync(fullPath, 'utf8');
+        const rewritten = resolveSpecRootReference(content);
+        if (rewritten !== content) fs.writeFileSync(fullPath, rewritten);
+      }
+    }
+  };
+  walk(stagedDir);
+}
+
 /**
  * HIGH-LEVEL: In-place fs walk: rewrite all .md files under stagedDir for the given runtime.
  *
@@ -3032,6 +3149,17 @@ function rewriteStagedSkillBodies(stagedDir, opts) {
   const attribution = resolveAttribution ? resolveAttribution(runtime) : undefined;
 
   applyRuntimeContentRewritesInPlace(stagedDir, runtime, pathPrefix, isGlobal, attribution);
+  // #2873 (4b): claude, global scope only — see
+  // applySpecRootReferenceToStagedSkills's doc comment for why this MUST run
+  // after the rewrite pass above, not before. `rewriteStagedSkillBodies` is
+  // the skills-kind seam (`kind.kind === 'skills'`), so this never touches a
+  // 'commands' or 'agents' kind body (rows 24/25 unaffected), and claude has
+  // no skills-kind entry at local scope, so this is already structurally
+  // scoped to global (row 23) — the explicit isGlobal check is defense-in-depth
+  // against that descriptor wiring ever changing.
+  if (runtime === 'claude' && isGlobal) {
+    applySpecRootReferenceToStagedSkills(stagedDir);
+  }
 }
 
 /**
@@ -3176,6 +3304,11 @@ export = {
   convertClaudeToAntigravityContent,
   convertClaudeCommandToAntigravitySkill,
   convertClaudeCommandToClaudeSkill,
+  // #2873 (4b): pure, scope-free transform — applied by the one call site
+  // that knows install scope (skillsKind's stage() in
+  // runtime-artifact-layout.cts), never inside convertClaudeCommandToClaudeSkill
+  // itself.
+  resolveSpecRootReference,
   convertClaudeCommandToKimiSkill,
   convertClaudeCommandToKimiCodeSkill,
   buildKimiAgentArtifacts,

src/runtime-artifact-layout.cts

@@ -395,6 +395,16 @@ function skillsKind(
       // undefined here); `isGlobalScope` projects it to the boolean
       // `realConverter`'s positional `isGlobal` arg requires.
       const isGlobal = isGlobalScope(scope);
+      // #2873 (4b): spec-root reachability is applied LATER in the pipeline —
+      // see `rewriteStagedSkillBodies` in runtime-artifact-conversion.cts, not
+      // here. This stage() closure runs BEFORE the staged directory's generic
+      // path-prefix rewrite pass (`applyRuntimeContentRewritesInPlace`'s
+      // `case 'claude'`), which unconditionally rewrites any bare (non-`@`)
+      // `~/.claude/` substring to the undocumented `$HOME/.claude/` form and
+      // only restores the `@`-prefixed form. Emitting the imperative
+      // tilde-path prose here would get silently mangled by that later pass;
+      // it must run AFTER it instead, once the `@`-include is in its final
+      // rewritten shape.
       const wrappedConverter = (content: string, skillName: string): string =>
         realConverter(content, skillName, runtime, cmdNames, isGlobal);
       return stageSkillsForRuntimeAsSkills(findInstallSourceRoot(configDir), resolved, wrappedConverter, prefix, nested, capabilityRegistry);