feat(spec,cli): warn the author when a discarded action alias costs them a handler (#3743)

.changeset/action-alias-conflict-warning.md

@@ -0,0 +1,52 @@
+---
+"@objectstack/spec": minor
+"@objectstack/cli": patch
+---
+
+feat(spec,cli): warn the author when a deprecated action alias is discarded (#3743)
+
+#3742 made `target` beat the deprecated `execute` alias everywhere and had the
+`ActionSchema` transform **drop** the alias from its output, so "two different
+scripts for one button" became unrepresentable. What it left behind: an author
+who declares both slots with different values still loses one of the two
+handlers they wrote, **silently**. Per Prime Directive #12 that belongs at
+authoring time, so it is now reported there.
+
+**New rule — `action-target-execute-conflict` (advisory).** An action declaring
+both `target` and `execute` with different values gets a warning naming both
+handlers, stating that `target` wins, and giving the one-line fix (delete
+`execute`). Identical values in both slots are harmless duplication and stay
+quiet. It never fails the build: the resulting stack is well-defined — the cost
+is a handler that never runs, not a broken artifact.
+
+The rule must run **pre-parse**, because the parse is what consumes the alias:
+once `ObjectStackDefinitionSchema` has run there is no `execute` key left to
+report. It therefore lives in `@objectstack/spec`
+(`lintDeprecatedAliases`, exported from the package root) and is wired into
+both layers that perform the discard:
+
+- **`defineStack`** — the dominant authoring path, and the one that consumes the
+  alias earliest: it parses inside your own config module, so by the time
+  `os build` loads that module the alias is already gone. It now warns on the
+  console before parsing (once per distinct conflict per process).
+- **`os build` / `os validate`** — a new pre-parse pass covering stacks that
+  skip strict `defineStack`: a plain object default-export,
+  `defineStack(…, { strict: false })`, and inline function handlers (`target` is
+  `z.string()`, so those cannot pass strict `defineStack` and are lowered by the
+  CLI instead). Both commands lint the same input, so they agree by construction
+  (#3782).
+
+Each layer reports only its own discards, so one authored conflict produces
+exactly one warning however the stack is compiled.
+
+**Behaviour fix in the same contract.** #3742 fixed compile-time precedence by
+probing for a *callable* `target` first, which left one combination still
+resolving the alias's way: a **string** `target` beside a **function** `execute`
+bound the alias and then overwrote the canonical ref the author wrote. `target`
+now wins in every combination of string/function across the two slots, matching
+the `ActionSchema` transform — so the new warning states one precedence rule
+that is true everywhere. If you relied on an inline `execute` function winning
+over a string `target`, move it into `target`; the warning names the action.
+
+Authoring is otherwise unchanged: `execute` alone is still accepted, still
+lowered into `target`, and still documented.

content/docs/protocol/objectui/actions.mdx

@@ -53,6 +53,8 @@ The `type` field selects how an action is dispatched. The complete enum is **`sc
 
 `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.
+
 ### Script Actions
 
 Run logic with no server round trip via `body` (an L1 CEL expression or L2 sandboxed JS). `body` is only meaningful when `type` is `script`, and declaring one on any other type is a parse-time error — those types dispatch on `target`, so the body would never be invoked.

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 } from '@objectstack/spec';
+import { ObjectStackDefinitionSchema, normalizeStackInput, lintDeprecatedAliases } from '@objectstack/spec';
 import { loadConfig } from '../utils/config.js';
 import { lowerCallables } from '../utils/lower-callables.js';
 import { validateStackExpressions } from '@objectstack/lint';
@@ -95,6 +95,53 @@ export default class Compile extends Command {
       if (!flags.json) printStep('Normalizing stack definition...');
       const normalized = normalizeStackInput(config as Record<string, unknown>);
 
+      // 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<string, unknown>);
+      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

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, type ConversionNotice } from '@objectstack/spec';
+import { ObjectStackDefinitionSchema, normalizeStackInput, lintDeprecatedAliases, type ConversionNotice } from '@objectstack/spec';
 import { loadConfig } from '../utils/config.js';
 import { validateStackExpressions } from '@objectstack/lint';
 import { validateListViewMode } from '@objectstack/lint';
@@ -601,12 +601,25 @@ export default class Validate extends Command {
       const viewRefErrors = viewRefLint.filter((f) => f.severity === 'error');
       const viewRefWarnings = viewRefLint.filter((f) => f.severity !== 'error');
 
-      const authoringLintErrors = [...flowLintErrors, ...autonumberErrors, ...viewRefErrors];
+      // 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<string, unknown>);
+      const aliasLintErrors = aliasLint.filter((f) => f.severity === 'error');
+      const aliasLintWarnings = aliasLint.filter((f) => f.severity !== 'error');
+
+      const authoringLintErrors = [...flowLintErrors, ...autonumberErrors, ...viewRefErrors, ...aliasLintErrors];
       const authoringLintWarnings = [
         ...flowLintWarnings,
         ...livenessLint,
         ...autonumberWarnings,
         ...viewRefWarnings,
+        ...aliasLintWarnings,
       ];
       if (authoringLintErrors.length > 0) {
         if (flags.json) {

packages/cli/src/utils/lower-callables.test.ts

@@ -92,6 +92,33 @@ 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', () => {
     const result = lowerCallables({
       objects: [{

packages/cli/src/utils/lower-callables.ts

@@ -175,9 +175,10 @@ export function lowerCallables(input: Record<string, unknown>): LoweringResult {
 
 /**
  * Lower a single action definition: detect a callable on `target` or on the
- * deprecated `execute` alias (canonical `target` first — #3713), register it,
- * optionally extract a metadata body, and drop the alias. Mutates a shallow
- * clone, never the input.
+ * 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.
  */
 function lowerActionCallable(
   raw: unknown,
@@ -197,13 +198,32 @@ function lowerActionCallable(
   // (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'
-      : typeof action.execute === 'function'
+      : !targetDeclared && typeof action.execute === 'function'
         ? 'execute'
         : null;
-  if (!handlerSlot) return action;
+  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;
   const ref = uniqueName(baseName, taken);
   taken.add(ref);

packages/spec/api-surface.json

@@ -1,5 +1,6 @@
 {
   ".": [
+    "ACTION_TARGET_EXECUTE_CONFLICT (const)",
     "ADMIN_FULL_ACCESS (const)",
     "ALL_CONVERSIONS (const)",
     "AUDIENCE_ANCHOR_POSITIONS (const)",
@@ -35,6 +36,7 @@
     "DatasourceMappingRule (type)",
     "DatasourceMappingRuleSchema (const)",
     "DefineStackOptions (interface)",
+    "DeprecatedAliasFinding (interface)",
     "EVERYONE_POSITION (const)",
     "EvalUser (type)",
     "EvalUserInput (type)",
@@ -144,11 +146,13 @@
     "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)",

packages/spec/src/index.ts

@@ -107,6 +107,12 @@ 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 } 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';