Uh oh!
There was an error while loading. Please reload this page.
feat(types)!: retire the ActionCondition branch shape from ActionSchema.condition - #5981
Merged
yinlianghui merged 1 commit intoAug 24, 2026
Merged
Conversation
…ma.condition
`condition` was declared as an `{ expression, then, else }` branch DSL with a
zod mirror that accepted it, and two docs pages taught it with worked examples.
Nothing ever read `expression` / `then` / `else`: the only consumer of the key
is `ActionRunner.execute`, which reads it as a PREDICATE gate. A branch object
carries no `source`, so the runner's normalizer read it as "no gate declared"
and executed the action unconditionally — predicate never evaluated, branches
never dispatched, zero diagnostics, `os validate` green.
Retired under enforce-or-remove (maintainer ruling 2026-08-09, route B) so the
key has one reading:
- remove `ActionCondition` and `ActionConditionSchema` (and both barrel exports)
- retype `ActionSchema.condition` to the predicate the runtime honours:
`boolean | string | { dialect?, source }` — the arms `ActionDef.condition`
carries, and the vocabulary `visible` / `disabled` already use
- rewrite both teaching sites to the live vocabulary: `condition` is a gate; a
branch is separate actions with mutually exclusive `condition`s
The zod parse verdict flips both ways, measured against origin/main: the branch
object goes from accepted to refused, and every live predicate spelling goes
from refused to accepted (the old schema required `expression`).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CSoz9uGhaaSgiq3hshtN7LContributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
yinlianghui
marked this pull request as ready for review
August 24, 2026 10:36
Uh oh!
There was an error while loading. Please reload this page.
yinlianghui
deleted the
claude/issue-3917-retire-action-condition-branch
branch
August 24, 2026 10:37
This was referenced Aug 24, 2026
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 freeto 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.
Fixes#3917
Maintainer ruling 2026-08-09, route B — retire the
{ expression, then, else }shape per enforce-or-remove (directive 「全部接受」 covering that round's whole decision inbox). Option A (honouring the branch DSL inActionRunner.execute) is explicitly not taken.What was wrong
@object-ui/typesdeclaredActionSchema.conditionas an{ expression, then, else }branch DSL and shipped a zod mirror (ActionConditionSchema) that accepted it. Nothing ever readexpression/then/else. The only consumer of the key isActionRunner.execute, which reads it as a predicate gate; a branch object carries nosource, so the runner's normalizer read it as "no gate declared" and executed the action unconditionally — predicate never evaluated,then/elsenever dispatched,os validate/os buildgreen, zero diagnostics. Two docs pages taught the shape with worked examples, so an author following the documentation ("amounts over 1000 go to manager approval") got unconditional execution.What this PR does
ActionCondition(crud.ts) andActionConditionSchema+ActionConditionSchemaType(zod/crud.zod.ts), and both barrel exports (index.ts,zod/index.zod.ts).ActionSchema.conditionto the predicate the runtime actually honours —boolean | string | { dialect?: string; source: string }— the same three armsActionRunner's ownActionDef.conditioncarries and the same vocabularyvisible/disableduse. The key survives as a gate; only the branch shape is retired, which is what makes the rewritten docs coherent with the type.content/docs/core/enhanced-actions.mdx(Conditional Execution — gate spellings, how the gate is read, branching as separate actions, a retirement note) andcontent/docs/api/schema-reference.md(ActionSchema property table + the JSON example).minor, per this repo's version-alignment rule reservingmajorfor following@objectstackacross a major (AGENTS.md 版本号策略;check-changeset-no-major.mjsenforces it). Same classification as theMobileOverridesretirement (finding(types):MobileOverrides.navigation发布了三值导航词表,全仓零读点 —— 与 #3985 同族的另一处「声明即全部」 #4919).One documentation claim is stated more precisely than the ruling's one-liner, because the code says so: a sequential
chainstops at the first step whoseconditiondoes not hold (a blocked step returnsAction condition not met, andexecuteChainsequential returns on the first failure —packages/core/src/actions/ActionRunner.ts). So the page teaches per-actionconditions on sibling actions for an either/or, and a stepconditionfor gating an optional follow-up. Teaching "put both branches in one sequential chain" would have re-created this card's own failure mode.Evidence
All gate runs below are on
24d653d5e(this branch's head, working tree clean).The zod parse verdict flips in both directions — measured, not asserted. A throwaway worktree at
origin/main(2aff580b5) parsed the same fixtures through the old schema; the numbers came out of vitest's own failure output, not from prose:conditionvalueorigin/main@2aff580b5{ expression, then, else }success: true)invalid_uniononcondition)false'data.amount > 1000''${data.amount > 1000}'{ dialect: 'cel', source: … }The old schema required
expression, so the spelling the runtime has always honoured was the one the schema rejected. Both directions are now pinned inphase2-schemas.test.ts(anit.eachover the four live spellings, plus a refusal test asserting the full parse is red and that the issue is on theconditionpath).pnpm exec vitest run packages/types/→Test Files 53 passed (53),Tests 581 passed (581).pnpm --filter @object-ui/types type-check(tsc --noEmit && tsc -p tsconfig.examples.json && tsc -p tsconfig.test.json) → exit 0.pnpm exec turbo run build $(node scripts/check-doc-snippet-types.mjs --build-filter) --concurrency=2→Tasks: 32 successful, 32 total. Every one of those packages is atscbuild of its ownsrc/against the new@object-ui/typesdist/*.d.ts, so the narrowing breaks no consumer.node scripts/check-doc-snippet-types.mjs→ controls live (sentinel … TS2305, resolution landed onpackages/types/dist/index.d.ts),Semantic phase: 135 of 135 block(s) judged, 0 failed.node scripts/check-doc-component-types.mjs→✅ Every documented component type is registered.node scripts/check-doc-links.mjs→Links are valid across 13 scan roots.node scripts/check-control-bytes.mjs→✅ OK (scanned 4951 tracked text file(s)).node scripts/check-changeset-presence.mjs→✅ 6 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s);node scripts/check-changeset-no-major.mjs→✅ No changeset declares a major bump.pnpm --filter @object-ui/types lint→ exit 0 (0 errors; 251 pre-existingno-explicit-anywarnings, none of them new — the diff removes one unused import and adds noany).Declared narrowing: the repo-wide
pnpm lint/pnpm testfarm was not run locally — CI runs it exactly once regardless. Type-aware linting is not enabled ineslint.config.js(noparserOptions.project/projectService), so this diff cannot move any untouched file's lint verdict; every.tsfile it touches was linted directly (6 files, 0 errors).The "zero consumers" premise, re-verified
Independently re-measured on this tree rather than taken from the card: the nine
ActionConditionread points enumerated in the dispatch are exact (the card'scrud.ts:212/crud.zod.ts:86had drifted to:205/:84, and the card omitted both test read points — thezod-mirror-parityregistry entry is keyed by the schema name and would have failed CI as a stale entry). A repo-wide grep for a consumer ofcondition.expression|then|elseoutsidepackages/typesreturns zero, and no JSON fixture, example app or e2e schema authors the branch shape. Premise holds; no fork.Out of scope, filed not fixed
A third teaching site writes the retired spelling —
content/docs/guide/schema-overview.md:84— which neither the card, the ruling nor the fence names. It sits inside a declared doc-snippet fragment, so no gate compiles it. Filed unassigned as #5980 rather than widening this PR's fenced file surface.Kept in draft per the dispatch contract: the PM seat reviews CI and decides readiness.
Generated by Claude Code