From 759691e3808cd7569e16e5916deddd16b69fadeb Mon Sep 17 00:00:00 2001 From: Steven Enamakel <31011319+senamakel@users.noreply.github.com> Date: Sat, 11 Apr 2026 12:15:45 -0700 Subject: [PATCH] feat(composio): improve toolkit sync and connection handling (#507) * Enhance release workflow with build target input and improved job structure - Added a new input parameter `build_target` to specify the environment (production or staging) for the release process. - Made `release_type` input optional with a default value of `patch`. - Refactored job names and dependencies to reflect the new build target logic, including conditional steps for production and staging environments. - Introduced a `resolve` step to determine build outputs based on the selected environment, enhancing the workflow's flexibility and clarity. - Updated the `create-release` job to depend on the new `prepare-build` job, ensuring proper execution flow based on the build target. * feat(composio): enhance Composio integration with toolkit management and testing - Added `KNOWN_COMPOSIO_TOOLKITS` constant to facilitate access to available toolkits. - Implemented unit tests for `useComposioIntegrations` to ensure correct behavior during toolkit and connection fetching, including error handling scenarios. - Updated `Skills` page to utilize the new `KNOWN_COMPOSIO_TOOLKITS` for improved toolkit display logic. - Refactored hooks to handle connection errors gracefully and maintain toolkit visibility. - Enhanced backend integration by updating Composio client configuration to streamline toolkit management. * refactor(dispatch): remove channel delivery instructions for Telegram - Deleted the `channel_delivery_instructions` function, which provided response guidelines for Telegram messages. This change simplifies the message processing logic in the `process_channel_message` function by eliminating unnecessary instructions, enhancing clarity and maintainability. * refactor(composio): simplify Composio client configuration and remove toggles - Updated the `build_composio_client` function to remove unnecessary configuration checks, as Composio is always enabled when the user is signed in. - Revised the `resolve_client` function to clarify error handling related to user authentication. - Streamlined the `IntegrationsConfig` structure by removing toggles for Composio and related backend settings, ensuring a consistent configuration approach across integrations. - Adjusted tests to reflect the removal of integration toggles and focus on core API key usage. * refactor(composio): remove composio disabled state and improve error handling - Eliminated the `disabled` state from the `useComposioIntegrations` hook, as Composio is always enabled when the user is authenticated. - Updated error handling to surface backend connection issues directly, replacing previous checks for a disabled state. - Revised tests to reflect the new error handling logic, ensuring clarity in toolkit fetch error reporting. * refactor(integrations): update authentication handling for client configuration - Revised the `build_client` function to prioritize app-session JWT for user authentication, enhancing clarity in the fallback mechanism to `config.api_key`. - Improved error messages in `resolve_client` and `build_client` to provide clearer guidance on authentication issues related to session tokens. - Streamlined comments and documentation to reflect the updated authentication flow, ensuring consistency across integration components. * refactor(composio): unwrap CLI envelope for API responses - Introduced a new `unwrapCliEnvelope` function to handle the response format from the Rust side, allowing for easier access to the flat shapes defined in `./types`. - Updated `listToolkits`, `listConnections`, `listTools`, `authorize`, `deleteConnection`, and `execute` functions to utilize the new unwrapping logic, improving response handling consistency across the Composio API. - Enhanced error handling by ensuring that responses without logs pass through unchanged, maintaining backward compatibility. * refactor(skills_agent): update agent description and enhance Composio tool integration - Revised the `when_to_use` description in `agent.toml` to clarify the role of the Skills Agent as a service integration specialist, emphasizing its capability to execute both Composio and QuickJS skill tools. - Expanded the `prompt.md` documentation to detail available tool surfaces and typical Composio flow, improving clarity on how to interact with external services. - Implemented category overrides for Composio tools in `tools.rs` to ensure they are recognized as part of the Skill category, allowing proper access through the skills sub-agent. - Added tests to verify that Composio tools are correctly filtered and accessible by the skills sub-agent, ensuring robust integration and functionality. * style: apply formatter output from pre-push checks * refactor(tests): update Gmail and Notion integration tests for composio - Refactored tests for the Skills page to integrate Gmail and Notion as composio tools, enhancing the testing framework. - Removed mock data and streamlined the test setup to reflect the current state of available skills. - Updated assertions to verify the rendering of connected and disconnected states for Gmail and Notion integrations, respectively. - Improved clarity and maintainability of test cases by consolidating mock implementations and removing redundant code. * refactor(skills): update toolkit categorization and enhance test assertions - Modified the Skills component to assign categories dynamically based on toolkit metadata, improving organization of displayed tools. - Updated test cases to reflect the new categorization, ensuring accurate rendering of tools under their respective categories instead of a generic 'Other' group. - Enhanced assertions in tests to verify the presence of specific tools and their categories, improving test coverage and reliability. * refactor(skills): streamline item creation in Skills component - Simplified the item creation logic in the Skills component by removing unnecessary line breaks, enhancing code readability without altering functionality. - This change contributes to cleaner code structure and maintainability in the Skills page. * refactor(tests): enhance Gmail and Notion integration tests for improved clarity - Updated the integration tests for Gmail and Notion on the Skills page to utilize the `within` function for more precise querying of elements within their respective sections. - Improved test assertions to ensure that the connected and disconnected states are accurately verified, enhancing the reliability of the tests. - Streamlined the test setup to better reflect the current structure of the Skills component, contributing to overall test maintainability. * feat(skills): enhance Composio integration error handling and logging - Added error handling for Composio integrations in the Skills component, displaying a user-friendly message when integration status is stale or an error occurs. - Implemented logging in development mode to provide insights into the state of Composio toolkits and connections, aiding in debugging. - Updated the item rendering logic to reflect the error state, ensuring users can retry fetching integrations when an error is detected. - Enhanced tests to verify the display of error messages and the functionality of the retry mechanism, improving overall test coverage and reliability. --- .github/workflows/release.yml | 162 ++++++++++++---- app/src/components/composio/toolkitMeta.ts | 2 + app/src/lib/composio/composioApi.ts | 42 +++- app/src/lib/composio/hooks.test.ts | 51 +++++ app/src/lib/composio/hooks.ts | 67 ++++--- app/src/pages/Skills.tsx | 182 ++++++++---------- .../Skills.composio-catalog.test.tsx | 81 ++++++++ .../Skills.third-party-gmail-sync.test.tsx | 83 +++----- ...ls.third-party-notion-debug-tools.test.tsx | 180 +++-------------- .../agent/agents/skills_agent/agent.toml | 2 +- .../agent/agents/skills_agent/prompt.md | 35 ++-- .../agent/harness/subagent_runner.rs | 84 ++++++++ src/openhuman/channels/runtime/dispatch.rs | 22 --- src/openhuman/composio/client.rs | 31 ++- src/openhuman/composio/ops.rs | 25 ++- src/openhuman/composio/tools.rs | 78 +++++++- src/openhuman/config/schema/tools.rs | 33 +--- src/openhuman/integrations/client.rs | 119 ++++++++++-- src/openhuman/integrations/mod.rs | 30 ++- src/openhuman/memory/embeddings.rs | 84 ++++++-- src/openhuman/tools/impl/network/composio.rs | 9 +- src/openhuman/tools/ops.rs | 5 +- 22 files changed, 883 insertions(+), 524 deletions(-) create mode 100644 app/src/lib/composio/hooks.test.ts create mode 100644 app/src/pages/__tests__/Skills.composio-catalog.test.tsx diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 97e3f3f2d..bb5298056 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -3,9 +3,18 @@ name: Release on: workflow_dispatch: inputs: + build_target: + description: Build target environment + required: true + type: choice + default: production + options: + - production + - staging release_type: description: Version increment type - required: true + required: false + default: patch type: choice options: - patch @@ -17,13 +26,13 @@ permissions: packages: write concurrency: - group: release-main + group: release-${{ github.event.inputs.build_target || 'production' }}-main cancel-in-progress: false # --------------------------------------------------------------------------- # Job dependency graph # -# prepare-release +# prepare-build # │ # ├─── create-release # │ │ @@ -46,14 +55,17 @@ jobs: # ========================================================================= # Phase 1: Version bump, commit, tag # ========================================================================= - prepare-release: - name: Prepare release commit and tag + prepare-build: + name: Prepare build context runs-on: ubuntu-latest environment: Production outputs: - version: ${{ steps.bump.outputs.version }} - tag: ${{ steps.bump.outputs.tag }} - sha: ${{ steps.push.outputs.sha }} + version: ${{ steps.resolve.outputs.version }} + tag: ${{ steps.resolve.outputs.tag }} + sha: ${{ steps.resolve.outputs.sha }} + build_ref: ${{ steps.resolve.outputs.build_ref }} + release_enabled: ${{ steps.resolve.outputs.release_enabled }} + base_url: ${{ steps.resolve.outputs.base_url }} steps: - name: Enforce main branch if: github.ref != 'refs/heads/main' @@ -62,25 +74,35 @@ jobs: exit 1 - name: Generate GitHub App token + if: inputs.build_target == 'production' id: app-token uses: tibdex/github-app-token@v1 with: app_id: ${{ secrets.XGITHUB_APP_ID }} private_key: ${{ secrets.XGITHUB_APP_PRIVATE_KEY }} - - name: Checkout main + - name: Checkout main for production release + if: inputs.build_target == 'production' uses: actions/checkout@v4 with: ref: main fetch-depth: 0 token: ${{ steps.app-token.outputs.token }} + - name: Checkout main for staging build + if: inputs.build_target == 'staging' + uses: actions/checkout@v4 + with: + ref: main + fetch-depth: 0 + - name: Setup Node.js uses: actions/setup-node@v4 with: node-version: 24.x - name: Configure Git + if: inputs.build_target == 'production' env: APP_TOKEN: ${{ steps.app-token.outputs.token }} run: | @@ -92,13 +114,16 @@ jobs: git pull origin main --ff-only - name: Compute next version and sync release files + if: inputs.build_target == 'production' id: bump run: node scripts/release/bump-version.js "${{ inputs.release_type }}" - name: Verify release version sync + if: inputs.build_target == 'production' run: node scripts/release/verify-version-sync.js "${{ steps.bump.outputs.version }}" - name: Ensure tag does not already exist + if: inputs.build_target == 'production' env: TAG: ${{ steps.bump.outputs.tag }} run: | @@ -113,6 +138,7 @@ jobs: fi - name: Commit, push and tag + if: inputs.build_target == 'production' id: push env: VERSION: ${{ steps.bump.outputs.version }} @@ -127,6 +153,33 @@ jobs: echo "sha=$(git rev-parse HEAD)" >> "$GITHUB_OUTPUT" + - name: Resolve build outputs + id: resolve + shell: bash + run: | + if [ "${{ inputs.build_target }}" = "production" ]; then + VERSION="${{ steps.bump.outputs.version }}" + TAG="${{ steps.bump.outputs.tag }}" + SHA="${{ steps.push.outputs.sha }}" + BUILD_REF="$TAG" + RELEASE_ENABLED="true" + BASE_URL="https://api.tinyhumans.ai/" + else + VERSION="$(node -p "require('./app/package.json').version")" + TAG="" + SHA="$(git rev-parse HEAD)" + BUILD_REF="$SHA" + RELEASE_ENABLED="false" + BASE_URL="https://staging-api.tinyhumans.ai/" + fi + + echo "version=$VERSION" >> "$GITHUB_OUTPUT" + echo "tag=$TAG" >> "$GITHUB_OUTPUT" + echo "sha=$SHA" >> "$GITHUB_OUTPUT" + echo "build_ref=$BUILD_REF" >> "$GITHUB_OUTPUT" + echo "release_enabled=$RELEASE_ENABLED" >> "$GITHUB_OUTPUT" + echo "base_url=$BASE_URL" >> "$GITHUB_OUTPUT" + # ========================================================================= # Phase 2: Create draft GitHub release # ========================================================================= @@ -134,7 +187,8 @@ jobs: name: Create GitHub release runs-on: ubuntu-latest environment: Production - needs: prepare-release + if: needs.prepare-build.outputs.release_enabled == 'true' + needs: prepare-build outputs: release_id: ${{ steps.create.outputs.release_id }} upload_url: ${{ steps.create.outputs.upload_url }} @@ -144,9 +198,9 @@ jobs: uses: actions/github-script@v7 with: script: | - const tag = '${{ needs.prepare-release.outputs.tag }}'; - const version = '${{ needs.prepare-release.outputs.version }}'; - const target = '${{ needs.prepare-release.outputs.sha }}'; + const tag = '${{ needs.prepare-build.outputs.tag }}'; + const version = '${{ needs.prepare-build.outputs.version }}'; + const target = '${{ needs.prepare-build.outputs.sha }}'; const { owner, repo } = context.repo; @@ -179,7 +233,12 @@ jobs: # ========================================================================= build-desktop: name: "Desktop: ${{ matrix.settings.artifact_suffix }}" - needs: [prepare-release, create-release] + needs: [prepare-build, create-release] + if: >- + always() + && needs.prepare-build.result == 'success' + && (needs.prepare-build.outputs.release_enabled != 'true' + || needs.create-release.result == 'success') runs-on: ${{ matrix.settings.platform }} environment: Production strategy: @@ -205,10 +264,10 @@ jobs: env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} steps: - - name: Checkout tag + - name: Checkout build ref uses: actions/checkout@v4 with: - ref: ${{ needs.prepare-release.outputs.tag }} + ref: ${{ needs.prepare-build.outputs.build_ref }} fetch-depth: 1 submodules: true @@ -291,7 +350,7 @@ jobs: id: config-overrides uses: actions/github-script@v7 env: - BASE_URL: ${{ vars.BASE_URL }} + BASE_URL: ${{ needs.prepare-build.outputs.base_url }} UPDATER_PUBLIC_KEY: ${{ secrets.UPDATER_PUBLIC_KEY || vars.UPDATER_PUBLIC_KEY }} UPDATER_ENDPOINT: ${{ vars.UPDATER_ENDPOINT }} UPDATER_REPO: tinyhumansai/openhuman @@ -309,7 +368,7 @@ jobs: run: yarn build env: NODE_ENV: production - VITE_BACKEND_URL: ${{ vars.BASE_URL }} + VITE_BACKEND_URL: ${{ needs.prepare-build.outputs.base_url }} VITE_SENTRY_DSN: ${{ vars.VITE_SENTRY_DSN }} VITE_DEBUG: ${{ vars.VITE_DEBUG }} # OAuth guardrails (#365): block openhuman://oauth/success on outdated desktop builds. @@ -383,6 +442,7 @@ jobs: CORE_BIN_NAME: ${{ steps.core-paths.outputs.core_bin_name }} - name: Build, package and upload to release + if: needs.prepare-build.outputs.release_enabled == 'true' uses: tauri-apps/tauri-action@v0.6.2 id: tauri-build env: @@ -391,7 +451,8 @@ jobs: # Signing + notarization are handled in a dedicated step afterwards # where we sign everything with hardened runtime + entitlements # (required by Apple notarization). - BASE_URL: ${{ vars.BASE_URL }} + BASE_URL: ${{ needs.prepare-build.outputs.base_url }} + VITE_BACKEND_URL: ${{ needs.prepare-build.outputs.base_url }} MACOSX_DEPLOYMENT_TARGET: ${{ matrix.settings.platform == 'macos-latest' && '10.15' || '' }} TAURI_SIGNING_PRIVATE_KEY_PASSWORD: ${{ secrets.TAURI_SIGNING_PRIVATE_KEY_PASSWORD || secrets.UPDATER_PRIVATE_KEY_PASSWORD }} TAURI_SIGNING_PRIVATE_KEY: ${{ secrets.TAURI_SIGNING_PRIVATE_KEY || secrets.UPDATER_PRIVATE_KEY }} @@ -408,6 +469,23 @@ jobs: owner: tinyhumansai repo: openhuman + - name: Build and package staging artifacts + if: needs.prepare-build.outputs.release_enabled != 'true' + shell: bash + env: + BASE_URL: ${{ needs.prepare-build.outputs.base_url }} + VITE_BACKEND_URL: ${{ needs.prepare-build.outputs.base_url }} + MACOSX_DEPLOYMENT_TARGET: ${{ matrix.settings.platform == 'macos-latest' && '10.15' || '' }} + TAURI_SIGNING_PRIVATE_KEY_PASSWORD: ${{ secrets.TAURI_SIGNING_PRIVATE_KEY_PASSWORD || secrets.UPDATER_PRIVATE_KEY_PASSWORD }} + TAURI_SIGNING_PRIVATE_KEY: ${{ secrets.TAURI_SIGNING_PRIVATE_KEY || secrets.UPDATER_PRIVATE_KEY }} + WITH_UPDATER: "true" + VITE_MINIMUM_SUPPORTED_APP_VERSION: ${{ vars.VITE_MINIMUM_SUPPORTED_APP_VERSION }} + VITE_LATEST_APP_DOWNLOAD_URL: ${{ vars.VITE_LATEST_APP_DOWNLOAD_URL }} + run: | + yarn --cwd app tauri build \ + -c '${{ steps.config-overrides.outputs.json }}' \ + ${{ matrix.settings.args }} + - name: Locate macOS .app bundle if: matrix.settings.platform == 'macos-latest' id: locate-app @@ -471,7 +549,7 @@ jobs: "${{ steps.locate-app.outputs.bundle_dir }}" - name: Re-upload notarized macOS artifacts to release - if: matrix.settings.platform == 'macos-latest' + if: matrix.settings.platform == 'macos-latest' && needs.prepare-build.outputs.release_enabled == 'true' shell: bash env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} @@ -480,7 +558,7 @@ jobs: bash scripts/release/upload-macos-artifacts.sh \ "${{ steps.locate-app.outputs.app_path }}" \ "${{ steps.locate-app.outputs.bundle_dir }}" \ - "${{ needs.prepare-release.outputs.version }}" \ + "${{ needs.prepare-build.outputs.version }}" \ "${{ matrix.settings.target }}" - name: Verify macOS app bundle sidecar layout @@ -511,7 +589,7 @@ jobs: xcrun stapler validate "$APP_PATH" || echo "WARNING: Staple validation failed" - name: Package CLI tarball and upload to release (unix) - if: matrix.settings.platform != 'windows-latest' + if: matrix.settings.platform != 'windows-latest' && needs.prepare-build.outputs.release_enabled == 'true' shell: bash env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} @@ -533,16 +611,16 @@ jobs: bash scripts/release/package-cli-tarball.sh \ "$BIN" \ - "${{ needs.prepare-release.outputs.version }}" \ + "${{ needs.prepare-build.outputs.version }}" \ "${{ matrix.settings.target }}" - name: Package CLI zip and upload to release (windows) - if: matrix.settings.platform == 'windows-latest' + if: matrix.settings.platform == 'windows-latest' && needs.prepare-build.outputs.release_enabled == 'true' shell: pwsh env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | - $Version = "${{ needs.prepare-release.outputs.version }}" + $Version = "${{ needs.prepare-build.outputs.version }}" $Target = "${{ matrix.settings.target }}" $BinPath = "${{ steps.cli-paths.outputs.cli_path }}" $ZipName = "openhuman-core-${Version}-${Target}.zip" @@ -559,6 +637,15 @@ jobs: gh release upload "v${Version}" $ZipName "${ZipName}.sha256" --repo tinyhumansai/openhuman --clobber Write-Host "[package-cli] Uploaded $ZipName to v${Version}" + - name: Upload staging desktop bundles + if: needs.prepare-build.outputs.release_enabled != 'true' + uses: actions/upload-artifact@v4 + with: + name: desktop-bundles-${{ matrix.settings.platform }}-${{ matrix.settings.artifact_suffix }} + path: | + app/src-tauri/target/${{ matrix.settings.target }}/release/bundle/** + target/${{ matrix.settings.target }}/release/bundle/** + - name: Upload standalone CLI artifacts uses: actions/upload-artifact@v4 with: @@ -571,17 +658,22 @@ jobs: # ========================================================================= build-docker: name: "Docker: build and push" - needs: [prepare-release, create-release] + needs: [prepare-build, create-release] + if: >- + always() + && needs.prepare-build.result == 'success' + && (needs.prepare-build.outputs.release_enabled != 'true' + || needs.create-release.result == 'success') runs-on: ubuntu-latest environment: Production env: REGISTRY: ghcr.io IMAGE_NAME: tinyhumansai/openhuman-core steps: - - name: Checkout tag + - name: Checkout build ref uses: actions/checkout@v4 with: - ref: ${{ needs.prepare-release.outputs.tag }} + ref: ${{ needs.prepare-build.outputs.build_ref }} fetch-depth: 1 - name: Set up Docker Buildx @@ -619,8 +711,11 @@ jobs: name: Publish draft release runs-on: ubuntu-latest environment: Production - needs: [prepare-release, create-release, build-desktop, build-docker] - if: needs.build-desktop.result == 'success' && needs.build-docker.result == 'success' + needs: [prepare-build, create-release, build-desktop, build-docker] + if: >- + needs.prepare-build.outputs.release_enabled == 'true' + && needs.build-desktop.result == 'success' + && needs.build-docker.result == 'success' steps: - name: Validate required installer assets exist uses: actions/github-script@v7 @@ -698,9 +793,10 @@ jobs: name: Remove release and tag if build failed runs-on: ubuntu-latest environment: Production - needs: [prepare-release, create-release, build-desktop, build-docker] + needs: [prepare-build, create-release, build-desktop, build-docker] if: >- always() + && needs.prepare-build.outputs.release_enabled == 'true' && needs.create-release.result == 'success' && (needs.build-desktop.result == 'failure' || needs.build-desktop.result == 'cancelled' || needs.build-docker.result == 'failure' || needs.build-docker.result == 'cancelled') @@ -731,7 +827,7 @@ jobs: script: | const owner = context.repo.owner; const repo = context.repo.repo; - const tag = '${{ needs.prepare-release.outputs.tag }}'; + const tag = '${{ needs.prepare-build.outputs.tag }}'; try { await github.rest.git.deleteRef({ owner, repo, ref: `tags/${tag}` }); @@ -747,7 +843,7 @@ jobs: - name: Delete staging Docker image continue-on-error: true env: - TAG: ${{ needs.prepare-release.outputs.tag }} + TAG: ${{ needs.prepare-build.outputs.tag }} run: | PACKAGE="openhuman-core" STAGING_TAG="staging-${TAG}" diff --git a/app/src/components/composio/toolkitMeta.ts b/app/src/components/composio/toolkitMeta.ts index c43c3d257..8c63f72db 100644 --- a/app/src/components/composio/toolkitMeta.ts +++ b/app/src/components/composio/toolkitMeta.ts @@ -70,6 +70,8 @@ const CATALOG: Record> = { }, }; +export const KNOWN_COMPOSIO_TOOLKITS = Object.freeze(Object.keys(CATALOG)); + export function composioToolkitMeta(slug: string): ComposioToolkitMeta { const key = slug.toLowerCase(); const hit = CATALOG[key]; diff --git a/app/src/lib/composio/composioApi.ts b/app/src/lib/composio/composioApi.ts index 7758e4f89..11d4af236 100644 --- a/app/src/lib/composio/composioApi.ts +++ b/app/src/lib/composio/composioApi.ts @@ -21,23 +21,46 @@ import type { ComposioToolsResponse, } from './types'; +/** + * Every `composio_*` op on the Rust side returns an `RpcOutcome` with a + * user-visible log line attached. `RpcOutcome::into_cli_compatible_json` + * (see `src/rpc/mod.rs`) therefore wraps the payload as + * `{ "result": , "logs": [...] }` before handing it to the + * JSON-RPC layer. This helper peels that envelope back off so every + * caller in this file can work with the flat shapes declared in + * `./types`. Responses without logs pass through unchanged. + */ +function unwrapCliEnvelope(value: unknown): T { + if ( + value !== null && + typeof value === 'object' && + 'result' in (value as Record) && + 'logs' in (value as Record) && + Array.isArray((value as { logs: unknown }).logs) + ) { + return (value as { result: T }).result; + } + return value as T; +} + // ── Read operations ─────────────────────────────────────────────── export async function listToolkits(): Promise { - return callCoreRpc({ method: 'openhuman.composio_list_toolkits' }); + const raw = await callCoreRpc({ method: 'openhuman.composio_list_toolkits' }); + return unwrapCliEnvelope(raw); } export async function listConnections(): Promise { - return callCoreRpc({ - method: 'openhuman.composio_list_connections', - }); + const raw = await callCoreRpc({ method: 'openhuman.composio_list_connections' }); + return unwrapCliEnvelope(raw); } export async function listTools(toolkits?: string[]): Promise { - return callCoreRpc({ + const raw = await callCoreRpc({ method: 'openhuman.composio_list_tools', params: toolkits && toolkits.length > 0 ? { toolkits } : {}, }); + return unwrapCliEnvelope(raw); } // ── Write operations ────────────────────────────────────────────── @@ -48,10 +71,11 @@ export async function listTools(toolkits?: string[]): Promise { - return callCoreRpc({ + const raw = await callCoreRpc({ method: 'openhuman.composio_authorize', params: { toolkit }, }); + return unwrapCliEnvelope(raw); } /** @@ -59,10 +83,11 @@ export async function authorize(toolkit: string): Promise { - return callCoreRpc({ + const raw = await callCoreRpc({ method: 'openhuman.composio_delete_connection', params: { connection_id: connectionId }, }); + return unwrapCliEnvelope(raw); } /** @@ -74,8 +99,9 @@ export async function execute( tool: string, args?: Record ): Promise { - return callCoreRpc({ + const raw = await callCoreRpc({ method: 'openhuman.composio_execute', params: { tool, arguments: args ?? {} }, }); + return unwrapCliEnvelope(raw); } diff --git a/app/src/lib/composio/hooks.test.ts b/app/src/lib/composio/hooks.test.ts new file mode 100644 index 000000000..62e7840c1 --- /dev/null +++ b/app/src/lib/composio/hooks.test.ts @@ -0,0 +1,51 @@ +import { renderHook, waitFor } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const mockListToolkits = vi.fn(); +const mockListConnections = vi.fn(); + +vi.mock('./composioApi', () => ({ + listToolkits: () => mockListToolkits(), + listConnections: () => mockListConnections(), +})); + +describe('useComposioIntegrations', () => { + beforeEach(() => { + vi.resetModules(); + vi.clearAllMocks(); + }); + + it('keeps toolkit cards visible when connections fetch fails', async () => { + const { useComposioIntegrations } = await import('./hooks'); + + mockListToolkits.mockResolvedValue({ toolkits: ['gmail', 'github', 'notion'] }); + mockListConnections.mockRejectedValue(new Error('backend connection listing failed')); + + const { result } = renderHook(() => useComposioIntegrations(0)); + + await waitFor(() => { + expect(result.current.loading).toBe(false); + }); + + expect(result.current.toolkits).toEqual(['gmail', 'github', 'notion']); + expect(result.current.connectionByToolkit.size).toBe(0); + expect(result.current.error).toBe('backend connection listing failed'); + }); + + it('surfaces toolkit fetch errors instead of hiding the UI (composio is always enabled)', async () => { + const { useComposioIntegrations } = await import('./hooks'); + + mockListToolkits.mockRejectedValue(new Error('backend unreachable')); + mockListConnections.mockRejectedValue(new Error('backend unreachable')); + + const { result } = renderHook(() => useComposioIntegrations(0)); + + await waitFor(() => { + expect(result.current.loading).toBe(false); + }); + + expect(result.current.toolkits).toEqual([]); + expect(result.current.connectionByToolkit.size).toBe(0); + expect(result.current.error).toBe('backend unreachable'); + }); +}); diff --git a/app/src/lib/composio/hooks.ts b/app/src/lib/composio/hooks.ts index d1607fd4c..9e48df0c7 100644 --- a/app/src/lib/composio/hooks.ts +++ b/app/src/lib/composio/hooks.ts @@ -16,29 +16,26 @@ export interface UseComposioIntegrationsResult { error: string | null; /** Force a refetch of toolkits + connections. */ refresh: () => Promise; - /** True when composio is disabled on the core side. */ - disabled: boolean; } /** * Fetches the Composio toolkit allowlist and current connections. * - * On mount it does one request of each, then re-fetches connections on - * a `pollIntervalMs` loop so the UI reacts to OAuth completions without + * Composio is always enabled on the core side — it's proxied through + * our backend, uses the same JWT as every other core RPC call, and has + * no client-side feature toggle. So the only failure modes here are + * network/backend errors, which get surfaced via `error`. + * + * On mount we do one request of each, then re-fetch connections on a + * `pollIntervalMs` loop so the UI reacts to OAuth completions without * the user having to manually refresh. Toolkits are only refetched on * explicit `refresh()` because the allowlist is stable. - * - * When the core reports that composio is disabled (feature toggle off, - * or integrations.enabled=false) the hook short-circuits into - * `disabled=true` and stops polling — callers can use that to show an - * "integrations disabled" hint instead of a spinner. */ export function useComposioIntegrations(pollIntervalMs = 5_000): UseComposioIntegrationsResult { const [toolkits, setToolkits] = useState([]); const [connections, setConnections] = useState([]); const [loading, setLoading] = useState(true); const [error, setError] = useState(null); - const [disabled, setDisabled] = useState(false); const mountedRef = useRef(true); useEffect(() => { @@ -49,27 +46,37 @@ export function useComposioIntegrations(pollIntervalMs = 5_000): UseComposioInte }, []); const refresh = useCallback(async () => { + let nextError: string | null = null; try { - const [toolkitsResp, connectionsResp] = await Promise.all([ + const [toolkitsResult, connectionsResult] = await Promise.allSettled([ listToolkits(), listConnections(), ]); if (!mountedRef.current) return; - setToolkits(toolkitsResp.toolkits ?? []); - setConnections(connectionsResp.connections ?? []); - setDisabled(false); - setError(null); - } catch (err) { - if (!mountedRef.current) return; - const message = err instanceof Error ? err.message : String(err); - // Detect the "composio disabled" error the Rust ops layer emits - // so we can render a distinct state rather than a red error. - if (/composio is disabled/i.test(message)) { - setDisabled(true); - setError(null); + + if (toolkitsResult.status === 'fulfilled') { + setToolkits(toolkitsResult.value.toolkits ?? []); } else { - setError(message); + const message = + toolkitsResult.reason instanceof Error + ? toolkitsResult.reason.message + : String(toolkitsResult.reason); + console.warn('[composio] toolkit fetch failed:', message); + nextError = message; } + + if (connectionsResult.status === 'fulfilled') { + setConnections(connectionsResult.value.connections ?? []); + } else { + const message = + connectionsResult.reason instanceof Error + ? connectionsResult.reason.message + : String(connectionsResult.reason); + console.warn('[composio] connection fetch failed:', message); + if (!nextError) nextError = message; + } + + setError(nextError); } finally { if (mountedRef.current) setLoading(false); } @@ -80,18 +87,20 @@ export function useComposioIntegrations(pollIntervalMs = 5_000): UseComposioInte void refresh(); if (pollIntervalMs <= 0) return; const id = window.setInterval(() => { - if (disabled) return; void listConnections() .then(resp => { if (!mountedRef.current) return; setConnections(resp.connections ?? []); }) - .catch(() => { - /* swallow — non-fatal for poll cadence */ + .catch(err => { + console.warn( + '[composio] polling connections failed:', + err instanceof Error ? err.message : String(err) + ); }); }, pollIntervalMs); return () => window.clearInterval(id); - }, [refresh, pollIntervalMs, disabled]); + }, [refresh, pollIntervalMs]); const connectionByToolkit = useMemo(() => { const map = new Map(); @@ -113,5 +122,5 @@ export function useComposioIntegrations(pollIntervalMs = 5_000): UseComposioInte return map; }, [connections]); - return { toolkits, connectionByToolkit, loading, error, refresh, disabled }; + return { toolkits, connectionByToolkit, loading, error, refresh }; } diff --git a/app/src/pages/Skills.tsx b/app/src/pages/Skills.tsx index 62f0fa9ad..401bfba6b 100644 --- a/app/src/pages/Skills.tsx +++ b/app/src/pages/Skills.tsx @@ -1,16 +1,18 @@ -import { useMemo, useState } from 'react'; +import { useEffect, useMemo, useState } from 'react'; import { useNavigate } from 'react-router-dom'; import ChannelSetupModal from '../components/channels/ChannelSetupModal'; import ComposioConnectModal from '../components/composio/ComposioConnectModal'; -import { composioToolkitMeta, type ComposioToolkitMeta } from '../components/composio/toolkitMeta'; +import { + composioToolkitMeta, + type ComposioToolkitMeta, + KNOWN_COMPOSIO_TOOLKITS, +} from '../components/composio/toolkitMeta'; import AutocompleteSetupModal from '../components/skills/AutocompleteSetupModal'; import ScreenIntelligenceSetupModal from '../components/skills/ScreenIntelligenceSetupModal'; -import { SKILL_ICONS, type SkillListEntry } from '../components/skills/shared'; -import UnifiedSkillCard, { ThirdPartySkillCard } from '../components/skills/SkillCard'; +import UnifiedSkillCard from '../components/skills/SkillCard'; import SkillCategoryFilter, { type SkillCategory } from '../components/skills/SkillCategoryFilter'; import SkillSearchBar from '../components/skills/SkillSearchBar'; -import SkillSetupModal from '../components/skills/SkillSetupModal'; import VoiceSetupModal from '../components/skills/VoiceSetupModal'; import { useAutocompleteSkillStatus } from '../features/autocomplete/useAutocompleteSkillStatus'; import { useScreenIntelligenceSkillStatus } from '../features/screen-intelligence/useScreenIntelligenceSkillStatus'; @@ -18,11 +20,8 @@ import { useVoiceSkillStatus } from '../features/voice/useVoiceSkillStatus'; import { useChannelDefinitions } from '../hooks/useChannelDefinitions'; import { useComposioIntegrations } from '../lib/composio/hooks'; import { type ComposioConnection, deriveComposioState } from '../lib/composio/types'; -import { useAvailableSkills } from '../lib/skills/hooks'; -import { installSkill } from '../lib/skills/skillsApi'; import { useAppSelector } from '../store/hooks'; import type { ChannelConnectionStatus, ChannelDefinition, ChannelType } from '../types/channels'; -import { IS_DEV } from '../utils/config'; const CHANNEL_ICONS: Record = { telegram: '\u2708\uFE0F', @@ -174,15 +173,13 @@ interface SkillItem { name: string; description: string; category: SkillCategory; - kind: 'builtin' | 'channel' | 'third-party' | 'composio'; + kind: 'builtin' | 'channel' | 'composio'; // For built-in route?: string; icon?: React.ReactNode; // For channel channelDef?: ChannelDefinition; channelStatus?: ChannelConnectionStatus; - // For third-party - skill?: SkillListEntry; // For composio composioToolkit?: ComposioToolkitMeta; composioConnection?: ComposioConnection; @@ -192,26 +189,20 @@ interface SkillItem { export default function Skills() { const navigate = useNavigate(); - const { skills: availableSkills, loading: skillsLoading } = useAvailableSkills(); const { definitions: channelDefs } = useChannelDefinitions(); const channelConnections = useAppSelector(state => state.channelConnections); const { toolkits: composioToolkits, connectionByToolkit: composioConnectionByToolkit, + error: composioError, refresh: refreshComposio, } = useComposioIntegrations(); - const [setupModalOpen, setSetupModalOpen] = useState(false); - const [activeSkillId, setActiveSkillId] = useState(null); - const [activeSkillName, setActiveSkillName] = useState(''); - const [activeSkillDescription, setActiveSkillDescription] = useState(''); - const [activeSkillHasSetup, setActiveSkillHasSetup] = useState(false); const [channelModalDef, setChannelModalDef] = useState(null); const [composioModalToolkit, setComposioModalToolkit] = useState( null ); - const [installing, setInstalling] = useState(null); const [screenIntelligenceModalOpen, setScreenIntelligenceModalOpen] = useState(false); const [autocompleteModalOpen, setAutocompleteModalOpen] = useState(false); const [voiceModalOpen, setVoiceModalOpen] = useState(false); @@ -222,6 +213,16 @@ export default function Skills() { const [searchQuery, setSearchQuery] = useState(''); const [selectedCategory, setSelectedCategory] = useState('All'); + useEffect(() => { + if (!import.meta.env.DEV) return; + console.debug('[skills][composio] hook result', { + toolkitCount: composioToolkits.length, + connectionCount: composioConnectionByToolkit.size, + hasError: Boolean(composioError), + error: composioError, + }); + }, [composioToolkits, composioConnectionByToolkit, composioError]); + const bestChannelStatus = (channelId: ChannelType): ChannelConnectionStatus => { const conns = channelConnections.connections[channelId]; if (!conns) return 'disconnected'; @@ -237,38 +238,23 @@ export default function Skills() { [channelDefs] ); - const skillsList: SkillListEntry[] = useMemo(() => { - return availableSkills - .filter(e => { - if (e.id.includes('_')) return false; - if (!IS_DEV && e.ignore_in_production) return false; - return true; - }) - .map(e => ({ - id: e.id, - name: e.name || e.id.charAt(0).toUpperCase() + e.id.slice(1), - description: e.description || '', - icon: SKILL_ICONS[e.id], - ignoreInProduction: e.ignore_in_production, - hasSetup: !!(e.setup && e.setup.required), - })); - }, [availableSkills]); - - const openSkillSetup = async (skill: SkillListEntry) => { - try { - setInstalling(skill.id); - await installSkill(skill.id); - } catch (err) { - console.warn(`[Skills] install failed for ${skill.id}, continuing anyway:`, err); - } finally { - setInstalling(null); + const composioCatalogToolkits = useMemo(() => { + const normalizedToolkits = composioToolkits.map(slug => slug.toLowerCase()); + const missingKnownToolkits = KNOWN_COMPOSIO_TOOLKITS.filter( + slug => !normalizedToolkits.includes(slug) + ); + if (import.meta.env.DEV && missingKnownToolkits.length > 0) { + console.debug('[skills][composio] filling gaps from KNOWN_COMPOSIO_TOOLKITS', { + toolkitCount: composioToolkits.length, + connectionCount: composioConnectionByToolkit.size, + hasError: Boolean(composioError), + missingKnownToolkits, + }); } - setActiveSkillId(skill.id); - setActiveSkillName(skill.name); - setActiveSkillDescription(skill.description); - setActiveSkillHasSetup(skill.hasSetup); - setSetupModalOpen(true); - }; + return Array.from(new Set([...KNOWN_COMPOSIO_TOOLKITS, ...normalizedToolkits])).sort((a, b) => + a.localeCompare(b) + ); + }, [composioToolkits, composioConnectionByToolkit, composioError]); // Unified item list const allItems: SkillItem[] = useMemo(() => { @@ -299,24 +285,11 @@ export default function Skills() { }); } - const sortedSkills = [...skillsList].sort((a, b) => a.name.localeCompare(b.name)); - for (const skill of sortedSkills) { - items.push({ - id: skill.id, - name: skill.name, - description: skill.description, - category: 'Other', - kind: 'third-party', - skill, - }); - } - // Composio toolkits — rendered with the same UnifiedSkillCard used // for channels/skills so they sit flush in the grid. Each entry is // keyed by slug and routed through `ComposioConnectModal` for the // authorize/OAuth/poll flow. - const sortedToolkits = [...composioToolkits].sort((a, b) => a.localeCompare(b)); - for (const slug of sortedToolkits) { + for (const slug of composioCatalogToolkits) { const meta = composioToolkitMeta(slug); const connection = composioConnectionByToolkit.get(meta.slug); items.push({ @@ -334,10 +307,9 @@ export default function Skills() { return items; // eslint-disable-next-line react-hooks/exhaustive-deps }, [ - skillsList, configurableChannels, channelConnections, - composioToolkits, + composioCatalogToolkits, composioConnectionByToolkit, ]); @@ -396,11 +368,26 @@ export default function Skills() { onChange={setSelectedCategory} /> - {skillsLoading ? ( -
-

Loading skills...

+ {composioError && ( +
+
+
+

+ Integrations are showing stale status +

+

{composioError}

+
+ +
- ) : filteredItems.length === 0 ? ( + )} + + {filteredItems.length === 0 ? (

No skills found

@@ -529,9 +516,11 @@ export default function Skills() { if (item.kind === 'composio') { const meta = item.composioToolkit!; const connection = item.composioConnection; - const state = deriveComposioState(connection); - const ctaLabel = - state === 'connected' + const hasComposioError = Boolean(composioError); + const state = hasComposioError ? 'error' : deriveComposioState(connection); + const ctaLabel = hasComposioError + ? 'Retry' + : state === 'connected' ? 'Manage' : state === 'pending' ? 'Waiting' @@ -540,30 +529,38 @@ export default function Skills() { : 'Connect'; const ctaVariant: 'primary' | 'sage' | 'amber' = state === 'connected' ? 'sage' : state === 'error' ? 'amber' : 'primary'; + const description = hasComposioError + ? `${item.description} ${composioError}` + : item.description; return ( setComposioModalToolkit(meta)} + onCtaClick={() => { + if (hasComposioError) { + void refreshComposio(); + return; + } + setComposioModalToolkit(meta); + }} /> ); } - // third-party - return ( - openSkillSetup(item.skill!)} - /> - ); })}
@@ -573,19 +570,6 @@ export default function Skills() { - {setupModalOpen && activeSkillId && ( - { - setSetupModalOpen(false); - setActiveSkillId(null); - }} - /> - )} - {channelModalDef && ( setChannelModalDef(null)} /> )} diff --git a/app/src/pages/__tests__/Skills.composio-catalog.test.tsx b/app/src/pages/__tests__/Skills.composio-catalog.test.tsx new file mode 100644 index 000000000..23e97a4b7 --- /dev/null +++ b/app/src/pages/__tests__/Skills.composio-catalog.test.tsx @@ -0,0 +1,81 @@ +import { fireEvent, screen, within } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import '../../test/mockDefaultSkillStatusHooks'; +import { renderWithProviders } from '../../test/test-utils'; +import Skills from '../Skills'; + +let composioRefresh = vi.fn(); +let composioError: string | null = null; +let composioToolkits: string[] = []; +let composioConnectionByToolkit = new Map(); + +vi.mock('../../hooks/useChannelDefinitions', () => ({ + useChannelDefinitions: () => ({ definitions: [], loading: false, error: null }), +})); + +vi.mock('../../lib/skills/skillsApi', () => ({ + installSkill: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock('../../lib/skills/hooks', () => ({ + useAvailableSkills: () => ({ skills: [], loading: false, refresh: vi.fn() }), +})); + +vi.mock('../../lib/composio/hooks', () => ({ + useComposioIntegrations: () => ({ + toolkits: composioToolkits, + connectionByToolkit: composioConnectionByToolkit, + refresh: composioRefresh, + loading: false, + error: composioError, + }), +})); + +describe('Skills page — Composio catalog fallback', () => { + beforeEach(() => { + composioRefresh = vi.fn(); + composioError = null; + composioToolkits = []; + composioConnectionByToolkit = new Map(); + }); + + it('shows known composio integrations in their configured category groups when the live toolkit list is empty', () => { + renderWithProviders(, { initialEntries: ['/skills'] }); + + expect(screen.getByRole('heading', { name: 'Productivity' })).toBeInTheDocument(); + expect(screen.getByRole('heading', { name: 'Tools & Automation' })).toBeInTheDocument(); + expect(screen.getByRole('heading', { name: 'Social' })).toBeInTheDocument(); + expect(screen.getByText('Google Calendar')).toBeInTheDocument(); + expect(screen.getByText('Google Drive')).toBeInTheDocument(); + expect(screen.getByText('Gmail')).toBeInTheDocument(); + expect(screen.getByText('Notion')).toBeInTheDocument(); + expect(screen.getByText('GitHub')).toBeInTheDocument(); + expect(screen.getByText('Linear')).toBeInTheDocument(); + expect(screen.getByText('Slack')).toBeInTheDocument(); + expect(screen.queryByRole('heading', { name: 'Other' })).not.toBeInTheDocument(); + }); + + it('shows a stale/error state instead of disconnected toolkits when composio loading fails', () => { + composioError = 'Backend unavailable'; + + renderWithProviders(, { initialEntries: ['/skills'] }); + + expect(screen.getByText('Integrations are showing stale status')).toBeInTheDocument(); + expect(screen.getByText('Backend unavailable')).toBeInTheDocument(); + + const productivitySection = screen + .getByRole('heading', { name: 'Productivity' }) + .closest('.rounded-2xl'); + expect(productivitySection).not.toBeNull(); + const gmailCard = within(productivitySection as HTMLElement) + .getByText('Gmail') + .closest('.rounded-xl'); + expect(gmailCard).not.toBeNull(); + expect(within(gmailCard as HTMLElement).getByText('Status unavailable')).toBeInTheDocument(); + expect(within(gmailCard as HTMLElement).getByText(/Backend unavailable/)).toBeInTheDocument(); + + fireEvent.click(screen.getAllByRole('button', { name: 'Retry' })[0]); + expect(composioRefresh).toHaveBeenCalledTimes(1); + }); +}); diff --git a/app/src/pages/__tests__/Skills.third-party-gmail-sync.test.tsx b/app/src/pages/__tests__/Skills.third-party-gmail-sync.test.tsx index fed24715c..2428f1574 100644 --- a/app/src/pages/__tests__/Skills.third-party-gmail-sync.test.tsx +++ b/app/src/pages/__tests__/Skills.third-party-gmail-sync.test.tsx @@ -1,79 +1,50 @@ -/** - * Skills page — 3rd Party Skills: manual sync triggers core `openhuman.skills_sync` - * via `skillManager.triggerSync` (see `lib/skills/manager.ts`). - */ -import { fireEvent, screen, waitFor } from '@testing-library/react'; -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { fireEvent, screen, within } from '@testing-library/react'; +import { describe, expect, it, vi } from 'vitest'; import '../../test/mockDefaultSkillStatusHooks'; import { renderWithProviders } from '../../test/test-utils'; import Skills from '../Skills'; -const gmailRegistryEntry = { - id: 'gmail', - name: 'Gmail', - version: '1.1.0', - description: 'Read and send email via Gmail.', - runtime: 'quickjs', - entry: 'index.js', - auto_start: false, - platforms: ['macos', 'linux', 'windows'], - setup: { required: true, label: 'Connect Gmail' }, -}; - -const mocks = vi.hoisted(() => ({ - triggerSync: vi.fn().mockResolvedValue(undefined), - startSkill: vi.fn().mockResolvedValue(undefined), -})); - vi.mock('../../hooks/useChannelDefinitions', () => ({ useChannelDefinitions: () => ({ definitions: [], loading: false, error: null }), })); -vi.mock('../../lib/skills/manager', () => ({ - skillManager: { triggerSync: mocks.triggerSync, startSkill: mocks.startSkill }, -})); - vi.mock('../../lib/skills/skillsApi', () => ({ installSkill: vi.fn().mockResolvedValue(undefined), })); vi.mock('../../lib/skills/hooks', () => ({ - useAvailableSkills: () => ({ skills: [gmailRegistryEntry], loading: false, refresh: vi.fn() }), - useSkillConnectionStatus: (skillId: string) => (skillId === 'gmail' ? 'connected' : 'offline'), - useSkillState: () => ({ - connection_status: 'connected', - auth_status: 'authenticated', - syncInProgress: false, - syncProgress: 0, - syncProgressMessage: '', - }), - useSkillDataDirectoryStats: () => undefined, + useAvailableSkills: () => ({ skills: [], loading: false, refresh: vi.fn() }), })); -describe('Skills page — 3rd Party Gmail sync', () => { - beforeEach(() => { - mocks.triggerSync.mockClear(); - mocks.startSkill.mockClear(); - }); +vi.mock('../../lib/composio/hooks', () => ({ + useComposioIntegrations: () => ({ + toolkits: ['gmail'], + connectionByToolkit: new Map([ + ['gmail', { id: 'conn_gmail_1', toolkit: 'gmail', status: 'ACTIVE' }], + ]), + refresh: vi.fn(), + loading: false, + error: null, + }), +})); - it('renders 3rd Party Skills and Gmail with a Sync control when connected', async () => { +describe('Skills page — Gmail composio integration', () => { + it('renders Gmail as a connected composio integration and opens its management modal', async () => { renderWithProviders(, { initialEntries: ['/skills'] }); - expect(screen.getByText('Gmail')).toBeInTheDocument(); + const productivitySection = screen + .getByRole('heading', { name: 'Productivity' }) + .closest('.rounded-2xl'); + expect(productivitySection).not.toBeNull(); + expect(within(productivitySection as HTMLElement).getByText('Gmail')).toBeInTheDocument(); + expect(within(productivitySection as HTMLElement).getByText('Connected')).toBeInTheDocument(); - // Sync button is inside the overflow menu — open it first - const moreBtn = screen.getByTitle('More actions'); - fireEvent.click(moreBtn); + fireEvent.click( + within(productivitySection as HTMLElement).getByRole('button', { name: 'Manage' }) + ); - const syncBtn = await screen.findByTestId('skill-sync-button-gmail'); - expect(syncBtn).toBeInTheDocument(); - - fireEvent.click(syncBtn); - - await waitFor(() => { - expect(mocks.triggerSync).toHaveBeenCalledTimes(1); - }); - expect(mocks.triggerSync).toHaveBeenCalledWith('gmail'); + expect(await screen.findByRole('heading', { name: 'Manage Gmail' })).toBeInTheDocument(); + expect(screen.getByText(/Gmail is connected\./i)).toBeInTheDocument(); }); }); diff --git a/app/src/pages/__tests__/Skills.third-party-notion-debug-tools.test.tsx b/app/src/pages/__tests__/Skills.third-party-notion-debug-tools.test.tsx index 4ba49c282..c496380d9 100644 --- a/app/src/pages/__tests__/Skills.third-party-notion-debug-tools.test.tsx +++ b/app/src/pages/__tests__/Skills.third-party-notion-debug-tools.test.tsx @@ -1,178 +1,48 @@ -/** - * Skills page — 3rd Party Notion: debug modal tool execution matches - * `openhuman-skills/src/core/notion/live-test.ts` tool exercises (sections 5b + 7). - * UI path: Skills → Debug → Tools tab → expand tool → Execute → `openhuman.skills_call_tool`. - */ -import { fireEvent, screen, waitFor } from '@testing-library/react'; -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { fireEvent, screen, within } from '@testing-library/react'; +import { describe, expect, it, vi } from 'vitest'; import '../../test/mockDefaultSkillStatusHooks'; import { renderWithProviders } from '../../test/test-utils'; import Skills from '../Skills'; -const notionRegistryEntry = { - id: 'notion', - name: 'Notion', - version: '1.2.0', - description: 'Notion workspace integration.', - runtime: 'quickjs', - entry: 'index.js', - auto_start: false, - platforms: ['macos', 'linux', 'windows'], - setup: { required: true, label: 'Connect Notion' }, -}; - -const notionSnapshot = { - skill_id: 'notion', - name: 'Notion', - status: 'running', - tools: [ - { name: 'sync-status', description: 'Sync status', inputSchema: { type: 'object' } }, - { name: 'list-users', description: 'List users', inputSchema: { type: 'object' } }, - { name: 'search', description: 'Search', inputSchema: { type: 'object' } }, - { name: 'list-pages', description: 'List pages', inputSchema: { type: 'object' } }, - { name: 'list-databases', description: 'List databases', inputSchema: { type: 'object' } }, - ], - error: null as string | null, - state: { connection_status: 'connected', auth_status: 'authenticated', is_initialized: true }, - setup_complete: true, - connection_status: 'connected', -}; - -const mocks = vi.hoisted(() => ({ - triggerSync: vi.fn().mockResolvedValue(undefined), - startSkill: vi.fn().mockResolvedValue(undefined), - callCoreRpc: vi - .fn() - .mockResolvedValue({ content: [{ type: 'text', text: '{"ok":true}' }], is_error: false }), -})); - vi.mock('../../hooks/useChannelDefinitions', () => ({ useChannelDefinitions: () => ({ definitions: [], loading: false, error: null }), })); -vi.mock('../../lib/skills/manager', () => ({ - skillManager: { triggerSync: mocks.triggerSync, startSkill: mocks.startSkill }, -})); - vi.mock('../../lib/skills/skillsApi', () => ({ installSkill: vi.fn().mockResolvedValue(undefined), })); -vi.mock('../../services/coreRpcClient', () => ({ callCoreRpc: mocks.callCoreRpc })); - vi.mock('../../lib/skills/hooks', () => ({ - useAvailableSkills: () => ({ skills: [notionRegistryEntry], loading: false, refresh: vi.fn() }), - useSkillConnectionStatus: (skillId: string) => (skillId === 'notion' ? 'connected' : 'offline'), - useSkillState: () => ({ - connection_status: 'connected', - auth_status: 'authenticated', - syncInProgress: false, - syncProgress: 0, - syncProgressMessage: '', - }), - useSkillDataDirectoryStats: () => undefined, - useSkillSnapshot: (skillId: string | undefined) => (skillId === 'notion' ? notionSnapshot : null), + useAvailableSkills: () => ({ skills: [], loading: false, refresh: vi.fn() }), })); -function expectToolCallArgs(toolName: string, args: Record) { - expect(mocks.callCoreRpc).toHaveBeenCalledWith({ - method: 'openhuman.skills_call_tool', - params: { skill_id: 'notion', tool_name: toolName, arguments: args }, - }); -} +vi.mock('../../lib/composio/hooks', () => ({ + useComposioIntegrations: () => ({ + toolkits: ['notion'], + connectionByToolkit: new Map(), + refresh: vi.fn(), + loading: false, + error: null, + }), +})); -describe('Skills page — Notion debug tools (live-test parity)', () => { - beforeEach(() => { - mocks.triggerSync.mockClear(); - mocks.startSkill.mockClear(); - mocks.callCoreRpc.mockClear(); - }); - - it('opens debug modal and executes tools via openhuman.skills_call_tool', async () => { +describe('Skills page — Notion composio integration', () => { + it('renders Notion as a disconnected composio integration and opens its connect modal', async () => { renderWithProviders(, { initialEntries: ['/skills'] }); - expect(screen.getByText('Notion')).toBeInTheDocument(); + expect(screen.getByRole('heading', { name: 'Productivity' })).toBeInTheDocument(); + const notionTitle = screen.getByText('Notion'); + const notionCard = notionTitle.closest('div.flex-1')?.parentElement; + expect(notionCard).not.toBeNull(); + expect(notionTitle).toBeInTheDocument(); + expect( + within(notionCard as HTMLElement).getByRole('button', { name: 'Connect' }) + ).toBeInTheDocument(); - // Debug button is inside the overflow menu — open it first - const moreBtn = screen.getByTitle('More actions'); - fireEvent.click(moreBtn); + fireEvent.click(within(notionCard as HTMLElement).getByRole('button', { name: 'Connect' })); - fireEvent.click(await screen.findByTestId('skill-debug-button-notion')); - - await waitFor(() => { - expect(screen.getByRole('heading', { name: /Debug: Notion/i })).toBeInTheDocument(); - }); - - fireEvent.click(screen.getByTestId('skill-debug-tab-tools')); - - // --- Section 5b / 7: same tools as live-test.ts (defaults + search query) --- - const steps: Array<{ tool: string; argsJson?: string }> = [ - { tool: 'sync-status' }, - { tool: 'list-users' }, - { tool: 'search', argsJson: '{"query":"test"}' }, - { tool: 'list-pages', argsJson: '{"page_size":10}' }, - { tool: 'list-databases' }, - ]; - - for (const step of steps) { - mocks.callCoreRpc.mockClear(); - fireEvent.click(screen.getByTestId(`skill-debug-tool-header-${step.tool}`)); - if (step.argsJson) { - fireEvent.change(screen.getByTestId(`skill-debug-tool-args-${step.tool}`), { - target: { value: step.argsJson }, - }); - } - fireEvent.click(screen.getByTestId(`skill-debug-execute-${step.tool}`)); - await waitFor(() => { - expect(mocks.callCoreRpc).toHaveBeenCalled(); - }); - const parsed = - step.argsJson !== undefined ? JSON.parse(step.argsJson) : ({} as Record); - expectToolCallArgs(step.tool, parsed); - } - - // --- live-test: repeat list-* with tryCache:false (same tool rows, new args) --- - mocks.callCoreRpc.mockClear(); - fireEvent.click(screen.getByTestId('skill-debug-tool-header-list-pages')); - fireEvent.change(screen.getByTestId('skill-debug-tool-args-list-pages'), { - target: { value: '{"page_size":10,"tryCache":false}' }, - }); - fireEvent.click(screen.getByTestId('skill-debug-execute-list-pages')); - await waitFor(() => expect(mocks.callCoreRpc).toHaveBeenCalled()); - expectToolCallArgs('list-pages', { page_size: 10, tryCache: false }); - - mocks.callCoreRpc.mockClear(); - fireEvent.click(screen.getByTestId('skill-debug-tool-header-list-databases')); - fireEvent.change(screen.getByTestId('skill-debug-tool-args-list-databases'), { - target: { value: '{"tryCache":false}' }, - }); - fireEvent.click(screen.getByTestId('skill-debug-execute-list-databases')); - await waitFor(() => expect(mocks.callCoreRpc).toHaveBeenCalled()); - expectToolCallArgs('list-databases', { tryCache: false }); - - mocks.callCoreRpc.mockClear(); - fireEvent.click(screen.getByTestId('skill-debug-tool-header-list-users')); - fireEvent.change(screen.getByTestId('skill-debug-tool-args-list-users'), { - target: { value: '{"tryCache":false}' }, - }); - fireEvent.click(screen.getByTestId('skill-debug-execute-list-users')); - await waitFor(() => expect(mocks.callCoreRpc).toHaveBeenCalled()); - expectToolCallArgs('list-users', { tryCache: false }); - }); - - it('Sync control calls skillManager.triggerSync(notion)', async () => { - renderWithProviders(, { initialEntries: ['/skills'] }); - - // Sync button is inside the overflow menu — open it first - const moreBtn = screen.getByTitle('More actions'); - fireEvent.click(moreBtn); - - fireEvent.click(await screen.findByTestId('skill-sync-button-notion')); - - await waitFor(() => { - expect(mocks.triggerSync).toHaveBeenCalledTimes(1); - }); - expect(mocks.triggerSync).toHaveBeenCalledWith('notion'); + expect(await screen.findByRole('heading', { name: 'Connect Notion' })).toBeInTheDocument(); + expect(screen.getByText(/Connect your Notion account through Composio\./i)).toBeInTheDocument(); }); }); diff --git a/src/openhuman/agent/agents/skills_agent/agent.toml b/src/openhuman/agent/agents/skills_agent/agent.toml index cf5df414e..f44c0acc1 100644 --- a/src/openhuman/agent/agents/skills_agent/agent.toml +++ b/src/openhuman/agent/agents/skills_agent/agent.toml @@ -1,6 +1,6 @@ id = "skills_agent" display_name = "Skills Agent" -when_to_use = "Skill tool specialist — executes installed QuickJS skill tools (Notion, Gmail, …). Use when the task should be completed via a user-installed skill rather than raw HTTP/file I/O. Pair with a `skill_filter` argument to scope to a single skill." +when_to_use = "Service integration specialist — executes skill-category tools that reach external SaaS. This covers Composio (1000+ OAuth integrations: Gmail, Notion, GitHub, Slack, …) as well as any installed QuickJS skill tools. Use when the task should be completed via a managed integration rather than raw HTTP/file I/O. Pair with a `skill_filter` argument to scope to a single QuickJS skill when applicable." temperature = 0.4 max_iterations = 10 sandbox_mode = "none" diff --git a/src/openhuman/agent/agents/skills_agent/prompt.md b/src/openhuman/agent/agents/skills_agent/prompt.md index f0be2d5a6..ceca84c08 100644 --- a/src/openhuman/agent/agents/skills_agent/prompt.md +++ b/src/openhuman/agent/agents/skills_agent/prompt.md @@ -1,23 +1,30 @@ # Skills Agent — Service Integration Specialist -You are the **Skills Agent**. You interact with connected services through skill tools. +You are the **Skills Agent**. You interact with connected external services — primarily through **Composio** (a managed OAuth gateway for 1000+ apps like Gmail, Notion, GitHub, Slack) and, when installed, user-provided **QuickJS skill tools**. -## Tool Naming Convention +## Available tool surfaces -Tools follow the pattern: `{skill_id}__{tool_name}` -Examples: `notion__create_page`, `gmail__send_email`, `notion__query_database` +1. **Composio tools** — a small meta-surface that discovers and executes Composio actions on the user's behalf: + - `composio_list_toolkits` — what integrations the backend allows (e.g. `gmail`, `notion`). + - `composio_list_connections` — which of those the user has already authorised. + - `composio_authorize` — start an OAuth handoff for a toolkit; returns a `connectUrl`. + - `composio_list_tools` — list available action schemas (optionally filtered by toolkit). Use the returned `function.name` slug as the `tool` argument to `composio_execute`. + - `composio_execute` — run a Composio action with `{ tool, arguments }` (e.g. `tool = "GMAIL_SEND_EMAIL"`). +2. **QuickJS skill tools** — when present, these follow the `{skill_id}__{tool_name}` convention (e.g. `notion__create_page`, `gmail__send_email`). They behave like any other tool but run inside the skill runtime. -## Capabilities +## Typical Composio flow -- Execute any registered skill tool -- Use injected memory context about previous interactions -- Handle rate limits with appropriate delays -- Recover from transient failures with retries +1. Call `composio_list_connections` to see what the user already has connected. +2. If the required toolkit is missing, call `composio_authorize` and return the `connectUrl` so the user can complete OAuth. +3. Once connected, call `composio_list_tools` (optionally scoped to one or two toolkits) to discover the action slug and its JSON schema. +4. Call `composio_execute` with the slug and argument object. ## Rules -- **Respect rate limits** — Notion: max 3 requests/second. Gmail: respect quota limits. -- **Handle errors gracefully** — OAuth token expiry, API errors, rate limits — retry or report clearly. -- **Use memory context** — Consult the injected memory context (provided in your system prompt) for details about the user's integrations and preferences. -- **Be precise** — Skill tools expect specific parameter formats. Validate before calling. -- **Report results** — State what action was taken and the outcome. +- **Prefer Composio** for standard SaaS tasks unless a QuickJS skill offers a better-fit capability. +- **Never fabricate action slugs.** Always pull them from `composio_list_tools` before calling `composio_execute`. +- **Respect rate limits** — Composio and upstream providers both throttle. Back off on errors rather than retrying tightly. +- **Handle OAuth expiry** — if an action fails with an auth error, surface the need to re-authorise rather than looping. +- **Use memory context** — consult the injected memory context for details about the user's integrations and preferences. +- **Be precise** — every tool expects a specific argument shape. Validate against the schema from `composio_list_tools` before calling. +- **Report results** — state what action was taken and the outcome, including any cost reported by Composio. diff --git a/src/openhuman/agent/harness/subagent_runner.rs b/src/openhuman/agent/harness/subagent_runner.rs index 371a840c6..3268a034f 100644 --- a/src/openhuman/agent/harness/subagent_runner.rs +++ b/src/openhuman/agent/harness/subagent_runner.rs @@ -868,6 +868,90 @@ mod tests { assert_eq!(names, vec!["notion__search", "notion__read"]); } + /// End-to-end verification that a sub-agent with + /// `category_filter = "skill"` (like the built-in `skills_agent`) sees + /// the real Composio tools alongside any other `Skill`-category tools + /// and does **not** see `System`-category tools. + /// + /// This is the regression test for "skills subagent has access to + /// composio tools": if any of the composio tool impls forgets to + /// override `category()` and falls back to the default `System`, it + /// gets filtered out here and this test fails. + #[test] + fn skills_subagent_filter_admits_composio_tools() { + use crate::openhuman::composio::client::ComposioClient; + use crate::openhuman::composio::tools::{ + ComposioAuthorizeTool, ComposioExecuteTool, ComposioListConnectionsTool, + ComposioListToolkitsTool, ComposioListToolsTool, + }; + use crate::openhuman::integrations::IntegrationClient; + use std::sync::Arc; + + // Build a throwaway composio client. The filter only touches + // `Tool::name()` and `Tool::category()`, so no HTTP calls happen. + let inner = + IntegrationClient::new("http://127.0.0.1:0".to_string(), "test-token".to_string()); + let client = ComposioClient::new(Arc::new(inner)); + + // Parent registry = the five real Composio tools + a couple of + // plain system-category stubs. We expect exactly the composio + // tools to survive the skills sub-agent's category filter. + let parent: Vec> = vec![ + Box::new(ComposioListToolkitsTool::new(client.clone())), + Box::new(ComposioListConnectionsTool::new(client.clone())), + Box::new(ComposioAuthorizeTool::new(client.clone())), + Box::new(ComposioListToolsTool::new(client.clone())), + Box::new(ComposioExecuteTool::new(client)), + stub("file_read"), + stub("shell"), + ]; + + // Mirror the skills_agent definition: wildcard tool scope, + // category_filter = Skill, no skill_filter. + let mut def = make_def_named_tools(&[]); + def.tools = ToolScope::Wildcard; + let idx = filter_tool_indices( + &parent, + &def.tools, + &def.disallowed_tools, + None, + Some(ToolCategory::Skill), + ); + + let surviving: Vec<&str> = idx.iter().map(|&i| parent[i].name()).collect(); + + // All five composio tools must be present. + for expected in &[ + "composio_list_toolkits", + "composio_list_connections", + "composio_authorize", + "composio_list_tools", + "composio_execute", + ] { + assert!( + surviving.contains(expected), + "skills sub-agent filter dropped composio tool `{}` — \ + did someone remove the `category()` override? \ + surviving = {:?}", + expected, + surviving, + ); + } + + // System-category tools must be filtered out. + assert!(!surviving.contains(&"file_read")); + assert!(!surviving.contains(&"shell")); + + // And we should see exactly 5 survivors, no more, no less. + assert_eq!( + surviving.len(), + 5, + "expected exactly 5 composio tools to pass the skills filter, \ + got {:?}", + surviving, + ); + } + #[test] fn subagent_mode_as_str_roundtrip() { assert_eq!(SubagentMode::Typed.as_str(), "typed"); diff --git a/src/openhuman/channels/runtime/dispatch.rs b/src/openhuman/channels/runtime/dispatch.rs index 4306dfce5..540cfc776 100644 --- a/src/openhuman/channels/runtime/dispatch.rs +++ b/src/openhuman/channels/runtime/dispatch.rs @@ -24,24 +24,6 @@ use tokio_util::sync::CancellationToken; /// real responses while keeping terminal output readable. const REPLY_LOG_TRUNCATE_CHARS: usize = 200; -fn channel_delivery_instructions(channel_name: &str) -> Option<&'static str> { - match channel_name { - "telegram" => Some( - "When responding on Telegram you may send media attachments using markers: \ - [IMAGE:], [DOCUMENT:], [VIDEO:], [AUDIO:], [VOICE:]. \ - You may also react to the user's message by placing [REACTION:] at the \ - very start of your reply. The reaction replaces the automatic acknowledgment \ - the user already saw. Choose based on actual message intent — for example: \ - 👍 agreement · ❤️ warmth/thanks · 🔥 excitement · 🤔 careful thought · \ - 🤯 surprise · 💯 strong agreement · ⚡ urgency · 👨‍💻 technical topic · \ - 🎉 celebration · 🙏 gratitude. \ - A reaction can be combined with a reply: [REACTION:🔥] Here's what I found… \ - Only react when it genuinely fits — skip it for neutral factual responses.", - ), - _ => None, - } -} - /// Returns `true` if `s` contains any of the given substrings. #[inline] fn contains_any(s: &str, words: &[&str]) -> bool { @@ -313,10 +295,6 @@ pub(crate) async fn process_channel_message( history.append(&mut prior_turns); history.push(ChatMessage::user(&enriched_message)); - if let Some(instructions) = channel_delivery_instructions(&msg.channel) { - history.push(ChatMessage::system(instructions)); - } - // Determine if this channel supports streaming draft updates let use_streaming = target_channel .as_ref() diff --git a/src/openhuman/composio/client.rs b/src/openhuman/composio/client.rs index 4043c217d..e26e0003f 100644 --- a/src/openhuman/composio/client.rs +++ b/src/openhuman/composio/client.rs @@ -164,9 +164,15 @@ impl ComposioClient { // we'd need to clone or expose the existing `reqwest::Client` // from `IntegrationClient`, which we intentionally avoid so the // public surface of that type doesn't widen for one caller. + // + // Mirror the TLS settings of the shared client + // (`use_rustls_tls + http1_only`) so this path has the same + // connection behaviour as the other backend calls. let http_client = reqwest::Client::builder() + .use_rustls_tls() + .http1_only() .timeout(std::time::Duration::from_secs(60)) - .connect_timeout(std::time::Duration::from_secs(10)) + .connect_timeout(std::time::Duration::from_secs(15)) .build()?; let resp = http_client @@ -200,21 +206,14 @@ impl ComposioClient { } } -/// Build a [`ComposioClient`] from the integrations config. Returns -/// `None` when either the integrations master switch is off, the -/// composio sub-toggle is off, or the backend URL / auth token are -/// missing. -pub fn build_composio_client( - config: &crate::openhuman::config::IntegrationsConfig, -) -> Option { - if !config.enabled { - tracing::debug!("[composio] integrations master switch off — skipping"); - return None; - } - if !config.composio.enabled { - tracing::debug!("[composio] composio toggle off — skipping"); - return None; - } +/// Build a [`ComposioClient`] from the root config. +/// +/// Composio is **always enabled** — there are no configuration flags +/// gating it. The backend URL and auth token come from the shared +/// core defaults (`config.api_url` / `config.api_key`) via +/// [`crate::openhuman::integrations::build_client`]. The only reason +/// this returns `None` is that the user isn't signed in yet. +pub fn build_composio_client(config: &crate::openhuman::config::Config) -> Option { let inner = crate::openhuman::integrations::build_client(config)?; Some(ComposioClient::new(inner)) } diff --git a/src/openhuman/composio/ops.rs b/src/openhuman/composio/ops.rs index 3d2a65411..75349a93a 100644 --- a/src/openhuman/composio/ops.rs +++ b/src/openhuman/composio/ops.rs @@ -25,12 +25,17 @@ use super::types::{ ComposioExecuteResponse, ComposioToolkitsResponse, ComposioToolsResponse, }; -/// Resolve a [`ComposioClient`] from `config.integrations`, or return an +/// Resolve a [`ComposioClient`] from the root config, or return an /// error string that the caller can surface over RPC. +/// +/// Composio is always enabled — it is proxied through our backend and +/// has no client-side toggle or API key. The only reason this fails is +/// that no app-session JWT has been stored yet (i.e. the user hasn't +/// completed sign-in / `auth_store_session`). fn resolve_client(config: &Config) -> OpResult { - build_composio_client(&config.integrations).ok_or_else(|| { - "composio is disabled (integrations.enabled or integrations.composio.enabled is off, \ - or backend_url/auth_token missing)" + build_composio_client(config).ok_or_else(|| { + "composio unavailable: no backend session token. Sign in first \ + (auth_store_session)." .to_string() }) } @@ -45,7 +50,7 @@ pub async fn composio_list_toolkits( let resp = client .list_toolkits() .await - .map_err(|e| format!("[composio] list_toolkits failed: {e}"))?; + .map_err(|e| format!("[composio] list_toolkits failed: {e:#}"))?; let count = resp.toolkits.len(); Ok(RpcOutcome::new( resp, @@ -63,7 +68,7 @@ pub async fn composio_list_connections( let resp = client .list_connections() .await - .map_err(|e| format!("[composio] list_connections failed: {e}"))?; + .map_err(|e| format!("[composio] list_connections failed: {e:#}"))?; let active = resp .connections .iter() @@ -87,7 +92,7 @@ pub async fn composio_authorize( let resp = client .authorize(toolkit) .await - .map_err(|e| format!("[composio] authorize failed: {e}"))?; + .map_err(|e| format!("[composio] authorize failed: {e:#}"))?; // Publish an event so any interested subscribers (e.g. UI refreshers, // analytics) can react to the new connection handoff. @@ -114,7 +119,7 @@ pub async fn composio_delete_connection( let resp = client .delete_connection(connection_id) .await - .map_err(|e| format!("[composio] delete_connection failed: {e}"))?; + .map_err(|e| format!("[composio] delete_connection failed: {e:#}"))?; Ok(RpcOutcome::new( resp, vec![format!("composio: connection {connection_id} deleted")], @@ -132,7 +137,7 @@ pub async fn composio_list_tools( let resp = client .list_tools(toolkits.as_deref()) .await - .map_err(|e| format!("[composio] list_tools failed: {e}"))?; + .map_err(|e| format!("[composio] list_tools failed: {e:#}"))?; let count = resp.tools.len(); Ok(RpcOutcome::new( resp, @@ -179,7 +184,7 @@ pub async fn composio_execute( elapsed_ms, }, ); - Err(format!("[composio] execute failed: {e}")) + Err(format!("[composio] execute failed: {e:#}")) } } } diff --git a/src/openhuman/composio/tools.rs b/src/openhuman/composio/tools.rs index f923d0de2..660c7c357 100644 --- a/src/openhuman/composio/tools.rs +++ b/src/openhuman/composio/tools.rs @@ -24,7 +24,7 @@ use async_trait::async_trait; use serde_json::{json, Value}; -use crate::openhuman::tools::traits::{PermissionLevel, Tool, ToolResult}; +use crate::openhuman::tools::traits::{PermissionLevel, Tool, ToolCategory, ToolResult}; use super::client::ComposioClient; @@ -56,6 +56,12 @@ impl Tool for ComposioListToolkitsTool { fn permission_level(&self) -> PermissionLevel { PermissionLevel::ReadOnly } + fn category(&self) -> ToolCategory { + // Composio proxies to external SaaS (Gmail, Notion, …), so it + // lives in the Skill category alongside QuickJS skill tools and + // is picked up by sub-agents with `category_filter = "skill"`. + ToolCategory::Skill + } async fn execute(&self, _args: Value) -> anyhow::Result { tracing::debug!("[composio] tool list_toolkits.execute"); match self.client.list_toolkits().await { @@ -96,6 +102,9 @@ impl Tool for ComposioListConnectionsTool { fn permission_level(&self) -> PermissionLevel { PermissionLevel::ReadOnly } + fn category(&self) -> ToolCategory { + ToolCategory::Skill + } async fn execute(&self, _args: Value) -> anyhow::Result { tracing::debug!("[composio] tool list_connections.execute"); match self.client.list_connections().await { @@ -147,6 +156,9 @@ impl Tool for ComposioAuthorizeTool { fn permission_level(&self) -> PermissionLevel { PermissionLevel::Write } + fn category(&self) -> ToolCategory { + ToolCategory::Skill + } async fn execute(&self, args: Value) -> anyhow::Result { let toolkit = args .get("toolkit") @@ -218,6 +230,9 @@ impl Tool for ComposioListToolsTool { fn permission_level(&self) -> PermissionLevel { PermissionLevel::ReadOnly } + fn category(&self) -> ToolCategory { + ToolCategory::Skill + } async fn execute(&self, args: Value) -> anyhow::Result { let toolkits = args.get("toolkits").and_then(|v| v.as_array()).map(|arr| { arr.iter() @@ -281,6 +296,9 @@ impl Tool for ComposioExecuteTool { // as write-level to respect channel permission caps. PermissionLevel::Write } + fn category(&self) -> ToolCategory { + ToolCategory::Skill + } async fn execute(&self, args: Value) -> anyhow::Result { let tool = args .get("tool") @@ -334,9 +352,7 @@ impl Tool for ComposioExecuteTool { /// Build the full set of composio agent tools when the integrations /// client is available and composio is enabled. Returns an empty vec /// otherwise so callers can always `.extend(...)` unconditionally. -pub fn all_composio_agent_tools( - config: &crate::openhuman::config::IntegrationsConfig, -) -> Vec> { +pub fn all_composio_agent_tools(config: &crate::openhuman::config::Config) -> Vec> { let Some(client) = super::client::build_composio_client(config) else { tracing::debug!("[composio] agent tools not registered — disabled or missing credentials"); return Vec::new(); @@ -353,3 +369,57 @@ pub fn all_composio_agent_tools( tracing::debug!(count = tools.len(), "[composio] agent tools registered"); tools } + +// ── Tests ─────────────────────────────────────────────────────────── + +#[cfg(test)] +mod tests { + use super::*; + use crate::openhuman::integrations::IntegrationClient; + use std::sync::Arc; + + /// Build a `ComposioClient` wired to a dummy backend. No network calls + /// are made in these tests — we only exercise the `Tool` trait's + /// metadata methods (`name`, `category`, `permission_level`, …), which + /// are pure accessors that don't touch the HTTP client. + fn fake_composio_client() -> ComposioClient { + let inner = + IntegrationClient::new("http://127.0.0.1:0".to_string(), "test-token".to_string()); + ComposioClient::new(Arc::new(inner)) + } + + /// Every composio tool must report `ToolCategory::Skill` so the + /// skills sub-agent (`category_filter = "skill"`) picks them up. + /// + /// If someone removes the override on any tool, this test flips to + /// `System` (the default from the `Tool` trait) and fails loudly. + #[test] + fn all_composio_tools_are_in_skill_category() { + let client = fake_composio_client(); + let tools: Vec> = vec![ + Box::new(ComposioListToolkitsTool::new(client.clone())), + Box::new(ComposioListConnectionsTool::new(client.clone())), + Box::new(ComposioAuthorizeTool::new(client.clone())), + Box::new(ComposioListToolsTool::new(client.clone())), + Box::new(ComposioExecuteTool::new(client)), + ]; + + for t in &tools { + assert_eq!( + t.category(), + ToolCategory::Skill, + "composio tool `{}` should be in Skill category so the \ + skills sub-agent picks it up via category_filter", + t.name() + ); + } + + // Sanity-check the expected names are all present. + let names: Vec<&str> = tools.iter().map(|t| t.name()).collect(); + assert!(names.contains(&"composio_list_toolkits")); + assert!(names.contains(&"composio_list_connections")); + assert!(names.contains(&"composio_authorize")); + assert!(names.contains(&"composio_list_tools")); + assert!(names.contains(&"composio_execute")); + } +} diff --git a/src/openhuman/config/schema/tools.rs b/src/openhuman/config/schema/tools.rs index 39ad525dc..34139c94c 100644 --- a/src/openhuman/config/schema/tools.rs +++ b/src/openhuman/config/schema/tools.rs @@ -256,24 +256,18 @@ impl Default for IntegrationToggle { /// Agent integration tools that proxy through the backend API. /// -/// When enabled, the agent gains access to tools like web search (Parallel), -/// location search (Google Places), and phone calls (Twilio). The backend -/// handles external API calls, billing, and rate limiting; the client only -/// forwards requests and displays results. +/// The backend URL and auth token are **not** configurable here — +/// they're always resolved from the core `config.api_url` / +/// `config.api_key` (the same values every other part of the app uses). +/// Composio in particular is unconditionally enabled and has no toggle: +/// as long as the user is signed in, composio tools are available. +/// +/// The per-tool `twilio`, `google_places`, and `parallel` flags below +/// are preserved because those integrations incur per-call costs that +/// the user may legitimately want to turn off; composio costs are +/// metered server-side, so there is no client-side toggle for it. #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, Default)] pub struct IntegrationsConfig { - /// Master switch — set to `true` to register integration tools. - #[serde(default)] - pub enabled: bool, - - /// Backend API base URL (e.g. "https://api.openhuman.ai"). - #[serde(default)] - pub backend_url: Option, - - /// JWT Bearer token for authenticating with the backend. - #[serde(default)] - pub auth_token: Option, - /// Twilio phone-call integration. #[serde(default)] pub twilio: IntegrationToggle, @@ -285,11 +279,4 @@ pub struct IntegrationsConfig { /// Parallel web search & content extraction integration. #[serde(default)] pub parallel: IntegrationToggle, - - /// Composio 1000+ app integrations proxied via the backend - /// (`/agent-integrations/composio/*`). When enabled, agents can - /// list toolkits, authorize OAuth connections, list/execute actions, - /// and receive trigger events over the Socket.IO bridge. - #[serde(default)] - pub composio: IntegrationToggle, } diff --git a/src/openhuman/integrations/client.rs b/src/openhuman/integrations/client.rs index a938ad416..3e799f4e7 100644 --- a/src/openhuman/integrations/client.rs +++ b/src/openhuman/integrations/client.rs @@ -1,6 +1,7 @@ //! Shared HTTP client for all integration tools. use super::types::{BackendResponse, IntegrationPricing}; +use std::error::Error as _; use std::sync::Arc; use std::time::Duration; @@ -15,9 +16,19 @@ pub struct IntegrationClient { impl IntegrationClient { pub fn new(backend_url: String, auth_token: String) -> Self { + // Match the TLS config used by `BackendOAuthClient` in + // `src/api/rest.rs`: force rustls + HTTP/1.1 so we get the same + // consistent cross-platform behaviour every other backend-proxied + // domain (billing, team, webhooks, referral, …) already relies + // on. The default builder picks up native-tls on macOS, which + // has historically failed on staging TLS handshakes while + // rustls succeeds — so the integrations client was the odd one + // out with raw "error sending request" failures. let http_client = reqwest::Client::builder() + .use_rustls_tls() + .http1_only() .timeout(Duration::from_secs(60)) - .connect_timeout(Duration::from_secs(10)) + .connect_timeout(Duration::from_secs(15)) .build() .expect("failed to build integration HTTP client"); @@ -45,7 +56,22 @@ impl IntegrationClient { .header("Content-Type", "application/json") .json(body) .send() - .await?; + .await + .map_err(|e| { + // Log the full error source chain so the caller gets + // something useful instead of reqwest's top-level + // "error sending request for url (…)" which hides the + // real cause (DNS / TLS / connect / timeout). + let mut chain = format!("{e}"); + let mut src: Option<&(dyn std::error::Error + 'static)> = e.source(); + while let Some(s) = src { + chain.push_str(" → "); + chain.push_str(&s.to_string()); + src = s.source(); + } + tracing::warn!("[integrations] POST {} failed: {}", url, chain); + anyhow::anyhow!("POST {} failed: {}", url, chain) + })?; let status = resp.status(); if !status.is_success() { @@ -80,7 +106,18 @@ impl IntegrationClient { .get(&url) .header("Authorization", format!("Bearer {}", self.auth_token)) .send() - .await?; + .await + .map_err(|e| { + let mut chain = format!("{e}"); + let mut src: Option<&(dyn std::error::Error + 'static)> = e.source(); + while let Some(s) = src { + chain.push_str(" → "); + chain.push_str(&s.to_string()); + src = s.source(); + } + tracing::warn!("[integrations] GET {} failed: {}", url, chain); + anyhow::anyhow!("GET {} failed: {}", url, chain) + })?; let status = resp.status(); if !status.is_success() { @@ -128,24 +165,66 @@ impl IntegrationClient { } } -/// Helper: build an `Arc` from config, or `None` if -/// integrations are disabled or misconfigured. -pub fn build_client( - config: &crate::openhuman::config::IntegrationsConfig, -) -> Option> { - if !config.enabled { - return None; - } - match ( - config.backend_url.as_deref().map(str::trim), - config.auth_token.as_deref().map(str::trim), - ) { - (Some(url), Some(token)) if !url.is_empty() && !token.is_empty() => Some(Arc::new( - IntegrationClient::new(url.to_owned(), token.to_owned()), - )), - _ => { +/// Helper: build an `Arc` from the root config, or +/// `None` if the user isn't signed in yet. +/// +/// Both the backend URL and the auth token come from **core defaults**: +/// +/// - backend URL → [`crate::api::config::effective_api_url`] applied to +/// `config.api_url` (which itself falls back to the `BACKEND_URL` / +/// `VITE_BACKEND_URL` env vars and finally the hosted default). +/// - auth token → [`crate::api::jwt::get_session_token`], i.e. the +/// app-session JWT written by `auth_store_session` — the same token +/// that billing, team, webhooks, referral, memory, etc. all use. +/// As a last-ditch fallback we also honour `config.api_key` so +/// non-interactive sidecar deployments that set `OPENHUMAN_API_KEY` +/// still work. +/// +/// There are no per-feature toggles for the shared client itself — +/// callers that need a kill switch (e.g. twilio, google_places, +/// parallel) gate tool registration at their own level. +pub fn build_client(config: &crate::openhuman::config::Config) -> Option> { + let backend_url = crate::api::config::effective_api_url(&config.api_url); + + // Primary: app-session JWT from the auth profile store. + let session_token = match crate::api::jwt::get_session_token(config) { + Ok(Some(tok)) => { + let trimmed = tok.trim().to_string(); + if trimmed.is_empty() { + None + } else { + Some(trimmed) + } + } + Ok(None) => None, + Err(e) => { + tracing::warn!("[integrations] failed to read session token: {e}"); + None + } + }; + + // Fallback: config.api_key for headless / CI deployments. + let auth_token = session_token.or_else(|| { + config + .api_key + .as_deref() + .map(str::trim) + .filter(|s| !s.is_empty()) + .map(str::to_owned) + }); + + match auth_token { + Some(token) => { + tracing::debug!( + backend_url = %backend_url, + "[integrations] client built (session token resolved)" + ); + Some(Arc::new(IntegrationClient::new(backend_url, token))) + } + None => { tracing::warn!( - "[integrations] enabled but backend_url or auth_token missing — skipping" + "[integrations] no auth token available — user is not signed in \ + (no app-session JWT and no config.api_key fallback)" ); None } diff --git a/src/openhuman/integrations/mod.rs b/src/openhuman/integrations/mod.rs index 5634cb971..6078f2a4a 100644 --- a/src/openhuman/integrations/mod.rs +++ b/src/openhuman/integrations/mod.rs @@ -55,30 +55,26 @@ mod tests { } #[test] - fn build_client_returns_none_when_disabled() { - let config = crate::openhuman::config::IntegrationsConfig::default(); + fn build_client_returns_none_when_no_auth_token() { + let mut config = crate::openhuman::config::Config::default(); + config.api_key = None; assert!(build_client(&config).is_none()); } #[test] - fn build_client_returns_none_when_url_missing() { - let config = crate::openhuman::config::IntegrationsConfig { - enabled: true, - backend_url: None, - auth_token: Some("tok".into()), - ..Default::default() - }; - assert!(build_client(&config).is_none()); + fn build_client_uses_core_api_key() { + // No per-integration config exists any more — the client is + // built solely from the core `config.api_key` / `config.api_url`. + let mut config = crate::openhuman::config::Config::default(); + config.api_key = Some("root-token".into()); + config.api_url = Some("https://api.example.test".into()); + assert!(build_client(&config).is_some()); } #[test] - fn build_client_rejects_whitespace_only_values() { - let config = crate::openhuman::config::IntegrationsConfig { - enabled: true, - backend_url: Some(" ".into()), - auth_token: Some("tok".into()), - ..Default::default() - }; + fn build_client_rejects_whitespace_only_api_key() { + let mut config = crate::openhuman::config::Config::default(); + config.api_key = Some(" ".into()); assert!(build_client(&config).is_none()); } } diff --git a/src/openhuman/memory/embeddings.rs b/src/openhuman/memory/embeddings.rs index d8a4426c9..aa6c9e736 100644 --- a/src/openhuman/memory/embeddings.rs +++ b/src/openhuman/memory/embeddings.rs @@ -186,31 +186,76 @@ impl EmbeddingProvider for FastembedEmbedding { let state = Arc::clone(&self.state); let provider = self.model.clone(); - let join_result = tokio::task::spawn_blocking(move || { + let join_result = tokio::task::spawn_blocking(move || -> anyhow::Result>> { ensure_fastembed_ort_dylib_path(); let mut guard = state.lock(); // Lazy initialization of the model on the first request. + // + // `fastembed::TextEmbedding::try_new` reaches into the `ort` + // crate's global environment, which uses a `std::sync::Mutex`. + // If any previous caller panicked while that mutex was held + // (common when the ONNX Runtime dylib path is wrong or a + // background init failed), every subsequent call panics with + // `"Mutex poisoned"`. Without `catch_unwind`, that panic + // propagates out of this `spawn_blocking` closure, kills the + // tokio blocking worker, and surfaces as a process-level + // stack trace — even though the caller only wanted an error. + // + // We trap the panic here, flip our own state to `Failed`, and + // return a regular `anyhow::Error` so every later call short- + // circuits on the cached failure without touching `ort` again. if matches!(*guard, FastembedState::Uninitialized) { - match fastembed::TextEmbedding::try_new( - fastembed::InitOptions::new( - fastembed::EmbeddingModel::from_str(&provider) - .unwrap_or(fastembed::EmbeddingModel::BGESmallENV15), + let provider_for_init = provider.clone(); + let init_result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + fastembed::TextEmbedding::try_new( + fastembed::InitOptions::new( + fastembed::EmbeddingModel::from_str(&provider_for_init) + .unwrap_or(fastembed::EmbeddingModel::BGESmallENV15), + ) + .with_show_download_progress(false), ) - .with_show_download_progress(false), - ) { - Ok(model) => *guard = FastembedState::Ready(model), - Err(err) => { + })); + + match init_result { + Ok(Ok(model)) => *guard = FastembedState::Ready(model), + Ok(Err(err)) => { let message = format!("fastembed init failed for {provider}: {err}"); - *guard = FastembedState::Failed(message.clone()); + tracing::error!(target: "memory.embeddings", "[embeddings] {message}"); + *guard = FastembedState::Failed(message); + } + Err(panic_payload) => { + let panic_msg = extract_panic_message(&panic_payload); + let message = format!( + "fastembed init panicked for {provider}: {panic_msg} — \ + the ONNX Runtime global environment is in a poisoned state. \ + Check ORT_DYLIB_PATH / ORT_LIB_LOCATION and restart the \ + process to retry." + ); + tracing::error!(target: "memory.embeddings", "[embeddings] {message}"); + *guard = FastembedState::Failed(message); } } } match &mut *guard { - FastembedState::Ready(model) => model - .embed(items, None) - .map_err(|e| anyhow::anyhow!("fastembed embed failed: {e}")), + FastembedState::Ready(model) => { + // Also guard the actual embed call — fastembed / ort + // can panic on certain inputs or runtime errors, and + // we want to surface those as regular errors too. + let embed_result = + std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + model.embed(items, None) + })); + match embed_result { + Ok(Ok(vectors)) => Ok(vectors), + Ok(Err(e)) => Err(anyhow::anyhow!("fastembed embed failed: {e}")), + Err(panic_payload) => { + let panic_msg = extract_panic_message(&panic_payload); + Err(anyhow::anyhow!("fastembed embed panicked: {panic_msg}")) + } + } + } FastembedState::Failed(message) => Err(anyhow::anyhow!(message.clone())), FastembedState::Uninitialized => { Err(anyhow::anyhow!("fastembed provider did not initialize")) @@ -223,6 +268,19 @@ impl EmbeddingProvider for FastembedEmbedding { } } +/// Best-effort extraction of a readable message from a `catch_unwind` payload. +/// Panics produced by `panic!("...")` downcast to `&'static str` or `String`; +/// everything else falls back to a generic label. +fn extract_panic_message(panic: &Box) -> String { + if let Some(s) = panic.downcast_ref::<&'static str>() { + (*s).to_string() + } else if let Some(s) = panic.downcast_ref::() { + s.clone() + } else { + "unknown panic payload".to_string() + } +} + // ── OpenAI-compatible embedding provider ───────────────────── /// Embedding provider for OpenAI and compatible APIs (e.g., LocalAI, Ollama). diff --git a/src/openhuman/tools/impl/network/composio.rs b/src/openhuman/tools/impl/network/composio.rs index b05fdb970..844aadf6a 100644 --- a/src/openhuman/tools/impl/network/composio.rs +++ b/src/openhuman/tools/impl/network/composio.rs @@ -8,7 +8,7 @@ use crate::openhuman::security::policy::ToolOperation; use crate::openhuman::security::SecurityPolicy; -use crate::openhuman::tools::traits::{Tool, ToolResult}; +use crate::openhuman::tools::traits::{Tool, ToolCategory, ToolResult}; use anyhow::Context; use async_trait::async_trait; use reqwest::Client; @@ -446,6 +446,13 @@ impl Tool for ComposioTool { }) } + fn category(&self) -> ToolCategory { + // Composio proxies to external SaaS (Gmail, Notion, …) — surface + // it in the Skill category so the skills sub-agent + // (`category_filter = "skill"`) can see and call it. + ToolCategory::Skill + } + async fn execute(&self, args: serde_json::Value) -> anyhow::Result { let action = args .get("action") diff --git a/src/openhuman/tools/ops.rs b/src/openhuman/tools/ops.rs index 6d4a641cb..56a2c7ad3 100644 --- a/src/openhuman/tools/ops.rs +++ b/src/openhuman/tools/ops.rs @@ -202,7 +202,7 @@ pub fn all_tools_with_runtime( } // ── Agent integration tools (backend-proxied) ───────────────── - if let Some(client) = crate::openhuman::integrations::build_client(&root_config.integrations) { + if let Some(client) = crate::openhuman::integrations::build_client(root_config) { tracing::debug!("[integrations] client built successfully"); if root_config.integrations.google_places.enabled { tools.push(Box::new( @@ -239,8 +239,7 @@ pub fn all_tools_with_runtime( // five agent tools (list_toolkits, list_connections, authorize, // list_tools, execute) when the composio toggle is on. See // `src/openhuman/composio/tools.rs` for per-tool details. - let composio_tools = - crate::openhuman::composio::all_composio_agent_tools(&root_config.integrations); + let composio_tools = crate::openhuman::composio::all_composio_agent_tools(root_config); if !composio_tools.is_empty() { tracing::debug!( count = composio_tools.len(),