[Feature request] Expose source on skill.list + allow listing by cwd (workspace scope)

[royenheart]

Background

I'm writing a "skills manager" plugin for dsh that enables/disables skills across three scopes — session > workspace > global. On the host side the plugin enforces "disabled" via a rank-50 skill provider; on the client side it reuses the existing skill.list RPC rather than adding a custom listing.

While building the scope filtering I hit a wall. To render the right set per panel —

Panel	Should list	Distinction needed
Global settings	Only global skills (user-dsh / user-agents)	global vs workspace
Workspace	Global + workspace skills (project-dsh / project-agents)	same
Session	Global + workspace skills	same

— the client must be able to tell a user-level skill from a project-level skill / global-level skill. Today it cannot: skill.list is session-addressed (it returns the session's project + user skills merged) and its wire rows carry no source.

Current state (why source is not on the wire)

This omission is deliberate, not accidental. SkillEntry's comment currently reads:

/** Skill catalog row (wire projection of the host SkillSummary; provider/source vocabulary stays host-side). */

and the skill subsystem docs (docs/subsystems/skills.md) state the same principle for the model-facing catalog:

The catalog contains sorted skill name and normalized, XML-escaped description only; it omits bodies, paths, sources, providers, and routing hints.

That decision is reasonable for its original purpose:

1. Minimize the exposure surface — the model's <available_skills> reminder and the /-trigger picker only need name + description to decide whether to load a skill; they don't need its origin.
2. Don't leak host-side implementation details — path / provider could expose local directories or provider identity (a privacy/trust boundary), and the source vocabulary is open (see below), so pinning it as a stable wire contract would couple consumers to every new source.

The existing client simply never needed the distinction; a scope-aware manager plugin does.

Proposal

Expose source on SkillEntry — a coarse, open string label (not a path, not a provider identity) — so feature plugins can distinguish globally-available skills from workspace-scoped ones without a second listing RPC. A companion change lets skill.list be addressed by cwd (instead of only an attached session), so a workspace-scoped panel can list that workspace's skills.

Why relaxing source is safe:

• SkillSource is itself an open string label, not a path, not a provider identity, and carries no sensitive information:

  export type SkillSource = 'project-dsh' | 'project-agents' | 'runtime' | 'user-dsh' | 'user-agents' | 'custom' | 'bundled' | (string & {})

• The trailing (string & {}) keeps it forward-compatible: a new source never breaks the wire contract; a client just sees a literal it doesn't recognize.
• It does not leak disk paths (project-agents is a category name, not the concrete <project>/.agents/skills path).

Appendix: full implementation patch

Verified in the local worktree: tsc -b tsconfig.host.json and tsc -b tsconfig.client.json both 0 errors.

packages/host/apiproxy/src/api/skills.ts:

-/** Skill catalog row (wire projection of the host SkillSummary; provider/source vocabulary stays host-side). */
+/** Skill catalog row (wire projection of the host SkillSummary). */
 export interface SkillEntry {
   /** Kebab-case identifier the user references as /name in the composer. */
   readonly name: string
   /** Short routing description. */
   readonly description: string
   /** Optional extra routing guidance. */
   readonly whenToUse?: string
   /** False marks a user-only skill (disable-model-invocation): invocable here, absent from the model catalog. */
   readonly modelInvocable: boolean
+  /**
+   * Host discovery source vocabulary (e.g. project-dsh, project-agents,
+   * user-dsh, user-agents, bundled, custom, runtime). Exposed so a feature
+   * plugin can distinguish globally-available skills from workspace-scoped
+   * ones without a second listing RPC.
+   */
+  readonly source: string
 }

packages/host/apiproxy/src/api/skills.schema.ts:

 export const skillEntrySchema = z.object({
   name: z.string().min(1),
   description: z.string(),
   whenToUse: z.string().optional(),
   modelInvocable: z.boolean(),
+  source: z.string().min(1),
 }) satisfies z.ZodType<Wire<SkillEntry>>

packages/host/apiproxy/src/api-proxy.ts:

             skills: skills.map(skill => ({
               name: skill.name,
               description: skill.description,
               ...skill.whenToUse === undefined ? {} : { whenToUse: skill.whenToUse },
               modelInvocable: skill.invocation.modelInvocable,
+              source: skill.source,
             })),

(Since source becomes required, the two test fixtures that hand-construct this wire type must add the field too: packages/client/connection/src/client/fixture.ts and packages/host/apiproxy/tests/fetch-carrier.spec.ts.)

Appendix: full implementation patch (II) — skill.list supports listing by cwd

The workspace scope has no usable "attached" session: skill.list's session-addressed path requires ctx.sessions.get() to hit a session already loaded in memory, while WorkspaceView.sessionIds is a persisted account that may contain sessions left over from a previous run and not currently attached. More importantly, the web profile sets the base host skill-filesystem row disabled: true (packages/bundle/web-app/cordis.patch.yml, whose comment says "presets own local discovery"), so skill discovery moved to each preset's own scope layer — thus simply calling skillRegistry.list() with cwd and scope: undefined cannot reach the filesystem provider and yields an empty catalog.

Minimal and general solution: relax the request from { sessionId } to { sessionId?; cwd? }, and in the cwd branch resolve the default preset's standing key as the scope:

packages/host/apiproxy/src/api/skills.ts:

 export interface SkillsApi {
-  /** Lists the user-invocable skill catalog for the session's project. */
-  list(request: RpcRequest<{ sessionId: SessionId }>): Promise<RpcResponse<{ skills: readonly SkillEntry[] }>>
+  /** Lists the user-invocable skill catalog by sessionId (resolves its cwd) or by cwd. */
+  list(request: RpcRequest<{ sessionId?: SessionId; cwd?: string }>): Promise<RpcResponse<{ skills: readonly SkillEntry[] }>>
 }

packages/host/apiproxy/src/api/skills.schema.ts:

 export const skillListRequestSchema = z.object({
-  sessionId: sessionIdSchema,
+  sessionId: sessionIdSchema.optional(),
+  cwd: z.string().min(1).optional(),
 }) satisfies z.ZodType<Wire<RequestPayload<'skill.list'>>>

packages/host/apiproxy/src/api-proxy.ts (key handler branch):

       async list(request) {
-        const { sessionId } = request.payload
-        const session = ctx.sessions.get(sessionId)
-        if (session === undefined) return err(request, { code: 'session-not-found', ... })
-        ...
-        const cwd = session.header.cwd
+        const { sessionId, cwd: directCwd } = request.payload
+        const presets = ctx.get('agentPresets')
+        let cwd: string
+        let scope: ScopeKey | undefined
+        if (directCwd !== undefined) {
+          cwd = directCwd
+          scope = undefined
+          if (presets !== undefined) {
+            try {
+              // Workspace listing has no session; use the default preset's
+              // standing key so its per-scope skill providers (filesystem) resolve.
+              scope = await presets.standingKeyFor()
+            } catch { scope = undefined }
+          }
+        } else if (sessionId !== undefined) {
+          // ... existing session resolution + presenterScopeFor ...
+        } else {
+          return err(request, { code: 'bad-request', message: 'skill.list requires sessionId or cwd', ... })
+        }
+        ...
       }

(presets.standingKeyFor() without an id returns the standing scope of defaultId; see packages/preset/agent-presets/src/index.ts.)

Verified locally: POST /api/skill.list { "payload": { "cwd": "<workspace>" } } returns that project's + user skills (with source); previously it was an empty array.

Appendix: two robustness and enforcement notes

A. A provider with an empty description drags down the whole session (suggestion: default to disable + warn, instead of throw)

ctx.skills.registerProvider's list candidates enforce description.length > 0 in validateCandidate, otherwise it throws (skill provider "…" returned skill "…" without a description), and that throw bubbles up to the agent request, breaking the whole conversation.

An enforcement provider for "disabled skills" (like this plugin) naturally returns a placeholder candidate for blocked skills — if the placeholder description is empty it triggers the crash above. A plugin can work around it by filling a non-empty description (I do), but it's more robust for the registry to degrade to "disabled" and warn for a candidate with no description rather than throw, so a typo in any third-party provider can't drag down the session.

B. A global enforcement provider is shadowed by the per-preset filesystem (web profile)

The web profile sets the base host skill-filesystem row disabled: true (packages/bundle/web-app/cordis.patch.yml), with discovery moved to each preset's own scope layer. And the ctx.skills registry merges "global layer + scope chain", where a nearer layer's same-name entry overrides a farther layer's (in collectFresh, merged.set(name, entry) runs per layer in order, and rank only decides within a single layer).

So an enforcement provider registered in the global layer (rank 50) is entirely shadowed by skill-filesystem (rank 100–600) in the preset layer — a disabled skill still appears in the catalog with modelInvocable: true. For cross-scope disabling plugins this is fatal: to enforce via a provider you'd have to register on the same layer as the filesystem (the preset layer) or a nearer layer.

General fix already adopted (make a "hard block" win across layers in collectFresh, deny wins):

   for (const layer of layers) {
     const collected = await this.collectLayer(layer, options)
     if (!collected.cacheable) cacheable = false
-    for (const entry of collected.entries) merged.set(entry.candidate.name, entry)
+    for (const entry of collected.entries) {
+      const existing = merged.get(entry.candidate.name)
+      // A fully-disabled candidate is a hard block: a nearer layer must not revive it.
+      if (existing !== undefined
+        && existing.candidate.invocation.modelInvocable === false
+        && existing.candidate.invocation.userInvocable === false) continue
+      merged.set(entry.candidate.name, entry)
+    }
   }

That is: treat "fully disabled (modelInvocable:false && userInvocable:false)" as a deny that a nearer layer must not revive; ordinary same-name replacement (shadow) still follows nearest-layer-wins. This is a first-class registry semantic, independent of any profile or specific plugin (a no-op in base/headless, restoring deny absoluteness in web). Verified locally: after disabling, skill.list no longer returns that skill, and tool-skill's snapshot().filter(isModelInvocable) also filters it out of the model catalog.

Not implemented, discussion-only alternative: make "fully disabled" an explicit SkillCandidate.blocked?: true (rather than inferring from the invocation combination modelInvocable:false && userInvocable:false). An explicit field is clearer and can't be misread as "user-only invocable (modelInvocable:false, userInvocable:true)", but it adds API surface; for now I use the combination check, keeping the minimal zero-new-API change.

Questions to confirm

1. Is resolving the default preset's standing key in the cwd branch (already merged into skill.list) appropriate? Is there a more canonical "get a scope without a session" entry point worth splitting into a separate skill.listByCwd?
2. Should source be tightened to an explicit enum (dropping the open (string & {}) tail) before going on the wire? I lean toward keeping the open string, to avoid breaking the contract with new sources.
3. If the direction works, I'd be glad to submit an implementation PR per repo conventions.

Related

• docs/subsystems/skills.md — the original design description "catalog omits bodies, paths, sources, providers, and routing hints".
• Discussion Allow third-party plugins to expose their own settings namespaces to the Web plugin-configuration section #1183 "Allow third-party plugins to expose their own settings namespaces…" — same theme of "give third-party plugins a minimal UI/interface extension surface".