Skip to content

Commit 6fefb1a

Browse files
xuyushun441-sysos-zhuangclaude
authored
fix(service-ai): share agent-alias registry across ESM/CJS builds + decouple agent catalog from it (#2190)
Live EE-rig verification of ADR-0063/0064 surfaced two coupled defects in the AI agent catalog/alias layer: 1. GET /api/v1/ai/agents omitted `build` even though build chat worked. 2. POST /api/v1/ai/agents/metadata_assistant/chat 404'd — the legacy alias was dead. Root cause (deeper than the suspected record-init order): `@objectstack/service-ai` ships BOTH an ESM (`import`→index.js) and a CJS (`require`→index.cjs) build, and the alias table was a module-level `new Map()` — so each build got its OWN copy. The cloud AI Studio plugin is bundled as CJS and `require`s the CJS copy to register `metadata_assistant`→`build`, while the framework agent routes load as ESM and read the ESM copy — the alias was written to a Map nobody read. - agent-aliases: anchor the registry (and its `data_chat`→`ask` seed) on a `Symbol.for` key on globalThis so the ESM and CJS builds share ONE table. - agent-runtime.listAgents: stop keying catalog membership solely on the in-memory alias-table values. Decide membership from an INTRINSIC, persisted package signal (`_provenance:'package'`/`_lock`, or a pre-translation `protection` block), with the alias values kept as a fallback union. A missed alias must never hide a real platform agent (ADR-0063 §2 still hides stray tenant agents). - ask-agent: carry a package `protection` block (lock:'full'); translate it to the runtime `_lock`/`_provenance` envelope at register via applyProtection so the catalog signal is present and the built-in persona is locked against overlay edits. Tests: globalThis-shared registry; listAgents surfaces a platform agent via the intrinsic signal with NO alias registered, and still hides an envelope-less tenant agent. Full service-ai suite green (409). Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent ae271d0 commit 6fefb1a

6 files changed

Lines changed: 145 additions & 20 deletions

File tree

packages/services/service-ai/src/__tests__/agent-aliases.test.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,21 @@ describe('agent-aliases', () => {
4646
registerAgentAlias('same', 'same');
4747
expect(resolveAgentAlias('same')).toBe('same');
4848
});
49+
50+
it('anchors the registry on globalThis so the ESM and CJS builds share one table', () => {
51+
// The package ships dual builds; a module-level `new Map()` would give each
52+
// its own copy and silently drop a cross-build alias (the real cause of the
53+
// `metadata_assistant`→`build` 404). The table must live on a well-known
54+
// global Symbol so both builds resolve to the SAME instance.
55+
registerAgentAlias('legacy_probe', 'ask');
56+
const shared = (globalThis as Record<symbol, unknown>)[
57+
Symbol.for('@objectstack/service-ai#agentNameAliases')
58+
] as Map<string, string> | undefined;
59+
expect(shared).toBeInstanceOf(Map);
60+
expect(shared!.get('legacy_probe')).toBe('ask');
61+
// A second "build copy" reading via the same global key sees the alias.
62+
expect(shared!.get('data_chat')).toBe('ask');
63+
});
4964
});
5065

