feat(spec,cli): warn the author when a discarded action alias costs them a handler (#3743) - #3838
Merged
Merged
Conversation
…hem a handler (#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. New advisory rule `action-target-execute-conflict`: an action declaring both slots with different values gets a warning naming both handlers, stating that `target` wins, and giving the one-line fix. Equal values 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. The rule has to run PRE-PARSE, because the parse is what consumes the alias: once `ObjectStackDefinitionSchema` has run there is no `execute` key left to report. #3743 proposed the CLI compile pipeline as its home, between `lowerCallables` and `safeParse`. That window is real but nearly always empty: `defineStack` parses inside the author's own config module, so for a `defineStack` app — i.e. almost every app — the alias is gone before `os build` ever loads the module. A CLI-only lint would have been structurally blind on the dominant authoring path. So the rule lives in `@objectstack/spec` (`lintDeprecatedAliases`, exported from the package root) and is wired into both layers that perform the discard: - `defineStack` warns on the console before parsing, once per distinct conflict per process — the earliest point the conflict exists. - `os build` / `os validate` run a new pre-parse pass over `normalized`, covering stacks that skip strict `defineStack`: a plain object default-export, `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) and the parity gate covers the new rule. Each layer reports only its own discards, so one authored conflict yields 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 string/function combination, matching the `ActionSchema` transform, so the new warning states one precedence rule that is true everywhere. Verified end to end on both authoring paths: a `defineStack` config warns once from the authoring gate and the CLI pass stays quiet; a plain-object config warns from the CLI pass for both the string/string and string/function pairs, and the emitted artifact keeps the canonical `target` with no `execute` on the wire. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JqC9ckaLbmhmytt65Gxi96
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
Bot
added
documentation
Contributor
📓 Docs Drift CheckThis PR changes 2 package(s): 110 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
…aw NUL byte `check:nul-bytes` (#3127) caught two literal U+0000 bytes in the `warnDeprecatedAliases` key. A raw NUL makes ripgrep treat the whole file as binary and silently return zero matches, so `stack.zod.ts` would have dropped out of code search and every grep-based lint. Written as the unicode escape it is byte-identical at runtime. Same separator in `deprecated-aliases.ts`'s dedupe key, with a note on why it must stay an escape, and regenerate the spec API-surface snapshot for the four new exports (0 breaking, 4 added). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JqC9ckaLbmhmytt65Gxi96
…3743) The objectui actions reference already stated the precedence — "if an action declares both, `target` wins and `execute` is discarded" — while saying nothing about the author ever finding out. Name the rule, both surfaces that raise it, that it never fails the build, and the one-line fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JqC9ckaLbmhmytt65Gxi96
os-zhuang
marked this pull request as ready for review
July 28, 2026 09:25
This was referenced Jul 28, 2026
os-zhuang
added a commit
that referenced
this pull request
Jul 28, 2026
…#3743) (#3854) #3838 introduced `lintDeprecatedAliases` — the pre-parse pass that reports an alias the parse itself is about to consume — with one rule, for `action.execute`. #3743 predicted the pass would earn its keep beyond that rule. It does: `execute` was never special. The spec has exactly THREE transforms that fold an alias into its canonical key and then drop it from the parsed output, and all three share one failure mode — declare both slots with different values and one is discarded with no signal, invisible to every downstream check because the parse already erased the evidence. Two more rules, same shape, same advisory severity, same two surfaces: field-requiredwhen-conditionalrequired-conflict — `FieldSchema` folds `conditionalRequired` into `requiredWhen` (#3754/#3764); the discarded predicate never gates the field. Covers fields on objects AND on object extensions, and compares the predicate TEXT so a bare string and the `{ dialect, source }` envelope it lowers into read as the same predicate. agent-knowledge-sources-topics-conflict — `AIKnowledgeSchema` folds `knowledge.topics` into `knowledge.sources` (#1878/#1891); the discarded list names RAG sources the agent never recruits from. Compares by SET, so the same sources in a different order stay quiet. The three findings now share one builder, so they phrase the common half identically (which slot wins, that the alias is dropped, delete it) while each still names what its own discarded value would have done. No CLI change: both call sites already loop over whatever the pass returns. Also corrects `content/docs/ai/agents.mdx`, which documented `knowledge` as `{ topics, indexes }` and used `topics` in all three examples — teaching the deprecated alias as the canonical key, and contradicting `skills/objectstack-ai/SKILL.md`, which already had it right. Follow-ups filed: #3855 (removal schedule for all three aliases), #3856 (objectui's now-dead `execute || target` fallback).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3743.
What this fixes
#3742 made
targetbeat the deprecatedexecutealias everywhere and had theActionSchematransform 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.New advisory rule —
action-target-execute-conflict. An action declaring both slots with different values gets a warning naming both handlers, stating thattargetwins, and giving the one-line fix (deleteexecute). Equal values 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.Where the rule had to go — a correction to the issue's premise
#3743 proposed the CLI compile pipeline as the home, in a new pre-parse hook between
lowerCallablesandsafeParse. That window is real, but for the dominant authoring path it is empty:defineStackparses inside the author's own config module, soexecuteis consumed at module-load time, long beforeos buildcallsloadConfig. Confirmed empirically — adefineStackconfig withtarget: 'preferredHandler', execute: 'legacyHandler'reaches the CLI as{"target":"preferredHandler"}, noexecutekey. A CLI-only lint would have been structurally blind on essentially every real app.So the rule lives in
@objectstack/spec(lintDeprecatedAliases, exported from the package root, next tonormalizeStackInput) and is wired into both layers that actually perform the discard:defineStackdefineStackapps — the dominant pathwarnGenericPasswordFields)os build/os validatepre-parse passdefineStack(…, { strict: false }), inline function handlerstargetisz.string(), so a function handler cannot pass strictdefineStackat all — it only ever reaches the pipeline via a non-strict path and is lowered by the CLIEach layer reports only its own discards —
defineStackdeliberately stays quiet instrict: falsemode, where nothing has been discarded yet — so one authored conflict produces exactly one warning however the stack is compiled. Both CLI commands lint the samenormalizedinput, so they agree by construction (#3782) andvalidate-build-gate-parity.test.tscovers the new rule with no change.Behaviour fix in the same contract
#3742 fixed compile-time precedence by probing for a callable
targetfirst, which left one combination still resolving the alias's way:lowerCallablesfell through to the alias, bundled it, and then overwroteaction.targetwith the resulting ref — the canonical slot the author wrote lost to the alias, the same inversion #3713 fixed for the function/function pair, one slot type over.targetnow wins in every string/function combination, matching theActionSchematransform, so the new warning can state one precedence rule and have it be true everywhere. The losing function alias is still deleted (it is not JSON-safe andActionSchemaexpects a string), and the warning is what tells the author which handler they lost.If you relied on an inline
executefunction winning over a stringtarget, move it intotarget— the warning names the action.Changes
packages/spec/src/shared/deprecated-aliases.ts— the rule set for the pre-parse window, pure and side-effect free; exported from the spec root barrelpackages/spec/src/stack.zod.ts—warnDeprecatedAliases+ the call indefineStack's strict pathpackages/cli/src/commands/compile.ts/validate.ts— the pre-parse pass, severity honoured so a future rule gates the build without rewiringpackages/cli/src/utils/lower-callables.ts— the string-target/function-executeprecedence fixpackages/spec/src/ui/action.zod.ts— TSDoc now points at the warningTesting
packages/spec/src/shared/deprecated-aliases.test.ts(new, 13 cases) — quiet paths (one slot only, equal strings, same function reference, empty string, no actions), object context, top-level/nested dedupe, inline functions, the mixed pair, map-shaped slots, multiple offenderspackages/spec/src/stack.test.ts—defineStackwarns once naming both handlers, still drops the alias, stays quiet for one-slot/equal-value configs, nags once across repeated definitions, and stays silent instrict: falsepackages/cli/src/utils/lower-callables.test.ts— a stringtargetsurvives a functionexecute, which is dropped, and the lowered stack stays JSON-safe@objectstack/spec6764 passed ·@objectstack/cli749 passed · all example apps pass, including app-showcase'sno-startup-warnings.test.ts— no app in the repo declares both slots, so the new warning adds no noiseEnd-to-end on both authoring paths (temporary configs, removed before commit):
Emitted artifact for the plain-object config — the canonical slot survives, nothing on the wire:
Generated by Claude Code