diff --git a/.changeset/runtime-config-install-local-derived.md b/.changeset/runtime-config-install-local-derived.md new file mode 100644 index 0000000000..4875193fc8 --- /dev/null +++ b/.changeset/runtime-config-install-local-derived.md @@ -0,0 +1,59 @@ +--- +"@objectstack/cloud-connection": patch +--- + +fix(cloud-connection): `features.installLocal` is derived from what is mounted; the constructor option becomes a ceiling (#8388) + +`GET /api/v1/runtime/config` reported `installLocal` straight from the +constructor option, next to a `marketplace` key that #8356 had just made an +observation. Two flags in one object, answered by different rules — and the +declared one is the key #8343 actually measured wrong on a real self-hosted +deployment: + +```json +{"features":{"installLocal":true,"marketplace":true, …}} +``` + +``` +GET /api/v1/marketplace/install-local -> 404 {"error":"Not found"} +POST /api/v1/marketplace/install-local -> 404 {"error":"Not found"} +``` + +Nothing checked that `MarketplaceInstallLocalPlugin` was mounted on the kernel +serving the response, so `new RuntimeConfigPlugin({ installLocal: true })` on a +runtime that never mounted it announced a capability whose route 404s, and the +Console rendered an install affordance that could not work. + +The flag is now **observed** per request, off the route table of the app serving +the response — the same seam #8356 built, read by a sibling predicate rather than +a shared one, because the browse predicate subtracts exactly the paths this one +requires. The two share the prefix constant, so "what counts as install-local" +has one definition and the flags cannot both claim, or both disown, the same +route. + +**The `installLocal` constructor option is kept, as a ceiling.** Hosts pass it +today, so it is not removed: + +- omitted or `true` — report what is actually mounted (what every host passing + `installLocal: true` already meant); +- `false` — report `false` even where the plugin is mounted, for an operator who + wants the affordance hidden. + +It deliberately cannot raise the answer. A plain override would have left the +measured defect standing: the CLI's own frozen `RUNTIME_CONFIG_OPTIONS` passes +`installLocal: true` unconditionally, so honouring `true` upward would keep +"declared `true`, route 404s" reachable on exactly the product path #8343 +reported, leaving the derivation inert where it is most needed. + +**What changes for hosts.** A runtime that mounts an install-local surface +reports exactly what it did before. A runtime that mounts none now reports +`installLocal: false` instead of whatever it declared — the correction. An +omitted option no longer means `false`: it defers to the observation, so a host +that mounts the plugin and forgot the flag now gets the truthful `true` it should +always have had. + +**Escape hatch, unchanged.** The derivation is the base value, not a veto: the +open-core `resolveFeatures` seam still merges over it (and over the ceiling), so a +host on an adapter whose raw app exposes no route ledger — where both derived +flags conservatively report `false`, with a warning logged once at mount time — +can still declare the capability it knows it serves. diff --git a/packages/cli/test/serve-marketplace-offline-runtime-config.test.ts b/packages/cli/test/serve-marketplace-offline-runtime-config.test.ts index e0bbef2968..4261979fb8 100644 --- a/packages/cli/test/serve-marketplace-offline-runtime-config.test.ts +++ b/packages/cli/test/serve-marketplace-offline-runtime-config.test.ts @@ -311,13 +311,35 @@ describe('#8389: the wiring plan mounts runtime-config on the offline arm', () = // One shared gate would have been the smaller change and would have // excluded exactly this box: it serves install-local (its own), serves no // runtime-config, and therefore has the #8389 defect in full. + // + // [#8388] "the host wires its OWN install-local" now has to mean the host + // MOUNTS one. `INSTALL_LOCAL` below is only an identity handed to the + // resolver, and since `features.installLocal` became an observation of the + // route table rather than the constructor flag, an identity with no route + // behind it models a box announcing a capability it cannot serve — #8343's + // measured defect, which is not this case's subject. So the host's own + // plugin is really started on the same app, ahead of the arm, exactly as + // the positive control mounts the real proxy. The assertions below are + // unchanged; what changed is that the fixture now earns them. const dir = tempStorageDir(); try { - const { wiring, app } = await bootOfflineArm({ plugins: [INSTALL_LOCAL], storageDir: dir }); + const { MarketplaceInstallLocalPlugin } = await import('@objectstack/cloud-connection'); + const { wiring, app } = await bootOfflineArm({ + plugins: [INSTALL_LOCAL], + storageDir: dir, + preMounted: [new MarketplaceInstallLocalPlugin({ + controlPlaneUrl: Serve.OFFLINE_CONTROL_PLANE, + storageDir: dir, + })], + }); expect(wiring.offlineInstallLocal, "the host's own install-local is left alone").toBe(false); expect(wiring.offlineRuntimeConfig).toBe(true); + // ...and the host's own mount is the ONLY install-local surface here — + // the arm added none, which is what `offlineInstallLocal: false` means. + expect(app.routes.some((r) => r.path.startsWith('/api/v1/marketplace/install-local'))).toBe(true); + const body = await readConfig(app); expect(body.features.installLocal).toBe(true); expect(body.features.marketplace).toBe(false); diff --git a/packages/cloud-connection/README.md b/packages/cloud-connection/README.md index d40d737e08..8c3dc6d2d2 100644 --- a/packages/cloud-connection/README.md +++ b/packages/cloud-connection/README.md @@ -40,9 +40,14 @@ const plugins = [ // DEFAULT_CLOUD_URL. 'off' is one of the documented disable sentinels and // is the value that actually resolves to no cloud. new MarketplaceInstallLocalPlugin({ controlPlaneUrl: cloudUrl || 'off' }), - // NOT cloud-gated: features.marketplace is derived from what is actually - // mounted, not from this constructor call, so a cloud-less runtime reports - // marketplace: false on its own — there is nothing here to keep in sync. + // NOT cloud-gated: BOTH features.marketplace and features.installLocal are + // derived from what is actually mounted, not from this constructor call, so + // a cloud-less runtime reports marketplace: false and installLocal: true on + // its own — there is nothing here to keep in sync. + // `installLocal: true` is therefore a CEILING, not a declaration: it is the + // default, and it cannot make the flag report a route this runtime never + // mounted. Pass `false` to hide the affordance on a box that could serve it; + // omitting it entirely behaves the same as `true`. // `''` here, unlike its neighbor above, is correct as-is: this plugin does // NOT re-resolve controlPlaneUrl through resolveCloudUrl(), so '' means // "stay on this origin" rather than "unset" — do not "fix" it to 'off'. diff --git a/packages/cloud-connection/src/runtime-config-install-local-derivation.test.ts b/packages/cloud-connection/src/runtime-config-install-local-derivation.test.ts new file mode 100644 index 0000000000..b1d2fe0350 --- /dev/null +++ b/packages/cloud-connection/src/runtime-config-install-local-derivation.test.ts @@ -0,0 +1,333 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `features.installLocal` is an OBSERVATION, with the constructor option kept + * only as a ceiling (#8388). + * + * #8356 derived `features.marketplace` from the serving app's route table and + * stopped there — its acceptance list named that key alone. So the two flags + * in one object were left answered by different rules, one observed and one + * declared, and the declared one is the key #8343 actually measured wrong on a + * real customer deployment: + * + * {"features":{"installLocal":true,"marketplace":true, …}} + * GET /api/v1/marketplace/install-local -> 404 {"error":"Not found"} + * POST /api/v1/marketplace/install-local -> 404 {"error":"Not found"} + * + * ## Why the negative direction is the whole point of this file + * + * A suite that only asserts the mounted case passes unchanged against the + * constructor flag it is meant to replace — it proves nothing about the defect. + * The load-bearing case is `reports false when the host DECLARED true and + * mounted nothing`: that is #8343's payload exactly, and it is the one that + * goes red the moment the derivation is reverted. + * + * ## Why the option is a ceiling and not a plain override + * + * A plain override would have honoured `true` upward, and the CLI's own frozen + * `RUNTIME_CONFIG_OPTIONS` passes `installLocal: true` unconditionally — so the + * derivation would have been inert on precisely the product path #8343 + * reported. Pinned in both directions below: `false` still lowers a mounted + * `true` (the published option keeps a real effect, per the triage ruling that + * it must not be removed), and `true` cannot raise an unmounted `false`. + * + * ## Why the fixtures mount the REAL plugin + * + * The derivation keys on a path prefix `MarketplaceInstallLocalPlugin` owns. A + * hand-spelled fixture would keep agreeing with a stale copy of that spelling + * long after the plugin changed it — the flag would flip to `false` in + * production with this suite still green. + */ + +import { describe, it, expect } from 'vitest'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { RuntimeConfigPlugin, type RuntimeConfigPluginConfig } from './runtime-config-plugin.js'; +import { MarketplaceInstallLocalPlugin } from './marketplace-install-local-plugin.js'; +import { MarketplaceProxyPlugin } from './marketplace-proxy-plugin.js'; + +interface RouteRecord { method: string; path: string } + +interface HonoShapedApp { + routes: RouteRecord[]; + handlers: Map; + get(path: string, handler?: any): void; + post(path: string, handler?: any): void; + put(path: string, handler?: any): void; + delete(path: string, handler?: any): void; + head(path: string, handler?: any): void; + all(path: string, handler?: any): void; + use(path: string, handler?: any): void; +} + +/** A raw app shaped like the Hono instance `getRawApp()` returns. */ +function createApp(): HonoShapedApp { + const routes: RouteRecord[] = []; + const handlers = new Map(); + const record = (method: string) => (path: string, handler?: any) => { + routes.push({ method, path }); + handlers.set(`${method} ${path}`, handler); + }; + return { + routes, + handlers, + get: record('GET'), + post: record('POST'), + put: record('PUT'), + delete: record('DELETE'), + head: record('HEAD'), + all: record('ALL'), + // Hono files middleware into the same ledger, under ALL. + use: record('ALL'), + }; +} + +/** Start a plugin against `app` and fire its `kernel:ready` hooks. */ +async function startOn(app: unknown, plugin: { start(ctx: any): Promise }): Promise { + const hooks: Array<() => Promise> = []; + const warnings: string[] = []; + const services: Record = { + 'http.server': { getRawApp: () => app }, + manifest: { register() {} }, + auth: { api: { getSession: async () => ({ user: { id: 'admin' } }) } }, + objectql: { syncSchemas: async () => undefined }, + }; + const ctx: any = { + logger: { info() {}, warn: (m: unknown) => { warnings.push(String(m)); }, error() {} }, + getService: (name: string) => { + const svc = services[name]; + if (svc === undefined) throw new Error(`no ${name}`); + return svc; + }, + hook: (_event: string, cb: () => Promise) => { hooks.push(cb); }, + }; + await plugin.start(ctx); + for (const cb of hooks) await cb(); + return warnings; +} + +/** Ask the mounted `/api/v1/runtime/config` handler for its payload. */ +async function readConfig(app: HonoShapedApp): Promise { + const handler = app.handlers.get('GET /api/v1/runtime/config'); + if (typeof handler !== 'function') throw new Error('runtime/config was never mounted'); + return handler({ + req: { header: () => undefined }, + json: (body: any) => body, + }); +} + +function runtimeConfig(config: RuntimeConfigPluginConfig = {}): RuntimeConfigPlugin { + return new RuntimeConfigPlugin({ controlPlaneUrl: '', singleEnvironment: true, ...config }); +} + +/** A temp ledger dir so the real install-local plugin touches no shared state. */ +function tempStorageDir(): string { + return mkdtempSync(join(tmpdir(), 'os-8388-')); +} + +/** Run `body` with a temp storage dir, cleaned up either way. */ +async function withStorageDir(body: (dir: string) => Promise): Promise { + const dir = tempStorageDir(); + try { + await body(dir); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +} + +function installLocalPlugin(dir: string): MarketplaceInstallLocalPlugin { + return new MarketplaceInstallLocalPlugin({ controlPlaneUrl: 'off', storageDir: dir }); +} + +describe('features.installLocal — a mounted install-local surface reports true', () => { + it('reports true from the REAL plugin alone, with NO constructor flag at all', async () => { + // The option is not passed, so a green here can only come from the + // route table. Under the old code this exact fixture reported `false`. + await withStorageDir(async (dir) => { + const app = createApp(); + await startOn(app, installLocalPlugin(dir)); + await startOn(app, runtimeConfig()); + + expect(await readConfig(app)).toMatchObject({ features: { installLocal: true } }); + // The fixture is only meaningful if install-local really mounted. + expect(app.routes.some((r) => r.path.startsWith('/api/v1/marketplace/install-local'))).toBe(true); + }); + }); + + it('is read per request, so a plugin mounted AFTER this one still counts', async () => { + // Plugin start() order across kernel:ready hooks is not guaranteed, so + // a mount-time snapshot would report whichever hook happened to run + // first. The handler runs long after every hook — that is the seam. + await withStorageDir(async (dir) => { + const app = createApp(); + await startOn(app, runtimeConfig()); + expect(await readConfig(app)).toMatchObject({ features: { installLocal: false } }); + + await startOn(app, installLocalPlugin(dir)); + expect(await readConfig(app)).toMatchObject({ features: { installLocal: true } }); + }); + }); + + it('counts a sub-path mount on its own — the ledger need not carry the bare prefix', async () => { + // `…/install-local/:manifestId` is a real registration of this plugin's + // (DELETE, and the two sample-data POSTs). A predicate keyed on exact + // equality would miss a runtime that mounted only those. + const app = createApp(); + app.delete('/api/v1/marketplace/install-local/:manifestId', () => {}); + await startOn(app, runtimeConfig()); + + expect(await readConfig(app)).toMatchObject({ features: { installLocal: true } }); + }); +}); + +describe('features.installLocal — nothing mounted reports false', () => { + it('THE #8343 PAYLOAD — reports false where the host DECLARED true and mounted nothing', async () => { + // This is the measured defect, reproduced as a fixture: a host passes + // installLocal: true to a runtime with no install-local route, and the + // Console is told to render an affordance whose endpoint 404s. Revert + // the derivation and this is the case that goes red. + const app = createApp(); + await startOn(app, runtimeConfig({ installLocal: true })); + + expect(await readConfig(app)).toMatchObject({ features: { installLocal: false } }); + expect(app.routes.some((r) => r.path.startsWith('/api/v1/marketplace/install-local'))).toBe(false); + }); + + it('reports false on a runtime with no marketplace mount at all', async () => { + const app = createApp(); + await startOn(app, runtimeConfig()); + + expect(await readConfig(app)).toMatchObject({ features: { installLocal: false } }); + }); + + it('broad middleware and an SPA catch-all are not evidence of an install route', async () => { + const app = createApp(); + app.use('/api/v1/*', () => {}); + app.get('/*', () => {}); + await startOn(app, runtimeConfig({ installLocal: true })); + + expect(await readConfig(app)).toMatchObject({ features: { installLocal: false } }); + }); + + it('an adjacent spelling is not the install-local namespace', async () => { + // Segment boundary, not a bare startsWith: `…/install-locality` is + // somebody else's route and claiming it would be this bug again. + const app = createApp(); + app.get('/api/v1/marketplace/install-locality', () => {}); + await startOn(app, runtimeConfig({ installLocal: true })); + + expect(await readConfig(app)).toMatchObject({ features: { installLocal: false } }); + }); + + it('reports false — and warns once — when the raw app exposes no route table', async () => { + // Do not claim a capability you could not verify; claiming it + // unverified is the defect. The warning makes the downgrade traceable, + // and now has to name BOTH derived keys. + const opaqueApp: any = { + handlers: new Map(), + get(path: string, handler?: any) { this.handlers.set(`GET ${path}`, handler); }, + }; + const warnings = await startOn(opaqueApp, runtimeConfig({ installLocal: true })); + + expect(await readConfig(opaqueApp)).toMatchObject({ features: { installLocal: false } }); + expect(warnings.filter((w) => w.includes('features.installLocal'))).toHaveLength(1); + }); +}); + +describe('features.installLocal — the constructor option survives as a CEILING', () => { + it('an explicit false LOWERS a mounted surface — the published option keeps a real effect', async () => { + // The option is not being retired (hosts pass it today, and deleting a + // published option is a maintainer-floor call). This is what it still + // does: an operator hiding the affordance on a box that could serve it. + await withStorageDir(async (dir) => { + const app = createApp(); + await startOn(app, installLocalPlugin(dir)); + await startOn(app, runtimeConfig({ installLocal: false })); + + expect(app.routes.some((r) => r.path.startsWith('/api/v1/marketplace/install-local'))).toBe(true); + expect(await readConfig(app)).toMatchObject({ features: { installLocal: false } }); + }); + }); + + it('an explicit true cannot RAISE an unmounted surface — a ceiling, not a source', async () => { + // The whole reason it is a ceiling: the CLI's own frozen + // RUNTIME_CONFIG_OPTIONS passes installLocal: true unconditionally, so + // honouring true upward would leave the derivation inert on exactly the + // path #8343 measured. + const app = createApp(); + await startOn(app, runtimeConfig({ installLocal: true })); + + expect(await readConfig(app)).toMatchObject({ features: { installLocal: false } }); + }); + + it('an OMITTED option is not an opt-out — it defers to the observation', async () => { + // Guards the `!== false` vs `!!` distinction in the constructor: the + // old default read an absent option as `false`, which after the + // derivation would have vetoed every truthful `true`. + await withStorageDir(async (dir) => { + const app = createApp(); + await startOn(app, installLocalPlugin(dir)); + await startOn(app, runtimeConfig({})); + + expect(await readConfig(app)).toMatchObject({ features: { installLocal: true } }); + }); + }); + + it('a host that knows better can still declare it through resolveFeatures', async () => { + // The open-core seam is unchanged and is what makes the ceiling safe: + // the derivation is the BASE value, not a veto, so an adapter with no + // observable route table is not a dead end. It outranks the ceiling + // too — resolveFeatures merges last, as it does for every base flag. + const app = createApp(); + await startOn(app, runtimeConfig({ + installLocal: false, + resolveFeatures: () => ({ installLocal: true }), + })); + + expect(await readConfig(app)).toMatchObject({ features: { installLocal: true } }); + }); +}); + +describe('the two derived flags stay independent', () => { + it('install-local alone reports installLocal WITHOUT claiming browse', async () => { + // #8356 excludes the install-local paths from the browse predicate on + // purpose; this is that exclusion observed from the other side. + await withStorageDir(async (dir) => { + const app = createApp(); + await startOn(app, installLocalPlugin(dir)); + await startOn(app, runtimeConfig()); + + expect(await readConfig(app)).toMatchObject({ + features: { installLocal: true, marketplace: false }, + }); + }); + }); + + it('a browse proxy alone reports marketplace WITHOUT claiming install-local', async () => { + // The mirror image, and the case a shared predicate would have broken: + // the REAL proxy mounts `/api/v1/marketplace/*`, which a naive + // "anything under the marketplace namespace" rule would read as an + // install route — re-creating #8343's 404 for the other key. + const app = createApp(); + await startOn(app, new MarketplaceProxyPlugin({ controlPlaneUrl: 'http://cloud.test', cacheDisabled: true })); + await startOn(app, runtimeConfig({ installLocal: true })); + + expect(await readConfig(app)).toMatchObject({ + features: { installLocal: false, marketplace: true }, + }); + }); + + it('both mounted reports both', async () => { + await withStorageDir(async (dir) => { + const app = createApp(); + await startOn(app, installLocalPlugin(dir)); + await startOn(app, new MarketplaceProxyPlugin({ controlPlaneUrl: 'http://cloud.test', cacheDisabled: true })); + await startOn(app, runtimeConfig()); + + expect(await readConfig(app)).toMatchObject({ + features: { installLocal: true, marketplace: true }, + }); + }); + }); +}); diff --git a/packages/cloud-connection/src/runtime-config-plugin.ts b/packages/cloud-connection/src/runtime-config-plugin.ts index 8b878abef7..6b15794aeb 100644 --- a/packages/cloud-connection/src/runtime-config-plugin.ts +++ b/packages/cloud-connection/src/runtime-config-plugin.ts @@ -56,6 +56,34 @@ * the original defect into the EE image, where both the host config and this * package's README kept a hand-maintained flag out of step with their own * mounting. + * + * ## `features.installLocal` is DERIVED too, with the option kept as a CEILING (#8388) + * + * #8356 derived `marketplace` and stopped there, leaving two keys in one + * object answered by different rules — one observed, one declared. The + * declared one was the constructor's `installLocal`, and it is the key #8343 + * actually measured wrong on a real customer deployment: `installLocal: true` + * in the served payload with + * `GET/POST /api/v1/marketplace/install-local -> 404 {"error":"Not found"}` + * behind it. The #8343 ruling's reasoning — a hand-maintained boolean makes + * every host responsible for keeping the flag in step with its own mounting, + * and hosts measurably do not — transfers to this key unchanged; it is simply + * the one the ruling was not asked about. + * + * The `installLocal` constructor option is **kept** (hosts pass it today), but + * it is now a **ceiling, not a source**: + * + * omitted / `true` -> the derived observation governs + * `false` -> `false`, even where the plugin IS mounted (opt-out) + * + * A ceiling rather than a plain override because the plain override would have + * left the measured defect standing: the CLI's own `RUNTIME_CONFIG_OPTIONS` + * passes `installLocal: true` unconditionally, so honouring `true` upward + * would keep "declared `true`, route 404s" reachable on the exact product path + * #8343 reported — the derivation would be inert precisely where it is needed. + * Nothing is lost: a host on an adapter whose routes cannot be observed still + * states the capability through `resolveFeatures`, which merges over this base + * exactly as it does for `marketplace`. */ import type { Plugin, PluginContext } from '@objectstack/core'; @@ -77,13 +105,20 @@ interface EnvRegistrySurface { * a browse surface. * * These literals are deliberately NOT imported from `marketplace-proxy-plugin` - * — the derivation must also see a browse surface this package never mounted - * (see {@link hasMarketplaceBrowseMount}), so keying it on the proxy module - * would narrow it back to one provider. The coupling to the proxy's own - * spelling is instead pinned by test: the positive direction mounts the REAL - * `MarketplaceProxyPlugin` onto the same app rather than hand-spelling its - * route, so a change to its prefix fails here rather than silently flipping - * this flag to `false`. + * / `marketplace-install-local-plugin` — the derivations must also see a + * surface this package never mounted (see {@link hasMarketplaceBrowseMount}), + * so keying them on a provider module would narrow them back to one provider. + * The coupling to each plugin's own spelling is instead pinned by test: both + * positive directions mount the REAL plugin onto the same app rather than + * hand-spelling its route, so a change to either prefix fails there rather + * than silently flipping a flag to `false`. + * + * `MARKETPLACE_INSTALL_LOCAL_PREFIX` is the **one** definition of "what + * install-local is", read in opposite directions by the two predicates below: + * negatively by browse (#8356 excludes it) and positively by install-local + * (#8388 requires it). That single constant is the part that genuinely has to + * be shared — it is what makes it impossible for the two flags to both claim, + * or both disown, the same route if the prefix ever moves. */ const MARKETPLACE_API_PREFIX = '/api/v1/marketplace'; const MARKETPLACE_INSTALL_LOCAL_PREFIX = `${MARKETPLACE_API_PREFIX}/install-local`; @@ -150,11 +185,70 @@ function isMarketplaceBrowsePattern(pattern: string): boolean { * through the `resolveFeatures` seam, which still merges over this base. */ function hasMarketplaceBrowseMount(rawApp: unknown): boolean { + return someRoutePattern(rawApp, isMarketplaceBrowsePattern); +} + +/** + * Does this registered route pattern mount the offline INSTALL-LOCAL surface? + * (#8388) + * + * Sibling of {@link isMarketplaceBrowsePattern}, not a reuse of it: browse + * subtracts these paths by design, so no single predicate can answer both + * questions. What the two share is the prefix constant, so "what counts as + * install-local" has one definition rather than two that can drift apart. + * + * `MarketplaceInstallLocalPlugin` mounts the bare prefix plus `/:manifestId` + * sub-paths, so a segment boundary — not a bare `startsWith` — decides + * membership: `…/install-local` and `…/install-local/anything` count, + * `…/install-locality` does not. That is deliberately one notch stricter than + * browse's exclusion, which subtracts every `startsWith` match. The asymmetry + * is in the safe direction for both keys: a near-miss spelling is claimed by + * neither flag, which is under-reporting, and under-reporting is the failure + * mode this whole family of fixes chose over the alternative. Tightening + * browse's exclusion to match would be a behaviour change to #8356's key and + * is not this card's to make. + */ +function isMarketplaceInstallLocalPattern(pattern: string): boolean { + if (!pattern.startsWith(MARKETPLACE_INSTALL_LOCAL_PREFIX)) return false; + const rest = pattern.slice(MARKETPLACE_INSTALL_LOCAL_PREFIX.length); + return rest === '' || rest.startsWith('/'); +} + +/** + * Is an install-local surface actually mounted on the app serving this + * response? (#8388) + * + * Every word of {@link hasMarketplaceBrowseMount}'s reasoning about *why the + * raw app's route table* applies here too, and one of them applies harder: + * `MarketplaceInstallLocalPlugin` also registers no service to look up — it + * announces itself only by mounting its routes on the raw app — so the route + * ledger is again the only place the question has an answer. Read per request + * for the same reason: `kernel:ready` hook order is not guaranteed, and by + * request time every hook has run. + * + * Unobservable adapter ⇒ `false`, same as browse: do not claim a capability + * you could not verify. That is not a regression against the old constructor + * flag — claiming it unverified IS #8343's measured defect. A host that knows + * better says so through `resolveFeatures`. + */ +function hasMarketplaceInstallLocalMount(rawApp: unknown): boolean { + return someRoutePattern(rawApp, isMarketplaceInstallLocalPattern); +} + +/** + * The route-ledger read both derivations share. + * + * This — not the predicates — is the genuinely common mechanism: locate the + * raw app's `routes` ledger, refuse to answer when there is none, and test + * every registered pattern. The predicates stay separate because they answer + * different questions about the same ledger. + */ +function someRoutePattern(rawApp: unknown, matches: (pattern: string) => boolean): boolean { const routes = (rawApp as { routes?: unknown } | null | undefined)?.routes; if (!Array.isArray(routes)) return false; return routes.some((route) => { const pattern = (route as { path?: unknown } | null | undefined)?.path; - return typeof pattern === 'string' && isMarketplaceBrowsePattern(pattern); + return typeof pattern === 'string' && matches(pattern); }); } @@ -188,7 +282,27 @@ export interface RuntimeConfigPluginConfig { * for marketplace + install). */ controlPlaneUrl?: string; - /** Override the `features.installLocal` flag. Default: false. */ + /** + * CEILING for the `features.installLocal` flag — no longer its source + * (#8388). + * + * The flag is derived from whether an install-local surface is really + * mounted on the app serving the response. This option can only lower that + * answer: + * + * - omitted or `true` — report what is mounted (the default, and what + * every host passing `installLocal: true` today already meant); + * - `false` — report `false` even where the plugin IS mounted, for a + * host that wants the affordance hidden. + * + * It deliberately cannot raise the answer: `true` on a runtime with no + * install-local route is #8343's measured defect (a capability whose route + * 404s), and re-admitting it here would make the derivation inert on the + * CLI's own path, which passes `installLocal: true` unconditionally. A host + * whose adapter exposes no route table, but which knows install-local is + * live, declares it through {@link resolveFeatures} — that hook still + * merges over this base, exactly as it does for `marketplace`. + */ installLocal?: boolean; /** * Override the `features.aiStudio` flag — whether the SPA should surface @@ -245,7 +359,12 @@ export class RuntimeConfigPlugin implements Plugin { readonly version = '1.0.0'; private readonly cloudUrl: string; - private readonly installLocal: boolean; + /** + * `false` only when the host explicitly opted out — see the config option. + * Named for what it now is (a bound on the derived answer) rather than for + * the answer itself, so a future edit cannot mistake it for the source. + */ + private readonly installLocalCeiling: boolean; private readonly aiStudio: boolean; private readonly singleEnvironment: boolean; private readonly productName: string; @@ -263,7 +382,9 @@ export class RuntimeConfigPlugin implements Plugin { this.cloudUrl = config.controlPlaneUrl === '' ? '' : (resolveCloudUrl(config.controlPlaneUrl) ?? ''); - this.installLocal = !!config.installLocal; + // `!== false`, not `!!` — an omitted option must not read as an opt-out + // now that the flag is derived; only an explicit `false` lowers it. + this.installLocalCeiling = config.installLocal !== false; this.aiStudio = config.aiStudio !== false; // default true (override-to-hide) this.singleEnvironment = !!config.singleEnvironment; // Prefer the plan-agnostic seam; fall back to the deprecated alias. @@ -308,15 +429,15 @@ export class RuntimeConfigPlugin implements Plugin { const rawApp = httpServer.getRawApp(); // Diagnosable once at mount time rather than per request: an - // adapter with no observable route ledger makes - // `features.marketplace` report false for the whole process, and - // a silently downgraded capability flag is hard to trace from the - // SPA end. See hasMarketplaceBrowseMount(). + // adapter with no observable route ledger makes BOTH derived flags + // report false for the whole process, and a silently downgraded + // capability flag is hard to trace from the SPA end. See + // hasMarketplaceBrowseMount() / hasMarketplaceInstallLocalMount(). if (!Array.isArray((rawApp as { routes?: unknown } | null | undefined)?.routes)) { ctx.logger?.warn?.( - '[RuntimeConfigPlugin] raw app exposes no route table — features.marketplace will report false ' - + '(a mounted browse surface cannot be observed here). Declare it via resolveFeatures if this ' - + 'runtime does serve marketplace browse.', + '[RuntimeConfigPlugin] raw app exposes no route table — features.marketplace and ' + + 'features.installLocal will report false (a mounted browse or install-local surface cannot ' + + 'be observed here). Declare them via resolveFeatures if this runtime does serve them.', ); } @@ -403,7 +524,12 @@ export class RuntimeConfigPlugin implements Plugin { defaultOrgId, defaultEnvironmentId, features: { - installLocal: this.installLocal, + // Observed, not declared (#8388) — the constructor + // option survives only as a ceiling, so a host cannot + // announce an install route it never mounted (#8343's + // measured symptom) but can still opt out of one it + // did. + installLocal: this.installLocalCeiling && hasMarketplaceInstallLocalMount(rawApp), // Observed, not declared (#8356) — re-read per request // because it is a property of the app, not of this // plugin's config. A host's resolveFeatures still