Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ failed, or stale results.
| Permit visible verification gaps | `workflow.allow_pass_with_gaps` | `false` rejects `PASS_WITH_GAPS` at delivery and PR gates; `true` retains the gaps as evidence. |
| Maintain reader-facing history | `workflow.maintain_changelog` | Managed delivery and Boatstack-prepared PRs require a categorized `CHANGELOG.md` entry. |
| Check for a systemic boundary | `workflow.boundary_analysis` | Planning guidance asks whether the request is a local symptom before scope expands. |
| Add frontend PR screenshots | `workflow.pr_visual_evidence` | `suggest` exposes missing screenshots as a gap; `require` blocks completed publication. A plan that approves visual scenarios lifts `suggest` to require semantics for that feature; `off` and a per-feature `not_relevant` decision (with a reason) are the escapes. Boatstack captures registered scenarios automatically during ship. |
| Add frontend PR screenshots | `workflow.pr_visual_evidence` | `suggest` exposes missing screenshots as a gap; `require` blocks completed publication. A plan that approves visual scenarios lifts `suggest` to require semantics for that feature; `off` and a per-feature `not_relevant` decision (with a reason) are the escapes. Boatstack captures registered scenarios automatically during ship; per-surface harnesses register as `project.commands["visual:<surface>"]` (`capability-register --surface`) and scenarios select them with a `surface` field. |
| Render screenshots inline on a private PR | `workflow.visual_evidence_publish.*` | `mode: external-host` uploads the captured PNGs to an anonymous expiring host so the comment renders inline even on a private repo; opt-in, never automatic. |
| Ignore old ambiguous deliveries | `workflow.ignored_deliveries` | Listed feature slugs are excluded from delivery-ambiguity resolution so past work stops blocking new work; new, unlisted ambiguous deliveries still pause. |
| Pursue the PR to merge, not just to open | `delivery.terminal` | `merged` keeps the read-only flow advisors naming post-publish steps (watch checks, route corrections) until the PR is observed merged; the default `published` ends the flow when the PR is open, exactly as before. |
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
### Per-surface visual capture harnesses

A repository with more than one product surface (a web app and an ops console, for example) can now register one capture command per surface: `capability-register --capability visual --surface web --command <command>` writes `project.commands["visual:web"]`, and a plan scenario selects it with an optional `surface` field (lowercase kebab). The surface-scoped command outranks the global `visual` command; a scenario without a surface, or a surface without its own command, uses the global command exactly as before. Capture resolves every scenario's command before any harness runs, and an unresolvable surface is refused naming the exact missing key. The harness contract gains `BOATSTACK_CAPTURE_SURFACE`.
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,20 @@ func ResolveCapability(name string, config ProjectConfig) (CapabilityResolution,
}
return CapabilityResolution{Name: capability.Name, Kind: "unavailable"}, nil
}

// ResolveCapabilityForSurface resolves a capability command for one product
// surface: the surface-scoped key ("visual:web") outranks the global alias
// ladder, and an empty or unregistered surface falls back to it exactly — a
// repository with only a global command keeps serving every surface.
func ResolveCapabilityForSurface(name, surface string, config ProjectConfig) (CapabilityResolution, error) {
capability, ok := LookupCapability(name)
if !ok {
return CapabilityResolution{}, fmt.Errorf("unknown evidence capability %q", name)
}
if surface = strings.TrimSpace(surface); surface != "" {
if command := strings.TrimSpace(config.Project.Commands[capability.Name+":"+surface]); command != "" {
return CapabilityResolution{Name: capability.Name, Kind: "repository-command", Command: command}, nil
}
}
return ResolveCapability(name, config)
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,40 @@ func TestLookupCapabilityExposesRegisteredMetadata(t *testing.T) {
t.Fatalf("visual capability metadata is incomplete: %#v", capability)
}
}

