[FEATURE] set up tests to validate imports work for all app #17845
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThis change introduces an automated package validation system. It adds a script to check package integrity and importability, several npm commands to run validations in different modes, a scheduled GitHub Actions workflow to run validations every three days, and a custom GitHub Action to create or update issues with validation results. Documentation and local test scripts are included. Changes
Sequence Diagram(s)sequenceDiagram
participant Workflow as Scheduled Workflow
participant Validator as generate-package-report.js
participant Artifact as GitHub Artifacts
participant IssueAction as create-package-validation-issue Action
participant GitHub as GitHub Issues
Workflow->>Validator: Run validation script (report, verbose)
Validator->>Artifact: Output JSON and text reports
Workflow->>Artifact: Upload reports as artifacts
Workflow->>IssueAction: Run action with report paths and context
IssueAction->>Artifact: Read validation report(s)
IssueAction->>GitHub: Create or update issue if failures found
IssueAction-->>Workflow: Output issue URL, status, failure count
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (1)
components/netlify/actions/get-site/get-site.mjs (1)
19-19: Missing await for async operation.The call to
this.netlify.getSite()appears to be asynchronous but is missing theawaitkeyword. This could lead to the response being a Promise instead of the actual site data.- const response = this.netlify.getSite(this.siteId); + const response = await this.netlify.getSite(this.siteId);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
.github/workflows/publish-packages.yaml(1 hunks)components/netlify/actions/get-site/get-site.mjs(1 hunks)components/netlify/package.json(1 hunks)scripts/validate-packages.js(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
components/netlify/package.json (1)
Learnt from: jcortes
PR: #14935
File: components/sailpoint/package.json:15-18
Timestamp: 2024-12-12T19:23:09.039Z
Learning: When developing Pipedream components, do not add built-in Node.js modules like fs to package.json dependencies, as they are native modules provided by the Node.js runtime.
scripts/validate-packages.js (1)
Learnt from: jcortes
PR: #14935
File: components/sailpoint/package.json:15-18
Timestamp: 2024-12-12T19:23:09.039Z
Learning: When developing Pipedream components, do not add built-in Node.js modules like fs to package.json dependencies, as they are native modules provided by the Node.js runtime.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
🔇 Additional comments (3)
components/netlify/package.json (1)
3-3: Version bump looks good.The version increment from 0.4.1 to 0.4.2 is appropriate for this change.
components/netlify/actions/get-site/get-site.mjs (1)
7-7: Version bump looks good.The version increment from 0.1.0 to 0.1.1 is appropriate.
.github/workflows/publish-packages.yaml (1)
49-54: Well-placed validation step.The validation step is correctly positioned after TypeScript compilation and before npm authentication. Setting
NODE_ENV=productionensures the validation runs in the appropriate context.
jcortes
force-pushed
the
fix-test-imports
branch
3 times, most recently
from
f1ecf13 to
c63b77b
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
scripts/generate-package-report.js (2)
196-203: Consider expanding dependency validation.Currently only validates
@pipedream/platformdependency version format. Based on the retrieved learning, built-in Node.js modules likefsshould not be in dependencies.Consider adding validation to detect built-in Node.js modules in dependencies:
function validateDependencies(packageJson, app) { if (!packageJson.dependencies) return; + // Check for built-in Node.js modules that shouldn't be in dependencies + const builtinModules = ['fs', 'path', 'crypto', 'http', 'https', 'util', 'os', 'stream']; + const invalidDeps = Object.keys(packageJson.dependencies).filter(dep => builtinModules.includes(dep)); + if (invalidDeps.length > 0) { + throw new Error(`Built-in Node.js modules should not be in dependencies: ${invalidDeps.join(', ')}`); + } + const platformDep = packageJson.dependencies['@pipedream/platform']; if (platformDep && !platformDep.match(/^[\^~]?\d+\.\d+\.\d+/)) { throw new Error(`Invalid @pipedream/platform version: ${platformDep}`); } }
222-259: Secure but complex import validation approach.The import validation creates temporary test files and uses dynamic imports. While this approach works, there are some considerations:
- The test file creation and cleanup is handled properly
- Timeouts are set to prevent hanging processes
- File path resolution uses absolute paths correctly
Consider adding error handling for file system operations:
try { fs.writeFileSync(testFile, testContent); execSync(`node ${testFile}`, { stdio: 'pipe', cwd: process.cwd(), timeout: 10000 }); } catch (error) { + // Ensure cleanup happens even if execSync fails + if (fs.existsSync(testFile)) { + try { + fs.unlinkSync(testFile); + } catch (cleanupError) { + console.warn(`Failed to cleanup test file: ${cleanupError.message}`); + } + } throw new Error(`Import test failed: ${error.message}`); } finally { - if (fs.existsSync(testFile)) { - fs.unlinkSync(testFile); - } + // Move cleanup to try block above for better error handling }.github/workflows/daily-package-validation.yaml (1)
13-164: Fix YAML formatting issues.The static analysis tool correctly identifies trailing spaces and missing newline. These should be cleaned up for consistency.
Remove trailing spaces from multiple lines and add newline at end of file:
runs-on: ubuntu-latest - + steps: - name: Checkout code uses: actions/[email protected] - + - name: Setup pnpm uses: pnpm/[email protected] with: version: 9.14.2 - + - name: Setup Node.js uses: actions/[email protected] with: node-version: 18 cache: 'pnpm' - + - name: Install dependencies run: pnpm install -r --no-frozen-lockfile - + - name: Compile TypeScript run: pnpm run build - + - name: Run Package Validation Report id: validation run: | node scripts/generate-package-report.js --report-only --verbose --output=validation-report.json > validation-report.txt 2>&1 echo "validation_exit_code=$?" >> $GITHUB_OUTPUT continue-on-error: true - + - name: Upload Validation Report uses: actions/upload-artifact@v4 with: name: package-validation-report-${{ github.run_number }} path: | validation-report.txt validation-report.json retention-days: 30 - + - name: Create Issue on Failures if: steps.validation.outputs.validation_exit_code != '0' uses: actions/github-script@v7 with: script: | const fs = require('fs'); - + // Read JSON report for structured data let reportData = null; let failedCount = 0; let summaryText = ''; - + try { const jsonReport = fs.readFileSync('validation-report.json', 'utf8'); reportData = JSON.parse(jsonReport); failedCount = reportData.summary.failed; summaryText = ` 📊 **Summary:** - Total Components: ${reportData.summary.total} - ✅ Validated: ${reportData.summary.validated} - ❌ Failed: ${reportData.summary.failed} - ⏭️ Skipped: ${reportData.summary.skipped} - 📈 Publishable: ${reportData.summary.publishable} - 📉 Failure Rate: ${reportData.summary.failureRate}% `; } catch (error) { // Fallback to text report const report = fs.readFileSync('validation-report.txt', 'utf8'); const failedPackages = report.match(/❌.*FAILED:/g) || []; failedCount = failedPackages.length; summaryText = `Failed to parse JSON report. Found ${failedCount} failures in text report.`; } - + if (failedCount > 0) { // Generate failed packages list let failedPackagesList = ''; if (reportData && reportData.failed) { const topFailures = reportData.failed.slice(0, 10); failedPackagesList = topFailures.map(pkg => `- **${pkg.packageName}** (${pkg.app}): ${pkg.failures.map(f => f.check).join(', ')}` ).join('\n'); if (reportData.failed.length > 10) { failedPackagesList += `\n- ... and ${reportData.failed.length - 10} more packages`; } } else { failedPackagesList = 'See full report for details.'; } - + const issueBody = ` # 📦 Daily Package Validation Report - ${new Date().toDateString()} - + ${summaryText} - + ## 🔗 Links - **Workflow Run**: [#${{ github.run_number }}](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}) - **Download Reports**: Check the workflow artifacts for detailed JSON and text reports - + ## ❌ Failed Packages ${failedPackagesList} - + ## Full Report The complete validation report is available as an artifact in the workflow run. - + ## Next Steps 1. Review the failed packages listed above 2. Check the full validation report artifact 3. Fix import/dependency issues in failing packages 4. Consider updating the validation criteria if needed - + --- *This issue was automatically created by the daily package validation workflow.* `; - + // Check if there's already an open issue for today const { data: issues } = await github.rest.issues.listForRepo({ owner: context.repo.owner, repo: context.repo.repo, labels: ['package-validation', 'automated'], state: 'open' }); - + const today = new Date().toDateString(); const existingIssue = issues.find(issue => issue.title.includes(today) && issue.title.includes('Package Validation Report') ); - + if (existingIssue) { // Update existing issue await github.rest.issues.createComment({ owner: context.repo.owner, repo: context.repo.repo, issue_number: existingIssue.number, body: `## Updated Report - Run #${{ github.run_number }} - + ${issueBody}` }); } else { // Create new issue await github.rest.issues.create({ owner: context.repo.owner, repo: context.repo.repo, title: `📦 Package Validation Report - ${today} - ${failedCount} failures`, body: issueBody, labels: ['package-validation', 'automated', 'bug'] }); } } - + - name: Post Success Summary if: steps.validation.outputs.validation_exit_code == '0' run: | echo "🎉 All packages validated successfully!" - echo "Daily validation completed with no issues." + echo "Daily validation completed with no issues." +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
.github/workflows/daily-package-validation.yaml(1 hunks)components/netlify/actions/get-site/get-site.mjs(1 hunks)components/netlify/package.json(1 hunks)package.json(1 hunks)scripts/generate-package-report.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/netlify/package.json
🧰 Additional context used
🧠 Learnings (1)
scripts/generate-package-report.js (1)
Learnt from: jcortes
PR: #14935
File: components/sailpoint/package.json:15-18
Timestamp: 2024-12-12T19:23:09.039Z
Learning: When developing Pipedream components, do not add built-in Node.js modules like fs to package.json dependencies, as they are native modules provided by the Node.js runtime.
🪛 YAMLlint (1.37.1)
.github/workflows/daily-package-validation.yaml
[error] 13-13: trailing spaces
(trailing-spaces)
[error] 17-17: trailing spaces
(trailing-spaces)
[error] 22-22: trailing spaces
(trailing-spaces)
[error] 28-28: trailing spaces
(trailing-spaces)
[error] 31-31: trailing spaces
(trailing-spaces)
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 41-41: trailing spaces
(trailing-spaces)
[error] 50-50: trailing spaces
(trailing-spaces)
[error] 57-57: trailing spaces
(trailing-spaces)
[error] 62-62: trailing spaces
(trailing-spaces)
[error] 83-83: trailing spaces
(trailing-spaces)
[error] 89-89: trailing spaces
(trailing-spaces)
[error] 98-98: trailing spaces
(trailing-spaces)
[error] 101-101: trailing spaces
(trailing-spaces)
[error] 103-103: trailing spaces
(trailing-spaces)
[error] 107-107: trailing spaces
(trailing-spaces)
[error] 110-110: trailing spaces
(trailing-spaces)
[error] 113-113: trailing spaces
(trailing-spaces)
[error] 119-119: trailing spaces
(trailing-spaces)
[error] 123-123: trailing spaces
(trailing-spaces)
[error] 131-131: trailing spaces
(trailing-spaces)
[error] 133-133: trailing spaces
(trailing-spaces)
[error] 134-134: trailing spaces
(trailing-spaces)
[error] 137-137: trailing spaces
(trailing-spaces)
[error] 145-145: trailing spaces
(trailing-spaces)
[error] 159-159: trailing spaces
(trailing-spaces)
[error] 164-164: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
🔇 Additional comments (8)
components/netlify/actions/get-site/get-site.mjs (1)
7-7: LGTM! Clean version bump.The version increment from "0.1.0" to "0.1.1" is a standard minor version update with no functional changes.
package.json (1)
39-42: LGTM! Well-structured validation scripts.The three new npm scripts provide flexible options for package validation:
- Basic validation with
validate:packages- Detailed output with
validate:packages:verbose- Report generation with
validate:packages:reportThe trailing comma addition after
build:docsfollows good maintainability practices.scripts/generate-package-report.js (3)
1-10: LGTM! Clean setup and argument parsing.The script setup uses appropriate Node.js built-ins and handles command line arguments properly. The argument parsing logic is straightforward and supports the expected flags.
42-61: Good filtering logic for package scope.The validation correctly filters for
@pipedream/packages and those withpublishConfig.access, which aligns with the PR objective of validating importable packages.
337-342: Good module export structure.The script properly exports the main function for external use while supporting direct execution. This follows Node.js best practices.
.github/workflows/daily-package-validation.yaml (3)
3-7: LGTM! Appropriate scheduling setup.The daily cron schedule at midnight UTC and manual workflow dispatch option provide good flexibility for both automated and on-demand validation runs.
35-41: Good validation step structure with proper error handling.The validation step correctly:
- Uses
continue-on-error: trueto ensure artifacts are uploaded even on failure- Captures the exit code for later conditional steps
- Redirects both stdout and stderr to the report file
51-159: Robust issue management logic with good fallback handling.The issue creation logic handles multiple scenarios well:
- Parses JSON report with fallback to text parsing
- Updates existing issues rather than creating duplicates
- Includes comprehensive information in issue body
- Proper error handling for JSON parsing failures
jcortes
force-pushed
the
fix-test-imports
branch
from
c63b77b to
07bf05f
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.github/workflows/daily-package-validation.yaml (3)
1-41: Well-structured workflow with room for improvement in script execution.The workflow setup is comprehensive and addresses the PR objectives effectively. However, consider improving the validation step execution:
Consider this improvement to the validation step:
- name: Run Package Validation Report id: validation run: | - node scripts/generate-package-report.js --report-only --verbose --output=validation-report.json > validation-report.txt 2>&1 - echo "validation_exit_code=$?" >> $GITHUB_OUTPUT + set +e + node scripts/generate-package-report.js --report-only --verbose --output=validation-report.json 2>&1 | tee validation-report.txt + validation_exit_code=$? + echo "validation_exit_code=$validation_exit_code" >> $GITHUB_OUTPUT + set -e continue-on-error: trueThis change ensures both stdout and stderr are captured in the text report while preserving the exit code properly.
51-158: Robust issue management logic with potential improvements.The JavaScript code for issue creation and management is comprehensive. Consider these improvements for better error handling and code clarity:
try { const jsonReport = fs.readFileSync('validation-report.json', 'utf8'); reportData = JSON.parse(jsonReport); failedCount = reportData.summary.failed; + + // Validate report structure + if (!reportData.summary || typeof reportData.summary.failed !== 'number') { + throw new Error('Invalid report structure'); + } + summaryText = ` 📊 **Summary:** - Total Components: ${reportData.summary.total} - ✅ Validated: ${reportData.summary.validated} - ❌ Failed: ${reportData.summary.failed} - ⏭️ Skipped: ${reportData.summary.skipped} - 📈 Publishable: ${reportData.summary.publishable} - 📉 Failure Rate: ${reportData.summary.failureRate}% `; } catch (error) { + console.log(`Failed to parse JSON report: ${error.message}`); // Fallback to text report const report = fs.readFileSync('validation-report.txt', 'utf8'); const failedPackages = report.match(/❌.*FAILED:/g) || []; failedCount = failedPackages.length; - summaryText = `Failed to parse JSON report. Found ${failedCount} failures in text report.`; + summaryText = `⚠️ Failed to parse JSON report (${error.message}). Found ${failedCount} failures in text report.`; }
13-13: Fix formatting issues identified by static analysis.The YAML file contains trailing spaces and is missing a newline at the end.
Apply this fix to remove trailing spaces and add the missing newline:
- runs-on: ubuntu-latest - + runs-on: ubuntu-latest + steps: - name: Checkout code uses: actions/[email protected] - + - name: Setup pnpm uses: pnpm/[email protected] with: version: 9.14.2 - + - name: Setup Node.js uses: actions/[email protected] with: node-version: 18 cache: 'pnpm' - + - name: Install dependencies run: pnpm install -r --no-frozen-lockfile - + - name: Compile TypeScript run: pnpm run build - + - name: Run Package Validation Report id: validation run: | node scripts/generate-package-report.js --report-only --verbose --output=validation-report.json > validation-report.txt 2>&1 echo "validation_exit_code=$?" >> $GITHUB_OUTPUT continue-on-error: true - + - name: Upload Validation Report uses: actions/upload-artifact@v4 with: name: package-validation-report-${{ github.run_number }} path: | validation-report.txt validation-report.json retention-days: 30 - + - name: Create Issue on Failures if: steps.validation.outputs.validation_exit_code != '0' uses: actions/github-script@v7 with: script: | const fs = require('fs'); - + // Read JSON report for structured data let reportData = null; let failedCount = 0; let summaryText = ''; - + try { const jsonReport = fs.readFileSync('validation-report.json', 'utf8'); reportData = JSON.parse(jsonReport); failedCount = reportData.summary.failed; summaryText = ` - + 📊 **Summary:** - Total Components: ${reportData.summary.total} - ✅ Validated: ${reportData.summary.validated} - ❌ Failed: ${reportData.summary.failed} - ⏭️ Skipped: ${reportData.summary.skipped} - 📈 Publishable: ${reportData.summary.publishable} - 📉 Failure Rate: ${reportData.summary.failureRate}% - + } catch (error) { // Fallback to text report const report = fs.readFileSync('validation-report.txt', 'utf8'); const failedPackages = report.match(/❌.*FAILED:/g) || []; failedCount = failedPackages.length; summaryText = `Failed to parse JSON report. Found ${failedCount} failures in text report.`; } - + if (failedCount > 0) { // Generate failed packages list let failedPackagesList = ''; if (reportData && reportData.failed) { const topFailures = reportData.failed.slice(0, 10); failedPackagesList = topFailures.map(pkg => - + `- **${pkg.packageName}** (${pkg.app}): ${pkg.failures.map(f => f.check).join(', ')}` ).join('\n'); if (reportData.failed.length > 10) { failedPackagesList += `\n- ... and ${reportData.failed.length - 10} more packages`; } } else { failedPackagesList = 'See full report for details.'; } - + const issueBody = ` # 📦 Daily Package Validation Report - ${new Date().toDateString()} - + ${summaryText} - + ## 🔗 Links - **Workflow Run**: [#${{ github.run_number }}](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}) - **Download Reports**: Check the workflow artifacts for detailed JSON and text reports - + ## ❌ Failed Packages ${failedPackagesList} - + ## Full Report The complete validation report is available as an artifact in the workflow run. - + ## Next Steps 1. Review the failed packages listed above 2. Check the full validation report artifact 3. Fix import/dependency issues in failing packages 4. Consider updating the validation criteria if needed - + --- *This issue was automatically created by the daily package validation workflow.* - + `; - + // Check if there's already an open issue for today const { data: issues } = await github.rest.issues.listForRepo({ owner: context.repo.owner, repo: context.repo.repo, labels: ['package-validation', 'automated'], state: 'open' }); - + const today = new Date().toDateString(); const existingIssue = issues.find(issue => - + issue.title.includes(today) && issue.title.includes('Package Validation Report') ); - + if (existingIssue) { // Update existing issue await github.rest.issues.createComment({ owner: context.repo.owner, repo: context.repo.repo, issue_number: existingIssue.number, body: `## Updated Report - Run #${{ github.run_number }} - + ${issueBody}` }); } else { // Create new issue await github.rest.issues.create({ owner: context.repo.owner, repo: context.repo.repo, title: `📦 Package Validation Report - ${today} - ${failedCount} failures`, body: issueBody, labels: ['package-validation', 'automated', 'bug'] }); } } - + - name: Post Success Summary if: steps.validation.outputs.validation_exit_code == '0' run: | echo "🎉 All packages validated successfully!" - echo "Daily validation completed with no issues." + echo "Daily validation completed with no issues." +Also applies to: 17-17, 22-22, 28-28, 31-31, 34-34, 41-41, 50-50, 57-57, 62-62, 83-83, 89-89, 98-98, 101-101, 103-103, 107-107, 110-110, 113-113, 119-119, 123-123, 131-131, 133-133, 134-134, 137-137, 145-145, 159-159, 164-164
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
.github/workflows/daily-package-validation.yaml(1 hunks)components/netlify/actions/get-site/get-site.mjs(1 hunks)components/netlify/package.json(1 hunks)package.json(1 hunks)scripts/generate-package-report.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- components/netlify/package.json
- scripts/generate-package-report.js
- package.json
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/daily-package-validation.yaml
[error] 13-13: trailing spaces
(trailing-spaces)
[error] 17-17: trailing spaces
(trailing-spaces)
[error] 22-22: trailing spaces
(trailing-spaces)
[error] 28-28: trailing spaces
(trailing-spaces)
[error] 31-31: trailing spaces
(trailing-spaces)
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 41-41: trailing spaces
(trailing-spaces)
[error] 50-50: trailing spaces
(trailing-spaces)
[error] 57-57: trailing spaces
(trailing-spaces)
[error] 62-62: trailing spaces
(trailing-spaces)
[error] 83-83: trailing spaces
(trailing-spaces)
[error] 89-89: trailing spaces
(trailing-spaces)
[error] 98-98: trailing spaces
(trailing-spaces)
[error] 101-101: trailing spaces
(trailing-spaces)
[error] 103-103: trailing spaces
(trailing-spaces)
[error] 107-107: trailing spaces
(trailing-spaces)
[error] 110-110: trailing spaces
(trailing-spaces)
[error] 113-113: trailing spaces
(trailing-spaces)
[error] 119-119: trailing spaces
(trailing-spaces)
[error] 123-123: trailing spaces
(trailing-spaces)
[error] 131-131: trailing spaces
(trailing-spaces)
[error] 133-133: trailing spaces
(trailing-spaces)
[error] 134-134: trailing spaces
(trailing-spaces)
[error] 137-137: trailing spaces
(trailing-spaces)
[error] 145-145: trailing spaces
(trailing-spaces)
[error] 159-159: trailing spaces
(trailing-spaces)
[error] 164-164: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (2)
components/netlify/actions/get-site/get-site.mjs (1)
7-7: LGTM! Clean version bump.The version increment from "0.1.0" to "0.1.1" is appropriate and aligns with the package validation framework being introduced in this PR.
.github/workflows/daily-package-validation.yaml (1)
42-50: Artifact upload configuration looks good.The artifact configuration with 30-day retention and structured naming is appropriate for daily validation reports.
jcortes
force-pushed
the
fix-test-imports
branch
from
07bf05f to
bd0faf1
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
scripts/generate-package-report.js (2)
52-52: Consider making the components directory configurable.The components directory is hard-coded. Consider accepting it as a command-line argument for better flexibility.
Add a new command-line option:
const isDryRun = args.includes('--dry-run'); const showHelp = args.includes('--help') || args.includes('-h'); +const componentsDir = args.find(arg => arg.startsWith('--dir='))?.split('=')[1] || 'components';Then update line 52:
function generatePackageReport() { - const componentsDir = 'components';
319-323: Remove redundant file existence check.This check is already performed in
validateMainFile. Consider removing it to avoid duplication.function validateImport(packageName, app, packageJson) { const mainFile = path.resolve('components', app, packageJson.main); - if (!fs.existsSync(mainFile)) { - throw new Error(`Main file not found: ${packageJson.main}`); - } - // Syntax check.github/workflows/daily-package-validation.yaml (1)
1-164: Remove trailing spaces throughout the file.The static analysis tool detected trailing spaces on multiple lines, which should be removed for consistency.
Remove trailing spaces from lines: 13, 17, 22, 28, 31, 34, 41, 50, 57, 62, 83, 89, 98, 101, 103, 107, 110, 113, 119, 123, 131, 133, 134, 137, 145, 159.
Also add a newline at the end of the file (line 164).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
.github/workflows/daily-package-validation.yaml(1 hunks)package.json(1 hunks)scripts/generate-package-report.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🧰 Additional context used
🧠 Learnings (1)
scripts/generate-package-report.js (1)
Learnt from: jcortes
PR: #14935
File: components/sailpoint/package.json:15-18
Timestamp: 2024-12-12T19:23:09.039Z
Learning: When developing Pipedream components, do not add built-in Node.js modules like fs to package.json dependencies, as they are native modules provided by the Node.js runtime.
🪛 YAMLlint (1.37.1)
.github/workflows/daily-package-validation.yaml
[error] 13-13: trailing spaces
(trailing-spaces)
[error] 17-17: trailing spaces
(trailing-spaces)
[error] 22-22: trailing spaces
(trailing-spaces)
[error] 28-28: trailing spaces
(trailing-spaces)
[error] 31-31: trailing spaces
(trailing-spaces)
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 41-41: trailing spaces
(trailing-spaces)
[error] 50-50: trailing spaces
(trailing-spaces)
[error] 57-57: trailing spaces
(trailing-spaces)
[error] 62-62: trailing spaces
(trailing-spaces)
[error] 83-83: trailing spaces
(trailing-spaces)
[error] 89-89: trailing spaces
(trailing-spaces)
[error] 98-98: trailing spaces
(trailing-spaces)
[error] 101-101: trailing spaces
(trailing-spaces)
[error] 103-103: trailing spaces
(trailing-spaces)
[error] 107-107: trailing spaces
(trailing-spaces)
[error] 110-110: trailing spaces
(trailing-spaces)
[error] 113-113: trailing spaces
(trailing-spaces)
[error] 119-119: trailing spaces
(trailing-spaces)
[error] 123-123: trailing spaces
(trailing-spaces)
[error] 131-131: trailing spaces
(trailing-spaces)
[error] 133-133: trailing spaces
(trailing-spaces)
[error] 134-134: trailing spaces
(trailing-spaces)
[error] 137-137: trailing spaces
(trailing-spaces)
[error] 145-145: trailing spaces
(trailing-spaces)
[error] 159-159: trailing spaces
(trailing-spaces)
[error] 164-164: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (1)
.github/workflows/daily-package-validation.yaml (1)
37-40: Fix exit code capture logic.The current approach won't capture the validation script's exit code correctly because of the redirection and the echo command always succeeding.
- name: Run Package Validation Report id: validation run: | - node scripts/generate-package-report.js --report-only --verbose --output=validation-report.json > validation-report.txt 2>&1 - echo "validation_exit_code=$?" >> $GITHUB_OUTPUT + node scripts/generate-package-report.js --report-only --verbose --output=validation-report.json > validation-report.txt 2>&1 || exit_code=$? + echo "validation_exit_code=${exit_code:-0}" >> $GITHUB_OUTPUT continue-on-error: trueLikely an incorrect or invalid review comment.
jcortes
force-pushed
the
fix-test-imports
branch
from
bd0faf1 to
b098c4a
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/daily-package-validation.yaml (1)
31-33:--no-frozen-lockfileflag undermines deterministic builds.A previous review already pointed this out; consider dropping the flag so CI uses the checked-in lockfile and avoids surprise dependency changes.
🧹 Nitpick comments (4)
.github/workflows/daily-package-validation.yaml (4)
8-10: Remove the temporarypushtrigger before merging.The workflow still listens to pushes on the
fix-test-importsbranch.
Keeping this after the feature branch is deleted will make the daily job run only on that branch (or do nothing), which is easy to overlook.- push: - branches: [ fix-test-imports ] # Temporary: for testing only
37-42: Capture the validation exit code explicitly for readability.
$?currently works, but it’s fragile if another command is inserted between the Node script and theecho.
Assigning it to a variable makes intent clear:- node scripts/generate-package-report.js --report-only --verbose --output=validation-report.json > validation-report.txt 2>&1 - echo "validation_exit_code=$?" >> $GITHUB_OUTPUT + exit_code=0 + node scripts/generate-package-report.js --report-only --verbose --output=validation-report.json > validation-report.txt 2>&1 || exit_code=$? + echo "validation_exit_code=${exit_code}" >> $GITHUB_OUTPUT
127-139: Potential pagination issue when searching for existing issues.
listForReporeturns a maximum of 30 items by default.
If the repo accumulates more than 30 open validation issues, the query may miss the existing issue for “today”, causing duplicate issues.Consider requesting additional pages or using
per_page: 100to be safe.
1-166: Trailing whitespace violations flagged by yamllint.Multiple lines contain trailing spaces, and the file lacks a final newline.
These do not break the workflow but will keep failing style checks.Run
yamlfmt/prettieror a similar tool to clean them up.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
.github/workflows/daily-package-validation.yaml(1 hunks)package.json(1 hunks)scripts/generate-package-report.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- scripts/generate-package-report.js
🧰 Additional context used
🧠 Learnings (1)
.github/workflows/daily-package-validation.yaml (2)
Learnt from: GTFalcao
PR: #13957
File: components/agility_cms/package.json:18-18
Timestamp: 2024-10-08T15:33:38.240Z
Learning: Before suggesting the removal of a dependency, thoroughly verify its usage across the entire codebase, even if not apparent in the diff.
Learnt from: GTFalcao
PR: #13957
File: components/agility_cms/package.json:18-18
Timestamp: 2024-09-16T19:39:11.875Z
Learning: Before suggesting the removal of a dependency, thoroughly verify its usage across the entire codebase, even if not apparent in the diff.
🪛 YAMLlint (1.37.1)
.github/workflows/daily-package-validation.yaml
[error] 15-15: trailing spaces
(trailing-spaces)
[error] 19-19: trailing spaces
(trailing-spaces)
[error] 24-24: trailing spaces
(trailing-spaces)
[error] 30-30: trailing spaces
(trailing-spaces)
[error] 33-33: trailing spaces
(trailing-spaces)
[error] 36-36: trailing spaces
(trailing-spaces)
[error] 43-43: trailing spaces
(trailing-spaces)
[error] 52-52: trailing spaces
(trailing-spaces)
[error] 59-59: trailing spaces
(trailing-spaces)
[error] 64-64: trailing spaces
(trailing-spaces)
[error] 85-85: trailing spaces
(trailing-spaces)
[error] 91-91: trailing spaces
(trailing-spaces)
[error] 100-100: trailing spaces
(trailing-spaces)
[error] 103-103: trailing spaces
(trailing-spaces)
[error] 105-105: trailing spaces
(trailing-spaces)
[error] 109-109: trailing spaces
(trailing-spaces)
[error] 112-112: trailing spaces
(trailing-spaces)
[error] 115-115: trailing spaces
(trailing-spaces)
[error] 121-121: trailing spaces
(trailing-spaces)
[error] 125-125: trailing spaces
(trailing-spaces)
[error] 133-133: trailing spaces
(trailing-spaces)
[error] 135-135: trailing spaces
(trailing-spaces)
[error] 136-136: trailing spaces
(trailing-spaces)
[error] 139-139: trailing spaces
(trailing-spaces)
[error] 147-147: trailing spaces
(trailing-spaces)
[error] 161-161: trailing spaces
(trailing-spaces)
[error] 166-166: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
- GitHub Check: Generate Package Validation Report
jcortes
force-pushed
the
fix-test-imports
branch
from
b098c4a to
0c1d315
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/daily-package-validation.yaml (1)
31-32: Drop--no-frozen-lockfilefor deterministic CI installsWe already flagged this in a prior review: allowing pnpm to ignore the lockfile means tomorrow’s run could test a different dependency graph, defeating the purpose of a reproducible validation job.
🧹 Nitpick comments (2)
.github/workflows/daily-package-validation.yaml (2)
25-29: Consider moving to Node 20 LTSNode 18 enters maintenance soon. Upgrading the CI runtime early avoids last-minute surprises and aligns with the active LTS.
15-16: Remove trailing whitespace to satisfy YAMLlintMultiple lines (15, 19, 24, 30 …) trigger the linter’s
trailing-spacesrule; Line 166 also lacks a final newline. Cleaning these avoids noisy lint failures in future pipelines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
.github/workflows/daily-package-validation.yaml(1 hunks)package.json(1 hunks)scripts/generate-package-report.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- scripts/generate-package-report.js
🧰 Additional context used
🧠 Learnings (1)
.github/workflows/daily-package-validation.yaml (2)
Learnt from: GTFalcao
PR: #13957
File: components/agility_cms/package.json:18-18
Timestamp: 2024-10-08T15:33:38.240Z
Learning: Before suggesting the removal of a dependency, thoroughly verify its usage across the entire codebase, even if not apparent in the diff.
Learnt from: GTFalcao
PR: #13957
File: components/agility_cms/package.json:18-18
Timestamp: 2024-09-16T19:39:11.875Z
Learning: Before suggesting the removal of a dependency, thoroughly verify its usage across the entire codebase, even if not apparent in the diff.
🪛 YAMLlint (1.37.1)
.github/workflows/daily-package-validation.yaml
[error] 15-15: trailing spaces
(trailing-spaces)
[error] 19-19: trailing spaces
(trailing-spaces)
[error] 24-24: trailing spaces
(trailing-spaces)
[error] 30-30: trailing spaces
(trailing-spaces)
[error] 33-33: trailing spaces
(trailing-spaces)
[error] 36-36: trailing spaces
(trailing-spaces)
[error] 43-43: trailing spaces
(trailing-spaces)
[error] 52-52: trailing spaces
(trailing-spaces)
[error] 59-59: trailing spaces
(trailing-spaces)
[error] 64-64: trailing spaces
(trailing-spaces)
[error] 85-85: trailing spaces
(trailing-spaces)
[error] 91-91: trailing spaces
(trailing-spaces)
[error] 100-100: trailing spaces
(trailing-spaces)
[error] 103-103: trailing spaces
(trailing-spaces)
[error] 105-105: trailing spaces
(trailing-spaces)
[error] 109-109: trailing spaces
(trailing-spaces)
[error] 112-112: trailing spaces
(trailing-spaces)
[error] 115-115: trailing spaces
(trailing-spaces)
[error] 121-121: trailing spaces
(trailing-spaces)
[error] 125-125: trailing spaces
(trailing-spaces)
[error] 133-133: trailing spaces
(trailing-spaces)
[error] 135-135: trailing spaces
(trailing-spaces)
[error] 136-136: trailing spaces
(trailing-spaces)
[error] 139-139: trailing spaces
(trailing-spaces)
[error] 147-147: trailing spaces
(trailing-spaces)
[error] 161-161: trailing spaces
(trailing-spaces)
[error] 166-166: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
- GitHub Check: Generate Package Validation Report
🔇 Additional comments (1)
.github/workflows/daily-package-validation.yaml (1)
37-42: Mismatch between script name and report-generating commandThe workflow summary and
package.jsonintroducevalidate:packages:report, yet the job runspnpm validate:packages.
Ifvalidate:packagesdoesn’t emit the JSON schema expected by the downstream Node script, the issue-creation logic will mis-parse or fail. Please verify the intended script name and output contract.- pnpm validate:packages -- --verbose --output=validation-report.json > validation-report.txt 2>&1 + pnpm run validate:packages:report -- --verbose --output=validation-report.json > validation-report.txt 2>&1
jcortes
force-pushed
the
fix-test-imports
branch
2 times, most recently
from
6ef1242 to
b2932ff
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
.github/workflows/daily-package-validation.yaml (2)
31-33: Remove--no-frozen-lockfileto guarantee deterministic installs
This flag bypasses the lock-file, so dependency versions may drift between runs and invalidate comparison results. The same issue was raised previously and is still unresolved.- - name: Install dependencies - run: pnpm install -r --no-frozen-lockfile + - name: Install dependencies + run: pnpm install -r
145-150:labelsmust be a comma-separated string, not an array
octokit.rest.issues.listForRepoandcreateexpect a string like"a,b". Passing an array silently ignores the filter and throws on create. This was already highlighted in an earlier review.- labels: ['package-validation', 'automated'], + labels: 'package-validation,automated', ... - labels: ['package-validation', 'automated', 'bug'] + labels: 'package-validation,automated,bug'Also applies to: 173-176
🧹 Nitpick comments (1)
.github/workflows/daily-package-validation.yaml (1)
1-184: Trailing whitespace + missing EOF newline break YAML-lint
The workflow contains many trailing spaces and lacks a trailing newline (see YAML-lint output). These nits do not fail Actions but do fail linters and clutter diffs.- echo "Daily validation completed with no issues." \ No newline at end of file + echo "Daily validation completed with no issues." +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
.github/workflows/daily-package-validation.yaml(1 hunks)package.json(1 hunks)scripts/generate-package-report.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- scripts/generate-package-report.js
🧰 Additional context used
🧠 Learnings (1)
.github/workflows/daily-package-validation.yaml (2)
Learnt from: GTFalcao
PR: #13957
File: components/agility_cms/package.json:18-18
Timestamp: 2024-10-08T15:33:38.240Z
Learning: Before suggesting the removal of a dependency, thoroughly verify its usage across the entire codebase, even if not apparent in the diff.
Learnt from: GTFalcao
PR: #13957
File: components/agility_cms/package.json:18-18
Timestamp: 2024-09-16T19:39:11.875Z
Learning: Before suggesting the removal of a dependency, thoroughly verify its usage across the entire codebase, even if not apparent in the diff.
🪛 YAMLlint (1.37.1)
.github/workflows/daily-package-validation.yaml
[error] 15-15: trailing spaces
(trailing-spaces)
[error] 19-19: trailing spaces
(trailing-spaces)
[error] 24-24: trailing spaces
(trailing-spaces)
[error] 30-30: trailing spaces
(trailing-spaces)
[error] 33-33: trailing spaces
(trailing-spaces)
[error] 36-36: trailing spaces
(trailing-spaces)
[error] 43-43: trailing spaces
(trailing-spaces)
[error] 52-52: trailing spaces
(trailing-spaces)
[error] 70-70: trailing spaces
(trailing-spaces)
[error] 77-77: trailing spaces
(trailing-spaces)
[error] 82-82: trailing spaces
(trailing-spaces)
[error] 103-103: trailing spaces
(trailing-spaces)
[error] 109-109: trailing spaces
(trailing-spaces)
[error] 118-118: trailing spaces
(trailing-spaces)
[error] 121-121: trailing spaces
(trailing-spaces)
[error] 123-123: trailing spaces
(trailing-spaces)
[error] 127-127: trailing spaces
(trailing-spaces)
[error] 130-130: trailing spaces
(trailing-spaces)
[error] 133-133: trailing spaces
(trailing-spaces)
[error] 139-139: trailing spaces
(trailing-spaces)
[error] 143-143: trailing spaces
(trailing-spaces)
[error] 151-151: trailing spaces
(trailing-spaces)
[error] 153-153: trailing spaces
(trailing-spaces)
[error] 154-154: trailing spaces
(trailing-spaces)
[error] 157-157: trailing spaces
(trailing-spaces)
[error] 165-165: trailing spaces
(trailing-spaces)
[error] 179-179: trailing spaces
(trailing-spaces)
[error] 184-184: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Lint Code Base
- GitHub Check: Generate Package Validation Report
🔇 Additional comments (1)
.github/workflows/daily-package-validation.yaml (1)
8-9: Temporary push trigger left in permanent workflow
push.branches: [ fix-test-imports ]will keep firing on every commit to that branch even after the feature is merged. Confirm this is intentional; otherwise drop the push trigger before merging.
jcortes
force-pushed
the
fix-test-imports
branch
from
b2932ff to
309330d
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (6)
scripts/generate-package-report.js (4)
287-289: Improve version format validation.The current regex pattern could match invalid versions like "abc1.2.3xyz". Consider using a more strict pattern or a semver library.
Apply this fix:
- if (!/^\d+\.\d+\.\d+/.test(packageJson.version)) { + if (!/^\d+\.\d+\.\d+(-[0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*)?(\+[0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*)?$/.test(packageJson.version)) { throw new Error(`Invalid version format: ${packageJson.version}`); }
313-315: Apply consistent version validation.The version validation pattern here should match the improved pattern suggested for package version validation.
Apply this fix:
- if (platformDep && !platformDep.match(/^[\^~]?\d+\.\d+\.\d+/)) { + if (platformDep && !platformDep.match(/^[\^~]?\d+\.\d+\.\d+(-[0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*)?(\+[0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*)?$/)) { throw new Error(`Invalid @pipedream/platform version: ${platformDep}`); }
336-336: Use a more unique temporary file name to avoid conflicts.The current filename could cause conflicts if multiple validation processes run simultaneously.
- const testFile = path.join('components', app, '__import_test__.mjs'); + const testFile = path.join('components', app, `__import_test_${Date.now()}_${process.pid}.mjs`);
446-447: Add error handling for file write operations.The file write operation could fail due to permissions or disk space issues.
- fs.writeFileSync(filename, JSON.stringify(report, null, 2)); - console.log(`\n📄 Report saved to: ${filename}`); + try { + fs.writeFileSync(filename, JSON.stringify(report, null, 2)); + console.log(`\n📄 Report saved to: ${filename}`); + } catch (error) { + console.error(`\n❌ Failed to save report to ${filename}: ${error.message}`); + throw error; + }.github/workflows/daily-package-validation.yaml (2)
32-32: Consider removing --no-frozen-lockfile flag for deterministic builds.Using
--no-frozen-lockfilecould lead to inconsistent validation results if dependencies change between runs.- run: pnpm install -r --no-frozen-lockfile + run: pnpm install -r
145-151:labelsmust be a comma-separated string, not an array
octokit.rest.issues.listForRepoandcreateexpectlabelsas a single string ("label1,label2").
Passing an array silently ignores the filter inlistForRepoand throws forcreate.- labels: ['package-validation', 'automated'], + labels: 'package-validation,automated', ... - labels: ['package-validation', 'automated', 'bug'] + labels: 'package-validation,automated,bug'Also applies to: 175-175
🧹 Nitpick comments (1)
.github/workflows/daily-package-validation.yaml (1)
184-184: Add newline at end of file.Files should end with a newline character for better compatibility with various tools.
echo "Daily validation completed with no issues." +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
.github/workflows/daily-package-validation.yaml(1 hunks)package.json(1 hunks)scripts/generate-package-report.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🧰 Additional context used
🧠 Learnings (2)
scripts/generate-package-report.js (2)
Learnt from: jcortes
PR: #14935
File: components/sailpoint/package.json:15-18
Timestamp: 2024-12-12T19:23:09.039Z
Learning: When developing Pipedream components, do not add built-in Node.js modules like fs to package.json dependencies, as they are native modules provided by the Node.js runtime.
Learnt from: GTFalcao
PR: #15436
File: components/printful/printful.app.mjs:55-63
Timestamp: 2025-01-29T22:59:38.825Z
Learning: Console.log statements should be removed before merging PRs to maintain code quality and prevent potential security risks from exposing sensitive information in logs.
.github/workflows/daily-package-validation.yaml (2)
Learnt from: GTFalcao
PR: #13957
File: components/agility_cms/package.json:18-18
Timestamp: 2024-10-08T15:33:38.240Z
Learning: Before suggesting the removal of a dependency, thoroughly verify its usage across the entire codebase, even if not apparent in the diff.
Learnt from: GTFalcao
PR: #13957
File: components/agility_cms/package.json:18-18
Timestamp: 2024-09-16T19:39:11.875Z
Learning: Before suggesting the removal of a dependency, thoroughly verify its usage across the entire codebase, even if not apparent in the diff.
🪛 YAMLlint (1.37.1)
.github/workflows/daily-package-validation.yaml
[error] 15-15: trailing spaces
(trailing-spaces)
[error] 19-19: trailing spaces
(trailing-spaces)
[error] 24-24: trailing spaces
(trailing-spaces)
[error] 30-30: trailing spaces
(trailing-spaces)
[error] 33-33: trailing spaces
(trailing-spaces)
[error] 36-36: trailing spaces
(trailing-spaces)
[error] 43-43: trailing spaces
(trailing-spaces)
[error] 52-52: trailing spaces
(trailing-spaces)
[error] 70-70: trailing spaces
(trailing-spaces)
[error] 77-77: trailing spaces
(trailing-spaces)
[error] 82-82: trailing spaces
(trailing-spaces)
[error] 103-103: trailing spaces
(trailing-spaces)
[error] 109-109: trailing spaces
(trailing-spaces)
[error] 118-118: trailing spaces
(trailing-spaces)
[error] 121-121: trailing spaces
(trailing-spaces)
[error] 123-123: trailing spaces
(trailing-spaces)
[error] 127-127: trailing spaces
(trailing-spaces)
[error] 130-130: trailing spaces
(trailing-spaces)
[error] 133-133: trailing spaces
(trailing-spaces)
[error] 139-139: trailing spaces
(trailing-spaces)
[error] 143-143: trailing spaces
(trailing-spaces)
[error] 151-151: trailing spaces
(trailing-spaces)
[error] 153-153: trailing spaces
(trailing-spaces)
[error] 154-154: trailing spaces
(trailing-spaces)
[error] 157-157: trailing spaces
(trailing-spaces)
[error] 165-165: trailing spaces
(trailing-spaces)
[error] 179-179: trailing spaces
(trailing-spaces)
[error] 184-184: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Lint Code Base
- GitHub Check: pnpm publish
- GitHub Check: Generate Package Validation Report
🔇 Additional comments (6)
scripts/generate-package-report.js (4)
1-50: LGTM! Well-structured CLI interface.The command-line argument parsing and help documentation are comprehensive and user-friendly.
51-85: LGTM! Excellent user experience for package discovery.The package discovery logic includes helpful error messages and suggestions when a package is not found.
296-307: LGTM! Proper validation of main file exports.The function correctly checks for both ES6 and CommonJS exports.
450-455: LGTM! Proper module structure.The script correctly supports both CLI execution and module import usage.
.github/workflows/daily-package-validation.yaml (2)
8-9: Remove temporary test branch trigger before merging.The push trigger for
fix-test-importsbranch is marked as temporary and should be removed before merging to production.
37-52: LGTM! Well-structured validation execution and artifact handling.The workflow correctly captures validation results while ensuring reports are always uploaded, even on failure.
jcortes
force-pushed
the
fix-test-imports
branch
from
309330d to
33fb956
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
.github/actions/create-package-validation-issue/src/index.js (2)
139-141: Fix labels parameter format for GitHub API.Based on past review comments, the
labelsparameter should be a comma-separated string, not an array, for the GitHub API.labels: ['package-validation', 'automated'], + labels: 'package-validation,automated',
179-179: Fix labels parameter format for issue creation.Similar to the listForRepo call, the labels should be a comma-separated string.
- labels: ['package-validation', 'automated', 'bug'] + labels: 'package-validation,automated,bug'
🧹 Nitpick comments (2)
.github/actions/create-package-validation-issue/action.yml (1)
30-30: Add missing newline at end of file.Static analysis detected a missing newline character at the end of the file.
runs: using: 'node20' - main: 'src/index.js' + main: 'src/index.js' +.github/workflows/daily-package-validation.yaml (1)
15-95: Fix formatting issues throughout the file.Multiple trailing spaces and missing newline at end of file detected by static analysis.
runs-on: ubuntu-latest - + steps: - name: Checkout code uses: actions/[email protected] - + - name: Setup pnpm uses: pnpm/[email protected] with: version: 9.14.2 - + - name: Setup Node.js uses: actions/[email protected] with: node-version: 18 cache: 'pnpm' - + - name: Install dependencies run: pnpm install -r --no-frozen-lockfile - + - name: Compile TypeScript run: pnpm run build - + - name: Run Package Validation Report id: validation run: | node scripts/generate-package-report.js --verbose --report-only --output=validation-report.json > validation-report.txt 2>&1 echo "validation_exit_code=$?" >> $GITHUB_OUTPUT continue-on-error: true - + - name: Upload Validation Report uses: actions/upload-artifact@v4 with: name: package-validation-report-${{ github.run_number }} path: | validation-report.txt validation-report.json retention-days: 30 - + - name: Check for failures id: check_failures run: | if [ -f "validation-report.json" ]; then FAILED_COUNT=$(node -e " const fs = require('fs'); try { const report = JSON.parse(fs.readFileSync('validation-report.json', 'utf8')); console.log(report.summary.failed || 0); } catch { console.log(0); } ") echo "failed_count=$FAILED_COUNT" >> $GITHUB_OUTPUT else echo "failed_count=0" >> $GITHUB_OUTPUT fi - + - name: Create Issue on Failures if: steps.check_failures.outputs.failed_count != '0' id: create_issue uses: ./.github/actions/create-package-validation-issue with: github-token: ${{ secrets.GITHUB_TOKEN }} validation-report-json: 'validation-report.json' validation-report-txt: 'validation-report.txt' workflow-run-number: ${{ github.run_number }} workflow-run-id: ${{ github.run_id }} server-url: ${{ github.server_url }} repository: ${{ github.repository }} - + - name: Post Issue Summary if: steps.create_issue.conclusion == 'success' && steps.create_issue.outputs.issue-url != '' run: | echo "📋 Issue created/updated: ${{ steps.create_issue.outputs.issue-url }}" echo "❌ Failed packages: ${{ steps.create_issue.outputs.failed-count }}" echo "🔄 Issue was ${{ steps.create_issue.outputs.issue-created == 'true' && 'created' || 'updated' }}" - + - name: Post Success Summary if: steps.check_failures.outputs.failed_count == '0' run: | echo "🎉 All packages validated successfully!" - echo "Daily validation completed with no issues." + echo "Daily validation completed with no issues." +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
.github/actions/create-package-validation-issue/README.md(1 hunks).github/actions/create-package-validation-issue/action.yml(1 hunks).github/actions/create-package-validation-issue/package.json(1 hunks).github/actions/create-package-validation-issue/src/index.js(1 hunks).github/actions/create-package-validation-issue/test/test-action.js(1 hunks).github/workflows/daily-package-validation.yaml(1 hunks)package.json(1 hunks)scripts/generate-package-report.js(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- .github/actions/create-package-validation-issue/package.json
- .github/actions/create-package-validation-issue/README.md
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- scripts/generate-package-report.js
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: when exporting a summary message in the `run` method of an action, ensure the message is correctly f...
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#12731
File: components/hackerone/actions/get-members/get-members.mjs:3-28
Timestamp: 2024-10-08T15:33:38.240Z
Learning: When exporting a summary message in the `run` method of an action, ensure the message is correctly formatted. For example, in the `hackerone-get-members` action, the correct format is `Successfully retrieved ${response.data.length} members`.
Applied to files:
.github/actions/create-package-validation-issue/src/index.js
📚 Learning: before suggesting the removal of a dependency, thoroughly verify its usage across the entire codebas...
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#13957
File: components/agility_cms/package.json:18-18
Timestamp: 2024-09-16T19:39:11.875Z
Learning: Before suggesting the removal of a dependency, thoroughly verify its usage across the entire codebase, even if not apparent in the diff.
Applied to files:
.github/workflows/daily-package-validation.yaml
🧬 Code Graph Analysis (1)
.github/actions/create-package-validation-issue/test/test-action.js (1)
.github/actions/create-package-validation-issue/src/index.js (1)
fs(3-3)
🪛 YAMLlint (1.37.1)
.github/actions/create-package-validation-issue/action.yml
[error] 30-30: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/daily-package-validation.yaml
[error] 15-15: trailing spaces
(trailing-spaces)
[error] 19-19: trailing spaces
(trailing-spaces)
[error] 24-24: trailing spaces
(trailing-spaces)
[error] 30-30: trailing spaces
(trailing-spaces)
[error] 33-33: trailing spaces
(trailing-spaces)
[error] 36-36: trailing spaces
(trailing-spaces)
[error] 43-43: trailing spaces
(trailing-spaces)
[error] 52-52: trailing spaces
(trailing-spaces)
[error] 70-70: trailing spaces
(trailing-spaces)
[error] 83-83: trailing spaces
(trailing-spaces)
[error] 90-90: trailing spaces
(trailing-spaces)
[error] 95-95: no new line character at the end of file
(new-line-at-end-of-file)
🪛 actionlint (1.7.7)
.github/workflows/daily-package-validation.yaml
85-85: property "issue-url" is not defined in object type {}
(expression)
86-86: property "issue-url" is not defined in object type {}
(expression)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
- GitHub Check: Generate Package Validation Report
🔇 Additional comments (2)
.github/actions/create-package-validation-issue/test/test-action.js (1)
1-176: Excellent test implementation!The test script provides comprehensive local testing capability with:
- Proper mocking of GitHub Actions environment and APIs
- Realistic sample validation data
- Clean module patching technique for dependency injection
- Proper cleanup of temporary files
This will be valuable for development and debugging of the action.
.github/actions/create-package-validation-issue/src/index.js (1)
5-195: Excellent implementation with robust error handling!The action implementation demonstrates several strong qualities:
- Comprehensive fallback logic from JSON to text reports
- Proper error handling throughout with informative logging
- Smart deduplication by checking for existing issues
- Well-structured issue body with clear categorization
- Appropriate use of GitHub Actions APIs
The code correctly handles edge cases and provides valuable debugging information.
jcortes
force-pushed
the
fix-test-imports
branch
from
33fb956 to
9987df1
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
.github/workflows/daily-package-validation.yaml (2)
8-10: Remove temporary push trigger before merge
Thepushtrigger forfix-test-importswas called out in earlier reviews and is still present. It will run the heavy daily job on every push to that branch.
31-33: Deterministic installs: drop--no-frozen-lockfileRe-raising the prior comment:
--no-frozen-lockfiledefeats reproducible builds and will let transitive updates slip in silently.
🧹 Nitpick comments (2)
.github/actions/create-package-validation-issue/action.yml (1)
31-41: Trim trailing whitespace & add newline at EOFYAML-lint points out trailing spaces on Lines 35 and 41 and the missing final newline. These are tiny, but they break our “clean-lint” rule for workflow files.
.github/workflows/daily-package-validation.yaml (1)
1-95: Minor YAML-lint noiseThere are multiple trailing-space warnings and a missing newline at EOF. Run
yamllint -d relaxedor strip withsed -i 's/[ \t]*$//'.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
.github/actions/create-package-validation-issue/.gitignore(1 hunks).github/actions/create-package-validation-issue/README.md(1 hunks).github/actions/create-package-validation-issue/action.yml(1 hunks).github/actions/create-package-validation-issue/package.json(1 hunks).github/actions/create-package-validation-issue/src/index.js(1 hunks).github/actions/create-package-validation-issue/test/test-action.js(1 hunks).github/workflows/daily-package-validation.yaml(1 hunks)package.json(1 hunks)scripts/generate-package-report.js(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- .github/actions/create-package-validation-issue/.gitignore
- .github/actions/create-package-validation-issue/test/test-action.js
- .github/actions/create-package-validation-issue/README.md
🚧 Files skipped from review as they are similar to previous changes (4)
- .github/actions/create-package-validation-issue/package.json
- .github/actions/create-package-validation-issue/src/index.js
- scripts/generate-package-report.js
- package.json
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: before suggesting the removal of a dependency, thoroughly verify its usage across the entire codebas...
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#13957
File: components/agility_cms/package.json:18-18
Timestamp: 2024-09-16T19:39:11.875Z
Learning: Before suggesting the removal of a dependency, thoroughly verify its usage across the entire codebase, even if not apparent in the diff.
Applied to files:
.github/workflows/daily-package-validation.yaml
📚 Learning: console.log statements should be removed before merging prs to maintain code quality and prevent pot...
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#15436
File: components/printful/printful.app.mjs:55-63
Timestamp: 2025-01-29T22:59:38.825Z
Learning: Console.log statements should be removed before merging PRs to maintain code quality and prevent potential security risks from exposing sensitive information in logs.
Applied to files:
.github/workflows/daily-package-validation.yaml
📚 Learning: for "dir" props in pipedream components, whether to mark them as optional depends on the action's fi...
Learnt from: js07
PR: PipedreamHQ/pipedream#17375
File: components/zerobounce/actions/get-validation-results-file/get-validation-results-file.mjs:23-27
Timestamp: 2025-07-01T17:07:48.193Z
Learning: For "dir" props in Pipedream components, whether to mark them as optional depends on the action's file I/O behavior - if an action always writes files as output, the "dir" prop should not be marked as optional.
Applied to files:
.github/workflows/daily-package-validation.yaml
🪛 YAMLlint (1.37.1)
.github/actions/create-package-validation-issue/action.yml
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 41-41: trailing spaces
(trailing-spaces)
[error] 54-54: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/daily-package-validation.yaml
[error] 15-15: trailing spaces
(trailing-spaces)
[error] 19-19: trailing spaces
(trailing-spaces)
[error] 24-24: trailing spaces
(trailing-spaces)
[error] 30-30: trailing spaces
(trailing-spaces)
[error] 33-33: trailing spaces
(trailing-spaces)
[error] 36-36: trailing spaces
(trailing-spaces)
[error] 43-43: trailing spaces
(trailing-spaces)
[error] 52-52: trailing spaces
(trailing-spaces)
[error] 70-70: trailing spaces
(trailing-spaces)
[error] 83-83: trailing spaces
(trailing-spaces)
[error] 90-90: trailing spaces
(trailing-spaces)
[error] 95-95: no new line character at the end of file
(new-line-at-end-of-file)
🪛 actionlint (1.7.7)
.github/workflows/daily-package-validation.yaml
85-85: property "issue-url" is not defined in object type {}
(expression)
86-86: property "issue-url" is not defined in object type {}
(expression)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
- GitHub Check: Generate Package Validation Report
jcortes
force-pushed
the
fix-test-imports
branch
from
9987df1 to
9df2b0e
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/actions/create-package-validation-issue/action.yml (1)
1-30: Still missingoutputs:block – workflow steps downstream will breakDown-stream workflow
scheduled-package-validation.yamlconsumessteps.create_issue.outputs.{issue-url,failed-count,issue-created}, yet this composite action still does not expose any outputs.
actionlintalready flags this and the workflow will error at runtime.+outputs: + issue-url: + description: "URL of the created or updated issue" + value: ${{ steps.run_issue.outputs.issue-url }} + failed-count: + description: "Number of failed packages" + value: ${{ steps.run_issue.outputs.failed-count }} + issue-created: + description: "Whether the issue was newly created" + value: ${{ steps.run_issue.outputs.issue-created }}You will also need to give the “Run issue creation” step an
id(e.g.id: run_issue) and ensuresrc/index.jswrites the three outputs viaecho "name=value" >> "$GITHUB_OUTPUT".
🧹 Nitpick comments (4)
.github/actions/create-package-validation-issue/action.yml (2)
31-41: Trim trailing whitespace to satisfy lintersLines 35 and 41 contain trailing spaces which trigger YAML-lint warnings. Remove them to keep the CI green.
- node-version: '20'␠ + node-version: '20' @@ - npm install␠ + npm install
54-54: Add a newline at EOFPOSIX tools expect text files to end with
\n. Append a final newline to silenceYAMLlint“no new line character at end of file”..github/workflows/scheduled-package-validation.yaml (2)
38-40: Unusedvalidation_exit_codeoutputThe exit code is stored in
$GITHUB_OUTPUTbut never read later. Either drop the variable or reference it where you need it (e.g. to decide success vs failure).
13-50: Clean up trailing whitespaceMultiple lines contain trailing spaces (YAML-lint warnings). Run
prettier --writeor similar to avoid noisy diffs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
.github/actions/create-package-validation-issue/.gitignore(1 hunks).github/actions/create-package-validation-issue/README.md(1 hunks).github/actions/create-package-validation-issue/action.yml(1 hunks).github/actions/create-package-validation-issue/package.json(1 hunks).github/actions/create-package-validation-issue/src/index.js(1 hunks).github/actions/create-package-validation-issue/test/test-action.js(1 hunks).github/workflows/scheduled-package-validation.yaml(1 hunks)package.json(1 hunks)scripts/generate-package-report.js(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- .github/actions/create-package-validation-issue/README.md
- .github/actions/create-package-validation-issue/test/test-action.js
🚧 Files skipped from review as they are similar to previous changes (5)
- .github/actions/create-package-validation-issue/.gitignore
- package.json
- .github/actions/create-package-validation-issue/package.json
- .github/actions/create-package-validation-issue/src/index.js
- scripts/generate-package-report.js
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/actions/create-package-validation-issue/action.yml
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 41-41: trailing spaces
(trailing-spaces)
[error] 54-54: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/scheduled-package-validation.yaml
[error] 13-13: trailing spaces
(trailing-spaces)
[error] 17-17: trailing spaces
(trailing-spaces)
[error] 22-22: trailing spaces
(trailing-spaces)
[error] 28-28: trailing spaces
(trailing-spaces)
[error] 31-31: trailing spaces
(trailing-spaces)
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 41-41: trailing spaces
(trailing-spaces)
[error] 50-50: trailing spaces
(trailing-spaces)
[error] 68-68: trailing spaces
(trailing-spaces)
[error] 81-81: trailing spaces
(trailing-spaces)
[error] 88-88: trailing spaces
(trailing-spaces)
[error] 93-93: no new line character at the end of file
(new-line-at-end-of-file)
🪛 actionlint (1.7.7)
.github/workflows/scheduled-package-validation.yaml
83-83: property "issue-url" is not defined in object type {}
(expression)
84-84: property "issue-url" is not defined in object type {}
(expression)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
🔇 Additional comments (1)
.github/workflows/scheduled-package-validation.yaml (1)
70-88: Workflow will fail until the action exposes the expected outputsThese expressions reference
steps.create_issue.outputs.*, but those keys are currently undefined because the composite action doesn’t declareoutputs:. After fixing the action, rename the step inside the composite torun_issue(or similar) so the values propagate.Until then the matrix will evaluate to empty strings and
actionlintblocks the workflow.
jcortes
force-pushed
the
fix-test-imports
branch
from
9df2b0e to
fb383e6
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/actions/create-package-validation-issue/action.yml (1)
28-54: Still missingoutputs:– downstream workflow will crash
scheduled-package-validation.yamlexpectssteps.create_issue.outputs.{issue-url,failed-count,issue-created}but this action still does not declare anyoutputs. The step will therefore expose no outputs and the workflow will fail at runtime (actionlint already warns).Add an
outputsblock right after theinputssection:27 + +outputs: + issue-url: + description: "URL of the created or updated issue" + value: ${{ steps.run_issue.outputs.issue_url }} + failed-count: + description: "Number of failed packages" + value: ${{ steps.run_issue.outputs.failed_count }} + issue-created: + description: "Whether the issue was newly created" + value: ${{ steps.run_issue.outputs.issue_created }}(Replace the
valueexpressions with whatever yoursrc/index.jsactually sets viacore.setOutput).
🧹 Nitpick comments (3)
.github/actions/create-package-validation-issue/action.yml (2)
31-41: Enable Node dependency caching for faster runs
Every invocation doesnpm installwith no cache, slowing the workflow. Consider:- - name: Install action dependencies - shell: bash - run: | - cd ${{ github.action_path }} - npm install + - name: Restore / install Node deps + uses: actions/setup-node@v4 + with: + node-version: '20' + cache: 'npm' + - run: npm ci --prefix "${{ github.action_path }}" + shell: bashThis leverages
actions/setup-nodecache support and usesnpm cifor deterministic installs.
32-54: YAML lint: trailing spaces & missing newline
Lines 35, 41 and 54 contain trailing whitespace; the file also lacks a terminating newline – both flagged by YAML-lint. Quick fix:35- 41- +.github/workflows/scheduled-package-validation.yaml (1)
15-52: Clean up ubiquitous trailing spaces
Multiple lines (15, 19, 24, 30, 33, 36, 43, 52 …) include trailing whitespace. While harmless, they keep appearing in lint output. Remove them to quiet the linter and keep diffs clean.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
.github/actions/create-package-validation-issue/.gitignore(1 hunks).github/actions/create-package-validation-issue/README.md(1 hunks).github/actions/create-package-validation-issue/action.yml(1 hunks).github/actions/create-package-validation-issue/package.json(1 hunks).github/actions/create-package-validation-issue/src/index.js(1 hunks).github/actions/create-package-validation-issue/test/test-action.js(1 hunks).github/workflows/scheduled-package-validation.yaml(1 hunks)package.json(1 hunks)scripts/generate-package-report.js(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- .github/actions/create-package-validation-issue/test/test-action.js
- .github/actions/create-package-validation-issue/README.md
🚧 Files skipped from review as they are similar to previous changes (5)
- .github/actions/create-package-validation-issue/.gitignore
- .github/actions/create-package-validation-issue/package.json
- package.json
- scripts/generate-package-report.js
- .github/actions/create-package-validation-issue/src/index.js
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/actions/create-package-validation-issue/action.yml
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 41-41: trailing spaces
(trailing-spaces)
[error] 54-54: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/scheduled-package-validation.yaml
[error] 15-15: trailing spaces
(trailing-spaces)
[error] 19-19: trailing spaces
(trailing-spaces)
[error] 24-24: trailing spaces
(trailing-spaces)
[error] 30-30: trailing spaces
(trailing-spaces)
[error] 33-33: trailing spaces
(trailing-spaces)
[error] 36-36: trailing spaces
(trailing-spaces)
[error] 43-43: trailing spaces
(trailing-spaces)
[error] 52-52: trailing spaces
(trailing-spaces)
[error] 70-70: trailing spaces
(trailing-spaces)
[error] 83-83: trailing spaces
(trailing-spaces)
[error] 90-90: trailing spaces
(trailing-spaces)
[error] 95-95: no new line character at the end of file
(new-line-at-end-of-file)
🪛 actionlint (1.7.7)
.github/workflows/scheduled-package-validation.yaml
85-85: property "issue-url" is not defined in object type {}
(expression)
86-86: property "issue-url" is not defined in object type {}
(expression)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
- GitHub Check: Generate Package Validation Report
🔇 Additional comments (2)
.github/workflows/scheduled-package-validation.yaml (2)
71-89: Step consumes undefined outputs from child action
steps.create_issue.outputs.{issue-url,failed-count,issue-created}are referenced but will be empty until the action exposes them (see previous comment). If the action isn’t updated, the workflow will fail withProperty does not existat Lines 85-89.No change needed here once the
outputsblock is added to the action, but keep the names byte-for-byte identical (issue-url, notissue_url).
84-90: Conditional summary step can still mis-fire on skipped action
if: steps.create_issue.conclusion == 'success' && …is fine, but note that whenCreate Issue on Failuresis skipped (e.g., zero failures)steps.create_issue.conclusionisnull, causing the expression to evaluate tofalse, so the step is skipped. All good, just confirming logic is intentional.
jcortes
force-pushed
the
fix-test-imports
branch
from
fb383e6 to
1524262
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
.github/actions/create-package-validation-issue/action.yml (3)
45-55: Pin third-party actions to a commit SHA for supply-chain safety
Using floating tags (actions/[email protected]) leaves the build open to supply-chain attacks if the tag is moved. Consider pinning to the full commit SHA published in the action’s release notes.-uses: actions/[email protected] +uses: actions/[email protected] # 3c2d0d8b3b9afab8d7e7cbb4e6c4f4e5b7d7d7d
50-55: Optimise dependency install inside composite action
npm installinstalls all deps (including dev) each run, which is slow for every workflow calling this composite action. In most cases you only need production deps and a deterministic install:-npm install +npm ci --omit=dev
49-55: Trailing whitespace & missing EOF newline
YAML-lint flags trailing spaces (Line 49, 55) and lack of a newline at the end of file (Line 69). While harmless at runtime, cleaning them avoids future noisy diffs.- npm install␠ + npm install … - INPUT_REPOSITORY: ${{ inputs.repository }}␠ + INPUT_REPOSITORY: ${{ inputs.repository }} +.github/workflows/scheduled-package-validation.yaml (3)
20-30: Action versions should be pinned to commit SHAs
For the same supply-chain reasons noted on the composite action, pin external actions (pnpm/action-setup,actions/setup-node,actions/checkout,actions/upload-artifact) to the exact commit SHA published by the maintainers.
53-70: Simplify failure-count extraction & avoid Node one-liners
Spawningnode -einside bash is brittle. Usejq(already available) or a smallgrep/awkto fetchsummary.failed. Example withjq:-FAILED_COUNT=$(node -e " - const fs = require('fs'); - try { - const report = JSON.parse(fs.readFileSync('validation-report.json', 'utf8')); - console.log(report.summary.failed || 0); - } catch { - console.log(0); - } -") +FAILED_COUNT=$(jq -r '.summary.failed // 0' validation-report.json 2>/dev/null || echo 0)This removes the Node dependency from a bash step.
1-95: Remove trailing whitespace & add newline at EOF
Multiple trailing-space warnings (Lines 15, 19, 24, 30 …) plus missing newline at EOF. Quick cleanup prevents future linter noise.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
.github/actions/create-package-validation-issue/.gitignore(1 hunks).github/actions/create-package-validation-issue/README.md(1 hunks).github/actions/create-package-validation-issue/action.yml(1 hunks).github/actions/create-package-validation-issue/package.json(1 hunks).github/actions/create-package-validation-issue/src/index.js(1 hunks).github/actions/create-package-validation-issue/test/test-action.js(1 hunks).github/workflows/scheduled-package-validation.yaml(1 hunks)package.json(1 hunks)scripts/generate-package-report.js(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- .github/actions/create-package-validation-issue/test/test-action.js
- .github/actions/create-package-validation-issue/README.md
🚧 Files skipped from review as they are similar to previous changes (5)
- .github/actions/create-package-validation-issue/.gitignore
- .github/actions/create-package-validation-issue/package.json
- package.json
- scripts/generate-package-report.js
- .github/actions/create-package-validation-issue/src/index.js
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/actions/create-package-validation-issue/action.yml
[error] 49-49: trailing spaces
(trailing-spaces)
[error] 55-55: trailing spaces
(trailing-spaces)
[error] 69-69: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/scheduled-package-validation.yaml
[error] 15-15: trailing spaces
(trailing-spaces)
[error] 19-19: trailing spaces
(trailing-spaces)
[error] 24-24: trailing spaces
(trailing-spaces)
[error] 30-30: trailing spaces
(trailing-spaces)
[error] 33-33: trailing spaces
(trailing-spaces)
[error] 36-36: trailing spaces
(trailing-spaces)
[error] 43-43: trailing spaces
(trailing-spaces)
[error] 52-52: trailing spaces
(trailing-spaces)
[error] 70-70: trailing spaces
(trailing-spaces)
[error] 83-83: trailing spaces
(trailing-spaces)
[error] 90-90: trailing spaces
(trailing-spaces)
[error] 95-95: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Lint Code Base
- GitHub Check: Generate Package Validation Report
🔇 Additional comments (2)
.github/actions/create-package-validation-issue/action.yml (1)
28-41: Outputs section now present – good catch from previous review
The previously-missingoutputsblock is now declared and aligns with the keys referenced in downstream workflows. Nice fix..github/workflows/scheduled-package-validation.yaml (1)
84-90: Typo in output reference – key isfailed-count
The summary step referencessteps.create_issue.outputs.failed-count, but the action exposesfailed-count. Correct reference already matches; good. However, double-check that quotation marks around the ternary expression (Line 89) aren’t needed; GitHub expression syntax can treat==inside echo as bash, not expressions.
jcortes
force-pushed
the
fix-test-imports
branch
from
1524262 to
e3ab2f3
Compare
jcortes
force-pushed
the
fix-test-imports
branch
from
e3ab2f3 to
59a46a3
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/scheduled-package-validation.yaml (1)
35-40:continue-on-error: truemay silently hide real failuresIf
generate-package-report.jscrashes before writingvalidation-report.json, the step is marked successful and the subsequentcheck_failuresblock treats the situation as "0 failures". That masks script regressions.
🧹 Nitpick comments (2)
.github/actions/create-package-validation-issue/action.yml (1)
49-69: Fix formatting issues.Static analysis detected trailing spaces and missing newline at end of file.
- name: Setup Node.js for action uses: actions/[email protected] with: node-version: '20' - + - name: Install action dependencies shell: bash run: | cd ${{ github.action_path }} npm install - + - name: Run issue creation id: run_issue_creation shell: bash run: | cd ${{ github.action_path }} node src/index.js env: INPUT_GITHUB_TOKEN: ${{ inputs.github-token }} INPUT_VALIDATION_REPORT_JSON: ${{ inputs.validation-report-json }} INPUT_VALIDATION_REPORT_TXT: ${{ inputs.validation-report-txt }} INPUT_WORKFLOW_RUN_NUMBER: ${{ inputs.workflow-run-number }} INPUT_WORKFLOW_RUN_ID: ${{ inputs.workflow-run-id }} INPUT_SERVER_URL: ${{ inputs.server-url }} - INPUT_REPOSITORY: ${{ inputs.repository }} + INPUT_REPOSITORY: ${{ inputs.repository }}.github/workflows/scheduled-package-validation.yaml (1)
51-93: Fix formatting issues throughout the file.Static analysis detected multiple trailing spaces and missing newline at end of file.
- name: Check for failures id: check_failures - run: | + run: | if [ -f "validation-report.json" ]; then FAILED_COUNT=$(node -e " const fs = require('fs'); try { const report = JSON.parse(fs.readFileSync('validation-report.json', 'utf8')); console.log(report.summary.failed || 0); } catch { console.log(0); } ") echo "failed_count=$FAILED_COUNT" >> $GITHUB_OUTPUT else echo "failed_count=0" >> $GITHUB_OUTPUT - fi + fi - name: Create Issue on Failures if: steps.check_failures.outputs.failed_count != '0' id: create_issue uses: ./.github/actions/create-package-validation-issue with: github-token: ${{ secrets.GITHUB_TOKEN }} validation-report-json: 'validation-report.json' validation-report-txt: 'validation-report.txt' workflow-run-number: ${{ github.run_number }} workflow-run-id: ${{ github.run_id }} server-url: ${{ github.server_url }} - repository: ${{ github.repository }} + repository: ${{ github.repository }} - name: Post Issue Summary if: steps.create_issue.conclusion == 'success' && steps.create_issue.outputs.issue-url != '' - run: | + run: | echo "📋 Issue created/updated: ${{ steps.create_issue.outputs.issue-url }}" echo "❌ Failed packages: ${{ steps.create_issue.outputs.failed-count }}" - echo "🔄 Issue was ${{ steps.create_issue.outputs.issue-created == 'true' && 'created' || 'updated' }}" + echo "🔄 Issue was ${{ steps.create_issue.outputs.issue-created == 'true' && 'created' || 'updated' }}" - name: Post Success Summary if: steps.check_failures.outputs.failed_count == '0' run: | echo "🎉 All packages validated successfully!" - echo "Scheduled validation completed with no issues." + echo "Scheduled validation completed with no issues."
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
.github/actions/create-package-validation-issue/.gitignore(1 hunks).github/actions/create-package-validation-issue/README.md(1 hunks).github/actions/create-package-validation-issue/action.yml(1 hunks).github/actions/create-package-validation-issue/package.json(1 hunks).github/actions/create-package-validation-issue/src/index.js(1 hunks).github/actions/create-package-validation-issue/test/test-action.js(1 hunks).github/workflows/scheduled-package-validation.yaml(1 hunks)package.json(1 hunks)scripts/generate-package-report.js(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .github/actions/create-package-validation-issue/README.md
🚧 Files skipped from review as they are similar to previous changes (5)
- .github/actions/create-package-validation-issue/.gitignore
- .github/actions/create-package-validation-issue/test/test-action.js
- package.json
- scripts/generate-package-report.js
- .github/actions/create-package-validation-issue/package.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: when exporting a summary message in the `run` method of an action, ensure the message is correctly f...
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#12731
File: components/hackerone/actions/get-members/get-members.mjs:3-28
Timestamp: 2024-07-04T18:11:59.822Z
Learning: When exporting a summary message in the `run` method of an action, ensure the message is correctly formatted. For example, in the `hackerone-get-members` action, the correct format is `Successfully retrieved ${response.data.length} members`.
Applied to files:
.github/actions/create-package-validation-issue/src/index.js
🧬 Code Graph Analysis (1)
.github/actions/create-package-validation-issue/src/index.js (2)
.github/actions/create-package-validation-issue/test/test-action.js (1)
require(156-156)scripts/generate-package-report.js (6)
require(3-3)report(559-569)failuresByCategory(512-512)failures(208-210)packageName(116-116)packageName(393-393)
🪛 YAMLlint (1.37.1)
.github/actions/create-package-validation-issue/action.yml
[error] 49-49: trailing spaces
(trailing-spaces)
[error] 55-55: trailing spaces
(trailing-spaces)
[error] 69-69: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/scheduled-package-validation.yaml
[error] 13-13: trailing spaces
(trailing-spaces)
[error] 17-17: trailing spaces
(trailing-spaces)
[error] 22-22: trailing spaces
(trailing-spaces)
[error] 28-28: trailing spaces
(trailing-spaces)
[error] 31-31: trailing spaces
(trailing-spaces)
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 41-41: trailing spaces
(trailing-spaces)
[error] 50-50: trailing spaces
(trailing-spaces)
[error] 68-68: trailing spaces
(trailing-spaces)
[error] 81-81: trailing spaces
(trailing-spaces)
[error] 88-88: trailing spaces
(trailing-spaces)
[error] 93-93: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: pnpm publish
🔇 Additional comments (7)
.github/actions/create-package-validation-issue/action.yml (1)
28-40: LGTM! Outputs properly implemented.The outputs section correctly addresses the previous concern about missing output declarations. The value expressions properly reference the step outputs from
run_issue_creation..github/workflows/scheduled-package-validation.yaml (1)
3-12: LGTM! Well-structured scheduling and triggers.The cron schedule (every 3 days) and manual trigger option provide good balance between regular monitoring and on-demand testing.
.github/actions/create-package-validation-issue/src/index.js (5)
5-26: LGTM! Robust input handling and validation.The dual input handling (core.getInput vs environment variables) properly supports both regular and composite GitHub Actions. Input validation is comprehensive and provides clear error messages.
54-91: Excellent error handling with graceful fallbacks.The JSON parsing with fallback to text report parsing provides robust error recovery. The file path resolution using GITHUB_WORKSPACE ensures correct file access in different action contexts.
161-215: Well-implemented issue management logic.The existing issue detection by title and date, combined with the update-vs-create logic, prevents issue duplication while maintaining good audit trails. The use of appropriate labels for filtering and organization is excellent.
217-233: Comprehensive output handling for action compatibility.The dual output approach (core.setOutput + GITHUB_OUTPUT file) ensures compatibility with both regular and composite GitHub Actions. This addresses the integration requirements perfectly.
241-274: LGTM! Clear failure categorization logic.The generateFailureCategoriesText function provides well-structured failure grouping with appropriate limits (3 examples per category) to keep issue content manageable while informative.
|
Hey @jcortes, I think it'll be easier for you to test it on your side. This is not affecting end user. Feel free to release it if you indicated that it works. |
3 participants
WHY
Resolves #17842
Summary by CodeRabbit
New Features
Documentation
Tests
Chores