Skip to content

fix(service-automation): reconcile declarative connectors against a set the metadata reload actually refreshes (#7742) - #7847

Merged
huangyiirene merged 1 commit into
mainfrom
claude/issue-7742-connector-reload-reconcile
Aug 11, 2026
Merged

fix(service-automation): reconcile declarative connectors against a set the metadata reload actually refreshes (#7742)#7847
huangyiirene merged 1 commit into
mainfrom
claude/issue-7742-connector-reload-reconcile

Conversation

@huangyiirene

Copy link
Copy Markdown
Collaborator

Fixes #7742

What was broken

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 straight into it is a Studio package publish into a running server.

Premise verified on origin/main before implementing. A test driving the real reload shape against a real SchemaRegistry shows the edit, the addition and the deletion are all no-ops:

× re-materializes an edited connector …        expected [] to deeply equal [ 'billing' ]
× tears down a connector no longer declared …  expected [] to deeply equal [ 'shipping' ]
× materializes a connector added by the reload  expected [ 'billing' ] to include 'shipping'

Root cause

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 (ingestReloadedObjects) and stops there. So the reconcile compared the boot world against itself and found nothing to do.

Every existing connector reload test drove the reload through a hand-mutated fake registry (state.declared = next and only then fire the hook), which is why the path looked covered.

The fix

readDeclaredConnectorItems 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 at all. 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 (mergePackageAwareOverlay: the overlay wins). Post-boot reconciles only — at boot the registry was just built and is current by construction, and this read costs a sys_metadata query and can fail, so neither belongs on the fail-loudly boot path.

Both reads fail safe: an absent, failing, or empty-while-the-registry-is-not answer is treated as no answer and never tears down a live connector; an announcement carrying 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 rather than as it booted. readFlowDefsFromProtocol's body became the shared readMetaItemsFromProtocol — same normalization, same null-means-no-answer contract, byte-identical log message for the flow type.

Scope note

Deliberately not changed: the artifact reload path in packages/metadata*. Making the reload re-ingest connector items into the registry for all consumers is domain:metadata territory; this lands the services-side shape the dispatch named — the automation plugin's reconcile reading a fresh input.

Tests

connector-reload-reingest.test.ts (8 tests) uses a real SchemaRegistry and deliberately never mutates it across the reload — the registry going stale is the fact under test. Assertions are on the observable effect (the old instance's close(), a re-materialization carrying the new providerConfig), not on call counts: edit / add / delete / another package's connector surviving / Studio-publish view / empty + failing view / no-collection payload.

Reverse-verified: with the fix reverted and the tests kept, the four behavioural cases fail exactly the way the QA run observed (no-op reconcile), while the never-tear-down guards pass on both sides.

Gates

  • @objectstack/service-automation suite: 928 passed / 76 files
  • Build closure (--filter @objectstack/service-automation...): 20/20
  • tsc --noEmit: no new errors (3 pre-existing in the untouched nested-region-parity.test.ts)
  • pnpm check:docs-audit-scope: pass
  • Downstream connector plugin suites (connector-mcp 23, connector-rest 16): pass
  • Changeset: .changeset/connector-reload-reingest.md (patch)

One flake observed once and not reproducible in isolation or on re-run: engine.test.ts > should execute unconditional branches in parallel (a wall-clock parallelism assertion, untouched by this diff).


Generated by Claude Code

…et the metadata reload actually refreshes (#7742)

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

vercel Bot commented Aug 11, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 11, 2026 8:38pm

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-automation.

3 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/automation/flows.mdx (via @objectstack/service-automation)
  • content/docs/kernel/services-checklist.mdx (via @objectstack/service-automation)
  • content/docs/plugins/packages.mdx (via @objectstack/service-automation)

2 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/implementation-status.mdx (via @objectstack/service-automation)
  • content/docs/releases/v9.mdx (via @objectstack/service-automation)

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@github-actions

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 31535703749 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Test Core (1/3) — 失败步骤: Run this shard's tests

    �[41m�[1m FAIL �[22m�[49m scripts/build-schemas-check-mode.test.ts�[2m > �[22mbuild-schemas.ts — the drift notice names the direction it measured (#5847)�[2m > �[22mclaims NO direction in a shallow ch
    

历史信号:

  • 本 PR 过去 24h 无队列失败记录(首次)。
  • 过去 24h 队列共有 71 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 在其他 PR 的同类评论里搜同名测试;出现过 ⇒ flaky 实锤,开 issue 修/隔离那条测试。修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 11, 2026

Copy link
Copy Markdown
Collaborator Author

Queue-ejection triage receipt (services seat): known flaky signature — re-queueing once, unchanged.

Readings: queue build 31535703749 failed in Test Core (1/3) on scripts/build-schemas-check-mode.test.ts › claims NO direction in a shallow ch[eckout] — a spec-scripts test this PR's diff (service-automation only, 3 files) never touches; a comment-search shows the same test name in queue-triage comments across multiple unrelated merged PRs this week (#6256, #6534, #6608 among them), and the test is by its own name shallow-checkout-sensitive — environment-dependent, not diff-dependent. Triage checklist case 2. Note for the queue steward's ledger (#5810): today's queue shows 71 failed builds in 24h; this signature deserves a ledger row if it is not already there — flagging, not self-adding, per the ledger's single-channel rule.

Action: one re-queue as-is. A second ejection on this signature hands the PR to the queue steward rather than another self re-queue.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

2 participants