refactor(resources): extend the axes for what every kind actually needs - #6391
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryLow Risk Overview Adds Docs in CLAUDE.md, sim-resource-views rules, and tests in Reviewed by Cursor Bugbot for commit 9804ff9. Configure here. |
Greptile SummaryThe PR extends the resource-view axes with administrative and permission-resolution grants, adds kind-specific list destinations, and routes knowledge navigation through the shared source abstraction.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/resources/grants.ts | Extends resource grants with administrative capability and permission-resolution state while preserving workspace and share semantics. |
| apps/sim/resources/source.ts | Adds exhaustive, encoded list-route resolution while retaining null navigation for share sources. |
| apps/sim/app/workspace/[workspaceId]/knowledge/[id]/knowledge-base.tsx | Routes deletion and breadcrumb navigation through the resource source’s list destination. |
| apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/resource-content.tsx | Expands the streaming-file grant literal to satisfy the new resource-grant contract. |
Reviews (2): Last reviewed commit: "docs(resources): describe the extended a..." | Re-trigger Greptile
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.
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.
6186723 to
9804ff9
Compare
|
On the
The direct sibling settles it:
No code change on this one. |
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 9804ff9. Configure here.
17de4bb
into
improvement/resource-views-final
Summary
Stacked on #6279. Three additions to
apps/sim/resources/, each uniform across all four kinds, so tables and knowledge can land without four per-kind workarounds.grants.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.grantsFromPermissionsalready receivedcanAdminand dropped it on the floor.grants.settled— whetherwrite/run/manageare final. The one member that describes the value rather than the viewer, and it has to sit there: a resolving membership and a genuinely denied one produce identical booleans, sowrite === falsecould not be told from "not yet". This reduces behavior change — without it both migrations would have had to accept a first-paint flicker where a disabled control no longer appears while permissions load. It also fixes a worse case: the table's lock notice is a one-shot latched toast that permanently loses its action if it fires beforemanageresolves.ResourceLink { to: 'list' }— the index route a kind lives under. Every kind has one, every detail surface needs it (breadcrumb root, and where to go after the thing being shown is deleted), and five call sites across two route trees hand-built that path.hrefForstill returnsnullin share scope, so the list route can't be reached 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.Type of Change
Testing
bun run type-check,bun run lint:check,bun run check:audits(23/23 includingcheck:resources:strict), and the full suite — 1497 files / 20675 tests, 0 failures.Verified the new tests can fail: breaking
settledto a constant and pointing the list link atresourceHrefturns three of them red.Behavior-neutral by construction — the only production changes are two
router.pushcalls resolving the same string throughhrefFor, and one grants literal gaining two fields.Checklist