Add new filtering to exclude changes with picks to closed branchesHEAD dev

Fixes: QTQAINFRA-7311 Change-Id: Ib6431254713baee94cc0ce19759fe7358d72c21d Reviewed-by: Daniel Smith <daniel.smith@qt.io>
author: Daniel Smith <daniel.smith@qt.io> 2025-07-25 10:59:55 +0200
committer: Daniel Smith <daniel.smith@qt.io> 2025-09-09 08:33:02 +0000
commit: 7bee94bb4f7afdac23e21edfb805f51cf02793d4 (patch)
tree: aa2633a5112432a462c312a27cdec2315f32a68c
parent: 006a9c773800731a174ead8d98aec02caa2b6339 (diff)
1 files changed, 239 insertions, 34 deletions
diff --git a/plugin_bots/cherrypick_status_db/cherrypick_status_db.js b/plugin_bots/cherrypick_status_db/cherrypick_status_db.js
index 602eaaf..5cb37ce 100644
--- a/plugin_bots/cherrypick_status_db/cherrypick_status_db.js
+++ b/plugin_bots/cherrypick_status_db/cherrypick_status_db.js
@@ -20,6 +20,9 @@ const safeJsonStringify = require('safe-json-stringify');
 
 // SQL Generation Helpers
 const STUCK_STATUSES = "('NOT_CREATED', 'MERGE_CONFLICT', 'NEW', 'DEFERRED', 'ABANDONED', 'FAILED_CI')";
+// A 'NOT_CREATED' branch is only considered truly blocked if its dependency is in one of these states.
+// A dependency that is also 'NOT_CREATED' is just a chain, not a blockage.
+const TRULY_STUCK_DEPENDENCY_STATUSES = "('MERGE_CONFLICT', 'NEW', 'DEFERRED', 'ABANDONED', 'FAILED_CI')";
 const NON_FINAL_STATUSES_FOR_STUCK_CHECK = "('MERGED', 'INTEGRATING', 'NOT_CREATED')";
 const MERGED_ABANDONED_STATUSES = "('MERGED', 'ABANDONED')";
 
