Skip to content

fix(cloud-connection): install-local judges datasource-name holders with one judge that reads the engine and fails closed, and a rehydrate never takes over a held name - #28418

Merged
objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/project-thread-wziudv
Oct 31, 2026
Merged

objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/project-thread-wziudv

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by Sam · project thread
Fixes #28143

Clause-②: no
Metadata-protocol: none

维护者速读

  • 改了什么:本地安装包(install-local)时,判断"数据源名字是否已被别人占用"改由 runtime 里唯一的判定函数 datasourceNameHolder 来做;它除了读元数据,还读引擎上的驱动和代码数据源集合,读不到时拒绝安装(503),不再放行。重启时的 rehydrate 遇到被别人占用的名字,不再接管,只记一条 error 日志;卸载只关闭包自己打开的连接池。
  • 为什么改:三个漏洞都在 main 上实测成立:元数据读失败时安装照样 200;只在引擎上注册的数据源(旧的 onEnable 桥)看不见,安装 200;早于 marketplace install-local: a package's declared datasources are never connected or registered as code-defined, so objects bound to them have no driver after install #28039 安装的包在重启时会覆盖 Setup 里同名数据源、把它变成只读,卸载时还会关掉 Setup 的连接池。
  • 风险与回滚:只影响声明了数据源的包;新增的拒绝只在名字确实被占或读不到时出现。回滚即 revert 本 PR。
  • 先看:packages/runtime/src/declared-datasources.ts(datasourceNameHolder、leaveHeldNames、withdrawPackageDatasources 的 found 判断),packages/cloud-connection/src/marketplace-install-local-plugin.ts 2a 段与 findHeldDatasourceName。
  • 两分钟验证:pnpm --filter @objectstack/runtime exec vitest run --maxWorkers=2 src/declared-datasources.test.ts,预期 17 个用例全绿。
  • 需要你做的:无,席位落地。

What changes

  • One judge, in the runtime. @objectstack/runtime exports datasourceNameHolder(ctx, appId, name) → free | held (holder) | unreadable (reading, cause). It reads, in order: the host's default; the metadata registry (an item not stamped with the package: a Setup row, another package's); the code-datasource set (a name another installed package recorded, or one the host registers from code); the data engine (objectql.getDriverByName, a driver with no metadata item: the legacy onEnable bridge). The package's own earlier registration or recorded pool is free, so a reinstall passes. An absent service holds nothing; a read that throws is unreadable, never free.
  • Install door, step 2a (marketplace-install-local-plugin.ts) judges with it. A held name is the existing 409 RESOURCE_CONFLICT (details.datasource), now including an engine-only holder. A holder that cannot be read is 503 SERVICE_UNAVAILABLE with details: { datasource, reading }, before any write, worded like the access-upgrade check's 503 ("a check that did not run is never taken for no holder"). A runtime without the judge is that 503 as well.
  • Installed calls of connectPackageDatasources (install and every rehydrate) judge each declared name after withdrawing the package's own earlier registrations. A held or unreadable name is left out of all three steps (no code registration, not recorded in the code-datasource set, not connected), and one error line names each, its holder, the consequence and the remedy. The boot path (installed unset) is unchanged and still hands connectDeclared the very list it was given.
  • Uninstall closes only what the package opened. The per-package record now keeps, beside the names, the ones whose connect answered already-registered; the withdrawal does not disconnect those (their holder's pool), while still removing the package's registration and name.

Why

The three edges the card names, each measured on origin/main 75ecaf4a before the fix, with the tests below run against the unchanged source:

  1. Read fault fails open. metadata.get throwing → the install answered 200 (expected 200 to be 503).
  2. Engine-only holder unseen. A driver on the engine with no metadata item → the install answered 200 (expected 200 to be 409).
  3. Rehydrate takes over a Setup row. With a Setup row and its pool holding pkg_store, an installed call returned pkg_store's verdict alongside the package's other datasource (it registered, recorded and "connected" the name); and an uninstall disconnected a pool the connect had only found (disconnected was ['pkg_store','pkg_vault'], expected ['pkg_vault']).

The card's choice for (1)+(2) was "fail closed or read the engine": both are done, since the engine read alone does not close (1).

Tests

  • packages/runtime/src/declared-datasources.test.ts: 9 new cases. The judge: each holder kind (default, Setup row, another package, engine-only), a code-set name, a metadata or engine read fault is unreadable, the package's own registration and recorded pool are free (with a control from another package's side), nothing anywhere is free. The installed call: a Setup row is not taken over and the uninstall leaves its pool and item alone; an engine-only holder is not taken over; an unreadable holder is not taken over and is reported; a pool found already-registered is not disconnected by the uninstall. The fake kernel gained an opt-in engine whose driver registry the fake connect fills, as the real one does.
  • packages/cloud-connection/src/marketplace-install-local-datasource-holder.test.ts (new): through the real install route, a metadata read fault answers 503 SERVICE_UNAVAILABLE and writes no ledger entry or registration; an engine-only holder answers 409 RESOURCE_CONFLICT; a control with an unrelated engine driver installs 200.

