diff --git a/labs/12-product-engineering-loop/boatstack-distribution/release-notes/2026-08-02-strengthen-screenshot-delivery.md b/labs/12-product-engineering-loop/boatstack-distribution/release-notes/2026-08-02-strengthen-screenshot-delivery.md new file mode 100644 index 00000000..e9d182ac --- /dev/null +++ b/labs/12-product-engineering-loop/boatstack-distribution/release-notes/2026-08-02-strengthen-screenshot-delivery.md @@ -0,0 +1,3 @@ +### Screenshot delivery is now privacy-gated and externally hosted + +Boatstack now treats registered visual-surface changes as screenshot candidates, binds capture reuse to the product diff, scenario, and command, accepts optional scenario receipts, and publishes only verified hosted links after human privacy review. Failed uploads or comments preserve the existing pull request for an idempotent retry. diff --git a/labs/12-product-engineering-loop/product-engineering-loop/SKILL.md b/labs/12-product-engineering-loop/product-engineering-loop/SKILL.md index 4e0e4ccf..7f5a613a 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/SKILL.md +++ b/labs/12-product-engineering-loop/product-engineering-loop/SKILL.md @@ -106,7 +106,7 @@ Before starting `/auto-plan` for a new feature, check `next-status --repo . --js 3. Separate facts, decisions, unknowns, and safely deferrable gaps. 4. Before proposing implementation tasks, inspect the repository and verify any assumptions about API routes, data access, UI components, authentication, server actions, streams, jobs, and external services. Do not guess application architecture. 5. If `workflow.boundary_analysis` is `true` in `project.json`: Evaluate if the requested change is a symptom of a missing systemic boundary (e.g., deficient data normalization, leaky validation, missing authorization edge). If it is, perform a rapid codebase scan for other vulnerabilities sharing this failure mode. Present this as a material product decision, showing concrete codebase evidence of the blast radius. Offer tiered implementation paths: [1a] Symptom Patch (fix only the requested route), or [1b] Programmatic Enforcement (refactor the edge and install a programmatic boundary to mathematically prevent this). If the user chooses programmatic enforcement, explicitly structure the plan into two delivery slices: Slice 1 establishes the programmatic boundary (hook, trigger, or strict test), and Slice 2 implements the feature using that boundary. -- When `workflow.pr_visual_evidence` is `suggest` or `require`, also record a structural `pr_visual_evidence` decision. Use `relevant` with one to three scenarios naming entry, state, viewport, and expected visible outcomes, or `not_relevant` with a reason. Discover repository-owned visual tooling but do not add or require framework-specific tooling. +- When `workflow.pr_visual_evidence` is `suggest` or `require`, also record one structural `pr_visual_evidence` decision reused through test, review, and ship. Changes below `project.visual_surfaces[].paths` are relevant. Use one to three scenarios naming user context, user goal, journey step, reviewer context, entry, state, viewport, surface, and expected visible outcomes, or `not_relevant` with a non-empty reason for review. Discover repository-owned visual tooling but do not add or require framework-specific tooling. 6. Express verified architectural information as typed `architecture_facts`. Each architecture fact must reference evidence IDs produced by Boatstack repository inspection. Do not create or invent evidence IDs. Reading one arbitrary repository file does not ground an unrelated architectural claim. 7. When an architectural question cannot be verified, record it in `architecture_unknowns`. Do not create an implementation task that depends on an unresolved architecture unknown. Create a bounded discovery task instead. 8. Every architecture-sensitive task must reference the facts it depends on through `requires_facts`. @@ -190,7 +190,7 @@ A published delivery is immutable. Record the append-only observation without ch - Derive tests from acceptance criteria and affected contracts, not only from the implementation. - Run existing relevant tests plus targeted new tests, linters, type checks, builds, and runtime checks. - When `journey_evidence` is relevant, run every compiled oracle and import typed results with `record-journey-results --feature --results `. Test and review gates reject missing, failed, manifest-mismatched, head-mismatched, or diff-stale results. -- For relevant PR visual scenarios, use the repository runner first, then a host browser against the existing development server, one supplied launch instruction, or an explicitly approved machine-local runtime. Do not modify repository dependencies or configuration for capture. Review each exact PNG for secrets and private data, then import the temporary manifest with `record-pr-visual-evidence`; keep the images outside the repository. +- For relevant PR visual scenarios, use the repository runner first, then a host browser against the existing development server, one supplied launch instruction, or an explicitly approved machine-local runtime. A harness may write `BOATSTACK_CAPTURE_RECEIPT` with scenario id, reached state or URL, named check results, and overall result; without it the PNG is only `CAPTURED`, never scenario-verified. Do not modify repository dependencies or configuration for capture. A human must review each exact PNG for secrets and private data and record `human-reviewed` before any external upload; keep the images outside the repository. - Treat model-authored tests and same-model self-review as evidence, not ground truth. - Validate that tests load and exercise the intended interface. For high-risk code, add an independent oracle such as contract fixtures, mutation testing, differential checks, staging verification, or human acceptance. - A failing check blocks the gate. A skipped check must include a reason and risk owner. `PASS_WITH_GAPS` is accepted only when `workflow.allow_pass_with_gaps` is true. @@ -211,7 +211,7 @@ A published delivery is immutable. Record the append-only observation without ch - Treat the actual committed diff as what changed, approved artifacts as why it changed, and evidence as the only support for completion claims. - In the visible Evidence table, link each managed claim to the current repository-relative evidence ledger using a readable link label; do not expose hashes or absolute paths. - Always include why, what changed, review order, evidence, gaps/risks, rollout/rollback, and collapsed provenance. Add UI evidence, security/privacy, migration, or operations sections only when relevant. -- When PR visual evidence is relevant or unresolved, show the exact fingerprinted PNGs and public-repository warning, include the structural Visual evidence table, and treat `o` or `u` as approval of the PR body plus one evidence comment. Prefer a signed-in host browser; after observing the upload, record its PR and comment URLs with `record-pr-visual-publication`. Otherwise surface the exact machine-local paths for manual attachment. `suggest` retains a visible gap, while `require` blocks completed publication. Preserve an opened PR and fix forward from `visual_pending` rather than opening a duplicate. +- When PR visual evidence is relevant or unresolved, show the exact fingerprinted PNGs and external-host privacy warning, include the structural Visual evidence table, and treat `o` or `u` as approval of the PR body plus one evidence comment. Boatstack defaults to Litterbox with a 72-hour expiry, verifies every hosted URL, and puts only hosted Markdown image links in the comment. Never attach image files to the PR or commit an evidence branch. `suggest` retains a visible gap, while `require` blocks completed publication. Preserve an opened PR and retry the same fingerprint and comment from `visual_pending` rather than opening a duplicate. - Internally generate the normalized context and preview skeleton with `pr-context --repo . --feature `, write `pr.md`, and validate it with `check-pr --repo . --preview `. Keep these helper names and their fingerprints out of the primary response. - Inspect the projected changed files, diff stat, high-risk matches, and actual diff before composing the brief. Commit messages are navigation aids, not proof of what changed. - Show the exact title and rendered body before any GitHub mutation. If no PR exists, render the one next action as: Reply `o` to open PR. If one exists, render: Reply `u` to update PR. 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 8c7b5185..e8242a3a 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/capture.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/capture.go @@ -19,11 +19,12 @@ const captureMaxAttempts = 3 // writing exactly one PNG to OutputPath. The contract is surfaced to the harness // as environment variables (see execCaptureRunner). type CaptureRequest struct { - Repo string - Capability string - Command string - Scenario PRVisualScenario - OutputPath string + Repo string + Capability string + Command string + Scenario PRVisualScenario + OutputPath string + ReceiptPath string } // CaptureRunner runs one scenario's repository capture command. It must produce @@ -49,6 +50,7 @@ func (execCaptureRunner) Run(request CaptureRequest) error { "BOATSTACK_CAPTURE_VIEWPORT="+request.Scenario.Viewport, "BOATSTACK_CAPTURE_SURFACE="+request.Scenario.Surface, "BOATSTACK_CAPTURE_OUTPUT="+request.OutputPath, + "BOATSTACK_CAPTURE_RECEIPT="+request.ReceiptPath, ) // The harness's authoritative output is the PNG on disk, not stdout; only // stderr is retained, as bounded diagnostics for a failed capture. @@ -147,33 +149,53 @@ func CaptureEvidence(options CaptureEvidenceOptions) (PRVisualEvidenceManifest, items := make([]PRVisualEvidenceItem, 0, len(scenarios)) for _, scenario := range scenarios { outputPath := filepath.Join(stagingDir, scenario.ID+".png") - if err := captureScenario(repo, capability, commands[scenario.ID], scenario, outputPath, feature, head, headCommit, diffHash, runner); err != nil { + receiptPath := filepath.Join(stagingDir, scenario.ID+".receipt.json") + if err := captureScenario(repo, capability, commands[scenario.ID], scenario, outputPath, receiptPath, feature, head, headCommit, diffHash, runner); err != nil { return PRVisualEvidenceManifest{}, err } + verificationStatus := "CAPTURED" + var receipt *PRVisualScenarioReceipt + if parsed, receiptErr := loadVisualScenarioReceipt(receiptPath, scenario.ID); receiptErr == nil { + receipt, verificationStatus = parsed, "SCENARIO_VERIFIED" + } else if !os.IsNotExist(receiptErr) { + return PRVisualEvidenceManifest{}, receiptErr + } items = append(items, PRVisualEvidenceItem{ - ScenarioID: scenario.ID, - Path: outputPath, - MIMEType: "image/png", - Viewport: scenario.Viewport, - CapturedAt: time.Now().UTC().Truncate(time.Second).Format(time.RFC3339), - Status: "captured", - PrivacyStatus: "clean", + ScenarioID: scenario.ID, + Path: outputPath, + MIMEType: "image/png", + Viewport: scenario.Viewport, + CapturedAt: time.Now().UTC().Truncate(time.Second).Format(time.RFC3339), + Status: "CAPTURED", + PrivacyStatus: "clean", + VerificationStatus: verificationStatus, + Receipt: receipt, }) } + scenarioRaw, err := MarshalJSON(scenarios) + if err != nil { + return PRVisualEvidenceManifest{}, err + } + commandRaw, err := MarshalJSON(commands) + if err != nil { + return PRVisualEvidenceManifest{}, err + } manifest := PRVisualEvidenceManifest{ - Key: key, + Key: key, // The manifest records the configured policy verbatim (informational); // the effective policy — including plan-escalated require semantics — // is re-derived by resolvePRVisualEvidence at every decode. - Policy: config.Workflow.PRVisualEvidence, - Relevance: relevance, - RelevanceSource: source, - Status: "PASS", - SourceCommit: headCommit, - ProductDiffSHA256: diffHash, - Scenarios: scenarios, - Items: items, + Policy: config.Workflow.PRVisualEvidence, + Relevance: relevance, + RelevanceSource: source, + Status: "PASS", + SourceCommit: headCommit, + ProductDiffSHA256: diffHash, + ScenarioDefinitionSHA256: SHA256Bytes(scenarioRaw), + CaptureCommandSHA256: SHA256Bytes(commandRaw), + Scenarios: scenarios, + Items: items, } saved, err := SavePRVisualEvidence(repo, manifest) if err != nil { @@ -248,9 +270,13 @@ func captureStagingDirectory(repo, key string) (string, error) { // retry budget. The fingerprint is stable for a given command, scenario, and // product diff, so a successful capture on the same commit is reused rather than // re-run. -func captureScenario(repo string, capability Capability, command string, scenario PRVisualScenario, outputPath, feature, head, headCommit, diffHash string, runner CaptureRunner) error { +func captureScenario(repo string, capability Capability, command string, scenario PRVisualScenario, outputPath, receiptPath, feature, head, headCommit, diffHash string, runner CaptureRunner) error { + scenarioRaw, err := MarshalJSON(scenario) + if err != nil { + return err + } fingerprint := SHA256Bytes([]byte(strings.Join([]string{ - command, scenario.ID, scenario.Viewport, scenario.Entry, scenario.State, headCommit, diffHash, + command, string(scenarioRaw), diffHash, }, "\x00"))) kind := "capture:" + capability.Name postcondition := fmt.Sprintf("valid PNG captured for scenario %s (%s)", scenario.ID, scenario.Viewport) @@ -288,8 +314,13 @@ func captureScenario(repo string, capability Capability, command string, scenari } return fmt.Errorf("capture of %s reports success but its artifact is missing", scenario.ID) } + // A new package fingerprint must not inherit an optional receipt left by + // an older harness run. PNG-only remains CAPTURED. + if err := os.Remove(receiptPath); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("clear stale receipt for %s: %w", scenario.ID, err) + } runErr := runner.Run(CaptureRequest{ - Repo: repo, Capability: capability.Name, Command: command, Scenario: scenario, OutputPath: outputPath, + Repo: repo, Capability: capability.Name, Command: command, Scenario: scenario, OutputPath: outputPath, ReceiptPath: receiptPath, }) if runErr == nil { runErr = verifyCapturedPNG(outputPath) @@ -312,6 +343,26 @@ func captureScenario(repo string, capability Capability, command string, scenari return fmt.Errorf("capture of scenario %s failed after %d attempts: %w", scenario.ID, captureMaxAttempts, lastErr) } +func loadVisualScenarioReceipt(path, scenarioID string) (*PRVisualScenarioReceipt, error) { + raw, err := os.ReadFile(path) + if err != nil { + return nil, err + } + var receipt PRVisualScenarioReceipt + if err := DecodeJSON("visual scenario receipt", path, raw, &receipt); err != nil { + return nil, err + } + if receipt.ScenarioID != scenarioID || strings.TrimSpace(receipt.Reached) == "" || len(receipt.Checks) == 0 || !strings.EqualFold(receipt.OverallResult, "PASS") { + return nil, fmt.Errorf("visual scenario receipt for %s is invalid or failing", scenarioID) + } + for _, check := range receipt.Checks { + if strings.TrimSpace(check.Name) == "" || !strings.EqualFold(check.Result, "PASS") { + return nil, fmt.Errorf("visual scenario receipt for %s contains an unnamed or failing check", scenarioID) + } + } + return &receipt, nil +} + func verifyCapturedPNG(path string) error { info, err := os.Lstat(path) if err != nil || !info.Mode().IsRegular() || info.Mode()&os.ModeSymlink != 0 { 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 922b0927..d856d27f 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 @@ -133,6 +133,49 @@ func TestCaptureEvidenceProducesManifestTrustedByPRContext(t *testing.T) { } } +// control-law: scenario-verification-requires-a-current-receipt +func TestCaptureEvidenceDoesNotReuseStaleOptionalReceipt(t *testing.T) { + repo := captureTestRepo(t, "reviewer-ready") + withReceipt := true + runner := &stubCaptureRunner{write: func(request CaptureRequest) error { + writeTestPNG(t, request.OutputPath) + if withReceipt { + return os.WriteFile(request.ReceiptPath, []byte(`{"scenario_id":"warning","reached_state_or_url":"/onboarding","checks":[{"name":"warning visible","result":"PASS"}],"overall_result":"PASS"}`), 0o600) + } + return nil + }} + first, err := CaptureEvidence(CaptureEvidenceOptions{Repo: repo, Capability: "visual", Feature: "reviewer-ready", Runner: runner}) + if err != nil { + t.Fatal(err) + } + if first.Items[0].VerificationStatus != "SCENARIO_VERIFIED" { + t.Fatalf("valid receipt was not verified: %#v", first.Items[0]) + } + + config, _, err := LoadConfig(filepath.Join(repo, ".product-loop", "project.json")) + if err != nil { + t.Fatal(err) + } + config.Project.Commands["visual"] = "changed-capture-command" + raw, err := MarshalJSON(config) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(repo, ".product-loop", "project.json"), raw, 0o644); err != nil { + t.Fatal(err) + } + runGit(t, repo, "add", ".product-loop/project.json") + runGit(t, repo, "commit", "-m", "change capture command") + withReceipt = false + second, err := CaptureEvidence(CaptureEvidenceOptions{Repo: repo, Capability: "visual", Feature: "reviewer-ready", Runner: runner}) + if err != nil { + t.Fatal(err) + } + if second.Items[0].VerificationStatus != "CAPTURED" || second.Items[0].Receipt != nil { + t.Fatalf("PNG-only rerun inherited a stale receipt: %#v", second.Items[0]) + } +} + func TestCaptureEvidenceFailsClosedOnNonConformantOutput(t *testing.T) { repo := captureTestRepo(t, "reviewer-ready") runner := &stubCaptureRunner{write: func(request CaptureRequest) error { diff --git a/labs/12-product-engineering-loop/product-engineering-loop/export.go b/labs/12-product-engineering-loop/product-engineering-loop/export.go index af2f3114..6ce4d38d 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/export.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/export.go @@ -149,6 +149,16 @@ func ValidateConfig(config ProjectConfig) error { if err := validateVisualEvidencePublish(config.Workflow.VisualEvidencePublish); err != nil { return err } + for _, surface := range config.Project.VisualSurfaces { + if !surfaceSlugPattern.MatchString(surface.ID) || len(surface.Paths) == 0 { + return fmt.Errorf("project.visual_surfaces require a lowercase-kebab id and at least one path") + } + for _, pattern := range surface.Paths { + if strings.TrimSpace(pattern) == "" || filepath.IsAbs(pattern) || strings.Contains(filepath.ToSlash(pattern), "../") { + return fmt.Errorf("project.visual_surfaces paths must be non-empty repository-relative patterns") + } + } + } return nil } @@ -352,7 +362,7 @@ func BuildExportBundle(configPath string, config ProjectConfig, rawConfig []byte "repair": "First run recovery-status --repo . with the user's exact free-form requested change, its observed source stage, bounded evidence when available, and --json. This resolver covers both active and current-branch published deliveries. On repair_active, read delivery-status, the current plan lock and acceptance criteria, the actual diff, and current receipts; classify the request and invoke record-change before any product edit. On draft_corrective_child, invoke record-change on the published parent, preserve its lock, receipts, slices, and publication evidence, and automatically prepare the suggested one-slice child plan with parent_delivery, exact correction, inherited intent, observed failure, returned existing_diff_sha256 and existing_changed_paths, verification requirements, and the resolved PR destination. Lead with The PR needs a corrective delivery. I prepared it for your approval. Then pause at the normal fingerprinted plan approval boundary; never reuse the parent's approval. An open PR reuses its verified head branch and is updated after fresh gates and publication confirmation. A merged or closed PR uses a fresh branch and PR; when a fingerprinted correction diff already exists, leave the original worktree untouched and transfer that exact reviewed diff into the fresh child only after approval. PUBLISHED_UNKNOWN may be drafted but its destination remains blocking at publication. Stop on BLOCKED and ask one targeted feature question using the returned blockers. If no managed target exists, continue ordinary conversation. Never discard pre-existing correction edits, edit runtime state directly, or bypass test, review, and ship gates. Never ask the user to repeat a denied push or PR mutation. If Cursor reports MainThreadShellExec not initialized, make Developer: Reload Window the one recovery action because Boatstack's hook did not start; reserve reinstall guidance for Boatstack runtime integrity errors.", "test-gate": "Read delivery-status and test only the active delivery slice. Run the internal repository safety check, build a requirement-to-evidence matrix, and treat self-authored tests as evidence rather than the sole oracle. If the active slice contains a systemic_boundary task, the evidence must prove the verification_oracle actively blocked or normalized a violation attempt (negative test). External writes require immutable target identity, transactional or fix-forward failure behavior, and an independent safety oracle. For relevant PR visual scenarios, use repository-owned capture first, then the host browser against the existing development server, one supplied launch instruction, or an approved machine-only runtime. Do not edit repository dependencies or configuration for capture. Review the exact PNGs for secrets and private data and import their temporary manifest with record-pr-visual-evidence. Commit the intentional slice product and evidence diff, then record-delivery-gate for the active feature and slice with --gate test and PASS or PASS_WITH_GAPS. Editing evidence Markdown alone never passes the gate. On pass respond Tests passed and make Run /review-gate the one next action. On failure respond Testing found a problem and make the required non-destructive repair the one next action.", "review-gate": "Read delivery-status and review the active slice's actual diff against authorized intent, invariants, risks, gaps, and test evidence. Run the internal repository safety check. Executable destructive capability is blocking even when ordinary tests pass. When workflow.maintain_changelog is true, verify the new CHANGELOG.md Unreleased entry accurately describes the actual reader-visible impact. When workflow.independent_review_for_high_risk is true and changed paths match project.high_risk_paths, use a human peer or separate agent and pass --reviewer-identity plus --review-method human_peer or separate_agent. On pass invoke record-delivery-gate for the same feature and slice with --gate review; it must reject changed or untested diffs, disallowed gaps, missing reviewer provenance, and malformed required changelog evidence. Then respond Review passed and make Run /ship-gate the one next action. When blocked respond Changes required and make the highest-priority blocking repair the one next action.", - "ship-gate": "Prepare a reviewer-ready PR only; do not merge or deploy without separate authorization. Require the current managed feature approval, lock, test evidence, review evidence, and a passing repository safety scan, and commit the intentional product/artifact diff before projection. Internally run pr-context --repo . --feature in json and template formats, project the approved intent, actual committed diff, decisions, evidence, gaps, rollout, rollback, safety outcome, and operator-only recovery boundary into its required pr.md path, then run check-pr --repo . --preview . Generate a clear, product-focused PR title that describes the user value or system outcome rather than listing technical components (do not use sequence prefixes like 'PR 1'). Always include why, what changed, review order, evidence, gaps/risks, rollout/rollback, and collapsed provenance. When PR visual evidence is relevant, pr-context runs the registered repository capture command itself whenever evidence is missing or stale, so capturing is not your step: review the exact fingerprinted local PNGs for secrets and private data, show the public-repository warning, render the structural Visual evidence section, and treat o or u as authorization for the exact PR package plus one Boatstack-owned evidence comment. Fall back to manual capture (host browser, capture-evidence, record-pr-visual-evidence) only when the context reports the capture capability unavailable or names a harness failure in pr_visual_evidence_capture_detail. Use a signed-in host browser to upload or update that comment when available and record the observed PR and comment URLs with record-pr-visual-publication; otherwise expose the local paths for manual attachment. Suggest records a visible gap; require blocks completed publication. Preserve an opened PR and fix forward from visual_pending after attachment failure. Add security/privacy, migration, or operations sections only when relevant. Show the exact title and rendered body before any GitHub mutation. If PR_ACTION is open, respond PR ready and render the one next action as: Reply `o` to open PR. If update, render: Reply `u` to update PR. If manual, preserve the preview and give one manual publication action. Continue accepting the full replies open PR and update PR for compatibility without advertising them. Only after the matching state-scoped shortcut or compatible full reply: commit only the reviewed pr.md, rerun check-pr and require the same preview fingerprint (PREVIEW_FINGERPRINT), then run publish-pr with --action open or update and that fingerprint. The publisher performs a non-force push and rechecks context before GitHub mutation. If the diff or evidence changes, regenerate instead. If a required check fails on the base branch too, record the evidence and recommend a separate repair PR. Never edit unrelated code in this approved feature branch; a policy-approved bypass requires explicit human authorization. After publication respond PR opened with the link and make Review the PR the one next action; never imply merge authorization. If publish-pr returns UPDATE_AVAILABLE, keep Review the PR as the only next action and append a collapsed update notice saying no files changed and /boatstack-update may be run from the clean default branch after this feature PR merges. Do not check for releases before successful publication.", + "ship-gate": "Prepare a reviewer-ready PR only; do not merge or deploy without separate authorization. Require the current managed feature approval, lock, test evidence, review evidence, and a passing repository safety scan, and commit the intentional product/artifact diff before projection. Internally run pr-context --repo . --feature in json and template formats, project the approved intent, actual committed diff, decisions, evidence, gaps, rollout, rollback, safety outcome, and operator-only recovery boundary into its required pr.md path, then run check-pr --repo . --preview . Generate a clear, product-focused PR title that describes the user value or system outcome rather than listing technical components (do not use sequence prefixes like 'PR 1'). Always include why, what changed, review order, evidence, gaps/risks, rollout/rollback, and collapsed provenance. When PR visual evidence is relevant, pr-context runs the registered repository capture command itself whenever evidence is missing or stale, so capturing is not your step: review the exact fingerprinted local PNGs for secrets and private data, show the external-host privacy warning, render the structural Visual evidence section, and treat o or u as authorization for the exact PR package plus one Boatstack-owned evidence comment. Fall back to manual capture (host browser, capture-evidence, record-pr-visual-evidence) only when the context reports the capture capability unavailable or names a harness failure in pr_visual_evidence_capture_detail. Require human privacy review, then let Boatstack upload through external hosting (Litterbox for 72 hours by default), verify every returned URL, and place only hosted Markdown image links in the existing evidence comment; never attach PNG files directly or commit them to an evidence branch. Suggest records a visible gap; require blocks completed publication. Preserve an opened PR and retry the same fingerprint and comment from visual_pending after upload, URL-verification, or comment failure. Add security/privacy, migration, or operations sections only when relevant. Show the exact title and rendered body before any GitHub mutation. If PR_ACTION is open, respond PR ready and render the one next action as: Reply `o` to open PR. If update, render: Reply `u` to update PR. If manual, preserve the preview and give one manual publication action. Continue accepting the full replies open PR and update PR for compatibility without advertising them. Only after the matching state-scoped shortcut or compatible full reply: commit only the reviewed pr.md, rerun check-pr and require the same preview fingerprint (PREVIEW_FINGERPRINT), then run publish-pr with --action open or update and that fingerprint. The publisher performs a non-force push and rechecks context before GitHub mutation. If the diff or evidence changes, regenerate instead. If a required check fails on the base branch too, record the evidence and recommend a separate repair PR. Never edit unrelated code in this approved feature branch; a policy-approved bypass requires explicit human authorization. After publication respond PR opened with the link and make Review the PR the one next action; never imply merge authorization. If publish-pr returns UPDATE_AVAILABLE, keep Review the PR as the only next action and append a collapsed update notice saying no files changed and /boatstack-update may be run from the clean default branch after this feature PR merges. Do not check for releases before successful publication.", "boatstack-update": "Prepare a visible Boatstack infrastructure update; never mix it into product work or merge it. Force check-update with the current helper when available, but treat it and doctor as diagnostics rather than repair authority. If the installed helper is unavailable, resolve the latest stable tag from the official GitHub release endpoint and continue with the checksum-verified target installer; never require the broken helper to fix itself. If current, respond Boatstack is current with No action required. Before mutation fetch the default ref, then require the current default branch whose HEAD equals origin/ and no product or user-owned edits; otherwise respond Update postponed and give one recovery action. Ensure no update PR or branch already exists and create chore/update-boatstack-v. Fetch the installer from that exact release tag: it must checksum-verify the target helper before consulting installed state. Run it with BOATSTACK_MODE=update, BOATSTACK_VERSION=, BOATSTACK_REPO=, and BOATSTACK_YES=1. Exact installed hook and generated-state migrations are automatic. If the verified target helper reports REPAIR_AVAILABLE, show repair-status, state that the repair remains in this update PR, and ask whether the user wants the exact update rerun with BOATSTACK_REPAIR=1; do not infer that authority from the update request. Never offer repair for user-owned, mixed, malformed, symlinked, product, network, or authentication failures. Downgrades additionally require separately requested BOATSTACK_ALLOW_DOWNGRADE=1. The verified update preserves configuration, adapters, integrations, and unrelated host settings, writes any repair backup to Git-common state, runs doctor, and touches only Boatstack infrastructure. After installation use prepare-update-pr --repo . --version --json. Show version and repair provenance, release notes and link, integration state, title, body, changed paths, checksums, rollout, and rollback. Respond Boatstack update ready and render the one next action as: Reply `o` to open update PR. Continue accepting the full reply open update PR for compatibility without advertising it. Only the matching state-scoped shortcut or compatible full reply authorizes publish-update-pr --repo . --preview --preview-fingerprint . The deterministic publisher stages only previewed paths, reuses an existing update commit, pushes normally, reconciles the exact branch and PR after an interrupted response, and opens at most one reviewer-ready PR. Never stage, commit, push, or open the update PR through free-form terminal calls. If GitHub auth is unavailable, preserve the branch and give one manual publication action. If operation-status reports EXECUTING, wait; if it reports RECONCILE_REQUIRED, reconcile instead of repeating publication. After publication respond Update PR opened with the link and make Review the PR the one next action. On one collision or health failure, respond Update needs attention and make addressing that named problem the one next action. Never merge automatically.", "review": "Alias of review-gate: review the actual diff against approved intent, invariants, risks, gaps, and test evidence. Use Review passed or Changes required and the same single-action routing as review-gate.", "ship": "Alias of ship-gate: prepare and preview the exact reviewer-ready title and body before any GitHub mutation. Require the state-scoped reply o to open or u to update the PR before publication, recheck the preview against current evidence, and never merge or deploy. Keep pre-existing unrelated failures out of the approved feature branch. Use PR ready before confirmation or PR opened after publication.", diff --git a/labs/12-product-engineering-loop/product-engineering-loop/flow_control.go b/labs/12-product-engineering-loop/product-engineering-loop/flow_control.go index 154df84c..51ce6efa 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/flow_control.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/flow_control.go @@ -162,13 +162,10 @@ func classifyNextActor(status NextStatus, next FlowNext) NextActor { status.ObservedStage == "PUBLISHED" && status.Lifecycle == "PUBLISHED_MERGED": return NextActorNone case status.ObservedStage == "PUBLISHED": - // An owed visual attachment splits by what it owes: a transient - // publisher failure (visual_pending) is work-derivable — the agent - // retries attach-evidence; manual_required owes operator authority (a - // signed-in browser or an external-host opt-in) and stays theirs. A - // fired goal escape still demotes unconditionally. + // Both the current visual_pending state and the legacy manual_required + // state are work-derivable external-host retries. // control-law: turn-ends-only-at-the-operator-frontier - if status.Lifecycle == "PUBLISHED_OPEN" && status.GoalEscape == "" && status.VisualPublication == "visual_pending" { + if status.Lifecycle == "PUBLISHED_OPEN" && status.GoalEscape == "" && (status.VisualPublication == "visual_pending" || status.VisualPublication == "manual_required") { return NextActorAgent } // Under the default published terminal, reviewing the open pull @@ -455,10 +452,9 @@ func prescribePlanning(repo string, status NextStatus) (*PrescribedCommand, stri // prescribeVisualAttach closes the owed-attachment gap of a published-open // PR so the flow never goes dark on visual_pending or manual_required. It // fires under BOTH terminals — the attachment completes publication, it is -// not merge pursuit. visual_pending prescribes the attach-evidence retry -// (work-derivable); manual_required prescribes recording the manually -// attached comment, owing the operator-observed URL. A fired goal escape -// prescribes nothing, exactly like the post-publish layer. +// not merge pursuit. Both current and legacy owed states prescribe the same +// externally hosted attach-evidence retry. A fired goal escape prescribes +// nothing, exactly like the post-publish layer. // control-law: prescriptive-closure-every-stage-names-a-runnable-command func prescribeVisualAttach(repo string, status NextStatus) (*PrescribedCommand, string) { if status.ObservedStage != "PUBLISHED" || status.Lifecycle != "PUBLISHED_OPEN" || status.Feature == "" || status.GoalEscape != "" { @@ -469,24 +465,12 @@ func prescribeVisualAttach(repo string, status NextStatus) (*PrescribedCommand, repoArgs = []string{"--repo", repo} } switch status.VisualPublication { - case "visual_pending": + case "visual_pending", "manual_required": cmd := &PrescribedCommand{ Verb: "attach-evidence", Args: append(repoArgs, "--feature", status.Feature), AutoDerivable: true, Transition: MarkerPublishedAttach, } - return cmd, "The PR is open; only its Boatstack visual-evidence comment is owed. If the publisher keeps failing, attach the fingerprinted PNGs manually and record the URL with record-pr-visual-publication." - case "manual_required": - cmd := &PrescribedCommand{ - Verb: "record-pr-visual-publication", Args: append(repoArgs, "--key", status.Feature), - RequiresHumanInput: []string{"--comment-url"}, - Transition: MarkerPublishedAttach, - } - if strings.TrimSpace(status.PRURL) != "" { - cmd.Args = append(cmd.Args, "--pr-url", status.PRURL) - } else { - cmd.RequiresHumanInput = append(cmd.RequiresHumanInput, "--pr-url") - } - return cmd, "No automatic publisher is available here: attach the fingerprinted PNGs to one PR comment yourself, then record the observed comment URL." + return cmd, "The PR is open; only its externally hosted Boatstack visual-evidence comment is owed. Retry the same evidence fingerprint and comment after host access recovers." } return nil, "" } diff --git a/labs/12-product-engineering-loop/product-engineering-loop/next.go b/labs/12-product-engineering-loop/product-engineering-loop/next.go index f601448e..ede85a47 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/next.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/next.go @@ -260,7 +260,7 @@ func publishedNextStatus(state DeliveryState, pr publishedPRObservation, termina case "visual_pending": status.Reason += " Its Boatstack visual-evidence comment is still owed; Boatstack can retry the attachment (attach-evidence)." case "manual_required": - status.Reason += " Its visual-evidence comment needs manual attachment; record the observed URL with record-pr-visual-publication." + status.Reason += " Its legacy visual-evidence state must be retried through external hosting." } } if status.GoalEscape != "" { diff --git a/labs/12-product-engineering-loop/product-engineering-loop/next_actor_conformance_test.go b/labs/12-product-engineering-loop/product-engineering-loop/next_actor_conformance_test.go index bbdb956e..f6c82c7c 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/next_actor_conformance_test.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/next_actor_conformance_test.go @@ -102,7 +102,7 @@ func TestNextActorFrontierBoundaries(t *testing.T) { Prescribed: &PrescribedCommand{Verb: "record-delivery-gate", RequiresHumanInput: []string{"--status", "--evidence"}, Transition: deliverycontrol.TransitionID("delivery.record_gate_test")}, }, NextActorAgent}, {"owed_visual_attach_retry_is_agents", NextStatus{ObservedStage: "PUBLISHED", Lifecycle: "PUBLISHED_OPEN", VisualPublication: "visual_pending"}, FlowNext{}, NextActorAgent}, - {"manual_visual_attachment_is_operators", NextStatus{ObservedStage: "PUBLISHED", Lifecycle: "PUBLISHED_OPEN", VisualPublication: "manual_required"}, FlowNext{}, NextActorOperator}, + {"legacy_manual_visual_state_retries_as_hosted", NextStatus{ObservedStage: "PUBLISHED", Lifecycle: "PUBLISHED_OPEN", VisualPublication: "manual_required"}, FlowNext{}, NextActorAgent}, {"escaped_pursuit_demotes_despite_owed_attachment", NextStatus{ObservedStage: "PUBLISHED", Lifecycle: "PUBLISHED_OPEN", VisualPublication: "visual_pending", GoalEscape: "pr_closed"}, FlowNext{Terminal: TerminalMerged}, NextActorOperator}, } for _, tc := range cases { diff --git a/labs/12-product-engineering-loop/product-engineering-loop/plan.go b/labs/12-product-engineering-loop/product-engineering-loop/plan.go index 6f597c85..d2887cd2 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/plan.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/plan.go @@ -463,6 +463,9 @@ func ValidatePlan(plan map[string]any, opts *ValidatePlanOptions) error { if err := requireConfiguredPRVisualEvidenceDecision(plan, opts); err != nil { return err } + if err := requireVisualSurfaceEvidence(plan, opts); err != nil { + return err + } if stringValue(plan["feature_id"]) == "" { return fmt.Errorf("feature_id is required") diff --git a/labs/12-product-engineering-loop/product-engineering-loop/plan_validation.go b/labs/12-product-engineering-loop/product-engineering-loop/plan_validation.go index ef4b47d2..0d752481 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/plan_validation.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/plan_validation.go @@ -4,6 +4,8 @@ import ( "fmt" "path/filepath" "regexp" + "sort" + "strings" ) // surfaceSlugPattern names a product surface (web, ops, admin-console): @@ -17,6 +19,55 @@ type ValidatePlanOptions struct { RepoRevision string } +// requireVisualSurfaceEvidence makes registered surface paths a shared, +// deterministic visual-impact input during planning. The approved decision is +// then reused by capture, review, and ship. +func requireVisualSurfaceEvidence(plan map[string]any, opts *ValidatePlanOptions) error { + if opts == nil || opts.RepoRoot == "" || opts.PlanPath == "" { + return nil + } + config, _, err := LoadConfig(WorkspaceFor(opts.RepoRoot).ProjectConfigPath()) + if err != nil || len(config.Project.VisualSurfaces) == 0 { + return nil + } + baseline, err := productBaseline(opts.RepoRoot, opts.PlanPath) + if err != nil { + return err + } + matched := visualSurfaceChangedPaths(baseline.ChangedPaths, config.Project.VisualSurfaces) + if len(matched) == 0 { + return nil + } + decision, _ := plan["pr_visual_evidence"].(map[string]any) + if stringValue(decision["relevance"]) != "relevant" { + return fmt.Errorf("changes under registered visual surfaces require relevant pr_visual_evidence scenarios: %s", strings.Join(matched, ", ")) + } + return nil +} + +func visualSurfaceChangedPaths(changed []string, surfaces []VisualSurface) []string { + seen := map[string]bool{} + for _, changedPath := range changed { + path := filepath.ToSlash(filepath.Clean(changedPath)) + for _, surface := range surfaces { + for _, pattern := range surface.Paths { + pattern = filepath.ToSlash(strings.TrimSpace(pattern)) + matched, _ := filepath.Match(filepath.FromSlash(pattern), filepath.FromSlash(path)) + prefix := strings.TrimSuffix(strings.TrimSuffix(pattern, "/**"), "/") + if matched || (prefix != pattern && (path == prefix || strings.HasPrefix(path, prefix+"/"))) { + seen[path] = true + } + } + } + } + result := make([]string, 0, len(seen)) + for path := range seen { + result = append(result, path) + } + sort.Strings(result) + return result +} + func validateArchitectureGrounding(plan map[string]any, opts *ValidatePlanOptions) error { var ledger map[string]EvidenceRecord if opts != nil && opts.PlanPath != "" && opts.RepoRoot != "" { @@ -195,7 +246,7 @@ func validatePRVisualEvidence(plan map[string]any) error { return fmt.Errorf("pr_visual_evidence.scenarios must be a list") } if relevance == "not_relevant" { - if stringValue(decision["reason"]) == "" { + if strings.TrimSpace(stringValue(decision["reason"])) == "" { return fmt.Errorf("not-relevant pr_visual_evidence requires a reason") } if len(scenarios) != 0 { diff --git a/labs/12-product-engineering-loop/product-engineering-loop/plan_validation_test.go b/labs/12-product-engineering-loop/product-engineering-loop/plan_validation_test.go index da050312..a2201c55 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/plan_validation_test.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/plan_validation_test.go @@ -1,6 +1,8 @@ package boatstack import ( + "os" + "path/filepath" "runtime" "strings" "testing" @@ -209,6 +211,10 @@ func TestValidatePRVisualEvidence(t *testing.T) { if err := validatePRVisualEvidence(plan); err == nil || !strings.Contains(err.Error(), "reason") { t.Fatalf("missing not-relevant reason was not rejected: %v", err) } + plan["pr_visual_evidence"] = map[string]any{"relevance": "not_relevant", "reason": " ", "scenarios": []any{}} + if err := validatePRVisualEvidence(plan); err == nil || !strings.Contains(err.Error(), "reason") { + t.Fatalf("whitespace-only not-relevant reason was not rejected: %v", err) + } } func TestConfiguredPRVisualEvidenceRequiresPlanDecision(t *testing.T) { @@ -224,3 +230,28 @@ func TestConfiguredPRVisualEvidenceRequiresPlanDecision(t *testing.T) { t.Fatal(err) } } + +// control-law: registered-visual-surfaces-share-one-impact-decision +func TestRegisteredVisualSurfaceChangeRequiresRelevantDecision(t *testing.T) { + repo := prTestRepoConfigured(t, func(config *ProjectConfig) { + config.Workflow.PRVisualEvidence = "suggest" + config.Project.VisualSurfaces = []VisualSurface{{ID: "web", Paths: []string{"apps/web/**"}}} + }) + path := filepath.Join(repo, "apps", "web", "page.tsx") + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte("export default function Page() {}\n"), 0o644); err != nil { + t.Fatal(err) + } + plan := validV2Plan() + plan["pr_visual_evidence"] = map[string]any{"relevance": "not_relevant", "reason": "no visual change", "scenarios": []any{}} + opts := &ValidatePlanOptions{RepoRoot: repo, PlanPath: filepath.Join(repo, ".product-loop", "features", "demo", "plan.md")} + if err := requireVisualSurfaceEvidence(plan, opts); err == nil || !strings.Contains(err.Error(), "apps/web/page.tsx") { + t.Fatalf("registered surface change did not trigger evidence: %v", err) + } + plan["pr_visual_evidence"] = map[string]any{"relevance": "relevant", "scenarios": []any{map[string]any{"id": "web", "entry": "/", "state": "ready", "viewport": "1440x900", "expected": []any{"page visible"}}}} + if err := requireVisualSurfaceEvidence(plan, opts); 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 b910f0b2..cce3533c 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/pr.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/pr.go @@ -114,6 +114,8 @@ func planVisualDecision(repo, feature string) (string, string, []PRVisualScenari scenarios = append(scenarios, PRVisualScenario{ ID: stringValue(row["id"]), Entry: stringValue(row["entry"]), State: stringValue(row["state"]), Viewport: stringValue(row["viewport"]), Expected: expected, Surface: stringValue(row["surface"]), + UserContext: stringValue(row["user_context"]), UserGoal: stringValue(row["user_goal"]), + JourneyStep: stringValue(row["journey_step"]), ReviewerContext: stringValue(row["reviewer_context"]), }) } return relevance, "managed-plan", scenarios, nil @@ -139,7 +141,11 @@ func ensureCurrentPRVisualEvidence(repo string, config ProjectConfig, mode, feat if err != nil { return "", nil } - if loaded, loadErr := LoadPRVisualEvidence(repo, key); loadErr == nil && loaded.Status == "PASS" && loaded.ProductDiffSHA256 == diffHash { + scenarioHash, commandHash, identityErr := currentVisualEvidenceIdentity(scenarios, config) + if identityErr != nil { + return boundedCaptureDetail(identityErr.Error()), nil + } + if loaded, loadErr := LoadPRVisualEvidence(repo, key); loadErr == nil && loaded.Status == "PASS" && loaded.ProductDiffSHA256 == diffHash && loaded.ScenarioDefinitionSHA256 == scenarioHash && loaded.CaptureCommandSHA256 == commandHash { return "", nil } // Every declared surface must resolve to a repository command for capture @@ -176,6 +182,30 @@ func ensureCurrentPRVisualEvidence(repo string, config ProjectConfig, mode, feat return "", nil } +func currentVisualEvidenceIdentity(scenarios []PRVisualScenario, config ProjectConfig) (string, string, error) { + scenarioHash, err := visualScenarioDefinitionHash(scenarios) + if err != nil { + return "", "", err + } + commands, err := resolveScenarioCaptureCommands("visual", scenarios, config) + if err != nil { + return scenarioHash, "", err + } + commandRaw, err := MarshalJSON(commands) + if err != nil { + return "", "", err + } + return scenarioHash, SHA256Bytes(commandRaw), nil +} + +func visualScenarioDefinitionHash(scenarios []PRVisualScenario) (string, error) { + scenarioRaw, err := MarshalJSON(scenarios) + if err != nil { + return "", err + } + return SHA256Bytes(scenarioRaw), nil +} + // boundedCaptureDetail folds a harness error into a single bounded line so a // flaky harness cannot flood context JSON or denial text. func boundedCaptureDetail(detail string) string { @@ -211,6 +241,7 @@ func resolvePRVisualEvidence(repo string, config ProjectConfig, mode, feature, h } status := "NOT_APPLICABLE" var manifest *PRVisualEvidenceManifest + plannedScenarios := scenarios if policy != "off" && relevance != "not_relevant" { loaded, loadErr := LoadPRVisualEvidence(repo, key) if loadErr == nil { @@ -221,7 +252,15 @@ func resolvePRVisualEvidence(repo string, config ProjectConfig, mode, feature, h // 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 { + identityCurrent := true + if mode == "managed" && relevance == "relevant" { + scenarioHash, commandHash, identityErr := currentVisualEvidenceIdentity(plannedScenarios, config) + identityCurrent = loaded.ScenarioDefinitionSHA256 == scenarioHash && loaded.CaptureCommandSHA256 != "" + if identityErr == nil { + identityCurrent = identityCurrent && loaded.CaptureCommandSHA256 == commandHash + } + } + if loaded.ProductDiffSHA256 == diffHash && identityCurrent { status = loaded.Status } else { status = "NOT_VERIFIED" @@ -282,24 +321,21 @@ func publishPRVisualEvidence(repo, prURL string, context PRContext, publisher PR } // attachVisualEvidence performs the one publisher call and records the -// observed outcome: manual_required without a publisher, visual_pending on a -// publisher failure (PR preserved, fix forward), published on an observable -// comment URL. Shared by first publication (publishPRVisualEvidence) and the +// observed outcome: visual_pending without a publisher or on a publisher +// failure (PR preserved, fix forward), published on an observable comment URL. +// Shared by first publication (publishPRVisualEvidence) and the // attach-evidence retry, so both paths record identical states. func attachVisualEvidence(repo, prURL string, manifest PRVisualEvidenceManifest, publisher PRVisualEvidencePublisher, policy string) error { now := time.Now().UTC().Truncate(time.Second).Format(time.RFC3339) if publisher == nil { _, recordErr := recordPRVisualPublication(repo, manifest, PRVisualPublication{ - State: "manual_required", PRURL: prURL, UpdatedAt: now, - Detail: "attach the fingerprinted local PNG files to the Boatstack visual-evidence comment", + State: "visual_pending", PRURL: prURL, CommentURL: manifest.Publication.CommentURL, UpdatedAt: now, + Detail: "external-host publication is unavailable; retry the same evidence fingerprint and comment", }) if recordErr != nil { - return fmt.Errorf("PR opened but manual visual-evidence fallback could not be recorded: %w", recordErr) + return fmt.Errorf("PR opened but pending visual-evidence state could not be recorded: %w", recordErr) } - if policy == "require" { - return fmt.Errorf("PR opened at %s but required visual evidence still needs manual attachment; update the same PR after attachment", prURL) - } - return nil + return fmt.Errorf("PR opened at %s but external visual evidence is pending; retry the same PR after host access is available", prURL) } commentURL, publishErr := publisher.PublishVisualEvidence(repo, prURL, manifest.Publication.CommentURL, manifest) if publishErr != nil { @@ -310,6 +346,10 @@ func attachVisualEvidence(repo, prURL string, manifest PRVisualEvidenceManifest, return fmt.Errorf("PR opened at %s but visual evidence publication failed; preserve the PR and fix forward: %w", prURL, publishErr) } if strings.TrimSpace(commentURL) == "" { + _, _ = recordPRVisualPublication(repo, manifest, PRVisualPublication{ + State: "visual_pending", PRURL: prURL, CommentURL: manifest.Publication.CommentURL, + UpdatedAt: now, Detail: "visual evidence publisher returned no observable comment URL", + }) return fmt.Errorf("visual evidence publisher returned no observable comment URL") } _, err := recordPRVisualPublication(repo, manifest, PRVisualPublication{ @@ -347,7 +387,7 @@ func RetryVisualAttachment(repo, feature string, publisher PRVisualEvidencePubli return PRVisualEvidenceManifest{}, fmt.Errorf("visual evidence for %q records no pull request; publish-pr owns first publication", feature) } if publisher == nil { - return PRVisualEvidenceManifest{}, fmt.Errorf("no visual publisher is available in this environment; attach the fingerprinted PNGs to one PR comment yourself and record the observed URL with record-pr-visual-publication --key %s --pr-url %s --comment-url ", key, prURL) + return PRVisualEvidenceManifest{}, fmt.Errorf("no external visual publisher is available; retry the same evidence fingerprint and comment when host access is available") } if err := attachVisualEvidence(resolved, prURL, manifest, publisher, ""); err != nil { return PRVisualEvidenceManifest{}, err 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 10c9cc92..c014f9e8 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 @@ -659,11 +659,33 @@ func TestRequiredPRVisualEvidenceBlocksPublicationBeforeMutation(t *testing.T) { // approved scenario, bound to the given source commit and product diff. func savePassVisualManifest(t *testing.T, repo, feature, sourceCommit, diffHash string) { t.Helper() + config, _, err := LoadConfig(filepath.Join(repo, ".product-loop", "project.json")) + if err != nil { + t.Fatal(err) + } + _, _, scenarios, err := planVisualDecision(repo, feature) + if err != nil { + t.Fatal(err) + } + scenarioHash, commandHash, err := currentVisualEvidenceIdentity(scenarios, config) + if err != nil { + // Manual evidence remains bindable even when no automatic command is + // registered: use the explicit empty command-set identity. + scenarioRaw, marshalErr := MarshalJSON(scenarios) + if marshalErr != nil { + t.Fatal(marshalErr) + } + commandRaw, marshalErr := MarshalJSON(map[string]string{}) + if marshalErr != nil { + t.Fatal(marshalErr) + } + scenarioHash, commandHash = SHA256Bytes(scenarioRaw), SHA256Bytes(commandRaw) + } 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, + Status: "PASS", SourceCommit: sourceCommit, ProductDiffSHA256: diffHash, ScenarioDefinitionSHA256: scenarioHash, CaptureCommandSHA256: commandHash, Scenarios: []PRVisualScenario{{ID: "warning", Entry: "/onboarding", State: "picker open", Viewport: "1440x900", Expected: []string{"warning visible"}}}, Items: []PRVisualEvidenceItem{{ ScenarioID: "warning", Path: pngPath, Viewport: "1440x900", diff --git a/labs/12-product-engineering-loop/product-engineering-loop/references/config-schema.md b/labs/12-product-engineering-loop/product-engineering-loop/references/config-schema.md index 520a35b0..0fdfe867 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/references/config-schema.md +++ b/labs/12-product-engineering-loop/product-engineering-loop/references/config-schema.md @@ -8,6 +8,7 @@ boatstack-config-field:project.default_branch boatstack-config-field:project.context boatstack-config-field:project.commands boatstack-config-field:project.high_risk_paths +boatstack-config-field:project.visual_surfaces boatstack-config-field:project.migration boatstack-config-field:project.migration.apply_command boatstack-config-field:project.migration.verify_command @@ -78,6 +79,7 @@ This is the exhaustive serialization contract, not a list of recommended user ed - `visual` / `screenshot` / `e2e` (string, optional): The repository-owned visual capture harness Boatstack runs automatically during ship. A surface-scoped key `visual:` (e.g. `visual:web`, `visual:ops`; lowercase kebab surface, registered with `capability-register --surface`) outranks the global key for scenarios that declare that `surface`; scenarios without one, or without a surface key, use the global command exactly as before. - Other command names (string, optional): Additional repository-owned commands such as `build`, `lint`, or `typecheck`. - `high_risk_paths` (array of strings, optional): Glob patterns of files requiring independent reviewer sign-off before shipping. +- `visual_surfaces` (array of objects, optional): Registered product surfaces. Each object has a lowercase-kebab `id` and repository-relative `paths`; changes below these paths are screenshot candidates and cannot use `not_relevant`. - `migration` (object, optional): Declares how migrations are graded by EFFECT against a disposable database, so a committed migration stays a data artifact for the guard while its real effect is executed and observed by a conformance harness. Both commands run via `sh -c` with the disposable database coordinate in the environment as `BOATSTACK_MIGRATE_DB`; when `apply_command` is absent, grading is skipped. - `apply_command` (string, optional): The command that applies the migration set to the disposable database. - `verify_command` (string, optional): The command that asserts the post-migration invariant; a non-zero exit grades the migration FAIL. @@ -90,8 +92,8 @@ This is the exhaustive serialization contract, not a list of recommended user ed - `maintain_changelog` (boolean, optional): Whether a reader-visible `CHANGELOG.md` entry is required for each delivery slice. - `boundary_analysis` (boolean, optional): Agent-mediated planning guidance that presents local repair versus programmatic enforcement as a material product decision. - `pr_visual_evidence` (string, optional): `off`, `suggest`, or `require`. Omission is `off`. Relevant PRs use machine-local PNG evidence without committing media to Git; `suggest` records missing evidence as a visible gap and `require` blocks completed publication. When the approved plan declares `relevance: relevant` with scenarios, `suggest` ships with require semantics for that feature (a plan that promises pixels cannot ship without them) — even when no capture capability is registered yet. The two escapes are `off` (global) and a `not_relevant` plan decision with a reason (per feature, for genuinely nonvisual changes). Boatstack runs a registered capture command (`project.commands.visual`) automatically during ship, so under normal provisioning the escalation is invisible. -- `visual_evidence_publish` (object, optional): Agent-mediated publish control for how captured PNG bytes reach the pull-request comment. Omission keeps the default: commit the bytes to a public Boatstack-owned evidence branch and render them inline, but only for a **public** GitHub origin (a private origin falls back to manual attachment). Fields: - - `mode` (string, optional): `external-host` opts the repository — including a **private** one — into uploading the exact PNG bytes to an anonymous expiring host so the comment renders inline anywhere. It is **never auto-selected** because it publishes screenshot bytes to a third party; only this explicit value turns it on. Empty keeps the default public-branch behavior. +- `visual_evidence_publish` (object, optional): Publish control for externally hosted screenshots. Omission defaults to Litterbox with a 72-hour expiry. Upload is refused until every PNG has explicit human privacy review. Fields: + - `mode` (string, optional): Compatibility value `external-host`; external hosting is always used. - `host` (string, optional): `litterbox` (default) or `catbox`. Only meaningful when `mode` is `external-host`. `litterbox` auto-expires uploads; `catbox` is permanent. - `expiry` (string, optional): `1h`, `12h`, `24h`, or `72h` (default `72h`). Only meaningful for an expiring host; the PR comment reminds reviewers of the host and this window. - `ignored_deliveries` (array of strings, optional): Deterministic ambiguity control. Feature slugs of past deliveries to exclude from delivery-ambiguity resolution so historical work no longer blocks new work. New, unlisted ambiguous deliveries still pause the workflow. diff --git a/labs/12-product-engineering-loop/product-engineering-loop/references/workflow.md b/labs/12-product-engineering-loop/product-engineering-loop/references/workflow.md index 23b23d05..b48e3d61 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/references/workflow.md +++ b/labs/12-product-engineering-loop/product-engineering-loop/references/workflow.md @@ -411,7 +411,7 @@ Project the approved feature and actual committed diff into a reviewer-ready tit Store the exact preview at `.product-loop/features//pr.md`. Its non-rendered frontmatter records the title, base/head branches, managed feature, and context fingerprint; the remaining Markdown is the exact GitHub body. The preview artifact itself is excluded from the product-diff fingerprint so committing it does not create a self-referential hash. -PR schema v3 always records `pr_visual_evidence_policy`, `pr_visual_evidence_status`, `pr_visual_evidence_count`, and `pr_visual_evidence_fingerprint`. Relevant or unresolved PRs contain a structured **Visual evidence** section. Show the exact local images and public-repository privacy warning before confirmation. The state-scoped `o` or `u` authorizes the fingerprinted PR package: title, body, and one Boatstack-owned visual-evidence comment. A host browser may upload or update that comment; otherwise expose the exact local PNGs for manual attachment. If the PR mutation succeeds but attachment fails, preserve the PR, record `visual_pending`, and fix forward. Under `require`, do not mark managed delivery published until the attachment is observed. +PR schema v3 always records `pr_visual_evidence_policy`, `pr_visual_evidence_status`, `pr_visual_evidence_count`, and `pr_visual_evidence_fingerprint`. Relevant or unresolved PRs contain a structured **Visual evidence** section. Show the exact local images and external-host privacy warning before confirmation. The state-scoped `o` or `u` authorizes the fingerprinted PR package: title, body, and one Boatstack-owned visual-evidence comment. After human privacy review, Boatstack uploads to the configured external host (Litterbox for 72 hours by default), verifies every returned URL, and writes only hosted Markdown image links. If upload, URL verification, or comment mutation fails, preserve the PR and comment identity, record `visual_pending`, and retry the same fingerprint. Never attach PNG files directly or commit them to a branch. Under `require`, do not mark managed delivery published until the hosted comment is observed. Before publication, show the exact title and rendered body. Use **PR ready** and exactly one action. When no PR exists, render: Reply `o` to open PR. When one exists, render: Reply `u` to update PR. Only the corresponding state-scoped shortcut or compatible full reply authorizes opening or updating the PR. After confirmation, commit only the reviewed `pr.md`, recheck the same preview fingerprint, committed product diff, plan approval, build lock, test evidence, and review evidence, then let the deterministic publisher perform a normal push and the selected GitHub action. It records the fingerprinted package before execution. A lost response enters reconciliation against the exact remote branch and PR; it never opens another PR blindly. Any package drift blocks publication and requires a new preview; never force-push. diff --git a/labs/12-product-engineering-loop/product-engineering-loop/runtime.go b/labs/12-product-engineering-loop/product-engineering-loop/runtime.go index 75f42112..9ddeb6ee 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/runtime.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/runtime.go @@ -68,12 +68,18 @@ type DeliveryPolicy struct { } type Project struct { - Name string `json:"name"` - DefaultBranch string `json:"default_branch,omitempty"` - Context []string `json:"context,omitempty"` - Commands map[string]string `json:"commands"` - HighRiskPaths []string `json:"high_risk_paths,omitempty"` - Migration MigrationConfig `json:"migration,omitempty"` + Name string `json:"name"` + DefaultBranch string `json:"default_branch,omitempty"` + Context []string `json:"context,omitempty"` + Commands map[string]string `json:"commands"` + HighRiskPaths []string `json:"high_risk_paths,omitempty"` + VisualSurfaces []VisualSurface `json:"visual_surfaces,omitempty"` + Migration MigrationConfig `json:"migration,omitempty"` +} + +type VisualSurface struct { + ID string `json:"id"` + Paths []string `json:"paths"` } // MigrationConfig declares how a project APPLIES and VERIFIES its migrations against @@ -98,13 +104,8 @@ type Workflow struct { BoundaryAnalysis bool `json:"boundary_analysis,omitempty"` PRVisualEvidence string `json:"pr_visual_evidence,omitempty"` // VisualEvidencePublish selects how programmatic visual evidence reaches a PR. - // The nil zero value keeps Boatstack's default: commit the exact PNG bytes to a - // public Boatstack-owned evidence branch and render them inline, but only for a - // PUBLIC GitHub origin (a private origin falls back to manual attachment). - // Setting mode to "external-host" opts a repository — including a private one — - // into uploading the bytes to an anonymous expiring host so the comment renders - // inline anywhere; it is never auto-selected because it publishes screenshot - // bytes to a third party. + // The nil zero value uses Litterbox with a 72-hour expiry. Publication still + // requires explicit human privacy review for every screenshot. VisualEvidencePublish *VisualEvidencePublish `json:"visual_evidence_publish,omitempty"` // IgnoredDeliveries lists feature slugs of past deliveries to exclude from // delivery-ambiguity resolution. New, unlisted ambiguous deliveries still @@ -116,7 +117,7 @@ type Workflow struct { // zero value of each field resolves to a default at use, so a partially-specified // block (mode only) still works. type VisualEvidencePublish struct { - Mode string `json:"mode,omitempty"` // "" (default public-branch) | "external-host" + Mode string `json:"mode,omitempty"` // "" | "external-host" (external hosting is always used) Host string `json:"host,omitempty"` // external-host only: "litterbox" (default) | "catbox" Expiry string `json:"expiry,omitempty"` // expiring host only: "1h" | "12h" | "24h" | "72h" (default "72h") } diff --git a/labs/12-product-engineering-loop/product-engineering-loop/visual_attach_conformance_test.go b/labs/12-product-engineering-loop/product-engineering-loop/visual_attach_conformance_test.go index 6d89445d..a6084644 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/visual_attach_conformance_test.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/visual_attach_conformance_test.go @@ -10,7 +10,7 @@ import ( // // A published-open slice with an owed visual publication never resolves to a // dark prescription: visual_pending prescribes the agent-owned attach-evidence -// retry, manual_required prescribes recording the operator-attached comment, +// retry, and legacy manual_required maps to the same hosted retry, // and both fire under BOTH terminals because the attachment completes the // publication itself — it is not merge pursuit. @@ -39,16 +39,13 @@ func TestOwedVisualAttachmentNeverResolvesDark(t *testing.T) { } }) - t.Run("manual_required_prescribes_the_recording", func(t *testing.T) { + t.Run("manual_required_prescribes_the_hosted_retry", func(t *testing.T) { cmd, _ := prescribeVisualAttach(".", publishedOpenStatus("manual_required")) - if cmd == nil || cmd.Verb != "record-pr-visual-publication" { - t.Fatalf("manual_required did not prescribe the recording: %+v", cmd) - } - if cmd.AutoDerivable || strings.Join(cmd.RequiresHumanInput, " ") != "--comment-url" { - t.Fatalf("the observed comment URL must be owed to the operator: %+v", cmd) + if cmd == nil || cmd.Verb != "attach-evidence" || !cmd.AutoDerivable { + t.Fatalf("manual_required did not prescribe the hosted retry: %+v", cmd) } - if !strings.Contains(strings.Join(cmd.Args, " "), "--pr-url https://github.com/example/repo/pull/7") { - t.Fatalf("the recorded PR URL is state-derived and must be in Args: %v", cmd.Args) + if strings.Join(cmd.Args, " ") != "--feature demo" { + t.Fatalf("legacy retry arguments are not state-derived: %v", cmd.Args) } }) diff --git a/labs/12-product-engineering-loop/product-engineering-loop/visual_delivery_strengthening_test.go b/labs/12-product-engineering-loop/product-engineering-loop/visual_delivery_strengthening_test.go new file mode 100644 index 00000000..bd6bb505 --- /dev/null +++ b/labs/12-product-engineering-loop/product-engineering-loop/visual_delivery_strengthening_test.go @@ -0,0 +1,163 @@ +package boatstack + +import ( + "net/http" + "net/http/httptest" + "path/filepath" + "strings" + "testing" + "time" +) + +// control-law: registered-visual-surfaces-share-one-impact-decision +func TestVisualSurfaceChangesAreScreenshotCandidates(t *testing.T) { + got := visualSurfaceChangedPaths([]string{"apps/web/page.tsx", "api/server.go"}, []VisualSurface{{ID: "web", Paths: []string{"apps/web/**"}}}) + if len(got) != 1 || got[0] != "apps/web/page.tsx" { + t.Fatalf("unexpected candidates: %#v", got) + } +} + +// control-law: scenario-verification-requires-a-receipt +func TestPNGOnlyAndReceiptHarnessStatusesStaySeparate(t *testing.T) { + dir := t.TempDir() + missing := filepath.Join(dir, "missing.json") + if _, err := loadVisualScenarioReceipt(missing, "checkout"); err == nil { + t.Fatal("missing receipt must not verify a scenario") + } + receiptPath := filepath.Join(dir, "receipt.json") + receipt := []byte(`{"scenario_id":"checkout","reached_state_or_url":"/checkout","checks":[{"name":"total visible","result":"PASS"}],"overall_result":"PASS"}`) + if err := atomicWrite(receiptPath, receipt); err != nil { + t.Fatal(err) + } + got, err := loadVisualScenarioReceipt(receiptPath, "checkout") + if err != nil || got.OverallResult != "PASS" { + t.Fatalf("receipt was not verified: %#v %v", got, err) + } +} + +// control-law: external-upload-requires-human-privacy-review +func TestExternalUploadRefusesWithoutHumanPrivacyReview(t *testing.T) { + repo := visualTestRepo(t) + runGit(t, repo, "remote", "add", "origin", "https://github.com/o/n.git") + manifest := PRVisualEvidenceManifest{Key: "x", Items: []PRVisualEvidenceItem{{ScenarioID: "s", PrivacyStatus: "clean"}}} + _, err := (ExternalHostVisualEvidencePublisher{}).PublishVisualEvidence(repo, "https://github.com/o/n/pull/1", "", manifest) + if err == nil || !strings.Contains(err.Error(), "HUMAN_REVIEWED") { + t.Fatalf("expected privacy refusal, got %v", err) + } +} + +// control-law: only-verified-hosted-links-enter-the-comment +func TestHostedURLVerificationRejectsInvalidResponse(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusNotFound) })) + defer server.Close() + if err := verifyHostedVisualURL(externalHostSpec{endpoint: server.URL, label: "example.invalid"}, server.URL+"/missing.png"); err == nil { + t.Fatal("invalid hosted URL was accepted") + } +} + +func TestHostedURLVerificationRejectsUnexpectedDomain(t *testing.T) { + err := verifyHostedVisualURL(externalHostSpec{endpoint: "https://litterbox.catbox.moe/upload", label: "litter.catbox.moe"}, "https://example.com/image.png") + if err == nil || !strings.Contains(err.Error(), "unexpected domain") { t.Fatalf("unexpected host URL was accepted: %v", err) } +} + +func TestExternalCommentIncludesJourneyContext(t *testing.T) { + manifest := PRVisualEvidenceManifest{Key: "x", Scenarios: []PRVisualScenario{{ID: "s", Entry: "/start", State: "ready", Viewport: "800x600", Surface: "web", UserContext: "new customer", UserGoal: "finish checkout", JourneyStep: "confirm order", ReviewerContext: "verify price clarity", Expected: []string{"total visible"}}}, Items: []PRVisualEvidenceItem{{ScenarioID: "s"}}} + body := composeExternalHostComment(externalHostSpec{label: "host"}, "72h", map[string]string{"s": "https://host/s.png"}, manifest) + for _, want := range []string{"new customer", "finish checkout", "confirm order", "verify price clarity", "/start", "ready", "web"} { + if !strings.Contains(body, want) { + t.Fatalf("missing %q in comment:\n%s", want, body) + } + } +} + +func TestExternalCommentCannotInjectUnverifiedImageLinks(t *testing.T) { + manifest := PRVisualEvidenceManifest{Key: "x", Scenarios: []PRVisualScenario{{ID: "s", Viewport: "800x600", Expected: []string{"![leak](file:///private.png)"}}}, Items: []PRVisualEvidenceItem{{ScenarioID: "s"}}} + body := composeExternalHostComment(externalHostSpec{label: "host"}, "72h", map[string]string{"s": "https://host/s.png"}, manifest) + if strings.Count(body, "![") != 1 || !strings.Contains(body, "![s](https://host/s.png)") { + t.Fatalf("comment admitted an unverified image link:\n%s", body) + } +} + +func TestScenarioAndCommandChangesInvalidateEvidenceIdentity(t *testing.T) { + config := ProjectConfig{Project: Project{Commands: map[string]string{"visual": "capture-v1"}}} + scenarios := []PRVisualScenario{{ID: "s", Entry: "/", State: "ready", Viewport: "800x600", Expected: []string{"visible"}}} + s1, c1, err := currentVisualEvidenceIdentity(scenarios, config) + if err != nil { + t.Fatal(err) + } + scenarios[0].UserGoal = "complete purchase" + s2, _, err := currentVisualEvidenceIdentity(scenarios, config) + if err != nil || s1 == s2 { + t.Fatal("scenario change did not stale identity") + } + config.Project.Commands["visual"] = "capture-v2" + _, c2, err := currentVisualEvidenceIdentity(scenarios, config) + if err != nil || c1 == c2 { + t.Fatal("command change did not stale identity") + } +} + +func TestPublicationStateDoesNotChangeEvidenceFingerprint(t *testing.T) { + manifest := PRVisualEvidenceManifest{SchemaVersion: visualEvidenceSchemaVersion, Key: "x", Policy: "require", Relevance: "not_relevant", RelevanceSource: "managed-plan", Reason: "nonvisual", Status: "NOT_APPLICABLE"} + first, err := visualManifestFingerprint(manifest) + if err != nil { + t.Fatal(err) + } + manifest.Publication = PRVisualPublication{State: "visual_pending", PRURL: "https://github.com/o/n/pull/1", CommentURL: "https://github.com/o/n/pull/1#issuecomment-2"} + second, err := visualManifestFingerprint(manifest) + if err != nil || first != second { + t.Fatal("publication retry changed evidence identity") + } +} + +// control-law: partial-host-failure-preserves-pr-and-evidence-identity +func TestPartialExternalUploadLeavesVisualPendingOnSameFingerprint(t *testing.T) { + repo := visualTestRepo(t) + runGit(t, repo, "remote", "add", "origin", "https://github.com/o/n.git") + dir := t.TempDir() + paths := []string{filepath.Join(dir, "one.png"), filepath.Join(dir, "two.png")} + for _, path := range paths { + writeTestPNG(t, path) + } + now := time.Now().UTC().Truncate(time.Second).Format(time.RFC3339) + manifest, err := SavePRVisualEvidence(repo, PRVisualEvidenceManifest{ + Key: "partial", Policy: "require", Relevance: "relevant", RelevanceSource: "managed-plan", Status: "PASS", + SourceCommit: "head", ProductDiffSHA256: strings.Repeat("a", 64), ScenarioDefinitionSHA256: strings.Repeat("b", 64), CaptureCommandSHA256: strings.Repeat("c", 64), + Scenarios: []PRVisualScenario{{ID: "one", Entry: "/", State: "one", Viewport: "800x600", Expected: []string{"one"}}, {ID: "two", Entry: "/", State: "two", Viewport: "800x600", Expected: []string{"two"}}}, + Items: []PRVisualEvidenceItem{{ScenarioID: "one", Path: paths[0], Viewport: "800x600", CapturedAt: now, Status: "CAPTURED", PrivacyStatus: "human-reviewed", VerificationStatus: "CAPTURED"}, {ScenarioID: "two", Path: paths[1], Viewport: "800x600", CapturedAt: now, Status: "CAPTURED", PrivacyStatus: "human-reviewed", VerificationStatus: "CAPTURED"}}, + }) + if err != nil { + t.Fatal(err) + } + uploads := 0 + var server *httptest.Server + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet { + w.Header().Set("Content-Type", "image/png") + _, _ = w.Write([]byte("png")) + return + } + uploads++ + if uploads == 2 { + http.Error(w, "partial failure", http.StatusServiceUnavailable) + return + } + _, _ = w.Write([]byte(server.URL + "/one.png")) + })) + defer server.Close() + original := visualExternalHosts["litterbox"] + visualExternalHosts["litterbox"] = externalHostSpec{endpoint: server.URL, label: "local", withExpiry: true} + defer func() { visualExternalHosts["litterbox"] = original }() + prURL := "https://github.com/o/n/pull/7" + err = attachVisualEvidence(repo, prURL, manifest, ExternalHostVisualEvidencePublisher{}, "require") + if err == nil { + t.Fatal("partial upload unexpectedly published") + } + pending, loadErr := LoadPRVisualEvidence(repo, manifest.Key) + if loadErr != nil { + t.Fatal(loadErr) + } + if pending.Publication.State != "visual_pending" || pending.Publication.PRURL != prURL || pending.Fingerprint != manifest.Fingerprint { + t.Fatalf("partial failure lost PR or evidence identity: %#v", pending) + } +} diff --git a/labs/12-product-engineering-loop/product-engineering-loop/visual_evidence.go b/labs/12-product-engineering-loop/product-engineering-loop/visual_evidence.go index 5653a68a..b8066fba 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/visual_evidence.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/visual_evidence.go @@ -25,21 +25,41 @@ type PRVisualScenario struct { // (e.g. "web", "ops"), selecting a surface-scoped harness command // (project.commands["visual:"]) over the global one. omitempty // keeps existing manifest fingerprints byte-stable. - Surface string `json:"surface,omitempty"` + Surface string `json:"surface,omitempty"` + UserContext string `json:"user_context,omitempty"` + UserGoal string `json:"user_goal,omitempty"` + JourneyStep string `json:"journey_step,omitempty"` + ReviewerContext string `json:"reviewer_context,omitempty"` +} + +type PRVisualScenarioCheck struct { + Name string `json:"name"` + Result string `json:"result"` +} + +// PRVisualScenarioReceipt is optional harness output. A valid receipt proves the +// named scenario checks; PNG validity alone never creates this status. +type PRVisualScenarioReceipt struct { + ScenarioID string `json:"scenario_id"` + Reached string `json:"reached_state_or_url"` + Checks []PRVisualScenarioCheck `json:"checks"` + OverallResult string `json:"overall_result"` } type PRVisualEvidenceItem struct { - ScenarioID string `json:"scenario_id"` - Path string `json:"path"` - SHA256 string `json:"sha256"` - MIMEType string `json:"mime_type"` - Width int `json:"width"` - Height int `json:"height"` - DurationMS int `json:"duration_ms"` - Viewport string `json:"viewport"` - CapturedAt string `json:"captured_at"` - Status string `json:"status"` - PrivacyStatus string `json:"privacy_status"` + ScenarioID string `json:"scenario_id"` + Path string `json:"path"` + SHA256 string `json:"sha256"` + MIMEType string `json:"mime_type"` + Width int `json:"width"` + Height int `json:"height"` + DurationMS int `json:"duration_ms"` + Viewport string `json:"viewport"` + CapturedAt string `json:"captured_at"` + Status string `json:"status"` + PrivacyStatus string `json:"privacy_status"` + VerificationStatus string `json:"verification_status,omitempty"` + Receipt *PRVisualScenarioReceipt `json:"receipt,omitempty"` } type PRVisualPublication struct { @@ -51,19 +71,21 @@ type PRVisualPublication struct { } type PRVisualEvidenceManifest struct { - SchemaVersion int `json:"schema_version"` - Key string `json:"key"` - Policy string `json:"policy"` - Relevance string `json:"relevance"` - RelevanceSource string `json:"relevance_source"` - Reason string `json:"reason,omitempty"` - Status string `json:"status"` - SourceCommit string `json:"source_commit"` - ProductDiffSHA256 string `json:"product_diff_sha256"` - Scenarios []PRVisualScenario `json:"scenarios,omitempty"` - Items []PRVisualEvidenceItem `json:"items,omitempty"` - Publication PRVisualPublication `json:"publication"` - Fingerprint string `json:"fingerprint"` + SchemaVersion int `json:"schema_version"` + Key string `json:"key"` + Policy string `json:"policy"` + Relevance string `json:"relevance"` + RelevanceSource string `json:"relevance_source"` + Reason string `json:"reason,omitempty"` + Status string `json:"status"` + SourceCommit string `json:"source_commit"` + ProductDiffSHA256 string `json:"product_diff_sha256"` + ScenarioDefinitionSHA256 string `json:"scenario_definition_sha256,omitempty"` + CaptureCommandSHA256 string `json:"capture_command_sha256,omitempty"` + Scenarios []PRVisualScenario `json:"scenarios,omitempty"` + Items []PRVisualEvidenceItem `json:"items,omitempty"` + Publication PRVisualPublication `json:"publication"` + Fingerprint string `json:"fingerprint"` } type PRVisualCapabilityReceipt struct { @@ -187,6 +209,9 @@ func visualCapabilityPath(repo string) (string, error) { func visualManifestFingerprint(manifest PRVisualEvidenceManifest) (string, error) { copy := manifest copy.Fingerprint = "" + // Publication is retry state, not evidence identity. A failed upload or + // comment update must retry the same package fingerprint and comment. + copy.Publication = PRVisualPublication{} raw, err := MarshalJSON(copy) if err != nil { return "", err @@ -236,7 +261,10 @@ func validateVisualManifest(manifest PRVisualEvidenceManifest) error { return fmt.Errorf("visual evidence items must reference a scenario and describe a valid PNG") } seenItems[item.ScenarioID] = true - if item.Status != "captured" || item.Viewport != scenarioViewports[item.ScenarioID] { + if item.Status != "captured" && item.Status != "CAPTURED" { + return fmt.Errorf("visual evidence items require CAPTURED status") + } + if item.Viewport != scenarioViewports[item.ScenarioID] { return fmt.Errorf("visual evidence items require captured status and the approved scenario viewport") } if item.PrivacyStatus != "clean" && item.PrivacyStatus != "human-reviewed" { @@ -245,6 +273,24 @@ func validateVisualManifest(manifest PRVisualEvidenceManifest) error { if _, err := time.Parse(time.RFC3339, item.CapturedAt); err != nil { return fmt.Errorf("visual evidence captured_at must be RFC3339: %w", err) } + if item.VerificationStatus == "" { + item.VerificationStatus = "CAPTURED" + } + if item.VerificationStatus != "CAPTURED" && item.VerificationStatus != "SCENARIO_VERIFIED" { + return fmt.Errorf("visual evidence verification_status must be CAPTURED or SCENARIO_VERIFIED") + } + if item.VerificationStatus == "SCENARIO_VERIFIED" { + if item.Receipt == nil || item.Receipt.ScenarioID != item.ScenarioID || strings.TrimSpace(item.Receipt.Reached) == "" || len(item.Receipt.Checks) == 0 || !strings.EqualFold(item.Receipt.OverallResult, "PASS") { + return fmt.Errorf("SCENARIO_VERIFIED requires a valid passing receipt for the same scenario") + } + for _, check := range item.Receipt.Checks { + if strings.TrimSpace(check.Name) == "" || !strings.EqualFold(check.Result, "PASS") { + return fmt.Errorf("SCENARIO_VERIFIED requires named passing checks") + } + } + } else if item.Receipt != nil { + return fmt.Errorf("a scenario receipt must produce SCENARIO_VERIFIED") + } } if manifest.Status == "PASS" && (manifest.Relevance != "relevant" || len(manifest.Items) != len(manifest.Scenarios)) { return fmt.Errorf("PASS visual evidence requires one screenshot for every relevant scenario") diff --git a/labs/12-product-engineering-loop/product-engineering-loop/visual_evidence_test.go b/labs/12-product-engineering-loop/product-engineering-loop/visual_evidence_test.go index 2b8011ee..a8f9dd30 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/visual_evidence_test.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/visual_evidence_test.go @@ -213,6 +213,9 @@ func TestRetryVisualAttachmentCompletesOwedPublication(t *testing.T) { if retried.Publication.State != "published" || retried.Publication.PRURL != prURL || retried.Publication.CommentURL == "" { t.Fatalf("retry did not complete the owed publication: %#v", retried.Publication) } + if retried.Fingerprint != manifest.Fingerprint { + t.Fatalf("retry changed the evidence fingerprint: before=%s after=%s", manifest.Fingerprint, retried.Fingerprint) + } again, err := RetryVisualAttachment(repo, "feature-warning", &fakeVisualPublisher{err: os.ErrPermission}) if err != nil || again.Publication.State != "published" { t.Fatalf("published attachment must be an idempotent no-op: %#v %v", again.Publication, err) @@ -231,14 +234,14 @@ func TestRetryVisualAttachmentRefusesWhatItDoesNotOwn(t *testing.T) { t.Fatalf("pre-publication manifest was not routed to publish-pr: %v", err) } context := PRContext{PRVisualEvidencePolicy: "suggest", PRVisualEvidenceStatus: "PASS", PRVisualEvidence: &manifest} - if err := publishPRVisualEvidence(repo, "https://github.com/example/repo/pull/4", context, nil); err != nil { - t.Fatal(err) + if err := publishPRVisualEvidence(repo, "https://github.com/example/repo/pull/4", context, nil); err == nil || !strings.Contains(err.Error(), "external visual evidence is pending") { + t.Fatalf("missing external publisher did not preserve a pending PR: %v", err) } - if _, err := RetryVisualAttachment(repo, "feature-warning", nil); err == nil || !strings.Contains(err.Error(), "record-pr-visual-publication") { - t.Fatalf("missing publisher must name the manual recording verb: %v", err) + if _, err := RetryVisualAttachment(repo, "feature-warning", nil); err == nil || !strings.Contains(err.Error(), "external visual publisher") { + t.Fatalf("missing publisher must preserve the hosted retry: %v", err) } recovered, err := RetryVisualAttachment(repo, "feature-warning", &fakeVisualPublisher{commentURL: "https://github.com/example/repo/pull/4#issuecomment-1"}) if err != nil || recovered.Publication.State != "published" { - t.Fatalf("manual_required with a live publisher should still recover: %#v %v", recovered.Publication, err) + t.Fatalf("legacy owed state with a live publisher should still recover: %#v %v", recovered.Publication, err) } } diff --git a/labs/12-product-engineering-loop/product-engineering-loop/visual_publisher.go b/labs/12-product-engineering-loop/product-engineering-loop/visual_publisher.go index e591c51d..9a72dfaf 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/visual_publisher.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/visual_publisher.go @@ -3,9 +3,11 @@ package boatstack import ( "bytes" "fmt" + "html" "io" "mime/multipart" "net/http" + "net/url" "os" "path/filepath" "regexp" @@ -32,14 +34,8 @@ var ( originSlugHTTP = regexp.MustCompile(`^https?://github\.com/([^/]+)/(.+?)(?:\.git)?$`) ) -// SelectVisualPublisher returns a programmatic publisher when the repository can -// actually render committed evidence — a GitHub origin with gh available and -// authenticated. The default publisher renders inline only for a PUBLIC repository -// (raw.githubusercontent.com does not serve private content to anonymous markdown -// renderers). A repository may opt into the external-host publisher via -// workflow.visual_evidence_publish.mode="external-host" to render inline even when -// private. Otherwise it returns nil so the caller's manual-attachment fallback stays -// in force (which never blocks a suggest-policy PR). +// SelectVisualPublisher always chooses external hosting. Screenshot bytes never +// enter Git or a pull-request attachment path. func SelectVisualPublisher(repo string) PRVisualEvidencePublisher { resolved, err := ResolveRepository(repo) if err != nil { @@ -51,23 +47,15 @@ func SelectVisualPublisher(repo string) PRVisualEvidencePublisher { if _, _, err := originRepoSlug(resolved); err != nil { return nil } - // External-host mode is opt-in only: it publishes screenshot bytes to a - // third-party anonymous host, so it is never auto-selected — only this explicit - // config value turns it on, and it works for a private origin too. - if publish := visualPublishConfig(resolved); publish != nil && publish.Mode == "external-host" { + if publish := visualPublishConfig(resolved); publish != nil { return ExternalHostVisualEvidencePublisher{Host: publish.Host, Expiry: publish.Expiry} } - visibility, err := commandOutput(resolved, "gh", "repo", "view", "--json", "visibility", "--jq", ".visibility") - if err != nil || !strings.EqualFold(strings.TrimSpace(visibility), "public") { - return nil - } - return GitVisualEvidencePublisher{} + return ExternalHostVisualEvidencePublisher{Host: defaultExternalHost, Expiry: defaultExternalExpiry} } // visualPublishConfig reads the repository's visual-evidence publish preferences from // the generated project config, returning nil when the config is absent, unreadable, -// or leaves the block unset so the caller falls back to the default public-branch -// behavior. +// or leaves the block unset so the caller uses the external-host defaults. func visualPublishConfig(repo string) *VisualEvidencePublish { config, _, err := LoadConfig(WorkspaceFor(repo).ProjectConfigPath()) if err != nil { @@ -76,92 +64,10 @@ func visualPublishConfig(repo string) *VisualEvidencePublish { return config.Workflow.VisualEvidencePublish } -// PublishVisualEvidence commits the manifest's exact PNG bytes to the evidence -// branch, then posts or updates the single Boatstack-owned comment on the PR. +// PublishVisualEvidence remains as a compatibility entry point but uses the +// external-host path. No screenshot publisher writes image bytes to Git. func (GitVisualEvidencePublisher) PublishVisualEvidence(repo, prURL, existingCommentURL string, manifest PRVisualEvidenceManifest) (string, error) { - resolved, err := ResolveRepository(repo) - if err != nil { - return "", err - } - owner, name, err := originRepoSlug(resolved) - if err != nil { - return "", err - } - prNumber, err := prNumberFromURL(prURL) - if err != nil { - return "", err - } - if len(manifest.Items) == 0 { - return "", fmt.Errorf("visual evidence has no screenshots to publish") - } - commitSHA, err := pushEvidenceCommit(resolved, manifest) - if err != nil { - return "", err - } - body := composeVisualEvidenceComment(owner, name, commitSHA, manifest) - return upsertEvidenceComment(resolved, owner, name, prNumber, existingCommentURL, manifest.Key, body) -} - -// pushEvidenceCommit builds a commit carrying the exact PNG bytes with Git plumbing -// against a temporary index — never touching the working tree — and pushes it to the -// Boatstack-owned evidence branch, accumulating onto the branch's prior tip when one -// exists. It returns the commit SHA, which pins immutable raw content URLs. -func pushEvidenceCommit(repo string, manifest PRVisualEvidenceManifest) (string, error) { - branch, err := evidenceBranchName(manifest.Key) - if err != nil { - return "", err - } - indexFile, err := os.CreateTemp("", "boatstack-evidence-index-*") - if err != nil { - return "", err - } - indexPath := indexFile.Name() - indexFile.Close() - os.Remove(indexPath) // git wants to create the index itself - defer os.Remove(indexPath) - git := func(arguments ...string) (string, error) { - return gitIndexedCommand(repo, indexPath, arguments...) - } - - var parent string - if tip := strings.TrimSpace(gitOutput(repo, "ls-remote", "origin", "refs/heads/"+branch)); tip != "" { - parent = strings.Fields(tip)[0] - if _, err := commandOutput(repo, "git", "-C", repo, "fetch", "origin", branch); err != nil { - return "", fmt.Errorf("cannot fetch the visual-evidence branch to extend it: %w", err) - } - if _, err := git("read-tree", parent); err != nil { - return "", err - } - } - for _, item := range manifest.Items { - blob, err := git("hash-object", "-w", "--", item.Path) - if err != nil { - return "", err - } - path := evidenceBlobPath(manifest.Key, item.SHA256) - if _, err := git("update-index", "--add", "--cacheinfo", "100644,"+strings.TrimSpace(blob)+","+path); err != nil { - return "", err - } - } - tree, err := git("write-tree") - if err != nil { - return "", err - } - tree = strings.TrimSpace(tree) - message := "boatstack: visual evidence for " + manifest.Key + " (" + manifest.Fingerprint + ")" - commitArgs := []string{"commit-tree", tree, "-m", message} - if parent != "" { - commitArgs = append(commitArgs, "-p", parent) - } - commit, err := git(commitArgs...) - if err != nil { - return "", err - } - commit = strings.TrimSpace(commit) - if _, err := commandOutput(repo, "git", "-C", repo, "push", "origin", commit+":refs/heads/"+branch); err != nil { - return "", fmt.Errorf("cannot push the visual-evidence commit without rewriting history: %w", err) - } - return commit, nil + return (ExternalHostVisualEvidencePublisher{Host: defaultExternalHost, Expiry: defaultExternalExpiry}).PublishVisualEvidence(repo, prURL, existingCommentURL, manifest) } // upsertEvidenceComment posts the composed body to exactly one Boatstack-owned @@ -204,12 +110,6 @@ func upsertEvidenceComment(repo, owner, name, prNumber, existingCommentURL, key, "-F", "body=@"+bodyPath, "--jq", ".html_url") } -// gitIndexedCommand runs git against a scoped, temporary index so evidence commits -// never disturb the repository's real index or working tree. -func gitIndexedCommand(repo, indexPath string, arguments ...string) (string, error) { - return commandOutputEnv(repo, []string{"GIT_INDEX_FILE=" + indexPath}, "git", append([]string{"-C", repo}, arguments...)...) -} - func originRepoSlug(repo string) (string, string, error) { remote, err := commandOutput(repo, "git", "-C", repo, "remote", "get-url", "origin") if err != nil { @@ -238,57 +138,10 @@ func commentIDFromURL(commentURL string) string { return "" } -func evidenceBranchName(key string) (string, error) { - safe, err := safeCacheSegment(key, "visual evidence key") - if err != nil { - return "", err - } - return "boatstack-visual-evidence/" + safe, nil -} - -func evidenceBlobPath(key, sha string) string { - return key + "/" + sha + ".png" -} - -func rawContentURL(owner, name, commitSHA, path string) string { - return "https://raw.githubusercontent.com/" + owner + "/" + name + "/" + commitSHA + "/" + path -} - func visualEvidenceCommentMarker(key string) string { return "" } -// composeVisualEvidenceComment renders the single Boatstack-owned comment: a hidden -// marker for idempotent reuse, the trust fingerprints, the standing public-repository -// privacy warning, and one image per scenario pinned to the evidence commit. -func composeVisualEvidenceComment(owner, name, commitSHA string, manifest PRVisualEvidenceManifest) string { - itemsByScenario := make(map[string]PRVisualEvidenceItem, len(manifest.Items)) - for _, item := range manifest.Items { - itemsByScenario[item.ScenarioID] = item - } - var builder strings.Builder - builder.WriteString(visualEvidenceCommentMarker(manifest.Key) + "\n") - builder.WriteString("### Visual evidence\n\n") - builder.WriteString("Screenshots are human-review evidence, not mechanical proof. These images are committed to a public branch and are publicly accessible.\n\n") - builder.WriteString(fmt.Sprintf("Source commit `%s` · product diff `%s` · fingerprint `%s`\n\n", manifest.SourceCommit, manifest.ProductDiffSHA256, manifest.Fingerprint)) - rendered := 0 - for _, scenario := range manifest.Scenarios { - item, ok := itemsByScenario[scenario.ID] - if !ok { - continue - } - caption := strings.Join(scenario.Expected, "; ") - builder.WriteString(fmt.Sprintf("**%s** — %s (`%s`)\n\n", scenario.ID, caption, scenario.Viewport)) - url := rawContentURL(owner, name, commitSHA, evidenceBlobPath(manifest.Key, item.SHA256)) - builder.WriteString(fmt.Sprintf("![%s](%s)\n\n", scenario.ID, url)) - rendered++ - } - if rendered == 0 { - builder.WriteString("_No captured scenarios to display._\n") - } - return builder.String() -} - // externalHostSpec describes an anonymous image host used by external-host mode. type externalHostSpec struct { endpoint string @@ -341,6 +194,11 @@ func (p ExternalHostVisualEvidencePublisher) PublishVisualEvidence(repo, prURL, if len(manifest.Items) == 0 { return "", fmt.Errorf("visual evidence has no screenshots to publish") } + for _, item := range manifest.Items { + if item.PrivacyStatus != "human-reviewed" { + return "", fmt.Errorf("refusing external upload for scenario %s without HUMAN_REVIEWED privacy status", item.ScenarioID) + } + } host := strings.TrimSpace(p.Host) if host == "" { host = defaultExternalHost @@ -359,12 +217,44 @@ func (p ExternalHostVisualEvidencePublisher) PublishVisualEvidence(repo, prURL, if err != nil { return "", err } + if err := verifyHostedVisualURL(spec, url); err != nil { + return "", fmt.Errorf("verify hosted URL for %s: %w", item.ScenarioID, err) + } urls[item.ScenarioID] = url } body := composeExternalHostComment(spec, expiry, urls, manifest) return upsertEvidenceComment(resolved, owner, name, prNumber, existingCommentURL, manifest.Key, body) } +func verifyHostedVisualURL(spec externalHostSpec, value string) error { + parsed, err := url.Parse(value) + if err != nil || (parsed.Scheme != "https" && parsed.Scheme != "http") || parsed.Hostname() == "" { + return fmt.Errorf("host returned an invalid absolute HTTP URL") + } + endpoint, endpointErr := url.Parse(spec.endpoint) + localTestHost := endpointErr == nil && (endpoint.Hostname() == "127.0.0.1" || endpoint.Hostname() == "localhost" || endpoint.Hostname() == "::1") + if !localTestHost && !strings.EqualFold(parsed.Hostname(), spec.label) { + return fmt.Errorf("host returned URL for unexpected domain %q", parsed.Hostname()) + } + request, err := http.NewRequest(http.MethodGet, value, nil) + if err != nil { + return err + } + response, err := http.DefaultClient.Do(request) + if err != nil { + return err + } + defer response.Body.Close() + if response.StatusCode < 200 || response.StatusCode >= 400 { + return fmt.Errorf("hosted URL returned HTTP %d", response.StatusCode) + } + contentType := strings.ToLower(response.Header.Get("Content-Type")) + if contentType != "" && !strings.HasPrefix(contentType, "image/") { + return fmt.Errorf("hosted URL returned non-image content type %q", contentType) + } + return nil +} + // uploadToExternalHost POSTs one PNG to an anonymous host and returns the hosted URL. // The host answers with the URL as plain text; any non-200 or non-URL body is a // failure so the caller can fix forward without emitting a broken image. @@ -430,9 +320,10 @@ func composeExternalHostComment(spec externalHostSpec, expiry string, urls map[s if !ok { continue } - caption := strings.Join(scenario.Expected, "; ") - builder.WriteString(fmt.Sprintf("**%s** — %s (`%s`)\n\n", scenario.ID, caption, scenario.Viewport)) - builder.WriteString(fmt.Sprintf("![%s](%s)\n\n", scenario.ID, url)) + caption := visualCommentText(strings.Join(scenario.Expected, "; ")) + builder.WriteString(fmt.Sprintf("**%s** — %s (`%s`)\n\n", visualCommentText(scenario.ID), caption, visualCommentText(scenario.Viewport))) + builder.WriteString(fmt.Sprintf("User context: %s \nUser goal: %s \nJourney step: %s \nReviewer context: %s \nEntry: `%s` · State: `%s` · Surface: `%s`\n\n", visualCommentText(scenario.UserContext), visualCommentText(scenario.UserGoal), visualCommentText(scenario.JourneyStep), visualCommentText(scenario.ReviewerContext), visualCommentText(scenario.Entry), visualCommentText(scenario.State), visualCommentText(scenario.Surface))) + builder.WriteString(fmt.Sprintf("![%s](%s)\n\n", visualCommentText(scenario.ID), url)) rendered++ } if rendered == 0 { @@ -446,3 +337,9 @@ func composeExternalHostComment(spec externalHostSpec, expiry string, urls map[s } return builder.String() } + +func visualCommentText(value string) string { + value = strings.Join(strings.Fields(value), " ") + value = html.EscapeString(value) + return strings.NewReplacer("\\", "\\\\", "!", "\\!", "[", "\\[", "]", "\\]", "`", "\\`").Replace(value) +} diff --git a/labs/12-product-engineering-loop/product-engineering-loop/visual_publisher_test.go b/labs/12-product-engineering-loop/product-engineering-loop/visual_publisher_test.go index 8b1fa4e8..6503c0a3 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/visual_publisher_test.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/visual_publisher_test.go @@ -59,76 +59,6 @@ func TestCommentIDFromURL(t *testing.T) { } } -func TestEvidenceBranchNameRejectsUnsafeKey(t *testing.T) { - if _, err := evidenceBranchName("../escape"); err == nil { - t.Fatal("path-traversal key must be rejected") - } - branch, err := evidenceBranchName("firm-status") - if err != nil || branch != "boatstack-visual-evidence/firm-status" { - t.Fatalf("got %q, %v", branch, err) - } -} - -func TestRawContentURLPinsCommit(t *testing.T) { - url := rawContentURL("o", "n", "abc123", evidenceBlobPath("firm-status", "deadbeef")) - want := "https://raw.githubusercontent.com/o/n/abc123/firm-status/deadbeef.png" - if url != want { - t.Fatalf("got %q, want %q", url, want) - } -} - -func TestComposeVisualEvidenceCommentRendersScenariosAndWarnings(t *testing.T) { - manifest := PRVisualEvidenceManifest{ - Key: "firm-status", - SourceCommit: "src123", - ProductDiffSHA256: "diff456", - Fingerprint: "fp789", - Scenarios: []PRVisualScenario{ - {ID: "VS-1", Entry: "/clients", State: "hover", Viewport: "1440x900", Expected: []string{"portal card is blue"}}, - {ID: "VS-2", Entry: "/clients", State: "default", Viewport: "1440x900", Expected: []string{"amber badge"}}, - }, - Items: []PRVisualEvidenceItem{ - {ScenarioID: "VS-1", SHA256: "hash1"}, - {ScenarioID: "VS-2", SHA256: "hash2"}, - }, - } - body := composeVisualEvidenceComment("o", "n", "commitSHA", manifest) - - if !strings.HasPrefix(body, visualEvidenceCommentMarker("firm-status")) { - t.Fatal("comment must open with the idempotency marker") - } - for _, want := range []string{ - "publicly accessible", // standing privacy warning - "human-review evidence", // not-mechanical-proof warning - "src123", "diff456", "fp789", // trust fingerprints - "VS-1", "VS-2", // both scenarios - "portal card is blue", "amber badge", - rawContentURL("o", "n", "commitSHA", "firm-status/hash1.png"), - rawContentURL("o", "n", "commitSHA", "firm-status/hash2.png"), - } { - if !strings.Contains(body, want) { - t.Fatalf("comment body missing %q\n---\n%s", want, body) - } - } -} - -func TestComposeVisualEvidenceCommentSkipsUncapturedScenarios(t *testing.T) { - manifest := PRVisualEvidenceManifest{ - Key: "firm-status", - Scenarios: []PRVisualScenario{ - {ID: "VS-1", Viewport: "1440x900", Expected: []string{"x"}}, - }, - // No items → nothing captured. - } - body := composeVisualEvidenceComment("o", "n", "c", manifest) - if strings.Contains(body, "![VS-1]") { - t.Fatal("uncaptured scenario must not render an image") - } - if !strings.Contains(body, "No captured scenarios") { - t.Fatal("expected an explicit empty-state note") - } -} - func TestUploadToExternalHostPostsFormAndReturnsURL(t *testing.T) { dir := t.TempDir() png := filepath.Join(dir, "VS-1.png") @@ -136,7 +66,8 @@ func TestUploadToExternalHostPostsFormAndReturnsURL(t *testing.T) { var gotReqtype, gotTime, gotFilename string var gotBytes int - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var server *httptest.Server + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { gotReqtype = r.FormValue("reqtype") gotTime = r.FormValue("time") file, header, err := r.FormFile("fileToUpload") @@ -280,12 +211,18 @@ func TestExternalHostPublishVisualEvidenceUploadsAndUpserts(t *testing.T) { png := filepath.Join(repo, "VS-1.png") writeTestPNG(t, png) - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var server *httptest.Server + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet { + w.Header().Set("Content-Type", "image/png") + _, _ = w.Write([]byte("png")) + return + } if _, _, err := r.FormFile("fileToUpload"); err != nil { http.Error(w, err.Error(), http.StatusBadRequest) return } - _, _ = w.Write([]byte("https://litter.catbox.moe/hosted.png")) + _, _ = w.Write([]byte(server.URL + "/hosted.png")) })) defer server.Close() @@ -316,8 +253,10 @@ exit 1 ProductDiffSHA256: "diff", Fingerprint: "fp", Scenarios: []PRVisualScenario{{ID: "VS-1", Viewport: "1440x900", Expected: []string{"blue"}}}, - Items: []PRVisualEvidenceItem{{ScenarioID: "VS-1", Path: png}}, + Items: []PRVisualEvidenceItem{{ScenarioID: "VS-1", Path: png, PrivacyStatus: "human-reviewed"}}, } + // Keep the returned hosted URL on the local verifier. + visualExternalHosts["litterbox"] = externalHostSpec{endpoint: server.URL, label: "litter.catbox.moe", withExpiry: true} publisher := ExternalHostVisualEvidencePublisher{Host: "litterbox", Expiry: "24h"} commentURL, err := publisher.PublishVisualEvidence(repo, "https://github.com/o/n/pull/9", "", manifest) if err != nil {