Repository navigation
Commit df34d37
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)
<!-- claude-projects-attribution: {"github_login":"os-zhuang"} -->
_Requested by **Sam** · [project
thread](https://claude.ai/code/project/chan_01LjZwNRFiG2fkp8SNf6rHNx?thread=cmsg_01LjZwNRFiG2fkp8SNf6rHNxHicY1SvtJqLC6d7EFA3M2v)_
Fixes #28143
Clause-②: no
Metadata-protocol: none
## 维护者速读
- 改了什么:本地安装包(install-local)时,判断"数据源名字是否已被别人占用"改由 runtime 里唯一的判定函数
`datasourceNameHolder`
来做;它除了读元数据,还读引擎上的驱动和代码数据源集合,读不到时拒绝安装(503),不再放行。重启时的 rehydrate
遇到被别人占用的名字,不再接管,只记一条 error 日志;卸载只关闭包自己打开的连接池。
- 为什么改:三个漏洞都在 main 上实测成立:元数据读失败时安装照样 200;只在引擎上注册的数据源(旧的 onEnable
桥)看不见,安装 200;早于 #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).
<details><summary>Ablations (one-off, not kept)</summary>
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
ddb9ddc -> 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 ddb9ddc ->
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
86f1411 -> 32eb6f3d33e1`; `Tests 1 failed | 2 passed (3)`; restored
`blob == HEAD`.
</details>
## Human Minimum
- [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic.
- [x] 2. Every name I added uses the casing for its kind.
- [x] 3. No formatter ran; `pnpm lint` passes.
- [x] 4. One issue, one branch off an up-to-date `main`, one pull
request.
- [x] 5. A change that publishes carries a changeset, and both
declaration lines are filled in here and in the changeset.
- [x] 6. `packages/spec` touched: n/a, not touched.
- [x] 7. No hand edits to `content/docs/references/`,
`packages/*/CHANGELOG.md` or `content/docs/releases/`.
- [x] 8. Title, body and comments are in English (the 维护者速读 block is the
repo's required Chinese maintainer summary).
- [x] The *Before Submitting* list in CONTRIBUTING.md is done.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01QUs3BuoLdzNi2KSkEgUS5q)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent af6ca00 commit df34d37
6 files changed
Lines changed: 595 additions & 45 deletions
File tree
- .changeset
- packages
- cloud-connection/src
- runtime/src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1812 | 1812 | | |
1813 | 1813 | | |
1814 | 1814 | | |
| 1815 | + | |
| 1816 | + | |
| 1817 | + | |
| 1818 | + | |
1815 | 1819 | | |
| 1820 | + | |
| 1821 | + | |
| 1822 | + | |
| 1823 | + | |
| 1824 | + | |
| 1825 | + | |
| 1826 | + | |
| 1827 | + | |
| 1828 | + | |
| 1829 | + | |
| 1830 | + | |
| 1831 | + | |
| 1832 | + | |
| 1833 | + | |
| 1834 | + | |
| 1835 | + | |
| 1836 | + | |
1816 | 1837 | | |
1817 | 1838 | | |
1818 | 1839 | | |
| |||
3601 | 3622 | | |
3602 | 3623 | | |
3603 | 3624 | | |
3604 | | - | |
3605 | | - | |
3606 | | - | |
3607 | | - | |
3608 | | - | |
| 3625 | + | |
| 3626 | + | |
| 3627 | + | |
| 3628 | + | |
| 3629 | + | |
| 3630 | + | |
| 3631 | + | |
| 3632 | + | |
| 3633 | + | |
| 3634 | + | |
| 3635 | + | |
| 3636 | + | |
| 3637 | + | |
| 3638 | + | |
3609 | 3639 | | |
3610 | 3640 | | |
3611 | 3641 | | |
3612 | 3642 | | |
3613 | 3643 | | |
3614 | | - | |
| 3644 | + | |
3615 | 3645 | | |
3616 | 3646 | | |
3617 | 3647 | | |
3618 | 3648 | | |
3619 | | - | |
3620 | | - | |
3621 | | - | |
3622 | | - | |
3623 | | - | |
3624 | | - | |
3625 | | - | |
3626 | | - | |
3627 | | - | |
| 3649 | + | |
| 3650 | + | |
| 3651 | + | |
| 3652 | + | |
| 3653 | + | |
| 3654 | + | |
| 3655 | + | |
| 3656 | + | |
| 3657 | + | |
3628 | 3658 | | |
3629 | | - | |
3630 | | - | |
3631 | | - | |
3632 | | - | |
| 3659 | + | |
| 3660 | + | |
| 3661 | + | |
| 3662 | + | |
| 3663 | + | |
3633 | 3664 | | |
3634 | 3665 | | |
| 3666 | + | |
| 3667 | + | |
| 3668 | + | |
| 3669 | + | |
| 3670 | + | |
| 3671 | + | |
| 3672 | + | |
3635 | 3673 | | |
3636 | 3674 | | |
3637 | 3675 | | |
| |||
0 commit comments