From 3083bdb932757e2b393dc59788b9dc273922bebe Mon Sep 17 00:00:00 2001
From: Doug Mealing <doug@dougmealing.com>
Date: Sat, 3 Oct 2026 11:12:20 -0400
Subject: [PATCH 1/5] fix(pipeline): keep foreign-owner PR bodies clean and
 refuse PRs with unresolved intent mismatch

When the PR targets a repository whose owner differs from the fork that
was pushed, the generated body now omits the ## Intent section and every
local-file evidence reference the maintainer cannot open. Inline evidence
text, links, and attachments stay; same-owner bodies are unchanged.

Review now tags intent-contradiction findings with the structured
category intent-conformance, and the PR step refuses to create or update
the pull request while a completed review still holds one (it was
approved, not fixed). The branch stays pushed.
---
 .../content/docs/reference/pipeline-steps.md  |   4 +-
 internal/pipeline/steps/common.go             |   3 +-
 internal/pipeline/steps/intent_prompt.go      |   2 +-
 internal/pipeline/steps/pr.go                 |  82 +++++++-
 internal/pipeline/steps/pr_upstream_test.go   | 195 ++++++++++++++++++
 internal/pipeline/steps/prsummary.go          |  11 +-
 internal/types/findings.go                    |  11 +-
 7 files changed, 292 insertions(+), 16 deletions(-)
 create mode 100644 internal/pipeline/steps/pr_upstream_test.go

