feat(interfaces): add the interfaces module and share them publicly - #6186
Open
waleedlatif1 wants to merge 20 commits into
Open
feat(interfaces): add the interfaces module and share them publicly#6186waleedlatif1 wants to merge 20 commits into
waleedlatif1 wants to merge 20 commits into
Conversation
Merges 270 commits of staging into the interfaces branch. The branch had moved the file viewer to `components/resources/file-view/**` while staging spent those commits editing it in place, so most conflicts were that rename crossed with real feature work. Kept from both sides, rather than picking a winner: - staging's collaborative editing, agent-stream protocol, folders/pinning, browser+terminal panels, and desktop title-bar lane - the branch's resource-view abstraction, interfaces module, and the extractions it introduced (AnchoredContextMenu, memberFilterOptions, useBackgroundContextMenu, restoreWithUniqueName, chat/special-tags split) Where the two sides refactored the same code, staging's newer behavior was re-hosted inside the branch's structure; where staging deliberately removed something (search-modal highlighting, `useAutoScroll` options, the workflow-alias modules), the removal was honored.
Picks up the 8 commits staging gained during the first merge, including the tool-registry client-boundary work. Both branches had independently split `mergeToolParameters` into a registry-free leaf module for the same reason; staging's version of `tools/merge-params.ts` is kept, and `tools/params.ts` takes staging's `getToolMetadata` read alongside it.
…nup pass
The branch declared `workspace_interface` in the Drizzle schema and queries it
from 11 sites, but no migration ever created it — deploying as-is would have
500'd every interfaces route. Adds 0280_workspace_interfaces.sql (purely
additive; passes check:migrations).
Also restores alphabetical ordering in the generated tool catalog, where the
staging merge had placed UserInterfaceOperation before TerminalOperation and
would have diffed against the generator.
Cleanup pass (effects/state/memo/callback/react-query/url-state/emcn/comments):
- interfaces list held a whole InterfaceDefinition in state, so the delete
dialog narrated a stale name after a concurrent rename; derive it from the
query by id instead
- layout writes refetched the detail they had just adopted from the response;
invalidate on failure only
- file share-badge invalidation moved from onSuccess to onSettled
- drop three dead useMemos and six unobserved useCallbacks
- emcn: single source for the chip end-adornment chrome, compose
chipFieldTextClass rather than re-deriving it, restore the accessible names
("Generate password" not "Generate") lost when GeneratedPasswordInput was
replaced, and align tokens/scale in file-view and the shared empty state
… test
Removes the cloud-review helper's find/search test and the CI step that
installed ripgrep for it — the runners no longer need the binary. The rest of
that suite (path containment on read, diff-coordinate validation, the tool
allowlist, credential scrubbing) is unaffected.
Action items evaluated and applied:
- ChipSelect/ChipDropdown gained a real `error` prop. This branch had added
`aria-invalid` forwarding without any paint, so a rejected dropdown announced
as invalid while looking untouched; the prop matches ChipInput's contract
rather than painting off the ARIA attribute.
- ChipDropdown/ChipSelect now compose ChipChevronDown instead of re-deriving its
slot (output is byte-identical; the branch had already sanctioned the
`text-current` escape hatch these needed).
- Single source for the chip end-adornment chrome; `focus-within:outline-none`
hoisted to the tag-input CVA base.
- Module remove control drops an `!p-1` override for the canonical Chip the same
file already uses twice.
- Drag-affordance palette literal -> `--text-muted`; ChatInput is mounted inside
workspace chrome now, so the raw purple rendered against the design system.
- Corrected the stale `public_share.resourceType` docs ('interface' is live) and
the combobox `role="combobox"` TSDoc; dropped the zero-reference
`shareKeys.lists()`.
Explicitly rejected after evaluation: capping `listInterfaces` (every sibling
list is unbounded too), removing the routes' try/catch (8 of 15 map domain
errors to 409/404/400 that withRouteHandler would flatten to 500), inverting the
chat-module import edge (16 precedents, no rule), and widening
ResourceEmptyState for one consumer.
…gistry A table rendered inside an interface module resolved cell text with a local `resolveCellKind` that handled boolean/null/json/date/string and let everything else fall through to `JSON.stringify`. So a currency column read `1234.5` in a module and `$1,234.50` in the grid, and a select column rendered its stored option id (`opt_open`) instead of the option name. Both now route through `columnTypeOf(column).formatForDisplay`, exactly as the grid does — the registry is already client-safe, so this costs no boundary. Also lands the security property the migration playbook asks for: a test that `resolveCellRender` can never emit the `sim-resource` kind without a `currentWorkspaceId`. That kind's renderer mounts four workspace-authenticated queries, so reaching it from a share surface would fire cookie-authenticated reads for an anonymous viewer. The guard existed and was untested; the test asserts the same URL corpus positively (with a workspace id it MUST resolve to sim-resource) and negatively, so it cannot rot into a vacuous pass. Consolidation from the playbook's "still owed" list: - `toFieldErrorDetails` was byte-identical in both submit routes — one definition in `lib/interfaces/form-submission`, so a member and an anonymous visitor cannot receive different 400 shapes. - `createFormField` was byte-identical in the inspector and the canvas form module, where one copy's comment promised an invariant only a shared function can deliver. Now in `lib/interfaces/form-config`.
…ayer Prep for relocating the table's presentational subtree to `components/resources/table-view/`. Each of these would have made that move either impossible or dishonest, so they land first, separately, with the tree still behaving identically. - `StatusBadge` moves to `components/execution-status/`. The tables grid drew it from `logs/utils`, which imports `getBlock` — so every table cell was pulling the whole block registry for a six-line badge. Three surfaces render it (logs list, log details, grid cells); none of them wanted that edge. - `sim-resource-cell` imported `ChatMessageContext` from the home route's re-export; it is defined in `components/chat/types`. Now read from the source. - `RemoteTableSelection` moves from the presence hook into the grid's own types, inverting the dependency: the overlay that draws a remote selection is presentational, the hook that produces it holds an authenticated socket. The overlay's two absolute self-imports become relative. - `CellContent` takes an `editor` slot instead of importing `InlineEditor`. `apps/sim` has no `sideEffects: false`, so the static import shipped the whole write path (and its authenticated `useTimezone`) regardless of `isEditing`. `workspaceId` also becomes optional — that is the security seam the `cell-render` test pins, and it was previously required, so nothing could exercise the undefined path a share surface needs. Interfaces consolidation from the playbook: - One `MODULE_RESOURCE_COPY` table for both authoring surfaces; `ResourcePickerField` now takes a `kind` instead of five copy props. The two pickers are deliberately NOT merged — one takes items as props, the other wires one query per branch so an unwired table module never fetches workflows. - `module-chrome.ts` now owns the interface grid geometry that was declared twice, including the placement custom properties, unified to `--module-row`/`--module-col`.
Completes the presentational/editing split at the row level. `DataRow` took an `InlineEditor` import and constructed the editor itself, which would have carried the whole write path — and its authenticated `useTimezone` — into the moved view layer. It now takes a `renderCellEditor` callback and renders whatever the host returns. `TableGrid` supplies it, which is where it belongs: the closure needs the pending-update state and the save/cancel handlers that own the mutation. A surface with no write path passes nothing and renders no editor. `DataRow.workspaceId` also becomes optional, matching `CellContent` — the two together are the seam a share-scope surface needs. Note the failure mode this deliberately avoids: making the prop optional means `tsc` stays green whether or not the shell wires it, so the callsite and the memo comparator were both updated in this change rather than left to a later one.
…table-view Behaviour-neutral relocation. 18 files move with ~100% rename similarity; every other changed line in the diff is an import specifier, verified mechanically — filtering import lines out of the `table-grid.tsx` hunk leaves nothing. What moved is what draws a table: the row, the cells (render + content + resource chip), the headers, the column/selection primitives, find, the remote selection overlay, the select pill, and the pure value/grid helpers. What stayed is what writes one: `TableGrid` itself, the inline editor, the expanded-cell popover, and every mutation surface around them. Two types crossed with it — `SaveReason` and `EditingCell` describe the view's editing *protocol*, not the route's query state, so they now live in the unit while `QueryOptions` and `ContextMenuState` stay behind. The 4,420-line grid deliberately does NOT move. It reads `useParams()`, `useUserPermissionsContext()` and ten mutation hooks; it is the shell, and splitting it is not a prerequisite for giving the view layer an address. Worth recording for the next person: `apps/sim/tsconfig.json` excludes test files, so `tsc` stayed green while two moved tests still imported their subjects through dead absolute paths. Only vitest caught it. A green typecheck proves nothing about tests after a move.
…odule fork
`TableView({ source, grants, host })` joins `FileView` and `InterfaceView` as a
canonical resource view, registered in `CANONICAL_UNITS` so R1a/R2/R4/R6 now
police it. `table` moves from "no canonical view yet" to a real one.
The interfaces table module stops re-implementing a table. It had grown its own
451-line renderer — cell resolver, favicon-link builder, offset pagination — and
that fork is now three files lighter: mint a table source from the surrounding
interface source, mount the view. 163 lines become 74.
What the module gains by not being a fork: every cell kind the grid draws.
Booleans, dates, JSON, links, pinned columns, select pills, and workflow-output
state now render identically in a module and in the grid, because they are the
same component rather than an approximation of it.
The share arm is addressed by `(token, grantId)` and carries no table id at all
— the server derives it from the stored layout per request. `workspaceId` is
threaded from the source and is `undefined` there, which is what keeps the
`sim-resource` chip (and the four workspace-authenticated queries its renderer
mounts) off a public page. `cells/cell-render.test.ts` is what holds that.
Two things this deliberately does NOT do:
- It does not touch the tables page or the mothership panel. Both still mount
the full editing shell. Making the panel read-only is a product decision about
whether you can edit a table from chat, not a refactor, and it does not belong
in a change whose value is deleting a fork.
- It does not unify the row page size. The page drains 1000 for client-side sort
and filter; this view takes 100. `pageSize` is in the query key, so they do not
share row cache — which the code now says plainly instead of claiming
otherwise. Fixing it changes behaviour on one surface or the other.
Caught while doing it: `CellRender` returns `null` for an empty cell because the
grid's virtualizer owns row height. A panel has nothing holding the row open, so
an all-empty row collapsed. The floor lives on a wrapper in this view rather than
in `CellRender`, so the grid is untouched. A pre-existing test caught it.
Two corrections the plan got wrong, learned by doing it: the 4,420-line grid never needed to move (it is the shell), and the move needed four unblocking changes first — each of which was worth landing on its own. Also notes what is still open: the standalone share surface, the mothership panel, and the row page-size split.
…t address Both surfaces that mount the table keep every capability they had. The mothership panel still mounts the full editing shell — editing a table from chat is a product decision, not something to drop as a side effect of a refactor — it just says `host='panel'` instead of `embedded`. Three prop smells go with it: - `embedded?: boolean` becomes `host: 'page' | 'panel'`, the same vocabulary the canonical views use. `'public'` is deliberately unrepresentable: the shell holds a write path, and an anonymous surface mounts `TableView`. - `workspaceId` / `tableId` become required instead of optional-with-a- `useParams()`-fallback. That fallback was documented as "page mode reads from useParams", which is another way of saying the component could only ever exist once per page. - `page.tsx` now names `tableId` in its params type. It always had it — the type just did not say so. The ids stay plain rather than becoming a `ResourceSource`: `page.tsx` is a Server Component and a source carries functions, so it cannot cross the RSC boundary. That is the same reason the public interface page hands over a plain seed and lets the client mint the source, and the prop doc now says so. Behaviour is identical by construction — `embedded` is simply derived as `host !== 'page'`, and the permission context the panel reads is untouched.
…cal units `components/resources/**` no longer imports from `app/workspace/[workspaceId]/**` at all. Three edges remained after the table migration, all in the file view's collaboration layer, and both targets were already shared across trees rather than route-specific: - `presence-avatars` -> `components/presence/`. Its own doc said "shared by workflow and files"; it had five consumers across three trees and imports nothing workspace-specific. - `untitled-title` -> `lib/uploads/utils/`. Pure string helpers over a file name, imported by the files page and the markdown editor. Both are pure `git mv` — the diff is three renames with zero content change, and every consumer moved to the new specifier. R3b drops 41 -> 38. The remainder is not this branch's: it is `ee/**` and `components/settings/**` importing the shared settings chrome that lives under `app/workspace/[workspaceId]/settings/components`. 30 of those predate this branch and 8 arrived with the staging merge. Relocating that chrome is a real change to a dozen EE surfaces and belongs in its own PR — but no canonical resource view depends on it any more, which was the point of the rule.
…alse positive Two things the migration notes got wrong, now fixed in the places that assert them. The playbook claimed an open table and an embedded one "should hit one cache entry, and forking that doubles every fetch". Half right, and the wrong half was driving a planned change. The SCHEMA is keyed on the table id alone, so it genuinely is one entry on every surface. ROWS are not, and should not be: `pageSize` is in the key, the page pulls 1000-row pages so `ensureAllRowsLoaded` can drain for select-all/export in few round trips, and a panel wants a fast first screen and never drains. One number would either make a chat panel fetch 1000 rows to show ten, or make every bulk operation ten times the requests. The divergence is correct; both the hook and the playbook now say so. `MiniTablePanel` -> `LandingLeadsPanel`. It is hardcoded landing-hero markup with no queries and no relationship to the table resource, so a `Mini` + resource-noun name misread as a per-consumer fork of the real view — to a reader and to the audit. R1b baseline drops 12 -> 11. The R1b note itself was also wrong, claiming every `Embedded*` drops off as its kind gets a canonical view. The `*Actions` members are tab chrome, not views: `EmbeddedFileActions` and `EmbeddedInterfaceActions` outlived their migrations, and `EmbeddedTableActions` just outlived table's. Only the content components go.
Findings from a full-branch audit pass. Security: - The public form-submit route returned the workflow run's entire `output` to an anonymous visitor. A form module has no `outputConfigs`, so unlike the chat path there is nothing redacting it — for an agent block that means `toolCalls.list`: the literal arguments sent to every tool and the literal responses. No client consumed it. Dropped from the public contract; the authenticated in-app response is unchanged. - The interface share PUT validated the org policy against `authType ?? 'public'` while `upsertResourceShare` persists `authType ?? stored ?? 'public'`, so a bare re-enable was checked against the wrong mode in both directions. Now mirrors the file route, whose comment names this exact bug. - The public *file* OTP route compared codes with `!==`. The interfaces route added `safeCompare`; brought the files route up to it. Correctness: - A combined name+layout PATCH pre-validated with no `previous`, making the pre-flight stricter than the write it guards — renaming an interface that referenced an archived table would 400 on a layout the write accepts. - `DataRow`'s memo comparator observed `renderCellEditor` unconditionally. That callback is rebuilt on every keystroke that opens an editor and on every in-flight save, so every mounted row re-rendered where one needed to. Gated on the editing row, replacing the gate the editor-injection refactor dropped. - `CellContent` derives both the overlay and the dimmed cell from the editor's presence, so "editing with no editor" — a blank, unreadable cell — is unreachable rather than merely unused. CI: - `check:resources` was in package.json but ran nowhere. Added to test-build. Cleanup: deleted an unused `tableResourceId` and a duplicate `PendingTagIndicator`; consolidated the interface row mapper and the form submit-label default onto one definition each; completed a `vi.mock` factory missing three error classes the route narrows on with `instanceof`; corrected stale doc references and a comment asserting a Tailwind limitation that does not exist.
… verify Two gaps recovered from the copilot-tool and duplicate-implementation audits. - `MAX_INTERFACE_DESCRIPTION_LENGTH` was enforced only in the HTTP contract, but the contract is not the only writer: the copilot's `user_interface` tool calls `createInterface`/`updateInterfaceDescription` directly, so the cap did not apply to it. An unbounded description is then re-serialized into every subsequent get/list response and into the copilot's VFS metadata each turn — recurring context cost, not just row bloat. Now asserted beside the name check that was already there for the same reason. - The public file OTP route rate-limited the send path but not verify, while the interfaces route limits both. Added the matching per-IP bucket, kept separate from the send bucket so failed verifies cannot throttle a legitimate resend.
It is a dependency-free complexity-ratio helper shared by a route-tree test and a components/chat test, so living under app/workspace made the canonical tree import back into the route tree. Moves to components/chat, where both consumers reach it in the allowed direction.
# Conflicts: # apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/editor-context-menu.tsx # apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts # apps/sim/app/workspace/[workspaceId]/home/types.ts # apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/context-menu/context-menu.tsx # apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx # apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/chat.tsx # apps/sim/components/chat/chat-context-kind-registry/chat-context-kind-registry.tsx # apps/sim/components/resources/file-view/components/rich-markdown-editor/rich-markdown-editor.tsx # apps/sim/components/resources/file-view/components/text-editor/text-editor.tsx # apps/sim/components/resources/file-view/components/use-selection-copy-bridge.test.tsx # apps/sim/components/resources/file-view/components/use-selection-copy-bridge.ts # apps/sim/components/ui/generated-password-input.tsx # apps/sim/lib/workflows/streaming/streaming.ts # apps/sim/stores/panel/types.ts # scripts/check-api-validation-contracts.ts
The read-only TableView carried `overflow-clip text-ellipsis` on its cell kinds but no `whitespace-nowrap` — which the grid gets from its own CELL_CONTENT. An ellipsis needs a single non-wrapping line, so those classes were inert: text wrapped to several lines and rows grew unevenly where the grid renders one line and an ellipsis. Adds the rule plus `min-w-0` so the flex container can shrink below the now-unwrapped content. Also pins the header, matching the grid, so a long table in a panel keeps its column labels while scrolling. Both are scoped to table-view.tsx; the grid's own rendering path is untouched.
Contributor
|
Too many files changed for review. ( |
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 35187658 | Triggered | Username Password | ca6bed4 | apps/desktop/src/main/browser-credentials/vault.test.ts | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryHigh Risk Overview Public chrome converges on a single Files and shares: public Hygiene: landing mock renames Reviewed by Cursor Bugbot for commit fda21df. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit fda21df. Configure here.
| const newFiles: AttachedFile[] = [] | ||
| const maxSize = 10 * 1024 * 1024 | ||
| const maxFiles = 15 | ||
| const maxSize = maxFileSizeBytes |
There was a problem hiding this comment.
Wrong max size in upload error
Low Severity
When a file exceeds maxFileSizeBytes, the upload error always says “max 10MB” even though validation uses the prop. Interface chat modules pass a 2MB cap, so users see the wrong limit in the message.
Reviewed by Cursor Bugbot for commit fda21df. Configure here.
| textareaRef.current?.focus() | ||
| }, []) | ||
|
|
||
| const canSubmit = (inputValue.trim().length > 0 || attachedFiles.length > 0) && !isStreaming |
There was a problem hiding this comment.
Enter submits disabled composer
Medium Severity
This PR adds disabled and canSubmit so the interface chat composer stays inert in edit mode and when the viewer cannot run, and the send button honors those flags. handleSubmit and the Enter-key path still only gate on streaming and empty content, so pressing Enter can call onSubmit and trigger a workflow run while the UI shows a disabled composer.
Reviewed by Cursor Bugbot for commit fda21df. Configure here.
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
2 participants
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
/i/[token]file | folder) withinterface— samepublicSharetable, same four auth types, no migration needed for the type itself0280_workspace_interfaces.sql— theworkspace_interfacetable was inschema.tswith 11 live query sites and no migration. Deploy blocker on its owncomponents/resources/table-view/unit so an interface module and a public share render every cell kind the grid does — booleans, dates, JSON, links, currency, select pills — instead of approximating them per surfacescripts/check-resource-views.ts(source / grants / host axes) and wirescheck:resourcesinto CISecurity
Found and fixed while auditing this branch:
outputto anonymous visitors. A form module has no output selection, so nothing redacted it — for an agent block that includestoolCalls.list: literal tool arguments and literal responses. No client consumed itauthType ?? 'public'while the persist layer falls back to the stored mode, so a bare re-enable was checked against the wrong auth type in both directions(token, grantId)and the server derives the resource from the stored layout per request!==and rate-limited send but not verify; brought it to parity with the new routeNotes for review
constants.ts,utils.ts,values.ts,select-pill.tsx,table-primitives.tsx,table-find.tsxand all four headers are byte-identical after the move, andcell-render.tsxdiffers by import lines onlyTableViewdeclaresgrants/hostand deliberately does not read them — documented on the propscheck:resources:strictfails on an R3b baseline of 30 vs an actual 38. The script and its baseline are both new here; the 8 came in from staging via the merge, allee/**and settings chrome. Left for a reviewer to call rather than raised to go greenType of Change
Testing
18507 tests / 1347 files pass,
tscclean acrossapps/sim,packages/emcn,apps/realtime,lint:check23/23, andcheck:boundaries/check:api-validation:strict/check:resources/check:desktop-bridge/check:migrationsall pass.Not yet exercised in a browser — the tables cell rendering and the public share page want a click-through.
Checklist