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/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/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/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/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/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', ], }, { 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) { 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. ---