custom openapi integrations [SDK]

[natansil]

No description provided.

[claude]

Claude finished @natansil's task —— View job

---

Code Review for Custom Integrations Implementation

✅ Code Quality and Architecture

The implementation follows good software engineering practices:

• Well-structured module architecture with clear separation between types, implementation, and tests
• Consistent naming conventions and proper TypeScript typing throughout
• Good documentation with comprehensive JSDoc comments and examples
• Follows existing SDK patterns by integrating seamlessly with the proxy-based integrations system

🔍 Potential Bugs and Edge Cases

1. Type assertion issue in custom-integrations.ts:83:

   return response as unknown as CustomIntegrationCallResponse;

   The double type assertion suggests the axios response type might not align with the expected return type. Consider verifying if the axios interceptor properly transforms the response.

2. Parameter normalization edge case: The normalizeParams function handles both camelCase and snake_case, but if a user provides both (e.g., pathParams AND path_params), the snake_case version takes precedence silently. This could lead to confusion.

3. Missing null checks: While the function validates slug and operationId, it doesn't validate if they are empty strings.

⚡ Performance Considerations

• Efficient parameter normalization: The implementation creates minimal object copies
• No unnecessary async operations: Direct pass-through to axios is efficient
• Proxy pattern: Integrates well with the existing dynamic proxy system without performance overhead

🔒 Security Analysis

1. ✅ Credentials protection: The implementation correctly proxies requests through Base44's backend, never exposing credentials to the frontend
2. ✅ Input validation: Basic validation for required parameters is in place
3. ⚠️ Headers passthrough: The headers parameter allows arbitrary headers to be passed. Consider documenting which headers are allowed/filtered by the backend to prevent header injection attacks
4. ⚠️ No input sanitization: The payload, path_params, and query_params are passed directly without sanitization. Ensure the backend properly validates and sanitizes these inputs

📊 Test Coverage

Unit Tests (258 lines):

• ✅ Comprehensive coverage of happy paths
• ✅ Parameter normalization (camelCase to snake_case)
• ✅ Error handling for 404, 502 errors
• ✅ Validation of required parameters
• ✅ Integration with existing packages
• ⚠️ Missing test for simultaneous camelCase and snake_case parameters
• ⚠️ Missing test for empty string validation

E2E Tests (119 lines):

• ✅ Tests for non-existent integrations and operations
• ✅ Parameter validation
• ✅ Real integration testing (when configured)
• ⚠️ Limited coverage - mostly relies on environment variables for real testing

📝 Recommendations

1. Improve type safety:

   // Instead of double assertion
   const responseData = response.data as CustomIntegrationCallResponse;
   return responseData;

2. Add parameter conflict warning:

   if (params.pathParams && params.path_params) {
     console.warn('Both pathParams and path_params provided. Using path_params.');
   }

3. Validate empty strings:

   if (!slug || slug.trim() === '') {
     throw new Error("Integration slug is required and cannot be empty");
   }

4. Add more unit tests:

   • Test for both camelCase and snake_case parameters provided simultaneously
   • Test for empty string slug/operationId
   • Test for very large payloads

5. Document security considerations:

   • Add comments about which headers are allowed/filtered
   • Document any payload size limits
   • Note any rate limiting considerations

✨ Overall Assessment

This is a well-implemented feature that extends the SDK's capabilities in a clean and maintainable way. The code quality is high, with good documentation and test coverage. The main concerns are minor type safety improvements and additional edge case handling. The security model is sound, leveraging Base44's backend as a proxy to protect credentials.

The implementation successfully achieves its goal of allowing workspace administrators to integrate custom APIs via OpenAPI specs while maintaining the SDK's existing patterns and security model.

[github-actions]

🚀 Package Preview Available!

---

Install this PR's preview build with npm:

npm i @base44-preview/sdk@0.8.10-pr.62.2685d84

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/sdk@0.8.10-pr.62.2685d84"

Or add it to your package.json dependencies:

{
  "dependencies": {
    "@base44/sdk": "npm:@base44-preview/sdk@0.8.10-pr.62.2685d84"
  }
}

• 📦 Preview Package: @base44-preview/sdk@0.8.10-pr.62.2685d84
• 🔗 View this commit on GitHub

