feat(connectors): add getConnection({ connectorId }) overload for BYO_SHARED

[cowchimp]

Backend functions can now retrieve the OAuth access token + connection config for a workspace-registered connector (backed by an OrganizationConnector that the app builder consented to) via a new overload on getConnection:

  await base44.asServiceRole.connectors.getConnection({ connectorId: "..." });

Existing string-arg callers are unchanged — they continue to resolve to the platform-SHARED path. The overload dispatches on argument shape (string vs object) and hits a new server route GET /apps/{app_id}/external-auth/tokens/by-connector/{connector_id} for the workspace-connector lookup.

[claude]

Claude finished @cowchimp's task —— View job

---

PR Review: Add getConnection({ connectorId }) overload for BYO_SHARED

Summary

This PR adds support for workspace-registered connectors by introducing a new overload for the getConnection method. The implementation allows backend functions to retrieve OAuth access tokens for connectors using a connector ID rather than just integration type strings.

Code Quality and Best Practices ✅

Strengths:

• Clean implementation of function overloading that maintains backward compatibility
• Clear separation of concerns between the two code paths
• Consistent error handling patterns across both overloads
• Type definitions are well-documented with comprehensive examples
• Follows existing code patterns and conventions in the codebase

Minor suggestions:

• The error message on line 59 in connectors.ts could be more specific for the object case: "Connector ID or integration type is required"

Potential Bugs or Issues ⚠️

1. Type assertion concerns (connectors.ts:53-60):

   • The type narrowing could be more robust. Consider using a type guard function to ensure type safety
   • Edge case: What if someone passes { connectorId: 123 } (number instead of string)?

2. Response type mismatch (connectors.ts:62):

   • The function expects ConnectorAccessTokenResponse but doesn't validate the actual response structure from the new endpoint
   • Both endpoints should ideally return the same response structure

Performance Considerations ✅

• No significant performance concerns identified
• The conditional logic for URL selection is minimal and efficient
• HTTP requests are the main bottleneck, which is unavoidable

Security Concerns 🔒

1. Input validation: The connectorId validation only checks for emptiness and string type. Consider:

   • Adding format validation (e.g., alphanumeric + hyphens only)
   • Maximum length validation to prevent potential DoS

2. Authorization: The code doesn't show how authorization is handled for the new endpoint. Ensure:

   • The service role token has appropriate permissions for the new endpoint
   • Connector access is properly scoped to the requesting app

Test Coverage ✅

Well covered:

• Happy path for both string and object overloads
• Null/undefined connection_config handling
• Error cases for empty strings and invalid types

Missing test cases:

1. Invalid connectorId format (special characters, extremely long strings)
2. Network error handling scenarios
3. Invalid response structure from the new endpoint
4. Edge case: { connectorId: null } or { connectorId: undefined }

Recommendations

1. Add stricter input validation:

   const CONNECTOR_ID_PATTERN = /^[a-zA-Z0-9-_]+$/;
   const MAX_CONNECTOR_ID_LENGTH = 100;
   
   if (!CONNECTOR_ID_PATTERN.test(arg.connectorId) || 
       arg.connectorId.length > MAX_CONNECTOR_ID_LENGTH) {
     throw new Error("Invalid connector ID format");
   }

2. Consider adding a type guard:

   function isConnectorIdArg(arg: unknown): arg is { connectorId: string } {
     return typeof arg === 'object' && 
            arg !== null && 
            'connectorId' in arg &&
            typeof (arg as any).connectorId === 'string';
   }

3. Add integration tests to verify the actual API endpoints work as expected

Overall Assessment

This is a well-implemented feature that extends the SDK's capabilities while maintaining backward compatibility. The code follows established patterns and includes good documentation. With the suggested improvements around input validation and test coverage, this will be a solid addition to the SDK.

---

[github-actions]

🚀 Package Preview Available!

---

Install this PR's preview build with npm:

npm i @base44-preview/[email protected]

Prefer not to change any import paths? Install using npm alias so your code still imports @base44/sdk:

npm i "@base44/sdk@npm:@base44-preview/[email protected]"

Or add it to your package.json dependencies:

{
  "dependencies": {
    "@base44/sdk": "npm:@base44-preview/[email protected]"
  }
}

• 📦 Preview Package: @base44-preview/[email protected]
• 🔗 View this commit on GitHub

---

_{Preview published to npm registry — try new features instantly!}

[claude]

Claude finished @cowchimp's task —— View job

---

