From c50ecb1477e5c48fa13d995ff215c53ca6327f8e Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Aug 2026 07:31:32 +0000 Subject: [PATCH 1/3] fix(service-automation): hoist the retry-backoff helper out of the class body so esbuild stops renaming AutomationServicePlugin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `private static` called from an instance method made the class reference itself by name inside its own body; esbuild emits `var X = class _X { … }` for that shape, so the shipped class was called `_AutomationServicePlugin` and the `automation` capability's class-name identity in the CLI matched nothing. Isolated in its own commit: `packages/services/service-automation` is OUTSIDE this card's declared file surface. It is the same one-line idiom as #8645's named instance, surfaced by the enumeration the card mandates, and the guard cannot be green without it — see the PR body. Refs #8645 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NaS1PAHJcPfAA2acnV53Tn --- .../services/service-automation/src/plugin.ts | 29 ++++++++++++++----- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/packages/services/service-automation/src/plugin.ts b/packages/services/service-automation/src/plugin.ts index f7ecd0d23a..6086985084 100644 --- a/packages/services/service-automation/src/plugin.ts +++ b/packages/services/service-automation/src/plugin.ts @@ -225,6 +225,26 @@ export const DECLARATIVE_RETRY_BASE_MS = 5_000; /** Ceiling for the degraded-instance retry backoff (#3017). */ export const DECLARATIVE_RETRY_MAX_MS = 300_000; +/** + * Backoff for degraded-instance retries (#3017): base · 2^(attempts-1), capped. + * + * At module scope rather than as a `private static` on the plugin, and that is + * load-bearing rather than stylistic (#8645): its one call site sits in an + * INSTANCE method, so the static spelling made the class reference itself by + * name inside its own body. esbuild rewrites such a class into + * `var X = class _X { … _X … }` — binding the inner reference to the class + * binding — and the emitted class then reports `_X` as its `.name`. The CLI's + * `Serve.providesCapability` recognises a host-supplied provider by + * `constructor.name` as well as `plugin.name`, so against the shipped build the + * `AutomationServicePlugin` class-name identity matched nothing and the + * `automation` capability guard was running on its registered-id limb alone. + * `packages/cli/test/serve-capability-identity.test.ts` now enforces the + * equality; keep self-references out of plugin class bodies. + */ +function declarativeRetryDelayMs(attempts: number): number { + return Math.min(DECLARATIVE_RETRY_BASE_MS * 2 ** Math.max(0, attempts - 1), DECLARATIVE_RETRY_MAX_MS); +} + /** * Deterministic JSON stringify (keys sorted at every level) so a signature is * stable regardless of authored key order — two materialization inputs that @@ -1578,11 +1598,6 @@ export class AutomationServicePlugin implements Plugin { }; } - /** Backoff for degraded-instance retries (#3017): base · 2^(attempts-1), capped. */ - private static declarativeRetryDelayMs(attempts: number): number { - return Math.min(DECLARATIVE_RETRY_BASE_MS * 2 ** Math.max(0, attempts - 1), DECLARATIVE_RETRY_MAX_MS); - } - private clearDeclarativeRetryTimer(): void { if (this.declarativeRetryTimer !== undefined) { clearTimeout(this.declarativeRetryTimer); @@ -1601,9 +1616,7 @@ export class AutomationServicePlugin implements Plugin { this.clearDeclarativeRetryTimer(); if (this.destroyed || this.degradedInstances.size === 0) return; const delay = Math.min( - ...[...this.degradedInstances.values()].map((d) => - AutomationServicePlugin.declarativeRetryDelayMs(d.attempts), - ), + ...[...this.degradedInstances.values()].map((d) => declarativeRetryDelayMs(d.attempts)), ); const timer = setTimeout(() => { this.declarativeRetryTimer = undefined; From 7f3875169dae76e8b15c81cfe3f55c8c97be1e18 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Aug 2026 07:31:32 +0000 Subject: [PATCH 2/3] fix(cloud-connection): stop MarketplaceProxyPlugin renaming itself in the shipped build MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `MarketplaceProxyPlugin.prototype.version` inside the class body made esbuild emit `var MarketplaceProxyPlugin = class _MarketplaceProxyPlugin { … }`, so the built class reported `_MarketplaceProxyPlugin` and the class-name limb of every identity registry naming it matched nothing. The self-reference also read a field that was never there: `version` is an instance field, so `prototype.version` was `undefined` and the outbound User-Agent always announced the `?? '1.0.0'` fallback. Refs #8645 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NaS1PAHJcPfAA2acnV53Tn --- .../src/marketplace-proxy-plugin.ts | 32 +++++++++++++++++-- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/packages/cloud-connection/src/marketplace-proxy-plugin.ts b/packages/cloud-connection/src/marketplace-proxy-plugin.ts index dc23121d61..cbc37826f8 100644 --- a/packages/cloud-connection/src/marketplace-proxy-plugin.ts +++ b/packages/cloud-connection/src/marketplace-proxy-plugin.ts @@ -37,6 +37,32 @@ import type { IHttpServer } from '@objectstack/spec/contracts'; const MARKETPLACE_PREFIX = '/api/v1/marketplace'; +/** + * This plugin's own version, declared at module scope and read from there by + * both the class field and the outbound User-Agent. + * + * ⛔ Do NOT reach back through the class (`MarketplaceProxyPlugin.prototype.version`, + * the spelling this replaced). A class that references itself BY NAME inside its + * own body is rewritten by esbuild into `var X = class _X { … _X … }` so the inner + * reference binds to the class binding rather than the outer `var` — and the + * emitted class then reports `_X` as its `.name`. That silently killed the + * class-name limb of every identity registry naming this plugin (#8645): the CLI's + * `Serve.providesCapability` recognises a host-supplied provider by + * `constructor.name` as well as `plugin.name`, and against the shipped build the + * class-name limb matched nothing at all, leaving the guard running on one limb + * without knowing it. Same trap for any static/prototype self-reference in any + * plugin class — use `this.x` or a module-scope constant like this one. + * + * The self-reference was also reading a field that was never there: + * `version` is an instance field, so `prototype.version` was always `undefined` + * and the User-Agent below always announced the `?? '1.0.0'` fallback, whatever + * this plugin's real version was. + */ +const MARKETPLACE_PROXY_VERSION = '1.1.0'; + +/** The outbound User-Agent for every request this proxy makes to the cloud. */ +const PROXY_USER_AGENT = `objectos-marketplace-proxy/${MARKETPLACE_PROXY_VERSION}`; + /** * In-memory cache for GET/HEAD marketplace responses. * @@ -150,7 +176,7 @@ export interface MarketplaceProxyPluginConfig { export class MarketplaceProxyPlugin implements Plugin { readonly name = 'com.objectstack.runtime.marketplace-proxy'; - readonly version = '1.1.0'; + readonly version = MARKETPLACE_PROXY_VERSION; private readonly cloudUrl: string; private readonly publicBaseUrl: string; @@ -288,7 +314,7 @@ export class MarketplaceProxyPlugin implements Plugin { // don't pay for the body when nothing changed. const revalHeaders: Record = { 'Accept': accept, - 'User-Agent': `objectos-marketplace-proxy/${MarketplaceProxyPlugin.prototype.version ?? '1.0.0'}`, + 'User-Agent': PROXY_USER_AGENT, }; if (acceptLang) revalHeaders['Accept-Language'] = acceptLang; if (hit.etag) revalHeaders['If-None-Match'] = hit.etag; @@ -318,7 +344,7 @@ export class MarketplaceProxyPlugin implements Plugin { // it to the cloud host. Forward only the // identifying headers cloud might log. 'Accept': accept, - 'User-Agent': `objectos-marketplace-proxy/${MarketplaceProxyPlugin.prototype.version ?? '1.0.0'}`, + 'User-Agent': PROXY_USER_AGENT, }; if (acceptLang) reqHeaders['Accept-Language'] = acceptLang; const resp = await fetch(target, { method: 'GET', headers: reqHeaders }); From 8f4e8e7a98ba3a803d391204d2445a593639c0a3 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Aug 2026 07:31:32 +0000 Subject: [PATCH 3/3] test(cli): enforce every declared class-name identity against the runtime Ctor.name Across CAPABILITY_PROVIDERS and the four marketplace identity lists, each declared class-name identity must equal the runtime name of the export it names in the BUILT package, and must satisfy `providesCapability` through the class-name limb alone. The `*_IDENTITIES` statics are re-derived from `Serve`, so a fifth list cannot be added unenumerated. Retires #8357's local "modulo one leading underscore" accommodation rather than leaving a third spelling of the same rule. Fixes #8645 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NaS1PAHJcPfAA2acnV53Tn --- ...capability-class-name-identity-enforced.md | 51 +++++++ .../test/serve-capability-identity.test.ts | 142 ++++++++++++++++++ ...-marketplace-cloud-host-precedence.test.ts | 31 ++-- 3 files changed, 210 insertions(+), 14 deletions(-) create mode 100644 .changeset/capability-class-name-identity-enforced.md diff --git a/.changeset/capability-class-name-identity-enforced.md b/.changeset/capability-class-name-identity-enforced.md new file mode 100644 index 0000000000..10fe7eaddc --- /dev/null +++ b/.changeset/capability-class-name-identity-enforced.md @@ -0,0 +1,51 @@ +--- +"@objectstack/cloud-connection": patch +"@objectstack/service-automation": patch +--- + +fix(cloud-connection,service-automation): stop two plugin classes renaming themselves in the shipped build, and enforce the class-name identity limb against `Ctor.name` (#8645) + +`Serve.providesCapability` (`packages/cli/src/commands/serve.ts`) decides whether a +host already supplied a capability's provider by comparing, by equality, both a +loaded plugin's `name` and its `constructor.name` against a declared identity +list. Every identity registry in that file therefore declares two spellings per +provider — the registered `plugin.name` id and the exported class name — and the +class-name spelling is a claim about the **built** artifact. + +**Measured against the built packages, two of the 27 declared class-name +identities matched nothing at all:** + +``` +MISMATCH CAPABILITY_PROVIDERS.automation declared=AutomationServicePlugin runtime=_AutomationServicePlugin +MISMATCH Serve.MARKETPLACE_PROXY_IDENTITIES declared=MarketplaceProxyPlugin runtime=_MarketplaceProxyPlugin +``` + +Both classes referenced themselves **by name inside their own body** — +`MarketplaceProxyPlugin.prototype.version` building the outbound proxy +User-Agent, and a `private static` backoff helper called from an instance method +in the automation plugin. esbuild rewrites such a class into +`var X = class _X { … _X … }` so the inner reference binds to the class binding +rather than the outer `var`, and the emitted class reports `_X` as its `.name`. + +There was no user-visible impact, because every guard naming these plugins also +declares the registered id, which the instance carries as a plain field no +bundler touches. What was dead is the **redundancy**: a guard running on one +limb it does not know it is running on is one rename away from failing open — +and failing open here means silently mounting a second instance over a host's +own. + +Both source idioms are replaced with module-scope declarations, so the shipped +classes keep their names. The marketplace proxy's self-reference was also +reading a field that was never there (`version` is an instance field, so +`prototype.version` was always `undefined`): its outbound `User-Agent` announced +the `?? '1.0.0'` fallback on every request and now announces the plugin's real +version, `1.1.0`. + +The enforcement half lives in `packages/cli/test/serve-capability-identity.test.ts`: +every declared class-name identity, across `CAPABILITY_PROVIDERS` and the four +marketplace identity lists, is now compared to the runtime `Ctor.name` of the +export it names, and must satisfy `providesCapability` through the class-name +limb alone. The `*_IDENTITIES` statics are re-derived from `Serve` itself, so a +fifth list cannot be added without being enumerated. #8357's local +"modulo one leading underscore" accommodation is retired rather than left as a +third spelling of the same rule. diff --git a/packages/cli/test/serve-capability-identity.test.ts b/packages/cli/test/serve-capability-identity.test.ts index 585d59d0f5..65d6bd423e 100644 --- a/packages/cli/test/serve-capability-identity.test.ts +++ b/packages/cli/test/serve-capability-identity.test.ts @@ -215,6 +215,9 @@ describe('#7652: declared identities match what the provider packages register', const Ctor = mod[exportName]; expect(Ctor, `${pkg} does not export ${exportName}`).toBeTypeOf('function'); expect(identities).toContain(exportName); + // The class-name limb is compared to the RUNTIME class name by the #8645 + // block below — this line only pins the registry's own spelling, which is + // why it could not see `_MarketplaceProxyPlugin`. // Options-taking constructors are given an empty object; every provider // here defaults its options, so this is the real construction path. @@ -226,3 +229,142 @@ describe('#7652: declared identities match what the provider packages register', ).toSatisfy((n: string) => identities.includes(n)); }); }); + +/** + * #8645 — the class-name limb must equal what the class is CALLED in the + * SHIPPED build, and every registry that declares one is enumerated here. + * + * What was measured, and why the drift block above could not see it: + * `Serve.providesCapability` recognises a provider by comparing + * `plugin.constructor.name` against the declared identities, so a class-name + * identity is a genuine SECOND way to recognise a provider only while it equals + * the class's runtime `.name`. Nothing compared those two. The block above + * asserts `identities` CONTAINS `spec.export` — the registry against the + * registry's own spelling — and then asserts the constructed instance's `name` + * is declared, which exercises the OTHER limb. + * + * For `MarketplaceProxyPlugin` the two had already parted company. The source + * referenced the class by name inside its own body, esbuild rewrote it into + * `var MarketplaceProxyPlugin = class _MarketplaceProxyPlugin { … }` so the + * inner reference binds to the class binding, and the built class reported + * `_MarketplaceProxyPlugin` — an identity nothing declares. The guard kept + * working on its registered-`name` limb alone, without knowing that was the + * only limb it had. A guard running on one limb it does not know it is running + * on is one rename away from failing OPEN. + * + * These assertions read the BUILT package through each package's exports map, + * because `dist/` is the artifact the class-name limb is a claim about. A + * source-only check would have stayed green through exactly this defect. + */ + +/** + * The bare identity lists on `Serve` — the ones carrying no `pkg`/`export` + * metadata to derive from — mapped to the export each one is a claim about. + * The coverage test re-derives the `*_IDENTITIES` statics from `Serve` itself, + * so a fifth list cannot be added without landing here. + */ +const IDENTITY_LISTS: Record = { + INSTALL_LOCAL_IDENTITIES: { + list: Serve.INSTALL_LOCAL_IDENTITIES, + pkg: '@objectstack/cloud-connection', + export: 'MarketplaceInstallLocalPlugin', + }, + RUNTIME_CONFIG_IDENTITIES: { + list: Serve.RUNTIME_CONFIG_IDENTITIES, + pkg: '@objectstack/cloud-connection', + export: 'RuntimeConfigPlugin', + }, + MARKETPLACE_PROXY_IDENTITIES: { + list: Serve.MARKETPLACE_PROXY_IDENTITIES, + pkg: '@objectstack/cloud-connection', + export: 'MarketplaceProxyPlugin', + }, + CLOUD_CONNECTION_IDENTITIES: { + // Reached through `createCloudConnectionPlugin`, but a factory is not what + // lands in the kernel — the identity names the class the factory returns. + list: Serve.CLOUD_CONNECTION_IDENTITIES, + pkg: '@objectstack/cloud-connection', + export: 'CloudConnectionPlugin', + }, +}; + +type IdentitySource = { + label: string; + pkg: string; + export: string; + identities: readonly string[]; +}; + +/** Every declared class-name identity in `serve.ts`, from both registry shapes. */ +const IDENTITY_SOURCES: IdentitySource[] = [ + ...Object.entries(Serve.CAPABILITY_PROVIDERS).flatMap(([cap, spec]) => [ + { label: `CAPABILITY_PROVIDERS.${cap}`, pkg: spec.pkg, export: spec.export, identities: spec.identities }, + ...(spec.extras ?? []).map((ex) => ({ + label: `CAPABILITY_PROVIDERS.${cap} → ${ex.export}`, + pkg: ex.pkg, + export: ex.export, + identities: ex.identities, + })), + ]), + ...Object.entries(IDENTITY_LISTS).map(([name, entry]) => ({ + label: `Serve.${name}`, + pkg: entry.pkg, + export: entry.export, + identities: entry.list, + })), +]; + +describe('#8645: every declared class-name identity equals the runtime class name', () => { + it('enumerates every identity registry on Serve — a new list cannot escape', () => { + const onServe = Object.getOwnPropertyNames(Serve).filter((k) => k.endsWith('_IDENTITIES')); + expect( + onServe.sort(), + 'a new `*_IDENTITIES` list on Serve must declare the export it names here, ' + + 'or its class-name limb ships unchecked', + ).toEqual(Object.keys(IDENTITY_LISTS).sort()); + }); + + it.each(IDENTITY_SOURCES)( + '$label — the built $export is really called $export', + async ({ label, pkg, export: exportName, identities }) => { + const mod = (await import(/* @vite-ignore */ pkg)) as Record; + const Ctor = mod[exportName]; + expect(Ctor, `${pkg} does not export ${exportName}`).toBeTypeOf('function'); + + const runtimeName = (Ctor as { name: string }).name; + expect( + runtimeName, + `${label} declares the class-name identity '${exportName}', but the class ` + + `${pkg} ships is called '${runtimeName}' — the limb matches nothing. If a ` + + 'leading underscore appears here, the class references itself by name inside ' + + 'its own body and esbuild renamed it; fix the source idiom (`this.x` / a ' + + 'module-scope constant), do not pin the bundler output.', + ).toBe(exportName); + expect(identities, `${label} must declare the runtime class name`).toContain(runtimeName); + + // …and the limb must actually fire through the resolver, on its own. + // `Object.create` gives an instance of the BUILT class without running a + // constructor, and with no own `name` — so only the class-name limb can + // satisfy this, which is the redundancy the registry claims to have. + const bare = Object.create((Ctor as { prototype: object }).prototype) as unknown; + expect( + Serve.providesCapability([bare], identities), + `${label}: the class-name limb must recognise ${exportName} by itself`, + ).toBe(true); + }, + ); + + it('no registry declares a class-name identity beyond the export it names', () => { + for (const src of IDENTITY_SOURCES) { + // `_`-prefixed spellings are matched deliberately: pinning a bundler's + // rename (`_MarketplaceProxyPlugin`) into the registry would make the + // assertion above pass while restating the defect as a declaration. + const classNames = src.identities.filter((id) => /^_*[A-Z][A-Za-z0-9_]*$/.test(id)); + expect( + classNames, + `${src.label} declares class-name identities other than '${src.export}' — ` + + 'an identity nothing is called can never match', + ).toEqual([src.export]); + } + }); +}); diff --git a/packages/cli/test/serve-marketplace-cloud-host-precedence.test.ts b/packages/cli/test/serve-marketplace-cloud-host-precedence.test.ts index 037751b6e3..4888a8f829 100644 --- a/packages/cli/test/serve-marketplace-cloud-host-precedence.test.ts +++ b/packages/cli/test/serve-marketplace-cloud-host-precedence.test.ts @@ -272,23 +272,26 @@ describe('#8357: the identities the cloud arm matches on are the real ones', () expect(surface.identities()).toContain(real.name); expect(Serve.providesCapability([real], surface.identities())).toBe(true); - // The class-name limb, compared modulo ONE leading underscore. Measured, - // not defensive: esbuild rewrites `export class X { … X.prototype … }` - // — a class that references itself by name inside its own body — into - // `var X = class _X { … _X.prototype … }`, so the BUILT class reports - // `_MarketplaceProxyPlugin`. The registry deliberately keeps the source - // spelling; pinning the bundler's is pinning an artifact. Stripping one - // underscore still catches a genuine rename, which is what this guard is - // for. See the `name`-limb test below for why the guard holds anyway. - expect(surface.identities()).toContain(real.constructor.name.replace(/^_/, '')); + // The class-name limb, compared to the BUILT class name exactly (#8645). + // This used to strip one leading underscore, because + // `MarketplaceProxyPlugin` referenced itself by name inside its own body + // and esbuild emitted `var X = class _X { … }` — so the shipped class was + // called `_MarketplaceProxyPlugin` and this limb matched nothing. The + // source idiom is fixed and the equality is now enforced for every + // registry in `serve.ts` by `serve-capability-identity.test.ts`; the + // accommodation is retired rather than left as a third spelling of one + // rule. If this line ever fails with a `_`-prefixed name, fix the source + // idiom (`this.x` / a module-scope constant) — do not strip it here. + expect(surface.identities()).toContain(real.constructor.name); }); } - it('the registered NAME alone satisfies every guard — the limb that survives bundling', () => { - // Load-bearing given the underscore above: against the shipped build the - // class-name limb is dead for at least one of these four, so the guard has - // to fire on `plugin.name` by itself or it does not fire at all on the - // deployments that matter. + it('the registered NAME alone satisfies every guard — the limb no bundler can touch', () => { + // Still asserted with the class-name limb repaired (#8645), because the two + // limbs are independent claims: `plugin.name` is a plain string field that + // no bundler rewrites, so it is what recognises a host instance reached + // through a factory, a subclass, or a re-export. The class-name limb is the + // redundancy on top — enforced now, not assumed. for (const surface of SURFACES) { expect( Serve.providesCapability([{ name: surface.pluginName }], surface.identities()),