diff --git a/.changeset/runtime-authoring-gate.md b/.changeset/runtime-authoring-gate.md new file mode 100644 index 0000000000..37b4ca3fa2 --- /dev/null +++ b/.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. diff --git a/packages/cli/src/commands/compile.ts b/packages/cli/src/commands/compile.ts index db6686b774..6256aff36b 100644 --- a/packages/cli/src/commands/compile.ts +++ b/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'; diff --git a/packages/cli/src/commands/lint.ts b/packages/cli/src/commands/lint.ts index 6d71845874..751c11da72 100644 --- a/packages/cli/src/commands/lint.ts +++ b/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'; diff --git a/packages/cli/src/commands/validate.ts b/packages/cli/src/commands/validate.ts index 60091e9a4d..d81660500b 100644 --- a/packages/cli/src/commands/validate.ts +++ b/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'; diff --git a/packages/cli/src/lint/score.ts b/packages/cli/src/lint/score.ts index 2bccecde0c..8a42073c8a 100644 --- a/packages/cli/src/lint/score.ts +++ b/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 = { diff --git a/packages/cli/test/authoring-rule-command-parity.test.ts b/packages/cli/test/authoring-rule-command-parity.test.ts index e3c5d83e41..9312d5d80c 100644 --- a/packages/cli/test/authoring-rule-command-parity.test.ts +++ b/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'); diff --git a/packages/cli/test/data-model-rules.test.ts b/packages/cli/test/data-model-rules.test.ts index ccf7007972..03527b4917 100644 --- a/packages/cli/test/data-model-rules.test.ts +++ b/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); diff --git a/packages/cli/test/validate-build-gate-parity.test.ts b/packages/cli/test/validate-build-gate-parity.test.ts index caa3b532d5..2f42bf345c 100644 --- a/packages/cli/test/validate-build-gate-parity.test.ts +++ b/packages/cli/test/validate-build-gate-parity.test.ts @@ -14,7 +14,7 @@ import { join } from 'node:path'; * ## What changed, and what this file still guards * * The metadata rules the two commands share now come from ONE table - * (`src/lint/authoring-rules.ts`, #4409), and its own ratchet — + * (`@objectstack/lint`'s `authoring-rules.ts`, #4409/#4463), and its own ratchet — * `src/commands/authoring-rule-wiring.test.ts` — proves all three authoring * commands run the identical gating set. That is a stronger guarantee than the * source diff this file used to do, and it covers `os lint` too. @@ -60,7 +60,7 @@ const SHARED_NON_REGISTRY_GATES: readonly string[] = [ * Each entry is a deliberate assertion that the check CANNOT be made read-only * — it needs the emitted artifact, the bundler, or filesystem output. A gate * that merely *reads* the parsed stack does not belong here; wire it into - * `validate.ts`, or better, register it in `src/lint/authoring-rules.ts` so all + * `validate.ts`, or better, register it in `@objectstack/lint`'s `authoring-rules.ts` so all * three authoring commands get it at once. */ const BUILD_ONLY_GATES: Readonly> = { @@ -120,7 +120,7 @@ describe('os validate is the read-only superset of os build (#3782, #4409)', () expect( missing, `os build runs ${missing.length} gate(s) that os validate does not: ${missing.join(', ')}.\n` + - `Register it in packages/cli/src/lint/authoring-rules.ts so all three authoring commands run ` + + `Register it in packages/lint/src/authoring-rules.ts so all three authoring commands run ` + `it, wire it into validate.ts by hand and add it to SHARED_NON_REGISTRY_GATES, or add it to ` + `BUILD_ONLY_GATES with a reason.`, ).toEqual([]); diff --git a/packages/lint/package.json b/packages/lint/package.json index f7c75a7e42..8f1f0734b6 100644 --- a/packages/lint/package.json +++ b/packages/lint/package.json @@ -2,7 +2,7 @@ "name": "@objectstack/lint", "version": "17.0.0-rc.1", "license": "Apache-2.0", - "description": "Static, build-time validation for an ObjectStack metadata graph — dashboard widget bindings, CEL/predicate expressions, and more. Pure (stack) => Issue[] functions shared by the CLI's `os validate` and any other consumer (e.g. AI authoring). Depends on @objectstack/spec; never on a runtime.", + "description": "Static, build-time validation for an ObjectStack metadata graph \u2014 dashboard widget bindings, CEL/predicate expressions, and more. Pure (stack) => Issue[] functions shared by the CLI's `os validate` and any other consumer (e.g. AI authoring). Depends on @objectstack/spec; never on a runtime.", "type": "module", "main": "dist/index.js", "types": "dist/index.d.ts", @@ -11,10 +11,15 @@ "types": "./dist/index.d.ts", "import": "./dist/index.js", "require": "./dist/index.cjs" + }, + "./runtime": { + "types": "./dist/runtime.d.ts", + "import": "./dist/runtime.js", + "require": "./dist/runtime.cjs" } }, "scripts": { - "build": "tsup --config ../../tsup.config.ts", + "build": "tsup", "dev": "tsc -w", "test": "vitest run", "typecheck": "tsc --noEmit" diff --git a/packages/cli/src/commands/authoring-rule-wiring.test.ts b/packages/lint/src/authoring-rule-wiring.test.ts similarity index 67% rename from packages/cli/src/commands/authoring-rule-wiring.test.ts rename to packages/lint/src/authoring-rule-wiring.test.ts index 7ef82819a7..687894f9b6 100644 --- a/packages/cli/src/commands/authoring-rule-wiring.test.ts +++ b/packages/lint/src/authoring-rule-wiring.test.ts @@ -1,9 +1,15 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. // -// The ratchet behind `lint/authoring-rules.ts` (#4409). Supersedes -// `reference-integrity-wiring.test.ts`, which guarded the same seam for one -// rule family (#3583 §5 D5, #4384) — the suite is now one entry in the registry -// this file guards, so its invariants are carried below rather than duplicated. +// The ratchet behind `authoring-rules.ts` (#4409; extended to the runtime +// surface in #4463). Supersedes `reference-integrity-wiring.test.ts`, which +// guarded the same seam for one rule family (#3583 §5 D5, #4384) — the suite is +// now one entry in the registry this file guards, so its invariants are carried +// below rather than duplicated. +// +// It moved here from `packages/cli/src/commands/` with the registry itself: the +// table is no longer the CLI's, and a guard that lives in one of the four +// consumers reads as if that consumer were privileged. It scans all four by +// repo-relative path. // // ## Why a test and not a comment // @@ -12,6 +18,14 @@ // rule's unit tests pass, and the only symptom is that `os validate`, `os build` // and `os lint` quietly disagree about the same stack. // +// #4463 is the same defect with the doors counted properly. The three commands +// agreed with each other perfectly — and the runtime write path, the only door +// a Studio tenant or an MCP/AI author has, ran none of the 26 rules at all. A +// guard that asks "do the three commands agree?" answers YES on that state, +// which is why invariant 5 below asks the different question: is every SURFACE +// on the table, and does each one consume the table rather than a list of its +// own? +// // That failure mode was repaired four times before anyone guarded the mode // itself: the reference-integrity suite (#3583), the four CLI-local authoring // lints wired into `build` alone (#3782), `validateReadonlyFlowWrites` missing @@ -36,6 +50,10 @@ // anywhere asked whether its coverage should follow. // 4. No command hand-wires a rule. Every remaining direct call is on the // ratchet below with a reason, and adding one is an explicit edit. +// 5. Every rule answers the SURFACE question — it either runs on the runtime +// publish gate (with the metadata types it judges) or records why not — and +// the runtime gate consumes this table rather than naming rules itself +// (#4463). // // ## Why it scans source // @@ -44,22 +62,25 @@ // reason `@objectstack/lint`'s `lazy-deps.test.ts` scans `src/` rather than // probing a module cache. Behavioural coverage of what each rule FINDS lives in // that rule's own tests; this file guards only the seam between the registry and -// the three call sites. +// its four call sites. import { existsSync, readFileSync } from 'node:fs'; import { dirname, join } from 'node:path'; import { fileURLToPath } from 'node:url'; import { describe, it, expect } from 'vitest'; -import { REFERENCE_INTEGRITY_RULES } from '@objectstack/lint'; +import { REFERENCE_INTEGRITY_RULES } from './reference-integrity-suite.js'; import { AUTHORING_COMMANDS, AUTHORING_RULES, + AUTHORING_SURFACES, authoringRulesFor, type AuthoringCommand, -} from '../lint/authoring-rules.js'; +} from './authoring-rules.js'; +import { runtimeAuthoringRulesFor, runtimeGatedTypes, stackKeyForType } from './runtime-gate.js'; -const commandsDir = dirname(fileURLToPath(import.meta.url)); -const repoRoot = join(commandsDir, '..', '..', '..', '..'); +const srcDir = dirname(fileURLToPath(import.meta.url)); +const repoRoot = join(srcDir, '..', '..', '..'); +const commandsDir = join(repoRoot, 'packages/cli/src/commands'); /** The command source file each authoring command lives in. */ const COMMAND_FILES: Readonly> = { @@ -68,6 +89,13 @@ const COMMAND_FILES: Readonly> = { lint: 'lint.ts', }; +/** + * The runtime surface's own call site: the ONE place the metadata write path + * reaches the shared core. Scanned for the same two things the command files + * are — that it runs the registry, and that it names no rule of its own. + */ +const RUNTIME_GATE_FILE = 'packages/metadata-protocol/src/runtime-authoring-gate.ts'; + const sourceOf = (file: string) => readFileSync(join(commandsDir, file), 'utf8'); /** @@ -112,6 +140,22 @@ const LINT_IMPORT_RATCHET: Readonly> = { diffAccessMatrix: 'The other half of the D6 snapshot gate: it compares a committed `access-matrix.json` against the ' + 'matrix above. Reads a file next to the config, so it cannot run where that file may not exist.', + // The registry's own API. Importing THESE is the sanctioned wiring — it is + // importing a RULE that is not (#4409). They became cross-package imports in + // #4463 when the table moved to `@objectstack/lint`, so the import scan sees + // them where it previously saw a relative path. + runAuthoringRules: + 'The registry runner itself — the ONE call every surface is required to make. Ratcheted so the ' + + 'import scan does not read the sanctioned wiring as a hand-wired rule.', + authoringRulesFor: + 'Registry query (which rules does this command run), used to report coverage in `--json` output. ' + + 'Reads the table; runs nothing.', + splitBySeverity: + 'Pure partition of a finding list into gating vs advisory. Carries no rule identity at all.', + lintDataModel: + "`os lint`'s data-model best-practice sweep — see DIRECT_CALL_RATCHET above for why it is " + + 'deliberately lint-only. It relocated into `@objectstack/lint` with the rest of the rules in ' + + '#4463, so the import scan now sees it too.', }; /** Every registry rule name, plus the member names of the suite it embeds. */ @@ -132,6 +176,15 @@ const REGISTRY_NAMES = new Set([ */ const UNWIRED_RULE_LEDGER: Readonly> = {}; +/** + * Names the unwired-rule closure treats as wired-with-a-reason: anything the + * DIRECT_CALL_RATCHET already justifies. Before #4463 those rules lived in the + * CLI and never appeared on `@objectstack/lint`'s export surface, so the two + * ratchets could not overlap; the relocation made them overlap, and answering + * the same question twice in two ledgers is how ledgers rot. + */ +const ratchetedElsewhere = (name: string) => name in DIRECT_CALL_RATCHET; + /** * Every `validate*` / `lint*` symbol the lint package's public barrel exports — * read from source for the same reason the call-site scans are: vitest inlines @@ -218,7 +271,7 @@ describe('authoring-rule registry wiring (#4409)', () => { unratcheted, `${COMMAND_FILES[command]} hand-wires ${unratcheted.length} rule(s) the registry does not know ` + `about: ${unratcheted.join(', ')}.\n` + - `Add each to AUTHORING_RULES in packages/cli/src/lint/authoring-rules.ts so all three commands ` + + `Add each to AUTHORING_RULES in packages/lint/src/authoring-rules.ts so all three commands ` + `run it — or, if it genuinely is not a shared author-time rule (it needs the filesystem, the ` + `emitted artifact, or it belongs to os lint's own style rubric), add it to DIRECT_CALL_RATCHET ` + `in this file WITH the reason. Silence is the one option that is not available.`, @@ -289,6 +342,114 @@ describe('authoring-rule registry wiring (#4409)', () => { ).toEqual([]); }); + // ── The fourth door: the runtime publish gate (#4463) ──────────────── + // + // Everything above measures the three CLI commands against each other. They + // agreed with each other perfectly on the day #4463 was filed, and the + // metadata write path — the only door a Studio tenant, a REST `/meta` client + // or an MCP/AI author has — ran zero of the 26 rules. These four cases ask + // the question that state answers NO to. + + describe('runtime publish surface', () => { + it('every rule answers the surface question — runs there, or says why not', () => { + const silent = AUTHORING_RULES.filter((r) => !r.surfaces.includes('runtime-publish')) + .filter((r) => (r.surfaceReason ?? '').trim().length < 40) + .map((r) => r.name); + + expect( + silent, + `${silent.length} rule(s) neither run on the runtime publish gate nor record why: ` + + `${silent.join(', ')}.\n` + + `Set \`surfaces: CLI_AND_RUNTIME\` with \`runtimeTypes\`, or record a substantive ` + + `\`surfaceReason\`. Silence is what #4409 fixed on the command axis and what #4463 found ` + + `still true on the surface axis — a rule nobody decided about runs nowhere by default, and ` + + `the default door is the one every tenant uses.`, + ).toEqual([]); + }); + + it('every runtime-wired rule declares the metadata types it judges', () => { + const undeclared = AUTHORING_RULES.filter((r) => r.surfaces.includes('runtime-publish')) + .filter((r) => (r.runtimeTypes ?? []).length === 0) + .map((r) => r.name); + + expect( + undeclared, + `${undeclared.join(', ')} claim(s) the runtime surface without naming a metadata type. The ` + + `gate dispatches on the written item's type, so an empty \`runtimeTypes\` is a rule that ` + + `is wired and runs on nothing — the #4449 shape, one surface over.`, + ).toEqual([]); + }); + + it('every runtime-gated metadata type maps to a stack key', () => { + // Without a mapping the gate silently no-ops for that type: it would find + // the rules, build no snapshot, and return clean. Exactly the "looks + // wired, enforces nothing" state this whole file exists to make loud. + const unmapped = runtimeGatedTypes().filter((t) => stackKeyForType(t) === null); + expect( + unmapped, + `runtime-gated type(s) with no stack-key mapping in runtime-gate.ts: ${unmapped.join(', ')}. ` + + `The gate cannot build a snapshot for them, so the rules that declare them run on nothing.`, + ).toEqual([]); + }); + + it('the runtime gate consumes the registry and names no rule of its own', () => { + const path = join(repoRoot, RUNTIME_GATE_FILE); + expect(existsSync(path), `${RUNTIME_GATE_FILE} must exist — it IS the runtime surface`).toBe(true); + const source = readFileSync(path, 'utf8'); + + expect(source, `${RUNTIME_GATE_FILE} must run the shared core`).toMatch( + /\brunRuntimeAuthoringRules\s*\(/, + ); + + // The same subtraction the three commands get: a rule named at the gate + // is a rule that can drift from the table. + const handWired = ruleCallsIn(source).filter((name) => REGISTRY_NAMES.has(name)); + expect( + handWired, + `${RUNTIME_GATE_FILE} calls registry rule(s) directly: ${handWired.join(', ')}.\n` + + `The runtime gate must reach them ONLY through runRuntimeAuthoringRules(). Hand-wiring one ` + + `here rebuilds, on a fourth surface, the exact drift #3583 → #4409 took five repairs to end.`, + ).toEqual([]); + + // And it must not reach past the kernel-safe entry: `@objectstack/lint`'s + // root barrel pulls the react/jsx rules' module graph, which is the one + // thing the boot path may not name (`lazy-deps.test.ts`). + expect( + source, + `${RUNTIME_GATE_FILE} must import from '@objectstack/lint/runtime', not the root barrel — ` + + `the root entry reaches the typescript/sucrase rules the kernel boot path must not name.`, + ).not.toMatch(/from\s*['"]@objectstack\/lint['"]/); + }); + + it('the runtime gate and the CLI commands read the SAME array', () => { + // The property the issue asked to be provable: delete a rule from + // AUTHORING_RULES and BOTH sides lose it in the same commit. Asserted by + // identity of membership, not by an import statement — an import proves a + // module was loaded, never that the rule set came from it. + for (const rule of AUTHORING_RULES) { + if (!rule.surfaces.includes('runtime-publish')) continue; + for (const type of rule.runtimeTypes ?? []) { + expect( + runtimeAuthoringRulesFor(type).map((r) => r.name), + `${rule.name} declares runtime type '${type}' but the runtime gate does not run it`, + ).toContain(rule.name); + } + expect( + authoringRulesFor('build').map((r) => r.name), + `${rule.name} runs at the runtime publish gate but not on os build — the two publish verbs ` + + `must not disagree`, + ).toContain(rule.name); + } + + // Non-vacuous: #4463's worked example really is on both sides. + expect(runtimeGatedTypes()).toContain('flow'); + expect(runtimeAuthoringRulesFor('flow').map((r) => r.name)).toContain('validateApprovalApprovers'); + expect(runtimeAuthoringRulesFor('flow').map((r) => r.name)).toContain('validateStackExpressions'); + // A type nobody gated returns nothing rather than everything. + expect(runtimeAuthoringRulesFor('translation')).toEqual([]); + }); + }); + it('every rule declares a source file that exists', () => { const missing = AUTHORING_RULES.filter((r) => !existsSync(join(repoRoot, r.source))).map( (r) => `${r.name} → ${r.source}`, @@ -329,6 +490,7 @@ describe('authoring-rule registry wiring (#4409)', () => { it('every rule @objectstack/lint exports is wired into a registry', () => { const unwired = exportedLintRules() .filter((name) => !REGISTRY_NAMES.has(name)) + .filter((name) => !ratchetedElsewhere(name)) .filter((name) => !(name in UNWIRED_RULE_LEDGER)); expect( @@ -337,7 +499,7 @@ describe('authoring-rule registry wiring (#4409)', () => { `${unwired.join(', ')}.\n` + `A rule on the public export surface reads — to a human and to an AI author alike — as a ` + `check the platform performs. Either register it in AUTHORING_RULES ` + - `(packages/cli/src/lint/authoring-rules.ts) so all three commands run it, or add it to ` + + `(packages/lint/src/authoring-rules.ts) so all three commands run it, or add it to ` + `UNWIRED_RULE_LEDGER in this file WITH the real consumer that justifies it — or delete it ` + `under ADR-0049 enforce-or-remove. Advertising it while running it nowhere is the one option ` + `that is not available (Prime Directive #10).`, @@ -404,6 +566,12 @@ describe('authoring-rule registry wiring (#4409)', () => { expect(REFERENCE_INTEGRITY_RULES.length).toBeGreaterThan(0); // The rule whose absence from `os lint` motivated the suite's own guard. expect(REFERENCE_INTEGRITY_RULES.map((r) => r.name)).toContain('validateReadonlyFlowWrites'); + // #4463: every entry declares `cli`, and at least one declares the runtime + // gate — a table where nothing does would pass every surface case above by + // vacuity, which is precisely the state the issue was filed about. + expect(AUTHORING_SURFACES).toEqual(['cli', 'runtime-publish']); + expect(AUTHORING_RULES.every((r) => r.surfaces.includes('cli'))).toBe(true); + expect(AUTHORING_RULES.filter((r) => r.surfaces.includes('runtime-publish')).length).toBeGreaterThan(0); }); it('the source scans still match something (non-vacuous)', () => { diff --git a/packages/cli/src/lint/authoring-rules.ts b/packages/lint/src/authoring-rules.ts similarity index 65% rename from packages/cli/src/lint/authoring-rules.ts rename to packages/lint/src/authoring-rules.ts index 4d927a3471..ac6b36d03c 100644 --- a/packages/cli/src/lint/authoring-rules.ts +++ b/packages/lint/src/authoring-rules.ts @@ -1,9 +1,14 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * The author-time rule registry — WHICH rules `os validate`, `os build` and - * `os lint` run, declared as data, with a written reason for every narrowing - * (#4409). + * The author-time rule registry — WHICH rules each authoring surface runs, + * declared as data, with a written reason for every narrowing (#4409, #4463). + * + * Lives in `@objectstack/lint` (not the CLI) since #4463: the CLI is one + * CONSUMER of this table, not its home. The runtime metadata write path + * (`saveMetaItem` / `publishMetaItem` — the Studio, REST `/meta` and MCP door) + * is the other, and it reads the same array through `./runtime.js`. That is the + * whole point: a shared core plus N gates, never N rule engines. * * ## Why this exists * @@ -26,7 +31,7 @@ * repair removed an instance and left the MODE — a rule's command coverage was * whatever its author remembered to type, and forgetting was silent. This file * replaces "remembering" with a table, and the guard in - * `commands/authoring-rule-wiring.test.ts` makes a narrowing an explicit, + * `authoring-rule-wiring.test.ts` makes a narrowing an explicit, * reasoned edit instead of an omission. * * ## The invariant @@ -35,6 +40,16 @@ * as strong as the weakest command an author or CI happens to run, so a gating * rule with partial coverage is not a stricter check — it is a coin flip. * + * #4463 is the same sentence one layer out: the three commands are three doors + * in ONE wall, and the runtime metadata write path is a fourth wall with no + * door at all. A tenant editing in Studio, a REST `/meta` PUT and an MCP/AI + * author all reach `saveMetaItem`, which ran a per-type Zod `safeParse` and + * nothing else — so the very stack `os build` refuses walked in through the + * only door most tenants have. {@link AuthoringRule.surfaces} is what makes + * "which rules run at that door?" a table entry rather than a second wiring + * list, and `authoring-rule-wiring.test.ts` ratchets it exactly as it ratchets + * the three commands. + * * An `advisory` rule (never emits `error`) MAY be scoped to fewer commands, but * only with a `scopeReason` recorded here. The distinction that matters is not * cost, it is consequence: a missing advisory costs the author a hint, a missing @@ -55,6 +70,13 @@ * fails on a direct import, because that is precisely how a rule ends up running * on two commands out of three. * + * Then answer the fourth-door question in the same edit: does it belong on + * `surfaces: ['cli', 'runtime-publish']`? Say yes (with `runtimeTypes`) or say + * why not (with `surfaceReason`). There is no third option — the guard rejects + * an entry that answers neither, which is the whole mechanism: #4409 was + * "nobody wrote down which commands", #4463 was "nobody wrote down which + * doors", and both were invisible until someone measured. + * * ## What is NOT in here * * This registry covers the rules the three commands SHARE. Two neighbouring @@ -72,36 +94,37 @@ * metadata rules, and each is wired where its input exists. */ -import { - validateStackExpressions, - validateListViewMode, - validateFunctionalCompleteness, - validateViewContainers, - validateWidgetBindings, - validateDashboardActionRefs, - validateFilterTokens, - validateReferenceIntegrity, - validateResponsiveStyles, - validateJsxPages, - validateReactPages, - validatePageSourceStyling, - validateCapabilityReferences, - validateFlowTriggerReadiness, - validateApprovalApprovers, - validateRecordTitle, - validateSemanticRoles, - validateFormLayout, - validateSeedReplaySafety, - validateSeedStateMachine, - validateVisibilityPredicates, - validateSecurityPosture, - validateOrgAxisRedLines, - validateActionLocations, -} from '@objectstack/lint'; -import { lintFlowPatterns } from '../utils/lint-flow-patterns.js'; -import { lintLivenessProperties } from '../utils/lint-liveness-properties.js'; -import { lintAutonumberFormats } from '../utils/lint-autonumber-formats.js'; -import { lintViewRefs } from '../utils/lint-view-refs.js'; +// Imported per-module, never through `./index.js`: the barrel would make this +// file a cycle partner of its own package entry, and the runtime surface +// (`./runtime.js`) needs a graph it can reason about rule by rule. +import { validateStackExpressions } from './validate-expressions.js'; +import { validateListViewMode } from './validate-list-view-mode.js'; +import { validateFunctionalCompleteness } from './validate-functional-completeness.js'; +import { validateViewContainers } from './validate-view-containers.js'; +import { validateWidgetBindings } from './validate-widget-bindings.js'; +import { validateDashboardActionRefs } from './validate-dashboard-action-refs.js'; +import { validateFilterTokens } from './validate-filter-tokens.js'; +import { validateReferenceIntegrity } from './reference-integrity-suite.js'; +import { validateResponsiveStyles } from './validate-responsive-styles.js'; +import { validateJsxPages } from './validate-jsx-pages.js'; +import { validateReactPages } from './validate-react-pages.js'; +import { validatePageSourceStyling } from './validate-page-source-styling.js'; +import { validateCapabilityReferences } from './validate-capability-references.js'; +import { validateFlowTriggerReadiness } from './validate-flow-trigger-readiness.js'; +import { validateApprovalApprovers } from './validate-approval-approvers.js'; +import { validateRecordTitle } from './validate-record-title.js'; +import { validateSemanticRoles } from './validate-semantic-roles.js'; +import { validateFormLayout } from './validate-form-layout.js'; +import { validateSeedReplaySafety } from './validate-seed-replay-safety.js'; +import { validateSeedStateMachine } from './validate-seed-state-machine.js'; +import { validateVisibilityPredicates } from './validate-visibility-predicates.js'; +import { validateSecurityPosture } from './validate-security-posture.js'; +import { validateOrgAxisRedLines } from './validate-org-axis-red-lines.js'; +import { validateActionLocations } from './validate-action-locations.js'; +import { lintFlowPatterns } from './lint-flow-patterns.js'; +import { lintLivenessProperties } from './lint-liveness-properties.js'; +import { lintAutonumberFormats } from './lint-autonumber-formats.js'; +import { lintViewRefs } from './lint-view-refs.js'; import { lintUniqueDeclarations } from './data-model-rules.js'; type AnyRec = Record; @@ -112,6 +135,27 @@ type AnyRec = Record; export const AUTHORING_COMMANDS = ['validate', 'build', 'lint'] as const; export type AuthoringCommand = (typeof AUTHORING_COMMANDS)[number]; +/** + * The authoring SURFACES this registry serves (#4463). + * + * - `cli` — `os validate` / `os build` / `os lint`. Which of the three is a + * separate axis ({@link AuthoringRule.commands}); every rule here runs on the + * `cli` surface, because that is where the registry was born. + * - `runtime-publish` — the metadata write path's PUBLISH gate: a + * `state: 'active'` `saveMetaItem`, and the draft→active promotion in + * `publishMetaItem`. Studio saves, REST `/meta` item CRUD and MCP/AI + * authoring all funnel through those two, so wiring the surface once covers + * all three doors (the maintainer ruling on #4463: one shared core, one + * runtime gate, not a gate per surface). + * + * Draft saves are deliberately NOT a surface: a draft is allowed to be a + * half-finished thing, and gating one would break the Studio editing loop for + * no safety gain — the draft cannot execute until it is published, and + * publishing runs this table (#4463 D1). + */ +export const AUTHORING_SURFACES = ['cli', 'runtime-publish'] as const; +export type AuthoringSurface = (typeof AUTHORING_SURFACES)[number]; + /** `error` gates. `warning` advises. `info` is a suggestion (`os lint` grades it as one). */ export type AuthoringSeverity = 'error' | 'warning' | 'info'; @@ -179,12 +223,82 @@ export interface AuthoringRule { source: string; /** REQUIRED when `commands` is not all three: why this rule is scoped. */ scopeReason?: string; + /** + * Which authoring surfaces run this rule (#4463). Always contains `'cli'`. + * + * Adding `'runtime-publish'` is what puts the rule on the metadata write + * path's publish gate; {@link runtimeTypes} then says which metadata types' + * writes it inspects. Leaving it off REQUIRES {@link surfaceReason} — the + * same discipline `scopeReason` applies to the command axis, for the same + * reason: the whole defect class #4409 and #4463 describe is coverage that + * narrowed silently. + */ + surfaces: readonly AuthoringSurface[]; + /** + * REQUIRED when `surfaces` includes `'runtime-publish'`: the SINGULAR + * metadata type names whose runtime write this rule inspects (e.g. `flow`). + * + * The runtime gate builds a per-write stack snapshot and only runs the rules + * that declare the written type, which is #4463 D2 option (c) expressed as + * data. Widening a rule to another type is a one-line edit here, not new + * wiring at the gate. + */ + runtimeTypes?: readonly string[]; + /** REQUIRED when `surfaces` omits `'runtime-publish'`: why the runtime gate does not run it. */ + surfaceReason?: string; run: (stack: AnyRec, ctx: AuthoringRuleContext) => readonly AuthoringFinding[]; } /** Every command runs every rule unless an entry says otherwise. */ const ALL: readonly AuthoringCommand[] = AUTHORING_COMMANDS; +/** The CLI-only surface set — the default for a rule the runtime gate does not (yet) run. */ +const CLI_ONLY: readonly AuthoringSurface[] = ['cli']; +/** Runs on both the three CLI commands and the runtime publish gate. */ +const CLI_AND_RUNTIME: readonly AuthoringSurface[] = ['cli', 'runtime-publish']; + +// ─── Why a rule is not on the runtime publish gate ────────────────── +// +// #4463's P1 slice deliberately wires ONE metadata type (`flow`) and the four +// rule families the issue named (flow / approval / expression / reference). +// Every other rule carries one of the reasons below rather than silence. They +// are shared constants because the reason really is the same for a group of +// rules — writing twenty near-identical sentences would hide which ones differ. + +/** + * The rule reads a stack-wide COLLECTION the per-write snapshot does not carry + * (pages, dashboards, navigation, translations, seeds, permission sets). The + * runtime universe can answer these — it is strictly more complete than the + * CLI's single-package view — but building that snapshot is #4463 P2, and + * shipping the rule against a partial snapshot would invent findings for + * metadata the tenant simply did not include in THIS write. A false 422 on the + * only door a Studio tenant has is worse than the gap it would close. + */ +const RUNTIME_NEEDS_FULL_SNAPSHOT = + 'P2 (#4463): reads a stack-wide collection the per-write snapshot does not carry, so running it ' + + 'now would report the rest of the tenant\'s metadata as missing rather than judging this write.'; + +/** + * The rule parses authored SOURCE (react/jsx page bodies, L2 JS hook/action + * bodies) through `typescript` / `sucrase`. Those are exactly the dependencies + * `lazy-deps.test.ts` keeps off the kernel boot path, and `@objectstack/lint`'s + * runtime entry is guarded to load neither. Studio's page editor has its own + * save-time compile path; this gate is not where that check belongs. + */ +const RUNTIME_HEAVY_SOURCE_PARSE = + 'Not runtime-safe: parses authored source through typescript/sucrase, the two dependencies the ' + + 'kernel boot path must never load (lazy-deps.test.ts). Studio compiles page source on its own path.'; + +/** + * The rule judges an OBJECT/field declaration. Object writes are the hottest + * metadata path there is (every Studio field edit) and the blast radius of a + * wrong 422 there is the whole product, so P1 does not gate them — the issue's + * own worked example, and every acceptance criterion on it, is a flow. + */ +const RUNTIME_OBJECT_WRITES_P2 = + 'P2 (#4463): judges an object/field declaration. Object writes are the hottest metadata path in ' + + 'the product, so P1 gates `flow` first and widens once the gate has real traffic behind it.'; + /** * `ExprIssue` is the one rule finding that carries no rule id of its own — it * predates the `{ rule, path, hint }` shape every other rule settled on. Given @@ -209,6 +323,12 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ input: 'parsed', commands: ALL, source: 'packages/lint/src/validate-expressions.ts', + // Runtime publish gate (#4463): the EXPRESSION family. On a flow write it + // checks every script-node callable and every declared predicate the flow + // carries, against the live object universe — the same parse `os build` + // runs, now at the door Studio/REST/MCP authors actually use. + surfaces: CLI_AND_RUNTIME, + runtimeTypes: ['flow'], run: (stack) => validateStackExpressions(stack).map((i) => ({ severity: i.severity ?? 'error', @@ -228,6 +348,8 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ input: 'normalized', commands: ALL, source: 'packages/lint/src/validate-list-view-mode.ts', + surfaces: CLI_ONLY, + surfaceReason: RUNTIME_NEEDS_FULL_SNAPSHOT, run: (stack) => validateListViewMode(stack), }, // [ADR-0078] A Zod-VALID instance that silently does nothing: a `summary` @@ -246,6 +368,8 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ input: 'normalized', commands: ALL, source: 'packages/lint/src/validate-functional-completeness.ts', + surfaces: CLI_ONLY, + surfaceReason: RUNTIME_OBJECT_WRITES_P2, run: (stack) => validateFunctionalCompleteness(stack), }, // A flat list-view object in `views: []` parses to an EMPTY container @@ -257,6 +381,8 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ input: 'normalized', commands: ALL, source: 'packages/lint/src/validate-view-containers.ts', + surfaces: CLI_ONLY, + surfaceReason: RUNTIME_NEEDS_FULL_SNAPSHOT, run: (stack) => validateViewContainers(stack), }, // ADR-0021 (#1719/#1721) — a widget's `dataset`/`dimensions`/`values` and its @@ -267,6 +393,8 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ input: 'parsed', commands: ALL, source: 'packages/lint/src/validate-widget-bindings.ts', + surfaces: CLI_ONLY, + surfaceReason: RUNTIME_NEEDS_FULL_SNAPSHOT, run: (stack) => validateWidgetBindings(stack), }, // ADR-0049 / #3367 — a header or widget action naming a `script`/`modal` @@ -278,6 +406,8 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ input: 'parsed', commands: ALL, source: 'packages/lint/src/validate-dashboard-action-refs.ts', + surfaces: CLI_ONLY, + surfaceReason: RUNTIME_NEEDS_FULL_SNAPSHOT, run: (stack) => validateDashboardActionRefs(stack), }, // #3574 — a filter value like `{current_user}` resolves in no vocabulary, @@ -290,6 +420,8 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ input: 'parsed', commands: ALL, source: 'packages/lint/src/validate-filter-tokens.ts', + surfaces: CLI_ONLY, + surfaceReason: RUNTIME_NEEDS_FULL_SNAPSHOT, run: (stack) => validateFilterTokens(stack), }, // The reference-integrity suite (#3583 §5 D5) — itself a registry, of the @@ -303,6 +435,15 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ input: 'parsed', commands: ALL, source: 'packages/lint/src/reference-integrity-suite.ts', + // Runtime publish gate (#4463): the REFERENCE family. On a flow snapshot the + // members that answer a flow question run (`validateFlowTemplatePaths`, + // `validateFlowNodeWrites`, `validateReadonlyFlowWrites`, + // `validateObjectReferences`); the page/nav/AI members see no such + // collection in the snapshot and return nothing, and the two that would + // load `typescript` need a hook/action/react body the snapshot never + // carries — which is what `runtime-lazy-deps.test.ts` pins. + surfaces: CLI_AND_RUNTIME, + runtimeTypes: ['flow'], run: (stack) => validateReferenceIntegrity(stack), }, // ADR-0065 — a styled node's responsiveStyles must be scopable (needs an @@ -313,6 +454,8 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ input: 'parsed', commands: ALL, source: 'packages/lint/src/validate-responsive-styles.ts', + surfaces: CLI_ONLY, + surfaceReason: RUNTIME_NEEDS_FULL_SNAPSHOT, run: (stack) => validateResponsiveStyles(stack), }, // ADR-0080 — a `kind:'jsx'` page's `source` is parsed (never executed) and @@ -324,6 +467,8 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ input: 'parsed', commands: ALL, source: 'packages/lint/src/validate-jsx-pages.ts', + surfaces: CLI_ONLY, + surfaceReason: RUNTIME_HEAVY_SOURCE_PARSE, run: (stack, ctx) => validateJsxPages(stack, ctx.sduiManifest ? { manifest: ctx.sduiManifest as never } : {}), }, @@ -336,6 +481,8 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ input: 'parsed', commands: ALL, source: 'packages/lint/src/validate-react-pages.ts', + surfaces: CLI_ONLY, + surfaceReason: RUNTIME_HEAVY_SOURCE_PARSE, run: (stack) => validateReactPages(stack), }, // ADR-0065, source tier — Tailwind `className` in a `kind:'html'`/`'react'` @@ -346,6 +493,8 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ input: 'parsed', commands: ALL, source: 'packages/lint/src/validate-page-source-styling.ts', + surfaces: CLI_ONLY, + surfaceReason: RUNTIME_NEEDS_FULL_SNAPSHOT, run: (stack) => validatePageSourceStyling(stack), }, // ADR-0066 ⑨ — a `requiredPermissions` entry naming a capability registered @@ -357,6 +506,11 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ input: 'parsed', commands: ALL, source: 'packages/lint/src/validate-capability-references.ts', + surfaces: CLI_ONLY, + surfaceReason: 'P2 (#4463): the ONE rule the runtime universe makes strictly stronger — the advisory hedge ("another ' + + 'installed package may provide it") is decidable against the live capability registry, so it ' + + 'graduates from advisory to gating there rather than merely being ported. That promotion is a ' + + 'severity change on a published rule id and belongs in its own PR, not riding a wiring change.', run: (stack) => validateCapabilityReferences(stack), }, // A record-change flow whose start-node objectName matches nothing never @@ -368,6 +522,11 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ input: 'normalized', commands: ALL, source: 'packages/lint/src/validate-flow-trigger-readiness.ts', + // Runtime publish gate (#4463): the FLOW family. Advisory at this surface + // too — its findings are logged, not thrown (P1 gates on `error` only; P2 + // puts advisories on the response for Studio to render). + surfaces: CLI_AND_RUNTIME, + runtimeTypes: ['flow'], run: (stack) => validateFlowTriggerReadiness(stack), }, // ADR-0090 D3 fallout — an approval `{ type: 'role' }` resolves against the @@ -382,6 +541,12 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ input: 'parsed', commands: ALL, source: 'packages/lint/src/validate-approval-approvers.ts', + // Runtime publish gate (#4463): the APPROVAL family, and the issue's worked + // example both times. #4409 fixed it for `os build`; a tenant saving the + // same broken expression approver from Studio still sailed through, because + // `approver.value` is just a string to Zod. It is not just a string here. + surfaces: CLI_AND_RUNTIME, + runtimeTypes: ['flow'], run: (stack) => validateApprovalApprovers(stack), }, // ADR-0079 — `titleFormat` is retired in favour of `nameField`, and an object @@ -393,6 +558,8 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ input: 'parsed', commands: ALL, source: 'packages/lint/src/validate-record-title.ts', + surfaces: CLI_ONLY, + surfaceReason: RUNTIME_OBJECT_WRITES_P2, run: (stack) => validateRecordTitle(stack), }, // ADR-0085 — `stageField` / `highlightFields` / `Field.group` are pointers @@ -404,6 +571,8 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ input: 'parsed', commands: ALL, source: 'packages/lint/src/validate-semantic-roles.ts', + surfaces: CLI_ONLY, + surfaceReason: RUNTIME_OBJECT_WRITES_P2, run: (stack) => validateSemanticRoles(stack), }, // #2578 / #4449 — a form section's field reference that resolves to nothing @@ -419,6 +588,8 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ input: 'parsed', commands: ALL, source: 'packages/lint/src/validate-form-layout.ts', + surfaces: CLI_ONLY, + surfaceReason: RUNTIME_NEEDS_FULL_SNAPSHOT, run: (stack) => validateFormLayout(stack), }, // ADR-0078 Phase 3 (Tier-A `action-locations`) — an action that declares no @@ -433,6 +604,8 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ input: 'parsed', commands: ALL, source: 'packages/lint/src/validate-action-locations.ts', + surfaces: CLI_ONLY, + surfaceReason: RUNTIME_NEEDS_FULL_SNAPSHOT, run: (stack) => validateActionLocations(stack), }, // framework#3434 — seeds replay on every boot, so a `mode: 'insert'` dataset @@ -443,6 +616,8 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ input: 'parsed', commands: ALL, source: 'packages/lint/src/validate-seed-replay-safety.ts', + surfaces: CLI_ONLY, + surfaceReason: RUNTIME_NEEDS_FULL_SNAPSHOT, run: (stack) => validateSeedReplaySafety(stack), }, // framework#3433 follow-up — #3433 exempts seed writes from the @@ -455,6 +630,8 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ input: 'parsed', commands: ALL, source: 'packages/lint/src/validate-seed-state-machine.ts', + surfaces: CLI_ONLY, + surfaceReason: RUNTIME_NEEDS_FULL_SNAPSHOT, run: (stack) => validateSeedStateMachine(stack), }, // ADR-0089 D3b — deprecated visibility aliases and a mis-layered binding root. @@ -466,6 +643,8 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ input: 'normalized', commands: ALL, source: 'packages/lint/src/validate-visibility-predicates.ts', + surfaces: CLI_ONLY, + surfaceReason: RUNTIME_NEEDS_FULL_SNAPSHOT, run: (stack) => validateVisibilityPredicates(stack), }, // #1874 — flow authoring anti-patterns. Advisory by default; a finding marked @@ -479,7 +658,13 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ tier: 'gating', input: 'parsed', commands: ALL, - source: 'packages/cli/src/utils/lint-flow-patterns.ts', + source: 'packages/lint/src/lint-flow-patterns.ts', + // Runtime publish gate (#4463): the FLOW family's anti-pattern half. Its + // three `error` rules are the sharpest fit for a publish gate that exists — + // `flow-runas-unscoped` is metadata the automation engine REFUSES to + // execute, so publishing it can only ever produce a broken flow. + surfaces: CLI_AND_RUNTIME, + runtimeTypes: ['flow'], run: (stack) => lintFlowPatterns(stack).map((f) => ({ severity: f.severity ?? 'warning', @@ -499,7 +684,9 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ tier: 'advisory', input: 'parsed', commands: ALL, - source: 'packages/cli/src/utils/lint-liveness-properties.ts', + source: 'packages/lint/src/lint-liveness-properties.ts', + surfaces: CLI_ONLY, + surfaceReason: RUNTIME_OBJECT_WRITES_P2, run: (stack) => lintLivenessProperties(stack).map((f) => ({ severity: 'warning' as const, @@ -518,7 +705,9 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ tier: 'gating', input: 'parsed', commands: ALL, - source: 'packages/cli/src/utils/lint-autonumber-formats.ts', + source: 'packages/lint/src/lint-autonumber-formats.ts', + surfaces: CLI_ONLY, + surfaceReason: RUNTIME_OBJECT_WRITES_P2, run: (stack) => lintAutonumberFormats(stack).map((f) => ({ severity: f.severity, @@ -537,7 +726,9 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ tier: 'gating', input: 'parsed', commands: ALL, - source: 'packages/cli/src/utils/lint-view-refs.ts', + source: 'packages/lint/src/lint-view-refs.ts', + surfaces: CLI_ONLY, + surfaceReason: RUNTIME_NEEDS_FULL_SNAPSHOT, run: (stack) => lintViewRefs(stack).map((f) => ({ severity: f.severity, @@ -556,7 +747,9 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ tier: 'advisory', input: 'parsed', commands: ['validate', 'build'], - source: 'packages/cli/src/lint/data-model-rules.ts', + source: 'packages/lint/src/data-model-rules.ts', + surfaces: CLI_ONLY, + surfaceReason: RUNTIME_OBJECT_WRITES_P2, scopeReason: "`os lint` already reports this rule through `lintDataModel`, which calls it directly as R10 of " + 'its best-practice sweep — registering it for `lint` as well would report every finding twice. ' + @@ -581,6 +774,11 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ input: 'parsed', commands: ALL, source: 'packages/lint/src/validate-security-posture.ts', + surfaces: CLI_ONLY, + surfaceReason: 'Already gated at this surface by a DIFFERENT mechanism: plugin-security registers an ADR-0094 ' + + 'authoring gate on `object` (`registerAuthoringGate`) that enforces the same OWD posture rules on ' + + 'every runtime write. Running the linter here as well would double-report one refusal in two ' + + 'vocabularies. Consolidating the two onto this table is P2 (#4463), and is a merge, not a hole.', run: (stack) => validateSecurityPosture(stack), }, // ADR-0105 D6 — the org tree is a REPORTING dimension. An RLS policy or @@ -593,6 +791,8 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ input: 'parsed', commands: ALL, source: 'packages/lint/src/validate-org-axis-red-lines.ts', + surfaces: CLI_ONLY, + surfaceReason: RUNTIME_NEEDS_FULL_SNAPSHOT, run: (stack) => validateOrgAxisRedLines(stack), }, ]; diff --git a/packages/cli/src/lint/data-model-rules.ts b/packages/lint/src/data-model-rules.ts similarity index 100% rename from packages/cli/src/lint/data-model-rules.ts rename to packages/lint/src/data-model-rules.ts diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index 3bc31b9758..eed219cb90 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -348,3 +348,95 @@ export type { } from './reference-integrity-suite.js'; export { buildAccessMatrix, diffAccessMatrix } from './build-access-matrix.js'; + +// ─── Rules relocated from `@objectstack/cli` (#4463) ───────────────── +// +// These five lived in `packages/cli/src/{utils,lint}/` while the registry that +// runs them lived beside them. That was fine while the CLI was the only +// consumer; it stopped being fine the moment the runtime write path had to run +// the SAME table, because the kernel cannot depend on the CLI. They moved here +// — the package the rules always belonged in — so `authoring-rules.ts` can live +// here too and both surfaces read one array. The CLI now imports them from this +// barrel; no rule logic changed in the move. + +export { lintFlowPatterns } from './lint-flow-patterns.js'; +export type { FlowLintFinding } from './lint-flow-patterns.js'; +export { + FLOW_TIME_RELATIVE_ANTIPATTERN, + FLOW_DATE_EQUALITY_FILTER, + FLOW_PHANTOM_AGGREGATION, + FLOW_DOUBLE_BRACE_INTERP, + FLOW_BARE_DOLLAR_REF, + FLOW_APPROVAL_REVISE_DEAD_END, + FLOW_APPROVAL_REVISE_UNMARKED_BACKEDGE, + FLOW_APPROVAL_REVISE_DISABLED, + FLOW_RUNAS_UNSCOPED, + FLOW_ERROR_LABEL_NOT_FAULT, + FLOW_BRANCH_LABEL_UNMATCHED, + FLOW_DECISION_UNCONDITIONAL_BRANCH, + FLOW_DEFAULT_EDGE_WITH_CONDITION, + FLOW_MULTIPLE_DEFAULT_EDGES, + FLOW_INERT_NODE_CONDITION, +} from './lint-flow-patterns.js'; + +export { lintLivenessProperties } from './lint-liveness-properties.js'; +export type { LivenessLintFinding } from './lint-liveness-properties.js'; +export { LIVENESS_DEAD_PROPERTY, LIVENESS_EXPERIMENTAL_PROPERTY } from './lint-liveness-properties.js'; + +export { lintAutonumberFormats } from './lint-autonumber-formats.js'; +export type { AutonumberLintFinding } from './lint-autonumber-formats.js'; +export { + AUTONUMBER_UNKNOWN_FIELD, + AUTONUMBER_OPTIONAL_FIELD, + AUTONUMBER_SELF_REFERENCE, + AUTONUMBER_LITERAL_TOKEN, +} from './lint-autonumber-formats.js'; + +export { lintViewRefs } from './lint-view-refs.js'; +export type { ViewRefFinding } from './lint-view-refs.js'; +export { + VIEW_KEY_COLLISION, + VIEW_REF_FORM_TARGET_MISSING, + VIEW_REF_FORM_TARGET_KIND, +} from './lint-view-refs.js'; + +export { lintUniqueDeclarations, lintDataModel, UNIQUE_DOUBLE_DECLARATION } from './data-model-rules.js'; +export type { LintIssue, Severity } from './data-model-rules.js'; + +// ─── The registry itself (#4409, relocated #4463) ──────────────────── +// +// The single source of truth for WHICH rules run WHERE. `os validate`, +// `os build`, `os lint` and the runtime metadata publish gate all read this +// one array — see `authoring-rules.ts` for why any second list is a bug. + +export { + AUTHORING_RULES, + AUTHORING_COMMANDS, + AUTHORING_SURFACES, + EXPRESSION_INVALID, + authoringRulesFor, + runAuthoringRules, + splitBySeverity, +} from './authoring-rules.js'; +export type { + AuthoringCommand, + AuthoringFinding, + AuthoringRule, + AuthoringRuleContext, + AuthoringRuleInputTier, + AuthoringRuleRun, + AuthoringRuleTier, + AuthoringSeverity, + AuthoringSurface, +} from './authoring-rules.js'; + +// The runtime publish gate over that registry. Also published as the +// `@objectstack/lint/runtime` subpath — the entry the kernel boot path imports, +// so a consumer there never names the graph that reaches the source parsers. +export { + runRuntimeAuthoringRules, + runtimeAuthoringRulesFor, + runtimeGatedTypes, + stackKeyForType, +} from './runtime-gate.js'; +export type { RuntimeGateResult, RuntimeStackContext } from './runtime-gate.js'; diff --git a/packages/cli/src/utils/lint-autonumber-formats.test.ts b/packages/lint/src/lint-autonumber-formats.test.ts similarity index 100% rename from packages/cli/src/utils/lint-autonumber-formats.test.ts rename to packages/lint/src/lint-autonumber-formats.test.ts diff --git a/packages/cli/src/utils/lint-autonumber-formats.ts b/packages/lint/src/lint-autonumber-formats.ts similarity index 100% rename from packages/cli/src/utils/lint-autonumber-formats.ts rename to packages/lint/src/lint-autonumber-formats.ts diff --git a/packages/cli/src/utils/lint-flow-patterns.test.ts b/packages/lint/src/lint-flow-patterns.test.ts similarity index 100% rename from packages/cli/src/utils/lint-flow-patterns.test.ts rename to packages/lint/src/lint-flow-patterns.test.ts diff --git a/packages/cli/src/utils/lint-flow-patterns.ts b/packages/lint/src/lint-flow-patterns.ts similarity index 100% rename from packages/cli/src/utils/lint-flow-patterns.ts rename to packages/lint/src/lint-flow-patterns.ts diff --git a/packages/cli/src/utils/lint-liveness-properties.test.ts b/packages/lint/src/lint-liveness-properties.test.ts similarity index 100% rename from packages/cli/src/utils/lint-liveness-properties.test.ts rename to packages/lint/src/lint-liveness-properties.test.ts diff --git a/packages/cli/src/utils/lint-liveness-properties.ts b/packages/lint/src/lint-liveness-properties.ts similarity index 100% rename from packages/cli/src/utils/lint-liveness-properties.ts rename to packages/lint/src/lint-liveness-properties.ts diff --git a/packages/cli/src/utils/lint-view-refs.test.ts b/packages/lint/src/lint-view-refs.test.ts similarity index 100% rename from packages/cli/src/utils/lint-view-refs.test.ts rename to packages/lint/src/lint-view-refs.test.ts diff --git a/packages/cli/src/utils/lint-view-refs.ts b/packages/lint/src/lint-view-refs.ts similarity index 100% rename from packages/cli/src/utils/lint-view-refs.ts rename to packages/lint/src/lint-view-refs.ts diff --git a/packages/lint/src/runtime-gate.test.ts b/packages/lint/src/runtime-gate.test.ts new file mode 100644 index 0000000000..16e01cf241 --- /dev/null +++ b/packages/lint/src/runtime-gate.test.ts @@ -0,0 +1,198 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// Behaviour of the runtime publish gate (#4463). +// +// The wiring guard (`authoring-rule-wiring.test.ts`) proves the gate READS the +// shared registry. This file proves it FINDS things — the distinction #4449 +// was filed about, where a rule was registered, exported, unit-tested and +// produced output on no real stack. + +import { describe, it, expect } from 'vitest'; +import { + runRuntimeAuthoringRules, + runtimeAuthoringRulesFor, + runtimeGatedTypes, + stackKeyForType, +} from './runtime-gate.js'; + +/** The issue's measured example: an approval flow whose expression approver is broken CEL. */ +const brokenApprovalFlow = { + name: 'leave_approval', + label: 'Leave approval', + trigger: { type: 'record_change', object: 'leave_request', events: ['create'] }, + nodes: [ + { id: 'start', type: 'start' }, + { + id: 'approve', + type: 'approval', + config: { approvers: [{ type: 'expression', value: 'record.owner ==' }] }, + }, + ], +}; + +/** The same flow with an approver expression that parses and uses a legal root. */ +const cleanApprovalFlow = { + ...brokenApprovalFlow, + nodes: [ + { id: 'start', type: 'start' }, + { + id: 'approve', + type: 'approval', + config: { + approvers: [{ type: 'expression', value: 'current.owner' }], + emptyApproverPolicy: 'reject', + }, + }, + ], +}; + +const objects = [ + { name: 'leave_request', fields: { owner: { type: 'text' }, status: { type: 'text' } } }, +]; + +describe('runtime publish gate (#4463)', () => { + it('rejects the issue\'s worked example with a located, fixable finding', () => { + const result = runRuntimeAuthoringRules({ + type: 'flow', + item: brokenApprovalFlow, + context: { objects }, + }); + + const finding = result.errors.find((f) => f.rule === 'approval-expression-invalid'); + expect( + finding, + `the broken CEL approver must be REFUSED at the runtime publish gate — this exact body is what ` + + `#4409 taught the three CLI commands to reject, and what a Studio tenant could still save.`, + ).toBeDefined(); + + // D3: the four keys the 422 envelope carries. A gate that says "no" without + // saying WHERE and HOW TO FIX is a gate an author routes around. + expect(finding!.path).toBe('flows[0].nodes[1].config.approvers[0].value'); + expect(finding!.where).toContain('leave_approval'); + expect(finding!.message).toMatch(/does not parse as CEL/); + expect(finding!.hint.length).toBeGreaterThan(10); + expect(finding!.severity).toBe('error'); + }); + + it('passes the same flow once the expression is valid', () => { + const result = runRuntimeAuthoringRules({ + type: 'flow', + item: cleanApprovalFlow, + context: { objects }, + }); + expect(result.errors, `a valid flow must publish: ${JSON.stringify(result.errors)}`).toEqual([]); + // The rules still RAN — "clean" and "nothing ran" must be distinguishable. + expect(result.rulesRun.length).toBeGreaterThan(0); + }); + + it('runs the four rule families #4463 P1 wired, from the shared table', () => { + const { rulesRun } = runRuntimeAuthoringRules({ + type: 'flow', + item: cleanApprovalFlow, + context: { objects }, + }); + expect(rulesRun).toEqual(runtimeAuthoringRulesFor('flow').map((r) => r.name)); + expect(rulesRun).toContain('validateStackExpressions'); // expression + expect(rulesRun).toContain('validateApprovalApprovers'); // approval + expect(rulesRun).toContain('validateFlowTriggerReadiness'); // flow + expect(rulesRun).toContain('lintFlowPatterns'); // flow + expect(rulesRun).toContain('validateReferenceIntegrity'); // reference + }); + + it('a rule that gates only `error` leaves warnings out of the blocking set', () => { + // The empty-approver-policy advisory rides along on both flows above. It + // must never be a reason to refuse a publish (P1 gates on `error`; P2 puts + // advisories on the response). + const result = runRuntimeAuthoringRules({ + type: 'flow', + item: brokenApprovalFlow, + context: { objects }, + }); + expect(result.advisories.every((f) => f.severity !== 'error')).toBe(true); + expect(result.errors.every((f) => f.severity === 'error')).toBe(true); + }); + + // ── D4: only this write is judged ──────────────────────────────────── + + it('does not blame a write for a PRE-EXISTING violation in the context', () => { + // A tenant's stored object carries a validation rule with broken CEL — + // written before the gate existed, and legal to keep reading (ADR-0087's + // asymmetry). Publishing an unrelated flow must not 422 because of it. + const brokenContext = [ + { + name: 'leave_request', + fields: { owner: { type: 'text' } }, + validationRules: [{ name: 'bad', expression: 'record.owner ==', message: 'x' }], + }, + ]; + + const result = runRuntimeAuthoringRules({ + type: 'flow', + item: cleanApprovalFlow, + context: { objects: brokenContext }, + }); + + expect( + result.errors, + `the gate blocks NEW writes only (#4463 D4). A stored row's pre-existing violation showing up ` + + `as an error on somebody else's publish would make the tenant's metadata un-editable, which ` + + `is a worse outcome than the hole this gate closes.`, + ).toEqual([]); + }); + + it('still catches the write when the context is ALSO broken', () => { + // The subtraction must not swallow a real finding just because the context + // is noisy: the fingerprints differ, so the flow's own error survives. + const brokenContext = [ + { + name: 'leave_request', + fields: { owner: { type: 'text' } }, + validationRules: [{ name: 'bad', expression: 'record.owner ==', message: 'x' }], + }, + ]; + const result = runRuntimeAuthoringRules({ + type: 'flow', + item: brokenApprovalFlow, + context: { objects: brokenContext }, + }); + expect(result.errors.map((f) => f.rule)).toContain('approval-expression-invalid'); + }); + + // ── Dispatch ───────────────────────────────────────────────────────── + + it('runs nothing for a metadata type no rule gates', () => { + const result = runRuntimeAuthoringRules({ type: 'translation', item: { name: 'x' } }); + expect(result.rulesRun).toEqual([]); + expect(result.errors).toEqual([]); + }); + + it('tolerates a body that is not an object', () => { + for (const item of [null, undefined, 'a string', 42]) { + expect(runRuntimeAuthoringRules({ type: 'flow', item }).errors).toEqual([]); + } + }); + + it('works with no object context at all', () => { + // A host without a registry (a metadata-only store, a test double) still + // gets the rules that need no object universe, and must not throw. + const result = runRuntimeAuthoringRules({ type: 'flow', item: brokenApprovalFlow }); + expect(result.errors.map((f) => f.rule)).toContain('approval-expression-invalid'); + }); + + it('exposes its dispatch table as data', () => { + expect(runtimeGatedTypes()).toContain('flow'); + expect(stackKeyForType('flow')).toBe('flows'); + expect(stackKeyForType('no_such_type')).toBeNull(); + }); + + it('a rule that throws degrades to a warning instead of failing the write', () => { + // A gate whose internal error blocks a publish is worse than the hole it + // closes: the author gets a refusal they cannot act on. Proven through the + // real dispatch path with a body shaped to break a walk. + const cyclic: Record = { name: 'loop', nodes: [] }; + cyclic.self = cyclic; + expect(() => + runRuntimeAuthoringRules({ type: 'flow', item: cyclic, context: { objects } }), + ).not.toThrow(); + }); +}); diff --git a/packages/lint/src/runtime-gate.ts b/packages/lint/src/runtime-gate.ts new file mode 100644 index 0000000000..fadb81b9f1 --- /dev/null +++ b/packages/lint/src/runtime-gate.ts @@ -0,0 +1,224 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The RUNTIME publish gate over the author-time rule registry (#4463). + * + * ## The hole this closes + * + * #4409/#4445 put 26 author-time rules behind one table and made `os validate`, + * `os build` and `os lint` run it by construction. All three are CLI commands. + * The metadata WRITE path — Studio's designer, REST `/meta` item CRUD, an + * MCP/AI author — reaches `saveMetaItem`, which ran a per-type Zod `safeParse` + * and nothing else. Zero of the 26 rules ran there. For a tenant that is not + * one weak door among four, it is the ONLY door: `os lint` cannot see a + * `sys_metadata` overlay row at all, so there was no command they could run + * instead. + * + * It also happens to be the door AI authors use. That is the axis the #4463 + * ruling weighted highest: metadata written by a model is exactly the metadata + * most likely to be subtly wrong, and it was arriving through the one entrance + * with no checks on it. + * + * ## Shape + * + * This module is a GATE, not a rule engine. It owns no judgement: every finding + * it returns came from {@link AUTHORING_RULES}, the same array the three CLI + * commands run. Delete a rule from that table and this gate stops enforcing it + * in the same commit — which is the property the issue asked for, and the + * reason a second "runtime rule list" was never on the table. + * + * ## Why it evaluates DIFFERENTIALLY + * + * The rules are `(stack) => findings`. A runtime write is one ITEM. The gate + * therefore builds a per-write snapshot — the written item, plus the live + * registry's objects as resolution context — and runs the rules TWICE: once on + * the context alone, once with the item grafted in. Only findings the item + * ADDED are attributable to this write. + * + * That is not defensive padding, it is the D4 requirement made structural: + * + * - a tenant's existing `sys_metadata` rows may already violate a rule that did + * not exist when they were written, and the read path must keep serving them + * (ADR-0087's asymmetry). Gating on the absolute finding set would make an + * unrelated legacy row block every future save; + * - the context is resolution material, not subject matter. Without the + * subtraction, saving flow A would 422 because object B — untouched, already + * stored, possibly shipped in a package — has a bad predicate. + * + * The cost is two passes over a small in-memory snapshot, on a PUBLISH (not on + * a draft autosave). That is the correct place to spend it. + */ + +import { + AUTHORING_RULES, + type AuthoringFinding, + type AuthoringRule, + type AuthoringRuleContext, +} from './authoring-rules.js'; + +type AnyRec = Record; + +/** + * The stack-key each gated metadata type occupies in a stack view. + * + * Only the types some rule declares in `runtimeTypes` need an entry; the guard + * in `authoring-rule-wiring.test.ts` fails if a declared type is missing one, + * so widening the gate cannot half-land. + */ +const TYPE_TO_STACK_KEY: Readonly> = { + flow: 'flows', + object: 'objects', + view: 'views', + action: 'actions', + page: 'pages', + dashboard: 'dashboards', + agent: 'agents', + hook: 'hooks', + seed: 'seeds', +}; + +/** Everything the gate needs from the host runtime to build a snapshot. */ +export interface RuntimeStackContext { + /** + * The live object declarations (registry + tenant overlay), as authored. + * + * Objects are the resolution universe almost every rule needs: `record.` + * in a CEL predicate, a flow node's target object, a template path. This is + * where the runtime is strictly BETTER informed than `os lint`, which sees one + * package's config file and has to hedge. + */ + objects?: readonly unknown[]; +} + +/** One rule's verdict at the runtime surface, carrying which rule produced it. */ +export interface RuntimeGateResult { + /** Findings the written item ADDED, severity `error` — the reason to refuse the write. */ + errors: AuthoringFinding[]; + /** Findings the written item added at `warning` / `info`. Never blocks (#4463 P1). */ + advisories: AuthoringFinding[]; + /** Names of the registry rules that actually ran, in registry order. */ + rulesRun: string[]; +} + +/** + * The rules the runtime publish gate runs for a write of `type` — read off + * {@link AUTHORING_RULES}, never a list of its own. + * + * @param type Singular metadata type name (`flow`, `object`, …). + */ +export function runtimeAuthoringRulesFor(type: string): readonly AuthoringRule[] { + return AUTHORING_RULES.filter( + (r) => r.surfaces.includes('runtime-publish') && (r.runtimeTypes ?? []).includes(type), + ); +} + +/** Every singular metadata type at least one rule gates at the runtime surface. */ +export function runtimeGatedTypes(): string[] { + const types = new Set(); + for (const rule of AUTHORING_RULES) { + if (!rule.surfaces.includes('runtime-publish')) continue; + for (const t of rule.runtimeTypes ?? []) types.add(t); + } + return [...types].sort(); +} + +/** The stack key a metadata type occupies in a stack view, or null when unmapped. */ +export function stackKeyForType(type: string): string | null { + return TYPE_TO_STACK_KEY[type] ?? null; +} + +/** Stable identity of a finding, so two rule passes can be set-differenced. */ +const fingerprint = (f: AuthoringFinding) => `${f.rule}\u0000${f.where}\u0000${f.path}\u0000${f.message}`; + +function runRules( + rules: readonly AuthoringRule[], + stack: AnyRec, + ctx: AuthoringRuleContext, +): AuthoringFinding[] { + const findings: AuthoringFinding[] = []; + for (const rule of rules) { + // A rule that throws on an unexpected runtime body must not take the write + // down with it: the gate's job is to REFUSE bad metadata, and an internal + // error is not a verdict about the author's document. Surfaced as a + // warning-tier finding so it is neither silent nor fatal. + try { + findings.push(...rule.run(stack, ctx)); + } catch (err) { + findings.push({ + severity: 'warning', + rule: 'authoring-rule-threw', + where: rule.name, + path: rule.source, + message: `rule ${rule.name} threw while judging this write: ${err instanceof Error ? err.message : String(err)}`, + hint: + 'This is a bug in the rule, not in the metadata — the write was not blocked by it. ' + + 'Please report it with the body that triggered it.', + }); + } + } + return findings; +} + +/** + * Judge one about-to-be-published metadata item against the shared registry. + * + * Returns an empty `errors` array when the item is clean OR when no rule gates + * its type — callers must not treat "no rules ran" as a failure, and + * `rulesRun` is there so a caller can tell the two apart. + * + * Pure: no I/O, no `process.env`, no logging. The escape hatch and the HTTP + * status live at the call site, where the request context is. + */ +export function runRuntimeAuthoringRules(args: { + /** Singular metadata type of the item being written. */ + type: string; + /** The item body as it will be persisted. */ + item: unknown; + /** Live resolution context from the host runtime. */ + context?: RuntimeStackContext; + /** ADR-0080 SDUI manifest, when the host has one. */ + sduiManifest?: unknown; +}): RuntimeGateResult { + const rules = runtimeAuthoringRulesFor(args.type); + const empty: RuntimeGateResult = { errors: [], advisories: [], rulesRun: [] }; + if (rules.length === 0) return empty; + + const stackKey = stackKeyForType(args.type); + if (!stackKey) return empty; + if (!args.item || typeof args.item !== 'object') return empty; + + const item = args.item as AnyRec; + const itemName = typeof item.name === 'string' ? item.name : undefined; + const contextObjects = (args.context?.objects ?? []) as AnyRec[]; + const ctx: AuthoringRuleContext = { sduiManifest: args.sduiManifest }; + + // When the written type IS the context collection (an `object` write), the + // item must REPLACE its stored self rather than erase the other objects — + // otherwise every lookup in the tenant's model reads as dangling. Written + // generally so widening `runtimeTypes` to `object` is a data edit, not a + // rewrite of this function. + const writesIntoContext = stackKey === 'objects'; + const baselineObjects = writesIntoContext + ? contextObjects.filter((o) => !itemName || o?.name !== itemName) + : contextObjects; + + // Baseline: the resolution context WITHOUT the written item. Anything found + // here is somebody else's pre-existing condition and is not this write's to + // answer for (#4463 D4 — the gate blocks new writes, never stored rows). + const baseline: AnyRec = { objects: baselineObjects }; + // Candidate: the same context with this write's item added. For a non-object + // type it is the SOLE member of its own collection, so index-0 paths in the + // findings are unambiguously this write. + const candidate: AnyRec = writesIntoContext + ? { objects: [...baselineObjects, item] } + : { objects: baselineObjects, [stackKey]: [item] }; + + const before = new Set(runRules(rules, baseline, ctx).map(fingerprint)); + const added = runRules(rules, candidate, ctx).filter((f) => !before.has(fingerprint(f))); + + return { + errors: added.filter((f) => f.severity === 'error'), + advisories: added.filter((f) => f.severity !== 'error'), + rulesRun: rules.map((r) => r.name), + }; +} diff --git a/packages/lint/src/runtime-lazy-deps.test.ts b/packages/lint/src/runtime-lazy-deps.test.ts new file mode 100644 index 0000000000..bcaa407b63 --- /dev/null +++ b/packages/lint/src/runtime-lazy-deps.test.ts @@ -0,0 +1,150 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// The kernel boot-path contract for `@objectstack/lint/runtime` (#4463). +// +// `lazy-deps.test.ts` next door pins that IMPORTING the package loads neither +// `typescript` (~9 MB) nor `sucrase`. That was enough while the only consumer +// was the CLI, which may load anything. #4463 gave the package a consumer on +// the kernel boot path — `@objectstack/metadata-protocol`, reached by every +// runtime metadata write — and that consumer needs the stronger claim: +// +// RUNNING the gate, on a real body of a really-gated type, loads neither. +// +// Import-time laziness alone would not have said this. The registry statically +// names the react/jsx rules (one table, by design — see `authoring-rules.ts`), +// so the module graph is present; what must never happen is a runtime write +// TRIGGERING one. The rules #4463 wired to `runtime-publish` — flow, approval, +// expression, reference — read structured metadata and parse no authored +// source, so they cannot. This file is what keeps that true when the next rule +// is wired to the surface: widen `runtimeTypes` to a type whose snapshot +// carries a hook body or a react page and this goes red, which is the moment to +// stop and think, not a moment to relax the assertion. + +import { execFileSync } from 'node:child_process'; +import { existsSync } from 'node:fs'; +import { createRequire } from 'node:module'; +import { dirname, join } from 'node:path'; +import { fileURLToPath, pathToFileURL } from 'node:url'; +import { describe, it, expect } from 'vitest'; + +const srcDir = dirname(fileURLToPath(import.meta.url)); +const distDir = join(srcDir, '..', 'dist'); + +const LAZY_DEPS = ['typescript', 'sucrase']; + +const depLoaded = (cache: Record | undefined, dep: string) => + Object.keys(cache ?? {}).some((p) => p.split(/[/\\]/).join('/').includes(`/node_modules/${dep}/`)); + +/** The #4463 worked example: the body a Studio tenant could publish before the gate existed. */ +const GATED_FLOW = { + name: 'leave_approval', + nodes: [ + { id: 'start', type: 'start' }, + { + id: 'approve', + type: 'approval', + config: { approvers: [{ type: 'expression', value: 'record.owner ==' }] }, + }, + ], +}; +const OBJECTS = [{ name: 'leave_request', fields: { owner: { type: 'text' } } }]; + +// Same shape as lazy-deps.test.ts: a spawned child is the only place a native +// require-cache probe means anything, because vitest inlines static imports +// through its transform and they never reach that cache. +const childBody = ` + const probe = require('node:module').createRequire(process.cwd() + '/probe.js'); + const loaded = (dep) => Object.keys(probe.cache ?? {}).some((p) => p.split(/[/\\\\]/).join('/').includes('/node_modules/' + dep + '/')); + const fail = (msg) => { console.error(msg); process.exit(1); }; + const check = (mod) => { + for (const dep of ${JSON.stringify(LAZY_DEPS)}) { + if (loaded(dep)) fail(dep + ' was loaded merely by importing @objectstack/lint/runtime'); + } + const result = mod.runRuntimeAuthoringRules({ + type: 'flow', + item: ${JSON.stringify(GATED_FLOW)}, + context: { objects: ${JSON.stringify(OBJECTS)} }, + }); + if (!result.errors.some((f) => f.rule === 'approval-expression-invalid')) { + fail('the gate produced no finding — the probe below would then be vacuously true'); + } + for (const dep of ${JSON.stringify(LAZY_DEPS)}) { + if (loaded(dep)) fail(dep + ' was loaded by RUNNING the runtime publish gate'); + } + console.log('OK'); + }; +`; + +// Cold-loading a dist entry in a spawned node on a busy runner takes seconds. +const COLD_LOAD_TIMEOUT_MS = 30_000; + +describe('@objectstack/lint/runtime (kernel boot-path contract, #4463)', () => { + it.skipIf(!existsSync(join(distDir, 'runtime.cjs')))( + 'built CJS runtime entry loads no heavy dep, at import OR while gating', + () => { + const out = execFileSync( + process.execPath, + ['-e', `${childBody}; check(require(${JSON.stringify(join(distDir, 'runtime.cjs'))}));`], + { encoding: 'utf8' }, + ); + expect(out).toContain('OK'); + }, + COLD_LOAD_TIMEOUT_MS, + ); + + it.skipIf(!existsSync(join(distDir, 'runtime.js')))( + 'built ESM runtime entry loads no heavy dep, at import OR while gating', + () => { + const out = execFileSync( + process.execPath, + [ + '--input-type=module', + '-e', + `import { createRequire } from 'node:module'; + const require = createRequire(process.cwd() + '/probe.js'); + ${childBody}; + check(await import(${JSON.stringify(pathToFileURL(join(distDir, 'runtime.js')).href)}));`, + ], + { encoding: 'utf8' }, + ); + expect(out).toContain('OK'); + }, + COLD_LOAD_TIMEOUT_MS, + ); + + it('gating a flow in-process loads neither dep, and still finds the defect', async () => { + const req = createRequire(import.meta.url); + const { runRuntimeAuthoringRules } = await import('./runtime.js'); + + const result = runRuntimeAuthoringRules({ + type: 'flow', + item: GATED_FLOW, + context: { objects: OBJECTS }, + }); + // Non-vacuity first: a probe over a gate that found nothing proves nothing. + expect(result.errors.map((f) => f.rule)).toContain('approval-expression-invalid'); + + for (const dep of LAZY_DEPS) { + expect( + depLoaded(req.cache, dep), + `${dep} loaded while gating a flow — the metadata write path is on the kernel boot path and ` + + `may not pay for a source parser to publish a flow`, + ).toBe(false); + } + }); + + it('the runtime entry re-exports the gate and nothing that carries a parser', async () => { + const mod = await import('./runtime.js'); + expect(typeof mod.runRuntimeAuthoringRules).toBe('function'); + expect(typeof mod.runtimeAuthoringRulesFor).toBe('function'); + expect(typeof mod.runtimeGatedTypes).toBe('function'); + // The source-parsing rules must not be reachable through this entry by + // name: a consumer that can call them can pay for them by accident. + for (const forbidden of ['validateReactPages', 'validateReactPageProps', 'validateJsxPages']) { + expect( + forbidden in mod, + `${forbidden} must not be exported from the kernel-safe entry`, + ).toBe(false); + } + }); +}); diff --git a/packages/lint/src/runtime.ts b/packages/lint/src/runtime.ts new file mode 100644 index 0000000000..3b49359027 --- /dev/null +++ b/packages/lint/src/runtime.ts @@ -0,0 +1,31 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `@objectstack/lint/runtime` — the KERNEL-SAFE entry (#4463). + * + * The metadata write path (`@objectstack/metadata-protocol`) sits on the kernel + * boot path and must reach the shared rule core without dragging the lint + * package's gate-only dependencies (`typescript` ~9 MB, `sucrase`) into it. + * That constraint is real and it is guarded from both ends: + * + * - `lazy-deps.test.ts` pins that no `src/` file eagerly imports either dep, so + * importing this entry loads neither; + * - `runtime-lazy-deps.test.ts` pins the stronger claim this entry needs — that + * RUNNING the gate on a real, gated body loads neither either, because the + * rules #4463 wired to `runtime-publish` (flow / approval / expression / + * reference) never parse authored source. + * + * The deliberate NON-goal: this is not a second, lighter rule set. It re-exports + * a filtered view of the ONE registry in `authoring-rules.ts`. If the two ever + * disagree it is a bug, and `authoring-rule-wiring.test.ts` is what makes them + * unable to. + */ + +export { + runRuntimeAuthoringRules, + runtimeAuthoringRulesFor, + runtimeGatedTypes, + stackKeyForType, +} from './runtime-gate.js'; +export type { RuntimeGateResult, RuntimeStackContext } from './runtime-gate.js'; +export type { AuthoringFinding, AuthoringSeverity } from './authoring-rules.js'; diff --git a/packages/lint/tsup.config.ts b/packages/lint/tsup.config.ts new file mode 100644 index 0000000000..cfbcbdb9eb --- /dev/null +++ b/packages/lint/tsup.config.ts @@ -0,0 +1,20 @@ +import { defineConfig } from 'tsup'; + +/** + * Package-local config (#4463): `@objectstack/lint` ships TWO entries, so it + * cannot use the repo-root `tsup.config.ts` (single `src/index.ts`). + * + * - `index` — the full authoring surface, used by the CLI. + * - `runtime` — the kernel-safe subset the metadata write path imports. Kept a + * separate entry so a consumer on the boot path never even names the module + * graph that reaches the react/jsx source parsers. + */ +export default defineConfig({ + entry: ['src/index.ts', 'src/runtime.ts'], + splitting: false, + sourcemap: true, + clean: true, + dts: !process.env.OS_SKIP_DTS, + format: ['esm', 'cjs'], + target: 'es2020', +}); diff --git a/packages/metadata-protocol/package.json b/packages/metadata-protocol/package.json index 94ed232497..9ebc1da497 100644 --- a/packages/metadata-protocol/package.json +++ b/packages/metadata-protocol/package.json @@ -34,6 +34,7 @@ "dependencies": { "@objectstack/core": "workspace:*", "@objectstack/formula": "workspace:*", + "@objectstack/lint": "workspace:*", "@objectstack/metadata-core": "workspace:*", "@objectstack/spec": "workspace:*", "@objectstack/types": "workspace:*", diff --git a/packages/metadata-protocol/src/protocol.runtime-authoring-gate.test.ts b/packages/metadata-protocol/src/protocol.runtime-authoring-gate.test.ts new file mode 100644 index 0000000000..5eeb2c1827 --- /dev/null +++ b/packages/metadata-protocol/src/protocol.runtime-authoring-gate.test.ts @@ -0,0 +1,327 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #4463 — the runtime authoring gate, end to end through `saveMetaItem`. + * + * The measured example from the issue, run at the door it was measured at: a + * tenant saves an approval flow whose `expression` approver is broken CEL + * (`record.owner ==`). `ApproverSchema.value` is a `z.string()`, so the + * per-type Zod gate is green; before this change the body landed in + * `sys_metadata`, `registerFlow` registered it, and the node failed at its + * entry the first time the flow fired. `os lint` had rejected that exact body + * since #4409 — and there is no `os lint` for a Studio tenant, because a + * `sys_metadata` overlay row is not in the CLI's config file at all. + * + * These tests pin all four decisions, not just the refusal: + * D1 — `active` is gated, `draft` is not, and publishing a draft IS gated. + * D3 — the refusal is a 422 in the existing structured-issues envelope. + * D4 — `OS_ALLOW_UNLINTED_METADATA_WRITES=1` degrades it to a loud log. + * plus: nothing persists on a refusal. + * + * Harness: the real repository write path over a stub engine — the same shape + * as `protocol.save-flow-canonicalization.test.ts`, because a gate INSIDE + * `saveMetaItem` cannot be tested against a harness that mocks `saveMetaItem`. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { ObjectStackProtocolImplementation } from './protocol.js'; + +/** The issue's body. Zod-valid: `approvers[].value` is just a string to the schema. */ +const brokenApprovalFlow = () => ({ + name: 'leave_approval', + label: 'Leave Approval', + type: 'autolaunched', + status: 'active', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { + id: 'approve', + type: 'approval', + label: 'Approve', + config: { approvers: [{ type: 'expression', value: 'record.owner ==' }] }, + }, + ], + edges: [{ id: 'e1', source: 'start', target: 'approve' }], +}); + +/** The same flow with an approver expression that parses and uses a legal root. */ +const validApprovalFlow = () => { + const flow = brokenApprovalFlow(); + flow.nodes[1]!.config = { + approvers: [{ type: 'expression', value: 'current.owner' }], + emptyApproverPolicy: 'reject', + } as any; + return flow; +}; + +interface Row { + id: string; + type: string; + name: string; + organization_id: string | null; + state: string; + metadata: string; + checksum?: string; +} + +const keyOf = (w: Record) => + `${w.type}|${w.name}|${w.organization_id ?? '__env__'}|${w.state ?? 'active'}`; + +function makeStubEngine() { + const rows = new Map(); + let nextId = 0; + const findRow = (w: Record): { key: string; row: Row } | null => { + if (w.id !== undefined) { + for (const [k, r] of rows) if (r.id === w.id) return { key: k, row: r }; + return null; + } + for (const [k, r] of rows) { + if (w.type !== undefined && r.type !== w.type) continue; + if (w.name !== undefined && r.name !== w.name) continue; + if (w.organization_id !== undefined && r.organization_id !== w.organization_id) continue; + if (w.state !== undefined && r.state !== w.state) continue; + return { key: k, row: r }; + } + return null; + }; + const engine: any = { + async findOne(_t: string, opts: { where: Record }) { + return findRow(opts.where)?.row ?? null; + }, + async find(_t: string, opts: { where: Record }) { + return Array.from(rows.values()).filter((r) => { + if (opts.where.type && r.type !== opts.where.type) return false; + if (opts.where.organization_id !== undefined + && r.organization_id !== opts.where.organization_id) return false; + if (opts.where.state && r.state !== opts.where.state) return false; + return true; + }); + }, + async insert(_t: string, data: Record) { + if (_t === 'sys_metadata_audit') return { id: 'audit_skip' }; + nextId += 1; + const row = { id: `r_${nextId}`, ...(data as any) } as Row; + rows.set(keyOf(data), row); + return { id: row.id }; + }, + async update(_t: string, data: Record, opts: { where: Record }) { + const found = findRow(opts.where); + if (!found) return { id: null }; + rows.set(found.key, { ...found.row, ...(data as any) }); + return { id: found.row.id }; + }, + async delete(_t: string, opts: { where: Record }) { + const found = findRow(opts.where); + if (!found) return { deleted: 0 }; + rows.delete(found.key); + return { deleted: 1 }; + }, + registry: { + registerItem: () => {}, + registerObject: () => {}, + // The live object universe the rules resolve names against — the + // input `os lint` cannot have and this surface can (#4463 D2). + listItems: (type: string) => + type === 'object' + ? [{ name: 'leave_request', fields: { owner: { type: 'text' } } }] + : [], + getItem: () => undefined, + }, + }; + return { engine, rows }; +} + +/** `environmentId` set: the gate, like every ADR-0005 authorization gate, is tenant-scoped. */ +function makeProtocol() { + const { engine, rows } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine, () => new Map(), 'env_test'); + return { protocol: protocol as any, rows }; +} + +const flowRows = (rows: Map) => + Array.from(rows.values()).filter((r) => r.type === 'flow'); + +const save = (protocol: any, item: unknown, extra: Record = {}) => + protocol.saveMetaItem({ type: 'flow', name: 'leave_approval', item, ...extra }); + +describe('runtime authoring gate on saveMetaItem (#4463)', () => { + let warn: ReturnType; + beforeEach(() => { + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + delete process.env.OS_ALLOW_UNLINTED_METADATA_WRITES; + }); + afterEach(() => { + warn.mockRestore(); + delete process.env.OS_ALLOW_UNLINTED_METADATA_WRITES; + }); + + // ── D1 + D3: the refusal ───────────────────────────────────────────── + + it('refuses an ACTIVE save of the broken approval flow with a 422', async () => { + const { protocol, rows } = makeProtocol(); + + await expect(save(protocol, brokenApprovalFlow())).rejects.toThrow(/invalid_metadata/); + + const err = await save(protocol, brokenApprovalFlow()).catch((e: any) => e); + expect(err.status).toBe(422); + expect(err.code).toBe('INVALID_METADATA'); + + // D3 — the structured envelope Studio already renders for a Zod + // failure, carrying the four keys an author needs to act. + const issue = err.issues.find((i: any) => i.rule === 'approval-expression-invalid'); + expect(issue, `issues: ${JSON.stringify(err.issues)}`).toBeDefined(); + expect(issue.path).toBe('flows[0].nodes[1].config.approvers[0].value'); + expect(issue.where).toContain('leave_approval'); + expect(issue.message).toMatch(/does not parse as CEL/); + expect(issue.hint.length).toBeGreaterThan(10); + + // Which rules produced the verdict — so "clean" and "nothing ran" are + // distinguishable from the outside. + expect(err.rulesRun).toContain('validateApprovalApprovers'); + + // And nothing landed. A gate that rejects AFTER persisting is a log line. + expect(flowRows(rows)).toEqual([]); + }); + + it('allows the same flow once the expression is valid', async () => { + const { protocol, rows } = makeProtocol(); + const result = await save(protocol, validApprovalFlow()); + expect(result.success).toBe(true); + expect(flowRows(rows).length).toBe(1); + }); + + // ── D1: drafts are never gated ─────────────────────────────────────── + + it('lets the identical body through as a DRAFT', async () => { + const { protocol, rows } = makeProtocol(); + + const result = await save(protocol, brokenApprovalFlow(), { mode: 'draft' }); + + expect( + result.success, + `a draft is allowed to be half-finished — gating one would destroy the Studio editing loop ` + + `for no safety gain, because a draft cannot execute (#4463 D1).`, + ).toBe(true); + const states = flowRows(rows).map((r) => r.state); + expect(states).toContain('draft'); + expect(states, 'a draft save must not mint an active row').not.toContain('active'); + }); + + it('gates the draft→active PROMOTION, so the draft door is not a bypass', async () => { + const { protocol } = makeProtocol(); + await save(protocol, brokenApprovalFlow(), { mode: 'draft' }); + + const err = await protocol + .publishMetaItem({ type: 'flow', name: 'leave_approval' }) + .catch((e: any) => e); + + expect( + err?.status, + `without this, anyone could save ?mode=draft and POST /publish to walk straight past the ` + + `gate — which is exactly what Studio's designer does on every edit.`, + ).toBe(422); + expect(err.issues.map((i: any) => i.rule)).toContain('approval-expression-invalid'); + }); + + it('publishes a draft that is clean', async () => { + const { protocol } = makeProtocol(); + await save(protocol, validApprovalFlow(), { mode: 'draft' }); + const result = await protocol.publishMetaItem({ type: 'flow', name: 'leave_approval' }); + expect(result.success).toBe(true); + }); + + // ── D4: the escape hatch ───────────────────────────────────────────── + + it('OS_ALLOW_UNLINTED_METADATA_WRITES=1 allows the write and says so loudly', async () => { + process.env.OS_ALLOW_UNLINTED_METADATA_WRITES = '1'; + const { protocol, rows } = makeProtocol(); + + const result = await save(protocol, brokenApprovalFlow()); + expect(result.success).toBe(true); + expect(flowRows(rows).length).toBe(1); + + const shouted = (warn.mock.calls as unknown[][]) + .map((c) => String(c[0])) + .filter((m) => m.includes('OS_ALLOW_UNLINTED_METADATA_WRITES')); + expect(shouted.length, 'the hatch makes a violation TOLERATED, never invisible').toBe(1); + expect(shouted[0]).toContain('approval-expression-invalid'); + expect(shouted[0]).toContain('leave_approval'); + }); + + // ── Scope: what the gate must NOT do ───────────────────────────────── + + it('does not gate control-plane (package-author) writes', async () => { + // `environmentId === undefined` is the package author's own channel — + // the same carve-out every ADR-0005 authorization gate above it makes. + const { engine, rows } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine, () => new Map()) as any; + const result = await protocol.saveMetaItem({ + type: 'flow', + name: 'leave_approval', + item: brokenApprovalFlow(), + }); + expect(result.success).toBe(true); + expect(flowRows(rows).length).toBe(1); + }); + + it('does not gate `os migrate meta --stored`, which rewrites rows that already exist', async () => { + // D4's other half. The migration heals stored bodies into the current + // dialect; it is not an author publishing anything. Gating it would + // mean a tenant holding one pre-existing violation could never + // canonicalize that row — the migration would report `failed` and + // leave the body in the OLDER dialect, which is worse than the state + // it was asked to improve. `source` is server-stated (never forwarded + // from a request), so this cannot be spelled past the gate by a caller. + const { protocol, rows } = makeProtocol(); + const result = await save(protocol, brokenApprovalFlow(), { source: 'migrate-stored' }); + expect(result.success).toBe(true); + expect(flowRows(rows).length).toBe(1); + }); + + it('DOES gate an ordinary save that merely looks like one (no source spoofing)', async () => { + // The carve-out is on one exact server-stated token; anything else — + // including a caller's guess at it — still meets the gate. + const { protocol } = makeProtocol(); + for (const source of [undefined, 'protocol.saveMetaItem', 'migrate', 'migrate-stored-ish']) { + const err = await save(protocol, brokenApprovalFlow(), source ? { source } : {}) + .catch((e: any) => e); + expect(err?.status, `source=${String(source)} must still be gated`).toBe(422); + } + }); + + it('does not gate a metadata type no rule declares (P1 wires `flow` only)', async () => { + // `object` writes are deliberately outside P1 — see the registry's + // RUNTIME_OBJECT_WRITES_P2 reason. A type nobody declared must pass + // through untouched rather than be silently half-checked. + const { protocol } = makeProtocol(); + const result = await protocol.saveMetaItem({ + type: 'object', + name: 'leave_request', + item: { + name: 'leave_request', + label: 'Leave Request', + fields: { owner: { type: 'text', label: 'Owner' } }, + }, + }); + expect(result.success).toBe(true); + }); + + it('survives a host whose registry cannot list objects', async () => { + // Context gathering is best-effort: a metadata-only store still writes, + // it just gets the rules that need no object universe. It must never be + // the reason a write fails. + const { engine, rows } = makeStubEngine(); + engine.registry.listItems = () => { throw new Error('no registry here'); }; + const protocol = new ObjectStackProtocolImplementation(engine, () => new Map(), 'env_test') as any; + + await expect( + protocol.saveMetaItem({ type: 'flow', name: 'leave_approval', item: validApprovalFlow() }), + ).resolves.toMatchObject({ success: true }); + expect(flowRows(rows).length).toBe(1); + + // …and the refusal still happens without that context. + const err = await protocol + .saveMetaItem({ type: 'flow', name: 'other_flow', item: { ...brokenApprovalFlow(), name: 'other_flow' } }) + .catch((e: any) => e); + expect(err.status).toBe(422); + }); +}); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index ef69f1b65a..a9cf6c9d64 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -6,6 +6,7 @@ import type { import { IDataEngine, engineCanRollBack } from '@objectstack/core'; import { readEnvWithDeprecation } from '@objectstack/types'; import type { MetadataHostEngine } from './host-engine.js'; +import { evaluateRuntimeAuthoringGate } from './runtime-authoring-gate.js'; import { SysMetadataRepository, type SysMetadataEngine } from './sys-metadata-repository.js'; import { ConflictError, assertProtocolCompat, type MetadataItem } from '@objectstack/metadata-core'; import type { @@ -1926,6 +1927,71 @@ export class ObjectStackProtocolImplementation implements }); } + /** + * The #4463 runtime authoring gate — the fourth door. + * + * Runs the SHARED author-time rule registry (`@objectstack/lint`'s + * `AUTHORING_RULES`, the same table `os validate` / `os build` / `os lint` + * run) over a body about to go `active`, and throws the 422 its gating + * findings earn. Draft saves are never gated (D1) — publishing the draft + * runs this. + * + * Deliberately NOT a {@link registerAuthoringGate} registration: those are + * per-type and single-slot, owned by the domain plugin that registers them + * (plugin-security holds `object`). This is the platform's own gate and it + * must be unconditional — a plugin cannot displace it, and no surface can + * skip it, because every surface reaches this class. + * + * The rules resolve names against the LIVE object universe, which is + * strictly better information than the CLI's single-package view — the + * inversion #4463 D2 points out: the same rule can be more decisive here + * than it can be at build time. + */ + private assertRuntimeAuthoringRules(evt: { + type: string; name: string; state: 'draft' | 'active'; body: unknown; source?: string; + }): void { + if (this.environmentId === undefined) return; + if (evt.state !== 'active') return; + // `os migrate meta --stored --apply` rewrites rows that ALREADY EXIST + // into the current dialect. It is not an author publishing anything — + // it is the platform healing its own storage — and #4463 D4 is explicit + // that the gate blocks new writes while stored rows keep their + // ADR-0087 path. Gating it would invert the tool: a tenant with one + // pre-existing violation could never canonicalize that row, and the + // migration would report `failed` while leaving the body in the OLDER + // dialect — strictly worse than the state it was asked to improve. + // + // Safe as a carve-out because `source` is stated by the SERVER, never + // forwarded from a request (see the note at the top of `saveMetaItem`): + // no caller can spell its way past the gate. `duplicatePackage` is + // deliberately NOT here — it mints brand-new rows under new names, and + // a copy of a broken flow is a new broken flow. + if (evt.source === 'migrate-stored') return; + const singular = PLURAL_TO_SINGULAR[evt.type] ?? evt.type; + + // Resolution context. Best-effort: a host without a registry (a + // metadata-only test double) still writes, it just gets the rules that + // need no object universe. Never let context-gathering fail a write. + let objects: unknown[] = []; + try { + if (typeof this.engine.registry?.listItems === 'function') { + objects = [...this.engine.registry.listItems('object')]; + if (objects.length === 0) objects = [...this.engine.registry.listItems('objects')]; + } + } catch { + objects = []; + } + + const err = evaluateRuntimeAuthoringGate({ + type: singular, + name: evt.name, + state: evt.state, + body: evt.body, + objects, + }); + if (err) throw err; + } + /** * Run the registered projector for a just-persisted mutation (ADR-0094). * Returns `undefined` when no projector is registered for the type; @@ -6616,6 +6682,20 @@ export class ObjectStackProtocolImplementation implements } } + // The #4463 runtime authoring gate — the shared author-time rule + // registry, on the write path. `active` saves only (D1): this is the + // publish verb, and it is the same table `os build` gates on. Placed + // immediately after the schema check because a rule reads a body the + // schema already accepted — a Zod failure is the more basic verdict and + // must be the one the author sees first. + this.assertRuntimeAuthoringRules({ + type: request.type, + name: request.name, + state: mode === 'draft' ? 'draft' : 'active', + body: request.item, + source: writeSource, + }); + // Pre-persistence authoring gate (#3050): a domain plugin may veto the // body before it persists (throws propagate to the caller with their // status/code). Runs for BOTH draft and publish-mode saves, so a later @@ -7352,6 +7432,26 @@ export class ObjectStackProtocolImplementation implements await this.ensureOverlayIndex(); const orgId = request.organizationId ?? null; const repo = this.getOverlayRepo(orgId); + + // #4463 D1 — the OTHER way a body reaches `active`. `saveMetaItem` + // gates a direct active save and deliberately lets every draft through; + // that permission is only sound if the draft→active promotion gates. + // Without this the gate would be trivially bypassable by anyone who + // saves `?mode=draft` and then POSTs `/publish` — which is exactly what + // Studio's designer surface does on every edit. + const draftForGate = await repo.get( + { type: singularType, name: request.name, org: orgId ?? 'env' } as Parameters[0], + { state: 'draft' }, + ); + if (draftForGate) { + this.assertRuntimeAuthoringRules({ + type: singularType, + name: request.name, + state: 'active', + body: draftForGate.body, + }); + } + const artifactBacked = this.isArtifactBacked(singularType, request.name); const intent: 'override-artifact' | 'runtime-only' = artifactBacked ? 'override-artifact' : 'runtime-only'; diff --git a/packages/metadata-protocol/src/runtime-authoring-gate.ts b/packages/metadata-protocol/src/runtime-authoring-gate.ts new file mode 100644 index 0000000000..eec15253e3 --- /dev/null +++ b/packages/metadata-protocol/src/runtime-authoring-gate.ts @@ -0,0 +1,170 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The RUNTIME authoring gate — the fourth door (#4463). + * + * ## What was open + * + * #4409/#4445 collected 26 author-time rules into one registry and made + * `os validate`, `os build` and `os lint` run it by construction. All three are + * CLI commands. Every runtime metadata write — Studio's designer, REST `/meta` + * item CRUD, an MCP/AI agent authoring a flow — lands in + * {@link ObjectStackProtocolImplementation.saveMetaItem}, which ran a per-type + * Zod `safeParse` and stopped. None of the 26 rules ran there. + * + * The issue's measured example: a tenant saves an approval flow whose + * `expression` approver is broken CEL (`record.owner ==`). `approver.value` is + * a `z.string()`, so the schema is green; the row lands in `sys_metadata`, + * `registerFlow` registers it, and the node fails at its entry the first time + * the flow fires. `os lint` had rejected that exact body since #4409 — and a + * Studio tenant has no `os lint`. `sys_metadata` overlay rows are not in the + * CLI's config file at all, so there was no command they could have run. + * + * ## Shape (the #4463 ruling) + * + * ONE shared core, ONE runtime gate — not a gate per surface. This module is + * that gate. It holds no rules: `@objectstack/lint/runtime` filters the same + * `AUTHORING_RULES` array the CLI runs, and `authoring-rule-wiring.test.ts` + * fails if this file ever names a rule itself. + * + * ## The four decisions it implements + * + * - **D1 — where the gate is.** `state: 'active'` only. A draft save is always + * let through: a draft is allowed to be half-finished, gating one would + * destroy the Studio editing loop, and a draft cannot execute. `active` IS + * the publish verb, the same verb `os build` gates. + * - **D2 — shape mismatch.** The rules are `(stack) => findings`; a runtime + * write is one item. The core builds a per-write snapshot and evaluates + * differentially — see `runtime-gate.ts` in `@objectstack/lint`. + * - **D3 — severity → HTTP.** Gating findings become the SAME 422 + * `invalid_metadata`-shaped envelope the Zod failure already produces, so + * Studio needs no new protocol: `issues[]` with `rule` / `path` / `message` / + * `hint`. Advisory findings do not block (P2 puts them on the response). + * - **D4 — escape hatch + migration.** The gate blocks NEW writes only; stored + * rows keep being read (the ADR-0087 asymmetry `applyConversionsToStoredItem` + * already proved right). `OS_ALLOW_UNLINTED_METADATA_WRITES=1` degrades the + * refusal to a loud log for a migration window. + */ + +import { runRuntimeAuthoringRules, type AuthoringFinding } from '@objectstack/lint/runtime'; + +/** The structured issue shape a 422 carries — D3's "reuse the Zod envelope". */ +export interface RuntimeAuthoringIssue { + /** Stable diagnostic rule id (`approval-expression-invalid`, …). */ + rule: string; + /** Config path inside the submitted body. */ + path: string; + /** Human-readable location (`flow "leave_approval" · node "approve"`). */ + where: string; + /** What is wrong. */ + message: string; + /** How to fix it. */ + hint: string; + severity: 'error' | 'warning' | 'info'; +} + +/** + * The escape hatch (#4463 D4). + * + * `OS_ALLOW_*` per Prime Directive #9 — deliberately ungrouped and ugly, + * because it is an opt-OUT of a check that ships ON. Existing `sys_metadata` + * rows written before this gate existed may violate a rule; re-saving one + * during a migration must not be impossible. Setting it makes the violation + * TOLERATED, never invisible: every refusal it converts is logged in full. + */ +function unlintedWritesAllowed(): boolean { + return typeof process !== 'undefined' && process.env?.OS_ALLOW_UNLINTED_METADATA_WRITES === '1'; +} + +/** One warn per `type|name|rule` per process — Studio republishes the same body a lot. */ +const _advisoryWarned = new Set(); + +const toIssue = (f: AuthoringFinding): RuntimeAuthoringIssue => ({ + rule: f.rule, + path: f.path, + where: f.where, + message: f.message, + hint: f.hint, + severity: f.severity, +}); + +/** + * Judge an about-to-be-published metadata body and return the `Error` the + * caller must throw, or `null` to allow the write. + * + * Returning the error rather than throwing it mirrors + * {@link ObjectStackProtocolImplementation.assertLockAllowsWrite}: the caller + * owns the audit trail and the throw site. + * + * @param args.state Lifecycle the body is being written into. Anything but + * `'active'` returns `null` immediately (D1). + */ +export function evaluateRuntimeAuthoringGate(args: { + /** Singular metadata type (`flow`, …). */ + type: string; + name: string; + state: 'draft' | 'active'; + body: unknown; + /** Live object declarations, the resolution universe for the rules. */ + objects?: readonly unknown[]; + /** ADR-0080 SDUI manifest when the host has one. */ + sduiManifest?: unknown; +}): Error | null { + // D1 — drafts are never gated. Publishing one runs this same function. + if (args.state !== 'active') return null; + + const result = runRuntimeAuthoringRules({ + type: args.type, + item: args.body, + context: { objects: args.objects ?? [] }, + ...(args.sduiManifest !== undefined ? { sduiManifest: args.sduiManifest } : {}), + }); + + // P1 gates on `error` only. Advisories are surfaced as a deduped log rather + // than dropped — running a rule and discarding its verdict is the same + // "declared ≠ enforced" shape this gate exists to close, one notch quieter. + // Putting them on the response (and rendering them in Studio) is P2. + for (const advisory of result.advisories) { + const key = `${args.type}|${args.name}|${advisory.rule}|${advisory.path}`; + if (_advisoryWarned.has(key)) continue; + _advisoryWarned.add(key); + console.warn( + `[Protocol] authoring advisory on ${args.type}/${args.name}: ` + + `[${advisory.rule}] ${advisory.where} — ${advisory.message} (${advisory.hint})`, + ); + } + + if (result.errors.length === 0) return null; + + const issues = result.errors.map(toIssue); + const summary = issues + .slice(0, 3) + .map((i) => `${i.path || i.where || ''}: [${i.rule}] ${i.message}`) + .join('; '); + const detail = summary + (issues.length > 3 ? ` (+${issues.length - 3} more)` : ''); + + if (unlintedWritesAllowed()) { + // Loud by construction (#4463 acceptance): the operator who set the + // hatch gets the whole refusal in the log, every time, un-deduped — + // this is a migration window, not a supported steady state. + console.warn( + `[Protocol] OS_ALLOW_UNLINTED_METADATA_WRITES=1 — ALLOWING a publish of ` + + `${args.type}/${args.name} that ${issues.length} author-time gating rule(s) reject: ${detail}. ` + + `The rules that ran: ${result.rulesRun.join(', ')}. Unset the variable once the metadata is fixed; ` + + `the runtime will execute this body as published.`, + ); + return null; + } + + const err = new Error( + `[invalid_metadata] ${args.type}/${args.name} failed author-time validation: ${detail}`, + ); + (err as any).code = 'INVALID_METADATA'; + (err as any).status = 422; + (err as any).issues = issues; + // Which rules produced the verdict, so a caller can tell "clean" from + // "nothing ran" without guessing (route 3 of the surface-ownership rules: + // absence must be loud). + (err as any).rulesRun = result.rulesRun; + return err; +} diff --git a/packages/objectql/src/protocol-publish-package-drafts.test.ts b/packages/objectql/src/protocol-publish-package-drafts.test.ts index a007d12189..4819328b43 100644 --- a/packages/objectql/src/protocol-publish-package-drafts.test.ts +++ b/packages/objectql/src/protocol-publish-package-drafts.test.ts @@ -279,7 +279,20 @@ describe('protocol.publishMetaItem — seed self-apply', () => { (protocol as any).isArtifactBacked = () => false; (protocol as any).applyObjectRegistryMutation = () => {}; (protocol as any).ensureObjectStorage = async () => {}; + // The double must be as WIDE as `SysMetadataRepository`, not as narrow as + // the one call the test happens to care about. `promoteDraftForPublish` + // reads the pending draft before promoting it (#4463 — the author-time + // rules gate the draft→active transition, or `?mode=draft` + `POST + // /publish` would be a free way around the gate `saveMetaItem` applies). + // A double missing `get` failed with `repo.get is not a function`, which + // is the #4550 shape: a stand-in narrower than the contract it stands in + // for turns an unrelated feature into a phantom regression in another + // package. Answering `null` (no draft pending) is also legitimate — the + // point is that the method EXISTS; here it returns the body under test so + // the gate sees what the promotion will actually publish. (protocol as any).getOverlayRepo = () => ({ + get: async (_ref: unknown, opts?: { state?: string }) => + opts?.state === 'draft' ? { body } : null, promoteDraft: async () => ({ version: 'sha256:x', seq: 7, item: { body } }), }); const applySeedBodies = vi diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 18a2a4313e..fb00502e88 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -976,6 +976,9 @@ importers: '@objectstack/formula': specifier: workspace:* version: link:../formula + '@objectstack/lint': + specifier: workspace:* + version: link:../lint '@objectstack/metadata-core': specifier: workspace:* version: link:../metadata-core