fix(showcase): gate the restricted tier option on the built-in org_admin position - #18658
Merged
os-try-charles merged 1 commit intoSep 17, 2026
Merged
Conversation
…min position The cascading-select fixture gated its `restricted` tier option with `'admin' in current_user.positions`, but `admin` is never on the server's positions axis: a membership row's `sys_member.role = 'admin'` is normalized by `mapMembershipRole` into the built-in identity name `org_admin` before it is pushed onto `positions`. The predicate was therefore false for every principal server-side, while the file's own docblock asserted that the rule-validator enforces the same rule against a non-admin submitter. Name the built-in position instead, and correct the docblock so it no longer asserts server-side behaviour that does not hold — it now also records why the raw membership word `admin` never reaches the axis, which is the trap an author copying this fixture would otherwise inherit. Claude-Session: https://claude.ai/code/session_017ef78bLdybu3AffehKkhfk Co-authored-by: Claude <noreply@anthropic.com>
Collaborator
Author
席内复核(派发席
|
os-try-charles
marked this pull request as ready for review
September 17, 2026 13:07
os-try-charles
deleted the
claude/issue-15943-showcase-org-admin-position
branch
September 17, 2026 13:30
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
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 #15943
Clause-②: no — this narrows a predicate from a name that never matched to the one that does. Nothing is widened: the accepted set for a non-admin is unchanged (measured below), and the only principal whose verdict moves is one the docblock already claimed the option was for.
What was wrong
examples/app-showcase's cascading-select fixture gated itsrestrictedtier option with'admin' in current_user.positions, butadminis never on the server's positions axis. A membership row'ssys_member.role = 'admin'is normalized bymapMembershipRoleinto the built-in identity nameorg_adminbefore it is pushed ontopositions(packages/core/src/security/resolve-authz-context.ts:793-794). The predicate was therefore false for every principal server-side, while the file's own docblock asserted that the rule-validator enforces the same rule against a non-admin who submits the value anyway.What changed
One file, two places — the predicate and the docblock that describes it:
tier.restricted.visibleWhennow reads'org_admin' in current_user.positions, matching the built-in name the docs already demonstrate (content/docs/permissions/positions.mdx:90).adminnever reaches the axis — the trap an author copying this fixture would otherwise inherit.Triage's option 2 (seeding a
sys_positionrow literally namedadmin) was refused and is not done here: it would make the flagship example demonstrate a position name trivially confusable with the built-in one.Counts, each with a firing control
Tree
objectstack-ai/objectstack, worktreeobjectstack-issue-15943, BASEcf39b83c0, HEAD63fffbf4b.'admin' in current_user.positionsin the changed file at HEADGuangdongin the same file: 1 — reader live'org_admin' in current_user.positionsin the changed file at HEADgit diff BASE HEAD -- PATH | grep -c GuangdongGuangdongis present in the file — so the diff-scoped reader discriminates rather than matching everythinggrep -rn 'current_user.positions' examples/examples/app-showcase/CHANGELOG.md:4044, a historical entry — a true positive outside the fix surfaceBoth directions, measured
mapMembershipRoleread from the built artifactpackages/spec/dist/index.mjs(built in this worktree at HEAD63fffbf4b):Both legs then import a real object-definition file from this tree — AFTER is the fixed file on HEAD, BEFORE is the BASE blob written out verbatim with one export binding renamed — and drive them through the real objectql rule-validator from its built
dist. The predicate text is read off the imported metadata, not retyped:sys_member.role = 'admin'user resolves toorg_admin, the predicate is true, the option is offered and the rule-validator accepts the submit. The sentence the docblock has been asserting all along is now true.ValidationError. The green was not bought by loosening the predicate.standardvalue stays accepted for the non-admin in both legs, so the harness is not refusing everything.The BEFORE leg's landing was proven on disk (
git hash-objectof the written file vs the BASE blob, old-predicate lines 2 / new-predicate lines 0, control termGuangdong1), and its removal was proven by observed state — file absent,git status --porcelainempty,git diff HEADempty — never by an exit code. The runner carries anEXIT INT TERMtrap with absolute paths. No test file is left behind: this is a one-off measurement, not a shipped fixture.Verification
pnpm --workspace-concurrency=2 --filter '@objectstack/example-showcase^...' buildunder the shared verify lock:VERDICT command-exit 0.pnpm --filter @objectstack/example-showcase typecheck && ... testunder the lock:VERDICT command-exit 0; 29 test files, 381 tests passed.dispatch-gates --commands --repo objectstack-ai/objectstackderived 35; all 35 were run with exit codes recorded before any pipe, and reconciled:✓ dispatch-gates --ran: 35 derived famil(ies) accounted for — 34 run, 1 NOT-MEASURED (1 DERIVED from a recorded exit 3).33 green. The two non-zero readings are not this diff:pnpm check:dual-build-cjs-loadsexit 3 = PREREQUISITE NOT MET — it reads built output and this worktree built only the showcase's dependency closure, not every package. Nothing was measured; recorded as such, not as a failure.pnpm check:cross-package-test-inputsexit 1, and the finding namespackages/cli/test/init-created-files-summary.e2e.test.tsdescendingpackages/spec/dist/— a file this diff does not touch. Proven pre-existing by ablation: with the BASE content of the one changed file restored on disk (landing proven bygit hash-objectequality with the BASE blob), the gate fails identically. Restore proven by hash equality with the HEAD blob plus an emptygit status. See the acceptance note below.Changeset — measured,
skip-changesetThe one criterion is whether anything published moves.
examples/app-showcase/package.json:private: true, and it declares nofiles[]at all.packages/spec—privateunset,files=["dist","json-schema","liveness","prompts","llms.txt","README.md","src/**/*.zod.ts","CHANGELOG.md","api-surface","spec-changes.json"];packages/lint—privateunset,files=["dist","README.md","CHANGELOG.md"](it does ship itsCHANGELOG.md). The reader discriminates.pnpm check:published-filesexit 0 on this diff.cac62d5f9("add the loop { parallel } fixture", test(showcase): add the loop { parallel } fixture that unblocks the flow-run-step-nesting clause #17514), carried no changeset either.Nothing in any manifest's
files[]moves, so this PR takes theskip-changesetlabel rather than an empty changeset.Acceptance notes (out of scope, not filed from this seat)
packages/cli/test/init-created-files-summary.e2e.test.tsdescendspackages/spec/dist/and no declared glob reaches inside that radius, sopnpm check:cross-package-test-inputsreds whenever a diff pulls the@objectstack/cliscan into the run. Reproduced here at BASE content, i.e. independently of this PR. Dedupe words:cross-package-test-inputs,init-created-files-summary,spec/dist walk,heldBy,undeclared walk root.e2e/live/cascading-options.spec.tsmentions the role gate only in prose, and no repo test asserts thatrestrictedis offered to an admin. That absence is exactly why this defect sat unnoticed, but a missing pin is not itself a reproducible defect, a contract violation or an authoring trap. Next person to reach it: whoever next edits this fixture or that live spec.packages/objectql/src/validation/rule-validator.option-visibility.test.tsuses a synthetic'admin' in current_user.positionsfixture. That is the generic evaluator under test with an arbitrary position name, not a claim about the identity axis, so it is correct as written; recorded only so the next reader does not mistake it for a second instance of this card.Sibling card #15136 remains open and is untouched here; this fix is independent of it in both directions.
Generated by Claude Code