fix(tfa): code-enforced PR validation gate for build-level causation #3
+852
−22
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,135 @@ | ||||||
| // Code-enforced PR validation gate (Fix A). | ||||||
| // | ||||||
| // Every coordinator's `related_prs` exits through `loop.mjs` `out()`, which | ||||||
| // calls `validateAndDeduplicatePRs` here to cross-check each claimed PR | ||||||
| // against the evidence file's ground truth. A PR that fails validation is | ||||||
| // downgraded to `verdict: "ruled-out (<reason>)"` — never silently dropped. | ||||||
|
|
||||||
| /** | ||||||
| * Extract owner/repo from a PR entry's link or url field. | ||||||
| * Handles GitHub PR URLs like "https://github.com/owner/repo/pull/123". | ||||||
| */ | ||||||
| function extractOwnerRepo(entry) { | ||||||
| const raw = entry?.link || entry?.url || ""; | ||||||
| const m = String(raw).match(/github\.com\/([^/]+\/[^/]+)\/pull\//); | ||||||
| return m ? m[1] : null; | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Build the lookup key for a PR: "owner/repo" string. | ||||||
| * Prefers explicit owner+repo fields; falls back to parsing link/url. | ||||||
| */ | ||||||
| function resolveRepo(entry) { | ||||||
| if (entry?.owner && entry?.repo) return `${entry.owner}/${entry.repo}`; | ||||||
| return extractOwnerRepo(entry); | ||||||
| } | ||||||
|
|
||||||
| /** Strip a leading "#" so "#123" and 123 compare equal. */ | ||||||
| function normalise(n) { | ||||||
| return String(n).replace(/^#/, ""); | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Validate a single suspect PR entry against the evidence file. | ||||||
| * | ||||||
| * @param {object} entry - PR entry with owner, repo, number, merged_at, link/url | ||||||
| * @param {object} evidenceDoc - folded evidence file from readEvidenceFile | ||||||
| * @returns {{valid: boolean, reason?: "not-in-evidence" | "shipped-after" | "wrong-base-branch"}} | ||||||
| */ | ||||||
| export function validateSuspectPR(entry, evidenceDoc) { | ||||||
| const repoKey = resolveRepo(entry); | ||||||
| if (!repoKey) return { valid: false, reason: "not-in-evidence" }; | ||||||
|
|
||||||
| const repoEvidence = evidenceDoc?.github?.[repoKey]; | ||||||
| const prsInWindow = repoEvidence?.prsInWindow ?? []; | ||||||
|
|
||||||
| const num = entry?.number ?? entry?.pr; | ||||||
| if (num == null) return { valid: false, reason: "not-in-evidence" }; | ||||||
|
|
||||||
| const match = prsInWindow.find( | ||||||
| (p) => normalise(p.pr) === normalise(num), | ||||||
| ); | ||||||
| if (!match) return { valid: false, reason: "not-in-evidence" }; | ||||||
|
Author
There was a problem hiding this comment. [Medium] This same arrow function is also defined at line 90. Extract to module scope.
Suggested change
Reviewer: fallback |
||||||
|
|
||||||
| // Branch check: the PR's ground-truth baseRefName must match the build's | ||||||
| // ACTUAL working branch (resolved once at Step 4 from fetchBuildInsights) — | ||||||
| // never assumed to be main/master. A PR merged into the wrong branch could | ||||||
| // not have shipped to this build regardless of timing. | ||||||
| const workingBranch = repoEvidence?.workingBranch; | ||||||
| if (workingBranch && match.baseRefName && match.baseRefName !== workingBranch) { | ||||||
| return { valid: false, reason: "wrong-base-branch" }; | ||||||
| } | ||||||
|
|
||||||
| // Merge-window check: merged_at must not be after the build's started_at. | ||||||
| const startedAt = evidenceDoc?.suspectWindow?.startedAt; | ||||||
| if (startedAt) { | ||||||
| const mergedMs = Date.parse(entry.merged_at ?? match.mergedAt ?? ""); | ||||||
| const startedMs = Date.parse(startedAt); | ||||||
| if (!Number.isNaN(mergedMs) && !Number.isNaN(startedMs) && mergedMs > startedMs) { | ||||||
| return { valid: false, reason: "shipped-after" }; | ||||||
| } | ||||||
| } else { | ||||||
| // Cannot falsify on window — log but allow. | ||||||
| console.warn(`[pr-validation] suspectWindow.startedAt absent — skipping merge-window check`); | ||||||
| } | ||||||
|
|
||||||
| return { valid: true }; | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Deduplicate and validate an array of suspect PR entries against the evidence file. | ||||||
| * | ||||||
| * - Deduplicates by (owner, repo, number). | ||||||
| * - Enriches each entry with ground-truth fields from the evidence file. | ||||||
| * - Valid entries keep their existing verdict. | ||||||
| * - Invalid entries are downgraded to `verdict: "ruled-out (<reason>)"`. | ||||||
| * - All entries (valid + ruled-out) are returned — nothing is silently dropped. | ||||||
| * | ||||||
| * @param {Array} relatedPrs - the TFA agent's claimed suspect PRs | ||||||
| * @param {object} evidenceDoc - folded evidence file from readEvidenceFile | ||||||
| * @returns {Array} deduplicated, validated PR entries | ||||||
| */ | ||||||
| export function validateAndDeduplicatePRs(relatedPrs, evidenceDoc) { | ||||||
| if (!Array.isArray(relatedPrs) || relatedPrs.length === 0) return []; | ||||||
|
|
||||||
| // Deduplicate by (repo, number). | ||||||
|
Author
There was a problem hiding this comment. [Medium] Dedup keeps first entry, silently drops fields from later duplicates The dedup loop keeps the first entry for a given repo+number key. If a later duplicate carries fields the first lacks (e.g. Suggestion: Merge duplicates using a field-level merge (similar to Reviewer: fallback |
||||||
| const seen = new Map(); | ||||||
| for (const entry of relatedPrs) { | ||||||
| const repoKey = resolveRepo(entry); | ||||||
| const num = entry?.number ?? entry?.pr; | ||||||
| const dedup = repoKey && num != null | ||||||
| ? `${repoKey}#${String(num).replace(/^#/, "")}` | ||||||
| : `anon-${seen.size}`; | ||||||
| if (!seen.has(dedup)) seen.set(dedup, entry); | ||||||
| } | ||||||
|
|
||||||
| const results = []; | ||||||
| for (const entry of seen.values()) { | ||||||
| const repoKey = resolveRepo(entry); | ||||||
| const num = entry?.number ?? entry?.pr; | ||||||
|
Author
There was a problem hiding this comment. [Low] Ground-truth enrichment can silently overwrite verdict This spreads all non-null evidence fields over the entry. Currently safe only because evidence PRs lack a Suggestion: Reviewer: fallback |
||||||
|
|
||||||
| // Enrich from evidence ground truth when possible. | ||||||
| const repoEvidence = repoKey ? evidenceDoc?.github?.[repoKey] : null; | ||||||
| const groundTruth = (repoEvidence?.prsInWindow ?? []).find( | ||||||
| (p) => num != null && normalise(p.pr) === normalise(num), | ||||||
| ); | ||||||
|
|
||||||
| // Build the output entry: ground-truth fields override LLM transcription. | ||||||
| // Exclude `verdict` explicitly — a ground-truth PR entry has no business | ||||||
| // setting this coordinator-owned field, even if one ever appeared there. | ||||||
| const enriched = groundTruth | ||||||
| ? { ...entry, ...Object.fromEntries(Object.entries(groundTruth).filter(([k, v]) => v != null && k !== "verdict")) } | ||||||
| : { ...entry }; | ||||||
|
|
||||||
| const { valid, reason } = validateSuspectPR(entry, evidenceDoc); | ||||||
| if (!valid) { | ||||||
| enriched.verdict = `ruled-out (${reason})`; | ||||||
| console.warn(`[pr-validation] PR ${repoKey ?? "?"}#${num ?? "?"} downgraded: ${reason}`); | ||||||
| } | ||||||
| // Valid entries keep their existing verdict (typically "supported"). | ||||||
|
|
||||||
| results.push(enriched); | ||||||
| } | ||||||
|
|
||||||
| return results; | ||||||
| } | ||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Low] Validation only runs for RESOLVED status
A FAILED or PENDING result with hallucinated
related_prsskips validation. Likely intentional but worth documenting the design choice.Reviewer: fallback