Acceptance notes

  • "Records it on the install record" is read as the runtime's per-package install record (what the uninstall reads): a held name is not in it, so the uninstall never touches it. The rehydrate's report is the error line. Nothing new is added to the GET listing or the ledger file; a listing marker like notLoaded would be a separate wire decision.
  • A kernel with no metadata service is not an outage: no metadata holder can exist there, and the package's own registration is skipped the same way, so the judge goes on to the code set and the engine rather than refusing.
  • On a rehydrate, an unreadable holder is left alone too (not taken over, reported), which leaves that package datasource unconnected until a restart reads the holder.
  • One file outside the claimed surface: packages/runtime/src/index.ts, the barrel line exporting datasourceNameHolder and its type.

Verification

All readings are on head 6aa099f6af (base origin/main 75ecaf4a; the 5 commits main gained since touch none of these files), in a dedicated worktree.

  • Measured on main first: the two new test files run against the unchanged source of each package: cloud-connection Tests 2 failed | 1 passed (3) (expected 200 to be 503, expected 200 to be 409, control green); runtime Tests 9 failed | 8 passed (17) (the 5 judge cases have no export to call; the 4 installed-call cases fail on the takeover and the eviction).
  • runtime: pnpm --filter @objectstack/runtime build && test && typecheck under the verify lock → VERDICT command-exit 0; Test Files 492 passed (492), Tests 8010 passed | 19 skipped (8029).
  • cloud-connection: pnpm --filter @objectstack/cloud-connection test && typecheck under the lock → VERDICT command-exit 0; Test Files 63 passed (63), Tests 768 passed (768).
  • Dogfood, the one file this path owns: install-local-declared-datasources.dogfood.test.ts after a full pnpm build → Tests 8 passed (8) (refusals of showcase_external and default, install, uninstall, rehydrate all unchanged).
  • Gates: node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derived 60 commands; all 60 exited 0. Three first answered 3 / timed out for a missing prerequisite (teardown-shape self-test on a shallow clone; check:dual-build-cjs-loads without a full build; check:type-check-debt cut by my own 300 s cap during its closure build) and were re-run to exit 0 after git fetch --unshallow and a full pnpm build. --ran: 60 derived, 60 run, 0 NOT-MEASURED, 0 UNRUN.
  • Lint: pnpm lint → exit 0.
  • check:platform-checklist not run: nothing exported is renamed or removed (one export added).
  • Dogfood battery: n/a (no ! in the title, no (narrowing) arm on the claim's Clause-②: line).
Ablations (one-off, not kept)

Each through node scripts/ablation-replace.mjs under the verify lock; the unit suites import the mutated module from source, so no rebuild was involved. Predictions written before each run.

  1. declared-datasources.ts, anchor if (holding.status === 'free') continue; → … || holding) continue; (nothing is ever left out). Predicted the three not-taken-over cases red. anchor 1 -> 0, blob ddb9ddcbc7ef -> 6f7e7e257f17; Tests 3 failed | 14 passed (17), exactly those three; restored blob == HEAD, git diff HEAD empty.
  2. declared-datasources.ts, the withdrawal's && !record?.found.has(name) dropped (every recorded pool closed). Predicted the found-pool case red. anchor 1 -> 0, blob ddb9ddcbc7ef -> 334b4cea3bdb; Tests 1 failed | 16 passed (17); restored blob == HEAD.
  3. marketplace-install-local-plugin.ts, the 503 branch made unreachable (a guard that is always false: the current time is negative). Predicted the read-fault case red. anchor 1 -> 0, blob 86f1411152ed -> 32eb6f3d33e1; Tests 1 failed | 2 passed (3); restored blob == HEAD.

