From 47885698894e2d269eb98963bd55990f86b87705 Mon Sep 17 00:00:00 2001 From: Yichen Jiang Date: Sun, 9 Aug 2026 22:10:38 +0800 Subject: [PATCH] feat(skill): layer the host skill registry per scope like the tools registry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The skills capability moves back to the host plane as one singleton, layered with the ScopedLayers infrastructure the tools registry established: a registration files into its calling context's layer (repository plugins and host rows land global, a preset's skill-local lands in that preset's layer), and a read merges the global layer with the viewing scope's chain, the nearest layer winning duplicate names outright while rank keeps deciding duplicates within one layer. The web-app bundle re-enables the base skill registry row (skill-local and tool-skill stay preset-owned), the standard preset drops its isolate skills realm for bare rows over the host registry, tool-skill passes the calling agent as the view scope, and the gateway's skills domain reads the host registry in the presenter scope — a cold session now resolves its recorded preset's standing key instead of failing. --- .../agent-presets/standard/agent.cordis.yml | 22 +- docs/module-graph.i18n.yaml | 4 +- docs/module-graph.md | 3 +- docs/module-graph.zh.md | 5 +- packages/bundle/web-app/cordis.patch.yml | 10 +- packages/host/apiproxy/src/api-proxy.ts | 22 +- .../tests/api-proxy-agent-preset.spec.ts | 53 ++++ packages/skill/skill/package.json | 2 + packages/skill/skill/src/index.ts | 245 ++++++++++++------ packages/skill/skill/tests/skill.spec.ts | 192 ++++++++++++++ packages/skill/skill/tsconfig.json | 3 + packages/skill/tool-skill/src/index.ts | 8 +- .../skill/tool-skill/tests/tool-skill.spec.ts | 37 +++ pnpm-lock.yaml | 3 + 14 files changed, 495 insertions(+), 114 deletions(-) diff --git a/apps/cli/config/agent-presets/standard/agent.cordis.yml b/apps/cli/config/agent-presets/standard/agent.cordis.yml index aa150ab3fa..e24b7e3c69 100644 --- a/apps/cli/config/agent-presets/standard/agent.cordis.yml +++ b/apps/cli/config/agent-presets/standard/agent.cordis.yml @@ -77,20 +77,16 @@ # ── skills ────────────────────────────────────────────────────────────────── -- id: skills - name: cordis:group - group: true - isolate: - skills: true - config: - - id: skill - name: '@deepseek-ai/dsh-skill' +# The skill REGISTRY lives in the host composition and is layered per scope: +# these rows register into THIS preset's layer of it, so they need no realm. +# `skill-local` contributes local-root discovery for agents on this preset, and +# `tool-skill` gives them the catalog and loader; the merged catalog also +# carries whatever the deployment registered globally (repository plugins). +- id: skill-local + name: '@deepseek-ai/dsh-skill-local' - - id: skill-local - name: '@deepseek-ai/dsh-skill-local' - - - id: tool-skill - name: '@deepseek-ai/dsh-tool-skill' +- id: tool-skill + name: '@deepseek-ai/dsh-tool-skill' # ── goals ─────────────────────────────────────────────────────────────────── diff --git a/docs/module-graph.i18n.yaml b/docs/module-graph.i18n.yaml index 2150ba66cf..ef1760c9fe 100644 --- a/docs/module-graph.i18n.yaml +++ b/docs/module-graph.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write docs/module-graph.md -module-graph.md: e976945dffc6ad87cbc6e052790c54b14818ffe9 -module-graph.zh.md: 85485309846fe9442e377520afb6d740ee95c2be +module-graph.md: 0488f68fa6acd2c0618690504de33768d9f64781 +module-graph.zh.md: 8508d5b47d70568fa6978d24ce6bab7dcb31d63f diff --git a/docs/module-graph.md b/docs/module-graph.md index e976945dff..0488f68fa6 100644 --- a/docs/module-graph.md +++ b/docs/module-graph.md @@ -382,6 +382,7 @@ flowchart TD pkg_system_prompt --> pkg_scope pkg_skill --> pkg_invariants pkg_skill --> pkg_llm + pkg_skill --> pkg_scope pkg_web --> pkg_invariants pkg_web --> pkg_llm pkg_api_gateway --> pkg_client_connection @@ -1241,7 +1242,7 @@ flowchart TD | [`llm-pi-ai`](../packages/llm/llm-pi-ai) | `llm` | [`credentials`](../packages/credentials/credentials), [`environment`](../packages/util/environment), [`invariants`](../packages/support/invariants), [`llm`](../packages/llm/llm), [`settings`](../packages/settings/settings), [`timeout`](../packages/util/timeout) | | [`session`](../packages/core/session) | `core` | [`brand`](../packages/util/brand), [`invariants`](../packages/support/invariants), [`llm`](../packages/llm/llm), [`scope`](../packages/core/scope), [`type-meta`](../packages/typert/type-meta) | | [`system-prompt`](../packages/core/system-prompt) | `core` | [`invariants`](../packages/support/invariants), [`llm`](../packages/llm/llm), [`scope`](../packages/core/scope) | -| [`skill`](../packages/skill/skill) | `skill` | [`invariants`](../packages/support/invariants), [`llm`](../packages/llm/llm) | +| [`skill`](../packages/skill/skill) | `skill` | [`invariants`](../packages/support/invariants), [`llm`](../packages/llm/llm), [`scope`](../packages/core/scope) | | [`web`](../packages/web/web) | `web` | [`invariants`](../packages/support/invariants), [`llm`](../packages/llm/llm) | | [`api-gateway`](../packages/api/gateway) | `api` | [`client-connection`](../packages/client/connection), [`invariants`](../packages/support/invariants), [`typert-registry`](../packages/typert/registry) | | [`client-locale`](../packages/client/locale) | `client` | [`client-runtime`](../packages/client/runtime), [`client-ui-primitives`](../packages/client/ui-primitives), [`client-ui-slots`](../packages/client/ui-slots), [`invariants`](../packages/support/invariants) | diff --git a/docs/module-graph.zh.md b/docs/module-graph.zh.md index 8548530984..8508d5b47d 100644 --- a/docs/module-graph.zh.md +++ b/docs/module-graph.zh.md @@ -384,6 +384,7 @@ flowchart TD pkg_system_prompt --> pkg_scope pkg_skill --> pkg_invariants pkg_skill --> pkg_llm + pkg_skill --> pkg_scope pkg_web --> pkg_invariants pkg_web --> pkg_llm pkg_api_gateway --> pkg_client_connection @@ -1195,7 +1196,7 @@ flowchart TD pkg_acp_demo --> pkg_workspace_context ``` -| 包 | 分组 | 依赖项 | +| Package | Group | Depends on | | --- | --- | --- | | [`invariants`](../packages/support/invariants) | `support` | — | | [`atomic-write`](../packages/util/atomic-write) | `util` | [`invariants`](../packages/support/invariants) | @@ -1243,7 +1244,7 @@ flowchart TD | [`llm-pi-ai`](../packages/llm/llm-pi-ai) | `llm` | [`credentials`](../packages/credentials/credentials), [`environment`](../packages/util/environment), [`invariants`](../packages/support/invariants), [`llm`](../packages/llm/llm), [`settings`](../packages/settings/settings), [`timeout`](../packages/util/timeout) | | [`session`](../packages/core/session) | `core` | [`brand`](../packages/util/brand), [`invariants`](../packages/support/invariants), [`llm`](../packages/llm/llm), [`scope`](../packages/core/scope), [`type-meta`](../packages/typert/type-meta) | | [`system-prompt`](../packages/core/system-prompt) | `core` | [`invariants`](../packages/support/invariants), [`llm`](../packages/llm/llm), [`scope`](../packages/core/scope) | -| [`skill`](../packages/skill/skill) | `skill` | [`invariants`](../packages/support/invariants), [`llm`](../packages/llm/llm) | +| [`skill`](../packages/skill/skill) | `skill` | [`invariants`](../packages/support/invariants), [`llm`](../packages/llm/llm), [`scope`](../packages/core/scope) | | [`web`](../packages/web/web) | `web` | [`invariants`](../packages/support/invariants), [`llm`](../packages/llm/llm) | | [`api-gateway`](../packages/api/gateway) | `api` | [`client-connection`](../packages/client/connection), [`invariants`](../packages/support/invariants), [`typert-registry`](../packages/typert/registry) | | [`client-locale`](../packages/client/locale) | `client` | [`client-runtime`](../packages/client/runtime), [`client-ui-primitives`](../packages/client/ui-primitives), [`client-ui-slots`](../packages/client/ui-slots), [`invariants`](../packages/support/invariants) | diff --git a/packages/bundle/web-app/cordis.patch.yml b/packages/bundle/web-app/cordis.patch.yml index 1d0dac3cf2..14a96231a3 100644 --- a/packages/bundle/web-app/cordis.patch.yml +++ b/packages/bundle/web-app/cordis.patch.yml @@ -228,8 +228,14 @@ - id: tool-str-replace-editor disabled: true -- id: skill - disabled: true +# The `skill` REGISTRY stays in the host plane. It is host+per-scope layered +# (the tools-registry shape): deployment-level providers — repository plugins, +# a host skill-local row — register into its global layer, while a preset's +# `skill-local` registers into that preset's layer, and each agent reads the +# merged catalog its scope chain selects. Only the per-agent rows move behind +# presets: the base host `skill-local` row is disabled here (presets own local +# discovery), and `tool-skill` is what a preset mounts to give its agent the +# catalog and loader at all. - id: skill-local disabled: true diff --git a/packages/host/apiproxy/src/api-proxy.ts b/packages/host/apiproxy/src/api-proxy.ts index 23d7fc2d0f..237bc348a6 100644 --- a/packages/host/apiproxy/src/api-proxy.ts +++ b/packages/host/apiproxy/src/api-proxy.ts @@ -2567,9 +2567,9 @@ export function createApiProxy(ctx: Context, defaults: ApiProxyDefaults): ApiPro }, skills: { - // Skill lookup never touches the Agent registry: the session address - // resolves to a canonical cwd from the host-resident session header, so - // listing skills cannot create or resume an agent as a side effect. + // Skill lookup never creates or resumes an agent: the session address + // resolves to a canonical cwd from the host-resident session header, and + // the view scope is the live agent or the preset's standing key. async list(request) { const { sessionId } = request.payload const session = ctx.sessions.get(sessionId) @@ -2586,12 +2586,10 @@ export function createApiProxy(ctx: Context, defaults: ApiProxyDefaults): ApiPro return err(request, { code: 'internal', message: `session "${sessionId}" has no project cwd`, details: {} }) } const cwd = session.header.cwd - // The registry is per session when a preset mounts one — a preset - // ships its own skill directory, so the catalog IS the session's — and - // that instance sits behind an `isolate` realm no host context - // resolves. Address it through the live agent; `agents.get` keeps the - // no-side-effect stance above (a cold session creates nothing and - // falls through to whatever the host composes). + // The host registry is layered per scope and serves every session. A + // composition may still realm-mount its own registry instead; that + // instance is invisible to host contexts, so address it through the + // live agent (`agents.get` keeps the no-side-effect stance above). const live = ctx.agents.get(sessionId) const presets = ctx.get('agentPresets') const scoped = live === undefined ? undefined : presets?.serviceFor(live, 'skills') @@ -2603,8 +2601,12 @@ export function createApiProxy(ctx: Context, defaults: ApiProxyDefaults): ApiPro if (skillRegistry === undefined) { return err(request, { code: 'internal', message: 'skill registry is absent: neither this session\'s agent preset nor the host composition mounts @deepseek-ai/dsh-skill', details: {} }) } + // The scope presenters resolve in — the live agent, else the recorded + // preset's standing key, else the global layer — so a cold session's + // '/' popup lists the catalog its composition actually serves. + const scope = await presenterScopeFor(sessionId, session.header) try { - const skills = (await skillRegistry.list({ cwd })).filter(isUserInvocable) + const skills = (await skillRegistry.list({ cwd, scope })).filter(isUserInvocable) return ok(request, { skills: skills.map(skill => ({ name: skill.name, diff --git a/packages/host/apiproxy/tests/api-proxy-agent-preset.spec.ts b/packages/host/apiproxy/tests/api-proxy-agent-preset.spec.ts index d8f48d8867..7517e65fd6 100644 --- a/packages/host/apiproxy/tests/api-proxy-agent-preset.spec.ts +++ b/packages/host/apiproxy/tests/api-proxy-agent-preset.spec.ts @@ -260,6 +260,59 @@ describe('a capability the session\'s preset mounts', () => { }) }) +describe('skills over the layered host registry', () => { + it('passes the live agent as the view scope to the host registry', async () => { + const { api, ctx } = await harness(['standard']) + const seen: unknown[] = [] + ctx.provide('skills', { + list: (options: { scope?: unknown }) => { + seen.push(options.scope) + return Promise.resolve([]) + }, + } as never) + await api.sessions.create(request({ sessionId: SessionId('h1'), agentPreset: 'standard' })) + + const response = await api.skills.list(request({ sessionId: SessionId('h1') })) + + expect(response.result).toMatchObject({ ok: true, value: { skills: [] } }) + expect(seen).toEqual([ctx.agents.get(SessionId('h1'))]) + }) + + it('resolves a cold session to its recorded preset standing key', async () => { + const { api, ctx } = await harness(['standard', 'core-web']) + const seen: unknown[] = [] + ctx.provide('skills', { + list: (options: { scope?: unknown }) => { + seen.push(options.scope) + return Promise.resolve([]) + }, + } as never) + ctx.sessions.create(SessionId('h2'), { meta: { cwd: '/workspace/cold', agentPreset: 'core-web' } }) + + const response = await api.skills.list(request({ sessionId: SessionId('h2') })) + + expect(response.result).toMatchObject({ ok: true, value: { skills: [] } }) + expect(seen).toEqual([standingKeys.get('core-web')]) + }) + + it('serves the global view when the roster no longer supplies the recorded preset', async () => { + const { api, ctx } = await harness(['standard']) + const seen: unknown[] = [] + ctx.provide('skills', { + list: (options: { scope?: unknown }) => { + seen.push(options.scope) + return Promise.resolve([]) + }, + } as never) + ctx.sessions.create(SessionId('h3'), { meta: { cwd: '/workspace/cold', agentPreset: 'gone' } }) + + const response = await api.skills.list(request({ sessionId: SessionId('h3') })) + + expect(response.result).toMatchObject({ ok: true, value: { skills: [] } }) + expect(seen).toEqual([undefined]) + }) +}) + describe('session.history presenter scope', () => { it('asks the roster for the RECORDED preset\'s standing key on a cold read', async () => { const { api } = await harness(['standard', 'core-web']) diff --git a/packages/skill/skill/package.json b/packages/skill/skill/package.json index f77f56f6d1..da51610ec3 100644 --- a/packages/skill/skill/package.json +++ b/packages/skill/skill/package.json @@ -27,6 +27,7 @@ "peerDependencies": { "@deepseek-ai/dsh-invariants": "^0.0.1", "@deepseek-ai/dsh-llm": "^0.0.1", + "@deepseek-ai/dsh-scope": "^0.0.1", "cordis": "^4.0.0-rc.7" }, "dependencies": { @@ -35,6 +36,7 @@ "devDependencies": { "@deepseek-ai/dsh-invariants": "workspace:^", "@deepseek-ai/dsh-llm": "workspace:^", + "@deepseek-ai/dsh-scope": "workspace:^", "cordis": "^4.0.0-rc.7" } } diff --git a/packages/skill/skill/src/index.ts b/packages/skill/skill/src/index.ts index 18c878d74b..c013933547 100644 --- a/packages/skill/skill/src/index.ts +++ b/packages/skill/skill/src/index.ts @@ -12,6 +12,8 @@ import { Context, Service } from 'cordis' import { assertNever } from '@deepseek-ai/dsh-llm' +import { NamedEntries, ScopedLayers, scopeChainOf, scopeOf } from '@deepseek-ai/dsh-scope' +import type { ScopeKey, ScopeLayer } from '@deepseek-ai/dsh-scope' import z from 'schemastery' import type Schema from 'schemastery' @@ -106,6 +108,17 @@ export interface SkillLookupOptions { readonly signal?: AbortSignal | undefined } +/** + * Registry read options: provider lookup context plus the viewing scope. + * The registry consumes `scope` to select layers; providers receive the same + * borrowed options object and read only their {@link SkillLookupOptions} + * contract from it. + */ +export interface SkillViewOptions extends SkillLookupOptions { + /** Viewing scope (the calling agent); omitted reads the global layer alone. */ + readonly scope?: ScopeKey | undefined +} + /** * Return whether a skill may be advertised to and loaded by a model. * @param skill - skill metadata carrying resolved invocation controls. @@ -290,17 +303,56 @@ interface IndexedCandidate { provider: SkillProvider providerOrder: number localOrder: number + /** Owning layer, so a stale-definition invalidation can verify the exact registration is still live. */ + layer: SkillLayer } -interface CollectResult { +/** One provider registration retained by its layer. */ +interface RegisteredProvider { + provider: SkillProvider + /** Service-wide monotonic registration order, the within-layer rank tiebreak. */ + order: number +} + +interface LayerCollectResult { entries: IndexedCandidate[] cacheable: boolean } +interface CollectResult { + entries: Map + cacheable: boolean +} + +/** One scope's complete skill-registry contribution. */ +class SkillLayer implements ScopeLayer { + /** Providers registered through contexts carrying this scope, insertion-ordered. */ + readonly providers: NamedEntries + /** Runtime skills registered through contexts carrying this scope. */ + readonly runtime = new Map() + + constructor(scope: ScopeKey | undefined) { + this.providers = new NamedEntries(name => new Error(scope === undefined + ? `a skill provider named "${name}" is already registered` + : `a skill provider named "${name}" is already registered in this scope`)) + } + + /** Whether every contribution table in this aggregate layer is empty. */ + isEmpty(): boolean { + return this.providers.isEmpty() && this.runtime.size === 0 + } +} + /** - * Registry of skill providers. It merges provider catalogs with stable - * first-wins duplicate handling, exposes sorted invocation-neutral summaries, and - * loads full skill bodies on demand. + * Layered registry of skill providers, the host+per-scope shape the tools + * registry established. A registration files into the layer of its calling + * context's scope ({@link scopeOf}): host rows and repository plugins land in + * the global layer, while a plugin mounted by an agent preset's standing + * composition lands in that preset's layer. A read merges the global layer + * with the viewing scope's chain — the nearest layer's entry wins a duplicate + * name outright, and the rank order decides duplicates only within one layer. + * It exposes sorted invocation-neutral summaries and loads full skill bodies + * on demand. */ export class SkillService extends Service { static Config: Schema = z.object({ @@ -308,12 +360,16 @@ export class SkillService extends Service { }) private readonly collectCacheMaxEntries: number - private readonly providers = new Map() - private readonly runtime = new Map() - private readonly collectCache = new Map() - private providerRevision = 0 + private readonly layers = new ScopedLayers( + scope => new SkillLayer(scope), + () => { this.invalidateCache() }, + ) + private readonly collectCache = new Map>() + private revision = 0 private nextProviderOrder = 0 - private runtimeRevision = 0 + /** Stable identities for cache keys; scope keys are opaque identity-compared objects. */ + private readonly scopeIds = new WeakMap() + private nextScopeId = 1 constructor(ctx: Context, config: Config = {}) { super(ctx, 'skills') @@ -322,21 +378,27 @@ export class SkillService extends Service { } /** - * Register a borrowed same-process provider synchronously during plugin apply. Duplicate and - * reserved names throw; remote initialization belongs in `list()`. Fiber disposal unregisters - * the provider and invalidates catalog caches. + * Register a borrowed same-process provider synchronously during plugin + * apply, into the calling context's layer: a scoped context (an agent + * preset's standing mount) registers for that scope alone, an unscoped + * context registers globally. Duplicate names within one layer and reserved + * names throw; remote initialization belongs in `list()`. Fiber disposal + * unregisters the provider and invalidates catalog caches. * @param create - synchronous factory receiving this registration's lifecycle and invalidation control. * @returns the exact Cordis effect disposer that unregisters this provider; * composite effects may yield it directly to preserve teardown ordering. */ registerProvider(create: (control: SkillProviderControl) => SkillProvider): () => void { const lifecycle = new AbortController() - let active = false + let registration: { layer: SkillLayer; name: string } | undefined let provider: SkillProvider const control: SkillProviderControl = { signal: lifecycle.signal, invalidate: () => { - if (active) this.invalidateProvider(provider) + const active = registration + if (active !== undefined && active.layer.providers.get(active.name)?.provider === provider) { + this.invalidateCache() + } }, } try { @@ -345,26 +407,21 @@ export class SkillService extends Service { if (name === RUNTIME_PROVIDER) { throw new Error(`"${RUNTIME_PROVIDER}" is reserved for runtime skill registrations`) } - if (this.providers.has(name)) { - throw new Error(`a skill provider named "${name}" is already registered`) - } - const providers = this.providers const order = this.nextProviderOrder - const invalidateCache = (): void => { this.invalidateCache() } this.nextProviderOrder += 1 - const dispose = this.ctx.effect(function* () { - active = true - providers.set(name, { provider, order }) - invalidateCache() - yield () => { - active = false - providers.delete(name) - lifecycle.abort(new Error(`skill provider "${name}" disposed`)) - invalidateCache() - } - }, 'skills.registerProvider()') - // oxlint-disable-next-line typescript/no-misused-promises -- synchronous cleanup; preserve exact disposer identity - return dispose + return this.layers.effect( + this.ctx, + (layer) => { + const undo = layer.providers.insert(name, { provider, order }) + registration = { layer, name } + return () => { + registration = undefined + undo() + lifecycle.abort(new Error(`skill provider "${name}" disposed`)) + } + }, + { label: 'skills.registerProvider()' }, + ) } catch (error) { lifecycle.abort(error) throw error @@ -372,16 +429,19 @@ export class SkillService extends Service { } /** - * Register a borrowed readonly runtime skill. Project entries outrank runtime entries, which - * outrank user entries. Same-name runtime entries are first-wins; a duplicate logs a warning and - * receives a no-op disposer so it cannot remove the winner. + * Register a borrowed readonly runtime skill into the calling context's + * layer. Project entries outrank runtime entries, which outrank user + * entries, within one layer. Same-name runtime entries in one layer are + * first-wins; a duplicate logs a warning and receives a no-op disposer so + * it cannot remove the winner. * @param skill - the skill definition input; omitted invocation and provider fields receive defaults. * @returns the exact Cordis effect disposer, preserving composite teardown order and invalidating caches. */ register(skill: SkillRegistration): () => void { validateRuntimeSkill(skill) - const existing = this.runtime.get(skill.name) - if (existing !== undefined) { + const scope = scopeOf(this.ctx) + const existingLayer = scope === undefined ? this.layers.global : this.layers.peek(scope) + if (existingLayer !== undefined && existingLayer.runtime.has(skill.name)) { this.ctx.logger.warn(`runtime skill "${skill.name}" ignored because it is already registered`) return () => {} } @@ -390,21 +450,14 @@ export class SkillService extends Service { invocation: skill.invocation ?? { modelInvocable: true, userInvocable: true }, provider: skill.provider ?? RUNTIME_PROVIDER, } - const runtime = this.runtime - const updateRevision = (): void => { this.runtimeRevision += 1 } - const invalidateCache = (): void => { this.invalidateCache() } - const dispose = this.ctx.effect(function* () { - runtime.set(definition.name, definition) - updateRevision() - invalidateCache() - yield () => { - runtime.delete(definition.name) - updateRevision() - invalidateCache() - } - }, 'skills.register()') - // oxlint-disable-next-line typescript/no-misused-promises -- synchronous cleanup; direct return preserves disposer identity - return dispose + return this.layers.effect( + this.ctx, + (layer) => { + layer.runtime.set(definition.name, definition) + return () => { layer.runtime.delete(definition.name) } + }, + { label: 'skills.register()' }, + ) } /** @@ -412,10 +465,10 @@ export class SkillService extends Service { * model or user invocation policy at their operational boundary. Lookup * options and provider candidates are readonly same-process values borrowed * throughout discovery. - * @param options - lookup options; `cwd` selects project roots and `signal` cancels discovery. + * @param options - view options; `scope` selects the viewing agent's layers, `cwd` selects project roots, and `signal` cancels discovery. * @returns all sorted winning summaries. */ - async list(options: SkillLookupOptions = {}): Promise { + async list(options: SkillViewOptions = {}): Promise { return (await this.snapshot(options)).skills } @@ -423,15 +476,14 @@ export class SkillService extends Service { * Observe the current invocation-neutral catalog and whether discovery completed within a stable revision. * Incomplete observations are never cached, allowing consumers to retain last-good state and * retry on their next request boundary. - * @param options - lookup options; `cwd` selects project roots and `signal` cancels discovery. + * @param options - view options; `scope` selects the viewing agent's layers, `cwd` selects project roots, and `signal` cancels discovery. * @returns sorted summaries plus discovery-completeness state. */ - async snapshot(options: SkillLookupOptions = {}): Promise { + async snapshot(options: SkillViewOptions = {}): Promise { const collected = await this.collect(options) return { - skills: collected.entries - .map(entry => entry.candidate) - .map(toSummary) + skills: [...collected.entries.values()] + .map(entry => toSummary(entry.candidate)) .sort(compareSkillSummary), complete: collected.cacheable, } @@ -442,14 +494,15 @@ export class SkillService extends Service { * provider. Cancellation is rechecked after selection, including cache hits, and raced against * loading so an uncooperative provider cannot hang the caller. * @param name - kebab-case skill name. - * @param options - lookup options; `cwd` selects workspace-sensitive skills and `signal` cancels work. + * @param options - view options; `scope` selects the viewing agent's layers, + * `cwd` selects workspace-sensitive skills, and `signal` cancels work. * @returns the full skill, including body content, or `undefined`. */ - async get(name: string, options: SkillLookupOptions = {}): Promise { + async get(name: string, options: SkillViewOptions = {}): Promise { if (!isSkillName(name)) return undefined const collected = await this.collect(options) throwIfAborted(options.signal) - const match = collected.entries.find(entry => entry.candidate.name === name) + const match = collected.entries.get(name) if (match === undefined) return undefined const definition = await waitWithAbort( match.provider.get(match.candidate, options), @@ -458,25 +511,27 @@ export class SkillService extends Service { if (definition === undefined) return undefined validateDefinition(definition) if (definition.name !== match.candidate.name) { - this.invalidateProvider(match.provider) + this.invalidateEntry(match) return undefined } return definition } - private async collect(options: SkillLookupOptions): Promise { + private async collect(options: SkillViewOptions): Promise { throwIfAborted(options.signal) let attempt = 1 while (true) { - const providerRevision = this.providerRevision - const runtimeRevision = this.runtimeRevision - const key = collectCacheKey(options, providerRevision, runtimeRevision) + const revision = this.revision + // The chain is part of the key rather than assumed stable: a blank-session + // recompose re-parents an existing scope without touching this registry, + // and only a chain-bearing key makes the next read see the new preset. + const key = this.collectCacheKey(options.cwd, scopeChainOf(options.scope), revision) const cached = this.collectCache.get(key) if (cached !== undefined) return { entries: cached, cacheable: true } const result = await this.collectFresh(options) throwIfAborted(options.signal) - if (providerRevision !== this.providerRevision || runtimeRevision !== this.runtimeRevision) { + if (revision !== this.revision) { if (attempt < MAX_COLLECT_ATTEMPTS) { attempt += 1 continue @@ -494,8 +549,24 @@ export class SkillService extends Service { } } - private async collectFresh(options: SkillLookupOptions): Promise { - const collected = await this.listAllCandidates(options) + private async collectFresh(options: SkillViewOptions): Promise { + // Global first, then existing chain overlays farthest ancestor first and + // the exact scope last, so the nearest layer's same-name entry replaces + // the farther ones — the tools registry's shadowing rule. Rank decides + // duplicates only within one layer. + const layers = [this.layers.global, ...this.layers.chainLayers(options.scope)] + const merged = new Map() + let cacheable = true + 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) + } + return { entries: merged, cacheable } + } + + private async collectLayer(layer: SkillLayer, options: SkillLookupOptions): Promise { + const collected = await this.listLayerCandidates(layer, options) collected.entries.sort(compareIndexedCandidates) const seen = new Set() const result: IndexedCandidate[] = [] @@ -511,21 +582,22 @@ export class SkillService extends Service { return { entries: result, cacheable: collected.cacheable } } - private async listAllCandidates(options: SkillLookupOptions): Promise { + private async listLayerCandidates(layer: SkillLayer, options: SkillLookupOptions): Promise { throwIfAborted(options.signal) const candidates: IndexedCandidate[] = [] let cacheable = true let runtimeOrder = 0 - for (const skill of [...this.runtime.values()].sort((a, b) => compareCodePoints(a.name, b.name))) { + for (const skill of [...layer.runtime.values()].sort((a, b) => compareCodePoints(a.name, b.name))) { candidates.push({ candidate: runtimeCandidate(skill), provider: RUNTIME_SKILL_PROVIDER, providerOrder: -1, localOrder: runtimeOrder, + layer, }) runtimeOrder += 1 } - for (const { provider, order } of [...this.providers.values()]) { + for (const { provider, order } of [...layer.providers.values()]) { let localOrder = 0 let output: unknown try { @@ -540,7 +612,7 @@ export class SkillService extends Service { if (!observation.complete) cacheable = false for (const candidate of observation.candidates) { validateCandidate(candidate, provider.name) - candidates.push({ candidate, provider, providerOrder: order, localOrder }) + candidates.push({ candidate, provider, providerOrder: order, localOrder, layer }) localOrder += 1 } } @@ -548,14 +620,29 @@ export class SkillService extends Service { } private invalidateCache(): void { - this.providerRevision += 1 + this.revision += 1 this.collectCache.clear() this.notifyChange() } - private invalidateProvider(provider: SkillProvider): void { + /** Invalidate after a stale definition load, only while the exact registration that produced the entry is still live. */ + private invalidateEntry(entry: IndexedCandidate): void { /* v8 ignore else -- A definition load can outlive the exact provider registration it selected. */ - if (this.providers.get(provider.name)?.provider === provider) this.invalidateCache() + if (entry.layer.providers.get(entry.provider.name)?.provider === entry.provider) this.invalidateCache() + } + + private scopeId(key: ScopeKey): number { + let id = this.scopeIds.get(key) + if (id === undefined) { + id = this.nextScopeId + this.nextScopeId += 1 + this.scopeIds.set(key, id) + } + return id + } + + private collectCacheKey(cwd: string | undefined, chain: ScopeKey[], revision: number): string { + return JSON.stringify({ cwd, scopes: chain.map(key => this.scopeId(key)), revision }) } /** Notify catalog observers without making their refresh work load-bearing. */ @@ -729,10 +816,6 @@ function assertPositiveInteger(name: string, value: number, minimum = 1): void { } } -function collectCacheKey(options: SkillLookupOptions, providerRevision: number, runtimeRevision: number): string { - return JSON.stringify({ cwd: options.cwd, providerRevision, runtimeRevision }) -} - function waitWithAbort(promise: Promise, signal: AbortSignal | undefined): Promise { if (signal === undefined) return promise throwIfAborted(signal) diff --git a/packages/skill/skill/tests/skill.spec.ts b/packages/skill/skill/tests/skill.spec.ts index d48263cfe0..103e34166c 100644 --- a/packages/skill/skill/tests/skill.spec.ts +++ b/packages/skill/skill/tests/skill.spec.ts @@ -1,5 +1,6 @@ import { describe, expect, it, vi } from 'vitest' import { Context } from 'cordis' +import { createScope, scopeOf, setScopeParent } from '@deepseek-ai/dsh-scope' import SkillService, { isModelInvocable, isUserInvocable, @@ -49,6 +50,13 @@ function registerProvider(ctx: Context, provider: SkillProvider): () => void { return ctx.skills.registerProvider(() => provider) } +/** The skills service as a scoped caller resolves it (scope contexts declare no inject). */ +function scopedSkills(ctx: Context): SkillService { + const skills = ctx.get('skills') + if (skills === undefined) throw new Error('skills service missing') + return skills +} + describe('SkillService registry', () => { it('registers providers, resolves duplicates first-wins, and disposes providers', async () => { const ctx = new Context() @@ -894,6 +902,26 @@ describe('SkillService registry', () => { await expect(ctx.skills.get('vanished-skill')).resolves.toBeUndefined() }) + it('propagates a load failure raced against an armed abort signal', async () => { + const ctx = new Context() + await ctx.plugin(SkillService) + registerProvider(ctx, { + name: 'failing-loader', + list: () => Promise.resolve([{ + name: 'failing-skill', + description: 'Failing', + invocation: { modelInvocable: true, userInvocable: true }, + provider: 'failing-loader', + source: 'test', + rank: 10, + locator: 'failing', + }]), + get: () => Promise.reject(new Error('load failed')), + }) + const controller = new AbortController() + await expect(ctx.skills.get('failing-skill', { signal: controller.signal })).rejects.toThrow('load failed') + }) + it('contains a provider rejection whose string coercion throws', async () => { const ctx = new Context() await ctx.plugin(SkillService) @@ -1076,3 +1104,167 @@ describe('renderSkillContent', () => { expect(text).toContain('Keep and as-is.') }) }) + +describe('SkillService scoped layers', () => { + it('files a scoped provider into its layer and merges it into that scope view only', async () => { + const ctx = new Context() + await ctx.plugin(SkillService) + registerProvider(ctx, new MemoryProvider([memorySkill('global-skill', 'Global', 100)])) + const preset = createScope(ctx, { preset: 'a' }) + const presetProvider: SkillProvider = { + name: 'preset-local', + async list() { + return [{ + name: 'preset-skill', + description: 'Preset', + invocation: { modelInvocable: true, userInvocable: true }, + provider: 'preset-local', + source: 'preset', + rank: 300, + locator: { content: 'Preset body.' }, + }] + }, + async get(candidate) { + return { ...candidate, content: (candidate.locator as { content: string }).content } + }, + } + scopedSkills(preset.ctx).registerProvider(() => presetProvider) + + expect((await ctx.skills.list()).map(skill => skill.name)).toEqual(['global-skill']) + const scoped = await ctx.skills.list({ scope: scopeOf(preset.ctx) }) + expect(scoped.map(skill => skill.name)).toEqual(['global-skill', 'preset-skill']) + expect((await ctx.skills.get('preset-skill', { scope: scopeOf(preset.ctx) }))?.content).toBe('Preset body.') + expect(await ctx.skills.get('preset-skill')).toBeUndefined() + await preset.dispose() + }) + + it('lets the nearest layer win a duplicate name regardless of rank', async () => { + const ctx = new Context() + await ctx.plugin(SkillService) + registerProvider(ctx, new MemoryProvider([memorySkill('shared-name', 'Global wins ranks', 10)])) + const preset = createScope(ctx, { preset: 'shadow' }) + scopedSkills(preset.ctx).registerProvider(() => ({ + name: 'preset-local', + async list() { + return [{ + name: 'shared-name', + description: 'Preset shadow', + invocation: { modelInvocable: true, userInvocable: true }, + provider: 'preset-local', + source: 'preset', + rank: 900, + locator: { content: 'Preset shadow body.' }, + }] + }, + async get(candidate: SkillCandidate) { + return { ...candidate, content: (candidate.locator as { content: string }).content } + }, + })) + + const scoped = await ctx.skills.list({ scope: scopeOf(preset.ctx) }) + expect(scoped).toHaveLength(1) + expect(scoped[0]?.description).toBe('Preset shadow') + expect((await ctx.skills.get('shared-name', { scope: scopeOf(preset.ctx) }))?.content).toBe('Preset shadow body.') + expect((await ctx.skills.list())[0]?.description).toBe('Global wins ranks') + await preset.dispose() + }) + + it('resolves the scope chain so an agent key inherits its preset layer and recompose follows the new parent', async () => { + const ctx = new Context() + await ctx.plugin(SkillService) + const presetA = createScope(ctx, { preset: 'a' }) + const presetB = createScope(ctx, { preset: 'b' }) + for (const [scope, label] of [[presetA, 'a'], [presetB, 'b']] as const) { + scopedSkills(scope.ctx).register({ + name: `skill-${label}`, + description: `Skill ${label}`, + source: 'preset', + content: `Body ${label}.`, + }) + } + const agentKey = {} + setScopeParent(agentKey, scopeOf(presetA.ctx) as object) + expect((await ctx.skills.list({ scope: agentKey })).map(skill => skill.name)).toEqual(['skill-a']) + // A blank-session recompose re-parents the same key without any registry write. + setScopeParent(agentKey, scopeOf(presetB.ctx) as object) + expect((await ctx.skills.list({ scope: agentKey })).map(skill => skill.name)).toEqual(['skill-b']) + await presetA.dispose() + await presetB.dispose() + }) + + it('scopes provider-name uniqueness per layer and reports scoped duplicates distinctly', async () => { + const ctx = new Context() + await ctx.plugin(SkillService) + registerProvider(ctx, new MemoryProvider([])) + const presetA = createScope(ctx, { preset: 'a' }) + const presetB = createScope(ctx, { preset: 'b' }) + scopedSkills(presetA.ctx).registerProvider(() => new MemoryProvider([memorySkill('a-only', 'A', 100)])) + scopedSkills(presetB.ctx).registerProvider(() => new MemoryProvider([memorySkill('b-only', 'B', 100)])) + expect(() => scopedSkills(presetA.ctx).registerProvider(() => new MemoryProvider([]))) + .toThrow('a skill provider named "memory" is already registered in this scope') + expect((await ctx.skills.list({ scope: scopeOf(presetA.ctx) })).map(skill => skill.name)).toEqual(['a-only']) + expect((await ctx.skills.list({ scope: scopeOf(presetB.ctx) })).map(skill => skill.name)).toEqual(['b-only']) + await presetA.dispose() + await presetB.dispose() + }) + + it('keeps runtime duplicate handling per layer and shadows a global runtime name', async () => { + const ctx = new Context() + await ctx.plugin(SkillService) + const warn = vi.fn() + ctx.logger.warn = warn as never + ctx.skills.register({ name: 'told-twice', description: 'Global runtime', source: 'runtime', content: 'Global body.' }) + const preset = createScope(ctx, { preset: 'runtime' }) + const disposeShadow = scopedSkills(preset.ctx).register({ + name: 'told-twice', + description: 'Preset runtime', + source: 'preset', + content: 'Preset body.', + }) + expect(warn).not.toHaveBeenCalled() + scopedSkills(preset.ctx).register({ name: 'told-twice', description: 'Ignored', source: 'preset', content: 'Ignored.' }) + expect(warn).toHaveBeenCalledWith('runtime skill "told-twice" ignored because it is already registered') + expect((await ctx.skills.get('told-twice', { scope: scopeOf(preset.ctx) }))?.content).toBe('Preset body.') + expect((await ctx.skills.get('told-twice'))?.content).toBe('Global body.') + disposeShadow() + expect((await ctx.skills.get('told-twice', { scope: scopeOf(preset.ctx) }))?.content).toBe('Global body.') + await preset.dispose() + }) + + it('drops a disposed scoped registration from its scope view and notifies change', async () => { + const ctx = new Context() + await ctx.plugin(SkillService) + const changes = vi.fn() + ctx.on('skills/change', changes) + const preset = createScope(ctx, { preset: 'hmr' }) + const provider = new MemoryProvider([memorySkill('scoped-skill', 'Scoped', 100)]) + scopedSkills(preset.ctx).registerProvider(() => provider) + expect((await ctx.skills.list({ scope: scopeOf(preset.ctx) })).map(skill => skill.name)).toEqual(['scoped-skill']) + const notified = changes.mock.calls.length + await preset.dispose() + expect(changes.mock.calls.length).toBeGreaterThan(notified) + expect(await ctx.skills.list({ scope: scopeOf(preset.ctx) })).toEqual([]) + }) + + it('invalidates through a scoped provider control only while its exact registration is live', async () => { + const ctx = new Context() + await ctx.plugin(SkillService) + const preset = createScope(ctx, { preset: 'invalidate' }) + const provider = new MemoryProvider([memorySkill('watched', 'Watched', 100)]) + let control: { invalidate: () => void } | undefined + const dispose = scopedSkills(preset.ctx).registerProvider((given) => { + control = given + return provider + }) + const scope = scopeOf(preset.ctx) + expect((await ctx.skills.list({ scope })).map(skill => skill.name)).toEqual(['watched']) + provider.replace([memorySkill('replaced', 'Replaced', 100)]) + control?.invalidate() + expect((await ctx.skills.list({ scope })).map(skill => skill.name)).toEqual(['replaced']) + dispose() + provider.replace([memorySkill('ignored', 'Ignored', 100)]) + control?.invalidate() + expect(await ctx.skills.list({ scope })).toEqual([]) + await preset.dispose() + }) +}) diff --git a/packages/skill/skill/tsconfig.json b/packages/skill/skill/tsconfig.json index 82e62d7c91..8fb99e3b59 100644 --- a/packages/skill/skill/tsconfig.json +++ b/packages/skill/skill/tsconfig.json @@ -15,6 +15,9 @@ { "path": "../../../vendor/schemastery" }, + { + "path": "../../core/scope" + }, { "path": "../../llm/llm" }, diff --git a/packages/skill/tool-skill/src/index.ts b/packages/skill/tool-skill/src/index.ts index 813f9b2d27..634fb7ce02 100644 --- a/packages/skill/tool-skill/src/index.ts +++ b/packages/skill/tool-skill/src/index.ts @@ -128,7 +128,9 @@ export function apply(ctx: Context, config: Config = {}): void { if (!isSkillName(args.name)) { throw new Error(`invalid skill name "${args.name}"`) } - const lookup = { cwd: exec.agent?.session.header.cwd, signal: exec.signal } + // The agent is its own scope key, so the lookup resolves the layered + // registry exactly as this agent's composition sees it. + const lookup = { cwd: exec.agent?.session.header.cwd, signal: exec.signal, scope: exec.agent } const summary = (await ctx.skills.list(lookup)).find(skill => skill.name === args.name) if (!summary) { throw new Error(`skill "${args.name}" is unknown or no longer available`) @@ -181,7 +183,7 @@ export function apply(ctx: Context, config: Config = {}): void { const names = invokedSkillNames(messages) if (names.length === 0) return decision signal.throwIfAborted() - const lookup = { cwd: agent.session.header.cwd, signal } + const lookup = { cwd: agent.session.header.cwd, signal, scope: agent } const injections: UserMessage[] = [] for (const name of names) { const skill = await ctx.skills.get(name, lookup) @@ -217,7 +219,7 @@ export function apply(ctx: Context, config: Config = {}): void { signal.throwIfAborted() const toolVisible = ctx.tools.get(skillTool.name, agent) === skillTool const snapshot = toolVisible - ? await ctx.skills.snapshot({ cwd: agent.session.header.cwd, signal }) + ? await ctx.skills.snapshot({ cwd: agent.session.header.cwd, signal, scope: agent }) : { skills: [], complete: true } signal.throwIfAborted() if (!snapshot.complete) return decision diff --git a/packages/skill/tool-skill/tests/tool-skill.spec.ts b/packages/skill/tool-skill/tests/tool-skill.spec.ts index 5e0fe58855..7878566ac2 100644 --- a/packages/skill/tool-skill/tests/tool-skill.spec.ts +++ b/packages/skill/tool-skill/tests/tool-skill.spec.ts @@ -640,6 +640,43 @@ describe('dsh-tool-skill', () => { expect(JSON.stringify(result.content)).not.toContain('First body.') }) + it('resolves the layered registry as the calling agent sees it', async () => { + const home = await tempDir('tool-scoped-layer') + const ctx = await setup(home) + const { agent, scope } = await mintAgentScope(ctx, '/workspace/scoped') + const scopedSkills = scope.ctx.get('skills') + if (scopedSkills === undefined) throw new Error('skills service missing') + scopedSkills.register({ + name: 'preset-only-skill', + description: 'Visible to the scoped agent alone', + source: 'preset', + content: 'Preset-only body.', + }) + + expect(JSON.stringify(await composePrefixForAgent(ctx, agent))).toContain('preset-only-skill') + expect(JSON.stringify(await composePrefix(ctx, '/workspace/other'))).not.toContain('preset-only-skill') + + const scoped = await ctx.tools.execute({ + signal: testToolSignal, + callId: CallId('scoped-load'), + name: 'skill', + arguments: { name: 'preset-only-skill' }, + agent, + }) + expect(scoped.isError).toBe(false) + expect(JSON.stringify(scoped.content)).toContain('Preset-only body.') + + const foreign = await ctx.tools.execute({ + signal: testToolSignal, + callId: CallId('foreign-load'), + name: 'skill', + arguments: { name: 'preset-only-skill' }, + agent: agentForCwd('/workspace/other'), + }) + expect(foreign.isError).toBe(true) + await scope.dispose() + }) + it('retains the last-good catalog while any provider discovery is incomplete', async () => { const home = await tempDir('tool-incomplete-catalog') const ctx = await setup(home) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9499f1ceba..6ffb34f39e 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -5815,6 +5815,9 @@ importers: '@deepseek-ai/dsh-llm': specifier: workspace:^ version: link:../../llm/llm + '@deepseek-ai/dsh-scope': + specifier: workspace:^ + version: link:../../core/scope cordis: specifier: ^4.0.0-rc.7 version: link:../../../vendor/cordis