Repository navigation
Decide which PR evidence fields are required for each provider #1284
Description
What happened?
#1283 makes every PR evidence field optional, so the CLI stops crashing. Some fields shouldn't be optional. Evidence with no PR URL, no commit SHA or no reviewer name could point at the wrong thing, and today it would be recorded as fine.
Some replies also succeed when they shouldn't. If GitHub's follow-up page of commits or reviews comes back empty, the extra commits or reviews are dropped with no error. The evidence looks complete but isn't.
Expected
We agree a list of required fields for each provider (GitHub, GitLab, Bitbucket, Azure DevOps). Then:
- A missing required field fails with an error that names the field and the PR.
- A list that comes back shorter than the provider said fails, rather than recording part of it.
- Everything else stays optional, as bug: PR attestation crashes when a provider leaves a field empty #1283 sets it.
Steps to reproduce
The odd-reply tests from #1283 show what each field does today.
CLI version
All versions.
Activity
AlexKantor87 commentedon Oct 21, 2026
Two more questions came up while planning #1283.
Approvals with no username. If a provider sends an approval without a name, #1283 records it with an empty username. A policy that checks "the approver isn't the author" would pass it. Two unnamed approvals on the same commit would also merge into one. Should an unnamed approval count?
Entries we can't read. When an item in a list (a commit, a review, a PR) comes back empty, #1283 fails rather than skipping it. Skipping could hide a "changes requested" review. This ticket should confirm that's what we want.
AlexKantor87 commentedon Oct 22, 2026
Two more for this ticket, from review of the #1283 PRs.
Missing dates are sent as 0. A commit with no date is sent with
"timestamp": 0, which the server stores as 1 January 1970, not "unknown". Commit timestamps have noomitempty, unlike the PR's own times. Decide whether a missing date should be left out, or count as required.Approvals with no identity now fail. Earlier I asked whether an unnamed approval should count. For now, #1283 makes it an error on every provider, because counting it would record approval evidence attributed to nobody. This ticket should confirm that or change it.
AlexKantor87 commentedon Oct 22, 2026
Correction to my last comment: approvals with no identity no longer fail.
GitHub sends a deleted account's review with no author. Failing on it would block every attestation for a PR approved by someone who has since left, with no way round it. So #1283 now does this on every provider:
This ticket should still confirm that, alongside the other required-field questions.
AlexKantor87 commentedon Oct 25, 2026
Another correction, after Marko's review on #1287: unnamed approvals no longer stay in the reviews list either.
Policies read
reviewsas well asapprovers. So an approval by nobody inreviewscould still be counted. #1283 now uses the rule Bitbucket already had onmain, on every provider:reviews, it's kept only if it requests changes. Dropping that would read as no objection.This ticket should still confirm that.