-
Notifications
You must be signed in to change notification settings - Fork 3
Add support for oauth connectors #104
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
cbdf7c4
d8a7418
5790f15
276f74c
81b48bc
82df351
31d0fcc
55d53ce
9f12d4d
cb4332f
9aa2e77
710a296
fd39557
29af2a0
4b9a80b
e6c3f3f
e736a77
28591ac
4b4f92b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,129 @@ | ||
| import { Command } from "commander"; | ||
| import { cancel, log, select, isCancel } from "@clack/prompts"; | ||
| import { | ||
| initiateOAuth, | ||
| SUPPORTED_INTEGRATIONS, | ||
| INTEGRATION_DISPLAY_NAMES, | ||
| getIntegrationDisplayName, | ||
| } from "@/core/connectors/index.js"; | ||
| import type { IntegrationType } from "@/core/connectors/index.js"; | ||
| import { runCommand, runTask } from "../../utils/index.js"; | ||
| import type { RunCommandResult } from "../../utils/runCommand.js"; | ||
| import { theme } from "../../utils/theme.js"; | ||
| import { assertValidIntegrationType, waitForOAuthCompletion } from "./utils.js"; | ||
|
|
||
| async function promptForIntegrationType(): Promise<IntegrationType> { | ||
| const options = Object.entries(INTEGRATION_DISPLAY_NAMES).map( | ||
| ([type, displayName]) => ({ | ||
| value: type, | ||
| label: displayName, | ||
| }) | ||
| ); | ||
|
|
||
| const selected = await select({ | ||
| message: "Select an integration to connect:", | ||
| options, | ||
| }); | ||
|
|
||
| if (isCancel(selected)) { | ||
| cancel("Operation cancelled."); | ||
| process.exit(0); | ||
| } | ||
|
|
||
| return selected as IntegrationType; | ||
| } | ||
|
|
||
| async function pollForOAuthCompletion( | ||
| integrationType: IntegrationType, | ||
| connectionId: string | ||
| ): Promise<{ success: boolean; accountEmail?: string; error?: string }> { | ||
| return await runTask( | ||
| "Waiting for authorization...", | ||
| async () => { | ||
| return await waitForOAuthCompletion(integrationType, connectionId); | ||
| }, | ||
| { | ||
| successMessage: "Authorization completed!", | ||
| errorMessage: "Authorization failed", | ||
| } | ||
| ); | ||
| } | ||
|
|
||
| export async function addConnector( | ||
| integrationType?: string | ||
| ): Promise<RunCommandResult> { | ||
| // Get type from argument or prompt | ||
| let selectedType: IntegrationType; | ||
| if (integrationType) { | ||
| assertValidIntegrationType(integrationType, SUPPORTED_INTEGRATIONS); | ||
| selectedType = integrationType; | ||
| } else { | ||
| selectedType = await promptForIntegrationType(); | ||
| } | ||
|
|
||
| const displayName = getIntegrationDisplayName(selectedType); | ||
|
|
||
| // Initiate OAuth flow | ||
| const initiateResponse = await runTask( | ||
| `Initiating ${displayName} connection...`, | ||
| async () => { | ||
| return await initiateOAuth(selectedType); | ||
| }, | ||
| { | ||
| successMessage: `${displayName} OAuth initiated`, | ||
| errorMessage: `Failed to initiate ${displayName} connection`, | ||
| } | ||
| ); | ||
|
|
||
| // Check if already authorized | ||
| if (initiateResponse.alreadyAuthorized) { | ||
| return { | ||
| outroMessage: `Already connected to ${theme.styles.bold(displayName)}`, | ||
| }; | ||
| } | ||
|
|
||
| // Check if connected by different user | ||
| if (initiateResponse.error === "different_user" && initiateResponse.otherUserEmail) { | ||
| throw new Error( | ||
| `This app is already connected to ${displayName} by ${initiateResponse.otherUserEmail}` | ||
| ); | ||
| } | ||
|
|
||
| // Validate we have required fields | ||
| if (!initiateResponse.redirectUrl || !initiateResponse.connectionId) { | ||
| throw new Error("Invalid response from server: missing redirect URL or connection ID"); | ||
| } | ||
|
|
||
| // Show authorization URL | ||
| log.info( | ||
| `Please authorize ${displayName} at:\n${theme.colors.links(initiateResponse.redirectUrl)}` | ||
| ); | ||
|
|
||
| // Poll for completion | ||
| const result = await pollForOAuthCompletion( | ||
| selectedType, | ||
| initiateResponse.connectionId | ||
| ); | ||
|
|
||
| if (!result.success) { | ||
| throw new Error(result.error || "Authorization failed"); | ||
| } | ||
|
|
||
| const accountInfo = result.accountEmail | ||
| ? ` as ${theme.styles.bold(result.accountEmail)}` | ||
| : ""; | ||
|
|
||
| return { | ||
| outroMessage: `Successfully connected to ${theme.styles.bold(displayName)}${accountInfo}`, | ||
| }; | ||
| } | ||
|
|
||
| export const connectorsAddCommand = new Command("add") | ||
| .argument("[type]", "Integration type (e.g., slack, notion, googlecalendar)") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a new behavior, do we want the user / agent write
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just continued to read the code and i see you do prompt to the user if no argument specified.. cool 👍, i still think we should maybe align the flags since this is the behavior so far |
||
| .description("Connect an OAuth integration") | ||
| .action(async (type?: string) => { | ||
| await runCommand(() => addConnector(type), { | ||
| requireAuth: true, | ||
| requireAppConfig: true, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. default is already true |
||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| import { Command } from "commander"; | ||
| import { connectorsAddCommand } from "./add.js"; | ||
| import { connectorsRemoveCommand } from "./remove.js"; | ||
|
|
||
| export const connectorsCommand = new Command("connectors") | ||
| .description("Manage OAuth connectors") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NIT: Is this the right description for the command? might be worth talking to Chris |
||
| .addCommand(connectorsAddCommand) | ||
| .addCommand(connectorsRemoveCommand); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,157 @@ | ||
| import { Command } from "commander"; | ||
| import { cancel, confirm, select, isCancel } from "@clack/prompts"; | ||
| import { | ||
| listConnectors, | ||
| disconnectConnector, | ||
| removeConnector, | ||
| isValidIntegration, | ||
| getIntegrationDisplayName, | ||
| } from "@/core/connectors/index.js"; | ||
| import type { IntegrationType, Connector } from "@/core/connectors/index.js"; | ||
| import { runCommand, runTask } from "../../utils/index.js"; | ||
| import type { RunCommandResult } from "../../utils/runCommand.js"; | ||
| import { theme } from "../../utils/theme.js"; | ||
|
|
||
| interface RemoveOptions { | ||
| hard?: boolean; | ||
| yes?: boolean; | ||
| } | ||
|
|
||
| interface ConnectorInfo { | ||
| type: IntegrationType; | ||
| displayName: string; | ||
| accountEmail?: string; | ||
| } | ||
|
|
||
| function mapBackendConnectors(connectors: Connector[]): ConnectorInfo[] { | ||
| return connectors | ||
| .filter((c) => isValidIntegration(c.integrationType)) | ||
| .map((c) => ({ | ||
| type: c.integrationType, | ||
| displayName: getIntegrationDisplayName(c.integrationType), | ||
|
Check failure on line 31 in src/cli/commands/connectors/remove.ts
|
||
| accountEmail: (c.accountInfo?.email || c.accountInfo?.name) ?? undefined, | ||
| })); | ||
| } | ||
|
|
||
| function validateConnectorType( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. duplicate function like in add.ts file if i remember correctly, lets move to be reused or remove alltogether |
||
| type: string, | ||
| connectors: ConnectorInfo[] | ||
| ): ConnectorInfo { | ||
| if (!isValidIntegration(type)) { | ||
| throw new Error(`Invalid connector type: ${type}`); | ||
| } | ||
|
|
||
| const connector = connectors.find((c) => c.type === type); | ||
| if (!connector) { | ||
| throw new Error(`No ${getIntegrationDisplayName(type)} connector found`); | ||
| } | ||
|
|
||
| return connector; | ||
| } | ||
|
|
||
| async function promptForConnectorToRemove( | ||
| connectors: ConnectorInfo[] | ||
| ): Promise<ConnectorInfo> { | ||
| const options = connectors.map((c) => { | ||
| let label = c.displayName; | ||
| if (c.accountEmail) { | ||
| label += ` (${c.accountEmail})`; | ||
| } | ||
| return { | ||
| value: c.type, | ||
| label, | ||
| }; | ||
| }); | ||
|
|
||
| const selected = await select({ | ||
| message: "Select a connector to remove:", | ||
| options, | ||
| }); | ||
|
|
||
| if (isCancel(selected)) { | ||
| cancel("Operation cancelled."); | ||
| process.exit(0); | ||
| } | ||
|
|
||
| return connectors.find((c) => c.type === selected)!; | ||
| } | ||
|
|
||
| export async function removeConnectorCommand( | ||
| integrationType?: string, | ||
| options: RemoveOptions = {} | ||
| ): Promise<RunCommandResult> { | ||
| const isHardDelete = options.hard === true; | ||
|
|
||
| // Fetch backend connectors | ||
| const backendConnectors = await runTask( | ||
| "Fetching connectors...", | ||
| async () => listConnectors(), | ||
| { | ||
| successMessage: "Connectors loaded", | ||
| errorMessage: "Failed to fetch connectors", | ||
| } | ||
| ); | ||
|
|
||
| const connectors = mapBackendConnectors(backendConnectors); | ||
|
|
||
| if (connectors.length === 0) { | ||
| return { | ||
| outroMessage: "No connectors to remove", | ||
| }; | ||
| } | ||
|
|
||
| // Get type from argument or prompt | ||
| const selectedConnector = integrationType | ||
| ? validateConnectorType(integrationType, connectors) | ||
| : await promptForConnectorToRemove(connectors); | ||
|
|
||
| const displayName = selectedConnector.displayName; | ||
| const accountInfo = selectedConnector.accountEmail | ||
| ? ` (${selectedConnector.accountEmail})` | ||
| : ""; | ||
|
|
||
| // Confirm removal (skip if --yes flag is provided) | ||
| if (!options.yes) { | ||
| const actionWord = isHardDelete ? "Permanently remove" : "Remove"; | ||
| const shouldRemove = await confirm({ | ||
| message: `${actionWord} ${displayName}${accountInfo}?`, | ||
| }); | ||
|
|
||
| if (isCancel(shouldRemove) || !shouldRemove) { | ||
| cancel("Operation cancelled."); | ||
| process.exit(0); | ||
| } | ||
| } | ||
|
|
||
| // Perform removal | ||
| await runTask( | ||
| `Removing ${displayName}...`, | ||
| async () => { | ||
| if (isHardDelete) { | ||
| await removeConnector(selectedConnector.type); | ||
| } else { | ||
| await disconnectConnector(selectedConnector.type); | ||
| } | ||
| }, | ||
| { | ||
| successMessage: `${displayName} removed`, | ||
| errorMessage: `Failed to remove ${displayName}`, | ||
| } | ||
| ); | ||
|
|
||
| return { | ||
| outroMessage: `Successfully removed ${theme.styles.bold(displayName)}`, | ||
| }; | ||
| } | ||
|
|
||
| export const connectorsRemoveCommand = new Command("remove") | ||
| .argument("[type]", "Integration type to remove (e.g., slack, notion)") | ||
| .option("--hard", "Permanently remove the connector (cannot be undone)") | ||
| .option("-y, --yes", "Skip confirmation prompt") | ||
| .description("Remove an OAuth integration") | ||
| .action(async (type: string | undefined, options: RemoveOptions) => { | ||
| await runCommand(() => removeConnectorCommand(type, options), { | ||
| requireAuth: true, | ||
| requireAppConfig: true, | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| import pWaitFor from "p-wait-for"; | ||
| import { checkOAuthStatus } from "@/core/connectors/api.js"; | ||
| import type { IntegrationType } from "@/core/connectors/consts.js"; | ||
|
|
||
| const OAUTH_POLL_INTERVAL_MS = 2000; | ||
| const OAUTH_POLL_TIMEOUT_MS = 5 * 60 * 1000; // 5 minutes | ||
|
|
||
| export interface OAuthCompletionResult { | ||
| success: boolean; | ||
| accountEmail?: string; | ||
| error?: string; | ||
| } | ||
|
|
||
| /** | ||
| * Polls for OAuth completion status. | ||
| * Returns when status becomes ACTIVE or FAILED, or times out. | ||
| */ | ||
| export async function waitForOAuthCompletion( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this function belongs to the core/ folder - doing pWaitFor is a CLI behavior - core package should expose all relevant method (api, config ...) that the CLI can use in order to give the user the right UX.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moving this to the CLI will also simplify things like onPending on so on, so try to expose from core/ the right methods that are needed, and everything related to the experince should be in the CLI |
||
| integrationType: IntegrationType, | ||
| connectionId: string, | ||
| options?: { | ||
| onPending?: () => void; | ||
| } | ||
| ): Promise<OAuthCompletionResult> { | ||
| let accountEmail: string | undefined; | ||
| let error: string | undefined; | ||
|
|
||
| try { | ||
| await pWaitFor( | ||
| async () => { | ||
| const status = await checkOAuthStatus(integrationType, connectionId); | ||
|
|
||
| if (status.status === "ACTIVE") { | ||
| accountEmail = status.accountEmail ?? undefined; | ||
| return true; | ||
| } | ||
|
|
||
| if (status.status === "FAILED") { | ||
| error = status.error || "Authorization failed"; | ||
| throw new Error(error); | ||
| } | ||
|
|
||
| // PENDING - continue polling | ||
| options?.onPending?.(); | ||
| return false; | ||
| }, | ||
| { | ||
| interval: OAUTH_POLL_INTERVAL_MS, | ||
| timeout: OAUTH_POLL_TIMEOUT_MS, | ||
| } | ||
| ); | ||
|
|
||
| return { success: true, accountEmail }; | ||
| } catch (err) { | ||
| if (err instanceof Error && err.message.includes("timed out")) { | ||
| return { success: false, error: "Authorization timed out. Please try again." }; | ||
| } | ||
| return { success: false, error: error || (err instanceof Error ? err.message : "Unknown error") }; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Asserts that a string is a valid integration type, throwing if not. | ||
| */ | ||
| export function assertValidIntegrationType( | ||
| type: string, | ||
| supportedIntegrations: readonly string[] | ||
| ): asserts type is IntegrationType { | ||
| if (!supportedIntegrations.includes(type)) { | ||
| const supportedList = supportedIntegrations.join(", "); | ||
| throw new Error( | ||
| `Unsupported connector: ${type}\nSupported connectors: ${supportedList}` | ||
| ); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just noting that i don't like that some commands use
process.exit(0)in the middle of the code, can cause unexpected behavior (for tests for example)We need to have a standard way to stop execution (maybe throw an Exception that is caught and stops the execution)