Skip to content

Commit aced5f7

Browse files
authored
fix: address release review findings (#8276)
* fix(desktop): admit browser download saves before reading the body PUT /api/desktop/tool/file buffered up to 100 MiB of the request body before the use case checked that the tool call exists, is a running browser_save_download claimed by the desktop, and belongs to the caller's run, so any signed-in user could force large reads with bogus toolCallIds. Add admitBrowserDownloadSave (same operation and binding resolution as saveBrowserDownload, no claim or audit), following the existing admitCreateWorkspaceFile pattern, and run it before the body read. The save use case still re-validates and claims atomically after the read, so a failed or oversized upload never burns the single-use claim. * fix(desktop): walk cross-realm shadow roots when resolving upload targets resolveFileInputTarget climbed out of shadow roots with `root instanceof ShadowRoot`. Elements in same-origin iframes belong to the frame's realm, so the check was false for their shadow roots and the ancestor walk stopped at the shadow boundary, failing browser_upload_file with "no nearby file input". Use the file's duck-typed `'host' in root` idiom like every other shadow-host hop. * fix(chat): keep an activity's completed title off runs while it still waits on the user Every run of a main-lane activity segment received the segment-wide activity, including its completedTitle. A finished multi-call run before a pending approval or terminal handoff (which splits the segment into runs) therefore read the past-tense completed title while the activity was still unfinished. Runs now receive the completed title only once every call in the segment has finished; until then they keep the activity's in-progress title and summarize their own calls. Fully finished activities render exactly as before. * fix(search): keep partial coverage through persisted search compaction compactRetrievalCitations kept only citations, dropping data.retrieval, and stripToolResultOutput applies it on both save and load. A reloaded timed-out search with no matches therefore rendered "No results", which the live UI and the tool itself deliberately avoid because partial results cannot establish absence. Compaction now keeps a bounded retrieval: { status: 'partial' } marker (never the full retrieval object), and re-compacting compacted output keeps it. Complete searches compact exactly as before. * fix(organization): route chat URLs the same way as organization Home The chat page gated on (mothershipAvailable || memberScoped) while OrganizationHome renders nothing without copilot.use, or without Build and Search. A viewer with copilot.use but neither Build nor Search, and a Search-only viewer opening an assistant chat, got a blank page. Apply Home's two redirects (Search when copilot.use is denied, workspace settings when neither Build nor Search is allowed) before loading the chat, through one getOrganizationHomeRedirect shared by both pages so they cannot drift again; the later copilot.use redirect becomes unreachable and is removed. * fix(organization): keep a custom date range when Home restores Search results organizationHomeParsers already reads from/to, but OrganizationHomeContent passed only source and updated to searchFiltersFromParams, so a Home URL with updated=custom restored an unbounded search. Pass from/to through, matching the workspace results view, and add them to the effect deps. * fix(chat): project desktop tabs in organization Assistant chats Organization Home passed projectsDesktopTabs: false to useChat in Assistant mode, which only nulls the native active-tab ids used for the strip's fallback. ChatResourcePanel still projects the chat scope's browser and terminal tabs in every mode, and chat link clicks in the desktop app open those tabs through the same projection, so Assistant chats showed desktop tabs while ignoring which one the desktop app remembers (the #7793 reopen behavior). Build Home's options with getMothershipUseChatOptions, like workspace Home, so tabs project consistently in every mode. Assistant requests still attach no resources. * fix(home): make the chat/resource panel divider keyboard-adjustable The resource panel divider (moved into the shared ChatPanelLayout this release, identical to main's home.tsx) was pointer-only: a separator with no tabIndex, key handling, or aria-value*, so keyboard users could not focus or resize it. Mirror the file text-editor split: the divider is now a focusable separator with ArrowLeft/ArrowRight steps and Home/End. Both separators now read keys through one readSeparatorKey helper (modifier and IME guard included). Keyboard widths go through the same MIN/max clamps as the drag (keyboardPanelWidth next to panelWidthAt), land without the width transition like the window-resize clamp (shared writeWidthInstantly), are ignored during a live drag, and claim the resource view like a pointer resize. aria-valuenow/min/ max are written imperatively on focus, key, and drag end, preserving the hook's zero-render resize design. Focus-visible outline matches the text-editor split. * docs(rules): scope member avatar aria-hidden to rows that show the name The settings-pages rule said every member avatar renders aria-hidden because the name is always beside it. MemberRow shows only the email, so its avatar correctly stays labelled (role=img, aria-label=name), matching the emcn Avatar TSDoc. Reword the rule and regenerate the Cursor projection. * docs(skills): clarify service-mode selector guidance in add-connector The skill said service mode requires shared selectors, but GitLab (service mode only) uses plain host/project inputs. Align with the live-search README: verification is mandatory; shared selectors apply only to resource pickers. * fix(home): keep a focused divider's value current through window resizes The window-resize clamp updated the panel width but not the focused divider's aria-valuemax/valuenow, so assistive tech kept the old bounds until the divider was focused again. The clamp now also reports to the divider while it holds focus, including when the pinned width stays within the new bounds.
1 parent 0ede40b commit aced5f7

32 files changed

Lines changed: 861 additions & 202 deletions

‎.agents/skills/add-connector/SKILL.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ argument-hint: <service-name> [api-docs-url]
88

99
## Choose the connector runtime first
1010

11-
For **Sim Search**, use the live provider workflow in [the federated Search developer guide](../../../apps/sim/lib/sim-search/live/README.md#adding-a-live-search-connector). Its browser-safe provider catalog owns provider IDs, API origins, credential aliases, and account modes; its typed runtime registry requires both search and read handlers. `ConnectorMeta` remains the owner of logos and setup fields. Member mode has no admin resource filters. Service mode requires independent live source verification and shared selectors. Do not implement a Search source by adding a crawler, embeddings, or a scheduled ACL build.
11+
For **Sim Search**, use the live provider workflow in [the federated Search developer guide](../../../apps/sim/lib/sim-search/live/README.md#adding-a-live-search-connector). Its browser-safe provider catalog owns provider IDs, API origins, credential aliases, and account modes; its typed runtime registry requires both search and read handlers. `ConnectorMeta` remains the owner of logos and setup fields. Member mode has no admin resource filters. Service mode requires independent live source verification; resource pickers use shared selectors with canonical manual-input pairs, and plain inputs are fine where no picker applies. Do not implement a Search source by adding a crawler, embeddings, or a scheduled ACL build.
1212

1313
The ingestion instructions below apply to **ordinary knowledge-base connectors** and the explicit legacy Search backend (`SIM_SEARCH_LIVE=false`). If a provider supports both, implement and test both runtimes; adding `search: true` to metadata alone does not implement federated search. Preserve indexing documentation and behavior for those KB/legacy callers.
1414

‎.claude/rules/sim-settings-pages.md‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -252,11 +252,12 @@ and — on activatable rows only — the hover band. Never hand-roll any of it,
252252

253253

254254
**One member avatar.** Every member list, owner cell, and ranking renders emcn
255-
`<Avatar size='xs' name={…} src={…} aria-hidden />` — a 14px photo, or the initial
256-
on the neutral disc (`aria-hidden` because the name is always beside it). Never
257-
hand-roll an avatar or give a person a `getUserColor` hash; per-person colors
258-
belong to live collaboration (presence, cursors), where the color matches that
259-
person's cursor.
255+
`<Avatar size='xs' name={…} src={…} />` — a 14px photo, or the initial on the
256+
neutral disc. Pass `aria-hidden` only when the member's name is visibly rendered
257+
beside it; an email-only row (`MemberRow`) keeps the avatar labelled because it
258+
carries the name. Never hand-roll an avatar or give a person a `getUserColor`
259+
hash; per-person colors belong to live collaboration (presence, cursors), where
260+
the color matches that person's cursor.
260261

261262
## Header action order
262263

‎.cursor/rules/sim-settings-pages.mdc‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -249,11 +249,12 @@ and — on activatable rows only — the hover band. Never hand-roll any of it,
249249

250250

251251
**One member avatar.** Every member list, owner cell, and ranking renders emcn
252-
`<Avatar size='xs' name={…} src={…} aria-hidden />` — a 14px photo, or the initial
253-
on the neutral disc (`aria-hidden` because the name is always beside it). Never
254-
hand-roll an avatar or give a person a `getUserColor` hash; per-person colors
255-
belong to live collaboration (presence, cursors), where the color matches that
256-
person's cursor.
252+
`<Avatar size='xs' name={…} src={…} />` — a 14px photo, or the initial on the
253+
neutral disc. Pass `aria-hidden` only when the member's name is visibly rendered
254+
beside it; an email-only row (`MemberRow`) keeps the avatar labelled because it
255+
carries the name. Never hand-roll an avatar or give a person a `getUserColor`
256+
hash; per-person colors belong to live collaboration (presence, cursors), where
257+
the color matches that person's cursor.
257258

258259
## Header action order
259260

‎apps/desktop/src/main/browser-agent/page-functions.test.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -912,6 +912,21 @@ describe('collectSnapshot', () => {
912912
expect(captured.input.ownerDocument).toBe(document)
913913
})
914914

915+
it('climbs out of a shadow root inside a same-origin frame to find the file input', () => {
916+
const frame = document.createElement('iframe')
917+
document.body.append(frame)
918+
const childDocument = frame.contentDocument as Document
919+
childDocument.body.innerHTML = '<div id="host"></div><input type="file">'
920+
const host = childDocument.getElementById('host') as HTMLElement
921+
const button = childDocument.createElement('button')
922+
host.attachShadow({ mode: 'open' }).append(button)
923+
const input = childDocument.querySelector('input') as HTMLInputElement
924+
register(button)
925+
926+
expect(button.getRootNode()).not.toBeInstanceOf(ShadowRoot)
927+
expect(resolveFileInputTarget(0)).toEqual({ input, document: childDocument })
928+
})
929+
915930
it('refuses a disconnected or stale upload reference', () => {
916931
const input = document.createElement('input')
917932
input.type = 'file'

‎apps/desktop/src/main/browser-agent/page-functions.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3348,7 +3348,7 @@ export function resolveFileInputTarget(id: number): {
33483348
)
33493349
if (candidates.length === 1) input = candidates[0]
33503350
const root = scope.getRootNode()
3351-
scope = scope.parentElement ?? (root instanceof ShadowRoot ? root.host : null)
3351+
scope = scope.parentElement ?? ('host' in root ? (root.host as Element) : null)
33523352
}
33533353
if (!input) throw new Error('The selected element has no nearby file input.')
33543354
if (input.matches(':disabled')) throw new Error('The file input is disabled.')

‎apps/sim/app/api/desktop/tool/file/route.test.ts‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,14 @@ import { NextRequest } from 'next/server'
66
import { beforeEach, describe, expect, it, vi } from 'vitest'
77
import { OrchestrationError } from '@/lib/core/orchestration/types'
88

9-
const { mockRead, mockSave } = vi.hoisted(() => ({ mockRead: vi.fn(), mockSave: vi.fn() }))
9+
const { mockAdmit, mockRead, mockSave } = vi.hoisted(() => ({
10+
mockAdmit: vi.fn(),
11+
mockRead: vi.fn(),
12+
mockSave: vi.fn(),
13+
}))
1014

1115
vi.mock('@/lib/browser-agent/application/browser-file-transfer', () => ({
16+
admitBrowserDownloadSave: mockAdmit,
1217
readBrowserUploadFile: {
1318
operation: { id: 'files.read_content', minimumRole: 'read', workspaceApiKey: 'allow' },
1419
execute: mockRead,
@@ -43,6 +48,7 @@ describe('/api/desktop/tool/file', () => {
4348
beforeEach(() => {
4449
vi.clearAllMocks()
4550
mockGetSession.mockResolvedValue(session)
51+
mockAdmit.mockResolvedValue(undefined)
4652
mockRead.mockResolvedValue({ file: { name: 'Q3 plan.pdf' }, content: Buffer.from('%PDF') })
4753
mockSave.mockResolvedValue({
4854
file: { name: 'report.csv', size: 3, folderPath: null, vfsNamespace: 'files' },
@@ -92,6 +98,21 @@ describe('/api/desktop/tool/file', () => {
9298
put('toolCallId=c&name=a.csv', new Uint8Array(1), { 'content-length': String(2 ** 31) })
9399
)
94100
expect(oversized.status).toBe(413)
101+
expect(mockAdmit).not.toHaveBeenCalled()
102+
expect(mockSave).not.toHaveBeenCalled()
103+
})
104+
105+
it('refuses an unadmitted save without reading its body', async () => {
106+
mockAdmit.mockRejectedValueOnce(
107+
new OrchestrationError('not_found', 'Browser file transfer not found')
108+
)
109+
const request = put('toolCallId=unknown&name=a.csv', new Uint8Array([1, 2, 3]))
110+
111+
const res = await PUT(request)
112+
113+
expect(res.status).toBe(404)
114+
expect(mockAdmit).toHaveBeenCalledWith(principal, { toolCallId: 'unknown', name: 'a.csv' })
115+
expect(request.bodyUsed).toBe(false)
95116
expect(mockSave).not.toHaveBeenCalled()
96117
})
97118

‎apps/sim/app/api/desktop/tool/file/route.ts‎

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
} from '@/lib/api/server/routes'
1414
import { withRequestId } from '@/lib/api/server/routes/request-id'
1515
import {
16+
admitBrowserDownloadSave,
1617
readBrowserUploadFile,
1718
saveBrowserDownload,
1819
} from '@/lib/browser-agent/application/browser-file-transfer'
@@ -52,8 +53,9 @@ export const POST = defineInternalBinaryRoute({
5253
* PUT /api/desktop/tool/file?toolCallId=…&name=…
5354
*
5455
* Raw `withRouteHandler`: the body is the download's bytes, so it is admitted by declared length
55-
* and read under a byte ceiling only after the session is authenticated, then handed to the
56-
* application use case that binds it to its claimed `browser_save_download` call.
56+
* and read under a byte ceiling only after the session is authenticated and the application has
57+
* admitted the caller's claimed `browser_save_download` call, then handed to the use case that
58+
* re-validates and claims that call.
5759
*/
5860
export const PUT = withRouteHandler(async (request: NextRequest) => {
5961
let principal
@@ -73,6 +75,13 @@ export const PUT = withRouteHandler(async (request: NextRequest) => {
7375
}
7476
const parsed = await parseRequest(saveBrowserDownloadContract, request, {})
7577
if (!parsed.success) return parsed.response
78+
const { toolCallId, name } = parsed.data.query
79+
80+
try {
81+
await admitBrowserDownloadSave(principal, { toolCallId, name })
82+
} catch (error) {
83+
return saveErrorResponse(error)
84+
}
7685

7786
let content: Buffer
7887
try {
@@ -93,16 +102,21 @@ export const PUT = withRouteHandler(async (request: NextRequest) => {
93102
try {
94103
const { file } = await saveBrowserDownload.execute({
95104
principal,
96-
input: { toolCallId: parsed.data.query.toolCallId, name: parsed.data.query.name, content },
105+
input: { toolCallId, name, content },
97106
request,
98107
})
99108
return NextResponse.json({ path: workspaceFileVfsPath(file), name: file.name, size: file.size })
100109
} catch (error) {
101-
const response = internalFileErrorPolicies.concealContentAuthorization.project(error)
102-
if (!response) throw error
103-
return NextResponse.json(withRequestId(response.body), {
104-
status: response.status,
105-
headers: response.headers,
106-
})
110+
return saveErrorResponse(error)
107111
}
108112
})
113+
114+
/** Projects a typed download-save failure, rethrowing anything the policy does not classify. */
115+
function saveErrorResponse(error: unknown): NextResponse {
116+
const response = internalFileErrorPolicies.concealContentAuthorization.project(error)
117+
if (!response) throw error
118+
return NextResponse.json(withRequestId(response.body), {
119+
status: response.status,
120+
headers: response.headers,
121+
})
122+
}

‎apps/sim/app/o/[organizationId]/chat/[chatId]/page.tsx‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { getAccessibleCopilotChatAuth } from '@/lib/mothership/chat/lifecycle'
66
import { WORKSPACE_SETTINGS_PATH } from '@/lib/navigation/paths'
77
import { getOrganizationSurfaceContext } from '@/lib/organizations/surface'
88
import OrganizationChatLoading from '@/app/o/[organizationId]/chat/[chatId]/loading'
9+
import { getOrganizationHomeRedirect } from '@/app/o/[organizationId]/home/home-redirect'
910
import { OrganizationHome } from '@/app/o/[organizationId]/home/organization-home'
1011

1112
export const metadata: Metadata = { title: 'Chat' }
@@ -20,8 +21,8 @@ export default async function OrganizationChatPage({
2021
if (!session?.user?.id) notFound()
2122
const context = await getOrganizationSurfaceContext(organizationId, session.user.id)
2223
if (!context) notFound()
23-
if (!context.mothershipAvailable && !context.searchAccess.memberScoped)
24-
redirect(WORKSPACE_SETTINGS_PATH)
24+
const homeRedirect = getOrganizationHomeRedirect(context, organizationId)
25+
if (homeRedirect) redirect(homeRedirect)
2526
const chat = await getAccessibleCopilotChatAuth(chatId, session.user.id, {
2627
principal: { kind: 'session', userId: session.user.id, sessionId: session.session.id },
2728
})
@@ -38,7 +39,6 @@ export default async function OrganizationChatPage({
3839
</Suspense>
3940
)
4041
}
41-
if (!context.mothershipAvailable) redirect(WORKSPACE_SETTINGS_PATH)
4242
return (
4343
<Suspense fallback={<OrganizationChatLoading />}>
4444
<OrganizationHome
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import { organizationRoutes, WORKSPACE_SETTINGS_PATH } from '@/lib/navigation/paths'
2+
import type { OrganizationSurfaceContext } from '@/lib/organizations/surface'
3+
4+
/**
5+
* Where organization Home and its chat URLs send a viewer Home cannot serve, or `null` when
6+
* Home renders. Search when Chat is off but member Search is on; workspace settings when the
7+
* viewer can neither both chat and build, nor search as a member.
8+
*/
9+
export function getOrganizationHomeRedirect(
10+
context: Pick<OrganizationSurfaceContext, 'mothershipAvailable' | 'canBuild' | 'searchAccess'>,
11+
organizationId: string
12+
): string | null {
13+
if (!context.mothershipAvailable && context.searchAccess.memberScoped) {
14+
return organizationRoutes(organizationId).search
15+
}
16+
if (!(context.mothershipAvailable && context.canBuild) && !context.searchAccess.memberScoped) {
17+
return WORKSPACE_SETTINGS_PATH
18+
}
19+
return null
20+
}

‎apps/sim/app/o/[organizationId]/home/organization-home.test.tsx‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,10 @@ vi.mock('@/lib/core/utils/browser-storage', () => ({
6969
vi.mock('@/app/o/[organizationId]/providers/organization-provider', () => ({
7070
useOrganizationContext: mocks.context,
7171
}))
72-
vi.mock('@/app/workspace/[workspaceId]/home/hooks/use-chat', () => ({ useChat: mocks.chat }))
72+
vi.mock('@/app/workspace/[workspaceId]/home/hooks/use-chat', () => ({
73+
getMothershipUseChatOptions: (options: object) => ({ ...options, mothership: true }),
74+
useChat: mocks.chat,
75+
}))
7376
vi.mock('@/hooks/queries/mothership-chats', () => ({
7477
useMarkMothershipChatRead: () => ({ mutate: mocks.markRead }),
7578
}))
@@ -602,7 +605,7 @@ describe('Home permission-selected harness', () => {
602605
expect(mocks.chat).toHaveBeenLastCalledWith(
603606
{ organizationId: 'organization-a' },
604607
'search-a',
605-
expect.objectContaining({ requestMode: 'assistant', projectsDesktopTabs: false })
608+
expect.objectContaining({ requestMode: 'assistant', mothership: true })
606609
)
607610
})
608611
it('keeps old Build history readable after permission removal without a writable composer', async () => {
@@ -892,6 +895,23 @@ it('restores an explicitly selected search panel on empty Home without reusing t
892895
expect(mocks.send).not.toHaveBeenCalled()
893896
})
894897

898+
it('restores a custom date range with the search panel', async () => {
899+
mocks.activeResource = 'search:organization:organization-a'
900+
await act(async () =>
901+
renderHome(<OrganizationHome />, '?q=Orion&updated=custom&from=2026-09-01&to=2026-09-03')
902+
)
903+
expect(mocks.addResource).toHaveBeenCalledWith(
904+
expect.objectContaining({
905+
search: expect.objectContaining({
906+
filters: {
907+
modifiedAfter: new Date(2026, 8, 1).toISOString(),
908+
modifiedBefore: new Date(2026, 8, 4, 0, 0, 0, -1).toISOString(),
909+
},
910+
}),
911+
})
912+
)
913+
})
914+
895915
it('does not reopen closed search results merely because a query remains in the URL', async () => {
896916
await act(async () => renderHome(<OrganizationHome />, '?searchLevel=adaptive&q=Orion'))
897917
expect(mocks.addResource).not.toHaveBeenCalled()

0 commit comments

Comments
 (0)