From a1947cd793626f6ab80998b23f02fef705601484 Mon Sep 17 00:00:00 2001 From: bigboateng Date: Wed, 22 Jul 2026 14:24:57 +0100 Subject: [PATCH] Add PR visual evidence contract --- .../boatstack-distribution/CONFIGURATION.md | 18 +- .../boatstack-distribution/GENERATED_FILES.md | 2 + .../boatstack-distribution/TROUBLESHOOTING.md | 4 + .../2026-07-22-pr-visual-evidence.md | 3 + .../product-engineering-loop/SKILL.md | 3 + .../assets/templates/plan.md | 5 + .../cmd/boatstack-helper/main.go | 54 ++- .../product-engineering-loop/export.go | 9 +- .../product-engineering-loop/migrate_test.go | 15 + .../product-engineering-loop/plan.go | 8 +- .../plan_validation.go | 73 ++- .../plan_validation_test.go | 38 +- .../product-engineering-loop/pr.go | 320 +++++++++++-- .../product-engineering-loop/pr_test.go | 97 +++- .../references/artifacts.md | 5 + .../references/config-schema.md | 2 + .../references/workflow.md | 6 + .../product-engineering-loop/runtime.go | 11 +- .../visual_evidence.go | 429 ++++++++++++++++++ .../visual_evidence_test.go | 196 ++++++++ .../project.example.json | 3 +- .../tests/test_product_loop.py | 8 +- 22 files changed, 1242 insertions(+), 67 deletions(-) create mode 100644 labs/12-product-engineering-loop/boatstack-distribution/release-notes/2026-07-22-pr-visual-evidence.md create mode 100644 labs/12-product-engineering-loop/product-engineering-loop/visual_evidence.go create mode 100644 labs/12-product-engineering-loop/product-engineering-loop/visual_evidence_test.go diff --git a/labs/12-product-engineering-loop/boatstack-distribution/CONFIGURATION.md b/labs/12-product-engineering-loop/boatstack-distribution/CONFIGURATION.md index 413589ebb..d8ca0271b 100644 --- a/labs/12-product-engineering-loop/boatstack-distribution/CONFIGURATION.md +++ b/labs/12-product-engineering-loop/boatstack-distribution/CONFIGURATION.md @@ -14,6 +14,7 @@ boatstack-config-field:workflow.independent_review_for_high_risk boatstack-config-field:workflow.allow_pass_with_gaps boatstack-config-field:workflow.maintain_changelog boatstack-config-field:workflow.boundary_analysis +boatstack-config-field:workflow.pr_visual_evidence boatstack-config-field:workspace boatstack-config-field:workspace.enabled boatstack-config-field:workspace.mode @@ -40,6 +41,7 @@ Boatstack keeps delivery policy in `.boatstack-project.json` so the same project | Allow a gate to pass with recorded gaps | `workflow.allow_pass_with_gaps` | A gate may report a pass with visible, retained gaps instead of requiring a gap-free result. | | Keep reader-facing release history | `workflow.maintain_changelog` | Every managed delivery slice and Boatstack-prepared ad-hoc PR must update `CHANGELOG.md`. | | Look for a missing systemic boundary | `workflow.boundary_analysis` | Planning checks whether the request is a local symptom and asks before expanding it into boundary work. | +| Attach fresh screenshots to frontend PRs | `workflow.pr_visual_evidence` | Boatstack structures visual review evidence without adding media or frontend tooling to Git. | | Start features in fresh Git workspaces | `workspace` | Boatstack can create a branch or linked worktree and manage local cleanup under the selected policy. | | Limit generated host adapters | `adapters` | Only the named Cursor, Claude Code, Codex, Gemini CLI, or GitHub surfaces are exported. | | Add supported specialist workflows | `integrations` | The installer records whether gstack or Spec Kit was requested and its installed state. | @@ -79,7 +81,8 @@ JSON does not support comments, so the explanations follow the example. "independent_review_for_high_risk": true, "allow_pass_with_gaps": true, "maintain_changelog": false, - "boundary_analysis": false + "boundary_analysis": false, + "pr_visual_evidence": "off" }, "workspace": { "enabled": true, @@ -141,6 +144,7 @@ The defaults below describe an omitted JSON field. A fresh installer-generated c | `allow_pass_with_gaps` | `false` | When `true`, verification may pass with explicitly recorded outstanding gaps. It does not hide or discard them. | | `maintain_changelog` | `false` | When `true`, requires a reader-visible `CHANGELOG.md` entry for every managed delivery slice and Boatstack-prepared ad-hoc PR. | | `boundary_analysis` | `false` | When `true`, planning checks whether a request indicates a missing systemic boundary. Scope expansion remains a material human decision; choosing programmatic enforcement produces a boundary slice followed by the feature slice. | +| `pr_visual_evidence` | `off` | `suggest` records missing relevant screenshots as a visible PR gap; `require` blocks completed PR publication until current screenshot evidence is available. Media remains machine-local until PR attachment. | ### `workspace` @@ -196,6 +200,18 @@ Add a categorized entry under `CHANGELOG.md`'s current `Unreleased` heading. See This adds a product decision when repository evidence suggests that a local request is a symptom of a broader missing boundary. It does not silently turn every feature into a refactor. +### Add screenshots to relevant pull requests + +```json +{ + "workflow": { + "pr_visual_evidence": "suggest" + } +} +``` + +`suggest` keeps delivery nonblocking when capture or attachment is unavailable and exposes the missing evidence as a PR gap. Use `require` only when every relevant frontend PR must publish current screenshots. Boatstack stores PNG bytes in Git-common machine state, never in the product tree. + ### Require independent review for high-risk paths ```json diff --git a/labs/12-product-engineering-loop/boatstack-distribution/GENERATED_FILES.md b/labs/12-product-engineering-loop/boatstack-distribution/GENERATED_FILES.md index 8d49d5bda..bbce3fd8e 100644 --- a/labs/12-product-engineering-loop/boatstack-distribution/GENERATED_FILES.md +++ b/labs/12-product-engineering-loop/boatstack-distribution/GENERATED_FILES.md @@ -46,6 +46,8 @@ When Boatstack improves a branch that did not use the full workflow, it stores t The preview's frontmatter is publication metadata; the remaining Markdown is the exact GitHub body. The preview is excluded from its own product-diff fingerprint, but any other diff or evidence change makes it stale. +PR schema v3 includes structural visual-evidence policy, status, count, and fingerprint fields. Screenshot binaries are never generated repository files: they live in Git-common Boatstack state until one PR evidence comment is published or manual attachment is required. Each imported PNG carries a `clean` or `human-reviewed` privacy receipt; upload observation is recorded separately so a failed comment can resume against the same PR. + ## Worktrees, fresh clones, and updates One verified runtime is cached under the clone's Git common directory and keyed by Boatstack version, source commit, operating system, and architecture. Linked worktrees share that cache. Their first guarded command atomically restores the ignored local helper and install lock, then evaluates the original command. Hydration uses no network and produces no tracked diff. diff --git a/labs/12-product-engineering-loop/boatstack-distribution/TROUBLESHOOTING.md b/labs/12-product-engineering-loop/boatstack-distribution/TROUBLESHOOTING.md index 7039c108e..a47b471e9 100644 --- a/labs/12-product-engineering-loop/boatstack-distribution/TROUBLESHOOTING.md +++ b/labs/12-product-engineering-loop/boatstack-distribution/TROUBLESHOOTING.md @@ -117,6 +117,10 @@ Boatstack found an installed generated file that no longer matches its previous A new commit, changed evidence, changed approval artifact, or base-branch update invalidated the preview. Ask Boatstack to regenerate it. Do not copy the old body forward. +## Visual evidence is unavailable or stale + +Confirm the development launch instruction and retry the bounded health probe. Boatstack reuses a machine capability receipt only while its Boatstack version, lockfile, launch command, browser version, framework configuration, and health state still match. Under `suggest`, keep the missing screenshot visible as a PR gap or attach the displayed local PNG manually. Under `require`, recapture and publish to the same PR; do not open a duplicate PR. If the PR already opened before upload failed, preserve it and fix forward from `visual_pending`. + ## A phased plan cannot push or open its next PR Plan approval is not publication authority. Run `delivery-status` through the active diff --git a/labs/12-product-engineering-loop/boatstack-distribution/release-notes/2026-07-22-pr-visual-evidence.md b/labs/12-product-engineering-loop/boatstack-distribution/release-notes/2026-07-22-pr-visual-evidence.md new file mode 100644 index 000000000..c4beacf3b --- /dev/null +++ b/labs/12-product-engineering-loop/boatstack-distribution/release-notes/2026-07-22-pr-visual-evidence.md @@ -0,0 +1,3 @@ +### Attach fresh visual evidence to frontend pull requests + +Repositories can opt into `workflow.pr_visual_evidence` with `suggest` or `require`. Boatstack now makes visual relevance and up to three PNG scenarios structural in managed plans and PR schema v3, binds screenshot hashes to the current PR context, keeps media in machine-local Git-common state, and supports one browser-published evidence comment with manual and fix-forward fallbacks. Existing repositories remain unchanged while the field is omitted or `off`. 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 9239406a4..bf8802ed7 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/SKILL.md +++ b/labs/12-product-engineering-loop/product-engineering-loop/SKILL.md @@ -95,6 +95,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. 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`. @@ -176,6 +177,7 @@ A published delivery is immutable. Record the observation against it, then plan - After build completes, the source Plan-mode file is no longer a runtime prerequisite. Test, review, and ship use the approved lock, actual diff, and accumulated evidence; provenance remains recorded in the lock. - 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. +- 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. - 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. @@ -196,6 +198,7 @@ A published delivery is immutable. Record the observation against it, then plan - 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. - 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/assets/templates/plan.md b/labs/12-product-engineering-loop/product-engineering-loop/assets/templates/plan.md index 1471769d9..57822c055 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/assets/templates/plan.md +++ b/labs/12-product-engineering-loop/product-engineering-loop/assets/templates/plan.md @@ -40,6 +40,11 @@ "blocks": ["T-1"] } ], + "pr_visual_evidence": { + "relevance": "not_relevant", + "reason": "", + "scenarios": [] + }, "tasks": [ { "id": "T-1", diff --git a/labs/12-product-engineering-loop/product-engineering-loop/cmd/boatstack-helper/main.go b/labs/12-product-engineering-loop/product-engineering-loop/cmd/boatstack-helper/main.go index 39e1cf5cb..25f98f642 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/cmd/boatstack-helper/main.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/cmd/boatstack-helper/main.go @@ -309,6 +309,52 @@ func recordDeliveryGateCommand(arguments []string) int { return 0 } +func recordPRVisualEvidenceCommand(arguments []string) int { + flags := flag.NewFlagSet("record-pr-visual-evidence", flag.ContinueOnError) + repo := flags.String("repo", ".", "repository whose Git-common state owns the evidence") + manifest := flags.String("manifest", "", "JSON manifest containing local PNG paths") + if err := flags.Parse(arguments); err != nil { + return 2 + } + if *manifest == "" { + return fail(fmt.Errorf("record-pr-visual-evidence requires --manifest")) + } + recorded, err := boatstack.ImportPRVisualEvidence(*repo, *manifest) + if err != nil { + return fail(err) + } + value, err := boatstack.MarshalJSON(recorded) + if err != nil { + return fail(err) + } + fmt.Print(string(value)) + return 0 +} + +func recordPRVisualPublicationCommand(arguments []string) int { + flags := flag.NewFlagSet("record-pr-visual-publication", flag.ContinueOnError) + repo := flags.String("repo", ".", "repository whose Git-common state owns the evidence") + key := flags.String("key", "", "managed feature or ad-hoc branch evidence key") + prURL := flags.String("pr-url", "", "published pull request URL") + commentURL := flags.String("comment-url", "", "observable Boatstack evidence comment URL") + if err := flags.Parse(arguments); err != nil { + return 2 + } + if *key == "" || *prURL == "" || *commentURL == "" { + return fail(fmt.Errorf("record-pr-visual-publication requires --key, --pr-url, and --comment-url")) + } + recorded, err := boatstack.RecordPRVisualPublication(*repo, *key, *prURL, *commentURL) + if err != nil { + return fail(err) + } + value, err := boatstack.MarshalJSON(recorded) + if err != nil { + return fail(err) + } + fmt.Print(string(value)) + return 0 +} + func deliveryStatusCommand(arguments []string) int { flags := flag.NewFlagSet("delivery-status", flag.ContinueOnError) repo := flags.String("repo", ".", "repository containing the managed delivery") @@ -656,7 +702,7 @@ func publishPRCommand(arguments []string) int { verb = "updated" } fmt.Printf("PASS: PR %s without merge authorization\nPR_URL=%s\n", verb, url) - + feature := "" if preview, err := boatstack.ParsePRPreview(*previewPath); err == nil { feature = preview.Feature @@ -735,7 +781,7 @@ func workspaceStatusCommand(arguments []string) int { func run() int { if len(os.Args) < 2 { - fmt.Fprintln(os.Stderr, "usage: boatstack-helper ") + fmt.Fprintln(os.Stderr, "usage: boatstack-helper ") return 2 } switch os.Args[1] { @@ -771,6 +817,10 @@ func run() int { return recordChangeCommand(os.Args[2:]) case "record-delivery-gate": return recordDeliveryGateCommand(os.Args[2:]) + case "record-pr-visual-evidence": + return recordPRVisualEvidenceCommand(os.Args[2:]) + case "record-pr-visual-publication": + return recordPRVisualPublicationCommand(os.Args[2:]) case "pr-context": return prContextCommand(os.Args[2:]) case "check-pr": 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 2ad05803f..0761c0505 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/export.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/export.go @@ -121,6 +121,9 @@ func ValidateConfig(config ProjectConfig) error { if err := validateWorkspaceConfig(config.Workspace); err != nil { return err } + if policy := strings.TrimSpace(config.Workflow.PRVisualEvidence); policy != "" && policy != "off" && policy != "suggest" && policy != "require" { + return fmt.Errorf("workflow.pr_visual_evidence must be \"off\", \"suggest\", or \"require\"") + } return nil } @@ -264,13 +267,13 @@ func BuildExportBundle(configPath string, config ProjectConfig, rawConfig []byte operations := map[string]string{ "boatstack-next": "Run the project-local helper next-status --repo . --json. This operation is strictly read-only: do not run the reported operation, edit artifacts, contact GitHub, or advance a gate. Translate the structured result into the canonical response contract. Show the verified feature and active slice when present. Distinguish NOT_STARTED and SOURCE_PLAN_READY, whose next operation is auto-plan, from FEATURE_COMPLETE, which responds Feature complete and requires no action. If verification_status is BLOCKED, name the ambiguity or invalid evidence and make its safe restoration the one action; never clear artifacts. Conversation, terminal, worktree, or process observations may be included as clearly labeled context only and must never override the repository-backed result. Otherwise make the returned next_operation the one next action.", "boatstack-run": "First run the read-only next-status --repo . --json. If SOURCE_PLAN_READY, execute auto-plan without Git preflight and pause at its normal decision or approval boundary. If NOT_STARTED, respond Start a Boatstack feature and ask the user to save exactly one host Plan-mode file, then run /auto-plan; do not fetch or require a feature branch. If FEATURE_COMPLETE, respond Feature complete with No action required without requiring a remote or fetching. Stop on UNVERIFIED, BLOCKED, ambiguous, stale, or invalid state. Before executing the first delivery-stage next_operation (build, repair, test-gate, review-gate, or ship-gate), run the project-local helper run-preflight --repo . --json; planning and plan-gate do not require it. Stop on a blocked preflight; never merge, rebase, force-push, discard changes, switch branches, or create a constrained delivery branch to repair freshness. Then execute exactly the verified next_operation using the canonical operation semantics, verify the resulting repository state, and resolve again. Continue across every declared delivery slice. Pause for the exact plan approval reply a, any material product decision, and the exact PR publication reply o or u; after a valid reply in the current host session, automatically continue the run. A run request never supplies approval or publication authority. For a same-intent test or review failure, use repair, record the observation, and retry from the returned stage, up to three complete automated repair-and-gate cycles for the active slice in this invocation. Stop immediately on an amendment, ambiguity, unsafe or destructive capability, stale evidence, branch mismatch, unsupported recovery, or exhausted repair budget. If Cursor reports MainThreadShellExec not initialized, explain that Cursor failed before the Boatstack hook started and make Developer: Reload Window the one recovery action; do not recommend reinstall unless Boatstack reports a missing, drifted, unsafe, or checksum-invalid runtime. Do not use conversation as workflow evidence and do not create durable autopilot state. Report the feature, active slice, stages completed during this invocation, completion or pause reason, repair-cycle count, and exactly one next action. Ship means publishing every declared slice PR for review; never merge or deploy.", - "auto-plan": "Discover exactly one saved Plan-mode file and refine it into a Markdown-only draft feature package whose canonical structured artifact is plan.md. Run check-plan read-only. If workflow.boundary_analysis is true, evaluate if the change is a symptom of a missing systemic boundary and perform a rapid codebase scan for other vulnerabilities. Present this as a material product decision with tiered paths: [1a] Symptom Patch or [1b] Programmatic Enforcement (Slice 1 for the boundary, Slice 2 for the feature). Record affected_paths and structured side_effects for external writes; use an immutable target identity, transactional or fix-forward recovery, and destructive=false. When workflow.maintain_changelog is true, include CHANGELOG.md in every delivery slice's affected paths. Keep internal phases as tasks in one delivery slice. Only when the accepted outcome explicitly needs multiple PRs, declare ordered delivery_slices and assign every task exactly once; plan approval never authorizes publication. Do not implement, create JSON or locks, or imply acceptance. If ready, respond with Plan ready and make Run /plan-gate the one next action. If decisions remain, respond with I need your input and ask only 1-3 material questions.", + "auto-plan": "Discover exactly one saved Plan-mode file and refine it into a Markdown-only draft feature package whose canonical structured artifact is plan.md. Run check-plan read-only. If workflow.boundary_analysis is true, evaluate if the change is a symptom of a missing systemic boundary and perform a rapid codebase scan for other vulnerabilities. Present this as a material product decision with tiered paths: [1a] Symptom Patch or [1b] Programmatic Enforcement (Slice 1 for the boundary, Slice 2 for the feature). When workflow.pr_visual_evidence is suggest or require, record a structural pr_visual_evidence decision: relevant with one to three entry/state/viewport/expected scenarios, or not_relevant with a reason. Discover existing visual tooling but never require a frontend framework or add repository tooling during planning. Record affected_paths and structured side_effects for external writes; use an immutable target identity, transactional or fix-forward recovery, and destructive=false. When workflow.maintain_changelog is true, include CHANGELOG.md in every delivery slice's affected paths. Keep internal phases as tasks in one delivery slice. Only when the accepted outcome explicitly needs multiple PRs, declare ordered delivery_slices and assign every task exactly once; plan approval never authorizes publication. Do not implement, create JSON or locks, or imply acceptance. If ready, respond with Plan ready and make Run /plan-gate the one next action. If decisions remain, respond with I need your input and ask only 1-3 material questions.", "plan-gate": "Run check-plan read-only, present its fingerprint and all open decisions, and require explicit human approval. While plan approval is pending, the normal user action is the exact standalone reply a. Trim surrounding whitespace and match a case-insensitively; do not treat [a] or an a embedded in other text as approval. Continue accepting the full reply approve for compatibility, but do not advertise it in the user-facing response. Resolve approved_by from an explicit supplied identity, otherwise from the authenticated GitHub login when available; ask one short identity follow-up only when neither exists, and never invent a placeholder name (e.g., Sam, Eve) and never infer it from a filesystem username, commit history, or agent identity. On approval invoke record-approval with the resolved human, RFC3339 timestamp, and exact displayed fingerprint so it writes only approval.md. While pending, respond Ready for your approval and render the one next action as: Reply `a` to approve. After recording, respond Approved — ready to build and make entering the host execution mode and running /build the one next action. Remain in Plan mode; do not compile or request an early mode switch.", "build": "First confirm the host is in an execution-capable mode. If the mode transition is rejected or product-code writes remain unavailable, return READY_FOR_BUILD internally without activating the plan, compiling JSON, or writing a lock. Only then locate plan.md and approval.md and run activate-plan before the first product-code edit. Stop if it reports BLOCKED. Read delivery-status and implement only the active delivery slice task_ids. When workflow.maintain_changelog is true, add a concise entry grounded in the active slice's actual changes under the current CHANGELOG.md Unreleased heading before recording test evidence. Use only the one allowed category needed by the entry and do not add empty category headings. If the file is absent, create the documented minimal skeleton with ## [Unreleased] - YYYY-MM-DD and the first categorized entry; if it exists, add to the current file without rewriting its history or layout. Run the internal repository safety check after operational or high-risk edits; a destructive capability blocks execution and gate progression but does not block reviewable source editing. Implementation tactics remain open inside the approved boundary, but push and PR mutation are never build tactics and are denied while managed delivery is active. On success respond Build complete and make Run /test-gate the one next action. When a new product decision blocks work, respond Build needs a decision and ask only that question.", "repair": "First run next-status --repo . --json. Repair requires an active managed delivery and the user's exact free-form requested change. If NOT_STARTED or SOURCE_PLAN_READY, respond No active delivery to repair and make /auto-plan the one next action; do not ask for repair details. If DRAFT_PLAN or APPROVED, route to the returned plan-gate or build operation because no managed delivery exists yet. If FEATURE_COMPLETE and the user supplied an exact correction, preserve the published evidence and plan a linked Boatstack feature with parent_delivery set to the completed feature; otherwise ask for the exact correction. Stop on BLOCKED or INVALID_STATE and preserve all artifacts. For an active delivery, read delivery-status, the current plan lock and acceptance criteria, the actual diff, and current receipts. Compare the exact request with approved intent. Classify it as implementation_repair, verification_repair, review_repair, requirement_amendment, or needs_clarification, then invoke record-change before any product edit. Same-intent repairs may proceed at the returned RESUME_STAGE; requirement amendments and ambiguous intent must stop for a concise plan amendment or one clarifying question. Never edit changes.md or managed delivery state directly. After a repair, reuse the existing /test-gate and /review-gate; do not invent repair-specific gates. 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. 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.", + "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 approved 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 rather than commits, PR metadata, artifacts, or test commands. On pass invoke record-delivery-gate for the same feature and slice with --gate review; it must reject a changed or untested diff and a missing or malformed required changelog entry. 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; add UI evidence, security/privacy, migration, or operations sections only when the diff makes them 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 or unresolved, show the exact fingerprinted local PNGs and 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. 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.", "boatstack-update": "Prepare a visible Boatstack infrastructure update; never mix it into product work or merge it. First run the current helper doctor and force check-update. If current, respond Boatstack is current with No action required. Before mutation fetch the default ref, then require the current clean default branch whose HEAD equals origin/; otherwise respond Update postponed and make finishing the current feature, switching to the clean default branch, and rerunning /boatstack-update the one action. Ensure no update PR or branch already exists, create chore/update-boatstack-v, then run the installer fetched from that exact release tag with BOATSTACK_MODE=update, BOATSTACK_VERSION=, BOATSTACK_REPO=, and BOATSTACK_YES=1. Use install.sh on macOS/Linux and install.ps1 on Windows. The verified update must preserve configuration, adapters, integrations, and user-owned host settings, run doctor, and touch only Boatstack infrastructure. Show the version transition, release notes and link, integration state, exact diff, 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 staging the installer-reported paths, committing chore: update Boatstack to , normal push, and opening a reviewer-ready update PR. If GitHub auth is unavailable, preserve the branch and give one manual publication action. 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/migrate_test.go b/labs/12-product-engineering-loop/product-engineering-loop/migrate_test.go index 35a31a00a..d6a3c4bbb 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/migrate_test.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/migrate_test.go @@ -214,6 +214,21 @@ func TestValidateConfig_AcceptanceTable(t *testing.T) { } } +func TestValidateConfigPRVisualEvidencePolicy(t *testing.T) { + for _, policy := range []string{"", "off", "suggest", "require"} { + config := testConfig() + config.Workflow.PRVisualEvidence = policy + if err := ValidateConfig(config); err != nil { + t.Fatalf("policy %q should be accepted: %v", policy, err) + } + } + config := testConfig() + config.Workflow.PRVisualEvidence = "sometimes" + if err := ValidateConfig(config); err == nil || !strings.Contains(err.Error(), "pr_visual_evidence") { + t.Fatalf("invalid policy was not rejected: %v", err) + } +} + func TestDoctor_SchemaBehindAndAhead(t *testing.T) { oldOverride := currentConfigSchemaVersionOverride defer func() { currentConfigSchemaVersionOverride = oldOverride }() 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 9a62ffa8d..85876c5ef 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/plan.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/plan.go @@ -406,7 +406,7 @@ func ValidatePlan(plan map[string]any, opts *ValidatePlanOptions) error { if !ok || (version != float64(1) && version != float64(2)) { return fmt.Errorf("schema_version must be 1 or 2") } - + if version == float64(2) { if err := validateArchitectureGrounding(plan, opts); err != nil { return err @@ -416,6 +416,12 @@ func ValidatePlan(plan map[string]any, opts *ValidatePlanOptions) error { if err := validateSystemicBoundaries(plan); err != nil { return err } + if err := validatePRVisualEvidence(plan); err != nil { + return err + } + if err := requireConfiguredPRVisualEvidenceDecision(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 714a283c1..4ba2bd508 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 @@ -33,7 +33,7 @@ func validateArchitectureGrounding(plan map[string]any, opts *ValidatePlanOption return fmt.Errorf("architecture fact ID must be present and unique") } factIDs[id] = true - + kind := stringValue(fact["kind"]) validKinds := map[string]bool{ "route_exists": true, "route_absent": true, "symbol_exists": true, @@ -48,7 +48,7 @@ func validateArchitectureGrounding(plan map[string]any, opts *ValidatePlanOption evidenceLevel := EvidenceVerified premiseStatus := PremiseValid evidenceIDs, ok := stringSlice(fact["evidence_ids"]) - + if !ok || len(evidenceIDs) == 0 { evidenceLevel = EvidenceAbsent } else { @@ -63,7 +63,7 @@ func validateArchitectureGrounding(plan map[string]any, opts *ValidatePlanOption if opts != nil && opts.RepoRevision != "" && record.RepositoryRevision != opts.RepoRevision { evidenceLevel = EvidenceSupported } - + // Basic operation check if kind == "route_absent" && record.Operation != "repository_search" && record.Operation != "route_lookup" { premiseStatus = PremiseInvalid @@ -115,7 +115,7 @@ func validateArchitectureGrounding(plan map[string]any, opts *ValidatePlanOption return fmt.Errorf("task %s references unknown architecture fact %s", id, req) } } - + // check blocked by unknowns for _, unk := range unknowns { blocks, _ := stringSlice(unk["blocks"]) @@ -153,4 +153,67 @@ func validateSystemicBoundaries(plan map[string]any) error { } } return nil -} \ No newline at end of file +} + +func validatePRVisualEvidence(plan map[string]any) error { + value, present := plan["pr_visual_evidence"] + if !present { + return nil + } + decision, ok := value.(map[string]any) + if !ok { + return fmt.Errorf("pr_visual_evidence must be an object") + } + relevance := stringValue(decision["relevance"]) + if relevance != "relevant" && relevance != "not_relevant" { + return fmt.Errorf("pr_visual_evidence.relevance must be relevant or not_relevant") + } + scenarios, ok := objectSlice(decision["scenarios"]) + if !ok && decision["scenarios"] != nil { + return fmt.Errorf("pr_visual_evidence.scenarios must be a list") + } + if relevance == "not_relevant" { + if stringValue(decision["reason"]) == "" { + return fmt.Errorf("not-relevant pr_visual_evidence requires a reason") + } + if len(scenarios) != 0 { + return fmt.Errorf("not-relevant pr_visual_evidence must not define scenarios") + } + return nil + } + if len(scenarios) == 0 || len(scenarios) > 3 { + return fmt.Errorf("relevant pr_visual_evidence requires one to three scenarios") + } + seen := map[string]bool{} + for _, scenario := range scenarios { + id := stringValue(scenario["id"]) + if id == "" || seen[id] { + return fmt.Errorf("pr_visual_evidence scenario ids must be present and unique") + } + seen[id] = true + for _, field := range []string{"entry", "state", "viewport"} { + if stringValue(scenario[field]) == "" { + return fmt.Errorf("pr_visual_evidence scenario %s requires %s", id, field) + } + } + expected, ok := stringSlice(scenario["expected"]) + if !ok || len(expected) == 0 { + return fmt.Errorf("pr_visual_evidence scenario %s requires expected visible outcomes", id) + } + } + return nil +} + +func requireConfiguredPRVisualEvidenceDecision(plan map[string]any, opts *ValidatePlanOptions) error { + if opts == nil || opts.RepoRoot == "" { + return nil + } + config, _, err := LoadConfig(filepath.Join(opts.RepoRoot, ".product-loop", "project.json")) + if err != nil || normalizedPRVisualEvidencePolicy(config.Workflow.PRVisualEvidence) == "off" { + return nil + } + if _, present := plan["pr_visual_evidence"]; !present { + return fmt.Errorf("configured workflow.pr_visual_evidence requires a structural plan decision") + } + return nil +} 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 e99b5a67d..9048db7ea 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 @@ -124,7 +124,7 @@ func TestPLAN_INVALIDExitsRepair(t *testing.T) { func TestValidateSystemicBoundaries(t *testing.T) { plan := validV2Plan() - + // Valid configuration plan["systemic_boundaries"] = []any{ map[string]any{ @@ -174,3 +174,39 @@ func TestValidateSystemicBoundaries(t *testing.T) { t.Fatalf("expected error for insufficient slices, got: %v", err) } } + +func TestValidatePRVisualEvidence(t *testing.T) { + plan := validV2Plan() + plan["pr_visual_evidence"] = map[string]any{ + "relevance": "relevant", + "scenarios": []any{map[string]any{ + "id": "warning", "entry": "/onboarding", "state": "picker open", "viewport": "1440x900", + "expected": []any{"warning visible"}, + }}, + } + if err := validatePRVisualEvidence(plan); err != nil { + t.Fatal(err) + } + plan["pr_visual_evidence"].(map[string]any)["scenarios"] = []any{} + if err := validatePRVisualEvidence(plan); err == nil || !strings.Contains(err.Error(), "one to three") { + t.Fatalf("empty relevant scenarios were not rejected: %v", err) + } + plan["pr_visual_evidence"] = map[string]any{"relevance": "not_relevant", "scenarios": []any{}} + if err := validatePRVisualEvidence(plan); err == nil || !strings.Contains(err.Error(), "reason") { + t.Fatalf("missing not-relevant reason was not rejected: %v", err) + } +} + +func TestConfiguredPRVisualEvidenceRequiresPlanDecision(t *testing.T) { + repo := prTestRepoConfigured(t, func(config *ProjectConfig) { + config.Workflow.PRVisualEvidence = "suggest" + }) + plan := validV2Plan() + if err := requireConfiguredPRVisualEvidenceDecision(plan, &ValidatePlanOptions{RepoRoot: repo}); err == nil || !strings.Contains(err.Error(), "structural plan decision") { + t.Fatalf("configured policy did not require a decision: %v", err) + } + plan["pr_visual_evidence"] = map[string]any{"relevance": "not_relevant", "reason": "backend-only", "scenarios": []any{}} + if err := requireConfiguredPRVisualEvidenceDecision(plan, &ValidatePlanOptions{RepoRoot: repo}); 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 06e7de2b7..afe086eb8 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/pr.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/pr.go @@ -11,9 +11,10 @@ import ( "sort" "strconv" "strings" + "time" ) -const prPreviewSchemaVersion = 2 +const prPreviewSchemaVersion = 3 var prStatusPattern = regexp.MustCompile(`(?i)^(PASS|PASS_WITH_GAPS|NOT_VERIFIED|BLOCKED)$`) @@ -31,44 +32,132 @@ type PRSource struct { } type PRContext struct { - SchemaVersion int `json:"schema_version"` - Mode string `json:"mode"` - Feature string `json:"feature,omitempty"` - SliceID string `json:"slice_id,omitempty"` - SliceIndex int `json:"slice_index,omitempty"` - TotalSlices int `json:"total_slices,omitempty"` - BaseBranch string `json:"base_branch"` - HeadBranch string `json:"head_branch"` - BaseCommit string `json:"base_commit"` - MergeBaseCommit string `json:"merge_base_commit"` - HeadCommit string `json:"head_commit"` - ProductDiffSHA256 string `json:"product_diff_sha256"` - ContextFingerprint string `json:"context_fingerprint"` - ChangedFiles []string `json:"changed_files"` - Commits []string `json:"commits"` - DiffStat string `json:"diff_stat"` - ContextPaths []string `json:"context_paths,omitempty"` - ProjectCommands map[string]string `json:"project_commands,omitempty"` - HighRiskFiles []string `json:"high_risk_files,omitempty"` - GateStatus map[string]string `json:"gate_status,omitempty"` - SafetyStatus string `json:"safety_status"` - SafetyFindings []SafetyFinding `json:"safety_findings,omitempty"` - Sources []PRSource `json:"sources,omitempty"` - PreviewPath string `json:"preview_path"` + SchemaVersion int `json:"schema_version"` + Mode string `json:"mode"` + Feature string `json:"feature,omitempty"` + SliceID string `json:"slice_id,omitempty"` + SliceIndex int `json:"slice_index,omitempty"` + TotalSlices int `json:"total_slices,omitempty"` + BaseBranch string `json:"base_branch"` + HeadBranch string `json:"head_branch"` + BaseCommit string `json:"base_commit"` + MergeBaseCommit string `json:"merge_base_commit"` + HeadCommit string `json:"head_commit"` + ProductDiffSHA256 string `json:"product_diff_sha256"` + ContextFingerprint string `json:"context_fingerprint"` + ChangedFiles []string `json:"changed_files"` + Commits []string `json:"commits"` + DiffStat string `json:"diff_stat"` + ContextPaths []string `json:"context_paths,omitempty"` + ProjectCommands map[string]string `json:"project_commands,omitempty"` + HighRiskFiles []string `json:"high_risk_files,omitempty"` + GateStatus map[string]string `json:"gate_status,omitempty"` + SafetyStatus string `json:"safety_status"` + SafetyFindings []SafetyFinding `json:"safety_findings,omitempty"` + PRVisualEvidencePolicy string `json:"pr_visual_evidence_policy"` + PRVisualEvidenceStatus string `json:"pr_visual_evidence_status"` + PRVisualEvidenceCount int `json:"pr_visual_evidence_count"` + PRVisualEvidenceFingerprint string `json:"pr_visual_evidence_fingerprint"` + PRVisualEvidenceRelevance string `json:"pr_visual_evidence_relevance"` + PRVisualEvidenceSource string `json:"pr_visual_evidence_source"` + PRVisualEvidence *PRVisualEvidenceManifest `json:"pr_visual_evidence,omitempty"` + Sources []PRSource `json:"sources,omitempty"` + PreviewPath string `json:"preview_path"` } type PRPreview struct { - SchemaVersion int - Title string - Mode string - Feature string - SliceID string - BaseBranch string - HeadBranch string - ContextFingerprint string - Body string - Path string - Fingerprint string + SchemaVersion int + Title string + Mode string + Feature string + SliceID string + BaseBranch string + HeadBranch string + ContextFingerprint string + PRVisualEvidencePolicy string + PRVisualEvidenceStatus string + PRVisualEvidenceCount int + PRVisualEvidenceFingerprint string + Body string + Path string + Fingerprint string +} + +func planVisualDecision(repo, feature string) (string, string, []PRVisualScenario, error) { + plan, err := LoadPlan(filepath.Join(repo, ".product-loop", "features", feature, "plan.md")) + if err != nil { + return "unresolved", "managed-plan", nil, err + } + value, ok := plan["pr_visual_evidence"].(map[string]any) + if !ok { + return "unresolved", "managed-plan", nil, nil + } + relevance := stringValue(value["relevance"]) + if relevance == "not_relevant" { + return relevance, "managed-plan", nil, nil + } + rows, _ := objectSlice(value["scenarios"]) + scenarios := make([]PRVisualScenario, 0, len(rows)) + for _, row := range rows { + expected, _ := stringSlice(row["expected"]) + scenarios = append(scenarios, PRVisualScenario{ + ID: stringValue(row["id"]), Entry: stringValue(row["entry"]), State: stringValue(row["state"]), + Viewport: stringValue(row["viewport"]), Expected: expected, + }) + } + return relevance, "managed-plan", scenarios, nil +} + +func resolvePRVisualEvidence(repo string, config ProjectConfig, mode, feature, head, headCommit, diffHash string) (string, string, int, string, string, string, *PRVisualEvidenceManifest, error) { + policy := normalizedPRVisualEvidencePolicy(config.Workflow.PRVisualEvidence) + relevance, source := "unresolved", "agent-proposed" + var scenarios []PRVisualScenario + if mode == "managed" { + var err error + relevance, source, scenarios, err = planVisualDecision(repo, feature) + if err != nil { + return "", "", 0, "", "", "", nil, err + } + } + key, err := visualEvidenceKey(mode, feature, head) + if err != nil { + return "", "", 0, "", "", "", nil, err + } + status := "NOT_APPLICABLE" + var manifest *PRVisualEvidenceManifest + if policy != "off" && relevance != "not_relevant" { + loaded, loadErr := LoadPRVisualEvidence(repo, key) + if loadErr == nil { + manifest = &loaded + relevance, source, scenarios = loaded.Relevance, loaded.RelevanceSource, loaded.Scenarios + if loaded.SourceCommit == headCommit && loaded.ProductDiffSHA256 == diffHash { + status = loaded.Status + } else { + status = "NOT_VERIFIED" + manifest = nil + } + } else { + status = "NOT_VERIFIED" + } + if policy == "require" && status != "PASS" { + status = "BLOCKED" + } + } + payload := map[string]any{ + "schema_version": visualEvidenceSchemaVersion, "policy": policy, "status": status, + "relevance": relevance, "relevance_source": source, "scenarios": scenarios, + } + count := 0 + if manifest != nil { + payload["manifest_fingerprint"] = manifest.Fingerprint + payload["items"] = manifest.Items + count = len(manifest.Items) + } + raw, err := MarshalJSON(payload) + if err != nil { + return "", "", 0, "", "", "", nil, err + } + return policy, status, count, SHA256Bytes(raw), relevance, source, manifest, nil } type PRPublishOptions struct { @@ -76,6 +165,56 @@ type PRPublishOptions struct { PreviewPath string ExpectedFingerprint string Action string + VisualPublisher PRVisualEvidencePublisher +} + +// PRVisualEvidencePublisher is implemented by a host that can upload exact +// machine-local PNG bytes to one Boatstack-owned pull-request comment. ExistingCommentURL +// is empty on first publication and lets later updates reuse the same comment. +type PRVisualEvidencePublisher interface { + PublishVisualEvidence(repo, prURL, existingCommentURL string, manifest PRVisualEvidenceManifest) (commentURL string, err error) +} + +func publishPRVisualEvidence(repo, prURL string, context PRContext, publisher PRVisualEvidencePublisher) error { + if context.PRVisualEvidenceStatus == "NOT_APPLICABLE" || context.PRVisualEvidence == nil { + return nil + } + manifest, err := LoadPRVisualEvidence(repo, context.PRVisualEvidence.Key) + if err != nil || manifest.Fingerprint != context.PRVisualEvidence.Fingerprint { + return fmt.Errorf("PR opened but visual evidence became stale; preserve the PR and recapture before updating it") + } + if manifest.Publication.State == "published" && manifest.Publication.PRURL == prURL && strings.TrimSpace(manifest.Publication.CommentURL) != "" { + return nil + } + 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", + }) + if recordErr != nil { + return fmt.Errorf("PR opened but manual visual-evidence fallback could not be recorded: %w", recordErr) + } + if context.PRVisualEvidencePolicy == "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 + } + commentURL, publishErr := publisher.PublishVisualEvidence(repo, prURL, manifest.Publication.CommentURL, manifest) + if publishErr != nil { + _, _ = recordPRVisualPublication(repo, manifest, PRVisualPublication{ + State: "visual_pending", PRURL: prURL, CommentURL: manifest.Publication.CommentURL, + UpdatedAt: now, Detail: publishErr.Error(), + }) + 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) == "" { + return fmt.Errorf("visual evidence publisher returned no observable comment URL") + } + _, err = recordPRVisualPublication(repo, manifest, PRVisualPublication{ + State: "published", PRURL: prURL, CommentURL: strings.TrimSpace(commentURL), UpdatedAt: now, + }) + return err } func commandOutput(repo string, name string, arguments ...string) (string, error) { @@ -478,6 +617,20 @@ func PreparePRContext(options PRContextOptions) (PRContext, error) { if err != nil { return PRContext{}, err } + visualPolicy, visualStatus, visualCount, visualFingerprint, visualRelevance, visualSource, visualManifest, err := resolvePRVisualEvidence( + repo, config, mode, options.Feature, head, headCommit, SHA256Bytes(diff), + ) + if err != nil { + return PRContext{}, err + } + fingerprintPayload, err = MarshalJSON(map[string]any{ + "base": json.RawMessage(fingerprintPayload), + "pr_visual_evidence_policy": visualPolicy, "pr_visual_evidence_status": visualStatus, + "pr_visual_evidence_count": visualCount, "pr_visual_evidence_fingerprint": visualFingerprint, + }) + if err != nil { + return PRContext{}, err + } return PRContext{ SchemaVersion: prPreviewSchemaVersion, Mode: mode, Feature: options.Feature, SliceID: sliceID, SliceIndex: sliceIndex, TotalSlices: totalSlices, @@ -488,7 +641,11 @@ func PreparePRContext(options PRContextOptions) (PRContext, error) { HighRiskFiles: highRiskChangedFiles(changed, config.Project.HighRiskPaths), GateStatus: gateStatus, Sources: sources, SafetyStatus: safety.Status, SafetyFindings: safety.Findings, - PreviewPath: previewPath, + PRVisualEvidencePolicy: visualPolicy, PRVisualEvidenceStatus: visualStatus, + PRVisualEvidenceCount: visualCount, PRVisualEvidenceFingerprint: visualFingerprint, + PRVisualEvidenceRelevance: visualRelevance, PRVisualEvidenceSource: visualSource, + PRVisualEvidence: visualManifest, + PreviewPath: previewPath, }, nil } @@ -506,6 +663,8 @@ func parsePRFrontmatter(value string) (map[string]string, string, error) { allowed := map[string]bool{ "boatstack_pr_version": true, "title": true, "mode": true, "feature": true, "slice": true, "base": true, "head": true, "context_fingerprint": true, + "pr_visual_evidence_policy": true, "pr_visual_evidence_status": true, + "pr_visual_evidence_count": true, "pr_visual_evidence_fingerprint": true, } for _, line := range strings.Split(frontmatter, "\n") { key, raw, found := strings.Cut(line, ":") @@ -520,7 +679,7 @@ func parsePRFrontmatter(value string) (map[string]string, string, error) { if _, exists := fields[key]; exists { return nil, "", fmt.Errorf("duplicate PR frontmatter field: %s", key) } - if key == "boatstack_pr_version" { + if key == "boatstack_pr_version" || key == "pr_visual_evidence_count" { fields[key] = raw continue } @@ -530,7 +689,7 @@ func parsePRFrontmatter(value string) (map[string]string, string, error) { } fields[key] = decoded } - for _, key := range []string{"boatstack_pr_version", "title", "mode", "feature", "base", "head", "context_fingerprint"} { + for _, key := range []string{"boatstack_pr_version", "title", "mode", "feature", "base", "head", "context_fingerprint", "pr_visual_evidence_policy", "pr_visual_evidence_status", "pr_visual_evidence_count", "pr_visual_evidence_fingerprint"} { if _, exists := fields[key]; !exists { return nil, "", fmt.Errorf("PR frontmatter is missing %s", key) } @@ -538,6 +697,35 @@ func parsePRFrontmatter(value string) (map[string]string, string, error) { return fields, body, nil } +func validateVisualEvidenceSection(body, status string, count int) error { + if status == "NOT_APPLICABLE" { + return nil + } + visual := section(body, "## Visual evidence") + if visual == "" { + return fmt.Errorf("PR body requires a non-empty Visual evidence section") + } + rows := 0 + for _, line := range strings.Split(visual, "\n") { + trimmed := strings.TrimSpace(line) + if !strings.HasPrefix(trimmed, "|") || strings.Contains(strings.ToLower(trimmed), "| scenario ") || strings.Contains(trimmed, "---") { + continue + } + cells := strings.Split(strings.Trim(trimmed, "|"), "|") + if len(cells) != 5 { + return fmt.Errorf("Visual evidence rows require Scenario, Viewport, Commit, Result, and Publication columns") + } + rows++ + } + if rows != count && status == "PASS" { + return fmt.Errorf("Visual evidence row count %d does not match pr_visual_evidence_count %d", rows, count) + } + if rows == 0 { + return fmt.Errorf("Visual evidence requires at least one structured row") + } + return nil +} + func section(value, heading string) string { start := strings.Index(value, heading) if start < 0 { @@ -636,7 +824,13 @@ func ParsePRPreview(path string) (PRPreview, error) { SchemaVersion: version, Title: strings.TrimSpace(fields["title"]), Mode: fields["mode"], Feature: fields["feature"], SliceID: fields["slice"], BaseBranch: fields["base"], HeadBranch: fields["head"], ContextFingerprint: fields["context_fingerprint"], Body: body, Path: path, - Fingerprint: SHA256Bytes(value), + PRVisualEvidencePolicy: fields["pr_visual_evidence_policy"], PRVisualEvidenceStatus: fields["pr_visual_evidence_status"], + PRVisualEvidenceFingerprint: fields["pr_visual_evidence_fingerprint"], + Fingerprint: SHA256Bytes(value), + } + preview.PRVisualEvidenceCount, err = strconv.Atoi(fields["pr_visual_evidence_count"]) + if err != nil || preview.PRVisualEvidenceCount < 0 || preview.PRVisualEvidenceCount > 3 { + return PRPreview{}, fmt.Errorf("pr_visual_evidence_count must be between 0 and 3") } if preview.Title == "" || strings.Contains(preview.Title, "\n") || len([]rune(preview.Title)) > 120 { return PRPreview{}, fmt.Errorf("PR title must be one non-empty line of at most 120 characters") @@ -662,6 +856,15 @@ func ParsePRPreview(path string) (PRPreview, error) { if len(preview.ContextFingerprint) != 64 { return PRPreview{}, fmt.Errorf("PR preview requires a valid context fingerprint") } + if preview.PRVisualEvidencePolicy != "off" && preview.PRVisualEvidencePolicy != "suggest" && preview.PRVisualEvidencePolicy != "require" { + return PRPreview{}, fmt.Errorf("unsupported pr_visual_evidence_policy") + } + if !map[string]bool{"PASS": true, "PASS_WITH_GAPS": true, "NOT_VERIFIED": true, "NOT_APPLICABLE": true, "BLOCKED": true}[preview.PRVisualEvidenceStatus] { + return PRPreview{}, fmt.Errorf("unsupported pr_visual_evidence_status") + } + if len(preview.PRVisualEvidenceFingerprint) != 64 { + return PRPreview{}, fmt.Errorf("PR preview requires a valid pr_visual_evidence_fingerprint") + } for _, heading := range []string{ "## Why this change", "## What changed", "## Review order", "## Evidence", "## Operational safety", "## Known gaps and risks", "## Rollout and rollback", @@ -676,6 +879,9 @@ func ParsePRPreview(path string) (PRPreview, error) { if err := validateEvidenceTable(body, preview.Mode); err != nil { return PRPreview{}, err } + if err := validateVisualEvidenceSection(body, preview.PRVisualEvidenceStatus, preview.PRVisualEvidenceCount); err != nil { + return PRPreview{}, err + } return preview, nil } @@ -721,7 +927,9 @@ func CheckPRPreview(repoPath, previewPath string) (PRPreview, PRContext, error) if filepath.Clean(expectedPath) != filepath.Clean(actualPath) { return PRPreview{}, PRContext{}, fmt.Errorf("PR preview must be stored at %s", context.PreviewPath) } - if preview.Mode != context.Mode || preview.SliceID != context.SliceID || preview.BaseBranch != context.BaseBranch || preview.HeadBranch != context.HeadBranch || preview.ContextFingerprint != context.ContextFingerprint { + if preview.Mode != context.Mode || preview.SliceID != context.SliceID || preview.BaseBranch != context.BaseBranch || preview.HeadBranch != context.HeadBranch || preview.ContextFingerprint != context.ContextFingerprint || + preview.PRVisualEvidencePolicy != context.PRVisualEvidencePolicy || preview.PRVisualEvidenceStatus != context.PRVisualEvidenceStatus || + preview.PRVisualEvidenceCount != context.PRVisualEvidenceCount || preview.PRVisualEvidenceFingerprint != context.PRVisualEvidenceFingerprint { return PRPreview{}, PRContext{}, fmt.Errorf("PR preview is stale or does not match the current branch context; regenerate it") } if context.Mode == "managed" { @@ -794,6 +1002,9 @@ func PublishPR(options PRPublishOptions) (string, error) { if options.Action != "open" && options.Action != "update" { return "", fmt.Errorf("publication action must be open or update") } + if context.PRVisualEvidencePolicy == "require" && context.PRVisualEvidenceStatus != "PASS" { + return "", fmt.Errorf("PR publication is blocked until required visual evidence is current") + } dirty, err := dirtyPaths(repo) if err != nil { return "", err @@ -838,19 +1049,26 @@ func PublishPR(options PRPublishOptions) (string, error) { if err != nil { return "", err } + url = strings.TrimSpace(url) + if err := publishPRVisualEvidence(repo, url, context, options.VisualPublisher); err != nil { + return "", err + } if context.Mode == "managed" { - if err := MarkDeliveryPublished(repo, context.Feature, context.SliceID, strings.TrimSpace(url)); err != nil { + if err := MarkDeliveryPublished(repo, context.Feature, context.SliceID, url); err != nil { return "", fmt.Errorf("PR opened but delivery state could not advance: %w", err) } if err := extractSystemicBoundaries(repo, context.Feature); err != nil { fmt.Fprintf(os.Stderr, "WARNING: could not extract systemic boundaries: %v\n", err) } } - return strings.TrimSpace(url), nil + return url, nil } if _, err := commandOutput(repo, "gh", "pr", "edit", existingURL, "--title", preview.Title, "--body-file", temporaryPath); err != nil { return "", err } + if err := publishPRVisualEvidence(repo, existingURL, context, options.VisualPublisher); err != nil { + return "", err + } if context.Mode == "managed" { if err := MarkDeliveryPublished(repo, context.Feature, context.SliceID, existingURL); err != nil { return "", fmt.Errorf("PR updated but delivery state could not advance: %w", err) @@ -882,7 +1100,7 @@ func extractSystemicBoundaries(repo, feature string) error { return err } defer f.Close() - + for _, b := range boundaries { boundary, _ := b.(map[string]any) id := stringValue(boundary["id"]) @@ -903,7 +1121,7 @@ func PRPreviewTemplate(context PRContext) string { safetySummary := "Repository safety scan: `" + context.SafetyStatus + "`. Destructive recovery remains operator-only outside Boatstack." lines := []string{ "---", - "boatstack_pr_version: 2", + "boatstack_pr_version: 3", "title: " + quote("Describe the product or user value of this change (e.g., 'Enable historical data migration')"), "mode: " + quote(context.Mode), "feature: " + quote(context.Feature), @@ -911,6 +1129,10 @@ func PRPreviewTemplate(context PRContext) string { "base: " + quote(context.BaseBranch), "head: " + quote(context.HeadBranch), "context_fingerprint: " + quote(context.ContextFingerprint), + "pr_visual_evidence_policy: " + quote(context.PRVisualEvidencePolicy), + "pr_visual_evidence_status: " + quote(context.PRVisualEvidenceStatus), + fmt.Sprintf("pr_visual_evidence_count: %d", context.PRVisualEvidenceCount), + "pr_visual_evidence_fingerprint: " + quote(context.PRVisualEvidenceFingerprint), "---", "## Why this change", "", "Explain the user or engineering outcome.", "", "## What changed", "", "| Area | Before | After | Reviewer focus |", "|---|---|---|---|", "| | | | |", "", @@ -920,6 +1142,14 @@ func PRPreviewTemplate(context PRContext) string { "## Known gaps and risks", "", "List explicit gaps or say that no material gaps are known.", "", "## Rollout and rollback", "", "Describe deployment impact and the smallest safe rollback.", "", } + if context.PRVisualEvidenceStatus != "NOT_APPLICABLE" { + lines = append(lines, + "## Visual evidence", "", + "Screenshots are human-review evidence, not mechanical proof. Public-repository attachments are publicly accessible.", "", + "| Scenario | Viewport | Commit | Result | Publication |", "|---|---|---|---|---|", + "| Describe the approved state | viewport | "+context.HeadCommit+" | `"+context.PRVisualEvidenceStatus+"` | Boatstack evidence comment or manual fallback |", "", + ) + } if context.TotalSlices > 1 { lines = append(lines, fmt.Sprintf("> *(This is PR %d of %d in the `%s` feature)*", context.SliceIndex, context.TotalSlices, context.Feature), "") } 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 d2b81fec8..633d7f1d8 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 @@ -6,6 +6,7 @@ import ( "os/exec" "path/filepath" "runtime" + "strconv" "strings" "testing" ) @@ -75,7 +76,7 @@ func quoted(value string) string { func previewDocument(context PRContext, title, body string) string { return strings.Join([]string{ "---", - "boatstack_pr_version: 2", + "boatstack_pr_version: 3", "title: " + quoted(title), "mode: " + quoted(context.Mode), "feature: " + quoted(context.Feature), @@ -83,6 +84,10 @@ func previewDocument(context PRContext, title, body string) string { "base: " + quoted(context.BaseBranch), "head: " + quoted(context.HeadBranch), "context_fingerprint: " + quoted(context.ContextFingerprint), + "pr_visual_evidence_policy: " + quoted(context.PRVisualEvidencePolicy), + "pr_visual_evidence_status: " + quoted(context.PRVisualEvidenceStatus), + "pr_visual_evidence_count: " + strconv.Itoa(context.PRVisualEvidenceCount), + "pr_visual_evidence_fingerprint: " + quoted(context.PRVisualEvidenceFingerprint), "---", strings.TrimSpace(body), "", @@ -158,6 +163,19 @@ func activateManagedFeature(t *testing.T, repo, feature string) string { plan := validPlan() plan["feature_id"] = feature plan["spec_path"] = "feature-spec.md" + config, _, err := LoadConfig(filepath.Join(repo, ".product-loop", "project.json")) + if err != nil { + t.Fatal(err) + } + if normalizedPRVisualEvidencePolicy(config.Workflow.PRVisualEvidence) != "off" { + plan["pr_visual_evidence"] = map[string]any{ + "relevance": "relevant", + "scenarios": []any{map[string]any{ + "id": "warning", "entry": "/onboarding", "state": "picker open", "viewport": "1440x900", + "expected": []any{"warning visible"}, + }}, + } + } if err := os.WriteFile(filepath.Join(directory, "source-plan.md"), []byte("# Host plan\n"), 0o644); err != nil { t.Fatal(err) } @@ -387,6 +405,83 @@ func TestPRPreviewRejectsMissingSectionsMalformedRowsAndStaleDiff(t *testing.T) } } +func visualEvidenceBody(base, status string) string { + return strings.TrimSpace(base) + ` + +## Visual evidence + +Screenshots are human-review evidence, not mechanical proof. + +| Scenario | Viewport | Commit | Result | Publication | +|---|---|---|---|---| +| Sheet picker warning | 1440x900 | current | ` + "`" + status + "`" + ` | Boatstack evidence comment or manual fallback | +` +} + +func TestPRVisualEvidenceIsStructuralForManagedAndAdHocPreviews(t *testing.T) { + repo := prTestRepoConfigured(t, func(config *ProjectConfig) { + config.Workflow.PRVisualEvidence = "suggest" + }) + context, err := PreparePRContext(PRContextOptions{Repo: repo}) + if err != nil { + t.Fatal(err) + } + if context.PRVisualEvidencePolicy != "suggest" || context.PRVisualEvidenceStatus != "NOT_VERIFIED" || context.PRVisualEvidenceCount != 0 { + t.Fatalf("unexpected visual evidence context: %#v", context) + } + body := visualEvidenceBody(fixturePRBody(t), context.PRVisualEvidenceStatus) + previewPath := writePreview(t, repo, context, "Expose visual review evidence", body) + if _, _, err := CheckPRPreview(repo, previewPath); err != nil { + t.Fatal(err) + } + withoutVisual := strings.Replace(body, "## Visual evidence", "## Screenshot notes", 1) + if err := os.WriteFile(previewPath, []byte(previewDocument(context, "Expose visual review evidence", withoutVisual)), 0o644); err != nil { + t.Fatal(err) + } + if _, err := ParsePRPreview(previewPath); err == nil || !strings.Contains(err.Error(), "Visual evidence") { + t.Fatalf("missing structural visual section was not rejected: %v", err) + } +} + +func TestManagedPRVisualEvidenceUsesApprovedPlanScenarios(t *testing.T) { + repo := prTestRepoConfigured(t, func(config *ProjectConfig) { + config.Workflow.PRVisualEvidence = "suggest" + }) + activateManagedFeature(t, repo, "reviewer-ready") + context, err := PreparePRContext(PRContextOptions{Repo: repo, Feature: "reviewer-ready"}) + if err != nil { + t.Fatal(err) + } + if context.Mode != "managed" || context.PRVisualEvidenceRelevance != "relevant" || context.PRVisualEvidenceSource != "managed-plan" || context.PRVisualEvidenceStatus != "NOT_VERIFIED" { + t.Fatalf("managed visual decision was not projected: %#v", context) + } + previewPath := writePreview(t, repo, context, "Expose approved visual review scenario", visualEvidenceBody(managedPRBody(), context.PRVisualEvidenceStatus)) + if _, _, err := CheckPRPreview(repo, previewPath); err != nil { + t.Fatal(err) + } +} + +func TestRequiredPRVisualEvidenceBlocksPublicationBeforeMutation(t *testing.T) { + repo := prTestRepoConfigured(t, func(config *ProjectConfig) { + config.Workflow.PRVisualEvidence = "require" + }) + context, err := PreparePRContext(PRContextOptions{Repo: repo}) + if err != nil { + t.Fatal(err) + } + if context.PRVisualEvidenceStatus != "BLOCKED" { + t.Fatalf("required missing evidence should block, got %s", context.PRVisualEvidenceStatus) + } + previewPath := writePreview(t, repo, context, "Require visual review evidence", visualEvidenceBody(fixturePRBody(t), context.PRVisualEvidenceStatus)) + preview, _, err := CheckPRPreview(repo, previewPath) + if err != nil { + t.Fatal(err) + } + if _, err := PublishPR(PRPublishOptions{Repo: repo, PreviewPath: previewPath, ExpectedFingerprint: preview.Fingerprint, Action: "open"}); err == nil || !strings.Contains(err.Error(), "required visual evidence") { + t.Fatalf("required visual evidence did not block publication: %v", err) + } +} + func TestPublishPRRequiresExactConfirmationAndUsesBodyWithoutFrontmatter(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("fake gh fixture uses a POSIX shell; publication behavior is covered by cross-platform pure-Go checks") diff --git a/labs/12-product-engineering-loop/product-engineering-loop/references/artifacts.md b/labs/12-product-engineering-loop/product-engineering-loop/references/artifacts.md index a5fb53255..64d7f62d1 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/references/artifacts.md +++ b/labs/12-product-engineering-loop/product-engineering-loop/references/artifacts.md @@ -23,6 +23,7 @@ Artifacts separate facts, decisions, unknowns, incompleteness, and evidence. Com | Side-effect declaration | Affected paths, immutable external target, reversibility, failure policy, and destructive flag | A task can write outside the repository | | Runbook | Deploy, observe, recover, and roll back | Operational behavior changes | | Evidence ledger | Commands, results, review evidence, screenshots, CI and runtime links | Every gate | +| PR visual manifest | Machine-local scenario, source revision, screenshot hashes, capture metadata, and publication state | Relevant PR capture and publication | | PR preview | Exact reviewer-ready title/body plus a hidden fingerprint of the committed diff and evidence | Ship gate, before opening or updating GitHub | | Move ledger | Failure class, intervention, prediction, paired result, decision | Improving the loop itself | @@ -87,6 +88,10 @@ ledger while the publisher rechecks the matching receipts. The generated host hook fragments and launchers are committed installation infrastructure. Their policy is immutable in project configuration. The machine-local helper is ignored and restored by the installer. Safety evidence belongs in the feature evidence ledger: target identity, failure behavior, independent oracle, operational-diff scan, and the operator-only recovery boundary. A source edit is reviewable evidence, not permission to execute it. +## PR visual evidence boundary + +When `workflow.pr_visual_evidence` is enabled, the approved plan records whether screenshots are relevant and names no more than three review scenarios. PNG bytes and capability receipts live under Git-common Boatstack state; committed ledgers retain only compact metadata and hashes. PR schema v3 binds the policy, status, count, and manifest fingerprint to the preview. Screenshots are human-review evidence rather than mechanical correctness proof. + ## Templates Copy only the templates required for the current slice from `assets/templates/`. Do not create empty ceremony. The feature spec, question ledger, test plan, gap ledger, and evidence ledger are the usual minimum for material product work. 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 bf392c8e8..1fad7a96d 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 @@ -14,6 +14,7 @@ boatstack-config-field:workflow.independent_review_for_high_risk boatstack-config-field:workflow.allow_pass_with_gaps boatstack-config-field:workflow.maintain_changelog boatstack-config-field:workflow.boundary_analysis +boatstack-config-field:workflow.pr_visual_evidence boatstack-config-field:workspace boatstack-config-field:workspace.enabled boatstack-config-field:workspace.mode @@ -61,6 +62,7 @@ This reference document defines the schema and version history of `.boatstack-pr - `allow_pass_with_gaps` (boolean, optional): Whether the delivery verification allows outstanding questions or gaps. - `maintain_changelog` (boolean, optional): Whether a reader-visible `CHANGELOG.md` entry is required for each delivery slice. - `boundary_analysis` (boolean, optional): Whether planning checks for a missing systemic boundary and 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. ### workspace Fields 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 7dc207462..d3d86732e 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 @@ -238,6 +238,8 @@ Validation must be derived before implementation. Each check records: Subjective work is not exempt from validation. Convert ambiguity into an approved reference, rubric, scenario, threshold, and evidence owner. If materially different interpretations remain or no defensible oracle exists, keep the plan `BLOCKED` at `PLAN_GATE`. +When `workflow.pr_visual_evidence` is `suggest` or `require`, every managed plan also records a `pr_visual_evidence` decision. A relevant decision defines one to three scenarios with an entry surface, required state, viewport, and expected visible outcomes. A not-relevant decision records its reason. Planning may discover repository-owned visual tooling but must not require Storybook, Playwright, or another framework-specific dependency. + ### `PLAN -> PLAN_GATE` Run `boatstack-helper check-plan --plan /plan.md`, present the full draft and returned fingerprint, then require an exact standalone `a`, the compatible full reply `approve`, or a change request. End the pending user-facing response with exactly this Markdown: Reply `a` to approve. The check is read-only. Do not interpret silence, `[a]`, an `a` embedded in other text, a new implementation question, a tool permission, or permission to build as plan approval. @@ -308,6 +310,8 @@ Create requirement-to-evidence traceability. Use this evidence ladder: The riskier the slice, the less acceptable same-model, self-authored tests are as the only oracle. +For relevant visual scenarios, resolve capture capability in this order: repository-owned visual tooling, a host browser against the existing development server, one human-supplied launch instruction, then explicit machine-local runtime setup. Capture must not edit source, dependency manifests, lockfiles, or test configuration. Bind each PNG to the current commit, product diff, scenario, viewport, SHA-256, and a `clean` or `human-reviewed` privacy receipt. `suggest` records unavailable capture as a visible gap; `require` retains a ship blocker. + External-write evidence must establish immutable target identity, transactional or fix-forward behavior, and an independent safety oracle. A dry run that only prints the intended command does not prove the live target or failure behavior. Before passing the gate, commit the intentional active-slice product and evidence diff @@ -353,6 +357,8 @@ 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. + 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 perform a normal push and the selected GitHub action. Any drift blocks publication and requires a new preview; never force-push. For managed work, publication also requires current test and review receipts for the 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 cc4b558d2..8d63abfcc 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/runtime.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/runtime.go @@ -47,11 +47,12 @@ type Project struct { } type Workflow struct { - HumanPlanApproval bool `json:"human_plan_approval"` - IndependentReviewForHighRisk bool `json:"independent_review_for_high_risk"` - AllowPassWithGaps bool `json:"allow_pass_with_gaps"` - MaintainChangelog bool `json:"maintain_changelog"` - BoundaryAnalysis bool `json:"boundary_analysis,omitempty"` + HumanPlanApproval bool `json:"human_plan_approval"` + IndependentReviewForHighRisk bool `json:"independent_review_for_high_risk"` + AllowPassWithGaps bool `json:"allow_pass_with_gaps"` + MaintainChangelog bool `json:"maintain_changelog"` + BoundaryAnalysis bool `json:"boundary_analysis,omitempty"` + PRVisualEvidence string `json:"pr_visual_evidence,omitempty"` } type IntegrationState struct { 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 new file mode 100644 index 000000000..ca4de7e8d --- /dev/null +++ b/labs/12-product-engineering-loop/product-engineering-loop/visual_evidence.go @@ -0,0 +1,429 @@ +package boatstack + +import ( + "bytes" + "context" + "fmt" + "image/png" + "net/http" + "os" + "path/filepath" + "sort" + "strings" + "time" +) + +const visualEvidenceSchemaVersion = 1 + +type PRVisualScenario struct { + ID string `json:"id"` + Entry string `json:"entry"` + State string `json:"state"` + Viewport string `json:"viewport"` + Expected []string `json:"expected"` +} + +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"` +} + +type PRVisualPublication struct { + State string `json:"state"` + PRURL string `json:"pr_url,omitempty"` + CommentURL string `json:"comment_url,omitempty"` + UpdatedAt string `json:"updated_at,omitempty"` + Detail string `json:"detail,omitempty"` +} + +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"` +} + +type PRVisualCapabilityReceipt struct { + SchemaVersion int `json:"schema_version"` + BoatstackVersion string `json:"boatstack_version"` + LockfileSHA256 string `json:"lockfile_sha256,omitempty"` + LaunchCommandHash string `json:"launch_command_sha256,omitempty"` + BrowserVersion string `json:"browser_version,omitempty"` + FrameworkConfigSHA string `json:"framework_config_sha256,omitempty"` + HealthStatus string `json:"health_status"` + VerifiedAt string `json:"verified_at"` +} + +type PRVisualCaptureCapability struct { + Kind string `json:"kind"` + Command string `json:"command,omitempty"` +} + +// ResolvePRVisualCaptureCapability implements the portable capability cut. It +// selects repository-owned tooling before host or machine-local capabilities. +func ResolvePRVisualCaptureCapability(repo string, config ProjectConfig, hostBrowser bool, suppliedLaunch string, expectedReceipt PRVisualCapabilityReceipt) (PRVisualCaptureCapability, error) { + for _, name := range []string{"visual", "screenshot", "e2e"} { + if command := strings.TrimSpace(config.Project.Commands[name]); command != "" { + return PRVisualCaptureCapability{Kind: "repository-command", Command: command}, nil + } + } + if hostBrowser { + return PRVisualCaptureCapability{Kind: "host-browser"}, nil + } + if suppliedLaunch = strings.TrimSpace(suppliedLaunch); suppliedLaunch != "" { + return PRVisualCaptureCapability{Kind: "supplied-launch", Command: suppliedLaunch}, nil + } + if _, err := LoadPRVisualCapability(repo, expectedReceipt); err == nil { + return PRVisualCaptureCapability{Kind: "machine-runtime"}, nil + } + return PRVisualCaptureCapability{Kind: "unavailable"}, nil +} + +func ProbePRVisualReadiness(parent context.Context, url string, timeout time.Duration) error { + if timeout <= 0 { + timeout = 15 * time.Second + } + ctx, cancel := context.WithTimeout(parent, timeout) + defer cancel() + request, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + if err != nil { + return err + } + response, err := (&http.Client{Timeout: timeout}).Do(request) + if err != nil { + return fmt.Errorf("visual readiness probe failed: %w", err) + } + defer response.Body.Close() + if response.StatusCode < 200 || response.StatusCode >= 400 { + return fmt.Errorf("visual readiness probe returned HTTP %d", response.StatusCode) + } + return nil +} + +func normalizedPRVisualEvidencePolicy(value string) string { + value = strings.TrimSpace(value) + if value == "" { + return "off" + } + return value +} + +func visualEvidenceKey(mode, feature, head string) (string, error) { + key := feature + if mode == "ad-hoc" { + key = previewSlug(head) + } + return safeCacheSegment(key, "visual evidence key") +} + +func visualEvidenceDirectory(repo, key string) (string, error) { + key, err := safeCacheSegment(key, "visual evidence key") + if err != nil { + return "", err + } + common, err := gitCommonDir(repo) + if err != nil { + return "", err + } + return filepath.Join(common, "boatstack", "visual-evidence", key), nil +} + +func visualEvidenceManifestPath(repo, key string) (string, error) { + directory, err := visualEvidenceDirectory(repo, key) + if err != nil { + return "", err + } + return filepath.Join(directory, "manifest.json"), nil +} + +func visualCapabilityPath(repo string) (string, error) { + common, err := gitCommonDir(repo) + if err != nil { + return "", err + } + return filepath.Join(common, "boatstack", "visual-evidence", "capability.json"), nil +} + +func visualManifestFingerprint(manifest PRVisualEvidenceManifest) (string, error) { + copy := manifest + copy.Fingerprint = "" + raw, err := MarshalJSON(copy) + if err != nil { + return "", err + } + return SHA256Bytes(raw), nil +} + +func validateVisualManifest(manifest PRVisualEvidenceManifest) error { + if manifest.SchemaVersion != visualEvidenceSchemaVersion { + return fmt.Errorf("visual evidence schema_version must be %d", visualEvidenceSchemaVersion) + } + if _, err := safeCacheSegment(manifest.Key, "visual evidence key"); err != nil { + return err + } + policy := normalizedPRVisualEvidencePolicy(manifest.Policy) + if policy != "off" && policy != "suggest" && policy != "require" { + return fmt.Errorf("visual evidence policy must be off, suggest, or require") + } + if manifest.Relevance != "relevant" && manifest.Relevance != "not_relevant" && manifest.Relevance != "unresolved" { + return fmt.Errorf("visual evidence relevance must be relevant, not_relevant, or unresolved") + } + if manifest.RelevanceSource != "managed-plan" && manifest.RelevanceSource != "human-provided" && manifest.RelevanceSource != "repository-evidenced" && manifest.RelevanceSource != "agent-proposed" { + return fmt.Errorf("unsupported visual evidence relevance source") + } + if manifest.Relevance == "not_relevant" && strings.TrimSpace(manifest.Reason) == "" { + return fmt.Errorf("not-relevant visual evidence requires a reason") + } + if len(manifest.Scenarios) > 3 || len(manifest.Items) > 3 { + return fmt.Errorf("visual evidence supports at most three scenarios and screenshots") + } + allowedStatus := map[string]bool{"PASS": true, "PASS_WITH_GAPS": true, "NOT_VERIFIED": true, "NOT_APPLICABLE": true, "BLOCKED": true} + if !allowedStatus[manifest.Status] { + return fmt.Errorf("unsupported visual evidence status %q", manifest.Status) + } + seenScenarios := map[string]bool{} + scenarioViewports := map[string]string{} + for _, scenario := range manifest.Scenarios { + if scenario.ID == "" || seenScenarios[scenario.ID] || scenario.Entry == "" || scenario.State == "" || scenario.Viewport == "" || len(scenario.Expected) == 0 { + return fmt.Errorf("visual evidence scenarios require unique ids, entry, state, viewport, and expected outcomes") + } + seenScenarios[scenario.ID] = true + scenarioViewports[scenario.ID] = scenario.Viewport + } + seenItems := map[string]bool{} + for _, item := range manifest.Items { + if !seenScenarios[item.ScenarioID] || seenItems[item.ScenarioID] || item.MIMEType != "image/png" || item.DurationMS != 0 || item.SHA256 == "" || item.Width < 1 || item.Height < 1 { + 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] { + return fmt.Errorf("visual evidence items require captured status and the approved scenario viewport") + } + if item.PrivacyStatus != "clean" && item.PrivacyStatus != "human-reviewed" { + return fmt.Errorf("visual evidence items require privacy_status clean or human-reviewed") + } + if _, err := time.Parse(time.RFC3339, item.CapturedAt); err != nil { + return fmt.Errorf("visual evidence captured_at must be RFC3339: %w", err) + } + } + 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") + } + return nil +} + +// SavePRVisualEvidence copies exact PNG bytes into Git-common Boatstack state, +// normalizes their metadata, and atomically records a fingerprinted manifest. +func SavePRVisualEvidence(repo string, manifest PRVisualEvidenceManifest) (PRVisualEvidenceManifest, error) { + repo, err := ResolveRepository(repo) + if err != nil { + return PRVisualEvidenceManifest{}, err + } + manifest.SchemaVersion = visualEvidenceSchemaVersion + manifest.Policy = normalizedPRVisualEvidencePolicy(manifest.Policy) + directory, err := visualEvidenceDirectory(repo, manifest.Key) + if err != nil { + return PRVisualEvidenceManifest{}, err + } + if err := rejectSymlinkComponents(filepath.Dir(filepath.Dir(directory)), directory); err != nil { + return PRVisualEvidenceManifest{}, err + } + if previous, loadErr := LoadPRVisualEvidence(repo, manifest.Key); loadErr == nil && manifest.Publication.CommentURL == "" { + manifest.Publication.PRURL = previous.Publication.PRURL + manifest.Publication.CommentURL = previous.Publication.CommentURL + if manifest.Publication.State == "" { + manifest.Publication.State = "pending" + } + } + for index := range manifest.Items { + item := &manifest.Items[index] + info, err := os.Lstat(item.Path) + if err != nil || !info.Mode().IsRegular() || info.Mode()&os.ModeSymlink != 0 { + return PRVisualEvidenceManifest{}, fmt.Errorf("visual evidence PNG is missing or unsafe: %s", item.Path) + } + value, err := os.ReadFile(item.Path) + if err != nil { + return PRVisualEvidenceManifest{}, err + } + configuration, err := png.DecodeConfig(bytes.NewReader(value)) + if err != nil { + return PRVisualEvidenceManifest{}, fmt.Errorf("visual evidence must be a valid PNG: %w", err) + } + hash := SHA256Bytes(value) + destination := filepath.Join(directory, "assets", hash+".png") + if err := atomicWriteMode(destination, value, 0o600); err != nil { + return PRVisualEvidenceManifest{}, err + } + item.Path = destination + item.SHA256 = hash + item.MIMEType = "image/png" + item.Width = configuration.Width + item.Height = configuration.Height + item.DurationMS = 0 + } + sort.Slice(manifest.Items, func(i, j int) bool { return manifest.Items[i].ScenarioID < manifest.Items[j].ScenarioID }) + manifest.Fingerprint = "" + if err := validateVisualManifest(manifest); err != nil { + return PRVisualEvidenceManifest{}, err + } + manifest.Fingerprint, err = visualManifestFingerprint(manifest) + if err != nil { + return PRVisualEvidenceManifest{}, err + } + raw, err := MarshalJSON(manifest) + if err != nil { + return PRVisualEvidenceManifest{}, err + } + path, err := visualEvidenceManifestPath(repo, manifest.Key) + if err != nil { + return PRVisualEvidenceManifest{}, err + } + if err := atomicWriteMode(path, raw, 0o600); err != nil { + return PRVisualEvidenceManifest{}, err + } + return manifest, nil +} + +func ImportPRVisualEvidence(repo, manifestPath string) (PRVisualEvidenceManifest, error) { + raw, err := os.ReadFile(manifestPath) + if err != nil { + return PRVisualEvidenceManifest{}, err + } + var manifest PRVisualEvidenceManifest + if err := DecodeJSON("import PR visual evidence", manifestPath, raw, &manifest); err != nil { + return PRVisualEvidenceManifest{}, err + } + return SavePRVisualEvidence(repo, manifest) +} + +func LoadPRVisualEvidence(repo, key string) (PRVisualEvidenceManifest, error) { + path, err := visualEvidenceManifestPath(repo, key) + if err != nil { + return PRVisualEvidenceManifest{}, err + } + raw, err := os.ReadFile(path) + if err != nil { + return PRVisualEvidenceManifest{}, err + } + var manifest PRVisualEvidenceManifest + if err := DecodeJSON("load PR visual evidence", path, raw, &manifest); err != nil { + return PRVisualEvidenceManifest{}, err + } + if err := validateVisualManifest(manifest); err != nil { + return PRVisualEvidenceManifest{}, err + } + expected, err := visualManifestFingerprint(manifest) + if err != nil { + return PRVisualEvidenceManifest{}, err + } + if manifest.Fingerprint != expected { + return PRVisualEvidenceManifest{}, fmt.Errorf("visual evidence manifest fingerprint is stale") + } + for _, item := range manifest.Items { + if hash, err := SHA256File(item.Path); err != nil || hash != item.SHA256 { + return PRVisualEvidenceManifest{}, fmt.Errorf("visual evidence screenshot is missing or stale: %s", item.ScenarioID) + } + } + return manifest, nil +} + +func recordPRVisualPublication(repo string, manifest PRVisualEvidenceManifest, publication PRVisualPublication) (PRVisualEvidenceManifest, error) { + manifest.Publication = publication + manifest.Fingerprint = "" + if err := validateVisualManifest(manifest); err != nil { + return PRVisualEvidenceManifest{}, err + } + var err error + manifest.Fingerprint, err = visualManifestFingerprint(manifest) + if err != nil { + return PRVisualEvidenceManifest{}, err + } + raw, err := MarshalJSON(manifest) + if err != nil { + return PRVisualEvidenceManifest{}, err + } + path, err := visualEvidenceManifestPath(repo, manifest.Key) + if err != nil { + return PRVisualEvidenceManifest{}, err + } + if err := atomicWriteMode(path, raw, 0o600); err != nil { + return PRVisualEvidenceManifest{}, err + } + return manifest, nil +} + +func RecordPRVisualPublication(repo, key, prURL, commentURL string) (PRVisualEvidenceManifest, error) { + if strings.TrimSpace(prURL) == "" || strings.TrimSpace(commentURL) == "" { + return PRVisualEvidenceManifest{}, fmt.Errorf("PR and visual evidence comment URLs are required") + } + manifest, err := LoadPRVisualEvidence(repo, key) + if err != nil { + return PRVisualEvidenceManifest{}, err + } + return recordPRVisualPublication(repo, manifest, PRVisualPublication{ + State: "published", PRURL: strings.TrimSpace(prURL), CommentURL: strings.TrimSpace(commentURL), + UpdatedAt: time.Now().UTC().Truncate(time.Second).Format(time.RFC3339), + }) +} + +func SavePRVisualCapability(repo string, receipt PRVisualCapabilityReceipt) error { + receipt.SchemaVersion = visualEvidenceSchemaVersion + receipt.BoatstackVersion = Version + if receipt.VerifiedAt == "" { + receipt.VerifiedAt = time.Now().UTC().Truncate(time.Second).Format(time.RFC3339) + } + if receipt.HealthStatus != "ready" && receipt.HealthStatus != "unavailable" { + return fmt.Errorf("visual capability health_status must be ready or unavailable") + } + path, err := visualCapabilityPath(repo) + if err != nil { + return err + } + raw, err := MarshalJSON(receipt) + if err != nil { + return err + } + return atomicWriteMode(path, raw, 0o600) +} + +func LoadPRVisualCapability(repo string, expected PRVisualCapabilityReceipt) (PRVisualCapabilityReceipt, error) { + path, err := visualCapabilityPath(repo) + if err != nil { + return PRVisualCapabilityReceipt{}, err + } + raw, err := os.ReadFile(path) + if err != nil { + return PRVisualCapabilityReceipt{}, err + } + var actual PRVisualCapabilityReceipt + if err := DecodeJSON("load PR visual capability", path, raw, &actual); err != nil { + return PRVisualCapabilityReceipt{}, err + } + if actual.SchemaVersion != visualEvidenceSchemaVersion || actual.BoatstackVersion != Version || actual.HealthStatus != "ready" || + actual.LockfileSHA256 != expected.LockfileSHA256 || actual.LaunchCommandHash != expected.LaunchCommandHash || + actual.BrowserVersion != expected.BrowserVersion || actual.FrameworkConfigSHA != expected.FrameworkConfigSHA { + return PRVisualCapabilityReceipt{}, fmt.Errorf("visual evidence capability receipt is stale") + } + if _, err := time.Parse(time.RFC3339, actual.VerifiedAt); err != nil { + return PRVisualCapabilityReceipt{}, fmt.Errorf("visual capability verified_at must be RFC3339: %w", err) + } + return actual, nil +} 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 new file mode 100644 index 000000000..8b60db4e4 --- /dev/null +++ b/labs/12-product-engineering-loop/product-engineering-loop/visual_evidence_test.go @@ -0,0 +1,196 @@ +package boatstack + +import ( + "context" + "image" + "image/color" + "image/png" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +func visualTestRepo(t *testing.T) string { + t.Helper() + repo := t.TempDir() + runGit(t, repo, "init", "-b", "main") + runGit(t, repo, "config", "user.name", "Boatstack Test") + runGit(t, repo, "config", "user.email", "boatstack@example.invalid") + if err := os.WriteFile(filepath.Join(repo, "README.md"), []byte("fixture\n"), 0o644); err != nil { + t.Fatal(err) + } + runGit(t, repo, "add", "README.md") + runGit(t, repo, "commit", "-m", "base") + return repo +} + +func TestPRVisualCapabilityCutCoversRepositoryAndHostBrowserConsumers(t *testing.T) { + repo := visualTestRepo(t) + config := testConfig() + config.Project.Commands["e2e"] = "npm run e2e" + capability, err := ResolvePRVisualCaptureCapability(repo, config, true, "npm run dev", PRVisualCapabilityReceipt{}) + if err != nil || capability.Kind != "repository-command" { + t.Fatalf("repository capability did not win: %#v %v", capability, err) + } + delete(config.Project.Commands, "e2e") + capability, err = ResolvePRVisualCaptureCapability(repo, config, true, "npm run dev", PRVisualCapabilityReceipt{}) + if err != nil || capability.Kind != "host-browser" { + t.Fatalf("host browser consumer was not selected: %#v %v", capability, err) + } + + server := httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) { + response.WriteHeader(http.StatusNoContent) + })) + defer server.Close() + if err := ProbePRVisualReadiness(context.Background(), server.URL, time.Second); err != nil { + t.Fatalf("representative dev-server readiness failed: %v", err) + } +} + +func writeTestPNG(t *testing.T, path string) { + t.Helper() + file, err := os.Create(path) + if err != nil { + t.Fatal(err) + } + canvas := image.NewRGBA(image.Rect(0, 0, 4, 3)) + canvas.Set(1, 1, color.RGBA{R: 240, G: 160, B: 20, A: 255}) + if err := png.Encode(file, canvas); err != nil { + file.Close() + t.Fatal(err) + } + if err := file.Close(); err != nil { + t.Fatal(err) + } +} + +func TestPRVisualEvidenceIsMachineLocalFreshAndExact(t *testing.T) { + repo := visualTestRepo(t) + pngPath := filepath.Join(t.TempDir(), "warning.png") + writeTestPNG(t, pngPath) + manifest, err := SavePRVisualEvidence(repo, PRVisualEvidenceManifest{ + Key: "feature-warning", Policy: "suggest", Relevance: "relevant", RelevanceSource: "human-provided", + Status: "PASS", SourceCommit: runGit(t, repo, "rev-parse", "HEAD"), ProductDiffSHA256: strings.Repeat("a", 64), + Scenarios: []PRVisualScenario{{ID: "warning", Entry: "/onboarding", State: "picker open", Viewport: "1440x900", Expected: []string{"warning visible"}}}, + Items: []PRVisualEvidenceItem{{ScenarioID: "warning", Path: pngPath, Viewport: "1440x900", CapturedAt: time.Now().UTC().Truncate(time.Second).Format(time.RFC3339), Status: "captured", PrivacyStatus: "human-reviewed"}}, + Publication: PRVisualPublication{State: "pending"}, + }) + if err != nil { + t.Fatal(err) + } + if manifest.Items[0].Width != 4 || manifest.Items[0].Height != 3 || !strings.Contains(manifest.Items[0].Path, filepath.Join("boatstack", "visual-evidence")) { + t.Fatalf("unexpected normalized visual evidence: %#v", manifest.Items[0]) + } + if status := runGit(t, repo, "status", "--short"); status != "" { + t.Fatalf("visual evidence changed the product tree: %s", status) + } + loaded, err := LoadPRVisualEvidence(repo, "feature-warning") + if err != nil || loaded.Fingerprint != manifest.Fingerprint { + t.Fatalf("fresh visual evidence did not reload: %#v %v", loaded, err) + } + if err := os.WriteFile(loaded.Items[0].Path, []byte("changed"), 0o600); err != nil { + t.Fatal(err) + } + if _, err := LoadPRVisualEvidence(repo, "feature-warning"); err == nil || !strings.Contains(err.Error(), "stale") { + t.Fatalf("changed screenshot was not rejected: %v", err) + } +} + +func TestPRVisualEvidenceRequiresPrivacyReview(t *testing.T) { + repo := visualTestRepo(t) + pngPath := filepath.Join(t.TempDir(), "warning.png") + writeTestPNG(t, pngPath) + _, err := SavePRVisualEvidence(repo, PRVisualEvidenceManifest{ + Key: "feature-warning", Policy: "suggest", Relevance: "relevant", RelevanceSource: "human-provided", + Status: "PASS", SourceCommit: runGit(t, repo, "rev-parse", "HEAD"), ProductDiffSHA256: strings.Repeat("a", 64), + Scenarios: []PRVisualScenario{{ID: "warning", Entry: "/onboarding", State: "picker open", Viewport: "1440x900", Expected: []string{"warning visible"}}}, + Items: []PRVisualEvidenceItem{{ScenarioID: "warning", Path: pngPath, Viewport: "1440x900", CapturedAt: time.Now().UTC().Truncate(time.Second).Format(time.RFC3339), Status: "captured"}}, + Publication: PRVisualPublication{State: "pending"}, + }) + if err == nil || !strings.Contains(err.Error(), "privacy_status") { + t.Fatalf("missing privacy review was not rejected: %v", err) + } +} + +func TestPRVisualCapabilityReceiptInvalidatesChangedInputs(t *testing.T) { + repo := visualTestRepo(t) + receipt := PRVisualCapabilityReceipt{ + LockfileSHA256: "lock", LaunchCommandHash: "launch", BrowserVersion: "browser-1", + FrameworkConfigSHA: "config", HealthStatus: "ready", + } + if err := SavePRVisualCapability(repo, receipt); err != nil { + t.Fatal(err) + } + if _, err := LoadPRVisualCapability(repo, receipt); err != nil { + t.Fatal(err) + } + receipt.BrowserVersion = "browser-2" + if _, err := LoadPRVisualCapability(repo, receipt); err == nil || !strings.Contains(err.Error(), "stale") { + t.Fatalf("changed capability input was not rejected: %v", err) + } +} + +type fakeVisualPublisher struct { + commentURL string + err error + existing string +} + +func (publisher *fakeVisualPublisher) PublishVisualEvidence(repo, prURL, existingCommentURL string, manifest PRVisualEvidenceManifest) (string, error) { + publisher.existing = existingCommentURL + return publisher.commentURL, publisher.err +} + +func savedVisualManifest(t *testing.T, repo, key string) PRVisualEvidenceManifest { + t.Helper() + pngPath := filepath.Join(t.TempDir(), "warning.png") + writeTestPNG(t, pngPath) + manifest, err := SavePRVisualEvidence(repo, PRVisualEvidenceManifest{ + Key: key, Policy: "suggest", Relevance: "relevant", RelevanceSource: "repository-evidenced", + Status: "PASS", SourceCommit: runGit(t, repo, "rev-parse", "HEAD"), ProductDiffSHA256: strings.Repeat("b", 64), + Scenarios: []PRVisualScenario{{ID: "warning", Entry: "/onboarding", State: "picker open", Viewport: "1440x900", Expected: []string{"warning visible"}}}, + Items: []PRVisualEvidenceItem{{ScenarioID: "warning", Path: pngPath, Viewport: "1440x900", CapturedAt: time.Now().UTC().Truncate(time.Second).Format(time.RFC3339), Status: "captured", PrivacyStatus: "human-reviewed"}}, + Publication: PRVisualPublication{State: "pending"}, + }) + if err != nil { + t.Fatal(err) + } + return manifest +} + +func TestPRVisualPublisherReusesOneCommentAndRecordsPendingFailure(t *testing.T) { + repo := visualTestRepo(t) + manifest := savedVisualManifest(t, repo, "feature-warning") + context := PRContext{PRVisualEvidencePolicy: "suggest", PRVisualEvidenceStatus: "PASS", PRVisualEvidence: &manifest} + publisher := &fakeVisualPublisher{commentURL: "https://github.com/example/repo/pull/1#issuecomment-2"} + if err := publishPRVisualEvidence(repo, "https://github.com/example/repo/pull/1", context, publisher); err != nil { + t.Fatal(err) + } + published, err := LoadPRVisualEvidence(repo, manifest.Key) + if err != nil || published.Publication.State != "published" { + t.Fatalf("publication was not recorded: %#v %v", published.Publication, err) + } + updated := savedVisualManifest(t, repo, "feature-warning") + context.PRVisualEvidence = &updated + if err := publishPRVisualEvidence(repo, "https://github.com/example/repo/pull/1", context, publisher); err != nil { + t.Fatal(err) + } + if publisher.existing != published.Publication.CommentURL { + t.Fatalf("existing evidence comment was not reused: %q", publisher.existing) + } + + pending := savedVisualManifest(t, repo, "feature-failure") + context.PRVisualEvidence = &pending + failing := &fakeVisualPublisher{err: os.ErrPermission} + if err := publishPRVisualEvidence(repo, "https://github.com/example/repo/pull/2", context, failing); err == nil || !strings.Contains(err.Error(), "fix forward") { + t.Fatalf("publication failure was not routed to fix-forward: %v", err) + } + failed, err := LoadPRVisualEvidence(repo, pending.Key) + if err != nil || failed.Publication.State != "visual_pending" { + t.Fatalf("visual-pending state was not retained: %#v %v", failed.Publication, err) + } +} diff --git a/labs/12-product-engineering-loop/project.example.json b/labs/12-product-engineering-loop/project.example.json index b8c0781ac..0a64607cf 100644 --- a/labs/12-product-engineering-loop/project.example.json +++ b/labs/12-product-engineering-loop/project.example.json @@ -25,7 +25,8 @@ "human_plan_approval": true, "independent_review_for_high_risk": true, "allow_pass_with_gaps": true, - "maintain_changelog": false + "maintain_changelog": false, + "pr_visual_evidence": "off" }, "integrations": { "gstack": { diff --git a/labs/12-product-engineering-loop/tests/test_product_loop.py b/labs/12-product-engineering-loop/tests/test_product_loop.py index 43eb0b282..86858ceaf 100644 --- a/labs/12-product-engineering-loop/tests/test_product_loop.py +++ b/labs/12-product-engineering-loop/tests/test_product_loop.py @@ -1220,19 +1220,23 @@ def test_ad_hoc_pr_preview_cli_forward_flow(self) -> None: template = self.run_helper( "pr-context", "--repo", repo, "--format", "template" ) - self.assertIn("boatstack_pr_version: 2", template.stdout) + self.assertIn("boatstack_pr_version: 3", template.stdout) self.assertIn("## Review order", template.stdout) body = (SKILL / "testdata/reviewer-pr-body.md").read_text().strip() preview = "\n".join([ "---", - "boatstack_pr_version: 2", + "boatstack_pr_version: 3", "title: " + json.dumps("Make repository checks predictable"), "mode: " + json.dumps(context["mode"]), "feature: \"\"", "base: " + json.dumps(context["base_branch"]), "head: " + json.dumps(context["head_branch"]), "context_fingerprint: " + json.dumps(context["context_fingerprint"]), + "pr_visual_evidence_policy: " + json.dumps(context["pr_visual_evidence_policy"]), + "pr_visual_evidence_status: " + json.dumps(context["pr_visual_evidence_status"]), + "pr_visual_evidence_count: " + str(context["pr_visual_evidence_count"]), + "pr_visual_evidence_fingerprint: " + json.dumps(context["pr_visual_evidence_fingerprint"]), "---", body, "",