Human Minimum

  • 1. Schemas start as Zod; packages/spec holds no runtime logic.
  • 2. Every name I added uses the casing for its kind.
  • 3. No formatter ran; pnpm lint passes.
  • 4. One issue, one branch off an up-to-date main, one pull request.
  • 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset.
  • 6. packages/spec touched: n/a, not touched.
  • 7. No hand edits to content/docs/references/, packages/*/CHANGELOG.md or content/docs/releases/.
  • 8. Title, body and comments are in English (the 维护者速读 block is the repo's required Chinese maintainer summary).
  • The Before Submitting list in CONTRIBUTING.md is done.

Generated by Claude Code

@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Oct 31, 2026
@github-actions

github-actions Bot commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/cloud-connection, @objectstack/runtime, touching 16 documentable anchor(s). ⚠️ 1 changed file(s) yielded no anchor (packages/runtime/src/index.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

9 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/client-sdk.mdx (via SERVICE_UNAVAILABLE (literal, a string literal in handleInstall))
  • content/docs/api/error-catalog.mdx (via SERVICE_UNAVAILABLE (literal, a string literal in handleInstall))
  • content/docs/api/index.mdx (via SERVICE_UNAVAILABLE (literal, a string literal in handleInstall))
  • content/docs/automation/webhooks.mdx (via SERVICE_UNAVAILABLE (literal, a string literal in handleInstall))
  • content/docs/deployment/cli.mdx (via MarketplaceInstallLocalPlugin (symbol, a top-level class))
  • content/docs/permissions/authorization.mdx (via SERVICE_UNAVAILABLE (literal, a string literal in handleInstall))
  • content/docs/permissions/sharing-rules.mdx (via SERVICE_UNAVAILABLE (literal, a string literal in handleInstall))
  • content/docs/protocol/kernel/error-handling.mdx (via SERVICE_UNAVAILABLE (literal, a string literal in handleInstall))
  • content/docs/ui/print-pages.mdx (via SERVICE_UNAVAILABLE (literal, a string literal in handleInstall))

⛔ 5 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17/17-0.mdx (via SERVICE_UNAVAILABLE (literal, a string literal in handleInstall))
  • content/docs/releases/v17/17-3.mdx (via SERVICE_UNAVAILABLE (literal, a string literal in handleInstall))
  • content/docs/releases/v17/17-4.mdx (via SERVICE_UNAVAILABLE (literal, a string literal in handleInstall))
  • content/docs/releases/v17/17-7.mdx (via SERVICE_UNAVAILABLE (literal, a string literal in handleInstall))
  • content/docs/releases/v18/18-2.mdx (via SERVICE_UNAVAILABLE (literal, a string literal in handleInstall))

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.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/runtime/src/index.ts) — pages documenting those are invisible to this run
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 60 of 212 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 32 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json a727ba26b17280bc5f3cdd9a9d6addbc5356389a → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 8b59bdef9aa40458b8b950c1a4b2f6e08d3bde0e — the merge of head 039a82db8075949ddf17955460b14673445d3366 into base a727ba26b17280bc5f3cdd9a9d6addbc5356389a, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 8b59bdef9aa40458b8b950c1a4b2f6e08d3bde0e && git checkout 8b59bdef9aa40458b8b950c1a4b2f6e08d3bde0e
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin a727ba26b17280bc5f3cdd9a9d6addbc5356389a 039a82db8075949ddf17955460b14673445d3366 && git checkout -B drift-repro a727ba26b17280bc5f3cdd9a9d6addbc5356389a && git merge --no-ff 039a82db8075949ddf17955460b14673445d3366

node scripts/docs-audit/affected-docs.mjs --json a727ba26b17280bc5f3cdd9a9d6addbc5356389a

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs a727ba26b17280bc5f3cdd9a9d6addbc5356389a → pass the list as
args.docs, on the commit named under Which tree this was computed on.

…stall-local 503)

Claude-Session: https://claude.ai/code/session_01QUs3BuoLdzNi2KSkEgUS5q
Co-authored-by: Claude <noreply@anthropic.com>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#4 · os-zhuang · session_01QUs3BtyH9zgxKzYrJiH18J

Contract review

Served-tier: TIER_DEFAULT
Head-sha: 039a82db8075949ddf17955460b14673445d3366
Local-runs: none

Face: the declaration face alone (Clause-②: yes (widening) in the changeset at this head). There is no packages/spec/src/** path and no governed path, so per contract-review.md the record is the lane seat's at TIER_DEFAULT. Inputs: card #28143, claim 6542814313 and amendment, the os-dev-report 6543432410, the source diff read whole at 6aa099f6af, and the one-file delta to this head (the changeset only).

① Derived judgments

  • datasourceNameHolder and DatasourceNameHolding (new exports from @objectstack/runtime): right. Additive; no existing export changes. The judge reads, in order, the host's default, the metadata registry (an item stamped with this package is its own), the names other installed packages recorded and the host's code-datasource set, then the data engine's drivers. A read that throws is unreadable, never free; a service the kernel lacks holds nothing. One judge serves both the install door and the rehydrate, as the card asks.
  • Install door, 503 SERVICE_UNAVAILABLE with details: { datasource, reading } (new answer): right. It replaces the old fail-open continue (card edge 1), runs before any write, and uses an error code the spec's API vocabulary already carries. A runtime without the judge also answers 503 rather than skipping the check.
  • Install door, engine-only holder now 409 RESOURCE_CONFLICT: right. It is the existing refusal applied to a holder the old check missed (edge 2), not a new code.
  • Rehydrate (connectPackageDatasources, installed call): right. Names are judged after the package's own earlier registrations are withdrawn, so what still holds a name is another holder's. A held or unreadable name is left out of the registration, the code set, the record and the connect, with one error line naming it and the remedy (edge 3). With nothing left out, the boot's list is still handed over by reference (finding: AppPlugin hands the RAW artifact bundle to job scheduling and datasource registration, so a retired duration key (jobs[].timeout, turso config.timeout, memory config.persistence.autoSaveInterval) is silently dropped #23029's identity pin).
  • Uninstall: right. Withdrawal disconnects only pools the package's own connect opened; one answered already-registered is recorded as found and left to its holder.
  • Nothing else moves: no packages/spec path, stored column, metadata key or route added. Metadata-protocol: none holds.

② Semver level

  • @objectstack/runtime patch, @objectstack/cloud-connection patch: accepted. A non-protocol widening ships patch under AGENTS.md's changeset rule, and the changeset at this head names the widening in one sentence.

③ Boundary flags

  • PR body line Clause-②: no: stale. The changeset at this head and the claim amendment are the declaration of record; the ACCEPT records the correction.
  • "install record": read as the runtime's per-package record; no marker on the persisted install record or listing. A marker there would be a new stored shape, so it is out of scope here and recorded in the ACCEPT.

Implemented-by: claude/project-thread-wziudv
Reviewed-by: session_01QUs3BtyH9zgxKzYrJiH18J

VERDICT: PASS

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#4 · os-zhuang · session_01QUs3BtyH9zgxKzYrJiH18J

ACCEPT · PR #28418 · Head-sha 039a82db8075949ddf17955460b14673445d3366 · 2026-10-31T12:58Z

维护者速读

  • 改了什么: install-local 安装和重启后的 rehydrate 改用 runtime 里同一个判定 datasourceNameHolder 判断数据源名字归谁:它也读数据引擎,读不到时拒绝(503),被别人占用时 409;rehydrate 遇到别人占用的名字不再接管,只记一条 error 日志;卸载只关闭本包自己打开的连接池。
  • 为什么改: fix(cloud-connection): an install-local install connects and records the package's declared datasources #28132 留下三个缺口:元数据读失败时放行安装、只注册在引擎上的数据源看不见、marketplace install-local: a package's declared datasources are never connected or registered as code-defined, so objects bound to them have no driver after install #28039 之前装的包在重启时会覆盖 Setup 里同名数据源,卸载时还会关掉 Setup 的连接池。
  • 风险与回滚: 新增两种拒绝(引擎占用 409、读不到 503),都发生在任何写入之前;rehydrate 只是跳过被占用的名字。回滚即 revert 本 PR。
  • 先看: packages/runtime/src/declared-datasources.ts 的 datasourceNameHolder 与 leaveHeldNames;marketplace-install-local-plugin.ts 的 503 分支。
  • 两分钟验证: pnpm --filter @objectstack/runtime exec vitest run src/declared-datasources.test.ts 与 pnpm --filter @objectstack/cloud-connection exec vitest run src/marketplace-install-local-datasource-holder.test.ts。
  • 需要你做的: 无,席位落地。

席位意见:一个判定服务安装和重启两条路,读失败按"未判定"拒绝而不是放行,卸载不再关别人的池;可合。

席位核对

  • Form: 6 files, +595/-45. check-governed-merges.mjs --pr 28418: NOT governed, 640 lines ≤ 3000, Tier S. Declaration of record: Clause-②: yes (widening) (changeset at this head, claim amendment on cloud-connection: install-local's held-datasource-name check fails open on a metadata read fault, and a rehydrate does not re-judge a name a Setup row took before #28039 #28143, TIER_DEFAULT review 6543485690 PASS at this head). Metadata-protocol: none holds. Both packages patch.
  • Premise measured: the dev measured all three edges standing on main 75ecaf4a (read fault 200, engine-only holder 200, rehydrate took over a Setup row and uninstall disconnected a found pool). Seat read the source diff whole; the judgments are in the review.
  • Gates (check-runs at the pinned head): 48 runs: 35 success, 13 skipped, 0 failed, 0 pending; mergeable, clean. Dev: 60/60 gates exit 0, --ran 0 NOT-MEASURED, pnpm lint 0; runtime 8010 and cloud-connection 768 tests green with typecheck; install-local dogfood 8/8; three ablations red as predicted.
  • Accepted deviations: packages/runtime/src/index.ts is outside the claimed surface; it carries only the export line the door needs. The PR body still says Clause-②: no; the relay cannot patch the body, so this record and the changeset are the declaration of record.
  • Out of scope and where it goes: "install record" is read as the runtime's per-package record. No marker is written on the persisted install record or listing for a name the rehydrate left alone; that would be a new stored shape and is not owed here. Skills-docs: none.
  • Landing clause: Tier S ordinary queue landing. The seat flips ready and enables auto-merge through the relay, then confirms MERGED and closes out cloud-connection: install-local's held-datasource-name check fails open on a metadata read fault, and a rehydrate does not re-judge a name a Setup row took before #28039 #28143.

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

1 participant