From 2a18188f8d6899b85f3090ea849eebe2ec4aa697 Mon Sep 17 00:00:00 2001 From: Jack Zhuang <50353452+hotlong@users.noreply.github.com> Date: Thu, 3 Sep 2026 09:07:30 +0800 Subject: [PATCH] fix(ai): author the canonical agent id everywhere the platform teaches one (#14461) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `skills/objectstack-ai` tells authors `data_chat` / `metadata_assistant` are "not vocabulary — always write `ask` / `build`", and every live example the platform shipped taught the opposite. Measured rather than assumed — the card left this undecidable because the consumer lives in the closed `cloud` repo. At `cloud` main@3856fbf7, `service-ai-studio/src/agents/metadata-assistant-agent.ts:12,40` ships the record as `name: BUILD_AGENT_NAME` = 'build' and `plugin.ts:58` registers `metadata_assistant` as a one-way, resolution-only legacy alias. The canonical id IS `build`; the reading that made this a ruling is falsified. - `studio.app.ts` — the repo's only `app.defaultAgent` usage, re-pinned from the alias to `'build'`. Not cosmetic: alias resolution depends on an in-memory `registerAgentAlias` call having run, which cloud documents silently no-op'ing under bundle load ordering; the canonical id never touches the alias table. - `mcp-server-runtime.ts` — the `agent_prompt` `agentName` example named two retired aliases and neither canonical id, to every MCP client that asked. - `validate-ai-agent-authoring` — the value limb reused the four-name roster, so the gate deliberately passed the spelling the catalogue bans. The two limbs now read different tables: declaration keeps all four (shadowing is shadowing), value takes `ask` / `build` with legacy aliases under their own rule id `default-agent-legacy-alias`, because an alias resolves and an unknown name does not. #6041's operative decisions are intact — warning tier, no Zod enum. Co-Authored-By: Claude Opus 5 --- .../default-agent-canonical-spelling.md | 72 ++++++++++++ packages/lint/src/index.ts | 1 + .../src/validate-ai-agent-authoring.test.ts | 69 ++++++++++- .../lint/src/validate-ai-agent-authoring.ts | 110 +++++++++++++++--- packages/mcp/src/mcp-server-runtime.ts | 9 +- .../platform-objects/src/apps/studio.app.ts | 26 ++++- 6 files changed, 262 insertions(+), 25 deletions(-) create mode 100644 .changeset/default-agent-canonical-spelling.md diff --git a/.changeset/default-agent-canonical-spelling.md b/.changeset/default-agent-canonical-spelling.md new file mode 100644 index 0000000000..89e3a5fd18 --- /dev/null +++ b/.changeset/default-agent-canonical-spelling.md @@ -0,0 +1,72 @@ +--- +"@objectstack/platform-objects": patch +"@objectstack/lint": patch +"@objectstack/mcp": patch +--- + +fix(ai): author the CANONICAL agent id everywhere the platform teaches one — Studio's pin, the MCP prompt example, and the lint's value roster (#14461) + +`skills/objectstack-ai` tells authors that `data_chat` and `metadata_assistant` +"are **not** vocabulary — always write `ask` / `build`". The platform then +taught the opposite from every live example it ships. Nothing was broken at +runtime; what was wrong is what an author copies. + +**Studio's pin.** `studio.app.ts` was the repo's ONLY `app.defaultAgent` usage, +and it spelled the alias: + +``` +- defaultAgent: 'metadata_assistant', ++ defaultAgent: 'build', +``` + +The triage card left this undecidable — if the cloud plugin registered the +agent under the legacy id, re-pinning would be a behaviour change in a +consumer this repo cannot see. Measured instead of assumed, at `cloud` +`main@3856fbf7`: `service-ai-studio/src/agents/metadata-assistant-agent.ts:12,40` +ships the record as `name: BUILD_AGENT_NAME` = `'build'`, and `plugin.ts:58` +registers `metadata_assistant` as a **one-way, resolution-only** legacy alias. +The canonical id *is* `build`; the old pin reached it by detour. + +Nor is the re-pin cosmetic. Alias resolution depends on an in-memory +`registerAgentAlias` call having run at plugin init, and cloud carries two +defensive docblocks about that registration silently no-op'ing for real under +bundle load ordering (`service-ai-studio/src/plugin.ts:44-57`, +`service-ai/src/agent-runtime.ts:30-41` — "a missed alias must never hide a +real platform agent like `build`"). The canonical id never touches the alias +table, so this drops a load-order dependency from the platform's own flagship +authoring surface. On the UI side nothing moves: `objectui`'s +`AGENT_ALIAS_GROUPS` is bidirectional and canonical-first, and +`SURFACE_DEFAULT['studio-build']` was already `'build'`. + +**The MCP prompt example.** `mcp-server-runtime.ts`'s `agent_prompt` argument +described itself as `'Name of the agent to load (e.g. "data_chat", +"metadata_assistant")'` — two retired aliases, neither canonical id present. +That string is served to every MCP client asking what to pass, so the one +surface that suggests a spelling to an LLM suggested the two the catalogue +forbids. Now `(e.g. "ask", "build")`. + +**The lint's value roster.** `validate-ai-agent-authoring`'s `defaultAgent` +**value** limb reused the four-name `PLATFORM_AGENT_NAMES` set, so it +deliberately passed `metadata_assistant` — the gate that exists to make +authoring mistakes loud waved through the exact spelling the catalogue bans, +which is the silent-tolerance shape ADR-0078 exists to close, committed by the +gate itself. The two limbs now read different tables, because they ask +different questions: + +- **declaration limb** — unchanged, still all four names. Declaring + `metadata_assistant` shadows the `build` record through the alias exactly as + declaring `build` does. +- **value limb** — canonical `ask` / `build` only. A legacy alias gets its own + rule id `default-agent-legacy-alias` (exported) and its own wording, because + an alias **resolves** (the app gets the agent it meant — a spelling defect) + while an unknown name does **not** (the pin is inert). Describing the alias + as "no effect" would send an author hunting a bug that is not there. + +Both of the #6041 ruling's operative decisions are kept intact: still +`warning` tier, still no Zod enum narrowing. `defaultAgent: 'metadata_assistant'` +keeps parsing, building, and resolving — the only change is that authoring it +now says so. + +Not breaking: nothing an author can write was removed, and both aliases stay +resolvable for old bookmarks and persisted `agent_id`s, which is the only job +ADR-0063 §2 ever gave them. diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index fd64d1aa13..1bf50e0df7 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -623,6 +623,7 @@ export { validateAiAgentAuthoring, AGENT_AUTHORING_WITHDRAWN, DEFAULT_AGENT_OUTSIDE_ROSTER, + DEFAULT_AGENT_LEGACY_ALIAS, } from './validate-ai-agent-authoring.js'; export type { AiAgentAuthoringFinding, diff --git a/packages/lint/src/validate-ai-agent-authoring.test.ts b/packages/lint/src/validate-ai-agent-authoring.test.ts index 3dd05fcf80..8955be1b08 100644 --- a/packages/lint/src/validate-ai-agent-authoring.test.ts +++ b/packages/lint/src/validate-ai-agent-authoring.test.ts @@ -5,6 +5,7 @@ import { validateAiAgentAuthoring, AGENT_AUTHORING_WITHDRAWN, DEFAULT_AGENT_OUTSIDE_ROSTER, + DEFAULT_AGENT_LEGACY_ALIAS, } from './validate-ai-agent-authoring.js'; describe('validate-ai-agent-authoring', () => { @@ -86,22 +87,80 @@ describe('validate-ai-agent-authoring', () => { }); // Names the offending value. expect(findings[0].message).toContain('"sales_copilot"'); - // Names the allowed set (canonical + legacy aliases). + // Names the allowed set — the CANONICAL two only (#14461). The legacy + // aliases must not appear here: this string is the prescription, and + // offering `metadata_assistant` as a thing to write is the very defect + // #14461 closed. expect(findings[0].message).toContain('ask'); expect(findings[0].message).toContain('build'); - expect(findings[0].message).toContain('data_chat'); - expect(findings[0].message).toContain('metadata_assistant'); + expect(findings[0].message).not.toContain('data_chat'); + expect(findings[0].message).not.toContain('metadata_assistant'); expect(findings[0].hint).toContain('ask'); expect(findings[0].hint).toContain('build'); + expect(findings[0].hint).not.toContain('metadata_assistant'); }); - it('passes every canonical platform agent name and every legacy alias', () => { - for (const defaultAgent of ['ask', 'build', 'data_chat', 'metadata_assistant']) { + it('passes the canonical platform agent names', () => { + for (const defaultAgent of ['ask', 'build']) { const stack = { apps: [{ name: 'app', defaultAgent }] }; expect(validateAiAgentAuthoring(stack), defaultAgent).toEqual([]); } }); + describe('legacy alias values (issue #14461)', () => { + // Studio itself pinned `metadata_assistant` while the published skill + // told authors never to write it, and this rule — reusing the four-name + // roster — waved the alias through. The value limb now judges against + // the canonical two, and an alias gets its own id and prescription. + it.each([ + ['metadata_assistant', 'build'], + ['data_chat', 'ask'], + ])('flags %s and prescribes %s', (alias, canonical) => { + const findings = validateAiAgentAuthoring({ + apps: [{ name: 'studio', defaultAgent: alias }], + }); + expect(findings).toHaveLength(1); + expect(findings[0]).toMatchObject({ + severity: 'warning', + rule: DEFAULT_AGENT_LEGACY_ALIAS, + where: 'app "studio".defaultAgent', + path: 'apps[0].defaultAgent', + }); + expect(findings[0].message).toContain(`"${alias}"`); + expect(findings[0].message).toContain(`"${canonical}"`); + expect(findings[0].hint).toContain(`defaultAgent: '${canonical}'`); + }); + + it('says the alias RESOLVES — it is a spelling defect, not a broken pin', () => { + // The distinction the separate rule id exists to carry: an unknown + // name is inert at runtime, an alias is not. A message that described + // the alias as "no effect" would be false, and an author who read it + // would go looking for a bug that is not there. + const [alias] = validateAiAgentAuthoring({ + apps: [{ name: 'studio', defaultAgent: 'metadata_assistant' }], + }); + const [unknown] = validateAiAgentAuthoring({ + apps: [{ name: 'crm', defaultAgent: 'sales_copilot' }], + }); + expect(alias.message).toContain('still resolves'); + expect(alias.message).not.toContain('has no effect'); + expect(unknown.message).toContain('has no effect'); + }); + + it('leaves the DECLARATION limb reading all four names', () => { + // The two limbs ask different questions, so they keep different + // rosters: declaring `metadata_assistant` shadows the `build` record + // through the alias exactly as declaring `build` does, and that + // judgement is unchanged by #14461. + for (const name of ['ask', 'build', 'data_chat', 'metadata_assistant']) { + const findings = validateAiAgentAuthoring({ agents: [{ name }] }); + expect(findings, name).toHaveLength(1); + expect(findings[0].rule, name).toBe(AGENT_AUTHORING_WITHDRAWN); + expect(findings[0].message, name).toContain('PLATFORM agent id'); + } + }); + }); + it('is silent when defaultAgent is absent, empty, or not a string', () => { expect(validateAiAgentAuthoring({ apps: [{ name: 'a' }] })).toEqual([]); expect(validateAiAgentAuthoring({ apps: [{ name: 'a', defaultAgent: '' }] })).toEqual([]); diff --git a/packages/lint/src/validate-ai-agent-authoring.ts b/packages/lint/src/validate-ai-agent-authoring.ts index 15843d3df8..e4517f2de7 100644 --- a/packages/lint/src/validate-ai-agent-authoring.ts +++ b/packages/lint/src/validate-ai-agent-authoring.ts @@ -42,6 +42,35 @@ * at warning tier, reusing `PLATFORM_AGENT_NAMES` rather than narrowing the * schema to an enum (a breaking authoring change ADR-0063 already walked * back once). + * + * ## Why the value limb no longer reads the same roster (issue #14461) + * + * `PLATFORM_AGENT_NAMES` holds FOUR names, and reusing it for the value limb + * meant this gate accepted `defaultAgent: 'metadata_assistant'` — the exact + * spelling `skills/objectstack-ai` tells authors is "not vocabulary". The + * platform then taught it from its own only live example: `studio.app.ts` + * pinned the alias. So an AI author copying the one working example in the + * repo wrote the forbidden spelling and this rule waved it through — the + * silent-tolerance shape ADR-0078 exists to close, committed by the gate + * itself. + * + * The maintainer ruling on #14461 (2026-09-03) re-pins Studio to `build` and + * SPLITS the two limbs' rosters, keeping both of #6041's operative decisions + * intact (warning tier, no Zod enum): + * + * - the DECLARATION limb still reads all four names. Its question is "does + * this record shadow a platform record?", and declaring `metadata_assistant` + * shadows `build` through the alias exactly as declaring `build` does. That + * judgement is unchanged. + * - the VALUE limb reads `CANONICAL_AGENT_NAMES` only, and a legacy alias + * gets its own rule id and wording ({@link DEFAULT_AGENT_LEGACY_ALIAS}). + * Its question is "is this the right thing to WRITE?", and the answer for + * an alias is no even though it resolves. + * + * The alias limb stays `warning`, not `error`, and for a sharper reason than + * the roster limb: an aliased pin is not broken. It resolves, the app gets the + * agent it meant, and nothing a user can see is wrong — which is precisely why + * the signal has to be an authoring-time nudge rather than a build break. */ export const AGENT_AUTHORING_WITHDRAWN = 'agent-authoring-withdrawn'; @@ -49,6 +78,9 @@ export const AGENT_AUTHORING_WITHDRAWN = 'agent-authoring-withdrawn'; /** `app.defaultAgent` names something outside the platform agent roster. */ export const DEFAULT_AGENT_OUTSIDE_ROSTER = 'default-agent-outside-roster'; +/** `app.defaultAgent` spells a platform agent by its RETIRED alias (#14461). */ +export const DEFAULT_AGENT_LEGACY_ALIAS = 'default-agent-legacy-alias'; + export type AiAgentAuthoringSeverity = 'error' | 'warning'; export interface AiAgentAuthoringFinding { @@ -81,14 +113,38 @@ function strName(v: unknown): string | undefined { } /** - * The two platform agent ids (`ask`, `build`) plus their two legacy aliases - * (`data_chat` → `ask`, `metadata_assistant` → `build`, registered via the - * cloud alias registry — ADR-0063 §2). A stack that re-declares any of these - * four names is doing something different from inventing a custom persona - * (it is shadowing a platform record, directly or through its alias), so it - * gets its own wording. + * The two platform agent ids — the only two names that are AUTHORING + * vocabulary (ADR-0063 §1). This is the roster the `app.defaultAgent` value + * limb judges against. */ -const PLATFORM_AGENT_NAMES = new Set(['ask', 'build', 'data_chat', 'metadata_assistant']); +const CANONICAL_AGENT_NAMES: readonly string[] = ['ask', 'build']; + +/** + * Retired spellings → the canonical id each resolves to (`data_chat` → `ask`, + * `metadata_assistant` → `build`), registered one-way in the cloud alias + * registry at plugin init — ADR-0063 §2. Resolution-only: they are not + * separate records, and the agent catalog shows each agent once under its + * canonical name. Kept resolvable for old bookmarks and persisted `agent_id`s; + * never for new authoring (#14461). + */ +const LEGACY_AGENT_ALIASES = new Map([ + ['data_chat', 'ask'], + ['metadata_assistant', 'build'], +]); + +/** + * Every name that refers to a platform agent, canonically or through its + * alias. A stack that re-declares any of these four is doing something + * different from inventing a custom persona (it is shadowing a platform + * record, directly or through its alias), so it gets its own wording. + * + * Deliberately NOT the roster the value limb reads — see the docblock's + * "#14461" section for why the two questions take different tables. + */ +const PLATFORM_AGENT_NAMES = new Set([ + ...CANONICAL_AGENT_NAMES, + ...LEGACY_AGENT_ALIASES.keys(), +]); /** * Flag every agent declared in a stack. Returns findings (empty = clean, @@ -132,25 +188,51 @@ export function validateAiAgentAuthoring(stack: AnyRec): AiAgentAuthoringFinding }); } - const roster = [...PLATFORM_AGENT_NAMES].join(', '); + const roster = CANONICAL_AGENT_NAMES.join(', '); const apps = asArray(stack.apps); for (let appIdx = 0; appIdx < apps.length; appIdx++) { const app = apps[appIdx]; const defaultAgent = strName(app.defaultAgent); - if (!defaultAgent || PLATFORM_AGENT_NAMES.has(defaultAgent)) continue; + if (!defaultAgent || CANONICAL_AGENT_NAMES.includes(defaultAgent)) continue; const appName = strName(app.name) ?? `#${appIdx}`; + const canonical = LEGACY_AGENT_ALIASES.get(defaultAgent); + + // [#14461] Two different defects share this slot, and collapsing them + // would misdescribe both: an alias RESOLVES (the app gets the agent it + // meant) and an unknown name does NOT (the pin is inert). Separate rule + // ids so a consumer can act on them separately. + if (canonical) { + findings.push({ + severity: 'warning', + rule: DEFAULT_AGENT_LEGACY_ALIAS, + where: `app "${appName}".defaultAgent`, + path: `apps[${appIdx}].defaultAgent`, + message: + `app "${appName}" pins \`defaultAgent\` to "${defaultAgent}", the RETIRED alias of the ` + + `platform agent "${canonical}". It still resolves — the alias registry maps legacy ` + + `names to canonical ones for old bookmarks and persisted \`agent_id\`s (ADR-0063 §2) — ` + + `so nothing is broken at runtime; what is wrong is the spelling in the artifact. It is ` + + `also the weaker pin: resolution depends on the owning package's in-process alias ` + + `registration having run, which the canonical id does not.`, + hint: + `Write \`defaultAgent: '${canonical}'\`. The aliases are back-compat resolution, not ` + + `authoring vocabulary — always author the canonical id (${roster}).`, + }); + continue; + } + findings.push({ severity: 'warning', rule: DEFAULT_AGENT_OUTSIDE_ROSTER, where: `app "${appName}".defaultAgent`, path: `apps[${appIdx}].defaultAgent`, message: - `app "${appName}" pins \`defaultAgent\` to "${defaultAgent}", which is not in the ` + - `platform agent roster (${roster}). The kernel ships exactly two agents (ADR-0063 §2) ` + - `and resolves this key against them and their legacy aliases only — an unrecognized ` + - `name is not rejected, it silently falls back to the platform default at runtime, so ` + - `the pin has no effect and the value drifts from what actually serves the app.`, + `app "${appName}" pins \`defaultAgent\` to "${defaultAgent}", which is not a platform ` + + `agent (${roster}). The kernel ships exactly two agents (ADR-0063 §2) and resolves this ` + + `key against them and their legacy aliases only — an unrecognized name is not rejected, ` + + `it silently falls back to the platform default at runtime, so the pin has no effect and ` + + `the value drifts from what actually serves the app.`, hint: `Set \`defaultAgent\` to one of the platform agent names: ${roster}. If the goal is a ` + `dedicated persona or capability, express it as skills instead — they attach to "ask" ` + diff --git a/packages/mcp/src/mcp-server-runtime.ts b/packages/mcp/src/mcp-server-runtime.ts index 3545ae84aa..292ece4f4a 100644 --- a/packages/mcp/src/mcp-server-runtime.ts +++ b/packages/mcp/src/mcp-server-runtime.ts @@ -1346,7 +1346,14 @@ export class MCPServerRuntime { description: 'Load an agent\'s system prompt with optional UI context. ' + 'Use the agentName argument to select which agent\'s instructions to use.', argsSchema: { - agentName: z.string().describe('Name of the agent to load (e.g. "data_chat", "metadata_assistant")'), + // [#14461] The example names the two CANONICAL platform agent ids. + // It used to read `"data_chat", "metadata_assistant"` — both retired + // aliases, neither canonical id present — so every MCP client asking + // what to pass was taught the exact two spellings + // `skills/objectstack-ai` forbids. The aliases still resolve (cloud's + // one-way alias registry), so nothing broke; what was wrong is that + // this is the suggestion an author copies. + agentName: z.string().describe('Name of the agent to load (e.g. "ask", "build")'), objectName: z.string().optional().describe('Current object the user is viewing'), recordId: z.string().optional().describe('Currently selected record ID'), viewName: z.string().optional().describe('Current view name'), diff --git a/packages/platform-objects/src/apps/studio.app.ts b/packages/platform-objects/src/apps/studio.app.ts index b91b2b2fb0..042219fb3a 100644 --- a/packages/platform-objects/src/apps/studio.app.ts +++ b/packages/platform-objects/src/apps/studio.app.ts @@ -43,11 +43,27 @@ export const STUDIO_APP: App = { reason: 'Core developer workbench shipped by @objectstack/platform-objects — see ADR-0010.', docsUrl: 'https://objectstack.ai/docs/references/shared/protection', }, - // Studio is the metadata-authoring host, so its ambient copilot is - // pinned to the schema-architect agent. Resolved by the ambient chat - // endpoint via `app.defaultAgent` — no UI-side `?agent=` override - // needed. Every other app falls back to the data-query agent. - defaultAgent: 'metadata_assistant', + // Studio is the metadata-authoring host, so its ambient copilot is pinned + // to `build`, the authoring agent. Resolved by the ambient chat endpoint + // via `app.defaultAgent` — no UI-side `?agent=` override needed. Every + // other app falls back to `ask`, the data-query agent. + // + // [#14461] This spelled `'metadata_assistant'` until the maintainer ruling + // (2026-09-03): the legacy alias that `skills/objectstack-ai` tells authors + // is "not vocabulary", taught from the repo's ONLY live `defaultAgent` + // example. `build` is not a rename in flight — it is the record's canonical + // id today (`cloud` `service-ai-studio/src/agents/metadata-assistant-agent.ts:40` + // ships `name: BUILD_AGENT_NAME` = `'build'`, and `plugin.ts:58` registers + // `metadata_assistant` as a ONE-WAY legacy alias, resolution-only). + // + // Nor is the re-pin cosmetic. The alias resolves only if an in-memory + // `registerAgentAlias` call has actually run, and cloud carries two + // defensive docblocks about that registration silently no-op'ing under + // bundle load ordering (`service-ai-studio/src/plugin.ts:44-57`, + // `service-ai/src/agent-runtime.ts:30-41`). The canonical id never touches + // the alias table, so this drops a load-order dependency from the + // platform's own flagship authoring surface. + defaultAgent: 'build', branding: { primaryColor: '#6366f1', // Indigo-500 — distinct from Setup's slate },