diff --git a/docs/src/content/docs/reference/pipeline-steps.md b/docs/src/content/docs/reference/pipeline-steps.md
index 40cae706a3..ef957d6d74 100644
--- a/docs/src/content/docs/reference/pipeline-steps.md
+++ b/docs/src/content/docs/reference/pipeline-steps.md
@@ -248,13 +248,15 @@ Creates or updates a pull request.
 - PR title: agent-generated from the final branch delta with user intent when available, in conventional commit format (`type(scope): description` or `type: description`); user-facing product impact should use `feat` or `fix` so release automation can pick it up; when a scope is used, it should be the primary affected real module/package from the changed paths and kept broad rather than file-level. If drafting fails, the fallback uses the neutral title `chore: update pull request` rather than inferring scope from earlier commits.
 - Bounds the PR-drafting agent with [`agent_timeout`](/no-mistakes/reference/global-config/#agent_timeout): an expired budget cancels the agent and uses that same fallback rather than leaving the run active indefinitely; a late successful title after the deadline is not used
 - The PR stage exclusively owns the complete branch-scope description. It drafts `## What Changed` from the actual final diff after local mutating stages finish, and its fallback lists the final changed paths and statuses.
-- PR body includes a `## Intent` section when user intent is available, the final-diff `## What Changed`, and regenerated `## Risk Assessment`, `## Testing`, and `## Pipeline` sections from recorded step results and rounds. Only `## What Changed` describes the complete final branch scope; the deterministic sections remain evidence for the commit each step inspected. Auto-fix results in `## Pipeline` render as an issue -> fix -> verification narrative using recorded outcomes: applied changes, a confirmed no-change attempt, or an attempt whose result was not recorded. Test details show the live-validation verdict, scenario table, and recorded commands in both `## Testing` and the relevant Pipeline rounds.
+- Refuses to create or update the pull request, failing the step with the branch left pushed, while the completed Review step still holds an intent-conformance finding: Review tags a finding `intent-conformance` when the change contradicts an authoritative intent, so one still present after Review completed was approved rather than fixed
+- PR body includes a `## Intent` section when user intent is available and the PR is not a foreign-owner PR, the final-diff `## What Changed`, and regenerated `## Risk Assessment`, `## Testing`, and `## Pipeline` sections from recorded step results and rounds. Only `## What Changed` describes the complete final branch scope; the deterministic sections remain evidence for the commit each step inspected. Auto-fix results in `## Pipeline` render as an issue -> fix -> verification narrative using recorded outcomes: applied changes, a confirmed no-change attempt, or an attempt whose result was not recorded. Test details show the live-validation verdict, scenario table, and recorded commands in both `## Testing` and the relevant Pipeline rounds.
 - `## Pipeline` keeps the existing human-readable signature and includes the stable structured step attestation documented below. Bitbucket Cloud PR descriptions omit HTML-only features (`<details>`, `<code>`, `<video>`, and the attestation comment) because Cloud renders Python-Markdown and escapes raw HTML.
 - Generated PR bodies are capped at 63,488 bytes, leaving a 2 KB safety buffer below GitHub's 65,536-character body limit.
 - When a body would exceed that cap, the PR step first omits older `## Pipeline` update rounds at clean update boundaries, keeps the newest rounds when possible, and points reviewers to the run log for the full pipeline history.
 - Intent, `## What Changed`, risk, and testing sections are kept ahead of pipeline history; if those sections or the newest pipeline update are still too large, the PR step truncates at line or section boundaries and adds an explicit marker.
 - The regenerated `## Testing` section prefers the recorded `testing_summary` as prose, uses a compact recorded-check count when no summary is available, includes produced evidence artifacts from `path`, `url`, or `content` fields when available, and only adds an outcome with run count and total duration when it is failed or needed as a fallback
 - Evidence artifacts render compactly in PR bodies: repository-relative `path` artifacts and `url` artifacts become `Evidence` links, `content` artifacts appear in collapsible details blocks, GitHub PRs convert repository-relative paths to blob URLs and published evidence to commit-pinned blob or raw URLs, readable UTF-8 text files from the run's evidence directory are embedded inline with truncation for large files, and binary, visual, or over-budget local artifacts render as non-link local file references
+- A foreign-owner PR - a GitHub fork routing whose `fork_url` owner differs from the parent repository's owner - omits the `## Intent` section and every local-file evidence reference the maintainer cannot open; an artifact with only a local path is dropped, while inline evidence text, links, and attachments stay. Same-owner PRs are unchanged.
 - Before the PR is created or updated, the assembled title and body pass through a final home-directory redaction. The home-directory portion of any absolute path - the operator's own home, and `/home/<user>`, `/Users/<user>`, or `C:\Users\<user>` generally - is rewritten to `~` while the rest of the path survives, so the run's evidence and worktree locations, captured command output, artifact paths, and agent prose cannot publish the operator's account name. Redaction is unconditional and runs after every length cap.
 - For Azure DevOps, the PR description is capped at 4000 characters (UTF-16 code units, matching .NET's measurement): the agent is told about the cap and asked to keep the `## What Changed` section compact; if the assembled body still overruns, the `## Testing` section is dropped first because it can embed artifact and log content, preferentially preserving Intent, What Changed, Risk Assessment, and Pipeline; a final connector-level clamp truncates with a visible marker as a last-resort backstop
 
diff --git a/internal/pipeline/steps/common.go b/internal/pipeline/steps/common.go
index 97bc65629c..926f579fe4 100644
--- a/internal/pipeline/steps/common.go
+++ b/internal/pipeline/steps/common.go
@@ -311,7 +311,8 @@ var reviewFindingsSchema = json.RawMessage(`{
 					"line": {"type": "integer"},
 					"description": {"type": "string"},
 					"action": {"type": "string", "enum": ["no-op", "auto-fix", "ask-user"]},
-					"review_scope": {"type": "string", "enum": ["source", "pipeline-owned-delivery", "external-delivery"]}
+					"review_scope": {"type": "string", "enum": ["source", "pipeline-owned-delivery", "external-delivery"]},
+					"category": {"type": "string", "enum": ["intent-conformance"]}
 				},
 				"required": ["severity", "description", "action", "review_scope"]
 			}
diff --git a/internal/pipeline/steps/intent_prompt.go b/internal/pipeline/steps/intent_prompt.go
index 365f19d3d7..ff50701f47 100644
--- a/internal/pipeline/steps/intent_prompt.go
+++ b/internal/pipeline/steps/intent_prompt.go
@@ -91,7 +91,7 @@ func intentConformanceReviewClause(sctx *pipeline.StepContext) string {
 	// Pipeline-owned delivery outcomes (push, PR open/update, CI) are owned by
 	// later steps; review must not treat their absence as an intent
 	// contradiction. Source-verifiable required/forbidden behavior stays hard.
-	return "\n\nIntent conformance (required): the User intent above is authoritative acceptance criteria, not a hint. If the change contradicts it - it removes or omits a source-verifiable behavior the criteria mark as REQUIRED, or adds a behavior they mark as FORBIDDEN - you MUST emit an \"ask-user\" finding that quotes the specific criterion and the contradicting diff hunk (or, for a removed required behavior, notes what the criteria require that is now absent from the change), even if the change is otherwise risk-clean. Do not resolve such a contradiction yourself and do not classify it \"auto-fix\". Do not treat deferred pipeline-owned delivery outcomes (remote branch not yet pushed, pull request not yet opened or updated, CI not yet observed for this run) as contradictions at this phase; later pipeline steps own those. Conformance does not replace correctness review: an authoritative intent obliges flagging contradictions but never substitutes for checking that the algorithm is correct. An implementation that satisfies every required constraint can still compute a wrong value, label, or set; flag those as ordinary source findings."
+	return "\n\nIntent conformance (required): the User intent above is authoritative acceptance criteria, not a hint. If the change contradicts it - it removes or omits a source-verifiable behavior the criteria mark as REQUIRED, or adds a behavior they mark as FORBIDDEN - you MUST emit an \"ask-user\" finding with category \"intent-conformance\" that quotes the specific criterion and the contradicting diff hunk (or, for a removed required behavior, notes what the criteria require that is now absent from the change), even if the change is otherwise risk-clean. Do not resolve such a contradiction yourself and do not classify it \"auto-fix\". Set category \"intent-conformance\" only on such contradictions and omit category on every other finding. Do not treat deferred pipeline-owned delivery outcomes (remote branch not yet pushed, pull request not yet opened or updated, CI not yet observed for this run) as contradictions at this phase; later pipeline steps own those. Conformance does not replace correctness review: an authoritative intent obliges flagging contradictions but never substitutes for checking that the algorithm is correct. An implementation that satisfies every required constraint can still compute a wrong value, label, or set; flag those as ordinary source findings."
 }
 
 // cleanedUserIntent returns the trimmed, secret-redacted, adversarial-stripped
diff --git a/internal/pipeline/steps/pr.go b/internal/pipeline/steps/pr.go
index 1ad946d687..4de00f8e60 100644
--- a/internal/pipeline/steps/pr.go
+++ b/internal/pipeline/steps/pr.go
@@ -82,6 +82,9 @@ func (s *PRStep) Execute(sctx *pipeline.StepContext) (*pipeline.StepOutcome, err
 		sctx.Log(fmt.Sprintf("skipping PR creation: %v", err))
 		return &pipeline.StepOutcome{Skipped: true, SkipReason: err.Error()}, nil
 	}
+	if err := refuseUnresolvedIntentConformance(sctx); err != nil {
+		return nil, err
+	}
 
 	// Resolve the branch base so PR summaries cover the full branch delta.
 	baseSHA := resolveBranchBaseSHA(ctx, sctx.WorkDir, sctx.Run.BaseSHA, baseBranch)
@@ -406,7 +409,7 @@ func (s *PRStep) buildPipelineSection(sctx *pipeline.StepContext, provider scm.P
 	}
 
 	pipelineMD, riskLine = BuildPipelineSummaryFor(steps, rounds, sctx.Run.HeadSHA, provider)
-	testingMD = buildPRTestingSummary(steps, rounds, sctx.Repo.UpstreamURL, sctx.Run.HeadSHA, sctx.WorkDir, testEvidenceDir(sctx), publishRunEvidence(sctx), provider, s.attachRunEvidenceMedia(sctx, provider, steps, rounds))
+	testingMD = buildPRTestingSummary(steps, rounds, sctx.Repo.UpstreamURL, sctx.Run.HeadSHA, sctx.WorkDir, testEvidenceDir(sctx), publishRunEvidence(sctx), provider, s.attachRunEvidenceMedia(sctx, provider, steps, rounds), isForeignOwnerPR(sctx))
 	return pipelineMD, riskLine, testingMD
 }
 
@@ -518,9 +521,7 @@ func appendGeneratedSections(body, riskLine, testingMD, pipelineMD string) strin
 func buildPRBody(body, riskLine, testingMD, pipelineMD string, sctx *pipeline.StepContext) string {
 	body = stripGeneratedSections(body)
 	sections := appendGeneratedSectionsToCleanBody(body, riskLine, testingMD, pipelineMD)
-	// Neutralized for the same reason as in prependIntentSection: intent is
-	// agent-extracted text placed ahead of the pipeline section.
-	cleaned := neutralizeAttestationMarkers(cleanedUserIntent(sctx))
+	cleaned := prBodyIntent(sctx)
 	if cleaned == "" {
 		return sections
 	}
@@ -1275,10 +1276,7 @@ func isGeneratedSectionHeading(line string) bool {
 // rather than being paraphrased by the agent. Returns body unchanged when
 // no intent is available.
 func prependIntentSection(body string, sctx *pipeline.StepContext) string {
-	// Intent is agent-extracted text that lands ahead of the pipeline section,
-	// so it can shadow the real attestation the same way the Testing section
-	// can. See appendGeneratedSectionsToCleanBodyWithinLimit.
-	cleaned := neutralizeAttestationMarkers(cleanedUserIntent(sctx))
+	cleaned := prBodyIntent(sctx)
 	if cleaned == "" {
 		return body
 	}
@@ -1289,6 +1287,74 @@ func prependIntentSection(body string, sctx *pipeline.StepContext) string {
 	return section + "\n\n" + body
 }
 
+// prBodyIntent returns the intent text the PR body's "## Intent" section
+// carries, or "" when the body must not carry one. A pull request against a
+// repository another owner holds goes to a maintainer who never saw the run's
+// task intent, so it is omitted there (see isForeignOwnerPR).
+//
+// Intent is agent-extracted text that lands ahead of the pipeline section, so
+// it can shadow the real attestation the same way the Testing section can; it
+// is neutralized for that reason. See
+// appendGeneratedSectionsToCleanBodyWithinLimit.
+func prBodyIntent(sctx *pipeline.StepContext) string {
+	if isForeignOwnerPR(sctx) {
+		return ""
+	}
+	return neutralizeAttestationMarkers(cleanedUserIntent(sctx))
+}
+
+// isForeignOwnerPR reports whether the pull request targets a base repository
+// owned by someone other than the pushing account: a configured fork whose
+// owner differs from the parent's. Without a fork the branch is pushed to the
+// base repository itself, so the pushing account is treated as its owner.
+func isForeignOwnerPR(sctx *pipeline.StepContext) bool {
+	if sctx == nil || sctx.Repo == nil {
+		return false
+	}
+	fork := strings.TrimSpace(sctx.Repo.ForkURL)
+	if fork == "" {
+		return false
+	}
+	return !strings.EqualFold(remoteOwner(fork), remoteOwner(sctx.Repo.UpstreamURL))
+}
+
+func remoteOwner(remote string) string {
+	owner, _, _ := strings.Cut(scm.RepoPath(remote), "/")
+	return owner
+}
+
+// refuseUnresolvedIntentConformance stops the PR step while the completed
+// review still holds an intent-conformance finding. The step's findings are
+// those of its latest round, so a finding still present there was approved
+// through rather than fixed: the change contradicts the run's authoritative
+// intent, and publishing it would hand a maintainer a PR whose own review
+// says it does not do what it claims. The branch stays pushed.
+func refuseUnresolvedIntentConformance(sctx *pipeline.StepContext) error {
+	steps, err := sctx.DB.GetStepsByRun(sctx.Run.ID)
+	if err != nil {
+		return fmt.Errorf("read review findings before opening a pull request: %w", err)
+	}
+	for _, sr := range steps {
+		if sr.StepName != types.StepReview || sr.Status != types.StepStatusCompleted || sr.FindingsJSON == nil {
+			continue
+		}
+		findings, err := types.ParseFindingsJSON(*sr.FindingsJSON)
+		if err != nil {
+			return fmt.Errorf("read review findings before opening a pull request: %w", err)
+		}
+		var ids []string
+		for _, item := range findings.Items {
+			if item.Category == types.FindingCategoryIntentConformance {
+				ids = append(ids, item.ID)
+			}
+		}
+		if len(ids) > 0 {
+			return fmt.Errorf("refusing to open or update the pull request: review was approved with unresolved intent-conformance finding(s) %s - the change does not match the run's intent; fix the change or rerun with a corrected --intent (the branch stays pushed)", strings.Join(ids, ", "))
+		}
+	}
+	return nil
+}
+
 func fallbackPRContent(sctx *pipeline.StepContext, finalDiff, riskLine, testingMD, pipelineMD string, bodyLimit int) prContent {
 	title := "chore: update pull request"
 	diffSummary := strings.TrimSpace(finalDiff)
diff --git a/internal/pipeline/steps/pr_upstream_test.go b/internal/pipeline/steps/pr_upstream_test.go
new file mode 100644
index 0000000000..b0707edb35
--- /dev/null
+++ b/internal/pipeline/steps/pr_upstream_test.go
@@ -0,0 +1,195 @@
+package steps
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/kunchenguid/no-mistakes/internal/agent"
+	"github.com/kunchenguid/no-mistakes/internal/config"
+	"github.com/kunchenguid/no-mistakes/internal/pipeline"
+	"github.com/kunchenguid/no-mistakes/internal/scm"
+	"github.com/kunchenguid/no-mistakes/internal/types"
+)
+
+const (
+	upstreamTestIntent     = "stale task intent the maintainer never saw"
+	upstreamTestInlineText = "inline evidence: checkout returned 200"
+	upstreamTestEmbedText  = "embedded evidence file body"
+)
+
+// newUpstreamBodyContext builds a PR step context whose Test step recorded a
+// path-only local artifact, a captioned local artifact, an embeddable evidence
+// file, and an inline content artifact, plus an authoritative intent.
+func newUpstreamBodyContext(t *testing.T, upstreamURL, forkURL string) (*pipeline.StepContext, string) {
+	t.Helper()
+	dir, baseSHA, headSHA := setupGitRepo(t)
+	ag := &mockAgent{
+		name: "test",
+		runFn: func(ctx context.Context, opts agent.RunOpts) (*agent.Result, error) {
+			return &agent.Result{Output: json.RawMessage(`{"title":"fix: tidy","body":"## What Changed\n\n- tidy"}`)}, nil
+		},
+	}
+	sctx := newTestContextWithDBRecords(t, ag, dir, baseSHA, headSHA, config.Commands{})
+	sctx.Repo.UpstreamURL = upstreamURL
+	sctx.Repo.ForkURL = forkURL
+	sctx.UserIntent = upstreamTestIntent
+	sctx.EvidenceDir = t.TempDir()
+
+	localOnly := filepath.Join(sctx.EvidenceDir, "missing.bin")
+	captioned := filepath.Join(sctx.EvidenceDir, "missing-captioned.bin")
+	embedded := filepath.Join(sctx.EvidenceDir, "server.log")
+	if err := os.WriteFile(embedded, []byte(upstreamTestEmbedText), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	findings := fmt.Sprintf(`{"findings":[],"summary":"","testing_summary":"Evidence was collected.","artifacts":[`+
+		`{"kind":"log","label":"Local only log","path":%q},`+
+		`{"kind":"log","label":"Captioned log","path":%q,"content":"captioned evidence text"},`+
+		`{"kind":"log","label":"Server log","path":%q},`+
+		`{"kind":"log","label":"Inline log","content":%q}]}`, localOnly, captioned, embedded, upstreamTestInlineText)
+	testStep, err := sctx.DB.InsertStepResult(sctx.Run.ID, types.StepTest)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := sctx.DB.UpdateStepStatus(testStep.ID, types.StepStatusCompleted); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := sctx.DB.InsertStepRound(testStep.ID, 1, "initial", &findings, nil, 300); err != nil {
+		t.Fatal(err)
+	}
+	return sctx, baseSHA
+}
+
+func buildUpstreamTestBody(t *testing.T, sctx *pipeline.StepContext, baseSHA string, bodyLimit int) string {
+	t.Helper()
+	content, err := (&PRStep{}).buildPRContent(sctx, "feature", "main", baseSHA, scm.ProviderGitHub, bodyLimit)
+	if err != nil {
+		t.Fatal(err)
+	}
+	return content.Body
+}
+
+func TestPRBody_ForeignOwnerOmitsIntentAndLocalPathEvidence(t *testing.T) {
+	t.Parallel()
+	for _, limit := range []int{0, 60000} {
+		t.Run(fmt.Sprintf("limit_%d", limit), func(t *testing.T) {
+			t.Parallel()
+			sctx, baseSHA := newUpstreamBodyContext(t, "https://github.com/parent-owner/widgets.git", "git@github.com:fork-owner/widgets.git")
+			body := buildUpstreamTestBody(t, sctx, baseSHA, limit)
+
+			for _, unwanted := range []string{"## Intent", upstreamTestIntent, "local file:", "missing.bin", "missing-captioned.bin"} {
+				if strings.Contains(body, unwanted) {
+					t.Fatalf("foreign-owner PR body must not contain %q, got:\n%s", unwanted, body)
+				}
+			}
+			for _, want := range []string{"## What Changed", "## Testing", upstreamTestInlineText, "captioned evidence text", upstreamTestEmbedText, pipelineAttestationCommentPrefix} {
+				if !strings.Contains(body, want) {
+					t.Fatalf("foreign-owner PR body missing %q, got:\n%s", want, body)
+				}
+			}
+		})
+	}
+}
+
+func TestPRBody_SameOwnerKeepsIntentAndLocalPathEvidence(t *testing.T) {
+	t.Parallel()
+	for _, limit := range []int{0, 60000} {
+		t.Run(fmt.Sprintf("limit_%d", limit), func(t *testing.T) {
+			t.Parallel()
+			sctx, baseSHA := newUpstreamBodyContext(t, "https://github.com/owner/widgets.git", "")
+			noFork := buildUpstreamTestBody(t, sctx, baseSHA, limit)
+			for _, want := range []string{"## Intent\n\n" + upstreamTestIntent, "local file:", "missing.bin", upstreamTestInlineText, pipelineAttestationCommentPrefix} {
+				if !strings.Contains(noFork, want) {
+					t.Fatalf("same-owner PR body missing %q, got:\n%s", want, noFork)
+				}
+			}
+
+			// A fork registered under the same owner (any case) is not a
+			// foreign-owner PR, so its body is byte-identical.
+			sctx.Repo.ForkURL = "git@github.com:OWNER/widgets.git"
+			if sameOwnerFork := buildUpstreamTestBody(t, sctx, baseSHA, limit); sameOwnerFork != noFork {
+				t.Fatalf("same-owner fork body differs from no-fork body\nno fork:\n%s\nsame-owner fork:\n%s", noFork, sameOwnerFork)
+			}
+		})
+	}
+}
+
+func insertCompletedReview(t *testing.T, sctx *pipeline.StepContext, findings string) {
+	t.Helper()
+	review, err := sctx.DB.InsertStepResult(sctx.Run.ID, types.StepReview)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := sctx.DB.UpdateStepStatus(review.ID, types.StepStatusCompleted); err != nil {
+		t.Fatal(err)
+	}
+	if findings == "" {
+		return
+	}
+	if err := sctx.DB.SetStepFindings(review.ID, findings); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestPRStep_RefusesWhileApprovedIntentConformanceFindingIsUnresolved(t *testing.T) {
+	t.Parallel()
+	dir, baseSHA, headSHA := setupGitRepo(t)
+	env, logFile := fakeGH(t, "")
+	sctx := newTestContextWithDBRecords(t, &mockAgent{name: "test"}, dir, baseSHA, headSHA, config.Commands{})
+	sctx.Env = env
+	insertCompletedReview(t, sctx, `{"findings":[{"id":"review-1","severity":"error","description":"range does not contain the change the intent marks as its subject","action":"ask-user","review_scope":"source","category":"intent-conformance"}],"summary":"intent mismatch","risk_level":"high","risk_rationale":"needs maintainer decision"}`)
+
+	_, err := (&PRStep{}).Execute(sctx)
+	if err == nil {
+		t.Fatal("expected PR step to refuse while an approved intent-conformance finding is unresolved")
+	}
+	if !strings.Contains(err.Error(), "intent") || !strings.Contains(err.Error(), "review-1") {
+		t.Fatalf("refusal must name the intent-conformance finding, got: %v", err)
+	}
+	if data, readErr := os.ReadFile(logFile); readErr == nil && strings.Contains(string(data), "pr create") {
+		t.Fatalf("refused PR step must not create a PR, gh log:\n%s", data)
+	}
+}
+
+func TestPRStep_ProceedsWhenReviewHoldsNoIntentConformanceFinding(t *testing.T) {
+	t.Parallel()
+	dir, baseSHA, headSHA := setupGitRepo(t)
+	env, logFile := fakeGH(t, "")
+	ag := &mockAgent{
+		name: "test",
+		runFn: func(ctx context.Context, opts agent.RunOpts) (*agent.Result, error) {
+			return &agent.Result{Output: json.RawMessage(`{"title":"fix: tidy","body":"## What Changed\n\n- tidy"}`)}, nil
+		},
+	}
+	sctx := newTestContextWithDBRecords(t, ag, dir, baseSHA, headSHA, config.Commands{})
+	sctx.Env = env
+	insertCompletedReview(t, sctx, `{"findings":[{"id":"review-1","severity":"warning","description":"naming nit","action":"ask-user","review_scope":"source"}],"summary":"nit","risk_level":"low","risk_rationale":"small"}`)
+
+	if _, err := (&PRStep{}).Execute(sctx); err != nil {
+		t.Fatalf("PR step must proceed when no intent-conformance finding is held, got: %v", err)
+	}
+	data, err := os.ReadFile(logFile)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !strings.Contains(string(data), "pr create") {
+		t.Fatalf("expected PR creation, gh log:\n%s", data)
+	}
+}
+
+// The PR-step refusal keys on the structured category, so the reviewer must be
+// told to emit it and the schema must accept it.
+func TestReviewIntentConformanceCategoryIsRequestedAndAccepted(t *testing.T) {
+	t.Parallel()
+	sctx := &pipeline.StepContext{UserIntent: "add a Bar() helper", IntentSource: "agent"}
+	if clause := intentConformanceReviewClause(sctx); !strings.Contains(clause, `category "intent-conformance"`) {
+		t.Fatalf("conformance clause must request the intent-conformance category, got:\n%s", clause)
+	}
+	if !strings.Contains(string(reviewFindingsSchema), `"category": {"type": "string", "enum": ["`+types.FindingCategoryIntentConformance+`"]}`) {
+		t.Fatalf("review findings schema must accept the intent-conformance category:\n%s", reviewFindingsSchema)
+	}
+}
diff --git a/internal/pipeline/steps/prsummary.go b/internal/pipeline/steps/prsummary.go
index b346be7052..4cd544a1ed 100644
--- a/internal/pipeline/steps/prsummary.go
+++ b/internal/pipeline/steps/prsummary.go
@@ -96,6 +96,10 @@ type testingSummaryOptions struct {
 	// uploaded at PR render time. Nil means nothing was uploaded; the renderer
 	// then keeps today's local-path or commit-pinned link.
 	attachments map[string]string
+	// omitLocalPaths drops local-file references a reader cannot open: a PR
+	// against a repository another owner holds (see isForeignOwnerPR). An
+	// artifact with nothing else to show is omitted; inline text is kept.
+	omitLocalPaths bool
 }
 
 // BuildPipelineSummary produces a deterministic markdown section from step results and rounds.
@@ -298,10 +302,10 @@ func BuildTestingSummaryForPR(steps []*db.StepResult, rounds map[string][]*db.St
 }
 
 func BuildTestingSummaryForPRWithProvider(steps []*db.StepResult, rounds map[string][]*db.StepRound, upstreamURL, ref, repoRoot, evidenceRoot string, links *evidenceLinks, provider scm.Provider) string {
-	return buildPRTestingSummary(steps, rounds, upstreamURL, ref, repoRoot, evidenceRoot, links, provider, nil)
+	return buildPRTestingSummary(steps, rounds, upstreamURL, ref, repoRoot, evidenceRoot, links, provider, nil, false)
 }
 
-func buildPRTestingSummary(steps []*db.StepResult, rounds map[string][]*db.StepRound, upstreamURL, ref, repoRoot, evidenceRoot string, links *evidenceLinks, provider scm.Provider, attachments map[string]string) string {
+func buildPRTestingSummary(steps []*db.StepResult, rounds map[string][]*db.StepRound, upstreamURL, ref, repoRoot, evidenceRoot string, links *evidenceLinks, provider scm.Provider, attachments map[string]string, omitLocalPaths bool) string {
 	opts := testingSummaryOptionsForGitHub(upstreamURL, ref)
 	opts.flavor = prBodyFlavorFor(provider)
 	opts.compactArtifacts = true
@@ -311,6 +315,7 @@ func buildPRTestingSummary(steps []*db.StepResult, rounds map[string][]*db.StepR
 	opts.evidenceRoot = evidenceRoot
 	opts.evidence = links
 	opts.attachments = attachments
+	opts.omitLocalPaths = omitLocalPaths
 	return buildTestingSummary(steps, rounds, opts)
 }
 
@@ -944,7 +949,7 @@ func repoRelativeArtifactPath(target string, opts testingSummaryOptions) string
 }
 
 func localArtifactPath(target string, opts testingSummaryOptions) string {
-	if target == "" || !filepath.IsAbs(target) {
+	if opts.omitLocalPaths || target == "" || !filepath.IsAbs(target) {
 		return ""
 	}
 	if _, ok := artifactPathRelativeToRoot(target, opts.repoRoot); ok {
diff --git a/internal/types/findings.go b/internal/types/findings.go
index 4c0c996143..8ca7d87a67 100644
--- a/internal/types/findings.go
+++ b/internal/types/findings.go
@@ -103,6 +103,12 @@ const (
 	FindingCategoryCIReviewBot     = "ci-review-bot"
 )
 
+// FindingCategoryIntentConformance marks a review finding that the change
+// contradicts the run's authoritative intent (see the review prompt's intent
+// conformance clause). The PR step refuses to open or update a pull request
+// while a completed review still holds one: it was approved, not fixed.
+const FindingCategoryIntentConformance = "intent-conformance"
+
 // Test scenario result constants: the vocabulary the test step's evidence
 // prompt instructs the agent to use for each derived scenario.
 //
@@ -172,8 +178,9 @@ type Finding struct {
 	UserInstructions string `json:"user_instructions,omitempty"`
 	ReviewScope      string `json:"review_scope,omitempty"`
 	// Category separates the combined document+lint housekeeping pass's
-	// findings into their owning gates and the CI step's findings by kind
-	// (see the FindingCategoryCI* constants). Empty everywhere else.
+	// findings into their owning gates, the CI step's findings by kind (see
+	// the FindingCategoryCI* constants), and flags review intent-conformance
+	// findings (FindingCategoryIntentConformance). Empty everywhere else.
 	Category string `json:"category,omitempty"`
 	// Check is the provider check name a CI finding was derived from. CheckID
 	// is the provider's opaque identity for that exact check, so same-named

From fcb4f25d66ac2c27e9dd9dd8cf7a4351db39f670 Mon Sep 17 00:00:00 2001
From: Doug Mealing <doug@dougmealing.com>
Date: Sat, 3 Oct 2026 11:13:57 -0400
Subject: [PATCH 2/5] test(pipeline): share the PR drafting stub across
 foreign-owner tests

---
 internal/pipeline/steps/pr_upstream_test.go | 25 +++++++++------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/internal/pipeline/steps/pr_upstream_test.go b/internal/pipeline/steps/pr_upstream_test.go
index b0707edb35..fb9b3add90 100644
--- a/internal/pipeline/steps/pr_upstream_test.go
+++ b/internal/pipeline/steps/pr_upstream_test.go
@@ -22,19 +22,22 @@ const (
 	upstreamTestEmbedText  = "embedded evidence file body"
 )
 
+func tidyPRAgent() *mockAgent {
+	return &mockAgent{
+		name: "test",
+		runFn: func(ctx context.Context, opts agent.RunOpts) (*agent.Result, error) {
+			return &agent.Result{Output: json.RawMessage(`{"title":"fix: tidy","body":"## What Changed\n\n- tidy"}`)}, nil
+		},
+	}
+}
+
 // newUpstreamBodyContext builds a PR step context whose Test step recorded a
 // path-only local artifact, a captioned local artifact, an embeddable evidence
 // file, and an inline content artifact, plus an authoritative intent.
 func newUpstreamBodyContext(t *testing.T, upstreamURL, forkURL string) (*pipeline.StepContext, string) {
 	t.Helper()
 	dir, baseSHA, headSHA := setupGitRepo(t)
-	ag := &mockAgent{
-		name: "test",
-		runFn: func(ctx context.Context, opts agent.RunOpts) (*agent.Result, error) {
-			return &agent.Result{Output: json.RawMessage(`{"title":"fix: tidy","body":"## What Changed\n\n- tidy"}`)}, nil
-		},
-	}
-	sctx := newTestContextWithDBRecords(t, ag, dir, baseSHA, headSHA, config.Commands{})
+	sctx := newTestContextWithDBRecords(t, tidyPRAgent(), dir, baseSHA, headSHA, config.Commands{})
 	sctx.Repo.UpstreamURL = upstreamURL
 	sctx.Repo.ForkURL = forkURL
 	sctx.UserIntent = upstreamTestIntent
@@ -159,13 +162,7 @@ func TestPRStep_ProceedsWhenReviewHoldsNoIntentConformanceFinding(t *testing.T)
 	t.Parallel()
 	dir, baseSHA, headSHA := setupGitRepo(t)
 	env, logFile := fakeGH(t, "")
-	ag := &mockAgent{
-		name: "test",
-		runFn: func(ctx context.Context, opts agent.RunOpts) (*agent.Result, error) {
-			return &agent.Result{Output: json.RawMessage(`{"title":"fix: tidy","body":"## What Changed\n\n- tidy"}`)}, nil
-		},
-	}
-	sctx := newTestContextWithDBRecords(t, ag, dir, baseSHA, headSHA, config.Commands{})
+	sctx := newTestContextWithDBRecords(t, tidyPRAgent(), dir, baseSHA, headSHA, config.Commands{})
 	sctx.Env = env
 	insertCompletedReview(t, sctx, `{"findings":[{"id":"review-1","severity":"warning","description":"naming nit","action":"ask-user","review_scope":"source"}],"summary":"nit","risk_level":"low","risk_rationale":"small"}`)
 

From e118cf55d0d02d555185e45c4c9c47773b3a7f2f Mon Sep 17 00:00:00 2001
From: Doug Mealing <doug@dougmealing.com>
Date: Sat, 3 Oct 2026 11:14:09 -0400
Subject: [PATCH 3/5] docs(agents): record foreign-owner PR body and
 intent-mismatch refusal owners

---
 AGENTS.md | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/AGENTS.md b/AGENTS.md
index 5543d822de..e405f0591c 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -94,6 +94,11 @@ Safest local verification sequence after non-trivial changes:
 - The override reason also rides the `run_completed` IPC delta (`ipc.Event.CIOverrideReason`, derived in `emitRunEvent` from step rows and gated on `run.Status == RunCompleted`), so an attached TUI's `renderOutcomeBanner` shows `⚠ Pipeline passed with override` on the live event path without waiting for a snapshot - otherwise the banner disagrees with axi's `passed-with-override`.
 - Tests: `internal/pipeline/executor_approval_override_test.go` (incl. `TestExecutor_ApprovalOverride_RunCompletedEventCarriesReason`), `internal/pipeline/steps/ci_approval_override_test.go`, `internal/cli/axi_test.go` (`TestOutcomeForRun`), `internal/tui/action_bar_test.go` (`TestOutcomeBanner_CIOverrideShowsReason`), `internal/tui/events_test.go` (`TestModel_ApplyEvent_RunCompletedCarriesCIOverride`).
 
+**Foreign-Owner PR Bodies and Approved Intent Mismatch (`internal/pipeline/steps/pr.go`)**
+
+- `isForeignOwnerPR` (fork owner differs from the parent owner) is the single switch that drops the `## Intent` section (`prBodyIntent`) and local-path evidence (`testingSummaryOptions.omitLocalPaths`) from generated PR bodies; same-owner bodies must stay byte-identical and the attestation marker is never touched.
+- Review tags intent contradictions with the structured `category: intent-conformance` (`types.FindingCategoryIntentConformance`); `refuseUnresolvedIntentConformance` fails the PR step while a completed review still holds one. Key on the category, never on finding prose. Regressions: `internal/pipeline/steps/pr_upstream_test.go`.
+
 **Stale Actions Event Replay Can Resurrect a Superseded Check (`require-no-mistakes`)**
 
 - `.github/actions/require-no-mistakes/verify.py` reads the PR body/head SHA it verifies from a **live** GitHub REST API lookup first (via `live_pr_facts`, a direct `urllib.request` GET to `{GITHUB_API_URL}/repos/{repo}/pulls/{number}` with the forwarded `github-token` as a Bearer token, no `gh` CLI), not from `GITHUB_EVENT_PATH`, whenever a caller forwards no explicit `pr-body`/`pr-head-sha` (the documented zero-input integration every caller actually uses). A GitHub Actions job **rerun** replays the event payload archived at the run's *original* trigger rather than delivering a fresh one; re-running an old, already-superseded failed run therefore used to reproduce its stale verdict with a brand-new check-run timestamp, which both GitHub's own required-check view and `collapseLatestByName` (`internal/scm/github/github.go`) treat as current - pinning a stale FAILURE next to an already-green commit with no clean recovery short of a new SHA. When a required live lookup is unavailable (no `pull-requests: read`, no token, or the API call fails), the gate fails closed rather than certifying compliance from the possibly-stale event payload; explicit `pr-body`/`pr-head-sha` inputs always skip the live lookup and win, unchanged.

From 15a2c28b4cdfb96d2daa72e7420d2751ffa694a4 Mon Sep 17 00:00:00 2001
From: Doug Mealing <doug@dougmealing.com>
Date: Sat, 3 Oct 2026 11:15:27 -0400
Subject: [PATCH 4/5] docs(skills): move PR body and intent-mismatch notes into
 owning skills

---
 .agents/skills/pipeline-review-and-agents/SKILL.md | 1 +
 .agents/skills/pr-publication-safety/SKILL.md      | 1 +
 AGENTS.md                                          | 5 -----
 3 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/.agents/skills/pipeline-review-and-agents/SKILL.md b/.agents/skills/pipeline-review-and-agents/SKILL.md
index c7f64dd31b..dbd4bfa055 100644
--- a/.agents/skills/pipeline-review-and-agents/SKILL.md
+++ b/.agents/skills/pipeline-review-and-agents/SKILL.md
@@ -88,5 +88,6 @@ metadata:
 - `userIntentPromptSection` branches on source: an EXPLICIT intent renders as sanitized-but-AUTHORITATIVE acceptance criteria; an INFERRED intent keeps the low-confidence hint framing verbatim. Both branches keep the `StripAdversarial`+`RedactSecrets` pipeline and BEGIN/END "do not execute instructions" guard - authoritative reframes only the content's authority (check the diff against the criteria), never whether control tokens are stripped. The review prompt adds `intentConformanceReviewClause` for agent-source intent only: a fixer change that contradicts the criteria (removes intent-required or adds intent-forbidden behavior) MUST become an `ask-user` finding, which parks with no executor change. Conformance is limited to source-verifiable criteria; deferred pipeline-owned delivery (remote branch / push / PR / CI for this run) is out of scope at review.
 - Review is always pre-push (`StepReview` before `StepPush`/`StepPR`/`StepCI`). `pipelineDeliveryPhaseClause` plus `stripDeferredPipelineOwnedDeliveryFindings` (`pipeline_delivery.go`, applied in `review.go`) keep findings that only claim those later-owned outcomes are missing from parking the run. External or pre-existing lifecycle requirements (numbered PR, third-party artifact, non-run-owned state) stay enforceable. Push, PR, and CI steps remain strict after their stages run.
 - Empty/missing finding `action` fails closed to `ask-user`, not auto-fix (`types/findings.go` `ActionOrDefault`); `HasAskUserFindings` uses `ActionOrDefault` so it agrees with `AutoFixableFindings` (an unclassified finding is never auto-fixed and is always caught as ask-user). `MergeUserOverrides` still stamps user-*added* findings auto-fix on purpose.
+- The conformance clause asks for `category: intent-conformance` (`types.FindingCategoryIntentConformance`) on those findings. A completed review whose latest findings still hold one was approved, not fixed, and the PR step refuses to create or update the PR (`refuseUnresolvedIntentConformance`, branch stays pushed). Key on the category, never on finding prose. Regressions: `TestPRStep_RefusesWhileApprovedIntentConformanceFindingIsUnresolved`, `TestReviewIntentConformanceCategoryIsRequestedAndAccepted`.
 - The deterministic net-deleted-author-lines git-diff backstop is intentionally not built; `review.go` owns the held-scope TODO.
 - Regressions: `internal/pipeline/steps/intent_prompt_test.go`, `internal/pipeline/steps/review_test.go` (`TestReviewStep_ConformanceObligationTracksIntentProvenance`, `TestReviewStep_RereviewFlagsIntentContradictionAsAskUser`), `internal/pipeline/steps/pipeline_delivery_test.go`, `internal/pipeline/steps/review_pipeline_delivery_test.go`, `internal/pipeline/executor_intent_conformance_test.go`, `internal/types/findings_test.go`, e2e `TestIntentJourney` (inferred-source framing), e2e `TestReviewPipelineOwnedPRCriterionDoesNotPark` / `TestReviewExternalPRLifecycleStillParks`.
diff --git a/.agents/skills/pr-publication-safety/SKILL.md b/.agents/skills/pr-publication-safety/SKILL.md
index 348af18d49..7a8466e8dd 100644
--- a/.agents/skills/pr-publication-safety/SKILL.md
+++ b/.agents/skills/pr-publication-safety/SKILL.md
@@ -15,4 +15,5 @@ metadata:
 - The PR body must contain exactly ONE live pipeline-attestation marker, the run's own. `require-no-mistakes` (`.github/actions/require-no-mistakes/verify.py`) binds the FIRST marker in the RAW body to the PR head, so a foreign copy placed earlier fails a PR the pipeline did produce - and a code fence is no defense, because that scan is raw text. Step agents embed foreign markers routinely, by capturing a generated PR body as evidence.
 - A CI repair that publishes a new head rewrites only that live marker's `head_sha` in the current PR body (`restampPublishedAttestation`) and does not send a title. It never inserts a marker that was not already there. Hosts without a PR content reader skip the restamp instead of failing the push. Regressions: `TestCIStep_PublishRepairRebindsAttestationAcrossRepairPushes`, `TestCIStep_PublishRepairDoesNotMintAttestation`, `TestCIStep_PublishRepairSkipsRestampWithoutReader`, `TestRestampPRAttestation_PreservesContentEditedWhilePreparingRewrite`, `TestUpdatePROmitsTitleWhenEmpty`.
 - Neutralize at the assembly choke point (`appendGeneratedSectionsToCleanBodyWithinLimit` plus the two intent paths), never per render path. `pipelineMD` alone carries the real marker and is left intact; `BuildPipelineSummaryFor` neutralizes its own step-detail blocks, which quote agent text. A first attempt put this in `escapePipelineFoldMarkers` - per-render-path - and shipped three live foreign markers to #831 anyway. Regressions: `TestPRStep_ForeignAttestationsInEveryComponentDoNotShadowTheRealOne` (all components at once), plus the per-component guards in `pr_test.go`.
+- `isForeignOwnerPR` (`pr.go`: a fork whose owner differs from the parent's) is the single switch that drops the `## Intent` section (`prBodyIntent`, which both intent paths share) and local-path evidence references (`testingSummaryOptions.omitLocalPaths`) from the body; inline evidence text, links, and attachments stay. Same-owner bodies must stay byte-identical. Regressions: `TestPRBody_ForeignOwnerOmitsIntentAndLocalPathEvidence`, `TestPRBody_SameOwnerKeepsIntentAndLocalPathEvidence`.
 - Regressions: `internal/safepath/redact_test.go`, `internal/pipeline/steps/pr_homepath_test.go`.
diff --git a/AGENTS.md b/AGENTS.md
index e405f0591c..5543d822de 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -94,11 +94,6 @@ Safest local verification sequence after non-trivial changes:
 - The override reason also rides the `run_completed` IPC delta (`ipc.Event.CIOverrideReason`, derived in `emitRunEvent` from step rows and gated on `run.Status == RunCompleted`), so an attached TUI's `renderOutcomeBanner` shows `⚠ Pipeline passed with override` on the live event path without waiting for a snapshot - otherwise the banner disagrees with axi's `passed-with-override`.
 - Tests: `internal/pipeline/executor_approval_override_test.go` (incl. `TestExecutor_ApprovalOverride_RunCompletedEventCarriesReason`), `internal/pipeline/steps/ci_approval_override_test.go`, `internal/cli/axi_test.go` (`TestOutcomeForRun`), `internal/tui/action_bar_test.go` (`TestOutcomeBanner_CIOverrideShowsReason`), `internal/tui/events_test.go` (`TestModel_ApplyEvent_RunCompletedCarriesCIOverride`).
 
-**Foreign-Owner PR Bodies and Approved Intent Mismatch (`internal/pipeline/steps/pr.go`)**
-
-- `isForeignOwnerPR` (fork owner differs from the parent owner) is the single switch that drops the `## Intent` section (`prBodyIntent`) and local-path evidence (`testingSummaryOptions.omitLocalPaths`) from generated PR bodies; same-owner bodies must stay byte-identical and the attestation marker is never touched.
-- Review tags intent contradictions with the structured `category: intent-conformance` (`types.FindingCategoryIntentConformance`); `refuseUnresolvedIntentConformance` fails the PR step while a completed review still holds one. Key on the category, never on finding prose. Regressions: `internal/pipeline/steps/pr_upstream_test.go`.
-
 **Stale Actions Event Replay Can Resurrect a Superseded Check (`require-no-mistakes`)**
 
 - `.github/actions/require-no-mistakes/verify.py` reads the PR body/head SHA it verifies from a **live** GitHub REST API lookup first (via `live_pr_facts`, a direct `urllib.request` GET to `{GITHUB_API_URL}/repos/{repo}/pulls/{number}` with the forwarded `github-token` as a Bearer token, no `gh` CLI), not from `GITHUB_EVENT_PATH`, whenever a caller forwards no explicit `pr-body`/`pr-head-sha` (the documented zero-input integration every caller actually uses). A GitHub Actions job **rerun** replays the event payload archived at the run's *original* trigger rather than delivering a fresh one; re-running an old, already-superseded failed run therefore used to reproduce its stale verdict with a brand-new check-run timestamp, which both GitHub's own required-check view and `collapseLatestByName` (`internal/scm/github/github.go`) treat as current - pinning a stale FAILURE next to an already-green commit with no clean recovery short of a new SHA. When a required live lookup is unavailable (no `pull-requests: read`, no token, or the API call fails), the gate fails closed rather than certifying compliance from the possibly-stale event payload; explicit `pr-body`/`pr-head-sha` inputs always skip the live lookup and win, unchanged.

From bc5251d6a0de61abd1757a42fbca9a761bf498fb Mon Sep 17 00:00:00 2001
From: Doug Mealing <doug@dougmealing.com>
Date: Sat, 3 Oct 2026 11:33:29 -0400
Subject: [PATCH 5/5] test(pipeline): reuse step-seeding helpers and parse the
 review schema in foreign-owner tests

---
 internal/pipeline/steps/pr_upstream_test.go | 93 +++++++++++++--------
 1 file changed, 56 insertions(+), 37 deletions(-)

diff --git a/internal/pipeline/steps/pr_upstream_test.go b/internal/pipeline/steps/pr_upstream_test.go
index fb9b3add90..3ecbc619d5 100644
--- a/internal/pipeline/steps/pr_upstream_test.go
+++ b/internal/pipeline/steps/pr_upstream_test.go
@@ -49,21 +49,16 @@ func newUpstreamBodyContext(t *testing.T, upstreamURL, forkURL string) (*pipelin
 	if err := os.WriteFile(embedded, []byte(upstreamTestEmbedText), 0o644); err != nil {
 		t.Fatal(err)
 	}
-	findings := fmt.Sprintf(`{"findings":[],"summary":"","testing_summary":"Evidence was collected.","artifacts":[`+
-		`{"kind":"log","label":"Local only log","path":%q},`+
-		`{"kind":"log","label":"Captioned log","path":%q,"content":"captioned evidence text"},`+
-		`{"kind":"log","label":"Server log","path":%q},`+
-		`{"kind":"log","label":"Inline log","content":%q}]}`, localOnly, captioned, embedded, upstreamTestInlineText)
-	testStep, err := sctx.DB.InsertStepResult(sctx.Run.ID, types.StepTest)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if err := sctx.DB.UpdateStepStatus(testStep.ID, types.StepStatusCompleted); err != nil {
-		t.Fatal(err)
-	}
-	if _, err := sctx.DB.InsertStepRound(testStep.ID, 1, "initial", &findings, nil, 300); err != nil {
-		t.Fatal(err)
-	}
+	findings := findingsJSON(t, types.Findings{
+		TestingSummary: "Evidence was collected.",
+		Artifacts: []types.TestArtifact{
+			{Kind: "log", Label: "Local only log", Path: localOnly},
+			{Kind: "log", Label: "Captioned log", Path: captioned, Content: "captioned evidence text"},
+			{Kind: "log", Label: "Server log", Path: embedded},
+			{Kind: "log", Label: "Inline log", Content: upstreamTestInlineText},
+		},
+	})
+	insertCompletedStep(t, sctx, types.StepTest, findings, "")
 	return sctx, baseSHA
 }
 
@@ -121,30 +116,25 @@ func TestPRBody_SameOwnerKeepsIntentAndLocalPathEvidence(t *testing.T) {
 	}
 }
 
-func insertCompletedReview(t *testing.T, sctx *pipeline.StepContext, findings string) {
-	t.Helper()
-	review, err := sctx.DB.InsertStepResult(sctx.Run.ID, types.StepReview)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if err := sctx.DB.UpdateStepStatus(review.ID, types.StepStatusCompleted); err != nil {
-		t.Fatal(err)
-	}
-	if findings == "" {
-		return
-	}
-	if err := sctx.DB.SetStepFindings(review.ID, findings); err != nil {
-		t.Fatal(err)
-	}
-}
-
 func TestPRStep_RefusesWhileApprovedIntentConformanceFindingIsUnresolved(t *testing.T) {
 	t.Parallel()
 	dir, baseSHA, headSHA := setupGitRepo(t)
 	env, logFile := fakeGH(t, "")
 	sctx := newTestContextWithDBRecords(t, &mockAgent{name: "test"}, dir, baseSHA, headSHA, config.Commands{})
 	sctx.Env = env
-	insertCompletedReview(t, sctx, `{"findings":[{"id":"review-1","severity":"error","description":"range does not contain the change the intent marks as its subject","action":"ask-user","review_scope":"source","category":"intent-conformance"}],"summary":"intent mismatch","risk_level":"high","risk_rationale":"needs maintainer decision"}`)
+	insertCompletedStep(t, sctx, types.StepReview, findingsJSON(t, types.Findings{
+		Items: []types.Finding{{
+			ID:          "review-1",
+			Severity:    "error",
+			Description: "range does not contain the change the intent marks as its subject",
+			Action:      "ask-user",
+			ReviewScope: "source",
+			Category:    types.FindingCategoryIntentConformance,
+		}},
+		Summary:       "intent mismatch",
+		RiskLevel:     "high",
+		RiskRationale: "needs maintainer decision",
+	}), "")
 
 	_, err := (&PRStep{}).Execute(sctx)
 	if err == nil {
@@ -164,7 +154,18 @@ func TestPRStep_ProceedsWhenReviewHoldsNoIntentConformanceFinding(t *testing.T)
 	env, logFile := fakeGH(t, "")
 	sctx := newTestContextWithDBRecords(t, tidyPRAgent(), dir, baseSHA, headSHA, config.Commands{})
 	sctx.Env = env
-	insertCompletedReview(t, sctx, `{"findings":[{"id":"review-1","severity":"warning","description":"naming nit","action":"ask-user","review_scope":"source"}],"summary":"nit","risk_level":"low","risk_rationale":"small"}`)
+	insertCompletedStep(t, sctx, types.StepReview, findingsJSON(t, types.Findings{
+		Items: []types.Finding{{
+			ID:          "review-1",
+			Severity:    "warning",
+			Description: "naming nit",
+			Action:      "ask-user",
+			ReviewScope: "source",
+		}},
+		Summary:       "nit",
+		RiskLevel:     "low",
+		RiskRationale: "small",
+	}), "")
 
 	if _, err := (&PRStep{}).Execute(sctx); err != nil {
 		t.Fatalf("PR step must proceed when no intent-conformance finding is held, got: %v", err)
@@ -186,7 +187,25 @@ func TestReviewIntentConformanceCategoryIsRequestedAndAccepted(t *testing.T) {
 	if clause := intentConformanceReviewClause(sctx); !strings.Contains(clause, `category "intent-conformance"`) {
 		t.Fatalf("conformance clause must request the intent-conformance category, got:\n%s", clause)
 	}
-	if !strings.Contains(string(reviewFindingsSchema), `"category": {"type": "string", "enum": ["`+types.FindingCategoryIntentConformance+`"]}`) {
-		t.Fatalf("review findings schema must accept the intent-conformance category:\n%s", reviewFindingsSchema)
-	}
+	// Assert on the parsed schema, not the literal's formatting: a reflow of
+	// the JSON string must not decide whether the contract holds.
+	var schema map[string]any
+	if err := json.Unmarshal(reviewFindingsSchema, &schema); err != nil {
+		t.Fatalf("review findings schema is not valid JSON: %v", err)
+	}
+	properties, _ := schema["properties"].(map[string]any)
+	findingsProp, _ := properties["findings"].(map[string]any)
+	items, _ := findingsProp["items"].(map[string]any)
+	itemProperties, _ := items["properties"].(map[string]any)
+	categoryProp, _ := itemProperties["category"].(map[string]any)
+	enum, ok := categoryProp["enum"].([]any)
+	if !ok {
+		t.Fatalf("review findings schema finding-item category has no enum: %#v", itemProperties)
+	}
+	for _, value := range enum {
+		if value == types.FindingCategoryIntentConformance {
+			return
+		}
+	}
+	t.Fatalf("review findings schema category enum must accept %q, got: %v", types.FindingCategoryIntentConformance, enum)
 }