Skip to content

Commit 3a0dbb9

Browse files
committed
fix(managed-agents): harden reconnect loop; fix credential-picker regression
- drive completion off terminal events + authoritative session status (drop the fragile busy-clock) - drain full event history so a long session's tail is never cut off - skip idless events in catch-up; require an id before replying to custom_tool_use - gate the shared credential-selector service-account lookup on credentialKind and use the non-throwing helper (was crashing multi-service OAuth pickers) - audit-log the list route's credential access; refresh stale tool docs
1 parent ccdb679 commit 3a0dbb9

9 files changed

Lines changed: 203 additions & 340 deletions

File tree

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/credential-selector/credential-selector.tsx

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import {
1212
type OAuthProvider,
1313
parseProvider,
1414
} from '@/lib/oauth'
15-
import { getMissingRequiredScopes, getServiceByProviderAndId } from '@/lib/oauth/utils'
15+
import { getMissingRequiredScopes, getServiceConfigByServiceId } from '@/lib/oauth/utils'
1616
import { ConnectOAuthModal } from '@/app/workspace/[workspaceId]/components/connect-oauth-modal'
1717
import {
1818
ConnectServiceAccountModal,
@@ -117,10 +117,11 @@ export function CredentialSelector({
117117
}, [rawCredentials, isTriggerMode, credentialKind, subBlock.allowServiceAccounts])
118118

119119
// Resolved service-account provider metadata for the token-paste connect
120-
// modal (only used when `credentialKind === 'service-account'`).
120+
// modal. Gated on `credentialKind` and using the non-throwing lookup so it
121+
// never runs (or throws) for the OAuth / custom-bot pickers.
121122
const serviceAccountService = useMemo(
122-
() => (serviceId ? getServiceByProviderAndId(provider, serviceId) : undefined),
123-
[provider, serviceId]
123+
() => (credentialKind === 'service-account' ? getServiceConfigByServiceId(serviceId) : null),
124+
[credentialKind, serviceId]
124125
)
125126

126127
const selectedCredential = useMemo(

apps/sim/app/workspace/[workspaceId]/w/components/sidebar/hooks/use-drag-drop.test.tsx

Lines changed: 0 additions & 124 deletions
This file was deleted.

apps/sim/app/workspace/[workspaceId]/w/components/sidebar/hooks/use-drag-drop.ts

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -649,21 +649,11 @@ export function useDragDrop(options: UseDragDropOptions = {}) {
649649
if (target && container.contains(target)) return
650650
handleDragEnd()
651651
}
652-
/**
653-
* `dragend` always fires on the drag source at the end of any drag operation, including
654-
* Esc-cancels and drops on non-droppable targets. Without this reset, a non-sidebar drag
655-
* that entered the list (flipping `isDragging` on via `initDragOver`) but ended without a
656-
* `drop` inside the container would strand `isDragging` at `true` — leaving the absolutely
657-
* positioned edge drop zones mounted over the first/last rows and stealing their grab band.
658-
*/
659-
const onWindowDragEnd = () => handleDragEnd()
660652
container.addEventListener('dragleave', onLeave)
661653
window.addEventListener('drop', onWindowDrop, true)
662-
window.addEventListener('dragend', onWindowDragEnd, true)
663654
return () => {
664655
container.removeEventListener('dragleave', onLeave)
665656
window.removeEventListener('drop', onWindowDrop, true)
666-
window.removeEventListener('dragend', onWindowDragEnd, true)
667657
}
668658
}, [isDragging, handleDragEnd])
669659

apps/sim/lib/managed-agents/run-session.test.ts

