From 3c74a5b823572f068228c26f82c396d10f76d97b Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Mon, 22 Jun 2026 11:46:42 +0800 Subject: [PATCH] docs(adr): revise ADR-0063 to two agents bound by surface; add ADR-0064 tool scoping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reverses ADR-0040's unified single-assistant core (merged first cut of ADR-0063 in #2164 kept it). ask and build are two products for different needs — like Claude Chat vs Claude Code — bound by the surface the user is in, not a per-turn intent classifier or a roster. Keeps ADR-0040's "user never picks", re-grounded as surface binding. - ADR-0063 (revised): two agents (ask ≈ Chat, free/OSS; build ≈ Code, paid/cloud); skills (+tools/MCP) are the only third-party extension primitive; *.agent.ts closed to third parties; skill→agent surface affinity; ask/build canonical naming; supersedes ADR-0040 core + §3. - ADR-0064 (new): tool scoping — an agent's tools = the union of its skills' tools; remove the global fall-through; resolves ADR-0040 §4. - Assessment: four-repo touchpoint inventory + findings that drove D2. Co-Authored-By: Claude Opus 4.8 --- ...ents-skills-are-the-extension-primitive.md | 125 +++++++++--------- docs/adr/0064-tool-scoping-to-agent.md | 80 +++++++++++ ...-ask-build-agent-development-assessment.md | 107 +++++++++++++++ 3 files changed, 250 insertions(+), 62 deletions(-) create mode 100644 docs/adr/0064-tool-scoping-to-agent.md create mode 100644 docs/audits/2026-06-ask-build-agent-development-assessment.md diff --git a/docs/adr/0063-two-kernel-agents-skills-are-the-extension-primitive.md b/docs/adr/0063-two-kernel-agents-skills-are-the-extension-primitive.md index 22d5da83bd..88bb3d67d0 100644 --- a/docs/adr/0063-two-kernel-agents-skills-are-the-extension-primitive.md +++ b/docs/adr/0063-two-kernel-agents-skills-are-the-extension-primitive.md @@ -1,69 +1,74 @@ -# ADR-0063: Two kernel agents (`ask` / `build`); skills are the only third-party extension primitive +# ADR-0063: Two agents (`ask` / `build`), bound by surface; skills are the only third-party extension primitive **Status**: Proposed (2026-06-22) **Deciders**: ObjectStack Protocol Architects -**Supersedes**: [ADR-0040 §3 and §4](./0040-unified-assistant-and-agent-binding.md) — "custom agents are a builder/admin feature" is withdrawn; the follow-up tool-scoping contract is reframed. -**Builds on**: [ADR-0040](./0040-unified-assistant-and-agent-binding.md) (unified assistant — the end user never picks an agent), [ADR-0033](./0033-ai-assisted-metadata-authoring.md) (draft → verify → publish), [ADR-0038](./0038-build-verification-loop.md) (verify-fix-reverify discipline carried by skills) -**Consumers**: `@objectstack/spec` (agent/skill types), `@objectstack/service-ai` (the open-source `ask` shell), `../cloud/service-ai-studio` (the cloud-only `build` shell), `../objectui` (chat surfaces) +**Supersedes**: [ADR-0040](./0040-unified-assistant-and-agent-binding.md) — its core decision (a *single* unified assistant selected by *per-turn intent classification*) is **reversed**. ADR-0040's UX win ("the end user never picks from a roster") is **kept** but re-grounded: the *surface* binds the agent, not a classifier and not a dropdown. §3 (custom tenant agents) is withdrawn; §4 (tool-scoping) is handed to [ADR-0064](./0064-tool-scoping-to-agent.md). +**Builds on**: [ADR-0033](./0033-ai-assisted-metadata-authoring.md) (draft → verify → publish), [ADR-0038](./0038-build-verification-loop.md) (verify-fix-reverify discipline carried by skills) +**Consumers**: `@objectstack/spec` (agent/skill types), `@objectstack/service-ai` (the open-source `ask` agent), `../cloud/service-ai-studio` (the cloud-only `build` agent), `../objectui` (chat surfaces) **Premise**: pre-launch, no back-compat debt beyond the alias table — specify the target end-state directly. +> **Revises** the first cut of ADR-0063 (merged in #2164), which framed the two as "shells split by what they mutate" while leaving ADR-0040's unified assistant in place. On reflection the unified assistant was itself the error; this revision separates the two agents by *surface*. + --- ## TL;DR -The kernel keeps **exactly two agents**, and they are **platform-owned shells**, not a roster: +The kernel ships **exactly two agents**, and they are **two products for two different needs**, not two intents of one assistant: -- **`ask`** — the data shell. Reads/queries/explores records and executes business actions. **Open-source, free** (`@objectstack/service-ai`). -- **`build`** — the authoring shell. Mutates *metadata* (objects, fields, views, flows). **Cloud-only, paid** (`../cloud/service-ai-studio`). +- **`ask`** — the data product (≈ **Claude Chat**). Conversational read/query/explore over records, and execution of business actions. End-user audience, RLS-bounded, fast turns. **Open-source, free** (`@objectstack/service-ai`). +- **`build`** — the authoring product (≈ **Claude Code**). Agentic mutation of *metadata* (objects, fields, views, flows) through a plan → draft → verify → publish loop. Builder/admin audience, governance-gated, long-running pinned sessions. **Cloud-only, paid** (`../cloud/service-ai-studio`). -Everything else — domain depth, playbooks, persona, judgment, reach to external systems — is delivered as **skills** (+ tools / MCP), loaded by relevance into whichever shell applies. **Third parties extend the platform by authoring `*.skill.ts` and tools, never `*.agent.ts`.** ADR-0040's "custom agents are a builder feature" is withdrawn. +**The user never picks an agent from a roster — the surface they are in binds it.** You are in the data console → `ask`; you are in the builder/Studio → `build`. Exactly as you open *claude.ai* or you open *Claude Code* — you choose a product, not an agent. -This is the analogue of Claude: **`build` ≈ Code, `ask` ≈ Chat.** Claude Code is enormously capable and ships **zero** user-authored agents — its power is skills + tools/MCP + platform-owned subagents. ObjectStack adopts the same shape. +Everything else — domain depth, playbooks, persona, judgment, reach to external systems — is delivered as **skills** (+ tools / MCP), each bound to one agent by affinity. **Third parties extend the platform by authoring `*.skill.ts` and tools, never `*.agent.ts`.** Claude Code is enormously capable and ships **zero** user-authored agents — its power is skills + tools/MCP + platform-owned subagents. ObjectStack adopts the same shape. --- -## Context — why "custom agents" was old thinking - -ADR-0040 already removed the agent *picker* from end users, but it preserved `*.agent.ts` as a **builder/admin extension surface** (§3) and named tool-scoping as a follow-up (§4). The design conversation that produced this ADR found that surface to be a category error: +## Context — why ADR-0040's unified assistant was the wrong call -1. **The platform is metadata-driven, so domain knowledge is already shared.** The `ask` shell reads the full metadata registry — objects, fields, actions, flows. It is *structurally domain-aware for free* on any app that is loaded. "A custom agent is needed to teach the assistant the industry" is false: the ontology is in the metadata, not in the agent. +ADR-0040 collapsed data Q&A and metadata authoring into **one** assistant carrying *all* skills, switched by a **per-turn intent classifier**. On reflection that was a design error, for the same reason Anthropic ships Claude Chat and Claude Code as **separate products**: they serve genuinely different needs, with different interaction models, audiences, risk profiles, and pricing. -2. **What is left over is exactly the definition of a skill.** Persona, judgment, playbooks, instructions, bundled tools, reach to external systems — that is `SKILL.md` (instructions + resources, relevance-routed by frontmatter `description`, progressively disclosed). Describing that as a "custom agent" is just using the wrong word for a skill. +| | `ask` (≈ Chat) | `build` (≈ Code) | +|---|---|---| +| Need | conversational data Q&A + act | agentic authoring of the app itself | +| Audience / permission | end user · row-level security | builder/admin · governance gate | +| Mutates | records (business data) | metadata (the app definition) | +| Interaction rhythm | quick question → answer | plan → draft → verify → publish, self-correcting, long-running | +| Blast radius | one user's data, RLS-scoped | the app for everyone | +| Commerce | open-source · free | cloud-only · paid | -3. **Skills compose; agents don't.** A real request is cross-domain (CRM + accounting + an ISV connector in one breath). You can only be *in* one agent — so agent-first fragments the workflow and re-introduces a routing/selection problem. Skill-first loads every relevant skill into the **same** turn. Agents partition work; skills unify it. +These are not two *intents* of one assistant — they are two *products*. The seam (**"what you change": records vs. the app definition**) carries the architectural boundary, the governance boundary, **and** the commercial boundary at once: three lines, one cut. -4. **Skills scale; agents don't.** Progressive disclosure makes 1,000 skills cost ~nothing until relevant. 1,000 agents is a selection nightmare. A metadata platform's surface (objects/actions/flows) grows without bound — only skill-first scales. +**ADR-0040's own incident argues for separation, not unification.** Its motivating failure (staging, 2026-06-11) was a user in the *Data* assistant asking it to "build a library app"; it flailed because it lacked the authoring disciplines. ADR-0040 read this as "the data persona was missing build skills" and bolted all skills onto one assistant. The correct reading: **that request should never have reached the data surface.** In a Chat/Code-separated world, app-building happens in the builder surface. The incident is evidence the *surfaces were leaking*, not evidence for merging the personas. The per-turn classifier ADR-0040 introduced is a fragile router that re-implements — worse, and at the wrong layer — the boundary a clean product split gives for free (misclassification, wrong-discipline leakage, mid-conversation handoff). -**Conclusion:** the extension *primitive* is the skill. Agents are *orchestration shells* the platform owns — like Claude Code's `Explore` / `Plan` subagents, spawned by the platform, never authored by tenants. +**What ADR-0040 got right, and we keep:** the end user must not face a roster/dropdown of agents. We preserve that — but the resolution is **surface binding**, not a classifier and not a menu. --- ## Decision -### 1. Two kernel shells, split by what they mutate +### 1. Two agents, two products, bound by surface -| | `ask` shell | `build` shell | -|---|---|---| -| Claude analogue | Chat | Code | -| **Mutates** | records (business data) | metadata (the app definition) | -| Governance | row-level security; end-user permission | draft → verify → publish; builder/admin permission | -| Persona | Business Application Assistant | Schema architect | -| **Packaging / commerce** | **open-source · free** (`@objectstack/service-ai`) | **cloud-only · paid** (`../cloud/service-ai-studio`) | +The two agents above are platform-owned. The agent is resolved deterministically from **where the user is**, never chosen per-turn or from a roster: -The seam is **"what you change," not "what domain you are in."** Both shells share the same metadata ontology; they differ only in mutation target, risk surface, permission model, and verification discipline. That single seam carries the architectural boundary, the governance boundary, **and** the commercial boundary at once — three lines, one cut. +- Data console / embedded app chat → **`ask`**. +- Builder / Studio authoring surface → **`build`**. +- Resolution chain stays `app.defaultAgent` → surface default → platform default; the surface sets the default, so the user makes **no** selection. +- No per-turn intent classifier. A `build`-shaped request arriving at an `ask` surface is **declined and redirected to the builder**, not silently re-routed into authoring. -This is why the count is exactly two: merging them blurs a real governance boundary (record edits vs. app-definition edits have different blast radius and audiences); splitting further re-creates the roster ADR-0040 killed. +Why exactly two: merging them blurs the records-vs-app-definition governance boundary (different blast radius, audience, verification discipline, price); splitting further re-creates the roster this ADR refuses. ### 2. Skills (+ tools / MCP) are the only third-party extension primitive -- Third parties author `*.skill.ts` and contribute tools / MCP connectors. They do **not** author agents. -- `*.agent.ts` is **closed to third parties.** It remains an internal definition surface for platform-owned shells and subagents only. -- ADR-0040 §3 ("custom agents are a builder/admin feature", `app.defaultAgent` binding of tenant agents) is **withdrawn**. There is no commercial path for a tenant to define a new agent species that would have to live inside a paid platform component anyway. +- The platform is metadata-driven, so domain *structure* is already shared: both agents read the full metadata registry (objects, fields, actions, flows) and are structurally domain-aware for free. "A custom agent is needed to teach the assistant the industry" is false — the ontology is in the metadata. +- What is left over — persona, judgment, playbooks, instructions, bundled tools, external reach — is **exactly the definition of a skill**. Calling that a "custom agent" is using the wrong word for a skill. +- **Skills compose; agents don't.** A real request can span domains (CRM + accounting + an ISV connector); skill-first loads every relevant skill into the same turn, agent-first forces one. **Skills scale; agents don't** — progressive disclosure makes 1,000 skills cost ~nothing until relevant; 1,000 agents is a selection nightmare. +- Therefore: third parties author `*.skill.ts` and tools / MCP connectors. `*.agent.ts` is **closed to third parties** — internal only, for the two platform agents and platform-owned subagents (the `Explore`/`Plan` analogues). **ADR-0040 §3 (tenant custom agents bound via `app.defaultAgent`) is withdrawn.** -### 3. Skill ↔ shell affinity (the one new contract) +### 3. Skill ↔ agent affinity (the one new contract) -For relevance-routing to work, a skill must declare which shell(s) it applies to: +Because the two agents are distinct, each skill declares which one it belongs to: ```ts defineSkill({ @@ -77,13 +82,11 @@ defineSkill({ - `metadata_authoring`, `solution_design` → `build` - A cross-cutting ISV domain skill → usually `ask`, occasionally `both` -This is the analogue of a Claude Code skill being scoped to the Code surface. Without affinity, the `build` shell drowns in data-analysis skills and vice versa. +This is the analogue of a Claude Code skill being scoped to the Code surface. Affinity is also what makes tool scoping clean (ADR-0064): an agent's tool set is the union of its skills' tools, and a skill cannot attach to an agent whose surface it does not match. -### 4. Naming — one canonical word per shell, by user intent +### 4. Naming — one canonical word per agent, by user intent -The canonical ids are the **verb of user intent**, one word per shell, used everywhere: - -| | data shell | authoring shell | +| | data agent | authoring agent | |---|---|---| | **Canonical id / spoken name** | **`ask`** | **`build`** | | Legacy alias (permanent, silent — back-compat only) | `data_chat` | `metadata_assistant` | @@ -91,44 +94,42 @@ The canonical ids are the **verb of user intent**, one word per shell, used ever Rules: -- Name by **user intent (a verb)**, not by domain noun. `ask` + `build` share one axis ("ask my app" / "build my app"); `data` + `build` would mix a noun with a verb and the ambiguity grows back. -- `ask` is preferred over `data` because the shell also executes actions — "ask → answer → act" describes it; "data" implies read-only. -- The commercial line reads naturally off the names: **Ask your app (free) / Build your app (paid).** -- **"data agent" / "metadata assistant" are aliases, never vocabulary.** Documentation and code use `ask` / `build` exclusively. ADRs describe the seam ("`ask` mutates records, `build` mutates metadata") using the ids — they do not introduce a parallel "data agent" label. -- Do **not** re-rename ids (no `ask` → `data` churn). The fix for today's ambiguity is eliminating the residual "data-chat"口径 drift in code/files, not another rename. - -### 5. Tier-aware routing +- Name by **user intent (a verb)**: `ask` + `build` share one axis ("ask my app" / "build my app"). `data` + `build` would mix a noun with a verb and the ambiguity grows back. +- `ask` over `data` because the agent also executes actions — "ask → answer → act"; "data" implies read-only. +- The commercial line reads off the names: **Ask your app (free) / Build your app (paid).** +- `data_chat` / `metadata_assistant` are **aliases, never vocabulary**; docs and code use `ask` / `build` exclusively. +- **`build` is not yet canonical in code** — the cloud agent id is still `metadata_assistant`. Make `build` canonical there, mirroring the `ask` rename. No further id churn after that. -ADR-0040's per-turn intent routing must not assume both shells exist: +### 5. Surface binding, not per-turn routing; tier degradation -- On the **open-source / free** deployment only the `ask` shell is present. A `build`-intent turn must degrade gracefully ("authoring needs the cloud Build assistant"), not dead-end on a missing shell. -- **`build`-affinity skills only light up where the `build` shell exists** (cloud). Skill authors must know a `surface:'build'` skill is inert on OSS. This is intentional tiering, not a bug — but it must be documented at the authoring contract. -- Therefore the **free `ask` shell + open `ask`-skill ecosystem must stand on its own.** The free-tier value story rests entirely on it; if `ask` alone is weak, the tiering is hollow. +- The **surface** picks the agent (§1). There is no per-turn intent classifier to maintain, mis-tune, or mis-fire. +- On the **open-source / free** deployment only `ask` (and its `surface:'ask'` skills) exists. The builder surface and `build` agent simply aren't present; an `ask`-surface user who asks to build the app is told app-building lives in the (cloud) Builder, with no half-built attempt. +- **`build`-affinity skills only light up where the `build` agent exists** (cloud). A `surface:'build'` skill is inert on OSS by design — documented at the authoring contract, not a bug. +- Therefore the **free `ask` agent + open `ask`-skill ecosystem must stand on its own.** The free-tier value story rests entirely on it. --- ## Consequences **Positive** -- One mental model end-to-end: two shells, everything else is a skill. The "agent zoo" / choice-confusion failure mode is designed out, not managed. -- The capability-boundary gap ADR-0040 §4 worried about (registry-global tools, no per-agent constraint) **largely dissolves**: there is no tenant agent to constrain. Tools are scoped by *shell* (platform-owned) + *skill affinity*, not by a tenant-authored agent. §4's tool-scoping is reframed from "make custom agents safe" to "scope tools to the two shells." +- One mental model: two products, bound by surface; everything else is a skill. The "agent zoo" / choice-confusion failure mode is designed out, and so is the fragile per-turn classifier. +- The capability-boundary gap ADR-0040 §4 worried about narrows sharply: there is no tenant agent to constrain, and an agent's tools are exactly its skills' tools. Residual cross-agent sharing (read-only `describe_object` / `list_objects`) is handled by ADR-0064. - Architecture, governance, and pricing align on a single seam. **Negative / costs** -- Withdraws a feature ADR-0040 proposed; any tenant-agent assumptions in `../objectui` / Studio must be removed. -- Adds the `surface` affinity field to the skill schema and a routing change so each shell only loads its skills. -- Requires the residual-naming cleanup below to actually remove the ambiguity. +- Reverses ADR-0040's core: the unified all-skills `ask`, the per-turn intent classifier, and the `buildRegisterActive` degradation shim are removed (see cleanup). +- Withdraws tenant custom agents: flip the `agent` metadata-type flags and filter the runtime catalog. +- Adds the `surface` affinity field to the skill schema and the scoping work in ADR-0064. --- -## Follow-up: residual-naming cleanup (separate small PR) - -The `ask` id is already canonical, but code/files still speak "data-chat", which is the physical source of the id≠name ambiguity. Clean it up: +## Follow-up work (tracked in the consolidated issue) -- [ ] `packages/services/service-ai/src/agents/data-chat-agent.ts` → `ask-agent.ts` (keep the export surface; update `index.ts`). -- [ ] `DEFAULT_DATA_AGENT_NAME` → `ASK_AGENT_NAME` (retain `LEGACY_DATA_AGENT_NAME = 'data_chat'` as the alias constant). -- [ ] Sweep docs/prose for "data agent" / "Data Assistant" → "ask agent" / the `ask` id; keep `data_chat` only in the alias table. -- [ ] Add `surface: 'ask' | 'build' | 'both'` to the skill schema; backfill the four built-in skills. -- [ ] Make per-turn routing tier-aware (graceful `build`-intent degradation when the `build` shell is absent). +Implementation, none of which changes a *public* id: -These are mechanical and independently shippable; none changes a public id. +- [ ] **Split the personas.** Remove the unified `ask`-carries-all-skills definition and the per-turn intent preamble; `ask` carries only `surface:'ask'` skills, `build` only `surface:'build'`. Delete the `buildRegisterActive` degradation shim in `agent-runtime.ts` (no longer needed once surfaces are separate). +- [ ] **Make `build` canonical** in `../cloud/service-ai-studio` (`metadata_assistant` → `build`), registering `metadata_assistant` as the permanent alias — mirroring `data_chat`→`ask`. +- [ ] **Finish the `ask` rename drift**: `service-ai/src/agents/data-chat-agent.ts` → `ask-agent.ts`; `DEFAULT_DATA_AGENT_NAME` → `ASK_AGENT_NAME` (keep `LEGACY_DATA_AGENT_NAME`). Prose sweep "Data/Metadata Assistant" → `ask`/`build`. +- [ ] **Withdraw tenant agents**: set `agent` metadata-type `allowRuntimeCreate:false, allowOrgOverride:false` in `metadata-plugin.zod.ts`; filter custom agents from the runtime catalog; drop the `app.defaultAgent` custom-agent guidance in the cloud plugin. +- [ ] **Add `surface: 'ask' | 'build' | 'both'`** to `SkillSchema`; backfill the four built-in skills. +- [ ] **Bind agent → surface** in `../objectui` (data console → `ask`, Studio → `build`); keep the picker hidden/builder-only. diff --git a/docs/adr/0064-tool-scoping-to-agent.md b/docs/adr/0064-tool-scoping-to-agent.md new file mode 100644 index 0000000000..68ed4db636 --- /dev/null +++ b/docs/adr/0064-tool-scoping-to-agent.md @@ -0,0 +1,80 @@ +# ADR-0064: Tool scoping — an agent's tools are exactly its skills' tools + +**Status**: Proposed (2026-06-22) +**Deciders**: ObjectStack Protocol Architects +**Builds on**: [ADR-0063](./0063-two-kernel-agents-skills-are-the-extension-primitive.md) (two agents bound by surface; skill ↔ agent affinity), [ADR-0033](./0033-ai-assisted-metadata-authoring.md) (draft-gated authoring tools) +**Resolves**: [ADR-0040 §4](./0040-unified-assistant-and-agent-binding.md) ("tools are registry-global; a custom agent can change persona but not constrain capability") — the named follow-up contract. +**Consumers**: `@objectstack/service-ai` (ToolRegistry, AgentRuntime), `../cloud/service-ai-studio` (authoring tools), `@objectstack/spec` (no schema change required) + +--- + +## TL;DR + +Today every agent can see every registered tool: the `ToolRegistry` is **global**, and the `actions_executor` skill deliberately **falls through** to the global list. That is the ADR-0040 §4 gap — the reason a data assistant *could* author metadata. + +With [ADR-0063](./0063-two-kernel-agents-skills-are-the-extension-primitive.md) settling **two distinct agents bound by surface**, the fix is structural, not a new permission system: + +> **An agent's available tools = the union of the tools declared by the skills bound to that agent. Nothing falls through to the global registry.** + +The registry stays global for *registration*; *resolution* is scoped by the agent's skill bundle, which is itself scoped by `surface` affinity. `ask` literally has no authoring tools in its tool set, so it cannot author — by construction, not by a runtime check. + +--- + +## Context + +Findings from the 2026-06 ask/build assessment, confirmed across three packages: + +- **One global `ToolRegistry` per AI service** (`service-ai/src/tools/tool-registry.ts`); `register()` takes no agent/skill/surface argument. +- **`AgentRuntime.buildRequestOptions()`** composes an agent's tools from `agent.tools[]` + each active skill's `tools[]`, deduped — *but*: +- **`actions_executor` intentionally declares an empty `tools: []` and relies on a fall-through** so the resolver hands it the whole `action_*` set from the global registry (`service-ai/src/skills/actions-executor-skill.ts`). This fall-through is the hole: any agent loading that skill — or any resolver bug — exposes the global list. +- **Cloud authoring tools register globally** (`service-ai-studio/src/plugin.ts`); `describe_object` / `list_objects` are explicitly commented as shared by both agents. + +So scoping exists only by convention (which skills an agent happens to carry), with a deliberate bypass. ADR-0040 §4 flagged this; ADR-0063 makes it fixable cleanly because there is now no tenant agent and each skill has a single-surface home. + +## Decision + +### 1. Resolution is closed over the skill bundle + +`AgentRuntime` computes an agent's tool set strictly as: + +``` +tools(agent) = ⋃ { skill.tools | skill ∈ agent.skills ∧ skill.surface ∈ {agent.surface, 'both'} } +``` + +No tool reaches the model unless a bound, surface-compatible skill names it. **Remove the global fall-through.** Wildcards stay, but resolve against the registry *filtered to the names a skill claims*, not the whole registry: + +- `actions_executor` declares `tools: ['action_*']` and the resolver expands `action_*` against registered tools — but only because the skill *claims* the pattern, not via fall-through. The empty-array + fall-through hack is deleted. + +### 2. Registry stays global; a tool may be claimed by many skills + +Tools are still registered once on the shared registry (no per-agent registration, no duplication). Scoping is a *read-time filter*, not a partitioned store. A tool can be claimed by skills on different surfaces — that is how genuinely shared read tools work: + +- `describe_object`, `list_objects`, `query_data` are claimed by a `surface:'both'` **`schema_reader`** skill (or listed in both `data_explorer` and a build skill). They are read-only and safe to share. +- Authoring/mutation tools (`create_metadata`, `apply_blueprint`, `add_field`, …) are claimed **only** by `surface:'build'` skills. They are therefore absent from `ask`'s tool set on every deployment, and absent entirely on OSS where no `build` skill is registered. + +### 3. Affinity is enforced at bind time + +A skill whose `surface` is incompatible with an agent is a **load error**, not a silent drop: binding a `surface:'build'` skill to `ask` fails fast in `resolveActiveSkills`. This makes "ask can't author" a checked invariant, not an emergent property. + +### 4. No new schema + +This needs no field beyond ADR-0063's `skill.surface`. `ToolSchema` gains **no** `scopedToSurface` field — scoping is derived (tool ← skill ← agent), not declared on the tool. Declaring it on the tool too would create a second source of truth that can disagree with the skill bundle. (Tool-level `permissions` for RLS/role checks is orthogonal and stays.) + +## Consequences + +**Positive** +- Closes the ADR-0040 §4 hole structurally: `ask` cannot author because authoring tools are not in its set — no runtime guard to forget. +- Single source of truth for "who can call what": the skill bundle. No tool-side scoping to drift. +- Shared read tools remain shared, explicitly, via a `surface:'both'` reader skill. + +**Negative / costs** +- Deletes the `actions_executor` fall-through and the global-list reliance; any resolver path depending on fall-through must be migrated to explicit claims. +- Requires auditing currently-global tools into surface-correct skills (esp. the shared read tools → a `both` reader skill). +- A misfiled skill `surface` now silently narrows or widens an agent's reach — so `surface` correctness is load-bearing and must be covered by tests. + +## Follow-up work (consolidated issue) + +- [ ] Remove the global fall-through in tool resolution; resolve wildcards against skill-claimed names only. +- [ ] Introduce a `surface:'both'` `schema_reader` skill owning `describe_object`/`list_objects`/`query_data`; stop dual-listing by comment. +- [ ] Make incompatible skill↔agent binding a fast load error in `resolveActiveSkills`. +- [ ] Tests: assert `tools(ask)` contains no `create_*`/`*_metadata`/blueprint tools on cloud and OSS; assert authoring tools present only under `build`. diff --git a/docs/audits/2026-06-ask-build-agent-development-assessment.md b/docs/audits/2026-06-ask-build-agent-development-assessment.md new file mode 100644 index 0000000000..4ed2d13709 --- /dev/null +++ b/docs/audits/2026-06-ask-build-agent-development-assessment.md @@ -0,0 +1,107 @@ +# Assessment: `ask` / `build` agent development surface (2026-06-22) + +**Type**: development assessment / landscape (NOT an ADR — this routes work, it does not decide). +**Scope**: every code touchpoint for the two kernel agents across `@objectstack/service-ai` (OSS `ask`), `@objectstack/service-ai-studio` (cloud `build`), `@objectstack/spec` (agent/skill/tool schemas), and `../objectui` (chat surface). +**Frames**: [ADR-0040](../adr/0040-unified-assistant-and-agent-binding.md), [ADR-0063](../adr/0063-two-kernel-agents-skills-are-the-extension-primitive.md), [ADR-0064](../adr/0064-tool-scoping-to-agent.md). +**Method**: four parallel read-only inventories. File:line references below are accurate as of this worktree. + +> **Outcome (see §5):** D2 resolved to **two distinct agents bound by surface**; ADR-0040's unified-assistant core is reversed. + +--- + +## 1. Touchpoint inventory + +Status legend — **D** decided (cite ADR) · **O** open decision (needs an ADR) · **I** implementation (issue/PR). + +### `ask` agent — OSS `packages/services/service-ai/` +| Touchpoint | File | Status | Landing | +|---|---|---|---| +| `ask` agent record (id `ask`, role "Business Application Assistant", **4 skills listed**, guardrails, react planning) | `src/agents/data-chat-agent.ts:42` | D | ADR-0063 | +| Boot UPSERT of agent + `data_explorer`/`actions_executor` skills | `src/plugin.ts:823` | D | — | +| Alias resolution `data_chat`→`ask` (per-plugin `registerAgentAlias`) | `src/agents/agent-aliases.ts:24` | D | — | +| File still named `data-chat-agent.ts`; `DEFAULT_DATA_AGENT_NAME` const | `src/agents/data-chat-agent.ts` | I | issue | +| `data_explorer` / `actions_executor` skills (no `surface` field) | `src/skills/*.ts` | I | ADR-0063 + issue | +| **Global** ToolRegistry — no per-agent/shell scoping | `src/tools/tool-registry.ts:49` | D→I | **ADR-0064** | +| Intent routing is LLM-side (prompt directive) — to be removed | `src/agents/data-chat-agent.ts:52` | I | ADR-0063 (reversed) | +| `buildRegisterActive` degradation shim — to be deleted | `src/agent-runtime.ts:179` | I | ADR-0063 | +| Agent access gate + daily message quota | `src/routes/agent-routes.ts:147`, `src/quota/agent-chat-quota.ts` | D | — | + +### `build` agent — cloud `packages/service-ai-studio/` +| Touchpoint | File | Status | Landing | +|---|---|---|---| +| Build agent record — **canonical id still `metadata_assistant`** (role "Schema Architect", 2 skills, no tools) | `src/agents/metadata-assistant-agent.ts:26` | I | issue (rename → `build`) | +| `metadata_authoring` skill (12 authoring tools, draft-gated, ADR-0038 discipline) | `src/skills/metadata-authoring-skill.ts:18` | D | — | +| `solution_design` skill (blueprint propose/apply, plan-first) | `src/skills/solution-design-skill.ts:15` | D | — | +| All authoring tools registered **globally**; `describe_object`/`list_objects` explicitly shared with `ask` | `src/plugin.ts`, `src/tools/*.tool.ts` | D→I | **ADR-0064** | +| Plugin attaches on `ai:ready`; absent on OSS → authoring dark, manual draft still works | `src/plugin.ts:56` | D | ADR-0063 §5 | +| Blueprint object quota (`AI_STUDIO_MAX_BLUEPRINT_OBJECTS`, default 20) | `src/tools/blueprint-tools.ts:84` | D | — | +| Comment: tenants customize via **custom agent bound through `app.defaultAgent`** | `src/plugin.ts:274` | D | ADR-0063 §2 (withdraw) | + +### `@objectstack/spec` +| Touchpoint | File | Status | Landing | +|---|---|---|---| +| AgentSchema (no `surface`; guardrails agent-level only) | `src/ai/agent.zod.ts:130` | D | — | +| SkillSchema — **no `surface` field** | `src/ai/skill.zod.ts:57` | I | ADR-0063 + issue | +| ToolSchema — no shell/agent scoping field (and none needed) | `src/ai/tool.zod.ts:63` | D | ADR-0064 §4 | +| Registry: `agent` type `allowRuntimeCreate:true, allowOrgOverride:true` | `src/kernel/metadata-plugin.zod.ts:684` | D→I | ADR-0063 §2 (flip false) | +| No deployment guardrail-floor schema | (absent) | O | defer (D4) | + +### `../objectui` chat surface +| Touchpoint | File | Status | Landing | +|---|---|---|---| +| Agent resolution: `app.defaultAgent` → `ask` → first active; alias-aware | `packages/plugin-chatbot/src/useAgents.ts:71` | D | — | +| Alias groups `['ask','data_chat']`, `['build','metadata_assistant']` | `packages/plugin-chatbot/src/agentAliases.ts:22` | D | — | +| Agent picker — **hidden by default**, gated by `VITE_AI_SHOW_AGENT_PICKER` | `packages/app-shell/src/layout/ConsoleFloatingChatbot.tsx:486` | D | ADR-0040 (kept) | +| No "create agent" UI; AgentPreview read-only; catalog routes to custom agents by name | `.../metadata-admin/previews/AgentPreview.tsx` | D/I | issue | +| Two surfaces `/ai/ask` and `/ai/build` with distinct empty-states | `.../console/ai/AiChatPage.tsx:176` | D | ADR-0063 §1 (surface binding) | +| `aiStudio` flag off → build agent suppressed, FAB falls back to `ask` | `.../layout/ConsoleLayout.tsx:79` | I | — | + +--- + +## 2. Findings + +**F1 — Two coexisting agent models, partly contradictory (the headline).** ADR-0040 ships a *unified* assistant: the `ask` record lists **all four** skills and degrades to data-only when build skills aren't registered (`agent-runtime.ts:179`). But the cloud package *also* ships a **separate** `metadata_assistant` agent with only the two build skills, pinned by Studio. So "build vs data" exists simultaneously as (a) two registers inside one `ask` agent and (b) two distinct agent records. **This forced the D2 decision** (§5). + +**F2 — Tool scoping is genuinely absent everywhere.** Every repo confirms a single **global** `ToolRegistry`; any agent can see any registered tool; `actions_executor` deliberately falls through to the global list. ADR-0040 §4 / "scope tools to the agents" is **unbuilt** → ADR-0064. + +**F3 — Naming drift is two-sided.** `ask` is canonical but the file is still `data-chat-agent.ts`. **`build` is NOT canonical in code** — the cloud id is still `metadata_assistant`; `build` exists only as a UI/alias. The rename must cover the *build* side too. + +**F4 — Tenant custom agents are fully wired today.** `agent` type is `allowRuntimeCreate:true, allowOrgOverride:true`; the cloud plugin documents `app.defaultAgent` custom binding; objectui routes to catalog custom agents (no create UI). Withdrawing them is a real change. + +**F5 — Tier degradation already partially exists.** The `buildRegisterActive` gate self-constrains `ask` on OSS — but it is the unified-model shim and is removed under D2 (surfaces are separated instead). + +**F6 — The "guardrail floor" is asserted but unimplemented.** Only per-agent guardrails exist; no deployment/tier floor. Lower priority once tenant agents are withdrawn (no tenant agent to floor). + +--- + +## 3. Open decisions → where they landed + +| # | Decision | Verdict | Container | +|---|---|---|---| +| **D2** | unified assistant (0040) vs two distinct agents | **RESOLVED → two distinct agents, surface-bound** | ADR-0063 (revised) | +| **D1** | tool scoping mechanism | resolved as "tools = skills' tools" | **ADR-0064** | +| D3 | OSS↔cloud composition | folds into D2 (separate agents, package-gated) | ADR-0063 §5 | +| D4 | deployment guardrail-floor schema | defer (low pri post-F4) | later | +| D5 | `surface: 'ask'\|'build'\|'both'` skill field | adopted | ADR-0063 §3 + issue | +| I1 | rename drift (both sides), prose sweep | impl | issue | +| I2 | withdraw tenant agents (flip flags, filter catalog) | impl | issue | +| I3 | split personas; surface-binding in objectui | impl | issue | + +--- + +## 4. Recommendation (historical — superseded by §5) + +Originally floated "one assistant, two tiers." **Not taken** — see §5. + +--- + +## 5. Decision taken (2026-06-22) + +D2 resolved **against** the "one assistant, two tiers" option §4 floated. Rationale (architect): Claude Chat and Claude Code are deliberately **separate products for different needs**, and ADR-0040's unified assistant + per-turn intent classifier was the design error — its own staging incident is evidence the *surfaces leaked*, not that the personas should merge. + +**Resolution: two distinct agents (`ask` / `build`), bound by surface.** ADR-0040's unified-assistant core is reversed; "user never picks a roster" is kept but re-grounded as *surface binding*. See: + +- **[ADR-0063](../adr/0063-two-kernel-agents-skills-are-the-extension-primitive.md)** (revised) — two agents bound by surface; skills-only third-party extension; `surface` affinity; naming; supersedes ADR-0040 core + §3. +- **[ADR-0064](../adr/0064-tool-scoping-to-agent.md)** — tool scoping: an agent's tools = its skills' tools (resolves ADR-0040 §4). + +Implementation (split personas, make `build` canonical, finish `ask` rename, withdraw tenant agents, `surface` field, surface-binding in objectui) is consolidated in the follow-up issue. D4 (guardrail floor) remains deferred.