From d6e073fff18e29e2dbfc3ce5a7c138198559a52f Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 11 Sep 2026 06:38:31 +0000 Subject: [PATCH 1/3] feat(mcp): contribute Connect an Agent into the Account app nav MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `POST /api/v1/keys` mints a `sys_api_key` bound to the caller and the page says the key "acts as you", but the only nav entry sat in Setup behind `requiredPermissions: ['setup.access']` — so every non-admin following the two-step guide stopped at step 1 while the endpoint behind the button accepted them all along. Adds a second `navigationContributions` entry in the same bundle, targeting the `account` app's `grp_account_developer` group beside the `nav_account_api_keys` entry already shipping there. The Setup entry stays for admins. Backend, authorization and the published "acts as you" promise do not move, and no gate of any kind is added or removed: a contribution registers exactly when the page registers. Claude-Session: https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c Co-authored-by: Claude --- packages/mcp/src/connect-ui.ts | 65 ++++++++++++++++++++++++++++++++-- 1 file changed, 63 insertions(+), 2 deletions(-) diff --git a/packages/mcp/src/connect-ui.ts b/packages/mcp/src/connect-ui.ts index 42ada6a679..886bd3a7a7 100644 --- a/packages/mcp/src/connect-ui.ts +++ b/packages/mcp/src/connect-ui.ts @@ -1,7 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. /** - * "Connect an agent" Setup page — plugin-carried UI metadata (#2714 Phase 1, + * "Connect an agent" page — plugin-carried UI metadata (#2714 Phase 1, * objectui#2363). * * The page ships WITH the MCP capability (same principle as the marketplace @@ -16,6 +16,29 @@ * Registered by {@link MCPServerPlugin} on `kernel:ready`, gated on the same * default-on switch as the HTTP surface — an opted-out deployment * (`OS_MCP_SERVER_ENABLED=false`) gets no page and no nav entry. + * + * ## Two nav entry points, one page (#16746) + * + * `POST /api/v1/keys` mints a `sys_api_key` bound to the CALLER, and this page + * says the key "acts as you" — so the surface that mints it is a per-user + * credential surface, not an administrative one. Reached only through Setup it + * could not keep that promise: Setup declares `requiredPermissions: + * ['setup.access']`, so every non-admin following the two-step guide stopped at + * step 1 while the endpoint behind the button accepted them all along. + * + * The fix is a SECOND contribution, into the `account` app's Developer group + * beside the `nav_account_api_keys` entry already shipping there. The Setup + * entry stays for admins. ⛔ Ungating Setup is NOT the fix and was measured: + * the app-level `setup.access` gate fires BEFORE the group gate, so dropping + * the group gate alone changes nothing, and dropping both serves 14+ unrelated + * Setup surfaces (Users, Organization, Branding, Feature Flags, …) to every + * signed-in user — far past what this card asks for. ⛔ Nor is a + * `requiresService: 'mcp'` gate on an `account.app.ts` entry: the `mcp` service + * registers unconditionally in `init()` while this bundle registers behind + * `isMcpServerEnabled()`, so such an entry outlives its page and 404s on an + * opted-out deployment. A contribution registers exactly when the page + * registers, which is why both entries live in THIS bundle and neither carries + * a gate of its own. */ import type { Page } from '@objectstack/spec/ui'; @@ -59,8 +82,22 @@ export const CONNECT_AGENT_UI_BUNDLE = { type: 'plugin', scope: 'system', name: 'Connect an Agent UI', - description: 'Setup page + navigation for connecting MCP clients to this environment.', + description: 'Connect-an-Agent page + Setup and Account navigation for connecting MCP clients to this environment.', pages: [CONNECT_AGENT_PAGE], + // Both entries point at the one `connect_agent` page and share the item id. + // That is legal and deliberate, read off the fold rather than assumed: the + // registry keys contributions by TARGET APP + // (`appNavContributions: Map`) and `applyNavContributions(app)` + // consults only `get(app.name)`, so a nav item id is unique within one app's + // navigation tree and nothing indexes it across apps — no id-keyed registry, + // no de-duplication by id, and the translation bundles are keyed + // `apps..navigation.` (per-app namespaces, so one id yields two + // distinct keys). Sharing it keeps ONE identity for one destination. + // + // ⛔ The two items are separate object literals, not one shared const: the + // fold `structuredClone`s the APP but pushes `...c.items` by reference, so a + // shared literal would put the same object in two apps' navigation trees and + // any in-place consumer edit would leak across them. navigationContributions: [ { app: 'setup', @@ -76,5 +113,29 @@ export const CONNECT_AGENT_UI_BUNDLE = { }, ], }, + { + // The per-user half. `ACCOUNT_APP` declares no `requiredPermissions` — + // deliberately, so every authenticated user reaches their own security + // surface — and `grp_account_developer` already carries + // `nav_account_api_keys`. Targeted by NAME; `packages/platform-objects` + // is not edited. + app: 'account', + group: 'grp_account_developer', + // `priority` orders contributions among THEMSELVES within the group, and + // the group's own static children always precede them. Stated rather + // than defaulted, at the schema's declared default: no other package + // contributes into this group today, so there is nothing to interleave + // with and no reason to claim a position. + priority: 200, + items: [ + { + id: 'nav_connect_agent', + type: 'page', + pageName: 'connect_agent', + label: 'Connect an Agent', + icon: 'bot', + }, + ], + }, ], }; From 5bfe2e6dd57b907723326cfe04597418339a8695 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 11 Sep 2026 06:54:32 +0000 Subject: [PATCH 2/3] test(mcp): pin the Account-app Connect-an-Agent contribution and its Setup guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pins the half this package owns: the contribution is aimed at the ungated `account` app / `grp_account_developer` group, its item carries nothing the server-side nav filter could strip for a permissionless caller, the Setup entry is byte-unchanged, and — the load-bearing one — this bundle declares no permission key and no `apps` collection, so it cannot reach the card by widening Setup instead. Both contributions are parsed against the real `NavigationContributionSchema`, which is what makes the shared item id an accepted fact rather than an unenforced one. Claude-Session: https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c Co-authored-by: Claude --- .../mcp/src/connect-agent-account-nav.test.ts | 171 ++++++++++++++++++ 1 file changed, 171 insertions(+) create mode 100644 packages/mcp/src/connect-agent-account-nav.test.ts diff --git a/packages/mcp/src/connect-agent-account-nav.test.ts b/packages/mcp/src/connect-agent-account-nav.test.ts new file mode 100644 index 0000000000..f71f47e376 --- /dev/null +++ b/packages/mcp/src/connect-agent-account-nav.test.ts @@ -0,0 +1,171 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #16746 — the Connect-an-Agent page must be reachable by the principal the +// page's own promise is about. +// +// `POST /api/v1/keys` mints a `sys_api_key` bound to the CALLER and the page +// says the key "acts as you". Reached only through Setup it could not keep that +// promise: `SETUP_APP` declares `requiredPermissions: ['setup.access']`, so +// every non-admin following the shipped two-step guide — and the runtime's own +// error text (`plugin.ts`, `README.md`: "mint a key in Setup → Connect an +// Agent") — stopped at step 1 while the endpoint behind the button accepted +// them all along. Maintainer ruling 2026-09-08, option A: open the key card to +// every signed-in user; backend, authorization and the published promise do +// not move. +// +// --------------------------------------------------------------------------- +// What this file pins, and what it deliberately does NOT claim +// --------------------------------------------------------------------------- +// ⚠️ The acceptance criterion is a WIRE fact about two apps — a permissionless +// principal sees `nav_connect_agent` in the `account` app's nav while +// `GET /api/v1/meta/apps/setup` keeps answering 403 PERMISSION_DENIED — and +// that fact cannot be measured from this package: `@objectstack/mcp` declares +// no dependency on `@objectstack/rest` (the RBAC-by-route harness), on +// `@objectstack/objectql` (`SchemaRegistry.applyNavContributions`, the fold) or +// on `@objectstack/platform-objects` (`SETUP_APP` / `ACCOUNT_APP`). ⛔ So this +// file does not reimplement any of them — a second copy of the fold is exactly +// the divergence `cli/src/utils/nav-contribution-groups.ts` refuses to write. +// +// What it pins instead is the half this package OWNS, stated as the two +// properties the wire fact rests on: +// +// 1. the contribution is aimed at the ungated app and group, and carries +// nothing the server-side nav filter could strip for a permissionless +// caller (`requiredPermissions` / `requiresService`); +// 2. this bundle cannot widen Setup — the accident that would make a +// "the entry is visible" assertion pass for the wrong reason. +// +// The second is the load-bearing one. Ungating Setup was measured on the real +// composition and refused: the app-level `setup.access` gate fires BEFORE the +// group gate (so dropping `group_integrations`' gate alone changes nothing), +// and dropping both serves 14+ unrelated Setup surfaces to every signed-in +// user. Nothing in this bundle may reintroduce that, so the walk below asserts +// the bundle declares no permission key anywhere and no `apps` collection that +// could redeclare `SETUP_APP` without its gate. + +import { describe, it, expect } from 'vitest'; +import { NavigationContributionSchema } from '@objectstack/spec/ui'; + +import { CONNECT_AGENT_UI_BUNDLE } from './connect-ui.js'; + +type AnyRec = Record; + +const contributions = CONNECT_AGENT_UI_BUNDLE.navigationContributions as AnyRec[]; +const byApp = (app: string): AnyRec[] => contributions.filter((c) => c.app === app); +const itemsOf = (c: AnyRec): AnyRec[] => (c.items ?? []) as AnyRec[]; + +/** Every key present anywhere in a value, depth-first — objects and arrays. */ +function everyKey(value: unknown, out: string[] = []): string[] { + if (Array.isArray(value)) { + for (const entry of value) everyKey(entry, out); + } else if (value && typeof value === 'object') { + for (const [key, child] of Object.entries(value)) { + out.push(key); + everyKey(child, out); + } + } + return out; +} + +describe('#16746 — Connect an Agent reaches the per-user Account app', () => { + it('contributes into the `account` app, into the group that already ships API Keys', () => { + const account = byApp('account'); + expect(account).toHaveLength(1); + // `ACCOUNT_APP` declares no `requiredPermissions` (deliberately: every + // authenticated user must reach their own security surface, RLS scopes the + // rows), and `grp_account_developer` is the group already carrying + // `nav_account_api_keys`. Both are read by NAME here — this card edits + // nothing in `@objectstack/platform-objects`. + expect(account[0]).toMatchObject({ app: 'account', group: 'grp_account_developer' }); + }); + + it('aims one `page` item at `connect_agent`, the page this same bundle registers', () => { + const items = itemsOf(byApp('account')[0]); + expect(items).toHaveLength(1); + expect(items[0]).toMatchObject({ + id: 'nav_connect_agent', + type: 'page', + pageName: 'connect_agent', + }); + // The destination must be a page this bundle actually ships, or the entry + // is a 404-when-clicked shown to every signed-in user — the precise defect + // that ruled out gating an `account.app.ts` entry on `requiresService: + // 'mcp'` (the service registers unconditionally in `init()`; this bundle + // registers behind `isMcpServerEnabled()`). + const pageNames = (CONNECT_AGENT_UI_BUNDLE.pages ?? []).map((p) => p.name); + expect(pageNames).toContain(items[0].pageName); + }); + + it('carries nothing the per-user caller could be filtered on', () => { + // THE criterion, stated at the layer this package owns. The server-side + // nav filter strips an item for a caller missing its `requiredPermissions` + // and for an absent `requiresService` capability. An entry carrying either + // would be invisible to exactly the principal this card is about — a + // permissionless one — while every assertion above still passed. + const item = itemsOf(byApp('account')[0])[0]; + expect(item).not.toHaveProperty('requiredPermissions'); + expect(item).not.toHaveProperty('requiresService'); + expect(item).not.toHaveProperty('visible'); + }); + + it('leaves the Setup entry exactly as it was — admins keep the page where the guide points', () => { + const setup = byApp('setup'); + expect(setup).toHaveLength(1); + expect(setup[0]).toMatchObject({ app: 'setup', group: 'group_integrations', priority: 110 }); + expect(itemsOf(setup[0])).toEqual([ + { id: 'nav_connect_agent', type: 'page', pageName: 'connect_agent', label: 'Connect an Agent', icon: 'bot' }, + ]); + }); + + it('⛔ cannot widen Setup — no permission key and no app redeclaration anywhere in the bundle', () => { + // The risk the green has to prove, not merely pass. A test asserting only + // "the account entry exists" would go green just as happily on a diff that + // reached the card by ungating Setup instead — measured to serve 14+ + // unrelated Setup surfaces (Users, Organization, Branding, Feature Flags, + // …) to every signed-in user. This bundle is the one file that changed, so + // it is where that accident would have to be written. + const keys = new Set(everyKey(JSON.parse(JSON.stringify(CONNECT_AGENT_UI_BUNDLE)))); + expect([...keys].filter((k) => k === 'requiredPermissions')).toEqual([]); + // An `apps: [...]` collection here could redeclare `SETUP_APP` — last + // registration wins — and drop its `setup.access` gate without touching + // `platform-objects` at all. + expect(CONNECT_AGENT_UI_BUNDLE).not.toHaveProperty('apps'); + // Every contribution aims at a named group. A contribution with no `group` + // appends at the app's TOP level, which for Setup would put the entry + // outside `group_integrations`' gate. + for (const c of contributions) expect(typeof c.group).toBe('string'); + }); + + it('shares the item id across the two apps, and the fold says that is scoped per app', () => { + // Answered from the fold rather than from taste, because a wrong answer + // here is a silent one. `SchemaRegistry` keys contributions by TARGET APP + // (`appNavContributions: Map`) and `applyNavContributions(app)` + // consults only `get(app.name)`, so a nav item id is unique within ONE + // app's navigation tree; nothing indexes it across apps (no id-keyed + // registry, no de-duplication by id), and the translation bundles are + // keyed `apps..navigation.`, which makes one shared id two + // distinct keys. One destination therefore keeps one identity. + const targets = contributions.map((c) => c.app); + expect(new Set(targets).size).toBe(targets.length); + expect(contributions.flatMap((c) => itemsOf(c).map((i) => i.id))).toEqual([ + 'nav_connect_agent', + 'nav_connect_agent', + ]); + // ⛔ …and they are two literals, not one shared const. The fold + // `structuredClone`s the APP but pushes `...c.items` BY REFERENCE, so one + // shared object would sit in two apps' navigation trees at once and any + // in-place consumer edit would leak from one app into the other. + expect(itemsOf(byApp('setup')[0])[0]).not.toBe(itemsOf(byApp('account')[0])[0]); + }); + + it('both contributions parse against the real spec contract', () => { + // The shared id is not merely unenforced — it is accepted by the schema + // that governs the surface, checked against the spec rather than asserted + // about it. + for (const c of contributions) { + const parsed = NavigationContributionSchema.safeParse(c); + expect(parsed.error?.issues ?? []).toEqual([]); + expect(parsed.success).toBe(true); + } + }); +}); From 0ae6542425b0cccc1029e71028f7d7e668ce6008 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 11 Sep 2026 07:12:32 +0000 Subject: [PATCH 3/3] chore(changeset): patch @objectstack/mcp for the Account-app Connect-an-Agent entry Claude-Session: https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c Co-authored-by: Claude --- .changeset/16746-connect-agent-account-nav.md | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 .changeset/16746-connect-agent-account-nav.md diff --git a/.changeset/16746-connect-agent-account-nav.md b/.changeset/16746-connect-agent-account-nav.md new file mode 100644 index 0000000000..f0808a8d62 --- /dev/null +++ b/.changeset/16746-connect-agent-account-nav.md @@ -0,0 +1,50 @@ +--- +'@objectstack/mcp': patch +--- + +Connect an Agent is reachable from the Account app, so a non-admin can mint their own key + +`POST /api/v1/keys` mints a `sys_api_key` bound to the **caller**, and the +Connect-an-Agent page says the key "acts as you". But the page's only navigation +entry sat in the Setup app, which declares `requiredPermissions: +['setup.access']` — so every non-admin following the shipped two-step guide, and +every reader of the runtime's own error text (`packages/mcp/src/plugin.ts`: +*"mint an API key (Setup → Connect an Agent, or POST /api/v1/keys)"*, and +`README.md`), stopped at step 1 while the endpoint behind the button had accepted +them all along. Measured before: a principal with no system permissions gets +`403 PERMISSION_DENIED` on `GET /api/v1/meta/apps/setup` and `nav_connect_agent` +is absent from the wire. + +`CONNECT_AGENT_UI_BUNDLE` now carries a **second** `navigationContributions` +entry, targeting the `account` app's `grp_account_developer` group beside the +`nav_account_api_keys` entry already shipping there. Measured after, over the +real composition (real `SETUP_APP` / `ACCOUNT_APP` / `SETUP_NAV_CONTRIBUTIONS`, +the real fold and the real RBAC-by-route filter): the same permissionless +principal gets `200` on `GET /api/v1/meta/apps/account` with +`grp_account_developer` carrying `['nav_account_api_keys', +'nav_account_oauth_apps', 'nav_connect_agent']`, while `apps/setup` still +answers `403 PERMISSION_DENIED` with `connect_agent` absent from that body. + +**Nothing else moves.** No backend change, no authorization change, no change to +which permissions exist, and the published "acts as you" promise is unchanged — +it simply becomes keepable for the users it was written for. The Setup entry +stays exactly as it was, so admins keep the page where the guide points, and no +gate is added or removed anywhere: a navigation contribution registers exactly +when the page registers, so an opted-out deployment +(`OS_MCP_SERVER_ENABLED=false`) still gets no page and neither entry. + +⛔ Ungating Setup was **not** the fix, and was measured rather than assumed: the +app-level `setup.access` gate fires before the group gate, so dropping the group +gate alone changes nothing, and dropping both serves 14+ unrelated Setup +surfaces (Users, Organization, Business Units, Branding, Feature Flags, …) to +every signed-in user. ⛔ Nor was a `requiresService: 'mcp'` gate on an +`account.app.ts` entry: the `mcp` service registers unconditionally in `init()` +while this bundle registers behind `isMcpServerEnabled()`, so such an entry +would outlive its page and 404 for every signed-in user on an opted-out +deployment. + +Both entries deliberately share the item id `nav_connect_agent` — one +destination, one identity. That is scoped, not a collision: `SchemaRegistry` +keys contributions by target app and `applyNavContributions(app)` consults only +that app's bucket, so a nav item id is unique within one app's navigation tree, +and the translation bundles are keyed `apps..navigation.`.