From f76068ce17edcef9e3eb8bc741d3985c73b587f6 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 15:00:15 +0000 Subject: [PATCH] fix(service-automation): a stored connector's configuration fault is skipped at boot, never fatal Claude-Session: https://claude.ai/code/session_01FEkQ5oQNecNsthbZA1bZTp Co-authored-by: Claude --- ...698-stored-connector-fault-soft-at-boot.md | 15 ++ ...nector-package-ref-restart.dogfood.test.ts | 26 +++- .../src/connector-materialization.test.ts | 146 +++++++++++++++++- .../services/service-automation/src/plugin.ts | 89 ++++++----- 4 files changed, 220 insertions(+), 56 deletions(-) create mode 100644 .changeset/28698-stored-connector-fault-soft-at-boot.md diff --git a/.changeset/28698-stored-connector-fault-soft-at-boot.md b/.changeset/28698-stored-connector-fault-soft-at-boot.md new file mode 100644 index 00000000000..c8323442385 --- /dev/null +++ b/.changeset/28698-stored-connector-fault-soft-at-boot.md @@ -0,0 +1,15 @@ +--- +"@objectstack/service-automation": patch +--- + +fix(service-automation): a stored connector with a configuration fault never stops the platform starting + +A provider-bound connector saved in the environment (through the metadata door or Studio, environment-wide or in an `authored` package) that later stops being valid, for example because the plugin offering its provider was removed, stopped the next start of the platform. The same fault met while the platform runs only skipped that connector. + +Clause-②: no — no accepted value and no public surface changes: the same stored definitions are refused as before; a start now carries on past them instead of stopping. + +Metadata-protocol: none + +- **Before.** At start, any configuration fault of a stored connector (a provider no installed plugin offers, a host credential resolver that throws, a name a plugin connector already holds, a configuration its provider refuses) stopped the start. Only a refused package-relative ref was skipped. +- **Now.** Every configuration fault of a stored connector takes the path it takes while the platform runs: it is logged with the same record, that connector is not served, and the host's own connectors beside it serve. A connector the host's own stack declares, and an installed package's, still stop the start on a configuration fault, as before. +- **Not covered here.** A stored connector that takes a name a plugin connector registers later in the start is a separate card. diff --git a/packages/qa/dogfood/test/stored-connector-package-ref-restart.dogfood.test.ts b/packages/qa/dogfood/test/stored-connector-package-ref-restart.dogfood.test.ts index 440e1bc3f4d..ea5af34fa65 100644 --- a/packages/qa/dogfood/test/stored-connector-package-ref-restart.dogfood.test.ts +++ b/packages/qa/dogfood/test/stored-connector-package-ref-restart.dogfood.test.ts @@ -1,7 +1,8 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. // // GOLDEN REGRESSION — a stored connector entry whose package-relative ref is -// refused (#28594) never stops the platform from booting. +// refused (#28594), or that carries any other configuration fault (#28698), +// never stops the platform from booting. // // ## Why a restart // @@ -20,11 +21,14 @@ // metadata door, an environment-wide `openapi` entry naming a // package-relative file, the same entry in an `authored` package, an // environment-wide stdio `mcp` entry, and a control carrying its document -// inline. The second boot: +// inline. [#28698] It also saves an environment-wide entry naming a provider +// no installed plugin offers (accepted at save, skipped after boot: the shape +// a row takes when the plugin that offered its provider is removed). The +// second boot: // // 1. starts (no throw); -// 2. does not list either refused entry, and its boot record names each -// entry and its ref; +// 2. does not list any refused entry, and its boot record names each entry +// and its ref (or, for the unknown provider, the provider); // 3. lists the stored inline control `ready` with its operation (the rows // were hydrated, so the refusal above was read at boot, not skipped); // 4. lists the showcase's own file-ref and stdio entries `ready` (control). @@ -47,6 +51,8 @@ const AUTHORED_FILE = 'srr_authored_file'; /** An `authored` package created through the package door. */ const AUTHORED_PACKAGE_ID = 'com.dogfood.storedrestart'; const INLINE = 'srr_inline'; +/** [#28698] A stored entry whose provider no installed plugin offers. */ +const UNKNOWN_PROVIDER = 'srr_unknown_provider'; const HOST_FILE_ENTRY = 'showcase_status_openapi'; const HOST_STDIO_ENTRY = 'showcase_mcp_tools'; @@ -144,6 +150,10 @@ describe('dogfood: a stored connector entry refused its package-relative ref doe saves[AUTHORED_FILE] = await json(await first.apiAs( token, 'PUT', `/meta/connector/${AUTHORED_FILE}?package=${AUTHORED_PACKAGE_ID}`, openapi(AUTHORED_FILE, FILE_REF), )); + saves[UNKNOWN_PROVIDER] = await json(await first.apiAs(token, 'PUT', `/meta/connector/${UNKNOWN_PROVIDER}`, { + ...openapi(UNKNOWN_PROVIDER, DOCUMENT), + provider: 'no_such_provider', + })); saves[INLINE] = await json(await first.apiAs(token, 'PUT', `/meta/connector/${INLINE}`, openapi(INLINE, DOCUMENT))); before = await settle(first, token, [INLINE]); } finally { @@ -175,6 +185,7 @@ describe('dogfood: a stored connector entry refused its package-relative ref doe } expect(entryOf(before, INLINE)?.state, shown(before, INLINE)).toBe('ready'); expect(entryOf(before, ENV_FILE), shown(before, ENV_FILE)).toBeUndefined(); + expect(entryOf(before, UNKNOWN_PROVIDER), shown(before, UNKNOWN_PROVIDER)).toBeUndefined(); }); it('the second boot over the same database starts', () => { @@ -192,6 +203,13 @@ describe('dogfood: a stored connector entry refused its package-relative ref doe expect(record).toContain(ref); }); + it('[#28698] a stored entry naming a provider no plugin offers is not listed after the restart, and the boot record names it', () => { + expect(entryOf(after, UNKNOWN_PROVIDER), shown(after, UNKNOWN_PROVIDER)).toBeUndefined(); + const record = bootLines.find((l) => l.includes(`connector instance '${UNKNOWN_PROVIDER}' declares provider 'no_such_provider'`)); + expect(record, `no boot record names '${UNKNOWN_PROVIDER}'`).toBeDefined(); + expect(record).toContain('no provider factory is registered'); + }); + it('controls: the stored inline entry, and the showcase\'s own file-ref and stdio entries, list ready after the restart', () => { expect(entryOf(after, INLINE)?.state, shown(after, INLINE)).toBe('ready'); expect(entryOf(after, INLINE).actions.map((a: any) => a.key)).toEqual(['GetThings']); diff --git a/packages/services/service-automation/src/connector-materialization.test.ts b/packages/services/service-automation/src/connector-materialization.test.ts index 5b5fdb8e3f1..39cd8789e52 100644 --- a/packages/services/service-automation/src/connector-materialization.test.ts +++ b/packages/services/service-automation/src/connector-materialization.test.ts @@ -1037,16 +1037,15 @@ describe('#28594 — a definition stored in this environment is refused its pack await kernel.shutdown(); }); - it('control: a configuration fault other than a refused ref (an unknown provider) still fails the boot', async () => { - // The soft path is the stored entry's refused ref alone; a provider the - // runtime does not offer keeps the fail-loud contract. + it('control: the host\'s own entry with a configuration fault (an unknown provider) still fails the boot', async () => { + // The soft path is a stored entry's alone (#28698): the host's + // registered declaration, faulty as filed, keeps the fail-loud contract. const { factory } = makeRefProvider(); + const faulty = { ...hostEntry, provider: 'no_such_provider' }; const err = await rejectionOf( - boot([{ ...hostEntry, provider: 'no_such_provider' }], { - providerFactory: factory, packageRoot: fixtureRoot, packages, registered, - }), + boot([{ ...faulty }], { providerFactory: factory, packageRoot: fixtureRoot, packages, registered: [faulty] }), ); - expect(err.message).toContain('host_pkg'); + expect(err.message).toContain(`connector instance 'host_pkg' declares provider 'no_such_provider'`); }); it('control: a stored entry carrying its document inline materializes — neither member is reached', async () => { @@ -1085,6 +1084,139 @@ describe('#28594 — a definition stored in this environment is refused its pack }); }); +// ── #28698 — every configuration fault of a STORED entry is soft at boot ──── +// +// #28594 made a stored entry's refused ref soft at boot. Every other +// configuration fault the boot reconcile meets on a stored entry takes the same +// path (#28698): logged with the record a reload writes, the entry skipped, +// the host's entries beside it served. The host's own entry with the same fault +// keeps failing the boot. + +describe('#28698 — a stored entry\'s configuration fault never fails the boot; the host\'s still does', () => { + const HOST = 'com.acme.host'; + const packages = { [HOST]: { source: 'code' } }; + const ownedBy = (item: T, packageId: string) => ({ ...item, _packageId: packageId }); + const bearer = { type: 'bearer', credentialRef: 'OS_TEST_28698_VAULT_REF' } as ConnectorInstanceAuth; + + /** A factory that materializes every entry, except one whose providerConfig asks it to refuse. */ + function makeProvider() { + const calls: string[] = []; + const factory: ConnectorProviderFactory = (ctx) => { + calls.push(ctx.name); + if (ctx.providerConfig.refuse === true) throw new Error(`providerConfig of '${ctx.name}' is not valid: refuse is set`); + return { + def: { name: ctx.name, label: ctx.label, type: 'api', authentication: { type: 'none' }, actions: [{ key: 'ping', label: 'Ping' }] } as unknown as Connector, + handlers: { ping: async () => ({ ok: true }) }, + }; + }; + return { factory, calls }; + } + const throwingResolver: CredentialResolver = () => { throw new Error('the vault could not be reached'); }; + + /** The host's entry served beside every stored fault: registered, so it is the host's. */ + const hostServed = ownedBy(providerConnector('host_served'), HOST); + + /** + * One fault, spelled for an entry `name`: what the entry carries, the boot + * options it needs, and the record text that names it. + */ + const FAULTS: Array<{ + fault: string; + entry: (name: string) => Record; + opts?: Partial; + livePlugin?: boolean; + record: (name: string) => string; + }> = [ + { + fault: 'a provider no plugin offers', + entry: (name) => providerConnector(name, { provider: 'no_such_provider' }), + record: (name) => `connector instance '${name}' declares provider 'no_such_provider', but no provider factory is registered`, + }, + { + fault: 'an invalid retryConfig', + entry: (name) => ({ ...providerConnector(name), retryConfig: { maxAttempts: -3 } }), + record: (name) => `connector instance '${name}' (provider 'fake'): retryConfig is not a valid connector retry policy`, + }, + { + fault: 'a credential resolver that throws', + entry: (name) => providerConnector(name, { auth: bearer }), + opts: { credentialResolver: throwingResolver }, + record: (name) => `connector instance '${name}' (provider 'fake'): auth resolution failed`, + }, + { + fault: 'a name a plugin connector holds', + entry: (name) => providerConnector(name), + livePlugin: true, + record: (name) => `declarative provider-bound instance '${name}' collides with a plugin-registered connector`, + }, + { + fault: 'a providerConfig its provider refuses', + entry: (name) => providerConnector(name, { providerConfig: { refuse: true } }), + record: (name) => `failed to materialize connector instance '${name}' via provider 'fake'`, + }, + ]; + + it.each(FAULTS.map((f) => [f.fault, f] as const))('a stored entry with %s: the boot starts, the entry is skipped with its record, the host\'s entry serves', async (_fault, f) => { + const { factory } = makeProvider(); + const logged: Array<{ msg: string; meta?: unknown }> = []; + const name = 'stored_faulty'; + const kernel = await boot([{ ...hostServed }, f.entry(name)], { + providerFactory: factory, packages, registered: [hostServed], logged, + ...(f.livePlugin ? { registerLivePlugin: [name] } : {}), + ...f.opts, + }); + const engine = automationOf(kernel); + expect(engine.getRegisteredConnectors()).toContain('host_served'); + // Skipped: nothing declarative serves the name (a plugin connector may). + expect(engine.getConnectorOrigin(name)).not.toBe('declarative'); + const record = logged.find((l) => l.msg.includes(f.record(name))); + expect(record, `no record names '${name}': ${JSON.stringify(logged.map((l) => l.msg))}`).toBeDefined(); + await kernel.shutdown(); + }); + + it.each(FAULTS.map((f) => [f.fault, f] as const))('control: the host\'s own entry with %s still fails the boot', async (_fault, f) => { + const { factory } = makeProvider(); + const name = 'host_faulty'; + const faulty = ownedBy(f.entry(name), HOST); + const err = await rejectionOf(boot([{ ...faulty }], { + providerFactory: factory, packages, registered: [faulty], + ...(f.livePlugin ? { registerLivePlugin: [name] } : {}), + ...f.opts, + })); + expect(err.message).toContain(f.record(name)); + }); + + it('a stored entry duplicating the host\'s name is skipped with its record; the host\'s entry serves', async () => { + const { factory, calls } = makeProvider(); + const logged: Array<{ msg: string; meta?: unknown }> = []; + const kernel = await boot( + [{ ...hostServed }, providerConnector('host_served', { providerConfig: { other: true } })], + { providerFactory: factory, packages, registered: [hostServed], logged }, + ); + expect(automationOf(kernel).getRegisteredConnectors()).toContain('host_served'); + expect(calls).toEqual(['host_served']); + expect(logged.some((l) => l.msg.includes(`duplicate declarative connector instance name 'host_served'`))).toBe(true); + await kernel.shutdown(); + }); + + it('control: two host entries of one name still fail the boot', async () => { + const { factory } = makeProvider(); + const err = await rejectionOf(boot([{ ...hostServed }, { ...hostServed }], { + providerFactory: factory, packages, registered: [hostServed], + })); + expect(err.message).toContain(`duplicate declarative connector instance name 'host_served'`); + }); + + it('control: an INSTALLED package\'s entry with a configuration fault still fails the boot (#28534 unchanged)', async () => { + const { factory } = makeProvider(); + const installed = ownedBy(providerConnector('installed_faulty', { provider: 'no_such_provider' }), 'com.acme.installed'); + const err = await rejectionOf(boot([installed], { + providerFactory: factory, packages: { ...packages, 'com.acme.installed': { source: 'marketplace' } }, registered: [installed], + })); + expect(err.message).toContain(`connector instance 'installed_faulty' declares provider 'no_such_provider'`); + }); +}); + // ── #3017 — upstream unavailable: degrade + retry, never a dead boot ───────── // // A provider factory that throws the CONNECTOR_UPSTREAM_UNAVAILABLE marker diff --git a/packages/services/service-automation/src/plugin.ts b/packages/services/service-automation/src/plugin.ts index 5b135890c1f..7e700bd2a0a 100644 --- a/packages/services/service-automation/src/plugin.ts +++ b/packages/services/service-automation/src/plugin.ts @@ -2656,7 +2656,9 @@ export class AutomationServicePlugin implements Plugin { * provider, invalid `providerConfig`, a host credential resolver that * throws, name conflict, duplicate — **throws**, which is fatal to * bootstrap under both LiteKernel and ObjectKernel (the ADR's "fail - * loudly" contract). + * loudly" contract). [#28698] Except a STORED entry's (a row of the + * environment, not the host's declaration): that one is logged and + * skipped exactly as on a reload, so a row at rest never stops a boot. * - **Reload** (`fatal: false`, called from `metadata:reloaded`): the same * problems are **logged and the offending entry is skipped**, so a bad * Studio publish / dev reload never crashes a live server. A changed @@ -2755,6 +2757,12 @@ export class AutomationServicePlugin implements Plugin { if (!declared) return; // no registry, no reloaded artifact — nothing declared this.reconciledDeclaration = declaredInstanceFingerprint(declared); + // [#28534] The engine's package rows and residual-owner record, read + // once per run (each lookup is live), answer which declaring packages + // are not the host's own ({@link nonHostDeclaringPackageOf}). + const declaringReaders = this.declaringPackageReadersOrNone(ctx); + const storedEntry = (entry: unknown): boolean => nonHostDeclaringPackageOf(entry, declaringReaders)?.why === 'stored'; + // Report a reconcile problem: fatal (boot) throws; soft (reload) logs. // // #5575 — `msg` MUST be a single-line, self-contained sentence, and an @@ -2775,8 +2783,19 @@ export class AutomationServicePlugin implements Plugin { // A `fail(msg)` with no cause logs exactly what it always did: `cause` // is only described when it is present, so the causeless reports (dup // name, §4 conflict, unknown provider) gain no `"error":"undefined"`. - const fail = (msg: string, cause?: unknown): void => { - if (opts.fatal) { + // + // [#28594, #28698] `stored`: the fault is a STORED entry's (a row of + // the environment, {@link nonHostDeclaringPackageOf}), not one the + // host's stack declares. A row at rest is hydrated into the registry + // before this boot reconcile reads it, so failing the boot on it would + // let one stored definition (accepted when it was saved, and since + // made invalid: its provider no longer offered, a policy a newer + // runtime refuses) stop the platform starting, with no door left to + // fix it through. So a stored entry's fault takes the soft path in + // BOTH modes: the same record, the entry skipped, the boot carrying + // on. A host-declared entry's keeps the fail-loud contract at boot. + const fail = (msg: string, cause?: unknown, stored = false): void => { + if (opts.fatal && !stored) { throw new Error(cause === undefined ? msg : `${msg} cause: ${thrownMessageText(cause)}`); } // Third argument, per the `Logger` contract's `error(message, error?, @@ -2796,7 +2815,11 @@ export class AutomationServicePlugin implements Plugin { const bound = entry as DeclaredConnectorItem & { name: string; provider: string }; if (bound.enabled === false) continue; if (desired.has(bound.name)) { - fail(`[Automation] duplicate declarative connector instance name '${bound.name}' — connector names must be unique (ADR-0097).`); + fail( + `[Automation] duplicate declarative connector instance name '${bound.name}' — connector names must be unique (ADR-0097).`, + undefined, + storedEntry(bound), + ); continue; } desired.set(bound.name, { entry: bound, signature: connectorInstanceSignature(bound) }); @@ -2905,11 +2928,6 @@ export class AutomationServicePlugin implements Plugin { if (!desired.has(name)) this.credentialRefusalAttempts.delete(name); } - // [#28534] The engine's package rows and residual-owner record, read - // once per run (each lookup is live), answer which declaring packages - // are not the host's own ({@link nonHostDeclaringPackageOf}). - const declaringReaders = this.declaringPackageReadersOrNone(ctx); - // 2. Add new instances and re-materialize changed ones (signature diff). let changed = 0; for (const [name, { entry, signature }] of desired) { @@ -2933,6 +2951,8 @@ export class AutomationServicePlugin implements Plugin { if (held?.held && held.signature === signature) continue; const provider = entry.provider; + const notHost = nonHostDeclaringPackageOf(entry, declaringReaders); + const stored = notHost?.why === 'stored'; // §4 conflict: the name is held by a plugin-registered connector (not // one of ours). Never silently replace across origins. @@ -2940,6 +2960,8 @@ export class AutomationServicePlugin implements Plugin { fail( `[Automation] connector name conflict: declarative provider-bound instance '${name}' collides with a ` + `plugin-registered connector of the same name — there is no silent precedence (ADR-0097 §4). Rename one.`, + undefined, + stored, ); continue; } @@ -2952,6 +2974,8 @@ export class AutomationServicePlugin implements Plugin { `Install the connector plugin that supplies it (openapi → @objectstack/connector-openapi, mcp → @objectstack/connector-mcp, ` + `rest → @objectstack/connector-rest) in the stack's plugins: array. Installed providers: ` + `[${installed.join(', ') || 'none'}] (ADR-0097).`, + undefined, + stored, ); continue; } @@ -2999,11 +3023,12 @@ export class AutomationServicePlugin implements Plugin { // (`AutomationServicePluginOptions.credentialResolver` — a // secrets service, or anything that validates with Zod), so // what lands here is not ours to assume anything about, and its - // contract is unchanged: fatal at boot, logged and skipped on - // a reload. + // contract is unchanged: fatal at boot for the host's own + // entry, logged and skipped on a reload or for a stored one. fail( `[Automation] connector instance '${name}' (provider '${provider}'): auth resolution failed (ADR-0097 §3).`, err, + stored, ); continue; } @@ -3029,6 +3054,7 @@ export class AutomationServicePlugin implements Plugin { fail( `[Automation] connector instance '${name}' (provider '${provider}'): auth resolution failed (ADR-0097 §3).`, err, + stored, ); continue; } @@ -3064,6 +3090,7 @@ export class AutomationServicePlugin implements Plugin { `[Automation] connector instance '${name}' (provider '${provider}'): a key it names cannot sign or ` + `verify (ADR-0143 D2.3).`, err, + stored, ); continue; } @@ -3140,26 +3167,13 @@ export class AutomationServicePlugin implements Plugin { `[Automation] connector instance '${name}' (provider '${provider}'): retryConfig is not a valid ` + `connector retry policy (integration/connector.zod.ts RetryConfigSchema).`, parsed.error, + stored, ); continue; } retryConfig = parsed.data; } - const notHost = nonHostDeclaringPackageOf(entry, declaringReaders); - // [#28594] Set when this entry's factory met the refusal of a STORED - // entry's package ref, so the catch below can tell that fault apart - // from one the factory raised itself (a provider wraps the refusal - // in its own message, so the thrown value cannot say it). - let storedRefRefused = false; - const refuseRef = (member: 'reads' | 'paths'): ((relativePath: string) => Promise) => { - const refusal = createInstalledPackageRefRefusal(name, notHost!, member); - if (notHost?.why !== 'stored') return refusal; - return async (relativePath: string) => { - storedRefRefused = true; - return refusal(relativePath); - }; - }; const providerCtx: ConnectorProviderContext = { name, label: entry.label ?? name, @@ -3187,12 +3201,12 @@ export class AutomationServicePlugin implements Plugin { // [#28594] So is a definition stored in this environment. loadPackageFile: notHost === undefined ? createPackageFileLoader(this.options.packageRoot) - : refuseRef('reads'), + : createInstalledPackageRefRefusal(name, notHost, 'reads'), // #22434 — the same root as a location (e.g. a launched // process's working directory), under the same confinement. resolvePackagePath: notHost === undefined ? createPackagePathResolver(this.options.packageRoot) - : refuseRef('paths'), + : createInstalledPackageRefRefusal(name, notHost, 'paths'), }; let materialization; @@ -3300,23 +3314,6 @@ export class AutomationServicePlugin implements Plugin { // [#27992] A configuration fault met by an instance that is // degraded — its backoff retry, or a reload — ends the retry: // ADR-0097 does not retry a configuration fault. - // [#28594] A stored entry refused its package ref is the - // environment's row, not the host's declaration, and a row at - // rest is hydrated into the registry before this boot reconcile - // reads it. Failing the boot on it would let one stored - // definition (one that worked before this refusal existed) stop - // the platform starting, with no door left to fix it through. - // So it takes the soft path in BOTH modes: logged, the entry - // skipped, the boot carrying on. Every other configuration fault - // keeps the fail-loud contract. - if (storedRefRefused && opts.fatal) { - ctx.logger.error( - `[Automation] failed to materialize connector instance '${name}' via provider '${provider}' (ADR-0097).`, - undefined, - describeThrownForLog(thrown), - ); - continue; - } if (!opts.fatal && this.degradedInstances.has(name)) { this.failDegradedConnectorInstance(engine, ctx, { name, @@ -3333,10 +3330,12 @@ export class AutomationServicePlugin implements Plugin { // people to write, and the first one that validates its // `providerConfig` with Zod throws a message whose first line is // `[`. The cause goes to `meta` on the soft path, and is appended - // to the thrown message on the fatal one. + // to the thrown message on the fatal one. [#28594] A stored + // entry's (its refused package ref among them) is soft at boot. fail( `[Automation] failed to materialize connector instance '${name}' via provider '${provider}' (ADR-0097).`, thrown, + stored, ); continue; }