5166
describe('AgentRuntime.loadAgent (alias-aware)', () => {

packages/services/service-ai/src/__tests__/chatbot-features.test.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -756,6 +756,39 @@ describe('AgentRuntime', () => {
756756
expect(agents).toHaveLength(1);
757757
expect(agents[0].name).toBe('ask');
758758
});
759+
760+
// ── Catalog membership is decoupled from the in-memory alias table ──────
761+
// A platform agent must surface from an INTRINSIC, persisted signal so a
762+
// missed `registerAgentAlias` call (bundle load ordering) never hides it.
763+
764+
it('surfaces a platform agent carrying the `_provenance:"package"` envelope even when its name is NOT alias-registered', async () => {
765+
const unaliased = { ...BUILD_AGENT, name: 'buildx', _provenance: 'package' };
766+
(metadataService.list as any).mockResolvedValue([unaliased]);
767+
const agents = await runtime.listAgents();
768+
expect(agents.map((a) => a.name)).toContain('buildx');
769+
});
770+
771+
it('surfaces an agent carrying a `_lock` envelope when not alias-registered', async () => {
772+
const locked = { ...BUILD_AGENT, name: 'buildz', _lock: 'full' };
773+
(metadataService.list as any).mockResolvedValue([locked]);
774+
const agents = await runtime.listAgents();
775+
expect(agents.map((a) => a.name)).toContain('buildz');
776+
});
777+
778+
it('surfaces an agent carrying the pre-translation `protection` block when not alias-registered', async () => {
779+
const withBlock = { ...BUILD_AGENT, name: 'buildy', protection: { lock: 'full', reason: 'x' } };
780+
(metadataService.list as any).mockResolvedValue([withBlock]);
781+
const agents = await runtime.listAgents();
782+
expect(agents.map((a) => a.name)).toContain('buildy');
783+
});
784+
785+
it('hides a stray tenant custom agent (no platform envelope, not alias-registered)', async () => {
786+
const { protection: _omit, ...buildNoProtection } = BUILD_AGENT;
787+
const tenant = { ...buildNoProtection, name: 'tenant_custom_agent' };
788+
(metadataService.list as any).mockResolvedValue([ASK_AGENT, tenant]);
789+
const agents = await runtime.listAgents();
790+
expect(agents.map((a) => a.name)).toEqual(['ask']);
791+
});
759792
});
760793
});
761794

packages/services/service-ai/src/agent-runtime.ts

Lines changed: 46 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,36 @@ import { SchemaRetriever, type ObjectShape } from './schema-retriever.js';
1313
import { ASK_AGENT_NAME } from './agents/index.js';
1414
import { resolveAgentAlias, platformAgentNames } from './agents/agent-aliases.js';
1515

