fix: pack-scoped skill ids, dangling-link repair, session-scoped skills via SDK plugins

CHANGELOG.md

@@ -25,6 +25,56 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
   `.cancel`, all gated on `settings:write`. Claude is wired today; the mechanism
   is per-backend and the others follow.
 
+- **Session-scoped pack skills** (`pipeline.skillScope: "session"`). Until now
+  a trusted pack's registry skills reached a session one way only: symlinked
+  into `~/.claude/skills`, where every Claude Code session on the machine — not
+  just codeoid's — discovered them. On a machine whose `~/.claude` is owned by
+  something else (an org bundle linked by hand, another toolkit) that is the
+  wrong scope. With `skillScope: "session"` nothing is linked; codeoid
+  synthesizes a Claude-Code-plugin-shaped directory per registry
+  (`~/.codeoid/plugins/<registry>/`: a manifest plus one symlink per real skill
+  directory in the registry cache, under the same lstat guard as global
+  linking) and hands it to the SDK's `plugins` option for pack-activated
+  sessions and pipeline phases, so the methodology's skills exist inside
+  codeoid runs and nowhere else. Plugin skills resolve both bare (`/spec`) and
+  namespaced (`/<registry>:spec`), so pack `command:` values are unchanged; on a
+  bare-name collision the user- or project-tier skill wins, as it does for a
+  global link. The same trust rule applies (an untrusted pack contributes no
+  runnable skills either way); the read sandbox and the skill-command grants
+  scan the plugin tier too. The default stays `global`;
+  `CODEOID_PIPELINE_SKILL_SCOPE` sets it per invocation. Non-Claude backends
+  ignore the plugin dirs, as they already ignore pack subagents.
+  (docs/pack-loading.md §3a)
+
+### Fixed
+
+- **Two installed packs declaring the same skill or gate id overwrote each
+  other.** Registries were daemon-wide and keyed by bare id, so installing a
+  second pack that also declared `review`, `ship`, or `tests_pass` replaced the
+  first's entries last-wins (the boot log said so: `skill "review" already
+  registered — overwriting`), and a run from the first pack then drove the
+  second pack's skill. Packs now register under `<packId>/<id>`; the engine, the
+  skill phase kind, and create-time validation resolve a run's own pack entry
+  first and fall back to the bare id, so built-in gates (`always`, `manual`) and
+  directly registered entries keep resolving exactly as before. Phase defs, CLI
+  output, and the web Pack Browser still show the ids as authored.
+
+  One deliberate consequence: an explicit-`phases` plan (wire `pipeline.create`
+  with `phases`, not `pack`) can no longer borrow an installed pack's skill or
+  gate by its bare id — that borrowing was the same leakage, just from the
+  other side. Name the entry by its qualified id (`skill: "org-dev/spec"`), or
+  create the run with `pack`; the create error now lists the qualified ids that
+  exist. The web UI and CLI always send `pack`, so only direct API/SDK clients
+  are affected, and `pipeline.pack.list` now reports the daemon's `skillScope`.
+
+- **A dangling skill symlink blocked that skill from ever being linked again.**
+  An older loader linked registry skills from a temp clone under `/tmp`; after
+  the cache moved, those links pointed at nothing. `existsSync` is false for a
+  dangling link, so `#linkSkills` tried to create it, hit `EEXIST`, warned, and
+  left the skill broken on every subsequent install and trust. A dangling link
+  is now repaired in place; a real directory or a live link is still never
+  touched.
+
 ## [0.4.0] - 2026-07-29
 
 codeoid moves to the Highflame npm org. npm has no way to transfer a package

docs/pack-loading.md

@@ -47,7 +47,7 @@ pipeline runtime on. It owns:
 | `refresh(name?)` | `git pull` a cached registry |
 | `available()` | packs found across caches, not yet installed |
 | `installed()` | loaded packs + metadata + trust + selected flag + status |
-| `install(ref, { trusted })` | resolve a pack (registry `id` or explicit dir) → `loadPack` → persist to `config.pipeline.packs` → `installPack` into the live manager (if any) → link the registry's `skills/` into `~/.claude/skills` so the pack is *runnable* |
+| `install(ref, { trusted })` | resolve a pack (registry `id` or explicit dir) → `loadPack` → persist to `config.pipeline.packs` → `installPack` into the live manager (if any) → under `skillScope: global`, link the registry's `skills/` into `~/.claude/skills` so the pack is *runnable* (see §3a) |
 | `remove(id)` | unregister from the manager + drop from config |
 | `trust(id, trusted)` | update config trust + reload the pack at the new trust |
 | `select(id)` | set `config.pipeline.defaultPack` |
