Skip to content

fix(metadata-protocol): the metadata write refusal reports the package door — ITEM_LOCKED / WRITABLE_PACKAGE_REQUIRED are emitted where they apply - #8185

Merged
huangyiirene merged 7 commits into
mainfrom
claude/issue-7682-package-writability-refusal
Aug 13, 2026
Merged

fix(metadata-protocol): the metadata write refusal reports the package door — ITEM_LOCKED / WRITABLE_PACKAGE_REQUIRED are emitted where they apply#8185
huangyiirene merged 7 commits into
mainfrom
claude/issue-7682-package-writability-refusal

Conversation

@huangyiirene

@huangyiirenehuangyiirene commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Part of #7682

Restores an invariant the error-code ledger already declared: the metadata write refusal now reports the package door when the caller named a read-only base, instead of always blaming the metadata type's registry flags.

The defect

PUT /api/v1/meta/object/showcase_task answered 403 NOT_OVERRIDABLE ("'object' is not allowOrgOverride in the registry") — the same code, status and sentence whether ?package= pointed at a read-only package or a writable one. The refusal discriminated on the metadata type's registry flags and never read the base the caller named, so the two codes the ledger registers to @objectstack/metadata-protocol for the package-writability condition — ITEM_LOCKED and WRITABLE_PACKAGE_REQUIRED — were never emitted on this path at all. Declared, not enforced.

Premise verification

Both of the dispatch's mechanism assumptions hold, and one deserves stating because it decides where the fix belongs:

  • The discriminator is live, now at sys-metadata-repository.tsassertAllowed (intent === 'runtime-only' ? 'NOT_CREATABLE' : 'NOT_OVERRIDABLE') — re-located by reading, as instructed.
  • The writability fact is reachable at that call site, with no new plumbing: PutOptions.packageId is the base ?package= resolves to, this.engine is the same engine the protocol hands the repository, and isWritablePackage (packages/metadata-protocol/src/package-writability.ts) already reads exactly those two surfaces structurally. It is imported, never re-spelled — that shared-rule argument is the module's own stated reason to exist.
  • The repository really is the emitter the QA run measured. saveMetaItem's own artifact-backed refusal sits behind environmentId !== undefined, and the showcase is assembled by the CLI's lightweight host-config path (new ObjectQLPlugin(), no environmentId). meta-object-owd-gate.test.ts states the same reading independently ("SysMetadataRepository.assertAllowed() refuses an object overlay of a PACKAGED item outright"), and the end-to-end block in the new suite proves it by driving saveMetaItem on that topology.

The fix