16+
/**
17+
* True when an agent record is platform-owned and therefore belongs in the
18+
* runtime catalog (ADR-0063 §2 — only `ask`/`build` surface; stray tenant
19+
* custom agents are hidden).
20+
*
21+
* Catalog membership is decided from an INTRINSIC, persisted signal so it does
22+
* NOT depend on an in-memory `registerAgentAlias` call having run (a missed
23+
* alias must never hide a real platform agent like `build`):
24+
* - the runtime protection envelope stamped on built-ins at registration —
25+
* `_provenance === 'package'` / `_lock` / `_packageId`; or
26+
* - a still-public `protection` block — the same envelope before the loader
27+
* translates it, which is how it lands on records written through the direct
28+
* `metadataService.register` path that does not run `applyProtection`.
29+
*
30+
* Falls back to the canonical platform-agent name set (the alias-table values)
31+
* so prior behaviour is strictly extended, never narrowed. A runtime-created
32+
* tenant agent carries none of these, so it stays filtered out.
33+
*/
34+
function isPlatformAgentRecord(raw: unknown, name: string, platform: Set<string>): boolean {
35+
if (platform.has(name)) return true;
36+
if (!raw || typeof raw !== 'object') return false;
37+
const r = raw as Record<string, unknown>;
38+
return (
39+
r._provenance === 'package' ||
40+
r._lock != null ||
41+
r._packageId != null ||
42+
(typeof r.protection === 'object' && r.protection != null)
43+
);
44+
}
45+
1646
/**
1747
* Context passed alongside a user message when chatting with an agent.
1848
*
@@ -74,21 +104,26 @@ export class AgentRuntime {
74104
const rawItems = await this.metadataService.list('agent');
75105
const agents: Array<{ name: string; label: string; role: string }> = [];
76106

77-
// ADR-0063 §2 — the runtime catalog surfaces only platform-owned agents
78-
// (`ask`/`build`). Tenant custom agents are withdrawn; any stray custom
79-
// record persisted before the withdrawal is filtered out here so it never
80-
// appears in the picker or the agents list.
107+
// ADR-0063 §2 — the runtime catalog surfaces only PLATFORM-OWNED agents
108+
// (`ask`/`build`); a stray tenant custom record (e.g. one persisted before
109+
// tenant agents were withdrawn) is filtered out so it never shows in the
110+
// picker. Membership is driven by an INTRINSIC, persisted package-provenance
111+
// signal (see {@link isPlatformAgentRecord}), NOT by the in-memory alias
112+
// table alone: coupling the catalog to the alias map made a missed
113+
// `registerAgentAlias` call (bundle load ordering) silently drop a real agent
114+
// like `build` from the list even though its record exists and chat works.
115+
// The alias-table values stay in the union as a belt-and-suspenders fallback.
81116
const platform = platformAgentNames();
82117

83118
for (const raw of rawItems) {
84119
const result = AgentSchema.safeParse(raw);
85-
if (result.success && result.data.active && platform.has(result.data.name)) {
86-
agents.push({
87-
name: result.data.name,
88-
label: result.data.label,
89-
role: result.data.role,
90-
});
91-
}
120+
if (!result.success || !result.data.active) continue;
121+
if (!isPlatformAgentRecord(raw, result.data.name, platform)) continue;
122+
agents.push({
123+
name: result.data.name,
124+
label: result.data.label,
125+
role: result.data.role,
126+
});
92127
}
93128

94129
return agents;

packages/services/service-ai/src/agents/agent-aliases.ts

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,32 @@
2020
* Aliases are resolution-only: they are NOT separate metadata records, so the
2121
* agent list (`GET /api/v1/ai/agents`) still shows each agent exactly once
2222
* under its canonical name.
23+
*
24+
* ── Why the registry is anchored on `globalThis` ────────────────────────────
25+
* This module ships as BOTH an ESM (`import` → `dist/index.js`) and a CJS
26+
* (`require` → `dist/index.cjs`) build. A bare module-level `new Map()` gives
27+
* EACH build its own copy, so an alias registered through one build is invisible
28+
* to a reader in the other. That is the exact bug this fixes: the cloud AI Studio
29+
* plugin is bundled as CJS and `require`s the CJS copy to call
30+
* {@link registerAgentAlias}, while the framework's agent routes are loaded as
31+
* ESM and read the ESM copy via {@link resolveAgentAlias} — so `metadata_assistant`
32+
* resolved to nothing and `/agents/metadata_assistant/chat` 404'd even though the
33+
* Studio had "registered" the alias. Anchoring the Map (and its seed) on a
34+
* `Symbol.for` key makes the two builds share ONE table.
2335
*/
24-
const AGENT_NAME_ALIASES = new Map<string, string>([
25-
// The framework's own data agent rename.
26-
['data_chat', 'ask'],
27-
]);
36+
const ALIAS_REGISTRY_KEY: unique symbol = Symbol.for('@objectstack/service-ai#agentNameAliases');
37+
38+
/** The single process-wide alias table, created (and seeded) on first touch. */
39+
function aliasRegistry(): Map<string, string> {
40+
const g = globalThis as typeof globalThis & { [ALIAS_REGISTRY_KEY]?: Map<string, string> };
41+
let map = g[ALIAS_REGISTRY_KEY];
42+
if (!map) {
43+
// Seed the framework's own data agent rename on first access.
44+
map = new Map<string, string>([['data_chat', 'ask']]);
45+
g[ALIAS_REGISTRY_KEY] = map;
46+
}
47+
return map;
48+
}
2849

2950
/**
3051
* Register a legacy→canonical agent-name alias. Idempotent; a later call for the
@@ -33,18 +54,18 @@ const AGENT_NAME_ALIASES = new Map<string, string>([
3354
*/
3455
export function registerAgentAlias(legacy: string, canonical: string): void {
3556
if (legacy && canonical && legacy !== canonical) {
36-
AGENT_NAME_ALIASES.set(legacy, canonical);
57+
aliasRegistry().set(legacy, canonical);
3758
}
3859
}
3960

