Skip to content

docs(approvals): unblock main — drop duplicated role warning, generalize approver literal - #3120

Closed
os-zhuang wants to merge 1 commit into
mainfrom
fix/docs-role-word-approvals-lint
Closed

docs(approvals): unblock main — drop duplicated role warning, generalize approver literal#3120
os-zhuang wants to merge 1 commit into
mainfrom
fix/docs-role-word-approvals-lint

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Why

lint.yml has been red on main since #3113 — the Reserved-word ("role") docs ratchet step fails, so every open PR inherits it (e.g. #3116). Reproduces from a clean origin/main:

check-role-word: 1 problem(s)
• content/docs/automation/approvals.mdx: role-word count grew 5 → 9. New occurrences are banned (ADR-0090 D3).

#3113 added four occurrences to content/docs/automation/approvals.mdx without ratcheting the baseline.

The decision: reword, not a baseline bump

Both obvious framings turned out to be wrong, so the evidence is worth stating.

"role is deprecated, so reword it away" — false.role is live and load-bearing:

  • packages/spec/src/automation/approval.zod.ts still enumerates 'role'alongside'position' in ApproverType — they coexist and mean different things; neither is a migration half.
  • packages/spec/src/automation/approval.test.ts explicitly asserts 'role' parses.
  • expandRoleUsers resolves it against sys_member.role at runtime.
  • No deprecation marker anywhere on it.

role here is the better-auth org-membership tier (sys_member.role: owner/admin/member) — precisely ADR-0090 D3's single documented exception ("third-party schema we do not own"). #2738 (216fa9a) did not deprecate it; it addedposition beside it and documented role as "the membership tier it actually is".

"They're legitimate, so bump the baseline 5 → 9" — legitimate, but not necessary. Three of the four are a near-verbatim duplicate of the callout 80 lines earlier, and that earlier one is better (it names the approval-role-not-membership-tier lint rule). The fourth was an over-narrow API description. The baseline is for occurrences that can't be removed without losing accuracy — these can.

What changed

  • Removed the duplicated warning in the lifecycle Steps. Kept the fact that section actually needs (an approver entry resolving to nobody parks the run forever — the consequence) and cross-referenced the authoritative warning under 3. The approval node.
  • Generalized approverId: it was documented as accepting role:<r>. The runtime falls back to a generic `${a.type}:${a.value}` literal for every approver type (expandApprovers), and rest-server.ts only splits on commas — it never validates the prefix. <type>:<value> is the real contract; role:<r> described one arbitrary instance of it.

Net: same information, one authoritative home, no accuracy lost, and the reserved-word surface shrinks back to 5 instead of freezing 4 avoidable occurrences. The surviving 5 are the one better-auth boundary callout — deliberately untouched, since renaming a third-party identifier we don't own would make the docs wrong.

Verification

$ node scripts/check-role-word.mjs
check-role-word: OK (49 baselined file(s), no new occurrences).

scripts/role-word-baseline.json is unmodified — the diff is one file. The sibling lint.yml doc gates also pass locally:

  • check:doc-authoring199 files clean
  • check:release-notesOK

