From b90f211fe456e5585c28fbfc4c23a27a9c0bd221 Mon Sep 17 00:00:00 2001 From: bigboateng Date: Thu, 30 Jul 2026 11:19:49 +0100 Subject: [PATCH] fix(boatstack): key visual-evidence trust to product identity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Trusting a manifest only at an identical head commit had no fixpoint: publication requires committing the reviewed pr.md, which moves HEAD while the product diff (which excludes pr.md) is unchanged, so PASS evidence always degraded to NOT_VERIFIED before publish-pr could see it. Trust is now keyed to ProductDiffSHA256 alone; SourceCommit stays recorded for provenance and the preview template names the capture commit instead of assuming HEAD. Invariants pinned by tests: - committing pr.md never invalidates PASS evidence (status and fingerprint stable across the preview commit) - any product-diff change stales evidence immediately, and require coerces it to BLOCKED Disclosure-Reviewed: reviewed — public-safe only, private facet kept out of this commit --- ...visual-evidence-survives-preview-commit.md | 3 + .../product-engineering-loop/capture.go | 5 +- .../product-engineering-loop/capture_test.go | 8 +- .../product-engineering-loop/pr.go | 20 +++- .../product-engineering-loop/pr_test.go | 96 +++++++++++++++++++ 5 files changed, 122 insertions(+), 10 deletions(-) create mode 100644 labs/12-product-engineering-loop/boatstack-distribution/release-notes/2026-07-30-visual-evidence-survives-preview-commit.md diff --git a/labs/12-product-engineering-loop/boatstack-distribution/release-notes/2026-07-30-visual-evidence-survives-preview-commit.md b/labs/12-product-engineering-loop/boatstack-distribution/release-notes/2026-07-30-visual-evidence-survives-preview-commit.md new file mode 100644 index 000000000..b987cf249 --- /dev/null +++ b/labs/12-product-engineering-loop/boatstack-distribution/release-notes/2026-07-30-visual-evidence-survives-preview-commit.md @@ -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. diff --git a/labs/12-product-engineering-loop/product-engineering-loop/capture.go b/labs/12-product-engineering-loop/product-engineering-loop/capture.go index 857203650..2c6414772 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/capture.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/capture.go @@ -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 { diff --git a/labs/12-product-engineering-loop/product-engineering-loop/capture_test.go b/labs/12-product-engineering-loop/product-engineering-loop/capture_test.go index 17ce97207..9b02af440 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/capture_test.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/capture_test.go @@ -101,10 +101,10 @@ 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) } @@ -112,7 +112,7 @@ func TestCaptureEvidenceProducesManifestTrustedByPRContext(t *testing.T) { 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) } diff --git a/labs/12-product-engineering-loop/product-engineering-loop/pr.go b/labs/12-product-engineering-loop/product-engineering-loop/pr.go index 903caf5c4..f08b95ee0 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/pr.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/pr.go @@ -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 @@ -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" @@ -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 @@ -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 { diff --git a/labs/12-product-engineering-loop/product-engineering-loop/pr_test.go b/labs/12-product-engineering-loop/product-engineering-loop/pr_test.go index 6e5f27be9..2d963d463 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/pr_test.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/pr_test.go @@ -9,6 +9,7 @@ import ( "strconv" "strings" "testing" + "time" ) func runGit(t *testing.T, repo string, arguments ...string) string { @@ -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")