---

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

[netanelgilad]

Why is this needed?

[natansil]

I think it's defensive code. if AI decides to use pathParams and/or queryParams in the call, then SDK will translate to path_params and query_params.
Do you think it's redundant?

[netanelgilad]

yeah, seems to me. We don't do it anywhere else, so even just for the sake of consistency

[claude]

Claude finished @natansil's task —— View job

---

📋 Review Tasks

• Read and analyze all changed files
• Evaluate code quality and architecture
• Identify potential bugs and edge cases
• Assess performance implications
• Review security considerations
• Analyze test coverage
• Provide comprehensive feedback

---

Code Review for Custom OpenAPI Integrations

✅ Code Quality and Best Practices

Strengths:

• Clean architecture: Well-structured module that integrates seamlessly with the existing SDK patterns
• Strong typing: Comprehensive TypeScript interfaces with proper JSDoc documentation
• Good separation of concerns: Types separated into dedicated .types.ts file
• Excellent documentation: Clear examples and detailed parameter descriptions

Areas for Improvement:

• The double type assertion at custom-integrations.ts:42 suggests a type mismatch:

  return response as unknown as CustomIntegrationCallResponse;

  This should be cleaned up to avoid the double assertion.

🐛 Potential Bugs and Issues

1. Empty string validation: The validation only checks for falsy values but doesn't handle empty strings:

   if (!slug) {  // This allows empty strings like ""

   Should validate: if (!slug || slug.trim() === '')

2. Type assertion concern: The axios interceptor comment suggests data extraction happens automatically, but the double type assertion indicates the types don't align properly. This could hide runtime errors.

3. Parameter normalization removal: Based on the review discussion, the removal of camelCase to snake_case normalization could break existing code if any consumers were using camelCase parameters. Consider documenting this breaking change clearly.

⚡ Performance Considerations

• Efficient implementation: Direct pass-through to axios without unnecessary transformations
• No performance bottlenecks: The implementation is lightweight and doesn't add overhead
• Good memory usage: No unnecessary object cloning or data retention

🔒 Security Concerns

1. ✅ Credential protection: Properly proxies through Base44 backend, keeping credentials secure

2. ⚠️ Input sanitization: No validation on the content of payload, path_params, query_params, or headers. While the backend should handle this, consider:

   • Documenting that the backend validates/sanitizes these inputs
   • Adding size limits for payloads to prevent DoS attacks
   • Documenting which headers are allowed/filtered by the backend

3. ⚠️ Header injection risk: The headers parameter allows arbitrary headers. Document which headers the backend filters to prevent header injection attacks.

🧪 Test Coverage

Unit Tests (221 lines):

• ✅ Comprehensive happy path testing
• ✅ Error handling (404, 502)
• ✅ Parameter validation
• ✅ Integration with other packages (Core, custom packages)
• ❌ Missing test for empty string validation
• ❌ Missing test for very large payloads
• ❌ Missing test for invalid header injection attempts

E2E Tests (119 lines):

• ✅ Non-existent integration/operation handling
• ✅ Parameter validation
• ✅ Real integration testing framework (with env vars)
• ⚠️ Limited coverage - mostly framework setup, actual tests depend on environment

📝 Recommendations

1. Fix the type assertion:

   // Instead of double assertion, fix the types or add a type guard
   const response = await axios.post<{ data: CustomIntegrationCallResponse }>(
     `/apps/${appId}/integrations/custom/${slug}/${operationId}`,
     params ?? {}
   );
   return response.data;

2. Improve input validation:

   if (!slug?.trim()) {
     throw new Error("Integration slug is required and cannot be empty");
   }
   if (!operationId?.trim()) {
     throw new Error("Operation ID is required and cannot be empty");
   }

3. Add missing tests:

   • Test empty string validation
   • Test payload size limits
   • Test header validation/filtering

4. Document breaking changes: If camelCase support was intentionally removed, this should be clearly documented in migration guides.

5. Consider adding request options: For future extensibility, consider adding timeout, retry, or other axios config options.

✨ Summary

This is a well-implemented feature that successfully extends the SDK's capabilities. The code follows good practices and integrates cleanly with the existing architecture. The main concerns are minor: the type assertion should be cleaned up, input validation could be stricter, and a few edge cases need test coverage. The security model is sound with proper credential protection through the backend proxy.

The removal of camelCase parameter support (as discussed in the review comments) aligns with consistency across the SDK, though it should be documented as a potential breaking change.

[claude]

Claude finished @natansil's task —— View job

---

📋 Review Checklist

• Read and analyze all changed files
• Evaluate code quality and architecture
• Identify potential bugs and edge cases
• Assess performance implications
• Review security considerations
• Analyze test coverage
• Provide comprehensive feedback

---

Code Review for Custom OpenAPI Integrations

✅ Code Quality and Architecture

Strengths:

• Clean modular design: The implementation follows the SDK's established patterns with dedicated module files for implementation and types
• Excellent TypeScript support: Comprehensive type definitions with proper JSDoc documentation
• Good separation of concerns: Types in .types.ts, implementation in .ts
• Clear API design: The custom.call(slug, operationId, params) interface is intuitive and consistent
• Well-documented examples: JSDoc includes practical usage examples

Areas for Improvement:

• The double type assertion in custom-integrations.ts:42 is a code smell:

  return response as unknown as CustomIntegrationCallResponse;

  This suggests the axios interceptor's type transformation isn't properly typed.

🐛 Potential Bugs and Edge Cases

1. Good validation improvements: The latest commit addressed the empty string validation issue I noticed, now properly checking slug?.trim() and operationId?.trim()

2. Type assertion concern: The double assertion as unknown as is a red flag. The comment mentions the axios interceptor extracts response.data, but the types don't align. This could hide runtime type mismatches.

3. No parameter conflicts: The removal of camelCase normalization (as discussed in review comments) eliminates the parameter precedence confusion issue.

⚡ Performance Considerations

• Efficient implementation: Direct pass-through to axios without unnecessary transformations
• No performance bottlenecks: The proxy pattern integration adds minimal overhead
• Memory efficient: No unnecessary object cloning or data retention
• Large payload support: Tests confirm it handles large payloads (1000 items tested)

🔒 Security Analysis

1. ✅ Credential protection: Properly routes through Base44 backend, keeping API credentials secure

2. ✅ Basic input validation: Validates required parameters (slug, operationId)

3. ⚠️ No content sanitization: The implementation passes payload, path_params, query_params, and headers directly without sanitization. While the backend should handle this, consider:

   • Documenting that backend validates/sanitizes all inputs
   • Adding client-side payload size limits to prevent DoS
   • Documenting which headers are filtered by the backend

4. ⚠️ Header injection risk: Arbitrary headers can be passed. Document which headers the backend filters to prevent security issues.

🧪 Test Coverage Analysis

Unit Tests (317 lines):

• ✅ Comprehensive happy path testing
• ✅ Error scenarios (404, 502)
• ✅ Parameter validation (null, undefined, empty strings, whitespace)
• ✅ Large payload handling
• ✅ Header passthrough
• ✅ Integration with other SDK modules
• ✅ Good edge case coverage after latest commit

E2E Tests (119 lines):

• ✅ Framework for real integration testing
• ✅ Error handling for non-existent integrations/operations
• ✅ Parameter validation
• ⚠️ Actual integration tests depend on environment variables

📝 Key Recommendations

1. Fix the type assertion:

   // Instead of the double assertion, properly type the axios response
   const response = await axios.post<{ data: CustomIntegrationCallResponse }>(
     `/apps/${appId}/integrations/custom/${slug}/${operationId}`,
     params ?? {}
   );
   return response.data;

2. Add security documentation: Document in the types file which headers are filtered and any payload size limits enforced by the backend.

3. Consider adding request configuration: For future extensibility, consider supporting timeout and retry options.

✨ Summary

This is a well-implemented feature that successfully extends the SDK's integration capabilities. The code follows established patterns, has good test coverage, and properly handles security through backend proxying. The removal of camelCase normalization simplifies the API and aligns with SDK consistency.

The main technical debt is the type assertion that should be cleaned up. The security model is sound but would benefit from clearer documentation about backend validation and filtering.

Overall, this implementation is production-ready and adds valuable functionality for workspace administrators to integrate custom APIs seamlessly.

[claude]

Claude finished @natansil's task —— View job

---

📋 Review Checklist

• Check for CLAUDE.md repo guidelines
• Read and analyze all changed files
• Evaluate code quality and architecture
• Identify potential bugs and edge cases
• Assess performance implications
• Review security considerations
• Analyze test coverage
• Provide comprehensive feedback

---

Code Review for Custom OpenAPI Integrations

✅ Code Quality and Architecture

Strengths:

• Clean modular design: Well-structured implementation following the SDK's established patterns
• Excellent documentation: Comprehensive JSDoc comments with practical usage examples
• Strong TypeScript support: Proper type definitions and interfaces
• Good separation of concerns: Types in .types.ts, implementation in .ts
• Seamless integration: The custom property integrates cleanly with the existing proxy-based integrations system

Issues Found:

• Type assertion problem in custom-integrations.ts:49:

  return response as unknown as CustomIntegrationCallResponse;

  The double type assertion (as unknown as) is a code smell indicating type misalignment between axios response and expected return type.

🐛 Potential Bugs and Edge Cases

1. Good validation coverage: The latest commit properly validates empty strings and whitespace for both slug and operationId using ?.trim()

2. Parameter normalization removed: Based on review comments, I see the camelCase to snake_case conversion was removed. However, the code at lines 35-40 still shows this conversion:

   const { pathParams, queryParams, ...rest } = params ?? {};
   const body = {
     ...rest,
     ...(pathParams && { path_params: pathParams }),
     ...(queryParams && { query_params: queryParams }),
   };

   This appears to contradict the review discussion about removing normalization.

⚡ Performance Considerations

• Efficient implementation: Direct pass-through to axios without unnecessary overhead
• No performance bottlenecks: The proxy pattern adds minimal overhead
• Large payload support: Tests confirm handling of 1000+ item payloads
• Memory efficient: No unnecessary object cloning or data retention

🔒 Security Analysis

1. ✅ Credential protection: Properly routes through Base44 backend, keeping API credentials secure

2. ✅ Basic input validation: Validates required parameters

3. ⚠️ No content sanitization: The implementation passes payload, path_params, query_params, and headers directly to the backend. Recommendations:

   • Document that the backend is responsible for all input validation/sanitization
   • Consider documenting payload size limits enforced by the backend
   • Specify which headers are filtered by the backend to prevent header injection

4. ⚠️ Arbitrary headers: The headers parameter allows any headers to be passed. Document which headers the backend filters for security.

🧪 Test Coverage Analysis

Unit Tests (356 lines):

• ✅ Comprehensive happy path testing
• ✅ Error scenarios (404, 502)
• ✅ Parameter validation (null, undefined, empty strings, whitespace)
• ✅ Large payload handling (1000 items)
• ✅ Header passthrough
• ✅ Integration with other SDK modules (Core, custom packages)
• ✅ Parameter normalization tests (though this contradicts the review discussion)

E2E Tests (119 lines):

• ✅ Framework for real integration testing
• ✅ Error handling for non-existent integrations/operations
• ✅ Parameter validation
• ⚠️ Actual integration tests depend on environment variables

📝 Key Recommendations

1. Fix the type assertion:

   // Instead of double assertion, properly type the response
   const response = await axios.post<CustomIntegrationCallResponse>(
     `/apps/${appId}/integrations/custom/${slug}/${operationId}`,
     body
   );
   return response.data;

2. Clarify parameter normalization: The code still shows camelCase to snake_case conversion, but the review discussion suggests this should be removed. Either:

   • Remove the conversion code if not needed
   • Keep it and update tests/documentation to reflect this behavior

3. Add security documentation: In the types file, document:

   • Which headers are filtered by the backend
   • Any payload size limits
   • That backend handles all input validation/sanitization

4. Consider future extensibility: Add support for request options like timeout or retry configuration.

✨ Summary

This is a well-implemented feature that extends the SDK with custom OpenAPI integration capabilities. The code follows established patterns, has good test coverage, and maintains security through backend proxying.

The main technical issues are:

1. The type assertion that should be cleaned up
2. Inconsistency between the code (which still does parameter normalization) and the review discussion (which suggests it should be removed)

The security model is sound but would benefit from clearer documentation about backend validation and filtering.