Notes for the reviewer

  • Docs-only → skip-changeset.
  • ⚠️ There is an uncommitted, unpushed draft of the same fix in a sibling worktree on branch fix/role-word-approvals (no PR opened). It reaches a similar conclusion but cross-references a broken anchor (#the-approval-node; the heading is ### 3. The approval node). If that agent is still active, close whichever of these lands second.

🤖 Generated with Claude Code

…teral (#3113 lint fix)
`lint.yml` has been red on main since #3113: the reserved-word ratchet
(ADR-0090 D3) caught content/docs/automation/approvals.mdx growing 5 → 9
occurrences of "role". Every open PR inherits the failure.
The four new occurrences are NOT a fifth legitimate boundary needing a
baseline bump — three of them are a near-verbatim duplicate of the callout
80 lines earlier, and the fourth is an over-narrow API description:
- The lifecycle Steps repeated the "type: 'role' is not a position" warning
that "3. The approval node" already carries (and carries better — it names
the `approval-role-not-membership-tier` lint rule). Kept the fact the
lifecycle section actually needs (an unresolved approver list parks the run
forever) and cross-referenced the authoritative warning instead.
- `approverId` was documented as accepting `role:<r>`. The runtime falls back
to a generic `` `${a.type}:${a.value}` `` literal for EVERY approver type
(approval-service.ts expandApprovers), and the REST layer only splits on
commas — it never validates the prefix. `<type>:<value>` is the accurate
contract; `role:<r>` described one arbitrary instance of it.
Net: same information, one authoritative home, no accuracy lost, and the
baseline stays at 5 rather than freezing 4 avoidable occurrences.
Deliberately NOT rewording the surviving 5: `role` there is the better-auth
org-membership tier (`sys_member.role`), which is D3's single documented
exception. It is live, not deprecated — spec `ApproverType` still enumerates
'role' alongside 'position' (approval.zod.ts, asserted in approval.test.ts)
and `expandRoleUsers` resolves it against `sys_member.role`. Renaming a
third-party identifier we do not own would make the docs wrong.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@os-zhuangos-zhuang added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Jul 17, 2026
@vercel

vercelBot commented Jul 17, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
specReadyReadyPreview, CommentJul 17, 2026 12:14pm

Request Review

@os-zhuang

Copy link
Copy Markdown
ContributorAuthor

关闭:与 #3122 重复,且两者都是创可贴——只把文档里的重复表述删掉,没碰造成混淆的根因ApproverType.role 这个名字本身)。

改为按 #3133 的结论一次做对:把 role 改名成 ADR 早已规定的投影名 org_membership_level,文档随之不再需要写这个保留词,role-word 棘轮的红是改名的副产品而不是单独一轮修补。新 PR 会引用本 PR 与 #3122

@os-zhuang
os-zhuang deleted the fix/docs-role-word-approvals-lint branch July 17, 2026 12:22
xuyushun441-sys pushed a commit that referenced this pull request Jul 17, 2026
…l` (#3133)
`ApproverType.role` was the last platform surface projecting the word
ADR-0090 D3 reserved. Renaming it also unbreaks `lint.yml` on main: the
role-word ratchet has been red since #3113 (approvals.mdx 5 → 9), and the
docs stop needing the word once the type is spelled correctly.
D3's exception does not cover this enum. It protects better-auth's own
`sys_member.role` COLUMN — third-party schema we cannot rename. `ApproverType`
is ours: an authoring surface, i.e. the *projection*, which D3 says is spelled
`org_membership_level` and labelled "organization membership", never "role".
The sentence licensing the leak is itself false. ADR-0090 D3:203 claims
`sys_member.role` is "already relabelled `org_membership_level` in the platform
projection (ADR-0057 D7)" — but `org_membership_level` appeared nowhere in the
codebase (one comment in position.zod.ts), and ADR-0057 D7:335 lists that
relabel under "Deferred (evidence-gated, P4)". The projection never landed, so
the word reached authors.
The name manufactured a silent failure ("hotcrm class"): every sibling surface
renamed to `position` (`sys_role`, `ShareRecipientType.role`, `ctx.roles[]`),
so `{ type: 'role', value: 'sales_manager' }` reads as a position's legacy
spelling. It resolves against the membership tier, finds no member row, falls
back to an inert `role:sales_manager` literal, and the request waits forever.
Repo-wide, `type: 'role'` had ZERO real callers — only lint tests and the docs
warning that exists to undo the confusion the name creates.
- spec: `ApproverType` gains `org_membership_level`; `role` kept as a
deprecated alias for one window so a published 15.x flow keeps loading.
`DEPRECATED_APPROVER_TYPES` + `canonicalApproverType()` are the single
source for the mapping (runtime and lint both read it).
- plugin-approvals: resolves on the canonical type, warns on the deprecated
spelling, `expandRoleUsers` → `expandMembershipTierUsers`. The `type:value`
fallback literal deliberately keeps the AUTHORED spelling — 15.x wrote
`role:<v>` into `sys_approval_approver` / `pending_approvers`, and
canonicalising it here would orphan every stored slot.
- lint: `approval-role-not-membership-tier` →
`approval-approver-not-membership-tier` (the rule id carried the word too),
plus `approval-approver-type-deprecated`. Mutually exclusive: a bad VALUE
wins, because prescribing `org_membership_level` for a position name is
wrong advice — the fix there is `position`.
- docs/skill/reference + role-word baseline ratcheted DOWN (approvals.mdx
5 → 1, automation SKILL.md 3 → 1); api-surface snapshot regenerated
(0 breaking, 2 added).
Studio still offers "Role" and its picker calls `client.list('role')` on a
metadata type D3 deleted — that picker is already dead, and the dropdown is
objectui's own hardcoded copy of this enum. Tracked as objectui follow-up in
degrades to free text (strictly better than a picker that lists nothing).
Closes#3133. Supersedes #3120 and #3122, which only deleted the duplicated
docs callout without touching the name that causes the confusion.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
xuyushun441-sys pushed a commit that referenced this pull request Jul 17, 2026
…l` (#3133)
`ApproverType.role` was the last platform surface projecting the word
ADR-0090 D3 reserved. Renaming it also unbreaks `lint.yml` on main: the
role-word ratchet has been red since #3113 (approvals.mdx 5 → 9), and the
docs stop needing the word once the type is spelled correctly.
D3's exception does not cover this enum. It protects better-auth's own
`sys_member.role` COLUMN — third-party schema we cannot rename. `ApproverType`
is ours: an authoring surface, i.e. the *projection*, which D3 says is spelled
`org_membership_level` and labelled "organization membership", never "role".
The sentence licensing the leak is itself false. ADR-0090 D3:203 claims
`sys_member.role` is "already relabelled `org_membership_level` in the platform
projection (ADR-0057 D7)" — but `org_membership_level` appeared nowhere in the
codebase (one comment in position.zod.ts), and ADR-0057 D7:335 lists that
relabel under "Deferred (evidence-gated, P4)". The projection never landed, so
the word reached authors.
The name manufactured a silent failure ("hotcrm class"): every sibling surface
renamed to `position` (`sys_role`, `ShareRecipientType.role`, `ctx.roles[]`),
so `{ type: 'role', value: 'sales_manager' }` reads as a position's legacy
spelling. It resolves against the membership tier, finds no member row, falls
back to an inert `role:sales_manager` literal, and the request waits forever.
Repo-wide, `type: 'role'` had ZERO real callers — only lint tests and the docs
warning that exists to undo the confusion the name creates.
- spec: `ApproverType` gains `org_membership_level`; `role` kept as a
deprecated alias for one window so a published 15.x flow keeps loading.
`DEPRECATED_APPROVER_TYPES` + `canonicalApproverType()` are the single
source for the mapping (runtime and lint both read it).
- plugin-approvals: resolves on the canonical type, warns on the deprecated
spelling, `expandRoleUsers` → `expandMembershipTierUsers`. The `type:value`
fallback literal deliberately keeps the AUTHORED spelling — 15.x wrote
`role:<v>` into `sys_approval_approver` / `pending_approvers`, and
canonicalising it here would orphan every stored slot.
- lint: `approval-role-not-membership-tier` →
`approval-approver-not-membership-tier` (the rule id carried the word too),
plus `approval-approver-type-deprecated`. Mutually exclusive: a bad VALUE
wins, because prescribing `org_membership_level` for a position name is
wrong advice — the fix there is `position`.
- docs/skill/reference + role-word baseline ratcheted DOWN (approvals.mdx
5 → 1, automation SKILL.md 3 → 1); api-surface snapshot regenerated
(0 breaking, 2 added).
Studio still offers "Role" and its picker calls `client.list('role')` on a
metadata type D3 deleted — that picker is already dead, and the dropdown is
objectui's own hardcoded copy of this enum. Tracked as objectui follow-up in
degrades to free text (strictly better than a picker that lists nothing).
Closes#3133. Supersedes #3120 and #3122, which only deleted the duplicated
docs callout without touching the name that causes the confusion.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
os-zhuang added a commit that referenced this pull request Jul 17, 2026
…l` (#3137)
Closes#3133. Renames the last platform surface projecting the ADR-0090 D3 reserved word: `ApproverType.role` → `org_membership_level`, with `role` kept as a deprecated alias for one window (resolves identically, warns). Publishes `xEnumDeprecated` on the node configSchema so Studio drops the deprecated spelling from the approver-type picker. Supersedes #3120/#3122. objectui half: objectstack-ai/objectui#2643 (merged).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/sskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@os-zhuang