Merge pull request #100 from intelligence-assist/fix/smart-wait-for-all-checks

Fix automated PR review triggering with smart wait-for-all-checks logic

.codecov.yml

@@ -0,0 +1,28 @@
+codecov:
+  require_ci_to_pass: false
+  
+coverage:
+  status:
+    project:
+      default:
+        target: auto
+        threshold: 1%
+        base: auto
+        # Only check coverage on main branch
+        if_ci_failed: error
+    patch:
+      default:
+        target: auto
+        threshold: 1%
+        base: auto
+        # Only check coverage on main branch  
+        if_ci_failed: error
+
+comment:
+  layout: "reach,diff,flags,tree"
+  behavior: default
+  require_changes: false
+
+github_checks:
+  # Disable check suites to prevent hanging on non-main branches
+  annotations: false

.github/workflows/ci.yml

@@ -121,10 +121,12 @@ jobs:
     - name: Upload coverage to Codecov
       uses: codecov/codecov-action@v5
       with:
+        token: ${{ secrets.CODECOV_TOKEN }}
         file: ./coverage/lcov.info
         flags: unittests
         name: codecov-umbrella
         fail_ci_if_error: false
+        verbose: true
 
   # Security scans - run on GitHub for faster execution
   security:

.github/workflows/pr.yml

@@ -63,14 +63,7 @@ jobs:
         GITHUB_WEBHOOK_SECRET: 'test-secret'
         GITHUB_TOKEN: 'test-token'
 
-    - name: Upload coverage
-      if: matrix.node-version == '20.x'
-      uses: codecov/codecov-action@v5
-      with:
-        file: ./coverage/lcov.info
-        flags: unittests
-        name: codecov-umbrella
-        fail_ci_if_error: false
+    # Coverage is only uploaded on main branch via ci.yml workflow
 
   # Integration tests - moderate complexity
   test-integration:

CLAUDE.md

@@ -199,9 +199,11 @@ The `awsCredentialProvider.js` utility handles credential retrieval and rotation
 - `ANTHROPIC_API_KEY`: Anthropic API key for Claude access
 
 ### Optional Environment Variables
-- `PR_REVIEW_WAIT_FOR_ALL_CHECKS`: Set to `"true"` to wait for all check suites to complete successfully before triggering PR review (default: `"true"`). This prevents duplicate reviews from different check suites.
+- `PR_REVIEW_WAIT_FOR_ALL_CHECKS`: Set to `"true"` to wait for all meaningful check suites to complete successfully before triggering PR review (default: `"true"`). Uses smart logic to handle conditional jobs and skipped checks, preventing duplicate reviews from different check suites.
 - `PR_REVIEW_TRIGGER_WORKFLOW`: Name of a specific GitHub Actions workflow that should trigger PR reviews (e.g., `"Pull Request CI"`). Only used if `PR_REVIEW_WAIT_FOR_ALL_CHECKS` is `"false"`.
 - `PR_REVIEW_DEBOUNCE_MS`: Delay in milliseconds before checking all check suites status (default: `"5000"`). This accounts for GitHub's eventual consistency.
+- `PR_REVIEW_MAX_WAIT_MS`: Maximum time to wait for stale in-progress check suites before considering them failed (default: `"1800000"` = 30 minutes).
+- `PR_REVIEW_CONDITIONAL_TIMEOUT_MS`: Time to wait for conditional jobs that never start before skipping them (default: `"300000"` = 5 minutes).
 
 ## Code Style Guidelines
 - JavaScript with Node.js

README.md

@@ -9,7 +9,7 @@
 
 ![Claude GitHub Webhook brain factory - AI brain connected to GitHub octocat via assembly line of Docker containers](./assets/brain_factory.png)
 
-Deploy Claude Code as a fully autonomous GitHub bot. Mention @Claude in any issue or PR, and watch AI-powered development happen end-to-end. Claude can implement complete features, review code, merge PRs, wait for CI builds, and run for hours autonomously until tasks are completed. Production-ready microservice with container isolation, automated workflows, and intelligent project management.
+Deploy Claude Code as a fully autonomous GitHub bot. Mention @Claude in any issue or PR, and watch AI-powered development happen end-to-end. Claude can implement complete features, review code, merge PRs, wait for CI builds, and run for hours autonomously until tasks are completed. Production-ready microservice with container isolation, automated workflows, and intelligent project management. 
 
 ## What This Does
 

docker-compose.yml

@@ -23,10 +23,12 @@ services:
       - DEFAULT_BRANCH=${DEFAULT_BRANCH:-main}
       - CLAUDE_USE_CONTAINERS=1
       - CLAUDE_CONTAINER_IMAGE=claudecode:latest
