Merge origin/main into claude/issue-4711-check-mode-manifest-write

Author: claude

.changeset/app-area-fail-open-gates-removed.md

@@ -0,0 +1,58 @@
+---
+'@objectstack/spec': major
+---
+
+feat(spec)!: retire the two fail-open app-area gates — `app.areas[].visible` and `app.areas[].requiredPermissions` (#4651)
+
+These were **not** inert authoring keys. They were capability gates that **failed
+open**: an author wrote `requiredPermissions: ['sales.admin']` on a navigation
+area, got a clean parse and a stored value, and the area — with everything under
+it — was served and rendered to **every user**.
+
+**This is a breaking change with a real migration.** Both keys are authorable
+metadata keys on a `.strict()` schema, so existing `app` metadata that declares
+either one now **fails to parse** with the prescription below. `authorable-surface.json`
+is net **−2 keys**. This is not the "zero metadata migration" shape of the
+same-window renames (#4661 C8, #4684 C9) — those kept every key.
+
+**The retirement kit:**
+
+| FROM | TO | Fix |
+|---|---|---|
+| `app.areas[].requiredPermissions` | *(removed)* | Delete the key. Gate each of the area's `navigation` items with `requiredPermissions` / `requiresService`, or gate the whole app with `requiredPermissions` on the AppSchema. |
+| `app.areas[].visible` | *(removed)* | Delete the key. Move the same CEL expression onto the area's `navigation` items — a navigation **item**'s `visible` is evaluated per item by the shell. |
+
+The retired alias spellings `visibleWhen` / `visibleOn` / `permissions` carry the
+same prescriptions rather than renaming onto keys that are themselves gone.
+
+Run `os migrate meta --from 16` to rewrite existing sources automatically
+(ADR-0087 D2 conversion `app-area-fail-open-gates-removed`, wired into the
+protocol-17 D3 chain step).
+
+**Why they read alive — and why that made them worse than dead.** The *same key
+names* are genuinely enforced one level up and one level down:
+
+- **app-level** `requiredPermissions` — server-side: an app whose required
+  permissions the caller lacks is dropped from `/meta` entirely;
+- **item-level** `requiredPermissions` / `requiresService` — stripped server-side
+  from the app's top-level `navigation` tree, and re-checked in the shell;
+  item-level `visible` is a real CEL gate in the shell.
+
+Three layers, of which the middle one was theatre — `filterAppForUser` reads the
+app's `requiredPermissions` and then walks **only** `item.navigation`; it never
+touches `item.areas`, and the client renders every area in the switcher. ADR-0078
+false compliance, the same shape as `capabilities.readOnly` (#4583).
+
+**Removed rather than enforced (ADR-0049), deliberately.** Enforcing area gates
+is not wrong, it is unscoped: it needs semantics settled first — when an area is
+filtered out, do its items disappear everywhere, or still participate in other
+areas? does the server bind `user` for area-level CEL? — and a retirement must
+not invent an authorization mechanism. Removing a gate that never gated is
+strictly safer than shipping a major with it still declared, which would have
+kept authors writing it for all of 17.x.
+
+**One caveat the prescription carries rather than hides:** per-item gating
+*inside* an area is enforced by the shell only, because the server does not walk
+`areas`. Anything that must never reach the browser belongs in the app's
+top-level `navigation` tree, or in its own app. Trading one false belief for a
+weaker one would have repeated the defect this removal exists to end.

.changeset/runtime-authoring-gate.md

@@ -0,0 +1,52 @@
+---
+"@objectstack/metadata-protocol": minor
+"@objectstack/lint": minor
+"@objectstack/cli": patch
+---
+
+Author-time rules now gate the RUNTIME metadata write path, not just the CLI (#4463)
+
+The 26 author-time rules `os validate` / `os build` / `os lint` share (#4409) ran on
+those three commands and nowhere else. Every runtime metadata write — Studio's
+designer, REST `/meta` item CRUD, an MCP/AI agent authoring a flow — reaches
+`saveMetaItem`, which did a per-type Zod `safeParse` and stopped. For a tenant that
+was not the weakest of four doors, it was the **only** door: a `sys_metadata`
+overlay row is not in the CLI's config file, so there was no command they could run
+instead. An approval flow whose `expression` approver is broken CEL
+(`record.owner ==`) is Zod-valid, so it saved, registered, and failed at the node's
+entry the first time it fired — the exact body `os lint` had rejected since #4409.
+
+**One shared core, one runtime gate.**
+
+- The rule registry moved from `packages/cli` into `@objectstack/lint`
+  (`AUTHORING_RULES`), and the CLI now calls it there. Five rule modules moved with
+  it (`lintFlowPatterns`, `lintLivenessProperties`, `lintAutonumberFormats`,
+  `lintViewRefs`, `data-model-rules`), unchanged. There is one table; a second one
+  cannot be introduced without failing `authoring-rule-wiring.test.ts`.
+- New kernel-safe subpath export **`@objectstack/lint/runtime`** — the entry the
+  metadata write path imports. Running the gate loads neither `typescript` nor
+  `sucrase`, pinned by a new `runtime-lazy-deps.test.ts` alongside the existing
+  `lazy-deps.test.ts`, which is unchanged.
+- Each registry entry now declares `surfaces` (`cli` / `runtime-publish`) plus
+  either the metadata `runtimeTypes` it judges or a written `surfaceReason`. The
+  ratchet fails an entry that answers neither.
+
+**Behaviour**
+
+- A `state: 'active'` `saveMetaItem` — and the draft→active promotion in
+  `publishMetaItem` — of a **flow** runs the flow / approval / expression /
+  reference rule families. A gating finding is refused with **422
+  `INVALID_METADATA`**, in the same structured envelope the Zod failure already
+  used, with `rule` / `path` / `where` / `message` / `hint` per issue.
+- **Draft saves are never gated** — a draft is allowed to be half-finished and
+  cannot execute.
+- Only the write is judged: the rules run twice (context with and without the
+  submitted item) and only findings the item *added* can refuse it, so a
+  pre-existing violation in a stored row never blocks an unrelated save. Stored
+  rows keep being read.
+- Escape hatch **`OS_ALLOW_UNLINTED_METADATA_WRITES=1`** turns the refusal into a
+  loud log for a migration window. Unset it once the metadata is fixed — the
+  runtime executes what it published.
+
+Only `flow` writes are gated in this pass; every other metadata type carries a
+recorded reason in the registry.

content/docs/references/ui/app.mdx

@@ -211,8 +211,6 @@ const result = ActionNavItemSchema.parse(data);
 | **label** | `string` | ✅ | Area display label |
 | **icon** | `string` | optional | Area icon name |
 | **description** | `string` | optional | Area description |
-| **visible** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | Visibility predicate (CEL) for this area. |
-| **requiredPermissions** | `string[]` | optional | Permissions required to access this area |
 | **navigation** | `{ id: string; label: string; icon?: string; order?: number; … } \| { id: string; label: string; icon?: string; order?: number; … } \| { id: string; label: string; icon?: string; order?: number; … } \| { id: string; label: string; icon?: string; order?: number; … } \| { id: string; label: string; icon?: string; order?: number; … } \| { id: string; label: string; icon?: string; order?: number; … } \| { id: string; label: string; icon?: string; order?: number; … } \| { type: 'separator'; id?: string; order?: number } \| { id: string; label: string; icon?: string; order?: number; … }[]` | ✅ | Navigation items within this area |
 
 

docs/protocol-upgrade-guide.md

@@ -168,6 +168,8 @@ Separately, `object.managedBy: 'system'` is retired in favour of `'system-data'`
 
 Finally, five keys retire because the advisory lint could never have warned about them (#4509): mapping `extractQuery` / `errorPolicy` / `batchSize`, and app `contextSelectors[].includeAll` / `.placement`. Four of the five carry schema DEFAULTS, and a default materialises at parse time — so the liveness lint cannot tell a value the author wrote from one the schema supplied, and marking them would have warned on every mapping and every selector in existence. For a key in that state removal is not the escalation after a warning; it is the only channel that ever reaches the author, which is why they ship inside the 17.0.0 window rather than after a deprecation cycle. What they claimed: `extractQuery` promised an export path no exporter implements (exports go through the ordinary query API); `errorPolicy` offered skip/abort/retry where error handling belongs to the import REQUEST; `batchSize` sized batches the write path sizes itself; `placement` offered a topbar that places nothing. `includeAll` is the one worth reading twice — it was not unread but deliberately DISOBEYED, because context selectors are mandatory-scope and an "All" row would clear the scope: on Studio's package selector that means listing the platform's own system/cloud kernel packages to a developer who scoped to their package. `STUDIO_APP` authored `includeAll: true` against a renderer that ignored it. The mapping prescription for `batchSize` deliberately offers no rename: bulk-action, connector, sync, offline, seed-loader and NoSQL-cursor `batchSize` are all live, but each is a different key sizing its own path — the same trap `datasource.retryPolicy` vs `hook`/`job` `retryPolicy` had to defuse one issue earlier.
 
+The sharpest removal in this step is two keys wide: `app.areas[].visible` and `app.areas[].requiredPermissions` (#4651). Read the class before the count — these were not inert authoring keys but FAIL-OPEN access gates. The server-side authority (`filterAppForUser`) checks the app's `requiredPermissions` and then walks ONLY the top-level `navigation` tree; it never reads `item.areas` at all, and the client renders every area in the switcher. So an author writing `requiredPermissions: ['sales.admin']` on an area got a clean parse, a stored value, and an area visible to everybody — and had every reason to believe otherwise, because the SAME key names are genuinely enforced one level up and one level down: app-level `requiredPermissions` drops the whole app server-side, and a navigation ITEM's `requiredPermissions` / `requiresService` are stripped server-side and re-checked in the shell, whose item-level `visible` is a real CEL gate. Three layers, of which the middle one was theatre. Enforcing instead was weighed and deliberately not taken here: it needs semantics decided first (does filtering an area remove its items everywhere? does the server bind `user` for area CEL?), and a retirement must not invent an authorization mechanism — while shipping a major with the gate still declared would have kept authors writing it for all of 17.x. The rewrite is lossless in outcome (the keys changed nothing), so what an upgrading author has to re-decide is only where the gate really goes: onto the items inside the area, or onto the app. One honest caveat the prescription carries rather than hides — per-item gating INSIDE an area is enforced by the shell only, since the server does not walk `areas`, so anything that must never reach the browser belongs in the top-level tree or in its own app.
+
 The same window converges the retry policy (#4661). `@objectstack/spec/automation` and `@objectstack/spec/system` each exported a `RetryPolicy`/`RetryPolicySchema` resolving to a DIFFERENT declaration, so which shape a consumer got depended only on the import path (#4411) — yet both computed `delay = base * multiplier^(retry-1)` and both executors implemented that same formula. One declaration now serves both entries with the union of their capabilities, so `job.retryPolicy` gains the `maxRetryDelayMs` ceiling and `jitter` (both enforced in `runWithPolicy`, not merely declared — jitter is what stops a fleet of jobs that failed on one outage from retrying in lockstep). The single authorable casualty is the automation spelling of the base delay: `retryDelayMs` → `backoffMs`, a pure rename that replays losslessly and is what the already-enforced retry policies (`job.retryPolicy`, `hook.retryPolicy`) call it.
 
 The subtle half is the defaults, and it is worth stating because no gate can see it: `job.retryPolicy` defaulted `maxRetries: 3` / `backoffMultiplier: 2` while the automation shape defaulted 0 / 1, and the authorable-surface gate compares KEY SETS — a changed default is invisible to it, to the tombstone mechanism and to `spec_changes` alike. The merged declaration takes 0 / 1 (retry replays side effects, so it is opt-in — the same reading already recorded in `flow-retry-max-retries-required`), and the conversion writes the pre-17 numbers into every existing `job.retryPolicy` that omitted them. Deployed stacks therefore keep their exact behaviour; what changes is only what a NEWLY authored omission means.
@@ -192,6 +194,7 @@ The same enforce-or-remove pass reaches the event vocabulary: `DataEventType` dr
 | `permission-rls-priority-removed` | `permission.rowLevelSecurity.priority` | RLS-policy key 'priority' removed (#3896 audit — policies OR-combine, so the promised conflict-resolution semantics cannot exist; dropping it changes no outcome) | retired — `migrate meta` only |
 | `tool-inert-authoring-keys-removed` | `tool.category / tool.permissions / tool.active / tool.builtIn` | tool keys 'category'/'permissions'/'active'/'builtIn' removed (#3896 close-out — authorable and inert; permissions gated nothing, active:false withdrew nothing) | retired — `migrate meta` only |
 | `app-dead-authoring-keys-removed` | `app.version / app.aria / app.objects / app.apis / app.sharing / app.embed / app.mobileNavigation / app.contextSelectors.includeAll / app.contextSelectors.placement / app.homePageId / app.areas.order` | app keys 'version'/'aria'/'objects'/'apis'/'sharing'/'embed'/'mobileNavigation'/'homePageId' plus contextSelectors 'includeAll'/'placement' and areas 'order' removed (liveness audits #4001, #4509, #4667 — never read; sharing/embed declared a public surface no route enforced, mobileNavigation was fully unimplemented, includeAll was deliberately disobeyed because an 'All' row would clear a mandatory scope, the landing page IS the first nav item, and no renderer ever sorted areas) | retired — `migrate meta` only |
+| `app-area-fail-open-gates-removed` | `app.areas.visible / app.areas.requiredPermissions` | navigation-area keys 'visible'/'requiredPermissions' removed (#4651, ADR-0049 — FAIL-OPEN access gates: no layer ever read them, so a 'hidden' or permission-gated area was served and rendered to every user, while the identically named keys on a navigation ITEM and on the APP are enforced; gate the items inside the area, or gate the app) | retired — `migrate meta` only |
 | `field-required-notnull-explicit` | `object.fields.*.required / object.fields.*.storage.notNull` | required fields gain explicit 'storage.notNull: true' (ADR-0113 — pre-17 'required' implied the column constraint; post-17 it is only the write contract) | retired — `migrate meta` only |
 | `action-inert-keys-removed` | `action.shortcut / action.bulkEnabled` | action keys 'shortcut'/'bulkEnabled' removed (#3896 close-out — no keydown path dispatches shortcuts; the multi-select toolbar reads the view's bulkActions) | retired — `migrate meta` only |
 | `flow-inert-keys-removed` | `flow.active / flow.template / flow.nodes[].outputSchema / flow.errorHandling.fallbackNodeId` | flow keys 'active'/'template', node 'outputSchema' and errorHandling 'fallbackNodeId' removed (#3896 close-out — active:false never stopped a flow; status is the enforced lifecycle) | retired — `migrate meta` only |

packages/cli/src/commands/compile.ts

@@ -16,7 +16,7 @@ import {
 import { loadConfig } from '../utils/config.js';
 import { lowerCallables } from '../utils/lower-callables.js';
 import { buildAccessMatrix, diffAccessMatrix } from '@objectstack/lint';
-import { runAuthoringRules, splitBySeverity, authoringRulesFor } from '../lint/authoring-rules.js';
+import { runAuthoringRules, splitBySeverity, authoringRulesFor } from '@objectstack/lint';
 import { resolveSduiManifest } from '../utils/sdui-manifest.js';
 import { preflightRequiredCapabilities, renderCapabilityMessage } from '../utils/capability-preflight.js';
 import { collectAndLintDocs } from '../utils/collect-docs.js';

packages/cli/src/commands/lint.ts

@@ -7,8 +7,7 @@ import { normalizeStackInput } from '@objectstack/spec';
 import { PROTOCOL_MAJOR } from '@objectstack/spec/kernel';
 import { loadConfig, BUNDLE_REQUIRE_EXTERNALS } from '../utils/config.js';
 import { computeI18nCoverage, type CoverageIssue } from '../utils/i18n-coverage.js';
-import { lintDataModel } from '../lint/data-model-rules.js';
-import { runAuthoringRules } from '../lint/authoring-rules.js';
+import { lintDataModel, runAuthoringRules } from '@objectstack/lint';
 import { resolveSduiManifest } from '../utils/sdui-manifest.js';
 import { collectAndLintDocs } from '../utils/collect-docs.js';
 import { scoreMetadata } from '../lint/score.js';

packages/cli/src/commands/validate.ts

@@ -13,7 +13,7 @@ import {
   type ConversionNotice,
 } from '@objectstack/spec';
 import { loadConfig } from '../utils/config.js';
-import { runAuthoringRules, splitBySeverity, authoringRulesFor } from '../lint/authoring-rules.js';
+import { runAuthoringRules, splitBySeverity, authoringRulesFor } from '@objectstack/lint';
 import { resolveSduiManifest } from '../utils/sdui-manifest.js';
 import { preflightRequiredCapabilities, renderCapabilityMessage } from '../utils/capability-preflight.js';
 import { collectAndLintDocs } from '../utils/collect-docs.js';

packages/cli/src/lint/score.ts

@@ -17,7 +17,7 @@
 
 import { ObjectStackDefinitionSchema, normalizeStackInput } from '@objectstack/spec';
 import { lintConfig } from '../commands/lint.js';
-import type { LintIssue, Severity } from './data-model-rules.js';
+import type { LintIssue, Severity } from '@objectstack/lint';
 
 /** Penalty weights per issue class. Schema errors are the most severe. */
 export const SCORE_WEIGHTS = {

packages/cli/test/authoring-rule-command-parity.test.ts

@@ -28,7 +28,7 @@ import { mkdtempSync, rmSync, writeFileSync } from 'node:fs';
 import { tmpdir } from 'node:os';
 import { join } from 'node:path';
 import { fileURLToPath } from 'node:url';
-import { AUTHORING_COMMANDS, runAuthoringRules, type AuthoringCommand } from '../src/lint/authoring-rules.js';
+import { AUTHORING_COMMANDS, runAuthoringRules, type AuthoringCommand } from '@objectstack/lint';
 
 const cliBin = join(fileURLToPath(new URL('.', import.meta.url)), '..', 'bin', 'run-dev.js');
 

packages/cli/test/data-model-rules.test.ts

@@ -1,5 +1,5 @@
 import { describe, expect, it } from 'vitest';
-import { lintDataModel, lintUniqueDeclarations } from '../src/lint/data-model-rules';
+import { lintDataModel, lintUniqueDeclarations } from '@objectstack/lint';
 import { lintConfig } from '../src/commands/lint';
 
 const rulesOf = (issues: { rule: string }[]) => issues.map((i) => i.rule);