Uh oh!
There was an error while loading. Please reload this page.
fix(app-shell): approver membership-tier picker reads the server-published enum; delegated_admin stops rendering "(invalid)" - #5327
Merged
os-support-ai merged 2 commits intoAug 19, 2026
Conversation
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
os-support-ai
marked this pull request as ready for review
August 19, 2026 15:26
os-support-ai
commented
Aug 19, 2026
CollaboratorAuthor
ACCEPT — PM review, round 17.
Merging via the queue. Generated by Claude Code |
Uh oh!
There was an error while loading. Please reload this page.
os-support-ai
deleted the
claude/issue-5309-membership-tier-server-enum
branch
August 19, 2026 15:27
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#5309
The approver membership-tier select hardcoded three tiers.
delegated_admincould not be picked, and a spec-valid stored value rendered asdelegated_admin (invalid)— a legitimately-saved approver labelled invalid to the author's face.What changed
ReferenceCombobox'sorg-membership-levelbranch now resolves its vocabulary through a new exportedmembershipLevelOptions(source):resolveRefKind'ssource.values(the spec'sxRef.sourcesannotation, carried through byjson-schema-to-fields) is used verbatim — order included. This is the same precedence rule the file already states for record lookups inrecordLookupFor, and it is the only source that cannot drift from what the engine accepts.BUILTIN_MEMBERSHIP_ROLE_OPTIONS(@objectstack/spec/identity), which that package documents as "the picker's vocabulary" and ships with labels. Same move, and the same reason, asKIND_TO_RECORD_LOOKUP(objectui#3017).(invalid)keeps its meaning: a value outside the vocabulary the server actually published is still flagged.Label echoes updated in
flow-node-config.ts. The type-picker label becameOrganization membership tierrather than being re-spelled with four values: enumerating the vocabulary inside a label is the same staleness class this card is about.The pin fork window — measured, and it does not bind
Triage flagged that this repo's spec pin may not cover objectstack#9942. Measured: it does not. This repo pins
@objectstack/spec@17.0.0(lockfile-exact), and:So the approver projection still publishes three values and nothing here asserts it — that pin would be red for reasons unrelated to this change.
But the same pin already carries
BUILTIN_MEMBERSHIP_ROLE_OPTIONSas the complete four-value list with labels, including{label: "Delegated Admin", value: "delegated_admin"}. Deriving the fallback from that — upstream's own source for the #9806 ruling thatORG_MEMBERSHIP_LEVELSisBUILTIN_MEMBERSHIP_ROLES— makes the fix and its tests fully assertable today. After the pin bump the two derivations converge, so nothing here needs revisiting.No spec pin bump is in this PR, per the standing rule that a bump is never a rider.
Verification
Re-derived on
origin/main@6e6884aadfirst. All results below are at528bfe5f1.FlowReferenceField.membershipTier.test.tsx— 11 passed.packages/app-shell/src/views/metadata-admin/inspectors/— 55 files, 609 passed, 1 skipped.packages/app-shell/— 450 files, 4343 passed, 1 skipped, 0 failed (source tree identical to528bfe5f1; the only later change was the changeset.md).pnpm --filter @object-ui/app-shell type-check— clean.lint— 0 errors; the 3 warning sites in the touched file are outside every diff hunk.528bfe5f1:check:control-bytes,check:spec-symbols,check:esm-specifiers,check:phantom-deps,check:self-import,check-changeset-no-major— all pass.Vitest was run from the repository root throughout.
Build artifact between the edit and the thing under test
Stated explicitly because this leg reads a value published by
@objectstack/spec:vitest.config.mtsaliases every@object-ui/*specifier to that package'ssrc/, and the test imports./FlowReferenceFieldrelatively — so vitest transforms the edited source directly. Nodistof app-shell participates.@objectstack/spechas no vitest alias, so it resolves to the publishednode_modules/@objectstack/spec/dist/. That artifact is the pin under test, which is exactly what production consumes.type-checkfirst failed withCannot find module '@object-ui/react'untilpnpm --filter '@object-ui/app-shell^...' buildran. (That run's exit code also read0through the pipe while the output saidExit status 2— the result was read from the output, not the code.)Reverse verification — four legs, predictions declared before each run
Predictions were written down first; all four legs matched exactly. A wholesale revert would have deleted the exported
membershipLevelOptionsand broken the file's import — every test red, discriminating nothing — so each half of the fix was ablated separately, then together.origin/mainbehaviourLeg B reproduced the reported symptom verbatim — the DOM dump contains:
Note leg A vs A+B on
still flags a value the SERVER says is outside its vocabulary: red under A, green under A+B. With the three-value list the tier genuinely is outside the vocabulary, so the flag is correct there for the wrong reason — which is why the combined leg is reported rather than leg A alone.Assertions that pin nothing about this change, stated rather than counted:
still flags genuinely dirty legacy data(sales_manager) — green on all four legs. It guards against a fix-by-deletion of the(invalid)affordance, not against this regression. Annotated as such in the test.falls back for a source that is not an enum at all— green on all four legs; it pins the type narrowing, which no leg exercises.Every other assertion is red on at least one leg. Each leg was restored with
git checkout HEAD -- FILEfrom the committed fix, and the restore was verified byte-identical (git status --porcelainempty) before the next run; the restored tree re-ran green at 11/11.Counter-probes:
toContain('delegated_admin')is paired withnot.toContain('sales_manager')on the same array, so the positive assertion is shown capable of failing.Scope
packages/app-shell/**only. In-flight #5254's surface was checked before starting — it ispackages/components/**(renderers/form/*+ its tests) and does not reachpackages/app-shell, so no breach.Generated by Claude Code