@@ -39,7 +42,7 @@ function getStuckClause(tqtcOnly = false) {
       -- Clause 2: Exclude rows where the only non-final branches are 'NOT_CREATED'
       --           AND none of those 'NOT_CREATED' branches depend on a truly stuck branch.
       AND NOT (
-        -- Condition (a): All non-final branches are 'NOT_CREATED'
+        -- Condition (a): All non-final branches are 'NOT_CREATED' or 'INTEGRATING'
         NOT EXISTS (
           SELECT 1
           FROM jsonb_each(branches) AS non_final_check
@@ -54,7 +57,7 @@ function getStuckClause(tqtcOnly = false) {
           WHERE nc_branch.value->>'status' = 'NOT_CREATED'
           AND nc_branch.value->>'depends_on' IS NOT NULL
           ${tqtcNcBranchFilter}
-          AND branches->(nc_branch.value->>'depends_on')->>'status' IN ${STUCK_STATUSES}
+          AND branches->(nc_branch.value->>'depends_on')->>'status' IN ${TRULY_STUCK_DEPENDENCY_STATUSES}
           ${tqtcNcDependsOnFilter}
         )
       )
@@ -105,6 +108,32 @@ function getExclAbandonClause(tqtcOnly = false) {
     `;
 }
 
+function getExclClosedClause() {
+    return `
+      AND NOT (
+        -- This whole clause is true if a change should be excluded.
+        -- Exclude if:
+        -- 1. There is at least one non-merged branch
+        -- 2. ALL non-merged branches are on branches marked as 'closed'
+        -- 3. And none of those branches are in a critical, active state that requires attention
+        EXISTS (
+            SELECT 1
+            FROM jsonb_each(branches) AS b
+            WHERE b.value->>'status' <> 'MERGED'
+        )
+        AND
+        NOT EXISTS (
+            -- This subquery finds any OPEN (not-closed), non-merged branches.
+            -- If it finds one, the parent NOT EXISTS is false, and the change is NOT excluded.
+            SELECT 1
+            FROM jsonb_each(branches) AS b(k, v)
+            WHERE NOT (v ->> 'status' IN ('MERGED', 'INTEGRATING'))
+            AND COALESCE((v->>'isBranchClosed')::boolean, false) = false
+        )
+      )
+    `;
+}
+
 function envOrConfig(ID) {
   return process.env[ID] || config[ID];
 }
@@ -188,7 +217,7 @@ class cherrypick_status_db {
       }
 
       // Extract parameters, defaulting boolean flags to false if undefined
-      let { repos, owner, page, branches: branchesParam, hashtags: hashtagsParam, onlyStuck = false, exclAbandon = false, onlyExternalTqtc = false, changeId, subject } = req.body;
+      let { repos, owner, page, branches: branchesParam, hashtags: hashtagsParam, onlyStuck = false, exclAbandon = false, exclClosed = false, onlyExternalTqtc = false, changeId, subject } = req.body;
       let branchesArray = [];
       let hashtagsArray = [];
 
@@ -332,6 +361,10 @@ class cherrypick_status_db {
           queryText += getExclAbandonClause(false);
         }
 
+        if (exclClosed) {
+          queryText += getExclClosedClause();
+        }
+
         queryText += ' ORDER BY merged_dt DESC';
         queryText += ' LIMIT 25';
         if (page) {
@@ -431,7 +464,7 @@ class cherrypick_status_db {
 
           let gerritQuery = `change:${changeId} (project:${project} OR project:${counterpartProject})`;
 
-          const gerritApiUrl = `${gerritRESTTools.gerritBaseURL("changes")}/?q=${encodeURIComponent(gerritQuery)}&o=CURRENT_REVISION`;
+          const gerritApiUrl = `${gerritRESTTools.gerritBaseURL("changes")}/?q=${encodeURIComponent(gerritQuery)}&o=CURRENT_REVISION&o=CURRENT_COMMIT`;
 
           let gerritChanges;
           try {
@@ -443,8 +476,33 @@ class cherrypick_status_db {
           }
 
           // 3. Re-calculate dependencies and build the new branches object
-          const targetBranches = gerritChanges.map(c => c.branch);
-          const finalBranches = this.generateDBBranchesJSON(sourceBranch, targetBranches);
+          // Combine intended targets (from Pick-to headers) with actual branches of existing changes.
+          const allPickToTargets = new Set();
+          for (const gerritChange of gerritChanges) {
+              if (gerritChange.revisions && gerritChange.revisions[gerritChange.current_revision] && gerritChange.revisions[gerritChange.current_revision].commit) {
+                  const commitMessage = gerritChange.revisions[gerritChange.current_revision].commit.message;
+                  const pickedTo = toolbox.findPickToBranches('PICKSTATUS_REFRESH', commitMessage);
+                  pickedTo.forEach(b => allPickToTargets.add(b));
+              }
+          }
+
+          // Final target list starts with verbatim Pick-to targets.
+          const finalTargetBranches = new Set(allPickToTargets);
+          const cleaner = b => b ? b.replace(/^(?:(?:tqtc\/)?(?:lts|esm)-)?/, '') : null;
+
+          // Add cleaned branch names for existing changes that were not in any Pick-to list.
+          // This should generally work for changes which were picked to a specific branch
+          // since the pickbot will respect specific branches in the Pick-to header
+          // even though bare branches are the standard. Not doing this check
+          // could result in multiple entries for the same real branch.
+          for (const gerritChange of gerritChanges) {
+              if (!allPickToTargets.has(gerritChange.branch)) {
+                  finalTargetBranches.add(cleaner(gerritChange.branch));
+              }
+          }
+
+          const targetBranches = Array.from(finalTargetBranches);
+          const finalBranches = await this.generateDBBranchesJSON(sourceProject, sourceBranch, targetBranches);
 
           // 4. Add the original merged change to the structure
           const sourceCleanBranch = sourceBranch.replace(/^(?:(?:tqtc\/)?(?:lts|esm)-)?/, '');
@@ -453,6 +511,7 @@ class cherrypick_status_db {
               status: 'MERGED',
               depends_on: null,
               isTqtc: isSourceTqtc,
+              isBranchClosed: false, // The source branch is by definition open
               last_date: dbRecord.merged_dt,
               url: dbRecord.url
           };
@@ -706,11 +765,150 @@ class cherrypick_status_db {
     return validOrigin;
   }
 
+  async _determineBranchClosure(project, targetBranches) {
+    const client = await postgreSQLClient.pool.connect();
+    try {
+        const cleaner = b => b ? b.replace(/^(?:(?:tqtc\/)?(?:lts|esm)-)?/, '') : null;
+        const openBranches = new Set();
+        const isTqtcProject = project.includes('/tqtc-');
+        const publicRepo = isTqtcProject ? project.replace('/tqtc-', '/') : project;
+
+        let publicRepoExists = !isTqtcProject;
+        if (isTqtcProject) {
+            const checkPublicRepoQuery = {
+                text: `SELECT 1 FROM jira_branches WHERE project = $1 LIMIT 1`,
+                values: [publicRepo]
+            };
+            const checkRes = await client.query(checkPublicRepoQuery);
+            publicRepoExists = checkRes.rows.length > 0;
+        }
+
+        if (publicRepoExists) {
+            let publicBranchesToQuery = new Set();
+            for (const targetBranch of targetBranches) {
+                const cleanKey = cleaner(targetBranch);
+                if (cleanKey) {
+                    publicBranchesToQuery.add(cleanKey);
+                    publicBranchesToQuery.add(`lts-${cleanKey}`);
+                }
+            }
+
+            const jiraQuery = {
+                text: `SELECT branch FROM jira_branches WHERE project = $1 AND branch = ANY($2)`,
+                values: [publicRepo, Array.from(publicBranchesToQuery)]
+            };
+            publicBranchesToQuery.clear();
+            const jiraRes = await client.query(jiraQuery);
+            if (jiraRes.rows) {
+                for (const row of jiraRes.rows) {
+                    publicBranchesToQuery.add(row.branch);
+                }
+            }
+
+            const query = {
+                text: `SELECT branch FROM closed_branches WHERE repo = $1 AND branch = ANY($2)`,
+                values: [publicRepo, Array.from(publicBranchesToQuery)]
+            };
+            const res = await client.query(query);
+            const closedPublicBranches = new Set();
+            if (res.rows) {
+                for (const row of res.rows) {
+                    closedPublicBranches.add(row.branch);
+                }
+            }
+
+            for (const branch of publicBranchesToQuery) {
+                if (!closedPublicBranches.has(branch)) {
+                    openBranches.add(cleaner(branch));
+                }
+            }
+
+            const tqtcRepo = isTqtcProject ? project : project.replace('/', '/tqtc-');
+            for (const closedBranch of closedPublicBranches) {
+                let branchesToCheck = [
+                    closedBranch,
+                    `tqtc/${closedBranch}`,
+                    `tqtc/lts-${closedBranch}`,
+                    `tqtc/esm-${closedBranch}`
+                ];
+
+                const tqtcQuery = {
+                    text: `SELECT branch FROM closed_branches WHERE repo = $1 AND branch = ANY($2)`,
+                    values: [tqtcRepo, branchesToCheck]
+                };
+                const tqtcRes = await client.query(tqtcQuery);
+                if (tqtcRes.rows) {
+                    for (const row of tqtcRes.rows) {
+                        const index = branchesToCheck.findIndex(b => b === row.branch);
+                        if (index > -1) {
+                            branchesToCheck.splice(index, 1);
+                        }
+                    }
+                }
+
+                if (branchesToCheck.length > 0) {
+                    const jiraTqtcQuery = {
+                        text: `SELECT branch FROM jira_branches WHERE project = $1 AND branch = ANY($2)`,
+                        values: [tqtcRepo, branchesToCheck]
+                    };
+                    const jiraTqtcRes = await client.query(jiraTqtcQuery);
+                    if (jiraTqtcRes.rows) {
+                        for (const row of jiraTqtcRes.rows) {
+                            openBranches.add(cleaner(row.branch));
+                        }
+                    }
+                }
+            }
+        } else {
+            const tqtcRepo = isTqtcProject ? project : project.replace('/', '/tqtc-');
+            for (const branch of targetBranches) {
+                let branchesToCheck = [
+                  branch,
+                  cleaner(branch),
+                  `tqtc/${cleaner(branch)}`,
+                  `tqtc/lts-${cleaner(branch)}`,
+                  `tqtc/esm-${cleaner(branch)}`
+                ];
+
+                const tqtcQuery = {
+                    text: `SELECT branch FROM closed_branches WHERE repo = $1 AND branch = ANY($2)`,
+                    values: [tqtcRepo, branchesToCheck]
+                };
+                const tqtcRes = await client.query(tqtcQuery);
+                if (tqtcRes.rows) {
+                    for (const row of tqtcRes.rows) {
+                        const index = branchesToCheck.findIndex(b => b === row.branch);
+                        if (index > -1) {
+                            branchesToCheck.splice(index, 1);
+                        }
+                    }
+                }
+
+                if (branchesToCheck.length > 0) {
+                    const jiraTqtcQuery = {
+                        text: `SELECT branch FROM jira_branches WHERE project = $1 AND branch = ANY($2)`,
+                        values: [tqtcRepo, branchesToCheck]
+                    };
+                    const jiraTqtcRes = await client.query(jiraTqtcQuery);
+                    if (jiraTqtcRes.rows) {
+                        for (const row of jiraTqtcRes.rows) {
+                            openBranches.add(cleaner(row.branch));
+                        }
+                    }
+                }
+            }
+        }
+        return openBranches;
+    } finally {
+        client.release();
+    }
+  }
+
   // Function to generate a JSON object representing the dependencies of branches
   // based on the waterfall function to calculate dependencies.
   // Results from the waterfall function are transformed into a flat JSON object
   // with the parent as the key.
-  generateDBBranchesJSON(baseBranch, targets) {
+  async generateDBBranchesJSON(project, baseBranch, targets) {
 
     function calculateDependencies(sourceBranch, pickList) {
       // Recursively calculate the dependencies of a branch using the waterfall function.
@@ -757,15 +955,20 @@ class cherrypick_status_db {
     // Get relationships with branch names
     const parentRelationships = invertAndFlattenHierarchy(calculateDependencies(baseBranch, targets));
 
+    // Determine which branches are closed
+    const openBranches = await this._determineBranchClosure(project, Object.keys(parentRelationships));
+
     const finalJSON = {};
 
     // Build the JSON passed to the database.
     const cleaner = b => b ? b.replace(/^(?:(?:tqtc\/)?(?:lts|esm)-)?/, '') : null;
     for (const targetBranch in parentRelationships) {
-      finalJSON[cleaner(targetBranch)] = {
+      const cleanKey = cleaner(targetBranch);
+      finalJSON[cleanKey] = {
         status: 'NOT_CREATED',
         depends_on: cleaner(parentRelationships[targetBranch]),
         isTqtc: /tqtc\//.test(targetBranch), // Default based on raw branch name. Will be updated on status event.
+        isBranchClosed: !openBranches.has(cleanKey),
         last_date: null,
         url: null
       };
@@ -775,20 +978,21 @@ class cherrypick_status_db {
   }
 
 
-  updateDB(change, newStatus, patchSet) {
+  async updateDB(change, newStatus, patchSet) {
     const project = change.project;
     const branch = change.branch;
     const changeId = change.id;
     // Clean the branch name for use as a key in the JSONB object
     const cleanBranch = branch.replace(/^(?:(?:tqtc\/)?(?:lts|esm)-)?/, '');
 
-    // Query first to see if the change ID exists.
-    postgreSQLClient.pool.query(
-      `SELECT dev_change_id FROM cherry_picks_tracking
-       WHERE dev_change_id LIKE $1
-       LIMIT 1`,
-      [`%${changeId}%`]
-    ).then(res => {
+    try {
+      // Query first to see if the change ID exists.
+      const res = await postgreSQLClient.pool.query(
+        `SELECT dev_change_id FROM cherry_picks_tracking
+         WHERE dev_change_id LIKE $1
+         LIMIT 1`,
+        [`%${changeId}%`]
+      );
       this.logger.log(`Found ${res.rows.length} existing rows for change ID ${changeId}`, 'silly', 'PICKSTATUS');
 
       // If rows exist, UPDATE the specific branch status.
@@ -818,12 +1022,12 @@ class cherrypick_status_db {
           ]
         };
 
-        return postgreSQLClient.pool.query(updateQuery)
-          .then(() => this.logger.log(`Pick Status update successful for ${devChangeId} with data ${safeJsonStringify({
-            branch: cleanBranch,
-            status: newStatus,
-            isTqtc: isTqtc
-          })}`, 'debug', 'PICKSTATUS'));
+        await postgreSQLClient.pool.query(updateQuery);
+        this.logger.log(`Pick Status update successful for ${devChangeId} with data ${safeJsonStringify({
+          branch: cleanBranch,
+          status: newStatus,
+          isTqtc: isTqtc
+        })}`, 'debug', 'PICKSTATUS');
       }
       // If no rows exist, INSERT a new record (If it has pick targets).
       else {
@@ -833,7 +1037,7 @@ class cherrypick_status_db {
             return;
         }
 
-        const branchesJSON = this.generateDBBranchesJSON(branch, pickToBranches);
+        const branchesJSON = await this.generateDBBranchesJSON(project, branch, pickToBranches);
 
         // Determine if the *triggering* change/branch is TQTC
         const isTqtc = /tqtc-/.test(project) || /tqtc\//.test(branch);
@@ -844,6 +1048,7 @@ class cherrypick_status_db {
           status: newStatus,
           depends_on: null,
           isTqtc: isTqtc,
+          isBranchClosed: false, // The source branch is by definition open
           last_date: new Date(change.updated) || new Date(),
           url: change.url
         };
@@ -889,20 +1094,20 @@ class cherrypick_status_db {
           });
         }
 
-        return postgreSQLClient.pool.query(insertQuery)
-          .then(() => this.logger.log(`Inserted cherry-pick status for ${PKChangeId} with data ${
-            safeJsonStringify({
-              branch: cleanBranch,
-              status: newStatus,
-              branches: branchesJSON,
-              hashtags: change.hashtags
-            })
-            }`, 'debug', 'PICKSTATUS'));
+        await postgreSQLClient.pool.query(insertQuery);
+        this.logger.log(`Inserted cherry-pick status for ${PKChangeId} with data ${
+          safeJsonStringify({
+            branch: cleanBranch,
+            status: newStatus,
+            branches: branchesJSON,
+            hashtags: change.hashtags
+          })
+          }`, 'debug', 'PICKSTATUS');
       }
-    }).catch(err => {
+    } catch (err) {
       // Catch errors from the initial SELECT or subsequent INSERT/UPDATE
       this.logger.log(`Database operation failed for change ID ${changeId}: ${err}`, 'error', 'PICKSTATUS');
-    });
+    }
   }
 }
author	Daniel Smith <daniel.smith@qt.io>	2025-07-25 10:59:55 +0200
committer	Daniel Smith <daniel.smith@qt.io>	2025-09-09 08:33:02 +0000
commit	7bee94bb4f7afdac23e21edfb805f51cf02793d4 (patch)
tree	aa2633a5112432a462c312a27cdec2315f32a68c
parent	006a9c773800731a174ead8d98aec02caa2b6339 (diff)