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/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 (`
`, ``, `