Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions .claude/rules/sim-resource-views.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ Enforced by `bun run check:resources` (strict CI gate: `bun run check:resources:
| Axis | Type | Replaces |
| --- | --- | --- |
| `source` | `WorkspaceSource<K> \| ShareSource<K>`, 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.
Expand Down Expand Up @@ -87,7 +87,8 @@ return <FileView source={source} grants={grants} host='panel' streaming={streami

- Import from the **unit barrel** (`@/components/resources/file-view`), never a file inside it.
- Copy that differs between workspace and share belongs on the **source** (`source.unavailableCopy`), not in the view. A share must never say "workspace" — that is what stops the view becoming an existence oracle.
- Links belong on the source too (`source.hrefFor(link)`), which returns `null` in share scope so nobody hand-builds `/workspace/${token}/…`.
- Links belong on the source too (`source.hrefFor(link)`), which returns `null` in share scope so nobody hand-builds `/workspace/${token}/…`. Three destinations: `{ to: 'self' }`, `{ to: 'resource', kind, id }`, and `{ to: 'list' }` for the index route the kind lives under — a breadcrumb root, or where to go after the resource being shown is deleted.
- `grants.settled` says whether `write`/`run`/`manage` are final. A resolving membership and a denied one produce identical booleans, so a surface that renders an affordance disabled while permissions load — or fires a one-shot effect — must check it rather than reading `write === false` as a decision.
- `host` decides chrome and URL ownership. `hostOwnsUrl(host)` is the single place the "embedded views do not write nuqs keys" rule lives.

## Never do this
Expand Down
4 changes: 2 additions & 2 deletions .cursor/rules/sim-resource-views.mdc
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ Enforced by `bun run check:resources`; strict gate `bun run check:resources:stri
| Axis | Type | Replaces |
| --- | --- | --- |
| `source` | `WorkspaceSource<K> \| ShareSource<K>`, 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.
Expand All @@ -41,7 +41,7 @@ const source = workspaceSource({ kind: 'file', workspaceId, resourceId: file.id
return <FileView source={source} grants={grantsFromPermissions(permissions)} host='panel' />
```

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

Expand Down
2 changes: 1 addition & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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<K> | ShareSource<K>`, 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 (`<unit>.tsx` · `index.ts` · `components/<child>/` · `hooks/` · `utils/` · `types.ts`), so moving between them costs nothing.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
},
}
)
Expand Down Expand Up @@ -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,
Expand Down
52 changes: 52 additions & 0 deletions apps/sim/resources/grants.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,26 @@ describe('grantsFromPermissions', () => {
expect(grantsFromPermissions({ canRead: false, canEdit: false, canAdmin: false })).toEqual({
write: false,
run: false,
manage: false,
settled: true,
})
})

it('lets a read-only member run', () => {
expect(grantsFromPermissions({ canRead: true, canEdit: false, canAdmin: false })).toEqual({
write: false,
run: true,
manage: false,
settled: true,
})
})

it('lets an editor run', () => {
expect(grantsFromPermissions({ canRead: true, canEdit: true, canAdmin: false })).toEqual({
write: true,
run: true,
manage: false,
settled: true,
})
})

Expand All @@ -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
Expand All @@ -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) {
Expand Down
49 changes: 45 additions & 4 deletions apps/sim/resources/grants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

/**
Expand All @@ -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
}

/**
Expand All @@ -38,24 +72,31 @@ 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,
}
}

/**
* 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,
}
}
30 changes: 30 additions & 0 deletions apps/sim/resources/source.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<ResourceKind, string> = {
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',
Expand Down Expand Up @@ -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()
}
Expand Down
49 changes: 43 additions & 6 deletions apps/sim/resources/source.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<ResourceKind, string> = {
Expand Down Expand Up @@ -77,6 +88,26 @@ export type ResourceSource<K extends ResourceKind = ResourceKind> = K extends Re
? WorkspaceSource<K> | ShareSource<K>
: 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
Expand Down Expand Up @@ -124,10 +155,16 @@ export function workspaceSource<K extends ResourceKind>({
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)
}
},
}
}

Expand Down
Loading