Skip to content
Merged
Show file tree
Hide file tree
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
### Visual evidence stays trusted when you commit the reviewed pr.md

Boatstack now trusts recorded visual evidence by product identity: the manifest stays `PASS` while the product diff is unchanged. Before this change, the mandatory commit of the reviewed `pr.md` moved the head commit and always degraded `PASS` evidence to `NOT_VERIFIED`, so `require` could not reach publication with current screenshots. Any change to product content still makes the evidence stale immediately. The capture commit stays recorded and the PR preview names it in the Visual evidence table.
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,9 @@ func CaptureEvidence(options CaptureEvidenceOptions) (PRVisualEvidenceManifest,
}

// captureProductDiff reproduces the pr-context product-diff fingerprint so a
// captured manifest is trusted (PASS) by resolvePRVisualEvidence: same head
// commit and same product diff.
// captured manifest is trusted (PASS) by resolvePRVisualEvidence: same product
// diff. The head commit is recorded for provenance only — trust is keyed to
// product identity so committing the reviewed pr.md never stales evidence.
func captureProductDiff(repo, base, feature, head string) (headCommit, diffHash string, err error) {
baseCommit, err := resolveBaseCommit(repo, base)
if err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,18 +101,18 @@ func TestCaptureEvidenceProducesManifestTrustedByPRContext(t *testing.T) {
t.Fatalf("capture mutated the product tree: %s", status)
}

// The manifest must be trusted by the same resolver pr-context uses: identical
// head commit and product diff → status is the manifest's PASS, not NOT_VERIFIED.
// The manifest must be trusted by the same resolver pr-context uses: an
// identical product diff → status is the manifest's PASS, not NOT_VERIFIED.
head := runGit(t, repo, "rev-parse", "--abbrev-ref", "HEAD")
headCommit, diffHash, err := captureProductDiff(repo, "main", "reviewer-ready", head)
_, diffHash, err := captureProductDiff(repo, "main", "reviewer-ready", head)
if err != nil {
t.Fatal(err)
}
config, _, err := LoadConfig(filepath.Join(repo, ".product-loop", "project.json"))
if err != nil {
t.Fatal(err)
}
_, status, count, _, _, _, resolved, err := resolvePRVisualEvidence(repo, config, "managed", "reviewer-ready", head, headCommit, diffHash)
_, status, count, _, _, _, resolved, err := resolvePRVisualEvidence(repo, config, "managed", "reviewer-ready", head, diffHash)
if err != nil {
t.Fatal(err)
}
Expand Down
20 changes: 16 additions & 4 deletions labs/12-product-engineering-loop/product-engineering-loop/pr.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func planVisualDecision(repo, feature string) (string, string, []PRVisualScenari
return relevance, "managed-plan", scenarios, nil
}

func resolvePRVisualEvidence(repo string, config ProjectConfig, mode, feature, head, headCommit, diffHash string) (string, string, int, string, string, string, *PRVisualEvidenceManifest, error) {
func resolvePRVisualEvidence(repo string, config ProjectConfig, mode, feature, head, diffHash string) (string, string, int, string, string, string, *PRVisualEvidenceManifest, error) {
policy := normalizedPRVisualEvidencePolicy(config.Workflow.PRVisualEvidence)
relevance, source := "unresolved", "agent-proposed"
var scenarios []PRVisualScenario
Expand All @@ -131,7 +131,12 @@ func resolvePRVisualEvidence(repo string, config ProjectConfig, mode, feature, h
if loadErr == nil {
manifest = &loaded
relevance, source, scenarios = loaded.Relevance, loaded.RelevanceSource, loaded.Scenarios
if loaded.SourceCommit == headCommit && loaded.ProductDiffSHA256 == diffHash {
// Trust is keyed to product identity only: the preview pr.md is
// excluded from the product diff yet must be committed before
// publication, so a head-commit equality would invalidate every
// PASS manifest on that mandatory commit. SourceCommit stays
// recorded for provenance and the evidence comment.
if loaded.ProductDiffSHA256 == diffHash {
status = loaded.Status
} else {
status = "NOT_VERIFIED"
Expand Down Expand Up @@ -653,7 +658,7 @@ func PreparePRContext(options PRContextOptions) (PRContext, error) {
return PRContext{}, err
}
visualPolicy, visualStatus, visualCount, visualFingerprint, visualRelevance, visualSource, visualManifest, err := resolvePRVisualEvidence(
repo, config, mode, options.Feature, head, headCommit, SHA256Bytes(diff),
repo, config, mode, options.Feature, head, SHA256Bytes(diff),
)
if err != nil {
return PRContext{}, err
Expand Down Expand Up @@ -1240,11 +1245,18 @@ func PRPreviewTemplate(context PRContext) string {
"## Rollout and rollback", "", "Describe deployment impact and the smallest safe rollback.", "",
}
if context.PRVisualEvidenceStatus != "NOT_APPLICABLE" {
// The Commit column names the commit the pixels were captured from;
// evidence stays trusted across preview-only commits, so this can
// legitimately trail HeadCommit.
evidenceCommit := context.HeadCommit
if context.PRVisualEvidence != nil && strings.TrimSpace(context.PRVisualEvidence.SourceCommit) != "" {
evidenceCommit = context.PRVisualEvidence.SourceCommit
}
lines = append(lines,
"## Visual evidence", "",
"Screenshots are human-review evidence, not mechanical proof. Public-repository attachments are publicly accessible.", "",
"| Scenario | Viewport | Commit | Result | Publication |", "|---|---|---|---|---|",
"| Describe the approved state | viewport | "+context.HeadCommit+" | `"+context.PRVisualEvidenceStatus+"` | Boatstack evidence comment or manual fallback |", "",
"| Describe the approved state | viewport | "+evidenceCommit+" | `"+context.PRVisualEvidenceStatus+"` | Boatstack evidence comment or manual fallback |", "",
)
}
if context.TotalSlices > 1 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"strconv"
"strings"
"testing"
"time"
)

func runGit(t *testing.T, repo string, arguments ...string) string {
Expand Down Expand Up @@ -648,6 +649,101 @@ func TestRequiredPRVisualEvidenceBlocksPublicationBeforeMutation(t *testing.T) {
}
}

// savePassVisualManifest records PASS evidence for the fixture feature's
// approved scenario, bound to the given source commit and product diff.
func savePassVisualManifest(t *testing.T, repo, feature, sourceCommit, diffHash string) {
t.Helper()
pngPath := filepath.Join(t.TempDir(), "warning.png")
writeTestPNG(t, pngPath)
if _, err := SavePRVisualEvidence(repo, PRVisualEvidenceManifest{
Key: feature, Policy: "suggest", Relevance: "relevant", RelevanceSource: "managed-plan",
Status: "PASS", SourceCommit: sourceCommit, ProductDiffSHA256: diffHash,
Scenarios: []PRVisualScenario{{ID: "warning", Entry: "/onboarding", State: "picker open", Viewport: "1440x900", Expected: []string{"warning visible"}}},
Items: []PRVisualEvidenceItem{{
ScenarioID: "warning", Path: pngPath, Viewport: "1440x900",
CapturedAt: time.Now().UTC().Truncate(time.Second).Format(time.RFC3339),
Status: "captured", PrivacyStatus: "human-reviewed",
}},
Publication: PRVisualPublication{State: "pending"},
}); err != nil {
t.Fatal(err)
}
}

func TestCommittingPreviewNeverInvalidatesPassVisualEvidence(t *testing.T) {
repo := prTestRepoConfigured(t, func(config *ProjectConfig) {
config.Workflow.PRVisualEvidence = "suggest"
})
activateManagedFeature(t, repo, "reviewer-ready")
captureCommit := runGit(t, repo, "rev-parse", "HEAD")
context, err := PreparePRContext(PRContextOptions{Repo: repo, Feature: "reviewer-ready"})
if err != nil {
t.Fatal(err)
}
savePassVisualManifest(t, repo, "reviewer-ready", captureCommit, context.ProductDiffSHA256)
fresh, err := PreparePRContext(PRContextOptions{Repo: repo, Feature: "reviewer-ready"})
if err != nil {
t.Fatal(err)
}
if fresh.PRVisualEvidenceStatus != "PASS" || fresh.PRVisualEvidenceCount != 1 {
t.Fatalf("recorded PASS evidence was not trusted: %#v", fresh)
}
previewPath := writePreview(t, repo, fresh, "Keep evidence trusted across the preview commit", visualEvidenceBody(managedPRBody(), fresh.PRVisualEvidenceStatus))
runGit(t, repo, "add", fresh.PreviewPath)
runGit(t, repo, "commit", "-m", "record exact PR preview")
committed, err := PreparePRContext(PRContextOptions{Repo: repo, Feature: "reviewer-ready"})
if err != nil {
t.Fatal(err)
}
if committed.PRVisualEvidenceStatus != "PASS" {
t.Fatalf("committing the reviewed pr.md invalidated PASS evidence: %s", committed.PRVisualEvidenceStatus)
}
if committed.PRVisualEvidenceFingerprint != fresh.PRVisualEvidenceFingerprint {
t.Fatalf("preview commit changed the visual evidence fingerprint")
}
if _, _, err := CheckPRPreview(repo, previewPath); err != nil {
t.Fatalf("committed preview no longer checks: %v", err)
}
if committed.PRVisualEvidence == nil || committed.PRVisualEvidence.SourceCommit != captureCommit {
t.Fatalf("evidence provenance lost its capture commit: %#v", committed.PRVisualEvidence)
}
if template := PRPreviewTemplate(committed); !strings.Contains(template, captureCommit) {
t.Fatalf("preview template does not name the capture commit")
}
}

func TestProductDiffChangeInvalidatesPassVisualEvidence(t *testing.T) {
repo := prTestRepoConfigured(t, func(config *ProjectConfig) {
config.Workflow.PRVisualEvidence = "suggest"
})
activateManagedFeature(t, repo, "reviewer-ready")
context, err := PreparePRContext(PRContextOptions{Repo: repo, Feature: "reviewer-ready"})
if err != nil {
t.Fatal(err)
}
savePassVisualManifest(t, repo, "reviewer-ready", runGit(t, repo, "rev-parse", "HEAD"), context.ProductDiffSHA256)
config, _, err := LoadConfig(filepath.Join(repo, ".product-loop", "project.json"))
if err != nil {
t.Fatal(err)
}
changedDiff := strings.Repeat("c", 64)
_, status, _, _, _, _, _, err := resolvePRVisualEvidence(repo, config, "managed", "reviewer-ready", context.HeadBranch, changedDiff)
if err != nil {
t.Fatal(err)
}
if status != "NOT_VERIFIED" {
t.Fatalf("product change did not stale the evidence: %s", status)
}
config.Workflow.PRVisualEvidence = "require"
_, status, _, _, _, _, _, err = resolvePRVisualEvidence(repo, config, "managed", "reviewer-ready", context.HeadBranch, changedDiff)
if err != nil {
t.Fatal(err)
}
if status != "BLOCKED" {
t.Fatalf("require did not coerce stale evidence to BLOCKED: %s", status)
}
}

func TestPublishPRRequiresExactConfirmationAndUsesBodyWithoutFrontmatter(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("fake gh fixture uses a POSIX shell; publication behavior is covered by cross-platform pure-Go checks")
Expand Down
Loading