Uh oh!
There was an error while loading. Please reload this page.
fix(core,app-shell): close ActionDef — delete the index signature, converge visible/disabled on the spec (#4046) - #4285
Merged
Conversation
…nverge visible/disabled on the spec (#4046) Step 3 of objectstack#4075, executing the maintainer's 2026-08-06 ruling now that its upstream half shipped in @objectstack/spec 17.0.0-rc.6 (objectstack#5970, PR objectstack#6450). - `visible` / `disabled` derive from the spec's unified three-arm shape (`boolean | string(CEL) | { dialect, source }`); `visible` loses its local `| boolean` restatement, `disabled` gains the envelope arm it never had. - `[key: string]: any` deleted from `ActionDef`. `ActionContext` keeps its own — a runtime data bag is legitimately open, a declared contract is not — and the asymmetry is now pinned in both directions. - Five keys the deletion surfaced promoted to real fields: the `navigation` alias spelling (`to` / `external` / `newTab` / `replace`) and `description`. - `DeclaredActionsBar` drops its `(action as any)` predicate casts. - The #4097 ruling-B documentation block lands at `interpolateTarget`'s non-array `params` branch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
…rness
CI shard 3/4 went red on `description rejects a number` while the same case was
green locally, and the difference is the harness's own resolution model rather
than anything about `ActionDef`.
`erroringLines` builds its program with a default compiler host and no `paths`,
so it reads whatever is on disk. Every other derivation it checks resolves
through `@objectstack/spec`, an installed package; `description` derives from
`UIActionSchema['description']` in `@object-ui/types`, a WORKSPACE package that
resolves through its built `dist/index.d.ts`. The unit-test job does not build
workspace packages first, so the indexed access degraded to `any`, the case was
accepted, and the row went red for an unrelated reason.
`tsconfig.typetests.json` is a real project in the dependency graph, so CI's Type
Check job builds `@object-ui/types` before compiling it and the
`@ts-expect-error` on `{ description: 42 }` there is enforced for real — which is
why Type Check was green on the same commit. The pin therefore moves rather than
weakens; an assertion whose colour depends on whether someone ran a build is not
a pin.
Reproduced locally by moving `packages/types/dist` aside: 37 passed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
yinlianghui
marked this pull request as ready for review
August 11, 2026 11:28
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 11, 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
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#4046
Step 3 of the staged narrowing, executing the maintainer's 2026-08-06 ruling now that its upstream half shipped. Steps 1 (#3032) and 2 (#3190) are landed history; the card's status header, not its body's staged plan, is the current state.
The ruling, quoted as delivered:
Premise check
All three legs verified on the post-#4080 tip (
4cb0562b5) before implementing. The card's line anchor had moved — it citedActionRunner.ts:185, the index signature is at:332— but the substance held.ActionDefstill ends with[key: string]: anyActionRunner.ts:33217.0.0-rc.6DeclaredActionsBarstill carries(action as any).disabled:162and:280, untouched by #4080The spec probe was run as six real
ActionSchema.safeParsecalls rather than read off the typings, because the boolean arm is exactly what changed:One thing worth recording, because it nearly produced a wrong premise: the same probe against rc.5 rejects
visible: true.mainmoved to rc.6 recently enough that a stale checkout still reports rc.5, so anchor this check onorigin/main's lockfile, not on a working tree.The convergence
Both keys now derive from one spec field each — no local restatement:
visibleloses the hand-written| booleanbecause the spec adopted that arm, so restating it locally would now be a second contract rather than a declared tolerance.disabledgains the envelope arm it never had — its absence is precisely whyDeclaredActionsBarhad to reach around the type.conditionis untouched: it is objectui dialect with no spec counterpart, outside this ruling.Consumer triage
Deleting the index signature produced a five-error wall, not a wall of hundreds — and the two halves landed in different places. Every one resolved; none stopped.
to,external,newTab,replace,descriptionActionRunner.tsexecuteActionRunner.ts:1076The four
navigation-alias keys were ruled legitimate back in step 1 and have sat inNAVIGATION_ALIAS_KEYSever since — declared as data, never as fields, because step 2's scope was the 18 spec-owned keys.executeNavigationreads them off the action itself, so they reached their own reader through the index signature. Promoted, and deliberately not@deprecated: step 2's acceptance ruled on exactly this point ("navigation 别名四键不标弃用(第 1 步盘点已判定其合法,派发指令有误)"). Hand-written rather than derived because there is no spec field to derive from — that is what makes them dialect.descriptionwas the only key to error outsidecore: twoTS2353s, ataction-group.tsx:246andaction-menu.tsx:228. It is authorable (@object-ui/types'UIActionSchema,ui-action.ts:521), forwarded by all four action renderers becausecheck:action-forward-parityrequires it, and read by the param dialog for its subtitle (#4192) — authorable, forwarded, read, and undeclared. Derived fromUIActionSchema['description'], the same source the parity gate calls authorable for a declared surface. No consumer call site was edited: declaring the key correctly at the producer is what fixed both errors, which is the contract-first shape.executeis category (ii) with a twist worth stating: the fix is not to declare it. It is a live spec tombstone —@objectstack/speckeeps it only so the parser can reject it by name — so declaring it to make the read compile would re-legitimize a retired key. It now reads off an explicitly untyped view of the action:What did NOT retire, against step 1's expectation
Step 1 wrote that the dev-mode warning and
executeScript's rename prescription "can retire" once the index signature came down. Measured, and kept — they cover a disjoint population, not the same one twice:tscsees actions authored as TypeScript, and now rejects an unknown key there outright.sys_metadatarows are rehydrated UNPARSED, so pre-17 metadata reachesrunner.execute()as a plain object no compiler ever looked at. That is the populationexecute: 'markDone'actually lives in.Retiring them would have closed the compile-time half while re-opening the runtime half.
ActionContextkeeps its index signature for the card's own reason (runtime data bag, genuinely open), and that asymmetry is now pinned in both directions rather than assumed.Pins
Type level, two files, deliberately not redundant. A new
actionDef-closed-surface.test.tsstates the contract as ordinary TypeScript compiled bytsconfig.typetests.json(@ts-expect-erroronexecute:, on the typotargt:, on a tombstone, and on a wrong-typed value for each promoted key; bare assignments for all six accepted arms).actionKeys.types.test.tskeeps driving the compiler API with its control interface — which is what proves the rejections come from the deletion and not from an unrelated diagnostic — and itstargtrow flipped fromrejected: falsetotrue, which was always its stated completion condition.The inverted pin inverted.
actionKeys.pin.test.tsasserted the index signature was still there, naming its own retirement condition ("the day this fails, step 3 has landed"). It now asserts the opposite — and via the AST, nottoContain('[key: string]: any')as before, for two reasons: both files now discuss the index signature in prose, so a text-negative pin would go red on a comment; and the AST form can carry the half that matters,ActionContextKEEPING its own. A pin that only checkedActionDefwould stay green through a change that "tidied up"ActionContexttoo.The six arms, both faces. All six already had runtime coverage and it stays where it is —
ActionRunner.disabledGate.test.tsdrives a 12-row table through the execution gate,ActionEngine.visibility.test.tsdoes the same forvisible. What neither can assert is the join, because both reach runtime through a cast (as unknown as ActionDef,as ActionDef) — correct there, since their tables carry junk shapes the closed type now rejects, but it means they would keep passing if the type stopped admitting the arm being exercised. So the new file asserts the same literal type-checking and evaluating, on both polarities per arm.That negative half is not decoration: it caught a real flaw in the first draft of the helper. A
type: 'script'action with no registered script fails on its own terms, so a helper askingresult.success === falsescored those failures as "blocked" and reported all six arms blocking — including the three that must release. The helper now reads the gate's own error string.#2990's runtime prescription branch is green and stays, per the card.
Reverse verification — the direction is inverted, and that is the mechanism
The fix was taken out with a targeted edit restoring only the index signature, then restored with
git checkout HEAD --(nevergit stash— shared stack). Direction predicted before running, and it matched exactly.The naive expectation is "the bad line goes red". It cannot: restoring the index signature makes
{ execute: … }and{ targt: … }compile, which is the opposite of an error. The red arrives at the suppressions — an@ts-expect-errorwith nothing left to suppress is itself a build failure:Exactly two, at
executeandtargt— the two undeclared keys. Every other suppression stayed used, because each sits on a declared property whose own type still rejects the value; that split is what shows the pins are measuring the deletion rather than the promotions.The
exit=0on the first line is worth keeping: the type-level guarantee lives entirely intsconfig.typetests.json. Without the explicit include added here, this whole change would have been unguarded by CI's type gate.One CI-only red, and what it taught (second commit)
The first push was green on Type Check and Lint and red on Test shard 3/4, on a single case —
description rejects a number— that had been green locally. The cause is worth recording because it is a trap for the next person adding a pin to that harness, not a defect in this change:actionKeys.types.test.tsbuilds its own program with a default compiler host and nopaths, so it reads whatever is on disk. Every other derivation it checks resolves through@objectstack/spec, an ordinary installed package.descriptionderives fromUIActionSchema['description']in@object-ui/types— a workspace package that resolves through its builtdist/index.d.ts, and the unit-test job does not build workspace packages first. Withdiststale or absent the indexed access degrades toany,{ description: 42 }is accepted, and the row goes red for a reason that has nothing to do withActionDef.tsconfig.typetests.jsonhas no such problem — it is a real project in the dependency graph, so CI's Type Check job builds@object-ui/typesbefore compiling it. That is exactly why Type Check was green on the same commit while the harness was red, and it means the@ts-expect-erroron{ description: 42 }there is enforced for real. So the twodescriptionrows moved to that file rather than being weakened: an assertion whose colour depends on whether someone ran a build is not a pin.Reproduced locally by moving
packages/types/distaside before re-running — 37 passed, where the pre-fix harness would have produced the two CI failures.Gates
The consumer sweep is the downstream/prefix direction —
...@object-ui/coreis 36 packages — but the runs above are the whole workspace, a superset of it, and core's dist was rebuilt before any downstream judgement so nothing read a stale.d.ts.check:action-forward-parity— green, with a delta worth explainingThe set diff is exactly one key removed and none added:
That is the intended consequence of reading the tombstone off an untyped view: the gate derives
runtime-readfrom property accesses bound to theActionDefparameter, andexecuteis no longer one. Every surface's owed count is unchanged (24/24/24/24/12), becauseowed = authorable ∩ runtime-read − retiredalready subtracted it. The set is simply honest now — a retired key is not something any renderer should be forwarding.The #4097 rider
Ruling B's documentation block lands at
interpolateTarget's non-arrayparamsbranch, per that card's re-scoping ("it rides the NEXT card touchingActionRunner.ts"). Comment only — zero behaviour change. It names the branch an INTERNAL runtime value-bag channel rather than an authoring surface, cites objectstack#6828 and the ruling, and records the hazard the ruling flagged: the branch's safety rests entirely on the producer refusing an authored object-formparams, so a future spec loosening would silently re-admit an authored scope — with the upstream refusal pin and the two local pins as the guards.Out of scope, filed
execute({ … })literal disables the excess-property check this card just installed. Measured two ways: the deletion producedTS2353at the two spread-free sites and silence at the other two, and a direct probe inserting an invented key reproduced the split. It bounds this card's own guarantee, so it is filed unlabeled for triage rather than as an observation.action?.titlefallback reads a key no author can write and no producer sets #4282 (finding) —useConsoleActionRuntime'stitle: action?.label || action?.titlereads a key no author can write and no producer sets. Dormant; also a reminder that closingActionDefcannot reach a binding typedany.Changesets
@object-ui/coreminor and@object-ui/app-shellpatch. Minor follows #3190's measured precedent: step 2 shipped minor for a strictly larger breaking edge (shortcut/bulkEnabledwent from compiling to compile errors), andcheck-changeset-no-majoris green. Noskip-changeset(#3724).Generated by Claude Code