From 218d2aa144f37089b3ae861ad2f060fa98e3e203 Mon Sep 17 00:00:00 2001 From: Shining <250120269+chronoai-shining@users.noreply.github.com> Date: Tue, 16 Jun 2026 14:42:23 +0800 Subject: [PATCH 1/7] =?UTF-8?q?feat(api):=20rename=20grant=20level=20read?= =?UTF-8?q?=5Fwrite=20=E2=86=92=20write=20(#1127)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../src/domains/skills/crud/authorize.test.ts | 18 +++---- ornn-api/src/domains/skills/crud/authorize.ts | 18 +++---- .../src/domains/skills/crud/grants.test.ts | 50 ++++++++++++------- ornn-api/src/domains/skills/crud/grants.ts | 20 +++++--- .../src/domains/skills/crud/routes.test.ts | 8 +-- ornn-api/src/domains/skills/crud/routes.ts | 12 ++--- .../src/domains/skills/crud/service.test.ts | 22 ++++---- ornn-api/src/domains/skillsets/authorize.ts | 2 +- ornn-api/src/domains/skillsets/service.ts | 2 +- ornn-api/src/domains/skillsets/types.ts | 2 +- ornn-api/src/shared/types/index.ts | 12 ++--- 11 files changed, 91 insertions(+), 75 deletions(-) diff --git a/ornn-api/src/domains/skills/crud/authorize.test.ts b/ornn-api/src/domains/skills/crud/authorize.test.ts index 4ad9c5b1..787f3ba1 100644 --- a/ornn-api/src/domains/skills/crud/authorize.test.ts +++ b/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); diff --git a/ornn-api/src/domains/skills/crud/authorize.ts b/ornn-api/src/domains/skills/crud/authorize.ts index 56cb9488..4d00cd91 100644 --- a/ornn-api/src/domains/skills/crud/authorize.ts +++ b/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; diff --git a/ornn-api/src/domains/skills/crud/grants.test.ts b/ornn-api/src/domains/skills/crud/grants.test.ts index 4f6ec581..db1e7399 100644 --- a/ornn-api/src/domains/skills/crud/grants.test.ts +++ b/ornn-api/src/domains/skills/crud/grants.test.ts @@ -22,7 +22,7 @@ import { describe("effectiveGrants", () => { it("returns the explicit grants array when present", () => { - const grants: SkillGrant[] = [{ type: "user", id: "u1", level: "read_write" }]; + const grants: SkillGrant[] = [{ type: "user", id: "u1", level: "write" }]; expect(effectiveGrants({ grants })).toBe(grants); }); @@ -45,7 +45,7 @@ describe("effectiveGrants", () => { }); describe("deriveGrantsFromLegacy", () => { - it("maps users + orgs to READ grants, never read_write", () => { + it("maps users + orgs to READ grants, never write", () => { const out = deriveGrantsFromLegacy(["u1", "u2"], ["o1"]); expect(out).toEqual([ { type: "user", id: "u1", level: "read" }, @@ -66,8 +66,8 @@ describe("legacyListsFromGrants (dual-write projection)", () => { it("places every grant id in its legacy list regardless of level (read visibility, no escalation)", () => { const grants: SkillGrant[] = [ { type: "user", id: "u1", level: "read" }, - { type: "user", id: "u2", level: "read_write" }, - { type: "org", id: "o1", level: "read_write" }, + { type: "user", id: "u2", level: "write" }, + { type: "org", id: "o1", level: "write" }, ]; expect(legacyListsFromGrants(grants)).toEqual({ sharedWithUsers: ["u1", "u2"], @@ -83,29 +83,29 @@ describe("legacyListsFromGrants (dual-write projection)", () => { }); describe("normalizeGrants", () => { - it("collapses duplicate (type,id) keeping the highest level (read_write wins)", () => { + it("collapses duplicate (type,id) keeping the highest level (write wins)", () => { expect( normalizeGrants([ { type: "user", id: "u1", level: "read" }, - { type: "user", id: "u1", level: "read_write" }, + { type: "user", id: "u1", level: "write" }, ]), - ).toEqual([{ type: "user", id: "u1", level: "read_write" }]); + ).toEqual([{ type: "user", id: "u1", level: "write" }]); }); - it("does not let a later read downgrade an earlier read_write", () => { + it("does not let a later read downgrade an earlier write", () => { expect( normalizeGrants([ - { type: "org", id: "o1", level: "read_write" }, + { type: "org", id: "o1", level: "write" }, { type: "org", id: "o1", level: "read" }, ]), - ).toEqual([{ type: "org", id: "o1", level: "read_write" }]); + ).toEqual([{ type: "org", id: "o1", level: "write" }]); }); it("trims ids and drops empties, preserving first-appearance order", () => { expect( normalizeGrants([ { type: "user", id: " u2 ", level: "read" }, - { type: "user", id: "", level: "read_write" }, + { type: "user", id: "", level: "write" }, { type: "user", id: "u1", level: "read" }, ]), ).toEqual([ @@ -118,18 +118,18 @@ describe("normalizeGrants", () => { expect( normalizeGrants([ { type: "user", id: "x", level: "read" }, - { type: "org", id: "x", level: "read_write" }, + { type: "org", id: "x", level: "write" }, ]), ).toEqual([ { type: "user", id: "x", level: "read" }, - { type: "org", id: "x", level: "read_write" }, + { type: "org", id: "x", level: "write" }, ]); }); }); describe("levelAllowsWrite", () => { - it("only read_write permits writing", () => { - expect(levelAllowsWrite("read_write")).toBe(true); + it("only write permits writing", () => { + expect(levelAllowsWrite("write")).toBe(true); expect(levelAllowsWrite("read")).toBe(false); }); }); @@ -145,11 +145,11 @@ describe("coerceStoredGrants", () => { expect( coerceStoredGrants([ { type: "user", id: " u1 ", level: "read" }, - { type: "org", id: "o1", level: "read_write" }, + { type: "org", id: "o1", level: "write" }, ]), ).toEqual([ { type: "user", id: "u1", level: "read" }, - { type: "org", id: "o1", level: "read_write" }, + { type: "org", id: "o1", level: "write" }, ]); }); @@ -161,12 +161,24 @@ describe("coerceStoredGrants", () => { { type: "user", id: "u1", level: "admin" }, 42, null, - { type: "user", id: "u2", level: "read_write" }, + { type: "user", id: "u2", level: "write" }, ]), - ).toEqual([{ type: "user", id: "u2", level: "read_write" }]); + ).toEqual([{ type: "user", id: "u2", level: "write" }]); }); it("returns an empty array for an empty stored array (explicit no-grants)", () => { expect(coerceStoredGrants([])).toEqual([]); }); + + it("coerces a stored legacy read_write level → write (#1127, no drop)", () => { + expect( + coerceStoredGrants([ + { type: "user", id: "u1", level: "read_write" }, + { type: "org", id: "o1", level: "read" }, + ]), + ).toEqual([ + { type: "user", id: "u1", level: "write" }, + { type: "org", id: "o1", level: "read" }, + ]); + }); }); diff --git a/ornn-api/src/domains/skills/crud/grants.ts b/ornn-api/src/domains/skills/crud/grants.ts index 4743853e..3bcb66d2 100644 --- a/ornn-api/src/domains/skills/crud/grants.ts +++ b/ornn-api/src/domains/skills/crud/grants.ts @@ -31,7 +31,7 @@ import type { export const skillGrantSchema = z.object({ type: z.enum(["user", "org"]), id: z.string().min(1).max(128), - level: z.enum(["read", "read_write"]), + level: z.enum(["read", "write"]), }); /** @@ -88,7 +88,7 @@ export function effectiveGrants(src: GrantSource): SkillGrant[] { /** * Map the legacy read-only allow-lists onto READ-level typed grants. Used by * the boot migration and by the read-time fallback. Never produces a - * `read_write` grant — legacy sharing was read-only, so migrating it must + * `write` grant — legacy sharing was read-only, so migrating it must * not escalate anyone. */ export function deriveGrantsFromLegacy( @@ -106,7 +106,7 @@ export function deriveGrantsFromLegacy( * dual-write (#1123). EVERY grant id — regardless of level — lands in the * matching legacy list so code that only understands the read lists (an * older pod mid-rolling-deploy) still resolves correct READ visibility for - * read_write grantees, and never escalates anyone to write (old code has no + * write grantees, and never escalates anyone to write (old code has no * write concept). Dropped by the post-rollout cleanup once all pods read * `grants`. */ @@ -122,7 +122,7 @@ export function legacyListsFromGrants(grants: readonly SkillGrant[]): LegacyShar /** * Normalize an incoming grants list: trim ids, drop empties, and collapse - * duplicate `(type, id)` pairs keeping the HIGHEST level (`read_write` wins + * duplicate `(type, id)` pairs keeping the HIGHEST level (`write` wins * over `read`) so a principal can never hold two conflicting grants on the * same skill. Order is preserved by first appearance. */ @@ -136,7 +136,7 @@ export function normalizeGrants(grants: readonly SkillGrant[]): SkillGrant[] { const prev = byKey.get(key); if (!prev) order.push(key); const level: SkillPermissionLevel = - prev?.level === "read_write" || g.level === "read_write" ? "read_write" : "read"; + prev?.level === "write" || g.level === "write" ? "write" : "read"; byKey.set(key, { type: g.type, id, level }); } return order.map((k) => byKey.get(k)!); @@ -144,7 +144,7 @@ export function normalizeGrants(grants: readonly SkillGrant[]): SkillGrant[] { /** True when `level` permits writing (updating content / metadata). */ export function levelAllowsWrite(level: SkillPermissionLevel): boolean { - return level === "read_write"; + return level === "write"; } /** @@ -162,9 +162,13 @@ export function coerceStoredGrants(raw: unknown): SkillGrant[] | undefined { const e = entry as Record; const type = e.type; const id = typeof e.id === "string" ? e.id.trim() : ""; - const level = e.level; + // #1127 renamed the level `read_write` → `write`. Defensively coerce a + // stored legacy `read_write` here 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. + const level = e.level === "read_write" ? "write" : e.level; if ((type !== "user" && type !== "org") || !id) continue; - if (level !== "read" && level !== "read_write") continue; + if (level !== "read" && level !== "write") continue; out.push({ type, id, level }); } return out; diff --git a/ornn-api/src/domains/skills/crud/routes.test.ts b/ornn-api/src/domains/skills/crud/routes.test.ts index 8774c9fa..2f2b62e0 100644 --- a/ornn-api/src/domains/skills/crud/routes.test.ts +++ b/ornn-api/src/domains/skills/crud/routes.test.ts @@ -916,7 +916,7 @@ describe("PUT /skills/:id", () => { expect(calls).toEqual(["updateSkill"]); }); - test("200 ZIP republish for a read_write grantee — content edit is the write tier (#1123)", async () => { + test("200 ZIP republish for a write grantee — content edit is the write tier (#1123)", async () => { const calls: string[] = []; const app = buildApp({ userId: "editor", @@ -926,7 +926,7 @@ describe("PUT /skills/:id", () => { skillDoc({ createdBy: OWNER, isPrivate: true, - grants: [{ type: "user", id: "editor", level: "read_write" }], + grants: [{ type: "user", id: "editor", level: "write" }], }), }, service: { @@ -945,7 +945,7 @@ describe("PUT /skills/:id", () => { expect(calls).toEqual(["updateSkill"]); }); - test("403 when a read_write grantee tries to flip visibility — that is admin-only (#1123)", async () => { + test("403 when a write grantee tries to flip visibility — that is admin-only (#1123)", async () => { const app = buildApp({ userId: "editor", permissions: [UPDATE], @@ -954,7 +954,7 @@ describe("PUT /skills/:id", () => { skillDoc({ createdBy: OWNER, isPrivate: true, - grants: [{ type: "user", id: "editor", level: "read_write" }], + grants: [{ type: "user", id: "editor", level: "write" }], }), }, }); diff --git a/ornn-api/src/domains/skills/crud/routes.ts b/ornn-api/src/domains/skills/crud/routes.ts index 517991d2..4ae6c26f 100644 --- a/ornn-api/src/domains/skills/crud/routes.ts +++ b/ornn-api/src/domains/skills/crud/routes.ts @@ -999,8 +999,8 @@ export function createSkillRoutes(config: SkillRoutesConfig): Hono<{ Variables: throw AppError.notFound("skill_not_found", `Skill '${guid}' not found`); } const actor = await buildActorContext(c); - // Updating content + metadata is the READ_WRITE tier (#1123): author, - // platform admin, or a read_write grantee. Changing `isPrivate` is a + // Updating content + metadata is the WRITE tier (#1123): author, + // platform admin, or a write grantee. Changing `isPrivate` is a // permission change (ADMIN tier) — gated separately below once parsed. if (!canWriteSkill(existing, actor)) { throw AppError.forbidden( @@ -1065,7 +1065,7 @@ export function createSkillRoutes(config: SkillRoutesConfig): Hono<{ Variables: throw AppError.badRequest("no_update", "No update data provided. Send a ZIP file and/or isPrivate field."); } - // Visibility is an ADMIN-tier change (#1123): a read_write grantee may + // Visibility is an ADMIN-tier change (#1123): a write grantee may // replace content but must not flip public/private. Enforce only when // `isPrivate` is actually being changed so a no-op resend by an editor // doesn't 403. @@ -1160,7 +1160,7 @@ export function createSkillRoutes(config: SkillRoutesConfig): Hono<{ Variables: const updated = await skillService.getSkill(guid); - const readWriteGrants = (updated.grants ?? []).filter((g) => g.level === "read_write").length; + const writeGrants = (updated.grants ?? []).filter((g) => g.level === "write").length; trackActivity(authCtx.userId, authCtx.email, authCtx.displayName, "skill.permissions_changed", { skillId: guid, skillName: updated.name, @@ -1168,7 +1168,7 @@ export function createSkillRoutes(config: SkillRoutesConfig): Hono<{ Variables: sharedWithUsers: updated.sharedWithUsers.length, sharedWithOrgs: updated.sharedWithOrgs.length, // #1123 — richer payload: how many grants confer write. - readWriteGrants, + writeGrants, }); // Permissions change can flip eligibility — sync handles both @@ -1204,7 +1204,7 @@ export function createSkillRoutes(config: SkillRoutesConfig): Hono<{ Variables: throw AppError.notFound("skill_not_found", `Skill '${guid}' not found`); } const actor = await buildActorContext(c); - // ADMIN tier — transfer is a danger-zone op, never a read_write grant. + // ADMIN tier — transfer is a danger-zone op, never a write grant. if (!canManageSkill(existing, actor)) { throw AppError.forbidden( "forbidden", diff --git a/ornn-api/src/domains/skills/crud/service.test.ts b/ornn-api/src/domains/skills/crud/service.test.ts index 59de2068..ef3cf420 100644 --- a/ornn-api/src/domains/skills/crud/service.test.ts +++ b/ornn-api/src/domains/skills/crud/service.test.ts @@ -478,7 +478,7 @@ describe("SkillService.setSkillPermissions — org-membership gate (#815)", () = expect(state.updateCalled).toBe(false); }); - it("(e) accepts typed grants directly, including read_write (#1123)", async () => { + it("(e) accepts typed grants directly, including write (#1123)", async () => { const { service, state } = makePermissionsService( makeSkillDoc({ guid: "guid-1", createdBy: "owner-1", isPrivate: true }), ); @@ -494,18 +494,18 @@ describe("SkillService.setSkillPermissions — org-membership gate (#815)", () = { isPrivate: true, grants: [ - { type: "user", id: "editor", level: "read_write" }, - { type: "org", id: "org-A", level: "read_write" }, + { type: "user", id: "editor", level: "write" }, + { type: "org", id: "org-A", level: "write" }, ], }, actor, ); expect(state.updateCalled).toBe(true); - expect(result.grants).toContainEqual({ type: "user", id: "editor", level: "read_write" }); - expect(result.grants).toContainEqual({ type: "org", id: "org-A", level: "read_write" }); + expect(result.grants).toContainEqual({ type: "user", id: "editor", level: "write" }); + expect(result.grants).toContainEqual({ type: "org", id: "org-A", level: "write" }); }); - it("(f) a read_write org grant still honours the #815 non-member gate (#1123)", async () => { + it("(f) a write org grant still honours the #815 non-member gate (#1123)", async () => { const { service, state } = makePermissionsService( makeSkillDoc({ guid: "guid-1", createdBy: "owner-1", isPrivate: true }), ); @@ -520,7 +520,7 @@ describe("SkillService.setSkillPermissions — org-membership gate (#815)", () = await service.setSkillPermissions( "guid-1", "owner-1", - { isPrivate: true, grants: [{ type: "org", id: "org-Z", level: "read_write" }] }, + { isPrivate: true, grants: [{ type: "org", id: "org-Z", level: "write" }] }, actor, ); } catch (err) { @@ -543,7 +543,7 @@ describe("SkillService.setSkillPermissions — org-membership gate (#815)", () = const result = await service.setSkillPermissions( "guid-1", "owner-1", - { isPrivate: true, grants: [{ type: "user", id: "owner-1", level: "read_write" }] }, + { isPrivate: true, grants: [{ type: "user", id: "owner-1", level: "write" }] }, actor, ); // The author holds implicit ADMIN — a self-grant is redundant and dropped. @@ -604,7 +604,7 @@ describe("SkillService.transferSkillOwnership (#1123)", () => { guid: "guid-1", createdBy: "owner-1", isPrivate: true, - grants: [{ type: "org", id: "org-A", level: "read_write" }], + grants: [{ type: "org", id: "org-A", level: "write" }], }), ); const result = await service.transferSkillOwnership( @@ -616,7 +616,7 @@ describe("SkillService.transferSkillOwnership (#1123)", () => { expect(result.createdBy).toBe("newbie"); expect(result.createdByEmail).toBe("new@x.io"); // Existing org grant preserved; prior owner added as read. - expect(result.grants).toContainEqual({ type: "org", id: "org-A", level: "read_write" }); + expect(result.grants).toContainEqual({ type: "org", id: "org-A", level: "write" }); expect(result.grants).toContainEqual({ type: "user", id: "owner-1", level: "read" }); }); @@ -626,7 +626,7 @@ describe("SkillService.transferSkillOwnership (#1123)", () => { guid: "guid-1", createdBy: "owner-1", isPrivate: true, - grants: [{ type: "user", id: "newbie", level: "read_write" }], + grants: [{ type: "user", id: "newbie", level: "write" }], }), ); const result = await service.transferSkillOwnership( diff --git a/ornn-api/src/domains/skillsets/authorize.ts b/ornn-api/src/domains/skillsets/authorize.ts index 0e9d4c09..d4333c82 100644 --- a/ornn-api/src/domains/skillsets/authorize.ts +++ b/ornn-api/src/domains/skillsets/authorize.ts @@ -38,7 +38,7 @@ export function canReadSkillset( /** * True when `actor` may UPDATE the skillset's content/metadata (publish a new - * version) — the READ_WRITE tier (#1123). Delegates to the skill gate. + * version) — the WRITE tier (#1123). Delegates to the skill gate. */ export function canWriteSkillset( skillset: SkillsetOwnership, diff --git a/ornn-api/src/domains/skillsets/service.ts b/ornn-api/src/domains/skillsets/service.ts index 23ec65f7..9e9255b3 100644 --- a/ornn-api/src/domains/skillsets/service.ts +++ b/ornn-api/src/domains/skillsets/service.ts @@ -177,7 +177,7 @@ export class SkillsetService { if (!existing) { throw AppError.notFound("skillset_not_found", `Skillset '${guid}' not found`); } - // Publishing a new version is a content/metadata edit — the READ_WRITE + // Publishing a new version is a content/metadata edit — the WRITE // tier (#1123). Permissions/transfer/delete remain ADMIN-only below. if (!canWriteSkill(existing, actor)) { throw AppError.forbidden("forbidden", "You do not have permission to update this skillset"); diff --git a/ornn-api/src/domains/skillsets/types.ts b/ornn-api/src/domains/skillsets/types.ts index 3075c11d..f2d9b0d0 100644 --- a/ornn-api/src/domains/skillsets/types.ts +++ b/ornn-api/src/domains/skillsets/types.ts @@ -211,7 +211,7 @@ export interface SkillsetDocument { /** Explicit per-org grants (NyxID org user_ids). */ sharedWithOrgs: string[]; /** - * Typed access grants (#1123) — canonical read/read-write ACL, mirroring + * Typed access grants (#1123) — canonical read/write ACL, mirroring * `SkillDocument.grants`. Optional for back-compat; readers fall back to * deriving read grants from the legacy lists via `effectiveGrants`. */ diff --git a/ornn-api/src/shared/types/index.ts b/ornn-api/src/shared/types/index.ts index 98f8093e..2943fba5 100644 --- a/ornn-api/src/shared/types/index.ts +++ b/ornn-api/src/shared/types/index.ts @@ -74,13 +74,13 @@ export class AppError extends Error { /** * Permission level a grant confers on a skill / skillset (#1123). * - `read` → view / pull / execute / read versions. - * - `read_write` → READ plus update the skill's content & metadata. Does + * - `write` → READ plus update the skill's content & metadata. Does * NOT include any admin / danger-zone operation (change permissions, * transfer ownership, delete skill/version, toggle deprecation, manage * dist-tags, bind NyxID service) — those stay with the owner (`createdBy`) * and platform admins only. */ -export type SkillPermissionLevel = "read" | "read_write"; +export type SkillPermissionLevel = "read" | "write"; /** The kind of principal a grant targets. */ export type SkillGrantPrincipalType = "user" | "org"; @@ -92,11 +92,11 @@ export type SkillGrantPrincipalType = "user" | "org"; * * - `type` — `user` (NyxID person user_id) or `org` (NyxID org user_id). * - `id` — the principal's NyxID id. - * - `level`— `read` or `read_write`. + * - `level`— `read` or `write`. * * The author (`createdBy`) is never represented here — they hold implicit * ADMIN. Public skills (`isPrivate === false`) are readable by everyone - * regardless of grants; `read_write` only ever applies to explicit grants. + * regardless of grants; `write` only ever applies to explicit grants. */ export interface SkillGrant { type: SkillGrantPrincipalType; @@ -148,7 +148,7 @@ export interface SkillDocument { sharedWithOrgs: string[]; /** * Typed access grants (#1123) — the canonical source of truth for who can - * read vs. read-write the skill, beyond the author + platform admin. + * read vs. write the skill, beyond the author + platform admin. * * Optional for back-compat: skills created before #1123 carry only the * legacy `sharedWithUsers` / `sharedWithOrgs` read lists. Readers MUST fall @@ -373,7 +373,7 @@ export interface SkillDetailResponse { /** Org user_ids granted access — every admin/member of these orgs can read the skill. */ sharedWithOrgs: string[]; /** - * Typed access grants (#1123) — the canonical read/read-write ACL. Always + * Typed access grants (#1123) — the canonical read/write ACL. Always * present in responses (the service emits `effectiveGrants(skill)` so even * an un-migrated skill surfaces its legacy lists as read grants here). */ From 1e90b4e7999db97e89387a20ca28ac966e57ea82 Mon Sep 17 00:00:00 2001 From: Shining <250120269+chronoai-shining@users.noreply.github.com> Date: Tue, 16 Jun 2026 14:42:33 +0800 Subject: [PATCH 2/7] =?UTF-8?q?feat(api):=20boot=20migration=20renaming=20?= =?UTF-8?q?read=5Fwrite=20grants=20=E2=86=92=20write=20(#1127)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- ornn-api/src/bootstrap.ts | 14 ++++- .../skills/crud/grants.migration.test.ts | 62 +++++++++++++++++-- .../domains/skills/crud/grants.migration.ts | 54 ++++++++++++++++ 3 files changed, 125 insertions(+), 5 deletions(-) diff --git a/ornn-api/src/bootstrap.ts b/ornn-api/src/bootstrap.ts index 59f6b9eb..034a26ce 100644 --- a/ornn-api/src/bootstrap.ts +++ b/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 diff --git a/ornn-api/src/domains/skills/crud/grants.migration.test.ts b/ornn-api/src/domains/skills/crud/grants.migration.test.ts index 3657edee..c1efe1d7 100644 --- a/ornn-api/src/domains/skills/crud/grants.migration.test.ts +++ b/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" }]); + }); +}); diff --git a/ornn-api/src/domains/skills/crud/grants.migration.ts b/ornn-api/src/domains/skills/crud/grants.migration.ts index 37c4292b..52d820f2 100644 --- a/ornn-api/src/domains/skills/crud/grants.migration.ts +++ b/ornn-api/src/domains/skills/crud/grants.migration.ts @@ -81,3 +81,57 @@ export async function backfillTypedGrants(db: Db): Promise { + const counts: Record = {}; + for (const coll of GRANTED_COLLECTIONS) { + const res = await db + .collection(coll) + .updateMany({ "grants.level": "read_write" }, [RENAME_LEVEL_STAGE]); + counts[coll] = res.modifiedCount; + } + const result: LevelRenameResult = { + skillsRenamed: counts.skills ?? 0, + skillsetsRenamed: counts.skillsets ?? 0, + }; + if (result.skillsRenamed > 0 || result.skillsetsRenamed > 0) { + logger.info({ ...result }, "read_write → write grant-level rename complete (#1127)"); + } + return result; +} From ea07a3e1c39675cb645a70bb336c3501c3393b1f Mon Sep 17 00:00:00 2001 From: Shining <250120269+chronoai-shining@users.noreply.github.com> Date: Tue, 16 Jun 2026 14:42:33 +0800 Subject: [PATCH 3/7] =?UTF-8?q?feat(sdk):=20rename=20grant=20level=20read?= =?UTF-8?q?=5Fwrite=20=E2=86=92=20write=20(TS=20+=20Python)=20(#1127)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 --- sdk/python/src/ornn_sdk/client.py | 4 ++-- sdk/python/src/ornn_sdk/types.py | 4 ++-- sdk/python/tests/test_client.py | 12 ++++++------ sdk/typescript/src/client.ts | 4 ++-- sdk/typescript/src/tests/client.test.ts | 10 +++++----- sdk/typescript/src/types.ts | 4 ++-- 6 files changed, 19 insertions(+), 19 deletions(-) diff --git a/sdk/python/src/ornn_sdk/client.py b/sdk/python/src/ornn_sdk/client.py index 126fe81b..a0c3f223 100644 --- a/sdk/python/src/ornn_sdk/client.py +++ b/sdk/python/src/ornn_sdk/client.py @@ -252,7 +252,7 @@ def set_skill_permissions( """Apply a new ACL state to a skill (#1123). ``grants`` is the canonical typed ACL — each entry confers ``read`` - or ``read_write`` to one user or org. The legacy ``shared_with_*`` + or ``write`` to one user or org. The legacy ``shared_with_*`` lists are still accepted (the server maps each entry to a ``read``-level grant) but are superseded by ``grants``; pass one or the other, not both. ``is_private=False`` makes the skill fully @@ -372,7 +372,7 @@ def set_skillset_permissions( """Update a skillset's visibility / ACL (#1123). ``grants`` is the canonical typed ACL — each entry confers ``read`` - or ``read_write`` to one user or org. The legacy ``shared_with_*`` + or ``write`` to one user or org. The legacy ``shared_with_*`` lists are still accepted (the server maps each entry to a ``read``-level grant) but are superseded by ``grants``; pass one or the other, not both. diff --git a/sdk/python/src/ornn_sdk/types.py b/sdk/python/src/ornn_sdk/types.py index bff31eef..65c0d4d9 100644 --- a/sdk/python/src/ornn_sdk/types.py +++ b/sdk/python/src/ornn_sdk/types.py @@ -21,8 +21,8 @@ SkillsetKind = Literal["generic", "consensus-supported"] # Access level a grant confers (#1123). `read` allows pull/execute; -# `read_write` additionally allows publishing new versions / editing. -SkillPermissionLevel = Literal["read", "read_write"] +# `write` additionally allows publishing new versions / editing (and implies read). +SkillPermissionLevel = Literal["read", "write"] GrantTarget = Literal["user", "org"] diff --git a/sdk/python/tests/test_client.py b/sdk/python/tests/test_client.py index 019bb557..1d3c5bc5 100644 --- a/sdk/python/tests/test_client.py +++ b/sdk/python/tests/test_client.py @@ -733,7 +733,7 @@ def test_set_skill_permissions_sends_grants_and_unwraps_skill(self) -> None: "data": { "skill": _skill_data( isPrivate=True, - grants=[{"type": "user", "id": "bob", "level": "read_write"}], + grants=[{"type": "user", "id": "bob", "level": "write"}], ) }, "error": None, @@ -743,15 +743,15 @@ def test_set_skill_permissions_sends_grants_and_unwraps_skill(self) -> None: result = ornn.set_skill_permissions( "sk-1", is_private=True, - grants=[SkillGrant(type="user", id="bob", level="read_write")], + grants=[SkillGrant(type="user", id="bob", level="write")], ) # The typed grant rides on the wire in canonical shape. sent = _json.loads(route.calls.last.request.content) assert sent["isPrivate"] is True - assert sent["grants"] == [{"type": "user", "id": "bob", "level": "read_write"}] + assert sent["grants"] == [{"type": "user", "id": "bob", "level": "write"}] # Response `{ skill }` envelope is unwrapped + grants parse back. assert isinstance(result, SkillDetail) - assert result.grants == [SkillGrant(type="user", id="bob", level="read_write")] + assert result.grants == [SkillGrant(type="user", id="bob", level="write")] assert route.called @respx.mock @@ -800,7 +800,7 @@ def test_skill_detail_parses_grants(self) -> None: "data": _skill_data( grants=[ {"type": "user", "id": "u1", "level": "read"}, - {"type": "org", "id": "o1", "level": "read_write"}, + {"type": "org", "id": "o1", "level": "write"}, ] ), "error": None, @@ -810,7 +810,7 @@ def test_skill_detail_parses_grants(self) -> None: detail = ornn.get("sk-1") assert detail.grants == [ SkillGrant(type="user", id="u1", level="read"), - SkillGrant(type="org", id="o1", level="read_write"), + SkillGrant(type="org", id="o1", level="write"), ] @respx.mock diff --git a/sdk/typescript/src/client.ts b/sdk/typescript/src/client.ts index c57ee71b..115a0db7 100644 --- a/sdk/typescript/src/client.ts +++ b/sdk/typescript/src/client.ts @@ -274,7 +274,7 @@ export class OrnnClient { /** * Update a skill's visibility / sharing ACL (#1123). `grants` is the * canonical typed ACL — each entry grants a user or org a `read` or - * `read_write` level. The legacy `sharedWith*` arrays are still accepted + * `write` level. The legacy `sharedWith*` arrays are still accepted * (mapped to `read`-level grants) for backward compat. Returns the * refreshed skill detail. */ @@ -345,7 +345,7 @@ export class OrnnClient { /** * Update a skillset's visibility / sharing ACL (#1123). `grants` is the * canonical typed ACL — each entry grants a user or org a `read` or - * `read_write` level. The legacy `sharedWith*` arrays are still accepted + * `write` level. The legacy `sharedWith*` arrays are still accepted * (mapped to `read`-level grants) for backward compat. */ async setSkillsetPermissions( diff --git a/sdk/typescript/src/tests/client.test.ts b/sdk/typescript/src/tests/client.test.ts index 861c2f6e..a544a402 100644 --- a/sdk/typescript/src/tests/client.test.ts +++ b/sdk/typescript/src/tests/client.test.ts @@ -381,7 +381,7 @@ describe("OrnnClient", () => { skill: { id: "sk-1", isPrivate: false, - grants: [{ type: "user", id: "u-2", level: "read_write" }], + grants: [{ type: "user", id: "u-2", level: "write" }], }, }, error: null, @@ -390,7 +390,7 @@ describe("OrnnClient", () => { const client = new OrnnClient({ baseUrl: "https://x", fetch: fetchMock }); const res = await client.setSkillPermissions("sk-1", { isPrivate: false, - grants: [{ type: "user", id: "u-2", level: "read_write" }], + grants: [{ type: "user", id: "u-2", level: "write" }], }); expect(captured.method).toBe("PUT"); expect(captured.url).toBe("https://x/api/v1/skills/sk-1/permissions"); @@ -398,10 +398,10 @@ describe("OrnnClient", () => { // The typed grant shape (#1123) is sent verbatim on the wire. expect(JSON.parse(captured.body)).toEqual({ isPrivate: false, - grants: [{ type: "user", id: "u-2", level: "read_write" }], + grants: [{ type: "user", id: "u-2", level: "write" }], }); expect(res.id).toBe("sk-1"); - expect(res.grants).toEqual([{ type: "user", id: "u-2", level: "read_write" }]); + expect(res.grants).toEqual([{ type: "user", id: "u-2", level: "write" }]); }); test("setSkillPermissions(): throws OrnnError on validation failure (400)", async () => { @@ -409,7 +409,7 @@ describe("OrnnClient", () => { jsonResponse(400, { status: 400, code: "validation_error", - detail: "grants[0].level must be read|read_write", + detail: "grants[0].level must be read|write", }), ); const client = new OrnnClient({ baseUrl: "https://x", fetch: fetchMock }); diff --git a/sdk/typescript/src/types.ts b/sdk/typescript/src/types.ts index 0f0394bf..d4b9f4a0 100644 --- a/sdk/typescript/src/types.ts +++ b/sdk/typescript/src/types.ts @@ -13,9 +13,9 @@ export type SearchScope = "public" | "private" | "mine" | "mixed" | "shared-with /** * Access level a grant confers (#1123). `read` allows pull/execute; - * `read_write` additionally allows publishing new versions / editing. + * `write` additionally allows publishing new versions / editing. */ -export type SkillPermissionLevel = "read" | "read_write"; +export type SkillPermissionLevel = "read" | "write"; /** * One typed ACL entry (#1123) — the canonical sharing primitive for From 96a42dc32731bf22883ae159ff590fe478310efd Mon Sep 17 00:00:00 2001 From: Shining <250120269+chronoai-shining@users.noreply.github.com> Date: Tue, 16 Jun 2026 14:42:48 +0800 Subject: [PATCH 4/7] =?UTF-8?q?feat(web):=20rename=20grant=20level=20read?= =?UTF-8?q?=5Fwrite=20=E2=86=92=20write=20(#1127)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 --- .../src/components/permissions/PermissionsEditor.tsx | 8 ++++---- .../src/components/skill/PermissionsModal.test.tsx | 8 ++++---- ornn-web/src/components/skill/SkillVisibilityCard.tsx | 10 +++++----- .../components/skillset/SkillsetPermissionsModal.tsx | 2 +- ornn-web/src/types/domain.ts | 6 +++--- 5 files changed, 17 insertions(+), 17 deletions(-) diff --git a/ornn-web/src/components/permissions/PermissionsEditor.tsx b/ornn-web/src/components/permissions/PermissionsEditor.tsx index eb847cb8..947d9e53 100644 --- a/ornn-web/src/components/permissions/PermissionsEditor.tsx +++ b/ornn-web/src/components/permissions/PermissionsEditor.tsx @@ -44,14 +44,14 @@ const key = (p: { type: string; id: string }) => `${p.type}:${p.id}`; /** * Compose the canonical grants from the two audiences. When public, read * grants are redundant (everyone reads) so only write grants are emitted. - * A principal in both audiences resolves to read_write (write wins). + * A principal in both audiences resolves to write (write wins). */ function buildGrants(read: Principal[], write: Principal[], isPublic: boolean): SkillGrant[] { const map = new Map(); if (!isPublic) { for (const p of read) map.set(key(p), { type: p.type, id: p.id, level: "read" }); } - for (const p of write) map.set(key(p), { type: p.type, id: p.id, level: "read_write" }); + for (const p of write) map.set(key(p), { type: p.type, id: p.id, level: "write" }); return [...map.values()]; } @@ -78,7 +78,7 @@ export function PermissionsEditor({ const [isPublic, setIsPublic] = useState(!initialIsPrivate); const [readGrantees, setReadGrantees] = useState(() => toPrincipals(initialGrants, "read")); const [writeGrantees, setWriteGrantees] = useState(() => - toPrincipals(initialGrants, "read_write"), + toPrincipals(initialGrants, "write"), ); // Resolve user-grant labels once on mount (the typeahead supplies labels @@ -150,7 +150,7 @@ export function PermissionsEditor({ const grants = buildGrants(readGrantees, writeGrantees, isPublic); const initialBuilt = buildGrants( toPrincipals(initialGrants, "read"), - toPrincipals(initialGrants, "read_write"), + toPrincipals(initialGrants, "write"), !initialIsPrivate, ); if (isPrivate === initialIsPrivate && signature(grants) === signature(initialBuilt)) { diff --git a/ornn-web/src/components/skill/PermissionsModal.test.tsx b/ornn-web/src/components/skill/PermissionsModal.test.tsx index 6b8a173c..96ad3e86 100644 --- a/ornn-web/src/components/skill/PermissionsModal.test.tsx +++ b/ornn-web/src/components/skill/PermissionsModal.test.tsx @@ -107,17 +107,17 @@ describe("PermissionsModal — two-tab Read/Write editor (#1125)", () => { fireEvent.click(screen.getByRole("button", { name: /write access/i })); fireEvent.click(screen.getByRole("checkbox", { name: "Org One" })); - // Save → public (isPrivate:false) + a read_write org grant; no read grant + // Save → public (isPrivate:false) + a write org grant; no read grant // (public makes read grants redundant, so they're dropped). fireEvent.click(screen.getByRole("button", { name: /^save$/i })); expect(mutateAsync).toHaveBeenCalledWith({ isPrivate: false, - grants: [{ type: "org", id: "org-1", level: "read_write" }], + grants: [{ type: "org", id: "org-1", level: "write" }], }); }); - it("seeds the Write tab from an existing read_write grant", () => { - renderModal(skill({ grants: [{ type: "org", id: "org-1", level: "read_write" }] })); + it("seeds the Write tab from an existing write grant", () => { + renderModal(skill({ grants: [{ type: "org", id: "org-1", level: "write" }] })); // The Write tab shows a count badge of 1. const writeTab = screen.getByRole("button", { name: /write access/i }); expect(writeTab.textContent).toContain("1"); diff --git a/ornn-web/src/components/skill/SkillVisibilityCard.tsx b/ornn-web/src/components/skill/SkillVisibilityCard.tsx index 3731d492..9bd2cab4 100644 --- a/ornn-web/src/components/skill/SkillVisibilityCard.tsx +++ b/ornn-web/src/components/skill/SkillVisibilityCard.tsx @@ -24,8 +24,8 @@ export interface SkillVisibilityCardProps { isPrivate: boolean; sharedWithUsersCount: number; sharedWithOrgsCount: number; - /** How many grants confer read-write (#1123). Shown as a "can edit" line. */ - readWriteCount?: number; + /** How many grants confer write (#1123). Shown as a "can edit" line. */ + writeCount?: number; isOwner: boolean; onManagePermissions: () => void; } @@ -34,7 +34,7 @@ export function SkillVisibilityCard({ isPrivate, sharedWithUsersCount, sharedWithOrgsCount, - readWriteCount = 0, + writeCount = 0, isOwner, onManagePermissions, }: SkillVisibilityCardProps) { @@ -100,10 +100,10 @@ export function SkillVisibilityCard({ {t("skillDetail.shareOrgs", "organizations")} - {readWriteCount > 0 && ( + {writeCount > 0 && (
  • - {readWriteCount} + {writeCount} {t("skillDetail.shareCanEdit", "can edit")}
    • diff --git a/ornn-web/src/components/skillset/SkillsetPermissionsModal.tsx b/ornn-web/src/components/skillset/SkillsetPermissionsModal.tsx index 862dedd0..f06a5a44 100644 --- a/ornn-web/src/components/skillset/SkillsetPermissionsModal.tsx +++ b/ornn-web/src/components/skillset/SkillsetPermissionsModal.tsx @@ -4,7 +4,7 @@ * Thin wrapper: the Modal shell + the skillset permissions mutation, around * the shared two-tab `PermissionsEditor`. Kept in lock-step with the skill * `PermissionsModal` (they now share the editor + selector). This also brings - * skillsets the read/read-write level support they never had under the old + * skillsets the read/write level support they never had under the old * single-ladder UI. * * @module components/skillset/SkillsetPermissionsModal diff --git a/ornn-web/src/types/domain.ts b/ornn-web/src/types/domain.ts index 4819702d..7f879773 100644 --- a/ornn-web/src/types/domain.ts +++ b/ornn-web/src/types/domain.ts @@ -42,10 +42,10 @@ export type SkillSource = /** * Permission level a grant confers (#1123). `read` = view/pull/execute; - * `read_write` = also update the skill's content & metadata (no admin — + * `write` = also update the skill's content & metadata (no admin — * permission management, transfer, and delete stay with the owner + admins). */ -export type SkillPermissionLevel = "read" | "read_write"; +export type SkillPermissionLevel = "read" | "write"; /** One typed access grant on a skill / skillset (#1123). */ export interface SkillGrant { @@ -69,7 +69,7 @@ export interface SkillDetail extends SkillSummary { /** Org user_ids this skill has been explicitly shared with. */ sharedWithOrgs: string[]; /** - * Typed access grants (#1123) — the canonical read/read-write ACL. Always + * Typed access grants (#1123) — the canonical read/write ACL. Always * present in API responses; the legacy `sharedWith*` arrays above mirror * the read-visibility of these for back-compat. */ From 084d49a67bcb1630596fd4a04993989bebf2c552 Mon Sep 17 00:00:00 2001 From: Shining <250120269+chronoai-shining@users.noreply.github.com> Date: Tue, 16 Jun 2026 14:42:48 +0800 Subject: [PATCH 5/7] feat(web): show content-edit UI to write-grantees (#1127) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../src/components/skill/SkillHeroStrip.tsx | 4 +- ornn-web/src/hooks/useSkillAccess.ts | 61 +++++++++++++++++++ ornn-web/src/hooks/useSkillDetail.ts | 11 ++-- .../src/pages/skill/EditSkillPage.test.tsx | 34 +++++++++++ ornn-web/src/pages/skill/EditSkillPage.tsx | 24 +++++++- ornn-web/src/pages/skill/SkillDetailPage.tsx | 17 +++--- 6 files changed, 134 insertions(+), 17 deletions(-) create mode 100644 ornn-web/src/hooks/useSkillAccess.ts diff --git a/ornn-web/src/components/skill/SkillHeroStrip.tsx b/ornn-web/src/components/skill/SkillHeroStrip.tsx index 26dd5a73..b87c86c4 100644 --- a/ornn-web/src/components/skill/SkillHeroStrip.tsx +++ b/ornn-web/src/components/skill/SkillHeroStrip.tsx @@ -25,7 +25,6 @@ interface SkillHeroStripProps { pullCount7d?: number | undefined; versionAudit?: AuditRecord | undefined; isAuthenticated: boolean; - isOwner: boolean; ownerDisplayName: string; ownerAvatarUrl?: string | null | undefined; onTryPlayground: () => void; @@ -87,7 +86,6 @@ export function SkillHeroStrip({ pullCount7d, versionAudit, isAuthenticated, - isOwner, ownerDisplayName, ownerAvatarUrl, onTryPlayground, @@ -203,7 +201,7 @@ export function SkillHeroStrip({ replaced the older three-dots menu (#411). Each renders independently when its handler is provided, so the row degrades gracefully (e.g. no edit icon for non-owners). */} - {isOwner && onEditSkill && ( + {onEditSkill && ( + )} - {/* Upload new package */} + {/* Update Package — WRITE tier (owner / admin / write-grantee). */} + {canWrite && (

    {t("editSkill.updatePackageHeading", "Update Package")} @@ -220,6 +239,7 @@ export function EditSkillPage() { )} + )} diff --git a/ornn-web/src/pages/skill/SkillDetailPage.tsx b/ornn-web/src/pages/skill/SkillDetailPage.tsx index cce64767..ca2ab1fb 100644 --- a/ornn-web/src/pages/skill/SkillDetailPage.tsx +++ b/ornn-web/src/pages/skill/SkillDetailPage.tsx @@ -75,6 +75,7 @@ export function SkillDetailPage() { isAuthenticated, isOwner, isAdminUser, + canWrite, canManageVersions, auditSummaryByVersion, versionAudit, @@ -167,12 +168,12 @@ export function SkillDetailPage() { pullCount7d={pullCount7d} versionAudit={versionAudit} isAuthenticated={isAuthenticated} - isOwner={isOwner} ownerDisplayName={ownerDisplayName} ownerAvatarUrl={ownerAvatarUrl} onTryPlayground={() => navigate(`/playground?skill=${encodeURIComponent(skill.name)}`)} onDownloadPackage={rawZip ? handleDownloadPackage : undefined} - onEditSkill={isOwner ? () => navigate(`/skills/${skill.guid}/edit`) : undefined} + // Content edit is the write tier (#1127): owner, admin, or write-grantee. + onEditSkill={canWrite ? () => navigate(`/skills/${skill.guid}/edit`) : undefined} /> {/* ── Audit-version banner (yellow/red/missing only; green is silent) ── */} @@ -249,7 +250,7 @@ export function SkillDetailPage() { /> )} - {isOwner && ( + {canWrite && (