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

.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.

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<string, { list: readonly string[]; pkg: string; export: string }> = {
+ 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<string, unknown>;
+ 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]);
+ }
+ });
+});

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()),

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<string, string> = {
  '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 });

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;