From ac42a2f1bbfb5891b9fe6f12d13b454af9e9cb37 Mon Sep 17 00:00:00 2001 From: Jonathan Flatt Date: Wed, 28 May 2025 16:01:28 -0500 Subject: [PATCH] fix: address PR review feedback on workflows - Fix integration test fallback to prevent masking real failures - Add deployment script validation before execution - Add environment file existence validation - Add continue-on-error for Codecov uploads to prevent CI failures - Use GitHub Actions artifacts to share Docker images between jobs - Significantly improves E2E test performance by avoiding Docker rebuilds These changes address all feedback points from PR review: - Better error handling and reliability - Improved performance with Docker image sharing - Added validation checks for critical resources - Prevents external service issues from breaking the workflow --- .github/workflows/ci.yml | 20 +++++++++++++++- .github/workflows/deploy.yml | 45 ++++++++++++++++++++++++++++++++++++ .github/workflows/pr.yml | 45 ++++++++++++++++++++++++------------ 3 files changed, 94 insertions(+), 16 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5c6ffe2..c8ee728 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -41,13 +41,29 @@ jobs: GITHUB_WEBHOOK_SECRET: 'test-secret' GITHUB_TOKEN: 'test-token' + - name: Check for integration tests + id: check-integration-tests + run: | + if grep -q '"test:integration"' package.json; then + echo "Integration tests found in package.json" + echo "has_integration_tests=true" >> $GITHUB_OUTPUT + else + echo "No integration tests found in package.json" + echo "has_integration_tests=false" >> $GITHUB_OUTPUT + fi + - name: Run integration tests - run: npm run test:integration || echo "No integration tests found, skipping" + if: steps.check-integration-tests.outputs.has_integration_tests == 'true' + run: npm run test:integration env: NODE_ENV: test BOT_USERNAME: '@TestBot' GITHUB_WEBHOOK_SECRET: 'test-secret' GITHUB_TOKEN: 'test-token' + + - name: Skip integration tests + if: steps.check-integration-tests.outputs.has_integration_tests != 'true' + run: echo "Integration tests script not found in package.json, skipping" - name: Run e2e tests run: npm run test:e2e @@ -67,9 +83,11 @@ jobs: - name: Upload coverage reports to Codecov uses: codecov/codecov-action@v5 + continue-on-error: true with: token: ${{ secrets.CODECOV_TOKEN }} slug: intelligence-assist/claude-hub + fail_ci_if_error: false # Security scans security: diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 14b21b5..928a405 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -40,6 +40,28 @@ jobs: ALLOWED_REPOS_STAGING=${{ vars.ALLOWED_REPOS_STAGING }} EOF + - name: Validate deployment script + run: | + if [ ! -f ./scripts/deploy/deploy-staging.sh ]; then + echo "::error::Deployment script not found: ./scripts/deploy/deploy-staging.sh" + exit 1 + fi + if [ ! -x ./scripts/deploy/deploy-staging.sh ]; then + echo "::error::Deployment script is not executable: ./scripts/deploy/deploy-staging.sh" + chmod +x ./scripts/deploy/deploy-staging.sh + echo "Made deployment script executable" + fi + + - name: Validate environment file + run: | + if [ ! -f .env.staging ]; then + echo "::error::Environment file not found: .env.staging" + exit 1 + fi + # Check if env file has required variables + grep -q "GITHUB_APP_ID_STAGING" .env.staging || echo "::warning::GITHUB_APP_ID_STAGING not found in env file" + grep -q "GITHUB_WEBHOOK_SECRET_STAGING" .env.staging || echo "::warning::GITHUB_WEBHOOK_SECRET_STAGING not found in env file" + - name: Deploy to staging run: | export $(cat .env.staging | xargs) @@ -117,6 +139,29 @@ jobs: DEPLOYMENT_VERSION=${{ steps.version.outputs.version }} EOF + - name: Validate deployment script + run: | + if [ ! -f ./scripts/deploy/deploy-production.sh ]; then + echo "::error::Deployment script not found: ./scripts/deploy/deploy-production.sh" + exit 1 + fi + if [ ! -x ./scripts/deploy/deploy-production.sh ]; then + echo "::error::Deployment script is not executable: ./scripts/deploy/deploy-production.sh" + chmod +x ./scripts/deploy/deploy-production.sh + echo "Made deployment script executable" + fi + + - name: Validate environment file + run: | + if [ ! -f .env ]; then + echo "::error::Environment file not found: .env" + exit 1 + fi + # Check if env file has required variables + grep -q "GITHUB_APP_ID" .env || echo "::warning::GITHUB_APP_ID not found in env file" + grep -q "GITHUB_WEBHOOK_SECRET" .env || echo "::warning::GITHUB_WEBHOOK_SECRET not found in env file" + grep -q "DEPLOYMENT_VERSION" .env || echo "::warning::DEPLOYMENT_VERSION not found in env file" + - name: Deploy to production run: | export $(cat .env | xargs) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 51443f6..23610dc 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -93,9 +93,11 @@ jobs: - name: Upload coverage reports to Codecov uses: codecov/codecov-action@v5 + continue-on-error: true with: token: ${{ secrets.CODECOV_TOKEN }} slug: intelligence-assist/claude-hub + fail_ci_if_error: false # Integration tests - moderate complexity test-integration: @@ -169,6 +171,21 @@ jobs: # Wait for both builds to complete wait + + - name: Save Docker images for e2e tests + run: | + # Save images to tarball artifacts for reuse in e2e tests + mkdir -p /tmp/docker-images + docker save claude-github-webhook:latest -o /tmp/docker-images/claude-github-webhook.tar + docker save claude-code-runner:latest -o /tmp/docker-images/claude-code-runner.tar + echo "Docker images saved for later reuse" + + - name: Upload Docker images as artifacts + uses: actions/upload-artifact@v4 + with: + name: docker-images + path: /tmp/docker-images/ + retention-days: 1 - name: Test Docker containers run: | @@ -203,22 +220,20 @@ jobs: - name: Set up Docker Buildx uses: docker/setup-buildx-action@v3 - - name: Restore Docker images from cache + - name: Download Docker images from artifacts + uses: actions/download-artifact@v4 + with: + name: docker-images + path: /tmp/docker-images + + - name: Load Docker images from artifacts run: | - # Use cached images from docker-build job - docker buildx build \ - --cache-from type=gha,scope=pr-main \ - --load \ - -t claude-github-webhook:latest \ - -f Dockerfile . & - - docker buildx build \ - --cache-from type=gha,scope=pr-claudecode \ - --load \ - -t claude-code-runner:latest \ - -f Dockerfile.claudecode . & - - wait + # Load images from saved artifacts (much faster than rebuilding) + echo "Loading Docker images from artifacts..." + docker load -i /tmp/docker-images/claude-github-webhook.tar + docker load -i /tmp/docker-images/claude-code-runner.tar + echo "Images loaded successfully:" + docker images | grep -E "claude-github-webhook|claude-code-runner" - name: Setup Node.js uses: actions/setup-node@v4