From a1710ad01562253d3879df645327a9cb32da6fe1 Mon Sep 17 00:00:00 2001 From: bigboateng Date: Thu, 30 Jul 2026 11:37:39 +0100 Subject: [PATCH 1/2] feat(boatstack): plan-approved visual scenarios escalate suggest to require MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A plan that promises pixels cannot ship without them. When the approved plan declares pr_visual_evidence relevance relevant with scenarios, the configured suggest policy now ships with require semantics for that feature: publication blocks until current PASS evidence exists — deliberately even when no capture capability is registered (a missing harness is a provisioning gap the denial names, never a license to ship unverified). control-law: plan-approved-scenarios-imply-require - one predicate (visualEscalationApplies) decides escalation; everything downstream reads only the effective policy - context carries pr_visual_evidence_policy_source (configured|plan-escalated); the publication denial says why suggest blocked - escapes stay explicit: off globally, not_relevant + reason per feature - one-time effect: existing suggest+relevant previews report a changed context fingerprint after upgrade (release-noted) Disclosure-Reviewed: reviewed — public-safe only, private facet kept out of this commit --- .../boatstack-distribution/CONFIGURATION.md | 2 +- ...-approved-scenarios-escalate-to-require.md | 3 + .../product-engineering-loop/capture.go | 5 +- .../product-engineering-loop/capture_test.go | 2 +- .../product-engineering-loop/denial.go | 3 + .../product-engineering-loop/pr.go | 25 +++- .../product-engineering-loop/pr_test.go | 115 ++++++++++++++++-- .../references/config-schema.md | 2 +- .../product-engineering-loop/safety.go | 3 + .../visual_evidence.go | 13 ++ 10 files changed, 152 insertions(+), 21 deletions(-) create mode 100644 labs/12-product-engineering-loop/boatstack-distribution/release-notes/2026-07-30-plan-approved-scenarios-escalate-to-require.md diff --git a/labs/12-product-engineering-loop/boatstack-distribution/CONFIGURATION.md b/labs/12-product-engineering-loop/boatstack-distribution/CONFIGURATION.md index a316d2129..ecd7a510b 100644 --- a/labs/12-product-engineering-loop/boatstack-distribution/CONFIGURATION.md +++ b/labs/12-product-engineering-loop/boatstack-distribution/CONFIGURATION.md @@ -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. | +| 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. | | 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. | diff --git a/labs/12-product-engineering-loop/boatstack-distribution/release-notes/2026-07-30-plan-approved-scenarios-escalate-to-require.md b/labs/12-product-engineering-loop/boatstack-distribution/release-notes/2026-07-30-plan-approved-scenarios-escalate-to-require.md new file mode 100644 index 000000000..27fc32e16 --- /dev/null +++ b/labs/12-product-engineering-loop/boatstack-distribution/release-notes/2026-07-30-plan-approved-scenarios-escalate-to-require.md @@ -0,0 +1,3 @@ +### Plan-approved visual scenarios now ship with require semantics + +When the approved plan declares `pr_visual_evidence` relevance `relevant` with scenarios, the configured `suggest` policy escalates to require semantics for that feature: publication blocks until current PASS evidence exists, even when no capture capability is registered yet. A plan that promises pixels can no longer ship with a `NOT_VERIFIED` gap. The escapes stay explicit: `off` globally, or a per-feature `not_relevant` decision with a reason for genuinely nonvisual changes. Because Boatstack now captures registered scenarios automatically during ship, provisioned repositories will not notice the escalation. One-time effect after upgrading: an existing `suggest` preview for a feature with declared scenarios reports a changed context fingerprint — regenerate the preview with `pr-context` before publishing. diff --git a/labs/12-product-engineering-loop/product-engineering-loop/capture.go b/labs/12-product-engineering-loop/product-engineering-loop/capture.go index 2c6414772..c3481d285 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/capture.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/capture.go @@ -163,7 +163,10 @@ func CaptureEvidence(options CaptureEvidenceOptions) (PRVisualEvidenceManifest, manifest := PRVisualEvidenceManifest{ Key: key, - Policy: config.Workflow.PRVisualEvidence, + // The manifest records the configured policy verbatim (informational); + // the effective policy — including plan-escalated require semantics — + // is re-derived by resolvePRVisualEvidence at every decode. + Policy: config.Workflow.PRVisualEvidence, Relevance: relevance, RelevanceSource: source, Status: "PASS", diff --git a/labs/12-product-engineering-loop/product-engineering-loop/capture_test.go b/labs/12-product-engineering-loop/product-engineering-loop/capture_test.go index 9b02af440..183e8dfdf 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/capture_test.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/capture_test.go @@ -112,7 +112,7 @@ func TestCaptureEvidenceProducesManifestTrustedByPRContext(t *testing.T) { if err != nil { t.Fatal(err) } - _, status, count, _, _, _, resolved, err := resolvePRVisualEvidence(repo, config, "managed", "reviewer-ready", head, diffHash) + _, status, count, _, _, _, _, resolved, err := resolvePRVisualEvidence(repo, config, "managed", "reviewer-ready", head, diffHash) if err != nil { t.Fatal(err) } diff --git a/labs/12-product-engineering-loop/product-engineering-loop/denial.go b/labs/12-product-engineering-loop/product-engineering-loop/denial.go index d661b04e1..8981cc129 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/denial.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/denial.go @@ -541,6 +541,9 @@ func denialFor(host string, finding SafetyFinding) Denial { } d.Qualifier = "visual evidence is owed" d.Detail = "PR publication is blocked until required visual evidence is current for " + target + "." + if finding.PolicySource == "plan-escalated" { + d.Detail += " The approved plan declares visual scenarios, so the configured suggest policy ships with require semantics for this feature." + } if reason := strings.TrimSpace(finding.Reason); reason != "" { d.Detail += " Automatic capture reported: " + reason + "." } 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 9e934d828..c3ef0c278 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/pr.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/pr.go @@ -64,6 +64,9 @@ type PRContext struct { PRVisualEvidenceFingerprint string `json:"pr_visual_evidence_fingerprint"` PRVisualEvidenceRelevance string `json:"pr_visual_evidence_relevance"` PRVisualEvidenceSource string `json:"pr_visual_evidence_source"` + // PRVisualEvidencePolicySource is "configured", or "plan-escalated" when + // a plan-approved visual decision lifts suggest to require semantics. + PRVisualEvidencePolicySource string `json:"pr_visual_evidence_policy_source,omitempty"` // PRVisualEvidenceCaptureDetail explains why automatic capture could not // produce current evidence. Deliberately outside the context fingerprint: // a flaky harness message must not destabilize preview equality. @@ -183,7 +186,7 @@ func boundedCaptureDetail(detail string) string { return detail } -func resolvePRVisualEvidence(repo string, config ProjectConfig, mode, feature, head, diffHash string) (string, string, int, string, string, string, *PRVisualEvidenceManifest, error) { +func resolvePRVisualEvidence(repo string, config ProjectConfig, mode, feature, head, diffHash string) (string, string, int, string, string, string, string, *PRVisualEvidenceManifest, error) { policy := normalizedPRVisualEvidencePolicy(config.Workflow.PRVisualEvidence) relevance, source := "unresolved", "agent-proposed" var scenarios []PRVisualScenario @@ -191,12 +194,20 @@ func resolvePRVisualEvidence(repo string, config ProjectConfig, mode, feature, h var err error relevance, source, scenarios, err = planVisualDecision(repo, feature) if err != nil { - return "", "", 0, "", "", "", nil, err + return "", "", 0, "", "", "", "", nil, err } } + // The effective policy is what everything downstream (coercion, preview + // frontmatter, the publication block) reads. Escalation keys on the + // plan's own decision, before any manifest overrides it. + policySource := "configured" + if visualEscalationApplies(policy, relevance, len(scenarios)) { + policy = "require" + policySource = "plan-escalated" + } key, err := visualEvidenceKey(mode, feature, head) if err != nil { - return "", "", 0, "", "", "", nil, err + return "", "", 0, "", "", "", "", nil, err } status := "NOT_APPLICABLE" var manifest *PRVisualEvidenceManifest @@ -235,9 +246,9 @@ func resolvePRVisualEvidence(repo string, config ProjectConfig, mode, feature, h } raw, err := MarshalJSON(payload) if err != nil { - return "", "", 0, "", "", "", nil, err + return "", "", 0, "", "", "", "", nil, err } - return policy, status, count, SHA256Bytes(raw), relevance, source, manifest, nil + return policy, status, count, SHA256Bytes(raw), relevance, source, policySource, manifest, nil } type PRPublishOptions struct { @@ -735,7 +746,7 @@ func PreparePRContext(options PRContextOptions) (PRContext, error) { if err != nil { return PRContext{}, err } - visualPolicy, visualStatus, visualCount, visualFingerprint, visualRelevance, visualSource, visualManifest, err := resolvePRVisualEvidence( + visualPolicy, visualStatus, visualCount, visualFingerprint, visualRelevance, visualSource, visualPolicySource, visualManifest, err := resolvePRVisualEvidence( repo, config, mode, options.Feature, head, SHA256Bytes(diff), ) if err != nil { @@ -762,6 +773,7 @@ func PreparePRContext(options PRContextOptions) (PRContext, error) { PRVisualEvidencePolicy: visualPolicy, PRVisualEvidenceStatus: visualStatus, PRVisualEvidenceCount: visualCount, PRVisualEvidenceFingerprint: visualFingerprint, PRVisualEvidenceRelevance: visualRelevance, PRVisualEvidenceSource: visualSource, + PRVisualEvidencePolicySource: visualPolicySource, PRVisualEvidenceCaptureDetail: captureDetail, PRVisualEvidence: visualManifest, PreviewPath: previewPath, @@ -1127,6 +1139,7 @@ func PublishPR(options PRPublishOptions) (string, error) { Source: "publication", BlockingFeature: context.Feature, Reason: context.PRVisualEvidenceCaptureDetail, + PolicySource: context.PRVisualEvidencePolicySource, } return "", fmt.Errorf("%s", denialWithOptions(repo, "", finding).Render(RenderPlain)) } 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 fe40ea689..10c9cc923 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 @@ -620,9 +620,14 @@ func TestManagedPRVisualEvidenceUsesApprovedPlanScenarios(t *testing.T) { if err != nil { t.Fatal(err) } - if context.Mode != "managed" || context.PRVisualEvidenceRelevance != "relevant" || context.PRVisualEvidenceSource != "managed-plan" || context.PRVisualEvidenceStatus != "NOT_VERIFIED" { + // A plan-approved visual decision escalates suggest to require semantics, + // so missing evidence is BLOCKED here, not a shippable NOT_VERIFIED gap. + if context.Mode != "managed" || context.PRVisualEvidenceRelevance != "relevant" || context.PRVisualEvidenceSource != "managed-plan" || context.PRVisualEvidenceStatus != "BLOCKED" { t.Fatalf("managed visual decision was not projected: %#v", context) } + if context.PRVisualEvidencePolicy != "require" || context.PRVisualEvidencePolicySource != "plan-escalated" { + t.Fatalf("plan-approved scenarios did not escalate suggest: %#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) @@ -728,15 +733,17 @@ func TestProductDiffChangeInvalidatesPassVisualEvidence(t *testing.T) { t.Fatal(err) } changedDiff := strings.Repeat("c", 64) - _, status, _, _, _, _, _, err := resolvePRVisualEvidence(repo, config, "managed", "reviewer-ready", context.HeadBranch, changedDiff) + // The plan declares relevant scenarios, so suggest ships with require + // semantics: stale evidence is BLOCKED, never a shippable gap. + _, status, _, _, _, _, _, _, err := resolvePRVisualEvidence(repo, config, "managed", "reviewer-ready", context.HeadBranch, changedDiff) if err != nil { t.Fatal(err) } - if status != "NOT_VERIFIED" { - t.Fatalf("product change did not stale the evidence: %s", status) + if status != "BLOCKED" { + t.Fatalf("product change did not stale the evidence to a block: %s", status) } config.Workflow.PRVisualEvidence = "require" - _, status, _, _, _, _, _, err = resolvePRVisualEvidence(repo, config, "managed", "reviewer-ready", context.HeadBranch, changedDiff) + _, status, _, _, _, _, _, _, err = resolvePRVisualEvidence(repo, config, "managed", "reviewer-ready", context.HeadBranch, changedDiff) if err != nil { t.Fatal(err) } @@ -745,6 +752,92 @@ func TestProductDiffChangeInvalidatesPassVisualEvidence(t *testing.T) { } } +// Invariant: a plan that promises pixels cannot ship without them — suggest +// escalates to require semantics for plan-approved scenarios even when no +// capture capability is registered, and the denial says why. +func TestSuggestEscalatesToRequireWhenPlanDeclaresScenarios(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.PRVisualEvidencePolicy != "require" || context.PRVisualEvidencePolicySource != "plan-escalated" || context.PRVisualEvidenceStatus != "BLOCKED" { + t.Fatalf("plan-approved scenarios did not escalate: %#v", context) + } + previewPath := writePreview(t, repo, context, "Escalate approved visual scenarios", visualEvidenceBody(managedPRBody(), context.PRVisualEvidenceStatus)) + preview, _, err := CheckPRPreview(repo, previewPath) + if err != nil { + t.Fatal(err) + } + _, err = PublishPR(PRPublishOptions{Repo: repo, PreviewPath: previewPath, ExpectedFingerprint: preview.Fingerprint, Action: "open"}) + if err == nil { + t.Fatal("escalated require did not block publication") + } + for _, needle := range []string{"required visual evidence", "require semantics", "capability-register"} { + if !strings.Contains(err.Error(), needle) { + t.Fatalf("escalated denial is missing %q:\n%s", needle, err.Error()) + } + } +} + +// The escalation predicate's full semantics table: `off` is the global +// opt-out, not_relevant the per-feature escape, and configured require needs +// no escalation. Capability availability is deliberately absent — a missing +// harness is a provisioning gap, not a license to ship unverified. +func TestVisualEscalationPredicate(t *testing.T) { + cases := []struct { + policy, relevance string + scenarios int + want bool + }{ + {"suggest", "relevant", 1, true}, + {"suggest", "relevant", 3, true}, + {"suggest", "relevant", 0, false}, + {"suggest", "not_relevant", 0, false}, + {"suggest", "unresolved", 1, false}, + {"require", "relevant", 1, false}, + {"off", "relevant", 1, false}, + } + for _, c := range cases { + if got := visualEscalationApplies(c.policy, c.relevance, c.scenarios); got != c.want { + t.Errorf("visualEscalationApplies(%s, %s, %d) = %v, want %v", c.policy, c.relevance, c.scenarios, got, c.want) + } + } +} + +// Invariant: a not_relevant plan decision (with its reason) keeps the +// configured suggest semantics — the per-feature escape for nonvisual changes. +func TestNotRelevantPlanKeepsSuggestSemantics(t *testing.T) { + repo := prTestRepoConfigured(t, func(config *ProjectConfig) { + config.Workflow.PRVisualEvidence = "suggest" + }) + directory := filepath.Join(repo, ".product-loop", "features", "log-rotation") + if err := os.MkdirAll(directory, 0o755); err != nil { + t.Fatal(err) + } + plan := validPlan() + plan["feature_id"] = "log-rotation" + plan["pr_visual_evidence"] = map[string]any{ + "relevance": "not_relevant", + "reason": "backend log rotation has no reviewer-visible surface", + } + writeMarkdownPlan(t, filepath.Join(directory, "plan.md"), plan, true) + config, _, err := LoadConfig(filepath.Join(repo, ".product-loop", "project.json")) + if err != nil { + t.Fatal(err) + } + policy, status, _, _, _, _, policySource, _, err := resolvePRVisualEvidence(repo, config, "managed", "log-rotation", "feat/log-rotation", strings.Repeat("d", 64)) + if err != nil { + t.Fatal(err) + } + if policy != "suggest" || policySource != "configured" || status != "NOT_APPLICABLE" { + t.Fatalf("not_relevant did not keep suggest semantics: policy=%s source=%s status=%s", policy, policySource, status) + } +} + // Invariant: declared-relevant visual evidence is captured by ship itself, // never prescribed to the agent, and repeat context preparation is a no-op // while the product diff is unchanged. @@ -783,9 +876,9 @@ func TestPreparePRContextAutoCapturesRelevantVisualEvidence(t *testing.T) { } } -// Invariant: a failing harness degrades to exactly today's suggest behavior — -// a visible NOT_VERIFIED gap with a bounded detail, never a context error. -func TestAutoCaptureFailureDegradesToTodayUnderSuggest(t *testing.T) { +// Invariant: a failing harness never errors context preparation — it records +// a bounded detail, and the plan-escalated require semantics hold the block. +func TestAutoCaptureFailureRecordsBoundedDetail(t *testing.T) { repo := prTestRepoConfigured(t, func(config *ProjectConfig) { config.Workflow.PRVisualEvidence = "suggest" config.Project.Commands["visual"] = "repo-owned-harness" @@ -798,8 +891,8 @@ func TestAutoCaptureFailureDegradesToTodayUnderSuggest(t *testing.T) { if err != nil { t.Fatal(err) } - if context.PRVisualEvidenceStatus != "NOT_VERIFIED" { - t.Fatalf("harness failure should leave the recorded gap, got %s", context.PRVisualEvidenceStatus) + if context.PRVisualEvidenceStatus != "BLOCKED" { + t.Fatalf("escalated require should block on a failed capture, got %s", context.PRVisualEvidenceStatus) } if !strings.Contains(context.PRVisualEvidenceCaptureDetail, "dev server unreachable") { t.Fatalf("capture detail lost the harness failure: %q", context.PRVisualEvidenceCaptureDetail) @@ -821,7 +914,7 @@ func TestAutoCaptureUnavailableCapabilityFallsBackToPrescribedPath(t *testing.T) if err != nil { t.Fatal(err) } - if runner.calls != 0 || context.PRVisualEvidenceStatus != "NOT_VERIFIED" { + if runner.calls != 0 || context.PRVisualEvidenceStatus != "BLOCKED" { t.Fatalf("unavailable capability did not fall back cleanly: calls=%d status=%s", runner.calls, context.PRVisualEvidenceStatus) } if !strings.Contains(context.PRVisualEvidenceCaptureDetail, "capability-register") { 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 2d29ce72e..e5df48667 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 @@ -79,7 +79,7 @@ This is the exhaustive serialization contract, not a list of recommended user ed - `allow_pass_with_gaps` (boolean, optional): Deterministic gate control. `false` rejects `PASS_WITH_GAPS`; `true` preserves explicit gaps. - `maintain_changelog` (boolean, optional): Whether a reader-visible `CHANGELOG.md` entry is required for each delivery slice. - `boundary_analysis` (boolean, optional): Agent-mediated planning guidance that presents local repair versus programmatic enforcement as a material product decision. -- `pr_visual_evidence` (string, optional): `off`, `suggest`, or `require`. Omission is `off`. Relevant PRs use machine-local PNG evidence without committing media to Git; `suggest` records missing evidence as a visible gap and `require` blocks completed publication. +- `pr_visual_evidence` (string, optional): `off`, `suggest`, or `require`. Omission is `off`. Relevant PRs use machine-local PNG evidence without committing media to Git; `suggest` records missing evidence as a visible gap and `require` blocks completed publication. When the approved plan declares `relevance: relevant` with scenarios, `suggest` ships with require semantics for that feature (a plan that promises pixels cannot ship without them) — even when no capture capability is registered yet. The two escapes are `off` (global) and a `not_relevant` plan decision with a reason (per feature, for genuinely nonvisual changes). Boatstack runs a registered capture command (`project.commands.visual`) automatically during ship, so under normal provisioning the escalation is invisible. - `visual_evidence_publish` (object, optional): Agent-mediated publish control for how captured PNG bytes reach the pull-request comment. Omission keeps the default: commit the bytes to a public Boatstack-owned evidence branch and render them inline, but only for a **public** GitHub origin (a private origin falls back to manual attachment). Fields: - `mode` (string, optional): `external-host` opts the repository — including a **private** one — into uploading the exact PNG bytes to an anonymous expiring host so the comment renders inline anywhere. It is **never auto-selected** because it publishes screenshot bytes to a third party; only this explicit value turns it on. Empty keeps the default public-branch behavior. - `host` (string, optional): `litterbox` (default) or `catbox`. Only meaningful when `mode` is `external-host`. `litterbox` auto-expires uploads; `catbox` is permanent. diff --git a/labs/12-product-engineering-loop/product-engineering-loop/safety.go b/labs/12-product-engineering-loop/product-engineering-loop/safety.go index 6d15550c0..e301fa634 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/safety.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/safety.go @@ -26,6 +26,9 @@ type SafetyFinding struct { AttemptedPath string `json:"attempted_path,omitempty"` OperationID string `json:"operation_id,omitempty"` OperationState string `json:"operation_state,omitempty"` + // PolicySource explains a policy-derived denial: "plan-escalated" when a + // plan-approved visual decision lifts suggest to require semantics. + PolicySource string `json:"policy_source,omitempty"` AttemptNumber int `json:"attempt_number,omitempty"` ReconciliationRequired bool `json:"reconciliation_required,omitempty"` // RepeatCount is how many times this same denial (category at stage) has diff --git a/labs/12-product-engineering-loop/product-engineering-loop/visual_evidence.go b/labs/12-product-engineering-loop/product-engineering-loop/visual_evidence.go index 1f22727df..9fc9bfe70 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/visual_evidence.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/visual_evidence.go @@ -130,6 +130,19 @@ func normalizedPRVisualEvidencePolicy(value string) string { return value } +// visualEscalationApplies decides when the configured suggest policy ships +// with require semantics: the approved plan declares visual relevance with +// concrete scenarios. A plan that promises pixels cannot ship without them. +// The escalation is deliberately independent of capture-capability +// availability — a missing harness is a provisioning gap the publication +// denial names, never a license to ship unverified. `off` remains the global +// opt-out (the predicate never fires) and a not_relevant plan decision (with +// its reason) remains the per-feature escape for genuinely nonvisual changes. +// control-law: plan-approved-scenarios-imply-require +func visualEscalationApplies(configured, relevance string, scenarioCount int) bool { + return configured == "suggest" && relevance == "relevant" && scenarioCount > 0 +} + func visualEvidenceKey(mode, feature, head string) (string, error) { key := feature if mode == "ad-hoc" { From 82a998f9db6b9c12383ba4eb1b1b553fbab2ba1a Mon Sep 17 00:00:00 2001 From: bigboateng Date: Thu, 30 Jul 2026 11:51:44 +0100 Subject: [PATCH 2/2] feat(boatstack): flow prescribes the owed visual-attachment retry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A published PR with Publication.State visual_pending or manual_required previously resolved dark in flow next. Now: - new attach-evidence verb (RetryVisualAttachment): retries exactly the operator-confirmed evidence package against the recorded PR URL; publication authority is never re-asked; idempotent when published; refuses pre-publication manifests (publish-pr owns first publication) - attachVisualEvidence extracted from publishPRVisualEvidence so first publication and the retry record identical states - NextStatus.VisualPublication observed best-effort read-only (frontier-reports-never-mutates); reasons name the owed attachment - prescribeVisualAttach consulted before prescribePostPublish, under BOTH terminals (attaching evidence completes publication, it is not merge pursuit); marker published.attach_evidence never auto-driven - actor typing: visual_pending = agent (work-derivable retry); manual_required = operator (owes a signed-in browser or the observed comment URL); a fired goal escape still demotes and stops Disclosure-Reviewed: reviewed — public-safe only, private facet kept out of this commit --- ...-07-30-visual-attach-retry-prescription.md | 3 + .../coverage_conformance_test.go | 1 + .../cmd/boatstack-helper/main.go | 26 +++++- .../denial_solutions_conformance_test.go | 2 +- .../product-engineering-loop/flow_control.go | 68 +++++++++++++- .../product-engineering-loop/flow_frontier.go | 2 +- .../product-engineering-loop/next.go | 37 +++++++- .../next_actor_conformance_test.go | 3 + .../product-engineering-loop/pr.go | 50 +++++++++- .../product-engineering-loop/statemap.go | 2 +- .../visual_attach_conformance_test.go | 92 +++++++++++++++++++ .../visual_evidence_test.go | 48 ++++++++++ 12 files changed, 322 insertions(+), 12 deletions(-) create mode 100644 labs/12-product-engineering-loop/boatstack-distribution/release-notes/2026-07-30-visual-attach-retry-prescription.md create mode 100644 labs/12-product-engineering-loop/product-engineering-loop/visual_attach_conformance_test.go diff --git a/labs/12-product-engineering-loop/boatstack-distribution/release-notes/2026-07-30-visual-attach-retry-prescription.md b/labs/12-product-engineering-loop/boatstack-distribution/release-notes/2026-07-30-visual-attach-retry-prescription.md new file mode 100644 index 000000000..090d083f8 --- /dev/null +++ b/labs/12-product-engineering-loop/boatstack-distribution/release-notes/2026-07-30-visual-attach-retry-prescription.md @@ -0,0 +1,3 @@ +### The flow now prescribes the owed visual-evidence attachment + +A published PR whose Boatstack evidence comment failed to attach (`visual_pending`) or needs manual attachment (`manual_required`) no longer goes dark in `flow next`. The new `attach-evidence --repo --feature` verb retries exactly the operator-confirmed evidence package against the recorded PR — publication authority is never re-asked, and an already attached comment is a no-op. `flow next` prescribes the retry as the agent's step for a transient publisher failure, and prescribes `record-pr-visual-publication` (owing the observed comment URL) as the operator's step when no automatic publisher is available. Both fire under the `published` and `merged` terminals, because attaching evidence completes the publication itself. diff --git a/labs/12-product-engineering-loop/product-engineering-loop/cmd/boatstack-helper/coverage_conformance_test.go b/labs/12-product-engineering-loop/product-engineering-loop/cmd/boatstack-helper/coverage_conformance_test.go index cfce070a0..452a70a9d 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/cmd/boatstack-helper/coverage_conformance_test.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/cmd/boatstack-helper/coverage_conformance_test.go @@ -63,6 +63,7 @@ var nonDeliveryVerbs = map[string]bool{ "provision-capability": true, "capability-register": true, "record-pr-visual-publication": true, + "attach-evidence": true, // PR construction / verification helpers reached around the ship gate. "check-pr": true, // Detached Supervision lifecycle (control-plane ownership, not delivery moves). 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 2658c8440..0e25bcd5b 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 @@ -705,6 +705,28 @@ func recordPRVisualPublicationCommand(arguments []string) int { return 0 } +func attachEvidenceCommand(arguments []string) int { + flags := flag.NewFlagSet("attach-evidence", flag.ContinueOnError) + repo := flags.String("repo", ".", "repository whose Git-common state owns the evidence") + feature := flags.String("feature", "", "managed Boatstack feature slug") + if err := flags.Parse(arguments); err != nil { + return 2 + } + if *feature == "" { + return fail(fmt.Errorf("attach-evidence requires --feature")) + } + manifest, err := boatstack.RetryVisualAttachment(*repo, *feature, boatstack.SelectVisualPublisher(*repo)) + if err != nil { + return fail(err) + } + value, err := boatstack.MarshalJSON(manifest) + 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") @@ -1487,7 +1509,7 @@ func workspaceSyncCommand(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] { @@ -1567,6 +1589,8 @@ func run() int { return capabilityRegisterCommand(os.Args[2:]) case "record-pr-visual-publication": return recordPRVisualPublicationCommand(os.Args[2:]) + case "attach-evidence": + return attachEvidenceCommand(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/denial_solutions_conformance_test.go b/labs/12-product-engineering-loop/product-engineering-loop/denial_solutions_conformance_test.go index a765741b5..5097c32d7 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/denial_solutions_conformance_test.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/denial_solutions_conformance_test.go @@ -123,7 +123,7 @@ func TestTamperDenialNamesDeclaredOwnerVerbs(t *testing.T) { ".git/boatstack/mutations/v1/abc.json": {"activate-plan", "undo"}, ".git/boatstack/quarantine/demo/receipt.json": {"repair-state"}, "state-root/boatstack/registry.json": {"attach", "detach"}, - ".git/boatstack/visual-evidence/x/manifest.json": {"record-pr-visual-evidence", "capture-evidence", "record-pr-visual-publication"}, + ".git/boatstack/visual-evidence/x/manifest.json": {"record-pr-visual-evidence", "capture-evidence", "record-pr-visual-publication", "attach-evidence"}, "boatstack/repositories/sample/binding.json": {"attach", "detach", "activate"}, } for attempted, want := range cases { diff --git a/labs/12-product-engineering-loop/product-engineering-loop/flow_control.go b/labs/12-product-engineering-loop/product-engineering-loop/flow_control.go index 83adb0bda..0e90b21b5 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/flow_control.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/flow_control.go @@ -105,6 +105,11 @@ const ( // control-law: merged-terminal-prescribes-merge-never-executes-it MarkerPublishedWatch = deliverycontrol.TransitionID("published.watch_checks") MarkerPublishedMerge = deliverycontrol.TransitionID("published.merge") + // MarkerPublishedAttach names the owed-attachment retry of a published + // PR's visual-evidence comment. Unlike the merged-terminal markers above + // it fires under BOTH terminals: attaching evidence completes the + // publication itself, it is not merge pursuit. + MarkerPublishedAttach = deliverycontrol.TransitionID("published.attach_evidence") ) // NextActor names who performs the prescribed next step. The operator owns a @@ -157,6 +162,15 @@ func classifyNextActor(status NextStatus, next FlowNext) NextActor { status.ObservedStage == "PUBLISHED" && status.Lifecycle == "PUBLISHED_MERGED": return NextActorNone case status.ObservedStage == "PUBLISHED": + // An owed visual attachment splits by what it owes: a transient + // publisher failure (visual_pending) is work-derivable — the agent + // retries attach-evidence; manual_required owes operator authority (a + // signed-in browser or an external-host opt-in) and stays theirs. A + // fired goal escape still demotes unconditionally. + // control-law: turn-ends-only-at-the-operator-frontier + if status.Lifecycle == "PUBLISHED_OPEN" && status.GoalEscape == "" && status.VisualPublication == "visual_pending" { + return NextActorAgent + } // Under the default published terminal, reviewing the open pull // request is the operator's act — unchanged. Under the merged // terminal, the frontier extends: the phases whose next step is @@ -438,6 +452,45 @@ func prescribePlanning(repo string, status NextStatus) (*PrescribedCommand, stri } } +// prescribeVisualAttach closes the owed-attachment gap of a published-open +// PR so the flow never goes dark on visual_pending or manual_required. It +// fires under BOTH terminals — the attachment completes publication, it is +// not merge pursuit. visual_pending prescribes the attach-evidence retry +// (work-derivable); manual_required prescribes recording the manually +// attached comment, owing the operator-observed URL. A fired goal escape +// prescribes nothing, exactly like the post-publish layer. +// control-law: prescriptive-closure-every-stage-names-a-runnable-command +func prescribeVisualAttach(repo string, status NextStatus) (*PrescribedCommand, string) { + if status.ObservedStage != "PUBLISHED" || status.Lifecycle != "PUBLISHED_OPEN" || status.Feature == "" || status.GoalEscape != "" { + return nil, "" + } + var repoArgs []string + if repo != "" && repo != "." { + repoArgs = []string{"--repo", repo} + } + switch status.VisualPublication { + case "visual_pending": + cmd := &PrescribedCommand{ + Verb: "attach-evidence", Args: append(repoArgs, "--feature", status.Feature), + AutoDerivable: true, Transition: MarkerPublishedAttach, + } + return cmd, "The PR is open; only its Boatstack visual-evidence comment is owed. If the publisher keeps failing, attach the fingerprinted PNGs manually and record the URL with record-pr-visual-publication." + case "manual_required": + cmd := &PrescribedCommand{ + Verb: "record-pr-visual-publication", Args: append(repoArgs, "--key", status.Feature), + RequiresHumanInput: []string{"--comment-url"}, + Transition: MarkerPublishedAttach, + } + if strings.TrimSpace(status.PRURL) != "" { + cmd.Args = append(cmd.Args, "--pr-url", status.PRURL) + } else { + cmd.RequiresHumanInput = append(cmd.RequiresHumanInput, "--pr-url") + } + return cmd, "No automatic publisher is available here: attach the fingerprinted PNGs to one PR comment yourself, then record the observed comment URL." + } + return nil, "" +} + // prescribePostPublish closes the prescriptive loop past publish, but ONLY // under the merged terminal: with the published default this function returns // nothing and post-publish behavior is exactly what it always was. The @@ -606,10 +659,17 @@ func nextControlFromStatus(repo string, status NextStatus) (FlowNext, error) { } } } - // Past publish the oracle sits at its sink and prescribes nothing; under - // the merged terminal the observation-derived post-publish layer takes - // over. It fills only an empty prescription — it can never override an - // oracle move. + // Past publish the oracle sits at its sink and prescribes nothing. An + // owed visual attachment is consulted first and under BOTH terminals — + // it completes the publication itself — then, under the merged terminal + // only, the observation-derived post-publish layer. Each fills only an + // empty prescription — neither can override an oracle move. + if out.Prescribed == nil { + if cmd, followUp := prescribeVisualAttach(repo, status); cmd != nil { + out.Prescribed = cmd + out.FollowUp = followUp + } + } if out.Prescribed == nil { if cmd, followUp := prescribePostPublish(repo, status, out.Terminal); cmd != nil { out.Prescribed = cmd diff --git a/labs/12-product-engineering-loop/product-engineering-loop/flow_frontier.go b/labs/12-product-engineering-loop/product-engineering-loop/flow_frontier.go index 84f9b87f4..59e8689bf 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/flow_frontier.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/flow_frontier.go @@ -86,7 +86,7 @@ func ResolveFrontier(repoPath string) (FlowFrontier, error) { continue } branch, _, prURL := deliveryBranchAndSlice(state) - status := publishedNextStatus(state, observePRTarget(repo, prURL, branch), resolveDeliveryTerminal(repo, state.Feature)) + status := publishedNextStatus(state, observePRTarget(repo, prURL, branch), resolveDeliveryTerminal(repo, state.Feature), observeVisualPublication(repo, state.Feature)) frontier.Rows = append(frontier.Rows, frontierRowFromStatus(repo, status)) } for _, row := range frontier.Rows { diff --git a/labs/12-product-engineering-loop/product-engineering-loop/next.go b/labs/12-product-engineering-loop/product-engineering-loop/next.go index b9e406578..eb85659d2 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/next.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/next.go @@ -35,6 +35,9 @@ type NextStatus struct { PRURL string `json:"pr_url,omitempty"` HeadBranch string `json:"head_branch,omitempty"` ParentDelivery string `json:"parent_delivery,omitempty"` + // VisualPublication surfaces an owed evidence attachment of a published + // PR ("visual_pending" or "manual_required"); empty otherwise. + VisualPublication string `json:"visual_publication,omitempty"` } func blockedNextStatus(stage, operation, reason string, ambiguity ...string) NextStatus { @@ -146,7 +149,7 @@ func nextForPublished(repo string, state DeliveryState) NextStatus { pr := observePublishedPR(repo, state) persistObservedTerminalPRState(repo, state, pr) terminal := resolveDeliveryTerminal(repo, state.Feature) - status := publishedNextStatus(state, pr, terminal) + status := publishedNextStatus(state, pr, terminal, observeVisualPublication(repo, state.Feature)) // A fired escape is cached best-effort so the demotion holds offline in a // fresh session — the same bounded bypass as the terminal PRState cache. // control-law: goal-escape-demotes-to-operator-and-stops @@ -160,7 +163,28 @@ func nextForPublished(repo string, state DeliveryState) NextStatus { // published NextStatus. Split from nextForPublished so the frontier report can // present the same projection without nextForPublished's best-effort terminal // cache write. control-law: frontier-reports-never-mutates -func publishedNextStatus(state DeliveryState, pr publishedPRObservation, terminal DeliveryTerminal) NextStatus { +// observeVisualPublication reads the owed-attachment state of a feature's +// visual evidence, best-effort and read-only: any load failure is today's +// empty answer, never a block, and only the two owed states surface — +// "pending" belongs to first publication (publish-pr) and "published" owes +// nothing. control-law: frontier-reports-never-mutates +func observeVisualPublication(repo, feature string) string { + key, err := visualEvidenceKey("managed", feature, "") + if err != nil { + return "" + } + manifest, err := LoadPRVisualEvidence(repo, key) + if err != nil { + return "" + } + switch manifest.Publication.State { + case "visual_pending", "manual_required": + return manifest.Publication.State + } + return "" +} + +func publishedNextStatus(state DeliveryState, pr publishedPRObservation, terminal DeliveryTerminal, visualPublication string) NextStatus { _, sliceID, _ := deliveryBranchAndSlice(state) status := NextStatus{ SchemaVersion: nextStatusSchemaVersion, VerificationStatus: "VERIFIED", @@ -205,6 +229,15 @@ func publishedNextStatus(state DeliveryState, pr publishedPRObservation, termina default: status.Reason = fmt.Sprintf("Feature %q is published, but its PR state could not be verified.", state.Feature) } + if pr.Lifecycle == "PUBLISHED_OPEN" { + status.VisualPublication = visualPublication + switch visualPublication { + case "visual_pending": + status.Reason += " Its Boatstack visual-evidence comment is still owed; Boatstack can retry the attachment (attach-evidence)." + case "manual_required": + status.Reason += " Its visual-evidence comment needs manual attachment; record the observed URL with record-pr-visual-publication." + } + } if status.GoalEscape != "" { status.Reason = fmt.Sprintf("Feature %q is published; the merged-goal pursuit is paused because %s. Record the correction to start a new cycle, or handle the pull request yourself.", state.Feature, goalEscapeReason(status.GoalEscape)) } diff --git a/labs/12-product-engineering-loop/product-engineering-loop/next_actor_conformance_test.go b/labs/12-product-engineering-loop/product-engineering-loop/next_actor_conformance_test.go index 864bab5a1..bbdb956ed 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/next_actor_conformance_test.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/next_actor_conformance_test.go @@ -101,6 +101,9 @@ func TestNextActorFrontierBoundaries(t *testing.T) { {"owed_evidence_stays_agents", NextStatus{ObservedStage: "BUILD"}, FlowNext{ Prescribed: &PrescribedCommand{Verb: "record-delivery-gate", RequiresHumanInput: []string{"--status", "--evidence"}, Transition: deliverycontrol.TransitionID("delivery.record_gate_test")}, }, NextActorAgent}, + {"owed_visual_attach_retry_is_agents", NextStatus{ObservedStage: "PUBLISHED", Lifecycle: "PUBLISHED_OPEN", VisualPublication: "visual_pending"}, FlowNext{}, NextActorAgent}, + {"manual_visual_attachment_is_operators", NextStatus{ObservedStage: "PUBLISHED", Lifecycle: "PUBLISHED_OPEN", VisualPublication: "manual_required"}, FlowNext{}, NextActorOperator}, + {"escaped_pursuit_demotes_despite_owed_attachment", NextStatus{ObservedStage: "PUBLISHED", Lifecycle: "PUBLISHED_OPEN", VisualPublication: "visual_pending", GoalEscape: "pr_closed"}, FlowNext{Terminal: TerminalMerged}, NextActorOperator}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { 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 c3ef0c278..5473b9cdc 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/pr.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/pr.go @@ -277,6 +277,15 @@ func publishPRVisualEvidence(repo, prURL string, context PRContext, publisher PR if manifest.Publication.State == "published" && manifest.Publication.PRURL == prURL && strings.TrimSpace(manifest.Publication.CommentURL) != "" { return nil } + return attachVisualEvidence(repo, prURL, manifest, publisher, context.PRVisualEvidencePolicy) +} + +// attachVisualEvidence performs the one publisher call and records the +// observed outcome: manual_required without a publisher, visual_pending on a +// publisher failure (PR preserved, fix forward), published on an observable +// comment URL. Shared by first publication (publishPRVisualEvidence) and the +// attach-evidence retry, so both paths record identical states. +func attachVisualEvidence(repo, prURL string, manifest PRVisualEvidenceManifest, publisher PRVisualEvidencePublisher, policy string) error { now := time.Now().UTC().Truncate(time.Second).Format(time.RFC3339) if publisher == nil { _, recordErr := recordPRVisualPublication(repo, manifest, PRVisualPublication{ @@ -286,7 +295,7 @@ func publishPRVisualEvidence(repo, prURL string, context PRContext, publisher PR if recordErr != nil { return fmt.Errorf("PR opened but manual visual-evidence fallback could not be recorded: %w", recordErr) } - if context.PRVisualEvidencePolicy == "require" { + if policy == "require" { return fmt.Errorf("PR opened at %s but required visual evidence still needs manual attachment; update the same PR after attachment", prURL) } return nil @@ -302,12 +311,49 @@ func publishPRVisualEvidence(repo, prURL string, context PRContext, publisher PR if strings.TrimSpace(commentURL) == "" { return fmt.Errorf("visual evidence publisher returned no observable comment URL") } - _, err = recordPRVisualPublication(repo, manifest, PRVisualPublication{ + _, err := recordPRVisualPublication(repo, manifest, PRVisualPublication{ State: "published", PRURL: prURL, CommentURL: strings.TrimSpace(commentURL), UpdatedAt: now, }) return err } +// RetryVisualAttachment retries the owed evidence comment of an already +// published feature PR — the exact fingerprinted package the operator +// confirmed at publication; publication authority is never re-asked. It is +// idempotent: an already published attachment is a no-op. +func RetryVisualAttachment(repo, feature string, publisher PRVisualEvidencePublisher) (PRVisualEvidenceManifest, error) { + resolved, err := ResolveRepository(repo) + if err != nil { + return PRVisualEvidenceManifest{}, err + } + key, err := visualEvidenceKey("managed", feature, "") + if err != nil { + return PRVisualEvidenceManifest{}, err + } + manifest, err := LoadPRVisualEvidence(resolved, key) + if err != nil { + return PRVisualEvidenceManifest{}, fmt.Errorf("no recorded visual evidence for feature %q: %w", feature, err) + } + state := manifest.Publication.State + if state == "published" && strings.TrimSpace(manifest.Publication.CommentURL) != "" { + return manifest, nil + } + if state != "visual_pending" && state != "manual_required" { + return PRVisualEvidenceManifest{}, fmt.Errorf("visual evidence for %q owes no attachment retry (publication state %q); first publication is owned by publish-pr", feature, state) + } + prURL := strings.TrimSpace(manifest.Publication.PRURL) + if prURL == "" { + return PRVisualEvidenceManifest{}, fmt.Errorf("visual evidence for %q records no pull request; publish-pr owns first publication", feature) + } + if publisher == nil { + return PRVisualEvidenceManifest{}, fmt.Errorf("no visual publisher is available in this environment; attach the fingerprinted PNGs to one PR comment yourself and record the observed URL with record-pr-visual-publication --key %s --pr-url %s --comment-url ", key, prURL) + } + if err := attachVisualEvidence(resolved, prURL, manifest, publisher, ""); err != nil { + return PRVisualEvidenceManifest{}, err + } + return LoadPRVisualEvidence(resolved, key) +} + func gitCommand(repo string, arguments ...string) (string, error) { return commandOutput(repo, "git", append([]string{"-C", repo}, arguments...)...) } diff --git a/labs/12-product-engineering-loop/product-engineering-loop/statemap.go b/labs/12-product-engineering-loop/product-engineering-loop/statemap.go index bf46d3d15..baaa3f869 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/statemap.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/statemap.go @@ -228,7 +228,7 @@ func StateRegistry() []StateEntry { }, { Name: "visual-evidence", Class: ClassRuntimeShared, Partition: "git-common", Gitignored: true, GuardProtected: true, - OwnerVerbs: []string{"record-pr-visual-evidence", "capture-evidence", "record-pr-visual-publication"}, + OwnerVerbs: []string{"record-pr-visual-evidence", "capture-evidence", "record-pr-visual-publication", "attach-evidence"}, Sample: staticSample(filepath.FromSlash(".git/boatstack/visual-evidence/sample/manifest.json")), }, { diff --git a/labs/12-product-engineering-loop/product-engineering-loop/visual_attach_conformance_test.go b/labs/12-product-engineering-loop/product-engineering-loop/visual_attach_conformance_test.go new file mode 100644 index 000000000..6d89445dc --- /dev/null +++ b/labs/12-product-engineering-loop/product-engineering-loop/visual_attach_conformance_test.go @@ -0,0 +1,92 @@ +package boatstack + +import ( + "strings" + "testing" +) + +// control-law: prescriptive-closure-every-stage-names-a-runnable-command +// control-law: turn-ends-only-at-the-operator-frontier +// +// A published-open slice with an owed visual publication never resolves to a +// dark prescription: visual_pending prescribes the agent-owned attach-evidence +// retry, manual_required prescribes recording the operator-attached comment, +// and both fire under BOTH terminals because the attachment completes the +// publication itself — it is not merge pursuit. + +func publishedOpenStatus(visualPublication string) NextStatus { + return NextStatus{ + VerificationStatus: "VERIFIED", + ObservedStage: "PUBLISHED", Lifecycle: "PUBLISHED_OPEN", Feature: "demo", + PRURL: "https://github.com/example/repo/pull/7", VisualPublication: visualPublication, + } +} + +func TestOwedVisualAttachmentNeverResolvesDark(t *testing.T) { + t.Run("visual_pending_prescribes_the_retry", func(t *testing.T) { + cmd, followUp := prescribeVisualAttach(".", publishedOpenStatus("visual_pending")) + if cmd == nil || cmd.Verb != "attach-evidence" || !cmd.AutoDerivable { + t.Fatalf("visual_pending did not prescribe the derivable retry: %+v", cmd) + } + if strings.Join(cmd.Args, " ") != "--feature demo" { + t.Fatalf("retry arguments are not state-derived: %v", cmd.Args) + } + if cmd.Transition != MarkerPublishedAttach { + t.Fatalf("retry must carry the attach marker, got %s", cmd.Transition) + } + if followUp == "" { + t.Fatal("the retry prescription owes its manual-fallback follow-up") + } + }) + + t.Run("manual_required_prescribes_the_recording", func(t *testing.T) { + cmd, _ := prescribeVisualAttach(".", publishedOpenStatus("manual_required")) + if cmd == nil || cmd.Verb != "record-pr-visual-publication" { + t.Fatalf("manual_required did not prescribe the recording: %+v", cmd) + } + if cmd.AutoDerivable || strings.Join(cmd.RequiresHumanInput, " ") != "--comment-url" { + t.Fatalf("the observed comment URL must be owed to the operator: %+v", cmd) + } + if !strings.Contains(strings.Join(cmd.Args, " "), "--pr-url https://github.com/example/repo/pull/7") { + t.Fatalf("the recorded PR URL is state-derived and must be in Args: %v", cmd.Args) + } + }) + + t.Run("attach_fires_under_the_published_default_terminal", func(t *testing.T) { + repo := nextTestRepo(t) + next, err := nextControlFromStatus(repo, publishedOpenStatus("visual_pending")) + if err != nil { + t.Fatal(err) + } + if next.Terminal != TerminalPublished { + t.Fatalf("fixture must exercise the published default, got %s", next.Terminal) + } + if next.Prescribed == nil || next.Prescribed.Verb != "attach-evidence" { + t.Fatalf("owed attachment resolved dark under the published terminal: %+v", next.Prescribed) + } + if next.Actor != NextActorAgent { + t.Fatalf("the derivable retry is the agent's step, got %s", next.Actor) + } + }) + + t.Run("no_owed_attachment_prescribes_nothing", func(t *testing.T) { + if cmd, _ := prescribeVisualAttach(".", publishedOpenStatus("")); cmd != nil { + t.Fatalf("nothing is owed but something was prescribed: %+v", cmd) + } + }) + + t.Run("goal_escape_still_demotes_and_stops", func(t *testing.T) { + status := publishedOpenStatus("visual_pending") + status.GoalEscape = "pr_closed" + if cmd, _ := prescribeVisualAttach(".", status); cmd != nil { + t.Fatalf("a fired escape must prescribe nothing: %+v", cmd) + } + }) + + t.Run("attach_marker_is_never_auto_driven", func(t *testing.T) { + cmd, _ := prescribeVisualAttach(".", publishedOpenStatus("visual_pending")) + if canAutoDrive(cmd, autoDrivableTransitions) { + t.Fatal("the attach retry must be prescribed, never driven") + } + }) +} diff --git a/labs/12-product-engineering-loop/product-engineering-loop/visual_evidence_test.go b/labs/12-product-engineering-loop/product-engineering-loop/visual_evidence_test.go index 8b60db4e4..2b8011ee2 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/visual_evidence_test.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/visual_evidence_test.go @@ -194,3 +194,51 @@ func TestPRVisualPublisherReusesOneCommentAndRecordsPendingFailure(t *testing.T) t.Fatalf("visual-pending state was not retained: %#v %v", failed.Publication, err) } } + +// Invariant: the attach retry completes exactly the owed publication — the +// confirmed fingerprinted package against its recorded PR — and an already +// published attachment is a no-op that never re-consults the publisher. +func TestRetryVisualAttachmentCompletesOwedPublication(t *testing.T) { + repo := visualTestRepo(t) + manifest := savedVisualManifest(t, repo, "feature-warning") + context := PRContext{PRVisualEvidencePolicy: "suggest", PRVisualEvidenceStatus: "PASS", PRVisualEvidence: &manifest} + prURL := "https://github.com/example/repo/pull/3" + if err := publishPRVisualEvidence(repo, prURL, context, &fakeVisualPublisher{err: os.ErrPermission}); err == nil { + t.Fatal("fixture publication was expected to fail into visual_pending") + } + retried, err := RetryVisualAttachment(repo, "feature-warning", &fakeVisualPublisher{commentURL: "https://github.com/example/repo/pull/3#issuecomment-9"}) + if err != nil { + t.Fatal(err) + } + if retried.Publication.State != "published" || retried.Publication.PRURL != prURL || retried.Publication.CommentURL == "" { + t.Fatalf("retry did not complete the owed publication: %#v", retried.Publication) + } + again, err := RetryVisualAttachment(repo, "feature-warning", &fakeVisualPublisher{err: os.ErrPermission}) + if err != nil || again.Publication.State != "published" { + t.Fatalf("published attachment must be an idempotent no-op: %#v %v", again.Publication, err) + } +} + +// Refusals: the retry never usurps first publication (publish-pr owns it) and +// a missing publisher routes to the manual recording verb by name. +func TestRetryVisualAttachmentRefusesWhatItDoesNotOwn(t *testing.T) { + repo := visualTestRepo(t) + if _, err := RetryVisualAttachment(repo, "missing-feature", &fakeVisualPublisher{commentURL: "x"}); err == nil || !strings.Contains(err.Error(), "no recorded visual evidence") { + t.Fatalf("missing manifest was not refused: %v", err) + } + manifest := savedVisualManifest(t, repo, "feature-warning") + if _, err := RetryVisualAttachment(repo, "feature-warning", &fakeVisualPublisher{commentURL: "x"}); err == nil || !strings.Contains(err.Error(), "publish-pr") { + t.Fatalf("pre-publication manifest was not routed to publish-pr: %v", err) + } + context := PRContext{PRVisualEvidencePolicy: "suggest", PRVisualEvidenceStatus: "PASS", PRVisualEvidence: &manifest} + if err := publishPRVisualEvidence(repo, "https://github.com/example/repo/pull/4", context, nil); err != nil { + t.Fatal(err) + } + if _, err := RetryVisualAttachment(repo, "feature-warning", nil); err == nil || !strings.Contains(err.Error(), "record-pr-visual-publication") { + t.Fatalf("missing publisher must name the manual recording verb: %v", err) + } + recovered, err := RetryVisualAttachment(repo, "feature-warning", &fakeVisualPublisher{commentURL: "https://github.com/example/repo/pull/4#issuecomment-1"}) + if err != nil || recovered.Publication.State != "published" { + t.Fatalf("manual_required with a live publisher should still recover: %#v %v", recovered.Publication, err) + } +}