diff --git a/.changeset/ai-agent-catalog-platform-filter.md b/.changeset/ai-agent-catalog-platform-filter.md new file mode 100644 index 0000000000..1601d1342d --- /dev/null +++ b/.changeset/ai-agent-catalog-platform-filter.md @@ -0,0 +1,15 @@ +--- +"@objectstack/service-ai": patch +--- + +fix(ai): keep platform agents in the runtime catalog regardless of alias-map timing. + +`AgentRuntime.listAgents()` filtered the catalog to platform agents by the +in-memory `registerAgentAlias` values, so a missed/late alias registration +(e.g. a cloud package's module-load `registerAgentAlias('metadata_assistant', +'build')` not yet applied) silently dropped a real platform agent like `build` +from `GET /api/v1/ai/agents`. The catalog now recognises a platform agent by +the intrinsic package-protection envelope stamped on built-ins at registration +(`_provenance`/`_lock`/`_packageId`, or a still-public `protection` block), +with the alias-table values kept only as a belt-and-suspenders fallback. Stray +tenant custom agents (ADR-0063 §2, withdrawn) still stay filtered out. diff --git a/packages/services/service-ai/src/__tests__/agent-aliases.test.ts b/packages/services/service-ai/src/__tests__/agent-aliases.test.ts index d149f2fa6b..ff77606a65 100644 --- a/packages/services/service-ai/src/__tests__/agent-aliases.test.ts +++ b/packages/services/service-ai/src/__tests__/agent-aliases.test.ts @@ -46,6 +46,21 @@ describe('agent-aliases', () => { registerAgentAlias('same', 'same'); expect(resolveAgentAlias('same')).toBe('same'); }); + + it('anchors the registry on globalThis so the ESM and CJS builds share one table', () => { + // The package ships dual builds; a module-level `new Map()` would give each + // its own copy and silently drop a cross-build alias (the real cause of the + // `metadata_assistant`→`build` 404). The table must live on a well-known + // global Symbol so both builds resolve to the SAME instance. + registerAgentAlias('legacy_probe', 'ask'); + const shared = (globalThis as Record)[ + Symbol.for('@objectstack/service-ai#agentNameAliases') + ] as Map | undefined; + expect(shared).toBeInstanceOf(Map); + expect(shared!.get('legacy_probe')).toBe('ask'); + // A second "build copy" reading via the same global key sees the alias. + expect(shared!.get('data_chat')).toBe('ask'); + }); }); describe('AgentRuntime.loadAgent (alias-aware)', () => { diff --git a/packages/services/service-ai/src/__tests__/chatbot-features.test.ts b/packages/services/service-ai/src/__tests__/chatbot-features.test.ts index 5f0bbbccbe..a72e1df498 100644 --- a/packages/services/service-ai/src/__tests__/chatbot-features.test.ts +++ b/packages/services/service-ai/src/__tests__/chatbot-features.test.ts @@ -756,6 +756,39 @@ describe('AgentRuntime', () => { expect(agents).toHaveLength(1); expect(agents[0].name).toBe('ask'); }); + + // ── Catalog membership is decoupled from the in-memory alias table ────── + // A platform agent must surface from an INTRINSIC, persisted signal so a + // missed `registerAgentAlias` call (bundle load ordering) never hides it. + + it('surfaces a platform agent carrying the `_provenance:"package"` envelope even when its name is NOT alias-registered', async () => { + const unaliased = { ...BUILD_AGENT, name: 'buildx', _provenance: 'package' }; + (metadataService.list as any).mockResolvedValue([unaliased]); + const agents = await runtime.listAgents(); + expect(agents.map((a) => a.name)).toContain('buildx'); + }); + + it('surfaces an agent carrying a `_lock` envelope when not alias-registered', async () => { + const locked = { ...BUILD_AGENT, name: 'buildz', _lock: 'full' }; + (metadataService.list as any).mockResolvedValue([locked]); + const agents = await runtime.listAgents(); + expect(agents.map((a) => a.name)).toContain('buildz'); + }); + + it('surfaces an agent carrying the pre-translation `protection` block when not alias-registered', async () => { + const withBlock = { ...BUILD_AGENT, name: 'buildy', protection: { lock: 'full', reason: 'x' } }; + (metadataService.list as any).mockResolvedValue([withBlock]); + const agents = await runtime.listAgents(); + expect(agents.map((a) => a.name)).toContain('buildy'); + }); + + it('hides a stray tenant custom agent (no platform envelope, not alias-registered)', async () => { + const { protection: _omit, ...buildNoProtection } = BUILD_AGENT; + const tenant = { ...buildNoProtection, name: 'tenant_custom_agent' }; + (metadataService.list as any).mockResolvedValue([ASK_AGENT, tenant]); + const agents = await runtime.listAgents(); + expect(agents.map((a) => a.name)).toEqual(['ask']); + }); }); }); diff --git a/packages/services/service-ai/src/agent-runtime.ts b/packages/services/service-ai/src/agent-runtime.ts index 1bf6e835ce..8c0b156d37 100644 --- a/packages/services/service-ai/src/agent-runtime.ts +++ b/packages/services/service-ai/src/agent-runtime.ts @@ -13,6 +13,36 @@ import { SchemaRetriever, type ObjectShape } from './schema-retriever.js'; import { ASK_AGENT_NAME } from './agents/index.js'; import { resolveAgentAlias, platformAgentNames } from './agents/agent-aliases.js'; +/** + * True when an agent record is platform-owned and therefore belongs in the + * runtime catalog (ADR-0063 §2 — only `ask`/`build` surface; stray tenant + * custom agents are hidden). + * + * Catalog membership is decided from an INTRINSIC, persisted signal so it does + * NOT depend on an in-memory `registerAgentAlias` call having run (a missed + * alias must never hide a real platform agent like `build`): + * - the runtime protection envelope stamped on built-ins at registration — + * `_provenance === 'package'` / `_lock` / `_packageId`; or + * - a still-public `protection` block — the same envelope before the loader + * translates it, which is how it lands on records written through the direct + * `metadataService.register` path that does not run `applyProtection`. + * + * Falls back to the canonical platform-agent name set (the alias-table values) + * so prior behaviour is strictly extended, never narrowed. A runtime-created + * tenant agent carries none of these, so it stays filtered out. + */ +function isPlatformAgentRecord(raw: unknown, name: string, platform: Set): boolean { + if (platform.has(name)) return true; + if (!raw || typeof raw !== 'object') return false; + const r = raw as Record; + return ( + r._provenance === 'package' || + r._lock != null || + r._packageId != null || + (typeof r.protection === 'object' && r.protection != null) + ); +} + /** * Context passed alongside a user message when chatting with an agent. * @@ -74,21 +104,26 @@ export class AgentRuntime { const rawItems = await this.metadataService.list('agent'); const agents: Array<{ name: string; label: string; role: string }> = []; - // ADR-0063 §2 — the runtime catalog surfaces only platform-owned agents - // (`ask`/`build`). Tenant custom agents are withdrawn; any stray custom - // record persisted before the withdrawal is filtered out here so it never - // appears in the picker or the agents list. + // ADR-0063 §2 — the runtime catalog surfaces only PLATFORM-OWNED agents + // (`ask`/`build`); a stray tenant custom record (e.g. one persisted before + // tenant agents were withdrawn) is filtered out so it never shows in the + // picker. Membership is driven by an INTRINSIC, persisted package-provenance + // signal (see {@link isPlatformAgentRecord}), NOT by the in-memory alias + // table alone: coupling the catalog to the alias map made a missed + // `registerAgentAlias` call (bundle load ordering) silently drop a real agent + // like `build` from the list even though its record exists and chat works. + // The alias-table values stay in the union as a belt-and-suspenders fallback. const platform = platformAgentNames(); for (const raw of rawItems) { const result = AgentSchema.safeParse(raw); - if (result.success && result.data.active && platform.has(result.data.name)) { - agents.push({ - name: result.data.name, - label: result.data.label, - role: result.data.role, - }); - } + if (!result.success || !result.data.active) continue; + if (!isPlatformAgentRecord(raw, result.data.name, platform)) continue; + agents.push({ + name: result.data.name, + label: result.data.label, + role: result.data.role, + }); } return agents; diff --git a/packages/services/service-ai/src/agents/agent-aliases.ts b/packages/services/service-ai/src/agents/agent-aliases.ts index ca4ba103ef..a29776772d 100644 --- a/packages/services/service-ai/src/agents/agent-aliases.ts +++ b/packages/services/service-ai/src/agents/agent-aliases.ts @@ -20,11 +20,32 @@ * Aliases are resolution-only: they are NOT separate metadata records, so the * agent list (`GET /api/v1/ai/agents`) still shows each agent exactly once * under its canonical name. + * + * ── Why the registry is anchored on `globalThis` ──────────────────────────── + * This module ships as BOTH an ESM (`import` → `dist/index.js`) and a CJS + * (`require` → `dist/index.cjs`) build. A bare module-level `new Map()` gives + * EACH build its own copy, so an alias registered through one build is invisible + * to a reader in the other. That is the exact bug this fixes: the cloud AI Studio + * plugin is bundled as CJS and `require`s the CJS copy to call + * {@link registerAgentAlias}, while the framework's agent routes are loaded as + * ESM and read the ESM copy via {@link resolveAgentAlias} — so `metadata_assistant` + * resolved to nothing and `/agents/metadata_assistant/chat` 404'd even though the + * Studio had "registered" the alias. Anchoring the Map (and its seed) on a + * `Symbol.for` key makes the two builds share ONE table. */ -const AGENT_NAME_ALIASES = new Map([ - // The framework's own data agent rename. - ['data_chat', 'ask'], -]); +const ALIAS_REGISTRY_KEY: unique symbol = Symbol.for('@objectstack/service-ai#agentNameAliases'); + +/** The single process-wide alias table, created (and seeded) on first touch. */ +function aliasRegistry(): Map { + const g = globalThis as typeof globalThis & { [ALIAS_REGISTRY_KEY]?: Map }; + let map = g[ALIAS_REGISTRY_KEY]; + if (!map) { + // Seed the framework's own data agent rename on first access. + map = new Map([['data_chat', 'ask']]); + g[ALIAS_REGISTRY_KEY] = map; + } + return map; +} /** * Register a legacy→canonical agent-name alias. Idempotent; a later call for the @@ -33,18 +54,18 @@ const AGENT_NAME_ALIASES = new Map([ */ export function registerAgentAlias(legacy: string, canonical: string): void { if (legacy && canonical && legacy !== canonical) { - AGENT_NAME_ALIASES.set(legacy, canonical); + aliasRegistry().set(legacy, canonical); } } /** Resolve a (possibly legacy) agent name to its canonical id, or itself. */ export function resolveAgentAlias(name: string): string { - return AGENT_NAME_ALIASES.get(name) ?? name; + return aliasRegistry().get(name) ?? name; } /** Test/diagnostics helper: a snapshot of the current alias table. */ export function agentAliasEntries(): Array<[string, string]> { - return Array.from(AGENT_NAME_ALIASES.entries()); + return Array.from(aliasRegistry().entries()); } /** @@ -58,5 +79,5 @@ export function agentAliasEntries(): Array<[string, string]> { * agent record (e.g. one persisted before tenant agents were withdrawn). */ export function platformAgentNames(): Set { - return new Set(AGENT_NAME_ALIASES.values()); + return new Set(aliasRegistry().values()); } diff --git a/packages/services/service-ai/src/agents/ask-agent.ts b/packages/services/service-ai/src/agents/ask-agent.ts index 07de43cfcd..a2172c0d8a 100644 --- a/packages/services/service-ai/src/agents/ask-agent.ts +++ b/packages/services/service-ai/src/agents/ask-agent.ts @@ -99,4 +99,17 @@ Always answer in the same language the user is using. Detailed tool-usage guidan maxIterations: 10, allowReplan: true, }, + + // ADR-0063 §2 / ADR-0010 §3.7 — built-in platform agent. Tenants extend the + // platform with skills + tools, never by editing this persona, so it is fully + // locked against overlay edits/deletes. (The platform's own boot-time refresh + // writes through the authoritative register path, which the lock does not gate.) + // This author-protection envelope is ALSO the intrinsic, persisted signal that + // `AgentRuntime.listAgents()` keys off to keep `ask` in the catalog regardless + // of whether the in-memory alias table happened to be populated — a missed + // alias registration must never hide a real platform agent. + protection: { + lock: 'full', + reason: 'Built-in platform assistant shipped by @objectstack/service-ai.', + }, }; diff --git a/packages/services/service-ai/src/plugin.ts b/packages/services/service-ai/src/plugin.ts index e97585b457..ee3789d98f 100644 --- a/packages/services/service-ai/src/plugin.ts +++ b/packages/services/service-ai/src/plugin.ts @@ -5,6 +5,7 @@ import { readEnvWithDeprecation } from '@objectstack/types'; import type { IAIService, IAIConversationService, IAnalyticsService, IAutomationService, IDataEngine, IEmbedder, IMetadataService, LLMAdapter } from '@objectstack/spec/contracts'; import { EMBEDDER_SERVICE } from '@objectstack/spec/contracts'; import type * as AI from '@objectstack/spec/ai'; +import { applyProtection } from '@objectstack/spec/shared'; import { AIService } from './ai-service.js'; import type { AIServiceConfig } from './ai-service.js'; import { buildAIRoutes } from './routes/ai-routes.js'; @@ -847,7 +848,14 @@ export class AIServicePlugin implements Plugin { ctx.logger.warn(`[AI] Failed to register built-in ${type} ${name}`, err instanceof Error ? { error: err.message } : { error: String(err) }); } }; - await upsertBuiltin('agent', ASK_AGENT.name, ASK_AGENT); + // Translate the agent's author `protection` block into the runtime + // `_lock`/`_provenance:'package'` envelope before persisting (the + // direct `metadataService.register` path does NOT run the loader's + // `applyProtection`, so do it here). That envelope is both the + // ADR-0010 lock and the intrinsic signal `AgentRuntime.listAgents()` + // uses to keep `ask` in the catalog independent of the alias table. + // Clone first so the shared `ASK_AGENT` export is never mutated. + await upsertBuiltin('agent', ASK_AGENT.name, applyProtection({ ...ASK_AGENT })); // Path A rename (`data_chat`→`ask`): drop the stale legacy agent // record on upgrade so the catalog doesn't list the agent twice. The // legacy NAME stays resolvable for chat via the alias table; this only