From a94e321d15f78a3476a548c5dc5b5f8e9f2c63c2 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 20:37:59 +0000 Subject: [PATCH] fix(service-automation): reconcile declarative connectors against a set the metadata reload actually refreshes (#7742) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A connector edit followed by a metadata reload changed nothing: no reconcile, no teardown, no re-materialize — the pre-edit connector kept serving until the process restarted. `os dev` masks it (the serve child restarts on recompile), so the trigger that walks into it is a Studio package publish into a running server. Confirmed on origin/main before the fix: an edited, an added and a deleted connector are all no-ops on the reload path. The reconcile was fine; its INPUT was a boot snapshot. `reconcileDeclaredConnectors` read `ql.registry.listItems('connector')`, and no reload path re-ingests connector items into that registry — ObjectQL's own `metadata:reloaded` handler re-ingests the payload's OBJECT definitions and stops there. So the reconcile compared the boot world against itself and found nothing to do. Every existing test drove the reload through a hand-mutated fake registry, which is why the path looked covered. `readDeclaredConnectorItems` now folds the sources a reload does refresh over that registry read, one per trigger: * the artifact carried on the `metadata:reloaded` payload — the dev/HMR trigger, and the only place an edited or deleted definition exists. Held as plugin state, so a degraded-instance retry firing minutes later does not fall back to the boot snapshot and rebuild the pre-edit instance. The fold is a replacement scoped to the packages the artifact speaks for (its manifest id + the `_packageId` stamped on its items), not a union: a union can never observe a deletion, while an unscoped replacement would tear down a connector another package contributed. * `protocol.getMetaItems({ type: 'connector' })` — the flattened `/meta` view the flow re-sync already reads, which layers the `sys_metadata` rows a publish promotes to active over the registry (overlay wins). Post-boot reconciles only: at boot the registry was just built and is current by construction, and that read costs a `sys_metadata` query and can fail — neither belongs on the fail-loudly boot path. Both reads fail safe. An absent, failing, or empty-while-the-registry-is-not answer is "no answer" and never tears down a live connector, and an announcement with no connector collection at all (a publish's bare `{ changed }`, or an artifact with no `connectors:` key) leaves every instance alone — only an artifact carrying an EMPTY array is the honest "none left". An unchanged entry still hashes to the same signature, so reloads do not churn live connections. The descriptor audit beside the reconcile reads the same declaration, so its warning describes the stack as it is now. `readFlowDefsFromProtocol`'s body is now the shared `readMetaItemsFromProtocol` — same normalization, same `null`-means-no-answer contract, byte-identical log message for the flow type. New tests use a REAL `SchemaRegistry` and deliberately never mutate it across the reload, which is the fact the old harness hid. Reverse-verified: with the fix reverted, the edit / add / delete / publish cases fail exactly the way the QA run observed, while the three never-tear-down guards pass on both sides. --- .changeset/connector-reload-reingest.md | 39 +++ .../src/connector-reload-reingest.test.ts | 307 ++++++++++++++++++ .../services/service-automation/src/plugin.ts | 254 +++++++++++++-- 3 files changed, 570 insertions(+), 30 deletions(-) create mode 100644 .changeset/connector-reload-reingest.md create mode 100644 packages/services/service-automation/src/connector-reload-reingest.test.ts diff --git a/.changeset/connector-reload-reingest.md b/.changeset/connector-reload-reingest.md new file mode 100644 index 0000000000..ce4a17a940 --- /dev/null +++ b/.changeset/connector-reload-reingest.md @@ -0,0 +1,39 @@ +--- +"@objectstack/service-automation": patch +--- + +fix(service-automation): a metadata reload now reconciles declarative connectors instead of no-op'ing against a stale registry (#7742) + +Editing a declarative provider-bound `connectors:` entry and reloading metadata +changed nothing: no teardown, no re-materialize, the pre-edit connector kept +serving until the process restarted. `os dev` masked it — it restarts the serve +child on recompile — but a **Studio package publish** into a running server +walked straight into it. + +The reconcile's INPUT was the problem, not the reconcile. It read +`ql.registry.listItems('connector')`, which is a BOOT snapshot: the artifact +reload re-ingests OBJECT definitions into that registry (ObjectQL's own +`metadata:reloaded` handler) and nothing re-ingests connector items, so the +reconcile compared the boot world against itself and found nothing to do. Every +existing test drove the reload through a hand-mutated fake registry, which is +why it looked covered. + +The reconcile (and the descriptor audit beside it) now reads the declaration +from the sources a reload actually refreshes, folded over that registry read: + +- the **artifact carried on the `metadata:reloaded` payload** — the dev/HMR + reload trigger, and the only place an edited or deleted connector definition + exists. The fold is scoped to the packages the artifact speaks for, so a + connector contributed by an unrelated plugin package survives a reload, while + one deleted from the reloaded stack is torn down; +- **`protocol.getMetaItems({ type: 'connector' })`** — the flattened `/meta` + view the flow re-sync already reads, which layers the `sys_metadata` rows a + Studio publish promotes to active over the registry. Consulted on post-boot + reconciles only; boot keeps its registry read, whose snapshot is current by + construction. + +Both reads fail safe: an absent, failing, or empty answer is treated as "no +answer" and never tears down a live connector, and an announcement carrying no +connector collection at all (a publish's bare `{ changed }`) leaves every +instance alone. An unchanged entry still hashes to the same signature and is +left untouched, so reloads do not churn live connections. diff --git a/packages/services/service-automation/src/connector-reload-reingest.test.ts b/packages/services/service-automation/src/connector-reload-reingest.test.ts new file mode 100644 index 0000000000..a9a83af770 --- /dev/null +++ b/packages/services/service-automation/src/connector-reload-reingest.test.ts @@ -0,0 +1,307 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #7742 — the `metadata:reloaded` connector reconcile must read a set the +// reload actually refreshed. +// +// Every other connector reconcile test (./connector-materialization.test.ts) +// drives the reload through a HAND-MUTATED fake registry: the harness swaps the +// array `listItems('connector')` returns and only then fires the hook, so the +// reconcile always sees the new definition. Nothing on the real reload path +// does that swap. `MetadataPlugin._reloadAndAnnounce` re-ingests the artifact +// into the MetadataManager and announces `metadata:reloaded`; ObjectQL's own +// handler re-ingests the payload's OBJECT definitions into the SchemaRegistry, +// and nothing re-ingests its `connector` items. So on the real path the +// registry the reconcile reads still holds the BOOT snapshot, and a +// connector-only edit reconciles to a no-op: no teardown, no re-materialize, +// the pre-edit connector keeps serving. +// +// These tests therefore use a REAL `SchemaRegistry` and deliberately never +// mutate it across the reload — the registry going stale is the fact under +// test, not an oversight — and assert on the OBSERVABLE effect of the +// reconcile: the old instance's `close()` and a re-materialization carrying the +// new `providerConfig`. + +import { describe, it, expect } from 'vitest'; +import { LiteKernel } from '@objectstack/core'; +import { SchemaRegistry } from '@objectstack/objectql'; +import type { + Connector, + ConnectorProviderContext, + ConnectorProviderFactory, +} from '@objectstack/spec/integration'; +import { AutomationServicePlugin } from './plugin.js'; +import type { AutomationEngine } from './engine.js'; + +const flush = () => new Promise((r) => setTimeout(r, 0)); + +const PKG = 'com.acme.billing'; + +/** A provider-bound declarative connector entry — the shape an artifact carries. */ +function providerConnector(name: string, endpoint: string) { + return { + name, + label: name, + type: 'api', + provider: 'fake', + providerConfig: { endpoint }, + }; +} + +/** + * A provider factory that records each materialization's `providerConfig` and + * each teardown, so a test can prove a re-materialization happened AND that it + * carried the edited definition (rather than re-running the boot one). + */ +function makeRecordingProvider() { + const materialized: Array<{ name: string; endpoint: unknown }> = []; + const closed: string[] = []; + const factory: ConnectorProviderFactory = (ctx: ConnectorProviderContext) => { + materialized.push({ + name: ctx.name, + endpoint: (ctx.providerConfig as Record | undefined)?.endpoint, + }); + 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 }) }, + close: async () => { closed.push(ctx.name); }, + }; + }; + return { factory, materialized, closed }; +} + +/** + * Boot the automation plugin over a REAL `SchemaRegistry` holding the boot-time + * connector items, and expose `reloadArtifact(...)` — which fires exactly what + * `MetadataPlugin._reloadAndAnnounce` fires (`{ changed, metadata }`, the + * freshly parsed artifact) and, like the real path, leaves the registry alone. + */ +async function bootWithRealRegistry( + bootConnectors: unknown[], + opts: { served?: () => unknown[] | undefined; packageOf?: (item: any) => string } = {}, +) { + const registry = new SchemaRegistry({ multiTenant: false } as never); + for (const item of bootConnectors) { + registry.registerItem('connector', item, 'name' as never, opts.packageOf?.(item) ?? PKG); + } + const { factory, materialized, closed } = makeRecordingProvider(); + + let captured: any; + 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) { + captured = ctx; + ctx.registerService('objectql', { registry }); + if (opts.served) { + // Stands in for the protocol's flattened `/meta/connector` view: + // the registry read with the `sys_metadata` overlay rows a Studio + // publish promoted to active layered over it. + ctx.registerService('protocol', { + getMetaItems: async ({ type }: { type: string }) => { + if (type !== 'connector') return []; + const served = opts.served!(); + // `undefined` stands for a read that FAILS (the + // sys_metadata query throwing), not an empty view. + if (served === undefined) throw new Error('sys_metadata unavailable'); + return served; + }, + }); + } + ctx.getService('automation').registerConnectorProvider('fake', factory); + }, + async start() {}, + } as never); + await kernel.bootstrap(); + await flush(); + + /** The dev artifact reload: MetadataPlugin's payload, registry left as-is. */ + const reloadArtifact = async (connectors: unknown[] | undefined) => { + await captured.trigger('metadata:reloaded', { + changed: ['connector/billing'], + metadata: { + manifest: { id: PKG, version: '1.0.0' }, + objects: [], + ...(connectors === undefined ? {} : { connectors }), + }, + }); + await flush(); + }; + + return { + kernel, + registry, + reloadArtifact, + materialized, + closed, + engine: kernel.getService('automation') as AutomationEngine, + trigger: async (payload?: unknown) => { + await captured.trigger('metadata:reloaded', payload); + await flush(); + }, + }; +} + +describe('#7742 — connector reconcile reads a set the reload refreshed', () => { + it('re-materializes an edited connector on a dev artifact reload that never touches the registry', async () => { + const h = await bootWithRealRegistry([providerConnector('billing', 'https://old.example')]); + expect(h.materialized).toEqual([{ name: 'billing', endpoint: 'https://old.example' }]); + + // The edit lands in the artifact — and ONLY there, exactly as on the + // real reload path. + await h.reloadArtifact([providerConnector('billing', 'https://new.example')]); + + // Observable effect: the boot instance was torn down and a new one + // materialized from the EDITED definition. + expect(h.closed).toEqual(['billing']); + expect(h.materialized).toEqual([ + { name: 'billing', endpoint: 'https://old.example' }, + { name: 'billing', endpoint: 'https://new.example' }, + ]); + // …and it is the live, dispatchable one. + expect(h.engine.getRegisteredConnectors()).toContain('billing'); + + await h.kernel.shutdown(); + }); + + it('leaves an unedited connector alone (no reconnect churn on every reload)', async () => { + const h = await bootWithRealRegistry([providerConnector('billing', 'https://old.example')]); + + await h.reloadArtifact([providerConnector('billing', 'https://old.example')]); + + expect(h.closed).toEqual([]); + expect(h.materialized).toHaveLength(1); + + await h.kernel.shutdown(); + }); + + it('tears down a connector the reloaded artifact no longer declares', async () => { + const h = await bootWithRealRegistry([ + providerConnector('billing', 'https://old.example'), + providerConnector('shipping', 'https://ship.example'), + ]); + expect(h.materialized).toHaveLength(2); + + // `shipping` deleted from the stack; the registry still lists it. + await h.reloadArtifact([providerConnector('billing', 'https://old.example')]); + + expect(h.closed).toEqual(['shipping']); + expect(h.engine.getRegisteredConnectors()).not.toContain('shipping'); + expect(h.engine.getRegisteredConnectors()).toContain('billing'); + + await h.kernel.shutdown(); + }); + + it('materializes a connector added by the reloaded artifact', async () => { + const h = await bootWithRealRegistry([providerConnector('billing', 'https://old.example')]); + + await h.reloadArtifact([ + providerConnector('billing', 'https://old.example'), + providerConnector('shipping', 'https://ship.example'), + ]); + + expect(h.engine.getRegisteredConnectors()).toContain('shipping'); + expect(h.materialized).toEqual([ + { name: 'billing', endpoint: 'https://old.example' }, + { name: 'shipping', endpoint: 'https://ship.example' }, + ]); + + await h.kernel.shutdown(); + }); + + it('leaves another package’s connector alone when an app reloads', async () => { + // The fold is scoped to what the reloaded artifact speaks for. A + // connector contributed by a DIFFERENT package is absent from that + // artifact for the obvious reason — it was never in it — and reading + // that absence as a deletion would take an unrelated integration down + // on every recompile. + const h = await bootWithRealRegistry( + [providerConnector('billing', 'https://old.example'), providerConnector('slack', 'https://slack.example')], + { packageOf: (item) => (item.name === 'slack' ? 'com.objectstack.connector-slack' : PKG) }, + ); + expect(h.materialized).toHaveLength(2); + + await h.reloadArtifact([providerConnector('billing', 'https://new.example')]); + + expect(h.closed).toEqual(['billing']); // only the edited one + expect(h.engine.getRegisteredConnectors()).toContain('slack'); + expect(h.engine.getRegisteredConnectors()).toContain('billing'); + + await h.kernel.shutdown(); + }); + + // The named PRODUCTION trigger. A Studio package publish promotes the + // authored `sys_metadata` rows to active and announces `metadata:reloaded` + // with `{ changed }` only — no artifact — so the payload fold above cannot + // see the edit. What CAN is the protocol's flattened `/meta/connector` view: + // the same registry read with those active overlay rows layered over it + // (`mergePackageAwareOverlay` — a row wins over the registry entry it + // shadows). The reconcile reads that view on every post-boot run. + describe('Studio package publish (no artifact on the payload)', () => { + it('re-materializes off the published view while the registry stays stale', async () => { + let served: unknown[] = [providerConnector('billing', 'https://old.example')]; + const h = await bootWithRealRegistry([providerConnector('billing', 'https://old.example')], { + served: () => served, + }); + expect(h.materialized).toHaveLength(1); + + // The publish: the overlay row now carries the edited definition, + // and the registry — as in a real scoped kernel — does not. + served = [providerConnector('billing', 'https://published.example')]; + await h.trigger({ changed: ['connector/billing'] }); + + expect(h.closed).toEqual(['billing']); + expect(h.materialized[1]).toEqual({ name: 'billing', endpoint: 'https://published.example' }); + + await h.kernel.shutdown(); + }); + + it('an empty or failing published view never tears down live connectors', async () => { + let served: unknown[] | undefined = [providerConnector('billing', 'https://old.example')]; + const h = await bootWithRealRegistry([providerConnector('billing', 'https://old.example')], { + served: () => served, + }); + + // Empty answer while the registry still lists the connector: the + // view is a superset of the registry by construction, so this is + // "not served the way we assume", not "the stack declares none". + served = []; + await h.trigger({ changed: ['object/account'] }); + expect(h.closed).toEqual([]); + expect(h.engine.getRegisteredConnectors()).toContain('billing'); + + // Same for a read that throws. + served = undefined; + await h.trigger({ changed: ['object/account'] }); + expect(h.closed).toEqual([]); + expect(h.engine.getRegisteredConnectors()).toContain('billing'); + + await h.kernel.shutdown(); + }); + }); + + it('a payload carrying no connector collection changes nothing', async () => { + const h = await bootWithRealRegistry([providerConnector('billing', 'https://old.example')]); + + // A reload whose artifact has no `connectors:` key at all, and a bare + // announce with no payload (the Studio publish shape): neither may be + // read as "the stack declares no connectors" and tear the live one down. + await h.reloadArtifact(undefined); + await h.trigger({ changed: ['object/account'] }); + await h.trigger(); + + expect(h.closed).toEqual([]); + expect(h.engine.getRegisteredConnectors()).toContain('billing'); + + await h.kernel.shutdown(); + }); +}); diff --git a/packages/services/service-automation/src/plugin.ts b/packages/services/service-automation/src/plugin.ts index d26cca586c..f7ecd0d23a 100644 --- a/packages/services/service-automation/src/plugin.ts +++ b/packages/services/service-automation/src/plugin.ts @@ -287,6 +287,61 @@ interface DeclaredConnectorItem { auth?: ConnectorInstanceAuth; } +/** + * [#7742] Fold the connector collection of a freshly reloaded artifact into the + * connector items read out of the ObjectQL registry, and return the set the + * reconcile should treat as declared. + * + * The registry is a BOOT snapshot for connectors. `MetadataPlugin`'s reload + * re-ingests the artifact into the MetadataManager and announces + * `metadata:reloaded`; ObjectQL's handler re-ingests that payload's OBJECT + * definitions into the SchemaRegistry — and nothing re-ingests its `connector` + * items. So after a connector-only edit the registry still describes the + * pre-edit world, and a reconcile reading it alone is a no-op: the old + * connector keeps serving until the process restarts (masked in `os dev`, + * which restarts the serve child, but not on a Studio package publish). + * + * The fold is a REPLACEMENT scoped to what the artifact owns, not a union — + * a union could never observe a deletion, and deletion is half of what a + * reconcile is for: + * + * - a registry entry whose name the artifact re-declares is superseded by the + * artifact's (an edit re-materializes; an unchanged one still hashes to the + * same signature and is left alone); + * - a registry entry owned by one of the artifact's own packages but ABSENT + * from it was deleted from the stack — dropped, so the reconcile tears it + * down; + * - every other registry entry is kept. That is the load-bearing half of the + * scoping: a connector contributed by a plugin package the artifact says + * nothing about must survive a reload of an unrelated app. + * + * `owners` is the artifact's package coordinates (see + * {@link AutomationServicePlugin.captureReloadedConnectors}). When it is empty + * — an artifact with no manifest id, whose items were never stamped — the + * middle rule cannot fire and the fold degrades to name-wise replacement: an + * edited or added connector is still picked up, a deleted one waits for the + * restart it always waited for. + */ +export function mergeDeclaredConnectorSources( + registryItems: readonly unknown[], + artifact: { items: readonly unknown[]; owners: ReadonlySet } | undefined, +): unknown[] { + if (!artifact) return [...registryItems]; + const declaredNames = new Set( + artifact.items + .map((item) => (item as DeclaredConnectorItem)?.name) + .filter((name): name is string => typeof name === 'string' && name.length > 0), + ); + const kept = registryItems.filter((item) => { + const name = (item as DeclaredConnectorItem)?.name; + if (typeof name === 'string' && declaredNames.has(name)) return false; + const owner = (item as { _packageId?: unknown })?._packageId; + if (typeof owner === 'string' && artifact.owners.has(owner)) return false; + return true; + }); + return [...kept, ...artifact.items]; +} + /** * Descriptor-only contract audit (#2612): declarative `connectors:` stack * entries are catalog descriptors — they are registered as metadata but never @@ -412,6 +467,23 @@ export class AutomationServicePlugin implements Plugin { */ private degradedInstances = new Map(); private declarativeRetryTimer?: ReturnType; + /** + * [#7742] The connector collection carried by the most recent + * `metadata:reloaded` payload — the ONE view of the declarative connectors + * that a reload actually refreshes. See + * {@link AutomationServicePlugin.captureReloadedConnectors} for why the + * ObjectQL registry alone cannot answer after a reload, and + * {@link mergeDeclaredConnectorSources} for how the two are combined. + * + * Kept as plugin state rather than read off the payload at the call site + * because a reconcile has three callers and only one of them has a payload: + * a degraded-instance retry (or a later publish-triggered reconcile) firing + * minutes after the reload must still see the artifact the user last + * compiled, not the boot snapshot it would otherwise fall back to — which + * would tear down the freshly materialized instance and rebuild the + * pre-edit one. + */ + private reloadedConnectors?: { items: unknown[]; owners: ReadonlySet }; /** Serializes reconcile runs — see {@link materializeDeclaredConnectors}. */ private reconcileQueue: Promise = Promise.resolve(); private destroyed = false; @@ -822,8 +894,16 @@ export class AutomationServicePlugin implements Plugin { // restart. Re-register every current flow (registerFlow re-binds its trigger // idempotently — ScheduleTrigger.start cancels + reschedules) and unregister // flows that vanished so their jobs stop. - ctx.hook('metadata:reloaded', async () => { + ctx.hook('metadata:reloaded', async (payload?: unknown) => { await this.resyncFlowsFromProtocol(ctx); + // #7742 — take the connector collection off the payload FIRST. The + // reconcile below used to read `listItems('connector')` alone, and + // nothing on the reload path ever re-ingests connector items into + // that registry (ObjectQL's own handler re-ingests the payload's + // OBJECTS and stops there) — so it reconciled the boot snapshot + // against itself and did nothing at all. See + // {@link captureReloadedConnectors}. + this.captureReloadedConnectors(payload); // A Studio publish / dev reload can add, change, or remove declarative // provider-bound connector instances — reconcile the live registry so // a newly-published instance becomes dispatchable (and a removed one @@ -832,7 +912,7 @@ export class AutomationServicePlugin implements Plugin { await this.materializeDeclaredConnectors(ctx, { fatal: false }); // Re-audit so the inert-descriptor warning stays current for plain // descriptors (see auditDeclaredConnectors). - this.auditDeclaredConnectors(ctx); + await this.auditDeclaredConnectors(ctx); }); // ── Cold-boot bind via the PROTOCOL's flattened flow view ───────────── @@ -856,7 +936,7 @@ export class AutomationServicePlugin implements Plugin { // Every plugin's init()/start() has completed here, so connector // plugins have registered their runtime connectors — the earliest // point the declared-vs-registered comparison is meaningful. - this.auditDeclaredConnectors(ctx); + await this.auditDeclaredConnectors(ctx); }); // ── Silent-miss audit: unbound triggered flows (2026-07-17 eval) ────── @@ -977,25 +1057,122 @@ export class AutomationServicePlugin implements Plugin { } /** - * Descriptor-only contract audit (#2612) — warn, once per boot/reload, - * about declarative `connectors:` entries that declare actions but have no - * runtime registration (see {@link findInertDeclaredConnectors}). Reads - * the same ObjectQL registry `registerApp` writes declarative connector - * metadata into. Best-effort: without an ObjectQL registry there is - * nothing declared, hence nothing to audit. + * [#7742] Record the connector collection of a `metadata:reloaded` payload + * so the reconcile that follows reads the artifact that was just loaded + * instead of the boot-time registry snapshot. See + * {@link mergeDeclaredConnectorSources} for the merge this feeds. + * + * A payload with no `metadata.connectors` ARRAY leaves the previous + * snapshot untouched — deliberately, and this is the distinction the whole + * capture turns on. Absence means "this announcement carries no connector + * view": a Studio publish announces `{ changed }` with no artifact at all, + * and a compiled artifact omits the key entirely when the stack declares no + * connectors. Reading either as "the stack now declares none" would tear + * down every live instance on the next unrelated publish. An artifact that + * carries an EMPTY array is the honest "none left" and does tear down — + * that shape is exactly what the compiler emits when the last entry is + * deleted from a stack that had one. + * + * `owners` scopes the deletion half of the merge to the packages this + * artifact speaks for: its manifest id (the same coordinate + * `MetadataPlugin._parseAndRegisterArtifact` stamps items with) plus any + * `_packageId` already stamped on the items themselves. */ - private auditDeclaredConnectors(ctx: PluginContext): void { - if (!this.engine) return; - let declared: unknown[] = []; + private captureReloadedConnectors(payload: unknown): void { + const metadata = (payload as { metadata?: Record } | undefined)?.metadata; + const items = metadata?.connectors; + if (!Array.isArray(items)) return; + const owners = new Set(); + const manifestId = + (metadata as { manifest?: { id?: unknown } } | undefined)?.manifest?.id + ?? (metadata as { id?: unknown } | undefined)?.id; + if (typeof manifestId === 'string' && manifestId.length > 0) owners.add(manifestId); + for (const item of items) { + const owner = (item as { _packageId?: unknown })?._packageId; + if (typeof owner === 'string' && owner.length > 0) owners.add(owner); + } + this.reloadedConnectors = { items, owners }; + } + + /** + * The declarative `connectors:` entries this plugin should treat as the + * current declaration — the input to both the descriptor audit and the + * ADR-0097 reconcile. + * + * Three sources, because no single one of them is current after a reload + * (#7742) — and the two reload TRIGGERS refresh different ones: + * + * - the ObjectQL registry `registerApp` writes connector metadata into. It + * is authoritative at boot and for everything a plugin package + * contributes, and it is never refreshed for connectors afterwards. + * - `protocol.getMetaItems({ type: 'connector' })` — the same flattened + * `/meta` view the flow re-sync reads. It IS that registry read plus the + * `sys_metadata` overlay rows layered over it, which is what a **Studio + * package publish** promotes to active: the named production trigger. + * Only consulted `post` = true (after boot). At boot the registry has + * just been built and is current by construction, while this read costs + * a `sys_metadata` query and can fail — neither belongs on the + * fail-loudly boot path. + * - the artifact carried by the last `metadata:reloaded` payload, folded + * in by {@link mergeDeclaredConnectorSources} — the half that covers the + * **dev artifact reload**, whose payload is the only place an edited (or + * deleted) connector definition exists at all. + * + * A protocol read that answers with NOTHING while the registry has entries + * is treated as no answer rather than as an empty declaration: the view is + * a superset of the registry by construction, so an empty one means the + * type is not being served the way we assume — and acting on it would tear + * down every live connector on a reload. Same rule as the flow re-sync's + * `null` read: never tear down on an absent answer. + */ + private async readDeclaredConnectorItems( + ctx: PluginContext, + opts: { post: boolean }, + ): Promise { + let registryItems: unknown[] | undefined; try { const ql = ctx.getService<{ registry?: { listItems?: (type: string) => unknown[] }; }>('objectql'); - declared = ql?.registry?.listItems?.('connector') ?? []; + registryItems = ql?.registry?.listItems?.('connector') ?? []; } catch { - return; + registryItems = undefined; // no registry service + } + + let base = registryItems; + if (opts.post) { + const served = await this.readMetaItemsFromProtocol(ctx, 'connector'); + // An empty served view is believed only when the registry is itself + // present and empty — then the two agree and nothing is declared. + // Empty while the registry HAS entries (or while there is no + // registry to compare it against) is "not served the way we assume". + if (served && (served.length > 0 || registryItems?.length === 0)) { + base = served; + } + } + + // Before #7742 a missing registry ended the read. It still does — unless + // a reload declared connectors, which is a declaration in its own right. + if (base === undefined) { + if (!this.reloadedConnectors) return undefined; + base = []; } - if (declared.length === 0) return; + return mergeDeclaredConnectorSources(base, this.reloadedConnectors); + } + + /** + * Descriptor-only contract audit (#2612) — warn, once per boot/reload, + * about declarative `connectors:` entries that declare actions but have no + * runtime registration (see {@link findInertDeclaredConnectors}). Reads the + * same declaration the reconcile does ({@link readDeclaredConnectorItems}), + * so the warning describes the stack as it is NOW rather than as it booted + * (#7742). Best-effort: with nothing declared there is nothing to audit. + */ + private async auditDeclaredConnectors(ctx: PluginContext): Promise { + if (!this.engine) return; + // Both call sites are post-boot hooks (`kernel:ready`, `metadata:reloaded`). + const declared = await this.readDeclaredConnectorItems(ctx, { post: true }); + if (!declared || declared.length === 0) return; const live = new Set(this.engine.getConnectorDescriptors().map((d) => d.name)); const inert = findInertDeclaredConnectors(declared, live); if (inert.length === 0) return; @@ -1032,8 +1209,14 @@ export class AutomationServicePlugin implements Plugin { * entry's old connector keeps serving until the new one materializes * successfully. * - * Reads the same ObjectQL registry the descriptor audit uses; without one - * there is nothing declared, hence nothing to reconcile. + * Reads the same declaration the descriptor audit does + * ({@link readDeclaredConnectorItems}); with nothing declared there is + * nothing to reconcile. That reader is what makes a RELOAD reconcile + * meaningful at all (#7742): reading only the ObjectQL registry — which no + * reload path re-ingests connector items into — reconciled the boot + * snapshot against itself, so every run of this function on + * `metadata:reloaded` was a no-op and the pre-edit connector kept serving + * until the process restarted. * * **Upstream-availability exception (#3017):** a provider factory that * throws the `CONNECTOR_UPSTREAM_UNAVAILABLE` marker (e.g. `connector-mcp` @@ -1063,15 +1246,11 @@ export class AutomationServicePlugin implements Plugin { // rescheduled at the end if instances remain degraded. this.clearDeclarativeRetryTimer(); - let declared: unknown[] = []; - try { - const ql = ctx.getService<{ - registry?: { listItems?: (type: string) => unknown[] }; - }>('objectql'); - declared = ql?.registry?.listItems?.('connector') ?? []; - } catch { - return; // no registry — nothing declared - } + // `post` = every reconcile that is not the boot one: the reload hook and + // the degraded-retry timer alike may see connectors a Studio publish + // wrote after boot. + const declared = await this.readDeclaredConnectorItems(ctx, { post: !opts.fatal }); + if (!declared) return; // no registry, no reloaded artifact — nothing declared // Report a reconcile problem: fatal (boot) throws; soft (reload) logs. // @@ -1529,6 +1708,21 @@ export class AutomationServicePlugin implements Plugin { private async readFlowDefsFromProtocol( ctx: PluginContext, ): Promise | null> { + return (await this.readMetaItemsFromProtocol(ctx, 'flow')) as Array<{ name?: string }> | null; + } + + /** + * One read of the protocol's flattened `/meta/` view, normalized and + * stripped of read decorations — the shared body of + * {@link readFlowDefsFromProtocol} and the connector half of + * {@link readDeclaredConnectorItems} (#7742). `null` means "no answer" + * (no protocol service, or the read failed); callers must treat that as + * leave-everything-alone, never as an empty declaration. + */ + private async readMetaItemsFromProtocol( + ctx: PluginContext, + type: string, + ): Promise { let protocol: { getMetaItems?(q: { type: string }): Promise } | undefined; try { protocol = ctx.getService('protocol'); @@ -1539,25 +1733,25 @@ export class AutomationServicePlugin implements Plugin { let raw: unknown; try { - raw = await protocol.getMetaItems({ type: 'flow' }); + raw = await protocol.getMetaItems({ type }); } catch (err) { // #5048 — structured `meta`, not string interpolation (same reason as // the register seams below; see ./thrown-cause-diagnostics.ts). ctx.logger.warn( - "[Automation] flow read from protocol failed: getMetaItems('flow')", + `[Automation] ${type} read from protocol failed: getMetaItems('${type}')`, describeThrownForLog(err), ); return null; } // getMetaItems hands back a bare array or an `{ items: [...] }` envelope, - // and each entry is either the flow doc or an `{ item: }` wrapper. + // and each entry is either the doc itself or an `{ item: }` wrapper. const list = Array.isArray(raw) ? raw : (((raw as { items?: unknown[] })?.items) ?? []); return list.map((entry) => { const doc = entry && typeof entry === 'object' && 'item' in entry ? (entry as { item: unknown }).item : entry; - return stripReadDecorations(doc) as { name?: string }; + return stripReadDecorations(doc); }); }