PR Review: Add getConnection({ connectorId }) overload for BYO_SHARED

Summary

This PR adds support for workspace-registered connectors by introducing a new overload for the getConnection method. The implementation allows backend functions to retrieve OAuth access tokens for connectors using a connector ID rather than just integration type strings.

Code Quality and Best Practices ✅

Strengths:

• Clean implementation of function overloading that maintains backward compatibility
• Clear separation of concerns between the two code paths
• Consistent error handling patterns across both overloads
• Type definitions are well-documented with comprehensive examples
• Follows existing code patterns and conventions in the codebase

Minor suggestions:

• The error message on line 59 in connectors.ts could be more specific for the object case. Currently shows "Integration type is required and must be a string" which doesn't apply to the connector ID case

Potential Bugs or Issues ⚠️

1. Type narrowing edge cases (connectors.ts:53-60):

   • The type checking could fail with edge cases like { connectorId: 123 } (number instead of string)
   • Consider adding a more robust type guard or validation

2. Response type consistency (connectors.ts:62):

   • The function expects ConnectorAccessTokenResponse from both endpoints
   • Should verify that the new /by-connector/ endpoint returns the same structure

Performance Considerations ✅

• No significant performance concerns identified
• The conditional logic for URL selection is minimal and efficient
• HTTP requests are the main bottleneck, which is unavoidable

Security Concerns 🔒

1. Input validation:

   • The connectorId validation only checks for emptiness and string type
   • Consider adding:
     • Format validation (e.g., alphanumeric + hyphens only)
     • Maximum length validation to prevent potential DoS

2. Authorization:

   • Ensure the service role token has appropriate permissions for the new endpoint
   • Verify connector access is properly scoped to the requesting app

Test Coverage ✅

Well covered:

• Happy path for both string and object overloads
• Null/undefined connection_config handling
• Error cases for empty strings

Missing test cases:

1. Invalid connectorId format (special characters, extremely long strings)
2. Edge case: { connectorId: null } or { connectorId: undefined }
3. Object with invalid structure: { wrongKey: "value" }
4. Network error handling scenarios
5. Invalid response structure from the new endpoint

Recommendations

1. Fix error message (connectors.ts:59):

   throw new Error("Connector ID or integration type is required");

2. Add stricter input validation:

   const CONNECTOR_ID_PATTERN = /^[a-zA-Z0-9-_]+$/;
   const MAX_CONNECTOR_ID_LENGTH = 100;
   
   if (!CONNECTOR_ID_PATTERN.test(arg.connectorId) || 
       arg.connectorId.length > MAX_CONNECTOR_ID_LENGTH) {
     throw new Error("Invalid connector ID format");
   }

3. Consider adding a type guard:

   function isConnectorIdArg(arg: unknown): arg is { connectorId: string } {
     return typeof arg === 'object' && 
            arg !== null && 
            'connectorId' in arg &&
            typeof (arg as any).connectorId === 'string';
   }

Overall Assessment

This is a well-implemented feature that extends the SDK's capabilities while maintaining backward compatibility. The code follows established patterns and includes good documentation. With the suggested improvements around input validation and test coverage, this will be a solid addition to the SDK.

[claude]

Claude finished @cowchimp's task —— View job

---

PR Review: Add getConnection({ connectorId }) overload for BYO_SHARED

Summary

This PR introduces a new overload for the getConnection method that accepts an object with a connectorId property, enabling support for workspace-registered connectors. The implementation maintains backward compatibility while adding a new endpoint route for connector-based lookups.

Code Quality and Best Practices ✅

Strengths:

• Clean implementation of function overloading that maintains backward compatibility
• Clear separation of concerns between the two code paths
• Comprehensive JSDoc documentation with examples
• Type definitions are well-documented
• Follows existing code patterns and conventions

Areas for improvement:

• The error message on line 59 in connectors.ts doesn't match the new overload case - it says "Integration type is required" when it should also mention connector ID
• Type narrowing could be more robust to handle edge cases

Potential Bugs or Issues ⚠️

1. Error message inconsistency (connectors.ts:59):

   • The error message states "getConnection requires either an integration type string or an object with a connectorId string" which is good for the general case
   • However, the specific validation error on line 55 says "Connector ID is required and must be a string" while line 50 says "Integration type is required and must be a string"
   • As @yoavf suggested, the final error should cover both cases

2. Type validation edge cases (connectors.ts:53):

   • The current check typeof arg.connectorId === "string" would pass for { connectorId: 123 } (number) but then fail at line 54
   • Consider adding stricter type validation or using a type guard