@@ -56,6 +56,49 @@ pipeline runtime on. It owns:
 skill/review gates work, but its shell `command` gates fail closed until an
 explicit `trust`. Matches the sandbox zero-standing-privilege posture.
 
+### 3a. Skill scope — machine-wide symlinks or per-session plugins
+
+A trusted pack's registry `skills/` can reach a session two ways, chosen by
+`config.pipeline.skillScope`:
+
+| `skillScope` | How the skills reach a session | Who else sees them |
+| --- | --- | --- |
+| `global` (default) | `#linkSkills` symlinks each `skills/<name>` into `~/.claude/skills` on install / trust / refresh (additive; a **dangling** link left by an older loader is repaired, a real dir or live link is never touched) | every Claude Code session on the machine — the user's own same-named skills win collisions |
+| `session` | nothing is linked; `resolveActivation()` returns `skillsPluginDir`, a synthesized Claude-Code-plugin dir (`~/.codeoid/plugins/<registry>/` = `.claude-plugin/plugin.json` + a real `skills/` holding one symlink per real skill directory in `<cache>/skills`) that the Claude backend passes to the SDK `plugins` option for that session's turns | only pack-activated codeoid sessions and pipeline phases |
+
+Both scopes apply the same trust rule (an untrusted pack contributes no
+runnable skills either way) and the same lstat guard (a `skills/<name>` that
+is itself a symlink in the registry is never propagated — under session scope
+a whole-dir link would have exposed it *and* widened the read sandbox to its
+real parent), and both scan the skills for their `!`…`` substitutions so the
+command grants (#233) and the read sandbox work identically. Plugin skills
+resolve both bare (`/spec`) and namespaced (`/<registry>:spec`), so pack
+`command:` values are unchanged. On a bare-name collision the user- or
+project-tier skill wins under both scopes (verified against the binary); a
+pack that wants the registry's version regardless names it
+`/<registry>:<skill>`. Stale plugin links (a skill removed on `registry
+refresh`) are pruned on the next activation; a real directory an operator
+places in the plugin's `skills/` is left alone. `CODEOID_PIPELINE_SKILL_SCOPE`
+sets the scope per invocation.
+
+`session` is the scope for a machine whose `~/.claude` is owned by something
+else (an org bundle symlinked by hand, another toolkit): the methodology's
+skills exist inside codeoid runs and nowhere else. Switching an existing
+machine from `global` to `session` does not remove links already made — delete
+the `~/.claude/skills/<name>` symlinks that point into `~/.codeoid/packs/` if
+you want them gone. Non-Claude backends ignore `pluginDirs` today, exactly as
+they ignore pack subagents.
+
+Registry skills and gates are registered under `<packId>/<id>` (`scoped.ts`)
+so two installed packs declaring the same bare id (`review`, `ship`,
+`tests_pass`) coexist; a run created from a pack resolves its own pack's entry
+first and falls back to the bare id for built-in gates (`always`, `manual`).
+An explicit-`phases` plan has no pack to scope by: it resolves built-ins and
+directly registered entries by bare id, and an installed pack's entries by
+their qualified id (`skill: "org-dev/spec"`). It can no longer borrow a pack's
+entry by bare id — the create error lists the qualified ids that exist.
+`pipeline.pack.list` reports the daemon's live `skillScope`.
+
 Persistence goes through one shared config mutator (`mutateConfigFile`) that
 read → mutates → validates against `RootSchema` → atomically writes `0o600` —
 reusing the settings-store path so config integrity is enforced in one place.

packages/protocol/src/types.ts

@@ -2464,6 +2464,11 @@ export interface PackListResultMsg {
   installed: PackWire[];
   available: AvailablePackWire[];
   registries: RegistryWire[];
+  /** How a trusted pack's registry skills reach sessions on this daemon
+   *  (`config.pipeline.skillScope`): `global` = symlinked into `~/.claude/skills`
+   *  machine-wide; `session` = exposed only inside pack-activated sessions as an
+   *  SDK plugin. Optional (additive) — absent from older daemons. */
+  skillScope?: "global" | "session";
 }
 
 // =============================================================================

src/config.ts

@@ -770,6 +770,15 @@ const PipelineSchema = z
     modelTiers: z.record(z.string().min(1).max(64), ModelBindingSchema).default({}),
     // Key = "<packId>/<roleName>" — both ids are ≤64 chars, plus the slash.
     modelRoles: z.record(z.string().min(1).max(129), ModelBindingSchema).default({}),
+    /**
+     * How a trusted pack's registry skills reach a session (docs/pack-loading.md
+     * §3a). `global` (default): symlinked into `~/.claude/skills`, so every
+     * Claude Code session on the machine sees them. `session`: never linked;
+     * exposed as a per-session SDK plugin only inside pack-activated codeoid
+     * sessions and pipeline runs — the machine-wide `~/.claude` stays whatever
+     * the operator manages by hand.
+     */
+    skillScope: z.enum(["global", "session"]).default("global"),
     packs: z
       .array(
         z.object({
@@ -794,7 +803,15 @@ const PipelineSchema = z
       )
       .default([]),
   })
-  .default({ enabled: true, defaultPack: null, packs: [], registries: [], modelTiers: {}, modelRoles: {} });
+  .default({
+    enabled: true,
+    defaultPack: null,
+    packs: [],
+    registries: [],
+    modelTiers: {},
+    modelRoles: {},
+    skillScope: "global",
+  });
 
 /**
  * Push notifications (docs/push.md). When a session blocks on a tool approval,
@@ -1070,6 +1087,10 @@ export interface CodeoidConfig {
      *  (schema default {}). */
     modelTiers?: Record<string, { provider: string; model?: string }>;
     modelRoles?: Record<string, { provider: string; model?: string }>;
+    /** `global` (default) symlinks trusted pack skills machine-wide; `session`
+     *  exposes them only inside pack-activated sessions via an SDK plugin
+     *  (docs/pack-loading.md §3a). Optional in the type; loadConfig defaults it. */
+    skillScope?: "global" | "session";
   };
   /**
    * Per-backend provider settings. Optional in the type so hand-built test
@@ -1198,6 +1219,9 @@ const ENV_OVERRIDES: readonly EnvOverride[] = [
   // without touching config.json (on by default; set false to opt out). Other
   // pipeline knobs are file-config only, matching the dispatch/conductor convention.
   { env: "CODEOID_PIPELINE_ENABLED", path: "pipeline.enabled", kind: "boolean" },
+  // Skill scope (global | session) — per-invocation, e.g. a sandbox image that
+  // must never write into ~/.claude/skills. The schema enum rejects other values.
+  { env: "CODEOID_PIPELINE_SKILL_SCOPE", path: "pipeline.skillScope", kind: "string" },
   { env: "CODEOID_FALLBACK_MODEL", path: "session.fallbackModel", kind: "string" },
   // Hooks kill switch — disable every configured hook per-invocation without
   // touching config.json. Entries themselves are file-config only.

src/daemon/collaboration.ts

@@ -496,7 +496,7 @@ export function roleChildPosture(
   child: PlannedChild,
   parentSessionId: string,
   constitution: string,
-  adopted?: { packId: string; role: RoleDef },
+  adopted?: { packId: string; role: RoleDef; skillsPluginDir?: string },
 ): {
   role: "worker";
   workerShape: "ship" | "scout";
@@ -512,6 +512,10 @@ export function roleChildPosture(
     };
     roleName: string;
     subagents: never[];
+    /** Session-scoped pack skills (`pipeline.skillScope: "session"`), carried
+     *  from the adopted pack so a child can run the methodology's slash skills
+     *  exactly as it could under a global-scope link. */
+    skillsPluginDir?: string;
   };
   collaborationRole: {
     parentSessionId: string;
@@ -555,6 +559,7 @@ export function roleChildPosture(
           },
       roleName: child.roleName,
       subagents: [],
+      ...(adopted?.skillsPluginDir ? { skillsPluginDir: adopted.skillsPluginDir } : {}),
     },
     collaborationRole: {
       parentSessionId,

src/daemon/pipeline/engine.ts

@@ -22,6 +22,7 @@ import type {
 } from "./interface";
 import { isTerminal } from "./interface";
 import { errMessage } from "./errors";
+import { resolveScoped } from "./scoped";
 
 /** Defensive cap against a mis-authored retry loop (each retry is one step). */
 const MAX_STEPS = 10_000;
@@ -199,7 +200,9 @@ export class PipelineEngine {
     phase: PhaseDef,
     at: "entry" | "exit",
   ): Promise<GateVerdict> {
-    const g = this.#registries.gates.resolve(id);
+    // The run's own pack entry first (`<packId>/<id>`), then a bare built-in
+    // (`always` / `manual`) or directly registered gate — see scoped.ts.
+    const g = resolveScoped(this.#registries.gates, pipeline.packId, id);
     if (!g) return { pass: false, reason: `unknown ${at} gate "${id}"` };
     try {
       return await g.evaluate({ pipeline: clone(pipeline), phase });

src/daemon/pipeline/manager.ts

@@ -18,10 +18,11 @@ import { resolveModelIdForProvider } from "../models.js";
 import { type ModelBinding, type ModelBindingConfig, resolveBinding } from "./binding";
 import { registerBuiltins } from "./builtin";
 import { PipelineEngine } from "./engine";
-import type { Pack, PhaseDef, PipelineRegistries, PipelineState } from "./interface";
+import type { Pack, PhaseDef, PipelineRegistries, PipelineState, Registry } from "./interface";
 import { isTerminal } from "./interface";
 import { createRegistries } from "./registry";
 import type { PhaseRunner } from "./runner";
+import { hasScoped, scopedMatches } from "./scoped";
 import { makeSkillPhaseKind } from "./skill-kind";
 import type { PipelineStore } from "./store";
 
@@ -106,7 +107,7 @@ export class PipelineManager {
   create(opts: CreatePipelineOpts): PipelineState {
     const { phases: plan, pack } = this.#resolvePhases(opts);
     const phases = this.#bindModels(plan, pack, opts);
-    this.#validate(phases);
+    this.#validate(phases, pack?.id);
     const ts = Date.now();
     const state: PipelineState = {
       id: randomUUID(),
@@ -427,25 +428,37 @@ export class PipelineManager {
     });
   }
 
-  #validate(phases: PhaseDef[]): void {
+  /** `packId` scopes gate/skill lookups to the pack the plan came from
+   *  (`<packId>/<id>` first, bare second — scoped.ts); absent for explicit
+   *  plans, which resolve built-ins and directly registered entries by bare id
+   *  and an installed pack's entries by their qualified `<packId>/<id>`. An
+   *  explicit plan naming a pack's entry by bare id is told the qualified ids
+   *  that exist rather than a flat "unknown". */
+  #validate(phases: PhaseDef[], packId?: string): void {
     if (phases.length === 0) throw new Error("pipeline must declare at least one phase");
     const seen = new Set<string>();
+    const unknown = <T extends { id: string }>(what: string, reg: Registry<T>, id: string): string => {
+      const hint = packId ? [] : scopedMatches(reg, id);
+      return hint.length > 0
+        ? `unknown ${what} "${id}" — installed packs declare it as ${hint.map((h) => `"${h}"`).join(", ")}; name it that way, or create the run with \`pack\``
+        : `unknown ${what} "${id}"`;
+    };
     for (const p of phases) {
       if (seen.has(p.id)) throw new Error(`duplicate phase id "${p.id}"`);
       seen.add(p.id);
       if (!this.#registries.phases.has(p.kind)) {
         throw new Error(`phase "${p.id}": unknown kind "${p.kind}"`);
       }
-      if (p.gate && !this.#registries.gates.has(p.gate)) {
-        throw new Error(`phase "${p.id}": unknown gate "${p.gate}"`);
+      if (p.gate && !hasScoped(this.#registries.gates, packId, p.gate)) {
+        throw new Error(`phase "${p.id}": ${unknown("gate", this.#registries.gates, p.gate)}`);
       }
-      if (p.entryGate && !this.#registries.gates.has(p.entryGate)) {
-        throw new Error(`phase "${p.id}": unknown entry gate "${p.entryGate}"`);
+      if (p.entryGate && !hasScoped(this.#registries.gates, packId, p.entryGate)) {
+        throw new Error(`phase "${p.id}": ${unknown("entry gate", this.#registries.gates, p.entryGate)}`);
       }
       if (p.kind === "skill") {
         if (!p.skill) throw new Error(`phase "${p.id}": kind "skill" requires a skill id`);
-        if (!this.#registries.skills.has(p.skill)) {
-          throw new Error(`phase "${p.id}": unknown skill "${p.skill}"`);
+        if (!hasScoped(this.#registries.skills, packId, p.skill)) {
+          throw new Error(`phase "${p.id}": ${unknown("skill", this.#registries.skills, p.skill)}`);
         }
       }
     }