Repository navigation
Commit 9b2953d
fix(runtime)!: PATCH /packages/:id refuses a read-only package with 422 WRITABLE_PACKAGE_REQUIRED (#24719)
Fixes #24705
Clause-②: no (narrowing)
## What changed
`PATCH /api/v1/packages/:id` now asks `requireWritablePackage` (that is,
`isWritablePackage`, the ADR-0070 D2 read-only rule) for the path's id.
It asks after `requireManageMetadata` and the walled refusal, and before
the body is read, in the order install-overwrite, disable, publish,
revert and delete already use (manage, then walled, then writable). A
manifest edit of a package the deployment ships is refused `422
WRITABLE_PACKAGE_REQUIRED`: booted by code, installed from the
marketplace, or a stored row whose manifest `scope` is `system` /
`cloud`. The rule is not re-derived at this door.
- `packages/runtime/src/domains/packages.ts`: the gate, one call plus
its comment, on the `PATCH /packages/:id` arm only. Verb `edit`, so the
refusal reads "Cannot edit package 'ID': it is read-only (provided by
code or an installed app)...".
-
`packages/runtime/src/domains/packages-manifest-patch-readonly.test.ts`:
the pins, 38 cases on both limbs (the `protocol` service's
`updatePackage`, and the no-protocol registry fallback).
- `.changeset/24705-patch-manifest-readonly-package.md`:
`@objectstack/runtime` `minor` under the launch-window convention for a
breaking narrowing, with FROM / TO for an integrator, the `Clause-②`
line, and the ADR-0087 disposition `not-required
(no-migration-prescription)`.
A `CONTRACT_REVIEW_TIER` review is owed before the queue (narrowing);
the seat runs it.
## Answers, path by path
| Request | Before | After |
|---|---|---|
| `PATCH` on a read-only id, with `name` / `description` / `version`
(flat, or under `manifest`) | `200`, manifest edited in the registry
(and `sys_packages` written on the protocol limb) | `422
WRITABLE_PACKAGE_REQUIRED`, nothing written |
| `PATCH` on a read-only id, with a body the arm itself refuses (`{}`, a
non-SemVer `version`, a `dependencies`-only body on today's `main`) |
`400` | `422 WRITABLE_PACKAGE_REQUIRED`: the gate precedes the body |
| `PATCH` on a package an organization owns | `200` / `400` per the body
| unchanged |
| `PATCH` on an id nothing installed | `404` | `404`, unchanged |
## Serial with PR #24703
PR #24703 (#24670) edits the body handling of this same arm a few lines
below the gate, adding `dependencies`. This PR is built on `origin/main`
without it, does not touch the body handling, and lands first; PR #24703
then merges `main`. Because the gate is asked before the body is read,
the `{ dependencies: {} }` refusal rows hold on both trees: on today's
`main` that body answered `400` before this gate, and with PR #24703 it
would answer `200`; both become `422` for a read-only id.
## Measured
**Reproduction at base `1c1144fcd`.** The new pin file, committed ahead
of the fix (`ade20c179`), against the unchanged arm: `Tests 32 failed |
6 passed (38)`. Every refusal row is red, with `expected 200 to be 422`
for the editable bodies and `expected 400 to be 422` for the `{
dependencies }` and `{}` bodies. Every CONTROL row is green.
**At head `5697a4d22`:** the new pin file run together with the seven
suites that exercise the `/packages` arms (`packages-readonly-gate`,
`packages-install-overwrite-readonly`, `packages-capability-gate`,
`packages-walled-write-refusal`, `error-passthrough`,
`packages-single-door`, `http-dispatcher`): `Test Files 8 passed (8)`,
`Tests 459 passed (459)`.
**Ablation**, on the committed fix (`5697a4d22`).
`scripts/ablation-replace.mjs` replaced the gate call
`requireWritablePackage(deps, qlService, id, 'edit')` with `null` plus a
marker: anchor 1 to 0, marker 0 to 1, blob `d00fe003b219` to
`90eadd49957b`. The test reaches `packages.ts` through
`http-dispatcher.ts`'s relative import, so `src` is what runs and no
rebuild was needed. Result: `Tests 32 failed | 6 passed (38)`. The 32
refusal rows went red; the 6 CONTROL rows stayed green. Restore: blob
equals HEAD `d00fe003b219`, `git diff HEAD` is empty, and `git status
--porcelain` is clean.
## The dispatch's mechanism assumptions
1. **Order:** confirmed. The arm called `requireManageMetadata` and
`refuseWalledCatalogPackageWrite` and nothing else. Disable (`:1602`),
publish (`:1629`) and delete (`:2309`) ask manage, then walled, then
`requireWritablePackage`; delete reads its `keepData` arity between the
second and third, and revert (`:2114`) asks the gate after the
protocol's stored-row answer. This arm uses manage, walled, writable.
2. **Unknown id:** `isWritablePackage` answers writable for a non-empty
id that no booted manifest and no registry row name, so the gate falls
through and the arm's own `404` stands. Pinned on both limbs: the
fallback's `registry.updatePackageManifest` answers `undefined`, and the
protocol's `updatePackage` throws its `404`. The predicate's empty-id
branch (not writable) is unreachable here, because the domain splits the
path with `filter(Boolean)`.
3. **Precedent:** followed `62f959dd6` (PR #24055): the same changeset
form, ADR-0087 marker and pin shape (a real `ObjectQL` engine,
`registerApp` for the code and marketplace kinds, a stored row for the
scope-only kinds, both limbs, spies on every write, CONTROL rows on the
same spies).
4. **Who sends this PATCH:**
- objectui at the `.objectui-sha` pin `12dc7b47c` (read with `git show`
at that commit, nothing checked out): the one caller is
`PackageFormDialog` in edit mode, opened by `EditPackageDialog` from the
Packages page's detail sheet
(`packages/app-shell/src/views/metadata-admin/PackagesPage.tsx`). It
always sends `{ name, description, version }`.
- Which packages the dialog offers to edit: the sheet shows the Edit
button, beside Disable, Publish, Revert and Delete, whenever `isKernel`
is false, and `isKernel` is `manifest.scope === 'system' ||
manifest.scope === 'cloud'`. So a `system` / `cloud` package is never
offered Edit. A code-booted or marketplace-installed package whose
manifest `scope` is `project` or absent is offered Edit; that is the
case that now answers `422`, as Disable, Publish, Revert and Delete on
the same sheet already do for it. The dialog's fall-through error arm
renders the envelope's message, so the person reads the refusal sentence
(static read; not measured in a browser).
- `packages/client`: `packages.update(id, patch)` sends this PATCH; only
its own tests call it. The CLI has no caller.
- No test or dogfood case in this repo asserts a `200` PATCH on a
read-only package (sweep below), so no pin flips.
## Full-repo pin sweep
Searched for every test, dogfood and conformance case that sends `PATCH`
to `/packages/:id` (not `/disable` or `/enable`), and for
`WRITABLE_PACKAGE_REQUIRED` across the repo. Each hit names a writable
or unknown package, so none flips:
- `http-dispatcher.test.ts` (5 cases), `error-passthrough.test.ts`,
`packages-capability-gate.test.ts`: engine doubles with no `manifests`
and no `scope`, so writable.
- `dispatcher-body-after-anonymous-floor.hono.integration.test.ts`
(`pkg_x`): unknown, so writable; it still reaches the body.
- `packages-walled-write-refusal.test.ts` and
`wall-environment-metadata-refusal.dogfood.test.ts`: the walled `403`
precedes the gate; the CONTROL operator edit there targets a package the
same test created through `POST /packages` (authored).
- `docs/qa/platform-checklist/areas/api-backend.json`: the manifest-edit
step uses a scratch package the item installs itself.
## Tests and gates (local, at `5697a4d22`)
- `pnpm --filter '@objectstack/runtime^...' build` (the dependency
closure; this diff changes no package in it): verify-lock `VERDICT
command-exit 0`.
- `pnpm --filter @objectstack/runtime typecheck`: `VERDICT command-exit
0`, with `check:test-typecheck: OK`. The new test file is in the
`tsconfig.test.json` program (`--listFiles` count 1) and carries no
ledger entry, so it has no tsc error.
- `pnpm --filter @objectstack/runtime exec vitest run --project local
--maxWorkers=2`: `Test Files 427 passed (427)`, `Tests 6759 passed | 19
skipped (6778)`, `VERDICT command-exit 0`.
- `pnpm lint` (repo-wide `eslint . --no-inline-config`): exit 0.
- `node scripts/pm/dispatch-gates.mjs --commands` (66 families, from
merge base `1c1144fcd`): 66 exit 0. `check:dual-build-cjs-loads` first
exited 3 (`PREREQUISITE NOT MET`: not every package had a `dist/`);
re-run after `check:type-check-debt` built every package: exit 0, 111
require entry points across 69 packages load. `--ran` reconciliation: 66
derived, 66 run, 0 NOT-MEASURED, 0 UNRUN.
- `check-adr-0087-registration --base origin/main`: exit 0, one
declared-breaking changeset carrying `not-required
(no-migration-prescription)`. `check-changeset-no-major --base
origin/main --event` with this body as the payload: exit 0, level axis
`no (narrowing)` with the one moved package graded `minor`.
- `test:repo` was not run locally: the new test reads nothing outside
its package (`check:cross-package-test-inputs` exit 0). CI owns it.
`Lint & Repo Gates`, `TypeScript Type Check`, `Test Core` and `Build
Core`: CI on this PR.
- `origin/main` was not merged before opening. Main has gained six
commits since the base (`73d4cc739` to `54159fd74`). None touches
`packages/runtime`, `package-writability.ts`, `protocol.updatePackage`
or the registry's package methods; the `objectql` and
`metadata-protocol` changes are in hooks, validation and the seed
loader. CI runs on the merge ref.
## Acceptance notes
- **Observation, not filed (a static read; not reproduced in a
browser):** objectui's Packages detail sheet decides which action
buttons to show from `manifest.scope` (`isKernel`), while its Scope
badge reads the served `writable` verdict. So a code-booted or
marketplace package whose `scope` is `project` or absent is offered
Edit, Disable, Publish, Revert and Delete, and each of those now answers
`422`. Read at the pin `12dc7b47c`,
`packages/app-shell/src/views/metadata-admin/PackagesPage.tsx`.
- **Doc surfaces left as they are (outside the claimed file surface):**
`requireWritablePackage`'s doc block in `packages.ts` lists the doors
that ask it and does not name this one; the arm's own comment does. The
JSDoc of `packages/client`'s `packages.update` does not mention the
read-only refusal.
- **Carried from the card, still NOT measured:** what a code package's
stored `sys_packages` row does on the next boot after an edit. This PR
stops new edits; rows an earlier edit wrote are not touched.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01LmjU2x1faLZ39DVTFyVca6)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 4bc3bec commit 9b2953d
3 files changed
Lines changed: 256 additions & 0 deletions
File tree
- .changeset
- packages/runtime/src/domains
| 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 | + | |
| 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 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2255 | 2255 | | |
2256 | 2256 | | |
2257 | 2257 | | |
| 2258 | + | |
| 2259 | + | |
| 2260 | + | |
| 2261 | + | |
| 2262 | + | |
| 2263 | + | |
| 2264 | + | |
| 2265 | + | |
| 2266 | + | |
2258 | 2267 | | |
2259 | 2268 | | |
2260 | 2269 | | |
| |||
0 commit comments