diff --git a/.changeset/28016-held-connector-refusal.md b/.changeset/28016-held-connector-refusal.md new file mode 100644 index 00000000000..86c23e0fcac --- /dev/null +++ b/.changeset/28016-held-connector-refusal.md @@ -0,0 +1,16 @@ +--- +'@objectstack/service-automation': patch +--- + +fix(service-automation): a `connector_action` dispatch to a held connector instance names what recovers it, instead of saying the platform retries it + +Clause-②: no +Metadata-protocol: none + +A degraded connector instance that the platform does not retry (it is **held**) used to answer a dispatch with "Dispatch is unavailable until that is resolved; the platform retries automatically." Nothing retried it. Now the refusal names the fix: + +- **A configuration fault met on a retry** (#27992), or **a credential written into the declaration that the upstream refused**: "Dispatch is unavailable until its declaration is edited (or the server restarts); the platform does not retry it." +- **A stored credential the upstream refused** (#27237): "Dispatch is unavailable until credential '' is set again (or the server restarts); the platform does not retry it." +- **An instance on the backoff** (an unreachable upstream, an environment or vault credential the upstream refused) keeps "the platform retries automatically". + +The engine's degraded-connector record carries the hold: `registerDegradedConnector` takes an optional fifth argument, `RegisteredConnector` gains an optional `degradedHold`, and `AutomationEngine.getConnectorDegradedHold(name)` reads it. The engine's `Connector registered DEGRADED` warning no longer says the materializer retries a held instance. A degraded reason that ends with a full stop no longer gets a second one in the refusal. diff --git a/content/docs/automation/connectors.mdx b/content/docs/automation/connectors.mdx index ae19d1dcbe0..0235c040415 100644 --- a/content/docs/automation/connectors.mdx +++ b/content/docs/automation/connectors.mdx @@ -454,6 +454,7 @@ retry): | Instance `name` collides with a plugin-registered connector | **Hard boot error** — there is no silent precedence between the two worlds | | stdio transport without host opt-in | **Hard boot error** with the `declarativeStdio` fix inline — a security rejection is never retried | | MCP server unreachable / `tools/list` fails; remote spec URL unreachable or transiently failing (408/429/5xx) | **Not fatal**: the instance registers **degraded** (visible on `GET /api/v1/automation/connectors` with the reason), dispatches fail with that reason, and the platform retries on a 5s→5min backoff — recovery is automatic | +| A retry of a degraded instance reaches the upstream and meets a configuration fault (an `include` naming a tool or operation the upstream does not offer) | **Not fatal**: the instance stays **degraded**, now with the configuration reason, and is **not retried** — dispatches fail saying to edit its declaration; an edit of the entry (or a restart) re-attempts it | | Wrong spec URL (other 4xx) or unparseable document | Configuration fault — **hard boot error** | | A bad entry arrives via **runtime reload** (Studio publish / dev reload) | Logged and **skipped**, never crashing the live server; a changed instance's old connector keeps serving until its replacement materializes | diff --git a/packages/services/service-automation/src/builtin/connector-nodes.ts b/packages/services/service-automation/src/builtin/connector-nodes.ts index 8550f1b6a3a..2491245ee72 100644 --- a/packages/services/service-automation/src/builtin/connector-nodes.ts +++ b/packages/services/service-automation/src/builtin/connector-nodes.ts @@ -4,6 +4,7 @@ import type { PluginContext } from '@objectstack/core'; import { defineActionDescriptor } from '@objectstack/spec/automation'; import type { AutomationEngine, ConnectorActionContext } from '../engine.js'; import { refuseNode } from '../guard-refusal.js'; +import { describeHold } from '../plugin-extension-api.js'; import { resolveValueSlotMap } from './value-slot-map.js'; /** @@ -63,17 +64,24 @@ export function registerConnectorNodes(engine: AutomationEngine, ctx: PluginCont const handler = engine.resolveConnectorAction(cfg.connectorId, cfg.actionId); if (!handler) { // A degraded declarative instance (#3017) is registered but has no - // handlers — say WHY dispatch fails and that recovery is automatic, - // instead of the generic wiring hint below. The reason names the - // cause: an unreachable upstream, or (#27237) a credential that - // is not set. + // handlers — say WHY dispatch fails and what recovers it, instead + // of the generic wiring hint below. The reason names the cause: an + // unreachable upstream, or (#27237) a credential that is not set or + // was refused. [#28016] Recovery is automatic only on the backoff: + // a held instance (a configuration fault met on a retry, a refused + // credential the platform will not re-send) names its fix instead. const degraded = engine.getConnectorDegradedReason(cfg.connectorId); if (degraded) { + const hold = engine.getConnectorDegradedHold(cfg.connectorId); + // A reason that is a full sentence keeps one full stop. + const reason = degraded.replace(/\.\s*$/, ''); return { success: false, error: - `connector_action '${node.id}': connector '${cfg.connectorId}' is degraded — ${degraded}. ` - + `Dispatch is unavailable until that is resolved; the platform retries automatically.`, + `connector_action '${node.id}': connector '${cfg.connectorId}' is degraded — ${reason}. ` + + (hold + ? `Dispatch is unavailable until ${describeHold(hold)} (or the server restarts); the platform does not retry it.` + : `Dispatch is unavailable until that is resolved; the platform retries automatically.`), }; } return { diff --git a/packages/services/service-automation/src/connector-credential-door.test.ts b/packages/services/service-automation/src/connector-credential-door.test.ts index f8b0a4f6fad..7c9dd1fc1b6 100644 --- a/packages/services/service-automation/src/connector-credential-door.test.ts +++ b/packages/services/service-automation/src/connector-credential-door.test.ts @@ -722,3 +722,64 @@ describe('[#27237] contract review: an encoded spelling of the value is withheld } }); }); + +/** [#28016] Run a one-node flow dispatching to `billing.request` and return its refusal text. */ +async function dispatchRefusal(engine: AutomationEngine): Promise { + engine.registerFlow('call_billing', { + name: 'call_billing', + label: 'Call billing', + type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'call', type: 'connector_action', label: 'Request', connectorConfig: { connectorId: 'billing', actionId: 'request', input: {} } }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'call' }, + { id: 'e2', source: 'call', target: 'end' }, + ], + } as never); + const result = await engine.execute('call_billing'); + expect(result.success).toBe(false); + return String(result.error); +} + +describe('[#28016] a dispatch to an instance held on a refused stored credential says to set it again', () => { + it('refused at materialization (the mcp shape): set the credential again; nothing retries it', async () => { + const { engine, provider } = await boot([billing()]); + provider.state.rejectAtMaterialization = true; + await engine.setConnectorCredential({ ref: REF, value: DUMMY_VALUE, context: ADMIN }); + + const error = await dispatchRefusal(engine); + expect(error).toContain("connector 'billing' is degraded — "); + expect(error).toContain(`until credential '${REF}' is set again (or the server restarts); the platform does not retry it.`); + expect(error).not.toMatch(/retries automatically/); + expect(error).not.toContain('.. Dispatch'); + expect(error.includes(DUMMY_VALUE)).toBe(false); + }); + + it('refused on a call (the rest / openapi shape): set the credential again; nothing retries it', async () => { + const { engine, provider, call } = await boot([billing()]); + await engine.setConnectorCredential({ ref: REF, value: DUMMY_VALUE, context: ADMIN }); + provider.state.callStatus = 401; + await call(); + + const error = await dispatchRefusal(engine); + expect(error).toContain(`until credential '${REF}' is set again (or the server restarts); the platform does not retry it.`); + expect(error).not.toMatch(/retries automatically/); + expect(error).not.toContain('.. Dispatch'); + }); + + it('control: an environment value refused on a call is on the backoff, and the refusal says the platform retries it', async () => { + process.env[REF] = DUMMY_ENV_VALUE; + envKeys.push(REF); + const { engine, provider, call } = await boot([billing()]); + provider.state.callStatus = 403; + await call(); + + const error = await dispatchRefusal(engine); + expect(error).toContain('The instance is retried with backoff.'); + expect(error).toContain('Dispatch is unavailable until that is resolved; the platform retries automatically.'); + expect(error).not.toMatch(/does not retry/); + }); +}); diff --git a/packages/services/service-automation/src/connector-held-dispatch-refusal.test.ts b/packages/services/service-automation/src/connector-held-dispatch-refusal.test.ts new file mode 100644 index 00000000000..f020887854d --- /dev/null +++ b/packages/services/service-automation/src/connector-held-dispatch-refusal.test.ts @@ -0,0 +1,164 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #28016 — a `connector_action` dispatch to a degraded instance says what +// recovers it. An instance on the #3017 backoff recovers by itself, so its +// refusal says the platform retries it. A HELD instance is not retried: +// a configuration fault met on a backoff retry (#27992), or a credential the +// upstream refused when only an edit of the declaration changes what it sends +// (#27237). Its refusal names the fix and never says it retries. The stored- +// credential hold (set the credential again) is pinned beside its door, in +// connector-credential-door.test.ts. + +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { LiteKernel, ObjectLogger } from '@objectstack/core'; +import type { Connector, ConnectorProviderFactory } from '@objectstack/spec/integration'; +import { + ConnectorCredentialRejectedError, + ConnectorUpstreamUnavailableError, +} from '@objectstack/spec/integration'; +import { AutomationServicePlugin, DECLARATIVE_RETRY_BASE_MS } from './plugin.js'; +import { AutomationEngine } from './engine.js'; + +type Upstream = 'down' | 'refuses-include' | 'refuses-credential'; + +/** An mcp-shaped provider: the marker errors for down / refused credential, a plain Error for a bad `include`. */ +function makeProvider(upstream: Upstream) { + const state = { upstream, calls: 0 }; + const factory: ConnectorProviderFactory = (ctx) => { + state.calls++; + if (state.upstream === 'down') { + throw new ConnectorUpstreamUnavailableError( + `connector-mcp provider: connector '${ctx.name}' could not reach its MCP server: connect ECONNREFUSED`, + ); + } + if (state.upstream === 'refuses-credential') { + throw new ConnectorCredentialRejectedError( + `connector-mcp provider: connector '${ctx.name}' was refused by its upstream (HTTP 401).`, + 401, + ); + } + throw new Error( + `connector-mcp provider: connector '${ctx.name}' providerConfig.include names 'nope', which the MCP server does not list.`, + ); + }; + return { factory, state }; +} + +const ENTRY = { name: 'upstream', label: 'upstream', type: 'api', provider: 'mcp', providerConfig: { url: 'https://upstream.example', include: ['nope'] } }; + +async function boot(factory: ConnectorProviderFactory) { + const kernel = new LiteKernel({ logger: { level: 'silent' } } as never); + kernel.use(new AutomationServicePlugin()); + kernel.use({ + name: 'test.harness', + type: 'standard' as const, + version: '1.0.0', + dependencies: ['com.objectstack.service-automation'], + async init(ctx: any) { + ctx.registerService('objectql', { + registry: { listItems: (type: string) => (type === 'connector' ? [ENTRY] : []) }, + }); + ctx.getService('automation').registerConnectorProvider('mcp', factory); + }, + async start() {}, + } as never); + await kernel.bootstrap(); + return { kernel, engine: kernel.getService('automation') as AutomationEngine }; +} + +/** Run a one-node flow dispatching to `upstream.ping` and return its refusal text. */ +async function dispatchRefusal(engine: AutomationEngine): Promise { + engine.registerFlow('call_upstream', { + name: 'call_upstream', + label: 'Call upstream', + type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'call', type: 'connector_action', label: 'Ping', connectorConfig: { connectorId: 'upstream', actionId: 'ping', input: {} } }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'call' }, + { id: 'e2', source: 'call', target: 'end' }, + ], + } as never); + const result = await engine.execute('call_upstream'); + expect(result.success).toBe(false); + return String(result.error); +} + +describe('#28016 — a dispatch to a held connector instance names its fix, not a retry', () => { + afterEach(() => { + vi.useRealTimers(); + }); + + it('a configuration fault met on the backoff retry: edit the declaration; nothing retries it', async () => { + vi.useFakeTimers(); + const { factory, state } = makeProvider('down'); + const { kernel, engine } = await boot(factory); + state.upstream = 'refuses-include'; + await vi.advanceTimersByTimeAsync(DECLARATIVE_RETRY_BASE_MS); + expect(state.calls).toBe(2); + + const error = await dispatchRefusal(engine); + expect(error).toContain("connector 'upstream' is degraded — "); + expect(error).toContain("include names 'nope'"); + expect(error).toContain('until its declaration is edited (or the server restarts); the platform does not retry it.'); + expect(error).not.toMatch(/retries automatically/); + expect(error).not.toContain('.. Dispatch'); + + await kernel.shutdown(); + }); + + it('a credential written into the declaration, refused by the upstream: edit the declaration; nothing retries it', async () => { + const { factory } = makeProvider('refuses-credential'); + const { kernel, engine } = await boot(factory); + expect(engine.getConnectorDegradedReason('upstream')).toMatch(/refused by its upstream \(HTTP 401\)/); + + const error = await dispatchRefusal(engine); + expect(error).toContain('until its declaration is edited (or the server restarts); the platform does not retry it.'); + expect(error).not.toMatch(/retries automatically/); + expect(error).not.toContain('.. Dispatch'); + + await kernel.shutdown(); + }); + + it('control: an unreachable upstream is on the backoff, and the refusal says the platform retries it', async () => { + vi.useFakeTimers(); + const { factory, state } = makeProvider('down'); + const { kernel, engine } = await boot(factory); + await vi.advanceTimersByTimeAsync(DECLARATIVE_RETRY_BASE_MS); + expect(state.calls).toBe(2); + + const error = await dispatchRefusal(engine); + expect(error).toContain('could not reach its MCP server'); + expect(error).toContain('Dispatch is unavailable until that is resolved; the platform retries automatically.'); + expect(error).not.toMatch(/does not retry/); + + await kernel.shutdown(); + }); +}); + +describe('#28016 — the degraded record carries the hold', () => { + const husk = { name: 'billing', label: 'billing', type: 'api', authentication: { type: 'none' }, actions: [] } as unknown as Connector; + + it('a held degrade stores what re-attempts it and its warn says it is not retried; an unheld one clears it', () => { + const logger = new ObjectLogger({ level: 'silent' }); + const warn = vi.spyOn(logger, 'warn'); + const engine = new AutomationEngine(logger); + const lastWarn = () => String(warn.mock.calls[warn.mock.calls.length - 1]?.[0]); + + engine.registerDegradedConnector(husk, 'refused', 'declarative', undefined, { kind: 'credential', ref: 'BILLING_TOKEN' }); + expect(engine.getConnectorDegradedHold('billing')).toEqual({ kind: 'credential', ref: 'BILLING_TOKEN' }); + expect(lastWarn()).toContain( + "the materializer does not retry it until credential 'BILLING_TOKEN' is set again (or a restart).", + ); + expect(lastWarn()).not.toContain('retries with backoff'); + + // The next degrade of the same name replaces the record: on the backoff, no hold. + engine.registerDegradedConnector(husk, 'could not reach it', 'declarative'); + expect(engine.getConnectorDegradedHold('billing')).toBeUndefined(); + expect(lastWarn()).toContain('the materializer retries with backoff.'); + expect(engine.getConnectorDegradedHold('unregistered')).toBeUndefined(); + }); +}); diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index 788897de5b2..83ea311d762 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -667,8 +667,27 @@ export interface RegisteredConnector { readonly state: ConnectorState; /** Why the connector is degraded — set only when `state` is `degraded`. */ readonly degradedReason?: string; + /** + * [#28016] Set only on a degraded connector the platform does NOT retry: + * what re-attempts it instead. Absent on an instance on the #3017 backoff. + */ + readonly degradedHold?: ConnectorHold; } +/** + * [#28016] What re-attempts a **held** degraded instance — one neither the + * retry timer nor an unchanged reload re-attempts (a restart always does): + * + * - `declaration`: an edit of its declaration — a configuration fault met on + * a retry (#27992), or a credential written into the declaration that the + * upstream refused (#27237); + * - `credential`: setting the stored credential `ref` again — a value stored + * through the credential door that the upstream refused (#27237). + */ +export type ConnectorHold = + | { readonly kind: 'declaration' } + | { readonly kind: 'credential'; readonly ref: string }; + // The connector **provider** contract (ADR-0097) — ConnectorProviderFactory, // ConnectorProviderContext, ConnectorMaterialization — lives in // `@objectstack/spec/integration` so a connector plugin can implement a factory @@ -3982,12 +4001,16 @@ export class AutomationEngine implements IAutomationService { * @param cause the thrown value behind the degrade, for the log record's * structured `meta`. Optional — a caller with no thrown value in hand * still gets `degradedReason` in the record. + * @param hold [#28016] set when the platform will NOT retry the instance: + * what re-attempts it instead, stored as `degradedHold` and named by a + * `connector_action` refusal. Omitted, the instance is on the backoff. */ registerDegradedConnector( def: Connector, reason: string, origin: ConnectorOrigin = 'declarative', cause?: unknown, + hold?: ConnectorHold, ): void { // The body lives in ./plugin-extension-api.ts (#25222). const host: ThisParameterType = { @@ -3995,7 +4018,7 @@ export class AutomationEngine implements IAutomationService { connectors: this.connectors, logger: this.logger, }; - registerDegradedConnector.call(host, def, reason, origin, cause); + registerDegradedConnector.call(host, def, reason, origin, cause, hold); } /** Enforce the ADR-0097 §4 two-sources-of-truth rule; warn on same-origin replace. */ @@ -4142,6 +4165,17 @@ export class AutomationEngine implements IAutomationService { return reg?.state === 'degraded' ? (reg.degradedReason ?? 'upstream unavailable') : undefined; } + /** + * [#28016] What re-attempts a held degraded connector, or `undefined` when + * it is not held — `ready`, unregistered, or degraded on the #3017 backoff. + * Lets the `connector_action` node say "retries automatically" only when + * that is true. + */ + getConnectorDegradedHold(name: string): ConnectorHold | undefined { + const reg = this.connectors.get(name); + return reg?.state === 'degraded' ? reg.degradedHold : undefined; + } + /** * Register a connector **provider factory** (ADR-0097 §2). A connector * plugin (e.g. `@objectstack/connector-openapi`) calls this at `init()` under diff --git a/packages/services/service-automation/src/plugin-extension-api.ts b/packages/services/service-automation/src/plugin-extension-api.ts index 4f9bba57d16..bebdfc2a8ec 100644 --- a/packages/services/service-automation/src/plugin-extension-api.ts +++ b/packages/services/service-automation/src/plugin-extension-api.ts @@ -33,7 +33,7 @@ import { FLOW_RESERVED_NODE_TYPES, defineActionDescriptor, resolveFlowTriggerKin import type { Connector, ConnectorOrigin } from '@objectstack/spec/integration'; import { ConnectorSchema } from '@objectstack/spec/integration'; import { describeThrownForLog } from './thrown-cause-diagnostics.js'; -import type { AutomationEngine, ConnectorActionHandler, FlowTrigger, FlowTriggerBinding, NodeExecutor } from './engine.js'; +import type { AutomationEngine, ConnectorActionHandler, ConnectorHold, FlowTrigger, FlowTriggerBinding, NodeExecutor } from './engine.js'; /** * The `AutomationEngine` members the moved bodies read, each typed as the class @@ -366,22 +366,42 @@ export function registerDegradedConnector( reason: string, origin: ConnectorOrigin, cause?: unknown, + hold?: ConnectorHold, ): void { const parsed = ConnectorSchema.parse(def); this.assertSameOriginOrFree(parsed.name, origin); - this.connectors.set(parsed.name, { def: parsed, handlers: {}, origin, state: 'degraded', degradedReason: reason }); + this.connectors.set(parsed.name, { + def: parsed, + handlers: {}, + origin, + state: 'degraded', + degradedReason: reason, + ...(hold ? { degradedHold: hold } : {}), + }); // `warn(message, meta?)` per the `Logger` contract — no `Error` slot // below `error`, so the cause belongs in argument TWO here. this.logger.warn( - `Connector registered DEGRADED: ${parsed.name} (origin: ${origin}) — no actions and no handlers ` + - `until its upstream is reachable; a connector_action dispatching to it fails with the stored ` + - `reason, and the materializer retries with backoff.`, + `Connector registered DEGRADED: ${parsed.name} (origin: ${origin}) — no actions and no handlers` + + (hold + ? `; a connector_action dispatching to it fails with the stored reason, and the materializer ` + + `does not retry it until ${describeHold(hold)} (or a restart).` + : ` until its upstream is reachable; a connector_action dispatching to it fails with the stored ` + + `reason, and the materializer retries with backoff.`), cause === undefined ? { degradedReason: reason } : { degradedReason: reason, ...describeThrownForLog(cause) }, ); } +/** + * [#28016] What re-attempts a held connector, as the clause a sentence ends + * on: "… until (or a restart)". The one spelling for the engine's + * degrade record and the `connector_action` refusal. + */ +export function describeHold(hold: ConnectorHold): string { + return hold.kind === 'credential' ? `credential '${hold.ref}' is set again` : 'its declaration is edited'; +} + /** The body of {@link AutomationEngine.assertSameOriginOrFree}, moved unedited; that method's docblock is its contract. */ export function assertSameOriginOrFree(this: Pick, name: string, origin: ConnectorOrigin): void { const existing = this.connectors.get(name); diff --git a/packages/services/service-automation/src/plugin.ts b/packages/services/service-automation/src/plugin.ts index 95e8c37606d..6a0a751a146 100644 --- a/packages/services/service-automation/src/plugin.ts +++ b/packages/services/service-automation/src/plugin.ts @@ -2562,7 +2562,14 @@ export class AutomationServicePlugin implements Plugin { this.degradedInstances.set(info.name, { attempts: 1, signature: info.signature, reason: info.reason, held: true }); if (!info.hasLive) { try { - engine.registerDegradedConnector(this.buildDegradedHuskDef(info.name, info.entry), info.reason, 'declarative', info.cause); + // [#28016] Held: the husk says so, and a dispatch names the edit. + engine.registerDegradedConnector( + this.buildDegradedHuskDef(info.name, info.entry), + info.reason, + 'declarative', + info.cause, + { kind: 'declaration' }, + ); } catch (err) { ctx.logger.warn( `[Automation] could not register degraded husk for '${info.name}' — the instance stays absent from the ` + @@ -3090,7 +3097,16 @@ export class AutomationServicePlugin implements Plugin { } if (!hasLive) { try { - engine.registerDegradedConnector(this.buildDegradedHuskDef(info.name, info.entry), info.reason, 'declarative', info.cause); + // [#28016] A held husk names what re-attempts it: setting the + // stored value again, or (no `credentialRef`) an edit of the + // declaration that carries the credential. + engine.registerDegradedConnector( + this.buildDegradedHuskDef(info.name, info.entry), + info.reason, + 'declarative', + info.cause, + held ? (stored ? { kind: 'credential', ref: stored.ref } : { kind: 'declaration' }) : undefined, + ); } catch (err) { ctx.logger.warn( `[Automation] could not register degraded husk for '${info.name}' — the instance stays absent from the ` +