Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 85 additions & 7 deletions .github/scripts/post-merge-validation-tracker.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ const AUTOMATED_TEST_PATTERNS = [
/(^|\/)e2e\//,
/(^|\/)wdio\//
];
const SKIP_RELEASE_VALIDATION_LABEL = 'skip-release-validation';

if (!githubToken) throw new Error('Missing GITHUB_TOKEN env var');
if (!spreadsheetId) throw new Error('Missing SHEET_ID env var');
Expand Down Expand Up @@ -223,7 +224,7 @@ async function readRows(authClient, title) {
const res = await sheets.spreadsheets.values.get({
spreadsheetId,
auth: authClient,
range: `${title}!A3:H`,
range: `${title}!A3:J`,
});
return res.data.values || [];
} catch (e) {
Expand All @@ -237,7 +238,7 @@ async function appendRows(authClient, title, rows) {
await sheets.spreadsheets.values.append({
spreadsheetId,
auth: authClient,
range: `${title}!A4:H`,
range: `${title}!A4:J`,
valueInputOption: 'USER_ENTERED',
insertDataOption: 'INSERT_ROWS',
requestBody: { values: rows },
Expand Down Expand Up @@ -384,6 +385,73 @@ function splitByReleaseAndTitle(items) {
return { relevant, skippedByTitle };
}

function getAutoSkipLabelsForPR(labels, repoName) {
const labelNames = (labels || []).map((label) => label.name).filter(Boolean);
const hasSkipAll = labelNames.includes(SKIP_RELEASE_VALIDATION_LABEL);

const granularLabels = labelNames.filter((name) =>
/^skip-release-validation\[(android|ios|design|chrome|firefox)\]$/i.test(name),
);

const lowerRepoName = String(repoName || '').toLowerCase();
const isMobile = lowerRepoName.endsWith('-mobile');
const isExtension = lowerRepoName.endsWith('-extension');

const validGranularLabels = granularLabels.filter((name) => {
const target = name.toLowerCase();
if (isMobile && (target.includes('[chrome]') || target.includes('[firefox]'))) {
return false;
}
if (isExtension && (target.includes('[android]') || target.includes('[ios]'))) {
return false;
}
return true;
});

return {
hasSkipAll,
validLabels: validGranularLabels,
};
}

function applyAutoSkipToRow(row, labels, repoName) {

const { hasSkipAll, validLabels } = getAutoSkipLabelsForPR(labels, repoName);
if (!hasSkipAll && validLabels.length === 0) {
return row;
}

const updatedRow = [...row];
const validLabelSet = new Set(validLabels.map((label) => label.toLowerCase()));
const isMobile = String(repoName || '').toLowerCase().endsWith('-mobile');

const shouldSkipFirstValidated =
hasSkipAll ||
validLabelSet.has('skip-release-validation[design]') ||
validLabelSet.has(isMobile ? 'skip-release-validation[android]' : 'skip-release-validation[chrome]');
const shouldSkipSecondValidated =
hasSkipAll ||
validLabelSet.has('skip-release-validation[design]') ||
validLabelSet.has(isMobile ? 'skip-release-validation[ios]' : 'skip-release-validation[firefox]');

if (shouldSkipFirstValidated) {
updatedRow[6] = 'Skipped';
}
if (shouldSkipSecondValidated) {
updatedRow[7] = 'Skipped';
}

const labelsForComment = hasSkipAll
? [SKIP_RELEASE_VALIDATION_LABEL, ...validLabels]
: validLabels;

if (shouldSkipFirstValidated || shouldSkipSecondValidated) {
updatedRow[9] = `Release validation automatically skipped due to PR labels: ${labelsForComment.join(', ')}`;
}

return updatedRow;
}

// Add efficient version detection with caching
let versionCache = new Map(); // Cache version bumps per repo

Expand Down Expand Up @@ -538,6 +606,10 @@ async function buildTabGrouping(owner, repo, relevantItems, sinceDateISO) {
for (const pr of prs) {
// Check if PR modifies automated test files
const automatedTestsModified = await checkAutomatedTestFiles(owner, repo, pr.number);
const validatedA = '';
const validatedB = '';
const designValidation = '';
const comments = '';

const row = [
makePrHyperlinkCell(pr.html_url, pr.title, pr.number),
Expand All @@ -546,10 +618,16 @@ async function buildTabGrouping(owner, repo, relevantItems, sinceDateISO) {
extractSize(pr.labels || []),
automatedTestsModified,
extractTeam(pr.labels || []),
'',
'',
validatedA,
validatedB,
designValidation,
comments,
];
tabToRows.get(title).entries.push({ row, mergedAtIso: pr.closed_at || '' });
tabToRows.get(title).entries.push({
row,
mergedAtIso: pr.closed_at || '',
labels: pr.labels || [],
});
}
}

Expand Down Expand Up @@ -736,7 +814,7 @@ async function processTab(authClient, title, entries, platformType) {
const sortedRows = entries
.slice()
.sort((a, b) => new Date(a.mergedAtIso) - new Date(b.mergedAtIso))
.map((e) => e.row);
.map((e) => applyAutoSkipToRow(e.row, e.labels, repo));
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

processTab passes wrong repo format to auto-skip

Medium Severity

processTab is defined at module scope so the repo reference on this line resolves to the module-level repo ("MetaMask/metamask-extension", full owner/repo format) rather than the repo-name-only value ("metamask-extension") that processRepo receives as its parameter. applyAutoSkipToRow and getAutoSkipLabelsForPR accept a parameter named repoName and happen to work only because endsWith('-mobile') / endsWith('-extension') match both formats. If the detection logic is ever tightened, this will silently break. The repo name (not the full path) needs to be passed explicitly, e.g. as a parameter to processTab.

Additional Locations (2)
Fix in Cursor Fix in Web

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The finding is accurate as a code hygiene observation — processTab closing over the module-level repo instead of receiving an explicit parameter is a latent coupling issue — but it is not a live bug for our two supported repos:

.endsWith('-mobile')` and .endsWith('-extension')

const deduped = [];
for (const r of sortedRows) {
const num = parsePrNumberFromCell(r[0]);
Expand Down Expand Up @@ -830,4 +908,4 @@ async function main() {
main().catch((e) => {
console.error(e);
process.exit(1);
});
});
Loading