From e25a28f9a59e13ed602012170f8990d27caf8199 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 14:09:13 +0000 Subject: [PATCH 1/3] feat(spec)!: retire the last three deprecated authorable aliases (#3855) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Protocol 17 removes the three keys a schema transform used to fold into a canonical slot and drop from the parsed output. Every slot now has exactly one spelling. action.execute -> action.target field.conditionalRequired -> field.requiredWhen agent.knowledge.topics -> agent.knowledge.sources All three are pure key renames with unchanged values, and no runtime behaviour changes: each was already lowered at parse time and erased before any consumer saw it. What shrinks is the authorable surface. CHANGE DOCUMENTATION IS THE PRIMARY CHANNEL, not the error. Three registry entries carry the removal: - Three D2 conversions (`toMajor: 17`, `retiredFromLoadPath: true` from the day they land — 17 gives the aliases no acceptance window, deliberately). They put the rename into `spec-changes.json` (ADR-0087 D4), which composes across majors, so a consumer jumping several at once gets ONE answer rather than N changelogs. The generated upgrade guide and the `spec_changes` MCP tool are projections of that record. - A step-17 migration chain entry referencing them, so `os migrate meta --from ` rewrites the consumer's source mechanically. Verified end to end: all three keys rename in one pass. Same shape as `object-compactLayout-to-highlightFields`, which is already a retired-from-load-path entry paired with a schema tombstone. WHY THE KEYS REJECT RATHER THAN VANISH. None of the three schemas is `.strict()`, so deleting a key makes Zod silently STRIP it — the metadata parses clean and the setting never takes effect: a script action bound to nothing, a field that is never required, an agent recruiting no RAG context. `FieldSchema` already carries a comment about the last time that happened (`dataQuality` / `cached`, #3726 / #3733). Each key is therefore tombstoned via a new `retiredKey()` helper: declared as `never`, so writing it is a tsc error at the authoring site AND a parse error carrying the rename and the `migrate meta` pointer. It also renders as a `[REMOVED]` row in the generated reference docs. `lintDeprecatedAliases` retires with its subject. It warned when an author declared both an alias and its canonical key, because the parse resolved that silently; the parse now rejects, which is strictly louder. Its `defineStack` and CLI wiring goes with it — the conversion-notice wiring from #3872 stays. `lowerCallables` stops binding a function on `execute`. It runs before the parse, so binding it there would have kept the removed alias quietly working for inline-function authoring while every other style rejected it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01JqC9ckaLbmhmytt65Gxi96 --- .changeset/retire-three-deprecated-aliases.md | 76 +++++ content/docs/ai/agents.mdx | 2 +- content/docs/data-modeling/fields.mdx | 16 +- content/docs/protocol/objectui/actions.mdx | 4 +- content/docs/references/ai/agent.mdx | 14 +- content/docs/references/data/field.mdx | 4 +- content/docs/references/ui/action.mdx | 2 +- packages/cli/src/commands/compile.ts | 49 +-- packages/cli/src/commands/validate.ts | 17 +- .../cli/src/utils/lower-callables.test.ts | 89 +---- packages/cli/src/utils/lower-callables.ts | 62 +--- packages/spec/api-surface.json | 6 - packages/spec/src/ai/agent.test.ts | 33 +- packages/spec/src/ai/agent.zod.ts | 23 +- packages/spec/src/conversions/registry.ts | 167 ++++++++- packages/spec/src/data/field.test.ts | 102 +++--- packages/spec/src/data/field.zod.ts | 51 ++- packages/spec/src/index.ts | 12 - packages/spec/src/migrations/registry.ts | 35 ++ .../src/shared/deprecated-aliases.test.ts | 316 ------------------ .../spec/src/shared/deprecated-aliases.ts | 305 ----------------- packages/spec/src/shared/retired-key.ts | 62 ++++ packages/spec/src/stack.test.ts | 114 ------- packages/spec/src/stack.zod.ts | 37 -- packages/spec/src/ui/action.test.ts | 100 +++--- packages/spec/src/ui/action.zod.ts | 49 +-- 26 files changed, 576 insertions(+), 1171 deletions(-) create mode 100644 .changeset/retire-three-deprecated-aliases.md delete mode 100644 packages/spec/src/shared/deprecated-aliases.test.ts delete mode 100644 packages/spec/src/shared/deprecated-aliases.ts create mode 100644 packages/spec/src/shared/retired-key.ts diff --git a/.changeset/retire-three-deprecated-aliases.md b/.changeset/retire-three-deprecated-aliases.md new file mode 100644 index 0000000000..a55a382dad --- /dev/null +++ b/.changeset/retire-three-deprecated-aliases.md @@ -0,0 +1,76 @@ +--- +"@objectstack/spec": major +"@objectstack/cli": patch +--- + +feat(spec)!: retire the last three deprecated authorable aliases (#3855) + +Protocol 17 removes the three keys that a schema transform used to fold into a +canonical slot and drop from the parsed output. Every slot now has exactly one +spelling. + +## Migration + +| Removed | Use instead | Value shape | +|---|---|---| +| `action.execute` | `action.target` | unchanged — a handler / flow / URL ref | +| `field.conditionalRequired` | `field.requiredWhen` | unchanged — a CEL predicate | +| `agent.knowledge.topics` | `agent.knowledge.sources` | unchanged — a list of source tags | + +All three are **pure key renames**. Nothing about the value changes, and no +runtime behaviour changes: each alias was already lowered into its canonical key +at parse time and erased before any consumer saw it, so what shrinks is the +authorable surface, not the semantics. + +**Run `os migrate meta --from `.** It rewrites your source +mechanically — these renames are registered as protocol-17 chain steps, so the +tool applies all three (and every earlier step you skipped) in one pass. Manual +alternative: rename the key. That is the entire fix. + +```diff +- actions: [{ name: 'convert', type: 'script', execute: 'convertHandler' }] ++ actions: [{ name: 'convert', type: 'script', target: 'convertHandler' }] + +- fields: { due_date: { type: 'date', conditionalRequired: 'record.stage == "closed"' } } ++ fields: { due_date: { type: 'date', requiredWhen: 'record.stage == "closed"' } } + +- knowledge: { topics: ['faq', 'policies'], indexes: ['docs'] } ++ knowledge: { sources: ['faq', 'policies'], indexes: ['docs'] } +``` + +## Why these reject instead of being ignored + +None of the three schemas is `.strict()`, so deleting a key outright makes Zod +**silently strip** it: the metadata would parse clean and the setting would +simply never take effect — a script action bound to nothing, a field that is +never required, an agent recruiting no RAG context. `FieldSchema` already +carries a comment about the last time that happened (`dataQuality` / `cached`, +#3726 / #3733). + +So each removed key is **tombstoned**: it stays declared as `never`, which makes +writing it a `tsc` error at the authoring site *and* a parse error carrying the +rename. You cannot lose the setting quietly. + +## Where to find this if you missed it + +The removal is in the machine-readable change manifest (`spec-changes.json`, +ADR-0087 D4) as three protocol-17 conversions. Per-major manifests **compose**, +so jumping several majors at once still yields a single answer rather than N +changelogs to reconcile — the generated upgrade guide and the `spec_changes` MCP +tool are both projections of that record. + +## Also removed + +`lintDeprecatedAliases` and its rule-id exports (`ACTION_TARGET_EXECUTE_CONFLICT`, +`FIELD_REQUIREDWHEN_CONDITIONALREQUIRED_CONFLICT`, +`AGENT_KNOWLEDGE_SOURCES_TOPICS_CONFLICT`, `DeprecatedAliasFinding`, +`formatDeprecatedAliasFinding`). That pass existed to warn when an author +declared both an alias and its canonical key, because the parse resolved the +conflict silently. With the aliases gone the parse **rejects** instead, which is +strictly louder — the rule has no subject left. If you imported any of these, +delete the import; there is no replacement because the condition it reported can +no longer occur. + +The CLI's inline-handler lowering also stops binding a function on `execute`. It +runs before the parse, so binding it there would have kept the removed alias +quietly working for one authoring style while every other style rejected it. diff --git a/content/docs/ai/agents.mdx b/content/docs/ai/agents.mdx index e1ce193058..d8979f07c9 100644 --- a/content/docs/ai/agents.mdx +++ b/content/docs/ai/agents.mdx @@ -137,7 +137,7 @@ own `ask` / `build` records use exactly these fields): | `model` | Provider + model config (`provider`: `openai` \| `azure_openai` \| `anthropic` \| `local`) | | `skills` | Skill names to attach (the primary Agent → Skill → Tool capability model) | | `tools` | Direct tool **references** `{ type, name, description }` — `type` is `action` \| `flow` \| `query` \| `vector_search`; `name` points at an existing Action/Flow/query | -| `knowledge` | RAG access: `{ sources: string[], indexes: string[] }`. `sources` is the canonical key the renderer reads; `topics` is a deprecated alias folded into it at parse time | +| `knowledge` | RAG access: `{ sources: string[], indexes: string[] }`. `sources` is the only key; the `topics` alias was removed in protocol 17 (#3855) — `os migrate meta --from 16` rewrites it | There is no `type` field and no fixed agent "type" taxonomy — behaviour comes from persona, instructions, skills, and tools. There are no `triggers` / `schedule` diff --git a/content/docs/data-modeling/fields.mdx b/content/docs/data-modeling/fields.mdx index 5616f66052..acf1c67f04 100644 --- a/content/docs/data-modeling/fields.mdx +++ b/content/docs/data-modeling/fields.mdx @@ -314,7 +314,6 @@ For values that must be encrypted at rest (API keys, tokens, DB passwords), use | `visibleWhen` | `string \| Expression` | CEL predicate; field is shown only when `TRUE` | | `readonlyWhen` | `string \| Expression` | CEL predicate; field is read-only when `TRUE` | | `requiredWhen` | `string \| Expression` | CEL predicate; field is required when `TRUE` | -| `conditionalRequired` | `string \| Expression` | Deprecated alias of `requiredWhen` — lowered into it at parse time, then dropped | | `multiple` | `boolean` | Allow multiple values (for select, lookup, file) | ```typescript @@ -343,17 +342,10 @@ visibility, read-only state, and required markers live as the draft record changes, including row-scoped rules inside inline master-detail grids. The server enforces `requiredWhen` on submit and ignores writes to fields whose `readonlyWhen` predicate is `TRUE`, so client-side affordances are not the -only guard. Use `requiredWhen` for new metadata; keep `conditionalRequired` -only for older packages that already emitted it. The alias is still accepted on -input, but it is lowered into `requiredWhen` at parse time and then removed from -the parsed metadata, so every consumer reads one canonical slot — if a field -declares both, `requiredWhen` wins and the alias is discarded. Declaring both -with *different* predicates means the `conditionalRequired` one never gates the -field, so it raises an advisory `field-requiredwhen-conditionalrequired-conflict` -warning naming both — from `defineStack` when it parses your config, and from -`os build` / `os validate` for stacks that skip strict `defineStack`. It never -fails the build; the fix is to delete `conditionalRequired`. The same predicate -in both slots is harmless duplication and stays quiet. +only guard. Use `requiredWhen`. The deprecated `conditionalRequired` alias was removed in +protocol 17 (#3855): authoring it is rejected with an error naming the +replacement, not silently stripped, and `os migrate meta --from 16` rewrites +existing sources automatically. ## Naming Conventions diff --git a/content/docs/protocol/objectui/actions.mdx b/content/docs/protocol/objectui/actions.mdx index e73f291d64..c0abeaf4f0 100644 --- a/content/docs/protocol/objectui/actions.mdx +++ b/content/docs/protocol/objectui/actions.mdx @@ -51,9 +51,7 @@ The `type` field selects how an action is dispatched. The complete enum is **`sc | `api` | Call an API endpoint (`method` defaults to `POST`). | Yes | | `form` | Open a FormView by name, routed to `/console/forms/:name`. | Yes | -`target` is the canonical binding for every non-`script` type. The deprecated `execute` field is lowered into `target` during parsing and then **removed from the parsed metadata**, so every consumer — server dispatch and renderer alike — reads exactly one slot. If an action declares both, `target` wins and `execute` is discarded. - -Declaring both with *different* values means one of the two handlers you wrote never runs, so it raises an advisory `action-target-execute-conflict` warning naming both — from `defineStack` when it parses your config, and from `os build` / `os validate` for stacks that skip strict `defineStack`. It never fails the build; the fix is to delete `execute`. The same value in both slots is harmless duplication and stays quiet. +`target` is the canonical binding for every non-`script` type, and since protocol 17 it is the **only** one. The deprecated `execute` alias was removed (#3855): authoring it is rejected with an error naming the replacement, not silently stripped. Run `os migrate meta --from 16` to rewrite existing sources automatically — the removal also ships in the machine-readable change manifest (`spec-changes.json`), which composes across however many majors you are jumping. ### Script Actions diff --git a/content/docs/references/ai/agent.mdx b/content/docs/references/ai/agent.mdx index 8611e59f17..b3c9e414bc 100644 --- a/content/docs/references/ai/agent.mdx +++ b/content/docs/references/ai/agent.mdx @@ -30,7 +30,7 @@ const result = AIKnowledge.parse(data); | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | | **sources** | `string[]` | optional | Knowledge sources/tags to recruit RAG context from. Canonical key consumed by the agent renderer (objectui AgentPreview KnowledgeSummary). | -| **topics** | `string[]` | optional | Deprecated alias for `sources` (spec key ≠ consumed key drift, liveness audit #1878/#1891). Folded into `sources` at parse time; prefer `sources`. | +| **topics** | `any` | optional | [REMOVED] `knowledge.topics` was removed in @objectstack/spec 17 (#3855) — use `knowledge.sources`. Rename the key; the value (a list of source tags) is unchanged. Run `os migrate meta --from 16` to rewrite it automatically. | | **indexes** | `string[]` | ✅ | Vector Store Indexes | @@ -75,19 +75,19 @@ const result = AIKnowledge.parse(data); | **avatar** | `string` | optional | | | **role** | `string` | ✅ | The persona/role (e.g. "Senior Support Engineer") | | **instructions** | `string` | ✅ | System Prompt / Prime Directives | -| **model** | `{ provider?: Enum<'openai' \| 'azure_openai' \| 'anthropic' \| 'local'>; model: string; temperature?: number; maxTokens?: number; … }` | optional | | +| **model** | `{ provider: Enum<'openai' \| 'azure_openai' \| 'anthropic' \| 'local'>; model: string; temperature: number; maxTokens?: number; … }` | optional | | | **lifecycle** | `{ id: string; description?: string; contextSchema?: Record; initial: string; … }` | optional | [EXPERIMENTAL — not enforced] State machine defining the agent conversation flow and constraints. Parsed but no runtime consumer yet (liveness #1878/#1893). | -| **surface** | `Enum<'ask' \| 'build'>` | optional | Product surface this agent binds ('ask' \| 'build') — ADR-0063 §1 | +| **surface** | `Enum<'ask' \| 'build'>` | ✅ | Product surface this agent binds ('ask' \| 'build') — ADR-0063 §1 | | **skills** | `string[]` | optional | Skill names to attach (Agent→Skill→Tool architecture) | | **tools** | `{ type: Enum<'action' \| 'flow' \| 'query' \| 'vector_search'>; name: string; description?: string }[]` | optional | Direct tool references (legacy fallback) | -| **knowledge** | `{ sources?: string[]; topics?: string[]; indexes: string[] }` | optional | RAG access | -| **active** | `boolean` | optional | | +| **knowledge** | `{ sources?: string[]; topics?: any; indexes: string[] }` | optional | RAG access | +| **active** | `boolean` | ✅ | | | **access** | `string[]` | optional | Who can chat with this agent | | **permissions** | `string[]` | optional | Required permission-set capabilities | -| **planning** | `{ maxIterations?: integer }` | optional | Autonomous reasoning and planning configuration | +| **planning** | `{ maxIterations: integer }` | optional | Autonomous reasoning and planning configuration | | **memory** | `{ longTerm?: object; reflectionInterval?: integer }` | optional | [EXPERIMENTAL — not enforced] Agent memory management. Parsed but no runtime consumer yet (liveness #1878/#1893). | | **guardrails** | `{ maxTokensPerInvocation?: integer; maxExecutionTimeSec?: integer; blockedTopics?: string[] }` | optional | [EXPERIMENTAL — not enforced] Safety guardrails for the agent. Parsed but not enforced — real limits come from the quota service (liveness #1878/#1893). | -| **structuredOutput** | `{ format: Enum<'json_object' \| 'json_schema' \| 'regex' \| 'grammar' \| 'xml'>; schema?: Record; strict?: boolean; retryOnValidationFailure?: boolean; … }` | optional | [EXPERIMENTAL — not enforced] Structured output format and validation configuration. Parsed but no runtime consumer yet (liveness #1878/#1893). | +| **structuredOutput** | `{ format: Enum<'json_object' \| 'json_schema' \| 'regex' \| 'grammar' \| 'xml'>; schema?: Record; strict: boolean; retryOnValidationFailure: boolean; … }` | optional | [EXPERIMENTAL — not enforced] Structured output format and validation configuration. Parsed but no runtime consumer yet (liveness #1878/#1893). | | **protection** | `{ lock: Enum<'none' \| 'no-overlay' \| 'no-delete' \| 'full'>; reason: string; docsUrl?: string }` | optional | Package author protection block — lock policy for this agent. | | **_lock** | `Enum<'none' \| 'no-overlay' \| 'no-delete' \| 'full'>` | optional | Item-level lock — controls overlay & delete (ADR-0010). | | **_lockReason** | `string` | optional | Human-readable reason shown when a write is refused by _lock. | diff --git a/content/docs/references/data/field.mdx b/content/docs/references/data/field.mdx index a2244ec727..490c174351 100644 --- a/content/docs/references/data/field.mdx +++ b/content/docs/references/data/field.mdx @@ -117,8 +117,8 @@ const result = Address.parse(data); | **group** | `string` | optional | Field group name for organizing fields in forms and layouts (e.g., "contact_info", "billing", "system") | | **visibleWhen** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | Predicate (CEL) — field is shown only when TRUE (else hidden). e.g. P`record.type == 'invoice'` | | **readonlyWhen** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | Predicate (CEL) — field is read-only when TRUE. e.g. P`record.status == 'paid'` | -| **requiredWhen** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | Predicate (CEL) — field is required when TRUE. Canonical slot; the deprecated `conditionalRequired` alias is lowered into this one at parse time. | -| **conditionalRequired** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | @deprecated — Use requiredWhen instead. Lowered into requiredWhen during parsing and dropped from the parsed output; when both are set, requiredWhen wins. | +| **requiredWhen** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | Predicate (CEL) — field is required when TRUE. The only slot; the `conditionalRequired` alias was removed in protocol 17 (#3855). | +| **conditionalRequired** | `any` | optional | [REMOVED] `conditionalRequired` was removed in @objectstack/spec 17 (#3855) — use `requiredWhen`. Rename the key; the value (a CEL predicate) is unchanged. Run `os migrate meta --from 16` to rewrite it automatically. | | **widget** | `string` | optional | Form widget override — names a registered field component (resolved as `field:`) to render this field instead of the `type` default. Degrades to the `type` renderer when unregistered. e.g. "object-ref", "filter-condition", "recipient-picker". | | **hidden** | `boolean` | optional | Hidden from default UI | | **readonly** | `boolean` | optional | Read-only — never editable in forms, AND server-enforced on BOTH write paths: a non-system write to this field is silently dropped from the payload on UPDATE (#2948/#3003) and on INSERT (#3043; a create can no longer directly seed e.g. `approval_status: "approved"`), symmetric with `readonlyWhen`. A stripped INSERT field still falls back to its `defaultValue`. Exempt from the strip: `isSystem` writes (seed replay, migration), and an opt-in "historical" import (`preserveAudit`, #3493) — which admits a whitelist (the audit/timestamp family plus author-declared business `readonly` fields). A normal (non-system) import is NOT system-context and still strips. | diff --git a/content/docs/references/ui/action.mdx b/content/docs/references/ui/action.mdx index b6cc865511..b0ce874890 100644 --- a/content/docs/references/ui/action.mdx +++ b/content/docs/references/ui/action.mdx @@ -87,7 +87,7 @@ const result = Action.parse(data); | **target** | `string` | optional | URL, Script Name, Flow ID, or API Endpoint. Supports $`{param.X}` and $`{ctx.X}` interpolation. | | **openIn** | `Enum<'self' \| 'new-tab'>` | optional | For type:'url' — where to open `target`. 'new-tab' opens a new browser tab; 'self' navigates in place. When omitted, external/absolute URLs open in a new tab and relative URLs navigate in place. Static execution option — keep it OUT of `params` (which is user-input-collection only). | | **body** | `{ language: 'expression'; source: string } \| { language: 'js'; source: string; capabilities?: Enum<'api.read' \| 'api.write' \| 'api.transaction' \| 'crypto.uuid' \| 'crypto.hash' \| 'log'>[]; timeoutMs?: integer; … }` | optional | Action body — expression (L1) or sandboxed JS (L2). Only used when type is `script`. | -| **execute** | `string` | optional | @deprecated — Use target instead. Lowered into target during parsing and dropped from the parsed output; when both are set, target wins. | +| **execute** | `any` | optional | [REMOVED] `execute` was removed in @objectstack/spec 17 (#3855) — use `target`. Rename the key; the value (a handler / flow / URL ref) is unchanged. Run `os migrate meta --from 16` to rewrite it automatically. | | **params** | `{ name?: string; field?: string; objectOverride?: string; label?: string; … }[]` | optional | Input parameters required from user | | **variant** | `Enum<'primary' \| 'secondary' \| 'danger' \| 'ghost' \| 'link'>` | optional | Button visual variant for styling (primary = highlighted, danger = destructive, ghost = transparent) | | **order** | `number` | optional | Sort order within a location group (lower = higher). Promotes/demotes an action toward the record_header primary button; stable, so actions without `order` keep their registration order. | diff --git a/packages/cli/src/commands/compile.ts b/packages/cli/src/commands/compile.ts index fb1426d40a..09bec64fce 100644 --- a/packages/cli/src/commands/compile.ts +++ b/packages/cli/src/commands/compile.ts @@ -5,7 +5,7 @@ import path from 'path'; import fs from 'fs'; import chalk from 'chalk'; import { ZodError } from 'zod'; -import { ObjectStackDefinitionSchema, normalizeStackInput, lintDeprecatedAliases, type ConversionNotice } from '@objectstack/spec'; +import { ObjectStackDefinitionSchema, normalizeStackInput, type ConversionNotice } from '@objectstack/spec'; import { loadConfig } from '../utils/config.js'; import { lowerCallables } from '../utils/lower-callables.js'; import { validateStackExpressions } from '@objectstack/lint'; @@ -115,53 +115,6 @@ export default class Compile extends Command { } } - // 2a. [#3743] PRE-PARSE authoring lint. Everything in the post-parse lint - // block (3d and below) reads `result.data`, which is the wrong side of - // the fence for an alias the pipeline itself consumes: `lowerCallables` - // drops a function-valued `execute` and the `ActionSchema` transform - // drops a string one (#3742), so by then "the author declared both - // slots" is no longer representable. This pass runs on `normalized` — - // before lowering, before parse — where both slots are still as - // written, and it is the same input `os validate` lints, so the two - // surfaces agree by construction (#3782). - // - // It reports the discards THIS pipeline performs. A stack authored - // with strict `defineStack` has already been parsed inside its own - // config module, so its alias was consumed — and warned about — there; - // this pass covers everything that skipped that gate: a plain object - // default-export, `defineStack(…, { strict: false })`, and inline - // function handlers (`target: z.string()` rejects those, so they can - // only reach the pipeline via a non-strict path and are lowered here). - // - // Advisory today; `severity: 'error'` is honoured so a future rule - // gates the build here without further wiring. - if (!flags.json) printStep('Checking deprecated aliases (#3743)...'); - const aliasLint = lintDeprecatedAliases(normalized as Record); - const aliasLintErrors = aliasLint.filter((f) => f.severity === 'error'); - const aliasLintWarnings = aliasLint.filter((f) => f.severity !== 'error'); - if (aliasLintWarnings.length > 0 && !flags.json) { - console.log(''); - for (const f of aliasLintWarnings) { - printWarning(`${f.where}: ${f.message}`); - console.log(chalk.dim(` ${f.hint}`)); - console.log(chalk.dim(` rule: ${f.rule}`)); - } - } - if (aliasLintErrors.length > 0) { - if (flags.json) { - await emitJson({ success: false, error: 'deprecated alias check failed', issues: aliasLintErrors }, 0, { compact: true }); - this.exit(1); - } - console.log(''); - printError(`Deprecated alias check failed (${aliasLintErrors.length} issue${aliasLintErrors.length > 1 ? 's' : ''})`); - for (const f of aliasLintErrors) { - console.log(` • ${f.where}: ${f.message}`); - console.log(chalk.dim(` ${f.hint}`)); - console.log(chalk.dim(` rule: ${f.rule}`)); - } - this.exit(1); - } - // 2b. Lower inline `function` handlers (Hook.handler, top-level // `functions`) to stable string refs BEFORE Zod parse. This // guarantees we extract the user's real function identity (Zod's diff --git a/packages/cli/src/commands/validate.ts b/packages/cli/src/commands/validate.ts index ec9e3a536f..ea879437fc 100644 --- a/packages/cli/src/commands/validate.ts +++ b/packages/cli/src/commands/validate.ts @@ -6,7 +6,7 @@ import { createRequire } from 'node:module'; import { join, dirname } from 'node:path'; import chalk from 'chalk'; import { ZodError } from 'zod'; -import { ObjectStackDefinitionSchema, normalizeStackInput, lintDeprecatedAliases, type ConversionNotice } from '@objectstack/spec'; +import { ObjectStackDefinitionSchema, normalizeStackInput, type ConversionNotice } from '@objectstack/spec'; import { loadConfig } from '../utils/config.js'; import { validateStackExpressions } from '@objectstack/lint'; import { validateListViewMode } from '@objectstack/lint'; @@ -592,25 +592,12 @@ export default class Validate extends Command { const viewRefErrors = viewRefLint.filter((f) => f.severity === 'error'); const viewRefWarnings = viewRefLint.filter((f) => f.severity !== 'error'); - // Deprecated aliases (#3743). The ONE lint here that reads `normalized` - // rather than `result.data`, and it has to: the parse consumes the alias - // it reports (`ActionSchema` folds `execute` into `target` and drops it), - // so a post-parse read is structurally blind to the conflict. `os build` - // lints the same pre-parse input, so both surfaces see the same findings. - // A stack authored with strict `defineStack` was already parsed — and - // warned about — inside its own config module; this covers the paths that - // reach the CLI with the alias intact. - const aliasLint = lintDeprecatedAliases(normalized as Record); - const aliasLintErrors = aliasLint.filter((f) => f.severity === 'error'); - const aliasLintWarnings = aliasLint.filter((f) => f.severity !== 'error'); - - const authoringLintErrors = [...flowLintErrors, ...autonumberErrors, ...viewRefErrors, ...aliasLintErrors]; + const authoringLintErrors = [...flowLintErrors, ...autonumberErrors, ...viewRefErrors]; const authoringLintWarnings = [ ...flowLintWarnings, ...livenessLint, ...autonumberWarnings, ...viewRefWarnings, - ...aliasLintWarnings, ]; if (authoringLintErrors.length > 0) { if (flags.json) { diff --git a/packages/cli/src/utils/lower-callables.test.ts b/packages/cli/src/utils/lower-callables.test.ts index 41ae8dc023..f6ec974a6b 100644 --- a/packages/cli/src/utils/lower-callables.test.ts +++ b/packages/cli/src/utils/lower-callables.test.ts @@ -3,59 +3,39 @@ import { describe, it, expect } from 'vitest'; import { lowerCallables } from './lower-callables.js'; -// ── #3713: `target` vs the deprecated `execute` alias — one precedence, one slot ── +// ── #3855: `target` is the only handler slot ──────────────────────────────── // -// `execute` is the deprecated alias of `target`. Before this was aligned, three -// readers resolved "the author declared both" in two directions: the spec -// transform kept `target`, objectui's ActionRunner did `execute || target`, and -// THIS compile step preferred `execute`. The compile-step half was the nastiest: -// it bundled the `execute` function and then overwrote `action.target` with the -// resulting ref, so the function the author actually declared on `target` was -// silently dropped from the build. -describe('lowerCallables — action handler slot precedence (#3713)', () => { +// `execute` was the deprecated alias of `target`. #3713/#3742/#3743 aligned the +// three readers that disagreed about it; protocol 17 removes it outright. +// +// The load-bearing rule for THIS step: lowering runs BEFORE the parse, so it +// must not bind a function-valued `execute`. Doing so would consume the key and +// the schema tombstone would never fire — the removed alias would keep working +// in one authoring style (inline function) while being rejected in every other, +// which is the dialect split the removal exists to end. +describe('lowerCallables — only `target` binds a handler (#3855)', () => { const actionsOf = (result: { lowered: Record }) => (result.lowered as { actions: Array> }).actions; - it('binds the canonical `target` function when both slots carry a callable', () => { + it('binds an inline function on the canonical `target`', () => { const result = lowerCallables({ actions: [{ name: 'convert', label: 'Convert', type: 'script', target: function preferredHandler() { return 'preferred'; }, - execute: function legacyHandler() { return 'legacy'; }, }], }); const [action] = actionsOf(result); const ref = action.target as string; expect(typeof ref).toBe('string'); - // The bundled function must be the one from `target`, not from `execute`. expect(result.functions[ref]()).toBe('preferred'); - expect(Object.values(result.functions).map((fn) => fn())).not.toContain('legacy'); - }); - - it('drops the `execute` alias once a ref is bound to `target`', () => { - const result = lowerCallables({ - actions: [{ - name: 'convert', - label: 'Convert', - type: 'script', - target: function preferredHandler() { return 'preferred'; }, - execute: function legacyHandler() { return 'legacy'; }, - }], - }); - - const [action] = actionsOf(result); - // A leftover alias is stale by construction — and a function-valued one - // would fail `ActionSchema` (it expects a string) further down the pipeline. - expect('execute' in action).toBe(false); - // The lowered stack must be JSON-safe: no function values survive. expect(() => JSON.stringify(result.lowered)).not.toThrow(); - expect(JSON.stringify(result.lowered)).not.toContain('legacy'); }); - it('still lowers an `execute`-only callable (back-compat) onto `target`', () => { + it('leaves a function on the removed `execute` alias for the parse to reject', () => { + // Binding it here would be the alias quietly surviving its own removal. const result = lowerCallables({ actions: [{ name: 'legacy_only', @@ -66,16 +46,12 @@ describe('lowerCallables — action handler slot precedence (#3713)', () => { }); const [action] = actionsOf(result); - const ref = action.target as string; - expect(typeof ref).toBe('string'); - expect(result.functions[ref]()).toBe('legacy'); - expect('execute' in action).toBe(false); + expect(result.count).toBe(0); + expect(action.target).toBeUndefined(); + expect(typeof action.execute).toBe('function'); }); - it('leaves a string-valued handler pair untouched (the spec transform owns it)', () => { - // No callable to lower here, so the compile step must not editorialise: the - // spec's `ActionSchema` transform is the single place that resolves a - // string/string pair (keeping `target`, dropping `execute`). + it('leaves a string `execute` alone too — the parse owns the rejection', () => { const result = lowerCallables({ actions: [{ name: 'strings', @@ -92,34 +68,7 @@ describe('lowerCallables — action handler slot precedence (#3713)', () => { expect(result.count).toBe(0); }); - // #3743: "`target` first" meant "a CALLABLE `target` first", which left one - // combination still resolving the alias's way — a string `target` beside a - // function `execute` bundled the alias and then overwrote the canonical ref - // the author wrote. Same inversion as above, one slot type over. - it('keeps a string `target` when the alias holds an inline function', () => { - const result = lowerCallables({ - actions: [{ - name: 'mixed', - label: 'Mixed', - type: 'script', - target: 'preferredHandler', - execute: function legacyHandler() { return 'legacy'; }, - }], - }); - - const [action] = actionsOf(result); - expect(action.target).toBe('preferredHandler'); - expect(result.count).toBe(0); - // The losing alias still has to go: a function value is not JSON-safe and - // `ActionSchema` expects a string, so leaving it would trade the silent - // discard for a confusing parse error. `lintDeprecatedAliases` is what - // tells the author which handler they lost (#3743). - expect('execute' in action).toBe(false); - expect(() => JSON.stringify(result.lowered)).not.toThrow(); - expect(JSON.stringify(result.lowered)).not.toContain('legacy'); - }); - - it('applies the same precedence to actions nested under an object', () => { + it('binds a callable `target` on an action nested under an object', () => { const result = lowerCallables({ objects: [{ name: 'crm_deal', @@ -128,7 +77,6 @@ describe('lowerCallables — action handler slot precedence (#3713)', () => { label: 'Convert', type: 'script', target: function preferredHandler() { return 'preferred'; }, - execute: function legacyHandler() { return 'legacy'; }, }], }], }); @@ -137,6 +85,5 @@ describe('lowerCallables — action handler slot precedence (#3713)', () => { const [action] = objects[0].actions; const ref = action.target as string; expect(result.functions[ref]()).toBe('preferred'); - expect('execute' in action).toBe(false); }); }); diff --git a/packages/cli/src/utils/lower-callables.ts b/packages/cli/src/utils/lower-callables.ts index 1de9d6dbf6..3c084d6264 100644 --- a/packages/cli/src/utils/lower-callables.ts +++ b/packages/cli/src/utils/lower-callables.ts @@ -105,8 +105,8 @@ export function lowerCallables(input: Record): LoweringResult { } // 1b. Lower inline action handlers found inside `objects[*].actions[*]` - // and `actions[*]`. We accept either `target: fn` or the deprecated - // `execute: fn`; `target` wins when both are present (#3713). + // and `actions[*]`. Only `target: fn` — the `execute` alias was removed + // in protocol 17 (#3855) and is left for the parse to reject by name. if (Array.isArray(lowered.objects)) { lowered.objects = (lowered.objects as unknown[]).map((rawObj) => { if (!isPlainObject(rawObj)) return rawObj; @@ -174,11 +174,17 @@ export function lowerCallables(input: Record): LoweringResult { } /** - * Lower a single action definition: detect a callable on `target` or on the - * deprecated `execute` alias, register it, optionally extract a metadata body, - * and drop the alias. A declared `target` — string or function — always wins - * over `execute` (#3713 / #3743), matching the `ActionSchema` transform. Mutates - * a shallow clone, never the input. + * Lower a single action definition: detect a callable on `target`, register it, + * and optionally extract a metadata body. Mutates a shallow clone, never the + * input. + * + * `target` is the ONLY handler slot. The deprecated `execute` alias was removed + * in protocol 17 (#3855) and this step must not keep it alive behind the + * schema's back: lowering runs BEFORE the parse, so quietly binding a + * function-valued `execute` here would consume the key and the tombstone would + * never fire — the author would get the alias silently working in one authoring + * style and rejected in every other. Leaving it untouched hands it to + * `ActionSchema`, which rejects it with the rename prescription. */ function lowerActionCallable( raw: unknown, @@ -192,39 +198,10 @@ function lowerActionCallable( const baseName = typeof action.name === 'string' && action.name.length > 0 ? `${ownerLabel}_${action.name}` : `${ownerLabel}_anon_action`; - // #3713: `target` is the canonical slot and `execute` its deprecated alias, so - // probe `target` FIRST. This used to prefer `execute`, which made the compile - // step the third reader of this pair — and it disagreed with the spec transform - // (which keeps `target` when both are set). An author who inlined a function in - // both slots got the `execute` one bundled while `action.target = ref` below - // silently overwrote the `target` function they had actually declared. - // - // #3743: "`target` first" means the DECLARED target, not merely a callable one. - // Probing `typeof action.target === 'function'` left one combination still - // resolving the alias's way: a STRING `target` beside a function `execute` fell - // through to the alias, bundled it, and then overwrote the canonical ref the - // author wrote — the same inversion #3713 fixed for the function/function pair, - // one slot type over. `target` now wins in every combination, so the - // `action-target-execute-conflict` warning can state one precedence rule and - // have it be true everywhere. - const targetDeclared = - typeof action.target === 'function' || (typeof action.target === 'string' && action.target.length > 0); - const handlerSlot: 'execute' | 'target' | null = - typeof action.target === 'function' - ? 'target' - : !targetDeclared && typeof action.execute === 'function' - ? 'execute' - : null; - if (!handlerSlot) { - // A function-valued alias that lost to a string `target` still has to go: it - // is not JSON-safe (`JSON.stringify` drops it from the artifact without a - // word) and `ActionSchema` expects a string, so leaving it would trade a - // silent discard for a confusing parse error. The lint above has already - // told the author which handler they lost. - if (typeof action.execute === 'function') delete action.execute; - return action; - } - const fn = action[handlerSlot] as AnyFn; + // Only a callable `target` is lowered. A function on the removed `execute` + // alias is deliberately left in place so the parse rejects it by name. + if (typeof action.target !== 'function') return action; + const fn = action.target as AnyFn; const ref = uniqueName(baseName, taken); taken.add(ref); functions[ref] = fn; @@ -234,10 +211,5 @@ function lowerActionCallable( } // Keep a string-named target so the legacy executor can still resolve it. action.target = ref; - // Drop the alias unconditionally, matching the spec transform's "canonical wins, - // alias disappears" rule (#3713). Once `target` carries the bundled ref, any - // leftover `execute` is stale by construction — and a function-valued one would - // fail `ActionSchema` (it expects a string) further down the pipeline. - if ('execute' in action) delete action.execute; return action; } diff --git a/packages/spec/api-surface.json b/packages/spec/api-surface.json index faa324aeb4..834163aeb7 100644 --- a/packages/spec/api-surface.json +++ b/packages/spec/api-surface.json @@ -1,8 +1,6 @@ { ".": [ - "ACTION_TARGET_EXECUTE_CONFLICT (const)", "ADMIN_FULL_ACCESS (const)", - "AGENT_KNOWLEDGE_SOURCES_TOPICS_CONFLICT (const)", "ALL_CONVERSIONS (const)", "AUDIENCE_ANCHOR_POSITIONS (const)", "Agent (type)", @@ -37,7 +35,6 @@ "DatasourceMappingRule (type)", "DatasourceMappingRuleSchema (const)", "DefineStackOptions (interface)", - "DeprecatedAliasFinding (interface)", "EVERYONE_POSITION (const)", "EvalUser (type)", "EvalUserInput (type)", @@ -52,7 +49,6 @@ "ExpressionMetaSchema (const)", "ExpressionSchema (const)", "F (const)", - "FIELD_REQUIREDWHEN_CONDITIONALREQUIRED_CONFLICT (const)", "GUEST_POSITION (const)", "MAP_SUPPORTED_FIELDS (const)", "MEMBERSHIP_ROLE_ADMIN (const)", @@ -148,13 +144,11 @@ "expandViewContainerWithDiagnostics (function)", "expression (function)", "findClosestMatches (function)", - "formatDeprecatedAliasFinding (function)", "formatSuggestion (function)", "formatZodError (function)", "formatZodIssue (function)", "isAggregatedViewContainer (function)", "isKnownPlatformCapability (function)", - "lintDeprecatedAliases (function)", "mapMembershipRole (function)", "normalizeMetadataCollection (function)", "normalizePluginMetadata (function)", diff --git a/packages/spec/src/ai/agent.test.ts b/packages/spec/src/ai/agent.test.ts index 7ba8426b72..41386776aa 100644 --- a/packages/spec/src/ai/agent.test.ts +++ b/packages/spec/src/ai/agent.test.ts @@ -102,21 +102,38 @@ describe('AIKnowledgeSchema', () => { expect(() => AIKnowledgeSchema.parse(knowledge)).not.toThrow(); }); - it('folds the deprecated topics alias into sources at parse time (#1891)', () => { - const parsed = AIKnowledgeSchema.parse({ + it('rejects the removed `topics` alias instead of silently stripping it (#3855)', () => { + // #1891 folded `topics` into `sources` and dropped it; protocol 17 removes + // it from the spec. Tombstoned rather than deleted because + // `AIKnowledgeSchema` is not `.strict()` — a silent strip would leave the + // agent recruiting no RAG context at all, which is #1878's original bug. + expect(() => AIKnowledgeSchema.parse({ topics: ['product_docs', 'faq'], indexes: ['vector_store_main'], - }); - expect(parsed.sources).toEqual(['product_docs', 'faq']); - expect('topics' in parsed).toBe(false); + })).toThrow(/`knowledge.topics` was removed/); + }); + + it('carries the upgrade prescription in the rejection itself', () => { + let message = ''; + try { + AIKnowledgeSchema.parse({ topics: ['legacy'], indexes: [] }); + } catch (err) { + message = String(err); + } + expect(message).toContain('use `knowledge.sources`'); + expect(message).toContain('os migrate meta'); }); - it('lets canonical sources win when both keys are present', () => { - const parsed = AIKnowledgeSchema.parse({ + it('rejects `topics` even when the canonical `sources` is also present', () => { + expect(() => AIKnowledgeSchema.parse({ sources: ['canonical'], topics: ['legacy'], indexes: [], - }); + })).toThrow(/`knowledge.topics` was removed/); + }); + + it('accepts the canonical `sources` on its own', () => { + const parsed = AIKnowledgeSchema.parse({ sources: ['canonical'], indexes: [] }); expect(parsed.sources).toEqual(['canonical']); expect('topics' in parsed).toBe(false); }); diff --git a/packages/spec/src/ai/agent.zod.ts b/packages/spec/src/ai/agent.zod.ts index b9a0bbfd20..29553132c0 100644 --- a/packages/spec/src/ai/agent.zod.ts +++ b/packages/spec/src/ai/agent.zod.ts @@ -1,6 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { z } from 'zod'; +import { retiredKey } from '../shared/retired-key'; import { ProtectionSchema } from '../shared/protection.zod'; import { MetadataProtectionFields } from '../kernel/metadata-protection.zod'; import { StateMachineSchema } from '../automation/state-machine.zod'; @@ -33,18 +34,18 @@ export const AIToolSchema = lazySchema(() => z.object({ */ export const AIKnowledgeSchema = lazySchema(() => z.object({ sources: z.array(z.string()).optional().describe('Knowledge sources/tags to recruit RAG context from. Canonical key consumed by the agent renderer (objectui AgentPreview KnowledgeSummary).'), - topics: z.array(z.string()).optional().describe('Deprecated alias for `sources` (spec key ≠ consumed key drift, liveness audit #1878/#1891). Folded into `sources` at parse time; prefer `sources`.'), + /** + * [REMOVED in protocol 17 — #3855] The deprecated alias of `sources`. + * Tombstoned rather than deleted: `AIKnowledgeSchema` is not `.strict()`, so a + * plain deletion would silently strip the key and the agent would quietly + * recruit no RAG context (#1878's original failure, restored). + */ + topics: retiredKey( + "`knowledge.topics` was removed in @objectstack/spec 17 (#3855) — use `knowledge.sources`. " + + 'Rename the key; the value (a list of source tags) is unchanged. ' + + "Run `os migrate meta --from 16` to rewrite it automatically.", + ), indexes: z.array(z.string()).describe('Vector Store Indexes'), -}).transform((input) => { - // #1891: fold the deprecated `topics` alias into the canonical `sources` and - // drop it from the output (mirrors `normalizeVisibleWhen`, ADR-0089 D2) so - // authoring `topics` is no longer a silent no-op — the renderer reads - // `sources`. The canonical key wins when both are present. - const { topics, ...rest } = input; - if (rest.sources === undefined && topics !== undefined) { - return { ...rest, sources: topics }; - } - return rest; })); /** diff --git a/packages/spec/src/conversions/registry.ts b/packages/spec/src/conversions/registry.ts index a48b07403a..99d0b1102e 100644 --- a/packages/spec/src/conversions/registry.ts +++ b/packages/spec/src/conversions/registry.ts @@ -15,7 +15,7 @@ * Until P2 exists these remain the permanent, replayable transform history. */ -import type { MetadataConversion } from './types.js'; +import type { ConversionApplication, MetadataConversion } from './types.js'; import { mapCollection, mapFlowNodes, mapPages, renameConfigKey, renameKey } from './walk.js'; /** @@ -607,6 +607,170 @@ const pageComponentVisibilityToVisibleWhen: MetadataConversion = { }, }; +/* ── Protocol 17: the three fold-and-drop aliases retire (#3855) ─────────────── + * + * `execute`, `conditionalRequired` and `topics` were each folded into their + * canonical key by a schema transform and dropped from the parsed output. They + * are now removed from the spec outright, and each schema TOMBSTONES its key + * with a fix-it error (`retiredKey`, `shared/retired-key.ts`) so the loader + * cannot quietly accept it — the same shape as + * `object-compactLayout-to-highlightFields` above. + * + * All three are therefore `retiredFromLoadPath: true` from the day they land: + * there is no alias window, deliberately. What the entries buy is the two + * things a consumer actually needs, neither of which is an error message: + * + * - they appear in `CONVERSIONS_BY_MAJOR[17]`, so `spec-changes.json` (D4) + * carries them — and the generated upgrade guide and the `spec_changes` MCP + * tool are projections of that record, composed across however many majors + * the consumer is jumping; + * - the step-17 chain entry references them by id, so + * `os migrate meta --from 16` REWRITES the consumer's source mechanically + * instead of asking them to hand-edit. + * + * The tombstone error is the backstop for someone who did neither, and it says + * so by pointing at `migrate meta`. + */ + +// `Dict` / `isDict` are module-private in `walk.ts`. Re-declared locally rather +// than widening that module's exports, which would grow the package API surface +// for an internal one-line type guard. +type Dict = Record; +const isDict = (v: unknown): v is Dict => typeof v === 'object' && v !== null && !Array.isArray(v); +type Emit = (detail: ConversionApplication) => void; + +/** Rename a key on every field of every object (and object extension). Fields + * are a RECORD keyed by field name, so `mapCollection` does not reach them. */ +function mapObjectFieldsKey(stack: Dict, collection: string, from: string, to: string, emit: Emit): Dict { + return mapCollection(stack, collection, (owner, path) => { + const fields = owner.fields; + if (!isDict(fields)) return owner; + let changed = false; + const next: Dict = {}; + for (const [name, def] of Object.entries(fields)) { + if (!isDict(def)) { + next[name] = def; + continue; + } + const renamed = renameKey(def, from, to); + if (renamed) { + emit({ from, to, path: `${path}.fields.${name}.${to}` }); + next[name] = renamed; + changed = true; + } else { + next[name] = def; + } + } + return changed ? { ...owner, fields: next } : owner; + }); +} + +/** + * Action `execute` → `target` (protocol 17, #3713 / #3742 / #3855). + * + * A pure key rename — the value (a handler/flow/URL ref) is unchanged. Actions + * appear both top-level and nested under their object, so both are walked. + */ +const actionExecuteToTarget: MetadataConversion = { + id: 'action-execute-to-target', + toMajor: 17, + retiredFromLoadPath: true, + surface: 'action.execute', + summary: "action key 'execute' → 'target' (the deprecated handler alias, #3713)", + apply(stack, emit) { + const renameOn = (action: Dict, path: string): Dict => { + const renamed = renameKey(action, 'execute', 'target'); + if (!renamed) return action; + emit({ from: 'execute', to: 'target', path: `${path}.target` }); + return renamed; + }; + const withTopLevel = mapCollection(stack, 'actions', renameOn); + return mapCollection(withTopLevel, 'objects', (obj, path) => { + const nested = mapCollection(obj, 'actions', (action, actionPath) => + renameOn(action, `${path}.${actionPath}`), + ); + return nested; + }); + }, + fixture: { + before: { + actions: [{ name: 'convert', label: 'Convert', type: 'script', execute: 'convertHandler' }], + }, + after: { + actions: [{ name: 'convert', label: 'Convert', type: 'script', target: 'convertHandler' }], + }, + expectedNotices: 1, + }, +}; + +/** + * Field `conditionalRequired` → `requiredWhen` (protocol 17, #3754 / #3855). + * + * A pure key rename — the value (a CEL predicate, bare or enveloped) is + * unchanged. Covers object fields and object-extension fields: the same + * `FieldSchema`, so the same alias. + */ +const fieldConditionalRequiredToRequiredWhen: MetadataConversion = { + id: 'field-conditionalRequired-to-requiredWhen', + toMajor: 17, + retiredFromLoadPath: true, + surface: 'field.conditionalRequired', + summary: "field key 'conditionalRequired' → 'requiredWhen' (the deprecated predicate alias, #3754)", + apply(stack, emit) { + const withObjects = mapObjectFieldsKey(stack, 'objects', 'conditionalRequired', 'requiredWhen', emit); + return mapObjectFieldsKey(withObjects, 'objectExtensions', 'conditionalRequired', 'requiredWhen', emit); + }, + fixture: { + before: { + objects: [{ + name: 'crm_task', + label: 'Task', + fields: { due_date: { type: 'date', conditionalRequired: 'record.stage == "closed"' } }, + }], + }, + after: { + objects: [{ + name: 'crm_task', + label: 'Task', + fields: { due_date: { type: 'date', requiredWhen: 'record.stage == "closed"' } }, + }], + }, + expectedNotices: 1, + }, +}; + +/** + * Agent `knowledge.topics` → `knowledge.sources` (protocol 17, #1891 / #3855). + * + * A pure key rename — the value (a list of RAG source tags) is unchanged. + */ +const agentKnowledgeTopicsToSources: MetadataConversion = { + id: 'agent-knowledge-topics-to-sources', + toMajor: 17, + retiredFromLoadPath: true, + surface: 'agent.knowledge.topics', + summary: "agent knowledge key 'topics' → 'sources' (the deprecated RAG-source alias, #1891)", + apply(stack, emit) { + return mapCollection(stack, 'agents', (agent, path) => { + const knowledge = agent.knowledge; + if (!isDict(knowledge)) return agent; + const renamed = renameKey(knowledge, 'topics', 'sources'); + if (!renamed) return agent; + emit({ from: 'topics', to: 'sources', path: `${path}.knowledge.sources` }); + return { ...agent, knowledge: renamed }; + }); + }, + fixture: { + before: { + agents: [{ name: 'support_bot', knowledge: { topics: ['faq', 'policies'], indexes: ['docs'] } }], + }, + after: { + agents: [{ name: 'support_bot', knowledge: { sources: ['faq', 'policies'], indexes: ['docs'] } }], + }, + expectedNotices: 1, + }, +}; + /** * All conversions, keyed by the protocol major that introduced the canonical * shape. Newest majors last; ordering within a major is application order. @@ -616,6 +780,7 @@ export const CONVERSIONS_BY_MAJOR: Readonly { }); }); -describe('FieldSchema - conditionalRequired → requiredWhen migration (#3754)', () => { - // These three cases used to assert that `conditionalRequired` SURVIVED parse, - // which pinned the alias into the output and left every consumer to invent its - // own precedence — the condition that produced #3713 for `action.execute`. The - // alias is now lowered into the canonical `requiredWhen` and dropped. - it('lowers a conditionalRequired formula into requiredWhen', () => { - const result = FieldSchema.parse({ +describe('FieldSchema — the `conditionalRequired` alias is REMOVED (#3855)', () => { + // #3754 lowered the alias into `requiredWhen` and dropped it from the output. + // Protocol 17 removes it from the spec entirely. + // + // TOMBSTONED rather than deleted, because `FieldSchema` is deliberately not + // `.strict()` — this object already carries a comment about the last time a + // key outlived its schema and got silently stripped (`dataQuality` / `cached`, + // #3726 / #3733). A silent strip here would mean the field is simply never + // required, with nothing to read anywhere. + it('rejects an authored `conditionalRequired` instead of silently stripping it', () => { + expect(() => FieldSchema.parse({ type: 'text', conditionalRequired: "status = 'closed_won'", - }); - expect(result.requiredWhen).toEqual({ dialect: 'cel', source: "status = 'closed_won'" }); - expect('conditionalRequired' in result).toBe(false); + })).toThrow(/`conditionalRequired` was removed/); }); - it('should accept a field without conditionalRequired (optional)', () => { - const result = FieldSchema.parse({ - type: 'text', - }); - expect(result.requiredWhen).toBeUndefined(); - expect('conditionalRequired' in result).toBe(false); + it('carries the upgrade prescription in the rejection itself', () => { + let message = ''; + try { + FieldSchema.parse({ type: 'text', conditionalRequired: 'amount > 1000' }); + } catch (err) { + message = String(err); + } + expect(message).toContain('use `requiredWhen`'); + expect(message).toContain('os migrate meta'); }); - it('should allow combining required and conditionalRequired', () => { - const result = FieldSchema.parse({ - type: 'text', - required: false, - conditionalRequired: 'amount > 1000', - }); - expect(result.required).toBe(false); - expect(result.requiredWhen).toEqual({ dialect: 'cel', source: 'amount > 1000' }); - expect('conditionalRequired' in result).toBe(false); - }); - - it('keeps requiredWhen when BOTH are declared, and drops the alias', () => { - const result = FieldSchema.parse({ + it('rejects it even when the canonical `requiredWhen` is also present', () => { + expect(() => FieldSchema.parse({ type: 'text', requiredWhen: "record.status == 'sent'", conditionalRequired: 'record.amount > 1000', - }); - expect(result.requiredWhen).toEqual({ dialect: 'cel', source: "record.status == 'sent'" }); - expect('conditionalRequired' in result).toBe(false); - expect(Object.keys(result)).not.toContain('conditionalRequired'); - expect(JSON.parse(JSON.stringify(result)).conditionalRequired).toBeUndefined(); + })).toThrow(/`conditionalRequired` was removed/); }); - it('survives a field nested in an object (the real authoring path)', () => { - // The alias has to be gone from the metadata a renderer actually receives, - // not just from a bare FieldSchema.parse() in a unit test. - const parsed = ObjectSchema.parse({ + it('rejects it on the real authoring path — a field nested in an object', () => { + // The removal has to bite where authors actually write fields, not only in + // a bare `FieldSchema.parse()`. + expect(() => ObjectSchema.parse({ name: 'crm_deal', label: 'Deal', fields: { amount: { type: 'currency', conditionalRequired: "record.stage == 'closing'" }, }, + })).toThrow(/`conditionalRequired` was removed/); + }); + + it('accepts the canonical `requiredWhen`, alone or beside `required`', () => { + const result = FieldSchema.parse({ + type: 'text', + required: false, + requiredWhen: 'amount > 1000', }); - const amount = parsed.fields.amount as Record; - expect(amount.requiredWhen).toEqual({ dialect: 'cel', source: "record.stage == 'closing'" }); - expect('conditionalRequired' in amount).toBe(false); + expect(result.required).toBe(false); + expect(result.requiredWhen).toEqual({ dialect: 'cel', source: 'amount > 1000' }); + expect('conditionalRequired' in result).toBe(false); + }); + + it('accepts a field that declares neither', () => { + const result = FieldSchema.parse({ type: 'text' }); + expect(result.requiredWhen).toBeUndefined(); + expect('conditionalRequired' in result).toBe(false); }); }); @@ -1022,16 +1026,18 @@ describe('FieldSchema - conditional field rules (visibleWhen / readonlyWhen / re expect(result.requiredWhen).toBeUndefined(); }); - it('requiredWhen and its alias conditionalRequired can NOT coexist — canonical wins, alias dropped (#3754)', () => { - // Inverted from the original assertion, which pinned the two keys as - // coexisting in the parsed output. Coexistence is the whole problem: it makes - // every consumer re-derive the precedence, which is how #3713 produced a - // button that ran one script on the server and a different one in the browser. - const result = FieldSchema.parse({ + it('requiredWhen has no alias left to coexist with (#3855)', () => { + // The original assertion pinned the two keys as coexisting in the output; + // #3754 inverted it to "canonical wins, alias dropped"; #3855 removes the + // alias outright, so the pair is now unrepresentable at the input too — the + // strongest form of the same guarantee. + expect(() => FieldSchema.parse({ type: 'text', requiredWhen: "record.status == 'sent'", conditionalRequired: "record.status == 'closed_won'", - }); + })).toThrow(/`conditionalRequired` was removed/); + + const result = FieldSchema.parse({ type: 'text', requiredWhen: "record.status == 'sent'" }); expect(result.requiredWhen).toEqual({ dialect: 'cel', source: "record.status == 'sent'" }); expect('conditionalRequired' in result).toBe(false); }); diff --git a/packages/spec/src/data/field.zod.ts b/packages/spec/src/data/field.zod.ts index d6ecad75de..d2562b3695 100644 --- a/packages/spec/src/data/field.zod.ts +++ b/packages/spec/src/data/field.zod.ts @@ -1,6 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { z } from 'zod'; +import { retiredKey } from '../shared/retired-key'; import { SystemIdentifierSchema } from '../shared/identifiers.zod'; import { ExpressionInputSchema } from '../shared/expression.zod'; import { FilterConditionSchema } from './filter.zod'; @@ -524,14 +525,20 @@ export const FieldSchema = lazySchema(() => z.object({ */ visibleWhen: ExpressionInputSchema.optional().describe("Predicate (CEL) — field is shown only when TRUE (else hidden). e.g. P`record.type == 'invoice'`"), readonlyWhen: ExpressionInputSchema.optional().describe("Predicate (CEL) — field is read-only when TRUE. e.g. P`record.status == 'paid'`"), - requiredWhen: ExpressionInputSchema.optional().describe("Predicate (CEL) — field is required when TRUE. Canonical slot; the deprecated `conditionalRequired` alias is lowered into this one at parse time."), + requiredWhen: ExpressionInputSchema.optional().describe("Predicate (CEL) — field is required when TRUE. The only slot; the `conditionalRequired` alias was removed in protocol 17 (#3855)."), - /** Conditional Requirements - * @deprecated Alias of `requiredWhen`. Accepted on input, lowered into - * `requiredWhen` at parse time, then **removed from the parsed output** - * (#3754) — so every consumer reads one canonical slot. When both are set, - * `requiredWhen` wins and this value is discarded. */ - conditionalRequired: ExpressionInputSchema.optional().describe('@deprecated — Use requiredWhen instead. Lowered into requiredWhen during parsing and dropped from the parsed output; when both are set, requiredWhen wins.'), + /** + * [REMOVED in protocol 17 — #3855] The deprecated alias of `requiredWhen`. + * Tombstoned rather than deleted: `FieldSchema` is deliberately not + * `.strict()`, so a plain deletion would silently strip the key and the field + * would never be required — the ADR-0104 / #3733 failure class this object + * already carries a comment about. + */ + conditionalRequired: retiredKey( + '`conditionalRequired` was removed in @objectstack/spec 17 (#3855) — use `requiredWhen`. ' + + 'Rename the key; the value (a CEL predicate) is unchanged. ' + + 'Run `os migrate meta --from 16` to rewrite it automatically.', + ), /** * Form widget override. Names a registered field/UI component to render this @@ -603,35 +610,15 @@ export const FieldSchema = lazySchema(() => z.object({ // driver builds indexes from the object's `indexes[]` array; a field-level // `index: true` created no index. Declare the index in object `indexes[]`. externalId: z.boolean().default(false).describe('Is external ID for upsert operations'), -}).transform((data) => { - // #3754: lower the deprecated `conditionalRequired` alias into the canonical - // `requiredWhen` and DROP it from the output — the shape #3713/#3742 settled on - // for `action.execute` → `target`, and #1891 for `agent.knowledge.topics` → - // `sources`. Canonical wins when both are set. - // - // Why this is worth doing even though today's readers all agree: leaving both - // keys live in the parsed output means every consumer has to re-implement the - // precedence, and that is exactly the condition that produced #3713 — there the - // server kept `target` while objectui's ActionRunner preferred the alias, so one - // button ran two different scripts. The server side here happens to get it right - // (`rule-validator.ts` reads `requiredWhen ?? conditionalRequired`), but nothing - // in the contract *made* it right. Folding once, here, removes the chance. - // - // Authors may still write `conditionalRequired` — it stays on the parse-input - // type (`FieldParseInput`) and in the reference docs; only the output is canonical. - const { conditionalRequired, ...rest } = data; - if (conditionalRequired && !rest.requiredWhen) { - return { ...rest, requiredWhen: conditionalRequired }; - } - return rest; })); export type Field = z.infer; /** - * Author-facing parse INPUT for a field. Unlike {@link Field} (the parsed output) - * this still carries the deprecated `conditionalRequired` alias, which the schema - * lowers into `requiredWhen` at parse time (#3754). Distinct from the - * `FieldInput` factory-helper type further down, which is `Partial`. + * Author-facing parse INPUT for a field. Since protocol 17 (#3855) it no longer + * carries the removed `conditionalRequired` alias — the key is tombstoned, so + * writing it is a `tsc` error at the authoring site as well as a parse error. + * Distinct from the `FieldInput` factory-helper type further down, which is + * `Partial`. */ export type FieldParseInput = z.input; export type SelectOption = z.infer; diff --git a/packages/spec/src/index.ts b/packages/spec/src/index.ts index b42014a1c1..326be1389e 100644 --- a/packages/spec/src/index.ts +++ b/packages/spec/src/index.ts @@ -107,18 +107,6 @@ export { suggestFieldType, findClosestMatches, formatSuggestion } from './shared export { normalizeMetadataCollection, normalizeStackInput, normalizePluginMetadata, MAP_SUPPORTED_FIELDS, METADATA_ALIASES } from './shared/metadata-collection.zod'; export type { MetadataCollectionInput, MapSupportedField, NormalizeStackInputOptions } from './shared/metadata-collection.zod'; -// Pre-parse authoring lint (#3743) — the one window where a deprecated alias is -// still visible, since the parse itself resolves and drops it. `defineStack` -// warns from here; the CLI runs the same rules over stacks that skip it. -export { - lintDeprecatedAliases, - formatDeprecatedAliasFinding, - ACTION_TARGET_EXECUTE_CONFLICT, - FIELD_REQUIREDWHEN_CONDITIONALREQUIRED_CONFLICT, - AGENT_KNOWLEDGE_SOURCES_TOPICS_CONFLICT, -} from './shared/deprecated-aliases'; -export type { DeprecatedAliasFinding } from './shared/deprecated-aliases'; - // Metadata conversion layer (ADR-0087 D2) — old-shape → canonical-shape transforms applied at load. export * from './conversions/index.js'; diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index 7beb2f525d..5799453ac5 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -322,6 +322,40 @@ const step16: MigrationStep = { ], }; +/** + * Protocol 17 step. + * + * Mechanical, and mechanical only: the three deprecated aliases that a schema + * transform used to fold into a canonical key and drop from the parsed output + * (`action.execute`, `field.conditionalRequired`, `agent.knowledge.topics`) are + * removed from the spec. Each is a pure key rename with an unchanged value, so + * the whole break replays losslessly — there is no semantic residue and the + * `semantic` list is deliberately empty. + * + * The three conversions are `retiredFromLoadPath` from the day they land: 17 + * gives the aliases no acceptance window at all, and each schema tombstones its + * key with a fix-it error instead. This step is what makes that affordable — + * `os migrate meta --from ` rewrites the consumer's source rather than + * leaving them to hand-edit against a changelog. + */ +const step17: MigrationStep = { + toMajor: 17, + rationale: + 'Protocol 17 removes the last three deprecated authorable aliases: action ' + + '`execute` (use `target`), field `conditionalRequired` (use `requiredWhen`), and ' + + 'agent `knowledge.topics` (use `knowledge.sources`). Each was already lowered into ' + + 'its canonical key at parse time and dropped from the parsed output, so no runtime ' + + 'behaviour changes — only the authorable surface shrinks to one spelling per slot. ' + + 'All three are pure key renames with unchanged values and replay losslessly; the ' + + 'schemas reject the removed spellings with a fix-it error naming the replacement.', + conversionIds: [ + 'action-execute-to-target', + 'field-conditionalRequired-to-requiredWhen', + 'agent-knowledge-topics-to-sources', + ], + semantic: [], +}; + /** All migration steps, keyed by the major they migrate into. */ export const MIGRATIONS_BY_MAJOR: Readonly> = { 11: step11, @@ -330,6 +364,7 @@ export const MIGRATIONS_BY_MAJOR: Readonly> = { 14: step14, 15: step15, 16: step16, + 17: step17, }; /** The majors that have a step, ascending. */ diff --git a/packages/spec/src/shared/deprecated-aliases.test.ts b/packages/spec/src/shared/deprecated-aliases.test.ts deleted file mode 100644 index 2174cd411c..0000000000 --- a/packages/spec/src/shared/deprecated-aliases.test.ts +++ /dev/null @@ -1,316 +0,0 @@ -// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. - -import { describe, it, expect } from 'vitest'; -import { - lintDeprecatedAliases, - ACTION_TARGET_EXECUTE_CONFLICT, - FIELD_REQUIREDWHEN_CONDITIONALREQUIRED_CONFLICT, - AGENT_KNOWLEDGE_SOURCES_TOPICS_CONFLICT, -} from './deprecated-aliases'; - -// ── #3743: the discarded `execute` alias now has an author-facing warning ── -// -// #3742 made `target` win everywhere and had the `ActionSchema` transform DROP -// `execute` from its output, so "two different scripts for one button" became -// unrepresentable. What it left: when an author declares both slots with -// different values, the losing one is thrown away **silently**. Prime Directive -// #12 wants that surfaced at authoring time, which is why this lint exists — and -// why it runs PRE-parse: the transform it reports on has already erased the -// evidence by the time anything downstream gets to look. - -const action = (extra: Record) => ({ - name: 'convert', - label: 'Convert', - type: 'script', - ...extra, -}); - -describe('lintDeprecatedAliases — quiet paths', () => { - it('says nothing when only the canonical `target` is declared', () => { - expect(lintDeprecatedAliases({ actions: [action({ target: 'preferredHandler' })] })).toEqual([]); - }); - - it('says nothing when only the deprecated `execute` is declared', () => { - // The alias alone is lowered into `target` and works. Deprecation nagging is - // a separate decision from reporting a DISCARDED handler; this rule reports - // the discard only. - expect(lintDeprecatedAliases({ actions: [action({ execute: 'legacyHandler' })] })).toEqual([]); - }); - - it('says nothing when both slots carry the SAME value', () => { - // Redundant, not contradictory — nothing the author wrote is lost. - const stack = { actions: [action({ target: 'sameHandler', execute: 'sameHandler' })] }; - expect(lintDeprecatedAliases(stack)).toEqual([]); - }); - - it('says nothing when both slots hold the very same function reference', () => { - const handler = function sharedHandler() { return 'shared'; }; - const stack = { actions: [action({ target: handler, execute: handler })] }; - expect(lintDeprecatedAliases(stack)).toEqual([]); - }); - - it('treats an empty-string slot as undeclared', () => { - const stack = { actions: [action({ target: '', execute: 'legacyHandler' })] }; - expect(lintDeprecatedAliases(stack)).toEqual([]); - }); - - it('tolerates a stack with no actions at all', () => { - expect(lintDeprecatedAliases({})).toEqual([]); - expect(lintDeprecatedAliases({ objects: [{ name: 'crm_deal' }] })).toEqual([]); - }); -}); - -describe('lintDeprecatedAliases — action-target-execute-conflict', () => { - it('flags the string/string pair from #3713 and names both handlers', () => { - const stack = { actions: [action({ target: 'preferredHandler', execute: 'legacyHandler' })] }; - const findings = lintDeprecatedAliases(stack); - - expect(findings).toHaveLength(1); - const [f] = findings; - expect(f.rule).toBe(ACTION_TARGET_EXECUTE_CONFLICT); - // Advisory: the stack is well-defined and shippable — the cost is a handler - // that never runs, not a broken build. - expect(f.severity).toBe('warning'); - expect(f.where).toBe(`action 'convert'`); - // Both values must appear, or the author cannot tell WHICH handler they lost. - expect(f.message).toContain(`'preferredHandler'`); - expect(f.message).toContain(`'legacyHandler'`); - // …and the precedence has to be stated, not implied. - expect(f.message).toContain(`'target' wins`); - // The one-line fix (#3743 suggested shape). - expect(f.hint).toContain(`Delete 'execute'`); - }); - - it('carries object context for an action nested under its object', () => { - const stack = { - objects: [{ - name: 'crm_deal', - actions: [action({ target: 'preferredHandler', execute: 'legacyHandler' })], - }], - }; - const [f] = lintDeprecatedAliases(stack); - expect(f.where).toBe(`action 'convert' on object 'crm_deal'`); - }); - - it('reports ONE finding for an action that appears both top-level and nested', () => { - // The loader auto-populates `objects[*].actions` from `actions[*].objectName`, - // so the same authored action routinely shows up twice. One mistake, one line. - const both = action({ target: 'preferredHandler', execute: 'legacyHandler' }); - const stack = { - objects: [{ name: 'crm_deal', actions: [both] }], - actions: [both], - }; - const findings = lintDeprecatedAliases(stack); - expect(findings).toHaveLength(1); - // Object-nested is walked first, so the surviving finding keeps the context. - expect(findings[0].where).toBe(`action 'convert' on object 'crm_deal'`); - }); - - it('flags inline functions too — the silent discard is the same', () => { - const stack = { - actions: [action({ - target: function preferredHandler() { return 'preferred'; }, - execute: function legacyHandler() { return 'legacy'; }, - })], - }; - const [f] = lintDeprecatedAliases(stack); - expect(f.rule).toBe(ACTION_TARGET_EXECUTE_CONFLICT); - expect(f.message).toContain('an inline function'); - }); - - it('flags the mixed string-target / function-execute pair', () => { - // The combination #3742 left resolving the alias's way in `lowerCallables`. - // Both halves of #3743 meet here: the precedence fix makes `target` win, and - // this warning is what tells the author the inline function is gone. - const stack = { - actions: [action({ - target: 'preferredHandler', - execute: function legacyHandler() { return 'legacy'; }, - })], - }; - const [f] = lintDeprecatedAliases(stack); - expect(f.message).toContain(`'preferredHandler'`); - expect(f.message).toContain('an inline function'); - }); - - it('reads map-shaped action slots, injecting the map key as the name', () => { - const stack = { - objects: { - crm_deal: { - actions: { - convert: { label: 'Convert', type: 'script', target: 'preferredHandler', execute: 'legacyHandler' }, - }, - }, - }, - }; - const [f] = lintDeprecatedAliases(stack); - expect(f.where).toBe(`action 'convert' on object 'crm_deal'`); - }); - - it('reports every distinct offending action', () => { - const stack = { - actions: [ - action({ name: 'convert', target: 'a', execute: 'b' }), - action({ name: 'archive', target: 'c', execute: 'd' }), - action({ name: 'clean', target: 'e' }), - ], - }; - expect(lintDeprecatedAliases(stack).map((f) => f.where)).toEqual([ - `action 'convert'`, - `action 'archive'`, - ]); - }); -}); - -// ── The two sibling fold-and-drop aliases (#3743 follow-up) ────────────────── -// -// `execute` was never special: `FieldSchema` folds `conditionalRequired` into -// `requiredWhen` and `AIKnowledgeSchema` folds `topics` into `sources`, both -// dropping the alias from the parsed output exactly the same way. That makes -// all three invisible to every post-parse check, so all three belong to this -// pre-parse pass — which is the reason the pass exists as a rule SET rather -// than one hard-coded rule. - -describe('lintDeprecatedAliases — field-requiredwhen-conditionalrequired-conflict', () => { - const objectWith = (field: Record) => ({ - objects: [{ - name: 'crm_task', - fields: { due_date: { type: 'date', label: 'Due', ...field } }, - }], - }); - - it('flags a field declaring both predicates with different sources', () => { - const findings = lintDeprecatedAliases(objectWith({ - requiredWhen: 'record.stage == "closed"', - conditionalRequired: 'record.amount > 0', - })); - - expect(findings).toHaveLength(1); - const [f] = findings; - expect(f.rule).toBe(FIELD_REQUIREDWHEN_CONDITIONALREQUIRED_CONFLICT); - expect(f.severity).toBe('warning'); - expect(f.where).toBe(`field 'due_date' on object 'crm_task'`); - expect(f.message).toContain('record.stage == "closed"'); - expect(f.message).toContain('record.amount > 0'); - expect(f.message).toContain(`'requiredWhen' wins`); - expect(f.message).toContain('never gates the field'); - expect(f.hint).toContain(`Delete 'conditionalRequired'`); - }); - - it('stays quiet when only one predicate slot is declared', () => { - expect(lintDeprecatedAliases(objectWith({ requiredWhen: 'record.paid' }))).toEqual([]); - expect(lintDeprecatedAliases(objectWith({ conditionalRequired: 'record.paid' }))).toEqual([]); - }); - - it('compares the predicate TEXT, not the wrapper', () => { - // A bare string and the `{ dialect, source }` envelope it lowers into are - // the same predicate written two ways — nothing is lost either way. - expect(lintDeprecatedAliases(objectWith({ - requiredWhen: 'record.paid', - conditionalRequired: { dialect: 'cel', source: 'record.paid' }, - }))).toEqual([]); - }); - - it('reads the envelope form on both slots', () => { - const [f] = lintDeprecatedAliases(objectWith({ - requiredWhen: { dialect: 'cel', source: 'record.a' }, - conditionalRequired: { dialect: 'cel', source: 'record.b' }, - })); - expect(f.rule).toBe(FIELD_REQUIREDWHEN_CONDITIONALREQUIRED_CONFLICT); - expect(f.message).toContain('record.a'); - expect(f.message).toContain('record.b'); - }); - - it('covers fields added by an object extension', () => { - // An extension adds/overrides fields on another package's object through - // the same `FieldSchema`, so it carries the same silent discard. - const [f] = lintDeprecatedAliases({ - objectExtensions: [{ - objectName: 'sys_user', - fields: { nickname: { type: 'text', requiredWhen: 'record.a', conditionalRequired: 'record.b' } }, - }], - }); - expect(f.where).toBe(`field 'nickname' on object extension 'sys_user'`); - }); - - it('tolerates an object with no fields', () => { - expect(lintDeprecatedAliases({ objects: [{ name: 'crm_task' }] })).toEqual([]); - }); -}); - -describe('lintDeprecatedAliases — agent-knowledge-sources-topics-conflict', () => { - const agentWith = (knowledge: Record) => ({ - agents: [{ name: 'support_bot', label: 'Support', knowledge }], - }); - - it('flags an agent declaring both source lists with different members', () => { - const findings = lintDeprecatedAliases(agentWith({ - sources: ['faq', 'policies'], - topics: ['legacy_kb'], - })); - - expect(findings).toHaveLength(1); - const [f] = findings; - expect(f.rule).toBe(AGENT_KNOWLEDGE_SOURCES_TOPICS_CONFLICT); - expect(f.severity).toBe('warning'); - expect(f.where).toBe(`agent 'support_bot' knowledge`); - expect(f.message).toContain(`'faq', 'policies'`); - expect(f.message).toContain(`'legacy_kb'`); - expect(f.message).toContain(`'sources' wins`); - expect(f.message).toContain('never recruited into RAG context'); - expect(f.hint).toContain(`Delete 'topics'`); - }); - - it('stays quiet when only one list is declared', () => { - expect(lintDeprecatedAliases(agentWith({ sources: ['faq'] }))).toEqual([]); - expect(lintDeprecatedAliases(agentWith({ topics: ['faq'] }))).toEqual([]); - }); - - it('stays quiet when the two lists have the same members in any order', () => { - // Order and repetition are not meaningful for either slot, so the same set - // written twice loses nothing. - expect(lintDeprecatedAliases(agentWith({ - sources: ['faq', 'policies'], - topics: ['policies', 'faq', 'faq'], - }))).toEqual([]); - }); - - it('treats an empty list as undeclared', () => { - expect(lintDeprecatedAliases(agentWith({ sources: [], topics: ['faq'] }))).toEqual([]); - }); - - it('tolerates an agent with no knowledge block', () => { - expect(lintDeprecatedAliases({ agents: [{ name: 'support_bot' }] })).toEqual([]); - }); -}); - -describe('lintDeprecatedAliases — all three rules together', () => { - it('reports each rule independently in one pass', () => { - const stack = { - objects: [{ - name: 'crm_task', - fields: { due_date: { type: 'date', requiredWhen: 'record.a', conditionalRequired: 'record.b' } }, - actions: [{ name: 'convert', type: 'script', target: 'preferred', execute: 'legacy' }], - }], - agents: [{ name: 'support_bot', knowledge: { sources: ['faq'], topics: ['legacy_kb'] } }], - }; - - expect(lintDeprecatedAliases(stack).map((f) => f.rule).sort()).toEqual([ - ACTION_TARGET_EXECUTE_CONFLICT, - AGENT_KNOWLEDGE_SOURCES_TOPICS_CONFLICT, - FIELD_REQUIREDWHEN_CONDITIONALREQUIRED_CONFLICT, - ].sort()); - }); - - it('returns nothing for a stack that uses only canonical keys', () => { - const stack = { - objects: [{ - name: 'crm_task', - fields: { due_date: { type: 'date', requiredWhen: 'record.a' } }, - actions: [{ name: 'convert', type: 'script', target: 'preferred' }], - }], - agents: [{ name: 'support_bot', knowledge: { sources: ['faq'] } }], - }; - expect(lintDeprecatedAliases(stack)).toEqual([]); - }); -}); diff --git a/packages/spec/src/shared/deprecated-aliases.ts b/packages/spec/src/shared/deprecated-aliases.ts deleted file mode 100644 index b5384dcf61..0000000000 --- a/packages/spec/src/shared/deprecated-aliases.ts +++ /dev/null @@ -1,305 +0,0 @@ -// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. - -/** - * PRE-PARSE authoring lint for DEPRECATED ALIASES (#3743). - * - * A deprecated alias is resolved by the parse itself: `ActionSchema`'s transform - * folds `execute` into `target` and DROPS it from the output (#3713 / #3742). So - * every check that runs on parsed metadata — the CLI's `lintFlowPatterns`, - * `lintViewRefs`, `@objectstack/lint`'s validators, a renderer, the runtime — is - * structurally blind to the one thing worth reporting here: that the author - * declared the alias at all, let alone that they put a *different* value in it. - * - * That leaves exactly one window: **after `normalizeStackInput`, before the - * parse.** This module is the rule set for that window, kept in `spec` rather - * than in a lint package because the two places that own that window both live - * upstream of `@objectstack/lint`: - * - * 1. `defineStack` (`stack.zod.ts`) — the dominant authoring path, and the one - * that consumes the alias EARLIEST. It parses inside the author's own - * config module, so by the time `os build` loads that module the alias is - * already gone. A CLI-only warning would therefore never fire for a - * `defineStack` app — i.e. for almost every app. - * 2. The CLI's `os build` / `os validate` pre-parse pass — which is where a - * stack that did NOT go through strict `defineStack` gets caught: a plain - * object default-export, `defineStack(…, { strict: false })`, or an inline - * function handler (`z.string()` rejects those, so they cannot pass through - * strict `defineStack` at all and are lowered by the CLI instead). - * - * Each layer warns only for the discards IT performs, so an authored conflict - * produces exactly one warning no matter which path the stack takes. - * - * ── Scope: every fold-and-drop alias in the spec ────────────────────────────── - * - * A rule belongs here when the transform makes the alias *unrepresentable* on - * the parsed output — that is what puts it out of reach of every downstream - * check. The spec has exactly three such transforms, and all three share one - * failure mode: declare both slots with different values and the alias is - * discarded without a word. - * - * action-target-execute-conflict — WARNING - * `ActionSchema` (#3713 / #3742). The discarded `execute` names a handler - * that never runs. - * - * field-requiredwhen-conditionalrequired-conflict — WARNING - * `FieldSchema` (#3754 / #3764). The discarded `conditionalRequired` is a - * predicate that never gates the field. - * - * agent-knowledge-sources-topics-conflict — WARNING - * `AIKnowledgeSchema` (#1878 / #1891). The discarded `topics` names RAG - * sources the agent never recruits from. - * - * All three are advisory rather than fatal, because the resulting stack is - * well-defined and shippable; the cost is a value that never takes effect, not - * a broken build. Identical values in both slots are harmless duplication — - * nothing the author wrote is lost — and stay quiet. `severity` is modelled and - * honoured by every call site, so promoting a rule gates both CLI surfaces at - * once (#3782) with no rewiring. - */ - -export interface DeprecatedAliasFinding { - /** Author-facing location, e.g. `action 'convert' on object 'crm_deal'`. */ - where: string; - /** What was discarded and why — states the precedence explicitly. */ - message: string; - /** The one-line fix. */ - hint: string; - /** Stable rule id, e.g. {@link ACTION_TARGET_EXECUTE_CONFLICT}. */ - rule: string; - /** - * `'error'` FAILS the build; `'warning'` (the default when absent) prints and - * continues. `os build` and `os validate` both filter on this field, so a rule - * promoted here gates both surfaces at once (#3782). - */ - severity?: 'error' | 'warning'; -} - -type AnyRec = Record; - -export const ACTION_TARGET_EXECUTE_CONFLICT = 'action-target-execute-conflict'; -export const FIELD_REQUIREDWHEN_CONDITIONALREQUIRED_CONFLICT = 'field-requiredwhen-conditionalrequired-conflict'; -export const AGENT_KNOWLEDGE_SOURCES_TOPICS_CONFLICT = 'agent-knowledge-sources-topics-conflict'; - -/** Normalise a record-or-map metadata slot into an array, injecting `name` from - * the map key (mirrors the helper in the CLI's sibling authoring lints). */ -function asArray(v: unknown): AnyRec[] { - if (Array.isArray(v)) return v as AnyRec[]; - if (v && typeof v === 'object') return Object.entries(v as AnyRec).map(([name, def]) => ({ name, ...(def as AnyRec) })); - return []; -} - -/** A handler slot counts as declared when it holds a non-empty string ref or an - * inline callable. Both forms are authorable and both resolve by the same - * `target`-wins precedence. */ -function isDeclaredHandler(v: unknown): boolean { - return typeof v === 'function' || (typeof v === 'string' && v.length > 0); -} - -/** Render a handler slot for an author-facing message. An inline function has no - * useful printable value — naming the shape is what makes the message land. */ -function describeHandler(v: unknown): string { - return typeof v === 'function' ? 'an inline function' : `'${String(v)}'`; -} - -/** Pull the predicate text out of an `ExpressionInput` — a bare string, or the - * `{ dialect, source }` envelope it is lowered into. Anything else is not an - * authored predicate. */ -function expressionSource(v: unknown): string | undefined { - if (typeof v === 'string') return v; - if (v && typeof v === 'object' && typeof (v as AnyRec).source === 'string') return (v as AnyRec).source as string; - return undefined; -} - -function isDeclaredExpression(v: unknown): boolean { - const src = expressionSource(v); - return typeof src === 'string' && src.length > 0; -} - -function describeExpression(v: unknown): string { - return `\`${expressionSource(v)}\``; -} - -/** A knowledge slot counts as declared when it holds a non-empty string array. */ -function isDeclaredList(v: unknown): boolean { - return Array.isArray(v) && v.length > 0; -} - -function describeList(v: unknown): string { - return `[${(v as unknown[]).map((s) => `'${String(s)}'`).join(', ')}]`; -} - -/** Set-equality: `sources: ['a','b']` beside `topics: ['b','a']` recruits the - * same context, so nothing the author wrote is lost and the rule stays quiet. - * Order and repetition are not meaningful for either slot. */ -function sameMembers(a: unknown, b: unknown): boolean { - const norm = (v: unknown) => [...new Set((v as unknown[]).map((x) => String(x)))].sort().join('\u0000'); - return norm(a) === norm(b); -} - -/** The one shape every rule here reports. Sharing the skeleton keeps the three - * messages phrased identically where they say the same thing (which slot wins, - * that the alias is dropped, delete it) while each still names what its own - * discarded value would have done. */ -interface AliasConflict { - /** Sentence subject, e.g. `Action`, `Field`, `Agent knowledge`. */ - subject: string; - where: string; - canonicalKey: string; - aliasKey: string; - /** Rendered values, already quoted/bracketed for display. */ - canonical: string; - alias: string; - /** What the discarded value would have done, e.g. "never runs". */ - lost: string; - /** Rule-specific half of the hint. */ - fix: string; - rule: string; -} - -function aliasFinding(c: AliasConflict): DeprecatedAliasFinding { - return { - where: c.where, - message: - `${c.subject} declares both '${c.canonicalKey}' (${c.canonical}) and the deprecated alias ` + - `'${c.aliasKey}' (${c.alias}). '${c.canonicalKey}' wins: '${c.aliasKey}' is dropped while the stack ` + - `is compiled and never reaches the runtime or a renderer, so ${c.alias} ${c.lost}.`, - hint: - `Delete '${c.aliasKey}' — it is a deprecated alias of '${c.canonicalKey}', not a second value. ${c.fix}`, - rule: c.rule, - severity: 'warning', - }; -} - -/** - * Collect deprecated-alias findings from a NORMALIZED (not yet parsed) stack. - * - * Pure and side-effect free: callers decide how to surface the findings — - * `defineStack` warns on the console, the CLI folds them into its authoring-lint - * block. Safe to call on a stack that still holds inline functions. - */ -export function lintDeprecatedAliases(stack: AnyRec): DeprecatedAliasFinding[] { - const findings: DeprecatedAliasFinding[] = []; - - // An action commonly appears BOTH top-level and nested under its object (the - // loader auto-populates `objects[*].actions` from `actions[*].objectName`), - // and a field can be declared on an object and again on an extension of it. - // Dedupe by rule + identity + both slot values: one authored mistake, one - // finding. `\u0000` is written as an escape, never as a raw byte — a literal - // NUL makes ripgrep treat the whole file as binary and silently drop it from - // every grep-based lint (`scripts/check-nul-bytes.mjs` enforces this). - const seen = new Set(); - const firstTime = (rule: string, ...parts: string[]) => { - const key = [rule, ...parts].join('\u0000'); - if (seen.has(key)) return false; - seen.add(key); - return true; - }; - - // ── action.target vs action.execute (#3713 / #3742) ──────────────────────── - const checkAction = (action: AnyRec, ownerObject?: string) => { - if (!action) return; - const { target, execute } = action; - if (!isDeclaredHandler(target) || !isDeclaredHandler(execute)) return; - // Same value in both slots (equal strings, or the very same function): the - // alias is redundant, not contradictory, and nothing the author wrote is - // lost. Staying quiet keeps the rule's signal-to-noise at 1. - if (target === execute) return; - - const name = typeof action.name === 'string' && action.name ? action.name : '(unnamed)'; - const canonical = describeHandler(target); - const alias = describeHandler(execute); - if (!firstTime(ACTION_TARGET_EXECUTE_CONFLICT, name, canonical, alias)) return; - - findings.push(aliasFinding({ - subject: 'Action', - where: ownerObject ? `action '${name}' on object '${ownerObject}'` : `action '${name}'`, - canonicalKey: 'target', - aliasKey: 'execute', - canonical, - alias, - lost: 'never runs', - fix: `If ${alias} is the handler you meant to bind, put it in 'target' instead.`, - rule: ACTION_TARGET_EXECUTE_CONFLICT, - })); - }; - - // ── field.requiredWhen vs field.conditionalRequired (#3754 / #3764) ──────── - const checkField = (field: AnyRec, ownerLabel: string) => { - if (!field) return; - const { requiredWhen, conditionalRequired } = field; - if (!isDeclaredExpression(requiredWhen) || !isDeclaredExpression(conditionalRequired)) return; - // Compare the predicate TEXT, not the wrapper: `'record.paid'` and - // `{ dialect: 'cel', source: 'record.paid' }` are the same predicate written - // two ways, and neither loses anything. - if (expressionSource(requiredWhen) === expressionSource(conditionalRequired)) return; - - const name = typeof field.name === 'string' && field.name ? field.name : '(unnamed)'; - const canonical = describeExpression(requiredWhen); - const alias = describeExpression(conditionalRequired); - if (!firstTime(FIELD_REQUIREDWHEN_CONDITIONALREQUIRED_CONFLICT, name, canonical, alias)) return; - - findings.push(aliasFinding({ - subject: 'Field', - where: `field '${name}' on ${ownerLabel}`, - canonicalKey: 'requiredWhen', - aliasKey: 'conditionalRequired', - canonical, - alias, - lost: 'never gates the field', - fix: `If ${alias} is the predicate you meant, put it in 'requiredWhen' instead.`, - rule: FIELD_REQUIREDWHEN_CONDITIONALREQUIRED_CONFLICT, - })); - }; - - // ── agent.knowledge.sources vs .topics (#1878 / #1891) ───────────────────── - const checkAgent = (agent: AnyRec) => { - const knowledge = agent?.knowledge; - if (!knowledge || typeof knowledge !== 'object') return; - const { sources, topics } = knowledge as AnyRec; - if (!isDeclaredList(sources) || !isDeclaredList(topics)) return; - if (sameMembers(sources, topics)) return; - - const name = typeof agent.name === 'string' && agent.name ? agent.name : '(unnamed)'; - const canonical = describeList(sources); - const alias = describeList(topics); - if (!firstTime(AGENT_KNOWLEDGE_SOURCES_TOPICS_CONFLICT, name, canonical, alias)) return; - - findings.push(aliasFinding({ - subject: 'Agent knowledge', - where: `agent '${name}' knowledge`, - canonicalKey: 'sources', - aliasKey: 'topics', - canonical, - alias, - lost: 'is never recruited into RAG context', - fix: `If ${alias} names the sources you meant, put them in 'sources' instead.`, - rule: AGENT_KNOWLEDGE_SOURCES_TOPICS_CONFLICT, - })); - }; - - // Object-nested actions first so the retained (deduped) finding keeps object - // context; the same object walk covers its fields. - for (const obj of asArray(stack.objects)) { - const object = typeof obj.name === 'string' ? obj.name : undefined; - for (const action of asArray(obj.actions)) checkAction(action, object); - for (const field of asArray(obj.fields)) checkField(field, `object '${object ?? '(unnamed)'}'`); - } - for (const action of asArray(stack.actions)) checkAction(action); - // An extension adds/overrides fields on an object owned by another package — - // same `FieldSchema`, same transform, same silent discard. - for (const ext of asArray(stack.objectExtensions)) { - const target = typeof ext.objectName === 'string' ? ext.objectName - : typeof ext.name === 'string' ? ext.name - : '(unnamed)'; - for (const field of asArray(ext.fields)) checkField(field, `object extension '${target}'`); - } - for (const agent of asArray(stack.agents)) checkAgent(agent); - - return findings; -} - -/** One-line console rendering, shared so `defineStack` and any other - * console-bound caller phrase the warning identically. */ -export function formatDeprecatedAliasFinding(f: DeprecatedAliasFinding): string { - return `${f.where}: ${f.message}\n ${f.hint}\n rule: ${f.rule}`; -} diff --git a/packages/spec/src/shared/retired-key.ts b/packages/spec/src/shared/retired-key.ts new file mode 100644 index 0000000000..07d16ff35c --- /dev/null +++ b/packages/spec/src/shared/retired-key.ts @@ -0,0 +1,62 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Tombstones for RETIRED authorable keys (#3855). + * + * Removing a key an author can write has one hard requirement: the removal must + * be **audible**. None of the schemas that carried a deprecated alias is + * `.strict()` — `FieldSchema` says so in a comment and records that the trap + * already bit once (`dataQuality` / `cached` outlived their keys by a release + * and were silently stripped, #3726 / #3733, the ADR-0104 class). So simply + * deleting the key from the Zod object does not produce an error; it produces a + * **silent strip**, which is the exact failure mode #3713 → #3743 → #3838 → + * #3854 spent four PRs eliminating, reintroduced one layer down. + * + * A tombstone keeps the key declared but makes it unwritable, so the removal + * lands in the two channels an upgrading author — very often an AI (ADR-0033) — + * actually reads: + * + * 1. **`tsc`.** The input type becomes `never`, so assigning anything to the + * key fails to compile at the authoring site, before anything runs. + * 2. **The parse.** A value reaching the runtime raises the prescription + * itself — not a generic "unrecognized key". This matters because upgrades + * do not happen in the order we imagine: someone jumping several majors at + * once gets the same message as someone stepping one, whereas a load-time + * conversion (ADR-0087 D2) only covers N−1 and would have already retired. + * + * The prescription — not a "deprecated" label — is the payload. AGENTS.md + * ("Removing an authorable spec key also requires a tombstone entry … so the + * rejection itself carries the prescription") names the compile/validation + * error as the one upgrade channel every consumer is guaranteed to hit: an + * agent bumping `@objectstack/spec` sees THIS string, not our docs site. Write + * it as an instruction, with the FROM → TO mapping and the one-line fix. + * + * Tombstones age out, exactly like the `UNKNOWN_KEY_GUIDANCE` entries in + * `data/object.zod.ts`: drop one ~two majors after the removal, by which point + * it is archaeology rather than an upgrade (the history lives in CHANGELOG.md). + */ + +import { z } from 'zod'; + +/** + * Declare a key that has been REMOVED from the spec. + * + * Accepts only `undefined` — i.e. absence. Any authored value is rejected with + * `guidance`, and `z.input` types the key as `never` so the same mistake fails + * `tsc` first. + * + * @param guidance - The upgrade prescription. State what replaced the key, the + * version that removed it, and the one-line fix — this string IS the migration + * doc for anyone who hits it. + * + * @example + * ```ts + * conditionalRequired: retiredKey( + * '`conditionalRequired` was removed in @objectstack/spec 17.0.0 (#3855). ' + + * 'Rename the key to `requiredWhen` — the value (a CEL predicate) is unchanged.', + * ), + * ``` + */ +export function retiredKey(guidance: string) { + return z.never({ error: () => guidance }).optional().describe(`[REMOVED] ${guidance}`); +} diff --git a/packages/spec/src/stack.test.ts b/packages/spec/src/stack.test.ts index 504b8937a7..d46e2bdc19 100644 --- a/packages/spec/src/stack.test.ts +++ b/packages/spec/src/stack.test.ts @@ -1355,120 +1355,6 @@ describe('defineStack — at most one App per package (ADR-0019 D1/D3)', () => { }); }); -// ── #3743: the alias the parse eats must be reported before it is eaten ── -// -// `defineStack` runs `ObjectStackDefinitionSchema.safeParse` INSIDE the author's -// own config module, and that parse folds `execute` into `target` and drops it -// (#3713/#3742). So this is the last moment the conflict exists at all: a lint -// anywhere downstream — the CLI's compile pipeline included — reads a stack that -// no longer has an `execute` key to complain about. Advisory, never fatal: the -// stack that comes out is well-defined, the author has just lost one of the two -// handlers they wrote. -describe('defineStack — deprecated alias warnings (#3743)', () => { - const manifest = { id: 'p', version: '1.0.0', type: 'app' as const, name: 'P', namespace: 'demo' }; - const obj = { name: 'demo_task', label: 'Task', fields: { title: { type: 'text' as const } } }; - const stackWith = (action: Record) => ({ - manifest, - objects: [obj], - actions: [{ label: 'A', type: 'script' as const, objectName: 'demo_task', ...action }], - }); - - // `warnDeprecatedAliases` nags once per DISTINCT conflict for the life of the - // process, so every case below uses its own action name / handler values. - let warn: ReturnType; - beforeEach(() => { - warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); - }); - afterEach(() => { - warn.mockRestore(); - }); - - it('warns — naming both handlers — when the two slots disagree', () => { - expect(() => - defineStack(stackWith({ name: 'conflicting_a', target: 'preferred_a', execute: 'legacy_a' })), - ).not.toThrow(); - - expect(warn).toHaveBeenCalledTimes(1); - const msg = warn.mock.calls[0][0] as string; - expect(msg).toContain('defineStack:'); - expect(msg).toContain(`'preferred_a'`); - expect(msg).toContain(`'legacy_a'`); - expect(msg).toContain('action-target-execute-conflict'); - }); - - it('still drops the alias and keeps `target` — the warning changes nothing', () => { - const stack = defineStack( - stackWith({ name: 'conflicting_b', target: 'preferred_b', execute: 'legacy_b' }), - ); - const action = (stack.actions as Array>)[0]; - expect(action.target).toBe('preferred_b'); - expect('execute' in action).toBe(false); - }); - - it('stays quiet when only one slot is declared', () => { - defineStack(stackWith({ name: 'canonical_only', target: 'preferred_c' })); - defineStack(stackWith({ name: 'alias_only', execute: 'legacy_c' })); - expect(warn).not.toHaveBeenCalled(); - }); - - it('stays quiet when both slots carry the same handler', () => { - defineStack(stackWith({ name: 'same_both', target: 'same_d', execute: 'same_d' })); - expect(warn).not.toHaveBeenCalled(); - }); - - it('nags once per distinct conflict, however often the stack is defined', () => { - const config = stackWith({ name: 'conflicting_e', target: 'preferred_e', execute: 'legacy_e' }); - defineStack(config); - defineStack(config); - defineStack(config); - expect(warn).toHaveBeenCalledTimes(1); - }); - - it('does not warn in non-strict mode — nothing has been discarded yet', () => { - // With no parse there is no discard: `execute` survives on the returned - // stack, and whichever layer eventually consumes it reports it. Warning here - // too would double-report the same authored mistake. - const stack = defineStack( - stackWith({ name: 'conflicting_f', target: 'preferred_f', execute: 'legacy_f' }), - { strict: false }, - ); - expect(warn).not.toHaveBeenCalled(); - const action = (stack.actions as Array>)[0]; - expect(action.execute).toBe('legacy_f'); - }); - - it('warns for every rule in the pass, not just the action one', () => { - // The wiring is rule-agnostic — it loops over whatever `lintDeprecatedAliases` - // returns — so a field alias must surface through `defineStack` on the same - // terms as an action alias. `FieldSchema` folds `conditionalRequired` into - // `requiredWhen` and drops it, which is invisible one line later. - const stack = defineStack({ - manifest, - objects: [{ - name: 'demo_task', - label: 'Task', - fields: { - title: { type: 'text' as const }, - due_date: { - type: 'date' as const, - requiredWhen: 'record.stage == "closed"', - conditionalRequired: 'record.amount > 0', - }, - }, - }], - }); - - expect(warn).toHaveBeenCalledTimes(1); - const msg = warn.mock.calls[0][0] as string; - expect(msg).toContain('field-requiredwhen-conditionalrequired-conflict'); - expect(msg).toContain(`field 'due_date' on object 'demo_task'`); - - // …and the parse still drops the alias, exactly as before the warning. - const field = (stack.objects as Array<{ fields: Record> }>)[0].fields.due_date; - expect('conditionalRequired' in field).toBe(false); - }); -}); - // ── ADR-0087 D2 conversion notices reach the author ───────────────────────── // // A conversion is deliberately silent about FIXING the shape — zero consumer diff --git a/packages/spec/src/stack.zod.ts b/packages/spec/src/stack.zod.ts index f094d5551b..bb94629ed5 100644 --- a/packages/spec/src/stack.zod.ts +++ b/packages/spec/src/stack.zod.ts @@ -10,7 +10,6 @@ import { TranslationBundleSchema, TranslationConfigSchema } from './system/trans import { hasPlatformObjectPrefix } from './system/constants/platform-object-names'; import { objectStackErrorMap, formatZodError } from './shared/error-map.zod'; import { normalizeStackInput, type MetadataCollectionInput, type MapSupportedField } from './shared/metadata-collection.zod'; -import { lintDeprecatedAliases, formatDeprecatedAliasFinding } from './shared/deprecated-aliases'; import type { ConversionNotice } from './conversions/types.js'; // Data Protocol @@ -1046,29 +1045,6 @@ function validateKnownCapabilities(config: ObjectStackDefinition): string[] { return errors; } -/** - * Findings already reported this process, so a stack that is defined more than - * once (a dev-server reload, a config imported by several test files) nags once - * per distinct conflict. The key carries the action AND both slot values, so a - * genuinely different conflict is never suppressed. Mirrors the warn-once shape - * of `warnGenericPasswordFields` in `object.zod.ts`. - */ -const warnedAliasFindings = new Set(); - -/** - * [#3743] Surface every deprecated-alias conflict the parse is about to resolve - * silently. Advisory: this never throws, because the stack that comes out is - * well-defined — the author just loses one of the two handlers they wrote. - */ -function warnDeprecatedAliases(normalized: Record): void { - for (const finding of lintDeprecatedAliases(normalized)) { - const key = `${finding.rule}\u0000${finding.where}\u0000${finding.message}`; - if (warnedAliasFindings.has(key)) continue; - warnedAliasFindings.add(key); - console.warn(`defineStack: ${formatDeprecatedAliasFinding(finding)}`); - } -} - /** Conversion notices already reported this process — same warn-once reason. */ const warnedConversionNotices = new Set(); @@ -1114,22 +1090,9 @@ export function defineStack( if (!strict) { // Non-strict mode: skip validation (advanced use cases only). - // No alias warning here on purpose: with no parse there is no discard yet — - // the alias survives on the returned stack, and whichever layer eventually - // consumes it (the CLI's pre-parse pass on `os build`/`os validate`) is the - // one that reports it. Each layer warns for its OWN discards, so an authored - // conflict yields exactly one warning however the stack is compiled. return mergeActionsIntoObjects(normalized as ObjectStackDefinition); } - // [#3743] The LAST moment a deprecated alias is still visible. The parse below - // resolves `execute` into `target` and drops it (#3713/#3742), and this parse - // runs inside the author's own config module — so by the time `os build` reads - // the exported stack there is nothing left to lint. Warning here is what makes - // the discard visible to the author who wrote both handlers (Prime Directive - // #12); it is advisory and never blocks, since the resulting stack is - // well-defined — the cost is a handler that never runs. - warnDeprecatedAliases(normalized as Record); // Strict mode (default): parse with custom error map, then cross-reference validate const result = ObjectStackDefinitionSchema.safeParse(normalized, { diff --git a/packages/spec/src/ui/action.test.ts b/packages/spec/src/ui/action.test.ts index 67d3ac320d..733a985cfc 100644 --- a/packages/spec/src/ui/action.test.ts +++ b/packages/spec/src/ui/action.test.ts @@ -1085,59 +1085,67 @@ describe('ActionSchema - order', () => { // Protocol Improvement Tests: execute → target migration & target validation // ============================================================================ -describe('ActionSchema - execute → target migration', () => { - it('should auto-migrate execute to target when target is not set', () => { - const result = ActionSchema.parse({ +describe('ActionSchema — the `execute` alias is REMOVED (#3855)', () => { + // #3713 → #3742 → #3855: `execute` was the deprecated alias of `target`. It + // was lowered into `target` at parse time and dropped from the output; as of + // protocol 17 it is gone from the spec entirely. + // + // It is TOMBSTONED rather than deleted, because `ActionSchema` is not + // `.strict()`: a plain deletion would have silently stripped the key, leaving + // the action with no handler bound and no diagnostic — the #2169 "Mark Done + // does nothing" shape. The rejection is what makes the removal audible, and + // it carries the fix so an upgrading agent does not have to find our docs. + const parseWithExecute = () => + ActionSchema.parse({ name: 'legacy_action', label: 'Legacy', type: 'script', execute: 'legacyHandler', }); - expect(result.target).toBe('legacyHandler'); - // #3713: the alias is consumed, not carried alongside the canonical slot. - expect('execute' in result).toBe(false); + + it('rejects an authored `execute` instead of silently stripping it', () => { + expect(parseWithExecute).toThrow(); }); - it('should preserve target over execute when both are set', () => { - const result = ActionSchema.parse({ - name: 'both_fields', - label: 'Both', - type: 'script', - target: 'preferredHandler', - execute: 'legacyHandler', - }); - expect(result.target).toBe('preferredHandler'); + it('carries the upgrade prescription in the rejection itself', () => { + // This string is the one upgrade channel every consumer is guaranteed to + // hit — it must name the replacement AND the automated fix. + let message = ''; + try { + parseWithExecute(); + } catch (err) { + message = String(err); + } + expect(message).toContain('`execute` was removed'); + expect(message).toContain('use `target`'); + expect(message).toContain('os migrate meta'); }); - it('should DROP execute from the parsed output so no consumer can disagree (#3713)', () => { - // The divergence this pins: the spec kept `target` when both were set, while - // objectui's ActionRunner did `execute || target` — so one button ran - // `preferredHandler` server-side and `legacyHandler` client-side, silently. - // Lowering the alias and removing it makes the conflict *unrepresentable* - // rather than merely agreed-upon — same shape as `agent.knowledge.topics` - // → `sources` (#1891), which asserts `'topics' in parsed === false`. - const both = ActionSchema.parse({ + it('rejects `execute` even when the canonical `target` is also present', () => { + // The old behaviour silently discarded the alias here. Now the author is + // told, because one of the two handlers they wrote was going to be lost. + expect(() => ActionSchema.parse({ name: 'both_fields', label: 'Both', type: 'script', target: 'preferredHandler', execute: 'legacyHandler', - }); - expect('execute' in both).toBe(false); - expect(Object.keys(both)).not.toContain('execute'); - expect(JSON.parse(JSON.stringify(both)).execute).toBeUndefined(); + })).toThrow(/`execute` was removed/); + }); - // Authors may still WRITE `execute` — only the parsed output is canonical. - const aliasOnly = ActionSchema.parse({ - name: 'alias_only', - label: 'Alias', + it('accepts the canonical `target` on its own', () => { + const parsed = ActionSchema.parse({ + name: 'canonical', + label: 'Canonical', type: 'url', - execute: 'https://example.com/report', + target: 'https://example.com/report', }); - expect(aliasOnly.target).toBe('https://example.com/report'); - expect('execute' in aliasOnly).toBe(false); + expect(parsed.target).toBe('https://example.com/report'); + expect('execute' in parsed).toBe(false); }); +}); +describe('ActionSchema - target validation', () => { it('should reject a script with neither target/execute nor body', () => { // #2169: a script action with no handler binding registers nothing. expect(() => ActionSchema.parse({ @@ -1224,14 +1232,24 @@ describe('ActionSchema - target required for non-script types', () => { expect(() => ActionSchema.parse({ name: 'api_ok', label: 'API', type: 'api', target: '/api/endpoint' })).not.toThrow(); }); - it('should accept non-script types when execute is provided (auto-migrated)', () => { - const result = ActionSchema.parse({ + it('rejects a non-script type bound through the removed `execute` alias (#3855)', () => { + // Before protocol 17 this parsed: `execute` was lowered into `target`, so a + // flow action bound through the alias worked. The alias is gone, so the + // action now fails BOTH on the removed key and on its missing `target` — + // and the first message tells the author which key to rename. + expect(() => ActionSchema.parse({ name: 'flow_legacy', label: 'Flow Legacy', type: 'flow', execute: 'my_flow', - }); - expect(result.target).toBe('my_flow'); + })).toThrow(/`execute` was removed/); + + expect(ActionSchema.parse({ + name: 'flow_canonical', + label: 'Flow Canonical', + type: 'flow', + target: 'my_flow', + }).target).toBe('my_flow'); }); }); @@ -1271,7 +1289,7 @@ describe('ACTION_LOCATIONS — canonical source of truth', () => { name: 'with_locations', label: 'With Locations', type: 'script', - execute: 'true', + target: 'true', locations: all, }); expect(action.locations).toEqual(all); @@ -1283,7 +1301,7 @@ describe('ACTION_LOCATIONS — canonical source of truth', () => { name: 'bad_location', label: 'Bad', type: 'script', - execute: 'true', + target: 'true', locations: ['record_section', 'not_a_real_location'], }) ).toThrow(); @@ -1302,7 +1320,7 @@ describe('ACTION_LOCATIONS — canonical source of truth', () => { }); it('[ADR-0066 D4] requiredPermissions is optional (absent ⇒ undefined)', () => { - const action = ActionSchema.parse({ name: 'mark_done', label: 'Mark Done', type: 'script', execute: 'true' }); + const action = ActionSchema.parse({ name: 'mark_done', label: 'Mark Done', type: 'script', target: 'true' }); expect(action.requiredPermissions).toBeUndefined(); }); }); diff --git a/packages/spec/src/ui/action.zod.ts b/packages/spec/src/ui/action.zod.ts index 76f15bc848..f78f538bf4 100644 --- a/packages/spec/src/ui/action.zod.ts +++ b/packages/spec/src/ui/action.zod.ts @@ -1,6 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { z } from 'zod'; +import { retiredKey } from '../shared/retired-key'; import { FieldType } from '../data/field.zod'; import { SnakeCaseIdentifierSchema } from '../shared/identifiers.zod'; import { ExpressionInputSchema } from '../shared/expression.zod'; @@ -249,13 +250,10 @@ const TARGET_REQUIRED_TYPES: ReadonlySet = new Set( * - `type: 'api'` — `target` is **required** (the API endpoint to call). * - `type: 'form'` — `target` is **required** (the FormView name to open, routed to `/console/forms/:name`). * - * The `execute` field is **deprecated** and will be removed in a future version. - * If `execute` is provided without `target`, it is lowered into `target` at parse - * time. Either way `execute` is **dropped from the parsed output** (#3713), so - * every consumer reads one canonical slot; when both are declared, `target` wins. - * Declaring both with *different* values discards the `execute` handler, so it - * raises the advisory `action-target-execute-conflict` warning (#3743) — from - * `defineStack` and from `os build`/`os validate` — rather than passing silently. + * The `execute` alias was **removed in protocol 17** (#3855). `target` is the + * only handler slot, so no consumer has a second slot to disagree about. An + * authored `execute` is rejected with the rename prescription rather than + * silently stripped; `os migrate meta --from 16` rewrites it for you. * * @example Good action names * - 'on_close_deal' @@ -470,12 +468,16 @@ export const ActionSchema = lazySchema(() => z.object({ body: HookBodySchema.optional().describe('Action body — expression (L1) or sandboxed JS (L2). Only used when type is `script`.'), /** - * @deprecated Use `target` instead. Accepted on input, lowered into `target` - * during parsing, then **removed from the parsed output** (#3713) — so a - * renderer only ever sees the canonical `target`. When both are set, `target` - * wins and this value is discarded. + * [REMOVED in protocol 17 — #3855] The deprecated alias of `target`. + * Tombstoned rather than deleted: `ActionSchema` is not `.strict()`, so a + * plain deletion would silently strip the key and the action would bind no + * handler at all — the #2169 "Mark Done does nothing" shape, restored. */ - execute: z.string().optional().describe('@deprecated — Use target instead. Lowered into target during parsing and dropped from the parsed output; when both are set, target wins.'), + execute: retiredKey( + '`execute` was removed in @objectstack/spec 17 (#3855) — use `target`. ' + + 'Rename the key; the value (a handler / flow / URL ref) is unchanged. ' + + 'Run `os migrate meta --from 16` to rewrite it automatically.', + ), /** User Input Requirements */ params: z.array(ActionParamSchema).optional().describe('Input parameters required from user'), @@ -662,29 +664,6 @@ export const ActionSchema = lazySchema(() => z.object({ /** ARIA accessibility attributes */ aria: AriaPropsSchema.optional().describe('ARIA accessibility attributes'), -}).transform((data) => { - // #3713: lower the deprecated `execute` alias into the canonical `target` AND - // drop it from the output — the same "canonical wins, alias disappears" shape - // as `agent.knowledge.topics` → `sources` (#1891). - // - // Keeping both slots on the parsed output made the conflict *representable*, - // and the readers resolved it in OPPOSITE directions: this transform preferred - // `target`, while objectui's ActionRunner did `execute || target`. An action - // declaring both therefore ran `target` server-side and `execute` client-side - // — two different scripts for one button, with no error anywhere. - // - // `target` wins, and the alias no longer reaches a renderer. The server runtime - // never reads `execute` at all (`isHeadlessInvokableAction` gates on - // `target || body`, and the dispatch candidate chain probes `target`/`name` — - // see runtime/src/action-execution.ts), so authoring `execute` works *solely* - // because it is lowered here: dropping it costs the server nothing and takes - // the ambiguity off the wire. Authors may still write `execute` — it stays on - // the input type (`ActionInput`), only the parsed output is canonical. - const { execute, ...rest } = data; - if (execute && !rest.target) { - return { ...rest, target: execute }; - } - return rest; }).refine((data) => { // Require `target` for types that reference an external resource if (TARGET_REQUIRED_TYPES.has(data.type) && !data.target) { From a08645daec0b4b3744c2b2939625a9a148fac34f Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 14:11:55 +0000 Subject: [PATCH 2/3] docs(skills): stop teaching the three removed aliases (#3855) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The docs-drift check surfaced seven places still presenting the aliases as usable — four of them in `skills/`, which is what an AI reads to author metadata AND what ships to third parties via `npx skills add`. Leaving them would publish a corpus that generates metadata protocol 17 rejects: the "declared ≠ enforced" trap, pointed the other way. skills/objectstack-ui — "conditionalRequired is only a back-compat alias" skills/objectstack-formula — "treat conditionalRequired as a read alias", plus a CEL-surface table row for it skills/objectstack-data — "a deprecated compatibility alias" skills/objectstack-ai — "topics is a deprecated alias" data-modeling/validation-rules.mdx, field-types.mdx — table rows describing the alias as lowered at parse time All now say REMOVED / parse error, or are dropped where they were only a row in a table of live keys. Historical release notes are left alone — they correctly describe what those versions did. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01JqC9ckaLbmhmytt65Gxi96 --- content/docs/data-modeling/field-types.mdx | 1 - content/docs/data-modeling/validation-rules.mdx | 1 - skills/objectstack-ai/SKILL.md | 2 +- skills/objectstack-data/SKILL.md | 4 ++-- skills/objectstack-formula/SKILL.md | 5 ++--- skills/objectstack-ui/SKILL.md | 4 ++-- 6 files changed, 7 insertions(+), 10 deletions(-) diff --git a/content/docs/data-modeling/field-types.mdx b/content/docs/data-modeling/field-types.mdx index 9bf4a62dce..1afbbd9c25 100644 --- a/content/docs/data-modeling/field-types.mdx +++ b/content/docs/data-modeling/field-types.mdx @@ -642,7 +642,6 @@ These properties are available on **all** field types: | `visibleWhen` | `Expression` | — | Show field only when predicate is true | | `readonlyWhen` | `Expression` | — | Make field read-only when predicate is true | | `requiredWhen` | `Expression` | — | Require field when predicate is true | -| `conditionalRequired` | `Expression` | — | Deprecated alias of `requiredWhen` — lowered into it at parse time, then dropped from the parsed metadata | --- diff --git a/content/docs/data-modeling/validation-rules.mdx b/content/docs/data-modeling/validation-rules.mdx index 5576569af5..d1ef9e5b69 100644 --- a/content/docs/data-modeling/validation-rules.mdx +++ b/content/docs/data-modeling/validation-rules.mdx @@ -34,7 +34,6 @@ These properties apply to **all** field types and are validated by the base `Fie | `visibleWhen` | `string \| Expression` | — | CEL predicate; field is shown only when `TRUE` | | `readonlyWhen` | `string \| Expression` | — | CEL predicate; field is read-only when `TRUE` | | `requiredWhen` | `string \| Expression` | — | CEL predicate; field is required when `TRUE` | -| `conditionalRequired` | `string \| Expression` | — | Deprecated alias of `requiredWhen` — lowered into it at parse time, then dropped from the parsed metadata | --- diff --git a/skills/objectstack-ai/SKILL.md b/skills/objectstack-ai/SKILL.md index 6416c784a9..8358458154 100644 --- a/skills/objectstack-ai/SKILL.md +++ b/skills/objectstack-ai/SKILL.md @@ -169,7 +169,7 @@ To grant data exploration to your own (platform-internal) agent, add | `tools` | Direct tool references — legacy fallback | | `surface` | `'ask' \| 'build'` — the product surface this agent is (default `'ask'`) | | `model` | LLM model configuration — `provider`, `model`, `temperature`, `maxTokens`, `topP` | -| `knowledge` | RAG access — `sources` (canonical; `topics` is a deprecated alias) + `indexes` | +| `knowledge` | RAG access — `sources` + `indexes`. The `topics` alias was REMOVED in protocol 17; emitting it is a parse error | | `guardrails` | `maxTokensPerInvocation`, `maxExecutionTimeSec`, `blockedTopics` | | `structuredOutput` | Output format (JSON schema, regex, etc.) | | `planning` | Autonomous reasoning — `maxIterations` (default 10) | diff --git a/skills/objectstack-data/SKILL.md b/skills/objectstack-data/SKILL.md index f5b4f8c287..04660ccaca 100644 --- a/skills/objectstack-data/SKILL.md +++ b/skills/objectstack-data/SKILL.md @@ -197,8 +197,8 @@ export const Invoice = ObjectSchema.create({ removing `readonly`. Writing a `readonly` field from a `runAs:'user'` `update_record` node is a build-time **error** (`os validate` / `os build`). - Use `requiredWhen` for conditional requiredness; the ObjectQL validator - enforces it on submit. `conditionalRequired` is a deprecated compatibility - alias, not the preferred authoring field. + enforces it on submit. The `conditionalRequired` alias was REMOVED in + protocol 17 — emitting it is a parse error. - For inline `master_detail` grids, predicates are evaluated row-by-row against the child row's `record`, so line-item rules should live on child fields. - For complex predicates, load **objectstack-formula** and emit CEL via diff --git a/skills/objectstack-formula/SKILL.md b/skills/objectstack-formula/SKILL.md index de2625ab38..d8f505e4a6 100644 --- a/skills/objectstack-formula/SKILL.md +++ b/skills/objectstack-formula/SKILL.md @@ -233,8 +233,8 @@ P`!isBlank(record.po_number)` ``` For field-level conditional rules, emit the canonical field properties: -`visibleWhen`, `readonlyWhen`, and `requiredWhen`. Treat -`conditionalRequired` as a read/compatibility alias only. +`visibleWhen`, `readonlyWhen`, and `requiredWhen`. Never emit +`conditionalRequired` — it was REMOVED in protocol 17 and is a parse error. ❌ Salesforce-flavor — **fails CEL compile**: `objectstack build` errors with a located message, and the flow engine throws if it ever reaches runtime (see the @@ -312,7 +312,6 @@ to the envelope. |:---|:---|:---| | `Field` | `expression` (when `type: 'formula'`) | cel | | `Field` | `visibleWhen` / `readonlyWhen` / `requiredWhen` | cel | -| `Field` | `conditionalRequired` (deprecated alias of `requiredWhen`) | cel | | `View` / `Page` | `visibleWhen` (form section/field, page component) | cel | | `Field` | `defaultValue` (M9.9b) | cel | | `ConditionalValidation` | `when` | cel | diff --git a/skills/objectstack-ui/SKILL.md b/skills/objectstack-ui/SKILL.md index bab2cf6441..28a4319403 100644 --- a/skills/objectstack-ui/SKILL.md +++ b/skills/objectstack-ui/SKILL.md @@ -181,8 +181,8 @@ DATA MODEL field, not in the form view. ObjectUI forms consume: | `requiredWhen` | Mark required when true | ObjectQL validates requiredness on submit | Inline master-detail grids evaluate these rules row-by-row against the child -row. Use `requiredWhen` for new metadata; `conditionalRequired` is only a -back-compat alias. Load **objectstack-formula** when authoring non-trivial CEL. +row. Use `requiredWhen` — the `conditionalRequired` alias was REMOVED in +protocol 17 and is now a parse error. Load **objectstack-formula** when authoring non-trivial CEL. --- From 73ad23fcc7963fe5db782edcb0f9cd0d5190f8c7 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 14:25:07 +0000 Subject: [PATCH 3/3] fix(qa): drop the retired `conditionalRequired` row from the expression ledger (#3855) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dogfood expression-conformance ledger (ADR-0058 D7) still listed `data/field.zod.ts:conditionalRequired` as a CEL-bearing surface. With the key removed the ledger entry went stale and its own ratchet caught it: STALE covers — surface no longer in source: data/field.zod.ts:conditionalRequired Exactly what that ledger is for — the removal has to be acknowledged in every place that claimed the surface exists, not just in the schema. Also clarifies `rule-validator.ts`. Its `requiredWhen ?? conditionalRequired` read is NOT removed and NOT dead: the validator is handed RAW, unparsed field definitions, which includes metadata STORED under protocol <= 16. Dropping the read would silently stop enforcing those rules on existing deployments — a runtime regression this spec change does not otherwise cause, and precisely the kind of silent loss the whole line of work exists to prevent. The comment now says the key is removed from the spec (so nobody reads it as still authorable) and that the fallback retires when stored metadata is migrated, not when the spec key went away. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01JqC9ckaLbmhmytt65Gxi96 --- .../objectql/src/validation/rule-validator.ts | 17 +++++++++++------ .../test/expression-conformance.ledger.ts | 3 +-- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/packages/objectql/src/validation/rule-validator.ts b/packages/objectql/src/validation/rule-validator.ts index 53c64dda1e..ede6bc39bb 100644 --- a/packages/objectql/src/validation/rule-validator.ts +++ b/packages/objectql/src/validation/rule-validator.ts @@ -421,12 +421,17 @@ interface ConditionalFieldOption { interface ConditionalFieldDef { requiredWhen?: string | Expression; - // Back-compat alias of `requiredWhen`. Since #3754 `FieldSchema`'s transform - // lowers this into `requiredWhen` and drops it, so PARSED metadata never carries - // it — but the alias-reading fallback below stays deliberately: this validator is - // also handed RAW, unparsed field definitions (see the `conditionalRequired` - // case in rule-validator.test.ts), and those still need honouring. Canonical - // first, always — never `conditionalRequired ?? requiredWhen`. + // Retired authorable key. #3754 lowered it into `requiredWhen` and dropped it, + // so PARSED metadata never carried it; #3855 removed it from the spec outright, + // so it can no longer be AUTHORED at all — `FieldSchema` rejects it. + // + // The alias-reading fallback below stays deliberately, and its job has changed: + // this validator is also handed RAW, unparsed field definitions, which includes + // metadata STORED under protocol <= 16. Dropping the read here would silently + // stop enforcing those rules on existing deployments — a runtime regression the + // spec change does not otherwise cause. It retires when that stored metadata is + // migrated (`os migrate meta`), not when the spec key went away. + // Canonical first, always — never `conditionalRequired ?? requiredWhen`. conditionalRequired?: string | Expression; readonlyWhen?: string | Expression; /** Static, unconditional read-only flag (`field.readonly`). #2948. */ diff --git a/packages/qa/dogfood/test/expression-conformance.ledger.ts b/packages/qa/dogfood/test/expression-conformance.ledger.ts index f00de776f2..3abf3d9678 100644 --- a/packages/qa/dogfood/test/expression-conformance.ledger.ts +++ b/packages/qa/dogfood/test/expression-conformance.ledger.ts @@ -93,14 +93,13 @@ export const EXPRESSION_SURFACE: ExprSurface[] = [ }, { id: 'cel-field-rule', - summary: 'field UI rules (requiredWhen / readonlyWhen / visibleWhen / conditionalRequired)', + summary: 'field UI rules (requiredWhen / readonlyWhen / visibleWhen)', dialect: 'cel', mode: 'interpret', state: 'enforced', failPolicy: 'fail-soft-log', enforcement: '@objectstack/formula celEngine (interpret) — console (objectui) + server', covers: [ 'data/field.zod.ts:requiredWhen', 'data/field.zod.ts:readonlyWhen', 'data/field.zod.ts:visibleWhen', - 'data/field.zod.ts:conditionalRequired', ], }, {