diff --git a/.claude/rules/sim-resource-views.md b/.claude/rules/sim-resource-views.md index 54dc0ce6650..a38735bca44 100644 --- a/.claude/rules/sim-resource-views.md +++ b/.claude/rules/sim-resource-views.md @@ -25,7 +25,7 @@ Enforced by `bun run check:resources` (strict CI gate: `bun run check:resources: | Axis | Type | Replaces | | --- | --- | --- | | `source` | `WorkspaceSource \| ShareSource`, discriminated on `via` | `workspaceId`, `token`, `contentSource`, `isPublic`, `isShared` | -| `grants` | `{ write: boolean; run: boolean }` | `canEdit`, `canRun`, `canAdmin`, `canDelete`, `disableEdit/Insert/Delete` | +| `grants` | `{ write; run; manage; settled }` | `canEdit`, `canRun`, `canAdmin`, `canDelete`, `disableEdit/Insert/Delete` | | `host` | `'page' \| 'panel' \| 'public'` | `embedded`, `isEmbedded`, `compact`, `minimal` | There is no fourth axis. Agent streaming is **one optional prop on `FileView`** (`streaming?: FileViewStreaming`), because only files stream. @@ -87,7 +87,8 @@ return \| ShareSource`, discriminated on `via` | `workspaceId`, `token`, `contentSource`, `isPublic` | -| `grants` | `{ write: boolean; run: boolean }` | `canEdit`, `canRun`, `canAdmin`, `disableEdit/Insert/Delete` | +| `grants` | `{ write; run; manage; settled }` | `canEdit`, `canRun`, `canAdmin`, `disableEdit/Insert/Delete` | | `host` | `'page' \| 'panel' \| 'public'` | `embedded`, `isEmbedded`, `compact`, `minimal` | There is no fourth axis. Agent streaming is one optional prop on `FileView` (`streaming?`), because only files stream. @@ -41,7 +41,7 @@ const source = workspaceSource({ kind: 'file', workspaceId, resourceId: file.id return ``` -Import from the unit barrel (`@/components/resources/file-view`), never a file inside it. Scope-dependent copy lives on `source.unavailableCopy`; links on `source.hrefFor(link)` (which returns `null` in share scope). `hostOwnsUrl(host)` is the one place the "embedded views do not write nuqs keys" rule lives. +Import from the unit barrel (`@/components/resources/file-view`), never a file inside it. Scope-dependent copy lives on `source.unavailableCopy`; links on `source.hrefFor(link)` — `{ to: 'self' | 'resource' | 'list' }`, returning `null` in share scope. `hostOwnsUrl(host)` is the one place the "embedded views do not write nuqs keys" rule lives. ## Never diff --git a/CLAUDE.md b/CLAUDE.md index de5831ce870..67b94da3c9f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -422,7 +422,7 @@ A **resource** is a thing a workspace holds that can also be shared — a file, Views are mounted against exactly **three axes**, defined in `apps/sim/resources/**` (pure TypeScript — no React, no `'use client'`, because a Server Component builds a share source during SSR): - `source` — where the data comes from and by what address: `WorkspaceSource | ShareSource`, discriminated on `via`. Replaces `workspaceId`, `token`, `contentSource`, `isPublic`. `ShareSource` declares `workspaceId?: never`, so a share token can no longer be laundered through a workspace-shaped slot. -- `grants` — what this viewer may do: `{ write, run }`. Replaces `canEdit`, `canRun`, `canAdmin`, `disableEdit/Insert/Delete`. +- `grants` — what this viewer may do: `{ write, run, manage }`, plus `settled` (whether those three are final, or still resolving — a denied member and a loading one are otherwise indistinguishable). Replaces `canEdit`, `canRun`, `canAdmin`, `disableEdit/Insert/Delete`. - `host` — who owns the URL, the router, the document frame: `'page' | 'panel' | 'public'`. Replaces `embedded`. `hostOwnsUrl(host)` is the one place the "embedded views do not write nuqs keys" rule lives. There is no fourth axis; agent streaming is one optional prop on `FileView`. Consumers CONSTRUCT the axes and MOUNT the view — never wrap it in a passthrough, never reach past its barrel, never reimplement its UI because it lacks a seam (add the seam), never import `@/app/workspace/[workspaceId]/**` from an anonymous surface (`app/f/**`, `app/(interfaces)/**`), and never read `useRouter`/`useParams`/`useQueryState`/`useUserPermissionsContext` inside a unit. A kind with no canonical view yet — `table` alone today — is simply absent from the view list in the check's `CANONICAL_UNITS` — no flag, shim, or placeholder. Every unit has the same layout (`.tsx` · `index.ts` · `components//` · `hooks/` · `utils/` · `types.ts`), so moving between them costs nothing. diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/resource-content.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/resource-content.tsx index eefcabbb3a6..73913d4d753 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/resource-content.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/resource-content.tsx @@ -129,8 +129,16 @@ interface ResourceContentProps { onBrowserOverlayControllerChange?: (controller: BrowserPanelOverlayController | null) => void } -/** The agent owns the file while it is streaming; nothing is edited from here. */ -const STREAMING_FILE_GRANTS: ResourceGrants = { write: false, run: false } +/** + * The agent owns the file while it is streaming; nothing is edited from here. + * Settled by construction — this is a literal, not a resolving membership. + */ +const STREAMING_FILE_GRANTS: ResourceGrants = { + write: false, + run: false, + manage: false, + settled: true, +} /** * Grace window kept locked after the agent stops streaming into the file, so the lock bridges the diff --git a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/knowledge-base.tsx b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/knowledge-base.tsx index 0651ff9f5f8..1c877b52dfd 100644 --- a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/knowledge-base.tsx +++ b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/knowledge-base.tsx @@ -422,7 +422,8 @@ export function KnowledgeBase({ { onSuccess: () => { removeKnowledgeBase(id) - router.push(`/workspace/${workspaceId}/knowledge`) + const list = source.hrefFor({ to: 'list' }) + if (list) router.push(list) }, } ) @@ -622,7 +623,10 @@ export function KnowledgeBase({ { label: 'Knowledge Base', icon: Database, - onClick: () => router.push(`/workspace/${workspaceId}/knowledge`), + onClick: () => { + const list = source.hrefFor({ to: 'list' }) + if (list) router.push(list) + }, }, { label: knowledgeBaseCrumbLabel, diff --git a/apps/sim/resources/grants.test.ts b/apps/sim/resources/grants.test.ts index a0db86e0cb7..b6347acdb02 100644 --- a/apps/sim/resources/grants.test.ts +++ b/apps/sim/resources/grants.test.ts @@ -21,6 +21,8 @@ describe('grantsFromPermissions', () => { expect(grantsFromPermissions({ canRead: false, canEdit: false, canAdmin: false })).toEqual({ write: false, run: false, + manage: false, + settled: true, }) }) @@ -28,6 +30,8 @@ describe('grantsFromPermissions', () => { expect(grantsFromPermissions({ canRead: true, canEdit: false, canAdmin: false })).toEqual({ write: false, run: true, + manage: false, + settled: true, }) }) @@ -35,6 +39,8 @@ describe('grantsFromPermissions', () => { expect(grantsFromPermissions({ canRead: true, canEdit: true, canAdmin: false })).toEqual({ write: true, run: true, + manage: false, + settled: true, }) }) @@ -45,6 +51,40 @@ describe('grantsFromPermissions', () => { } }) + it('grants manage exactly to an admin', () => { + for (const permissions of ALL_PERMISSIONS) { + expect(grantsFromPermissions(permissions).manage).toBe(permissions.canAdmin) + } + }) + + /** + * The distinction the field exists for. A resolving membership and a genuine + * no-access member produce identical capability booleans, so without `settled` + * a surface cannot tell "you may not" from "we do not know yet" — and both + * disabled-during-load chrome and one-shot latched effects need to. + */ + it('reports an unresolved membership as unsettled, with the same capabilities as a denied one', () => { + const loading = grantsFromPermissions({ + canRead: false, + canEdit: false, + canAdmin: false, + isLoading: true, + }) + const denied = grantsFromPermissions({ canRead: false, canEdit: false, canAdmin: false }) + + expect(loading.settled).toBe(false) + expect(denied.settled).toBe(true) + expect(loading.write).toBe(denied.write) + expect(loading.run).toBe(denied.run) + expect(loading.manage).toBe(denied.manage) + }) + + it('treats a caller that tracks no loading state as settled', () => { + for (const permissions of ALL_PERMISSIONS) { + expect(grantsFromPermissions(permissions).settled).toBe(true) + } + }) + it('never runs anything without at least read', () => { for (const permissions of ALL_PERMISSIONS) { if (permissions.canRead || permissions.canEdit) continue @@ -66,6 +106,18 @@ describe('grantsForShare', () => { } }) + it('never manages, for any kind', () => { + for (const kind of RESOURCE_KINDS) { + expect(grantsForShare(kind).manage).toBe(false) + } + }) + + it('is always settled — a token resolves capabilities outright', () => { + for (const kind of RESOURCE_KINDS) { + expect(grantsForShare(kind).settled).toBe(true) + } + }) + it('is never more capable than a read-only member', () => { const member = grantsFromPermissions({ canRead: true, canEdit: false, canAdmin: false }) for (const kind of RESOURCE_KINDS) { diff --git a/apps/sim/resources/grants.ts b/apps/sim/resources/grants.ts index 00563f586c4..f099014c4a1 100644 --- a/apps/sim/resources/grants.ts +++ b/apps/sim/resources/grants.ts @@ -16,6 +16,34 @@ export interface ResourceGrants { * surface sends. */ readonly run: boolean + /** + * May change how the resource is governed rather than what it contains — the + * admin-only affordances. Table column locks are the first: an owner decides + * which columns an editor may not touch, which is a different question from + * whether this viewer may write, and the settings an editor is locked out of + * are the ones that lock them out. + * + * On this axis rather than as a per-view `canAdmin` prop because that is the + * vocabulary the axis exists to replace — `check-resource-views.ts` bans the + * name outright. + */ + readonly manage: boolean + /** + * Whether the three capabilities above are final, or still resolving. + * + * The one member that describes this *value* rather than the viewer, and it + * has to sit here: a consumer reading `write === false` cannot otherwise tell + * "this viewer may not write" from "we do not know yet", because + * {@link grantsFromPermissions} maps both to the same booleans. Surfaces that + * render an affordance disabled while permissions load — rather than popping + * it in afterwards — need that distinction, and one-shot effects need it + * badly: firing a latched notice before `manage` resolves permanently drops + * the action it was supposed to carry. + * + * `true` wherever capabilities are known at construction, which is every + * caller that tracks no loading state at all. + */ + readonly settled: boolean } /** @@ -27,6 +55,12 @@ export interface WorkspacePermissionSnapshot { readonly canRead: boolean readonly canEdit: boolean readonly canAdmin: boolean + /** + * Whether the membership is still being fetched. Optional because a caller + * that resolves permissions synchronously has no such state — and its absence + * correctly reads as settled. + */ + readonly isLoading?: boolean } /** @@ -38,10 +72,12 @@ export interface WorkspacePermissionSnapshot { * That is precisely the state this function maps to `run: false`. */ export function grantsFromPermissions(permissions: WorkspacePermissionSnapshot): ResourceGrants { - const { canRead, canEdit } = permissions + const { canRead, canEdit, canAdmin, isLoading } = permissions return { write: canEdit, run: canEdit || canRead, + manage: canAdmin, + settled: !isLoading, } } @@ -49,13 +85,18 @@ export function grantsFromPermissions(permissions: WorkspacePermissionSnapshot): * Grants for an anonymous share visitor. * * A share never writes, and today never runs: every shareable kind is served as - * read-only bytes. `kind` is taken anyway because running is a per-kind property - * — it turns on for a kind whose public surface gains an execution route — so - * callers already pass what that decision will be keyed on. + * read-only bytes, and never manages. `kind` is taken anyway because running is + * a per-kind property — it turns on for a kind whose public surface gains an + * execution route — so callers already pass what that decision will be keyed on. + * + * Always settled: an anonymous visitor's capabilities are known the moment the + * token resolves, so there is no loading state to represent. */ export function grantsForShare(_kind: ResourceKind): ResourceGrants { return { write: false, run: false, + manage: false, + settled: true, } } diff --git a/apps/sim/resources/source.test.ts b/apps/sim/resources/source.test.ts index 2ff31844a18..d8ed0666b9e 100644 --- a/apps/sim/resources/source.test.ts +++ b/apps/sim/resources/source.test.ts @@ -91,6 +91,35 @@ describe('workspaceSource', () => { ) }) + /** + * The five call sites that hand-built these paths before `{ to: 'list' }` + * existed lived in two different route trees, which is exactly how a route + * rename escapes one of them. + */ + it('resolves a list link to its own kind index, for every kind', () => { + const expected: Record = { + file: '/workspace/ws_1/files', + table: '/workspace/ws_1/tables', + knowledge: '/workspace/ws_1/knowledge', + log: '/workspace/ws_1/logs', + } + + for (const kind of RESOURCE_KINDS) { + const source = workspaceSource({ kind, workspaceId: 'ws_1', resourceId: 'id_1' }) + expect(source.hrefFor({ to: 'list' })).toBe(expected[kind]) + } + }) + + it('escapes the workspace id on a list link too', () => { + const source = workspaceSource({ + kind: 'table', + workspaceId: 'ws/../../evil', + resourceId: 'tbl_1', + }) + + expect(source.hrefFor({ to: 'list' })).toBe('/workspace/ws%2F..%2F..%2Fevil/tables') + }) + it('escapes ids so a hostile id cannot graft extra path or query onto the route', () => { const source = workspaceSource({ kind: 'file', @@ -164,6 +193,7 @@ describe('shareSource', () => { for (const kind of SHAREABLE_KINDS) { const source = makeShareSource(kind) expect(source.hrefFor({ to: 'self' })).toBeNull() + expect(source.hrefFor({ to: 'list' })).toBeNull() for (const target of RESOURCE_KINDS) { expect(source.hrefFor({ to: 'resource', kind: target, id: 'id_1' })).toBeNull() } diff --git a/apps/sim/resources/source.ts b/apps/sim/resources/source.ts index a9582b7bf50..f51f249fc60 100644 --- a/apps/sim/resources/source.ts +++ b/apps/sim/resources/source.ts @@ -3,8 +3,19 @@ import type { ResourceKind, ResourceSeed, ShareableKind } from '@/resources/kind /** Why a resource could not be shown. */ export type UnavailableReason = 'missing' | 'transient' -/** A destination a view may want to link to: itself, or another resource by id. */ -export type ResourceLink = { to: 'self' } | { to: 'resource'; kind: ResourceKind; id: string } +/** + * A destination a view may want to link to: itself, another resource by id, or + * the index route its own kind lives under. + * + * `list` is here rather than in a per-kind module because every kind has one and + * every detail surface needs it — a breadcrumb root, and the redirect after the + * resource it was showing is deleted. Five call sites hand-built that path + * before this member existed. + */ +export type ResourceLink = + | { to: 'self' } + | { to: 'resource'; kind: ResourceKind; id: string } + | { to: 'list' } /** Display noun per kind, used by the copy the base builds. */ const RESOURCE_NOUN: Record = { @@ -77,6 +88,26 @@ export type ResourceSource = K extends Re ? WorkspaceSource | ShareSource : never +/** + * The in-app index route a kind lives under, used by {@link workspaceSource}'s + * `hrefFor` for `{ to: 'list' }`. Sibling to {@link resourceHref}, and exhaustive + * for the same reason: a kind added without a list route fails to compile here + * rather than sending a breadcrumb somewhere that does not exist. + */ +function resourceListHref(workspaceId: string, kind: ResourceKind): string { + const workspace = `/workspace/${encodeURIComponent(workspaceId)}` + switch (kind) { + case 'file': + return `${workspace}/files` + case 'table': + return `${workspace}/tables` + case 'knowledge': + return `${workspace}/knowledge` + case 'log': + return `${workspace}/logs` + } +} + /** * The in-app route for a resource, used by {@link workspaceSource}'s `hrefFor`. * The one table — every in-app destination for a resource is spelled here and @@ -124,10 +155,16 @@ export function workspaceSource({ return `Something went wrong loading this ${noun}. Try again.` } }, - hrefFor: (link) => - link.to === 'self' - ? resourceHref(workspaceId, kind, resourceId) - : resourceHref(workspaceId, link.kind, link.id), + hrefFor: (link) => { + switch (link.to) { + case 'self': + return resourceHref(workspaceId, kind, resourceId) + case 'resource': + return resourceHref(workspaceId, link.kind, link.id) + case 'list': + return resourceListHref(workspaceId, kind) + } + }, } }