Fix crypto module loading issue with pnpm and add integration tests#20
Merged
robdimarco-atxp merged 9 commits intomainfrom Aug 26, 2025
Conversation
- Fixed template literal substitution in consumer test script - Added proper global replacement for TEST_DATA_HASH and TEST_HEX_OUTPUT - Fixed ESLint no-require-imports rule violation - Tests now pass locally for npm, pnpm, and yarn 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Remove npm cache usage from all GitHub Actions workflows - Add npm cache clean and fresh install to resolve @rollup/rollup-linux-x64-gnu issue - Apply workaround from npm CLI bug #4828 and Vite discussion - Updated package-manager-integration.yml, test.yml, and release.yaml workflows Fixes: npm/cli#4828 Ref: vitejs/vite#15532 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Restore npm cache usage in all GitHub Actions workflows - Use conditional logic: try npm ci first, fallback to cache clear only on failure - Maintains fast builds (~20s) when cache works properly - Only pays performance penalty (~45s) when rollup optional dependency bug hits - Best of both worlds: speed + reliability 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add Math.random() based UUID fallback for environments where crypto modules can't load synchronously - Maintains RFC 4122 compliance with proper version 4 UUID format - Fixes "randomUUID requires synchronous module loading (CommonJS)" error in CI external tests - Fallback order: Web Crypto API → Node crypto module → Math.random() fallback 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Replace Bash-specific conditional syntax with GitHub Actions step conditions
- Use 'continue-on-error' and 'if: steps.npm-ci.outcome == failure' pattern
- Add platform-specific file removal for Windows (PowerShell) vs Unix (Bash)
- Fixes "Missing '(' after 'if' in if statement" PowerShell error
- Maintains rollup optional dependency workaround functionality
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
- Replace Unix 'mkdir -p' with cross-platform fs.mkdirSync({recursive: true})
- Fixes "A subdirectory or file -p already exists" error on Windows
- Windows Command Prompt doesn't recognize Unix mkdir -p flag
- Uses Node.js built-in fs module for reliable cross-platform directory creation
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the crypto module loading issue that was causing "Cannot find module 'crypto'" errors when using pnpm, and adds comprehensive integration tests to prevent future regressions.
🐛 Problem Resolved
eval('require')pattern failing in certain runtime environments✅ Solution Implemented
1. Enhanced Crypto Module Loading (
packages/atxp-common/src/platform/index.ts)requirebefore falling back toeval('require')globalThis.cryptowhen available (Node.js 16+)node:cryptoimports for better explicit resolution2. Comprehensive Integration Test Suite
packages/atxp-common/src/platform/packageManager.integration.test.ts3. Automated CI/CD Testing
.github/workflows/package-manager-integration.yml4. Documentation & Developer Experience
npm run test:package-managers,npm run test:integrationdocs/PACKAGE_MANAGER_TESTING.md🧪 Test Results
All package managers now work correctly:
🚀 Benefits
📝 Breaking Changes
None - this is a backward-compatible fix that only improves existing functionality.
🔍 Verification Steps
Before (❌ Broken)
After (✅ Working)
🤖 Generated with Claude Code