diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index ea201ed4e..3e567837b 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -18,6 +18,13 @@ entry. See `CONTRIBUTING.md` § Releases & changelog. ## [Unreleased] +### Fixed — service-grant gate covers legacy rows, plugin-facing callers, and per-plugin factories (#470 C2b, PR #783) + +- Filled the dated `ctx.services.get` legacy allowlist with the currently-real built-in and hub-plugin rows the first audit missed: some service names are hidden behind exported constants (`PROCESS_MEMORY_SERVICE_NAME`, `PLUGIN_CAPABILITIES_SERVICE`, `CHANNEL_RESOLVER_SERVICE`, …) and some channel repos resolve them through shared `@omadia/channel-sdk` helpers rather than a literal string in the plugin's own file. The boot-breaking orchestrator/orchestrator-extras gaps are now grandfathered explicitly until their manifests catch up. +- Added `test/pluginServiceGrantCoverage.test.ts`, which derives service reads from every built-in `middleware/packages/*/manifest.yaml` plus its `src/**/*.ts` call sites and fails loud on undeclared or stale legacy rows instead of trusting a hand-maintained snapshot. +- Threaded the plugin's `ServiceCaller` through plugin-facing accessors that resolved services outside `ctx.services.get` (`ctx.memory`, the knowledge-graph accessor, `ctx.mcp`, `ctx.subAgents`, `ctx.llm`, `ctx.events`) so `perCallerService(...)` providers see the consuming plugin instead of the kernel. +- Made `perCallerService(...)` truthful to its docs: one implementation is now memoized per consuming plugin and per factory object, so repeat reads by the same plugin reuse the same instance while a replaced provider starts cold automatically. + ### Fixed — verifier: hallucinated record references no longer pass with a disclaimer (#129, PR #781) - **Behaviour change (blocking).** A qualitative answer that names a concrete diff --git a/middleware/package-lock.json b/middleware/package-lock.json index 85a2389b4..79f6572c7 100644 --- a/middleware/package-lock.json +++ b/middleware/package-lock.json @@ -9169,7 +9169,7 @@ "peerDependencies": { "@omadia/api-key-auth": "^0.1.0", "@omadia/channel-sdk": "^0.1.0", - "@omadia/plugin-api": "^0.1.0", + "@omadia/plugin-api": "*", "express": "^5.1.0", "zod": "^4.0.0" } @@ -9569,7 +9569,7 @@ }, "packages/plugin-api": { "name": "@omadia/plugin-api", - "version": "0.1.0", + "version": "1.0.0", "license": "MIT", "engines": { "node": ">=20" diff --git a/middleware/packages/harness-channel-api/package.json b/middleware/packages/harness-channel-api/package.json index d1756ec28..cc019cdf7 100644 --- a/middleware/packages/harness-channel-api/package.json +++ b/middleware/packages/harness-channel-api/package.json @@ -14,7 +14,7 @@ "peerDependencies": { "@omadia/api-key-auth": "^0.1.0", "@omadia/channel-sdk": "^0.1.0", - "@omadia/plugin-api": "^0.1.0", + "@omadia/plugin-api": "*", "express": "^5.1.0", "zod": "^4.0.0" }, diff --git a/middleware/packages/plugin-api/CHANGELOG.md b/middleware/packages/plugin-api/CHANGELOG.md new file mode 100644 index 000000000..a5ec82599 --- /dev/null +++ b/middleware/packages/plugin-api/CHANGELOG.md @@ -0,0 +1,91 @@ +# Changelog — `@omadia/plugin-api` + +The type contract every omadia plugin compiles against. The package is +`private: true` and is not published to npm; plugin repositories consume it by +`file:` link or a vendored `.d.ts` (epic #470, decision D1). + +Versioning is SemVer over the **exported type surface**. Removing or narrowing +an exported type, or adding a required member to an interface a plugin +implements, is a major. + +## 1.0.0 — 2026-08-20 + +First stable cut of the contract. Two breaking changes are taken together, +deliberately, in one major — **now**, while the installed base is still zero +and every consumer is a repository we control. There is no published `0.x` +range on npm and no third-party plugin pinned to one, so the cost of the break +is a coordinated bump across the sibling repos rather than an ecosystem event. +Deferring it would only have made it expensive (epic #470, `implementation.md` +§1 row 4). + +### Breaking + +- **Removed the dev-platform job types and their context accessor.** No longer + exported (spelled out on one line, once, so a consumer grepping its own source finds this entry): `DevJobKind`, `DevJobStatus`, `DevJobDescriptor`, `DevJobCreateRequest`, `DevJobEventRecord`, `DevJobsAccessor`, `PluginContext.devJobs`. + + They were never usable. Nothing ever registered the backing host service, so + every call threw, and no manifest in this repository, in the private byte5 + plugin set, or in any sibling plugin repository ever declared the matching + permission (`specs/470-dev-platform-plugin/dormant-capabilities.md` §2). The + view types survive core-locally under `middleware/src/` and travel with the + extraction into its own repository, where the plugin will own them as + `@omadia/dev-platform-plugin-api`. They are deliberately not re-published + from here for zero consumers. + + *Migration:* none required — no working code can exist against a surface that + threw on every call. A stale manifest still declaring the legacy permission + key keeps installing and activating unchanged; unknown permission keys are + ignored, not rejected (regression-pinned in + `test/manifestDevJobsLegacyKey.test.ts`). + +- **`ctx.services.get(name)` is now gated on the manifest.** A plugin may only + resolve capability names it declares in `requires:` (or `provides:`, to read + back its own registration). An undeclared name throws the new + `ServiceNotDeclaredError` instead of returning the implementation. + + Previously the accessor was a bare pass-through: any installed plugin could + ask for any registered service — including `graphPool`, the same Postgres + pool the kernel uses — with no declaration and nothing in the install dialog + (epic #470, bug B1). + + *Migration:* add the capability to the manifest's `requires:` list, e.g. + `requires: ["graphPool@^1"]`. The service-registry key **is** the capability + name. A dated allowlist + (`LEGACY_UNDECLARED_SERVICE_GRANTS_2026_08_20` in + `middleware/src/platform/pluginServiceGrants.ts`) grandfathers the exact + (plugin, capability) pairs an audit found in shipped plugins: those warn once + and resolve. The allowlist is closed — a different plugin, or a different + name, still throws. + + *Note:* `requires:` is also the activation dependency, so declaring an + optionally-consumed capability makes it mandatory. Expressing an optional + requirement is an open design question and the reason the allowlist exists at + all rather than every row being fixed in place. + +- **`ServicesAccessor.provide` / `.replace` widened to + `T | PerCallerFactory`.** Source-compatible for every existing call; only + code that *implements* `ServicesAccessor` (the kernel, and test doubles that + type themselves against it) sees the change. + +### Added + +- `perCallerService(factory)` — register a service that mints one + implementation per consuming plugin. The factory receives a `ServiceCaller` + (`{ agentId, pluginId }`) built from the id the **kernel** activated the + consumer under, never from an argument the consumer supplies. This is what + lets a provider attribute, scope or filter per consumer without asking the + consumer to name itself — the self-attribution hole that removing the + accessor above would otherwise have opened (epic #470 §2.2). +- `ServiceCaller`, `PerCallerFactory`, `isPerCallerService`, + `resolvePerCallerService` — the supporting surface. The factory is a + symbol-branded object rather than a bare function, so a service that *is* a + function can never be mistaken for a factory. +- `ServiceNotDeclaredError` — typed, carrying `pluginId`, `capability` and + `manifestField`, so a plugin can tell "the operator has not installed a + provider" (`get` returns `undefined`) from "I forgot to declare this" (this + throw). The two used to look identical. + +## 0.1.0 + +Initial extraction of the plugin-facing types out of the middleware kernel, so +plugin packages could import them without reaching back into `middleware/src`. diff --git a/middleware/packages/plugin-api/api-snapshot/plugin-api.d.ts.snap b/middleware/packages/plugin-api/api-snapshot/plugin-api.d.ts.snap index 3c363a59a..5698aadc3 100644 --- a/middleware/packages/plugin-api/api-snapshot/plugin-api.d.ts.snap +++ b/middleware/packages/plugin-api/api-snapshot/plugin-api.d.ts.snap @@ -1454,11 +1454,28 @@ constructor(raw: string, detail: string); } export declare function parseCapabilityRef(raw: string): CapabilityRef; export declare function capabilitiesMatch(provider: CapabilityRef, consumer: CapabilityRef): boolean; +export interface ServiceCaller { +readonly agentId: string; +readonly pluginId: string; +} +declare const PER_CALLER_FACTORY: unique symbol; +export interface PerCallerFactory { +readonly [PER_CALLER_FACTORY]: (caller: ServiceCaller) => T; +} +export declare function perCallerService(factory: (caller: ServiceCaller) => T): PerCallerFactory; +export declare function isPerCallerService(value: unknown): value is PerCallerFactory; +export declare function resolvePerCallerService(factory: PerCallerFactory, caller: ServiceCaller): T; +export declare class ServiceNotDeclaredError extends Error { +readonly pluginId: string; +readonly capability: string; +readonly manifestField = "requires"; +constructor(pluginId: string, capability: string); +} export interface ServicesAccessor { get(name: string): T | undefined; has(name: string): boolean; -provide(name: string, impl: T): () => void; -replace(name: string, impl: T): () => void; +provide(name: string, impl: T | PerCallerFactory): () => void; +replace(name: string, impl: T | PerCallerFactory): () => void; } export interface NativeToolSpec { readonly name: string; @@ -1759,6 +1776,7 @@ export declare class MigrationHookError extends Error { readonly migrationCause: unknown; constructor(agentId: string, fromVersion: string, toVersion: string, cause: unknown); } +export {}; // ===== privacyMode.d.ts ===== export declare const PRIVACY_MODE_CONFIG_KEY = "_privacy_mode"; diff --git a/middleware/packages/plugin-api/package.json b/middleware/packages/plugin-api/package.json index bd01dd3ce..92db7df9a 100644 --- a/middleware/packages/plugin-api/package.json +++ b/middleware/packages/plugin-api/package.json @@ -1,6 +1,6 @@ { "name": "@omadia/plugin-api", - "version": "0.1.0", + "version": "1.0.0", "private": true, "type": "module", "main": "dist/index.js", diff --git a/middleware/packages/plugin-api/src/pluginContext.ts b/middleware/packages/plugin-api/src/pluginContext.ts index 305761e3c..1dba29a6f 100644 --- a/middleware/packages/plugin-api/src/pluginContext.ts +++ b/middleware/packages/plugin-api/src/pluginContext.ts @@ -396,6 +396,143 @@ export function capabilitiesMatch( return provider.name === consumer.name && provider.major === consumer.major; } +// --------------------------------------------------------------------------- +// Service resolution — the grant gate and per-caller attribution (epic #470 B1) +// --------------------------------------------------------------------------- + +/** + * Who is asking for a service. Every field is the **kernel-known** installed + * plugin id — `createPluginContext` fills it from the id the kernel activated + * the plugin under, never from an argument the caller supplies. A provider can + * therefore trust it for attribution, scoping and per-tenant filtering. + * + * `agentId` and `pluginId` are the same value under two names: the kernel's + * internal term is `agentId`, the manifest/registry term is `pluginId`. Both + * are present so a provider can read whichever name its own domain uses + * without a lookup table. + */ +export interface ServiceCaller { + /** Kernel-known installed plugin id (kernel-internal name for it). */ + readonly agentId: string; + /** The same kernel-known id under the manifest's name for it. */ + readonly pluginId: string; +} + +/** Brand for {@link PerCallerFactory}. A unique symbol, so a plain value a + * plugin happens to register can never be mistaken for a factory — including + * a value that *is* a function, which is why the factory is wrapped in a + * branded object rather than detected by `typeof impl === 'function'`. */ +const PER_CALLER_FACTORY = Symbol.for('@omadia/plugin-api.perCallerService'); + +/** + * A service registration that mints one implementation **per consuming + * plugin** instead of sharing a single instance. + * + * Build one with {@link perCallerService}; it is otherwise opaque. Resolution + * is memoized by the FACTORY OBJECT and then by `caller.pluginId`, so one + * provider instance is reused for repeat reads by the same consuming plugin, + * while a re-registered provider starts cold automatically because it is a + * different factory object. + */ +export interface PerCallerFactory { + readonly [PER_CALLER_FACTORY]: (caller: ServiceCaller) => T; +} + +/** + * Per-caller factory cache. + * + * Keying first on the wrapper object means a provider swap self-invalidates: + * `ctx.services.replace(name, perCallerService(...))` registers a fresh object, + * so the old cache becomes unreachable without any explicit lifecycle hook. + * Keying second on `caller.pluginId` makes the contract literal: one + * implementation per consuming plugin. + */ +const perCallerFactoryCache = new WeakMap< + PerCallerFactory, + Map +>(); + +/** + * Wrap a factory so the kernel invokes it once per consuming plugin, handing + * it the {@link ServiceCaller}. The factory must therefore be idempotent for a + * given caller: repeat reads by the same plugin receive the cached result, not + * a freshly constructed instance. + * + * ctx.services.provide( + * 'repoGrants', + * perCallerService((caller) => grantsScopedTo(caller.pluginId)), + * ); + * + * Why this exists (epic #470 §2.2): before it, a provider that needed to know + * which plugin was calling had exactly one option — take the id as an argument + * from the consumer (`listGrantedRepoIds(myOwnPluginId)`). That is + * self-attribution: the caller names itself, and nothing stops it naming + * someone else. Routing attribution through the kernel closes that by + * construction. + * + * Value providers are unaffected: `provide(name, impl)` with a plain value + * keeps returning that exact value to every consumer. + */ +export function perCallerService( + factory: (caller: ServiceCaller) => T, +): PerCallerFactory { + return { [PER_CALLER_FACTORY]: factory }; +} + +/** Narrow an arbitrary registration to a per-caller factory. */ +export function isPerCallerService( + value: unknown, +): value is PerCallerFactory { + return ( + typeof value === 'object' && + value !== null && + typeof (value as Record)[PER_CALLER_FACTORY] === 'function' + ); +} + +/** Invoke a per-caller factory. Exported for the kernel's registry; plugins + * never need it — `ctx.services.get` already resolves the factory. */ +export function resolvePerCallerService( + factory: PerCallerFactory, + caller: ServiceCaller, +): T { + let byPlugin = perCallerFactoryCache.get(factory); + if (!byPlugin) { + byPlugin = new Map(); + perCallerFactoryCache.set(factory, byPlugin); + } + if (byPlugin.has(caller.pluginId)) { + return byPlugin.get(caller.pluginId) as T; + } + const resolved = factory[PER_CALLER_FACTORY](caller); + byPlugin.set(caller.pluginId, resolved); + return resolved; +} + +/** + * Thrown by `ctx.services.get(name)` when the plugin's manifest does not + * declare `name` as a capability it `requires` (or `provides`). + * + * Typed so a plugin can distinguish "the operator has not installed a + * provider" (`get` returns `undefined`) from "I forgot to declare this" + * (this throw) — two very different bugs that used to look identical. + */ +export class ServiceNotDeclaredError extends Error { + public readonly pluginId: string; + public readonly capability: string; + /** The manifest field that would grant it. */ + public readonly manifestField = 'requires'; + constructor(pluginId: string, capability: string) { + super( + `plugin '${pluginId}' called ctx.services.get('${capability}') but its manifest does not declare that capability — ` + + `add '${capability}@' to the manifest's \`requires:\` list (or \`provides:\` if this plugin is the provider)`, + ); + this.name = 'ServiceNotDeclaredError'; + this.pluginId = pluginId; + this.capability = capability; + } +} + /** * Accessor for plugin-bereitgestellte (plugin-provided) services. * @@ -407,6 +544,19 @@ export function capabilitiesMatch( * const graph = ctx.services.get('graph'); * if (!graph) { // provider not installed — handle gracefully } * + * **`get` is manifest-gated (epic #470 B1).** The service-registry key IS the + * capability name, so a plugin may only resolve names it declared in its + * manifest's `requires:` (or `provides:`, for reading back its own + * registration). An undeclared name throws {@link ServiceNotDeclaredError} + * instead of handing over the implementation. Before this gate any installed + * plugin could ask for any service — including `graphPool`, the same Postgres + * pool core uses — with no manifest declaration and nothing in the install + * dialog. + * + * `has` stays ungated: it answers a yes/no existence question and hands over + * no capability, so gating it would only turn feature-probing into + * exception-handling. + * * Well-known service names and their accessor interfaces are documented * alongside the providing plugin. Plugins that depend on a specific service * should declare the provider in their manifest's `depends_on` so the @@ -414,16 +564,26 @@ export function capabilitiesMatch( */ export interface ServicesAccessor { /** Returns the registered provider for the given service, or undefined - * if no provider is installed. */ + * if no provider is installed. + * + * Throws {@link ServiceNotDeclaredError} when this plugin's manifest does + * not declare `name` — that is a manifest bug, not a missing provider, and + * the two must not be reported the same way. */ get(name: string): T | undefined; - /** Whether a provider is currently registered. */ + /** Whether a provider is currently registered. Ungated — see the interface + * doc. */ has(name: string): boolean; /** Register THIS plugin as the provider for the given service name. * Returns a dispose handle — the plugin's `close()` MUST invoke it to * symmetrically unregister the service on deactivate. Throws on * duplicate-provider (two plugins cannot both claim the same name; the - * operator must uninstall one). */ - provide(name: string, impl: T): () => void; + * operator must uninstall one). + * + * `impl` is normally the shared implementation every consumer receives. + * Wrap it in {@link perCallerService} instead to mint one implementation + * per consuming plugin, with the kernel-known caller id supplied by the + * kernel. */ + provide(name: string, impl: T | PerCallerFactory): () => void; /** * OB-71 (palaia capture-pipeline): wrap an already-registered provider * with a decorator. The previous provider stays live behind the wrapper; @@ -434,8 +594,11 @@ export interface ServicesAccessor { * decorator for the named capability (e.g. `harness-orchestrator-extras` * wrapping `knowledgeGraph` with the capture-filter). Treat the swap as * a coordinated handoff, not a competing provider. + * + * Accepts a {@link perCallerService} wrapper on the same terms as + * `provide`. */ - replace(name: string, impl: T): () => void; + replace(name: string, impl: T | PerCallerFactory): () => void; } /** diff --git a/middleware/src/platform/pluginContext.ts b/middleware/src/platform/pluginContext.ts index 0f8323fc5..17d180be6 100644 --- a/middleware/src/platform/pluginContext.ts +++ b/middleware/src/platform/pluginContext.ts @@ -52,6 +52,7 @@ import { type EmitResult, EventNotDeclaredError, ConductorUnavailableError, + type ServiceCaller, } from '@omadia/plugin-api'; import type { DomainTool } from '@omadia/orchestrator'; import { turnContext } from '@omadia/orchestrator'; @@ -92,6 +93,7 @@ import type { PluginStatusRegistry } from './pluginStatusRegistry.js'; import { createMemoryAccessor } from './memoryAccessor.js'; import { SCRATCH_DIR } from './paths.js'; import type { ServiceRegistry } from './serviceRegistry.js'; +import { createServiceGrantGate } from './pluginServiceGrants.js'; /** * The plugin-facing types (PluginContext, SecretsAccessor, ConfigAccessor, @@ -229,9 +231,24 @@ export function createPluginContext( domain = `unknown.${safeId}`; } + // Epic #470 (B1) — the consumer seam `serviceRegistry.ts` always said would + // exist. `get` resolves only capabilities this plugin's manifest declares; + // everything else throws `ServiceNotDeclaredError`. Built once per context: + // a manifest cannot change under a live plugin. + const assertServiceGranted = createServiceGrantGate({ agentId, catalog, log }); + + // Caller identity handed to per-caller service factories. Built from the + // kernel-known `agentId`, never from an argument — that is the whole point + // (§2.2: a self-attributed accessor is not an accessor, it is a suggestion). + const serviceCaller: ServiceCaller = Object.freeze({ + agentId, + pluginId: agentId, + }); + const services: ServicesAccessor = { get(name: string): T | undefined { - return serviceRegistry.get(name); + assertServiceGranted(name); + return serviceRegistry.get(name, serviceCaller); }, has(name: string): boolean { return serviceRegistry.has(name); @@ -266,7 +283,10 @@ export function createPluginContext( // — the active Agent slug is resolved from the turn context at call time, so // the same plugin invoked under two Agents writes to two disjoint trees. // Plugins cannot see each other's — or another orchestrator's — memory. - const memoryStoreService = serviceRegistry.get('memoryStore'); + const memoryStoreService = serviceRegistry.get( + 'memoryStore', + serviceCaller, + ); const memory: MemoryAccessor | undefined = memoryStoreService && memoryDeclared(agentId, catalog) ? createMemoryAccessor({ @@ -725,6 +745,7 @@ export function createPluginContext( callerAgentId: agentId, permissions: extractSubAgentPermissions(agentId, catalog), serviceRegistry, + serviceCaller, }); // OB-29-2 — KnowledgeGraphAccessor: present iff the manifest declares @@ -734,6 +755,7 @@ export function createPluginContext( callerAgentId: agentId, entitySystems: extractEntitySystems(agentId, catalog), serviceRegistry, + serviceCaller, }); // OB-29-3 — LlmAccessor: present iff the manifest declares @@ -742,6 +764,7 @@ export function createPluginContext( callerAgentId: agentId, permissions: extractLlmPermissions(agentId, catalog), serviceRegistry, + serviceCaller, activeProvider: resolveActiveProvider(registry, agentId), vault, }); @@ -763,18 +786,24 @@ export function createPluginContext( // so the #462 audit log and the scan-policy dispatch guard see the plugin. const mcpAllowed = catalog.get(agentId)?.plugin.permissions_summary.mcp === true; const mcp: McpAccessor | undefined = mcpAllowed - ? createPluginMcpAccessor(agentId, serviceRegistry) + ? createPluginMcpAccessor(agentId, serviceRegistry, serviceCaller) : undefined; const eventsAllowed = catalog.get(agentId)?.plugin.permissions_summary.events_emit === true; const events: EventsAccessor | undefined = eventsAllowed ? { emit(id: string, payload: Record) { - const router = serviceRegistry.get('conductorEventRouter'); + const router = serviceRegistry.get( + 'conductorEventRouter', + serviceCaller, + ); if (!router) throw new ConductorUnavailableError(); // Deny-by-default fails CLOSED: with no catalog we cannot prove the plugin declared // this id, so we reject rather than allow an unverified emit. - const eventCatalog = serviceRegistry.get('eventCatalogRegistry'); + const eventCatalog = serviceRegistry.get( + 'eventCatalogRegistry', + serviceCaller, + ); if (!eventCatalog || !eventCatalog.allows(agentId, id)) throw new EventNotDeclaredError(agentId, id); return router.emit(id, payload, agentId); }, @@ -857,10 +886,16 @@ function mcpHostRowToConfig(row: McpHostServerRow): McpHostServiceConfig { /** Exported for direct unit testing (issue #458). */ export function createPluginMcpAccessor( pluginId: string, - serviceRegistry: { get(name: string): T | undefined }, + serviceRegistry: { + get(name: string, caller?: ServiceCaller): T | undefined; + }, + caller: ServiceCaller = Object.freeze({ + agentId: pluginId, + pluginId, + }), ): McpAccessor { const host = (): McpHostService => { - const service = serviceRegistry.get('mcp'); + const service = serviceRegistry.get('mcp', caller); if (!service) { throw new Error('MCP host service unavailable — the core did not wire ctx.mcp'); } @@ -963,12 +998,13 @@ interface SubAgentAccessorOptions { callerAgentId: string; permissions: SubAgentPermissions | undefined; serviceRegistry: ServiceRegistry; + serviceCaller: ServiceCaller; } function createSubAgentAccessor( opts: SubAgentAccessorOptions, ): SubAgentAccessor | undefined { - const { callerAgentId, permissions, serviceRegistry } = opts; + const { callerAgentId, permissions, serviceRegistry, serviceCaller } = opts; if (!permissions) return undefined; // Per-instance call counter. Resets when a fresh ctx is created (which @@ -1007,6 +1043,7 @@ function createSubAgentAccessor( } const tool = serviceRegistry.get( `subAgent:${targetAgentId}`, + serviceCaller, ); if (!tool) { throw new UnknownSubAgentError(callerAgentId, targetAgentId); @@ -1034,12 +1071,13 @@ interface KnowledgeGraphAccessorOptions { callerAgentId: string; entitySystems: readonly string[]; serviceRegistry: ServiceRegistry; + serviceCaller: ServiceCaller; } function createKnowledgeGraphAccessor( opts: KnowledgeGraphAccessorOptions, ): KnowledgeGraphAccessor | undefined { - const { callerAgentId, entitySystems, serviceRegistry } = opts; + const { callerAgentId, entitySystems, serviceRegistry, serviceCaller } = opts; if (entitySystems.length === 0) return undefined; // We don't pre-resolve the KG impl: doing it lazily lets the plugin // boot even when the kg-provider activates later (provider-ordering is @@ -1047,7 +1085,10 @@ function createKnowledgeGraphAccessor( // throwing KgServiceUnavailableError if no provider is around. const allowed = new Set(entitySystems); function resolveKg(): KnowledgeGraph { - const kg = serviceRegistry.get('knowledgeGraph'); + const kg = serviceRegistry.get( + 'knowledgeGraph', + serviceCaller, + ); if (!kg) throw new KgServiceUnavailableError(callerAgentId); return kg; } @@ -1194,6 +1235,7 @@ interface LlmAccessorOptions { callerAgentId: string; permissions: LlmPermissions | undefined; serviceRegistry: ServiceRegistry; + serviceCaller: ServiceCaller; /** Provider that serves THIS plugin's `ctx.llm` (per-plugin pin → global → * anthropic). Drives both class-ref whitelist resolution AND which provider * the call is built on, so gate and execution stay in lockstep. */ @@ -1207,8 +1249,14 @@ interface LlmAccessorOptions { function createLlmAccessor( opts: LlmAccessorOptions, ): LlmAccessor | undefined { - const { callerAgentId, permissions, serviceRegistry, activeProvider, vault } = - opts; + const { + callerAgentId, + permissions, + serviceRegistry, + serviceCaller, + activeProvider, + vault, + } = opts; if (!permissions) return undefined; let callsUsed = 0; @@ -1224,7 +1272,9 @@ function createLlmAccessor( let buildPromise: Promise | undefined; const resolveServingProvider = (): Promise => { if (activeProvider === 'anthropic') { - return Promise.resolve(serviceRegistry.get('llm')); + return Promise.resolve( + serviceRegistry.get('llm', serviceCaller), + ); } if (buildPromise === undefined) { buildPromise = (async () => { diff --git a/middleware/src/platform/pluginServiceGrants.ts b/middleware/src/platform/pluginServiceGrants.ts new file mode 100644 index 000000000..4a58f2305 --- /dev/null +++ b/middleware/src/platform/pluginServiceGrants.ts @@ -0,0 +1,282 @@ +/** + * Grant gate for `ctx.services.get` — epic #470, bug B1. + * + * WHAT WAS WRONG + * -------------- + * `pluginContext.ts` exposed the service registry as a bare pass-through: + * + * get(name: string) { return serviceRegistry.get(name); } + * + * Any installed plugin could therefore ask for any registered service — + * `graphPool` (the same Postgres pool core uses), `tigrisStore`, + * `anthropicClient` — with no manifest declaration, no operator consent, and + * nothing about it in the install dialog. `serviceRegistry.ts`'s own header + * conceded the design: *"This registry is a naked service-locator; enforcement + * lives at the consumer seam."* This file IS that seam. + * + * THE RULE + * -------- + * The service-registry key IS the capability name — `pluginContext.ts` states + * it in the capability docblock ("Capability-names are ALSO used as + * service-registry keys"), and every provider follows it. So the manifest + * already carries the declaration the gate needs, and no new manifest field is + * invented here: + * + * - `requires: ["knowledgeGraph@^1"]` grants `get('knowledgeGraph')`. + * - `provides: ["memoryStore@1"]` grants `get('memoryStore')` — a plugin + * may always read back its own registration; it holds the implementation + * anyway, so this is not an escalation. + * - anything else throws `ServiceNotDeclaredError`, naming both the + * capability and the manifest field that would grant it. + * + * WHY THERE IS AN ALLOWLIST + * ------------------------- + * A call-site audit across this repo's built-in plugin packages and all ten + * sibling plugin repos (`~/sources/omadia-*`) found that today's `requires:` + * lists are far from complete — 63 (plugin, capability) pairs are consumed + * without being declared. Turning the gate fail-closed in one step would break + * every one of them, including shipped Hub plugins this PR cannot edit. + * + * So the gate is fail-closed for everything EXCEPT the exact pairs the audit + * found, which warn once and resolve. The allowlist is dated, closed, and + * keyed per plugin id: a *different* plugin asking for `graphPool` still + * throws, and a *new* undeclared name in an allowlisted plugin still throws. + * It grandfathers history, it does not open a door. + * + * Two of the entries are not laziness but a genuine naming defect worth + * recording: `harness-plugin-privacy-guard` declares the capability + * `privacy.redact@1` but registers the service under the key `privacyRedact`. + * Capability name and service key disagree, so no `requires:` entry could + * grant it. That mismatch has to be fixed on one side or the other before the + * corresponding allowlist rows can be dropped. + * + * The first audit missed some rows for two concrete reasons: several service + * names are hidden behind exported constants (`NUDGE_STATE_SERVICE_NAME`, + * `PROCESS_MEMORY_SERVICE_NAME`, `PLUGIN_CAPABILITIES_SERVICE`, …) instead of + * literal strings, and some channel plugins resolve capabilities through + * shared `@omadia/channel-sdk` helpers rather than a literal + * `ctx.services.get('...')` inside the plugin's own source file. + * + * RETIRING IT + * ----------- + * Each row is retired by adding the capability to that plugin's manifest — but + * note `requires:` is also the *activation* dependency (`resolveEligiblePlugins` + * holds back a consumer whose requires are unmet), so a plugin that consumes a + * service *optionally* cannot express that today. Declaring it would make an + * optional dependency mandatory and could stop the plugin activating. That + * missing "optional requires" expression is the open design question this + * allowlist defers, not a shortcut around work that is already possible. + */ + +import { + ServiceNotDeclaredError, + parseCapabilityRef, +} from '@omadia/plugin-api'; + +import type { PluginCatalog } from '../plugins/manifestLoader.js'; + +/** + * Audited legacy grants — snapshot taken 2026-08-20. + * + * Keyed by the kernel-known plugin id, valued with the exact service names + * that plugin resolves today without declaring them. CLOSED SET: adding a row + * means a shipped plugin regressed and needs a manifest fix, not a wider gate. + * + * Sources: `middleware/packages/*` (built-ins) and the ten standalone plugin + * repos under `~/sources/omadia-*`, read at their `main`. + */ +export const LEGACY_UNDECLARED_SERVICE_GRANTS_2026_08_20: Readonly< + Record +> = Object.freeze({ + // -- built-in plugin packages (middleware/packages/*) --------------------- + '@omadia/plugin-office': Object.freeze(['privacyRedact']), + '@omadia/verifier': Object.freeze(['graphPool', 'odoo.client']), + '@omadia/knowledge-graph-inmemory': Object.freeze(['turnContext']), + '@omadia/knowledge-graph-neon': Object.freeze(['turnContext']), + '@omadia/diagrams': Object.freeze(['memoryStore']), + '@omadia/ui-orchestrator': Object.freeze([ + 'agentToolInvoker', + 'canvasOutputRegistry', + 'deterministicActionRegistry', + ]), + '@omadia/plugin-plan-runner': Object.freeze([ + 'knowledgeGraph', + 'processMemory', + 'turnHookRegistry', + ]), + '@omadia/orchestrator-extras': Object.freeze([ + 'agentPriorities', + 'graphPool', + 'processMemory', + ]), + '@omadia/ui-channel': Object.freeze(['graphTenantId']), + '@omadia/orchestrator': Object.freeze([ + 'attachmentBindings', + 'audienceGrants', + 'graphPool', + 'installedPluginConfigReader', + 'installedPluginToolsReadyReader', + 'llmProviderCatalog', + 'microsoft365.graph', + 'nativeToolRegistry', + 'nudgeProviders', + 'nudgeStateStore', + 'palaiaExcerpt', + 'pluginCapabilities', + 'privacyRedact', + 'processMemory', + 'responseGuard', + 'sessionBriefing', + 'tigrisStore', + 'turnHookRegistry', + 'turnReceiptStore', + ]), + // -- standalone plugin repos (shipped via hub.omadia.ai) ----------------- + '@omadia/channel-discord': Object.freeze([ + 'channelResolver', + 'chatAgent', + ]), + '@omadia/channel-slack': Object.freeze([ + 'channelResolver', + 'chatAgent', + ]), + '@omadia/channel-teams': Object.freeze([ + 'anthropicClient', + 'channelDirectoryRegistry', + 'channelResolver', + 'conductorAwaitResolver', + 'embeddingClient', + 'graphPool', + 'graphTenantId', + 'microsoft365.graph', + 'routinesIntegration', + 'tigrisStore', + 'topicDetector', + 'turnContext', + 'uiRouteCatalog', + ]), + '@omadia/channel-telegram': Object.freeze([ + 'channelResolver', + 'memoryStore', + 'turnContext', + ]), + '@omadia/channel-whatsapp': Object.freeze([ + 'channelResolver', + 'chatAgent', + ]), + '@omadia/integration-odoo': Object.freeze(['entityRefBus']), + '@omadia/agent-odoo-hr': Object.freeze([ + 'odoo.agentToolkit.hr', + 'odoo.client', + ]), + '@omadia/agent-odoo-accounting': Object.freeze([ + 'odoo.agentToolkit.accounting', + ]), + '@omadia/agent-confluence': Object.freeze([ + 'confluence.client', + 'confluence.toolkit', + ]), +}); + +/** Why a `services.get` call was allowed — or wasn't. */ +export type ServiceGrantOutcome = + | 'declared' + | 'self-provided' + | 'legacy-allowlist' + | 'undeclared'; + +/** + * Every capability name the plugin's manifest declares — `requires` (consume) + * plus `provides` (read back its own registration). + * + * A plugin with no catalog entry declares nothing, so it is granted nothing. + * That mirrors `scratchEnabled`, which also denies on an absent entry: an id + * the kernel cannot find a manifest for is an id whose permissions cannot be + * checked, and unknown permissions are denied permissions. + */ +export function declaredServiceNames( + agentId: string, + catalog: PluginCatalog, +): ReadonlySet { + const entry = catalog.get(agentId); + if (!entry) return new Set(); + const names = new Set(); + for (const raw of [ + ...(entry.plugin.requires ?? []), + ...(entry.plugin.provides ?? []), + ]) { + try { + names.add(parseCapabilityRef(raw).name); + } catch { + // Malformed entry — the loader already warned. A name we cannot parse + // grants nothing, which is the fail-closed direction. + } + } + return names; +} + +/** Classify one `services.get(name)` call. Pure — no logging, no throwing, so + * it can be asserted directly in tests. */ +export function classifyServiceGrant( + agentId: string, + name: string, + declared: ReadonlySet, + catalog: PluginCatalog, +): ServiceGrantOutcome { + if (declared.has(name)) { + const entry = catalog.get(agentId); + const provides = entry?.plugin.provides ?? []; + const selfProvided = provides.some((raw) => { + try { + return parseCapabilityRef(raw).name === name; + } catch { + return false; + } + }); + return selfProvided ? 'self-provided' : 'declared'; + } + const legacy = LEGACY_UNDECLARED_SERVICE_GRANTS_2026_08_20[agentId]; + if (legacy?.includes(name)) return 'legacy-allowlist'; + return 'undeclared'; +} + +export interface ServiceGrantGateOptions { + agentId: string; + catalog: PluginCatalog; + /** Where the one-time legacy warning goes. */ + log: (...args: unknown[]) => void; +} + +/** + * Build the per-plugin gate. The returned function is called for every + * `ctx.services.get(name)` and either returns (allowed) or throws + * {@link ServiceNotDeclaredError}. + * + * The declared set is computed once per plugin context rather than per call: + * a context is created at activation and the manifest cannot change under a + * live plugin. + * + * Legacy warnings are emitted once per (plugin, capability) so a service + * resolved inside a per-turn hot path cannot flood the log. + */ +export function createServiceGrantGate( + opts: ServiceGrantGateOptions, +): (name: string) => void { + const { agentId, catalog, log } = opts; + const declared = declaredServiceNames(agentId, catalog); + const warned = new Set(); + + return function assertServiceGranted(name: string): void { + const outcome = classifyServiceGrant(agentId, name, declared, catalog); + if (outcome === 'undeclared') { + throw new ServiceNotDeclaredError(agentId, name); + } + if (outcome === 'legacy-allowlist' && !warned.has(name)) { + warned.add(name); + log( + `[services] '${agentId}' resolved '${name}' without declaring it — allowed by the dated legacy allowlist (2026-08-20). ` + + `Add '${name}@' to the plugin's manifest \`requires:\`; the allowlist is a migration ramp, not a permission.`, + ); + } + }; +} diff --git a/middleware/src/platform/serviceRegistry.ts b/middleware/src/platform/serviceRegistry.ts index 9b747150f..14f73779f 100644 --- a/middleware/src/platform/serviceRegistry.ts +++ b/middleware/src/platform/serviceRegistry.ts @@ -26,8 +26,28 @@ * may only read graph scopes tagged with its own agentId or `public`. The * scope wrapping happens in `createPluginContext`, not here. This registry is * a naked service-locator; enforcement lives at the consumer seam. + * + * That seam now exists (epic #470, B1): `pluginServiceGrants.ts`, called from + * `createPluginContext`. `ctx.services.get` resolves only capabilities the + * plugin's manifest declares. This class stays deliberately unenforcing — core + * resolves its own services through it, and a registry that policed its own + * callers could not serve both. */ +import { + isPerCallerService, + resolvePerCallerService, + type ServiceCaller, +} from '@omadia/plugin-api'; + +/** Attribution used when core resolves a service for itself rather than on + * behalf of a plugin. A per-caller factory can branch on it to hand the + * kernel an unscoped implementation. */ +export const KERNEL_SERVICE_CALLER: ServiceCaller = Object.freeze({ + agentId: '@omadia/core', + pluginId: '@omadia/core', +}); + /** The known well-known service names. An open string union so future * additions (e.g. 'diagrams', 'attachments', 'memory') don't require a * cross-module refactor — a provider calls `provide('diagrams', impl)` and @@ -109,8 +129,22 @@ export class ServiceRegistry { return this.track(owner, name, dispose); } - get(name: ServiceName): T | undefined { - return this.providers.get(name) as T | undefined; + /** + * Resolve a provider. + * + * When the registration is a {@link perCallerService} factory, it is + * invoked with `caller` and the result is returned — so a provider that + * needs to know who is asking gets the id from the kernel rather than from + * an argument the consumer supplied (epic #470 §2.2). + * + * `caller` defaults to {@link KERNEL_SERVICE_CALLER}: core's own direct + * `.get()` call sites keep working unchanged and are attributed to the + * kernel, not to whichever plugin happens to be on the stack. + */ + get(name: ServiceName, caller: ServiceCaller = KERNEL_SERVICE_CALLER): T | undefined { + const raw = this.providers.get(name); + if (isPerCallerService(raw)) return resolvePerCallerService(raw, caller); + return raw as T | undefined; } has(name: ServiceName): boolean { diff --git a/middleware/test/manifestDevJobsLegacyKey.test.ts b/middleware/test/manifestDevJobsLegacyKey.test.ts index 33296a446..be9740d5d 100644 --- a/middleware/test/manifestDevJobsLegacyKey.test.ts +++ b/middleware/test/manifestDevJobsLegacyKey.test.ts @@ -17,6 +17,8 @@ import { strict as assert } from 'node:assert'; import { describe, it } from 'node:test'; +import { ServiceNotDeclaredError } from '@omadia/plugin-api'; + import type { Plugin } from '../src/api/admin-v1.js'; import { adaptManifestV1 } from '../src/plugins/manifestLoader.js'; import type { @@ -126,10 +128,16 @@ describe('legacy permissions.devJobs manifests stay loadable', () => { // And the rest of the context is intact: the plugin activates normally. assert.equal(ctx.agentId, LEGACY_ID); assert.equal(typeof ctx.services.get, 'function'); - assert.equal( - ctx.services.get('devJobs'), - undefined, - 'no provider registers devJobs, so the service route yields nothing either', + // And the service-locator route is closed too. Before epic #470 B1 this + // returned `undefined` (no provider registered); now the manifest gate + // rejects it outright, because the stale permission key above is not a + // capability declaration and grants nothing. Either way the deleted + // accessor is unreachable — the gate just says so out loud instead of + // looking like a missing installation. + assert.throws( + () => ctx.services.get('devJobs'), + ServiceNotDeclaredError, + 'the legacy permission key grants nothing through the service locator either', ); }); }); diff --git a/middleware/test/pluginServiceGrantCoverage.test.ts b/middleware/test/pluginServiceGrantCoverage.test.ts new file mode 100644 index 000000000..4f7af53a9 --- /dev/null +++ b/middleware/test/pluginServiceGrantCoverage.test.ts @@ -0,0 +1,465 @@ +import { strict as assert } from 'node:assert'; +import { Dirent, promises as fs } from 'node:fs'; +import path from 'node:path'; + +import { describe, it } from 'node:test'; +import ts from 'typescript'; +import { parseDocument } from 'yaml'; + +import { parseCapabilityRef } from '@omadia/plugin-api'; + +import { LEGACY_UNDECLARED_SERVICE_GRANTS_2026_08_20 } from '../src/platform/pluginServiceGrants.js'; + +const MIDDLEWARE_ROOT = path.resolve(import.meta.dirname, '..'); +const PACKAGES_ROOT = path.join(MIDDLEWARE_ROOT, 'packages'); +const KERNEL_SRC_ROOT = path.join(MIDDLEWARE_ROOT, 'src'); + +interface ManifestInfo { + readonly pluginId: string; + readonly manifestPath: string; + readonly packageRoot: string; + readonly declaredNames: ReadonlySet; + readonly sourceFiles: readonly string[]; +} + +interface ServiceUse { + readonly capability: string; + readonly file: string; + readonly line: number; +} + +interface CoverageSnapshot { + readonly manifests: readonly ManifestInfo[]; + readonly observedByPlugin: ReadonlyMap; + readonly undeclaredFindings: readonly string[]; + readonly scanFailures: readonly string[]; +} + +let cachedSnapshot: CoverageSnapshot | undefined; + +describe('plugin service grant coverage', () => { + it('every built-in services.get call is declared or allowlisted', async () => { + const snapshot = await loadCoverageSnapshot(); + assert.deepEqual( + snapshot.scanFailures, + [], + `service-grant scanner failed:\n${snapshot.scanFailures.join('\n')}`, + ); + assert.deepEqual( + snapshot.undeclaredFindings, + [], + `undeclared built-in service grants found:\n${snapshot.undeclaredFindings.join('\n')}`, + ); + }); + + it('the built-in allowlist has no stale rows', async () => { + const snapshot = await loadCoverageSnapshot(); + assert.deepEqual( + snapshot.scanFailures, + [], + `service-grant scanner failed:\n${snapshot.scanFailures.join('\n')}`, + ); + + const builtInIds = new Set(snapshot.manifests.map((m) => m.pluginId)); + const findings: string[] = []; + + // These plugin ids live in sibling repos outside this worktree. This repo + // can verify their manifests were allowlisted intentionally, but it cannot + // prove their current call sites still exist because their source is not + // under middleware/packages/* here. + const unverifiableStandaloneIds = new Set([ + '@omadia/agent-confluence', + '@omadia/agent-odoo-accounting', + '@omadia/agent-odoo-hr', + '@omadia/channel-discord', + '@omadia/channel-slack', + '@omadia/channel-teams', + '@omadia/channel-telegram', + '@omadia/channel-whatsapp', + '@omadia/integration-odoo', + ]); + + for (const [pluginId, legacyNames] of Object.entries( + LEGACY_UNDECLARED_SERVICE_GRANTS_2026_08_20, + )) { + if (unverifiableStandaloneIds.has(pluginId)) continue; + if (!builtInIds.has(pluginId)) continue; + const observed = new Set( + (snapshot.observedByPlugin.get(pluginId) ?? []).map((use) => use.capability), + ); + for (const capability of legacyNames) { + if (!observed.has(capability)) { + findings.push( + `${pluginId} still allowlists '${capability}', but no real built-in call site in middleware/packages/* now resolves it. Remove the stale row or restore the call before keeping it grandfathered.`, + ); + } + } + } + + assert.deepEqual( + findings, + [], + `stale built-in allowlist rows found:\n${findings.join('\n')}`, + ); + }); +}); + +async function loadCoverageSnapshot(): Promise { + if (cachedSnapshot) return cachedSnapshot; + + const manifests = await loadBuiltInManifests(); + const program = await createWorkspaceProgram(); + const checker = program.getTypeChecker(); + const scanFailures: string[] = []; + const undeclaredFindings: string[] = []; + const observedByPlugin = new Map(); + + for (const manifest of manifests) { + const observed = collectServiceUsesForManifest( + manifest, + program, + checker, + scanFailures, + ); + observedByPlugin.set(manifest.pluginId, observed); + + const declared = manifest.declaredNames; + const legacy = new Set( + LEGACY_UNDECLARED_SERVICE_GRANTS_2026_08_20[manifest.pluginId] ?? [], + ); + for (const use of observed) { + if (declared.has(use.capability) || legacy.has(use.capability)) continue; + undeclaredFindings.push( + `${manifest.pluginId} resolves '${use.capability}' at ${use.file}:${String(use.line)} without declaring it. Either add '${use.capability}@' to manifest.yaml or add a dated row to LEGACY_UNDECLARED_SERVICE_GRANTS_2026_08_20 for this legacy pair.`, + ); + } + } + + cachedSnapshot = { + manifests, + observedByPlugin, + undeclaredFindings, + scanFailures, + }; + return cachedSnapshot; +} + +async function loadBuiltInManifests(): Promise { + const entries = await fs.readdir(PACKAGES_ROOT, { withFileTypes: true }); + const manifests: ManifestInfo[] = []; + + for (const entry of entries) { + if (!entry.isDirectory()) continue; + const packageRoot = path.join(PACKAGES_ROOT, entry.name); + const manifestPath = path.join(packageRoot, 'manifest.yaml'); + try { + await fs.access(manifestPath); + } catch { + continue; + } + manifests.push(await parseManifest(manifestPath, packageRoot)); + } + + manifests.sort((a, b) => a.pluginId.localeCompare(b.pluginId, 'en')); + return manifests; +} + +async function parseManifest( + manifestPath: string, + packageRoot: string, +): Promise { + const raw = await fs.readFile(manifestPath, 'utf8'); + const doc = parseDocument(raw); + if (doc.errors.length > 0) { + throw new Error( + `${relativeToMiddleware(manifestPath)} failed to parse:\n${doc.errors + .map((error) => String(error)) + .join('\n')}`, + ); + } + + const parsed = doc.toJSON() as Record | null; + if (!parsed || typeof parsed !== 'object') { + throw new Error( + `${relativeToMiddleware(manifestPath)} did not parse to an object manifest`, + ); + } + + const identity = asRecord(parsed['identity']); + const pluginId = asString(identity?.['id']); + if (!pluginId) { + throw new Error( + `${relativeToMiddleware(manifestPath)} is missing identity.id`, + ); + } + + const requires = asStringArray(parsed['requires'], manifestPath, 'requires'); + const provides = asStringArray(parsed['provides'], manifestPath, 'provides'); + const declaredNames = new Set(); + for (const rawCapability of [...requires, ...provides]) { + declaredNames.add(parseCapabilityRef(rawCapability).name); + } + + const sourceRoot = path.join(packageRoot, 'src'); + const sourceFiles = await collectTypeScriptFiles(sourceRoot); + return { + pluginId, + manifestPath, + packageRoot, + declaredNames, + sourceFiles, + }; +} + +async function createWorkspaceProgram(): Promise { + const packageEntries = await fs.readdir(PACKAGES_ROOT, { withFileTypes: true }); + const packageSourceFiles = await Promise.all( + packageEntries + .filter((entry) => entry.isDirectory()) + .map((entry) => + collectTypeScriptFiles(path.join(PACKAGES_ROOT, entry.name, 'src')), + ), + ); + const allSourceFiles = [ + ...(await collectTypeScriptFiles(KERNEL_SRC_ROOT)), + ...packageSourceFiles.flat(), + ]; + const paths = await buildWorkspacePaths(); + return ts.createProgram({ + rootNames: allSourceFiles, + options: { + target: ts.ScriptTarget.ES2022, + module: ts.ModuleKind.NodeNext, + moduleResolution: ts.ModuleResolutionKind.NodeNext, + strict: true, + skipLibCheck: true, + esModuleInterop: true, + allowJs: false, + baseUrl: MIDDLEWARE_ROOT, + paths, + }, + }); +} + +async function buildWorkspacePaths(): Promise> { + const out: Record = {}; + const entries = await fs.readdir(PACKAGES_ROOT, { withFileTypes: true }); + for (const entry of entries) { + if (!entry.isDirectory()) continue; + const packageRoot = path.join(PACKAGES_ROOT, entry.name); + const packageJsonPath = path.join(packageRoot, 'package.json'); + try { + const raw = await fs.readFile(packageJsonPath, 'utf8'); + const pkg = JSON.parse(raw) as { name?: unknown }; + if (typeof pkg.name !== 'string' || pkg.name.length === 0) continue; + const srcIndex = path.relative( + MIDDLEWARE_ROOT, + path.join(packageRoot, 'src', 'index.ts'), + ); + const srcWildcard = path.relative( + MIDDLEWARE_ROOT, + path.join(packageRoot, 'src', '*'), + ); + out[pkg.name] = [srcIndex]; + out[`${pkg.name}/*`] = [srcWildcard]; + } catch { + continue; + } + } + return out; +} + +function collectServiceUsesForManifest( + manifest: ManifestInfo, + program: ts.Program, + checker: ts.TypeChecker, + scanFailures: string[], +): readonly ServiceUse[] { + const observed: ServiceUse[] = []; + + for (const filePath of manifest.sourceFiles) { + const sourceFile = program.getSourceFile(filePath); + if (!sourceFile) { + scanFailures.push( + `${manifest.pluginId} source file ${relativeToMiddleware(filePath)} was not loaded into the TypeScript program`, + ); + continue; + } + + walk(sourceFile, (node) => { + if (!isServicesGetCall(node)) return; + const firstArg = node.arguments[0]; + const location = formatNodeLocation(sourceFile, firstArg ?? node); + if (!firstArg) { + scanFailures.push( + `${manifest.pluginId} uses ctx.services.get with no argument at ${location}`, + ); + return; + } + const resolved = resolveServiceName(firstArg, checker); + if ('error' in resolved) { + scanFailures.push(`${manifest.pluginId} ${location}: ${resolved.error}`); + return; + } + observed.push({ + capability: resolved.name, + file: relativeToMiddleware(filePath), + line: sourceFile.getLineAndCharacterOfPosition(firstArg.getStart()).line + 1, + }); + }); + } + + return observed; +} + +function isServicesGetCall(node: ts.Node): node is ts.CallExpression { + if (!ts.isCallExpression(node)) return false; + const callee = node.expression; + if (!ts.isPropertyAccessExpression(callee) || callee.name.text !== 'get') { + return false; + } + const target = callee.expression; + return ts.isPropertyAccessExpression(target) && target.name.text === 'services'; +} + +function resolveServiceName( + arg: ts.Expression, + checker: ts.TypeChecker, +): { name: string } | { error: string } { + if (ts.isStringLiteral(arg) || ts.isNoSubstitutionTemplateLiteral(arg)) { + return { name: arg.text }; + } + if (!ts.isIdentifier(arg)) { + return { + error: `ctx.services.get argument must be a string literal or identifier, saw ${ts.SyntaxKind[arg.kind]}`, + }; + } + const symbol = checker.getSymbolAtLocation(arg); + if (!symbol) { + return { + error: `could not resolve identifier '${arg.text}' to a declaration`, + }; + } + return resolveLiteralFromSymbol(symbol, checker, new Set()); +} + +function resolveLiteralFromSymbol( + symbol: ts.Symbol, + checker: ts.TypeChecker, + seen: Set, +): { name: string } | { error: string } { + const target = + symbol.flags & ts.SymbolFlags.Alias ? checker.getAliasedSymbol(symbol) : symbol; + if (seen.has(target)) { + return { error: `identifier resolution looped at '${target.getName()}'` }; + } + seen.add(target); + + for (const declaration of target.declarations ?? []) { + if (ts.isVariableDeclaration(declaration)) { + const initializer = declaration.initializer; + if ( + initializer && + (ts.isStringLiteral(initializer) || + ts.isNoSubstitutionTemplateLiteral(initializer)) + ) { + return { name: initializer.text }; + } + if (initializer && ts.isIdentifier(initializer)) { + const next = checker.getSymbolAtLocation(initializer); + if (next) return resolveLiteralFromSymbol(next, checker, seen); + } + } + } + + const sourcePaths = Array.from( + new Set( + (target.declarations ?? []) + .map((declaration) => declaration.getSourceFile().fileName) + .filter(Boolean), + ), + ).map(relativeToMiddleware); + + return { + error: + `identifier '${target.getName()}' does not resolve to a string-literal const in middleware/src or middleware/packages/*/src` + + (sourcePaths.length > 0 + ? ` (declarations: ${sourcePaths.join(', ')})` + : ''), + }; +} + +function walk(node: ts.Node, visit: (node: ts.Node) => void): void { + visit(node); + node.forEachChild((child) => walk(child, visit)); +} + +async function collectTypeScriptFiles(root: string): Promise { + const out: string[] = []; + let entries: Dirent[]; + try { + entries = await fs.readdir(root, { withFileTypes: true }); + } catch { + return out; + } + + for (const entry of entries) { + const fullPath = path.join(root, entry.name); + if (entry.isDirectory()) { + out.push(...(await collectTypeScriptFiles(fullPath))); + continue; + } + if ( + entry.isFile() && + entry.name.endsWith('.ts') && + !entry.name.endsWith('.d.ts') && + !entry.name.endsWith('.test.ts') + ) { + out.push(fullPath); + } + } + + return out; +} + +function asRecord(value: unknown): Record | undefined { + return value && typeof value === 'object' && !Array.isArray(value) + ? (value as Record) + : undefined; +} + +function asString(value: unknown): string | undefined { + return typeof value === 'string' && value.length > 0 ? value : undefined; +} + +function asStringArray( + value: unknown, + manifestPath: string, + field: 'requires' | 'provides', +): string[] { + if (value === undefined) return []; + if (!Array.isArray(value)) { + throw new Error( + `${relativeToMiddleware(manifestPath)} field '${field}' must be an array of strings`, + ); + } + const out: string[] = []; + for (const entry of value) { + if (typeof entry !== 'string' || entry.length === 0) { + throw new Error( + `${relativeToMiddleware(manifestPath)} field '${field}' contains a non-string capability entry`, + ); + } + out.push(entry); + } + return out; +} + +function relativeToMiddleware(absPath: string): string { + return path.relative(MIDDLEWARE_ROOT, absPath).replaceAll(path.sep, '/'); +} + +function formatNodeLocation(sourceFile: ts.SourceFile, node: ts.Node): string { + const pos = sourceFile.getLineAndCharacterOfPosition(node.getStart()); + return `${relativeToMiddleware(sourceFile.fileName)}:${String(pos.line + 1)}`; +} diff --git a/middleware/test/pluginServiceGrantGate.test.ts b/middleware/test/pluginServiceGrantGate.test.ts new file mode 100644 index 000000000..fa180dd5c --- /dev/null +++ b/middleware/test/pluginServiceGrantGate.test.ts @@ -0,0 +1,580 @@ +/** + * Epic #470 — B1: `ctx.services.get` is grant-gated. + * + * Before this, `pluginContext.ts` handed the service registry straight + * through. Any installed plugin could resolve any registered service — the + * `graphPool` Postgres pool included — with no manifest declaration, no + * operator consent, and nothing in the install dialog. + * + * These tests pin the four properties that make the gate worth having: + * 1. an undeclared capability THROWS, and the error names both the + * capability and the manifest field that would grant it; + * 2. a declared one (via `requires`, or `provides` for reading back your + * own registration) resolves exactly as before; + * 3. a per-caller factory is invoked with the KERNEL-known id — a consumer + * cannot talk the provider into attributing the call to someone else; + * 4. the dated legacy allowlist warns once and then allows, and is closed: + * a different plugin, or a different name, still throws. + * + * Counter-proof (documented in the PR): reverting the gate in + * `pluginContext.ts` to `return serviceRegistry.get(name)` makes cases 1 + * and 4 fail — the undeclared read succeeds and no warning is emitted. + */ + +import { strict as assert } from 'node:assert'; +import { describe, it } from 'node:test'; + +import { InMemoryMemoryStore } from '@omadia/memory'; +import { + ServiceNotDeclaredError, + perCallerService, + type KnowledgeGraph, + type ServiceCaller, +} from '@omadia/plugin-api'; + +import type { Plugin } from '../src/api/admin-v1.js'; +import { adaptManifestV1 } from '../src/plugins/manifestLoader.js'; +import type { PluginCatalog } from '../src/plugins/manifestLoader.js'; +import { createPluginContext } from '../src/platform/pluginContext.js'; +import type { CreatePluginContextOptions } from '../src/platform/pluginContext.js'; +import { + KERNEL_SERVICE_CALLER, + ServiceRegistry, +} from '../src/platform/serviceRegistry.js'; +import { + LEGACY_UNDECLARED_SERVICE_GRANTS_2026_08_20, + classifyServiceGrant, + declaredServiceNames, +} from '../src/platform/pluginServiceGrants.js'; + +// --- fixtures -------------------------------------------------------------- + +/** + * Build a real `Plugin` through the real manifest adapter rather than a + * hand-rolled object literal: the gate reads `plugin.requires` / + * `plugin.provides`, and those fields are produced by `adaptManifestV1`. A + * fixture that bypasses the adapter could pass while the adapter drops the + * very entries the gate depends on. + */ +function pluginOf( + id: string, + requires: string[], + provides: string[] = [], + permissions: Record = {}, +): Plugin { + const manifest = { + schema_version: '1', + identity: { + id, + name: id, + version: '1.0.0', + kind: 'extension', + domain: 'test.gate', + }, + requires, + provides, + permissions, + }; + const plugin = adaptManifestV1(manifest); + assert.ok(plugin, `fixture manifest for ${id} must adapt`); + rawManifestByPlugin.set(plugin, manifest); + return plugin; +} + +/** + * The raw manifest document behind each fixture plugin. + * + * `adaptManifestV1` does not carry every permission block onto the adapted + * `Plugin`: `memoryDeclared` reads `permissions.memory` off the catalog + * entry's UNPARSED `manifest` field, not off `permissions_summary`. A fixture + * that stored `manifest: {}` therefore silently produced `ctx.memory === + * undefined` no matter what permissions it declared — the accessor under test + * was never built, so the test could only ever assert on nothing. + */ +const rawManifestByPlugin = new WeakMap(); + +function catalogOf(...plugins: Plugin[]): PluginCatalog { + const entries = new Map( + plugins.map((plugin) => [ + plugin.id, + { + plugin, + manifest: rawManifestByPlugin.get(plugin) ?? {}, + source_path: 'test', + source_kind: 'manifest-v1', + }, + ]), + ); + return { + get: (id: string) => entries.get(id), + list: () => [...entries.values()], + } as unknown as PluginCatalog; +} + +interface CtxFixture { + ctx: ReturnType; + registry: ServiceRegistry; + logs: string[]; +} + +function makeCtx( + agentId: string, + catalog: PluginCatalog, + registry = new ServiceRegistry(), +): CtxFixture { + const stub = (): (() => void) => (): void => {}; + const logs: string[] = []; + const ctx = createPluginContext({ + agentId, + vault: { + get: async (): Promise => undefined, + listKeys: async (): Promise => [], + }, + registry: { has: () => true, list: () => [], get: () => undefined }, + catalog, + serviceRegistry: registry, + nativeToolRegistry: { register: stub, registerHandler: stub }, + routeRegistry: { register: stub, disposeBySource: () => 0 }, + jobScheduler: { register: stub, stopForPlugin: (): void => {} }, + notificationRouter: { dispatch: (): void => {}, registerChannel: stub }, + uiRouteCatalog: { register: stub, registerNav: stub }, + logger: (...args: unknown[]): void => { + logs.push(args.map(String).join(' ')); + }, + } as unknown as CreatePluginContextOptions); + return { ctx, registry, logs }; +} + +// --- 1. undeclared is denied ---------------------------------------------- + +describe('services.get — undeclared capabilities are denied', () => { + it('throws ServiceNotDeclaredError for a capability the manifest never mentions', () => { + const catalog = catalogOf(pluginOf('@test/consumer', ['knowledgeGraph@^1'])); + const { ctx, registry } = makeCtx('@test/consumer', catalog); + registry.provide('graphPool', { pool: 'the real one' }); + + assert.throws( + () => ctx.services.get('graphPool'), + (err: unknown) => { + assert.ok( + err instanceof ServiceNotDeclaredError, + 'must be the typed error, not a generic throw', + ); + assert.equal(err.capability, 'graphPool'); + assert.equal(err.pluginId, '@test/consumer'); + assert.equal(err.manifestField, 'requires'); + assert.match(err.message, /graphPool/); + assert.match(err.message, /requires:/); + return true; + }, + ); + }); + + it('denies even when no provider is registered — a manifest bug is not a missing provider', () => { + const catalog = catalogOf(pluginOf('@test/consumer', [])); + const { ctx } = makeCtx('@test/consumer', catalog); + assert.throws( + () => ctx.services.get('tigrisStore'), + ServiceNotDeclaredError, + 'undeclared must throw rather than silently return undefined — otherwise the plugin author cannot tell the two apart', + ); + }); + + it('denies a plugin the kernel has no manifest for at all', () => { + const { ctx, registry } = makeCtx('@test/ghost', catalogOf()); + registry.provide('graphPool', { pool: 1 }); + assert.throws( + () => ctx.services.get('graphPool'), + ServiceNotDeclaredError, + 'no catalog entry means no declarations to check, and unknown permissions are denied permissions', + ); + }); + + it('leaves `has` ungated — existence is not a capability', () => { + const catalog = catalogOf(pluginOf('@test/consumer', [])); + const { ctx, registry } = makeCtx('@test/consumer', catalog); + registry.provide('graphPool', { pool: 1 }); + assert.equal(ctx.services.has('graphPool'), true); + }); +}); + +// --- 2. declared resolves -------------------------------------------------- + +describe('services.get — declared capabilities resolve', () => { + it('resolves a capability listed in `requires`', () => { + const catalog = catalogOf(pluginOf('@test/consumer', ['knowledgeGraph@^1'])); + const { ctx, registry } = makeCtx('@test/consumer', catalog); + const impl = { kg: true }; + registry.provide('knowledgeGraph', impl); + assert.equal(ctx.services.get('knowledgeGraph'), impl); + }); + + it('accepts both `name@1` and `name@^1` spellings', () => { + const catalog = catalogOf(pluginOf('@test/consumer', ['memoryStore@1'])); + const { ctx, registry } = makeCtx('@test/consumer', catalog); + registry.provide('memoryStore', { m: 1 }); + assert.deepEqual(ctx.services.get('memoryStore'), { m: 1 }); + }); + + it('lets a provider read back the capability it provides', () => { + const catalog = catalogOf( + pluginOf('@test/provider', [], ['memoryStore@1']), + ); + const { ctx } = makeCtx('@test/provider', catalog); + ctx.services.provide('memoryStore', { own: true }); + assert.deepEqual(ctx.services.get('memoryStore'), { own: true }); + assert.equal( + classifyServiceGrant( + '@test/provider', + 'memoryStore', + declaredServiceNames('@test/provider', catalog), + catalog, + ), + 'self-provided', + ); + }); + + it('returns undefined — not a throw — for a declared capability with no provider installed', () => { + const catalog = catalogOf(pluginOf('@test/consumer', ['knowledgeGraph@^1'])); + const { ctx } = makeCtx('@test/consumer', catalog); + assert.equal(ctx.services.get('knowledgeGraph'), undefined); + }); +}); + +// --- 3. per-caller factory attribution ------------------------------------ + +describe('services.provide — per-caller factories are kernel-attributed', () => { + it('invokes the factory with the kernel-known id, not one the caller supplies', () => { + const catalog = catalogOf( + pluginOf('@test/provider', [], ['repoGrants@1']), + pluginOf('@test/consumer', ['repoGrants@^1']), + ); + const registry = new ServiceRegistry(); + const seen: ServiceCaller[] = []; + + const provider = makeCtx('@test/provider', catalog, registry); + provider.ctx.services.provide( + 'repoGrants', + perCallerService((caller) => { + seen.push(caller); + return { scopedTo: caller.pluginId }; + }), + ); + + const consumer = makeCtx('@test/consumer', catalog, registry); + const accessor = consumer.ctx.services.get<{ scopedTo: string }>('repoGrants'); + + assert.deepEqual(accessor, { scopedTo: '@test/consumer' }); + assert.equal(seen.length, 1); + assert.equal(seen[0]?.agentId, '@test/consumer'); + assert.equal( + seen[0]?.pluginId, + '@test/consumer', + 'attribution must come from the id the kernel activated the plugin under', + ); + }); + + it('cannot be spoofed — the consumer has no argument that reaches the factory', () => { + const catalog = catalogOf( + pluginOf('@test/provider', [], ['repoGrants@1']), + pluginOf('@test/evil', ['repoGrants@^1']), + ); + const registry = new ServiceRegistry(); + makeCtx('@test/provider', catalog, registry).ctx.services.provide( + 'repoGrants', + perCallerService((caller) => ({ scopedTo: caller.pluginId })), + ); + + const evil = makeCtx('@test/evil', catalog, registry); + // The only surface a consumer controls is the service NAME. There is no + // second parameter through which it could name itself `@test/provider`. + const accessor = evil.ctx.services.get<{ scopedTo: string }>('repoGrants'); + assert.equal(accessor?.scopedTo, '@test/evil'); + assert.equal( + (evil.ctx.services.get as (n: string, ...rest: unknown[]) => unknown) + .length, + 1, + 'services.get takes exactly one parameter — the name', + ); + }); + + it('reuses the same instance for repeated reads by the same caller', () => { + const catalog = catalogOf( + pluginOf('@test/provider', [], ['repoGrants@1']), + pluginOf('@test/a', ['repoGrants@^1']), + ); + const registry = new ServiceRegistry(); + let calls = 0; + makeCtx('@test/provider', catalog, registry).ctx.services.provide( + 'repoGrants', + perCallerService((caller) => { + calls += 1; + return { scopedTo: caller.agentId, token: Symbol(caller.agentId) }; + }), + ); + + const ctx = makeCtx('@test/a', catalog, registry).ctx; + const first = ctx.services.get<{ + scopedTo: string; + token: symbol; + }>('repoGrants'); + const second = ctx.services.get<{ + scopedTo: string; + token: symbol; + }>('repoGrants'); + + assert.equal(calls, 1); + assert.equal(first, second); + assert.equal(first?.scopedTo, '@test/a'); + }); + + it('mints a distinct implementation per consuming plugin', () => { + const catalog = catalogOf( + pluginOf('@test/provider', [], ['repoGrants@1']), + pluginOf('@test/a', ['repoGrants@^1']), + pluginOf('@test/b', ['repoGrants@^1']), + ); + const registry = new ServiceRegistry(); + makeCtx('@test/provider', catalog, registry).ctx.services.provide( + 'repoGrants', + perCallerService((caller) => ({ scopedTo: caller.agentId })), + ); + + const a = makeCtx('@test/a', catalog, registry).ctx.services.get<{ + scopedTo: string; + }>('repoGrants'); + const b = makeCtx('@test/b', catalog, registry).ctx.services.get<{ + scopedTo: string; + }>('repoGrants'); + + assert.equal(a?.scopedTo, '@test/a'); + assert.equal(b?.scopedTo, '@test/b'); + assert.notEqual(a, b); + }); + + it('treats a replaced provider as a cold cache because the factory object changed', () => { + const catalog = catalogOf( + pluginOf('@test/provider', [], ['repoGrants@1']), + pluginOf('@test/a', ['repoGrants@^1']), + ); + const registry = new ServiceRegistry(); + const providerCtx = makeCtx('@test/provider', catalog, registry).ctx; + providerCtx.services.provide( + 'repoGrants', + perCallerService(() => ({ generation: 'first' as const })), + ); + + const consumerCtx = makeCtx('@test/a', catalog, registry).ctx; + const first = consumerCtx.services.get<{ generation: string }>('repoGrants'); + + providerCtx.services.replace( + 'repoGrants', + perCallerService(() => ({ generation: 'second' as const })), + ); + const second = + consumerCtx.services.get<{ generation: string }>('repoGrants'); + + assert.equal(first?.generation, 'first'); + assert.equal(second?.generation, 'second'); + assert.notEqual(first, second); + }); + + it('attributes core’s own direct registry reads to the kernel', () => { + const registry = new ServiceRegistry(); + registry.provide( + 'repoGrants', + perCallerService((caller) => ({ scopedTo: caller.pluginId })), + ); + assert.deepEqual(registry.get('repoGrants'), { + scopedTo: KERNEL_SERVICE_CALLER.pluginId, + }); + }); + + it('keeps value providers untouched — including a value that IS a function', () => { + const catalog = catalogOf( + pluginOf('@test/provider', [], ['callable@1']), + pluginOf('@test/consumer', ['callable@^1']), + ); + const registry = new ServiceRegistry(); + const fn = (): string => 'i am the service itself'; + makeCtx('@test/provider', catalog, registry).ctx.services.provide( + 'callable', + fn, + ); + const got = makeCtx('@test/consumer', catalog, registry).ctx.services.get< + typeof fn + >('callable'); + assert.equal( + got, + fn, + 'a plain function registration must be returned as-is, never mistaken for a factory', + ); + assert.equal(got?.(), 'i am the service itself'); + }); +}); + +describe('plugin-facing accessors keep per-caller attribution', () => { + it('builds ctx.memory from the consuming plugin caller, never @omadia/core', () => { + const catalog = catalogOf( + pluginOf('@test/memory-user', [], [], { + memory: { reads: ['notes'] }, + }), + ); + const registry = new ServiceRegistry(); + const seen: ServiceCaller[] = []; + registry.provide( + 'memoryStore', + perCallerService((caller) => { + seen.push(caller); + return new InMemoryMemoryStore(); + }), + ); + + const ctx = makeCtx('@test/memory-user', catalog, registry).ctx; + assert.ok(ctx.memory); + assert.equal(seen.length, 1); + assert.equal(seen[0]?.pluginId, '@test/memory-user'); + assert.equal(seen[0]?.agentId, '@test/memory-user'); + }); + + it('resolves the knowledge-graph accessor with the consuming plugin caller', async () => { + const catalog = catalogOf( + pluginOf('@test/kg-user', [], [], { + graph: { entity_systems: ['notes'] }, + }), + ); + const registry = new ServiceRegistry(); + const seen: ServiceCaller[] = []; + registry.provide( + 'knowledgeGraph', + perCallerService((caller) => { + seen.push(caller); + return { + stats: async () => ({ + nodes: 0, + edges: 0, + byNodeType: {}, + byEdgeType: {}, + }), + } as unknown as KnowledgeGraph; + }), + ); + + const ctx = makeCtx('@test/kg-user', catalog, registry).ctx; + assert.ok(ctx.knowledgeGraph); + await ctx.knowledgeGraph!.stats(); + assert.equal(seen.length, 1); + assert.equal(seen[0]?.pluginId, '@test/kg-user'); + assert.equal(seen[0]?.agentId, '@test/kg-user'); + }); +}); + +// --- 4. the dated legacy allowlist ---------------------------------------- + +describe('services.get — dated legacy allowlist (2026-08-20)', () => { + const LEGACY_ID = '@omadia/orchestrator'; + + it('warns exactly once per capability and then allows', () => { + const catalog = catalogOf(pluginOf(LEGACY_ID, ['knowledgeGraph@^1'])); + const { ctx, registry, logs } = makeCtx(LEGACY_ID, catalog); + const pool = { pool: true }; + registry.provide('graphPool', pool); + + assert.equal(ctx.services.get('graphPool'), pool); + assert.equal(ctx.services.get('graphPool'), pool); + assert.equal(ctx.services.get('graphPool'), pool); + + const warnings = logs.filter((l) => l.includes("resolved 'graphPool'")); + assert.equal( + warnings.length, + 1, + 'a service resolved in a per-turn hot path must not flood the log', + ); + assert.match(warnings[0] ?? '', /legacy allowlist/); + assert.match(warnings[0] ?? '', /2026-08-20/); + }); + + it('is closed: another plugin asking for the same name still throws', () => { + const catalog = catalogOf(pluginOf('@test/newcomer', [])); + const { ctx, registry } = makeCtx('@test/newcomer', catalog); + registry.provide('graphPool', { pool: true }); + assert.throws( + () => ctx.services.get('graphPool'), + ServiceNotDeclaredError, + 'the allowlist grandfathers audited pairs, it does not whitelist names globally', + ); + }); + + it('is closed: an allowlisted plugin asking for a NEW name still throws', () => { + const catalog = catalogOf(pluginOf(LEGACY_ID, [])); + const { ctx, registry } = makeCtx(LEGACY_ID, catalog); + registry.provide('somethingNew', { x: 1 }); + assert.throws(() => ctx.services.get('somethingNew'), ServiceNotDeclaredError); + }); + + it('classifies a declared name as declared even when it is also allowlisted', () => { + // `@omadia/orchestrator` is allowlisted for `graphPool`; once the manifest + // declares it, the row is dead weight and the classification says so. + const catalog = catalogOf(pluginOf(LEGACY_ID, ['graphPool@^1'])); + assert.equal( + classifyServiceGrant( + LEGACY_ID, + 'graphPool', + declaredServiceNames(LEGACY_ID, catalog), + catalog, + ), + 'declared', + ); + }); + + it('is frozen, so nothing can widen it at runtime', () => { + assert.equal( + Object.isFrozen(LEGACY_UNDECLARED_SERVICE_GRANTS_2026_08_20), + true, + ); + assert.throws(() => { + ( + LEGACY_UNDECLARED_SERVICE_GRANTS_2026_08_20 as Record< + string, + readonly string[] + > + )['@test/attacker'] = ['graphPool']; + }); + assert.equal( + LEGACY_UNDECLARED_SERVICE_GRANTS_2026_08_20['@test/attacker'], + undefined, + ); + // and the per-plugin arrays too — freezing only the outer object would + // leave `[...].push('graphPool')` open. + assert.throws(() => { + ( + LEGACY_UNDECLARED_SERVICE_GRANTS_2026_08_20[ + '@omadia/orchestrator' + ] as string[] + ).push('anthropicClient'); + }); + }); +}); + +// --- 5. the declaration reader -------------------------------------------- + +describe('declaredServiceNames', () => { + it('unions requires and provides, stripping the version', () => { + const catalog = catalogOf( + pluginOf('@test/p', ['a@^1', 'b@2'], ['c@1', 'd@^3']), + ); + assert.deepEqual( + [...declaredServiceNames('@test/p', catalog)].sort(), + ['a', 'b', 'c', 'd'], + ); + }); + + it('drops malformed entries instead of granting them', () => { + const catalog = catalogOf(pluginOf('@test/p', ['no-version', 'ok@1'])); + assert.deepEqual([...declaredServiceNames('@test/p', catalog)], ['ok']); + }); + + it('grants nothing for an unknown plugin', () => { + assert.equal(declaredServiceNames('@test/nobody', catalogOf()).size, 0); + }); +}); diff --git a/specs/470-dev-platform-plugin/README.md b/specs/470-dev-platform-plugin/README.md index f11fb8f9a..e62e23e26 100644 --- a/specs/470-dev-platform-plugin/README.md +++ b/specs/470-dev-platform-plugin/README.md @@ -164,7 +164,7 @@ node scripts/check-core-decoupling.mjs --update # lower the baseline ``` The ratchet counts Dev Platform references across 14 disjoint zones and **fails if the count -rises, per zone**. Baseline **3,288**. It only ever falls; raising it needs a hand-edited baseline, so +rises, per zone**. Baseline **3,300**. It only ever falls; raising it needs a hand-edited baseline, so a new coupling shows up in review instead of slipping in. That is what makes the checklist's staleness survivable — a file inventory goes stale on @@ -189,6 +189,18 @@ regression. That has happened three times (PR #529, then #537's embedding work): the guard fires, the raise is hand-edited, and the reason is recorded in the commit. A rise is only wrong when *core* re-acquires a dependency. +**C2b is a third kind of rise, and the smallest: +5, all documentation of a removal.** +`middleware/packages` 84 → 89, from the new `packages/plugin-api/CHANGELOG.md`. A changelog +recording that the `DevJob*` types were deleted has to name them, or a consumer grepping its +own source for `DevJobDescriptor` finds nothing and the record is worthless. Three of the five +lines are literal strings that cannot be reworded at all — a spec path, a test filename, and +the future package name `@omadia/dev-platform-plugin-api`; the other two are the removal +heading and the six type names, deliberately collapsed onto one line each. The count was +first +31: the example capability in the new gate tests and a `perCallerService` docblock both +used `devJobs` gratuitously, and both were reworded to a neutral name rather than excused — +`middleware/test` is back at its baseline of 1,030, unchanged. Nothing in core references the +dev platform as a result of this PR; the residue is prose *about* code that left. + **Definition of done:** ratchet reads `0`, every row of `acceptance.md` §2 passes, and the install/uninstall criteria in `acceptance.md` §3 pass. diff --git a/specs/470-dev-platform-plugin/acceptance.md b/specs/470-dev-platform-plugin/acceptance.md index 26f763eaa..4707820be 100644 --- a/specs/470-dev-platform-plugin/acceptance.md +++ b/specs/470-dev-platform-plugin/acceptance.md @@ -16,7 +16,7 @@ every row passes *and* the decoupling ratchet reads zero. | Guard | What it proves | Status | |---|---|---| -| `scripts/check-core-decoupling.mjs` + CI job `core decoupling ratchet (#470)` | Core does not re-acquire Dev Platform references while the extraction is in flight | **In place.** Baseline **3,288** across **14** zones, per-zone regression check | +| `scripts/check-core-decoupling.mjs` + CI job `core decoupling ratchet (#470)` | Core does not re-acquire Dev Platform references while the extraction is in flight | **In place.** Baseline **3,300** across **14** zones, per-zone regression check | | `middleware/test/devplatform/**` (54 files) | The behaviour itself, at unit/integration level. These **move with the plugin** and must stay green in the new repo | In place, moves in P4 | | §2 capability matrix below | Nothing is silently dropped in the move | **Written here; not yet automated** | | §3 install/uninstall | The result is genuinely installable | **Not yet built** — needs P3/P4 | @@ -48,6 +48,14 @@ Two flaws were found and fixed, and one limitation is inherent: dev-platform-shaped". It does **not** mean the plugin works, that nothing was lost, or that a coupling expressed without a matching identifier is gone. §2 and §3 are what cover those, and neither is automated yet. +- **Inherent — it counts prose as well as code.** C2b raised `middleware/packages` 84 → 89 + (baseline 3,296 → 3,300) purely for `packages/plugin-api/CHANGELOG.md`, which has to name + the `DevJob*` types in order to record that they were removed. Three of those five lines + are unrewordable literals (a spec path, a test filename, the future + `@omadia/dev-platform-plugin-api` package name). The first measurement was +31; the + avoidable 26 were reworded away rather than excused, leaving `middleware/test` at its + 1,030 baseline. Documenting a removal counts the same as performing one — worth knowing + before reading a raise as a regression. So: the ratchet is a necessary condition for done, not a sufficient one. diff --git a/specs/470-dev-platform-plugin/decoupling-baseline.json b/specs/470-dev-platform-plugin/decoupling-baseline.json index 08cc7bcd4..ff3bec6e4 100644 --- a/specs/470-dev-platform-plugin/decoupling-baseline.json +++ b/specs/470-dev-platform-plugin/decoupling-baseline.json @@ -1,9 +1,9 @@ { - "total": 3296, + "total": 3300, "zones": { "middleware/src": 1576, "middleware/test": 1030, - "middleware/packages": 84, + "middleware/packages": 89, "middleware/scripts": 8, "middleware/sidecars": 195, "middleware/migrations": 69, diff --git a/specs/470-dev-platform-plugin/plan.md b/specs/470-dev-platform-plugin/plan.md index a4505215f..1041eaa04 100644 --- a/specs/470-dev-platform-plugin/plan.md +++ b/specs/470-dev-platform-plugin/plan.md @@ -243,8 +243,14 @@ exactly one consumer; extracting a package for one consumer is speculative gener ### 4.2 What stays in core -- `DevJob*` types in `@omadia/plugin-api` — a published, versioned contract that - third-party plugins consume via `ctx.devJobs`. +- ~~`DevJob*` types in `@omadia/plugin-api` — a published, versioned contract that + third-party plugins consume via `ctx.devJobs`.~~ **Superseded — §4.1 wins.** + `implementation.md` §2.5 recorded that §4.1 and §4.2 contradicted each other on this + point and that the plugin-owned answer is the correct one; C2a (#555) then deleted the + accessor outright and C2b removed the types from the package. They are core-local in + `middleware/src/` today and leave with the extraction. There were never any third-party + consumers — nothing ever provided the backing host service, so every call threw + (`dormant-capabilities.md` §2). - `DevJobStepPort` / `devJobStepEffect.ts` — core-owned conductor port interfaces, already devplatform-free by design. - `mintAppJwt` — moves **out** of devplatform into `src/platform/githubAppJwt.ts`, closing