Skip to content

Commit fdee6cc

Browse files
tannerlinsleytannerlinsley
authored
perf(chat): render sends immediately and reduce response latency (#1339)
* fix(chat): clear selected message when switching conversations * fix(chat): exempt verified admins from included spending caps * perf(chat): parallelize authorized response preparation * perf(chat): show sends immediately and reduce response preparation
1 parent 0113dd2 commit fdee6cc

20 files changed

Lines changed: 1048 additions & 96 deletions

‎harness-tests/core/conversation-http.test.ts‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,38 @@ it('rejects ambiguous workspace selectors without accessing storage', async () =
157157
expect(mocks.resolveIdentity).not.toHaveBeenCalled()
158158
})
159159

160+
it.each(['signed-out', 'locked'] as const)(
161+
'rejects %s sends before looking up the conversation object',
162+
async (access) => {
163+
mocks.currentUser.mockResolvedValue(
164+
access === 'signed-out' ? null : { userId: 'user', capabilities: [] },
165+
)
166+
const req = new Request(request().url, {
167+
method: 'POST',
168+
headers: { Origin: 'https://tanstack.com' },
169+
body: JSON.stringify({ text: 'hello', messageId: 'message' }),
170+
})
171+
expect((await handleConversationSend(req, 'c', 'send')).status).toBe(
172+
access === 'signed-out' ? 401 : 403,
173+
)
174+
expect(mocks.resolveIdentity).not.toHaveBeenCalled()
175+
expect(mocks.getByName).not.toHaveBeenCalled()
176+
expect(mocks.begin).not.toHaveBeenCalled()
177+
},
178+
)
179+
it('rejects sends whose ownership check fails before accessing the object', async () => {
180+
mocks.resolveIdentity.mockRejectedValue(new Error('authorization failed'))
181+
const req = new Request(request().url, {
182+
method: 'POST',
183+
headers: { Origin: 'https://tanstack.com' },
184+
body: JSON.stringify({ text: 'hello', messageId: 'message' }),
185+
})
186+
await expect(handleConversationSend(req, 'c', 'send')).rejects.toThrow(
187+
'authorization failed',
188+
)
189+
expect(mocks.getByName).not.toHaveBeenCalled()
190+
expect(mocks.begin).not.toHaveBeenCalled()
191+
})
160192
it('denies cross-origin submission before looking up the account', async () => {
161193
const req = new Request(request().url, {
162194
method: 'POST',
@@ -201,6 +233,7 @@ it('uses server identity and context rather than request-supplied values', async
201233
}),
202234
})
203235
expect((await handleConversationSend(req, 'c', 'send')).status).toBe(200)
236+
expect(mocks.bind).not.toHaveBeenCalled()
204237
expect(mocks.begin.mock.calls[0][0]).toMatchObject({
205238
userId: 'user',
206239
fixture: false,

‎harness-tests/core/conversation-location.test.ts‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
import { describe, expect, it } from 'vitest'
2-
import { conversationLocation } from '../../src/chat/core/conversation-destination'
2+
import {
3+
conversationLocation,
4+
selectedConversationLocation,
5+
} from '../../src/chat/core/conversation-destination'
36
import { fileViewPath } from '../../src/chat/core/files'
47
import {
58
signInDestination,
@@ -66,3 +69,18 @@ describe('canonical conversation links', () => {
6669
expect(signInDestination('https://evil.example/chat/c/room')).toBe('/')
6770
})
6871
})
72+
73+
it('selects another conversation without carrying the previous message path', () => {
74+
const result = selectedConversationLocation(
75+
{ workspaceId: 'one', botId: 'other', conversationId: 'other-room' },
76+
validateWorkspaceSearch({
77+
message: 'previous-message',
78+
panel: 'usage',
79+
details: true,
80+
}),
81+
)
82+
expect(result.to).toBe('/chat/c/$conversationId')
83+
expect(result.params).toEqual({ conversationId: 'other-room' })
84+
expect(result.search).not.toHaveProperty('message')
85+
expect(result.search.panel).toBe('usage')
86+
})

‎harness-tests/core/mcp-connections-preparation.test.ts‎

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
import { beforeEach, expect, it, vi } from 'vitest'
2+
import { defaultPolicy } from '../../src/chat/core/types'
3+
4+
const mocks = vi.hoisted(() => ({
5+
policy: vi.fn(),
6+
credentials: vi.fn(),
7+
accounts: vi.fn(),
8+
plugins: vi.fn(),
9+
runtimeAccounts: vi.fn(),
10+
kody: vi.fn(),
11+
}))
12+
vi.mock('../../src/chat/workspace-policy.server', () => ({
13+
readWorkspacePolicy: mocks.policy,
14+
}))
15+
vi.mock('../../src/chat/server/credentials', () => ({
16+
readCredentials: mocks.credentials,
17+
}))
18+
vi.mock('../../src/chat/server/mcp-accounts', () => ({
19+
McpAccounts: class {
20+
configuredServers = mocks.accounts
21+
},
22+
}))
23+
vi.mock('../../src/chat/server/plugin-connections', () => ({
24+
pluginMcpConnections: mocks.plugins,
25+
}))
26+
vi.mock('../../src/chat/server/mcp-account-runtime', () => ({
27+
runtimeMcpAccounts: mocks.runtimeAccounts,
28+
}))
29+
vi.mock('../../src/chat/server/kody', () => ({ kodyConnection: mocks.kody }))
30+
import { connectedMcpServers } from '../../src/chat/server/mcp-connections'
31+
32+
const env = {
33+
ENCRYPTION_KEY: 'synthetic-fixture',
34+
KODY_ORIGIN: 'https://example.test',
35+
}
36+
const scope = { workspaceId: 'fixture' }
37+
beforeEach(() => {
38+
vi.resetAllMocks()
39+
mocks.policy.mockResolvedValue(defaultPolicy)
40+
mocks.credentials.mockResolvedValue(null)
41+
mocks.accounts.mockResolvedValue([])
42+
mocks.plugins.mockResolvedValue([])
43+
mocks.runtimeAccounts.mockResolvedValue([])
44+
})
45+
46+
it('lists authorized accounts while the independent credential read is pending', async () => {
47+
let release = () => {}
48+
const gate = new Promise<null>((resolve) => {
49+
release = () => resolve(null)
50+
})
51+
mocks.credentials.mockReturnValue(gate)
52+
const result = connectedMcpServers(
53+
env,
54+
'fixture-user',
55+
defaultPolicy,
56+
undefined,
57+
scope,
58+
)
59+
try {
60+
await vi.waitFor(() => expect(mocks.accounts).toHaveBeenCalledOnce())
61+
release()
62+
await expect(result).resolves.toEqual([])
63+
} finally {
64+
release()
65+
await result
66+
}
67+
})
68+
69+
it('does not read credentials or accounts when workspace authorization fails', async () => {
70+
mocks.policy.mockRejectedValue(new Error('Synthetic revoked membership'))
71+
await expect(
72+
connectedMcpServers(env, 'fixture-user', defaultPolicy, undefined, scope),
73+
).resolves.toEqual([])
74+
expect(mocks.credentials).not.toHaveBeenCalled()
75+
expect(mocks.accounts).not.toHaveBeenCalled()
76+
})
77+
78+
it('does not read Kody credentials when that capability is disabled', async () => {
79+
await expect(
80+
connectedMcpServers(
81+
env,
82+
'fixture-user',
83+
{ ...defaultPolicy, allowKody: false },
84+
undefined,
85+
scope,
86+
),
87+
).resolves.toEqual([])
88+
expect(mocks.credentials).not.toHaveBeenCalled()
89+
expect(mocks.accounts).toHaveBeenCalledOnce()
90+
})
91+
92+
it('settles the account read before reporting a credential failure', async () => {
93+
let release = () => {}
94+
const gate = new Promise<[]>((resolve) => {
95+
release = () => resolve([])
96+
})
97+
mocks.accounts.mockReturnValue(gate)
98+
mocks.credentials.mockRejectedValue(new Error('Synthetic credential failure'))
99+
let settled = false
100+
const result = connectedMcpServers(
101+
env,
102+
'fixture-user',
103+
defaultPolicy,
104+
undefined,
105+
scope,
106+
)
107+
const observed = result.then(
108+
() => {
109+
settled = true
110+
},
111+
() => {
112+
settled = true
113+
},
114+
)
115+
try {
116+
await vi.waitFor(() => expect(mocks.accounts).toHaveBeenCalledOnce())
117+
expect(settled).toBe(false)
118+
release()
119+
await expect(result).rejects.toThrow('Synthetic credential failure')
120+
} finally {
121+
release()
122+
await observed
123+
}
124+
})
125+
126+
it('resolves a selected Kody connection with fresh runtime authorization', async () => {
127+
mocks.credentials.mockResolvedValue({
128+
kody: { access_token: 'metadata-token' },
129+
})
130+
mocks.kody.mockResolvedValue({
131+
id: 'kody',
132+
label: 'Kody',
133+
url: 'https://example.test/mcp',
134+
accessToken: 'fresh-runtime-token',
135+
})
136+
const result = await connectedMcpServers(
137+
env,
138+
'fixture-user',
139+
defaultPolicy,
140+
'kody',
141+
scope,
142+
)
143+
expect(result).toHaveLength(1)
144+
expect(result[0].accessToken).toBe('fresh-runtime-token')
145+
expect(mocks.kody).toHaveBeenCalledWith(env, 'fixture-user')
146+
expect(mocks.accounts).not.toHaveBeenCalled()
147+
})
148+
149+
it('does not read unrelated Kody credentials when resolving a selected MCP account', async () => {
150+
await expect(
151+
connectedMcpServers(
152+
env,
153+
'fixture-user',
154+
defaultPolicy,
155+
'mcp:fixture-account',
156+
scope,
157+
),
158+
).resolves.toEqual([])
159+
expect(mocks.credentials).not.toHaveBeenCalled()
160+
expect(mocks.runtimeAccounts).toHaveBeenCalledWith(
161+
env,
162+
{ workspaceId: 'fixture', userId: 'fixture-user' },
163+
{ id: 'fixture-account' },
164+
)
165+
})

‎harness-tests/core/run-models.test.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,33 @@ beforeEach(() => {
7373
})
7474

7575
describe('run model configuration', () => {
76+
it('reuses credentials already read for this run without a second database query', async () => {
77+
configured('openai', 'gpt-5-mini', { apiKey: 'fresh-server-key' })
78+
const result = await resolveRunModel(env, scope, state.credentials)
79+
expect(result.connection.apiKey).toBe('fresh-server-key')
80+
expect(result.selection.provider).toBe('openai')
81+
expect(state.read).not.toHaveBeenCalled()
82+
})
83+
84+
it('treats an already-read missing credential row as included without rereading', async () => {
85+
configured('openai', 'gpt-5-mini')
86+
const result = await resolveRunModel(env, scope, null)
87+
expect(result.selection.provider).toBe('included')
88+
expect(state.read).not.toHaveBeenCalled()
89+
})
90+
91+
it('keeps fixture isolation when a caller supplies personal credentials', async () => {
92+
configured('openai', 'gpt-5-mini')
93+
const result = await resolveRunModel(
94+
env,
95+
{ ...scope, fixture: true },
96+
state.credentials,
97+
)
98+
expect(result.selection.provider).toBe('included')
99+
expect(result.connection.apiKey).toBeUndefined()
100+
expect(state.read).not.toHaveBeenCalled()
101+
})
102+
76103
it('uses the configured included model and preserves its existing default behavior', async () => {
77104
const result = await resolveRunModel(env, scope)
78105
expect(result.selection).toEqual(select('included', env.INCLUDED_MODEL))

0 commit comments

Comments
 (0)