mirror of
https://github.com/nexu-io/open-design.git
synced 2026-10-27 09:51:12 +01:00
fix(connectors): preserve OAuth state and advertised tool counts (#1036)
* fix(connectors): preserve oauth state and advertised counts * test(connectors): type fixture for advertised count * docs(connectors): align tool count badge contract * docs(connectors): clarify curated tool names role
This commit is contained in:
@@ -59,11 +59,11 @@ export interface ConnectorDetail {
|
||||
/**
|
||||
* The hand-curated catalog subset. Stable across hydration: never
|
||||
* extended by provider discovery, only ever the static catalog
|
||||
* names. UIs surfacing a single "N tools" summary (the connector
|
||||
* card / drawer header badge) should read this so the displayed
|
||||
* count doesn't lurch when an API key flips on (issue #748). The
|
||||
* full provider inventory is still discoverable in the drawer's
|
||||
* tools section, which renders `tools` directly.
|
||||
* names. This preserves the static catalog baseline for consumers
|
||||
* that need that curated subset, but it is not the advertised
|
||||
* provider inventory count. UI summary badges should use `toolCount`
|
||||
* when present; the drawer's rendered tool rows still come from
|
||||
* `tools` directly.
|
||||
*
|
||||
* Optional in the type only for fixture brevity; daemon-built
|
||||
* `ConnectorDetail` payloads always carry it.
|
||||
@@ -95,10 +95,10 @@ export interface ConnectorCatalogDefinition {
|
||||
/**
|
||||
* The hand-curated subset of `allowedToolNames` that is fixed at the
|
||||
* catalog level — never extended by provider discovery (issue #748).
|
||||
* Optional: when omitted, downstream consumers (the wire detail and
|
||||
* the badge helper) fall back to `allowedToolNames`, which is the
|
||||
* right behavior for non-Composio connectors that don't have a
|
||||
* dynamic discovery layer in the first place.
|
||||
* Optional: when omitted, serialized wire details fall back to
|
||||
* `allowedToolNames`, which is the right preview subset for
|
||||
* non-Composio connectors that don't have a dynamic discovery layer
|
||||
* in the first place.
|
||||
*/
|
||||
curatedToolNames?: string[];
|
||||
/** Display-only count of provider tools. This may be known before tool schemas are hydrated. */
|
||||
|
||||
@@ -1035,12 +1035,10 @@ export class ComposioConnectorProvider {
|
||||
.map((tool) => tool.name);
|
||||
const allowedToolNames = [...new Set([...staticDefinition.allowedToolNames, ...autoAllowedLiveToolNames])];
|
||||
// `curatedToolNames` mirrors the static catalog ONLY — it
|
||||
// intentionally never picks up `autoAllowedLiveToolNames`. The
|
||||
// wire detail and the connector card / drawer header badge read
|
||||
// this so the "N tools" summary stays at the catalog baseline
|
||||
// (e.g. 2 for GitHub) instead of inflating by the tens of
|
||||
// read-only auto-approved tools that show up after Composio
|
||||
// discovery (issue #748). The execution-time gate keeps using
|
||||
// intentionally never picks up `autoAllowedLiveToolNames`. It
|
||||
// preserves the static catalog baseline, while summary badges use
|
||||
// `toolCount` when present to reflect the advertised provider
|
||||
// inventory. The execution-time gate keeps using
|
||||
// `allowedToolNames`, so the dynamic auto-allow behavior is
|
||||
// preserved end-to-end.
|
||||
const curatedToolNames = [...staticDefinition.allowedToolNames];
|
||||
|
||||
@@ -312,15 +312,6 @@ export function clearConnectorAuthorizationPending(
|
||||
}
|
||||
|
||||
export function getConnectorDisplayToolCount(connector: ConnectorDetail): number {
|
||||
// Prefer the hand-curated catalog subset so the card/header badge stays
|
||||
// stable across Composio hydration: `toolCount` is sourced from the
|
||||
// provider's totalItems (a GitHub-style toolkit jumps from 2 to 48 once an
|
||||
// API key is configured), and `tools.length` shifts the same way after
|
||||
// hydration. `curatedToolNames` is fixed at the catalog and is what the
|
||||
// "N tools" badge is meant to reflect (issue #748).
|
||||
if (connector.curatedToolNames && connector.curatedToolNames.length > 0) {
|
||||
return connector.curatedToolNames.length;
|
||||
}
|
||||
return connector.toolCount ?? connector.tools.length;
|
||||
}
|
||||
|
||||
|
||||
@@ -351,7 +351,6 @@ export async function connectConnector(connectorId: string): Promise<ConnectorAc
|
||||
if (useExternalBrowser) {
|
||||
const opened = await openExternal(json.auth.redirectUrl);
|
||||
if (!opened) {
|
||||
void cancelConnectorAuthorization(connectorId);
|
||||
return { connector: json.connector ?? null, error: popupBlockedMessage() };
|
||||
}
|
||||
} else if (authWindow) {
|
||||
@@ -359,7 +358,6 @@ export async function connectConnector(connectorId: string): Promise<ConnectorAc
|
||||
} else {
|
||||
const redirected = window.open(json.auth.redirectUrl, '_blank');
|
||||
if (!redirected) {
|
||||
void cancelConnectorAuthorization(connectorId);
|
||||
return { connector: json.connector ?? null, error: popupBlockedMessage() };
|
||||
}
|
||||
}
|
||||
|
||||
@@ -86,6 +86,34 @@ describe('connector display sorting', () => {
|
||||
expect(connector.tools).toEqual([]);
|
||||
});
|
||||
|
||||
it('prefers advertised tool counts over curated preview tool names', () => {
|
||||
const connector = {
|
||||
id: 'github',
|
||||
name: 'GitHub',
|
||||
provider: 'Composio',
|
||||
category: 'Developer',
|
||||
status: 'connected' as const,
|
||||
toolCount: 846,
|
||||
tools: [
|
||||
{
|
||||
title: 'Search repositories',
|
||||
name: 'github.github_search_repositories',
|
||||
safety: { sideEffect: 'read' as const, approval: 'auto' as const, reason: 'Read-only search.' },
|
||||
refreshEligible: true,
|
||||
},
|
||||
{
|
||||
title: 'Get issue',
|
||||
name: 'github.github_get_issue',
|
||||
safety: { sideEffect: 'read' as const, approval: 'auto' as const, reason: 'Read-only get.' },
|
||||
refreshEligible: true,
|
||||
},
|
||||
],
|
||||
curatedToolNames: ['github.github_search_repositories', 'github.github_get_issue'],
|
||||
};
|
||||
|
||||
expect(getConnectorDisplayToolCount(connector)).toBe(846);
|
||||
});
|
||||
|
||||
it('appends paginated preview tools without duplicating rows', () => {
|
||||
const current = {
|
||||
id: 'canvas',
|
||||
|
||||
@@ -287,7 +287,7 @@ describe('connectConnector', () => {
|
||||
error: 'Popup blocked. Allow popups for Open Design and try again.',
|
||||
});
|
||||
expect(open).toHaveBeenCalledTimes(2);
|
||||
expect(fetchMock).toHaveBeenCalledWith('/api/connectors/github/authorization/cancel', {
|
||||
expect(fetchMock).not.toHaveBeenCalledWith('/api/connectors/github/authorization/cancel', {
|
||||
method: 'POST',
|
||||
});
|
||||
});
|
||||
@@ -350,7 +350,7 @@ describe('connectConnector', () => {
|
||||
});
|
||||
expect(open).not.toHaveBeenCalled();
|
||||
expect(openExternal).toHaveBeenCalledWith('https://example.com/oauth');
|
||||
expect(fetchMock).toHaveBeenCalledWith('/api/connectors/github/authorization/cancel', {
|
||||
expect(fetchMock).not.toHaveBeenCalledWith('/api/connectors/github/authorization/cancel', {
|
||||
method: 'POST',
|
||||
});
|
||||
});
|
||||
|
||||
@@ -45,8 +45,8 @@ export interface ConnectorDetail {
|
||||
* read-only with auto approval. Used by the agent layer to gate
|
||||
* which tools are invocable. Note: this list **grows** on Composio
|
||||
* hydration (a GitHub-style provider can add tens of read tools to
|
||||
* the catalog baseline of 2), so it is not the right anchor for a
|
||||
* stable UI summary count — see `curatedToolNames` for that.
|
||||
* the catalog baseline of 2), so it is not the right anchor for the
|
||||
* advertised provider tool count — see `toolCount` for that.
|
||||
*
|
||||
* Optional in the type only to keep test fixtures terse — the daemon
|
||||
* always populates this from `connectorDefinitionToDetail` so wire
|
||||
@@ -56,11 +56,11 @@ export interface ConnectorDetail {
|
||||
/**
|
||||
* Hand-curated catalog subset. Stable across hydration: never
|
||||
* extended by provider discovery, only the static catalog names.
|
||||
* UIs surfacing a single "N tools" summary (the connector card and
|
||||
* drawer header badges) should read this so the displayed count
|
||||
* doesn't lurch when an API key flips on (issue #748). The full
|
||||
* provider inventory is still discoverable in the drawer's tools
|
||||
* section, which renders `tools` directly.
|
||||
* This preserves the static catalog baseline for consumers that
|
||||
* need that curated subset, but it does not represent the
|
||||
* advertised provider inventory. UI summary badges should use
|
||||
* `toolCount` when present, while the drawer's rendered tool rows
|
||||
* still come from `tools` directly.
|
||||
*
|
||||
* Optional in the type only for fixture brevity, see `allowedToolNames`.
|
||||
*/
|
||||
|
||||
Reference in New Issue
Block a user