-      # Wait for all checks by default, or use specific workflow trigger
+      # Smart wait for all meaningful checks by default, or use specific workflow trigger
       - PR_REVIEW_WAIT_FOR_ALL_CHECKS=${PR_REVIEW_WAIT_FOR_ALL_CHECKS:-true}
       - PR_REVIEW_TRIGGER_WORKFLOW=${PR_REVIEW_TRIGGER_WORKFLOW:-}
       - PR_REVIEW_DEBOUNCE_MS=${PR_REVIEW_DEBOUNCE_MS:-5000}
+      - PR_REVIEW_MAX_WAIT_MS=${PR_REVIEW_MAX_WAIT_MS:-1800000}
+      - PR_REVIEW_CONDITIONAL_TIMEOUT_MS=${PR_REVIEW_CONDITIONAL_TIMEOUT_MS:-300000}
       # Point to secret files instead of env vars
       - GITHUB_TOKEN_FILE=/run/secrets/github_token
       - ANTHROPIC_API_KEY_FILE=/run/secrets/anthropic_api_key

src/controllers/githubController.js

@@ -1069,14 +1069,17 @@ Please perform a comprehensive review of PR #${pr.number} in repository ${repo.f
 }
 
 /**
- * Checks if all check suites for a PR are complete and successful
+ * Checks if all meaningful check suites for a PR are complete and successful
+ * Uses smart logic to handle conditional jobs, timeouts, and skipped checks
  * @param {Object} options - Options object
  * @param {Object} options.repo - The repository object
  * @param {Array} options.pullRequests - Array of pull requests
- * @returns {Promise<boolean>} - True if all checks are complete and successful
+ * @returns {Promise<boolean>} - True if all meaningful checks are complete and successful
  */
 async function checkAllCheckSuitesComplete({ repo, pullRequests }) {
   const debounceDelayMs = parseInt(process.env.PR_REVIEW_DEBOUNCE_MS || '5000', 10);
+  const maxWaitTimeMs = parseInt(process.env.PR_REVIEW_MAX_WAIT_MS || '1800000', 10); // 30 min default
+  const conditionalJobTimeoutMs = parseInt(process.env.PR_REVIEW_CONDITIONAL_TIMEOUT_MS || '300000', 10); // 5 min default
 
   try {
     // Add a small delay to account for GitHub's eventual consistency
@@ -1094,6 +1097,7 @@ async function checkAllCheckSuitesComplete({ repo, pullRequests }) {
         });
 
         const checkSuites = checkSuitesResponse.check_suites || [];
+        const now = Date.now();
 
         logger.info(
           {
@@ -1105,20 +1109,149 @@ async function checkAllCheckSuitesComplete({ repo, pullRequests }) {
               id: cs.id,
               app: cs.app?.name,
               status: cs.status,
-              conclusion: cs.conclusion
+              conclusion: cs.conclusion,
+              createdAt: cs.created_at,
+              updatedAt: cs.updated_at
             }))
           },
           'Retrieved check suites for PR'
         );
 