Lines changed: 57 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ const { mocks } = vi.hoisted(() => ({
1111
sendSessionEvents: vi.fn(),
1212
openSessionStream: vi.fn(),
1313
listSessionEvents: vi.fn(),
14-
getSessionUsage: vi.fn(),
14+
getSession: vi.fn(),
1515
readSSEEvents: vi.fn(),
1616
sleep: vi.fn(),
1717
},
@@ -23,7 +23,7 @@ vi.mock('@/lib/managed-agents/session-client', () => ({
2323
sendSessionEvents: mocks.sendSessionEvents,
2424
openSessionStream: mocks.openSessionStream,
2525
listSessionEvents: mocks.listSessionEvents,
26-
getSessionUsage: mocks.getSessionUsage,
26+
getSession: mocks.getSession,
2727
}))
2828
vi.mock('@/lib/core/utils/sse', () => ({ readSSEEvents: mocks.readSSEEvents }))
2929
vi.mock('@sim/utils/helpers', () => ({ sleep: mocks.sleep }))
@@ -56,74 +56,84 @@ const msg = (id: string, text: string): AnthropicSessionEvent => ({
5656
type: 'agent.message',
5757
content: [{ type: 'text', text }],
5858
})
59+
const idle = (id: string, stop: string): AnthropicSessionEvent => ({
60+
id,
61+
type: 'session.status_idle',
62+
stop_reason: { type: stop },
63+
})
5964

6065
beforeEach(() => {
6166
vi.clearAllMocks()
6267
mocks.createSession.mockResolvedValue({ id: 'sess_1' })
6368
mocks.sendUserMessage.mockResolvedValue(undefined)
6469
mocks.openSessionStream.mockResolvedValue({})
65-
mocks.getSessionUsage.mockResolvedValue(null)
70+
mocks.listSessionEvents.mockResolvedValue([])
71+
mocks.getSession.mockResolvedValue(null)
6672
})
6773

6874
describe('runManagedAgentSession', () => {
69-
it('accumulates agent.message text and completes on end_turn', async () => {
70-
scriptStreamBatches([
71-
[
72-
msg('e1', 'Hello '),
73-
msg('e2', 'world'),
74-
{ id: 'e3', type: 'session.status_idle', stop_reason: { type: 'end_turn' } },
75-
],
76-
])
75+
it('accumulates agent.message text and completes on end_turn (terminal event)', async () => {
76+
scriptStreamBatches([[msg('e1', 'Hello '), msg('e2', 'world'), idle('e3', 'end_turn')]])
77+
mocks.getSession.mockResolvedValue({
78+
status: 'idle',
79+
usage: { inputTokens: 12, outputTokens: 3 },
80+
})
7781

7882
const result = await runManagedAgentSession({ ...BASE })
7983

80-
expect(result).toEqual({ ok: true, content: 'Hello world', sessionId: 'sess_1' })
84+
expect(result).toEqual({
85+
ok: true,
86+
content: 'Hello world',
87+
sessionId: 'sess_1',
88+
inputTokens: 12,
89+
outputTokens: 3,
90+
})
8191
expect(mocks.listSessionEvents).not.toHaveBeenCalled()
8292
})
8393

84-
it('surfaces cumulative token usage on success (best-effort)', async () => {
85-
scriptStreamBatches([
86-
[
87-
msg('e1', 'ok'),
88-
{ id: 'e2', type: 'session.status_idle', stop_reason: { type: 'end_turn' } },
89-
],
90-
])
91-
mocks.getSessionUsage.mockResolvedValue({ inputTokens: 120, outputTokens: 45 })
94+
it('completes via authoritative status when the stream goes quiet after progress', async () => {
95+
// Stream: some text, no terminal, then closes. Reconnect: nothing new.
96+
scriptStreamBatches([[msg('e1', 'partial')], []])
97+
// First getSession (quiet-reconnect check) → idle; final getSession → usage.
98+
mocks.getSession
99+
.mockResolvedValueOnce({ status: 'idle' })
100+
.mockResolvedValue({ status: 'idle', usage: { inputTokens: 5 } })
92101

93102
const result = await runManagedAgentSession({ ...BASE })
94103

95-
expect(result).toEqual({
96-
ok: true,
97-
content: 'ok',
98-
sessionId: 'sess_1',
99-
inputTokens: 120,
100-
outputTokens: 45,
101-
})
104+
expect(result.ok).toBe(true)
105+
expect(result.content).toBe('partial')
106+
expect(result.inputTokens).toBe(5)
107+
})
108+
109+
it('keeps waiting while status is running, then completes on a later terminal event', async () => {
110+
// Stream 1: text, closes (no terminal). Reconnect: nothing new, status running → backoff.
111+
// Stream 2: end_turn → complete.
112+
scriptStreamBatches([[msg('e1', 'thinking')], [idle('e2', 'end_turn')]])
113+
mocks.getSession
114+
.mockResolvedValueOnce({ status: 'running' })
115+
.mockResolvedValue({ status: 'idle' })
116+
117+
const result = await runManagedAgentSession({ ...BASE })
118+
119+
expect(result.ok).toBe(true)
120+
expect(result.content).toBe('thinking')
121+
expect(mocks.sleep).toHaveBeenCalled() // backed off while running
102122
})
103123

104-
it('does NOT false-timeout after requires_action followed by progress then a quiet reconnect', async () => {
105-
// Stream 1: only a requires_action idle (busy), then the stream closes.
106-
// Stream 2: closes immediately with nothing new.
107-
scriptStreamBatches([
108-
[{ id: 'r1', type: 'session.status_idle', stop_reason: { type: 'requires_action' } }],
109-
[],
110-
])
111-
// Catch-up 1 surfaces real progress (m2); catch-up 2 has nothing unseen.
112-
mocks.listSessionEvents
113-
.mockResolvedValueOnce([
114-
{ id: 'r1', type: 'session.status_idle', stop_reason: { type: 'requires_action' } },
115-
msg('m2', 'progress'),
116-
])
117-
.mockResolvedValueOnce([
118-
{ id: 'r1', type: 'session.status_idle', stop_reason: { type: 'requires_action' } },
119-
msg('m2', 'progress'),
120-
])
124+
it('does not complete on a fresh idle before the agent has started', async () => {
125+
// Stream closes immediately with nothing; catch-up empty; status idle but no
126+
// activity yet → must NOT complete. Then it starts and finishes.
127+
scriptStreamBatches([[], [idle('e1', 'end_turn')]])
128+
mocks.getSession
129+
.mockResolvedValueOnce({ status: 'idle' }) // pre-start idle — must be ignored
130+
.mockResolvedValue({ status: 'idle' })
121131

122132
const result = await runManagedAgentSession({ ...BASE })
123133

124-
expect(result).toEqual({ ok: true, content: 'progress', sessionId: 'sess_1' })
125-
// A false timeout would have slept on backoff and returned an error.
126-
expect(mocks.sleep).not.toHaveBeenCalled()
134+
expect(result.ok).toBe(true)
135+
// Completed via the real end_turn on reopen, not the premature idle.
136+
expect(mocks.openSessionStream).toHaveBeenCalledTimes(2)
127137
})
128138

129139
it('surfaces a session.error as a failure with the error message', async () => {
@@ -138,7 +148,6 @@ describe('runManagedAgentSession', () => {
138148

139149
it('rejects an empty user message before creating a session', async () => {
140150
const result = await runManagedAgentSession({ ...BASE, userMessage: ' ' })
141-
142151
expect(result.ok).toBe(false)
143152
expect(mocks.createSession).not.toHaveBeenCalled()
144153
})

0 commit comments

Comments
 (0)