diff --git a/.changeset/metadata-writable-hatch-package-door.md b/.changeset/metadata-writable-hatch-package-door.md new file mode 100644 index 0000000000..d063ee121e --- /dev/null +++ b/.changeset/metadata-writable-hatch-package-door.md @@ -0,0 +1,59 @@ +--- +"@objectstack/metadata-protocol": patch +--- + +fix(metadata-protocol): `OS_METADATA_WRITABLE` no longer unlocks a write into a read-only package (#8146) + +With the documented operator hatch set, `PUT /api/v1/meta/permission/showcase_contributor?package=com.example.showcase` +answered **200** against `com.example.showcase` — a **read-only** package — while +Studio rendered that same permission matrix fully disabled behind a "Read-only" +badge. Two surfaces answered the same question differently, and the row landed +`{ package_id: 'com.example.showcase', organization_id: null }`: bound *into* the +package the deployment ships. + +**Maintainer ruling, 2026-08-12 (option B): the badge is telling the truth and +the server should refuse.** The hatch is a **metadata-type-level** unlock by its +own shipped documentation — `content/docs/deployment/environment-variables.mdx` +defines it as treating named types "as `allowOrgOverride: true` … overridden +per-org", and this package's CHANGELOG records that it "deliberately does not +unlock the org dimension". A type-level unlock says nothing about the **package** +dimension, so the 200 was a bug rather than a policy choice. + +The package door now sits **above** the hatch limb and **below** every registry +limb in `SysMetadataRepository.assertAllowed`. A hatch write that **names** a +read-only base is refused with the codes the error-code ledger already registers +for the package-writability condition: + +- **`override-artifact`** → `403 ITEM_LOCKED`, carrying `lockSource: 'package'` + and the package id — the server-side counterpart of Studio's badge. +- **`runtime-only`** → `422 WRITABLE_PACKAGE_REQUIRED`, the same code and + prescription `saveMetaItem` already emits for ADR-0070 D1. + +**What deliberately keeps working — the hatch is narrowed, not retired.** Only a +write that *names* a read-only base is refused. Verified by measurement before +the change was written, and pinned as tests: + +- a **package-less** hatch write still lands the env-wide overlay + (`{ package_id: null, organization_id: null }`); +- under an org kernel it still lands the **per-org override** the variable's + documentation promises (`{ package_id: null, organization_id: }`); +- a hatch write naming a **writable** base still lands; +- an **ADR-0005 org overlay** of a code-shipped item is untouched — it names the + read-only package it customizes by construction and returns at the registry + limb, above the door. + +No documentation changes and no capability is retired. + +**The refusal no longer prescribes the step the caller already took.** When the +hatch is open, the `ITEM_LOCKED` message states that `OS_METADATA_WRITABLE` +unlocks the type and not package writability, and points at the remedy that +actually works (retry without `?package=`). The previous sentence — "set +`OS_METADATA_WRITABLE=`" — would otherwise be emitted *while that variable +is set*, which is the shape that makes an automated client retry forever. With +the hatch closed, that sentence is still offered, because then it is true. + +**Known boundary (#8184, not fixed here):** on a scoped kernel +(`environmentId !== undefined`) `saveMetaItem` refuses earlier, in `protocol.ts`, +with the undiscriminated `NOT_OVERRIDABLE`, so this refusal is not reachable on +that topology. Not a regression — that branch answered `NOT_OVERRIDABLE` before +this change too. diff --git a/packages/metadata-protocol/src/sys-metadata-repository.package-writability.test.ts b/packages/metadata-protocol/src/sys-metadata-repository.package-writability.test.ts index bacfbad5e6..12f1785449 100644 --- a/packages/metadata-protocol/src/sys-metadata-repository.package-writability.test.ts +++ b/packages/metadata-protocol/src/sys-metadata-repository.package-writability.test.ts @@ -37,24 +37,19 @@ * - **delete is untouched** — #6960 moved the delete side deliberately and * warns against symmetrising; `DeleteOptions` carries no `packageId`. * - * ## ⛔ `OS_METADATA_WRITABLE` is deliberately UNCOVERED here (#8146) + * - **[#8146] the hatch is type-level** — `OS_METADATA_WRITABLE` no longer + * unlocks a write that NAMES a read-only base. See that block's own + * docblock; it carries the ruling and the measurement NARROW rests on. * - * The absence is a decision, not an oversight, so do not "complete" this suite - * by adding a case for it. A hatch write into a read-only package currently - * SUCCEEDS — re-measured on `main` while #7682 was in flight: `success: true`, - * with the row landing at `package_id = com.example.showcase`, i.e. bound INTO - * the read-only package rather than as the per-org override the variable's own - * documentation describes (`content/docs/deployment/environment-variables.mdx` - * — "treats them as `allowOrgOverride: true`", a TYPE-level unlock). The - * maintainer ruling of 2026-08-12 on #8146 holds that this should refuse, so - * any test of it here would be green *because the bug is present*, and that is - * a shape this repo does not merge (PM ruling, PR #8185 patch round). + * ## [#8146] `OS_METADATA_WRITABLE` — from "deliberately uncovered" to pinned * - * #7682 does not touch that path — the hatch limb returns before the new - * package door, exactly as it did before — and #8146 already names its own - * deliverable as "the refusal plus a rejection pin asserting `code` and - * `status`". That pin belongs to #8146, written against the fixed behaviour; - * nothing is lost by this suite staying silent until then. + * An earlier revision of this docblock told the next reader NOT to add a hatch + * case, because on `main` a hatch write into a read-only package SUCCEEDED and + * any test of it would have been green *because the bug was present* (a shape + * this repo does not merge — PM ruling, PR #8185 patch round). That is no + * longer the state of the file: #8146 landed the refusal, so the pin it always + * owed is written below against the FIXED behaviour. The prohibition is + * discharged, not still standing — do not read the history as a live warning. */ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; @@ -89,8 +84,15 @@ function makeFakeEngine() { const rows = new Map(); const historyRows: Row[] = []; - const keyOf = (w: Record) => - `${String(w.type)}|${String(w.name)}|${String(w.organization_id ?? 'null')}|${String(w.state ?? 'active')}`; + // [#8146] Keyed by TABLE as well as identity. `recordMetadataAudit` inserts + // an audit row carrying the same `type`/`name`/`organization_id`, so a + // table-blind key let the audit row overwrite the `sys_metadata` row it + // describes — measured while verifying #8146's premise, where it made a + // correctly-bound row read back as `package_id: null`. Only a test that + // reads the row BACK (the preservation cases below) can see this, which is + // why it survived until a case needed the binding rather than the throw. + const keyOf = (w: Record, table = 'sys_metadata') => + `${table}|${String(w.type)}|${String(w.name)}|${String(w.organization_id ?? 'null')}|${String(w.state ?? 'active')}`; const findRow = (where: Record) => { if (where.id !== undefined) { @@ -130,8 +132,8 @@ function makeFakeEngine() { historyRows.push(h); return { id: h.id as string }; } - const k = keyOf(data); - const row: Row = { id: `r_${rows.size + 1}`, ...data }; + const k = keyOf(data, table); + const row: Row = { id: `r_${rows.size + 1}`, __table: table, ...data }; rows.set(k, row); return { id: row.id as string }; }, @@ -157,6 +159,17 @@ function makeFakeEngine() { const objectBody = { name: 'showcase_task', label: 'Task', fields: { name: { type: 'text', label: 'Name' } } }; +/** + * [#8146] A spec-VALID permission set — the QA run's `showcase_contributor`. + * + * It has to parse, because `saveMetaItem` runs the Zod gate BEFORE the + * authorization door: an invalid body answers `INVALID_METADATA` / 422 and + * never reaches `assertAllowed`, which would make a refusal pin green for + * entirely the wrong reason. (Measured — the first draft of this fixture used + * `permissions: {}` and every case came back 422 from the schema.) + */ +const permissionBody = { name: 'showcase_contributor', label: 'Contributor', objects: {} }; + /** * `put` with everything but the base fixed, so each case differs in ONE way. * @@ -269,9 +282,12 @@ describe('#7682 — the refusal discriminates on package writability', () => { }); it('a permission set belonging to a read-only package is refused by the package door', async () => { - // The card's own hatch case, minus the hatch — see the suite docblock for - // why the hatch half is deliberately uncovered here. With no hatch set, - // the read-only base is what the refusal names, which is #7682's whole + // The card's own hatch case, minus the hatch: this is #7682's half, so it + // holds with the hatch CLOSED and must keep holding independently of + // #8146. (The hatch half is no longer uncovered — it is pinned in the + // `#8146` describe below; this comment used to say otherwise, which + // #8185 wrote while that gap was still open.) With no hatch set, the + // read-only base is what the refusal names, which is #7682's whole // point, and `permission` is the type the QA run used. const err = await putWith(repo, { type: 'permission', name: 'showcase_contributor', intent: 'override-artifact', packageId: READ_ONLY_PKG, @@ -311,6 +327,176 @@ describe('#7682 — the refusal discriminates on package writability', () => { }); }); + // ── [#8146] the hatch unlocks the TYPE, never the PACKAGE ───────────── + + /** + * #8146 — `OS_METADATA_WRITABLE` answered 200 on a read-only package while + * Studio rendered the same matrix disabled with a "Read-only" badge. + * + * **Maintainer ruling, 2026-08-12 (option B):** the badge is telling the + * truth and the server should refuse. The hatch is a TYPE-level unlock by + * its own shipped documentation — `environment-variables.mdx:305` defines it + * as treating named types "as `allowOrgOverride: true` … overridden per-org" + * — so it says nothing about the PACKAGE dimension. + * + * **How far the refusal reaches: NARROW** (PM ruling, veto window unopposed). + * Only a write that NAMES a read-only base is refused; a package-less hatch + * write still lands the overlay the documentation promises. NARROW is only + * honest if that overlay is real, so it was measured on this topology before + * the fix was written, and both halves are pinned below as PRESERVATION + * cases: package-less lands `{ package_id: null, organization_id: null }` + * env-wide, and `{ package_id: null, organization_id: }` under an org + * kernel — the documented per-org override, intact. + * + * ⛔ The BROADER reading (the hatch never unlocks a write against an item a + * read-only package provides, named or not) is NOT implemented and must not + * be "completed" here: it retires the hatch's only documented use and needs a + * maintainer decision plus a docs/ADR change. + * + * One measurement worth carrying, because it narrows what this card proves: + * `{ package_id: , organization_id: null }` is ALSO what a genuine + * `allowOrgOverride` overlay writes whenever the caller names the package it + * customizes (the `view` case in "no allow decision moves" above). So the + * defect was never "the hatch writes a row shape nothing else produces" — it + * is precisely the ruling's own sentence: a type-level unlock reached the + * package dimension. + */ + describe('#8146 — OS_METADATA_WRITABLE does not unlock a read-only package', () => { + /** Open the hatch for `permission`, the type the QA run used. */ + function openHatch(types = 'permission') { + process.env.OS_METADATA_WRITABLE = types; + resetEnvWritableMetadataTypes(); + ObjectStackProtocolImplementation.resetEnvWritableCache(); + } + + // ── the refusal: both halves of the ADR-0112 envelope ─────────────── + + it('the card\'s reproduction is refused: ITEM_LOCKED / 403, where it used to answer 200', async () => { + openHatch(); + const err = await putWith(repo, { + type: 'permission', name: 'showcase_contributor', + intent: 'override-artifact', packageId: READ_ONLY_PKG, + }); + + // `code` AND `status` — the ruling names both, and a message-only + // assertion cannot tell this refusal from the type door's. + expect(err).toMatchObject({ code: 'ITEM_LOCKED', status: 403 }); + // A refusal, not a report: nothing may reach `sys_metadata`. + expect(Array.from(engine.rows.values())).toEqual([]); + }); + + it('read-only-by-SCOPE is the same door — the hatch does not discriminate between the two signals', async () => { + openHatch(); + const err = await putWith(repo, { + type: 'permission', name: 'showcase_contributor', + intent: 'override-artifact', packageId: PLATFORM_PKG, + }); + expect(err).toMatchObject({ code: 'ITEM_LOCKED', status: 403, packageId: PLATFORM_PKG }); + }); + + it('a runtime-only hatch write into a read-only base: WRITABLE_PACKAGE_REQUIRED / 422', async () => { + // `job` has neither channel, so the hatch is the only thing that could + // have allowed this. Naming a read-only base takes it back — with the + // code whose prescription is TRUE for a create (a writable base helps). + openHatch('job'); + const err = await putWith(repo, { + type: 'job', name: 'nightly', intent: 'runtime-only', packageId: READ_ONLY_PKG, + }); + expect(err).toMatchObject({ code: 'WRITABLE_PACKAGE_REQUIRED', status: 422, packageId: READ_ONLY_PKG }); + }); + + it('does NOT prescribe the hatch that is already set — the false-prescription trap', async () => { + // Before #8146 this message ended "…or set OS_METADATA_WRITABLE=permission + // to grant a runtime escape hatch". Emitted while that variable IS set, + // it prescribes the step the caller already took — the shape that makes + // an automated client (or an AI agent) retry the same request forever. + openHatch(); + const err = await putWith(repo, { + type: 'permission', name: 'showcase_contributor', + intent: 'override-artifact', packageId: READ_ONLY_PKG, + }) as { message?: string }; + const message = String(err.message); + + expect(message).not.toMatch(/set OS_METADATA_WRITABLE/); + // …and it states the true remedy the measurement below proves exists. + expect(message).toContain('does not apply here'); + expect(message).toContain("Retry without '?package='"); + }); + + // ── PRESERVATION: what NARROW deliberately keeps working ──────────── + + it('a package-less hatch write still lands the env-wide overlay, bound to NO package', async () => { + // THE PREMISE. If this ever goes red, NARROW is preserving nothing and + // the fork (NARROW vs BROAD) goes back to the maintainer — it is not a + // test to "repair" by relaxing it. + openHatch(); + const err = await putWith(repo, { + type: 'permission', name: 'showcase_contributor', intent: 'override-artifact', + }); + + expect(err).toBeNull(); + const metaRows = Array.from(engine.rows.values()).filter((r) => r.__table === 'sys_metadata'); + expect(metaRows).toHaveLength(1); + expect(metaRows[0]).toMatchObject({ package_id: null, organization_id: null }); + }); + + it('a package-less hatch write under an ORG kernel lands the per-org override the docs promise', async () => { + // `environment-variables.mdx:305` — the hatch treats named types "as + // `allowOrgOverride: true` … overridden per-org". This is that sentence, + // executed: the row binds to the org and to no package. + openHatch(); + const orgRepo = new SysMetadataRepository({ + engine: engine as never, organizationId: 'org_acme', orgLabel: 'org_acme', + }); + const err = await putWith(orgRepo, { + type: 'permission', name: 'showcase_contributor', intent: 'override-artifact', + }); + + expect(err).toBeNull(); + const metaRows = Array.from(engine.rows.values()).filter((r) => r.__table === 'sys_metadata'); + expect(metaRows).toHaveLength(1); + expect(metaRows[0]).toMatchObject({ package_id: null, organization_id: 'org_acme' }); + }); + + it('a hatch write naming a WRITABLE base still lands — the door reads writability, not the hatch', async () => { + openHatch(); + const err = await putWith(repo, { + type: 'permission', name: 'showcase_contributor', + intent: 'override-artifact', packageId: WRITABLE_PKG, + }); + + expect(err).toBeNull(); + const metaRows = Array.from(engine.rows.values()).filter((r) => r.__table === 'sys_metadata'); + expect(metaRows[0]).toMatchObject({ package_id: WRITABLE_PKG }); + }); + + it('the ADR-0005 overlay is untouched: a registry-allowed type still overlays a read-only package', async () => { + // The limb ordering IS the fix, so it needs a pin on BOTH sides of the + // door. `view` is allowOrgOverride and returns at the registry limb — + // above the door — with the hatch open for an unrelated type. If the + // door had been placed one limb higher, this goes red and the whole + // overlay model closes. + openHatch(); + const err = await putWith(repo, { + type: 'view', name: 'case_grid', intent: 'override-artifact', packageId: READ_ONLY_PKG, + }); + expect(err).toBeNull(); + const metaRows = Array.from(engine.rows.values()).filter((r) => r.__table === 'sys_metadata'); + expect(metaRows[0]).toMatchObject({ package_id: READ_ONLY_PKG }); + }); + + it('with the hatch CLOSED the refusal still offers it — the prescription is chosen, not deleted', async () => { + // The other side of the false-prescription pin: opening the hatch (on a + // package-less write) remains a real answer, so the sentence must + // survive when the hatch is not already set. + const err = await putWith(repo, { + type: 'permission', name: 'showcase_contributor', + intent: 'override-artifact', packageId: READ_ONLY_PKG, + }) as { message?: string }; + expect(String(err.message)).toContain('set OS_METADATA_WRITABLE=permission'); + }); + }); + // ── the delete verb is deliberately not symmetrised (#6960) ─────────── it('delete keeps its own codes — DeleteOptions names no base', async () => { @@ -333,11 +519,13 @@ describe('#7682 — the refusal discriminates on package writability', () => { * * ⚠️ On a SCOPED kernel (`environmentId` set) the protocol refuses first, with * `NOT_OVERRIDABLE`, and this fix is not reachable — that second refusal point - * lives in `protocol.ts`, which this card is not authorised to edit. Filed - * separately; the asymmetry is stated here so the next reader measures it - * instead of assuming this suite covers both kernels. + * lives in `protocol.ts`, which neither #7682 nor #8146 is authorised to edit. + * It is filed as **#8184**, and it is deliberately NOT covered here: this file + * pins ONE kernel. ⛔ Do not read a green run of this suite as evidence that + * the scoped kernel refuses too — it does not, and #8146's hatch refusal below + * inherits exactly the same boundary. */ -describe('#7682 — through saveMetaItem on the host-config topology', () => { +describe('#7682 / #8146 — through saveMetaItem on the host-config topology', () => { function boot() { const engine = makeFakeEngine() as unknown as Record; (engine as { registry: Record }).registry = { @@ -347,9 +535,12 @@ describe('#7682 — through saveMetaItem on the host-config topology', () => { listItems: () => [], getItem: () => undefined, // A hit here is what makes the name artifact-backed, i.e. an - // `override-artifact` intent — the card's `showcase_task`. + // `override-artifact` intent — #7682's `showcase_task`, and [#8146] + // `showcase_contributor`, the permission set the QA run drove the hatch + // against. Both ship from the read-only package. getArtifactItem: (type: string, name: string) => - type === 'object' && name === 'showcase_task' + (type === 'object' && name === 'showcase_task') + || (type === 'permission' && name === 'showcase_contributor') ? { name, _packageId: READ_ONLY_PKG } : undefined, }; @@ -387,4 +578,52 @@ describe('#7682 — through saveMetaItem on the host-config topology', () => { expect(readOnly).toMatchObject({ code: 'ITEM_LOCKED', status: 403 }); expect(writable).toMatchObject({ code: 'NOT_OVERRIDABLE', status: 403 }); }, 30_000); + + /** + * [#8146] The QA run's own request, verbatim, through the same door. + * + * `PUT /api/v1/meta/permission/showcase_contributor?package=com.example.showcase` + * with `OS_METADATA_WRITABLE=permission` answered **200** on `main` — measured + * again on this tree before the fix, landing + * `{ package_id: 'com.example.showcase', organization_id: null }`. That is the + * write the ruling calls a bug, and this is where it is now refused. + */ + it('[#8146] the hatch write the QA run measured is refused: ITEM_LOCKED / 403, nothing persisted', async () => { + const { engine, protocol } = boot(); + process.env.OS_METADATA_WRITABLE = 'permission'; + resetEnvWritableMetadataTypes(); + ObjectStackProtocolImplementation.resetEnvWritableCache(); + + const err = await protocol + .saveMetaItem({ + type: 'permission', name: 'showcase_contributor', + item: permissionBody, packageId: READ_ONLY_PKG, + }) + .then(() => null, (e: unknown) => e); + + expect(err).toMatchObject({ code: 'ITEM_LOCKED', status: 403, packageId: READ_ONLY_PKG }); + const metaRows = Array.from((engine as unknown as { rows: Map }).rows.values()) + .filter((r) => r.__table === 'sys_metadata'); + expect(metaRows).toEqual([]); + }, 30_000); + + it('[#8146] the same hatch write WITHOUT ?package= still lands, bound to no package', async () => { + // The preservation half, end to end. NARROW refuses the named base and + // nothing else; this is the behaviour `environment-variables.mdx` promises + // and the reason the broad reading was not taken. + const { engine, protocol } = boot(); + process.env.OS_METADATA_WRITABLE = 'permission'; + resetEnvWritableMetadataTypes(); + ObjectStackProtocolImplementation.resetEnvWritableCache(); + + const err = await protocol + .saveMetaItem({ type: 'permission', name: 'showcase_contributor', item: permissionBody }) + .then(() => null, (e: unknown) => e); + + expect(err).toBeNull(); + const metaRows = Array.from((engine as unknown as { rows: Map }).rows.values()) + .filter((r) => r.__table === 'sys_metadata'); + expect(metaRows).toHaveLength(1); + expect(metaRows[0]).toMatchObject({ package_id: null, organization_id: null }); + }, 30_000); }); diff --git a/packages/metadata-protocol/src/sys-metadata-repository.ts b/packages/metadata-protocol/src/sys-metadata-repository.ts index ae83ba51e0..5d30cedfb0 100644 --- a/packages/metadata-protocol/src/sys-metadata-repository.ts +++ b/packages/metadata-protocol/src/sys-metadata-repository.ts @@ -1077,23 +1077,41 @@ export class SysMetadataRepository implements MetadataRepository { * * What this reads, and what it deliberately does NOT: * - * - **It changes no ALLOW decision.** Every allow limb above returns before - * this point, so a write that succeeds today still succeeds — including - * the ADR-0005 case that makes the naive "refuse writes into a read-only - * package" gate wrong: an org overlay of a code-shipped item *always* - * names the read-only package it customizes, and refusing that would - * close the whole overlay model. The env hatch (`OS_METADATA_WRITABLE`) - * returns two limbs above for the same reason — #7682's hatch-vs-Studio - * -badge half is a separate maintainer decision (#8146) and must not move - * as a side effect of this one. Pinned by the hatch case in - * `sys-metadata-repository.package-writability.test.ts`. - * - **It refuses nothing new.** This is the code-selection for writes that - * are ALREADY refused; the difference is which true fact the refusal - * reports. Making the package door an allow→deny gate here would extend - * ADR-0070 D1 (measured on `saveMetaItem` creates) to `promoteDraft` / - * `restoreVersion` / `revertCommit`, which route through {@link put} and - * carry the row's OWN binding — i.e. it would break republishing and - * repair of legacy package-bound rows, surfaces nobody measured. + * - **It changes exactly ONE allow decision: the env hatch (#8146).** Every + * REGISTRY limb still returns before this point, so a write allowed by + * the type registry still succeeds — including the ADR-0005 case that + * makes the naive "refuse writes into a read-only package" gate wrong: an + * org overlay of a code-shipped item *always* names the read-only package + * it customizes, and refusing that would close the whole overlay model. + * What moved BELOW the door is `OS_METADATA_WRITABLE`, and only it. + * - **Why that one moved — the #8146 ruling.** Maintainer ruling of + * 2026-08-12 (option B): Studio's "Read-only" badge is telling the truth + * and the server should refuse, because the hatch is a **type**-level + * unlock by its own shipped documentation + * (`content/docs/deployment/environment-variables.mdx` — it treats named + * types "as `allowOrgOverride: true` … overridden per-org") and the + * package CHANGELOG's "deliberately does not unlock the org dimension". + * A type-level unlock says nothing about the PACKAGE dimension, so a + * hatch write that NAMES a read-only base is refused here. + * - **How far that reaches — NARROW, and the measurement it rests on.** The + * door reads the base the caller NAMED, so a package-LESS hatch write is + * untouched and still lands the overlay the documentation promises. That + * is measured, not assumed: on this topology a package-less hatch write + * lands `{ package_id: null, organization_id: null }` env-wide and + * `{ package_id: null, organization_id: }` under an org kernel — + * both pinned in `sys-metadata-repository.package-writability.test.ts`. + * The BROADER reading (the hatch never unlocks a write against an item a + * read-only package provides, named or not) would retire the hatch's only + * documented use and is deliberately NOT implemented: it needs a + * maintainer decision plus a docs/ADR change, not a code edit. + * - **Beyond the hatch it still refuses nothing new.** For every other + * intent this remains the code-selection for writes ALREADY refused; the + * difference is which true fact the refusal reports. Widening it into a + * general allow→deny gate would extend ADR-0070 D1 (measured on + * `saveMetaItem` creates) to `promoteDraft` / `restoreVersion` / + * `revertCommit`, which route through {@link put} and carry the row's OWN + * binding — i.e. it would break republishing and repair of legacy + * package-bound rows, surfaces nobody measured. * - **Which code, by intent.** The two ledgered codes are not * interchangeable, and each is the one whose prescription is TRUE for its * case: @@ -1153,20 +1171,26 @@ export class SysMetadataRepository implements MetadataRepository { } } - // Phase 3a-env-writable: env-var escape hatch. + // Phase 3a-env-writable: env-var escape hatch. [#8146] READ here but + // APPLIED below the package door — the door needs to know whether the + // hatch is what would otherwise have allowed this write, because that + // changes which remediation is true. const env = envWritableMetadataTypes(); - if (env.has(singular) || env.has(type)) return; + const hatchOpen = env.has(singular) || env.has(type); - // [#7682] The package door. Only reached once every allow limb above has - // declined, so it re-reports an existing refusal — see the TSDoc for why - // it is a code selection and not a gate. + // [#7682 / #8146] The package door. Above the hatch limb, below every + // registry limb — see the TSDoc for why that is the whole ruling. const namedBase = typeof packageId === 'string' && packageId.length > 0; if (namedBase && !isWritablePackage(this.engine, packageId)) { throw intent === 'runtime-only' - ? SysMetadataRepository.readOnlyBaseCreateError(type, packageId as string) - : SysMetadataRepository.readOnlyBaseOverrideError(type, packageId as string); + ? SysMetadataRepository.readOnlyBaseCreateError(type, packageId as string, hatchOpen) + : SysMetadataRepository.readOnlyBaseOverrideError(type, packageId as string, hatchOpen); } + // [#8146] The hatch unlocks the TYPE — for a write that named no base, or + // named a writable one. It never reaches the package dimension. + if (hatchOpen) return; + const allowed = [ ...OVERLAY_ALLOWED_TYPES, ...envWritableMetadataTypes(), @@ -1195,11 +1219,18 @@ export class SysMetadataRepository implements MetadataRepository { * differently at each. The message stays user-actionable because here the * prescription is true — a writable base really is what this write needs. */ - private static readOnlyBaseCreateError(type: string, packageId: string): Error { + private static readOnlyBaseCreateError(type: string, packageId: string, hatchOpen = false): Error { const err: any = new Error( `[writable_package_required] Cannot create ${type} in package '${packageId}': ` + `that package is read-only (provided by code or an installed app), so it is not a writable base. ` - + `Switch to a writable package in the package selector, or create a new one, and retry.`, + + `Switch to a writable package in the package selector, or create a new one, and retry.` + // [#8146] Said only when the hatch IS set, because otherwise it is noise. + // Without it the operator who just set the variable reads a refusal that + // looks like the variable was ignored, and sets it again. + + (hatchOpen + ? ` (OS_METADATA_WRITABLE is set for '${PLURAL_TO_SINGULAR[type] ?? type}': it unlocks the metadata ` + + `TYPE, not package writability, so it does not make a read-only package a writable base.)` + : ''), ); err.code = 'WRITABLE_PACKAGE_REQUIRED'; err.status = 422; @@ -1213,21 +1244,40 @@ export class SysMetadataRepository implements MetadataRepository { * * `ITEM_LOCKED` rather than `WRITABLE_PACKAGE_REQUIRED`: switching packages * cannot help — the artifact is code-shipped wherever the caller points — - * so the refusal states the lock and prescribes the two things that DO move - * it (edit the source and redeploy, or open the documented operator hatch). + * so the refusal states the lock and prescribes what DOES move it. [#8146] + * That prescription is now chosen by `hatchOpen`, because the two cases have + * genuinely different remedies: with the hatch CLOSED, opening it (on a + * package-less write) is one of the real answers; with it already OPEN, this + * door is refusing *despite* it — by ruling — and repeating "set + * OS_METADATA_WRITABLE" would be a false prescription of exactly the kind + * PR #8185's patch round rejected. Same code and status either way: the + * condition is one condition, and only the remedy differs. * `lockSource: 'package'` is ADR-0010's own reserved value for a lock the * PACKAGE layer asserts, which is what makes this distinguishable from the * item-level `_lock` refusal (`assertLockAllowsWrite`) that carries a `lock` * value read off the item. This one claims no `_lock`, because the item * declares none. */ - private static readOnlyBaseOverrideError(type: string, packageId: string): Error { + private static readOnlyBaseOverrideError(type: string, packageId: string, hatchOpen = false): Error { + const singular = PLURAL_TO_SINGULAR[type] ?? type; const err: any = new Error( `[item_locked] Cannot overlay '${type}' in package '${packageId}': that package is read-only ` + `(provided by code or an installed app) and the type has no per-org overlay channel ` + `(allowOrgOverride=false), so this item is locked against runtime edits. ` - + `Edit the source artifact and redeploy, or set OS_METADATA_WRITABLE=${PLURAL_TO_SINGULAR[type] ?? type} ` - + `to grant a runtime escape hatch. See docs/adr/0010-metadata-protection-model.md.`, + // [#8146] The prescription is chosen by whether the hatch is ALREADY + // open, because "set OS_METADATA_WRITABLE" is FALSE once it is set — + // this door refuses with it set, by ruling. A refusal that prescribes + // the step the caller already took is what makes an automated client + // (and an AI agent) retry the same request forever. + + (hatchOpen + ? `OS_METADATA_WRITABLE=${singular} is set, and it does not apply here: the hatch unlocks the ` + + `metadata TYPE (treating it as allowOrgOverride), never a package's writability. ` + + `Retry without '?package=' to land the env-wide / per-org overlay the hatch does grant, ` + + `or edit the source artifact and redeploy.` + : `Edit the source artifact and redeploy, or set OS_METADATA_WRITABLE=${singular} ` + + `to grant a runtime escape hatch on this TYPE (it does not unlock package writability, ` + + `so pair it with a package-less write).`) + + ` See docs/adr/0010-metadata-protection-model.md.`, ); err.code = 'ITEM_LOCKED'; err.status = 403;