// Invariant: a surface-scoped command outranks the global alias, and its
// absence falls back to the global ladder exactly — a repository with one
// global harness keeps serving every surface (zero-value behavior).
func TestResolveCapabilityForSurfacePrefersSurfaceScopedCommand(t *testing.T) {
config := testConfig()
delete(config.Project.Commands, "visual")
delete(config.Project.Commands, "screenshot")
delete(config.Project.Commands, "e2e")
config.Project.Commands["visual"] = "npm run capture:visual"
config.Project.Commands["visual:web"] = "npm run capture:web"

resolution, err := ResolveCapabilityForSurface("visual", "web", config)
if err != nil {
t.Fatal(err)
}
if resolution.Command != "npm run capture:web" {
t.Fatalf("surface key did not outrank the global alias: %#v", resolution)
}
for _, surface := range []string{"", "ops"} {
resolution, err = ResolveCapabilityForSurface("visual", surface, config)
if err != nil {
t.Fatal(err)
}
if resolution.Command != "npm run capture:visual" {
t.Fatalf("surface %q did not fall back to the global alias: %#v", surface, resolution)
}
}
delete(config.Project.Commands, "visual")
resolution, err = ResolveCapabilityForSurface("visual", "ops", config)
if err != nil {
t.Fatal(err)
}
if resolution.Kind != "unavailable" {
t.Fatalf("unregistered surface with no global command must be unavailable: %#v", resolution)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ func (execCaptureRunner) Run(request CaptureRequest) error {
"BOATSTACK_CAPTURE_ENTRY="+request.Scenario.Entry,
"BOATSTACK_CAPTURE_STATE="+request.Scenario.State,
"BOATSTACK_CAPTURE_VIEWPORT="+request.Scenario.Viewport,
"BOATSTACK_CAPTURE_SURFACE="+request.Scenario.Surface,
"BOATSTACK_CAPTURE_OUTPUT="+request.OutputPath,
)
// The harness's authoritative output is the PNG on disk, not stdout; only
Expand Down Expand Up @@ -96,14 +97,6 @@ func CaptureEvidence(options CaptureEvidenceOptions) (PRVisualEvidenceManifest,
if err != nil {
return PRVisualEvidenceManifest{}, fmt.Errorf("capture requires a valid Boatstack project configuration: %w", err)
}
resolution, err := ResolveCapability(name, config)
if err != nil {
return PRVisualEvidenceManifest{}, err
}
if resolution.Kind != "repository-command" {
return PRVisualEvidenceManifest{}, fmt.Errorf("evidence capability %q is unavailable: register a repository command (project.commands) or provision it first", name)
}

relevance, source, scenarios, err := planVisualDecision(repo, feature)
if err != nil {
return PRVisualEvidenceManifest{}, err
Expand All @@ -114,6 +107,13 @@ func CaptureEvidence(options CaptureEvidenceOptions) (PRVisualEvidenceManifest,
if len(scenarios) == 0 {
return PRVisualEvidenceManifest{}, fmt.Errorf("no %s scenarios declared in the plan (pr_visual_evidence.scenarios)", name)
}
// Every scenario's command must resolve before any capture runs — a
// surface-scoped key outranks the global alias; a missing surface key
// with no global fallback is named exactly, never captured around.
commands, err := resolveScenarioCaptureCommands(name, scenarios, config)
if err != nil {
return PRVisualEvidenceManifest{}, err
}

head, err := gitCommand(repo, "rev-parse", "--abbrev-ref", "HEAD")
if err != nil {
Expand Down Expand Up @@ -147,7 +147,7 @@ func CaptureEvidence(options CaptureEvidenceOptions) (PRVisualEvidenceManifest,
items := make([]PRVisualEvidenceItem, 0, len(scenarios))
for _, scenario := range scenarios {
outputPath := filepath.Join(stagingDir, scenario.ID+".png")
if err := captureScenario(repo, capability, resolution.Command, scenario, outputPath, feature, head, headCommit, diffHash, runner); err != nil {
if err := captureScenario(repo, capability, commands[scenario.ID], scenario, outputPath, feature, head, headCommit, diffHash, runner); err != nil {
return PRVisualEvidenceManifest{}, err
}
items = append(items, PRVisualEvidenceItem{
Expand Down Expand Up @@ -182,6 +182,28 @@ func CaptureEvidence(options CaptureEvidenceOptions) (PRVisualEvidenceManifest,
return saved, nil
}

// resolveScenarioCaptureCommands resolves the harness command for every
// scenario up front (surface key first, global alias fallback), so capture
// either runs with a complete command map or fails naming the exact missing
// registration before any harness executes.
func resolveScenarioCaptureCommands(name string, scenarios []PRVisualScenario, config ProjectConfig) (map[string]string, error) {
commands := make(map[string]string, len(scenarios))
for _, scenario := range scenarios {
resolution, err := ResolveCapabilityForSurface(name, scenario.Surface, config)
if err != nil {
return nil, err
}
if resolution.Kind != "repository-command" {
if surface := strings.TrimSpace(scenario.Surface); surface != "" {
return nil, fmt.Errorf("evidence capability %q is unavailable for surface %q: register project.commands[%q] (capability-register --capability %s --surface %s --command <command>) or a global command", name, surface, name+":"+surface, name, surface)
}
return nil, fmt.Errorf("evidence capability %q is unavailable: register a repository command (capability-register --capability %s --command <command>) or provision it first", name, name)
}
commands[scenario.ID] = resolution.Command
}
return commands, nil
}

// captureProductDiff reproduces the pr-context product-diff fingerprint so a
// captured manifest is trusted (PASS) by resolvePRVisualEvidence: same product
// diff. The head commit is recorded for provenance only — trust is keyed to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,3 +179,112 @@ func TestCaptureEvidenceRequiresAResolvedCapabilityCommand(t *testing.T) {
t.Fatalf("capture ran without a resolved repository command: %v", err)
}
}

// Invariant: capture resolves the harness per scenario surface — each
// scenario runs its own registered command with the surface in the request —
// and an unresolvable surface fails before any harness runs, naming the exact
// missing registration key.
func TestCaptureEvidenceResolvesPerSurfaceCommands(t *testing.T) {
repo := captureTestRepo(t, "reviewer-ready")
directory := filepath.Join(repo, ".product-loop", "features", "reviewer-ready")
plan := validPlan()
plan["feature_id"] = "reviewer-ready"
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"}, "surface": "web",
},
map[string]any{
"id": "console", "entry": "/ops/queues", "state": "backlog shown", "viewport": "1280x800",
"expected": []any{"queue depth visible"}, "surface": "ops",
},
},
}
writeMarkdownPlan(t, filepath.Join(directory, "plan.md"), plan, true)
runGit(t, repo, "add", ".")
runGit(t, repo, "commit", "-m", "declare per-surface scenarios")

configPath := filepath.Join(repo, ".product-loop", "project.json")
config, _, err := LoadConfig(configPath)
if err != nil {
t.Fatal(err)
}
config.Project.Commands["visual:web"] = "run-web-harness"
value, err := MarshalJSON(config)
if err != nil {
t.Fatal(err)
}
if err := os.WriteFile(configPath, value, 0o644); err != nil {
t.Fatal(err)
}
runGit(t, repo, "add", ".")
runGit(t, repo, "commit", "-m", "register web surface harness")

// One surface key missing and no usable... the global "visual" command is
// still registered in the fixture, so ops falls back to it: capture runs.
commandsSeen := map[string]string{}
surfacesSeen := map[string]string{}
runner := &stubCaptureRunner{write: func(request CaptureRequest) error {
commandsSeen[request.Scenario.ID] = request.Command
surfacesSeen[request.Scenario.ID] = request.Scenario.Surface
writeTestPNG(t, request.OutputPath)
return nil
}}
if _, err := CaptureEvidence(CaptureEvidenceOptions{Repo: repo, Capability: "visual", Feature: "reviewer-ready", Runner: runner}); err != nil {
t.Fatalf("per-surface capture failed: %v", err)
}
if commandsSeen["warning"] != "run-web-harness" || surfacesSeen["warning"] != "web" {
t.Fatalf("web scenario did not run its surface harness: %q (%q)", commandsSeen["warning"], surfacesSeen["warning"])
}
if commandsSeen["console"] != "exit 1" || surfacesSeen["console"] != "ops" {
t.Fatalf("ops scenario did not fall back to the global command: %q (%q)", commandsSeen["console"], surfacesSeen["console"])
}

// Remove the global fallback: the ops surface now has no registration and
// capture refuses up front, naming the exact missing key.
delete(config.Project.Commands, "visual")
delete(config.Project.Commands, "screenshot")
delete(config.Project.Commands, "e2e")
value, err = MarshalJSON(config)
if err != nil {
t.Fatal(err)
}
if err := os.WriteFile(configPath, value, 0o644); err != nil {
t.Fatal(err)
}
runGit(t, repo, "add", ".")
runGit(t, repo, "commit", "-m", "remove global harness")
priorCalls := runner.calls
if _, err := CaptureEvidence(CaptureEvidenceOptions{Repo: repo, Capability: "visual", Feature: "reviewer-ready", Runner: runner}); err == nil || !strings.Contains(err.Error(), "visual:ops") {
t.Fatalf("unresolvable surface was not named: %v", err)
}
if runner.calls != priorCalls {
t.Fatal("capture ran a harness despite an unresolvable surface")
}
}

// Invariant: capability-register --surface writes the surface-scoped command
// key, and the registered command is what surface resolution selects.
func TestRegisterCapabilityCommandWithSurface(t *testing.T) {
repo := captureTestRepo(t, "reviewer-ready")
registered, err := RegisterCapabilityCommand(repo, "visual", "ops", "npm run capture:ops")
if err != nil {
t.Fatal(err)
}
if registered.Alias != "visual:ops" {
t.Fatalf("surface registration wrote the wrong key: %#v", registered)
}
config, _, err := LoadConfig(filepath.Join(repo, ".product-loop", "project.json"))
if err != nil {
t.Fatal(err)
}
resolution, err := ResolveCapabilityForSurface("visual", "ops", config)
if err != nil || resolution.Command != "npm run capture:ops" {
t.Fatalf("registered surface command did not resolve: %#v %v", resolution, err)
}
if _, err := RegisterCapabilityCommand(repo, "visual", "Web Ops", "x"); err == nil || !strings.Contains(err.Error(), "kebab") {
t.Fatalf("invalid surface slug was not rejected: %v", err)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -662,14 +662,15 @@ func capabilityRegisterCommand(arguments []string) int {
flags := flag.NewFlagSet("capability-register", flag.ContinueOnError)
repo := flags.String("repo", ".", "repository whose Boatstack configuration owns the command")
capability := flags.String("capability", "visual", "evidence capability to register a command for")
surface := flags.String("surface", "", "optional product surface (e.g. web, ops) to scope the command to")
command := flags.String("command", "", "repository command that produces the evidence")
if err := flags.Parse(arguments); err != nil {
return 2
}
if *command == "" {
return fail(fmt.Errorf("capability-register requires --command"))
}
registered, err := boatstack.RegisterCapabilityCommand(*repo, *capability, *command)
registered, err := boatstack.RegisterCapabilityCommand(*repo, *capability, *surface, *command)
if err != nil {
return fail(err)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,14 @@ package boatstack
import (
"fmt"
"path/filepath"
"regexp"
)

// surfaceSlugPattern names a product surface (web, ops, admin-console):
// lowercase kebab, matching the project.commands["visual:<surface>"] key
// convention shared by plan scenarios and capability-register --surface.
var surfaceSlugPattern = regexp.MustCompile(`^[a-z0-9]+(-[a-z0-9]+)*$`)

type ValidatePlanOptions struct {
PlanPath string
RepoRoot string
Expand Down Expand Up @@ -196,6 +202,9 @@ func validatePRVisualEvidence(plan map[string]any) error {
return fmt.Errorf("pr_visual_evidence scenario %s requires %s", id, field)
}
}
if surface := stringValue(scenario["surface"]); surface != "" && !surfaceSlugPattern.MatchString(surface) {
return fmt.Errorf("pr_visual_evidence scenario %s surface must be a lowercase kebab slug", id)
}
expected, ok := stringSlice(scenario["expected"])
if !ok || len(expected) == 0 {
return fmt.Errorf("pr_visual_evidence scenario %s requires expected visible outcomes", id)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,20 @@ func TestValidatePRVisualEvidence(t *testing.T) {
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": "relevant",
"scenarios": []any{map[string]any{
"id": "warning", "entry": "/onboarding", "state": "picker open", "viewport": "1440x900",
"expected": []any{"warning visible"}, "surface": "admin-console",
}},
}
if err := validatePRVisualEvidence(plan); err != nil {
t.Fatalf("valid surface slug was rejected: %v", err)
}
plan["pr_visual_evidence"].(map[string]any)["scenarios"].([]any)[0].(map[string]any)["surface"] = "Web Ops"
if err := validatePRVisualEvidence(plan); err == nil || !strings.Contains(err.Error(), "surface") {
t.Fatalf("invalid surface slug was 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)
Expand Down
14 changes: 7 additions & 7 deletions labs/12-product-engineering-loop/product-engineering-loop/pr.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func planVisualDecision(repo, feature string) (string, string, []PRVisualScenari
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,
Viewport: stringValue(row["viewport"]), Expected: expected, Surface: stringValue(row["surface"]),
})
}
return relevance, "managed-plan", scenarios, nil
Expand Down Expand Up @@ -142,12 +142,12 @@ func ensureCurrentPRVisualEvidence(repo string, config ProjectConfig, mode, feat
if loaded, loadErr := LoadPRVisualEvidence(repo, key); loadErr == nil && loaded.Status == "PASS" && loaded.ProductDiffSHA256 == diffHash {
return "", nil
}
resolution, err := ResolveCapability("visual", config)
if err != nil || resolution.Kind != "repository-command" {
// The agent-mediated capture rungs (host browser, supplied launch)
// are deliberately not automated here; without a repository-owned
// command the prescribed path stays exactly as it was.
return "no visual capture capability is registered; register one with capability-register --capability visual --command <command>", nil
// Every declared surface must resolve to a repository command for capture
// to be automatic; the agent-mediated rungs (host browser, supplied
// launch) are deliberately not automated here, so any unresolvable
// scenario keeps the prescribed path exactly as it was.
if _, resolveErr := resolveScenarioCaptureCommands("visual", scenarios, config); resolveErr != nil {
return boundedCaptureDetail(resolveErr.Error()), nil
}
dirtyBefore, err := dirtyPaths(repo)
if err != nil {
Expand Down
Loading
Loading