feat: member-derived skillset visibility (#1136)#1137
Merged
chronoai-shining merged 14 commits intoJun 16, 2026
Conversation
First step of member-derived skillset visibility: add `membersAllPublic`
+ `memberVisibilityState` ("all-public" | "restricted" | "unresolvable")
to `SkillsetDocument`, plumbed through create (seeded all-public), update
(recompute-only setters), and mapDoc (defaults to all-public when absent
so existing docs read cleanly until the boot backfill runs).
No behaviour change yet — these fields are written but not yet computed
from members or consulted by the read/discovery gates. The legacy
owner-set `isPrivate`/`sharedWith*`/`grants` on skillsets are marked
inert (kept for rollback); subsequent commits make visibility derived.
Part of #1136
A skill visibility change must recompute every skillset that references it, so we need to answer "which skillsets contain skill X?" cheaply. Add a multikey index on `skillset_versions.members` and a `findSkillsetGuidsByMember(name, guid)` lookup. Member refs come in several grammars (`name@major.minor`, `guid@major.minor`, `name@dist-tag`), so the query anchors on `^(name|guid)@` — the `@` boundary prevents prefix bleed (`rev` must not match `review@1.0`) and the name is regex-escaped so a metachar in a skill name is matched literally. Scans all (immutable) versions; callers dedupe to the guid and recompute only each skillset's latest version. Part of #1136.
A skillset has no owner-set visibility — its reach is bounded by its least-privileged member. Add the recompute that derives the denormalized cache (`membersAllPublic` + `memberVisibilityState`) from the latest version's direct member refs: - all members public → all-public - ≥1 private (resolvable) → restricted - ≥1 ref no longer resolves → unresolvable Classification resolves each ref under SYSTEM via the shared skill version loader, so the ref grammar (name/guid, version/dist-tag/latest) stays single-sourced with the closure walk. To classify without a second lookup, `ResolvedVersion` now carries the skill's `isPrivate` (the closure algorithm ignores it; under SYSTEM it faithfully reports privacy since the read gate passes everything). `recomputeForSkill(name, guid)` fans the recompute over every skillset referencing a changed skill (via the reverse member index) and returns the affected guids for downstream owner-notification work. Writes go through a dedicated `setDerivedVisibility` repo method that does NOT bump `updatedBy`/`updatedOn` — the cache reflects member privacy, not a user edit. Not yet wired into routes. Part of #1136.
The repo seeds a new skillset as all-public, which is wrong the moment a member is private. Wire `recomputeSkillsetVisibility` into `createSkillset` (after the first version is written) and `publishVersion` (after `latestVersion` advances) so the derived cache reflects the actual member set immediately — a skillset created with a private member is "restricted", and a publish that swaps members rederives against the new latest version. The skill-side write path drives the same recompute reactively in a later commit; this covers the skillset-side writes. Part of #1136.
Existing skillsets predate the derived-visibility cache, so their `membersAllPublic` / `memberVisibilityState` default to the seeded all-public until something recomputes them. Add an idempotent one-shot backfill (`backfillDerivedVisibility`) that recomputes the cache for every skillset on startup, wired into `wireSkillsets` and awaited right after `ensureIndexes`. A single skillset's recompute failure is logged and skipped — one bad or unresolvable member set must not abort the backfill or block boot. Adds `SkillsetRepository.listAllGuids` (projection-only) to drive the iteration cheaply. Part of #1136.
A skillset has no owner-set visibility — a caller may read it iff they
can read every member skill. Replace the standalone `canReadSkillset`
delegate with a service-level, per-caller gate:
- `getSkillsetForRead` resolves each member ref under the caller via
the shared skill loader; the owner/admin always see the detail WITH
the members they can't read (`unreadableMembers`, request-scoped) so
they can repair access, while everyone else gets a flat 404 the
moment any member is unreadable — never leaking which one.
- the closure resolver drops its standalone entry gate; the per-member
loader already enforces visibility node-by-node
(`skill_dependency_not_found`, no leak).
The detail/search responses now carry the derived `memberVisibilityState`
(authoritative) + `unreadableMembers`; the legacy `isPrivate`/`grants`
are inert. The skillsets `authorize.ts` (a thin, now-unused delegate to
the skill gates) is deleted.
Part of #1136.
Discovery now follows the member-derived model: a skillset is
discoverable by a caller iff they can read all its members. Rewrite the
skillset repository's scoped read off the derived `memberVisibilityState`
(NOT the shared skill `applyScope`, which assumes owner-set `isPrivate`):
- `public` / `mine` → exact Mongo pagination on denormalized fields
(`findCheapScope`); the all-public fast-path keeps the common browse
case O(1).
- `private` / `mixed` / `shared-with-me` → `findLiveScopeCandidates`
returns a bounded candidate superset, then the search service
live-checks each restricted-by-others candidate via the new
`SkillsetService.canDiscoverSkillset` (per-caller member readability)
and paginates in-memory. A restricted skillset surfaces only to
callers who can read every member; never leaked otherwise. The
candidate cap is logged when hit (no silent truncation).
The search service now takes the resolved `ActorContext` (the live gate
resolves member refs under the caller, so it needs org memberships, not
just the user id) and is wired with the skillset service. Drops the now
unused `applyScope`/`applyExtraFilters` import and the inert legacy
`sharedWith*Any`/`createdByAny` skillset filters.
Part of #1136.
Add the owner-side notification fired when a member skill's visibility change costs the skillset OWNER read access, so they know the skillset's reach shrank and can repair it. `notifySkillsetMemberUnreadable` coalesces to one notification per skillset per recompute (the caller passes the full unreadable member set) and deep-links to the skillset. Single-source the category vocabulary: `NOTIFICATION_CATEGORIES` is now the one canonical list, with both the `NotificationCategory` type and the boot migration's allow-list derived from it. This also fixes a latent drift — the hand-maintained allow-list had already lost `launchPromo.codeDelivered`, so those notifications were being deleted on every reboot. Not yet wired into the recompute path. Part of #1136.
Wire the derived-visibility recompute into every skill mutation that
changes a skill's readability: `PUT /skills/:id` (privacy flip only),
`PUT /skills/:id/permissions`, `POST /skills/:id/transfer-ownership`,
`PUT /skills/:id/nyxid-service`, and `DELETE /skills/:id`. Each fires a
fire-and-forget `fireSkillsetRecompute({guid, name})`, mirroring the
existing `fireMirrorSync` pattern — never blocking the response.
`SkillsetService.recomputeForChangedSkill` recomputes the cache for
every skillset referencing the skill, then — for each affected skillset
whose OWNER the change cost member-read access — fires one
`skillset.member_unreadable` notification. To avoid re-notifying on
unrelated changes while a skillset is already broken, a notification
fires only when the just-changed skill is among the owner's now-unreadable
members. Owner-readability is best-effort: a background task has no way
to resolve the owner's org memberships, so org-granted access can't be
confirmed here — the skillset page recomputes the authoritative set
under the owner's own token.
Bootstrap binds the hook via a forward reference because the skill
routes are built before the skillset wiring (which injects the skill
service); the hook is only invoked at request time, after boot.
Part of #1136.
A skillset's visibility is now wholly derived from its members, so the owner-set permission path is dead and misleading. Remove `PUT /skillsets/:id/permissions`, `SkillsetService.setPermissions`, the `skillsetPermissionsSchema` / `SkillsetPermissionsInput`, and the now-unused grant-resolution imports. Drop the obsolete visibility- transition tests; the closure test no longer needs to "make it public" to admit an anon caller (there is no entry gate — the private member dep is what blocks them). The legacy `isPrivate`/`grants` columns stay on the schema, inert, for this release (rollback safety); a later cleanup drops them. Part of #1136.
…ref (#1136) prefer-const flagged the let forward reference; a const holder object filled in after the skillset wiring exists is equivalent and lint-clean.
A skillset has no owner-set visibility — its reach is bounded by its member skills. Surface that in the UI and remove the owner-set permissions path: - types carry the derived `memberVisibilityState` + per-caller `unreadableMembers`; the legacy `isPrivate`/`grants` are inert. - a read-only `SkillsetVisibilityBadge` (Public / Restricted / Broken) replaces the isPrivate pill in the hero strip and browse cards. - the detail page's right-rail visibility card becomes the read-only `SkillsetDerivedVisibilityCard` (no "Manage permissions" action; explains that reach is widened by sharing the member skills), and an owner/admin `SkillsetMemberWarningBanner` lists members the caller can no longer read with the fix. - the `SkillsetPermissionsModal`, `updateSkillsetPermissions` API, `useUpdateSkillsetPermissions` hook, and `SkillsetPermissionsInput` type are removed (the backend endpoint is gone). Part of #1136.
Add the `skillset.member_unreadable` category to the frontend notification union and the category-label maps (bell detail modal + notifications page) so the owner-side member-access-loss notification renders with a "Skillset" label and deep-links to the skillset. Also adds the `launchPromo.codeDelivered` category the union had been missing. Part of #1136.
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
Redesigns skillset visibility so a skillset has no owner-set visibility — its reach is bounded by its least-privileged member skill:
Owners can no longer manually set a skillset's visibility. To widen a skillset's reach, expose the underlying member skills to the intended audience. The page warns the owner (and an in-product notification fires) when a member skill becomes unreadable and shrinks the skillset's reach.
Closes #1136. Follow-up to #1125. Related to #1134.
Backend (
ornn-api)membersAllPublic+memberVisibilityState(all-public/restricted/unresolvable) on each skillset, computed from the latest version's direct members under SYSTEM. Recomputed on create/publish, reactively on member-skill changes, and by an idempotent boot backfill.skillset_versions.membersmultikey index +findSkillsetGuidsByMemberso a skill change can find the skillsets containing it.getSkillsetForReadresolves each member under the caller; owner/admin always see the detail withunreadableMembers(for repair), everyone else gets a flat 404 the moment a member is unreadable (no leak of which one). The closure resolver drops its standalone entry gate — the per-member loader already enforces visibility.public/minepaginate exactly on the denormalized fields (all-public fast-path);private/mixed/shared-with-mefetch bounded candidates and live-check each restricted-by-others candidate per caller. A restricted skillset is discoverable only by callers who can read every member.fireSkillsetRecomputewired into the 5 visibility-affecting skill routes (privacy flip, permissions, transfer, nyxid-service, delete); fires a one-per-skillsetskillset.member_unreadableowner notification when the change cost the owner access. Best-effort owner-readability (a background task can't resolve the owner's org memberships — the page recomputes the authoritative set under the owner's token).PUT /skillsets/:id/permissions,setPermissions, the permissions schema. LegacyisPrivate/grantscolumns stay inert this release (rollback safety).launchPromo.codeDeliveredrows on reboot).Frontend (
ornn-web)SkillsetVisibilityBadge(Public / Restricted / Broken) in the hero strip + browse cards.SkillsetDerivedVisibilityCard(no "Manage permissions" — explains reach is widened by sharing the members) + an owner/adminSkillsetMemberWarningBannerlisting unreadable members with the fix.SkillsetPermissionsModal+ permissions API/hook/type.skillset.member_unreadablenotification (deep-links to the skillset).Verification
bun run typecheck(api + web + sdk) — cleanbun run lint— 0 errors (2 pre-existing warnings in an untouched file)bun run test:api— 1949 pass / 0 failbun run test:web— 573 pass / 0 fail🤖 Generated with Claude Code