fix(action): one precedence for target vs deprecated execute — lower the alias, then drop it (#3713) - #3742
Merged
Conversation
…wer the alias, then drop it (#3713) `execute` is the deprecated alias of `target`, and three readers resolved "the author declared both" in TWO OPPOSITE directions: - `ActionSchema`'s transform (spec) → kept `target` - objectui `ActionRunner.executeScript` → `execute || target` - CLI compile step (`lowerCallables`) → preferred a fn on `execute` So an action declaring both ran `preferredHandler` server-side and `legacyHandler` client-side — two different scripts for one button, silently, with no error anywhere. `target` now wins everywhere, and the transform DROPS the alias from its output, so the conflict is unrepresentable rather than merely agreed-upon — the same "canonical wins, alias disappears" shape as `agent.knowledge.topics` → `sources` (#1891). Note the server runtime never read `execute` at all (`isHeadlessInvokableAction` gates on `target || body`; the dispatch candidate chain probes `target`/`name`), so authoring it worked solely because it was lowered at parse time — dropping it costs the server nothing and takes the ambiguity off the wire. It also fixes unpatched renderers by construction: an `execute || target` reader falls through to `target` once the alias is gone. The CLI had the same bug in compile-time form, which #3713 did not record: with a function inlined in both slots it bundled the `execute` one and then overwrote `action.target` with that ref, silently discarding the function the author declared on `target`. It now probes `target` first and drops the alias. Authoring is unchanged — `execute` is still accepted on input (`ActionInput`), still lowered, still documented. Consumers of the PARSED metadata must read `target`; `z.infer<typeof ActionSchema>` no longer carries `execute`, so a stale reader fails to compile instead of silently reading `undefined`. Also updates the liveness ledger entry (the divergence it recorded is now resolved, and the cited evidence is the lowering, not a second field reader). Verified: spec 6711 tests, cli 652 (5 new), runtime 655, lint 467, plus the liveness and generated-docs gates. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JDwRktLBFwgtV5Svxm4LJm
|
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:
|
os-zhuang
marked this pull request as ready for review
July 28, 2026 02:07
os-zhuang
added a commit
that referenced
this pull request
Jul 28, 2026
… and drop the alias (#3754) (#3764) Second instance of the alias-drift shape #3742 fixed for `action.execute`, found while verifying that fix. `requiredWhen` is canonical and `conditionalRequired` is its documented deprecated alias, but `FieldSchema` had NO canonicalization at all: both keys stayed live in the parsed output, leaving every consumer to re-implement the precedence — the condition that produced #3713. The alias surviving parse was even test-pinned, including a case named "requiredWhen and its alias conditionalRequired can coexist", the inverse of the contract #3742 had just established one field over. `FieldSchema` now lowers the alias into `requiredWhen` at parse time and removes it from the output; canonical wins when both are declared. The pinning tests are inverted, and a new case asserts the alias is gone from a field parsed through `ObjectSchema` — the path a renderer actually receives. This fixes no live bug: every reader we can see already prefers the canonical key (`rule-validator.ts` reads `requiredWhen ?? conditionalRequired`). Nothing in the contract MADE that right — this is hardening, removing the chance rather than a defect. objectql's fallback is deliberately kept: `evaluateValidationRules` is also handed raw, unparsed field definitions, which still carry the alias. Authoring is unchanged. Consumers of the parsed metadata must read `requiredWhen`; `z.infer<typeof FieldSchema>` no longer carries the alias, so a stale reader fails to compile instead of silently reading `undefined`. Adds `FieldParseInput` (`z.input`) for the author-facing shape — `FieldInput` was already taken by an unrelated `Partial<Field>` factory type in the same file. Closes #3754.
This was referenced Jul 28, 2026
os-zhuang
added a commit
that referenced
this pull request
Jul 28, 2026
…hem a handler (#3743) (#3838) #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 rule has to run PRE-PARSE, because the parse is what consumes the alias. #3743 proposed the CLI compile pipeline as its home; that window is real but nearly always empty, because `defineStack` parses inside the author's own config module — for a `defineStack` app the alias is gone before `os build` ever loads the module. So the rule lives in `@objectstack/spec` (`lintDeprecatedAliases`) and is wired into both layers that perform the discard: `defineStack` warns before parsing (once per distinct conflict), and `os build`/`os validate` run a pre-parse pass for stacks that skip strict `defineStack` (plain object export, `strict: false`, inline function handlers). Each layer reports only its own discards, so one conflict yields one warning. Behaviour fix in the same contract: #3742 probed for a CALLABLE `target` first, which left a string `target` beside a function `execute` still resolving the alias's way — it bound the alias and overwrote the canonical ref the author wrote. `target` now wins in every string/function combination, matching the `ActionSchema` transform. Also documents the rule in the objectui actions reference, which already stated the precedence while saying nothing about the author finding out.
This was referenced Jul 28, 2026
os-zhuang
added a commit
that referenced
this pull request
Jul 30, 2026
…to the protocol-17 conversion layer (#3796) (#3976) * feat(spec,automation): graduate the seven flow-node config aliases into the protocol-17 conversion layer (#3796) FlowNodeSchema.config is an unconstrained record, so the executors were the only statement of which config key is canonical — and seven deprecated aliases lived there: object→objectName behind the readAliasedConfig shim, plus six open-coded ?? fallbacks (notify to/subject/body/url, script functionName/input) with no warning, no ledger, and no retirement path. All seven graduate into the ADR-0087 D2 conversion layer as protocol-17 live-window entries (flow-node-crud-object-alias, flow-node-notify-config-aliases, flow-node-script-config-aliases): a stored flow authored with an alias is rewritten to the canonical key at load — normalizeStackInput and the AutomationEngine.registerFlow rehydration seam alike — with a structured ConversionNotice per rewrite. The executors read canonical keys only, and the emptied readAliasedConfig shim is deleted. actionUrl (not url) is the deliberate canonical of its pair, resolving the contradiction where the notify descriptor documented url as canonical while the executor precedence, tests, and examples all preferred actionUrl: the whole downstream chain already uses that name (sys_notification.action_url, the channel contract, the REST read model), and url elsewhere means an HTTP endpoint to call. The choice is behaviour-preserving. Also: step-17 migration chain + regenerated spec-changes.json / protocol-upgrade-guide.md carry the new entries; PD #12 records the endgame; common-patterns.mdx now authors canonical keys; lint comments updated to reference the conversions instead of the deleted shim. Refs #3796, #3713, #3742, #3754, #3795, ADR-0087. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MFnoExtwnGhNDWXNvn7RKN * docs(releases): v17 notes cover the seven flow-node config alias graduations (#3796) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MFnoExtwnGhNDWXNvn7RKN --------- Co-authored-by: Claude <noreply@anthropic.com>
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 #3713.
The divergence
executeis the deprecated alias oftarget. Three readers resolved "the author declared both" in two opposite directions — #3713 found two of them; the third turned up while verifying:ActionSchematransform —packages/spec/src/ui/action.zod.tstargetActionRunner.executeScript—execute || targetexecutelowerCallables(packages/cli/src/utils/lower-callables.ts)executeSo
defineAction({ type: 'script', target: 'preferredHandler', execute: 'legacyHandler' })ranpreferredHandlerserver-side andlegacyHandlerclient-side — two different scripts for one button, silently.The CLI half was the nastiest of the three: with a function inlined in both slots it bundled the
executeone and then overwroteaction.targetwith that ref, so the function the author actually declared ontargetwas silently dropped from the build.What changed
targetwins everywhere, and the transform drops the alias from its output — the issue's second (stronger) option: the conflict becomes unrepresentable rather than merely agreed-upon. Same "canonical wins, alias disappears" shape asagent.knowledge.topics→sources(#1891), which asserts'topics' in parsed === false.Two findings from verification that make dropping clearly the right call:
executeat all.isHeadlessInvokableActiongates ontarget || bodyand the dispatch candidate chain probestarget/name(packages/runtime/src/action-execution.ts). Authoringexecuteworked solely because the transform lowered it — sotargetwas already the de-facto canonical slot server-side, and dropping the alias costs the server nothing.execute || targetreader falls through totargetonceexecuteis absent, so the spec-side fix alone resolves the divergence for any client receiving parsed metadata — the objectui change is defence-in-depth for paths that bypass parse (cached payloads, raw metadata-fs reads), not a prerequisite.Compatibility
Authoring is unchanged.
executeis still accepted on input (ActionInput=z.input), still lowered totarget, still listed in the reference docs and JSON Schema. Nothing to migrate in app metadata.Consumers of the parsed metadata must read the canonical slot:
parsedAction.execute→ TOparsedAction.targetaction.execute || action.targetbecomesaction.targetz.infer<typeof ActionSchema>no longer carriesexecute, so a stale reader fails to compile rather than silently readingundefined. Swept this repo: no consumer readsexecuteoff a parsed action, and no example/skill/platform-object authors it.Files
packages/spec/src/ui/action.zod.ts— transform lowers and drops;.describe()+ TSDoc updatedpackages/cli/src/utils/lower-callables.ts— probetargetfirst, drop the alias after bindingpackages/spec/src/ui/action.test.ts— the drop is pinned ('execute' in result === false, key list, JSON round-trip, andexecute-only still lowering for atarget-required type)packages/cli/src/utils/lower-callables.test.ts— new; 5 cases covering both-callable precedence, alias drop,execute-only back-compat, string/string left to the spec, and object-nested actionspackages/spec/liveness/action.json+liveness/README.md— divergence recorded as resolved; evidence corrected to the parse-time lowering (and now resolves cleanly against the gate's path check)content/docs/protocol/objectui/actions.mdx+ regeneratedcontent/docs/references/ui/action.mdxVerification
@objectstack/spec@objectstack/cli@objectstack/runtime@objectstack/lintpnpm buildcheck:livenesscheck:docs/check:skill-docsA concurrent
pnpm testrun also flaggeddriver-mongodb,driver-sql,plugin-hono-server,service-analytics,service-automation,service-packageandmetadata-protocol; each passes standalone (verified formetadata-protocolandlint) — resource contention from running 98 vitest tasks in parallel in the container, unrelated to this change.Follow-ups
ActionRunner.tsexecuteScripttoconst script = action.target || action.execute;. Not a prerequisite — see "fixes unpatched renderers" above — but it closes the parse-bypassing paths.executeis now documented, tested and consistent, but still silent, and the compile-time lints all run post-parse (compile.tslintsresult.data) where the alias is already gone — surfacing it needs a pre-parse hook, which is a bigger change than this fix warrants (Prime Directive chore: version packages #10).Generated by Claude Code