From f27a0dcfb5f91d5185d7fe7f9186c64fe29cbd6b Mon Sep 17 00:00:00 2001 From: Coder Date: Thu, 27 Aug 2026 14:03:40 +0700 Subject: [PATCH] fix(cordis-host-runner): make inspect provider registration idempotent on identical manifest The Host cordisInspect registry threw on any duplicate provider id, but tool-cordis is mounted per-preset (standing scope), so two presets that both include it (e.g. `maximum` and a user copy such as `design`) collide. `register()` now replaces the stored entry when the incoming manifest is structurally identical (same id, description, and methods); a provider with the same id but a different manifest still throws as before. Added `jsonEqual` and `sameManifest` module-private helpers; new spec covers first register, identical re-register, different-manifest reject, empty-id validation, and method-array mismatch. --- .../src/inspect-registry.ts | 40 ++++++++- .../tests/inspect-registry.spec.ts | 86 +++++++++++++++++++ 2 files changed, 125 insertions(+), 1 deletion(-) create mode 100644 packages/extensions/cordis-host-runner/tests/inspect-registry.spec.ts diff --git a/packages/extensions/cordis-host-runner/src/inspect-registry.ts b/packages/extensions/cordis-host-runner/src/inspect-registry.ts index 97f199d839..7997052dc9 100644 --- a/packages/extensions/cordis-host-runner/src/inspect-registry.ts +++ b/packages/extensions/cordis-host-runner/src/inspect-registry.ts @@ -56,12 +56,21 @@ export class CordisInspectRegistryService extends Service { /** * Register one Host provider. + * + * Throws only when a different manifest claims the same provider id. + * An identical re-registration (same id, description, and every method) + * replaces the stored entry so the newest registrant's query closure is + * active. This allows multiple plugin mounts (e.g. two agent presets) that + * register the same capability to coexist without collision. * @param registration - manifest and local query handler. * @returns idempotent disposer. */ register(registration: HostCordisInspectProviderRegistration): () => void { const manifest = validateManifest(registration.manifest) - if (this.providers.has(manifest.id)) throw new Error(`Host Cordis inspect provider "${manifest.id}" is already registered`) + const existing = this.providers.get(manifest.id) + if (existing !== undefined && !sameManifest(existing.manifest, manifest)) { + throw new Error(`Host Cordis inspect provider "${manifest.id}" is already registered`) + } const stored = { ...registration, manifest } this.providers.set(manifest.id, stored) return () => { @@ -198,6 +207,35 @@ export class CordisInspectRegistryService extends Service { } } +/** Deep equality for JSON-compatible values (objects, arrays, primitives). */ +function jsonEqual(a: unknown, b: unknown): boolean { + if (a === b) return true + if (typeof a !== 'object' || typeof b !== 'object' || a === null || b === null) return false + if (Array.isArray(a) || Array.isArray(b)) { + if (!Array.isArray(a) || !Array.isArray(b) || a.length !== b.length) return false + return a.every((entry, index) => jsonEqual(entry, b[index])) + } + const left = a as Record + const right = b as Record + const keys = Object.keys(left) + if (keys.length !== Object.keys(right).length) return false + return keys.every(key => key in right && jsonEqual(left[key], right[key])) +} + +/** Whether two manifests describe the same provider capability. */ +function sameManifest(a: CordisInspectProviderManifest, b: CordisInspectProviderManifest): boolean { + if (a.id !== b.id || a.description !== b.description) return false + if (a.methods.length !== b.methods.length) return false + return a.methods.every((method, index) => { + const other = b.methods[index] + return other !== undefined + && method.name === other.name + && method.description === other.description + && jsonEqual(method.inputSchema, other.inputSchema) + && jsonEqual(method.outputSchema, other.outputSchema) + }) +} + function view(platform: CordisInspectPlatform, manifest: CordisInspectProviderManifest): CordisInspectProviderView { return { platform, ...manifest, methods: [...manifest.methods] } } diff --git a/packages/extensions/cordis-host-runner/tests/inspect-registry.spec.ts b/packages/extensions/cordis-host-runner/tests/inspect-registry.spec.ts new file mode 100644 index 0000000000..13c78635e4 --- /dev/null +++ b/packages/extensions/cordis-host-runner/tests/inspect-registry.spec.ts @@ -0,0 +1,86 @@ +import { Context } from '@deepseek-ai/cordis' +import { describe, expect, it } from 'vitest' +import { CordisInspectRegistryService, type HostCordisInspectProviderRegistration } from '../src/inspect-registry.ts' + +function makeProvider(id: string, desc: string): HostCordisInspectProviderRegistration { + return { + manifest: { + id, + description: desc, + methods: [{ + name: 'list', + description: 'List things.', + inputSchema: { type: 'object', properties: {}, additionalProperties: false }, + outputSchema: { description: 'anything' }, + }], + }, + async query() { return {} }, + } +} + +describe('CordisInspectRegistryService', () => { + it('accepts the first registration of a provider id', () => { + const ctx = new Context() + const svc = new CordisInspectRegistryService(ctx) + const p = makeProvider('Service', 'desc') + const dispose = svc.register(p) + expect(svc.list()).toContainEqual(expect.objectContaining({ id: 'Service', platform: 'host' })) + dispose() + expect(svc.list()).not.toContainEqual(expect.objectContaining({ id: 'Service' })) + }) + + it('replaces on identical re-registration without throwing', () => { + const ctx = new Context() + const svc = new CordisInspectRegistryService(ctx) + const p = makeProvider('Service', 'desc') + const disposeA = svc.register(p) + const disposeB = svc.register(p) // same object — identical manifest + // Still in the directory (replaced, not removed) + expect(svc.list()).toContainEqual(expect.objectContaining({ id: 'Service', platform: 'host' })) + // disposeA should be a no-op because the map entry was replaced + disposeA() + expect(svc.list()).toContainEqual(expect.objectContaining({ id: 'Service', platform: 'host' })) + // disposeB removes the current registration + disposeB() + expect(svc.list()).not.toContainEqual(expect.objectContaining({ id: 'Service' })) + }) + + it('rejects a different manifest claiming the same id', () => { + const ctx = new Context() + const svc = new CordisInspectRegistryService(ctx) + svc.register(makeProvider('Service', 'original')) + expect(() => svc.register(makeProvider('Service', 'different'))) + .toThrow('Host Cordis inspect provider "Service" is already registered') + }) + + it('rejects empty provider id', () => { + const ctx = new Context() + const svc = new CordisInspectRegistryService(ctx) + expect(() => svc.register(makeProvider('', 'empty'))) + .toThrow('Cordis inspect provider id must not be empty') + }) + + it('detects method-array length mismatch as unequal', () => { + const ctx = new Context() + const svc = new CordisInspectRegistryService(ctx) + const a: HostCordisInspectProviderRegistration = { + manifest: { + id: 'X', description: 'd', + methods: [{ name: 'a', description: 'a', inputSchema: {}, outputSchema: {} }], + }, + async query() { return {} }, + } + const b: HostCordisInspectProviderRegistration = { + manifest: { + id: 'X', description: 'd', + methods: [ + { name: 'a', description: 'a', inputSchema: {}, outputSchema: {} }, + { name: 'b', description: 'b', inputSchema: {}, outputSchema: {} }, + ], + }, + async query() { return {} }, + } + svc.register(a) + expect(() => svc.register(b)).toThrow('already registered') + }) +})