Repository navigation
Commit 6d5191e
Fixes #22741
Clause-②: yes (widening)
## What
An import row for a fault the QuickJS sandbox raises itself now reads
the create door's fault sentence instead of the runner's diagnostic
text. These faults are the CPU budget, the wall-clock ceiling, a denied
capability, an unavailable `ctx.api`, a failed install or marshal, and a
nested body's crash relayed across a VM hop. `POST /api/v1/data/:object`
and `/createMany` answer them `500 INTERNAL_ERROR` `Internal server
error`. The sync `/import` row and the `/import/jobs` stored row relayed
`.message` instead, for example `hook 'mz_lock_insert' exceeded CPU
budget of 50ms (after 0 pump iterations)`.
## Landing point, and why no runner marker
- **Predicate:** `isSandboxOwnFault(error)` in
`packages/types/src/data-error-classification.ts`, beside
`isSandboxOrigin`. It answers `true` when `name === 'SandboxError'` and
`isSandboxOrigin` is false (no non-empty `innerMessage`). Both halves
are read off the producer. `quickjs-runner.ts` sets `this.name =
'SandboxError'`, and its `innerMessage` contract says the field is
undefined for "the sandbox's own internal errors (capability denials,
timeouts, marshalling failures)". The producer already marks its own
faults, so the runner needs no new marker and `packages/runtime` is
untouched.
- **Why the class name and not `instanceof`:** `@objectstack/runtime`
depends on `@objectstack/core` (`packages/runtime/package.json`), so
core cannot import the class. `isEngineDuplicateRecordEnvelope` in the
same file recognises a class by its declared contract for the same
reason, and `objectql`'s `hook-withheld-readonly-fault.ts` already reads
`'SandboxError'` by name.
- **Row:** `toFailedResult` (`packages/core/src/utils/import-runner.ts`)
now computes `sandboxed = isSandboxOrigin(err) ||
isSandboxOwnFault(err)`, and takes the door's sentence when `sandboxed
&& door.status >= 500`. That is the rule PR #22740 applies to a crash. ⛔
There is no second withholding rule: `mapDataError` still decides which
errors are faults and what their sentence is. The row's `code` stays
`IMPORT_ROW_FAILED`.
- ⛔ Still not "every door 5xx": a driver error is neither a sandbox
origin nor a sandbox fault, and its row keeps `sanitizeRowError` text.
## Measured before the fix
This ran through the real `QuickJSScriptRunner` at `eed637c09f`, feeding
the real `mapDataError` and `runImport` (a scratch test, not committed).
Import object `mz_locked`:
| fault (real runner) | `name` / `innerMessage` | create door | row
before |
|---|---|---|---|
| CPU budget (`for(;;){}`, 50ms) | `SandboxError` / undefined | 500
`Internal server error` | `hook 'mz_lock_insert' exceeded CPU budget of
50ms (after 0 pump iterations)` |
| wall-clock ceiling (never-settling host call) | `SandboxError` /
undefined | 500 `Internal server error` | `hook 'mz_lock_insert'
exceeded wall-clock ceiling of 300ms while awaiting host calls (after 40
pump iterations)` |
| `api.read` denied on another object | `SandboxError` / undefined | 500
`Internal server error` | `capability 'api.read' not granted to hook
'mz_lock_insert' (called ctx.api.object('mz_other').find)` |
| `log` denied | `SandboxError` / undefined | 500 `Internal server
error` | `capability 'log' not granted to hook 'mz_lock_insert'` |
| missing `ctx.api` method | `SandboxError` / undefined | 500 `Internal
server error` | `ctx.api.object('mz_other').find not implemented` |
| `api.read` denied on the import's own object | `SandboxError` /
undefined | **404 `OBJECT_NOT_FOUND`** `Object 'mz_locked' is not
registered` | the capability text |
The last row is a separate door defect (see Acceptance notes). The row
does not copy a 4xx sentence, so it keeps the capability text there.
## The enumeration pin
`packages/core/src/utils/import-runner-sandbox-fault-row.test.ts` sits
in core's `repo` vitest project and is listed in
`vitest.repo-tests.json`, because it reads
`packages/runtime/src/sandbox/quickjs-runner.ts`. Core's declared radius
`packages/**/*.ts` already covers that read. The test parses the runner
with the TypeScript compiler, so the two `throw new SandboxError(`
examples inside doc comments are not sites.
- **§1, 15 fault sites:** every `new SandboxError(…)` with no
`innerMessage` (`:235`, `:240`, `:660`, `:891`, `:917`, `:1025`,
`:1104`, `:1199`, `:1553`) and every `throwSandboxFault(…)` call
(`:634`, `:730`, `:756`, `:803`, `:1173`, `:1180`). Each message is
rendered from its own template literal with a fixed sample per
interpolation. Each case first asserts the door's verdict (`500`, `{
error: 'Internal server error', code: 'INTERNAL_ERROR' }`, the text
withheld) as anti-vacuity. It then asserts `isSandboxOwnFault` and that
the row equals `{ row: 1, ok: false, action: 'failed', error:
DOOR_SENTENCE, code: 'IMPORT_ROW_FAILED' }`. The batched
`insertManyData` path is driven once for the budget fault.
- **The census:** the two non-literal single-argument constructions
(`message` in `throwSandboxFault`, and
`withoutSandboxErrorPrefix(String(errStr))` in the pump loop) must be
exactly the declared relays. A site the pin cannot render, such as a new
interpolation with no sample, is red and the failure names it. Each
named kind keeps its measured floor: CPU budget 1, wall-clock 1,
capability denial 5, `ctx.api` unavailable 1, install 4, marshal 1,
nested transaction 1, missing method 1. The class `name` is read from
the producer's constructor, so a rename there reaches every case.
- **§2, the relay half:** the four constructions that do set
`innerMessage` (`:280`, `:349`, `:403`, `:448`) are rendered after a
nested body crashed (`TypeError: boom`). That is the `.message` the
outer pump rebuilds as a fault with no `innerMessage`. The door
withholds it, and the row takes the door's sentence.
- **§3, controls:** the same budget text on a plain `Error` keeps
`sanitizeRowError`'s reading (the predicate reads class identity, never
the message). A code-defined hook's native `TypeError` keeps its text.
Driver text keeps `sanitizeRowError`'s reading although its door answers
5xx. A body's refusal or crash, and non-error values, answer `false`.
`import-runner-sandbox-refusal-row.test.ts` §4, the driver-text control
PR #22740's ablation B pinned, is not edited.
## Ablations
All were run from committed HEAD `2180c575bf` through `node
scripts/ablation-replace.mjs`. In each, the anchor hit once and moved
from 1 to 0, the blob changed, and the restore was proven with the blob
equal to HEAD and `git diff HEAD` empty. The command was `pnpm --filter
@objectstack/core exec vitest run --project repo --maxWorkers=2
src/utils/import-runner-sandbox-fault-row.test.ts`.
| leg | mutation | result |
|---|---|---|
| A | `toFailedResult`: `isSandboxOrigin(err) \|\|
isSandboxOwnFault(err)` becomes `isSandboxOrigin(err)` | **20 failed / 5
passed**: every §1 site, the batched path and all four §2 relays went
red; the census and the 4 controls stayed green |
| B | `isSandboxOwnFault` body becomes `return false && …` | **20 failed
/ 5 passed** |
| C | producer `this.name = 'SandboxError'` becomes `'SandboxFault'` in
`quickjs-runner.ts` | **20 failed / 5 passed**: the cross-package name
contract is pinned |
| D | a new fault site the pin cannot render: `:660` becomes a template
with `${txState.depth}` | **1 failed / 23 passed**: the census, naming
`quickjs-runner.ts:660: no sample for the interpolation txState.depth` |
The test imports `./import-runner` relatively, and core's vitest aliases
`@objectstack/types` to its source, so no `dist/` sits on these paths.
The runner is read from disk.
## Tests
All on HEAD `bb09729a0a` (the branch merged with `origin/main`
`55382dc02a`). The dependency closure was built first with `pnpm
--filter '@objectstack/core...' build` (exit 0).
- `pnpm --filter @objectstack/core exec vitest run --project local
--project repo --maxWorkers=2`: **100 files / 2451 tests passed** (all
100 test files in the package, both projects).
- `pnpm --filter @objectstack/types exec vitest run --project local
--project repo --maxWorkers=2`: **28 files / 765 tests passed**.
- `pnpm --filter @objectstack/types typecheck`: exit 0. `pnpm --filter
@objectstack/core typecheck`: exit 0. The test layer stays at its 4
pinned signatures, and the new test adds none.
- The new file alone: 25/25 passed (the census, 15 §1 sites, the batched
path, 4 §2 relays, 4 controls).
- **Gates:** `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` derived 66 families from this diff, and all
66 exit 0. Two of them, `check:dual-build-cjs-loads` and
`check:lean-entry-closure`, first answered `PREREQUISITE NOT MET` (exit
3, not measured). Both were re-run green on the same HEAD after
`check:type-check-debt`'s full build. `--ran` reconciliation: 66
derived, 66 run, 0 NOT-MEASURED (a derived zero, since every line
carries its exit code). Re-run on `23a4a8a83f` (the changeset commit):
the same 66 families, all exit 0, none exit 3; `--ran` 66 derived, 66
run, 0 NOT-MEASURED.
- **Lint, a declared narrowing:** `pnpm exec eslint --no-inline-config
--format json` on the three changed `.ts` files gives 3 files, 0 errors
and 0 warnings (the count is read from the JSON). The population was
read from ESLint itself: `isPathIgnored` is false and
`calculateConfigForFile` yields rules for all three. This repo never
enables type-aware linting (`parserOptions.project` is null for all
three, and `eslint.config.mjs` says so), so the diff cannot move the
verdict on any untouched file. The repo-wide `pnpm lint` is left to CI.
- **CI:** `in_progress` when this PR opened, and not waited on.
## Acceptance notes
- **A door defect, outside this card:** `mapDataError`'s unknown-object
heuristic, `lower.includes("'OBJECT'") && lower.includes('not')`,
answers `404 OBJECT_NOT_FOUND` `Object 'OBJECT' is not registered` for a
sandbox fault whose text names the requested object. A capability denial
on that object, such as `capability 'api.read' not granted to hook '…'
(called ctx.api.object('OBJECT').find)`, does. It was measured through
the real runner as shown above. The row does not copy that 4xx, so it
keeps the capability text there. If the door is fixed to 500, the row
follows with no change here. This is reported to the seat for filing.
- **Population versus triage's count:** triage named 8 no-`innerMessage`
sites (`:235`, `:240`, `:660`, `:891`, `:917`, `:1025`, `:1104`,
`:1199`). The enumeration also finds the marshal failure (`:1553`), the
six `throwSandboxFault` capability and `ctx.api` sites, and the two
relays (`:432`, `:1524`). The PM's 17 `new SandboxError` lines are 15
constructions plus 2 inside doc comments.
- **Mechanism note:** `isSandboxOrigin` is not module-private. It is in
the file's trailing `export { … }` block, so `@objectstack/types`
already exports it, and core imported it before this PR.
- **Public surface:** `@objectstack/types`' `.` entry gains one export,
`isSandboxOwnFault(error: unknown): boolean`. It was measured on the
built `dist/index.d.ts` (the declaration and the export list) and on
`dist/index.d.mts`. No other name changes, and nothing re-exports
`@objectstack/types` wholesale (`git grep`), so it cannot collide. The
seat settled the line as `Clause-②: yes (widening)`. The changeset
declares it too and takes `minor` for `@objectstack/types`;
`@objectstack/core` stays `patch`.
- **Test wiring:** `packages/core/vitest.repo-tests.json` gains the new
file, which the core `repo` project requires for a test that reads
outside the package.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01ADzJtzYTLUfgrRZHxkagkX)_
---------
Co-authored-by: Claude <noreply@anthropic.com>
1 parent d952358 commit 6d5191e
5 files changed
Lines changed: 419 additions & 3 deletions
File tree
- .changeset
- packages
- core
- src/utils
- types/src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 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 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
| 323 | + | |
| 324 | + | |
| 325 | + | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
| 335 | + | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
| 341 | + | |
| 342 | + | |
| 343 | + | |
| 344 | + | |
| 345 | + | |
| 346 | + | |
| 347 | + | |
| 348 | + | |
| 349 | + | |
| 350 | + | |
| 351 | + | |
| 352 | + | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
0 commit comments