From 01f01ab1f35b4c8802803853da6df5b28488aebb Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 7 Aug 2026 16:20:07 -0700 Subject: [PATCH 1/2] refactor(resources): extend the axes for what every kind actually needs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three gaps the file and log migrations never hit, all of which tables and knowledge hit immediately. Extending the axis once beats four per-kind workarounds, and each addition is uniform across every kind. - `ResourceGrants.manage` — admin-only governance of the resource, as distinct from writing its content. Table column locks are the first: an owner decides which columns an editor may not touch, and the settings an editor is locked out of are the ones that lock them out. `grantsFromPermissions` already received `canAdmin` and dropped it on the floor. - `ResourceGrants.settled` — whether the capabilities above are final. The one member that describes the value rather than the viewer, and it has to live beside them: a resolving membership and a genuinely denied one produce identical booleans, so `write === false` could not be told from "not yet". Surfaces that render an affordance disabled during load need that, and one-shot latched effects need it badly — the table's lock notice fires once and permanently loses its action if it fires before `manage` resolves. Without this field both surfaces would have had to accept a first-paint flicker; with it they stay byte-identical. - `ResourceLink` gains `{ to: 'list' }` — the index route a kind lives under. Every kind has one, every detail surface needs it (breadcrumb root, and the redirect after the thing it was showing is deleted), and five call sites across two route trees hand-built that path. `hrefFor` still returns null in share scope, so the list route cannot be hand-built from a token either. Knowledge's two list pushes now go through `hrefFor`. The table's two follow in its own PR, once it builds a source. Verified the new tests fail without the code: breaking `settled` to a constant and pointing the list link at `resourceHref` turns three of them red. --- .../resource-content/resource-content.tsx | 12 ++++- .../knowledge/[id]/knowledge-base.tsx | 8 ++- apps/sim/resources/grants.test.ts | 52 +++++++++++++++++++ apps/sim/resources/grants.ts | 49 +++++++++++++++-- apps/sim/resources/source.test.ts | 30 +++++++++++ apps/sim/resources/source.ts | 43 ++++++++++++--- 6 files changed, 180 insertions(+), 14 deletions(-) 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..d8bd1a31a3b 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 = { @@ -82,6 +93,20 @@ export type ResourceSource = K extends Re * The one table — every in-app destination for a resource is spelled here and * nowhere else, so a surface cannot drift onto a route that no longer exists. */ +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` + } +} + function resourceHref(workspaceId: string, kind: ResourceKind, id: string): string { const workspace = `/workspace/${encodeURIComponent(workspaceId)}` const resource = encodeURIComponent(id) @@ -124,10 +149,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) + } + }, } } From 9804ff90be128a6554633645c1ee4a758776c20f Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 7 Aug 2026 16:25:40 -0700 Subject: [PATCH 2/2] docs(resources): describe the extended axis where the rules already live MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLAUDE.md, .claude/rules and .cursor/rules all still spelled `grants` as `{ write, run }` and `hrefFor` as self-or-resource. Adds `manage`/`settled` and the `{ to: 'list' }` destination, plus the one thing a reader has to know about `settled`: a denied member and a loading one produce identical capability booleans, so `write === false` is not a decision until `settled` says it is. Also splits the TSDoc that `resourceListHref` landed under — it was describing `resourceHref` and would have documented the wrong function. --- .claude/rules/sim-resource-views.md | 5 +++-- .cursor/rules/sim-resource-views.mdc | 4 ++-- CLAUDE.md | 2 +- apps/sim/resources/source.ts | 12 +++++++++--- 4 files changed, 15 insertions(+), 8 deletions(-) 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/resources/source.ts b/apps/sim/resources/source.ts index d8bd1a31a3b..f51f249fc60 100644 --- a/apps/sim/resources/source.ts +++ b/apps/sim/resources/source.ts @@ -89,9 +89,10 @@ export type ResourceSource = K extends Re : never /** - * 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 - * nowhere else, so a surface cannot drift onto a route that no longer exists. + * 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)}` @@ -107,6 +108,11 @@ function resourceListHref(workspaceId: string, kind: ResourceKind): string { } } +/** + * 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 + * nowhere else, so a surface cannot drift onto a route that no longer exists. + */ function resourceHref(workspaceId: string, kind: ResourceKind, id: string): string { const workspace = `/workspace/${encodeURIComponent(workspaceId)}` const resource = encodeURIComponent(id)