[ED-25748] Fix: Healer includes hard failures from earlier attempts of re-run evidence runs

actions/playwright-test-healer/scripts/collect-log-hard-failures.js

@@ -68,9 +68,17 @@ function listFailedJobIds(repo, runId) {
 }
 
 function collectHardFailureTitles(repo, runId) {
+	const { listFailedJobsAcrossAttempts } = require('./run-attempts');
 	const titles = new Set();
 
-	for (const jobId of listFailedJobIds(repo, runId)) {
+	// Collect failures from all attempts of the run, not just the latest
+	const failedJobs = listFailedJobsAcrossAttempts(repo, runId);
+
+	console.log(
+		`Collecting hard failures from ${failedJobs.length} failed job(s) across ${Math.max(...failedJobs.map((j) => j.attempt), 1)} attempt(s)`,
+	);
+
+	for (const { attempt, jobId } of failedJobs) {
 		const log = fetchJobLog(repo, jobId);
 
 		for (const title of parseHardFailureTitles(log)) {
@@ -81,6 +89,38 @@ function collectHardFailureTitles(repo, runId) {
 	return [...titles];
 }
 
+/**
+ * Collects hard failures with the attempt numbers they failed in.
+ * Returns a map of test title -> array of attempt numbers where it failed.
+ */
+function collectHardFailuresWithAttempts(repo, runId) {
+	const { listFailedJobsAcrossAttempts } = require('./run-attempts');
+	const failuresByTest = new Map();
+
+	const failedJobs = listFailedJobsAcrossAttempts(repo, runId);
+
+	console.log(
+		`Collecting hard failures with attempts from ${failedJobs.length} failed job(s) across ${Math.max(...failedJobs.map((j) => j.attempt), 1)} attempt(s)`,
+	);
+
+	for (const { attempt, jobId } of failedJobs) {
+		const log = fetchJobLog(repo, jobId);
+		const titles = parseHardFailureTitles(log);
+
+		for (const title of titles) {
+			if (!failuresByTest.has(title)) {
+				failuresByTest.set(title, []);
+			}
+			const attempts = failuresByTest.get(title);
+			if (!attempts.includes(attempt)) {
+				attempts.push(attempt);
+			}
+		}
+	}
+
+	return failuresByTest;
+}
+
 function main() {
 	const repo = process.env.GITHUB_REPOSITORY;
 	const runId = process.env.SOURCE_RUN_ID;
@@ -108,4 +148,8 @@ if (require.main === module) {
 	}
 }
 
-module.exports = { DEFAULT_HARD_FAILURES_PATH };
+module.exports = {
+	DEFAULT_HARD_FAILURES_PATH,
+	collectHardFailuresWithAttempts,
+	parseHardFailureTitles,
+};

actions/playwright-test-healer/scripts/find-test-evidence.js

@@ -48,6 +48,11 @@ function isShardArtifact(artifact) {
 /**
  * Trace-backed evidence beats log-only evidence, because the healer skill
  * can replay a trace but can only read an `error-context.md`.
+ * 
+ * When choosing between evidence from different attempts, prefer:
+ * 1. Evidence with traces over log-only evidence
+ * 2. Evidence from later attempts (more recent failures)
+ * 3. Earlier shard indices (arbitrary tiebreaker)
  */
 function pickBestEvidence(candidates) {
 	const usable = (candidates || []).filter(
@@ -59,9 +64,17 @@ function pickBestEvidence(candidates) {
 	}
 
 	const sorted = [...usable].sort((a, b) => {
+		// Prefer trace-backed evidence
 		if (a.hasTrace !== b.hasTrace) {
 			return a.hasTrace ? -1 : 1;
 		}
+		// Prefer later attempts (more recent)
+		const attemptA = a.attempt || 1;
+		const attemptB = b.attempt || 1;
+		if (attemptA !== attemptB) {
+			return attemptB - attemptA;
+		}
+		// Tiebreaker: shard index
 		return String(a.shardIndex).localeCompare(String(b.shardIndex));
 	});
 
@@ -72,21 +85,32 @@ function pickBestEvidence(candidates) {
  * Groups already-discovered result directories by the shard artifact they
  * came from, so the shard index travels with the evidence. Verification
  * needs it to re-run the test the way that shard ran it.
+ * 
+ * When a run has been re-run, directories from different attempts of the
+ * same artifact name are kept separate, annotated with their attempt number.
  */
 function buildEvidenceCandidates(testName, resultDirs) {
-	const byArtifact = new Map();
+	// Key by artifact name + attempt to keep attempts separate
+	const byArtifactAndAttempt = new Map();
 
 	for (const dir of resultDirs || []) {
 		const artifactName = dir.artifactName || '';
-		if (!byArtifact.has(artifactName)) {
-			byArtifact.set(artifactName, []);
+		const attempt = dir.attempt || 1;
+		const key = `${artifactName}::attempt-${attempt}`;
+
+		if (!byArtifactAndAttempt.has(key)) {
+			byArtifactAndAttempt.set(key, {
+				artifactName,
+				attempt,
+				dirs: [],
+			});
 		}
-		byArtifact.get(artifactName).push(dir);
+		byArtifactAndAttempt.get(key).dirs.push(dir);
 	}
 
 	const candidates = [];
 
-	for (const [artifactName, dirs] of byArtifact) {
+	for (const { artifactName, attempt, dirs } of byArtifactAndAttempt.values()) {
 		const matched = matchResultDirectories(testName, dirs);
 
 		if (matched.length === 0) {
@@ -95,6 +119,7 @@ function buildEvidenceCandidates(testName, resultDirs) {
 
 		candidates.push({
 			artifactName,
+			attempt,
 			shardIndex: shardIndexFromArtifactName(artifactName),
 			matchedDirs: matched.map((dir) => dir.dirName),
 			hasTrace: matched.some((dir) => dir.hasTrace),
@@ -127,16 +152,8 @@ function pickShardArtifacts(artifacts, profile = loadProfile()) {
 }
 
 function listShardArtifacts(repo, runId, profile) {
-	const artifacts = gh([
-		'api',
-		`repos/${repo}/actions/runs/${runId}/artifacts?per_page=100`,
-		'--paginate',
-		'--jq',
-		'.artifacts[] | tojson',
-	])
-		.split('\n')
-		.filter(Boolean)
-		.map((line) => JSON.parse(line));
+	const { listArtifactsAcrossAttempts } = require('./run-attempts');
+	const artifacts = listArtifactsAcrossAttempts(repo, runId);
 	return pickShardArtifacts(artifacts, profile);
 }
 
@@ -236,29 +253,23 @@ function listRecentRunIds(repo, profile) {
 /**
  * Reads one shard artifact's directory names without keeping the download:
  * only the layout matters here, and trace zips are large.
+ * 
+ * @param {string} repo - Repository in "owner/repo" format
+ * @param {object} artifact - Artifact metadata with {id, name, attempt}
+ * @returns {Array<{dirName: string, artifactName: string, attempt: number, hasTrace: boolean}>}
  */
-function readArtifactResultDirs(repo, runId, artifactName) {
+function readArtifactResultDirs(repo, artifact) {
+	const { downloadArtifactById } = require('./run-attempts');
 	const downloadDir = fs.mkdtempSync(
 		path.join(os.tmpdir(), 'heal-evidence-'),
 	);
 
 	try {
-		execFileSync(
-			'gh',
-			[
-				'run',
-				'download',
-				String(runId),
-				'--repo',
-				repo,
-				'-n',
-				artifactName,
-				'-D',
-				downloadDir,
-			],
-			{ stdio: 'inherit' },
+		downloadArtifactById(repo, artifact.id, downloadDir);
+	} catch (error) {
+		console.log(
+			`::warning::Could not download artifact ${artifact.name} (attempt ${artifact.attempt}, id ${artifact.id}): ${error.message}`,
 		);
-	} catch {
 		fs.rmSync(downloadDir, { recursive: true, force: true });
 		return [];
 	}
@@ -269,7 +280,8 @@ function readArtifactResultDirs(repo, runId, artifactName) {
 			.filter((entry) => entry.isDirectory())
 			.map((entry) => ({
 				dirName: entry.name,
-				artifactName,
+				artifactName: artifact.name,
+				attempt: artifact.attempt,
 				hasTrace: hasTraceInDirectory(
 					path.join(downloadDir, entry.name),
 				),
@@ -286,10 +298,62 @@ function findEvidenceInRun(repo, runId, testName, profile) {
 		return null;
 	}
 
+	// Determine which attempts this test actually failed in by checking job logs
+	const {
+		collectHardFailuresWithAttempts,
+	} = require('./collect-log-hard-failures');
+	const failuresByTest = collectHardFailuresWithAttempts(repo, runId);
+	const failedAttempts = failuresByTest.get(testName) || [];
+
+	if (failedAttempts.length > 0) {
+		console.log(
+			`Test "${testName}" failed in attempt(s): ${failedAttempts.join(', ')}`,
+		);
+	} else {
+		console.log(
+			`::warning::Test "${testName}" not found in job logs; will check all attempts' artifacts`,
+		);
+	}
+
 	const resultDirs = [];
 
+	// Group artifacts by name to identify duplicates across attempts
+	const artifactsByName = new Map();
 	for (const artifact of artifacts) {
-		resultDirs.push(...readArtifactResultDirs(repo, runId, artifact.name));
+		if (!artifactsByName.has(artifact.name)) {
+			artifactsByName.set(artifact.name, []);
+		}
+		artifactsByName.get(artifact.name).push(artifact);
+	}
+
+	// For each artifact name, prioritize attempts where the test failed
+	for (const [name, versions] of artifactsByName) {
+		// Sort: failed attempts first (latest first), then other attempts (latest first)
+		const sorted = [...versions].sort((a, b) => {
+			const aFailed = failedAttempts.includes(a.attempt);
+			const bFailed = failedAttempts.includes(b.attempt);
+
+			if (aFailed && !bFailed) return -1;
+			if (!aFailed && bFailed) return 1;
+
+			// Both failed or both didn't fail: prefer later attempt
+			return b.attempt - a.attempt;
+		});
+
+		for (const artifact of sorted) {
+			const dirs = readArtifactResultDirs(repo, artifact);
+			resultDirs.push(...dirs);
+
+			// If we found evidence for this test in this artifact, we can stop
+			// checking other attempts of this same artifact name
+			const evidence = findEvidenceInResultDirs(testName, dirs);
+			if (evidence) {
+				console.log(
+					`Found evidence in attempt ${artifact.attempt} artifact "${artifact.name}"`,
+				);
+				break;
+			}
+		}
 	}
 
 	const evidence = findEvidenceInResultDirs(testName, resultDirs);

actions/playwright-test-healer/scripts/restore-nightly-playwright-builds.js

@@ -72,39 +72,37 @@ function planBuildRestore(artifacts) {
 }
 
 function listRunArtifacts(repo, runId) {
-	return execFileSync(
-		'gh',
-		[
-			'api',
-			`repos/${repo}/actions/runs/${runId}/artifacts?per_page=100`,
-			'--paginate',
-			'--jq',
-			'.artifacts[] | tojson',
-		],
-		{ encoding: 'utf8', maxBuffer: 64 * 1024 * 1024 },
-	)
-		.split('\n')
-		.filter(Boolean)
-		.map((line) => JSON.parse(line));
+	const { listArtifactsAcrossAttempts } = require('./run-attempts');
+	// Get artifacts from all attempts, but prefer the latest when there are duplicates
+	const allArtifacts = listArtifactsAcrossAttempts(repo, runId);
+
+	// For builds, we typically want the latest attempt's artifacts
+	// Group by name and take the one with the highest attempt number
+	const latestByName = new Map();
+	for (const artifact of allArtifacts) {
+		const existing = latestByName.get(artifact.name);
+		if (!existing || artifact.attempt > existing.attempt) {
+			latestByName.set(artifact.name, artifact);
+		}
+	}
+
+	return Array.from(latestByName.values());
 }
 
 function downloadArtifact(repo, runId, artifactName, destinationDir) {
 	fs.mkdirSync(destinationDir, { recursive: true });
-	execFileSync(
-		'gh',
-		[
-			'run',
-			'download',
-			String(runId),
-			'--repo',
-			repo,
-			'-n',
-			artifactName,
-			'-D',
-			destinationDir,
-		],
-		{ stdio: 'inherit' },
-	);
+
+	// When downloading by name after a re-run, gh may get confused by duplicate names.
+	// Use our helper that downloads by artifact ID instead.
+	const artifacts = listRunArtifacts(repo, runId);
+	const artifact = artifacts.find((a) => a.name === artifactName);
+
+	if (!artifact) {
+		throw new Error(`Artifact ${artifactName} not found in run ${runId}`);
+	}
+
+	const { downloadArtifactById } = require('./run-attempts');
+	downloadArtifactById(repo, artifact.id, destinationDir);
 }
 
 function walkFiles(rootDir, visitor) {