Skip to content

Commit 7ffdfbf

Browse files
fix(copilot): service-account discovery must un-gate after the block GAs
Review round on #5786: describeServiceAccountForOAuthProvider used `getBlock(...)?.preview ?? true`, which treats a GA'd gating block — one that dropped its `preview` flag, exactly slack_v2's documented migration — as still gated, so the custom bot would stay omitted from VFS discovery forever after GA even though the UI shows it. Reuse the canonical isHiddenUnder(null, block) predicate instead, so a non-preview block is visible. Adds service-account-gate.test.ts covering preview → omit, GA → include, and missing → fail-closed with a mocked getBlock (the block registry is globally stubbed, so the real slack_v2 preview flag isn't observable through serializeIntegrationSchema).
1 parent 1d97ff0 commit 7ffdfbf

3 files changed

Lines changed: 59 additions & 8 deletions

File tree

apps/sim/lib/copilot/vfs/serializers.test.ts

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -316,13 +316,10 @@ describe('serializeIntegrationSchema — service-account auth', () => {
316316
expect(schema.auth.serviceAccount).toBeUndefined()
317317
})
318318

319-
it('omits serviceAccount when the flow is gated by a preview block (slack custom bot ↔ slack_v2)', () => {
320-
// slack_v2 is preview: true, so the shared schema must not leak the custom
321-
// bot — parallel to how preview tools stay out of the shared aggregates.
322-
const schema = JSON.parse(serializeIntegrationSchema(oauthTool('slack_send', 'slack')))
323-
expect(schema.auth.type).toBe('oauth')
324-
expect(schema.auth.serviceAccount).toBeUndefined()
325-
})
319+
// The preview-gate behavior (slack custom bot ↔ slack_v2) is covered in
320+
// service-account-gate.test.ts, which mocks getBlock — the block registry is
321+
// globally stubbed here, so slack_v2's real `preview: true` isn't observable
322+
// through serializeIntegrationSchema.
326323
})
327324

328325
describe('serializeCredentials — type distinguishes reconnect flow', () => {

apps/sim/lib/copilot/vfs/serializers.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { isSubBlockHidden } from '@/lib/workflows/subblocks/visibility'
1111
import { getBlock } from '@/blocks'
1212
import { isCustomBlockType } from '@/blocks/custom/build-config'
1313
import type { BlockConfig, SubBlockConfig } from '@/blocks/types'
14+
import { isHiddenUnder } from '@/blocks/visibility/context'
1415
import { DYNAMIC_MODEL_PROVIDERS, PROVIDER_DEFINITIONS } from '@/providers/models'
1516
import type { ToolConfig, ToolHostingCondition } from '@/tools/types'
1617

@@ -57,7 +58,15 @@ export function describeServiceAccountForOAuthProvider(
5758
const serviceAccountProviderId = getServiceAccountProviderForProviderId(oauthProvider)
5859
if (!serviceAccountProviderId) return undefined
5960
const gatingBlockType = getServiceAccountGatingBlockType(serviceAccountProviderId)
60-
if (gatingBlockType && (getBlock(gatingBlockType)?.preview ?? true)) return undefined
61+
if (gatingBlockType) {
62+
const gatingBlock = getBlock(gatingBlockType)
63+
// Omit when the gating block is missing (fail-closed) or hidden by the
64+
// canonical predicate. Passing `null` vis reduces `isHiddenUnder` to the
65+
// static preview check — so once the block GAs and drops `preview`, it is
66+
// no longer hidden and discovery includes it again, matching the renderer.
67+
// Hand-rolling `?.preview ?? true` would keep it omitted forever after GA.
68+
if (!gatingBlock || isHiddenUnder(null, gatingBlock)) return undefined
69+
}
6170
return { connectNoun: getServiceAccountConnectNoun(serviceAccountProviderId) }
6271
}
6372

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { beforeEach, describe, expect, it, vi } from 'vitest'
5+
6+
const { mockGetBlock } = vi.hoisted(() => ({ mockGetBlock: vi.fn() }))
7+
vi.mock('@/blocks', () => ({ getBlock: mockGetBlock }))
8+
9+
import { describeServiceAccountForOAuthProvider } from '@/lib/copilot/vfs/serializers'
10+
11+
describe('describeServiceAccountForOAuthProvider — preview gate', () => {
12+
beforeEach(() => {
13+
vi.clearAllMocks()
14+
})
15+
16+
it('omits a service account whose gating block is still a preview block', () => {
17+
mockGetBlock.mockReturnValue({ type: 'slack_v2', preview: true })
18+
expect(describeServiceAccountForOAuthProvider('slack')).toBeUndefined()
19+
})
20+
21+
it('includes it once the gating block GAs and drops preview', () => {
22+
// slack_v2's documented GA migration removes `preview`. Discovery must then
23+
// surface the custom bot, matching what the UI shows. A hand-rolled
24+
// `?.preview ?? true` would keep it omitted forever — the "sticks after GA"
25+
// regression; reusing isHiddenUnder(null, block) fixes it.
26+
mockGetBlock.mockReturnValue({ type: 'slack_v2' })
27+
expect(describeServiceAccountForOAuthProvider('slack')).toEqual({ connectNoun: 'custom bot' })
28+
})
29+
30+
it('fail-closes (omits) when the gating block is missing entirely', () => {
31+
mockGetBlock.mockReturnValue(undefined)
32+
expect(describeServiceAccountForOAuthProvider('slack')).toBeUndefined()
33+
})
34+
35+
it('includes an ungated provider without consulting the block registry', () => {
36+
expect(describeServiceAccountForOAuthProvider('notion')).toEqual({
37+
connectNoun: 'integration secret',
38+
})
39+
expect(mockGetBlock).not.toHaveBeenCalled()
40+
})
41+
42+
it('returns undefined for a provider with no service-account flow', () => {
43+
expect(describeServiceAccountForOAuthProvider('github')).toBeUndefined()
44+
})
45+
})

0 commit comments

Comments
 (0)