refactor(resources): migrate log and knowledge onto the axes; collapse tab chrome - #6280
Merged
waleedlatif1 merged 1 commit intoAug 7, 2026
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryCursor Bugbot is generating a summary for commit 2fcbe72. Configure here. |
Contributor
Greptile SummaryThis refactor migrates knowledge and log resources to their canonical resource views while consolidating embedded tab actions and host-owned state.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/app/workspace/[workspaceId]/knowledge/[id]/hooks/use-knowledge-list-state.ts | Centralizes six knowledge-list state keys and correctly selects URL-backed or local state according to host ownership. |
| apps/sim/components/resources/knowledge-view/knowledge-view.tsx | Introduces the canonical knowledge resource presentation while retaining grant-gated interaction behavior. |
| apps/sim/components/resources/log-view/log-view.tsx | Introduces the canonical log presentation and explicitly gates traces, snapshots, and execution payloads through the caller-provided visibility flag. |
| apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/resource-content.tsx | Mounts canonical resource views in the panel and consolidates supported tab actions into a typed dispatch map. |
| apps/sim/resources/kinds.ts | Removes the obsolete schedule resource kind and updates the canonical resource type model. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
Page[Workspace page host] --> Axes[Source + grants + host]
Panel[Mothership panel host] --> Axes
Axes --> Knowledge[Canonical KnowledgeView]
Axes --> Log[Canonical LogView]
Page --> URL[URL-owned list and tab state]
Panel --> Local[Local list and tab state]
Actions[RESOURCE_TAB_ACTIONS map] --> Panel
Reviews (2): Last reviewed commit: "refactor(resources): migrate log and kno..." | Re-trigger Greptile
waleedlatif1
force-pushed
the
integrate/resource-migrations
branch
from
August 5, 2026 03:50
2fcbe72 to
f164ef5
Compare
…e tab chrome Continues the resource-view layer. Three kinds now have a canonical view (file, log, knowledge) and only `table` remains — deliberately, because the mothership mounts its editing shell, which is a product decision rather than unfinished work. **log** — `log-details/` splits into `components/resources/log-view/` (the view) and a 198-line shell that keeps resize, keyboard nav and close chrome. Four leaked contexts resolved: `useParams` → `source.workspaceId`, `useRouter` → an `onNavigate` prop, `useQueryState(tab)` → view-owned state gated by `hostOwnsUrl(host)`, `usePermissionConfig` → a required `showExecutionInternals` prop (deliberately not folded into `grants`: it gates *seeing* trace internals, not write or run, and an optional field would default permissive — the wrong failure mode for a permission-group restriction). A fifth leak the plan missed — `file-download.tsx` holding its own `useRouter` — was caught by the gate. **knowledge** — `base.tsx` (1733 lines) splits into a 909-line canonical view and a 974-line shell. The read surface (document list, filters, sort, pagination, unavailable state) is the view; upload, connectors, tag editing, rename, delete and bulk operations stay in the shell, exactly as the tables editing grid kept its write path. All six nuqs keys now route through `useKnowledgeListState(host)`, whose every write sits behind `hostOwnsUrl`. **tab chrome** — the five `Embedded*Actions` collapse into one kind-keyed `ResourceTabActions`. They differed only in icon, copy and destination, which is a config table. `knowledge` and `table` destinations now resolve through `workspaceSource().hrefFor()` instead of hand-built strings. **schedule** is removed as a resource kind: its page was deleted upstream, its `resourceHref` case pointed at a route that no longer exists, and `MothershipResourceType` never carried it, so it was unreachable. Behavior change, intended and limited to embedded surfaces: the mothership panel no longer writes the log `?tab` key or the six knowledge keys into the host page's address bar. That is the bug the `host` axis exists to fix. Both route pages keep their deep-linkable params unchanged. `Resource` and `InlineRenameInput` moved to `components/` — mandatory, since a canonical unit may not import the workspace route tree. The workspace barrel re-exports both, so its consumers are byte-unchanged. Ratchets: shadow-named components 8 → 2 (the 2 left are deliberate — `EmbeddedWorkflow`, because a workflow is a live collaborative session rather than a document with an address, and `EmbeddedFolder`, because a folder is structure inside a resource); cross-tree imports 38 → 37. None raised. Verification: apps/sim + 22 packages typecheck, biome clean, 18626 tests pass, and all 15 repo gates pass including both strict variants. Not visually verified.
waleedlatif1
force-pushed
the
integrate/resource-migrations
branch
from
August 5, 2026 04:04
f164ef5 to
11f8735
Compare
Collaborator
Author
Collaborator
Author
|
@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 11f8735. Configure here.
waleedlatif1
merged commit Aug 7, 2026
006e9a7
into
improvement/resource-views-final
5 checks passed
waleedlatif1
added a commit
that referenced
this pull request
Aug 8, 2026
…ost's URL Step 3, and the one intended behavior change in this migration — landing alone so it is reviewable on its own. `useQueryStates(tableDetailParsers)` was called unconditionally and all eleven writers ran in both hosts, so opening a table in the mothership panel wrote `?sort` / `?dir` / `?table-view` onto `/home`. The `view` -> `table-view` wire-key rename exists precisely because of that collision — the workaround documented the bug instead of fixing it. This is the same fix #6280 already applied to the log `?tab` key and the six knowledge keys. `useTableDetailState({ host })` wires both branches unconditionally and returns the same `[state, setState]` shape `useQueryStates` did, so every one of the eleven call sites is unchanged. A host that owns the URL keeps the query params; an embedded one holds the identical values locally. The local branch is one state object, not three: several writers set multiple keys in a single call and rely on that landing as one update, and three setters would tear midway through the view-resolution latch. That deletes the `inheritedParams` guard (~28 lines) outright. It existed only to detect a view id left on the host URL by a previously-open resource; with the panel on local state there is nothing to inherit. The parsers move to `lib/table/detail-search-params.ts`. They cannot stay in the route tree — the hook that owns the URL may not import it — and they cannot move into the unit either, because a unit may not call nuqs at all. A pure, server-safe parser module is the one home both halves may reach. What changes for a panel user: sort/view no longer survive a hard reload of the host page, and no longer leak between two tabs open on different tables. Both route pages keep their deep-linkable params unchanged. Verified the six panel-isolation tests go red when the host gate is forced open.
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
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
Stacked on #6279 — review that first. This PR's diff is 76 files against it.
tablenow renders through one canonical viewEmbedded*Actionscomponents into a singleResourceTabActionsdriven by aRESOURCE_TAB_ACTIONSmap, replacing five near-identical kind-specific components with one lookupuse-knowledge-list-state, with all six URL keys (q,enabled,sort,dir,page,addConnector) gated onhostOwnsUrlso the embedded panel keeps them local instead of writing them to the page's URLshowExecutionInternalsexplicitly into the log view rather than reading permissions internally, so the same view serves both hostsscheduleresource kindRatchets:
Embedded*components 8 → 2, cross-tree workspace imports 38 → 37.Type of Change
Testing
Typecheck clean, all 15 repo gates pass, biome clean, full suite green (18,626 tests).
Not visually verified. The knowledge base page is the highest risk here — its header, toolbar, and table moved under
ResourceRoot, and its URL params now flow through the new hook.Checklist