-        // Check if any check suite is still in progress or has failed
+        // Categorize check suites for smarter processing
+        const meaningfulSuites = [];
+        const skippedSuites = [];
+        const timeoutSuites = [];
+
         for (const suite of checkSuites) {
-          // Skip neutral conclusions (like skipped checks)
+          const createdTime = new Date(suite.created_at).getTime();
+          const updatedTime = new Date(suite.updated_at).getTime();
+          const ageMs = now - createdTime;
+          const stalenessMs = now - updatedTime;
+
+          // Skip suites that were explicitly skipped or marked neutral
           if (suite.conclusion === 'neutral' || suite.conclusion === 'skipped') {
+            skippedSuites.push({
+              id: suite.id,
+              app: suite.app?.name,
+              conclusion: suite.conclusion,
+              reason: 'explicitly_skipped'
+            });
             continue;
           }
 
-          // If any check is in progress, we should wait
+          // Skip suites that never started and are old (likely conditional jobs that didn't trigger)
+          if (suite.status === 'queued' && ageMs > conditionalJobTimeoutMs) {
+            timeoutSuites.push({
+              id: suite.id,
+              app: suite.app?.name,
+              status: suite.status,
+              ageMs: ageMs,
+              reason: 'conditional_job_timeout'
+            });
+            logger.info(
+              {
+                repo: repo.full_name,
+                pr: pr.number,
+                checkSuite: suite.id,
+                app: suite.app?.name,
+                ageMs: ageMs,
+                conditionalJobTimeoutMs: conditionalJobTimeoutMs
+              },
+              'Skipping check suite that never started (likely conditional job)'
+            );
+            continue;
+          }
+
+          // Skip empty check suites that have no check runs (common with misconfigured external apps)
+          if (suite.status === 'queued' && suite.latest_check_runs_count === 0 && ageMs > 60000) { // 1 minute grace period
+            timeoutSuites.push({
+              id: suite.id,
+              app: suite.app?.name,
+              status: suite.status,
+              ageMs: ageMs,
+              reason: 'empty_check_suite'
+            });
+            logger.info(
+              {
+                repo: repo.full_name,
+                pr: pr.number,
+                checkSuite: suite.id,
+                app: suite.app?.name,
+                ageMs: ageMs,
+                checkRunsCount: suite.latest_check_runs_count
+              },
+              'Skipping empty check suite with no check runs (likely misconfigured external app)'
+            );
+            continue;
+          }
+
+          // Skip suites that have been stale for too long without updates
+          if (suite.status === 'in_progress' && stalenessMs > maxWaitTimeMs) {
+            timeoutSuites.push({
+              id: suite.id,
+              app: suite.app?.name,
+              status: suite.status,
+              stalenessMs: stalenessMs,
+              reason: 'stale_in_progress'
+            });
+            logger.warn(
+              {
+                repo: repo.full_name,
+                pr: pr.number,
+                checkSuite: suite.id,
+                app: suite.app?.name,
+                stalenessMs: stalenessMs
+              },
+              'Skipping stale check suite that has been in progress too long'
+            );
+            continue;
+          }
+
+          // This is a meaningful check suite that we should wait for
+          meaningfulSuites.push(suite);
+        }
+
+        logger.info(
+          {
+            repo: repo.full_name,
+            pr: pr.number,
+            totalSuites: checkSuites.length,
+            meaningfulSuites: meaningfulSuites.length,
+            skippedSuites: skippedSuites.length,
+            timeoutSuites: timeoutSuites.length,
+            skippedDetails: skippedSuites,
+            timeoutDetails: timeoutSuites
+          },
+          'Categorized check suites for smart processing'
+        );
+
+        // If no meaningful suites found, something might be wrong
+        if (meaningfulSuites.length === 0) {
+          logger.warn(
+            {
+              repo: repo.full_name,
+              pr: pr.number,
+              totalSuites: checkSuites.length
+            },
+            'No meaningful check suites found - all were skipped or timed out'
+          );
+          // If we only have skipped/neutral suites, consider that as "passed"
+          if (checkSuites.length > 0 && skippedSuites.length + timeoutSuites.length === checkSuites.length) {
+            logger.info(
+              {
+                repo: repo.full_name,
+                pr: pr.number
+              },
+              'All check suites were skipped/conditional - considering as passed'
+            );
+            continue; // Move to next PR
+          }
+          return false;
+        }
+
+        // Check meaningful check suites
+        for (const suite of meaningfulSuites) {
+          // If any meaningful check is still in progress, we should wait
           if (suite.status !== 'completed') {
             logger.info(
               {
@@ -1128,12 +1261,12 @@ async function checkAllCheckSuitesComplete({ repo, pullRequests }) {
                 app: suite.app?.name,
                 status: suite.status
               },
-              'Check suite still in progress'
+              'Meaningful check suite still in progress'
             );
             return false;
           }
 
-          // If any check failed, we shouldn't review
+          // If any meaningful check failed, we shouldn't review
           if (suite.conclusion !== 'success') {
             logger.info(
               {
@@ -1143,11 +1276,20 @@ async function checkAllCheckSuitesComplete({ repo, pullRequests }) {
                 app: suite.app?.name,
                 conclusion: suite.conclusion
               },
-              'Check suite did not succeed'
+              'Meaningful check suite did not succeed'
             );
             return false;
           }
         }
+
+        logger.info(
+          {
+            repo: repo.full_name,
+            pr: pr.number,
+            passedSuites: meaningfulSuites.length
+          },
+          'All meaningful check suites completed successfully'
+        );
       } catch (error) {
         logger.error(
           {
@@ -1161,7 +1303,14 @@ async function checkAllCheckSuitesComplete({ repo, pullRequests }) {
       }
     }
 
-    // All checks passed!
+    // All meaningful checks passed!
+    logger.info(
+      {
+        repo: repo.full_name,
+        prCount: pullRequests.length
+      },
+      'All PRs have meaningful check suites completed successfully'
+    );
     return true;
   } catch (error) {
     logger.error(