Skip to content

Commit bd46dfb

Browse files
fix(dashboards): menu order, org-chat entitlement, mention ids, contract bounds (#8495)
* fix(mothership): order dashboards between chats and tables in resource menus * chore(rules): drop the resource-menu sidebar-mirroring rule * fix(dashboards): scope entitlements, keep mention ids, and tighten contracts --------- Co-authored-by: Waleed Latif <walif6@gmail.com>
1 parent e1e8689 commit bd46dfb

12 files changed

Lines changed: 132 additions & 106 deletions

File tree

‎.claude/rules/sim-list-ordering.md‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
---
2-
description: List and menu ordering that mirrors the sidebar or toolbar, with one separator before the destructive action
2+
description: List and menu ordering that mirrors the toolbar or settings nav, encoded once, with one separator before the destructive action
33
paths:
44
- "apps/sim/app/**/*.tsx"
55
- "apps/sim/ee/**/*.tsx"
@@ -8,7 +8,7 @@ paths:
88

99
# List & Menu Ordering
1010

11-
**A list orders itself the way the user already reads the same things somewhere else.** Dropdowns, context menus, tab strips, command palettes, and settings navs are all *second* presentations of a set the user has already seen — in the sidebar, in a toolbar, in a column-header row. When the second presentation reorders that set, the user re-reads it from scratch every time.
11+
**A list orders itself the way the user already reads the same things somewhere else.** Dropdowns, context menus, tab strips, command palettes, and settings navs are all *second* presentations of a set the user has already seen — in a toolbar, in the settings nav, in a column-header row. When the second presentation reorders that set, the user re-reads it from scratch every time.
1212

1313
This is not a style preference. Order is the cheapest affordance a list has, and the only one that costs nothing to get right.
1414

@@ -18,13 +18,14 @@ Before writing a list of items, find where the user sees those same items *first
1818

1919
| The list | Mirrors |
2020
| --- | --- |
21-
| Resource menus (`+` attach, `@` mention, resource-tab `+`) | the workspace **sidebar**, top-down |
2221
| A row / root **context menu** | that surface's **toolbar**, left-to-right → top-to-bottom |
2322
| Settings tab strip, recently-deleted tabs | the **settings nav**, top-down |
2423
| A "New …" menu | the order those things appear once created |
2524

2625
Left-to-right becomes top-to-bottom. A toolbar reading `Filter · Sort · Export · Delete` becomes a menu reading Filter, Sort, Export, Delete — never alphabetized, never grouped by implementation, never "destructive last" unless the toolbar already puts it last.
2726

27+
Resource menus (`+` attach, `@` mention, resource-tab `+`) do not mirror the sidebar. Their order is a product decision encoded in `RESOURCE_MENU_ORDER` (see below), and every resource menu shares it.
28+
2829
Platform-only entries (desktop **Browser** and **Terminal**) trail the shared set rather than interleaving, so the common prefix is identical on every platform.
2930

3031
## Grouping: a rule marks a change in what the action acts on
@@ -107,10 +108,10 @@ grouping wants the standard grouping.
107108
An order duplicated across surfaces is an order that will drift. Export **one** constant and sort by it — do not hand-maintain a matching literal per menu.
108109

109110
```ts
110-
/** Top-down order for every menu listing resource families, mirroring the sidebar. */
111+
/** Top-down order for every menu listing resource families. */
111112
export const RESOURCE_MENU_ORDER: readonly MothershipResourceType[] = [
112-
'integration', 'task', 'table', 'file', 'filefolder',
113-
'knowledgebase', 'log', 'workflow', 'folder', 'browser', 'terminal', 'generic',
113+
'integration', 'task', 'dashboard', 'table', 'file', 'filefolder',
114+
'knowledgebase', 'workflow', 'log', 'folder', 'browser', 'terminal', 'generic',
114115
]
115116

116117
export function byResourceMenuOrder<T extends { type: MothershipResourceType }>(a: T, b: T) {

‎.cursor/rules/sim-list-ordering.mdc‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
---
2-
description: "List and menu ordering that mirrors the sidebar or toolbar, with one separator before the destructive action"
2+
description: "List and menu ordering that mirrors the toolbar or settings nav, encoded once, with one separator before the destructive action"
33
globs: ["apps/sim/app/**/*.tsx","apps/sim/ee/**/*.tsx","apps/sim/components/**/*.tsx"]
44
---
55

66
<!-- Generated from .claude/rules/sim-list-ordering.md by `bun run skills:sync`. Edit the source, not this file. -->
77

88
# List & Menu Ordering
99

10-
**A list orders itself the way the user already reads the same things somewhere else.** Dropdowns, context menus, tab strips, command palettes, and settings navs are all *second* presentations of a set the user has already seen — in the sidebar, in a toolbar, in a column-header row. When the second presentation reorders that set, the user re-reads it from scratch every time.
10+
**A list orders itself the way the user already reads the same things somewhere else.** Dropdowns, context menus, tab strips, command palettes, and settings navs are all *second* presentations of a set the user has already seen — in a toolbar, in the settings nav, in a column-header row. When the second presentation reorders that set, the user re-reads it from scratch every time.
1111

1212
This is not a style preference. Order is the cheapest affordance a list has, and the only one that costs nothing to get right.
1313

@@ -17,13 +17,14 @@ Before writing a list of items, find where the user sees those same items *first
1717

1818
| The list | Mirrors |
1919
| --- | --- |
20-
| Resource menus (`+` attach, `@` mention, resource-tab `+`) | the workspace **sidebar**, top-down |
2120
| A row / root **context menu** | that surface's **toolbar**, left-to-right → top-to-bottom |
2221
| Settings tab strip, recently-deleted tabs | the **settings nav**, top-down |
2322
| A "New …" menu | the order those things appear once created |
2423

2524
Left-to-right becomes top-to-bottom. A toolbar reading `Filter · Sort · Export · Delete` becomes a menu reading Filter, Sort, Export, Delete — never alphabetized, never grouped by implementation, never "destructive last" unless the toolbar already puts it last.
2625

26+
Resource menus (`+` attach, `@` mention, resource-tab `+`) do not mirror the sidebar. Their order is a product decision encoded in `RESOURCE_MENU_ORDER` (see below), and every resource menu shares it.
27+
2728
Platform-only entries (desktop **Browser** and **Terminal**) trail the shared set rather than interleaving, so the common prefix is identical on every platform.
2829

2930
## Grouping: a rule marks a change in what the action acts on
@@ -106,10 +107,10 @@ grouping wants the standard grouping.
106107
An order duplicated across surfaces is an order that will drift. Export **one** constant and sort by it — do not hand-maintain a matching literal per menu.
107108

108109
```ts
109-
/** Top-down order for every menu listing resource families, mirroring the sidebar. */
110+
/** Top-down order for every menu listing resource families. */
110111
export const RESOURCE_MENU_ORDER: readonly MothershipResourceType[] = [
111-
'integration', 'task', 'table', 'file', 'filefolder',
112-
'knowledgebase', 'log', 'workflow', 'folder', 'browser', 'terminal', 'generic',
112+
'integration', 'task', 'dashboard', 'table', 'file', 'filefolder',
113+
'knowledgebase', 'workflow', 'log', 'folder', 'browser', 'terminal', 'generic',
113114
]
114115

115116
export function byResourceMenuOrder<T extends { type: MothershipResourceType }>(a: T, b: T) {

‎CLAUDE.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ The `'use client'` server boundary, the app/worker runtime env split, and featur
9191
- **Components**: `'use client'` only for hooks or browser APIs. Structure order, extraction thresholds, and list-render rules: `.claude/rules/sim-components.md`. Render-performance idioms (lazy-init refs, hoisting, `Map` pre-indexing, `[...arr].sort()` never `toSorted()` on client paths): `.claude/rules/sim-react-performance.md`. For effect/state/memo/callback anti-patterns use the `/you-might-not-need-*` skills and verify against the running UI.
9292
- **State ownership**: React Query owns server data — never `useState` + `fetch`; shareable client view-state (tabs, filters, search, pagination, selected id) lives in the URL via `nuqs`; Zustand owns global client state; `useState` owns UI-only state. Hooks: `.claude/rules/sim-hooks.md`. Stores (`devtools`, `persist` only with an explicit `partialize` whitelist, workflow value invariants): `.claude/rules/sim-stores.md`. URL state: `.claude/rules/sim-url-state.md`.
9393
- **Utils**: inline a helper with one consumer; create `utils.ts` when 2+ files share it — in `lib/` (app-wide) or `feature/utils/` (feature-scoped). Check `lib/` before writing a new one.
94-
- **Lists and menus** mirror the order the user already reads elsewhere (sidebar, toolbar), encoded in one exported order constant; a separator marks only a change in what the action acts on (typically one, before the destructive action): `.claude/rules/sim-list-ordering.md`.
94+
- **Lists and menus** mirror the order the user already reads elsewhere (toolbar, settings nav), encoded in one exported order constant (resource menus share `RESOURCE_MENU_ORDER`, a product order that does not mirror the sidebar); a separator marks only a change in what the action acts on (typically one, before the destructive action): `.claude/rules/sim-list-ordering.md`.
9595
- **Caching**: `lru-cache` with a `max` ceiling, never a hand-rolled TTL `Map`; a lifecycle map is not a cache; cache the gate, never the credential: `.claude/rules/sim-caching.md`.
9696

9797
## API Contracts and Routes

‎apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-registry/resource-registry.tsx‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -289,17 +289,16 @@ export const RESOURCE_REGISTRY: Record<MothershipResourceType, ResourceTypeConfi
289289
export const MENTION_PREVIEW_DEFAULT_LIMIT = 5
290290

291291
/**
292-
* Top-down order for every menu that lists resource families, mirroring the
293-
* workspace sidebar so a user reads the same sequence in both places. The two
294-
* desktop-only panels trail the workspace resources, matching where they surface
295-
* in the app. `folder`/`filefolder` never render as their own entry — they feed
292+
* Top-down order for every menu that lists resource families (`+` attach, `@`
293+
* mention, resource-tab `+`). It is its own product order, not a copy of the
294+
* sidebar. The two desktop-only panels trail the workspace resources. `folder`/`filefolder` never render as their own entry — they feed
296295
* their family's folder tree — but are ordered beside it so a menu that ever does
297296
* surface them lands in the right place.
298297
*/
299298
export const RESOURCE_MENU_ORDER: readonly MothershipResourceType[] = [
300-
'dashboard',
301299
'integration',
302300
'task',
301+
'dashboard',
303302
'table',
304303
'file',
305304
'filefolder',

‎apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3423,7 +3423,7 @@ export function useChat(
34233423
? { viewId: (c.currentView ? c.currentView.viewId : c.viewId) ?? undefined }
34243424
: {}),
34253425
...('fileId' in c && c.fileId ? { fileId: c.fileId } : {}),
3426-
...(c.kind === 'dashboard' ? { dashboardId: c.dashboardId } : {}),
3426+
...('dashboardId' in c && c.dashboardId ? { dashboardId: c.dashboardId } : {}),
34273427
...('folderId' in c && c.folderId ? { folderId: c.folderId } : {}),
34283428
...(c.kind === 'skill' && 'skillId' in c ? { skillId: c.skillId } : {}),
34293429
...(c.kind === 'integration' && 'blockType' in c ? { blockType: c.blockType } : {}),

‎apps/sim/lib/api/contracts/dashboards.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ export const dashboardRecordSchema = z.object({
1313
type: z.literal('dashboard'),
1414
name: z.string(),
1515
updatedAt: z.string(),
16-
revision: z.string(),
16+
revision: dashboardRevisionSchema,
1717
})
1818

1919
/** A workspace has at most one dashboard, which Sim builds; both fields are null until then. */
@@ -25,7 +25,7 @@ export const readWorkspaceDashboardContract = defineRouteContract({
2525
mode: 'json',
2626
schema: z.object({
2727
dashboard: dashboardRecordSchema.nullable(),
28-
content: z.string().nullable(),
28+
content: dashboardContentSchema.nullable(),
2929
}),
3030
},
3131
})

‎apps/sim/lib/dashboards/repository.integration.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,8 @@ describe('dashboard repository in PostgreSQL', () => {
5353
})
5454

5555
it('updates only at the expected revision and advances it', async () => {
56-
const current = (await insertWorkspaceDashboard('ws-c', 'first', 'user-1'))!
56+
const current = await insertWorkspaceDashboard('ws-c', 'original', 'user-1')
57+
if (!current) throw new Error('ws-c dashboard was not created')
5758
const updated = await updateDashboardContent(current.id, 'edited', 'user-2', current.revision)
5859
expect(updated).toMatchObject({
5960
content: 'edited',

‎apps/sim/lib/mothership/chat/display-message.test.ts‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,20 @@ describe('display-message', () => {
154154
])
155155
})
156156

157+
it('keeps the dashboard id on a reopened dashboard mention', () => {
158+
const display = toDisplayMessage({
159+
id: 'msg-dashboard',
160+
role: 'user',
161+
content: '@Dashboard',
162+
timestamp: '2024-01-01T00:00:00.000Z',
163+
contexts: [{ kind: 'dashboard', label: 'Dashboard', dashboardId: 'dash-1' }],
164+
})
165+
166+
expect(display.contexts).toEqual([
167+
{ kind: 'dashboard', label: 'Dashboard', dashboardId: 'dash-1' },
168+
])
169+
})
170+
157171
it('preserves browser and terminal selection metadata for reopened messages', () => {
158172
const display = toDisplayMessage({
159173
id: 'msg-selection',
@@ -208,20 +222,6 @@ describe('display-message', () => {
208222
])
209223
})
210224

211-
it('keeps the dashboard id of a reopened dashboard mention', () => {
212-
const display = toDisplayMessage({
213-
id: 'msg-dashboard',
214-
role: 'user',
215-
content: '@Dashboard',
216-
timestamp: '2024-01-01T00:00:00.000Z',
217-
contexts: [{ kind: 'dashboard', label: 'Dashboard', dashboardId: 'dashboard-1' }],
218-
})
219-
220-
expect(display.contexts).toEqual([
221-
{ kind: 'dashboard', label: 'Dashboard', dashboardId: 'dashboard-1' },
222-
])
223-
})
224-
225225
it.each(['pending', 'executing', 'awaiting_approval'])(
226226
'shows a %s row of a stored message as interrupted, not running',
227227
(state) => {

‎apps/sim/lib/mothership/chat/display-message.ts‎

Lines changed: 9 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@ import type { PersistedContentBlock } from '@/lib/api/contracts/copilot-messages
22
import { getMothershipAttachmentPreviewUrl } from '@/lib/mothership/chat/attachment-preview'
33
import { isLiveAssistantMessageId } from '@/lib/mothership/chat/live-message-id'
44
import type { PersistedMessage } from '@/lib/mothership/chat/persisted-message'
5-
import { isUnsettledToolState, withBlockTiming } from '@/lib/mothership/chat/persisted-message'
5+
import {
6+
copyPersistedMessageContext,
7+
isUnsettledToolState,
8+
withBlockTiming,
9+
} from '@/lib/mothership/chat/persisted-message'
610
import {
711
MothershipStreamV1CompletionStatus,
812
MothershipStreamV1EventType,
@@ -141,26 +145,10 @@ function toDisplayContexts(
141145
contexts: PersistedMessage['contexts']
142146
): ChatMessageContext[] | undefined {
143147
if (!contexts || contexts.length === 0) return undefined
144-
return contexts.map((c) => ({
145-
kind: c.kind as ChatContextKind,
146-
label: c.label,
147-
...(c.workflowId ? { workflowId: c.workflowId } : {}),
148-
...(c.knowledgeId ? { knowledgeId: c.knowledgeId } : {}),
149-
...(c.tableId ? { tableId: c.tableId } : {}),
150-
...(c.viewId ? { viewId: c.viewId } : {}),
151-
...(c.fileId ? { fileId: c.fileId } : {}),
152-
...(c.dashboardId ? { dashboardId: c.dashboardId } : {}),
153-
...(c.folderId ? { folderId: c.folderId } : {}),
154-
...(c.chatId ? { chatId: c.chatId } : {}),
155-
...(c.blockType ? { blockType: c.blockType } : {}),
156-
...(c.skillId ? { skillId: c.skillId } : {}),
157-
...(c.serverId ? { serverId: c.serverId } : {}),
158-
...(c.fileName ? { fileName: c.fileName } : {}),
159-
...(c.tableName ? { tableName: c.tableName } : {}),
160-
...(c.tabId ? { tabId: c.tabId } : {}),
161-
...(c.terminalId ? { terminalId: c.terminalId } : {}),
162-
...(c.selection ? { selection: { ...c.selection } } : {}),
163-
}))
148+
return contexts.map((c) => {
149+
const copy = copyPersistedMessageContext(c)
150+
return { ...copy, kind: copy.kind as ChatContextKind }
151+
})
164152
}
165153

166154
const WORKSPACE_FILE_TOOL = 'prepare_file_edit'

‎apps/sim/lib/mothership/chat/payload.test.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,23 @@ describe('buildCopilotRequestPayload', () => {
372372
}
373373
)
374374

375+
it('never grants the workspace-only dashboards entitlement to an organization chat', async () => {
376+
mockDashboardAvailability.mockResolvedValue(true)
377+
const payload = await buildCopilotRequestPayload(
378+
{
379+
message: 'Show my dashboard',
380+
userId: 'actor',
381+
userMessageId: 'message-1',
382+
organizationId: 'org-1',
383+
principal: { kind: 'session' as const, userId: 'actor' },
384+
mode: 'agent',
385+
model: '',
386+
},
387+
{ selectedModel: '' }
388+
)
389+
expect(payload.entitlements).toEqual([])
390+
})
391+
375392
beforeEach(() => {
376393
mockTrackChatUpload.mockResolvedValue({ displayName: 'payroll.xlsx' })
377394
mockSecretNames.mockResolvedValue({ names: [] })

0 commit comments

Comments
 (0)