A refused write that named a read-only base now says so, and which code depends on intent, because the two ledgered codes are not interchangeable:

  • override-artifact — an artifact backs the name and it ships from a package the deployment provides → 403 ITEM_LOCKED, carrying lockSource: 'package' (ADR-0010's own reserved value for a lock the package layer asserts) and the package id. WRITABLE_PACKAGE_REQUIRED would be a false prescription here: switching bases cannot help, because the artifact is code-shipped wherever the caller points, so an auto-retrying client would simply fail again. This is the server-side counterpart of the "Read-only" badge Studio already renders.
  • runtime-only — no artifact under this name, so the caller is authoring a NEW item into a read-only base → 422 WRITABLE_PACKAGE_REQUIRED with the package id: the same code, status and prescription saveMetaItem already emits for exactly this condition (ADR-0070 D1), now also stated at the single persistence route, for callers that do not pass through that gate.

That partition was PM-ruled (option A) on this PR's patch round, on the false-prescription argument above.

What deliberately does NOT move

No allow decision changes. Every allow limb returns before the new door, so this is the code selection inside the refusal branch, not a new gate. That is load-bearing rather than cautious:

  • an ADR-0005 org overlay names the read-only package it customizes by construction, so a package door that refused would close the overlay model itself — pinned;
  • making it an allow→deny gate would extend ADR-0070 D1 — measured on saveMetaItem creates — to promoteDraft / restoreVersion / revertCommit, which route through put carrying the row's own binding, i.e. it would break republishing and repair of legacy package-bound rows on surfaces nobody measured.

Also unchanged: a write naming no base keeps NOT_OVERRIDABLE / NOT_CREATABLE verbatim (isWritablePackage(null) is false by design — "no base resolved" — so reading it without the caller-named guard would re-code every ordinary env-local refusal in the product), and the DELETE verb is untouched (DeleteOptions names no package; #6960 moved that side on purpose and warns against symmetrising).

OS_METADATA_WRITABLE is deliberately uncovered here (#8146)

The hatch path is untouched by this change — structurally, its limb returns before the new door — and this PR ships no test of it, which the suite docblock records so the gap reads as a decision rather than an oversight.

An earlier revision of this branch carried a labelled characterization pin of today's behaviour. It was removed on PM ruling, and the reasoning is worth keeping: however well labelled, that case was green because the bug is present, and the 2026-08-12 ruling on #8146 (option B — the server should refuse; the badge is telling the truth) said plainly not to merge it. #8146 already names "the refusal plus a rejection pin asserting code and status" as its own deliverable, so the pin gets written against the fixed behaviour, which is where it belongs.

New evidence carried to that decision, re-measured on current main at the ruling's request (the original measurement was against two-week-old builds). The hatch write still succeeds end to end through saveMetaItem on the host-config topology:

{ "success": true, "seq": 1, "state": "active",
"message": "Saved customization overlay (env-wide, state=active) — type=permission, name=showcase_contributor" }

and the row lands { package_id: "com.example.showcase", organization_id: null } — i.e. bound into the read-only package, not the per-org override the variable's own documentation describes ("treats them as allowOrgOverride: true", a type-level unlock). Which of the two readings of option B follows from that is the maintainer's call on #8146; nothing here pre-empts it.

Tests

New: packages/metadata-protocol/src/sys-metadata-repository.package-writability.test.ts (9 cases). Every refusal case pins bothcode and status; the read-only/writable pair is asserted as a difference, so a suite that started answering the new codes on both sides would go red.

Reverse verification, direction predicted before running: reverting the source and keeping the suite turned the discrimination cases red while leaving the preservation cases (the ADR-0005 overlay landing, the no-base codes, the delete codes) green — measured exactly so. The green ones are the point: they do not depend on this change, which is what makes them honest pins rather than artifacts of it.

metadata-protocol 76 files / 1109 tests passed (after merging origin/main)
objectql 191 files / 3390 tests passed
rest 102 files / 1728 tests passed
runtime 145 files / 2195 tests passed

Consumer sweep direction, stated so it can be reviewed: those three downstream packages were selected by hand as the ones that exercise this refusal (the full ...@objectstack/metadata-protocol prefix set is effectively the whole repo); the ^... suffix filter was used only for building dependency closures. Two of the three initially failed to collect on unbuilt sibling dists in a fresh worktree — the stale-artefact trap, not this change — and are green after their closures were built.

Gates: check:cross-package-test-inputs, check:durability-log-level (both named by scripts/pm/dispatch-gates.mjs for this diff), plus check:error-code-casing (new error codes), check:nul-bytes + a widened control-byte self-scan, the spec error-code-ledger test, ESLint on both files, and the changeset gates. All green. Both codes were already registered to this package in the ledger, so nothing in packages/spec moves.

What this does not close

Hence Part of #7682 rather than Fixes — PM-ruled on the same patch round.


Generated by Claude Code

…ot only the type's flags (#7682)
`PUT /api/v1/meta/object/showcase_task` answered `403 NOT_OVERRIDABLE`
identically whether `?package=` named a read-only or a writable package:
`SysMetadataRepository.assertAllowed` discriminated on the metadata TYPE's
registry flags and never read the base the caller named. `ITEM_LOCKED` and
`WRITABLE_PACKAGE_REQUIRED` are both registered to this package in the
error-code ledger and neither was ever emitted on this path.
The refusal branch now reads the named base through the shared
`isWritablePackage` predicate (imported from `package-writability.ts`, the same
one `saveMetaItem`'s ADR-0070 D1 gate and the `/packages` lifecycle gate use):
* `override-artifact` into a read-only base -> 403 ITEM_LOCKED, carrying
ADR-0010's reserved `lockSource: 'package'` and the package id. "Pick a
writable base" would be a false prescription here, so the code names the
lock instead.
* `runtime-only` into a read-only base -> 422 WRITABLE_PACKAGE_REQUIRED, the
same code, status and prescription `saveMetaItem` already emits for exactly
this condition, now also stated at the single persistence route.
No ALLOW decision moves: every allow limb returns before this point, so an
ADR-0005 overlay (which names the read-only package it customizes by
construction) and the documented `OS_METADATA_WRITABLE` hatch behave exactly as
before -- both pinned, the hatch because #8146 must be free to move it
deliberately. A write naming no base keeps the previous codes verbatim, and the
DELETE verb is untouched (#6960).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AmES43BMDg4bPrxTdi5q7t
@vercel

vercelBot commented Aug 12, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 13, 2026 1:55am

Request Review

@github-actionsgithub-actionsBot added size/l documentation Improvements or additions to documentation tests tooling labels Aug 12, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/metadata-protocol.

3 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/concepts/metadata-lifecycle.mdx(via @objectstack/metadata-protocol)
  • content/docs/kernel/services-checklist.mdx(via @objectstack/metadata-protocol)
  • content/docs/protocol/kernel/http-protocol.mdx(via @objectstack/metadata-protocol)

1 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/v9.mdx(via @objectstack/metadata-protocol)

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

…TED, per the 2026-08-12 #8146 ruling
An in-flight maintainer ruling landed on #7682 after this work started: #8146 is
settled as option B — a hatch write into a read-only package should REFUSE, the
Studio badge is telling the truth, and the hatch is type-level by its own
shipped documentation. The dispatch's instruction to "preserve and pin" that
behaviour is therefore superseded, and the ruling says explicitly not to land a
pin asserting it as correct.
No behaviour change here. The hatch limb was never touched by this PR (it
returns before the new package door), so nothing to revert; what changes is the
CLAIM the suite and the changeset were making about it. The case is relabelled
as a characterization pin of today's answer and kept, deliberately, as the
tripwire the #8146 fix must invert.
Re-measured on current main at that ruling's request, since the original
measurement was against two-week-old builds: it still reproduces end to end
through saveMetaItem on the host-config topology — `success: true`, and the row
lands with `package_id = com.example.showcase`, i.e. bound INTO the read-only
package rather than as the per-org override the variable's documentation
describes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AmES43BMDg4bPrxTdi5q7t
…he gap as deliberate (#7682)
PM ruling on PR #8185's patch round. The case asserted that a hatch write into
a read-only package SUCCEEDS — green because the bug is present — and the
2026-08-12 ruling on #8146 said "don't merge it". A better label does not change
what the assertion claims, so it goes.
My tripwire argument for keeping it is answered: #8146 already names "the
refusal plus a rejection pin asserting code and status" as its own deliverable,
so the pin gets written against the FIXED behaviour, which is where it belongs.
The opposite risk is real too — a deliberately-red pin gets "repaired" to green
by someone who never read why it was there.
The suite docblock now states that the hatch path is deliberately uncovered
pending #8146, so the absence reads as a decision rather than an oversight, and
carries the re-measurement: the row lands bound INTO the read-only package
(package_id = com.example.showcase, organization_id = null) rather than as the
per-org override the variable's own documentation describes. The changeset says
the same.
No source change: `assertAllowed` is correct as shipped, and both open questions
were ruled in its favour (the ITEM_LOCKED / WRITABLE_PACKAGE_REQUIRED partition,
and `Part of #7682`). The `permission` case is kept and moved beside its
siblings — with no hatch set it asserts the refusal, which is the card's point.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AmES43BMDg4bPrxTdi5q7t
…suite docblock (#7682)
Formatting only, no assertion or source change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AmES43BMDg4bPrxTdi5q7t
…e'] (#7682)
CI's TypeScript Type Check went red on my push: the metadata-protocol DEBT
entry records 63 raw tsc errors and `tsc --noEmit` reported 64. The +1 was mine
and real:
sys-metadata-repository.package-writability.test.ts(166,21): error TS2322:
Type 'string' is not assignable to type '"object" | "view" | ... |
"email_template"'.
`putWith`'s `opts.type` was `string`; `MetaRef['type']` is a literal union. The
suite ran green under vitest either way — this package has no `typecheck`
script, so its type surface is only judged by the ledger in CI, which is
exactly the gap that let a widened parameter through a local run.
Fixed at the source, not in the ledger: DEBT is a ratchet that may only shrink,
and raising an entry for a file added in the same PR would be the worst version
of that. `tsc --noEmit` now reports 63 for the package, matching the recorded
entry, with zero errors attributable to either file this PR touches.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AmES43BMDg4bPrxTdi5q7t
@huangyiirene
huangyiirene marked this pull request as ready for review August 13, 2026 02:14
@huangyiirene
huangyiirene added this pull request to the merge queueAug 13, 2026
Merged via the queue into main with commit ebf7d98Aug 13, 2026
26 checks passed
@huangyiirene
huangyiirene deleted the claude/issue-7682-package-writability-refusal branch August 13, 2026 02:27
os-zhuang pushed a commit that referenced this pull request Aug 13, 2026
…d' comment
#8185 wrote it while the OS_METADATA_WRITABLE gap was still open; this
change closes that gap, so the comment now contradicts the suite docblock.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012WMpuAfA2KSdDjGF6tm1bH
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@huangyiirene@claude