3. Missing test coverage:

   • No test for { connectorId: null } or { connectorId: undefined }
   • No test for objects with incorrect structure like { wrongKey: "value" }
   • No test for non-string connectorId values

Performance Considerations ✅

• No significant performance concerns
• The conditional logic for URL selection is minimal and efficient
• HTTP requests are the main bottleneck, which is unavoidable

Security Concerns 🔒

1. Input validation:

   • The connectorId validation only checks for non-empty string
   • Consider adding:
     • Format validation (e.g., alphanumeric + hyphens)
     • Maximum length validation to prevent DoS
     • Pattern validation to ensure valid connector ID format

2. Authorization:

   • The code assumes the service role token has appropriate permissions for the new endpoint
   • Ensure the /by-connector/ endpoint properly validates app access to the requested connector

Test Coverage 🧪

Well covered:

• Happy path for both overloads
• Null/undefined connection_config handling
• Empty string validation

Missing coverage:

• Invalid connectorId formats or types
• Edge cases like { connectorId: null }, { connectorId: 123 }
• Objects with unexpected structure
• Network error scenarios

Recommendations

1. Update error message as suggested by @yoavf:

   // Line 59 in connectors.ts
   throw new Error(
     "getConnection requires either an integration type string or an object with a connectorId string"
   );

2. Add input validation:

   // Add after line 54
   if (!/^[a-zA-Z0-9-_]+$/.test(arg.connectorId) || arg.connectorId.length > 100) {
     throw new Error("Invalid connector ID format");
   }

3. Add missing test cases:

   test("throws when connectorId is not a string", async () => {
     await expect(
       base44.asServiceRole.connectors.getConnection({ connectorId: 123 as any })
     ).rejects.toThrow("Connector ID is required and must be a string");
   });
   
   test("throws when object has wrong structure", async () => {
     await expect(
       base44.asServiceRole.connectors.getConnection({ wrongKey: "value" } as any)
     ).rejects.toThrow("getConnection requires either an integration type string or an object with a connectorId string");
   });

Overall Assessment

This is a well-implemented feature that extends the SDK's connector capabilities. The code is clean, follows established patterns, and maintains backward compatibility. The main improvements needed are around error messaging (as already noted by @yoavf) and adding some edge case test coverage. With these minor adjustments, this will be a solid addition to the SDK.

[claude]

Claude finished @cowchimp's task —— View job

---

PR Review: Add getWorkspaceConnection for BYO_SHARED connectors

Summary

This PR adds support for workspace-registered connectors by introducing a new getWorkspaceConnection method. Originally implemented as an overload for getConnection({ connectorId }), it was refactored to a separate method based on reviewer feedback. The implementation allows backend functions to retrieve OAuth access tokens for workspace-registered connectors.

Code Quality and Best Practices ✅

Strengths:

• Clean implementation with clear separation between platform and workspace connector methods
• Consistent error handling patterns across all connector methods
• Comprehensive JSDoc documentation with clear examples
• Good type safety with proper interfaces
• Follows existing code patterns and conventions
• Previous error message issue was addressed in commit 7d3563f

Areas for improvement:

• Consider adding a common validation utility for connector/integration IDs to reduce code duplication

Potential Bugs or Issues ✅

The implementation looks solid. Previous reviews raised concerns about:

1. Error message clarity - This was fixed in commit 7d3563f
2. Type narrowing for overloads - No longer relevant after refactoring to separate method
3. Response type consistency - Both endpoints use the same ConnectorAccessTokenResponse type

No new bugs or issues identified in the current implementation.

Performance Considerations ✅

• No performance concerns identified
• Direct API calls without unnecessary overhead
• HTTP requests are the natural bottleneck

Security Concerns 🔒

Input validation:

• Current validation checks for non-empty strings ✅
• Consider adding additional validation:

  // Prevent potential injection or DoS
  const CONNECTOR_ID_MAX_LENGTH = 100;
  const CONNECTOR_ID_PATTERN = /^[a-zA-Z0-9-_]+$/;
  
  if (connectorId.length > CONNECTOR_ID_MAX_LENGTH) {
    throw new Error("Connector ID too long");
  }
  if (!CONNECTOR_ID_PATTERN.test(connectorId)) {
    throw new Error("Invalid connector ID format");
  }

Authorization:

• The service role token should have appropriate permissions for the new endpoint
• Ensure the backend validates that the app has access to the requested connector

Test Coverage ✅

