feat: separate read/write access (drop read_write) + edit UI for write-grantees (#1127)

.changeset/read-write-split.md

@@ -0,0 +1,10 @@
+---
+"ornn-api": minor
+"ornn-web": minor
+---
+
+Separate read and write access (drop the `read_write` level) and show edit UI to write-grantees.
+
+The combined `read_write` grant level is renamed to `write`: each grant is now `read` or `write` (a `write` grant implies read), with no combined label. Existing `read_write` grants migrate to `write` automatically at boot — non-disruptive and rollback-safe.
+
+This also fixes a bug where a user with **write** access saw no edit controls: the skill detail page (Edit button, inline file editor, Save) and the edit page now reveal content-editing to write-grantees, not just the owner, while admin actions (permissions, transfer, delete, visibility) remain owner/admin-only.

docs/ARCHITECTURE.md

@@ -43,7 +43,7 @@ so dashboards can disambiguate from frontend events of the same name):
 | `api.skill.published` | skill create + version publish | `skillId`, `skillVersion`, `isNewSkill` |
 | `user.login` / `user.logout` | session open / close | — |
 | `skill.created` / `.updated` / `.deleted` / `.version_deleted` | mutation routes | `skillId`, `skillName`, `version`, `adminAction?` |
-| `skill.visibility_changed` / `.permissions_changed` | visibility + sharing flips | `skillId`, `isPrivate`, `sharedWithUsers`, `sharedWithOrgs`, `readWriteGrants` (count of `read_write` grants, #1123) |
+| `skill.visibility_changed` / `.permissions_changed` | visibility + sharing flips | `skillId`, `isPrivate`, `sharedWithUsers`, `sharedWithOrgs`, `writeGrants` (count of `write` grants, #1123) |
 | `skill.ownership_transferred` | ownership handed to another user (#1123) | `skillId`, `skillName`, `priorOwnerId`, `newOwnerId` |
 | `skill.refresh` / `.source_linked` / `.source_unlinked` | source-pointer ops | `skillId`, `repo`, `ref`, `commit` |
 | `skill.nyxid_service_tied` / `.agentseal_rescanned` | tie + admin-rescan | `skillId`, `isSystemSkill`, `score` |

docs/CONVENTIONS.md

@@ -346,7 +346,7 @@ Endpoint-specific. Rules:
 Format: `ornn:<resource>:<action>`. These are the **request scopes** NyxID
 mints onto an access token — `requirePermission(...)` middleware checks them
 per route. They gate *who may call an endpoint at all*; they are **distinct
-from** the per-object READ / READ_WRITE / ADMIN tier (§5.4), which decides
+from** the per-object READ / WRITE / ADMIN tier (§5.4), which decides
 *what the caller may do to a specific skill/skillset*. A caller needs both:
 the route scope to reach the handler, and the object tier to act on the
 target.
@@ -355,7 +355,7 @@ target.
 |---|---|
 | `ornn:skill:read` | Read skills (respects visibility) |
 | `ornn:skill:create` | Create skills (upload, pull from GitHub) |
-| `ornn:skill:update` | Update / publish / refresh / change permissions / transfer ownership / toggle deprecation / bind NyxID service (+ object ADMIN/READ_WRITE per §5.4) |
+| `ornn:skill:update` | Update / publish / refresh / change permissions / transfer ownership / toggle deprecation / bind NyxID service (+ object ADMIN/WRITE per §5.4) |
 | `ornn:skill:delete` | Delete a skill or a single version (+ object ADMIN per §5.4) |
 | `ornn:skill:build` | Invoke skill generation endpoints (high LLM cost) |
 | `ornn:playground:use` | Invoke playground chat (runs user code) |
@@ -396,11 +396,11 @@ tier decides whether they may act on *this specific* skill/skillset.
 
 | Tier | Gate | Who qualifies | What it grants |
 |---|---|---|---|
-| **READ** | `canReadSkill` | Public skill → anyone. Private → author, platform admin, or any grantee (`read` **or** `read_write`), directly or via a granted org. | View / pull / execute / list versions. |
-| **READ_WRITE** | `canWriteSkill` | Author **OR** platform admin **OR** a `read_write` grantee (direct or via a granted org). | READ, plus update the skill's **content + metadata only** (publish a new version). |
+| **READ** | `canReadSkill` | Public skill → anyone. Private → author, platform admin, or any grantee (`read` **or** `write`), directly or via a granted org. | View / pull / execute / list versions. |
+| **WRITE** | `canWriteSkill` | Author **OR** platform admin **OR** a `write` grantee (direct or via a granted org). | READ, plus update the skill's **content + metadata only** (publish a new version). |
 | **ADMIN** | `canManageSkill` | Author **OR** platform admin **only**. | Change permissions, transfer ownership, delete skill/version, toggle deprecation, manage dist-tags, bind a NyxID service. |
 
-A `read_write` grantee is **never** an admin — the danger-zone operations stay
+A `write` grantee is **never** an admin — the danger-zone operations stay
 with the author (`createdBy`) and platform admins (`ornn:admin:skill`). Org
 grants resolve uniformly: every admin/member of a granted org inherits the
 grant's level. The org-membership gates fail soft on an unresolved NyxID
@@ -416,15 +416,16 @@ skill/skillset detail responses and accepted by the permissions endpoints
 {
   "grants": [
     { "type": "user", "id": "<nyxid-person-user-id>", "level": "read" },
-    { "type": "org",  "id": "<nyxid-org-user-id>",    "level": "read_write" }
+    { "type": "org",  "id": "<nyxid-org-user-id>",    "level": "write" }
   ]
 }
 ```
 
 - `type` — `"user"` (a NyxID person user_id) or `"org"` (a NyxID org user_id).
 - `id` — the principal's NyxID id (1..128 chars).
-- `level` — `"read"` or `"read_write"`. An invalid value is rejected with
-  `invalid_permission_level` (a `validation_error` subcode — see `docs/ERRORS.md`).
+- `level` — `"read"` or `"write"` (a `write` grant implies read). An invalid
+  value is rejected with `invalid_permission_level` (a `validation_error`
+  subcode — see `docs/ERRORS.md`).
 
 The author (`createdBy`) is never represented in `grants` — they hold implicit
 ADMIN. The legacy read-only `sharedWithUsers` / `sharedWithOrgs` arrays are
@@ -440,7 +441,7 @@ POST /v1/skillsets/:id/transfer-ownership    { "newOwnerUserId": "<id>" }
 ```
 
 - **Auth:** ADMIN tier (`canManageSkill`) — author or platform admin only. A
-  `read_write` grantee can never transfer. Rides on the existing
+  `write` grantee can never transfer. Rides on the existing
   `ornn:skill:update` request scope; **no new scope** was added.
 - **Behavior:** immediate, synchronous transfer. The target becomes the new
   owner (`createdBy`); the prior owner is kept as a **READ** grantee (retains
@@ -627,7 +628,7 @@ Per-test teardown is the test's responsibility; shared fixtures live in `tests/f
 - [ ] `X-Request-ID` on every response; `requestId` in every error body
 - [ ] Query params camelCase; arrays as repeated keys; `q` for search
 - [ ] Required request scopes from the catalog declared in OpenAPI `security` (§5.2)
-- [ ] Object-level authz, where the target is an owned resource, gates on the READ / READ_WRITE / ADMIN tier (§5.4) — distinct from the route scope
+- [ ] Object-level authz, where the target is an owned resource, gates on the READ / WRITE / ADMIN tier (§5.4) — distinct from the route scope
 - [ ] Content negotiation for multi-representation resources
 - [ ] SSE events named `<resource>_<event>` snake_case
 - [ ] Deprecation uses RFC 8594 headers

docs/ERRORS.md

@@ -67,9 +67,9 @@ The uploaded payload is not a parseable ZIP — a malformed or unreadable archiv
 
 ### invalid_permission_level
 
-A typed `grants` entry on a permissions request (#1123) carried a `level` outside the allowed set. The only accepted values are `read` and `read_write` (see [`docs/CONVENTIONS.md`](CONVENTIONS.md) §5.4). Surfaced from `PUT /api/v1/skills/{id}/permissions` and `PUT /api/v1/skillsets/{id}/permissions`.
+A typed `grants` entry on a permissions request (#1123) carried a `level` outside the allowed set. The only accepted values are `read` and `write` (see [`docs/CONVENTIONS.md`](CONVENTIONS.md) §5.4). Surfaced from `PUT /api/v1/skills/{id}/permissions` and `PUT /api/v1/skillsets/{id}/permissions`.
 
-**Client action:** set every grant's `level` to `read` or `read_write` and retry.
+**Client action:** set every grant's `level` to `read` or `write` and retry.
 
 ### invalid_transfer_target
 

ornn-api/src/bootstrap.ts

@@ -146,7 +146,7 @@ import { wireAdmin } from "./domains/admin/bootstrap";
 // per-provider arrays). One-time, idempotent, runs before any
 // LlmProvidersService consumer reads from disk.
 import { migrateModelCatalogIntoProviders } from "./domains/settings/llmProviders/migration";
-import { backfillTypedGrants } from "./domains/skills/crud/grants.migration";
+import { backfillTypedGrants, renameReadWriteGrantsToWrite } from "./domains/skills/crud/grants.migration";
 import { createLlmPickerRoutes } from "./domains/settings/llmProviders/routes";
 
 // OpenAPI spec
@@ -690,6 +690,18 @@ export async function bootstrap(
     ),
   );
 
+  // ---- read_write → write grant-level rename (#1127) ----
+  // The combined `read_write` level was renamed to `write`. Rewrite any
+  // existing grant carrying the legacy value. Idempotent + non-disruptive
+  // (write confers what read_write did); `coerceStoredGrants` covers any doc
+  // not yet rewritten, so failure is non-fatal.
+  await renameReadWriteGrantsToWrite(db).catch((err) =>
+    logger.error(
+      { err: err instanceof Error ? err.message : String(err) },
+      "read_write→write rename failed — coerceStoredGrants maps legacy values at read time, no data loss",
+    ),
+  );
+
   // The picker route — `GET /me/models?surface=...` — reads from the
   // per-provider arrays via `LlmProvidersService` (already constructed
   // upstream as part of `domains/settings/...`). The section-default

ornn-api/src/domains/skills/crud/authorize.test.ts

@@ -161,10 +161,10 @@ describe("canReadSkill (#1123 tiers)", () => {
     expect(canReadSkill(s, actor("stranger"))).toBe(false);
   });
 
-  it("any grant level confers read — read AND read_write grantees can read", () => {
+  it("any grant level confers read — read AND write grantees can read", () => {
     const s = skill("owner", [
       { type: "user", id: "reader", level: "read" },
-      { type: "user", id: "editor", level: "read_write" },
+      { type: "user", id: "editor", level: "write" },
     ]);
     expect(canReadSkill(s, actor("reader"))).toBe(true);
     expect(canReadSkill(s, actor("editor"))).toBe(true);
@@ -189,24 +189,24 @@ describe("canReadSkill (#1123 tiers)", () => {
   });
 });
 
-describe("canWriteSkill (#1123 READ_WRITE tier)", () => {
+describe("canWriteSkill (#1123 WRITE tier)", () => {
   it("author + platform admin may write", () => {
     const s = skill("owner", []);
     expect(canWriteSkill(s, actor("owner"))).toBe(true);
     expect(canWriteSkill(s, actor("x", { admin: true }))).toBe(true);
   });
 
-  it("a read grantee may NOT write; a read_write grantee may", () => {
+  it("a read grantee may NOT write; a write grantee may", () => {
     const s = skill("owner", [
       { type: "user", id: "reader", level: "read" },
-      { type: "user", id: "editor", level: "read_write" },
+      { type: "user", id: "editor", level: "write" },
     ]);
     expect(canWriteSkill(s, actor("reader"))).toBe(false);
     expect(canWriteSkill(s, actor("editor"))).toBe(true);
   });
 
-  it("a read_write org grant lets every member write", () => {
-    const s = skill("owner", [{ type: "org", id: "org-a", level: "read_write" }]);
+  it("a write org grant lets every member write", () => {
+    const s = skill("owner", [{ type: "org", id: "org-a", level: "write" }]);
     expect(canWriteSkill(s, actor("u", { orgs: ["org-a"] }))).toBe(true);
     expect(canWriteSkill(s, actor("u", { orgs: ["org-b"] }))).toBe(false);
   });
@@ -225,8 +225,8 @@ describe("canWriteSkill (#1123 READ_WRITE tier)", () => {
 });
 
 describe("canManageSkill (#1123 ADMIN tier)", () => {
-  it("only author + platform admin administer — a read_write grantee never does", () => {
-    const s = skill("owner", [{ type: "user", id: "editor", level: "read_write" }]);
+  it("only author + platform admin administer — a write grantee never does", () => {
+    const s = skill("owner", [{ type: "user", id: "editor", level: "write" }]);
     expect(canManageSkill(s, actor("owner"))).toBe(true);
     expect(canManageSkill(s, actor("x", { admin: true }))).toBe(true);
     expect(canManageSkill(s, actor("editor"))).toBe(false);

ornn-api/src/domains/skills/crud/authorize.ts

@@ -14,21 +14,21 @@
  *   - PRIVATE skill:
  *     - author (`createdBy === actor.userId`) → yes
  *     - platform admin (`ornn:admin:skill`) → yes
- *     - actor holds ANY grant (read or read_write), directly or via
+ *     - actor holds ANY grant (read or write), directly or via
  *       membership of a granted org → yes
  *     - else → no
  *
- * READ_WRITE — `canWriteSkill` (update content + metadata only):
+ * WRITE — `canWriteSkill` (update content + metadata only):
  *   - author → yes
  *   - platform admin → yes
- *   - actor holds a `read_write` grant, directly or via a granted org → yes
+ *   - actor holds a `write` grant, directly or via a granted org → yes
  *   - else → no
  *
  * ADMIN — `canManageSkill` (change permissions, transfer ownership, delete
  * skill/version, toggle deprecation, manage dist-tags, bind NyxID service):
  *   - author → yes
  *   - platform admin → yes
- *   - else → 403. A `read_write` grantee is deliberately NOT an admin.
+ *   - else → 403. A `write` grantee is deliberately NOT an admin.
  *
  * Org grants resolve uniformly: every admin/member of a granted org inherits
  * the grant's level. The org-membership-based gates fail soft on an
@@ -117,7 +117,7 @@ export async function buildActorContext(
 
 /**
  * Returns true when `actor` is allowed to READ the skill. Any grant level
- * (read or read_write) confers read — a read_write grantee is always also a
+ * (read or write) confers read — a write grantee is always also a
  * reader. Public skills are readable by everyone.
  */
 export function canReadSkill(skill: SkillOwnership, actor: ActorContext): boolean {
@@ -129,22 +129,22 @@ export function canReadSkill(skill: SkillOwnership, actor: ActorContext): boolea
 
 /**
  * Returns true when `actor` may UPDATE the skill's content + metadata — the
- * READ_WRITE tier (#1123). Author + platform admin always qualify; otherwise
- * a `read_write` grant (direct, or via membership of a granted org) is
+ * WRITE tier (#1123). Author + platform admin always qualify; otherwise
+ * a `write` grant (direct, or via membership of a granted org) is
  * required. Deliberately NOT sufficient for admin/danger-zone ops — those
  * gate on `canManageSkill`.
  */
 export function canWriteSkill(skill: SkillOwnership, actor: ActorContext): boolean {
   if (actor.isPlatformAdmin) return true;
   if (skill.createdBy === actor.userId) return true;
-  return actorMatchesGrant(skill, actor, (g) => g.level === "read_write");
+  return actorMatchesGrant(skill, actor, (g) => g.level === "write");
 }
 
 /**
  * Returns true when `actor` may ADMINISTER the skill — change permissions,
  * transfer ownership, delete skill/version, toggle deprecation, manage
  * dist-tags, bind a NyxID service. Author + platform admin only; a
- * read_write grantee can never administer.
+ * write grantee can never administer.
  */
 export function canManageSkill(skill: SkillOwnership, actor: ActorContext): boolean {
   if (actor.isPlatformAdmin) return true;

ornn-api/src/domains/skills/crud/grants.migration.test.ts

@@ -12,7 +12,7 @@
 import { afterAll, beforeAll, beforeEach, describe, expect, it } from "bun:test";
 import { MongoMemoryServer } from "mongodb-memory-server";
 import { MongoClient, type Db } from "mongodb";
-import { backfillTypedGrants } from "./grants.migration";
+import { backfillTypedGrants, renameReadWriteGrantsToWrite } from "./grants.migration";
 
 let mongo: MongoMemoryServer;
 let client: MongoClient;
@@ -92,15 +92,15 @@ describe("backfillTypedGrants", () => {
       isPrivate: true,
       sharedWithUsers: ["u1"],
       sharedWithOrgs: [],
-      grants: [{ type: "user", id: "u1", level: "read_write" }],
+      grants: [{ type: "user", id: "u1", level: "write" }],
     });
 
     const res = await backfillTypedGrants(db);
     expect(res.skillsBackfilled).toBe(0);
 
     const doc = await db.collection("skills").findOne({ _id: "s4" as never });
-    // read_write grant preserved — migration must not clobber a richer ACL.
-    expect(doc?.grants).toEqual([{ type: "user", id: "u1", level: "read_write" }]);
+    // write grant preserved — migration must not clobber a richer ACL.
+    expect(doc?.grants).toEqual([{ type: "user", id: "u1", level: "write" }]);
   });
 
   it("is a no-op on a second run", async () => {
@@ -131,3 +131,57 @@ describe("backfillTypedGrants", () => {
     expect(doc?.grants).toEqual([{ type: "org", id: "o9", level: "read" }]);
   });
 });
+
+describe("renameReadWriteGrantsToWrite (#1127)", () => {
+  it("renames legacy read_write grants to write, leaving read grants + other fields intact", async () => {
+    await db.collection("skills").insertOne({
+      _id: "r1" as never,
+      name: "legacy-rw",
+      isPrivate: true,
+      sharedWithUsers: ["u1", "u2"],
+      sharedWithOrgs: ["o1"],
+      grants: [
+        { type: "user", id: "u1", level: "read_write" },
+        { type: "user", id: "u2", level: "read" },
+        { type: "org", id: "o1", level: "read_write" },
+      ],
+    });
+
+    const res = await renameReadWriteGrantsToWrite(db);
+    expect(res.skillsRenamed).toBe(1);
+
+    const doc = await db.collection("skills").findOne({ _id: "r1" as never });
+    expect(doc?.grants).toEqual([
+      { type: "user", id: "u1", level: "write" },
+      { type: "user", id: "u2", level: "read" },
+      { type: "org", id: "o1", level: "write" },
+    ]);
+    // Non-destructive: legacy lists + privacy untouched.
+    expect(doc?.sharedWithUsers).toEqual(["u1", "u2"]);
+    expect(doc?.isPrivate).toBe(true);
+  });
+
+  it("is idempotent — a second run renames nothing", async () => {
+    await db.collection("skills").insertOne({
+      _id: "r2" as never,
+      name: "rw-once",
+      grants: [{ type: "user", id: "u1", level: "read_write" }],
+    });
+    const first = await renameReadWriteGrantsToWrite(db);
+    expect(first.skillsRenamed).toBe(1);
+    const second = await renameReadWriteGrantsToWrite(db);
+    expect(second.skillsRenamed).toBe(0);
+  });
+
+  it("renames skillsets too", async () => {
+    await db.collection("skillsets").insertOne({
+      _id: "rs1" as never,
+      name: "set-rw",
+      grants: [{ type: "org", id: "o9", level: "read_write" }],
+    });
+    const res = await renameReadWriteGrantsToWrite(db);
+    expect(res.skillsetsRenamed).toBe(1);
+    const doc = await db.collection("skillsets").findOne({ _id: "rs1" as never });
+    expect(doc?.grants).toEqual([{ type: "org", id: "o9", level: "write" }]);
+  });
+});