Repository navigation
fix(app-shell): stop offering org.id in ConditionBuilder's subject dropdown - #9949
Merged
os-tesla merged 3 commits intoSep 18, 2026
Merged
Conversation
…dropdown The row builder's subject dropdown carries its own default vocabulary, which objectui#9645 did not reach: it narrowed the raw editor's autocomplete only. So `org.id` stayed one click away at every mount that declares no vocabulary. `org` is bound by no host — `buildExpressionScope` publishes the identity roots and no `org` — and `@objectstack/formula`'s `SCOPE_ROOTS` has none either, so the record-scope validator rejects `org.id` outright and names a record field that does not exist as the remedy. This editor was advertising a subject its own linter refuses: the `app` case objectui#8155 settled for `ConditionalFormattingEditor`, reached through a different control. `user.*` is deliberately left in place. The engine accepts it and the browser-side evaluator binds it, so it is a subject that really works at the client-evaluated mounts; a mount whose host does not bind it narrows its own list through the existing `subjects.context` vocabulary. The asymmetry that licensed objectui#9645's narrowing does not transfer here — `roots` feeds suggestions, while this list is the row builder's only subject control. `REFERENCE_ROOTS` deliberately keeps `org`: it answers whether a typed VALUE is a reference, and quoting one would rebuild the silently-false predicate objectui#6293 fixed. Co-authored-by: Claude <noreply@anthropic.com>
…ry docblock The `ConditionSubjectVocabulary` docblock gave the number of record-scoped mount sites as a figure. It had already expired, and this branch's own census contradicts it. Replaced with a pointer to the instrument that re-derives it, per AGENTS.md #9 — the same repair this branch already made to the matching claim in `ConditionBuilder.subjectVocabulary.test.tsx`. Co-authored-by: Claude <noreply@anthropic.com>
Contributor
✅ 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
|
1 similar comment
Contributor
✅ 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
|
…wn subjects
A hook `condition` and an object validation rule's guard are evaluated by
objectstack against `{ record, previous }` and nothing else — measured at
source: `wrapDeclarativeHook` in `hook-wrappers.ts`, and `checkPredicate` /
`checkConditional` in `validation/rule-validator.ts`. Neither binds `user`.
So a `user.*` subject picked from the dropdown at those two mounts compiled a
row that can never match: the hook wrapper throws on an unevaluable condition,
and the rule validator is fail-closed, rejecting every write to the object.
Both mounts now declare `context: RECORD_CONDITION_SUBJECTS`, the dropdown's
mirror of `RECORD_CONDITION_ROOTS` — same ruling, the other door.
Declared per mount, never defaulted, and deliberately NOT derived from
`scope === 'record'`: that test does not separate the hosts. The action
`visible` / `disabled` mounts declare `scope="record"` and are evaluated in the
browser by `useCondition`, whose scope comes from `buildExpressionScope` and
really does bind `user` — narrowing them would take away a subject that works.
`scope` is a claim about how the CEL is LINTED, not about what the host binds.
The `ConditionWidget` mount in `widgets.tsx` is left alone on purpose: it is
polymorphic over `CONDITION_SCOPE_BY_METADATA_TYPE`, which carries both `hook`
(server, two bindings) and `action` (client, `user` bound), so one `context`
there would be right for one type and wrong for another.
Co-authored-by: Claude <noreply@anthropic.com>
Contributor
✅ 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
|
os-tesla
marked this pull request as ready for review
September 18, 2026 20:41
os-tesla
deleted the
claude/issue-9855-condition-builder-context-subjects
branch
September 18, 2026 21:01
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.
Part of #9855.
Closes the card's
org.idhalf. Theuser.*half is deliberately NOT done — the measurement below says narrowing it by default would be wrong — so this does not use a closing keyword. The remaining half is stated as an open question at the bottom for the seat to route.What the card asked me to re-measure, and what I found
The card's engine leg was explicitly unverified (the filing seat could not resolve
@objectstack/formula). It is resolvable —17.4.0, frompackages/app-shell, which declares it. pnpm keeps it undernode_modules/.pnpm/, so a look at the rootnode_modulesreports a false absence.Measured against that installed engine,
origin/main00c4df509, 2026-09-18T19:12Z:SCOPE_ROOTS?validateExpressionatscope: 'record'org.idok: false— rejected, remedy namesrecord.orguser.id/user.email/user.role/user.isAdminok: true— cleanrecord.id,previous == null(controls)ok: true— clean⇒ the card's engine claim reproduces verbatim for
organd refutes foruser. The two roots are on opposite sides of the ruling this repo already made, so they are not one edit.The governing precedent — the file's own docblock, quoted
And the sibling ruling it descends from, objectui#8155 as
ROW_PREDICATE_ROOTSstates it:orgis that case, byte for byte, reached through a different control: refused by the engine, bound by no host (buildExpressionScopepublishes the identity roots and noorg).useris theosmirror case from the same ruling — accepted by the engine AND bound bybuildExpressionScopefor every predicate evaluated in the browser.My census — measured, not inherited
origin/main00c4df509, 2026-09-18T19:11Z. Six production files, seven mount sites:context?scopeActionDefaultInspector— Visible whenrecordActionDefaultInspector— Disabled whenrecordHookDefaultInspector— Run only whenrecordObjectValidationsPanelrecordConditionWidget(widgets.tsx)PageBlockInspector— Visible whenFlowNodeConfigField— entry condition⇒ six of seven mount sites took the default, and every one of them was offering
org.id.#6296docblock's "all five that mount it today" and thesubjectVocabularypin's "five files / six mount sites … none of them passessubjects" had both expired — the flow trigger declares acontexttoday. Per AGENTS.md #9 I replaced the figure with a pointer rather than a fresh number.Why
user.*is not narrowed hereThe asymmetry that licensed objectui#9645's narrowing does not transfer to this control, and that is the whole reason this was a second card rather than a widening:
rootsfeeds the raw editor's suggestions — a mount that loses one loses an offer and keeps the spelling.scope === 'record'does not separate the hosts either: the Action visible/disabled mounts declarescope="record"and are client-evaluated, whereuserreally is bound and really does match. The file states in its own words that this component cannot tell server- from client-evaluated apart from anything a mount passes. So narrowinguser.*belongs to the server-evaluated mounts as a declared vocabulary, through thesubjects.contextmechanism objectui#6296 already built — not to a default here.Ablation — the input the two implementations disagree about
Mutation = re-add
org.idto the default list, i.e. undo the fix, at a default-vocabulary mount.anchor hit 0 times) rather than writing nothing and reporting success. Re-run from a file-based replacement:83efca0ecbfe83718bf9682ba44e627460eea4a3,org.idoccurrences 082a96cc974d3e7c23883642285bda8e584597630; occurrences 0 → 183efca0e…— hash equality; occurrences 1 → 0;git diff HEADempty (exit 0)trap ... EXIT INT TERMwith absolute paths throughout; the restore is proved by hash and empty diff, never by an exit code.The three that reddened:
offers only subjects the record scope ACCEPTS — every one lints clean(the new gate — it can fail)`org.id` is refused by the engine — the reading the removal rests onoffers record.FIELD for the catalog, plus the record/user context, and nothing elseThe
user.*must-not-break case stayed green under mutation, which is correct: the mutation only re-addedorg.Tests
New
ConditionBuilder.contextSubjects.test.tsxreads the default vocabulary off the rendered dropdown and puts each subject to the real validator, so it never restates the engine's answer — it reddens if a subject is added that the engine refuses, and equally if the engine starts refusing one still offered. It also pins the must-not-break half (user.*stays) and that a storedorg.idpredicate still opens in row mode with its subject selectable, so withdrawing an offer cannot strand metadata an author already wrote.objectui#9854's
mountRootspin is untouched and still green.Verification
VERDICT command-exit 0@object-ui/app-shelltype-checkCannot find module '@object-ui/*'was unbuilt siblings, not the diff)check:control-bytes,check:new-line-citations,check:vi-mock-specifiers,check:vi-mock-inherit,check:vi-mock-override-shape,check:test-path-roots,check:changeset-claims,check:pending-changeset-literals,check:unreferenced-sourcescheck-changeset-presence,check-changeset-no-majoreslint, narrowed and declared: run on exactly the 3 changed source files, exit 0, 0 errors, 3 warnings. The narrowing is a measurement, not a skip — (i) the population is read from
eslint.config.js's ownfiles/ignores; (ii) the file count3is from--format json; (iii) no type-aware program is configured (noprojectService/parserOptions.project), so a file's verdict depends only on its own bytes and this diff cannot move any untouched file's result. The same 3 warnings are present on the base version of the file (measured, not assumed), so this diff adds zero findings. Repo-wide lint is CI's run.Not a governed surface —
check-governed-queue-guard.mjs --teston this diff:NOT GOVERNED — 4 path(s) checked.Open question handed back — the
user.*half⛔ Not answered here, because answering it is a product choice about specific mounts' vocabulary and #8167 records that three of those mounts' tiers are still open.
Should the server-evaluated mounts declare a narrower
subjects.context? At the hookcondition,user.*lints clean and silently never matches if the host really binds{ record, previous }only.context: [record.id]. One line each, uses the existing mechanism. Needs the host binding confirmed at source in objectstack first.scope === 'record'. ⛔ Recommended against, and this is the measured objection: the Action visible/disabled mounts declarescope="record"and ARE client-evaluated withuserbound, so C removes a working subject from them.Recommendation: B, gated on measuring the hook wrapper's bindings in objectstack — it is the only option that narrows exactly where the root is unbound, and it is what both the
contextdocblock and objectui#9645 prescribe (declared, not inferred).Generated by Claude Code
Round 2 — option B, written by the seat
The dev did ⛔ not PATCH this body (a dev writes its PR body once, at the opening call; later changes are NAMED in its report and written by the seat). This section is the seat's, appended at 2026-09-18T19:57Z with the clock read by this same write.
Two mounts qualified, not one. The seat measured objectstack's hook wrapper —
packages/objectql/src/hook-wrappers.ts:253–:268, a hookconditionis evaluated against{ record, previous }and nothing else — and the dev re-derived that itself rather than inheriting it, then measured the side the seat had not named:packages/objectql/src/validation/rule-validator.ts, wherecheckPredicatecallsExpressionEngine.evaluate(expr, { record, previous: previous ?? undefined })and an unevaluable guard rejects the write (「rejected, not skipped」). Seat-verified at source in the same act. ⇒ the hookconditionmount and the validationwhen/conditionmount both declarecontext: RECORD_CONDITION_SUBJECTS; the client-evaluated mounts are untouched and the shared default is unchanged.Part of #9855stays, and it is deliberate. The card stays open until this half is reviewed; an auto-close with a live open question is the irreversible direction.⛔ Correction to this card's own dispatch, recorded so the next reader does not inherit it: the claim that
@objectstack/formula「is not installed in the shared checkout」 is a false absence. It is installed (17.4.0) and resolvable frompackages/app-shell, which declares it; pnpm keeps it undernode_modules/.pnpm/, so only a look at the ROOTnode_modulesreports it missing. The filing seat drew a NOT-MEASURED on that basis and this seat relayed it into the dispatch without re-measuring it. The engine leg was measurable all along, and measuring it is what split this card:orgis absent fromSCOPE_ROOTS(removed), whileuseris bound at every client-evaluated mount (kept).Generated by Claude Code