Well covered:

• Happy path for workspace connector retrieval
• Null/undefined connection_config handling
• Empty string validation
• Null parameter validation
• Consistent test structure with other methods

Consider adding:

1. Edge cases for invalid connector ID formats (special characters, very long strings)
2. Test with numeric connector ID: { connectorId: 123 as any }
3. Network error scenarios (though this might be covered elsewhere)

Architecture Decision

The refactor from overload to separate method is a good architectural choice:

• ✅ Clearer API surface - no ambiguity about parameter types
• ✅ Better TypeScript support - no need for complex type guards
• ✅ Easier to document and understand
• ✅ Follows single responsibility principle

Overall Assessment

This is a well-implemented feature that extends the SDK's connector capabilities. The refactor to a separate getWorkspaceConnection method improves API clarity while maintaining all the functionality. The code is clean, well-tested, and follows established patterns. With minor enhancements to input validation, this is ready for production use.

Todo List:

• Read the changed files to understand the implementation
• Review previous feedback to check what was addressed
• Analyze code quality and best practices
• Check for potential bugs or issues
• Evaluate performance considerations
• Assess security concerns
• Review test coverage
• Provide comprehensive feedback

[claude]

Claude finished @cowchimp's task —— View job

---

PR Review: Add getWorkspaceConnection for BYO_SHARED connectors

Summary

This PR adds support for workspace-registered connectors by introducing a new getWorkspaceConnection method. Originally implemented as an overload, it was refactored to a separate method based on reviewer feedback. The implementation allows backend functions to retrieve OAuth access tokens for workspace-registered connectors using a connector ID.

Code Quality and Best Practices ✅

Strengths:

• Clean implementation with clear separation between platform and workspace connector methods
• Consistent error handling patterns across all connector methods
• Comprehensive JSDoc documentation with clear examples
• Good type safety with proper interfaces
• Follows existing code patterns and conventions
• Previous error message issue was addressed in commit 7d3563f

Areas for improvement:

• Consider extracting common validation logic to reduce code duplication across methods

Potential Bugs or Issues ✅

The implementation appears solid. Previous concerns have been addressed:

1. Error message clarity - Fixed in commit 7d3563f
2. Type narrowing for overloads - No longer applicable after refactoring to separate method
3. Response type consistency - Both endpoints use the same ConnectorAccessTokenResponse type

No new bugs identified in the current implementation.

Performance Considerations ✅

• No performance concerns identified
• Direct API calls without unnecessary overhead
• HTTP requests are the natural bottleneck

Security Concerns 🔒

Input validation:

• Current validation checks for non-empty strings ✅
• Consider adding additional validation:

  // Prevent potential injection or DoS
  const CONNECTOR_ID_MAX_LENGTH = 100;
  const CONNECTOR_ID_PATTERN = /^[a-zA-Z0-9-_]+$/;
  
  if (connectorId.length > CONNECTOR_ID_MAX_LENGTH) {
    throw new Error("Connector ID too long");
  }
  if (!CONNECTOR_ID_PATTERN.test(connectorId)) {
    throw new Error("Invalid connector ID format");
  }

Authorization:

• Ensure the service role token has appropriate permissions for the new endpoint
• Verify the backend validates that the app has access to the requested connector

Test Coverage ✅

Well covered:

• Happy path for workspace connector retrieval
• Null/undefined connection_config handling
• Empty string validation
• Null parameter validation
• Consistent test structure with other methods

Consider adding:

1. Edge cases for invalid connector ID formats (special characters, very long strings)
2. Test with numeric connector ID: await base44.asServiceRole.connectors.getWorkspaceConnection(123 as any)
3. Network error scenarios (though this might be covered elsewhere)

Architecture Decision

The refactor from overload to separate method is a good architectural choice:

• ✅ Clearer API surface - no ambiguity about parameter types
• ✅ Better TypeScript support - no need for complex type guards
• ✅ Easier to document and understand
• ✅ Follows single responsibility principle

Overall Assessment

This is a well-implemented feature that extends the SDK's connector capabilities. The refactor to a separate getWorkspaceConnection method improves API clarity while maintaining all the functionality. The code is clean, well-tested, and follows established patterns. With minor enhancements to input validation, this is ready for production use.

Todo List:

• Read the changed files to understand the implementation
• Review previous feedback to check what was addressed
• Analyze code quality and best practices
• Check for potential bugs or issues
• Evaluate performance considerations
• Assess security concerns
• Review test coverage
• Provide comprehensive feedback