Repository navigation
Commit b3bd75d
fix(mcp): a coded sandboxed hook refusal answers the body's sentence, not the wrapper naming the hook (#31268)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"}
-->
_Requested by **Leehom** · [project
thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1X3apZSKwFDd6zvjM4gJPKAy)_
Fixes #31246
Clause-②: no
Metadata-protocol: none
## 维护者速读
- 改了什么:沙箱 hook(或 action 主体)抛出带错误码的拒绝时,MCP 的所有工具和记录资源现在只回 hook
作者写的那句话(`Locked.`),错误码和状态码不变;读法用 `/api/v1/data` 同一个函数
`sandboxBusinessMessage`,没有复制逻辑。
- 为什么改:之前 MCP 回的是沙箱调试包装 `hook 'NAME' threw: Error: Locked.`,把 hook
内部名字暴露给 agent;REST 对同一个拒绝只回那句话。
- 风险与回滚:只改 `@objectstack/mcp`
的两处读法,patch;非沙箱的带码拒绝逐字不变,沙箱里崩溃的主体也保持原样。回滚即还原这两处。
- 先看:`packages/mcp/src/mcp-http-tools.ts` 的
`thrownRefusalEnvelope`;`packages/mcp/src/mcp-server-runtime.ts` 记录资源的
catch;`packages/mcp/src/mcp-coded-refusal.test.ts` 的 #31246 块。
- 两分钟验证:`pnpm --filter @objectstack/mcp exec vitest run
src/mcp-coded-refusal.test.ts -t 31246`,预期 16 个用例全部通过。
- 需要你做的:无,席位落地。
## What changes
A QuickJS hook or action body that runs `throw Object.assign(new
Error('Locked.'), { code: 'RECORD_LOCKED', status: 409 })` leaves the
runner as a `SandboxError` with `message` = `hook 'NAME' threw: Error:
Locked.`, `innerMessage` = `Locked.`, `code` and `status` as declared
(measured on the real `QuickJSScriptRunner` at `main` 169ff7e). MCP
rendered `message`, so the agent read the hook's internal name;
`/api/v1/data` answers `409 RECORD_LOCKED` with `Locked.`
(`mapDataError` → `armSentence` → `sandboxBusinessMessage`).
- `thrownRefusalEnvelope` (`packages/mcp/src/mcp-http-tools.ts`) reads
`message` as `sandboxBusinessMessage(err) ?? messageOf(err)`.
`sandboxBusinessMessage` is the one reader in `@objectstack/types` that
the `/data` door uses; there is no second copy of its logic in
`packages/mcp`. The code and status are unchanged.
- The record resource's catch (`MCPServerRuntime.bridgeResources`,
`packages/mcp/src/mcp-server-runtime.ts`) answered a coded refusal with
its own `err.message`. It now uses the envelope's `message`, so it gives
the same sentence as the tools. The code-less `{ error }` answer is
unchanged.
The envelope is the one reader, so every face that renders a coded
refusal moves together: the 13 tools (`list_objects`, `describe_object`,
`validate_expression`, the six record data tools, `delete_records`,
`list_actions`, `run_action`, `resume_run`), the per-row grouping of
`delete_records` (`rowRefusalOf`), the record resource, and the object
resources' coded refusal (`objectResourceRefusal`).
| Throw | Before (MCP) | Now (MCP) | `/api/v1/data` |
|---|---|---|---|
| coded sandboxed refusal (`RECORD_LOCKED`/409, inner `Locked.`) |
`RECORD_LOCKED` · `hook 'demo_guard' threw: Error: Locked.` · 409 |
`RECORD_LOCKED` · `Locked.` · 409 | `RECORD_LOCKED` · `Locked.` · 409 |
| CONTROL: coded non-sandbox refusal, including one whose own text reads
like the wrapper | its message | its message, byte for byte | (not this
card) |
## Tests
- `packages/mcp/src/mcp-coded-refusal.test.ts`: a new `#31246` describe
on the shared `/mcp` HTTP harness. Each of the 13 tools (the `SITES`
table, which is held equal to the registered tool list) answers the
coded sandboxed refusal with `{ code: 'RECORD_LOCKED', message:
INNER_SENTENCE, status: 409 }`. Each case also checks that this equals
`mapDataError`'s answer to the same throw, and that the hook name and
`threw` are absent. CONTROL: three coded non-sandbox throws (a plain
sentence, a sentence that reads like the wrapper, and a `SandboxError`
with an empty `innerMessage`) render byte for byte as `JSON.stringify({
error: { code, message, status } }, null, 2)`. In the record-resource
describe: the coded sandboxed refusal answers `{ error: INNER_SENTENCE,
code, status }`, and a CONTROL keeps a coded non-sandbox wrapper-like
sentence.
- `packages/mcp/src/mcp-delete-records.test.ts`: a coded sandboxed row
refusal is grouped as `{ code: 'RECORD_LOCKED', status: 409, reason:
INNER_SENTENCE }`, and the call goes on.
<details><summary>Red first and ablations</summary>
Red first on `bdefd49f` (after PR #31245 merged, before the fix): the
two files answered `Tests 15 failed | 91 passed (106)`. The 13 tool
rows, the resource row and the `delete_records` row failed; the CONTROLs
passed. After the fix: `Tests 106 passed (106)`.
Ablation 1 (`node scripts/ablation-replace.mjs`, `mcp-http-tools.ts`):
`message: sandboxBusinessMessage(err) ?? messageOf(err),` replaced by
`message: messageOf(err) as string,`. Output: `ok mutation landed:
anchor 1 -> 0, blob 9cca1c0 -> 58b6b2bf2013`, then `Tests 15 failed
| 91 passed (106)`, then `ok restored: blob == HEAD (9cca1c0) and
git diff HEAD is empty`.
Ablation 2 (`mcp-server-runtime.ts`): `error: envelope.message,`
replaced by `error: message as string,`. Output: `ok mutation landed:
anchor 1 -> 0, blob 38c17f5 -> c385c405c4f1`, then `Tests 1 failed
| 105 passed (106)` (the resource row only), then `ok restored: blob ==
HEAD (38c17f5)`.
The tests import the package's source, so no `dist/` is involved.
</details>
## Acceptance notes
Item 2 of the card: each listed face was measured against the REST door
that serves the same verb, using five throws: V1 coded sandboxed
refusal, V2 code-less sandboxed abort, V3 bare `Error`, V4 driver string
code `SQLITE_BUSY`, V5 coded sandboxed **crash** (inner `TypeError:
boom`, code `RECORD_LOCKED`, status 409; the real runner's shape). MCP
was measured over `/mcp` on this branch with a bridge member throwing
each value. REST was measured with the door's own responder:
`mapDataError` for `/data`, `handleRouteError` for `/meta`, and the
runtime `HttpDispatcher.handleActions` for `/actions`.
| Face | Throw | MCP answers | REST answers | Reading |
|---|---|---|---|---|
| every face | V1 | inner sentence (this PR) | inner sentence | fixed
here |
| every face | V5 coded crash | `RECORD_LOCKED` · `hook 'demo_guard'
threw: TypeError: boom` · 409 | `/data`, `/meta`: `500 INTERNAL_ERROR`
`Internal server error`; `/actions`: 500 `INTERNAL_ERROR` | divergence,
not fixed by the shared reader (`sandboxBusinessMessage` declines a
crash by contract); finding |
| metadata tools, `list_actions` | V3 | bare text `socket hang up` |
`/meta`: `500 INTERNAL_ERROR` `Internal server error` | divergence;
finding |
| metadata tools, `list_actions` | V4 | `SQLITE_BUSY` · driver text |
`/meta`: `500 DATABASE_ERROR` `Internal data error` | divergence;
finding |
| record resource | V2 | `{ error: "hook 'demo_guard' threw: Error: Not
enough leave" }` | `/data`: `400 HOOK_ABORTED` `Not enough leave` |
divergence (the #31218 code-less half on this face); finding |
| record resource | V3 / V4 | `{ error: 'socket hang up' }` / `{ error,
code: 'SQLITE_BUSY' }` | `500 INTERNAL_ERROR` / `500 DATABASE_ERROR` |
divergence; same finding |
| `run_action` / `resume_run` | V2 | bare wrapper text naming the body |
`/actions`: `400 HOOK_ABORTED` inner sentence | divergence; finding |
| `run_action` / `resume_run` | V3 | bare text, no code | `/actions`:
`400 VALIDATION_ERROR` same text | divergence (code only); same finding
|
| `run_action` / `resume_run` | V4 | `SQLITE_BUSY` · text, no status |
`/actions`: `400 VALIDATION_ERROR` · text · `declaredCode: SQLITE_BUSY`
| divergence; same finding |
| record data tools | V4 | `SQLITE_BUSY` · driver text, no status (coded
path is read first) | `/data`: `500 DATABASE_ERROR` `Internal data
error` | divergence; finding |
| record data tools | V2 / V3 | `HOOK_ABORTED` 400 / `INTERNAL_ERROR`
500 | same | converged by #31218 |
| `delete_records` outer catch | V2 / V3 | bare message | n/a | kept as
intended, see below |
Kept as intended: `delete_records`' outer catch stays on
`errorResultWithCode`. It also renders this file's own code-less
sentences (`delete_records could not read the records the query on 'OBJ'
answered`), which `mapDataError`'s unknown-object heuristic would
misread as `404 OBJECT_NOT_FOUND` (the PR #31245 note). Its fault answer
still quotes the faulting row's thrown message, as
`mcp-delete-records.test.ts` pins on purpose. Each divergence above is
listed for the seat to file, one card per face.
## Verification
Base: `bdefd49f` (`main`, with PR #31245 merged); head: `a8855476`.
- `pnpm --filter @objectstack/mcp test`: `Test Files 60 passed (60)`,
`Tests 806 passed (806)`, lock `VERDICT command-exit 0`.
- `pnpm --filter @objectstack/mcp typecheck`: exit 0 (includes
`check:test-typecheck`: 53 errors / 8 pinned signatures held).
- `pnpm exec eslint --no-inline-config` on the four touched `.ts` files:
exit 0 (`@objectstack/mcp` has no `lint` script).
<details><summary>Derived gates (node scripts/pm/dispatch-gates.mjs
--commands, 60 commands, all on a885547)</summary>
- `node scripts/check-adr-0087-registration.mjs --base origin/main` ::
exit 0
- `node scripts/check-adr-0087-registration.mjs --self-test` :: exit 0
- `node scripts/check-changeset-no-major.mjs --base origin/main` :: exit
0
- `node scripts/check-closing-keyword-parity.mjs` :: exit 0
- `node scripts/check-closing-keyword-parity.mjs --self-test` :: exit 0
- `node scripts/check-comment-mask-adoption.mjs` :: exit 0
- `node scripts/check-comment-mask-adoption.mjs --self-test` :: exit 0
- `node scripts/check-comment-mask-corpus.mjs` :: exit 0
- `node scripts/check-dts-emitted.mjs --self-test` :: exit 0
- `node scripts/check-empty-changeset.mjs --base origin/main` :: exit 0
- `node scripts/check-empty-changeset.mjs --self-test` :: exit 0
- `node scripts/check-issue-citations.mjs` :: exit 0
- `node scripts/check-plugin-teardown-shape.mjs` :: exit 0
- `node scripts/check-plugin-teardown-shape.mjs --self-test` :: exit 0
- `node scripts/ci/assert-turbo-remote-cache.mjs --self-test` :: exit 0
- `node scripts/ci/plan-test-core-shards.mjs --self-test` :: exit 0
- `node scripts/ci/prune-turbo-cache.mjs --self-test` :: exit 0
- `node scripts/ci/snapshot-next.mjs --self-test` :: exit 0
- `node scripts/docs-audit/check-affected-docs.mjs` :: exit 0
- `node scripts/docs-audit/check-drift-comment.mjs` :: exit 0
- `node scripts/pm/release-rehearsal-clone.mjs --self-test` :: exit 0
- `node scripts/release-pending-publish.mjs --self-test` :: exit 0
- `pnpm --filter @objectstack/spec run check:duration-unit-keys` :: exit
0
- `pnpm check:changeset-gate-self-tests` :: exit 0
- `pnpm check:ci-filter-parity` :: exit 0
- `pnpm check:closed-ceremony-text` :: exit 0
- `pnpm check:cross-package-test-inputs` :: exit 0
- `pnpm check:dispatcher-error-vocabulary` :: exit 0
- `pnpm check:doc-authoring` :: exit 0
- `pnpm check:driver-memory-census` :: exit 0
- `pnpm check:dts-closure` :: exit 0
- `pnpm check:dual-build-cjs-loads` :: exit 0
- `pnpm check:engine-double-contract` :: exit 0
- `pnpm check:error-status-conformance` :: exit 0
- `pnpm check:gitlink-declared` :: exit 0
- `pnpm check:issue-citations` :: exit 0
- `pnpm check:keyed-text-bounds` :: exit 0
- `pnpm check:lean-entry-closure` :: exit 0
- `pnpm check:logger-receiver-detach` :: exit 0
- `pnpm check:nul-bytes` :: exit 0
- `pnpm check:objectql-double-limit` :: exit 0
- `pnpm check:org-identifier` :: exit 0
- `pnpm check:page-declaration-shape` :: exit 0
- `pnpm check:platform-object-tenancy-census` :: exit 0
- `pnpm check:published-files` :: exit 0
- `pnpm check:query-options-erasure` :: exit 0
- `pnpm check:refd-timer-probe` :: exit 0
- `pnpm check:registry-log-declared` :: exit 0
- `pnpm check:rest-log-spy-declared` :: exit 0
- `pnpm check:slot-lookup` :: exit 0
- `pnpm check:sourcemap-no-sources-content` :: exit 0
- `pnpm check:system-context-census` :: exit 0
- `pnpm check:test-source-alias` :: exit 0
- `pnpm check:tier-file-adoption` :: exit 0
- `pnpm check:type-check-coverage` :: exit 0
- `pnpm check:type-check-debt` :: exit 0
- `pnpm check:undeclared-dep-imports` :: exit 0
- `pnpm check:vouch-for-reference-callers` :: exit 0
- `pnpm check:watch-hint-literal` :: exit 0
- `pnpm check:where-matcher` :: exit 0
`node scripts/pm/dispatch-gates.mjs --ran`: `60 derived famil(ies)
accounted for — 60 run, 0 NOT-MEASURED`. Two gates first exited 3 on a
prerequisite and were re-run after it was met:
`check-plugin-teardown-shape --self-test` (shallow clone; fetched its
pinned fixture commit) and `check:dual-build-cjs-loads` (needed every
package's dist, built by `check:type-check-debt`). Both then exited 0.
Not run locally (CI's): `test:repo`, the dogfood shards, the type-check
lanes.
</details>
## Human Minimum
- [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic.
(Not touched.)
- [x] 2. Every name I added uses the casing for its kind.
- [x] 3. No formatter ran; `pnpm lint` passes. (eslint on the touched
files.)
- [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: its generated artifacts are
regenerated and committed. (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.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01FEkQ5oWCPWH6i2Lsts9cdP
Co-authored-by: Claude <noreply@anthropic.com>1 parent 2f369cd commit b3bd75d
5 files changed
Lines changed: 137 additions & 3 deletions
File tree
- .changeset
- packages/mcp/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 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
424 | 424 | | |
425 | 425 | | |
426 | 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 | + | |
| 489 | + | |
| 490 | + | |
| 491 | + | |
427 | 492 | | |
428 | 493 | | |
429 | 494 | | |
| |||
476 | 541 | | |
477 | 542 | | |
478 | 543 | | |
| 544 | + | |
| 545 | + | |
| 546 | + | |
| 547 | + | |
| 548 | + | |
| 549 | + | |
| 550 | + | |
| 551 | + | |
| 552 | + | |
| 553 | + | |
| 554 | + | |
| 555 | + | |
479 | 556 | | |
480 | 557 | | |
481 | 558 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
653 | 653 | | |
654 | 654 | | |
655 | 655 | | |
| 656 | + | |
| 657 | + | |
| 658 | + | |
| 659 | + | |
| 660 | + | |
| 661 | + | |
| 662 | + | |
| 663 | + | |
| 664 | + | |
| 665 | + | |
| 666 | + | |
| 667 | + | |
| 668 | + | |
| 669 | + | |
| 670 | + | |
| 671 | + | |
| 672 | + | |
| 673 | + | |
| 674 | + | |
| 675 | + | |
| 676 | + | |
| 677 | + | |
| 678 | + | |
| 679 | + | |
| 680 | + | |
| 681 | + | |
| 682 | + | |
656 | 683 | | |
657 | 684 | | |
658 | 685 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
50 | 50 | | |
51 | 51 | | |
52 | 52 | | |
53 | | - | |
| 53 | + | |
54 | 54 | | |
55 | 55 | | |
56 | 56 | | |
| |||
1118 | 1118 | | |
1119 | 1119 | | |
1120 | 1120 | | |
| 1121 | + | |
| 1122 | + | |
| 1123 | + | |
| 1124 | + | |
| 1125 | + | |
| 1126 | + | |
| 1127 | + | |
| 1128 | + | |
| 1129 | + | |
| 1130 | + | |
| 1131 | + | |
| 1132 | + | |
| 1133 | + | |
1121 | 1134 | | |
1122 | 1135 | | |
1123 | 1136 | | |
1124 | 1137 | | |
1125 | 1138 | | |
1126 | 1139 | | |
1127 | 1140 | | |
1128 | | - | |
| 1141 | + | |
1129 | 1142 | | |
1130 | 1143 | | |
1131 | 1144 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1567 | 1567 | | |
1568 | 1568 | | |
1569 | 1569 | | |
| 1570 | + | |
| 1571 | + | |
| 1572 | + | |
1570 | 1573 | | |
1571 | 1574 | | |
1572 | 1575 | | |
| |||
1575 | 1578 | | |
1576 | 1579 | | |
1577 | 1580 | | |
1578 | | - | |
| 1581 | + | |
1579 | 1582 | | |
1580 | 1583 | | |
1581 | 1584 | | |
| |||
0 commit comments