-
Notifications
You must be signed in to change notification settings - Fork 3
fix!: remove unified API integration and migrate to connector-based naming #219
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Delete handlers.stackone-hris.ts containing unified HRIS endpoint mocks - Remove stackoneHrisHandlers import and usage from handlers.ts - Update OAS mock server URL from /unified to base URL The unified API endpoints are no longer needed as integrations should be referenced by their specific integration names rather than the unified category prefix.
…fic names Replace category prefixes (hris_, ats_, crm_) with actual integration provider names (bamboohr_, workday_, salesforce_) across the entire codebase. This change aligns tool naming with StackOne's integration model where tools are scoped to specific provider implementations rather than generic categories. Changes: - Update mock handlers to use bamboohr_*, workday_*, salesforce_* - Rename all test fixtures and assertions - Update examples to reference specific integrations - Change tool.ts schema from 'category' to 'integration' field - Update README documentation with integration-specific examples - Modify tool discovery/search to use integration names The integration-based naming better reflects the actual StackOne API structure where each account links to a specific provider (e.g., BambooHR, Workday) rather than abstract categories. This makes tool filtering and discovery more intuitive for end users.
…nt examples Remove action filtering from single-account examples where it serves no purpose. When using a single account ID, all returned tools are already scoped to that account's integration, making action-based filtering redundant. Changes: - Remove actions: ['bamboohr_*'] filter from basic examples - Add accountId to toolset initialisation in README examples - Simplify fetchTools() calls to no arguments for single-account cases - Keep filtering in multi-account examples where it adds value Filtering remains useful for: - Multi-account scenarios (filter by provider or action type) - Security/permission control (restrict to read-only operations) - Demonstrated in fetch-tools.ts and meta-tools.ts examples
Add realistic multi-account example to demonstrate when tool filtering is meaningful. Single-account usage doesn't need filtering since all returned tools are already scoped to that account's integration. Filtering becomes valuable when: - Using multiple account IDs (e.g., BambooHR + Workday) - Wanting to restrict to specific integration's tools - Implementing permission/security controls The updated example shows: - Single account: simple fetchTools() with no filters - Multiple accounts: returns tools from both integrations - Multiple accounts with filter: restrict to one integration's tools
Add validation in fetchTools() to detect and reject unified API tools which indicate improper account configuration. Extract unified API prefix to a constant for maintainability. Changes: - Add UNIFIED_API_PREFIX constant to consts.ts - Add validateToolName() method to check for unified_ prefix - Throw ToolSetConfigError when unified API tools are detected - Add test case verifying error is thrown for unified tools Unified API tools (unified_*) are legacy and should not be used. Their presence indicates the account lacks proper version configuration. This validation helps users identify configuration issues early.
commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 20 files
Prompt for AI agents (all 1 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="examples/openai-responses-integration.ts">
<violation number="1" location="examples/openai-responses-integration.ts:25">
P1: Mismatch between fetched tools and expected tool call: The filter `*_list_*` only fetches list operations, but the assertion expects `bamboohr_get_employee` (a get operation) which won't be available. Either change the filter to include get operations (e.g., `['*_list_*', '*_get_*']`) or update the assertion to expect a list tool.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| // Fetch tools via MCP | ||
| const tools = await toolset.fetchTools({ | ||
| actions: ['_list_*'], | ||
| actions: ['*_list_*'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: Mismatch between fetched tools and expected tool call: The filter *_list_* only fetches list operations, but the assertion expects bamboohr_get_employee (a get operation) which won't be available. Either change the filter to include get operations (e.g., ['*_list_*', '*_get_*']) or update the assertion to expect a list tool.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At examples/openai-responses-integration.ts, line 25:
<comment>Mismatch between fetched tools and expected tool call: The filter `*_list_*` only fetches list operations, but the assertion expects `bamboohr_get_employee` (a get operation) which won't be available. Either change the filter to include get operations (e.g., `['*_list_*', '*_get_*']`) or update the assertion to expect a list tool.</comment>
<file context>
@@ -20,9 +20,9 @@ const openaiResponsesIntegration = async (): Promise<void> => {
+ // Fetch tools via MCP
const tools = await toolset.fetchTools({
- actions: ['_list_*'],
+ actions: ['*_list_*'],
});
const openAIResponsesTools = tools.toOpenAIResponses();
</file context>
| actions: ['*_list_*'], | |
| actions: ['*_list_*', '*_get_*'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR removes the legacy Unified API integration and migrates the codebase from category-based tool naming (e.g., hris_*, ats_*, crm_*) to integration-specific naming (e.g., bamboohr_*, workday_*, salesforce_*). This aligns the naming convention with StackOne's actual integration model where each account is linked to a specific provider.
Key changes:
- Added validation to reject unified API tools with clear error messaging
- Updated all tool references across tests, mocks, examples, and documentation
- Removed the legacy unified HRIS handler and related mock endpoints
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/consts.ts | Added UNIFIED_API_PREFIX constant to identify legacy unified API tools |
| src/toolsets.ts | Implemented validateToolName method to reject unified API tools during fetch |
| src/toolsets.test.ts | Added comprehensive test coverage for unified API validation; updated glob pattern tests with integration-specific names |
| src/tool.ts | Renamed category variable to integration throughout for clarity and accuracy |
| src/tool.test.ts | Updated all mock tools and test assertions from category-based to integration-specific names |
| src/utils/tfidf-index.test.ts | Updated test fixtures to use integration-specific tool names |
| src/rpc-client.test.ts | Updated RPC action test examples to use integration-specific names |
| mocks/handlers.ts | Removed import and reference to deleted stackoneHrisHandlers |
| mocks/handlers.stackone-hris.ts | Deleted entire file containing legacy unified HRIS endpoint handlers |
| mocks/handlers.stackone-rpc.ts | Updated mock RPC handlers to use integration-specific action names |
| mocks/handlers.stackone-ai.ts | Updated AI handler mocks with integration-specific tool names and adjusted server URL |
| mocks/handlers.openai.ts | Updated OpenAI mock handlers to return integration-specific tool calls |
| examples/index.ts | Simplified quickstart to fetch all account tools; updated tool name references |
| examples/openai-integration.ts | Removed unnecessary filtering; updated account ID placeholder and assertions |
| examples/openai-responses-integration.ts | Updated account ID placeholder, tool name assertions, and filter pattern |
| examples/ai-sdk-integration.ts | Removed unnecessary filtering; updated account ID placeholder |
| examples/fetch-tools.ts | Updated tool name examples to use integration-specific names |
| examples/meta-tools.ts | Updated all account ID placeholders, tool filters, and tool name references to integration-specific naming |
| examples/planning.ts | Updated account ID variable names and values; changed tool filters to integration-specific patterns |
| README.md | Added accountId to examples; updated system prompt references; improved multi-account usage documentation with practical examples |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
glebedel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please double check that the actions in the example actually exist for Bamboohr (not sure if it was just a replace of hris_ or if you took a real action name)
Resolve conflicts: - Remove examples/planning.ts (deleted in main) - Accept upstream changes to examples/README.md - Accept upstream changes to lefthook.yaml - Merge src/toolsets.ts changes
Summary
This PR removes the legacy Unified API integration and migrates all tool naming from category-based prefixes (
hris_*,ats_*,crm_*) to integration-specific names (bamboohr_*,workday_*,salesforce_*).What Changed
mocks/handlers.stackone-hris.tsand all unified endpoint mocksWhy
The unified API approach (
unified_hris_list_employees) is legacy and doesn't align with StackOne's actual integration model where:Integration-based naming (
bamboohr_list_employees) makes:Testing
All 304 tests pass including new validation test for unified API rejection.
Summary by cubic
Removed the legacy Unified API and switched tool/action names from category prefixes (hris_, ats_, crm_) to specific integration names (bamboohr_, workday_, salesforce_) for clarity and alignment with StackOne connectors. Added validation to reject unified_* tools and updated docs, examples, mocks, and tests.
Refactors
Migration
Written for commit c7ca930. Summary will update automatically on new commits.