feat: separate read/write access (drop read_write) + edit UI for write-grantees (#1127)#1128
Merged
Merged
Conversation
Separate read and write access: the combined `read_write` level is gone. `SkillPermissionLevel` is now `"read" | "write"` — a `write` grant implies read (you can't edit what you can't see) but there's no combined label and a principal holds at most one level. Behaviour is unchanged from #1123: `canReadSkill` (any grant ⇒ read), `canWriteSkill` (a `write` grant), `canManageSkill` (owner/admin) are the same gates with the value renamed. Updated the Zod enum, `normalizeGrants` (write wins), `levelAllowsWrite`, the `permissions_changed` analytics count (`readWriteGrants` → `writeGrants`), and the skillset delegation comments. `coerceStoredGrants` defensively maps a stored legacy `read_write` → `write` at read time, so a doc the boot migration hasn't rewritten yet (or one written by an older pod mid-rolling-deploy) still resolves to write — never dropped, never silently downgraded. Part of #1127
Rewrite any stored grant carrying the legacy `read_write` level to `write` on skills + skillsets, at boot, right after the typed-grants backfill. Idempotent (only docs with a `read_write` grant match) and non-destructive (read grants + all other fields untouched; `write` confers exactly what `read_write` did). Failure is non-fatal — `coerceStoredGrants` maps the legacy value at read time, so no access is lost. Integration tests pin the rename (read_write→write, read grants intact, legacy lists + privacy preserved), idempotency, and the skillset twin. Part of #1127
`SkillPermissionLevel` becomes `read | write` in both SDKs, matching the API. Value-only rename — method names and shapes are unchanged. Tests updated; TS vitest + Python pytest/ruff/mypy green. Part of #1127
`SkillPermissionLevel` → `read | write` in the web model; the permissions editor's Write tab emits `write` grants; the visibility-card count prop `readWriteCount` → `writeCount`. Value-only rename matching the API/SDK. Part of #1127
Fix the bug where a user with write access saw no edit affordances: the UI
gated everything on `isOwner` and never learned about the write tier.
Add a shared `useSkillAccess(skill)` hook → { isOwner, isAdmin, canWrite,
canManage }, mirroring the backend gates (`canWrite` = owner OR platform
admin OR a `write` grant matching the user directly or via a granted org,
resolved against `useMyOrgs`). Re-point the content-edit affordances — the
Edit button, the inline file editor, and Save — at `canWrite`, while
admin actions (permissions, transfer, delete, visibility, refresh, version
mgmt) stay on owner/admin. `SkillHeroStrip`'s edit button now keys off the
handler's presence (dropped its `isOwner` prop). EditSkillPage gates its
package update on `canWrite` and its visibility toggle on `canManage`, with
a read-only notice for non-writers.
Tests: write-grantee sees Update Package but not the visibility toggle; a
read-only viewer sees the notice and no controls.
Part of #1127
CONVENTIONS §5.4 tier renamed READ_WRITE → WRITE, grant level value and prose updated to `write` (write implies read); ERRORS accepted levels now read/write; ARCHITECTURE analytics field readWriteGrants → writeGrants. Part of #1127
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
1 participant
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.
Summary
Separates read and write access — the combined
read_writelevel is gone — and fixes the bug where a user with write access saw no edit UI.Closes #1127
What changed
Model:
read_write→write(API + both SDKs + web + docs). Each grant isreadorwrite; awritegrant implies read, but there's no combined label and a principal holds at most one level. Gate behaviour is unchanged from #1123 — only the value renamed.Migration (non-disruptive, rollback-safe): a boot step rewrites any stored
read_writegrant →writeon skills + skillsets (idempotent; read grants + legacy lists + privacy untouched).coerceStoredGrantsalso maps a storedread_write→writeat read time, so an un-migrated doc (or one written by an older pod mid-deploy) never loses access.The bug fix — write-grantees can now edit: a shared
useSkillAccess(skill)hook computes the frontend tiers (canWrite= owner / platform admin / awritegrant matching the user directly or via a granted org). The Edit button, inline file editor, and Save are re-pointed atcanWrite; admin actions (permissions, transfer, delete, visibility, refresh, version mgmt) stay owner/admin. EditSkillPage gates its package update oncanWriteand its visibility toggle oncanManage, with a read-only notice for non-writers.Verification
🤖 Generated with Claude Code