Repository navigation
Commit ae5bd5b
fix(service-datasource): name the reason and the bound objects in the datasource delete refusal (#30513)
<!-- claude-projects-attribution: {"github_login":"os-bill"} -->
_Requested by **Bill** · [project
thread](https://claude.ai/code/project/chan_016iMXWSZDzqWRWvWzdNo8wg?thread=cmsg_016iMXWSZDzqWRWvWzdNo8wgXfJgMndozTHYhLa6rZRghr)_
domain:server#2 · os-bill · session_01Utp6Cb4nPCxmCZ7EzB1A1v
Fixes #30444
Clause-②: yes (widening) — the bound-object refusal keeps `400`
`DATASOURCE_ADMIN_ERROR` and its message, and gains `error.details`;
nothing a client reads today moves.
Metadata-protocol: none
## 维护者速读
- 改了什么:删除一个仍有对象在用的数据源时,拒绝答复除了原来那句英文,还在 `error.details` 里带上机器可读的原因
`objects_bound`、数量和这些对象的名字(排好序的完整列表)。状态码、错误码和英文原句都不变。
- 为什么改:以前拒绝只有一句英文和一个数字,客户端分不清这是哪种
400,也说不出是哪些对象挡着;设置页只能自己再去读对象元数据才能告诉管理员。现在任何客户端(设置页、CLI、API
调用方)都能用自己的语言直接列出对象名。
- 风险与回滚:只加字段,不改已有的字段。名字和数量出自同一次读取,二者不会对不上;元数据读不全时的 503 拒绝原样不动;其他 400
一律不带 `details`,路由只转发自己声明了 400 信封的拒绝的 `details`,驱动错误身上碰巧有 `details`
也不会漏出去。回滚即撤本 PR。
- 先看:`packages/services/service-datasource/src/admin-routes.ts` 的
`badRequest`(转发 `details` 的三重条件);`datasource-admin-service.ts` 的
`boundObjectsError` 与 `removeDatasource`;`datasource-admin-plugin.ts` 的
`countBoundObjectsDiagnosed`(名字与数量同出一次读取)。
- 两分钟验证:`pnpm --filter @objectstack/service-datasource exec vitest run
src/__tests__/admin-routes.test.ts`,预期 1 个文件全绿,其中 `#30444` 一组 5 条。
- 需要你做的:无,席位落地。
## What changes
**The rule: the bound-object refusal says why and names what is in the
way, in `error.details`, from the read that produced its count.**
- **`DatasourceAdminService.removeDatasource`
(`@objectstack/service-datasource`).** The bound-object refusal is built
by `boundObjectsError`: the same message (`Cannot remove datasource
'NAME': N object(s) are still bound to it.`), plus `code:
'DATASOURCE_ADMIN_ERROR'`, `status: 400` and `details: { reason:
'objects_bound', count, objects }`. `objects` is the full list of bound
object names, sorted; no cap (the list is bounded by the objects bound
to one datasource).
- **One read.**
`DatasourceAdminServiceConfig.countBoundObjectsDiagnosed` may now return
`objects?: string[]`, and the plugin wiring returns the names of the
same filtered `listObjects()` rows it counts. The count in the message,
`details.count` and `details.objects` all come from that one result. ⛔
Not `listObjectsBoundTo`: that is a second scan over another registry
(the engine's), and a refusal built from two reads could say "2 objects"
and name three. A host that supplies only `countBoundObjects` (no
diagnosed read) gets `{ reason, count }` without `objects`.
- **`badRequest` in `admin-routes.ts`.** A thrown error whose `status`
is `400`, whose `code` is the route's own `SERVICE_ERROR_CODE[service]`
and whose `details` is a plain object is answered with that `details`.
All three must hold, the same pattern as the existing `503` and `404`
relays, so a driver or library error that carries a stray `details`
field keeps answering the bare message. Every other 400 answers exactly
as before.
- **`details.reason` spelling.** snake_case, following the envelope's
existing `details.reason` values in `plugin-auth` (`invalid_body`,
`user_not_found`, `cross_organization`, …) and AGENTS.md's rule for
machine-name data values. `service-settings` spells its one reason in
kebab-case (`locked-by-env`); this package had no prior
`details.reason`.
- **Unchanged:** the status, the code (no new ledger code;
`packages/spec` untouched), the English message, the degraded-read
refusal (`503` `SERVICE_UNAVAILABLE`, `metadataIncompleteError`), the
code-defined refusal, the 204 delete. The
`IDatasourceAdminService.removeDatasource` docblock names the new
`details`. No hand-written doc page states this refusal (searched
`content/docs` outside `references/` and `releases/` for the message and
the code).
## Why
#30444: the refusal was an English sentence with a count and no
machine-readable reason or names, so no client could tell it from any
other 400 of this family or name the objects without re-reading metadata
(PR #30442 does exactly that in Setup).
## Verification
Measured first, at `6c4c43ab` (real `DatasourceAdminService` + real
`registerDatasourceAdminRoutes` on `HonoHttpServer`, a runtime
datasource with two bound objects and a code-defined one):
- `DELETE /api/v1/datasources/parts_db` → `400`
`{"success":false,"error":{"code":"DATASOURCE_ADMIN_ERROR","message":"Cannot
remove datasource 'parts_db': 2 object(s) are still bound to it."}}`, no
`details`.
- `DELETE /api/v1/datasources/code_db` → `400`
`{"success":false,"error":{"code":"DATASOURCE_ADMIN_ERROR","message":"Datasource
'code_db' is code-defined and cannot be removed at runtime."}}`.
- `badRequest` forwarded the message alone (`sendError(res, 400, code,
message)`, no `extra`).
<details><summary>Tests</summary>
- `admin-routes.test.ts` › new top-level describe `DELETE
/datasources/:name — the bound-object refusal carries details (#30444)`,
over the real service and routes: two bound objects answer `400`
`DATASOURCE_ADMIN_ERROR`, the unchanged message and `details { reason:
'objects_bound', count: 2, objects: ['bin', 'part'] }` (sent unsorted;
the harness's `listObjectsBoundTo` answers a different name on purpose),
and the record survives; a count-only host answers `{ reason, count }`;
nothing bound deletes with `204`; the code-defined 400 carries no
`details` (exact body); an error with a stray `details` and no 400
envelope answers the bare message (exact body).
- `datasource-removal-bound-count-outage.test.ts` › the plugin-wiring
describe: the diagnosed read now also asserts `objects` (`['wh_line',
'wh_order']`), and the predating-`listDiagnosed` case's exact `toEqual`
gains `objects: ['wh_order']`. Every other assertion in that file is
unchanged and green, the degraded 503 ones included.
- `pnpm --filter @objectstack/service-datasource test`: Test Files 71
passed (71), Tests 1174 passed (1174), at `bba4b00f` and again at
`f4cc8056` (after merging `main`, 14 commits, none in this package or
its route dependencies). `typecheck`: exit 0 (the package tsconfig
compiles `src/__tests__`, counted with `--listFiles`).
</details>
<details><summary>Ablations (scripts/ablation-replace.mjs; the suite
imports src directly, no build between legs)</summary>
| # | Mutation | Result |
|:--|:--|:--|
| A1 | `badRequest` relays `details` from any error (drop the
`status`/`code` conjuncts) | stray-`details` pin red (1 failed) |
| A2 | `badRequest`'s default 400 branch attaches a `details` |
code-defined pin, stray-`details` pin and the existing `surfaces
lifecycle errors` exact-body pin red (3 failed) |
| A3 | names taken from `listObjectsBoundTo` (a second scan) instead of
the count's read | two-bound pin and count-only pin red (2 failed) |
| A4 | `badRequest` stops forwarding `details` | two-bound pin and
count-only pin red (2 failed) |
Each mutation landed (anchor 1 to 0, replacement 0 to 1); each restore
left `git diff HEAD` empty.
</details>
<details><summary>Gates</summary>
- `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` on `bba4b00f`: 62 families, the same 62 on
`f4cc8056`. All 62 run, exit captured per line before any pipe: 60 exit
0 on the first pass; `check-plugin-teardown-shape --self-test` exit 3
(shallow clone, pinned fixture commit fetched, rerun exit 0);
`check:dual-build-cjs-loads` exit 3 (no `dist`, full build under the
lock, rerun exit 0). `--ran`: 62 derived, 62 run, 0 NOT-MEASURED.
- `pnpm lint` (`eslint . --no-inline-config`): exit 0 on `f4cc8056`.
- Full dogfood under the verify lock on `bba4b00f` (after a full `turbo
run build`): VERDICT command-exit 0, Test Files 419 passed, 3 skipped
(422), Tests 3492 passed, 36 skipped (3528). The merge of `main` adds no
file in this package.
</details>
## Acceptance notes
- The bound-object count (and now the names) comes from the metadata
service's `listObjects()`, while `listObjectsBoundTo` reads the engine
registry; the plugin's own comment says the former is a boot-time copy
that does not see runtime saves. This PR keeps the guard's read as it
was (ruling 3: one read, the degraded path untouched) and does not
measure whether a runtime-saved object escapes the count. Noted, not
filed.
- Once this lands, Setup (PR #30442) may switch from reading object
metadata to `error.details.objects`; that is the ui lane's call.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01Utp6Cb4nPCxmCZ7EzB1A1v)_
Co-authored-by: Claude <noreply@anthropic.com>1 parent 8a0516c commit ae5bd5b
7 files changed
Lines changed: 207 additions & 14 deletions
File tree
- .changeset
- packages/services/service-datasource/src
- __tests__
- contracts
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3 | 3 | | |
4 | 4 | | |
5 | 5 | | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
6 | 11 | | |
7 | 12 | | |
8 | 13 | | |
| |||
368 | 373 | | |
369 | 374 | | |
370 | 375 | | |
| 376 | + | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
| 380 | + | |
| 381 | + | |
| 382 | + | |
| 383 | + | |
| 384 | + | |
| 385 | + | |
| 386 | + | |
| 387 | + | |
| 388 | + | |
| 389 | + | |
| 390 | + | |
| 391 | + | |
| 392 | + | |
| 393 | + | |
| 394 | + | |
| 395 | + | |
| 396 | + | |
| 397 | + | |
| 398 | + | |
| 399 | + | |
| 400 | + | |
| 401 | + | |
| 402 | + | |
| 403 | + | |
| 404 | + | |
| 405 | + | |
| 406 | + | |
| 407 | + | |
| 408 | + | |
| 409 | + | |
| 410 | + | |
| 411 | + | |
| 412 | + | |
| 413 | + | |
| 414 | + | |
| 415 | + | |
| 416 | + | |
| 417 | + | |
| 418 | + | |
| 419 | + | |
| 420 | + | |
| 421 | + | |
| 422 | + | |
| 423 | + | |
| 424 | + | |
| 425 | + | |
| 426 | + | |
| 427 | + | |
| 428 | + | |
| 429 | + | |
| 430 | + | |
| 431 | + | |
| 432 | + | |
| 433 | + | |
| 434 | + | |
| 435 | + | |
| 436 | + | |
| 437 | + | |
| 438 | + | |
| 439 | + | |
| 440 | + | |
| 441 | + | |
| 442 | + | |
| 443 | + | |
| 444 | + | |
| 445 | + | |
| 446 | + | |
| 447 | + | |
| 448 | + | |
| 449 | + | |
| 450 | + | |
| 451 | + | |
| 452 | + | |
| 453 | + | |
| 454 | + | |
| 455 | + | |
| 456 | + | |
| 457 | + | |
| 458 | + | |
| 459 | + | |
| 460 | + | |
| 461 | + | |
| 462 | + | |
| 463 | + | |
| 464 | + | |
| 465 | + | |
| 466 | + | |
| 467 | + | |
| 468 | + | |
| 469 | + | |
| 470 | + | |
| 471 | + | |
| 472 | + | |
| 473 | + | |
| 474 | + | |
| 475 | + | |
| 476 | + | |
| 477 | + | |
| 478 | + | |
| 479 | + | |
| 480 | + | |
| 481 | + | |
| 482 | + | |
| 483 | + | |
| 484 | + | |
| 485 | + | |
| 486 | + | |
| 487 | + | |
| 488 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
277 | 277 | | |
278 | 278 | | |
279 | 279 | | |
| 280 | + | |
| 281 | + | |
280 | 282 | | |
281 | 283 | | |
282 | 284 | | |
| |||
295 | 297 | | |
296 | 298 | | |
297 | 299 | | |
298 | | - | |
| 300 | + | |
299 | 301 | | |
300 | 302 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
658 | 658 | | |
659 | 659 | | |
660 | 660 | | |
| 661 | + | |
| 662 | + | |
| 663 | + | |
| 664 | + | |
| 665 | + | |
| 666 | + | |
| 667 | + | |
| 668 | + | |
| 669 | + | |
| 670 | + | |
| 671 | + | |
| 672 | + | |
661 | 673 | | |
662 | 674 | | |
663 | 675 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
188 | 188 | | |
189 | 189 | | |
190 | 190 | | |
191 | | - | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
192 | 194 | | |
193 | 195 | | |
194 | 196 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
560 | 560 | | |
561 | 561 | | |
562 | 562 | | |
563 | | - | |
564 | | - | |
| 563 | + | |
| 564 | + | |
| 565 | + | |
| 566 | + | |
| 567 | + | |
| 568 | + | |
| 569 | + | |
| 570 | + | |
| 571 | + | |
565 | 572 | | |
566 | | - | |
| 573 | + | |
567 | 574 | | |
568 | 575 | | |
569 | 576 | | |
570 | 577 | | |
571 | 578 | | |
572 | 579 | | |
| 580 | + | |
573 | 581 | | |
574 | 582 | | |
575 | 583 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
333 | 333 | | |
334 | 334 | | |
335 | 335 | | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
336 | 341 | | |
337 | 342 | | |
338 | 343 | | |
339 | | - | |
| 344 | + | |
340 | 345 | | |
341 | 346 | | |
342 | 347 | | |
| |||
527 | 532 | | |
528 | 533 | | |
529 | 534 | | |
| 535 | + | |
| 536 | + | |
| 537 | + | |
| 538 | + | |
| 539 | + | |
| 540 | + | |
| 541 | + | |
| 542 | + | |
| 543 | + | |
| 544 | + | |
| 545 | + | |
| 546 | + | |
| 547 | + | |
| 548 | + | |
| 549 | + | |
| 550 | + | |
| 551 | + | |
| 552 | + | |
| 553 | + | |
| 554 | + | |
| 555 | + | |
| 556 | + | |
| 557 | + | |
| 558 | + | |
| 559 | + | |
| 560 | + | |
| 561 | + | |
| 562 | + | |
| 563 | + | |
| 564 | + | |
| 565 | + | |
| 566 | + | |
| 567 | + | |
| 568 | + | |
| 569 | + | |
530 | 570 | | |
531 | 571 | | |
532 | 572 | | |
| |||
1154 | 1194 | | |
1155 | 1195 | | |
1156 | 1196 | | |
1157 | | - | |
1158 | | - | |
1159 | | - | |
| 1197 | + | |
| 1198 | + | |
| 1199 | + | |
| 1200 | + | |
1160 | 1201 | | |
1161 | 1202 | | |
1162 | 1203 | | |
| |||
1168 | 1209 | | |
1169 | 1210 | | |
1170 | 1211 | | |
1171 | | - | |
1172 | | - | |
1173 | | - | |
1174 | | - | |
1175 | | - | |
| 1212 | + | |
| 1213 | + | |
| 1214 | + | |
1176 | 1215 | | |
1177 | 1216 | | |
1178 | 1217 | | |
| |||
0 commit comments