4061
/** Resolve a (possibly legacy) agent name to its canonical id, or itself. */
4162
export function resolveAgentAlias(name: string): string {
42-
return AGENT_NAME_ALIASES.get(name) ?? name;
63+
return aliasRegistry().get(name) ?? name;
4364
}
4465

4566
/** Test/diagnostics helper: a snapshot of the current alias table. */
4667
export function agentAliasEntries(): Array<[string, string]> {
47-
return Array.from(AGENT_NAME_ALIASES.entries());
68+
return Array.from(aliasRegistry().entries());
4869
}
4970

5071
/**
@@ -58,5 +79,5 @@ export function agentAliasEntries(): Array<[string, string]> {
5879
* agent record (e.g. one persisted before tenant agents were withdrawn).
5980
*/
6081
export function platformAgentNames(): Set<string> {
61-
return new Set(AGENT_NAME_ALIASES.values());
82+
return new Set(aliasRegistry().values());
6283
}

packages/services/service-ai/src/agents/ask-agent.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,4 +99,17 @@ Always answer in the same language the user is using. Detailed tool-usage guidan
9999
maxIterations: 10,
100100
allowReplan: true,
101101
},
102+
103+
// ADR-0063 §2 / ADR-0010 §3.7 — built-in platform agent. Tenants extend the
104+
// platform with skills + tools, never by editing this persona, so it is fully
105+
// locked against overlay edits/deletes. (The platform's own boot-time refresh
106+
// writes through the authoritative register path, which the lock does not gate.)
107+
// This author-protection envelope is ALSO the intrinsic, persisted signal that
108+
// `AgentRuntime.listAgents()` keys off to keep `ask` in the catalog regardless
109+
// of whether the in-memory alias table happened to be populated — a missed
110+
// alias registration must never hide a real platform agent.
111+
protection: {
112+
lock: 'full',
113+
reason: 'Built-in platform assistant shipped by @objectstack/service-ai.',
114+
},
102115
};

packages/services/service-ai/src/plugin.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { readEnvWithDeprecation } from '@objectstack/types';
55
import type { IAIService, IAIConversationService, IAnalyticsService, IAutomationService, IDataEngine, IEmbedder, IMetadataService, LLMAdapter } from '@objectstack/spec/contracts';
66
import { EMBEDDER_SERVICE } from '@objectstack/spec/contracts';
77
import type * as AI from '@objectstack/spec/ai';
8+
import { applyProtection } from '@objectstack/spec/shared';
89
import { AIService } from './ai-service.js';
910
import type { AIServiceConfig } from './ai-service.js';
1011
import { buildAIRoutes } from './routes/ai-routes.js';
@@ -847,7 +848,14 @@ export class AIServicePlugin implements Plugin {
847848
ctx.logger.warn(`[AI] Failed to register built-in ${type} ${name}`, err instanceof Error ? { error: err.message } : { error: String(err) });
848849
}
849850
};
850-
await upsertBuiltin('agent', ASK_AGENT.name, ASK_AGENT);
851+
// Translate the agent's author `protection` block into the runtime
852+
// `_lock`/`_provenance:'package'` envelope before persisting (the
853+
// direct `metadataService.register` path does NOT run the loader's
854+
// `applyProtection`, so do it here). That envelope is both the
855+
// ADR-0010 lock and the intrinsic signal `AgentRuntime.listAgents()`
856+
// uses to keep `ask` in the catalog independent of the alias table.
857+
// Clone first so the shared `ASK_AGENT` export is never mutated.
858+
await upsertBuiltin('agent', ASK_AGENT.name, applyProtection({ ...ASK_AGENT }));
851859
// Path A rename (`data_chat`→`ask`): drop the stale legacy agent
852860
// record on upgrade so the catalog doesn't list the agent twice. The
853861
// legacy NAME stays resolvable for chat via the alias table; this only

0 commit comments

Comments
 (0)