Skip to content

RoleDefinition.permissions is required and read by nothing — every consumer uses only name/inherits #4288

Description

@yinlianghui

Observation-class finding, surfaced while wiring @object-ui/permissions' tests into tsc for #4040 (tranche 1, PR #4287). Nothing a user meets today — filed rather than fixed because resolving it either way changes a public exported type.

What

packages/types/src/permissions.ts:38:

export interface RoleDefinition {
  name: string;
  label: string;
  description?: string;
  inherits?: string[];
  system?: boolean;
  permissions: ObjectLevelPermission[];   // required
}

permissions is the only required member besides name and label, and it is documented as the role's direct object-level grants.

Nothing in this repository reads it. A repo-wide search for role.permissions / roleDef.permissions / r.permissions over packages/** and apps/** (excluding dist and node_modules) returns nothing. The four places that take a RoleDefinition[] all use name and inherits only:

  • packages/permissions/src/evaluator.ts:18, and resolveRoles at :92 — walks inherits, matches on name
  • packages/permissions/src/store.ts:34
  • packages/permissions/src/PermissionProvider.tsx:22

Every grant the evaluator actually applies comes from the separate ObjectPermissionConfig[], keyed by object with a roles map inside it. So there are two declared homes for "what a role may do", and only one of them is wired.

How it surfaced

Five of @object-ui/permissions' declared TEST_DEBT errors were TS2741 — fixtures annotated RoleDefinition that omit permissions:

src/__tests__/evaluator.test.ts(16,7): error TS2741: Property 'permissions' is missing in type '{ name: string; label: string; }' but required in type 'RoleDefinition'.
src/__tests__/store.test.ts(14,3): error TS2741: ...

PR #4287 gives each fixture permissions: [], which is accurate for those roles and leaves the type alone. That is the right fix for the test files, and it is also the thing that makes the dormancy visible: five fixtures independently omitted a required field, and every case still asserted exactly what its author meant, because no code path was ever going to look.

Why it is finding and not queued

No user-visible behaviour depends on it either way. It is one of two shapes, and telling them apart is a product question, not a code one:

  • The field is aspirational — role-attached grants were designed and never wired. Then it is ADR-0049 enforce-or-remove territory: either build the reader or retire the declaration.
  • The field is intended and merely optional in practice — every real role today gets its grants from ObjectPermissionConfig. Then it should be permissions?:, and the required-ness is what is wrong.

Either resolution edits packages/types' published surface, so it wants a ruling rather than a guess. Cheap to settle whenever the permissions surface next gets attention.

Refs #4040, #4287, objectstack-ai/objectstack#4118.

Activity

  1. claude commented on Aug 11, 2026

    @claude
    Contributor

    Findings triage (grade): hold — finding stays. RoleDefinition.permissions is a required member of a public exported type (packages/types/src/permissions.ts) that nothing in the repo reads — the evaluator family walks name/inherits only. Resolving it either way (optional-ize, remove, or wire a consumer) changes a public exported type, which is a contract call, not a lane fix; but with zero consumers and zero user impact there is no pull to force that call today. Held as the declared-but-unread record; escalate to a decision card when the @object-ui/types permission surface is next revised (the #4040 tranche work that surfaced this is the likely trigger).

    本评论来自分诊座位 Routine(#5474 试点),不构成认领。


    Generated by Claude Code

  2. os-zhuang commented on Aug 11, 2026

    @os-zhuang
    Contributor

    Findings triage: promoted — finding → pm:queue as a member of sweep #4328 (dead-surface deletions batch 3). Not individually dispatchable while the sweep is open; the sweep PR closes it via Fixes.

    Premise re-verified on origin/main @ b16be40: .permissions has zero production reads in packages/permissions/src — resolveRoles (evaluator.ts:92) walks only name/inherits, and the sole non-test hit is a test comment documenting exactly this state. The sweep row allows "remove" or an explicitly argued demotion-to-optional; the PR must state which and why.

    本评论来自分诊座位 Routine(#5474 试点),不构成认领。


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions