feat: Add version sync detection between npm package and Docker image

[ghostwriternr]

Implements automatic detection of version mismatches between the npm package and Docker container image to prevent compatibility issues.

Changes

• Embed SANDBOX_VERSION as environment variable in Docker image
• Add /api/version endpoint to container server
• Add getVersion() method to UtilityClient with backward compatibility
• Implement automatic version checking on sandbox startup
• Log warnings when versions don't match
• Add comprehensive test coverage for new functionality

The version check runs asynchronously on sandbox startup and logs warnings for mismatches. Backward compatibility is maintained - old containers without the version endpoint return 'unknown' gracefully.

Fixes #131

🤖 Generated with Claude Code

[changeset-bot]

🦋 Changeset detected

Latest commit: 3d40510

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@repo/sandbox-container	Patch
@cloudflare/sandbox	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[claude]

Pull Request Review

Thank you for implementing version sync detection! This is a valuable feature that will help prevent compatibility issues. Here's my comprehensive review:

✅ Strengths

1. Excellent backward compatibility - The graceful fallback to 'unknown' when the version endpoint doesn't exist ensures old containers continue working
2. Non-blocking approach - Running version check asynchronously in onStart() ensures it doesn't delay sandbox initialization
3. Comprehensive test coverage - Both client and handler tests cover success cases, error cases, and edge cases
4. Clean separation of concerns - Version checking logic is properly isolated in the Sandbox class
5. Docker build integration - Clever use of --build-arg ensures version is automatically embedded

🐛 Potential Issues

1. Critical: Version sync maintenance burden (packages/sandbox/src/version.ts:5)

The hardcoded SDK_VERSION = '0.4.4' will quickly become out of sync. The comment acknowledges this should be auto-generated but hasn't been implemented yet.

Recommendation: Implement automated version sync before merging. Options:

• Use a build script to generate version.ts from package.json
• Import version directly from package.json
• Add a pre-commit hook or CI check to verify versions match

2. Comment mismatch in utility-client.ts:113

Comment says "return null" but code returns 'unknown'. Update comment to match implementation.

3. Missing timestamp in VersionResponse type

The handler returns a timestamp field (misc-handler.ts:66) but the VersionResponse interface doesn't include it (utility-client.ts:23-25). While not causing runtime errors, this creates type inconsistency.

Recommendation: Add timestamp?: string; to VersionResponse interface for completeness

⚡ Performance Considerations

1. Version check timing - The async version check fires immediately on onStart(), which adds an HTTP request during sandbox initialization. This is acceptable since:

   • It's non-blocking (doesn't delay startup)
   • It only happens once per sandbox lifecycle
   • The health of this tradeoff depends on sandbox startup frequency

2. No caching - Each sandbox instance checks version independently. For high-volume applications with many sandbox instances, consider caching the result.

🔒 Security Considerations

✅ No security concerns identified:

• Version endpoint is read-only
• No user input is processed
• Environment variable usage is safe
• No sensitive information exposed

🧪 Test Coverage

Excellent coverage overall:

• ✅ Container handler tests (misc-handler.test.ts)
• ✅ Client tests (utility-client.test.ts)
• ✅ Success, failure, and backward compatibility cases

Missing test: No integration test for the checkVersionCompatibility() method in sandbox.ts. Consider adding tests for:

• Version mismatch warning
• Version match (debug log)
• Unknown version warning
• Error handling when version check fails

📝 Code Quality

1. Good error handling - Proper try-catch blocks and graceful degradation
2. Clear logging - Appropriate log levels (warn for mismatch, debug for errors)
3. Consistent patterns - Follows existing codebase conventions
4. Documentation - JSDoc comments are helpful

Minor Nitpicks

1. Long string concatenation (sandbox.ts:184-187) - Consider using template literals instead of concatenation
2. Redundant variable assignment (sandbox.ts:167) - Could use SDK_VERSION directly

🎯 Recommendations Priority

Must fix before merge:

1. Implement automated version sync (issue Version Packages #1)
2. Fix comment mismatch (issue Version Packages #2)

Should fix:
3. Add timestamp to VersionResponse interface (issue #3)
4. Add integration tests for checkVersionCompatibility()

Nice to have:
5. Use template literals for long strings
6. Remove redundant variable assignment

Summary

This is a solid implementation that solves the stated problem (issue #131). The backward compatibility approach is excellent, and the test coverage is comprehensive. The main concern is the manual version maintenance, which should be automated before merging to prevent the exact type of version mismatch this PR aims to detect!

Overall: Approve with required changes ⚠️

---

Review generated with assistance from Claude Code

[claude]

Pull Request Review: Version Sync Detection

Summary

This PR implements automatic detection of version mismatches between the npm package and Docker container image. The implementation is well-structured and includes comprehensive test coverage. Overall, this is a solid implementation with good backward compatibility considerations.

✅ Strengths

1. Excellent Backward Compatibility

• Gracefully handles old containers without the /api/version endpoint by returning 'unknown'
• Non-blocking implementation - version check runs asynchronously and doesn't prevent sandbox startup
• Comprehensive error handling in utility-client.ts:107-119

2. Strong Test Coverage

• Comprehensive unit tests for both client and server components
• Tests cover edge cases: missing env vars, network failures, 404 responses
• Validation that SDK_VERSION stays in sync with package.json

3. Good Architecture

• Clean separation of concerns: container handler, client method, sandbox integration
• Proper logging at appropriate levels (debug for success, warn for mismatches)
• Version constant properly integrated into the build pipeline

4. Build Integration

• Version propagation through Docker build args is well-implemented
• Changeset automation properly updates the version constant
• Docker scripts correctly pass SANDBOX_VERSION build arg

🔍 Issues & Recommendations

Critical: Version Sync Issue

Location: packages/sandbox/src/version.ts:6

export const SDK_VERSION = '0.4.4';

Issue: The hardcoded version 0.4.4 doesn't match the current package.json version 0.4.5. This defeats the purpose of version checking.

Root Cause: The PR description mentions this PR was created when version was 0.4.4, but the package has since been bumped to 0.4.5 (likely merged after this branch was created).

Fix Required:

export const SDK_VERSION = '0.4.5';

Recommendation: Rebase this branch on main to pick up the latest version, or manually update this file. The changeset automation should handle this going forward, but it needs to be correct before merge.

Minor: Code Quality

1. Missing newline at EOF

Location: packages/sandbox-container/src/handlers/misc-handler.ts:71

The file should end with a newline character per Unix conventions and most style guides.

2. Missing newline at EOF

Location: packages/sandbox-container/src/routes/setup.ts:265

Same issue - file should end with newline.

3. Missing newline at EOF

Location: packages/sandbox/src/clients/utility-client.ts:120

Same issue - file should end with newline.

Enhancement Suggestions

1. Add Type Safety to Version Response

Location: packages/sandbox-container/src/handlers/misc-handler.ts:60-69

Currently the response is an inline object. Consider importing the VersionResponse type from shared types:

import type { VersionResponse } from '@repo/shared';

private async handleVersion(request: Request, context: RequestContext): Promise<Response> {
  const version = process.env.SANDBOX_VERSION || 'unknown';

  const response: VersionResponse = {
    success: true,
    version,
    timestamp: new Date().toISOString(),
  };

  return this.createTypedResponse(response, context);
}

This ensures type consistency between client and server.

2. Consider Adding Version to Health Check

The version information could be useful in the existing /api/health endpoint response for operational monitoring:

interface HealthCheckResult {
  success: boolean;
  status: string;
  timestamp: string;
  version?: string; // Optional to maintain backward compatibility
}

This would allow monitoring systems to track version information without an additional API call.

3. Improve Version Mismatch Warning Message

Location: packages/sandbox/src/sandbox.ts:186-190

The warning for 'unknown' version could be more actionable:

this.logger.warn(
  'Container version check: Container version could not be determined. ' +
  'This indicates an outdated container image (< 0.4.4). ' +
  'Current SDK version is ' + sdkVersion + '. ' +
  'Please rebuild your container: npm run docker:local or docker pull cloudflare/sandbox:' + sdkVersion
);

🔒 Security Review

✅ No security concerns identified

• Environment variable access is read-only
• No user input is processed in the version endpoint
• Version information disclosure is acceptable for debugging purposes
• Proper error handling prevents information leakage

⚡ Performance Considerations

✅ Performance is well-handled

• Version check is asynchronous and doesn't block startup (sandbox.ts:166-169)
• Only called once per sandbox initialization
• Minimal overhead from a single HTTP GET request
• Failed version checks don't impact functionality

📋 Testing

✅ Excellent test coverage

• Unit tests for MiscHandler.handleVersion() with multiple scenarios
• Tests for UtilityClient.getVersion() including error cases
• Version sync validation test ensures SDK_VERSION matches package.json
• Tests verify backward compatibility with old containers

Missing Test Coverage:

• E2E test verifying the version check actually runs on sandbox startup
• Test for the changeset automation script updating version.ts

📝 Documentation

Suggestions:

1. Add JSDoc to SDK_VERSION constant explaining it's auto-updated
2. Consider adding a note in the main README about version compatibility
3. Document the version check behavior for users

🎯 Action Items Before Merge

Must Fix:

1. ✋ Update packages/sandbox/src/version.ts from 0.4.4 to 0.4.5
2. ✋ Add newlines at end of files (or configure Biome to auto-fix)

Should Consider:

3. Add type import for VersionResponse in misc-handler.ts
4. Enhance warning messages with specific actionable steps

Nice to Have:

5. Add E2E test for version checking on startup
6. Add version to health check endpoint response
7. Add documentation about version checking feature

🎉 Conclusion

This is a high-quality PR that solves an important problem (version mismatches) with good design choices. The implementation is clean, well-tested, and properly considers backward compatibility.

The critical issue is the version sync (0.4.4 vs 0.4.5) which must be fixed before merge. The other items are minor code quality improvements that would be nice to have but aren't blockers.

Recommendation: ✅ Approve after fixing the version sync issue

Great work on this feature! 🚀

---

Review performed by Claude Code

[claude]

PR Review: Version Sync Detection

Summary

This PR implements automatic detection of version mismatches between the npm package and Docker container image. The implementation is well-structured with good backward compatibility and comprehensive test coverage.

✅ Strengths

Code Quality

• Clean separation of concerns: Version endpoint in container, client method, and sandbox startup check are well organized
• Comprehensive test coverage: Both utility-client.test.ts and misc-handler.test.ts have excellent coverage including edge cases
• Good error handling: Graceful degradation when version endpoint doesn't exist (backward compatibility)
• Clear documentation: Comments explain the auto-update mechanism and manual edit warnings

Best Practices

• Backward compatibility: Returns 'unknown' for old containers without the version endpoint, preventing breaking changes
• Async version check: Runs in background on startup without blocking initialization
• Proper TypeScript types: VersionResponse interface properly exported and used
• Build automation: .github/changeset-version.ts updated to auto-sync SDK_VERSION on releases

🔍 Issues Found

Critical: Version Out of Sync

Location: packages/sandbox/src/version.ts:6

The SDK_VERSION is hardcoded as '0.4.4' but package.json shows version '0.4.5'. This defeats the purpose of version checking.

// Current (incorrect)
export const SDK_VERSION = '0.4.4';

// Should be
export const SDK_VERSION = '0.4.5';

Impact: Version mismatch warnings will trigger incorrectly, reducing trust in the feature.

Fix: Update to match package.json or ensure the changeset automation runs before this PR merges.

Issue: Error Handling Could Be More Specific

Location: packages/sandbox/src/sandbox.ts:167-169

The catch block converts all errors to Error type, losing specific error information:

.catch(error => {
  this.logger.error('Version compatibility check failed', error instanceof Error ? error : new Error(String(error)));
});

Suggestion: Log the original error for better debugging:

.catch(error => {
  this.logger.error('Version compatibility check failed', error instanceof Error ? error : { error: String(error) });
});

Minor: Test Environment Pollution

Location: packages/sandbox-container/tests/handlers/misc-handler.test.ts:151-156

Tests modify process.env.SANDBOX_VERSION without cleanup between tests, potentially causing test pollution.

Suggestion: Add proper cleanup:

beforeEach(() => {
  delete process.env.SANDBOX_VERSION;
});

afterEach(() => {
  delete process.env.SANDBOX_VERSION;
});

🎯 Recommendations

1. Consider Adding Version Comparison Logic

The current implementation uses strict string equality (containerVersion !== sdkVersion). This could cause false positives for compatible versions like 0.4.4 vs 0.4.4-beta.

Suggestion: Consider using semver comparison for more intelligent matching, or document that exact version matching is intentional.

2. Add Type Safety for Environment Variable

Location: packages/sandbox-container/src/handlers/misc-handler.ts:60

const version = process.env.SANDBOX_VERSION || 'unknown';

Consider creating a typed getter function to centralize this logic:

function getSandboxVersion(): string {
  return process.env.SANDBOX_VERSION ?? 'unknown';
}

3. Test Coverage Gap

The integration between Dockerfile ARG → ENV → API endpoint isn't tested. Consider adding an E2E test that:

1. Builds a Docker image with a specific version
2. Starts a container
3. Calls /api/version
4. Verifies the version matches

4. Documentation Enhancement

Consider adding:

• A note in the README about version synchronization
• Troubleshooting guide for version mismatch warnings
• Documentation on when to expect version mismatches (during development, using local images, etc.)

🔒 Security

No security concerns identified. The version endpoint appropriately:

• Returns read-only information
• Doesn't expose sensitive data
• Has proper CORS headers (tested)

🚀 Performance

No performance concerns:

• Version check runs asynchronously
• Doesn't block sandbox startup
• Single HTTP request with minimal overhead
• Cached in memory after first check (via SDK_VERSION constant)

📝 Summary

This is a well-implemented feature that addresses issue #131 effectively. The main concern is the version sync issue in version.ts which needs to be fixed before merge. The code quality is high with good test coverage and proper error handling.

Recommendation: Request changes to fix the version sync issue, then approve.

---

Review generated with attention to code quality, test coverage, backward compatibility, and operational concerns.

[claude]

Pull Request Review: Version Sync Detection

Summary

This PR implements automatic version mismatch detection between the npm package and Docker container image. The implementation is well-designed with good backward compatibility and comprehensive test coverage.

Overall Assessment

✅ Approved - This is a solid implementation that addresses a real operational need. The code quality is high, tests are comprehensive, and backward compatibility is maintained.

---

🎯 Strengths

1. Excellent Architecture

• Clean separation of concerns between client (UtilityClient) and server (MiscHandler)
• Proper use of existing patterns (extends BaseHttpClient, follows handler conventions)
• Consistent with codebase style

2. Strong Backward Compatibility

• Gracefully handles old containers without /api/version endpoint
• Returns 'unknown' for old containers instead of failing
• Non-blocking version check runs asynchronously on startup
• Errors don't prevent sandbox initialization

3. Comprehensive Test Coverage

• Container handler tests cover all scenarios (version set, not set, CORS headers)
• Client tests cover success, failure, network errors, and various version formats
• Version sync test ensures SDK_VERSION matches package.json
• Tests verify semver format validation

4. Automated Version Management

• Smart automation in .github/changeset-version.ts to sync versions
• Docker build args properly pass version to container
• Clear documentation about auto-updating

---

🔍 Code Quality Analysis

Docker Implementation ✅

ARG SANDBOX_VERSION=unknown
ENV SANDBOX_VERSION=${SANDBOX_VERSION}

• Good: Provides sensible default (unknown)
• Good: Clear variable naming
• Good: Proper ARG → ENV pattern

Client Implementation ✅

Location: packages/sandbox/src/clients/utility-client.ts:98-120

async getVersion(): Promise<string> {
  try {
    const response = await this.get<VersionResponse>('/api/version');
    this.logSuccess('Version retrieved', response.version);
    return response.version;
  } catch (error) {
    this.logger.debug('Failed to get container version (may be old container)', { error });
    return 'unknown';
  }
}

Strengths:

• Excellent error handling with graceful degradation
• Good use of debug logging (doesn't pollute logs for expected backward compat scenario)
• Clean, simple interface

Server Implementation ✅

Location: packages/sandbox-container/src/handlers/misc-handler.ts:59-69

private async handleVersion(request: Request, context: RequestContext): Promise<Response> {
  const version = process.env.SANDBOX_VERSION || 'unknown';
  const response = {
    success: true,
    version,
    timestamp: new Date().toISOString(),
  };
  return this.createTypedResponse(response, context);
}

Strengths:

• Simple and focused
• Consistent with other handlers
• Proper use of createTypedResponse with CORS headers

Version Check Implementation ✅

Location: packages/sandbox/src/sandbox.ts:162-213

Strengths:

• Non-blocking async execution (doesn't delay startup)
• Comprehensive error handling with proper catch block
• Clear, helpful warning messages for users
• Debug logging for success case
• Proper differentiation between 'unknown' vs version mismatch

---

🐛 Potential Issues

Minor Issues

1. Missing Type Export in Container ⚠️

Location: packages/sandbox-container/src/core/types.ts

The VersionResponse interface is not exported from the container package's types file, but it should be for consistency.

Suggestion:

// In packages/sandbox-container/src/core/types.ts
export interface VersionResponse {
  success: true;
  version: string;
  timestamp: string;
}

2. Type Consistency Consideration ⚠️

Location: packages/sandbox/src/clients/utility-client.ts:23-25

The VersionResponse is defined in the client but not shared. While this works, consider if it should be in @repo/shared since it crosses the client/container boundary (like PingResponse, CommandsResponse).

Current:

export interface VersionResponse extends BaseApiResponse {
  version: string;
}

Consideration: This is fine as-is, but for consistency with other cross-boundary types, it could live in a shared package.

---

🚀 Performance Considerations

✅ Excellent performance characteristics:

1. Non-blocking startup: Version check runs asynchronously and doesn't delay sandbox initialization
2. Minimal overhead: Single GET request on startup, no ongoing polling
3. Efficient caching: Version is checked once at startup, not repeatedly
4. Fast endpoint: /api/version reads from environment variable (O(1))
5. No retry logic needed: Version check is best-effort, failures are logged but don't block

No performance concerns identified.

---

🔒 Security Assessment

✅ No security concerns identified:

1. Read-only operation: Endpoint only reads environment variable
2. No sensitive data: Version numbers are not sensitive
3. Proper CORS: Uses existing CORS middleware consistently
4. No injection risk: Environment variable is set at build time
5. No authentication needed: Version is public information

Security posture is appropriate for this feature.

---

🧪 Test Coverage Analysis

Excellent Coverage ✅

Container Tests (misc-handler.test.ts)

• ✅ Version returned from environment variable
• ✅ Returns 'unknown' when env var not set
• ✅ CORS headers included
• ✅ Proper response format validation

Client Tests (utility-client.test.ts)

• ✅ Successful version retrieval
• ✅ Multiple version formats (1.0.0, 2.5.3-beta, etc.)
• ✅ 404 handling (backward compatibility)
• ✅ Network failure handling
• ✅ 'unknown' response handling

Version Sync Test (version.test.ts)

• ✅ SDK_VERSION matches package.json
• ✅ Semver format validation

Missing Test Scenarios (Optional)

While coverage is comprehensive, you could add:

1. E2E test: Verify actual version mismatch warnings appear in logs
2. Integration test: Test full flow from sandbox startup through version check
3. Edge case: Test behavior with malformed version strings in environment

Note: These are nice-to-haves, not critical gaps.

---

📋 Suggestions for Improvement

1. Add Version to Health Check Response (Optional)

Consider including version in the existing /api/health endpoint:

// In misc-handler.ts
private async handleHealth(request: Request, context: RequestContext): Promise<Response> {
  const response = {
    success: true,
    status: 'healthy',
    version: process.env.SANDBOX_VERSION || 'unknown', // Add this
    timestamp: new Date().toISOString(),
  };
  return this.createTypedResponse(response, context);
}

Benefit: Reduces number of endpoints; version is useful debug info in health checks.

2. Consider Structured Version Comparison (Future Enhancement)

For future versions, consider using a semver library for smarter version comparison:

// Future enhancement
import semver from 'semver';

if (semver.major(containerVersion) !== semver.major(sdkVersion)) {
  this.logger.error('Major version mismatch!'); // Breaking change
} else if (semver.minor(containerVersion) < semver.minor(sdkVersion)) {
  this.logger.warn('Minor version behind'); // Missing features
}

Benefit: Could provide more nuanced warnings (major vs minor mismatches).

3. Export Type from Container (Minor Fix)

Export VersionResponse from container types for consistency:

// packages/sandbox-container/src/core/types.ts
export interface VersionResponse {
  success: true;
  version: string;
  timestamp: string;
}

---

🎓 Best Practices Demonstrated

This PR demonstrates excellent software engineering practices:

1. ✅ Backward compatibility: Handles old containers gracefully
2. ✅ Fail-safe design: Errors don't break functionality
3. ✅ Comprehensive testing: Multiple test files, edge cases covered
4. ✅ Clear documentation: Good comments and changeset description
5. ✅ Consistent patterns: Follows existing codebase conventions
6. ✅ Automation: Version sync automated via changeset script
7. ✅ Separation of concerns: Clean client/server separation
8. ✅ Observability: Appropriate logging at debug/warn levels

---

✅ Final Verdict

APPROVED - This PR is ready to merge.

Checklist

• ✅ Code quality: Excellent
• ✅ Best practices: Followed consistently
• ✅ Potential bugs: None identified
• ✅ Performance: Optimal (non-blocking, efficient)
• ✅ Security: No concerns
• ✅ Test coverage: Comprehensive
• ✅ Documentation: Clear and sufficient
• ✅ Backward compatibility: Excellent

Minor Recommendations (Non-Blocking)

1. Consider exporting VersionResponse from container types for consistency
2. Optional: Add version to health check endpoint
3. Optional: Future enhancement for semver-aware comparison

Great work! This feature will significantly help users diagnose version mismatch issues in production. The implementation is clean, well-tested, and production-ready.

---

Review completed by Claude Code